fix: IPNS signed-record verify coverage chokepoint and non-CAS sequence gate#544
Merged
Conversation
Entire-Checkpoint: 46695c75b0c7
Entire-Checkpoint: 88f4300e1d31
Entire-Checkpoint: 7c85d5bd147c
Entire-Checkpoint: cdc7df5fa516
Entire-Checkpoint: 473de6ab6524
Entire-Checkpoint: a31b672a7238
Entire-Checkpoint: 1b8650057230
- Wave-0 probe: parseCborData not importable from 'ipns' main entry - Fallback: import decode from 'cborg' directly - cborg 4.5.8 matches ipns transitive version Entire-Checkpoint: e76f5aaacd8e
- Round-trip with build_cbor_data for known value and sequences - Rejects non-map CBOR, missing Value key, missing Sequence key - Rejects negative sequence (u64::try_from fails) Entire-Checkpoint: 8f70cb7f36a7
- Inverse of build_cbor_data: decodes CBOR map to extract Value and Sequence - Matches CborValue::Bytes for Value key (not Text, per build_cbor_data) - Converts CborValue::Integer via i128->u64 two-step (ciborium Integer newtype) - Returns CborEncodingFailed for missing fields, non-map, bad UTF-8, negative seq - All 6 unit tests pass Entire-Checkpoint: cb7bcf6e8475
…e/src/verify.rs - New verify.rs with VerifyError enum and VerifiedResolve struct - bind_verified pure helper drives unit tests without network dependency - resolve_ipns_verified wraps resolve_ipns + verify_ipns_resolve_signature + bind_verified - D-08: VerifiedResolve.cid is the embedded value with /ipfs/ stripped - D-07: embedded seq must match response sequence_number - D-04: Legacy variant for all-absent records - V7: error messages reference ipns_name/cid only, no signature bytes - All 5 bind_verified unit tests pass Entire-Checkpoint: 4b30e061e435
…fied - events.rs spawn_metadata_refresh: D-02 scoped fail on Invalid - fs.rs FilePointer resolve: D-02 scoped fail on Invalid - metadata.rs remote_merge, bin entry, file-metadata resolve: D-02 scoped - publish.rs resolve_sequence soft: Invalid falls back to cache - publish.rs resolve_sequence_strict: Invalid returns Err - replay.rs resolve_folder_key: D-03 hard fail-closed preserved - replay.rs fetch_merge_publish_parent: Invalid retains journal entry - All Legacy paths warn and re-resolve with DB CID per D-04 Entire-Checkpoint: 43681d634a9b
…veIpnsRecord RED: three tests verify that D-07/D-08 binding mismatches throw; positive and legacy-not-bound tests also added. Binding not yet implemented. Entire-Checkpoint: 2cc9dde893b6
After Ed25519 signature verification, decode the signed CBOR `data` field via cborg and compare embedded Value and Sequence against the response cid and sequenceNumber. Mismatch throws with a descriptive error that propagates through the catch block. Legacy records remain unbound per D-04. Mirrors the Rust bind_verified posture in verify.rs for D-05 convergence. Entire-Checkpoint: 495bb45f65df
Entire-Checkpoint: 02fce3c77d02
…onfig Cargo.lock reflects ciborium added to cipherbox-fuse for verify.rs test module. Entire-Checkpoint: 572bed975484
…FolderIpns - Red tests covering all 9 D-09 behavior cases: first-publish seq>1 rejected, seq 0 and 1 accepted, idempotent embedded=N (no DB increment, latestCid updated), forward embedded=N+1 (increment), rollback embedded<N (400), wild-jump embedded>N+1 (400), unconditional without CAS, CAS-409 precedence over D-09 (conflict wins) - Gate is NOT yet implemented; all 5 new D-09 reject tests fail Entire-Checkpoint: ac0b25833ab6
…psertFolderIpns Replace the CAS-gated S1 sequence block with the D-09 unconditional gate: - First publish (no existing row): allows embedded seq 0 or 1; rejects higher values (wedge-poison prevention, T-58-08) - Existing row at N, embedded=N: idempotent republish; skips DB increment but still updates latestCid and signedRecord (TEE 6-hour re-sign path) - Existing row at N, embedded=N+1: forward publish; increments DB sequence - Existing row at N, embedded<N: rollback rejected (T-58-09) - Existing row at N, embedded>N+1: wild-jump rejected (T-58-10) - CAS 409 check stays before D-09 gate so concurrent-modification keeps its 409 Update existing tests to supply D-09-valid sequences for existing-row scenarios (forward = DB_seq+1); add isIdempotentRepublish flag to guard the DB increment. Entire-Checkpoint: 0d07a4a79846
Entire-Checkpoint: 82c79f102e57
- Delete local verifyIpnsSignature and resolveIpnsRecord verify body - Import resolveIpnsRecord from @cipherbox/sdk-core with apiAxios/apiUrl/getAccessToken ctx - Remove now-unused imports: verifyEd25519, concatBytes, deriveIpnsName, IPNS_SIGNATURE_PREFIX, ipnsControllerResolveRecord, logger - Web inherits 58-01 CBOR binding and partial-fields fail-closed check automatically - All 14 callers unchanged (same public signature) Entire-Checkpoint: d3676149211a
Entire-Checkpoint: 9b5173483ea6
- scripts/gen-ipns-verify-vectors.mjs: one-shot Node ESM generator using @noble/ed25519 and cborg to produce deterministic signed vectors - tests/vectors/ipns/verify.json: 7-case shared cross-language fixture covering valid, tampered-sig, name-mismatch, cid-swapped, seq-mismatch, partial-fields, and legacy-absent (D-11) - cid-swapped and seq-mismatch carry genuine Ed25519 signatures over their mismatching CBOR data so only the binding check (not the sig check) fails Entire-Checkpoint: c5294ad08b17
- crates/fuse/tests/ipns_verify_vectors.rs: loads shared verify.json and asserts all 7 cases using the real verify_ipns_resolve_signature (cipherbox-api-client) and decode_ipns_cbor_data (cipherbox-core) - Located in crates/fuse to avoid the dependency cycle: api-client and core both depend on crypto, so crypto cannot dev-depend on either - cid-swapped and seq-mismatch assert invalid via the binding path (their Ed25519 sigs are valid; only the binding check rejects them) - cargo test -p cipherbox-fuse --test ipns_verify_vectors passes all 7 Entire-Checkpoint: fa0b1b811df9
- packages/sdk-core/src/__tests__/ipns.test.ts: append D-11/D-12 describe block loading verify.json and asserting all 7 cases via resolveIpnsRecord - cid-swapped and seq-mismatch cases pass REAL fixture data bytes through cborDecode so the binding layer is exercised against genuine vector bytes - partial-fields asserts fail-closed before verifyEd25519 is called - legacy-absent asserts signatureVerified=false (D-04 path) - All 26 ipns tests pass; JSON import resolves via resolveJsonModule=true Entire-Checkpoint: 3c64c98730bc
- 58-04-SUMMARY.md: documents generator approach, Rust consumer location rationale, JS fixture import path, and D-12 CI gate compliance - STATE.md: advance completed_plans to 186, add P04 metrics row - ROADMAP.md: mark 58-04 plan checkbox complete Entire-Checkpoint: 41b69f9f0e44
Entire-Checkpoint: 32a25944bcaf
…IpnsRecord Entire-Checkpoint: f5f9bdf78710
…n bind_verified Entire-Checkpoint: b7240dae8c2e
Entire-Checkpoint: db91736a6e0c
Entire-Checkpoint: 7b22b6f3c1cc
|
Warning Review limit reached
More reviews will be available in 55 minutes and 23 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (13)
WalkthroughPhase 58 adds end-to-end IPNS signature verification coverage: a new Rust CBOR decode helper and verified resolve chokepoint route all nine FUSE resolve sites through fail-closed binding checks; the API's ChangesPhase 58 IPNS Signature-Verify Coverage
Sequence Diagram(s)sequenceDiagram
participant FUSESite as FUSE Call Site
participant resolve_ipns_verified
participant ApiClient
participant bind_verified
participant decode_ipns_cbor_data
rect rgba(70, 130, 180, 0.5)
note over FUSESite,decode_ipns_cbor_data: Verified resolve path (events/fs/metadata/publish/replay)
FUSESite->>resolve_ipns_verified: resolve_ipns_verified(api, ipns_name)
resolve_ipns_verified->>ApiClient: resolve_ipns(ipns_name)
ApiClient-->>resolve_ipns_verified: IpnsResolveResponse
resolve_ipns_verified->>ApiClient: verify_ipns_resolve_signature(response)
ApiClient-->>resolve_ipns_verified: verdict
resolve_ipns_verified->>bind_verified: verdict + response
bind_verified->>decode_ipns_cbor_data: base64-decoded CBOR bytes
decode_ipns_cbor_data-->>bind_verified: embedded Value and Sequence
bind_verified-->>resolve_ipns_verified: VerifiedResolve or VerifyError
resolve_ipns_verified-->>FUSESite: Ok(VerifiedResolve) or Err(Legacy/Invalid/Api)
end
rect rgba(180, 100, 60, 0.5)
note over FUSESite: Error dispatch per site
FUSESite-->>FUSESite: Legacy → warn + fallback raw resolve
FUSESite-->>FUSESite: Invalid → fail-closed error (hard-fail for folder-key)
FUSESite-->>FUSESite: Api → propagate error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
release:cipherbox-core:feat
Contributor
Release Preview
Cascade Details
|
Rename scripts/gen-ipns-verify-vectors.mjs → .ts with strict TypeScript annotations (VectorEntry interface, typed helpers, async ed25519 API), drop the unused createIpnsRecord binding, update the run instruction to npx tsx, add the file to tsconfig.scripts.json include, and update the Rust doc comment and planning todo to reference the new filename. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Entire-Checkpoint: c477ad416aa9
Extend the eslint files glob from *.{js,mjs,cjs,ts,tsx} to include mts
and cts, and widen the lint-staged key to match the same set of
extensions so pre-commit linting fires on all TypeScript module variants.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Entire-Checkpoint: df8ebf8e98fc
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@crates/core/src/ipns.rs`:
- Around line 89-99: The loop currently allows duplicate known keys like "Value"
and "Sequence" to be silently overwritten by the last matching entry. Add
validation to detect and reject duplicates: before assigning to value_bytes when
matching the "Value" key, check if value_bytes is already Some and return an
error if so; similarly, before assigning to sequence when matching the
"Sequence" key, check if sequence is already Some and return an error if so.
This ensures that ambiguous or duplicate entries fail closed rather than
silently accepting the last occurrence.
In `@crates/fuse/src/verify.rs`:
- Around line 21-23: The VerifyError::Legacy variant currently carries no data,
forcing callers to re-resolve IPNS when encountering legacy records. Modify the
Legacy variant to include the raw CID and sequence fields from the response so
the data can be reused. Update all locations where VerifyError::Legacy is
constructed (likely around lines 66-67 and 130-131) to pass these fields when
creating the error. Then update all match branches handling VerifyError::Legacy
to use the carried CID and sequence directly instead of calling
cipherbox_api_client::ipns::resolve_ipns again, eliminating the redundant second
resolution.
In `@packages/sdk-core/src/ipns/index.ts`:
- Around line 262-270: Add explicit type validation for the embeddedSeq variable
before BigInt coercion. Instead of relying only on the typeof 'bigint' check and
then using BigInt(embeddedSeq as number), first validate that embeddedSeq is
either a bigint type or a safe integer using Number.isSafeInteger() for number
types. Throw an error if embeddedSeq is a string, boolean, or unsafe number.
After creating embeddedSeqBigInt, add a bounds check to ensure it falls within
the valid u64 range (0n to (1n << 64n) - 1n) to match Rust's validation and
prevent signature validation bypass from malformed CBOR records.
In `@scripts/gen-ipns-verify-vectors.mjs`:
- Line 67: Remove the unused `createIpnsRecord` variable from the destructuring
assignment in the import statement on the line where both createIpnsRecord and
deriveIpnsName are imported from the pathToFileURL(CORE_PATH) module. Keep only
deriveIpnsName in the destructuring since createIpnsRecord is never referenced
in the script and is causing the linting error.
🪄 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: f5d9ff4f-c307-49c9-a419-40f2ff610bcd
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!**/pnpm-lock.yaml
📒 Files selected for processing (43)
.planning/PROJECT.md.planning/ROADMAP.md.planning/STATE.md.planning/config.json.planning/phases/58-ipns-signature-verify-coverage/58-01-PLAN.md.planning/phases/58-ipns-signature-verify-coverage/58-01-SUMMARY.md.planning/phases/58-ipns-signature-verify-coverage/58-02-PLAN.md.planning/phases/58-ipns-signature-verify-coverage/58-02-SUMMARY.md.planning/phases/58-ipns-signature-verify-coverage/58-03-PLAN.md.planning/phases/58-ipns-signature-verify-coverage/58-03-SUMMARY.md.planning/phases/58-ipns-signature-verify-coverage/58-04-PLAN.md.planning/phases/58-ipns-signature-verify-coverage/58-04-SUMMARY.md.planning/phases/58-ipns-signature-verify-coverage/58-CONTEXT.md.planning/phases/58-ipns-signature-verify-coverage/58-DISCUSSION-LOG.md.planning/phases/58-ipns-signature-verify-coverage/58-PATTERNS.md.planning/phases/58-ipns-signature-verify-coverage/58-RESEARCH.md.planning/phases/58-ipns-signature-verify-coverage/58-REVIEW.md.planning/phases/58-ipns-signature-verify-coverage/58-SECURITY.md.planning/phases/58-ipns-signature-verify-coverage/58-VALIDATION.md.planning/phases/58-ipns-signature-verify-coverage/58-VERIFICATION.md.planning/todos/completed/2026-06-20-ipns-publish-validate-embedded-sequence-without-cas.md.planning/todos/completed/2026-06-20-ipns-resolve-verify-coverage-and-web-sdk-dedup.md.planning/todos/pending/2026-06-22-desktop-resolve-ipns-verified-coverage.md.planning/todos/pending/2026-06-22-phase58-simplify-cleanup.mdapps/api/src/ipns/ipns.service.spec.tsapps/api/src/ipns/ipns.service.tsapps/web/src/services/ipns.service.tscrates/core/src/ipns.rscrates/fuse/Cargo.tomlcrates/fuse/src/events.rscrates/fuse/src/fs.rscrates/fuse/src/lib.rscrates/fuse/src/metadata.rscrates/fuse/src/publish.rscrates/fuse/src/replay.rscrates/fuse/src/verify.rscrates/fuse/tests/ipns_verify_vectors.rspackages/sdk-core/package.jsonpackages/sdk-core/src/__tests__/ipns.test.tspackages/sdk-core/src/ipns/index.tsrelease-please-config.jsonscripts/gen-ipns-verify-vectors.mjstests/vectors/ipns/verify.json
Greptile SummaryThis PR completes the IPNS signature-verify coverage story by adding a CBOR cid/sequence binding check (D-07/D-08), introducing a single
Confidence Score: 5/5Safe to merge; the core CBOR-binding and D-09 sequence-gate logic is correct on both language paths and backed by 8 shared cross-language vectors. The three open items carried over from the prior review are captured in deferred todos, scoped as non-regressions from baseline, and were already present before this PR. The new code — CBOR binding, D-09 gate, web dedup — is correctly implemented and well-tested. crates/fuse/src/events.rs and the other five FUSE resolve sites still make a second network call in the Legacy fallback arm; tracked in the pending todo. Important Files Changed
Reviews (3): Last reviewed commit: "docs: capture IPNS first-publish sequenc..." | Re-trigger Greptile |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #544 +/- ##
==========================================
+ Coverage 65.94% 68.22% +2.27%
==========================================
Files 148 177 +29
Lines 11473 20331 +8858
Branches 1300 1302 +2
==========================================
+ Hits 7566 13871 +6305
- Misses 3662 6216 +2554
+ Partials 245 244 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
…code Entire-Checkpoint: fc4af95fa986
…nding Entire-Checkpoint: cbf3ef1f3f62
Entire-Checkpoint: 51dc77acae25
The Phase 58 D-07 sequence binding compared the signed CBOR embedded Sequence against the API response sequenceNumber with strict equality. That is stricter than the publish-side D-09 gate it mirrors: D-09 accepts an embedded sequence of 0 OR 1 on first publish and unconditionally stores DB sequenceNumber=1. The Rust/FUSE publish paths embed the IPNS-native 0 (next_file_publish_sequence, first child-folder publish) while the TS SDK embeds 1 — both legitimate. So a resolved first-generation record carries embedded=0 with response sequenceNumber=1, which the strict binding wrongly rejected as a tamper. This broke the desktop E2E round-trip on all platforms (FilePointer verify and FUSE replay self-verify) with "IPNS sequence binding mismatch: embedded=0, response seq=1". The folder SDK E2E path passed because folders embed 1 (== DB). Relax both the Rust (verify.rs bind_verified) and JS (sdk-core resolveIpnsRecord) bindings to accept the single documented skew (resp_seq == 1 && embedded == 0); strict equality is still required for every other sequence, and the cid binding stays strict. bind_verified now returns the DB-authoritative resp_seq (not embedded_seq) so downstream forward math (next publish = seq + 1) computes 2, not an idempotent re-sign at 1. Adds a first-publish-skew cross-language vector (case 8) plus Rust and sdk-core unit tests, including negative scoping tests (embedded=0 with resp>=2 still rejected). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Entire-Checkpoint: e0c89e0b5ef1
Records the publish-path inconsistency surfaced by the #544 resolve-binding hotfix (FUSE embeds Sequence=0 on first publish, TS SDK embeds 1, API stores DB=1) and the open question of whether the TEE re-sign path trips the D-09 rollback gate for embed-0 records. Deferred as future hardening. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Entire-Checkpoint: 2d090124fa17
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 58 — IPNS Signature-Verify Coverage (HARD-09)
Finishes the IPNS signed-record verification story left after Phase 51 /
#529: binds every resolved record to its signed CBOR-embedded CID/sequence, folds verification into a single safe-by-default Rust chokepoint, validates the embedded publish sequence even when CAS is omitted, de-duplicates the web vs sdk-core resolve copies, and adds shared cross-language verify vectors.What changed
crates/core/src/ipns.rsgainsdecode_ipns_cbor_data; both the Rust verify path (crates/fuse/src/verify.rs) and sdk-coreresolveIpnsRecordnow decode the signed CBORdataand reject when the embeddedValue(cid)/Sequencedoes not match the responsecid/sequenceNumber. A mismatch is classified identically to an invalid signature (fail-closed). Closes the swap gap on both languages.resolve_ipns_verifiedchokepoint (D-01/D-02) — newcrates/fuse/src/verify.rswrapper; all 9 FUSE resolve sites (events, fs, publish ×2, metadata ×3, replay parent-merge + folder-key) route through it. Per-operation scoped fail-closed (a verify failure refuses the unverified CID but fails only that operation; the 30s poll self-heals). The folder-key descent keeps its existing hard fail-closed posture (D-03). Legacy all-absent records still allowed and flagged (D-04).apps/api/src/ipns/ipns.service.tsupsertFolderIpnsnow validates the embedded sequence unconditionally (no longer gated onexpectedSequenceNumber): first publish allows{0,1}, forwardN+1increments,embedded == Nis an idempotent re-sign (no DB increment butlatestCid/signedRecordupdated — protects the TEE 6-hour path), rollback< Nand wild-jump> N+1are rejected. Every non-CAS publish path was enumerated and provenDB+1-or-idempotent first.apps/web/src/services/ipns.service.tsdeletes its localverifyIpnsSignature/resolveIpnsRecordand delegates to@cipherbox/sdk-core(ctx axios injection +withPerfpreserved). The web now inherits the CBOR binding it previously lacked.tests/vectors/ipns/verify.json(7 cases: valid, tampered-sig, name-mismatch, cid-swapped, seq-mismatch, partial-fields, legacy-absent) consumed by bothcrates/fuse/tests/ipns_verify_vectors.rs(cargo) and the sdk-core vitest suite. Rust↔JS byte-construction drift now fails an already-required CI check.Verification
cargo test(core / fuse + vectors)58-SECURITY.md): SECURED, 19/19 threats closed (ASVS L1).58-VALIDATION.md): compliant, 0 gaps, 35-row coverage map.Trust model
DB CID remains the authoritative trust root; signature verification is defense-in-depth (Medium). That is why resolve-side failures are scoped (not whole-mount) and legacy records stay allowed.
Deferred (captured as todos, not in this PR)
resolve_ipnssites inapps/desktop/src-tauri/(prepopulate.rs, vault.rs) through the verified wrapper — out of scope for this phase, no regression from baseline.signature_verifiedfield, a test-string clarity rename, a deadjournal_entrybranch, unused fixture key fields).🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes