feat(api): share item-name backfill endpoint#505
Conversation
… shares Phase 48 REQ-4 left a gap: itemNameEncrypted was only accepted on share CREATE paths, so the web lazy-backfill (decision A2) could re-wrap legacy plaintext itemName but had nowhere to persist it. Add PATCH /shares/:shareId/item-name (sharer-only, stores client ciphertext as-is, server never encrypts), regenerate the typed client, and wire+re-enable backfillSentShareItemNames in the web share service. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Entire-Checkpoint: b886f47dabf9
Phase 47 shipped via PR #494 but its 5 SUMMARY.md files were never written, tripping a false-positive resume-incomplete gate. Reconstruct them from the plans and merged source. Add the missing 48-VERIFICATION.md (3.5/4; REQ-4 legacy backfill now resolved by the item-name endpoint). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Entire-Checkpoint: e0c3a8080f74
Advance STATE.md now that phase 48 has all summaries + verification and the REQ-4 itemName backfill gap is resolved. 33/33 phases, 146/146 plans. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Entire-Checkpoint: 027e67269db2
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds a new ChangesitemNameEncrypted Backfill API and Web Integration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Release Preview
Cascade Details
|
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Entire-Checkpoint: f7d244dcc682
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #505 +/- ##
==========================================
+ Coverage 63.90% 63.92% +0.02%
==========================================
Files 141 141
Lines 10863 10875 +12
Branches 1205 1209 +4
==========================================
+ Hits 6942 6952 +10
Misses 3678 3678
- Partials 243 245 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/services/share.service.ts (1)
199-210:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winClear transient plaintext bytes after wrapping the item name.
Line 204 creates plaintext bytes (
TextEncoder().encode(share.itemName)) but they are never zeroed after use, leaving sensitive metadata in memory longer than necessary.Proposed fix
- const wrapped = await wrapKey(new TextEncoder().encode(share.itemName), recipientPubKey); - const itemNameEncrypted = bytesToHex(wrapped); - - // Persist the re-wrapped ciphertext for this legacy row. Only the sharer - // may update it; the server stores the ciphertext as-is. - await sharesControllerUpdateShareItemName(share.shareId, { itemNameEncrypted }); - backfilled += 1; + const plaintextNameBytes = new TextEncoder().encode(share.itemName); + let wrapped: Uint8Array | null = null; + try { + wrapped = await wrapKey(plaintextNameBytes, recipientPubKey); + const itemNameEncrypted = bytesToHex(wrapped); + + // Persist the re-wrapped ciphertext for this legacy row. Only the sharer + // may update it; the server stores the ciphertext as-is. + await sharesControllerUpdateShareItemName(share.shareId, { itemNameEncrypted }); + backfilled += 1; + } finally { + plaintextNameBytes.fill(0); + wrapped?.fill(0); + }As per coding guidelines, TypeScript crypto paths must clear sensitive data from memory after use.
🤖 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 `@apps/web/src/services/share.service.ts` around lines 199 - 210, The plaintext bytes created by TextEncoder().encode(share.itemName) are not being cleared from memory after the wrapKey call completes, leaving sensitive metadata exposed. Store the result of new TextEncoder().encode(share.itemName) in a variable before passing it to wrapKey, then after the wrapKey call returns and itemNameEncrypted is set, explicitly zero out the plaintext bytes variable by calling .fill(0) on it to clear the sensitive data from memory. This ensures the item name plaintext does not remain in memory longer than necessary.Source: Coding guidelines
🤖 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.
Outside diff comments:
In `@apps/web/src/services/share.service.ts`:
- Around line 199-210: The plaintext bytes created by
TextEncoder().encode(share.itemName) are not being cleared from memory after the
wrapKey call completes, leaving sensitive metadata exposed. Store the result of
new TextEncoder().encode(share.itemName) in a variable before passing it to
wrapKey, then after the wrapKey call returns and itemNameEncrypted is set,
explicitly zero out the plaintext bytes variable by calling .fill(0) on it to
clear the sensitive data from memory. This ensures the item name plaintext does
not remain in memory longer than necessary.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ab2b2e0a-d434-47b9-ac4e-b78c61531c16
⛔ Files ignored due to path filters (131)
packages/api-client/openapi.jsonis excluded by!packages/api-client/**packages/api-client/src/generated/auth/auth.tsis excluded by!**/generated/**,!**/generated/**,!packages/api-client/**packages/api-client/src/generated/device-approval/device-approval.tsis excluded by!**/generated/**,!**/generated/**,!packages/api-client/**packages/api-client/src/generated/health/health.tsis excluded by!**/generated/**,!**/generated/**,!packages/api-client/**packages/api-client/src/generated/identity/identity.tsis excluded by!**/generated/**,!**/generated/**,!packages/api-client/**packages/api-client/src/generated/invites/invites.tsis excluded by!**/generated/**,!**/generated/**,!packages/api-client/**packages/api-client/src/generated/ipfs/ipfs.tsis excluded by!**/generated/**,!**/generated/**,!packages/api-client/**packages/api-client/src/generated/ipns/ipns.tsis excluded by!**/generated/**,!**/generated/**,!packages/api-client/**packages/api-client/src/generated/root/root.tsis excluded by!**/generated/**,!**/generated/**,!packages/api-client/**packages/api-client/src/generated/share-invites/share-invites.tsis excluded by!**/generated/**,!**/generated/**,!packages/api-client/**packages/api-client/src/generated/shares/shares.tsis excluded by!**/generated/**,!**/generated/**,!packages/api-client/**packages/api-client/src/generated/tee/tee.tsis excluded by!**/generated/**,!**/generated/**,!packages/api-client/**packages/api-client/src/generated/vault/vault.tsis excluded by!**/generated/**,!**/generated/**,!packages/api-client/**packages/api-client/src/models/addShareKeysDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/authMethodResponseDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/authMethodResponseDtoType.tsis excluded by!packages/api-client/**packages/api-client/src/models/batchPublishIpnsDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/batchPublishIpnsResponseDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/batchUnenrollIpnsDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/batchUnenrollIpnsResponseDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/childKeyDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/childKeyDtoKeyType.tsis excluded by!packages/api-client/**packages/api-client/src/models/claimChildKeyDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/claimChildKeyDtoKeyType.tsis excluded by!packages/api-client/**packages/api-client/src/models/claimInviteDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/claimInviteResponseDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/connectionTestRequestDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/connectionTestResponseDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/connectionTestResponseDtoProtocol.tsis excluded by!packages/api-client/**packages/api-client/src/models/createApprovalDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/createInviteDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/createInviteDtoItemType.tsis excluded by!packages/api-client/**packages/api-client/src/models/createShareDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/createShareDtoItemType.tsis excluded by!packages/api-client/**packages/api-client/src/models/createShareDtoPermission.tsis excluded by!packages/api-client/**packages/api-client/src/models/createShareResponseDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/createShareResponseDtoItemType.tsis excluded by!packages/api-client/**packages/api-client/src/models/createShareResponseDtoPermission.tsis excluded by!packages/api-client/**packages/api-client/src/models/deleteAccountDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/deleteAccountResponseDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/desktopRefreshDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/deviceApprovalControllerCreateRequest201.tsis excluded by!packages/api-client/**packages/api-client/src/models/deviceApprovalControllerGetPending200Item.tsis excluded by!packages/api-client/**packages/api-client/src/models/deviceApprovalControllerGetStatus200.tsis excluded by!packages/api-client/**packages/api-client/src/models/deviceApprovalControllerGetStatus200Status.tsis excluded by!packages/api-client/**packages/api-client/src/models/googleLoginDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/googleLoginDtoIntent.tsis excluded by!packages/api-client/**packages/api-client/src/models/healthControllerCheck200.tsis excluded by!packages/api-client/**packages/api-client/src/models/healthControllerCheck200Details.tsis excluded by!packages/api-client/**packages/api-client/src/models/healthControllerCheck200DetailsDatabase.tsis excluded by!packages/api-client/**packages/api-client/src/models/healthControllerCheck200Error.tsis excluded by!packages/api-client/**packages/api-client/src/models/healthControllerCheck200Info.tsis excluded by!packages/api-client/**packages/api-client/src/models/healthControllerCheck200InfoDatabase.tsis excluded by!packages/api-client/**packages/api-client/src/models/identityControllerGetJwks200.tsis excluded by!packages/api-client/**packages/api-client/src/models/identityControllerGetJwks200KeysItem.tsis excluded by!packages/api-client/**packages/api-client/src/models/identityControllerGetWalletNonce200.tsis excluded by!packages/api-client/**packages/api-client/src/models/identityTokenResponseDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/index.tsis excluded by!packages/api-client/**packages/api-client/src/models/initVaultDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/inviteChildKeyDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/inviteChildKeyDtoKeyType.tsis excluded by!packages/api-client/**packages/api-client/src/models/inviteDataResponseDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/inviteDataResponseDtoItemType.tsis excluded by!packages/api-client/**packages/api-client/src/models/inviteDataResponseDtoStatus.tsis excluded by!packages/api-client/**packages/api-client/src/models/inviteResponseDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/inviteResponseDtoItemType.tsis excluded by!packages/api-client/**packages/api-client/src/models/inviteResponseDtoStatus.tsis excluded by!packages/api-client/**packages/api-client/src/models/inviteStatusResponseDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/inviteStatusResponseDtoStatus.tsis excluded by!packages/api-client/**packages/api-client/src/models/ipfsControllerUploadBody.tsis excluded by!packages/api-client/**packages/api-client/src/models/ipnsControllerResolveRecordParams.tsis excluded by!packages/api-client/**packages/api-client/src/models/linkMethodDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/linkMethodDtoLoginType.tsis excluded by!packages/api-client/**packages/api-client/src/models/loginDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/loginDtoLoginType.tsis excluded by!packages/api-client/**packages/api-client/src/models/loginResponseDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/logoutResponseDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/lookupUserResponseDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/paginatedReceivedSharesDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/paginatedSentSharesDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/pendingRotationResponseDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/pendingRotationResponseDtoItemType.tsis excluded by!packages/api-client/**packages/api-client/src/models/publishIpnsDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/publishIpnsEntryDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/publishIpnsEntryDtoRecordType.tsis excluded by!packages/api-client/**packages/api-client/src/models/publishIpnsResponseDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/quotaResponseDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/receivedShareResponseDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/receivedShareResponseDtoItemType.tsis excluded by!packages/api-client/**packages/api-client/src/models/receivedShareResponseDtoPermission.tsis excluded by!packages/api-client/**packages/api-client/src/models/registerCidDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/registerCidResponseDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/resolveIpnsResponseDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/respondApprovalDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/respondApprovalDtoAction.tsis excluded by!packages/api-client/**packages/api-client/src/models/sendOtpDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/sendOtpResponseDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/sentShareResponseDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/sentShareResponseDtoItemType.tsis excluded by!packages/api-client/**packages/api-client/src/models/sentShareResponseDtoPermission.tsis excluded by!packages/api-client/**packages/api-client/src/models/setByoStatusDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/setByoStatusResponseDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/shareInvitesControllerListInvitesParams.tsis excluded by!packages/api-client/**packages/api-client/src/models/shareKeyEntryDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/shareKeyEntryDtoKeyType.tsis excluded by!packages/api-client/**packages/api-client/src/models/shareKeyResponseDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/shareKeyResponseDtoKeyType.tsis excluded by!packages/api-client/**packages/api-client/src/models/sharesControllerGetReceivedSharesParams.tsis excluded by!packages/api-client/**packages/api-client/src/models/sharesControllerGetSentSharesParams.tsis excluded by!packages/api-client/**packages/api-client/src/models/sharesControllerLookupUserParams.tsis excluded by!packages/api-client/**packages/api-client/src/models/teeKeysDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/testLoginDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/testLoginResponseDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/tokenResponseDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/unlinkMethodDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/unlinkMethodResponseDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/unpinDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/unpinResponseDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/updateEncryptedKeyDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/updateItemNameDto.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/**packages/api-client/src/models/uploadResponseDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/vaultConfigResponseDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/vaultExportDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/vaultResponseDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/vaultResponseDtoTeeKeys.tsis excluded by!packages/api-client/**packages/api-client/src/models/verifyOtpDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/verifyOtpDtoIntent.tsis excluded by!packages/api-client/**packages/api-client/src/models/walletVerifyDto.tsis excluded by!packages/api-client/**packages/api-client/src/models/walletVerifyDtoIntent.tsis excluded by!packages/api-client/**
📒 Files selected for processing (15)
.planning/STATE.md.planning/phases/47-sdk-folder-state-publish-consolidation/47-01-SUMMARY.md.planning/phases/47-sdk-folder-state-publish-consolidation/47-02-SUMMARY.md.planning/phases/47-sdk-folder-state-publish-consolidation/47-03-SUMMARY.md.planning/phases/47-sdk-folder-state-publish-consolidation/47-04-SUMMARY.md.planning/phases/47-sdk-folder-state-publish-consolidation/47-05-SUMMARY.md.planning/phases/48-sdk-self-bootstrap-regression-fix-and-shared-folder-metadata/48-VERIFICATION.mdapps/api/src/shares/dto/index.tsapps/api/src/shares/dto/update-item-name.dto.tsapps/api/src/shares/shares.controller.spec.tsapps/api/src/shares/shares.controller.tsapps/api/src/shares/shares.service.spec.tsapps/api/src/shares/shares.service.tsapps/web/src/services/share.service.tsrelease-please-config.json
Two-party spec for Phase 48 REQ-4: seed a legacy share (plaintext itemName, NULL itemNameEncrypted) via POST /shares with the field omitted, trigger the owner-side backfill by uploading a file, then assert the ciphertext is persisted at rest (null -> valid hex != plaintext, longer than plaintext), the recipient still sees the correct name, and the backfill is idempotent. All 7 steps pass against the local full stack. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Entire-Checkpoint: ffdbce177943
Address CodeRabbit review on PR #505: backfillSentShareItemNames encoded the plaintext display name into a Uint8Array that was never zeroed. Wrap the wrap+persist in try/finally and fill(0) the plaintext bytes (and the ciphertext buffer) after use, per the project crypto rule to clear sensitive data from memory. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Entire-Checkpoint: c68bbeb847dd
|
Addressed the CodeRabbit review finding in
Verified: This was CodeRabbit's only actionable item (the rest of the review was file-list/info); it was an 'outside diff range' comment with no resolvable inline thread. |
Summary
Closes the last open item from Phase 48 (REQ-4): encrypting share
itemNameat rest now works for legacy shares too.Phase 48 added at-rest
itemNameEncryptedon share create paths, but left a gap — there was no update endpoint acceptingitemNameEncrypted, so the web client's lazy backfill (decision A2) could re-wrap a legacy plaintextitemNamefor the recipient but had nowhere to persist it (void itemNameEncrypted). This PR adds that endpoint and wires the backfill through it.The server stays zero-knowledge: it never encrypts, only persists client-supplied ciphertext.
Changes
feat(api): share item-name backfill endpointPATCH /shares/:shareId/item-name— sharer-only; stores the client-supplied ECIES ciphertext as-is (mirrors the existingPATCH /shares/:shareId/encrypted-keypattern).UpdateItemNameDto(even-length hex,MaxLength2500) +SharesService.updateShareItemName(load → 404 → sharer-only 403 → persist).@cipherbox/api-client) →sharesControllerUpdateShareItemName.backfillSentShareItemNamesnow persists via the new endpoint, and the backfill pass is re-enabled infetchAllSentShares(best-effort, idempotent viashouldBackfill, fire-and-forget so list loads never block).docs(planning): bookkeepingSUMMARY.mdfiles (Phase 47 shipped via#494but its summaries were never written, which tripped a false-positive resume gate).48-VERIFICATION.mdand advancedSTATE.md— milestone v1.1 now 33/33 phases, 146/146 plans.Requirements Addressed
itemNameat rest, now complete for legacy rows.Verification
apps/apishares suites — 171 passed (added 3 service + 3 controller specs for the new endpoint).tsc --noEmitclean; eslint clean.pnpm api:generate— generated client +openapi.jsonregenerated and committed (satisfies thecheck-api-client.shpre-commit gate).tests/web-e2e/tests/share-itemname-backfill.spec.ts— seeds a legacy share (plaintextitemName, NULLitemNameEncrypted), triggers the owner-side backfill via a file upload, then asserts the persisteditemNameEncryptedis valid ciphertext (null → hex ≠ plaintext, longer than plaintext), the recipient still displays the correct name, and the backfill is idempotent. All 7 steps pass against the local full stack (~52s). Runs in CI viaweb-e2e.yml(playwright test, unfiltered) on main-push or when dispatched against the branch.byteaPostgres read to byte-prove "server never saw plaintext" — web-e2e has no DB client, so the spec'sGET /shares/senthex-≠-plaintext assertion is the in-harness proxy. Closeable later by adding apgutil.Key Decisions
/encrypted-key,/permission,/hideconvention (no generic bulk-update endpoint exists).shouldBackfillidempotence short-circuit.Summary by CodeRabbit
PATCH /shares/:shareId/item-nameendpoint to persist encrypted share item names (ciphertext) for legacy shares.