ci: sync Bazel clippy lints and fix uncovered violations

This commit is contained in:
Michael Bolin
2026-03-31 15:36:14 -07:00
parent 9a8730f31e
commit a1f9eda176
22 changed files with 347 additions and 61 deletions
+38
View File
@@ -76,6 +76,44 @@ common:ci-bazel --build_metadata=TAG_workflow=bazel
build:clippy --aspects=@rules_rust//rust:defs.bzl%rust_clippy_aspect build:clippy --aspects=@rules_rust//rust:defs.bzl%rust_clippy_aspect
build:clippy --output_groups=+clippy_checks build:clippy --output_groups=+clippy_checks
build:clippy --@rules_rust//rust/settings:clippy.toml=//codex-rs:clippy.toml build:clippy --@rules_rust//rust/settings:clippy.toml=//codex-rs:clippy.toml
# Keep this deny-list in sync with `codex-rs/Cargo.toml` `[workspace.lints.clippy]`.
# Cargo applies those lint levels to member crates that opt into `[lints] workspace = true`
# in their own `Cargo.toml`, but `rules_rust` Bazel clippy does not read Cargo lint levels.
# `clippy.toml` can configure lint behavior, but it cannot set allow/warn/deny/forbid levels.
build:clippy --@rules_rust//rust/settings:clippy_flag=-Dwarnings
build:clippy --@rules_rust//rust/settings:clippy_flag=--deny=clippy::expect_used
build:clippy --@rules_rust//rust/settings:clippy_flag=--deny=clippy::identity_op
build:clippy --@rules_rust//rust/settings:clippy_flag=--deny=clippy::manual_clamp
build:clippy --@rules_rust//rust/settings:clippy_flag=--deny=clippy::manual_filter
build:clippy --@rules_rust//rust/settings:clippy_flag=--deny=clippy::manual_find
build:clippy --@rules_rust//rust/settings:clippy_flag=--deny=clippy::manual_flatten
build:clippy --@rules_rust//rust/settings:clippy_flag=--deny=clippy::manual_map
build:clippy --@rules_rust//rust/settings:clippy_flag=--deny=clippy::manual_memcpy
build:clippy --@rules_rust//rust/settings:clippy_flag=--deny=clippy::manual_non_exhaustive
build:clippy --@rules_rust//rust/settings:clippy_flag=--deny=clippy::manual_ok_or
build:clippy --@rules_rust//rust/settings:clippy_flag=--deny=clippy::manual_range_contains
build:clippy --@rules_rust//rust/settings:clippy_flag=--deny=clippy::manual_retain
build:clippy --@rules_rust//rust/settings:clippy_flag=--deny=clippy::manual_strip
build:clippy --@rules_rust//rust/settings:clippy_flag=--deny=clippy::manual_try_fold
build:clippy --@rules_rust//rust/settings:clippy_flag=--deny=clippy::manual_unwrap_or
build:clippy --@rules_rust//rust/settings:clippy_flag=--deny=clippy::needless_borrow
build:clippy --@rules_rust//rust/settings:clippy_flag=--deny=clippy::needless_borrowed_reference
build:clippy --@rules_rust//rust/settings:clippy_flag=--deny=clippy::needless_collect
build:clippy --@rules_rust//rust/settings:clippy_flag=--deny=clippy::needless_late_init
build:clippy --@rules_rust//rust/settings:clippy_flag=--deny=clippy::needless_option_as_deref
build:clippy --@rules_rust//rust/settings:clippy_flag=--deny=clippy::needless_question_mark
build:clippy --@rules_rust//rust/settings:clippy_flag=--deny=clippy::needless_update
build:clippy --@rules_rust//rust/settings:clippy_flag=--deny=clippy::redundant_clone
build:clippy --@rules_rust//rust/settings:clippy_flag=--deny=clippy::redundant_closure
build:clippy --@rules_rust//rust/settings:clippy_flag=--deny=clippy::redundant_closure_for_method_calls
build:clippy --@rules_rust//rust/settings:clippy_flag=--deny=clippy::redundant_static_lifetimes
build:clippy --@rules_rust//rust/settings:clippy_flag=--deny=clippy::trivially_copy_pass_by_ref
build:clippy --@rules_rust//rust/settings:clippy_flag=--deny=clippy::uninlined_format_args
build:clippy --@rules_rust//rust/settings:clippy_flag=--deny=clippy::unnecessary_filter_map
build:clippy --@rules_rust//rust/settings:clippy_flag=--deny=clippy::unnecessary_lazy_evaluations
build:clippy --@rules_rust//rust/settings:clippy_flag=--deny=clippy::unnecessary_sort_by
build:clippy --@rules_rust//rust/settings:clippy_flag=--deny=clippy::unnecessary_to_owned
build:clippy --@rules_rust//rust/settings:clippy_flag=--deny=clippy::unwrap_used
# Shared config for Bazel-backed argument-comment-lint. # Shared config for Bazel-backed argument-comment-lint.
build:argument-comment-lint --aspects=//tools/argument-comment-lint:lint_aspect.bzl%rust_argument_comment_lint_aspect build:argument-comment-lint --aspects=//tools/argument-comment-lint:lint_aspect.bzl%rust_argument_comment_lint_aspect
@@ -0,0 +1,234 @@
#!/usr/bin/env python3
from __future__ import annotations
import argparse
import re
import sys
import tomllib
from pathlib import Path
ROOT = Path(__file__).resolve().parents[2]
DEFAULT_CARGO_TOML = ROOT / "codex-rs" / "Cargo.toml"
DEFAULT_BAZELRC = ROOT / ".bazelrc"
BAZEL_CLIPPY_FLAG_PREFIX = "build:clippy --@rules_rust//rust/settings:clippy_flag="
BAZEL_SPECIAL_FLAGS = {"-Dwarnings"}
VALID_LEVELS = {"allow", "warn", "deny", "forbid"}
LONG_FLAG_RE = re.compile(
r"^--(?P<level>allow|warn|deny|forbid)=clippy::(?P<lint>[a-z0-9_]+)$"
)
SHORT_FLAG_RE = re.compile(r"^-(?P<level>[AWDF])clippy::(?P<lint>[a-z0-9_]+)$")
SHORT_LEVEL_NAMES = {
"A": "allow",
"W": "warn",
"D": "deny",
"F": "forbid",
}
def main() -> int:
parser = argparse.ArgumentParser(
description=(
"Verify that Bazel clippy flags in .bazelrc stay in sync with "
"codex-rs/Cargo.toml [workspace.lints.clippy]."
)
)
parser.add_argument(
"--cargo-toml",
type=Path,
default=DEFAULT_CARGO_TOML,
help="Path to the workspace Cargo.toml to inspect.",
)
parser.add_argument(
"--bazelrc",
type=Path,
default=DEFAULT_BAZELRC,
help="Path to the .bazelrc file to inspect.",
)
args = parser.parse_args()
cargo_toml = args.cargo_toml.resolve()
bazelrc = args.bazelrc.resolve()
cargo_lints = load_workspace_clippy_lints(cargo_toml)
bazel_lints = load_bazel_clippy_lints(bazelrc)
missing = sorted(cargo_lints.keys() - bazel_lints.keys())
extra = sorted(bazel_lints.keys() - cargo_lints.keys())
mismatched = sorted(
lint
for lint in cargo_lints.keys() & bazel_lints.keys()
if cargo_lints[lint] != bazel_lints[lint]
)
if missing or extra or mismatched:
print_sync_error(
cargo_toml=cargo_toml,
bazelrc=bazelrc,
cargo_lints=cargo_lints,
bazel_lints=bazel_lints,
missing=missing,
extra=extra,
mismatched=mismatched,
)
return 1
print(
"Bazel clippy flags in "
f"{display_path(bazelrc)} match "
f"{display_path(cargo_toml)} [workspace.lints.clippy]."
)
return 0
def load_workspace_clippy_lints(cargo_toml: Path) -> dict[str, str]:
workspace = tomllib.loads(cargo_toml.read_text())["workspace"]
clippy_lints = workspace["lints"]["clippy"]
parsed: dict[str, str] = {}
for lint, level in clippy_lints.items():
if not isinstance(level, str):
raise SystemExit(
f"expected string lint level for clippy::{lint} in {cargo_toml}, got {level!r}"
)
normalized = level.strip().lower()
if normalized not in VALID_LEVELS:
raise SystemExit(
f"unsupported lint level {level!r} for clippy::{lint} in {cargo_toml}"
)
parsed[lint] = normalized
return parsed
def load_bazel_clippy_lints(bazelrc: Path) -> dict[str, str]:
parsed: dict[str, str] = {}
line_numbers: dict[str, int] = {}
for lineno, line in enumerate(bazelrc.read_text().splitlines(), start=1):
if not line.startswith(BAZEL_CLIPPY_FLAG_PREFIX):
continue
flag = line.removeprefix(BAZEL_CLIPPY_FLAG_PREFIX).strip()
if flag in BAZEL_SPECIAL_FLAGS:
continue
parsed_flag = parse_bazel_lint_flag(flag)
if parsed_flag is None:
continue
lint, level = parsed_flag
if lint in parsed:
raise SystemExit(
f"duplicate Bazel clippy entry for clippy::{lint} at "
f"{bazelrc}:{line_numbers[lint]} and {bazelrc}:{lineno}"
)
parsed[lint] = level
line_numbers[lint] = lineno
return parsed
def parse_bazel_lint_flag(flag: str) -> tuple[str, str] | None:
long_match = LONG_FLAG_RE.match(flag)
if long_match:
return long_match["lint"], long_match["level"]
short_match = SHORT_FLAG_RE.match(flag)
if short_match:
return short_match["lint"], SHORT_LEVEL_NAMES[short_match["level"]]
return None
def print_sync_error(
*,
cargo_toml: Path,
bazelrc: Path,
cargo_lints: dict[str, str],
bazel_lints: dict[str, str],
missing: list[str],
extra: list[str],
mismatched: list[str],
) -> None:
cargo_toml_display = display_path(cargo_toml)
bazelrc_display = display_path(bazelrc)
example_manifest = find_workspace_lints_example_manifest()
print(
"ERROR: Bazel clippy flags are out of sync with Cargo workspace clippy lints.",
file=sys.stderr,
)
print(file=sys.stderr)
print(
f"Cargo defines the source of truth in {cargo_toml_display} "
"[workspace.lints.clippy].",
file=sys.stderr,
)
if example_manifest is not None:
print(
"Cargo applies those lint levels to member crates that opt into "
f"`[lints] workspace = true`, for example {example_manifest}.",
file=sys.stderr,
)
print(
"Bazel clippy does not ingest Cargo lint levels automatically, and "
"`clippy.toml` can configure lint behavior but cannot set allow/warn/deny/forbid.",
file=sys.stderr,
)
print(
f"Update {bazelrc_display} so its `build:clippy` "
"`clippy_flag` entries match Cargo.",
file=sys.stderr,
)
if missing:
print(file=sys.stderr)
print("Missing Bazel entries:", file=sys.stderr)
for lint in missing:
print(f" {render_bazelrc_line(lint, cargo_lints[lint])}", file=sys.stderr)
if mismatched:
print(file=sys.stderr)
print("Mismatched lint levels:", file=sys.stderr)
for lint in mismatched:
cargo_level = cargo_lints[lint]
bazel_level = bazel_lints[lint]
print(
f" clippy::{lint}: Cargo has {cargo_level}, Bazel has {bazel_level}",
file=sys.stderr,
)
print(
f" expected: {render_bazelrc_line(lint, cargo_level)}",
file=sys.stderr,
)
if extra:
print(file=sys.stderr)
print("Extra Bazel entries with no Cargo counterpart:", file=sys.stderr)
for lint in extra:
print(f" {render_bazelrc_line(lint, bazel_lints[lint])}", file=sys.stderr)
def render_bazelrc_line(lint: str, level: str) -> str:
return f"{BAZEL_CLIPPY_FLAG_PREFIX}--{level}=clippy::{lint}"
def display_path(path: Path) -> str:
try:
return str(path.relative_to(ROOT))
except ValueError:
return str(path)
def find_workspace_lints_example_manifest() -> str | None:
for cargo_toml in sorted((ROOT / "codex-rs").glob("**/Cargo.toml")):
if cargo_toml == DEFAULT_CARGO_TOML:
continue
data = tomllib.loads(cargo_toml.read_text())
if data.get("lints", {}).get("workspace") is True:
return str(cargo_toml.relative_to(ROOT))
return None
if __name__ == "__main__":
sys.exit(main())
+3
View File
@@ -17,6 +17,9 @@ jobs:
- name: Verify codex-rs Cargo manifests inherit workspace settings - name: Verify codex-rs Cargo manifests inherit workspace settings
run: python3 .github/scripts/verify_cargo_workspace_manifests.py run: python3 .github/scripts/verify_cargo_workspace_manifests.py
- name: Verify Bazel clippy flags match Cargo workspace lints
run: python3 .github/scripts/verify_bazel_clippy_lints.py
- name: Setup pnpm - name: Setup pnpm
uses: pnpm/action-setup@a8198c4bff370c8506180b035930dea56dbd5288 # v5 uses: pnpm/action-setup@a8198c4bff370c8506180b035930dea56dbd5288 # v5
with: with:
+1 -1
View File
@@ -309,7 +309,7 @@ where
D: Deserializer<'de>, D: Deserializer<'de>,
T: Deserialize<'de>, T: Deserialize<'de>,
{ {
Option::<Vec<T>>::deserialize(deserializer).map(|opt| opt.unwrap_or_default()) Option::<Vec<T>>::deserialize(deserializer).map(Option::unwrap_or_default)
} }
#[derive(Clone, Debug, Deserialize)] #[derive(Clone, Debug, Deserialize)]
+3 -3
View File
@@ -287,7 +287,7 @@ where
F: Fn(&codex_protocol::protocol::EventMsg) -> Option<T>, F: Fn(&codex_protocol::protocol::EventMsg) -> Option<T>,
{ {
let ev = wait_for_event(codex, |ev| matcher(ev).is_some()).await; let ev = wait_for_event(codex, |ev| matcher(ev).is_some()).await;
matcher(&ev).unwrap() matcher(&ev).expect("EventMsg should match matcher predicate")
} }
pub async fn wait_for_event_with_timeout<F>( pub async fn wait_for_event_with_timeout<F>(
@@ -417,7 +417,7 @@ pub mod fs_wait {
let deadline = Instant::now() + timeout; let deadline = Instant::now() + timeout;
loop { loop {
if path.exists() { if path.exists() {
return Ok(path.clone()); return Ok(path);
} }
let now = Instant::now(); let now = Instant::now();
if now >= deadline { if now >= deadline {
@@ -427,7 +427,7 @@ pub mod fs_wait {
match rx.recv_timeout(remaining) { match rx.recv_timeout(remaining) {
Ok(Ok(_event)) => { Ok(Ok(_event)) => {
if path.exists() { if path.exists() {
return Ok(path.clone()); return Ok(path);
} }
} }
Ok(Err(err)) => return Err(err.into()), Ok(Err(err)) => return Err(err.into()),
+2
View File
@@ -1,3 +1,5 @@
#![allow(clippy::unwrap_used)]
use std::collections::VecDeque; use std::collections::VecDeque;
use std::sync::Arc; use std::sync::Arc;
use std::sync::Mutex; use std::sync::Mutex;
+3 -3
View File
@@ -270,7 +270,7 @@ fn docker_command_success<const N: usize>(args: [&str; N]) -> Result<()> {
let output = Command::new("docker") let output = Command::new("docker")
.args(args) .args(args)
.output() .output()
.with_context(|| format!("run docker {:?}", args))?; .with_context(|| format!("run docker {args:?}"))?;
if !output.status.success() { if !output.status.success() {
return Err(anyhow!( return Err(anyhow!(
"docker {:?} failed: stdout={} stderr={}", "docker {:?} failed: stdout={} stderr={}",
@@ -286,7 +286,7 @@ fn docker_command_capture_stdout<const N: usize>(args: [&str; N]) -> Result<Stri
let output = Command::new("docker") let output = Command::new("docker")
.args(args) .args(args)
.output() .output()
.with_context(|| format!("run docker {:?}", args))?; .with_context(|| format!("run docker {args:?}"))?;
if !output.status.success() { if !output.status.success() {
return Err(anyhow!( return Err(anyhow!(
"docker {:?} failed: stdout={} stderr={}", "docker {:?} failed: stdout={} stderr={}",
@@ -346,7 +346,7 @@ impl TestCodexBuilder {
pub fn with_model(self, model: &str) -> Self { pub fn with_model(self, model: &str) -> Self {
let new_model = model.to_string(); let new_model = model.to_string();
self.with_config(move |config| { self.with_config(move |config| {
config.model = Some(new_model.clone()); config.model = Some(new_model);
}) })
} }
+4 -4
View File
@@ -327,13 +327,13 @@ unsafe fn ensure_allow_mask_aces_with_inheritance_impl(
if !p_sd.is_null() { if !p_sd.is_null() {
LocalFree(p_sd as HLOCAL); LocalFree(p_sd as HLOCAL);
} }
return Err(anyhow!("SetNamedSecurityInfoW failed: {}", code3)); return Err(anyhow!("SetNamedSecurityInfoW failed: {code3}"));
} }
} else { } else {
if !p_sd.is_null() { if !p_sd.is_null() {
LocalFree(p_sd as HLOCAL); LocalFree(p_sd as HLOCAL);
} }
return Err(anyhow!("SetEntriesInAclW failed: {}", code2)); return Err(anyhow!("SetEntriesInAclW failed: {code2}"));
} }
} }
if !p_sd.is_null() { if !p_sd.is_null() {
@@ -401,7 +401,7 @@ pub unsafe fn add_allow_ace(path: &Path, psid: *mut c_void) -> Result<bool> {
&mut p_sd, &mut p_sd,
); );
if code != ERROR_SUCCESS { if code != ERROR_SUCCESS {
return Err(anyhow!("GetNamedSecurityInfoW failed: {}", code)); return Err(anyhow!("GetNamedSecurityInfoW failed: {code}"));
} }
// Already has write? Skip costly DACL rewrite. // Already has write? Skip costly DACL rewrite.
if dacl_has_write_allow_for_sid(p_dacl, psid) { if dacl_has_write_allow_for_sid(p_dacl, psid) {
@@ -467,7 +467,7 @@ pub unsafe fn add_deny_write_ace(path: &Path, psid: *mut c_void) -> Result<bool>
&mut p_sd, &mut p_sd,
); );
if code != ERROR_SUCCESS { if code != ERROR_SUCCESS {
return Err(anyhow!("GetNamedSecurityInfoW failed: {}", code)); return Err(anyhow!("GetNamedSecurityInfoW failed: {code}"));
} }
let mut added = false; let mut added = false;
if !dacl_has_write_deny_for_sid(p_dacl, psid) { if !dacl_has_write_deny_for_sid(p_dacl, psid) {
+2 -3
View File
@@ -194,8 +194,7 @@ pub fn audit_everyone_writable(
} }
crate::logging::log_note( crate::logging::log_note(
&format!( &format!(
"AUDIT: world-writable scan FAILED; cwd={cwd:?}; checked={checked}; duration_ms={elapsed_ms}; flagged:{}", "AUDIT: world-writable scan FAILED; cwd={cwd:?}; checked={checked}; duration_ms={elapsed_ms}; flagged:{list}",
list
), ),
logs_base_dir, logs_base_dir,
); );
@@ -229,7 +228,7 @@ pub fn apply_world_writable_scan_and_denies(
logs_base_dir, logs_base_dir,
) { ) {
log_note( log_note(
&format!("AUDIT: failed to apply capability deny ACEs: {}", err), &format!("AUDIT: failed to apply capability deny ACEs: {err}"),
logs_base_dir, logs_base_dir,
); );
} }
+1 -1
View File
@@ -34,7 +34,7 @@ fn make_random_cap_sid_string() -> String {
let b = rng.next_u32(); let b = rng.next_u32();
let c = rng.next_u32(); let c = rng.next_u32();
let d = rng.next_u32(); let d = rng.next_u32();
format!("S-1-5-21-{}-{}-{}-{}", a, b, c, d) format!("S-1-5-21-{a}-{b}-{c}-{d}")
} }
fn persist_caps(path: &Path, caps: &CapSids) -> Result<()> { fn persist_caps(path: &Path, caps: &CapSids) -> Result<()> {
@@ -480,7 +480,7 @@ pub fn main() -> Result<()> {
let _ = send_error(&pipe_write, "spawn_failed", err.to_string()); let _ = send_error(&pipe_write, "spawn_failed", err.to_string());
return Err(err); return Err(err);
} }
let log_dir_owned = log_dir.map(|p| p.to_path_buf()); let log_dir_owned = log_dir.map(Path::to_path_buf);
let out_thread = spawn_output_reader( let out_thread = spawn_output_reader(
Arc::clone(&pipe_write), Arc::clone(&pipe_write),
stdout_handle, stdout_handle,
@@ -144,7 +144,7 @@ pub fn read_frame<R: Read>(mut reader: R) -> Result<Option<FramedMessage>> {
} }
let len = u32::from_le_bytes(len_buf) as usize; let len = u32::from_le_bytes(len_buf) as usize;
if len > MAX_FRAME_LEN { if len > MAX_FRAME_LEN {
anyhow::bail!("frame too large: {}", len); anyhow::bail!("frame too large: {len}");
} }
let mut payload = vec![0u8; len]; let mut payload = vec![0u8; len];
reader.read_exact(&mut payload)?; reader.read_exact(&mut payload)?;
@@ -93,7 +93,7 @@ mod windows_impl {
} else { } else {
cur.join(gitdir) cur.join(gitdir)
}; };
return resolved.parent().map(|p| p.to_path_buf()).or(Some(cur)); return resolved.parent().map(Path::to_path_buf).or(Some(cur));
} }
return Some(cur); return Some(cur);
} }
@@ -270,17 +270,22 @@ mod windows_impl {
} }
let caps = load_or_create_cap_sids(codex_home)?; let caps = load_or_create_cap_sids(codex_home)?;
let (psid_to_use, cap_sids) = match &policy { let (psid_to_use, cap_sids) = match &policy {
SandboxPolicy::ReadOnly { .. } => ( SandboxPolicy::ReadOnly { .. } => {
unsafe { convert_string_sid_to_sid(&caps.readonly).unwrap() }, #[allow(clippy::unwrap_used)]
vec![caps.readonly.clone()], let psid = unsafe { convert_string_sid_to_sid(&caps.readonly).unwrap() };
), (psid, vec![caps.readonly])
SandboxPolicy::WorkspaceWrite { .. } => ( }
unsafe { convert_string_sid_to_sid(&caps.workspace).unwrap() }, SandboxPolicy::WorkspaceWrite { .. } => {
#[allow(clippy::unwrap_used)]
let psid = unsafe { convert_string_sid_to_sid(&caps.workspace).unwrap() };
(
psid,
vec![ vec![
caps.workspace.clone(), caps.workspace,
crate::cap::workspace_cap_sid_for_cwd(codex_home, cwd)?, crate::cap::workspace_cap_sid_for_cwd(codex_home, cwd)?,
], ],
), )
}
SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } => { SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } => {
unreachable!("DangerFullAccess handled above") unreachable!("DangerFullAccess handled above")
} }
@@ -302,7 +307,7 @@ mod windows_impl {
let runner_exe = find_runner_exe(codex_home, logs_base_dir); let runner_exe = find_runner_exe(codex_home, logs_base_dir);
let runner_cmdline = runner_exe let runner_cmdline = runner_exe
.to_str() .to_str()
.map(|s| s.to_string()) .map(ToString::to_string)
.unwrap_or_else(|| "codex-command-runner.exe".to_string()); .unwrap_or_else(|| "codex-command-runner.exe".to_string());
let runner_full_cmd = format!( let runner_full_cmd = format!(
"{} {} {}", "{} {} {}",
@@ -365,7 +370,7 @@ mod windows_impl {
), ),
logs_base_dir, logs_base_dir,
); );
return Err(anyhow::anyhow!("CreateProcessWithLogonW failed: {}", err)); return Err(anyhow::anyhow!("CreateProcessWithLogonW failed: {err}"));
} }
if let Err(err) = connect_pipe(h_pipe_in) { if let Err(err) = connect_pipe(h_pipe_in) {
@@ -451,7 +456,7 @@ mod windows_impl {
if exit_code == 0 { if exit_code == 0 {
log_success(&command, logs_base_dir); log_success(&command, logs_base_dir);
} else { } else {
log_failure(&command, &format!("exit code {}", exit_code), logs_base_dir); log_failure(&command, &format!("exit code {exit_code}"), logs_base_dir);
} }
Ok(CaptureResult { Ok(CaptureResult {
+4 -4
View File
@@ -48,7 +48,7 @@ fn prepend_path(env_map: &mut HashMap<String, String>, prefix: &str) {
.cloned() .cloned()
.or_else(|| env::var("PATH").ok()) .or_else(|| env::var("PATH").ok())
.unwrap_or_default(); .unwrap_or_default();
let parts: Vec<String> = existing.split(';').map(|s| s.to_string()).collect(); let parts: Vec<String> = existing.split(';').map(ToString::to_string).collect();
if parts if parts
.first() .first()
.map(|p| p.eq_ignore_ascii_case(prefix)) .map(|p| p.eq_ignore_ascii_case(prefix))
@@ -74,7 +74,7 @@ fn reorder_pathext_for_stubs(env_map: &mut HashMap<String, String>) {
let exts: Vec<String> = default let exts: Vec<String> = default
.split(';') .split(';')
.filter(|e| !e.is_empty()) .filter(|e| !e.is_empty())
.map(|s| s.to_string()) .map(ToString::to_string)
.collect(); .collect();
let exts_norm: Vec<String> = exts.iter().map(|e| e.to_ascii_uppercase()).collect(); let exts_norm: Vec<String> = exts.iter().map(|e| e.to_ascii_uppercase()).collect();
let want = [".BAT", ".CMD"]; let want = [".BAT", ".CMD"];
@@ -110,7 +110,7 @@ fn ensure_denybin(tools: &[&str], denybin_dir: Option<&Path>) -> Result<PathBuf>
fs::create_dir_all(&base)?; fs::create_dir_all(&base)?;
for tool in tools { for tool in tools {
for ext in [".bat", ".cmd"] { for ext in [".bat", ".cmd"] {
let path = base.join(format!("{}{}", tool, ext)); let path = base.join(format!("{tool}{ext}"));
if !path.exists() { if !path.exists() {
let mut f = File::create(&path)?; let mut f = File::create(&path)?;
f.write_all(b"@echo off\\r\\nexit /b 1\\r\\n")?; f.write_all(b"@echo off\\r\\nexit /b 1\\r\\n")?;
@@ -162,7 +162,7 @@ pub fn apply_no_network_to_env(env_map: &mut HashMap<String, String>) -> Result<
let base = ensure_denybin(&["ssh", "scp"], /*denybin_dir*/ None)?; let base = ensure_denybin(&["ssh", "scp"], /*denybin_dir*/ None)?;
for tool in ["curl", "wget"] { for tool in ["curl", "wget"] {
for ext in [".bat", ".cmd"] { for ext in [".bat", ".cmd"] {
let p = base.join(format!("{}{}", tool, ext)); let p = base.join(format!("{tool}{ext}"));
if p.exists() { if p.exists() {
let _ = fs::remove_file(&p); let _ = fs::remove_file(&p);
} }
+3 -3
View File
@@ -122,7 +122,7 @@ fn select_identity(
}; };
let password = decode_password(&chosen)?; let password = decode_password(&chosen)?;
Ok(Some(SandboxIdentity { Ok(Some(SandboxIdentity {
username: chosen.username.clone(), username: chosen.username,
password, password,
})) }))
} }
@@ -187,8 +187,8 @@ pub fn require_logon_sandbox_creds(
proxy_enforced, proxy_enforced,
}, },
crate::setup::SetupRootOverrides { crate::setup::SetupRootOverrides {
read_roots: Some(needed_read.clone()), read_roots: Some(needed_read),
write_roots: Some(needed_write.clone()), write_roots: Some(needed_write),
}, },
)?; )?;
identity = select_identity(network_identity, codex_home)?; identity = select_identity(network_identity, codex_home)?;
+13 -5
View File
@@ -335,14 +335,20 @@ mod windows_impl {
let (h_token, psid_generic, psid_workspace): (HANDLE, *mut c_void, Option<*mut c_void>) = unsafe { let (h_token, psid_generic, psid_workspace): (HANDLE, *mut c_void, Option<*mut c_void>) = unsafe {
match &policy { match &policy {
SandboxPolicy::ReadOnly { .. } => { SandboxPolicy::ReadOnly { .. } => {
let psid = convert_string_sid_to_sid(&caps.readonly).unwrap(); #[allow(clippy::expect_used)]
let psid =
convert_string_sid_to_sid(&caps.readonly).expect("valid readonly SID");
let (h, _) = super::token::create_readonly_token_with_cap(psid)?; let (h, _) = super::token::create_readonly_token_with_cap(psid)?;
(h, psid, None) (h, psid, None)
} }
SandboxPolicy::WorkspaceWrite { .. } => { SandboxPolicy::WorkspaceWrite { .. } => {
let psid_generic = convert_string_sid_to_sid(&caps.workspace).unwrap(); #[allow(clippy::expect_used)]
let psid_generic =
convert_string_sid_to_sid(&caps.workspace).expect("valid workspace SID");
let ws_sid = workspace_cap_sid_for_cwd(codex_home, cwd)?; let ws_sid = workspace_cap_sid_for_cwd(codex_home, cwd)?;
let psid_workspace = convert_string_sid_to_sid(&ws_sid).unwrap(); #[allow(clippy::expect_used)]
let psid_workspace =
convert_string_sid_to_sid(&ws_sid).expect("valid workspace SID");
let base = super::token::get_current_token_for_restriction()?; let base = super::token::get_current_token_for_restriction()?;
let h_res = create_workspace_write_token_with_caps_from( let h_res = create_workspace_write_token_with_caps_from(
base, base,
@@ -363,7 +369,7 @@ mod windows_impl {
&& let Ok(base) = super::token::get_current_token_for_restriction() && let Ok(base) = super::token::get_current_token_for_restriction()
{ {
if let Ok(bytes) = super::token::get_logon_sid_bytes(base) { if let Ok(bytes) = super::token::get_logon_sid_bytes(base) {
let mut tmp = bytes.clone(); let mut tmp = bytes;
let psid2 = tmp.as_mut_ptr() as *mut c_void; let psid2 = tmp.as_mut_ptr() as *mut c_void;
allow_null_device(psid2); allow_null_device(psid2);
} }
@@ -536,7 +542,7 @@ mod windows_impl {
if exit_code == 0 { if exit_code == 0 {
log_success(&command, logs_base_dir); log_success(&command, logs_base_dir);
} else { } else {
log_failure(&command, &format!("exit code {}", exit_code), logs_base_dir); log_failure(&command, &format!("exit code {exit_code}"), logs_base_dir);
} }
if !persist_aces { if !persist_aces {
@@ -569,9 +575,11 @@ mod windows_impl {
ensure_codex_home_exists(codex_home)?; ensure_codex_home_exists(codex_home)?;
let caps = load_or_create_cap_sids(codex_home)?; let caps = load_or_create_cap_sids(codex_home)?;
#[allow(clippy::expect_used)]
let psid_generic = let psid_generic =
unsafe { convert_string_sid_to_sid(&caps.workspace) }.expect("valid workspace SID"); unsafe { convert_string_sid_to_sid(&caps.workspace) }.expect("valid workspace SID");
let ws_sid = workspace_cap_sid_for_cwd(codex_home, cwd)?; let ws_sid = workspace_cap_sid_for_cwd(codex_home, cwd)?;
#[allow(clippy::expect_used)]
let psid_workspace = let psid_workspace =
unsafe { convert_string_sid_to_sid(&ws_sid) }.expect("valid workspace SID"); unsafe { convert_string_sid_to_sid(&ws_sid) }.expect("valid workspace SID");
let current_dir = cwd.to_path_buf(); let current_dir = cwd.to_path_buf();
+2 -2
View File
@@ -43,7 +43,7 @@ pub fn make_env_block(env: &HashMap<String, String>) -> Vec<u16> {
}); });
let mut w: Vec<u16> = Vec::new(); let mut w: Vec<u16> = Vec::new();
for (k, v) in items { for (k, v) in items {
let mut s = to_wide(format!("{}={}", k, v)); let mut s = to_wide(format!("{k}={v}"));
s.pop(); s.pop();
w.extend_from_slice(&s); w.extend_from_slice(&s);
w.push(0); w.push(0);
@@ -149,7 +149,7 @@ pub unsafe fn create_process_as_user(
creation_flags, creation_flags,
); );
logging::debug_log(&msg, logs_base_dir); logging::debug_log(&msg, logs_base_dir);
return Err(anyhow!("CreateProcessAsUserW failed: {}", err)); return Err(anyhow!("CreateProcessAsUserW failed: {err}"));
} }
Ok(CreatedProcess { Ok(CreatedProcess {
process_info: pi, process_info: pi,
@@ -35,7 +35,7 @@ pub fn read_acl_mutex_exists() -> Result<bool> {
if err == ERROR_FILE_NOT_FOUND { if err == ERROR_FILE_NOT_FOUND {
return Ok(false); return Ok(false);
} }
return Err(anyhow::anyhow!("OpenMutexW failed: {}", err)); return Err(anyhow::anyhow!("OpenMutexW failed: {err}"));
} }
unsafe { unsafe {
CloseHandle(handle); CloseHandle(handle);
@@ -312,8 +312,7 @@ fn lock_sandbox_dir(
); );
if set != 0 { if set != 0 {
return Err(anyhow::anyhow!( return Err(anyhow::anyhow!(
"SetEntriesInAclW sandbox dir failed: {}", "SetEntriesInAclW sandbox dir failed: {set}",
set
)); ));
} }
let path_w = to_wide(dir.as_os_str()); let path_w = to_wide(dir.as_os_str());
@@ -328,8 +327,7 @@ fn lock_sandbox_dir(
); );
if res != 0 { if res != 0 {
return Err(anyhow::anyhow!( return Err(anyhow::anyhow!(
"SetNamedSecurityInfoW sandbox dir failed: {}", "SetNamedSecurityInfoW sandbox dir failed: {res}",
res
)); ));
} }
if !new_dacl.is_null() { if !new_dacl.is_null() {
@@ -424,7 +422,7 @@ fn real_main() -> Result<()> {
}); });
let report = SetupErrorReport { let report = SetupErrorReport {
code: failure.code, code: failure.code,
message: failure.message.clone(), message: failure.message,
}; };
if let Err(write_err) = write_setup_error_report(&payload.codex_home, &report) { if let Err(write_err) = write_setup_error_report(&payload.codex_home, &report) {
let _ = log_line( let _ = log_line(
@@ -496,7 +494,7 @@ fn run_read_acl_only(payload: &Payload, log: &mut File) -> Result<()> {
if !refresh_errors.is_empty() { if !refresh_errors.is_empty() {
log_line( log_line(
log, log,
&format!("read ACL run completed with errors: {:?}", refresh_errors), &format!("read ACL run completed with errors: {refresh_errors:?}"),
)?; )?;
if payload.refresh_only { if payload.refresh_only {
anyhow::bail!("read ACL run had errors"); anyhow::bail!("read ACL run had errors");
@@ -637,7 +635,7 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<(
} }
} }
let cap_sid_str = caps.workspace.clone(); let cap_sid_str = caps.workspace;
let sandbox_group_sid_str = let sandbox_group_sid_str =
string_from_sid_bytes(&sandbox_group_sid).map_err(anyhow::Error::msg)?; string_from_sid_bytes(&sandbox_group_sid).map_err(anyhow::Error::msg)?;
let write_mask = let write_mask =
@@ -895,7 +893,7 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<(
if refresh_only && !refresh_errors.is_empty() { if refresh_only && !refresh_errors.is_empty() {
log_line( log_line(
log, log,
&format!("setup refresh completed with errors: {:?}", refresh_errors), &format!("setup refresh completed with errors: {refresh_errors:?}"),
)?; )?;
anyhow::bail!("setup refresh had errors"); anyhow::bail!("setup refresh had errors");
} }
@@ -201,7 +201,7 @@ fn run_setup_refresh_inner(
&format!("setup refresh: exited with status {status:?}"), &format!("setup refresh: exited with status {status:?}"),
Some(&sandbox_dir(request.codex_home)), Some(&sandbox_dir(request.codex_home)),
); );
return Err(anyhow!("setup refresh failed with status {}", status)); return Err(anyhow!("setup refresh failed with status {status}"));
} }
Ok(()) Ok(())
} }
+3 -4
View File
@@ -78,7 +78,7 @@ unsafe fn set_default_dacl(h_token: HANDLE, sids: &[*mut c_void]) -> Result<()>
&mut p_new_dacl, &mut p_new_dacl,
); );
if res != ERROR_SUCCESS { if res != ERROR_SUCCESS {
return Err(anyhow!("SetEntriesInAclW failed: {}", res)); return Err(anyhow!("SetEntriesInAclW failed: {res}"));
} }
let mut info = TokenDefaultDaclInfo { let mut info = TokenDefaultDaclInfo {
default_dacl: p_new_dacl, default_dacl: p_new_dacl,
@@ -95,8 +95,7 @@ unsafe fn set_default_dacl(h_token: HANDLE, sids: &[*mut c_void]) -> Result<()>
LocalFree(p_new_dacl as HLOCAL); LocalFree(p_new_dacl as HLOCAL);
} }
return Err(anyhow!( return Err(anyhow!(
"SetTokenInformation(TokenDefaultDacl) failed: {}", "SetTokenInformation(TokenDefaultDacl) failed: {err}",
err
)); ));
} }
if !p_new_dacl.is_null() { if !p_new_dacl.is_null() {
@@ -277,7 +276,7 @@ unsafe fn enable_single_privilege(h_token: HANDLE, name: &str) -> Result<()> {
} }
let err = GetLastError(); let err = GetLastError();
if err != 0 { if err != 0 {
return Err(anyhow!("AdjustTokenPrivileges error {}", err)); return Err(anyhow!("AdjustTokenPrivileges error {err}"));
} }
Ok(()) Ok(())
} }
+1 -1
View File
@@ -83,7 +83,7 @@ pub fn format_last_error(err: i32) -> String {
std::ptr::null_mut(), std::ptr::null_mut(),
); );
if len == 0 || buf_ptr.is_null() { if len == 0 || buf_ptr.is_null() {
return format!("Win32 error {}", err); return format!("Win32 error {err}");
} }
let slice = std::slice::from_raw_parts(buf_ptr, len as usize); let slice = std::slice::from_raw_parts(buf_ptr, len as usize);
let mut s = String::from_utf16_lossy(slice); let mut s = String::from_utf16_lossy(slice);