diff --git a/AGENTS.md b/AGENTS.md index 756a45a..4bff6e5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -13,12 +13,16 @@ src/ index.ts UI entry — mount/unmount, app state, action handlers. Talks to the backend only via api.rpc(). No fs, no fetch. server.ts Backend — HTTP + WebSocket, launched by CloudCLI as a - subprocess. Reads queue YAML directly; proxies every - mutation to control-api.ts. Owns the file watcher and - the session launcher. - control-api.ts The single outbound mutation path to task-queue-mcp. - Extracted from server.ts so the auth and transport - guards are unit-testable without booting the server. + subprocess. Reads and writes the queue only through + control-api.ts (task-queue-mcp's HTTP API); parses no + queue YAML. Owns the file watcher (a change trigger) + and the session launcher. + control-api.ts The single outbound path to task-queue-mcp: queue reads + (queueGet) and mutations (callControlApi). Extracted from + server.ts so the auth and transport guards are + unit-testable without booting the server. + queue-token.ts Loads the plugin's client token from the fixed file + under $HOME, failing closed on mode or content. ws-guard.ts The WebSocket upgrade decision, as a pure function of (peer address, Origin, allowlist). Extracted for the same reason: server.ts listens at import time. @@ -68,20 +72,22 @@ importable. esbuild and `tsc` (`allowImportingTsExtensions`) both accept `.ts`. ## Invariants -- **The plugin never writes queue YAML directly.** Reads are direct (fast, watchable); every mutation goes through `control-api.ts` to the MCP control API, inheriting its transition validation, `fcntl` locking, and atomic writes. A new mutation means a new control-API action, never an `fs.writeFile`. +- **The plugin never reads or writes queue YAML directly.** Every mutation goes through `control-api.ts` to the MCP control API, inheriting its transition validation, `fcntl` locking, and atomic writes. Since v0.11.0 every read does too (`queueGet` → `GET /tasks`, `GET /tasks/{id}`), inheriting the queue's TTL, dead-letter and status rules; before that this file globbed the YAML and applied none of them. A new mutation means a new control-API action, never an `fs.writeFile`; a new read means an API call, never a `yamlLoad` of a queue file. The `fs.watch` on the queue root is a change trigger and reads nothing. +- **`truncated` is rendered, never dropped.** `GET /tasks` returns at most 1000 records and says when it cut some off. The header and the dead-letters badge show it. If a view ever needs more than one page, report the number and the use rather than paging silently or asking for the cap to be raised. - **`ControlAction` must match the MCP's route set.** The union type in `control-api.ts`, the route regex in `server.ts`, and the MCP's custom routes are three copies of one contract. Change one, change all three. The two copies that live in *this* repo are now pinned to each other by a source-level test in `control-api.test.ts` — nothing detected the drift before, because adding an action to the union alone compiles and the failure mode is a button that 404s against the plugin's own backend. The third copy is in another repo and still needs a human. - **The task-queue vocabulary lives in `vocabulary.ts`, once, and is gated against its owner.** This plugin does not own the queue's statuses, task types, or workflow modes — `task-queue-mcp`'s `src/tools/queue.py` does. It used to carry four partial hand-written copies (`STATUS_ORDER`, `NON_TERMINAL_STATUSES`, `DETAIL_NON_TERMINAL_STATUSES`, the `statusColor` switch); none of them learned about `routing-failed`, so for months the status most in need of an operator sorted *below* `cancelled`, rendered the same grey as `parked`, and was not offered by the status filter. `manual-then-auto` was the same omission one field over. Two mechanisms hold the line and they are different in kind: `npm run gate:vocabulary` fetches the MCP's `main` and fails on any difference, which catches "upstream changed and we did not"; and the UI maps are `Record` keyed by the vocabulary itself, so adding a status without giving it a sort position and a colour is a `tsc` error rather than a silent fallthrough to `?? 9` and `muted`. Do not weaken either — a `Record` accepts anything and covers nothing, which is precisely how this happened. - **The vocabulary gate has no skip-on-no-network path, and tracks `main`, not a pin.** It exits non-zero if it cannot read the upstream. A check that quietly passes when it could not reach its source of truth is indistinguishable from one that verified something, and that shape is how the dispatcher-side instance (vikunja#324) stayed open for months. It likewise exits non-zero if it parses zero set literals, rather than reporting a vacuous pass over an empty comparison. A vocabulary change merged upstream turning this repo's CI red is the alarm working. `TASK_QUEUE_MCP_REF` exists only for a paired pre-merge change from a shell; **CI must not set it**. - **The gate is its own CI step.** A red vocabulary means "go edit `src/vocabulary.ts`" — a different instruction from any unit failure. Folded into `npm test` either could hide the other, and an expected-red check buried inside a longer step masks whether anything before it ran. - **A queued `manual-then-auto` survives the Start button.** `toWorkflowMode` maps the UI's `review` to `semi-auto` *unless* the task was queued `manual-then-auto`, in which case it passes that through. Both gate this leg; only the second lets the tasks the session spawns run `auto`. Flattening it — which is what this did before #543 — re-pins the whole downstream chain to `semi-auto`, which is the failure vikunja#533 added the mode to fix. An explicit `auto` Start still overrides. - **The plugin acts as `operator`, never as an agent.** Every proxied mutation sends `actor: 'operator'`. This is what makes the MCP's `amend_task` authorization accept it; the plugin must never assert an agent's identity. -- **Mutations fail closed without the secret.** `callControlApi` returns 500 and never attempts the fetch when `TASK_QUEUE_API_SECRET` is empty, and logs why. Do not add a fallback that proceeds without it. +- **The client token comes from a fixed file under `$HOME`, never the env.** `queue-token.ts` reads `$HOME/.config/cloudcli-plugin-task-queue/token` and fails closed (missing, empty, not a regular file, any group/other bit), and `callControlApi` / `queueGet` then return 500 without fetching. Do not add an `env:` grant for a credential to `manifest.json`, and do not make the path an env var. A manifest grant only works if the CloudCLI host holds the value, and every Claude session CloudCLI launches inherits the host's env. That is exactly how the old `TASK_QUEUE_API_SECRET` reached every session (vikunja#396). A test pins the manifest's `env:` grants. +- **The token goes in `X-Task-Queue-Token`, never `Authorization`.** task-queue-mcp's framework offers any bearer to its agent-token verifier; the control routes read only their own header. - **The version comes from `package.json`.** `server.ts` reads it at startup rather than hardcoding a copy — a hardcoded constant silently drifted and reported a stale version on `/health` for two releases. `package.json` and `manifest.json` must also agree. - **There is exactly one agent roster, and it is not in this repo.** `~/scripts/agent-launch.yml` (override: `AGENT_LAUNCH_POLICY`) is read by this plugin *and* by `task-dispatcher.py`. A hardcoded `AGENT_PROJECTS` map used to live in `server.ts`; it drifted, lost an agent, and that drift is vikunja#523. Do not add a literal roster back — extend the file. - **A run-as agent goes through its launcher, always.** An entry with `run_as_user` is spawned as `sudo -n -u …`, never as `claude`. Spawning `claude` directly for such an agent bypasses the launcher's identity guard and yields a session that appears as that agent in every log while holding none of its credentials. A missing or non-executable launcher is refused by name — there is no fallback path, deliberately. - **The launch policy fails closed, loudly.** A missing or malformed policy file disables Start with a named error. It must never degrade to an empty policy: an empty policy makes `run_as_user` absent for every agent, which is precisely the impersonation above. - **Path guards resolve symlinks, and there is one guard.** `resolveAllowedPath` in `path-guard.ts` calls `fs.realpathSync` *before* the prefix compare; `path.resolve` alone normalises `..` but follows nothing, so a symlink inside an allowed prefix would escape it. The trailing `path.sep` on each prefix is load-bearing — without it `/comms-other` matches `/comms`. Both the context-ref preview and the headless-run reader call this one function. Do not write a second check; a subtly different copy is how this property gets lost. -- **The dead-letters section is read-only, and its count is not conditional on being open.** `GET /dead-letters` reads `dead-letters/*.yml` directly like every other read here; the one mutation (requeue) goes through `control-api.ts` like every other mutation. The count loads on every refresh rather than on expand — a number that appears only after the operator opens the section is a number nobody sees, which is the failure this whole surface exists to end. The healthy value is zero and the heading renders that too. +- **The dead-letters section is read-only, and its count is not conditional on being open.** `GET /dead-letters` reads `GET /tasks?status=failed&include_dead_letters=true` and keeps records whose `queue_location` is `dead-letters` (never by status: a `failed` task in the live queue has no Requeue); the one mutation (requeue) goes through `control-api.ts` like every other mutation. The count loads on every refresh rather than on expand — a number that appears only after the operator opens the section is a number nobody sees, which is the failure this whole surface exists to end. The healthy value is zero and the heading renders that too. - **Dead letters are grouped by `failed_reason`, never listed flat.** Seventeen records carrying one identical reason are one bug that fired seventeen times; seventeen sibling rows read as seventeen unrelated problems, which is roughly how they were treated for three months (vikunja#557). The grouping is a pure function in `dead-letters.ts` with tests, not a loop in the panel. - **A dead-letter record with no `id` is skipped, not rendered.** A row that cannot be addressed is a row whose Requeue button could not work, and offering one would be a lie. Likewise an unparseable failure timestamp renders as unknown — `ago()` maps an invalid date to "just now", which on this surface would claim a three-month-old drop happened seconds ago. - **The queue watcher does not see `dead-letters/`.** `fs.watch` is not recursive on Linux and the watch is on the queue root, so a task arriving in the subdirectory fires no event. That is deliberate: the section is collapsed, a dead letter is not live work, and the dispatcher unlinks the original from the root as it dead-letters — which does fire. Do not add a recursive watch to stream a surface nobody is watching live. @@ -108,7 +114,7 @@ npm test npm run gate:vocabulary # parity with task-queue-mcp main — needs network, by design ``` -Tests cover `control-api.ts` (the secret gate, task-id validation, header and body shape per action, transport-failure mapping, pass-through of the MCP's authorization rejections, and the union/route-regex drift gate), `dead-letters.ts` (the real dispatcher record shape including the `Date`-valued `created` js-yaml hands back, the id-less and reason-less fallbacks, and the grouping rule against the live seventeen-identical-reasons case), `ws-guard.ts` (all three upgrade cases, including the loopback-with-no-Origin one that v0.4.0 broke), `launch-policy.ts` (every closed-set rejection, whole-document rejection, and both argv shapes), `path-guard.ts` (a **real** symlink escape, traversal, the `/comms-other` sibling case — with real files in a tmpdir, because a mocked `fs` cannot demonstrate that realpath runs first), `launch-log.ts` (round-trip against the real `launchLogName`, the live bare-UUID orphans, path-shaped route ids, fence extraction including the dropped unterminated case, and the birthtime-after-mtime fallback), `vocabulary.ts` (every status has a sort position and a colour, `routing-failed` sorts above `in-progress` and is not muted, the derived non-terminal set, and the `manual-then-auto` pass-through through `buildLaunchArgv`), `gates/python-sets.ts` (the real `queue.py` shape including interleaved comments, and the two constructs that must NOT parse as string sets — a derived set and an annotated dict), and the reconnect schedule. The UI panels are not otherwise unit-tested — verify them in CloudCLI after `./deploy.sh && pm2 restart cloudcli`. +Tests cover `queue-token.ts` (real files in a tmpdir: missing, empty, directory, every group/other mode bit refused, `0600` and `0400` accepted, and no error carrying the file's content), `control-api.ts` (the token gate, the `X-Task-Queue-Token` header and the absence of `Authorization` and the retired secret header, the read helper and its query builder, the manifest's `env:` grants, task-id validation, header and body shape per action, transport-failure mapping, pass-through of the MCP's authorization rejections, and the union/route-regex drift gate), `dead-letters.ts` (the real dispatcher record shape including the `Date`-valued `created` js-yaml hands back, the id-less and reason-less fallbacks, and the grouping rule against the live seventeen-identical-reasons case), `ws-guard.ts` (all three upgrade cases, including the loopback-with-no-Origin one that v0.4.0 broke), `launch-policy.ts` (every closed-set rejection, whole-document rejection, and both argv shapes), `path-guard.ts` (a **real** symlink escape, traversal, the `/comms-other` sibling case — with real files in a tmpdir, because a mocked `fs` cannot demonstrate that realpath runs first), `launch-log.ts` (round-trip against the real `launchLogName`, the live bare-UUID orphans, path-shaped route ids, fence extraction including the dropped unterminated case, and the birthtime-after-mtime fallback), `vocabulary.ts` (every status has a sort position and a colour, `routing-failed` sorts above `in-progress` and is not muted, the derived non-terminal set, and the `manual-then-auto` pass-through through `buildLaunchArgv`), `gates/python-sets.ts` (the real `queue.py` shape including interleaved comments, and the two constructs that must NOT parse as string sets — a derived set and an annotated dict), and the reconnect schedule. The UI panels are not otherwise unit-tested — verify them in CloudCLI after `./deploy.sh && pm2 restart cloudcli`. Two build/test gotchas worth knowing before you touch either script: diff --git a/CHANGELOG.md b/CHANGELOG.md index 0747138..704526b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,54 @@ All notable changes to this project will be documented in this file. +## [0.11.0] - 2026-09-29 + +Own client token, and every read through task-queue-mcp's API. Build +`operator-panel-2026-09-p2-queue-read-api`; vikunja#396. **Requires task-queue-mcp +v0.11.0 or later** and a token file (below). + +### Changed + +- **The plugin authenticates with its own client token**, read from + `$HOME/.config/cloudcli-plugin-task-queue/token` and sent as `X-Task-Queue-Token`. + `manifest.json` no longer requests `env:TASK_QUEUE_API_SECRET`, and no replacement + grant was added. A manifest grant only works if the CloudCLI host holds the value, and + every Claude session CloudCLI launches inherits the host's environment, so the old + shared secret sat in every agent session. The host now holds neither the token nor its + path. Writes are recorded with `channel: cloudcli`. +- **Fails closed** when the token file is missing, empty, not a regular file, or has any + group or other permission bit. The UI error names the path and the problem; the token + is never logged. A missing file is re-checked on the next request. +- **Every queue read goes through the API** (`GET /tasks`, `GET /tasks/{id}`). That covers + the task list, the task detail, the Start route's task lookup, the dead-letters section + and the headless-run status index. The backend no longer parses queue YAML at all. The + list now follows the queue owner's rules: finished tasks past their `ttl_days` age out, + as they do for every agent's `list_tasks`, where this plugin previously showed every + file in the directory. Task detail now also resolves archived and dead-lettered ids. +- **Truncation is shown.** The API returns at most 1000 records per read. When it cuts + records off, the header reads `truncated: showing N of M`, and the dead-letters badge + says the count may be low. +- **Start launches only live, unfinished work.** Task lookup now also resolves archived and + dead-lettered records, which the old directory scan never saw. `POST /tasks/:id/start` + refuses any task outside the live queue or in a terminal status with a 409 and the reason + (`launchRefusal` in `launch-guards.ts`), so the launch surface is no wider than before. +- **`TASK_QUEUE_API` must be `https://`, or `http://` to a loopback host.** The token goes + on every read and write, so the plugin refuses to send it in cleartext to another host + (each request fails with an error naming the variable). The default passes. +- The `tasks` WebSocket event no longer carries a file `count`, which the UI never read. + The queue watcher is now a change trigger only. + +### Removed + +- `TASK_QUEUE_API_SECRET` and the `X-Task-Queue-Secret` header. + +### Security + +- **js-yaml 4.3.1 → 4.3.2** (GHSA-2883-xcg3-v3hh, high: merge keys with empty sources + are not bounded by `maxTotalMergeKeys`). The plugin parses one YAML file, the + operator-owned launch policy, so exposure was low; the production-dependency audit gate + failed on it. + ## [0.10.0] - 2026-08-29 Tracker: vikunja#560. Build plan: agent-workflow-interop-2026-08, Phase 5.5 and 5.6. diff --git a/README.md b/README.md index 0c1b8ad..33cb23e 100644 --- a/README.md +++ b/README.md @@ -15,30 +15,33 @@ A CloudCLI tab plugin that gives [task-queue-mcp](https://github.com/TadMSTR/tas ## Architecture -Four moving parts. The asymmetry is the important bit: **reads go direct to the queue files, writes always go through the MCP server.** +Four moving parts. **Every read and every write goes through task-queue-mcp's HTTP API** (since v0.11.0, which needs task-queue-mcp v0.11.0 or later). The backend only watches the queue directory to learn *when* to re-read. ```mermaid flowchart LR UI["Plugin UI
dist/index.js"] -->|api.rpc| BE["Plugin backend
dist/server.js"] - BE -->|"read: YAML"| Q[("Task queue
*.yml")] - BE -->|"write: POST + X-Task-Queue-Secret"| MCP["task-queue-mcp
control API :8485"] - MCP -->|validated write| Q - BE -.->|"WebSocket: file-change events"| UI + BE -->|"GET /tasks, GET /tasks/:id
X-Task-Queue-Token"| MCP["task-queue-mcp
HTTP API :8485"] + BE -->|"POST /tasks/:id/*
X-Task-Queue-Token"| MCP + MCP -->|validated read / write| Q[("Task queue
*.yml")] + Q -.->|"fs.watch: change trigger"| BE + BE -.->|"WebSocket: tasks event"| UI ``` -Reading directly keeps the list fast and lets the backend watch the directory for live updates. Routing every write through `task-queue-mcp` means mutations inherit its transition validation, `fcntl` locking, and atomic writes, so the plugin can never leave a task in a state the queue's own rules forbid. **The plugin never writes queue YAML directly.** +Routing writes through `task-queue-mcp` means mutations inherit its transition validation, `fcntl` locking, and atomic writes, so the plugin can never leave a task in a state the queue's own rules forbid. Routing reads through it means the plugin shows what the queue's owner says is there, with its TTL, dead-letter and status rules, instead of a second parser's opinion. Before v0.11.0 the plugin globbed the YAML itself and applied none of those rules. **The plugin never reads or writes queue YAML directly.** - **UI** (`dist/index.js`) — renders the tab panel: a filterable task list and a detail view with history timeline, amendments, and context-ref previews. - **Backend** (`dist/server.js`) — HTTP + WebSocket server launched by CloudCLI. Picks a free ephemeral port at startup and reports it to CloudCLI as JSON on stdout. The UI reaches it through CloudCLI's plugin RPC API (`api.rpc()`). -Live updates arrive over WebSocket: the backend watches the queue directory and pushes a `tasks` event when files change; the UI debounces refreshes by 2s. +Live updates arrive over WebSocket: the backend watches the queue directory and pushes a `tasks` event when files change; the UI debounces, then re-reads through the API. + +If the API truncated a read (it returns at most 1000 records per call), the header says **truncated: showing N of M** and the dead-letters badge says the count may be low. It is never hidden. ## Features - Task list with filters by agent, status, and task type, grouped by target agent - Detail view: full task data, history timeline, amendments, and context-ref file previews (confined to the queue and comms directories) - Session launch — **review mode** (plan permission; the agent presents a summary and waits) or **auto mode** (the agent claims the task and executes) -- Lifecycle actions, all proxied through the shared-secret control API as actor `operator`: +- Lifecycle actions, all proxied through the control API as actor `operator`. Each is recorded in the task's history with `channel: cloudcli`: - **Approve** a submitted or pending task - **Cancel** a non-terminal task — a graceful terminal record, never deleted, instead of mislabelling it `failed` - **Park / Unpark** — pause a task without losing sight of it. A parked task stays in the list, renders muted with an `Unpark` button, is exempt from TTL expiry, and won't be picked up until you unpark it. Unparking returns it to the status it was parked from. @@ -175,21 +178,36 @@ pm2 restart cloudcli | Variable | Default | Purpose | |----------|---------|---------| -| `TASK_QUEUE_API` | `http://127.0.0.1:8485` | Base URL of the task-queue-mcp HTTP control API. Configurable — the default assumes the MCP server runs loopback-local to CloudCLI. | -| `TASK_QUEUE_API_SECRET` | — | Shared secret sent as `X-Task-Queue-Secret` on every mutation. **Required** — mutations fail closed if unset. Comes from your secret store, never from source. | +| `TASK_QUEUE_API` | `http://127.0.0.1:8485` | Base URL of the task-queue-mcp HTTP API. Configurable — the default assumes the MCP server runs loopback-local to CloudCLI. Must be `https://`, or `http://` to a loopback host: the client token goes on every request, so anything else is refused. | | `CLOUDCLI_ORIGIN` | — | Additional allowed WebSocket origin, and the origin the CloudCLI host's plugin proxy sends on its upstream leg. Both sides read the same variable so they cannot disagree. `http://localhost:3001` and `http://127.0.0.1:3001` are always allowed. | | `AGENT_LAUNCH_POLICY` | `~/scripts/agent-launch.yml` | Path to the launch policy file (see [Session launch behaviour](#session-launch-behaviour)). | +## The client token + +The plugin authenticates to task-queue-mcp with **its own client token**, read from a fixed file: + +``` +$HOME/.config/cloudcli-plugin-task-queue/token +``` + +- The file holds the plaintext token and nothing else (a trailing newline is ignored). It must be a regular file with mode `0600` or `0400`, in a directory you keep `0700`. +- task-queue-mcp holds only the token's `sha256:` digest, registered with `read,operator-write` scopes as client `cloudcli`. See task-queue-mcp's README for minting a token and its digest. +- The token is sent as `X-Task-Queue-Token`, never `Authorization`. + +**Fails closed.** If the file is missing, empty, not a regular file, or has any group or other permission bit, every read and write fails. The UI shows an error naming the path and the problem, and the CloudCLI process's stderr log records it once. The token itself is never logged. A missing file is re-checked on the next request, so writing it takes effect without a restart. A token that has loaded is kept until CloudCLI restarts, so a rotated token needs a restart. + +**Why a file and not an env var.** Before v0.11.0 the plugin used a shared secret granted through the manifest (`env:TASK_QUEUE_API_SECRET`). A manifest grant only works if the CloudCLI host process holds the variable, and **every Claude session CloudCLI launches inherits the host's environment**. So the credential for the queue's control API sat in every agent session. The host already passes `HOME` to every plugin, so a fixed path under it needs no manifest grant, no host variable, and no change to CloudCLI. The host never holds the token or its path. + +This is containment, not a boundary: the file is readable by the user CloudCLI runs as. What it buys is that no process's environment carries the credential, the plugin's writes are attributable (`channel: cloudcli`), and its token can be revoked on its own. + ### How the plugin receives its env vars -CloudCLI launches the backend as a subprocess and **strips host environment variables from it by default**, including secrets. A host var reaches the plugin only when **both** are true: +CloudCLI launches the backend as a subprocess and **strips host environment variables from it by default**. A host var reaches the plugin only when **both** are true: -1. `manifest.json` declares it — `permissions: ["env:TASK_QUEUE_API", "env:TASK_QUEUE_API_SECRET", "env:CLOUDCLI_ORIGIN"]`, and +1. `manifest.json` declares it — `permissions: ["env:TASK_QUEUE_API", "env:CLOUDCLI_ORIGIN"]`, and 2. the var is on CloudCLI's host-side plugin env allowlist. -This needs a CloudCLI build with permission-gated env passthrough. **Without it the launcher silently strips the secret and every mutation fails closed** — the UI reports an error, but nothing about the failure names the passthrough as the cause. If mutations fail while reads work, check this first. The control-API call path logs missing-secret and unreachable-transport failures to the CloudCLI process's stderr log, and never logs the secret value. - -Adding a new env var means updating *both* the manifest `permissions` and the host allowlist, or it is silently refused. +Adding a new env var means updating *both* the manifest `permissions` and the host allowlist, or it is silently refused. **Do not use this path for a credential.** Anything the host holds reaches every session it launches; use a file under `$HOME`, as the token does. ## Backend API @@ -198,7 +216,7 @@ The backend exposes a small HTTP API consumed by the UI via `api.rpc()`. | Method | Path | Description | |--------|------|-------------| | `GET` | `/health` | Liveness check; returns `{status, uptime, version}` | -| `GET` | `/tasks` | List tasks; query params `agent`, `status`, `type` | +| `GET` | `/tasks` | List tasks; query params `agent`, `status`, `type`. Returns `{tasks, count, truncated}` | | `GET` | `/tasks/:id` | Task detail plus context-ref previews | | `POST` | `/tasks/:id/start` | Launch a session; body `{mode: "review"\|"auto"}`. Spawns locally, writes a run record, and records the launch in the task's history | | `POST` | `/tasks/:id/approve` | Approve — proxied | @@ -208,13 +226,13 @@ The backend exposes a small HTTP API consumed by the UI via `api.rpc()`. | `POST` | `/tasks/:id/unpark` | Unpark; body `{note?, status?}` — proxied | | `POST` | `/tasks/:id/amend` | Append an amendment; body `{amendment, reason?}` — proxied | | `POST` | `/tasks/:id/requeue` | Requeue a dead-lettered task; body `{note?}` — proxied | -| `GET` | `/dead-letters` | List dead-lettered tasks. Read-only | +| `GET` | `/dead-letters` | List dead-lettered tasks; returns `{deadLetters, truncated}`. Read-only | | `GET` | `/headless-runs` | List headless agent runs; query param `agent`. Read-only | | `GET` | `/headless-runs/:id` | One run's full log text plus scraped commands; `:id` is `-`. Read-only | -Reads are served directly from the queue YAML. Every proxied mutation carries the `X-Task-Queue-Secret` header and an `actor` of `operator`. +Task, dead-letter and headless-run status reads are served from task-queue-mcp's read API. Every request to task-queue-mcp carries the plugin's `X-Task-Queue-Token`. A read the API refuses or cannot serve returns `502` with the API's error message. -WebSocket upgrade is handled on the same port. Clients receive `{type: "connected", version}` on connect and `{type: "tasks", count, changed}` when task files change. +WebSocket upgrade is handled on the same port. Clients receive `{type: "connected", version}` on connect and `{type: "tasks", changed}` when task files change. The upgrade handler gates on the **peer address** first: the server binds `127.0.0.1` on an ephemeral port, so a non-loopback peer is refused outright. An `Origin` is then checked against the allowlist **only if one is present**. A loopback peer that sends no `Origin` is accepted, because that is what CloudCLI's own plugin WS proxy looks like — the `ws` client library sends no `Origin` unless one is passed, and that leg is already authenticated by CloudCLI before the proxy is invoked. A present-but-wrong `Origin` is still refused. diff --git a/manifest.json b/manifest.json index 48d55df..36db53e 100644 --- a/manifest.json +++ b/manifest.json @@ -1,7 +1,7 @@ { "name": "task-queue", "displayName": "Task Queue", - "version": "0.10.0", + "version": "0.11.0", "description": "Task queue dashboard \u2014 view, filter, and launch agent tasks.", "author": "TadMSTR", "icon": "icon.svg", @@ -11,7 +11,6 @@ "server": "dist/server.js", "permissions": [ "env:TASK_QUEUE_API", - "env:TASK_QUEUE_API_SECRET", "env:CLOUDCLI_ORIGIN" ] } diff --git a/package-lock.json b/package-lock.json index cc913cc..af830c5 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,14 +1,14 @@ { "name": "cloudcli-plugin-task-queue", - "version": "0.3.0", + "version": "0.11.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "cloudcli-plugin-task-queue", - "version": "0.3.0", + "version": "0.11.0", "dependencies": { - "js-yaml": "^4.2.0", + "js-yaml": "^4.3.2", "ws": "^8.20.0" }, "devDependencies": { @@ -537,9 +537,9 @@ } }, "node_modules/js-yaml": { - "version": "4.3.1", - "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-4.3.1.tgz", - "integrity": "sha512-CY6crGq313MX8GkwvB7tzgp99vjQxY1++5y10/BKN/GUfHqWaOGQMNZkBvqSzsZKWk/ijwHlWzzkLulsGHhjWQ==", + "version": "4.3.2", + "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-4.3.2.tgz", + "integrity": "sha512-SFNOvSJ+Dgf/9An904Yx+CgSlIPCkIpao4qo51lpee25TIRejdH3rhR4EZMGoNx3/TP3O+wzWuiTFl4sqbltzA==", "funding": [ { "type": "github", diff --git a/package.json b/package.json index 6f94df9..bd06950 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "cloudcli-plugin-task-queue", - "version": "0.10.0", + "version": "0.11.0", "private": true, "type": "module", "scripts": { @@ -17,7 +17,7 @@ "typescript": "^5.5.0" }, "dependencies": { - "js-yaml": "^4.2.0", + "js-yaml": "^4.3.2", "ws": "^8.20.0" } } diff --git a/src/control-api.ts b/src/control-api.ts index fc94c83..70a327f 100644 --- a/src/control-api.ts +++ b/src/control-api.ts @@ -1,20 +1,32 @@ -// MCP control API proxy (mutations). +// task-queue-mcp HTTP API client: queue mutations and queue reads. // -// The single validated, shared-secret-gated write path for queue mutations -// (approve/cancel/status/park/unpark/amend/requeue). All of them proxy here so -// they inherit the MCP core's transition validation + fcntl locking; the plugin -// never mutates task YAML directly. Reads stay direct (see server.ts). +// Mutations (approve/cancel/status/park/unpark/amend/requeue) proxy to the control API so +// they inherit the MCP core's transition validation and fcntl locking; the plugin never +// mutates task YAML. Since v0.11.0 READS go through the API too (GET /tasks, +// GET /tasks/{id}), so the TTL, dead-letter and status rules live in one place, the queue's +// owner, instead of being re-implemented here. // -// `requeue` is the operator's path out of the dead-letter queue. It is gated on the MCP -// side by the same shared secret as every other route here, and its MCP *tool* twin -// refuses any agent identity outright — a plugin acting as `operator` is exactly the -// caller it is meant for. +// Every request carries this plugin's own client token in `X-Task-Queue-Token`. It is never +// sent as `Authorization`: task-queue-mcp's framework authenticates bearers app-wide, and +// the control routes deliberately read only their own header. See queue-token.ts for where +// the token comes from and why it is not an env var. // -// Extracted from server.ts so the auth/transport guards are unit-testable -// without booting the plugin's HTTP server. +// `requeue` is the operator's path out of the dead-letter queue. Its MCP *tool* twin +// refuses any agent identity outright; a plugin acting as `operator` over the control API +// is exactly the caller it is meant for. +// +// Extracted from server.ts so the auth/transport guards are unit-testable without booting +// the plugin's HTTP server. + +import type { TokenResult } from './queue-token.ts'; const VALID_ID = /^[a-zA-Z0-9_-]+$/; +export const TOKEN_HEADER = 'X-Task-Queue-Token'; + +/** The largest page task-queue-mcp's GET /tasks returns. */ +export const LIST_PAGE_MAX = 1000; + export type ControlAction = 'approve' | 'cancel' | 'status' | 'park' | 'unpark' | 'amend' | 'requeue'; @@ -24,62 +36,138 @@ export interface ControlApiResult { } export interface ControlApiOptions { - /** Base URL of the control API, no trailing slash (e.g. http://127.0.0.1:8485). */ + /** Base URL of the API, no trailing slash (e.g. http://127.0.0.1:8485). */ apiBase: string; - /** Shared secret; an empty string means the plugin never received it. */ - secret: string; + /** The loaded client token, or why it could not be loaded. */ + token: TokenResult; /** Injectable fetch, for tests. Defaults to the global fetch. */ fetchImpl?: typeof fetch; } /** - * Proxy a queue mutation to the MCP control API. The shared secret is sent as - * an `X-Task-Queue-Secret` header. Returns a `{ status, data }` result — it - * never throws; transport failures are mapped to a 502. + * Why `apiBase` must not carry the token, or null if it may. + * + * The token (read + operator-write) goes on every request, reads included. Over plain + * HTTP to another host, anything on the path could take it and act as the operator. So + * `http://` is accepted only for a loopback host — the default, `http://127.0.0.1:8485` — + * and anything else must be `https://`. */ -export async function callControlApi( - taskId: string, - action: ControlAction, - body: Record, - opts: ControlApiOptions, -): Promise { - if (!VALID_ID.test(taskId)) { - return { status: 400, data: { ok: false, error: 'invalid task id' } }; +export function insecureApiBase(apiBase: string): string | null { + let url: URL; + try { + url = new URL(apiBase); + } catch { + return `TASK_QUEUE_API ${JSON.stringify(apiBase)} is not a URL`; } - if (!opts.secret) { - // Previously silent: the request never left the plugin, and nothing recorded - // that it tried. Log to stderr (captured into ~/.pm2/logs/cloudcli-error.log) - // so a misconfigured passthrough is diagnosable instead of a mystery 500. - console.error( - `[task-queue] control API call for ${action} on task ${taskId} aborted: ` + - 'TASK_QUEUE_API_SECRET is empty in the plugin process. The host must grant it ' + - 'via the manifest `permissions` (env:TASK_QUEUE_API_SECRET) and have it set in its own env.', - ); - return { status: 500, data: { ok: false, error: 'TASK_QUEUE_API_SECRET not configured' } }; + if (url.protocol === 'https:' && url.hostname) return null; + if (url.protocol === 'http:') { + // URL keeps the brackets on an IPv6 literal. + const host = url.hostname.toLowerCase(); + if (host === 'localhost' || host === '[::1]' || /^127(\.\d{1,3}){3}$/.test(host)) return null; } + return `TASK_QUEUE_API ${JSON.stringify(apiBase)} refused: the client token is only sent over https://, or over http:// to a loopback host`; +} + +/** + * A request that never left the plugin because it has no usable token. Logged to stderr + * (captured into the CloudCLI process's PM2 error log) and returned as a 500 whose error + * names the file, so the UI says exactly what to fix. + */ +function noToken(what: string, error: string): ControlApiResult { + console.error(`[task-queue] ${what} aborted: ${error}`); + return { status: 500, data: { ok: false, error } }; +} +async function send( + url: string, + init: RequestInit, + what: string, + opts: ControlApiOptions, +): Promise { const doFetch = opts.fetchImpl ?? fetch; - const url = `${opts.apiBase}/tasks/${encodeURIComponent(taskId)}/${action}`; try { - const resp = await doFetch(url, { - method: 'POST', - headers: { - 'Content-Type': 'application/json', - 'X-Task-Queue-Secret': opts.secret, - }, - body: JSON.stringify({ actor: 'operator', ...body }), - }); + const resp = await doFetch(url, init); let data: unknown = {}; try { data = await resp.json(); } catch { data = {}; } + if (resp.status === 401 || resp.status === 403) { + // The token loaded but the server refused it: revoked, never registered, or missing + // a scope. Say so in the log; the response body carries no token either way. + console.error(`[task-queue] ${what} refused by task-queue-mcp (${resp.status}): ${JSON.stringify(data)}`); + } return { status: resp.status, data }; } catch (err) { - console.error( - `[task-queue] control API ${action} on task ${taskId} unreachable at ${url}: ${(err as Error).message}`, - ); + console.error(`[task-queue] ${what} unreachable at ${url}: ${(err as Error).message}`); // SECURITY[accepted]: err.message (a Node fetch connection error, e.g. ECONNREFUSED — // not a stack trace or internal path) is surfaced to the CloudCLI UI. Client is Ted's // authenticated, loopback-bound operator UI; matches the accepted OE-02 precedent from // cloudcli-plugin-plane. Genericize if this endpoint is ever exposed beyond loopback. - return { status: 502, data: { ok: false, error: `control API unreachable: ${(err as Error).message}` } }; + return { status: 502, data: { ok: false, error: `task-queue API unreachable: ${(err as Error).message}` } }; } } + +/** + * Proxy a queue mutation to the control API. Returns a `{ status, data }` result and never + * throws; transport failures are mapped to a 502. + */ +export async function callControlApi( + taskId: string, + action: ControlAction, + body: Record, + opts: ControlApiOptions, +): Promise { + if (!VALID_ID.test(taskId)) { + return { status: 400, data: { ok: false, error: 'invalid task id' } }; + } + const what = `control API ${action} on task ${taskId}`; + const insecure = insecureApiBase(opts.apiBase); + if (insecure) return noToken(what, insecure); + if (!opts.token.ok) return noToken(what, opts.token.error); + + return send( + `${opts.apiBase}/tasks/${encodeURIComponent(taskId)}/${action}`, + { + method: 'POST', + headers: { 'Content-Type': 'application/json', [TOKEN_HEADER]: opts.token.token }, + // The server pins the actor to `operator` and ignores this field; it is sent so the + // request says what it is when read in isolation. + body: JSON.stringify({ actor: 'operator', ...body }), + }, + what, + opts, + ); +} + +/** + * GET a read route. `route` is a path relative to the API base, starting with `/`, with + * any query string already encoded (see tasksQuery). Never throws. + */ +export async function queueGet(route: string, opts: ControlApiOptions): Promise { + const what = `read ${route.split('?')[0]}`; + const insecure = insecureApiBase(opts.apiBase); + if (insecure) return noToken(what, insecure); + if (!opts.token.ok) return noToken(what, opts.token.error); + return send( + `${opts.apiBase}${route}`, + { method: 'GET', headers: { [TOKEN_HEADER]: opts.token.token } }, + what, + opts, + ); +} + +/** The route for GET /tasks with these filters. Empty values are dropped, not sent. */ +export function tasksQuery(params: Record): string { + const q = new URLSearchParams(); + for (const [k, v] of Object.entries(params)) { + if (v === undefined || v === '') continue; + q.set(k, String(v)); + } + const s = q.toString(); + return s ? `/tasks?${s}` : '/tasks'; +} + +/** A one-line description of a failed API result, for the UI. Never includes a token. */ +export function apiErrorMessage(result: ControlApiResult): string { + const data = result.data as { error?: unknown } | null; + const detail = typeof data?.error === 'string' ? data.error : 'no detail'; + return `task-queue API ${result.status}: ${detail}`; +} diff --git a/src/index.ts b/src/index.ts index 6227b60..735cbc6 100644 --- a/src/index.ts +++ b/src/index.ts @@ -10,6 +10,10 @@ import { createWsClient, WsClient } from './panels/ws-client.ts'; interface AppState { tasks: Task[]; + /** Records the API matched, which exceeds tasks.length when it truncated. */ + taskCount: number; + /** The API's page limit cut the list. Rendered in the header, never hidden. */ + tasksTruncated: boolean; selectedTaskId: string | null; selectedTask: Task | null; contextPreviews: Map; @@ -22,6 +26,7 @@ interface AppState { selectedRun: HeadlessRunDetail | null; runAgentFilter: string; deadLetters: DeadLetter[]; + deadLettersTruncated: boolean; /** Collapsed by default — the healthy count is zero. */ deadLettersExpanded: boolean; } @@ -33,6 +38,8 @@ export function mount(container: HTMLElement, api: PluginAPI): void { const state: AppState = { tasks: [], + taskCount: 0, + tasksTruncated: false, selectedTaskId: null, selectedTask: null, contextPreviews: new Map(), @@ -45,6 +52,7 @@ export function mount(container: HTMLElement, api: PluginAPI): void { selectedRun: null, runAgentFilter: '', deadLetters: [], + deadLettersTruncated: false, deadLettersExpanded: false, }; @@ -88,8 +96,10 @@ export function mount(container: HTMLElement, api: PluginAPI): void { async function loadTasks(): Promise { try { - const res = await api.rpc('GET', 'tasks') as { tasks: Task[] }; + const res = await api.rpc('GET', 'tasks') as { tasks: Task[]; count?: number; truncated?: boolean }; state.tasks = res.tasks ?? []; + state.taskCount = res.count ?? state.tasks.length; + state.tasksTruncated = res.truncated === true; state.error = null; } catch (err) { state.error = (err as Error).message; @@ -122,11 +132,13 @@ export function mount(container: HTMLElement, api: PluginAPI): void { // a count nobody sees. That is the failure mode this whole surface exists to end. async function loadDeadLetters(): Promise { try { - const res = await api.rpc('GET', 'dead-letters') as { deadLetters: DeadLetter[] }; + const res = await api.rpc('GET', 'dead-letters') as { deadLetters: DeadLetter[]; truncated?: boolean }; state.deadLetters = res.deadLetters ?? []; + state.deadLettersTruncated = res.truncated === true; } catch { // A failure here must not blank the task list — this section is secondary. state.deadLetters = []; + state.deadLettersTruncated = false; } } @@ -411,6 +423,7 @@ export function mount(container: HTMLElement, api: PluginAPI): void { renderDeadLetters(root, { deadLetters: state.deadLetters, + truncated: state.deadLettersTruncated, expanded: state.deadLettersExpanded, colors: c, onToggle: () => { @@ -438,6 +451,9 @@ export function mount(container: HTMLElement, api: PluginAPI): void { header.innerHTML = ` Task Queue ${state.tasks.length} tasks + ${state.tasksTruncated + ? `truncated: showing ${state.tasks.length} of ${state.taskCount}` + : ''} diff --git a/src/launch-guards.ts b/src/launch-guards.ts index c780589..4e4b2b5 100644 --- a/src/launch-guards.ts +++ b/src/launch-guards.ts @@ -35,6 +35,7 @@ */ import fs from 'node:fs'; +import { isTerminal } from './vocabulary.ts'; import path from 'node:path'; /** Where each agent's server-side environment file lives. */ @@ -181,3 +182,21 @@ export function preLaunchEnv( return { ok: true, env }; } + +/** + * Why a Start must not launch a session for this task, or null if it may. + * + * The task lookup goes through task-queue-mcp's `GET /tasks/{id}`, which also resolves + * archived and dead-lettered records. Before v0.11.0 the plugin scanned only the live + * queue directory, so Start could never reach them. Viewing them is useful; launching an + * agent at one is not. A dead letter's route is to Requeue, and a finished task has + * nothing left to do. The agent would be refused when it tried to claim, but by then a + * session has already been spawned. + */ +export function launchRefusal(task: { queue_location?: unknown; status?: unknown }): string | null { + const location = typeof task.queue_location === 'string' ? task.queue_location : 'queue'; + if (location !== 'queue') return `task is in ${location}, not the live queue`; + const status = typeof task.status === 'string' ? task.status : ''; + if (isTerminal(status)) return `task is ${status}`; + return null; +} diff --git a/src/panels/dead-letters.ts b/src/panels/dead-letters.ts index 9e48d0f..546804d 100644 --- a/src/panels/dead-letters.ts +++ b/src/panels/dead-letters.ts @@ -18,6 +18,8 @@ import { groupByReason } from '../dead-letters.ts'; */ interface DeadLettersOptions { deadLetters: DeadLetter[]; + /** The API's page limit cut the read, so the count below is a floor. */ + truncated: boolean; expanded: boolean; colors: ThemeColors; onToggle: () => void; @@ -53,6 +55,7 @@ export function renderDeadLetters(parent: HTMLElement, opts: DeadLettersOptions) : `${count} dropped · ${groupByReason(deadLetters).length} distinct reason${ groupByReason(deadLetters).length === 1 ? '' : 's' }`; + if (opts.truncated) badge.textContent += ' · truncated, there may be more'; head.appendChild(badge); section.appendChild(head); diff --git a/src/queue-token.ts b/src/queue-token.ts new file mode 100644 index 0000000..a3371f4 --- /dev/null +++ b/src/queue-token.ts @@ -0,0 +1,92 @@ +// The plugin's task-queue-mcp client token, read from a fixed file under $HOME. +// +// Until v0.11.0 the plugin authenticated with TASK_QUEUE_API_SECRET, granted through the +// manifest's `env:` permission. That grant only works if the CloudCLI HOST process holds +// the value, and everything in the host's environment is inherited by every Claude session +// CloudCLI launches. So the one credential gating the queue's control API sat in the +// environment of every agent session on the host (vikunja#396). +// +// The token is therefore NOT an env var, and neither is its path. The host passes HOME to +// every plugin, so a fixed path under it needs no manifest grant, no host variable and no +// change to CloudCLI. The host never holds the token or knows where it is. +// +// This is containment, not a boundary: the file is readable by the user CloudCLI runs as, +// which is also the user its sessions run as. What it removes is the credential from every +// process's environment, and it makes this client's writes attributable (`channel: +// cloudcli`) and its token revocable on its own. + +import fs from 'node:fs'; +import path from 'node:path'; + +/** Where the token lives, relative to HOME. Documented in the README; do not make it an env var. */ +export function tokenPath(home: string): string { + return path.join(home, '.config', 'cloudcli-plugin-task-queue', 'token'); +} + +export type TokenResult = + | { ok: true; token: string } + | { ok: false; error: string }; + +interface TokenFs { + openSync(p: string, flags: number): number; + fstatSync(fd: number): { isFile(): boolean; mode: number }; + readFileSync(fd: number, enc: 'utf-8'): string; + closeSync(fd: number): void; + constants: { O_RDONLY: number; O_NOFOLLOW: number; O_NONBLOCK: number }; +} + +/** + * Read and check the token file. Never throws, and no error message contains any of the + * file's content: a file that fails a check may still hold a live token. + * + * Fails closed when the file is missing, is not a regular file, is empty, or carries ANY + * group or other permission bit. A token file others can read is not one this plugin + * should be quietly using; the fix is `chmod 600`, and the error says so. + * + * The checks and the read go through ONE file descriptor. Checking the path with stat() + * and then reading the path again would let the file be swapped between the two, so the + * bytes read would come from a file whose mode was never checked. O_NOFOLLOW refuses a + * symlink at the final component, and O_NONBLOCK keeps a FIFO planted at the path from + * hanging the open; fstat then rejects it as not a regular file. + */ +export function loadToken(file: string, fsImpl: TokenFs = fs): TokenResult { + const { O_RDONLY, O_NOFOLLOW, O_NONBLOCK } = fsImpl.constants; + let fd: number; + try { + fd = fsImpl.openSync(file, O_RDONLY | O_NOFOLLOW | O_NONBLOCK); + } catch (err) { + const code = (err as NodeJS.ErrnoException).code; + if (code === 'ENOENT') return { ok: false, error: `task-queue token file ${file} is missing` }; + if (code === 'ELOOP') return { ok: false, error: `task-queue token file ${file} is a symlink` }; + return { ok: false, error: `task-queue token file ${file} is unreadable (${code ?? 'error'})` }; + } + try { + const st = fsImpl.fstatSync(fd); + if (!st.isFile()) { + return { ok: false, error: `task-queue token file ${file} is not a regular file` }; + } + if ((st.mode & 0o077) !== 0) { + const mode = (st.mode & 0o777).toString(8).padStart(3, '0'); + return { + ok: false, + error: `task-queue token file ${file} is accessible to group or others (mode ${mode}); chmod 600 it`, + }; + } + let text: string; + try { + text = fsImpl.readFileSync(fd, 'utf-8'); + } catch (err) { + const code = (err as NodeJS.ErrnoException).code; + return { ok: false, error: `task-queue token file ${file} is unreadable (${code ?? 'error'})` }; + } + // Whitespace is trimmed so a trailing newline from `echo` or an editor does not become + // part of the token. A token is URL-safe base64 and never contains whitespace. + const token = text.trim(); + if (!token) { + return { ok: false, error: `task-queue token file ${file} is empty` }; + } + return { ok: true, token }; + } finally { + try { fsImpl.closeSync(fd); } catch { /* already closed */ } + } +} diff --git a/src/server.ts b/src/server.ts index f53d7a3..ec23c96 100644 --- a/src/server.ts +++ b/src/server.ts @@ -5,14 +5,23 @@ import os from 'node:os'; import { spawn } from 'node:child_process'; import { randomUUID } from 'node:crypto'; import { WebSocketServer, WebSocket } from 'ws'; -import { load as yamlLoad } from 'js-yaml'; -import { callControlApi, type ControlAction } from './control-api.ts'; +import { + callControlApi, + queueGet, + tasksQuery, + apiErrorMessage, + insecureApiBase, + LIST_PAGE_MAX, + type ControlAction, + type ControlApiOptions, +} from './control-api.ts'; +import { loadToken, tokenPath, type TokenResult } from './queue-token.ts'; import { evaluateUpgrade, allowedOrigins } from './ws-guard.ts'; import { resolveAllowedPath } from './path-guard.ts'; import type { DeadLetter, HeadlessRun, HeadlessRunDetail } from './types.ts'; import { toDeadLetter } from './dead-letters.ts'; import { isTerminal } from './vocabulary.ts'; -import { preLaunchEnv } from './launch-guards.ts'; +import { preLaunchEnv, launchRefusal } from './launch-guards.ts'; import { loadRunRecord, outcomeLabel, @@ -47,11 +56,9 @@ import { // ── Constants ────────────────────────────────────────────────────────── const HOME = process.env.HOME ?? os.homedir(); +// Watched as a change trigger only (see startWatcher). Nothing in this file parses the +// queue's YAML any more; every read goes through task-queue-mcp's read API. const TASK_QUEUE_DIR = path.join(HOME, '.claude', 'task-queue'); -// Written by task-dispatcher when a task exhausts its routing retries. Read-only here; -// the one mutation that touches it (requeue) goes through the MCP control API like every -// other mutation. See listDeadLetters. -const DEAD_LETTER_DIR = path.join(TASK_QUEUE_DIR, 'dead-letters'); const START_TIME = Date.now(); // Read from package.json rather than hardcoding. A hardcoded copy silently drifted @@ -69,11 +76,42 @@ const VERSION: string = (() => { const VALID_ID = /^[a-zA-Z0-9_-]+$/; -// MCP control API — the single validated, shared-secret-gated mutation path. All -// queue mutations (approve/cancel/status/park/unpark/amend/requeue) proxy here so -// they inherit the MCP core's transition validation + fcntl locking. Reads stay direct. +// task-queue-mcp's HTTP API — the single validated path for every queue read and write. +// Mutations inherit the MCP core's transition validation and fcntl locking; reads inherit +// its TTL, dead-letter and status rules. See control-api.ts. const TASK_QUEUE_API = (process.env.TASK_QUEUE_API ?? 'http://127.0.0.1:8485').replace(/\/$/, ''); -const TASK_QUEUE_API_SECRET = process.env.TASK_QUEUE_API_SECRET ?? ''; + +// This plugin's own client token, from a fixed file under HOME — never from the env, and +// the path is not configurable through the env either. See queue-token.ts for why. +// +// Loaded at startup and cached once it loads. A failed load is retried on the next +// request, so writing the file (or fixing its mode) takes effect without a CloudCLI +// restart; a token that has loaded is kept until the plugin restarts, which is what +// rotation needs anyway. +const TOKEN_FILE = tokenPath(HOME); +let tokenState: TokenResult | null = null; + +function queueToken(): TokenResult { + if (tokenState?.ok) return tokenState; + const next = loadToken(TOKEN_FILE); + // Log a failure once per distinct reason rather than on every request. + if (!next.ok && (!tokenState || tokenState.ok || tokenState.error !== next.error)) { + process.stderr.write(`[task-queue] ${next.error} — every queue read and write will fail\n`); + } + tokenState = next; + return next; +} + +function apiOpts(): ControlApiOptions { + return { apiBase: TASK_QUEUE_API, token: queueToken() }; +} + +/** A read the API refused or could not serve. Carries the status to return to the UI. */ +class QueueReadError extends Error { + constructor(readonly status: number, message: string) { + super(message); + } +} // Agent launch policy — ONE roster, shared with task-dispatcher.py. See // launch-policy.ts. The hardcoded AGENT_PROJECTS map that used to live here was a @@ -99,10 +137,10 @@ const LAUNCH_POLICY: { policy: LaunchPolicy | null; error: string | null } = (() } })(); -// ── Task operations (direct YAML file reader) ───────────────────────── +// ── Task reads (task-queue-mcp read API) ────────────────────────────── -// Deliberately looser than the `Task` in types.ts: this parses arbitrary YAML off disk, -// so every field is optional and unknown keys pass through to the client untouched. The UI +// Deliberately looser than the `Task` in types.ts: this is whatever the API returned, so +// every field is optional and unknown keys pass through to the client untouched. The UI // side gets the strict shape. Only the fields this file actually reads are named. interface Task { id?: string; @@ -111,95 +149,83 @@ interface Task { status?: string; summary?: string; created?: unknown; + queue_location?: string; /** Passed to the launcher on Start so a `manual-then-auto` task keeps its chain. */ workflow_mode?: string; payload?: Record; [key: string]: unknown; } -// Files are named TIMESTAMP-.yml; scan by id field value. -function findTaskFilePath(taskId: string): string | null { - if (!VALID_ID.test(taskId)) return null; - try { - if (!fs.existsSync(TASK_QUEUE_DIR)) return null; - const files = fs.readdirSync(TASK_QUEUE_DIR).filter(f => f.endsWith('.yml') && !f.endsWith('.tmp')); - for (const file of files) { - try { - const f = path.join(TASK_QUEUE_DIR, file); - const content = fs.readFileSync(f, 'utf-8'); - const task = yamlLoad(content) as Task; - if (task?.id === taskId) return f; - } catch { /* skip */ } - } - return null; - } catch { - return null; - } +interface TaskPage { + tasks: Task[]; + /** How many records matched, which may exceed tasks.length. */ + count: number; + /** True when the API's page limit cut records off. Rendered, never hidden. */ + truncated: boolean; } -function listTasks(filters: { target_agent?: string; status?: string; task_type?: string } = {}): Task[] { - try { - if (!fs.existsSync(TASK_QUEUE_DIR)) return []; - const files = fs.readdirSync(TASK_QUEUE_DIR).filter(f => f.endsWith('.yml') && !f.endsWith('.tmp')); - const tasks: Task[] = []; - for (const file of files) { - try { - const content = fs.readFileSync(path.join(TASK_QUEUE_DIR, file), 'utf-8'); - const task = yamlLoad(content) as Task; - if (!task?.id) continue; - if (filters.target_agent && task.target_agent !== filters.target_agent) continue; - if (filters.status && task.status !== filters.status) continue; - if (filters.task_type && task.task_type !== filters.task_type) continue; - tasks.push(task); - } catch { /* skip corrupt file */ } - } - return tasks; - } catch { - return []; +/** + * One page of tasks from GET /tasks, at the API's page ceiling. + * + * The API applies the queue's own rules: open work is listed however old it is, finished + * work ages out at its ttl_days, and dead letters are excluded unless asked for. Before + * v0.11.0 this function globbed the queue directory and applied none of them. + */ +async function listTasks( + filters: { target_agent?: string; status?: string; task_type?: string; include_dead_letters?: boolean } = {}, +): Promise { + const result = await queueGet(tasksQuery({ ...filters, limit: LIST_PAGE_MAX }), apiOpts()); + if (result.status !== 200) throw new QueueReadError(502, apiErrorMessage(result)); + const data = result.data as Partial; + const tasks = Array.isArray(data.tasks) ? data.tasks : []; + const page = { + tasks, + count: typeof data.count === 'number' ? data.count : tasks.length, + truncated: data.truncated === true, + }; + if (page.truncated) { + process.stderr.write( + `[task-queue] task list truncated: ${page.tasks.length} of ${page.count} matching records returned\n`, + ); } + return page; } -function getTask(taskId: string): Task | null { - const f = findTaskFilePath(taskId); - if (!f) return null; - try { - return yamlLoad(fs.readFileSync(f, 'utf-8')) as Task; - } catch { - return null; - } +/** GET /tasks/{id}. Null when no record has the id; throws when the API fails. */ +async function getTask(taskId: string): Promise { + if (!VALID_ID.test(taskId)) return null; + const result = await queueGet(`/tasks/${encodeURIComponent(taskId)}`, apiOpts()); + // Only a 404 means "no such task". The id was validated above, so a 400 from the API is + // a disagreement worth seeing, and is surfaced like any other API failure rather than + // rendered as "task not found". + if (result.status === 404) return null; + if (result.status !== 200) throw new QueueReadError(502, apiErrorMessage(result)); + return ((result.data as { task?: Task }).task) ?? null; } // ── Dead letters (read-only) ────────────────────────────────────────── /** - * Every record in dead-letters/, shaped for the UI. + * Every dead-lettered record, shaped for the UI, plus whether the read was truncated. + * + * Read through the API with include_dead_letters. Every dead letter carries status + * `failed`, so the query filters on it to keep the page small, and the API sorts dead + * letters first so a page limit cuts live `failed` records before it cuts a dead letter. + * Records are then kept by `queue_location`, never by status: a `failed` task in the live + * queue is not a dead letter and has no Requeue. * - * A missing directory is an empty list, not an error — the healthy state of this queue is - * zero, and a plugin that errored when nothing had failed would be broken most of the time. * Records with no `id` are skipped rather than rendered: a row we cannot address is a row * whose Requeue button could not work, and offering one would be a lie. - * - * This reads YAML directly, like every other read in this file. The single mutation on - * these records — requeue — still goes through the control API. */ -function listDeadLetters(): DeadLetter[] { - let names: string[]; - try { - names = fs.readdirSync(DEAD_LETTER_DIR); - } catch { - return []; +async function listDeadLetters(): Promise<{ deadLetters: DeadLetter[]; truncated: boolean }> { + const page = await listTasks({ status: 'failed', include_dead_letters: true }); + const deadLetters: DeadLetter[] = []; + for (const t of page.tasks) { + if (t.queue_location !== 'dead-letters') continue; + const dl = toDeadLetter(t as Record); + if (dl) deadLetters.push(dl); } - - const letters: DeadLetter[] = []; - for (const name of names) { - if (!name.endsWith('.yml') || name.endsWith('.tmp')) continue; - try { - const raw = yamlLoad(fs.readFileSync(path.join(DEAD_LETTER_DIR, name), 'utf-8')); - const dl = toDeadLetter((raw ?? {}) as Record); - if (dl) letters.push(dl); - } catch { /* skip corrupt file */ } - } - return letters; + return { deadLetters, truncated: page.truncated }; } // ── Context ref preview ─────────────────────────────────────────────── @@ -336,7 +362,7 @@ async function recordStartInQueue( status: current, allow_override: true, note: `Session launched from CloudCLI in ${mode} mode`, - }, { apiBase: TASK_QUEUE_API, secret: TASK_QUEUE_API_SECRET }); + }, apiOpts()); if (status !== 200) { console.error( `[task-queue] launched ${taskId} but could not record it in the task's history: ` @@ -517,10 +543,18 @@ const HEADLESS_MAX_BYTES = 512 * 1024; * * A prefix collision resolves to `unknown` rather than to an arbitrary one of the two. */ -function queueIndexByPrefix(): Map { +async function queueIndexByPrefix(): Promise> { const idx = new Map(); const collided = new Set(); - for (const t of listTasks()) { + // A failed read leaves every run's status `unknown`, which is what the view renders for + // a run it cannot match. It must not fail the whole runs list. + let tasks: Task[] = []; + try { + tasks = (await listTasks()).tasks; + } catch (err) { + process.stderr.write(`[task-queue] headless runs: queue status unavailable: ${(err as Error).message}\n`); + } + for (const t of tasks) { const id = typeof t.id === 'string' ? t.id : ''; if (!id) continue; const key = id.slice(0, 8); @@ -583,8 +617,8 @@ function launchStems(): Map { return stems; } -function listHeadlessRuns(agentFilter?: string): HeadlessRun[] { - const queue = queueIndexByPrefix(); +async function listHeadlessRuns(agentFilter?: string): Promise { + const queue = await queueIndexByPrefix(); const runs: HeadlessRun[] = []; for (const parsed of launchStems().values()) { @@ -627,7 +661,7 @@ function listHeadlessRuns(agentFilter?: string): HeadlessRun[] { return runs; } -function readHeadlessRun(id: string): HeadlessRunDetail | null { +async function readHeadlessRun(id: string): Promise { // `id` is validated as - and the filename is then REBUILT via // launchLogName, so the caller's string never reaches the filesystem as a path even // before the realpath guard runs. Two independent barriers, deliberately. @@ -671,7 +705,7 @@ function readHeadlessRun(id: string): HeadlessRunDetail | null { // reader of all of them. if (!logReadable && !record) return null; - const match = queueIndexByPrefix().get(parsed.taskId8); + const match = (await queueIndexByPrefix()).get(parsed.taskId8); return { id: runId(parsed.agent, parsed.taskId8), agent: parsed.agent, @@ -711,11 +745,10 @@ function startWatcher(broadcast: (msg: object) => void): void { if (watchDebounce) clearTimeout(watchDebounce); watchDebounce = setTimeout(() => { watchDebounce = null; - // Count current tasks - try { - const files = fs.readdirSync(TASK_QUEUE_DIR).filter(f => f.endsWith('.yml') && !f.endsWith('.tmp')); - broadcast({ type: 'tasks', count: files.length, changed: filename }); - } catch { /* skip */ } + // A change trigger and nothing more: the UI re-reads through the API on this event. + // It used to carry a file count, which the UI never read, and which was one more + // reader of the queue directory with its own idea of what counts as a task. + broadcast({ type: 'tasks', changed: filename }); }, 1000); }); } @@ -767,15 +800,14 @@ const server = http.createServer(async (req, res) => { if (status) filters.status = status; if (taskType) filters.task_type = taskType; - const tasks = listTasks(filters); - res.end(JSON.stringify({ tasks })); + res.end(JSON.stringify(await listTasks(filters))); return; } // Get task detail const taskMatch = pathname.match(/^\/tasks\/([a-zA-Z0-9_-]+)$/); if (taskMatch && req.method === 'GET') { - const task = getTask(taskMatch[1]); + const task = await getTask(taskMatch[1]); if (!task) { res.statusCode = 404; res.end(JSON.stringify({ error: 'not found' })); return; } // Get context ref previews const previews: Record = {}; @@ -790,14 +822,14 @@ const server = http.createServer(async (req, res) => { // List dead letters. Read-only; grouping happens in the panel via groupByReason. if (pathname === '/dead-letters' && req.method === 'GET') { - res.end(JSON.stringify({ deadLetters: listDeadLetters() })); + res.end(JSON.stringify(await listDeadLetters())); return; } // List headless runs. Read-only. if (pathname === '/headless-runs' && req.method === 'GET') { const agent = url.searchParams.get('agent'); - res.end(JSON.stringify({ runs: listHeadlessRuns(agent ?? undefined) })); + res.end(JSON.stringify({ runs: await listHeadlessRuns(agent ?? undefined) })); return; } @@ -805,7 +837,7 @@ const server = http.createServer(async (req, res) => { // character class as /tasks/:id and is never treated as a path — see readHeadlessRun. const runMatch = pathname.match(/^\/headless-runs\/([a-zA-Z0-9_-]+)$/); if (runMatch && req.method === 'GET') { - const run = readHeadlessRun(runMatch[1]); + const run = await readHeadlessRun(runMatch[1]); if (!run) { res.statusCode = 404; res.end(JSON.stringify({ error: 'not found' })); return; } res.end(JSON.stringify(run)); return; @@ -834,8 +866,14 @@ const server = http.createServer(async (req, res) => { })); return; } - const taskData = getTask(startMatch[1]); + const taskData = await getTask(startMatch[1]); if (!taskData) { res.statusCode = 404; res.end(JSON.stringify({ error: 'task not found' })); return; } + const refusal = launchRefusal(taskData); + if (refusal) { + res.statusCode = 409; + res.end(JSON.stringify({ ok: false, error: `not launching: ${refusal}` })); + return; + } const result = launchSession( startMatch[1], taskData.target_agent ?? '', @@ -848,8 +886,8 @@ const server = http.createServer(async (req, res) => { return; } - // Queue mutations — all proxied to the MCP control API (the single validated, - // shared-secret-gated write path). No direct YAML mutation happens in the plugin. + // Queue mutations — all proxied to the MCP control API (the single validated write + // path, under this plugin's own token). No direct YAML mutation happens in the plugin. const mutationMatch = pathname.match(/^\/tasks\/([a-zA-Z0-9_-]+)\/(approve|cancel|status|park|unpark|amend|requeue)$/); if (mutationMatch && req.method === 'POST') { const mTaskId = mutationMatch[1]; @@ -878,10 +916,7 @@ const server = http.createServer(async (req, res) => { } } - const { status: apiStatus, data } = await callControlApi(mTaskId, action, body, { - apiBase: TASK_QUEUE_API, - secret: TASK_QUEUE_API_SECRET, - }); + const { status: apiStatus, data } = await callControlApi(mTaskId, action, body, apiOpts()); res.statusCode = apiStatus; res.end(JSON.stringify(data)); return; @@ -892,6 +927,13 @@ const server = http.createServer(async (req, res) => { res.end(JSON.stringify({ error: 'not found' })); } catch (err) { + if (err instanceof QueueReadError) { + // The API's own message (it names a missing token file, a refused token or an + // unreachable server) is what the operator needs to see. It never carries a token. + res.statusCode = err.status; + res.end(JSON.stringify({ error: err.message })); + return; + } process.stderr.write(`[task-queue] ${(err as Error).message}\n`); res.statusCode = 500; res.end(JSON.stringify({ error: 'internal server error' })); @@ -945,5 +987,10 @@ server.on('upgrade', (req, socket, head) => { server.listen(0, '127.0.0.1', () => { const addr = server.address() as { port: number }; console.log(JSON.stringify({ ready: true, port: addr.port })); + // Report a refused TASK_QUEUE_API at boot too; every request would fail on it anyway. + const insecure = insecureApiBase(TASK_QUEUE_API); + if (insecure) process.stderr.write(`[task-queue] ${insecure} — every queue read and write will fail\n`); + // Load (and, if broken, report) the token at boot rather than on the first click. + if (queueToken().ok) process.stderr.write(`[task-queue] client token loaded from ${TOKEN_FILE}\n`); startWatcher(broadcast); }); diff --git a/src/tests/control-api.test.ts b/src/tests/control-api.test.ts index 94ca752..e9b52d1 100644 --- a/src/tests/control-api.test.ts +++ b/src/tests/control-api.test.ts @@ -2,6 +2,10 @@ import assert from 'node:assert/strict'; import test from 'node:test'; import { callControlApi } from '../control-api.ts'; +import type { TokenResult } from '../queue-token.ts'; + +const GOOD: TokenResult = { ok: true, token: 'synthetic-not-a-real-token' }; +const MISSING: TokenResult = { ok: false, error: 'task-queue token file /home/x/.config/cloudcli-plugin-task-queue/token is missing' }; // A fetch spy: records calls and returns a canned Response-like object. function spyFetch(status = 200, json: unknown = { ok: true }) { @@ -13,24 +17,24 @@ function spyFetch(status = 200, json: unknown = { ok: true }) { return { impl, calls }; } -test('missing secret returns 500 and never attempts fetch', async () => { +test('a missing token returns 500 naming the file, and never attempts fetch', async () => { const fetchSpy = spyFetch(); const result = await callControlApi('task-abc', 'approve', {}, { apiBase: 'http://127.0.0.1:8485', - secret: '', + token: MISSING, fetchImpl: fetchSpy.impl, }); assert.equal(result.status, 500); - assert.deepEqual(result.data, { ok: false, error: 'TASK_QUEUE_API_SECRET not configured' }); - assert.equal(fetchSpy.calls.length, 0, 'fetch must not be called when the secret is missing'); + assert.deepEqual(result.data, { ok: false, error: MISSING.ok ? '' : MISSING.error }); + assert.equal(fetchSpy.calls.length, 0, 'fetch must not be called without a token'); }); test('invalid task id returns 400 and never attempts fetch', async () => { const fetchSpy = spyFetch(); const result = await callControlApi('bad id!', 'approve', {}, { apiBase: 'http://127.0.0.1:8485', - secret: 'synthetic-not-a-real-secret', + token: GOOD, fetchImpl: fetchSpy.impl, }); @@ -38,11 +42,11 @@ test('invalid task id returns 400 and never attempts fetch', async () => { assert.equal(fetchSpy.calls.length, 0); }); -test('a configured secret is sent as X-Task-Queue-Secret and status passes through', async () => { +test('the token is sent as X-Task-Queue-Token, never Authorization, and status passes through', async () => { const fetchSpy = spyFetch(200, { ok: true, status: 'approved' }); const result = await callControlApi('task-abc', 'approve', {}, { apiBase: 'http://127.0.0.1:8485', - secret: 'synthetic-not-a-real-secret', + token: GOOD, fetchImpl: fetchSpy.impl, }); @@ -52,7 +56,9 @@ test('a configured secret is sent as X-Task-Queue-Secret and status passes throu assert.equal(url, 'http://127.0.0.1:8485/tasks/task-abc/approve'); assert.equal(init.method, 'POST'); const headers = init.headers as Record; - assert.equal(headers['X-Task-Queue-Secret'], 'synthetic-not-a-real-secret'); + assert.equal(headers['X-Task-Queue-Token'], 'synthetic-not-a-real-token'); + assert.equal(headers['X-Task-Queue-Secret'], undefined, 'the retired shared-secret header must not be sent'); + assert.equal(headers.Authorization, undefined, 'a client token must never be offered as a bearer'); // actor defaults to operator; caller body is merged in assert.deepEqual(JSON.parse(init.body as string), { actor: 'operator' }); }); @@ -61,7 +67,7 @@ test('a transport failure is mapped to 502', async () => { const failingFetch = (async () => { throw new Error('ECONNREFUSED'); }) as unknown as typeof fetch; const result = await callControlApi('task-abc', 'cancel', { note: 'nope' }, { apiBase: 'http://127.0.0.1:8485', - secret: 'synthetic-not-a-real-secret', + token: GOOD, fetchImpl: failingFetch, }); @@ -74,7 +80,7 @@ test('park and unpark route to their own control-API paths', async () => { const fetchSpy = spyFetch(); const result = await callControlApi('task-abc', action, { note: 'via CloudCLI' }, { apiBase: 'http://127.0.0.1:8485', - secret: 'synthetic-not-a-real-secret', + token: GOOD, fetchImpl: fetchSpy.impl, }); @@ -91,7 +97,7 @@ test('unpark can carry an explicit target status', async () => { const fetchSpy = spyFetch(); await callControlApi('task-abc', 'unpark', { status: 'approved' }, { apiBase: 'http://127.0.0.1:8485', - secret: 'synthetic-not-a-real-secret', + token: GOOD, fetchImpl: fetchSpy.impl, }); @@ -109,7 +115,7 @@ test('amend sends the amendment text and defaults the actor to operator', async { amendment: 'scope narrowed', reason: 'Amended via CloudCLI' }, { apiBase: 'http://127.0.0.1:8485', - secret: 'synthetic-not-a-real-secret', + token: GOOD, fetchImpl: fetchSpy.impl, }, ); @@ -131,7 +137,7 @@ test('an authorization rejection from the control API passes through unmodified' const fetchSpy = spyFetch(400, { ok: false, error: "actor 'developer' may not amend this task" }); const result = await callControlApi('task-abc', 'amend', { amendment: 'x' }, { apiBase: 'http://127.0.0.1:8485', - secret: 'synthetic-not-a-real-secret', + token: GOOD, fetchImpl: fetchSpy.impl, }); @@ -145,7 +151,7 @@ test('requeue posts to the requeue route as operator', async () => { const fetchSpy = spyFetch(200, { ok: true, task_id: 'x', requeued_from: 'dead-letters' }); const result = await callControlApi('task-abc', 'requeue', { note: 'Requeued via CloudCLI' }, { apiBase: 'http://127.0.0.1:8485', - secret: 'synthetic-not-a-real-secret', + token: GOOD, fetchImpl: fetchSpy.impl, }); @@ -157,13 +163,13 @@ test('requeue posts to the requeue route as operator', async () => { }); }); -test('requeue fails closed without the secret, like every other mutation', async () => { +test('requeue fails closed without a token, like every other mutation', async () => { // Requeue puts work back in front of an agent. It must not be the one action that // slipped past the gate. const fetchSpy = spyFetch(); const result = await callControlApi('task-abc', 'requeue', {}, { apiBase: 'http://127.0.0.1:8485', - secret: '', + token: MISSING, fetchImpl: fetchSpy.impl, }); @@ -177,7 +183,7 @@ test("the MCP's 404 for a non-dead-lettered task passes through unchanged", asyn const fetchSpy = spyFetch(404, { ok: false, error: 'not found' }); const result = await callControlApi('task-abc', 'requeue', {}, { apiBase: 'http://127.0.0.1:8485', - secret: 'synthetic-not-a-real-secret', + token: GOOD, fetchImpl: fetchSpy.impl, }); @@ -213,3 +219,101 @@ test('the ControlAction union and the server route regex name the same actions', assert.deepEqual(routeActions, unionActions); assert.ok(unionActions.includes('requeue'), 'the fixture itself must be non-trivial'); }); + +// ── reads (v0.11.0) ─────────────────────────────────────────────────── + +import { queueGet, tasksQuery, apiErrorMessage, LIST_PAGE_MAX } from '../control-api.ts'; + +test('queueGet sends the token as X-Task-Queue-Token on a GET', async () => { + const fetchSpy = spyFetch(200, { ok: true, tasks: [], count: 0, truncated: false }); + const result = await queueGet('/tasks?limit=5', { + apiBase: 'http://127.0.0.1:8485', + token: GOOD, + fetchImpl: fetchSpy.impl, + }); + assert.equal(result.status, 200); + const { url, init } = fetchSpy.calls[0]; + assert.equal(url, 'http://127.0.0.1:8485/tasks?limit=5'); + assert.equal(init.method, 'GET'); + const headers = init.headers as Record; + assert.equal(headers['X-Task-Queue-Token'], 'synthetic-not-a-real-token'); + assert.equal(headers.Authorization, undefined); +}); + +test('queueGet without a token fails closed and never fetches', async () => { + const fetchSpy = spyFetch(); + const result = await queueGet('/tasks', { + apiBase: 'http://127.0.0.1:8485', + token: MISSING, + fetchImpl: fetchSpy.impl, + }); + assert.equal(result.status, 500); + assert.equal(fetchSpy.calls.length, 0); +}); + +test('a refused read passes its status and error through', async () => { + const fetchSpy = spyFetch(403, { ok: false, error: 'scope read required' }); + const result = await queueGet('/tasks', { + apiBase: 'http://127.0.0.1:8485', + token: GOOD, + fetchImpl: fetchSpy.impl, + }); + assert.equal(result.status, 403); + assert.equal(apiErrorMessage(result), 'task-queue API 403: scope read required'); +}); + +test('tasksQuery drops empty filters and encodes the rest', () => { + assert.equal(tasksQuery({}), '/tasks'); + assert.equal( + tasksQuery({ target_agent: 'developer', status: '', task_type: undefined, limit: LIST_PAGE_MAX }), + '/tasks?target_agent=developer&limit=1000', + ); + assert.equal(tasksQuery({ status: 'a b&c' }), '/tasks?status=a+b%26c'); + assert.equal(tasksQuery({ include_dead_letters: true }), '/tasks?include_dead_letters=true'); +}); + +// ── the manifest grants no credential (vikunja#396) ─────────────────── + +test('the manifest requests no task-queue credential from the host env', async () => { + // A credential granted through the manifest has to be in the CloudCLI host's own + // environment, and every agent session CloudCLI launches inherits that environment. + // That is how the shared secret reached every session. The token is read from a file. + const fs = await import('node:fs'); + const manifest = JSON.parse( + fs.readFileSync(new URL('../../manifest.json', import.meta.url), 'utf-8'), + ) as { permissions?: string[] }; + const env = (manifest.permissions ?? []).filter(p => p.startsWith('env:')); + assert.deepEqual(env.sort(), ['env:CLOUDCLI_ORIGIN', 'env:TASK_QUEUE_API']); +}); + +// ── transport guard (CodeRabbit on the bot's twin PR) ───────────────── + +import { insecureApiBase } from '../control-api.ts'; + +test('loopback http and any https are accepted', () => { + for (const base of [ + 'http://127.0.0.1:8485', 'http://127.0.0.2:8485', 'http://localhost:8485', + 'http://[::1]:8485', 'https://tasks.example.com', 'https://10.0.0.5:8485', + ]) { + assert.equal(insecureApiBase(base), null, base); + } +}); + +test('cleartext to another host, and non-URLs, are refused', () => { + for (const base of [ + 'http://10.0.0.5:8485', 'http://tasks.example.com', 'http://127.0.0.1.evil.example', + 'ftp://127.0.0.1', '127.0.0.1:8485', '', + ]) { + assert.notEqual(insecureApiBase(base), null, base); + } +}); + +test('an insecure base never sends the token, on reads or writes', async () => { + const fetchSpy = spyFetch(); + const opts = { apiBase: 'http://10.0.0.5:8485', token: GOOD, fetchImpl: fetchSpy.impl }; + const read = await queueGet('/tasks', opts); + const write = await callControlApi('task-abc', 'approve', {}, opts); + assert.equal(read.status, 500); + assert.equal(write.status, 500); + assert.equal(fetchSpy.calls.length, 0); +}); diff --git a/src/tests/launch-guards.test.ts b/src/tests/launch-guards.test.ts index dbe2f94..ae22733 100644 --- a/src/tests/launch-guards.test.ts +++ b/src/tests/launch-guards.test.ts @@ -239,3 +239,26 @@ test('an expired OAuth blocks a direct launch even with a bearer token', () => { assert.equal(preLaunchEnv('developer', null, {}, { envRoot: root, oauthPath: f }).ok, false); }); + +// ── launchRefusal: Start launches only live, unfinished work ────────── + +import { launchRefusal } from '../launch-guards.ts'; + +test('Start refuses archived and dead-lettered records', () => { + assert.match(launchRefusal({ queue_location: 'archive', status: 'approved' }) ?? '', /archive/); + assert.match(launchRefusal({ queue_location: 'dead-letters', status: 'failed' }) ?? '', /dead-letters/); +}); + +test('Start refuses a terminal task in the live queue', () => { + for (const status of ['completed', 'failed', 'cancelled']) { + assert.match(launchRefusal({ queue_location: 'queue', status }) ?? '', new RegExp(status)); + } +}); + +test('Start allows live, unfinished work', () => { + for (const status of ['submitted', 'approved', 'in-progress', 'parked', 'routing-failed']) { + assert.equal(launchRefusal({ queue_location: 'queue', status }), null, status); + } + // A record with no location predates queue_location and was read from the live queue. + assert.equal(launchRefusal({ status: 'approved' }), null); +}); diff --git a/src/tests/queue-token.test.ts b/src/tests/queue-token.test.ts new file mode 100644 index 0000000..60ba35c --- /dev/null +++ b/src/tests/queue-token.test.ts @@ -0,0 +1,92 @@ +import assert from 'node:assert/strict'; +import { execFileSync } from 'node:child_process'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import test from 'node:test'; + +import { loadToken, tokenPath } from '../queue-token.ts'; + +// Real files in a tmpdir, not a mocked fs: the checks under test are the kernel's answers +// to stat(), and a mock can only restate what the test already assumes. + +const SECRET_SHAPED = 'synthetic-token-value-0123456789abcdef'; + +function tmp(): string { + return fs.mkdtempSync(path.join(os.tmpdir(), 'tq-token-')); +} + +function write(dir: string, content: string, mode = 0o600): string { + const f = path.join(dir, 'token'); + fs.writeFileSync(f, content, { mode }); + fs.chmodSync(f, mode); // writeFileSync's mode is masked by umask; set it exactly + return f; +} + +test('the token path is fixed under HOME', () => { + assert.equal(tokenPath('/home/x'), '/home/x/.config/cloudcli-plugin-task-queue/token'); +}); + +test('a 0600 file with a token loads, trailing newline trimmed', () => { + const f = write(tmp(), `${SECRET_SHAPED}\n`); + assert.deepEqual(loadToken(f), { ok: true, token: SECRET_SHAPED }); +}); + +test('a missing file fails closed and names the path', () => { + const f = path.join(tmp(), 'token'); + const r = loadToken(f); + assert.equal(r.ok, false); + assert.match(r.ok ? '' : r.error, /is missing/); + assert.ok(!r.ok && r.error.includes(f)); +}); + +for (const content of ['', '\n', ' \n\t']) { + test(`an empty file (${JSON.stringify(content)}) fails closed`, () => { + const r = loadToken(write(tmp(), content)); + assert.equal(r.ok, false); + assert.match(r.ok ? '' : r.error, /is empty/); + }); +} + +for (const mode of [0o640, 0o604, 0o644, 0o660, 0o610]) { + test(`mode ${mode.toString(8)} fails closed without revealing the content`, () => { + const r = loadToken(write(tmp(), SECRET_SHAPED, mode)); + assert.equal(r.ok, false); + const error = r.ok ? '' : r.error; + assert.match(error, /group or others/); + assert.match(error, /chmod 600/); + assert.ok(!error.includes(SECRET_SHAPED), 'the error must not carry the token'); + }); +} + +test('mode 0400 is accepted', () => { + assert.equal(loadToken(write(tmp(), SECRET_SHAPED, 0o400)).ok, true); +}); + +test('a directory at the path fails closed', () => { + const d = tmp(); + const f = path.join(d, 'token'); + fs.mkdirSync(f, { mode: 0o700 }); + const r = loadToken(f); + assert.equal(r.ok, false); + assert.match(r.ok ? '' : r.error, /not a regular file/); +}); + +test('a symlink at the token path fails closed, even to a 0600 file', () => { + const d = tmp(); + const real = write(d, SECRET_SHAPED); // 0600, would pass every other check + const link = path.join(d, 'link'); + fs.symlinkSync(real, link); + const r = loadToken(link); + assert.equal(r.ok, false); + assert.match(r.ok ? '' : r.error, /symlink/); +}); + +test('a FIFO at the token path fails closed without hanging', () => { + const d = tmp(); + const f = path.join(d, 'token'); + execFileSync('mkfifo', ['-m', '600', f]); + const r = loadToken(f); + assert.equal(r.ok, false); + assert.match(r.ok ? '' : r.error, /not a regular file/); +});