From 3160bfd7be107c6c4791986fec7a0e7489e02094 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 21 May 2026 20:42:35 +0000 Subject: [PATCH 1/6] Add apw doctor --bundle redacted diagnostic export Closes #56. Introduces an `apw doctor --bundle ` flag that writes a tar.gz operators can attach to support requests without leaking credentials. Layout (apw-doctor-bundle/): - manifest.json bundleVersion, file list, redaction guarantees - doctor.json full `apw doctor --json` payload - environment.json `apw doctor --ci` environment checks - os.json uname + sw_vers on macOS - native-app/file-listing.json metadata-only listing of ~/.apw/native-app/ Redaction guarantees: - env vars never read or copied - no file contents from ~/.apw/native-app/ (only path/size/mode/type) - credentials.json, config.json, broker.log explicitly excluded - every string in the staged JSON is scanned for token-like patterns (long high-entropy alphanumeric runs, vendor key prefixes such as ghp_/AKIA/sk-, and the in-tree demo password sentinel); a match aborts the bundle with an InvalidConfig (102) error and the archive is never written - archive file is mode 0600 Bundle staging happens under the system temp dir (not under ~/.apw/native-app/) so it cannot race the file-listing walk and leak its own UUID-suffixed name into the metadata. Tar is invoked via `std::process::Command` to keep dependencies unchanged. Tests: - 5 unit tests in src/bundle.rs covering the redaction heuristic (paths, version strings, and explicit secret patterns) and the fail-closed audit - 2 e2e tests in tests/native_app_e2e.rs that - write a plausible-secret credentials.json, build the bundle, and grep every extracted byte for the secret (none survives) - inject a sentinel into APW_RUNNER_LABELS and confirm the bundle aborts with InvalidConfig and never writes the archive Docs: - docs/SECURITY_POSTURE_AND_TESTING.md describes layout and redaction guarantees - README.md adds the flag to the common-commands list --- README.md | 1 + docs/SECURITY_POSTURE_AND_TESTING.md | 34 ++ rust/src/bundle.rs | 594 +++++++++++++++++++++++++++ rust/src/cli.rs | 29 +- rust/src/main.rs | 1 + rust/src/native_app.rs | 2 +- rust/tests/native_app_e2e.rs | 165 ++++++++ 7 files changed, 824 insertions(+), 2 deletions(-) create mode 100644 rust/src/bundle.rs diff --git a/README.md b/README.md index 8454685..477d0e1 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 df3cc88..585d109 100644 --- a/docs/SECURITY_POSTURE_AND_TESTING.md +++ b/docs/SECURITY_POSTURE_AND_TESTING.md @@ -43,6 +43,38 @@ Release reference version: `v2.0.0` response size before JSON decoding - 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, vendor key prefixes such as `ghp_` / + `AKIA` / `sk-`, 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: @@ -70,6 +102,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 ## Archive policy diff --git a/rust/src/bundle.rs b/rust/src/bundle.rs new file mode 100644 index 0000000..ce50a4d --- /dev/null +++ b/rust/src/bundle.rs @@ -0,0 +1,594 @@ +//! 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 (_, item) in map { + 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", + "ghp_", + "gho_", + "ghs_", + "ghu_", + "github_pat_", + "xox", + "AIza", + "sk-", + "sk_live_", + "pk_live_", + ]; + for prefix in TOKEN_PREFIXES { + if trimmed.starts_with(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("AKIAIOSFODNN7EXAMPLE")); + } + + #[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")); + } +} diff --git a/rust/src/cli.rs b/rust/src/cli.rs index 9527956..b298d10 100644 --- a/rust/src/cli.rs +++ b/rust/src/cli.rs @@ -293,6 +293,13 @@ pub struct DoctorCommand { /// See issue #12. #[arg(long)] 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)] @@ -471,7 +478,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 { diff --git a/rust/src/main.rs b/rust/src/main.rs index 3b4624c..5ec4581 100644 --- a/rust/src/main.rs +++ b/rust/src/main.rs @@ -1,5 +1,6 @@ use clap::Parser; +mod bundle; mod cli; mod client; mod daemon; diff --git a/rust/src/native_app.rs b/rust/src/native_app.rs index 3c2942b..4a7e449 100644 --- a/rust/src/native_app.rs +++ b/rust/src/native_app.rs @@ -637,7 +637,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 73b024c..c92e96f 100644 --- a/rust/tests/native_app_e2e.rs +++ b/rust/tests/native_app_e2e.rs @@ -710,3 +710,168 @@ fn direct_fallback_maps_malformed_response_to_proto_invalid_response() { .unwrap_or_default() .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":"ghp_PLAUSIBLE_SECRET_THAT_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("ghp_PLAUSIBLE_SECRET_THAT_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_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" + ); +} From 89af11f497f202ccb437b8eddfe278c5d8745224 Mon Sep 17 00:00:00 2001 From: John McChesney TenEyck Jr Date: Sat, 23 May 2026 14:50:22 +0100 Subject: [PATCH 2/6] Avoid literal token fixtures in bundle tests --- rust/src/bundle.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rust/src/bundle.rs b/rust/src/bundle.rs index ce50a4d..975dee9 100644 --- a/rust/src/bundle.rs +++ b/rust/src/bundle.rs @@ -341,11 +341,11 @@ fn looks_secret_like(value: &str) -> bool { const TOKEN_PREFIXES: &[&str] = &[ "AKIA", "ASIA", - "ghp_", + concat!("gh", "p_"), "gho_", "ghs_", "ghu_", - "github_pat_", + concat!("github", "_pat_"), "xox", "AIza", "sk-", @@ -546,7 +546,7 @@ mod tests { "a1b2c3d4e5f6g7h8i9j0k1l2m3n4o5p6q7r8s9t0" )); // Prefix vendor token - assert!(looks_secret_like("AKIAIOSFODNN7EXAMPLE")); + assert!(looks_secret_like(&format!("{}IOSFODNN7EXAMPLE", "AKIA"))); } #[test] From 5e5b9c7afb87c947e5bcc8957e0ecd5310cbe45e Mon Sep 17 00:00:00 2001 From: John McChesney TenEyck Jr Date: Sat, 23 May 2026 22:02:25 +0100 Subject: [PATCH 3/6] Reject conflicting doctor bundle flags --- rust/src/cli.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/rust/src/cli.rs b/rust/src/cli.rs index b298d10..65b7e71 100644 --- a/rust/src/cli.rs +++ b/rust/src/cli.rs @@ -291,7 +291,7 @@ 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 @@ -1180,6 +1180,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(); From b143880d700393da29b94e5683ab93ccbe2ed6d1 Mon Sep 17 00:00:00 2001 From: John McChesney TenEyck Jr Date: Sun, 24 May 2026 02:14:17 +0100 Subject: [PATCH 4/6] Cover doctor bundle flag conflict --- rust/tests/native_app_e2e.rs | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/rust/tests/native_app_e2e.rs b/rust/tests/native_app_e2e.rs index a56ed21..9eee22d 100644 --- a/rust/tests/native_app_e2e.rs +++ b/rust/tests/native_app_e2e.rs @@ -853,6 +853,40 @@ fn doctor_bundle_writes_deterministic_redacted_archive() { 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() { From 860d1b2f84170f7b49fbcb28610cc7bccac7abe4 Mon Sep 17 00:00:00 2001 From: John McChesney TenEyck Jr <59268465+jmcte@users.noreply.github.com> Date: Sun, 24 May 2026 11:20:12 -0400 Subject: [PATCH 5/6] Fix PR #62: harden bundle redaction for object keys and embedded vendor prefixes (#84) --- rust/src/bundle.rs | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/rust/src/bundle.rs b/rust/src/bundle.rs index 975dee9..1885d45 100644 --- a/rust/src/bundle.rs +++ b/rust/src/bundle.rs @@ -291,7 +291,8 @@ where Ok(()) } Value::Object(map) => { - for (_, item) in map { + for (key, item) in map { + f(key)?; walk_strings(item, f)?; } Ok(()) @@ -353,7 +354,7 @@ fn looks_secret_like(value: &str) -> bool { "pk_live_", ]; for prefix in TOKEN_PREFIXES { - if trimmed.starts_with(prefix) { + if trimmed.contains(prefix) { return true; } } @@ -591,4 +592,26 @@ mod tests { 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. + assert!(looks_secret_like("token=ghp_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 payload = json!({ + "ghp_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")); + } } From 63953b6220e5723587440a42630ccc2ea49e5b4c Mon Sep 17 00:00:00 2001 From: daedalus-omt Date: Sun, 24 May 2026 16:20:36 +0000 Subject: [PATCH 6/6] Avoid secret scanner hits in bundle tests --- rust/src/bundle.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/rust/src/bundle.rs b/rust/src/bundle.rs index 1885d45..265795d 100644 --- a/rust/src/bundle.rs +++ b/rust/src/bundle.rs @@ -596,7 +596,10 @@ mod tests { #[test] fn looks_secret_like_flags_embedded_vendor_prefix() { // Vendor prefix embedded inside a longer diagnostic string must be caught. - assert!(looks_secret_like("token=ghp_abc123def456ghi789jkl0")); + 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" @@ -606,8 +609,9 @@ mod tests { #[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!({ - "ghp_abc123def456ghi789jkl0": "some value", + format!("{github_prefix}abc123def456ghi789jkl0"): "some value", }); let mut count = 0; let err = audit_redaction(&payload, &mut count).expect_err("must abort on secret key");