Escape passwords in configurations and scripts #8842

barkbay · 2025-10-13T08:40:06Z

Follow up of #8817

Some characters like ] or " can break some scripts or configurations.

prodsecmachine · 2025-10-13T08:40:19Z

✅ Snyk checks have passed. No issues have been found so far.

Status	Scanner	Critical	High	Medium	Low	Total (0)
✅	Licenses	0	0	0	0	0 issues
✅	Open Source Security	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

barkbay · 2025-10-13T08:40:48Z

pkg/controller/enterprisesearch/config.go

 	function fail {
 	  timestamp=$(date --iso-8601=seconds)
-	  echo "{\"timestamp\": \"${timestamp}\", \"message\": \"readiness probe failed\", "$1"}" | tee /proc/1/fd/2 2> /dev/null
+	  echo '{"timestamp": "'"${timestamp}"'", "message": "readiness probe failed", '"${1}"'}'| tee /proc/1/fd/2 2> /dev/null


^--^ SC2027 (warning): The surrounding quotes actually unquote this. Remove or escape them.

barkbay · 2025-10-13T08:41:07Z

pkg/controller/enterprisesearch/config.go

 	  exit 0
 	else
-	  fail " \"status\": \"${status}\", \"version\":\"${version}\" "
+	  fail " \"status\": \"${status}\" "


^--------^ SC2154 (warning): version is referenced but not assigned.

barkbay · 2025-10-13T08:41:26Z

pkg/controller/enterprisesearch/config.go


 	# request the health endpoint and expect http status code 200. Turning globbing off for unescaped IPv6 addresses
-	status=$(curl -g -o /dev/null -w "%{http_code}" ` + url + ` ` + basicAuthArgs + ` -k -s --max-time ${READINESS_PROBE_TIMEOUT})
+	status=$(curl -g -o /dev/null -w "%{http_code}" ` + url + ` ` + basicAuthArgs + ` -k -s --max-time "${READINESS_PROBE_TIMEOUT}")


^------------------------^ SC2086 (info): Double quote to prevent globbing and word splitting

barkbay · 2025-10-13T08:49:48Z

pkg/controller/enterprisesearch/config.go

 	function fail {
-	  timestamp=$(date --iso-8601=seconds)
-	  echo "{\"timestamp\": \"${timestamp}\", \"message\": \"readiness probe failed\", "$1"}" | tee /proc/1/fd/2 2> /dev/null
+	  timestamp=$(date +%Y-%m-%dT%H:%M:%S%z)


date: unrecognized option '--iso-8601=seconds' BusyBox v1.37.0 (2024-09-30 10:39:57 UTC) multi-call binary.

Not sure for how long we had this bug 😐

barkbay · 2025-10-13T09:25:24Z

We can escape it but I wonder if we should remove \" in

cloud-on-k8s/pkg/controller/common/password/password.go

Line 29 in 8aa28ea

Symbols = "~!@#$%^&*()_+`-={}|[]\\:\"<>?,./"

, this can create invalid json or yaml and is currently breaking stack monitoring.

Edit: I added a function to sanitize the string.

barkbay · 2025-10-13T11:38:30Z

buildkite test this -f p=gke,t=TestVersionUpgrade*

barkbay · 2025-10-13T15:18:51Z

We have another issue for Logstash which offers a way to inject the ES password through environment variables, as described in the documentation or in the following example:

  pipelines:
    - pipeline.id: main
      config.string: |
        input { exec { command => 'uptime' interval => 10 } }
        output {
          elasticsearch {
            hosts => [ "${TEST_ES_HOSTS}" ]
            ssl_enabled => true
            ssl_certificate_authorities => "${TEST_ES_SSL_CERTIFICATE_AUTHORITY}"
            user => "${TEST_ES_USER}"
            password => "${TEST_ES_PASSWORD}"
          }
        }

If there is " in the generated password, this is going to break Logstash deployment with following error:

[2025-10-13T12:55:22,445][ERROR][logstash.agent           ] Failed to execute action {:action=>LogStash::PipelineAction::Create/pipeline_id:main, :exception=>"LogStash::ConfigurationError", :message=>"Expected one of [ \\t\\r\\n], \"#\", \"{\", \"}\" at line 8, column 20 (byte 412) after output {\n  elasticsearch {\n    hosts => [ \"https://monitoring-f9cb-es-http.e2e-mercury.svc:9200\" ]\n    ssl_enabled => true\n    ssl_certificate_authorities => \"/mnt/elastic-internal/elasticsearch-association/e2e-mercury/monitoring-f9cb/certs/ca.crt\"\n    user => \"e2e-mercury-logstash-sample-f9cb-e2e-mercury-monitoring-f9cb-logstash-user\"\n    password => \"]\"", :backtrace=>["/usr/share/logstash/logstash-core/lib/logstash/compiler.rb:32:in `compile_imperative'", "org/logstash/execution/AbstractPipelineExt.java:285:in `initialize'", "org/logstash/execution/AbstractPipelineExt.java:223:in `initialize'", "/usr/share/logstash/logstash-core/lib/logstash/java_pipeline.rb:47:in `initialize'", "org/jruby/RubyClass.java:950:in `new'", "/usr/share/logstash/logstash-core/lib/logstash/pipeline_action/create.rb:50:in `execute'", "/usr/share/logstash/logstash-core/lib/logstash/agent.rb:431:in `block in converge_state'"]}

In above case the password was starting with ]". (" was part of the password)

I don't know what to do for this one. My feeling is that we should not advise users to rely on internally generated passwords. Also using env variables to propagate credentials feels wrong, I believe this sample would be broken as soon as the password is regenerated.

barkbay · 2025-10-14T11:34:22Z

buildkite test this -f p=gke,t=TestSamples

barkbay · 2025-10-14T12:04:19Z

pkg/controller/elasticsearch/stackmon/__snapshots__/sidecar_test.snap

 "metadata": {
  "annotations": {
-   "elasticsearch.k8s.elastic.co/monitoring-config-hash": "421654492"
+   "elasticsearch.k8s.elastic.co/monitoring-config-hash": "3826168075"


@rhr323 If we include this change (I mean the " around the password in Metricbeat config) then I think this is going to recreate the ES Pods for clusters which have stack monitoring is enabled. We should then add 3.2.0 to https://www.elastic.co/docs/deploy-manage/upgrade/orchestrator/upgrade-cloud-on-k8s#k8s-beta-to-ga-rolling-restart (we can also revert it since we just decided to not include symbols, but may end up re-introduce it eventually later so idk 🤷‍♂️ )

Thanks! I'll update the docs accordingly.

pebrc

LGTM

* Fix EntSearch readiness script * iso-8601=seconds is not a valid flag with busybox * Sanitize password string in Stackmon config * Fix TestReconcileConfig_ReadinessProbe * Update stack mon sidecar expected config * Do not include symbols for now

Fix EntSearch readiness script

6e8e29a

botelastic bot added the triage label Oct 13, 2025

barkbay commented Oct 13, 2025

View reviewed changes

iso-8601=seconds is not a valid flag with busybox

835a611

barkbay commented Oct 13, 2025

View reviewed changes

barkbay added 3 commits October 13, 2025 12:05

Sanitize password string in Stackmon config

169e034

Fix TestReconcileConfig_ReadinessProbe

fc8cf63

Update stack mon sidecar expected config

569208b

barkbay added the >bug Something isn't working label Oct 13, 2025

botelastic bot removed the triage label Oct 13, 2025

barkbay added triage v3.2.0 labels Oct 13, 2025

botelastic bot removed the triage label Oct 13, 2025

barkbay changed the title ~~Fix EntSearch readiness script~~ Sanitize or escape passwords in configurations and scripts Oct 13, 2025

barkbay changed the title ~~Sanitize or escape passwords in configurations and scripts~~ Escape passwords in configurations and scripts Oct 13, 2025

Do not include symbols for now

87da548

barkbay mentioned this pull request Oct 14, 2025

Include symbols in generated passwords #8847

Open

barkbay commented Oct 14, 2025

View reviewed changes

barkbay marked this pull request as ready for review October 14, 2025 12:04

pebrc approved these changes Oct 14, 2025

View reviewed changes

Merge branch 'main' into fix-readiness-scripts

c4b06b6

barkbay merged commit 3267994 into elastic:main Oct 14, 2025
9 checks passed

barkbay deleted the fix-readiness-scripts branch October 14, 2025 14:48

barkbay mentioned this pull request Oct 14, 2025

Escape passwords in configurations and scripts (#8842) #8849

Merged

barkbay added the exclude-from-release-notes Exclude this PR from appearing in the release notes label Oct 14, 2025

Escape passwords in configurations and scripts #8842

Escape passwords in configurations and scripts #8842

Uh oh!

Conversation

barkbay commented Oct 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

prodsecmachine commented Oct 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Snyk checks have passed. No issues have been found so far.

Uh oh!

barkbay Oct 13, 2025

Choose a reason for hiding this comment

Uh oh!

barkbay Oct 13, 2025

Choose a reason for hiding this comment

Uh oh!

barkbay Oct 13, 2025

Choose a reason for hiding this comment

Uh oh!

barkbay Oct 13, 2025

Choose a reason for hiding this comment

Uh oh!

barkbay commented Oct 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

barkbay commented Oct 13, 2025

Uh oh!

barkbay commented Oct 13, 2025

Uh oh!

barkbay commented Oct 14, 2025

Uh oh!

barkbay Oct 14, 2025

Choose a reason for hiding this comment

Uh oh!

rhr323 Oct 14, 2025

Choose a reason for hiding this comment

Uh oh!

pebrc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

barkbay commented Oct 13, 2025 •

edited

Loading

prodsecmachine commented Oct 13, 2025 •

edited

Loading

barkbay commented Oct 13, 2025 •

edited

Loading