fix(slice-1): F2 — second offer returns 409 Conflict not panic

Adversarial review F2 (Med-High, confirmed): accept_offer asserted
audio_mid.is_none() on a path fed by unauthenticated external input.
A second POST /v1/sessions/:id/offer panicked the handler task instead
of returning a clean 4xx.

Replace the assert! with a typed RtcSessionError::AlreadyNegotiated.
routes.rs maps the variant to 409 Conflict, distinct from 400 Bad
Request for malformed SDP. debug_assert! documents the invariant for
tests; the production path returns Err, never panics.

TDD: unit test second_accept_offer_returns_already_negotiated_not_panic
red on the panic message; green after the typed error. Integration test
double_post_offer_returns_409_conflict_not_panic red on 400; green after
routes.rs is updated.

.gitignore

@@ -21,3 +21,4 @@
 # Local worktrees (per using-git-worktrees skill; isolation, not content)
 .worktrees/
 worktrees/
+docs/reviews/.*.kate-swp

crates/rutster-media/src/rtc_session.rs

@@ -51,6 +51,13 @@ pub enum RtcSessionError {
     Codec(#[from] opus::Error),
     #[error("UDP socket bind failed: {0}")]
     Socket(#[from] std::io::Error),
+    /// Adversarial review F2: `accept_offer` called twice for the same
+    /// session. The old `assert!` panicked on a path fed by external input;
+    /// this typed variant lets `routes.rs` map it to `409 Conflict`. The
+    /// `Channel` reaches `Connecting` on the first offer; a second offer
+    /// is a real conflict (the SDP negotiation is locked in), not a 400.
+    #[error("session already negotiated; only one offer per session")]
+    AlreadyNegotiated,
 }
 
 use str0m::change::SdpOffer;
@@ -180,7 +187,18 @@ impl RtcSession {
     /// host candidate (the UDP socket we just bound) *before* calling
     /// `accept_offer` so the answer carries it.
     pub fn accept_offer(&mut self, offer_sdp: &str) -> Result<String, RtcSessionError> {
-        assert!(self.audio_mid.is_none(), "accept_offer called twice");
+        // F2 (adversarial review): a second `accept_offer` was an `assert!`
+        // that panicked the handler — client-triggerable via double
+        // `POST /v1/sessions/:id/offer`. Typed error → `routes.rs` maps to
+        // `409 Conflict`. `debug_assert!` documents the invariant for tests
+        // (the production path returns the typed error, never panics).
+        if self.audio_mid.is_some() {
+            return Err(RtcSessionError::AlreadyNegotiated);
+        }
+        debug_assert!(
+            self.audio_mid.is_none(),
+            "accept_offer called twice (debug_assert only; production path returns Err)"
+        );
 
         // Register our local UDP socket as a host candidate. str0m includes
         // this candidate's address + the ICE creds it generates in the SDP
@@ -305,4 +323,28 @@ a=candidate:1 1 UDP 2113667327 192.168.1.42 50000 typ host\r
         let _ = session.accept_offer(BROWSER_SDP_OFFER).expect("answer");
         assert_eq!(session.channel_state(), ChannelState::Connecting);
     }
+
+    /// Adversarial review F2: a second `POST /v1/sessions/:id/offer` (or
+    /// any second call to `accept_offer`) must NOT panic the handler —
+    /// it returns a typed error so `routes.rs` can map it to `409 Conflict`.
+    /// The old `assert!(audio_mid.is_none())` panicked on a path fed by
+    /// unauthenticated external input; the typed error makes the contract
+    /// explicit. `debug_assert!` documents the invariant for tests.
+    #[test]
+    fn second_accept_offer_returns_already_negotiated_not_panic() {
+        let mut session = RtcSession::new().expect("session");
+        let _ = session
+            .accept_offer(BROWSER_SDP_OFFER)
+            .expect("first offer ok");
+        match session.accept_offer(BROWSER_SDP_OFFER) {
+            Err(RtcSessionError::AlreadyNegotiated) => { /* expected */ }
+            other => panic!("expected AlreadyNegotiated, got {other:?}"),
+        }
+        // Idempotency of the rejection: a third call still returns the
+        // same typed error, never escalates to a panic.
+        match session.accept_offer(BROWSER_SDP_OFFER) {
+            Err(RtcSessionError::AlreadyNegotiated) => { /* still expected */ }
+            other => panic!("third offer: expected AlreadyNegotiated, got {other:?}"),
+        }
+    }
 }

crates/rutster/src/routes.rs

@@ -64,6 +64,14 @@ pub async fn post_offer(
             answer_sdp,
         )
             .into_response(),
+        // F2: a second offer for the same session is a real conflict (the
+        // SDP negotiation is already locked in), not a malformed request.
+        // Distinct status code surfaces the operational shape to clients
+        // without forcing them to parse the error body.
+        Err(rutster_media::RtcSessionError::AlreadyNegotiated) => {
+            tracing::warn!(session_id = %id, "second offer rejected: already negotiated");
+            StatusCode::CONFLICT.into_response()
+        }
         Err(e) => {
             tracing::error!(error = ?e, "SDP accept failed");
             StatusCode::BAD_REQUEST.into_response()

crates/rutster/tests/api_integration.rs

@@ -10,6 +10,31 @@ use axum::http::{Request, StatusCode};
 use rutster::session_map::AppState;
 use tower::ServiceExt; // enables `oneshot` on the Router for sync tests
 
+/// A minimal SDP offer Chrome would send. Mirrors the unit-test fixture in
+/// `rutster-media::rtc_session::tests::BROWSER_SDP_OFFER`; not shared across
+/// crates because the integration test lives outside `src/`. Carries all
+/// the str0m-required fields (BUNDLE group, fingerprint, ICE creds, Opus
+/// rtpmap) so `accept_offer` returns `Ok(answer)` and sets `audio_mid`.
+const SDP_OFFER: &str = "\
+v=0\r
+o=- 4593482934 2 IN IP4 127.0.0.1\r
+s=-\r
+t=0 0\r
+a=group:BUNDLE 0\r
+m=audio 9 UDP/TLS/RTP/SAVPF 111\r
+c=IN IP4 0.0.0.0\r
+a=rtcp:9 IN IP4 0.0.0.0\r
+a=ice-ufrag:abcd\r
+a=ice-pwd:abcdefghijklmnopqrstuvwxyz0123456789\r
+a=fingerprint:sha-256 AB:CD:EF:00:11:22:33:44:55:66:77:88:99:AA:BB:CC:DD:EE:FF:00:11:22:33:44:55:66:77:88:99:AA:BB:CC:DD\r
+a=setup:actpass\r
+a=mid:0\r
+a=sendrecv\r
+a=rtpmap:111 opus/48000/2\r
+a=fmtp:111 minptime=10;useinbandfec=1\r
+a=candidate:1 1 UDP 2113667327 192.168.1.42 50000 typ host\r
+";
+
 #[tokio::test]
 async fn post_v1_sessions_returns_a_session_id() {
     let app = rutster::routes::router(AppState::new());
@@ -47,3 +72,70 @@ async fn get_root_serves_html() {
         Some("text/html; charset=utf-8")
     );
 }
+
+/// Adversarial review F2: a second `POST /v1/sessions/:id/offer` must
+/// return `409 Conflict`, **not** panic the handler. The old `assert!`
+/// panicked on a client-reachable path; the fix routes a typed
+/// `RtcSessionError::AlreadyNegotiated` to `409 Conflict` (distinct from
+/// `400 Bad Request` for malformed SDP).
+#[tokio::test]
+async fn double_post_offer_returns_409_conflict_not_panic() {
+    let app = rutster::routes::router(AppState::new());
+
+    // Mint a session, then POST the offer. The first POST should succeed
+    // (200 OK with an SDP answer body) — proves the harness's fixture is
+    // valid and `audio_mid` is now `Some(_)`.
+    let session_id = create_session(&app).await;
+    let first = post_offer(&app, &session_id, SDP_OFFER).await;
+    assert_eq!(
+        first.status(),
+        StatusCode::OK,
+        "first offer must succeed for the test to be meaningful"
+    );
+
+    // Second POST to the same session: F2's bug path. Pre-fix, the handler
+    // task panicked at `assert!(self.audio_mid.is_none(), ...)`; the panic
+    // propagated up through `app.oneshot(...).await` and failed the test as
+    // an error (panicked task), not as a status code. Post-fix, the
+    // handler returns `409 Conflict` cleanly.
+    let second = post_offer(&app, &session_id, SDP_OFFER).await;
+    assert_eq!(
+        second.status(),
+        StatusCode::CONFLICT,
+        "second offer must return 409 Conflict, got {} (body: {:?})",
+        second.status(),
+        axum::body::to_bytes(second.into_body(), 1024).await
+    );
+}
+
+async fn create_session(app: &axum::Router) -> String {
+    let resp = app
+        .clone()
+        .oneshot(
+            Request::builder()
+                .method("POST")
+                .uri("/v1/sessions")
+                .body(Body::empty())
+                .unwrap(),
+        )
+        .await
+        .unwrap();
+    assert_eq!(resp.status(), StatusCode::OK);
+    let body = axum::body::to_bytes(resp.into_body(), 1024).await.unwrap();
+    let v: serde_json::Value = serde_json::from_slice(&body).unwrap();
+    v["session_id"].as_str().unwrap().to_string()
+}
+
+async fn post_offer(app: &axum::Router, session_id: &str, sdp: &str) -> axum::response::Response {
+    app.clone()
+        .oneshot(
+            Request::builder()
+                .method("POST")
+                .uri(format!("/v1/sessions/{session_id}/offer"))
+                .header("content-type", "application/sdp")
+                .body(Body::from(sdp.to_owned()))
+                .unwrap(),
+        )
+        .await
+        .unwrap()
+}