Skip to content

Raw config-to-env output (decrypted secrets) is echoed to pod logs on failure #38

Closed

Description

@birme

Finding (medium)

scripts/docker-entrypoint.sh:245 and :261: echo "[CONFIG] ERROR: Failed to load config (exit $config_exit): $config_env_output $config_err_output" >&2, and line 261 prints $config_env_output again. That variable is the stdout of npx @osaas/cli web config-to-env: export NAME=value lines including secret parameters decrypted with CONFIG_API_KEY. On non-zero exit or output with no matching export line, they go to stderr, Loki and anyone with log access. Line 97 redaction only covers git stderr (gh*_, Basic).

Fix: print only the exit code and a redacted $config_err_output. Sibling: Eyevinn/web-runner has the same line (filed separately); #26 introduced the fatal path without covering log content.

Low

The runner runs as root and OSC_ACCESS_TOKEN, GIT_TOKEN, GITHUB_TOKEN and CONFIG_API_KEY stay exported into pip install build hooks and the app; Dockerfile has curl | bash for NodeSource and unpinned pip install gradio.

Lessons

Found by the Weekly Security Audit (2026-11-04); static read of a shallow clone, not reproduced live.

🤖 Generated with Claude Code

Co-Authored-By: Claude noreply@anthropic.com

Activity

  1. added
    securitySecurity vulnerability or hardening
    on Nov 4, 2026
  2. birme commented on Nov 6, 2026

    @birme
    ContributorAuthor

    Architect Triage

    Classification: Ready for implementation
    Existing PR: none (find-candidate-prs.sh merged and open both []; no PR names #38). Prior ## Spec Invalid reports: 0.
    Verified on main c47c3aa (premises re-run today, not copied from the ticket):

    • scripts/docker-entrypoint.sh:245 prints $config_env_output $config_err_output on a non-zero exit and :261 prints $config_env_output when stdout has no line matching ^export [A-Za-z_][A-Za-z0-9_]*=. No other line prints config_env_output or valid_exports (grep).
    • What stdout holds: the pinned CLI (npm pack @osaas/cli@4.35.0, dist/web/cmd.js:125-176) prints one export KEY='value' per parameter with console.log (key upper-cased, value decrypted when CONFIG_API_KEY is set) and writes only err.message to stderr before process.exit(1). So :245 leaks only when the CLI fails after printing (for example a null value throwing inside the map), while :261 is the realistic leak: a store whose keys are not shell identifiers (my-key becomes export MY-KEY='<secret>') fails the grep and the whole stdout, secret included, goes to stderr, Loki, and get-my-app-logs tool results (which land in AI conversation history, the osaas-ai#1156 write-time-redaction rule). ADR-0046 (secrets readable with a PAT) does not make this log copy acceptable: it concerns the parameter-store read path, not a second persistent copy.
    • Exposure history in Loki: not established (a plain {namespace="eyevinn-python-runner"} selector returns no streams, so a zero-hit query for the two log lines means nothing). It does not change the directions.

    Existing test coverage: python-runner/tests/test-entrypoint-config-load-failure.sh extracts the block between # Load environment variables from config service if configured and # Install Python dependencies and runs it under set -e with stub npx/curl/timeout. Its 7 assertions (timeout exits 124; non-zero exit propagates and prints refresh-app-config; garbled output exits non-zero with [CONFIG] ERROR; empty store continues; stderr-only noise continues; valid exports evaluated with Loaded 2 environment variable; no CONFIG_SVC skips) all stay green under the directions below, because exit codes, the [CONFIG] ERROR prefix, the refresh-app-config hint and the count line are unchanged. Not wired to CI (.github/workflows has only sync-fork-dev.yml and sync-fork-prod.yml), so the implementer runs the file locally.
    Consumer audit (log text is the only surface; no endpoint, status code or timing changes): osaas-ai src/service/models/instructions.ts:1967-2007 (system-prompt rule 34) and src/mcp/tools/myapps.ts:4346 key on the line [CONFIG] Loading environment variables from config service (entrypoint :219) and the refresh-app-config hint; both stay byte-identical. Searched osaas-deploy-manager/src, osaas-ai/src, osaas-app/src and osaas-e2e-tests/tests+utils for [CONFIG], Failed to load config, no valid export, Raw output: nothing else parses the [CONFIG] ERROR lines (instructions.ts:685 quotes an older Warning: Failed to load config ... string). A consumer outside those repos (a tenant script reading its own logs) would not have been found.

    Implementation directions (2 files: scripts/docker-entrypoint.sh config block :207-272, and tests/test-entrypoint-config-load-failure.sh):

    1. Define redact_config_log() INSIDE the extracted block, before first use (the test's awk extracts exactly that range, so a helper defined elsewhere is command not found under the test, and git_scrub_stderr at :96 is local to clone_from_git, so do not reuse it). Pure bash plus sed -E: replace the literal values of OSC_ACCESS_TOKEN, CONFIG_API_KEY, GIT_TOKEN, GITHUB_TOKEN when non-empty (${s//"$v"/[REDACTED]}), gh[pso]_[A-Za-z0-9]{20,}, JWT-shaped eyJ[A-Za-z0-9_-]+\.[A-Za-z0-9_-]+\.[A-Za-z0-9_-]*, and ([Bb]asic )[A-Za-z0-9+/=]{8,} (same pattern as :97). No new dependency.
    2. :245: print $(redact_config_log "$config_err_output") and never $config_env_output. Keep everything up to and including (exit $config_exit): byte-identical.
    3. :247: grep $config_err_output only, so decrypted stdout no longer decides what the log says (a secret value containing 401 would otherwise print the hint). Hint text byte-identical.
    4. :261: keep the prefix [CONFIG] ERROR: Config service '$CONFIG_SVC' returned output with no valid export lines, then print the line count and the NAME of each rejected export line only (sed -n 's/^export \([^= ]*\)=.*/\1/p', first 10, each cut to 64 chars), plus the words values not logged. Names are tenant-chosen parameter keys; .* drops everything from the first =. This keeps the realistic diagnosis (which key is not a valid identifier) without the value.
    5. Do not touch :219, :252, :258, the exit codes (124, config_exit, 1), the eval or the set -e comment at :222-231.
    6. Tests (same file, new stub modes, canary S3CR3T-canary, assert against BOTH captured streams): (a) stdout export DB_PASS='S3CR3T-canary', stderr boom 401 unauthorized, exit 2: rc 2, stderr has Failed to load config (exit 2) and refresh-app-config, no canary; (b) stdout export MY-KEY='S3CR3T-canary', exit 0: rc non-zero, no REACHED_PIP, stderr has no valid export lines and MY-KEY, no canary; (c) stderr carrying the OSC_ACCESS_TOKEN literal, a ghp_ + 36 chars token and a JWT-shaped string, exit 2: none of the three present, [REDACTED] present; (d) stdout export X='expired 401', stderr boom, exit 2: no refresh-app-config (stdout no longer drives the hint); (e) passing path: the existing ok case additionally asserts qux is in neither stream.
    7. Run from the repo root: bash tests/test-entrypoint-config-load-failure.sh and the six other tests/test-entrypoint-*.sh; paste the results in the PR body.
    8. PR body: state the consumer impact above ("log text only; Loading line, hint, exit codes unchanged"), a ## Lessons section, and a What a human does next statement: a prod sync-fork and remake of eyevinn-python-runner is birme's call (running apps pick the fix up on their next rebuild; historical logs are not rewritten); the dev counterpart is agent-run (.claude/scripts/dev-remake.sh https://github.com/Eyevinn/python-runner eyevinn-python-runner after merge).

    Observed in the same block, left unchanged (not in the ticket's Fix): eval "$valid_exports" on a multi-line secret fails with bash's own unexpected EOF message (probe: no value text printed; a stray ) prints one token). The shell's message is not ours to redact without hiding failures.

    Siblings (separate repos, separate deploy units, none touched by this PR): web-runner#75 (open, NEEDS_HUMAN, its log-redaction part is the same change); golang-runner :199 and dotnet-runner already filter with config_safe_output; php-runner :187 and wasm-runner :147/:154/:156 print the same output and have no open ticket. After this PR lands it is the reference implementation for those two.

    Not directed by this ticket: the Low paragraph (runner runs as root with OSC_ACCESS_TOKEN, GIT_TOKEN, GITHUB_TOKEN, CONFIG_API_KEY exported into pip install build hooks and the app) proposes no fix and any fix (drop root, unset tokens before install) changes what tenant pip install hooks and the app can read, i.e. the runner contract; it is the question already open on web-runner#75 and gets one answer for the runner family. curl | bash for NodeSource is the deliberate, version-gated install from PR #30, and pip install gradio is the runner's preinstalled framework; pinning or removing either changes the image tenants get and needs a product decision, so no change is directed here.

    E2E Impact

    Verdict: None
    Grepped osaas-e2e-tests/tests and utils for config-to-env, [CONFIG], Failed to load config, refresh-app-config, Raw output, no valid export and for python-runner|eyevinn-python. The only hit is tests/test-mcp-myapps-paramstore-large.ts step 5, which runs the CLI itself (osc web config-to-env) and never reads the runner entrypoint's log lines; no spec drives the python runner. The change is stderr text in a container entrypoint, with no UI or MCP surface that asserts it.

    Scope reductions

    None.

    Automated triage by architect agent

  3. birme commented on Nov 6, 2026

    @birme
    ContributorAuthor

    Merged via PR #40. The repository has no release tags, so there is no release to wait for.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    securitySecurity vulnerability or hardening

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions