Refuse a state-changing request another page sent

docs/DECISIONS.md

@@ -138,6 +138,38 @@ repeats toward the spread; a floor below three; raising the default
 ceiling when a width is committed; and the model choosing a width
 without a person seeing it before the run.
 
+## 2026-10-03: a state-changing request another page sent is refused before any handler
+
+**Decision:** `auth::refuse_cross_site` runs on every route, inside the
+CORS layer. It passes GET, HEAD and OPTIONS. Any other method passes only
+if `Sec-Fetch-Site` is `same-origin` or `none`, or, from a browser that
+does not send that header, if `Origin` matches `Host`, or if `Origin` is
+named with `--allow-origin`. A request with neither header passes,
+because it did not come from a page: curl, the CLI and the Mac app.
+Everything else gets 403 before a handler runs. First slice of #235.
+
+**Why CORS was not enough.** The 2026-08-20 entry stopped a foreign page
+reading this server's answers. A browser still sends a "simple" request,
+a POST with a `text/plain` body or none, and the loopback install has no
+token. A handler that takes no JSON body ran for any page the person
+visited. The ones that take JSON were safe only because their extractor
+refuses the content type, which is not a rule anybody wrote down.
+
+**Why `Sec-Fetch-Site` and not `Origin` against `Host`.** The container's
+nginx rewrites `Host` to the upstream and attaches the token for anyone
+who reaches it, so neither `Host` nor the token can tell the UI from
+another page there. A page cannot set `Sec-Fetch-Site`, and the UI's own
+requests read `same-origin` through the proxy.
+
+**Another loopback port is refused.** It reads `same-site`, because a site
+ignores ports. A dev server the agent started is another program, and
+#235 is about to put one in a frame next to this page.
+
+**Ruled out:** a token for the loopback install, which is the 2026-08-20
+entry's "larger change" and still is; checking `Origin` against `Host`
+alone (breaks the container UI); and guarding only the routes found to
+take no body, which is the per-handler accident this replaces.
+
 ---
 
 ## 2026-09-19: a pretraining run trains on a real corpus, or it says it did not

zorp-web/src/api.rs

@@ -218,6 +218,10 @@ fn api_router(state: AppState) -> Router {
             state.clone(),
             crate::auth::require_token,
         ))
+        .layer(axum::middleware::from_fn_with_state(
+            state.clone(),
+            crate::auth::refuse_cross_site,
+        ))
         .layer(cors)
         .with_state(state)
 }

zorp-web/src/auth.rs

@@ -39,3 +39,64 @@ pub async fn require_token(
         _ => (StatusCode::UNAUTHORIZED, "missing or wrong token").into_response(),
     }
 }
+
+/// Refuses a state-changing request that a browser says came from another
+/// page.
+///
+/// CORS stops a foreign page from reading what this server answers. It does
+/// not stop the browser from sending a "simple" request, a POST with no
+/// custom header and a `text/plain` body or none at all, and on the ordinary
+/// loopback install there is no token to stop it either. A route whose
+/// handler takes no JSON body would run for any page the person happened to
+/// visit. A route that does take JSON is protected only because its
+/// extractor refuses the content type, which is a side effect and not a
+/// rule.
+///
+/// `Sec-Fetch-Site` is the signal, because a page cannot set it. It reads
+/// `same-origin` for the UI's own requests even through the container's
+/// nginx, which rewrites `Host` and attaches the token for anyone who
+/// reaches it, so neither of those can decide this. It reads `same-site`,
+/// not `same-origin`, for a page on another loopback port, which is
+/// refused: a dev server is another program. A browser too old to send it
+/// falls back to comparing `Origin` with `Host`. A request with neither
+/// header is not from a browser page (curl, the CLI, the Mac app) and
+/// passes. An origin named with `--allow-origin` passes either way.
+pub async fn refuse_cross_site(
+    State(state): State<AppState>,
+    request: Request,
+    next: Next,
+) -> Response {
+    use axum::http::{header, Method};
+    if matches!(
+        *request.method(),
+        Method::GET | Method::HEAD | Method::OPTIONS
+    ) {
+        return next.run(request).await;
+    }
+    let headers = request.headers();
+    let text = |name| headers.get(name).and_then(|v| v.to_str().ok());
+    let origin = text(header::ORIGIN);
+    let named = origin.is_some_and(|o| state.allowed_origins.iter().any(|a| a == o));
+    let allowed = named
+        || match text(header::HeaderName::from_static("sec-fetch-site")) {
+            Some(site) => site == "same-origin" || site == "none",
+            None => match origin {
+                None => true,
+                Some(origin) => text(header::HOST).is_some_and(|host| {
+                    origin
+                        .strip_prefix("http://")
+                        .or_else(|| origin.strip_prefix("https://"))
+                        == Some(host)
+                }),
+            },
+        };
+    if allowed {
+        next.run(request).await
+    } else {
+        (
+            StatusCode::FORBIDDEN,
+            "refusing a request another page sent: only this server's own page may change anything",
+        )
+            .into_response()
+    }
+}

zorp-web/tests/cross_site.rs

@@ -0,0 +1,163 @@
+//! A state-changing request another page sent is refused before any handler
+//! runs.
+//!
+//! CORS keeps a foreign page from reading this server's answers, not from
+//! sending a "simple" request, and the loopback install has no token. So a
+//! route whose handler takes no JSON body would run for any page the person
+//! visited. `POST /api/sessions` is one such route and always answers 200
+//! with an id when it runs, so a 403 here means the handler was never
+//! reached.
+
+use std::net::SocketAddr;
+
+async fn spawn(state: zorp_web::state::AppState) -> SocketAddr {
+    let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
+    let addr = listener.local_addr().unwrap();
+    tokio::spawn(async move {
+        axum::serve(listener, zorp_web::api::router_with_state(state))
+            .await
+            .unwrap();
+    });
+    addr
+}
+
+/// POST `path` with the given headers and a `text/plain` body, the shape a
+/// page can send without a preflight. Returns the status.
+async fn post(addr: SocketAddr, path: &str, headers: &[(&'static str, String)]) -> u16 {
+    let url = format!("http://{addr}{path}");
+    let headers = headers.to_vec();
+    tokio::task::spawn_blocking(move || {
+        let mut request = ureq::post(&url).set("Content-Type", "text/plain");
+        for (name, value) in &headers {
+            request = request.set(name, value);
+        }
+        match request.send_string("x") {
+            Ok(r) => r.status(),
+            Err(ureq::Error::Status(code, _)) => code,
+            Err(e) => panic!("request failed: {e}"),
+        }
+    })
+    .await
+    .unwrap()
+}
+
+#[tokio::test]
+async fn a_page_on_another_site_cannot_create_anything() {
+    let addr = spawn(zorp_web::state::AppState::new()).await;
+    let status = post(
+        addr,
+        "/api/sessions",
+        &[
+            ("Origin", "https://evil.example".into()),
+            ("Sec-Fetch-Site", "cross-site".into()),
+        ],
+    )
+    .await;
+    assert_eq!(status, 403, "a page the person visited created a session");
+}
+
+/// A dev server on another loopback port is the same site but not the same
+/// origin. It is another program, and it is refused.
+#[tokio::test]
+async fn a_page_on_another_loopback_port_is_refused() {
+    let addr = spawn(zorp_web::state::AppState::new()).await;
+    let status = post(
+        addr,
+        "/api/sessions",
+        &[
+            ("Origin", "http://127.0.0.1:5173".into()),
+            ("Sec-Fetch-Site", "same-site".into()),
+        ],
+    )
+    .await;
+    assert_eq!(status, 403);
+}
+
+/// The trust route takes no body at all, which is why it needs this most.
+/// Without the guard an unknown agent answers 404 from inside the handler.
+#[tokio::test]
+async fn trusting_an_agent_from_another_page_never_reaches_the_handler() {
+    let addr = spawn(zorp_web::state::AppState::new()).await;
+    let status = post(
+        addr,
+        "/api/agents/project/anything/trust",
+        &[
+            ("Origin", "https://evil.example".into()),
+            ("Sec-Fetch-Site", "cross-site".into()),
+        ],
+    )
+    .await;
+    assert_eq!(status, 403);
+}
+
+/// The UI's own requests, served by this server or through the container's
+/// nginx, which rewrites `Host`, both say `same-origin`.
+#[tokio::test]
+async fn the_servers_own_page_is_allowed() {
+    let addr = spawn(zorp_web::state::AppState::new()).await;
+    let status = post(
+        addr,
+        "/api/sessions",
+        &[
+            ("Origin", "http://localhost:8080".into()),
+            ("Sec-Fetch-Site", "same-origin".into()),
+        ],
+    )
+    .await;
+    assert_eq!(status, 200);
+}
+
+/// curl, the CLI and the Mac app send neither header, and are not a page.
+#[tokio::test]
+async fn a_request_that_is_not_from_a_page_is_allowed() {
+    let addr = spawn(zorp_web::state::AppState::new()).await;
+    assert_eq!(post(addr, "/api/sessions", &[]).await, 200);
+}
+
+/// A browser too old to send `Sec-Fetch-Site` is judged on `Origin` against
+/// `Host`.
+#[tokio::test]
+async fn without_fetch_metadata_the_origin_must_match_the_host() {
+    let addr = spawn(zorp_web::state::AppState::new()).await;
+    assert_eq!(
+        post(
+            addr,
+            "/api/sessions",
+            &[("Origin", format!("http://{addr}"))]
+        )
+        .await,
+        200
+    );
+    assert_eq!(
+        post(
+            addr,
+            "/api/sessions",
+            &[("Origin", "https://evil.example".into())]
+        )
+        .await,
+        403
+    );
+    assert_eq!(
+        post(addr, "/api/sessions", &[("Origin", "null".into())]).await,
+        403
+    );
+}
+
+/// An origin named with `--allow-origin` is the container split's UI, and
+/// passes whatever the browser says about the site.
+#[tokio::test]
+async fn a_named_origin_is_allowed() {
+    let state =
+        zorp_web::state::AppState::new().with_allowed_origins(vec!["http://localhost:8080".into()]);
+    let addr = spawn(state).await;
+    let status = post(
+        addr,
+        "/api/sessions",
+        &[
+            ("Origin", "http://localhost:8080".into()),
+            ("Sec-Fetch-Site", "same-site".into()),
+        ],
+    )
+    .await;
+    assert_eq!(status, 200);
+}