Repository navigation
fix(sandbox): reserve /run, /run/nvx, and the /run/nvx/time tree as share targets - #579
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
The focused changes consistently enforce the intended path boundaries, with regression coverage and no identified blocking issues.
0 open findings
What changed in this PR
Fixes #486 by reserving runtime-owned sandbox share targets, preventing hidden shares and workload startup failures.
Changes:
- Rejects
/run,/run/nvx, and the/run/nvx/timetree in host and guest validation. - Adds regression cases while preserving unrelated
/runtargets.
| File | Description |
|---|---|
| scripts/test_nvx_tools.py | Adds reserved-target and allowed-target regression cases. |
| scripts/nvx_tools/sandbox.py | Extends host-side reserved mount targets. |
| guest/common/nvx-init-agent | Applies matching guest-side restrictions. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Thanks for this fix! A heads-up: #597 (PR 4 of the #359 stack) moves |
452853c to
fc608d9
Compare
fc608d9 to
b002b50
Compare
b002b50 to
b5e3cab
Compare
b5e3cab to
8e07207
Compare
8e07207 to
3dfccb9
Compare
3dfccb9 to
3eb19ab
Compare
73fea09 to
acffa84
Compare
acffa84 to
bc020d5
Compare
bc020d5 to
4dbeeb0
Compare
Open PR #579 (jay23606, fixes #486) reserves the guest paths that the container runtime of a layered sandbox creates or binds over: it creates /run and /run/nvx below the container root, and binds its time ABI state over /run/nvx/time. A live share at one of them would be shadowed, or would expose host data to the runtime's writes. #579 changes scripts/nvx_tools/sandbox.py, which #359 retires, so this ports its behavior to the launch profile that replaced it, as the maintainer asked while #579 is open: - nvx::host::sandbox::mount: RESERVED_MOUNT_TARGETS gains the /run/nvx/time tree, and RESERVED_EXACT_MOUNT_TARGETS /run and /run/nvx, with #579's reasons. validate_mount_target refuses them with the messages that the Python check gave: `sandbox mount target /run is reserved`, and `... overlaps the reserved /run/nvx/time tree` for the tree. Other paths below /run, such as /run/secrets, /run/nvx/other, or /run/nvx/timer, stay valid. - guest/common/nvx-init-agent refuses the same targets, as #579 does, so that a guest refuses them even from a host that would not. - The guest-agent test of aggregate children gains #579's rows for /run, /run/nvx, and /run/nvx/time/state. - doc/usage.md and doc/run.md list the new reserved targets, which #579 left undocumented. tests/fixtures/sandbox.json, which the retired Python launch profile recorded, held nine targets and four shares that #579 now refuses. Its records of those cases are re-evaluated with that Python code, taken from the commit before #359 retired it, with #579's change to sandbox.py applied: 7 targets at /run, one at /run/nvx, and one at /run/nvx/time become refusals, as do 4 shares at /run, and no other record changes. The Rust unit tests check the new refusals, and paths beside them that stay valid. Part of #359. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
PR #576 (jay23606, refs #556) narrowed the sandbox to one EROFS distro layer and the scratch disk: the runtime and custom layers went, and their MMIO windows and interrupts stay unassigned. It merged after #595 had ported the launch profile to nvx-rs, and it changed scripts/nvx_tools/sandbox.py, which #359 retires, the guest agent, the sandbox-blocks scenario, and the docs, but not the launch profile of nvx-rs, which still accepted the three roles. Now that nvx.py sandbox runs nvx-rs, this ports #576's change to that launch profile, with the messages and checks of #576: - nvx::host::sandbox::layer: LAYER_ROLES, the roles that --layer accepts and a launch holds, is distro alone, so `--layer runtime,...` fails with `unsupported layer role 'runtime'; choose distro`, as does any other role. LayerRole keeps the retired Runtime and Custom roles, which from_name still reads, because the configuration of a sandbox that an older nvx.py provisioned may name them. - SandboxLaunch::new checks the duplicate roles first, over every role, and then requires exactly one layer, the distro layer: `a sandbox requires exactly one read-only distro layer`, which replaces `one to three read-only layers`. As in #576, the check holds for a saved launch as much as for parsed options, so `nvx.py sandbox start` of such a sandbox fails before OpenVMM starts. The golden records follow. In tests/fixtures/sandbox.json, the layer and launch cases are re-evaluated with the retired Python launch profile, from the commit before #359 retired it, which holds #576, with #579's change to sandbox.py applied. 168 layer values of the retired roles, 146 of which parsed and 22 of which had an invalid UUID, become refusals of their role, which is checked first; the refusals of 28 values of unknown roles list only distro; and 136 launches with retired roles now fail with the duplicate roles or the distro-layer requirement, which come before their other checks. No other record changes. The launch harness builds a retired role's layer as a saved configuration names it, since --layer no longer parses it. In tests/fixtures/cli.json, by hand, for POSIX and Windows alike: - `validation: invalid layer role` lists only distro, and a new `validation: retired layer role` shows `--layer runtime,...` refused with the same usage error; - `dry run: every layer and option` takes the distro layer alone, so its OpenVMM command loses the runtime and custom blocks and their nvx_layer tokens; - `lifecycle: network and launch options` provisions the distro layer alone, so its config.json and its OpenVMM arguments lose the runtime layer. A new CLI test edits a provisioned sandbox's configuration to add a runtime layer, as an older nvx.py wrote it, and checks that start refuses it with status 1 and the message above, leaving no runtime state, and that deprovision still removes it. Part of #359. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Open PR #579 (jay23606, fixes #486) reserves the guest paths that the container runtime of a layered sandbox creates or binds over: it creates /run and /run/nvx below the container root, and binds its time ABI state over /run/nvx/time. A live share at one of them would be shadowed, or would expose host data to the runtime's writes. #579 changes scripts/nvx_tools/sandbox.py, which #359 retires, so this ports its behavior to the launch profile that replaced it, as the maintainer asked while #579 is open: - nvx::host::sandbox::mount: RESERVED_MOUNT_TARGETS gains the /run/nvx/time tree, and RESERVED_EXACT_MOUNT_TARGETS /run and /run/nvx, with #579's reasons. validate_mount_target refuses them with the messages that the Python check gave: `sandbox mount target /run is reserved`, and `... overlaps the reserved /run/nvx/time tree` for the tree. Other paths below /run, such as /run/secrets, /run/nvx/other, or /run/nvx/timer, stay valid. - guest/common/nvx-init-agent refuses the same targets, as #579 does, so that a guest refuses them even from a host that would not. - The guest-agent test of aggregate children gains #579's rows for /run, /run/nvx, and /run/nvx/time/state. - doc/usage.md and doc/run.md list the new reserved targets, which #579 left undocumented. tests/fixtures/sandbox.json, which the retired Python launch profile recorded, held nine targets and four shares that #579 now refuses. Its records of those cases are re-evaluated with that Python code, taken from the commit before #359 retired it, with #579's change to sandbox.py applied: 7 targets at /run, one at /run/nvx, and one at /run/nvx/time become refusals, as do 4 shares at /run, and no other record changes. The Rust unit tests check the new refusals, and paths beside them that stay valid. Part of #359. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
PR #576 (jay23606, refs #556) narrowed the sandbox to one EROFS distro layer and the scratch disk: the runtime and custom layers went, and their MMIO windows and interrupts stay unassigned. It merged after #595 had ported the launch profile to nvx-rs, and it changed scripts/nvx_tools/sandbox.py, which #359 retires, the guest agent, the sandbox-blocks scenario, and the docs, but not the launch profile of nvx-rs, which still accepted the three roles. Now that nvx.py sandbox runs nvx-rs, this ports #576's change to that launch profile, with the messages and checks of #576: - nvx::host::sandbox::layer: LAYER_ROLES, the roles that --layer accepts and a launch holds, is distro alone, so `--layer runtime,...` fails with `unsupported layer role 'runtime'; choose distro`, as does any other role. LayerRole keeps the retired Runtime and Custom roles, which from_name still reads, because the configuration of a sandbox that an older nvx.py provisioned may name them. - SandboxLaunch::new checks the duplicate roles first, over every role, and then requires exactly one layer, the distro layer: `a sandbox requires exactly one read-only distro layer`, which replaces `one to three read-only layers`. As in #576, the check holds for a saved launch as much as for parsed options, so `nvx.py sandbox start` of such a sandbox fails before OpenVMM starts. The golden records follow. In tests/fixtures/sandbox.json, the layer and launch cases are re-evaluated with the retired Python launch profile, from the commit before #359 retired it, which holds #576, with #579's change to sandbox.py applied. 168 layer values of the retired roles, 146 of which parsed and 22 of which had an invalid UUID, become refusals of their role, which is checked first; the refusals of 28 values of unknown roles list only distro; and 136 launches with retired roles now fail with the duplicate roles or the distro-layer requirement, which come before their other checks. No other record changes. The launch harness builds a retired role's layer as a saved configuration names it, since --layer no longer parses it. In tests/fixtures/cli.json, by hand, for POSIX and Windows alike: - `validation: invalid layer role` lists only distro, and a new `validation: retired layer role` shows `--layer runtime,...` refused with the same usage error; - `dry run: every layer and option` takes the distro layer alone, so its OpenVMM command loses the runtime and custom blocks and their nvx_layer tokens; - `lifecycle: network and launch options` provisions the distro layer alone, so its config.json and its OpenVMM arguments lose the runtime layer. A new CLI test edits a provisioned sandbox's configuration to add a runtime layer, as an older nvx.py wrote it, and checks that start refuses it with status 1 and the message above, leaving no runtime state, and that deprovision still removes it. Part of #359. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Open PR #579 (jay23606, fixes #486) reserves the guest paths that the container runtime of a layered sandbox creates or binds over: it creates /run and /run/nvx below the container root, and binds its time ABI state over /run/nvx/time. A live share at one of them would be shadowed, or would expose host data to the runtime's writes. #579 changes scripts/nvx_tools/sandbox.py, which #359 retires, so this ports its behavior to the launch profile that replaced it, as the maintainer asked while #579 is open: - nvx::host::sandbox::mount: RESERVED_MOUNT_TARGETS gains the /run/nvx/time tree, and RESERVED_EXACT_MOUNT_TARGETS /run and /run/nvx, with #579's reasons. validate_mount_target refuses them with the messages that the Python check gave: `sandbox mount target /run is reserved`, and `... overlaps the reserved /run/nvx/time tree` for the tree. Other paths below /run, such as /run/secrets, /run/nvx/other, or /run/nvx/timer, stay valid. - guest/common/nvx-init-agent refuses the same targets, as #579 does, so that a guest refuses them even from a host that would not. - The guest-agent test of aggregate children gains #579's rows for /run, /run/nvx, and /run/nvx/time/state. - doc/usage.md and doc/run.md list the new reserved targets, which #579 left undocumented. tests/fixtures/sandbox.json, which the retired Python launch profile recorded, held nine targets and four shares that #579 now refuses. Its records of those cases are re-evaluated with that Python code, taken from the commit before #359 retired it, with #579's change to sandbox.py applied: 7 targets at /run, one at /run/nvx, and one at /run/nvx/time become refusals, as do 4 shares at /run, and no other record changes. The Rust unit tests check the new refusals, and paths beside them that stay valid. Part of #359. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
PR #576 (jay23606, refs #556) narrowed the sandbox to one EROFS distro layer and the scratch disk: the runtime and custom layers went, and their MMIO windows and interrupts stay unassigned. It merged after #595 had ported the launch profile to nvx-rs, and it changed scripts/nvx_tools/sandbox.py, which #359 retires, the guest agent, the sandbox-blocks scenario, and the docs, but not the launch profile of nvx-rs, which still accepted the three roles. Now that nvx.py sandbox runs nvx-rs, this ports #576's change to that launch profile, with the messages and checks of #576: - nvx::host::sandbox::layer: LAYER_ROLES, the roles that --layer accepts and a launch holds, is distro alone, so `--layer runtime,...` fails with `unsupported layer role 'runtime'; choose distro`, as does any other role. LayerRole keeps the retired Runtime and Custom roles, which from_name still reads, because the configuration of a sandbox that an older nvx.py provisioned may name them. - SandboxLaunch::new checks the duplicate roles first, over every role, and then requires exactly one layer, the distro layer: `a sandbox requires exactly one read-only distro layer`, which replaces `one to three read-only layers`. As in #576, the check holds for a saved launch as much as for parsed options, so `nvx.py sandbox start` of such a sandbox fails before OpenVMM starts. The golden records follow. In tests/fixtures/sandbox.json, the layer and launch cases are re-evaluated with the retired Python launch profile, from the commit before #359 retired it, which holds #576, with #579's change to sandbox.py applied. 168 layer values of the retired roles, 146 of which parsed and 22 of which had an invalid UUID, become refusals of their role, which is checked first; the refusals of 28 values of unknown roles list only distro; and 136 launches with retired roles now fail with the duplicate roles or the distro-layer requirement, which come before their other checks. No other record changes. The launch harness builds a retired role's layer as a saved configuration names it, since --layer no longer parses it. In tests/fixtures/cli.json, by hand, for POSIX and Windows alike: - `validation: invalid layer role` lists only distro, and a new `validation: retired layer role` shows `--layer runtime,...` refused with the same usage error; - `dry run: every layer and option` takes the distro layer alone, so its OpenVMM command loses the runtime and custom blocks and their nvx_layer tokens; - `lifecycle: network and launch options` provisions the distro layer alone, so its config.json and its OpenVMM arguments lose the runtime layer. A new CLI test edits a provisioned sandbox's configuration to add a runtime layer, as an older nvx.py wrote it, and checks that start refuses it with status 1 and the message above, leaving no runtime state, and that deprovision still removes it. Part of #359. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Jay (@jay23606) please rebase this PR. |
…are targets The host already rejects these targets, but the test only asserted /etc. Add the /run, /run/nvx, and /run/nvx/time cases, and assert that /run/secrets and /run/nvx/other stay allowed. Co-Authored-By: Claude Haiku 5.5 <noreply@anthropic.com> Signed-off-by: jay23606 <4602056+jay23606@users.noreply.github.com>
4dbeeb0 to
20831f3
Compare
|
Hello Jay (@jay23606) I would appreciate if you asked your agents to stop automatically rebasing this. The current workflow is preventing me to run your PRs on our CI and thus preventing them to be merged. |
e033d40
into
microsoft:dev
Fixes #486
Summary
/runand/run/nvxbelow the container root and bind-mounts its time ABI state over/run/nvx/time, after the agent has mounted the live shares. A share at those targets was silently hidden, or every start failed.nvx.py sandboxand the guest agent now reject/run,/run/nvx(exactly) and/run/nvx/time(with everything below it) before boot, as they already do for/etc. Other/runtargets such as/run/secretsand/run/nvx/otherstay allowed.Scope
validate_microvm_guest_mount_targetis not changed here. It lives in the vendoredopenvmm/tree and needs its own change; the host and guest now reject these targets before it matters.Test
test_mount_target_validation_rejects_unsafe_and_reserved_targetsgains the new reserved and allowed cases. It fails ondevand passes here./runrows, which run and pass on this Windows host through Git Bash.python -m unittest test_nvx_tools: 532 pass, 54 skipped as before. ruff, ruff format, and pyright strict on Linux are clean.🤖 Generated with Claude Code