Skip to content

fix: validate and confine service mount paths - #3655

Open
helix-nine wants to merge 1 commit into
masterfrom
fix/mount-path-validation
Open

fix: validate and confine service mount paths#3655
helix-nine wants to merge 1 commit into
masterfrom
fix/mount-path-validation

Conversation

@helix-nine

@helix-nine helix-nine commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Normalize the paths handed to the host-side service mount effect and verify the resolved result stays under its intended base directory (the target package volume for the source, the calling container's rootfs for the mountpoint). Well-formed callers (the SDK only generates plain relative paths) are unaffected.
  • util::io::canonicalize, which tolerates a not-yet-existing tail, now folds parent-dir components in that tail lexically instead of re-appending them verbatim, so the returned path always matches what the kernel resolves.
  • Keep the internal bind-mount helper on the local start-container CLI only, which is how container-runtime invokes it.
  • Unit tests for the path handling; changelog entry under 0.4.0.2.

Test plan

  • cargo test -p start-core --features=test --lib — all pass, including in the pinned build container with the CI command (--release --features=test,dev,unstable)
  • make start-core-format (pinned nightly) and prettier both clean

@helix-nine
helix-nine force-pushed the fix/mount-path-validation branch from e6f9eb4 to c9b202c Compare August 10, 2026 23:13
dr-bonez
dr-bonez previously approved these changes Aug 10, 2026
@helix-nine

Copy link
Copy Markdown
Contributor Author

Thanks — for the record, the approval at HEAD c9b202c covers the final tree (the restructure push predates it). Build matrix is still running; leaving the merge to you once it's green.

@helix-nine

Copy link
Copy Markdown
Contributor Author

Heads-up on the new head (7aa66a5): the test job failed on the approved head with a linker error (undefined hidden symbol: futures_channel…poll_canceled) that reproduces deterministically in the pinned start9/cargo-zigbuild build container (rustc 1.93.0-beta.5) but not on stable 1.94 — a toolchain codegen bug, not a code defect. Bisected it in-container: the functional change builds clean; what trips it is the effects-handler-surface regression test I had added (any of three formulations — the test perturbs test-binary codegen into the bug).

Delta since the approval: that test is dropped, nothing else changed. The remaining unit tests cover the path handling itself, and the exact committed tree was re-verified with the CI command in the same container (--release --features=test,dev,unstable: 556 passed). The container's beta toolchain is worth bumping independently — any unrelated change can trip the same bug. Sorry for the re-review round-trip.

Resolve the paths handed to the host-side mount effect and verify the
result stays under its base directory (the target volume for the source,
the caller's rootfs for the mountpoint).

util::io::canonicalize, which tolerates a not-yet-existing tail, now folds
parent-dir components in that tail lexically instead of re-appending them
verbatim, so the returned path always matches what the kernel resolves.

Also limit the internal bind-mount helper to the local start-container CLI,
where container-runtime invokes it, instead of registering it on the shared
effects handler.
@helix-nine
helix-nine force-pushed the fix/mount-path-validation branch from 7aa66a5 to 0228066 Compare August 11, 2026 20:33
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