Skip to content

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

Open
caarlos0 wants to merge 3 commits into
microsoft:mainfrom
caarlos0:tools
Open

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

Conversation

@caarlos0

@caarlos0 caarlos0 commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

📖 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:

  1. 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.
  2. 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 AI review requested due to automatic review settings July 23, 2026 17:17
@caarlos0
caarlos0 requested a review from a team as a code owner July 23, 2026 17:17

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/core/mxc_engine/src/policy.rs Outdated
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>
Comment thread src/core/mxc_engine/src/policy.rs
@caarlos0

Copy link
Copy Markdown
Contributor Author

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>
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.

3 participants