Emit captureDenials JSON output - #701
Conversation
856b723 to
ae8672f
Compare
ae8672f to
7665d2e
Compare
There was a problem hiding this comment.
Pull request overview
Adds caller-facing JSON output for captureDenials, converting internal ETL traces into uniquely named denial reports with structured stderr pointers.
Changes:
- Adds JSON document and pointer emission with failure propagation.
- Resolves capability SIDs and updates the denial resource model.
- Adds tests, generated schema changes, and documentation.
Reviewed changes
Copilot reviewed 23 out of 25 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
.github/copilot-instructions.md |
Documents the learning-mode output architecture. |
docs/learning-mode/capabilities.md |
Documents JSON output and pointer behavior. |
docs/schema.md |
Updates the outputPath example. |
schemas/dev/mxc-config.schema.0.8.0-dev.json |
Regenerates the output-path description. |
sdk/node/src/generated/wire.ts |
Regenerates TypeScript wire documentation. |
src/Cargo.lock |
Adds the core learning-mode dependency. |
src/backends/appcontainer/common/Cargo.toml |
Adds learning_mode_core. |
src/backends/appcontainer/common/src/base_container_runner.rs |
Decodes ETL and emits JSON output. |
src/backends/learning_mode/windows/examples/lm_analyze.rs |
Emits the new JSON document format. |
src/backends/learning_mode/windows/src/capability_names.rs |
Adds capability SID resolution. |
src/backends/learning_mode/windows/src/etl_decode.rs |
Produces resource-based denial records. |
src/backends/learning_mode/windows/src/extractors.rs |
Extracts and resolves capability identities. |
src/backends/learning_mode/windows/src/lib.rs |
Registers the capability resolver module. |
src/backends/learning_mode/windows/src/tdh_decode.rs |
Adds binary SID decoding. |
src/core/learning_mode_core/src/analyze.rs |
Updates analyzer terminology. |
src/core/learning_mode_core/src/emit.rs |
Implements document and pointer serialization. |
src/core/learning_mode_core/src/frame.rs |
Removes obsolete framed-record support. |
src/core/learning_mode_core/src/lib.rs |
Exports the new document API. |
src/core/learning_mode_core/src/model.rs |
Renames denial path to resource. |
src/core/learning_mode_core/src/summary.rs |
Updates summary documentation. |
src/core/wxc_common/src/config_parser.rs |
Updates output-path validation terminology. |
src/core/wxc_common/src/models.rs |
Documents the JSON deliverable path. |
src/core/wxc_common/src/wire.rs |
Updates the wire contract documentation. |
src/testing/wxc_e2e_tests/tests/e2e_windows.rs |
Adds ignored live capture E2E coverage. |
tests/examples/29_capture_denials.json |
Updates the capture example text. |
7665d2e to
a098f31
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 25 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
docs/learning-mode/capabilities.md:165
- This reverses the implemented mode behavior. Event 28 capability denials are decoded in
blockmode (extractors.rs:220-229andetl_decode.rs:402-465), while currentallowevents lack a resource identifier and are omitted. Document capability denials as currently surfaced underblock, notallow.
`accessType` is one of `read`, `write`, `execute`, `unknown`. (Capability
denials are only recorded under `allow` today — see the mode caveat.)
docs/learning-mode/capabilities.md:177
- The example path omits the random suffix even though the documented and implemented run ID is
process_id + random_suffix. Show both components so callers do not infer that the actual filename can be predicted from the PID alone.
{"type":"captureDenials","outputPath":"C:\\logs\\denials.4321.json","exitCode":0,"totalDenials":2,"deniedResourcesTruncated":false}
src/backends/learning_mode/windows/src/capability_names.rs:17
- Windows named/custom capability SIDs use the
S-1-15-3-1024-marker followed by eight SHA-256-derived 32-bit RIDs, not four. The four-RID description and the matching synthetic fixture later in this file do not represent an SID produced byDeriveCapabilitySidsFromName; update both to the real shape.
//! - 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`).
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 25 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
src/backends/appcontainer/common/src/base_container_runner.rs:1558
- The raw ETL cleanup error is discarded here. A sharing violation or permission failure therefore leaves the internal trace in the temp directory while
wait()still reports success, contradicting the documented deletion guarantee and potentially accumulating sensitive path data. TreatNotFoundas already cleaned, but merge any other removal failure intocapture_resultas is done for incomplete JSON cleanup.
if let Some(etl) = &etl_path {
let _ = std::fs::remove_file(etl);
src/core/wxc_common/src/wire.rs:238
- The enclosing
processContainer.captureDenialsdescription still says the ETL is retained “for later inspection” (wire.rs:209-211), and that stale text is emitted into both the dev schema and generated TypeScript type. This now contradicts this field documentation and the implementation, which deletes the ETL and exposes JSON. Update the parent-field rustdoc and regenerate both artifacts.
/// 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.<run-id>.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.)
52f0ab1 to
a3fe440
Compare
a3fe440 to
ed97942
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 25 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
src/backends/appcontainer/common/src/base_container_runner.rs:1541
try_wait()also finalizes capture by callingrun_teardown(), but unlikewait()it never populateslast_exit_code. Any SDK caller that polls to completion therefore gets the real code fromtry_wait()while the JSON summary and pointer reportexitCode: -1. Record the observed code before teardown in theWAIT_OBJECT_0branch and add coverage for the polling path.
let exit_code = self.last_exit_code.unwrap_or(-1);
src/backends/appcontainer/common/src/base_container_runner.rs:1623
eprintln!panics when writing stderr fails, so a closed/failed stderr turnswait()into an unwind after the JSON file is created instead of returning an I/O error. Write throughstd::io::Writeand propagate the failure so the caller receives the same error contract as seal/decode/file-write failures.
eprintln!("{}", pointer.to_line());
src/backends/appcontainer/common/src/base_container_runner.rs:1557
- The internal ETL deletion error is discarded, so a sharing/permission failure silently leaves uniquely named trace files in the temp directory while capture reports success. Over time this can leak denial data and consume disk space, contradicting the documented delete-after-decode contract. Treat non-
NotFoundcleanup failures as teardown errors while preserving any primary capture error.
// The internal ETL temp is never handed to callers.
if let Some(etl) = &etl_path {
let _ = std::fs::remove_file(etl);
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 25 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
src/backends/appcontainer/common/src/base_container_runner.rs:1558
- The internal ETL is documented as deleted, but every
remove_fileerror is discarded. A transient lock or permission failure therefore leaves denial traces accumulating in the temp directory whilewait()reports success. Treat cleanup failure as a teardown error (preserving any earlier capture error), or arrange a reported/retried cleanup path.
if let Some(etl) = &etl_path {
let _ = std::fs::remove_file(etl);
}
src/backends/appcontainer/common/src/base_container_runner.rs:1541
try_wait()callsrun_teardown()with the realcodebut never assignslast_exit_code, so successful polling waits emitexitCode: -1in both the JSON document and pointer while returning the actual child code. Setself.last_exit_code = Some(code as i32)in theWAIT_OBJECT_0branch before teardown (and add coverage for the polling path).
let exit_code = self.last_exit_code.unwrap_or(-1);
ed97942 to
e62ca8a
Compare
e62ca8a to
424bff1
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 25 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
src/core/wxc_common/src/models.rs:606
- The updated field documentation now describes the JSON deliverable, but the enclosing
ContainerPolicy::capture_denialsandCaptureDenialsConfigdocumentation still says the runner records attempts to an ETL trace. Update those type-level comments so API documentation consistently distinguishes the internal ETL from the caller-facing JSON file.
/// Protocol: "tcp" or "udp". Default: "tcp".
src/backends/appcontainer/common/src/base_container_runner.rs:1557
- This silently ignores deletion failures for the internal ETL, despite the new contract stating that MXC deletes this potentially sensitive temporary trace. A transient lock or permission error will leave unreported files accumulating in the temp directory; handle the cleanup result explicitly (preserving any primary decode/write error).
let _ = std::fs::remove_file(etl);
src/backends/appcontainer/common/src/base_container_runner.rs:1541
try_wait()reads the real process code and callsrun_teardown()without assigninglast_exit_code, so polling callers receiveexitCode: -1in both the JSON document and pointer even thoughtry_wait()returns the actual code. Assign the code before teardown on that path and add a polling regression test.
let exit_code = self.last_exit_code.unwrap_or(-1);
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 25 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
src/backends/appcontainer/common/src/base_container_runner.rs:1642
- If stderr is closed, this returns an error after the JSON file has already been created at a randomized path. Since this pointer is the caller's only way to discover that path and the raw write error does not include it, the file becomes an unreachable orphan (and repeated failures can leak denial data in the temp directory). On pointer failure, either remove the output file or wrap the error with
output_pathso the caller can recover and clean it.
write_denials_output_pointer(&mut stderr, &pointer)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 25 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
src/backends/learning_mode/windows/src/extractors.rs:252
- Filter each preferred name before selecting it. As written, a present but unusable
CapabilityName/Capability(for example""or<unsupported>) prevents the valid SID branches from running, so this event is dropped even whenPackageSidcan identify the denied capability. Apply the usability check to each candidate beforeor_else, then fall back to the SID fields.
let object_name = find_prop(&parts.props, "CapabilityName")
.or_else(|| find_prop(&parts.props, "Capability"))
.map(|v| v.trim_matches('"').to_string())
.or_else(|| {
find_prop(&parts.props, "PackageSid")
src/core/wxc_common/src/wire.rs:230
- The enclosing
ProcessContainer::capture_denialsdocumentation at lines 209–211 still presents the ETL as the caller-inspected output. That text is also emitted into the generated schema/TypeScript docs, so it now conflicts with this JSON-deliverable contract. Update the parent field documentation to identify the ETL as internal and the JSON document as the caller-facing output.
/// 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": [...],
39bccb8 to
30e91fb
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 25 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
src/core/learning_mode_core/src/emit.rs:83
- The
typetag cannot by itself distinguish this pointer from arbitrary workload output: in the run-to-completion path the child inheritswxc-exec's stderr, so an untrusted command can print the same JSON shape with a forgedoutputPath. The new E2E reader compounds this by selecting the first matching line. Define and enforce an ordering rule (for example, the runner-generated pointer is the final matching JSON line and consumers scan from the end), or publish it over a separate authenticated channel; update the E2E parser and documentation accordingly.
/// 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.
src/backends/learning_mode/windows/src/capability_names.rs:18
- Hash-derived capability SIDs contain the
1024marker followed by eight 32-bit SHA-256 words, not four. The two new custom-SID test fixtures also use the four-word shape, so they do not represent a SID returned byDeriveCapabilitySidsFromName; update this description and both fixtures to the fullS-1-15-3-1024-<8 RIDs>form.
//! - 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
30e91fb to
65b74b2
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 25 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
src/backends/appcontainer/common/src/base_container_runner.rs:1703
- The CLI child runs with
StdioMode::Inherit, so it shares this stderr stream. If the workload exits after writing a stderr fragment without a trailing newline, thiswriteln!appends the JSON directly to that fragment (for example,warning{"type":"captureDenials",...}), and the advertised one-line pointer is no longer valid JSON. Start the control record with a newline (or move it to a separately framed channel) so it always begins on a line boundary.
writeln!(writer, "{}", pointer.to_line())?;
writer.flush()
src/backends/appcontainer/common/src/base_container_runner.rs:1669
- This publishes a trusted control message on the same stderr handle inherited by the untrusted workload. The child can emit an identical
{"type":"captureDenials","outputPath":...}object before it exits; the new E2E parser even selects the first match, so a caller following that pattern can be redirected to an attacker-chosen host path. Use a control channel the sandboxed process cannot write to, or define and enforce final-record semantics after reap (including updating consumers/tests to select the last exact pointer object).
This issue also appears on line 1702 of the same file.
let stderr = std::io::stderr();
let mut stderr = stderr.lock();
publish_denials_output_pointer(output_path, &mut stderr, &pointer)
…pability 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.<pid>.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 <unsupported>.
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>
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
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
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
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
65b74b2 to
20b4dea
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 25 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
src/backends/learning_mode/windows/src/capability_names.rs:18
- The named-capability SID shape is documented incorrectly here:
DeriveCapabilitySidsFromNameuses a1024marker followed by eight 32-bit SHA-256-derived RIDs, not four. Using the real shape avoids teaching future decoder work and fixtures a truncated SID format.
//! - 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
src/backends/learning_mode/windows/src/extractors.rs:258
- Validate each candidate before falling back to the next property. As written, a present but empty/placeholder
CapabilityNameorPackageSidproducesSome(...), prevents the later fallback from running, and is only rejected by the final filter, so a valid SID in a lower-priority field is silently dropped.
let object_name = find_prop(&parts.props, "CapabilityName")
.or_else(|| find_prop(&parts.props, "Capability"))
.map(|v| v.trim_matches('"').to_string())
.or_else(|| {
find_prop(&parts.props, "PackageSid")
📖 Description
Adds the caller-facing denial output layer on top of PR4 #699.
denialsandsummary.captureDenialspointer line on stderr so callers can locate the actual deliverable.wait()instead of reporting a successful sandbox run with missing capture output.resourcefor both filesystem paths and capability identities and preserves de-duplicated denial summaries.This PR is intentionally stacked on #699 and should merge after #696 and #699.
🔗 References
🔍 Validation
cargo test -p learning_mode_corecargo test -p learning_mode_windows --all-targetscargo test -p appcontainer_commoncargo test -p wxc_common capture_denialscargo test -p wxc_e2e_tests --no-runcargo clippy -- -D warningsblock: exit 0, valid JSON document with 3 denialsallow: exit 0, valid JSON document, structured pointer, and permissive security warning✅ Checklist
Cargo.lock, thedependency-feed-checkcheck passes (see docs/pull-requests.md)📋 Issue Type
GitHub Actions runs the PR validation build automatically. The ADO pipeline
(
MXC-PR-Build) is the Azure version of the PR pipeline, kept in parity with the GitHubActions build; it runs on merge to
main, and Microsoft reviewers with write access can trigger iton a PR with
/azp run. See docs/pull-requests.md.If the
dependency-feed-checkcheck fails on a new dependency, the crate must be added tothe feed before the PR can pass. See docs/pull-requests.md
for the steps.
Microsoft Reviewers: Open in CodeFlow