amadan
ledger: publish is additive per-ledger, bounded title, store timeouts, negative-cached misses
The final whole-branch review's code findings, in one wave. publish replaced the whole manifest. With the edge's per-ledger allowlist that made `carlos ledger publish -app a prompts` silently unpublish carbon and wipe the title — while printing "published a: prompts now public" — and made publishing two ledgers one call at a time impossible. It is now read-modify-write: ledger.UpdateManifest CASes on the manifest's etag and re-READS and re-APPLIES on a lost race, so two contributors publishing two chains at once end with both published. public:true adds the named ledgers and keeps the stored title when the request omits one; public:false removes just those, flipping public off only when the set empties; an unpublish against an app with no manifest is a no-op, not a manifest conjured from nothing. The response is the RESULTING manifest and the CLI prints it — "published titogo: public ledgers now [carbon prompts]" — because after a read-modify-write only the server can say what is public now. Also: Manifest.Title is trimmed and capped at 120 bytes at the write door (member text served publicly at ledgers.json); all five ledger handlers wrap the request context with the same 15s store timeout their apicli.go siblings use; the edge negative-caches the "no readable public manifest" decision for the same 60s the positive cache trusts an object, bounded at 1024 entries because the prober picks the account and app path segments; Verify pins its walk to the ledger under test (every entry names it, every key followed lives under its own entries/ prefix), so a chain cannot graft another ledger's history onto itself and still verify clean; and the ledger verbs stop registering -registry, which named the direct-bucket mode ledgerAPISession exists to refuse.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
11 files changed,
+779
−69
apimode.go+16 −8apimode_test.go+65 −10cli.go+41 −14internal/console/apiledger.go+97 −9internal/console/apiledger_test.go+149 −0internal/edge/status.go+66 −9internal/edge/status_test.go+118 −0internal/ledger/manifest.go+55 −19internal/ledger/manifest_test.go+101 −0internal/ledger/verify.go+16 −0internal/ledger/verify_test.go+55 −0
diff --git a/apimode.go b/apimode.go| index 5bb5048db..0f22b2679 100644 |
| --- a/apimode.go |
| +++ b/apimode.go |
| @@ -418,19 +418,27 @@ func (s *apiSession) ledgerList(ctx context.Context, account, app string) ([]byt |
| return body, nil |
| } |
| -// ledgerPublish sets which of an app's ledgers are served publicly (and |
| -// under what title) through the console. There is no noop/created |
| -// distinction to report — handleAPILedgerPublish answers every success with |
| -// the same "{}" body — so this stays error-only, like rollback. |
| -func (s *apiSession) ledgerPublish(ctx context.Context, account, app string, m ledger.Manifest) error { |
| +// ledgerPublish adds (or, with public:false, removes) named ledgers from the |
| +// set an app serves publicly. The console applies it as a read-modify-write |
| +// on the stored manifest, so the request says what to CHANGE and only the |
| +// response says what is public NOW — a set that may well name ledgers this |
| +// call never mentioned. That resulting manifest is the return value: without |
| +// it the CLI could only echo the caller's own argument back at them, which |
| +// is exactly how someone comes to believe they unpublished a chain they |
| +// didn't. |
| +func (s *apiSession) ledgerPublish(ctx context.Context, account, app string, m ledger.Manifest) (ledger.Manifest, error) { |
| status, body, err := s.postJSON(ctx, apiPath(account, app, "/ledgers/publish"), m) |
| if err != nil { |
| - return err |
| + return ledger.Manifest{}, err |
| } |
| if status != http.StatusOK { |
| - return apiErrorFromBody(status, body) |
| + return ledger.Manifest{}, apiErrorFromBody(status, body) |
| + } |
| + var out ledger.Manifest |
| + if err := json.Unmarshal(body, &out); err != nil { |
| + return ledger.Manifest{}, fmt.Errorf("ledger publish: unreadable response: %w", err) |
| } |
| - return nil |
| + return out, nil |
| } |
| // ledgerBlob posts raw bytes (not JSON) to the ledger's blobs door — a |
diff --git a/apimode_test.go b/apimode_test.go| index 7f28e7757..8a05cecfe 100644 |
| --- a/apimode_test.go |
| +++ b/apimode_test.go |
| @@ -1364,21 +1364,30 @@ func TestAPISessionLedgerList(t *testing.T) { |
| } |
| } |
| +// TestAPISessionLedgerPublish: publish is read-modify-write server-side, so |
| +// the answer is the RESULTING manifest — the set that is public now, which |
| +// may name ledgers this request never mentioned. A method that discarded the |
| +// body would leave the CLI with nothing honest to print. |
| func TestAPISessionLedgerPublish(t *testing.T) { |
| var gotPath, gotBody string |
| ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { |
| gotPath = r.URL.Path |
| b, _ := io.ReadAll(r.Body) |
| gotBody = string(b) |
| - w.Write([]byte("{}")) |
| + json.NewEncoder(w).Encode(ledger.Manifest{ |
| + Schema: 1, Public: true, Ledgers: []string{"prompts", "carbon"}, Title: "T"}) |
| })) |
| defer ts.Close() |
| s := newTestSession(ts.URL) |
| - if err := s.ledgerPublish(context.Background(), "acct", "app", |
| - ledger.Manifest{Public: true, Ledgers: []string{"carbon"}, Title: "T"}); err != nil { |
| + m, err := s.ledgerPublish(context.Background(), "acct", "app", |
| + ledger.Manifest{Public: true, Ledgers: []string{"carbon"}, Title: "T"}) |
| + if err != nil { |
| t.Fatalf("ledgerPublish: %v", err) |
| } |
| + if !m.Public || len(m.Ledgers) != 2 || m.Ledgers[1] != "carbon" { |
| + t.Fatalf("ledgerPublish = %+v, want the server's RESULTING manifest, not the request", m) |
| + } |
| if gotPath != "/api/cli/apps/acct/app/ledgers/publish" { |
| t.Fatalf("path = %q", gotPath) |
| } |
| @@ -1439,6 +1448,24 @@ func TestAPISessionLedgerBlobError(t *testing.T) { |
| } |
| } |
| +// TestLedgerVerbsOfferNoRegistryFlag: -registry names the local database |
| +// that resolves an account in DIRECT-bucket mode, and ledgerAPISession |
| +// refuses direct mode outright (spec §2.3). A ledger verb that advertised |
| +// -registry would be offering a mode it will then refuse — and the flag |
| +// would be dead besides, since nothing on this path opens a registry (the |
| +// requireNoRegistryTouch assertions elsewhere in this file are the other |
| +// half of that proof). |
| +func TestLedgerVerbsOfferNoRegistryFlag(t *testing.T) { |
| + fs := flag.NewFlagSet("ledger", flag.ContinueOnError) |
| + ledgerAccountFlag(fs) |
| + if fs.Lookup("account") == nil { |
| + t.Fatal("ledger verbs must still take -account") |
| + } |
| + if f := fs.Lookup("registry"); f != nil { |
| + t.Fatalf("ledger verbs register -registry (%q) — direct-bucket mode is refused, not configured", f.Usage) |
| + } |
| +} |
| + |
| // ---- Task 6: the ledger verbs wired to resolveMode / public HTTPS -------- |
| // TestLedgerAppendVerbAPIMode pins runLedgerAppend's console wiring: it |
| @@ -1638,7 +1665,10 @@ func TestLedgerPublishVerbAPIMode(t *testing.T) { |
| gotPath = r.URL.Path |
| b, _ := io.ReadAll(r.Body) |
| gotBody = string(b) |
| - w.Write([]byte("{}")) |
| + // The server's answer names a ledger this call never mentioned: |
| + // publish is additive, so the resulting set is bigger than the ask. |
| + json.NewEncoder(w).Encode(ledger.Manifest{ |
| + Schema: 1, Public: true, Ledgers: []string{"prompts", "carbon"}, Title: "Titogo"}) |
| })) |
| defer ts.Close() |
| loginTo(t, ts.URL, "vicky@keymail.dev") |
| @@ -1652,13 +1682,17 @@ func TestLedgerPublishVerbAPIMode(t *testing.T) { |
| if !strings.Contains(gotBody, `"public":true`) || !strings.Contains(gotBody, `"carbon"`) || !strings.Contains(gotBody, `"Titogo"`) { |
| t.Fatalf("publish body = %q", gotBody) |
| } |
| - if !strings.Contains(buf.String(), "published titogo: carbon now public") { |
| - t.Fatalf("stdout = %q", buf.String()) |
| + // The line reports the RESULTING public set, not the request's argument |
| + // — printing "carbon now public" while prompts is also public is how a |
| + // caller ends up believing they unpublished something they didn't. |
| + if !strings.Contains(buf.String(), "published titogo: public ledgers now [prompts carbon]") { |
| + t.Fatalf("stdout = %q, want the resulting public set", buf.String()) |
| } |
| } |
| // TestLedgerPublishVerbUnpublish pins -public=false's own message (an |
| -// unpublish is not the same act as a publish, and must not read like one). |
| +// unpublish is not the same act as a publish, and must not read like one), |
| +// and that it too reports what is left public rather than what was removed. |
| func TestLedgerPublishVerbUnpublish(t *testing.T) { |
| useTempCredentials(t) |
| clearDeploymentEnv(t) |
| @@ -1667,7 +1701,8 @@ func TestLedgerPublishVerbUnpublish(t *testing.T) { |
| ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { |
| b, _ := io.ReadAll(r.Body) |
| gotBody = string(b) |
| - w.Write([]byte("{}")) |
| + json.NewEncoder(w).Encode(ledger.Manifest{ |
| + Schema: 1, Public: true, Ledgers: []string{"prompts"}, Title: "Titogo"}) |
| })) |
| defer ts.Close() |
| loginTo(t, ts.URL, "vicky@keymail.dev") |
| @@ -1678,8 +1713,28 @@ func TestLedgerPublishVerbUnpublish(t *testing.T) { |
| if !strings.Contains(gotBody, `"public":false`) { |
| t.Fatalf("publish body = %q, want public:false", gotBody) |
| } |
| - if !strings.Contains(buf.String(), "unpublished titogo: carbon no longer public") { |
| - t.Fatalf("stdout = %q", buf.String()) |
| + if !strings.Contains(buf.String(), "unpublished titogo: public ledgers now [prompts]") { |
| + t.Fatalf("stdout = %q, want what is still public", buf.String()) |
| + } |
| +} |
| + |
| +// TestLedgerPublishVerbUnpublishLast: when the last ledger goes, the line |
| +// says so plainly rather than printing an empty bracket. |
| +func TestLedgerPublishVerbUnpublishLast(t *testing.T) { |
| + useTempCredentials(t) |
| + clearDeploymentEnv(t) |
| + |
| + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { |
| + json.NewEncoder(w).Encode(ledger.Manifest{Schema: 1, Public: false, Title: "Titogo"}) |
| + })) |
| + defer ts.Close() |
| + loginTo(t, ts.URL, "vicky@keymail.dev") |
| + |
| + buf := captureStdout(t) |
| + runLedgerPublish([]string{"-app", "titogo", "-account", apiTestAccount, "-public=false", "carbon"}) |
| + |
| + if !strings.Contains(buf.String(), "unpublished titogo: no public ledgers") { |
| + t.Fatalf("stdout = %q, want the emptied-set message", buf.String()) |
| } |
| } |
diff --git a/cli.go b/cli.go| index f89a99d79..3594d88b7 100644 |
| --- a/cli.go |
| +++ b/cli.go |
| @@ -1152,10 +1152,13 @@ commands: |
| publish manifest (what's public, if |
| anything) |
| publish -app <a> [-account <acct>] [-public=true|false] [-title <t>] <ledger>... |
| - set which ledgers this app serves |
| - publicly, and under what title |
| - (-public defaults to true; publish |
| - -public=false unpublishes) |
| + add these ledgers to what this app |
| + serves publicly (additive: the ones |
| + already public stay public, and an |
| + omitted -title keeps the stored one); |
| + -public=false removes just these, |
| + taking the app private only when |
| + nothing is left |
| blob -app <a> [-account <acct>] <ledger> <file>... |
| upload one or more content-addressed |
| blobs (create-only; re-upload of |
| @@ -1221,10 +1224,23 @@ func ledgerAPISession(consoleFlag string) *apiSession { |
| return sess |
| } |
| +// ledgerAccountFlag registers the ONE account flag a ledger verb has, in |
| +// place of accountFlags. accountFlags also registers -registry, the local |
| +// registry database that resolves an account name in DIRECT-bucket mode — |
| +// and ledgerAPISession refuses direct mode outright (spec §2.3: no bucket |
| +// credentials in a terminal). A -registry flag on these verbs is therefore |
| +// dead: nothing here ever opens a registry, and offering the flag advertises |
| +// a mode that does not exist. The account is resolved server-side, by the |
| +// console, against the caller's own memberships — which is why -account |
| +// takes a sqid OR a name here and needs no local database to do it. |
| +func ledgerAccountFlag(fs *flag.FlagSet) *string { |
| + return fs.String("account", "ops", "account (sqid or name) the app belongs to — resolved by the console") |
| +} |
| + |
| func runLedgerAppend(args []string) { |
| fs := flag.NewFlagSet("ledger append", flag.ExitOnError) |
| app := fs.String("app", "", "app name") |
| - acctRaw, _ := accountFlags(fs) |
| + acctRaw := ledgerAccountFlag(fs) |
| consoleFlag := fs.String("console", "", "console origin to append through (default: the only logged-in one)") |
| fs.Parse(args) |
| if *app == "" || fs.NArg() != 2 { |
| @@ -1268,7 +1284,7 @@ func runLedgerAppend(args []string) { |
| func runLedgerHead(args []string) { |
| fs := flag.NewFlagSet("ledger head", flag.ExitOnError) |
| app := fs.String("app", "", "app name") |
| - acctRaw, _ := accountFlags(fs) |
| + acctRaw := ledgerAccountFlag(fs) |
| consoleFlag := fs.String("console", "", "console origin to read through (default: the only logged-in one)") |
| fs.Parse(args) |
| if *app == "" || fs.NArg() != 1 { |
| @@ -1294,7 +1310,7 @@ func runLedgerHead(args []string) { |
| func runLedgerList(args []string) { |
| fs := flag.NewFlagSet("ledger list", flag.ExitOnError) |
| app := fs.String("app", "", "app name") |
| - acctRaw, _ := accountFlags(fs) |
| + acctRaw := ledgerAccountFlag(fs) |
| consoleFlag := fs.String("console", "", "console origin to read through (default: the only logged-in one)") |
| fs.Parse(args) |
| if *app == "" { |
| @@ -1342,7 +1358,7 @@ func runLedgerList(args []string) { |
| func runLedgerPublish(args []string) { |
| fs := flag.NewFlagSet("ledger publish", flag.ExitOnError) |
| app := fs.String("app", "", "app name") |
| - acctRaw, _ := accountFlags(fs) |
| + acctRaw := ledgerAccountFlag(fs) |
| public := fs.Bool("public", true, "serve these ledgers publicly (-public=false unpublishes)") |
| title := fs.String("title", "", "display title for the published set") |
| consoleFlag := fs.String("console", "", "console origin to publish through (default: the only logged-in one)") |
| @@ -1360,20 +1376,31 @@ func runLedgerPublish(args []string) { |
| sess := ledgerAPISession(*consoleFlag) |
| fmt.Fprintf(stdout, "using %s as %s\n", sess.console, sess.address) |
| m := ledger.Manifest{Public: *public, Ledgers: names, Title: *title} |
| - if err := sess.ledgerPublish(context.Background(), *acctRaw, *app, m); err != nil { |
| + got, err := sess.ledgerPublish(context.Background(), *acctRaw, *app, m) |
| + if err != nil { |
| log.Fatal(err) |
| } |
| - if *public { |
| - fmt.Fprintf(stdout, "published %s: %s now public\n", *app, strings.Join(names, ", ")) |
| - } else { |
| - fmt.Fprintf(stdout, "unpublished %s: %s no longer public\n", *app, strings.Join(names, ", ")) |
| + // Report the RESULTING public set, never the argument: publish is |
| + // additive and unpublish removes by name, so the app's public set after |
| + // this call is something only the server can say. Echoing `names` here |
| + // would tell a caller who published `prompts` that prompts is what is |
| + // public — when carbon is public too, or, on an unpublish, when carbon is |
| + // what is left. |
| + verb := "published" |
| + if !*public { |
| + verb = "unpublished" |
| + } |
| + if len(got.Ledgers) == 0 { |
| + fmt.Fprintf(stdout, "%s %s: no public ledgers\n", verb, *app) |
| + return |
| } |
| + fmt.Fprintf(stdout, "%s %s: public ledgers now [%s]\n", verb, *app, strings.Join(got.Ledgers, " ")) |
| } |
| func runLedgerBlob(args []string) { |
| fs := flag.NewFlagSet("ledger blob", flag.ExitOnError) |
| app := fs.String("app", "", "app name") |
| - acctRaw, _ := accountFlags(fs) |
| + acctRaw := ledgerAccountFlag(fs) |
| consoleFlag := fs.String("console", "", "console origin to upload through (default: the only logged-in one)") |
| fs.Parse(args) |
| if *app == "" || fs.NArg() < 2 { |
diff --git a/internal/console/apiledger.go b/internal/console/apiledger.go| index 2195a0fa6..a4bd2aa78 100644 |
| --- a/internal/console/apiledger.go |
| +++ b/internal/console/apiledger.go |
| @@ -20,6 +20,7 @@ |
| package console |
| import ( |
| + "context" |
| "encoding/json" |
| "errors" |
| "fmt" |
| @@ -27,6 +28,7 @@ import ( |
| "log" |
| "net/http" |
| "os" |
| + "slices" |
| "strconv" |
| "strings" |
| "time" |
| @@ -35,6 +37,13 @@ import ( |
| "github.com/carlosframework/platform/internal/release" |
| ) |
| +// ledgerStoreTimeout bounds every store round trip these handlers make, the |
| +// same 15s its siblings in apicli.go use: a wedged bucket must cost one |
| +// request 15 seconds, not a console goroutine forever. Each handler derives |
| +// its own context from the request's, so a client that hangs up still |
| +// cancels immediately — this only adds a ceiling. |
| +const ledgerStoreTimeout = 15 * time.Second |
| + |
| // ledgerWriteAllowed is the write budget every ledger WRITE door passes, in |
| // the same shape and with the same key as promote/rollback/new-account: the |
| // principal a write budget belongs to is the PERSON, not the terminal they |
| @@ -104,7 +113,9 @@ func (s *Server) handleAPILedgerAppend(w http.ResponseWriter, r *http.Request) { |
| jsonError(w, http.StatusRequestEntityTooLarge, "payload exceeds the 64 KiB cap") |
| return |
| } |
| - a, err := ledger.Append(r.Context(), s.cfg.Releases, acct.Sqid, app, name, addr, |
| + ctx, cancel := context.WithTimeout(r.Context(), ledgerStoreTimeout) |
| + defer cancel() |
| + a, err := ledger.Append(ctx, s.cfg.Releases, acct.Sqid, app, name, addr, |
| in.Payload, time.Now) |
| if err != nil { |
| // Every caller-caused failure has been answered above — an invalid |
| @@ -128,7 +139,9 @@ func (s *Server) handleAPILedgerHead(w http.ResponseWriter, r *http.Request) { |
| jsonError(w, http.StatusBadRequest, "invalid ledger name") |
| return |
| } |
| - b, _, err := s.cfg.Releases.Get(r.Context(), ledger.HeadKey(acct.Sqid, app, name)) |
| + ctx, cancel := context.WithTimeout(r.Context(), ledgerStoreTimeout) |
| + defer cancel() |
| + b, _, err := s.cfg.Releases.Get(ctx, ledger.HeadKey(acct.Sqid, app, name)) |
| if errors.Is(err, os.ErrNotExist) { |
| http.NotFound(w, r) |
| return |
| @@ -170,7 +183,9 @@ func (s *Server) handleAPILedgerBlob(w http.ResponseWriter, r *http.Request) { |
| jsonError(w, http.StatusBadRequest, "could not read the request body") |
| return |
| } |
| - sha, err := ledger.PutBlob(r.Context(), s.cfg.Releases, acct.Sqid, app, name, body) |
| + ctx, cancel := context.WithTimeout(r.Context(), ledgerStoreTimeout) |
| + defer cancel() |
| + sha, err := ledger.PutBlob(ctx, s.cfg.Releases, acct.Sqid, app, name, body) |
| if err != nil { |
| s.ledgerStoreError(w, fmt.Sprintf("ledger blob %s/%s/%s", acct.Sqid, app, name), err) |
| return |
| @@ -184,7 +199,9 @@ func (s *Server) handleAPILedgerList(w http.ResponseWriter, r *http.Request) { |
| if !ok { |
| return |
| } |
| - m, err := ledger.ReadManifest(r.Context(), s.cfg.Releases, acct.Sqid, app) |
| + ctx, cancel := context.WithTimeout(r.Context(), ledgerStoreTimeout) |
| + defer cancel() |
| + m, err := ledger.ReadManifest(ctx, s.cfg.Releases, acct.Sqid, app) |
| var manifest *ledger.Manifest |
| switch { |
| case err == nil: |
| @@ -203,7 +220,7 @@ func (s *Server) handleAPILedgerList(w http.ResponseWriter, r *http.Request) { |
| // directory, so the children are exactly the chain names; ledgers.json |
| // is a file at this level, and ListDirs never returns files. |
| prefix := "ledgers/" + acct.Sqid + "/" + app + "/" |
| - dirs, err := s.cfg.Releases.ListDirs(r.Context(), prefix) |
| + dirs, err := s.cfg.Releases.ListDirs(ctx, prefix) |
| if err != nil { |
| s.ledgerStoreError(w, fmt.Sprintf("ledger list %s/%s", acct.Sqid, app), err) |
| return |
| @@ -220,6 +237,35 @@ func (s *Server) handleAPILedgerList(w http.ResponseWriter, r *http.Request) { |
| json.NewEncoder(w).Encode(map[string]any{"manifest": manifest, "ledgers": names}) |
| } |
| +// maxLedgerTitle bounds Manifest.Title, the one member-supplied string on |
| +// this API that is served verbatim to the public (ledgers.json, and from |
| +// there whatever page renders it). 120 bytes is a display title's worth; the |
| +// cap is here, at the write door, because that is the only place a bound |
| +// holds for every reader that will ever fetch the object. |
| +const maxLedgerTitle = 120 |
| + |
| +// 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 |
| +// handler that stored the request's manifest wholesale would silently |
| +// unpublish every chain the caller didn't name in this one call, while |
| +// answering success. Hence: |
| +// |
| +// - public:true — add the named ledgers to the stored set (order kept, no |
| +// duplicates), set public, and replace the title only if the request |
| +// carries one. Publishing a second chain is not a request to erase the |
| +// app's title. |
| +// - public:false — remove the named ledgers. public flips off only when |
| +// the set empties, so unpublishing one of two chains leaves the other |
| +// serving. The title survives: unpublishing is not forgetting. |
| +// |
| +// The CAS is ledger.UpdateManifest's, which re-reads and re-applies on a |
| +// lost race rather than re-putting a stale copy — two contributors |
| +// publishing two chains at once must end with both published. |
| +// |
| +// The response is the RESULTING manifest, not an empty ack: the caller's |
| +// request said what to change, and only the answer can say what is public |
| +// now. |
| func (s *Server) handleAPILedgerPublish(w http.ResponseWriter, r *http.Request) { |
| addr, acct, app, ok := s.apiAccountApp(w, r) |
| if !ok { |
| @@ -240,16 +286,58 @@ func (s *Server) handleAPILedgerPublish(w http.ResponseWriter, r *http.Request) |
| return |
| } |
| } |
| - if err := ledger.WriteManifest(r.Context(), s.cfg.Releases, acct.Sqid, app, in); err != nil { |
| + title := strings.TrimSpace(in.Title) |
| + if len(title) > maxLedgerTitle { |
| + jsonError(w, http.StatusBadRequest, |
| + fmt.Sprintf("title too long (max %d bytes)", maxLedgerTitle)) |
| + return |
| + } |
| + |
| + // An unpublish against an app with no manifest is a no-op success: there |
| + // is nothing to take private, and conjuring a public:false manifest for |
| + // an app that never published one would be a write with no meaning. |
| + ctx, cancel := context.WithTimeout(r.Context(), ledgerStoreTimeout) |
| + defer cancel() |
| + if !in.Public { |
| + if _, err := ledger.ReadManifest(ctx, s.cfg.Releases, acct.Sqid, app); errors.Is(err, os.ErrNotExist) { |
| + w.Header().Set("Content-Type", "application/json") |
| + json.NewEncoder(w).Encode(ledger.Manifest{}) |
| + return |
| + } |
| + } |
| + |
| + m, err := ledger.UpdateManifest(ctx, s.cfg.Releases, acct.Sqid, app, func(m *ledger.Manifest) { |
| + if in.Public { |
| + m.Public = true |
| + for _, n := range in.Ledgers { |
| + if !slices.Contains(m.Ledgers, n) { |
| + m.Ledgers = append(m.Ledgers, n) |
| + } |
| + } |
| + if title != "" { |
| + m.Title = title |
| + } |
| + return |
| + } |
| + m.Ledgers = slices.DeleteFunc(m.Ledgers, func(n string) bool { |
| + return slices.Contains(in.Ledgers, n) |
| + }) |
| + if len(m.Ledgers) == 0 { |
| + m.Public = false |
| + } |
| + }) |
| + if err != nil { |
| s.ledgerStoreError(w, fmt.Sprintf("ledger publish %s/%s", acct.Sqid, app), err) |
| return |
| } |
| // Flipping an app's ledgers public is exactly the kind of act the audit |
| // trail exists for, and the ONLY one on this API that changes what the |
| // world can see — the same trail promote, rollback and ship write to. |
| + // The line records the RESULTING state, which is what a reader of the |
| + // trail needs to know; the request alone would not say it. |
| s.audit(acct, app, addr, "ledger-publish", |
| - "set ledgers public="+strconv.FormatBool(in.Public)+ |
| - " ["+strings.Join(in.Ledgers, " ")+"] (cli)") |
| + "set ledgers public="+strconv.FormatBool(m.Public)+ |
| + " ["+strings.Join(m.Ledgers, " ")+"] (cli)") |
| w.Header().Set("Content-Type", "application/json") |
| - w.Write([]byte("{}")) |
| + json.NewEncoder(w).Encode(m) |
| } |
diff --git a/internal/console/apiledger_test.go b/internal/console/apiledger_test.go| index 050c81e09..b49ff0746 100644 |
| --- a/internal/console/apiledger_test.go |
| +++ b/internal/console/apiledger_test.go |
| @@ -313,6 +313,155 @@ func TestAPILedgerPublishAndList(t *testing.T) { |
| } |
| } |
| +// publishLedgers posts one publish call and decodes the RESULTING manifest |
| +// the handler answers with — the shape a CLI needs in order to tell the user |
| +// what is public now rather than what they just asked for. |
| +func publishLedgers(t *testing.T, fx *consoleFixture, token, body string) (int, ledger.Manifest) { |
| + t.Helper() |
| + status, out := postBody(t, fx.ts.URL+"/api/cli/apps/bab/titogo/ledgers/publish", token, body) |
| + var m ledger.Manifest |
| + if status == http.StatusOK { |
| + if err := json.Unmarshal(out, &m); err != nil { |
| + t.Fatalf("publish body %s: %v", out, err) |
| + } |
| + } |
| + return status, m |
| +} |
| + |
| +// TestAPILedgerPublishIsAdditive is the defect this handler was written |
| +// around: an app's manifest is a SET of published ledgers accrued over |
| +// separate publish calls, and the edge reads that set as a per-ledger |
| +// ALLOWLIST. A publish that replaced the whole manifest would silently |
| +// unpublish every ledger the caller didn't name in this one call — while the |
| +// CLI printed a success line — and would make publishing two ledgers |
| +// impossible one call at a time. |
| +func TestAPILedgerPublishIsAdditive(t *testing.T) { |
| + fx, token := ledgerFixture(t, "code-ledger-additive") |
| + |
| + status, m := publishLedgers(t, fx, token, `{"public":true,"ledgers":["carbon"],"title":"Titogo"}`) |
| + if status != http.StatusOK { |
| + t.Fatalf("first publish = %d", status) |
| + } |
| + if !m.Public || len(m.Ledgers) != 1 || m.Ledgers[0] != "carbon" || m.Title != "Titogo" { |
| + t.Fatalf("first publish = %+v", m) |
| + } |
| + |
| + // Publishing a SECOND ledger keeps the first, and an omitted title keeps |
| + // the stored one (a caller publishing one more chain is not asking to |
| + // erase the app's title). |
| + status, m = publishLedgers(t, fx, token, `{"public":true,"ledgers":["prompts"]}`) |
| + if status != http.StatusOK { |
| + t.Fatalf("second publish = %d", status) |
| + } |
| + if len(m.Ledgers) != 2 || m.Ledgers[0] != "carbon" || m.Ledgers[1] != "prompts" { |
| + t.Fatalf("second publish = %+v, want [carbon prompts] in order", m.Ledgers) |
| + } |
| + if m.Title != "Titogo" { |
| + t.Fatalf("title = %q, want the stored title preserved when the request omits one", m.Title) |
| + } |
| + |
| + // Re-publishing one that is already public is idempotent — no duplicate. |
| + if _, m = publishLedgers(t, fx, token, `{"public":true,"ledgers":["carbon"]}`); len(m.Ledgers) != 2 { |
| + t.Fatalf("re-publish = %+v, want no duplicate", m.Ledgers) |
| + } |
| + // A non-empty title replaces the stored one. |
| + if _, m = publishLedgers(t, fx, token, `{"public":true,"ledgers":["carbon"],"title":"Tito"}`); m.Title != "Tito" { |
| + t.Fatalf("title = %q, want the new one", m.Title) |
| + } |
| + |
| + // The store agrees with what the handler reported. |
| + stored, err := ledger.ReadManifest(context.Background(), fx.rel, "bab", "titogo") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if len(stored.Ledgers) != 2 || stored.Title != "Tito" || !stored.Public { |
| + t.Fatalf("stored manifest = %+v, want it to match the reported state", stored) |
| + } |
| +} |
| + |
| +// TestAPILedgerUnpublishRemovesOne: unpublish is the mirror — it removes the |
| +// named ledgers and flips public off ONLY when nothing is left. Unpublishing |
| +// one of two chains must not take the other one dark. |
| +func TestAPILedgerUnpublishRemovesOne(t *testing.T) { |
| + fx, token := ledgerFixture(t, "code-ledger-unpublish") |
| + |
| + if status, _ := publishLedgers(t, fx, token, `{"public":true,"ledgers":["carbon","prompts"],"title":"T"}`); status != http.StatusOK { |
| + t.Fatalf("publish = %d", status) |
| + } |
| + |
| + status, m := publishLedgers(t, fx, token, `{"public":false,"ledgers":["carbon"]}`) |
| + if status != http.StatusOK { |
| + t.Fatalf("unpublish = %d", status) |
| + } |
| + if !m.Public { |
| + t.Fatalf("unpublish of one of two = %+v, want public still true", m) |
| + } |
| + if len(m.Ledgers) != 1 || m.Ledgers[0] != "prompts" { |
| + t.Fatalf("unpublish left %v, want [prompts]", m.Ledgers) |
| + } |
| + |
| + // Unpublishing the last one empties the list AND flips public off, so |
| + // the app's own ledgers.json index stops serving too. |
| + status, m = publishLedgers(t, fx, token, `{"public":false,"ledgers":["prompts"]}`) |
| + if status != http.StatusOK { |
| + t.Fatalf("final unpublish = %d", status) |
| + } |
| + if m.Public || len(m.Ledgers) != 0 { |
| + t.Fatalf("final unpublish = %+v, want public:false and an empty list", m) |
| + } |
| + if m.Title != "T" { |
| + t.Fatalf("title = %q, want it kept — unpublishing is not forgetting", m.Title) |
| + } |
| +} |
| + |
| +// TestAPILedgerUnpublishWithoutManifest: unpublishing an app that never |
| +// published is a no-op success, not a 404 and not a manifest conjured out of |
| +// nothing. |
| +func TestAPILedgerUnpublishWithoutManifest(t *testing.T) { |
| + fx, token := ledgerFixture(t, "code-ledger-unpublish-none") |
| + |
| + status, m := publishLedgers(t, fx, token, `{"public":false,"ledgers":["carbon"]}`) |
| + if status != http.StatusOK { |
| + t.Fatalf("unpublish with no manifest = %d, want 200", status) |
| + } |
| + if m.Public || len(m.Ledgers) != 0 { |
| + t.Fatalf("= %+v, want the zero manifest", m) |
| + } |
| + if _, _, err := fx.rel.Get(context.Background(), ledger.ManifestKey("bab", "titogo")); err == nil { |
| + t.Fatal("a no-op unpublish wrote a manifest, want none") |
| + } |
| +} |
| + |
| +// TestAPILedgerPublishTitleCap: the title is member-supplied text served |
| +// publicly at ledgers.json, so it is bounded at the write door — the same |
| +// place every other public-facing input on this API is bounded. |
| +func TestAPILedgerPublishTitleCap(t *testing.T) { |
| + fx, token := ledgerFixture(t, "code-ledger-title") |
| + |
| + long := `{"public":true,"ledgers":["carbon"],"title":"` + strings.Repeat("t", 121) + `"}` |
| + status, out := postBody(t, fx.ts.URL+"/api/cli/apps/bab/titogo/ledgers/publish", token, long) |
| + if status != http.StatusBadRequest { |
| + t.Fatalf("oversize title = %d %s, want 400", status, out) |
| + } |
| + if !strings.Contains(string(out), "title too long") { |
| + t.Fatalf("refusal = %s, want it to name the cap", out) |
| + } |
| + if _, _, err := fx.rel.Get(context.Background(), ledger.ManifestKey("bab", "titogo")); err == nil { |
| + t.Fatal("a refused publish wrote a manifest") |
| + } |
| + |
| + // Exactly at the cap is fine, and surrounding whitespace is trimmed |
| + // rather than counted. |
| + at := strings.Repeat("t", 120) |
| + status, m := publishLedgers(t, fx, token, `{"public":true,"ledgers":["carbon"],"title":" `+at+` "}`) |
| + if status != http.StatusOK { |
| + t.Fatalf("title at the cap = %d, want 200", status) |
| + } |
| + if m.Title != at { |
| + t.Fatalf("title = %q, want it trimmed to the %d-byte value", m.Title, len(at)) |
| + } |
| +} |
| + |
| // 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 777581e62..90439336c 100644 |
| --- a/internal/edge/status.go |
| +++ b/internal/edge/status.go |
| @@ -88,21 +88,63 @@ type statusHandler struct { |
| mu sync.Mutex |
| cache map[string]statusCached |
| + // misses is the negative half of the ledger gate: manifest key → when |
| + // the "not servable" decision was taken. See markMiss. |
| + misses map[string]time.Time |
| } |
| +// statusMissCacheMax bounds the negative cache. It exists because the |
| +// positive cache's bound — "get only records an entry after a SUCCESSFUL |
| +// store read, so only objects that exist can grow it" — is exactly the |
| +// property a negative cache gives up: the prober picks the account and app |
| +// segments, so the set of manifest keys they can name is unbounded. 1024 |
| +// apps' worth of decisions is far more than any real deployment publishes |
| +// and costs a few tens of KiB; past it the map is CLEARED rather than |
| +// evicted one entry at a time, because there is no access pattern here worth |
| +// approximating with an LRU — the honest read of a full map is "this is |
| +// being probed", and a probe's entries are the ones worth dropping. |
| +const statusMissCacheMax = 1024 |
| + |
| // newStatusHandler builds the handler for one status route. store is shared |
| // with the static serving class (see EnableStatic's doc comment) — the |
| // status page has no per-route config of its own (no app, no channel), so |
| // unlike newStaticHandler this takes no registry.Route. |
| func newStatusHandler(store release.Store) *statusHandler { |
| return &statusHandler{ |
| - store: store, |
| - ttl: statusCacheTTL, |
| - now: time.Now, |
| - cache: make(map[string]statusCached), |
| + store: store, |
| + ttl: statusCacheTTL, |
| + now: time.Now, |
| + cache: make(map[string]statusCached), |
| + misses: make(map[string]time.Time), |
| } |
| } |
| +// missFresh reports whether the "not servable" decision for this manifest |
| +// key was taken recently enough to reuse. |
| +func (h *statusHandler) missFresh(key string) bool { |
| + h.mu.Lock() |
| + defer h.mu.Unlock() |
| + at, ok := h.misses[key] |
| + 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. |
| +// |
| +// The bound is the point: see statusMissCacheMax. |
| +func (h *statusHandler) markMiss(key string) { |
| + h.mu.Lock() |
| + defer h.mu.Unlock() |
| + if len(h.misses) >= statusMissCacheMax { |
| + if _, ok := h.misses[key]; !ok { |
| + clear(h.misses) |
| + } |
| + } |
| + h.misses[key] = h.now() |
| +} |
| + |
| func (h *statusHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { |
| p := path.Clean(r.URL.Path) |
| if strings.HasPrefix(p, ledgerPrefix) { |
| @@ -152,10 +194,13 @@ func (h *statusHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { |
| // public app, one HEAD per ledger — so they ride h.get's shared 60s |
| // cache. Revoking public therefore takes at most one TTL, and a cached |
| // public manifest keeps serving through a store outage exactly like any |
| -// other cached object (get's stale-on-error half). The cache map cannot |
| -// be grown by a stranger probing random sqids: get only writes an entry |
| -// AFTER a successful store read (see its tail), so a miss — the whole |
| -// of "no such app" — is never cached. Bounded by what actually exists. |
| +// other cached object (get's stale-on-error half). That map cannot be |
| +// grown by a stranger probing random sqids: get only writes an entry |
| +// AFTER a successful store read (see its tail), so it is bounded by what |
| +// actually exists. The REFUSALS a prober generates are remembered |
| +// separately, in the explicitly bounded negative cache (markMiss), so a |
| +// probe costs a bucket GET at most once per TTL per app rather than once |
| +// per request. |
| // - entries/*.json and blobs/* are IMMUTABLE and unbounded in number |
| // (an entry per append, blobs up to ledger.MaxBlob each), so they are |
| // NOT cached in this process at all: a direct store read plus a |
| @@ -189,13 +234,25 @@ func (h *statusHandler) serveLedger(w http.ResponseWriter, r *http.Request, p st |
| } |
| sqid, app := parts[1], parts[2] |
| - mb, _, err := h.get(r.Context(), ledger.ManifestKey(sqid, app), "application/json") |
| + // The negative half of the gate, checked BEFORE the store: a miss is |
| + // never recorded by get (it only caches successful reads), and |
| + // status-kind responses never enter the shared response cache, so |
| + // without this every probe at a made-up account/app costs one bucket GET |
| + // — and the prober writes the path. |
| + mkey := ledger.ManifestKey(sqid, app) |
| + if h.missFresh(mkey) { |
| + http.NotFound(w, r) |
| + return |
| + } |
| + mb, _, err := h.get(r.Context(), mkey, "application/json") |
| if err != nil { |
| + h.markMiss(mkey) |
| http.NotFound(w, r) // fail-closed: no readable manifest = not public |
| return |
| } |
| var m ledger.Manifest |
| if err := json.Unmarshal(mb, &m); err != nil || !m.Public { |
| + h.markMiss(mkey) |
| http.NotFound(w, r) |
| return |
| } |
diff --git a/internal/edge/status_test.go b/internal/edge/status_test.go| index c66260550..bb698ae71 100644 |
| --- a/internal/edge/status_test.go |
| +++ b/internal/edge/status_test.go |
| @@ -7,10 +7,12 @@ package edge |
| import ( |
| "context" |
| "encoding/json" |
| + "fmt" |
| "net/http" |
| "net/http/httptest" |
| "os" |
| "path/filepath" |
| + "sync" |
| "testing" |
| "time" |
| @@ -367,6 +369,122 @@ func TestLedgerCacheClasses(t *testing.T) { |
| } |
| } |
| +// countingStore records how many times each key is read, so a test can |
| +// assert what a request COST rather than only what it answered. |
| +type countingStore struct { |
| + release.Store |
| + mu sync.Mutex |
| + gets map[string]int |
| +} |
| + |
| +func newCountingStore(st release.Store) *countingStore { |
| + return &countingStore{Store: st, gets: make(map[string]int)} |
| +} |
| + |
| +func (c *countingStore) Get(ctx context.Context, key string) ([]byte, string, error) { |
| + c.mu.Lock() |
| + c.gets[key]++ |
| + c.mu.Unlock() |
| + return c.Store.Get(ctx, key) |
| +} |
| + |
| +func (c *countingStore) count(key string) int { |
| + c.mu.Lock() |
| + defer c.mu.Unlock() |
| + return c.gets[key] |
| +} |
| + |
| +// TestLedgerManifestMissesAreNegativeCached: status-kind routes are exempt |
| +// from the shared response cache, and get() only records an entry after a |
| +// SUCCESSFUL read — so without a negative cache every anonymous request to |
| +// /ledgers/<random>/<random>/x/HEAD.json costs exactly one bucket GET, and a |
| +// stranger with a loop is billing the deployment one S3 request per packet. |
| +// The decision "no readable public manifest here" is cached for the same 60s |
| +// the positive answer is. |
| +func TestLedgerManifestMissesAreNegativeCached(t *testing.T) { |
| + st := newCountingStore(release.NewDirStore(t.TempDir())) |
| + h := newStatusHandler(st) |
| + h.ttl = 50 * time.Millisecond |
| + fakeNow := time.Now() |
| + h.now = func() time.Time { return fakeNow } |
| + |
| + key := ledger.ManifestKey("nope", "nothing") |
| + for i := range 5 { |
| + if rr := getStatus(t, h, "/ledgers/nope/nothing/carbon/HEAD.json"); rr.Code != http.StatusNotFound { |
| + t.Fatalf("probe %d = %d, want 404", i, rr.Code) |
| + } |
| + } |
| + if got := st.count(key); got != 1 { |
| + t.Fatalf("%s was read %d times for 5 probes, want 1 (the miss must be cached)", key, got) |
| + } |
| + |
| + // Different objects under the SAME app share the one decision — the |
| + // gate is the app's manifest, so walking an app's key space is not a way |
| + // around it. |
| + for _, p := range []string{ |
| + "/ledgers/nope/nothing/ledgers.json", |
| + "/ledgers/nope/nothing/carbon/entries/20260101T000000Z-000000000000.json", |
| + "/ledgers/nope/nothing/carbon/blobs/" + ledger.SHA256Hex([]byte("x")), |
| + } { |
| + if rr := getStatus(t, h, p); rr.Code != http.StatusNotFound { |
| + t.Fatalf("GET %s = %d, want 404", p, rr.Code) |
| + } |
| + } |
| + if got := st.count(key); got != 1 { |
| + t.Fatalf("%s was read %d times, want still 1", key, got) |
| + } |
| + |
| + // Past the TTL the decision is re-taken — publishing an app takes effect |
| + // within one tick, exactly like the positive cache's staleness bound. |
| + fakeNow = fakeNow.Add(51 * time.Millisecond) |
| + if rr := getStatus(t, h, "/ledgers/nope/nothing/carbon/HEAD.json"); rr.Code != http.StatusNotFound { |
| + t.Fatalf("post-TTL probe = %d, want 404", rr.Code) |
| + } |
| + if got := st.count(key); got != 2 { |
| + t.Fatalf("%s read %d times after the TTL expired, want 2", key, got) |
| + } |
| + |
| + // A non-public app is the same cached decision: the manifest read that |
| + // answered it is itself cached, so neither costs a second GET. |
| + seedLedger(t, st, false) |
| + pkey := ledger.ManifestKey("bab", "platform") |
| + before := st.count(pkey) |
| + for range 3 { |
| + if rr := getStatus(t, h, "/ledgers/bab/platform/carbon/HEAD.json"); rr.Code != http.StatusNotFound { |
| + t.Fatalf("private app = %d, want 404", rr.Code) |
| + } |
| + } |
| + if got := st.count(pkey) - before; got > 1 { |
| + t.Fatalf("a private app cost %d manifest reads for 3 probes, want at most 1", got) |
| + } |
| +} |
| + |
| +// 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 |
| +// exhaustion the immutable classes are deliberately uncached to avoid, |
| +// reintroduced through the back door. |
| +func TestLedgerMissCacheIsBounded(t *testing.T) { |
| + st := release.NewDirStore(t.TempDir()) |
| + h := newStatusHandler(st) |
| + for i := range statusMissCacheMax * 3 { |
| + p := fmt.Sprintf("/ledgers/acct%d/app%d/carbon/HEAD.json", i, i) |
| + if rr := getStatus(t, h, p); rr.Code != http.StatusNotFound { |
| + t.Fatalf("probe %d = %d, want 404", i, rr.Code) |
| + } |
| + } |
| + h.mu.Lock() |
| + n := len(h.misses) |
| + h.mu.Unlock() |
| + if n > statusMissCacheMax { |
| + t.Fatalf("negative cache holds %d entries, over the %d bound", n, statusMissCacheMax) |
| + } |
| + // The positive cache is untouched by any of it: nothing was ever read. |
| + if keys := cacheKeys(h); len(keys) != 0 { |
| + t.Fatalf("probing populated the positive cache: %v", keys) |
| + } |
| +} |
| + |
| // TestLedgerServingStaleManifestHonored pins the interaction with get()'s |
| // stale-on-error half: the manifest is just another cached object, so once a |
| // public manifest has been read, a store outage keeps the already-cached |
diff --git a/internal/ledger/manifest.go b/internal/ledger/manifest.go| index b9ea88e78..67407997e 100644 |
| --- a/internal/ledger/manifest.go |
| +++ b/internal/ledger/manifest.go |
| @@ -21,30 +21,66 @@ func ReadManifest(ctx context.Context, st release.Store, sqid, app string) (Mani |
| return m, nil |
| } |
| -// WriteManifest is read-etag-then-Put with one retry on a lost race — |
| -// the manifest is small owner-intent state where last-writer-wins is |
| -// correct (the putOverwrite pattern, internal/status/fleet.go). |
| +// WriteManifest replaces the whole manifest: read-etag-then-Put with one |
| +// retry on a lost race, last-writer-wins (the putOverwrite pattern, |
| +// internal/status/fleet.go). It is the right call for a caller that HAS the |
| +// whole intended value — a seeded fixture, a box-side tool writing the set |
| +// it just computed. A caller changing one ledger's publication wants |
| +// 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 { |
| - m.Schema = Schema |
| - body, err := json.Marshal(m) |
| - if err != nil { |
| - return err |
| - } |
| + _, err := UpdateManifest(ctx, st, sqid, app, func(cur *Manifest) { *cur = m }) |
| + return err |
| +} |
| + |
| +// manifestAttempts bounds UpdateManifest's CAS retry loop, matching |
| +// appendAttempts: publish contention is two contributors publishing the same |
| +// app in the same moment, and four attempts outlast that without spinning. |
| +const manifestAttempts = 4 |
| + |
| +// UpdateManifest is the manifest's read-modify-write door: it reads the |
| +// stored manifest (a missing one is the ZERO value, not an error — the first |
| +// publish of an app creates it), hands it to apply, and CASes the result |
| +// back on the etag it read. A lost race re-READS and re-APPLIES rather than |
| +// re-putting a stale copy, which is the whole point: the manifest is a SET |
| +// accrued across separate publish calls, so "carbon and prompts published at |
| +// the same moment" must end with both, not with whichever writer was slower. |
| +// |
| +// Contention that outlasts the retries surfaces as release.ErrCASConflict |
| +// (the console maps it to a retry-worthy 503) — never as a silent no-op. |
| +func UpdateManifest(ctx context.Context, st release.Store, sqid, app string, |
| + apply func(*Manifest)) (Manifest, error) { |
| key := ManifestKey(sqid, app) |
| - for attempt := 0; ; attempt++ { |
| - _, etag, err := st.Get(ctx, key) |
| + var lastErr error |
| + for range manifestAttempts { |
| + var m Manifest |
| + b, etag, err := st.Get(ctx, key) |
| ifMatch := etag |
| - if errors.Is(err, os.ErrNotExist) { |
| - ifMatch = "" |
| - } else if err != nil { |
| - return err |
| + switch { |
| + case errors.Is(err, os.ErrNotExist): |
| + ifMatch = "" // create-only: no manifest yet |
| + case err != nil: |
| + return Manifest{}, err |
| + default: |
| + if err := json.Unmarshal(b, &m); err != nil { |
| + return Manifest{}, err |
| + } |
| } |
| - _, err = st.Put(ctx, key, body, ifMatch) |
| - if err == nil { |
| - return nil |
| + |
| + apply(&m) |
| + m.Schema = Schema |
| + body, err := json.Marshal(m) |
| + if err != nil { |
| + return Manifest{}, err |
| } |
| - if !errors.Is(err, release.ErrCASConflict) || attempt >= 1 { |
| - return err |
| + if _, err := st.Put(ctx, key, body, ifMatch); err != nil { |
| + if errors.Is(err, release.ErrCASConflict) { |
| + lastErr = err |
| + continue |
| + } |
| + return Manifest{}, err |
| } |
| + return m, nil |
| } |
| + return Manifest{}, lastErr |
| } |
diff --git a/internal/ledger/manifest_test.go b/internal/ledger/manifest_test.go| index 58b7156f0..48ff0beab 100644 |
| --- a/internal/ledger/manifest_test.go |
| +++ b/internal/ledger/manifest_test.go |
| @@ -4,6 +4,8 @@ import ( |
| "context" |
| "errors" |
| "os" |
| + "slices" |
| + "strings" |
| "testing" |
| "github.com/carlosframework/platform/internal/release" |
| @@ -33,3 +35,102 @@ func TestManifestRoundTrip(t *testing.T) { |
| t.Fatal("manifest overwrite did not take") |
| } |
| } |
| + |
| +// TestUpdateManifestReadModifyWrite: the manifest is a SET of published |
| +// ledgers accrued over many separate publish calls, so the write path that |
| +// serves those calls must read-modify-write. A whole-value replace here is |
| +// what silently unpublishes an app's other ledgers. |
| +func TestUpdateManifestReadModifyWrite(t *testing.T) { |
| + st := release.NewDirStore(t.TempDir()) |
| + ctx := context.Background() |
| + |
| + // 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.Public = true |
| + m.Ledgers = append(m.Ledgers, "carbon") |
| + m.Title = "CARLOS" |
| + }) |
| + if err != nil { |
| + t.Fatalf("first update: %v", err) |
| + } |
| + if !m.Public || m.Schema != Schema || m.Title != "CARLOS" || !slices.Equal(m.Ledgers, []string{"carbon"}) { |
| + t.Fatalf("first update = %+v, want the created manifest", m) |
| + } |
| + |
| + // A second update sees the first one's state. |
| + m, err = UpdateManifest(ctx, st, "bab", "platform", func(m *Manifest) { |
| + m.Ledgers = append(m.Ledgers, "prompts") |
| + }) |
| + if err != nil { |
| + t.Fatalf("second update: %v", err) |
| + } |
| + if !slices.Equal(m.Ledgers, []string{"carbon", "prompts"}) || m.Title != "CARLOS" { |
| + t.Fatalf("second update = %+v, want carbon preserved beside prompts", m) |
| + } |
| + stored, err := ReadManifest(ctx, st, "bab", "platform") |
| + if err != nil || !slices.Equal(stored.Ledgers, []string{"carbon", "prompts"}) { |
| + t.Fatalf("stored = %+v %v, want what the update returned", stored, err) |
| + } |
| +} |
| + |
| +// conflictOnce loses the first ledgers.json CAS race to a writer that landed |
| +// a DIFFERENT manifest — the shape that tells a retry apart from a re-apply. |
| +// A loop that retried with its own stale copy would erase "iron". |
| +type conflictOnce struct { |
| + release.Store |
| + lost bool |
| +} |
| + |
| +func (c *conflictOnce) Put(ctx context.Context, key string, body []byte, ifMatch string) (string, error) { |
| + if strings.HasSuffix(key, "/ledgers.json") && !c.lost { |
| + c.lost = true |
| + if _, err := c.Store.Put(ctx, key, []byte(`{"schema":1,"public":true,"ledgers":["iron"],"title":"Other"}`), ifMatch); err != nil { |
| + return "", err |
| + } |
| + return "", release.ErrCASConflict |
| + } |
| + return c.Store.Put(ctx, key, body, ifMatch) |
| +} |
| + |
| +// TestUpdateManifestCASReapplies: on a lost race the update must re-READ and |
| +// re-APPLY, not re-Put its stale copy — otherwise two contributors publishing |
| +// 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.Public = true |
| + if !slices.Contains(m.Ledgers, "carbon") { |
| + m.Ledgers = append(m.Ledgers, "carbon") |
| + } |
| + }) |
| + if err != nil { |
| + t.Fatalf("update: %v", err) |
| + } |
| + if !st.lost { |
| + t.Fatal("the fake never forced a CAS conflict") |
| + } |
| + if !slices.Equal(m.Ledgers, []string{"iron", "carbon"}) { |
| + t.Fatalf("after a lost race = %+v, want the winner's iron preserved beside carbon", m) |
| + } |
| + if m.Title != "Other" { |
| + t.Fatalf("title = %q, want the winner's title preserved", m.Title) |
| + } |
| +} |
| + |
| +// TestUpdateManifestSurfacesLostRaces: contention that outlasts the retries |
| +// 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) { |
| + m.Public = true |
| + }); !errors.Is(err, release.ErrCASConflict) { |
| + t.Fatalf("update under total contention = %v, want a CAS conflict", err) |
| + } |
| +} |
| + |
| +type alwaysConflict struct{ release.Store } |
| + |
| +func (a alwaysConflict) Put(context.Context, string, []byte, string) (string, error) { |
| + return "", release.ErrCASConflict |
| +} |
diff --git a/internal/ledger/verify.go b/internal/ledger/verify.go| index 5547ce44c..97cd9debb 100644 |
| --- a/internal/ledger/verify.go |
| +++ b/internal/ledger/verify.go |
| @@ -6,6 +6,7 @@ import ( |
| "fmt" |
| "io" |
| "net/http" |
| + "strings" |
| "github.com/carlosframework/platform/internal/release" |
| ) |
| @@ -56,12 +57,24 @@ func Verify(ctx context.Context, get Getter, sqid, app, name string) (VerifyResu |
| return VerifyResult{}, fmt.Errorf("ledger: HEAD corrupt: %w", err) |
| } |
| + // The walk follows keys the CHAIN supplies (HEAD.json's head_key, then |
| + // each entry's prev_key), so those keys are exactly as trustworthy as the |
| + // bytes under verification. Pinning them to this ledger's own entries/ |
| + // prefix is what stops a chain from grafting another ledger's history — |
| + // or any other object in the bucket — onto its own and still verifying |
| + // clean: every hash would agree, because the hashes are only ever |
| + // checked against what the chain itself claims. |
| + want := Prefix(sqid, app, name) + "entries/" |
| + |
| key, wantSHA := head.HeadKey, head.HeadSHA256 |
| walked := 0 |
| for key != "" { |
| if walked >= head.Count { |
| return VerifyResult{}, fmt.Errorf("ledger: chain longer than HEAD count %d", head.Count) |
| } |
| + if !strings.HasPrefix(key, want) { |
| + return VerifyResult{}, fmt.Errorf("ledger: %s: chain leaves ledger %q — key is outside %s", key, name, want) |
| + } |
| eb, err := get(ctx, key) |
| if err != nil { |
| return VerifyResult{}, fmt.Errorf("ledger: %s: %w", key, err) |
| @@ -73,6 +86,9 @@ func Verify(ctx context.Context, get Getter, sqid, app, name string) (VerifyResu |
| if err := json.Unmarshal(eb, &e); err != nil { |
| return VerifyResult{}, fmt.Errorf("ledger: %s corrupt: %w", key, err) |
| } |
| + if e.Ledger != name { |
| + return VerifyResult{}, fmt.Errorf("ledger: %s: entry names ledger %q, walking %q", key, e.Ledger, name) |
| + } |
| key, wantSHA = e.PrevKey, e.PrevSHA256 |
| walked++ |
| } |
diff --git a/internal/ledger/verify_test.go b/internal/ledger/verify_test.go| index c3916aa92..4139c2fab 100644 |
| --- a/internal/ledger/verify_test.go |
| +++ b/internal/ledger/verify_test.go |
| @@ -61,6 +61,61 @@ func TestVerifyDetectsTamper(t *testing.T) { |
| } |
| } |
| +// TestVerifyPinsTheWalkToItsOwnLedger: a verifier follows prev_key, and |
| +// prev_key is bytes the chain itself supplies — so a chain that points its |
| +// walk at ANOTHER ledger's entries (or at an object outside the ledger |
| +// prefix entirely) must fail, not quietly borrow that object's history and |
| +// report a clean verification. The count and the hashes can all be made to |
| +// agree; what cannot be forged is the entry's own ledger name and the prefix |
| +// its keys live under. |
| +func TestVerifyPinsTheWalkToItsOwnLedger(t *testing.T) { |
| + ctx := context.Background() |
| + st := release.NewDirStore(t.TempDir()) |
| + // A real two-entry chain in "iron", whose genesis we will graft onto a |
| + // hand-built "carbon" chain. |
| + if _, err := Append(ctx, st, "bab", "platform", "iron", "w", |
| + json.RawMessage(`{"i":0}`), fixedNow); err != nil { |
| + t.Fatal(err) |
| + } |
| + ib, _, err := st.Get(ctx, HeadKey("bab", "platform", "iron")) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + var ih Head |
| + if err := json.Unmarshal(ib, &ih); err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| + // carbon's single entry chains to iron's genesis: every hash checks out, |
| + // the walk terminates, and the count matches. |
| + e := Entry{Schema: Schema, Ledger: "carbon", Writer: "w", TS: "2026-08-03T21:00:00Z", |
| + Payload: json.RawMessage(`{"i":1}`), PrevKey: ih.HeadKey, PrevSHA256: ih.HeadSHA256} |
| + eb, err := json.Marshal(e) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + sha := SHA256Hex(eb) |
| + key := EntryKey("bab", "platform", "carbon", e.TS, sha) |
| + if _, err := st.Put(ctx, key, eb, ""); err != nil { |
| + t.Fatal(err) |
| + } |
| + hb, err := json.Marshal(Head{Schema: Schema, UpdatedAt: e.TS, HeadKey: key, HeadSHA256: sha, Count: 2}) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, err := st.Put(ctx, HeadKey("bab", "platform", "carbon"), hb, ""); err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| + _, err = Verify(ctx, StoreGetter(st), "bab", "platform", "carbon") |
| + if err == nil { |
| + t.Fatal("a chain walking into another ledger's entries verified clean") |
| + } |
| + if !strings.Contains(err.Error(), "carbon") { |
| + t.Fatalf("error = %v, want it to name the ledger the walk left", err) |
| + } |
| +} |
| + |
| func TestVerifyOverHTTP(t *testing.T) { |
| dir := t.TempDir() |
| st := release.NewDirStore(dir) |