chore: integrate case permission fixes for notification work
This commit is contained in:
@@ -293,9 +293,9 @@ impl Tool for WorkspaceTicketReadTool {
|
|||||||
))
|
))
|
||||||
.map_err(|error| ToolError::ExecutionFailed(error.to_string()))?;
|
.map_err(|error| ToolError::ExecutionFailed(error.to_string()))?;
|
||||||
if !response.is_success() {
|
if !response.is_success() {
|
||||||
return Err(ToolError::ExecutionFailed(format!(
|
return Err(ToolError::ExecutionFailed(ticket_api_error_message(
|
||||||
"Workspace Ticket API request failed with HTTP status {}",
|
response.status,
|
||||||
response.status
|
&response.body,
|
||||||
)));
|
)));
|
||||||
}
|
}
|
||||||
let response_value: Value = serde_json::from_str(&response.body).map_err(|error| {
|
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 {
|
fn allows_tool(self, name: &str) -> bool {
|
||||||
// Publishing a decision tool is not an authority grant. The injected
|
// Profile tool policy controls case-decision access in both modes.
|
||||||
// Workspace transport lets the Backend validate the current assignment
|
// The Backend verifies actor/Workspace identity and CAS/replay, not
|
||||||
// or author/orchestrator permission on every decision, in both modes.
|
// orchestration assignment as an additional case-decision permission.
|
||||||
READ_ONLY_TOOL_NAMES.contains(&name)
|
READ_ONLY_TOOL_NAMES.contains(&name)
|
||||||
|| (name == "TicketCaseUpdate" && (self.authoring || self.thread || self.workflow))
|
|| (name == "TicketCaseUpdate" && (self.authoring || self.thread || self.workflow))
|
||||||
|| (self.authoring && AUTHORING_TOOL_NAMES.contains(&name))
|
|| (self.authoring && AUTHORING_TOOL_NAMES.contains(&name))
|
||||||
@@ -921,9 +921,9 @@ impl WorkspaceHttpTicketBackend {
|
|||||||
TicketError::Conflict(format!("ticket REST request failed: {error}"))
|
TicketError::Conflict(format!("ticket REST request failed: {error}"))
|
||||||
})?;
|
})?;
|
||||||
if !response.is_success() {
|
if !response.is_success() {
|
||||||
return Err(TicketError::Conflict(format!(
|
return Err(TicketError::Conflict(ticket_api_error_message(
|
||||||
"ticket REST API request failed with HTTP status {}",
|
response.status,
|
||||||
response.status
|
&response.body,
|
||||||
)));
|
)));
|
||||||
}
|
}
|
||||||
serde_json::from_str(&response.body)
|
serde_json::from_str(&response.body)
|
||||||
@@ -997,9 +997,9 @@ impl WorkspaceHttpTicketBackend {
|
|||||||
TicketError::Conflict(format!("ticket REST request failed: {error}"))
|
TicketError::Conflict(format!("ticket REST request failed: {error}"))
|
||||||
})?;
|
})?;
|
||||||
if !response.is_success() {
|
if !response.is_success() {
|
||||||
return Err(TicketError::Conflict(format!(
|
return Err(TicketError::Conflict(ticket_api_error_message(
|
||||||
"ticket REST API request failed with HTTP status {}",
|
response.status,
|
||||||
response.status
|
&response.body,
|
||||||
)));
|
)));
|
||||||
}
|
}
|
||||||
Ok(TicketBackendOperationResult::Unit)
|
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::<server_api::RepositoryApiError>(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 {
|
fn map_ticket_tool_error(error: ToolError) -> WipOperationError {
|
||||||
match error {
|
match error {
|
||||||
ToolError::InvalidArgument(message) => {
|
ToolError::InvalidArgument(message) => {
|
||||||
@@ -2112,17 +2162,19 @@ fn map_ticket_tool_error(error: ToolError) -> WipOperationError {
|
|||||||
format!("{code}: {message}"),
|
format!("{code}: {message}"),
|
||||||
),
|
),
|
||||||
ToolError::ExecutionFailed(message) => {
|
ToolError::ExecutionFailed(message) => {
|
||||||
if message.contains("HTTP status 401") || message.contains("HTTP status 403") {
|
// Parse the transport status before the public detail. A message
|
||||||
protocol_failure(ProtocolErrorCode::PermissionDenied, message)
|
// mentioning a different HTTP status must not change classification.
|
||||||
} else if message.contains("HTTP status 404") {
|
let status = message
|
||||||
protocol_failure(ProtocolErrorCode::NotFound, message)
|
.split_once("HTTP status ")
|
||||||
} else if message.contains("HTTP status 400")
|
.and_then(|(_, tail)| tail.split(|c: char| c == ':' || c.is_whitespace()).next())
|
||||||
|| message.contains("HTTP status 409")
|
.and_then(|status| status.parse::<u16>().ok());
|
||||||
|| message.contains("HTTP status 422")
|
match status {
|
||||||
{
|
Some(401 | 403) => protocol_failure(ProtocolErrorCode::PermissionDenied, message),
|
||||||
protocol_failure(ProtocolErrorCode::InvalidArguments, message)
|
Some(404) => protocol_failure(ProtocolErrorCode::NotFound, message),
|
||||||
} else {
|
Some(400 | 409 | 422) => {
|
||||||
WipOperationError::OutcomeUnknown(message)
|
protocol_failure(ProtocolErrorCode::InvalidArguments, message)
|
||||||
|
}
|
||||||
|
_ => WipOperationError::OutcomeUnknown(message),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
ToolError::Internal(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 [
|
||||||
|
"<html>proxy-private-detail</html>".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]
|
#[test]
|
||||||
fn case_backend_sends_explicit_judgment_without_domain_author_claim() {
|
fn case_backend_sends_explicit_judgment_without_domain_author_claim() {
|
||||||
let client = decision_client(200, TicketState::Closed);
|
let client = decision_client(200, TicketState::Closed);
|
||||||
@@ -3282,6 +3463,15 @@ mod tests {
|
|||||||
WipOperationError::Protocol(ProtocolError { code: actual, .. }) if actual == code
|
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!(
|
assert!(matches!(
|
||||||
map_ticket_tool_error(ToolError::ExecutionFailed("transport disconnected".into())),
|
map_ticket_tool_error(ToolError::ExecutionFailed("transport disconnected".into())),
|
||||||
WipOperationError::OutcomeUnknown(_)
|
WipOperationError::OutcomeUnknown(_)
|
||||||
|
|||||||
@@ -12496,7 +12496,9 @@ async fn log_failed_api_response(request: Request, next: Next) -> Response {
|
|||||||
let response = next.run(request).await;
|
let response = next.run(request).await;
|
||||||
let status = response.status();
|
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.
|
// Generated routes bypass ApiError::into_response; inspect their typed error, not the body.
|
||||||
let generated_error = response
|
let generated_error = response
|
||||||
.extensions()
|
.extensions()
|
||||||
@@ -16051,23 +16053,6 @@ fn execute_ticket_rest_operation_on_host(
|
|||||||
}
|
}
|
||||||
validate_ticket_repository_operation(api, &operation)?;
|
validate_ticket_repository_operation(api, &operation)?;
|
||||||
let before = target.as_ref().and_then(|id| backend.show(id.clone()).ok());
|
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
|
let mut ticket_item_check = source
|
||||||
.as_ref()
|
.as_ref()
|
||||||
.cloned()
|
.cloned()
|
||||||
@@ -18305,68 +18290,67 @@ async fn apply_ticket_case_update(
|
|||||||
Json(mut request): Json<ticket::TicketCaseUpdate>,
|
Json(mut request): Json<ticket::TicketCaseUpdate>,
|
||||||
) -> ApiResult<Json<ticket::Ticket>> {
|
) -> ApiResult<Json<ticket::Ticket>> {
|
||||||
let actor = actor.cloned();
|
let actor = actor.cloned();
|
||||||
api.host_executors.host()?.run(move || {
|
api.host_executors
|
||||||
let actor = actor.as_ref();
|
.host()?
|
||||||
|
.run(move || {
|
||||||
|
let actor = actor.as_ref();
|
||||||
|
|
||||||
validate_workspace_scope(&api, &workspace_id)?;
|
validate_workspace_scope(&api, &workspace_id)?;
|
||||||
let ticket_id = resolve_workspace_ticket_reference(&api, &workspace_id, &ticket_id)?;
|
let ticket_id = resolve_workspace_ticket_reference(&api, &workspace_id, &ticket_id)?;
|
||||||
let mut backend = browser_ticket_backend(&api)?;
|
let mut backend = browser_ticket_backend(&api)?;
|
||||||
let before = backend
|
let before = backend
|
||||||
.show(TicketIdOrSlug::Id(ticket_id.clone()))
|
.show(TicketIdOrSlug::Id(ticket_id.clone()))
|
||||||
.map_err(Error::from)?;
|
.map_err(Error::from)?;
|
||||||
if let Some(source) = source {
|
if let Some(source) = source {
|
||||||
let author = format!("worker:{}/{}", source.runtime_id, source.worker_id);
|
let author = format!("worker:{}/{}", source.runtime_id, source.worker_id);
|
||||||
let replay_event = before.events.iter().find(|event| {
|
let replay_event = before.events.iter().find(|event| {
|
||||||
event.author.as_deref() == Some(author.as_str())
|
event.author.as_deref() == Some(author.as_str())
|
||||||
&& event.attributes.get("operation_key") == Some(&request.operation_key)
|
&& event.attributes.get("operation_key") == Some(&request.operation_key)
|
||||||
&& event.attributes.get("source_runtime_id") == Some(&source.runtime_id)
|
&& event.attributes.get("source_runtime_id") == Some(&source.runtime_id)
|
||||||
&& event.attributes.get("source_worker_id") == Some(&source.worker_id)
|
&& event.attributes.get("source_worker_id") == Some(&source.worker_id)
|
||||||
});
|
});
|
||||||
let replay = replay_event.is_some();
|
// Profile tool policy authorizes this operation at the Runtime boundary.
|
||||||
let context = worker_ticket_source_context(&api, &workspace_id, &source, Some(&before));
|
// Assignment is audit context, not an additional case-decision permission.
|
||||||
let is_orchestrator =
|
let context =
|
||||||
find_workspace_orchestrator(&api).is_some_and(|worker| worker.worker == source);
|
worker_ticket_source_context(&api, &workspace_id, &source, Some(&before));
|
||||||
if !replay && context.actor_role != "ticket_worker" && !is_orchestrator {
|
request.author = Some(author);
|
||||||
return Err(Error::WorkspacePermissionDenied(
|
let attributes = if let Some(event) = replay_event {
|
||||||
"Ticket state decisions require the assigned Worker or registered Workspace Orchestrator".into(),
|
// Replay authenticates the same actor, but retains the receipt's
|
||||||
).into());
|
// original work identity. Completion/reopen never revives that grant.
|
||||||
}
|
event
|
||||||
request.author = Some(author);
|
.attributes
|
||||||
let attributes = if let Some(event) = replay_event {
|
.iter()
|
||||||
// Replay authenticates the same actor, but retains the receipt's
|
.filter(|(key, _)| key.starts_with("source_"))
|
||||||
// original work identity. Completion/reopen never revives that grant.
|
.map(|(key, value)| (key.clone(), value.clone()))
|
||||||
event
|
.collect()
|
||||||
.attributes
|
} else {
|
||||||
.iter()
|
context.attributes("case_update")
|
||||||
.filter(|(key, _)| key.starts_with("source_"))
|
};
|
||||||
.map(|(key, value)| (key.clone(), value.clone()))
|
backend = backend.with_event_attributes(attributes);
|
||||||
.collect()
|
} else if let Some(actor) = actor {
|
||||||
} else {
|
// Include both verified IDs in the author (and thus the receipt fingerprint).
|
||||||
context.attributes("case_update")
|
// JSON tuple encoding avoids ambiguous identity concatenation; proof tokens,
|
||||||
};
|
// auth method and signature timestamps are not replay identity.
|
||||||
backend = backend.with_event_attributes(attributes);
|
let identity = serde_json::to_string(&(&actor.account_id, &actor.user_id))
|
||||||
} else if let Some(actor) = actor {
|
.map_err(|error| {
|
||||||
// Include both verified IDs in the author (and thus the receipt fingerprint).
|
Error::Config(format!("failed to encode case actor: {error}"))
|
||||||
// JSON tuple encoding avoids ambiguous identity concatenation; proof tokens,
|
})?;
|
||||||
// auth method and signature timestamps are not replay identity.
|
request.author = Some(format!("workspace-user:{identity}"));
|
||||||
let identity = serde_json::to_string(&(&actor.account_id, &actor.user_id))
|
backend = backend.with_event_attributes(BTreeMap::from([
|
||||||
.map_err(|error| Error::Config(format!("failed to encode case actor: {error}")))?;
|
("source_account_id".into(), actor.account_id.clone()),
|
||||||
request.author = Some(format!("workspace-user:{identity}"));
|
("source_user_id".into(), actor.user_id.clone()),
|
||||||
backend = backend.with_event_attributes(BTreeMap::from([
|
]));
|
||||||
("source_account_id".into(), actor.account_id.clone()),
|
} else {
|
||||||
("source_user_id".into(), actor.user_id.clone()),
|
// Only the test-only header adapter can reach this legacy attribution.
|
||||||
]));
|
request.author = Some("workspace-user".into());
|
||||||
} else {
|
}
|
||||||
// Only the test-only header adapter can reach this legacy attribution.
|
let ticket = backend
|
||||||
request.author = Some("workspace-user".into());
|
.update_case(&ticket_id, request)
|
||||||
}
|
.map_err(Error::from)?;
|
||||||
let ticket = backend
|
recover_ticket_notifications(&api);
|
||||||
.update_case(&ticket_id, request)
|
Ok(Json(ticket))
|
||||||
.map_err(Error::from)?;
|
})
|
||||||
recover_ticket_notifications(&api);
|
.await
|
||||||
Ok(Json(ticket))
|
|
||||||
|
|
||||||
}).await
|
|
||||||
}
|
}
|
||||||
|
|
||||||
fn reject_non_browser_reopen_auth(headers: &HeaderMap) -> Result<()> {
|
fn reject_non_browser_reopen_auth(headers: &HeaderMap) -> Result<()> {
|
||||||
@@ -51608,7 +51592,7 @@ mod tests {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[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 dir = tempfile::tempdir().unwrap();
|
||||||
let api = test_api(dir.path()).await;
|
let api = test_api(dir.path()).await;
|
||||||
let backend = browser_ticket_backend(&api).unwrap();
|
let backend = browser_ticket_backend(&api).unwrap();
|
||||||
@@ -51625,23 +51609,26 @@ mod tests {
|
|||||||
},
|
},
|
||||||
expected_closure_operation_key: None,
|
expected_closure_operation_key: None,
|
||||||
author: None,
|
author: None,
|
||||||
operation_key: "forged".into(),
|
operation_key: "authorized-case-decision".into(),
|
||||||
expected_content_digest: ticket::ticket_content_digest(&before),
|
expected_content_digest: ticket::ticket_content_digest(&before),
|
||||||
expected_state: TicketState::Open,
|
expected_state: TicketState::Open,
|
||||||
explanation: "Not my task".into(),
|
explanation: "Authorized case decision independent of assignment".into(),
|
||||||
references: Vec::new(),
|
references: Vec::new(),
|
||||||
};
|
};
|
||||||
let response = scoped_ticket_case_update(
|
let Json(closed) = scoped_ticket_case_update(
|
||||||
State(api.clone()),
|
State(api.clone()),
|
||||||
ticket_check_headers(&other),
|
ticket_check_headers(&other),
|
||||||
AxumPath((TEST_WORKSPACE_ID.into(), ticket.id.clone())),
|
AxumPath((TEST_WORKSPACE_ID.into(), ticket.id.clone())),
|
||||||
Json(request),
|
Json(request),
|
||||||
)
|
)
|
||||||
.await
|
.await
|
||||||
.unwrap_err()
|
.unwrap();
|
||||||
.into_response();
|
assert_eq!(closed.meta.workflow_state, TicketState::Closed);
|
||||||
assert_eq!(response.status(), StatusCode::FORBIDDEN);
|
assert_eq!(
|
||||||
assert_eq!(backend.show(ticket.id.into()).unwrap(), before);
|
closed.events.last().unwrap().attributes["source_actor_role"],
|
||||||
|
"worker"
|
||||||
|
);
|
||||||
|
assert_eq!(backend.show(ticket.id.into()).unwrap(), closed);
|
||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
|
|||||||
@@ -919,3 +919,91 @@ async fn saturated_host_before_command_start_returns_availability_without_a_prov
|
|||||||
host.drain().await.unwrap();
|
host.drain().await.unwrap();
|
||||||
fixture.api.store.db_executor().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());
|
||||||
|
}
|
||||||
|
|||||||
@@ -715,7 +715,7 @@ async fn worker_control_source_lookup_waits_off_tokio_and_retains_finite_host_ow
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test(flavor = "current_thread")]
|
#[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"] {
|
for operation in ["tracking", "case"] {
|
||||||
let fixture = fixture().await;
|
let fixture = fixture().await;
|
||||||
let ticket = browser_ticket_backend(&fixture.api)
|
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.host_executor.drain()).await.unwrap();
|
||||||
observed(fixture.executor.drain()).await.unwrap();
|
observed(fixture.executor.drain()).await.unwrap();
|
||||||
assert_eq!(leases.load(Ordering::SeqCst), 0);
|
assert_eq!(leases.load(Ordering::SeqCst), 0);
|
||||||
assert_eq!(
|
let after = backend.show(TicketIdOrSlug::Id(ticket.id)).unwrap();
|
||||||
backend.show(TicketIdOrSlug::Id(ticket.id)).unwrap(),
|
if operation == "tracking" {
|
||||||
before,
|
assert_eq!(
|
||||||
"failed live authority must not make a decision"
|
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"
|
||||||
|
);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -34,59 +34,62 @@ fn signed_case_request(
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[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 dir = tempfile::tempdir().unwrap();
|
||||||
let (mut api, execution) = test_api_with_recording_backend(dir.path()).await;
|
let (mut api, execution) = test_api_with_recording_backend(dir.path()).await;
|
||||||
let identity = RuntimeIdentityMaterial::generate("runtime-case").unwrap();
|
let identity = RuntimeIdentityMaterial::generate("runtime-case").unwrap();
|
||||||
configure_runtime_request_auth(&mut api, &identity, "runtime-case");
|
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 backend = browser_ticket_backend(&api).unwrap();
|
||||||
let reference = backend
|
let reference = backend
|
||||||
.create(ticket::NewTicket::new("Case authority"))
|
.create(ticket::NewTicket::new(
|
||||||
|
"Case permission is independent of assignment",
|
||||||
|
))
|
||||||
.unwrap();
|
.unwrap();
|
||||||
seed_case_assignment(
|
let authority = case_authority_snapshot(&api);
|
||||||
&api,
|
|
||||||
&reference.id,
|
|
||||||
&RuntimeWorkerRef::new("runtime-case", "owner"),
|
|
||||||
"unauthorized-owner",
|
|
||||||
);
|
|
||||||
let app = build_router(api.clone());
|
let app = build_router(api.clone());
|
||||||
for action in [
|
for (index, action) in [
|
||||||
ticket::TicketCaseAction::Close {
|
ticket::TicketCaseAction::Close {
|
||||||
reason: ticket::TicketClosureReason::Completed,
|
reason: ticket::TicketClosureReason::Completed,
|
||||||
},
|
},
|
||||||
ticket::TicketCaseAction::Reopen,
|
|
||||||
ticket::TicketCaseAction::CorrectClosure {
|
ticket::TicketCaseAction::CorrectClosure {
|
||||||
reason: ticket::TicketClosureReason::NotPlanned,
|
reason: ticket::TicketClosureReason::NotPlanned,
|
||||||
},
|
},
|
||||||
] {
|
ticket::TicketCaseAction::Reopen,
|
||||||
if !matches!(action, ticket::TicketCaseAction::Close { .. }) {
|
]
|
||||||
let before = backend.show(reference.id.clone().into()).unwrap();
|
.into_iter()
|
||||||
if before.meta.workflow_state == TicketState::Open {
|
.enumerate()
|
||||||
backend
|
{
|
||||||
.update_case(
|
|
||||||
&reference.id,
|
|
||||||
case_test_request(
|
|
||||||
&before,
|
|
||||||
"fixture-close",
|
|
||||||
ticket::TicketCaseAction::Close {
|
|
||||||
reason: ticket::TicketClosureReason::Completed,
|
|
||||||
},
|
|
||||||
),
|
|
||||||
)
|
|
||||||
.unwrap();
|
|
||||||
}
|
|
||||||
}
|
|
||||||
let before = backend.show(reference.id.clone().into()).unwrap();
|
let before = backend.show(reference.id.clone().into()).unwrap();
|
||||||
let input = case_test_request(&before, "unauthorized", action);
|
let input = case_test_request(&before, &format!("case-{index}"), action);
|
||||||
let authority = case_authority_snapshot(&api);
|
let first = case_response(
|
||||||
case_response(
|
|
||||||
app.clone(),
|
app.clone(),
|
||||||
signed_case_request(&identity, Some("other"), &reference.id, &input),
|
signed_case_request(&identity, Some("companion"), &reference.id, &input),
|
||||||
StatusCode::FORBIDDEN,
|
StatusCode::OK,
|
||||||
)
|
)
|
||||||
.await;
|
.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_eq!(case_authority_snapshot(&api), authority);
|
||||||
}
|
}
|
||||||
assert!(execution.protocol_methods().is_empty());
|
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,
|
StatusCode::CONFLICT,
|
||||||
)
|
)
|
||||||
.await;
|
.await;
|
||||||
let reopen = case_test_request(
|
// Releasing execution grants does not revoke Profile case-decision access.
|
||||||
&closed,
|
|
||||||
"released-worker-reopen",
|
|
||||||
ticket::TicketCaseAction::Reopen,
|
|
||||||
);
|
|
||||||
case_response(
|
|
||||||
app.clone(),
|
|
||||||
signed_case_request(&identity, Some("owner"), &reference.id, &reopen),
|
|
||||||
StatusCode::FORBIDDEN,
|
|
||||||
)
|
|
||||||
.await;
|
|
||||||
let correction = case_test_request(
|
let correction = case_test_request(
|
||||||
&closed,
|
&closed,
|
||||||
"released-worker-correction",
|
"released-worker-correction",
|
||||||
@@ -273,13 +266,38 @@ async fn signed_assigned_worker_case_replay_keeps_original_actor_after_release_w
|
|||||||
},
|
},
|
||||||
);
|
);
|
||||||
case_response(
|
case_response(
|
||||||
app,
|
app.clone(),
|
||||||
signed_case_request(&identity, Some("owner"), &reference.id, &correction),
|
signed_case_request(&identity, Some("owner"), &reference.id, &correction),
|
||||||
StatusCode::FORBIDDEN,
|
StatusCode::OK,
|
||||||
)
|
)
|
||||||
.await;
|
.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_eq!(case_authority_snapshot(&api), released);
|
||||||
|
|
||||||
assert!(execution.protocol_methods().is_empty());
|
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
|
reference.id
|
||||||
);
|
);
|
||||||
let generic = generic_case_router(api.clone());
|
let generic = generic_case_router(api.clone());
|
||||||
for worker in [Some("other"), None] {
|
case_response(
|
||||||
case_response(
|
generic.clone(),
|
||||||
generic.clone(),
|
signed_case_request(&identity, None, &reference.id, &input),
|
||||||
signed_case_request(&identity, worker, &reference.id, &input),
|
StatusCode::FORBIDDEN,
|
||||||
StatusCode::FORBIDDEN,
|
)
|
||||||
)
|
.await;
|
||||||
.await;
|
assert_eq!(backend.show(reference.id.clone().into()).unwrap(), before);
|
||||||
assert_eq!(backend.show(reference.id.clone().into()).unwrap(), before);
|
|
||||||
}
|
|
||||||
let closed = case_response(
|
let closed = case_response(
|
||||||
generic.clone(),
|
generic.clone(),
|
||||||
runtime_source_request(
|
runtime_source_request(
|
||||||
|
|||||||
Reference in New Issue
Block a user