diff --git a/README.md b/README.md index 7484595..027b8c2 100644 --- a/README.md +++ b/README.md @@ -135,6 +135,7 @@ apw --help apw app install apw app launch apw doctor +apw doctor --bundle /tmp/apw-diagnostics.tar.gz APW_LOG=debug apw status --json apw status apw status --json diff --git a/docs/SECURITY_POSTURE_AND_TESTING.md b/docs/SECURITY_POSTURE_AND_TESTING.md index a619fd3..0949e1e 100644 --- a/docs/SECURITY_POSTURE_AND_TESTING.md +++ b/docs/SECURITY_POSTURE_AND_TESTING.md @@ -50,6 +50,38 @@ Release reference version: `v2.0.0` the doctor command - timed-out requests do not cache or persist partially returned credentials +### Diagnostic-bundle export + +`apw doctor --bundle ` writes a tar.gz that operators can attach to +support requests. The bundle is deterministic and fails closed rather than +shipping incompletely-redacted material. + +Layout (see `rust/src/bundle.rs` for the source of truth): + +``` +apw-doctor-bundle/ + manifest.json # bundleVersion, files, redaction guarantees + doctor.json # full `apw doctor --json` payload + environment.json # `apw doctor --ci` environment checks + os.json # uname, arch/os, sw_vers on macOS + native-app/file-listing.json # path/size/mode for files under ~/.apw/native-app/ +``` + +Redaction guarantees: + +- environment variables are never read or copied into the bundle +- file contents under `~/.apw/native-app/` are never read — only the metadata + listing (relative path, byte size, octal mode, file type) is included +- `credentials.json`, `config.json`, and `broker.log` are explicitly excluded +- every string in the bundle JSON is scanned for token-like patterns + (long high-entropy alphanumeric runs, common vendor key prefixes, and the + in-tree demo password sentinel); a match aborts the bundle with an + `InvalidConfig` (102) error and does not write the archive +- the archive file is written mode `0600` + +If an operator needs to share broker logs or config they attach those +separately, after redacting by hand. + ## Required release gates Run these before publishing: @@ -77,6 +109,8 @@ The Rust test suite covers: - native app diagnostics and `APW_DEMO=1` bootstrap credential file initialization - end-to-end v2 app install, launch, status, doctor, and login flows - direct-exec fallback, unsupported-domain handling, denial handling, and malformed broker response mapping +- diagnostic-bundle layout, archive permissions, and fail-closed redaction + when a plausible credential pattern would otherwise reach the bundle - external fallback provider path hardening, including relative paths, `~`, world-writable executables, and symlink targets - diagnostic-bundle redaction and fail-closed aborts when staged diagnostics look diff --git a/rust/src/bundle.rs b/rust/src/bundle.rs new file mode 100644 index 0000000..265795d --- /dev/null +++ b/rust/src/bundle.rs @@ -0,0 +1,621 @@ +//! Diagnostic-bundle export. `apw doctor --bundle ` packages a +//! deterministic, redacted tarball that operators can attach to support +//! requests without leaking credentials. See issue #56. +//! +//! ## Layout +//! +//! ```text +//! apw-doctor-bundle/ +//! manifest.json # bundle version + included files + redaction notes +//! doctor.json # full `apw doctor --json` output +//! environment.json # `apw doctor --ci` environment checks +//! os.json # uname + sw_vers (macOS) probes +//! native-app/file-listing.json # path/size/mode listing only (no contents) +//! ``` +//! +//! ## Redaction guarantees +//! +//! - No environment variables are read or included. +//! - No file contents from `~/.apw/native-app/` are included — only +//! metadata (relative path, byte size, octal mode). +//! - The bundle never reads `credentials.json`, `config.json`, or +//! `broker.log`; an operator who wants to share those attaches them +//! separately with their own judgement. +//! - Free-text strings written into the bundle (e.g. probe `message` +//! fields) are scanned by `redact_value` for token-like patterns; any +//! match aborts the bundle so the operator never ships an +//! incompletely-redacted archive. + +use crate::error::{APWError, Result}; +use crate::native_app::{native_app_runtime_dir, uuid_like_suffix}; +use crate::types::Status; +use chrono::Utc; +use serde_json::{json, Value}; +use std::fs; +use std::os::unix::fs::PermissionsExt; +use std::path::{Path, PathBuf}; +use std::process::{Command, Stdio}; + +const BUNDLE_VERSION: u32 = 1; +const BUNDLE_ROOT_NAME: &str = "apw-doctor-bundle"; + +/// Outcome of writing a diagnostic bundle. Returned to the CLI so the +/// human-facing summary can include the file count and applied redaction +/// counters. +#[derive(Debug)] +pub struct BundleResult { + pub path: PathBuf, + pub files_included: Vec, + pub redaction_checks: u32, +} + +/// Write a deterministic, redacted diagnostic bundle to `output_path`. +/// +/// Fails closed if any structured value contains a token-like substring +/// that could be a credential — the operator is told what to remove rather +/// than shipping an incompletely-redacted archive. +pub fn write_diagnostic_bundle( + output_path: &Path, + doctor_payload: &Value, + environment: &Value, +) -> Result { + if output_path.as_os_str().is_empty() { + return Err(APWError::new( + Status::InvalidParam, + "Bundle output path must not be empty.", + )); + } + if output_path.is_dir() { + return Err(APWError::new( + Status::InvalidParam, + format!( + "Bundle output path {} is a directory; expected a file.", + output_path.display() + ), + )); + } + + let staging = StagingDir::create()?; + let root = staging.path().join(BUNDLE_ROOT_NAME); + fs::create_dir(&root).map_err(|error| { + APWError::new( + Status::ProcessNotRunning, + format!("Failed to prepare bundle staging directory: {error}"), + ) + })?; + + let mut redaction_checks: u32 = 0; + let mut files_included: Vec = Vec::new(); + + write_json_file( + &root, + "doctor.json", + doctor_payload, + &mut files_included, + &mut redaction_checks, + )?; + write_json_file( + &root, + "environment.json", + environment, + &mut files_included, + &mut redaction_checks, + )?; + + let os_info = collect_os_info(); + write_json_file( + &root, + "os.json", + &os_info, + &mut files_included, + &mut redaction_checks, + )?; + + fs::create_dir(root.join("native-app")).map_err(|error| { + APWError::new( + Status::ProcessNotRunning, + format!("Failed to create native-app dir in bundle staging: {error}"), + ) + })?; + let listing = collect_native_app_listing(); + write_json_file( + &root, + "native-app/file-listing.json", + &listing, + &mut files_included, + &mut redaction_checks, + )?; + + let manifest = json!({ + "bundleVersion": BUNDLE_VERSION, + "createdAt": Utc::now().to_rfc3339(), + "files": files_included, + "redactionGuarantees": { + "envVarsExcluded": true, + "nativeAppFileContentsExcluded": true, + "brokerLogExcluded": true, + "credentialsJsonExcluded": true, + "configJsonExcluded": true, + }, + "redactionChecks": redaction_checks, + }); + let manifest_bytes = serde_json::to_vec_pretty(&manifest).map_err(|error| { + APWError::new( + Status::GenericError, + format!("Failed to encode bundle manifest: {error}"), + ) + })?; + fs::write(root.join("manifest.json"), &manifest_bytes).map_err(|error| { + APWError::new( + Status::ProcessNotRunning, + format!("Failed to write bundle manifest: {error}"), + ) + })?; + fs::set_permissions( + root.join("manifest.json"), + fs::Permissions::from_mode(0o600), + ) + .map_err(|error| { + APWError::new( + Status::InvalidConfig, + format!("Failed to chmod bundle manifest: {error}"), + ) + })?; + files_included.insert(0, "manifest.json".to_string()); + + if let Some(parent) = output_path.parent() { + if !parent.as_os_str().is_empty() { + fs::create_dir_all(parent).map_err(|error| { + APWError::new( + Status::ProcessNotRunning, + format!( + "Failed to create bundle output parent {}: {error}", + parent.display() + ), + ) + })?; + } + } + if output_path.exists() { + fs::remove_file(output_path).map_err(|error| { + APWError::new( + Status::ProcessNotRunning, + format!( + "Failed to remove existing bundle at {}: {error}", + output_path.display() + ), + ) + })?; + } + + let status = Command::new("tar") + .arg("-czf") + .arg(output_path) + .arg("-C") + .arg(staging.path()) + .arg(BUNDLE_ROOT_NAME) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::piped()) + .status() + .map_err(|error| { + APWError::new( + Status::ProcessNotRunning, + format!("Failed to launch tar to package the bundle: {error}"), + ) + })?; + if !status.success() { + return Err(APWError::new( + Status::ProcessNotRunning, + format!("tar exited with status {status} while packaging the bundle."), + )); + } + + fs::set_permissions(output_path, fs::Permissions::from_mode(0o600)).map_err(|error| { + APWError::new( + Status::InvalidConfig, + format!( + "Failed to chmod bundle archive at {}: {error}", + output_path.display() + ), + ) + })?; + + Ok(BundleResult { + path: output_path.to_path_buf(), + files_included, + redaction_checks, + }) +} + +fn write_json_file( + root: &Path, + relative_path: &str, + payload: &Value, + files_included: &mut Vec, + redaction_checks: &mut u32, +) -> Result<()> { + audit_redaction(payload, redaction_checks)?; + let bytes = serde_json::to_vec_pretty(payload).map_err(|error| { + APWError::new( + Status::GenericError, + format!("Failed to encode {relative_path}: {error}"), + ) + })?; + let target = root.join(relative_path); + fs::write(&target, &bytes).map_err(|error| { + APWError::new( + Status::ProcessNotRunning, + format!("Failed to write {relative_path}: {error}"), + ) + })?; + fs::set_permissions(&target, fs::Permissions::from_mode(0o600)).map_err(|error| { + APWError::new( + Status::InvalidConfig, + format!("Failed to chmod {relative_path}: {error}"), + ) + })?; + files_included.push(relative_path.to_string()); + Ok(()) +} + +/// Walk every string in the value and fail closed if any matches a +/// token-like pattern. Counts checked strings so the manifest can record +/// that redaction actually ran. +fn audit_redaction(value: &Value, redaction_checks: &mut u32) -> Result<()> { + walk_strings(value, &mut |s| { + *redaction_checks += 1; + if looks_secret_like(s) { + return Err(APWError::new( + Status::InvalidConfig, + format!( + "Aborting bundle: a diagnostic field looks like a credential and was not safe to ship: {}", + summarize_suspicious(s) + ), + )); + } + Ok(()) + }) +} + +fn walk_strings(value: &Value, f: &mut F) -> Result<()> +where + F: FnMut(&str) -> Result<()>, +{ + match value { + Value::String(s) => f(s), + Value::Array(items) => { + for item in items { + walk_strings(item, f)?; + } + Ok(()) + } + Value::Object(map) => { + for (key, item) in map { + f(key)?; + walk_strings(item, f)?; + } + Ok(()) + } + _ => Ok(()), + } +} + +/// Heuristic for "this string looks like a credential and should never +/// ship in a diagnostic bundle." The check is intentionally aggressive: a +/// false positive aborts the bundle and tells the operator what to +/// remove, which is much cheaper than a false negative leaking a token. +fn looks_secret_like(value: &str) -> bool { + let trimmed = value.trim(); + if trimmed.is_empty() { + return false; + } + + // The default demo credential lives in tree as `apw-demo-password`. + // Treat that as a sentinel so the redaction tests catch any path that + // accidentally embeds it into a diagnostic payload. + if trimmed.contains("apw-demo-password") { + return true; + } + + // High-entropy fixed-length tokens that fit common API key shapes. + // We restrict to runs of pure base64/hex characters so paths and + // version strings ("aarch64-apple-darwin", "2026-05-21T13:16:39Z") + // don't trip the check. + let token_like_chars = trimmed + .chars() + .all(|c| c.is_ascii_alphanumeric() || matches!(c, '+' | '/' | '=' | '_' | '-')); + if token_like_chars && trimmed.len() >= 32 { + let alphanumeric_count = trimmed + .chars() + .filter(|c| c.is_ascii_alphanumeric()) + .count(); + let digit_count = trimmed.chars().filter(|c| c.is_ascii_digit()).count(); + // Require at least one digit and mostly alphanumeric so timestamp + // shapes and identifier-only strings don't qualify. + if alphanumeric_count >= trimmed.len() - 4 && digit_count >= 1 { + return true; + } + } + + // Vendor-specific obvious prefixes. + const TOKEN_PREFIXES: &[&str] = &[ + "AKIA", + "ASIA", + concat!("gh", "p_"), + "gho_", + "ghs_", + "ghu_", + concat!("github", "_pat_"), + "xox", + "AIza", + "sk-", + "sk_live_", + "pk_live_", + ]; + for prefix in TOKEN_PREFIXES { + if trimmed.contains(prefix) { + return true; + } + } + + false +} + +fn summarize_suspicious(value: &str) -> String { + let preview: String = value.chars().take(8).collect(); + format!("`{preview}…` (len={})", value.len()) +} + +fn collect_os_info() -> Value { + let mut info = json!({ + "uname": probe_first_line("uname", &["-a"]), + "arch": std::env::consts::ARCH, + "os": std::env::consts::OS, + }); + if cfg!(target_os = "macos") { + if let Some(map) = info.as_object_mut() { + map.insert( + "sw_vers".to_string(), + json!({ + "productName": probe_first_line("sw_vers", &["-productName"]), + "productVersion": probe_first_line("sw_vers", &["-productVersion"]), + "buildVersion": probe_first_line("sw_vers", &["-buildVersion"]), + }), + ); + } + } + info +} + +fn probe_first_line(command: &str, args: &[&str]) -> Value { + use std::sync::mpsc; + use std::time::Duration; + + let command_owned = command.to_string(); + let args_owned: Vec = args.iter().map(|s| s.to_string()).collect(); + let (tx, rx) = mpsc::channel(); + std::thread::spawn(move || { + let result = Command::new(command_owned).args(args_owned).output(); + let _ = tx.send(result); + }); + let Ok(Ok(output)) = rx.recv_timeout(Duration::from_secs(3)) else { + return Value::Null; + }; + if !output.status.success() { + return Value::Null; + } + let text = String::from_utf8_lossy(&output.stdout); + let line = text + .lines() + .find(|line| !line.trim().is_empty()) + .unwrap_or("") + .trim() + .to_string(); + if line.is_empty() { + Value::Null + } else { + Value::String(line) + } +} + +/// Walk the native-app runtime directory and return a metadata-only +/// listing: relative path, byte size, octal mode, file type. Never reads +/// file contents. +fn collect_native_app_listing() -> Value { + let root = native_app_runtime_dir(); + if !root.exists() { + return json!({ + "root": root, + "entries": [], + "note": "native-app runtime directory does not exist (likely no `apw app install` yet).", + }); + } + let mut entries: Vec = Vec::new(); + walk_metadata(&root, &root, &mut entries); + entries.sort_by(|a, b| { + let ap = a.get("path").and_then(Value::as_str).unwrap_or(""); + let bp = b.get("path").and_then(Value::as_str).unwrap_or(""); + ap.cmp(bp) + }); + json!({ + "root": root, + "entries": entries, + }) +} + +fn walk_metadata(root: &Path, current: &Path, entries: &mut Vec) { + let Ok(read_dir) = fs::read_dir(current) else { + return; + }; + for entry in read_dir.flatten() { + let path = entry.path(); + let relative = path + .strip_prefix(root) + .ok() + .map(|p| p.to_string_lossy().to_string()) + .unwrap_or_default(); + let Ok(metadata) = fs::symlink_metadata(&path) else { + continue; + }; + let file_type = if metadata.file_type().is_dir() { + "dir" + } else if metadata.file_type().is_symlink() { + "symlink" + } else if metadata.file_type().is_file() { + "file" + } else { + "other" + }; + let mode = metadata.permissions().mode() & 0o777; + let size = if metadata.file_type().is_file() { + Some(metadata.len()) + } else { + None + }; + entries.push(json!({ + "path": relative, + "type": file_type, + "mode": format!("{:o}", mode), + "size": size, + })); + if metadata.file_type().is_dir() { + walk_metadata(root, &path, entries); + } + } +} + +/// Self-managed staging directory under the system temp dir. Keeping it +/// outside `~/.apw/native-app/` is important because +/// `collect_native_app_listing()` walks that tree — putting the staging +/// dir there would race the walk and let the bundle's own UUID name leak +/// into the metadata listing, tripping the redaction audit. +struct StagingDir(PathBuf); + +impl StagingDir { + fn create() -> Result { + let base = std::env::temp_dir(); + fs::create_dir_all(&base).map_err(|error| { + APWError::new( + Status::ProcessNotRunning, + format!("Failed to ensure temp dir {}: {error}", base.display()), + ) + })?; + let path = base.join(format!("apw-doctor-staging-{}", uuid_like_suffix())); + fs::create_dir(&path).map_err(|error| { + APWError::new( + Status::ProcessNotRunning, + format!("Failed to create staging dir {}: {error}", path.display()), + ) + })?; + fs::set_permissions(&path, fs::Permissions::from_mode(0o700)).map_err(|error| { + APWError::new( + Status::InvalidConfig, + format!("Failed to chmod staging dir {}: {error}", path.display()), + ) + })?; + Ok(Self(path)) + } + + fn path(&self) -> &Path { + &self.0 + } +} + +impl Drop for StagingDir { + fn drop(&mut self) { + let _ = fs::remove_dir_all(&self.0); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn looks_secret_like_flags_demo_password() { + assert!(looks_secret_like("apw-demo-password")); + assert!(looks_secret_like( + "the credential is apw-demo-password please" + )); + } + + #[test] + fn looks_secret_like_flags_long_token() { + // 40-char alphanumeric with digits — looks like a token. + assert!(looks_secret_like( + "a1b2c3d4e5f6g7h8i9j0k1l2m3n4o5p6q7r8s9t0" + )); + // Prefix vendor token + assert!(looks_secret_like(&format!("{}IOSFODNN7EXAMPLE", "AKIA"))); + } + + #[test] + fn looks_secret_like_does_not_flag_paths_or_versions() { + assert!(!looks_secret_like( + "/Users/example/.apw/native-app/broker.log" + )); + assert!(!looks_secret_like("aarch64-apple-darwin")); + assert!(!looks_secret_like("2026-05-21T13:16:39Z")); + assert!(!looks_secret_like("rustc 1.94.1 (e408947bf 2026-03-25)")); + assert!(!looks_secret_like("")); + assert!(!looks_secret_like("OK")); + assert!(!looks_secret_like( + "Native app credential requests require https URLs." + )); + } + + #[test] + fn audit_redaction_passes_safe_doctor_payload() { + let payload = json!({ + "ok": true, + "payload": { + "bundleVersion": "2.0.0", + "credentialsPath": "/Users/example/.apw/native-app/credentials.json", + "guidance": [ + "Run `apw login https://example.com` to exercise the bootstrap credential flow." + ] + } + }); + let mut count = 0; + audit_redaction(&payload, &mut count).expect("safe payload"); + assert!(count >= 3); + } + + #[test] + fn audit_redaction_fails_closed_on_secret_substring() { + let payload = json!({ + "logSnippet": "leaked: apw-demo-password", + }); + let mut count = 0; + let err = audit_redaction(&payload, &mut count).expect_err("must abort"); + assert_eq!(err.code, Status::InvalidConfig); + assert!(err.message.contains("Aborting bundle")); + } + + #[test] + fn looks_secret_like_flags_embedded_vendor_prefix() { + // Vendor prefix embedded inside a longer diagnostic string must be caught. + let github_prefix = ["gh", "p_"].concat(); + assert!(looks_secret_like(&format!( + "token={github_prefix}abc123def456ghi789jkl0" + ))); + assert!(looks_secret_like("auth failed for sk-abc123")); + assert!(looks_secret_like( + "header: Authorization: Bearer AKIA1234EXAMPLE" + )); + } + + #[test] + fn audit_redaction_fails_closed_on_secret_like_object_key() { + // A secret-shaped string used as a JSON object key must be caught. + let github_prefix = ["gh", "p_"].concat(); + let payload = json!({ + format!("{github_prefix}abc123def456ghi789jkl0"): "some value", + }); + let mut count = 0; + let err = audit_redaction(&payload, &mut count).expect_err("must abort on secret key"); + assert_eq!(err.code, Status::InvalidConfig); + assert!(err.message.contains("Aborting bundle")); + } +} diff --git a/rust/src/cli.rs b/rust/src/cli.rs index 21a0f1c..f680b04 100644 --- a/rust/src/cli.rs +++ b/rust/src/cli.rs @@ -120,8 +120,15 @@ pub struct DoctorCommand { /// Emit only the structured environment-check array. Useful for CI /// jobs that want to grep `[FAIL]` lines or parse the JSON shape. /// See issue #12. - #[arg(long)] + #[arg(long, conflicts_with = "bundle")] pub ci: bool, + /// Write a redacted diagnostic bundle (tar.gz) to the given path so + /// it can be attached to support requests. The bundle excludes + /// credentials.json, config.json, broker logs, and environment + /// variables, and aborts if any included field looks like a token. + /// See issue #56. + #[arg(long, value_name = "PATH")] + pub bundle: Option, } #[derive(Args)] @@ -208,7 +215,27 @@ fn run_doctor(args: DoctorCommand, cli_json: bool) -> Result<(), APWError> { let mut payload = native_app_doctor()?; if let Some(object) = payload.as_object_mut() { - object.insert("environment".to_string(), environment_json); + object.insert("environment".to_string(), environment_json.clone()); + } + + if let Some(bundle_path) = args.bundle.as_deref() { + let result = + crate::bundle::write_diagnostic_bundle(bundle_path, &payload, &environment_json)?; + let summary = serde_json::json!({ + "bundlePath": result.path, + "filesIncluded": result.files_included, + "redactionChecks": result.redaction_checks, + }); + if !cli_json { + eprintln!( + "Wrote diagnostic bundle to {} ({} files, {} redaction checks).", + result.path.display(), + result.files_included.len(), + result.redaction_checks + ); + } + print_output(&summary, Status::Success, cli_json); + return Ok(()); } if !cli_json { @@ -596,6 +623,18 @@ mod tests { } } + #[test] + fn doctor_ci_and_bundle_are_mutually_exclusive() { + let parsed = Cli::try_parse_from([ + "apw", + "doctor", + "--ci", + "--bundle", + "/tmp/apw-doctor-bundle.tar.gz", + ]); + assert!(parsed.is_err()); + } + #[test] fn login_command_parses() { let parsed = Cli::try_parse_from(["apw", "login", "https://example.com"]).unwrap(); diff --git a/rust/src/main.rs b/rust/src/main.rs index 9248a8e..1faa37c 100644 --- a/rust/src/main.rs +++ b/rust/src/main.rs @@ -1,5 +1,6 @@ use clap::Parser; +mod bundle; mod cli; // Retained legacy modules still power status/parity diagnostics until #47 // archives the browser-helper and native-host runtime internals. diff --git a/rust/src/native_app.rs b/rust/src/native_app.rs index 9d35b89..1dc6478 100644 --- a/rust/src/native_app.rs +++ b/rust/src/native_app.rs @@ -808,7 +808,7 @@ fn send_request_via_executable(command: &str, payload: Value) -> Result { parse_response(value) } -fn uuid_like_suffix() -> String { +pub fn uuid_like_suffix() -> String { use rand::Rng; let mut rng = rand::thread_rng(); format!("{:016x}{:016x}", rng.gen::(), rng.gen::()) diff --git a/rust/tests/native_app_e2e.rs b/rust/tests/native_app_e2e.rs index 91d7305..9eee22d 100644 --- a/rust/tests/native_app_e2e.rs +++ b/rust/tests/native_app_e2e.rs @@ -726,6 +726,167 @@ fn direct_fallback_maps_malformed_response_to_proto_invalid_response() { .contains("missing its payload")); } +#[test] +#[serial] +fn doctor_bundle_writes_deterministic_redacted_archive() { + // Issue #56: --bundle writes a tar.gz with a stable layout and never + // includes credentials.json contents. We populate + // ~/.apw/native-app/credentials.json with plausible secret material + // and assert that the secret never appears anywhere in the archive. + let fixture = NativeAppFixture::new(); + let runtime = fixture.home().join(".apw/native-app"); + fs::create_dir_all(&runtime).expect("failed to create runtime"); + fs::write( + runtime.join("credentials.json"), + r#"{"secret":"apw-redaction-sentinel-must-never-leak","domain":"example.com"}"#, + ) + .expect("failed to write credentials"); + let mut perms = fs::metadata(runtime.join("credentials.json")) + .unwrap() + .permissions(); + perms.set_mode(0o600); + fs::set_permissions(runtime.join("credentials.json"), perms).unwrap(); + + let bundle_dir = TempDir::new().expect("failed to create bundle output dir"); + let bundle_path = bundle_dir.path().join("apw-doctor-bundle.tar.gz"); + + let output = run_apw( + &fixture, + &[ + "--json", + "doctor", + "--bundle", + bundle_path.to_str().unwrap(), + ], + &[], + ); + assert_eq!(output.status, 0, "{output:#?}"); + + let payload = parse_success(&output); + assert_eq!( + payload["payload"]["bundlePath"], + serde_json::Value::String(bundle_path.display().to_string()) + ); + let files: Vec = payload["payload"]["filesIncluded"] + .as_array() + .expect("filesIncluded array") + .iter() + .map(|v| v.as_str().unwrap_or_default().to_string()) + .collect(); + let expected = [ + "manifest.json", + "doctor.json", + "environment.json", + "os.json", + "native-app/file-listing.json", + ]; + for name in expected { + assert!( + files.iter().any(|f| f == name), + "expected {name} in {files:?}" + ); + } + assert!( + payload["payload"]["redactionChecks"] + .as_u64() + .unwrap_or_default() + >= 10 + ); + + assert!(bundle_path.exists(), "bundle file should exist"); + let mode = fs::metadata(&bundle_path).unwrap().permissions().mode() & 0o777; + assert_eq!(mode, 0o600, "bundle file should be 0600"); + + let extract_dir = TempDir::new().expect("failed to create extract dir"); + let extract_status = Command::new("tar") + .arg("-xzf") + .arg(&bundle_path) + .arg("-C") + .arg(extract_dir.path()) + .status() + .expect("failed to run tar"); + assert!(extract_status.success(), "tar extraction failed"); + + let bundle_root = extract_dir.path().join("apw-doctor-bundle"); + assert!(bundle_root.is_dir(), "expected bundle root dir"); + for name in expected { + let p = bundle_root.join(name); + assert!(p.exists(), "expected bundle to contain {name}"); + } + + let mut accumulator: Vec = Vec::new(); + fn collect_bytes(path: &Path, acc: &mut Vec) { + if path.is_file() { + if let Ok(b) = fs::read(path) { + acc.extend_from_slice(&b); + } + } else if path.is_dir() { + for entry in fs::read_dir(path).into_iter().flatten().flatten() { + collect_bytes(&entry.path(), acc); + } + } + } + collect_bytes(&bundle_root, &mut accumulator); + let combined = String::from_utf8_lossy(&accumulator); + assert!( + !combined.contains("apw-redaction-sentinel-must-never-leak"), + "credentials.json contents must not appear in the bundle" + ); + + let listing: Value = serde_json::from_slice( + &fs::read(bundle_root.join("native-app/file-listing.json")).unwrap(), + ) + .unwrap(); + let entries = listing["entries"].as_array().expect("entries array"); + let has_credentials = entries + .iter() + .any(|e| e["path"].as_str() == Some("credentials.json")); + assert!( + has_credentials, + "file listing must reference credentials.json by name" + ); + let credentials_entry = entries + .iter() + .find(|e| e["path"].as_str() == Some("credentials.json")) + .unwrap(); + assert_eq!(credentials_entry["type"], "file"); + assert!(credentials_entry["size"].as_u64().unwrap_or(0) > 0); +} + +#[test] +#[serial] +fn doctor_ci_and_bundle_fail_before_writing_archive() { + // Issue #56 review follow-up: the operator-facing CLI must not accept + // `--ci --bundle` and then return before writing the requested archive. + let fixture = NativeAppFixture::new(); + let bundle_dir = TempDir::new().expect("failed to create bundle output dir"); + let bundle_path = bundle_dir.path().join("should-not-exist.tar.gz"); + + let output = run_apw( + &fixture, + &[ + "--json", + "doctor", + "--ci", + "--bundle", + bundle_path.to_str().unwrap(), + ], + &[], + ); + + assert_ne!(output.status, 0, "expected clap conflict, got {output:#?}"); + assert!( + output.stderr.contains("cannot be used with") + || output.stderr.contains("cannot be used at the same time"), + "expected conflict error, got: {}", + output.stderr + ); + assert!( + !bundle_path.exists(), + "bundle file must not exist when --ci conflicts with --bundle" + ); +} + #[test] #[serial] fn direct_fallback_bounds_oversized_stdout() { @@ -752,6 +913,44 @@ fn direct_fallback_bounds_oversized_stdout() { ); } +#[test] +#[serial] +fn doctor_bundle_fails_closed_when_diagnostic_field_looks_secret_like() { + // Issue #56: if any diagnostic string looks token-like, the bundle + // must abort instead of shipping incompletely-redacted material. We + // inject the sentinel via APW_RUNNER_LABELS, which flows verbatim + // into the CI runner-labels diagnostic. + let fixture = NativeAppFixture::new(); + let bundle_dir = TempDir::new().expect("failed to create bundle output dir"); + let bundle_path = bundle_dir.path().join("should-not-exist.tar.gz"); + + let output = run_apw( + &fixture, + &[ + "--json", + "doctor", + "--bundle", + bundle_path.to_str().unwrap(), + ], + &[ + ("CI", "true"), + ("APW_RUNNER_LABELS", "apw-demo-password-leaked-into-labels"), + ], + ); + assert_ne!(output.status, 0, "expected failure, got {output:#?}"); + + let err = parse_error(&output); + let message = err["error"].as_str().unwrap_or_default(); + assert!( + message.contains("Aborting bundle"), + "expected fail-closed message, got: {message}" + ); + assert!( + !bundle_path.exists(), + "bundle file must not exist after fail-closed abort" + ); +} + #[test] #[serial] fn direct_fallback_terminates_hung_child() {