feat(api): let the app reach the box's own API, with a role attached - #865
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 14b7e16b61
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| repository: srcfl/ftw-webapp | ||
| ref: main | ||
| path: .app |
There was a problem hiding this comment.
When contract/registry.yaml changes in either repository, this job compares the proposed version only with the other repository's already-merged main, so the first half of every coordinated contract update necessarily fails CI; this commit itself changes the registry while saying the box must merge first. If the app applies the documented reciprocal check, neither half can satisfy required checks without an override. Compare against the paired revision or adopt a single authoritative source so contract updates remain mergeable.
Useful? React with 👍 / 👎.
| w.panicked = true | ||
| } | ||
| }() | ||
| h.cfg.API.ServeHTTP(w, r) |
There was a problem hiding this comment.
For an APIReq using HEAD, the Go mux selects the corresponding GET handler, but this in-process call supplies apiWriter rather than net/http's request-aware response writer. Consequently, every handler write becomes api.chunk data and a HEAD of endpoints such as /api/status returns the complete GET body (and can even be reported truncated), defeating HEAD semantics and its bandwidth benefit. The passthrough needs to discard body writes for HEAD while retaining the status and headers.
Useful? React with 👍 / 👎.
| meta.role = role | ||
| meta.epoch++ | ||
| i.mu.Unlock() | ||
|
|
||
| return i.save() |
There was a problem hiding this comment.
If saving applink.json fails, for example because the filesystem is full or read-only, SetRole returns an error after already changing the in-memory grant. The API therefore reports that a promotion failed while the target's live session immediately gains owner privileges through GrantFor; after restart the old on-disk role returns. Persist atomically before publishing the new grant, or restore the old role and epoch when save fails.
Useful? React with 👍 / 👎.
| s.handle("GET /api/loadmodel", Read, s.handleLoadModel) | ||
| s.handle("POST /api/loadmodel/profile", Configure, s.handleLoadModelProfile) | ||
| s.handle("POST /api/loadmodel/reset", Configure, s.handleLoadModelReset) | ||
| s.handle("GET /api/research/load/dump", Read, s.handleLoadResearchDump) |
There was a problem hiding this comment.
An enrolled viewer can request /api/research/load/dump?days=365 because this route is marked Read, even though the handler returns an application/gzip whole-file archive that the passthrough never carries. The handler loads a year of history, forecasts, and prices and starts constructing the tarball before apiWriter rejects its media type, so repeated requests can consume substantial database and CPU resources while always ending in E_UNSUPPORTED_MEDIA. Mark this archive local-only so it is refused before the handler runs.
Useful? React with 👍 / 👎.
The app could name six things to ask its box. The box's own page can name 132, so every new view in the app cost a box release. The session now carries the box's HTTP API directly — api.req in, a status and a byte stream back — and a view over a route the box already serves is the app's own work alone. This is not a widening of the trust boundary. Those 132 routes are already served on the home LAN with no authentication at all: anything on the network can call them. Reaching the same handlers through a Noise session, pinned to a device the box enrolled optically, with a role attached and a passkey ceremony in front of every write, is strictly stronger than what households run today. Five things keep it from becoming a hole, and the tier model is the one that took the work. Deciding a route's tier from its HTTP method looked obvious and was wrong twice: GET /api/caldav/credentials hands back a password that is a write channel into dispatch, and POST /api/self_tune/start drives every battery through ±3000 W. Both were reproduced against a viewer and an ordinary owner. So every one of the 132 routes now carries an explicit tier declared beside its handler, forgetting one does not compile, and a path no route claims is closed rather than served. The sweep moved 15 routes to local-only — anything whose answer is a credential or a whole file, or whose act needs somebody at the box. Roles were named in contract/registry.yaml and enforced nowhere: every enrolment was effectively an owner. An enrolment now carries one, the box checks the scope, and a viewer is a viewer because the box says no rather than because the app hides a button. Two doors, and what each proves decides what it opens. The LAN proves presence, so it is the only door that admits a new owner or mints a spoken code. A session proves enrolment and nothing about location, so it sees the roster, invites a viewer and revokes. The role is never a default: absent or unparseable is a 400. Without that, opening the door would have turned every "invite the family to look" into handing over the house — the app asked in a query string, the box read a body, and an absent body meant owner. A code the box shows completes the set: 40 bits of Crockford base32, minted only on the LAN, five-minute life, burned after five wrong tries, redeemed in Noise message 1. It is the way back when there is no other device. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The guard compares this repository's copy against the app's default branch, and the app's compares against this one's. A change that lands in both at once therefore waits for itself: neither side can go first, and the guard's own error message tells you to change both copies in the same pair of pull requests, which is the thing it forbids. A pull request that names where its pair lives is now compared against that branch. Everything else still meets the default branch, which is the drift this job exists to catch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
github.event.pull_request.body is captured when the run is queued, so a pair declared after the last push is invisible to it — the job compares against the default branch and looks like it worked. Ask the API for the body instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
55a732d to
86af6e4
Compare
|
Post-merge finding: this contract job fails on every push to Confirmed in master runs 31160622940, 31162164286, and the current run 31164667816. PR checks stay green because they have a PR number. The product tests pass; this is a workflow event-handling defect. The pair-selection step should call |
1 participant
The app could name six things to ask its box. This page can name 132, so every new view in the app cost a box release. The session now carries the box's HTTP API directly —
api.reqin, a status and a byte stream back.Pairs with srcfl/ftw-webapp — merge this first.
Why this is not a widening of the trust boundary
Those 132 routes are already served on the home LAN with no authentication at all. Anything on the network can call them. Reaching the same handlers through a Noise session, pinned to a device the box enrolled optically, with a role attached and a passkey ceremony in front of every write, is strictly stronger than what households run today. The LAN is the thing still waiting to be fixed; this is a step toward it, and the identity seam it adds is what that will be built on.
The tier model, which took two attempts
Deciding a route's tier from its HTTP method looked obvious and was wrong twice, both reproduced:
GET /api/caldav/credentialsis a GET, so it was a read — and it hands back a password that is a write channel into dispatch. A family member given read-only access walked away able to drive the house.POST /api/self_tune/startwas carried as ordinary configuration. It drives every battery through ±1000 W and ±3000 W step patterns.So every route now carries an explicit tier declared beside its handler. Forgetting one does not compile, an unknown tier panics at startup, and a path no route claims is closed rather than served. The sweep of all 132 moved 15 to local-only — anything whose answer is a credential or a whole file, or whose act needs somebody at the box.
GET /api/configis among them: its secret masking returns early on a driver-catalogue read error, so secrecy that depends on a directory read succeeding is not a read.Roles, and the trap in the middle of them
contract/registry.yamlhas named the scopes and the owner/viewer roles for a long time. Nothing enforced them: every enrolment was effectively an owner.Opening the sharing routes to the session without fixing that would have been worse than leaving them shut. The app asked for
role=viewerin a query string; the box read the role from a body; an absent body defaulted to owner. Proven against the real handler: the app's shape mintedrole="owner"while the app believed it had asked for viewer. It was masked only because a 403 got there first.The role is never a default now — absent or unparseable is a 400.
Two doors, and what each proves decides what it opens. The LAN proves presence, so it is the only door that admits a new owner or mints a spoken code. A session proves enrolment and nothing about location, so it sees the roster, invites a viewer and revokes.
The box code
40 bits of Crockford base32, minted only on the LAN, five-minute life, burned after five wrong tries, redeemed in Noise message 1. It is the way back when there is no other device and no sealed copy — the floor that always works.
Review notes
contract/registry.yamlis byte-identical to the app's copy, and a check now fails when they differ — it drifted three ways under a test that compared this file against itself.🤖 Generated with Claude Code
Contract-pair: srcfl/ftw-webapp@15-api-passthrough-energy-sharing