Skip to content

feat: add capture sanitization controls - #468

Merged
sam-cook merged 4 commits into
mainfrom
codex/kon-3129-capture-sanitization
Sep 18, 2026
Merged

sam-cook merged 4 commits into
mainfrom
codex/kon-3129-capture-sanitization

Conversation

@sam-cook

@sam-cook sam-cook commented Sep 17, 2026 •

Copy link
Copy Markdown
Collaborator

Hi There, I just made a Pull Request!

Adds opt-in sanitization controls for captured network URLs and console output while preserving the existing raw-capture behavior.

  • sanitize: true strips query strings and fragments.
  • A sanitizer function supports customer-defined redaction.
  • Console sanitization lives inside the rrweb console plugin wrapper.
  • Sanitizer failures fail closed and produce one loud warning when CSR_DEBUG is enabled.
  • Network requests and application console calls continue unchanged.

✔️ Checklist

  • All tests are passing
  • Relevant documentation updated
  • linter/style run on changed files
  • Tests added for new functionality
  • Regression tests added for bug fixes
  • Tested in a corresponding example app

Validation

  • yarn prettier --check .
  • yarn run -T eslint --no-eslintrc --config .eslintrc 'csr/*/src/**/*.{tsx,ts}'
  • yarn test:csr — 17 files, 244 tests passed
  • yarn build:csr
  • git diff --check

Generated with Codex.

Co-authored-by: Codex <noreply@openai.com>
sam-cook and others added 3 commits September 18, 2026 10:53
`Recorder.stop()` cleared the network sanitizer, but `restoreNetwork()`
cannot cancel in-flight requests — their patched handlers still emit
afterwards. Those emits took the no-sanitizer branch and shipped raw
URLs, and `uploader.close()` does not reject later frames, so they
uploaded. Stop often coincides with unload, which is exactly when
requests are in flight. The fields are reassigned on every `start()`,
so the clearing had no other purpose. Adds a regression test.

Simplifications, no behavior change:

- Collapse `sanitizeCapturedValue`, `sanitizeCapturedValues`,
  `createOnceCaptureLogger`, `logSanitizerFailure` and
  `applyBuiltInSanitizer` into one `createValueSanitizer` factory.
  Call sites no longer thread `(value, sanitizer, source, logger)`,
  and the duplicated try/catch around the diagnostic logger is gone.
- Drop `emitNetworkRequestEvent` and one `Recorder` field.
- Drop `isConsoleLogData`/`isConsoleLogLevel`/`isStringArray`. rrweb's
  `LogData` is always `{ level, payload: string[], trace: string[] }`,
  and the only levels it emits are the ones we pass plus error/warn,
  so the guard could never reject real data. Malformed fields still
  fail closed.
- Stop routing debug messages by sniffing a `[CSR] SECURITY:` prefix.
  The message text lived in csr-recorder and the match lived in
  session-recording with no shared constant, so a reworded message
  would have silently degraded the routing.
- Fix the README paragraph order — a sentence ended in `:` followed by
  an unrelated paragraph — and stop duplicating the detail in both.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Replaces the one `createValueSanitizer` factory with four small pieces,
each with one job and each testable on its own:

- `stripUrlsInText` — the console built-in policy
- `getValueSanitizer` — policy selection plus string-return enforcement
- `createOnceLogger` — log at most once, never throw at the host app
- `dropOnFailure` — fail closed while building one captured event

The real reduction is in how failure travels. `ValueSanitizer` now
throws instead of returning `string | undefined`, so callers no longer
propagate `undefined` by hand. That deletes `sanitizeField` and its
`.every` type-predicate narrowing, and collapses the console observer's
two undefined checks into one.

Malformed plugin data now fails closed through the same path, because
mapping over a non-array throws. That restores the SECURITY warning the
previous commit had dropped, with no dedicated branch for it.

Measured with eslint's `complexity` rule, complexity added over main:

  as submitted  53
  first pass    31
  this commit   28

  capture-sanitizer.ts        peak 5 -> 3
  console-capture-plugin.ts   27 -> 10 total, 8 functions -> 4

`recorder.ts` goes up 2, the cost of the extra closure on its single
emit site.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The existing failure tests use a sanitizer that throws for every value,
so they proved per-event retry but never that a later good event still
gets captured. These cover the selective case on both channels: the
event whose value the sanitizer cannot handle is skipped, and the next
one is captured normally. Recording and the host application continue
either way.

Both passed on the first run — this documents existing behavior rather
than changing it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@sam-cook

sam-cook commented Sep 18, 2026 •

Copy link
Copy Markdown
Collaborator Author

Local browser integration test completed:

  • Existing raw console and network capture remains unchanged.
  • Built-in sanitization strips query strings and fragments from console text, fetch metadata, and XHR metadata.
  • Custom sanitizers redact console and network values.
  • A failed sanitizer drops only the affected event; later events are still captured.
  • Failure warnings are emitted once per channel.
  • Fetch and XHR receive the original unsanitized request URL.
  • An in-flight request that completes after stop() still emits sanitized metadata.
  • The in-flight application request itself remains unchanged.

@sam-cook
sam-cook merged commit 971a9d9 into main Sep 18, 2026
4 checks passed
@sam-cook
sam-cook deleted the codex/kon-3129-capture-sanitization branch September 18, 2026 10:57
@github-actions github-actions Bot mentioned this pull request Sep 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants