From afca9b85ef40afc4a4fc6df96896823e9c8755e1 Mon Sep 17 00:00:00 2001 From: Dev Chiniwala Date: Tue, 8 Sep 2026 18:26:07 +0530 Subject: [PATCH] fix(companion): prevent withheld keys leaking through SSE on scrub failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit scrubEvent had a single try-catch around JSON.parse and scrub(). When scrub() threw (e.g. RangeError on deeply nested JSON), the catch treated it the same as "not JSON" and returned the original unscrubbed line — sending resumeCursors and sshAlias to the device. Split the catch: parse failure still passes through (not JSON, nothing to scrub), scrub failure replaces the data with an empty object. This mirrors the fix already applied in the proxy's JSON response path (proxy.ts lines 520–548), whose comment explicitly calls the scenario "not hypothetical". Co-Authored-By: Claude Opus 4.6 --- companion/src/wire.ts | 14 +++++++++++--- companion/test/wire.test.ts | 13 +++++++++++++ 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/companion/src/wire.ts b/companion/src/wire.ts index 95f8b30..6ba4840 100644 --- a/companion/src/wire.ts +++ b/companion/src/wire.ts @@ -136,13 +136,21 @@ function scrubEvent(event: string): string { if (!line.startsWith("data:")) return line; const raw = line.slice(5).trimStart(); if (!raw) return line; + let parsed; try { - return `data: ${JSON.stringify(scrub(JSON.parse(raw)))}`; + parsed = JSON.parse(raw); } catch { - // not JSON: pass it through rather than dropping it. A frame this - // code does not understand is still the harness's to send. return line; } + try { + return `data: ${JSON.stringify(scrub(parsed))}`; + } catch { + // scrub() recurses, so a body nested deep enough throws RangeError + // where JSON.parse handles it fine. Passing the original line through + // sends exactly what the scrubber exists to withhold — the same class + // of issue the proxy's JSON response path already handles. + return "data: {}"; + } }) .join(eol); } diff --git a/companion/test/wire.test.ts b/companion/test/wire.test.ts index c3af6c0..1db197d 100644 --- a/companion/test/wire.test.ts +++ b/companion/test/wire.test.ts @@ -166,4 +166,17 @@ describe("createSseScrubber", () => { it("still handles a bare CR, which the spec also allows", () => { expect(createSseScrubber()('data: {"a":1,"resumeCursors":{}}\r\r')).toBe('data: {"a":1}\r\r'); }); + + it("replaces unscrubable JSON data instead of leaking withheld keys", () => { + // scrub() recurses; a body nested a few thousand deep throws RangeError + // where JSON.parse handles it fine. A single try-catch around both treated + // that as "not JSON" and returned the original unscrubbed line — sending + // exactly what the scrubber exists to withhold. + let json = '{"resumeCursors":{"ghost":"leak-me"}}'; + for (let i = 0; i < 12_000; i++) json = `{"n":${json}}`; + const out = createSseScrubber()(`data: ${json}\n\n`); + expect(out).not.toContain("resumeCursors"); + expect(out).not.toContain("leak-me"); + expect(out).toBe("data: {}\n\n"); + }); });