feat(core): add recursive key-based redaction#371
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Thank you for following the naming conventions! 🙏 |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
📝 WalkthroughWalkthroughAdds dot-notation path globs to RedactConfig.paths, centralizes glob compilation, and implements compiled path matchers with recursive path-based redaction helpers. Integrates matchers into redactEvent and auditDiff, updates the auditRedactPreset to use key-name globs, and updates docs and tests for the new path-glob semantics. ChangesPath-Glob Redaction
Sequence DiagramsequenceDiagram
participant Config as RedactConfig
participant Event as redactEvent
participant Compiler as compileRedactPathMatchers
participant Matchers as RedactPathMatchers
participant Redactor as redactPathsInTree
participant Output as RedactedEvent
Config->>Event: call redactEvent(event, config)
Event->>Compiler: compileRedactPathMatchers(config.paths)
Compiler->>Matchers: return compiled matchers
Event->>Redactor: redactPathsInTree(eventTree, Matchers, config.replacement)
Redactor->>Output: mutated/redacted event
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/evlog/src/types.ts (1)
119-126:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winKeep the
builtins: falsedocs aligned with the new key-based selectors.Line 121 still says disabling built-ins leaves only custom
paths/patterns, butkeysandkeyPatternsstill apply too. The public contract is correct in code; the JSDoc is now the part that's stale.📝 Proposed doc fix
- * - `false` → no built-ins, only custom `paths`/`patterns` + * - `false` → no built-ins, only custom `paths`/`keys`/`keyPatterns`/`patterns`🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/evlog/src/types.ts` around lines 119 - 126, Update the JSDoc for the builtins?: property to reflect the new key-based selectors: clarify that setting builtins: false disables only built-in patterns but custom paths/patterns as well as keys and keyPatterns still remain active; reference the builtins property and the related keys and keyPatterns options so readers know those selectors still apply when builtins is false (edit the comment block above builtins?: in types.ts).packages/evlog/src/audit.ts (2)
849-876:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMake the built-in header redaction case-insensitive.
With
keysonly containing lowercase names,Authorization,Cookie, andSet-Cookieare not redacted by the shared exact-match key matcher. That contradicts the preset docs and can leak credentials when callers log header objects with canonical casing. PreferkeyPatternslike/^authorization$/iover exact lowercase header keys here.Proposed change
export const auditRedactPreset: RedactConfig = { keys: [ 'password', 'passwordHash', 'token', 'apiKey', 'secret', 'accessToken', 'refreshToken', 'cardNumber', 'cvv', 'ssn', - 'authorization', - 'cookie', - 'set-cookie', ], + keyPatterns: [ + /^authorization$/i, + /^cookie$/i, + /^set-cookie$/i, + ], }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/evlog/src/audit.ts` around lines 849 - 876, The preset auditRedactPreset currently lists header names in the keys array using lowercase (e.g., 'authorization', 'cookie', 'set-cookie') which fails to match canonical-cased headers; update auditRedactPreset so header redaction is case-insensitive by removing those exact-header entries from keys and adding equivalent case-insensitive regexes in a new or existing keyPatterns array (e.g., /^authorization$/i, /^cookie$/i, /^set-cookie$/i); keep the other credential keys unchanged and ensure the redaction logic consumes keyPatterns alongside keys when matching (refer to auditRedactPreset and the keys/keyPatterns match usage).
327-358:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winRestore
redactPathscompatibility before routing through the key matcher.
buildKeyMatcher()only matches exact key names, so documented inputs likeredactPaths: ['user.password']no longer match/user/passwordhere. That silently stops redaction for existing callers and can leak secrets into the patch plusincludeBefore/includeAftersnapshots. Keep dotted-path matching forredactPaths, or introduce a separate key-based option and preserve the old contract on this path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/docs/skills/build-audit-logs/SKILL.md`:
- Line 285: The page mixes US and UK spellings for authorization/authorisation;
pick one spelling and standardize all occurrences across this skill page
(including keys used in auditRedactPreset and any references in RedactConfig,
e.g., the string "authorization"/"authorisation" and any mentions inside
audit.changes.before/after) so the same variant is used everywhere; update the
auditRedactPreset (or merged RedactConfig) entries and all textual references to
match the chosen variant.
In `@packages/evlog/src/redact.ts`:
- Around line 395-404: deserializeRegexList currently constructs RegExp directly
and will throw on invalid serialized entries; update deserializeRegexList to
validate each entry's shape (allow RegExp, string, or object with string source
and optional string flags), wrap the RegExp construction for each entry in a
try/catch so malformed source/flags are skipped (do not throw the whole
function), and preserve the existing type by returning only successful RegExp
instances (keeping the final filter p is RegExp); reference the
deserializeRegexList function and the patterns/keyPatterns deserialization path
when making this change.
- Around line 68-85: redactValueByKeys currently treats any non-array object as
a plain object and rebuilds it via Object.entries, which corrupts Dates, URLs,
Errors and class instances; change the object-handling branch in
redactValueByKeys so you only recurse into plain objects (e.g. detect plain
objects with Object.getPrototypeOf(value) === Object.prototype ||
Object.getPrototypeOf(value) === null or a similar isPlainObject check) and
leave non-plain objects untouched (return value) unless their key matches
matcher; keep the existing Array.isArray behavior and ensure matcher/replacement
logic still runs for object keys when the object is plain—this prevents
auditDiff from receiving corrupted before/after/patch values.
---
Outside diff comments:
In `@packages/evlog/src/audit.ts`:
- Around line 849-876: The preset auditRedactPreset currently lists header names
in the keys array using lowercase (e.g., 'authorization', 'cookie',
'set-cookie') which fails to match canonical-cased headers; update
auditRedactPreset so header redaction is case-insensitive by removing those
exact-header entries from keys and adding equivalent case-insensitive regexes in
a new or existing keyPatterns array (e.g., /^authorization$/i, /^cookie$/i,
/^set-cookie$/i); keep the other credential keys unchanged and ensure the
redaction logic consumes keyPatterns alongside keys when matching (refer to
auditRedactPreset and the keys/keyPatterns match usage).
In `@packages/evlog/src/types.ts`:
- Around line 119-126: Update the JSDoc for the builtins?: property to reflect
the new key-based selectors: clarify that setting builtins: false disables only
built-in patterns but custom paths/patterns as well as keys and keyPatterns
still remain active; reference the builtins property and the related keys and
keyPatterns options so readers know those selectors still apply when builtins is
false (edit the comment block above builtins?: in types.ts).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 0ede3606-c2c9-424b-8aa7-52c5d98453ea
📒 Files selected for processing (10)
.changeset/recursive-key-redaction.mdapps/docs/content/2.learn/6.redaction.mdapps/docs/content/4.use-cases/4.audit/05.compliance.mdapps/docs/content/6.reference/1.configuration.mdapps/docs/skills/build-audit-logs/SKILL.mdpackages/evlog/src/audit.tspackages/evlog/src/redact.tspackages/evlog/src/types.tspackages/evlog/test/core/audit.test.tspackages/evlog/test/core/redact.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
password→ any-depth match), key-name globs, and custom replacements.Documentation
Refactor
Tests