fix(telemetry): redact error_message spans before they leave the machine (#1947)

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 <REDACTED-{id}> 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 → <REDACTED-github.pat>
with context intact; redactor unavailable → null; raw bytes on disk never
contain the token.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Garry Tan 2026-06-11 20:38:06 -07:00
parent b3085f137e
commit c8f078b482
No known key found for this signature in database
GPG Key ID: C1F69E85C74EFE1D
3 changed files with 83 additions and 1 deletions

View File

@ -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 <REDACTED-{id}>; 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")\""

View File

@ -427,6 +427,28 @@ export function applyRedactions(
return { body, diff: diffLines.reverse().join("\n"), skipped };
}
/**
* Replace EVERY finding's span with `<REDACTED-{id}>`, 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) + `<REDACTED-${t.f.id}>` + 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;

View File

@ -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('<REDACTED-github.pat>');
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');