Conversation
|
@codex review |
There was a problem hiding this comment.
Pull request overview
Implements a host-owned, lease-private controlled-session controller channel (Unix socket) and wires it into Docker planning so the controller can connect via a read-only bind mount while the workload remains isolated.
Changes:
- Add
controlledsession.PrivateChannelV1with Linux transport that creates a controller-owned socket directory, verifies peer UID/GID, supports a single claim, and serializes framed I/O with cancellation support. - Update controlled-session Docker planning to mount the session-channel directory read-only for the controller and to use the shared socket-name constant.
- Add unit + Docker integration coverage, including a tiny helper binary used to assert controller-only read-only behavior inside a container.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| internal/dockerdeploy/testdata/session_channel_helper/main.go | Helper program used by Docker integration tests to connect to the private channel and assert the bind mount is not writable. |
| internal/dockerdeploy/testdata/session_channel_helper/dial_unsupported.go | Non-Linux stub for the helper’s Unix-dial function. |
| internal/dockerdeploy/testdata/session_channel_helper/dial_unix.go | Linux implementation of the helper’s Unix-dial function. |
| internal/dockerdeploy/controlled_session_plan.go | Uses the shared private-channel socket name and mounts the session-channel read-only for the controller. |
| internal/dockerdeploy/controlled_session_channel.go | Adds plan-to-private-channel configuration and preparation helpers in dockerdeploy. |
| internal/dockerdeploy/controlled_session_channel_test.go | Unit tests asserting channel config uses only frozen plan authority and rejects mutated plans. |
| internal/dockerdeploy/controlled_session_channel_integration_test.go | Docker integration test proving the controller can connect while the bind mount is read-only. |
| internal/controlledsession/channel.go | Core private channel + controller connection implementation (claim lifecycle, framing I/O, cancellation/deadlines). |
| internal/controlledsession/channel_unsupported.go | Non-Linux stubs for private-channel transport + disconnect detection. |
| internal/controlledsession/channel_linux.go | Linux transport: creates/chowns/chmods socket dir, accepts one connection, reads SO_PEERCRED UID/GID. |
| internal/controlledsession/channel_linux_test.go | Linux unit tests for claim behavior, failure classification, identity checks, and bounded/serialized I/O. |
| docs/CONTROLLED_SESSION_DESIGN.md | Updates design doc to describe the new lease-private session channel behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Codex Review: Didn't find any major issues. Hooray! 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 |
|
Codex Review: Didn't find any major issues. Delightful! 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.