feat(host-proxy): add targetClientId support to HostTransferProxy and message envelopes#29431
Conversation
… message envelopes
|
@codexbot review |
|
@devin-ai-integration[bot] please review |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| /** | ||
| * Look up the targetClientId for a given transferId without consuming the entry. | ||
| * Routes call this to verify ownership without affecting the transfer state. | ||
| * Returns null when untargeted (no validation needed). | ||
| */ | ||
| getTargetClientIdForTransfer(transferId: string): string | null { | ||
| return this.transfers.get(transferId)?.targetClientId ?? null; | ||
| } |
There was a problem hiding this comment.
🚩 Host-transfer routes lack targetClientId validation on inbound endpoints
The host-bash (host-bash-routes.ts:53-64), host-file (host-file-routes.ts:46-56), and host-cu (host-cu-routes.ts:67-77) result routes all validate that the submitting client (via X-Vellum-Client-Id header) matches the targetClientId from the pending interaction. The host-transfer routes (host-transfer-routes.ts:117-151) do not perform this check on any of the three endpoints (GET content, PUT content, POST result). This means a non-targeted client could submit results or download/upload content for a transfer intended for a specific client. The getTargetClientIdForTransfer method was added at host-transfer-proxy.ts:576 with doc comment "Routes call this to verify ownership" but no route actually calls it. This is likely planned for a follow-up but creates an inconsistency with the other host proxy routes.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
credence-the-bot
Bot
merged commit 719e453
into
Credence/targeted-host-proxy-phase3
…sfer (#29440) * feat(host-proxy): add targetClientId support to HostTransferProxy and message envelopes (#29431) * feat(host-proxy): add targetClientId support to HostTransferProxy and message envelopes * fix(tests): add listClientsByCapability and getClientById to host-transfer-proxy mock --------- Co-authored-by: credence-the-bot <credence@vellum.ai> * feat(routes): enforce x-vellum-client-id ownership on host-transfer routes; add target_client_id to tool (#29434) * feat(routes): enforce x-vellum-client-id ownership on host-transfer routes; add target_client_id to tool * fix(tests): await .rejects assertions; guard executeLocal fallthrough on targetClientId --------- Co-authored-by: credence-the-bot <credence@vellum.ai> * feat(macos): targetClientId acceptance guard and x-vellum-client-id headers for host_transfer (#29435) * feat(macos): targetClientId acceptance guard and x-vellum-client-id headers for host_transfer * fix(swift): use setValue (not addValue) for extraHeaders in GatewayHTTPClient.get --------- Co-authored-by: credence-the-bot <credence@vellum.ai> --------- Co-authored-by: credence-the-bot[bot] <277301654+credence-the-bot[bot]@users.noreply.github.com> Co-authored-by: credence-the-bot <credence@vellum.ai>
Summary
Part of plan: targeted-host-proxy-phase3.md (PR 1 of 3)