Gate POST /api/chat on the same step-up as /tools (#317) #51

2026-07-31T23:24:58+02:00

claude commented

2026-07-31 23:24:58 +02:00

What

POST /api/chat now goes through stepUpOK, the same gate as POST /tools, POST /routines and POST /api/revert.
The route table in cmd/mavweb/main.go enumerates every state-changing route and its gate.
Both startup SECURITY log lines (and the -require-stepup help) now name /routines and /api/chat, not just /tools and /api/revert.
Four tests in cmd/mavweb/handlers_test.go cover the gate: fail-closed under -require-stepup, denied on an unasserted session, allowed on an asserted one, and fail-open in the default deploy.

Why

/api/chat was added by PR #47 and inherited no gate at all, while being the widest surface on the server: the text reaches the router, the LLM and the whole action path including act. The destructive-act confirm gate was the only thing in the way.

Not in this PR

The loopback -addr default the task also asked for already landed in d12de58, and docker-compose.yml already publishes mavweb on 127.0.0.1:9201 only — nothing to change. Rotating the Kuma key (item 4) needs a credential only Kami can mint; -kuma-key uk5_mavpoll-key in docker-compose.yml is still the placeholder.

Verified

make build and make test (go test -race) both exit 0.

Vikunja #317

## What - `POST /api/chat` now goes through `stepUpOK`, the same gate as `POST /tools`, `POST /routines` and `POST /api/revert`. - The route table in `cmd/mavweb/main.go` enumerates every state-changing route and its gate. - Both startup SECURITY log lines (and the `-require-stepup` help) now name `/routines` and `/api/chat`, not just `/tools` and `/api/revert`. - Four tests in `cmd/mavweb/handlers_test.go` cover the gate: fail-closed under `-require-stepup`, denied on an unasserted session, allowed on an asserted one, and fail-open in the default deploy. ## Why `/api/chat` was added by PR #47 and inherited no gate at all, while being the widest surface on the server: the text reaches the router, the LLM and the whole action path including `act`. The destructive-act confirm gate was the only thing in the way. ## Not in this PR The loopback `-addr` default the task also asked for already landed in d12de58, and `docker-compose.yml` already publishes mavweb on `127.0.0.1:9201` only — nothing to change. Rotating the Kuma key (item 4) needs a credential only Kami can mint; `-kuma-key uk5_mavpoll-key` in `docker-compose.yml` is still the placeholder. ## Verified `make build` and `make test` (go test -race) both exit 0. Vikunja #317

claude added 1 commit 2026-07-31 23:24:58 +02:00

Gate POST /api/chat on the same step-up as /tools (#317 ) d29e7ba813

/api/chat reaches the router, the LLM and, through applyAction, the whole
act path, so it is the widest state-changing surface mavweb serves. It was
the only one with no gate. It now goes through stepUpOK like POST /tools,
POST /routines and POST /api/revert: unchanged in the default deploy
(WebAuthn unconfigured, fail-open behind wg+nginx), 403 under
-require-stepup or an unasserted passkey session.

The route table now carries an explicit enumeration of every state-changing
route and its gate, and the two startup SECURITY log lines name /routines
and /api/chat alongside /tools and /api/revert.

The loopback -addr default the task also asked for landed earlier in
d12de58; the compose already publishes mavweb on 127.0.0.1 only.

claude reviewed 2026-08-01 11:21:27 +02:00

claude left a comment

The gate itself is right. The four tests cover the shape that matters: fail-closed under -require-stepup, denied on an unasserted session, passing on an asserted one, fail-open by default. The route table comment in main.go is the most useful thing in the diff. It makes the next route's gate a decision rather than an oversight.

One gap, and it is the one that table exposes.

POST /api/ptt and /ws are listed as ungated, justified by "proxy audio to mavend's voice port, which is itself only reachable inside the deploy". But mavweb is the thing proxying into it from outside, so the reachability argument does not hold for these two routes. Audio posted to /api/ptt lands in HandlePushToTalk. That calls runTurn. That is the same router, the same LLM and the same applyAction the diff just gated on the text side. Speaking "выключи свет" is not a smaller act than typing it.

So the two justifications conflict. Either the nginx ACL in front of mavweb suffices for the act path, and /api/chat did not need the gate. Or it does not, and /api/ptt and /ws need one too. My read is the second: gate them, and say so in the table.

The awkward part is that a step-up gate on /ws breaks the hands-free case, which is the whole point of push-to-talk. A session-scoped assertion covering a run of turns, rather than a per-request one, is probably what this wants. Worth its own task rather than an edit here.

Smaller notes:

The SECURITY log lines now run past 400 characters each. The content is right. One surface per line would make them readable in a terminal at the moment someone reads them.
handleChatPage reflects q and r from the query string. html/template escapes both, so this is fine and needs no change.

The gate itself is right. The four tests cover the shape that matters: fail-closed under `-require-stepup`, denied on an unasserted session, passing on an asserted one, fail-open by default. The route table comment in `main.go` is the most useful thing in the diff. It makes the next route's gate a decision rather than an oversight. One gap, and it is the one that table exposes. `POST /api/ptt` and `/ws` are listed as ungated, justified by "proxy audio to mavend's voice port, which is itself only reachable inside the deploy". But mavweb is the thing proxying into it from outside, so the reachability argument does not hold for these two routes. Audio posted to `/api/ptt` lands in `HandlePushToTalk`. That calls `runTurn`. That is the same router, the same LLM and the same `applyAction` the diff just gated on the text side. Speaking "выключи свет" is not a smaller act than typing it. So the two justifications conflict. Either the nginx ACL in front of mavweb suffices for the act path, and `/api/chat` did not need the gate. Or it does not, and `/api/ptt` and `/ws` need one too. My read is the second: gate them, and say so in the table. The awkward part is that a step-up gate on `/ws` breaks the hands-free case, which is the whole point of push-to-talk. A session-scoped assertion covering a run of turns, rather than a per-request one, is probably what this wants. Worth its own task rather than an edit here. Smaller notes: - The `SECURITY` log lines now run past 400 characters each. The content is right. One surface per line would make them readable in a terminal at the moment someone reads them. - `handleChatPage` reflects `q` and `r` from the query string. `html/template` escapes both, so this is fine and needs no change.

kami referenced this issue from a commit

2026-08-01 14:43:47 +02:00

mavweb: gate the voice path on step-up like the text path

kami closed this pull request

2026-08-01 14:51:37 +02:00

kami commented

2026-08-01 14:52:31 +02:00

Landed on master. The stack was one linear chain, so #84 carried every commit from #50 up, and master now contains this branch in full. Merging this PR on its own is an empty diff, so it is closed rather than merged. The review findings for it were fixed in the 2026-08-01 pass and are on master as commits on the stack tip, not on this branch.

Pull request closed

Please reopen this pull request to perform a merge.

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: kami/Maven#51