fix(pty): don't leak an outer session id into session-less PTYs - #178
Conversation
|
@codex review |
|
@codex security review |
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. |
|
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 Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
What
resolvePtyEnv(src/pty/createPty.ts) now treats a PTY spawned without asessionIdas session-less:AGENT_TTY_ACTIVEandAGENT_TTY_SESSION_IDare removed. Before,AGENT_TTY_ACTIVE=truewas always set and an outerAGENT_TTY_SESSION_IDpassed through unchanged.env(--env) still wins, so precedence is unchanged.sessionIdnow fails aninvariantrather than being silently treated as "no session".Sessions (
hostMain, which always passessessionId) behave exactly as before. The only session-less caller is thedoctorPTY spawn probe. Run inside an agent-tty session, it no longer claims to be a managed session or reports the outer session's ID.Why
Closes #177. The documented contract in
docs/USAGE.md("a session reports its own ID") didn't hold for PTYs without a session.Changes
src/pty/createPty.ts: drop inherited session vars when there's nosessionId; doc comment updated.test/unit/pty/createPty.test.ts: covers stripping inherited values with no session id, callerenvstill winning with no session id, no vars set with no session id, rejecting an empty session id. Existing session tests are kept.docs/USAGE.md: one sentence on session-less PTYs.No CLI JSON, schema, or manifest changes.
Validation
Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:high