Add exec-server process and filesystem RPCs#15090
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 79b46e346d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if let Some(process) = processes.get_mut(&process_id) { | ||
| process.exit_code = Some(exit_code); | ||
| } |
There was a problem hiding this comment.
watch_exit only sets exit_code and never evicts the RunningProcess entry. Each finished process keeps retained output (up to RETAINED_OUTPUT_BYTES_PER_PROCESS) and session state until connection shutdown, so long-lived connections that run many short-lived processes can accumulate unbounded memory/state.
Useful? React with 👍 / 👎.
| if let Some(process) = process_map.get(¶ms.process_id) { | ||
| process.session.terminate(); | ||
| true | ||
| } else { |
There was a problem hiding this comment.
terminate returns running: true whenever process_id exists in the map, without checking whether the process has already exited. Since exited entries are retained, terminate can claim a process is running when it is not, which breaks caller logic that relies on this flag.
Useful? React with 👍 / 👎.
| loop { | ||
| let (stream, peer_addr) = listener.accept().await?; | ||
| tokio::spawn(async move { | ||
| match accept_async(stream).await { | ||
| Ok(websocket) => { | ||
| run_connection(JsonRpcConnection::from_websocket( | ||
| websocket, |
There was a problem hiding this comment.
The websocket listener accepts any incoming peer and immediately starts a JSON-RPC exec session. Combined with unauthenticated initialize flow, binding to non-loopback addresses allows arbitrary remote clients to call process/start and execute commands (unauthenticated RCE).
Useful? React with 👍 / 👎.
starr-openai
force-pushed
the
starr/exec-server-exec-fresh
branch
2 times, most recently
from
c5071a9 to
84a6cbe
Compare
| stream, | ||
| chunk: chunk.clone(), | ||
| }); | ||
| while process.retained_bytes > RETAINED_OUTPUT_BYTES_PER_PROCESS { |
There was a problem hiding this comment.
unified exec needs the tail. we might need the same "drop middle strategy" as used in core.
Co-authored-by: Codex <[email protected]>
starr-openai
force-pushed
the
starr/exec-server-exec-fresh
branch
from
54d4423 to
dd1416a
Compare
starr-openai
changed the title
Co-authored-by: Codex <[email protected]>
2 participants
Rebased onto current
mainafter #15091 landed separately.This PR now carries the surviving exec-server process RPC and filesystem RPC implementation on top of the current exec-server stub / websocket-only / environment-abstraction shape on
main.It also includes the
rust-cisccache guard formacos-15-xlarge+x86_64-apple-darwinso this branch does not reproduce the same mixed-architectureringfailure that hit the earlier stack.