github-mirrors/netris-nestri
1
0
Fork 0
mirror of https://github.com/nestriness/nestri.git synced 2026-09-19 17:25:19 +03:00
Code Issues Packages Projects Releases Wiki Activity

fix: A host-only session cookie, and a column for where a host actually is (#331)

Browse Source 
Closes #330.

Two gaps that block a reverse proxy sitting in front of user-owned
hardware and
authenticating browsers on its behalf, plus one thing found on the way
that is
worse than either.

## 1. `machine` records where a host is

`machine` said who owns a host, which team it belongs to and when it was
last
seen, and nothing about how to reach it — so a request arriving for a
machine
could be authorised perfectly and then have nowhere to go.

`endpoint_id` is **reported, never assigned**: a host holds the secret
half of
that identity and is the only thing that can know the public half first,
so it
rides on the beat it already sends as itself. Migration `0013`.

- **Nullable**, because "has never reported one" is a real state that
every host
registered before today is in. Null reads as *not reachable yet*; a
default
  would read as an address and route somewhere wrong.
- **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 the authorisation in front of it
cannot
  catch.
- **Omitting the field leaves the stored value alone.** An agent that
does not
  say where it is has not moved, and an absent field must never read as
"nowhere" — that would take every host shipped before this field off the
map
  on its next beat. There is a test for exactly that.

## 2. The session cookie, and why it is not shaped the way the issue
asked

The issue asked for a `__Host-nestri-session` set on sign-in carrying
the access
token unwrapped. Building it turned up two reasons that cannot work, and
item 3
of the issue is the reason why:

1. **There is nowhere to set it.** The sign-in UI is served by the
issuer on its
own hostname, and the web client has no auth code at all. A `__Host-`
cookie
set at sign-in is host-only *to the issuer* — a host that does not need
one.
2. **The value would be a control-plane credential** sitting on hardware
the
   control plane does not run.

Item 3 said the hand-off had to be settled from both ends and did not
pick a
shape. It is settled now, in the way the issue's own two rules point at:
**the
proxy is an ordinary public OAuth client, one per hostname**, and what
crosses
in the URL is an **authorization code** — single-use, sixty seconds
long,
redeemed exactly once by the store that already exists here, and
exchanged over
a back channel. A code in an access log is worthless by the time anybody
reads
it, which is the hazard the issue named. No credential in a URL, and no
caller-supplied return address.

What this repo owes that flow is one rule, and it is the whole of the
change to
the issuer: **a client id that is a single hostname under the host zone,
whose
`redirect_uri` is `https` and that same hostname at one reserved path,
is an
allowed client.** Everything else keeps the behaviour it had.

Making the client id the hostname is load-bearing rather than tidy. A
token's
`aud` is its client id, so the session that comes back is **bound to the
host it
will live on**, with no change to how tokens are minted — and it is not
a
credential on any other host, nor a control-plane credential at all.
That is a
better answer than "the access token, exactly", and it costs nothing.

## 3. `/authorize` was an open redirector

Found while testing the above. A refused client's `redirect_uri` was
still used
to deliver the refusal — and the check that approves that URI is the one
that
just failed:

```
GET /authorize?client_id=web&redirect_uri=https://somewhere.example/callback
  -> 302 https://somewhere.example/callback?error=unauthorized_client
```

No sign-in required, on the hostname people are asked to type a password
into.
A refusal is now a page here. This is in `packages/auth` and is
independent of
everything above; it is in this PR because the flow above is built on
that path
and shipping one without the other would have been odd.

## Tests

Every case below fails against the unmodified code and passes after.
`379 pass,
0 fail` across `packages` and `apps`.

- `apps/auth/test/allow.test.ts` — the allowed case; a code is never
sent
anywhere but the client id; only the reserved path; `https` only; one
label,
because `a.b.zone` is not a host id; another zone does not get in by
using the
path; the three existing rules unchanged; and the refusal is a page
rather
  than a redirect.
- `packages/core/src/machine/machine.test.ts` — reported and read back,
a
silent beat leaves it alone, four malformed ids refused, two machines
cannot
  claim one id.
- `apps/api/test/heartbeat.test.ts` — a beat with no body is still a
beat, a
  host cannot report where *another* host is, a malformed id is a `400`.

## What this does not verify

- **No end-to-end run against a real browser.** The flow is exercised
from the
proxy's side and from this side separately; nothing has driven a browser
  through sign-in and out the other end.
- **The token's shape is asserted to the written contract, not captured
from a
running issuer.** If minting changes, these tests pass and a signed-in
person
  is redirected to sign in again.
- **`machine_endpoint_id_unique` is not exercised under concurrency.**
Two hosts
reporting the same id in the same instant is a database-level race that
the
  tests assert the *outcome* of, sequentially.
- **Nothing here makes a hand-off silent.** The issuer keeps no session
of its
  own, so a second hostname asks for an email code again. That is not a
regression — nothing anywhere is silent today — but it is the next piece
of
work, and it is what the issue's item 1 will eventually be, on the
issuer's
  own hostname.
- Pre-existing `tsc` errors remain in
`apps/api/app/utils/{hook,validator}.ts`
  and `packages/core/src/session/session.test.ts`; none is touched here.




<!-- greptile_comment -->

<h3>Greptile Summary</h3>

This PR adds a durable, host-reported endpoint identity and permits
tightly constrained OAuth callbacks for individual hostnames. It also
prevents rejected OAuth clients from controlling the error redirect.
- Adds a nullable, unique machine endpoint ID with migration and
heartbeat reporting.
- Preserves endpoint IDs when legacy agents send bodyless heartbeats.
- Converts duplicate endpoint claims into the API’s stable HTTP 409
conflict response.
- Allows HTTPS authorization callbacks only at the reserved path on the
matching single-label host.
- Returns unauthorized-client failures locally instead of redirecting to
an unapproved URI.

<h3>Confidence Score: 5/5</h3>

The PR appears safe to merge; the previously reported endpoint-conflict
failure is now translated and tested as a stable HTTP 409 response.

No actionable new failure remains. The prior endpoint-conflict finding
is fully fixed by translating PostgreSQL uniqueness failures into
`already_exists`, which the API maps to 409, with both domain-level and
route-level coverage.

<h3>Important Files Changed</h3>




| Filename | Overview |
|----------|----------|
| apps/auth/src/index.ts | Adds strict host callback authorization while
retaining the existing same-domain and local-development rules. |
| packages/auth/src/issuer.ts | Prevents unauthorized clients from using
their rejected redirect URI as an open-redirect destination. |
| packages/core/src/machine/index.ts | Persists optional endpoint
reports and translates duplicate endpoint claims into a stable domain
conflict. |
| apps/api/app/routes/machine.ts | Extends machine heartbeats with
optional endpoint reporting and documents the 409 response. |
| packages/core/migrations/0013_machine_endpoint_id.sql | Adds the
nullable endpoint column and database-enforced uniqueness invariant. |


<h3>Sequence Diagram</h3>

```mermaid
sequenceDiagram
    participant B as Browser
    participant A as Auth issuer
    participant P as Host proxy
    participant H as Machine
    participant D as Database

    H->>A: Authenticated heartbeat with optional endpointId
    A->>D: Update lastSeen and, when supplied, endpointId
    D-->>A: Stored or unique conflict
    A-->>H: 200 or typed 409

    B->>A: /authorize for host.nestri.link
    A->>A: Validate matching HTTPS reserved callback
    A-->>B: Provider flow
    B->>A: Complete authentication
    A-->>P: Short-lived authorization code
    P->>A: Exchange code
    A-->>P: Host-audience session tokens
```

<sub>Reviews (2): Last reviewed commit: ["fix(machine): a taken endpoint
id is a
c..."](51d25f3e8c)
| [Re-trigger
Greptile](https://app.greptile.com/api/retrigger?id=61026693)</sub>

<details><summary><h4>Context used (5)</h4></summary>

- Knowledge Base — [API HTTP composition and
authorization](https://app.greptile.com/nestri/-/custom-context/knowledge-base/nestrilabs/nestri/-/docs/api-http-and-auth.md)
- Knowledge Base — [Core domain and
persistence](https://app.greptile.com/nestri/-/custom-context/knowledge-base/nestrilabs/nestri/-/docs/core-domain-data.md)
- Knowledge Base — [Machines, pairing, and access
tokens](https://app.greptile.com/nestri/-/custom-context/knowledge-base/nestrilabs/nestri/-/docs/core-machines-and-pairing.md)
- Knowledge Base — [Authentication
platform](https://app.greptile.com/nestri/-/custom-context/knowledge-base/nestrilabs/nestri/-/docs/auth-platform.md)
- Knowledge Base — [Auth providers, sessions, and
storage](https://app.greptile.com/nestri/-/custom-context/knowledge-base/nestrilabs/nestri/-/docs/auth-providers-and-storage.md)
</details>


<!-- /greptile_comment -->
This commit is contained in:
Wanjohi
2026-09-06 21:24:36 +00:00
committed by GitHub
parent 0e94620808 51d25f3e8c
commit ec8b13d0c9
13 changed files with 3514 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
apps/api/app/middleware/auth.ts
View File

@@ -98,7 +98,7 @@ export const auth: MiddlewareHandler = async (c, next) => {
if (machineId && machineSecret) {
const machine = await Machine.authenticate({ id: machineId, secret: machineSecret });
if (machine) {
await Machine.touchLastSeen(machine.id);
await Machine.touchLastSeen({ id: machine.id });
return Actor.with(
{
type: 'machine',

26
apps/api/app/routes/machine.ts
View File

@@ -216,7 +216,7 @@ export namespace MachineApi {
tags: ['Machine'],
summary: 'Say the host is alive',
description:
'Records liveness for the calling machine and returns how often it should call back. The interval comes from the server on purpose: a fleet whose cadence can only change by shipping a new agent is a fleet whose cadence never changes. Takes no body — what a host is *running* is reported separately, and reporting a shape we cannot yet act on would be worse than reporting nothing.',
'Records liveness for the calling machine and returns how often it should call back. The interval comes from the server on purpose: a fleet whose cadence can only change by shipping a new agent is a fleet whose cadence never changes. The body carries one optional fact — where this host can be reached — because only a host can say that about itself and this is the call it already makes as itself. What a host is *running* is reported separately.',
responses: {
200: {
content: {
@@ -237,12 +237,32 @@ export namespace MachineApi {
},
description: 'The beat was recorded'
},
400: ErrorResponses[400],
403: ErrorResponses[403],
404: ErrorResponses[404]
404: ErrorResponses[404],
409: ErrorResponses[409]
}
}),
validator(
'json',
z
.object({
endpointId: Machine.EndpointId.optional().meta({
description:
'Where this host can be reached, as its own endpoint id. Omit it and the stored value is left alone — a host that does not mention where it is has not moved, and an absent field must never read as "nowhere"',
example: Examples.Machine.endpointId
})
})
// A host that has nothing to add sends no body at all, which
// is what every agent shipped before this field did.
.optional()
),
async (c) => {
const lastSeen = await Machine.touchLastSeen(Actor.machineID);
const body = c.req.valid('json');
const lastSeen = await Machine.touchLastSeen({
id: Actor.machineID,
endpointId: body?.endpointId
});
if (!lastSeen) {
// The credentials authenticated but the row is gone — a host
// deleted mid-beat. It must re-register rather than keep

88
apps/api/test/heartbeat.test.ts
View File

@@ -94,6 +94,94 @@ describe('POST /machine/heartbeat', () => {
expect((await Machine.fromID(host.id))?.lastSeen).toBeNull();
});
test('a host says where it is on the beat it already sends', async () => {
const host = await registeredHost('beat-endpoint');
const endpointId = 'd'.repeat(64);
// A beat carrying no body is what every agent shipped before this field
// sends, and it must still be a beat.
const bare = await app.request('/machine/heartbeat', {
method: 'POST',
headers: host.headers
});
expect(bare.status).toBe(200);
expect((await Machine.fromID(host.id))?.endpointId).toBeNull();
const res = await app.request('/machine/heartbeat', {
method: 'POST',
headers: { ...host.headers, 'content-type': 'application/json' },
body: JSON.stringify({ endpointId })
});
expect(res.status).toBe(200);
expect((await Machine.fromID(host.id))?.endpointId).toBe(endpointId);
// And a later beat that says nothing does not take the host off the map.
await app.request('/machine/heartbeat', { method: 'POST', headers: host.headers });
expect((await Machine.fromID(host.id))?.endpointId).toBe(endpointId);
});
test('a host cannot report where somebody else is', async () => {
// The report is authenticated as the machine it is about, and there is
// no field naming a different one. This is the assertion that keeps it
// that way: a body that tries anyway changes nothing.
const host = await registeredHost('beat-endpoint-other');
const victim = await registeredHost('beat-endpoint-victim');
const endpointId = 'e'.repeat(64);
const res = await app.request('/machine/heartbeat', {
method: 'POST',
headers: { ...host.headers, 'content-type': 'application/json' },
body: JSON.stringify({ endpointId, machineId: victim.id, id: victim.id })
});
expect(res.status).toBe(200);
expect((await Machine.fromID(host.id))?.endpointId).toBe(endpointId);
expect((await Machine.fromID(victim.id))?.endpointId).toBeNull();
});
test('an endpoint id that cannot be one is refused', async () => {
const host = await registeredHost('beat-endpoint-shape');
const res = await app.request('/machine/heartbeat', {
method: 'POST',
headers: { ...host.headers, 'content-type': 'application/json' },
body: JSON.stringify({ endpointId: 'not-an-endpoint-id' })
});
expect(res.status).toBe(400);
expect((await Machine.fromID(host.id))?.endpointId).toBeNull();
// Liveness is still recorded, and that is not a half-applied write:
// authenticating as this machine is itself proof it is alive, and the
// middleware records it before any route runs. What the refusal keeps
// out is the value that failed the check.
expect((await Machine.fromID(host.id))?.lastSeen).not.toBeNull();
});
test('claiming another host’s endpoint id is a conflict, not a fault', async () => {
const first = await registeredHost('beat-endpoint-taken-a');
const second = await registeredHost('beat-endpoint-taken-b');
const endpointId = 'f'.repeat(64);
await app.request('/machine/heartbeat', {
method: 'POST',
headers: { ...first.headers, 'content-type': 'application/json' },
body: JSON.stringify({ endpointId })
});
const res = await app.request('/machine/heartbeat', {
method: 'POST',
headers: { ...second.headers, 'content-type': 'application/json' },
body: JSON.stringify({ endpointId })
});
// The unique index is the invariant, so the database refusing is the
// expected way to find out — and an expected refusal reaching a host as
// a 500 tells it the server broke rather than that the id is taken.
expect(res.status).toBe(409);
expect((await res.json()) as any).toMatchObject({ type: 'already_exists' });
expect((await Machine.fromID(second.id))?.endpointId).toBeNull();
});
test('a user session cannot beat on a host’s behalf', async () => {
// A box holds credentials but is not its owner, and the reverse holds
// too: `machineOnly` exists so a route written for a host cannot be

110
apps/auth/src/index.ts
View File

@@ -2,6 +2,7 @@ import type { Hyperdrive } from '@cloudflare/workers-types';
import { issuer } from '@nestri/auth/index';
import { CodeProvider } from '@nestri/auth/provider/code';
import { CodeUI } from '@nestri/auth/ui/code';
import { isDomainMatch } from '@nestri/auth/util';
import { Actor } from '@nestri/core/actor';
import { PostgresCodeStore } from '@nestri/core/auth/authorization-code';
import { PostgresDeviceStore } from '@nestri/core/auth/device-grant';
@@ -45,6 +46,72 @@ type Env = {
*/
const DEVICE_CLIENTS = new Set(['desktop']);
/**
* The zone every user-owned host is reached under, and the one path on it that
* may receive an authorization code.
*
* A host is reached at `<id>.<zone>` through a proxy that authenticates
* browsers on its behalf. That proxy cannot be handed a session from here: a
* `__Host-` cookie is host-only by definition, so one set on this hostname is
* never sent to a different one, and a first request to a host's own name
* therefore arrives with no cookie whether or not the person is signed in.
*
* The proxy closes that by being an ordinary OAuth client — one per hostname —
* and exchanging a code for a session it can set on the hostname the browser is
* actually standing on. This is the rule that lets it: **the client id must be
* the hostname, and the redirect must be that same hostname at the reserved
* path below.**
*
* Making the client id the hostname is not a naming convention. A token is
* minted with its audience set to the client id, so it binds the session to the
* host it will live on — a cookie lifted off one host is not a credential on
* another, and it is not a credential here either. ref(d-0056)
*/
const HOST_ZONE = 'nestri.link';
const HOST_CALLBACK_PATH = '/__nestri/callback';
/**
* Whether `clientID` names a single host under {@link HOST_ZONE} and
* `redirectURI` is that same host's reserved callback.
*
* Every clause is load-bearing, because what is being decided is where this
* issuer will send an authorization code:
*
* - **`https` only.** A code is a one-time credential and belongs on a channel
* that cannot be read.
* - **The host must equal the client id exactly**, so a client can only ever
* receive a code at its own name.
* - **One label under the zone.** `a.b.<zone>` is not a host id, and must not
* be treated as one because `b.<zone>` might be.
* - **The path must be exactly the reserved one**, with no query and no
* fragment. A caller-chosen return address on a wildcard of hostnames is an
* open redirector on every one of them, and this is the parameter that would
* be it.
*/
function isHostCallback(clientID: string, redirectURI: string): boolean {
let url: URL;
try {
url = new URL(redirectURI);
} catch {
return false;
}
const label = clientID.toLowerCase().endsWith(`.${HOST_ZONE}`)
? clientID.toLowerCase().slice(0, -`.${HOST_ZONE}`.length)
: null;
if (!label || label.length === 0 || label.includes('.')) {
return false;
}
return (
url.protocol === 'https:' &&
url.host === clientID.toLowerCase() &&
url.pathname === HOST_CALLBACK_PATH &&
url.search === '' &&
url.hash === ''
);
}
/**
* Enough of an address to be worth trying to deliver to.
*
@@ -68,6 +135,40 @@ async function firstSteamLink(userID: string): Promise<string> {
return link?.id ?? '';
}
/**
* Which clients may start a flow here.
*
* The default rule allows a redirect back to whatever hostname the request
* arrived on, which is right for a site served beside this one and refuses the
* proxy in front of user-owned hosts — it redirects to a different registrable
* domain on purpose, so that no host's cookie can ever reach this one. That
* case is named here; everything else keeps the behaviour it had.
*
* Exported so it can be tested against a real `/authorize` request rather than
* by reading it.
*/
export const allowClient = async (
input: { clientID: string; redirectURI: string },
req: Request
): Promise<boolean> => {
if (isHostCallback(input.clientID, input.redirectURI)) {
return true;
}
let redirect: string;
try {
redirect = new URL(input.redirectURI).hostname;
} catch {
return false;
}
if (redirect === 'localhost' || redirect === '127.0.0.1') {
return true;
}
const forwarded = req.headers.get('x-forwarded-host');
const host = forwarded ? new URL(`https://${forwarded}`).hostname : new URL(req.url).hostname;
return isDomainMatch(redirect, host);
};
export default {
async fetch(request: Request, env: Env, ctx?: ExecutionContext) {
Env.init(env as unknown as Record<string, unknown>);
@@ -90,6 +191,15 @@ export default {
refreshStore: PostgresRefreshStore(),
deviceStore: PostgresDeviceStore(),
allowDeviceClient: async (clientID) => DEVICE_CLIENTS.has(clientID),
// The default rule allows a redirect back to whatever hostname the
// request arrived on, which is right for a site served beside this
// one and refuses the proxy in front of user-owned hosts — it
// redirects to a different registrable domain on purpose, so that
// no host's cookie can ever reach this one.
//
// So that case is named, and everything else keeps the behaviour it
// had.
allow: allowClient,
// One provider, on purpose.
//
// Verifying an email address is the only thing that brings an

150
apps/auth/test/allow.test.ts Normal file
View File

@@ -0,0 +1,150 @@
import { describe, expect, test } from 'bun:test';
import { issuer } from '@nestri/auth/index';
import { CodeProvider } from '@nestri/auth/provider/code';
import { MemoryStorage } from '@nestri/auth/storage/memory';
import { CodeUI } from '@nestri/auth/ui/code';
import { subjects } from '@nestri/core/auth/subjects';
import { allowClient } from '../src/index.js';
/**
* The same issuer the worker builds, with the database taken out and the real
* rule about which clients may start a flow left in.
*
* `allow` is the whole subject of this file, so unlike `worker.test.ts` it is
* not stubbed to `true`.
*/
const auth = issuer({
subjects,
storage: MemoryStorage(),
allow: allowClient,
providers: {
code: CodeProvider({
...CodeUI({ copy: { code_info: 'test' }, sendCode: async () => {} }),
sendCode: async () => {}
})
},
async success(context) {
return context.subject('user', { userID: 'usr_test123', linkedAccountID: '' });
}
});
/**
* Start an authorization and say only whether the client was allowed.
*
* An allowed client is redirected on towards the provider; a refused one is
* answered by the issuer itself. The distinction is the status, and nothing
* below cares about anything past it.
*/
async function allowed(clientID: string, redirectURI: string): Promise<boolean> {
const url = new URL('https://auth.internal/authorize');
url.searchParams.set('client_id', clientID);
url.searchParams.set('redirect_uri', redirectURI);
url.searchParams.set('response_type', 'code');
const response = await auth.request(url.toString());
if (response.status !== 302) {
return false;
}
// An allowed client is sent on to the provider, which is a path on this
// issuer. Anywhere else is not a sign-in beginning.
return (response.headers.get('location') ?? '').startsWith('/');
}
/**
* A browser that reaches one of these hostnames is standing on a different
* registrable domain from this issuer, and a session set here can never be
* sent there — a `__Host-` cookie has no `Domain` attribute and is host-only,
* which is exactly what it is for. The proxy in front of those hosts closes
* that by being an ordinary client and exchanging a code for a session it sets
* on the hostname the browser is actually on.
*
* Before this rule existed every case below was refused, including the first.
*/
describe('a host may receive a code at its own name', () => {
test('the reserved callback on the client id itself is allowed', async () => {
expect(await allowed('m123.nestri.link', 'https://m123.nestri.link/__nestri/callback')).toBe(
true
);
});
test('a code is never sent anywhere but the client id', async () => {
// The attack this refuses: a client that names itself as one host and
// asks for the code at another.
expect(await allowed('m123.nestri.link', 'https://evil.nestri.link/__nestri/callback')).toBe(
false
);
expect(await allowed('m123.nestri.link', 'https://evil.example/__nestri/callback')).toBe(false);
});
test('only the reserved path receives a code', async () => {
// Anything else under the hostname is served by the host itself, and a
// return address a caller chooses is an open redirector on every
// hostname in the zone.
expect(await allowed('m123.nestri.link', 'https://m123.nestri.link/')).toBe(false);
expect(
await allowed('m123.nestri.link', 'https://m123.nestri.link/__nestri/callback/../..')
).toBe(false);
expect(
await allowed('m123.nestri.link', 'https://m123.nestri.link/__nestri/callback?next=x')
).toBe(false);
});
test('a code goes over https or it does not go', async () => {
expect(await allowed('m123.nestri.link', 'http://m123.nestri.link/__nestri/callback')).toBe(
false
);
});
test('one label, because a deeper name is not a host id', async () => {
// `a.b.zone` must not be treated as a host id just because `b.zone`
// might be one.
expect(
await allowed('a.m123.nestri.link', 'https://a.m123.nestri.link/__nestri/callback')
).toBe(false);
expect(await allowed('nestri.link', 'https://nestri.link/__nestri/callback')).toBe(false);
});
test('another zone does not get in by using the path', async () => {
expect(await allowed('m123.example.com', 'https://m123.example.com/__nestri/callback')).toBe(
false
);
});
});
describe('everything else keeps the rule it had', () => {
test('a redirect back to where the request arrived is still allowed', async () => {
expect(await allowed('web', 'https://auth.internal/callback')).toBe(true);
});
test('local development is still allowed', async () => {
expect(await allowed('web', 'http://localhost:5173/callback')).toBe(true);
});
test('an unrelated domain is still refused', async () => {
expect(await allowed('web', 'https://somewhere.example/callback')).toBe(false);
});
});
/**
* A refusal is delivered here, not wherever the refused client asked.
*
* The issuer reports an error by redirecting to the caller's `redirect_uri`,
* which is right once that URI has been approved. This is the case where it has
* just been rejected — and honouring it there made `/authorize` an open
* redirector to anywhere at all, reachable without signing in, on the hostname
* people are asked to type a password into.
*/
describe('a refused client does not choose where the refusal goes', () => {
test('the refusal is a page here, not a redirect to the caller', async () => {
const url = new URL('https://auth.internal/authorize');
url.searchParams.set('client_id', 'web');
url.searchParams.set('redirect_uri', 'https://somewhere.example/callback');
url.searchParams.set('response_type', 'code');
const response = await auth.request(url.toString());
expect(response.status).toBe(400);
expect(response.headers.get('location')).toBeNull();
});
});

10
packages/auth/src/issuer.ts
View File

@@ -1702,6 +1702,16 @@ export function issuer<
if (err instanceof UnknownStateError) {
return auth.forward(c, await error(err, c.req.raw));
}
// A refused client does not get to choose where the refusal is delivered.
// Everything below reports an error by redirecting to the `redirect_uri`
// the caller supplied, which is correct once that URI has been approved
// and is an open redirector before it has: the check that approves it is
// the one that just failed, so honouring it here would turn every
// refusal into a redirect to anywhere at all — no sign-in required, on
// the hostname people are told to trust with a password.
if (err instanceof UnauthorizedClientError) {
return c.text(err.description || err.error, 400);
}
const authorization = await getAuthorization(c);
// A device grant has no redirect to carry the error back on, so it is
// said here instead. Without this the reporting path throws on a URL

26
packages/core/migrations/0013_machine_endpoint_id.sql Normal file
View File

@@ -0,0 +1,26 @@
-- Where a host actually is, so that a request authorised for it has somewhere
-- to go.
--
-- Until now this table said who owns a host, which team it belongs to and when
-- it was last seen, and nothing at all about how to reach it. A proxy that
-- authenticates a browser on a host's behalf can 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 this column
-- records what a host says about itself on a call it already makes as itself.
-- Nothing here mints one. ref(d-0010)
--
-- Nullable, because "has never reported one" is a real state rather than an
-- error: every host registered before this column existed is in it, and null
-- reads as "not reachable yet". A default would read as an address and route
-- somewhere wrong.
--
-- Unique, because an endpoint id belongs to exactly 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 this column can make that the
-- authorisation in front of it cannot catch. Postgres allows many nulls under a
-- unique index, so the hosts that have never reported are unaffected.
ALTER TABLE "machine" ADD COLUMN "endpoint_id" text;--> statement-breakpoint
CREATE UNIQUE INDEX "machine_endpoint_id_unique" ON "machine" USING btree ("endpoint_id");

2951
packages/core/migrations/meta/0013_snapshot.json Normal file
View File

File diff suppressed because it is too large Load Diff

7
packages/core/migrations/meta/_journal.json
View File

@@ -92,6 +92,13 @@
"when": 1788691753961,
"tag": "0012_steam_enrolment_without_a_token",
"breakpoints": true
},
{
"idx": 13,
"version": "7",
"when": 1788725541386,
"tag": "0013_machine_endpoint_id",
"breakpoints": true
}
]
}

3
packages/core/src/examples.ts
View File

@@ -129,7 +129,8 @@ export namespace Examples {
ownerUserId: Id('user'),
teamId: Id('team'),
label: 'living-room-box',
lastSeen: '2026-07-28T12:00:00.000Z'
lastSeen: '2026-07-28T12:00:00.000Z',
endpointId: 'a'.repeat(64)
};
export const SteamEnrolment = {

77
packages/core/src/machine/index.ts
View File

@@ -4,6 +4,7 @@ import { and, eq, isNull, sql } from 'drizzle-orm';
import z from 'zod';
import { Database } from '../db/index.js';
import { ErrorCodes, VisibleError } from '../error.js';
import { Examples } from '../examples.js';
import { fn } from '../fn.js';
import { Member } from '../team/member.js';
@@ -22,6 +23,19 @@ export namespace Machine {
/** Length in bytes before base64url encoding. */
const SECRET_BYTES = 32;
/**
* A host's endpoint id: 32 bytes of public key, lowercase hex.
*
* Checked for shape and nothing else. What it addresses is meaningless
* here — this is a string the control plane stores and hands back — so the
* only thing worth refusing is a value that cannot possibly be one, which
* is what keeps a truncated or double-encoded id from being written and
* then failing far away, at whoever tries to dial it.
*/
export const EndpointId = z.string().regex(/^[0-9a-f]{64}$/, {
message: 'An endpoint id is 64 lowercase hex characters'
});
export const Info = z
.object({
id: z.string().meta({
@@ -44,6 +58,11 @@ export namespace Machine {
lastSeen: z.iso.datetime().optional().nullable().meta({
description: 'When this machine last authenticated',
example: Examples.Machine.lastSeen
}),
endpointId: EndpointId.optional().nullable().meta({
description:
'Where this host can be reached, as its own endpoint id. Null until the host has reported one — it holds the secret half of this identity, so it is the only thing that can say what the public half is',
example: Examples.Machine.endpointId
})
})
.meta({
@@ -65,6 +84,12 @@ export namespace Machine {
.join('');
}
/** Postgres refusing a second row for the same key. */
function isUniqueViolation(err: unknown): boolean {
const e = err as { code?: string; cause?: { code?: string } };
return e?.code === '23505' || e?.cause?.code === '23505';
}
/** Length-independent, content-constant comparison of two hex digests. */
function secureEquals(a: string, b: string): boolean {
if (a.length !== b.length) {
@@ -195,17 +220,56 @@ export namespace Machine {
* Returns the stored timestamp rather than void so a caller can hand it
* straight back to the host — which is what lets a heartbeat be one round
* trip instead of a write followed by a read.
*
* `endpointId` rides along for the same reason. Where a host is reachable
* is a fact about the host, it changes when the agent's identity does, and
* a caller that has just proved it is that host is the only one who can
* report it — so it is written by the call that already says "still here",
* in the same statement, rather than by a second one that could succeed
* alone and leave the two facts disagreeing.
*/
export const touchLastSeen = fn(Info.shape.id, async (id) => {
return Database.use(async (tx) => {
export const touchLastSeen = fn(
Info.pick({ id: true }).extend({ endpointId: EndpointId.optional() }),
async (input) => {
try {
return await Database.use(async (tx) => {
return tx
.update(MachineTable)
.set({ lastSeen: sql`now()` })
.where(eq(MachineTable.id, id))
.set({
lastSeen: sql`now()`,
// Omitted rather than nulled when it is absent: a caller
// that does not mention where it is has not moved, and
// clearing the column would deregister a working host
// from every route that reads it.
...(input.endpointId ? { endpointId: input.endpointId } : {})
})
.where(eq(MachineTable.id, input.id))
.returning({ lastSeen: MachineTable.lastSeen })
.then((rows) => rows.at(0)?.lastSeen ?? null);
});
});
} catch (err) {
// Another host already holds this endpoint id. That is a
// conflict rather than a fault: the unique index is the
// invariant, so the database refusing is the expected way to
// find out, and letting it surface as a 500 would tell a host
// its beat broke the server.
//
// 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 — so the read would add a query and remove
// nothing.
if (isUniqueViolation(err)) {
throw new VisibleError(
'already_exists',
ErrorCodes.Validation.ALREADY_EXISTS,
'Another machine is already reachable at that endpoint id'
);
}
throw err;
}
}
);
/**
* Whether a host has beaten recently enough to place work on.
@@ -306,7 +370,8 @@ export namespace Machine {
ownerUserId: input.ownerUserId,
teamId: input.teamId,
label: input.label,
lastSeen: input.lastSeen?.toISOString() ?? null
lastSeen: input.lastSeen?.toISOString() ?? null,
endpointId: input.endpointId
};
}
}

16
packages/core/src/machine/machine.sql.ts
View File

@@ -29,6 +29,21 @@ export const MachineTable = pgTable(
.notNull()
.references(() => TeamTable.id, { onDelete: 'restrict' }),
label: text('label').notNull(),
// Where this host can actually be reached: its own endpoint id, as
// hex. A row can be authorised perfectly and still have nowhere to
// send the request without it, which is what this column fixes.
//
// **Reported, never assigned.** A host holds the secret half and is
// the only thing that can know the public one first, so the control
// plane records what it is told rather than handing one out. ref(d-0010)
//
// Nullable because a host that has never reported one is a real state
// — every host registered before this column existed is in it — and
// the honest reading of null is "not reachable yet" rather than a
// default that would route somewhere wrong. Unique because an endpoint
// id belongs to one host: two rows claiming the same one would send a
// request addressed to one machine to a different machine's agent.
endpointId: text('endpoint_id'),
// The secret itself is returned exactly once, at registration, and never
// stored: a leaked database must not yield working box credentials.
secretHash: text('secret_hash').notNull(),
@@ -36,6 +51,7 @@ export const MachineTable = pgTable(
},
(t) => [
uniqueIndex('machine_secret_hash_unique').on(t.secretHash),
uniqueIndex('machine_endpoint_id_unique').on(t.endpointId),
index('machine_owner_idx').on(t.ownerUserId),
index('machine_team_idx').on(t.teamId)
]

57
packages/core/src/machine/machine.test.ts
View File

@@ -87,17 +87,68 @@ describe('Machine heartbeat', () => {
expect((await Machine.fromID(machineId))?.lastSeen).toBeNull();
const first = await Machine.touchLastSeen(machineId);
const first = await Machine.touchLastSeen({ id: machineId });
expect(first).not.toBeNull();
const second = await Machine.touchLastSeen(machineId);
const second = await Machine.touchLastSeen({ id: machineId });
expect(second!.getTime()).toBeGreaterThanOrEqual(first!.getTime());
});
test('beating for a machine that is gone reports nothing rather than pretending', async () => {
// A host deleted mid-beat must be told to re-register, so this returns
// null and the route turns that into a 404.
expect(await Machine.touchLastSeen('mch_deletedmiddeletedmid___')).toBeNull();
expect(await Machine.touchLastSeen({ id: 'mch_deletedmiddeletedmid___' })).toBeNull();
});
test('a host reports where it is, and a beat that says nothing leaves it alone', async () => {
const owner = await newOwner('mch-endpoint');
const machineId = await Fixtures.machine(owner);
const endpointId = 'b'.repeat(64);
// Nothing assigns this. Until the host says so, there is nowhere to
// send a request that was authorised for it.
expect((await Machine.fromID(machineId))?.endpointId).toBeNull();
await Machine.touchLastSeen({ id: machineId, endpointId });
expect((await Machine.fromID(machineId))?.endpointId).toBe(endpointId);
// The regression this guards: an agent that beats without the field —
// every agent shipped before it existed — must not clear the column and
// take a working host off every route that reads it.
await Machine.touchLastSeen({ id: machineId });
expect((await Machine.fromID(machineId))?.endpointId).toBe(endpointId);
});
test('an endpoint id that cannot be one is refused before it is stored', async () => {
const owner = await newOwner('mch-endpoint-shape');
const machineId = await Fixtures.machine(owner);
// Truncated, upper-cased, and carrying an encoding that is not this
// one. Each would be written happily by a text column and would fail
// far away, at whoever tried to dial it.
for (const bad of ['abc', 'A'.repeat(64), `${'a'.repeat(63)}z`, `0x${'a'.repeat(64)}`]) {
expect(() => Machine.touchLastSeen({ id: machineId, endpointId: bad })).toThrow();
}
});
test('two machines cannot claim the same endpoint id', async () => {
const owner = await newOwner('mch-endpoint-unique');
const first = await Fixtures.machine(owner);
const second = await Fixtures.machine(owner);
const endpointId = 'c'.repeat(64);
await Machine.touchLastSeen({ id: first, endpointId });
// Two rows claiming one endpoint id would send a request addressed to
// one machine to another machine's agent, and the authorisation in
// front of it cannot catch that.
//
// A conflict rather than a fault, and that distinction is the test: the
// database refusing is the *expected* way to find out, so it must not
// reach a host as "your beat broke the server".
await expect(Machine.touchLastSeen({ id: second, endpointId })).rejects.toMatchObject({
type: 'already_exists'
});
});
test('online is derived from the last beat, not stored', async () => {