fix(security): close native SSE and credential-redaction gaps

[DivyamTalwar]

Summary

This release-hardening PR consolidates and completes the three security findings raised in PRs #31, #32, and #33. It protects the native companion SSE boundary and makes content-shaped credential redaction cover both third-party and Helmryth-native token formats without turning ordinary technical prose into redacted text.

Changes

1. Fail closed when companion SSE scrubbing cannot complete

companion/src/wire.ts now keeps JSON parsing failures separate from scrub failures:

• non-JSON data: lines remain pass-through for protocol compatibility;
• valid JSON that cannot be scrubbed (for example, extreme nesting that exhausts recursive traversal) is replaced with data: {};
• withheld fields such as resumeCursors and sshAlias can never be forwarded merely because the scrubber threw.

This closes the confirmed fail-open path identified by PR #31.

2. Redact GitLab and PyPI credentials in free-form content

server/redact.ts now recognizes:

• glpat-… GitLab personal access tokens;
• pypi-… PyPI macaroon tokens.

The PyPI matcher requires a 48-character payload, matching the long macaroon shape while avoiding common packaging prose such as pypi-publishing-workflow and pypi-mirror-configuration.

3. Redact Helmryth-native Box and webhook credentials

The same content-shaped redactor now covers:

• box_live_…, box_test_…, and box_prod_… Box API tokens using the documented environment qualifier and a 16-character payload floor;
• whsec_… webhook secrets.

The Box rule catches the product’s live/test/prod token forms, including mixed-case payloads, while leaving ordinary identifiers such as box_model_border, box_sizing_content, and box_background_color untouched. Key-name redaction still covers short or legacy box_ values stored under HELMRYTH_BOX_TOKEN.

4. Keep Electron diagnostics in redaction parity

The Electron diagnostics exporter maintains a JavaScript-only redaction table because it cannot import the TypeScript server module. Its patterns now mirror the server for GitLab, PyPI, qualified Box, and webhook credentials, with a dedicated parity regression test covering every new format.

Verification

Focused proof on the exact consolidated tree:

• companion/test/wire.test.ts: 16/16 passed;
• server/redact.test.ts: 13/13 passed;
• electron/diagnostics.test.mjs: 27/27 passed;
• combined focused suite: 56/56 passed;
• pnpm typecheck: passed;
• pnpm lint: passed;
• pnpm build: passed;
• git diff --check: passed.

The branch is based on the current main (92556ba, including the latest merged release-hardening work), has no unrelated files, and preserves the original contributor commits as cherry-picked changes. The final PR also adds regression cases for deep SSE payloads, PyPI false-positive prose, Box false-positive identifiers, and diagnostics parity.

Security impact

• Companion clients receive an empty safe JSON frame instead of an unscrubbed frame when sanitization fails.
• Native protocol logs and free-form transcript surfaces now remove additional credential formats.
• The patterns remain deliberately high precision; generic long strings, code, URLs, and ordinary documentation text are not redacted.

Rollback

Revert this PR. No data migration or persistent schema change is included.

Related PRs

• Supersedes fix(companion): prevent withheld keys leaking through SSE on scrub failure #31 (companion SSE scrub failure handling)
• Supersedes fix(redact): add GitLab and PyPI token prefixes to secret detection #32 (GitLab/PyPI content-shaped credentials)
• Supersedes fix(redact): catch product-native credentials in content-shaped secret detection #33 (Box/webhook content-shaped credentials)

[Copilot]

🔵 Needs a closer look

The Box pattern can redact ordinary identifiers such as box_background_color; refine it before approval.

Pull request overview

Hardens native SSE scrubbing and expands credential redaction for GitLab, PyPI, Box, and webhook tokens.

Changes:

• Fail closed when SSE scrubbing fails.
• Add credential patterns and regression tests.
• Preserve non-JSON SSE passthrough behavior.

File summaries

File	Changes
server/redact.ts	Adds credential-prefix redaction rules.
server/redact.test.ts	Tests token formats and false-positive exclusions.
companion/test/wire.test.ts	Covers deep, unscrubable SSE payloads.
companion/src/wire.ts	Separates parsing and scrubbing failure handling.

Review details

Suppressed comments (1)

server/redact.ts:57

• This pattern treats any identifier beginning with box_ as a credential once the suffix reaches 16 characters. For example, box_background_color has a 16-character suffix and is rewritten by redactSecretsInText, so ordinary CSS/configuration code is redacted despite this module's high-precision requirement. Please use a token-specific shape or otherwise add a safe distinction; the length floor alone is not sufficient.

  /\bbox_[A-Za-z0-9_-]{16,}/g, // ascii.dev box api token

• Files reviewed: 4/4 changed files
• Comments generated: 0
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[DivyamTalwar]

Addressed the Copilot review finding in commit 2d9f3fc: the Box content matcher now requires a live, test, or prod qualifier and covers a mixed-case payload. Added regression coverage for box_background_color, box_model_border, and box_sizing_content; the focused security suite remains 29/29 green. Fresh required CI is running on this updated head.

[Copilot]

🔵 Needs a closer look

Add regression coverage for the documented box_prod_... token form before approval.

Review details

Suppressed comments (1)

server/redact.test.ts:174

• The new rule supports three Box qualifiers, but the regression matrix only exercises live and test. A future typo or removal of the prod alternative would leave these tests green, so add a box_prod_... fixture to keep all documented native token forms covered.

      [`box ${"box_" + "test_"}ABCDEF0123456789`, /box_test_/],

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Lite

[DivyamTalwar]

Added the requested box_prod_... regression fixture in commit 6188f01. The focused wire/redaction suite remains 29/29 green; lint, typecheck, and diff checks pass. Fresh required CI is running for this final test-complete head.

[Copilot]

🟡 Changes recommended

Electron diagnostics redaction does not yet cover the new credential formats.

Get a fresh assessment by requesting another Copilot review.

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟢 Approval recommended

No unresolved blocking issues were identified.

Review details

• Files reviewed: 6/6 changed files
• Comments generated: 0 new
• Review effort level: Lite

[DivyamTalwar]

Approved after the final direct security review, GitHub Copilot follow-up, and full required CI.

PR #41 consolidates the three security fixes from PRs #31–#33: fail-closed native SSE scrubbing, high-precision GitLab/PyPI/Box/webhook token detection, and Electron diagnostics parity. The focused suites pass 56/56 (wire 16/16, server redaction 13/13, diagnostics 27/27); typecheck, lint, build, and diff checks also pass. Copilot's findings on Box false positives, box_prod coverage, and diagnostics parity are resolved.

Required macOS, Ubuntu, Windows, Swift/iOS, Registry/workerd, and package-smoke jobs are all green. The change is additive and has no data migration; rollback is a normal revert.