Uh oh!
There was an error while loading. Please reload this page.
forked from ianstormtaylor/superstruct
- Notifications
You must be signed in to change notification settings - Fork 1
fix: fix deep sensitive redaction#43
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Uh oh!
There was an error while loading. Please reload this page.
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
94370f0
fix: fix deep sensitive redaction
ccharly 5e9e9de
chore: comment
ccharly 63cd199
test: add missing test for arrays
ccharly c86b4f4
fix: fix map case
ccharly 7ae1968
fix: allow undefined map keys
ccharly 4e88633
Merge branch 'main' into cc/fix/fix-deep-sensitive
ccharly File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -121,19 +121,90 @@ export function withRedactedBranch( | ||
| // eslint-disable-next-line jsdoc/require-jsdoc | ||
| function* redactBranch(failures: Iterable<Failure>): Iterable<Failure> { | ||
| for (const failure of failures) { | ||
| yield { | ||
| ...failure, | ||
| // Replace the parent object in the branch with a sanitised copy. | ||
| // Reference equality (`===`) is intentional: we only want to touch | ||
| // this specific parent, not an unrelated object at a different depth | ||
| // that might happen to have the same shape. | ||
| branch: failure.branch.map((branchItem) => { | ||
| if (branchItem !== parentObj || !isObject(parentObj)) { | ||
| return branchItem; | ||
| if (!isObject(parentObj)) { | ||
| yield failure; | ||
| continue; | ||
| } | ||
| // Locate this specific parent. If it isn't in the branch (e.g. the | ||
| // failure came from a different scope), pass it through. | ||
| const parentIndex = failure.branch.indexOf(parentObj); | ||
| if (parentIndex === -1) { | ||
| yield failure; | ||
| continue; | ||
| } | ||
| // Sanitize the parent entry. | ||
| const branch = [...failure.branch]; | ||
| branch[parentIndex] = redactKeys(parentObj, sensitiveKeys); | ||
| // Walk backward through every ancestor in the branch. Each ancestor | ||
| // holds a direct reference to the child below it - without this step, | ||
| // outer objects would still expose the unredacted child through their | ||
| // own properties (e.g. root.wrapper.secret would remain visible | ||
| // even after wrapper was sanitised at branch[1]). | ||
| // | ||
| // For each level we use the original child reference to find which | ||
| // property key points to it (via ===), then replace that entry with | ||
| // the already-sanitised child from the patched branch. | ||
| for ( | ||
| let ancestorIndex = parentIndex - 1; | ||
| ancestorIndex >= 0; | ||
| ancestorIndex-- | ||
| ) { | ||
| const ancestor = branch[ancestorIndex]; | ||
| // No reference to update in this case. | ||
| if (!isObject(ancestor)) { | ||
| break; | ||
| } | ||
| // Keep the original child reference, so we can detect it. | ||
| const child = failure.branch[ancestorIndex + 1]; | ||
| // The already-sanitised child from the patched branch. | ||
| const childSanitized = branch[ancestorIndex + 1]; | ||
| // If we find the (original) child (not redacted) in one of the ancestor's | ||
| // properties/entries, we replace it with the redacted version (re-using the same | ||
| // sanitized child from the patched branch). | ||
| if (Array.isArray(ancestor)) { | ||
| const childIndex = ancestor.indexOf(child); | ||
| if (childIndex === -1) { | ||
| break; | ||
| } | ||
| return redactKeys(parentObj, sensitiveKeys); | ||
| }), | ||
| }; | ||
| const copy = [...ancestor]; | ||
| copy[childIndex] = childSanitized; | ||
| branch[ancestorIndex] = copy; | ||
| } else if (ancestor instanceof Map) { | ||
| // Use a flag instead of relying on `undefined` since that could be a valid key in a `Map`. | ||
| let childFound = false; | ||
| let childKey: unknown; | ||
| for (const [entryKey, entryValue] of ancestor) { | ||
| if (entryValue === child) { | ||
| childKey = entryKey; | ||
| childFound = true; | ||
| break; | ||
| } | ||
| } | ||
| if (!childFound) { | ||
| break; | ||
| } | ||
cursor[bot] marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| const copy = new Map(ancestor); | ||
| copy.set(childKey, childSanitized); | ||
| branch[ancestorIndex] = copy; | ||
| } else { | ||
| const childKey = Object.keys(ancestor).find( | ||
ccharly marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| (key) => ancestor[key] === child, | ||
| ); | ||
| if (childKey === undefined) { | ||
| break; | ||
| } | ||
| branch[ancestorIndex] = { ...ancestor, [childKey]: childSanitized }; | ||
| } | ||
| } | ||
| yield { ...failure, branch }; | ||
| } | ||
| } | ||
38 changes: 38 additions & 0 deletions
38 test/validation/sensitive/invalid-sibling-ancestor-array.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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| import { array, literal, object, sensitive, string } from '../../../src'; | ||
| // When a sensitive-entries object lives inside an array field, the array | ||
| // itself appears as an ancestor in the branch. The backward walk must handle | ||
| // this by copying the array and replacing the sanitised element, rather than | ||
| // treating it as a plain object with property keys. | ||
| const SecretStruct = object({ | ||
| secret: sensitive(string()), | ||
| encoding: literal('hex'), | ||
| }); | ||
| export const Struct = object({ | ||
| items: array(SecretStruct), | ||
| tag: literal('ok'), | ||
| }); | ||
| export const data = { | ||
| items: [{ secret: 'super-secret', encoding: 'invalid-encoding' }], | ||
| tag: 'ok', | ||
| }; | ||
| export const failures = [ | ||
| { | ||
| value: 'invalid-encoding', | ||
| type: 'literal', | ||
| refinement: undefined, | ||
| path: ['items', 0, 'encoding'], | ||
| branch: [ | ||
| { | ||
| items: [{ secret: '***', encoding: 'invalid-encoding' }], | ||
| tag: 'ok', | ||
| }, | ||
| [{ secret: '***', encoding: 'invalid-encoding' }], | ||
| { secret: '***', encoding: 'invalid-encoding' }, | ||
| 'invalid-encoding', | ||
| ], | ||
| }, | ||
| ]; |
51 changes: 51 additions & 0 deletions
51 test/validation/sensitive/invalid-sibling-ancestor-deep.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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| import { literal, object, sensitive, string } from '../../../src'; | ||
| // Three levels of nesting: RootStruct > WrapperStruct > AccountStruct. | ||
| // A sibling failure inside AccountStruct must redact sensitive data from | ||
| // ALL branch entries — including both intermediate and root ancestors. | ||
| const SecretStruct = object({ | ||
| secret: sensitive(string()), | ||
| encoding: literal('hex'), | ||
| }); | ||
| const WrapperStruct = object({ | ||
| account: SecretStruct, | ||
| tag: literal('ok'), | ||
| }); | ||
| export const Struct = object({ | ||
| wrapper: WrapperStruct, | ||
| id: literal('root'), | ||
| }); | ||
| export const data = { | ||
| wrapper: { | ||
| account: { secret: 'super-secret', encoding: 'invalid-encoding' }, | ||
| tag: 'ok', | ||
| }, | ||
| id: 'root', | ||
| }; | ||
| export const failures = [ | ||
| { | ||
| value: 'invalid-encoding', | ||
| type: 'literal', | ||
| refinement: undefined, | ||
| path: ['wrapper', 'account', 'encoding'], | ||
| branch: [ | ||
| { | ||
| wrapper: { | ||
| account: { secret: '***', encoding: 'invalid-encoding' }, | ||
| tag: 'ok', | ||
| }, | ||
| id: 'root', | ||
| }, | ||
| { | ||
| account: { secret: '***', encoding: 'invalid-encoding' }, | ||
| tag: 'ok', | ||
| }, | ||
| { secret: '***', encoding: 'invalid-encoding' }, | ||
| 'invalid-encoding', | ||
| ], | ||
| }, | ||
| ]; |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| import { literal, map, object, sensitive, string } from '../../../src'; | ||
| // When a sensitive-entries object lives inside a Map, the Map itself appears | ||
| // as an ancestor in the branch. Object.keys() returns [] for Maps, so the | ||
| // backward walk must handle this case by iterating the Map's entries directly. | ||
| const SecretStruct = object({ | ||
| secret: sensitive(string()), | ||
| tag: literal('ok'), | ||
| }); | ||
| export const Struct = map(string(), SecretStruct); | ||
| export const data = new Map([ | ||
| ['key', { secret: 'super-secret', tag: 'INVALID' }], | ||
| ]); | ||
| export const failures = [ | ||
| { | ||
| value: 'INVALID', | ||
| type: 'literal', | ||
| refinement: undefined, | ||
| path: ['key', 'tag'], | ||
| branch: [ | ||
| new Map([['key', { secret: '***', tag: 'INVALID' }]]), | ||
| { secret: '***', tag: 'INVALID' }, | ||
| 'INVALID', | ||
| ], | ||
| }, | ||
| ]; |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| import { literal, object, sensitive, string } from '../../../src'; | ||
| // When a sensitive() field lives inside a nested object(), a sibling-field | ||
| // failure must redact sensitive data from ALL branch entries — including | ||
| // ancestor objects that hold a reference to the inner (parent) object. | ||
| // Without the fix, branch[0] (the outer wrapper) still exposes secret | ||
| // via its `account` property. | ||
| const SecretStruct = object({ | ||
| secret: sensitive(string()), | ||
| encoding: literal('hex'), | ||
| }); | ||
| export const Struct = object({ | ||
| account: SecretStruct, | ||
| tag: literal('ok'), | ||
| }); | ||
| export const data = { | ||
| account: { secret: 'super-secret', encoding: 'invalid-encoding' }, | ||
| tag: 'ok', | ||
| }; | ||
| export const failures = [ | ||
| { | ||
| value: 'invalid-encoding', | ||
| type: 'literal', | ||
| refinement: undefined, | ||
| path: ['account', 'encoding'], | ||
| branch: [ | ||
| { | ||
| account: { secret: '***', encoding: 'invalid-encoding' }, | ||
| tag: 'ok', | ||
| }, | ||
| { secret: '***', encoding: 'invalid-encoding' }, | ||
| 'invalid-encoding', | ||
| ], | ||
| }, | ||
| ]; |
Oops, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
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.
Uh oh!
There was an error while loading. Please reload this page.