fix(observe): authorize OTS trajectory endpoints (ARN-187)

crates/temper-server/src/observe/evolution/trajectories.rs

@@ -221,6 +221,21 @@ pub(crate) async fn handle_post_ots_trajectory(
     headers: HeaderMap,
     body: String,
 ) -> Result<StatusCode, (StatusCode, String)> {
+    require_observe_auth(&state, &headers, "write_trajectories", "Trajectory")
+        .map_err(|sc| (sc, "unauthorized".to_string()))?;
+    // Resolve the target tenant through the shared scope helper (never the raw
+    // header): non-admins without a valid X-Tenant-Id in multi-tenant mode are
+    // rejected, and cross-tenant writes are already gated by the Cedar check
+    // above (which authorizes against the requested tenant's policies). `None`
+    // (admin/system cross-tenant or single-tenant compat) maps to the default
+    // tenant, matching prior behavior.
+    let tenant_scope =
+        observe_tenant_scope(&state, &headers).map_err(|sc| (sc, "unauthorized".to_string()))?;
+    let tenant = tenant_scope
+        .as_ref()
+        .map(|t| t.as_str())
+        .unwrap_or("default");
+
     // Parse the OTS trajectory JSON to extract indexed fields.
     let trajectory: serde_json::Value = serde_json::from_str(&body)
         .map_err(|e| (StatusCode::BAD_REQUEST, format!("invalid JSON: {e}")))?;
@@ -260,11 +275,6 @@ pub(crate) async fn handle_post_ots_trajectory(
         .map(|a| a.len() as i64)
         .unwrap_or(0);
 
-    let tenant = headers
-        .get("X-Tenant-Id")
-        .and_then(|v| v.to_str().ok())
-        .unwrap_or("default");
-
     let Some(store) = state.metadata_store_for_tenant(tenant).await else {
         tracing::warn!(
             tenant = %tenant,
@@ -328,9 +338,15 @@ pub(crate) async fn handle_get_ots_trajectories(
     headers: HeaderMap,
     Query(params): Query<OtsTrajectoryQueryParams>,
 ) -> Result<Json<serde_json::Value>, StatusCode> {
-    let tenant = headers
-        .get("X-Tenant-Id")
-        .and_then(|v| v.to_str().ok())
+    require_observe_auth(&state, &headers, "read_trajectories", "Trajectory")?;
+    // Resolve the tenant through the shared scope helper (never the raw header),
+    // matching `handle_trajectories`. `None` (admin/system cross-tenant or
+    // single-tenant compat) maps to the default tenant, since OTS listing is
+    // per-tenant by construction (`list_ots_trajectories` filters on tenant).
+    let tenant_scope = observe_tenant_scope(&state, &headers)?;
+    let tenant = tenant_scope
+        .as_ref()
+        .map(|t| t.as_str())
         .unwrap_or("default");
     let limit = params.limit.unwrap_or(50).min(500);
 
@@ -366,3 +382,7 @@ pub(crate) async fn handle_get_ots_trajectories(
         }
     }
 }
+
+#[cfg(test)]
+#[path = "trajectories_test.rs"]
+mod tests;

crates/temper-server/src/observe/evolution/trajectories_test.rs

@@ -0,0 +1,266 @@
+//! Security regression tests for the OTS trajectory endpoints (ARN-187).
+//!
+//! The OTS list/ingest handlers (`GET`/`POST /api/ots/trajectories`) previously
+//! read the raw `X-Tenant-Id` header and selected that tenant's store **without
+//! any authorization check**, so any caller could read another tenant's full
+//! agent-execution traces or forge writes into any tenant simply by setting a
+//! header. These tests lock in the gate: both handlers now call
+//! `require_observe_auth` and resolve the tenant via `observe_tenant_scope`,
+//! matching the sibling `handle_trajectories` / `handle_unmet_intent` handlers.
+//!
+//! NOTE: these tests live behind the `observe` feature (the whole `observe`/
+//! `api` module tree is `#[cfg(feature = "observe")]`), so they only compile
+//! and run with `--features observe` (or `cargo test --workspace`, where
+//! `temper-cli`/`temper-mcp` turn the feature on). A bare
+//! `cargo test -p temper-server` silently filters them out.
+
+use axum::Router;
+use axum::body::Body;
+use axum::http::{Request, StatusCode};
+use tower::ServiceExt;
+
+use temper_runtime::ActorSystem;
+
+use crate::ServerState;
+use crate::registry::SpecRegistry;
+
+/// Enforcing baseline policy: only `Admin` principals are permitted anything.
+/// Non-admin (Customer/Agent) principals are denied unless a per-tenant policy
+/// grants them access. Production never runs the permissive engine, so we swap
+/// it out here to exercise the real deny path.
+const ADMIN_ONLY_POLICY: &str = r#"permit(principal is Admin, action, resource);"#;
+
+/// A minimal, well-formed OTS trajectory payload.
+const SAMPLE_TRAJECTORY: &str =
+    r#"{"metadata":{"trajectory_id":"traj-1","agent_id":"a1","outcome":"success"},"turns":[]}"#;
+
+/// Build a `ServerState` whose Cedar engine actually enforces (admin-only
+/// baseline) instead of the permissive test default.
+fn enforcing_state() -> ServerState {
+    let system = ActorSystem::new("test-ots-auth");
+    let state = ServerState::from_registry(system, SpecRegistry::new());
+    state
+        .authz
+        .reload_policies(ADMIN_ONLY_POLICY)
+        .expect("baseline policy should parse");
+    state
+}
+
+/// Mount the management API (which owns `/ots/trajectories`) under `/api`.
+fn app(state: ServerState) -> Router {
+    Router::new()
+        .nest("/api", crate::api::build_api_router())
+        .with_state(state)
+}
+
+/// Exploit (a): an anonymous caller spoofs a victim tenant via `X-Tenant-Id`
+/// and reads its OTS traces. Before the fix this returned `200` with the
+/// victim tenant's data; it must now be denied.
+#[tokio::test]
+async fn unauthenticated_get_ots_trajectories_is_denied() {
+    let app = app(enforcing_state());
+    let resp = app
+        .oneshot(
+            Request::get("/api/ots/trajectories")
+                .header("X-Tenant-Id", "victim-tenant")
+                .body(Body::empty())
+                .unwrap(),
+        )
+        .await
+        .unwrap();
+    assert_eq!(
+        resp.status(),
+        StatusCode::FORBIDDEN,
+        "anonymous cross-tenant OTS read must be denied (was 200 before ARN-187 fix)"
+    );
+}
+
+/// Exploit (b): an anonymous caller forges an OTS trajectory into a victim
+/// tenant. Before the fix this returned `202/201`; it must now be denied.
+#[tokio::test]
+async fn unauthenticated_post_ots_trajectory_is_denied() {
+    let app = app(enforcing_state());
+    let resp = app
+        .oneshot(
+            Request::post("/api/ots/trajectories")
+                .header("content-type", "application/json")
+                .header("X-Tenant-Id", "victim-tenant")
+                .body(Body::from(SAMPLE_TRAJECTORY))
+                .unwrap(),
+        )
+        .await
+        .unwrap();
+    assert_eq!(
+        resp.status(),
+        StatusCode::FORBIDDEN,
+        "anonymous forged OTS write must be denied (was accepted before ARN-187 fix)"
+    );
+}
+
+/// A properly-authorized (admin) same-tenant GET still succeeds — the gate must
+/// not over-block legitimate callers.
+#[tokio::test]
+async fn authorized_admin_get_ots_trajectories_succeeds() {
+    let app = app(enforcing_state());
+    let resp = app
+        .oneshot(
+            Request::get("/api/ots/trajectories")
+                .header("X-Temper-Principal-Kind", "admin")
+                .header("X-Tenant-Id", "tenant-a")
+                .body(Body::empty())
+                .unwrap(),
+        )
+        .await
+        .unwrap();
+    assert_eq!(
+        resp.status(),
+        StatusCode::OK,
+        "authorized admin read should still succeed"
+    );
+}
+
+/// A properly-authorized (admin) same-tenant POST still succeeds.
+#[tokio::test]
+async fn authorized_admin_post_ots_trajectory_succeeds() {
+    let app = app(enforcing_state());
+    let resp = app
+        .oneshot(
+            Request::post("/api/ots/trajectories")
+                .header("content-type", "application/json")
+                .header("X-Temper-Principal-Kind", "admin")
+                .header("X-Tenant-Id", "tenant-a")
+                .body(Body::from(SAMPLE_TRAJECTORY))
+                .unwrap(),
+        )
+        .await
+        .unwrap();
+    // With no durable metadata store configured the handler short-circuits to
+    // `201 Created`; the point is that an authorized write is *not* forbidden.
+    assert_eq!(
+        resp.status(),
+        StatusCode::CREATED,
+        "authorized admin write should still succeed"
+    );
+}
+
+/// Cross-tenant isolation: a credential scoped to `tenant-a` can read its own
+/// trajectories but MUST NOT read `tenant-b`'s by spoofing the header.
+#[tokio::test]
+async fn tenant_scoped_credential_cannot_read_other_tenant() {
+    let system = ActorSystem::new("test-ots-cross-tenant");
+    let state = ServerState::from_registry(system, SpecRegistry::new());
+    state
+        .authz
+        .reload_policies(ADMIN_ONLY_POLICY)
+        .expect("baseline policy should parse");
+    // `tenant-a` grants its agents read access to trajectories. `tenant-b`
+    // grants this principal nothing (falls back to the admin-only baseline).
+    state
+        .authz
+        .reload_tenant_policies(
+            "tenant-a",
+            r#"permit(principal is Agent, action == Action::"read_trajectories", resource is Trajectory);"#,
+        )
+        .expect("tenant-a policy should parse");
+
+    let app = app(state);
+
+    // Same-tenant read (tenant-a agent → tenant-a) succeeds.
+    let same_tenant = app
+        .clone()
+        .oneshot(
+            Request::get("/api/ots/trajectories")
+                .header("X-Temper-Principal-Kind", "agent")
+                .header("X-Temper-Principal-Id", "agent-a")
+                .header("X-Tenant-Id", "tenant-a")
+                .body(Body::empty())
+                .unwrap(),
+        )
+        .await
+        .unwrap();
+    assert_eq!(
+        same_tenant.status(),
+        StatusCode::OK,
+        "same-tenant authorized read should succeed"
+    );
+
+    // Cross-tenant read (tenant-a agent → tenant-b) is denied.
+    let cross_tenant = app
+        .oneshot(
+            Request::get("/api/ots/trajectories")
+                .header("X-Temper-Principal-Kind", "agent")
+                .header("X-Temper-Principal-Id", "agent-a")
+                .header("X-Tenant-Id", "tenant-b")
+                .body(Body::empty())
+                .unwrap(),
+        )
+        .await
+        .unwrap();
+    assert_eq!(
+        cross_tenant.status(),
+        StatusCode::FORBIDDEN,
+        "tenant-a credential must not read tenant-b trajectories"
+    );
+}
+
+/// Cross-tenant isolation on the write path: a credential scoped to `tenant-a`
+/// can ingest its own trajectories but MUST NOT forge writes into `tenant-b`.
+#[tokio::test]
+async fn tenant_scoped_credential_cannot_write_other_tenant() {
+    let system = ActorSystem::new("test-ots-cross-tenant-write");
+    let state = ServerState::from_registry(system, SpecRegistry::new());
+    state
+        .authz
+        .reload_policies(ADMIN_ONLY_POLICY)
+        .expect("baseline policy should parse");
+    // `tenant-a` grants its agents write access; `tenant-b` grants this
+    // principal nothing (falls back to the admin-only baseline).
+    state
+        .authz
+        .reload_tenant_policies(
+            "tenant-a",
+            r#"permit(principal is Agent, action == Action::"write_trajectories", resource is Trajectory);"#,
+        )
+        .expect("tenant-a policy should parse");
+
+    let app = app(state);
+
+    // Same-tenant write (tenant-a agent → tenant-a) succeeds.
+    let same_tenant = app
+        .clone()
+        .oneshot(
+            Request::post("/api/ots/trajectories")
+                .header("content-type", "application/json")
+                .header("X-Temper-Principal-Kind", "agent")
+                .header("X-Temper-Principal-Id", "agent-a")
+                .header("X-Tenant-Id", "tenant-a")
+                .body(Body::from(SAMPLE_TRAJECTORY))
+                .unwrap(),
+        )
+        .await
+        .unwrap();
+    assert_eq!(
+        same_tenant.status(),
+        StatusCode::CREATED,
+        "same-tenant authorized write should succeed"
+    );
+
+    // Cross-tenant write (tenant-a agent → tenant-b) is denied.
+    let cross_tenant = app
+        .oneshot(
+            Request::post("/api/ots/trajectories")
+                .header("content-type", "application/json")
+                .header("X-Temper-Principal-Kind", "agent")
+                .header("X-Temper-Principal-Id", "agent-a")
+                .header("X-Tenant-Id", "tenant-b")
+                .body(Body::from(SAMPLE_TRAJECTORY))
+                .unwrap(),
+        )
+        .await
+        .unwrap();
+    assert_eq!(
+        cross_tenant.status(),
+        StatusCode::FORBIDDEN,
+        "tenant-a credential must not write tenant-b trajectories"
+    );
+}