Skip to content

fix(auth): authorize structural writes from stable ancestry#182

Closed
marcus-pousette wants to merge 1 commit into
auth/op-authored-at-claimsfrom
fix/auth-scope-source-parent
Closed

fix(auth): authorize structural writes from stable ancestry#182
marcus-pousette wants to merge 1 commit into
auth/op-authored-at-claimsfrom
fix/auth-scope-source-parent

Conversation

@marcus-pousette

Copy link
Copy Markdown
Collaborator

Stack

Stacked on #157. Merge #157 first, then retarget this PR to main.

In one sentence

Authorize structural writes against the node's stable source ancestry as well as its destination, so a scoped writer cannot pull an external node into its subtree.

What was wrong?

TreeCRDT insert is also an upsert: if the node already exists, it detaches the node from its current parent and attaches it to the requested parent.

Auth checked only the insert destination. A writer scoped to subtree A could therefore submit insert(existingExternalNode, parent=A) and steal a node from outside its grant.

SQLite local writes had a second version of the bypass. The extension prepared/applied the structural change before JavaScript awaited authorization inside the outer savepoint. A normal scope evaluator then saw the node at its new in-scope location instead of its old external location.

What changes?

  • Existing-node insert checks both the source node and destination parent.
  • A genuinely absent node is distinguished from missing ancestry and remains a valid new insert.
  • The SQLite extension returns the target node's existence and parent captured before the prepared write is committed.
  • Local auth evaluates the source through that explicit prior parent; it does not trust post-write ancestry.
  • Move, delete, tombstone, and payload authorization use the same stable source-state path.
  • An old extension that does not return pre-write state fails closed when local auth is enabled.
  • A custom evaluator that cannot honor an explicit prior parent also fails closed.

Why the extra absent result?

Before this PR, a missing target and an incomplete ancestor chain both returned unknown. Insert needs to tell those apart: a missing target has no source grant to check, while incomplete ancestry must never be treated as authorization.

Size and debloat

The PR is +497/-32 across 14 files:

  • production: approximately +239/-28
  • regressions and shared conformance: +251/-4
  • changeset: 7 lines

Pre-write state is captured from the already-open native CRDT session and returned with the existing local-op result; it does not add a separate preflight query/round trip.

Validation

  • auth: 47/47 tests
  • SQLite-node: 35/35 tests
  • native extension: 20/20 tests
  • TypeScript builds for protocol, interface, auth, sync-sqlite, conformance, and sqlite-node
  • strict workspace Clippy and feature-specific extension Clippy
  • SQLite static-link check
  • Rustfmt, Prettier, and diff checks

Regressions cover remote insert stealing, local insert/move cross-scope rejection, valid new inserts, valid in-scope moves, rollback, and native pre-state serialization.

Important remaining transaction issue

This fixes which tree state authorization evaluates. It does not remove the pre-existing arbitrary async authorization wait inside the outer SQLite savepoint. Concurrent use of the same runner can still interleave unrelated writes into that savepoint and an auth rejection can roll them back. That transaction-isolation redesign remains a separate critical PR rather than being hidden inside this scope patch.

@marcus-pousette

Copy link
Copy Markdown
Collaborator Author

Superseded by #188. The pre-write snapshot here stabilizes only the local authorization call; it is unsigned and unavailable to remote verifiers, so out-of-order structural delivery can still make honest replicas accept different scoped operation sets. #188 replaces this 497-line path with a smaller deterministic fail-closed rule and removes the obsolete machinery instead of stacking another patch on it.

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.

1 participant