feat(routes): enforce x-vellum-client-id ownership on host-transfer routes; add target_client_id to tool by credence-the-bot[bot] · Pull Request #29434 · vellum-ai/vellum-assistant

credence-the-bot · 2026-05-03T18:23:23Z

Summary

Add x-vellum-client-id ownership validation to GET/PUT content and POST result host-transfer routes (mirrors Phase 2 host-file/host-cu pattern)
Add target_client_id to host_file_transfer tool schema with multi-client ambiguity guard
Add route ownership tests in host-transfer-routes-targeted.test.ts
Regen openapi.yaml to include 400/403 additionalResponses on transfer routes

Part of plan: targeted-host-proxy-phase3.md (PR 2 of 3)

…outes; add target_client_id to tool

credence-the-bot · 2026-05-03T18:28:07Z

@codexbot review

credence-the-bot · 2026-05-03T18:28:09Z

@devin-ai-integration[bot] please review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 738f014ebf

ℹ️ 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".

chatgpt-codex-connector · 2026-05-03T18:28:11Z

+    const submittingClientId = (headers as Record<string, string>)["x-vellum-client-id"]?.trim() || undefined;
+    if (!submittingClientId) throw new BadRequestError("x-vellum-client-id header required for targeted transfer");
+    if (submittingClientId !== targetClientId) throw new ForbiddenError(`Client "${submittingClientId}" is not the owner of this transfer`);


Coordinate targeted transfer header enforcement with clients

These new targeted checks require x-vellum-client-id for transfer GET/PUT/result, but the current macOS transfer client does not send that header on transfer calls (clients/shared/Network/HostProxyClient.swift postTransferResult, pullTransferContent, pushTransferContent), while bash/file/cu calls do. As soon as target_client_id is used, /transfers/:transferId/content and /host-transfer-result will return 400 and the transfer cannot complete. Please ship client header propagation in lockstep or gate this enforcement until transfer callers include the header.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-05-03T18:28:11Z

+    const targetClientId =
+      typeof input.target_client_id === "string" && input.target_client_id !== ""
+        ? input.target_client_id
+        : undefined;


Reject targeted transfer when no host-file client is connected

target_client_id is parsed here, but if that client disconnects before execution, HostTransferProxy.instance.isAvailable() can be false and the code falls through to executeLocal(...), silently doing a local filesystem copy instead of a host-client transfer. For targeted requests this is incorrect behavior and can write to daemon/container paths unexpectedly; this path should return an explicit error when a target is specified but no capable client is connected.

Useful? React with 👍 / 👎.

devin-ai-integration

Devin Review found 4 potential issues.

View 2 additional findings in Devin Review.

devin-ai-integration · 2026-05-03T18:29:11Z

🚩 Pre-existing: resolveTransferContentGetHeaders calls getTransferContent after handler already consumed the entry

The HTTP adapter (http-adapter.ts:107-124) calls the handler first, then resolves responseHeaders. For the GET transfer content route, this means handleTransferContentGet calls getTransferContent(transferId) which deletes the transfer entry (host-transfer-proxy.ts:454), then resolveTransferContentGetHeaders calls getTransferContent again — but the entry is already deleted, so it returns null and the response falls back to bare Content-Type: application/octet-stream without Content-Length or X-Transfer-SHA256 headers. This is a pre-existing issue not introduced by this PR, but the new getTargetClientIdForTransfer call added in this PR runs non-destructively before the consuming call, so it doesn't worsen the situation.

(Refers to lines 65-84)

Was this helpful? React with 👍 or 👎 to provide feedback.

devin-ai-integration · 2026-05-03T18:29:13Z

+      expect(
+        handleTransferContentPut({
+          pathParams: { transferId: TEST_TRANSFER_ID },
+          headers: { "x-transfer-sha256": "abc" },
+          rawBody: new Uint8Array(Buffer.from("data")),
+        }),
+      ).rejects.toBeInstanceOf(BadRequestError);


🟡 Missing await on .rejects assertions causes PUT handler error-type tests to always pass

The handleTransferContentPut handler is async, so thrown errors become rejected promises. At lines 243-249 and 272-278, expect(promise).rejects.toBeInstanceOf(...) is called without await. In bun:test (same as Jest), .rejects returns a Promise that must be awaited — without it, the assertion result is silently discarded and the test passes regardless of whether the handler throws the expected error type. The rest of the codebase consistently uses await expect(...).rejects.toThrow(...) (e.g., assistant/src/__tests__/host-cu-proxy.test.ts:718, assistant/src/__tests__/host-proxy-base.test.ts:146). The companion side-effect tests (lines 252-264, 281-292) properly await the handler and verify guards work, so the security guards are partially covered, but the specific error-type verification is ineffective.

Suggested change

expect(

handleTransferContentPut({

pathParams: { transferId: TEST_TRANSFER_ID },

headers: { "x-transfer-sha256": "abc" },

rawBody: new Uint8Array(Buffer.from("data")),

}),

).rejects.toBeInstanceOf(BadRequestError);

await expect(

handleTransferContentPut({

pathParams: { transferId: TEST_TRANSFER_ID },

headers: { "x-transfer-sha256": "abc" },

rawBody: new Uint8Array(Buffer.from("data")),

}),

).rejects.toBeInstanceOf(BadRequestError);

Was this helpful? React with 👍 or 👎 to provide feedback.

devin-ai-integration · 2026-05-03T18:29:14Z

+      expect(
+        handleTransferContentPut({
+          pathParams: { transferId: TEST_TRANSFER_ID },
+          headers: { "x-vellum-client-id": "client-B", "x-transfer-sha256": "abc" },
+          rawBody: new Uint8Array(Buffer.from("data")),
+        }),
+      ).rejects.toBeInstanceOf(ForbiddenError);


🟡 Missing await on .rejects assertion for ForbiddenError test case

Same issue as the BadRequestError test: expect(promise).rejects.toBeInstanceOf(ForbiddenError) at line 272-278 is not awaited, so the assertion is never actually verified. The test will pass even if the handler throws a different error type or doesn't throw at all.

Suggested change

expect(

handleTransferContentPut({

pathParams: { transferId: TEST_TRANSFER_ID },

headers: { "x-vellum-client-id": "client-B", "x-transfer-sha256": "abc" },

rawBody: new Uint8Array(Buffer.from("data")),

}),

).rejects.toBeInstanceOf(ForbiddenError);

await expect(

handleTransferContentPut({

pathParams: { transferId: TEST_TRANSFER_ID },

headers: { "x-vellum-client-id": "client-B", "x-transfer-sha256": "abc" },

rawBody: new Uint8Array(Buffer.from("data")),

}),

).rejects.toBeInstanceOf(ForbiddenError);

Was this helpful? React with 👍 or 👎 to provide feedback.

devin-ai-integration · 2026-05-03T18:29:15Z

+    if (
+      targetClientId == null &&
+      context.transportInterface != null &&
+      !supportsHostProxy(context.transportInterface) &&
+      assistantEventHub.listClientsByCapability("host_file").length > 1
+    ) {
+      return { content: `Error: multiple clients support host_file. Specify which client to use with \`target_client_id\`. Run \`assistant clients list --capability host_file\` to see client IDs and labels.`, isError: true };
+    }


🚩 Ambiguity guard skipped for macOS-originated transfers with multiple host_file clients

The multi-client ambiguity guard at assistant/src/tools/host-filesystem/transfer.ts:100-107 checks !supportsHostProxy(context.transportInterface) using the no-arg overload, which returns true only for "macos". This means macOS-originated transfers skip the disambiguation error when multiple host_file clients are connected. In that scenario, the proxy's auto-resolve logic (host-transfer-proxy.ts:134-136) only sets resolvedTargetClientId when exactly one capable client exists — with multiple clients, the broadcast goes untargeted. This is likely intentional (macOS clients are themselves host-proxy capable, and typically only one macOS client would be connected per assistant), but worth confirming the design handles the two-macOS-clients edge case as expected.

Was this helpful? React with 👍 or 👎 to provide feedback.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 738f014ebf

ℹ️ 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".

chatgpt-codex-connector · 2026-05-03T18:31:33Z

+  const targetClientId = match.proxy.getTargetClientIdForTransfer(transferId);
+  if (targetClientId != null) {
+    const submittingClientId = (headers as Record<string, string>)["x-vellum-client-id"]?.trim() || undefined;
+    if (!submittingClientId) throw new BadRequestError("x-vellum-client-id header required for targeted transfer");


Keep targeted transfer routes compatible with current clients

This new header requirement breaks targeted host transfers end-to-end because the current client transfer paths do not send x-vellum-client-id: postTransferResult, pullTransferContent, and pushTransferContent call GatewayHTTPClient without that header (clients/shared/Network/HostProxyClient.swift lines 131-174), and GatewayHTTPClient.get has no extra-header parameter to add it. As soon as target_client_id is used (new tool input), the GET/PUT/result routes now return 400 for the legitimate target client, so targeted transfers fail rather than being secured.

Useful? React with 👍 / 👎.

… on targetClientId

credence-the-bot · 2026-05-03T18:39:46Z

@codexbot review

credence-the-bot · 2026-05-03T18:39:46Z

@devin-ai-integration[bot] please review

chatgpt-codex-connector · 2026-05-03T18:43:36Z

Codex Review: Didn't find any major issues. Keep it up!

ℹ️ 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".

…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>

feat(routes): enforce x-vellum-client-id ownership on host-transfer r…

738f014

…outes; add target_client_id to tool

credence-the-bot Bot requested a review from siddseethepalli as a code owner May 3, 2026 18:23

chatgpt-codex-connector Bot reviewed May 3, 2026

View reviewed changes

devin-ai-integration Bot reviewed May 3, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed May 3, 2026

View reviewed changes

fix(tests): await .rejects assertions; guard executeLocal fallthrough…

4295c8a

… on targetClientId

credence-the-bot Bot merged commit 649621f into Credence/targeted-host-proxy-phase3 May 3, 2026
13 checks passed

credence-the-bot Bot deleted the run-plan/host-transfer-xc/pr-2 branch May 3, 2026 18:42

credence-the-bot Bot mentioned this pull request May 3, 2026

feat(host-proxy): Phase 3 — targetClientId routing for host_file_transfer #29440

Merged

4 tasks

credence-the-bot Bot mentioned this pull request May 3, 2026

fix(host-transfer): emit Content-Length and X-Transfer-SHA256 on GET content #29449

Merged

Conversation

credence-the-bot Bot commented May 3, 2026 • edited by devin-ai-integration Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

credence-the-bot Bot commented May 3, 2026

Uh oh!

credence-the-bot Bot commented May 3, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot May 3, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot May 3, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot May 3, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot May 3, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot May 3, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot May 3, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot May 3, 2026

Choose a reason for hiding this comment

Uh oh!

credence-the-bot Bot commented May 3, 2026

Uh oh!

credence-the-bot Bot commented May 3, 2026

Uh oh!

Uh oh!

chatgpt-codex-connector Bot commented May 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

0 participants

credence-the-bot Bot commented May 3, 2026 •

edited by devin-ai-integration Bot

Loading