Skip to content

feat(lore-0105): require an API key on pubnet's /verify and /settle - #185

Open
adamkoot wants to merge 7 commits into
developfrom
lore-0105-api-keys-on-pubnet
Open

adamkoot wants to merge 7 commits into
developfrom
lore-0105-api-keys-on-pubnet

Conversation

@adamkoot

Copy link
Copy Markdown
Collaborator

Summary

Task 0105 (milestone 2, ADR 0007's amendment of 2026-11-03): a /verify or /settle request for stellar:pubnet now needs an API key, and testnet answers as before. Pubnet still refuses to start; this removes 0105 from what it waits for.

  • Key registry (ports/api-key-store.ts, adapters/postgres.ts, migration 0013_api_keys): a key's SHA-256, its owner and whether it is active or revoked. The key itself is stored nowhere. An owner has one active key, held by a partial unique index. A new table, so the release before this one keeps working.
  • The way in (http/admit.ts, http/api-key.ts): the routes read the body, take the network it names, and then either check the key and count the request under the key's owner (pubnet), or count it under the IP without reading Authorization (every other network). No key is 401 missing_api_key; anything but an active key is 401 invalid_api_key. Neither counts in any budget.
  • Rate limiter (http/rate-limit.ts): split into rateLimiter (byIp, byOwner) and the rateLimit middleware /discovery still uses. Its own commit, no behaviour change.
  • Owner to the spend policy (settlement/hooks.ts, settlement/settle.ts): Settle takes a caller, and BeforeSubmitContext.caller.owner carries it. The policy budgets as before; 0107 reads it.
  • Operator's script (src/keys.ts, keysMain in main.ts): pnpm keys issue <name> | revoke <name | owner> | list. issue prints the key once and stores its hash.
  • Docs: the facilitator README ("API keys on pubnet"), AD-11, the threat model's new "API keys" section, ADR 0007's amendment of 2026-11-13, ADR 0012.

The commits go from the registry to the refactor to the behaviour change, so they can be read one at a time.

Reviewers, please confirm:

  • @stkrolikiewicz (0126): /verify and /settle now read the body before they count a request, and require the key by paymentRequirements.network, so the rule already holds in a process that serves both networks. 0126 has to make stores.apiKeys and those counters the request's network's, and add a compose service for node dist/keys.js. The note is in the task.
  • @karolko9 (0090): the pubnet refusal's message no longer names 0105; it still names 0090, 0107, 0027 and 0048. The task's text follows.

Please confirm

  1. The body is read before the request is counted (AD-11, every /verify and /settle, testnet too). The network is in the body, so three things can't all hold in a process that serves both networks: testnet counted before the body is read, a pubnet 401 that makes no counter, and pubnet counted by key. A client over its budget now costs a parse of at most BODY_LIMIT_BYTES (64 KiB) where it cost nothing. A body that names no network (malformed, too large, not JSON) still counts under the IP. The new order runs on today's testnet process, so it carries real traffic before pubnet does.
  2. A request that names stellar:pubnet needs a key on today's testnet process too. It got 200 invalid_network, counted by IP; it now gets a 401. No testnet request changes.
  3. A revocation holds from the next request, with no cache. The registry is read on every pubnet request, one primary-key lookup on the rate limiter's pool.
  4. The limiter counts by the key's owner, not the key (settle:key:operator:acme, …:key:discord:<id>). A replaced key continues the count, and the spend policy gets the same identity. The task's second criterion said "counts under the key" and is reworded.
  5. A request refused for its key is unthrottled here. Counted by IP, one caller without a key could use up the budget of every seller behind its address (0060). Each such request costs a body parse and, for a well-formed key, one registry read; the edge is where a flood is stopped. Threat model, "Requests without a valid key, in bulk".
  6. The caller is not stored with the settlement. It reaches beforeSubmit and the verified and settled log lines only. onFinal gets the record, and a reservation keeps its scopes, so 0107 can commit or release without it.
  7. The script issues only operator: owners and has no rotate. Replacing a key is revoke, then issue. The production compose file gets no service for it: there is no pubnet database until 0126.

Testing

  • pnpm format:check && pnpm lint && pnpm typecheck && pnpm test && pnpm build && pnpm board on @x402 2.31.0, after the rebase onto feat(lore-0145): raise the @x402 packages to 2.31.0 #184 → pass. pnpm test: 20 of 20 tasks, the facilitator 1120 tests (3 skipped), 108 of them new. Locally the format and lint steps skip two git-ignored directories of mine (.vibe-wise/, packages/sdk/rust/target/).
  • Testnet, at 1e6507ab: npx tsx test/live/api-key-smoke.ts in apps/facilitator → 11 of 11 checks (test/live/results/api-key-smoke-testnet.json). A facilitator built by main(), payments from upstream's client, a key issued and revoked by the script.
    • Three payments of 0.0001 XLM settled, each counted under the IP: with no header 099755d3…, with a made-up key d545e0a8…, with the registered key f5f895f7….
    • A request naming stellar:pubnet: 401 without a key and with a made-up one, counting nowhere; with the key it reached the real verifier (invalid_network, this process serves testnet) and counted under verify:key:operator:smoke-…; after the revocation, 401.
  • Not run: a pubnet settlement, which needs a process that serves pubnet (0126), and deploy/scripts/check-production-postgres.sh, which needs Docker; nothing under deploy/ changes.
  • One timing test, settle.test.ts "keeps a repeat waiting past the settle timeout…" (the block 0074 lists), failed once in a full run before the rebase and passed in every run since.

A table of its own, api_keys: a key's SHA-256, its owner and whether
it is active or revoked. The key itself is stored nowhere, and the
table's CHECK refuses anything but a hash in that column.

An owner has one active key, held by a partial unique index, so the
operator's script and the portal (0106) can't disagree. ownerOf reads
on the rate limiter's pool: it runs before a request is let in.

A new table, so the release before this one keeps working.
rateLimiter counts one request for a client and says whether it may
go on; rateLimit is that as a middleware, by IP, as before. No route
changes: the next commit counts a caller with an API key by its
owner, which needs the counting apart from the IP.
A request whose paymentRequirements.network is stellar:pubnet must
carry "Authorization: Bearer <key>". Without an active key it gets a
401 (missing_api_key, invalid_api_key) before any check, and counts
in no budget. With one, the limiter counts it under the key's owner
instead of its IP, and /settle hands the owner to beforeSubmit, for
the per-owner fee budgets of 0107.

The rule follows the network the request names, not NETWORK, so both
routes now read the body before they count a request. A request for
any other network is counted by IP as before, and its Authorization
header is never read. The registry is read on every pubnet request,
so a revocation holds from the next one.

The pubnet refusal at startup no longer names this task.
pnpm keys issue <name> | revoke <name | owner> | list, over the same
registry the portal will write (0106). issue prints the new key once,
to stdout only, and stores its SHA-256; an owner with an active key
is refused until it is revoked. It needs only DATABASE_URL.
The facilitator README (callers, the key, the script), AD-11, the
threat model's new "API keys" section, and ADR 0007's amendment with
the decisions of 2026-11-13: no cache on revocation, the count by
owner, the body read before the count. The task records them, and
0090, 0106, 0107 and 0126 say what this task leaves them.
A live script: a facilitator built by main(), payments from
upstream's client, a key issued and revoked by the operator's script.
Testnet settles with no key, a made-up one and a registered one, each
counted by IP; a request naming stellar:pubnet gets 401 without an
active key and counts under the owner with one.
From the review of #185. The limiter's comment said a client over its
budget takes none of the store's connections, but the key's owner is
known only from the registry, so that read comes before the count, for
an owner over its budget too. The comments, the README and the threat
model now say so, and that revoking the key doesn't spare the read.

Two notes for later tasks: 0126 reads the request's network once for
the key rule and the handler, and 0107 decides whose quota a reopened
settlement is.

@stkrolikiewicz stkrolikiewicz left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two comments from my side: an answer to "Please confirm" 1 and the 0126 note, and one on the accepted flood risk. Neither blocks the merge.

export function admit({ json, limiter, apiKeys }: Admission): RequestHandler {
return async (req, res, next) => {
// body-parser never answers: it calls back, with its error for a body it refuses.
const bodyError = await new Promise<unknown>((resolve) => json(req, res, resolve));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirming "Please confirm" 1 and the 0126 note: reading the body before counting is fine with me. It keeps the rule right in a process that serves both networks, and the cost on today's testnet, a parse of at most 64 KiB for a client already over its budget, is small.

One addition for 0126: besides making stores.apiKeys and the counters the request's network's, both networks' admission then shares the limiter's pool. The made-up-key flood (next comment) would then hold testnet's payments up too. Worth a line in 0126's task.

const authorization = req.get("authorization");
if (authorization === undefined) return refuse(res, "missing_api_key");
const key = bearerApiKey(authorization);
const owner = key === undefined ? undefined : await apiKeys.ownerOf(apiKeyHash(key));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The threat model accepts the flood of made-up keys ("Requests without a valid key, in bulk"), and that's fine while pubnet still refuses to start. Two things before it opens:

  • The impact is wider than "some may get a 500". Every /verify and /settle is admitted through the limiter's pool of 3 connections (hit, and ownerOf here). While the flood lasts, no payment gets in, not only the keyed ones.
  • The mitigation leans on Cloudflare's rate rules. The Free plan has one rule for the whole zone, and ADR 0011 leaves a paid plan to 0038.

A cheap guard would bound it without counting refusals in a shared IP's budget:

  • a short in-process negative cache of unknown hashes (a minute, bounded in size);
  • or a refusal budget of its own per IP, e.g. <route>:refused:ip:<address>.

Not blocking. A note in 0126 or 0038 is enough if you'd rather leave it for later.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants