96d90a73
comments: eleven claims that argued with the code beside them
a73x 2026-08-23 11:42
Commit message
internal/agent/reconcile/reconcile.go
| Old | New | ||
|---|---|---|---|
| @@ -238,11 +238,13 @@ func tombstonedSet(snap *pb.Snapshot) map[string]bool { | |||
| 238 | // happens in workers, where VMTimeout bounds each VM's pass. | 238 | // happens in workers, where VMTimeout bounds each VM's pass. |
| 239 | func (e *Engine) Step(ctx context.Context, snap *pb.Snapshot) *pb.Report { | 239 | func (e *Engine) Step(ctx context.Context, snap *pb.Snapshot) *pb.Report { |
| 240 | // ── 1. Epoch fence ─────────────────────────────────────────────────────── | 240 | // ── 1. Epoch fence ─────────────────────────────────────────────────────── |
| 241 | // CRITICAL: the fence path must touch NOTHING: no SaveEpoch, no provisioner | 241 | // CRITICAL: the fence path must CHANGE nothing: no SaveEpoch, no create, |
| 242 | // calls, no dispatch, no state mutations. It returns the current actual state | 242 | // destroy or power call, no dispatch, no state mutations. It reads freely — |
| 243 | // so the control plane can observe what the agent actually has. A pass | 243 | // fenceReport asks Prov.Running for every non-quarantined record — because it |
| 244 | // dispatched by an EARLIER, accepted snapshot may still be running — the | 244 | // returns the current actual state so the control plane can observe what the |
| 245 | // fence refuses the stale snapshot, it does not freeze the host. | 245 | // agent actually has. A pass dispatched by an EARLIER, accepted snapshot may |
| 246 | // still be running: the fence refuses the stale snapshot, it does not freeze | ||
| 247 | // the host. | ||
| 246 | currentEpoch := e.St.Epoch() | 248 | currentEpoch := e.St.Epoch() |
| 247 | if snap.Epoch < currentEpoch { | 249 | if snap.Epoch < currentEpoch { |
| 248 | return e.fenceReport(currentEpoch) | 250 | return e.fenceReport(currentEpoch) |
| @@ -695,10 +697,11 @@ func (e *Engine) create(ctx context.Context, d *pb.VMSpec, rec state.Record, ok | |||
| 695 | // creates mean N image downloads, N image decodes and N multi-GB disk | 697 | // creates mean N image downloads, N image decodes and N multi-GB disk |
| 696 | // copies against one device, which can starve the state dir that Step scans | 698 | // copies against one device, which can starve the state dir that Step scans |
| 697 | // every tick (the heartbeat's one remaining blocking path) and can exhaust | 699 | // every tick (the heartbeat's one remaining blocking path) and can exhaust |
| 698 | // disk on the in-flight temporaries alone. Waiting here is free: the worker | 700 | // disk on the in-flight temporaries alone. The wait costs the VM nothing but |
| 699 | // is busy, so it publishes nothing and the report keeps its last-known row. | 701 | // time: it publishes the line below and then stays on it, so a queued VM |
| 700 | // A wait that outlives VMTimeout fails the attempt, and failCreate REFUNDS a | 702 | // reads as queued rather than as stalled. A wait that outlives VMTimeout |
| 701 | // ctx-expiry failure, so a queued VM never burns retry budget for waiting. | 703 | // fails the attempt, and failCreate REFUNDS a ctx-expiry failure, so a queued |
| 704 | // VM never burns retry budget for waiting. | ||
| 702 | say("waiting for a create slot on this host") | 705 | say("waiting for a create slot on this host") |
| 703 | release, err := e.acquireCreateSlot(ctx) | 706 | release, err := e.acquireCreateSlot(ctx) |
| 704 | if err != nil { | 707 | if err != nil { |
internal/agent/run/wire_darwin.go
| Old | New | ||
|---|---|---|---|
| @@ -89,7 +89,7 @@ func newPlatform(_ context.Context, cfg Config, st *state.Store) (platform, erro | |||
| 89 | pumps := serialpump.NewManager(vfkit.NewConsoleSource(prov.SocketPath), st.SerialLogPath) | 89 | pumps := serialpump.NewManager(vfkit.NewConsoleSource(prov.SocketPath), st.SerialLogPath) |
| 90 | prov.Pumps = pumps | 90 | prov.Pumps = pumps |
| 91 | 91 | ||
| 92 | // Asked once per report. Two file reads and an interface walk — no | 92 | // Asked once per report. One file read and an interface walk — no |
| 93 | // subprocess, no privileges, and nothing worth caching over a ten-second | 93 | // subprocess, no privileges, and nothing worth caching over a ten-second |
| 94 | // poll, where a cache would reintroduce exactly the staleness polling exists | 94 | // poll, where a cache would reintroduce exactly the staleness polling exists |
| 95 | // to avoid. | 95 | // to avoid. |
internal/agent/vfkit/leases.go
| Old | New | ||
|---|---|---|---|
| @@ -40,8 +40,8 @@ func (p *Provisioner) Address(vmID string) string { | |||
| 40 | // it from ever being asked a different question. | 40 | // it from ever being asked a different question. |
| 41 | func (p *Provisioner) NetworkAddress(string) string { return "" } | 41 | func (p *Provisioner) NetworkAddress(string) string { return "" } |
| 42 | 42 | ||
| 43 | // leaseAddress finds the address most recently leased to mac in the contents | 43 | // leaseAddress finds mac's address in the contents of macOS's dhcpd_leases |
| 44 | // of macOS's dhcpd_leases file, or "" if there is none. The file is a series | 44 | // file, or "" if there is none. The file is a series |
| 45 | // of brace-delimited stanzas of key=value lines: | 45 | // of brace-delimited stanzas of key=value lines: |
| 46 | // | 46 | // |
| 47 | // { | 47 | // { |
internal/server/api/tokens.go
| Old | New | ||
|---|---|---|---|
| @@ -13,7 +13,6 @@ import ( | |||
| 13 | // email bound to the tenant row, and the plane's jump-gate address (empty when | 13 | // email bound to the tenant row, and the plane's jump-gate address (empty when |
| 14 | // it runs no gate). It works identically for a PAT- or session-authenticated | 14 | // it runs no gate). It works identically for a PAT- or session-authenticated |
| 15 | // caller — both resolve to a tenant, and the email comes off that tenant's row. | 15 | // caller — both resolve to a tenant, and the email comes off that tenant's row. |
| 16 | // The default tenant, until claimed, has an empty email. | ||
| 17 | func (a *API) handleMe(w http.ResponseWriter, r *http.Request) { | 16 | func (a *API) handleMe(w http.ResponseWriter, r *http.Request) { |
| 18 | tenant := principalFromContext(r).Tenant | 17 | tenant := principalFromContext(r).Tenant |
| 19 | tn, ok, err := a.st.TenantByID(tenant) | 18 | tn, ok, err := a.st.TenantByID(tenant) |
internal/server/api/trustedcas.go
| Old | New | ||
|---|---|---|---|
| @@ -6,8 +6,10 @@ import ( | |||
| 6 | ) | 6 | ) |
| 7 | 7 | ||
| 8 | // The create path freezes a tenant's CA set onto the VM row through | 8 | // The create path freezes a tenant's CA set onto the VM row through |
| 9 | // store.FreezeCAs — the same helper the snapshot path uses, so both write an | 9 | // store.FreezeCAs, which is also what backfillTrustedCAs replays over rows |
| 10 | // identical record from one source. | 10 | // written before the set was recorded — so a frozen row reads the same however |
| 11 | // it got frozen. The snapshot path is not a caller: it serves the live set for | ||
| 12 | // a row that carries no record. | ||
| 11 | 13 | ||
| 12 | // trustedCAs renders a VM row's frozen CA set for the wire, or nil when the row | 14 | // trustedCAs renders a VM row's frozen CA set for the wire, or nil when the row |
| 13 | // carries no record — a VM created before the set was written down. nil is the | 15 | // carries no record — a VM created before the set was written down. nil is the |
internal/server/store/sessions_test.go
| Old | New | ||
|---|---|---|---|
| @@ -80,7 +80,10 @@ func TestSessionIDNotStoredInCleartext(t *testing.T) { | |||
| 80 | var stored string | 80 | var stored string |
| 81 | require.NoError(t, s.db.QueryRow(`SELECT id FROM sessions`).Scan(&stored)) | 81 | require.NoError(t, s.db.QueryRow(`SELECT id FROM sessions`).Scan(&stored)) |
| 82 | assert.NotEqual(t, id, stored, "the cookie value must not be what is on disk") | 82 | assert.NotEqual(t, id, stored, "the cookie value must not be what is on disk") |
| 83 | assert.Equal(t, hashToken(id), stored) | 83 | assert.Equal(t, hashToken(id), stored, |
| 84 | "the sessions row must hold hashToken(id) and nothing else: the schema comment says SHA-256 of the "+ | ||
| 85 | "session value, and a lookup that stored any other transform would still authenticate while "+ | ||
| 86 | "leaving the column undescribed by the DDL beside it") | ||
| 84 | 87 | ||
| 85 | // And the stored value must not itself work as a cookie — otherwise the row | 88 | // And the stored value must not itself work as a cookie — otherwise the row |
| 86 | // is still a bearer credential, just a differently-spelled one. | 89 | // is still a bearer credential, just a differently-spelled one. |
internal/server/store/store.go
| Old | New | ||
|---|---|---|---|
| @@ -150,8 +150,9 @@ CREATE TABLE IF NOT EXISTS meta ( | |||
| 150 | -- tenant ids MUST remain DOT-FREE: the jump gate's connect name is | 150 | -- tenant ids MUST remain DOT-FREE: the jump gate's connect name is |
| 151 | -- <tenant>.<name> and it splits on the FIRST dot (see sshgate/gate.go), while VM | 151 | -- <tenant>.<name> and it splits on the FIRST dot (see sshgate/gate.go), while VM |
| 152 | -- names are RFC1123 labels (also dot-free). A dotted tenant id would make the | 152 | -- names are RFC1123 labels (also dot-free). A dotted tenant id would make the |
| 153 | -- split ambiguous. Enforce this at tenant CRUD when it lands (no CRUD yet — the | 153 | -- split ambiguous. CreateTenantForIdentity is the only path that mints one and |
| 154 | -- only tenant is the seeded 'default'). | 154 | -- is where this is enforced: it derives the handle from an email with dots |
| 155 | -- flattened. | ||
| 155 | CREATE TABLE IF NOT EXISTS tenants ( | 156 | CREATE TABLE IF NOT EXISTS tenants ( |
| 156 | id TEXT PRIMARY KEY, | 157 | id TEXT PRIMARY KEY, |
| 157 | name TEXT NOT NULL, | 158 | name TEXT NOT NULL, |
| @@ -272,7 +273,8 @@ CREATE TABLE IF NOT EXISTS exposures ( | |||
| 272 | CREATE UNIQUE INDEX IF NOT EXISTS exposures_host_port_proto ON exposures(host_id, host_port, protocol); | 273 | CREATE UNIQUE INDEX IF NOT EXISTS exposures_host_port_proto ON exposures(host_id, host_port, protocol); |
| 273 | 274 | ||
| 274 | -- Console sessions. Server-side so revocation works and restarts keep | 275 | -- Console sessions. Server-side so revocation works and restarts keep |
| 275 | -- users signed in. id is 256-bit random hex; expiry enforced on read. | 276 | -- users signed in. id holds the SHA-256 of the session value, never the value |
| 277 | -- itself (see CreateSession); expiry enforced on read. | ||
| 276 | CREATE TABLE IF NOT EXISTS sessions ( | 278 | CREATE TABLE IF NOT EXISTS sessions ( |
| 277 | id TEXT PRIMARY KEY, | 279 | id TEXT PRIMARY KEY, |
| 278 | tenant TEXT NOT NULL REFERENCES tenants(id), | 280 | tenant TEXT NOT NULL REFERENCES tenants(id), |
| @@ -303,8 +305,9 @@ CREATE TABLE IF NOT EXISTS api_tokens ( | |||
| 303 | -- The tenant column (added via ensureColumn in Open — the table shipped | 305 | -- The tenant column (added via ensureColumn in Open — the table shipped |
| 304 | -- without it) scopes audit read per tenant. Rows written before any tenant is | 306 | -- without it) scopes audit read per tenant. Rows written before any tenant is |
| 305 | -- known — e.g. host.enroll.denied from an UNAUTHENTICATED enroll attempt — are | 307 | -- known — e.g. host.enroll.denied from an UNAUTHENTICATED enroll attempt — are |
| 306 | -- recorded against 'default' (the column's backfill/default value), since a row | 308 | -- recorded against SystemTenant, since a row must exist before an authenticated |
| 307 | -- must exist before an authenticated tenant is resolvable. | 309 | -- tenant is resolvable. SystemTenant is a scope no tenant-scoped read surfaces, |
| 310 | -- so these stay out of every tenant's audit view rather than landing in one. | ||
| 308 | CREATE TABLE IF NOT EXISTS audit_log ( | 311 | CREATE TABLE IF NOT EXISTS audit_log ( |
| 309 | id INTEGER PRIMARY KEY AUTOINCREMENT, | 312 | id INTEGER PRIMARY KEY AUTOINCREMENT, |
| 310 | at DATETIME NOT NULL, | 313 | at DATETIME NOT NULL, |
| @@ -1297,9 +1300,10 @@ func (s *Store) TenantForUserCA(pubkey string) (tenant string, ok bool, err erro | |||
| 1297 | return tenant, true, nil | 1300 | return tenant, true, nil |
| 1298 | } | 1301 | } |
| 1299 | 1302 | ||
| 1300 | // RemoveHost finalizes decommission: it returns the host's bridge CIDR to the | 1303 | // RemoveHost finalizes decommission: it deletes the host row, refusing |
| 1301 | // pool and deletes the host row. It refuses (in-transaction) while any VM rows | 1304 | // (in-transaction) while any VM rows remain for the host (not yet reaped). |
| 1302 | // remain for the host (not yet reaped). | 1305 | // Nothing is recycled — next_cidr_index only moves forward, so a removed host's |
| 1306 | // subnet is retired with it rather than handed to the next host to enroll. | ||
| 1303 | func (s *Store) RemoveHost(id string) error { | 1307 | func (s *Store) RemoveHost(id string) error { |
| 1304 | tx, err := s.db.Begin() | 1308 | tx, err := s.db.Begin() |
| 1305 | if err != nil { | 1309 | if err != nil { |
| @@ -1329,8 +1333,8 @@ func (s *Store) RemoveHost(id string) error { | |||
| 1329 | } | 1333 | } |
| 1330 | 1334 | ||
| 1331 | // ForceRemoveHost finalizes a host whose agent will never drain it (dead | 1335 | // ForceRemoveHost finalizes a host whose agent will never drain it (dead |
| 1332 | // hardware): it purges every VM row for the host, returns the bridge CIDR to | 1336 | // hardware): it purges every VM row for the host and deletes the host row, both |
| 1333 | // the pool, and deletes the host row — all in one transaction. Unlike the | 1337 | // in one transaction. Like RemoveHost it recycles nothing. Unlike the |
| 1334 | // graceful path it does NOT wait for the agent to ack destroys, so it must only | 1338 | // graceful path it does NOT wait for the agent to ack destroys, so it must only |
| 1335 | // be used when the host is known gone; any VMs still physically running are | 1339 | // be used when the host is known gone; any VMs still physically running are |
| 1336 | // orphaned with the hardware. Returns the number of VM rows purged. | 1340 | // orphaned with the hardware. Returns the number of VM rows purged. |