carlos / platform Public

ledgers: cap the published set at 64, negative-cache only real not-found

Publishing became additive read-modify-write, which removed the implicit
bound the 8 KiB body cap gave under replace semantics: names accumulate,
and ledgers.json is member-supplied content served publicly at the edge,
held in its in-memory cache and scanned linearly on every ledger request.
Cap the RESULTING set at 64 inside the apply — only the stored manifest
knows the current count, and UpdateManifest re-reads on a lost race, so a
check outside the closure could be raced past. apply can now REFUSE by
returning an error, which aborts before the Put, so a refused publish
writes nothing and answers 400 rather than a laundered 500.

At the edge, markMiss only on os.ErrNotExist: the probe defense is
nonexistent manifests, and pinning a 404 for a full TTL because the bucket
blipped while the cache was cold takes a live public app dark. The blipped
request still 404s; the next one retries. And correct markMiss's TTL claim
— a positive-cached public:false manifest can stamp a negative entry that
outlives it, so publishing takes effect within two ticks worst case.

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

Paul Campbell 2 months ago a57bbe9beb351c53f38b62900930ffa2450f8a30 parent 3a90c30

6 files changed, +240 −14

internal/console/apiledger.go +40 −2
internal/console/apiledger_test.go +67 −0
internal/edge/status.go +22 −5
internal/edge/status_test.go +60 −0
internal/ledger/manifest.go +15 −3
internal/ledger/manifest_test.go +36 −4

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

index a4bd2aa78..d6f32a48e 100644

--- a/internal/console/apiledger.go

+++ b/internal/console/apiledger.go

@@ -244,6 +244,26 @@ func (s *Server) handleAPILedgerList(w http.ResponseWriter, r *http.Request) {

// holds for every reader that will ever fetch the object.

const maxLedgerTitle = 120

+// maxLedgerCount bounds Manifest.Ledgers. Publishing is additive

+// read-modify-write, which removed the implicit bound the 8 KiB request-body

+// cap used to give when a publish stored the request's manifest wholesale:

+// names now ACCUMULATE across calls, and ledgers.json is member-supplied

+// content that is served publicly at the edge, held in the edge's in-memory

+// cache, and scanned linearly on every single ledger request (the allowlist

+// gate). 64 published chains per app is far past any real app and keeps that

+// per-request scan trivially short. Like the title's cap, it lives at the

+// write door, because that is the only place a bound holds for every reader

+// that will ever fetch the object.

+const maxLedgerCount = 64

+// errTooManyLedgers is how the publish apply below REFUSES from inside

+// ledger.UpdateManifest's CAS loop: the cap can only be judged against the

+// stored set, which the loop may re-read, so the check has to live where the

+// decision is made and travel back out as an error the handler recognises.

+// Its text is the client-facing message — it names the cap and nothing else,

+// so there is no path by which store internals reach a caller through it.

+var errTooManyLedgers = fmt.Errorf("too many public ledgers (max %d)", maxLedgerCount)

// handleAPILedgerPublish is READ-MODIFY-WRITE, per ledger. The manifest is a

// SET of published ledgers accrued across separate publish calls, and the

// edge reads that set as an ALLOWLIST (internal/edge/status.go) — so a

@@ -306,7 +326,7 @@ func (s *Server) handleAPILedgerPublish(w http.ResponseWriter, r *http.Request)

}

- m, err := ledger.UpdateManifest(ctx, s.cfg.Releases, acct.Sqid, app, func(m *ledger.Manifest) {

+ m, err := ledger.UpdateManifest(ctx, s.cfg.Releases, acct.Sqid, app, func(m *ledger.Manifest) error {

if in.Public {

m.Public = true

for _, n := range in.Ledgers {

@@ -314,10 +334,19 @@ func (s *Server) handleAPILedgerPublish(w http.ResponseWriter, r *http.Request)

m.Ledgers = append(m.Ledgers, n)

}

+ // The cap is checked HERE, on the manifest this attempt read, and

+ // on the RESULT — not on the request. Only the stored set knows

+ // how many names are already there, and UpdateManifest re-reads

+ // and re-applies on a lost race, so a check made once outside

+ // this closure could be stepped over by two publishers racing.

+ // Refusing from inside aborts before the Put: nothing is written.

+ if len(m.Ledgers) > maxLedgerCount {

+ return errTooManyLedgers

+ }

if title != "" {

m.Title = title

}

- return

+ return nil

}

m.Ledgers = slices.DeleteFunc(m.Ledgers, func(n string) bool {

return slices.Contains(in.Ledgers, n)

@@ -325,7 +354,16 @@ func (s *Server) handleAPILedgerPublish(w http.ResponseWriter, r *http.Request)

if len(m.Ledgers) == 0 {

m.Public = false

}

+ return nil

})

+ // A refusal is the CALLER's fault and must not be laundered through

+ // ledgerStoreError, which would log it and answer 500: this is the one

+ // error out of UpdateManifest that a client can act on, and the only one

+ // whose text is ours to show.

+ if errors.Is(err, errTooManyLedgers) {

+ jsonError(w, http.StatusBadRequest, errTooManyLedgers.Error())

+ return

+ }

if err != nil {

s.ledgerStoreError(w, fmt.Sprintf("ledger publish %s/%s", acct.Sqid, app), err)

return

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

index b49ff0746..bdd218192 100644

--- a/internal/console/apiledger_test.go

+++ b/internal/console/apiledger_test.go

@@ -13,8 +13,10 @@ import (

"context"

"encoding/json"

"errors"

+ "fmt"

"io"

"net/http"

+ "slices"

"strings"

"testing"

"time"

@@ -462,6 +464,71 @@ func TestAPILedgerPublishTitleCap(t *testing.T) {

}

+// TestAPILedgerPublishCountCap: the manifest became read-modify-write, which

+// removed the implicit bound the 8 KiB request cap used to give a whole-value

+// replace — names now ACCUMULATE. ledgers.json is member-supplied content

+// served publicly at the edge, held in the edge's in-memory cache and

+// linearly scanned on every ledger request, so the set is bounded at the

+// write door, on the RESULTING manifest, like every other public-facing input

+// here.

+func TestAPILedgerPublishCountCap(t *testing.T) {

+ fx, token := ledgerFixture(t, "code-ledger-count")

+ names := make([]string, 0, maxLedgerCount)

+ for i := range maxLedgerCount {

+ names = append(names, fmt.Sprintf("l%d", i))

+ }

+ full, err := json.Marshal(map[string]any{"public": true, "ledgers": names, "title": "T"})

+ if err != nil {

+ t.Fatal(err)

+ }

+ status, m := publishLedgers(t, fx, token, string(full))

+ if status != http.StatusOK {

+ t.Fatalf("publish of %d ledgers = %d, want 200 (the cap is a ceiling, not a fence below it)", maxLedgerCount, status)

+ }

+ if len(m.Ledgers) != maxLedgerCount {

+ t.Fatalf("published %d ledgers, want %d", len(m.Ledgers), maxLedgerCount)

+ }

+ // The one that would step over is refused as the caller's fault, and

+ // nothing new is written: not the name, not the title that rode with it.

+ status, out := postBody(t, fx.ts.URL+"/api/cli/apps/bab/titogo/ledgers/publish", token,

+ `{"public":true,"ledgers":["overflow"],"title":"nope"}`)

+ if status != http.StatusBadRequest {

+ t.Fatalf("publish over the cap = %d %s, want 400", status, out)

+ }

+ if !strings.Contains(string(out), "too many public ledgers") {

+ t.Fatalf("refusal = %s, want it to name the cap", out)

+ }

+ stored, err := ledger.ReadManifest(context.Background(), fx.rel, "bab", "titogo")

+ if err != nil {

+ t.Fatal(err)

+ }

+ if len(stored.Ledgers) != maxLedgerCount || slices.Contains(stored.Ledgers, "overflow") {

+ t.Fatalf("stored manifest = %+v, want the refused publish to have written nothing", stored)

+ }

+ if stored.Title != "T" {

+ t.Fatalf("title = %q, want the refused publish to have left it alone", stored.Title)

+ }

+ // Re-publishing one that is ALREADY on the list at the cap is fine — the

+ // bound is on the resulting set, and that set does not grow.

+ if status, m := publishLedgers(t, fx, token, `{"public":true,"ledgers":["l0"]}`); status != http.StatusOK ||

+ len(m.Ledgers) != maxLedgerCount {

+ t.Fatalf("idempotent re-publish at the cap = %d %+v, want 200 and no growth", status, m.Ledgers)

+ }

+ // And the way back down is never blocked: an app that hit the cap must

+ // still be able to unpublish.

+ status, m = publishLedgers(t, fx, token, `{"public":false,"ledgers":["l0"]}`)

+ if status != http.StatusOK {

+ t.Fatalf("unpublish at the cap = %d, want 200", status)

+ }

+ if len(m.Ledgers) != maxLedgerCount-1 || slices.Contains(m.Ledgers, "l0") {

+ t.Fatalf("unpublish at the cap left %v", m.Ledgers)

+ }

// TestAPILedgerHeadMissing: a ledger that was never written is the same

// stdlib 404 as an app you can't see — there is nothing to distinguish.

func TestAPILedgerHeadMissing(t *testing.T) {

diff --git a/internal/edge/status.go b/internal/edge/status.go

index 90439336c..f7ab59944 100644

--- a/internal/edge/status.go

+++ b/internal/edge/status.go

@@ -15,8 +15,10 @@ package edge

import (

"context"

"encoding/json"

+ "errors"

"log"

"net/http"

+ "os"

"path"

"slices"

"strings"

@@ -128,10 +130,15 @@ func (h *statusHandler) missFresh(key string) bool {

return ok && h.now().Sub(at) < h.ttl

}

-// markMiss records that an app is not servable — no manifest, an unreadable

-// or unparseable one, or public:false — for the same TTL the positive cache

-// trusts a fetched object. Publishing an app therefore takes effect within

-// one tick, the same bound revoking already had.

+// markMiss records that an app is not servable — no manifest, an unparseable

+// one, or public:false — for the same TTL the positive cache trusts a fetched

+// object. Publishing an app therefore takes effect within TWO ticks worst

+// case (~2×TTL), one typically: the two caches are stamped independently, so

+// a public:false manifest read at t=0 can still be serving from the positive

+// cache at t=59s and stamp a NEGATIVE entry then, which outlives it. Revoking

+// has the same shape from the other side. Both are bounded and small; the

+// alternative — invalidating across the two caches — would buy a second off a

+// 5-minute-cadence page.

// The bound is the point: see statusMissCacheMax.

func (h *statusHandler) markMiss(key string) {

@@ -246,7 +253,17 @@ func (h *statusHandler) serveLedger(w http.ResponseWriter, r *http.Request, p st

}

mb, _, err := h.get(r.Context(), mkey, "application/json")

if err != nil {

- h.markMiss(mkey)

+ // Only a genuine not-found is REMEMBERED. That is what the negative

+ // cache is for: a prober names manifests that do not exist, and the

+ // store answers os.ErrNotExist (release.Store's contract). Any other

+ // error is a blip — a throttle, an expired credential — and stamping

+ // it would take a live public app dark for the full TTL because the

+ // bucket hiccuped once while this key's positive cache happened to be

+ // cold. This request still 404s (fail-closed: nothing readable said

+ // public); the next one retries.

+ if errors.Is(err, os.ErrNotExist) {

+ h.markMiss(mkey)

+ }

http.NotFound(w, r) // fail-closed: no readable manifest = not public

return

}

diff --git a/internal/edge/status_test.go b/internal/edge/status_test.go

index bb698ae71..76cd05c80 100644

--- a/internal/edge/status_test.go

+++ b/internal/edge/status_test.go

@@ -7,6 +7,7 @@ package edge

import (

"context"

"encoding/json"

+ "errors"

"fmt"

"net/http"

"net/http/httptest"

@@ -459,6 +460,65 @@ func TestLedgerManifestMissesAreNegativeCached(t *testing.T) {

}

+// flakyStore fails every read while armed, with an error that is NOT

+// not-found — a bucket blip, a throttle, an expired credential — the class of

+// failure that resolves on its own.

+type flakyStore struct {

+ release.Store

+ mu sync.Mutex

+ broken bool

+func (f *flakyStore) breakIt(b bool) {

+ f.mu.Lock()

+ defer f.mu.Unlock()

+ f.broken = b

+func (f *flakyStore) Get(ctx context.Context, key string) ([]byte, string, error) {

+ f.mu.Lock()

+ broken := f.broken

+ f.mu.Unlock()

+ if broken {

+ return nil, "", errors.New("Throttling: slow down")

+ }

+ return f.Store.Get(ctx, key)

+// TestLedgerTransientStoreErrorIsNotNegativeCached: the negative cache exists

+// to make PROBING cheap to refuse, and a probe's manifest is not-found. A

+// transient store error is a different thing entirely: caching it would pin a

+// public app dark for the full TTL because the bucket hiccuped once while the

+// cache happened to be cold — the exact dishonesty the positive cache's

+// stale-on-error half exists to avoid, reintroduced from the other side. The

+// blipped request still 404s (fail-closed: nothing readable said public), but

+// the decision is not remembered, so the next request retries.

+func TestLedgerTransientStoreErrorIsNotNegativeCached(t *testing.T) {

+ base := release.NewDirStore(t.TempDir())

+ st := &flakyStore{Store: base}

+ seedLedger(t, base, true)

+ h := newStatusHandler(st)

+ // Cold cache + broken store: 404, because nothing readable says public.

+ st.breakIt(true)

+ if rr := getStatus(t, h, "/ledgers/bab/platform/carbon/HEAD.json"); rr.Code != http.StatusNotFound {

+ t.Fatalf("during the blip = %d, want 404", rr.Code)

+ }

+ h.mu.Lock()

+ n := len(h.misses)

+ h.mu.Unlock()

+ if n != 0 {

+ t.Fatalf("a transient store error stamped %d negative-cache entries, want 0", n)

+ }

+ // The store recovers well inside the 60s TTL: the very next request must

+ // serve, not sit behind a cached refusal.

+ st.breakIt(false)

+ if rr := getStatus(t, h, "/ledgers/bab/platform/carbon/HEAD.json"); rr.Code != http.StatusOK {

+ t.Fatalf("after the store recovered = %d, want 200 (the blip must not be negatively cached)", rr.Code)

+ }

// TestLedgerMissCacheIsBounded: the attacker picks the account and app path

// segments, so the set of distinct manifest keys a prober can name is

// unbounded — a negative cache that grew with it would be the memory

diff --git a/internal/ledger/manifest.go b/internal/ledger/manifest.go

index 67407997e..60c54a8c9 100644

--- a/internal/ledger/manifest.go

+++ b/internal/ledger/manifest.go

@@ -29,7 +29,10 @@ func ReadManifest(ctx context.Context, st release.Store, sqid, app string) (Mani

// UpdateManifest instead: replacing the whole value there unpublishes

// everything the caller didn't happen to name.

func WriteManifest(ctx context.Context, st release.Store, sqid, app string, m Manifest) error {

- _, err := UpdateManifest(ctx, st, sqid, app, func(cur *Manifest) { *cur = m })

+ _, err := UpdateManifest(ctx, st, sqid, app, func(cur *Manifest) error {

+ *cur = m

+ return nil

+ })

return err

}

@@ -48,8 +51,15 @@ const manifestAttempts = 4

// Contention that outlasts the retries surfaces as release.ErrCASConflict

// (the console maps it to a retry-worthy 503) — never as a silent no-op.

+//

+// apply may REFUSE: an error from it aborts the attempt BEFORE the Put and is

+// returned untouched, so nothing is written. That is what lets a caller

+// enforce a rule the request alone cannot decide — a cap on the RESULTING set

+// — on whatever manifest each attempt actually read, including the re-read

+// after a lost race, rather than on a snapshot that a concurrent publisher

+// has already moved past.

func UpdateManifest(ctx context.Context, st release.Store, sqid, app string,

- apply func(*Manifest)) (Manifest, error) {

+ apply func(*Manifest) error) (Manifest, error) {

key := ManifestKey(sqid, app)

var lastErr error

for range manifestAttempts {

@@ -67,7 +77,9 @@ func UpdateManifest(ctx context.Context, st release.Store, sqid, app string,

}

- apply(&m)

+ if err := apply(&m); err != nil {

+ return Manifest{}, err

+ }

m.Schema = Schema

body, err := json.Marshal(m)

if err != nil {

diff --git a/internal/ledger/manifest_test.go b/internal/ledger/manifest_test.go

index 48ff0beab..d7313defa 100644

--- a/internal/ledger/manifest_test.go

+++ b/internal/ledger/manifest_test.go

@@ -46,10 +46,11 @@ func TestUpdateManifestReadModifyWrite(t *testing.T) {

// Missing manifest is the zero value, not an error: the first publish

// of an app creates it.

- m, err := UpdateManifest(ctx, st, "bab", "platform", func(m *Manifest) {

+ m, err := UpdateManifest(ctx, st, "bab", "platform", func(m *Manifest) error {

m.Public = true

m.Ledgers = append(m.Ledgers, "carbon")

m.Title = "CARLOS"

+ return nil

})

if err != nil {

t.Fatalf("first update: %v", err)

@@ -59,8 +60,9 @@ func TestUpdateManifestReadModifyWrite(t *testing.T) {

}

// A second update sees the first one's state.

- m, err = UpdateManifest(ctx, st, "bab", "platform", func(m *Manifest) {

+ m, err = UpdateManifest(ctx, st, "bab", "platform", func(m *Manifest) error {

m.Ledgers = append(m.Ledgers, "prompts")

+ return nil

})

if err != nil {

t.Fatalf("second update: %v", err)

@@ -98,11 +100,12 @@ func (c *conflictOnce) Put(ctx context.Context, key string, body []byte, ifMatch

// two ledgers at once is a lost update, and the loser never learns.

func TestUpdateManifestCASReapplies(t *testing.T) {

st := &conflictOnce{Store: release.NewDirStore(t.TempDir())}

- m, err := UpdateManifest(context.Background(), st, "bab", "platform", func(m *Manifest) {

+ m, err := UpdateManifest(context.Background(), st, "bab", "platform", func(m *Manifest) error {

m.Public = true

if !slices.Contains(m.Ledgers, "carbon") {

m.Ledgers = append(m.Ledgers, "carbon")

}

+ return nil

})

if err != nil {

t.Fatalf("update: %v", err)

@@ -122,13 +125,42 @@ func TestUpdateManifestCASReapplies(t *testing.T) {

// is reported, never swallowed — the console maps it to a retry-worthy 503.

func TestUpdateManifestSurfacesLostRaces(t *testing.T) {

st := alwaysConflict{release.NewDirStore(t.TempDir())}

- if _, err := UpdateManifest(context.Background(), st, "bab", "platform", func(m *Manifest) {

+ if _, err := UpdateManifest(context.Background(), st, "bab", "platform", func(m *Manifest) error {

m.Public = true

+ return nil

}); !errors.Is(err, release.ErrCASConflict) {

t.Fatalf("update under total contention = %v, want a CAS conflict", err)

}

+// TestUpdateManifestApplyRefusalWritesNothing: apply is where a caller

+// enforces a rule only the STORED manifest can decide (the console's cap on

+// how many ledgers an app may publish). A refusal has to abort before the Put

+// — a loop that wrote first and reported the refusal afterwards would store

+// exactly the state it just called illegal.

+func TestUpdateManifestApplyRefusalWritesNothing(t *testing.T) {

+ ctx := context.Background()

+ st := release.NewDirStore(t.TempDir())

+ if err := WriteManifest(ctx, st, "bab", "platform",

+ Manifest{Public: true, Ledgers: []string{"carbon"}}); err != nil {

+ t.Fatal(err)

+ }

+ refused := errors.New("no")

+ if _, err := UpdateManifest(ctx, st, "bab", "platform", func(m *Manifest) error {

+ m.Ledgers = append(m.Ledgers, "prompts")

+ return refused

+ }); !errors.Is(err, refused) {

+ t.Fatalf("update = %v, want apply's own error returned untouched", err)

+ }

+ stored, err := ReadManifest(ctx, st, "bab", "platform")

+ if err != nil {

+ t.Fatal(err)

+ }

+ if !slices.Equal(stored.Ledgers, []string{"carbon"}) {

+ t.Fatalf("stored = %+v, want the refused change never written", stored)

+ }

type alwaysConflict struct{ release.Store }

func (a alwaysConflict) Put(context.Context, string, []byte, string) (string, error) {