fix: accept query token on dashboard settle POST (rebased, supersedes #27)

CHANGELOG.md

@@ -108,6 +108,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   string surviving a sanitiser is an invitation to re-introduce the traversal
   later; dot-runs are now collapsed before the character pass.
 
+- **Dashboard settle accepts `?token=`.** `POST /api/approvals/:id` used a
+  placeholder URL with `allowQuery: false`, so scripted clients that only had
+  the printed dashboard URL got a 401 that looked like a wrong secret. The
+  settle route now receives the real request URL and accepts the query token
+  the same way the GET routes do; the header still works, and `sameOrigin`
+  still blocks cross-origin browsers. Closes KNOWN-ISSUES §5.
+
 
 ### Changed
 

docs/FEATURE-settle-query-token.md

@@ -0,0 +1,20 @@
+# Feature: accept query token on dashboard settle POST
+
+## Why
+KNOWN-ISSUES §5: `POST /api/approvals/:id` calls `authorized(req, new URL('/', 'http://x'), false)`,
+so the real request URL (and `?token=`) is discarded and only `x-dashboard-token` works.
+Read routes accept query tokens. Scripted clients get a 401 that looks like a wrong token.
+
+## Acceptance
+1. `POST /api/approvals/:id?token=<valid>` with body `{"outcome":"allowed-once"}` and **no** header → **200** when ask pending (or 409 if already settled), never 401 for a valid query token.
+2. Header token still works (regression).
+3. Missing/invalid token still **401**.
+4. `sameOrigin` still rejects cross-origin browser POSTs (403).
+5. Unit test(s) in `test/dashboard.test.ts` cover query-token settle.
+6. Update `docs/KNOWN-ISSUES.md` §5 to Fixed.
+7. Comment on `authorized` / `approve` matches behaviour.
+
+## Non-goals
+- Do not remove header auth.
+- Do not weaken sameOrigin.
+- Do not change watcher/SSE semantics (§6).

docs/KNOWN-ISSUES.md

@@ -70,6 +70,21 @@ the remote mount.
 **Fix:** the tag is owned by an `ctx.effect` with a `remove()` teardown, the
 same ownership the host's own theme sheets use. PR #23.
 
+### 5. `POST /api/approvals/:id` ignored the query token
+
+**Observed:** a scripted approval using `?token=…` got
+`401 missing or invalid dashboard token` while every read route accepted the
+same query token.
+
+**Why:** the settle route called `authorized(req, new URL('/', 'http://x'), false)`
+— a placeholder URL with `allowQuery: false` — so the real `?token=` was discarded
+and only `x-dashboard-token` worked.
+
+**Fix:** `approve` now receives the request URL and passes `allowQuery: true`.
+Header still wins when present; `sameOrigin` still blocks cross-origin browsers.
+Covered by `test/dashboard.test.ts` ("POST settle accepts ?token= without the
+header").
+
 ### 7. The dashboard page was only a watcher half the time
 
 **Observed:** an approval raised while the Feature Loop page was open and
@@ -148,25 +163,6 @@ store key for a `_@deepseek-ai+…` suffix.
 
 ## Open — product warts
 
-### 5. `POST /api/approvals/:id` ignores the query token
-
-**Observed:** a scripted approval using `?token=…` got
-`401 missing or invalid dashboard token` while every read route accepted the
-same query token.
-
-**Why:** the settle route calls `authorized(req, url, false)` — `allowQuery` is
-`false` — so it takes the token from the `x-dashboard-token` header only. The
-read routes pass `true`.
-
-**Why it matters:** the two halves of one tiny API disagree about how to
-authenticate, and the failure mode is a 401 that looks like a wrong token rather
-than a wrong *mechanism*. Any non-browser client has to know the difference.
-
-**Suggested fix:** accept the query token here too (the route is token-gated
-already, and `sameOrigin` still guards the state change), or document the header
-requirement in the runbook. Not changed here — it alters the security posture of
-a state-changing route and deserves its own review.
-
 ### 6. Only an SSE client counts as a "watcher"
 
 **Observed:** polling `GET /api/state` every second never let the registry claim

docs/evidence/settle-query-token-live/result.txt

@@ -0,0 +1,3 @@
+settled=1
+exists=True
+got=query-token-ok

docs/evidence/settle-query-token-live/settle-query-response.json

@@ -0,0 +1 @@
+{"ok":true,"outcome":"allowed-once"}

src/dashboard.ts

@@ -849,7 +849,13 @@ export function startDashboard(
     return timingSafeEqual(a, b)
   }
 
-  /** Token from the header everywhere; query also accepted on GETs only. */
+  /**
+   * Token from the header, or from `?token=` when `allowQuery` is true.
+   * Settle POSTs pass `allowQuery: true` too: the route is already token-gated
+   * and `sameOrigin` still blocks cross-origin browsers, so rejecting a valid
+   * query token only taught scripted clients a 401 that looked like a wrong
+   * secret rather than a wrong mechanism (KNOWN-ISSUES §5).
+   */
   const authorized = (req: IncomingMessage, url: URL, allowQuery: boolean): boolean => {
     const header = req.headers['x-dashboard-token']
     if (tokenMatches(Array.isArray(header) ? header[0] : header)) return true
@@ -944,8 +950,15 @@ export function startDashboard(
     entry.settle('allowed-once', `allowed for this run: ${entry.toolName}${feedback === '' ? '' : ` — ${feedback}`}`)
   }
 
-  const approve = async (req: IncomingMessage, res: ServerResponse, id: string): Promise<void> => {
-    if (!authorized(req, new URL('/', 'http://x'), false)) return deny(res)
+  const approve = async (
+    req: IncomingMessage,
+    res: ServerResponse,
+    id: string,
+    url: URL,
+  ): Promise<void> => {
+    // Pass the real request URL so `?token=` works the same as on GET routes.
+    // Header still wins when present; query is the scripted-client path.
+    if (!authorized(req, url, true)) return deny(res)
     if (!sameOrigin(req)) return sendJson(res, 403, { error: 'cross-origin approval denied' })
     let body: unknown
     try {
@@ -1045,7 +1058,12 @@ export function startDashboard(
       return serveAsset(res, url.pathname)
     }
     if (method === 'POST' && url.pathname.startsWith('/api/approvals/')) {
-      return approve(req, res, decodeURIComponent(url.pathname.slice('/api/approvals/'.length)))
+      return approve(
+        req,
+        res,
+        decodeURIComponent(url.pathname.slice('/api/approvals/'.length)),
+        url,
+      )
     }
     // Deliberately NO `POST /api/apply` — and no GET for it either. Applying
     // a recommendation means a human copying its config snippet into their

test/dashboard.test.ts

@@ -75,16 +75,31 @@ async function post(
   dash: DashboardHandle,
   id: string,
   outcome: string,
-  opts: { token?: string, origin?: string, body?: string } = {},
+  opts: {
+    token?: string
+    origin?: string
+    body?: string
+    /**
+     * Put the token in `?token=` only. When set, the header is omitted unless
+     * `token` is also set — that combination is how we prove query auth alone.
+     */
+    queryToken?: string
+  } = {},
 ): Promise<Response> {
   const origin = opts.origin ?? new URL(dash.url).origin
-  return fetch(`${dash.url}api/approvals/${encodeURIComponent(id)}`, {
+  const q = opts.queryToken === undefined
+    ? ''
+    : `?token=${encodeURIComponent(opts.queryToken)}`
+  const headers: Record<string, string> = {
+    'content-type': 'application/json',
+    origin,
+  }
+  // Header when the caller asked for one. Query-only settles pass queryToken
+  // and leave token undefined so the request has no x-dashboard-token.
+  if (opts.token !== undefined) headers['x-dashboard-token'] = opts.token
+  return fetch(`${dash.url}api/approvals/${encodeURIComponent(id)}${q}`, {
     method: 'POST',
-    headers: {
-      'content-type': 'application/json',
-      ...opts.token === undefined ? {} : { 'x-dashboard-token': opts.token },
-      origin,
-    },
+    headers,
     body: opts.body ?? JSON.stringify({ outcome }),
   })
 }
@@ -388,6 +403,33 @@ test('POST validation: no token 401, cross-origin 403, bad outcome 400, bad body
   assert.equal(await pending, 'allowed-once')
 })
 
+test('POST settle accepts ?token= without the header (KNOWN-ISSUES §5)', async (t) => {
+  const { dash } = await started(t)
+  const close = await connectSse(dash)
+  t.after(close)
+
+  const pending = dash.answer(QUESTION, delegatingNext().next)
+  const { id } = (await getState(dash)).pending[0] as { id: string }
+
+  // Scripted client path: query only, no x-dashboard-token.
+  const res = await post(dash, id, 'allowed-once', { queryToken: dash.token })
+  assert.equal(res.status, 200, 'a valid query token must settle, not 401')
+  assert.deepEqual(await res.json(), { ok: true, outcome: 'allowed-once' })
+  assert.equal(await pending, 'allowed-once')
+
+  // Invalid query token still 401s (and must not look like a missing mechanism).
+  const pending2 = dash.answer(QUESTION, delegatingNext().next)
+  const { id: id2 } = (await getState(dash)).pending[0] as { id: string }
+  assert.equal(
+    (await post(dash, id2, 'allowed-once', { queryToken: 'not-the-token' })).status,
+    401,
+  )
+  // Header still works when both are present.
+  const ok = await post(dash, id2, 'rejected', { token: dash.token, queryToken: 'ignored-bad-query' })
+  assert.equal(ok.status, 200, 'header wins over a bad query token')
+  assert.equal(await pending2, 'rejected')
+})
+
 test('an abort (the ask was withdrawn) cancels the pending approval', async (t) => {
   const { dash } = await started(t)
   const close = await connectSse(dash)