Skip to content

fix(engine): read a file under the scope it sits in - #1912

Merged
FSM1 merged 3 commits into
mainfrom
fix/1909-read-under-the-files-scope
Sep 19, 2026
Merged

FSM1 merged 3 commits into
mainfrom
fix/1909-read-under-the-files-scope

Conversation

@FSM1

@FSM1 FSM1 commented Sep 19, 2026

Copy link
Copy Markdown
Owner

Result

A file read opened every record under the vault root scope and its read seed. Two cases failed with TrustViolation (adoption gate rejected at stage [unseal]: [seal-open-failed]):

  1. The owner cannot read a file inside a folder the owner shared. This is the more severe case. Measured on main at d1f05ee30: the new test the_owner_reads_a_file_inside_a_folder_it_granted fails on the granting device for a file written before the grant. A grant re-seals the folder interior into the new scope, so every file in a shared folder became unreadable to its owner, on every device.
  2. The recipient cannot read a file below a grafted root. Measured with the new test the_recipient_reads_a_file_below_a_grafted_root on a read grant and on a write grant.

Both tests fail on main and pass with this change.

The change

resolve_versions in crates/engine/src/facade.rs now picks the scope from the position of the file:

  • The scope is the nearest enclosing scope root this session knows (authored_scope_roots: minted, proved, unproved and grafted roots), or the vault root.
  • The read seed comes from scope_read_seed for that scope.
  • The floors come from a new helper, scope_floors, which is the floor_view decision. scope_read_seed now uses the same helper, so the seed and the floors cannot come from two different decisions. A grafted scope is measured in the floor namespace of the granting contact.
  • A node below no known scope root is ContentUnavailable, not a read under the vault root seed (security rule 6). An unproved scope root or a scope with no held seed is also ContentUnavailable.

Every read path goes through resolve_versions: read_content, read_version_content, file_versions, open_content_stream, and the version commands RestoreVersion and DeleteVersion. No new durable record, no seam change, no wire change.

Tests

  • crates/engine/tests/mount_convergence.rs
    • the_owner_reads_a_file_inside_a_folder_it_granted: head content, version list and prior version content, for a file written before the grant and a file written after it, on the granting device and on a second owner device.
    • the_recipient_reads_a_file_below_a_grafted_root: a real recipient session accepts the share through its mailbox and contact book, then reads both files (head, version list, prior version) under a read grant and under a write grant.
  • crates/engine/src/facade.rs a_file_below_no_held_scope_root_is_unavailable: a mutation check. With a vault root fallback in place of the refusal, the read returns the plaintext and the test fails.
  • tests/web-e2e/tests/contact-grant.spec.ts (smoke slice): the owner downloads the file added after the grant and the recipient downloads the same file from the received share. Both compare the bytes.

A file the grantee wrote is not covered here: the facade write guard on main refuses a write below a grafted root. The file is sealed under the same scope seed, so the same path reads it.

Local gates

  • cargo fmt --all --check, cargo clippy --workspace --all-targets -- -D warnings, cargo clippy -p cipherbox-engine -p cipherbox-wasm --target wasm32-unknown-unknown -- -D warnings: pass.
  • cargo test -p cipherbox-engine: pass.
  • pnpm lint:tracker-refs, eslint and prettier on the spec: pass.
  • coderabbit review --agent: review_completed, 0 findings.

Review passes

  • Simplify: one helper owns the floor namespace pick; the stale "subscope reads are a later slice" comment is removed.
  • Security: the scope comes from local gate-passing state, never from a record; a missing seed or scope is availability; the adopter bindings (id, scope, AAD unseal, floors) are unchanged.
  • Crypto-privacy: no primitive, KDF edge or wire change; the vault root seed no longer derives a read key for a record in another scope; the seed clone stays Zeroizing with the adopter as its terminal owner.

Closes #1909.

Body checks / follow-ups filed

  • Issue 1909: the body is accurate. The unmeasured case in it (an own interior scope) is measured above.
  • Filed fix(engine): an epoch-lagged file does not read after a cut #1911: an epoch-lagged file (sealed before a cut and not yet re-sealed) reads as TrustViolation [epoch]: [epoch-below-floor] until a write re-seals it. Measured on main. This PR does not change it.
  • PR 1910 refuses RestoreVersion below a grafted root only because of this defect. After this PR merges, that refusal can be lifted. This PR does not touch it.

Note

Fix engine to read files under their enclosing scope root

  • The child-record read/adoption resolver no longer assumes every node sits under the vault root; it now finds the enclosing root among authored scope roots and the vault root, obtains that scope's read seed and floor view via the new scope_floors helper, and passes the floor view to ChildAdopter.
  • A node with no enclosing held root now returns ContentUnavailable for both content reads and version-history lookups.
  • Adds engine, mount-convergence, and web e2e tests covering pre-grant and post-grant multi-version reads by owners and recipients, plus the unavailable case for unlinked children.
  • Risk: ChildAdopter callers in facade.rs and any out-of-tree consumers that relied on vault-root-only resolution will see ContentUnavailable instead of a best-effort read for nodes below non-held scope roots.

Macroscope summarized d7651dd.

Summary by CodeRabbit

  • Bug Fixes

    • Fixed content reads and version history for files within granted or shared folders.
    • Reads now correctly follow the applicable folder scope, including content created before or after access was granted.
    • Content outside an available scope is now reported as unavailable instead of returning incorrect results.
  • Tests

    • Added coverage for multi-version reads across owner, mounted-device, and recipient access.
    • Improved end-to-end checks to verify downloaded file contents and successful save actions.

resolve_versions opened every file record under the vault root scope and
its read seed. A grant re-seals the granted folder into a scope of its own,
and a file below a grafted root is sealed under the sharer's scope, so both
reads failed with a TrustViolation at the unseal stage.

The read now picks the scope from the file's position (the nearest
enclosing scope root this session knows, or the vault root), its read
seed, and the floor namespace of that scope. A node below no known scope
root is ContentUnavailable. One helper, scope_floors, serves both the seed
cache and the adopter.
@coderabbitai

coderabbitai Bot commented Sep 19, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

Warning

Review limit reached

Next included review available in 49 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: FSM1/cipher-box/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 30947f0c-defd-4e62-832e-43eb2f9a7197

📥 Commits

Reviewing files that changed from the base of the PR and between d7651dd and 91a1982.

📒 Files selected for processing (6)
  • crates/engine/src/facade.rs
  • crates/engine/src/grants/create.rs
  • crates/engine/src/grants/invite_mint.rs
  • crates/engine/src/grants/mod.rs
  • crates/engine/tests/mount_convergence.rs
  • tests/web-e2e/tests/contact-grant.spec.ts

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: FSM1/cipher-box/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: ad840468-6944-4f66-b1ab-5b31941778b4

📥 Commits

Reviewing files that changed from the base of the PR and between d1f05ee and d7651dd.

📒 Files selected for processing (3)
  • crates/engine/src/facade.rs
  • crates/engine/tests/mount_convergence.rs
  • tests/web-e2e/tests/contact-grant.spec.ts

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


Walkthrough

The engine now resolves content reads under the enclosing scope root and its floor store. Tests cover unavailable reads, multi-version reads for owners and recipients, and downloaded byte equality in the contact-grant browser flow.

Changes

Scoped content reads

Layer / File(s) Summary
Scope-aware read resolution
crates/engine/src/facade.rs
scope_read_seed uses scope_floors. Node reads use the enclosing authored or vault scope root. Missing scope data returns EngineError::ContentUnavailable.
Multi-version convergence coverage
crates/engine/tests/mount_convergence.rs
The tests create files with two versions and verify head and prior-version reads for owners, mounted devices, and recipients with Read and Write grants.
Browser download validation
tests/web-e2e/tests/contact-grant.spec.ts
The contact-grant test compares owner and recipient download bytes with the uploaded bytes and checks for vault action errors.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant Reader
  participant Engine
  participant ScopeResolver
  participant ChildAdopter
  Reader->>Engine: request content or version history
  Engine->>ScopeResolver: find enclosing scope root
  ScopeResolver-->>Engine: return scope root
  Engine->>ChildAdopter: resolve using scoped floors
  ChildAdopter-->>Engine: return content or ContentUnavailable
  Engine-->>Reader: return read result
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 72.73% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 2 files. (1 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Issue #1909 requires scoped reads for grafted roots and owner-created interior scopes. resolve_versions now selects the enclosing scope root, obtains that scope's read seed and floor_view, and map…
Out of Scope Changes check ✅ Passed The changes stay within issue #1909. The facade change implements scoped content resolution. The engine tests cover the required grafted-root and interior-scope cases. The web test adds end-to-end rea…
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: fixing file reads so they use the file's applicable scope.
Full details: Docstring Coverage

Explanation

Docstring coverage is 72.73% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 2 files. (1 skipped: 1 too large.)

✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@FSM1
FSM1 marked this pull request as ready for review September 19, 2026 08:50
@FSM1

FSM1 commented Sep 19, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@greptile-apps

greptile-apps Bot commented Sep 19, 2026

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

The PR appears safe to merge; no actionable correctness, security, or repository-rule violations remain.

Summary

This PR fixes file and version reads so records are opened under their nearest known enclosing scope rather than always under the vault root.

  • Shares immediately retain the minted scope’s read seed, allowing owner reads before a boundary walk completes.
  • Scope floor selection now consistently handles owned descendant and grafted scopes.
  • Unknown or unavailable scopes fail closed with ContentUnavailable.
  • Engine and browser tests cover owner and recipient reads across contact grants, invite links, versions, and multiple devices.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Read file or version] --> B[Find nearest enclosing known scope root]
    B -->|No root| C[ContentUnavailable]
    B --> D[Select scope floor namespace]
    D -->|No authority or seed| C
    D --> E[Refresh cached seed against durable floor]
    E -->|Seed below floor or floor unreadable| C
    E --> F[Build ChildAdopter with scope ID, seed, and floors]
    F --> G[Resolve and adopt record]
    G --> H[Return verified content or versions]
Loading

Reviews (3) · Last reviewed commit: "chore: merge main"

Comment thread crates/engine/src/facade.rs
@coderabbitai

coderabbitai Bot commented Sep 19, 2026

Copy link
Copy Markdown
⚠️ Action not completed

Already reviewed the last commit. Use @coderabbitai full review to rerun a review of the entire changeset.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

…s it

The grant or invite link that mints a scope now hands back the minted
scope's read seed and epoch, and the facade deposits it at once. The
owner's own scope roots for the floor namespace now include the roots this
session minted, so the seed lookup and the tick's eviction pass keep that
seed while no boundary walk has proved the scope.
@FSM1

FSM1 commented Sep 19, 2026

Copy link
Copy Markdown
Owner Author

Review disposition for d60afbc:

  • Greptile P1 "Fresh scopes remain unreadable": accepted and fixed. The reply in the thread gives the details. Thread resolved.
  • CodeRabbit web review on d7651dd: no actionable comments, no nitpicks, and no "Outside diff range" items.
  • CodeRabbit CLI (--base-commit merge base), one minor finding: "add a read while the record store is unavailable, before the endpoints heal" in the_owner_reads_a_shared_folder_no_walk_has_proved. Rejected for this PR. The test checks that the seed stays in memory while no walk can prove the scope, so it reads after the endpoints heal and before any tick. We tried the offline read. For the contact grant, it passes. For the write invite link, it returns a trust violation (seal-open-failed) with no tick after the link. The probable cause is the cached record that the offline read falls back to after the write cut; we did not confirm it. The seed is present for this read: the error is not ContentUnavailable. This is a different failure from the finding that this PR fixes, and we report it for a separate decision.

@FSM1
FSM1 merged commit 00e24aa into main Sep 19, 2026
43 checks passed
@FSM1
FSM1 deleted the fix/1909-read-under-the-files-scope branch September 19, 2026 10:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(engine): a file below a grafted root does not read

1 participant