oauth_payloads grows without bound — no sweep for expired rows #167

New issue

Open

opened 2026-09-16 08:46:16 +02:00 by lz · 0 comments

lz commented

2026-09-16 08:46:16 +02:00

Owner

Spun out of #151, flagged independently by two reviewers.

The gap

nexus/src/lib/server/oauth/adapter.ts filters expiry at read time (revive() returns undefined for a row past expires_at) and never deletes. Nothing sweeps the table. The only deletions are revokeByGrantId (reachable only from a logout or revocation flow, and config.ts leaves both features.revocation and features.introspection disabled) and oidc-provider's own Interaction.destroy() on a successful resume.

Why the existing precedent does not transfer

Read-time filtering is the established pattern here — api_tokens (AGENTS.md fact #24) and the quota store (fact #19) both do it, and #151 followed them deliberately. But those tables hold one row per durable entity: a token the operator deliberately minted, a quota window that is overwritten.

oauth_payloads gains a new row per grant, per authorization code, per access token, per refresh token, and per interaction ever issued. With rotateRefreshToken: true, a single long-lived MCP connection inserts a new row on every refresh and keeps the consumed predecessor. An interaction the operator abandons — closing the tab — leaks its row permanently, because destroy() only runs on the success path.

So it grows from ordinary protocol traffic with no operator action, and there is no visibility: no count surfaced anywhere, no log line, no periodic job.

Shape of a fix

src/server.ts already has the pattern for this, for the skills-installer staging sweep:

setInterval(() => { void sweep().catch((err) => logger.warn({ err }, '…')); }, INTERVAL).unref();

A sweep deleting rows where expires_at IS NOT NULL AND expires_at < now, run at boot and on an interval, would cover it. Two things to be careful of:

Do not delete consumed-but-unexpired rows. consume() marks rather than deletes precisely so a replayed authorization code is distinguishable from an unknown one — deleting early would downgrade the provider's response from "revoke the whole grant" to a plain refusal.
Keep the deletion predicate in a tested function rather than inline in server.ts, which has no test coverage by design.

Not urgent

No correctness impact, and the growth rate on a single-operator instance is slow. Filed so it is not rediscovered as a mystery when a long-lived instance's SQLite file becomes conspicuous.

Spun out of #151, flagged independently by two reviewers. ## The gap `nexus/src/lib/server/oauth/adapter.ts` filters expiry at **read time** (`revive()` returns `undefined` for a row past `expires_at`) and never deletes. Nothing sweeps the table. The only deletions are `revokeByGrantId` (reachable only from a logout or revocation flow, and `config.ts` leaves both `features.revocation` and `features.introspection` disabled) and `oidc-provider`'s own `Interaction.destroy()` on a **successful** resume. ## Why the existing precedent does not transfer Read-time filtering is the established pattern here — `api_tokens` (AGENTS.md fact #24) and the quota store (fact #19) both do it, and #151 followed them deliberately. But those tables hold **one row per durable entity**: a token the operator deliberately minted, a quota window that is overwritten. `oauth_payloads` gains a new row per grant, per authorization code, per access token, per refresh token, and per interaction ever issued. With `rotateRefreshToken: true`, a single long-lived MCP connection inserts a new row on every refresh and keeps the consumed predecessor. An interaction the operator abandons — closing the tab — leaks its row permanently, because `destroy()` only runs on the success path. So it grows from ordinary protocol traffic with no operator action, and there is no visibility: no count surfaced anywhere, no log line, no periodic job. ## Shape of a fix `src/server.ts` already has the pattern for this, for the skills-installer staging sweep: ```js setInterval(() => { void sweep().catch((err) => logger.warn({ err }, '…')); }, INTERVAL).unref(); ``` A sweep deleting rows where `expires_at IS NOT NULL AND expires_at < now`, run at boot and on an interval, would cover it. Two things to be careful of: - **Do not delete consumed-but-unexpired rows.** `consume()` marks rather than deletes precisely so a replayed authorization code is distinguishable from an unknown one — deleting early would downgrade the provider's response from "revoke the whole grant" to a plain refusal. - Keep the deletion predicate in a tested function rather than inline in `server.ts`, which has no test coverage by design. ## Not urgent No correctness impact, and the growth rate on a single-operator instance is slow. Filed so it is not rediscovered as a mystery when a long-lived instance's SQLite file becomes conspicuous.

lz added this to the MCP support (#140) milestone

2026-09-16 08:46:16 +02:00

lz referenced this issue

2026-09-16 09:21:44 +02:00

An OAuth 2.1 authorization server, so Claude can connect from a phone #168

lz referenced this issue

2026-09-17 05:06:47 +02:00

The MCP resource server, and the signing key out of the database #170