Skip to content

ci(windows): gate packaged sandbox lifecycle evidence - #3558

Open
liugddx wants to merge 1 commit into
apache:mainfrom
liugddx:ci/windows-sandbox-lifecycle-gate
Open

ci(windows): gate packaged sandbox lifecycle evidence#3558
liugddx wants to merge 1 commit into
apache:mainfrom
liugddx:ci/windows-sandbox-lifecycle-gate

Conversation

@liugddx

@liugddx liugddx commented Aug 23, 2026

Copy link
Copy Markdown
Member

Summary

Promotes existing Windows sandbox lifecycle evidence into the shared packaged verifier used by both the pull-request Release Windows check and the formal release workflow.

The verifier runs appcontainer-smoke.ps1 and acl-recovery-smoke.ps1 against the exact broker under win-unpacked. Those existing probes cover concurrent AppContainer launches, process/Job drain, residual ACE and ledger cleanup, and recovery after the broker is killed mid-launch. The pull-request path filter includes the packaged worker and Windows boundary source closure so relevant changes cannot skip this gate.

This changes release verification only. It does not modify Runtime, broker, launch, cancellation, retry, ACL, or sandbox policy behavior.

Refs #2142

Remaining Phase 4 work

This PR intentionally does not close the Phase 4 lifecycle checkbox. Explicit client cancellation, Runtime Host death during launch, sustained concurrency soak, unsettled-state recovery/quarantine, the wider adversarial matrix, and independent human security review remain separate follow-up gates.

Verification

Exact head 229d09c72, rebased without conflicts onto main 6e5e7562e after #3265 and the upstream ASF-header fix:

  • node --test scripts/ci-test-plan.test.mjs scripts/verify-windows-harness.test.mjs: 64 passed, 0 failed, 1 privilege-dependent symlink skip.
  • biome lint .: 2,611 files, no findings.
  • biome format .: 1,600 files, no changes.
  • ASF source-header audit: 2,745 covered / 130 reviewed exclusions.
  • unified release workflow contract: 1 passed.
  • Release workflow YAML and changed JavaScript syntax parsed successfully.
  • git diff --check passed.
  • range-diff confirms the rebase preserves this PR's implementation; the only harness context change is feat(win): Abort-path installer rollback with backup retention #3265's independent registry-ownership test, and both tests pass together.

L3 on the exact head:

AI use

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Codex traced the existing Windows evidence and release ownership, implemented the shared packaged gate and trigger contract, rebased it after #3265, retained both independent harness contracts, ran the local quality gate, and performed an author-side adversarial review. The commit includes a Generated-by: Codex trailer. AI review is not independent human security review.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, syntax checks, and affected platform-neutral suites pass locally
  • Fresh packaged Windows L3 passes on this exact head

Does this PR entail a change in behavior?

  • Yes - Windows package and formal release verification now fail when the packaged sandbox lifecycle evidence fails.
  • No

@liugddx
liugddx marked this pull request as ready for review August 23, 2026 06:24
@liugddx
liugddx force-pushed the ci/windows-sandbox-lifecycle-gate branch from 22c7ad6 to 3ce3d4d Compare August 23, 2026 08:46
@liugddx

liugddx commented Aug 23, 2026

Copy link
Copy Markdown
Member Author

Could a maintainer rerun the failed Release Windows check job for exact head 3ce3d4d35dea88d3b62ec66716e9aea7149d2fc1?

The failure occurred in the existing check:release suite during Package the Windows installer and ZIP, before this PR's packaged lifecycle verifier ran. verify-packaged-app.test.mjs expected the notice-version rejection ([email protected]) but one fixture read returned the earlier PTY package-identity error instead. Neither verify-packaged-app.test.mjs nor verify-packaged-app.mjs is in this PR's diff.

Evidence for rerun rather than a scope-expanding fix here:

The contributor token cannot rerun upstream Actions (Must have admin rights to Repository), and I do not want to add an empty CI-only commit. If it repeats, that is evidence to fix the existing asar fixture determinism separately; this PR still needs a fresh green packaged run before its L3 box is checked.

@liugddx
liugddx force-pushed the ci/windows-sandbox-lifecycle-gate branch from 3ce3d4d to 2862abf Compare August 23, 2026 09:42
@liugddx
liugddx force-pushed the ci/windows-sandbox-lifecycle-gate branch from 2862abf to 229d09c Compare August 23, 2026 10:41

@Astro-Han Astro-Han 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.

Review at exact head 229d09c727f4719c70b01e5e975023ffc09408a6.

The gate itself is sound. Both PowerShell verifiers are awaited in sequence and any non-zero exit rejects through runCommand, so missing or malformed lifecycle evidence fails closed rather than passing vacuously — the failure mode that makes a gate decorative. I confirmed the exact-head hosted run 32634441543 (job 97181975711) really executed the lifecycle step, and the evidence it gates on comes from that same fresh package: there is no path by which a stale artifact from an earlier run could satisfy it.

[P1] The path allowlist doesn't cover the sources the gate actually exercises

The new trigger entries reach the packaged worker and the Windows boundary driver:

- 'packages/runtime/src/filesystem-worker/**'
- 'packages/runtime/src/sandbox/**'
- 'packages/core/src/permission-profile.ts'
- 'packages/core/src/permission-profile-compiler.ts'

But the code in those directories imports Core modules that are not listed. Directly:

  • packages/core/src/sandbox-boundary.ts → imported by filesystem-worker/client.ts, operations.ts, protocol.ts
  • packages/core/src/windows-path.ts → imported by sandbox/windows-profile.ts, sandbox/windows-sandbox.ts

and one level further, sandbox-boundary.ts imports serialized-byte-length.js while windows-path.ts imports absolute-path.js. The Runtime side has the same gap for child-fd-input.ts, child-process-lifecycle.ts and process-tree-terminator.ts.

From Core, only the two permission-profile* files are listed. So a change confined to sandbox-boundary.ts — the module that defines the boundary this gate exists to protect — triggers neither pre-merge Windows lane. The W0 PR filter doesn't cover it either. A sandbox-boundary regression would land on main and surface only in the nightly, which is precisely the "found at the worst possible moment" situation the workflow header says this path was built to prevent.

This is worth raising because the PR is otherwise strictly an improvement; the gap is that the allowlist is a hand-copied prefix of the real dependency closure, and hand-copied closures drift the moment someone adds an import.

Suggested direction: rather than appending these seven files, derive the trigger set from the actual import graph of the packaged E2E entrypoints, and add a contract test asserting that every source reachable from those entrypoints appears in the workflow's paths. That converts "someone remembered to update the list" into a checked property, and it fails loudly when a new import escapes the gate. If deriving it is too large for this PR, adding the seven files plus that contract test would still close the drift.

Nothing else found. Verification: exact-head test and package are completed/success; 65/65 targeted tests plus a fake-rejection probe covering missing and malformed second-stage evidence. Windows packaging, PowerShell, the Rust broker, AppContainer/ACL, installer/update/rollback and the full repo suite were not run locally and are not claimed.

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.

2 participants