Skip to content

fix: parse compose config before secret sanitization so common-word variable values don't corrupt service detection#1529

Merged
mbecker20 merged 2 commits into
moghtech:2.3.0from
mvanhorn:fix/868-compose-config-parse-before-sanitize
Jul 18, 2026
Merged

fix: parse compose config before secret sanitization so common-word variable values don't corrupt service detection#1529
mbecker20 merged 2 commits into
moghtech:2.3.0from
mvanhorn:fix/868-compose-config-parse-before-sanitize

Conversation

@mvanhorn

Copy link
Copy Markdown

Summary

Service detection during stack bring-up parsed the docker compose config output after secret sanitization, so a secret value that collided with a service name, container name, or image reference corrupted the parsed service identifiers. This parses the raw config for service metadata first and sanitizes only what is stored and logged, with regression tests that reproduce the collision.

Motivation

When bringing up a stack, periphery runs docker compose config and parses the resulting YAML to enumerate the stack's services (ComposeUp in bin/periphery/src/api/compose.rs). The command was run through run_komodo_command_with_sanitization, which replaces secret values with placeholders before the output is returned. The service-enumeration parse then ran against that already-sanitized text.

That ordering corrupts service detection whenever a secret's value happens to collide with structural content of the compose config. A common case: an environment variable whose value is a plain word that also appears as a service name, container name, or image reference (for example a semaphore DB password in a stack whose service is also named semaphore). The sanitizer rewrites every occurrence of that value to a placeholder like <SEMAPHORE__SEMAPHORE_DB_USER>, so the parsed service ends up with a mangled name, or the parse breaks outright, and the stack's containers are tracked under the wrong identifiers.

Closes #868.

Solution

Parse the raw docker compose config output for service metadata first, then sanitize only the representation that is stored and logged:

  • ComposeUp now runs the compose-config command through the non-sanitizing run_komodo_shell_command / run_komodo_standard_command and keeps the raw stdout.
  • A new helper parse_services_and_sanitized_config extracts StackServiceNames (including deploy.replicas expansion) from the raw config, then returns the sanitized config string for res.merged_config.
  • The Compose Config log's command, stdout, and stderr are sanitized before the log is pushed, on both the success and the early-return failure paths, so no secret reaches the logs.
  • Sanitization is centralized as a reusable command::sanitize_string helper; run_komodo_command_with_sanitization now calls it too, so the existing sanitized paths are unchanged in behavior.

Scope is limited to the maintainer-confirmed sanitization-corruption bug in the standard ComposeUp path.

Relationship to #1176

This supersedes the earlier #1176, which proposed the same parse-before-sanitize idea but was closed unmerged (author-abandoned, no reviews). This version re-implements against current main, centralizes the sanitize call as a reusable lib/command helper rather than an ad-hoc replacement, and adds the regression coverage #1176 lacked so the fix is verifiable without Docker.

Testing

cargo test -p komodo_periphery (3 tests, all passing):

  • parses_services_before_sanitizing_compose_config - a replacer for a common word (semaphore) that is also the service name; asserts the service is still detected correctly from the raw config while the stored config is sanitized.
  • expands_replicas_from_raw_compose_config - deploy.replicas: 2 expands to worker-1 / worker-2 from the raw config, with an unrelated secret still sanitized.
  • rejects_malformed_compose_config - malformed YAML still surfaces the Failed to parse compose contents error.

cargo build -p komodo_periphery, cargo fmt --check, and git diff --check are clean.

@mbecker20 mbecker20 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks, will just make some small edits after merging

@mbecker20
mbecker20 changed the base branch from main to 2.3.0 July 18, 2026 20:07
@mbecker20
mbecker20 merged commit fad2a8b into moghtech:2.3.0 Jul 18, 2026
@mvanhorn

Copy link
Copy Markdown
Author

Appreciate the merge and the polish edits, @mbecker20 - parsing compose config before sanitization stops the common-word false positives.

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.

[Bug] Issue With Variable Interpolation

2 participants