Conversation
|
@codex review |
![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: 2461e575a6
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 90e76c9581
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 49571484d5
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f54bab897f
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 14a1d7c666
ℹ️ 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".
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
74917bf to
7b7dca4
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7b7dca479c
ℹ️ 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".
There was a problem hiding this comment.
Pull request overview
Adds durable “crash receipts” for controlled-session watchdog parent-loss events, while also tightening controlled-session Docker endpoint pinning and making runtime identity/account handling more architecture- and host-accurate.
Changes:
- Persist bounded, allowlisted controlled-session incident receipts on watchdog parent loss, with lock-protected listing + explicit acknowledgement APIs.
- Pin controlled-session operations (supervisor, watchdog cleanup, and recovery) to one verified local Docker endpoint recorded in durable ownership state.
- Improve identity/account correctness: accept full unsigned 32-bit UID/GID parsing in the probe and preserve root’s nonzero primary GID in container-local account generation.
Reviewed changes
Copilot reviewed 44 out of 44 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/probe/sandbox_exec.go | Switch UID/GID parsing to canonical uint32 credentials and add list parsing for supplementary groups. |
| internal/probe/sandbox_exec_test.go | Update expectations for uint32 groups and add full-range credential coverage + new failure cases. |
| internal/probe/sandbox_exec_linux.go | Update privilege-dropping to accept uint32 IDs and convert to native ints for x/sys calls. |
| internal/probe/local_account_linux.go | Preserve root group naming while allowing root UID with nonzero primary GID. |
| internal/probe/local_account_linux_test.go | Add coverage for root-with-nonzero-primary-GID behavior; adjust mismatch tests. |
| internal/dockerdeploy/runtime_host_filesystem_linux_test.go | Add regression test for ordinary paths on root mount vs host-root aliases. |
| internal/dockerdeploy/live_runs_test.go | Update controlled-session ownership recording to include durable Docker endpoint. |
| internal/dockerdeploy/live_run_recovery.go | Bind recovered cleanup to the recorded Docker endpoint before container operations. |
| internal/dockerdeploy/live_run_recovery_test.go | Ensure recovery uses the pinned endpoint and preserves incident receipts across cleanup. |
| internal/dockerdeploy/execution_plan_test.go | Update expectations for “root current-user” planning with nonzero primary GID. |
| internal/dockerdeploy/controlled_session_workload_pty.go | Add backend endpoint binding and ensure all lifecycle operations reuse one pinned Docker command spec. |
| internal/dockerdeploy/controlled_session_workload_pty_test.go | Add test asserting a single endpoint bind and consistent DOCKER_HOST/DOCKER_CONTEXT across operations. |
| internal/dockerdeploy/controlled_session_watchdog.go | On parent loss, produce a bounded incident receipt capturing allowlisted cleanup outcomes; optionally bind Docker endpoint. |
| internal/dockerdeploy/controlled_session_watchdog_unsupported.go | Update watchdog start signature for incident receipt target wiring (unsupported platforms). |
| internal/dockerdeploy/controlled_session_watchdog_test.go | Add receipt-related test coverage and time injection for deterministic timestamps. |
| internal/dockerdeploy/controlled_session_watchdog_linux.go | Pass an inherited incident receipt FD to the watchdog child; remove unused targets on disarm/early exit. |
| internal/dockerdeploy/controlled_session_watchdog_integration_linux_test.go | Integration coverage for receipt persistence after parent loss and subsequent retrieval. |
| internal/dockerdeploy/controlled_session_supervisor.go | Bind one verified local Docker endpoint, persist it in ownership, and pre-create/manage the incident receipt target for the watchdog. |
| internal/dockerdeploy/controlled_session_supervisor_test.go | Add coverage for single endpoint selection/binding across both containers; update ownership fixtures. |
| internal/dockerdeploy/controlled_session_incident.go | Add read-only incident receipt listing and explicit acknowledgement surfaces. |
| internal/dockerdeploy/controlled_session_incident_test.go | Verify retrieval is read-only and acknowledgements remove exactly one validated receipt. |
| internal/dockerdeploy/controlled_session_controller_test.go | Introduce a shared test Docker endpoint constant and update usage. |
| internal/dockerdeploy/control_admission_modes_test.go | Update controlled-session ownership fixtures to include Docker endpoint. |
| internal/dockerdeploy/compose.go | Add a helper to wrap a command runner with an already-verified pinned Docker endpoint. |
| internal/dockerdeploy/compose_test.go | Add coverage for the pinned-endpoint command runner wrapper and remote-endpoint rejection. |
| internal/dockerdeploy/application_sandbox_plan_test.go | Add coverage for root runtime identity with nonzero primary GID. |
| internal/dockerdeploy/application_runtime_layer_test.go | Ensure generated Dockerfile preserves root’s primary GID in install-local-account invocation. |
| internal/deploy/runtime_verifier.go | Permit root local accounts to use nonzero primary GID (still forbids non-root with GID 0). |
| internal/deploy/live_run_queue.go | Extend durable controlled-session ownership with docker_endpoint and validate it as a local endpoint. |
| internal/deploy/live_run_queue_file_test.go | Update fixtures to include the new durable Docker endpoint field. |
| internal/deploy/controlled_session_incident_receipt.go | New core receipt format + retention, locking, prepare/read/ack APIs, and canonical encoding/decoding. |
| internal/deploy/controlled_session_incident_receipt_test.go | Validate allowlist-only receipt content, retention behavior, and empty-target cleanup semantics. |
| internal/deploy/controlled_session_incident_lock_windows.go | Windows-specific “in use” check behavior for receipt targets (watchdog is Linux-only). |
| internal/deploy/controlled_session_incident_lock_unix.go | Unix advisory locking and “in use” checks for receipt targets. |
| internal/deploy/controlled_session_incident_directory_windows.go | Windows directory creation + ACL validation for private incident receipt directory. |
| internal/deploy/controlled_session_incident_directory_unix.go | Unix private incident directory creation + permission validation. |
| internal/deploy/controlled_session_cleanup_manifest.go | Add durable docker endpoint + exact incident receipt path to watchdog cleanup manifests. |
| internal/deploy/controlled_session_cleanup_manifest_test.go | Add coverage for remote endpoint rejection and incident receipt path derivation. |
| docs/CONTROLLED_SESSION_DESIGN.md | Document pinned Docker endpoint identity and durable crash receipt behavior/constraints. |
| docs/BLUEPRINT_ENVIRONMENT_MODEL.md | Document preserving root’s effective primary GID (no normalization to 0:0). |
| .changes/unreleased/+root-nonzero-primary-gid.yaml | Release note fragment for root primary-GID preservation fix. |
| .changes/unreleased/+controlled-session-docker-endpoint-pinning.yaml | Release note fragment for endpoint pinning security hardening. |
| .changes/unreleased/+controlled-session-crash-receipts.yaml | Release note fragment for durable crash receipts feature. |
| .changes/unreleased/+armv7-full-range-runtime-ids.yaml | Release note fragment for unsigned 32-bit runtime ID preservation on 32-bit targets. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| argv := args[separator+1:] | ||
| if *uid < 0 || *gid < 0 { | ||
| return sandboxExecPlanV1{}, fmt.Errorf("requires non-negative --uid and --gid") | ||
| parsedUID, err := parseCredentialV1(*uid) | ||
| if err != nil { | ||
| return sandboxExecPlanV1{}, fmt.Errorf("parse --uid: %w", err) |
| @@ -66,6 +67,7 @@ type ControlledSessionOwnershipV1 struct { | |||
| LiveRunID string `json:"live_run_id"` | |||
| BootSession string `json:"boot_session"` | |||
| SessionHandle string `json:"session_handle"` | |||
| DockerEndpoint string `json:"docker_endpoint"` | |||
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b9cf342ea4
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 83704da9bf
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a673fba540
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e743e67d9c
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5815f2c24d
ℹ️ 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".
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Stack created with Sapling. Best reviewed with ReviewStack.