mirror of
https://github.com/openai/codex.git
synced 2026-05-02 04:11:39 +03:00
## Why The argument-comment lint now has a packaged DotSlash artifact from [#15198](https://github.com/openai/codex/pull/15198), so the normal repo lint path should use that released payload instead of rebuilding the lint from source every time. That keeps `just clippy` and CI aligned with the shipped artifact while preserving a separate source-build path for people actively hacking on the lint crate. The current alpha package also exposed two integration wrinkles that the repo-side prebuilt wrapper needs to smooth over: - the bundled Dylint library filename includes the host triple, for example `@nightly-2025-09-18-aarch64-apple-darwin`, and Dylint derives `RUSTUP_TOOLCHAIN` from that filename - on Windows, Dylint's driver path also expects `RUSTUP_HOME` to be present in the environment Without those adjustments, the prebuilt CI jobs fail during `cargo metadata` or driver setup. This change makes the checked-in prebuilt wrapper normalize the packaged library name to the plain `nightly-2025-09-18` channel before invoking `cargo-dylint`, and it teaches both the wrapper and the packaged runner source to infer `RUSTUP_HOME` from `rustup show home` when the environment does not already provide it. After the prebuilt Windows lint job started running successfully, it also surfaced a handful of existing anonymous literal callsites in `windows-sandbox-rs`. This PR now annotates those callsites so the new cross-platform lint job is green on the current tree. ## What Changed - checked in the current `tools/argument-comment-lint/argument-comment-lint` DotSlash manifest - kept `tools/argument-comment-lint/run.sh` as the source-build wrapper for lint development - added `tools/argument-comment-lint/run-prebuilt-linter.sh` as the normal enforcement path, using the checked-in DotSlash package and bundled `cargo-dylint` - updated `just clippy` and `just argument-comment-lint` to use the prebuilt wrapper - split `.github/workflows/rust-ci.yml` so source-package checks live in a dedicated `argument_comment_lint_package` job, while the released lint runs in an `argument_comment_lint_prebuilt` matrix on Linux, macOS, and Windows - kept the pinned `nightly-2025-09-18` toolchain install in the prebuilt CI matrix, since the prebuilt package still relies on rustup-provided toolchain components - updated `tools/argument-comment-lint/run-prebuilt-linter.sh` to normalize host-qualified nightly library filenames, keep the `rustup` shim directory ahead of direct toolchain `cargo` binaries, and export `RUSTUP_HOME` when needed for Windows Dylint driver setup - updated `tools/argument-comment-lint/src/bin/argument-comment-lint.rs` so future published DotSlash artifacts apply the same nightly-filename normalization and `RUSTUP_HOME` inference internally - fixed the remaining Windows lint violations in `codex-rs/windows-sandbox-rs` by adding the required `/*param*/` comments at the reported callsites - documented the checked-in DotSlash file, wrapper split, archive layout, nightly prerequisite, and Windows `RUSTUP_HOME` requirement in `tools/argument-comment-lint/README.md`
308 lines
9.7 KiB
Rust
308 lines
9.7 KiB
Rust
use crate::desktop::LaunchDesktop;
|
|
use crate::logging;
|
|
use crate::winutil::format_last_error;
|
|
use crate::winutil::quote_windows_arg;
|
|
use crate::winutil::to_wide;
|
|
use anyhow::anyhow;
|
|
use anyhow::Result;
|
|
use std::collections::HashMap;
|
|
use std::ffi::c_void;
|
|
use std::path::Path;
|
|
use std::ptr;
|
|
use windows_sys::Win32::Foundation::GetLastError;
|
|
use windows_sys::Win32::Foundation::CloseHandle;
|
|
use windows_sys::Win32::Foundation::SetHandleInformation;
|
|
use windows_sys::Win32::Foundation::HANDLE;
|
|
use windows_sys::Win32::Foundation::HANDLE_FLAG_INHERIT;
|
|
use windows_sys::Win32::Foundation::INVALID_HANDLE_VALUE;
|
|
use windows_sys::Win32::Storage::FileSystem::ReadFile;
|
|
use windows_sys::Win32::System::Console::GetStdHandle;
|
|
use windows_sys::Win32::System::Console::STD_ERROR_HANDLE;
|
|
use windows_sys::Win32::System::Console::STD_INPUT_HANDLE;
|
|
use windows_sys::Win32::System::Console::STD_OUTPUT_HANDLE;
|
|
use windows_sys::Win32::System::Pipes::CreatePipe;
|
|
use windows_sys::Win32::System::Threading::CreateProcessAsUserW;
|
|
use windows_sys::Win32::System::Threading::CREATE_UNICODE_ENVIRONMENT;
|
|
use windows_sys::Win32::System::Threading::PROCESS_INFORMATION;
|
|
use windows_sys::Win32::System::Threading::STARTF_USESTDHANDLES;
|
|
use windows_sys::Win32::System::Threading::STARTUPINFOW;
|
|
|
|
pub struct CreatedProcess {
|
|
pub process_info: PROCESS_INFORMATION,
|
|
pub startup_info: STARTUPINFOW,
|
|
_desktop: LaunchDesktop,
|
|
}
|
|
|
|
pub fn make_env_block(env: &HashMap<String, String>) -> Vec<u16> {
|
|
let mut items: Vec<(String, String)> =
|
|
env.iter().map(|(k, v)| (k.clone(), v.clone())).collect();
|
|
items.sort_by(|a, b| {
|
|
a.0.to_uppercase()
|
|
.cmp(&b.0.to_uppercase())
|
|
.then(a.0.cmp(&b.0))
|
|
});
|
|
let mut w: Vec<u16> = Vec::new();
|
|
for (k, v) in items {
|
|
let mut s = to_wide(format!("{}={}", k, v));
|
|
s.pop();
|
|
w.extend_from_slice(&s);
|
|
w.push(0);
|
|
}
|
|
w.push(0);
|
|
w
|
|
}
|
|
|
|
unsafe fn ensure_inheritable_stdio(si: &mut STARTUPINFOW) -> Result<()> {
|
|
for kind in [STD_INPUT_HANDLE, STD_OUTPUT_HANDLE, STD_ERROR_HANDLE] {
|
|
let h = GetStdHandle(kind);
|
|
if h == 0 || h == INVALID_HANDLE_VALUE {
|
|
return Err(anyhow!("GetStdHandle failed: {}", GetLastError()));
|
|
}
|
|
if SetHandleInformation(h, HANDLE_FLAG_INHERIT, HANDLE_FLAG_INHERIT) == 0 {
|
|
return Err(anyhow!("SetHandleInformation failed: {}", GetLastError()));
|
|
}
|
|
}
|
|
si.dwFlags |= STARTF_USESTDHANDLES;
|
|
si.hStdInput = GetStdHandle(STD_INPUT_HANDLE);
|
|
si.hStdOutput = GetStdHandle(STD_OUTPUT_HANDLE);
|
|
si.hStdError = GetStdHandle(STD_ERROR_HANDLE);
|
|
Ok(())
|
|
}
|
|
|
|
/// # Safety
|
|
/// Caller must provide a valid primary token handle (`h_token`) with appropriate access,
|
|
/// and the `argv`, `cwd`, and `env_map` must remain valid for the duration of the call.
|
|
pub unsafe fn create_process_as_user(
|
|
h_token: HANDLE,
|
|
argv: &[String],
|
|
cwd: &Path,
|
|
env_map: &HashMap<String, String>,
|
|
logs_base_dir: Option<&Path>,
|
|
stdio: Option<(HANDLE, HANDLE, HANDLE)>,
|
|
use_private_desktop: bool,
|
|
) -> Result<CreatedProcess> {
|
|
let cmdline_str = argv
|
|
.iter()
|
|
.map(|a| quote_windows_arg(a))
|
|
.collect::<Vec<_>>()
|
|
.join(" ");
|
|
let mut cmdline: Vec<u16> = to_wide(&cmdline_str);
|
|
let env_block = make_env_block(env_map);
|
|
let mut si: STARTUPINFOW = std::mem::zeroed();
|
|
si.cb = std::mem::size_of::<STARTUPINFOW>() as u32;
|
|
// Some processes (e.g., PowerShell) can fail with STATUS_DLL_INIT_FAILED
|
|
// if lpDesktop is not set when launching with a restricted token.
|
|
// Point explicitly at the interactive desktop or a private desktop.
|
|
let desktop = LaunchDesktop::prepare(use_private_desktop, logs_base_dir)?;
|
|
si.lpDesktop = desktop.startup_info_desktop();
|
|
let mut pi: PROCESS_INFORMATION = std::mem::zeroed();
|
|
// Ensure handles are inheritable when custom stdio is supplied.
|
|
let inherit_handles = match stdio {
|
|
Some((stdin_h, stdout_h, stderr_h)) => {
|
|
si.dwFlags |= STARTF_USESTDHANDLES;
|
|
si.hStdInput = stdin_h;
|
|
si.hStdOutput = stdout_h;
|
|
si.hStdError = stderr_h;
|
|
for h in [stdin_h, stdout_h, stderr_h] {
|
|
if SetHandleInformation(h, HANDLE_FLAG_INHERIT, HANDLE_FLAG_INHERIT) == 0 {
|
|
return Err(anyhow!(
|
|
"SetHandleInformation failed for stdio handle: {}",
|
|
GetLastError()
|
|
));
|
|
}
|
|
}
|
|
true
|
|
}
|
|
None => {
|
|
ensure_inheritable_stdio(&mut si)?;
|
|
true
|
|
}
|
|
};
|
|
|
|
let creation_flags = CREATE_UNICODE_ENVIRONMENT;
|
|
let cwd_wide = to_wide(cwd);
|
|
let env_block_len = env_block.len();
|
|
|
|
let ok = CreateProcessAsUserW(
|
|
h_token,
|
|
std::ptr::null(),
|
|
cmdline.as_mut_ptr(),
|
|
std::ptr::null_mut(),
|
|
std::ptr::null_mut(),
|
|
inherit_handles as i32,
|
|
creation_flags,
|
|
env_block.as_ptr() as *mut c_void,
|
|
cwd_wide.as_ptr(),
|
|
&si,
|
|
&mut pi,
|
|
);
|
|
if ok == 0 {
|
|
let err = GetLastError() as i32;
|
|
let msg = format!(
|
|
"CreateProcessAsUserW failed: {} ({}) | cwd={} | cmd={} | env_u16_len={} | si_flags={} | creation_flags={}",
|
|
err,
|
|
format_last_error(err),
|
|
cwd.display(),
|
|
cmdline_str,
|
|
env_block_len,
|
|
si.dwFlags,
|
|
creation_flags,
|
|
);
|
|
logging::debug_log(&msg, logs_base_dir);
|
|
return Err(anyhow!("CreateProcessAsUserW failed: {}", err));
|
|
}
|
|
Ok(CreatedProcess {
|
|
process_info: pi,
|
|
startup_info: si,
|
|
_desktop: desktop,
|
|
})
|
|
}
|
|
|
|
/// Controls whether the child's stdin handle is kept open for writing.
|
|
#[allow(dead_code)]
|
|
pub enum StdinMode {
|
|
Closed,
|
|
Open,
|
|
}
|
|
|
|
/// Controls how stderr is wired for a pipe-spawned process.
|
|
#[allow(dead_code)]
|
|
pub enum StderrMode {
|
|
MergeStdout,
|
|
Separate,
|
|
}
|
|
|
|
/// Handles returned by `spawn_process_with_pipes`.
|
|
#[allow(dead_code)]
|
|
pub struct PipeSpawnHandles {
|
|
pub process: PROCESS_INFORMATION,
|
|
pub stdin_write: Option<HANDLE>,
|
|
pub stdout_read: HANDLE,
|
|
pub stderr_read: Option<HANDLE>,
|
|
}
|
|
|
|
/// Spawns a process with anonymous pipes and returns the relevant handles.
|
|
pub fn spawn_process_with_pipes(
|
|
h_token: HANDLE,
|
|
argv: &[String],
|
|
cwd: &Path,
|
|
env_map: &HashMap<String, String>,
|
|
stdin_mode: StdinMode,
|
|
stderr_mode: StderrMode,
|
|
use_private_desktop: bool,
|
|
) -> Result<PipeSpawnHandles> {
|
|
let mut in_r: HANDLE = 0;
|
|
let mut in_w: HANDLE = 0;
|
|
let mut out_r: HANDLE = 0;
|
|
let mut out_w: HANDLE = 0;
|
|
let mut err_r: HANDLE = 0;
|
|
let mut err_w: HANDLE = 0;
|
|
unsafe {
|
|
if CreatePipe(&mut in_r, &mut in_w, ptr::null_mut(), 0) == 0 {
|
|
return Err(anyhow!("CreatePipe stdin failed: {}", GetLastError()));
|
|
}
|
|
if CreatePipe(&mut out_r, &mut out_w, ptr::null_mut(), 0) == 0 {
|
|
CloseHandle(in_r);
|
|
CloseHandle(in_w);
|
|
return Err(anyhow!("CreatePipe stdout failed: {}", GetLastError()));
|
|
}
|
|
if matches!(stderr_mode, StderrMode::Separate)
|
|
&& CreatePipe(&mut err_r, &mut err_w, ptr::null_mut(), 0) == 0
|
|
{
|
|
CloseHandle(in_r);
|
|
CloseHandle(in_w);
|
|
CloseHandle(out_r);
|
|
CloseHandle(out_w);
|
|
return Err(anyhow!("CreatePipe stderr failed: {}", GetLastError()));
|
|
}
|
|
}
|
|
|
|
let stderr_handle = match stderr_mode {
|
|
StderrMode::MergeStdout => out_w,
|
|
StderrMode::Separate => err_w,
|
|
};
|
|
|
|
let stdio = Some((in_r, out_w, stderr_handle));
|
|
let spawn_result = unsafe {
|
|
create_process_as_user(
|
|
h_token,
|
|
argv,
|
|
cwd,
|
|
env_map,
|
|
/*logs_base_dir*/ None,
|
|
stdio,
|
|
use_private_desktop,
|
|
)
|
|
};
|
|
let created = match spawn_result {
|
|
Ok(v) => v,
|
|
Err(err) => {
|
|
unsafe {
|
|
CloseHandle(in_r);
|
|
CloseHandle(in_w);
|
|
CloseHandle(out_r);
|
|
CloseHandle(out_w);
|
|
if matches!(stderr_mode, StderrMode::Separate) {
|
|
CloseHandle(err_r);
|
|
CloseHandle(err_w);
|
|
}
|
|
}
|
|
return Err(err);
|
|
}
|
|
};
|
|
let pi = created.process_info;
|
|
|
|
unsafe {
|
|
CloseHandle(in_r);
|
|
CloseHandle(out_w);
|
|
if matches!(stderr_mode, StderrMode::Separate) {
|
|
CloseHandle(err_w);
|
|
}
|
|
if matches!(stdin_mode, StdinMode::Closed) {
|
|
CloseHandle(in_w);
|
|
}
|
|
}
|
|
|
|
Ok(PipeSpawnHandles {
|
|
process: pi,
|
|
stdin_write: match stdin_mode {
|
|
StdinMode::Open => Some(in_w),
|
|
StdinMode::Closed => None,
|
|
},
|
|
stdout_read: out_r,
|
|
stderr_read: match stderr_mode {
|
|
StderrMode::Separate => Some(err_r),
|
|
StderrMode::MergeStdout => None,
|
|
},
|
|
})
|
|
}
|
|
|
|
/// Reads a HANDLE until EOF and invokes `on_chunk` for each read.
|
|
pub fn read_handle_loop<F>(handle: HANDLE, mut on_chunk: F) -> std::thread::JoinHandle<()>
|
|
where
|
|
F: FnMut(&[u8]) + Send + 'static,
|
|
{
|
|
std::thread::spawn(move || {
|
|
let mut buf = [0u8; 8192];
|
|
loop {
|
|
let mut read_bytes: u32 = 0;
|
|
let ok = unsafe {
|
|
ReadFile(
|
|
handle,
|
|
buf.as_mut_ptr(),
|
|
buf.len() as u32,
|
|
&mut read_bytes,
|
|
ptr::null_mut(),
|
|
)
|
|
};
|
|
if ok == 0 || read_bytes == 0 {
|
|
break;
|
|
}
|
|
on_chunk(&buf[..read_bytes as usize]);
|
|
}
|
|
unsafe {
|
|
CloseHandle(handle);
|
|
}
|
|
})
|
|
}
|