Skip to content

feat(agent): audit package policy changes - #1980

Open
Benoît Cortier (CBenoit) wants to merge 5 commits into
cbenoit-policy-installer-migrationfrom
cbenoit-policy-audit-events
Open

feat(agent): audit package policy changes#1980
Benoît Cortier (CBenoit) wants to merge 5 commits into
cbenoit-policy-installer-migrationfrom
cbenoit-policy-audit-events

Conversation

@CBenoit

@CBenoit Benoît Cortier (CBenoit) commented Sep 8, 2026

Copy link
Copy Markdown
Member

Adds structured Windows Event Log auditing for package policy write attempts and outcomes (IDs 8000–8005), plus externally observed policy changes (8010–8011). Events publish after authoritative state changes, use bounded privacy-safe fields, and flow through a bounded asynchronous Event Log queue.

Adds EN/FR/DE message catalogs and embeds them in production Agent and Gateway binaries. Windows CI resolves mc.exe only from trusted installed SDK locations. Targeted policy audit tests and production Agent and Gateway builds cover the event contract and embedded resources.

Stacked on #1979. Extracted from #1963.

Issue: #1963

Record bounded write attempts and operation-specific outcomes without exposing policy content or blocking request admission on Event Log I/O.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Compile localized message resources for release and production builds using trusted installed Windows SDK tools.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Verify every shared event code and policy insertion string across both localized Windows message catalogs.

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

Copy link
Copy Markdown
Member Author

Implementation notes:

  • Preserves the shared audit contract: 8000/8001 record attempts and denials; 8002/8003 are Create failure/success; 8004/8005 are Update, Repair, or ReplaceIdentity failure/success; 8010/8011 record externally applied/rejected changes.
  • Publishes externally observed state before emitting 8010/8011 and records exactly one terminal API outcome through an atomic lifecycle guard.
  • Restricts Event Log fields to bounded actor, path, operation, outcome, reason, and policy identity metadata. Policy bodies, drafts, receipts, and store tokens are excluded.
  • Sends release/production Event Log writes through one bounded 256-entry nonblocking queue. Overflow drops only the Event Log copy and emits rate-limited tracing telemetry.
  • Adds EN/FR/DE catalogs, catalog parity checks, production resource embedding, and trusted installed-SDK mc.exe discovery without downloading executable PowerShell modules.
  • Targeted validation passed: sysevent-codes 3/3, now-package-broker 430 passed with 2 ignored, rustfmt, focused Clippy with warnings denied, and diff checks. The same audit/build/catalog content passed production Agent and Gateway resource builds with Windows SDK 10.0.28000.0 before the final installer-only restacks.
  • Extracted from feat(agent,agent-installer): add transactional policy store #1963 and stacked on fix(agent-installer): secure policy migration #1979. No breaking API or migration change is introduced by this layer.

Note

Human-tuned, LLM-assisted content.

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.

🟡 Changes recommended

The release/production mc.exe discovery still consults PATH (weakening the “trusted SDK locations only” goal) and the new async tests depend on thread-local audit capture that can be flaky under Tokio’s default multi-thread runtime.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds Windows Event Log auditing for package-policy write attempts/outcomes and external policy changes, wiring the broker’s policy-management path to emit structured sysevent entries backed by embedded .mc message catalogs.

Changes:

  • Introduces policy write/external-change audit events (IDs 8000–8005, 8010–8011) in sysevent-codes plus catalog parity tests.
  • Adds and embeds Devolutions Agent/Gateway Windows Event Log message catalogs via mc.exe during release/production builds.
  • Threads per-request write-audit context through the package broker server and policy store to record deny/fail/success and external-change observations.
File summaries
File Description
devolutions-gateway/devolutions-gateway.mc Adds policy management audit message templates (8000–8011).
devolutions-gateway/build.rs Embeds the Gateway .mc into release/production binaries; adds mc.exe discovery logic.
devolutions-agent/devolutions-agent.mc New Agent Windows Event Log message catalog including policy audit IDs.
devolutions-agent/build.rs Embeds the Agent .mc into release/production binaries.
crates/sysevent-codes/src/lib.rs Defines policy audit event codes and structured field builders (+ unit tests for field order).
crates/sysevent-codes/tests/message_catalog_parity.rs Validates both .mc catalogs define all event codes and correct insertion counts.
crates/now-package-broker/src/lib.rs Adds the Windows-only audit module.
crates/now-package-broker/src/audit.rs New audit implementation: bounded fields, async-safe recording, and Windows Event Log worker (prod).
crates/now-package-broker/src/auth.rs Exposes authenticated client executable path for auditing.
crates/now-package-broker/src/server/mod.rs Creates/scopes per-request policy write audit context for PUT /v1/policy.
crates/now-package-broker/src/policy_store/mod.rs Adds audited replace path and emits external-change audit events on publication.
crates/now-package-broker/Cargo.toml Adds sysevent dependencies needed for auditing.
Cargo.lock Locks new workspace dependency edges for auditing.
.github/workflows/ci.yml Finds mc.exe via installed Windows SDK roots and exports WindowsSdkVerBinPath for builds.
Review details

Suppressed comments (1)

crates/now-package-broker/src/policy_store/mod.rs:1360

  • This test relies on crate::audit::take_test_events() (thread-local in tests). With the default multi-thread Tokio runtime, the task can migrate threads across .await points and make the thread-local assertions flaky. Consider switching this test to a current-thread runtime.
        std::fs::create_dir_all(managed.parent().expect("managed path has a parent"))
            .expect("create managed directory");
        std::fs::write(&managed, b"managed").expect("write managed marker");
        crate::audit::take_test_events();
        store.reload_from_disk(ReloadCause::ExternalChange).await;
  • Files reviewed: 13/14 changed files
  • Comments generated: 4
  • Review effort level: Lite

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread devolutions-agent/build.rs Outdated
Comment thread devolutions-gateway/build.rs Outdated
Comment thread crates/now-package-broker/src/policy_store/mod.rs Outdated
Comment thread crates/now-package-broker/src/audit.rs
Restrict message compiler discovery to trusted SDK paths, keep thread-local audit assertions on one runtime thread, and avoid an unnecessary path allocation.

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

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.

🔵 Needs a closer look

It introduces new Windows Event Log auditing behavior across request handling, persistence, and build/CI plumbing, and should receive final human review despite only minor issues found.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

crates/now-package-broker/src/policy_store/mod.rs:405

  • audit_path = observation.canonical_path.clone() is done even when audit is None, which adds an avoidable allocation on the hot error paths. Since publish_external_observation returns a snapshot containing the same path, you can defer path materialization to the if let Some(audit) branch and borrow from management.configured_path (and apply the same pattern to the similar audit_path clones in the other error branches below).
  • Files reviewed: 13/14 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Borrow published management paths only when an audit terminal event is recorded.

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

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.

🔵 Needs a closer look

It introduces a cross-cutting Windows auditing pipeline and build/CI changes that should be validated by a human reviewer on Windows runners and release packaging flows.

Review details
  • Files reviewed: 13/14 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@CBenoit
Benoît Cortier (CBenoit) marked this pull request as ready for review September 9, 2026 01:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants