carlos / platform Public

status --instances: say what the account column is actually joined against

Review fixes on top of the door, none of them structural.

The honesty bug first. The readings are fleet-wide — every box writes
metrics/boxes/<box>/ and LoadMetrics sweeps all of it — but the routes
they are attributed against come from openPassRegistry, which opens this
console's own box-local registry. On a multi-box deployment every unit
sampled elsewhere matches no route, falls through to the platform case
and is charged to ops: a whole box's tenants would read as platform
overhead, which is the opposite of what the file's SCOPE comment
claimed. The comment now says what the code does and cites the same trap
sweepUsage already documents in economicspass.go, and the CLI prints the
limit under the table beside the hibernating-instance note — the way
`carlos routes` says whose routes it is showing. Inert today (both
deployments are single-box); the structural fix stays PR #137.

The --account filter test asserted nothing. No test in internal/console
wrote anything under metrics/boxes/, so LoadMetrics returned no boxes,
the loop over the answer's rows ran zero times, and deleting the
server-side filter left it green. There is a fixture now — one box, two
units owned by different accounts, one owned by no route — and two tests
on it: the filter narrows server-side, the unit -> route -> account join
attributes correctly, and a unit matching no route stays in the answer
labelled rather than being dropped. Both were confirmed to fail against
the mutation they exist to catch. A CLI-side test pins the (platform)
cell and both caveats.

The gate claim was overstated. The 404 hides the content and who holds
the operator bit; it does not hide the path, because a POST gets the
mux's 405 with an Allow header before any of this runs (verified: 405,
"Allow: GET, HEAD"). That is true of every method-scoped cliRoute, email
pause included, and is not this door's to fix — so the comment and
docs/site/cli.md say what is actually protected instead.

And the row build no longer walks the whole route list twice: the
account comes off the match routeForUnit already returned.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XxxmnFjdNxLc3M16ooFG95

Paul Campbell pushed by paul@keymail.dev 1 month ago e9790f9601920bb3df306ba03c9c9bc28547fbf2 parent b3ff9af

6 files changed, +259 −42

cli.go +5 −1
docs/site/cli.md +10 −3
internal/console/apistatusinstances.go +41 −15
internal/console/apistatusinstances_test.go +142 −16
status_instances.go +18 −7
status_instances_test.go +43 −0

CI passed — run details

diff --git a/cli.go b/cli.go

index a9d9aebbe..27b28cc6b 100644

--- a/cli.go

+++ b/cli.go

@@ -3047,7 +3047,11 @@ const statusUsage = `usage: carlos status --dry-run <dir>

(carlos auth login), operator only — a non-operator token

gets the same bare 404 an unknown route gets. --account

<sqid> narrows it to one account. A sleeping instance has

- no process and therefore no reading, so it does not appear.

+ no process and therefore no reading, so it does not appear;

+ and accounts are joined against the console's own box's

+ route registry, so an instance on another box in a fleet

+ can land under (platform). Both limits print with the

+ table.

// RunStatus is the "carlos status" verb: a fixture dry-run and a live read of

diff --git a/docs/site/cli.md b/docs/site/cli.md

index 2854f1bb0..52f32c552 100644

--- a/docs/site/cli.md

+++ b/docs/site/cli.md

@@ -829,9 +829,16 @@ carlos status --instances --account bac

It goes through the console rather than the status bucket, so it needs no AWS

credentials — which is the point of it existing. Operator only: a token

without the fleet-operator bit gets the same bare 404 an unknown route gets,

-because the answer names other people's accounts. A sleeping instance has no

-process and so no reading; it does not appear, and the command says so rather

-than letting a short list read as the whole tenancy.

+so it learns neither the rows nor whether it holds the bit. The path itself is

+not a secret — a wrong-method request gets a 405 from the router, like every

+other route here — but the answer is, because it names other people's

+accounts.

+Two things it cannot see, both printed under the table. A sleeping instance

+has no process and so no reading, so it does not appear. And the readings are

+fleet-wide while the account each one is attributed to comes from the

+console's own box's route registry, so on a multi-box deployment an instance

+running elsewhere can show up under `(platform)` rather than under its tenant.

`--dry-run <dir>` is the self-hoster's leak check. It builds a throwaway fleet

stuffed with fixture secrets, runs it through the real publish path into a

diff --git a/internal/console/apistatusinstances.go b/internal/console/apistatusinstances.go

index 550d2fe66..27b71e455 100644

--- a/internal/console/apistatusinstances.go

+++ b/internal/console/apistatusinstances.go

@@ -18,19 +18,41 @@

// for the browser page.

// GATE: cliUser + IsOperator, then a byte-identical 404 — the same shape

-// handleAPIEconomicsBackhaul and handleAPIEmailPause use. A non-operator must

-// not be able to tell this route from one that does not exist, because the

-// answer names other people's accounts. Registered through cliRoute with an

-// EMPTY role set, which closes it to every service credential outright: this

-// is an operator's view of the whole box, and no app-scoped machine token has

-// any business holding it.

+// handleAPIEconomicsBackhaul and handleAPIEmailPause use. What that buys is

+// worth stating exactly, because it is easy to overclaim: a non-operator

+// learns nothing about the CONTENT — whose accounts run what, on which box,

+// consuming what — and nothing about who holds the operator bit, since the

+// refusal is byte-identical to an unknown route's. It does NOT hide the

+// path's existence. The route is method-scoped, so a POST to it gets the

+// mux's own 405 with an Allow header, before any of this handler runs and to

+// any caller at all. That is true of every method-scoped cliRoute (email

+// pause included) and is not this door's to fix. Registered through cliRoute

+// with an EMPTY role set, which closes it to every service credential

+// outright: this is an operator's view of the whole fleet, and no app-scoped

+// machine token has any business holding it.

-// SCOPE: the console reads the metrics its own deployment publishes, which is

-// fleet-wide (every box writes metrics/boxes/<box>/), so unlike `carlos

-// routes` this answer is not box-local. What it CANNOT see is an instance

-// that is asleep — a hibernating instance has no cgroup and therefore no

-// sample. That is the right shape for the question ("who is using what RIGHT

-// NOW") and the CLI says so rather than letting a short list read as the

+// SCOPE: the two halves of the join do not reach equally far, and the answer

+// is only as fleet-wide as the shorter one. The READINGS are fleet-wide —

+// every box writes metrics/boxes/<box>/ and LoadMetrics sweeps all of it —

+// but the ROUTES they are attributed against come from openPassRegistry,

+// which opens this console's OWN box-local registry file. On a multi-box

+// deployment a unit sampled on any other box therefore matches no route

+// here, falls through routeForUnit's unmatched case, and lands in the answer

+// as platform overhead charged to the ops account: a whole box's tenants

+// read as the platform's own, which is the opposite of what an operator

+// asking "who is using what" would take from it. sweepUsage

+// (economicspass.go) carries this same trap and the same reasoning — both

+// production deployments are single-box today, so it is inert, and the

+// structural fix is routes-as-bucket-records (PR #137, unmerged), which

+// would give every pass a registry view spanning boxes instead of one box's

+// local file. Until that lands the CLI prints the limit beside the answer,

+// the way `carlos routes` does, rather than letting a short list read as the

+// whole fleet.

+//

+// The other limit is by design rather than a defect: an instance that is

+// ASLEEP has no cgroup and therefore no sample, so it is not here at all.

+// That is the right shape for the question ("who is using what RIGHT NOW")

+// and the CLI says so too, rather than letting a short list read as the

// whole tenancy.

package console

@@ -38,6 +60,8 @@ import (

"log"

"net/http"

"sort"

+ "github.com/carlosframework/platform/internal/release"

)

// apiStatusInstance is one awake instance. Account and app are joined on from

@@ -109,12 +133,14 @@ func (s *Server) handleAPIStatusInstances(w http.ResponseWriter, r *http.Request

for _, b := range boxes {

for _, u := range b.Units {

row := apiStatusInstance{Box: b.Box, Unit: u.Unit, CPU: u.CPUFrac, Mem: u.MemBytes}

+ // unitOwner's two cases, taken off the match already in hand

+ // rather than by walking the whole route list a second time —

+ // same rule, same answers, one pass.

if rt, ok := routeForUnit(routes, u.Unit); ok {

- row.Host, row.App = rt.Host, rt.App

+ row.Host, row.App, row.Account = rt.Host, rt.App, release.RouteAccount(rt)

} else {

- row.Platform = true

+ row.Platform, row.Account = true, release.OpsAccount

}

- row.Account, _, _ = unitOwner(routes, u.Unit)

if want != "" && row.Account != want {

continue

}

diff --git a/internal/console/apistatusinstances_test.go b/internal/console/apistatusinstances_test.go

index 9d18a856f..9d9081b1a 100644

--- a/internal/console/apistatusinstances_test.go

+++ b/internal/console/apistatusinstances_test.go

@@ -1,16 +1,26 @@

package console

import (

+ "context"

"encoding/json"

"net/http"

"testing"

+ "time"

+ "github.com/carlosframework/platform/internal/activator"

+ "github.com/carlosframework/platform/internal/metrics"

+ "github.com/carlosframework/platform/internal/registry"

+ "github.com/carlosframework/platform/internal/release"

)

// TestAPIStatusInstancesGate: operator only, with the same posture every

// operator door here has — anyone else sees the byte-identical 404 an unknown

// route gets. It matters more than usual on this door: the answer names other

-// people's accounts and what they are running, so "you are not an operator"

-// must not be distinguishable from "there is no such route".

+// people's accounts and what they are running, so the refusal must give away

+// neither the rows nor whether the caller holds the operator bit. It does not

+// hide the PATH — a wrong-method request gets the mux's 405 before any of

+// this — and the handler's comment says so; what is asserted here is what the

+// 404 actually buys.

func TestAPIStatusInstancesGate(t *testing.T) {

fx := newConsoleFixture(t)

token := cliToken(t, fx, "paul@keymail.dev", "code-apistatusinstances")

@@ -43,30 +53,146 @@ func TestAPIStatusInstancesGate(t *testing.T) {

}

+// tenancyFixture puts real readings behind the door: one box, three sampled

+// units, two of them owned by routes belonging to DIFFERENT accounts and one

+// owned by no route at all.

+//

+// It exists because without it the tests below assert nothing. No other test

+// in this package writes anything under metrics/boxes/, so LoadMetrics finds

+// no boxes, the handler's loop over box units never runs, every answer is an

+// empty array, and every loop a test writes over that answer runs zero times.

+// The --account filter test in particular stayed green with the server-side

+// filter deleted. A fixture is the difference between a test that reads like

+// a pin and one that is one.

+//

+// The unit names are built the way the handler matches them — through

+// activator.UnitName, which is what routeForUnit compares against — so what

+// is under test is the real matching rule rather than a string this file

+// agreed with itself about.

+func tenancyFixture(t *testing.T, fx *consoleFixture) {

+ t.Helper()

+ // fx.sqid is the built-in ops account; "cdef" is somebody else. Two

+ // tenants on one box is the whole point: a filter can only be shown to

+ // narrow if there is something for it to leave behind.

+ fx.addRoute(t, registry.Route{Host: "ours.example.com", Kind: "instance", Status: "ready",

+ App: "kass", Account: fx.sqid, Channel: "stable"})

+ fx.addRoute(t, registry.Route{Host: "theirs.example.com", Kind: "instance", Status: "ready",

+ App: "widgets", Account: "cdef", Channel: "stable"})

+ at := time.Now().UTC().Add(-time.Minute)

+ cpu, mem := 0.25, int64(1<<20)

+ if err := metrics.WriteRaw(context.Background(), fx.rel, metrics.RawChunk{

+ V: 1, Box: "box-tenancy", Hour: metrics.HourOf(at),

+ Samples: []metrics.Sample{{At: at, Units: map[string]metrics.UnitSample{

+ // A full reading, a partial one (CPU but no memory — the reason

+ // those fields are pointers), and a unit matching no route at

+ // all: the console's own, which is platform overhead.

+ activator.UnitName("ours.example.com"): {CPUFrac: &cpu, MemBytes: &mem},

+ activator.UnitName("theirs.example.com"): {CPUFrac: &cpu},

+ "console.service": {},

+ }}},

+ }); err != nil {

+ t.Fatal(err)

+ }

+// statusInstanceRows asks the door and decodes it, failing the test on any

+// answer that is not a 200 carrying an array.

+func statusInstanceRows(t *testing.T, fx *consoleFixture, token, query string) []apiStatusInstance {

+ t.Helper()

+ status, body := getBearer(t, fx.ts.URL+"/api/cli/status/instances"+query, token)

+ if status != http.StatusOK {

+ t.Fatalf("GET /api/cli/status/instances%s = %d %q", query, status, body)

+ }

+ var out struct {

+ Instances []apiStatusInstance `json:"instances"`

+ }

+ if err := json.Unmarshal(body, &out); err != nil {

+ t.Fatalf("unreadable: %v: %s", err, body)

+ }

+ return out.Instances

+// TestAPIStatusInstancesAttributesUnitsToAccounts pins the join itself: a

+// sampled unit is carried to the account that owns the route it belongs to,

+// and a unit belonging to no route stays in the answer marked as platform

+// overhead rather than being dropped. Dropping it would be the quieter bug of

+// the two — an operator asking what is on a box would get a list that omits

+// the box's own load and never says it did.

+func TestAPIStatusInstancesAttributesUnitsToAccounts(t *testing.T) {

+ fx := newConsoleFixture(t)

+ token := cliToken(t, fx, "paul@keymail.dev", "code-apistatusjoin")

+ if err := fx.st.SetOperator("paul@keymail.dev", true); err != nil {

+ t.Fatal(err)

+ }

+ tenancyFixture(t, fx)

+ rows := statusInstanceRows(t, fx, token, "")

+ byUnit := map[string]apiStatusInstance{}

+ for _, r := range rows {

+ byUnit[r.Unit] = r

+ }

+ if len(rows) != 3 || len(byUnit) != 3 {

+ t.Fatalf("unfiltered = %d rows %+v, want one per sampled unit", len(rows), rows)

+ }

+ ours := byUnit[activator.UnitName("ours.example.com")]

+ if ours.Account != fx.sqid || ours.App != "kass" || ours.Host != "ours.example.com" || ours.Platform {

+ t.Errorf("ours = %+v, want the ops account's kass route rather than platform", ours)

+ }

+ if ours.Box != "box-tenancy" || ours.CPU == nil || *ours.CPU != 0.25 || ours.Mem == nil || *ours.Mem != 1<<20 {

+ t.Errorf("ours = %+v, want box-tenancy carrying its own readings", ours)

+ }

+ theirs := byUnit[activator.UnitName("theirs.example.com")]

+ if theirs.Account != "cdef" || theirs.App != "widgets" || theirs.Host != "theirs.example.com" || theirs.Platform {

+ t.Errorf("theirs = %+v, want cdef's widgets route", theirs)

+ }

+ // A sampled unit with no memory reading must arrive as nil rather than 0:

+ // a zero is a number an operator would act on.

+ if theirs.Mem != nil {

+ t.Errorf("theirs mem = %d, want nil — the sample carried no reading", *theirs.Mem)

+ }

+ plat := byUnit["console.service"]

+ if !plat.Platform || plat.Account != release.OpsAccount || plat.App != "" || plat.Host != "" {

+ t.Errorf("console.service = %+v, want platform overhead charged to ops", plat)

+ }

// TestAPIStatusInstancesAccountFilterNarrows: --account is the "is THIS member

// hogging the box?" form of the question, which is the one that prompted the

-// door (discussion #11). An unknown account narrows to nothing rather than

-// falling back to everything — a filter that silently ignores itself is how

-// an operator reads another account's rows as the one they asked about.

+// door (discussion #11). The filter is applied on the SERVER, and it has to

+// be: a filter that silently ignores itself is how an operator reads another

+// account's rows as the one they asked about, and a client-side one would put

+// every account's rows on the wire to get there.

+//

+// Asserted against a populated box, so "narrows" means fewer rows than the

+// unfiltered answer rather than an empty list being trivially consistent with

+// every claim anyone could make about it.

func TestAPIStatusInstancesAccountFilterNarrows(t *testing.T) {

fx := newConsoleFixture(t)

token := cliToken(t, fx, "paul@keymail.dev", "code-apistatusfilter")

if err := fx.st.SetOperator("paul@keymail.dev", true); err != nil {

t.Fatal(err)

}

- status, body := getBearer(t, fx.ts.URL+"/api/cli/status/instances?account=nosuchacct", token)

- if status != http.StatusOK {

- t.Fatalf("operator with filter = %d %q", status, body)

+ tenancyFixture(t, fx)

+ if all := statusInstanceRows(t, fx, token, ""); len(all) != 3 {

+ t.Fatalf("unfiltered = %d rows %+v, want three — the fixture is what makes the rest of this test mean anything", len(all), all)

}

- var out struct {

- Instances []apiStatusInstance `json:"instances"`

+ // A known account: exactly its own row, and nobody else's.

+ only := statusInstanceRows(t, fx, token, "?account=cdef")

+ if len(only) != 1 {

+ t.Fatalf("?account=cdef = %d rows %+v, want just the one", len(only), only)

}

- if err := json.Unmarshal(body, &out); err != nil {

- t.Fatalf("unreadable: %v: %s", err, body)

+ if got, want := only[0].Unit, activator.UnitName("theirs.example.com"); got != want {

+ t.Errorf("?account=cdef returned %q, want %q", got, want)

}

- for _, r := range out.Instances {

- if r.Account != "nosuchacct" {

- t.Errorf("row for account %q survived a filter for %q", r.Account, "nosuchacct")

- }

+ // An unknown account narrows to nothing rather than falling back to

+ // everything.

+ if rows := statusInstanceRows(t, fx, token, "?account=nosuchacct"); len(rows) != 0 {

+ t.Errorf("?account=nosuchacct = %+v, want no rows at all", rows)

}

diff --git a/status_instances.go b/status_instances.go

index e7f269979..cfacc1511 100644

--- a/status_instances.go

+++ b/status_instances.go

@@ -54,9 +54,10 @@ func (s *apiSession) statusInstanceList(ctx context.Context, account string) ([]

if status != http.StatusOK {

if status == http.StatusNotFound {

// The same bare 404 an unknown route gives, and deliberately so:

- // the answer names other people's accounts, so a non-operator

- // must not be able to tell this door from one that is not there.

- // Which means this message has to cover both readings.

+ // the answer names other people's accounts, so the refusal says

+ // nothing about them and nothing about who holds the operator

+ // bit. Which leaves this message covering both readings, because

+ // the caller genuinely cannot tell them apart from here.

return nil, errors.New("not found — this needs the fleet-operator bit on your account, " +

"or this console predates `carlos status --instances` (deploy a console built from current main)")

}

@@ -115,11 +116,21 @@ func printStatusInstances(w io.Writer, rows []statusInstanceRow, account string)

cpuCell(r.CPU), memCell(r.Mem))

}

t.Flush()

- // Said every time. A hibernating instance has no process, so it has no

- // cgroup and no reading — it is not in this list and its absence is not

- // evidence that it does not exist. The question this verb answers is

- // "right now", and that limit belongs next to the answer.

+ // Both said every time, because both are ways a short list misleads.

+ //

+ // A hibernating instance has no process, so it has no cgroup and no

+ // reading — it is not in this list and its absence is not evidence that

+ // it does not exist. The question this verb answers is "right now", and

+ // that limit belongs next to the answer.

mutedf(w, "(awake instances only — a hibernating instance has no process to measure)")

+ // The readings are fleet-wide, but the console joins them against its own

+ // box's route registry, so a unit on any other box matches nothing and

+ // lands under (platform) rather than under its tenant. Inert on a

+ // single-box deployment and fixed structurally by PR #137; until then an

+ // operator reading this for "who is using what" has to be told, the same

+ // way `carlos routes` says whose routes it is showing.

+ mutedf(w, "(accounts are joined against the console's own box's route registry; "+

+ "an instance on another box in a fleet can land under (platform))")

}

// cpuCell renders a fraction of one core. A nil reading prints as a dash: the

diff --git a/status_instances_test.go b/status_instances_test.go

new file mode 100644

index 000000000..b94695541

--- /dev/null

+++ b/status_instances_test.go

@@ -0,0 +1,43 @@

+package main

+import (

+ "bytes"

+ "strings"

+ "testing"

+// TestPrintStatusInstancesLabelsPlatformAndStatesItsLimits is the CLI half of

+// what the console door promises. Two things have to survive rendering.

+//

+// A row that matched no route is the box's own overhead — the console, the

+// agent, a system unit — and it prints under its unit name and the word

+// (platform) rather than as two blank cells a reader would take for missing

+// data.

+//

+// And the table is followed, every time, by the two ways it misleads: a

+// hibernating instance has no process and so no reading, and the readings are

+// fleet-wide while the account each is attributed to comes from the console's

+// own box's registry. Neither caveat is conditional, because both are ways a

+// SHORT list reads as a complete one, and short is exactly when nobody thinks

+// to ask.

+func TestPrintStatusInstancesLabelsPlatformAndStatesItsLimits(t *testing.T) {

+ rows := []statusInstanceRow{

+ {Box: "box-a", Host: "app.example.com", Account: "cdef", App: "widgets",

+ CPU: f64(0.25), Mem: i64(1 << 20)},

+ {Box: "box-a", Unit: "console.service", Account: "bab", Platform: true},

+ }

+ var buf bytes.Buffer

+ printStatusInstances(&buf, rows, "")

+ out := buf.String()

+ for _, want := range []string{

+ "app.example.com", "cdef", "widgets", "0.250", "1MB",

+ "console.service", "(platform)",

+ "awake instances only",

+ "console's own box's route registry",

+ } {

+ if !strings.Contains(out, want) {

+ t.Errorf("missing %q in:\n%s", want, out)

+ }