2131530446

Committed 31 Oct 2025 09:39AM UTC coverage: 79.867%. First build

Build # 2131530446

Build Type

push

gitlab-ci

Committed by

lluiscampos

Commit Message

feat: Error out on comma in NO_PROXY

Instead of warning when the NO_PROXY environment variable contains a
comma, error out. The warning was easy to miss, and this is an invalid
configuration that should be reported as an error.

The error message is kept generic to apply to all mender clients that
use this shared code.

Changelog: Mender now errors if a comma is found in the NO_PROXY environment variable and explains that it only accepts space separated values.
Ticket: ME-586

Signed-off-by: Nick Anderson <nick@cmdln.org>

Run Details

0 of 2 new or added lines in 1 file covered. (0.0%)

7811 of 9780 relevant lines covered (79.87%)

14030.28 hits per line

Source File
Press 'n' to go to next uncovered line, 'b' for previous

85.23

/src/client_shared/conf/conf.cpp

// Copyright 2023 Northern.tech AS
//
//    Licensed under the Apache License, Version 2.0 (the "License");
//    you may not use this file except in compliance with the License.
//    You may obtain a copy of the License at
//
//        http://www.apache.org/licenses/LICENSE-2.0
//
//    Unless required by applicable law or agreed to in writing, software
//    distributed under the License is distributed on an "AS IS" BASIS,
//    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
//    See the License for the specific language governing permissions and
//    limitations under the License.

#include <client_shared/conf.hpp>

#include <string>
#include <cstdlib>
#include <cerrno>

#include <mender-version.h>

#include <common/error.hpp>
#include <common/expected.hpp>
#include <common/log.hpp>
#include <common/json.hpp>

namespace mender {
namespace client_shared {
namespace conf {

using namespace std;
namespace error = mender::common::error;
namespace expected = mender::common::expected;
namespace log = mender::common::log;
namespace json = mender::common::json;

const string kMenderVersion = MENDER_VERSION;

const DefaultPathsType DefaultPaths;

const ConfigErrorCategoryClass ConfigErrorCategory;

const char *ConfigErrorCategoryClass::name() const noexcept {
        return "ConfigErrorCategory";
}

string ConfigErrorCategoryClass::message(int code) const {
        switch (code) {
        case NoError:
                return "Success";
        case InvalidOptionsError:
                return "Invalid options given";
        }
        assert(false);
        return "Unknown";
}

error::Error MakeError(ConfigErrorCode code, const string &msg) {
        return error::Error(error_condition(code, ConfigErrorCategory), msg);
}


string GetEnv(const string &var_name, const string &default_value) {
        const char *value = getenv(var_name.c_str());
        if (value == nullptr) {
                return string(default_value);
        } else {
                return string(value);
        }
}

ExpectedOptionValue CmdlineOptionsIterator::Next() {
        string option = "";
        string value = "";

        if (start_ + pos_ >= end_) {
                return ExpectedOptionValue({"", ""});
        }

        if (past_double_dash_) {
                OptionValue opt_val {"", start_[pos_]};
                pos_++;
                return ExpectedOptionValue(opt_val);
        }

        if (start_[pos_] == "--") {
                past_double_dash_ = true;
                pos_++;
                return ExpectedOptionValue({"--", ""});
        }

        if (start_[pos_][0] == '-') {
                auto eq_idx = start_[pos_].find('=');
                if (eq_idx != string::npos) {
                        option = start_[pos_].substr(0, eq_idx);
                        value = start_[pos_].substr(eq_idx + 1, start_[pos_].size() - eq_idx - 1);
                        pos_++;
                } else {
                        option = start_[pos_];
                        pos_++;
                }

                if (opts_with_value_.count(option) != 0) {
                        // option with value
                        if ((value == "") && ((start_ + pos_ >= end_) || (start_[pos_][0] == '-'))) {
                                // the next item is not a value
                                error::Error err = MakeError(
                                        ConfigErrorCode::InvalidOptionsError, "Option " + option + " missing value");
                                return ExpectedOptionValue(expected::unexpected(err));
                        } else if (value == "") {
                                // only assign the next item as value if there was no value
                                // specified as '--opt=value' (parsed above)
                                value = start_[pos_];
                                pos_++;
                        }
                } else if (opts_wo_value_.count(option) == 0) {
                        // unknown option
                        error::Error err = MakeError(
                                ConfigErrorCode::InvalidOptionsError, "Unrecognized option '" + option + "'");
                        return ExpectedOptionValue(expected::unexpected(err));
                } else if (value != "") {
                        // option without a value, yet, there was a value specified as '--opt=value' (parsed
                        // above)
                        error::Error err = MakeError(
                                ConfigErrorCode::InvalidOptionsError,
                                "Option " + option + " doesn't expect a value");
                        return ExpectedOptionValue(expected::unexpected(err));
                }
        } else {
                switch (mode_) {
                case ArgumentsMode::AcceptBareArguments:
                        value = start_[pos_];
                        pos_++;
                        break;
                case ArgumentsMode::RejectBareArguments:
                        return expected::unexpected(MakeError(
                                ConfigErrorCode::InvalidOptionsError,
                                "Unexpected argument '" + start_[pos_] + "'"));
                case ArgumentsMode::StopAtBareArguments:
                        return ExpectedOptionValue({"", ""});
                }
        }

        return ExpectedOptionValue({std::move(option), std::move(value)});
}

expected::ExpectedSize MenderConfig::ProcessCmdlineArgs(
        vector<string>::const_iterator start, vector<string>::const_iterator end, const CliApp &app) {
        bool explicit_config_path = false;
        bool explicit_fallback_config_path = false;
        string log_file = "";
        string log_level;
        string trusted_cert;
        bool skip_verify_arg = false;
        bool version_arg = false;
        bool help_arg = false;

        CmdlineOptionsIterator opts_iter(
                start, end, GlobalOptsSetWithValue(), GlobalOptsSetWithoutValue());
        opts_iter.SetArgumentsMode(ArgumentsMode::StopAtBareArguments);
        auto ex_opt_val = opts_iter.Next();
        int arg_count = 0;
        while (ex_opt_val && ((ex_opt_val.value().option != "") || (ex_opt_val.value().value != ""))) {
                arg_count++;
                auto opt_val = ex_opt_val.value();
                if ((opt_val.option == "--config") || (opt_val.option == "-c")) {
                        paths.SetConfFile(opt_val.value);
                        explicit_config_path = true;
                } else if ((opt_val.option == "--fallback-config") || (opt_val.option == "-b")) {
                        paths.SetFallbackConfFile(opt_val.value);
                        explicit_fallback_config_path = true;
                } else if (
                        (opt_val.option == "--data") || (opt_val.option == "--datastore")
                        || (opt_val.option == "-d")) {
                        paths.SetDataStore(opt_val.value);
                } else if ((opt_val.option == "--log-file") || (opt_val.option == "-L")) {
                        log_file = opt_val.value;
                } else if ((opt_val.option == "--log-level") || (opt_val.option == "-l")) {
                        log_level = opt_val.value;
                } else if ((opt_val.option == "--trusted-certs") || (opt_val.option == "-E")) {
                        trusted_cert = opt_val.value;
                } else if (opt_val.option == "--skipverify") {
                        skip_verify_arg = true;
                } else if ((opt_val.option == "--version") || (opt_val.option == "-v")) {
                        version_arg = true;
                } else if ((opt_val.option == "--help") || (opt_val.option == "-h")) {
                        help_arg = true;
                        break;
                } else {
                        assert(false);
                }
                ex_opt_val = opts_iter.Next();
        }
        if (!ex_opt_val) {
                return expected::unexpected(ex_opt_val.error());
        }

        if (version_arg) {
                if (arg_count > 1 || opts_iter.GetPos() < static_cast<size_t>(end - start)) {
                        return expected::unexpected(error::Error(
                                make_error_condition(errc::invalid_argument),
                                "--version can not be combined with other commands and arguments"));
                } else {
                        cout << kMenderVersion << endl;
                        return expected::unexpected(error::MakeError(error::ExitWithSuccessError, ""));
                }
        }

        if (help_arg) {
                PrintCliHelp(app);
                return expected::unexpected(error::MakeError(error::ExitWithSuccessError, ""));
        }

        if (log_file != "") {
                auto err = log::SetupFileLogging(log_file, true);
                if (error::NoError != err) {
                        return expected::unexpected(err);
                }
        }

        SetLevel(log::kDefaultLogLevel);

        if (log_level != "") {
                auto ex_log_level = log::StringToLogLevel(log_level);
                if (!ex_log_level) {
                        return expected::unexpected(ex_log_level.error());
                }
                SetLevel(ex_log_level.value());
        }

        auto err = LoadConfigFile_(paths.GetFallbackConfFile(), explicit_fallback_config_path);
        if (error::NoError != err) {
                this->Reset();
                return expected::unexpected(err);
        }

        err = LoadConfigFile_(paths.GetConfFile(), explicit_config_path);
        if (error::NoError != err) {
                this->Reset();
                return expected::unexpected(err);
        }

        if (this->update_log_path != "") {
                paths.SetUpdateLogPath(this->update_log_path);
        }

        if (log_level == "" && this->daemon_log_level != "") {
                auto ex_log_level = log::StringToLogLevel(this->daemon_log_level);
                if (!ex_log_level) {
                        return expected::unexpected(ex_log_level.error());
                }
                SetLevel(ex_log_level.value());
        }

        if (trusted_cert != "") {
                this->server_certificate = trusted_cert;
        }

        if (skip_verify_arg) {
                this->skip_verify = true;
        }

        http_client_config_.server_cert_path = server_certificate;
        http_client_config_.client_cert_path = https_client.certificate;
        http_client_config_.client_cert_key_path = https_client.key;
        http_client_config_.ssl_engine = https_client.ssl_engine;
        http_client_config_.skip_verify = skip_verify;

        auto proxy = http::GetHttpProxyStringFromEnvironment();
        if (proxy) {
                http_client_config_.http_proxy = proxy.value();
        } else {
                return expected::unexpected(proxy.error());
        }

        proxy = http::GetHttpsProxyStringFromEnvironment();
        if (proxy) {
                http_client_config_.https_proxy = proxy.value();
        } else {
                return expected::unexpected(proxy.error());
        }

        proxy = http::GetNoProxyStringFromEnvironment();
        if (proxy) {
                http_client_config_.no_proxy = proxy.value();
        } else {
                return expected::unexpected(proxy.error());
        }

        if (http_client_config_.no_proxy.find(',') != string::npos) {
                return expected::unexpected(MakeError(
                        ConfigErrorCode::InvalidOptionsError,
                        "Unsupported syntax for NO_PROXY environment variable. Use space-separated values instead of coma-separated."));
        }

        return opts_iter.GetPos();
}

error::Error MenderConfig::LoadConfigFile_(const string &path, bool required) {
        auto ret = this->LoadFile(path);
        if (!ret) {
                if (required) {
                        // any failure when a file is required (e.g. path was given explicitly) means an error
                        log::Error("Failed to load config from '" + path + "': " + ret.error().message);
                        return ret.error();
                } else if (ret.error().IsErrno(ENOENT)) {
                        // File doesn't exist, OK for non-required
                        log::Debug("Failed to load config from '" + path + "': " + ret.error().message);
                        return error::NoError;
                } else {
                        // other errors (parsing errors,...) for default paths should produce warnings
                        log::Warning("Failed to load config from '" + path + "': " + ret.error().message);
                        return error::NoError;
                }
        }
        // else

        return error::NoError;
}

} // namespace conf
} // namespace client_shared
} // namespace mender

1	// Copyright 2023 Northern.tech AS
2	//
3	// Licensed under the Apache License, Version 2.0 (the "License");
4	// you may not use this file except in compliance with the License.
5	// You may obtain a copy of the License at
6	//
7	// http://www.apache.org/licenses/LICENSE-2.0
8	//
9	// Unless required by applicable law or agreed to in writing, software
10	// distributed under the License is distributed on an "AS IS" BASIS,
11	// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12	// See the License for the specific language governing permissions and
13	// limitations under the License.
14
15	#include <client_shared/conf.hpp>
16
17	#include <string>
18	#include <cstdlib>
19	#include <cerrno>
20
21	#include <mender-version.h>
22
23	#include <common/error.hpp>
24	#include <common/expected.hpp>
25	#include <common/log.hpp>
26	#include <common/json.hpp>
27
28	namespace mender {
29	namespace client_shared {
30	namespace conf {
31
32	using namespace std;
33	namespace error = mender::common::error;
34	namespace expected = mender::common::expected;
35	namespace log = mender::common::log;
36	namespace json = mender::common::json;
37
38	const string kMenderVersion = MENDER_VERSION;
39
40	const DefaultPathsType DefaultPaths;
41
42	const ConfigErrorCategoryClass ConfigErrorCategory;
43
44	const char *ConfigErrorCategoryClass::name() const noexcept {	×
45	return "ConfigErrorCategory";	×
46	}
47
48	string ConfigErrorCategoryClass::message(int code) const {	10✔
49	switch (code) {	10✔
50	case NoError:
51	return "Success";	×
52	case InvalidOptionsError:
53	return "Invalid options given";	10✔
54	}
55	assert(false);
56	return "Unknown";	×
57	}
58
59	error::Error MakeError(ConfigErrorCode code, const string &msg) {	5✔
60	return error::Error(error_condition(code, ConfigErrorCategory), msg);	93✔
61	}
62
63
64	string GetEnv(const string &var_name, const string &default_value) {	1,538✔
65	const char *value = getenv(var_name.c_str());	1,538✔
66	if (value == nullptr) {	1,538✔
67	return string(default_value);	1,533✔
68	} else {
69	return string(value);	5✔
70	}
71	}
72
73	ExpectedOptionValue CmdlineOptionsIterator::Next() {	700✔
74	string option = "";	700✔
75	string value = "";	700✔
76
77	if (start_ + pos_ >= end_) {	700✔
78	return ExpectedOptionValue({"", ""});	216✔
79	}
80
81	if (past_double_dash_) {	484✔
82	OptionValue opt_val {"", start_[pos_]};	6✔
83	pos_++;	3✔
84	return ExpectedOptionValue(opt_val);	3✔
85	}
86
87	if (start_[pos_] == "--") {	481✔
88	past_double_dash_ = true;	1✔
89	pos_++;	1✔
90	return ExpectedOptionValue({"--", ""});	1✔
91	}
92
93	if (start_[pos_][0] == '-') {	480✔
94	auto eq_idx = start_[pos_].find('=');	208✔
95	if (eq_idx != string::npos) {	208✔
96	option = start_[pos_].substr(0, eq_idx);	3✔
97	value = start_[pos_].substr(eq_idx + 1, start_[pos_].size() - eq_idx - 1);	3✔
98	pos_++;	3✔
99	} else {
100	option = start_[pos_];	205✔
101	pos_++;	205✔
102	}
103
104	if (opts_with_value_.count(option) != 0) {	208✔
105	// option with value
106	if ((value == "") && ((start_ + pos_ >= end_) \|\| (start_[pos_][0] == '-'))) {	170✔
107	// the next item is not a value
108	error::Error err = MakeError(
109	ConfigErrorCode::InvalidOptionsError, "Option " + option + " missing value");	4✔
110	return ExpectedOptionValue(expected::unexpected(err));	4✔
111	} else if (value == "") {	168✔
112	// only assign the next item as value if there was no value
113	// specified as '--opt=value' (parsed above)
114	value = start_[pos_];	166✔
115	pos_++;	166✔
116	}
117	} else if (opts_wo_value_.count(option) == 0) {	38✔
118	// unknown option
119	error::Error err = MakeError(
120	ConfigErrorCode::InvalidOptionsError, "Unrecognized option '" + option + "'");	48✔
121	return ExpectedOptionValue(expected::unexpected(err));	48✔
122	} else if (value != "") {	14✔
123	// option without a value, yet, there was a value specified as '--opt=value' (parsed
124	// above)
125	error::Error err = MakeError(
126	ConfigErrorCode::InvalidOptionsError,
127	"Option " + option + " doesn't expect a value");	2✔
128	return ExpectedOptionValue(expected::unexpected(err));	2✔
129	}
130	} else {
131	switch (mode_) {	272✔
132	case ArgumentsMode::AcceptBareArguments:	66✔
133	value = start_[pos_];
134	pos_++;	66✔
135	break;	66✔
136	case ArgumentsMode::RejectBareArguments:	61✔
137	return expected::unexpected(MakeError(	61✔
138	ConfigErrorCode::InvalidOptionsError,
139	"Unexpected argument '" + start_[pos_] + "'"));	183✔
140	case ArgumentsMode::StopAtBareArguments:
141	return ExpectedOptionValue({"", ""});	145✔
142	}
143	}
144
145	return ExpectedOptionValue({std::move(option), std::move(value)});	247✔
146	}
147
148	expected::ExpectedSize MenderConfig::ProcessCmdlineArgs(	161✔
149	vector<string>::const_iterator start, vector<string>::const_iterator end, const CliApp &app) {
150	bool explicit_config_path = false;
151	bool explicit_fallback_config_path = false;
152	string log_file = "";	161✔
153	string log_level;
154	string trusted_cert;
155	bool skip_verify_arg = false;
156	bool version_arg = false;
157	bool help_arg = false;
158
159	CmdlineOptionsIterator opts_iter(
160	start, end, GlobalOptsSetWithValue(), GlobalOptsSetWithoutValue());	483✔
161	opts_iter.SetArgumentsMode(ArgumentsMode::StopAtBareArguments);
162	auto ex_opt_val = opts_iter.Next();	161✔
163	int arg_count = 0;
164	while (ex_opt_val && ((ex_opt_val.value().option != "") \|\| (ex_opt_val.value().value != ""))) {	310✔
165	arg_count++;	153✔
166	auto opt_val = ex_opt_val.value();	153✔
167	if ((opt_val.option == "--config") \|\| (opt_val.option == "-c")) {	153✔
168	paths.SetConfFile(opt_val.value);
169	explicit_config_path = true;
170	} else if ((opt_val.option == "--fallback-config") \|\| (opt_val.option == "-b")) {	149✔
171	paths.SetFallbackConfFile(opt_val.value);
172	explicit_fallback_config_path = true;
173	} else if (
174	(opt_val.option == "--data") \|\| (opt_val.option == "--datastore")	281✔
175	\|\| (opt_val.option == "-d")) {	158✔
176	paths.SetDataStore(opt_val.value);	138✔
177	} else if ((opt_val.option == "--log-file") \|\| (opt_val.option == "-L")) {	10✔
178	log_file = opt_val.value;
179	} else if ((opt_val.option == "--log-level") \|\| (opt_val.option == "-l")) {	10✔
180	log_level = opt_val.value;
181	} else if ((opt_val.option == "--trusted-certs") \|\| (opt_val.option == "-E")) {	8✔
182	trusted_cert = opt_val.value;
183	} else if (opt_val.option == "--skipverify") {	8✔
184	skip_verify_arg = true;
185	} else if ((opt_val.option == "--version") \|\| (opt_val.option == "-v")) {	8✔
186	version_arg = true;
187	} else if ((opt_val.option == "--help") \|\| (opt_val.option == "-h")) {	4✔
188	help_arg = true;
189	break;	4✔
190	} else {
191	assert(false);
192	}
193	ex_opt_val = opts_iter.Next();	298✔
194	}
195	if (!ex_opt_val) {	161✔
196	return expected::unexpected(ex_opt_val.error());	×
197	}
198
199	if (version_arg) {	161✔
200	if (arg_count > 1 \|\| opts_iter.GetPos() < static_cast<size_t>(end - start)) {	4✔
201	return expected::unexpected(error::Error(	2✔
202	make_error_condition(errc::invalid_argument),	4✔
203	"--version can not be combined with other commands and arguments"));	6✔
204	} else {
205	cout << kMenderVersion << endl;	2✔
206	return expected::unexpected(error::MakeError(error::ExitWithSuccessError, ""));	6✔
207	}
208	}
209
210	if (help_arg) {	157✔
211	PrintCliHelp(app);	4✔
212	return expected::unexpected(error::MakeError(error::ExitWithSuccessError, ""));	12✔
213	}
214
215	if (log_file != "") {	153✔
216	auto err = log::SetupFileLogging(log_file, true);	×
217	if (error::NoError != err) {	×
218	return expected::unexpected(err);	×
219	}
220	}
221
222	SetLevel(log::kDefaultLogLevel);	153✔
223
224	if (log_level != "") {	153✔
225	auto ex_log_level = log::StringToLogLevel(log_level);	2✔
226	if (!ex_log_level) {	2✔
227	return expected::unexpected(ex_log_level.error());	×
228	}
229	SetLevel(ex_log_level.value());	2✔
230	}
231
232	auto err = LoadConfigFile_(paths.GetFallbackConfFile(), explicit_fallback_config_path);	306✔
233	if (error::NoError != err) {	153✔
234	this->Reset();	×
235	return expected::unexpected(err);	×
236	}
237
238	err = LoadConfigFile_(paths.GetConfFile(), explicit_config_path);	306✔
239	if (error::NoError != err) {	153✔
240	this->Reset();	×
241	return expected::unexpected(err);	×
242	}
243
244	if (this->update_log_path != "") {	153✔
245	paths.SetUpdateLogPath(this->update_log_path);
246	}
247
248	if (log_level == "" && this->daemon_log_level != "") {	153✔
249	auto ex_log_level = log::StringToLogLevel(this->daemon_log_level);	1✔
250	if (!ex_log_level) {	1✔
251	return expected::unexpected(ex_log_level.error());	×
252	}
253	SetLevel(ex_log_level.value());	1✔
254	}
255
256	if (trusted_cert != "") {	153✔
257	this->server_certificate = trusted_cert;	×
258	}
259
260	if (skip_verify_arg) {	153✔
261	this->skip_verify = true;	×
262	}
263
264	http_client_config_.server_cert_path = server_certificate;	153✔
265	http_client_config_.client_cert_path = https_client.certificate;	153✔
266	http_client_config_.client_cert_key_path = https_client.key;	153✔
267	http_client_config_.ssl_engine = https_client.ssl_engine;	153✔
268	http_client_config_.skip_verify = skip_verify;	153✔
269
270	auto proxy = http::GetHttpProxyStringFromEnvironment();	153✔
271	if (proxy) {	153✔
272	http_client_config_.http_proxy = proxy.value();	152✔
273	} else {
274	return expected::unexpected(proxy.error());	2✔
275	}
276
277	proxy = http::GetHttpsProxyStringFromEnvironment();	304✔
278	if (proxy) {	152✔
279	http_client_config_.https_proxy = proxy.value();	151✔
280	} else {
281	return expected::unexpected(proxy.error());	2✔
282	}
283
284	proxy = http::GetNoProxyStringFromEnvironment();	302✔
285	if (proxy) {	151✔
286	http_client_config_.no_proxy = proxy.value();	150✔
287	} else {
288	return expected::unexpected(proxy.error());	2✔
289	}
290
291	if (http_client_config_.no_proxy.find(',') != string::npos) {	150✔
NEW 292	return expected::unexpected(MakeError(	×
293	ConfigErrorCode::InvalidOptionsError,
NEW 294	"Unsupported syntax for NO_PROXY environment variable. Use space-separated values instead of coma-separated."));	×
295	}
296
297	return opts_iter.GetPos();
298	}
299
300	error::Error MenderConfig::LoadConfigFile_(const string &path, bool required) {	306✔
301	auto ret = this->LoadFile(path);	306✔
302	if (!ret) {	306✔
303	if (required) {	301✔
304	// any failure when a file is required (e.g. path was given explicitly) means an error
305	log::Error("Failed to load config from '" + path + "': " + ret.error().message);	×
306	return ret.error();	×
307	} else if (ret.error().IsErrno(ENOENT)) {	301✔
308	// File doesn't exist, OK for non-required
309	log::Debug("Failed to load config from '" + path + "': " + ret.error().message);	602✔
310	return error::NoError;	301✔
311	} else {
312	// other errors (parsing errors,...) for default paths should produce warnings
313	log::Warning("Failed to load config from '" + path + "': " + ret.error().message);	×
314	return error::NoError;	×
315	}
316	}
317	// else
318
319	return error::NoError;	5✔
320	}
321
322	} // namespace conf
323	} // namespace client_shared
324	} // namespace mender

mendersoftware / mender / 2131530446

Source File Press 'n' to go to next uncovered line, 'b' for previous

Source File
Press 'n' to go to next uncovered line, 'b' for previous