Reject embedded sandbox paths in Rust compiler outputs

Scan code-generating rustc outputs for the resolved ${pwd} value after successful compilation. Reject embedded CARGO_MANIFEST_DIR, OUT_DIR, and other sandbox paths while preserving compile-time include_str! and relative paths.

Inspect cargo_build_script OUT_DIR through test runfiles instead of retaining compile-action paths.

Assisted-by: OpenAI Codex

Author: dzbarsky

cargo/tests/cargo_build_script/location_expansion/build.rs

@@ -1,18 +1,30 @@
+fn execpath(name: &str) -> String {
+    let path = std::env::var(name).expect("Environment variable not set");
+    assert!(std::path::Path::new(&path).is_absolute());
+    assert!(std::path::Path::new(&path).exists());
+
+    let normalized = path.replace('\\', "/");
+    let (_, relative) = normalized
+        .split_once("/bazel-out/")
+        .expect("execpath does not contain bazel-out");
+    format!("bazel-out/{}", relative)
+}
+
 fn main() {
     println!(
         "cargo:rustc-env=DATA_ROOTPATH={}",
         std::env::var("DATA_ROOTPATH").expect("Environment variable not set")
     );
     println!(
         "cargo:rustc-env=DATA_EXECPATH={}",
-        std::env::var("DATA_EXECPATH").expect("Environment variable not set")
+        execpath("DATA_EXECPATH")
     );
     println!(
         "cargo:rustc-env=TOOL_ROOTPATH={}",
         std::env::var("TOOL_ROOTPATH").expect("Environment variable not set")
     );
     println!(
         "cargo:rustc-env=TOOL_EXECPATH={}",
-        std::env::var("TOOL_EXECPATH").expect("Environment variable not set")
+        execpath("TOOL_EXECPATH")
     );
 }

cargo/tests/cargo_build_script/location_expansion/test.rs

@@ -16,37 +16,14 @@ pub fn test_tool_rootpath() {
 
 #[test]
 pub fn test_execpath() {
-    // Replace `\` to ensure paths are consistent on Windows.`
-    let data_execpath = env!("DATA_EXECPATH").replace('\\', "/");
-    let tool_execpath = env!("TOOL_EXECPATH").replace('\\', "/");
-
-    let data_path = data_execpath
-        .split_at(
-            data_execpath
-                .find("/bazel-out/")
-                .unwrap_or_else(|| panic!("Failed to parse execroot from: {}", data_execpath)),
-        )
-        .1;
-    let tool_path = tool_execpath
-        .split_at(
-            tool_execpath
-                .find("/bazel-out/")
-                .unwrap_or_else(|| panic!("Failed to parse execroot from: {}", tool_execpath)),
-        )
-        .1;
-
-    let (data_cfg, data_short_path) = data_path.split_at(
-        data_path
-            .find("/bin/")
-            .unwrap_or_else(|| panic!("Failed to find bin in {}", data_path))
-            + "/bin/".len(),
-    );
-    let (tool_cfg, tool_short_path) = tool_path.split_at(
-        tool_path
-            .find("/bin/")
-            .unwrap_or_else(|| panic!("Failed to find bin in {}", tool_path))
-            + "/bin/".len(),
-    );
+    let data_execpath = env!("DATA_EXECPATH");
+    let tool_execpath = env!("TOOL_EXECPATH");
+    let (data_cfg, data_short_path) = data_execpath
+        .split_once("/bin/")
+        .unwrap_or_else(|| panic!("Failed to find bin in {}", data_execpath));
+    let (tool_cfg, tool_short_path) = tool_execpath
+        .split_once("/bin/")
+        .unwrap_or_else(|| panic!("Failed to find bin in {}", tool_execpath));
 
     assert_ne!(
         data_cfg, tool_cfg,

cargo/tests/cargo_build_script/nondeterministic_out_dir/BUILD.bazel

@@ -10,6 +10,8 @@ cargo_build_script(
 rust_test(
     name = "test",
     srcs = ["test.rs"],
+    data = [":build_script"],
     edition = "2021",
+    env = {"BUILD_SCRIPT_OUT_DIR": "$(rootpath :build_script)"},
     deps = [":build_script"],
 )

cargo/tests/cargo_build_script/nondeterministic_out_dir/test.rs

@@ -2,11 +2,23 @@
 //! If the runner failed to strip config.log / *.d / *.pc files, the TreeArtifact hash
 //! would change on every run, causing unnecessary rebuilds for all downstream crates.
 
+use std::path::PathBuf;
+
 const OUTPUT: &str = include_str!(concat!(env!("OUT_DIR"), "/output.txt"));
 
+fn build_script_out_dir() -> PathBuf {
+    PathBuf::from(
+        std::env::var_os("BUILD_SCRIPT_OUT_DIR").expect("BUILD_SCRIPT_OUT_DIR is not set"),
+    )
+}
+
 #[test]
 fn legitimate_output_survives_nondeterministic_file_removal() {
     assert_eq!(OUTPUT, "legitimate output");
+    assert_eq!(
+        std::fs::read_to_string(build_script_out_dir().join("output.txt")).unwrap(),
+        OUTPUT,
+    );
 }
 
 // Verify that volatile files written by the build script are absent from the
@@ -17,59 +29,59 @@ fn legitimate_output_survives_nondeterministic_file_removal() {
 #[test]
 fn config_log_removed() {
     assert!(
-        !std::path::Path::new(concat!(env!("OUT_DIR"), "/config.log")).exists(),
+        !build_script_out_dir().join("config.log").exists(),
         "config.log should have been removed from OUT_DIR"
     );
 }
 
 #[test]
 fn config_status_removed() {
     assert!(
-        !std::path::Path::new(concat!(env!("OUT_DIR"), "/config.status")).exists(),
+        !build_script_out_dir().join("config.status").exists(),
         "config.status should have been removed from OUT_DIR"
     );
 }
 
 #[test]
 fn makefile_removed() {
     assert!(
-        !std::path::Path::new(concat!(env!("OUT_DIR"), "/Makefile")).exists(),
+        !build_script_out_dir().join("Makefile").exists(),
         "Makefile should have been removed from OUT_DIR"
     );
 }
 
 #[test]
 fn makefile_config_removed() {
     assert!(
-        !std::path::Path::new(concat!(env!("OUT_DIR"), "/Makefile.config")).exists(),
+        !build_script_out_dir().join("Makefile.config").exists(),
         "Makefile.config should have been removed from OUT_DIR"
     );
 }
 
 #[test]
 fn config_cache_removed() {
     assert!(
-        !std::path::Path::new(concat!(env!("OUT_DIR"), "/config.cache")).exists(),
+        !build_script_out_dir().join("config.cache").exists(),
         "config.cache should have been removed from OUT_DIR"
     );
 }
 
 #[test]
 fn dot_d_files_removed() {
     assert!(
-        !std::path::Path::new(concat!(env!("OUT_DIR"), "/foo.d")).exists(),
+        !build_script_out_dir().join("foo.d").exists(),
         "foo.d should have been removed from OUT_DIR"
     );
     assert!(
-        !std::path::Path::new(concat!(env!("OUT_DIR"), "/baz.d")).exists(),
+        !build_script_out_dir().join("baz.d").exists(),
         "baz.d should have been removed from OUT_DIR"
     );
 }
 
 #[test]
 fn dot_pc_file_removed() {
     assert!(
-        !std::path::Path::new(concat!(env!("OUT_DIR"), "/foo.pc")).exists(),
+        !build_script_out_dir().join("foo.pc").exists(),
         "foo.pc should have been removed from OUT_DIR"
     );
 }

cargo/tests/cargo_build_script/tools_exec/build.rs

@@ -36,11 +36,12 @@ fn main() {
     test_encoded_rustflags();
     test_toolchain_var();
 
-    // Pass the TOOL_PATH along to the rust_test so we can assert on it.
-    println!(
-        "cargo:rustc-env=TOOL_PATH={}",
-        std::env::var("TOOL").unwrap()
-    );
+    // Pass the execution-root-relative tool path to the rust_test.
+    let tool_path = std::env::var("TOOL").unwrap().replace('\\', "/");
+    let (_, relative_tool_path) = tool_path
+        .split_once("/bazel-out/")
+        .expect("tool path does not contain bazel-out");
+    println!("cargo:rustc-env=TOOL_PATH=bazel-out/{}", relative_tool_path);
 
     // Assert that the cc and rust toolchain env vars existed and were executable.
     // We don't assert what happens when they're executed (in particular, we don't check for a

cargo/tests/unit/transitive_link_search_paths/transitive_link_search_paths_test.bzl

@@ -3,7 +3,13 @@
 load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts")
 load("//cargo:defs.bzl", "cargo_build_script")
 load("//rust:defs.bzl", "rust_binary", "rust_common", "rust_library", "rust_proc_macro")
-load("//test/unit:common.bzl", "assert_action_mnemonic", "assert_list_contains")
+load(
+    "//test/unit:common.bzl",
+    "assert_action_mnemonic",
+    "assert_list_contains",
+    "assert_list_contains_adjacent_elements",
+    "assert_list_contains_adjacent_elements_not",
+)
 
 def _transitive_link_search_paths_test_impl(ctx):
     env = analysistest.begin(ctx)
@@ -15,6 +21,28 @@ def _transitive_link_search_paths_test_impl(ctx):
     # of the dep of the proc_macro.
     asserts.equals(env, link_search_path_basenames, ["dep_build_script.linksearchpaths"])
 
+    action = tut.actions[0]
+    assert_action_mnemonic(env, action, "Rustc")
+    archive = tut[DefaultInfo].files.to_list()[0]
+    checked_output = archive
+    if any([arg.endswith("-windows-msvc") for arg in action.argv]):
+        object_files = [
+            output
+            for output in action.outputs.to_list()
+            if output.extension in ("o", "obj")
+        ]
+        asserts.equals(env, 1, len(object_files))
+        checked_output = object_files[0]
+        assert_list_contains_adjacent_elements_not(env, action.argv, [
+            "--check-output-for-working-dir",
+            archive.path,
+        ])
+
+    assert_list_contains_adjacent_elements(env, action.argv, [
+        "--check-output-for-working-dir",
+        checked_output.path,
+    ])
+
     return analysistest.end(env)
 
 transitive_link_search_paths_test = analysistest.make(_transitive_link_search_paths_test_impl)

rust/private/rustc.bzl

@@ -1879,11 +1879,24 @@ def rustc_compile(
         distributed_thin_lto and crate_info.type == "bin"
     )
 
-    # output_o is consumed by cc_common.link or exported for distributed
-    # ThinLTO. crate_info.output.basename keeps output_o distinct for targets
-    # that share crate_info.name, including rust_test targets for the same crate.
+    scan_msvc_archive_object = (
+        rust_toolchain.target_abi == "msvc" and
+        (
+            crate_info.type == "staticlib" or
+            (
+                crate_info.type in ("lib", "rlib") and
+                build_info != None and
+                build_info.out_dir != None
+            )
+        )
+    )
+
+    # output_o is consumed by cc_common.link, exported for distributed ThinLTO,
+    # or scanned instead of an MSVC archive containing native-object paths.
+    # crate_info.output.basename keeps output_o distinct for targets that share
+    # crate_info.name, including rust_test targets for the same crate.
     output_o = None
-    if use_cc_common_link or distributed_thin_lto:
+    if use_cc_common_link or distributed_thin_lto or scan_msvc_archive_object:
         output_o = ctx.actions.declare_file(crate_info.output.basename + ".o", sibling = crate_info.output)
 
     runtime_libs = get_cc_toolchain_runtime_libs(cc_toolchain, feature_configuration, crate_info.type, resolve_cc_runtime_linkage(ctx))
@@ -2119,6 +2132,17 @@ def rustc_compile(
         fail("No process wrapper was defined for {}".format(ctx.label))
 
     if not rust_toolchain._bootstrapping:
+        # Search code-generating rustc outputs for the resolved action working
+        # directory. RustcMetadata has separate arguments and no object code.
+        checked_outputs = [output_o] if scan_msvc_archive_object else [outputs[0]]
+        if output_o and output_o != checked_outputs[0]:
+            checked_outputs.append(output_o)
+        args.process_wrapper_flags.add_all(
+            checked_outputs,
+            before_each = "--check-output-for-working-dir",
+            expand_directories = False,
+        )
+
         # Run as normal
         ctx.actions.run(
             executable = process_wrapper,

test/integration/cc_common_link/unit/cc_common_link_test.bzl

@@ -73,10 +73,6 @@ with_exec_cfg = rule(
     },
 )
 
-def _outputs_object_file(action):
-    object_files = [output for output in action.outputs.to_list() if output.extension in ("o", "obj")]
-    return len(object_files) > 0
-
 def _use_cc_common_link_test(ctx):
     env = analysistest.begin(ctx)
     tut = analysistest.target_under_test(env)
@@ -85,7 +81,25 @@ def _use_cc_common_link_test(ctx):
     # When --experimental_use_cc_common_link is enabled the compile+link Rustc action produces a
     # .o/.obj file.
     rustc_action = [action for action in registered_actions if action.mnemonic == "Rustc"][0]
-    asserts.true(env, _outputs_object_file(rustc_action), "Rustc action did not output an object file")
+    object_files = [
+        output
+        for output in rustc_action.outputs.to_list()
+        if output.extension in ("o", "obj")
+    ]
+    asserts.true(env, len(object_files) > 0, "Rustc action did not output an object file")
+    object_file = object_files[0]
+    expected_working_dir_check_arguments = ["--check-output-for-working-dir", object_file.path]
+    asserts.true(
+        env,
+        any([
+            rustc_action.argv[index:index + 2] == expected_working_dir_check_arguments
+            for index in range(len(rustc_action.argv) - 1)
+        ]),
+        "Expected adjacent working-directory check arguments {} in {}".format(
+            expected_working_dir_check_arguments,
+            rustc_action.argv,
+        ),
+    )
 
     has_cpp_link_action = len([action for action in registered_actions if action.mnemonic == "CppLink"]) > 0
     asserts.true(env, has_cpp_link_action, "Expected that the target registers a CppLink action")