fix: parse compose config before secret sanitization so common-word variable values don't corrupt service detection#1529
Merged
mbecker20 merged 2 commits intoJul 18, 2026
Conversation
…t corrupt service detection
mbecker20
approved these changes
Jul 18, 2026
mbecker20
left a comment
Member
There was a problem hiding this comment.
Thanks, will just make some small edits after merging
Author
|
Appreciate the merge and the polish edits, @mbecker20 - parsing compose config before sanitization stops the common-word false positives. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Service detection during stack bring-up parsed the
docker compose configoutput 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 configand parses the resulting YAML to enumerate the stack's services (ComposeUpinbin/periphery/src/api/compose.rs). The command was run throughrun_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
semaphoreDB password in a stack whose service is also namedsemaphore). 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 configoutput for service metadata first, then sanitize only the representation that is stored and logged:ComposeUpnow runs the compose-config command through the non-sanitizingrun_komodo_shell_command/run_komodo_standard_commandand keeps the raw stdout.parse_services_and_sanitized_configextractsStackServiceNames(includingdeploy.replicasexpansion) from the raw config, then returns the sanitized config string forres.merged_config.Compose Configlog'scommand,stdout, andstderrare sanitized before the log is pushed, on both the success and the early-return failure paths, so no secret reaches the logs.command::sanitize_stringhelper;run_komodo_command_with_sanitizationnow 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
ComposeUppath.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 reusablelib/commandhelper 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: 2expands toworker-1/worker-2from the raw config, with an unrelated secret still sanitized.rejects_malformed_compose_config- malformed YAML still surfaces theFailed to parse compose contentserror.cargo build -p komodo_periphery,cargo fmt --check, andgit diff --checkare clean.