From e565ca3eea28cd654a10d1c64686b48403143e7f Mon Sep 17 00:00:00 2001 From: "Richie Gomez (he/him)" Date: Wed, 22 Jul 2026 13:24:27 -0700 Subject: [PATCH 1/5] captureDenials: JSON denials output file with PID-stamped path and capability resolution (PR5) Finalize the captureDenials deliverable as a single JSON document ({ "denials": [...], "summary": {...} }) that the launching app reads, replacing the NDJSON frame stream. wxc-exec seals the internal ETL, decodes it, writes the JSON file, and prints only the file-path pointer on stderr. Improvements: - PID-stamped output filename: the wxc-exec pid is inserted into the caller's outputPath stem (denials.json -> denials..json) so concurrent app instances don't collide; the real path is echoed in the stderr pointer (insert_pid_into_stem). - Rename DeniedResource.path -> resource, inclusive of file paths AND AppContainer capabilities. - Resolve capability identity: event-28 PackageSid/UserSid now decode via a new capability_names resolver (well-known S-1-15-3-1..12 SID -> policy name, else the SID string). Fix TDH_INTYPE_SID to 19 (was 22) so SID fields decode instead of rendering . Remove learning_mode_core::frame (NDJSON) in favor of the JSON document emitter. Regenerate dev schema + SDK wire types (description-only). Update schema.md, capabilities.md, and copilot-instructions.md. All changes VM-validated on a learning-mode-enabled Windows build. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/copilot-instructions.md | 3 +- docs/learning-mode/capabilities.md | 62 ++++- docs/schema.md | 7 +- schemas/dev/mxc-config.schema.0.8.0-dev.json | 2 +- sdk/node/src/generated/wire.ts | 2 +- src/Cargo.lock | 1 + src/backends/appcontainer/common/Cargo.toml | 1 + .../common/src/base_container_runner.rs | 219 +++++++++++++--- .../windows/examples/lm_analyze.rs | 19 +- .../windows/src/capability_names.rs | 113 +++++++++ .../learning_mode/windows/src/etl_decode.rs | 45 ++-- .../learning_mode/windows/src/extractors.rs | 71 +++++- src/backends/learning_mode/windows/src/lib.rs | 2 + .../learning_mode/windows/src/tdh_decode.rs | 41 +++ src/core/learning_mode_core/src/analyze.rs | 6 +- src/core/learning_mode_core/src/emit.rs | 238 +++++++++++------- src/core/learning_mode_core/src/frame.rs | 98 -------- src/core/learning_mode_core/src/lib.rs | 13 +- src/core/learning_mode_core/src/model.rs | 43 ++-- src/core/learning_mode_core/src/summary.rs | 16 +- src/core/wxc_common/src/config_parser.rs | 12 +- src/core/wxc_common/src/models.rs | 10 +- src/core/wxc_common/src/wire.rs | 15 +- .../wxc_e2e_tests/tests/e2e_windows.rs | 132 ++++++++++ tests/examples/29_capture_denials.json | 2 +- 25 files changed, 856 insertions(+), 317 deletions(-) create mode 100644 src/backends/learning_mode/windows/src/capability_names.rs delete mode 100644 src/core/learning_mode_core/src/frame.rs diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index e2b9683e..5db13ad1 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -191,7 +191,7 @@ The workspace is organized into six top-level directories under `src/`: | Directory | Purpose | Examples | |-----------|---------|----------| -| `core/` | Cross-platform foundation + per-platform aggregator binaries | `wxc_common/`, `wxc/`, `lxc/`, `mxc_darwin/`, `mxc_engine/`, `mxc-sdk/`, `mxc_pty/`, `mxc_build_common/`, `generated/` | +| `core/` | Cross-platform foundation + per-platform aggregator binaries | `wxc_common/`, `wxc/`, `lxc/`, `mxc_darwin/`, `mxc_engine/`, `mxc-sdk/`, `mxc_pty/`, `mxc_build_common/`, `learning_mode_core/`, `generated/` | | `backends/` | Backend-specific code (one subfolder per containment backend or backend support component) | `appcontainer/common`, `windows_sandbox/{daemon,guest,common,lifecycle}`, `isolation_session/{bindings,common}`, `learning_mode/windows`, `hyperlight/common`, `nanvix/{common,build_common,binaries,runner}`, `lxc/common`, `bubblewrap/common`, `wslc/common`, `seatbelt/common` | | `ffi/` | Foreign-function-interface crates (C ABI for language bindings) | `mxc_ffi/` | | `host/` | Host-side utilities | `wxc_host_prep/`, `wxc_winhttp_proxy_shim/` | @@ -207,6 +207,7 @@ The workspace is organized into six top-level directories under `src/`: - The lower-level execution surface lives in `wxc_common::sandbox_process`: the `SandboxBackend` trait (`validate` + `spawn(request, logger, StdioMode) -> Box` + a `diagnose_exit` hook) and the generic `Runner` adapter that bridges any `SandboxBackend` to the run-to-completion `ScriptRunner` (via `spawn(StdioMode::Inherit)` then `wait()`). `StdioMode::Pipes` hands the caller live stdin/stdout/stderr (what the `mxc-sdk` streaming path uses); `StdioMode::Inherit` lets the child inherit the host's stdio (what the executor binaries use, preserving the TTY under a pty). `SandboxBackend` is implemented for Seatbelt, Bubblewrap, and Windows ProcessContainer. - `mxc_ffi` (`ffi/mxc_ffi`, `crate-type = ["cdylib", "staticlib", "lib"]`) is a flat, panic-safe **C ABI over `mxc-sdk`** for language bindings. `mxc_run(policyJson, command, out)` runs a sandbox to completion, filling a `#[repr(C)] MxcRunResult` (status + exit_code + timed_out + owned stdout/stderr/error C strings); every entry point is `catch_unwind`-wrapped so a panic becomes a status code, never an unwind. Its `build.rs` runs **csbindgen** to generate the C# P/Invoke (`sdk/dotnet/Microsoft.Mxc.Sdk/Native/NativeMethods.g.cs`), gated behind the crate's **`dotnetsdk`** feature (off by default, so the whole-workspace backend build matrix doesn't compile csbindgen). The generated file is **not committed** (gitignored); the C# csproj regenerates it at build time and `scripts/check-dotnet-bindings-codegen.js` runs the codegen in CI and asserts the expected entry points are produced. The C ABI is **not a stable external contract** (native + binding are co-versioned and generated together; see the crate docs). It exposes three surfaces: **run-to-completion** (`mxc_run`), **streaming** (`mxc_spawn` → opaque `MxcSandbox` handle; `mxc_stream_read`/`write`/`flush`, `mxc_sandbox_take_stdin`/`stdout`/`stderr`, `mxc_sandbox_id`/`try_wait`/`wait`/`kill`/`free`, in `src/streaming.rs`), and the **state-aware lifecycle** (`mxc_state_aware` for the envelope phases + `mxc_state_aware_exec` returning a live streaming handle, in `src/state_aware.rs`). All three `.rs` files are csbindgen inputs in `build.rs`; the `MXC_STATUS_*` space already reserves the state-aware phase codes. - `mxc_pty` is the shared pty bridge used by the LXC backend (`lxc_common::lxc_bindings::attach_run`) so the inner shell sees a real TTY and host stdio is streamed live. (Seatbelt and Bubblewrap no longer use it: they spawn directly and let the child inherit the host's stdio — a TTY when the executor binary runs under a pty — via `SandboxBackend::spawn(StdioMode::Inherit)`.) +- `learning_mode_core` is the **cross-platform learning-mode / captureDenials model + output emitter**: `DeniedResource` (+ `ResourceType`/`AccessType`), `DenialSummary`, the `DenialAnalyzer` decode trait, and `emit` — which writes the on-disk denials deliverable as a **single JSON document** `{ "denials": [...], "summary": {...} }` (`write_document` / `DenialsDocument`) plus the one-line stderr `DenialsOutputPointer`. It carries no OS-specific code (must not depend on any `backends/*` crate); the Windows ETL decoder implementing `DenialAnalyzer` lives in `backends/learning_mode/windows`. When `processContainer.captureDenials` is set, the BaseContainer runner seals a unique internal ETL temp, decodes it via that backend, writes the JSON file (caller's `outputPath` with a unique per-run identifier stamped into the stem, e.g. `denials..json`, or a managed temp), deletes the ETL, and prints the pointer line (carrying the actual path) on stderr. Each denial's `resource` field holds the file path or the AppContainer capability name; capability denials resolve their capability SID to a friendly name via `backends/learning_mode/windows`'s `capability_names` (well-known `S-1-15-3-…` SID → policy name; custom hashed SIDs fall back to the SID string). - `mxc_build_common` is a build-time helper crate — all Windows binary crates use it in their `build.rs` to embed VersionInfo (ProductName, FileDescription, copyright, version+commit). When adding a new Windows binary crate, add `mxc_build_common` as a build-dependency and call `mxc_build_common::embed_version_info()` from `build.rs` - `nanvix_build_common` is a **build-only** helper crate (never linked into the runtime): it stages NanVix binaries next to the executable and resolves the `NANVIX_BIN` prefetch directory. The `nanvix_binaries`, `wxc`, and `lxc` build scripts consume it as a `[build-dependencies]` entry. Runtime constants it needs (binary/snapshot filenames) stay in `nanvix_common`. Keep build-only file-staging logic here, not in `nanvix_common` (which is a runtime dependency of `nanvix_runner`). - Platform-specific modules use `#[cfg(target_os = "windows")]` / `#[cfg(target_os = "linux")]` diff --git a/docs/learning-mode/capabilities.md b/docs/learning-mode/capabilities.md index 004eeb85..68de92d3 100644 --- a/docs/learning-mode/capabilities.md +++ b/docs/learning-mode/capabilities.md @@ -122,5 +122,63 @@ ungranted access is handled while it is recorded: - `mode: "allow"` maps onto `permissiveLearningMode` (allow-and-record) — the fleet-auditing flow. -The capture pipeline is delivered incrementally and is documented separately as -it lands. +### Output file the caller consumes + +After the sandboxed workload exits, MXC decodes the captured denials and writes +them to a **single JSON file** — the deliverable a host application reads to +regenerate its sandbox policy: + +```json +{ + "denials": [ + { + "resource": "C:\\Users\\test\\secret.txt", + "resourceType": "file", + "accessType": "read", + "pid": 1234, + "filetime": 132847890123456789 + }, + { + "resource": "internetClient", + "resourceType": "capability", + "accessType": "unknown", + "pid": 1234, + "filetime": 132847890123512345 + } + ], + "summary": { + "exitCode": 0, + "totalDenials": 2, + "deniedResourcesTruncated": false + } +} +``` + +- `denials` is already de-duplicated per `(resource, accessType)`, so + `summary.totalDenials` equals `denials.length`. +- `resource` is the user-visible identifier for the denied resource, + interpreted by `resourceType`: a canonical `C:\…` path for `file`, the + AppContainer **capability name** (e.g. `internetClient`) for `capability`, + and the raw resource identifier otherwise. Well-known capability SIDs are + resolved to their policy name; custom (hashed) capability SIDs that can't be + reversed fall back to the `S-1-15-3-…` SID string. +- `resourceType` is one of `file`, `ui`, `network`, `capability`, `other`; + `accessType` is one of `read`, `write`, `execute`, `unknown`. (Capability + denials are only recorded under `allow` today — see the mode caveat.) + +**Locating the file.** Set `captureDenials.outputPath` to name the file +explicitly (its parent directory must already exist). MXC inserts a unique +per-run identifier (process id plus random suffix) into the file stem +(`denials.json` → `denials..json`) so concurrent and sequential +captures using the same configured path do not collide. If `outputPath` is +omitted, MXC writes a managed per-run temp file. Either way, `wxc-exec` prints +**one structured pointer line** to its own **stderr** — carrying the *actual* +path — so the caller can locate the deliverable without scanning the filesystem: + +```json +{"type":"captureDenials","outputPath":"C:\\logs\\denials.4321.json","exitCode":0,"totalDenials":2,"deniedResourcesTruncated":false} +``` + +The pointer echoes the file's `summary`; the authoritative record is the file +itself. The intermediate ETW `.etl` trace is an internal, runner-managed temp +file that MXC decodes and then deletes — callers never see it. diff --git a/docs/schema.md b/docs/schema.md index e1782309..233eed1d 100644 --- a/docs/schema.md +++ b/docs/schema.md @@ -65,9 +65,10 @@ production configs and the dev schema when working on experimental features: // is logged (deny-by-default preserved). "allow": // access is allowed and logged (audit; relaxes // deny-by-default, emits a security warning). - "outputPath": "C:\\logs\\denials.etl" // attempts to a learning-mode ETL trace. The - } // parent dir must already exist; omit outputPath - // for a managed per-run temp file. + "outputPath": "C:\\logs\\denials.json" // JSON denials file the app reads. The parent + } // dir must already exist; a unique per-run id is stamped + // into the stem (denials..json) and the actual + // path printed on stderr. Omit outputPath for a managed temp file. }, "lxc": { // LXC-specific diff --git a/schemas/dev/mxc-config.schema.0.8.0-dev.json b/schemas/dev/mxc-config.schema.0.8.0-dev.json index e24b5d09..b62e6319 100644 --- a/schemas/dev/mxc-config.schema.0.8.0-dev.json +++ b/schemas/dev/mxc-config.schema.0.8.0-dev.json @@ -60,7 +60,7 @@ "description": "How each ungranted access check is handled while it is recorded. Both modes log every access the policy does not grant to the ETL trace; the mode only decides whether that access is blocked or allowed. Defaults to `block` when omitted." }, "outputPath": { - "description": "Absolute path where the denial ETL trace is written. The caller names the path; the OS opens it under the caller's own identity when the trace is sealed. When omitted, MXC writes the trace to a managed per-run temporary file. The parent directory must already exist.", + "description": "Absolute path where the JSON denials output file is written — the deliverable a consuming application reads to learn what the workload was denied. It is a single JSON document `{ \"denials\": [...], \"summary\": {...} }`. A per-run identifier (process id plus random suffix) is inserted into the file stem (e.g. `denials.json` -> `denials..json`) so concurrent and sequential captures do not collide; the actual path is reported on stderr. When omitted, MXC writes it to a managed per-run temporary file and prints its path on stderr. The parent directory must already exist. (The intermediate ETL trace is an internal, runner-managed temp file that is decoded then deleted.)", "type": [ "string", "null" diff --git a/sdk/node/src/generated/wire.ts b/sdk/node/src/generated/wire.ts index df0c0a64..9c5b31a8 100644 --- a/sdk/node/src/generated/wire.ts +++ b/sdk/node/src/generated/wire.ts @@ -46,7 +46,7 @@ export interface CaptureDenials { */ mode?: CaptureDenialsMode | null; /** - * Absolute path where the denial ETL trace is written. The caller names the path; the OS opens it under the caller's own identity when the trace is sealed. When omitted, MXC writes the trace to a managed per-run temporary file. The parent directory must already exist. + * Absolute path where the JSON denials output file is written — the deliverable a consuming application reads to learn what the workload was denied. It is a single JSON document `{ "denials": [...], "summary": {...} }`. A per-run identifier (process id plus random suffix) is inserted into the file stem (e.g. `denials.json` -> `denials..json`) so concurrent and sequential captures do not collide; the actual path is reported on stderr. When omitted, MXC writes it to a managed per-run temporary file and prints its path on stderr. The parent directory must already exist. (The intermediate ETL trace is an internal, runner-managed temp file that is decoded then deleted.) */ outputPath?: string | null; } diff --git a/src/Cargo.lock b/src/Cargo.lock index b8c59732..aa9f2548 100644 --- a/src/Cargo.lock +++ b/src/Cargo.lock @@ -88,6 +88,7 @@ version = "0.7.0" dependencies = [ "flatbuffers", "getrandom 0.2.17", + "learning_mode_core", "learning_mode_windows", "sandbox_spec", "serde", diff --git a/src/backends/appcontainer/common/Cargo.toml b/src/backends/appcontainer/common/Cargo.toml index 7e3d3ccf..4da831a4 100644 --- a/src/backends/appcontainer/common/Cargo.toml +++ b/src/backends/appcontainer/common/Cargo.toml @@ -27,6 +27,7 @@ sandbox_spec = { workspace = true } widestring = { workspace = true } winreg = { workspace = true } learning_mode_windows = { workspace = true } +learning_mode_core = { workspace = true } [dev-dependencies] tempfile.workspace = true diff --git a/src/backends/appcontainer/common/src/base_container_runner.rs b/src/backends/appcontainer/common/src/base_container_runner.rs index bbead9b9..f67d5431 100644 --- a/src/backends/appcontainer/common/src/base_container_runner.rs +++ b/src/backends/appcontainer/common/src/base_container_runner.rs @@ -12,11 +12,15 @@ use std::ffi::c_void; use std::fmt::Write; use std::io::IsTerminal; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::ptr; +use learning_mode_core::{ + write_document, DenialAnalyzer, DenialSummary, DenialsDocument, DenialsOutputPointer, +}; use learning_mode_windows::{ - CaptureSession, LearningModeApi, SecurityEnvironmentApi, PROCESS_SECURITY_ENVIRONMENT_FLAG_NONE, + CaptureSession, EtlDenialAnalyzer, LearningModeApi, SecurityEnvironmentApi, + PROCESS_SECURITY_ENVIRONMENT_FLAG_NONE, }; use windows::Win32::Foundation::{ @@ -827,19 +831,24 @@ impl BaseContainerRunner { } } - // Resolve the ETL output path: caller-specified when provided, else a - // managed per-run temp file the runner names (parsed / cleaned up by the - // consume + analyze stages). - let capture_output_path: Option = capture_denials - .as_ref() - .map(|cd| { - cd.output_path - .clone() - .map(PathBuf::from) - .map(Ok) - .unwrap_or_else(managed_capture_output_path) - }) - .transpose()?; + // Resolve two paths for the capture: + // * `capture_etl_path` — an always-internal, runner-managed temp `.etl` + // that the OS broker seals into. It is decoded then deleted in + // `run_teardown`; callers never see it. + // * `capture_output_path` — the JSON denials deliverable that consuming + // apps read: caller-specified via `captureDenials.outputPath` when + // provided, else a managed per-run temp `.json` file. + let (capture_etl_path, capture_output_path) = + if let Some(capture_config) = capture_denials.as_ref() { + ( + Some(managed_capture_output_path()?), + Some(unique_denials_output_path( + capture_config.output_path.as_deref(), + )?), + ) + } else { + (None, None) + }; let _ = writeln!(logger, "{EMOJI_SECTION} SECTION: Load API"); @@ -1415,6 +1424,7 @@ impl BaseContainerRunner { sid_string, proxy_coordinator: std::mem::take(&mut self.proxy_coordinator), capture_session, + capture_etl_path, capture_output_path, }) } @@ -1446,8 +1456,11 @@ struct BaseChild { /// is configured and the OS API is available). Sealed in `run_teardown` /// after the child exits. capture_session: Option, - /// Resolved ETL output path for the capture session (caller-specified or a - /// managed per-run temp file). `Some` iff `capture_session` is `Some`. + /// Internal runner-managed temp `.etl` the broker seals into. Decoded + /// then deleted in `run_teardown`. `Some` iff `capture_session` is `Some`. + capture_etl_path: Option, + /// Resolved JSON denials deliverable path (caller-specified or a managed + /// per-run temp file). `Some` iff `capture_session` is `Some`. capture_output_path: Option, } @@ -1540,8 +1553,13 @@ struct BaseContainerSandboxProcess { /// Live learning-mode capture session, moved from the `BaseChild`. Sealed /// in `run_teardown` once the child has exited and been reaped. capture_session: Option, - /// Resolved ETL output path for the capture session. + /// Internal runner-managed temp `.etl` the broker seals into. + capture_etl_path: Option, + /// Resolved JSON denials deliverable path. capture_output_path: Option, + /// Exit code of the child, recorded by `wait` before teardown so the + /// denials summary can carry it. `None` on the `Drop`/early-exit path. + last_exit_code: Option, } // SAFETY: the fields are Windows HANDLEs / handle-owning managers and owned @@ -1577,7 +1595,9 @@ impl BaseContainerSandboxProcess { proxy_coordinator: std::mem::take(&mut child.proxy_coordinator), teardown_result: None, capture_session: child.capture_session.take(), + capture_etl_path: child.capture_etl_path.take(), capture_output_path: child.capture_output_path.take(), + last_exit_code: None, } } @@ -1589,23 +1609,32 @@ impl BaseContainerSandboxProcess { // Seal the learning-mode ETL trace now that the child has exited and // been reaped (both `wait` and `Drop` kill + reap before calling this). - // `finish` stops the trace to the resolved output path, then closes the - // security environment. The output-file path is surfaced on stderr so a - // caller can locate the denials (the full NDJSON contract is finalized - // in the output/consume stage). + // The ETL is an internal temp: seal it, decode it into the JSON denials + // deliverable that consuming apps read, delete the temp, and print a + // one-line pointer to the deliverable on our own stderr so a caller can + // locate it. Any seal/decode/write failure is returned through `wait()`. let capture_result = if let Some(session) = self.capture_session.take() { + let etl_path = self.capture_etl_path.take(); let output_path = self.capture_output_path.take(); - match session.finish(output_path.as_deref()) { - Ok(()) => { - if let Some(path) = &output_path { - eprintln!("captureDenials: denials ETL written to {}", path.display()); + let exit_code = self.last_exit_code.unwrap_or(-1); + let result = match session.finish(etl_path.as_deref()) { + Ok(()) => match (&etl_path, &output_path) { + (Some(etl), Some(output)) => { + Self::decode_and_emit_denials(etl, output, exit_code) } - Ok(()) - } + _ => Err(std::io::Error::other( + "captureDenials internal output paths were not initialized", + )), + }, Err(error) => Err(std::io::Error::other(format!( "captureDenials failed to seal the denial trace: {error}" ))), + }; + // The internal ETL temp is never handed to callers. + if let Some(etl) = &etl_path { + let _ = std::fs::remove_file(etl); } + result } else { Ok(()) }; @@ -1642,6 +1671,75 @@ impl BaseContainerSandboxProcess { let _ = WaitForSingleObject(self.process.get(), u32::MAX); } } + + /// Decodes the sealed ETL at `etl_path` into the JSON denials document at + /// `output_path`, then prints a one-line [`DenialsOutputPointer`] to stderr. + fn decode_and_emit_denials( + etl_path: &std::path::Path, + output_path: &std::path::Path, + exit_code: i32, + ) -> std::io::Result<()> { + let analysis = EtlDenialAnalyzer.analyze(etl_path).map_err(|error| { + std::io::Error::other(format!( + "captureDenials failed to decode denials ETL: {error}" + )) + })?; + + let summary = DenialSummary::new( + exit_code, + analysis.denials.len(), + analysis.denied_resources_truncated, + ); + let document = DenialsDocument::new(analysis.denials, summary); + + let file = std::fs::File::create(output_path).map_err(|error| { + std::io::Error::other(format!( + "captureDenials failed to create denials output file {}: {error}", + output_path.display() + )) + })?; + let mut writer = std::io::BufWriter::new(file); + write_document(&mut writer, &document).map_err(|error| { + std::io::Error::other(format!( + "captureDenials failed to write denials output file {}: {error}", + output_path.display() + )) + })?; + + // Surface a single structured pointer line on our own stderr so a + // caller can locate the deliverable (the authoritative summary lives in + // the file). + let pointer = DenialsOutputPointer::new(output_path.to_string_lossy(), &document.summary); + eprintln!("{}", pointer.to_line()); + Ok(()) + } +} + +/// Inserts a per-run identifier into a denials output path's file stem so +/// concurrent and sequential captures using the same configured `outputPath` +/// produce distinct files instead of clobbering one another. +/// +/// `C:\app\denials.json` → `C:\app\denials..json`. A path with no +/// extension gets `.`; a bare filename (no parent) keeps its +/// directory-less form. +fn insert_run_id_into_stem(path: &Path, run_id: &str) -> PathBuf { + let Some(file_name) = path.file_name().and_then(|s| s.to_str()) else { + return path.to_path_buf(); + }; + let new_name = match path.extension().and_then(|s| s.to_str()) { + Some(ext) => { + let stem = path + .file_stem() + .and_then(|s| s.to_str()) + .unwrap_or(file_name); + format!("{stem}.{run_id}.{ext}") + } + None => format!("{file_name}.{run_id}"), + }; + match path.parent() { + Some(parent) if !parent.as_os_str().is_empty() => parent.join(new_name), + _ => PathBuf::from(new_name), + } } impl SandboxProcess for BaseContainerSandboxProcess { @@ -1734,6 +1832,9 @@ impl SandboxProcess for BaseContainerSandboxProcess { self.terminate_and_reap(); cancel_and_join_discard(stdout_thread, &self.stdout_canceller); cancel_and_join_discard(stderr_thread, &self.stderr_canceller); + // Record the child's exit code so `run_teardown` can stamp it into the + // denials summary. On a timeout / wait failure there is no exit code. + self.last_exit_code = result.as_ref().ok().copied(); let teardown_result = self.run_teardown(); combine_process_and_teardown_results(result, teardown_result) } @@ -1752,20 +1853,33 @@ impl Drop for BaseContainerSandboxProcess { } fn managed_capture_output_path() -> Result { + let suffix = random_capture_suffix()?; + Ok(std::env::temp_dir().join(format!( + "mxc_capture_denials_{}_{suffix}.etl", + std::process::id() + ))) +} + +fn unique_denials_output_path(configured_path: Option<&str>) -> Result { + let suffix = random_capture_suffix()?; + let run_id = format!("{}_{suffix}", std::process::id()); + Ok(match configured_path { + Some(path) => insert_run_id_into_stem(Path::new(path), &run_id), + None => std::env::temp_dir().join(format!("mxc_denials_{run_id}.json")), + }) +} + +fn random_capture_suffix() -> Result { let mut nonce = [0u8; 16]; getrandom::getrandom(&mut nonce).map_err(|error| { ScriptResponse::error(&format!( - "captureDenials could not generate a unique temporary output path: {error}" + "captureDenials could not generate a unique output path: {error}" )) })?; - let suffix = nonce + Ok(nonce .iter() .map(|byte| format!("{byte:02x}")) - .collect::(); - Ok(std::env::temp_dir().join(format!( - "mxc_capture_denials_{}_{suffix}.etl", - std::process::id() - ))) + .collect::()) } fn combine_process_and_teardown_results( @@ -1832,6 +1946,17 @@ mod tests { assert_eq!(first.extension().and_then(|ext| ext.to_str()), Some("etl")); } + #[test] + fn managed_denials_paths_are_unique_per_run() { + let first = unique_denials_output_path(None).expect("first path"); + let second = unique_denials_output_path(None).expect("second path"); + + assert_ne!(first, second); + assert_eq!(first.parent(), Some(std::env::temp_dir().as_path())); + assert_eq!(second.parent(), Some(std::env::temp_dir().as_path())); + assert_eq!(first.extension().and_then(|ext| ext.to_str()), Some("json")); + } + #[test] fn successful_process_reports_capture_teardown_failure() { let error = @@ -1841,6 +1966,30 @@ mod tests { assert!(error.to_string().contains("seal failed")); } + #[test] + fn insert_run_id_into_stem_injects_id_before_extension() { + let got = insert_run_id_into_stem(Path::new(r"C:\app\denials.json"), "1234_abcd"); + assert_eq!(got, PathBuf::from(r"C:\app\denials.1234_abcd.json")); + } + + #[test] + fn insert_run_id_into_stem_handles_no_extension() { + let got = insert_run_id_into_stem(Path::new(r"C:\app\denials"), "77_abcd"); + assert_eq!(got, PathBuf::from(r"C:\app\denials.77_abcd")); + } + + #[test] + fn insert_run_id_into_stem_handles_bare_filename() { + let got = insert_run_id_into_stem(Path::new("denials.json"), "9_abcd"); + assert_eq!(got, PathBuf::from("denials.9_abcd.json")); + } + + #[test] + fn insert_run_id_into_stem_preserves_multi_dot_stem() { + let got = insert_run_id_into_stem(Path::new(r"C:\app\out.denials.json"), "5_abcd"); + assert_eq!(got, PathBuf::from(r"C:\app\out.denials.5_abcd.json")); + } + #[test] fn is_api_not_implemented_classifies_disabled_feature() { assert!(is_api_not_implemented(ERROR_CALL_NOT_IMPLEMENTED.0)); diff --git a/src/backends/learning_mode/windows/examples/lm_analyze.rs b/src/backends/learning_mode/windows/examples/lm_analyze.rs index 31390238..611dd1b1 100644 --- a/src/backends/learning_mode/windows/examples/lm_analyze.rs +++ b/src/backends/learning_mode/windows/examples/lm_analyze.rs @@ -1,8 +1,8 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -//! Decode a sealed learning-mode `.etl` into the captureDenials RFC 7464 -//! JSON text sequence, or dump its raw ETW events for schema discovery. +//! Decode a sealed learning-mode `.etl` into the captureDenials JSON +//! output document, or dump its raw ETW events for schema discovery. //! //! This is a developer diagnostic for inspecting captured traces manually. //! End users and SDK agents do not invoke it: the BaseContainer runner seals @@ -11,7 +11,7 @@ //! Usage: //! //! ```text -//! # Emit the DeniedResource JSON text sequence (0x1E-framed) to stdout: +//! # Emit the DeniedResource JSON document to stdout: //! cargo run -p learning_mode_windows --example lm_analyze -- --exit-code //! //! # Dump every decoded event (id + property name/value pairs): @@ -57,7 +57,7 @@ mod windows_impl { eprintln!("--exit-code is required unless --raw is used"); return 2; }; - emit_json_sequence(path, exit_code) + emit_json(path, exit_code) } } @@ -66,8 +66,8 @@ mod windows_impl { args.get(index + 1)?.parse().ok() } - /// Decodes denials and writes the RFC 7464 JSON text sequence to stdout. - fn emit_json_sequence(path: &Path, exit_code: i32) -> i32 { + /// Decodes denials and writes the JSON output document to stdout. + fn emit_json(path: &Path, exit_code: i32) -> i32 { let analysis = match EtlDenialAnalyzer.analyze(path) { Ok(d) => d, Err(e) => { @@ -80,16 +80,17 @@ mod windows_impl { analysis.denials.len(), analysis.denied_resources_truncated, ); + let document = emit::DenialsDocument::new(analysis.denials, summary); let stdout = std::io::stdout(); let mut handle = stdout.lock(); - if let Err(e) = emit::write_stream(&mut handle, &analysis.denials, &summary) { + if let Err(e) = emit::write_document(&mut handle, &document) { eprintln!("write failed: {e}"); return 1; } eprintln!( "lm_analyze: {} unique denial(s){}", - analysis.denials.len(), - if analysis.denied_resources_truncated { + document.denials.len(), + if document.summary.denied_resources_truncated { " (truncated)" } else { "" diff --git a/src/backends/learning_mode/windows/src/capability_names.rs b/src/backends/learning_mode/windows/src/capability_names.rs new file mode 100644 index 00000000..7cd6ae92 --- /dev/null +++ b/src/backends/learning_mode/windows/src/capability_names.rs @@ -0,0 +1,113 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +//! Capability SID → friendly-name resolution. +//! +//! Brokered-capability denials (see [`crate::extractors`]) identify the +//! denied capability by its *capability SID*, not by name. AppContainer +//! capability SIDs live under the well-known `S-1-15-3-…` domain, and there +//! is no OS reverse API — Windows only exposes `DeriveCapabilitySidsFromName` +//! (name → SID). So resolution splits two ways: +//! +//! - A small set of **legacy** capabilities have short, fixed RIDs +//! (`S-1-15-3-1` == `internetClient`, `S-1-15-3-3` == +//! `privateNetworkClientServer`, …). Those are enumerated in +//! [`WELL_KNOWN`] and resolve to a human-readable name. +//! - Every **modern / custom** capability SID is derived by hashing the +//! capability's UTF-16 name into four 32-bit RIDs (`S-1-15-3-w-x-y-z`). +//! That hash is one-way, so we cannot recover the name — we surface the +//! SID string verbatim, which is still a stable, greppable identifier a +//! consumer can map back to a policy entry. +//! +//! Only the friendly name is directly consumable when a host regenerates its +//! sandbox policy, because the policy `capabilities` field is name-based +//! (MXC converts names → SIDs at launch, never the reverse). Resolving the +//! well-known network/library capabilities — the ones MXC actually uses — is +//! therefore what makes a capability denial actionable for config +//! regeneration; the SID fallback is a best-effort diagnostic identifier. +//! +//! [`resolve`] is the single entry point: it maps a canonical SID string to +//! the friendly name when known, otherwise echoes the SID unchanged. + +/// Legacy AppContainer capability SIDs with well-known short RIDs, paired +/// with the capability name used in sandbox policy. +/// +/// The RID is the single sub-authority following the `S-1-15-3-` prefix. +/// Names match the manifest / `SandboxPolicy` capability identifiers so a +/// consumer can feed a resolved denial straight back into policy. +const WELL_KNOWN: &[(u32, &str)] = &[ + (1, "internetClient"), + (2, "internetClientServer"), + (3, "privateNetworkClientServer"), + (4, "picturesLibrary"), + (5, "videosLibrary"), + (6, "musicLibrary"), + (7, "documentsLibrary"), + (8, "enterpriseAuthentication"), + (9, "sharedUserCertificates"), + (10, "removableStorage"), + (11, "appointments"), + (12, "contacts"), +]; + +/// Resolves a capability SID string to a friendly capability name. +/// +/// Accepts a canonical SID string (as produced by the TDH SID decoder, e.g. +/// `S-1-15-3-1`). Returns the well-known capability name when the SID is a +/// legacy `S-1-15-3-` with a recognised RID; otherwise returns the +/// input SID unchanged (custom capability SIDs are one-way hashes and cannot +/// be reversed). A non-SID input is echoed back untouched. +#[must_use] +pub fn resolve(sid: &str) -> String { + if let Some(rid) = legacy_capability_rid(sid) { + if let Some((_, name)) = WELL_KNOWN.iter().find(|(r, _)| *r == rid) { + return (*name).to_string(); + } + } + sid.to_string() +} + +/// Returns the single RID of a legacy `S-1-15-3-` capability SID. +/// +/// Returns `None` for anything that is not exactly a three-part +/// `S-1-15-3-` SID (custom capabilities carry four hashed RIDs and are +/// intentionally excluded). +fn legacy_capability_rid(sid: &str) -> Option { + let rest = sid.strip_prefix("S-1-15-3-")?; + // A legacy capability SID has exactly one sub-authority after the domain + // prefix; hashed custom SIDs have four, so reject anything with a `-`. + if rest.contains('-') { + return None; + } + rest.parse::().ok() +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn resolves_well_known_legacy_capabilities() { + assert_eq!(resolve("S-1-15-3-1"), "internetClient"); + assert_eq!(resolve("S-1-15-3-3"), "privateNetworkClientServer"); + assert_eq!(resolve("S-1-15-3-12"), "contacts"); + } + + #[test] + fn unknown_legacy_rid_echoes_sid() { + assert_eq!(resolve("S-1-15-3-9999"), "S-1-15-3-9999"); + } + + #[test] + fn custom_hashed_capability_sid_is_not_reversed() { + // Four hashed RIDs -> one-way, echoed verbatim. + let custom = "S-1-15-3-1024-1065365936-1281604716-3511738428-1654721687"; + assert_eq!(resolve(custom), custom); + } + + #[test] + fn non_capability_sid_is_passed_through() { + assert_eq!(resolve("S-1-5-18"), "S-1-5-18"); + assert_eq!(resolve("not-a-sid"), "not-a-sid"); + } +} diff --git a/src/backends/learning_mode/windows/src/etl_decode.rs b/src/backends/learning_mode/windows/src/etl_decode.rs index 1ab062af..c5b85eec 100644 --- a/src/backends/learning_mode/windows/src/etl_decode.rs +++ b/src/backends/learning_mode/windows/src/etl_decode.rs @@ -99,9 +99,9 @@ impl<'visitor> Accumulator<'visitor> { } fn add_raw_denial(&mut self, raw: RawDenial) { - let path = if raw.resource_type == learning_mode_core::ResourceType::File { + let resource = if raw.resource_type == learning_mode_core::ResourceType::File { match path_norm::to_user_visible(&raw.object_name) { - Some(path) if path_norm::is_user_visible_absolute(&path) => path, + Some(resource) if path_norm::is_user_visible_absolute(&resource) => resource, Some(_) => return, None if path_norm::is_user_visible_absolute(&raw.object_name) => { raw.object_name.clone() @@ -111,22 +111,25 @@ impl<'visitor> Accumulator<'visitor> { } else { path_norm::to_user_visible(&raw.object_name).unwrap_or_else(|| raw.object_name.clone()) }; - let dedup_path = match raw.resource_type { + let dedup_resource = match raw.resource_type { learning_mode_core::ResourceType::File | learning_mode_core::ResourceType::Other => { - path.to_ascii_lowercase() + resource.to_ascii_lowercase() } - _ => path.clone(), + _ => resource.clone(), }; - if self.seen.contains(&(dedup_path.clone(), raw.access_type)) { + if self + .seen + .contains(&(dedup_resource.clone(), raw.access_type)) + { return; } if self.denials.len() >= MAX_UNIQUE_DENIALS { self.truncated = true; return; } - self.seen.insert((dedup_path, raw.access_type)); + self.seen.insert((dedup_resource, raw.access_type)); self.denials.push(DeniedResource { - path, + resource, resource_type: raw.resource_type, access_type: raw.access_type, pid: raw.pid, @@ -204,9 +207,9 @@ pub fn visit_raw_events( Ok(accumulator.raw_event_count) } -/// De-duplicates raw denials by `(user-visible path, accessType)`, -/// normalising kernel paths to drive-letter form and preserving first-seen -/// order. +/// De-duplicates raw denials by `(user-visible resource, accessType)`, +/// normalising case-insensitive Windows file/registry identifiers while +/// preserving first-seen display spelling and order. #[cfg(test)] fn dedup_to_resources>(raws: I) -> AnalysisResult { let mut accumulator = Accumulator::analyze(); @@ -375,11 +378,11 @@ mod tests { raw(r"C:\b", AccessType::Read, ResourceType::File), ]; let out = dedup_to_resources(denials).denials; - assert_eq!(out.len(), 3, "unique (path, access) pairs"); - assert_eq!(out[0].path, r"C:\a"); + assert_eq!(out.len(), 3, "unique (resource, access) pairs"); + assert_eq!(out[0].resource, r"C:\a"); assert_eq!(out[0].access_type, AccessType::Read); assert_eq!(out[1].access_type, AccessType::Write); - assert_eq!(out[2].path, r"C:\b"); + assert_eq!(out[2].resource, r"C:\b"); } #[test] @@ -389,8 +392,8 @@ mod tests { raw(r"C:\a", AccessType::Read, ResourceType::File), ]; let out = dedup_to_resources(denials).denials; - assert_eq!(out[0].path, r"C:\z"); - assert_eq!(out[1].path, r"C:\a"); + assert_eq!(out[0].resource, r"C:\z"); + assert_eq!(out[1].resource, r"C:\a"); } #[test] @@ -429,7 +432,7 @@ mod tests { ]; let out = dedup_to_resources(denials).denials; assert_eq!(out.len(), 1); - assert_eq!(out[0].path, r"C:\Data\File.txt"); + assert_eq!(out[0].resource, r"C:\Data\File.txt"); } #[test] @@ -539,16 +542,16 @@ mod tests { let out = resources_from_events(&events).denials; assert_eq!(out.len(), 3); - assert_eq!(out[0].path, r"C:\data\test\bin\"); + assert_eq!(out[0].resource, r"C:\data\test\bin\"); assert_eq!(out[0].resource_type, ResourceType::File); assert_eq!(out[0].access_type, AccessType::Write); assert_eq!(out[0].pid, 5480); - assert_eq!(out[1].path, r"\REGISTRY\USER\.DEFAULT\Console"); + assert_eq!(out[1].resource, r"\REGISTRY\USER\.DEFAULT\Console"); assert_eq!(out[1].resource_type, ResourceType::Other); assert_eq!(out[1].access_type, AccessType::Read); - assert_eq!(out[2].path, "S-1-15-3-1"); + assert_eq!(out[2].resource, "internetClient"); assert_eq!(out[2].resource_type, ResourceType::Capability); assert_eq!(out[2].access_type, AccessType::Unknown); assert_eq!(out[2].pid, 0x1acc, "pid from payload ProcessId"); @@ -587,7 +590,7 @@ mod tests { let out = resources_from_events(&events).denials; assert_eq!(out.len(), 1); - assert_eq!(out[0].path, r"C:\data\test\bin\"); + assert_eq!(out[0].resource, r"C:\data\test\bin\"); assert_eq!(out[0].access_type, AccessType::Write); } diff --git a/src/backends/learning_mode/windows/src/extractors.rs b/src/backends/learning_mode/windows/src/extractors.rs index 6ba29f98..67704423 100644 --- a/src/backends/learning_mode/windows/src/extractors.rs +++ b/src/backends/learning_mode/windows/src/extractors.rs @@ -39,9 +39,10 @@ //! record (`Denied` / `PackageSid` / `ProcessId`), emitted under //! `block`; `allow` folds the same information into the //! empty-`ObjectType` event 14 above. Mapped to [`ResourceType::Capability`]. -//! Records are emitted only when TDH surfaces a decoded capability -//! identifier; otherwise they are omitted until SID/ACE decoding is -//! available. +//! The capability *name* is resolved from the `PackageSid` capability SID +//! via [`crate::capability_names`] (well-known SID → friendly name; custom +//! hashed capabilities fall back to the SID string). Records without a +//! decoded identifier are omitted. use learning_mode_core::{AccessType, ResourceType}; use windows::core::GUID; @@ -224,8 +225,11 @@ pub fn build_denial_from_learning_mode( /// Emitted under `block`. The record reports a `Denied` boolean; we /// only surface actual denials. The originating process is taken from the /// payload `ProcessId` (which is more precise than the ETW header pid for -/// brokered checks) when present, else the header pid. Records without a -/// decoded capability identifier are omitted. +/// brokered checks) when present, else the header pid. The capability name +/// comes from the `PackageSid` capability SID, resolved to its friendly +/// policy name via [`crate::capability_names`] (custom hashed capabilities +/// fall back to the SID string). Records without a decoded identifier are +/// omitted. pub fn build_denial_from_capability( parts: &DecodedEventParts, pid: u32, @@ -243,13 +247,25 @@ pub fn build_denial_from_capability( .and_then(|v| parse_u32(v)) .unwrap_or(pid); - // Emit only when the decoder has surfaced a usable identifier. Empty - // capability records collapse during dedup and cannot guide policy fixes. + // Resolve the denied capability's name. The event carries it as a + // capability SID (`PackageSid`, rendered `S-1-15-3-…` by the TDH SID + // decoder); map well-known capability SIDs to their friendly policy name + // and fall back to the SID string for custom (hashed) capabilities. let object_name = find_prop(&parts.props, "CapabilityName") .or_else(|| find_prop(&parts.props, "Capability")) - .or_else(|| find_prop(&parts.props, "PackageSid")) .map(|v| v.trim_matches('"').to_string()) - .filter(|name| !name.is_empty() && name != "" && name != "")?; + .or_else(|| { + find_prop(&parts.props, "PackageSid") + .or_else(|| find_prop(&parts.props, "CapabilitySid")) + .or_else(|| find_prop(&parts.props, "Sid")) + .map(|v| crate::capability_names::resolve(v.trim_matches('"'))) + }) + .filter(|name| { + !name.is_empty() + && name != "" + && name != "" + && name != "" + })?; Some(RawDenial { pid, @@ -583,7 +599,42 @@ mod tests { assert_eq!(ev.access_type, AccessType::Unknown); // pid comes from the payload ProcessId (0x1acc), not the header. assert_eq!(ev.pid, 0x1acc); - assert_eq!(ev.object_name, "S-1-15-3-1"); + assert_eq!(ev.object_name, "internetClient"); + } + + #[test] + fn capability_denial_resolves_well_known_sid_to_name() { + // PackageSid rendered by the TDH SID decoder as a legacy capability + // SID -> resolved to its friendly policy name. + let p = parts( + 28, + &[ + ("ProcessId", "0x10"), + ("PackageSid", "\"S-1-15-3-1\""), + ("Denied", "true"), + ], + ); + let ev = extract_denial(&p, 1, FIXED_FILETIME).unwrap(); + assert_eq!(ev.resource_type, ResourceType::Capability); + assert_eq!(ev.object_name, "internetClient"); + } + + #[test] + fn capability_denial_custom_sid_falls_back_to_sid_string() { + // A custom (hashed) capability SID cannot be reversed, so the SID + // string itself is surfaced. + let sid = "S-1-15-3-1024-1065365936-1281604716-3511738428-1654721687"; + let sid_quoted = format!("\"{sid}\""); + let p = parts( + 28, + &[ + ("ProcessId", "0x10"), + ("PackageSid", sid_quoted.as_str()), + ("Denied", "true"), + ], + ); + let ev = extract_denial(&p, 1, FIXED_FILETIME).unwrap(); + assert_eq!(ev.object_name, sid); } #[test] diff --git a/src/backends/learning_mode/windows/src/lib.rs b/src/backends/learning_mode/windows/src/lib.rs index 294caa9e..09fb8eeb 100644 --- a/src/backends/learning_mode/windows/src/lib.rs +++ b/src/backends/learning_mode/windows/src/lib.rs @@ -35,6 +35,8 @@ mod lifecycle; #[cfg(target_os = "windows")] mod secenv; +#[cfg(target_os = "windows")] +mod capability_names; #[cfg(target_os = "windows")] mod etl_decode; #[cfg(target_os = "windows")] diff --git a/src/backends/learning_mode/windows/src/tdh_decode.rs b/src/backends/learning_mode/windows/src/tdh_decode.rs index 5320dfb1..e5f4944f 100644 --- a/src/backends/learning_mode/windows/src/tdh_decode.rs +++ b/src/backends/learning_mode/windows/src/tdh_decode.rs @@ -654,6 +654,47 @@ mod tests { let (value, consumed) = format_property_value(TDH_INTYPE_SID, 0, bytes.as_ptr(), bytes.len()); assert_eq!(value, "S-1-15-3-1"); + assert_eq!(consumed, bytes.len()); // 8 header + 2*4 sub-authorities + } + + #[test] + fn format_property_value_sid_decodes_package_sid() { + // A package SID (S-1-15-2-...): authority 15, first sub-authority 2 + // (package type), then 7 hash RIDs — 8 sub-authorities total. + let rids: [u32; 8] = [ + 2, 1596788311, 800953881, 392591971, 523554621, 937337662, 1483227527, 333310193, + ]; + let mut bytes = vec![1u8, rids.len() as u8]; + bytes.extend_from_slice(&[0, 0, 0, 0, 0, 15]); // authority 15 + for rid in rids { + bytes.extend_from_slice(&rid.to_le_bytes()); + } + let (val, consumed) = format_property_value(TDH_INTYPE_SID, 0, bytes.as_ptr(), bytes.len()); + assert_eq!( + val, + "S-1-15-2-1596788311-800953881-392591971-523554621-937337662-1483227527-333310193" + ); + assert_eq!(consumed, bytes.len()); // 8 header + 8*4 sub-authorities + } + + #[test] + fn format_property_value_sid_decodes_well_known_local_system() { + // A 1-sub-authority well-known SID: S-1-5-18 (LocalSystem), the shape + // carried by the event-28 `UserSid` field. + let mut bytes = vec![1u8, 1]; + bytes.extend_from_slice(&[0, 0, 0, 0, 0, 5]); // authority 5 (NT) + bytes.extend_from_slice(&18u32.to_le_bytes()); + let (val, consumed) = format_property_value(TDH_INTYPE_SID, 0, bytes.as_ptr(), bytes.len()); + assert_eq!(val, "S-1-5-18"); + assert_eq!(consumed, bytes.len()); + } + + #[test] + fn format_property_value_sid_truncated_is_placeholder() { + // SubAuthorityCount claims 3 (needs 20 bytes) but only 8 present. + let bytes = [1u8, 3, 0, 0, 0, 0, 0, 15]; + let (val, consumed) = format_property_value(TDH_INTYPE_SID, 0, bytes.as_ptr(), bytes.len()); + assert_eq!(val, ""); assert_eq!(consumed, bytes.len()); } } diff --git a/src/core/learning_mode_core/src/analyze.rs b/src/core/learning_mode_core/src/analyze.rs index 74ff2d17..473ead30 100644 --- a/src/core/learning_mode_core/src/analyze.rs +++ b/src/core/learning_mode_core/src/analyze.rs @@ -62,9 +62,9 @@ pub enum AnalyzeError { /// Decodes a platform-native capture source into de-duplicated denials. /// -/// Implementors return bounded unique `(path, accessType)` observations and +/// Implementors return bounded unique `(resource, accessType)` observations and /// whether additional unique records were truncated; the caller wraps them with a -/// [`crate::summary::DenialSummary`] and emits an RFC 7464 JSON text sequence via +/// [`crate::summary::DenialSummary`] and writes the JSON output document via /// [`crate::emit`]. pub trait DenialAnalyzer { /// Analyses the capture at `source_path`, returning its bounded denial @@ -95,7 +95,7 @@ mod tests { #[test] fn analyzer_is_object_safe_and_returns_denials() { let denials = vec![DeniedResource { - path: r"C:\a".to_string(), + resource: r"C:\a".to_string(), resource_type: ResourceType::File, access_type: AccessType::Read, pid: 1, diff --git a/src/core/learning_mode_core/src/emit.rs b/src/core/learning_mode_core/src/emit.rs index c14e7c87..b7656835 100644 --- a/src/core/learning_mode_core/src/emit.rs +++ b/src/core/learning_mode_core/src/emit.rs @@ -1,83 +1,144 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -//! RFC 7464 JSON text sequence emitter for captureDenials output files. +//! Plain-JSON emitter for captureDenials output files. //! -//! The output file follows the [RFC 7464] "JSON Text Sequences" framing: -//! every record is preceded by an ASCII Record Separator (`0x1E`) and -//! terminated by a line feed (`0x0A`): +//! The output file is a single JSON document with two fields — the array +//! of unique denials and a terminating summary: //! -//! ```text -//! {"type":"denial", ...} -//! {"type":"denial", ...} -//! {"type":"summary", ...} +//! ```json +//! { +//! "denials": [ +//! { "resource": "C:\\Users\\test\\secret.txt", "resourceType": "file", +//! "accessType": "read", "pid": 1234, "filetime": 132847890123456789 } +//! ], +//! "summary": { "exitCode": 0, "totalDenials": 1, +//! "deniedResourcesTruncated": false } +//! } //! ``` //! -//! The `0x1E` prefix is load-bearing: it effectively never appears in -//! legitimate workload output, so a consumer can split on it to reliably -//! separate MXC envelopes from any interleaved bytes. The summary frame -//! terminates the stream for one `wxc-exec` invocation. +//! A host application reads the whole file, deserialises it into +//! [`DenialsDocument`], and regenerates its sandbox policy from the +//! `denials` array. The document is self-contained: one file is written +//! per `wxc-exec` invocation, so there is no record framing to parse. +//! +//! Separately, the runner prints a one-line [`DenialsOutputPointer`] to +//! its own stderr so a caller can locate the file without scanning the +//! filesystem — see that type for the pointer contract. //! //! This module is transport-agnostic — it writes to any [`io::Write`], //! so the same code path serves the on-disk output file and in-memory //! test buffers. -//! -//! [RFC 7464]: https://www.rfc-editor.org/rfc/rfc7464 use std::io::{self, Write}; -use crate::frame::DenialFrame; +use serde::{Deserialize, Serialize}; + use crate::model::DeniedResource; use crate::summary::DenialSummary; -/// ASCII Record Separator (`0x1E`), prefixed to every framed record. -pub const RECORD_SEPARATOR: u8 = 0x1E; +/// The complete on-disk captureDenials output document. +/// +/// Serialises to a single JSON object `{ "denials": [...], "summary": {...} }`. +/// `denials` is already de-duplicated by the decoder, so consumers can +/// treat every entry as a unique `(resource, accessType)` observation, and +/// `summary.total_denials` equals `denials.len()` for a non-truncated +/// capture. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct DenialsDocument { + /// The unique denials observed during the capture. + pub denials: Vec, + /// Terminating summary (exit code, count, truncation flag). + pub summary: DenialSummary, +} + +impl DenialsDocument { + /// Builds a document from a de-duplicated denial set and its summary. + #[must_use] + pub fn new(denials: Vec, summary: DenialSummary) -> Self { + Self { denials, summary } + } +} -/// Writes one framed record: `RS` + compact JSON + `LF`. +/// Writes the captureDenials document as pretty-printed JSON followed by a +/// trailing newline, then flushes the writer. /// /// # Errors /// /// Returns any [`io::Error`] from the underlying writer, or a -/// serialisation error surfaced as [`io::ErrorKind::InvalidData`]. -pub fn write_frame(writer: &mut W, frame: &DenialFrame) -> io::Result<()> { - let json = serde_json::to_vec(frame).map_err(io::Error::other)?; - writer.write_all(&[RECORD_SEPARATOR])?; +/// serialisation error surfaced as [`io::ErrorKind::Other`]. +pub fn write_document(writer: &mut W, document: &DenialsDocument) -> io::Result<()> { + let json = serde_json::to_vec_pretty(document).map_err(io::Error::other)?; writer.write_all(&json)?; - writer.write_all(b"\n") + writer.write_all(b"\n")?; + writer.flush() } -/// Writes a complete captureDenials stream: one `denial` frame per -/// resource, followed by exactly one terminating `summary` frame, then -/// flushes the writer. -/// -/// The caller is responsible for having already de-duplicated -/// `resources`; `summary.total_denials` should equal `resources.len()` -/// for a non-truncated capture. -/// -/// # Errors +/// The single structured line a runner prints to its own stderr to point a +/// caller at a freshly written captureDenials output file. /// -/// Returns the first [`io::Error`] encountered while writing or flushing. -pub fn write_stream( - writer: &mut W, - resources: &[DeniedResource], - summary: &DenialSummary, -) -> io::Result<()> { - for resource in resources { - let frame = DenialFrame::Denial(resource.clone()); - write_frame(writer, &frame)?; +/// It is one self-describing JSON object on its own line, tagged +/// `"type":"captureDenials"` so a consumer scanning `wxc-exec`'s stderr can +/// distinguish it from arbitrary workload output. It echoes the file's +/// [`DenialSummary`] so a caller can decide whether to open the file at all. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct DenialsOutputPointer { + /// Discriminator; always the string `"captureDenials"`. + #[serde(rename = "type")] + pub kind: String, + /// Absolute path to the JSON denials output file. + pub output_path: String, + /// Exit code of the sandboxed child (mirrors the file's summary). + pub exit_code: i32, + /// Count of unique denials written (mirrors the file's summary). + pub total_denials: usize, + /// Whether the emitted denial set was truncated (mirrors the summary). + pub denied_resources_truncated: bool, +} + +impl DenialsOutputPointer { + /// The fixed `type` discriminator value. + pub const KIND: &'static str = "captureDenials"; + + /// Builds a pointer for `output_path` echoing `summary`. + #[must_use] + pub fn new(output_path: impl Into, summary: &DenialSummary) -> Self { + Self { + kind: Self::KIND.to_string(), + output_path: output_path.into(), + exit_code: summary.exit_code, + total_denials: summary.total_denials, + denied_resources_truncated: summary.denied_resources_truncated, + } + } + + /// Serialises the pointer to a single-line JSON string (no trailing + /// newline). A fixed-shape struct of strings/ints/bools always + /// serialises, so this is effectively infallible. + #[must_use] + pub fn to_line(&self) -> String { + serde_json::to_string(self).unwrap_or_else(|_| { + format!( + r#"{{"type":"{}","outputPath":"","exitCode":{},"totalDenials":{},"deniedResourcesTruncated":{}}}"#, + Self::KIND, + self.exit_code, + self.total_denials, + self.denied_resources_truncated + ) + }) } - write_frame(writer, &DenialFrame::Summary(*summary))?; - writer.flush() } #[cfg(test)] mod tests { use super::*; - use crate::model::{AccessType, ResourceType}; + use crate::model::{AccessType, DeniedResource, ResourceType}; - fn sample(path: &str) -> DeniedResource { + fn sample(resource: &str) -> DeniedResource { DeniedResource { - path: path.to_string(), + resource: resource.to_string(), resource_type: ResourceType::File, access_type: AccessType::Read, pid: 100, @@ -86,60 +147,63 @@ mod tests { } #[test] - fn write_frame_prefixes_rs_and_terminates_lf() { + fn write_document_emits_single_json_object_with_denials_and_summary() { + let doc = DenialsDocument::new( + vec![sample(r"C:\a"), sample(r"C:\b")], + DenialSummary::new(0, 2, false), + ); let mut buf = Vec::new(); - write_frame(&mut buf, &DenialFrame::Denial(sample(r"C:\a"))).unwrap(); - assert_eq!(buf[0], RECORD_SEPARATOR); - assert_eq!(*buf.last().unwrap(), b'\n'); - // Exactly one RS and one LF for a single record. - assert_eq!(buf.iter().filter(|&&b| b == RECORD_SEPARATOR).count(), 1); - assert_eq!(buf.iter().filter(|&&b| b == b'\n').count(), 1); + write_document(&mut buf, &doc).unwrap(); + + let text = String::from_utf8(buf).unwrap(); + assert!(text.ends_with('\n')); + // No RFC 7464 record separators — this is a plain JSON document. + assert!(!text.contains('\u{1e}')); + let value: serde_json::Value = serde_json::from_str(&text).unwrap(); + assert_eq!(value["denials"].as_array().unwrap().len(), 2); + assert_eq!(value["summary"]["totalDenials"], 2); + assert_eq!(value["denials"][0]["resourceType"], "file"); } #[test] - fn write_stream_emits_denials_then_single_summary() { - let resources = vec![sample(r"C:\a"), sample(r"C:\b")]; - let summary = DenialSummary::new(0, 2, false); + fn write_document_round_trips() { + let doc = DenialsDocument::new(vec![sample(r"C:\a")], DenialSummary::new(7, 1, true)); let mut buf = Vec::new(); - write_stream(&mut buf, &resources, &summary).unwrap(); - - let text = String::from_utf8(buf).unwrap(); - let records: Vec<&str> = text - .split(RECORD_SEPARATOR as char) - .filter(|s| !s.is_empty()) - .collect(); - assert_eq!(records.len(), 3, "2 denials + 1 summary"); - assert!(records[0].contains("\"type\":\"denial\"")); - assert!(records[1].contains("\"type\":\"denial\"")); - assert!(records[2].contains("\"type\":\"summary\"")); - assert!(records[2].contains("\"totalDenials\":2")); + write_document(&mut buf, &doc).unwrap(); + let parsed: DenialsDocument = serde_json::from_slice(&buf).unwrap(); + assert_eq!(doc, parsed); } #[test] - fn written_stream_round_trips_by_splitting_on_rs() { - let resources = vec![sample(r"C:\a")]; - let summary = DenialSummary::new(7, 1, true); + fn empty_capture_still_writes_document_with_summary() { + let doc = DenialsDocument::new(vec![], DenialSummary::new(0, 0, false)); let mut buf = Vec::new(); - write_stream(&mut buf, &resources, &summary).unwrap(); + write_document(&mut buf, &doc).unwrap(); + let parsed: DenialsDocument = serde_json::from_slice(&buf).unwrap(); + assert!(parsed.denials.is_empty()); + assert_eq!(parsed.summary.total_denials, 0); + } - let text = String::from_utf8(buf).unwrap(); - let frames: Vec = text - .split(RECORD_SEPARATOR as char) - .filter(|s| !s.trim().is_empty()) - .map(|s| serde_json::from_str(s.trim()).unwrap()) - .collect(); - assert_eq!(frames.len(), 2); - assert_eq!(frames[0], DenialFrame::Denial(sample(r"C:\a"))); - assert_eq!(frames[1], DenialFrame::Summary(summary)); + #[test] + fn pointer_is_single_line_tagged_json_echoing_summary() { + let summary = DenialSummary::new(3, 5, true); + let pointer = DenialsOutputPointer::new(r"C:\out\denials.json", &summary); + let line = pointer.to_line(); + + assert!(!line.contains('\n')); + let value: serde_json::Value = serde_json::from_str(&line).unwrap(); + assert_eq!(value["type"], "captureDenials"); + assert_eq!(value["outputPath"], r"C:\out\denials.json"); + assert_eq!(value["exitCode"], 3); + assert_eq!(value["totalDenials"], 5); + assert_eq!(value["deniedResourcesTruncated"], true); } #[test] - fn empty_capture_still_writes_summary() { + fn pointer_round_trips_through_json() { let summary = DenialSummary::new(0, 0, false); - let mut buf = Vec::new(); - write_stream(&mut buf, &[], &summary).unwrap(); - let text = String::from_utf8(buf).unwrap(); - assert!(text.contains("\"type\":\"summary\"")); - assert_eq!(text.matches('\n').count(), 1); + let pointer = DenialsOutputPointer::new("out.json", &summary); + let parsed: DenialsOutputPointer = serde_json::from_str(&pointer.to_line()).unwrap(); + assert_eq!(pointer, parsed); } } diff --git a/src/core/learning_mode_core/src/frame.rs b/src/core/learning_mode_core/src/frame.rs deleted file mode 100644 index 2505caae..00000000 --- a/src/core/learning_mode_core/src/frame.rs +++ /dev/null @@ -1,98 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -//! The self-describing wire records that make up a captureDenials output -//! stream. -//! -//! A captureDenials output file is a sequence of JSON records, each -//! tagged with a `type` discriminator so a consumer can dispatch without -//! positional assumptions: -//! -//! ```text -//! {"type":"denial","path":"...","resourceType":"file","accessType":"read","pid":123,"filetime":"..."} -//! {"type":"denial", ...} -//! {"type":"summary","exitCode":0,"totalDenials":2,"deniedResourcesTruncated":false} -//! ``` -//! -//! [`DenialFrame`] is an internally-tagged enum over the two record -//! kinds, so `serde` handles the `type` field and consumers get an -//! exhaustive match. The framing bytes (RFC 7464 record separators) are -//! added by [`crate::emit`]; this module only defines the JSON objects. - -use serde::{Deserialize, Serialize}; - -use crate::model::DeniedResource; -use crate::summary::DenialSummary; - -/// One record in a captureDenials output stream. -/// -/// Serialises with an internal `type` tag (`"denial"` / `"summary"`) -/// flattened alongside the payload fields, matching the on-disk JSON sequence -/// contract consumers parse. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -#[serde(tag = "type", rename_all = "lowercase")] -pub enum DenialFrame { - /// A single denied resource observation. - Denial(DeniedResource), - /// The terminating summary; exactly one per stream, written last. - Summary(DenialSummary), -} - -impl From for DenialFrame { - fn from(resource: DeniedResource) -> Self { - DenialFrame::Denial(resource) - } -} - -impl From for DenialFrame { - fn from(summary: DenialSummary) -> Self { - DenialFrame::Summary(summary) - } -} - -#[cfg(test)] -mod tests { - use super::*; - use crate::model::{AccessType, ResourceType}; - - #[test] - fn denial_frame_carries_type_tag() { - let frame = DenialFrame::from(DeniedResource { - path: r"C:\x".to_string(), - resource_type: ResourceType::File, - access_type: AccessType::Read, - pid: 7, - filetime: 9, - }); - let json = serde_json::to_string(&frame).unwrap(); - assert!(json.starts_with("{\"type\":\"denial\""), "got {json}"); - assert!(json.contains("\"path\":\"C:\\\\x\""), "got {json}"); - } - - #[test] - fn summary_frame_carries_type_tag() { - let frame = DenialFrame::from(DenialSummary::new(0, 1, false)); - let json = serde_json::to_string(&frame).unwrap(); - assert!(json.starts_with("{\"type\":\"summary\""), "got {json}"); - assert!(json.contains("\"totalDenials\":1"), "got {json}"); - } - - #[test] - fn frame_round_trips_through_json() { - let denial = DenialFrame::from(DeniedResource { - path: r"C:\y".to_string(), - resource_type: ResourceType::Capability, - access_type: AccessType::Unknown, - pid: 3, - filetime: 4, - }); - let parsed: DenialFrame = - serde_json::from_str(&serde_json::to_string(&denial).unwrap()).unwrap(); - assert_eq!(denial, parsed); - - let summary = DenialFrame::from(DenialSummary::new(2, 5, true)); - let parsed: DenialFrame = - serde_json::from_str(&serde_json::to_string(&summary).unwrap()).unwrap(); - assert_eq!(summary, parsed); - } -} diff --git a/src/core/learning_mode_core/src/lib.rs b/src/core/learning_mode_core/src/lib.rs index bde6a79e..ccfee6bd 100644 --- a/src/core/learning_mode_core/src/lib.rs +++ b/src/core/learning_mode_core/src/lib.rs @@ -12,15 +12,14 @@ //! [`DeniedResource`] records. The per-OS decoder implements //! [`DenialAnalyzer`]; this crate owns the trait and the model. //! 3. **Emit** — the records plus a terminating [`DenialSummary`] are -//! written to an RFC 7464 JSON text sequence that host applications read to +//! written to a single JSON output file that host applications read to //! regenerate their sandbox policy. See [`emit`]. //! //! This crate is the cross-platform hinge between stages 2 and 3: it //! defines the public [`DeniedResource`] model, the [`DenialSummary`] -//! terminator, the self-describing [`DenialFrame`] wire records, the -//! RFC 7464 JSON text sequence [`emit`]ter, and the [`DenialAnalyzer`] decode trait. -//! It carries no OS-specific code so the wire format never encodes a -//! platform assumption. +//! terminator, the [`DenialsDocument`] output shape plus its [`emit`]ter, +//! and the [`DenialAnalyzer`] decode trait. It carries no OS-specific code +//! so the wire format never encodes a platform assumption. //! //! ## Mode caveat //! @@ -34,12 +33,10 @@ pub mod analyze; pub mod emit; -pub mod frame; pub mod model; pub mod summary; pub use analyze::{AnalysisResult, AnalyzeError, DenialAnalyzer}; -pub use emit::{write_frame, write_stream, RECORD_SEPARATOR}; -pub use frame::DenialFrame; +pub use emit::{write_document, DenialsDocument, DenialsOutputPointer}; pub use model::{AccessType, DedupKey, DeniedResource, ResourceType}; pub use summary::DenialSummary; diff --git a/src/core/learning_mode_core/src/model.rs b/src/core/learning_mode_core/src/model.rs index a5b8f793..a04b4aa9 100644 --- a/src/core/learning_mode_core/src/model.rs +++ b/src/core/learning_mode_core/src/model.rs @@ -6,8 +6,8 @@ //! [`DeniedResource`] is the shape every backend decoder emits, every //! transport carries, and every SDK consumer parses. New OS backends //! produce it from their native sources (Windows ETW today; Linux/macOS -//! later); the JSON text sequence (see [`crate::emit`]) is a framed -//! stream of these records plus a trailing [`crate::summary::DenialSummary`]. +//! later); the JSON output file (see [`crate::emit`]) is just an array of +//! these records plus a trailing [`crate::summary::DenialSummary`]. //! //! The types stay tiny and cross-platform so the wire format never //! accidentally encodes a Windows-only assumption. The Windows ETL @@ -75,10 +75,10 @@ pub enum AccessType { Unknown, } -/// One denied `(path, accessType)` observation surfaced to consumers. +/// One denied `(resource, accessType)` observation surfaced to consumers. /// /// A `DeniedResource` describes a single resource the sandboxed workload -/// was denied access to. Per-`(path, accessType)` de-duplication happens +/// was denied access to. Per-`(resource, accessType)` de-duplication happens /// in the decoder, so consumers can treat the emitted stream as already /// unique. /// @@ -88,7 +88,7 @@ pub enum AccessType { /// use learning_mode_core::{AccessType, DeniedResource, ResourceType}; /// /// let denial = DeniedResource { -/// path: r"C:\Users\test\secret.txt".to_string(), +/// resource: r"C:\Users\test\secret.txt".to_string(), /// resource_type: ResourceType::File, /// access_type: AccessType::Read, /// pid: 1234, @@ -101,11 +101,19 @@ pub enum AccessType { #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct DeniedResource { - /// User-visible canonicalised resource identifier. On Windows this is - /// drive-letter form (`C:\Users\...`) with NT-device-namespace - /// prefixes (`\??\`, `\Device\HarddiskVolumeN\`) already stripped by - /// the decoder. Network endpoints (when implemented) use `host:port`. - pub path: String, + /// User-visible identifier for the denied resource, interpreted per + /// [`resource_type`](Self::resource_type): + /// - [`File`](ResourceType::File): canonicalised drive-letter path + /// (`C:\Users\...`) with NT-device-namespace prefixes (`\??\`, + /// `\Device\HarddiskVolumeN\`) already stripped by the decoder. + /// - [`Capability`](ResourceType::Capability): the AppContainer + /// capability name (e.g. `internetClient`), resolved from the + /// capability SID; unresolved custom capabilities fall back to the + /// `S-1-15-3-…` SID string. + /// - [`Network`](ResourceType::Network) (when implemented): `host:port`. + /// - [`Ui`](ResourceType::Ui) / [`Other`](ResourceType::Other): the raw + /// resource identifier the source event carried (may be empty). + pub resource: String, /// Type of resource (see [`ResourceType`]). pub resource_type: ResourceType, @@ -126,7 +134,7 @@ pub struct DeniedResource { pub filetime: u64, } -/// De-duplication key for a denial: the `(path, accessType)` pair. +/// De-duplication key for a denial: the `(resource, accessType)` pair. /// /// Decoders collapse the many raw kernel access-check events a workload /// generates (locale code re-reading the same key on every `printf`, @@ -134,12 +142,12 @@ pub struct DeniedResource { pub type DedupKey = (String, AccessType); impl DeniedResource { - /// Returns the `(path, accessType)` de-duplication key for this - /// denial. Cloning the path is intentional so the key can outlive a + /// Returns the `(resource, accessType)` de-duplication key for this + /// denial. Cloning the resource is intentional so the key can outlive a /// borrow of `self` while a decoder accumulates into a set. #[must_use] pub fn dedup_key(&self) -> DedupKey { - (self.path.clone(), self.access_type) + (self.resource.clone(), self.access_type) } } @@ -153,13 +161,14 @@ mod tests { // camelCase keys and lowercased enum strings. A future serde // rename would silently break every downstream parser. let r = DeniedResource { - path: r"C:\Users\test\file.txt".to_string(), + resource: r"C:\Users\test\file.txt".to_string(), resource_type: ResourceType::File, access_type: AccessType::Read, pid: 1234, filetime: 132_847_890_123_456_789, }; let json = serde_json::to_string(&r).unwrap(); + assert!(json.contains("\"resource\":\"C:"), "got {json}"); assert!(json.contains("\"resourceType\":\"file\""), "got {json}"); assert!(json.contains("\"accessType\":\"read\""), "got {json}"); assert!( @@ -171,7 +180,7 @@ mod tests { #[test] fn denied_resource_round_trips_through_json() { let r = DeniedResource { - path: r"C:\foo\bar.txt".to_string(), + resource: r"C:\foo\bar.txt".to_string(), resource_type: ResourceType::Capability, access_type: AccessType::Write, pid: 9999, @@ -219,7 +228,7 @@ mod tests { #[test] fn dedup_key_pairs_path_and_access_type() { let r = DeniedResource { - path: r"C:\a".to_string(), + resource: r"C:\a".to_string(), resource_type: ResourceType::File, access_type: AccessType::Read, pid: 1, diff --git a/src/core/learning_mode_core/src/summary.rs b/src/core/learning_mode_core/src/summary.rs index a7401369..26cb0cca 100644 --- a/src/core/learning_mode_core/src/summary.rs +++ b/src/core/learning_mode_core/src/summary.rs @@ -1,28 +1,28 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -//! Terminating summary frame for a captureDenials output stream. +//! Terminating summary for a captureDenials output document. //! -//! Exactly one [`DenialSummary`] is written after the last -//! [`crate::model::DeniedResource`] in an RFC 7464 JSON text sequence. It gives +//! Exactly one [`DenialSummary`] accompanies the +//! [`crate::model::DeniedResource`] array in a JSON output file. It gives //! consumers the child's exit code, the count of unique denials, and a //! flag indicating whether the decoder had to truncate the set (so a UX //! can tell the user "showing N of many"). use serde::{Deserialize, Serialize}; -/// The final frame in a captureDenials output stream. +/// The terminating summary of a captureDenials output document. /// -/// `total_denials` is the number of *unique* `(path, accessType)` pairs, -/// which matches the number of `denial` frames that preceded this one. +/// `total_denials` is the number of *unique* `(resource, accessType)` pairs, +/// which matches the length of the document's `denials` array. #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct DenialSummary { /// Exit code of the sandboxed child process. pub exit_code: i32, - /// Count of unique `(path, accessType)` denials emitted (equals the - /// number of `denial` frames preceding this summary). + /// Count of unique `(resource, accessType)` denials emitted (equals the + /// length of the document's `denials` array). pub total_denials: usize, /// `true` when the decoder capped the emitted set and additional diff --git a/src/core/wxc_common/src/config_parser.rs b/src/core/wxc_common/src/config_parser.rs index 8f26bbef..6590ac11 100644 --- a/src/core/wxc_common/src/config_parser.rs +++ b/src/core/wxc_common/src/config_parser.rs @@ -647,9 +647,9 @@ fn map_wire_containment(c: Option<&wire::Containment>) -> ContainmentBackend { /// Validates a caller-specified `processContainer.captureDenials.outputPath`: it /// must be an absolute path whose parent directory already exists (the runner -/// opens the file there when it seals the trace, under the caller's own -/// identity). The path itself must not be an existing directory. A relative -/// path, directory path, or missing parent yields an actionable error. +/// writes the JSON denials output file there after the workload exits). The +/// path itself must not be an existing directory. A relative path, directory +/// path, or missing parent yields an actionable error. fn validate_capture_denials_output_path(path: &str, logger: &mut Logger) -> Result<(), WxcError> { let candidate = std::path::Path::new(path); if !candidate.is_absolute() { @@ -2586,7 +2586,7 @@ mod tests { #[test] fn capture_denials_accepts_valid_absolute_output_path() { let dir = tempfile::tempdir().expect("temp dir"); - let path = dir.path().join("denials.etl"); + let path = dir.path().join("denials.json"); let path_json = serde_json::to_string(&path.to_string_lossy()).unwrap(); let json = format!( r#"{{ @@ -2610,7 +2610,7 @@ mod tests { let json = r#"{ "process": {"commandLine": "print('test')"}, "containment": "processcontainer", - "processContainer": {"captureDenials": {"outputPath": "relative/denials.etl"}} + "processContainer": {"captureDenials": {"outputPath": "relative/denials.json"}} }"#; let encoded = base64_encode(json.as_bytes()); let mut logger = test_logger(); @@ -2626,7 +2626,7 @@ mod tests { fn capture_denials_missing_parent_dir_rejected() { let dir = tempfile::tempdir().expect("temp dir"); // Parent directory `nonexistent` is never created. - let path = dir.path().join("nonexistent").join("denials.etl"); + let path = dir.path().join("nonexistent").join("denials.json"); let path_json = serde_json::to_string(&path.to_string_lossy()).unwrap(); let json = format!( r#"{{ diff --git a/src/core/wxc_common/src/models.rs b/src/core/wxc_common/src/models.rs index 41ecbee6..3451c253 100644 --- a/src/core/wxc_common/src/models.rs +++ b/src/core/wxc_common/src/models.rs @@ -573,8 +573,14 @@ pub struct CaptureDenialsConfig { /// How each ungranted access check is handled while it is recorded. /// Defaults to [`CaptureDenialsMode::Block`]. pub mode: CaptureDenialsMode, - /// Absolute path where the denial ETL trace is written. When `None`, the - /// runner falls back to a managed per-run temporary file. + /// Absolute path where the JSON denials output file is written — the + /// deliverable a consuming application reads. The runner inserts a per-run + /// identifier into the file stem (`denials.json` -> + /// `denials..json`) so concurrent and sequential captures don't + /// collide, and reports the actual path on stderr. When `None`, the runner + /// falls back to a managed per-run temporary file and prints its path on + /// stderr. (The intermediate ETL trace is an internal runner temp that is + /// decoded then deleted.) pub output_path: Option, } diff --git a/src/core/wxc_common/src/wire.rs b/src/core/wxc_common/src/wire.rs index cdce4b0d..a11ab20d 100644 --- a/src/core/wxc_common/src/wire.rs +++ b/src/core/wxc_common/src/wire.rs @@ -225,10 +225,17 @@ pub struct CaptureDenials { /// mode only decides whether that access is blocked or allowed. Defaults to /// `block` when omitted. pub mode: Option, - /// Absolute path where the denial ETL trace is written. The caller names - /// the path; the OS opens it under the caller's own identity when the trace - /// is sealed. When omitted, MXC writes the trace to a managed per-run - /// temporary file. The parent directory must already exist. + /// Absolute path where the JSON denials output file is written — the + /// deliverable a consuming application reads to learn what the workload + /// was denied. It is a single JSON document `{ "denials": [...], + /// "summary": {...} }`. A per-run identifier (process id plus random + /// suffix) is inserted into the file stem (e.g. `denials.json` -> + /// `denials..json`) so concurrent and sequential captures do not + /// collide; the actual path is reported on stderr. When omitted, MXC + /// writes it to a managed per-run temporary file and prints its path on + /// stderr. The parent directory must already exist. (The intermediate ETL + /// trace is an internal, runner-managed temp file that is decoded then + /// deleted.) pub output_path: Option, } diff --git a/src/testing/wxc_e2e_tests/tests/e2e_windows.rs b/src/testing/wxc_e2e_tests/tests/e2e_windows.rs index ad135328..b388fd36 100644 --- a/src/testing/wxc_e2e_tests/tests/e2e_windows.rs +++ b/src/testing/wxc_e2e_tests/tests/e2e_windows.rs @@ -272,6 +272,129 @@ fn processcontainer_proxy() { assert_success(&result); } +/// Drives `processContainer.captureDenials` end to end and asserts the +/// output-file contract: after the child exits, MXC writes a single JSON +/// denials document (`{ "denials": [...], "summary": {...} }`) to the +/// caller-named `outputPath`, and prints a one-line structured pointer +/// (`{"type":"captureDenials",...}`) to stderr. +/// +/// Skips cleanly on hosts without the brokered learning-mode trace API (any +/// unsupported build), where capture surfaces a `BackendUnavailable` +/// error instead of running. +fn processcontainer_capture_denials_output_file() { + let output_path = std::env::temp_dir().join(format!( + "mxc_e2e_denials_{}.json", + SystemTime::now() + .duration_since(UNIX_EPOCH) + .map(|d| d.as_nanos()) + .unwrap_or(0) + )); + let _ = std::fs::remove_file(&output_path); + + let config = serde_json::json!({ + "process": { "commandLine": "cmd.exe /c echo capture-denials-e2e", "timeout": 30000 }, + "containment": "processcontainer", + "processContainer": { + "captureDenials": { + "mode": "block", + "outputPath": output_path.to_string_lossy(), + } + } + }); + + let result = run_wxc_config_value( + "processcontainer_capture_denials_output_file", + &config, + &["--debug"], + ); + let combined = result.combined_output_with_decoded_base64(); + + // Hosts without the brokered learning-mode API can't run capture at all. + if combined.contains("learning-mode trace API is not available") { + println!( + "SKIPPED: processcontainer_capture_denials_output_file requires the brokered \ + learning-mode trace API" + ); + let _ = std::fs::remove_file(&output_path); + return; + } + if result.is_missing_process_prerequisite() { + println!( + "SKIPPED: processcontainer_capture_denials_output_file requires local sandbox \ + runtime prerequisites not available here" + ); + let _ = std::fs::remove_file(&output_path); + return; + } + + assert_success(&result); + + // The runner prints exactly one structured pointer line on stderr. Parse + // it: the actual deliverable path has a per-run identifier stamped into + // the stem, so we must read the path the pointer reports, not the one we + // requested. + let pointer_line = result + .stderr + .lines() + .find(|l| l.contains(r#""type":"captureDenials""#)) + .unwrap_or_else(|| { + panic!( + "stderr should carry the captureDenials pointer line\n--- stderr ---\n{}", + result.stderr + ) + }); + let pointer: serde_json::Value = + serde_json::from_str(pointer_line.trim()).expect("pointer line should be valid JSON"); + let emitted_path = pointer + .get("outputPath") + .and_then(|v| v.as_str()) + .expect("pointer must carry outputPath"); + + // The emitted path must differ from the requested one by the injected + // per-run identifier, so concurrent and sequential captures don't clash. + let requested_stem = output_path + .file_stem() + .and_then(|s| s.to_str()) + .expect("requested stem"); + assert!( + emitted_path.contains(requested_stem) && emitted_path != output_path.to_string_lossy(), + "emitted outputPath {emitted_path} should be the requested path with a run id stamped in" + ); + + // The deliverable is a single JSON document with denials + summary. + let bytes = std::fs::read(emitted_path) + .unwrap_or_else(|e| panic!("captureDenials output file {emitted_path} should exist: {e}")); + let doc: serde_json::Value = + serde_json::from_slice(&bytes).expect("captureDenials output file should be valid JSON"); + let denials = doc + .get("denials") + .and_then(|d| d.as_array()) + .unwrap_or_else(|| panic!("output document must have a `denials` array: {doc}")); + // Every denial identifies its resource under the `resource` key (file path + // or capability name); `path` is retired. + for denial in denials { + assert!( + denial.get("resource").is_some(), + "each denial must carry a `resource` key: {denial}" + ); + assert!( + denial.get("path").is_none(), + "denials must not use the retired `path` key: {denial}" + ); + } + let summary = doc + .get("summary") + .expect("output document must have a `summary`"); + assert!( + summary.get("totalDenials").is_some() + && summary.get("exitCode").is_some() + && summary.get("deniedResourcesTruncated").is_some(), + "summary must carry exitCode/totalDenials/deniedResourcesTruncated: {summary}" + ); + + let _ = std::fs::remove_file(emitted_path); +} + // --------------------------------------------------------------------------- // Tests // --------------------------------------------------------------------------- @@ -296,6 +419,15 @@ fn test_processcontainer_lpac() { with_test_lock(processcontainer_lpac); } +#[test] +#[ignore] // Live capture needs the brokered learning-mode API + BFS velocity key; skips otherwise +fn test_processcontainer_capture_denials_output_file() { + if !cached_has_wxc_exe() { + return; + } + with_test_lock(processcontainer_capture_denials_output_file); +} + #[test] #[ignore] // Requires velocity key 61714527 (BFS deadlock fix) enabled fn test_filesystem_bfs() { diff --git a/tests/examples/29_capture_denials.json b/tests/examples/29_capture_denials.json index fad80eac..3417c2a5 100644 --- a/tests/examples/29_capture_denials.json +++ b/tests/examples/29_capture_denials.json @@ -3,7 +3,7 @@ "containerId": "CLI-CaptureDenials", "containment": "processcontainer", "process": { - "commandLine": "python -c \"print('Capture-denials demo: ungranted access attempts are recorded to an ETL trace')\"" + "commandLine": "python -c \"print('Capture-denials demo: ungranted access attempts are recorded to a JSON denials file')\"" }, "processContainer": { "capabilities": ["internetClient"], From ecee5d63467ccdb68577c205c29eb72280d93268 Mon Sep 17 00:00:00 2001 From: "Richie Gomez (he/him)" Date: Wed, 29 Jul 2026 11:19:47 -0700 Subject: [PATCH 2/5] Address PR 701 review feedback Stream denial JSON directly, remove incomplete output files after write failures, prevent output collisions, and correct the capture mode and generated filename documentation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: dfea4d42-e8a5-42ef-978a-6434e8bf89ec --- docs/learning-mode/capabilities.md | 8 +- .../common/src/base_container_runner.rs | 82 ++++++++++++++++--- src/core/learning_mode_core/src/emit.rs | 43 +++++++++- 3 files changed, 115 insertions(+), 18 deletions(-) diff --git a/docs/learning-mode/capabilities.md b/docs/learning-mode/capabilities.md index 68de92d3..affa2023 100644 --- a/docs/learning-mode/capabilities.md +++ b/docs/learning-mode/capabilities.md @@ -163,8 +163,10 @@ regenerate its sandbox policy: resolved to their policy name; custom (hashed) capability SIDs that can't be reversed fall back to the `S-1-15-3-…` SID string. - `resourceType` is one of `file`, `ui`, `network`, `capability`, `other`; - `accessType` is one of `read`, `write`, `execute`, `unknown`. (Capability - denials are only recorded under `allow` today — see the mode caveat.) + `accessType` is one of `read`, `write`, `execute`, `unknown`. Capability + denials are recorded under `block`; current `allow` traces expose capability + checks as empty-`ObjectType` access events that are omitted because they do + not carry a stable capability identifier. **Locating the file.** Set `captureDenials.outputPath` to name the file explicitly (its parent directory must already exist). MXC inserts a unique @@ -176,7 +178,7 @@ omitted, MXC writes a managed per-run temp file. Either way, `wxc-exec` prints path — so the caller can locate the deliverable without scanning the filesystem: ```json -{"type":"captureDenials","outputPath":"C:\\logs\\denials.4321.json","exitCode":0,"totalDenials":2,"deniedResourcesTruncated":false} +{"type":"captureDenials","outputPath":"C:\\logs\\denials.4321_0123456789abcdef0123456789abcdef.json","exitCode":0,"totalDenials":2,"deniedResourcesTruncated":false} ``` The pointer echoes the file's `summary`; the authoritative record is the file diff --git a/src/backends/appcontainer/common/src/base_container_runner.rs b/src/backends/appcontainer/common/src/base_container_runner.rs index f67d5431..7d307eaf 100644 --- a/src/backends/appcontainer/common/src/base_container_runner.rs +++ b/src/backends/appcontainer/common/src/base_container_runner.rs @@ -1692,19 +1692,7 @@ impl BaseContainerSandboxProcess { ); let document = DenialsDocument::new(analysis.denials, summary); - let file = std::fs::File::create(output_path).map_err(|error| { - std::io::Error::other(format!( - "captureDenials failed to create denials output file {}: {error}", - output_path.display() - )) - })?; - let mut writer = std::io::BufWriter::new(file); - write_document(&mut writer, &document).map_err(|error| { - std::io::Error::other(format!( - "captureDenials failed to write denials output file {}: {error}", - output_path.display() - )) - })?; + write_denials_output_file(output_path, |writer| write_document(writer, &document))?; // Surface a single structured pointer line on our own stderr so a // caller can locate the deliverable (the authoritative summary lives in @@ -1715,6 +1703,45 @@ impl BaseContainerSandboxProcess { } } +fn write_denials_output_file( + output_path: &Path, + write: impl FnOnce(&mut std::io::BufWriter) -> std::io::Result<()>, +) -> std::io::Result<()> { + let file = std::fs::OpenOptions::new() + .write(true) + .create_new(true) + .open(output_path) + .map_err(|error| { + std::io::Error::other(format!( + "captureDenials failed to create denials output file {}: {error}", + output_path.display() + )) + })?; + + let write_result = { + let mut writer = std::io::BufWriter::new(file); + write(&mut writer) + }; + if let Err(error) = write_result { + let write_error = std::io::Error::other(format!( + "captureDenials failed to write denials output file {}: {error}", + output_path.display() + )); + return match std::fs::remove_file(output_path) { + Ok(()) => Err(write_error), + Err(cleanup_error) if cleanup_error.kind() == std::io::ErrorKind::NotFound => { + Err(write_error) + } + Err(cleanup_error) => Err(std::io::Error::other(format!( + "{write_error}; additionally failed to remove incomplete output file {}: {cleanup_error}", + output_path.display() + ))), + }; + } + + Ok(()) +} + /// Inserts a per-run identifier into a denials output path's file stem so /// concurrent and sequential captures using the same configured `outputPath` /// produce distinct files instead of clobbering one another. @@ -1957,6 +1984,35 @@ mod tests { assert_eq!(first.extension().and_then(|ext| ext.to_str()), Some("json")); } + #[test] + fn failed_denials_write_removes_incomplete_output() { + let directory = tempfile::tempdir().expect("temp directory"); + let output_path = directory.path().join("denials.json"); + + let error = write_denials_output_file(&output_path, |writer| { + std::io::Write::write_all(writer, b"{\"partial\":")?; + Err(std::io::Error::other("simulated write failure")) + }) + .expect_err("write should fail"); + + assert!(error.to_string().contains("simulated write failure")); + assert!(!output_path.exists()); + } + + #[test] + fn denials_output_does_not_overwrite_an_existing_file() { + let directory = tempfile::tempdir().expect("temp directory"); + let output_path = directory.path().join("denials.json"); + std::fs::write(&output_path, b"existing").expect("seed output"); + + write_denials_output_file(&output_path, |_| Ok(())).expect_err("collision should fail"); + + assert_eq!( + std::fs::read(&output_path).expect("read existing output"), + b"existing" + ); + } + #[test] fn successful_process_reports_capture_teardown_failure() { let error = diff --git a/src/core/learning_mode_core/src/emit.rs b/src/core/learning_mode_core/src/emit.rs index b7656835..76aeefda 100644 --- a/src/core/learning_mode_core/src/emit.rs +++ b/src/core/learning_mode_core/src/emit.rs @@ -69,8 +69,7 @@ impl DenialsDocument { /// Returns any [`io::Error`] from the underlying writer, or a /// serialisation error surfaced as [`io::ErrorKind::Other`]. pub fn write_document(writer: &mut W, document: &DenialsDocument) -> io::Result<()> { - let json = serde_json::to_vec_pretty(document).map_err(io::Error::other)?; - writer.write_all(&json)?; + serde_json::to_writer_pretty(&mut *writer, document).map_err(io::Error::other)?; writer.write_all(b"\n")?; writer.flush() } @@ -136,6 +135,29 @@ mod tests { use super::*; use crate::model::{AccessType, DeniedResource, ResourceType}; + struct BoundedWriteSize { + bytes: Vec, + max_write_size: usize, + } + + impl Write for BoundedWriteSize { + fn write(&mut self, buf: &[u8]) -> io::Result { + if buf.len() > self.max_write_size { + return Err(io::Error::other(format!( + "write of {} bytes exceeded limit {}", + buf.len(), + self.max_write_size + ))); + } + self.bytes.extend_from_slice(buf); + Ok(buf.len()) + } + + fn flush(&mut self) -> io::Result<()> { + Ok(()) + } + } + fn sample(resource: &str) -> DeniedResource { DeniedResource { resource: resource.to_string(), @@ -174,6 +196,23 @@ mod tests { assert_eq!(doc, parsed); } + #[test] + fn write_document_streams_without_a_document_sized_write() { + let denials = (0..1000) + .map(|index| sample(&format!(r"C:\{index}"))) + .collect::>(); + let doc = DenialsDocument::new(denials, DenialSummary::new(0, 1000, false)); + let mut writer = BoundedWriteSize { + bytes: Vec::new(), + max_write_size: 128, + }; + + write_document(&mut writer, &doc).unwrap(); + + let parsed: DenialsDocument = serde_json::from_slice(&writer.bytes).unwrap(); + assert_eq!(parsed.denials.len(), 1000); + } + #[test] fn empty_capture_still_writes_document_with_summary() { let doc = DenialsDocument::new(vec![], DenialSummary::new(0, 0, false)); From 9cc483d79fd763337ae89e7bbeab7d6317725181 Mon Sep 17 00:00:00 2001 From: "Richie Gomez (he/him)" Date: Wed, 29 Jul 2026 14:55:00 -0700 Subject: [PATCH 3/5] Harden denial output finalization Preserve polling exit codes, propagate internal ETL cleanup failures, and make pointer plus secondary teardown diagnostics safe when stderr is unavailable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: dfea4d42-e8a5-42ef-978a-6434e8bf89ec --- docs/learning-mode/capabilities.md | 6 +- .../common/src/base_container_runner.rs | 113 ++++++++++++++++-- src/core/learning_mode_core/src/emit.rs | 2 +- src/core/learning_mode_core/src/model.rs | 2 +- 4 files changed, 108 insertions(+), 15 deletions(-) diff --git a/docs/learning-mode/capabilities.md b/docs/learning-mode/capabilities.md index affa2023..b1473a20 100644 --- a/docs/learning-mode/capabilities.md +++ b/docs/learning-mode/capabilities.md @@ -136,14 +136,14 @@ regenerate its sandbox policy: "resourceType": "file", "accessType": "read", "pid": 1234, - "filetime": 132847890123456789 + "filetime": "132847890123456789" }, { "resource": "internetClient", "resourceType": "capability", "accessType": "unknown", "pid": 1234, - "filetime": 132847890123512345 + "filetime": "132847890123512345" } ], "summary": { @@ -167,6 +167,8 @@ regenerate its sandbox policy: denials are recorded under `block`; current `allow` traces expose capability checks as empty-`ObjectType` access events that are omitted because they do not carry a stable capability identifier. +- `filetime` is a decimal string containing the Windows `FILETIME` value, so + JavaScript consumers retain all 64 bits without numeric precision loss. **Locating the file.** Set `captureDenials.outputPath` to name the file explicitly (its parent directory must already exist). MXC inserts a unique diff --git a/src/backends/appcontainer/common/src/base_container_runner.rs b/src/backends/appcontainer/common/src/base_container_runner.rs index 7d307eaf..ca18de82 100644 --- a/src/backends/appcontainer/common/src/base_container_runner.rs +++ b/src/backends/appcontainer/common/src/base_container_runner.rs @@ -1630,11 +1630,11 @@ impl BaseContainerSandboxProcess { "captureDenials failed to seal the denial trace: {error}" ))), }; - // The internal ETL temp is never handed to callers. - if let Some(etl) = &etl_path { - let _ = std::fs::remove_file(etl); - } - result + let cleanup_result = etl_path + .as_deref() + .map(remove_internal_capture_file) + .unwrap_or(Ok(())); + combine_capture_and_cleanup_results(result, cleanup_result) } else { Ok(()) }; @@ -1698,11 +1698,52 @@ impl BaseContainerSandboxProcess { // caller can locate the deliverable (the authoritative summary lives in // the file). let pointer = DenialsOutputPointer::new(output_path.to_string_lossy(), &document.summary); - eprintln!("{}", pointer.to_line()); - Ok(()) + let stderr = std::io::stderr(); + let mut stderr = stderr.lock(); + write_denials_output_pointer(&mut stderr, &pointer) + } +} + +fn remove_internal_capture_file(path: &Path) -> std::io::Result<()> { + match std::fs::remove_file(path) { + Ok(()) => Ok(()), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(()), + Err(error) => Err(std::io::Error::other(format!( + "captureDenials failed to remove internal ETL file {}: {error}", + path.display() + ))), } } +fn combine_capture_and_cleanup_results( + capture_result: std::io::Result<()>, + cleanup_result: std::io::Result<()>, +) -> std::io::Result<()> { + match (capture_result, cleanup_result) { + (Ok(()), Ok(())) => Ok(()), + (Err(capture_error), Ok(())) => Err(capture_error), + (Ok(()), Err(cleanup_error)) => Err(cleanup_error), + (Err(capture_error), Err(cleanup_error)) => Err(std::io::Error::other(format!( + "{capture_error}; additionally failed to clean up the internal ETL: {cleanup_error}" + ))), + } +} + +fn write_denials_output_pointer( + writer: &mut impl std::io::Write, + pointer: &DenialsOutputPointer, +) -> std::io::Result<()> { + writeln!(writer, "{}", pointer.to_line())?; + writer.flush() +} + +fn write_stderr_line_best_effort(message: std::fmt::Arguments<'_>) { + let stderr = std::io::stderr(); + let mut stderr = stderr.lock(); + let _ = std::io::Write::write_fmt(&mut stderr, format_args!("{message}\n")); + let _ = std::io::Write::flush(&mut stderr); +} + fn write_denials_output_file( output_path: &Path, write: impl FnOnce(&mut std::io::BufWriter) -> std::io::Result<()>, @@ -1800,12 +1841,14 @@ impl SandboxProcess for BaseContainerSandboxProcess { if self.capture_session.is_none() && self.teardown_result.is_none() { return Ok(Some(code as i32)); } + let exit_code = code as i32; + self.last_exit_code = Some(exit_code); // A terminal observation must not become visible before // captureDenials is sealed. This is the path used by polling // SDK waits, so finalize synchronously before reporting exit. self.terminate_and_reap(); let teardown_result = self.run_teardown(); - combine_process_and_teardown_results(Ok(code as i32), teardown_result).map(Some) + combine_process_and_teardown_results(Ok(exit_code), teardown_result).map(Some) } WAIT_TIMEOUT => Ok(None), _ => Err(std::io::Error::other("WaitForSingleObject failed")), @@ -1874,7 +1917,9 @@ impl Drop for BaseContainerSandboxProcess { // leak as an orphan). self.terminate_and_reap(); if let Err(error) = self.run_teardown() { - eprintln!("captureDenials teardown failed during drop: {error}"); + write_stderr_line_best_effort(format_args!( + "captureDenials teardown failed during drop: {error}" + )); } } } @@ -1918,9 +1963,9 @@ fn combine_process_and_teardown_results( (Ok(_), Err(teardown_error)) => Err(teardown_error), (Err(wait_error), Ok(())) => Err(wait_error), (Err(wait_error), Err(teardown_error)) => { - eprintln!( + write_stderr_line_best_effort(format_args!( "captureDenials teardown also failed after process wait failure: {teardown_error}" - ); + )); Err(wait_error) } } @@ -1958,6 +2003,21 @@ mod tests { use wxc_common::models::{ClipboardPolicy, ProxyConfig, UiPolicy}; use wxc_common::ui_policy::EffectiveUiRestrictions; + struct FailingWriter; + + impl std::io::Write for FailingWriter { + fn write(&mut self, _buf: &[u8]) -> std::io::Result { + Err(std::io::Error::new( + std::io::ErrorKind::BrokenPipe, + "simulated stderr failure", + )) + } + + fn flush(&mut self) -> std::io::Result<()> { + Ok(()) + } + } + fn expected_mask(r: EffectiveUiRestrictions) -> u64 { to_job_object_uilimit_mask(&r) as u64 } @@ -2013,6 +2073,37 @@ mod tests { ); } + #[test] + fn missing_internal_etl_is_already_clean() { + let directory = tempfile::tempdir().expect("temp directory"); + let missing = directory.path().join("missing.etl"); + remove_internal_capture_file(&missing).expect("missing file should be clean"); + } + + #[test] + fn capture_and_etl_cleanup_failures_are_both_preserved() { + let error = combine_capture_and_cleanup_results( + Err(std::io::Error::other("decode failed")), + Err(std::io::Error::other("delete failed")), + ) + .expect_err("combined operation should fail"); + + let message = error.to_string(); + assert!(message.contains("decode failed")); + assert!(message.contains("delete failed")); + } + + #[test] + fn pointer_write_failure_is_returned() { + let summary = DenialSummary::new(0, 1, false); + let pointer = DenialsOutputPointer::new("denials.json", &summary); + + let error = write_denials_output_pointer(&mut FailingWriter, &pointer) + .expect_err("pointer write should fail"); + + assert_eq!(error.kind(), std::io::ErrorKind::BrokenPipe); + } + #[test] fn successful_process_reports_capture_teardown_failure() { let error = diff --git a/src/core/learning_mode_core/src/emit.rs b/src/core/learning_mode_core/src/emit.rs index 76aeefda..bf3ebd5f 100644 --- a/src/core/learning_mode_core/src/emit.rs +++ b/src/core/learning_mode_core/src/emit.rs @@ -10,7 +10,7 @@ //! { //! "denials": [ //! { "resource": "C:\\Users\\test\\secret.txt", "resourceType": "file", -//! "accessType": "read", "pid": 1234, "filetime": 132847890123456789 } +//! "accessType": "read", "pid": 1234, "filetime": "132847890123456789" } //! ], //! "summary": { "exitCode": 0, "totalDenials": 1, //! "deniedResourcesTruncated": false } diff --git a/src/core/learning_mode_core/src/model.rs b/src/core/learning_mode_core/src/model.rs index a04b4aa9..26110b27 100644 --- a/src/core/learning_mode_core/src/model.rs +++ b/src/core/learning_mode_core/src/model.rs @@ -194,7 +194,7 @@ mod tests { #[test] fn denied_resource_accepts_legacy_numeric_filetime() { let parsed: DeniedResource = serde_json::from_str( - r#"{"path":"C:\\foo","resourceType":"file","accessType":"read","pid":1,"filetime":42}"#, + r#"{"resource":"C:\\foo","resourceType":"file","accessType":"read","pid":1,"filetime":42}"#, ) .unwrap(); assert_eq!(parsed.filetime, 42); From fb80a04166cf80cb264712ae4200e4b4bec50069 Mon Sep 17 00:00:00 2001 From: "Richie Gomez (he/him)" Date: Wed, 29 Jul 2026 15:52:15 -0700 Subject: [PATCH 4/5] Remove unpublished denial output Delete the randomized JSON deliverable when stderr pointer publication fails, preserving both publication and cleanup errors so callers are not left with undiscoverable files. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: dfea4d42-e8a5-42ef-978a-6434e8bf89ec --- .../common/src/base_container_runner.rs | 37 ++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/src/backends/appcontainer/common/src/base_container_runner.rs b/src/backends/appcontainer/common/src/base_container_runner.rs index ca18de82..08596103 100644 --- a/src/backends/appcontainer/common/src/base_container_runner.rs +++ b/src/backends/appcontainer/common/src/base_container_runner.rs @@ -1700,7 +1700,7 @@ impl BaseContainerSandboxProcess { let pointer = DenialsOutputPointer::new(output_path.to_string_lossy(), &document.summary); let stderr = std::io::stderr(); let mut stderr = stderr.lock(); - write_denials_output_pointer(&mut stderr, &pointer) + publish_denials_output_pointer(output_path, &mut stderr, &pointer) } } @@ -1737,6 +1737,26 @@ fn write_denials_output_pointer( writer.flush() } +fn publish_denials_output_pointer( + output_path: &Path, + writer: &mut impl std::io::Write, + pointer: &DenialsOutputPointer, +) -> std::io::Result<()> { + if let Err(pointer_error) = write_denials_output_pointer(writer, pointer) { + return match std::fs::remove_file(output_path) { + Ok(()) => Err(pointer_error), + Err(cleanup_error) if cleanup_error.kind() == std::io::ErrorKind::NotFound => { + Err(pointer_error) + } + Err(cleanup_error) => Err(std::io::Error::other(format!( + "{pointer_error}; additionally failed to remove unpublished denials output file {}: {cleanup_error}", + output_path.display() + ))), + }; + } + Ok(()) +} + fn write_stderr_line_best_effort(message: std::fmt::Arguments<'_>) { let stderr = std::io::stderr(); let mut stderr = stderr.lock(); @@ -2104,6 +2124,21 @@ mod tests { assert_eq!(error.kind(), std::io::ErrorKind::BrokenPipe); } + #[test] + fn pointer_publication_failure_removes_output_file() { + let directory = tempfile::tempdir().expect("temp directory"); + let output_path = directory.path().join("denials.json"); + std::fs::write(&output_path, b"complete json").expect("seed output"); + let summary = DenialSummary::new(0, 1, false); + let pointer = DenialsOutputPointer::new(output_path.to_string_lossy(), &summary); + + let error = publish_denials_output_pointer(&output_path, &mut FailingWriter, &pointer) + .expect_err("pointer publication should fail"); + + assert_eq!(error.kind(), std::io::ErrorKind::BrokenPipe); + assert!(!output_path.exists()); + } + #[test] fn successful_process_reports_capture_teardown_failure() { let error = From 20b4deae3eb28813b383349a287b5a11f0f22ea8 Mon Sep 17 00:00:00 2001 From: "Richie Gomez (he/him)" Date: Wed, 29 Jul 2026 21:45:52 -0700 Subject: [PATCH 5/5] Preserve canonical path test after restack Update the PR5 resource-field assertion retained while restacking onto the canonical path decoder change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: dfea4d42-e8a5-42ef-978a-6434e8bf89ec --- src/backends/learning_mode/windows/src/etl_decode.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backends/learning_mode/windows/src/etl_decode.rs b/src/backends/learning_mode/windows/src/etl_decode.rs index c5b85eec..f15d8568 100644 --- a/src/backends/learning_mode/windows/src/etl_decode.rs +++ b/src/backends/learning_mode/windows/src/etl_decode.rs @@ -421,7 +421,7 @@ mod tests { let out = dedup_to_resources(denials).denials; assert_eq!(out.len(), 1); - assert_eq!(out[0].path, r"\\server\share\file.txt"); + assert_eq!(out[0].resource, r"\\server\share\file.txt"); } #[test]