Refactor controller into focused modules

Split controller logic into dedicated modules with doc comments, relocate runner selector tests, and update controller README to match the new layout.

Author: Protonk

controller/README.md

@@ -10,8 +10,26 @@ For the Swift runner implementation details, see `runner/README.md`.
 
 ## What lives in `controller/`
 
-- `controller/src/main.rs` — CLI parsing and specimen evaluation orchestration
-- `controller/src/json_contract.rs` — JSON key sorting helper (used by other bins)
+Core controller modules:
+
+- `controller/src/main.rs` — entry point and module wiring
+- `controller/src/cli.rs` — CLI usage text + top-level dispatch
+- `controller/src/run_flow.rs` — run orchestration and JSON envelope assembly
+- `controller/src/runner_select.rs` — runner selection + provenance
+- `controller/src/runner_client.rs` — wrapper around `pw-runner-client`
+- `controller/src/sandbox_log.rs` — unified-log capture mapping for sandbox denials
+- `controller/src/sonoma_cross_check.rs` — `sb_api_validator` cross-check flow
+- `controller/src/runner_commands.rs` — external runner install/list/verify/remove/refresh
+
+Support modules:
+
+- `controller/src/app_layout.rs` — app bundle layout + embedded tool resolution
+- `controller/src/plist.rs` — PlistBuddy helpers for Info.plist lookups
+- `controller/src/bundle.rs` — bundle metadata reader for external runners
+- `controller/src/request_patch.rs` — request JSON injection helpers
+- `controller/src/utils.rs` — shared time + output helpers
+- `controller/src/evidence.rs` — evidence manifest parsing + verification
+- `controller/src/json_contract.rs` — JSON envelope rendering with sorted keys
 - `controller/src/runner_manager.rs` — external runner registry + launchd wiring
 
 Standalone helper tools (embedded into the `.app`):

controller/src/app_layout.rs

@@ -0,0 +1,75 @@
+//! Bundle layout helpers for PolicyWitness.app.
+//!
+//! The controller resolves embedded tools relative to its own executable so the
+//! app bundle can be relocated without rewriting paths.
+
+use std::path::{Component, Path, PathBuf};
+
+use crate::plist::plist_key_string;
+
+pub const PW_RUNNER_SERVICE_DIR: &str = "PWRunner";
+
+#[derive(Clone)]
+pub struct PWRunnerBundleInfo {
+    pub bundle_id: String,
+    pub executable: String,
+}
+
+pub fn validate_tool_name(tool_name: &str) -> Result<(), String> {
+    let mut components = Path::new(tool_name).components();
+    match (components.next(), components.next()) {
+        (Some(Component::Normal(_)), None) => Ok(()),
+        _ => Err(format!(
+            "invalid tool name {tool_name:?} (must be a single path component)"
+        )),
+    }
+}
+
+pub fn app_root_from_current_exe() -> Result<PathBuf, String> {
+    let exe = std::env::current_exe().map_err(|e| format!("current_exe() failed: {e}"))?;
+    // Expected layout: PolicyWitness.app/Contents/MacOS/policy-witness
+    let contents_dir = exe
+        .parent()
+        .and_then(|p| p.parent())
+        .ok_or_else(|| format!("unexpected executable location: {}", exe.display()))?;
+    let app_root = contents_dir
+        .parent()
+        .ok_or_else(|| format!("unexpected executable location: {}", exe.display()))?;
+    Ok(app_root.to_path_buf())
+}
+
+pub fn resolve_contents_macos_tool(tool_name: &str) -> Result<PathBuf, String> {
+    validate_tool_name(tool_name)?;
+    let exe = std::env::current_exe().map_err(|e| format!("current_exe() failed: {e}"))?;
+    let contents_dir = exe
+        .parent()
+        .and_then(|p| p.parent())
+        .ok_or_else(|| format!("unexpected executable location: {}", exe.display()))?;
+    // Tools live alongside the controller under Contents/MacOS.
+    let candidate = contents_dir.join("MacOS").join(tool_name);
+    if candidate.exists() {
+        return Ok(candidate);
+    }
+    Err(format!(
+        "embedded tool not found in Contents/MacOS: {tool_name:?} (expected: {})",
+        candidate.display()
+    ))
+}
+
+pub fn resolve_pw_runner_bundle_info(app_root: &Path) -> Result<PWRunnerBundleInfo, String> {
+    let plist = app_root
+        .join("Contents")
+        .join("XPCServices")
+        .join(format!("{PW_RUNNER_SERVICE_DIR}.xpc"))
+        .join("Contents")
+        .join("Info.plist");
+    if !plist.exists() {
+        return Err(format!("missing PWRunner Info.plist: {}", plist.display()));
+    }
+    let bundle_id = plist_key_string(&plist, "CFBundleIdentifier")?;
+    let executable = plist_key_string(&plist, "CFBundleExecutable")?;
+    Ok(PWRunnerBundleInfo {
+        bundle_id,
+        executable,
+    })
+}

controller/src/bin/sandbox-log-observer.rs

@@ -1,3 +1,9 @@
+//! Unified-log observer for sandbox deny events.
+//!
+//! This tool runs outside the app sandbox to query the system log for deny
+//! messages associated with a specific process. It emits JSON so the controller
+//! can attach evidence without parsing raw log output.
+
 #[path = "../json_contract.rs"]
 #[allow(dead_code)]
 mod json_contract;
@@ -75,6 +81,7 @@ fn json_result(ok: bool) -> json_contract::JsonResult {
 
 fn is_pid_alive(pid: i32) -> bool {
     unsafe {
+        // kill(pid, 0) checks existence without delivering a signal.
         if kill(pid, 0) == 0 {
             return true;
         }
@@ -89,6 +96,7 @@ fn is_pid_alive(pid: i32) -> bool {
 fn sandbox_predicate(process_name: &str, pid: i32) -> String {
     let term = format!("Sandbox: {}({})", process_name, pid);
     let escaped = term.replace('"', "\\\"");
+    // Match both explicit deny lines and any sandbox line for the PID.
     format!(
         r#"((eventMessage CONTAINS[c] "{}") OR ((eventMessage CONTAINS[c] "deny") AND (eventMessage CONTAINS[c] "{}")))"#,
         escaped, pid
@@ -518,6 +526,7 @@ fn main() {
     let mut log_error: Option<String> = None;
     let mut blocked_reason: Option<String> = None;
 
+    // log stream gives live updates; log show is a point-in-time snapshot.
     let mode = if stream_mode { "stream" } else { "show" }.to_string();
     let duration_ms = duration.map(|d| d.as_millis() as u64);
 

controller/src/bundle.rs

@@ -0,0 +1,29 @@
+//! Bundle metadata helpers for external runners.
+//!
+//! External XPC services are packaged as bundles with Info.plist metadata; we
+//! read only the keys PolicyWitness needs to wire launchd and verify identity.
+
+use std::path::Path;
+
+use crate::plist::plist_key_string;
+
+pub struct BundleInfo {
+    pub bundle_id: String,
+    pub executable: String,
+    pub package_type: Option<String>,
+}
+
+pub fn read_bundle_info(bundle_path: &Path) -> Result<BundleInfo, String> {
+    let plist = bundle_path.join("Contents").join("Info.plist");
+    if !plist.exists() {
+        return Err(format!("missing Info.plist at {}", plist.display()));
+    }
+    let bundle_id = plist_key_string(&plist, "CFBundleIdentifier")?;
+    let executable = plist_key_string(&plist, "CFBundleExecutable")?;
+    let package_type = plist_key_string(&plist, "CFBundlePackageType").ok();
+    Ok(BundleInfo {
+        bundle_id,
+        executable,
+        package_type,
+    })
+}

controller/src/cli.rs

@@ -0,0 +1,72 @@
+//! Command-line entrypoint for the PolicyWitness controller.
+//!
+//! This module owns the public CLI surface and keeps the dispatch logic
+//! together so the rest of the controller can focus on orchestration.
+
+use serde_json::json;
+use std::ffi::OsString;
+
+use crate::json_contract;
+use crate::run_flow;
+use crate::runner_commands;
+
+pub fn print_usage() {
+    eprintln!(
+        "\
+usage:
+  policy-witness run <request.json> [--timeout-ms <n>] [--log-last <dur>] [--runner-mode <debuggable|byoxpc|machme>] [--instrumentation <json|@path>] [--sonoma-cross-check]
+  policy-witness runner <command> [options]
+
+notes:
+  - runs the selected PWRunner XPC service once and prints a single JSON result to stdout
+  - request.json is passed through to the runner client (or copied with runner mode/instrumentation injected)"
+    );
+}
+
+pub fn run(argv: Vec<OsString>) -> i32 {
+    if argv.is_empty() {
+        print_usage();
+        return 2;
+    }
+
+    let sub = argv[0].to_string_lossy().to_string();
+    let rest = &argv[1..];
+
+    if sub == "-h" || sub == "--help" || sub == "help" {
+        print_usage();
+        return 0;
+    }
+
+    match sub.as_str() {
+        "run" => match run_flow::cmd_run(rest) {
+            Ok(code) => code,
+            Err(err) => {
+                let result = json_contract::JsonResult {
+                    ok: false,
+                    rc: None,
+                    exit_code: Some(2),
+                    normalized_outcome: Some("tool_error".to_string()),
+                    errno: None,
+                    error: Some(err),
+                    stderr: None,
+                    stdout: None,
+                };
+                let data =
+                    json!({"error": "policy-witness run failed before producing a runner result"});
+                let _ = json_contract::print_envelope("run", result, &data);
+                2
+            }
+        },
+        "runner" => match runner_commands::cmd_runner(rest) {
+            Ok(code) => code,
+            Err(err) => {
+                eprintln!("{err}");
+                2
+            }
+        },
+        _ => {
+            print_usage();
+            2
+        }
+    }
+}

controller/src/evidence.rs

@@ -1,3 +1,8 @@
+//! Evidence manifest helpers for shipped binaries.
+//!
+//! The embedded manifest records hashes and entitlements so runs can report
+//! provenance without relying on external tooling.
+
 use serde::{Deserialize, Serialize};
 use serde_json::Value;
 use sha2::{Digest, Sha256};
@@ -68,6 +73,7 @@ pub fn sha256_hex(path: &Path) -> Result<String, String> {
     let mut file = File::open(path)
         .map_err(|e| format!("failed to open {}: {e}", path.display()))?;
     let mut hasher = Sha256::new();
+    // Stream the file to avoid loading large binaries in memory.
     let mut buf = [0u8; 8192];
     loop {
         let n = file
@@ -90,6 +96,7 @@ pub fn verify_manifest(manifest: &EvidenceManifest, app_root: &Path, manifest_pa
     let mut mismatches = Vec::new();
     let mut checked = 0usize;
 
+    // Walk each entry with a declared hash and compare it to disk.
     for entry in &manifest.entries {
         let expected = entry.sha256.clone();
         if expected.is_none() {

controller/src/json_contract.rs

@@ -1,3 +1,8 @@
+//! JSON envelope helpers for controller output.
+//!
+//! The controller emits a single JSON object per run. Keys are sorted so
+//! envelopes are stable for diffing and hashing.
+
 use serde::Serialize;
 use serde_json::{Map, Value};
 use std::time::{SystemTime, UNIX_EPOCH};
@@ -31,6 +36,7 @@ fn sort_value(value: &mut Value) {
             }
         }
         Value::Object(map) => {
+            // Sort object keys to keep output deterministic across runs.
             let mut entries: Vec<(String, Value)> =
                 map.iter().map(|(k, v)| (k.clone(), v.clone())).collect();
             entries.sort_by(|a, b| a.0.cmp(&b.0));