mirror of
https://github.com/openai/codex.git
synced 2026-08-23 13:09:46 +00:00
Embed the Windows sandbox setup manifest in Bazel builds (#38450)
## Why `rules_rust` drops the build script's per-binary linker directives, so Bazel builds can omit the `asInvoker` manifest from the Windows sandbox setup helper. ## What changed - Add per-binary compile data and Rust flags to `codex_rust_crate` so linker inputs remain scoped to the setup helper. - Embed the manifest directly for MSVC builds and compile it into a resource with hermetic LLVM tooling for GNU/LLVM cross-builds. - Disable the redundant build script under Bazel and avoid duplicating binary runfiles in integration test data. ## Testing Add a Windows integration test that loads the setup executable's manifest resource and verifies that it requests `asInvoker` execution with UI access disabled. GitOrigin-RevId: a77e7e627ee43810f5eaf7701bb4909bf855216b
This commit is contained in:
committed by
copyberry
parent
5cc65ecb98
commit
813dc5f08d
@@ -1,11 +1,55 @@
|
||||
load("//:defs.bzl", "codex_rust_crate")
|
||||
|
||||
# Cargo's build.rs emits rustc-link-arg-bin so only the setup helper gets the
|
||||
# asInvoker manifest. rules_rust warns on and drops that directive, so Bazel
|
||||
# spells out the same per-binary linker contract here.
|
||||
WINDOWS_SETUP_MANIFEST_RUSTC_FLAGS = select({
|
||||
"@llvm//constraints/windows/abi:gnullvm": [
|
||||
"-C",
|
||||
"link-arg=$(location :codex-windows-sandbox-setup-manifest-resource)",
|
||||
],
|
||||
"@llvm//constraints/windows/abi:msvc": [
|
||||
"-C",
|
||||
"link-arg=/MANIFEST:EMBED",
|
||||
"-C",
|
||||
"link-arg=/MANIFESTINPUT:$(location :codex-windows-sandbox-setup.manifest)",
|
||||
],
|
||||
"//conditions:default": [],
|
||||
})
|
||||
|
||||
# Copying build.rs's gnullvm /MANIFESTINPUT flags is not enough: lld-link
|
||||
# shells out to mt.exe, but the supported gnullvm cross-build executes on
|
||||
# Linux RBE. Compile the same RT_MANIFEST resource with hermetic LLVM instead.
|
||||
# -no-preprocess does not expand RT_MANIFEST, so use its numeric type, 24.
|
||||
# Keep the command on one line: Windows checkouts use CRLF, but Linux RBE runs it.
|
||||
genrule(
|
||||
name = "codex-windows-sandbox-setup-manifest-resource",
|
||||
srcs = ["codex-windows-sandbox-setup.manifest"],
|
||||
outs = ["codex-windows-sandbox-setup.manifest.res"],
|
||||
cmd = " && ".join([
|
||||
"printf '1 24 \"%s\"\\n' \"$(location :codex-windows-sandbox-setup.manifest)\" > \"$(@D)/codex-windows-sandbox-setup.manifest.rc\"",
|
||||
"$(location @llvm//tools:llvm-rc) -no-preprocess \"$(@D)/codex-windows-sandbox-setup.manifest.rc\"",
|
||||
]),
|
||||
tools = ["@llvm//tools:llvm-rc"],
|
||||
)
|
||||
|
||||
codex_rust_crate(
|
||||
name = "windows-sandbox-rs",
|
||||
binary_compile_data_extra = {
|
||||
"codex-windows-sandbox-setup": select({
|
||||
"@llvm//constraints/windows/abi:gnullvm": [":codex-windows-sandbox-setup-manifest-resource"],
|
||||
"@llvm//constraints/windows/abi:msvc": ["codex-windows-sandbox-setup.manifest"],
|
||||
"//conditions:default": [],
|
||||
}),
|
||||
},
|
||||
binary_rustc_flags_extra = {
|
||||
"codex-windows-sandbox-setup": WINDOWS_SETUP_MANIFEST_RUSTC_FLAGS,
|
||||
},
|
||||
binary_test_target_compatible_with = ["@platforms//os:windows"],
|
||||
build_script_data = [
|
||||
"codex-windows-sandbox-setup.manifest",
|
||||
],
|
||||
# Do not run build.rs under Bazel: rules_rust cannot translate its
|
||||
# rustc-link-arg-bin directives, and the target-local equivalents above
|
||||
# provide the link inputs and flags without the unsupported-directive warning.
|
||||
build_script_enabled = False,
|
||||
crate_name = "codex_windows_sandbox",
|
||||
test_data_extra = [
|
||||
":codex-command-runner",
|
||||
|
||||
83
codex-rs/windows-sandbox-rs/tests/helper_manifest.rs
Normal file
83
codex-rs/windows-sandbox-rs/tests/helper_manifest.rs
Normal file
@@ -0,0 +1,83 @@
|
||||
#![cfg(target_os = "windows")]
|
||||
|
||||
use anyhow::Context;
|
||||
use anyhow::Result;
|
||||
use std::io;
|
||||
use std::os::windows::ffi::OsStrExt;
|
||||
use std::path::PathBuf;
|
||||
use windows_sys::Win32::Foundation::FreeLibrary;
|
||||
use windows_sys::Win32::System::LibraryLoader::FindResourceW;
|
||||
use windows_sys::Win32::System::LibraryLoader::LOAD_LIBRARY_AS_DATAFILE;
|
||||
use windows_sys::Win32::System::LibraryLoader::LOAD_LIBRARY_AS_IMAGE_RESOURCE;
|
||||
use windows_sys::Win32::System::LibraryLoader::LoadLibraryExW;
|
||||
use windows_sys::Win32::System::LibraryLoader::LoadResource;
|
||||
use windows_sys::Win32::System::LibraryLoader::LockResource;
|
||||
use windows_sys::Win32::System::LibraryLoader::SizeofResource;
|
||||
use windows_sys::Win32::UI::WindowsAndMessaging::CREATEPROCESS_MANIFEST_RESOURCE_ID;
|
||||
use windows_sys::Win32::UI::WindowsAndMessaging::RT_MANIFEST;
|
||||
|
||||
/// The setup executable must expose an asInvoker manifest through the Windows resource API.
|
||||
#[test]
|
||||
fn setup_helper_embeds_as_invoker_manifest() -> Result<()> {
|
||||
let setup_executable = std::env::var_os("CARGO_BIN_EXE_codex-windows-sandbox-setup")
|
||||
.or_else(|| std::env::var_os("CARGO_BIN_EXE_codex_windows_sandbox_setup"))
|
||||
.map(PathBuf::from)
|
||||
.or_else(|| option_env!("CARGO_BIN_EXE_codex-windows-sandbox-setup").map(PathBuf::from))
|
||||
.context("locate the Windows sandbox setup executable")?;
|
||||
std::fs::metadata(&setup_executable)
|
||||
.with_context(|| format!("find setup helper {}", setup_executable.display()))?;
|
||||
let setup_path = setup_executable
|
||||
.as_os_str()
|
||||
.encode_wide()
|
||||
.chain(std::iter::once(0))
|
||||
.collect::<Vec<_>>();
|
||||
let module = unsafe {
|
||||
LoadLibraryExW(
|
||||
setup_path.as_ptr(),
|
||||
/*hfile*/ 0,
|
||||
LOAD_LIBRARY_AS_DATAFILE | LOAD_LIBRARY_AS_IMAGE_RESOURCE,
|
||||
)
|
||||
};
|
||||
if module == 0 {
|
||||
return Err(io::Error::last_os_error())
|
||||
.with_context(|| format!("load setup helper {}", setup_executable.display()));
|
||||
}
|
||||
|
||||
let resource = unsafe {
|
||||
FindResourceW(
|
||||
module,
|
||||
std::ptr::without_provenance(CREATEPROCESS_MANIFEST_RESOURCE_ID as usize),
|
||||
std::ptr::without_provenance(RT_MANIFEST as usize),
|
||||
)
|
||||
};
|
||||
if resource == 0 {
|
||||
return Err(io::Error::last_os_error()).context("find numeric RT_MANIFEST resource ID 1");
|
||||
}
|
||||
|
||||
let resource_size = unsafe { SizeofResource(module, resource) };
|
||||
let loaded_resource = unsafe { LoadResource(module, resource) };
|
||||
if loaded_resource.is_null() {
|
||||
return Err(io::Error::last_os_error()).context("load setup helper manifest resource");
|
||||
}
|
||||
let resource_data = unsafe { LockResource(loaded_resource) };
|
||||
if resource_data.is_null() {
|
||||
return Err(io::Error::last_os_error()).context("read setup helper manifest resource");
|
||||
}
|
||||
let manifest_bytes =
|
||||
unsafe { std::slice::from_raw_parts(resource_data.cast::<u8>(), resource_size as usize) };
|
||||
let manifest = std::str::from_utf8(manifest_bytes).context("decode setup helper manifest")?;
|
||||
assert!(
|
||||
manifest.contains("requestedExecutionLevel level=\"asInvoker\""),
|
||||
"setup helper manifest does not request asInvoker: {manifest}",
|
||||
);
|
||||
assert!(
|
||||
manifest.contains("uiAccess=\"false\""),
|
||||
"setup helper manifest does not disable UI access: {manifest}",
|
||||
);
|
||||
|
||||
if unsafe { FreeLibrary(module) } == 0 {
|
||||
return Err(io::Error::last_os_error()).context("unload setup helper resource module");
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
22
defs.bzl
22
defs.bzl
@@ -188,8 +188,10 @@ def codex_rust_crate(
|
||||
build_script_enabled = True,
|
||||
build_script_data = [],
|
||||
compile_data = [],
|
||||
binary_compile_data_extra = {},
|
||||
lib_data_extra = [],
|
||||
rustc_flags_extra = [],
|
||||
binary_rustc_flags_extra = {},
|
||||
rustc_env = {},
|
||||
deps_extra = [],
|
||||
integration_compile_data_extra = [],
|
||||
@@ -226,7 +228,11 @@ def codex_rust_crate(
|
||||
proc_macro: Whether this crate builds a proc-macro library.
|
||||
build_script_data: Data files exposed to the build script at runtime.
|
||||
compile_data: Non-Rust compile-time data for the library target.
|
||||
binary_compile_data_extra: Mapping from binary names to extra non-Rust
|
||||
compile-time data for those binary targets.
|
||||
lib_data_extra: Extra runtime data for the library target.
|
||||
binary_rustc_flags_extra: Mapping from binary names to extra rustc
|
||||
flags for those binary targets.
|
||||
rustc_env: Extra rustc_env entries to merge with defaults.
|
||||
deps_extra: Extra normal deps beyond @crates resolution.
|
||||
Typically only needed when features add additional deps.
|
||||
@@ -384,7 +390,10 @@ def codex_rust_crate(
|
||||
crate_root = main,
|
||||
deps = all_crate_deps() + maybe_deps + deps_extra,
|
||||
edition = crate_edition,
|
||||
rustc_flags = rustc_flags_extra + WINDOWS_RUSTC_LINK_FLAGS,
|
||||
# Keep per-binary Cargo link behavior scoped to the matching
|
||||
# generated rust_binary instead of leaking it to sibling binaries.
|
||||
compile_data = binary_compile_data_extra.get(binary, []),
|
||||
rustc_flags = rustc_flags_extra + binary_rustc_flags_extra.get(binary, []) + WINDOWS_RUSTC_LINK_FLAGS,
|
||||
# rules_rust substitutes workspace status values only for stamped
|
||||
# actions, so pass the existing key through to final binaries.
|
||||
rustc_env = {"STABLE_GIT_COMMIT": "{STABLE_GIT_COMMIT}"},
|
||||
@@ -441,6 +450,11 @@ def codex_rust_crate(
|
||||
integration_test_binaries = sanitized_binaries
|
||||
integration_test_cargo_env = cargo_env
|
||||
integration_test_cargo_env_runfiles = cargo_env_runfiles
|
||||
integration_test_data_extra = [
|
||||
data
|
||||
for data in test_data_extra
|
||||
if data not in cargo_env_runfiles
|
||||
]
|
||||
non_windows_sanitized_binaries = []
|
||||
non_windows_cargo_env = {}
|
||||
non_windows_cargo_env_runfiles = {}
|
||||
@@ -523,7 +537,7 @@ def codex_rust_crate(
|
||||
crate_name = test_crate_name,
|
||||
crate_root = test,
|
||||
srcs = [test],
|
||||
data = native.glob(["tests/**"], allow_empty = True) + integration_test_binaries + test_data_extra,
|
||||
data = native.glob(["tests/**"], allow_empty = True) + integration_test_binaries + integration_test_data_extra,
|
||||
compile_data = native.glob(["tests/**"], allow_empty = True) + integration_compile_data_extra,
|
||||
deps = all_crate_deps(normal = True, normal_dev = True) + maybe_deps + deps_extra,
|
||||
# Bazel has emitted both `codex-rs/<crate>/...` and
|
||||
@@ -562,7 +576,7 @@ def codex_rust_crate(
|
||||
crate_name = test_crate_name,
|
||||
crate_root = test,
|
||||
srcs = [test],
|
||||
data = native.glob(["tests/**"], allow_empty = True) + integration_test_binaries + test_data_extra,
|
||||
data = native.glob(["tests/**"], allow_empty = True) + integration_test_binaries + integration_test_data_extra,
|
||||
compile_data = native.glob(["tests/**"], allow_empty = True) + integration_compile_data_extra,
|
||||
deps = all_crate_deps(normal = True, normal_dev = True) + maybe_deps + deps_extra,
|
||||
# Bazel has emitted both `codex-rs/<crate>/...` and
|
||||
@@ -637,7 +651,7 @@ def codex_rust_crate(
|
||||
crate_name = test_crate_name,
|
||||
crate_root = test,
|
||||
srcs = [test],
|
||||
data = native.glob(["tests/**"], allow_empty = True) + integration_test_binaries + test_data_extra,
|
||||
data = native.glob(["tests/**"], allow_empty = True) + integration_test_binaries + integration_test_data_extra,
|
||||
compile_data = native.glob(["tests/**"], allow_empty = True) + integration_compile_data_extra,
|
||||
deps = all_crate_deps(normal = True, normal_dev = True) + maybe_deps + deps_extra,
|
||||
rustc_flags = rustc_flags_extra + WINDOWS_RUSTC_LINK_FLAGS + [
|
||||
|
||||
Reference in New Issue
Block a user