fix(server): session tools enforce the write ACL; /ws honours suspend; list_snapshots scoped - #410
Merged
Merged
Conversation
…; list_snapshots scoped Four confirmed security findings in escurel-server, one PR: 1. open_session now takes the AclCaller and refuses (JSON-RPC -32000, data code `forbidden`) a caller who may not write the page under ESCUREL_WRITE_ACL — a session's apply_op stream edits the page byte by byte, so it is gated by the SAME policy update_page enforces (mirrors the WS attach gate, #352, on the write side). 2. close_session RE-CHECKS that write policy at commit time (the ACL can change while a session is open); a refused commit returns update_page's {ok:false, issues:[forbidden]} shape and leaves the session open so it can still be discarded. apply_op stays keyed by session possession (session ids are unguessable). 3. list_snapshots follows the page's read ACL exactly like list_op_authors: denial is absence, not error — a page you may not read reports the same empty history as one that does not exist. 4. GET /ws applies the #247 tenant-suspend gate before the upgrade, matching POST /mcp: a suspended tenant refuses non-admin bearers with HTTP 403 (tenant_suspended); admin still connects to resume. Also narrows the event_subscribe resume CONTRACT to what the code does: since_event_id replay is best-effort and inbox-only (list_inbox is a queue, not an event log — events assigned/processed while disconnected are not replayed), correcting the "gap-free"/"lossless" wording in protocol.md, the ws.rs comment, and the consumer skill (0.6.27). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Aug 15, 2026
jrosskopf
added a commit
that referenced
this pull request
Aug 15, 2026
…417) #410 closed the case where the page existed and the caller was outside its write ACL. The same fail-open survives through absence: the gate reads the STORED page to decide, so with nothing to read it decides nothing and allows. alice -> open_session {page_id: "markdown/instances/note/no-such-note.md"} Ok {"session":"sess_…","head_version":"v0","ws_url":"/ws"} A session id, a head version and a socket URL for a page nobody wrote. Smaller than #410 — there is no content, so nothing is disclosed — but a session with no policy behind it is a write capability nothing evaluated, and closing it materialises a page that never passed a create grant. Refused with the same `forbidden` code and message shape as #410, deliberately not distinguishing "no such page" from "not yours": the two are one message on purpose. Session-only servers (`indexer = None`) keep their behaviour — absence is their normal state rather than a gap. Also adds the group-scoped pair. #410's test covers the OWNER-private shape (`visibility: owner` + `owner_field`); a multi-customer tenant carries isolation in group grants on the skill and on the instance's own `acl:` block instead, and that shape had no coverage. There is no bug behind it — it passes on main, which is worth knowing, because a downstream report claimed otherwise after probing a pin that predated #410. 1344 tests pass, fmt and clippy clean.
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.
Summary
Fixes four confirmed security findings in
crates/escurel-server:open_sessionreceived no caller;close_sessioncommitted viaupdate_page_asdirectly. Nowopen_sessiontakes theAclCallerand refuses (JSON-RPC-32000, data codeforbidden) a caller who may not write the page underESCUREL_WRITE_ACL— mirroring the WS attach gate (WS: a CRDT session is single-peer — one client's ops never reach another attached client #352) on the write side — andclose_sessionre-checks the same write policyupdate_pageuses at commit time (the ACL can change while a session is open). A refused commit returnsupdate_page's{ok:false, issues:[{code:"forbidden"}]}shape and leaves the session open for acommit:falsediscard.apply_opstays keyed by session possession.list_snapshotshad no caller/ACL — a non-owner could enumerate the snapshot history of owner-private pages. Now gated exactly liketool_list_op_authors:may_read_instance, with denial as absence (empty list, byte-identical to a page that does not exist — no existence oracle)./wsmissed the tenant-suspend gate. The upgrade now applies the same tenant_update: support status (suspend/resume), quotas, embedding_provider #247 rule asPOST /mcp: a suspended tenant refuses non-admin bearers with HTTP 403 (tenant_suspended) before the upgrade; admin still connects (toresume).event_subscribeoverclaimed gap-free resume. Contract fix only (behaviour unchanged):since_event_idreplay is best-effort and inbox-only —list_inboxis a queue, not an event log, so events assigned/processed while disconnected are not replayed. Corrected inprotocol.md§Resume (+ the "lossless" token-lifetime line), thews.rscomment, and the consumer skill.Skill sync (same PR):
references/02-tool-surface.md(session write-ACL gates + ACL-scopedlist_snapshots/list_op_authors),references/11-event-driven-agents.md(resume semantics),CHANGELOG.mdspliced0.6.27.schema.rstool descriptions foropen_session/close_session/list_snapshotsupdated to match.Test plan
New no-mock integration tests (real gateway, real DuckDB indexer, real CRDT backend, real OIDC TestIssuer, real tokio-tungstenite) in
crates/escurel-server/tests/suite/session_acl_and_gates.rs(declared intests/suite/main.rs), each red before the fix:non_owner_cannot_open_session_on_an_owner_private_pageacl_change_mid_session_denies_the_close_commit(also asserts the page body is untouched and the session stays discardable)list_snapshots_denial_reads_as_absence_not_as_an_error(positive control: the owner sees real seeded history)suspended_tenant_is_rejected_at_the_ws_upgrade(controls: agent connects while active; admin connects while suspended)Local four-command gate (un-piped exit codes):
cargo fmt --check= 0,cargo clippy --workspace --all-targets -- -D warnings= 0,cargo test --workspace --all-targets= 0 (all 434 escurel-server suite tests + workspace green),cargo build --workspace --release= 0.🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.