security: remediate September 2026 audit findings (WebSocket, sessions, identity, auth core) - #26
Merged
Merged
Conversation
WebSocket (ras-jsonrpc-bidirectional-server/-macro/-client): - W1 re-authorize held subscriptions on credential re-validation; add PermissionChangePolicy (DropSubscriptions | Close); mirror context subscriptions into the manager's topic index - W2 enforce max_message_size/max_frame_size at the transport - W3 SubscriptionLimits (per message, per connection, topic length) - W4 KeepaliveConfig (server ping + idle timeout) - W5 route WITH_PERMISSIONS checks through AuthProvider::check_permissions - C1 remove AuthConfig::JwtParams (token in URL); C2 percent-encode CustomParams; browser auth via ras-jsonrpc + token.<jwt> subprotocols, server selects ras-jsonrpc so the token is never echoed Sessions (ras-identity-session): - S1 drop inline write-locked cleanup from verify/begin_session - S2 iss/aud required by default (allow_unscoped_tokens opt-out) - S3 secret entropy + placeholder substring checks - S4 reject future iat/nbf; S5 max_sessions_per_user Identity (ras-identity-local, ras-identity-oauth2): - I1 skip_serializing password_hash / client_secret - I2 redact LocalAuthPayload Debug; I4 Argon2 on spawn_blocking with password length cap - I3 evict oldest pending OAuth2 flow instead of refusing (login lockout) - I5 optional callback code, fixed ProviderDenied error; I6 fixed message for upstream HTTP errors; I7 constant-time binding compare - I8 metadata_claims allow-list; I9 https-only endpoints by default Auth core / files / REST (ras-auth-core, ras-file-core/-macro, ras-rest-macro): - A1 rename weak CSRF modes to dangerous_*, deprecate old names, warn - A2 drop Serialize from AuthError; Display no longer lists held grants - F1 sanitize_filename + RFC 5987 filename* in attachment() - F2 generic bodies for axum path/query/multipart rejections Version bumps: ras-auth-core 0.3.0, ras-identity-session 0.4.0, ras-identity-local 0.3.0, ras-identity-oauth2 0.3.0, ras-jsonrpc-bidirectional-server 0.3.0, -client 0.3.0, -macro 0.2.1, ras-file-core 0.2.1, ras-file-macro 0.2.1, ras-rest-macro 0.3.1. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NZEonxsxS45oAEC7bUTfnL
… test Raw-socket test sends a frame header announcing an oversized payload without the body: the transport limit rejects it immediately, whereas a post-buffer check would wait for the payload. Verified to fail with the upgrade limits removed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NZEonxsxS45oAEC7bUTfnL
…anitizing Second review pass on the WebSocket and REST hardening: - egress gate: topic-routed messages are tagged (OutboundMessage) and re-checked against the connection's subscriptions right before the socket write, so a broadcast that raced re-authorization is dropped - connection cap enforced with a semaphore permit held for the connection's lifetime; bounded default of 10 000; generated builder exposes every knob - global subscription cap via ConnectionManager::total_subscription_count - zero revalidation/keepalive durations no longer panic tokio intervals - extractor rejection text sanitized (control chars, 256-byte cap) before it reaches tracing in generated REST and file handlers - chacha20 lock entry refreshed off the yanked 0.10.1 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NZEonxsxS45oAEC7bUTfnL
- DefaultConnectionManager::add_subscription enforces topic length, per-connection and global caps itself (atomic counter reserve); handler rolls back anything the manager refuses, so custom handlers cannot exceed - add/remove_subscription hold the connection entry guard across both the connection state and topic index updates: no stale index entries - WebSocketService::connection_permits is required; advisory admission fallback removed - sanitize_log_detail counts the ellipsis inside the 256-byte budget - tests: concurrent global-cap admission, 500-round add/remove interleaving (fails against the old ordering), greedy custom handler rollback Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NZEonxsxS45oAEC7bUTfnL
…he manager The service owns a SubscriptionAccounting counter shared by all of its connections (WebSocketService::subscription_accounting, required). After every handle_subscribe the handler reconciles the context against topic length, the per-connection cap and the global cap (atomic reserve), and only accepted topics reach the manager. Slots are released on unsubscribe, re-validation drops and disconnect. Integration test plugs a fully permissive custom ConnectionManager and a greedy handler into build_with_manager; fails with the server-side check removed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NZEonxsxS45oAEC7bUTfnL
…on path The context carries the service's SubscriptionPolicy (limits, shared accounting, manager). subscribe() enforces topic length and the per-connection cap under the write guard, reserves a global slot atomically and mirrors into the manager, returning an error on refusal; unsubscribe() releases both. The info field is private, so no unchecked path remains: subscriptions from on_connect or handle_request are limited and counted, and teardown releases only what was reserved (no underflow). Removes the handler-level reconciliation. Regression test subscribes greedily from on_connect and handle_subscribe; fails against an unchecked subscribe. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NZEonxsxS45oAEC7bUTfnL
Notify::notify_waiters drops wake-ups for tasks not yet parked, which made the atomic-admission test flaky on slow CI runners. Use a watch channel so a late waiter still observes the release. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NZEonxsxS45oAEC7bUTfnL
…ts in tests - Test job runs cargo-nextest (per-test process isolation, per-test timings, JUnit artifact); retries disabled via .config/nextest.toml so a flaky test fails instead of hiding; 2 min per-test hang cap (3 in CI) - Coverage job runs llvm-cov via nextest for HEAD and the merge-base on PRs, posts a sticky per-crate table with deltas and the files that dropped most (.github/scripts/coverage_summary.py); no external service - Weekly / on-demand flake-detector workflow runs the suite 5x at three parallelism levels - Two tests replaced bounded sleep-polls with 10 s deadline waits - README documents nextest and coverage Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NZEonxsxS45oAEC7bUTfnL
Test coverage (lines)Total: 92.71% (+0.97 vs base) — 18464/19915 lines
Files with coverage drops ≥ 1 point
|
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
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
Remediates every finding from the September 2026 security review (Codex) plus a follow-up gap sweep, then four more review rounds against the fixes themselves. 24 original issues plus 10 residuals, each with a regression test named by its ID. Codex has signed off on the final state.
Full per-crate detail is in
CHANGELOG.mdunder Unreleased (six dated sections, most recent first).WebSocket (
ras-jsonrpc-bidirectional-server/-macro/-client/-types)PermissionChangePolicy(drop or close). Topic-routed messages are tagged and re-checked at the socket write (egress gate), closing the revocation race.max_message_size/max_frame_sizeenforced at the transport, not after buffering.SubscriptionLimits(per message, per connection, topic length, global). Enforced insideConnectionContext::subscribe, the only mutation path, so they hold from any handler callback and with anyConnectionManager. Manager keeps state and topic index consistent under one guard.KeepaliveConfig(server ping + idle timeout). Zero durations no longer panic.WITH_PERMISSIONSrouted throughAuthProvider::check_permissionsover WebSocket.connection_permitsandsubscription_accountingare requiredWebSocketServicemethods.AuthConfig::JwtParamsremoved (token in URL). C2 custom params percent-encoded. Browser auth now works: client offersras-jsonrpc+token.<jwt>subprotocols, server selects the first so the token is never echoed.Sessions (
ras-identity-session)verify_session. S2iss/audrequired by default (SessionConfig::new(secret, iss, aud),allow_unscoped_tokens()opt-out). S3 secret entropy + placeholder checks. S4iat/nbffuture-dated rejection. S5max_sessions_per_user.Identity (
ras-identity-local,ras-identity-oauth2)password_hash/client_secretno longer serialized. I2 redactedLocalAuthPayloadDebug. I3 OAuth2 state store evicts instead of refusing (login lockout). I4 Argon2 onspawn_blocking, password length cap. I5 optional callbackcode, fixedProviderDenied. I6 fixed message for upstream HTTP errors. I7 constant-time binding compare. I8metadata_claimsallow-list. I9 https-only endpoints by default.Auth core / files / REST (
ras-auth-core,ras-file-core,ras-file-macro,ras-rest-macro)dangerous_*with deprecated aliases and startup warning. A2AuthErrorno longerSerialize; Display no longer lists held grants. F1sanitize_filename+ RFC 5987filename*. F2 generic bodies for extractor rejections, detail sanitized (sanitize_log_detail) before logging.Version bumps
Test plan
cargo test --workspace: 922 passed, 0 failedcargo clippy --workspace --all-targets -- -D warningscleancargo fmt --all --checkcleancargo deny check advisoriesok (yankedchacha20lock entry refreshed)--features wasm --target wasm32-unknown-unknown)subscribe— each test fails with its fix removedNot addressed:
lru/pasteadvisories come only fromratatuiin the example TUI (not a published crate); moving it to ratatui 0.30 is out of scope here.🤖 Generated with Claude Code
https://claude.ai/code/session_01NZEonxsxS45oAEC7bUTfnL