From 8d2763e3544c24e283990e6908f3a64735cd18c2 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Fri, 4 Sep 2026 10:32:23 +0700 Subject: [PATCH] Add authorization entry-point audit --- AUDIT.md | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) create mode 100644 AUDIT.md diff --git a/AUDIT.md b/AUDIT.md new file mode 100644 index 0000000..43886de --- /dev/null +++ b/AUDIT.md @@ -0,0 +1,76 @@ +# Authorization entry-point audit + +Scope reviewed: every route registered in `FleetApp.build`, every tool handler +registered in `FleetMcp`, and their shared `CallerResolver` and `Authz` gate. + +## Result + +No authorization-action mismatch was found. The only handler whose operation +changes with its arguments is `fleet_poll`. It selects `DRAIN` when `target` is +present and `READ` when it is absent, before it calls either service branch. + +`Fleetd` creates one `CallerResolver` and passes that same instance to both +`FleetMcp` and `FleetApp` (`Fleetd.java:615-650`, `720-722`). REST resolves it +in the Javalin pre-handler. MCP resolves it in the transport context extractor. + +## REST routes + +| Route | Gate and choice location | Branch/target review | Verdict | +|---|---|---|---| +| `GET /healthz` | None | Liveness probe only; intentionally open. | ok | +| `GET /metrics` | `METRICS`, start of `metrics` | No branch or target. | ok | +| `GET /sessions` | `READ`, start of `sessions` | No caller-selected target. | ok | +| `GET /agents` | `READ`, start of `agents` | No caller-selected target. | ok | +| `GET /members` | `READ`, start of `listMembers` | No caller-selected target. | ok | +| `GET /profiles` | `READ`, start of `profiles` | No caller-selected target. | ok | +| `GET /member-credentials` | `READ`, start of `memberCredentials` | No caller-selected target. The view exposes policy names and counts, not values. | ok | +| `POST /members` | `SPAWN`, before query/body parsing in `spawnMember` | Arguments select role, profile, cwd, and worktree. They do not select a different authority type. | ok | +| `DELETE /members/{paneId}` | `STOP`, after reading `paneId` in `stopMember` | Caller can select another pane, but only a primary has `STOP`. | ok | +| `POST /sessions/{id}/message` | `SEND`, after reading path `id` in `sendMessage` | Normal send, async send, and `turnId` answer all deliver a turn/message. `turnId` does not widen the roles allowed to send. | ok | +| `POST /sessions/{id}/reply` | `REPLY`, after reading path `id` in `replyMessage` | Caller can name a target, and `Authz` requires it to equal the connection-resolved terminal. | ok | +| `GET /sessions/{id}/replies` | `DRAIN`, after reading path `id` in `drainReplies` | Removes inbox entries; only a primary has `DRAIN`. | ok | +| `POST /sessions/{id}/ask` | `ASK`, after reading path `id` in `askMessage` | Caller can name a target, and `Authz` requires it to equal the connection-resolved terminal. | ok | +| `GET /sessions/{id}/status` | `READ`, after reading path `id` in `sessionStatus` | Any authenticated role may observe any session. This matches the `READ` policy, which intentionally does not use target ownership. | ok | +| `GET /tasks/{ticket}` | `READ`, start of `taskStatus` | Ticket polling is read-only; no service branch changes the action. | ok | + +## MCP tools + +| Tool | Gate and choice location | Branch/target review | Verdict | +|---|---|---|---| +| `fleet_send` | `SEND`, at the start of `sendHandler` | `coordId`, `turnId`, synchronous, and async forms all deliver a message or a turn answer. `sessionId` is read before the branch for audit target only. | ok | +| `fleet_reply` | `REPLY`, after deriving the connection terminal in `replyHandler` | No target argument exists. The service always receives the caller's own terminal. | ok | +| `fleet_ask` | `ASK`, after deriving the connection terminal in `askHandler` | No target argument exists. The service always receives the caller's own terminal. | ok | +| `fleet_status` | `READ`, start of `statusHandler` | Any authenticated role may query any session. This is the same intentional `READ` policy as the REST route. | ok | +| `fleet_poll` with `ticket` | `READ`, `pollAction(target)` before dispatch in `pollHandler` | Reads a task only. | ok | +| `fleet_poll` with `target` | `DRAIN`, `pollAction(target)` before dispatch in `pollHandler` | Drains and removes a target inbox. Only a primary has `DRAIN`. | ok | +| `fleet_ack` | `DRAIN`, start of `ackHandler` | Removes one target inbox entry. Only a primary has `DRAIN`. | ok | +| `fleet_spawn` | `SPAWN`, start of `spawnHandler` | Arguments select member configuration only. | ok | +| `fleet_list` | `READ`, start of `listHandler` | No caller-selected target. | ok | +| `fleet_stop` | `STOP`, after reading `paneId` in `stopHandler` | Caller can select another pane, but only a primary has `STOP`. | ok | +| `fleet_profiles` | `READ`, start of `profilesHandler` | No caller-selected target. | ok | +| `fleet_whoami` | `READ`, start of `whoamiHandler` | Reports the connection-resolved caller, not an argument. | ok | + +## Surface parity + +The matching route/tool pairs use the same action: + +| Operation | REST | MCP | Result | +|---|---|---|---| +| spawn | `SPAWN` | `SPAWN` | match | +| stop | `STOP` | `STOP` | match | +| send and ask answer | `SEND` | `SEND` | match | +| reply | `REPLY` | `REPLY` | match | +| ask | `ASK` | `ASK` | match | +| drain replies | `DRAIN` | `DRAIN` for `fleet_poll{target}` and `fleet_ack` | match | +| status | `READ` | `READ` | match | +| task/ticket polling | `READ` | `READ` | match | +| list/profiles | `READ` | `READ` | match | + +## Test risk + +`FleetMcpAuthzTest` has a direct regression test for the argument-dependent +`fleet_poll` action. It tests the shared role table for the other tools, but it +does not pin each handler's chosen action. This is not a current defect because +the reviewed handlers choose the matching action. A future change that adds an +argument-dependent operation should add a handler-level action-selection test, +like `pollingByTargetIsADrainAndPollingByTicketIsARead`.