Skip to content

Mask inside logged errors instead of passing them through - #362

Merged
terehov merged 4 commits into
fullstack-build:developmentfrom
nkuba:mask-inside-errors
Sep 10, 2026
Merged

terehov merged 4 commits into
fullstack-build:developmentfrom
nkuba:mask-inside-errors

Conversation

@nkuba

@nkuba nkuba commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Masking does not reach inside errors. With mask: { regex: [/SECRET_[0-9]+/] }:

log.info("key=SECRET_123");             // key=[***]
log.error(new Error("key=SECRET_123")); // key=SECRET_123

The error message, properties assigned to the error and the whole cause chain reach the JSON line, the pretty output and every transport in plaintext. Reported in #361, and for assigned properties in #214.

This PR makes the masking engine treat an Error like any other argument: it is replaced by a masked clone. The clone is a real Error with the same prototype, so instanceof and the JSON error detection still work, and no subclass constructor runs. The original error is not modified.

What gets masked inside an error:

  • mask.regex: the message, and the <name>: <message> header line of a V8 stack. Stack frames are left alone, so a broad pattern cannot break file positions.
  • mask.keys and mask.paths: every other own property (code, extensions, ...) and the cause chain. keys skips name, message and stack, so keys: ["name"] does not blank every error.

One behavior change for transports: with mask configured, nativeError is now the masked clone, not the caller's instance. Without mask nothing changes. The changelog entry sits under 5.2.0 for that reason.

Fixes #361

The masking engine has returned any Error untouched since 4.4.0, when the walk stopped writing the
placeholder into the caller's objects (fullstack-build#180) by skipping errors altogether. That left the gap reported
in fullstack-build#361 and earlier in fullstack-build#214: a secret in `error.message`, in an own property assigned to the error, or
down the `cause` chain reached the JSON line, the pretty error block and every transport's
`nativeError` in plaintext, while the same secret in a string argument was redacted.

Errors are now replaced by a masked clone, like every other argument. The clone is a genuine Error
re-pointed at the source's prototype, so `instanceof`, the `[object Error]` tag and the JSON
renderer's IErrorObject detection keep working, and no subclass constructor runs (fullstack-build#227). A cross-realm
error (node:vm, an iframe) counts as a real Error by its tag. Every own property is defined fresh on
the clone from a guarded read, so read-only and getter-only properties cannot throw (fullstack-build#217, fullstack-build#234) and
the caller's instance is never written to.

Inside an error, `name`, `message` and `stack` are exempt from `mask.keys`, because `keys: ["name"]`
is ordinary PII configuration and must not blank every error. `mask.regex` still applies to their text
and `mask.paths` can censor them explicitly. The stack is not regex-masked as a whole, since a token
or digit pattern would corrupt frame positions. Only the `<name>: <message>` header of a V8-style
stack is masked, with the same regexes as the message, so a header that was formatted before the
message changed cannot keep the secret either. Firefox and Safari stacks have no header and stay
untouched. Every other own property, `cause` included, is masked exactly like a plain object's.

The full browser bundle grows by about 0.3KB gzip for the new branch, so its budget moves from 21.8KB
to 22.2KB.
The README's masking section described masking as leak-proof without saying that errors were skipped,
and the Sentry recipe promised the caller's native Error instance under `nativeError`. Both now
describe the masked clone: what `regex`, `keys` and `paths` reach inside an error, why `keys` skips
`name`, `message` and `stack`, and that stack frames are never regex-masked. llms.txt gets the same
clause in its `mask` bullet, and the CHANGELOG its 5.2.0 entries.
@terehov

terehov commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Thank you. I’ll have a look at it in a few days 🙏

@terehov
terehov changed the base branch from master to development September 10, 2026 19:24
@terehov

terehov commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Thanks for this, the fix is solid and well tested.

I added one commit on top: DOMException (e.g. AbortError from fetch/AbortController) serves name and message through prototype getters that read internal slots, so they threw on the prototype-swapped clone and the error was logged as Error: with an empty message. The clone now copies those two from the source when its getter can't answer them (Node + browser test). I also added the masked-clone note to RECIPES.md.

@terehov terehov closed this Sep 10, 2026
@terehov terehov reopened this Sep 10, 2026
@codecov

codecov Bot commented Sep 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.01493% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 99.83%. Comparing base (c857dd4) to head (f2b01ec).
⚠️ Report is 24 commits behind head on development.

Files with missing lines Patch % Lines
src/core/masking.ts 97.02% 2 Missing ⚠️
Additional details and impacted files
@@               Coverage Diff               @@
##           development     #362      +/-   ##
===============================================
- Coverage       100.00%   99.83%   -0.17%     
===============================================
  Files               54       53       -1     
  Lines             7256     4643    -2613     
  Branches          2228     1369     -859     
===============================================
- Hits              7256     4635    -2621     
- Misses               0        8       +8     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@terehov
terehov merged commit 58d386a into fullstack-build:development Sep 10, 2026
15 of 23 checks passed
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.

Bug: masking does not apply inside an error

2 participants