Commit Graph

3 Commits

Author SHA1 Message Date
Michael Bolin
fa2a2f0be9 Use released DotSlash package for argument-comment lint (#15199)
## 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`
2026-03-20 03:19:22 +00:00
iceweasel-oai
95bdea93d2 use framed IPC for elevated command runner (#14846)
## Summary
This is PR 2 of the Windows sandbox runner split.

PR 1 introduced the framed IPC runner foundation and related Windows
sandbox infrastructure without changing the active elevated one-shot
execution path. This PR switches that elevated one-shot path over to the
new runner IPC transport and removes the old request-file bootstrap that
PR 1 intentionally left in place.

After this change, ordinary elevated Windows sandbox commands still
behave as one-shot executions, but they now run as the simple case of
the same helper/IPC transport that later unified_exec work will build
on.

## Why this is needed for unified_exec
Windows elevated sandboxed execution crosses a user boundary: the CLI
launches a helper as the sandbox user and has to manage command
execution from outside that security context. For one-shot commands, the
old request-file/bootstrap flow was sufficient. For unified_exec, it is
not.

Unified_exec needs a long-lived bidirectional channel so the parent can:
- send a spawn request
- receive structured spawn success/failure
- stream stdout and stderr incrementally
- eventually support stdin writes, termination, and other session
lifecycle events

This PR does not add long-lived sessions yet. It converts the existing
elevated one-shot path to use the same framed IPC transport so that PR 3
can add unified_exec session semantics on top of a transport that is
already exercised by normal elevated command execution.

## Scope
This PR:
- updates `windows-sandbox-rs/src/elevated_impl.rs` to launch the runner
with named pipes, send a framed `SpawnRequest`, wait for `SpawnReady`,
and collect framed `Output`/`Exit` messages
- removes the old `--request-file=...` execution path from
`windows-sandbox-rs/src/elevated/command_runner_win.rs`
- keeps the public behavior one-shot: no session reuse or interactive
unified_exec behavior is introduced here

This PR does not:
- add Windows unified_exec session support
- add background terminal reuse
- add PTY session lifecycle management

## Why Windows needs this and Linux/macOS do not
On Linux and macOS, the existing sandbox/process model composes much
more directly with long-lived process control. The parent can generally
spawn and own the child process (or PTY) directly inside the sandbox
model we already use.

Windows elevated sandboxing is different. The parent is not directly
managing the sandboxed process in the same way; it launches across a
different user/security context. That means long-lived control requires
an explicit helper process plus IPC for spawn, output, exit, and later
stdin/session control.

So the extra machinery here is not because unified_exec is conceptually
different on Windows. It is because the elevated Windows sandbox
boundary requires a helper-mediated transport to support it cleanly.

## Validation
- `cargo test -p codex-windows-sandbox`
2026-03-17 11:38:44 -07:00
iceweasel-oai
d0a693e541 windows-sandbox: add runner IPC foundation for future unified_exec (#14139)
# Summary

This PR introduces the Windows sandbox runner IPC foundation that later
unified_exec work will build on.

The key point is that this is intentionally infrastructure-only. The new
IPC transport, runner plumbing, and ConPTY helpers are added here, but
the active elevated Windows sandbox path still uses the existing
request-file bootstrap. In other words, this change prepares the
transport and module layout we need for unified_exec without switching
production behavior over yet.

Part of this PR is also a source-layout cleanup: some Windows sandbox
files are moved into more explicit `elevated/`, `conpty/`, and shared
locations so it is clearer which code is for the elevated sandbox flow,
which code is legacy/direct-spawn behavior, and which helpers are shared
between them. That reorganization is intentional in this first PR so
later behavioral changes do not also have to carry a large amount of
file-move churn.

# Why This Is Needed For unified_exec

Windows elevated sandboxed unified_exec needs a long-lived,
bidirectional control channel between the CLI and a helper process
running under the sandbox user. That channel has to support:

- starting a process and reporting structured spawn success/failure
- streaming stdout/stderr back incrementally
- forwarding stdin over time
- terminating or polling a long-lived process
- supporting both pipe-backed and PTY-backed sessions

The existing elevated one-shot path is built around a request-file
bootstrap and does not provide those primitives cleanly. Before we can
turn on Windows sandbox unified_exec, we need the underlying runner
protocol and transport layer that can carry those lifecycle events and
streams.

# Why Windows Needs More Machinery Than Linux Or macOS

Linux and macOS can generally build unified_exec on top of the existing
sandbox/process model: the parent can spawn the child directly, retain
normal ownership of stdio or PTY handles, and manage the lifetime of the
sandboxed process without introducing a second control process.

Windows elevated sandboxing is different. To run inside the sandbox
boundary, we cross into a different user/security context and then need
to manage a long-lived process from outside that boundary. That means we
need an explicit helper process plus an IPC transport to carry spawn,
stdin, output, and exit events back and forth. The extra code here is
mostly that missing Windows sandbox infrastructure, not a conceptual
difference in unified_exec itself.

# What This PR Adds

- the framed IPC message types and transport helpers for parent <->
runner communication
- the renamed Windows command runner with both the existing request-file
bootstrap and the dormant IPC bootstrap
- named-pipe helpers for the elevated runner path
- ConPTY helpers and process-thread attribute plumbing needed for
PTY-backed sessions
- shared sandbox/process helpers that later PRs will reuse when
switching live execution paths over
- early file/module moves so later PRs can focus on behavior rather than
layout churn

# What This PR Does Not Yet Do

- it does not switch the active elevated one-shot path over to IPC yet
- it does not enable Windows sandbox unified_exec yet
- it does not remove the existing request-file bootstrap yet

So while this code compiles and the new path has basic validation, it is
not yet the exercised production path. That is intentional for this
first PR: the goal here is to land the transport and runner foundation
cleanly before later PRs start routing real command execution through
it.

# Follow-Ups

Planned follow-up PRs will:

1. switch elevated one-shot Windows sandbox execution to the new runner
IPC path
2. layer Windows sandbox unified_exec sessions on top of the same
transport
3. remove the legacy request-file path once the IPC-based path is live

# Validation

- `cargo build -p codex-windows-sandbox`
2026-03-16 19:45:06 +00:00