fix: ledger redaction — preserve write paths and close a curl -u credential leak introduced by #1117

Merged
AN
2mo ago
+88-12

Issue for this PR

Closes #1245

Type of change

  • Bug fix

What does this PR do?

Security-relevant / expedited — please review promptly. origin/main's TypeScript CI job has been red since ~2026-09-04 07:28Z, blocking all PRs. This fixes both the broken-main failure and a live credential-leak regression it's coupled to.

Root cause: #1117 (14ba9bb9e7) added a filesystem-path masking rule to Telemetry.maskString (packages/opencode/src/altimate/telemetry/index.ts). SessionCompaction.redactLedgerDetail (packages/opencode/src/session/compaction.ts) calls Telemetry.maskString(value) before running its own -u/--user curl-credential redaction. That ordering has two consequences:

  1. Over-masking (4 of the 5 failing tests): every write path (2+ path segments) collapses to the literal <path>, so the session ledger — whose whole purpose is to tell the model which files it already wrote — can no longer report file paths at all.
  2. Credential leak (the 5th test, and the important one): when curl is invoked path-qualified (/usr/bin/curl -u alice password ...), maskString's path rule replaces the curl/curl.exe token with <path> before redactLedgerDetail's curl-context lookback ever runs. With the curl token gone, the lookback can't find it, curlContext comes back false, and the space-separated (non-colon-shaped) -u alice password idiom isn't otherwise recognized as credential-shaped — so it passes through redactLedgerDetail completely unredacted. This ledger text is persisted into a later model prompt across compaction (see the comment at compaction.ts:~657): a real credential leaking across a session/provider boundary.

The leak is main-only — not present in v0.10.0 or any released version.

Fix:

  1. Telemetry.maskString gains an opt-out for its path-masking pass: maskString(value, opts?: { maskPaths?: boolean }), default maskPaths: true — byte-for-byte unchanged for every existing caller. When maskPaths: false, only the #1117 path rule is skipped; every other mask (api keys, bearer tokens, emails, internal hosts, quote collapsing) still applies.
  2. redactLedgerDetail now calls maskString(value, { maskPaths: false }). This restores write-path fidelity (fixes the 4 over-mask tests), and — because the curl token survives — the curl-context lookback works again, closing the leak (fixes the 5th test).
  3. Belt-and-suspenders: redactLedgerDetail's curl-context detection is also derived independently from the pre-mask raw value (correlated ordinally against the masked-string matches, with a safe fallback to the prior masked-only check if the two ever disagree). This makes curl detection structurally immune to any future masking rule that happens to eat the command-name token — it no longer depends solely on maskPaths: false.
  4. No other maskString call site was touched — they all keep maskPaths: true (mcp/index.ts, sql-execute.ts, tool.ts, prompt.ts, register.ts, dispatcher.ts, registry.ts, warehouse-add.ts, project-scan.ts). #1117's telemetry path-masking protection is fully intact for its intended consumers — verified by test/telemetry/mask-file-paths.test.ts staying green.

Both telemetry/index.ts and compaction.ts are upstream-shared files; edits are wrapped in altimate_change markers. The marker guard (bun run script/upstream/analyze.ts --markers --base origin/main --strict) reports clean.

Related: this touches the same session-state-ledger security work referenced in compaction.ts (redactLedgerDetail's existing credential-redaction suite).

How did you verify your code works?

  • bun test packages/opencode/test/session/compaction-ledger.test.ts packages/opencode/test/session/compaction-ledger-history.test.ts — 54 pass, 0 fail (was 5 failing on main).
  • Added an explicit adversarial test reproducing the exact leak shape: redactLedgerDetail("/usr/bin/curl -u alice hunter2 https://example.com") and a /usr/local/bin/curl.exe variant, asserting the output contains neither alice nor hunter2. Confirmed it fails against pre-fix code via git stash on the two source files (leaving the new test in place) — reproduced the leak verbatim (<path> -u alice hunter2 https://example.com/) — then confirmed it passes after git stash pop.
  • bun test test/telemetry/mask-file-paths.test.ts test/altimate/telemetry-signals.test.ts test/mcp/mcp-bearer-auth.test.ts — 211 pass, 0 fail (confirms #1117's default-maskPaths path masking is untouched for its other callers).
  • Broader regression sweep: bun test across all test/session/compaction-*.test.ts, observation-mask, task-pin, uncounted-tail — 241 pass, 0 fail, 0 regressions.
  • bun run typecheck — clean across all 15 workspace packages.
  • bun run script/upstream/analyze.ts --markers --base origin/main --strict — clean.
  • Pushed and confirmed the TypeScript CI job goes green on the pushed commit.

Not independently re-verified: the pre-existing flaky timeouts in test/session/prompt.test.ts (loop calls LLM and returns assistant message, loop surfaces content-filter finishes as session errors) — confirmed these fail identically against unmodified origin/main (network/mock-provider timeout issue, unrelated to this change) and are not part of this PR's scope.

Screenshots / recordings

N/A — no UI change.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

This PR should NOT be merged by an automated agent — human review requested given the security sensitivity.

[!NOTE]
High Risk
Changes credential redaction on compaction ledger text that is persisted into later model prompts; the fix is targeted but security-critical.

Overview
Fixes a credential leak and ledger over-masking caused by running filesystem path masking on session-ledger text before compaction-specific redaction.

Telemetry.maskString now accepts optional { maskPaths?: boolean } (default true), so only the #1117 path pass can be skipped; API keys, bearer tokens, emails, and other rules are unchanged for existing callers. redactLedgerDetail calls maskString(value, { maskPaths: false }) so write paths stay visible in the state ledger and path-qualified commands like /usr/bin/curl are not collapsed to <path> before curl-context detection.

Hardening: -u/--user handling uses a shared USER_FLAG_RE, and curl context is inferred from both the masked string and the raw pre-mask value (ordinal-aligned), so a future mask rule that strips curl is less likely to reopen the leak.

Adds a regression test for path-qualified curl with space-separated -u user password.

Reviewed by Cursor Bugbot for commit 3463f0985af64e003a0aab18d5eb03ef961e26ee. Bugbot is set up for automated code reviews on this repo. Configure here.

Summary by CodeRabbit

  • Bug Fixes

    • Improved protection for credentials included in path-qualified curl commands, including space-separated username and password formats.
    • Preserved command paths in session activity details while continuing to redact sensitive credentials and tokens.
    • Added safeguards to ensure credential redaction remains reliable across different command representations.

  • Tests

    • Added regression coverage for credential leakage in path-qualified curl commands.

Summary by cubic

Fixes a credential leak and broken write-path reporting in the session ledger that #1117 introduced. Telemetry.maskString's new path-masking pass collapsed /usr/bin/curl to <path> before redactLedgerDetail's curl-context lookback ran, so path-qualified curl -u user password leaked unredacted into ledger text persisted across compaction; write paths also collapsed to <path>, breaking the ledger's file-reporting purpose. The leak is main-only, not in any release, and the change fixes the five tests that have been red on origin/main's TypeScript CI since ~2026-09-04 07:28Z.

Bug Fixes

  • Telemetry.maskString gains a maskPaths opt-out (default true, byte-for-byte unchanged for all existing callers); only the path pass is skipped, all other masks still apply.
  • redactLedgerDetail now calls maskString(value, { maskPaths: false }), restoring write-path fidelity and the curl-context lookback.
  • Curl-context detection is also derived from the raw pre-mask value (ordinally correlated with the masked matches), so a future mask rule that eats the command token can't reopen the leak.
  • Adds regression tests for path-qualified curl and curl.exe with space-separated -u user password, confirmed failing against pre-fix code.