carlos / platform Public

console: statements pass review fixes — validate storage account keys, document box-local registry limit

FIX 1 (documentation): statementsPass's doc comment now records that
unitOwner matches against this console's own box-local registry while the
pass iterates every box under metrics/boxes/ — on a multi-box deployment a
unit sampled on another box can fail to match any local route and silently
fall to release.OpsAccount, indistinguishable from intended platform
overhead. Both production deployments are single-box today (inert); names
routes-as-bucket-records (PR #137, unmerged) as the structural fix.
censusPass's doc comment corrected: counted by account SQID
(release.RouteAccount's sqid-shaped output, e.g. "bab"), not "NAME" — this
sits exactly where Task 11's implementer will read it.

FIX 2 (load-bearing for Task 11): guard the storage-day account segment
with release.ValidAccount before folding it into a statement key. A legacy
unqualified app prefix (apps/<app>/releases/, no account segment) makes
storagePass mint a StorageDay key whose segment before "/" is an APP name,
not an account sqid — left unguarded this mints a bogus statement (e.g.
usage/accounts/kass/2026-08.json) polluting the sqid-keyed space Task 11
joins on. An invalid segment now attributes to release.OpsAccount instead,
logged once per key per pass rather than once per day it recurs.

New test: TestStatementsPassRejectsInvalidStorageAccountSegment (uses
"my-app" — hyphenated, legal for release.ValidApp, illegal for
release.ValidAccount, since a plain-letters app name like "kass" happens
to pass ValidAccount's regex too and wouldn't exercise the guard).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Paul Campbell 2 months ago 48baae08d9248417ebfee57e6e188716b47d9200 parent aa54c46

2 files changed, +81 −4

internal/console/economicspass.go +35 −4
internal/console/economicspass_test.go +46 −0

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

index f90e8d1cf..005863b01 100644

--- a/internal/console/economicspass.go

+++ b/internal/console/economicspass.go

@@ -82,10 +82,11 @@ func (s *Server) openPassRegistry() (*registry.Registry, error) {

}

// censusPass snapshots per-account app/route counts, create-only. Counted by

-// account NAME: release.RouteAccount normalizes a route with no account

-// (Route.Account == "") to release.OpsAccount, the deployment operator's

-// built-in account — so pre-accounts routes land in one place instead of

-// scattering under "".

+// account SQID (release.RouteAccount's output, e.g. "bab" — never the

+// console store's display Name, e.g. "ops"): release.RouteAccount normalizes

+// a route with no account (Route.Account == "") to release.OpsAccount, the

+// deployment operator's built-in account — so pre-accounts routes land in

+// one place instead of scattering under "".

func (s *Server) censusPass(ctx context.Context, date string) error {

key := usage.CensusDayKey(date)

if _, _, err := s.cfg.Releases.Get(ctx, key); err == nil {

@@ -226,6 +227,17 @@ func unitOwner(routes []registry.Route, unit string) (acct, app string, ok bool)

// deleted_at), so this pass stops upserting its statement from the day of

// deletion onward — the account's last statement object simply stops moving

// forward, rather than the month closing it out explicitly.

+//

+// Second known shortfall: unitOwner attributes a sampled unit against

+// s.cfg.Registry, which is this CONSOLE's own box-local registry, while this

+// pass iterates units published by every box under metrics/boxes/. On a

+// multi-box deployment a unit sampled on a box other than this console's own

+// can fail to match any route in this registry and silently fall to

+// release.OpsAccount, indistinguishable from the intended platform-overhead

+// fallback. Both production deployments are single-box today, so this is

+// currently inert; the structural fix is routes-as-bucket-records (PR #137,

+// unmerged), which would give every pass a registry view that spans boxes

+// instead of one box's local file.

func (s *Server) statementsPass(ctx context.Context, now time.Time) error {

reg, err := s.openPassRegistry()

if err != nil {

@@ -259,6 +271,9 @@ func (s *Server) statementsPass(ctx context.Context, now time.Time) error {

}

return mm

}

+ // badStorageKeys logs a malformed storage-day key at most once per key

+ // for this whole pass, not once per day it recurs within the month.

+ badStorageKeys := map[string]bool{}

coveredDays := 0

for day := 1; day <= today; day++ {

@@ -333,6 +348,22 @@ func (s *Server) statementsPass(ctx context.Context, now time.Time) error {

if i := strings.IndexByte(key, '/'); i >= 0 {

acct = key[:i]

}

+ // A legacy unqualified app prefix (apps/<app>/releases/,

+ // no account segment) makes storagePass mint a StorageDay

+ // key whose segment before "/" is an APP name, not an

+ // account sqid. Left unguarded, that mints a bogus

+ // statement (e.g. usage/accounts/kass/2026-08.json)

+ // polluting the sqid-keyed space Task 11 joins on — so an

+ // account segment that doesn't pass release.ValidAccount

+ // attributes to release.OpsAccount instead, same as an

+ // unmatched unit.

+ if !release.ValidAccount(acct) {

+ if !badStorageKeys[key] {

+ badStorageKeys[key] = true

+ log.Printf("console: statements pass: storage day %s: key %q has an invalid account segment %q; attributing to %s", date, key, acct, release.OpsAccount)

+ }

+ acct = release.OpsAccount

+ }

bytesTotal := ap.BucketBytes + ap.StoreBytes

touch(acct)[usage.MeterStorageGBMonth] += (float64(bytesTotal) / 1e9) / float64(daysInMonth)

}

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

index d81a8d128..a363ace3e 100644

--- a/internal/console/economicspass_test.go

+++ b/internal/console/economicspass_test.go

@@ -404,3 +404,49 @@ func TestUnitOwnerMatchesStampedUnitColumn(t *testing.T) {

t.Fatalf("unitOwner(stamped unit) = %q %q %v", acct, app, ok)

}

+// TestStatementsPassRejectsInvalidStorageAccountSegment: a legacy

+// unqualified app prefix (apps/<app>/releases/, no account segment) makes

+// storagePass mint a StorageDay key whose segment before "/" is an APP

+// name, not an account sqid. release.ValidAccount is letters-only 3-16

+// chars, so a plain lowercase app name like "kass" happens to pass it too

+// — this fixture uses "my-app" (a hyphen, legal for release.ValidApp,

+// illegal for release.ValidAccount) as the bogus account segment, the same

+// shape storagePass would actually produce for an app whose name isn't

+// letters-only. statementsPass must not mint a statement under that bogus

+// key; the bytes must land on release.OpsAccount instead.

+func TestStatementsPassRejectsInvalidStorageAccountSegment(t *testing.T) {

+ fx := newConsoleFixture(t)

+ ctx := context.Background()

+ now := time.Date(2026, 8, 11, 6, 0, 0, 0, time.UTC)

+ if release.ValidAccount("my-app") {

+ t.Fatal("test fixture assumption broken: \"my-app\" passes release.ValidAccount")

+ }

+ putStoreJSON(t, fx.rel, usage.StorageDayKey("2026-08-10"), usage.StorageDay{

+ V: 1, Date: "2026-08-10", Apps: map[string]usage.AppStorage{

+ "my-app/releases": {BucketBytes: 31 * 1e9}, // 1 GB-month over 31 days

+ },

+ })

+ if err := fx.srv.statementsPass(ctx, now); err != nil {

+ t.Fatal(err)

+ }

+ if _, _, err := fx.rel.Get(ctx, usage.StatementKey("my-app", "2026-08")); !errors.Is(err, os.ErrNotExist) {

+ t.Fatalf("statement minted under the bogus account key \"my-app\": err=%v", err)

+ }

+ raw, _, err := fx.rel.Get(ctx, usage.StatementKey(release.OpsAccount, "2026-08"))

+ if err != nil {

+ t.Fatal(err)

+ }

+ var st usage.Statement

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

+ t.Fatal(err)

+ }

+ if got, want := st.Meters[usage.MeterStorageGBMonth], 1.0; math.Abs(got-want) > 1e-9 {

+ t.Fatalf("ops storage_gb_month = %v, want %v (invalid segment's bytes must land here)", got, want)

+ }