feat(policy): discover default-location dev-tool caches in available_tools_policy by caarlos0 · Pull Request #673 · microsoft/mxc

caarlos0 · 2026-07-23T17:17:01Z

📖 Description

Adds default-location developer-tool cache discovery to mxc_engine's
available_tools_policy (re-exported from mxc-sdk), so sandboxed builds can
read the toolchain caches/registries they need even when the user has not
exported CARGO_HOME / GOPATH / RUSTUP_HOME / … for the existing env-var
discovery to find.

available_tools_policy now merges two sources into one FilesystemPolicyResult:

Env-var + PATH discovery (always on, as before). Now additionally scopes
credential-bearing tool homes — currently CARGO_HOME — to their safe
build-input subdirectories instead of granting the whole root, so a token
stored beside the caches (~/.cargo/credentials.toml) is no longer readable.
Default-location home caches (opt-in via a new allow_dev_tool_caches
parameter, default off). Covers cargo/rustup, go, npm/pnpm/yarn/bun/deno +
Node version managers (nvm/fnm/volta/asdf/mise), pyenv/pipx/virtualenv,
Maven/Gradle/sdkman, .NET/NuGet, Ruby (gem/rbenv/rvm), Conan, and node-gyp.

Key properties:

Credential-safe scoping — only build-input subdirectories are granted,
never tool-home roots, so ~/.cargo/credentials.toml, ~/.npmrc,
~/.m2/settings.xml, ~/.gradle/gradle.properties, ~/.gem/credentials,
~/.nuget/NuGet.Config, ~/.config/gh, ~/.ssh, ~/.gnupg, and the .NET
CurrentUser X509 store all stay unreadable.
Read vs write — read-only by default; only scratch caches a build writes on
every run are read-write (go-build, ccache, sccache, ~/.gradle/caches +
wrapper/dists, and Cargo's root-level .package-cache/.global-cache lock
files). These are provisioned by the new materialize_tool_cache_writes so a
first build's grant survives a downstream existence filter; it uses
create_new to avoid truncating a concurrently-populated lock file.
Trusted home, by construction — the home is always the process's own
(std::env::home_dir), never a caller- or command-env-controlled value,
closing a symlink-redirect vector.
Unix-only — Windows layouts differ and are covered by the existing
%LOCALAPPDATA% env-var discovery; available_tools_policy returns no home
caches there.

The discovery function stays pure (no filesystem writes) so callers can
inspect/serialize the policy without side effects; materialization is a separate,
explicit step.

🔗 References

No linked issue. The default-location path list is a curated allow-list of common
toolchain cache/registry locations; the read/write split and credential-safe
scoping are documented inline in mxc_engine::policy.

🔍 Validation

cargo fmt --all -- --check — clean.
cargo clippy -p mxc_engine -p mxc-sdk --all-targets -- -D warnings — clean.
cargo test -p mxc_engine -p mxc-sdk — all pass, including new tests for the
platform path shapes (macOS Library/Caches vs Linux XDG), the read/write
classification (gradle caches + wrapper/dists, go-build), the credential
exclusion / ~/.dotnet narrowing / CARGO_HOME scoping, shim libexec
grants, the trusted-home containment guard, the allow_dev_tool_caches
on/off cases, and the materialize_tool_cache_writes dir-vs-file creation.

✅ 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 (no dependency changes)

📋 Issue Type

Bug fix
Feature
Task

Microsoft Reviewers: Open in CodeFlow

…tools_policy Add an opt-in second discovery source to `available_tools_policy` covering language-toolchain caches/registries/toolchains in their default home locations (cargo, go, npm, Maven, .NET, Ruby, Conan, Node version managers, …), so sandboxed builds work without the user exporting CARGO_HOME/GOPATH/RUSTUP_HOME/… The new grants are gated behind an `allow_dev_tool_caches` flag (default off); the existing env-var/PATH discovery stays always-on. - Credential-safe scoping: only build-input subdirectories are granted, never tool-home roots, so credential/key material stays unreadable (~/.cargo/credentials.toml, ~/.npmrc, ~/.m2/settings.xml, ~/.gradle/ gradle.properties, the .NET CurrentUser X509 store, …). The env-var half now scopes credential-bearing homes (CARGO_HOME) to safe subdirs too, rather than granting the whole root. - Read-only by default; only scratch caches a build writes on every run are read-write (go-build, ccache, sccache, ~/.gradle/caches + wrapper/dists, and Cargo's root-level .package-cache/.global-cache lock files). These are provisioned via the new `materialize_tool_cache_writes` so a first build's grant survives a downstream existence filter; it uses create_new to avoid truncating a concurrently-populated lock file. - The home is always the trusted process home (std::env::home_dir), never a caller- or command-env-controlled value, closing a symlink-redirect vector. - Unix-only; Windows layouts are covered by the existing %LOCALAPPDATA% env-var discovery. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 521575da-2087-4e55-87d3-28436b60ff4a Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>

Copilot

Pull request overview

This PR extends the Rust mxc_engine policy-discovery helpers to optionally include a curated allow-list of default-location developer tool caches under the user’s home directory, while tightening CARGO_HOME grants to credential-safe subdirectories and adding an explicit materialization step for write-cache candidates.

Changes:

Updated available_tools_policy to (a) scope credential-bearing tool homes like CARGO_HOME to safe subdirectories and (b) optionally merge in default-location home cache paths via a new allow_dev_tool_caches flag.
Added materialize_tool_cache_writes to provision candidate write-cache paths (dirs + Cargo lock/tracker files) before a deny-by-default sandbox run.
Updated mxc-sdk re-exports/docs and adjusted SDK helper tests for the new available_tools_policy signature.

Show a summary per file

File	Description
src/core/mxc-sdk/tests/sdk_helpers.rs	Updates tests for the new `available_tools_policy(env, allow_dev_tool_caches)` API.
src/core/mxc-sdk/src/lib.rs	Re-exports the new `materialize_tool_cache_writes` helper from `mxc_engine`.
src/core/mxc-sdk/README.md	Documents the new dev-tool-cache opt-in and the need to materialize write-cache grants.
src/core/mxc_engine/src/policy.rs	Core implementation: credential-safe scoping for `CARGO_HOME`, opt-in home cache discovery, write-candidate materialization, and extensive tests.
src/core/mxc_engine/src/lib.rs	Re-exports `materialize_tool_cache_writes` from the engine crate.

Review details

Comments suppressed due to low confidence (1)

src/core/mxc_engine/src/policy.rs:775

materialize_tool_cache_writes detects Cargo lock/tracker files via path.ends_with(...) against CARGO_RW_FILE_SUFFIXES (which are written with /). If a Windows-style path (e.g. from CARGO_HOME) uses \, this check won’t match and the code will fall through to create_dir_all, creating a directory at .package-cache / .global-cache instead of touching a file. Consider detecting by Path::file_name() so it’s separator-agnostic and always treats these as files.

        if CARGO_RW_FILE_SUFFIXES
            .iter()
            .any(|suffix| path.ends_with(suffix))
        {

Files reviewed: 5/5 changed files
Comments generated: 1
Review effort level: Low

Condense the doc comments and inline notes added for the dev-tool-cache discovery to keep the essential "why" (trusted-home, credential-safe scoping, create_new non-truncation) without the padding. Comment-only; no behavior change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 521575da-2087-4e55-87d3-28436b60ff4a Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>

caarlos0 · 2026-07-23T19:39:46Z

PS: I kind of wish these would be optional... thoughts?

Match the read-write Cargo lock/tracker files (`.package-cache`/`.global-cache`) on the final path component via `Path::file_name` instead of a `/`-prefixed string suffix. A relocated `CARGO_HOME` on Windows yields `\`-separated paths, so the old suffix match missed them and `create_dir_all`'d the lock file as a directory. Rename `CARGO_RW_FILE_SUFFIXES` -> `CARGO_RW_FILE_NAMES` and drop the misleading "Windows never matches" note. Adds a platform-native-separator regression test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 521575da-2087-4e55-87d3-28436b60ff4a Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>

bbonaby · 2026-07-25T18:41:13Z

-    let env = env_pairs(&[("PATH", &path_val), ("CARGO_HOME", &cwd)]);
+    let env = env_pairs(&[("PATH", &path_val), ("GOROOT", &cwd)]);

-    let result = available_tools_policy(Some(&env));
+    let result = available_tools_policy(Some(&env), false);


question: any reason to change the env variable name in the test?

bbonaby · 2026-07-26T05:31:42Z

+/// (the compiler scratch caches, `~/.gradle/caches` + `wrapper/dists`, and Cargo's
+/// root-level lock files). Cold dependency fetches stay a per-command bypass.
+#[cfg(not(target_os = "windows"))]
+fn home_tool_cache_policy_for(home: &str, macos: bool) -> HomeToolCachePolicy {


question: What issue in GitHub copilot made you want to spin up this PR for the home tool stuff? Was it not being able to build rust projects? Just trying to understand what this all now enables is all.

bbonaby · 2026-07-27T02:17:15Z

+    let join = |rel: &str| format!("{home}/{rel}");
+
+    // Read-only build inputs common to macOS and Linux.
+    const COMMON_READONLY: &[&str] = &[


suggestion: would probably prefer these constants to be in their own file outside of the function itself for readability and maintainability of the function itself

bbonaby · 2026-07-27T02:38:44Z

+pub fn materialize_tool_cache_writes(readwrite_paths: &[String]) -> Vec<String> {
+    for path in readwrite_paths {
+        let is_cargo_lock_file = Path::new(path)
+            .file_name()
+            .and_then(|name| name.to_str())
+            .is_some_and(|name| CARGO_RW_FILE_NAMES.contains(&name));
+        if is_cargo_lock_file {
+            if let Some(parent) = Path::new(path).parent() {
+                let _ = std::fs::create_dir_all(parent);
+            }
+            // `create_new` never truncates, so a `.package-cache`/`.global-cache`
+            // populated concurrently (or by Cargo) is not clobbered as
+            // `File::create` would; `AlreadyExists`/errors are ignored.
+            let _ = std::fs::OpenOptions::new()
+                .write(true)
+                .create_new(true)
+                .open(path);
+        } else {
+            let _ = std::fs::create_dir_all(path);
+        }
+    }
+    readwrite_paths
+        .iter()
+        .filter(|path| Path::new(path).exists())
+        .cloned()
+        .collect()
+}


Question: In this function, are we preemptively creating read-write paths that do not already exist because the sandboxed AI workload may need to create those directories or files during execution?

For example, if the workload were building a Rust project with Cargo, the current behavior could result in an access-denied error when Cargo tries to create a required directory. With this change, are we creating those directories ahead of time so the workload can write to them?

huzaifa-d · 2026-07-27T21:10:08Z

+        // drop non-existent ones (matching the env-var discovery). The write
+        // buckets stay unfiltered so `materialize_tool_cache_writes` can create
+        // them before a first build needs them.
+        policy.readonly.retain(|dir| directory_exists(dir));


This retain uses directory_exists, and ".rustup/settings.toml" on line 542 is a file, not a directory, so it is always filtered out and never reaches a real policy.

The test home_tool_cache_policy_for_grants_shim_libexec_and_narrows_dotnet asserts on the pure table rather than the filtered output, so it passes and hides this.

huzaifa-d · 2026-07-27T21:10:09Z

+/// created before a deny-by-default sandbox can use them; call
+/// [`materialize_tool_cache_writes`] on the returned
+/// [`readwrite_paths`](FilesystemPolicyResult::readwrite_paths) to do so.
+pub fn available_tools_policy(


Since this is a breaking public API change, an options struct or a enum, instead of a bool, which is descriptive would be better.

huzaifa-d · 2026-07-27T21:10:10Z

+/// [`readwrite_paths`](FilesystemPolicyResult::readwrite_paths). This is the one
+/// side-effecting step of discovery, kept separate so discovery stays pure;
+/// idempotent and best-effort. Returns the subset that now exists.
+pub fn materialize_tool_cache_writes(readwrite_paths: &[String]) -> Vec<String> {


This is pub, takes arbitrary caller-supplied paths, create_dir_all's them, follows symlinks, and discards every error with let _ = and no Logger. Expected?

Copilot AI review requested due to automatic review settings July 23, 2026 17:17

caarlos0 requested a review from a team as a code owner July 23, 2026 17:17

Copilot started reviewing on behalf of caarlos0 July 23, 2026 17:17 View session

Copilot AI reviewed Jul 23, 2026

View reviewed changes

Comment thread src/core/mxc_engine/src/policy.rs Outdated

asklar reviewed Jul 23, 2026

View reviewed changes

Comment thread src/core/mxc_engine/src/policy.rs

bbonaby reviewed Jul 27, 2026

View reviewed changes

huzaifa-d reviewed Jul 27, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(policy): discover default-location dev-tool caches in available_tools_policy - #673

feat(policy): discover default-location dev-tool caches in available_tools_policy#673
caarlos0 wants to merge 3 commits into
microsoft:mainfrom
caarlos0:tools

caarlos0 commented Jul 23, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

caarlos0 commented Jul 23, 2026

Uh oh!

bbonaby Jul 25, 2026

Uh oh!

bbonaby Jul 26, 2026

Uh oh!

bbonaby Jul 27, 2026

Uh oh!

bbonaby Jul 27, 2026

Uh oh!

huzaifa-d Jul 27, 2026

Uh oh!

huzaifa-d Jul 27, 2026

Uh oh!

huzaifa-d Jul 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

caarlos0 commented Jul 23, 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!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Review details

Uh oh!

Uh oh!

Uh oh!

caarlos0 commented Jul 23, 2026

Uh oh!

bbonaby Jul 25, 2026

Choose a reason for hiding this comment

Uh oh!

bbonaby Jul 26, 2026

Choose a reason for hiding this comment

Uh oh!

bbonaby Jul 27, 2026

Choose a reason for hiding this comment

Uh oh!

bbonaby Jul 27, 2026

Choose a reason for hiding this comment

Uh oh!

huzaifa-d Jul 27, 2026

Choose a reason for hiding this comment

Uh oh!

huzaifa-d Jul 27, 2026

Choose a reason for hiding this comment

Uh oh!

huzaifa-d Jul 27, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

caarlos0 commented Jul 23, 2026 •

edited

Loading