Skip to content

Escape passwords in configurations and scripts #8842

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Oct 14, 2025
Merged

Escape passwords in configurations and scripts #8842

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6e8e29a
Fix EntSearch readiness script
barkbay Oct 13, 2025
835a611
iso-8601=seconds is not a valid flag with busybox
barkbay Oct 13, 2025
169e034
Sanitize password string in Stackmon config
barkbay Oct 13, 2025
fc8cf63
Fix TestReconcileConfig_ReadinessProbe
barkbay Oct 13, 2025
569208b
Update stack mon sidecar expected config
barkbay Oct 13, 2025
87da548
Do not include symbols for now
barkbay Oct 14, 2025
c4b06b6
Merge branch 'main' into fix-readiness-scripts
barkbay Oct 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions pkg/controller/common/password/password.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,15 @@ const (

// Digits is the list of permitted digits.
Digits = "0123456789"

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

var (
defaultCharacterSet = strings.Join([]string{LowerLetters, UpperLetters, Digits, Symbols}, "")
characterSetWithoutSymbols = strings.Join([]string{LowerLetters, UpperLetters, Digits}, "")

// defaultCharacterSet is the default character set used for generating passwords.
// It includes lowercase letters, uppercase letters and digits, but excludes symbols.
// This is to ensure compatibility with configurations which do not offer a way to escape symbols.
defaultCharacterSet = characterSetWithoutSymbols
)

// RandomGenerator is an interface for generating random passwords.
Expand Down Expand Up @@ -63,7 +64,7 @@ func (r *randomPasswordGenerator) Generate(ctx context.Context) ([]byte, error)
}

// NewGenerator returns a password generator with the specified length.
// All character types (lowercase, uppercase, digits, symbols) are always used.
// All character types (lowercase, uppercase, digits) except symbols are used.
func NewGenerator(
client k8s.Client,
passwordLength int,
Expand Down
8 changes: 8 additions & 0 deletions pkg/controller/common/stackmon/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package stackmon
import (
"bytes"
"context"
"encoding/json"
"fmt"
"hash"
"hash/fnv"
Expand Down Expand Up @@ -203,6 +204,13 @@ func TemplateFuncs(
version semver.Version,
) template.FuncMap {
return template.FuncMap{
"sanitizeJSON": func(v any) (string, error) {
b, err := json.Marshal(v)
if err != nil {
return "", fmt.Errorf("json marshal failed: %w", err)
}
return string(b), nil
},
"isVersionGTE": func(minAllowedVersion string) (bool, error) {
minAllowedSemver, err := semver.Parse(minAllowedVersion)
if err != nil {
Expand Down
16 changes: 8 additions & 8 deletions pkg/controller/elasticsearch/stackmon/__snapshots__/sidecar_test.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ metricbeat.modules:
xpack.enabled: true
hosts: ["https://localhost:9200"]
username: elastic
password: secret
password: "secret"
ssl.enabled: true
# The ssl verification_mode is set to `certificate` in the config template to verify that the certificate is signed by a trusted authority,
# but does not perform any hostname verification. This is used when SSL is enabled with or without CA, to support self-signed certificate
Expand Down Expand Up @@ -57,7 +57,7 @@ metricbeat.modules:
xpack.enabled: true
hosts: ["https://localhost:9200"]
username: elastic
password: secret
password: "secret"
ssl.enabled: true
# The ssl verification_mode is set to `certificate` in the config template to verify that the certificate is signed by a trusted authority,
# but does not perform any hostname verification. This is used when SSL is enabled with or without CA, to support self-signed certificate
Expand Down Expand Up @@ -94,7 +94,7 @@ metricbeat.modules:
xpack.enabled: true
hosts: ["https://localhost:9200"]
username: elastic
password: secret
password: "secret"
ssl.enabled: true
# The ssl verification_mode is set to `certificate` in the config template to verify that the certificate is signed by a trusted authority,
# but does not perform any hostname verification. This is used when SSL is enabled with or without CA, to support self-signed certificate
Expand Down Expand Up @@ -133,7 +133,7 @@ metricbeat.modules:
xpack.enabled: true
hosts: ["https://localhost:9200"]
username: elastic
password: secret
password: "secret"
ssl.enabled: false
# The ssl verification_mode is set to `certificate` in the config template to verify that the certificate is signed by a trusted authority,
# but does not perform any hostname verification. This is used when SSL is enabled with or without CA, to support self-signed certificate
Expand Down Expand Up @@ -170,7 +170,7 @@ metricbeat.modules:
xpack.enabled: true
hosts: ["https://localhost:9200"]
username: elastic
password: secret
password: "secret"
ssl.enabled: true
# The ssl verification_mode is set to `certificate` in the config template to verify that the certificate is signed by a trusted authority,
# but does not perform any hostname verification. This is used when SSL is enabled with or without CA, to support self-signed certificate
Expand Down Expand Up @@ -207,7 +207,7 @@ setup.template.settings:
{
"metadata": {
"annotations": {
"elasticsearch.k8s.elastic.co/monitoring-config-hash": "421654492"
"elasticsearch.k8s.elastic.co/monitoring-config-hash": "3826168075"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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 🤷‍♂️ )

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I'll update the docs accordingly.

}
},
"spec": {
Expand Down Expand Up @@ -471,7 +471,7 @@ setup.template.settings:
{
"metadata": {
"annotations": {
"elasticsearch.k8s.elastic.co/monitoring-config-hash": "1805099278"
"elasticsearch.k8s.elastic.co/monitoring-config-hash": "3784706159"
}
},
"spec": {
Expand Down Expand Up @@ -707,7 +707,7 @@ setup.template.settings:
{
"metadata": {
"annotations": {
"elasticsearch.k8s.elastic.co/monitoring-config-hash": "1086800329"
"elasticsearch.k8s.elastic.co/monitoring-config-hash": "3685568186"
}
},
"spec": {
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/elasticsearch/stackmon/metricbeat.tpl.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ metricbeat.modules:
xpack.enabled: true
hosts: ["{{ .URL }}"]
username: {{ .Username }}
password: {{ .Password }}
password: {{ sanitizeJSON .Password }}
ssl.enabled: {{ .IsSSL }}
# The ssl verification_mode is set to `certificate` in the config template to verify that the certificate is signed by a trusted authority,
# but does not perform any hostname verification. This is used when SSL is enabled with or without CA, to support self-signed certificate
Expand Down
10 changes: 5 additions & 5 deletions pkg/controller/enterprisesearch/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,23 +118,23 @@ func readinessProbeScript(ent entv1.EnterpriseSearch, config *settings.Canonical
}
basicAuthArgs := "" // no credentials: no basic auth
if esAuth.Elasticsearch.Username != "" {
basicAuthArgs = fmt.Sprintf("-u %s:%s", esAuth.Elasticsearch.Username, esAuth.Elasticsearch.Password)
basicAuthArgs = fmt.Sprintf("-u %s:'%s'", esAuth.Elasticsearch.Username, esAuth.Elasticsearch.Password)
}

return []byte(`#!/usr/bin/env bash

# fail should be called as a last resort to help the user to understand why the probe failed
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)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

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 😐

echo '{"timestamp": "'"${timestamp}"'", "message": "readiness probe failed", '"${1}"'}'| tee /proc/1/fd/2 2> /dev/null
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

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

exit 1
}

# request timeout can be overridden from an environment variable
READINESS_PROBE_TIMEOUT=${READINESS_PROBE_TIMEOUT:=` + fmt.Sprintf("%d", ReadinessProbeTimeoutSec) + `}

# 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}")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

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

curl_rc=$?

if [[ ${curl_rc} -ne 0 ]]; then
Expand All @@ -144,7 +144,7 @@ func readinessProbeScript(ent entv1.EnterpriseSearch, config *settings.Canonical
if [[ ${status} == "200" ]]; then
exit 0
else
fail " \"status\": \"${status}\", \"version\":\"${version}\" "
fail " \"status\": \"${status}\" "
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

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

fi
`), nil
}
Expand Down
10 changes: 5 additions & 5 deletions pkg/controller/enterprisesearch/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -790,7 +790,7 @@ func TestReconcileConfig_ReadinessProbe(t *testing.T) {
},
},
ipFamily: corev1.IPv4Protocol,
wantCmd: `curl -g -o /dev/null -w "%{http_code}" https://127.0.0.1:3002/api/ent/v1/internal/health -k -s --max-time ${READINESS_PROBE_TIMEOUT}`, // no ES basic auth
wantCmd: `curl -g -o /dev/null -w "%{http_code}" https://127.0.0.1:3002/api/ent/v1/internal/health -k -s --max-time "${READINESS_PROBE_TIMEOUT}"`, // no ES basic auth
},
{
name: "create default readiness probe script (no es association, IPv6)",
Expand All @@ -805,7 +805,7 @@ func TestReconcileConfig_ReadinessProbe(t *testing.T) {
},
},
ipFamily: corev1.IPv6Protocol,
wantCmd: `curl -g -o /dev/null -w "%{http_code}" https://[::1]:3002/api/ent/v1/internal/health -k -s --max-time ${READINESS_PROBE_TIMEOUT}`, // no ES basic auth
wantCmd: `curl -g -o /dev/null -w "%{http_code}" https://[::1]:3002/api/ent/v1/internal/health -k -s --max-time "${READINESS_PROBE_TIMEOUT}"`, // no ES basic auth
},
{
name: "update existing readiness probe script if different",
Expand All @@ -830,7 +830,7 @@ func TestReconcileConfig_ReadinessProbe(t *testing.T) {
},
},
ipFamily: corev1.IPv4Protocol,
wantCmd: `curl -g -o /dev/null -w "%{http_code}" https://127.0.0.1:3002/api/ent/v1/internal/health -k -s --max-time ${READINESS_PROBE_TIMEOUT}`, // no ES basic auth
wantCmd: `curl -g -o /dev/null -w "%{http_code}" https://127.0.0.1:3002/api/ent/v1/internal/health -k -s --max-time "${READINESS_PROBE_TIMEOUT}"`, // no ES basic auth
},
{
name: "with ES association: use ES user credentials",
Expand All @@ -853,7 +853,7 @@ func TestReconcileConfig_ReadinessProbe(t *testing.T) {
},
},
ipFamily: corev1.IPv4Protocol,
wantCmd: `curl -g -o /dev/null -w "%{http_code}" https://127.0.0.1:3002/api/ent/v1/internal/health -u ns-sample-ent-user:password -k -s --max-time ${READINESS_PROBE_TIMEOUT}`,
wantCmd: `curl -g -o /dev/null -w "%{http_code}" https://127.0.0.1:3002/api/ent/v1/internal/health -u ns-sample-ent-user:'password' -k -s --max-time "${READINESS_PROBE_TIMEOUT}"`,
},
{
name: "with es credentials in a user-provided config secret",
Expand Down Expand Up @@ -881,7 +881,7 @@ func TestReconcileConfig_ReadinessProbe(t *testing.T) {
},
},
ipFamily: corev1.IPv4Protocol,
wantCmd: `curl -g -o /dev/null -w "%{http_code}" https://127.0.0.1:3002/api/ent/v1/internal/health -u myusername:mypassword -k -s --max-time ${READINESS_PROBE_TIMEOUT}`,
wantCmd: `curl -g -o /dev/null -w "%{http_code}" https://127.0.0.1:3002/api/ent/v1/internal/health -u myusername:'mypassword' -k -s --max-time "${READINESS_PROBE_TIMEOUT}"`,
},
}
for _, tt := range tests {
Expand Down