Review group ID path handling in set-group flow

[vikman90]

Summary

While reviewing the set-group flow in the development code path, we noticed that group identifiers are accepted with only a non-empty check before being used in filesystem path construction.

This looks worth tightening to avoid unexpected path resolution behavior and to make cleanup more robust.

Affected areas

centralized_configuration.cpp

Current validation only rejects empty strings:

wazuh-agent/src/agent/centralized_configuration/src/centralized_configuration.cpp

Lines 66 to 73 in 927df48

	groupIds = parameters.at(module_command::GROUPS_ARG).get<std::vector<std::string>>();
	if (!std::all_of(groupIds.begin(), groupIds.end(), [](const std::string& id) { return !id.empty(); }))
	{
	LogWarn("Group name can not be an empty string.");
	co_return module_command::CommandExecutionResult {
	module_command::Status::FAILURE,
	"CentralizedConfiguration group set failed, a group name can not be an empty string."};
	}

Later, each groupId is used directly to build temporary and destination paths:

wazuh-agent/src/agent/centralized_configuration/src/centralized_configuration.cpp

Lines 116 to 118 in 927df48

	const std::filesystem::path tmpGroupFile =
	m_fileSystemWrapper->temp_directory_path() /
	(groupId + "_" + CreateTmpFilename() + config::DEFAULT_SHARED_FILE_EXTENSION);

wazuh-agent/src/agent/centralized_configuration/src/centralized_configuration.cpp

Lines 159 to 160 in 927df48

	const std::filesystem::path destGroupFile = std::filesystem::path(config::DEFAULT_SHARED_CONFIG_PATH) /
	(groupId + config::DEFAULT_SHARED_FILE_EXTENSION);

Relevant references:

• src/agent/centralized_configuration/src/centralized_configuration.cpp:66-72
• src/agent/centralized_configuration/src/centralized_configuration.cpp:116-160

command_handler.cpp

CheckCommand() verifies that groups is present and is a JSON array, but it does not validate the contents of each entry:

wazuh-agent/src/agent/command_handler/src/command_handler.cpp

Lines 20 to 24 in 927df48

	const std::unordered_map<std::string, CommandDetails> VALID_COMMANDS_MAP = {
	{module_command::SET_GROUP_COMMAND,
	CommandDetails {module_command::CENTRALIZED_CONFIGURATION_MODULE,
	module_command::CommandExecutionMode::SYNC,
	{{module_command::GROUPS_ARG, nlohmann::json::value_t::array}}}},

Additional note

The invalid-file cleanup guard compares paths lexically:

wazuh-agent/src/agent/centralized_configuration/src/centralized_configuration.cpp

Lines 138 to 145 in 927df48

	if (m_fileSystemWrapper->exists(tmpGroupFile) &&
	tmpGroupFile.parent_path() == m_fileSystemWrapper->temp_directory_path())
	{
	if (!m_fileSystemWrapper->remove(tmpGroupFile))
	{
	LogWarn("Failed to delete invalid group file: {}", tmpGroupFile.string());
	}
	}

It may be safer to compare normalized/canonical paths here as well.

Suggested follow-up

• Add stricter validation for groupId values before any path construction.
• Consider allowing only a bounded set of characters for group identifiers.
• Normalize and verify final paths remain under the expected base directories before file operations.
• Update the cleanup guard to compare canonical paths instead of lexical parent paths.

Credit

Reported by Rayhan Ramdhany Hanaputra.