Skip to content

fix: quote/int-cast user values to prevent YAML injection - #188

Merged
ddecrulle merged 2 commits into
mainfrom
fix/yaml-injection-quote-values
Oct 26, 2026
Merged

ddecrulle merged 2 commits into
mainfrom
fix/yaml-injection-quote-values

Conversation

@ddecrulle

Copy link
Copy Markdown
Member

Problem

Onyxia fills chart values from user input and installs the chart as cluster-admin. Values interpolated without quoting (or wrapped only in literal quotes) can break out of their YAML scalar: a value with newlines and --- injects arbitrary manifests (e.g. a ClusterRoleBinding to cluster-admin). Same class of bug as InseeFrLab/helm-charts-interactive-services#332.

An audit rendering every free-text schema field with such a payload found 39 injectable fields in this repo's own templates and 22 more through third-party subcharts (plus fields behind disabled feature flags, e.g. s3.*).

Fix

Where the value lands Fix
YAML scalar in our templates | quote, | int (ports, replicas, probes), printf … | quote when text surrounds the value (repo:tag, connection strings, test args)
JSON / Python string in our templates (cloudbeaver.conf, opensearch test) | toJson
XML (hive-metastore, trino core-site), .properties (trino), shell (pgadmin), the S3 secret of the old library-chart (spark-history, spark-thrift-server), third-party subcharts (Bitnami postgresql, chromadb, dask, elasticsearch, milvus, qdrant) single-line pattern (^[^\n\r]*$) in values.schema.json — quoting is impossible there, helm rejects the value before rendering

pattern is used rather than format: helm enforces pattern but does not assert format (an email/uri field accepts any string).

Also: postgresql-cnpg did not render with its own defaults (imageCatalog.major: 18 vs the schema's string); the default is now "18" and it renders with | int, as the CNPG Cluster CRD expects an integer.

Regression test

utils/test_yaml_injection.py + .github/workflows/yaml-injection-test.yaml (same script as helm-charts-interactive-services): tests the charts changed by a PR / push to main, all charts on workflow_dispatch. Feature flags of each field's parents are switched on so gated values are covered.

Verification

  • Injection test: 22/22 charts safe; removing a single | quote is caught.
  • Default renders compared object by object before/after: unchanged except randomly generated secrets/certificates (differ on every render) and the intended quoting in cloudbeaver.conf and the opensearch test script.
  • helm lint passes on every chart; versions bumped.

Pre-existing bugs found (not fixed here)

  • opensearch: enabling route.enabled breaks the chart (app.kubernetes.io/component: - label).
  • opensearch: opensearch.StrongPassword uses the user's password as a printf format string, so a password containing % is altered.

Limits

  • The pattern protection relies on helm's schema validation at install time (i.e. Onyxia not passing --skip-schema-validation).
  • It covers the fields exposed in the schema. Keys passed beyond the form (e.g. Bitnami extraDeploy) are not covered by any chart-level fix; least privilege for the installing identity is the real boundary there.

🤖 Generated with Claude Code

ddecrulle and others added 2 commits October 23, 2026 15:48
Onyxia fills chart values from user input and installs the chart as
cluster-admin. Values interpolated unquoted (or wrapped only in literal quotes)
could break out of their YAML scalar and inject arbitrary manifests.

- Own templates, YAML scalars: `| quote`, `| int` for ports/replicas/probes,
  and `printf ... | quote` where literal text surrounds the value (images
  `repo:tag`, connection strings, test `args`).
- Own templates, non-YAML payloads: `| toJson` for JSON/Python string contexts
  (cloudbeaver.conf, opensearch test script).
- Values that land where escaping is impossible (XML in hive-metastore and
  trino core-site, trino .properties, pgadmin shell script, the S3 secret of the
  old library-chart used by spark-history / spark-thrift-server) or in
  third-party subcharts (Bitnami postgresql, chromadb, dask, elasticsearch,
  milvus, qdrant): a single-line `pattern` in values.schema.json, enforced by
  helm's schema validation before rendering.
- postgresql-cnpg: `imageCatalog.major` default is now the string the schema
  declares (the chart did not render with its own defaults), and it is rendered
  with `| int` as the CNPG Cluster CRD expects an integer.

Default renders are unchanged (compared object by object; only randomly
generated secrets/certificates differ between renders), except the intended
quoting in cloudbeaver.conf and the opensearch test script.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
utils/test_yaml_injection.py renders each chart with a `\n---` payload in every
free-text schema field (feature flags of the field's parents switched on) and
fails if a value breaks the rendered YAML or injects a document. Same script as
helm-charts-interactive-services.

The workflow tests the charts changed by a pull request or a push to main, and
all charts on workflow_dispatch.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant