BLOCKER: SSE paths remain visible after authorization ends #788

Open
opened 2026-10-02 13:10:46 +00:00 by kayg · 5 comments
Owner

Protocol audit assigned under #663; source base c4a61e8cf0. Source review only; no hostile payload, live exploit or crash-threshold measurement. The owner rule blocks authorization holes and crash/DoS risks. No product code is changed by this audit.

Evidence: Files events (crates/plugins/files/src/lib.rs:3834–3890) retains
the User string and emits paths from files_events. Notes events
(crates/plugins/notes/src/lib.rs:5261–5294) retains the User string and emits
path/kind from the event bus. Neither checks the credential or plugin state
again. serve.rs:166–211 holds a client-IP permit for the body lifetime but
does not revalidate authority. Request middleware in wire.rs:2433 runs only
when the stream opens. No credential-revocation cancellation is attached to
these bodies. The same source pattern remains in round-7a and perf-stream-668.

Impact: a revoked session or App Password can continue to receive future
Files and Notes names and changes through an existing stream. Disabling the
User or plugin does not make this body stop. There is no bounded security
lease. This is an authorization hole; it does not claim file-body disclosure.

Repair: attach a shared revocable stream lease to the authenticated credential,
User and allowed plugin/resource scope. Check it before emitting data, and
cancel on security-state changes. Add a bounded quiet-stream check. Preserve
the existing IP/User resource permits. Prefer content-free wakeups, with
current access checked again by the delta endpoint (DESIGN §58 rule 5).

Regression coverage: open a stream with each credential class, revoke or
expire it, disable the User/plugin, then publish a synthetic change. The old
stream must emit no path; a still-authorized stream must receive its own
change. Cover reconnect, quiet streams and permit release. #668 is adjacent
work, but it does not replace these old endpoints yet.

Duplicate check: searched all issue states for XML depth, connection cap, SSE revocation and MCP session. Read related #457, #328, #329 and #668. No matching repair issue identified. Track the repair with source-level tests and a safe local validation of the repaired boundary. Do not close this issue from the audit job.

Protocol audit assigned under #663; source base c4a61e8cf090170f35b1bed3350d9de20c83ecd5. Source review only; no hostile payload, live exploit or crash-threshold measurement. The owner rule blocks authorization holes and crash/DoS risks. No product code is changed by this audit. Evidence: Files `events` (`crates/plugins/files/src/lib.rs:3834–3890`) retains the User string and emits paths from `files_events`. Notes `events` (`crates/plugins/notes/src/lib.rs:5261–5294`) retains the User string and emits path/kind from the event bus. Neither checks the credential or plugin state again. `serve.rs:166–211` holds a client-IP permit for the body lifetime but does not revalidate authority. Request middleware in `wire.rs:2433` runs only when the stream opens. No credential-revocation cancellation is attached to these bodies. The same source pattern remains in round-7a and perf-stream-668. Impact: a revoked session or App Password can continue to receive future Files and Notes names and changes through an existing stream. Disabling the User or plugin does not make this body stop. There is no bounded security lease. This is an authorization hole; it does not claim file-body disclosure. Repair: attach a shared revocable stream lease to the authenticated credential, User and allowed plugin/resource scope. Check it before emitting data, and cancel on security-state changes. Add a bounded quiet-stream check. Preserve the existing IP/User resource permits. Prefer content-free wakeups, with current access checked again by the delta endpoint (DESIGN §58 rule 5). Regression coverage: open a stream with each credential class, revoke or expire it, disable the User/plugin, then publish a synthetic change. The old stream must emit no path; a still-authorized stream must receive its own change. Cover reconnect, quiet streams and permit release. #668 is adjacent work, but it does not replace these old endpoints yet. Duplicate check: searched all issue states for XML depth, connection cap, SSE revocation and MCP session. Read related #457, #328, #329 and #668. No matching repair issue identified. Track the repair with source-level tests and a safe local validation of the repaired boundary. Do not close this issue from the audit job.
Author
Owner

Added a server-owned wrapper for authenticated SSE bodies. It retains hash-only session identity or App Password ID, checks current grant/surface/Plugin enablement before every chunk, and checks quiet bodies every second. Inner stream resource permits drop with the wrapper. This also covers the newer Files wakeup endpoint and job feeds.
Validation is in progress. No completion or live exploit claim.

Added a server-owned wrapper for authenticated SSE bodies. It retains hash-only session identity or App Password ID, checks current grant/surface/Plugin enablement before every chunk, and checks quiet bodies every second. Inner stream resource permits drop with the wrapper. This also covers the newer Files wakeup endpoint and job feeds. Validation is in progress. No completion or live exploit claim.
Author
Owner

Review finding for #788: the shared SSE guard also wraps /api/v1/admin/jobs/events. set_role and reconcile_oidc_role revoke sessions, but a demotion can commit between require_live_session and the subsequent User read. Principal::valid now applies the existing path_is_or_below and Role::can_admin predicates to that current read, without another query. The bounded fixture probe retains its own session while changing its User role to model this boundary, consumes the initial refresh frame, and requires quiet EOF. Probe syntax passed; real-server execution is pending the fresh Rust build. Security timers use MissedTickBehavior::Skip so a delayed Index cannot cause catch-up query batches. Existing #665/#666/#667/#668 Files primitives remain unchanged.

Review finding for #788: the shared SSE guard also wraps /api/v1/admin/jobs/events. set_role and reconcile_oidc_role revoke sessions, but a demotion can commit between require_live_session and the subsequent User read. Principal::valid now applies the existing path_is_or_below and Role::can_admin predicates to that current read, without another query. The bounded fixture probe retains its own session while changing its User role to model this boundary, consumes the initial refresh frame, and requires quiet EOF. Probe syntax passed; real-server execution is pending the fresh Rust build. Security timers use MissedTickBehavior::Skip so a delayed Index cannot cause catch-up query batches. Existing #665/#666/#667/#668 Files primitives remain unchanged.
Author
Owner

Independent read-only review of target b3e7c14ad for #785.

P2: the new SSE session lease reads through the single writer connection.
crates/calternal-server/src/protocol_authority.rs:125 calls
require_live_session on each chunk and quiet lease. The SELECT at
crates/calternal-auth/src/store.rs:2548 uses self.pool. Production passes
db.writer_pool() at crates/calternal-server/src/wire.rs:1270, and
crates/calternal-db/src/db.rs:165 sets one connection.

This makes browser stream delivery and quiet checks wait behind write
transactions and adds repeated reads to the writer queue. This is a source
dependency, not a measured regression or an access-bypass claim. It conflicts
with the binding performance priority and the separate-reader design. The
target's DESIGN §58 is Agent discovery, not the performance contract cited
by the original issue.

Concrete fix: move the live-session SELECT to the existing read pool. Keep
User identity, revocation and both expiry predicates. Add a test with separate
pools and an unavailable writer; a live-session check must still succeed
through the reader. Do not add a second cached authority mechanism.

Existing #788 owns the related lease repair; no duplicate issue was created.
Review head: 4e739be474. No build, test, server,
browser or performance measurement was run under the LIGHT job restriction.

Independent read-only review of target b3e7c14ad for #785. P2: the new SSE session lease reads through the single writer connection. `crates/calternal-server/src/protocol_authority.rs:125` calls `require_live_session` on each chunk and quiet lease. The SELECT at `crates/calternal-auth/src/store.rs:2548` uses `self.pool`. Production passes `db.writer_pool()` at `crates/calternal-server/src/wire.rs:1270`, and `crates/calternal-db/src/db.rs:165` sets one connection. This makes browser stream delivery and quiet checks wait behind write transactions and adds repeated reads to the writer queue. This is a source dependency, not a measured regression or an access-bypass claim. It conflicts with the binding performance priority and the separate-reader design. The target's DESIGN §58 is Agent discovery, not the performance contract cited by the original issue. Concrete fix: move the live-session SELECT to the existing read pool. Keep User identity, revocation and both expiry predicates. Add a test with separate pools and an unavailable writer; a live-session check must still succeed through the reader. Do not add a second cached authority mechanism. Existing #788 owns the related lease repair; no duplicate issue was created. Review head: 4e739be4743d2fbc63d91ae2af9dd5e61a5ec652. No build, test, server, browser or performance measurement was run under the LIGHT job restriction.
Author
Owner

Finding #788: require_live_session queried the single-writer pool while the operation needs only session read access. The regression test holds the writer pool's only connection and confirms require_live_session completes through the separate read pool within 250 ms. The implementation now uses read_pool; cargo clippy for calternal-auth is running, with its test gate next.

Finding #788: `require_live_session` queried the single-writer pool while the operation needs only session read access. The regression test holds the writer pool's only connection and confirms `require_live_session` completes through the separate read pool within 250 ms. The implementation now uses `read_pool`; `cargo clippy` for `calternal-auth` is running, with its test gate next.
Author
Owner

cargo clippy -p calternal-auth --all-targets -- -D warnings passed. The full auth test command finished 72 passed; 17 failed; 1 ignored; the new session_checks_answer_while_the_writer_pool_is_busy regression passed. Most failures were fixture setup errors: SqliteAuthStore::connect("sqlite::memory:") returned Unavailable, which maps transient SQLite/pool errors. The host reported 98.97% average I/O pressure during this run. One existing Argon2 scheduling test also timed out before its verifier started. I did not rerun the gate under the one-run verification rule.

`cargo clippy -p calternal-auth --all-targets -- -D warnings` passed. The full auth test command finished `72 passed; 17 failed; 1 ignored`; the new `session_checks_answer_while_the_writer_pool_is_busy` regression passed. Most failures were fixture setup errors: `SqliteAuthStore::connect("sqlite::memory:")` returned `Unavailable`, which maps transient SQLite/pool errors. The host reported 98.97% average I/O pressure during this run. One existing Argon2 scheduling test also timed out before its verifier started. I did not rerun the gate under the one-run verification rule.
Sign in to join this conversation.
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
kayg/calternal#788
No description provided.