diff --git a/CHANGELOG.md b/CHANGELOG.md index 704526b..dde996b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,22 @@ All notable changes to this project will be documented in this file. +## [0.11.1] - 2026-09-30 + +Security fix from the `operator-panel-2026-09-p2-queue-read-api` audit (F-01, Medium; +CodeRabbit CR-03 on #13). + +### Security + +- **Redirects are never followed on token-bearing requests.** `fetch` follows redirects by + default, and Undici keeps custom headers across the hop, so a redirect from the configured + `TASK_QUEUE_API` would have delivered `X-Task-Queue-Token` (read + operator-write) to + whatever origin the `Location` named. `insecureApiBase()` only vets the configured base. + Every request now goes out with `redirect: 'manual'`, and any 3xx or opaque-redirect + response is refused as a 502 without reading its body. task-queue-mcp never redirects, + so there is no behaviour change on forge. A test with two real loopback servers confirms + the redirect target is never contacted, and fails against v0.11.0. + ## [0.11.0] - 2026-09-29 Own client token, and every read through task-queue-mcp's API. Build diff --git a/manifest.json b/manifest.json index 36db53e..783ca6f 100644 --- a/manifest.json +++ b/manifest.json @@ -1,7 +1,7 @@ { "name": "task-queue", "displayName": "Task Queue", - "version": "0.11.0", + "version": "0.11.1", "description": "Task queue dashboard \u2014 view, filter, and launch agent tasks.", "author": "TadMSTR", "icon": "icon.svg", diff --git a/package-lock.json b/package-lock.json index af830c5..3450310 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "cloudcli-plugin-task-queue", - "version": "0.11.0", + "version": "0.11.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "cloudcli-plugin-task-queue", - "version": "0.11.0", + "version": "0.11.1", "dependencies": { "js-yaml": "^4.3.2", "ws": "^8.20.0" diff --git a/package.json b/package.json index bd06950..3dee279 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "cloudcli-plugin-task-queue", - "version": "0.11.0", + "version": "0.11.1", "private": true, "type": "module", "scripts": { diff --git a/src/control-api.ts b/src/control-api.ts index 70a327f..df444cf 100644 --- a/src/control-api.ts +++ b/src/control-api.ts @@ -86,7 +86,21 @@ async function send( ): Promise { const doFetch = opts.fetchImpl ?? fetch; try { - const resp = await doFetch(url, init); + // Never follow a redirect. fetch's default is to follow, and Undici keeps custom + // request headers across the hop, X-Task-Queue-Token included. So a redirect from the + // configured base would hand this read+operator-write token to whatever origin the + // Location names. insecureApiBase() only vets the configured base, not where a + // response points next. task-queue-mcp never redirects, so any 3xx is a fault and is + // reported as one. (Audit 2026-09-30/operator-panel-2026-09-p2-queue-read-api F-01, + // CodeRabbit CR-03 on #13.) + const resp = await doFetch(url, { ...init, redirect: 'manual' }); + // The spec returns an opaque-redirect response (type 'opaqueredirect', status 0) for a + // manual redirect; Node's Undici returns the 3xx itself. Refuse both, and do not read + // the body: nothing in a redirect response is ours to act on. + if (resp.type === 'opaqueredirect' || (resp.status >= 300 && resp.status < 400)) { + console.error(`[task-queue] ${what} refused: task-queue-mcp answered with a redirect (${resp.status}); not following it`); + return { status: 502, data: { ok: false, error: `task-queue API answered with a redirect (${resp.status}); refused` } }; + } let data: unknown = {}; try { data = await resp.json(); } catch { data = {}; } if (resp.status === 401 || resp.status === 403) { diff --git a/src/tests/control-api.test.ts b/src/tests/control-api.test.ts index e9b52d1..fd5f9e9 100644 --- a/src/tests/control-api.test.ts +++ b/src/tests/control-api.test.ts @@ -317,3 +317,62 @@ test('an insecure base never sends the token, on reads or writes', async () => { assert.equal(write.status, 500); assert.equal(fetchSpy.calls.length, 0); }); + +// ── redirects are never followed (audit F-01 / CodeRabbit CR-03) ─────── + +import http from 'node:http'; +import type { AddressInfo } from 'node:net'; + +test('every request is sent with redirect: manual', async () => { + const fetchSpy = spyFetch(); + const opts = { apiBase: 'http://127.0.0.1:8485', token: GOOD, fetchImpl: fetchSpy.impl }; + await queueGet('/tasks', opts); + await callControlApi('task-abc', 'approve', {}, opts); + assert.equal(fetchSpy.calls.length, 2); + for (const { init } of fetchSpy.calls) assert.equal(init.redirect, 'manual'); +}); + +test('a 3xx or opaque-redirect response is a 502, not a result', async () => { + for (const shape of [ + { status: 302, type: 'basic' }, + { status: 307, type: 'basic' }, + { status: 0, type: 'opaqueredirect' }, + ]) { + const impl = (async () => ({ ...shape, json: async () => ({ ok: true }) })) as unknown as typeof fetch; + const r = await queueGet('/tasks', { apiBase: 'http://127.0.0.1:8485', token: GOOD, fetchImpl: impl }); + assert.equal(r.status, 502, JSON.stringify(shape)); + assert.match((r.data as { error: string }).error, /redirect/); + } +}); + +test('a real redirect to another origin never delivers the token there', async () => { + // Two real servers on loopback. A answers every request with a 307 to B, and B records + // whether the token arrived. This exercises Node's own fetch, not a stub: the property + // under test is Undici's behaviour when told redirect: 'manual'. + let reachedB = false; + let tokenAtB: string | undefined; + const b = http.createServer((req, res) => { + reachedB = true; + tokenAtB = req.headers['x-task-queue-token'] as string | undefined; + res.writeHead(200, { 'Content-Type': 'application/json' }).end('{"ok":true}'); + }); + await new Promise(r => b.listen(0, '127.0.0.1', () => r())); + const bPort = (b.address() as AddressInfo).port; + const a = http.createServer((req, res) => { + res.writeHead(307, { Location: `http://127.0.0.1:${bPort}${req.url}` }).end(); + }); + await new Promise(r => a.listen(0, '127.0.0.1', () => r())); + const aPort = (a.address() as AddressInfo).port; + try { + const opts = { apiBase: `http://127.0.0.1:${aPort}`, token: GOOD }; + const read = await queueGet('/tasks', opts); + const write = await callControlApi('task-abc', 'approve', {}, opts); + assert.equal(read.status, 502); + assert.equal(write.status, 502); + assert.equal(reachedB, false, 'the redirect target must never be contacted'); + assert.equal(tokenAtB, undefined); + } finally { + await new Promise(r => a.close(() => r())); + await new Promise(r => b.close(() => r())); + } +});