fix(diagnostics): harden the bug-report scrubber (5 audited leak/correctness gaps) (#856)
* fix(diagnostics): harden the bug-report scrubber against 5 audited leak/correctness gaps Audit of the (already-on-main) diagnostics/bug-report feature found the opt-in/ no-telemetry contract clean but 5 real gaps in the redaction + URL assembly. Fixed in both scrub twins (backend/core/scrub.py + frontend utils/bugReport.js): - Windows home paths with lowercase 'users' now redact (case-insensitive) — a spec-level PII leak: c:\users\john\… kept the username verbatim. - Broadened credential shapes (JWT/Bearer, Google AIza, Slack xox, AWS AKIA) + a URL query-secret pass (?token=/?api_key=… → value redacted, name kept) so a secret propagated from a backend error into error.message/.stack can't reach a public issue. The webview has no env backstop, so these shapes are its only defense. - Boundary-safe $HOME replace: a home of /Users/john no longer rewrites /Users/johnny to '~ny' (fragment leak + path mangling). - Bug-report URL now bounds the URL-ENCODED body length (~7k), not the raw length — a dense 6k markdown body encoded to ~9k and blew past GitHub's ceiling (silent truncation / failed open). Message body is capped too. - 9 new scrub regressions (backend) + 9 (frontend); all green. No API/behavior change beyond stricter redaction. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(changelog): note the bug-report scrubber hardening in [0.3.8] (#856) --------- Co-authored-by: mergetest <test@local> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Palash Debnath
co-authored by
Claude Opus 4.8
mergetest
Claude Opus 4.8
mergetest
parent
5e2d314efe
commit
7c0b0c2572
@@ -177,6 +177,15 @@ across dub, generate, and design (a corrupt-binary failure no longer poses as
|
||||
"get a free token" link. (#657, #669)
|
||||
### Fixed
|
||||
|
||||
- **Bug reports redact more secrets and every Windows username casing.** The
|
||||
opt-in bug-report scrubber now catches more credential shapes (JWT/Bearer,
|
||||
Google, Slack, AWS keys, and `?token=`/`?api_key=` URL secrets), redacts
|
||||
Windows home paths regardless of `Users`/`users` casing, and stops a superstring
|
||||
username (`/Users/john` vs `/Users/johnny`) from leaking a fragment. The
|
||||
prefilled-issue URL is now bounded by its *encoded* length so a large report
|
||||
can't silently truncate. Nothing new leaves the machine — this only makes the
|
||||
existing local-first, user-reviewed report stricter. (#856)
|
||||
|
||||
- **A hung TTS generate can no longer brick the backend ("Can't reach the local
|
||||
backend").** A GPU job that wedges on some Windows + CUDA setups occupies its
|
||||
worker forever — Python can't cancel the thread — so on the 1–2 worker pools we
|
||||
|
||||
+34
-7
@@ -36,18 +36,39 @@ _TOKEN_PATTERNS = (
|
||||
re.compile(r"github_pat_[A-Za-z0-9_]{20,}"), # GitHub fine-grained PAT
|
||||
re.compile(r"gh[pousr]_[A-Za-z0-9]{30,}"), # GitHub classic tokens
|
||||
re.compile(r"sk-[A-Za-z0-9_\-]{20,}"), # OpenAI-style API keys
|
||||
# A backend error can carry a secret from *any* provider (the LLM-providers
|
||||
# feature ships a dozen), so match the common credential shapes too, not
|
||||
# just the four vendors above — a leaked key in a public issue is real harm.
|
||||
re.compile(r"eyJ[A-Za-z0-9_\-]{8,}\.[A-Za-z0-9_\-]{8,}\.[A-Za-z0-9_\-]{6,}"), # JWT (Bearer)
|
||||
re.compile(r"AIza[0-9A-Za-z_\-]{35}"), # Google API key
|
||||
re.compile(r"xox[baprs]-[A-Za-z0-9\-]{10,}"), # Slack token
|
||||
re.compile(r"AKIA[0-9A-Z]{16}"), # AWS access key id
|
||||
re.compile(r"(?i)bearer\s+[A-Za-z0-9._\-]{16,}"), # opaque bearer tokens
|
||||
)
|
||||
|
||||
# Secrets carried in a URL query string (`?token=…`, `&api_key=…`). Redact the
|
||||
# VALUE while keeping the param name + separator so the URL stays legible. Bare
|
||||
# `key=` is intentionally excluded — too common in non-secret text; shaped keys
|
||||
# are already caught above and named env vars by the sweep below.
|
||||
_URL_SECRET_RE = re.compile(
|
||||
r"((?:access[_-]?token|api[_-]?key|apikey|auth[_-]?token|token|secret|password|passwd|pwd)=)"
|
||||
r"([^&\s\"'#]{6,})",
|
||||
re.IGNORECASE,
|
||||
)
|
||||
|
||||
# Home-directory shapes for all three supported platforms. Matched
|
||||
# pattern-wise (not just this machine's $HOME) so paths quoted from a
|
||||
# user's pasted log on another OS get cleaned too.
|
||||
# IGNORECASE because Windows is case-insensitive and tools routinely emit the
|
||||
# lowercase `c:\users\<name>` form, which the CLAUDE.md redaction spec still
|
||||
# requires to become `~`. `Users`/`users`, `Home`/`home` all match.
|
||||
_HOME_PATTERNS = (
|
||||
# Windows-with-forward-slashes must run BEFORE the bare macOS shape, or
|
||||
# `/Users/<name>` inside `C:/Users/<name>` gets eaten first, leaving `C:~`.
|
||||
re.compile(r"[A-Za-z]:/Users/[^/\s\"']+"), # Windows, forward slashes (file URLs, normalized traces)
|
||||
re.compile(r"/Users/[^/\s\"']+"), # macOS
|
||||
re.compile(r"/home/[^/\s\"']+"), # Linux
|
||||
re.compile(r"[A-Za-z]:\\Users\\[^\\\s\"']+"), # Windows, backslashes
|
||||
re.compile(r"[A-Za-z]:/Users/[^/\s\"']+", re.IGNORECASE), # Windows, forward slashes
|
||||
re.compile(r"/Users/[^/\s\"']+", re.IGNORECASE), # macOS
|
||||
re.compile(r"/home/[^/\s\"']+", re.IGNORECASE), # Linux
|
||||
re.compile(r"[A-Za-z]:\\Users\\[^\\\s\"']+", re.IGNORECASE), # Windows, backslashes
|
||||
)
|
||||
|
||||
# Values shorter than this are too entropy-poor to be real secrets and too
|
||||
@@ -85,19 +106,25 @@ def scrub_text(text: str | None) -> str:
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
# 2. Credential-shaped substrings.
|
||||
# 2. Credential-shaped substrings + URL query secrets.
|
||||
for pat in _TOKEN_PATTERNS:
|
||||
try:
|
||||
s = pat.sub(REDACTED, s)
|
||||
except Exception:
|
||||
pass
|
||||
try:
|
||||
s = _URL_SECRET_RE.sub(lambda m: m.group(1) + REDACTED, s)
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
# 3. This process's real home dir (covers symlinked/nonstandard homes
|
||||
# the generic patterns miss), then the per-OS shapes.
|
||||
# the generic patterns miss), then the per-OS shapes. Boundary-aware so
|
||||
# a home of `/Users/john` doesn't rewrite `/Users/johnny` to `~ny`
|
||||
# (leaking the fragment + mangling the path).
|
||||
try:
|
||||
home = os.path.expanduser("~")
|
||||
if home and home not in ("/", "~"):
|
||||
s = s.replace(home, "~")
|
||||
s = re.sub(re.escape(home) + r"(?=[/\\\s\"']|$)", "~", s)
|
||||
except Exception:
|
||||
pass
|
||||
for pat in _HOME_PATTERNS:
|
||||
|
||||
@@ -29,15 +29,28 @@ const TOKEN_PATTERNS = [
|
||||
/github_pat_[A-Za-z0-9_]{20,}/g, // GitHub fine-grained PAT
|
||||
/gh[pousr]_[A-Za-z0-9]{30,}/g, // GitHub classic tokens
|
||||
/sk-[A-Za-z0-9_-]{20,}/g, // OpenAI-style API keys
|
||||
// A backend error can carry a secret from any provider over HTTP into
|
||||
// error.message/.stack — the webview has no env-var backstop, so these
|
||||
// shapes are its only defense. Mirror backend/core/scrub.py.
|
||||
/eyJ[A-Za-z0-9_-]{8,}\.[A-Za-z0-9_-]{8,}\.[A-Za-z0-9_-]{6,}/g, // JWT (Bearer)
|
||||
/AIza[0-9A-Za-z_-]{35}/g, // Google API key
|
||||
/xox[baprs]-[A-Za-z0-9-]{10,}/g, // Slack token
|
||||
/AKIA[0-9A-Z]{16}/g, // AWS access key id
|
||||
/bearer\s+[A-Za-z0-9._-]{16,}/gi, // opaque bearer tokens
|
||||
];
|
||||
|
||||
// Secrets in a URL query string — redact the value, keep the param name.
|
||||
const URL_SECRET_RE =
|
||||
/((?:access[_-]?token|api[_-]?key|apikey|auth[_-]?token|token|secret|password|passwd|pwd)=)([^&\s"'#]{6,})/gi;
|
||||
|
||||
const HOME_PATTERNS = [
|
||||
// Windows-with-forward-slashes must run BEFORE the bare macOS shape, or
|
||||
// `/Users/<name>` inside `C:/Users/<name>` gets eaten first, leaving `C:~`.
|
||||
/(?:file:\/\/\/)?[A-Za-z]:\/Users\/[^/\s"']+/g, // Windows, forward slashes (webview stacks, file:/// URLs)
|
||||
/\/Users\/[^/\s"']+/g, // macOS
|
||||
/\/home\/[^/\s"']+/g, // Linux
|
||||
/[A-Za-z]:\\Users\\[^\\\s"']+/g, // Windows, backslashes
|
||||
// `i` flag: Windows is case-insensitive and tools emit lowercase `c:\users\`.
|
||||
/(?:file:\/\/\/)?[A-Za-z]:\/Users\/[^/\s"']+/gi, // Windows, forward slashes (webview stacks, file:/// URLs)
|
||||
/\/Users\/[^/\s"']+/gi, // macOS
|
||||
/\/home\/[^/\s"']+/gi, // Linux
|
||||
/[A-Za-z]:\\Users\\[^\\\s"']+/gi, // Windows, backslashes
|
||||
];
|
||||
|
||||
/** Redact credential-shaped substrings and home directories. */
|
||||
@@ -45,6 +58,7 @@ export function scrubText(text) {
|
||||
if (text == null) return '';
|
||||
let s = String(text);
|
||||
for (const pat of TOKEN_PATTERNS) s = s.replace(pat, REDACTED);
|
||||
s = s.replace(URL_SECRET_RE, (_m, name) => `${name}${REDACTED}`);
|
||||
for (const pat of HOME_PATTERNS) s = s.replace(pat, '~');
|
||||
return s;
|
||||
}
|
||||
@@ -52,7 +66,26 @@ export function scrubText(text) {
|
||||
// GitHub truncates very long prefill URLs; keep the encoded result well
|
||||
// under the ~8k practical ceiling so the user never loses the form.
|
||||
const MAX_STACK_CHARS = 1800;
|
||||
const MAX_BODY_CHARS = 6000;
|
||||
const MAX_MSG_CHARS = 1200;
|
||||
// The real ceiling is on the URL-ENCODED body, not the raw string: markdown
|
||||
// encodes ~1.3–1.6× larger (newlines→%0A, spaces→%20, backticks/#//), so a
|
||||
// 6000-char raw body can be ~9k encoded and blow past GitHub's limit. Bound
|
||||
// the encoded length directly.
|
||||
const MAX_ENCODED_BODY = 7000;
|
||||
|
||||
/** Trim `text` so its URL-encoded length is ≤ maxEncoded (binary search on
|
||||
* the raw cut point — exact, and cheap for report-sized strings). */
|
||||
function fitEncoded(text, maxEncoded) {
|
||||
if (encodeURIComponent(text).length <= maxEncoded) return text;
|
||||
let lo = 0;
|
||||
let hi = text.length;
|
||||
while (lo < hi) {
|
||||
const mid = Math.ceil((lo + hi) / 2);
|
||||
if (encodeURIComponent(text.slice(0, mid)).length <= maxEncoded) lo = mid;
|
||||
else hi = mid - 1;
|
||||
}
|
||||
return `${text.slice(0, lo)}\n… (truncated)`;
|
||||
}
|
||||
|
||||
/** Bound every context fetch: a backend that accepts the socket and then
|
||||
* stalls must not pin the report button / error-toast / boundary flow on the
|
||||
@@ -124,14 +157,19 @@ export async function buildBugReportUrl({ title = '[Bug] ', error } = {}) {
|
||||
// Seed the title with the failure so the issue list stays scannable;
|
||||
// the user can still edit it on github.com before submitting.
|
||||
if (title === '[Bug] ' && msg) title = `[Bug] ${msg.slice(0, 80)}`;
|
||||
// Cap the message in the body too — a large payload (validation dump,
|
||||
// HTML/JSON response body) would otherwise inflate the report past the
|
||||
// encoded URL ceiling.
|
||||
const msgForBody =
|
||||
msg.length > MAX_MSG_CHARS ? `${msg.slice(0, MAX_MSG_CHARS)}\n… (truncated)` : msg;
|
||||
let stack = error?.stack ? scrubText(error.stack) : '';
|
||||
if (stack.length > MAX_STACK_CHARS) stack = `${stack.slice(0, MAX_STACK_CHARS)}\n… (truncated)`;
|
||||
errorSection.push(
|
||||
'## Error',
|
||||
'',
|
||||
'```',
|
||||
msg,
|
||||
...(stack && stack !== msg ? [stack] : []),
|
||||
msgForBody,
|
||||
...(stack && stack !== msgForBody ? [stack] : []),
|
||||
'```',
|
||||
'',
|
||||
);
|
||||
@@ -162,7 +200,7 @@ export async function buildBugReportUrl({ title = '[Bug] ', error } = {}) {
|
||||
'<!-- step-by-step would help us reproduce -->',
|
||||
'',
|
||||
].join('\n');
|
||||
if (body.length > MAX_BODY_CHARS) body = `${body.slice(0, MAX_BODY_CHARS)}\n… (truncated)`;
|
||||
body = fitEncoded(body, MAX_ENCODED_BODY);
|
||||
|
||||
return `${ISSUES_URL}?title=${encodeURIComponent(title)}&labels=${encodeURIComponent('bug')}&body=${encodeURIComponent(body)}`;
|
||||
}
|
||||
|
||||
@@ -33,6 +33,43 @@ describe('scrubText — frontend twin of backend/core/scrub.py', () => {
|
||||
expect(scrubText(null)).toBe('');
|
||||
expect(scrubText(undefined)).toBe('');
|
||||
});
|
||||
|
||||
// ── Hardening regressions (diagnostics audit) ──────────────────────────
|
||||
it.each([['c:\\users\\john\\log.txt'], ['C:\\Users\\john\\log.txt'], ['C:/USERS/john/log.txt']])(
|
||||
'redacts Windows home regardless of case: %s',
|
||||
(raw) => {
|
||||
expect(scrubText(raw)).not.toContain('john');
|
||||
},
|
||||
);
|
||||
|
||||
// Built from low-entropy parts so they match the scrubber's shape without
|
||||
// tripping GitHub push-protection secret scanning.
|
||||
it.each([
|
||||
[`eyJ${'a'.repeat(20)}.${'b'.repeat(20)}.${'c'.repeat(20)}`], // JWT
|
||||
[`AIza${'B'.repeat(35)}`], // Google
|
||||
[`xox${'b-'}${'C'.repeat(20)}`], // Slack
|
||||
[`AKIA${'D'.repeat(16)}`], // AWS
|
||||
])('redacts broadened credential shape %s', (secret) => {
|
||||
expect(scrubText(`error: ${secret}`)).not.toContain(secret);
|
||||
});
|
||||
|
||||
it('redacts a URL query secret value but keeps the param name', () => {
|
||||
const out = scrubText('GET https://h/api?token=supersecretvalue12345&x=1');
|
||||
expect(out).not.toContain('supersecretvalue12345');
|
||||
expect(out).toContain('token=');
|
||||
expect(out).toContain('x=1');
|
||||
});
|
||||
});
|
||||
|
||||
describe('buildBugReportUrl — encoded length ceiling', () => {
|
||||
it('keeps the ENCODED body under the ceiling even when the raw body is dense', async () => {
|
||||
// A body full of chars that expand under encodeURIComponent (newlines,
|
||||
// spaces, backticks) must still yield a URL comfortably under ~8k.
|
||||
const error = new Error('x'.repeat(200));
|
||||
error.stack = `${'trace line with spaces\n'.repeat(500)}`;
|
||||
const url = await buildBugReportUrl({ error });
|
||||
expect(url.length).toBeLessThan(8000);
|
||||
});
|
||||
});
|
||||
|
||||
describe('buildBugReportUrl', () => {
|
||||
|
||||
@@ -106,3 +106,43 @@ def test_none_and_empty():
|
||||
|
||||
def test_non_string_coerced():
|
||||
assert scrub_text(42) == "42"
|
||||
|
||||
|
||||
# ── Hardening regressions (diagnostics audit) ─────────────────────────────
|
||||
|
||||
@pytest.mark.parametrize("raw", [
|
||||
r"c:\users\john\AppData\log.txt", # lowercase drive + Users
|
||||
r"C:\Users\john\AppData\log.txt", # canonical
|
||||
"C:/USERS/john/app/log.txt", # upper, forward slashes
|
||||
])
|
||||
def test_windows_home_case_insensitive(raw):
|
||||
# The username must never survive, regardless of Users/users casing.
|
||||
assert "john" not in scrub_text(raw)
|
||||
|
||||
|
||||
# Built from low-entropy parts (not real-secret literals) so they match the
|
||||
# scrubber's shape without tripping GitHub push-protection secret scanning.
|
||||
@pytest.mark.parametrize("secret", [
|
||||
"eyJ" + "a" * 20 + "." + "b" * 20 + "." + "c" * 20, # JWT
|
||||
"AIza" + "B" * 35, # Google API key
|
||||
"xox" + "b-" + "C" * 20, # Slack
|
||||
"AKIA" + "D" * 16, # AWS access key id
|
||||
])
|
||||
def test_broadened_token_shapes_redacted(secret):
|
||||
assert secret not in scrub_text(f"request failed: {secret} (401)")
|
||||
|
||||
|
||||
def test_url_query_secret_value_redacted_name_kept():
|
||||
out = scrub_text("open https://host/api?token=supersecretvalue12345&x=1")
|
||||
assert "supersecretvalue12345" not in out
|
||||
assert "token=" in out # param name preserved for legibility
|
||||
assert "x=1" in out # non-secret params untouched
|
||||
|
||||
|
||||
def test_home_superstring_not_corrupted(monkeypatch):
|
||||
# A home of /Users/john must not rewrite /Users/johnny to '~ny'.
|
||||
monkeypatch.setenv("HOME", "/Users/john")
|
||||
out = scrub_text("/Users/johnny/secret.wav")
|
||||
assert "~ny" not in out
|
||||
assert "johnny" not in out # still redacted by the generic macOS shape
|
||||
assert out == "~/secret.wav"
|
||||
|
||||
Reference in New Issue
Block a user