diff --git a/server-rs/crates/api-server/src/app.rs b/server-rs/crates/api-server/src/app.rs index 14ab3acde..e78583295 100644 --- a/server-rs/crates/api-server/src/app.rs +++ b/server-rs/crates/api-server/src/app.rs @@ -459,6 +459,25 @@ mod tests { ); } + #[tokio::test] + async fn agc_marker_does_not_bypass_authentication_or_change_failure_status() { + let app = build_router(AppState::new(AppConfig::default()).expect("state should build")); + + let response = app + .oneshot( + Request::builder() + .method("GET") + .uri("/api/editor/projects") + .header("x-genarrative-client", "agc") + .body(Body::empty()) + .expect("request should build"), + ) + .await + .expect("request should succeed"); + + assert_eq!(response.status(), StatusCode::UNAUTHORIZED); + } + #[cfg(any())] fn build_internal_creative_agent_app() -> Router { let mut config = AppConfig::default(); diff --git a/server-rs/crates/api-server/src/tracking.rs b/server-rs/crates/api-server/src/tracking.rs index b7d48a6fe..5ab2c66d4 100644 --- a/server-rs/crates/api-server/src/tracking.rs +++ b/server-rs/crates/api-server/src/tracking.rs @@ -131,13 +131,10 @@ pub async fn record_route_tracking_event_after_success( external_principal: Option<&ExternalApiPrincipal>, client_marker: Option, ) { - if !status.is_success() { - return; - } let Some(spec) = resolve_route_tracking_spec(method, path) else { return; }; - if spec.handled_by_existing_event { + if !should_record_route_tracking(status, &spec) { return; } @@ -161,6 +158,10 @@ pub async fn record_route_tracking_event_after_success( record_route_tracking_event_via_outbox_after_success(state, request_context, draft).await; } +fn should_record_route_tracking(status: StatusCode, spec: &RouteTrackingSpec) -> bool { + status.is_success() && !spec.handled_by_existing_event +} + fn resolve_tracking_identity( authenticated: Option<&AuthenticatedAccessToken>, external_principal: Option<&ExternalApiPrincipal>, @@ -1179,7 +1180,7 @@ mod tests { TrackingClientMarker, TrackingEventDraft, build_route_tracking_metadata, build_tracking_event_input, is_route_tracking_excluded, normalize_route_path, resolve_route_tracking_spec, resolve_tracking_client_marker, resolve_tracking_identity, - resolve_tracking_scope_id, route_spec, + resolve_tracking_scope_id, route_spec, should_record_route_tracking, }; fn build_test_authenticated(user_id: &str) -> AuthenticatedAccessToken { @@ -1397,6 +1398,103 @@ mod tests { assert_eq!(metadata["status"], 200); } + #[test] + fn successful_statuses_record_only_explicit_business_routes() { + let business_spec = route_spec( + "editor_projects_view", + "editor", + module_runtime::RuntimeTrackingScopeKind::User, + "anonymous", + ); + let handled_spec = super::manual_asset_route_spec("asset_upload_confirm"); + + for status in [ + axum::http::StatusCode::OK, + axum::http::StatusCode::CREATED, + axum::http::StatusCode::ACCEPTED, + axum::http::StatusCode::NO_CONTENT, + ] { + assert!( + should_record_route_tracking(status, &business_spec), + "{status} should record a successful business route" + ); + } + assert!(!should_record_route_tracking( + axum::http::StatusCode::OK, + &handled_spec + )); + } + + #[test] + fn client_marker_does_not_change_failure_status_or_excluded_route_semantics() { + let business_spec = resolve_route_tracking_spec( + &Method::POST, + "/api/external/v1/editor/images/generations", + ) + .expect("External v1 business route should resolve"); + for status in [ + axum::http::StatusCode::BAD_REQUEST, + axum::http::StatusCode::UNAUTHORIZED, + axum::http::StatusCode::FORBIDDEN, + axum::http::StatusCode::NOT_FOUND, + axum::http::StatusCode::INTERNAL_SERVER_ERROR, + axum::http::StatusCode::BAD_GATEWAY, + ] { + assert!( + !should_record_route_tracking(status, &business_spec), + "{status} must not create a successful route event" + ); + } + assert!( + resolve_route_tracking_spec(&Method::POST, "/api/external/v1/mcp").is_none(), + "AGC marker must not turn MCP into a business route" + ); + } + + #[test] + fn route_metadata_contains_no_authentication_or_secret_fields() { + let spec = route_spec( + "editor_projects_view", + "editor", + module_runtime::RuntimeTrackingScopeKind::User, + "anonymous", + ); + let request_context = RequestContext::new( + "request-225-safe-metadata".to_string(), + "GET /api/editor/projects".to_string(), + Duration::ZERO, + false, + ); + let metadata = build_route_tracking_metadata( + &spec, + &request_context, + &Method::GET, + "/api/editor/projects", + axum::http::StatusCode::OK, + Some(TrackingClientMarker::Agc), + ); + let object = metadata + .as_object() + .expect("route metadata should be an object"); + + for forbidden_key in [ + "authorization", + "accessToken", + "token", + "apiKey", + "cookie", + "signature", + "signedUrl", + "requestBody", + ] { + assert!( + !object.contains_key(forbidden_key), + "route metadata must not contain {forbidden_key}" + ); + } + assert_eq!(metadata["client"], "agc"); + } + #[test] fn route_normalization_preserves_static_segments_and_replaces_ids() { assert_eq!(