test(agent): cover policy management end to end

[Benoît Cortier (CBenoit)]

Exercises Agent policy management through protected standard-user and LocalSystem clients without weakening executable, ACL, path, or process-identity checks.

Covers read and validation behavior, standard-user write denial, Create, Update, Repair, stale conflict rejection, exact ConfirmOverwrite, restart persistence, durable managed authority, and observable audit outcomes. The Windows CI job runs both identities and the feature-gated route authorization tests.

Local PsExec execution could not install PSEXESVC, so the identity scenarios are delegated to CI; local builds, workspace tests, Clippy, formatting, runner parsing, and range validation pass.

Extracted from the production-shaped E2E support in #1963.

[Benoît Cortier (CBenoit)]

Implementation notes:

• Extends the existing production-shaped Agent policy tester without adding production bypasses or changing store, authentication, installer, or audit implementation.
• Stages the tester under protected LocalSystem-owned ACLs before running either a restricted standard-user token or LocalSystem.
• Covers validation, standard-user denial, Create, Update, Repair, stale Reject, exact ConfirmOverwrite, restart persistence, durable managed authority, and audit trace observability while retaining existing unsafe-path and watcher-token regressions.
• CI runs both identity modes and the feature-gated package-broker authorization tests.
• Local PsExec could not install PSEXESVC, so identity scenarios await Windows CI. Full serial workspace tests, Clippy, formatting, locked builds, runner parsing, and range checks pass locally.

Note

Human-tuned, LLM-assisted content.

[Copilot]

🟡 Changes recommended

The standard-user scenario runs both client and Agent under the same restricted identity, leaving the production cross-identity authorization boundary untested.

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

Pull request overview

Adds end-to-end Agent policy-management coverage across restricted and LocalSystem execution modes.

Changes:

• Expands lifecycle, authorization, persistence, and audit tests.
• Adds protected unelevated test orchestration.
• Runs identity and feature-gated route tests in Windows CI.

Documentation-specific review was not applicable.

File summaries

File	Description
crates/agent-policy-tester/src/windows.rs	Adds identity modes and policy lifecycle scenarios.
crates/agent-policy-tester/run-unelevated.ps1	Runs the tester with a restricted token.
crates/agent-policy-tester/run-as-system.ps1	Selects elevated test mode.
crates/agent-policy-tester/Cargo.toml	Adds Windows identity dependencies.
Cargo.lock	Records dependency updates.
.github/workflows/ci.yml	Runs both identities and route tests.

Review details

• Files reviewed: 5/6 changed files
• Comments generated: 1
• Review effort level: Balanced

---

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

[Copilot]

🔵 Needs a closer look

The security-sensitive Windows identity and ACL orchestration requires confirmation from the Windows CI execution.

Review details

• Files reviewed: 5/6 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

The psexec -l client runs at Low integrity and may be denied by the named pipe before testing the intended authorization response.

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

Review details

• Files reviewed: 5/6 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

Failed readiness validation can leave LocalSystem test processes running into later CI steps.

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

Review details

• Files reviewed: 5/6 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

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

[Copilot]

🔵 Needs a closer look

Three moderate test-coverage gaps must be addressed before approval.

Review details

Suppressed comments (3)

crates/agent-policy-tester/run-unelevated.ps1:350

• The new self-test does not exercise the cleanup guarantee it claims to cover. $launchAttempted is assigned true immediately before each Wait-ServerReadiness call and is only tested against itself; no code invokes the surrounding finally or verifies that Signal/status cleanup runs after failed readiness. A regression that removes or relocates $serverLaunchAttempted = $true could therefore pass SelfTest. Refactor the orchestration into a testable function with a fake launcher, or add an integration assertion that failed readiness still signals and observes server shutdown.

        $launchAttempted = $true
        Set-Content -LiteralPath $ready -Value (
            @{
                Nonce = "wrong-nonce"
                PipeName = "\\.\pipe\test"
                ServerPid = 100
                ServerSid = "S-1-5-18"
                AgentPid = 200
                AgentSid = "S-1-5-18"
            } | ConvertTo-Json -Compress
        )
        try {
            Wait-ServerReadiness -Path $ready -ServerStatusPath $status -ExpectedNonce "nonce" `
                -TimeoutMilliseconds 50 -LaunchValue 6256 -LaunchDiagnostics "started detached process" | Out-Null
            throw "Mismatched-readiness simulation unexpectedly succeeded"
        } catch {
            if ($_ -notmatch "readiness nonce mismatch" -or -not $launchAttempted) {

crates/agent-policy-tester/src/windows.rs:921

• The standard-user leg only exercises /v1/policy/management and /v1/policy/validate; it never calls GET /v1/policy, even though the policy-management contract keeps reads available to an authenticated standard user. A regression that makes active-policy reads administrator-only would therefore pass this scenario. Add a GET assertion here (the policy is missing in this harness, so expect HTTP 404).

    let management = policy_management_by_pipe(pipe_name).await?;
    ensure!(management["State"] == "Missing", "expected a missing policy");

crates/agent-policy-tester/src/windows.rs:1129

• This ConfirmOverwrite request uses current_token from the stale-conflict response, so the token is already fresh. It never verifies the contract that a stale token remains rejected when ConflictHandling is ConfirmOverwrite; a regression that turns this mode into an unconditional stale-token bypass would still pass. Preserve stale_token, submit it with ConfirmOverwrite, and assert 409/StalePolicyStoreToken before retrying with current_token.

    let confirmed = replace_policy_response(
        &agent,
        "Update",
        "ConfirmOverwrite",
        current_token.clone(),

• Files reviewed: 5/6 changed files
• Comments generated: 0 new
• Review effort level: Lite (auto)

Note

Copilot is running an experiment and ran this review at Lite.

[Benoît Cortier (CBenoit)]

Addressed the exact-head Copilot review coverage gaps in ff4031ba:

• The standard-user path now verifies the canonical missing-policy GET /v1/policy response.
• ConfirmOverwrite now explicitly rejects a stale store token before the current-token retry.
• The runner SelfTest now simulates failed readiness with malformed server status and proves the production shutdown helper signals, consumes status, and preserves failure.

No inline review threads were created by the review to resolve.

Note

Human-tuned, LLM-assisted content.

[Copilot]

🔵 Needs a closer look

The extensive multi-process Windows identity orchestration and security-boundary coverage require final human validation.

Review details

• Files reviewed: 5/6 changed files
• Comments generated: 0 new
• Review effort level: Lite (auto)

Note

Copilot is running an experiment and ran this review at Lite.