fix(agents): restore exec host=node routing and per-call override from auto#60788
Conversation
Greptile SummaryRestores two broken routing paths in Confidence Score: 5/5Safe to merge; all findings are P2 style/documentation suggestions that do not affect runtime correctness. Both logic fixes are correct and well-tested. The only open items are a missing test for the elevated+node+mismatched-requestedTarget silent-discard path (P2) and an inaccuracy in the PR's cited sandbox-escape mitigations — the real guard is the approval/ask flow, not lines 1338/1322 (P2 documentation concern, no code change needed). No files require special attention. Prompt To Fix All With AIThis is a comment left during a code review.
Path: src/agents/bash-tools.exec-runtime.ts
Line: 249-261
Comment:
**Elevated path ignores per-call `requestedTarget` override silently**
When `elevatedRequested=true` the function returns immediately based solely on `configuredTarget`, completely discarding `requestedTarget`. That means a caller passing `{ configuredTarget: "node", requestedTarget: "gateway", elevatedRequested: true }` gets back `effectiveHost: "node"` with no error and no log entry — the gateway request is silently dropped.
The test suite only exercises `requestedTarget: "node"` under the elevated+node path. Adding a case for a mismatched `requestedTarget` (e.g. `node`/`gateway`) would make the silent-discard behaviour explicit and prevent confusion if the elevated override logic is revisited later.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: src/agents/bash-tools.exec-runtime.ts
Line: 228-234
Comment:
**PR's cited sandbox-isolation mitigations don't fully apply to the `auto`→gateway override path**
The PR description cites two protections against sandbox escape via a per-call `host=gateway` override from an `auto` session:
- `bash-tools.exec.ts` line 1338 (`target.selectedTarget === "sandbox" && !sandbox`): this only fires when *sandbox is requested but unavailable*; it never fires when `selectedTarget === "gateway"`, so it does not prevent the escape.
- `bash-tools.exec.ts` line 1322 (`host === "sandbox" ? "deny" : "full"`): when the resolved host is `"gateway"`, the default `security` is `"full"` — *less* restrictive than the sandbox `"deny"` default, meaning the command allow-list gets wider, not narrower.
The actual safety net for this path is the `ask`/approval flow, which is unchanged. This is a documentation/rationale accuracy issue, not a code bug — the behaviour change itself is intentional and the approval layer does still apply. Future security reviewers relying on the cited lines as the isolation guarantee will be misled.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix(agents): resolve exec host=node rout..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![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: 6fd0b63f22
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
6fd0b63 to
32afe6d
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 32afe6ddc0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
32afe6d to
7b480da
Compare
obviyus
left a comment
obviyus
left a comment
There was a problem hiding this comment.
Reviewed latest changes; landing now.
7b480da to
f2815d0
Compare
2 participants
Summary
Problem: Since 2026.4.2,
exec host=nodeno longer routes commands to paired nodes — every command executes on the gateway instead. All four configuration paths are broken: globaltools.exec.host: "node"silently runs on gateway;tools.exec.host: "auto"with per-callhost=nodethrows"exec host not allowed"; subagenthost=nodealso throws; and disabling elevated does not help. This leaves zero remaining paths to execute commands on paired nodes from agents ([Bug]: exec host=node regression in 2026.4.2 — agent exec always routes to gateway #60772, same root cause as stale [Bug]: Agent exec ignores node binding — always routes to gateway despite correct config #20669).Root Cause: Two regressions in
resolveExecTarget()andisRequestedExecTargetAllowed()insrc/agents/bash-tools.exec-runtime.ts:elevatedRequestedunconditionally returnseffectiveHost: "gateway", ignoringconfiguredTarget: "node". When a channel has elevated defaults enabled (defaultLevel: "on"), the elevated mode resolves to"ask"→elevatedRequested = true→ gateway forced, even though the user explicitly configuredhost=node.isRequestedExecTargetAllowedusesrequestedTarget === configuredTarget. WhenconfiguredTargetis"auto", any per-call override to a concrete host (node,gateway,sandbox) is rejected. The comment says "auto is a routing strategy, not a wildcard allowlist", but this contradicts the documented behavior whereautoshould allow runtime host selection.Fix:
configuredTarget === "node", preserveeffectiveHost: "node"instead of forcing gateway. The node's own approval/security layer handles elevated semantics on the remote host. All other configured targets (auto,sandbox,gateway) still redirect elevated to gateway as before.isRequestedExecTargetAllowedto returntruewhenconfiguredTarget === "auto", enabling per-call overrides to any concrete host. Non-auto concrete targets still require exact match to prevent silent host-hopping.What changed:
src/agents/bash-tools.exec-runtime.ts: FixedisRequestedExecTargetAllowedto allow per-call overrides underauto; fixedresolveExecTargetelevated path to honournodebinding.src/agents/bash-tools.exec-runtime.test.ts: Updated 2 existing tests and added 5 new tests covering auto→node override, auto→gateway override, auto→sandbox override, cross-host rejection for concrete targets, elevated+node routing, and elevated+node without per-call override.What did NOT change (scope boundary): Node transport layer (
bash-tools.exec-host-node.ts), approval policy resolution (exec-approvals.ts), sandbox isolation checks inbash-tools.exec.ts, and the elevated gate logic increateExecTool.executeare all untouched. The fix is strictly limited to target selection inresolveExecTarget.Reproduction
system.runcapability advertised).tools.exec.host: "node"andtools.exec.node: "<node-id>"in config.exec hostnamefrom an agent session.tools.exec.host: "auto"and runexec host=node: hostname.Risk / Mitigation
autocould let a sandboxed session escape to gateway/node.bash-tools.exec.ts(line 1338:target.selectedTarget === "sandbox" && !sandboxcheck) and by the approval layer (line 1322:host === "sandbox" ? "deny" : "full"). The new tests explicitly verify that cross-host overrides between concrete targets (e.g.node→gateway) are still rejected. The elevated fix only applies toconfiguredTarget === "node", which is an explicit admin choice — no implicit escalation path is opened.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Fixes #60772