mirror of
https://github.com/nestriness/nestri.git
synced 2026-09-19 17:25:19 +03:00
b4776fad2f2bf232da430db3d420c4ee74396b60
173 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Wanjohi
|
b4776fad2f |
feat(core,api): record burn as rate segments, and refuse the next run when spent
The counters this fills are the ones the windows already knew how to read. What was missing was anything that put a number in them. Burn is recorded as segments: one stretch of one run at one unchanging rate, opened when the rate becomes true and closed when it stops being. Not a row per session, because a session's rate does not survive its own lifetime — a second run changes what the account spends per second while the first is still going, and a rate that applied from that moment must not be backdated over the time before it. Not a row per event either, because burn accrues against an envelope that is held rather than per thing consumed. Closing a segment is what moves burn into the counters, so a long run lands incrementally instead of all at the end. Burn that only arrives when a session stops is burn that cannot refuse the next one, and a bar that does not move while something is running is a bar nobody believes. The counters are written with the staleness rule as a single statement: add to the total if its stamp is still inside the window, otherwise start again from this amount. Reading and then deciding would be two statements with a gap, and the gap is where a concurrent tick doubles or vanishes. The first tick for a team and the thousandth are the same call, for the same reason. The gate sits at the one moment it is allowed to speak — before a run starts, never again. A limit refuses the next run and never interrupts one already going; someone losing a session mid-game to a meter does not come back. Every window is checked rather than the shortest, because they protect different things over different spans. The answer comes back with the created run rather than being thrown away: the response carries where each window stands, what the account spends per second now, and what one more run would cost. Every surface that can start a run has to show that before the click, and a second call for it is a call nobody makes. Asking twice would also let the number shown and the number billed disagree. Accrual is wired to the run's own state transition, in the same transaction that moves it. A session that went live without its meter starting is free hardware; one that ended without its meter stopping bills forever. Both are silent, so neither may be a second write that might not happen. |
||
|
Wanjohi
|
15631f5d25 |
feat(core,api): an organisation owns hardware, and a domain says who belongs
Two kinds of machine were modelled as one. A host somebody brings is theirs, reached through a team, and should die with their account. A host bought to serve other people's workloads is none of those things — and there was nowhere to put it, so it had to be registered under an employee's personal team, where it was that person's property and their account going away took it with them. Ownership becomes an either/or. A machine names a team or an organisation, exactly one, enforced by a check constraint rather than by convention: both null is a host nothing can bill, and both set is two answers to "whose is this?" where whichever join a query happens to take decides who pays. Hardware an organisation owns has no team and no person at all, which is the point. The organisation is deliberately not a billing subject and has no plan columns. It says who owns the metal; a team pays for what it uses either way. Membership is derived from a verified email domain rather than stored. An address is already the root identity, so a second record of who belongs where is a second answer that can disagree with the first — and deriving it means signing in with a personal address still gets an ordinary personal account, which is what lets one person hold a company account and use the consumer product. Nothing is granted on an unverified domain or an unverified address: either one is a string somebody typed. Entitlement on fleet hardware refuses everyone for now, with a reason that says so. What grants a run on metered hardware is a plan, and there is nothing to ask yet, so it fails closed rather than giving the expensive case away. The branch is written out so the plan check has one obvious place to land. Routes are read-only, and nothing seeds an organisation. Creating one grants membership to everyone who can receive mail at a domain, so it is an operator action against the database — a migration that inserted one would insert it into every deployment, including ones we have nothing to do with. See docs/deploy.md. |
||
|
Wanjohi
|
4a2a4412c2 |
style: apply the formatter across the tree
The formatter had never been run over these files, so `oxfmt` on a couple of touched files rewrote two dozen others and buried the real change. Running it everywhere once makes the next diff mean something. No behaviour changes: import ordering, line joining, and reindented JSON in the generated migration snapshots. Both test suites and both typechecks give the same answers as before, including the two type errors this does not fix. |
||
|
Wanjohi
|
40b4270161 |
refactor(api)!: remove the shared operator secret, and let hosts sync their own
A single secret that turned any request into an operator was the only credential several routes accepted, and it had no caller left: the device pairing it existed for is on hold, and nothing in this tree or any client sent it. What remained was a key that bypassed authentication entirely, required to boot, and checked by nobody. Every route behind it had a better answer available: - Library and game sync move to host credentials. Both took a `userId` in the body, which meant one secret could write into anybody's library. A host now says which of its enrolled users a batch is for, and that claim is checked against the Steam sign-ins it actually holds — one box carries several people's accounts, so the pair is the unit. - Download-state reporting narrows to hosts alone, and the body that could name a different host is gone. Which host is reporting comes from its own credentials, and a body that still names one is refused rather than ignored. - Linking a Steam account is always for the caller. - Creating a game by hand is deleted; syncing already upserts the catalogue. - Reading the waitlist is deleted. Every address on it belongs to someone who has not agreed to anything, and answering it over HTTP made that list something a leaked key could drain. - The pairing-code routes are deleted with the flow they served. The domain module and its table stay, so returning to it is a route file rather than a migration. Nothing in the API now accepts a credential that stands for more than one caller: every request resolves to a specific user or a specific host, which is what lets a route say "the caller's own library" and mean it. BREAKING CHANGE: the `x-nestri-admin-token` header is no longer accepted and `ADMIN_SHARED_SECRET` is no longer read. `POST /games`, `GET /waitlist` and the `/pairing-code` routes are gone; `POST /games/sync` and `POST /library/sync` now require host credentials and take `userId` in the body; `POST /steam/link` no longer accepts `userId`; `POST /games/download-state` no longer accepts `hostId`. |
||
|
Wanjohi
|
cfb8ec26a0 |
feat(machine): mint a public name, and stop routing on the id
A host's hostname was its primary key. That worked and disclosed three things it should not have: ids here are monotonic, so an id in a hostname tells anyone who reads a URL roughly when that machine was registered and where it falls among its owner's others; an id is the primary key, so a name that had to change could only change by re-registering the machine, which is changing its identity to fix its name; and the hostname is also the OAuth audience and the cookie scope, so the id travelled into redirect URLs and browser history. Machines now carry a minted name -- two words and four digits, unique across the fleet, DNS-safe by construction, which an id was not. The words are a curated list rather than a dictionary, because every pair is shown to strangers. Names the fleet's own infrastructure answers on are refused at mint time: one minted onto the edge's own label would take the published key path away from every host at once. Minting retries on the unique index rather than checking first, because two registrations in the same instant both read "free" and both write. Only a name collision retries; a duplicate id or secret means something a new name cannot fix. The migration adds the column in three steps. Generated as a NOT NULL column it fails outright against a populated table, and a default would be worse: every row would share one value on a routing key. |
||
|
Wanjohi
|
aecae0c69e |
style(auth): tighten the sign-in field, and stop calling the runtime a worker
The email field sat a little taller than the action below it. Two comments also described this page as rendering inside a worker, which is not how the control plane runs; what they were reaching for is that nothing preprocesses this file, so the design tokens are written out longhand. |
||
|
Wanjohi
|
74391714d0 |
feat(auth): draw the sign-in screen in the product's design language
The screen was still the upstream template's: its font, its accent, its logo, and a theme that tried to serve a light and a dark scheme from one set of colours by deriving each one from the background's lightness. That derivation is replaced with stated values, and the page is dark only. Black, a neutral grey ramp, one brand accent, Mona Sans for the display line and Geist for anything read or typed; two dashed bands closing into a box on a wide screen, a dashed vertical either side of the column, the wordmark, and the line of copy the rest of the product opens with. The email field keeps the fix from the previous commit and takes the brand colour on focus, which is the only place it appears besides the wordmark. Measured against a render of the same design built from its own source: the band rules, the column rules, the wordmark box and the button height land on identical pixels. |
||
|
Wanjohi
|
4ed36e0d38 |
fix(auth): give the sign-in input a text colour, and restore the theme
The email field computed its own background one step lighter than the page and never set `color`. Form controls do not inherit it, so the text someone typed was the UA default — black, over a near-black field. There was no `color-scheme` either, so the browser rendered the control in light appearance to begin with. The theme was a second, separate loss: `issuer()` still takes one and calls `setTheme`, but nothing had passed one since `packages/auth` became a vendored fork, so every sign-in rendered as OpenAuth — its font, its periwinkle, its logo. Restored from the pre-fork config, with the logo and favicon repointed because both URLs it carried now 404 and a broken `logo` renders a broken image rather than falling back. |
||
|
Wanjohi
|
b00f1064ae |
fix(api,auth)!: bind loopback by default, and let a container ask for more
Both servers bound `0.0.0.0`. That was harmless while the only deployment was docker-compose.yml, which publishes these ports on 127.0.0.1 and makes the container's own bind irrelevant. As ordinary processes on a rented machine there is no such wrapper, and `0.0.0.0` is a listener on the internet — in front of an issuer that sets cookies without `Secure` and mints sign-in codes, because it expects something else to be terminating TLS. So the default is `127.0.0.1` and `HOST` is there for the deployment that genuinely needs every interface. compose now sets `HOST: 0.0.0.0` explicitly, which is not a workaround: inside a container, binding loopback is what would make the published port unreachable. The right answer differs by deployment, which is why it is a variable rather than a constant. Both now log the address they bound, not the one they hoped for. BREAKING CHANGE: api and auth no longer listen on every interface by default. A deployment that relied on that must set HOST=0.0.0.0. |
||
Kristian Ollikainen
|
15ad60bf49 |
feat: nescapture pacing, improvements and deps update (#334)
Make nescapture better with proper queue family checks, FPS limiting,
semaphore usage and other.. also updated deps like pollster and
pixelforge.
<!-- greptile_comment -->
<!-- greptile_summary -->
<h2><a
href="https://app.greptile.com/api/retrigger?id=64083904"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://greptile-static-assets.s3.amazonaws.com/badges/RetriggerDark.svg?v=1"><source
media="(prefers-color-scheme: light)"
srcset="https://greptile-static-assets.s3.amazonaws.com/badges/Retrigger.svg?v=1"><img
alt="Retrigger"
src="https://greptile-static-assets.s3.amazonaws.com/badges/Retrigger.svg?v=1"
align="right"></picture></a>Confidence Score: 5/5</h2>
The final review contains no accepted findings, so the PR appears safe
to merge.
<h3>Summary</h3>
- This PR reworks `nescapture` frame capture around a four-slot DMA-BUF
ring, tracks presentation queue families, adds semaphore-based
capture/present ordering, introduces capture frame-rate pacing, carries
presentation timestamps through encoding, and updates Vulkan-related
dependencies.
<sub>Reviews (1) · Last reviewed commit: ["feat: Update deps and remove
deprecated
..."](
|
||
|
Kristian Ollikainen
|
8246aa5538 |
feat: resident guest init (#333)
Get this thing going..
<!-- greptile_comment -->
<!-- greptile_summary -->
<h2><a
href="https://app.greptile.com/api/retrigger?id=63134761"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://greptile-static-assets.s3.amazonaws.com/badges/RetriggerDark.svg?v=1"><source
media="(prefers-color-scheme: light)"
srcset="https://greptile-static-assets.s3.amazonaws.com/badges/Retrigger.svg?v=1"><img
alt="Retrigger"
src="https://greptile-static-assets.s3.amazonaws.com/badges/Retrigger.svg?v=1"
align="right"></picture></a>Confidence Score: 5/5</h2>
The PR appears safe to merge; all previous findings are resolved and the
latest readiness change introduces no established actionable regression.
<h3>Summary</h3>
- Establishes required guest filesystems, runtime directories, device
permissions, and service processes.
- Reports initialization and service deaths over the lifecycle channel.
- Supports launch, restart, and shutdown commands for a resident guest.
- Separates service and workload identities and configures per-launch
runtime environments.
- Removes the currently inactive nescope screenshot option and makes
capture-chain verification fail explicitly when compositor readback is
unavailable.
- Reworks the guest image around `nesinit` as PID 1 without a
distribution service manager.
<h3>Diagram</h3>
```mermaid
sequenceDiagram
participant Host
participant Init as nesinit
participant FS as Guest filesystems
participant Services as Service stack
participant Workload
Init->>Host: Ready(protocol version)
Host->>Init: Boot(mount descriptors)
Init->>FS: Establish and mount shares
Init->>Services: Spawn services in order
Services-->>Init: Required sockets ready
Init->>Host: Initialized(service names)
Host->>Init: Launch(id, exec, on_exit)
Init->>Workload: Spawn with isolated UID/runtime
Init->>Host: Started(id)
Workload-->>Init: Exit status
Init->>Host: WorkloadExited(id, status)
Host->>Init: Launch / Restart / Shutdown
```
<sub>Reviews (4) · Last reviewed commit: ["fix(nesinit): readiness is a
socket
that..."](
|
||
|
Wanjohi
|
51d25f3e8c |
fix(machine): a taken endpoint id is a conflict, not a server fault
A host reporting an endpoint id another machine already holds hit the unique index, and the raw refusal reached the global handler as a 500 -- telling a host its beat broke the server rather than that the id is taken. It is now the 409 every other conflict here gives, and the route documents it. Checked-then-written would be worse rather than better: two hosts reporting the same id in the same instant both read "nobody holds it" and both write, which is precisely what the index is for. The read would add a query and remove nothing. Before: expect(res.status).toBe(409) Received: 500 |
||
|
Wanjohi
|
49ae45624e |
fix(auth): a host may receive a code at its own name, and a refusal is not a redirect
Two changes to who may start a flow here, and where a refusal is delivered. A host reached at its own hostname sits on a different registrable domain from this issuer, deliberately: that is what stops a cookie set there from ever reaching this one. The default rule allows a redirect back to whatever hostname the request arrived on, so it refused exactly the case the separation created. Which is a real problem rather than a theoretical one, because a session cookie without a Domain attribute is host-only, so a browser arriving at one of those hostnames for the first time carries no cookie whether or not it is signed in, and sending it here to sign in again changes nothing. So a client id that is a single hostname under that zone, whose redirect_uri is https and that same hostname at one reserved path, is allowed. Making the client id the hostname is the load-bearing part: a token's audience is its client id, so the session that comes back is bound to the host it will live on and is not a credential anywhere else. Separately, and worth its own paragraph: a refused client's redirect_uri was still used to report the refusal. The check that approves that URI is the one that just failed, so /authorize was an open redirector to anywhere at all -- no sign-in required, on the hostname people are asked to type a password into. It is now a page here. Before: GET /authorize?client_id=web&redirect_uri=https://somewhere.example/callback -> 302 https://somewhere.example/callback?error=unauthorized_client |
||
|
Wanjohi
|
6603383ad1 |
feat(machine): record where a host can be reached, as the host reports it
The machine table said who owns a host, which team it belongs to and when it was last seen, and nothing about how to reach it. Anything standing in front of a host and authenticating browsers on its behalf could therefore authorise a request perfectly and then have nowhere to send it. Reported, never assigned. A host holds the secret half of this identity and is the only thing that can know the public half first, so it rides on the beat it already sends as itself. Omitting the field leaves the stored value alone -- an agent that does not mention where it is has not moved, and an absent field must never read as "nowhere", which would take every host shipped before this field off the map on its next beat. Nullable, because "has never reported one" is a real state that every host registered before today is in. Unique, because an endpoint id belongs to one host: two rows claiming the same one would send a request addressed to one machine to another machine's agent, which is the one mistake here that the authorisation in front of it cannot catch. |
||
|
Wanjohi
|
0e94620808 |
feat(nesinit): carry the session's address out of the guest (#328)
## What was missing `neshub` serves the session's address on a socket, and its own flag has always said how that address gets out: > *"neshub listens; nesinit dials and carries the ticket to the host, because > the person who needs it is outside this VM and stdout here is a log file > inside one."* Nothing dialled it. `grep -rn ticket apps/nesinit/src` returned nothing at all, so the address never left the guest — and the one lifecycle message for it, `Ticket`, had no sender. This adds the carrier. ## Three decisions worth reading **Polled, not read once.** An address is not a value, it is the best answer so far: an endpoint discovers more ways to reach it after it binds — a local one immediately, a relayed one seconds later. Reading once means whoever asked first decides, and the first answer is the one that works on a local network and fails from anywhere else. Only a *changed* answer is forwarded, so an unchanged one costs nothing. **Dials, does not listen.** The opposite of the payload relay next door, and deliberately so. There the guest listens because the workload starts later; here the server is the long-lived one. Dialling also makes "not bound yet" an error to retry rather than a connection to wait for without knowing whether it is coming — which is the ordinary case at boot, since this starts before the server does. **The address is never logged.** It is a capability to reach the session, and a log inside the guest is the one place it has no reason to be. The log line says whether it is the first one and nothing else. ## Failing first The carrier's tests fail against unmodified code by not compiling: `ticket.rs` does not exist and `session::run` takes three arguments. Said plainly rather than manufactured. The behavioural gap is better shown as the `grep` above — nothing in the guest ever sent a `Ticket`, so the message had one end. `nesinit`: **35 tests passing**, up from 27. Four on the carrier itself (an address arrives; a better one replaces it; a socket that is not there yet is waited out rather than failed; an empty answer is not an address) and two on the session (an address reaches the caller as `Ticket`; a carrier that stops does not end the session). ## Verified in a real guest Built static for musl, run as PID 1 in a real microVM under a real VMM with a real vsock. It dialled out, completed the handshake at version 2, took a boot descriptor, started its workload, read the address that workload published and sent it up the channel — twice, the second time because a better one appeared. The caller saw `nestri:local-only` and then `nestri:with-relays`. Guest boot to init was **310 ms**. ## Review round ( |
||
|
Wanjohi
|
f74de9beb8 |
fix(nesinit): do not mount over the share tree, and check who serves an address
Four findings from review, all of them real. The relay's directory was mounted on the tree a session's shares live in. A fresh tmpfs there hides every directory the image prepared underneath it: the install, the user state, the work directory, and the mount point the log share is attached to from fstab. A box would have come up with a socket and without any of the places its workload looks for its files, and the exact-path check could not notice, because what fstab mounts is a directory inside that tree rather than the tree itself. It moves to /run, which is where a runtime socket belongs, is a tmpfs already, and has nothing else mounted inside it. It was also owned by this process and closed to everyone else, which stopped the workload traversing it to reach the relay at all. The directory is now readable and searchable, and still writable by nothing but this process, which is what makes the socket in it unreplaceable; the socket itself is what the workload is allowed to connect to. The permission belongs on the socket rather than on the path. The address served to a reader was built once at startup and served forever, so a reader that polls for a better one could only ever get the first. An endpoint does not know all of its own addresses when it binds: the first is the one that works on the same network and fails from anywhere else. It is now rebuilt per read, which is what makes polling for it worth doing. And the address was taken from whoever held a path in a directory the workload can write. Workload code could unlink the socket a service was listening on, bind its own, and every read afterwards would hand the client an address of its choosing -- a session given to somebody else rather than a session that fails. The peer's credentials are now checked before a byte is read, from the kernel rather than from anything the peer says about itself, and an address served by the workload's own user is refused and said loudly. That check is only worth something while the workload has a user of its own, so the image grows one. Two users, and they must stay two: one runs the services that ship in the image, the other is who a workload runs as. Sharing one does not weaken the check, it makes every session fail it. A workload running as root is every user at once and cannot be told apart from anything; the check stands down there and says so at boot instead, because refusing root would refuse whatever legitimately serves the address as well. Also bumps tinyvec by a patch release. It does not build on this toolchain -- `vec` resolves to the module and not the macro -- which made every crate that depends on an endpoint, including this one, unbuildable. Pre-existing and nothing to do with this change; the lockfile said the same version before it. |
||
KAAL1
|
00a2bdab30 |
fix(nesinit): give one look at the address socket a deadline
There was a cap on how much this would read and none on how long it would wait. The far end can accept a connection and then write nothing, and a read with no deadline turns that into a poll loop that never runs again: the address already forwarded stays correct, and the better one that arrives afterwards is never seen. That is the failure this polls to avoid, reached by a different route. Five seconds, against a two second interval, so an answer that is merely slow still lands and one that is never coming is abandoned. The test hangs against the code as it was, which is the whole point of it. |
||
|
KAAL1
|
18864b97f0 |
fix(nesinit): mount what the guest needs before anything asks for it
The root arrives read-only and this process is PID 1, so until it mounts them there is no /proc and nowhere in the filesystem to put a socket. Nothing else in the guest is an init system, so nothing else was going to. The symptom was three failures that look unrelated and share one cause. On a real box the payload relay could not bind, with EROFS; whatever serves the session's address could not bind either, the same way; and this process could not make itself ineligible for the OOM killer, because /proc was not there to write to. What the caller saw was a workload that ran and published nothing, which is true and says nothing about why. /proc is mounted first and unconditionally: finding out what an image already mounted requires it, and it is therefore the one entry that cannot be checked that way itself. Everything after it is skipped when it is already present, so an image that does this properly is not mounted over. Failures warn rather than abort. Refusing to boot would replace a session that fails with a reason by a guest that never dialled out at all, and the second is harder to diagnose from the outside. The relay's directory is named by the module that owns the socket rather than spelled again here, with a test tying the two together: a rename that reached one and not the other would put the relay back exactly as it was. |
||
|
Wanjohi
|
fe5297acbd |
fix(core): document an id that is actually a valid id
The example generator emitted twenty-five payload characters where an id has twenty-six, so every documented id was twenty-nine characters — one short of the width the column holds and, since last commit, one short of what the schema publishing it will accept. Nothing caught it because an example is never parsed: it is copied into documentation and read by people. The width now comes from the generator's own constant instead of being typed out, in the two places that had counted it by hand. Counting twenty-six of anything by eye is a thing people get right once and never re-check. A test pins the three together — a generated id, the schema for one, and the documented example must all agree, for every prefix. It fails on the off-by-one that prompted this, and on a prefix without its separator, which would otherwise read as an id of that type because it starts with the same three letters. |
||
|
Wanjohi
|
3d24a8e130 |
feat(nesinit): carry the session's address out of the guest
Whatever serves media in a box knows how it can be reached, and the person who needs to know is not in the box. Standard output here is a log file inside a VM, so the control channel is the delivery path rather than a convenience -- which makes this init's job and not a detail of whichever component happens to bind the port. The socket it reads was already documented as being read this way; nothing read it. Polled rather than read once, because an address is not a value but the best answer so far. An endpoint discovers more ways to reach it after it binds, so the first answer is the one that works on a local network and fails from anywhere else. Only a changed answer is forwarded. It dials rather than listens, which is the opposite of the relay next door and deliberate: there the guest listens because the workload starts later, and here the server is the long-lived one. Dialling also makes a server that has not bound yet something to retry rather than something to wait for without knowing whether it is coming. The address itself is never logged. It is a capability to reach the session, and a log inside the guest is the one place it has no reason to be. A carrier that stops does not end a session: whatever was already reported is still correct, and the workload's exit still has to be. |
||
|
Wanjohi
|
64a90abf75 |
fix(api): a misshapen id is bad input, not a server fault
Ids are stored in a fixed-width column, so an overlong one is refused by Postgres rather than simply matching nothing. That refusal is not a foreign-key violation, so it fell through to the global error boundary and reached the caller as a 500 — telling a host to retry something that can never succeed. Measured: a 44-character user id returned 500, where an absent but well-formed one correctly returned 404. `Identifier.schema` is the natural place for the check and had no callers yet, so it now asserts the exact width an id has as well as its prefix — including the separator, without which `usrsomething` reads as a user id. The enrolment schema uses it for both foreign keys, so the refusal happens where the input arrives and names the field. Also index `steam_enrolment.user_id`. The primary key begins with the machine, which answers what one host holds and nothing else, so neither of the two things that read by user alone can use it: the cascade behind deleting a user, and asking which hosts hold a token for one person. The table's migration has not been released, so this is folded into it rather than following it with a correction. |
||
|
Wanjohi
|
6429ec4ff7 |
feat(api): record which host holds a Steam token for whom
A host that signs a person into Steam ends up holding a refresh token. The control plane needs to know that happened — to show it, and so a host that lost its disk can find out what it is expected to hold — but it must not know the credential, because the token is bound to the address that obtained it and a copy anywhere else is the account-theft signal Steam watches for. So `steam_enrolment` stores the outcome and has no token column, no encrypted token column, and no column that could hold one later. The safeguard is that the credential is never sent here at all; a nullable column would be the first step in undoing it, so a test asserts the column list exactly and fails if one appears. Three machine-authenticated routes go with it: report a completed sign-in, report that Steam refused the token, and list what this host should have. All three take the host from its own credentials, so a box can neither report onto nor read another box's hardware. Their bodies are strict, so a host that sends a token is told it is wrong rather than quietly believed — which also keeps the value out of the request log. The Steam id is deliberately not unique. One account signed in on two hosts is two rows and two tokens, and a unique index there would look like hygiene while refusing somebody their second box. There is no `pending` state: a sign-in challenge lives about two minutes inside one process, and nothing outside it needs to know it exists. Nothing revokes yet, and `last_ok_at` has no writer — a successful logon happens where there is no credential to report it with — so the column exists with the shape it will need and stays null rather than being filled with the nearest event that was easy to observe. |
||
|
Wanjohi
|
b296918ab4 |
feat(api): record what a host says it is running
A host agent already sends a full inventory snapshot on a cadence, and nothing served the endpoint it sends it to — so every one of those calls answered 404. It fails quietly by design, because a dropped snapshot is meant to be corrected by the next one, which is exactly why nobody noticed: the only symptom is a line in the agent's own log. Kept separate from the heartbeat because the two have different loss tolerance. A dropped beat moves a host towards offline and unplaces it; a dropped snapshot costs nothing until the next one arrives. Folding them together would let a malformed inventory field make a healthy host look dead. Three rules decide what a snapshot may do, and the last two are why this is one core function rather than a loop in the route: - a box we know, that the snapshot names, takes the reported state - a box we know that was running, and that the snapshot omits, is stopped and says so — absence inside a snapshot is information - a box the snapshot names that is not placed on the calling host is never created, only reported back as a divergence The scope is in the `where` clause and not in the agent asking politely about its own boxes: a machine credential is a long-lived secret sitting on hardware in somebody's living room. `pid` and `uptimeS` are accepted and deliberately dropped. A pid is a number in another machine's namespace, and uptime is derivable from a run's start time, which is already stored and already trustworthy. |
||
|
Wanjohi
|
46c3a67d4d |
fix(deploy): make bun run dev:server start and reach its settings
Two defects, both found by running it. Neither affects the container path, which is why the images passed: they run the same entrypoints from the repository root, and that turns out to be the load-bearing detail. **The issuer would not start at all.** `apps/auth/tsconfig.json` named React as the JSX runtime — left over from an earlier scaffold; there is no React anywhere in this repository. It only bit when something transpiled from that directory, and then the process died resolving `react/jsx-dev-runtime` from a sign-in screen in `packages/auth` before it bound a port. The package holding those components already said `hono/jsx`, and so did the root; this is the third place agreeing with them. **Neither process could see `.env`.** They ran with the working directory set to their own app, so the environment file the repository documents — the one at the root — was not the one they were offered. The issuer then correctly refused to send a sign-in code rather than logging one, which is the right behaviour and an opaque way to discover a path problem. Both now run from the root, which is also exactly what the images run. |
||
|
Wanjohi
|
9258c8dfef |
fix(deploy): make bun dev actually start, and sign-in actually work
Six defects found by running the thing rather than reading it. The previous change was verified by bundling, by tests, and by the container images — none of which start a Worker, so every one of these was invisible. **`bun dev` did not start.** It ran one multi-config process, which does not connect a service binding between the workers it loads; the API reported `AUTH [not connected]` and could not verify a token. It is two processes now, which is what the dev registry connects, and the second is backgrounded with the first killed on exit so stopping the pair stops both. **Neither server could bind.** Wrangler resolves `localhost` and takes `::1` first; a host with no IPv6 address on its loopback dies with a bind error from inside the runtime that names neither the app nor the port. `dev.ip` is pinned to `127.0.0.1`, and `inspector_port` is now distinct per app — it is not derived from the port above, so the second server to start died on an address already in use. **The API worker failed to evaluate.** A specifier ending in `.sql` is claimed by the bundler as a module of its own, so the schema file was emitted verbatim beside the bundle and the runtime threw on an export it could not find. The route was reaching past the domain module into the schema to spell a status; it now asks the domain module, which is the rule everywhere else here and happens to also avoid the hazard. **Signing in failed on the second request that touched the database.** A pool is cached per connection string, and on a Worker an I/O object created while handling one request may not be touched while handling another. The first request always succeeded, which is why it went unnoticed — a sign-in is several. The cache is now kept only where a process outlives its requests, which is the case it was added for. **The images named a base that podman will not resolve.** A short name needs a registry; the database service alongside them already spelled one. **Compose pinned container names.** The name is not scoped to the project, so a second checkout got the same three, and `down` in one stopped the other's containers. This is not hypothetical — it stopped a running development database while this was being tested. Verified by signing in end to end against both dev servers: a code requested over HTTP, read from the issuer's log, redeemed, exchanged for tokens, and presented to the API, which resolved it to the account the sign-in had just created. |
||
|
Wanjohi
|
f30a1432f8 |
fix(deploy): require every credential, and give sandbox its own domain
Three things review caught, and one shape correction. **No credential has a default any more.** The compose file shipped `ADMIN_SHARED_SECRET` falling back to a value written in this repository — and that header bypasses token verification entirely, so anyone reading the file could act as an operator against any deployment that had not overridden it. A default is worth less than it looks here: the deployment that never set the variable is exactly the one where the default is public. Every credential now comes from `.env`, and compose refuses to start naming the variable it wanted. That also takes the last literal password out of a tracked file. **The origin ports are on loopback.** Both services speak plain HTTP and mark no cookie `Secure`, because both expect to sit behind something that terminates TLS. Published on every interface they were a way to reach the issuer around that proxy, with sign-in codes and tokens in clear text. **Mail settings are passed through rather than fixed.** The issuer was pinned to printing sign-in codes to its log, and the three delivery settings never reached it — so the documented way to configure mail could not work, and every code and recipient went to the container log instead. Printing codes is now asked for in `.env` like everything else, and with nothing configured the issuer refuses to send rather than logging. **Sandbox becomes a domain rather than a prefix.** `api.sandbox.nestri.io` and `auth.sandbox.nestri.io`, because sandbox holds whatever is not production and that set grows. One certificate for `*.sandbox.nestri.io` then covers all of it, including unpredictable per-pull-request names, and cannot be presented for production's own domain — which the zone-wide wildcard the previous shape leaned on could. Also drops `STEAM_API_KEY`. It was declared in two type definitions and read by nothing: linking an account makes no outbound call that needs it. |
||
|
Wanjohi
|
51ababc900 |
feat(deploy): drop the IaC layer, and make both apps runnable as containers
Moving the issuer's state into Postgres removed the last thing that tied either app to one hosting provider. What was left was a deployment tool describing resources that no longer existed — so this replaces it with `wrangler`, which is what actually deploys a Worker, and adds a second way to run each app that involves no provider at all. Each app now has a `wrangler.jsonc` with an environment per stage, and a `Dockerfile` beside it. The handler is the same one in both cases; what differs is only where its settings come from. Two of them gained a second spelling so that nothing has to branch on the runtime: Postgres arrives as a pooled binding or as `DATABASE_URL`, and the route to the issuer is a service binding or `AUTH_INTERNAL_URL`. That last one is new, and it is a split the binding was already making without saying so. `AUTH_ISSUER_URL` has to be the issuer's public name, because it is compared literally against every token's `iss` claim — but the public name is often not routable from inside a deployment. So the name and the route are two settings now rather than one that cannot be both. DNS moves out of code and into `docs/dns.md`, which lists every hostname and what it is for. Six records that change roughly never did not need a tool, and the table outlives whatever is answering the names — which is the point, since some of them will stop being Workers. The sandbox hostnames are hyphenated rather than nested for the same reason: a certificate covering `*.nestri.io` covers one label and not two, so `api-sandbox.nestri.io` can become an ordinary origin later without a certificate having to be ordered for it first. Also drops `EMAIL_DEV_LOG` from committed configuration into `.dev.vars`, which `wrangler deploy` cannot upload. Printing a live sign-in code to a log should not be one forgotten override away from production. |
||
|
Wanjohi
|
f25c9af545 |
feat(auth): keep issuer state in Postgres
The issuer kept everything behind one get/set/remove/scan interface, which is what a library that must run on any provider's cache can offer. Three of the things kept there could not actually be served by it. An authorization code must be redeemable once and a refresh token spendable once, and through get and set the check and the write are separate steps — so two requests arriving together both read an unspent record, and both mint a session. In the refresh case that also means the reuse which reveals a stolen token is never recorded, because recording it is the write that the second caller overwrites. Each now has a table and an interface of its own: redeeming is one `delete ... returning`, spending is one `update ... where time_used is null returning *`, so exactly one caller is ever told it went first. This is the same argument the device grant already made, applied to the two records that had it too. Signing keys move for a different reason. Nothing races for them; they are the one record whose loss ends every session at once, and a cache is a place things may be evicted from. They are retired by setting a column rather than deleted, so the tokens they signed stay verifiable until they expire. Both credential tables store a hash and never the credential, as the device grant does. An authorization code travels in a query string and so passes through history, referrer headers and any log along the redirect; a refresh token resumes a session outright. What is left in the generic store is the rate-limit counters — written far more often than read, meaningless within the hour, and allowed to be approximate, since a lost increment costs one guess out of ten. Those move to Postgres too, so the only key-value binding this deploys with is gone and the control plane's state is one database. That was the point: nothing here now depends on a primitive a self-hoster cannot run. The generic scan also gained the separator on its prefix, so scanning `a` cannot return what is under `ab` — subjects and email addresses are both prefixes of longer subjects and email addresses. Deploying this signs everyone out. The signing keys and refresh tokens are in a store that is being left behind, so the issuer starts with a fresh key set and every existing token stops verifying. |
||
|
Wanjohi
|
54d5c81edb |
feat(api): hold a run to the attempt that claimed it
The agent side sends a claim token on every write; this side rejected the field outright, so every state report and every ticket publish answered 400. Both bodies now take it. Underneath that, nothing compared a holder. A run was reachable by any caller on the right machine, and a box names exactly one machine — so two attempts polling the same job presented identical credentials and were told apart only by which one's select landed first. That is timing, not a rule, and no caller could be told which case it was in. The row now remembers which attempt holds it. Taking a claim requires there to be no holder; every write after it requires the caller to be the holder. The same state reported by a different attempt is a lost race and not a retry, and is refused whatever the state is - which is the only thing that separates the two 200s from the 409s. The ticket is held to the claim too, for a worse reason than a double start: the client re-reads the address rather than keeping the first, so a ticket written by a losing attempt produces a client that connects, successfully, to a machine running nothing. The holder is never cleared, including on a terminal state, so a settled claim cannot be replayed and a finished run still records which attempt ran it. It is not in what goes out - holding one permits writing to a run, and the owner reading their own session is not the holder. |
||
|
KAAL1 (Bingus)
|
2faf7d77db |
feat(nesinit): mount what the descriptor names, and relay the layer it cannot read (#320)
Stacked on #319, which has PID 1, the channel and the trait but mounts
nothing.
Review that one first; this PR is the descriptor half.
- **The shares are mounted.** A tag names an export, the descriptor
names where
it lands, and every share goes on `nosuid` and `nodev` whether or not it
is
writable — a share is data handed to the guest, and no descriptor has a
way
to ask for a setuid binary or a device node in one. Mounting needs
privileges a test does not have, so the arguments and flags are derived
by a
function the tests assert; that is where the read-only decision lives.
- **Progress is two messages, not one.** `mounted` / `mount_failed` stay
apart
from `started` / `start_failed`, because a share that did not appear and
a
command that did not run want different things looked at. A failure
carries
the reason the operating system gave, verbatim, and the path it happened
on.
- **The second layer is relayed and never read.** Envelopes cross a unix
socket
to the workload and come back the same way. `body` is a string rather
than
nested JSON on purpose: a document this component can index into is a
document it can grow to depend on, and then the layer is not opaque any
more
and the boundary it exists to draw is gone.
- **An envelope is never logged** — not the body, not truncated, not at
debug
level. The channel name and a byte count are the whole of what may be
said
about one. `Payload`'s `Debug` is written by hand for the same reason.
- **A write to a channel nobody reads now ends the session** the same
way a
closed read does, and stops the workload. A caller that stopped
listening has
also stopped being able to say stop; that was two outcomes and is one.
## The tests, failing first
Progress reporting, with the mount result dropped (the state this branch
started from):
```
running 11 tests
test session::tests::an_unreadable_line_does_not_end_a_session ... ok
test session::tests::a_stop_is_idempotent_and_does_not_end_the_session ... ok
test session::tests::the_guest_speaks_first_and_says_its_version ... ok
test session::tests::a_closed_channel_stops_the_workload ... ok
test session::tests::the_descriptor_mounts_and_starts_what_it_names ... ok
test session::tests::an_envelope_crosses_the_session_in_both_directions_unread ... ok
test session::tests::a_share_that_will_not_mount_is_refused_before_anything_starts ... ok
test session::tests::an_exit_is_reported_and_the_workload_is_not_started_again ... FAILED
test session::tests::a_command_that_will_not_run_is_reported_apart_from_a_share_that_will_not_mount ... FAILED
test session::tests::a_signalled_workload_is_reported_as_signalled ... FAILED
test session::tests::a_relay_nothing_is_on_does_not_end_a_session ... FAILED
failures:
---- session::tests::an_exit_is_reported_and_the_workload_is_not_started_again stdout ----
thread 'session::tests::an_exit_is_reported_and_the_workload_is_not_started_again' (312969) panicked at apps/nesinit/src/session.rs:248:13:
assertion `left == right` failed
left: Started
right: Mounted
---- session::tests::a_command_that_will_not_run_is_reported_apart_from_a_share_that_will_not_mount stdout ----
thread 'session::tests::a_command_that_will_not_run_is_reported_apart_from_a_share_that_will_not_mount' (312963) panicked at apps/nesinit/src/session.rs:460:9:
assertion `left == right` failed
left: StartFailed { reason: "ENOENT: /usr/bin/workload" }
right: Mounted
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
---- session::tests::a_signalled_workload_is_reported_as_signalled stdout ----
thread 'session::tests::a_signalled_workload_is_reported_as_signalled' (312966) panicked at apps/nesinit/src/session.rs:248:13:
assertion `left == right` failed
left: Started
right: Mounted
---- session::tests::a_relay_nothing_is_on_does_not_end_a_session stdout ----
thread 'session::tests::a_relay_nothing_is_on_does_not_end_a_session' (312964) panicked at apps/nesinit/src/session.rs:248:13:
assertion `left == right` failed
left: Started
right: Mounted
failures:
session::tests::a_command_that_will_not_run_is_reported_apart_from_a_share_that_will_not_mount
session::tests::a_relay_nothing_is_on_does_not_end_a_session
session::tests::a_signalled_workload_is_reported_as_signalled
session::tests::an_exit_is_reported_and_the_workload_is_not_started_again
```
The read-only flag, ignored:
```
running 9 tests
test shutdown::tests::a_workload_that_leaves_in_time_is_not_killed ... ok
test shutdown::tests::the_workload_stops_before_anything_else_and_the_disks_flush_before_power ... ok
test shutdown::tests::a_workload_that_overstays_its_grace_is_killed_and_shutdown_still_finishes ... ok
test workload::tests::a_failure_names_the_path_it_happened_on ... ok
test workload::tests::a_writable_share_is_still_mounted_without_devices_or_setuid ... ok
test workload::tests::a_read_only_share_is_mounted_read_only ... FAILED
test session::tests::a_closed_channel_stops_the_workload ... ok
test session::tests::an_exit_is_reported_and_the_workload_is_not_started_again ... ok
test session::tests::a_signalled_workload_is_reported_as_signalled ... ok
failures:
---- workload::tests::a_read_only_share_is_mounted_read_only stdout ----
thread 'workload::tests::a_read_only_share_is_mounted_read_only' (311963) panicked at apps/nesinit/src/workload.rs:212:9:
assertion `left == right` failed
left: 0
right: 1
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
failures:
workload::tests::a_read_only_share_is_mounted_read_only
```
The relay quoting a line it could not decode — which is the second way a
body
reaches a log line, and the reason the failure path logs a length and
nothing
else:
```
running 2 tests
test payload::tests::an_envelope_crosses_in_both_directions_untouched ... ok
test payload::tests::nothing_the_relay_logs_contains_a_body ... FAILED
failures:
---- payload::tests::nothing_the_relay_logs_contains_a_body stdout ----
thread 'payload::tests::nothing_the_relay_logs_contains_a_body' (312716) panicked at apps/nesinit/src/payload.rs:202:9:
a body reached a log line:
2026-09-04T21:11:56.619025Z WARN nesinit::payload: ignoring an envelope that would not decode bytes=62 line={"channel":"identity","body":"a-credential-nobody-should-read"
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
failures:
payload::tests::nothing_the_relay_logs_contains_a_body
test result: FAILED. 1 passed; 1 failed; 0 ignored; 0 measured; 17 filtered out; finished in 0.06s
```
And the first way: a derived `Debug` instead of the hand-written one.
```
running 6 tests
test lifecycle::tests::a_mount_failure_keeps_its_reason_verbatim ... ok
test lifecycle::tests::a_signalled_exit_is_not_a_zero_exit ... ok
test lifecycle::tests::a_line_round_trips ... ok
test lifecycle::tests::defaults_cover_what_a_caller_may_leave_out ... ok
test lifecycle::tests::an_envelope_does_not_print_its_body ... FAILED
test lifecycle::tests::an_envelope_body_stays_a_string_in_both_directions ... ok
failures:
---- lifecycle::tests::an_envelope_does_not_print_its_body stdout ----
thread 'lifecycle::tests::an_envelope_does_not_print_its_body' (313791) panicked at crates/nesprotocol/src/lifecycle.rs:290:9:
the body reached a log line: Payload { channel: "identity", body: "a-credential-nobody-should-read" }
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
failures:
lifecycle::tests::an_envelope_does_not_print_its_body
test result: FAILED. 5 passed; 1 failed; 0 ignored; 0 measured; 9 filtered out; finished in 0.00s
error: test failed, to rerun pass `-p nesprotocol --lib`
```
All green after: 19 unit tests, 3 against real forked children, 15 in
`nesprotocol`.
## What this does not verify
- **Nothing has been mounted.** No `virtiofs` share has been mounted by
this
code, in a VM or anywhere else. What is tested is the source, target and
flag
word handed to the mount call; that the call succeeds against a real
virtio
transport, that the mount point is where a workload then finds its
files, and
that a `uid` mismatch surfaces as the permission error this is written
to
produce, are all unverified.
- **The relay has never carried a real workload's traffic.** Two
processes, a
real unix socket and bytes that come back unchanged is what the test
shows.
Whether the socket path is the right mechanism is openly a guess, and it
is
meant to be replaceable without anything above it moving.
- **"Never logged" is enforced more narrowly than it reads.** The
hand-written
`Debug` and the failure path's length-only line are both tested. The
capture
test cannot reliably see debug-level lines: callsite interest is cached
process-wide, so a line another test in the same binary reached first
never
arrives in the capture. A future `{:?}` on a whole envelope at debug
level
would not necessarily be caught by these tests, only by the `Debug` impl
keeping its shape.
- **`geometry` is parsed and carried, and nothing consumes it.** This
component
does not start the guest's own services yet. `ticket` exists as a
message
with no producer wired to it.
- **Still no VM, still no vsock, still no `uid` drop**, as in #319, and
no
number in this PR is measured.
- **No third-party workload has gone through any of this.** The claim
that a
descriptor plus a set of shares is enough to run something we did not
write
is untested, and our own workload is the weakest possible witness for
it.
## Since review
`d4d473f` — the relay may not stall the session and may not buffer
without end,
plus a descriptor with a nul byte in it is refused by name. Failing
first, in
order:
The session held still behind a workload that was not reading:
```
running 1 test
test session::tests::a_relay_that_is_not_draining_does_not_stall_the_session ... FAILED
failures:
---- session::tests::a_relay_that_is_not_draining_does_not_stall_the_session stdout ----
thread 'session::tests::a_relay_that_is_not_draining_does_not_stall_the_session' (326881) panicked at apps/nesinit/src/session.rs:670:10:
the session stalled on the relay: Elapsed(())
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
failures:
session::tests::a_relay_that_is_not_draining_does_not_stall_the_session
test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 22 filtered out; finished in 5.00s
error: test failed, to rerun pass `-p nesinit --lib`
```
A frame with no end to it:
```
running 1 test
test payload::tests::a_frame_that_never_ends_costs_the_connection_and_not_the_guest ... FAILED
failures:
---- payload::tests::a_frame_that_never_ends_costs_the_connection_and_not_the_guest stdout ----
thread 'payload::tests::a_frame_that_never_ends_costs_the_connection_and_not_the_guest' (327888) panicked at apps/nesinit/src/payload.rs:375:10:
the relay is still assembling a frame that never ends: Elapsed(())
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
failures:
payload::tests::a_frame_that_never_ends_costs_the_connection_and_not_the_guest
test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 22 filtered out; finished in 5.01s
error: test failed, to rerun pass `-p nesinit --lib`
```
And a tag that was quietly emptied instead of refused:
```
running 1 test
test workload::tests::a_descriptor_with_a_nul_byte_in_it_is_refused_by_name ... FAILED
failures:
---- workload::tests::a_descriptor_with_a_nul_byte_in_it_is_refused_by_name stdout ----
thread 'workload::tests::a_descriptor_with_a_nul_byte_in_it_is_refused_by_name' (327380) panicked at apps/nesinit/src/workload.rs:274:40:
an empty source would have been mounted: ("", "/mnt/user", 6)
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
failures:
workload::tests::a_descriptor_with_a_nul_byte_in_it_is_refused_by_name
test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 22 filtered out; finished in 0.00s
error: test failed, to rerun pass `-p nesinit --lib`
```
One behaviour changed rather than only hardened, and it is worth a
reviewer's
eye: **nothing is queued for a workload that is not on the relay.** An
envelope
that arrives with nobody connected is dropped, as is one that arrives
faster
than the workload reads. That follows the layer's own rule — what
crosses it is
re-sent when it changes, so a held copy is a stale copy — but it does
mean a
sender that assumes delivery is wrong to. Nothing here retries, and
nothing
tells the far end that a particular envelope was dropped.
23 unit tests, 5 against real forked children, 15 in `nesprotocol`.
<!-- greptile_comment -->
<h3>Greptile Summary</h3>
This PR mounts descriptor-defined virtiofs shares, reports mount and
process-start progress independently, and relays opaque payload
envelopes between the host channel and workload Unix socket. Changes
since the previous review also bound relay frames, prevent relay
backpressure from stalling lifecycle handling, reject descriptor strings
containing NUL bytes, and track whether a reaped PID remains valid.
- Mounts shares at descriptor-selected targets with `nosuid`, `nodev`,
and optional read-only flags.
- Adds bidirectional opaque payload forwarding with bounded,
non-blocking queues and body-safe logging.
- Adds distinct mounted/start lifecycle responses and failure reporting.
- Adds capped newline-delimited relay frames and drops stale or
backpressured envelopes.
- Reworks workload tracking to avoid signaling a PID after its exit has
been delivered.
<h3>Confidence Score: 5/5</h3>
The reviewed changes appear safe to merge, with no accepted new findings
or outstanding previous root-thread findings.
The resolved relay-stall, unbounded-frame, and invalid-NUL findings are
addressed by non-blocking delivery, capped frame assembly, and explicit
descriptor validation. The protocol-version concern was correctly
withdrawn under the coordinated version-2 deployment model. The
remaining PID check-to-signal race duplicates an existing prior comment
and therefore is not reposted or counted as a new finding.
<h3>Important Files Changed</h3>
| Filename | Overview |
|----------|----------|
| apps/nesinit/src/payload.rs | Adds the bounded, bidirectional
Unix-socket payload relay with non-blocking delivery and body-safe
logging. |
| apps/nesinit/src/session.rs | Integrates payload events with lifecycle
handling and separately reports mount and process-start outcomes. |
| apps/nesinit/src/workload.rs | Implements descriptor-driven virtiofs
mounts and switches process signaling to tracked reaper state. |
| apps/nesinit/src/reap.rs | Adds shared reaped-state tracking so
callers stop treating a delivered PID as the workload. |
| crates/nesprotocol/src/lifecycle.rs | Extends lifecycle messages with
mount progress and opaque payload envelopes while redacting payload
bodies from Debug output. |
| apps/nesinit/src/main.rs | Starts the payload relay before the
workload session and wires bounded relay ports into session handling. |
| apps/nesinit/README.md | Documents mount behavior, payload opacity,
delivery semantics, frame limits, and progress reporting. |
<h3>Sequence Diagram</h3>
```mermaid
sequenceDiagram
participant H as Host
participant N as nesinit
participant M as virtiofs mounts
participant W as Workload
H->>N: Boot descriptor
N->>M: Mount descriptor shares
M-->>N: Success or failure
N-->>H: mounted / mount_failed
N->>W: Start command
N-->>H: started / start_failed
H->>N: Payload envelope
N-->>W: Non-blocking Unix-socket relay
W->>N: Payload envelope
N-->>H: Payload envelope
W-->>N: Exit
N-->>H: workload_exited
```
<sub>Reviews (3): Last reviewed commit: ["fix(nesinit): the relay may
not stall
th..."](
|
||
|
Wanjohi
|
217e82a9a8 |
feat(auth): email is the root of an account, and Steam is a connection (#318)
## What landed
**Email is the root of an account.** A `user` is created by verifying an
email
address and nothing else. Every Steam account is now a connection
hanging off a
user that already exists, capped at four.
**And it is the only thing that creates one.** Signing in with a gaming
account
or with an SSH key are both unwired from the issuer. Each could mint a
user,
which makes an account only as recoverable as the thing that made it and
gives
one person as many accounts as they have gaming logins. The providers
still
exist under `packages/auth/src/provider/` and can be wired back;
connecting a
Steam account is unaffected, because that runs through `POST
/steam/link` in
`apps/api` against a user who already exists. A test asserts the two
routes are
not served, so they cannot come back quietly.
**The pin-code provider is wired**, and email delivery refuses rather
than
guesses. **The device authorization grant is served**, and it now ends
at a
question somebody has to answer.
## The review found five real defects. All five are fixed, and so are
six more
The first pass of this branch shipped a device flow that handed out
tokens
without asking anybody, an email path whose pin could be guessed
outright, and
three read-then-write races. Each fix below has a test that fails
without it —
verified by reverting the fix and watching the test go red, not by
assertion.
### Signing in was mistaken for saying yes
`GET /device?user_code=…` started a provider flow and provider success
approved
the grant. So the whole attack was: ask for a device code, mail somebody
the
pre-filled link, keep the device code, poll. They see an ordinary
sign-in
prompt, complete it correctly, and you hold their access **and refresh**
tokens.
They were never asked a question, because there wasn't one.
There is now. Signing in establishes who the browser belongs to; it does
not
establish that the person meant to hand an account to a program running
somewhere else. The flow ends at a page that names the client, shows the
user
code back so it can be compared against what the device is displaying,
and
offers Approve and Deny. Approving is a POST carrying a value placed in
the
cookie alongside it, so another site cannot submit it on their behalf.
Denial moved onto that page too. It was a `GET` anybody could fire with
no
authentication: a link scanner or a chat unfurler would cancel real
sign-ins,
and anyone who learned a user code could grief one.
### A six-digit pin with unlimited guesses and a day to use them
The code travelled in an encrypted cookie held by the caller,
verification
compared against that cookie, and a wrong answer re-rendered the form.
Nobody
has to be the person the code was mailed to — type someone else's
address into
the first screen and the code goes to their mailbox while the cookie
stays with
you. At that point the only thing between a stranger and an account was
a
million requests. The constant-time comparison was guarding a door you
could
keep knocking on.
Guesses are counted on the server now, under a name that rotates with
every
code. The placement is the point: a counter kept beside the code, in the
cookie,
is one the guesser winds back by replaying an older copy. Starting over
is still
allowed and still costs a fresh code sent to the mailbox being aimed at,
where
somebody notices. The cookie's twenty-four hour life is ten minutes.
Resend is
spaced and bounded, because it was otherwise a way to mail a stranger as
fast as
requests go out. Both refusals say the same thing, since which one it
was is a
fact about someone else's mailbox.
The user codes on the other side of the flow are rate limited too —
eight
characters over a twenty-five character alphabet is a large space but a
fixed
one, and the endpoint had no opinion about how often you asked.
### Three read-then-write races
- **A poll could erase an approval.** The grant was read, modified and
written
back whole, so a poll that read a pending record and then wrote its
bookkeeping undid an approval that landed in between, leaving the client
polling a dead grant until it aged out. The same window let an approval
overwrite a denial.
- **The connection cap counted nothing.** `select … for update` over the
connections a user already had locks the rows it finds, and finding none
locks
nothing — there are no gap locks under read committed. Six concurrent
links
against a cap of four produced six; the test asserts that.
- **Concurrent email sign-ins returned a driver error.** Two tabs
finishing the
same sign-in both found no user, and the loser got a raw constraint
violation
instead of the account the winner had just made.
The cap now counts under a lock on the account's own row, which is the
one thing
every caller for that account contends on. The email paths let the
unique index
arbitrate and read back what the winner wrote. Device grants moved out
of the
key-value store into a table, where approving is one conditional update
and
redeeming is one delete that returns what it deleted.
### Three more the review did not raise
- **`client_id` was never checked at either end.** Anyone could mint a
grant
naming any client, and any holder of a leaked device code could redeem
it. It
is validated at issue and has to match at redemption.
- **Tokens were minted at approval** and left in storage until
collected, so the
lifetime reported to the client overstated what was left, and a grant
nobody
collected still left a usable refresh token lying around. They are
minted at
redemption.
- **The device code was stored as written.** It is the credential the
tokens are
handed to, so what is kept is now its hash: enough to recognise it, not
enough
to present it.
### And the environment check that decided none of this mattered
Mail delivery threw only when the environment said `production`, and
logged the
recipient and the live code otherwise. The deployment sets no such
marker — see
`alchemy.run.ts` before this change — so production took the developer
branch,
printed every code to a retained log, and reported success while nobody
received
anything.
That is the cost of a fail-open default: the deployment that forgets its
mail
settings is exactly the one with no marker saying it is real, so it gets
the
lenient branch precisely when it should not. Turned around. Printing a
code is
asked for by name; absence of configuration is a refusal; two settings
out of
three is an error rather than a fallback. Stages anyone else can reach
are
checked at deploy time, so a missing setting stops the deploy with the
name of
the variable it wanted.
## Where device grants live, and why it is a table
Short-lived state that would sit happily in a cache, in Postgres anyway.
The
reason is not durability. Every transition has to happen exactly once
while two
parties touch the same record — a browser somebody is clicking through
and a
program polling every few seconds — and a store that can only read and
write
whole records cannot promise that. The key-value store behind the rest
of the
issuer has no compare-and-swap, so on it the poll/approval race and
single
redemption can be narrowed and never closed.
The issuer cannot reach the database, so the store is an interface
(`packages/auth/src/device.ts`) with two implementations: one in memory
for
tests, one in `packages/core/src/auth/device-grant.ts`. Every method is
a single
operation and no caller reads a grant, decides, and writes it back.
Migration
`0010` adds the table. Rows are swept when a grant is created rather
than on a
schedule, since a grant lives ten minutes and that is the only statement
that
adds one.
## `session.claim_token` is here on another lane's behalf
Migration `0009` adds `session.claim_token`, nullable `text`, no default
and no
backfill. **It is not identity work and carries no identity reason** —
do not go
looking for one. It records which attempt holds a session run; the
endpoint that
reads and writes it arrives separately. It is in this migration only
because a
schema change has one owner at a time. The column is declared in
`session.sql.ts` and the snapshot, so the next `drizzle-kit generate`
will not
try to drop it — `0010` was generated clean, which is the proof.
## The tests
```
$ bun test
268 pass
0 fail
738 expect() calls
Ran 268 tests across 21 files.
```
Baseline on `dev` before any of this, measured on the same database:
**198 pass,
0 fail, 531 expect() calls.**
The tests that matter are the ones that would have caught the defects,
so each
was checked by putting the defect back:
| Reverted | What goes red |
|---|---|
| the confirmation step | signing in through the link leaves the grant
pending |
| the guess counter | four tests, including one that replays an older
cookie |
| the lock on the account row | six connections against a cap of four |
| the unique-violation handling | a driver error where a sentence should
be |
| the field-level poll write | an approval erased by a poll behind it |
| the verification rate limit | four tests |
Concurrency is exercised by running the same call several times at once
against
a real database, because run one at a time all of it passes whether or
not any
of the protection exists.
## The migration, against a database built to be awkward
`packages/core/script/verify-migration-0009.sh` builds a database
containing the
rows that make the statements do work — an account with no address, two
accounts
holding one address in different cases, an account already over the cap,
a
soft-deleted row holding a live row's address — applies everything
before `0009`,
applies it, and checks each case. All nineteen checks still pass. The
negative
control still dies where it should: with the de-duplication statement
neutered,
`create unique index` fails on the first duplicate pair.
## What this still does not verify
**1. No real email has ever been sent.** The mailer is tested against a
stubbed
`fetch`: the URL, the bearer header, the body it builds, and that it
refuses
when unconfigured. The body shape follows the common `{from, to,
subject, text}`
convention and may need a field the first real provider wants. This now
fails
closed and the deploy refuses without settings, so the failure mode is
loud
rather than silent — but it is still untested against a provider.
**2. The migration is verified against a database I built, not the one
that
matters.** I do not know whether production has duplicate addresses, how
many
rows carry case or whitespace, or whether any account is already over
the cap.
The fixtures cover those because they are *possible*. Worth three
`select`s
against production before this is applied.
**3. The verification rate limit is approximate.** The counter is in the
key-value store, so a caller spread across a distributed edge can exceed
the
budget somewhat. The number that decides the question is whether
somebody is
working through the code space, and a handful either way does not change
it.
**4. The single-redemption and conditional-transition properties are
held
against Postgres, and the in-memory store is trusted rather than
proved.** The
memory implementation gets its atomicity from nothing suspending inside
a
method, which is true of it and is not a promise the interface makes. It
is for
tests and local runs.
**5. A person who signed up by email carries an empty connected-account
id in
their token.** The subject schema requires the field and an account with
no
connection has nothing to put there, so it gets `''` — the same value a
server-to-server caller has always carried, and the one consumer already
falls
back to `''`. The honest shape is an optional field, and making it
optional
touches `subjects.ts`, the actor model and the API middleware. Flagging
rather
than doing.
**6. The desktop client cannot actually complete this flow yet.** Its
`DeviceCode` struct has no field for the device code, so it has nothing
to poll
with. That is a different lane's file and nothing here touches it, but
the grant
is not reachable end to end until it does.
**7. The four-account cap and the user-code alphabet are asserted, not
measured.** Four comes from a household's size and a screen's width. The
alphabet excludes look-alikes on the same reasoning. No measurement
decides
either, and no test here pretends one does.
<!-- greptile_comment -->
<h3>Greptile Summary</h3>
This PR makes verified email the root account identity, turns Steam into
an attached account connection, adds provider-neutral email-code
delivery, and implements an RFC 8628 device authorization flow backed by
atomic PostgreSQL grant transitions. It also aligns the related
identity, session, and device-grant migrations and adds concurrency and
flow tests.
- Removes Steam and SSH as direct auth-worker sign-in providers.
- Adds email-code account creation and deployment-time mail
configuration checks.
- Adds explicit device approval, denial, polling throttling, client
binding, and one-time redemption.
- Serializes Steam-link cap enforcement and handles concurrent email
uniqueness conflicts.
- Adds and aligns migrations, snapshots, durable device-grant storage,
and `session.claim_token`.
- One non-blocking resend-limit replay issue remains.
<h3>Confidence Score: 5/5</h3>
The PR appears safe to merge, with one non-blocking email-delivery abuse
limitation that should be hardened.
The prior device-token theft, polling race, Steam-link concurrency,
email uniqueness, and session-schema findings are fixed in the current
code; the five corresponding threads were manually resolved without
explanatory replies. The remaining new issue permits bypassing the
intended email send cap through replay of an older provider cookie, but
the resend interval still bounds its rate and it does not compromise
account authentication.
**Files Needing Attention:** packages/auth/src/provider/code.ts
<details open><summary><h3>Security Review</h3></summary>
The device flow now requires an explicit, CSRF-protected confirmation
and uses atomic, client-bound redemption. One lower-impact abuse issue
remains: replaying an older code-provider cookie can bypass the intended
email send cap.
</details>
<h3>Important Files Changed</h3>
| Filename | Overview |
|----------|----------|
| packages/auth/src/issuer.ts | Adds an explicitly confirmed RFC 8628
device flow with client-bound, one-time token redemption. |
| packages/auth/src/provider/code.ts | Adds server-side attempt and send
accounting, but replacement flows leave earlier cookie-referenced
counters replayable. |
| packages/core/src/auth/device-grant.ts | Implements durable device
grants using atomic conditional approval, denial, polling updates, and
consumption. |
| packages/core/src/user/identity.ts | Makes email identity creation
conflict-aware and serializes Steam-link cap enforcement on the user
row. |
| apps/auth/src/index.ts | Reconfigures the deployed issuer around email
sign-in and PostgreSQL-backed desktop device authorization. |
| alchemy.run.ts | Requires complete mail configuration for permanent
stages and explicitly enables code logging only for ephemeral
development stages. |
| packages/core/migrations/0010_device_authorization_grant.sql | Adds
the device-grant enum, table, and unique indexes in alignment with the
Drizzle model and snapshot. |
<h3>Sequence Diagram</h3>
```mermaid
sequenceDiagram
participant D as Desktop client
participant A as Auth issuer
participant B as Browser
participant E as Email provider
participant DB as PostgreSQL
D->>A: POST /device/authorize
A->>DB: Create pending grant
A-->>D: device_code, user_code, interval
B->>A: Enter user_code
A->>E: Send email verification code
B->>A: Verify email code
A-->>B: Display client and user-code confirmation
B->>A: Approve or deny
A->>DB: Atomic terminal transition
loop Until terminal
D->>A: Poll /token
end
A->>DB: Delete-and-return approved grant
A-->>D: Access and refresh tokens
```
<details><summary>Prompt To Fix All With AI</summary>
`````markdown
### Issue 1
packages/auth/src/provider/code.ts:291-297
**Cookie replay bypasses send cap**
Replaying an earlier encrypted provider cookie bypasses `maxSends`. A resend creates a new flow with an incremented counter but leaves the old flow and its lower counter valid, so the old cookie can call `sendCode` again after each resend interval. This permits repeated unsolicited sign-in emails to an attacker-selected address, although the resend interval still limits their rate.
**How this was verified:** The resend check reads only the flow named by the presented cookie, while creating its replacement neither updates nor removes that old flow.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
`````
</details>
<sub>Reviews (3): Last reviewed commit: ["fix(auth): stop a caller
working
through..."](
|
||
|
KAAL1
|
a94b323edb |
fix(nesinit): the relay may not stall the session, and may not buffer without end
Three problems in the relay, all of them found in review. Handing an envelope over waited for room. That loop also carries stop, shutdown and the workload's exit, so a workload slow to read its own mail — or one that never connected — could hold the lifecycle layer still behind it. It never waits now: an envelope that will not fit is dropped, which costs nothing, because what crosses this layer is re-sent when it changes. Envelopes were queued for a workload that was not there. The queue filled with copies that would be stale by the time anyone connected, and filling it was what stalled the session. Nothing is held while the socket has nobody on it. A frame had no maximum length. The workload can write for as long as it likes without ever sending a newline, and the process assembling that is the one the kernel has been told not to kill, so the memory it takes comes out of everything else in the guest. Past 64 KiB the connection is dropped and the relay waits for the next one; the failure says how long the frame got and nothing about what was in it. Also, a tag or a mount point with a nul byte in it was quietly turned into an empty string, so an unmountable descriptor arrived later as a mount failure about something else, after the mount point had already been created. It is refused by name now, before anything is created. The relay's tests grew a harness that waits for the connection to be carried before sending anything down it, because dropping what arrives with nobody connected made "connected" something a test has to establish rather than assume. |
||
|
KAAL1
|
7b99f49f62 |
feat(nesinit): mount what the descriptor names, and relay the layer it cannot read
Builds on the previous change, which had PID 1, the channel and the trait but mounted nothing. The shares are mounted now: a tag names an export, the descriptor names where it lands, and every share goes on nosuid and nodev whether or not it is writable — a share is data handed to the guest, and no descriptor has a way to ask for a setuid binary or a device node in one. Mounting needs privileges a test does not have, so the arguments and flags are derived by a function the tests can assert, which is where the read-only decision lives. Progress is reported in two messages rather than one. A share that did not mount and a command that did not run are not the same incident, and each carries the reason the operating system gave and the path it happened on: a permission error on a named directory can be acted on, where "the share did not mount" cannot. The second layer is relayed and never read. Bytes arrive on the channel in an envelope, cross a unix socket to the workload, and come back the same way. The body is a string rather than nested JSON on purpose: a document this component can index into is a document it can grow to depend on, and then the layer is no longer opaque and the boundary it exists to draw is gone. An envelope is never logged — not the body, not truncated, not at debug level — and the channel name with a byte count is the whole of what may be said about one. The type's Debug is written by hand for the same reason, because a derived one puts the body one careless format string away from a log line. A write to a channel nobody is reading now ends the session the same way a closed read does. A caller that has stopped listening has also stopped being able to say stop, which is one situation and was two outcomes. The guest listens on the relay socket and the workload dials in, which is the convention the other guest sockets already use and removes the startup ordering problem: a workload that is not running yet has simply not connected yet. |
||
|
KAAL1
|
071241f944 |
fix(nesinit): a pid stops being the workload's the moment it is reaped
A pid is only a name for a process until that process is reaped; after that the kernel may hand the same number to something else. The handle kept the number, so a stop or a kill issued during shutdown — which every session outcome reaches — could land on a process nobody meant, and the one aimed at the workload would have been a SIGKILL. The registry now clears the flag as it delivers the exit, in the same call, and nothing signals a pid whose flag is down. Waiting for the workload on the way out takes the same answer: already reaped is already gone. There is a window left, between the reap and the delivery, and closing it entirely needs a handle the kernel keeps rather than a number. Recorded rather than papered over. |
||
|
Wanjohi
|
355d1492d9 |
fix(auth): give a sign-in code a budget of guesses and a short life
A six-digit code has a million values, and nothing was counting how many of them a caller tried. The code travelled in an encrypted cookie the caller held, verification compared against that cookie, and a wrong answer simply re-rendered the form. Nobody has to be the person the code was mailed to: type somebody else's address into the first screen and the code goes to their mailbox while the cookie stays with you. At that point the only thing between a stranger and an account is a million requests, and the constant-time comparison protecting the code was guarding a door you could just keep knocking on. Guesses are now counted on the server, under a name that changes with every code. That placement is the point: a counter kept beside the code, in the cookie, is a counter the guesser can wind back by replaying an older copy. Starting over is still allowed and still costs a fresh code sent to the mailbox being aimed at, which is where somebody notices. A correct code spends its record too, so its remaining guesses do not carry into the next one. The cookie also lived for twenty-four hours, which made the pin a password with a million possible values and a day to try them. Ten minutes now, and the code stops being accepted when the clock says so rather than when the cookie happens to go away. Resend had no limit either, so the button was a way to mail a stranger as fast as requests go out. Codes to one address are spaced, and one attempt at signing in can only ask for so many. Both refusals say the same thing on purpose. Which of the two it was is a fact about somebody else's mailbox. |
||
|
Wanjohi
|
36179150a1 |
fix(auth): make a device sign-in an answer somebody gave
Anybody could ask for a device code and be handed a link with the user code already in it. Following that link started a sign-in, and finishing the sign-in approved the grant. So sending somebody the link was enough: they saw an ordinary sign-in prompt, completed it, and whoever kept the device code polled and collected their access and refresh tokens. The victim never saw a question, because there was not one. There is now. Signing in says who the browser belongs to; it does not say the person meant to hand an account to a program somewhere else. Those are two questions and only the second authorizes anything, so the flow ends at a page that names the program, shows the code back so it can be compared with what the device is displaying, and offers Approve and Deny. Approving is a POST carrying a value from the cookie, so another site cannot submit it on somebody's behalf. Denial moved onto the same page: it used to be a GET anyone could fire, which meant a link scanner could cancel a real sign-in and a stranger with a user code could grief one. Three more things that were wrong underneath. The grant was read, modified and written back as a whole record. A poll that read a pending grant and then wrote its bookkeeping erased an approval that landed in between, and the client polled a dead grant until it expired. Grants moved to a table, where approving is one conditional update and redeeming is one delete that returns what it deleted, so neither party can undo the other and two polls cannot both be served. Tokens were minted when the person clicked and left sitting in storage until collected. They are minted at redemption now, so the lifetime the client is told about starts when it receives them, and a grant nobody collects leaves no usable refresh token behind. The client identifier was never checked, at either end. It is validated when the grant is created and has to match when the code is redeemed — without that, a leaked code is redeemable by anyone, and the identifier the token carries is whatever the last caller claimed. The device code is also stored as a hash now, since it is the credential the tokens are handed to. The store is an interface because the issuer cannot reach the database, and because the guarantees are the point: every method is one operation, and no caller reads a grant, decides, and writes it back. |
||
|
KAAL1
|
cb8f37a0e4 |
fix(nesinit): one thing reaps, and the workload stops alone
Three problems in the shutdown and reaping paths, all of them found in review. Reaping and waiting cannot be two mechanisms. `waitpid(-1, ...)` collects any child, so the reaper and a caller waiting on its own child race for the same status, and whichever loses gets nothing — losing the workload's exit, which is the one thing this component exists to report. The reaper is now the only waiter and hands each exit to whoever asked for that pid. Registering interest holds the same lock the delivery takes, so a child that exits before its caller is registered is still delivered rather than dropped; there is a test that fails without that. Killing the workload killed everything. `kill(-1, SIGKILL)` is every process init may signal, so a workload that overstayed its grace period took the guest's services with it, before the ordered stop the shutdown promises them had even started. It signals the one pid now. The shutdown had no workload to stop. It built a fresh handle with no pid, so the graceful stop was a no-op and the workload only died in the sweep that follows — which is exactly the order this was written to avoid. The handle the session used is now the handle the shutdown uses, and waiting for the workload waits for that pid rather than for any child to leave. |
||
|
Wanjohi
|
2c4e9d9b0b |
fix(auth): refuse to send a sign-in code rather than log one
The rule was "throw when the environment says production, otherwise log the code and carry on". The deployment sets no such marker, so the branch that ran was the developer one: every recipient and every usable sign-in code printed to a retained log, the screen reporting success, and nobody receiving anything. That is what a fail-open default costs. The deployment that forgets its mail settings is exactly the deployment with no marker saying it is a real one, so it takes the lenient branch precisely when it should not. Turned around: printing a live code is asked for by name and anything else is an error, so absence of configuration is a refusal instead of an assumption. Two settings out of three is also an error now, because it means somebody is halfway through wiring a provider up and a quiet fallback would hide the missing half. Stages anyone else can reach are checked at deploy time, so a missing setting stops the deploy with the name of the variable it wanted rather than surfacing later as a person waiting for mail that never comes. |
||
|
Wanjohi
|
affe1e3c73 |
refactor(auth): serve one provider, and make it the email one
Signing in with a gaming account or with an SSH key could both bring a user into existence. That makes an account only as recoverable as the thing that created it, and gives one person as many accounts as they have gaming logins — neither of which is what an account is supposed to be now that verifying an address is what creates one. Both are unwired rather than deleted. The provider implementations stay where they are, because connecting a gaming account is still something this product does; it just does it from the API, against a user who already exists, which is a connection hanging off an identity rather than an identity of its own. The worker test followed: it exercised the two flows that are gone, and now covers the one that is left plus an assertion that the other two are not routed, so they cannot come back quietly. |
||
|
KAAL1
|
a461cbafa5 |
feat(nesinit): PID 1 for a box — reaping, ordered shutdown, and one channel out
A microVM has no init unless something is it, and three of the jobs belong to nothing else in the guest: reaping whatever the workload orphans, turning a signal into an ordered shutdown, and being the guest end of the one channel out. None of it knows what it is running. The guest dials out on a fixed vsock port, says its protocol version first, is handed one boot descriptor — a command line, shares, output geometry, and what an exit means — and carries that out. There is no code path that branches on which workload started, which is the property the component exists to keep. It reports and does not supervise. When the workload ends, the exit goes up the channel and the session is over; `on_exit` says what the exit means, and starting something again is a decision for the end that can see whether restarting is repair or a loop. A signalled workload is reported as signalled with no exit code, because reporting 0 for a killed process makes a kill look like a clean run. Two seams keep this testable without a VM, which is the reason for both of them. Reaping runs against real forked children, with the subreaper bit making a test process inherit orphans the way PID 1 does. The channel is generic over the byte stream, so the exchange is driven over an in-memory pipe — the transport contributes nothing to the protocol beyond ordering and framing. The lifecycle types live in nesprotocol behind a feature, off by default: both ends of the channel read one definition and cannot drift from it silently, while the media components keep building without serde. Mounting shares is not implemented in this build. The descriptor's mounts are refused rather than ignored — a workload started without the shares it was promised fails later, somewhere else, for a reason nobody can see from here. |
||
|
Wanjohi
|
96b0cf8111 |
feat(auth): sign in with an email address
Wires the pin-code provider, which existed and was never reachable, and makes it the only branch that can create an account. Steam now resolves an existing connection instead of minting a user from a persona, and refuses when there is no account behind it — which is an answer the interface renders rather than an implicit signup. Delivery is a small provider-neutral POST rather than a vendor SDK: configure an endpoint, a key and a from address. With none of them set it logs the code outside production so a local sign-in works, and throws in production, because a screen that says "check your email" when nothing was sent leaves someone waiting instead of telling anybody. A person who has only ever signed in by email has no connected account, and the token says so with an empty value — the same one a server-to-server caller has always carried. |
||
|
Wanjohi
|
ecebc528ae |
feat(api): the session endpoint, and a claim that only one caller can win (#317)
## What this is
`POST /session`, `GET /session/:id`, and the three endpoints the host
agent
calls: `GET /machine/jobs`, `POST /session/:id/state`, `POST
/session/:id/ticket`.
Core had `Session` and `Box` and no HTTP surface at all; this adds the
surface
and the two things core was missing to support it safely.
### The access rule
An agent may only see or touch a run whose box is placed on its own
hardware,
and that is a `where` clause on all three agent endpoints — not a check
sitting
next to the query, and not the agent asking politely for its own work.
Host
credentials are long-lived secrets on hardware in somebody's living
room, so
the blast radius of one leaking is decided by the join and nowhere else.
`Session.listJobsForMachine`, `forMachine`, `compareAndSetState` and
`publishTicket` all scope to the machine inside the statement. The
compare-and-set is scoped there too, and not only by the read above it:
a
caller checking first and the write being scoped are different
properties, and
only one of them survives somebody adding a second caller.
"No such run" and "not your run" are the same 403 with the same body, so
reporting states at ids cannot be used to discover which ids exist.
### The claim
`Session.setState` updated on the id alone. Two agents polling the same
run
both succeeded and both started the same box. There is one host today,
which
is exactly why that would have been built wrong and stayed wrong.
`Session.compareAndSetState` puts the state being moved *out of* into
the
`where` clause, so the database picks the winner and the loser matches
zero
rows. `Session.transition` layers the classification on top and returns
one of
five outcomes, which the route maps:
| Case | Answer |
|---|---|
| same host re-reports a state it already reported | `200`, nothing
changes |
| a different host reports anything | `403` |
| a transition not in the table | `409`, row does not move |
| a legal transition something else won first | `409` |
| it happened | `200` |
`setState` is left exactly as it was — it is the unscoped primitive the
core
tests already pin, and the new path is additive.
### Placement
`POST /session` makes no placement decision. A box names its hardware,
so a
run inherits it by join, and the request body is `.strict()` so that
naming a
machine there is a validation error rather than a field quietly ignored.
The interface went where boxes are actually placed: `Placement.Placer`
in
`packages/core/src/box/placement.ts`, with `Placement.onlyHost` as the
implementation and `Box.createPlaced` as the one caller. A real
scheduler
replaces that file and nothing else.
## The test failing first
Tests were written and run against unmodified code. Verbatim, trimmed to
the
result lines (the full log is stack traces for the same failures):
```
(fail) POST /session > a request creates the job, in the envelope both ends read [199.00ms]
(fail) POST /session > creating a session makes no placement decision [55.00ms]
(fail) POST /session > a box somebody else owns is not there to run [106.00ms]
(fail) POST /session > a box already running refuses a second run rather than picking one [51.00ms]
(fail) POST /session > you can only play as an account you have linked [85.00ms]
(fail) POST /session > a host cannot ask for a session on its owner’s behalf [54.00ms]
(fail) POST /session > requesting a session requires a signed-in person [1.00ms]
(fail) GET /session/:id > the owner reads their own run, ticket and all [47.00ms]
(fail) GET /session/:id > somebody else’s run is not visible, and neither is its absence [97.00ms]
(fail) GET /session/:id > reading a run requires a signed-in person [1.00ms]
(fail) GET /machine/jobs > a host is handed the work for its own boxes, with the kind on the wire [50.00ms]
(fail) GET /machine/jobs > a host never sees work for a box on other hardware [84.00ms]
(fail) GET /machine/jobs > bad credentials are indistinguishable from none [41.00ms]
(fail) GET /machine/jobs > a person cannot poll for jobs [44.00ms]
(fail) POST /session/:id/state > the claim moves the row, and the job stops being offered [41.00ms]
(fail) POST /session/:id/state > the same host re-reporting a state it already reported is fine [54.00ms]
(fail) POST /session/:id/state > a different host reporting anything is refused, and learns nothing [99.00ms]
(fail) POST /session/:id/state > a transition that is not allowed is a conflict, and the row stays put [47.00ms]
(fail) POST /session/:id/state > a stopped run cannot be started again [47.00ms]
(fail) POST /session/:id/state > a duplicate live report does not extend a run somebody is billed for [44.00ms]
(fail) POST /session/:id/state > a state nobody defined is a validation error, not a conflict [49.00ms]
(fail) POST /session/:id/state > a person cannot report a state on their own session [48.00ms]
(fail) POST /session/:id/ticket > a later ticket replaces the first, because it is a better address [44.00ms]
(fail) POST /session/:id/ticket > a different host cannot publish an address for someone else’s run [75.00ms]
(fail) POST /session/:id/ticket > a stopped run has no address to publish [48.00ms]
(fail) POST /session/:id/ticket > a ticket has to say something [47.00ms]
(fail) Session routes in the spec > every path a caller needs is documented [48.00ms]
error: Cannot find module './placement.js' from '/home/…/packages/core/src/box/placement.test.ts'
(fail) Session jobs > a requested session is the job, and it carries its kind [37.00ms]
(fail) Session jobs > a job belongs to the machine its box is placed on and to no other [71.00ms]
(fail) Session jobs > only a requested session is work; a claimed one is not offered again [34.00ms]
(fail) Session claim > the claim is a compare-and-set, so the second attempt finds nothing to move [37.00ms]
(fail) Session claim > a machine that is not the box’s host cannot move the row [72.00ms]
(fail) Session claim > a session that does not exist is refused the same way as one that is not yours [31.00ms]
(fail) Session claim > re-reporting the state you already reported changes nothing [37.00ms]
(fail) Session claim > a transition off the table is refused and the row does not move [36.00ms]
(fail) Session claim > the timestamps survive a duplicate report, which is what billing rests on [33.00ms]
(fail) Session claim > publishing a ticket is scoped to the host too [73.00ms]
(fail) Session claim > a stopped session has no address to publish [36.00ms]
7 pass
39 fail
1 error
Ran 46 tests across 3 files. [2.88s]
```
(46 rather than 49 because the three `Placement` tests never ran — the
module
they import did not exist.)
## And passing after
```
49 pass
0 fail
126 expect() calls
Ran 49 tests across 3 files. [3.72s]
```
Full suite, against a fresh migrated database on `DATABASE_URL` and
`TEST_DATABASE_URL`:
```
181 pass
0 fail
479 expect() calls
Ran 181 tests across 16 files. [7.89s]
```
Baseline before this branch was `138 pass, 0 fail, 371 expect() calls`
across
14 files, so 43 tests and 108 assertions are new and nothing regressed.
`tsc --noEmit -p apps/api` reports the same five pre-existing errors in
`app/utils/hook.ts` and `app/utils/validator.ts` as it does on `dev`,
and none
in the files this branch adds.
## Shared files touched
- `apps/api/app/index.ts` — three lines: one import, and two mounts
(`/session`, plus `SessionApi.machineRoute` at `/machine`).
- `packages/core/src/box/index.ts` — one import and `Box.createPlaced`
added.
Nothing existing changed.
- `packages/core/src/session/session.test.ts` — `scene()` now also
returns
`machineId`; two new `describe` blocks appended.
- `packages/core/src/examples.ts` was **not** touched; the job schema's
examples reuse the existing `Examples.Session`, `Examples.Box` and
`Examples.Game` values.
No migration was created and none was needed.
## Judgement calls
1. **`GET /machine/jobs` lives in `session.ts`, mounted at `/machine`.**
The
path belongs under the prefix a host already uses for everything it asks
about itself, but the handler is this lane's code, so it is a second
Hono
instance (`SessionApi.machineRoute`) rather than an edit to
`machine.ts`.
2. **A job's payload beyond `kind` was not specified anywhere**, so it
is:
`kind`, `sessionId`, `boxId`, `boxTier`, `gameId`, `steamAppId`,
`linkedAccountId`. `steamAppId` is in there because the agent launches
by
store id and not by our internal one, and `boxTier` because the tier
also
sets output geometry. **This is the most likely place for the two
implementations to disagree** — see the disagreement note below.
3. **A run's terminal states refuse a ticket** (`409`). Nothing said
what
publishing an address for a stopped run should do; writing one can only
mislead a client that is still polling.
4. **A state report accepts only `starting`, `live`, `ended`,
`failed`.**
`requested` is written once, at creation, so reporting it is a `400`
rather
than a no-op.
5. **A box already running refuses a second run** with `409`. Not stated
in so
many words, but `Session.activeForBox` documents itself as the query
that
should start refusing rather than picking a winner silently, and this is
the
caller that would otherwise have made it pick.
6. **`Placement.onlyHost` refuses when there is more than one
candidate**
rather than taking the first row. Picking would be a scheduling policy
invented by accident and impossible to find later. It also refuses when
there are none, because `box.machineId` is not nullable and a placer
that
cannot answer must say so.
7. **`POST /session` returns `201`**, and its body is `.strict()`.
8. **Person-facing 404s, agent-facing 403s.** Both are the settled shape
for
their realm and both are asserted identical between "does not exist" and
"not yours".
9. **`POST /session` accepts an explicit `linkedAccountId`**, defaulting
to the
one the caller's session carries. A personal access token carries none,
so
requiring the session to supply it would make that credential unable to
start a run; the account is verified to belong to the caller either way.
## Where the specification was ambiguous or came out wrong
- **The lost-claim 409 is not reachable through the HTTP endpoint as
specified.** A box names exactly one machine, so both racers for a given
run
authenticate as *the same* machine — and the same machine re-reporting a
state it already reported is specified as `200`. Which of the two
answers a
caller gets therefore depends only on whether its read happened before
or
after the winner's write: concurrent gets `409`, a sequential retry gets
`200`. That is a defensible reading and it is what is implemented, but
it
means the two rules are distinguished by timing and not by anything a
caller
can see. If a second agent process per host is ever real, this needs a
claim
token in the request rather than a timing accident.
- **The job payload is unspecified**, which is the one part of the wire
the
agent parses and the one thing a document written before two
implementations
exist was supposed to pin. If the other end's test expects different
field
names, that is this gap and not a bug in either implementation, and it
should
be settled in the specification before either side moves.
- **Nothing said whether a person may report a state.** Implemented as
`403`:
terminal states are the agent's to write, and a person closing their app
is
a different fact from a run that stopped. That reading is also why
cancellation, listed below, has
nowhere to live yet.
## What this does not verify
- **No live round trip.** Every assertion here goes through
`app.request`
against a real Postgres. No real host agent has ever called any of these
five endpoints, and no client has ever read a ticket out of one.
- **The claim is not verified under concurrency.** The compare-and-set
is
pinned by stepping two attempts in sequence and showing the second
matches
zero rows. Two agents actually racing, on two connections, is not tested
and
cannot be from a single-process test.
- **The wire shape is asserted against this lane's reading of the
specification, not against the other end.** That is deliberate — each
end
writes its own test — but it means a shape agreement has *not* been
demonstrated. Both tests passing separately is the evidence, and it does
not
exist yet.
- **No ticket in this repository has ever been a real iroh ticket.**
They are
opaque strings to every assertion here; that the value survives a round
trip
says nothing about whether anything could connect to it.
- **`steamAppId` reaching the agent is untested end to end.** It is
asserted
present and correct in the poll response and nothing consumes it.
- **Nothing reaps a stuck `requested`.** A run whose host never polls
sits
there forever, and no screen distinguishes that from "starting soon".
Left
open deliberately: a timeout here would hide the gap rather than close
it.
- **Cancellation has no path.** A person who closes their app between
`requested` and `starting` leaves work that will be claimed and started
into
an empty room. There is no endpoint for it and this branch did not
invent
one.
- **Nobody writes `ended` when the agent itself dies.** The only writer
of
terminal states is the agent, so a host that loses power leaves a `live`
run
that keeps billing. The fix belongs with whatever decides host liveness
has
authority over a run's state, and that does not exist.
`Machine.isOnline`
exists but nothing joins it to a session.
- **Placement is verified only for the single-host case and the two
refusals.**
No test exercises a real alternative placer beyond an inline stub, and
nothing calls `Box.createPlaced` from an endpoint, because there is no
box
creation endpoint yet.
<!-- greptile_comment -->
<h3>Greptile Summary</h3>
This PR adds the HTTP and core-domain session lifecycle, including user
session requests, machine job polling, host-scoped state transitions and
ticket publication, placement support, box-state synchronization, and a
database constraint ensuring one active session per box.
- Adds authenticated user and machine session endpoints with
resource-level ownership and host scoping.
- Uses compare-and-set state transitions so only one caller can claim a
requested session.
- Adds a partial unique index and duplicate-data repair migration to
enforce one active run per box.
- Clears connection tickets when sessions become terminal.
- Adds placement abstractions and extensive API, domain, migration, and
concurrency-oriented tests.
- The latest changes document and test that ownership is checked per
user rather than per selected linked account; they do not resolve the
previously reported multi-account launch failure.
<h3>Confidence Score: 4/5</h3>
The PR is not yet safe to merge because selecting a different linked
account can still launch a game that only another account owns.
The previously reported ownership issue remains unresolved: the current
request path checks the game against the user-wide library and
separately verifies only that the selected linked account belongs to
that user. The new test explicitly accepts this behavior, so a user with
multiple Steam links can request a game through an account that does not
own it, causing the host launch to fail. The three resolved previous
findings are fully addressed or manually resolved and do not remain
outstanding.
**Files Needing Attention:** apps/api/app/routes/session.ts
<h3>Important Files Changed</h3>
| Filename | Overview |
|----------|----------|
| apps/api/app/routes/session.ts | Adds user and host session endpoints
with scoped authorization and validation, but the previously reported
selected-account ownership mismatch remains. |
| packages/core/src/session/index.ts | Adds job queries, host-scoped
compare-and-set transitions, ticket lifecycle handling, box-state
synchronization, and active-session conflict translation. |
| packages/core/src/session/session.sql.ts | Declares the partial unique
index enforcing at most one undeleted, unstopped session per box. |
| packages/core/migrations/0008_session_one_active_run_per_box.sql |
Repairs pre-existing duplicate active sessions, clears their stale
tickets, and creates the active-session unique index. |
| packages/core/src/box/placement.ts | Introduces a placement seam that
selects the sole owner host and refuses ambiguous or unavailable
placement. |
| apps/api/test/session.test.ts | Adds broad endpoint coverage and now
explicitly demonstrates the unresolved per-user rather than
per-linked-account ownership behavior. |
<h3>Sequence Diagram</h3>
```mermaid
sequenceDiagram
participant U as User client
participant A as Session API
participant D as Core domain
participant DB as PostgreSQL
participant H as Host agent
U->>A: POST /session
A->>D: Validate box, game, library, and linked account
D->>DB: Insert requested session
DB-->>D: Enforce one active session per box
H->>A: GET /machine/jobs
A->>D: List requested sessions scoped to host
D->>DB: Join session through box.machineId
H->>A: POST /session/:id/state (starting)
A->>D: Compare-and-set state for host
D->>DB: Atomic scoped transition
H->>A: POST /session/:id/ticket
A->>D: Publish ticket while addressable
U->>A: GET /session/:id
A-->>U: Current state and ticket
H->>A: POST /session/:id/state (ended/failed)
D->>DB: Set terminal state and clear ticket
```
<sub>Reviews (4): Last reviewed commit: ["fix(api): say what the library
check
act..."](
|
||
|
Wanjohi
|
7bdca1240f |
fix(api): say what the library check actually proves
A library entry records the person, not the account the games were synced from, and `POST /library/sync` is not even told which account a list came from. So the ownership check added for session requests asks "has somebody this person linked got this game?" and not "does the account about to play own it?" — for the one Steam account most people have those are the same sentence, and for two they are not. Confirmed rather than reasoned about: a person with two Steam links, a game synced at person level, and a request naming the second account is accepted today. The check stays, because it still turns a box that boots, tries to launch and fails minutes later into an immediate refusal, and it never refuses on account grounds that the data cannot support. What changes is the comment, which claimed the stronger property, and a test that pins the gap so it is found deliberately rather than by surprise. Closing it properly means recording the linked account on a library entry: a column, a sync contract that says which account a list belongs to, a uniqueness rule per account rather than per person, and a backfill with no correct answer for rows already written. That is a decision about what a library is, and inferring it here would be the kind of modelling taken by accident that this branch refuses elsewhere. |
||
|
Wanjohi
|
51dabddbd8 |
fix(api): a run drives the box under it, and needs a game and a claim
Four things the session endpoints did not do, or did wrongly. The box had three states and nothing wrote them. A box read `created` while a run on it was `live`, so every screen showing a person what their hardware is doing was reading a column no code had ever moved. A run reaching `live` now makes its box `running`, and a terminal run stops it: `ended` cleanly, `failed` not, carrying the reason the agent gave. Not every run state maps — a box has no `starting` on purpose, because that transition is synchronous from the agent's side and a state nobody sets is a state that lies. Both writes are one transaction, since "this run is live" and "the box under it is running" are one fact in two tables, and a box stuck `running` with nothing on it has nothing to correct it. `POST /session` accepted any game in the catalog. A run launches as a Steam account that has to own the game, so one outside the caller's library is a box that starts, tries to launch and fails minutes later with nothing to point at; it is now refused up front. Told apart from a game that does not exist rather than hidden, because the catalog is public and "you do not own this" is a sentence a person can act on. The library is a synced copy, so this refuses a game bought since the last sync — that is a staleness bug in the sync, not a reason to start runs that cannot work. Publishing a ticket only refused terminal runs, so a host could publish an address for a run it had never claimed. A ticket is the address of something being brought up, so only `starting` and `live` accept one, and the state is in the write rather than only in the check above it. The two refusals stay separate answers because they are different mistakes: one agent skipped a step, the other has nothing left to reach. The migration that adds the one-active-run index stopped older duplicate runs without clearing the ticket they had published, which is the invariant that same migration exists to establish. It clears it now, verified against a box carrying two unstopped runs. Nine tests, each checked against the unfixed code first. |
||
|
Wanjohi
|
0d8630379b |
fix(api): a box gets one run, and a stopped run keeps no address
Two invariants the session endpoint stated but did not hold. A box runs one thing at a time. `POST /session` read `activeForBox` and refused when something was already running, but the read and the insert are two statements with nothing between them: two requests that both saw "nothing is running" each got a row, and the job poll then handed the host the same box to start twice. Demonstrated at 2 rows and 2 jobs from one box. That is the failure the state claim exists to prevent, one step earlier, and it takes the same answer — a partial unique index on the predicate the read asks about, so the database refuses the second insert. `Session.request` turns that refusal into the same 409 in the same words, so a caller cannot tell which of the two caught it. The migration resolves any existing duplicates before creating the index, keeping each box's newest unstopped run because that is the one a person is waiting on, and stopping the rest rather than deleting them. Separately, a run that reached `ended` or `failed` kept the last ticket it published. Publishing a new one is already refused, so the stale address was both the only ticket a client could read for a dead run and the one nothing was allowed to replace — and a client that polls would dial it. Terminal transitions now clear it, in `setState` as well as in the compare-and-set, so the invariant does not depend on which writer stopped the run. Seven tests, each checked against the unfixed code first. The published descriptions for the ticket field and the read endpoint now say that a stopped run has no address. |
||
|
KAAL1 (Bingus)
|
e70f05e245 |
fix(nescope): make HDR reachable — start XWayland, advertise the opaque FourCCs (#314)
Three related fixes. Together they take HDR from unreachable to working
end to end on the XWayland path.
## XWayland was never started
Three lines had been commented out since the initial import: the call
that spawns XWayland, the guard that waits for it, and the `DISPLAY`
handed to the child. Every game therefore launched as a native Wayland
client. Nothing reported it -- the compositor still logged the X display
it was telling clients to point at, which is why it read as working.
That is also why HDR never fired. The colour space is signalled over a
protocol whose Vulkan layer lives in the game process and finds the
compositor through the X11 root window, so the one path able to carry it
was the one path no game was on. `ENABLE_GAMESCOPE_WSI` and `DXVK_HDR`
were already being set, which switched that layer on and then handed it
a
display it could not use.
Restoring the guard also fixes the ordering it was written for: the
launch
now happens after XWayland reports ready rather than ~40ms before it.
## Mesa was dropping every format we advertised alpha-only
Mesa tracks two flags per VkFormat -- one contributed by a format alpha
FourCC, one by its opaque FourCC -- and skips any format carrying only
one:
```c
if (!(disp_fmt->flags & WSI_WL_FMT_ALPHA) ||
!(disp_fmt->flags & WSI_WL_FMT_OPAQUE))
continue;
```
We advertised `ARGB8888` and `XRGB8888`, so `B8G8R8A8` survived and made
the list look like it was working. Everything else was alpha-only and
was
dropped in silence -- `ABGR8888` had been advertised all along while
`R8G8B8A8` never once appeared on a surface. Adding the opaque spellings
takes the surface from 6 formats to 21 and restores the three that carry
HDR.
The comment above that list claimed it was for XWayland DRI3 and that a
game swapchain format was independent of it. It was the opposite: the
list decides what a game can select, and deleting an entry removes that
format from every client.
## Verified against swapchains, not format lists
A client asking for `A2B10G10R10` + `HDR10_ST2084` now gets a swapchain
and the compositor is told colorspace `1000104008`; one asking for
`R16G16B16A16_SFLOAT` + scRGB linear gets `1000104002`. Previously both
were refused at creation -- the WSI layer re-checks the requested format
against the driver own surface list, so the colour space and the pixel
format arrive from two different places and only one was being supplied.
`apps/nescope/scripts/verify-hdr-formats.sh` asks what a client is
offered
from inside a child process, keeping the XCB and Wayland surfaces apart
since a game presents through the XCB one. The default mode guards both
halves of what the compositor controls; `--expect-layer` states the full
target and passes once a WSI layer is present. No new dependencies
(`vulkaninfo` + `python3`).
## Still open
HDR is XWayland-only, documented as a FIXME in `hdr.rs`. A WSI layer
binds
the swapchain factory on its own Wayland connection while a native
client
surface lives on the client one, and object IDs do not cross
connections.
The FIXME records the fix both reference implementations point at, and
the
trap to avoid when we take it: gating format injection on "the
compositor
supports HDR" rather than on being able to signal the surface hands a
client PQ pixels that arrive tagged as SDR, with nothing reporting an
error.
nescope ships no WSI layer of its own; the above was verified with the
stock gamescope one, which drives our protocol unmodified.
<!-- greptile_comment -->
<h3>Greptile Summary</h3>
This PR makes HDR-capable native Wayland presentation reachable, adds
the alpha/opaque DMA-BUF FourCC pairs Mesa requires, makes XWayland an
explicit compatibility mode, and adds an HDR surface-format diagnostic.
- Starts XWayland only with `--xwayland`, waits for readiness before
launching the child, and stops cleanly if startup fails or times out.
- Routes Proton through Wayland unconditionally while retaining
`DXVK_HDR` as an HDR-specific setting.
- Advertises paired alpha and opaque FourCC variants with portable
modifiers.
- Separates XCB and Wayland probe results, selects one hardware adapter,
and distinguishes probe failures from format regressions.
- Documents the limitations of the legacy gamescope WSI path and the
external-layer dependency.
<h3>Confidence Score: 5/5</h3>
The PR appears safe to merge; no outstanding correctness, security, or
repository-rule issue remains.
All previous findings are resolved in the current code, including the
XWayland failure lifecycle, removal of unsupported vendor modifiers,
corrected HDR documentation, reliable diagnostic failure handling,
per-GPU format selection, and unconditional Proton Wayland routing. The
changes since the previous review preserve diagnostic output handling
without introducing a new failure.
<h3>Important Files Changed</h3>
| Filename | Overview |
|----------|----------|
| apps/nescope/src/main.rs | Adds opt-in XWayland lifecycle handling,
readiness timeout, conditional DISPLAY propagation, and unconditional
Proton Wayland routing; the previous launch-environment finding is
fixed. |
| apps/nescope/src/state.rs | Stops the event loop on reported XWayland
startup failure and advertises portable paired alpha/opaque DMA-BUF
formats without vendor-specific modifiers. |
| apps/nescope/src/hdr.rs | Documents the working native Wayland HDR
path and accurately distinguishes it from the external, deliberately
disabled gamescope WSI route. |
| apps/nescope/scripts/verify-hdr-formats.sh | Adds a diagnostic that
keeps GPU and surface paths separate and now preserves the intended exit
behavior when filtered Vulkan diagnostics contain no matching lines. |
<h3>Flowchart</h3>
```mermaid
%%{init: {'theme': 'neutral'}}%%
flowchart LR
Launch[nescope child launch] --> Mode{--xwayland?}
Mode -->|No| Wayland[Native Wayland surface]
Mode -->|Yes| Wait[Start and await XWayland]
Wait -->|Ready| XCB[XCB / XWayland surface]
Wait -->|Error or 10s timeout| Stop[Log failure and stop]
Wayland --> Formats[Paired alpha and opaque FourCCs]
Formats --> HDR[HDR10 and scRGB formats available]
XCB --> SDR[X11 compatibility path without native HDR]
Proton[Proton child] -->|PROTON_ENABLE_WAYLAND=1| Wayland
```
<sub>Reviews (9): Last reviewed commit: ["nescope/scripts: guard the
diagnostic
pi..."](
|
||
|
KAAL1 (Bingus)
|
200bc9c75f |
fix(nescapture): stop defaulting unreadable formats to BGRA (#315)
Stacked on #313, which this depends on.
Found while checking whether the 10-bit HDR path actually works now that
a
client can obtain an HDR swapchain (see #314). **It does** — verified
end to
end rather than from the format list: a client requesting `A2B10G10R10`
+
`HDR10_ST2084` produces
```
pix_fmt=yuv420p10le color_range=pc
color_space=bt2020nc color_transfer=smpte2084 color_primaries=bt2020
```
which is a correctly tagged HDR10 stream, and the first pixel-level HDR
check
here with 10-bit rather than 8-bit input. Three ways it could have gone
wrong
instead.
## Unrecognised formats defaulted to BGRA
`vk_format_to_input_format` returned `BGRA` for anything it did not
know, which
reads a packed 10-bit or FP16 buffer as eight-bit channels. It now
returns
`None`, and the encode loop drops those frames with one log line per
format.
A stalled stream is a complaint. A stream at full frame rate carrying
nonsense
is not, and that is the failure this area keeps producing.
## Bit depth and input format had drifted apart
They were two separate matches on the same `VkFormat`. `A2R10G10B10`
counted as
ten-bit in one and had no entry in the other, so it fell back to
eight-bit
BGRA — the encoder configured for ten bits while the converter read
eight.
Depth now derives from the input format, so that disagreement is
unrepresentable. `A2R10G10B10` stays unmapped deliberately: a WSI layer
offers
it as one of its HDR pairs and the compositor dmabuf list advertises it,
but
the converter has no red-first 10-bit input, so there is nothing correct
to map
it to.
## The CPU fallback could not read either HDR format
It read four bytes per pixel for every format and encoded eight-bit
regardless, so a packed 10-bit buffer became garbage and an FP16 one was
half
an image of misread floats. It now refuses what it cannot read.
## Recorded, not fixed: the colour space we see is not always the one
requested
A FIXME at the point the value is read. A WSI layer rewrites
`imageColorSpace`
to `SRGB_NONLINEAR` before calling down — deliberately, since it carries
the
real colour space to the compositor out of band. We sit below it, so we
read
the rewrite. Measured, all three lines from one run:
```
[Gamescope WSI] ... colorspace: VK_COLOR_SPACE_HDR10_ST2084_EXT
swapchain created — format=A2B10G10R10 colorspace=SRGB_NONLINEAR
(re)init encoder: H265 Yuv420 Ten Bt709 → P010
```
Ten-bit right, BT.709 wrong: PQ samples encoded and tagged as SDR. The
same
client *without* the layer gives `Ten Bt2020` and an smpte2084 stream,
so this
is specific to the layer path — which is the path Proton titles take.
The fix cannot be local; the true colour space only exists in the
compositor,
which does receive it, so it needs a channel from there. Layer ordering
is not
a fix — we do not control it, and the non-layer path still needs the
Vulkan
value. Left out of this PR as a design change rather than a bug fix.
## Verification
- 4 new tests, 12 total, all passing.
- No new clippy warnings (diffed against the base branch).
- 10-bit HDR path: unchanged, still `Ten Bt2020` / smpte2084.
- SDR path: `verify-chain.sh` passes, 835 frames, 8-bit BGRA, brightness
agreement 0.57.
<!-- greptile_comment -->
<h3>Greptile Summary</h3>
This PR makes Vulkan format handling fail safely instead of interpreting
unsupported swapchain buffers as BGRA.
- Maps supported Vulkan formats to explicit converter inputs and derives
bit depth from that mapping.
- Drops unsupported GPU frames with rate-limited logging.
- Rejects unsupported HDR formats in the eight-bit CPU fallback.
- Documents the color-space limitation caused by rewritten WSI metadata.
- Adds tests covering unsupported, eight-bit, 10-bit, and FP16 formats.
<h3>Confidence Score: 5/5</h3>
The PR appears safe to merge, with no new actionable issues introduced
since the previous review.
The changes since the previous review are empty, the sole previous
finding was manually resolved after Greptile conceded it based on the
stacked PR dependency, and the full PR introduces no confirmed rule
violations or remaining correctness failures.
<h3>Important Files Changed</h3>
| Filename | Overview |
|----------|----------|
| apps/nescapture/src/encode.rs | Replaces unsafe BGRA fallback behavior
with explicit format validation, consistent bit-depth derivation,
guarded CPU fallback, and focused tests. |
| apps/nescapture/src/swapchain.rs | Documents the known WSI color-space
rewrite limitation at the point where swapchain metadata is recorded. |
<h3>Flowchart</h3>
```mermaid
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Captured Vulkan frame] --> B{Known converter input?}
B -->|No| C[Log format change and drop frame]
B -->|Yes| D[Derive input format and bit depth]
D --> E{DMA-BUF path available?}
E -->|Yes| F[GPU color conversion and encoding]
E -->|No| G{Eight-bit RGBA or BGRA?}
G -->|Yes| H[CPU conversion and encoding]
G -->|No| I[Return recoverable error]
```
<sub>Reviews (3): Last reviewed commit: ["docs(nescapture): the
colour-space note
..."](
|
||
|
KAAL1 (Bingus)
|
3389e6065f |
fix(nescapture): tag encoded streams full-range to match the samples written (#313)
## The bug
`nescapture` sets the colour converter full-range unconditionally, but
the video usability information carried pixelforge's **default
limited-range flag**. A compliant decoder then expanded 16–235 out of
samples that already covered 0–255 — darkening midtones and clipping
both ends.
pixelforge keeps two separate flags for this, one on the converter and
one on the colour description, and its own documentation says they must
agree. Only the first was being set.
The two lines are about forty apart, each is correct on its own, and the
comment above the second states the right intent while the call below it
does the opposite:
```rust
// GPU framebuffer captures are always full-range — use BT.709 full-range
// so the decoder doesn't apply limited-range expansion.
enc_cfg = enc_cfg.with_color_description(ColorDescription::bt709());
// ^ this constructor is limited-range
```
## Evidence
Measured on a Radeon RX 9060 XT, comparing the encoded result against
the compositor's own readback of the same frames:
| ground truth = `51` | before | after |
|---|---|---|
| flat background, decoded | **`38`** | `49–51` |
| mean luma, capture path vs readback | **10.41 apart** | **0.55 apart**
|
| luma histogram intersection | **0.090** | **0.913** |
| declared `color_range` | `tv` | `pc` |
**The encoded luma is byte-identical before and after** — `Y = 51.00`,
standard deviation `0.00` on both runs. Only the tag changed, which is
what identifies this as a signalling bug rather than a conversion one,
and why nothing short of a comparison against ground truth could see it:
the stream was valid, the frame rate was right, the picture was
recognisable, and every liveness check passed.
The HDR arm (`bt2020_pq`) carried the same defect and is fixed the same
way, but **has not been run** — no 10-bit verification here.
## `scripts/verify-chain.sh`
Runs a Vulkan workload under `nescope` with the layer active and
compares the encoded output against `nescope-shot`'s readback of the
same frames. Two paths that share almost no code see the same content,
so disagreement localises the fault; a single path cannot tell a correct
frame from a plausible-looking wrong one.
**Confirmed it fails when this change is reverted** — both the tag check
and the brightness-agreement check fire.
One note on its thresholds, since it is easy to get backwards: the
not-blank check is a low absolute floor plus a comparison against the
readback's own structure, rather than a fixed number. A fixed number was
tried first and was wrong in the worst way — the **broken** build scored
20.49 on it and the **fixed** build 17.74, because the range defect
stretched contrast and that reads as more detail. How much structure a
correct frame carries depends on what the workload drew, so the only
stable reference is ground truth measured in the same run.
## Not covered
`vkcube` rather than a real workload; 720p, H.264, 8-bit; one card, one
driver. XWayland, HUD detection and real swapchain formats are
untouched.
<!-- greptile_comment -->
<h3>Greptile Summary</h3>
The PR aligns encoded-stream color metadata with the full-range samples
produced by nescapture and updates the CPU fallback to BT.709 full-range
conversion.
- Updates pixelforge and configures matching converter color space,
range, and SDR reference white.
- Corrects Vulkan color-space mapping and adds regression tests for SDR,
HDR, and CPU fallback behavior.
- Adds SDR capture-chain and HDR comparison verification scripts.
<h3>Confidence Score: 5/5</h3>
The PR appears safe to merge.
No blocking failure remains.
<h3>Important Files Changed</h3>
| Filename | Overview |
|----------|----------|
| apps/nescapture/src/encode.rs | Aligns GPU and CPU conversion output
with encoded color metadata and adds focused regression coverage. |
| apps/nescapture/scripts/verify-chain.sh | Adds an end-to-end SDR
verifier using a static corner patch to avoid the previously reported
temporal mismatch. |
| apps/nescapture/scripts/verify-hdr.sh | Adds an HDR comparison harness
for inspecting conversion behavior across builds. |
| apps/nescapture/Cargo.toml | Advances pixelforge to the revision
providing the required color-conversion configuration. |
| Cargo.lock | Records the pixelforge update and resulting transitive
dependency refresh. |
<sub>Reviews (5): Last reviewed commit: ["test(nescapture): check the
HDR
conversi..."](
|
||
|
Wanjohi
|
bbe729e5c7 |
feat(api): the session endpoint, and a claim that only one caller can win
A run of a box had core support and no HTTP surface. This adds both halves of it: a person asks for a run and reads it back, and the host agent the box is placed on is handed the work and reports what happened. The access rule is the point. An agent may only see or touch a run whose box is placed on its own hardware, and that is a `where` clause on every one of the three agent endpoints rather than a check next to them — host credentials are long-lived secrets sitting on hardware in somebody's home, so what one leaking can reach has to be decided by the query. "No such run" and "not your run" are the same refusal, so ids cannot be discovered by reporting states at them. `Session.setState` updated on the id alone, which means two agents polling the same work both succeed and both start the same box. There is one host today, which is exactly why that would have been built wrong and stayed wrong. The state a run is moving out of is now part of the `where` clause, so the database picks the winner; the loser gets a conflict rather than a silent no-op. Three cases that look alike are kept apart: re-reporting a state you already reported changes nothing and is not an error, a transition that does not exist is refused with the run left where it was, and another host reporting anything is forbidden. Asking for a run makes no decision about where it happens — a box already names its hardware, so the run inherits it by join. Placement therefore gets an interface at box creation, where the decision actually is, with the single-host case as its implementation and a deliberate refusal when there is more than one candidate and no policy to choose with. Tests cover the wire shape from both sides, the query scoping, the claim, and the timestamp idempotence a run's billing rests on. |
||
|
Wanjohi
|
aaa1bbd0f4 |
fix(nesdoctor): the closing prose still said "launch times"
Missed when the histogram was corrected: the tool spent a paragraph asking people to send the JSON and described it as holding "installed titles with sizes and launch times", which is the same claim the histogram itself no longer makes. Steam stores when a title was last played, not each time it was launched. Found by running the installer end to end after the release — the shipped 0.3.0 binary prints it. Fixed here for the next one; it is prose in the closing paragraph, not a number anybody acted on. Same wording in the --json flag's doc comment, in the submit URL's comment, and in the consent prompt that asks to read the library at all. That last one matters most of the four: it is what somebody reads before saying yes. |
