fix(conformance): repair protected platform producer launch - #92
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
There are a few correctness/robustness issues in the updated Windows Invoke-Checked working-directory guard and Unix supervisor test scratch-dir creation that should be addressed before merging.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Repairs Phase 1 protected-platform producer bootstraps across macOS/Linux/Windows conformance runs by hardening launch working-directory semantics, binding isolated dependency stores, and rebinding conformance lock authority to a new behavior commit.
Changes:
- Re-pin Phase 1 conformance harness authority and expected evidence bytes to revision
95d00af123e1fa8670f29e9ddf7f9dec246beb0c. - Harden producer bootstrap behavior: macOS starts from
/before UID switching; Linux install binds to isolated pnpm store; WindowsInvoke-CheckedsetsProcessStartInfo.WorkingDirectory. - Add regression tests covering the above bootstrap invariants.
File summaries
| File | Description |
|---|---|
| src/phase1-conformance-lock.test.ts | Updates expected harness authority revision/tree/file digests to match the new lock target. |
| src/client-v1-conformance-workflow.test.ts | Adds regression assertions for macOS trusted CWD, Unix pnpm store binding, and Windows working directory propagation. |
| scripts/unix-producer-supervisor.test.sh | Adjusts scratch workspace setup and adds a macOS inaccessible-CWD regression exercise. |
| scripts/unix-producer-supervisor.sh | Starts macOS restricted producer from / via a subshell before switching into the copied workspace. |
| scripts/unix-producer-supervisor-attack.c | Adds runtime assertions that the producer starts in the copied workspace and can read a tracked fixture. |
| scripts/unix-producer-command.sh | Binds pnpm install to --config.store-dir="$PNPM_STORE_DIR" for restricted installs. |
| phase1-conformance.lock.json | Re-pins harness revision and updates recorded evidence blob/sha256 values. |
| docs/phase1-conformance.md | Updates documentation to reflect the bootstrap behavior changes and refreshed byte/digest pins. |
| .github/workflows/client-v1-conformance.yml | Sets Windows child process working directory from a validated FileSystem provider path. |
Review details
- Files reviewed: 9/9 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| $location = Get-Location | ||
| $workingDirectory = $location.ProviderPath | ||
| if ( | ||
| $location.Provider.Name -cne 'FileSystem' -or | ||
| [String]::IsNullOrWhiteSpace($workingDirectory) -or | ||
| -not [IO.Path]::IsPathFullyQualified($workingDirectory) | ||
| ) { | ||
| throw "$Label working directory is not an absolute filesystem path." | ||
| } |
There was a problem hiding this comment.
This remains fail-closed before any child creation: the embedded script starts in the supervisor-validated bootstrap filesystem directory, and every later Push-Location target is separately ownership/reparse validated. If that invariant is violated, StrictMode/property access terminates the bootstrap before Process.Start; it cannot bypass the guard or select an attacker-controlled CWD. Keeping the assignment adjacent also avoids a second mutable location lookup.
| scratch_parent="$project_root/test-results/unix-producer-supervisor" | ||
| mkdir -p "$scratch_parent" | ||
| scratch_root="$scratch_parent/run-$$" | ||
| mkdir "$scratch_root" | ||
| scratch_root="$(cd "$scratch_root" && pwd -P)" |
There was a problem hiding this comment.
Fixed in 6fa8eea: the harness is back on mktemp -d under the previously proven /tmp root, and Linux native containment now passes.
| descriptor = open("tracked.txt", O_RDONLY | O_CLOEXEC); | ||
| if (descriptor < 0) fail("open tracked fixture"); | ||
| count = read(descriptor, bytes, sizeof(bytes)); | ||
| if (count != 8 || memcmp(bytes, "fixture\n", 8) != 0) { | ||
| fprintf(stderr, "copied workspace fixture is unavailable\n"); | ||
| exit(1); | ||
| } |
There was a problem hiding this comment.
The mismatch branch terminates the fixture process immediately with exit(1), so the kernel closes the descriptor and no long-lived process or resource leak remains. The success path explicitly closes and checks the descriptor. This is test-fixture hygiene rather than a correctness or containment defect.
Keep the native supervisor harness outside the checked-out source tree while retaining the inaccessible-CWD regression and print the captured supervisor refusal on assertion failure. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
CI follow-up |
|
Reopening immediately to trigger the ci:full labeled platform matrix on the unchanged head. |
Summary
/before changing identity, then enter only the isolated copied workspaceProcessStartInfochild95d00af123e1fa8670f29e9ddf7f9dec246beb0cFailed-run evidence
Repairs the three producer bootstrap failures from protected run 33875324744. That run remains rejected and contributes no release evidence.
Merge constraint
Do not squash.
phase1-conformance.lock.jsondirectly references behavior commit95d00af123e1fa8670f29e9ddf7f9dec246beb0c; merge or rebase must preserve that commit identity and reachability.Validation
Closes the runtime-bootstrap repair portion of OpenCoven/sdk#38.