Skip to content

Add verbose Learning Mode denial logging - #916

Open
Richie Gomez (richiemsft) wants to merge 33 commits into
mainfrom
data-loop-decoder
Open

Add verbose Learning Mode denial logging#916
Richie Gomez (richiemsft) wants to merge 33 commits into
mainfrom
data-loop-decoder

Conversation

@richiemsft

@richiemsft Richie Gomez (richiemsft) commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

📖 Description

Adds deterministic verbose logging for every successful Learning Mode denial analysis while preserving the canonical policy-output contract and expanding canonical decoding for newly validated ETL shapes.

Each analysis now produces a transactional pair:

  • denials.json retains canonical, actionable denials, including complete file paths.
  • denials.verbose.json retains a bounded, deduplicated diagnostic superset. Complete file paths are replaced with <REDACTED>; standalone account names and username components in non-file identifiers are also redacted before bounding or hashing.

The verbose logging output includes canonical denial occurrences and closed classifications for outcomes omitted from canonical policy generation, including unusable resource paths, unsupported object types and event schemas, malformed payloads, decoder limits, and unresolved capability events. Stable non-file identifiers such as SIDs, capability names, provider GUIDs, PIDs, and named-object details remain available for diagnosis.

The guarded WPR path now preserves known-provider events through process-scoped selection and replay. Brokered event 28 is scoped using its payload ProcessId, including partial-decode recovery, rather than the broker's ETW header PID.

Native capture and plm stop share a transactional paired-output writer. Both JSON files are staged and synchronized before no-clobber promotion, with ownership- and digest-aware rollback. Audit mode consumes these capture artifacts directly and relocates both the JSON pair and retained ETL through no-clobber staging with identity- and digest-verified source cleanup, keeping metadata truthful without deleting files owned by another process.

Additional ETL findings

Validation across eight new namespace scenarios (16 retained ETLs: block and allow) identified additional canonical decoder shapes:

  • Event 28 is schema-dependent. UI-shaped payloads containing Category and Detail now emit canonical UI denials such as resource: "Handles", resourceType: "ui", rather than incorrectly treating the payload's package SID as a capability.
  • Named Section, SymbolicLink, and Timer access checks now emit canonical other resources with object-specific access-mask classification.
  • The UI Handles scenario represents JOB_OBJECT_UILIMIT_HANDLES, primarily cross-process HWND/USER-object handle access.
  • SCM, ETW provider/session, and power-policy scenario-specific checks still expose empty ObjectType and ObjectName. They remain bounded verbose logging diagnostics because the ETL provides no stable canonical resource identifier.

The updated decoder increased canonical coverage in 15 of the 16 supplied ETLs. The remaining ETW Session block trace was an early process-initialization failure and contained none of the newly supported named shapes.

🔗 References

Builds on the Learning Mode capture hardening from #847.
This is for issue #828

🔍 Validation

  • learning_mode_core: 43 tests passed
  • learning_mode_windows: 224 tests passed
  • appcontainer_common: 247 tests passed
  • plm: 226 tests passed, 1 ignored
  • wxc: 40 tests passed
  • Clippy passed with warnings denied for learning_mode_core, learning_mode_windows, and plm
  • Release wxc-exec.exe and plm.exe build
  • Guarded-WPR VM validation with canonical and typed verbose logging groups, zero overflow, and no username leakage
  • Production lm_analyze decoder run against all 16 supplied retained ETLs
  • Confirmed canonical ui/Handles, Timer, Section, and SymbolicLink output; confirmed event 28 UI no longer emits a bogus package-SID capability
  • Fixture accounting now follows the production handler exactly; out-of-scope events do not consume the processing budget
  • Create-new output pairs no longer create persistent per-run lock files; relocation publishes committed destinations and surfaces source-cleanup warnings
  • Malformed event 28 diagnostics classify UI/capability resources only when the schema name identifies the payload shape
  • GitHub Actions build, lint, integration, CodeQL, CLA, and dependency-feed checks passed before the latest ETL decoder commit; replacement checks run automatically

✅ Checklist

📋 Issue Type

  • Bug fix
  • Feature
  • Task

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 GitHub
Actions build; it runs on merge to main, and Microsoft reviewers with write access can trigger it
on a PR with /azp run. See docs/pull-requests.md.

If the dependency-feed-check check fails on a new dependency, the crate must be added to
the feed before the PR can pass. See docs/pull-requests.md
for the steps.

Microsoft Reviewers: Open in CodeFlow

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

Route wxc-exec --audit through captureDenials so native PSEC/V2 remains preferred and guarded WPR provides compatible analysis and ETL retention. Reuse canonical denials for adjusted policy generation and support retainEtl consistently across both capture providers.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 4eff3ee2-323c-494d-99b9-f7b54e495216
Update repository guidance for captureDenials-backed audit routing and guarded retainEtl parity.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 4eff3ee2-323c-494d-99b9-f7b54e495216
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 4eff3ee2-323c-494d-99b9-f7b54e495216
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 4eff3ee2-323c-494d-99b9-f7b54e495216
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 4eff3ee2-323c-494d-99b9-f7b54e495216
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 4eff3ee2-323c-494d-99b9-f7b54e495216
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 4eff3ee2-323c-494d-99b9-f7b54e495216
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 4eff3ee2-323c-494d-99b9-f7b54e495216
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 4eff3ee2-323c-494d-99b9-f7b54e495216
@richiemsft
Richie Gomez (richiemsft) changed the base branch from unified-audit-capture to main August 17, 2026 19:44
Emit a bounded, username-redacted Data Loop JSON sibling containing canonical denial occurrences and classified decoder outcomes. Keep paired output transactional across native, guarded, and audit capture paths.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 4eff3ee2-323c-494d-99b9-f7b54e495216
Copilot AI balanced review requested due to automatic review settings August 17, 2026 19:54
@microsoft-github-policy-service microsoft-github-policy-service Bot added the Copilot-Instructions PR modifies Copilot instruction files (.github/copilot-instructions.md or .github/instructions/) label Aug 17, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds bounded, redacted Data Loop diagnostics alongside Learning Mode denial output and integrates them across native capture, guarded WPR, and audit workflows.

Changes:

  • Adds the Data Loop model, aggregation, sanitization, and paired output.
  • Adds process-scoped guarded-WPR ETL relogging and retention.
  • Reworks --audit to consume captureDenials artifacts directly.

Reviewed changes

Copilot reviewed 29 out of 30 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
.github/copilot-instructions.md Updates Learning Mode architecture guidance.
README.md Revises audit-mode documentation.
docs/learning-mode/capabilities.md Documents Data Loop and retained ETL behavior.
src/Cargo.lock Records the new dependency edge.
src/backends/appcontainer/common/src/appcontainer_runner.rs Supports guarded ETL transfer.
src/backends/appcontainer/common/src/base_container_runner.rs Integrates retained guarded captures.
src/backends/appcontainer/common/src/capture_output.rs Adds paired denial/Data Loop output.
src/backends/appcontainer/common/src/guarded_capture.rs Extends guarded-capture interfaces.
src/backends/learning_mode/windows/src/capability_dacl.rs Adds diagnostic metadata to recovered denials.
src/backends/learning_mode/windows/src/etl_decode.rs Aggregates Data Loop outcomes and selects relog events.
src/backends/learning_mode/windows/src/etl_filter.rs Adds process-scoped ETL relogging.
src/backends/learning_mode/windows/src/extractors.rs Adds sanitization and diagnostic classification.
src/backends/learning_mode/windows/src/lib.rs Exports ETL filtering.
src/backends/learning_mode/windows/src/tdh_decode.rs Categorizes recoverable decoding failures.
src/core/learning_mode_core/src/analyze.rs Carries and bounds Data Loop results.
src/core/learning_mode_core/src/data_loop.rs Defines the Data Loop model and writer.
src/core/learning_mode_core/src/lib.rs Exports Data Loop APIs.
src/core/learning_mode_core/src/model.rs Makes classifications sortable.
src/core/mxc_engine/src/guarded_capture.rs Implements guarded trace transfer.
src/core/mxc_engine/src/lib.rs Exports audit runner resolution.
src/core/mxc_engine/src/run.rs Adds audit-specific resolver entry point.
src/core/wxc/Cargo.toml Adds Learning Mode core dependency.
src/core/wxc/src/audit.rs Relocates and post-processes capture artifacts.
src/core/wxc/src/main.rs Routes audit through captureDenials.
src/host/plm/readme.md Documents guarded analysis and validation.
src/host/plm/src/analysis.rs Returns canonical documents after writing.
src/host/plm/src/elevated.rs Adds filtered analysis/trace transfer protocol.
src/host/plm/src/log.rs Consumes guardian-returned analysis directly.
src/host/plm/src/secure_scratch.rs Manages filtered ETL scratch storage.
src/host/plm/src/stop.rs Extracts canonical-denial post-processing.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/backends/learning_mode/windows/src/etl_filter.rs Outdated
Comment thread src/host/plm/src/analysis.rs
Comment thread src/backends/learning_mode/windows/src/etl_decode.rs Outdated
Keep excluded namespace evidence uniquely identifiable, retain guarded-trace unknown schemas and brokered capability events, and emit canonical/Data Loop outputs as a transactional pair.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: acb632fc-7b75-4d0e-9be5-f11de51652bf
Use targeted broker PID decoding without aborting on unrelated malformed payloads, and make rollback ownership-aware, portable, and no-clobber.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: acb632fc-7b75-4d0e-9be5-f11de51652bf
Preserve malformed scoped capability events, retain uncached TDH schemas after the cache cap, and make rollback recovery no-clobber and deletion tolerant.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: acb632fc-7b75-4d0e-9be5-f11de51652bf
Detect in-place overwrites before deleting a promoted output and directly exercise TDH short-circuit and cache-capacity behavior.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: acb632fc-7b75-4d0e-9be5-f11de51652bf
Restore backups even when quarantined output cleanup fails, preserve both errors, and hash promoted files from a known offset.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: acb632fc-7b75-4d0e-9be5-f11de51652bf
Recover the brokered payload ProcessId when full event decoding fails after that property, and drop only events that remain unscopable.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: acb632fc-7b75-4d0e-9be5-f11de51652bf
Surface quarantine restoration failures with accurate rollback context and codify malformed capability PID fallback scoping.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: acb632fc-7b75-4d0e-9be5-f11de51652bf
Identify backup, quarantine, and destination paths in cleanup and restoration failures while preserving strict error propagation.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: acb632fc-7b75-4d0e-9be5-f11de51652bf
Copilot AI review requested due to automatic review settings August 18, 2026 02:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 33 out of 34 changed files in this pull request and generated no new comments.

Suppressed comments (4)

src/core/learning_mode_core/src/data_loop.rs:184

  • Once the signature set contains only canonical groups, every new distinct canonical event calls evict_one_noncanonical_group, which scans all 4,096 entries before overflowing it. At the 1,000,000-event bound this can perform billions of comparisons, turning an intentionally bounded diagnostic path into a CPU bottleneck. Track whether any noncanonical group exists (or partition/index the two classes) so the full scan is skipped when eviction cannot succeed.
        while self.signatures.len() >= MAX_DATA_LOOP_GROUPS
            || retained_bytes.saturating_add(serialized_len) > max_bytes
        {
            if !signature.reason.is_canonical_denial()
                || !self.evict_one_noncanonical_group(Some(retained_bytes))
            {

src/host/plm/src/elevated.rs:2341

  • This helper is now also used for guarded captureDenials retention, so create_dir_all makes a missing outputPath parent succeed only when guarded WPR plus retainEtl is selected. Native and non-retained captures still fail, and the documented contract says the parent must already exist. Preflight the configured output directory consistently before capture, rather than creating it as a side effect of ETL transfer.
    src/core/learning_mode_core/src/paired_output.rs:56
  • Every capture creates a unique .pair.lock file, but Drop only unlocks it and never removes it. Because capture output names include a per-run ID, these lock files are never reused and accumulate indefinitely in the configured output directory (or the system temp directory for managed outputs). Use an ephemeral/delete-on-close lock strategy, or add race-safe cleanup after the transaction.
impl Drop for OutputPairLock {
    fn drop(&mut self) {
        let _ = std::fs::File::unlock(&self.file);
    }

.github/copilot-instructions.md:216

  • This architecture entry still says the output layer writes a single JSON document, but this PR makes learning_mode_core own the DataLoopDocument and paired-output transaction as well. Leaving the repository guidance singular will mislead future changes about the required canonical/Data Loop pair; update this entry to describe both artifacts and their paired emission contract.
- `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`) and defines the serializable `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, native PSEC/V2 seals and decodes a managed ETL locally, while guarded WPR relogs its host-wide source into a process-scoped ETL before analysis; both routes write the same canonical JSON through shared output plumbing and return neutral `wxc_common` metadata. Explicit `retainEtl` preserves the native sealed trace or the guarded process-scoped relogged trace after a terminal wait; abandonment discards it. `wxc-exec` serializes the metadata as the one-line stderr pointer at the CLI boundary; Rust/C#/FFI callers receive it programmatically. 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).

Resolve overlapping Learning Mode audit capture changes while preserving Data Loop denial diagnostics.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: f9ac49b1-fdfe-4cca-9412-9faa99cef2ab
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: f9ac49b1-fdfe-4cca-9412-9faa99cef2ab
Copilot AI review requested due to automatic review settings August 18, 2026 21:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 22 out of 23 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/core/learning_mode_core/src/paired_output.rs:289

  • This has the same committed-state ambiguity for the retained ETL: promotion to destination_path succeeds before source cleanup, but a quarantine/delete failure is returned as Err. The audit caller then skips updating etl_path even though the destination exists and the source path may already have been renamed away, contradicting its truthful-metadata guarantee. Roll back the destination/source on cleanup failure or return a structured committed outcome that lets the caller update metadata.
    let promoted = promote_output_file(operation, temp, destination_path, kind)?;
    drop((source, promoted));
    remove_promoted_output_if_owned(source_path, source_identity, None).and_then(cleanup_result)

src/core/learning_mode_core/src/paired_output.rs:118

  • OutputPairLock::acquire creates a lock file, but Drop only unlocks it. Because captureDenials uses a unique canonical filename for every run, every successful capture permanently leaves a new .<run-specific-name>.pair.lock in the output directory. Avoid acquiring a persistent lock for CreateNew writes (the no-clobber promotions already arbitrate competing writers), or use a safely reusable lock path so capture directories do not grow without bound.
    let _pair_lock = OutputPairLock::acquire(operation, canonical_path)?;

src/core/learning_mode_core/src/paired_output.rs:252

  • A source-cleanup error is returned after the destination pair has already committed, and the committed destinations are not rolled back. audit::relocate_artifacts interprets this Err as meaning relocation did not occur and therefore leaves capture.output_path at the source; cleanup may already have removed one source member, so metadata becomes stale and a retry collides with the stranded destination pair. The relocation API needs either to restore the complete source/destination state before returning Err, or report a committed-with-cleanup-error outcome so the caller can publish the destination path.

This issue also appears on line 287 of the same file.

    let data_loop_cleanup =
        remove_promoted_output_if_owned(data_loop_source_path, data_loop_identity, None)
            .and_then(cleanup_result);
    let canonical_cleanup =
        remove_promoted_output_if_owned(canonical_source_path, canonical_identity, None)
            .and_then(cleanup_result);

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: f9ac49b1-fdfe-4cca-9412-9faa99cef2ab
Copilot AI review requested due to automatic review settings August 18, 2026 21:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 22 out of 23 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/core/learning_mode_core/src/paired_output.rs:36

  • The lock file is created for every output name but is never removed; Drop only unlocks it. Because capture outputs include a unique run ID, repeated captures leave an unbounded set of .pair.lock files in the temp or caller-selected output directory. Please use a lock lifecycle that safely removes its artifact, or a reusable lock name/location with bounded cleanup.
        let file = std::fs::OpenOptions::new()
            .read(true)
            .write(true)
            .create(true)
            .truncate(false)
            .open(&lock_path)

src/core/learning_mode_core/src/paired_output.rs:505

  • When the quarantined file matches but deletion fails, this returns a cleanup error without restoring the quarantine to final_path. Both relocation helpers have already committed their destination, so their callers may leave metadata pointing to the now-missing source path. Restore the quarantined file on deletion failure (using the same no-clobber safeguards) before propagating the error.
        Ok(true) => Ok(remove_if_present(&quarantine_path).err().map(|error| {
            std::io::Error::other(format!(
                "failed to remove quarantined promoted output {}: {error}",
                quarantine_path.display()
            ))

src/core/learning_mode_core/src/paired_output.rs:251

  • The destination pair has already committed before these independent source cleanups. If one cleanup succeeds and the other fails, this returns Err with one source removed and both destinations present; audit::relocate_artifacts publishes capture.output_path only on Ok, so metadata can keep pointing at an incomplete or missing source pair. Treat destination commit separately from cleanup warnings, or restore both source members/roll back the destination before returning an error.
    let data_loop_cleanup =
        remove_promoted_output_if_owned(data_loop_source_path, data_loop_identity, None)
            .and_then(cleanup_result);
    let canonical_cleanup =
        remove_promoted_output_if_owned(canonical_source_path, canonical_identity, None)

Comment thread docs/learning-mode/capabilities.md Outdated
@richiemsft Richie Gomez (richiemsft) linked an issue Aug 18, 2026 that may be closed by this pull request
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: f9ac49b1-fdfe-4cca-9412-9faa99cef2ab
Copilot AI review requested due to automatic review settings August 19, 2026 16:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 22 out of 23 changed files in this pull request and generated 1 comment.

Suppressed comments (4)

src/core/learning_mode_core/src/paired_output.rs:251

  • The destination pair has already committed before these independent source cleanups run. If either cleanup fails, the other cleanup can still remove its source and this function returns Err; audit::relocate_artifacts then leaves metadata pointing at the source, which may now be missing one or both files, even though the destination pair exists. Propagate a committed-destination state (or publish destination metadata before best-effort source cleanup) so cleanup errors cannot make metadata stale.
    let data_loop_cleanup =
        remove_promoted_output_if_owned(data_loop_source_path, data_loop_identity, None)
            .and_then(cleanup_result);
    let canonical_cleanup =
        remove_promoted_output_if_owned(canonical_source_path, canonical_identity, None)

src/core/learning_mode_core/src/paired_output.rs:26

  • Capture output names include a unique run ID, but Drop only unlocks this file and never removes it. Every successful capture therefore leaves a distinct .pair.lock behind (including in the system temp directory), causing unbounded lock-file accumulation. Use one reusable directory-scoped lock instead of a per-output lock name.
        let lock_name = format!(".{file_name}.pair.lock");

src/backends/learning_mode/windows/src/extractors.rs:150

  • This branch now emits UI-shaped event 28 records into canonical denials.json (the added test does so even for Denied=false); the new named-object mapping below likewise promotes Section/SymbolicLink/Timer records. These are material canonical-output changes, contradicting the PR's explicit promise that canonical policy output remains unchanged and the issue's scope of adding a second diagnostic file. Keep these outcomes Data-Loop-only, or revise the stated compatibility contract and scope.
        CAPABILITY_DENIAL_EVENT_ID if is_ui_violation_schema(parts) => {
            build_denial_from_learning_mode(parts, pid, filetime, provider)
        }
        CAPABILITY_DENIAL_EVENT_ID => build_denial_from_capability(parts, pid, filetime, provider),

src/core/learning_mode_core/src/data_loop.rs:78

  • This is not always the event-header PID: brokered event 28 signatures intentionally use the payload ProcessId. Documenting it as header-only gives consumers the wrong interpretation of the deduplication key.
    /// Process identifier from the event header.

Comment thread src/backends/learning_mode/windows/src/etl_decode.rs
Replace the public and internal Data Loop terminology with verbose logging, including the serialized analysis field and sibling artifact filename.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: f9ac49b1-fdfe-4cca-9412-9faa99cef2ab
Replace Data Loop names in APIs, transport fields, artifact paths, diagnostics, tests, and documentation with verbose logging terminology.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: f9ac49b1-fdfe-4cca-9412-9faa99cef2ab
Copilot AI review requested due to automatic review settings August 19, 2026 17:02
@richiemsft Richie Gomez (richiemsft) changed the title Add Data Loop denial diagnostics Add captureDenial verbose diagnostics Aug 19, 2026
Route decoded test fixtures directly through the production handler so scope filtering and processing limits use the same single accounting path.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: f9ac49b1-fdfe-4cca-9412-9faa99cef2ab
@richiemsft Richie Gomez (richiemsft) changed the title Add captureDenial verbose diagnostics Add verbose Learning Mode denial logging Aug 19, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 22 out of 23 changed files in this pull request and generated no new comments.

Suppressed comments (4)

src/core/learning_mode_core/src/paired_output.rs:303

  • A cleanup failure can leave the ETL metadata stale. remove_promoted_output_if_owned first renames the source to a quarantine path; if deleting that quarantine then fails, this returns Err while the destination is committed and the original source path is absent. Audit consequently does not update etl_path and leaves it pointing to a missing file. Restore the source before returning an error or report a committed-with-cleanup-error state so the caller publishes the destination.
    let promoted = promote_output_file(operation, temp, destination_path, kind)?;
    drop((source, promoted));
    remove_promoted_output_if_owned(source_path, source_identity, None).and_then(cleanup_result)

src/core/learning_mode_core/src/paired_output.rs:30

  • The lock name is derived from the per-run canonical filename, but the lock file is never removed. Because captureDenials inserts a unique run ID into each output name, repeated captures leave an unbounded set of .pair.lock files even after their JSON artifacts are relocated or deleted. Use one stable directory-level lock (or another cleanup-safe lock primitive) so persistent lock state stays bounded.
        let lock_name = format!(".{file_name}.pair.lock");

src/core/learning_mode_core/src/paired_output.rs:269

  • Source cleanup is not transactional after the destination pair commits. If one cleanup succeeds and the other fails, this returns Err with one source member already gone and both destination members present. audit::relocate_artifacts publishes the destination path only on Ok, so its metadata can remain pointed at a broken source pair. Make the committed state explicit to the caller, or ensure cleanup cannot partially remove the source pair before returning an error.

This issue also appears on line 301 of the same file.

    let verbose_logging_cleanup = remove_promoted_output_if_owned(
        verbose_logging_source_path,
        verbose_logging_identity,
        None,
    )
    .and_then(cleanup_result);
    let canonical_cleanup =
        remove_promoted_output_if_owned(canonical_source_path, canonical_identity, None)
            .and_then(cleanup_result);
    match (verbose_logging_cleanup, canonical_cleanup) {
        (Ok(()), Ok(())) => Ok(()),
        (Err(error), Ok(())) | (Ok(()), Err(error)) => Err(error),

src/core/learning_mode_core/src/verbose_logging.rs:346

  • The PR description specifies the public sibling as denials.data-loop.json, but this derives denials.verbose.json (and the added docs/tests use the latter). Consumers implementing the stated Data Loop naming contract will not find the artifact. Align the filename with the PR contract or update the contract before merge.
    let stem = output_path
        .file_stem()
        .and_then(|value| value.to_str())
        .unwrap_or(file_name);
    let sibling = format!("{stem}.verbose.json");

Copilot AI review requested due to automatic review settings August 19, 2026 17:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 22 out of 23 changed files in this pull request and generated no new comments.

Suppressed comments (4)

src/core/learning_mode_core/src/paired_output.rs:118

  • This lock file is never removed, and captureDenials uses a fresh run ID in every canonical filename. Consequently each successful capture leaves a distinct .*.pair.lock file in the configured output directory, causing unbounded artifact buildup. CreateNew already serializes the winner through no-clobber promotion, so only create the persistent lock for Replace operations.
    let _pair_lock = OutputPairLock::acquire(operation, canonical_path)?;

src/core/learning_mode_core/src/paired_output.rs:523

  • If deleting the quarantined file fails, this returns an error without restoring it to final_path. In relocation mode there is no backup, so the destination has already committed while the source path disappears; callers then keep metadata pointing at that missing source because relocation returned Err. Restore the quarantine before reporting cleanup failure (and avoid cleaning the other pair member after that failure), or return an outcome that explicitly reports the committed destination.
        Ok(true) => Ok(remove_if_present(&quarantine_path).err().map(|error| {
            std::io::Error::other(format!(
                "failed to remove quarantined promoted output {}: {error}",
                quarantine_path.display()
            ))

src/backends/learning_mode/windows/src/etl_decode.rs:475

  • Event 28 is schema-dependent: it can be either a capability denial or a UI violation. When payload decoding has failed, this branch has not inspected Category/Detail, yet it always records resourceType: capability, producing incorrect verbose diagnostics for malformed UI-shaped event 28 records. Omit the classification unless the schema name can reliably distinguish the shape.
                crate::extractors::CAPABILITY_DENIAL_EVENT_ID => (
                    Some(learning_mode_core::AccessType::Unknown),
                    Some(learning_mode_core::ResourceType::Capability),
                ),

src/core/learning_mode_core/src/lib.rs:40

  • The PR checklist says .github/copilot-instructions.md was updated, but that file is absent from the change set. These new modules change the documented learning_mode_core output architecture from a single canonical document to a transactional pair, so the repository instructions now describe the old contract. Update them in this PR or correct the checklist claim.
pub mod paired_output;
pub mod summary;
pub mod verbose_logging;

Avoid per-run create-new lock artifacts, preserve committed relocation metadata while surfacing source-cleanup warnings, classify malformed event 28 records by schema name, and update the documented output architecture.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: f9ac49b1-fdfe-4cca-9412-9faa99cef2ab
Copilot AI review requested due to automatic review settings August 19, 2026 17:49
@richiemsft

Copy link
Copy Markdown
Contributor Author

Addressed the four findings from Copilot review 4974861785 in 7cd698c: CreateNew paired writes no longer leave run-specific lock files; relocation now returns a committed outcome with explicit source-cleanup warnings so audit metadata always points to the valid destination; quarantine deletion failures restore the source path; malformed event 28 records are classified as UI/capability only when the schema name identifies the shape; and .github/copilot-instructions.md now documents the canonical + verbose output pair. Validation: 43 learning_mode_core tests, 224 learning_mode_windows tests, 40 wxc tests, formatting, and strict targeted clippy.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 24 out of 25 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/backends/learning_mode/windows/src/extractors.rs:647

  • Adding SymbolicLink here does not preserve its canonical identifier through the existing accumulator. add_raw_denial normalizes every non-file resource with path_norm::to_user_visible (etl_decode.rs:254-255), so the newly tested symbolic-link name \??\C: is emitted as C: rather than the raw Object Manager name promised for other resources. Limit filesystem normalization to ResourceType::File (or otherwise carry the object subtype) and add an end-to-end accumulator test for this shape.
        "Section" | "SymbolicLink" | "Timer" => ResourceType::Other,

src/core/learning_mode_core/src/paired_output.rs:234

  • This can return Err after both new files have already been committed. In particular, backups preserve the original attributes, and on Windows deleting a read-only backup fails; the code explicitly supports restoring read-only backups but does not make successful backup cleanup possible. plm stop will therefore report failure even though it replaced the output pair, leaving callers unable to distinguish a committed result from a failed write. Either reject non-cleanable targets before mutation or return post-commit cleanup warnings separately, as the relocation APIs do.
    drop((canonical_promoted, verbose_logging_promoted));
    remove_backups(
        canonical_backup.as_deref(),
        verbose_logging_backup.as_deref(),
    )
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Copilot-Instructions PR modifies Copilot instruction files (.github/copilot-instructions.md or .github/instructions/)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add a new captureDenial_verbose file

3 participants