[Seatbelt] Fix symlink and parent-traversal policy enforcement by theelliotm · Pull Request #1531 · microsoft/mxc

Elliot (theelliotm) · 2026-10-19T23:43:26Z

📖 Description

Resolve Seatbelt policy symlinks before applying .., preserving deny/read-only precedence across aliases.
Reject broken or inaccessible policy paths before launch; support not-yet-created trailing paths.
Separate macOS host preparation from platform-independent profile generation.
Add 22 executor regression cases to scheduled macOS validation and replace the blanket .. rejection test.
Report shared-resolver errors. Bubblewrap retains its existing deny-mask fallback; DNS-target resolution rejects errors.

🔗 References

No linked issue. Updates docs/backends/seatbelt/seatbelt-backend.md.

🔍 Validation

Command	Result
`cargo test --locked --quiet -p mxc-sdk --lib -- seatbelt`	Passed: 137 tests
`cargo fmt --all -- --check`	Passed
`cargo clippy --locked --quiet -p mxc-sdk --lib --tests -- -D warnings`	Passed
`cargo clippy --locked --quiet -p mxc-sdk --lib --tests --target x86_64-unknown-linux-gnu -- -D warnings`	Passed: Linux cross-target compile/lint
`cargo build --locked --quiet -p mxc_darwin`	Passed
`MXC_EXEC_MAC="$PWD/src/target/debug/mxc-exec-mac" bash tests/scripts/run_seatbelt_symlink_test.sh`	Passed: 22 scenarios
`MXC_EXEC_MAC="$PWD/src/target/debug/mxc-exec-mac" bash tests/scripts/run_seatbelt_options_test.sh`	Passed: 19 assertions, including Keychain access and profile overrides

✅ Checklist

Signed the Contributor License Agreement
Linked to an issue
Updated documentation (if applicable)
Updated Copilot instructions (if build, architecture, or conventions changed)
If this PR changes Cargo.lock, the dependency-feed-check check passes (see pull request builds)

📋 Issue Type

Bug fix
Feature
Task

Microsoft Reviewers: Open in CodeFlow

azure-pipelines · 2026-10-19T23:43:35Z

Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run.

Copilot

🟡 Changes recommended

The existing Linux symlink-loop test expects the previous diagnostic and will fail the proxy-network CI step.

1 open issue

Update symlink loop test for new resolution error · New

What changed in this PR

Corrects Seatbelt filesystem policy enforcement so symlink aliases and parent traversal preserve access restrictions.

Changes:

Resolves policy paths before profile generation and rejects resolution errors.
Separates macOS host preparation from platform-independent profile rendering.
Adds 22 executor regression scenarios and updates backend documentation.

File	Description
tests/scripts/run_seatbelt_symlink_test.sh	Adds symlink enforcement regressions.
tests/scripts/run_seatbelt_path_resolution_test.sh	Tests supported parent traversal.
tests/scripts/run_seatbelt_all_tests.sh	Includes the symlink suite.
tests/configs/seatbelt_path_dotdot.json	Exercises a parent-traversal grant.
src/mxc-sdk/src/core/mxc_common/mod.rs	Enables shared resolution on macOS.
src/mxc-sdk/src/core/mxc_common/filesystem_symlink.rs	Records path resolution failures.
src/mxc-sdk/src/backends/seatbelt/common/seatbelt_runner.rs	Prepares inputs before rendering.
src/mxc-sdk/src/backends/seatbelt/common/profile_preparation.rs	Resolves paths and discovers host directories.
src/mxc-sdk/src/backends/seatbelt/common/profile_builder.rs	Renders prepared profile inputs.
src/mxc-sdk/src/backends/seatbelt/common/mod.rs	Registers macOS preparation.
src/mxc-sdk/src/backends/bubblewrap/common/proxy_network.rs	Rejects DNS-target resolution errors.
src/mxc-sdk/src/backends/bubblewrap/common/bwrap_runner.rs	Logs failures while preserving mask fallback.
docs/backends/seatbelt/seatbelt-backend.md	Documents resolution and rejection behavior.

_{Review effort: Balanced}

Copilot

🔵 Needs a closer look

Host-specific containment behavior and the remaining path-validation concern require final human review.

No open issues 🎉

1 previously missed issue

In code that hasn't changed since last review

Validate symlink targets before canonicalizing readwrite paths

src/mxc-sdk/src/core/mxc_common/filesystem_symlink.rs:54

[P2] symlink_metadata checks the link itself, not whether its target can be traversed. For a link whose stored target is file/../secret, with file a regular file and secret an existing sibling, macOS canonicalization can fold .. without rejecting the invalid traversal. A readwritePaths entry for that link can therefore grant access to secret instead of failing before launch. Check the followed target with metadata before accepting canonicalization, and extend the macOS executor regression cases to reject parent traversal inside a symlink target.

1 resolved since last review

Update symlink loop test for new resolution error

_{Review effort: Balanced}

Huzaifa Danish (huzaifa-d) · 2026-10-20T17:05:33Z

The symlink-target validation concern in #1531 (review) looks worth looking into.

Huzaifa Danish (huzaifa-d) · 2026-10-21T18:21:38Z

-        crate::seatbelt_common::profile_builder::expand_tilde(path)
-            .unwrap_or_else(|_| path.to_string())
-    };
+    let expand = |path: &str| expand_tilde(path).unwrap_or_else(|_| path.to_string());


Consider using resolve_policy_path instead of expand_tilde in resolved_working_directory_opt. Otherwise, a working directory with a symlink and .. sets PWD and default HOME to the unresolved path instead of the real directory.

Huzaifa Danish (huzaifa-d) · 2026-10-21T18:21:39Z

    for p in &policy.denied_paths {
        if let Some(resolved) = resolve_through_symlinks(Path::new(p)) {
-            let resolved = resolved.to_str().ok_or_else(|| {
+            if let Some(error) = resolved.error {


Consider stopping .. popping in resolve_through_symlinks once a symlink fails to resolve, or skipping the mask in resolve_denied_paths when resolved.error is set. Right now, a denied path like /work/dangling/../secret pops the broken link and masks the unrelated path /work/secret.

Huzaifa Danish (huzaifa-d) · 2026-10-21T18:21:41Z

            ));
        }
-        let profile = build_profile_with_proxy(request, proxy_address).map_err(error_response)?;
+        let inputs = prepare_profile(request).map_err(error_response)?;


Consider calling prepare_profile in SeatbeltScriptRunner::validate as well as spawn. Currently, mxc-exec-mac --dry-run skips prepare_profile and passes on broken symlinks or loops that fail at launch.

Huzaifa Danish (huzaifa-d) · 2026-10-21T18:21:42Z

+            "HOME environment variable not set; cannot expand '~/Library/Keychains' for keychainAccess"
+                .to_string()
+        })?;
+        Some(format!("{home}/Library/Keychains"))


Consider resolving symlinks on the user_keychains path with resolve_policy_path when keychainAccess is enabled. If HOME sits under a symlinked root like /var/root or /home, the generated Seatbelt rule uses the unresolved path and will not match.

merge seatbelt path resolution with linux's

e59f98f

Copilot AI balanced review requested due to automatic review settings October 19, 2026 23:43

Copilot started reviewing on behalf of Elliot (theelliotm) October 19, 2026 23:44

View session

Copilot AI reviewed Oct 19, 2026

View reviewed changes

Comment thread src/mxc-sdk/src/backends/bubblewrap/common/proxy_network.rs

Elliot (theelliotm) self-assigned this Oct 19, 2026

Elliot (theelliotm) changed the title ~~Fix Seatbelt symlink and parent-traversal policy enforcement~~ [Seatbelt] Fix symlink and parent-traversal policy enforcement Oct 19, 2026

Update bwrap test to reflect early rejection

ef24bac

Copilot AI balanced review requested due to automatic review settings October 19, 2026 23:54

Copilot started reviewing on behalf of Elliot (theelliotm) October 19, 2026 23:54

View session

Copilot AI reviewed Oct 20, 2026

View reviewed changes

Elliot (theelliotm) marked this pull request as ready for review October 20, 2026 00:44

Elliot (theelliotm) requested a review from a team as a code owner October 20, 2026 00:44

Elliot (theelliotm) requested a review from Branden Bonaby (bbonaby) October 20, 2026 00:44

Huzaifa Danish (huzaifa-d) reviewed Oct 21, 2026

View reviewed changes

Navigation Menu

Repository navigation

[Seatbelt] Fix symlink and parent-traversal policy enforcement - #1531

[Seatbelt] Fix symlink and parent-traversal policy enforcement#1531
Elliot (theelliotm) wants to merge 2 commits into
mainfrom
user/emichlin/macos-symlink

Elliot (theelliotm) commented Oct 19, 2026 •

edited

Loading

Uh oh!

azure-pipelines Bot commented Oct 19, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Huzaifa Danish (huzaifa-d) commented Oct 20, 2026

Uh oh!

Huzaifa Danish (huzaifa-d) Oct 21, 2026

Uh oh!

Huzaifa Danish (huzaifa-d) Oct 21, 2026

Uh oh!

Huzaifa Danish (huzaifa-d) Oct 21, 2026

Uh oh!

Huzaifa Danish (huzaifa-d) Oct 21, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Elliot (theelliotm) commented Oct 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📖 Description

🔗 References

🔍 Validation

✅ Checklist

📋 Issue Type

Microsoft Reviewers: Open in CodeFlow

Uh oh!

azure-pipelines Bot commented Oct 19, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Huzaifa Danish (huzaifa-d) commented Oct 20, 2026

Uh oh!

Huzaifa Danish (huzaifa-d) Oct 21, 2026

Choose a reason for hiding this comment

Uh oh!

Huzaifa Danish (huzaifa-d) Oct 21, 2026

Choose a reason for hiding this comment

Uh oh!

Huzaifa Danish (huzaifa-d) Oct 21, 2026

Choose a reason for hiding this comment

Uh oh!

Huzaifa Danish (huzaifa-d) Oct 21, 2026

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Elliot (theelliotm) commented Oct 19, 2026 •

edited

Loading