fix(dsh-runtime): emit a failed result frame when model selection derivation throws

apps/daemon/src/origin-validation.ts

@@ -225,21 +225,16 @@ export function isLocalSameOrigin(
 
   const localHostAllowed = isAllowedBrowserHost(host, ports, bindHost, ipOnlyExtraOrigins);
   if (origin == null || origin === '') {
-    if (localHostAllowed) return true;
-    // Browsers (Firefox, Chrome) omit Origin on same-origin GET subresource
-    // requests per the Fetch spec, which made hostname entries in
-    // OD_ALLOWED_ORIGINS unreachable for legitimate same-origin GETs
-    // through a reverse proxy. Sec-Fetch-Site is set by the user agent and
-    // cannot be modified by JavaScript, so a value of "same-origin"
-    // attests that the request originated from the same origin as the
-    // target — a cross-site `<img>`/`<script>` exploit would carry
-    // "cross-site" instead. Only consult the broader allow-list once that
-    // signal is present.
-    const fetchSite = headerValue(req.headers?.['sec-fetch-site']);
-    if (fetchSite === 'same-origin') {
-      return isAllowedBrowserHost(host, ports, bindHost, extraAllowedOrigins);
-    }
-    return false;
+    // Only a loopback / private-LAN host (or an explicitly configured
+    // IP-literal origin) may omit the Origin header. Sec-Fetch-Site is
+    // intentionally NOT consulted here (issue #7041): browsers set it and
+    // JavaScript cannot modify it, but any non-browser HTTP client (curl,
+    // scripts) can forge it, so treating `Sec-Fetch-Site: same-origin` as
+    // an authorization signal would let a forged header reach the full
+    // OD_ALLOWED_ORIGINS allow-list on the non-loopback path. Reverse-proxy
+    // deployments whose public hostname is listed in OD_ALLOWED_ORIGINS
+    // must send an Origin header (or use bearer auth) instead.
+    return localHostAllowed;
   }
   // Reverse-proxy deployments (e.g. Nginx in front of the daemon) terminate
   // the browser connection at the proxy and open a fresh upstream

apps/daemon/tests/origin-validation.test.ts

@@ -701,15 +701,16 @@ describe('isLocalSameOrigin: OD_ALLOWED_ORIGINS bypass for reverse-proxy deploym
   });
 });
 
-// Firefox and Chrome omit the Origin header on same-origin GET requests per
-// the Fetch spec. When the daemon runs behind a remote-access proxy whose
-// public hostname is listed in OD_ALLOWED_ORIGINS, those legitimate
-// same-origin GETs (e.g. /api/app-config) get rejected by the no-Origin
-// host check because hostname entries in OD_ALLOWED_ORIGINS are only
-// honored via the IP-literal subset in that branch. Sec-Fetch-Site is set
-// by the browser and cannot be modified by JavaScript, so a value of
-// "same-origin" is a trustworthy substitute for the missing Origin header.
-describe('isLocalSameOrigin: Sec-Fetch-Site fallback for no-Origin same-origin GETs', () => {
+// Issue #7041 — Sec-Fetch-Site must not act as an authorization signal.
+// Browsers set Sec-Fetch-Site and JavaScript cannot modify it, but any
+// non-browser HTTP client (curl, scripts) can forge it trivially. For
+// no-Origin requests, the only acceptable authorization is a loopback /
+// private-LAN host (or an explicitly configured IP-literal origin); a
+// client-supplied Sec-Fetch-Site header never widens that. Reverse-proxy
+// deployments whose public hostname is listed in OD_ALLOWED_ORIGINS must
+// send an Origin header (or use bearer auth) instead — see #2477 for the
+// earlier behavior this fix intentionally closes.
+describe('isLocalSameOrigin: Sec-Fetch-Site is not an authorization signal (issue #7041)', () => {
   const ALLOWED = 'https://nas.example.ts.net';
   const previousAllowedOrigins = process.env.OD_ALLOWED_ORIGINS;
   const env: NodeJS.ProcessEnv = {
@@ -726,14 +727,17 @@ describe('isLocalSameOrigin: Sec-Fetch-Site fallback for no-Origin same-origin G
     else process.env.OD_ALLOWED_ORIGINS = previousAllowedOrigins;
   });
 
-  it('accepts a no-Origin request whose Host matches OD_ALLOWED_ORIGINS when Sec-Fetch-Site is same-origin', () => {
+  it('rejects a no-Origin request even when Host matches OD_ALLOWED_ORIGINS and Sec-Fetch-Site is same-origin (forged header cannot authorize)', () => {
+    // Issue #7041 repro: a non-browser client can forge Sec-Fetch-Site, so
+    // it must never substitute for the Origin header / bearer auth on the
+    // non-loopback path.
     const req = {
       headers: {
         host: 'nas.example.ts.net',
         'sec-fetch-site': 'same-origin',
       },
     };
-    expect(isLocalSameOrigin(req, 7456, env)).toBe(true);
+    expect(isLocalSameOrigin(req, 7456, env)).toBe(false);
   });
 
   it('still rejects a no-Origin request whose Host matches the allow-list but Sec-Fetch-Site is cross-site', () => {

packages/dsh-runtime/src/index.ts

@@ -258,13 +258,35 @@ async function execute(
   onHandle: (handle: AgentHandle | undefined) => void,
   signal: AbortSignal,
 ): Promise<void> {
-  const defaultSelection = ctx.agentDefaultModel.currentSelection();
-  const baseSelection = request.model
-    ? { provider: request.model.provider, model: request.model.id }
-    : defaultSelection;
-  const selection = request.reasoning_effort
-    ? { ...baseSelection, reasoningEffort: ReasoningEffortId(request.reasoning_effort) }
-    : baseSelection;
+  // Model selection derivation (provider/id/resume-id → resolved selection)
+  // runs before the try below, so a host-supplied value that the harness
+  // rejects (e.g. an unknown reasoning_effort) would throw OUTSIDE the
+  // handler's error path and reject the task promise with no result frame —
+  // `serve` would then exit the process silently and the host would wait on
+  // a response that never comes. Derive inside its own guard and emit an
+  // explicit failed frame so the protocol contract (every execute gets
+  // exactly one result) holds even for malformed selections.
+  let selection: ModelSelectionRef['current'];
+  try {
+    const defaultSelection = ctx.agentDefaultModel.currentSelection();
+    const baseSelection = request.model
+      ? { provider: request.model.provider, model: request.model.id }
+      : defaultSelection;
+    selection = request.reasoning_effort
+      ? { ...baseSelection, reasoningEffort: ReasoningEffortId(request.reasoning_effort) }
+      : baseSelection;
+  } catch (error: unknown) {
+    writeFrame(output, {
+      v: 1,
+      type: 'result',
+      request_id: request.request_id,
+      status: 'failed',
+      session_id: String(SessionId(request.resume_session_id ?? `od-${randomUUID()}`)),
+      resume_rejected: false,
+      error: errorFacts(error, 'DSH_PROFILE_INVALID_MODEL_SELECTION'),
+    });
+    return;
+  }
   const sessionId = SessionId(request.resume_session_id ?? `od-${randomUUID()}`);
   let handle: AgentHandle | undefined;
   let firstSeq = Number.POSITIVE_INFINITY;
@@ -439,7 +461,24 @@ async function serve(
         (nextHandle) => { handle = nextHandle; },
         taskAbort.signal,
       );
-      void task.finally(settle);
+      // Defense in depth: `execute` reports every failure via a result frame
+      // and resolves, so a rejection here would mean a bug in that contract.
+      // Still convert it into an explicit failed frame instead of an
+      // unhandled rejection (which would make the process exit silently and
+      // leave the host waiting on a result that never arrives).
+      void task
+        .catch((error: unknown) => {
+          writeFrame(output, {
+            v: 1,
+            type: 'result',
+            request_id: requestId ?? command.request_id,
+            status: 'failed',
+            session_id: `od-${randomUUID()}`,
+            resume_rejected: false,
+            error: errorFacts(error, 'DSH_PROFILE_EXECUTION_FAILED'),
+          });
+        })
+        .finally(settle);
     });
     lines.on('close', () => {
       if (!task) settle();

packages/dsh-runtime/tests/protocol.test.ts

@@ -334,4 +334,36 @@ describe('@open-design/dsh-runtime protocol', () => {
     assert.equal(frames.at(-1)?.status, 'cancelled');
     assert.equal(frames.at(-1)?.error, undefined);
   });
+
+  test('emits a failed result frame when model selection derivation throws (no silent exit)', async () => {
+    // Model selection (provider/id → resolved selection) is derived before
+    // the run itself. If that derivation throws — e.g. the default-model
+    // service is unavailable, or a host-supplied value is rejected — the
+    // execute must still settle the protocol contract: exactly one result
+    // frame, status failed. Before the fix the derivation sat outside the
+    // error path, so a throw rejected the task promise with no frame at all
+    // and the serve loop exited the process silently, leaving the host
+    // waiting on a result that never arrived.
+    const chunks: string[] = [];
+    const ctx = {
+      agentDefaultModel: {
+        currentSelection: () => {
+          throw new Error('default model unavailable');
+        },
+      },
+    };
+    await internals.execute(ctx as never, {
+      v: 1,
+      type: 'execute',
+      request_id: 'run-bad-selection',
+      cwd: '/project',
+      prompt: 'hi',
+      mcp_servers: [],
+    }, { write: (chunk: string) => chunks.push(chunk) }, () => {}, new AbortController().signal);
+
+    const frames = chunks.map((chunk) => JSON.parse(chunk) as { type: string; status?: string; error?: { code?: string } });
+    assert.equal(frames.filter((frame) => frame.type === 'result').length, 1);
+    assert.equal(frames.at(-1)?.status, 'failed');
+    assert.equal(frames.at(-1)?.error?.code, 'DSH_PROFILE_INVALID_MODEL_SELECTION');
+  });
 });