diff --git a/packages/opencode/src/altimate/telemetry/index.ts b/packages/opencode/src/altimate/telemetry/index.ts index 39c992b5d..08ca60459 100644 --- a/packages/opencode/src/altimate/telemetry/index.ts +++ b/packages/opencode/src/altimate/telemetry/index.ts @@ -1558,14 +1558,20 @@ export namespace Telemetry { // Each match replaces with a fixed redaction so length-based fingerprinting // can't reconstruct the original token. - export function maskString(s: string): string { + // altimate_change start — maskPaths opt-out for callers that redact on their + // own terms (see compaction.ts::redactLedgerDetail). Default true keeps + // every existing caller's behavior byte-for-byte unchanged; only a caller + // that explicitly passes maskPaths:false skips the #1117 path-masking pass, + // and every other rule (api keys, bearer tokens, emails, internal hosts, + // quote collapsing) still applies. + export function maskString(s: string, opts?: { maskPaths?: boolean }): string { // Consumers truncate masked output to <= 2000 chars; masking beyond 8 KB // buys nothing, and unbounded input is what turns any super-linear rule // into a stall. Input is cut FIRST so every rule — the linear credential/ // quote passes included — does bounded work, and the cut must never fail // a rule open across the boundary. No floor gates any of this: a floor is // a leak past the floor. - if (s.length <= PM_CAP) return pmMask(s) + if (s.length <= PM_CAP) return pmMask(s, opts) // the head ends at whitespace of any kind, so no token straddles it (a // head with no whitespace at all is one token: nothing is emitted) const ws = s.slice(0, PM_CAP).search(/\s\S*$/) @@ -1583,17 +1589,18 @@ export namespace Telemetry { // masked the same way with and without the continuation, cut back to // whitespace. Whatever the continuation changes is dropped, never // emitted half-proven. - const alone = pmMask(head) - const seen = pmMask(s.slice(0, at + PM_LOOKAHEAD)) + const alone = pmMask(head, opts) + const seen = pmMask(s.slice(0, at + PM_LOOKAHEAD), opts) let n = 0 while (n < alone.length && alone[n] === seen[n]) n++ if (n === alone.length) return alone const back = alone.slice(0, n).search(/\s\S*$/) return back >= 0 ? alone.slice(0, back).trimEnd() : "" } + // altimate_change end // the masking chain proper, on bounded input (see maskString) - function pmMask(s: string): string { + function pmMask(s: string, opts?: { maskPaths?: boolean }): string { let out = s // ANSI CSI sequences (colored subprocess stderr) would otherwise split // tokens so neither credential nor path rules can see them @@ -1604,7 +1611,13 @@ export namespace Telemetry { .replace(/"(?:[^"\\]|\\.)*"/g, "?") // Fast path: a string with no separator cannot contain a path — skip the // whole path stack (most telemetry strings carry no path at all). - if (out.includes("/") || out.includes("\\") || /(?` before the curlContext lookback below ever runs, which both + // destroys the write paths this ledger exists to report AND — the actual + // security bug — erases the `curl` token the lookback needs, so a + // path-qualified `curl -u user password` credential slips through + // unredacted. Every other telemetry mask (api keys, bearer tokens, + // emails, internal hosts, quote collapsing) still applies. + let masked = Telemetry.maskString(value, { maskPaths: false }) + + // altimate_change start — belt-and-suspenders curl detection. Derive + // curlContext for each `-u`/`--user` occurrence from the RAW, pre-mask + // `value` as well as from `masked`. maskPaths:false above is what keeps + // the `curl` token intact today; this makes detection structurally + // independent of that single flag, so a future masking rule that happens + // to eat the command-name token can't quietly reopen this leak. The two + // occurrence lists are correlated by ordinal position (same regex, same + // match order); if a prior mask ever changes how many times the pattern + // matches, this safely falls back to the masked-only signal — no worse + // than before this change. + const rawCurlByOrdinal = [...value.matchAll(USER_FLAG_RE)].map((m) => + isCurlContext(shellSegmentBefore(value, m.index + m[1].length)), + ) + const maskedOccurrenceCount = [...masked.matchAll(USER_FLAG_RE)].length + const ordinalsAligned = maskedOccurrenceCount === rawCurlByOrdinal.length + let ordinal = 0 + // altimate_change end // `-u` is also a benign flag for commands such as `git push -u` and // `python -u`. Redact it as authentication only in the current curl shell @@ -677,7 +717,7 @@ export namespace SessionCompaction { // `--user` follows the same rule so task literals are not discarded merely // because an unrelated CLI chose that option name. masked = masked.replace( - /(^|\s)(--user|-u)(?:(=|\s+)("[^"]*"|'[^']*'|[^\s,;]+)|([^\s,;]+))(?:(\s+)("[^"]*"|'[^']*'|[^\s,;]+))?/gi, + USER_FLAG_RE, ( match, lead: string, @@ -690,13 +730,17 @@ export namespace SessionCompaction { offset: number, whole: string, ) => { + // altimate_change — capture and advance the ordinal before any early + // return so it always tracks this callback's position in `masked`'s + // match sequence, matching how rawCurlByOrdinal was built. + const currentOrdinal = ordinal++ // Attached values are valid only for short `-u` (`-ualice:pass`). if (flag.toLowerCase() === "--user" && separator === undefined) return match const rawValue = (separatedValue ?? attachedValue ?? "").replace(/^["']|["']$/g, "") - // Windows invokes curl as `curl.exe`, and either platform may reach it - // through a path such as /usr/bin/curl or a Windows System32 path. - // Missing those spellings left the `-u` VALUE unredacted. - const curlContext = /(?:^|[\s/\\])curl(?:\.exe)?(?=\s|$)/i.test(shellSegmentBefore(whole, offset + lead.length)) + const maskedCurlContext = isCurlContext(shellSegmentBefore(whole, offset + lead.length)) + // altimate_change — OR in the raw-value signal (see block above). + const curlContext = + maskedCurlContext || (ordinalsAligned && (rawCurlByOrdinal[currentOrdinal] ?? false)) // Outside a curl context a colon-shaped value is treated as // user:password. The ONE exemption is an explicitly recognized // all-numeric UID:GID pair (`docker run --user 1000:1000`), which is a diff --git a/packages/opencode/test/session/compaction-ledger.test.ts b/packages/opencode/test/session/compaction-ledger.test.ts index afe0afc0c..bb724360f 100644 --- a/packages/opencode/test/session/compaction-ledger.test.ts +++ b/packages/opencode/test/session/compaction-ledger.test.ts @@ -439,6 +439,25 @@ describe("SessionCompaction.renderLedger", () => { expect(SessionCompaction.redactLedgerDetail("curlywurly -u alice script.py")).toBe("curlywurly -u alice script.py") }) + test("does not leak credentials via a path-qualified curl and the space-separated -u idiom (regression, #1117)", () => { + // #1117 added a filesystem-path masking rule to Telemetry.maskString. + // redactLedgerDetail used to call maskString BEFORE its own curl-context + // lookback, so `/usr/bin/curl` collapsed to `` first. The `curl` + // token was gone by the time the lookback ran: curlContext came back + // false, `-u alice hunter2` (space-separated, non-colon-shaped) isn't + // credentialShaped either, and the credential passed through untouched + // into ledger text that later gets persisted into a model prompt across + // compaction. This is the exact adversarial shape of that leak. + for (const command of [ + "/usr/bin/curl -u alice hunter2 https://example.com", + "/usr/local/bin/curl.exe -u alice hunter2 https://example.com", + ]) { + const detail = SessionCompaction.redactLedgerDetail(command) + expect(detail).not.toContain("alice") + expect(detail).not.toContain("hunter2") + } + }) + test("keeps non-credential colon-shaped values outside a curl context", () => { // `1000:1000` has no alphabetic character before the colon, so it is a // UID:GID pair rather than user:password and must survive in the ledger.