fix(messages): address crew review round 2 findings for #8/#9

- Move stop_sequences comment to correct location (output object, not image branch)
- Add runtime narrowing guard in handleAnthropicViaResponses to replace unsafe cast
- Propagate failed upstream status as 500 instead of 200 OK
- Apply media_type allowlist to non-stream-translation.ts (was only in Responses path)
- Apply prototype pollution guard to non-stream-translation.ts tool arg parsing
- Hoist DANGEROUS_KEYS to module-level constant in responses-to-anthropic.ts
- Add DANGEROUS_TOOL_KEYS module-level constant in non-stream-translation.ts
- Validate output_config.effort against allowlist before forwarding (new in R1 fix)

Test additions:
- Image with disallowed media_type (svg+xml) → dropped, text block preserved
- function_call with array JSON arguments → wrapped in _raw (non-object guard)
- function_call with constructor/prototype keys → stripped
- Fix vacuous-pass risk: assert block.type before narrowing in tool_use tests

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Author: HXYerror

src/routes/messages/anthropic-to-responses.ts

@@ -38,6 +38,12 @@ const ALLOWED_IMAGE_MEDIA_TYPES = new Set([
   "image/webp",
 ])
 
+// ---------------------------------------------------------------------------
+// Allowlisted reasoning effort values
+// ---------------------------------------------------------------------------
+
+const VALID_EFFORT_VALUES = new Set<string>(["low", "medium", "high"])
+
 // ---------------------------------------------------------------------------
 // Budget → effort tier mapping
 // ---------------------------------------------------------------------------
@@ -152,9 +158,6 @@ function translateUserMessage(
         // image block
         const imagePart = translateImageBlock(block)
         if (imagePart) contentParts.push(imagePart)
-        // Note: stop_sequences are also not forwarded — Responses API has no
-        // equivalent field; callers relying on stop sequences will not get that
-        // behaviour when using Responses-only models.
       }
     }
 
@@ -296,8 +299,13 @@ function translateReasoning(
   if (!thinking) return undefined
 
   if (thinking.type === "adaptive") {
-    // Use explicit effort from output_config when provided; default to medium
-    return { effort: outputConfig?.effort ?? "medium" }
+    const rawEffort = outputConfig?.effort
+    // Validate effort value against allowlist to prevent forwarding arbitrary strings
+    const effort =
+      rawEffort !== undefined && VALID_EFFORT_VALUES.has(rawEffort) ?
+        rawEffort
+      : "medium"
+    return { effort: effort }
   }
 
   // type === "enabled"
@@ -337,5 +345,8 @@ export function translateAnthropicToResponses(
     reasoning: translateReasoning(payload.thinking, payload.output_config),
     stream: payload.stream,
     user: payload.metadata?.user_id,
+    // NOTE: stop_sequences is not forwarded — the Responses API has no
+    // equivalent field.  Callers relying on stop sequences will not get that
+    // behaviour when using Responses-only models.
   }
 }

src/routes/messages/handler.ts

@@ -3,8 +3,6 @@ import type { Context } from "hono"
 import consola from "consola"
 import { streamSSE } from "hono/streaming"
 
-import type { ResponsesResponse } from "~/routes/responses/types"
-
 import { awaitApproval } from "~/lib/approval"
 import { getModelMode } from "~/lib/model-routing"
 import { checkRateLimit } from "~/lib/rate-limit"
@@ -136,7 +134,41 @@ async function handleAnthropicViaResponses(
     ...responsesPayload,
     stream: false,
   })
-  const sanitised = sanitiseResponsesOutput(rawResponse as ResponsesResponse)
+
+  // Runtime guard: createResponses returns ResponsesResponse for stream:false,
+  // but TypeScript types the return as a union — narrow explicitly.
+  if (!("output" in rawResponse)) {
+    consola.error(
+      "Unexpected non-response shape from createResponses:",
+      rawResponse,
+    )
+    return c.json(
+      {
+        error: {
+          message: "Upstream returned unexpected response shape",
+          type: "api_error",
+          code: "invalid_upstream_response",
+        },
+      },
+      502,
+    )
+  }
+
+  const typedResponse = rawResponse
+
+  // Surface upstream terminal failures as errors instead of 200 OK
+  if (typedResponse.status === "failed") {
+    const errMsg =
+      (typedResponse as unknown as { error?: { message?: string } }).error
+        ?.message ?? "Upstream model call failed"
+    consola.error("Responses API returned failed status:", errMsg)
+    return c.json(
+      { error: { message: errMsg, type: "api_error", code: "model_error" } },
+      500,
+    )
+  }
+
+  const sanitised = sanitiseResponsesOutput(typedResponse)
   const anthropicResponse = translateResponsesToAnthropic(sanitised)
 
   consola.debug(

src/routes/messages/non-stream-translation.ts

@@ -1,5 +1,16 @@
 import consola from "consola"
 
+// Allowlisted image media types for data URI construction (injection guard)
+const ALLOWED_IMAGE_MEDIA_TYPES = new Set([
+  "image/jpeg",
+  "image/png",
+  "image/gif",
+  "image/webp",
+])
+
+// Keys that trigger prototype pollution — stripped from upstream tool arguments
+const DANGEROUS_TOOL_KEYS = new Set(["__proto__", "constructor", "prototype"])
+
 import {
   type ChatCompletionResponse,
   type ChatCompletionsPayload,
@@ -216,12 +227,19 @@ function mapContent(
       }
       case "image": {
         if (block.source.type === "base64") {
-          contentParts.push({
-            type: "image_url",
-            image_url: {
-              url: `data:${block.source.media_type};base64,${block.source.data}`,
-            },
-          })
+          if (ALLOWED_IMAGE_MEDIA_TYPES.has(block.source.media_type)) {
+            contentParts.push({
+              type: "image_url",
+              image_url: {
+                url: `data:${block.source.media_type};base64,${block.source.data}`,
+              },
+            })
+          } else {
+            consola.warn(
+              "Skipping image with unsupported media_type in translation path:",
+              block.source.media_type,
+            )
+          }
         } else {
           // URL images are rejected by Copilot upstream — skip silently
           // (type kept for fidelity when round-tripping through native path)
@@ -370,10 +388,28 @@ function getAnthropicToolUseBlocks(
   if (!toolCalls) {
     return []
   }
-  return toolCalls.map((toolCall) => ({
-    type: "tool_use",
-    id: toolCall.id,
-    name: toolCall.function.name,
-    input: JSON.parse(toolCall.function.arguments) as Record<string, unknown>,
-  }))
+  return toolCalls.map((toolCall) => {
+    let parsedInput: Record<string, unknown>
+    try {
+      const raw: unknown = JSON.parse(toolCall.function.arguments)
+      // Non-object JSON (array, number, null, etc.) → wrap in _raw
+      parsedInput =
+        typeof raw !== "object" || raw === null || Array.isArray(raw) ?
+          { _raw: toolCall.function.arguments }
+          // Strip prototype-pollution keys before forwarding
+        : Object.fromEntries(
+            Object.entries(raw as Record<string, unknown>).filter(
+              ([k]) => !DANGEROUS_TOOL_KEYS.has(k),
+            ),
+          )
+    } catch {
+      parsedInput = { _raw: toolCall.function.arguments }
+    }
+    return {
+      type: "tool_use",
+      id: toolCall.id,
+      name: toolCall.function.name,
+      input: parsedInput,
+    }
+  })
 }

src/routes/messages/responses-to-anthropic.ts

@@ -4,6 +4,9 @@
  * Anthropic clients understand.
  */
 
+// Keys that trigger prototype pollution — stripped from upstream tool arguments
+const DANGEROUS_KEYS = new Set(["__proto__", "constructor", "prototype"])
+
 import type {
   ResponsesOutputFunctionCall,
   ResponsesOutputMessage,
@@ -89,18 +92,17 @@ function translateFunctionCallItem(
   let parsedInput: Record<string, unknown>
   try {
     const raw: unknown = JSON.parse(item.arguments)
-    if (typeof raw !== "object" || raw === null || Array.isArray(raw)) {
-      parsedInput = { _raw: item.arguments }
-    } else {
-      // Strip prototype-pollution keys before forwarding.
-      // Use Object.entries to avoid inherited properties and __proto__ setter tricks.
-      const DANGEROUS_KEYS = new Set(["__proto__", "constructor", "prototype"])
-      parsedInput = Object.fromEntries(
-        Object.entries(raw as Record<string, unknown>).filter(
-          ([k]) => !DANGEROUS_KEYS.has(k),
-        ),
-      )
-    }
+    // Non-object JSON (array, number, null, etc.) → wrap in _raw
+    parsedInput =
+      typeof raw !== "object" || raw === null || Array.isArray(raw) ?
+        { _raw: item.arguments }
+        // Strip prototype-pollution keys before forwarding.
+        // Use Object.entries to avoid inherited properties and __proto__ setter tricks.
+      : Object.fromEntries(
+          Object.entries(raw as Record<string, unknown>).filter(
+            ([k]) => !DANGEROUS_KEYS.has(k),
+          ),
+        )
   } catch {
     // If arguments are not valid JSON, wrap as a string
     parsedInput = { _raw: item.arguments }

tests/anthropic-to-responses.test.ts

@@ -94,6 +94,39 @@ describe("translateAnthropicToResponses — basic messages", () => {
     expect(parts[0].image_url).toBe("data:image/png;base64,abc123")
   })
 
+  test("image block with disallowed media_type → dropped (injection guard)", () => {
+    const result = translateAnthropicToResponses(
+      makeBasePayload({
+        messages: [
+          {
+            role: "user",
+            content: [
+              {
+                type: "text",
+                text: "Look at this",
+              },
+              {
+                type: "image",
+                source: {
+                  type: "base64",
+                  // svg+xml is not in the allowlist
+                  media_type: "image/svg+xml" as "image/png",
+                  data: "evil-data",
+                },
+              },
+            ],
+          },
+        ],
+      }),
+    )
+    const items = result.input as Array<unknown>
+    // Only the text block should survive; the disallowed image is dropped
+    const msg = items[0] as ResponsesInputMessage
+    const parts = msg.content as Array<{ type: string }>
+    expect(parts.every((p) => p.type !== "input_image")).toBe(true)
+    expect(parts.some((p) => p.type === "input_text")).toBe(true)
+  })
+
   test("user message with tool_result → function_call_output item", () => {
     const result = translateAnthropicToResponses(
       makeBasePayload({

tests/responses-to-anthropic.test.ts

@@ -219,12 +219,34 @@ describe("translateResponsesToAnthropic — function_call items", () => {
       ],
     })
     const result = translateResponsesToAnthropic(response)
+    expect(result.content[0].type).toBe("tool_use")
     const block = result.content[0]
     if (block.type === "tool_use") {
       expect(block.input).toEqual({ _raw: "not-json" })
     }
   })
 
+  test("function_call with array JSON arguments → wrapped in _raw (non-object guard)", () => {
+    const response = makeResponse({
+      output: [
+        {
+          type: "function_call",
+          id: "fc_arr",
+          call_id: "call_arr",
+          name: "array_tool",
+          arguments: JSON.stringify([1, 2, 3]),
+          status: "completed",
+        },
+      ],
+    })
+    const result = translateResponsesToAnthropic(response)
+    expect(result.content[0].type).toBe("tool_use")
+    const block = result.content[0]
+    if (block.type === "tool_use") {
+      expect(block.input).toEqual({ _raw: "[1,2,3]" })
+    }
+  })
+
   test("function_call with __proto__ in arguments → stripped (prototype pollution guard)", () => {
     const response = makeResponse({
       output: [
@@ -242,12 +264,38 @@ describe("translateResponsesToAnthropic — function_call items", () => {
       ],
     })
     const result = translateResponsesToAnthropic(response)
+    expect(result.content[0].type).toBe("tool_use")
     const block = result.content[0]
     if (block.type === "tool_use") {
       expect(Object.hasOwn(block.input, "__proto__")).toBe(false)
       expect(block.input.city).toBe("London")
     }
   })
+
+  test("function_call with constructor/prototype keys → stripped", () => {
+    const args =
+      '{"constructor":{"name":"hacked"},"prototype":{"x":1},"value":42}'
+    const response = makeResponse({
+      output: [
+        {
+          type: "function_call",
+          id: "fc_4",
+          call_id: "call_ctor",
+          name: "ctor_tool",
+          arguments: args,
+          status: "completed",
+        },
+      ],
+    })
+    const result = translateResponsesToAnthropic(response)
+    expect(result.content[0].type).toBe("tool_use")
+    const block = result.content[0]
+    if (block.type === "tool_use") {
+      expect(Object.hasOwn(block.input, "constructor")).toBe(false)
+      expect(Object.hasOwn(block.input, "prototype")).toBe(false)
+      expect(block.input.value).toBe(42)
+    }
+  })
 })
 
 // ---------------------------------------------------------------------------