Repository navigation
feat(mxc): pass driver_config denied_paths to MXC deniedPaths - #5033
shailendra-nv merged 3 commits into
Conversation
A read-write grant covers everything below it, so a file the workload must not see inside a granted folder, such as an agent profile's .env or auth.json, is readable, and an absent one can be created there. The MXC driver already builds MXC's filesystem.deniedPaths but always sends an empty list. Add an optional denied_paths list to the driver-owned driver_config.mxc block and pass it to MXC. Preflight normalizes each entry (backslash separators, no repeated or trailing separator, case-insensitive duplicates removed) and rejects, before any sandbox state is created: - entries that are not absolute paths starting with a drive letter, or that have a '.' or '..' component, a component ending in a dot or space, or a character Windows does not allow in file names; - entries that equal or contain a filesystem grant (a policy grant, the generation staging directory under cwd, or the sandbox executable): MXC drops a grant equal to a denied path and keeps access to a grant nested inside a denied directory. MXC matches paths after resolving 8.3 short names and junctions and ignoring letter case, so both sides are resolved with std::fs::canonicalize (through the deepest existing ancestor for absent paths) and compared ignoring Unicode letter case; a path that cannot be resolved is compared as written; - entries whose parent directory does not exist, which MXC refuses at launch (CreateProcessSecurityEnvironment, HRESULT 0x80070003). The error names the missing directory to deny instead. When the list is not empty, also send fallback.allowDaclMutation=false. MXC 1.0 defaults it to true, and on a host whose base container cannot enforce deniedPaths it would otherwise add deny entries to host file ACLs. The list is empty by default, so existing requests are unchanged. Document the field in the driver README and the gateway configuration page. Signed-off-by: Jeffrey Quesnelle <emozilla@nousresearch.com>
|
@shailendra-nv could you review this one? It needs #5032 accepted and linked in the Development sidebar ( |
|
All contributors have signed the DCO ✍️ ✅ |
|
I have read the DCO document and I hereby sign the DCO. |
shailendra-nv
left a comment
shailendra-nv
left a comment
There was a problem hiding this comment.
Requesting changes for two reproduced cases where an accepted deny-list configuration still exposes the intended protected contents: renaming a writable parent directory, and a device-namespace junction grant that bypasses overlap validation. Before this PR, requests containing denied_paths were rejected; these findings concern the newly accepted configurations.
Validation on native Windows ARM64:
| Suite | Total | Passed | Failed | Ignored |
|---|---|---|---|---|
| PR MXC library unit tests | 70 | 70 | 0 | 0 |
| Additional overlap regression | 1 | 0 | 1 | 0 |
| Total test executions | 71 | 70 | 1 | 0 |
The library suite ran with cargo test --offline --locked --target aarch64-pc-windows-msvc -p openshell-driver-mxc --lib -- --test-threads=1. All nine new deny-path tests passed, so please clarify or correct the PR description's statement that eight fail.
Separate direct real-MXC probes reproduced both findings using dummy files, fallback.allowDaclMutation=false, and the driver's default ProcessContainer privilege setting. These probes used MXC 1.0.0+7bf21024; the author's MXC 1.0.1 environment was not verified. These were filesystem enforcement probes, not full gateway E2E tests. If a newer MXC release fixes either behavior, the driver needs to enforce that minimum version or capability before accepting the configuration.
MXC applies deniedPaths and filesystem grants to paths. The workload can
rename a directory when the directory and its parent are both writable
to it, and everything below that directory then sits at a path that no
denied path or read-only grant names. Measured with MXC 1.0.0 and 1.0.1:
with C:\work granted read-write and C:\work\profile\.env denied, the
workload renames profile and reads the file under the new name.
Preflight now rejects a denied path below such a directory: two or more
levels below a read-write grant root, inside a read-write grant nested in
another one, or below a read-only grant whose root is two or more levels
inside a read-write grant. The most specific grant or denied path decides
a directory's access. The error names the directory and says to deny it
or to move the entry directly into the read-write grant root. Entries
directly inside a read-write grant root whose parent the workload cannot
write, and entries below a read-only grant root that cannot be renamed,
are still accepted.
Grants written as \\.\C:\, \\?\C:\, \??\C:\, \\?\GLOBALROOT\Device\...
or \\?\Volume{...}\, which MXC resolves, are now resolved like drive
paths before the overlap and rename checks, so a device-path grant
through a junction no longer hides an overlap with a denied directory.
While denied_paths is not empty, a UNC grant (MXC grants no access
through UNC paths) or a grant that does not resolve to a local drive is
rejected.
wxc_exec_real gains a probe-gated test that passes seven layouts through
preflight and, for each one it accepts, renames each denied path and every
directory above it in a real ProcessContainer and checks that the denied
contents stay unreadable.
The docs state that MXC does not check the target of a rename against
deniedPaths: in a writable directory the workload can rename its own
file onto a denied name, which replaces a denied file or creates an
absent denied path without exposing the original contents.
Signed-off-by: Jeffrey Quesnelle <emozilla@nousresearch.com>
|
Thanks for the review and the repros. Both findings are fixed in
#5032 still needs |
|
/ok to test |
|
/windows-ci |
@shailendra-nv, there was an error processing your request: See the following link for more information: https://docs.gha-runners.nvidia.com/cpr/e/1/ |
|
/ok to test e01a7cf |
shailendra-nv
added
the
test:windows
ChatGPT Codex Connector
Windows ARM64 CI: ✅ passedCommit
|
|
Validation at
CI and merge status:
No PR source changes were made for this validation. This is a test-results update; merge readiness is still pending. |
shailendra-nv
left a comment
shailendra-nv
left a comment
There was a problem hiding this comment.
Approved for the MXC denied-path changes at e01a7cf. The writable-ancestor rename and device-namespace overlap findings from my earlier review are fixed, their regression coverage was verified on native Windows ARM64, and both review threads are resolved. The recorded validation includes 75 passing driver unit tests, 10 passing gateway E2E scenarios, and 13 successful real-MXC tests with one runtime skip. Accepted issue #5032 is now linked.
The documented MXC rename-target limitation remains: replacing a denied file by renaming another file onto its name can affect integrity, while the original denied contents remain unreadable; callers needing protection from replacement must deny the containing directory.
This approval supersedes my earlier change request. Merge remains pending the Windows CI failure in the unchanged SQLite concurrency test (the Windows adjustment from #4862 is absent from this base), completion of the separate Windows CI run, and the repository's required approval from a listed maintainer.
|
/ok to test 8b18f49 |
Summary
The MXC driver accepts an optional
denied_pathslist indriver_config.mxcand passes it to MXC asfilesystem.deniedPaths, so a caller can hide files inside a filesystem grant, such as an agent profile's.envorauth.json. Preflight refuses entries MXC would resolve differently than requested, and with denied paths the driver tells MXC not to fall back to changing host file ACLs. Without the field, the MXC request is unchanged.Related Issue
Closes #5032
Changes
MxcSandboxConfig(crates/openshell-driver-mxc/src/driver.rs) gainsdenied_paths: Vec<String>with#[serde(default)];deny_unknown_fieldsstays, so an older driver rejects the field instead of ignoring it.InvalidArgumentfor an entry that is not an absolute drive path (C:\...), has a./..component or a component ending in a dot or space, or contains a character Windows does not allow in file names.check_denied_paths()runs invalidate_sandbox_create()before any state is created:InvalidArgumentfor an entry that equals or contains a filesystem grant MXC receives (policyread_write/read_onlypaths, thecwd\.openshell-runtimestaging directory, the sandbox executable);FailedPreconditionfor an entry whose parent directory does not exist, naming the outermost missing directory to deny instead.std::fs::canonicalize: junctions, symbolic links, 8.3 short names, on-disk case; an absent path resolves through its deepest existing ancestor), ignoring Unicode letter case, because MXC applies its overlap rules to resolved paths. Unresolvable parts are compared as written. The driver sends MXC the entries as written.oneshot_config_json()(mxc.rs) adds"fallback": {"allowDaclMutation": false}when there are denied paths. MXC 1.0 defaults it to true, so on a host whose base container cannot enforcedeniedPathsit would otherwise write deny entries into host file ACLs.boundary_filesystem()builds the ProcessContainer filesystem rules thatrun_lifecycle_inner()built inline, now with the denied paths instead of an empty list.denied_pathsin the driver README and indocs/how-it-works/gateways/configuration.mdxnext to the existing MXC--driver-config-jsonexample.The check runs at create time only; a policy update that adds filesystem paths reaches the workload through a new sandbox, which runs preflight again.
Why these rules
Measured with MXC 1.0.1 one-shot ProcessContainers (
--probetierbase-container,baseContainerSupportsDenyPaths: true):deniedPathsentryCreateProcessSecurityEnvironment failed (HRESULT = 0x80070003)::$DATA, trailing dot,\\?\,\\.\,GLOBALROOT, volume GUID, a symlink it creates)The two overlap shapes resolve in opposite directions and neither hides part of a grant, so preflight refuses both.
Why driver config
This follows RFC 0006: the MXC driver owns the nested schema and validation, the gateway forwards the
mxcblock unchanged, and the sandbox policy format does not change. Likecommandandcwd, it needsallow_driver_config = true, and it only removes access. #3221 put UI controls in a portable policy section with a driver capability; the deny list stays in driver config because its rules are MXC's and the Linux drivers' Landlock enforcement cannot remove a path below a grant. If you prefer the #3221 shape, the field can move tofilesystem_policywith a capability and the validation stays as is. On Linux, the Docker and Podman drivers already cover this case by bind-mounting an empty read-only file over the path through their own driver config (gateway opt-inenable_bind_mounts). MXC has no mount primitive, sodeniedPathsis its equivalent.Testing
Windows 11 x64 build 26340.9767, MXC 1.0.1, base
windows5d1bfbf92; Windows checks throughtasks/scripts/windows-msvc.ps1.windows:lint:x64,windows:test:x64(6639 passed, 41 skipped),windows:build:x64;cargo fmt --check; Linuxcargo clippy -p openshell-driver-mxc --all-targets -- -D warnings; markdownlint, license headers,check_docs_nav.py,fern check.driver.rs(absolute-path and spelling rejections, grant overlap, missing parent, 8.3 names, junctions, Unicode case, unresolvable paths, and the MXC request JSON includingfallback). The eight that compile onwindowsfail there;cargo test -p openshell-driver-mxc --libpasses 70.examples/run-mxc-e2e.ps1default scenarios pass 7/7 before and after (with two local shims for this host, not part of this PR).work\read-write runningtype profile\.env,echo x> profile\.env,echo x> profile\auth.json: all succeed withoutdenied_paths; withdenied_paths: [work\profile\.env, work\profile\auth.json]all printAccess is denied., the host files are untouched andprofile\notes.txtstays readable.denied_paths: [work], a missing parent, the 8.3 name ofwork's parent, and a host junction toworkall failsandbox createwith the errors above. The MXC request carries"fallback": {"allowDaclMutation": false}only when there are denies.Checklist