fix(sdk): allow duplicate KAOs on same KAS within a split (DSPX-3379)#937
fix(sdk): allow duplicate KAOs on same KAS within a split (DSPX-3379)#937dmihalcik-virtru wants to merge 4 commits into
Conversation
splitfix
splitLookupTableFactory threw when a TDF's KAO array had two entries with the same sid and KAS url, which is legitimate (e.g. multiple keys on one KAS for the same split). Treat each duplicate as an independent rewrap candidate so decrypt succeeds if any of them succeed, or surfaces a normal permission failure otherwise. DSPX-3379 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the Web SDK to support multiple Key Access Objects (KAOs) on the same Key Access Service (KAS) and split ID, preventing decryption failures when duplicate URLs or multiple keys are present. The splitLookupTableFactory now returns arrays of KAOs, and unwrapKey handles these lists during rewrap. Feedback highlights a potential bug where mismatched sid normalization between splitIds and accessibleSplits could bypass safety checks, and suggests de-duplicating identical KAOs to avoid redundant network requests. Additionally, the newly added specification file contains empty placeholder sections that should be completed or removed.
| ## Problem / Motivation | ||
| _Why does this work need to happen? What is the user/business pain?_ | ||
|
|
||
| ## Proposed Solution | ||
| _What will you build, at a functional level? Sketch the approach._ | ||
|
|
||
| ## Inputs / Outputs / Contracts | ||
| _Function signatures, data shapes, API contracts, CLI flags._ | ||
|
|
||
| ## Edge Cases & Constraints | ||
| _Boundary conditions, error states, performance limits, security considerations._ | ||
|
|
||
| ## Out of Scope | ||
| _What this work item explicitly does not cover._ | ||
|
|
||
| ## Acceptance Criteria | ||
| - [ ] _Clear, testable condition_ | ||
| - [ ] _…_ |
There was a problem hiding this comment.
The specification file contains several empty sections with placeholder template text (e.g., Problem / Motivation, Proposed Solution, Inputs / Outputs / Contracts, Edge Cases & Constraints, Out of Scope, and Acceptance Criteria). Please fill out these sections to provide the necessary context and design details for this change, or remove them if they are not applicable.
Address PR #937 review feedback: - Normalize `sid` to '' in `accessibleSplits` so mixed undefined/'' sids cannot bypass the disallowed-KAS check. - Deduplicate fully identical KAOs by (kid, wrappedKey) in `splitLookupTableFactory` to avoid redundant rewrap requests. - Fill out spec/DSPX-3379.md with the implemented design. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
If these changes look good, signoff on them with: If they aren't any good, please remove them with: |
Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
dmihalcik-virtru
force-pushed
the
DSPX-3379-splitfix
branch
from
0e458bb to
62dbef3
Compare
|
1 participant
Summary
splitLookupTableFactoryno longer throws when a TDF's KAO array contains multiple entries with the samesidand KASurl. The inner value type is nowKeyAccessObject[], and duplicates are appended.unwrapKeyregisters one rewrap candidate per KAO in the list (disambiguating duplicate URLs with a#<idx>suffix), soanyPooltries each in turn — succeeding if any succeed, otherwise surfacing a normal permission failure.Fixes DSPX-3379.
Test plan
make test— lib mocha + WTR suites pass (334 passing), including the newsplitLookupTableFactorycases. (web-app failure in local run was a missing Playwright Chromium binary, unrelated to this change.)(sid, url)pair end-to-end and confirm successful decrypt instead of the priorInvalidFileError("Repetition found …").🤖 Generated with Claude Code