refactor: consolidate crypto codecs, zeroization hygiene, and IPNS terminology#609
Merged
FSM1 merged 57 commits intoJul 11, 2026
Merged
Conversation
Entire-Checkpoint: a0afed81a2f9
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Z3zThQZPm6bVk2476WURL Entire-Checkpoint: bca41a8e1287
- Round-trip cases: empty, 1-byte, 40000-byte (crosses chunk boundary), fixed pattern - Known-vector parity oracle: (bytes, base64String) pair - Output type assertions for bytesToBase64/base64ToBytes - RED confirmed: helpers not yet defined in @cipherbox/crypto Entire-Checkpoint: dca3bce0f15c
- bytesToBase64/base64ToBytes added to utils/encoding.ts, copied verbatim from packages/core/src/node/encode.ts chunked-btoa implementation - Re-exported from utils/index.ts and top-level index.ts alongside hex helpers - Golden-vector suite green, build and typecheck pass Entire-Checkpoint: 26f069806a90
Entire-Checkpoint: 7aee40b07d1c
Entire-Checkpoint: 207e095f1322
Entire-Checkpoint: 9177001a7544
- Add caller-key-unchanged aliasing assertion (D-09) - Add round-trip parity assertion via importAesKey + decryptAesGcm Entire-Checkpoint: f77eeb523158
- Shared internal helper owns a local copy of the caller key and zeroes it in a finally block after crypto.subtle.importKey consumes the bytes - Caller-supplied key is never mutated (D-09: caller is terminal owner) - Exported from the aes barrel for test access only, not top-level Entire-Checkpoint: e43fe751bda7
- encryptAesGcm, encryptAesGcmAad, decryptAesGcm, decryptAesGcmAad,
encryptAesCtr, decryptAesCtr, decryptAesCtrRange now import their key
via the shared importAesKey helper instead of an inline unzeroed copy
- Widen importAesKey algorithm param to AlgorithmIdentifier so it
accepts the plain { name } algorithm object used by GCM and CTR
- seal.ts untouched (no independent key copy); no other logic changed
Entire-Checkpoint: 7da812e34303
- Add plan summary documenting importAesKey helper + 7-function routing Entire-Checkpoint: 2f7bb032b048
Entire-Checkpoint: a954e557b22c
…encryptedIpnsPrivateKey - Renamed RepublishEntry field in apps/api/src/tee/tee.service.ts - Renamed the teeEntries build key in apps/api/src/republish/republish.service.ts (value unchanged) - Renamed request-body field and decode call in apps/tee-worker/src/routes/republish.ts - Renamed decryptIpnsKey and decryptWithFallback params plus JSDoc in apps/tee-worker/src/services/key-manager.ts Entire-Checkpoint: e1b35c88096c
…vateKey field - Renamed fixtures, makeEntry() helper, and assertions in all three spec files - Updated 4 negative not.toHaveProperty assertions in republish.service.spec.ts to the canonical name - API + tee-worker suites verified green (72 + 76 passing) Entire-Checkpoint: db29e88fb50d
…ation Entire-Checkpoint: fe55ecbcfa89
Entire-Checkpoint: 7a45d8c485dc
Entire-Checkpoint: b358b0524cd7
Consolidates the duplicated Phase 71 root-ownership authorization gate from shares.service.ts createShare and share-invite.service.ts createInvite into a single exported assertRootOwnership function in apps/api/src/shares/root-ownership.util.ts. Same repository query, identical ForbiddenException message — no behavior change. Entire-Checkpoint: 8517a8fda79e
Entire-Checkpoint: acb9a8b0beee
Round-trip test for the bytes-in/bytes-out wrapIpnsKeyForTee signature (todo 2026-07-10-wrapipnskeyfortee-bytes-in-bytes-out). RED against the current hex-string signature. Entire-Checkpoint: 479ba6a9fa3c
Change wrapIpnsKeyForTee to accept and return raw Uint8Array, renaming the TEE public key param to the canonical teePublicKey. Removes the internal hex encode/decode so the helper matches the bytes-internal convention used everywhere else in sdk-core; hex now lives only at the 3 call sites. Implements todo 2026-07-10-wrapipnskeyfortee-bytes-in-bytes-out. Entire-Checkpoint: 36bb13f9b5c1
Move hex encode/decode of the TEE public key and wrapped result out of wrapIpnsKeyForTee and into folder/registration.ts, vault/index.ts, and file/index.ts — the call sites now hex-decode teeKeys.currentPublicKey before calling and hex-encode the returned bytes into encryptedIpnsPrivateKey. Also trims a stray hexToBytes mention from wrap.ts's JSDoc so the helper reads as fully bytes-only. sdk-core typechecks and the full unit suite passes with no behavior change. Entire-Checkpoint: 75c8f6688692
Entire-Checkpoint: 17b42456b072
Entire-Checkpoint: 5cf777e01b32
The v2.0 grant model superseded the per-recipient share re-wrap fan-out (shareCallbacks/ShareCallbacks) and the addShareKeysFn threaded through SharedWriteContext/SharedFolderState — every production construction site already passed a no-op. Removed the dead type, field, and every construction site across the SDK and web hooks, plus the now-meaningless .not.toHaveBeenCalled() test assertions. Rebuilt packages/sdk dist before running apps/web typecheck to avoid cross-package dist staleness. Entire-Checkpoint: 3c788b72c382
…onDto updateSharePermission/updatePermissionFn had no live caller in apps/web or client.ts (only its own test), and UpdatePermissionDto/ UpdatePermissionDtoPermission are generated api-client models with no backing route in the current openapi.json. Removed both, plus the deprecated ReceivedShare.encryptedIpnsKey field it was paired with (unread anywhere in the web app). No pnpm api:generate run — these are orphaned artifacts with no source route, so regeneration would not recreate them. Entire-Checkpoint: 056f4c090f43
Reword the ReceivedShare doc comment to stop naming the now-deleted updateSharePermission wrapper literally, keeping the plan's whole-tree drift grep clean. Entire-Checkpoint: cbae022041af
Wraps main()'s body from after the vaultKeyBlob load onward in try/finally and clears userPrivateKey, vaultKeyBlob.rootReadKey, vaultKeyBlob.rootWriteKey, and the derived fileReadKey/subReadKey (when the --folder-name branch runs) via clearBytes in the finally block — mirrors the existing edit-filepointer.mts/rename-folder.mts pattern. verify-filepointer.mts was the one script in this family that never cleared its key material. Entire-Checkpoint: 5584dff73442
Entire-Checkpoint: 4dde6fd97965
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Z3zThQZPm6bVk2476WURL Entire-Checkpoint: 56f9c434304b
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Z3zThQZPm6bVk2476WURL Entire-Checkpoint: 8d730bd7279c
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Z3zThQZPm6bVk2476WURL Entire-Checkpoint: 7be0d29834e0
Entire-Checkpoint: cf8dda6f85c2
Entire-Checkpoint: f5b244a243cc
Entire-Checkpoint: c03bcf72c821
Entire-Checkpoint: 312291dbf346
…ify-filepointer Entire-Checkpoint: f04f9add91a2
…heduling caveat Entire-Checkpoint: a34aa33b85e0
WalkthroughPhase 77 completes crypto hygiene and terminology canonicalization. It centralizes Base64 and AES key handling, renames encrypted IPNS key fields, removes obsolete SDK share plumbing, extracts root-ownership validation, and adds error-path key zeroization. ChangesCrypto hygiene and codec consolidation
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed: dependency version conflict. Check your lock file or package.json. 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 |
github-actions
Bot
added
release:crypto:feat
Contributor
Release Preview
Cascade Details
|
github-actions
Bot
added
the
release:cipherbox-sdk:fix
Greptile SummaryThis PR consolidates crypto helpers and standardizes IPNS private-key naming across the monorepo. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (2): Last reviewed commit: "docs(phase-77): extract phase learnings" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #609 +/- ##
===========================================
+ Coverage 70.16% 84.62% +14.45%
===========================================
Files 157 123 -34
Lines 15426 11128 -4298
Branches 1952 1950 -2
===========================================
- Hits 10824 9417 -1407
+ Misses 4348 1457 -2891
Partials 254 254
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Entire-Checkpoint: d9e2d7aa46c2
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/sdk/src/client.ts (1)
5134-5135: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate the stale shared-context documentation.
The
buildSharedWriteContextFromStatecomment still saysaddShareKeysis carried into the context, but this call no longer passes it andSharedFolderStateno longer defines it. Remove that reference so the documented contract matches the implementation.🤖 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/sdk/src/client.ts` around lines 5134 - 5135, Update the documentation comment for buildSharedWriteContextFromState to remove the stale reference to addShareKeys, ensuring it matches the current call and SharedFolderState contract.
🧹 Nitpick comments (2)
packages/sdk-core/src/__tests__/rotation/write-revocation.test.ts (1)
84-97: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider moving
unwrapKey/reWrapKeyintomockFnsfor consistency.Unlike
grant.test.tsandnavigate.test.tswhere all mocked functions are sourced frommockFns,unwrapKeyandreWrapKeyhere are inlinevi.fn()(lines 94-95). If any test case later needs to assert on their call arguments, the mock references won't be accessible. Moving them intomockFnsaligns with the established pattern and keeps the door open for assertions.♻️ Suggested refactor
const mockFns = vi.hoisted(() => ({ // ...existing mocks... wrapKey: vi.fn(), + unwrapKey: vi.fn(), + reWrapKey: vi.fn(), }));And in the mock return:
wrapKey: mockFns.wrapKey, - unwrapKey: vi.fn(), - reWrapKey: vi.fn(), + unwrapKey: mockFns.unwrapKey, + reWrapKey: mockFns.reWrapKey,🤖 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/sdk-core/src/__tests__/rotation/write-revocation.test.ts` around lines 84 - 97, Move the unwrapKey and reWrapKey mock references into the existing mockFns collection, then use those shared references in the `@cipherbox/crypto` mock alongside the other mocked functions. Preserve their current vi.fn() behavior while making the mocks accessible for call-argument assertions.packages/crypto/src/__tests__/encoding.test.ts (1)
15-62: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd exact-boundary and invalid-input cases.
The current coverage is good, but add tests for 32,768/32,769-byte inputs and malformed Base64 so the chunking boundary and decoder failure behavior remain explicit.
🤖 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/crypto/src/__tests__/encoding.test.ts` around lines 15 - 62, Add two new test cases in the 'Base64 Codec — Round-Trip' describe block to test the exact chunking boundary: one for a 32,768-byte array and one for a 32,769-byte array, both using the bytesToBase64 and base64ToBytes round-trip pattern similar to the existing 40000-byte test. Additionally, add a new describe block or test cases to validate that base64ToBytes properly handles malformed or invalid Base64 input strings and fails gracefully with appropriate error behavior.Source: Path instructions
🤖 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 @.planning/STATE.md:
- Around line 5-16: Update or remove the stale legacy footer in STATE.md so it
no longer reports the June 27–28, 2026 handoff; keep the July 11, 2026 metadata
near current_phase as the single authoritative session handoff.
In `@packages/sdk-core/src/vault/index.ts`:
- Around line 149-154: Update the wrapping flow around wrapIpnsKeyForTee in the
current method to zeroize the locally owned wrappedBytes buffer in a finally
block after bytesToHex completes, while preserving encryptedIpnsPrivateKey and
never clearing the caller-owned params.rootIpnsKeypair.privateKey.
---
Outside diff comments:
In `@packages/sdk/src/client.ts`:
- Around line 5134-5135: Update the documentation comment for
buildSharedWriteContextFromState to remove the stale reference to addShareKeys,
ensuring it matches the current call and SharedFolderState contract.
---
Nitpick comments:
In `@packages/crypto/src/__tests__/encoding.test.ts`:
- Around line 15-62: Add two new test cases in the 'Base64 Codec — Round-Trip'
describe block to test the exact chunking boundary: one for a 32,768-byte array
and one for a 32,769-byte array, both using the bytesToBase64 and base64ToBytes
round-trip pattern similar to the existing 40000-byte test. Additionally, add a
new describe block or test cases to validate that base64ToBytes properly handles
malformed or invalid Base64 input strings and fails gracefully with appropriate
error behavior.
In `@packages/sdk-core/src/__tests__/rotation/write-revocation.test.ts`:
- Around line 84-97: Move the unwrapKey and reWrapKey mock references into the
existing mockFns collection, then use those shared references in the
`@cipherbox/crypto` mock alongside the other mocked functions. Preserve their
current vi.fn() behavior while making the mocks accessible for call-argument
assertions.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 66fa8ca0-eab2-49ab-8363-a8556edc84b3
⛔ Files ignored due to path filters (3)
packages/api-client/src/models/index.tsis excluded by!packages/api-client/**packages/api-client/src/models/updatePermissionDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/updatePermissionDtoPermission.tsis excluded by!packages/api-client/**
📒 Files selected for processing (108)
.planning/PROJECT.md.planning/ROADMAP.md.planning/STATE.md.planning/phases/77-crypto-hygiene-and-terminology-canonicalization/77-01-PLAN.md.planning/phases/77-crypto-hygiene-and-terminology-canonicalization/77-01-SUMMARY.md.planning/phases/77-crypto-hygiene-and-terminology-canonicalization/77-02-PLAN.md.planning/phases/77-crypto-hygiene-and-terminology-canonicalization/77-02-SUMMARY.md.planning/phases/77-crypto-hygiene-and-terminology-canonicalization/77-03-PLAN.md.planning/phases/77-crypto-hygiene-and-terminology-canonicalization/77-03-SUMMARY.md.planning/phases/77-crypto-hygiene-and-terminology-canonicalization/77-04-PLAN.md.planning/phases/77-crypto-hygiene-and-terminology-canonicalization/77-04-SUMMARY.md.planning/phases/77-crypto-hygiene-and-terminology-canonicalization/77-05-PLAN.md.planning/phases/77-crypto-hygiene-and-terminology-canonicalization/77-05-SUMMARY.md.planning/phases/77-crypto-hygiene-and-terminology-canonicalization/77-06-PLAN.md.planning/phases/77-crypto-hygiene-and-terminology-canonicalization/77-06-SUMMARY.md.planning/phases/77-crypto-hygiene-and-terminology-canonicalization/77-07-PLAN.md.planning/phases/77-crypto-hygiene-and-terminology-canonicalization/77-07-SUMMARY.md.planning/phases/77-crypto-hygiene-and-terminology-canonicalization/77-08-PLAN.md.planning/phases/77-crypto-hygiene-and-terminology-canonicalization/77-08-SUMMARY.md.planning/phases/77-crypto-hygiene-and-terminology-canonicalization/77-09-PLAN.md.planning/phases/77-crypto-hygiene-and-terminology-canonicalization/77-09-SUMMARY.md.planning/phases/77-crypto-hygiene-and-terminology-canonicalization/77-10-PLAN.md.planning/phases/77-crypto-hygiene-and-terminology-canonicalization/77-10-SUMMARY.md.planning/phases/77-crypto-hygiene-and-terminology-canonicalization/77-PATTERNS.md.planning/phases/77-crypto-hygiene-and-terminology-canonicalization/77-RESEARCH.md.planning/phases/77-crypto-hygiene-and-terminology-canonicalization/77-SECURITY.md.planning/phases/77-crypto-hygiene-and-terminology-canonicalization/77-VALIDATION.md.planning/phases/77-crypto-hygiene-and-terminology-canonicalization/77-VERIFICATION.md.planning/todos/completed/2026-06-20-e2e-helper-scripts-zeroize-userprivatekey.md.planning/todos/completed/2026-06-28-zeroize-local-key-plaintext-copies-in-aes-helpers.md.planning/todos/completed/2026-06-29-dedup-base64-helpers-sdk-core-share.md.planning/todos/completed/2026-06-29-node-codec-base64-helper-dedup.md.planning/todos/completed/2026-07-01-rename-encrypted-ipns-key-canonical-field.md.planning/todos/completed/2026-07-02-retire-dead-sdk-share-scaffolding.md.planning/todos/completed/2026-07-03-drop-discarded-per-upload-ecies-wrapkey.md.planning/todos/completed/2026-07-03-hoist-base64tobytes-into-crypto-package.md.planning/todos/completed/2026-07-04-rename-ipnsprivatekeyencrypted-to-encryptedipnsprivatekey.md.planning/todos/completed/2026-07-10-extract-assert-root-ownership-helper.md.planning/todos/completed/2026-07-10-wrapipnskeyfortee-bytes-in-bytes-out.md.planning/todos/completed/2026-07-10-zeroize-createsubfolder-keys-on-error-path.mdapps/api/src/republish/republish.service.spec.tsapps/api/src/republish/republish.service.tsapps/api/src/shares/root-ownership.util.tsapps/api/src/shares/share-invite.service.tsapps/api/src/shares/shares.service.tsapps/api/src/tee/tee.service.spec.tsapps/api/src/tee/tee.service.tsapps/tee-worker/src/__tests__/republish.test.tsapps/tee-worker/src/routes/republish.tsapps/tee-worker/src/services/key-manager.tsapps/web/src/hooks/__tests__/useSharedWriteOps.test.tsapps/web/src/hooks/shared-folder-projection.tsapps/web/src/hooks/useAuth.tsapps/web/src/hooks/useFolderMutations.tsapps/web/src/hooks/useSharedNavigation.tsapps/web/src/hooks/useSharedNavigationActions.tsapps/web/src/stores/share.store.tspackages/core/src/node/decode.tspackages/core/src/node/encode.tspackages/core/src/node/seal.tspackages/crypto/src/__tests__/aes.test.tspackages/crypto/src/__tests__/encoding.test.tspackages/crypto/src/aes/decrypt-ctr.tspackages/crypto/src/aes/decrypt.tspackages/crypto/src/aes/encrypt-ctr.tspackages/crypto/src/aes/encrypt.tspackages/crypto/src/aes/import-key.tspackages/crypto/src/aes/index.tspackages/crypto/src/index.tspackages/crypto/src/utils/encoding.tspackages/crypto/src/utils/index.tspackages/sdk-core/scripts/verify-filepointer.mtspackages/sdk-core/src/__tests__/file/file-node.test.tspackages/sdk-core/src/__tests__/folder.test.tspackages/sdk-core/src/__tests__/rotation/grant-remint.test.tspackages/sdk-core/src/__tests__/rotation/write-revocation.test.tspackages/sdk-core/src/__tests__/share/grant.test.tspackages/sdk-core/src/__tests__/share/navigate.test.tspackages/sdk-core/src/__tests__/upload.test.tspackages/sdk-core/src/file/index.tspackages/sdk-core/src/folder/registration.tspackages/sdk-core/src/rotation/engine.tspackages/sdk-core/src/share/grant.tspackages/sdk-core/src/share/navigate.tspackages/sdk-core/src/tee/__tests__/wrap.test.tspackages/sdk-core/src/tee/wrap.tspackages/sdk-core/src/upload/index.tspackages/sdk-core/src/vault/index.tspackages/sdk/src/__tests__/client-extended.test.tspackages/sdk/src/__tests__/client-pinning.test.tspackages/sdk/src/__tests__/client-shared-write.test.tspackages/sdk/src/__tests__/client-upload-concurrency.test.tspackages/sdk/src/__tests__/context.test.tspackages/sdk/src/__tests__/enumerate-shared-subtree.test.tspackages/sdk/src/__tests__/folder-listing.test.tspackages/sdk/src/__tests__/helpers.tspackages/sdk/src/__tests__/move-in-shared-folder.test.tspackages/sdk/src/__tests__/owner-reconcile.test.tspackages/sdk/src/__tests__/resolve-shared-subfolder-write-key.test.tspackages/sdk/src/__tests__/shared-folder-tree.test.tspackages/sdk/src/__tests__/shared-write.test.tspackages/sdk/src/__tests__/upload-batch.test.tspackages/sdk/src/client.tspackages/sdk/src/index.tspackages/sdk/src/share/context.tspackages/sdk/src/share/index.tspackages/sdk/src/share/shared-write.tspackages/sdk/src/types.ts
💤 Files with no reviewable changes (10)
- packages/sdk/src/tests/enumerate-shared-subtree.test.ts
- packages/sdk/src/tests/resolve-shared-subfolder-write-key.test.ts
- packages/sdk/src/tests/folder-listing.test.ts
- packages/sdk/src/tests/client-shared-write.test.ts
- packages/sdk/src/index.ts
- packages/sdk/src/tests/context.test.ts
- packages/sdk/src/share/index.ts
- apps/web/src/hooks/tests/useSharedWriteOps.test.ts
- packages/sdk/src/tests/move-in-shared-folder.test.ts
- packages/sdk/src/tests/shared-write.test.ts
Comment on lines
+5
to
+16
| current_phase: 78 | ||
| current_phase_name: Recovery Tool v3, Vault-Load Guards, Web UX and CI Guards | ||
| status: verifying | ||
| stopped_at: Phase 75 complete (75-05-PLAN.md), shipping PR | ||
| last_updated: "2026-07-11T20:30:00.000Z" | ||
| stopped_at: Completed 77-09-PLAN.md | ||
| last_updated: "2026-07-11T10:03:19.194Z" | ||
| last_activity: 2026-07-11 | ||
| last_activity_desc: Phase 75 complete and shipping, Phase 76 pending | ||
| last_activity_desc: Phase 77 complete, transitioned to Phase 78 | ||
| progress: | ||
| total_phases: 22 | ||
| completed_phases: 17 | ||
| total_plans: 193 | ||
| completed_plans: 193 | ||
| total_plans: 198 | ||
| completed_plans: 198 |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Synchronize the duplicated session metadata.
The new Phase 77/78 handoff records July 11, 2026, but the legacy footer still reports June 27–28, 2026. Update or remove the stale footer so the planning state has one authoritative handoff point.
Also applies to: 599-622
🤖 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 @.planning/STATE.md around lines 5 - 16, Update or remove the stale legacy
footer in STATE.md so it no longer reports the June 27–28, 2026 handoff; keep
the July 11, 2026 metadata near current_phase as the single authoritative
session handoff.
Comment on lines
+149
to
+154
| const teePublicKeyBytes = hexToBytes(currentPublicKey); | ||
| const wrappedBytes = await wrapIpnsKeyForTee( | ||
| params.rootIpnsKeypair.privateKey, | ||
| currentPublicKey | ||
| teePublicKeyBytes | ||
| ); | ||
| encryptedIpnsPrivateKey = bytesToHex(wrappedBytes); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Zeroize wrappedBytes after hex encoding.
wrappedBytes contains the ECIES-wrapped IPNS private key and remains live after bytesToHex returns. Clear this locally-owned buffer in a finally block without clearing the caller-owned private key.
Proposed fix
const wrappedBytes = await wrapIpnsKeyForTee(
params.rootIpnsKeypair.privateKey,
teePublicKeyBytes
);
- encryptedIpnsPrivateKey = bytesToHex(wrappedBytes);
+ try {
+ encryptedIpnsPrivateKey = bytesToHex(wrappedBytes);
+ } finally {
+ clearBytes(wrappedBytes);
+ }As per coding guidelines, sensitive key material must be cleared from memory after use.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const teePublicKeyBytes = hexToBytes(currentPublicKey); | |
| const wrappedBytes = await wrapIpnsKeyForTee( | |
| params.rootIpnsKeypair.privateKey, | |
| currentPublicKey | |
| teePublicKeyBytes | |
| ); | |
| encryptedIpnsPrivateKey = bytesToHex(wrappedBytes); | |
| const teePublicKeyBytes = hexToBytes(currentPublicKey); | |
| const wrappedBytes = await wrapIpnsKeyForTee( | |
| params.rootIpnsKeypair.privateKey, | |
| teePublicKeyBytes | |
| ); | |
| try { | |
| encryptedIpnsPrivateKey = bytesToHex(wrappedBytes); | |
| } finally { | |
| clearBytes(wrappedBytes); | |
| } |
🤖 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/sdk-core/src/vault/index.ts` around lines 149 - 154, Update the
wrapping flow around wrapIpnsKeyForTee in the current method to zeroize the
locally owned wrappedBytes buffer in a finally block after bytesToHex completes,
while preserving encryptedIpnsPrivateKey and never clearing the caller-owned
params.rootIpnsKeypair.privateKey.
Source: Coding guidelines
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.
Phase 77 — Crypto Hygiene & Terminology Canonicalization
Refactor/hygiene phase: consolidate duplicated crypto codecs, tighten key-buffer zeroization, and canonicalize IPNS-key terminology across the monorepo. No behavior change to the crypto itself — every codec swap is byte-for-byte parity-gated.
What changed
bytesToBase64/base64ToBytesinto@cipherbox/crypto; replaced ~10 copy-pasted codecs (core/node/{encode,decode,seal},sdk-core/{rotation,share,file}) with imports. Golden-vector + node-codec parity gates prove byte-identical output.importAesKeyhelper owns a local key copy and zeroes it in afinally; all 7 AES-GCM/CTR functions route through it. D-09 preserved: the caller's borrowed buffer is never mutated.wrapIpnsKeyForTeebytes-in/bytes-out — hex boundary pushed to the 3 callers; helper borrowsipnsPrivateKey(never zeroes it).encryptedIpnsKeytoencryptedIpnsPrivateKeyacross API relay + tee-worker (sender and receiver renamed together).ipnsPrivateKeyEncryptedtoencryptedIpnsPrivateKey.ShareCallbacks,addShareKeysFn,updateSharePermission, and the orphanedUpdatePermissionDtoclient model.assertRootOwnershipextraction — single auditable root-ownership gate shared bycreateShare+createInvite(behavior-preserving).createSubfolderandverify-filepointer.mtszero minted/loaded keys on throw.Ship-phase gates
77-VERIFICATION.md)77-SECURITY.md)threats_open: 077-VALIDATION.md)105/106green — see caveat belowFixes applied during ship (defects this phase itself introduced/left)
hexToBytes/wrapIpnsKeyForTeeblock sat just outside the try, so a malformed TEE pubkey throwing inhexToBytesleaked the 3 minted keys. Moved thetryto open right after key generation so every post-mint throw (validation + wrap) zeroes. Added a forced-throw test.tryto open immediately afteruserPrivateKeyallocation so a throw during vault load still reaches thefinallyand clears the private key;finallynow tolerates an absent vault blob.SDK E2E caveat —
tee-republish.test.tsTest AThe only non-passing e2e is
tee-republishTest A (waitFortimeout). Root-caused and confirmed orthogonal to this phase: the republish batch loggedprocessed=0and the TEE worker received zero/republishcalls — the batch found no due schedule row (a localmakeScheduleDue-vs-enrollment-commit scheduling race), so the run never reached the renamed wire field. Reproduced identically on a clean, truncated DB (not state pollution). The renamedencryptedIpnsPrivateKeydecode+re-sign contract is instead proven by the tee-worker unit suite (76 pass, realdecryptWithFallback) and the api tee/republish specs. Deferred to the CIsdk-e2egate, which provisions a clean stack.Triage tally
wrapIpnsKeyForTee1:1 passthrough (deliberate D-09 seam) and a pre-existingclient.tscodec dedup (out of phase scope); security sweep — 1 unused-import false positive.🤖 Generated with Claude Code
Summary by CodeRabbit
Security
Improvements
Breaking Changes