Skip to content

test(process-container): split the Windows suite into per-area scripts and add schema 0.8 network coverage - #1217

Open
Elliot (theelliotm) wants to merge 33 commits into
mainfrom
user/emichlin/wpc-tests-0.8-network-coverage
Open

Elliot (theelliotm) wants to merge 33 commits into
mainfrom
user/emichlin/wpc-tests-0.8-network-coverage

Conversation

@theelliotm

@theelliotm Elliot (theelliotm) commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

📖 Description

The old process container tests in /tests/scripts hadn't been updated since 0.6 and had numerous bugs.

This PR rebuilds the Windows process-container validation suite: splits the monolithic
WinProcessContainer-Tests.ps1 into per-area scripts (like every other backend), adds the schema 0.8
directional networking coverage it never had, and removes the false greens that
let a mis-provisioned host report success having proven nothing.

Suite split. tests/scripts/WinProcessContainer-Tests.ps1 (1749 lines) is
replaced by run_processcontainer_all_tests.ps1 (probes the host once, writes
the resolved context to JSON, dispatches each area, merges results) plus 22
run_processcontainer_<area>_test.ps1 scripts that also run standalone. Shared
helpers live in tests/scripts/lib/WinProcessContainer.Common.ps1 (dot-sourced,
not imported: Initialize-WpcContext publishes the context into the caller's
scope) and WinProcessContainer.Native.ps1 (P/Invoke, dot-sourced only by the
three areas that need it). -Areas is aliased to the old -Phases, so existing
phase lists keep working.

Area key Script Covers
Probes run_processcontainer_probes_test.ps1 --probe (read-only)
T3Forced run_processcontainer_filesystem_matrix_test.ps1 natural detection → T3; rw/ro/denied matrix
T1DenyForced run_processcontainer_denied_paths_test.ps1 T1 deny-ACE empirical test
FsEnumerate run_processcontainer_filesystem_enumerate_test.ps1 filesystem.enumeratePaths
UiMitigationMatrix run_processcontainer_ui_mitigations_test.ps1 UI mitigation behavior matrix
UiPolicyMatrix run_processcontainer_ui_policy_matrix_test.ps1 UI policy resolution, every documented value
Capabilities run_processcontainer_capabilities_test.ps1 capabilities documented rejections
CaptureDenials run_processcontainer_capture_denials_test.ps1 captureDenials.outputPath validation
Lifecycle run_processcontainer_lifecycle_test.ps1 lifecycle.destroyOnExit / preservePolicy
Privilege run_processcontainer_privilege_test.ps1 leastPrivilege (LPAC)
GlobalAtomIsolation run_processcontainer_global_atom_test.ps1 GLOBALATOMS bidirectional isolation
DaclDisabled run_processcontainer_dacl_disabled_test.ps1 allowDaclMutation=false refusal
CrashRecovery run_processcontainer_crash_recovery_test.ps1 taskkill mid-run, DACL state recovery
NetworkCapabilityMatrix run_processcontainer_network_capability_test.ps1 0.8 egress × ingress capability matrix
NetworkModel3Equivalence run_processcontainer_network_model3_test.ps1 explicit deny == omitted == empty network
NetworkEgressRules run_processcontainer_network_egress_test.ps1 explicit egress rules (WFP / PSEC-only)
NetworkHostLoopback run_processcontainer_network_loopback_test.ps1 host-loopback policy
NetworkProxy run_processcontainer_network_proxy_test.ps1 0.8 runtime proxy (model 2)
NetworkRejections run_processcontainer_network_rejections_test.ps1 documented network reject surface
NetworkLegacy07 run_processcontainer_network_legacy07_test.ps1 legacy network fields (schema 0.7)
PathAliasing run_processcontainer_path_aliasing_test.ps1 path aliasing, most-specific-wins
ProcessPlumbing run_processcontainer_process_plumbing_test.ps1 env / cwd / exit code / timeout / teardown

New coverage.

  • Configs now emit 0.8.0-alpha with containment: processcontainer; they were
    emitting 0.6 with an appcontainer value that is in no stable enum.
  • Legacy network fields pin the config to 0.7.0-alpha, and mixing them with a
    directional key throws rather than silently emitting a 0.7 config carrying
    0.8-only keys.
  • Schema 0.8 directional networking: the egress × ingress capability matrix,
    model-3 equivalence, explicit egress rules, host loopback, the runtime proxy,
    and the documented reject surface.
  • The legacy 0.7 network lane, kept as its own area so the compatibility path is
    asserted separately from the directional one.
  • Filesystem enumerate-only grants (filesystem.enumeratePaths).
  • The UI policy resolution matrix, covering every documented value.
  • capabilities, captureDenials, lifecycle, and leastPrivilege rejection
    and acceptance surfaces.
  • Path aliasing and most-specific-wins resolution.
  • Process plumbing: env, cwd, exit codes, timeout, and surviving children.

Bug Fixes / Changes

  • Removed false positives caused by the sandbox not launching
  • Expected tier is now properly enforced.
  • The dispatcher now stages the plm.exe and winhttp-proxy-shim.exe sidecars that wxc-exec
    resolves next to its own image, without them the guarded-WPR captureDenials
    fallback and the legacy proxy path fail as launch errors instead of policy
    results
  • process-t1 now gets the same prepare-system-drive /
    prepare-null-device preparation as process-t3, since a T1 host still drives
    the AppContainer fallback tiers.
  • wxc-ui-probe reported FAIL when user32.dll would not
    load. Under ui.disable=true that is the expected outcome - the Win32k
    syscall-disable mitigation takes the GUI subsystem with it - so FAIL claimed the
    restriction was not enforced and would mask a real regression. Those cases now
    report INCONCLUSIVE with a diagnostic.
  • Process-container configs and tests renamed with the processcontainer- prefix.
  • SBOX tests, fallbacks, and assertions removed.

🔗 References

No tracking issue.

  • docs/ci-validation-infrastructure.md — updated for the new entry point, the
    -RequireTier gate, and the process-t1 host-preparation change.
  • tests/scripts/README.md — updated for the per-area scripts, the
    dot-sourcing requirement, and the T2 exclusion.
  • Contract under test: docs/process-container/networking.md and
    docs/sandbox-policy/0.8.0/networking/networking.md.

🔍 Validation

  • Ran in CI. All failures are genuine failures with Windows/MXC: https://github.com/microsoft/mxc/actions/runs/35294887011
  • Per-area scripts exercised standalone against a built tree.
  • All suite scripts parse under Windows PowerShell 5.1.
  • cargo test -p wxc_common -- config_parser — 406 passed, 0 failed (covers the
    corpus divergence-inventory change).

✅ Checklist

📋 Issue Type

  • Bug fix
  • Feature
  • Task

Elliot (theelliotm) and others added 23 commits September 11, 2026 21:45
…alse greens

WinProcessContainer-Tests.ps1 had extensive negative networking coverage and
almost no positive coverage, and nothing exercised the schema 0.8 directional
network shape at all. Configs were still emitted at 0.6 with a containment
value (`appcontainer`) that is not in the stable enum at either 0.7 or 0.8.

Generator
- New-Config now emits 0.8.0-alpha with `containment: processcontainer`, and
  gained cwd, capabilities, leastPrivilege and the directional network
  parameters (egress/ingress defaults, hostLoopback, explicit egress rules,
  runtime proxy, allowedProxyPeer).
- The legacy network fields stay on 0.7.0-alpha: supplying any of them pins
  the version, and mixing them with a directional key now throws rather than
  silently emitting a 0.7 config carrying 0.8-only keys.

New phases
- 8a capability matrix, 8b model-3 equivalence, 8c explicit egress rules,
  8d host loopback, 8e runtime proxy, 8f the documented reject surface,
  9 the legacy 0.7 lane incl. firewall rule teardown, 10 path aliasing,
  11 process plumbing (env, cwd, exit codes, timeout, survivors).

Assertions encode the documented expectation from
docs/process-container/networking.md and the 0.8 policy spec, so some are
expected to fail against current behavior.

Tier gate
- The suite derives expectations from the tier the host selects, so it was
  self-consistent anywhere -- and therefore silently useless on a host meant
  to be T1 that fell back to T3. -RequireTier makes a mismatch a hard abort,
  and the dispatcher passes base-container for process-t1 and
  appcontainer-dacl for process-t3.
- process-t1 now gets the same host preparation as process-t3; without
  prepare-system-drive the launch fails outright and no phase can report a
  policy result.

False greens
Most negative assertions here are satisfied by a run in which the sandbox
never launched, so a mis-provisioned host reported green having proven
nothing. Guards added and applied throughout: Get-NetVerdict's NORUN state,
Test-VerdictsRan for differential assertions, an unconditional marker echo
with Test-WorkloadRan, and Test-WasRejected to separate a policy rejection
from a timeout or a launch failure. Phase 8f gained a positive control.

Two related bugs found while testing:
- wxc-exec echoes its redacted config to stderr and the log on every failure
  path, so any assertion searching for a token was searching the harness's own
  input. Remove-ConfigEcho excises the three echoed sections; it must cut them
  out and keep the tail, since the echo is a prefix and all runner output
  (tier selection, capability list) follows it.
- Invoke-Wxc waited for exit before draining the redirected pipes, so a child
  that filled either buffer -- routine once a failure path dumps the whole
  config -- was recorded as a hang, turning real rejections into timeouts.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The process-container harness had grown to 3,414 lines / 189 KB, twice the
size of the next largest script in the repo, and was the only Windows backend
suite still shipped as a single file. Reshape it into the layout the WSLc and
Seatbelt suites already use: an entry script that dispatches to per-area
scripts, over a shared library.

  tests/scripts/run_processcontainer_all_tests.ps1   entry / dispatcher
  tests/scripts/run_processcontainer_<area>_test.ps1 19 area scripts
  tests/scripts/lib/WinProcessContainer.Common.ps1   shared helpers
  tests/scripts/lib/WinProcessContainer.Native.ps1   P/Invoke shims

Each area script also runs standalone, which is the point: iterating on one
area no longer means driving the whole suite.

The library is dot-sourced rather than imported as a module, deliberately.
Initialize-WpcContext publishes the suite context into the *calling* script's
scope, which is what lets the ~63 helpers and all the phase bodies move
verbatim -- they read those names bare. As a module the same names would bind
to the module scope and every helper would silently read $null.

Areas report merged structured results rather than one pass/fail apiece: each
child writes a JSON document the entry merges, so per-assertion detail
survives. A child that dies before writing one is recorded as a failure
instead of vanishing from the summary, and MXC-FATAL (exit 78) still stops
the whole suite on the spot.

Verified behavior-preserving against the pre-split harness on the same host:
124 assertions both before and after, with the 63 failures an identical set.
The deltas reconcile exactly -- two removed T2 assertions that were passing,
two deliberate skips that are now recorded.

T2 (appcontainer-bfs) coverage is dropped; it is off by default behind the
tier2_bfs Cargo feature and is not in use. No phase exercised it, and all
four assertions were redundant with guards that remain: two duplicated
Assert-BfsSafety, and two re-ran the exact regex Assert-NoBfscfg had already
raised MXC-FATAL on two lines earlier, so they could only ever print PASS.
Assert-BfsSafety and Assert-NoBfscfg are kept as guards rather than tests --
invoking bfscfg.exe hard-locks the bfs.sys minifilter on 25H2. Also drops the
dead -AllowBfsTierSelection switch, which no caller ever passed.

Two fixes found while splitting:

- ValidateSet binds to the variable, not just to parameter binding, so it
  re-fires when Initialize-WpcContext publishes context back into scope. An
  unspecified -RequireTier threw. Area scripts drop the attribute and let
  Initialize-WpcContext validate; the entry keeps it with '' in the set.
- Two areas both tagged their assertions P4c, making a failure in the merged
  summary unattributable. The deny-ACE area is now P4d.

A deliberate skip is recorded as a skip rather than returning silently,
because an area that records nothing at all is otherwise indistinguishable
from one that did nothing due to a bug -- both the child and the entry treat
zero assertions as a failure.

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

BaseContainer's "[ui subsystem]" telemetry comes from log_sandbox_spec(),
which decodes the legacy SBOX FlatBuffer and is called only under
`if !use_process_security_environment`. A run that takes the PSEC /
CreateProcessSecurityEnvironment path emits none of it, so the token greps
in Test-UiRestrictionsApplied / Test-Win32kMitigationApplied were unsound
in both directions there: positive assertions failed despite the
restriction holding, and the negated ones ("mitigation NOT applied")
passed vacuously.

Route all seven call sites through Record-UiTelemetryResult, which skips
when the log shows the PSEC spec being built. Detection is positive-proof,
so a run that dies before building any spec is still asserted and still
fails.

Also:
- Drop four orphaned tests/configs/network_*_test.json fixtures. They
  varied only enforcementMode, which the NetworkLegacy07 area already
  covers explicitly, and nothing referenced them.
- Re-derive child output color in the entry script. Console color is an
  attribute rather than bytes, so it does not survive a pipe, and a child
  killed mid-Write-Host left the console tinted for every later area.
- Prefix six process-container fixtures with processcontainer_ to match
  the convention the other backends already follow.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds four areas -- capabilities, captureDenials, lifecycle/process, and
privilege/legacy-proxy -- and deepens the single-value cases in each. Suite
grows from 126 to 228 assertions.

processContainer.capabilities, captureDenials, learningMode, leastPrivilege,
lifecycle.*, process.inheritDefaultEnv, telemetry.enabled, the containment
intent alias, the supported schema-version range, and the three legacy 0.7
network.proxy shapes had no coverage at all. Four New-Config parameters were
declared but never passed by any area, which reads as coverage that does not
exist.

Hardens Test-WasRejected against three false-rejection classes it did not
catch: a post-tier-selection failure, a typed backend_error, and
runner_unavailable. Each of those is a host limitation rather than a policy
rejection, and counting them as rejections credited the policy under test for
the host being under-provisioned.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Follows the schema-reference correction: telemetry is 0.9.0-alpha+, so an
earlier version must reject it rather than ignore the request.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Validation.Infrastructure.Tests.yml is the only consumer of the 'enabled'
plan, so without it the process-container entries added to that plan have
nothing to schedule them. Retargets the push trigger from the SIP branch to
this one now that the branch is based on main.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Nineteen assertions failed for reasons in the fixtures rather than the
product:

- Capability names appear only in the legacy SBOX spec dump, so the five
  assertions reading them are unobservable on a PSEC host. Gate them behind
  Test-SpecDumpAvailable via a shared Record-CapabilityLogged helper.
- Host firewall rules are an AppContainer primitive, so enforcementMode
  firewall/both and blockedHosts cannot enforce on a base-container host.
  Gate on tier as well as elevation.
- process.env replaces the environment outright, so a block holding only the
  variable under test cannot launch. Add Get-MinimalEnv.
- The selected-tier regex missed the word 'isolation'.
- UiIsolation is a closed wire enum, so an unrecognized value is rejected at
  parse rather than defaulted; assert the rejection instead.
- BaseContainer needs no DACL augmentation for denied paths where the OS
  supports them natively.
- Record the observed tail on the timeout and proxy assertions so a launch
  failure is diagnosable from the log.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Cuts comment lines from 2095 to 1677 across 26 files with no code changes.
Removes duplicated blocks, collapses the header boilerplate repeated in
every area script, and moves the shared parameter notes into
Initialize-WpcContext so they are written down once.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Test-WasRejected now requires positive evidence of a refusal (the Request
error banner or a typed config_parse/policy_validation code) and treats a
-1 exit as a launch failure, so an unexplained failure no longer counts as
a rejection. It also reads Stderr off the unwrapped result, which wrapped
Invoke-NetRun objects were bypassing.

Other fixes:
- the direct-egress proxy assertion now bypasses HTTP_PROXY, which curl was
  otherwise honoring and reporting BLOCKED for free
- denied-path, ping, and Win32k phases require a start marker, since all
  three were satisfied by a workload that never launched
- state-file checks compare against a baseline instead of deleting the
  shared dacl-restore directory, which held other processes' records
- firewall rules are correlated by their embedded timestamp
- the valid-outputPath control requires the run not to be refused at all
- the empty-policy tier assertion is labelled as a repeatability check

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Cuts:
- run_processcontainer_proxy_tests.ps1 was orphaned by the split; nothing
  referenced it.
- UnitTests shelled cargo test, duplicating the CI Rust lane and requiring a
  toolchain on a runner that deliberately tests a prebuilt artifact. It was
  the only consumer of CargoLog.
- EmptyRelease and DeniedRelease re-ran two trivial checks against the release
  binary; -SkipReleaseLane went with them.
- Assert-NoBfscfg and its ten call sites are gone. Assert-BfsSafety still
  refuses to run against a tier2_bfs binary, which is the check that keeps the
  suite from hanging a 25H2 host.

Each area used to declare a 17-parameter block so the entry script could
forward the resolved context, and every one of them repeated it. The entry
script now writes that context to a JSON file and passes -ContextJson; an
explicitly supplied parameter still wins, so standalone runs are unchanged.

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

Move the legacy 0.7 network shape out of New-Config behind a -RawNetwork
escape hatch and a local New-LegacyConfig wrapper in the one area that uses
it, collapse the repeated per-file headers, and drop the separator rules.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Restore the legacy 0.7 proxy shapes, which the New-Config cleanup broke.
- Add LOCALAPPDATA to Get-MinimalEnv: Windows refuses to create a contained
  process whose caller-supplied env lacks it, so three env tests never started.
- Share Get-SelectedTier, which tolerates the timestamps the logger writes
  mid-line, so the containment-intent tiers compare non-empty values.
- Replace the timeout workload's ping, which dies instantly in the container,
  with a cmd spin loop, and bound the elapsed time below as well as above --
  a child that exits on its own was scoring two assertions green.
- Accept either documented timeout message.
- P8e now asserts the documented pair (runs, or is rejected cleanly) instead of
  requiring a run, and skips the dependent egress check when nothing started.

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

wxc-ui-probe emitted FAIL -- which the harness scores as 'allowed' -- for the
seven user32-dependent tags whenever user32.dll would not load. That is exactly
what ui.disable=true causes, so the phase asserting that disable overrides every
permissive knob was reporting the mitigation working as the mitigation absent.
Those arms now emit INCONCLUSIVE, and the harness counts a missing GUI subsystem
under ui.disable=true as 'blocked', since it is strictly stronger than the
individual limit.

An INCONCLUSIVE nothing explains is now a failure, not a skip.

Also assert the documented floor once per run: with every UI knob permissive a
contained process should get every UI capability (UIPolicy_Schema.md:95,
os-version-support.md:185). It currently does not, and that stays red.

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

A build that supports JOB_OBJECT_UILIMIT_INJECTION and a probe that owned the
foreground leave no excuse for the injection limit not firing, so both sites
that recorded it as a warning now fail.

hostLoopback=allow that neither reaches the anchor nor produces a typed
rejection was recorded as a skip named 'rejected', which describes an outcome
that did not occur; it now fails.

Without SANDBOX_CAP_DENY_PATHS the enforcement test still cannot run, but the
documented refusal can be asserted, so that phase checks deniedPaths is refused
rather than skipping outright.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings September 18, 2026 19:29

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

Several harness paths can still produce false results or omit intended coverage, including proxy functionality, empty capabilities, sidecar builds, and stderr validation.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Reorganizes Windows ProcessContainer validation into focused suites, expands schema 0.8 coverage, strengthens failure detection, and updates CI tier enforcement.

Changes:

  • Splits the monolithic suite into 22 independently runnable areas with shared helpers.
  • Adds networking, filesystem, UI, lifecycle, capability, and process-plumbing coverage.
  • Updates CI preparation, fixture names, documentation, and UI probe semantics.
File summaries
File Description
docs/ci-validation-infrastructure.md Documents tier-gated validation.
scripts/ci/prepare-windows-host.ps1 Prepares T1 and T3 hosts.
scripts/ci/run_backend_validation_tests.ps1 Dispatches the new suite and sidecars.
sdk/node/tests/integration/test-helpers.ts Updates probe-script reference.
src/core/wxc/src/main.rs Updates recovery-test reference.
src/core/wxc_common/src/config_parser.rs Updates fixture inventory counts.
src/core/wxc_common/src/filesystem_dacl.rs Updates test references.
src/testing/wxc_e2e_tests/src/lib.rs Updates renamed fixture documentation.
src/testing/wxc_e2e_tests/tests/e2e_windows.rs Uses renamed fixtures.
src/testing/wxc_ui_probe/src/main.rs Reports unavailable GUI probes as inconclusive.
tests/configs/network_both_test.json Removes obsolete network fixture.
tests/configs/network_capabilities_test.json Removes obsolete network fixture.
tests/configs/network_default_test.json Removes obsolete network fixture.
tests/configs/network_firewall_test.json Removes obsolete network fixture.
tests/configs/processcontainer_basic_lpac.json Renames and corrects LPAC fixture.
tests/configs/processcontainer_basic_permissive.json Renames ProcessContainer fixture.
tests/configs/processcontainer_containment_intent.json Renames containment-intent fixture.
tests/configs/processcontainer_proxy_builtin_test.json Renames proxy fixture.
tests/configs/processcontainer_pwsh_setlocation.json Renames PowerShell fixture.
tests/configs/processcontainer_with_capabilities.json Renames capability fixture.
tests/scripts/README.md Documents the split suite.
tests/scripts/T3-Workloads.ps1 Updates suite references.
tests/scripts/WinProcessContainer-Tests.ps1 Removes the monolithic suite.
tests/scripts/lib/WinProcessContainer.Common.ps1 Adds shared harness functionality.
tests/scripts/lib/WinProcessContainer.Native.ps1 Adds native UI/atom helpers.
tests/scripts/run_lpacac_test.ps1 Uses renamed LPAC fixture.
tests/scripts/run_processcontainer_all_tests.ps1 Adds aggregate area dispatcher.
tests/scripts/run_processcontainer_capabilities_test.ps1 Adds capability coverage.
tests/scripts/run_processcontainer_capture_denials_test.ps1 Adds denial-capture coverage.
tests/scripts/run_processcontainer_crash_recovery_test.ps1 Adds DACL crash recovery coverage.
tests/scripts/run_processcontainer_dacl_disabled_test.ps1 Adds DACL refusal coverage.
tests/scripts/run_processcontainer_denied_paths_test.ps1 Adds denied-path coverage.
tests/scripts/run_processcontainer_filesystem_enumerate_test.ps1 Adds enumeration-only coverage.
tests/scripts/run_processcontainer_filesystem_matrix_test.ps1 Adds filesystem access matrix.
tests/scripts/run_processcontainer_global_atom_test.ps1 Adds atom isolation coverage.
tests/scripts/run_processcontainer_lifecycle_test.ps1 Adds lifecycle and environment coverage.
tests/scripts/run_processcontainer_network_capability_test.ps1 Adds network capability matrix.
tests/scripts/run_processcontainer_network_egress_test.ps1 Adds explicit egress-rule coverage.
tests/scripts/run_processcontainer_network_legacy07_test.ps1 Adds legacy networking coverage.
tests/scripts/run_processcontainer_network_loopback_test.ps1 Adds host-loopback coverage.
tests/scripts/run_processcontainer_network_model3_test.ps1 Adds default-deny equivalence coverage.
tests/scripts/run_processcontainer_network_proxy_test.ps1 Adds runtime-proxy coverage.
tests/scripts/run_processcontainer_network_rejections_test.ps1 Adds invalid-policy coverage.
tests/scripts/run_processcontainer_path_aliasing_test.ps1 Adds path-alias coverage.
tests/scripts/run_processcontainer_privilege_test.ps1 Adds LPAC and learning-mode coverage.
tests/scripts/run_processcontainer_probes_test.ps1 Adds tier/capability probe coverage.
tests/scripts/run_processcontainer_process_plumbing_test.ps1 Adds process behavior coverage.
tests/scripts/run_processcontainer_proxy_tests.ps1 Removes superseded proxy runner.
tests/scripts/run_processcontainer_ui_mitigations_test.ps1 Adds UI mitigation coverage.
tests/scripts/run_processcontainer_ui_policy_matrix_test.ps1 Adds full UI policy matrix.
tests/scripts/run_pwsh_test.ps1 Uses renamed PowerShell fixture.
Review details

Suppressed comments (3)

tests/scripts/lib/WinProcessContainer.Common.ps1:683

  • This drops an explicitly supplied empty capability list, so the Caps = @() case in run_processcontainer_capabilities_test.ps1 actually tests an omitted processContainer.capabilities field. Preserve parameter presence so that case emits "capabilities": [] and exercises the intended contract.
    if ($Capabilities.Count -gt 0)  { $pc['capabilities']  = @($Capabilities) }

tests/scripts/run_processcontainer_network_egress_test.ps1:98

  • The overlapping deny must target the configured anchor's actual port too. With a non-443 ExternalAnchorUrl, this deny does not overlap the allow/request, so the precedence assertion no longer tests D4.
    tests/scripts/lib/WinProcessContainer.Common.ps1:1091
  • The loopback test must bypass proxy configuration explicitly. Otherwise an ambient curl/WinHTTP proxy can route this request away from 127.0.0.1, making the test report BLOCKED because of proxy behavior rather than hostLoopback enforcement.
    return "$env:SystemRoot\System32\cmd.exe /c `"$curl --silent --show-error --max-time $TimeoutSec --output NUL $Url && echo NET=REACHED || echo NET=BLOCKED`""
  • Files reviewed: 45/51 changed files
  • Comments generated: 7
  • Review effort level: Balanced

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

Comment thread tests/scripts/lib/WinProcessContainer.Common.ps1 Outdated
Comment thread tests/scripts/run_processcontainer_capture_denials_test.ps1 Outdated
Comment thread tests/scripts/run_processcontainer_dacl_disabled_test.ps1 Outdated
Comment thread tests/scripts/run_processcontainer_network_egress_test.ps1 Outdated
Comment thread tests/scripts/run_processcontainer_network_proxy_test.ps1 Outdated
Comment thread tests/scripts/README.md Outdated
Comment thread tests/scripts/lib/WinProcessContainer.Native.ps1 Outdated
Copilot AI review requested due to automatic review settings September 18, 2026 23:54
@theelliotm
Elliot (theelliotm) marked this pull request as ready for review September 19, 2026 00:19
@theelliotm
Elliot (theelliotm) requested a review from a team September 19, 2026 00:19
@theelliotm
Elliot (theelliotm) requested a review from a team as a code owner September 19, 2026 00:19
@azure-pipelines

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

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.

Copilot review overview

🟡 Changes recommended

Several new assertions can produce false results or leave claimed ingress behavior untested.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 High severity

Open (1)
Resolved since last review (4)
Previously missed (3)

In code that hasn't changed since last review

Medium severity Test ingress and private-destination capability enforcement

tests/​scripts/​run_processcontainer_network_capability_test.ps1:57

Every case fetches the same public ExternalAnchorUrl, so these assertions only distinguish egress.default. In particular, allow/deny and allow/allow both succeed through internetClient even if ingress.default is ignored and privateNetworkClientServer is never granted. Add a private-network inbound or private-destination differential so the advertised ingress half of the capability matrix is exercised.

Medium severity Exercise denied ingress and loopback controls

tests/​scripts/​run_processcontainer_network_model3_test.ps1:66

This equality compares only the three public-egress verdicts. Model 3 also promises denied inbound and denied host loopback, so an empty or omitted block could accidentally default either ingress control to allow and this would still pass. Exercise those controls for all three spellings before calling them equivalent.

Medium severity Avoid false failure when sandbox blocks cmd.exe initialization

tests/​scripts/​run_processcontainer_ui_mitigations_test.ps1:159

The launch marker is emitted by cmd.exe, but that wrapper is itself the mitigated sandbox process. This suite already notes that cmd.exe may fail to initialize under ui.disable=true, so a correctly enforced mitigation can prevent the marker and make $ranB false. Use positive runner evidence that CreateProcess succeeded, or have the probe emit and flush a pre-user32 marker while invoking it directly.

Comment thread tests/scripts/run_processcontainer_process_plumbing_test.ps1 Outdated
Copilot AI review requested due to automatic review settings September 21, 2026 21:28

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.

Copilot review overview

🟡 Changes recommended

The native helper does not compile, and one fallback assertion can still report a false success.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 High severity · 1 Medium severity · 1 Low severity

Open (3)
Resolved since last review (1)

Comment thread tests/scripts/lib/WinProcessContainer.Native.ps1
Comment thread tests/scripts/run_processcontainer_privilege_test.ps1
Comment thread src/core/wxc_common/src/filesystem_dacl.rs Outdated
Copilot AI review requested due to automatic review settings September 21, 2026 21:44

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.

Copilot review overview

🟡 Changes recommended

The suite still has unsafe destructive probes, possible environment disclosure, and false-positive or false-negative test paths.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 3 High severity

Open (3)
Resolved since last review (3)
Previously missed (2)

In code that hasn't changed since last review

Medium severity Fail unsupported User32 loading instead of silently skipping injection tests

src/​testing/​wxc_ui_probe/​src/​main.rs:264

This new INJECTION=INCONCLUSIVE path is treated by both PowerShell consumers as the known foreground-ownership race and therefore recorded as a skip. On ui.disable=false, failure to load user32.dll is not that race and should fail the test; otherwise a broken GUI/probe setup silently removes all injection coverage. Distinguish this reason in the two harnesses (or emit a distinct outcome) so only the documented foreground race is skipped.

Medium severity Preserve ingress support capability and enforce expected proxy outcomes

tests/​scripts/​lib/​WinProcessContainer.Common.ps1:807

This predicate collapses “BaseContainer/PSEC selected” and “PSEC 1.1 ingress is supported.” The probe already exposes baseContainerSupportsIngressHostLoopbackAllow, but it is dropped from $Script:Caps; consequently the loopback and identity-less proxy tests accept either success or clean rejection even on a host that advertises support, allowing a regression that rejects valid host-loopback policy to pass. Preserve that probe fact and require success when it is true, rejection when false.

Comment thread tests/scripts/run_processcontainer_network_proxy_test.ps1
Comment thread tests/scripts/run_processcontainer_ui_mitigations_test.ps1
Comment thread tests/scripts/run_processcontainer_ui_policy_matrix_test.ps1
@jsidewhite

Copy link
Copy Markdown
Member

Less-Privileged AppContainer (LPAC) test runner.

should we delete this test? it tests nothing (LPAC isn't supported)


Refers to: tests/scripts/run_lpacac_test.ps1:4 in ecb9267. [](commit_id = ecb9267, deletion_comment = False)

param(
[string]$ContextJson,

[string]$ResultsJson,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

$ResultsJson

is this used?

[string]$ContextJson,

[string]$ResultsJson,
[string]$RequireTier,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

$RequireTier

is this used?

[string]$RequireTier,
[switch]$SkipNetwork,
[switch]$KeepArtifacts
)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

any used?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

:shipit:

Elliot (theelliotm) and others added 2 commits September 21, 2026 16:56
The omitted-capabilities case emits no processContainer object at all, so
reading .processContainer threw under Set-StrictMode -Version Latest and
failed Phase 12b with an unhandled exception. Route both lookups through a
presence-checked helper, matching the PSObject.Properties guard convention
used elsewhere in the suite.

Co-authored-by: Copilot <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.

Copilot review overview

🟡 Changes recommended

Inconclusive injection handling can hide probe failures, the schema boundary test is stale, and a T3 network lane has an irrelevant reachability prerequisite.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 4 Medium severity

Open (4)
Resolved since last review (3)

Comment thread tests/scripts/run_processcontainer_lifecycle_test.ps1
Comment thread tests/scripts/run_processcontainer_network_egress_test.ps1
Comment thread tests/scripts/run_processcontainer_ui_mitigations_test.ps1
Comment thread tests/scripts/run_processcontainer_ui_policy_matrix_test.ps1
…8-network-coverage

# Conflicts:
#	src/core/wxc_common/src/config_parser.rs
Copilot AI review requested due to automatic review settings September 22, 2026 05:17

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.

Comment on lines +1235 to +1237
} else {
$Script:AnchorReachable = Test-HostCanReachAnchor
if (-not $Script:AnchorReachable) {
Comment on lines +62 to +65
$cfg = New-Config -Name $name -CommandLine (Get-AnchorFetchCommand) `
-ReadWrite $fs.ReadWrite -ReadOnly $fs.ReadOnly `
-EgressDefault $case.Egress -IngressDefault $case.Ingress `
-HostLoopback 'deny' -TimeoutMs 30000
Comment on lines +90 to +93
$cfgCap = New-LegacyConfig -Name 'net07-explicit-capability' -CommandLine $cmd `
-DefaultPolicy 'allow' -EnforcementMode 'capabilities' `
-Rest @{ ReadWrite = $fs.ReadWrite; ReadOnly = $fs.ReadOnly
Capabilities = @('internetClient'); TimeoutMs = 30000 }
Comment on lines +100 to +104
$accepted = ($allow.Verdict -eq 'REACHED')
$rejected = Test-WasRejected $allow
Record-Result -Phase 'P8d' -Name 'hostLoopback=allow is either enforced or rejected, never silently dropped' `
-Pass ($accepted -or $rejected) `
-Detail "verdict=$($allow.Verdict); exit=$($allow.Result.ExitCode); timedOut=$($allow.Result.TimedOut); accepted=$accepted rejected=$rejected"
Comment on lines +94 to +98
$rejectedCleanly = Test-WasRejected $envRun
Record-Result -Phase 'P8e' -Name 'identity-less proxy config either runs or is rejected cleanly (never a bare OS error)' `
-Pass ($ran -or $rejectedCleanly) `
-Detail ("ran=$ran; rejectedAtValidation=$rejectedCleanly; exit=$($envRun.Result.ExitCode); " +
"stderr=$(Format-Snippet $envRun.Result.Stderr)")

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants