diff --git a/crates/worker/src/feature/builtin/ticket.rs b/crates/worker/src/feature/builtin/ticket.rs index b08da1f2c..e8fc7e1dc 100644 --- a/crates/worker/src/feature/builtin/ticket.rs +++ b/crates/worker/src/feature/builtin/ticket.rs @@ -293,9 +293,9 @@ impl Tool for WorkspaceTicketReadTool { )) .map_err(|error| ToolError::ExecutionFailed(error.to_string()))?; if !response.is_success() { - return Err(ToolError::ExecutionFailed(format!( - "Workspace Ticket API request failed with HTTP status {}", - response.status + return Err(ToolError::ExecutionFailed(ticket_api_error_message( + response.status, + &response.body, ))); } let response_value: Value = serde_json::from_str(&response.body).map_err(|error| { @@ -646,9 +646,9 @@ impl TicketFeatureAccess { } fn allows_tool(self, name: &str) -> bool { - // Publishing a decision tool is not an authority grant. The injected - // Workspace transport lets the Backend validate the current assignment - // or author/orchestrator permission on every decision, in both modes. + // Profile tool policy controls case-decision access in both modes. + // The Backend verifies actor/Workspace identity and CAS/replay, not + // orchestration assignment as an additional case-decision permission. READ_ONLY_TOOL_NAMES.contains(&name) || (name == "TicketCaseUpdate" && (self.authoring || self.thread || self.workflow)) || (self.authoring && AUTHORING_TOOL_NAMES.contains(&name)) @@ -921,9 +921,9 @@ impl WorkspaceHttpTicketBackend { TicketError::Conflict(format!("ticket REST request failed: {error}")) })?; if !response.is_success() { - return Err(TicketError::Conflict(format!( - "ticket REST API request failed with HTTP status {}", - response.status + return Err(TicketError::Conflict(ticket_api_error_message( + response.status, + &response.body, ))); } serde_json::from_str(&response.body) @@ -997,9 +997,9 @@ impl WorkspaceHttpTicketBackend { TicketError::Conflict(format!("ticket REST request failed: {error}")) })?; if !response.is_success() { - return Err(TicketError::Conflict(format!( - "ticket REST API request failed with HTTP status {}", - response.status + return Err(TicketError::Conflict(ticket_api_error_message( + response.status, + &response.body, ))); } Ok(TicketBackendOperationResult::Unit) @@ -2100,6 +2100,56 @@ fn authorize_native_ticket( } } +// Only render the public API error envelope, never an arbitrary proxy body or +// unknown fields. Backend owns redaction of its public message; bound it here +// and keep diagnostics to stable codes rather than raw provider details. +fn ticket_api_error_message(status: u16, body: &str) -> String { + let mut message = format!("ticket REST API request failed with HTTP status {status}"); + if body.len() > 64 * 1024 { + return message; + } + let Ok(error) = serde_json::from_str::(body) else { + return message; + }; + let detail = if error.message.trim().is_empty() { + &error.error + } else { + &error.message + }; + let mut chars = detail.trim().chars(); + let bounded: String = chars + .by_ref() + .take(1024) + .map(|c| if c.is_control() { ' ' } else { c }) + .collect(); + if !bounded.is_empty() { + message.push_str(": "); + message.push_str(&bounded); + if chars.next().is_some() { + message.push('…'); + } + } + let codes: Vec<_> = error + .diagnostics + .iter() + .take(4) + .map(|diagnostic| diagnostic.code.as_str()) + .filter(|code| { + !code.is_empty() + && code.len() <= 64 + && code + .bytes() + .all(|c| c.is_ascii_alphanumeric() || matches!(c, b'_' | b'-' | b'.')) + }) + .collect(); + if !codes.is_empty() { + message.push_str(" ["); + message.push_str(&codes.join(", ")); + message.push(']'); + } + message +} + fn map_ticket_tool_error(error: ToolError) -> WipOperationError { match error { ToolError::InvalidArgument(message) => { @@ -2112,17 +2162,19 @@ fn map_ticket_tool_error(error: ToolError) -> WipOperationError { format!("{code}: {message}"), ), ToolError::ExecutionFailed(message) => { - if message.contains("HTTP status 401") || message.contains("HTTP status 403") { - protocol_failure(ProtocolErrorCode::PermissionDenied, message) - } else if message.contains("HTTP status 404") { - protocol_failure(ProtocolErrorCode::NotFound, message) - } else if message.contains("HTTP status 400") - || message.contains("HTTP status 409") - || message.contains("HTTP status 422") - { - protocol_failure(ProtocolErrorCode::InvalidArguments, message) - } else { - WipOperationError::OutcomeUnknown(message) + // Parse the transport status before the public detail. A message + // mentioning a different HTTP status must not change classification. + let status = message + .split_once("HTTP status ") + .and_then(|(_, tail)| tail.split(|c: char| c == ':' || c.is_whitespace()).next()) + .and_then(|status| status.parse::().ok()); + match status { + Some(401 | 403) => protocol_failure(ProtocolErrorCode::PermissionDenied, message), + Some(404) => protocol_failure(ProtocolErrorCode::NotFound, message), + Some(400 | 409 | 422) => { + protocol_failure(ProtocolErrorCode::InvalidArguments, message) + } + _ => WipOperationError::OutcomeUnknown(message), } } ToolError::Internal(message) => WipOperationError::OutcomeUnknown(message), @@ -2462,6 +2514,135 @@ mod tests { }) } + #[tokio::test] + async fn ticket_api_errors_keep_public_details_in_tools_and_wip() { + for (status, code) in [ + (403, ProtocolErrorCode::PermissionDenied), + (404, ProtocolErrorCode::NotFound), + (409, ProtocolErrorCode::InvalidArguments), + ] { + let client = Arc::new(DecisionClient { + requests: Mutex::new(Vec::new()), + response: crate::worker::WorkspaceResponse { + status, + body: json!({ + "error": "Rejected", + "message": "Verified actor is required; unrelated HTTP status 401", + "diagnostics": [{"code":"ticket_actor_required", "severity":"error", "message":"provider-private-detail"}] + }).to_string(), + }, + }); + let (_, tool) = workspace_ticket_decision_definition( + client.clone(), + WorkspaceTicketDecisionKind::CaseUpdate, + )(); + let error = tool + .execute( + &decision_input().to_string(), + ToolExecutionContext::direct(), + ) + .await + .unwrap_err(); + let text = error.to_string(); + assert!(text.contains(&format!("HTTP status {status}")), "{text}"); + assert!(text.contains("Verified actor is required"), "{text}"); + assert!(text.contains("ticket_actor_required"), "{text}"); + assert!(!text.contains("provider-private-detail")); + assert!( + matches!(map_ticket_tool_error(error), WipOperationError::Protocol(ProtocolError {code: actual, ..}) if actual == code) + ); + + // Unit-returning mutations and read tools share the same formatter. + let error = WorkspaceHttpTicketBackend::request_unit( + client.clone(), + WorkspaceRequestMethod::Post, + "/test".into(), + None, + ) + .unwrap_err(); + assert!(error.to_string().contains("Verified actor is required")); + let (_, read) = + workspace_ticket_read_definition(client.clone(), WorkspaceTicketReadKind::Show)(); + let error = read + .execute(r#"{"id":"T-739"}"#, ToolExecutionContext::direct()) + .await + .unwrap_err(); + assert!(error.to_string().contains("ticket_actor_required")); + + let tools = + native_ticket_tools(client.clone(), TicketFeatureAccess::workspace_authoring()) + .unwrap(); + let error = execute_native_ticket_tool( + tools + .iter() + .find(|tool| tool.name == "TicketCaseUpdate") + .unwrap(), + &None, + decision_input(), + WipCallContext { + execution: ToolExecutionContext::direct(), + security_context: String::new(), + }, + ) + .await + .unwrap_err(); + assert!( + matches!(error, WipOperationError::Protocol(ProtocolError {code: actual, ref message}) if actual == code && message.contains("Verified actor is required")) + ); + } + } + + #[test] + fn ticket_api_errors_bound_public_fields_and_ignore_unstructured_bodies() { + let prefix = "ticket REST API request failed with HTTP status 502"; + for body in [ + "proxy-private-detail".into(), + "not JSON".into(), + json!({"message":"not the public envelope", "token":"private"}).to_string(), + json!({"error":"Rejected", "message":"detail", "token":"private"}).to_string(), + "x".repeat(64 * 1024 + 1), + ] { + assert_eq!(ticket_api_error_message(502, &body), prefix); + } + let body = + json!({"error":"Rejected", "message":format!("line\n\u{1b}{}", "あ".repeat(2000)), + "diagnostics":[{"code":"/private/path", "severity":"error", "message":"secret"}] }) + .to_string(); + let message = ticket_api_error_message(502, &body); + assert!(message.ends_with('…')); + assert!(message.len() < 4096); + assert!(!message.chars().any(char::is_control)); + assert!(!message.contains("secret") && !message.contains("/private/path")); + let message = ticket_api_error_message( + 502, + &json!({"error":"Bad Gateway", "message":"upstream reported HTTP status 403"}) + .to_string(), + ); + assert!(matches!( + map_ticket_tool_error(ToolError::ExecutionFailed(message)), + WipOperationError::OutcomeUnknown(_) + )); + } + + #[test] + fn case_tool_availability_is_profile_controlled_in_both_interfaces() { + for (access, allowed) in [ + (TicketFeatureAccess::read_only(), false), + (TicketFeatureAccess::review(), false), + (TicketFeatureAccess::workspace_authoring(), true), + (TicketFeatureAccess::work_report(), true), + (TicketFeatureAccess::workflow(), true), + ] { + assert_eq!(access.tool_names().contains(&"TicketCaseUpdate"), allowed); + let tools = + native_ticket_tools(decision_client(200, TicketState::Open), access).unwrap(); + assert_eq!( + tools.iter().any(|tool| tool.name == "TicketCaseUpdate"), + allowed + ); + } + } + #[test] fn case_backend_sends_explicit_judgment_without_domain_author_claim() { let client = decision_client(200, TicketState::Closed); @@ -3282,6 +3463,15 @@ mod tests { WipOperationError::Protocol(ProtocolError { code: actual, .. }) if actual == code )); } + // Assignment tools use a separate adapter but share this WIP classifier. + assert!(matches!( + map_ticket_tool_error(ToolError::ExecutionFailed("Assignment request failed with HTTP status 403: denied; HTTP status 409 is unrelated".into())), + WipOperationError::Protocol(ProtocolError { code: ProtocolErrorCode::PermissionDenied, .. }) + )); + assert!(matches!( + map_ticket_tool_error(ToolError::ExecutionFailed("Assignment tracking request failed with HTTP status 409: stale; HTTP status 403 is unrelated".into())), + WipOperationError::Protocol(ProtocolError { code: ProtocolErrorCode::InvalidArguments, .. }) + )); assert!(matches!( map_ticket_tool_error(ToolError::ExecutionFailed("transport disconnected".into())), WipOperationError::OutcomeUnknown(_) diff --git a/crates/workspace-server/src/server.rs b/crates/workspace-server/src/server.rs index 4f939d7f8..40ab8fc75 100644 --- a/crates/workspace-server/src/server.rs +++ b/crates/workspace-server/src/server.rs @@ -12486,7 +12486,9 @@ async fn log_failed_api_response(request: Request, next: Next) -> Response { let response = next.run(request).await; let status = response.status(); - if uri.path().starts_with("/api/") && (status.is_client_error() || status.is_server_error()) { + let is_api = uri.path().starts_with("/api/") + || canonical_ticket_operation_workspace_id(uri.path()).is_some(); + if is_api && (status.is_client_error() || status.is_server_error()) { // Generated routes bypass ApiError::into_response; inspect their typed error, not the body. let generated_error = response .extensions() @@ -16038,23 +16040,6 @@ fn execute_ticket_rest_operation_on_host( } validate_ticket_repository_operation(api, &operation)?; let before = target.as_ref().and_then(|id| backend.show(id.clone()).ok()); - if let Some(source) = source.as_ref() - && (generic_ticket_state_change(&operation).is_some() - || matches!( - operation, - TicketBackendOperation::QueueReady { .. } - | TicketBackendOperation::MarkReady { .. } - )) - { - let context = worker_ticket_source_context(api, workspace_id, source, before.as_ref()); - let is_orchestrator = - find_workspace_orchestrator(api).is_some_and(|worker| worker.worker == *source); - if context.actor_role != "ticket_worker" && !is_orchestrator { - return Err(Error::WorkspacePermissionDenied( - "Ticket state decisions require the assigned Worker or registered Workspace Orchestrator".into(), - ).into()); - } - } let mut ticket_item_check = source .as_ref() .cloned() @@ -18291,67 +18276,66 @@ async fn apply_ticket_case_update( Json(mut request): Json, ) -> ApiResult> { let actor = actor.cloned(); - api.host_executors.host()?.run(move || { - let actor = actor.as_ref(); + api.host_executors + .host()? + .run(move || { + let actor = actor.as_ref(); - validate_workspace_scope(&api, &workspace_id)?; - let ticket_id = resolve_workspace_ticket_reference(&api, &workspace_id, &ticket_id)?; - let mut backend = browser_ticket_backend(&api)?; - let before = backend - .show(TicketIdOrSlug::Id(ticket_id.clone())) - .map_err(Error::from)?; - if let Some(source) = source { - let author = format!("worker:{}/{}", source.runtime_id, source.worker_id); - let replay_event = before.events.iter().find(|event| { - event.author.as_deref() == Some(author.as_str()) - && event.attributes.get("operation_key") == Some(&request.operation_key) - && event.attributes.get("source_runtime_id") == Some(&source.runtime_id) - && event.attributes.get("source_worker_id") == Some(&source.worker_id) - }); - let replay = replay_event.is_some(); - let context = worker_ticket_source_context(&api, &workspace_id, &source, Some(&before)); - let is_orchestrator = - find_workspace_orchestrator(&api).is_some_and(|worker| worker.worker == source); - if !replay && context.actor_role != "ticket_worker" && !is_orchestrator { - return Err(Error::WorkspacePermissionDenied( - "Ticket state decisions require the assigned Worker or registered Workspace Orchestrator".into(), - ).into()); - } - request.author = Some(author); - let attributes = if let Some(event) = replay_event { - // Replay authenticates the same actor, but retains the receipt's - // original work identity. Completion/reopen never revives that grant. - event - .attributes - .iter() - .filter(|(key, _)| key.starts_with("source_")) - .map(|(key, value)| (key.clone(), value.clone())) - .collect() - } else { - context.attributes("case_update") - }; - backend = backend.with_event_attributes(attributes); - } else if let Some(actor) = actor { - // Include both verified IDs in the author (and thus the receipt fingerprint). - // JSON tuple encoding avoids ambiguous identity concatenation; proof tokens, - // auth method and signature timestamps are not replay identity. - let identity = serde_json::to_string(&(&actor.account_id, &actor.user_id)) - .map_err(|error| Error::Config(format!("failed to encode case actor: {error}")))?; - request.author = Some(format!("workspace-user:{identity}")); - backend = backend.with_event_attributes(BTreeMap::from([ - ("source_account_id".into(), actor.account_id.clone()), - ("source_user_id".into(), actor.user_id.clone()), - ])); - } else { - // Only the test-only header adapter can reach this legacy attribution. - request.author = Some("workspace-user".into()); - } - let ticket = backend - .update_case(&ticket_id, request) - .map_err(Error::from)?; - Ok(Json(ticket)) - - }).await + validate_workspace_scope(&api, &workspace_id)?; + let ticket_id = resolve_workspace_ticket_reference(&api, &workspace_id, &ticket_id)?; + let mut backend = browser_ticket_backend(&api)?; + let before = backend + .show(TicketIdOrSlug::Id(ticket_id.clone())) + .map_err(Error::from)?; + if let Some(source) = source { + let author = format!("worker:{}/{}", source.runtime_id, source.worker_id); + let replay_event = before.events.iter().find(|event| { + event.author.as_deref() == Some(author.as_str()) + && event.attributes.get("operation_key") == Some(&request.operation_key) + && event.attributes.get("source_runtime_id") == Some(&source.runtime_id) + && event.attributes.get("source_worker_id") == Some(&source.worker_id) + }); + // Profile tool policy authorizes this operation at the Runtime boundary. + // Assignment is audit context, not an additional case-decision permission. + let context = + worker_ticket_source_context(&api, &workspace_id, &source, Some(&before)); + request.author = Some(author); + let attributes = if let Some(event) = replay_event { + // Replay authenticates the same actor, but retains the receipt's + // original work identity. Completion/reopen never revives that grant. + event + .attributes + .iter() + .filter(|(key, _)| key.starts_with("source_")) + .map(|(key, value)| (key.clone(), value.clone())) + .collect() + } else { + context.attributes("case_update") + }; + backend = backend.with_event_attributes(attributes); + } else if let Some(actor) = actor { + // Include both verified IDs in the author (and thus the receipt fingerprint). + // JSON tuple encoding avoids ambiguous identity concatenation; proof tokens, + // auth method and signature timestamps are not replay identity. + let identity = serde_json::to_string(&(&actor.account_id, &actor.user_id)) + .map_err(|error| { + Error::Config(format!("failed to encode case actor: {error}")) + })?; + request.author = Some(format!("workspace-user:{identity}")); + backend = backend.with_event_attributes(BTreeMap::from([ + ("source_account_id".into(), actor.account_id.clone()), + ("source_user_id".into(), actor.user_id.clone()), + ])); + } else { + // Only the test-only header adapter can reach this legacy attribution. + request.author = Some("workspace-user".into()); + } + let ticket = backend + .update_case(&ticket_id, request) + .map_err(Error::from)?; + Ok(Json(ticket)) + }) + .await } fn reject_non_browser_reopen_auth(headers: &HeaderMap) -> Result<()> { @@ -51528,7 +51512,7 @@ mod tests { } #[tokio::test] - async fn ticket_decisions_reject_other_workers_and_browser_route_source_bypasses() { + async fn ticket_decisions_allow_verified_workers_without_target_assignment() { let dir = tempfile::tempdir().unwrap(); let api = test_api(dir.path()).await; let backend = browser_ticket_backend(&api).unwrap(); @@ -51545,23 +51529,26 @@ mod tests { }, expected_closure_operation_key: None, author: None, - operation_key: "forged".into(), + operation_key: "authorized-case-decision".into(), expected_content_digest: ticket::ticket_content_digest(&before), expected_state: TicketState::Open, - explanation: "Not my task".into(), + explanation: "Authorized case decision independent of assignment".into(), references: Vec::new(), }; - let response = scoped_ticket_case_update( + let Json(closed) = scoped_ticket_case_update( State(api.clone()), ticket_check_headers(&other), AxumPath((TEST_WORKSPACE_ID.into(), ticket.id.clone())), Json(request), ) .await - .unwrap_err() - .into_response(); - assert_eq!(response.status(), StatusCode::FORBIDDEN); - assert_eq!(backend.show(ticket.id.into()).unwrap(), before); + .unwrap(); + assert_eq!(closed.meta.workflow_state, TicketState::Closed); + assert_eq!( + closed.events.last().unwrap().attributes["source_actor_role"], + "worker" + ); + assert_eq!(backend.show(ticket.id.into()).unwrap(), closed); } #[tokio::test] diff --git a/crates/workspace-server/src/server/tests/api_logging_tests.rs b/crates/workspace-server/src/server/tests/api_logging_tests.rs index 2bac1950b..58d1bd4a3 100644 --- a/crates/workspace-server/src/server/tests/api_logging_tests.rs +++ b/crates/workspace-server/src/server/tests/api_logging_tests.rs @@ -919,3 +919,91 @@ async fn saturated_host_before_command_start_returns_availability_without_a_prov host.drain().await.unwrap(); fixture.api.store.db_executor().drain().await.unwrap(); } + +#[tokio::test] +async fn canonical_ticket_case_errors_are_logged_without_request_secrets() { + let dir = tempfile::tempdir().unwrap(); + let mut api = test_api(dir.path()).await; + let identity = RuntimeIdentityMaterial::generate("runtime-case-log").unwrap(); + configure_runtime_request_auth(&mut api, &identity, "runtime-case-log"); + seed_worker_source_member(&api, "runtime-case-log", "companion"); + let backend = browser_ticket_backend(&api).unwrap(); + let reference = backend + .create(ticket::NewTicket::new("Case logging")) + .unwrap(); + let before = backend.show(reference.id.clone().into()).unwrap(); + let input = case_test_request( + &before, + "case-log", + ticket::TicketCaseAction::Close { + reason: ticket::TicketClosureReason::Completed, + }, + ); + let mut body = serde_json::to_value(input).unwrap(); + body.as_object_mut().unwrap().remove("author"); + body["explanation"] = json!("body-secret"); + let path = format!( + "/workspaces/{TEST_WORKSPACE_ID}/tickets/{}/case", + reference.id + ); + let app = build_router(api.clone()); + let request = |worker, body: &Value| { + runtime_source_request( + &identity, + worker, + "POST", + &format!("{path}?token=query-secret"), + serde_json::to_vec(body).unwrap(), + ) + }; + // Runtime identity alone still cannot impersonate a Worker or human. + let (status, error, events) = request_with_logs(app.clone(), request(None, &body)).await; + assert_eq!(status, StatusCode::FORBIDDEN, "{error}"); + assert_eq!(events.len(), 1, "{events:?}"); + assert_eq!(events[0]["path"], path); + assert_eq!(events[0]["status"], 403); + assert_eq!(events[0]["message"], error["message"]); + assert!( + error["message"] + .as_str() + .unwrap() + .contains("verified Runtime Worker identity") + ); + assert_eq!(backend.show(reference.id.clone().into()).unwrap(), before); + + // Profile-authorized unassigned Workers reach the same CAS validation. + let mut stale = body.clone(); + stale["expected_content_digest"] = json!("stale"); + let (status, error, events) = + request_with_logs(app.clone(), request(Some("companion"), &stale)).await; + assert_eq!(status, StatusCode::CONFLICT, "{error}"); + assert_eq!(events.len(), 1, "{events:?}"); + assert_eq!(events[0]["path"], path); + assert_eq!(events[0]["status"], 409); + assert_eq!(events[0]["message"], error["message"]); + assert_eq!(backend.show(reference.id.clone().into()).unwrap(), before); + + let (status, result, events) = request_with_logs(app, request(Some("companion"), &body)).await; + assert_eq!(status, StatusCode::OK, "{result}"); + assert!(events.is_empty(), "{events:?}"); +} + +#[tokio::test] +async fn non_api_workspace_paths_are_not_logged_as_ticket_api_errors() { + let app = Router::new() + .route( + "/workspaces/not-a-ticket-page", + get(|| async { ApiError::from(Error::InvalidInput("not an API".into())) }), + ) + .layer(middleware::from_fn(log_failed_api_response)); + let (status, _, events) = request_with_logs( + app, + Request::builder() + .uri("/workspaces/not-a-ticket-page") + .body(Body::empty()) + .unwrap(), + ) + .await; + assert_eq!(status, StatusCode::BAD_REQUEST); + assert!(events.is_empty()); +} diff --git a/crates/workspace-server/src/server_residual_execution_tests.rs b/crates/workspace-server/src/server_residual_execution_tests.rs index 6cf3759fa..275a1deff 100644 --- a/crates/workspace-server/src/server_residual_execution_tests.rs +++ b/crates/workspace-server/src/server_residual_execution_tests.rs @@ -715,7 +715,7 @@ async fn worker_control_source_lookup_waits_off_tokio_and_retains_finite_host_ow } #[tokio::test(flavor = "current_thread")] -async fn ticket_tracking_and_case_authority_runtime_lookup_leave_db_and_tokio_free() { +async fn ticket_tracking_authority_and_case_audit_lookup_leave_db_and_tokio_free() { for operation in ["tracking", "case"] { let fixture = fixture().await; let ticket = browser_ticket_backend(&fixture.api) @@ -841,11 +841,25 @@ async fn ticket_tracking_and_case_authority_runtime_lookup_leave_db_and_tokio_fr observed(fixture.host_executor.drain()).await.unwrap(); observed(fixture.executor.drain()).await.unwrap(); assert_eq!(leases.load(Ordering::SeqCst), 0); - assert_eq!( - backend.show(TicketIdOrSlug::Id(ticket.id)).unwrap(), - before, - "failed live authority must not make a decision" - ); + let after = backend.show(TicketIdOrSlug::Id(ticket.id)).unwrap(); + if operation == "tracking" { + assert_eq!( + after, before, + "tracking still requires live assignment authority" + ); + } else { + // Profile-authorized case changes do not depend on a live + // Orchestrator lookup. Its failure only affects audit role context. + assert_eq!(after.meta.workflow_state, ticket::TicketState::Closed); + assert_eq!( + after.events.last().unwrap().attributes["source_actor_role"], + "worker" + ); + assert_eq!( + after.current_closure.unwrap().operation_key, + "case-operation" + ); + } } } diff --git a/crates/workspace-server/src/server_ticket_case_signed_tests.rs b/crates/workspace-server/src/server_ticket_case_signed_tests.rs index 8f7c38b52..fc824f147 100644 --- a/crates/workspace-server/src/server_ticket_case_signed_tests.rs +++ b/crates/workspace-server/src/server_ticket_case_signed_tests.rs @@ -34,59 +34,62 @@ fn signed_case_request( } #[tokio::test] -async fn signed_unassigned_worker_without_hints_cannot_close_reopen_or_correct_cases() { +async fn signed_unassigned_worker_can_close_correct_and_reopen_without_orchestration() { let dir = tempfile::tempdir().unwrap(); let (mut api, execution) = test_api_with_recording_backend(dir.path()).await; let identity = RuntimeIdentityMaterial::generate("runtime-case").unwrap(); configure_runtime_request_auth(&mut api, &identity, "runtime-case"); - seed_worker_source_member(&api, "runtime-case", "other"); + seed_worker_source_member(&api, "runtime-case", "companion"); let backend = browser_ticket_backend(&api).unwrap(); let reference = backend - .create(ticket::NewTicket::new("Case authority")) + .create(ticket::NewTicket::new( + "Case permission is independent of assignment", + )) .unwrap(); - seed_case_assignment( - &api, - &reference.id, - &RuntimeWorkerRef::new("runtime-case", "owner"), - "unauthorized-owner", - ); + let authority = case_authority_snapshot(&api); let app = build_router(api.clone()); - for action in [ + for (index, action) in [ ticket::TicketCaseAction::Close { reason: ticket::TicketClosureReason::Completed, }, - ticket::TicketCaseAction::Reopen, ticket::TicketCaseAction::CorrectClosure { reason: ticket::TicketClosureReason::NotPlanned, }, - ] { - if !matches!(action, ticket::TicketCaseAction::Close { .. }) { - let before = backend.show(reference.id.clone().into()).unwrap(); - if before.meta.workflow_state == TicketState::Open { - backend - .update_case( - &reference.id, - case_test_request( - &before, - "fixture-close", - ticket::TicketCaseAction::Close { - reason: ticket::TicketClosureReason::Completed, - }, - ), - ) - .unwrap(); - } - } + ticket::TicketCaseAction::Reopen, + ] + .into_iter() + .enumerate() + { let before = backend.show(reference.id.clone().into()).unwrap(); - let input = case_test_request(&before, "unauthorized", action); - let authority = case_authority_snapshot(&api); - case_response( + let input = case_test_request(&before, &format!("case-{index}"), action); + let first = case_response( app.clone(), - signed_case_request(&identity, Some("other"), &reference.id, &input), - StatusCode::FORBIDDEN, + signed_case_request(&identity, Some("companion"), &reference.id, &input), + StatusCode::OK, ) .await; - assert_eq!(backend.show(reference.id.clone().into()).unwrap(), before); + assert_eq!( + first["workflow_state"], + if index == 2 { "open" } else { "closed" } + ); + let after = backend.show(reference.id.clone().into()).unwrap(); + let event = after.events.last().unwrap(); + assert_eq!( + event.author.as_deref(), + Some("worker:runtime-case/companion") + ); + assert_eq!(event.attributes["source_actor_role"], "worker"); + assert!(!event.attributes.contains_key("source_assignment_id")); + assert_eq!( + case_response( + app.clone(), + signed_case_request(&identity, Some("companion"), &reference.id, &input), + StatusCode::OK, + ) + .await, + first + ); + assert_eq!(backend.show(reference.id.clone().into()).unwrap(), after); assert_eq!(case_authority_snapshot(&api), authority); } assert!(execution.protocol_methods().is_empty()); @@ -254,17 +257,7 @@ async fn signed_assigned_worker_case_replay_keeps_original_actor_after_release_w StatusCode::CONFLICT, ) .await; - let reopen = case_test_request( - &closed, - "released-worker-reopen", - ticket::TicketCaseAction::Reopen, - ); - case_response( - app.clone(), - signed_case_request(&identity, Some("owner"), &reference.id, &reopen), - StatusCode::FORBIDDEN, - ) - .await; + // Releasing execution grants does not revoke Profile case-decision access. let correction = case_test_request( &closed, "released-worker-correction", @@ -273,13 +266,38 @@ async fn signed_assigned_worker_case_replay_keeps_original_actor_after_release_w }, ); case_response( - app, + app.clone(), signed_case_request(&identity, Some("owner"), &reference.id, &correction), - StatusCode::FORBIDDEN, + StatusCode::OK, ) .await; - assert_eq!(backend.show(reference.id.into()).unwrap(), closed); + let stale_reopen = case_test_request(&closed, "stale-reopen", ticket::TicketCaseAction::Reopen); + case_response( + app.clone(), + signed_case_request(&identity, Some("owner"), &reference.id, &stale_reopen), + StatusCode::CONFLICT, + ) + .await; + let corrected = backend.show(reference.id.clone().into()).unwrap(); + let reopen = case_test_request( + &corrected, + "released-worker-reopen", + ticket::TicketCaseAction::Reopen, + ); + case_response( + app, + signed_case_request(&identity, Some("owner"), &reference.id, &reopen), + StatusCode::OK, + ) + .await; + let reopened = backend.show(reference.id.into()).unwrap(); + assert_eq!(reopened.meta.workflow_state, TicketState::Open); + assert_eq!( + reopened.events.last().unwrap().attributes["source_actor_role"], + "worker" + ); assert_eq!(case_authority_snapshot(&api), released); + assert!(execution.protocol_methods().is_empty()); } @@ -492,15 +510,13 @@ async fn typed_and_generic_case_dispatch_keep_the_same_verified_worker_and_recei reference.id ); let generic = generic_case_router(api.clone()); - for worker in [Some("other"), None] { - case_response( - generic.clone(), - signed_case_request(&identity, worker, &reference.id, &input), - StatusCode::FORBIDDEN, - ) - .await; - assert_eq!(backend.show(reference.id.clone().into()).unwrap(), before); - } + case_response( + generic.clone(), + signed_case_request(&identity, None, &reference.id, &input), + StatusCode::FORBIDDEN, + ) + .await; + assert_eq!(backend.show(reference.id.clone().into()).unwrap(), before); let closed = case_response( generic.clone(), runtime_source_request(