From f2d0b8c797a0cffbb552bdab0cb59e71834f338d Mon Sep 17 00:00:00 2001 From: Grace Wehner Date: Thu, 24 Sep 2026 13:16:24 -0700 Subject: [PATCH 1/4] Quote config map values written into generated shell files Follows #1790, which quoted the values written by tomlparser.rb. The same unquoted interpolation remains in two other parsers and in setGlobalEnvVar, so a value is subject to shell processing when the generated file is sourced rather than arriving as configured: quotes truncate it, a bare $ is substituted away, and surrounding whitespace is dropped. Reuses the get_command_linux helper introduced by #1790, defined locally in each parser to match how get_command_windows is already handled across the tree: - tomlparser-common-agent-config.rb and tomlparser-metric-collection-config.rb - setGlobalEnvVar, which re-emitted values into /opt/env_vars using double quotes, leaving them open to the same processing when that file is sourced Behaviour is unchanged for well-formed configuration. Across every affected setting and value shape, the variable bash ends up with after sourcing is identical to before, for both the shipped defaults and a fully populated config. The Windows KEY=VALUE writes are untouched, since those are split on '=' and must not carry quotes. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../scripts/tomlparser-common-agent-config.rb | 15 +- .../tomlparser-config-value-handling_test.rb | 138 ++++++++++++++++++ .../tomlparser-metric-collection-config.rb | 7 +- kubernetes/linux/main.sh | 4 +- 4 files changed, 157 insertions(+), 7 deletions(-) create mode 100644 build/common/installer/scripts/tomlparser-config-value-handling_test.rb diff --git a/build/common/installer/scripts/tomlparser-common-agent-config.rb b/build/common/installer/scripts/tomlparser-common-agent-config.rb index 2cf89f0740..b7bde92217 100644 --- a/build/common/installer/scripts/tomlparser-common-agent-config.rb +++ b/build/common/installer/scripts/tomlparser-common-agent-config.rb @@ -121,6 +121,11 @@ def get_command_windows(env_variable_name, env_variable_value) return "#{env_variable_name}=#{env_variable_value}\n" end +def get_command_linux(env_variable_name, env_variable_value) + escaped_value = env_variable_value.to_s.gsub("'") { "'\\''" } + return "export #{env_variable_name}='#{escaped_value}'\n" +end + if is_windows? # Write the settings to file, so that they can be set as environment variables file = File.open("setcommonagentenv.txt", "w") @@ -157,16 +162,16 @@ def get_command_windows(env_variable_name, env_variable_value) file = File.open("common_agent_config_env_var", "w") if !file.nil? if @disableTelemetry - file.write("export DISABLE_TELEMETRY=#{@disableTelemetry}\n") + file.write(get_command_linux("DISABLE_TELEMETRY", @disableTelemetry)) end if @enableHighLogScaleMode - file.write("export ENABLE_HIGH_LOG_SCALE_MODE=#{@enableHighLogScaleMode}\n") + file.write(get_command_linux("ENABLE_HIGH_LOG_SCALE_MODE", @enableHighLogScaleMode)) end if @enableCustomMetrics - file.write("export ENABLE_CUSTOM_METRICS=#{@enableCustomMetrics}\n") + file.write(get_command_linux("ENABLE_CUSTOM_METRICS", @enableCustomMetrics)) end - file.write("export AZMON_COLLECT_AMA_LOGS_PROCESS_METRICS=#{@amaLogsCollectProcessMetricsEnabled}\n") - file.write("export AZMON_KUBERNETES_METADATA_CACHE_TTL_SECONDS=#{@logEnableKubernetesMetadataCacheTTLSeconds}\n") + file.write(get_command_linux("AZMON_COLLECT_AMA_LOGS_PROCESS_METRICS", @amaLogsCollectProcessMetricsEnabled)) + file.write(get_command_linux("AZMON_KUBERNETES_METADATA_CACHE_TTL_SECONDS", @logEnableKubernetesMetadataCacheTTLSeconds)) # Close file after writing all environment variables file.close else diff --git a/build/common/installer/scripts/tomlparser-config-value-handling_test.rb b/build/common/installer/scripts/tomlparser-config-value-handling_test.rb new file mode 100644 index 0000000000..c732bc3303 --- /dev/null +++ b/build/common/installer/scripts/tomlparser-config-value-handling_test.rb @@ -0,0 +1,138 @@ +require "minitest/autorun" +require "fileutils" +require "tmpdir" +require "rbconfig" +require "open3" + +# kubernetes/linux/main.sh sources the shell files these parsers generate, so an unquoted value +# is subject to shell processing instead of arriving as configured. The values are rendered as +# single-quoted literals; these tests pin that a value containing shell syntax is left intact +# and that ordinary values still arrive unchanged. +class TomlParserConfigValueHandlingTest < Minitest::Test + REPO = File.expand_path("../../../..", __dir__) + SHELL_SYNTAX = "true$(touch EVALUATED)" + + COMMON = { + source: "build/common/installer/scripts/tomlparser-common-agent-config.rb", + mount: "/etc/config/settings/agent-settings", + output: "common_agent_config_env_var", + }.freeze + + METRIC = { + source: "build/linux/installer/scripts/tomlparser-metric-collection-config.rb", + mount: "/etc/config/settings/metric_collection_settings", + output: "config_metric_collection_env_var", + }.freeze + + LOG = { + source: "build/common/installer/scripts/tomlparser.rb", + mount: "/etc/config/settings/log-data-collection-settings", + output: "config_env_var", + }.freeze + + # Generates the shell file, then sources it the way main.sh does. Returns whether the shell + # processed the value and what bash ended up with. The parser's hardcoded image paths are + # repointed at the fixture, so this needs no agent image. + def source_generated(parser, toml, vars = []) + Dir.mktmpdir do |dir| + settings = File.join(dir, "settings") + File.write(settings, toml) + + source = File.read(File.join(REPO, parser[:source])) + .sub(parser[:mount].inspect, settings.inspect) + .sub('require_relative "/etc/fluent/plugin/constants"', + "require_relative #{File.join(REPO, "source/plugins/ruby/constants.rb").inspect}") + File.write(File.join(dir, "parser.rb"), source) + FileUtils.cp(File.join(REPO, "build/common/installer/scripts/ConfigParseErrorLogger.rb"), dir) + + Open3.capture3({ "AZMON_AGENT_CFG_SCHEMA_VERSION" => "v1" }, + RbConfig.ruby, File.join(dir, "parser.rb"), chdir: dir) + + script = ". ./#{parser[:output]}; " + vars.map { |v| "printf '%s\\n' \"$#{v}\"" }.join("; ") + out, = Open3.capture3("bash", "-c", script, chdir: dir) + return { evaluated: File.exist?(File.join(dir, "EVALUATED")), values: out.split("\n", -1) } + end + end + + def test_common_agent_shell_syntax_is_not_processed + result = source_generated(COMMON, <<~TOML, %w[DISABLE_TELEMETRY ENABLE_HIGH_LOG_SCALE_MODE ENABLE_CUSTOM_METRICS]) + [agent_settings.telemetry_config] + disable_telemetry = "#{SHELL_SYNTAX}" + + [agent_settings.high_log_scale] + enabled = "#{SHELL_SYNTAX}" + + [agent_settings.custom_metrics] + enabled = "#{SHELL_SYNTAX}" + TOML + + refute result[:evaluated], "sourcing the generated file let the shell process the config map value" + # The literal text survives, which is what keeps this from changing behaviour. What matters + # is that it is inert and never equals the "true" the consumers look for. + result[:values].first(3).each { |v| refute_equal "true", v.to_s.downcase } + end + + def test_metric_collection_shell_syntax_is_not_processed + result = source_generated(METRIC, <<~TOML, %w[AZMON_PV_COLLECT_KUBE_SYSTEM_METRICS]) + [metric_collection_settings.collect_kube_system_pv_metrics] + enabled = "#{SHELL_SYNTAX}" + TOML + + refute result[:evaluated], "sourcing the generated file let the shell process the config map value" + refute_equal "true", result[:values].first.to_s.downcase + end + + def test_log_collection_shell_syntax_is_not_processed + result = source_generated(LOG, <<~TOML, %w[AZMON_COLLECT_STDOUT_LOGS]) + [log_collection_settings.stdout] + enabled = "#{SHELL_SYNTAX}" + exclude_namespaces = ["kube-system","#{SHELL_SYNTAX}"] + [log_collection_settings.schema] + containerlog_schema_version = "#{SHELL_SYNTAX}" + [log_collection_settings.enrich_container_logs] + enabled = "#{SHELL_SYNTAX}" + TOML + + refute result[:evaluated], "sourcing the generated file let the shell process the config map value" + end + + # A value containing a single quote is the case naive escaping gets wrong. + def test_quote_in_value_cannot_break_out + result = source_generated(LOG, <<~TOML, %w[AZMON_STDOUT_EXCLUDED_NAMESPACES]) + [log_collection_settings.stdout] + enabled = true + exclude_namespaces = ["it's$(touch EVALUATED)"] + TOML + + refute result[:evaluated], "a single quote in the value broke out of the quoted literal" + assert_equal "it's$(touch EVALUATED)", result[:values].first + end + + def test_ordinary_values_are_unchanged + result = source_generated(COMMON, <<~TOML, %w[DISABLE_TELEMETRY ENABLE_HIGH_LOG_SCALE_MODE]) + [agent_settings.telemetry_config] + disable_telemetry = true + + [agent_settings.high_log_scale] + enabled = "TRUE" + TOML + + refute result[:evaluated] + assert_equal "true", result[:values][0] + assert_equal "TRUE", result[:values][1], "the value must reach bash exactly as configured" + end + + def test_ordinary_log_collection_values_are_unchanged + result = source_generated(LOG, <<~TOML, %w[AZMON_COLLECT_STDOUT_LOGS AZMON_STDOUT_EXCLUDED_NAMESPACES AZMON_CONTAINER_LOG_SCHEMA_VERSION]) + [log_collection_settings.stdout] + enabled = true + exclude_namespaces = ["kube-system","gatekeeper-system"] + [log_collection_settings.schema] + containerlog_schema_version = "v2" + TOML + + assert_equal "true", result[:values][0] + assert_equal "kube-system,gatekeeper-system", result[:values][1] + assert_equal "v2", result[:values][2] + end +end diff --git a/build/linux/installer/scripts/tomlparser-metric-collection-config.rb b/build/linux/installer/scripts/tomlparser-metric-collection-config.rb index 3001fdbafc..9c15032b68 100644 --- a/build/linux/installer/scripts/tomlparser-metric-collection-config.rb +++ b/build/linux/installer/scripts/tomlparser-metric-collection-config.rb @@ -57,11 +57,16 @@ def populateSettingValuesFromConfigMap(parsedConfig) end end +def get_command_linux(env_variable_name, env_variable_value) + escaped_value = env_variable_value.to_s.gsub("'") { "'\\''" } + return "export #{env_variable_name}='#{escaped_value}'\n" +end + # Write the settings to file, so that they can be set as environment variables file = File.open("config_metric_collection_env_var", "w") if !file.nil? - file.write("export AZMON_PV_COLLECT_KUBE_SYSTEM_METRICS=#{@collectPVKubeSystemMetrics}\n") + file.write(get_command_linux("AZMON_PV_COLLECT_KUBE_SYSTEM_METRICS", @collectPVKubeSystemMetrics)) # Close file after writing all metric collection setting environment variables file.close puts "****************End Metric Collection Settings Processing********************" diff --git a/kubernetes/linux/main.sh b/kubernetes/linux/main.sh index 55625148b8..6062b3ccc5 100644 --- a/kubernetes/linux/main.sh +++ b/kubernetes/linux/main.sh @@ -205,7 +205,9 @@ gracefulShutdown() { # usage: setGlobalEnvVar ENABLE_SIDECAR_SCRAPING true setGlobalEnvVar() { export "$1"="$2" - echo "export \"$1\"=\"$2\"" >> /opt/env_vars + # /opt/env_vars is sourced by later shells, so the value is written as a single-quoted + # literal. Double quotes would leave it open to shell processing on the way back in. + printf "export %s='%s'\n" "$1" "$(printf '%s' "$2" | sed "s/'/'\\\\''/g")" >> /opt/env_vars } touch /opt/env_vars touch /opt/dcr_env_var From b94b0ba34b3b30a23727ad651e69f78724a39651 Mon Sep 17 00:00:00 2001 From: Grace Wehner Date: Mon, 26 Oct 2026 10:58:05 -0700 Subject: [PATCH 2/4] Preserve quoted config values in bashrc Copy generated environment files verbatim so shell quoting survives startup. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../tomlparser-config-value-handling_test.rb | 37 +++++++++++++++++-- kubernetes/linux/main.sh | 12 +++--- 2 files changed, 40 insertions(+), 9 deletions(-) diff --git a/build/common/installer/scripts/tomlparser-config-value-handling_test.rb b/build/common/installer/scripts/tomlparser-config-value-handling_test.rb index c732bc3303..e7f6d27bce 100644 --- a/build/common/installer/scripts/tomlparser-config-value-handling_test.rb +++ b/build/common/installer/scripts/tomlparser-config-value-handling_test.rb @@ -11,6 +11,7 @@ class TomlParserConfigValueHandlingTest < Minitest::Test REPO = File.expand_path("../../../..", __dir__) SHELL_SYNTAX = "true$(touch EVALUATED)" + SHELL_BREAKOUT = "true'; touch EVALUATED; #" COMMON = { source: "build/common/installer/scripts/tomlparser-common-agent-config.rb", @@ -33,7 +34,7 @@ class TomlParserConfigValueHandlingTest < Minitest::Test # Generates the shell file, then sources it the way main.sh does. Returns whether the shell # processed the value and what bash ended up with. The parser's hardcoded image paths are # repointed at the fixture, so this needs no agent image. - def source_generated(parser, toml, vars = []) + def source_generated(parser, toml, vars = [], persist_to_bashrc: false) Dir.mktmpdir do |dir| settings = File.join(dir, "settings") File.write(settings, toml) @@ -48,8 +49,18 @@ def source_generated(parser, toml, vars = []) Open3.capture3({ "AZMON_AGENT_CFG_SCHEMA_VERSION" => "v1" }, RbConfig.ruby, File.join(dir, "parser.rb"), chdir: dir) - script = ". ./#{parser[:output]}; " + vars.map { |v| "printf '%s\\n' \"$#{v}\"" }.join("; ") - out, = Open3.capture3("bash", "-c", script, chdir: dir) + commands = if persist_to_bashrc + main = File.read(File.join(REPO, "kubernetes/linux/main.sh")) + file = Regexp.escape(parser[:output]) + copy_loop = main[/cat #{file} \| while[^\n]*\n[^\n]*\n\s*done|while[^\n]*\n[^\n]*\n\s*done < #{file}/] + raise "could not find the #{parser[:output]} copy loop in main.sh" unless copy_loop + + "#{copy_loop}; . \"$HOME/.bashrc\"" + else + ". ./#{parser[:output]}" + end + script = commands + "; " + vars.map { |v| "printf '%s\\n' \"$#{v}\"" }.join("; ") + out, = Open3.capture3({ "HOME" => dir }, "bash", "-c", script, chdir: dir) return { evaluated: File.exist?(File.join(dir, "EVALUATED")), values: out.split("\n", -1) } end end @@ -108,6 +119,26 @@ def test_quote_in_value_cannot_break_out assert_equal "it's$(touch EVALUATED)", result[:values].first end + def test_quote_survives_copy_to_bashrc + result = source_generated(COMMON, <<~TOML, %w[ENABLE_CUSTOM_METRICS], persist_to_bashrc: true) + [agent_settings.custom_metrics] + enabled = "#{SHELL_BREAKOUT}" + TOML + + refute result[:evaluated], "copying the generated file to .bashrc let the shell process the value" + assert_equal SHELL_BREAKOUT, result[:values].first + end + + def test_metric_quote_survives_copy_to_bashrc + result = source_generated(METRIC, <<~TOML, %w[AZMON_PV_COLLECT_KUBE_SYSTEM_METRICS], persist_to_bashrc: true) + [metric_collection_settings.collect_kube_system_pv_metrics] + enabled = "#{SHELL_BREAKOUT}" + TOML + + refute result[:evaluated], "copying the generated file to .bashrc let the shell process the value" + assert_equal SHELL_BREAKOUT, result[:values].first + end + def test_ordinary_values_are_unchanged result = source_generated(COMMON, <<~TOML, %w[DISABLE_TELEMETRY ENABLE_HIGH_LOG_SCALE_MODE]) [agent_settings.telemetry_config] diff --git a/kubernetes/linux/main.sh b/kubernetes/linux/main.sh index 6062b3ccc5..7e2a222f47 100644 --- a/kubernetes/linux/main.sh +++ b/kubernetes/linux/main.sh @@ -542,9 +542,9 @@ fi # common agent config settings applicable for all container types ruby tomlparser-common-agent-config.rb -cat common_agent_config_env_var | while read line; do - echo $line >> ~/.bashrc -done +while IFS= read -r line; do + printf '%s\n' "$line" >> ~/.bashrc +done < common_agent_config_env_var source common_agent_config_env_var # check if high log scale mode enabled @@ -878,9 +878,9 @@ if [ "${CONTAINER_TYPE}" != "PrometheusSidecar" ] && [ "${GENEVA_LOGS_INTEGRATIO #Parse the configmap to set the right environment variables for metric collection settings ruby tomlparser-metric-collection-config.rb - cat config_metric_collection_env_var | while read line; do - echo $line >>~/.bashrc - done + while IFS= read -r line; do + printf '%s\n' "$line" >> ~/.bashrc + done < config_metric_collection_env_var source config_metric_collection_env_var fi From daf3185b085a746da41dccec9f52dbcecdb10583 Mon Sep 17 00:00:00 2001 From: Grace Wehner Date: Mon, 26 Oct 2026 13:04:40 -0700 Subject: [PATCH 3/4] TEMP: publish Linux PR image for this branch (revert before merge) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .pipelines/azure_pipeline_mergedbranches.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.pipelines/azure_pipeline_mergedbranches.yaml b/.pipelines/azure_pipeline_mergedbranches.yaml index 61ef1f4ebb..5dde0bbdc4 100644 --- a/.pipelines/azure_pipeline_mergedbranches.yaml +++ b/.pipelines/azure_pipeline_mergedbranches.yaml @@ -259,7 +259,8 @@ extends: docker pull mcr.microsoft.com/azuremonitor/containerinsights/cidev/prometheus-collector/images:buildx-stable-1 docker buildx create --name dockerbuilder --driver docker-container --driver-opt image=mcr.microsoft.com/azuremonitor/containerinsights/cidev/prometheus-collector/images:buildx-stable-1 --use docker buildx inspect --bootstrap - if [ "$(Build.Reason)" != "PullRequest" ]; then + # TEMP: revert before merge. Publishes this branch's PR image for backdoor validation. + if [ "$(Build.Reason)" != "PullRequest" ] || [ "$(System.PullRequest.SourceBranch)" == "grwehner/harden-agent-config-value-handling" ]; then docker buildx build --platform $(BUILD_PLATFORMS) --tag ${{ variables.repoImageName }}:$(linuxImagetag) -f kubernetes/linux/Dockerfile.multiarch --metadata-file $(Build.ArtifactStagingDirectory)/linux/metadata.json --build-arg IMAGE_TAG=$(linuxTelemetryTag) --build-arg GOLANG_BASE_IMAGE=$(GOLANG_BASE_IMAGE) --build-arg CI_BASE_IMAGE=$(CI_BASE_IMAGE) --push --provenance=false . echo "##vso[task.logissue type=warning]Linux image built with tag: ${{ variables.repoImageName }}:$(linuxImagetag)" docker pull ${{ variables.repoImageName }}:$(linuxImagetag) From d3786a9e64522d358d0b7e879df2a3fa7418df39 Mon Sep 17 00:00:00 2001 From: Grace Wehner Date: Mon, 26 Oct 2026 14:50:50 -0700 Subject: [PATCH 4/4] Revert temporary branch image publishing Remove the branch-scoped PR push guard after the backdoor image was validated. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .pipelines/azure_pipeline_mergedbranches.yaml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.pipelines/azure_pipeline_mergedbranches.yaml b/.pipelines/azure_pipeline_mergedbranches.yaml index 5dde0bbdc4..61ef1f4ebb 100644 --- a/.pipelines/azure_pipeline_mergedbranches.yaml +++ b/.pipelines/azure_pipeline_mergedbranches.yaml @@ -259,8 +259,7 @@ extends: docker pull mcr.microsoft.com/azuremonitor/containerinsights/cidev/prometheus-collector/images:buildx-stable-1 docker buildx create --name dockerbuilder --driver docker-container --driver-opt image=mcr.microsoft.com/azuremonitor/containerinsights/cidev/prometheus-collector/images:buildx-stable-1 --use docker buildx inspect --bootstrap - # TEMP: revert before merge. Publishes this branch's PR image for backdoor validation. - if [ "$(Build.Reason)" != "PullRequest" ] || [ "$(System.PullRequest.SourceBranch)" == "grwehner/harden-agent-config-value-handling" ]; then + if [ "$(Build.Reason)" != "PullRequest" ]; then docker buildx build --platform $(BUILD_PLATFORMS) --tag ${{ variables.repoImageName }}:$(linuxImagetag) -f kubernetes/linux/Dockerfile.multiarch --metadata-file $(Build.ArtifactStagingDirectory)/linux/metadata.json --build-arg IMAGE_TAG=$(linuxTelemetryTag) --build-arg GOLANG_BASE_IMAGE=$(GOLANG_BASE_IMAGE) --build-arg CI_BASE_IMAGE=$(CI_BASE_IMAGE) --push --provenance=false . echo "##vso[task.logissue type=warning]Linux image built with tag: ${{ variables.repoImageName }}:$(linuxImagetag)" docker pull ${{ variables.repoImageName }}:$(linuxImagetag)