Skip to content

Escape $ in generated Docker Compose files - #5087

Merged
Json-Andriopoulos merged 2 commits into
zenml-io:developfrom
kounelisagis:bug/compose-dollar-interpolation
Aug 14, 2026
Merged

Escape $ in generated Docker Compose files#5087
Json-Andriopoulos merged 2 commits into
zenml-io:developfrom
kounelisagis:bug/compose-dollar-interpolation

Conversation

@kounelisagis

Copy link
Copy Markdown
Contributor

Docker Compose interpolates $VAR and ${VAR} across the whole compose file before starting containers. The SSH and HyperAI orchestrators write already resolved step environments (secrets included) and bind mount paths straight into the file, so any value containing $ was corrupted: a pa$$word secret reached the container as pa$word, host environment values leaked in, and values shaped like ${X:?} aborted docker compose up and the whole run.

Both orchestrators now serialize through a shared dump_compose_yaml helper that escapes every $ as $$, so values round trip verbatim. The escape covers the whole document because mount paths are affected too, not only the environment block.

Tests: helper round trip, plus an SSH regression asserting values containing $ survive.

@gitguardian

gitguardian Bot commented Jul 16, 2026

Copy link
Copy Markdown

锔忊渽 There are no secrets present in this pull request anymore.

If these secrets were true positive and are still valid, we highly recommend you to revoke them.
While these secrets were previously flagged, we no longer have a reference to the
specific commits where they were detected. Once a secret has been leaked into a git
repository, you should consider it compromised, even if it was deleted immediately.
Find here more information about risks.


馃 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

@github-actions

Copy link
Copy Markdown
Contributor

鈿狅笍 This PR has been inactive for 2 weeks and has been marked as stale.
Timeline:

  • Week 2 (now): First reminder - PR marked as stale
  • Week 4: PR will be automatically closed if no activity
    Please update this PR or leave a comment to keep it active. Any activity will reset the timer and remove the stale label.

@github-actions github-actions Bot added the stale label Jul 31, 2026
@bcdurak
bcdurak self-requested a review July 31, 2026 09:30
@bcdurak bcdurak removed the stale label Jul 31, 2026
@bcdurak
bcdurak requested review from Json-Andriopoulos and removed request for bcdurak August 5, 2026 09:43
Comment thread tests/unit/integrations/ssh/orchestrators/test_ssh_orchestrator.py Outdated
Comment thread src/zenml/orchestrators/utils.py Outdated
Comment thread src/zenml/orchestrators/utils.py Outdated
Comment thread tests/unit/orchestrators/test_utils.py Outdated

@Json-Andriopoulos Json-Andriopoulos left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for this PR @kounelisagis . I added some minor comments, feel free to address them at your own pace and ping me to approve & merge.

@Json-Andriopoulos Json-Andriopoulos added the no-release-notes Release notes will NOT be attached and used publicly for this PR. label Aug 13, 2026
@Json-Andriopoulos

Copy link
Copy Markdown
Contributor

@kounelisagis One last thing: To fix the failing lint checks you can do the following: ./scripts/format.sh and run ./scripts/lint.sh to verify that they are resolved. Thanks

Docker Compose interpolates $VAR and ${VAR} across the whole compose
file before creating containers. Both the SSH and HyperAI orchestrators
write already resolved step environments (secrets included) and bind
mount paths straight into the file, so any value containing $ was
silently mangled, had host environment values injected into it, or, for
forms like ${X:?}, aborted `docker compose up` and the whole run.

Route both orchestrators through a shared dump_compose_yaml helper that
escapes every $ as $$ on the serialized document, so every value round
trips verbatim. This mirrors what compose-go's own serializer emits and
is the only complete fix: the escape must cover the entire file, not
just the environment block (mount paths are affected too), and Compose
offers no --no-interpolate at `up` time.
@kounelisagis
kounelisagis force-pushed the bug/compose-dollar-interpolation branch from 609403a to 3b7bf5b Compare August 13, 2026 12:53
@Json-Andriopoulos Json-Andriopoulos added the run-slow-ci Tag that is used to trigger the slow-ci label Aug 14, 2026
@Json-Andriopoulos
Json-Andriopoulos merged commit 03dfe09 into zenml-io:develop Aug 14, 2026
80 of 84 checks passed
Json-Andriopoulos added a commit that referenced this pull request Aug 20, 2026
Docker Compose interpolates $VAR and ${VAR} across the whole compose
file before creating containers. Both the SSH and HyperAI orchestrators
write already resolved step environments (secrets included) and bind
mount paths straight into the file, so any value containing $ was
silently mangled, had host environment values injected into it, or, for
forms like ${X:?}, aborted `docker compose up` and the whole run.

Route both orchestrators through a shared dump_compose_yaml helper that
escapes every $ as $$ on the serialized document, so every value round
trips verbatim. This mirrors what compose-go's own serializer emits and
is the only complete fix: the escape must cover the entire file, not
just the environment block (mount paths are affected too), and Compose
offers no --no-interpolate at `up` time.

Co-authored-by: Json-Andriopoulos <Json-Andriopoulos@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-release-notes Release notes will NOT be attached and used publicly for this PR. run-slow-ci Tag that is used to trigger the slow-ci

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants