feat(host-bash): add target_client_id parameter to tool schema and executor#29314
Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| // Proxy to connected client for execution on the user's machine | ||
| // when a capable client is available (managed/cloud-hosted mode). | ||
| if (HostBashProxy.instance.isAvailable()) { |
There was a problem hiding this comment.
🔴 Explicit target_client_id silently falls through to local execution when no proxy client is connected
When a user specifies target_client_id but no host_bash-capable proxy client is connected, the command silently runs on the local/daemon machine instead of returning an error. Both guard checks at assistant/src/tools/host-terminal/host-shell.ts:208-218 and assistant/src/tools/host-terminal/host-shell.ts:221-232 require targetClientId == null to fire, so they're skipped when a target is provided. Then the HostBashProxy.instance.isAvailable() check at line 236 returns false (no clients), and execution falls through to the local spawn("bash", ...) path at line 369, where targetClientId is completely ignored. This means a command explicitly intended for a specific remote client runs unsandboxed on the daemon host — potentially the wrong machine with different permissions and data.
Prompt for agents
In assistant/src/tools/host-terminal/host-shell.ts, after the two existing guards (lines 208-232) and before the HostBashProxy.instance.isAvailable() check at line 236, add a third guard that catches the case where targetClientId is explicitly set but the proxy is not available (or the proxy is available but the specific target client isn't connected). When targetClientId is non-null and the proxy is unavailable, the function should return an error result like: 'Error: target client "<id>" is not connected or does not support host_bash.' rather than falling through to local execution. This ensures commands with an explicit target_client_id never silently run on the wrong machine. The HostBashProxy.request() method at host-bash-proxy.ts:87-94 already validates the target when the proxy IS available, so the new guard only needs to cover the case where isAvailable() returns false.
Was this helpful? React with 👍 or 👎 to provide feedback.
![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: f759370344
ℹ️ 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".
| targetClientId == null && | ||
| transportInterface != null && | ||
| !supportsHostProxy(transportInterface) && | ||
| !HostBashProxy.instance.isAvailable() |
There was a problem hiding this comment.
The new non-desktop guard only fires when target_client_id is absent, so a request from a non-host-proxy interface that does include target_client_id skips this check; if no host-bash client is connected, HostBashProxy.instance.isAvailable() is false and execution falls through to the local spawn path instead of returning the intended “no client connected” error. In practice this means a caller can ask for a specific remote client and still run the command on the daemon host/container, which is a wrong-target execution regression introduced by exposing target_client_id in the schema.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🚩 Test coverage does not exercise targetClientId with unavailable proxy
The test file at assistant/src/__tests__/host-shell-tool.test.ts covers proxy delegation when proxy is available (line 743+), input validation before proxying (lines 770+, 789+), and local fallback when proxy is unavailable (line 810+). However, there is no test for the scenario where target_client_id is specified but the proxy is not available — which is the gap that enables the bug. Adding such a test would catch the silent local execution.
Was this helpful? React with 👍 or 👎 to provide feedback.
credence-the-bot
Bot
merged commit d07adab
into
Credence/targeted-host-proxy
Summary
Part of plan: targeted-host-proxy.md (PR 5 of 8)