Repository navigation
feat(delivery): Frame untrusted peer messages at native boundaries - #285
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 54c3f5bfda
ℹ️ 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 peer_fingerprint is not None: | ||
| # Receiver-controlled code travels on private stdin; never add an | ||
| # argv/env/body switch that lets a sender assert authenticated identity. | ||
| core.peer_delivery_message('', 'codex', peer_fingerprint) |
There was a problem hiding this comment.
Budget the framed Windows Codex command line
When a WSL paired-device message contains many escaped controls or line breaks, this call validates that the final frame fits 32 KiB but does not check the Windows command line that the streamed core subsequently uses to launch codex.exe; the earlier command_units calculation covers only the much shorter unframed Python argv. For example, 5,361 \x01 characters produce an accepted 32,767-character frame but an inner Codex command of about 32,861 UTF-16 units, so CreateProcessW fails before Codex starts and the bridge incorrectly reports native_outcome_unknown. Compute the limit from the final framed Codex argv so this is refused deterministically before launching the native worker.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in the latest head: the shared Codex argv builder now budgets the fully framed inner command as well as the raw Python bridge command before native execution. Both send and resolve cover expanded controls/newlines, quoting and Unicode path boundaries, with known refusal and no launch on overflow. Ordinary Windows queue/dry-run uses the same guard. Focused fixtures passed; the strict full suite passed with the separately noted existing asynchronous teardown diagnostic. These are synthetic macOS checks, not live Windows delivery evidence.
Summary
Refs #232 (v1.1.0).
charsacross SSH.Type
Checklist
Verified against
macOS arm64; dependency-free core/docs checks on Python 3.14.8 and an isolated worktree Python 3.13.7 environment with test-only packaging 26.3, cryptography 50.0.2, websockets 17.2 and MCP 1.30.0. These dependencies were installed only in this worktree, not the user's/shared runtime.
Local checks include standalone generator, docs/i18n/link/command tests, core/adapter/queue fixtures, receiver byte budgets and real loopback socket/TLS fixtures. No actual Claude/Codex/Antigravity model session or remote machine received a message. Windows bridge context is synthetic; Windows-native API/platform acceptance remains CI/manual review, not a claim from macOS tests.
.venv/bin/python -X dev -W error::ResourceWarning -m unittest discover -s tests -t . -q: 802 tests, 12 expected skips, zero failures/errors; final strict run completed without ResourceWarning or asyncio AssertionError.python3 tools/generate_session_peer.py --checkandgit diff --check: pass.Notes
Framing changes the presentation and delivered character count; it does not make an agent immune to prompt injection, authenticate a local/SSH sender session, grant permissions, authenticate a person via a device key, confirm consumption, or authorize a third-party reply. Existing Reply-To v1 parsing and ordinary native JSON field absence/ACK semantics remain intact. Direct native CLI/API calls outside session-peer are not covered by this framing.
The companion skill is managed and released independently. Its exact compatibility snapshot remains unchanged; #232 stays open for that documented acceptance item and final review. No merge, tag, release, package publication, operating-service change or frozen handoff-contract edit is included.