From c8f078b4822708f5c9c6e947adfe7889bf33ef84 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Thu, 11 Jun 2026 20:38:06 -0700 Subject: [PATCH] fix(telemetry): redact error_message spans before they leave the machine (#1947) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit error_message was uploaded with only quote/newline escaping — stack traces and failed-API errors can embed credentials, private paths, and hostnames, and the sync path strips only _repo_slug/_branch. New lib/redact-engine.ts export redactFindingSpans(): replaces EVERY finding's span with regardless of tier (applyRedactions is the interactive PII-only path and exits nonzero on credential findings, so it can't serve machine egress). Returns null when a span can't be located — callers drop the whole payload rather than risk a leak. gstack-telemetry-log pipes error_message through it at LOG time, so the local JSONL at rest is clean too; surrounding text survives for crash triage. FAIL CLOSED: bun missing, engine error, or non-JSON-string output all null the field. Tests pin: embedded ghp_ token → with context intact; redactor unavailable → null; raw bytes on disk never contain the token. Co-Authored-By: Claude Fable 5 --- bin/gstack-telemetry-log | 27 ++++++++++++++++++++++++++- lib/redact-engine.ts | 22 ++++++++++++++++++++++ test/telemetry.test.ts | 35 +++++++++++++++++++++++++++++++++++ 3 files changed, 83 insertions(+), 1 deletion(-) diff --git a/bin/gstack-telemetry-log b/bin/gstack-telemetry-log index 03aa3db07..0dd78da57 100755 --- a/bin/gstack-telemetry-log +++ b/bin/gstack-telemetry-log @@ -18,6 +18,12 @@ set -uo pipefail GSTACK_DIR="${GSTACK_DIR:-$(cd "$(dirname "$0")/.." && pwd)}" +SCRIPT_DIR="$GSTACK_DIR/bin" +# Windows git-bash (#1950): pwd yields a POSIX path (/c/Users/...), which Bun +# on Windows cannot resolve as an ES module specifier in bun -e imports. +case "$(uname -s)" in + MINGW*|MSYS*|CYGWIN*) command -v cygpath >/dev/null 2>&1 && SCRIPT_DIR="$(cygpath -m "$SCRIPT_DIR")" ;; +esac STATE_DIR="${GSTACK_STATE_DIR:-$HOME/.gstack}" ANALYTICS_DIR="$STATE_DIR/analytics" JSONL_FILE="$ANALYTICS_DIR/skill-usage.jsonl" @@ -177,8 +183,27 @@ BRANCH="$(json_safe "$BRANCH")" ERR_FIELD="null" [ -n "$ERROR_CLASS" ] && ERR_FIELD="\"$(json_safe "$ERROR_CLASS")\"" +# error_message goes through the redaction engine before it touches disk +# (#1947): stack traces and failed-API errors can embed credentials, paths, +# and hostnames. Every finding span becomes ; the rest of the +# message survives for crash triage. The bun snippet emits a JSON-encoded +# string (quotes included) ready to drop into the printf below. FAIL CLOSED: +# if bun / the engine is unavailable, the scan errors, or the output doesn't +# look like a JSON string, the whole message becomes null — never raw. ERR_MSG_FIELD="null" -[ -n "$ERROR_MESSAGE" ] && ERR_MSG_FIELD="\"$(printf '%s' "$ERROR_MESSAGE" | head -c 200 | sed -e 's/\\/\\\\/g' -e 's/"/\\"/g' -e 's/ /\\t/g' | tr '\n\r' ' ')\"" +if [ -n "$ERROR_MESSAGE" ]; then + ERR_MSG_FIELD="$(printf '%s' "$ERROR_MESSAGE" | bun -e " +import { redactFindingSpans } from '$SCRIPT_DIR/../lib/redact-engine.ts'; +const input = await Bun.stdin.text(); +const out = redactFindingSpans(input, { repoVisibility: 'private' }); +if (out === null) process.exit(1); +console.log(JSON.stringify(out.slice(0, 200))); +" 2>/dev/null)" || ERR_MSG_FIELD="null" + case "$ERR_MSG_FIELD" in + \"*\") ;; # JSON string — safe to embed + *) ERR_MSG_FIELD="null" ;; + esac +fi STEP_FIELD="null" [ -n "$FAILED_STEP" ] && STEP_FIELD="\"$(json_safe "$FAILED_STEP")\"" diff --git a/lib/redact-engine.ts b/lib/redact-engine.ts index 02cf66829..42b650f1a 100644 --- a/lib/redact-engine.ts +++ b/lib/redact-engine.ts @@ -427,6 +427,28 @@ export function applyRedactions( return { body, diff: diffLines.reverse().join("\n"), skipped }; } +/** + * Replace EVERY finding's span with ``, regardless of tier or + * autoRedactable. For machine egress surfaces (telemetry error_message, + * #1947) where structure preservation doesn't matter and fail-closed beats + * fidelity. Returns null when any finding's span cannot be located — the + * caller must then drop the whole payload rather than risk leaking an + * unlocated secret. (Contrast applyRedactions, which is the interactive, + * autoRedactable-only, structure-preserving path.) + */ +export function redactFindingSpans(input: string, opts: ScanOptions = {}): string | null { + const { findings } = scan(input, opts); + const targets = findings.map((f) => ({ f, ...locateSpan(input, f) })); + if (targets.some((t) => t.start < 0)) return null; + // Right-to-left so earlier offsets remain valid after splicing. + targets.sort((a, b) => b.start - a.start); + let body = input; + for (const t of targets) { + body = body.slice(0, t.start) + `` + body.slice(t.end); + } + return body; +} + function locateSpan(input: string, f: Finding): { start: number; end: number } { // Re-derive the offset from line/col on the original text. let offset = 0; diff --git a/test/telemetry.test.ts b/test/telemetry.test.ts index 96bdf54c7..63c9e67f3 100644 --- a/test/telemetry.test.ts +++ b/test/telemetry.test.ts @@ -201,6 +201,41 @@ describe('gstack-telemetry-log', () => { expect(event.error_message).toContain('not found'); }); + test('redacts credential spans in error_message before they touch disk (#1947)', () => { + setConfig('telemetry', 'anonymous'); + const token = 'ghp_' + 'A1b2C3d4E5f6G7h8I9j0K1l2M3n4O5p6Q7r8'; + run( + `${BIN}/gstack-telemetry-log --skill qa --duration 10 --outcome error --error-message 'push failed: auth ${token} rejected by remote' --session-id red-1`, + ); + + const lines = readJsonl(); + expect(lines).toHaveLength(1); + const event = JSON.parse(lines[0]); + // The span is masked, the surrounding triage context survives. + expect(event.error_message).toContain(''); + expect(event.error_message).toContain('push failed'); + expect(event.error_message).not.toContain(token); + // Raw bytes on disk never contain the token either. + expect(lines[0]).not.toContain(token); + }); + + test('fails closed: error_message becomes null when the redactor is unavailable (#1947)', () => { + setConfig('telemetry', 'anonymous'); + const token = 'ghp_' + 'A1b2C3d4E5f6G7h8I9j0K1l2M3n4O5p6Q7r8'; + // PATH without bun: the redaction snippet cannot run, so the whole + // message must drop — never raw passthrough. + run( + `${BIN}/gstack-telemetry-log --skill qa --duration 10 --outcome error --error-message 'auth ${token} rejected' --session-id red-2`, + { PATH: '/usr/bin:/bin' }, + ); + + const lines = readJsonl(); + expect(lines).toHaveLength(1); + const event = JSON.parse(lines[0]); + expect(event.error_message).toBeNull(); + expect(lines[0]).not.toContain(token); + }); + test('creates analytics directory if missing', () => { // Remove analytics dir const analyticsDir = path.join(tmpDir, 'analytics');