From ab273e89ad6f2bcc58563dd6726766e69bd3f370 Mon Sep 17 00:00:00 2001 From: UebelAndre Date: Tue, 19 May 2026 06:39:34 -0700 Subject: [PATCH 1/2] Fix `OUT_DIR` sanitization in `cargo_build_script` --- .../private/cargo_build_script_runner/lib.rs | 73 ++++++++++---- .../BUILD.bazel | 3 + .../cross_crate_build_script_outputs/bin.rs | 3 + .../bin_link_build.rs | 3 + .../cross_crate_out_dir_files_test_suite.bzl | 94 +++++++++++++++++++ .../cross_crate_build_script_outputs/lib.rs | 1 + .../lib_build.rs | 39 ++++++++ rust/private/rustc.bzl | 23 +++++ 8 files changed, 220 insertions(+), 19 deletions(-) create mode 100644 cargo/tests/cargo_build_script/cross_crate_build_script_outputs/BUILD.bazel create mode 100644 cargo/tests/cargo_build_script/cross_crate_build_script_outputs/bin.rs create mode 100644 cargo/tests/cargo_build_script/cross_crate_build_script_outputs/bin_link_build.rs create mode 100644 cargo/tests/cargo_build_script/cross_crate_build_script_outputs/cross_crate_out_dir_files_test_suite.bzl create mode 100644 cargo/tests/cargo_build_script/cross_crate_build_script_outputs/lib.rs create mode 100644 cargo/tests/cargo_build_script/cross_crate_build_script_outputs/lib_build.rs diff --git a/cargo/private/cargo_build_script_runner/lib.rs b/cargo/private/cargo_build_script_runner/lib.rs index 95659dd24d..5faee0a1f8 100644 --- a/cargo/private/cargo_build_script_runner/lib.rs +++ b/cargo/private/cargo_build_script_runner/lib.rs @@ -222,8 +222,8 @@ impl BuildScriptOutput { CompileAndLinkFlags { compile_flags: compile_flags.join("\n"), - link_flags: Self::redact_paths(&link_flags.join("\n"), exec_root, out_dir), - link_search_paths: Self::redact_paths( + link_flags: Self::redact_flags(&link_flags.join("\n"), exec_root, out_dir), + link_search_paths: Self::redact_flags( &link_search_paths.join("\n"), exec_root, out_dir, @@ -231,24 +231,15 @@ impl BuildScriptOutput { } } - /// Replace the absolute exec-root with `${pwd}` and the relative - /// configuration-dependent `out_dir` path (e.g. - /// `bazel-out//bin/.../_bs.out_dir`) with `${out_dir}`. - /// - /// Both tokens are substituted by `process_wrapper` at action - /// execution time. Routing the `out_dir` portion through - /// `${out_dir}` lets Bazel's path mapping - /// (`--experimental_output_paths=strip`) rewrite it: the consumer - /// `Rustc` action passes the directory to `process_wrapper` via - /// `--out-dir ` from a `File`-typed `Args` entry, so the value - /// is the mapped `bazel-out/cfg/bin/...` path under path mapping and - /// the un-mapped path otherwise. Without this redaction, - /// build-script-emitted env vars (e.g. `cargo::rustc-env=FOO=$OUT_DIR/bar`) - /// would carry the un-mapped path through the `_bs.env` file and - /// cause the path-mapped Rustc action to look in the wrong location - /// at runtime. + fn redact_exec_root(value: &str, exec_root: &str) -> String { + value.replace(exec_root, "${pwd}") + } + + /// Redact for env vars: uses the generic `${out_dir}` token, resolved + /// by `process_wrapper`'s `--out-dir` flag. Safe because env files are + /// only consumed by the target that directly owns the build script. fn redact_paths(value: &str, exec_root: &str, out_dir: &str) -> String { - let with_pwd = value.replace(exec_root, "${pwd}"); + let with_pwd = Self::redact_exec_root(value, exec_root); if out_dir.is_empty() { with_pwd } else { @@ -256,6 +247,21 @@ impl BuildScriptOutput { } } + /// Redact for flags (link flags, link search paths): uses the full + /// `out_dir` relative path as the substitution key so each build + /// script gets a unique token. This avoids collisions when flag files + /// are consumed transitively by a target whose `--out-dir` points to + /// a different build script. The corresponding `--subst` entries are + /// added on the Starlark side for every transitive build info. + fn redact_flags(value: &str, exec_root: &str, out_dir: &str) -> String { + let with_pwd = Self::redact_exec_root(value, exec_root); + if out_dir.is_empty() { + with_pwd + } else { + with_pwd.replace(out_dir, &format!("${{{out_dir}}}")) + } + } + // The process-wrapper treats trailing backslashes as escapes for following newlines. // If the env var ends with a backslash (and accordingly doesn't have a following newline), // escape it so that it doesn't get turned into a newline by the process-wrapper. @@ -457,4 +463,33 @@ cargo::rustc-env=BAR=/abs/exec_root/elsewhere/file.rs "FOO=${pwd}/${out_dir}/op.rs\nBAR=${pwd}/elsewhere/file.rs" ); } + + /// Link search paths use the full `out_dir` path as the substitution + /// key so each build script gets a unique token. This avoids + /// collisions when the flag file is consumed transitively by a target + /// whose `--out-dir` points to a different build script. + #[test] + fn out_dir_in_flags_uses_full_path_as_substitution_key() { + let buff = Cursor::new( + " +cargo::rustc-link-search=/abs/exec_root/bazel-out/cfg/bin/pkg/_bs.out_dir +cargo::rustc-link-search=/abs/exec_root/other/path +", + ); + let reader = BufReader::new(buff); + let result = BuildScriptOutput::outputs_from_reader(reader); + assert_eq!( + BuildScriptOutput::outputs_to_flags( + &result, + "/abs/exec_root", + "bazel-out/cfg/bin/pkg/_bs.out_dir", + ), + CompileAndLinkFlags { + compile_flags: "".to_owned(), + link_flags: "".to_owned(), + link_search_paths: + "-L${pwd}/${bazel-out/cfg/bin/pkg/_bs.out_dir}\n-L${pwd}/other/path".to_owned(), + } + ); + } } diff --git a/cargo/tests/cargo_build_script/cross_crate_build_script_outputs/BUILD.bazel b/cargo/tests/cargo_build_script/cross_crate_build_script_outputs/BUILD.bazel new file mode 100644 index 0000000000..37e3145c08 --- /dev/null +++ b/cargo/tests/cargo_build_script/cross_crate_build_script_outputs/BUILD.bazel @@ -0,0 +1,3 @@ +load(":cross_crate_out_dir_files_test_suite.bzl", "cross_crate_out_dir_files_test_suite") + +cross_crate_out_dir_files_test_suite(name = "cross_crate_out_dir_files_test_suite") diff --git a/cargo/tests/cargo_build_script/cross_crate_build_script_outputs/bin.rs b/cargo/tests/cargo_build_script/cross_crate_build_script_outputs/bin.rs new file mode 100644 index 0000000000..c897a19ea0 --- /dev/null +++ b/cargo/tests/cargo_build_script/cross_crate_build_script_outputs/bin.rs @@ -0,0 +1,3 @@ +fn main() { + println!("Hello!") +} diff --git a/cargo/tests/cargo_build_script/cross_crate_build_script_outputs/bin_link_build.rs b/cargo/tests/cargo_build_script/cross_crate_build_script_outputs/bin_link_build.rs new file mode 100644 index 0000000000..49e7a19779 --- /dev/null +++ b/cargo/tests/cargo_build_script/cross_crate_build_script_outputs/bin_link_build.rs @@ -0,0 +1,3 @@ +fn main() { + println!("cargo:rustc-link-lib=static=cross_crate_lib"); +} diff --git a/cargo/tests/cargo_build_script/cross_crate_build_script_outputs/cross_crate_out_dir_files_test_suite.bzl b/cargo/tests/cargo_build_script/cross_crate_build_script_outputs/cross_crate_out_dir_files_test_suite.bzl new file mode 100644 index 0000000000..851a9ea5e8 --- /dev/null +++ b/cargo/tests/cargo_build_script/cross_crate_build_script_outputs/cross_crate_out_dir_files_test_suite.bzl @@ -0,0 +1,94 @@ +"""Unit tests for transitive OUT_DIR interactions.""" + +load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts") +load("//cargo:defs.bzl", "cargo_build_script") +load("//rust:defs.bzl", "rust_binary", "rust_library") +load( + "//test/unit:common.bzl", + "assert_action_mnemonic", +) + +def _assert_has_out_dir_subst(env, argv, out_dir_name): + """Assert argv has a --subst key=value pair where both sides end with out_dir_name.""" + for i in range(len(argv) - 1): + if argv[i] != "--subst": + continue + parts = argv[i + 1].split("=", 1) + if len(parts) == 2 and parts[0].endswith(out_dir_name) and parts[1].endswith(out_dir_name): + return + asserts.true( + env, + False, + "Expected --subst key=value where both sides end with '{}'".format(out_dir_name), + ) + +def _transitive_out_dir_subst_test_impl(ctx): + """Verify --subst entries exist for each transitive build script out_dir.""" + env = analysistest.begin(ctx) + tut = analysistest.target_under_test(env) + + action = tut.actions[0] + assert_action_mnemonic(env, action, "Rustc") + + _assert_has_out_dir_subst(env, action.argv, "lib_build_script.out_dir") + _assert_has_out_dir_subst(env, action.argv, "bin_link_build_script.out_dir") + + return analysistest.end(env) + +_transitive_out_dir_subst_test = analysistest.make( + _transitive_out_dir_subst_test_impl, + doc = """\ +Test that `--subst` values were used passed which is used to dereference command line +files that point to the OUT_DIR of a transitive dependency. `--subst` is an implementation +detail but as build script flags are written to a file, a starlark unit test cannot test this. +Instead the combination of a successfully built target and the tests here provide sufficient +regression testing. +""", +) + +def cross_crate_out_dir_files_test_suite(name): + """Test suite for cross-crate build script output directory resolution. + + Args: + name: Name of the test suite. + """ + cargo_build_script( + name = "lib_build_script", + srcs = ["lib_build.rs"], + tags = ["manual"], + ) + + rust_library( + name = "lib", + srcs = ["lib.rs"], + deps = [":lib_build_script"], + tags = ["manual"], + ) + + cargo_build_script( + name = "bin_link_build_script", + srcs = ["bin_link_build.rs"], + tags = ["manual"], + ) + + rust_binary( + name = "bin_link", + srcs = ["bin.rs"], + deps = [ + ":bin_link_build_script", + ":lib", + ], + tags = ["manual"], + ) + + _transitive_out_dir_subst_test( + name = "transitive_out_dir_subst_test", + target_under_test = ":bin_link", + ) + + native.test_suite( + name = name, + tests = [ + ":transitive_out_dir_subst_test", + ], + ) diff --git a/cargo/tests/cargo_build_script/cross_crate_build_script_outputs/lib.rs b/cargo/tests/cargo_build_script/cross_crate_build_script_outputs/lib.rs new file mode 100644 index 0000000000..8b13789179 --- /dev/null +++ b/cargo/tests/cargo_build_script/cross_crate_build_script_outputs/lib.rs @@ -0,0 +1 @@ + diff --git a/cargo/tests/cargo_build_script/cross_crate_build_script_outputs/lib_build.rs b/cargo/tests/cargo_build_script/cross_crate_build_script_outputs/lib_build.rs new file mode 100644 index 0000000000..534c887041 --- /dev/null +++ b/cargo/tests/cargo_build_script/cross_crate_build_script_outputs/lib_build.rs @@ -0,0 +1,39 @@ +use std::path::{Path, PathBuf}; +use std::{env, error::Error, fs}; + +fn is_msvc_linker(ld: &Path) -> bool { + ld.file_stem() + .and_then(|s| s.to_str()) + .is_some_and(|name| name.eq_ignore_ascii_case("link")) +} + +fn main() -> Result<(), Box> { + let out = &PathBuf::from(env::var("OUT_DIR")?); + + // Write an empty static archive into OUT_DIR. The downstream + // `bin_link` target links against it via `-l`, forcing the linker to + // locate the file through the transitive link search path. If the + // path resolves incorrectly, linking fails with "library not found". + let ld = env::var("LD").unwrap_or_default(); + if is_msvc_linker(Path::new(&ld)) { + // MSVC's link.exe requires a first-linker-member (symbol + // directory) after the ar magic. + let mut ar = Vec::new(); + ar.extend_from_slice(b"!\n"); + ar.extend_from_slice(b"/ "); // name (16 bytes) + ar.extend_from_slice(b"0 "); // mtime (12 bytes) + ar.extend_from_slice(b"0 "); // uid ( 6 bytes) + ar.extend_from_slice(b"0 "); // gid ( 6 bytes) + ar.extend_from_slice(b"100644 "); // mode ( 8 bytes) + ar.extend_from_slice(b"4 "); // size (10 bytes) + ar.extend_from_slice(b"`\n"); // end ( 2 bytes) + ar.extend_from_slice(&[0, 0, 0, 0]); // 0 symbols (big-endian u32) + fs::write(out.join("libcross_crate_lib.a"), &ar)?; + } else { + // GNU ld / gold / lld / Apple ld all accept bare ar magic. + fs::write(out.join("libcross_crate_lib.a"), b"!\n")?; + } + + println!("cargo:rustc-link-search={}", out.display()); + Ok(()) +} diff --git a/rust/private/rustc.bzl b/rust/private/rustc.bzl index f1a44fe9e5..bbe42c7146 100644 --- a/rust/private/rustc.bzl +++ b/rust/private/rustc.bzl @@ -1106,6 +1106,29 @@ def construct_arguments( expand_directories = False, ) + # Build-script flag files (link search paths, link flags) embed the + # build script's `out_dir` as a `${}` substitution token, using + # the full analysis-time path as the key so each build script gets a + # unique token. Add a `--subst` entry for every direct and transitive + # build script `out_dir` so `process_wrapper` can resolve each token. + # Routing through `File`-typed `Args` entries lets Bazel path mapping + # rewrite the value at execution time. + for dep_build_info in dep_info.transitive_build_infos.to_list(): + if dep_build_info.out_dir: + process_wrapper_flags.add_all( + [dep_build_info.out_dir], + before_each = "--subst", + format_each = dep_build_info.out_dir.path + "=%s", + expand_directories = False, + ) + if out_dir != None: + process_wrapper_flags.add_all( + [out_dir], + before_each = "--subst", + format_each = out_dir.path + "=%s", + expand_directories = False, + ) + # Arguments for launching rustc from the process wrapper. When a `File` is # provided via `tool_file`, add it directly so Bazel's path mapping can # rewrite the location; otherwise fall back to the bare string `tool_path`. From dc5c966541e552ce9263ed4ea2f2f1dbbc5b72c7 Mon Sep 17 00:00:00 2001 From: UebelAndre Date: Thu, 21 May 2026 06:56:28 -0700 Subject: [PATCH 2/2] Update docs --- .../cross_crate_out_dir_files_test_suite.bzl | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cargo/tests/cargo_build_script/cross_crate_build_script_outputs/cross_crate_out_dir_files_test_suite.bzl b/cargo/tests/cargo_build_script/cross_crate_build_script_outputs/cross_crate_out_dir_files_test_suite.bzl index 851a9ea5e8..06ddedba1c 100644 --- a/cargo/tests/cargo_build_script/cross_crate_build_script_outputs/cross_crate_out_dir_files_test_suite.bzl +++ b/cargo/tests/cargo_build_script/cross_crate_build_script_outputs/cross_crate_out_dir_files_test_suite.bzl @@ -38,11 +38,11 @@ def _transitive_out_dir_subst_test_impl(ctx): _transitive_out_dir_subst_test = analysistest.make( _transitive_out_dir_subst_test_impl, doc = """\ -Test that `--subst` values were used passed which is used to dereference command line -files that point to the OUT_DIR of a transitive dependency. `--subst` is an implementation -detail but as build script flags are written to a file, a starlark unit test cannot test this. -Instead the combination of a successfully built target and the tests here provide sufficient -regression testing. +Test that `--subst` values were passed as they are used to dereference command line +files that point to the OUT_DIR of a transitive cargo_build_script dependencies. `--subst` +is an implementation detail but as build script flags are written to a file, a starlark +unit test cannot test this. Instead the combination of a successfully built target and the +tests here provide sufficient regression testing. """, )