fix(sdk): allow the same KAS to wrap the same split (DSPX-3379)#967
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough
ChangesDuplicate KAO support
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Encryptor
participant Manifest
participant unwrapKey
participant KAS
Encryptor->>Manifest: Store repeated KAOs for one split
Manifest->>unwrapKey: Provide keyAccess entries
unwrapKey->>unwrapKey: Build and iterate KAO alternatives
loop Each alternative
unwrapKey->>KAS: Send rewrap request
KAS-->>unwrapKey: Return rewrapped key
end
unwrapKey-->>Encryptor: Return decrypted plaintext
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![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 resolves DSPX-3379 by allowing the same Key Access Server (KAS) to wrap the same split multiple times. It refactors splitLookupTableFactory to return a list of alternative Key Access Objects (KAOs) instead of throwing on duplicates, and updates unwrapKey to handle these duplicates as distinct alternatives. Feedback on the changes highlights a bug in splitLookupTableFactory where accessibleSplits does not normalize undefined sid values to empty strings, which can bypass safety checks. Additionally, a formatting improvement for a stack trace in the markdown specification file was suggested.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
0cc6e33 to
d370256
Compare
dmihalcik-virtru
changed the title
d370256 to
8fc59c3
Compare
X-Test Failure Report |
|
Addressed the automated review feedback:
|
8fc59c3 to
5636af8
Compare
cdc4346 to
a6c8401
Compare
X-Test Failure Report |
splitLookupTableFactory now returns split id -> KeyAccessObject[] (a disjunction of alternatives) instead of a url-keyed map, and no longer throws when a KAS repeats within a split. unwrapKey feeds each alternative to anyPool with a unique key so duplicate KAS entries are each tried until one succeeds. Matches go-sdk/java-sdk behavior; validated by xtest test_tdf_with_duplicate_kao_same_kas (opentdf/tests#555). Adds unit tests for duplicate + same-KAS-different-KID, an in-process decrypt-path test, and fills spec/DSPX-3379.md. Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
a6c8401 to
c8f7a70
Compare
|
2 participants
What
When a TDF's key-access-object (KAO) array has two entries with the same split
id and the same KAS URI, the web-sdk refused to decrypt, throwing
Unable to decrypt: Multiple keys detected for Key Access Server [...].This treats the KAOs for a split as a disjunction of alternatives (any one
succeeding unwraps the split) instead of a URL-keyed map that can hold at most
one KAO per KAS — matching how the split is already consumed downstream
(
anyPool) and how go-sdk / java-sdk already behave.Changes (
lib/tdf3/src/tdf.ts)splitLookupTableFactorynow returnsRecord<splitId, KeyAccessObject[]>andno longer throws on a repeated
(splitId, kasUrl); every allowed KAO is keptas an alternative. Disallowed-KAS accounting (
UnsafeUrlError) is unchanged.unwrapKeyfeeds each alternative toanyPoolwith a uniqueurl#kid#indexkey so duplicate KAS entries are each tried until one succeeds.
Why (DSPX-3379)
The same KAS wrapping the same split is legitimate — an authoring mistake (same
key used twice) or intentional multiple keys on one KAS. go/java read these
files fine; the web-sdk (e.g. secure viewer) failed. Expected: decrypt (or
permission-denied) exactly as if the copies lived on distinct URIs.
Tests
splitLookupTableFactorykeeps duplicate(sid, kas)andsame-KAS-different-KID entries as alternatives.
splitPlanwith a repeatedkas+sidproduces aduplicate-KAO TDF that decrypts via the full
unwrap/anyPoolpath.test_tdf_with_duplicate_kao_same_kas(chore(xtest): cross-SDK test for duplicate KAO on same KAS+split (DSPX-3379) tests#555) — CI isgreen for go, java, and this branch's js.
Spec:
spec/DSPX-3379.md.Depends on / validated by: opentdf/tests#555
Summary by CodeRabbit
Bug Fixes
Tests