From 6b85b2bcc1f3773e2c1987c77daf6c8f7eee5560 Mon Sep 17 00:00:00 2001 From: Alex Godoroja <50743382+Alexgodoroja@users.noreply.github.com> Date: Tue, 7 Jul 2026 15:20:12 -0700 Subject: [PATCH 1/2] scaffold+broker: dedicated .balance method for managed budget apps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Managed (credit-metered) apps could meter a per-user $-budget and surface it on the X-Pilot-Credits-Remaining header, but a keyless adapter only returns the response body — so an agent could never actually read its balance, and there was no first-class way to check funds before a spend op. This wires a dedicated balance method, end to end: - Broker answers a canonical /_pilot/balance for any credit app from its own per-caller ledger — no upstream call, no master key, no debit, never a 402, scoped to the caller (the pooled account balance is never disclosed). Reuses the existing serveCreditBalance (which also backs the optional creditBalancePath partner-endpoint shadow); the canonical path is always available alongside it. - Scaffolder auto-injects `.balance` into every `auth: managed` app (no submission field), a GET to that path — flowing through registration, the manifest `exposes` list, and `.help` like any authored method. - scaffold.BalanceMetaPath == broker.pilotBalancePath keeps the two ends in sync. - Tests: broker (canonical path seeds, reflects spend, free, never forwards) and scaffold (injected for managed only, generated + compiles + in the manifest). - Docs: MANAGED-KEY.md and the publishing playbook document the pattern for all broker apps that meter a budget. --- docs/MANAGED-KEY.md | 36 ++++++ docs/PUBLISHING-PLAYBOOK.md | 11 ++ internal/broker/broker.go | 13 ++- internal/broker/zz_balance_meta_test.go | 76 +++++++++++++ internal/scaffold/config.go | 32 ++++++ internal/scaffold/zz_balance_method_test.go | 119 ++++++++++++++++++++ 6 files changed, 286 insertions(+), 1 deletion(-) create mode 100644 internal/broker/zz_balance_meta_test.go create mode 100644 internal/scaffold/zz_balance_method_test.go diff --git a/docs/MANAGED-KEY.md b/docs/MANAGED-KEY.md index 838e380..a987a7e 100644 --- a/docs/MANAGED-KEY.md +++ b/docs/MANAGED-KEY.md @@ -205,6 +205,42 @@ and, once spent, return **`402 Payment Required`**. Add a `credit` block: with per-user key minting) are **mutually exclusive**. Both need a durable store in prod (`BROKER_DB`) so balances survive a restart. +### Checking the balance: the `.balance` method + +The remaining budget rides on the `X-Pilot-Credits-Remaining` header of every +metered response — but a keyless adapter only surfaces the response **body**, so +that header is invisible to the agent. So every **managed** app also gets a +dedicated, free balance method, wired automatically — no submission field needed: + +- The scaffolder injects **`.balance`** into any app with `auth: managed` + (see `Config.Resolve`). It is a `GET` to the broker's canonical + **`/_pilot/balance`** route (`scaffold.BalanceMetaPath` == + `broker.pilotBalancePath`), and it shows up in the manifest `exposes` list and + in `.help` like any other method. +- The broker answers `/_pilot/balance` for any **credit**-metered app **before** + the allow-list, seeds a first-seen caller so they see their full budget, and + returns the ledger read **without forwarding upstream, touching the master key, + or debiting** — a pure read that can **never** `402`, scoped to THIS caller (the + shared account's pooled balance is never disclosed): + + ```json + { "balance": "$1.80", "credits_remaining": 1800000, "credits_seed": 5000000, + "unit": "micro_usd", "scope": "per-pilot-user" } + ``` + +- `provision` apps keep their own `/_balance` route instead. A managed app may + additionally set `credit.balance_path` to **shadow a partner's own + account-balance endpoint** (so calling it returns the per-user budget instead of + leaking the pooled account) — that is answered by the same handler, alongside the + canonical `/_pilot/balance`. + +**Playbook — any broker app that meters a budget:** rely on this rather than +hand-rolling a balance endpoint. Set the `credit` block, and the adapter exposes +`.balance` for free; document in the app's `app_description` that agents should +call `.balance` (or read `X-Pilot-Credits-Remaining`) to check funds before a +spend op, and that spend ops return `402` with `credits_remaining` / +`credits_required` when the budget is exhausted. + ## Operating the broker ```bash diff --git a/docs/PUBLISHING-PLAYBOOK.md b/docs/PUBLISHING-PLAYBOOK.md index 6c86aae..da0e57a 100644 --- a/docs/PUBLISHING-PLAYBOOK.md +++ b/docs/PUBLISHING-PLAYBOOK.md @@ -54,6 +54,17 @@ Two orthogonal choices. Get these right first; everything else follows. > go-live: register the app + master key with the broker and SIGHUP it (Step 7.5). Until > that is done the managed app installs but every call 5xx's at the broker. +> **Budget + balance (managed apps).** To meter a **dollar budget per user** (not just a +> rate quota), add a `credit` block to the broker registration — each user is seeded a fixed +> amount and spend ops return `402` once it's gone; reads stay free (see +> [`MANAGED-KEY.md`](MANAGED-KEY.md#per-user-spending-budget-credit--402)). You get balance +> checking **for free**: every `managed` app auto-exposes a **`.balance`** method (a +> `GET` to the broker's canonical `/_pilot/balance`) that returns +> `{balance:"$X.XX", credits_remaining, credits_seed, unit:"micro_usd", scope:"per-pilot-user"}` +> without a partner call or a charge — no submission field to add. In the app's +> `app_description`, tell agents to call `.balance` (or read the +> `X-Pilot-Credits-Remaining` header) before a spend op. + ## Step 1 — Author `pilot.app.yaml` ```bash diff --git a/internal/broker/broker.go b/internal/broker/broker.go index 1c8925a..4523360 100644 --- a/internal/broker/broker.go +++ b/internal/broker/broker.go @@ -67,6 +67,13 @@ func microUSD(micros int) string { return fmt.Sprintf("$%d.%02d", micros/1_000_000, (micros%1_000_000)/10_000) } +// pilotBalancePath is the canonical per-user credit-balance route: every credit +// app answers it from the broker's own ledger (never forwarded), and the +// generated `.balance` adapter method dials it. MUST match +// scaffold.BalanceMetaPath. Always available on a credit app, in addition to any +// creditBalancePath that shadows a partner's own account-balance endpoint. +const pilotBalancePath = "/_pilot/balance" + // serveCreditBalance answers a credit app's balance path from the per-caller // ledger. It NEVER contacts the partner, so the shared master account's pooled // balance is not exposed — only this caller's own remaining budget. The caller is @@ -148,7 +155,11 @@ func (b *Broker) ServeHTTP(w http.ResponseWriter, r *http.Request) { // OWN ledger — never forwarding to the partner — so the shared account's // pooled balance is never disclosed. Returns only THIS caller's remaining // micro-$ budget (seeding on first sight, so the per-IP cap applies here too). - if app.creditEnabled() && app.creditBalancePath != "" && mpath == app.creditBalancePath { + // Two paths reach it: the canonical /_pilot/balance (what the generated + // .balance method dials — always available on a credit app) and an + // optional creditBalancePath that SHADOWS a partner's account-balance + // endpoint so it can't leak the pooled account. + if app.creditEnabled() && (mpath == pilotBalancePath || (app.creditBalancePath != "" && mpath == app.creditBalancePath)) { b.serveCreditBalance(w, r, app, string(caller)) return } diff --git a/internal/broker/zz_balance_meta_test.go b/internal/broker/zz_balance_meta_test.go new file mode 100644 index 0000000..559c3a3 --- /dev/null +++ b/internal/broker/zz_balance_meta_test.go @@ -0,0 +1,76 @@ +package broker + +import ( + "crypto/ed25519" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + "time" +) + +// balanceOf issues a signed GET to the canonical credit-balance route and returns +// the decoded credits_remaining plus the raw recorder. +func balanceOf(t *testing.T, b *Broker, priv ed25519.PrivateKey, now time.Time) (int, *httptest.ResponseRecorder) { + t.Helper() + rec := httptest.NewRecorder() + b.ServeHTTP(rec, signedReq(t, priv, "GET", "/io.pilot.test/_pilot/balance", nil, now)) + var out struct { + Balance string `json:"balance"` + CreditsRemaining int `json:"credits_remaining"` + CreditsSeed int `json:"credits_seed"` + Unit string `json:"unit"` + Scope string `json:"scope"` + } + _ = json.Unmarshal(rec.Body.Bytes(), &out) + if rec.Code == http.StatusOK { + if out.Unit != "micro_usd" || out.Scope != "per-pilot-user" { + t.Fatalf("balance unit/scope = %q/%q, want micro_usd/per-pilot-user (body %s)", out.Unit, out.Scope, rec.Body.String()) + } + } + return out.CreditsRemaining, rec +} + +// TestBalanceMeta_CanonicalFreeReadReflectsSpend verifies the canonical +// /_pilot/balance route: it seeds a fresh caller, reflects debits, never touches +// the upstream, and never charges — all without any balance_path config. +func TestBalanceMeta_CanonicalFreeReadReflectsSpend(t *testing.T) { + now := time.Unix(1_800_000_000, 0) + b, hits := creditBroker(t, 200, now) // credit app, no balance_path set + _, priv := newKey(t) + + // 1. First-ever call is the balance check itself → caller seeded to 100, + // reported, upstream never hit, header set. + bal, rec := balanceOf(t, b, priv, now) + if rec.Code != http.StatusOK { + t.Fatalf("balance status %d, want 200 (body %s)", rec.Code, rec.Body.String()) + } + if bal != 100 { + t.Fatalf("seeded balance = %d, want 100", bal) + } + if rec.Header().Get("X-Pilot-Credits-Remaining") != "100" { + t.Fatalf("balance header = %q, want 100", rec.Header().Get("X-Pilot-Credits-Remaining")) + } + if *hits != 0 { + t.Fatalf("balance check hit the upstream %d times, want 0", *hits) + } + + // 2. Spend 40 on /echo → remaining 60. + spend := httptest.NewRecorder() + b.ServeHTTP(spend, signedReq(t, priv, "POST", "/io.pilot.test/echo", []byte(`{}`), now)) + if spend.Code != http.StatusOK { + t.Fatalf("/echo status %d, want 200", spend.Code) + } + + // 3. Balance reflects the spend, still free and still no upstream hit. + hitsAfterSpend := *hits + for i := 0; i < 3; i++ { + bal, rec = balanceOf(t, b, priv, now) + if rec.Code != http.StatusOK || bal != 60 { + t.Fatalf("post-spend balance = %d (status %d), want 60", bal, rec.Code) + } + } + if *hits != hitsAfterSpend { + t.Fatal("balance check debited/forwarded — it must be a pure read") + } +} diff --git a/internal/scaffold/config.go b/internal/scaffold/config.go index 067cf8e..8ec6e6b 100644 --- a/internal/scaffold/config.go +++ b/internal/scaffold/config.go @@ -353,6 +353,13 @@ func (c *Config) LocalStores() []LocalStoreGrant { // whose http.path equals it as the provisioning call (no request body forwarded). const DefaultProvisionPath = "/_provision" +// BalanceMetaPath is the broker's reserved credit-balance route for managed +// (credit-metered) apps. A GET to this path returns the caller's remaining +// per-user budget straight from the broker's credit ledger — no partner API +// call, no debit, never a 402. Managed apps get a `.balance` method wired to +// it automatically (see Resolve). MUST match the broker's pilotBalancePath. +const BalanceMetaPath = "/_pilot/balance" + // ProvisionPath is the reserved provision route the generated adapter recognizes. func (c *Config) ProvisionPath() string { return DefaultProvisionPath } @@ -659,6 +666,31 @@ func (c *Config) Resolve() { x.Asset = "USDC" } } + // Managed (credit-metered) apps get a dedicated, free balance method wired to + // the broker's reserved credit-ledger route. It makes "how much budget do I + // have left?" a first-class call — not just a header on other responses — and + // it never costs anything. Injected before the normalization loop below so it + // picks up the same Kind/Duration/Timeout defaults as an authored method, and + // flows through registration, the manifest `exposes` list, and .help. + if c.Managed() { + balName := c.Namespace + ".balance" + has := false + for i := range c.Methods { + if c.Methods[i].Name == balName { + has = true + break + } + } + if !has { + c.Methods = append(c.Methods, Method{ + Name: balName, + Summary: "Your remaining Pilot budget for this app, read free from the broker's per-user credit ledger — returns {\"balance\":\"$X.XX\",\"credits_remaining\":,\"credits_seed\":,\"unit\":\"micro_usd\",\"scope\":\"per-pilot-user\"}. This is YOUR budget, not the shared account's. No partner API call, no charge, and never a 402. The same figure also rides on the X-Pilot-Credits-Remaining header of every metered response; call this when you just want to check what's left before a spend.", + Kind: "meta", + Duration: "fast", + HTTP: &HTTPRoute{Verb: "GET", Path: BalanceMetaPath}, + }) + } + } for i := range c.Methods { m := &c.Methods[i] if m.Kind == "" { diff --git a/internal/scaffold/zz_balance_method_test.go b/internal/scaffold/zz_balance_method_test.go new file mode 100644 index 0000000..02f8501 --- /dev/null +++ b/internal/scaffold/zz_balance_method_test.go @@ -0,0 +1,119 @@ +package scaffold + +import ( + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +const managedBalanceSpec = ` +id: io.pilot.paidapi +app_version: 0.1.0 +description: "A managed, credit-metered API." +backend: + base_url: https://api.example.com + auth: managed +methods: + - name: paidapi.run + summary: "Do a paid thing." + http: { verb: POST, path: /v1/run } +` + +const byoBalanceSpec = ` +id: io.pilot.freeapi +app_version: 0.1.0 +description: "A bring-your-own-key API (no broker, no budget)." +backend: + base_url: https://api.example.com + auth: byo +methods: + - name: freeapi.run + summary: "Do a thing." + http: { verb: GET, path: /v1/run } +` + +// hasMethod reports whether the resolved config carries a method by name. +func hasMethod(c *Config, name string) *Method { + for i := range c.Methods { + if c.Methods[i].Name == name { + return &c.Methods[i] + } + } + return nil +} + +// TestBalanceMethod_InjectedForManagedOnly pins the auto-injected balance method: +// managed (broker + budget) apps get a free `.balance` wired to the reserved +// credit route; byo apps (no broker) don't. +func TestBalanceMethod_InjectedForManagedOnly(t *testing.T) { + managed := parseSpec(t, managedBalanceSpec) + bal := hasMethod(managed, "paidapi.balance") + if bal == nil { + t.Fatal("managed app missing auto-injected paidapi.balance method") + } + if bal.HTTP == nil || bal.HTTP.Verb != "GET" || bal.HTTP.Path != BalanceMetaPath { + t.Fatalf("balance route = %+v, want GET %s", bal.HTTP, BalanceMetaPath) + } + if bal.Kind != "meta" { + t.Errorf("balance Kind = %q, want meta", bal.Kind) + } + + byo := parseSpec(t, byoBalanceSpec) + if hasMethod(byo, "freeapi.balance") != nil { + t.Error("byo app should NOT get a balance method (no broker/budget)") + } +} + +// TestBalanceMethod_Generated verifies the injected method reaches the generated +// adapter: registered against the reserved path, listed in the manifest exposes, +// discoverable in .help, and the project still compiles. +func TestBalanceMethod_Generated(t *testing.T) { + if testing.Short() { + t.Skip("skipping compile test in -short mode") + } + goBin, err := exec.LookPath("go") + if err != nil { + t.Skip("go toolchain not available") + } + cfg := parseSpec(t, managedBalanceSpec) + if errs := cfg.Validate(); len(errs) != 0 { + t.Fatalf("validate: %v", errs) + } + dir := t.TempDir() + if _, err := Generate(cfg, dir); err != nil { + t.Fatalf("generate: %v", err) + } + if sum, err := os.ReadFile(filepath.Join("..", "..", "go.sum")); err == nil { + _ = os.WriteFile(filepath.Join(dir, "go.sum"), sum, 0o644) + } + + main, err := os.ReadFile(filepath.Join(dir, "cmd", cfg.BinaryName, "main.go")) + if err != nil { + t.Fatalf("read main.go: %v", err) + } + for _, want := range []string{ + `d.Register("paidapi.balance"`, // registered + `pathTmpl: "` + BalanceMetaPath + `"`, // dialing the reserved route + } { + if !strings.Contains(string(main), want) { + t.Errorf("generated main.go missing: %s", want) + } + } + + mf, err := os.ReadFile(filepath.Join(dir, "manifest.json")) + if err != nil { + t.Fatalf("read manifest: %v", err) + } + if !strings.Contains(string(mf), `"paidapi.balance"`) { + t.Error("manifest exposes list missing paidapi.balance") + } + + cmd := exec.Command(goBin, "build", "./...") + cmd.Dir = dir + cmd.Env = append(os.Environ(), "GOFLAGS=-mod=mod") + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("generated managed project failed to compile: %v\n%s", err, out) + } +} From b51c40f38b39418c6e9ba8642585b8cb0fd1563a Mon Sep 17 00:00:00 2001 From: Teodor Calin Date: Fri, 24 Jul 2026 15:25:14 +0300 Subject: [PATCH 2/2] scaffold+broker: fix provisioned-app balance 403, cooldown-gated reads, path drift MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes from PR #73 review: 1. Scaffold's `if c.Managed()` injection guard fired for BOTH auth:managed and auth:provisioned (Managed() is true for both), but the broker only ever answers the canonical /_pilot/balance route for classic managed apps — provisioned apps route through serveProvisioned, which only recognizes ProvisionSpec.BalancePath (default /_balance) and 403s on anything else. Provisioned apps that don't hand-author their own `.balance` method (unlike io.pilot.smol) shipped one that always 403s. Gated the injection to `c.Managed() && !c.Provisioned()`. 2. serveCreditBalance seeded via seedCaller -> ProvisionStore.Provision on every hit, including repeats, and MemStore.Provision's mint_cooldown_ms check fires on every repeat touch — so a second `.balance` call inside the cooldown window 429'd with "re-provision cooldown" instead of returning the balance. Now an already-seeded caller is answered from ProvisionStore.Get (no cooldown check, no LastMint touch); only a genuine first-ever touch goes through the seed path, which can never itself hit the cooldown. Since the balance routes have no Store.Admit/Quota gate, added a light, generous per-(app,caller) token bucket (allowBalanceRead, burst 20 / refill 5/s) so the now-cooldown- exempt route isn't literally unbounded, applied to both serveCreditBalance and the provisioned serveBalance. 3. Extracted the duplicated "/_pilot/balance" literal (scaffold.BalanceMetaPath + broker.pilotBalancePath) into one shared constant, internal/pilotpath.Balance, so the two can no longer drift. 4. Added a cross-caller isolation test: two distinct signed identities against the same app each see only their own balance. Balance routes remain pure reads: no debit, no forward to upstream, no master-key path touched. Co-Authored-By: Claude Opus 4.8 --- internal/broker/broker.go | 114 ++++++++++-- internal/broker/provision.go | 11 +- internal/broker/zz_balance_fix_test.go | 189 ++++++++++++++++++++ internal/pilotpath/pilotpath.go | 21 +++ internal/scaffold/config.go | 32 +++- internal/scaffold/zz_balance_method_test.go | 38 ++++ 6 files changed, 382 insertions(+), 23 deletions(-) create mode 100644 internal/broker/zz_balance_fix_test.go create mode 100644 internal/pilotpath/pilotpath.go diff --git a/internal/broker/broker.go b/internal/broker/broker.go index 106b6bf..71451ed 100644 --- a/internal/broker/broker.go +++ b/internal/broker/broker.go @@ -13,8 +13,11 @@ import ( "net/http" "strconv" "strings" + "sync" "sync/atomic" "time" + + "github.com/pilot-protocol/app-template/internal/pilotpath" ) // Broker is the managed-key service: verify caller → trust/quota → inject the @@ -36,6 +39,12 @@ type Broker struct { AccessKeys *AccessKeys reg atomic.Pointer[Registry] // hot-swappable so the registry can reload without dropping traffic + + // balanceMu/balanceBuckets back a light per-(app,caller) token-bucket rate + // limit on the free balance-read routes (serveCreditBalance, serveBalance). + // See allowBalanceRead. + balanceMu sync.Mutex + balanceBuckets map[string]*balanceBucket } // New returns a Broker with sane defaults. @@ -46,8 +55,9 @@ func New(reg *Registry, store Store) *Broker { // The effective per-call deadline is the smaller of this and the app's // timeout_ms (set per app in the registry); this only stops it being the // surprise bottleneck. - Client: &http.Client{Timeout: 300 * time.Second}, - MaxBody: 8 << 20, + Client: &http.Client{Timeout: 300 * time.Second}, + MaxBody: 8 << 20, + balanceBuckets: map[string]*balanceBucket{}, } b.reg.Store(reg) return b @@ -76,31 +86,105 @@ func microUSD(micros int) string { // pilotBalancePath is the canonical per-user credit-balance route: every credit // app answers it from the broker's own ledger (never forwarded), and the -// generated `.balance` adapter method dials it. MUST match -// scaffold.BalanceMetaPath. Always available on a credit app, in addition to any -// creditBalancePath that shadows a partner's own account-balance endpoint. -const pilotBalancePath = "/_pilot/balance" +// generated `.balance` adapter method dials it. Sourced from +// pilotpath.Balance so this and scaffold.BalanceMetaPath can never drift apart. +// Always available on a credit app, in addition to any creditBalancePath that +// shadows a partner's own account-balance endpoint. +const pilotBalancePath = pilotpath.Balance + +// balanceBucket is a light per-(app,caller) token bucket. See allowBalanceRead. +type balanceBucket struct { + tokens float64 + last time.Time +} + +// balanceBurst/balanceRefillPerSec size the token bucket allowBalanceRead +// enforces on the free balance-read routes. They are deliberately generous — +// this is NOT the credit-mint cooldown (a read must never be gated by that, +// see allowBalanceRead's caller) and NOT the app's billable-call Quota (a free +// meta call must never spend it); it exists only so an intentionally +// un-quota'd, pre-Admit route is not literally unbounded. A caller can always +// burst a handful of legitimate "check my balance" calls back-to-back; only a +// tight hammering loop past the burst gets shed. +const ( + balanceBurst = 20 + balanceRefillPerSec = 5.0 +) + +// allowBalanceRead reports whether (app, caller) may have another balance read +// right now, consuming a token if so. Independent of ProvisionStore entirely — +// no cooldown, no Quota, no credit ledger touch — so it can never interact with +// either of those. +func (b *Broker) allowBalanceRead(app, caller string, now time.Time) bool { + k := app + "\x00" + caller + b.balanceMu.Lock() + defer b.balanceMu.Unlock() + if b.balanceBuckets == nil { + b.balanceBuckets = map[string]*balanceBucket{} + } + bk := b.balanceBuckets[k] + if bk == nil { + bk = &balanceBucket{tokens: balanceBurst, last: now} + b.balanceBuckets[k] = bk + } else if elapsed := now.Sub(bk.last).Seconds(); elapsed > 0 { + bk.tokens += elapsed * balanceRefillPerSec + if bk.tokens > balanceBurst { + bk.tokens = balanceBurst + } + bk.last = now + } + if bk.tokens < 1 { + return false + } + bk.tokens-- + return true +} // serveCreditBalance answers a credit app's balance path from the per-caller // ledger. It NEVER contacts the partner, so the shared master account's pooled -// balance is not exposed — only this caller's own remaining budget. The caller is -// seeded on first sight (so the per-IP cap applies), mirroring a first call. +// balance is not exposed — only this caller's own remaining budget. +// +// This is a READ, not a mint: unlike the metered-call path (which touches +// ProvisionStore.Provision on every call, deliberately, to keep the caller's +// last-mint timestamp fresh), a repeat balance check must never be rejected by +// the app's mint_cooldown_ms. So an ALREADY-seeded caller is answered straight +// from ProvisionStore.Get (no cooldown check, no LastMint touch, no re-seed); +// only a caller's genuine first-ever touch goes through the normal seed path +// (still enforcing the per-IP grant cap) — and a first touch can never itself +// hit the cooldown, since there is no prior mint to be too soon after. The +// route has no Store.Admit/Quota gate (a free meta call must never spend the +// app's billable-call quota), so allowBalanceRead applies a separate, light, +// generous rate limit instead. func (b *Broker) serveCreditBalance(w http.ResponseWriter, r *http.Request, app *AppEntry, caller string) { ps, ok := b.provStore() if !ok { writeJSON(w, http.StatusInternalServerError, map[string]string{"error": "store does not support credit metering"}) return } - ip := clientIP(r.Header.Get, r.RemoteAddr, b.IPTrust) - rec, err := b.seedCaller(ps, app.ID, caller, ip, app) + now := b.now() + if !b.allowBalanceRead(app.ID, caller, now) { + writeJSON(w, http.StatusTooManyRequests, map[string]string{"error": "balance: rate limited — retry shortly"}) + return + } + rec, exists, err := ps.Get(app.ID, caller) if err != nil { - if err == ErrCooldown { - writeJSON(w, http.StatusTooManyRequests, map[string]string{"error": "re-provision cooldown — retry shortly"}) - } else { - b.internalError(w, http.StatusInternalServerError, app.ID, "balance", err) - } + b.internalError(w, http.StatusInternalServerError, app.ID, "balance", err) return } + if !exists { + ip := clientIP(r.Header.Get, r.RemoteAddr, b.IPTrust) + rec, err = b.seedCaller(ps, app.ID, caller, ip, app) + if err != nil { + if err == ErrCooldown { + // Unreachable in practice: nothing to cool down from on a + // caller's first-ever touch. Handled defensively only. + writeJSON(w, http.StatusTooManyRequests, map[string]string{"error": "re-provision cooldown — retry shortly"}) + } else { + b.internalError(w, http.StatusInternalServerError, app.ID, "balance", err) + } + return + } + } w.Header().Set("X-Pilot-Credits-Remaining", strconv.Itoa(rec.Credits)) writeJSON(w, http.StatusOK, map[string]any{ "balance": microUSD(rec.Credits), diff --git a/internal/broker/provision.go b/internal/broker/provision.go index 198ca3d..eaf8de1 100644 --- a/internal/broker/provision.go +++ b/internal/broker/provision.go @@ -672,13 +672,22 @@ func (b *Broker) callerFromKey(app *AppEntry, token string) (CallerID, bool) { return CallerID(callerStr), true } -// serveBalance handles GET //_balance → the caller's credit balance. +// serveBalance handles GET //_balance → the caller's credit balance. Like +// serveCreditBalance, it never touches ProvisionStore.Provision (no cooldown, +// no re-seed — a pure Credit read), and like serveCreditBalance it sits before +// any Store.Admit/Quota gate, so it shares the same light, generous +// allowBalanceRead rate limit rather than the mint cooldown or the app's +// billable-call quota. func (b *Broker) serveBalance(w http.ResponseWriter, app *AppEntry, caller CallerID) { ps, ok := b.provStore() if !ok { writeJSON(w, http.StatusInternalServerError, map[string]string{"error": "store does not support provisioning"}) return } + if !b.allowBalanceRead(app.ID, string(caller), b.now()) { + writeJSON(w, http.StatusTooManyRequests, map[string]string{"error": "balance: rate limited — retry shortly"}) + return + } c, err := ps.Credit(app.ID, string(caller)) if err != nil { writeJSON(w, http.StatusInternalServerError, map[string]string{"error": "balance: " + err.Error()}) diff --git a/internal/broker/zz_balance_fix_test.go b/internal/broker/zz_balance_fix_test.go new file mode 100644 index 0000000..e3c1efa --- /dev/null +++ b/internal/broker/zz_balance_fix_test.go @@ -0,0 +1,189 @@ +package broker + +import ( + "net/http" + "net/http/httptest" + "strconv" + "testing" + "time" + + "github.com/pilot-protocol/app-template/internal/pilotpath" +) + +// creditBrokerCooldown is creditBroker's twin, but with a non-zero +// mint_cooldown_ms — the exact config shape that used to make a repeat +// `.balance` read 429 with "re-provision cooldown" instead of returning the +// balance (finding #2). +func creditBrokerCooldown(t *testing.T, upStatus int, cooldownMs int, now time.Time) (*Broker, *int) { + t.Helper() + var hits int + up := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + hits++ + w.WriteHeader(upStatus) + _, _ = w.Write([]byte(`{"ok":true}`)) + })) + t.Cleanup(up.Close) + reg, err := ParseRegistry([]byte(`[{ + "id":"io.pilot.test","upstream":"`+up.URL+`","key_env":"TEST_KEY", + "auth_header":"Authorization","auth_scheme":"Bearer", + "allow":["/echo"], + "credit":{"seed_credits":100,"default_cost":10,"mint_cooldown_ms":`+strconv.Itoa(cooldownMs)+`} + }]`), func(string) string { return "MASTERKEY" }) + if err != nil { + t.Fatalf("ParseRegistry: %v", err) + } + b := New(reg, NewMemStore()) + b.Verify = VerifyConfig{Now: fixedClock(now)} + return b, &hits +} + +// TestBalanceMeta_ReadIgnoresMintCooldown pins the fix for finding #2: a credit +// app configured with mint_cooldown_ms > 0 (to slow rapid re-provision churn on +// real mints) must NOT have that cooldown leak onto the free `.balance` read. +// Before the fix, serveCreditBalance called seedCaller -> ProvisionStore.Provision +// on every hit — including repeats — and MemStore.Provision's cooldown check +// fires on every repeat touch, so a second balance call inside the cooldown +// window 429'd with "re-provision cooldown" instead of returning the balance. +func TestBalanceMeta_ReadIgnoresMintCooldown(t *testing.T) { + now := time.Unix(1_800_000_000, 0) + // A full minute of cooldown — comfortably longer than the fixed test clock + // ever advances, so ANY repeat call within it would have 429'd pre-fix. + b, _ := creditBrokerCooldown(t, 200, 60_000, now) + _, priv := newKey(t) + + // First-ever call seeds the caller; this is the caller's genuine first + // touch, so it can never itself hit the cooldown (nothing to be too soon + // after). + bal, rec := balanceOf(t, b, priv, now) + if rec.Code != http.StatusOK { + t.Fatalf("first balance call: status %d, want 200 (body %s)", rec.Code, rec.Body.String()) + } + if bal != 100 { + t.Fatalf("seeded balance = %d, want 100", bal) + } + + // Repeat calls, still well inside the 60s cooldown window (fixed clock: + // zero time has elapsed at all) — every one of these must return the + // balance, never the mint-cooldown 429. + for i := 0; i < 5; i++ { + bal, rec = balanceOf(t, b, priv, now) + if rec.Code == http.StatusTooManyRequests { + t.Fatalf("repeat balance call %d: got 429 %s (mint cooldown leaked onto a read)", i, rec.Body.String()) + } + if rec.Code != http.StatusOK { + t.Fatalf("repeat balance call %d: status %d, want 200 (body %s)", i, rec.Code, rec.Body.String()) + } + if bal != 100 { + t.Fatalf("repeat balance call %d = %d, want 100 (unspent)", i, bal) + } + } + + // A real metered call is a mint touch (seedCaller -> ProvisionStore.Provision) + // and so is legitimately still subject to the cooldown by elapsed wall time — + // this test only asserts the READ path is exempt, not that cooldown + // enforcement disappeared everywhere. Advance past the cooldown window so the + // metered call itself isn't the thing under test here. + later := now.Add(61 * time.Second) + b.Verify = VerifyConfig{Now: fixedClock(later)} + spend := httptest.NewRecorder() + b.ServeHTTP(spend, signedReq(t, priv, "POST", "/io.pilot.test/echo", []byte(`{}`), later)) + if spend.Code != http.StatusOK { + t.Fatalf("/echo: status %d, want 200 (body %s)", spend.Code, spend.Body.String()) + } + bal, rec = balanceOf(t, b, priv, later) + if rec.Code != http.StatusOK || bal != 90 { + t.Fatalf("post-spend balance = %d (status %d), want 90", bal, rec.Code) + } +} + +// TestBalanceRateLimit_BurstThenSheds pins allowBalanceRead's light, generous +// token bucket: legitimate back-to-back balance checks (well within +// balanceBurst) always succeed, but a tight loop well past the burst +// eventually gets a 429 distinct from the mint-cooldown error, and recovers +// once the fixed clock is advanced past a refill interval. +func TestBalanceRateLimit_BurstThenSheds(t *testing.T) { + now := time.Unix(1_800_000_000, 0) + b, _ := creditBroker(t, 200, now) // no mint_cooldown_ms at all + _, priv := newKey(t) + + shed := false + var lastCode int + for i := 0; i < balanceBurst+10; i++ { + _, rec := balanceOf(t, b, priv, now) + lastCode = rec.Code + if rec.Code == http.StatusTooManyRequests { + shed = true + break + } + if rec.Code != http.StatusOK { + t.Fatalf("call %d: unexpected status %d (body %s)", i, rec.Code, rec.Body.String()) + } + } + if !shed { + t.Fatalf("expected the burst limiter to eventually 429 within %d calls, last status %d", balanceBurst+10, lastCode) + } + + // Advance the clock well past a full refill and confirm it recovers. + later := now.Add(10 * time.Second) + b.Verify = VerifyConfig{Now: fixedClock(later)} + _, rec := balanceOf(t, b, priv, later) + if rec.Code != http.StatusOK { + t.Fatalf("after refill: status %d, want 200 (body %s)", rec.Code, rec.Body.String()) + } +} + +// TestBalancePath_SharedConstant_NoDrift pins finding #3: scaffold's +// BalanceMetaPath and the broker's pilotBalancePath must be the exact same +// string, sourced from one place (pilotpath.Balance) rather than two +// independently-typed literals that could silently diverge. +func TestBalancePath_SharedConstant_NoDrift(t *testing.T) { + if pilotBalancePath != pilotpath.Balance { + t.Fatalf("broker.pilotBalancePath = %q, want pilotpath.Balance %q — the broker's route and the shared constant have drifted apart", pilotBalancePath, pilotpath.Balance) + } +} + +// TestBalanceMeta_CrossCallerIsolation pins finding #4: two distinct signed +// identities calling the SAME app's `.balance` route each see only their OWN +// remaining budget, never the other's (or the shared master account's pooled +// balance). +func TestBalanceMeta_CrossCallerIsolation(t *testing.T) { + now := time.Unix(1_800_000_000, 0) + b, _ := creditBroker(t, 200, now) // seed_credits 100, /echo costs 40 + _, alicePriv := newKey(t) + _, bobPriv := newKey(t) + + // Alice seeds to 100, then spends 40 on /echo -> 60 remaining. + aliceBal, rec := balanceOf(t, b, alicePriv, now) + if rec.Code != http.StatusOK || aliceBal != 100 { + t.Fatalf("alice seed balance = %d (status %d), want 100", aliceBal, rec.Code) + } + spend := httptest.NewRecorder() + b.ServeHTTP(spend, signedReq(t, alicePriv, "POST", "/io.pilot.test/echo", []byte(`{}`), now)) + if spend.Code != http.StatusOK { + t.Fatalf("alice /echo: status %d, want 200", spend.Code) + } + aliceBal, rec = balanceOf(t, b, alicePriv, now) + if rec.Code != http.StatusOK || aliceBal != 60 { + t.Fatalf("alice post-spend balance = %d (status %d), want 60", aliceBal, rec.Code) + } + + // Bob, a completely distinct identity on the same app, is seeded fresh to + // 100 — Alice's spend must not be visible to him, and his own reads must + // never move Alice's balance. + bobBal, rec := balanceOf(t, b, bobPriv, now) + if rec.Code != http.StatusOK || bobBal != 100 { + t.Fatalf("bob balance = %d (status %d), want 100 (own fresh seed, unaffected by alice's spend)", bobBal, rec.Code) + } + for i := 0; i < 2; i++ { + bobBal, rec = balanceOf(t, b, bobPriv, now) + if rec.Code != http.StatusOK || bobBal != 100 { + t.Fatalf("bob repeat balance = %d (status %d), want 100", bobBal, rec.Code) + } + } + + // Alice's balance is unchanged by any of Bob's calls. + aliceBal, rec = balanceOf(t, b, alicePriv, now) + if rec.Code != http.StatusOK || aliceBal != 60 { + t.Fatalf("alice balance after bob's activity = %d (status %d), want 60 (unaffected)", aliceBal, rec.Code) + } +} diff --git a/internal/pilotpath/pilotpath.go b/internal/pilotpath/pilotpath.go new file mode 100644 index 0000000..b66305c --- /dev/null +++ b/internal/pilotpath/pilotpath.go @@ -0,0 +1,21 @@ +// Package pilotpath holds route literals shared between the scaffold generator +// (internal/scaffold) and the broker server (internal/broker) — two +// independently-compiled packages that must agree on the same wire path +// without importing one another. Defining the literal once here, and having +// both sides reference it, means the two call sites cannot drift apart the +// way two independently-maintained "/_pilot/balance" string literals could. +package pilotpath + +// Balance is the broker's reserved, per-user credit-balance route for managed +// (credit-metered, non-provisioned) apps. A GET to this path returns the +// caller's remaining budget straight from the broker's own credit ledger — no +// partner API call, no debit, never a 402. +// +// - scaffold.BalanceMetaPath is the path scaffold wires the generated +// `.balance` method to (internal/scaffold/config.go). +// - broker's pilotBalancePath is the path the broker answers from its ledger +// (internal/broker/broker.go). +// +// Both are defined as this constant, not a re-typed literal, so they can never +// silently diverge. +const Balance = "/_pilot/balance" diff --git a/internal/scaffold/config.go b/internal/scaffold/config.go index b293416..624797f 100644 --- a/internal/scaffold/config.go +++ b/internal/scaffold/config.go @@ -19,6 +19,7 @@ import ( "github.com/pilot-protocol/app-template/internal/demo" "github.com/pilot-protocol/app-template/internal/nextsteps" + "github.com/pilot-protocol/app-template/internal/pilotpath" "gopkg.in/yaml.v3" ) @@ -464,12 +465,19 @@ func (c *Config) LocalStores() []LocalStoreGrant { // whose http.path equals it as the provisioning call (no request body forwarded). const DefaultProvisionPath = "/_provision" -// BalanceMetaPath is the broker's reserved credit-balance route for managed -// (credit-metered) apps. A GET to this path returns the caller's remaining -// per-user budget straight from the broker's credit ledger — no partner API -// call, no debit, never a 402. Managed apps get a `.balance` method wired to -// it automatically (see Resolve). MUST match the broker's pilotBalancePath. -const BalanceMetaPath = "/_pilot/balance" +// BalanceMetaPath is the broker's reserved credit-balance route for classic +// managed (credit-metered, non-provisioned) apps. A GET to this path returns +// the caller's remaining per-user budget straight from the broker's credit +// ledger — no partner API call, no debit, never a 402. Classic managed apps +// get a `.balance` method wired to it automatically (see Resolve). +// Provisioned apps do NOT: the broker only ever answers this canonical path +// for classic managed apps (internal/broker/broker.go); a provisioned app's +// balance lives at its own ProvisionSpec.BalancePath (broker-registry-only +// config scaffold cannot see), so a provisioned app must author its own +// `.balance` method pointed at that path (see io.pilot.smol) rather than +// have one auto-injected here. Sourced from pilotpath.Balance so this and the +// broker's pilotBalancePath can never drift apart. +const BalanceMetaPath = pilotpath.Balance // ProvisionPath is the reserved provision route the generated adapter recognizes. func (c *Config) ProvisionPath() string { return DefaultProvisionPath } @@ -856,7 +864,17 @@ func (c *Config) Resolve() { // it never costs anything. Injected before the normalization loop below so it // picks up the same Kind/Duration/Timeout defaults as an authored method, and // flows through registration, the manifest `exposes` list, and .help. - if c.Managed() { + // + // EXCLUDES provisioned apps: the broker only ever answers the canonical + // BalanceMetaPath ("/_pilot/balance") for classic managed apps — a + // provisioned app routes every call through serveProvisioned, which + // recognizes only its own ProvisionSpec.BalancePath (default "/_balance", + // broker-registry-only config scaffold has no visibility into). Injecting + // the canonical path for a provisioned app would ship a `.balance` + // method that always 403s. A provisioned app that wants a balance method + // must author one itself, pointed at its actual balance path (see + // io.pilot.smol's hand-authored `smol.balance` -> "/_balance"). + if c.Managed() && !c.Provisioned() { balName := c.Namespace + ".balance" has := false for i := range c.Methods { diff --git a/internal/scaffold/zz_balance_method_test.go b/internal/scaffold/zz_balance_method_test.go index 02f8501..40ece5f 100644 --- a/internal/scaffold/zz_balance_method_test.go +++ b/internal/scaffold/zz_balance_method_test.go @@ -34,6 +34,19 @@ methods: http: { verb: GET, path: /v1/run } ` +const provisionedBalanceSpec = ` +id: io.pilot.provapi +app_version: 0.1.0 +description: "A provisioned (per-user broker-minted key) API." +backend: + base_url: https://api.example.com + auth: provisioned +methods: + - name: provapi.run + summary: "Do a thing." + http: { verb: POST, path: /v1/run } +` + // hasMethod reports whether the resolved config carries a method by name. func hasMethod(c *Config, name string) *Method { for i := range c.Methods { @@ -66,6 +79,31 @@ func TestBalanceMethod_InjectedForManagedOnly(t *testing.T) { } } +// TestBalanceMethod_NotInjectedForProvisioned pins the fix for the provisioned +// scaffold-injection bug: a provisioned app (auth: provisioned) routes every +// call through the broker's serveProvisioned, which only ever recognizes its +// OWN ProvisionSpec.BalancePath (default "/_balance", registry-only config +// scaffold cannot see) — never the canonical BalanceMetaPath +// ("/_pilot/balance"). Before the fix, Provisioned() apps satisfied +// Managed() (true for both "managed" and "provisioned") and so got the +// canonical route auto-wired anyway, shipping a `.balance` method that +// would 403 on every single call. A provisioned app must instead author its +// own balance method pointed at its real path (see io.pilot.smol's +// hand-authored `smol.balance` -> "/_balance") — so scaffold must NOT +// auto-inject one it cannot correctly target. +func TestBalanceMethod_NotInjectedForProvisioned(t *testing.T) { + prov := parseSpec(t, provisionedBalanceSpec) + if !prov.Managed() { + t.Fatal("test spec sanity: provisioned app must report Managed() == true") + } + if !prov.Provisioned() { + t.Fatal("test spec sanity: provisioned app must report Provisioned() == true") + } + if bal := hasMethod(prov, "provapi.balance"); bal != nil { + t.Fatalf("provisioned app should NOT get an auto-injected balance method (it would always 403 — the broker only answers the canonical route for classic managed apps); got %+v", bal) + } +} + // TestBalanceMethod_Generated verifies the injected method reaches the generated // adapter: registered against the reserved path, listed in the manifest exposes, // discoverable in .help, and the project still compiles.