From 6f54d2939d00553e18f28d4e5ad30b16f317b275 Mon Sep 17 00:00:00 2001 From: XuPeng-SH Date: Thu, 22 Oct 2026 18:31:19 +0800 Subject: [PATCH 1/2] fix: unify workspace path authorization and local refusal reporting --- .../chat_stream/sse_loop/agentic_loop_turn.rs | 29 ++- .../cli/chat_stream/sse_loop/remote_turn.rs | 2 +- .../sse_loop/server_admission_host.rs | 14 +- crates/astra-cli/src/cli/command_router.rs | 69 +++++- crates/astra-cli/src/cli/exit_code.rs | 2 +- .../src/cli/turn/turn_failure_reporting.rs | 5 +- .../astra-cli/src/cli/turn/turn_settlement.rs | 67 ++++++ crates/astra-cli/src/edge_tools/fs.rs | 106 +++++++-- .../astra-cli/src/tests/chat_stream_tests.rs | 17 ++ crates/astra-sandbox/src/path.rs | 166 +++++++++---- crates/astra-sandbox/src/policy.rs | 40 ++-- crates/astra-test-harness/src/classify.rs | 5 + crates/astra-test-harness/src/criteria.rs | 2 + crates/astra-test-harness/src/exec.rs | 220 ++++++++++++++++-- crates/astra-test-harness/src/judger.rs | 4 + crates/astra-test-harness/src/report.rs | 2 + crates/astra-test-harness/src/runner.rs | 89 ++++++- crates/astra-test-harness/src/suite.rs | 174 ++++++++++++-- crates/astra-tools/src/executor.rs | 2 +- crates/astra-tools/src/fs_ops.rs | 121 +++------- docs/design/edge-runtime-tool-boundary.md | 13 ++ docs/reference/cli-commands.md | 3 +- docs/testing/capability-harness.md | 12 + 23 files changed, 923 insertions(+), 241 deletions(-) diff --git a/crates/astra-cli/src/cli/chat_stream/sse_loop/agentic_loop_turn.rs b/crates/astra-cli/src/cli/chat_stream/sse_loop/agentic_loop_turn.rs index 057703bdc5..33fb91438a 100644 --- a/crates/astra-cli/src/cli/chat_stream/sse_loop/agentic_loop_turn.rs +++ b/crates/astra-cli/src/cli/chat_stream/sse_loop/agentic_loop_turn.rs @@ -1439,12 +1439,30 @@ fn validate_workspace_coordination_host() // text-only turn or a surface with only read-only tools needs no writer host. async fn validate_local_workspace_host_for_tool_surface( executor: &ToolExecutor, -) -> Result<(), String> { +) -> Result<(), astra_core::ClassifiedError> { if executor .current_surface_requires_workspace_coordination() .await { - validate_workspace_coordination_host().map_err(|error| error.to_string())?; + validate_workspace_coordination_host().map_err(|error| { + use astra_tools::workspace_observation::WorkspaceLeaseFailure; + match error { + WorkspaceLeaseFailure::CoordinationHostUnsupported => { + astra_core::ClassifiedError::new( + astra_core::ErrorKind::ToolUnavailable, + error.to_string(), + ) + .with_details_json( + serde_json::json!({ + "code": astra_tools::TOOL_ERROR_KIND_WORKSPACE_HOST_UNSUPPORTED, + "retryable": false, + }) + .to_string(), + ) + } + other => astra_core::ClassifiedError::from(other.to_string()), + } + })?; } Ok(()) } @@ -1457,7 +1475,7 @@ async fn chat_turn_post_payload_after_prepare( ChatTurnPrepLineGuard, Option, ), - String, + astra_core::ClassifiedError, > { let ChatTurnPostPayloadRequest { server_admission_dispatched, @@ -1578,7 +1596,7 @@ async fn chat_turn_post_payload_after_prepare( pub(crate) async fn fetch_chat_turn_sse( ctx: ChatTurnSseFetchRequest<'_>, -) -> Result { +) -> Result { let ChatTurnSseFetchRequest { server_admission_dispatched, completion_checks, @@ -1731,7 +1749,8 @@ pub(crate) async fn fetch_chat_turn_sse( status.as_u16(), body.as_str(), compact_or_raw, - )); + ) + .into()); } // Paired with `Acquire` load in `PlanAssembleLineSpinner`: publish "HTTP response ready" diff --git a/crates/astra-cli/src/cli/chat_stream/sse_loop/remote_turn.rs b/crates/astra-cli/src/cli/chat_stream/sse_loop/remote_turn.rs index 5b9d0a5646..9bb6435126 100644 --- a/crates/astra-cli/src/cli/chat_stream/sse_loop/remote_turn.rs +++ b/crates/astra-cli/src/cli/chat_stream/sse_loop/remote_turn.rs @@ -383,7 +383,7 @@ pub(super) async fn consume_remote_turn( host.last_error_metadata.as_ref(), host.last_physical_run_id.as_deref(), ); - if rejected { + if rejected || (result.is_err() && !*host.server_admission_dispatched) { state.turn_event_buffer = None; state.step_recorder.discard_uncommitted(); state.interruption = None; diff --git a/crates/astra-cli/src/cli/chat_stream/sse_loop/server_admission_host.rs b/crates/astra-cli/src/cli/chat_stream/sse_loop/server_admission_host.rs index 5ac4389b6d..bf689a9110 100644 --- a/crates/astra-cli/src/cli/chat_stream/sse_loop/server_admission_host.rs +++ b/crates/astra-cli/src/cli/chat_stream/sse_loop/server_admission_host.rs @@ -1097,7 +1097,19 @@ impl CliServerAdmissionHost<'_> { refresh_root_permission_context(&mut state.permission_context, self.perm_manager).await; state.approval_overrides = self.perm_manager.export_session_overrides(); - let mut turn_result = turn_result?; + let mut turn_result = turn_result.inspect_err(|error| { + if let Some(details) = error + .details_json + .as_deref() + .and_then(|details| serde_json::from_str::(details).ok()) + { + self.last_error_code = details + .get("code") + .and_then(Value::as_str) + .map(str::to_owned); + self.last_error_metadata = Some(details); + } + })?; // Capture observed coordination before ingest can reject an incomplete // remote terminal. The existing success/failure journal commit owns diff --git a/crates/astra-cli/src/cli/command_router.rs b/crates/astra-cli/src/cli/command_router.rs index dc7c1d6536..7db8e72dd0 100644 --- a/crates/astra-cli/src/cli/command_router.rs +++ b/crates/astra-cli/src/cli/command_router.rs @@ -2841,15 +2841,16 @@ async fn execute_cli_command_impl( print_one_shot_completion_warning(&sr, exit_code, args.json); return Ok(exit_code); } + let failure_exit = hard_failure_exit_code(&e); if args.json { - let error = e.error.clone(); write_headless_stdout_line( &serde_json::to_string_pretty(&hard_failure_json_output(e)) .map_err(|error| error.to_string())?, )?; - return Err(error); + return Ok(failure_exit); } - return Err(e.error); + eprintln!("{}", hard_failure_diagnostic(&e)); + return Ok(failure_exit); } }; if wall_deadline_reached || user_interrupted { @@ -3793,7 +3794,27 @@ fn final_json_output(sr: &StreamResult, exit_code: ExitCode) -> serde_json::Valu /// Project observed failure facts without committing a successful turn or /// inferring a durable terminal from a transport error. Run identity comes /// only from the physical stream's validated root binding. +fn hard_failure_diagnostic(failure: &crate::TurnFailure) -> String { + match failure.partial.error_code.as_deref() { + Some(code) if failure.partial.server_admission_not_dispatched => { + format!("[{code}] {}", failure.error) + } + _ => failure.error.clone(), + } +} + +fn hard_failure_exit_code(failure: &crate::TurnFailure) -> ExitCode { + if failure.partial.server_admission_not_dispatched + && failure.partial.failure_kind == Some(astra_core::ErrorKind::ToolUnavailable) + { + ExitCode::ToolFailure + } else { + ExitCode::ApiError + } +} + fn hard_failure_json_output(failure: crate::TurnFailure) -> serde_json::Value { + let exit_code = hard_failure_exit_code(&failure); let partial = failure.partial; let classes = partial .tool_outcomes @@ -3826,7 +3847,13 @@ fn hard_failure_json_output(failure: crate::TurnFailure) -> serde_json::Value { tool_record_coverage_partial: true, ..Default::default() }; - let mut output = final_json_output(&result, ExitCode::ApiError); + let mut output = final_json_output(&result, exit_code); + if partial.server_admission_not_dispatched { + output["server_admission_not_dispatched"] = serde_json::json!(true); + if let Some(kind) = partial.failure_kind { + output["error_kind"] = serde_json::json!(kind.as_str()); + } + } output["tool_result_class_counts"] = classes; output["error"] = serde_json::json!(failure.error); output["error_code"] = serde_json::json!(partial.error_code); @@ -4083,8 +4110,8 @@ pub(crate) async fn run_print_mode( return Ok(ExitCode::Success); } Err(e) => { + let failure_exit = hard_failure_exit_code(&e); if output_format == "json" || stream_json_emitter.is_some() { - let error = e.error.clone(); let mut output = hard_failure_json_output(e); if let Some(emitter) = stream_json_emitter.as_ref() { let result_session_id = output["session_id"].as_str().map(str::to_string); @@ -4099,9 +4126,10 @@ pub(crate) async fn run_print_mode( .map_err(|error| error.to_string())?, )?; } - return Err(error); + return Ok(failure_exit); } - return Err(e.error); + eprintln!("{}", hard_failure_diagnostic(&e)); + return Ok(failure_exit); } }; @@ -4354,6 +4382,33 @@ mod exit_code_tests { }; use crate::cli::stream::streaming_types::VerdictEvent; + #[test] + fn local_host_failure_json_and_process_exit_share_classification() { + let failure = crate::TurnFailure { + error: "Local workspace execution requires an administrator to repair /tmp".into(), + partial: crate::PartialTurnData { + failure_kind: Some(astra_core::ErrorKind::ToolUnavailable), + error_code: Some(astra_tools::TOOL_ERROR_KIND_WORKSPACE_HOST_UNSUPPORTED.into()), + error_metadata: Some(serde_json::json!({"retryable":false})), + server_admission_not_dispatched: true, + ..Default::default() + }, + }; + assert_eq!( + super::hard_failure_exit_code(&failure), + ExitCode::ToolFailure + ); + let output = super::hard_failure_json_output(failure); + assert_eq!(output["exit_code"], 1); + assert_eq!(output["error_kind"], "tool_unavailable"); + assert_eq!(output["error_code"], "workspace_host_unsupported"); + assert_eq!(output["server_admission_not_dispatched"], true); + assert_eq!(output["success"], false); + assert!(output["run_id"].is_null()); + assert_eq!(output["prompt_tokens"], 0); + assert_eq!(output["tool_calls_count"], 0); + } + fn empty_stream_result() -> StreamResult { StreamResult::default() } diff --git a/crates/astra-cli/src/cli/exit_code.rs b/crates/astra-cli/src/cli/exit_code.rs index b9b9be791a..9fee2e2525 100644 --- a/crates/astra-cli/src/cli/exit_code.rs +++ b/crates/astra-cli/src/cli/exit_code.rs @@ -3,7 +3,7 @@ pub(crate) enum ExitCode { /// Success (0) Success = 0, - /// Tool execution failure (1) - at least one tool call failed + /// Tool failure (1) - a tool call failed or a required local capability is unavailable ToolFailure = 1, /// Cancellation (2) - the task was explicitly cancelled Cancelled = 2, diff --git a/crates/astra-cli/src/cli/turn/turn_failure_reporting.rs b/crates/astra-cli/src/cli/turn/turn_failure_reporting.rs index aae4e345b7..fe5c4bf2d6 100644 --- a/crates/astra-cli/src/cli/turn/turn_failure_reporting.rs +++ b/crates/astra-cli/src/cli/turn/turn_failure_reporting.rs @@ -290,7 +290,10 @@ pub(crate) fn report_turn_failure( &failure.partial, ); { - let error_kind = astra_core::ClassifiedError::from(failure.error.clone()).kind; + let error_kind = failure + .partial + .failure_kind + .unwrap_or_else(|| astra_core::ClassifiedError::from(failure.error.clone()).kind); err_event.metadata = Some(serde_json::json!({ "error_kind": error_kind.as_str(), "retryable": error_kind.is_retryable(), diff --git a/crates/astra-cli/src/cli/turn/turn_settlement.rs b/crates/astra-cli/src/cli/turn/turn_settlement.rs index be8cc383e4..b0c77bc09b 100644 --- a/crates/astra-cli/src/cli/turn/turn_settlement.rs +++ b/crates/astra-cli/src/cli/turn/turn_settlement.rs @@ -82,6 +82,24 @@ pub(crate) async fn settle_failed_turn( dispatch: &mut TurnDispatch<'_, '_>, failure: &mut crate::TurnFailure, ) { + if failure.partial.server_admission_not_dispatched { + if !dispatch.line.trim().is_empty() { + dispatch + .ui + .restore_input(dispatch.line, state.session_id.as_deref()); + } + dispatch.ui.show_error(&format!( + "[{}] {}", + failure + .partial + .error_code + .as_deref() + .unwrap_or("local_error"), + failure.error + )); + clear_recovery_scoped_turn_restrictions(state); + return; + } if failure.partial.admission_rejected { report_admission_rejection(state, dispatch.line, failure, dispatch.ui); clear_recovery_scoped_turn_restrictions(state); @@ -112,6 +130,55 @@ mod tests { use crate::cli::session::session_state::SessionState; use std::time::Instant; + #[tokio::test] + async fn local_rejection_restores_draft_without_journal_or_usage() { + let api = astra_thin_client::ThinClient::new("http://127.0.0.1:9", None).unwrap(); + let ctx = TurnContext { + api: &api, + profile: None, + post_commit_tx: None, + admission: None, + explain_analyze_terminal_degraded: None, + }; + let mut ui = crate::tests::TestUi::default(); + let mut state = SessionState::default(); + let usage = std::sync::Arc::new(std::sync::Mutex::new(None)); + let mut dispatch = TurnDispatch { + ctx: &ctx, + line: "write a file", + effective_line: "write a file", + user_intent: "write a file", + input_runtime_required_texts: &[], + input_runtime_volatile_texts: &[], + token: "synthetic-token", + session_id: "", + admitted_server_turn: None, + semantic_query_override: None, + turn_start: std::time::Instant::now(), + ui: &mut ui, + turn_usage_sink: Some(&usage), + }; + let mut failure = crate::TurnFailure { + error: "administrator must repair the root-owned temporary directory".into(), + partial: crate::PartialTurnData { + server_admission_not_dispatched: true, + failure_kind: Some(astra_core::ErrorKind::ToolUnavailable), + error_code: Some("workspace_host_unsupported".into()), + ..Default::default() + }, + }; + settle_failed_turn(&mut state, &mut dispatch, &mut failure).await; + assert_eq!(ui.restored_inputs, vec!["write a file"]); + assert!(state.journal.is_none()); + assert!(usage.lock().unwrap().is_none()); + assert!( + ui.errors + .iter() + .any(|message| message.contains("workspace_host_unsupported") + && message.contains("administrator")) + ); + } + #[tokio::test] async fn settle_successful_turn_clears_last_turn_interrupted() { let api = astra_thin_client::ThinClient::new("http://127.0.0.1:9", None).unwrap(); diff --git a/crates/astra-cli/src/edge_tools/fs.rs b/crates/astra-cli/src/edge_tools/fs.rs index b26463fdc4..130e3a3666 100644 --- a/crates/astra-cli/src/edge_tools/fs.rs +++ b/crates/astra-cli/src/edge_tools/fs.rs @@ -189,27 +189,6 @@ impl ToolExecutor { let p = expanded_home_path .as_deref() .unwrap_or_else(|| Path::new(path)); - let resolved = if p.is_absolute() { - p.to_path_buf() - } else { - self.project_root.join(p) - }; - - // Symlink loop / depth guard: canonicalize to detect circular symlinks. - // Skip for non-existent paths (let the caller produce a clear NotFound). - if resolved.exists() { - match resolved.canonicalize() { - Ok(_) => {} // reachable — no loop - Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} // race — let caller handle - Err(e) => { - return Err(format!( - "Error: cannot resolve '{}' (possible symlink loop or broken link): {e}", - path - ) - .into()); - } - } - } { let sp_guard = self @@ -231,7 +210,11 @@ impl ToolExecutor { }); } } - Ok(resolved) + Ok(if p.is_absolute() { + p.to_path_buf() + } else { + self.project_root.join(p) + }) } /// String API adapter for current filesystem callers that still consume @@ -2462,6 +2445,85 @@ mod tests { } } + #[tokio::test] + async fn public_file_tools_reject_relative_escape_without_mutation() { + let temp = tempfile::tempdir().unwrap(); + let root = temp.path().join("project"); + let outside = temp.path().join("outside"); + std::fs::create_dir_all(&root).unwrap(); + std::fs::create_dir_all(&outside).unwrap(); + let marker = outside.join("marker.txt"); + std::fs::write(&marker, "SYNTHETIC_BOUNDARY_MARKER").unwrap(); + let executor = test_executor_in(&root); + for (name, args) in [ + ("read_file", json!({"path":"../outside/marker.txt"})), + ( + "write_file", + json!({"path":"../outside/marker.txt", "delete":true}), + ), + ( + "write_file", + json!({"path":"../outside/new/deep.txt", "content":"changed"}), + ), + ( + "str_replace", + json!({"path":"../outside/marker.txt", "old_str":"SYNTHETIC_BOUNDARY_MARKER", "new_str":"changed"}), + ), + ] { + let result = executor.execute_with_metadata(name, &args).await; + assert!(result.is_error, "{name}: {result:?}"); + let output = result.output; + assert!( + output.contains("outside the project directory"), + "{name}: {output}" + ); + assert!( + !output.contains("SYNTHETIC_BOUNDARY_MARKER"), + "{name}: {output}" + ); + assert_eq!( + std::fs::read_to_string(&marker).unwrap(), + "SYNTHETIC_BOUNDARY_MARKER" + ); + assert!(!outside.join("new").exists()); + } + } + + #[test] + fn read_file_relative_escape_is_not_authorized_by_temp_prefix() { + let parent = tempfile::tempdir().unwrap(); + let project = parent.path().join("project"); + let outside = parent.path().join("outside"); + std::fs::create_dir(&project).unwrap(); + std::fs::create_dir(&outside).unwrap(); + let marker = "SYNTHETIC_OUTSIDE_BOUNDARY_MARKER"; + let target = outside.join("marker.txt"); + std::fs::write(&target, marker).unwrap(); + std::fs::write(project.join("inside.txt"), "inside control").unwrap(); + let executor = test_executor_in(&project); + assert!( + executor + .read_file(&json!({"path":"inside.txt"})) + .contains("inside control") + ); + assert!( + executor + .read_file(&json!({"path":target.to_str().unwrap()})) + .contains(marker), + "explicit absolute temp access is the passing control" + ); + let result = executor.read_file_with_metadata(&json!({"path":"../outside/marker.txt"})); + assert!( + !result.output.contains(marker), + "relative path disclosed the sibling fixture: {result:?}" + ); + assert!(result.is_error); + assert_eq!( + result.metadata.as_ref().unwrap()["error_kind"], + "sandbox_denied" + ); + } + // ── file_outline: integration via read_file ────────────────────────────── #[test] diff --git a/crates/astra-cli/src/tests/chat_stream_tests.rs b/crates/astra-cli/src/tests/chat_stream_tests.rs index 5fee3326f0..fda316c79f 100644 --- a/crates/astra-cli/src/tests/chat_stream_tests.rs +++ b/crates/astra-cli/src/tests/chat_stream_tests.rs @@ -2613,6 +2613,23 @@ async fn unsupported_workspace_host_rejects_public_turn_before_admission_http() ); assert_eq!(requests[0].method.as_str(), "GET"); assert_eq!(requests[0].url.path(), "/models"); + assert_eq!( + failure.partial.error_code.as_deref(), + Some("workspace_host_unsupported") + ); + assert!(failure.partial.server_admission_not_dispatched); + assert_eq!( + failure.partial.failure_kind, + Some(astra_core::ErrorKind::ToolUnavailable) + ); + assert_eq!( + failure.partial.error_metadata.as_ref().unwrap()["retryable"], + false + ); + assert!(!failure.partial.admission_rejected); + assert_eq!(failure.partial.prompt_tokens, 0); + assert_eq!(failure.partial.completion_tokens, 0); + assert_eq!(failure.partial.tool_calls_count, 0); assert!(failure.partial.run_id.is_none()); assert!(failure.partial.tool_call_records.is_empty()); } diff --git a/crates/astra-sandbox/src/path.rs b/crates/astra-sandbox/src/path.rs index 158389972a..f80116d35b 100644 --- a/crates/astra-sandbox/src/path.rs +++ b/crates/astra-sandbox/src/path.rs @@ -98,32 +98,22 @@ pub fn validate_path(policy: &SandboxPolicy, path: &str) -> Result PathBuf { /// Example: if `/home/user/proj/subdir` exists but `newfile.txt` doesn't, canonicalize /// `/home/user/proj/subdir` then append `newfile.txt`. pub fn canonicalize_parent_and_append(path: &Path) -> Result { - let mut current = path.to_path_buf(); - let mut suffix = Vec::new(); - - // Walk up until we find an existing ancestor - while !current.exists() { - if let Some(name) = current.file_name() { - suffix.push(name.to_os_string()); - } else { - break; - } - if !current.pop() { - break; - } - } - - // Canonicalize the existing ancestor - let canonical_base = if current.exists() { - current - .canonicalize() - .map_err(|e| SandboxPathError::ResolutionFailed { - requested: path.display().to_string(), - reason: e.to_string(), - })? - } else { - // Fallback: couldn't find any existing ancestor, normalize the whole path - return Ok(normalize_path(path)); + let failure = |error: std::io::Error| SandboxPathError::ResolutionFailed { + requested: path.display().to_string(), + reason: error.to_string(), }; - - // Append the suffix in reverse order (we collected bottom-up) - let mut result = canonical_base; - for segment in suffix.into_iter().rev() { - result.push(segment); + match path.canonicalize() { + Ok(canonical) => return Ok(canonical), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => {} + Err(error) => return Err(failure(error)), } + let mut result = PathBuf::new(); + for component in path.components() { + match component { + std::path::Component::CurDir => continue, + std::path::Component::ParentDir => { + result.pop(); + } + other => result.push(other.as_os_str()), + } + match result.canonicalize() { + Ok(canonical) => result = canonical, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => { + // A dangling symlink is not a new file. Do not authorize its + // spelling as though it were an ordinary missing component. + match std::fs::symlink_metadata(&result) { + Ok(metadata) if metadata.is_symlink() => return Err(failure(error)), + Err(error) if error.kind() != std::io::ErrorKind::NotFound => { + return Err(failure(error)); + } + _ => {} + } + } + Err(error) => return Err(failure(error)), + } + } Ok(result) } @@ -230,6 +221,81 @@ mod tests { SandboxPolicy::strict(root) } + #[test] + fn relative_authority_stays_in_workspace_despite_absolute_grants() { + let temp = tempfile::tempdir().unwrap(); + let root = temp.path().join("project"); + let outside = temp.path().join("outside"); + std::fs::create_dir_all(&root).unwrap(); + std::fs::create_dir_all(&outside).unwrap(); + std::fs::write(outside.join("marker"), "outside").unwrap(); + for mut policy in [ + SandboxPolicy::for_project(&root), + SandboxPolicy::strict(&root), + ] { + policy.allowed_paths.push(outside.clone()); + for input in ["../outside/marker", "../outside/new/deep/file"] { + assert!( + matches!( + validate_path(&policy, input), + Err(SandboxPathError::BoundaryEscape { .. }) + ), + "{input}" + ); + } + assert!(validate_path(&policy, outside.join("marker").to_str().unwrap()).is_ok()); + assert_eq!( + validate_path(&policy, "new/deep/../file").unwrap(), + root.canonicalize().unwrap().join("new/file") + ); + } + assert!(validate_path(&SandboxPolicy::permissive(&root), "../outside/marker").is_ok()); + } + + #[cfg(unix)] + #[test] + fn symlink_resolution_precedes_parent_normalization_and_missing_tails() { + use std::os::unix::fs::symlink; + let temp = tempfile::tempdir().unwrap(); + let root = temp.path().join("project"); + let outside = temp.path().join("outside/child"); + std::fs::create_dir_all(root.join("inside")).unwrap(); + std::fs::create_dir_all(&outside).unwrap(); + symlink(&outside, root.join("external")).unwrap(); + symlink(root.join("inside"), root.join("internal")).unwrap(); + symlink(&root, temp.path().join("alias")).unwrap(); + symlink(root.join("absent"), root.join("dangling")).unwrap(); + symlink(root.join("loop"), root.join("loop")).unwrap(); + let policy = SandboxPolicy::for_project(temp.path().join("alias")); + for input in [ + "external", + "external/../marker", + "external/new/deep/file", + "missing/../external/new", + ] { + assert!( + matches!( + validate_path(&policy, input), + Err(SandboxPathError::BoundaryEscape { .. }) + ), + "{input}" + ); + } + assert_eq!( + validate_path(&policy, "internal/new/deep").unwrap(), + root.canonicalize().unwrap().join("inside/new/deep") + ); + for input in ["dangling", "dangling/new", "loop", "loop/new"] { + assert!( + matches!( + validate_path(&policy, input), + Err(SandboxPathError::ResolutionFailed { .. }) + ), + "{input}" + ); + } + } + // ── Permissive isolation ───────────────────────────────────────────── #[test] diff --git a/crates/astra-sandbox/src/policy.rs b/crates/astra-sandbox/src/policy.rs index a9a5bd6203..f08210714e 100644 --- a/crates/astra-sandbox/src/policy.rs +++ b/crates/astra-sandbox/src/policy.rs @@ -3,22 +3,6 @@ use crate::path::{canonicalize_parent_and_append, normalize_path}; use std::path::{Path, PathBuf}; -fn unique_path_variants(path: &Path) -> Vec { - let mut variants = vec![normalize_path(path)]; - if let Ok(canonical) = path.canonicalize() - && !variants.iter().any(|existing| existing == &canonical) - { - variants.push(canonical); - } else if let Ok(canonical_parent) = canonicalize_parent_and_append(path) - && !variants - .iter() - .any(|existing| existing == &canonical_parent) - { - variants.push(canonical_parent); - } - variants -} - fn push_unique_path(paths: &mut Vec, path: PathBuf) { if !paths.iter().any(|existing| existing == &path) { paths.push(path); @@ -450,16 +434,22 @@ impl SandboxPolicy { if self.isolation == IsolationLevel::Permissive { return true; } - let path_variants = unique_path_variants(path); - let prefix_matches = |prefix: &Path| { - let prefix_variants = unique_path_variants(prefix); - path_variants.iter().any(|candidate| { - prefix_variants - .iter() - .any(|allowed| candidate.starts_with(allowed)) + canonicalize_parent_and_append(path) + .is_ok_and(|resolved| self.is_resolved_path_allowed(&resolved)) + } + + pub(crate) fn is_resolved_path_allowed(&self, resolved: &Path) -> bool { + self.is_within_project_root(resolved) + || self.allowed_paths.iter().any(|prefix| { + canonicalize_parent_and_append(prefix) + .is_ok_and(|prefix| resolved.starts_with(prefix)) }) - }; - prefix_matches(&self.project_root) || self.allowed_paths.iter().any(|ap| prefix_matches(ap)) + } + + /// Additional absolute grants cannot authorize relative workspace escapes. + pub(crate) fn is_within_project_root(&self, resolved: &Path) -> bool { + canonicalize_parent_and_append(&self.project_root) + .is_ok_and(|root| resolved.starts_with(root)) } /// Check if an environment variable should be passed to child process. diff --git a/crates/astra-test-harness/src/classify.rs b/crates/astra-test-harness/src/classify.rs index 7187f86ed1..50b1b39cd8 100644 --- a/crates/astra-test-harness/src/classify.rs +++ b/crates/astra-test-harness/src/classify.rs @@ -81,6 +81,11 @@ pub fn classify(outcome: &RunOutcome, criteria_results: &[CriterionResult]) -> F if outcome.exit_code < 0 { return FailureClass::Unknown; } + if outcome.is_pre_execution_failure() + && outcome.error_kind.as_deref() == Some("tool_unavailable") + { + return FailureClass::ToolUnavailable; + } // Exit 124 only proves that the outer deadline fired. If typed execution // evidence already proves model/tool progress, calling it an infrastructure // outage hides the product's pacing failure and tells reviewers to paper it diff --git a/crates/astra-test-harness/src/criteria.rs b/crates/astra-test-harness/src/criteria.rs index d52343200c..786eff0cdc 100644 --- a/crates/astra-test-harness/src/criteria.rs +++ b/crates/astra-test-harness/src/criteria.rs @@ -6466,6 +6466,8 @@ mod tests { fn outcome_with_tools(tools: &[&str]) -> RunOutcome { RunOutcome { + error_code: None, + server_admission_not_dispatched: false, model: "m".into(), exit_code: 0, text: "ok".into(), diff --git a/crates/astra-test-harness/src/exec.rs b/crates/astra-test-harness/src/exec.rs index e24303968c..6dfea08ab2 100644 --- a/crates/astra-test-harness/src/exec.rs +++ b/crates/astra-test-harness/src/exec.rs @@ -136,7 +136,9 @@ pub(crate) fn configured_astra_command(cfg: &RunnerConfig, case: &Case) -> tokio impl CaseExecutor for AstraCliExecutor { async fn execute(&self, case: &Case, model: &str) -> RunOutcome { let mut outcome = run_case_subprocess(&self.cfg, case, model).await; - if crate::criteria::requires_execution_capture(&case.criteria) { + if !outcome.is_pre_execution_failure() + && crate::criteria::requires_execution_capture(&case.criteria) + { let capture = crate::execution_capture::load(&self.cfg, case, &outcome).await; if let Some(stream) = outcome.stream_capture.as_mut() { match capture { @@ -432,12 +434,6 @@ async fn observe_machine_event_file( { observation.invalid = Some("machine event file ends with a partial line".into()); } - if let Ok(mut observation) = observation.lock() - && observation.invalid.is_none() - && observation.event_count == 0 - { - observation.invalid = Some("machine event file is empty".into()); - } Ok(()) } @@ -605,6 +601,8 @@ async fn run_case_subprocess(cfg: &RunnerConfig, case: &Case, model: &str) -> Ru Ok(dir) => dir, Err(error) => { return RunOutcome { + error_code: None, + server_admission_not_dispatched: false, model: model.into(), exit_code: -1, text: format!("failed to allocate machine-event directory: {error}"), @@ -667,6 +665,8 @@ async fn run_case_subprocess(cfg: &RunnerConfig, case: &Case, model: &str) -> Ru Ok(c) => c, Err(e) => { return RunOutcome { + error_code: None, + server_admission_not_dispatched: false, model: model.into(), exit_code: -1, text: format!("subprocess spawn error: {e}"), @@ -763,18 +763,38 @@ async fn run_case_subprocess(cfg: &RunnerConfig, case: &Case, model: &str) -> Ru deadline, ) .await; + // Empty is meaningful only after the terminal envelope and OS + // status agree. Corrupt or missing files never reach this exception. + let terminal = stdout.as_ref().ok().map(|bytes| { + parse_executor_outcome( + &String::from_utf8_lossy(bytes), + model, + status.code().unwrap_or(-1), + ) + }); + let machine = machine.and_then(|identity| { + let empty = machine_observation + .lock() + .map_err(|_| "machine event lock poisoned".to_string())? + .event_count + == 0; + if empty + && !terminal + .as_ref() + .is_some_and(RunOutcome::is_pre_execution_failure) + { + return Err("machine event file is empty".into()); + } + Ok(identity) + }); let (stdout, stderr, (observed_session_id, _observed_run_id)) = match ( stdout, stderr, machine, ) { (Ok(stdout), Ok(stderr), Ok(machine)) => (stdout, stderr, machine), - (Ok(stdout), Ok(stderr), Err(error)) => { + (Ok(_stdout), Ok(stderr), Err(error)) => { // Keep terminal diagnostics, but never use an unbound // identity to load a journal or certify task success. - let mut out = parse_executor_outcome( - &String::from_utf8_lossy(&stdout), - model, - status.code().unwrap_or(-1), - ); + let mut out = terminal.expect("stdout was collected"); out.exit_code = -1; out.session_id = None; out.run_id = None; @@ -798,16 +818,34 @@ async fn run_case_subprocess(cfg: &RunnerConfig, case: &Case, model: &str) -> Ru return finish_failed_evidence_outcome(cfg, &machine_observation, out).await; } }; - let stdout = String::from_utf8_lossy(&stdout).into_owned(); + let _ = stdout; let stderr = String::from_utf8_lossy(&stderr).into_owned(); - let process_exit = status.code().unwrap_or(-1); - let mut out = parse_executor_outcome(&stdout, model, process_exit); + let mut out = terminal.expect("stdout was collected"); if out.stderr.starts_with(PROTOCOL_ERROR_MARKER) && !stderr.is_empty() { out.stderr = format!("{}\n{stderr}", out.stderr); - } else if !out.stderr.starts_with(PROTOCOL_ERROR_MARKER) { - out.stderr = stderr; + } else if !out.stderr.starts_with(PROTOCOL_ERROR_MARKER) && !stderr.is_empty() { + if !out.stderr.is_empty() { + out.stderr.push('\n'); + } + out.stderr.push_str(&stderr); } out.duration_ms = start.elapsed().as_millis() as u64; + if out.is_pre_execution_failure() { + let has_events = machine_observation + .lock() + .expect("machine observation") + .event_count + != 0; + if has_events { + out.exit_code = -1; + out.session_id = None; + out.stderr = format!( + "{PROTOCOL_ERROR_MARKER} local rejection contains machine execution events" + ); + return finish_failed_evidence_outcome(cfg, &machine_observation, out).await; + } + return out; + } let cleanup_identity = reconcile_observed_session(&mut out, observed_session_id); if let Some(identity) = cleanup_identity { let cleanup = cleanup_observed_session(cfg, Some(&identity)).await; @@ -1281,6 +1319,8 @@ pub(crate) mod test_support { .get(&key) .cloned() .unwrap_or(RunOutcome { + error_code: None, + server_admission_not_dispatched: false, model: model.into(), exit_code: -1, text: format!("fake: no seed for {}/{}", case.name, model), @@ -1927,6 +1967,148 @@ mod tests { } } + #[cfg(unix)] + #[tokio::test] + async fn pre_execution_failure_allows_only_valid_empty_machine_evidence() { + let baseline = serde_json::json!({ + "trace_id":null, "request_id":null, "run_id":null, "session_id":null, + "text":"", "final_state":"failed", "interruption_kind":null, + "tool_result_class_counts":{}, "prompt_tokens":0, "fresh_prompt_tokens":0, + "cache":{"hit":false,"read_tokens":0,"creation_tokens":0}, + "completion_tokens":0,"llm_rounds":0,"tool_calls_count":0,"tools_used":[], + "persistence_error":null,"exit_code":1,"success":false, + "error_kind":"tool_unavailable","error_code":"workspace_host_unsupported", + "error":"administrator must repair the root-owned temporary directory", + "server_admission_not_dispatched":true + }); + for case in [ + "valid", + "selected_session", + "absent_fact", + "false_fact", + "wrong_type", + "progress", + "cached_progress", + "malformed_coverage", + "provider_coverage", + "authoritative_terminal", + "aggregate_progress", + "run", + "exit_mismatch", + "missing_file", + "corrupt_file", + "partial_file", + "bound_file", + ] { + let temp = tempfile::tempdir().unwrap(); + let shim = temp.path().join("local-failure"); + let mut terminal = baseline.clone(); + let mut process_exit = 1; + let events = match case { + "missing_file" => ":", + "corrupt_file" => r#"printf 'invalid\n' > "$arg""#, + "partial_file" => r#"printf '{' > "$arg""#, + "bound_file" => { + r#"printf '%s\n' '{"type":"session_bound","session_id":"550e8400-e29b-41d4-a716-446655440000"}' '{"type":"run_bound","session_id":"550e8400-e29b-41d4-a716-446655440000","run_id":"550e8400-e29b-41d4-a716-446655440001"}' > "$arg""# + } + _ => r#": > "$arg""#, + }; + match case { + "selected_session" => { + terminal["session_id"] = + serde_json::json!("550e8400-e29b-41d4-a716-446655440000") + } + "absent_fact" => { + terminal + .as_object_mut() + .unwrap() + .remove("server_admission_not_dispatched"); + } + "false_fact" => { + terminal["server_admission_not_dispatched"] = serde_json::json!(false) + } + "wrong_type" => { + terminal["server_admission_not_dispatched"] = serde_json::json!("true") + } + "progress" => terminal["llm_rounds"] = serde_json::json!(1), + "cached_progress" => terminal["cache"]["read_tokens"] = serde_json::json!(1), + "aggregate_progress" => terminal["prompt_tokens"] = serde_json::json!(1), + "malformed_coverage" => { + terminal["token_usage_coverage"] = serde_json::json!({ + "scope":"logical_provider_calls", "attempts":1,"provider_reported":1,"unavailable":0,"status":"none" + }) + } + "provider_coverage" => { + terminal["token_usage_coverage"] = serde_json::json!({ + "scope":"logical_provider_calls", "attempts":1,"provider_reported":1,"unavailable":0,"status":"complete" + }) + } + "authoritative_terminal" => { + terminal["server_terminal_authoritative"] = serde_json::json!(true) + } + "run" => terminal["run_id"] = serde_json::json!("run-1"), + "exit_mismatch" => process_exit = 3, + _ => {} + } + let cancelled = temp.path().join("cancelled-session"); + let cancelled_arg = shell_escape(cancelled.display().to_string()); + let script = format!( + r#"#!/bin/sh +if [ "$1" = session ]; then + if [ "$2" != cancel ] || [ "$3" != 550e8400-e29b-41d4-a716-446655440000 ]; then exit 88; fi + printf '%s\n' "$3" > {cancelled_arg} + printf '%s\n' '{{"session_id":"550e8400-e29b-41d4-a716-446655440000","status":"cancelled","execution_settled":true}}' + exit 0 +fi +next_is_events=0 +for arg in "$@"; do + if [ "$next_is_events" = 1 ]; then {events}; next_is_events=0; + elif [ "$arg" = --stream-events ]; then next_is_events=1; fi +done +printf '%s\n' '{terminal}' +exit {process_exit} +"# + ); + crate::test_support::write_executable_shim(&shim, &script).unwrap(); + let out = AstraCliExecutor::new(RunnerConfig::new(shim)) + .execute(&simple_case(), "m") + .await; + if case == "bound_file" { + assert_eq!( + std::fs::read_to_string(&cancelled).unwrap().trim(), + "550e8400-e29b-41d4-a716-446655440000" + ); + assert!(!out.stderr.contains("cleanup failed"), "{out:?}"); + assert!(out.session_id.is_none() && out.run_id.is_none()); + } else { + assert!( + !cancelled.exists(), + "{case} must not gain cancellation authority" + ); + } + if matches!(case, "valid" | "selected_session") { + assert_eq!(out.exit_code, 1, "{case}: {out:?}"); + assert_eq!( + out.error_code.as_deref(), + Some("workspace_host_unsupported") + ); + assert!(out.run_id.is_none()); + assert!(!out.stderr.starts_with(PROTOCOL_ERROR_MARKER)); + assert!(out.stderr.contains("administrator must repair")); + assert_eq!( + crate::classify::classify(&out, &[]), + crate::classify::FailureClass::ToolUnavailable + ); + } else { + assert_eq!(out.exit_code, -1, "{case}: {out:?}"); + assert!( + out.stderr.starts_with(PROTOCOL_ERROR_MARKER), + "{case}: {out:?}" + ); + } + } + } + #[tokio::test] async fn empty_machine_evidence_preserves_terminal_diagnostics_but_not_identity() { if !std::path::Path::new("/bin/sh").exists() { @@ -2656,6 +2838,8 @@ cat "$TERMINAL_FILE" async fn fake_executor_records_calls_and_returns_seeded_outcome() { let fe = test_support::FakeExecutor::new(); let mut seed = RunOutcome { + error_code: None, + server_admission_not_dispatched: false, model: "qwen-flash".into(), exit_code: 0, text: "hello".into(), diff --git a/crates/astra-test-harness/src/judger.rs b/crates/astra-test-harness/src/judger.rs index c13fdbf388..d80af25ca0 100644 --- a/crates/astra-test-harness/src/judger.rs +++ b/crates/astra-test-harness/src/judger.rs @@ -2020,6 +2020,8 @@ print({response:?}) fn dummy_outcome() -> RunOutcome { RunOutcome { + error_code: None, + server_admission_not_dispatched: false, model: "m".into(), exit_code: 0, text: "agent output".into(), @@ -2153,6 +2155,8 @@ print({response:?}) fn outcome_with_stderr(stderr: &str) -> RunOutcome { RunOutcome { + error_code: None, + server_admission_not_dispatched: false, model: "m".into(), exit_code: 0, text: "agent text".into(), diff --git a/crates/astra-test-harness/src/report.rs b/crates/astra-test-harness/src/report.rs index f1d3b4f159..c364fd2660 100644 --- a/crates/astra-test-harness/src/report.rs +++ b/crates/astra-test-harness/src/report.rs @@ -1259,6 +1259,8 @@ mod tests { fn mk_outcome() -> RunOutcome { RunOutcome { + error_code: None, + server_admission_not_dispatched: false, model: "m".into(), exit_code: 0, text: "hello".into(), diff --git a/crates/astra-test-harness/src/runner.rs b/crates/astra-test-harness/src/runner.rs index 057a9a3267..8573bddf3a 100644 --- a/crates/astra-test-harness/src/runner.rs +++ b/crates/astra-test-harness/src/runner.rs @@ -118,6 +118,12 @@ pub struct RunOutcome { /// can cause an interrupted outcome without being a user cancellation. #[serde(default)] pub error_kind: Option, + /// Stable source code retained independently of the failure category. + #[serde(default)] + pub error_code: Option, + /// True only when the CLI did not dispatch Server admission. + #[serde(default)] + pub server_admission_not_dispatched: bool, /// Counts of tool result classes observed during the run. #[serde(default)] pub tool_result_class_counts: std::collections::BTreeMap, @@ -130,6 +136,40 @@ pub struct RunOutcome { pub stream_capture: Option, } +impl RunOutcome { + /// A local failure has no physical execution to bind, inspect, or cancel. + pub(crate) fn is_pre_execution_failure(&self) -> bool { + self.server_admission_not_dispatched + && self.exit_code > 0 + && self.final_state.as_deref() == Some("failed") + && self + .error_kind + .as_deref() + .is_some_and(|kind| !kind.trim().is_empty()) + && self.run_id.is_none() + && self.text.is_empty() + && self.total_tool_calls == 0 + && self.cache_hits == 0 + && self.interruption_kind.is_none() + && self.prompt_tokens == 0 + && self.completion_tokens == 0 + && self.cached_input_tokens == 0 + && self.cache_creation_tokens == 0 + && self.turn_rounds == 0 + && self.tool_calls_count == 0 + && self.tools_used.is_empty() + && self + .tool_result_class_counts + .values() + .all(|count| *count == 0) + && self.token_usage_coverage.as_ref().is_none_or(|coverage| { + coverage.attempts == 0 + && coverage.provider_reported == 0 + && coverage.unavailable == 0 + }) + } +} + pub(crate) const MAX_STREAM_CAPTURE_BYTES: usize = 1024 * 1024; pub(crate) const MAX_STREAM_CAPTURE_RECORDS: usize = 4096; @@ -503,6 +543,8 @@ pub(crate) fn parse_json_outcome(stdout: &str, model: &str) -> RunOutcome { final_state: None, interruption_kind: None, error_kind: None, + error_code: None, + server_admission_not_dispatched: false, explain_capture: None, stream_capture: None, tool_result_class_counts: Default::default(), @@ -560,7 +602,11 @@ pub(crate) fn parse_json_outcome(stdout: &str, model: &str) -> RunOutcome { model: model.into(), exit_code: v.get("exit_code").and_then(|x| x.as_i64()).unwrap_or(0) as i32, text, - stderr: String::new(), + stderr: v + .get("error") + .and_then(|value| value.as_str()) + .unwrap_or("") + .to_owned(), session_id: v .get("session_id") .and_then(|x| x.as_str()) @@ -645,6 +691,14 @@ pub(crate) fn parse_json_outcome(stdout: &str, model: &str) -> RunOutcome { .get("error_kind") .and_then(|x| x.as_str()) .map(str::to_string), + error_code: v + .get("error_code") + .and_then(|value| value.as_str()) + .map(str::to_owned), + server_admission_not_dispatched: v + .get("server_admission_not_dispatched") + .and_then(|value| value.as_bool()) + .unwrap_or(false), tool_result_class_counts: v .get("tool_result_class_counts") .and_then(|x| x.as_object()) @@ -686,6 +740,21 @@ pub(crate) fn parse_strict_cli_outcome(stdout: &str, model: &str) -> Result Result RunOutcome "[astra-test] WARNING: stdout from astra chat --json violated the outcome envelope: {reason}" ); RunOutcome { + error_code: None, + server_admission_not_dispatched: false, model: model.into(), exit_code: -1, text: format!("invalid JSON outcome envelope: {reason}; payload={preview}"), diff --git a/crates/astra-test-harness/src/suite.rs b/crates/astra-test-harness/src/suite.rs index 5e9c567f66..8886c76112 100644 --- a/crates/astra-test-harness/src/suite.rs +++ b/crates/astra-test-harness/src/suite.rs @@ -44,13 +44,17 @@ fn attach_durable_judger_evidence(outcome: &mut RunOutcome, session: &SessionCap /// must never become an implicit deletion target. fn created_session_ids_for_cleanup(attempts: &[AttemptRecord]) -> Vec { let mut ids = BTreeSet::new(); - for id in attempts.iter().filter_map(|attempt| { - attempt - .outcome - .session_id - .as_deref() - .filter(|id| is_valid_server_session_id(id)) - }) { + for id in attempts + .iter() + .filter(|attempt| !attempt.outcome.is_pre_execution_failure()) + .filter_map(|attempt| { + attempt + .outcome + .session_id + .as_deref() + .filter(|id| is_valid_server_session_id(id)) + }) + { ids.insert(id.to_string()); } ids.into_iter().collect() @@ -819,7 +823,7 @@ impl<'a> SuiteRunner<'a> { // than left as the deterministic placeholder (which would // otherwise make every hard_judger step fail regardless of // the model's actual result). - if !self.no_judger { + if !step_outcome.is_pre_execution_failure() && !self.no_judger { for (criterion_index, criterion) in step.criteria.iter().enumerate() { if matches!( criterion, @@ -830,7 +834,7 @@ impl<'a> SuiteRunner<'a> { step_criteria_results[criterion_index] = result; } } - } else { + } else if !step_outcome.is_pre_execution_failure() { for (criterion_index, criterion) in step.criteria.iter().enumerate() { match criterion { Criterion::Judger { .. } => { @@ -848,8 +852,9 @@ impl<'a> SuiteRunner<'a> { } } let mut step_lifecycle_ok = true; - if step_outcome.exit_code != 0 - && !accepts_negative_terminal(&step.criteria, &step_outcome, None) + if step_outcome.is_pre_execution_failure() + || (step_outcome.exit_code != 0 + && !accepts_negative_terminal(&step.criteria, &step_outcome, None)) { step_lifecycle_ok = false; lifecycle_errors.push(format!( @@ -999,7 +1004,8 @@ impl<'a> SuiteRunner<'a> { } } - if case.steps.is_empty() { + let local_rejection = outcome.is_pre_execution_failure(); + if case.steps.is_empty() && !local_rejection { match outcome.session_id.as_deref() { Some(session_id) if !is_valid_server_session_id(session_id) => { lifecycle_errors.push(format!( @@ -1044,7 +1050,8 @@ impl<'a> SuiteRunner<'a> { crate::criteria::unconditional_memoria_settled_subsystem(&criteria) }; - let cleanup_enabled = self.runner_cfg.cleanup_created_sessions + let cleanup_enabled = !local_rejection + && self.runner_cfg.cleanup_created_sessions && case .extra_cli_args .iter() @@ -1080,7 +1087,8 @@ impl<'a> SuiteRunner<'a> { } else { None }; - if session.is_none() + if !local_rejection + && session.is_none() && (self.session_mode.should_load(case) || case.cleanup_memory_records || requires_session_capture(&criteria)) @@ -1104,6 +1112,9 @@ impl<'a> SuiteRunner<'a> { _ => missing_invocation_id = Some("root terminal outcome has no run_id".to_string()), } for (idx, step) in step_results.iter().enumerate() { + if step.outcome.is_pre_execution_failure() { + continue; + } match step.outcome.run_id.as_deref() { Some(run_id) if !run_id.trim().is_empty() => { invocation_run_ids.push(run_id.to_string()) @@ -1117,7 +1128,7 @@ impl<'a> SuiteRunner<'a> { invocation_run_ids.sort_unstable(); invocation_run_ids.dedup(); - if requires_durable_run_binding(&criteria) { + if !local_rejection && requires_durable_run_binding(&criteria) { let binding_error = if let Some(error) = missing_invocation_id.clone() { Some(format!( "hard durable evidence requires every invocation identity: {error}" @@ -1205,9 +1216,26 @@ impl<'a> SuiteRunner<'a> { crate::judger::semantic_child_evidence(&case.prompt, &outcome, adoption) }); + let local_failure = attempts + .iter() + .map(|attempt| &attempt.outcome) + .chain(step_results.iter().map(|step| &step.outcome)) + .find(|invocation| invocation.is_pre_execution_failure()); + // Always run the judger (unless --no-judger) — the quality // score is useful for diagnostics even when Hard criteria fail. - if !self.no_judger { + if local_failure.is_some() { + for (i, criterion) in criteria.iter().enumerate() { + if matches!( + criterion, + Criterion::Judger { .. } | Criterion::HardJudger { .. } + ) { + det[i].passed = false; + det[i].detail = + "judger not run: local execution prerequisites unavailable".into(); + } + } + } else if !self.no_judger { for (i, c) in criteria.iter().enumerate() { if !matches!(c, Criterion::Judger { .. } | Criterion::HardJudger { .. }) { continue; @@ -1261,7 +1289,8 @@ impl<'a> SuiteRunner<'a> { .filter(|c| c.severity == crate::criteria::CriterionSeverity::Hard) .all(|c| c.passed); let all_passed = det.iter().all(|c| c.passed) && steps_passed; - let product_passed = lifecycle_errors.is_empty() && hard_passed && steps_passed; + let product_passed = + local_failure.is_none() && lifecycle_errors.is_empty() && hard_passed && steps_passed; // Classify failure. let mut classification_criteria = det.clone(); @@ -1274,7 +1303,8 @@ impl<'a> SuiteRunner<'a> { let product_failure_class = if setup_failed { Some(crate::classify::FailureClass::PlatformSetupFailed) } else if !product_passed { - let terminal = if attempts.len() > 1 { root } else { &outcome }; + let terminal = + local_failure.unwrap_or(if attempts.len() > 1 { root } else { &outcome }); Some(classify(terminal, &classification_criteria)) } else { None @@ -1286,7 +1316,8 @@ impl<'a> SuiteRunner<'a> { }; // Digest on FAIL. - let (digest, digest_error) = if !product_passed + let (digest, digest_error) = if !local_rejection + && !product_passed && let Some(collector) = self.digest_collector && let Some(sid) = outcome.session_id.as_deref() { @@ -1775,6 +1806,109 @@ mod tests { } } + struct NoExternalWork; + impl SessionLoader for NoExternalWork { + fn load(&self, _: &str) -> Option { + panic!("local rejection must not load a journal"); + } + } + #[async_trait] + impl Judger for NoExternalWork { + async fn judge( + &self, + _: &Criterion, + _: &RunOutcome, + _: Option<&crate::judger::SemanticJudgerEvidence>, + ) -> Result { + panic!("local rejection must not invoke a provider"); + } + } + + #[tokio::test] + async fn local_rejection_never_loads_selected_session_or_calls_judger() { + let case = case_with( + "local-host", + vec![ + Criterion::SessionSubsystemHealthy { + settled_subsystem: None, + }, + Criterion::Judger { + question: "Was the task completed?".into(), + threshold: 0.7, + model: None, + }, + ], + ); + let executor = FakeExecutor::new(); + executor.seed( + &case.name, + "m", + RunOutcome { + model: "m".into(), + exit_code: 1, + session_id: Some("550e8400-e29b-41d4-a716-446655440000".into()), + final_state: Some("failed".into()), + error_kind: Some("tool_unavailable".into()), + error_code: Some("workspace_host_unsupported".into()), + server_admission_not_dispatched: true, + ..Default::default() + }, + ); + let external = NoExternalWork; + let mut runner = semantic_runner(&executor, &external, &external); + runner.runner_cfg.cleanup_created_sessions = true; + let report = runner.run_one(&case, "m", 0).await; + assert!(!report.is_passed()); + assert_eq!(report.failure_class, Some(FailureClass::ToolUnavailable)); + assert!(!report.outcome.stderr.contains("no run_id")); + assert_eq!(executor.calls.lock().unwrap().len(), 1); + } + + #[tokio::test] + async fn follow_up_local_rejection_skips_judging_and_preserves_admitted_root() { + let judger = Criterion::Judger { + question: "Was the task completed?".into(), + threshold: 0.7, + model: None, + }; + let mut case = case_with("follow-up-host", vec![judger.clone()]); + case.steps = vec![ + crate::case::CaseStep { + prompt: "write a file".into(), + criteria: vec![judger], + timeout_seconds: None, + }, + crate::case::CaseStep { + prompt: "must not continue".into(), + criteria: vec![], + timeout_seconds: None, + }, + ]; + let executor = FakeExecutor::new(); + let root = outcome_ok("m", "root answer", &[]); + let rejected = RunOutcome { + model: "m".into(), + exit_code: 1, + session_id: root.session_id.clone(), + final_state: Some("failed".into()), + error_kind: Some("tool_unavailable".into()), + error_code: Some("workspace_host_unsupported".into()), + server_admission_not_dispatched: true, + ..Default::default() + }; + executor.seed(&case.name, "m", root.clone()); + executor.seed(&format!("{}__step0", case.name), "m", rejected); + let runner = semantic_runner(&executor, &NoExternalWork, &NoopSessionLoader); + let report = runner.run_one(&case, "m", 0).await; + assert!(!report.is_passed()); + assert_eq!(report.failure_class, Some(FailureClass::ToolUnavailable)); + assert_eq!(report.outcome.run_id, root.run_id); + assert_eq!(report.outcome.text, root.text); + assert_eq!(report.steps.len(), 1); + assert!(report.steps[0].outcome.run_id.is_none()); + assert_eq!(executor.calls.lock().unwrap().len(), 2); + } + #[tokio::test] async fn matrix_binding_reaches_execution_steps_grading_and_reproducer() { struct Echo; @@ -2334,6 +2468,8 @@ mod tests { fn outcome_ok(model: &str, text: &str, tools: &[&str]) -> RunOutcome { RunOutcome { + error_code: None, + server_admission_not_dispatched: false, model: model.into(), exit_code: 0, text: text.into(), diff --git a/crates/astra-tools/src/executor.rs b/crates/astra-tools/src/executor.rs index cddaa381c2..2816882724 100644 --- a/crates/astra-tools/src/executor.rs +++ b/crates/astra-tools/src/executor.rs @@ -1262,7 +1262,7 @@ mod tests { let result = exec .execute( "write_file", - &serde_json::json!({"path": "../../etc/passwd", "delete": true}), + &serde_json::json!({"path": "../outside-marker", "delete": true}), ) .await; assert!( diff --git a/crates/astra-tools/src/fs_ops.rs b/crates/astra-tools/src/fs_ops.rs index ca1a9ceef2..fa9606d7db 100644 --- a/crates/astra-tools/src/fs_ops.rs +++ b/crates/astra-tools/src/fs_ops.rs @@ -228,22 +228,8 @@ fn validate_str_replace_anchor(edit_label: &str, old_str: &str) -> Result<(), St Ok(()) } -fn normalize_path(path: &Path) -> PathBuf { - path.components() - .fold(PathBuf::new(), |mut acc, component| { - match component { - std::path::Component::ParentDir => { - acc.pop(); - } - std::path::Component::CurDir => {} - other => acc.push(other), - } - acc - }) -} - fn unique_path_variants(path: &Path) -> Vec { - let mut variants = vec![normalize_path(path)]; + let mut variants = vec![astra_sandbox::normalize_path(path)]; if let Ok(canonical) = path.canonicalize() && !variants.iter().any(|existing| existing == &canonical) { @@ -258,17 +244,6 @@ fn unique_path_variants(path: &Path) -> Vec { variants } -fn is_within_workspace_root(path: &Path, workspace_root: &Path) -> bool { - let path_variants = unique_path_variants(path); - let root_variants = unique_path_variants(workspace_root); - - path_variants.iter().any(|candidate| { - root_variants - .iter() - .any(|root| candidate == root || candidate.starts_with(root)) - }) -} - pub(crate) fn relative_to_workspace_root(workspace_root: &Path, path: &Path) -> Option { let path_variants = unique_path_variants(path); let root_variants = unique_path_variants(workspace_root); @@ -314,62 +289,18 @@ pub fn resolve_path_sandboxed( relative: &str, allowed_paths: &[PathBuf], ) -> Result { - let input_is_absolute = Path::new(relative).is_absolute(); - let path = if input_is_absolute { - PathBuf::from(relative) - } else { - workspace_root.join(relative) - }; - - let normalized = normalize_path(&path); - - let final_path = if normalized.exists() { - normalized - .canonicalize() - .map_err(|e| format!("Cannot resolve path: {e}"))? - } else { - // Canonicalize parent directory to resolve symlinks in the path - // prefix even when the leaf doesn't exist yet. Without this, a - // symlink in the workspace pointing outside (e.g. → /etc) would - // bypass the sandbox check for not-yet-created files. - match normalized.parent() { - Some(parent) if parent.exists() => { - let canonical_parent = parent - .canonicalize() - .map_err(|e| format!("Cannot resolve parent path: {e}"))?; - match normalized.file_name() { - Some(name) => canonical_parent.join(name), - None => normalized, - } - } - _ => normalized, - } - }; - - // Check workspace root first. - if is_within_workspace_root(&final_path, workspace_root) { - return Ok(final_path); - } - - // The allowlist (e.g. `/tmp`) only applies to inputs the caller - // explicitly typed as absolute. A relative input that escapes the - // workspace via `..` and happens to land inside an allowed prefix is - // still a sandbox escape — the caller did not opt into the allowed - // root, the path traversal did. Without this guard, a workspace - // located under $TMPDIR can be escaped via `read_file("../foo")`. - if input_is_absolute { - for allowed in allowed_paths { - if is_within_workspace_root(&final_path, allowed) { - return Ok(final_path); - } + let mut policy = astra_sandbox::SandboxPolicy::for_project(workspace_root); + policy.allowed_paths = allowed_paths.to_vec(); + astra_sandbox::validate_path(&policy, relative).map_err(|error| { + if error.is_boundary_violation() { + format!( + "SANDBOX_DENIED: Path '{relative}' is outside workspace root '{}'", + workspace_root.display() + ) + } else { + error.to_string() } - } - - Err(format!( - "SANDBOX_DENIED: Path '{}' is outside workspace root '{}'", - relative, - workspace_root.display() - )) + }) } fn path_resolution_failed( @@ -3901,7 +3832,7 @@ mod tests { fn str_replace_resolves_non_secret_credential_reference() { let tmp = TempDir::new().unwrap(); let raw = "AWS_ACCESS_KEY_ID=AKIAIOSFODNN7EXAMPLE\n"; - let path = tmp.path().join("config.env"); + let path = tmp.path().join("config.conf"); std::fs::write(&path, raw).unwrap(); let (redacted, count) = astra_text_utils::credential_redaction::redact_credentials_in_text(raw); @@ -3911,7 +3842,7 @@ mod tests { let result = str_replace( tmp.path(), &serde_json::json!({ - "path": "config.env", + "path": "config.conf", "old_str": "[REDACTED:AWS_ACCESS_KEY:invalid]", "new_str": "[configured-access-key]" }), @@ -3926,7 +3857,7 @@ mod tests { let result = str_replace( tmp.path(), &serde_json::json!({ - "path": "config.env", + "path": "config.conf", "old_str": marker, "new_str": "[configured-access-key]" }), @@ -4017,7 +3948,7 @@ mod tests { fn str_replace_resolved_marker_equal_to_new_text_is_a_noop() { let tmp = TempDir::new().unwrap(); let raw = "AWS_ACCESS_KEY_ID=AKIAIOSFODNN7EXAMPLE\n"; - let path = tmp.path().join("config.env"); + let path = tmp.path().join("config.conf"); std::fs::write(&path, raw).unwrap(); let (redacted, count) = astra_text_utils::credential_redaction::redact_credentials_in_text(raw); @@ -4030,7 +3961,7 @@ mod tests { let result = str_replace( tmp.path(), &serde_json::json!({ - "path": "config.env", + "path": "config.conf", "old_str": marker, "new_str": "AKIAIOSFODNN7EXAMPLE" }), @@ -5103,10 +5034,24 @@ mod tests { assert_eq!(resolved, file.canonicalize().unwrap()); } + #[test] + fn canonical_path_policy_blocks_credential_files_before_reading() { + let root = TempDir::new().unwrap(); + std::fs::write(root.path().join("config.env"), "SYNTHETIC_CREDENTIAL_FILE").unwrap(); + let result = read_file(root.path(), &serde_json::json!({"path":"config.env"})); + assert!(result.is_error); + assert!( + result + .output + .contains("blocked as a sensitive credential path") + ); + assert!(!result.output.contains("SYNTHETIC_CREDENTIAL_FILE")); + } + #[test] fn path_traversal_blocked() { let tmp = TempDir::new().unwrap(); - let args = serde_json::json!({"path": "../../../etc/passwd"}); + let args = serde_json::json!({"path": "../outside-marker"}); let result = read_file(tmp.path(), &args); assert!(result.is_error); assert!(result.output.contains("SANDBOX_DENIED")); @@ -5261,7 +5206,7 @@ mod tests { ); let resolved = result.unwrap(); assert!( - is_within_workspace_root(&resolved, Path::new("/tmp")), + resolved.starts_with(Path::new("/tmp").canonicalize().unwrap()), "resolved temp path must stay within the configured temp root: {:?}", resolved ); diff --git a/docs/design/edge-runtime-tool-boundary.md b/docs/design/edge-runtime-tool-boundary.md index 72a92585bf..a28e6fb639 100644 --- a/docs/design/edge-runtime-tool-boundary.md +++ b/docs/design/edge-runtime-tool-boundary.md @@ -266,6 +266,13 @@ Side-effecting tools require stronger checks than read-only tools. | External mutation | explicit API authority and approval policy. | | Credential access | deny by default unless dedicated secret provider authorizes scoped access. | +Filesystem path authorization is owned by `astra-sandbox`. In Standard and Strict +isolation, relative inputs must stay within the selected workspace after resolving +symlinks and parent components, including nonexistent tails. Additional directory +grants apply to explicit absolute inputs; they cannot authorize relative traversal. +CLI and shared filesystem tools use this same resolver. Explicit Permissive mode +retains its broader authority and bypass-immune credential exclusions. + ### Workspace observation coordination Local shell and typed workspace writers must serialize each executor-owned @@ -278,6 +285,12 @@ Text-only and read-only surfaces do not require it. Unsupported hosts fail with an actionable local setup error before model execution; admission also rechecks the same trust boundary. Tool-level refusals use the non-retryable `workspace_host_unsupported` kind, distinct from a missing workspace binding. +A CLI preflight refusal preserves `error_kind=tool_unavailable`, +`error_code=workspace_host_unsupported`, non-retryable metadata, exit status `1`, +and `server_admission_not_dispatched=true` in terminal JSON. It creates no Run, +execution journal entry, accounting reconciliation, or usage publication. Human +output retains the stable code and administrator guidance. + Rebinding a workspace or changing cache roots cannot repair a host whose coordination directory is not trusted; use a supported host or have its administrator provision the required root-owned sticky directory. diff --git a/docs/reference/cli-commands.md b/docs/reference/cli-commands.md index 4d11efede6..6c64489c6f 100644 --- a/docs/reference/cli-commands.md +++ b/docs/reference/cli-commands.md @@ -302,5 +302,6 @@ level; verify the deployment's supported values before declaring its protocol. - CLIs share credential storage: `~/.astra/credentials.json` (tests may set `ASTRA_CREDENTIALS_DIR`) - `--profile` lets you isolate credentials by environment/user - API errors are returned with HTTP status and compact response body for easier debugging -- Once a chat stream has started, `astra chat --json` and `--print --output-format json` also return a structured result on hard failure (exit code 3). It retains the validated root run identity, partial output and observed usage, with `success: false` and partial evidence coverage. A request rejected before Run admission has no run identity; an error does not imply successful or resumable execution. +- Once a chat stream has started, `astra chat --json` and `--print --output-format json` also return a structured result on hard failure (exit code 3 for API failures). It retains the validated root run identity, partial output and observed usage, with `success: false` and partial evidence coverage. A request rejected before Run admission has no run identity; an error does not imply successful or resumable execution. +- An unsupported local workspace coordination host fails before admission with exit code `1`, `error_kind: "tool_unavailable"`, `error_code: "workspace_host_unsupported"`, non-retryable metadata, and `server_admission_not_dispatched: true`. No model or tool ran. Repair the host prerequisite described in the diagnostic; changing workspace grants does not repair it. - Interactive mode launches the TUI (requires a TTY; use `astra chat -m` or `--print` for non-interactive invocations). There is no separate console slash frontend. diff --git a/docs/testing/capability-harness.md b/docs/testing/capability-harness.md index 7cb826802b..e320405541 100644 --- a/docs/testing/capability-harness.md +++ b/docs/testing/capability-harness.md @@ -85,3 +85,15 @@ CLI stream observation uses its existing snapshot sink directly through CLI `harness_trace` copy and `RecordingKernel` wrapper are retired. Shared recorded traces, privacy projection and whole-session contract validation remain available to the execution harness. + +### Local failures before execution + +An existing empty machine-event file is valid only alongside a strict failed CLI +terminal envelope that agrees with the nonzero process status and explicitly +reports `server_admission_not_dispatched=true`, with no Run, output, model usage, +or tool execution. A selected Session is diagnostic context and grants no +inspection or cancellation authority. The harness retains the stable failure +code and category; it performs no session capture, cleanup, or semantic judging +for that invocation. The case remains failed. Missing files, malformed streams, +contradictory execution facts, and empty evidence for admitted runs remain +protocol failures. From b4fb38d279353b77fc86beb068f1c7cad1774812 Mon Sep 17 00:00:00 2001 From: XuPeng-SH Date: Thu, 22 Oct 2026 19:22:47 +0800 Subject: [PATCH 2/2] fix: preserve workspace recovery and CLI failure diagnostics --- crates/astra-cli/src/cli/command_router.rs | 25 ++- crates/astra-cli/src/cli/turn/turn_entry.rs | 153 +++++++++++++++++- .../src/cli/turn/turn_failure_reporting.rs | 9 ++ crates/astra-cli/src/cli/turn/turn_retry.rs | 77 ++++++--- .../astra-cli/src/cli/turn/turn_settlement.rs | 67 -------- .../src/cli/turn/turn_stream_runner.rs | 7 +- crates/astra-cli/tests/cli_pipe.rs | 24 ++- crates/astra-sandbox/src/path.rs | 34 +++- .../src/server/runtime_tool_executor.rs | 35 +++- docs/design/edge-runtime-tool-boundary.md | 5 +- docs/reference/cli-commands.md | 2 +- 11 files changed, 320 insertions(+), 118 deletions(-) diff --git a/crates/astra-cli/src/cli/command_router.rs b/crates/astra-cli/src/cli/command_router.rs index 7db8e72dd0..bf2e25a98d 100644 --- a/crates/astra-cli/src/cli/command_router.rs +++ b/crates/astra-cli/src/cli/command_router.rs @@ -2842,6 +2842,10 @@ async fn execute_cli_command_impl( return Ok(exit_code); } let failure_exit = hard_failure_exit_code(&e); + eprintln!( + "{}", + crate::cli::turn::turn_failure_reporting::turn_failure_diagnostic(&e) + ); if args.json { write_headless_stdout_line( &serde_json::to_string_pretty(&hard_failure_json_output(e)) @@ -2849,7 +2853,6 @@ async fn execute_cli_command_impl( )?; return Ok(failure_exit); } - eprintln!("{}", hard_failure_diagnostic(&e)); return Ok(failure_exit); } }; @@ -3791,18 +3794,6 @@ fn final_json_output(sr: &StreamResult, exit_code: ExitCode) -> serde_json::Valu final_json_output_with_context(sr, exit_code, trace_id, request_id) } -/// Project observed failure facts without committing a successful turn or -/// inferring a durable terminal from a transport error. Run identity comes -/// only from the physical stream's validated root binding. -fn hard_failure_diagnostic(failure: &crate::TurnFailure) -> String { - match failure.partial.error_code.as_deref() { - Some(code) if failure.partial.server_admission_not_dispatched => { - format!("[{code}] {}", failure.error) - } - _ => failure.error.clone(), - } -} - fn hard_failure_exit_code(failure: &crate::TurnFailure) -> ExitCode { if failure.partial.server_admission_not_dispatched && failure.partial.failure_kind == Some(astra_core::ErrorKind::ToolUnavailable) @@ -3813,6 +3804,9 @@ fn hard_failure_exit_code(failure: &crate::TurnFailure) -> ExitCode { } } +/// Project observed failure facts without committing a successful turn or +/// inferring a durable terminal from a transport error. Run identity comes +/// only from the physical stream's validated root binding. fn hard_failure_json_output(failure: crate::TurnFailure) -> serde_json::Value { let exit_code = hard_failure_exit_code(&failure); let partial = failure.partial; @@ -4111,6 +4105,10 @@ pub(crate) async fn run_print_mode( } Err(e) => { let failure_exit = hard_failure_exit_code(&e); + eprintln!( + "{}", + crate::cli::turn::turn_failure_reporting::turn_failure_diagnostic(&e) + ); if output_format == "json" || stream_json_emitter.is_some() { let mut output = hard_failure_json_output(e); if let Some(emitter) = stream_json_emitter.as_ref() { @@ -4128,7 +4126,6 @@ pub(crate) async fn run_print_mode( } return Ok(failure_exit); } - eprintln!("{}", hard_failure_diagnostic(&e)); return Ok(failure_exit); } }; diff --git a/crates/astra-cli/src/cli/turn/turn_entry.rs b/crates/astra-cli/src/cli/turn/turn_entry.rs index 5e70c7734e..7cf5f6fad8 100644 --- a/crates/astra-cli/src/cli/turn/turn_entry.rs +++ b/crates/astra-cli/src/cli/turn/turn_entry.rs @@ -245,6 +245,18 @@ async fn run_chat_turn(request: TurnExecutionRequest<'_>) -> TurnAttempt { return TurnAttempt::NotStarted(Box::new(failure)); } prepare_turn_adaptation(state, input.api, input.token, input.message).await; + let attempt = execute_stream_turn(TurnExecutionRequest { state, input }).await; + let not_started = match &attempt { + TurnAttempt::NotStarted(_) => true, + TurnAttempt::Interrupted(result) => result + .as_ref() + .as_ref() + .is_err_and(|failure| failure.partial.server_admission_not_dispatched), + TurnAttempt::Completed(_) => false, + }; + if not_started { + return attempt; + } if state .resume_guidance .as_ref() @@ -259,7 +271,7 @@ async fn run_chat_turn(request: TurnExecutionRequest<'_>) -> TurnAttempt { { state.diagnostics_context = None; } - execute_stream_turn(TurnExecutionRequest { state, input }).await + attempt } /// Establish the canonical identity transaction before an interactive turn @@ -884,6 +896,145 @@ mod tests { use crate::cli::session::session_state::SessionState; use crate::cli::stream::streaming_types::UsageAttribution; + #[tokio::test] + #[serial_test::serial] + async fn unsupported_host_preserves_recovery_through_turn_execution_and_settlement() { + use crate::cli::chat_stream::WORKSPACE_HOST_VALIDATION_FOR_TEST; + use crate::cli::turn::turn_settlement::TurnDispatch; + use astra_tools::workspace_observation::WorkspaceLeaseFailure; + use wiremock::matchers::{method, path}; + use wiremock::{Mock, MockServer, ResponseTemplate}; + + let _home = crate::test_utils::HomeGuard::temp(); + let _credentials = crate::test_utils::isolate_credentials(); + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/models")) + .respond_with( + ResponseTemplate::new(200) + .set_body_json(crate::test_utils::mock_model_catalog_json(&["test-model"])), + ) + .mount(&server) + .await; + let api = astra_thin_client::ThinClient::new(&server.uri(), None).unwrap(); + let session_id = uuid::Uuid::new_v4().to_string(); + let mut state = SessionState { + session_id: Some(session_id.clone()), + model: Some("test-model".into()), + turn: 7, + history: vec![("original task".into(), "unfinished".into())], + resume_guidance: Some("verify unfinished work".into()), + diagnostics_context: Some("existing diagnostic".into()), + resume_restricted_tools: vec!["bash".into()], + pending_bg_notifications: vec!["child completed".into()], + session_lessons_loaded: true, + perm_manager: crate::cli::permission_manager::PermissionManager::new(true), + tui_render_policy: Some(crate::cli::stream::stream_render::RenderPolicy::Silent), + unified_skill_registry: astra_runtime::skills::empty_unified_registry().clone(), + active_conversation: Some( + astra_turn_core::active_conversation::ActiveConversation::empty( + &crate::cli::cli_config::cli_utils::cli_user_id(), + &session_id, + ) + .unwrap(), + ), + ..SessionState::default() + }; + let required = vec![ + "verify unfinished work".into(), + "existing diagnostic".into(), + ]; + let input = super::TurnExecutionInput { + api: &api, + profile: None, + token: "synthetic-token", + message: "continue", + user_intent: "continue", + input_runtime_required_texts: &required, + input_runtime_volatile_texts: &[], + session_id: &session_id, + admitted_server_turn: None, + semantic_query_override: None, + explain_analyze_terminal_degraded: None, + }; + let attempt = WORKSPACE_HOST_VALIDATION_FOR_TEST + .scope( + Err(WorkspaceLeaseFailure::CoordinationHostUnsupported), + super::run_chat_turn(super::TurnExecutionRequest { + state: &mut state, + input, + }), + ) + .await; + let super::TurnAttempt::NotStarted(failure) = &attempt else { + panic!("unsupported host must remain an unstarted attempt"); + }; + assert_eq!( + failure.partial.error_code.as_deref(), + Some("workspace_host_unsupported") + ); + assert!(failure.partial.run_id.is_none()); + let usage = std::sync::Arc::new(std::sync::Mutex::new(None)); + let mut ui = crate::tests::TestUi::default(); + let context = TurnContext { + api: &api, + profile: None, + post_commit_tx: None, + admission: None, + explain_analyze_terminal_degraded: None, + }; + let mut dispatch = TurnDispatch { + ctx: &context, + line: "continue", + effective_line: "continue", + user_intent: "continue", + input_runtime_required_texts: &required, + input_runtime_volatile_texts: &[], + token: "synthetic-token", + session_id: &session_id, + admitted_server_turn: None, + semantic_query_override: None, + turn_start: std::time::Instant::now(), + ui: &mut ui, + turn_usage_sink: Some(&usage), + }; + let settled = super::settle_turn_attempt(&mut state, &mut dispatch, attempt, |_| { + Box::pin(async { panic!("unsupported host must not retry") }) + }) + .await + .unwrap(); + assert_eq!(settled, TurnSettlementOutcome::NotStarted); + assert_eq!(state.turn, 7); + assert_eq!( + state.history, + [("original task".into(), "unfinished".into())] + ); + assert_eq!( + state.resume_guidance.as_deref(), + Some("verify unfinished work") + ); + assert_eq!( + state.diagnostics_context.as_deref(), + Some("existing diagnostic") + ); + assert_eq!(state.resume_restricted_tools, ["bash"]); + assert_eq!(state.pending_bg_notifications, ["child completed"]); + assert!(state.journal.is_none()); + assert!(usage.lock().unwrap().is_none()); + assert_eq!(ui.restored_inputs, ["continue"]); + assert_eq!(ui.errors.len(), 1); + assert!(ui.errors[0].contains("workspace_host_unsupported")); + assert!(ui.errors[0].contains("administrator")); + let requests = server.received_requests().await.unwrap(); + assert!(!requests.is_empty()); + assert!( + requests + .iter() + .all(|request| request.method.as_str() == "GET" && request.url.path() == "/models"), + "no execution or refresh request is allowed" + ); + } + #[tokio::test] async fn conversation_input_cannot_execute_local_shell_syntax() { let directory = tempfile::tempdir().unwrap(); diff --git a/crates/astra-cli/src/cli/turn/turn_failure_reporting.rs b/crates/astra-cli/src/cli/turn/turn_failure_reporting.rs index fe5c4bf2d6..60af03da1e 100644 --- a/crates/astra-cli/src/cli/turn/turn_failure_reporting.rs +++ b/crates/astra-cli/src/cli/turn/turn_failure_reporting.rs @@ -9,6 +9,15 @@ use crate::cli::session::session_startup; use crate::cli::session::session_state::SessionState; use astra_services::session_journal; +pub(crate) fn turn_failure_diagnostic(failure: &crate::TurnFailure) -> String { + match failure.partial.error_code.as_deref() { + Some(code) if failure.partial.server_admission_not_dispatched => { + format!("[{code}] {}", failure.error) + } + _ => failure.error.clone(), + } +} + pub(crate) async fn reconcile_and_report_turn_failure( state: &mut SessionState, api: &astra_thin_client::ThinClient, diff --git a/crates/astra-cli/src/cli/turn/turn_retry.rs b/crates/astra-cli/src/cli/turn/turn_retry.rs index 717c638000..0153a7b324 100644 --- a/crates/astra-cli/src/cli/turn/turn_retry.rs +++ b/crates/astra-cli/src/cli/turn/turn_retry.rs @@ -1,6 +1,7 @@ //! Retry orchestration for recoverable turn failures. use super::turn_auth_retry::prepare_auth_refresh_retry; +use super::turn_failure_reporting::turn_failure_diagnostic; use super::turn_settlement::{ TurnDispatch, settle_failed_turn, settle_interrupted_turn, settle_successful_turn, }; @@ -35,22 +36,7 @@ pub(crate) async fn settle_turn_attempt( settle_not_started_turn(state, dispatch, &failure); Ok(TurnSettlementOutcome::NotStarted) } - TurnAttempt::Interrupted(result) => { - settle_interrupted_turn(state, dispatch, *result).await; - Ok(TurnSettlementOutcome::Interrupted) - } - TurnAttempt::Completed(result) => match *result { - Ok(result) => { - settle_successful_turn(state, dispatch, result).await; - Ok(TurnSettlementOutcome::Succeeded) - } - Err(mut failure) => { - // Once execution was attempted, retain and settle its facts. - // Only producer-owned local preflight may authorize a retry. - settle_failed_turn(state, dispatch, &mut failure).await; - Ok(TurnSettlementOutcome::Failed) - } - }, + attempt => Ok(settle_attempt_without_retry(state, dispatch, attempt).await), } } @@ -122,10 +108,10 @@ async fn try_retry_after_auth_refresh( }, }) .await; - Some(settle_retry_attempt(state, dispatch, retry).await) + Some(settle_attempt_without_retry(state, dispatch, retry).await) } -async fn settle_retry_attempt( +async fn settle_attempt_without_retry( state: &mut SessionState, dispatch: &mut TurnDispatch<'_, '_>, retry: TurnAttempt, @@ -136,6 +122,14 @@ async fn settle_retry_attempt( TurnSettlementOutcome::NotStarted } TurnAttempt::Interrupted(result) => { + if let Err(failure) = result.as_ref() + && failure.partial.server_admission_not_dispatched + { + // Stop never authorizes an auth retry, and an unsent request + // has no cancelled execution to journal or charge. + settle_not_started_turn(state, dispatch, failure); + return TurnSettlementOutcome::NotStarted; + } settle_interrupted_turn(state, dispatch, *result).await; TurnSettlementOutcome::Interrupted } @@ -157,7 +151,7 @@ fn settle_not_started_turn( dispatch: &mut TurnDispatch<'_, '_>, failure: &crate::TurnFailure, ) { - dispatch.ui.show_error(&failure.error); + dispatch.ui.show_error(&turn_failure_diagnostic(failure)); if !dispatch.line.trim().is_empty() { let _ = dispatch .ui @@ -353,7 +347,14 @@ mod tests { admission: None, explain_analyze_terminal_degraded: None, }; - for retry in [false, true] { + for (retry, local, cancelled) in [ + (false, false, false), + (true, false, false), + (false, true, false), + (true, true, false), + (false, true, true), + (true, true, true), + ] { let mut state = SessionState { session_id: Some("same-session".into()), turn: 7, @@ -380,12 +381,29 @@ mod tests { ui: &mut ui, turn_usage_sink: Some(&usage), }; - let attempt = TurnAttempt::NotStarted(Box::new(crate::TurnFailure { - error: "model selection rejected".into(), - partial: Default::default(), - })); + let failure = if local { + crate::TurnFailure { + error: "administrator must repair the coordination host".into(), + partial: crate::PartialTurnData { + server_admission_not_dispatched: true, + error_code: Some("workspace_host_unsupported".into()), + failure_kind: Some(astra_core::ErrorKind::ToolUnavailable), + ..Default::default() + }, + } + } else { + crate::TurnFailure { + error: "model selection rejected".into(), + partial: Default::default(), + } + }; + let attempt = if cancelled { + TurnAttempt::Interrupted(Box::new(Err(failure))) + } else { + TurnAttempt::NotStarted(Box::new(failure)) + }; let outcome = if retry { - super::settle_retry_attempt(&mut state, &mut dispatch, attempt).await + super::settle_attempt_without_retry(&mut state, &mut dispatch, attempt).await } else { settle_turn_attempt(&mut state, &mut dispatch, attempt, |_| { Box::pin(async { panic!("preflight rejection cannot execute a turn") }) @@ -405,7 +423,14 @@ mod tests { assert_eq!(state.pending_bg_notifications, ["child complete"]); assert_eq!(state.resume_restricted_tools, ["bash"]); assert_eq!(ui.restored_inputs, ["/model literal task"]); - assert_eq!(ui.errors, ["model selection rejected"]); + assert_eq!( + ui.errors, + [if local { + "[workspace_host_unsupported] administrator must repair the coordination host" + } else { + "model selection rejected" + }] + ); assert!(usage.lock().unwrap().is_none()); } } diff --git a/crates/astra-cli/src/cli/turn/turn_settlement.rs b/crates/astra-cli/src/cli/turn/turn_settlement.rs index b0c77bc09b..be8cc383e4 100644 --- a/crates/astra-cli/src/cli/turn/turn_settlement.rs +++ b/crates/astra-cli/src/cli/turn/turn_settlement.rs @@ -82,24 +82,6 @@ pub(crate) async fn settle_failed_turn( dispatch: &mut TurnDispatch<'_, '_>, failure: &mut crate::TurnFailure, ) { - if failure.partial.server_admission_not_dispatched { - if !dispatch.line.trim().is_empty() { - dispatch - .ui - .restore_input(dispatch.line, state.session_id.as_deref()); - } - dispatch.ui.show_error(&format!( - "[{}] {}", - failure - .partial - .error_code - .as_deref() - .unwrap_or("local_error"), - failure.error - )); - clear_recovery_scoped_turn_restrictions(state); - return; - } if failure.partial.admission_rejected { report_admission_rejection(state, dispatch.line, failure, dispatch.ui); clear_recovery_scoped_turn_restrictions(state); @@ -130,55 +112,6 @@ mod tests { use crate::cli::session::session_state::SessionState; use std::time::Instant; - #[tokio::test] - async fn local_rejection_restores_draft_without_journal_or_usage() { - let api = astra_thin_client::ThinClient::new("http://127.0.0.1:9", None).unwrap(); - let ctx = TurnContext { - api: &api, - profile: None, - post_commit_tx: None, - admission: None, - explain_analyze_terminal_degraded: None, - }; - let mut ui = crate::tests::TestUi::default(); - let mut state = SessionState::default(); - let usage = std::sync::Arc::new(std::sync::Mutex::new(None)); - let mut dispatch = TurnDispatch { - ctx: &ctx, - line: "write a file", - effective_line: "write a file", - user_intent: "write a file", - input_runtime_required_texts: &[], - input_runtime_volatile_texts: &[], - token: "synthetic-token", - session_id: "", - admitted_server_turn: None, - semantic_query_override: None, - turn_start: std::time::Instant::now(), - ui: &mut ui, - turn_usage_sink: Some(&usage), - }; - let mut failure = crate::TurnFailure { - error: "administrator must repair the root-owned temporary directory".into(), - partial: crate::PartialTurnData { - server_admission_not_dispatched: true, - failure_kind: Some(astra_core::ErrorKind::ToolUnavailable), - error_code: Some("workspace_host_unsupported".into()), - ..Default::default() - }, - }; - settle_failed_turn(&mut state, &mut dispatch, &mut failure).await; - assert_eq!(ui.restored_inputs, vec!["write a file"]); - assert!(state.journal.is_none()); - assert!(usage.lock().unwrap().is_none()); - assert!( - ui.errors - .iter() - .any(|message| message.contains("workspace_host_unsupported") - && message.contains("administrator")) - ); - } - #[tokio::test] async fn settle_successful_turn_clears_last_turn_interrupted() { let api = astra_thin_client::ThinClient::new("http://127.0.0.1:9", None).unwrap(); diff --git a/crates/astra-cli/src/cli/turn/turn_stream_runner.rs b/crates/astra-cli/src/cli/turn/turn_stream_runner.rs index afa570c74a..5486d4c5c0 100644 --- a/crates/astra-cli/src/cli/turn/turn_stream_runner.rs +++ b/crates/astra-cli/src/cli/turn/turn_stream_runner.rs @@ -114,7 +114,12 @@ pub(crate) async fn execute_stream_turn(request: TurnExecutionRequest<'_>) -> Tu if was_user_cancel { TurnAttempt::Interrupted(Box::new(result)) } else { - TurnAttempt::Completed(Box::new(result)) + match result { + Err(failure) if failure.partial.server_admission_not_dispatched => { + TurnAttempt::NotStarted(Box::new(failure)) + } + result => TurnAttempt::Completed(Box::new(result)), + } } } diff --git a/crates/astra-cli/tests/cli_pipe.rs b/crates/astra-cli/tests/cli_pipe.rs index 9fb94b34e0..dc727e75d6 100644 --- a/crates/astra-cli/tests/cli_pipe.rs +++ b/crates/astra-cli/tests/cli_pipe.rs @@ -532,7 +532,7 @@ async fn headless_hard_failure_preserves_only_the_admitted_root_identity() { let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); let base_url = format!("http://{}", listener.local_addr().unwrap()); let server = tokio::spawn(async move { axum::serve(listener, app).await.unwrap() }); - for mode in ["chat-json", "print-json", "stream-json"] { + for mode in ["chat-json", "print-json", "stream-json", "chat-text"] { let home = tempfile::tempdir().unwrap(); let mut command = tokio::process::Command::new(env!("CARGO_BIN_EXE_astra")); command.args([ @@ -544,16 +544,18 @@ async fn headless_hard_failure_preserves_only_the_admitted_root_identity() { "mock-model", "--no-instructions", ]); - if mode == "chat-json" { + if mode == "chat-json" || mode == "chat-text" { command.args([ "chat", "--no-resume", "--permission-mode", "auto", - "--json", "-m", "exercise hard failure", ]); + if mode == "chat-json" { + command.arg("--json"); + } } else { command.args([ "--print", @@ -602,6 +604,22 @@ async fn headless_hard_failure_preserves_only_the_admitted_root_identity() { "{mode}, admitted={admitted}: {}", String::from_utf8_lossy(&output.stderr) ); + if mode == "chat-text" { + assert_no_completed_turn(home.path().join(".astra/sessions"), SESSION); + continue; + } + let diagnostic = if !admitted && mode == "stream-json" { + "without a bridge run_id" + } else { + "402" + }; + assert_eq!( + String::from_utf8_lossy(&output.stderr) + .matches(diagnostic) + .count(), + 1, + "{mode}: product failure must be printed once" + ); let result: Value = if mode == "stream-json" { let records = stdout .lines() diff --git a/crates/astra-sandbox/src/path.rs b/crates/astra-sandbox/src/path.rs index f80116d35b..bf7b954c7a 100644 --- a/crates/astra-sandbox/src/path.rs +++ b/crates/astra-sandbox/src/path.rs @@ -181,7 +181,11 @@ pub fn canonicalize_parent_and_append(path: &Path) -> Result return Err(failure(error)), } - let mut result = PathBuf::new(); + let mut result = if path.is_absolute() { + PathBuf::new() + } else { + Path::new(".").canonicalize().map_err(failure)? + }; for component in path.components() { match component { std::path::Component::CurDir => continue, @@ -221,6 +225,34 @@ mod tests { SandboxPolicy::strict(root) } + #[test] + fn relative_workspace_resolves_missing_targets_from_the_current_directory() { + let root = Path::new(".").canonicalize().unwrap(); + let name = format!("astra-missing-{}", uuid::Uuid::new_v4()); + let policy = SandboxPolicy::for_project("."); + assert_eq!(validate_path(&policy, ".").unwrap(), root); + for input in [name.clone(), format!("{name}/new/deep.txt")] { + assert!(!root.join(&input).exists()); + let expected = root.join(&input); + assert_eq!(validate_path(&policy, &input).unwrap(), expected); + assert_eq!( + validate_path(&SandboxPolicy::for_project(&root), &input).unwrap(), + expected + ); + } + let outside = format!("../{name}/new.txt"); + assert_eq!( + canonicalize_parent_and_append(Path::new(&outside)).unwrap(), + root.parent().unwrap_or(&root).join(&name).join("new.txt") + ); + if root.parent().is_some() { + assert!(matches!( + validate_path(&policy, &outside), + Err(SandboxPathError::BoundaryEscape { .. }) + )); + } + } + #[test] fn relative_authority_stays_in_workspace_despite_absolute_grants() { let temp = tempfile::tempdir().unwrap(); diff --git a/crates/runtime/src/server/runtime_tool_executor.rs b/crates/runtime/src/server/runtime_tool_executor.rs index 3b7fa8bfd2..cdc0e1c66d 100644 --- a/crates/runtime/src/server/runtime_tool_executor.rs +++ b/crates/runtime/src/server/runtime_tool_executor.rs @@ -14030,11 +14030,40 @@ pub(crate) mod tests { #[tokio::test] async fn read_file_blocks_path_traversal() { - let (exec, _dir) = test_executor(); + let (exec, dir) = test_executor(); + let outside = tempfile::tempdir_in(dir.path().parent().unwrap()).unwrap(); + let marker = outside.path().join("marker.txt"); + std::fs::write(&marker, "SYNTHETIC_OUTSIDE_MARKER").unwrap(); + let relative = format!( + "../{}/marker.txt", + outside.path().file_name().unwrap().to_str().unwrap() + ); + let result = exec.execute("read_file", &json!({"path": relative})).await; + assert!(result.contains("SANDBOX_DENIED")); + assert!(!result.contains("SYNTHETIC_OUTSIDE_MARKER")); + assert_eq!( + std::fs::read_to_string(marker).unwrap(), + "SYNTHETIC_OUTSIDE_MARKER" + ); + } + + #[tokio::test] + async fn read_file_blocks_sensitive_workspace_file() { + let (exec, dir) = test_executor(); + let marker = dir.path().join("config.env"); + std::fs::write(&marker, "SYNTHETIC_CREDENTIAL_MARKER").unwrap(); let result = exec - .execute("read_file", &json!({"path": "../../etc/passwd"})) + .execute("read_file", &json!({"path": "config.env"})) .await; - assert!(result.contains("SANDBOX_DENIED")); + assert!( + result.contains("blocked as a sensitive credential path"), + "{result}" + ); + assert!(!result.contains("SYNTHETIC_CREDENTIAL_MARKER")); + assert_eq!( + std::fs::read_to_string(marker).unwrap(), + "SYNTHETIC_CREDENTIAL_MARKER" + ); } #[tokio::test] diff --git a/docs/design/edge-runtime-tool-boundary.md b/docs/design/edge-runtime-tool-boundary.md index a28e6fb639..ba830f0d96 100644 --- a/docs/design/edge-runtime-tool-boundary.md +++ b/docs/design/edge-runtime-tool-boundary.md @@ -289,7 +289,10 @@ A CLI preflight refusal preserves `error_kind=tool_unavailable`, `error_code=workspace_host_unsupported`, non-retryable metadata, exit status `1`, and `server_admission_not_dispatched=true` in terminal JSON. It creates no Run, execution journal entry, accounting reconciliation, or usage publication. Human -output retains the stable code and administrator guidance. +output retains the stable code and administrator guidance on stderr even when +stdout uses JSON or stream-json. Interactive refusal uses the existing +not-started settlement: the draft, recovery guidance and tool restrictions remain +available for a later attempt. Rebinding a workspace or changing cache roots cannot repair a host whose coordination directory is not trusted; use a supported host or have its diff --git a/docs/reference/cli-commands.md b/docs/reference/cli-commands.md index 6c64489c6f..fdb2afeba0 100644 --- a/docs/reference/cli-commands.md +++ b/docs/reference/cli-commands.md @@ -303,5 +303,5 @@ level; verify the deployment's supported values before declaring its protocol. - `--profile` lets you isolate credentials by environment/user - API errors are returned with HTTP status and compact response body for easier debugging - Once a chat stream has started, `astra chat --json` and `--print --output-format json` also return a structured result on hard failure (exit code 3 for API failures). It retains the validated root run identity, partial output and observed usage, with `success: false` and partial evidence coverage. A request rejected before Run admission has no run identity; an error does not imply successful or resumable execution. -- An unsupported local workspace coordination host fails before admission with exit code `1`, `error_kind: "tool_unavailable"`, `error_code: "workspace_host_unsupported"`, non-retryable metadata, and `server_admission_not_dispatched: true`. No model or tool ran. Repair the host prerequisite described in the diagnostic; changing workspace grants does not repair it. +- An unsupported local workspace coordination host fails before admission with exit code `1`, `error_kind: "tool_unavailable"`, `error_code: "workspace_host_unsupported"`, non-retryable metadata, and `server_admission_not_dispatched: true`. No model or tool ran. Interactive mode restores the draft and preserves recovery state. Human diagnostics remain on stderr in text, JSON, and stream-json modes. Repair the host prerequisite described in the diagnostic; changing workspace grants does not repair it. - Interactive mode launches the TUI (requires a TTY; use `astra chat -m` or `--print` for non-interactive invocations). There is no separate console slash frontend.