[ragnorc]

What & why

Diff page tokens are checksummed, not signed. Before this change, a diff continuation re-authorized the sides the request names, then reopened whichever commit ids the token carried. So a caller could take a token from a diff it may read, re-sign it naming commits of a branch its policy does not allow, and read that branch's entity changes. A server test showed this returning 200 on both GET /diff and GET /branches/diff.

A continuation now reopens a token commit only when the request's side reaches it:

• the side resolves to that commit now; or
• the side moves (a branch head, or the branch form's merge base), and either the commit was an earlier head of the side's branch, or the caller could name it as a side of its own (read on the branch that authored it). An earlier head is found by walking first parents from the branch's head through commits that branch published, within 1,024 commits, so a fork's first head (which main published) is still reached, and nothing below it is.

A side spelled as a commit id reaches only that commit. Unknown and unreadable commits get the same 400 (change cursor rejected: …), and the refusal comes before any snapshot opens, so it reveals nothing about whether a commit exists. A continuation after a branch moves is still answered, as the diff contract promises. The check runs on every door: served reads use the caller's policy, and the embedded diff_page (the CLI with --store, the embedded statement) uses an allow-all decision, so a token has one contract everywhere.

Each token commit now opens through what reached it: the side's resolved snapshot, the branch it headed, or its id. Before, a continuation reopened its sides' heads by id, and a lookup that missed asked each newer live branch's __manifest for the commit.

GET /diff, GET /branches/diff and the branch diff … after page statement now share one read-check helper in the server, so the branch form gets the same check. Before, the branch form authorized its two branch names but gave the engine no read check, so its continuations reopened token commits unchecked.

Backing issue / RFC

Refs #1009, which proposes the general fix: authenticated continuation tokens. This PR is the per-kind proof for diff tokens until then. GET /diff landed after v0.13.0 and has not been released, so no release ever had the hole, and there is no fixed changelog note.

• Fixes an accepted issue
• Is an RFC PR, or implements an accepted RFC
• Trivial fast-lane

Checklist

• Change is focused (one logical change)
• Tests added/updated for behavior changes
• Public docs updated (docs/user/branching/changes.md)
• Reviewed against docs/dev/invariants.md. Invariant 10 is the point of the change: policy is enforced at the engine on every page. No deny-list item is hit.

Local verification

On this branch at 7671578 (on main at 864495a):

• cargo fmt --all --check: clean
• cargo clippy --workspace --all-targets --locked -- -D warnings -W clippy::dbg_macro: clean
• RUST_MIN_STACK=16777216 cargo test -p omnigraph-engine -p omnigraph-server --locked --test integration -- changes:: changes_cost:: auth_policy:: data_routes::: engine 65 passed, 1 ignored (an existing ignore); server 130 passed

Before rebasing onto current main (the same change together with the commit-list paging PR):

• cargo test -p omnigraph-server --locked --test integration: 264 passed
• cargo test -p omnigraph-engine --locked --test integration -- changes:: lineage_projection:: warm_read_cost:: changes_cost:: branching::: 146 passed, 1 ignored (an existing ignore)
• cargo test -p omnigraph-engine --locked --lib changes::token: 12 passed
• With the new check skipped, the two strengthened auth tests fail with 200 for the re-signed tokens; with it restored, they pass.

Not run: the canonical workspace test graph, the GQT corpus, and the S3/Azure suites. CI covers them.

Notes for reviewers

• The forgery was reproduced before the fix. With the new check skipped, both strengthened auth tests (policy_commit_diff_forbidden_is_indistinguishable_from_unknown and policy_branch_review_needs_read_on_the_target) receive 200 for the re-signed tokens. With the check in place, they receive 400.
• Cost, measured by changes_cost::graph_diff_side_resolution_is_flat_in_live_branches. A continuation re-resolves its sides as the first page does; the branch form repeats the first page's merge-base capture. Both forms read 7 __manifest objects on the first page and 7 on the continuation, at 1 and at 9 idle branches. Before the open-through-the-side change, the continuation read 14 and then 22: it grew with live branches.
• An earlier head that another branch published, below the branch's last 1,024 commits, is refused with 400 (restart the diff). This is the walk's bound.

[cursor]

Review of 638e874 (merge base 864495a)

Recommendation: request changes for one small blocking item (doc accuracy). The security fix is correct, it is at the right layer, and the regression proof holds. This review posts as comments because this tool cannot submit a review state. Please treat the blocking item as "changes requested".

Why, what, how, impact

• Why. A diff page token is checksummed, not signed. Before this PR, a continuation re-authorized only the names in the request. Then it opened the commit ids in the token. A caller could edit a token and read entity changes of a branch that its policy denies. This breaks invariant 10: server reads apply their policy, and the engine enforces it.
• What. Before any snapshot opens, the engine checks each token commit against the sides of the request (authorize_diff_continuation, omnigraph.rs:3022-3078). A side spelled as a commit accepts only that commit. A branch side or a merge base accepts the commit it resolves to now, or a commit the caller may read as a side of its own. GET /branches/diff and branch diff … after page now give the engine the read check through the shared readable_diff_page (handlers.rs:4178). Before, the branch form gave the engine no read check.
• How it fails. Unknown and unreadable commits get the same 400 (diff_token_unreached). The refusal comes before a snapshot opens, so it is not an existence oracle.
• Impact. It closes a policy bypass on two routes that no release has shipped yet. There is no storage format change and no Lance change. Embedded callers keep their behavior.

Blocking

1. The doc says every continuation is checked, but the embedded path does not check (docs/user/branching/changes.md:75-78, inline). Omnigraph::diff_page passes no read check, so the CLI with --store and the embedded statement accept a token whose commits their sides never reach. A probe confirmed this on 638e874. Scope the sentence to served reads, or run the reach check on every path.

Optional (4)

1. A valid server-minted token is refused after a branch whose head another branch wrote moves (a fresh fork of protected main, a fast-forward, and feat(branch): start a branch at a graph commit #1016). This is proven by a probe on both commits (omnigraph.rs:3067, inline).
2. The cost note says "under a policy", but every served branch-form continuation pays one more capture_merge_base. This is not measured (omnigraph.rs:3053, inline).
3. Assert the refusal reason in the tests so they stay useful after the Authenticate continuation tokens: page tokens and feed cursors are checksummed, not signed #1009 MAC (auth_policy.rs:1804, inline).
4. Only server tests pin the new rule of the public engine API diff_page_readable. Testing rule 1 puts engine guarantees at the engine API. Consider one assertion beside changes.rs::graph_diff_pages_answer_for_the_first_pages_endpoints (a token naming an unreadable commit gives ChangeCursorRejected, and can_read is asked for that commit's branch).

Contract and affected class

• Violated invariant. Invariant 10. Every field in a token is a claim by the caller (Authenticate continuation tokens: page tokens and feed cursors are checksummed, not signed #1009).
• Full class. Every continuation that names graph state:
  • commit-changes page tokens are bound to the commit in the path (omnigraph.rs:2554): safe;
  • feed cursors and feed page tokens are checked against the first-parent chain (feed.rs:551, feed.rs:572): safe;
  • diff page tokens were the one unbound kind, and this PR fixes them;
  • commit-list page tokens (feat(commits): page a branch's commit list #1013) are the known open case that Authenticate continuation tokens: page tokens and feed cursors are checksummed, not signed #1009 records.
• Layer. The fix is in the engine's prepare_diff, which owns the contract. The server change only wires the branch form to the same check. This is the correct root-cause layer for a per-kind proof.
• Substrate. I read the Lance versioning and branch/tag pages. Lance refs and versions carry no access control, so this check belongs in OmniGraph. No Lance release changes that.

Trade-offs and liability

• Removed liability. A real read bypass, and two copies of the read-check closure (now one helper).
• Added liability. About 70 lines of per-kind proof, and one more rule that each new continuation kind must copy. After five more kinds like this, the per-kind analysis is the cost. Authenticate continuation tokens: page tokens and feed cursors are checksummed, not signed #1009 (a MAC) is the root fix. Because Authenticate continuation tokens: page tokens and feed cursors are checksummed, not signed #1009 keeps per-kind checks as defense in depth, this code is not throwaway.
• Semantics chosen. The rule is "the caller could read this commit directly", not "the first page minted this commit". An edited token can still name another readable commit, and the page then answers for it, with to_commit_id showing the substitution. That exposes nothing new. It also explains optional item 1: authorization equivalence refuses some valid tokens.
• Cost. The two-point form adds one commit classification only when a side moved. The branch form adds one merge-base capture for each served continuation. Both are bounded by the cost of the first page. Neither is measured.

Regression proof

GQT cannot express this contract. The served GQT target sends no bearer token and applies no policy, and the format cannot edit and re-checksum a token. The cheapest owner that sees the contract is the server suite auth_policy, which the PR extends.

Commit	Tree	Result
864495a (merge base)	base, plus the head's auth_policy.rs only	0 passed, 2 failed. auth_policy.rs:1802 and :2074 get left: 200, right: 400. The 200 bodies contain the feature-only and private-only rows: the predicted leak.
638e874 (head)	clean	2 passed, 0 failed, 0 ignored
638e874	clean	auth_policy:: + data_routes::: 129 passed, 0 failed

Commands (each side in its own CARGO_TARGET_DIR, with RUST_MIN_STACK=16777216):

cargo test -p omnigraph-server --locked --test integration --no-run
<built integration binary> --exact \
  auth_policy::policy_commit_diff_forbidden_is_indistinguishable_from_unknown \
  auth_policy::policy_branch_review_needs_read_on_the_target --test-threads=1
<built integration binary> auth_policy:: data_routes::

I checked that the head-only refusal text is in the head binary and not in the merge-base binary. With a shared target directory, Cargo reused the head engine for the base build. I discarded that run.

The two probes in the inline comments were temporary tests. I appended them to auth_policy.rs on both commits and then removed them.

CI on 638e874: 33 checks, 28 passed, 5 skipped (release and vocabulary jobs), 0 failed. This includes Test Workspace, GQ Logic Tests, Fix Regression Gate and the S3/Azure jobs.

Limits

• Not run locally: the canonical workspace graph, the GQT corpus, clippy, fmt, and S3/Azure. CI ran them on this exact head.
• Branch statement on the base: on the merge base, the branch test stops at the HTTP assertion. So the statement row of that test was not seen failing before the fix. It passes on the head.
• Cost: not measured, so the cost statements above come from code inspection only.
• Timing: I did not check timing side channels. A commit lookup miss and a hit may take different time. That is also true of the first page before this PR.