Skip to content

fix: mask secrets in TransferState text and unsigned JWTs - #248

Merged
erkamyaman merged 1 commit into
pangular-inspector:mainfrom
erkamyaman:sec/redaction-leaks
Oct 11, 2026
Merged

erkamyaman merged 1 commit into
pangular-inspector:mainfrom
erkamyaman:sec/redaction-leaks

Conversation

@erkamyaman

@erkamyaman erkamyaman commented Oct 10, 2026 •

Copy link
Copy Markdown
Member

What and why

An audit of redaction and agent exposure turned up three leaks. I traced each one through the code and reproduced it:

  • TransferState entries leak secret keys (P1). The page clips any entry over 20,000 characters to a JSON string. Text bodies (responseType: 'text', or an Analog body holding JSON text) also arrive as strings. On the server those strings only had JWT, bearer and key=value masking applied, so {"password":"hunter2","apiKey":"sk-live-..."} reached the http-payloads shared state, the panel and agents in clear. sanitizePayload now also masks "key": value pairs by key inside every string, using redactJsonText with isRedactedKey. That covers redaction.secretNames too.
  • The NativeScript overlay ignored the redaction config (P2). It never called setRedaction, so custom secretNames and unmask had no effect on its component inputs, signal values and signal history. The server stores those reports as sent. It now applies the config when the session starts, as overlay.ts and overlay-angular-native.ts already do.
  • Unsigned JWTs were never masked (P3). Tokens with alg: none have an empty signature (eyJ...eyJ....). All three JWT patterns (forms-privacy.ts, router.ts, analog-server-log.ts) required a non-empty third segment. They now share one exported pattern that allows an empty signature.

The audit also checked the config gating. Every RPC and agent tool is behind its inspector toggle, and every write tool is in ACTION_TOOLS, so nothing needed changing there.

How it was verified

  • pnpm commit:check
  • pnpm format:check
  • pnpm typecheck
  • pnpm test:devtools (1430 tests pass). I did not run pnpm test or pnpm test:panel; app/ is unchanged.
  • pnpm skills:check (not applicable)
  • Docs: no option, label or tool changed; this only masks more values, so no docs page needed an update
  • pnpm extension:build (not applicable, app/ unchanged)
  • Browser check with axe (not applicable, no UI change)

New tests:

  • http-payload-redaction.test.ts covers a clipped entry over 20,000 characters, a JSON text body, secretNames inside a text body, and an unsigned JWT in text and previews.
  • overlay-nativescript-session.test.ts checks that the session applies redaction.secretNames from the connection.

Every new test fails without its fix and passes with it.

Notes for reviewers

  • The shared JWT pattern no longer requires a leading \b, matching what router.ts already did. It can mask a little more text that merely looks like a token; that is the safe direction for this pattern.
  • Clipping still happens on the page before serialization. Clipping only after serialize would be cleaner, but the server-side masking covers the clipped string either way.

Generated by Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Sensitive values are now masked in JSON text within HTTP payloads, including when payloads are clipped or contain nested data.
    • JWT-like tokens are more consistently masked in text and previews, including tokens with empty signatures.
    • Configured secret names now apply to text bodies and are used when a NativeScript session starts, helping prevent sensitive data from appearing in captured information.

TransferState entries the page clipped to a string, and text bodies that
hold JSON, only had token-shaped text masked, so keys like password or
apiKey reached the panel in clear. Mask "key": value pairs in those
strings by key, including redaction.secretNames.

The NativeScript overlay never applied the redaction config, so custom
secretNames and unmask were ignored for its component and signal values.
Apply it when the session starts, as the other overlays do.

JWTs with an empty signature (alg: none) were not matched by any of the
three JWT patterns. Share one pattern that also matches them.
@github-actions github-actions Bot added the area: package The ng-devtools package (packages/ng-devtools) label Oct 10, 2026
@coderabbitai

coderabbitai Bot commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 55aff6eb-1b5a-4fe7-8def-9bff7d030133

📥 Commits

Reviewing files that changed from the base of the PR and between f245551 and fa028fd.


📒 Files selected for processing (7)
  • packages/devtools/src/__tests__/http-payload-redaction.test.ts
  • packages/devtools/src/__tests__/overlay-nativescript-session.test.ts
  • packages/devtools/src/analog-server-log.ts
  • packages/devtools/src/forms-privacy.ts
  • packages/devtools/src/http-payload.ts
  • packages/devtools/src/overlay-nativescript.ts
  • packages/devtools/src/router.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 4 remain after this review.



📝 Walkthrough

Walkthrough

The change updates JWT matching, applies key-based redaction to HTTP payload values before clipping, and configures form redaction from connection metadata when a NativeScript session starts.

Changes

Redaction behavior

Layer / File(s) Summary
Shared JWT matching
packages/devtools/src/forms-privacy.ts, packages/devtools/src/analog-server-log.ts, packages/devtools/src/router.ts
JWT is exported with updated segment and signature matching. The server log and router use the shared pattern.
HTTP payload masking
packages/devtools/src/http-payload.ts, packages/devtools/src/__tests__/http-payload-redaction.test.ts
HTTP payload values are recursively processed for secret keys before clipping. Tests cover TransferState entries, JSON text bodies, configured secret names, and unsigned JWTs.
NativeScript session configuration
packages/devtools/src/overlay-nativescript.ts, packages/devtools/src/__tests__/overlay-nativescript-session.test.ts
Session startup sets form redaction from connection metadata. A test checks that a configured secret name is redacted after overlay initialization.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix


Merge Risk | ⚪ Minimal · up to fa028

Merge Risk: ⚪ Minimal · up to fa028

No actionable issue is established from the available evidence. The change is mergeable after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to fa028

The changes strengthen secret masking in diagnostic data. No introduced vulnerability was established, but isolation of redaction settings between overlapping sessions remains uncertain.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The policy's potential influence extends to serializers sharing the same loaded privacy module, rather than being keyed to one RPC session. Actual exposure across concurrent consumers is unresolved; the inspected reporting sinks transmit application diagnostics to the connected devtools server.

Trust Boundaries and Controls

  • observed — Connection metadata supplies NativeScript's secretNames and unmask policy. Resolution validates string entries and bounds list size and name length, but these checks validate policy shape rather than authenticate its issuer.

Resilience and Maintainability Implications

  • inferred — Initial ordering avoids telemetry being generated before the connection policy is applied. A failed startup's persistent policy becomes security-relevant only if another consumer subsequently serializes data under it; that concurrent-consumer condition was not established.

Hardening Proposals

  • proposed — Make single-consumer ownership explicit and enforced, or bind redaction policy to each reporting session. Any cleanup restoration should be ownership-aware so an older session cannot overwrite a newer session's policy.

Pre-merge checks | Passed 4 | Failed 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 7 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly describes two primary changes: masking secrets in TransferState text and supporting unsigned JWT redaction. It does not mention the NativeScript redaction configuration change, but t…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
🧪 Generate unit tests (beta)
  • Create a new PR

🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@erkamyaman
erkamyaman merged commit 11f116e into pangular-inspector:main Oct 11, 2026
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: package The ng-devtools package (packages/ng-devtools)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants