Repository navigation
Reject an unmappable WSLc one-shot process.cwd - #1300
Merged
Merged
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Interior NULs can still truncate the working directory passed to the WSLc SDK.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
This PR makes WSLc one-shot launches reject unsupported process.cwd values instead of silently ignoring them.
Changes:
- Centralizes one-shot working-directory mapping and validation.
- Applies validation to buffered and streaming launches with regression tests.
- Documents distinct one-shot and state-aware path formats.
| File | Description |
|---|---|
wsl_container_runner.rs |
Validates and applies mapped cwd values. |
sandbox.rs |
Tests early streaming rejection. |
policy_mapping.rs |
Adds cwd mapping and validation helper. |
wsl-container-getting-started.md |
Documents WSLc cwd behavior. |
schema.md |
Clarifies backend-specific cwd formats. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The implementation consistently rejects unmappable values before normal launch paths and includes focused regression coverage and documentation.
Review effort: Balanced
Findings: None
Resolved since last review (1)
The WSLc one-shot runner mapped process.cwd from a local Windows drive path to /mnt/<drive> inside an if-let with no else branch, so any other value (an in-container path such as /workspace, a relative or drive-relative path, or a UNC path) was dropped and the workload ran in the container's default working directory. Add policy_mapping::container_working_directory as the single owner of the rule: a blank value is treated as omitted, a drive path is mapped, and any other value is an error that names it. validate_runner calls it, so both run-to-completion and streaming launches reject the value as policy_validation before any container work. start_container uses the same helper through a match whose Err arm keeps that entry point fail-closed. Document the one-shot cwd form in docs/schema.md and the WSL guide. Fixes #902 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The mapped cwd is handed to the WSLc SDK as a C string, so a value such as C:\work\0ignored was truncated to /mnt/c/work and the workload ran in a directory other than the one the request named. container_working_directory now rejects an interior NUL, matching the state-aware cstr_bytes guard. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4d5bc4fd-28cd-4313-be79-c61367eac45e
Gudge (MGudgin)
force-pushed
the
user/gudge/wslc_oneshot_cwd_fail_loud
branch
from
September 27, 2026 04:20
c5a81b8 to
715d585
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📖 Description
WSLc one-shot maps
process.cwdfrom a local Windows drive path to/mnt/<drive>. Any other value (an in-container path such as/workspace, a relative or drive-relative path, or a UNC path) was silently dropped, and the workload ran in the container's default working directory.This change keeps the existing one-shot path form and makes an unmappable value fail the launch:
policy_mapping::container_working_directoryowns the rule: a blank value is treated as omitted, a drive path is mapped, and any other value returns an error that names it.validate_runnercalls the helper, so run-to-completion and streaming launches (JSON configs,mxc-sdk, FFI) reject the value aspolicy_validationbefore any container work.start_containeruses the same helper through amatchwhoseErrarm keeps that entry point fail-closed.docs/schema.mdand the WSL guide document the one-shot and state-aware cwd forms.process.cwd"", whitespace-onlyC:\work,C:/work,C:/mnt/c/...(unchanged)/workspace,work,C:work,\\server\share🔗 References
Fixes #902. The cross-backend relative-cwd defect is tracked in #1299.
🔍 Validation
cargo test -p wslc_common: 242 passed, including new helper (with interior-NUL rejection),validate_runner, and streamingspawnregression tests. Thespawntest asserts that the rejection happens beforestart_containerruns.cargo clippy -p wslc_common --all-targets -- -D warningscargo fmt --all -- --check✅ Checklist
Cargo.lock, thedependency-feed-checkcheck passes (see docs/pull-requests.md)📋 Issue Type
Microsoft Reviewers: Open in CodeFlow