Skip to content

feat: SDK write-chain, write-revocation, bin re-link, and invite claim#583

Merged
FSM1 merged 70 commits into
mainfrom
feat/sdk-write-chain-bin-re-link-and-invite-claim
Jun 30, 2026
Merged

feat: SDK write-chain, write-revocation, bin re-link, and invite claim#583
FSM1 merged 70 commits into
mainfrom
feat/sdk-write-chain-bin-re-link-and-invite-claim

Conversation

@FSM1

@FSM1 FSM1 commented Jun 30, 2026

Copy link
Copy Markdown
Owner

Summary

Phase 65 builds the SDK write plane for the v2.0 metadata/sharing refactor: a write-body that carries Ed25519 signing material sealed under a separate writeKey, full (c) Ed25519 write-revocation per ADR-0001, bin restore as a pure re-link, and invite claim as a single-key re-wrap. Delivers WRITE-01..04. 7 plans across 4 waves, verified 4/4 must-haves, with the heaviest operation proven end-to-end against the live API.

What shipped

  • WRITE-01 — structured write chain. sealChildWriteKey/unsealChildWriteKey (role 0x04, AAD-bound) added to @cipherbox/core; packages/sdk/src/share/shared-write.ts rewritten onto the Phase-62 write-body model (six former stubs implemented). A read-only holder with only the readKey can never reach signing material — unsealNode(published, readKey) returns no writeBody (asserted in shared-write.test.ts).
  • WRITE-02/03/04 — write-revocation. rotateWriteFromNode (packages/sdk-core/src/rotation/engine.ts): full Ed25519 rotation of the write plane for a subtree, child-first cascade, new k51 name per node (first-publish seq 1n), parent re-point to the share root, tombstone-intent via teeUnenrollFn, surviving co-writer re-wrap via wrapKey, revoked recipient dropped. Read plane stays invariant.
  • WRITE-03 — offline co-writer. CannotWriteUntilRefetchError thrown by every shared-write op when the injected publish seam reports a tombstoned/rotated-out target.
  • Read-rotation safety. rotateOne now threads the real writeKey through all three re-seal sites; the all-zeros PLACEHOLDER_WRITE_KEY is removed (folds carried-forward FLAG-63-U1).
  • Bin restore is a pure re-link (sealChildReadKey of the node's own readKey under the destination parent — no content re-encryption); BinEntry.nodeReadKey/nodeIpnsName added; legacy originalFolderKeyEncrypted path deleted.
  • Invite claim re-wraps a single root readKey via the existing claimInviteReadKey primitive and persists one standard grant — no encryptedChildKeys fan-out.
  • D-04 phase gate — a real tests/sdk-e2e/src/suites/write-chain-rotation.test.ts round-trip against the live docker API.

Gates

  • pnpm typecheck (full dependency-ordered build+typecheck chain): green.
  • Unit tests: crypto 197, core 199, sdk-core 318, sdk 207, web shared-folder hook 9 — all pass.
  • Live D-04 e2e: write-chain-rotation 2/2 against the real API (asserts new k51 per node, parent re-point cascade, tombstone-intent, co-writer re-wrap, read-plane invariance).
  • Regression gate (crypto + core foundational suites): green.
  • Verification: 4/4 must-haves (65-VERIFICATION.md).

Notable deviation

Plan 65-04's SharedWriteContext reshape under-scoped its consumers; the post-merge gate caught it and fix(65-04) reconciled the surface: packages/sdk/src/share/context.ts, client.ts:buildSharedWriteContextFromState, SharedFolderState, plus a clearly Phase-68-marked placeholder bridge in apps/web/src/hooks/shared-folder-projection.ts (optional writeKey?/publishedNode? defaults; the web shared-write path was already non-functional pre-phase-65). Full typecheck + SDK + web-hook tests green after the fix.

Deferred / out of scope (tracked)

  • WRITE-04's live publish-gate 403/410 enforcement is a D-02 mock seam this phase; Phase 66 cuts it over live (matched by Phase 66 success criteria).
  • packages/sdk/src/client.ts subtree IPNS collect stub is unimplemented — no phase-65 plan owns client.ts; carry-forward.
  • Pre-existing tests/sdk-e2e type debt in other suites (batch-upload, ipns-consistency, file-operations, folder-crud, bin-operations) references types removed in the earlier metadata refactor — not touched by phase 65; tracked by existing remigrate-filepointer-era-e2e-helper-scripts + upload-batch-test-mock-type-drift todos. sdk-e2e is not in CI.
  • The bin-delete / empty-bin pin-leak todo was re-tagged resolves_phase: 6566 (its fix lives in apps/api/src/shares/*, which phase 65 is prohibited from touching).

Review hardening (CodeRabbit triage)

CodeRabbit raised 23 findings; triaged via parallel verify-then-fix worktree agents (18 fixed, 3 skipped with reason, 2 deferred to Phase 68, 1 deferred test-hardening todo):

  • Rotation — tombstone-intent now deferred until the entire subtree is published (child tombstones could previously fire before the parent was updated); rotateWriteFromNode returns the new root IPNS name; tighter key zeroization (post-publish, and childReadKey zeroed on the write-key-unseal error path); strengthened rotation/e2e assertions (the e2e now proves the persisted survivor descriptor unseals to the rotated root write key).
  • Shared-writeupdateSharedFile keeps WriteChildRef.childId aligned with the file Node id (no fresh UUID); moveInSharedFolder publishes the destination before removing the source; parent re-publish uses the next sequence number; fail-closed when no write key is resolvable; success-path key buffers zeroed before release.
  • BinrestoreFromBin fails closed on missing nodeRef/nodeIpnsName (no wrong-AAD binding); addToBin persists the bin entry before republishing the source (no orphan on failure).
  • InviteclaimInvite rejects empty/whitespace inviteToken/rootNodeId/rootIpnsName before any I/O.
  • Skipped (with reason): seal 0x04 length-guard (would diverge the verbatim 0x02 twin which also has no guard — validation is upstream); one trivially-true rotation assertion (the buffer is zeroed by design).
  • Deferred: client/web publishedNode writeback + real placeholder values (Phase 68 — see todo); the e2e fixed-index seed identification (test-hardening todo).

All gates re-run green after the fixes: full pnpm typecheck, sdk-core 318, sdk 208, live D-04 e2e 2/2.

Test plan

  • pnpm typecheck green
  • Unit suites (crypto/core/sdk-core/sdk/web hook) green
  • Live D-04 write-chain-rotation e2e green against local stack
  • 4/4 must-haves verified (65-VERIFICATION.md)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Added write-chain rotation support, including child write-key handling, write-enabled node rotation, and co-writer revocation/re-wrapping.
    • Added invite claiming to generate an individual read grant per claimer.
    • Added/finished shared-write operations for shared folders: create, upload, rename, delete, move, and update files.
    • Implemented recycle-bin restore as a pure re-link that preserves deleted item key history.
  • Bug Fixes
    • Improved rotation fail-closed behavior when write keys are missing/invalid.
    • Fixed bin metadata serialization to reliably preserve restored key material.

FSM1 and others added 30 commits June 29, 2026 23:52
Entire-Checkpoint: aed8bf6fa830
Entire-Checkpoint: f49b3b2fe198
Entire-Checkpoint: fe7e071404d5
Entire-Checkpoint: 3c9673822b65
WRITE-01 write-body Ed25519 under writeKey (role 0x04), WRITE-02 full
Ed25519 write-revocation (child-first cascade), WRITE-03 co-writer
re-wrap + offline error, WRITE-04 tombstone-intent; bin re-link +
invite re-wrap (Success Criterion 4). Mock-seam discipline (D-02);
sdk-e2e round-trip gate (D-04). OQ-1 (BinEntry.nodeReadKey) and OQ-2
(child-first cascade) resolved.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NUSYqMyCLbNGDLcn1cp1eh
Entire-Checkpoint: 9638c8677236
…ark research OQs resolved

Entire-Checkpoint: e025a94f56f3
Entire-Checkpoint: 2a373269bfea
- round-trip: sealChildWriteKey then unsealChildWriteKey recovers 32-byte key
- role isolation: role-0x02 blob rejected by role-0x04 unseal and vice versa
- AAD binding: childId / childKind / childGeneration / wrong key all throw
- terminal-owner: input buffers unchanged after both calls (D-09 / T-65-04)
… role 0x04

- verbatim copy of sealChildReadKey / unsealChildReadKey with role byte 0x02 → 0x04
- AAD: buildNodeAad(childId, kindByte, childGeneration, 0x04) — child-writekey role frozen
- no new imports: reuses sealAesGcmAad / unsealAesGcmAad / buildNodeAad from @cipherbox/crypto
- D-09 terminal-owner: neither function zeros caller-supplied buffers
- exported from packages/core/src/node/index.ts and packages/core/src/index.ts
- all 9 role-0x04 tests green (round-trip, cross-role rejection, AAD-mismatch, terminal-owner)
- RED: claimInvite import fails — function does not exist yet
- Asserts getInviteDataFn called once per claim
- Asserts insertShareFn called once with no encryptedChildKeys
- Asserts two independent claims of same invite yield two separate grants
- Add claimInvite to packages/sdk-core/src/share/grant.ts composing
  claimInviteReadKey with injected getInviteDataFn and insertShareFn callbacks
- Re-export claimInvite from packages/sdk-core/src/share/index.ts
- One re-wrapped root readKey per claimer, no per-child key fan-out
- Zero sdk-layer consumption of the fan-out path confirmed by grep gate
…mBin

- add nodeReadKey and nodeIpnsName fields to BinEntry in @cipherbox/core
- un-skip addToBin and restoreFromBin describe blocks
- remove legacy originalFolderKeyEncrypted and FolderChild references
- add 4 addToBin tests covering happy-path, revoke ordering, abort-on-revoke-fail, and not-loaded guard
- add 5 restoreFromBin tests including AEAD asymmetry proof via real sealChildReadKey
- switch vi.mock to importOriginal spread keeping real AES-GCM primitives for asymmetry test
addToBin:
- resolve child IPNS record to extract plaintext PublishedNode id and kind
- unseal nodeReadKey from source parent folderKey via unsealChildReadKey
- revoke shares before destructive folder mutation (fail-closed ordering)
- capture nodeReadKey and nodeIpnsName on BinEntry for restore

restoreFromBin:
- re-seal entry.nodeReadKey under destination folderKey via sealChildReadKey (role 0x02)
- build SealedChildRef with re-linked readKeySealed (no content re-encryption)
- add restored ref to target folder and publish; remove entry from bin

also fix test fixture: use valid UUID for nodeRef.id to satisfy uuidToBytes validator;
replace vi.restoreAllMocks with targeted sealSpy.mockRestore to prevent resetting
module-level vi.fn mocks (deriveBinIpnsKeypair) in subsequent test cases
Entire-Checkpoint: 02004cb54e8b
…plane preservation

- Tests 1-4: assert unsealNode called with nodeWriteKey, sealNode receives real writeKey
  not all-zeros placeholder, writeBody.ipnsPrivateKey preserved post-rotation
- Tests 5-6: fail-closed guard -- writeSealed present without valid writeKey throws
- Test 7: nodeKeySource.writeKey threads through BFS to sealNode
- All 6 write-plane assertions fail RED against placeholder engine
…ER_WRITE_KEY

- Add nodeWriteKey to RotateOneParams and nodeKeySource return type
- unsealNode now called with nodeWriteKey to recover write-body on rotation
- Fail-closed guard: throws when published.writeSealed present but nodeWriteKey
  is absent or all-zeros, mirroring the IPNS-key guard pattern
- writeKeyForReseal = node.writeBody ? nodeWriteKey : empty; used at all three
  sealNode sites: main reseal and both CAS-409 merge paths
- PLACEHOLDER_WRITE_KEY deleted from all three sites
- rotateReadFromNode BFS threads nodeKeySource.writeKey at root, child, grandchild
  and dirty-resume enqueue sites
- Existing IPNS fail-closed guard retained unchanged
- Folds FLAG-63-U1 / rotateone-placeholder-writekey-phase65
… security and WRITE-03 offline error

- Reshape SharedWriteContext: writeKey + readKey + publishedNode replaces raw folderKey and ipnsPrivateKey fields
- Add publishNodeFn and addToIpfsFn transport seams for mock-testable operations
- Add buildChildWriteLink helper sealing child writeKey under parent writeKey via role 0x04
- Add walkChildWriteKey helper walking the write chain via unsealChildWriteKey
- Prove WRITE-01 security: unsealNode without writeKey returns no writeBody; read-only holder cannot reach ipnsPrivateKey
- Prove NODE-03: SealedChildRef carries no write field; write link lives only in parent writeBody.writeChildren
- Implement createSharedSubfolder: mint child keypair, build write-body, seal, publish, add SealedChildRef + WriteChildRef to parent
- Implement uploadToSharedFolder: same pattern for file node with AES-256-GCM content encryption via addToIpfsFn
- Implement renameInSharedFolder: mutate read-body child name, preserve writeChildren, re-publish parent
- Implement deleteFromSharedFolder: remove from children and writeChildren by IPNS name, re-publish parent
- Implement updateSharedFile: caller-supplied fileReadKey + fileWriteKey + fileIpnsPrivateKey, re-seal + re-publish
- Implement moveInSharedFolder: re-seal child readKey under dest readKey via sealChildReadKey, walk write chain for writeKey re-link
- Add CannotWriteUntilRefetchError with stable code CANNOT_WRITE_UNTIL_REFETCH for tombstoned publish targets
- addShareKeysFn never invoked across all six operations
- All 29 shared-write tests pass including WRITE-01 security, write-link round-trip, and WRITE-03 tombstone tests
WriteChildRef.childId is a UUID from crypto.randomUUID matching Node.id,
not the IPNS name. Clarify deleteFromSharedFolder itemId semantics and
remove incorrect module-header annotation.
@github-actions github-actions Bot added release:sdk:feat Minor version bump (new feature) for sdk release:web:fix Patch version bump (bug fix) for web release:tee-worker:fix Patch version bump (bug fix) for tee-worker labels Jun 30, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Release Preview

Package Bump Label Source
core minor release:core:feat Direct (feat commit)
sdk minor release:sdk:feat Direct (feat commit)
sdk-core minor release:sdk-core:feat Direct (feat commit)
tee-worker patch release:tee-worker:fix Cascade (core minor)
web minor release:web:fix Direct (fix commit)

Cascade Details

  • core minor -> tee-worker patch (direct dependency)

@codecov

codecov Bot commented Jun 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 79.00485% with 173 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.63%. Comparing base (4ad615a) to head (200fe5e).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
packages/sdk/src/share/shared-write.ts 83.33% 77 Missing and 1 partial ⚠️
packages/sdk-core/src/rotation/engine.ts 80.00% 42 Missing and 2 partials ⚠️
packages/sdk/src/client.ts 19.35% 25 Missing ⚠️
packages/sdk-core/src/share/grant.ts 51.02% 24 Missing ⚠️
packages/core/src/bin/encrypt.ts 92.59% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##             main     #583       +/-   ##
===========================================
+ Coverage   64.64%   83.63%   +18.99%     
===========================================
  Files         152      116       -36     
  Lines       11585     8153     -3432     
  Branches     1314     1389       +75     
===========================================
- Hits         7489     6819      -670     
+ Misses       3853     1088     -2765     
- Partials      243      246        +3     
Flag Coverage Δ
api 83.63% <79.00%> (-0.02%) ⬇️
api-client 83.63% <79.00%> (-0.02%) ⬇️
core 83.63% <79.00%> (-0.02%) ⬇️
crypto 83.63% <79.00%> (-0.02%) ⬇️
desktop ?
sdk 83.63% <79.00%> (-0.02%) ⬇️
sdk-core 83.63% <79.00%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
packages/core/src/node/seal.ts 95.86% <100.00%> (+0.82%) ⬆️
packages/sdk/src/share/context.ts 100.00% <100.00%> (ø)
packages/core/src/bin/encrypt.ts 90.56% <92.59%> (+0.91%) ⬆️
packages/sdk-core/src/share/grant.ts 55.03% <51.02%> (-2.47%) ⬇️
packages/sdk/src/client.ts 80.76% <19.35%> (-1.60%) ⬇️
packages/sdk-core/src/rotation/engine.ts 82.73% <80.00%> (-1.40%) ⬇️
packages/sdk/src/share/shared-write.ts 84.24% <83.33%> (+69.95%) ⬆️

... and 36 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@greptile-apps

greptile-apps Bot commented Jun 30, 2026

Copy link
Copy Markdown

Greptile Summary

This PR implements the Phase 65 SDK write plane: a structured write chain with Ed25519 signing material sealed under a separate writeKey, full write-revocation (rotateWriteFromNode), bin restore as a pure re-link, and invite claim as a single-key re-wrap. All six shared-write stubs from Phase 62 are fully implemented, and all five findings from the previous review round have been addressed.

  • Write chain (WRITE-01): sealChildWriteKey/unsealChildWriteKey (role 0x04) added to @cipherbox/core; SharedWriteContext reshaped to carry readKey/writeKey/publishedNode; six shared-write operations implemented with tight key-zeroing discipline (D-09) and a CannotWriteUntilRefetchError for rotated-out co-writers (WRITE-03).
  • Write revocation (WRITE-02/03/04): rotateWriteFromNode in sdk-core performs a child-first recursive write-plane rotation — new k51 + Ed25519 keypair + writeKey per node, deferred tombstone-intent after the entire subtree is published, surviving co-writer re-wrap via wrapKey, revoked grant deletion — with read-plane invariance preserved throughout.
  • Bin restore & invite claim: addToBin captures nodeReadKey/nodeIpnsName before the destructive folder publish (correct fail-safe ordering); restoreFromBin re-seals via sealChildReadKey with no content re-encryption; claimInvite adds a full service-flow wrapper around claimInviteReadKey with key-copy snapshotting before the first await.

Confidence Score: 5/5

Safe to merge — all findings from the previous review round are addressed and no new blocking issues were found in the write-chain, rotation, bin, or invite-claim paths.

The write-rotation and shared-write implementations follow the documented security invariants carefully: key zeroing on all failure paths, fail-closed guards on missing/zero write keys, correct AAD generation matching (uses childRef.generation throughout), deferred tombstone ordering, and the publishNodeFn success-flag guard. The two remaining observations (versionFloor hardcoded to 0n on restore, and the misleading addToBin parameter name) are non-blocking style and correctness nits that do not affect runtime behaviour in the current system.

packages/sdk/src/bin/index.ts — restoreFromBin versionFloor and addToBin parameter naming

Important Files Changed

Filename Overview
packages/sdk-core/src/rotation/engine.ts Adds rotateWriteFromNode + rotateWriteSubtree (child-first Ed25519 rotation); threads real writeKey through all three rotateOne re-seal sites; removes PLACEHOLDER_WRITE_KEY; fail-closed guards on missing/zero write keys; deferred tombstone ordering correct.
packages/sdk/src/share/shared-write.ts All six Phase-62 stubs implemented on the Phase-65 write-body model; key-zeroing discipline (D-09) correct on both success and failure paths; deleteFromSharedFolder correctly splits itemId (IPNS) vs childNodeId (UUID); CannotWriteUntilRefetchError integrated at every publish site.
packages/sdk/src/bin/index.ts addToBin and restoreFromBin implemented; correct fail-safe ordering (bin save before folder publish in addToBin); correctly uses childRef.generation for unsealChildReadKey AAD. restoreFromBin hardcodes versionFloor: 0n instead of preserving the original 1n. addToBin's childId parameter is semantically an IPNS name despite the UUID-connoting name.
packages/core/src/bin/encrypt.ts Adds toBinWireForm/fromBinWireForm for Uint8Array↔hex conversion; correctly applied to encryptBinMetadata and decryptBinMetadata; fail-closed on wrong-length decoded key.
packages/core/src/node/seal.ts Adds sealChildWriteKey/unsealChildWriteKey with role byte 0x04 (distinct from read-key 0x02); AAD construction mirrors sealChildReadKey; D-09 documented; clean.
packages/sdk-core/src/share/grant.ts claimInvite adds a full service-flow wrapper; key-copy snapshotting before first await (FIX #11); ephemeralPrivateKeyCopy zeroed in finally; input validation for empty strings and key lengths; clean.
packages/sdk/src/client.ts buildSharedWriteContextFromState updated to carry readKey/writeKey/publishedNode; publishNodeFn guards pubResult.success (previously-fixed P1); deleteFromSharedFolder validates childNodeId at the SDK boundary; addToIpfsFn correctly routes through pinWithMode.
apps/web/src/hooks/shared-folder-projection.ts Placeholder writeKey (zero bytes) and PLACEHOLDER_PUBLISHED_NODE added for Phase-68 bridge; documented as non-functional until Phase-68 wiring; intentionally non-functional zero-key default.
tests/sdk-e2e/src/suites/write-chain-rotation.test.ts New D-04 e2e suite proving new k51 per node, parent re-point cascade, tombstone-intent, co-writer re-wrap, and read-plane invariance against the live docker API.
packages/sdk/src/share/context.ts SharedWriteContextParams updated to carry readKey/writeKey/publishedNode/publishNodeFn/addToIpfsFn; folderKey/ipnsPrivateKey removed from params (now derived from write-body); clean migration.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
  participant C as rotateWriteFromNode
  participant S as rotateWriteSubtree (recursive)
  participant IPFS
  participant IPNS
  participant CB as WriteRevocationCallbacks

  C->>S: rotate(rootIpnsName, readKey, writeKey, pendingTombstones[])
  S->>IPNS: resolveIpnsRecord(oldIpnsName)
  S->>IPFS: fetchFromIpfs(cid)
  S->>S: unsealNode(readKey, writeKey) — fail-closed if no writeBody
  loop for each WriteChildRef (child-first)
    S->>IPNS: resolveIpnsRecord(candidateChildRef)
    S->>IPFS: fetchFromIpfs(childCid)
    S->>S: unsealChildReadKey + unsealChildWriteKey
    S->>S: rotateWriteSubtree(child) [recursive]
    S->>S: zero childReadKey + childWriteKey (D-09)
  end
  S->>S: generateEd25519Keypair + generateRandomBytes(32)
  S->>S: rebuild write-body (newIPNSPrivKey, re-sealed child writeKeys)
  S->>S: sealNode(updatedNode, readKey, newWriteKey)
  S->>IPFS: addToIpfs(sealedNode)
  S->>IPNS: "createAndPublishIpnsRecord(newIpnsName, seq=1n)"
  S->>S: zero newKeypair.privateKey (D-09)
  S->>S: pendingTombstones.push(oldIpnsName)
  S->>C: WriteRotationResult
  C->>C: "verify rootResult.nodeId == rootNodeId"
  loop deferred tombstones
    C->>CB: teeUnenrollFn(oldIpnsName)
  end
  C->>CB: queryWriteGrantsFn(rootNodeId)
  loop for each grant
    alt grant.isRevoked
      C->>CB: deleteWriteGrantFn(shareId)
    else survivor
      C->>C: wrapKey(newWriteKey, recipientPublicKey)
      C->>CB: writeDescriptorRefPersistFn(shareId, wrapped)
    end
  end
  C->>C: zero rootResult.newWriteKey (finally / D-09)
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
  participant C as rotateWriteFromNode
  participant S as rotateWriteSubtree (recursive)
  participant IPFS
  participant IPNS
  participant CB as WriteRevocationCallbacks

  C->>S: rotate(rootIpnsName, readKey, writeKey, pendingTombstones[])
  S->>IPNS: resolveIpnsRecord(oldIpnsName)
  S->>IPFS: fetchFromIpfs(cid)
  S->>S: unsealNode(readKey, writeKey) — fail-closed if no writeBody
  loop for each WriteChildRef (child-first)
    S->>IPNS: resolveIpnsRecord(candidateChildRef)
    S->>IPFS: fetchFromIpfs(childCid)
    S->>S: unsealChildReadKey + unsealChildWriteKey
    S->>S: rotateWriteSubtree(child) [recursive]
    S->>S: zero childReadKey + childWriteKey (D-09)
  end
  S->>S: generateEd25519Keypair + generateRandomBytes(32)
  S->>S: rebuild write-body (newIPNSPrivKey, re-sealed child writeKeys)
  S->>S: sealNode(updatedNode, readKey, newWriteKey)
  S->>IPFS: addToIpfs(sealedNode)
  S->>IPNS: "createAndPublishIpnsRecord(newIpnsName, seq=1n)"
  S->>S: zero newKeypair.privateKey (D-09)
  S->>S: pendingTombstones.push(oldIpnsName)
  S->>C: WriteRotationResult
  C->>C: "verify rootResult.nodeId == rootNodeId"
  loop deferred tombstones
    C->>CB: teeUnenrollFn(oldIpnsName)
  end
  C->>CB: queryWriteGrantsFn(rootNodeId)
  loop for each grant
    alt grant.isRevoked
      C->>CB: deleteWriteGrantFn(shareId)
    else survivor
      C->>C: wrapKey(newWriteKey, recipientPublicKey)
      C->>CB: writeDescriptorRefPersistFn(shareId, wrapped)
    end
  end
  C->>C: zero rootResult.newWriteKey (finally / D-09)
Loading

Reviews (6): Last reviewed commit: "fix(65): use childRef.generation for bin..." | Re-trigger Greptile

Comment thread packages/sdk/src/share/shared-write.ts Outdated
Comment thread packages/sdk-core/src/rotation/engine.ts
Comment thread packages/sdk/src/share/shared-write.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 17

🧹 Nitpick comments (4)
packages/core/src/bin/types.ts (1)

53-59: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Narrow nodeRef to the fields the bin actually persists.

BinEntry.nodeRef is typed as a full Node, but packages/sdk/src/bin/index.ts only stores id, kind, and generation, padding the rest with placeholder values. That makes it easy for a later caller to treat this as a real node and read bogus metadata. A dedicated BinNodeRef shape would keep the persisted contract honest.

🤖 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/core/src/bin/types.ts` around lines 53 - 59, The persisted bin
reference is typed too broadly: BinEntry.nodeRef currently uses the full Node
type even though the bin only saves id, kind, and generation. Narrow this field
to a dedicated BinNodeRef shape and update the related bin persistence code in
packages/sdk/src/bin/index.ts to use that exact contract so callers cannot
mistake placeholder metadata for a real node. Keep the new type aligned with the
fields actually written and consumed by BinEntry and the restore path.
packages/core/src/__tests__/seal-write-chain.test.ts (1)

30-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use fresh key fixtures per test.

These shared Uint8Array constants make the suite order-dependent if a crypto helper ever mutates an input buffer. Factory helpers or per-test copies would keep the D-09 assertions isolated and easier to trust. As per path instructions, **/*.test.ts: Focus on test coverage, edge cases, and test quality.

🤖 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/core/src/__tests__/seal-write-chain.test.ts` around lines 30 - 37,
Use fresh key fixtures in the seal-write-chain tests instead of shared
Uint8Array constants, since a helper mutating an input would make the suite
order-dependent. Refactor the test setup in seal-write-chain.test.ts to create
new childWriteKey, parentWriteKey, and wrong parent key instances per test (or
via small factory helpers), keeping the D-09 assertions isolated and avoiding
shared mutable state across tests.

Source: Path instructions

packages/sdk/src/__tests__/bin.test.ts (1)

231-265: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add a regression assertion for the folder-side result.

These cases verify bin state and ordering, but they never assert the folder snapshot that client.ts consumes after addToBin() / restoreFromBin(). A small contract test around returned publishedChildren / sequence (or explicit folderTree mutation) would catch the stale-cache break here. As per path instructions, **/*.test.ts: Focus on test coverage, edge cases, and test quality.

Also applies to: 427-456

🤖 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/__tests__/bin.test.ts` around lines 231 - 265, Add a
regression assertion in the relevant bin tests for the folder-side contract that
`client.ts` relies on, not just `updatedBinState` and item ordering. After
`addToBin()` and/or `restoreFromBin()`, verify the returned folder snapshot
behavior through `publishedChildren` and sequence updates, or by asserting the
expected `folderTree` mutation, using the existing `addToBin` / `restoreFromBin`
test setup and symbols like `FolderTree`, `BinState`, and `client.ts`-consumed
results to catch stale-cache regressions.

Source: Path instructions

packages/sdk-core/src/__tests__/share/grant.test.ts (1)

276-449: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add a failure-path assertion for the “never persist on crypto failure” contract.

This suite covers the happy path well, but it never proves that insertShareFn stays untouched when getInviteDataFn rejects or reWrapKey throws. That’s the main guardrail in claimInvite, so it’s worth pinning down with an explicit test. As per path instructions, "**/*.test.ts: Focus on test coverage, edge cases, and test quality. Ensure tests are meaningful and not just for coverage metrics."

🤖 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__/share/grant.test.ts` around lines 276 - 449,
Add a failure-path test in claimInvite to assert that persistence never happens
when crypto or invite lookup fails. In the claimInvite describe block, add cases
where getInviteDataFn rejects and where mockFns.reWrapKey throws, then verify
insertShareFn is never called and the error propagates. Use the existing
claimInvite, getInviteDataFn, insertShareFn, and mockFns.reWrapKey symbols to
keep the new assertions aligned with the current helper setup.

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 `@packages/sdk-core/src/__tests__/rotation/write-body-reseal.test.ts`:
- Around line 202-215: The green-path tests around rotateOne are swallowing
failures with .catch(() => {}), which can mask regressions now that the mocked
unsealNode/sealNode behavior is in place. Update the affected test cases in
write-body-reseal.test.ts to let rotateOne reject naturally instead of catching
it, while still keeping the existing assertions on the captured mock arguments
for the successful path.

In `@packages/sdk-core/src/__tests__/rotation/write-revocation.test.ts`:
- Around line 256-267: The crypto mocks in the write revocation test are reusing
shared fixture arrays, which lets `rotateWriteFromNode` mutate `NEW_*` constants
and leak state into later tests. Update the mock setup around
`generateEd25519Keypair` and `generateRandomBytes` to return fresh key buffers
on each call in this test, and keep the `deriveIpnsName` expectations tied to
those new per-call values so the test remains isolated.

In `@packages/sdk-core/src/rotation/engine.ts`:
- Around line 1451-1480: The root write key in rotateWriteFromNode is only
cleared on the success path, so any failure in teeUnenrollFn,
queryWriteGrantsFn, wrapKey, or writeDescriptorRefPersistFn can leave sensitive
material in memory. Move rootResult.newWriteKey.fill(0) into a finally block
that always runs after the tombstone/grant processing, and preserve the existing
success/error behavior for the callbacks and grant handling. Keep the zeroing
scoped to the rotateWriteFromNode flow so the key is wiped exactly once after
all uses, even when an exception is thrown.
- Around line 1442-1460: The grant mutation flow is using rootNodeId without
verifying it matches the unsealed rotated root, which could target the wrong
node. In the rotation engine around rotateWriteSubtree, bind the authoritative
root from the unsealed result and compare its node.id to rootNodeId before
calling queryWriteGrantsFn, deleteWriteGrantFn, or persisting the descriptor. If
they differ, stop the rotation path and surface an error so grant rewrites only
proceed for the actual rotated root.
- Around line 1364-1371: The rotation flow in engine.ts is ignoring the result
of createAndPublishIpnsRecord, so a rejected first k51 publish can still proceed
to tombstone creation and grant mutation. Update the publish step to capture the
returned { success, sequenceNumber } from createAndPublishIpnsRecord and verify
success before continuing. If the publish is not successful, stop the rotation
path early and avoid the TEE unenroll and grant rewrap work in the surrounding
rotation logic.
- Around line 1320-1329: The child rotation flow in the parent sealing path
leaves each child’s `newWriteKey` alive after `sealChildWriteKey` finishes, even
though the parent is the terminal owner. Update the child-processing block
around `sealChildWriteKey` and the `idToChildResult`/`ipnsToChildResult`
mappings so the child write keys are cleared immediately after they are used to
seal the parent link. Wrap the cleanup in a `finally` so sensitive keys are
removed even if parent sealing or publish fails after some children have already
rotated.
- Around line 1218-1349: rotateWriteSubtree currently continues after unsealNode
returns a node with no writeBody and later reconstructs a new writeBody, which
can incorrectly mint a writable node from a read-only or unrecoverable
write-plane state. Add a fail-closed guard immediately after unsealNode in
rotateWriteSubtree so the function throws unless the recovered node has a valid
writeBody (and expected writeChildren before proceeding), and ensure any
crypto-related recovery path does not fallback to creating a fresh write body.
Reference the unsealNode call and the subsequent write-body rebuild logic in
rotateWriteSubtree when applying the check.

In `@packages/sdk-core/src/share/grant.ts`:
- Around line 300-328: The grant persistence in claimInvite currently validates
trimmed root identifiers but still passes the original untrimmed
params.rootNodeId and params.rootIpnsName into insertShareFn, so trim those
values once and persist the normalized versions instead. Update claimInvite in
grant.ts to use the trimmed rootNodeId and rootIpnsName variables consistently
for the emptiness checks and the insertShareFn payload, keeping the rest of the
flow unchanged.
- Around line 281-319: The claimInvite flow validates caller-owned key buffers
and then awaits getInviteDataFn before using them, so the original Uint8Array
values can be mutated or zeroized before claimInviteReadKey runs. In
claimInvite, snapshot ephemeralPrivateKey and claimerPublicKey into owned copies
before the await, use those copies for the rest of the async flow and the call
to claimInviteReadKey, and ensure the private-key copy is cleared in a finally
block after use.

In `@packages/sdk/src/__tests__/shared-write.test.ts`:
- Around line 412-459: The deleteFromSharedFolder tests are missing coverage for
write-body cleanup, so they can pass even if the republished parent still
retains the removed WriteChildRef. Update the delete tests around
deleteFromSharedFolder, buildSealedParent, and makeSWCtx to include a real
writeChildren entry for the child being deleted, then assert the republished
parent no longer contains that write-body reference in addition to checking
publishedChildren and the sequence number. Keep the existing addShareKeysFn
assertion, but strengthen the test setup so it validates both visible-child
removal and underlying write-body cleanup.

In `@packages/sdk/src/bin/index.ts`:
- Around line 333-336: The folder delete flow in `packages/sdk/src/bin/index.ts`
is only revoking shares for the single `childId`, but the contract requires
revoking every collected subtree IPNS name before the destructive publish.
Update the revoke step in the folder delete path to pass the full collected
subtree/item list from the delete traversal, not just the root child, so
`revokeSharesForItemsFn` is called for all descendants before mutation.
- Around line 467-488: Make the restore flow idempotent in the restore path
around `sdkCore.updateFolderMetadataAndPublish` so a retry after
`saveBinMetadata()` failure does not duplicate the restored child. Before
building `children: [...targetFolder.children, restoredItem]`, check whether the
target folder already contains the restored item (by `ipnsName` or equivalent
unique ref) and reuse the existing entry instead of appending again. Keep the
existing bin cleanup logic (`remainingEntries`, `saveBinMetadata`) unchanged so
a retried restore can finish removing the bin entry.
- Around line 378-399: The publish result from updateFolderMetadataAndPublish()
is being discarded in the bin op flow, leaving the in-memory folder state stale
after delete/restore. Update the bin operation path in index.ts (the code around
the publish call that builds the returned removedItem/updatedBinState) so it
either mutates folderTree with the publishedChildren/newSequenceNumber or
returns that folder delta alongside the bin state. Make the corresponding bin
ops that call updateFolderMetadataAndPublish() propagate the published folder
snapshot so client.ts can read the fresh state when emitting folder:updated.

In `@packages/sdk/src/share/shared-write.ts`:
- Around line 693-718: The shared-file rebuild in the file-node update path is
resetting immutable/history metadata, so preserve the original createdAt and
versions instead of overwriting them with now and an empty array. Update the
logic around nodeContent and fileNode to source these fields from the existing
file state (or extend the relevant params/data passed into shared-write.ts so
they are available here). Keep the caller-supplied fileNodeId and generation
handling unchanged, but ensure the rebuilt Node still carries forward the prior
creation time and version history.
- Around line 613-617: The delete logic in shared-write uses one itemId for two
different identifiers, but the read-body children are matched by ipnsName while
WriteChildRef.childId comes from the UUID created in
createSharedSubfolder()/uploadToSharedFolder(). Update the deletion flow around
shared-write deletion to accept and use separate identifiers for read-body and
write-body removal, and adjust the filters on swCtx.children and
parentWriteChildren accordingly so each side deletes by its own key.
- Around line 247-272: The updated parent envelope from resealAndPublishParent
is currently discarded, which lets later shared writes rebuild from a stale
parent and lose previously added write links. Update resealAndPublishParent to
return the newly sealed parent envelope alongside the published children and
sequence number, and thread that value through the SharedFolderState/shared
write flow so subsequent writes use the latest publishedNode instead of
re-unsealing the pre-write parent. Locate the change around
resealAndPublishParent and the code that reads SharedFolderState.publishedNode
when rebuilding later writes.

In `@tests/sdk-e2e/src/suites/write-chain-rotation.test.ts`:
- Around line 134-154: The helper in publishWriteCapableNode leaves the minted
IPNS private key on the returned keypair even though callers only need ipnsName.
Keep the plaintext seed scoped to publishWriteCapableNode, ensure
createAndPublishIpnsRecord still uses the keypair during publishing, and then
clear or zero the privateKey in a finally block before returning so sensitive
data is not retained in memory.

---

Nitpick comments:
In `@packages/core/src/__tests__/seal-write-chain.test.ts`:
- Around line 30-37: Use fresh key fixtures in the seal-write-chain tests
instead of shared Uint8Array constants, since a helper mutating an input would
make the suite order-dependent. Refactor the test setup in
seal-write-chain.test.ts to create new childWriteKey, parentWriteKey, and wrong
parent key instances per test (or via small factory helpers), keeping the D-09
assertions isolated and avoiding shared mutable state across tests.

In `@packages/core/src/bin/types.ts`:
- Around line 53-59: The persisted bin reference is typed too broadly:
BinEntry.nodeRef currently uses the full Node type even though the bin only
saves id, kind, and generation. Narrow this field to a dedicated BinNodeRef
shape and update the related bin persistence code in
packages/sdk/src/bin/index.ts to use that exact contract so callers cannot
mistake placeholder metadata for a real node. Keep the new type aligned with the
fields actually written and consumed by BinEntry and the restore path.

In `@packages/sdk-core/src/__tests__/share/grant.test.ts`:
- Around line 276-449: Add a failure-path test in claimInvite to assert that
persistence never happens when crypto or invite lookup fails. In the claimInvite
describe block, add cases where getInviteDataFn rejects and where
mockFns.reWrapKey throws, then verify insertShareFn is never called and the
error propagates. Use the existing claimInvite, getInviteDataFn, insertShareFn,
and mockFns.reWrapKey symbols to keep the new assertions aligned with the
current helper setup.

In `@packages/sdk/src/__tests__/bin.test.ts`:
- Around line 231-265: Add a regression assertion in the relevant bin tests for
the folder-side contract that `client.ts` relies on, not just `updatedBinState`
and item ordering. After `addToBin()` and/or `restoreFromBin()`, verify the
returned folder snapshot behavior through `publishedChildren` and sequence
updates, or by asserting the expected `folderTree` mutation, using the existing
`addToBin` / `restoreFromBin` test setup and symbols like `FolderTree`,
`BinState`, and `client.ts`-consumed results to catch stale-cache regressions.
🪄 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: c0e515f5-cdf6-4ea3-b3c2-456168297312

📥 Commits

Reviewing files that changed from the base of the PR and between c49b924 and 0e7e205.

📒 Files selected for processing (52)
  • .planning/REQUIREMENTS.md
  • .planning/ROADMAP.md
  • .planning/STATE.md
  • .planning/phases/65-sdk-write-chain-bin-re-link-and-invite-claim/65-01-PLAN.md
  • .planning/phases/65-sdk-write-chain-bin-re-link-and-invite-claim/65-01-SUMMARY.md
  • .planning/phases/65-sdk-write-chain-bin-re-link-and-invite-claim/65-02-PLAN.md
  • .planning/phases/65-sdk-write-chain-bin-re-link-and-invite-claim/65-02-SUMMARY.md
  • .planning/phases/65-sdk-write-chain-bin-re-link-and-invite-claim/65-03-PLAN.md
  • .planning/phases/65-sdk-write-chain-bin-re-link-and-invite-claim/65-03-SUMMARY.md
  • .planning/phases/65-sdk-write-chain-bin-re-link-and-invite-claim/65-04-PLAN.md
  • .planning/phases/65-sdk-write-chain-bin-re-link-and-invite-claim/65-04-SUMMARY.md
  • .planning/phases/65-sdk-write-chain-bin-re-link-and-invite-claim/65-05-PLAN.md
  • .planning/phases/65-sdk-write-chain-bin-re-link-and-invite-claim/65-05-SUMMARY.md
  • .planning/phases/65-sdk-write-chain-bin-re-link-and-invite-claim/65-06-PLAN.md
  • .planning/phases/65-sdk-write-chain-bin-re-link-and-invite-claim/65-06-SUMMARY.md
  • .planning/phases/65-sdk-write-chain-bin-re-link-and-invite-claim/65-07-PLAN.md
  • .planning/phases/65-sdk-write-chain-bin-re-link-and-invite-claim/65-07-SUMMARY.md
  • .planning/phases/65-sdk-write-chain-bin-re-link-and-invite-claim/65-CONTEXT.md
  • .planning/phases/65-sdk-write-chain-bin-re-link-and-invite-claim/65-DISCUSSION-LOG.md
  • .planning/phases/65-sdk-write-chain-bin-re-link-and-invite-claim/65-PATTERNS.md
  • .planning/phases/65-sdk-write-chain-bin-re-link-and-invite-claim/65-RESEARCH.md
  • .planning/phases/65-sdk-write-chain-bin-re-link-and-invite-claim/65-SECURITY.md
  • .planning/phases/65-sdk-write-chain-bin-re-link-and-invite-claim/65-VALIDATION.md
  • .planning/phases/65-sdk-write-chain-bin-re-link-and-invite-claim/65-VERIFICATION.md
  • .planning/todos/completed/2026-06-29-rotateone-placeholder-writekey-phase65.md
  • .planning/todos/pending/2026-06-23-bin-delete-and-empty-bin-leak-content-and-version-cid-pins.md
  • .planning/todos/pending/2026-06-30-phase68-shared-write-context-publishednode-wiring.md
  • .planning/todos/pending/2026-06-30-write-chain-e2e-seed-index-stability.md
  • apps/web/src/hooks/shared-folder-projection.ts
  • packages/core/src/__tests__/seal-write-chain.test.ts
  • packages/core/src/bin/types.ts
  • packages/core/src/index.ts
  • packages/core/src/node/index.ts
  • packages/core/src/node/seal.ts
  • packages/sdk-core/src/__tests__/rotation/write-body-reseal.test.ts
  • packages/sdk-core/src/__tests__/rotation/write-revocation.test.ts
  • packages/sdk-core/src/__tests__/share/grant.test.ts
  • packages/sdk-core/src/index.ts
  • packages/sdk-core/src/rotation/engine.ts
  • packages/sdk-core/src/rotation/index.ts
  • packages/sdk-core/src/share/grant.ts
  • packages/sdk-core/src/share/index.ts
  • packages/sdk/src/__tests__/bin.test.ts
  • packages/sdk/src/__tests__/client-shared-write.test.ts
  • packages/sdk/src/__tests__/context.test.ts
  • packages/sdk/src/__tests__/shared-write.test.ts
  • packages/sdk/src/bin/index.ts
  • packages/sdk/src/client.ts
  • packages/sdk/src/share/context.ts
  • packages/sdk/src/share/shared-write.ts
  • packages/sdk/src/types.ts
  • tests/sdk-e2e/src/suites/write-chain-rotation.test.ts

Comment thread packages/sdk-core/src/__tests__/rotation/write-body-reseal.test.ts
Comment thread packages/sdk-core/src/__tests__/rotation/write-revocation.test.ts Outdated
Comment thread packages/sdk-core/src/rotation/engine.ts
Comment thread packages/sdk-core/src/rotation/engine.ts
Comment thread packages/sdk-core/src/rotation/engine.ts Outdated
Comment thread packages/sdk/src/bin/index.ts Outdated
Comment thread packages/sdk/src/share/shared-write.ts
Comment thread packages/sdk/src/share/shared-write.ts Outdated
Comment thread packages/sdk/src/share/shared-write.ts Outdated
Comment thread tests/sdk-e2e/src/suites/write-chain-rotation.test.ts Outdated
FSM1 and others added 10 commits June 30, 2026 13:30
WriteChildRef.childId is a UUID minted at creation time; filtering by
params.itemId (an IPNS name) never matched, leaving a stale WriteChildRef
that later crashed rotateWriteFromNode.

Add childNodeId param to deleteFromSharedFolder (mirroring moveInSharedFolder)
so read-body is filtered by ipnsName and write-body by UUID.  Also extend
updateSharedFile with optional originalCreatedAt / originalVersions so callers
can preserve immutable metadata; Phase-68 will supply prior values.

Update client.deleteFromSharedFolder signature to thread childNodeId through.
Remove the misleading "childId === ipnsName" module-level comment.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add a test that seeds the parent write-body with a WriteChildRef, calls
deleteFromSharedFolder with both itemId and childNodeId, then unseals the
republished parent envelope and asserts writeBody.writeChildren is empty.

Extend buildSealedParent to accept optional writeChildren so tests can
seed real write-body state.  Update existing delete tests to supply
the new required childNodeId param.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
#6 fail closed when unsealed node has no recoverable write body, preventing
a read-only node from being promoted to write-capable without key proof.

#2+#7 zero each child newWriteKey immediately after sealChildWriteKey (inline
in the map callback) and add a post-Promise.all defensive sweep for any
unclaimed child results — D-09 terminal-owner rule.

#8 check createAndPublishIpnsRecord return value success flag before firing
tombstones or grant mutations so a non-throwing rejection cannot leave the
write plane inconsistent.

#9 assert rootResult.nodeId matches caller-supplied rootNodeId before grant
mutations to prevent rewrapping grants for a different node.

#10 wrap tombstone and grant callbacks in try/finally so the root write key
is zeroed even when a callback throws — D-09 terminal-owner rule.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
#4 remove swallowed .catch handlers in write-body-reseal green-path tests —
RED-phase workaround is no longer needed; let failures surface.

#5 return fresh Uint8Array copies from crypto mocks in write-revocation tests
so engine fill(0) zeroisation does not mutate shared fixture constants and
pollute subsequent tests.

#5 same treatment for generateRandomBytes write key mocks.

Test 8 in write-revocation now captures fresh copies of sealChildWriteKey
arguments via a custom mock implementation, since the engine zeroes the child
write key after sealing.

#20 zero the minted IPNS private key in publishWriteCapableNode e2e helper
after publish in a finally block, narrowing the return type to only ipnsName.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- FIX #16 restoreFromBin: check target folder for existing child by
  ipnsName before appending restoredItem so a retry after
  saveBinMetadata failure cannot duplicate the restored child ref.
- FIX #11 claimInvite: snapshot ephemeralPrivateKey and claimerPublicKey
  buffers before the first await so a caller zeroing the inputs mid-await
  cannot corrupt the subsequent re-wrap; zero the owned copy in finally.
- FIX #12 claimInvite: trim rootNodeId/rootIpnsName before using and
  persisting them so whitespace-padded inputs are not stored as-is.
- test(65): add idempotency test for restoreFromBin, trim-input test
  and snapshot-before-await test for claimInvite; update toBe →
  toStrictEqual for recipientPublicKey (now a snapshotted copy).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…vives JSON round-trip

Entire-Checkpoint: 19c3208fe45e
@FSM1

FSM1 commented Jun 30, 2026

Copy link
Copy Markdown
Owner Author

Good catch on the bin restore path — confirmed and fixed in 92ee561f5.

BinEntry.nodeReadKey is a Uint8Array, and the type doc promised hex wire-encoding, but encryptBinMetadata/decryptBinMetadata JSON.stringify'd the metadata directly — serialising the key to {"0":..,"1":..} and never rebuilding a Uint8Array, so restoreFromBin would have thrown at sealChildReadKey at runtime. As you noted, packages/sdk/src/__tests__/bin.test.ts mocks both helpers, and the real core round-trip test in packages/core/src/__tests__/bin.test.ts predated the field (entry data had nodeReadKey/nodeRef undefined), so the gap was invisible.

Fix:

  • encryptBinMetadata hex-encodes each entry's nodeReadKey before JSON.stringify; decryptBinMetadata rehydrates it to a Uint8Array after parse+validate (invalid hex fails closed → DECRYPTION_FAILED). nodeRef is built from primitives only, so no other field is affected.
  • Added a real (un-mocked) round-trip test asserting nodeReadKey survives as a 32-byte Uint8Array — it fails on the pre-fix {"0":..} object.

Verified: core bin 44/44, full pnpm typecheck green, sdk bin 21/21. The other write-chain/rotation/invite paths are covered by the live D-04 e2e (real crypto + real API).

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 (2)
packages/sdk/src/client.ts (2)

1735-1756: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Persist the newly published envelope.

state.publishedNode is the source for the next write-body unseal, but this path publishes a new published envelope and only adopts children/sequence later. Store the successful envelope as well, or the next shared write can operate from stale writeChildren.

Minimal local fix
         const pubResult = await sdkCore.createAndPublishIpnsRecord({
           ipnsPrivateKey,
           ipnsName,
           metadataCid: ipfsResult.cid,
           sequenceNumber,
           ctx: this.ctx,
         });
+        state.publishedNode = published;
         return { tombstoned: false, newSequenceNumber: pubResult.sequenceNumber };
🤖 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 1735 - 1756, The publish path in the
client state update is only creating the IPFS/IPNS record and returning the new
sequence, but it never persists the newly published envelope, so subsequent
shared writes can reuse stale writeChildren from state.publishedNode. Update the
publish flow in the client state handler around publishNodeFn to store the
successful published envelope into state.publishedNode (and keep it aligned with
the adopted children/sequence) immediately after a successful publish, so later
unseal/write operations use the latest envelope state.

1742-1748: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Honor BYO pinning for shared-write uploads.

addToIpfsFn and publishNodeFn call sdkCore.addToIpfs directly, bypassing pinWithMode; in external mode this routes shared-write encrypted bytes through CipherBox despite the client’s external-pinning contract.

Proposed fix
       addToIpfsFn: async (data) => {
-        const result = await sdkCore.addToIpfs(this.ctx, data);
+        const result = await this.pinWithMode(data, this.ctx);
         return { cid: result.cid };
       },
       publishNodeFn: async ({ published, ipnsName, ipnsPrivateKey, sequenceNumber }) => {
         const bytes = new TextEncoder().encode(JSON.stringify(published));
-        const ipfsResult = await sdkCore.addToIpfs(this.ctx, bytes);
+        const ipfsResult = await this.pinWithMode(bytes, this.ctx);
🤖 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 1742 - 1748, The shared-write upload
path in addToIpfsFn and publishNodeFn is bypassing BYO pinning by calling
sdkCore.addToIpfs directly, which can route external-mode uploads through
CipherBox instead of the client’s pinning contract. Update these code paths to
use the existing pinWithMode flow (or the shared helper it delegates to) so the
current pinning mode is honored before returning the CID / publishing IPNS
state. Keep the fix localized around addToIpfsFn, publishNodeFn, and pinWithMode
in packages/sdk/src/client.ts.
🤖 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 `@packages/core/src/bin/encrypt.ts`:
- Around line 47-53: The fromBinWireForm() rehydration path currently accepts
any hex string and converts it with hexToBytes without verifying the resulting
nodeReadKey length. Update the entry mapping inside fromBinWireForm() to
validate that the decoded Uint8Array is exactly 32 bytes, and reject malformed
input by throwing a clear error before returning metadata. Keep the fix
localized to fromBinWireForm() and the nodeReadKey handling so invalid key
material cannot reach restore/key-sealing code.

In `@packages/sdk/src/client.ts`:
- Around line 1851-1854: `deleteFromSharedFolder` currently accepts
`childNodeId` without enforcing it, so JS callers can omit it and leave the
write-body `WriteChildRef` stale. Add an explicit validation at the start of
`deleteFromSharedFolder` in `Client` to fail fast when `args.childNodeId` is
missing, and prevent the delegate call from proceeding unless both `itemId` and
`childNodeId` are present.

---

Outside diff comments:
In `@packages/sdk/src/client.ts`:
- Around line 1735-1756: The publish path in the client state update is only
creating the IPFS/IPNS record and returning the new sequence, but it never
persists the newly published envelope, so subsequent shared writes can reuse
stale writeChildren from state.publishedNode. Update the publish flow in the
client state handler around publishNodeFn to store the successful published
envelope into state.publishedNode (and keep it aligned with the adopted
children/sequence) immediately after a successful publish, so later unseal/write
operations use the latest envelope state.
- Around line 1742-1748: The shared-write upload path in addToIpfsFn and
publishNodeFn is bypassing BYO pinning by calling sdkCore.addToIpfs directly,
which can route external-mode uploads through CipherBox instead of the client’s
pinning contract. Update these code paths to use the existing pinWithMode flow
(or the shared helper it delegates to) so the current pinning mode is honored
before returning the CID / publishing IPNS state. Keep the fix localized around
addToIpfsFn, publishNodeFn, and pinWithMode in packages/sdk/src/client.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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: a5c05002-6cf7-4a5b-8b5b-179f81541f9c

📥 Commits

Reviewing files that changed from the base of the PR and between 0e7e205 and 92ee561.

📒 Files selected for processing (14)
  • .planning/todos/pending/2026-06-30-phase68-shared-write-context-publishednode-wiring.md
  • packages/core/src/__tests__/bin.test.ts
  • packages/core/src/bin/encrypt.ts
  • packages/sdk-core/src/__tests__/rotation/write-body-reseal.test.ts
  • packages/sdk-core/src/__tests__/rotation/write-revocation.test.ts
  • packages/sdk-core/src/__tests__/share/grant.test.ts
  • packages/sdk-core/src/rotation/engine.ts
  • packages/sdk-core/src/share/grant.ts
  • packages/sdk/src/__tests__/bin.test.ts
  • packages/sdk/src/__tests__/shared-write.test.ts
  • packages/sdk/src/bin/index.ts
  • packages/sdk/src/client.ts
  • packages/sdk/src/share/shared-write.ts
  • tests/sdk-e2e/src/suites/write-chain-rotation.test.ts
✅ Files skipped from review due to trivial changes (1)
  • .planning/todos/pending/2026-06-30-phase68-shared-write-context-publishednode-wiring.md
🚧 Files skipped from review as they are similar to previous changes (9)
  • packages/sdk-core/src/tests/share/grant.test.ts
  • packages/sdk-core/src/tests/rotation/write-body-reseal.test.ts
  • tests/sdk-e2e/src/suites/write-chain-rotation.test.ts
  • packages/sdk-core/src/tests/rotation/write-revocation.test.ts
  • packages/sdk-core/src/share/grant.ts
  • packages/sdk-core/src/rotation/engine.ts
  • packages/sdk/src/tests/shared-write.test.ts
  • packages/sdk/src/bin/index.ts
  • packages/sdk/src/share/shared-write.ts

Comment thread packages/core/src/bin/encrypt.ts
Comment thread packages/sdk/src/client.ts
FSM1 added 2 commits June 30, 2026 14:19
…eFromSharedFolder childNodeId

Entire-Checkpoint: c7f2d3a74cdf
…nor BYO pinning

Entire-Checkpoint: e99a4ebfa963
@FSM1

FSM1 commented Jun 30, 2026

Copy link
Copy Markdown
Owner Author

Addressing the 2 outside-diff-range comments on client.ts:

1. 1742-1748 — Honor BYO pinning for shared-write uploads → fixed in 6874c2603.
addToIpfsFn (encrypted content) now routes through pinWithMode, exactly like the non-shared upload path (pinWithMode at L784/966), so shared-write content never goes through CipherBox when the user opted external.

I deliberately left publishNodeFn's node-metadata upload on sdkCore.addToIpfs: that blob is the IPNS resolution target and must stay reachable by CipherBox's IPNS relay — routing it through pinWithMode in external mode would send it only to the user's node and break IPNS resolution for other clients. This mirrors the non-shared metadata-publish path (content → pinWithMode, metadata → addToIpfs).

2. 1735-1756 — Persist the newly published envelope → deferred to Phase 68 (tracked).
Correct that state.publishedNode isn't refreshed after a write. The minimal state.publishedNode = published is order-fragile (multi-publish ops like createSharedSubfolder call publishNodeFn per node, so the last call — not necessarily the parent — would win). The robust fix is to have the shared-write helper return the parent envelope and persist it in adoptSharedFolderResult alongside children/sequence. This is part of the deferred Phase-68 client wiring (the path is dormant today — the web seed uses a placeholder PublishedNode), tracked in .planning/todos/pending/2026-06-30-phase68-shared-write-context-publishednode-wiring.md (covers PR threads #9/#15/#17 + this one).

Comment thread packages/sdk/src/client.ts
…hNodeFn

The shared-write publishNodeFn closure ignored pubResult.success, so a 2xx
relay response carrying success:false would proceed as if the node were
committed — leaving the parent's SealedChildRef pointing at an IPNS name that
was never published. Guard on success and throw, mirroring rotateWriteSubtree.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Entire-Checkpoint: 9e04bdf55b05
Comment thread packages/sdk/src/bin/index.ts
…ration

addToBin reconstructed the child-readkey AAD from publishedNode.generation,
sourced from an independent IPNS resolve of the child. A stale-CID serve makes
that diverge from childRef.generation (the parent mirror the readKeySealed blob
was sealed under), so unsealChildReadKey fails GCM auth closed even when the
parent folder state is current. Pass childRef.generation per the §2.6
generation-source rule, matching moveItem and navigate.ts. Also capture
nodeRef.generation from childRef so restoreFromBin re-seals under the same
generation as the unsealed key.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Entire-Checkpoint: 1af3242d5c34
@FSM1 FSM1 merged commit d81c1b4 into main Jun 30, 2026
29 checks passed
@FSM1 FSM1 deleted the feat/sdk-write-chain-bin-re-link-and-invite-claim branch June 30, 2026 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release:core:feat Minor version bump (new feature) for core release:sdk:feat Minor version bump (new feature) for sdk release:sdk-core:feat Minor version bump (new feature) for sdk-core release:tee-worker:fix Patch version bump (bug fix) for tee-worker release:web:fix Patch version bump (bug fix) for web

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant