Add a backlog, consolidating deferred work that was only in commit messages

Several decisions had been consciously postponed - the Docker purge date, the
unencrypted dumps, the unestablished Docker-in-LXC cause - but they lived in
commit messages and chat, which is the same as nowhere.

Each entry records what would trigger picking it up, so none of it has to be
re-derived. Starts with the MCP tool-consolidation question: the answer is that
merging CRUD into one tool would cost the annotations, the scope boundary, the
schema clarity and the audit trail, and the useful rule is to merge only when
it is the same gesture with a different parameter.

Nothing here is urgent; that is the point of writing it down rather than doing it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W1Ee4Mc1X1SX8HgYa52zu7

.env.example

@@ -5,6 +5,9 @@ POSTGRES_PASSWORD="YOUR_PASSWORD"
 POSTGRES_DB="drinktracker"
 
 # NextAuth
+# Must be the exact public origin, with NO trailing slash. Beyond login redirects
+# it is the OAuth `issuer` and the MCP `resource` identifier, and connectors
+# compare both byte-for-byte - a mismatch fails discovery with no useful error.
 NEXTAUTH_URL="http://localhost:3000"
 NEXTAUTH_SECRET="generate-with: openssl rand -base64 32"
 AUTH_TRUST_HOST="true"  # Set to true when behind a reverse proxy
@@ -25,3 +28,9 @@ ENCRYPTION_KEY="generate-with: openssl rand -hex 32"
 # OpenAI-compatible router that picks the best model per request. LAN-only, plain HTTP.
 # Each user adds their own gateway API key in Settings; this is only the endpoint.
 SWITCHBOARD_BASE_URL="http://192.168.2.11:8787/v1"
+
+# MCP server (optional)
+# The `search` and `fetch` tools exist only for ChatGPT's deep-research mode.
+# Their generic names can muddy tool selection when several connectors are
+# attached at once, so set this to 0 on a Claude-only deployment to drop them.
+# MCP_CHATGPT_TOOLS="0"

BACKLOG.md

@@ -0,0 +1,116 @@
+# Backlog
+
+Things deliberately deferred, with enough context to pick them up cold. Not a
+wish list — everything here has been considered and consciously postponed, and
+each entry says what would trigger doing it.
+
+Last reviewed: 2026-08-10
+
+---
+
+## Consolidating MCP tools
+
+**Trigger: the model starts picking the wrong tool, or `tools/list` overhead becomes measurable.**
+
+There are 23 tools. The often-quoted ceiling of ~20 is a rule of thumb, not a
+threshold, and mis-selection is caused by tools that resemble each other far more
+than by raw count — these are mostly distinct. It has been working well in
+practice, so this is not currently a problem.
+
+The question that prompted this: could one `modify_drink` take an `action` of
+create/update/delete and replace three tools? Technically yes, but it is the
+wrong merge:
+
+- **Annotations stop meaning anything.** A tool that *can* delete has to be
+  marked `destructiveHint`, so a client that confirms destructive actions would
+  start prompting when you add a drink — or you mark it false and lose the
+  prompt when it matters.
+- **Scope enforcement gets coarser.** Reads need `drinks:read` and writes need
+  `drinks:write`. A merged tool must demand the union (a read-only token loses
+  read access) or branch internally, at which point the declared scope no longer
+  describes the tool and the consent screen misleads.
+- **The schema gets harder for the model.** `id` is required for update/delete
+  and meaningless for create. That is a discriminated union, which converts to
+  `anyOf` — exactly the shape models fill in wrong.
+- **The audit trail gets vaguer.** `McpAuditLog` records the tool name;
+  `delete_drink` says what happened, `modify_drink` does not.
+
+The rule worth keeping: **merge when it is the same gesture with a different
+parameter; keep separate when the intent or the risk differs.** `upsert_bar_item`
+("I bought gin" / "I finished the gin") and `promote_recipe` (drink vs wishlist)
+are good merges. CRUD is not.
+
+Cheaper levers if the count ever needs to come down:
+
+1. `MCP_CHATGPT_TOOLS=0` drops the `search`/`fetch` aliases: 23 → 21, free if
+   ChatGPT is unused.
+2. Remove low-value tools rather than merging good ones. `update_rating` is the
+   weakest — a drink can simply be re-rated.
+3. Sharpen descriptions. Selection failures are usually a description problem.
+
+Worth measuring the real `tools/list` payload cost before spending effort here.
+
+---
+
+## Refresh-token grace window
+
+**Trigger: a client is forced to re-authorize after a dropped network response.**
+
+Reuse of any refresh generation currently revokes the whole family, with no
+grace. That is correct against theft but harsh on a client whose response was
+lost in transit — it replays legitimately and destroys its own grant.
+
+The audit's advice was to build durable lineage first and only then consider
+grace. Lineage now exists (`OAuthRefreshToken`, one row per generation, atomic
+consume), so this is a safe change to make if it ever bites. Auth0 disables
+leeway by default; Okta defaults to 30 seconds, configurable to 60. A 5–10
+second window limited to the immediately preceding generation would be
+reasonable — anything older must still revoke the family.
+
+---
+
+## Encrypt backup dumps
+
+**Trigger: backups leave the LAN, for any reason.**
+
+`pg_dump` output goes to TrueNAS unencrypted and contains every user's data plus
+bcrypt password hashes. Defensible on a NAS you control on your own network;
+not defensible in cloud storage, a friend's NAS, or off-site replication.
+
+`gpg` or `age` before the `scp` in `/usr/local/bin/drinktracker-backup` is a
+couple of lines. Much easier to add before there is history to re-encrypt.
+
+Related: the off-site copy is in the same building, so it survives losing the
+LXC or the Proxmox host but not fire or theft.
+
+---
+
+## Purge the retained Docker artifacts
+
+**Trigger: 2026-09-07.**
+
+Docker is installed but disabled on the LXC; old containers and volumes were
+kept as a rollback path after the move to native systemd. Nothing has needed
+them. Remove after that date.
+
+---
+
+## Establish why Docker actually failed in this LXC
+
+**Trigger: only if Docker is ever wanted on this host again.**
+
+`deploy/README.md` originally blamed unprivileged-LXC nesting. That is wrong —
+`/proc/self/uid_map` reads `0 0 4294967295`, an identity mapping, so the
+container is privileged. The AppArmor and `ip_unprivileged_port_start` failures
+were real and reproduced, but the cause was never established. The doc now says
+so rather than substituting a fresh guess.
+
+---
+
+## Rotate the TrueNAS account password
+
+**Trigger: do it whenever convenient.**
+
+`getent passwd` run as root on TrueNAS prints the SHA-512 hash, and one ended up
+in a session transcript. Salted and not trivially reversible, but crackable
+offline against a weak password.

audit.md

@@ -0,0 +1,205 @@
+# Security audit: MCP and OAuth 2.1 surfaces
+
+Date: 2026-08-09
+Auditor: independent model review, commissioned after the OAuth server shipped.
+
+> **STATUS: all five findings were verified against the code and fixed.** Every
+> claim below was independently confirmed before any change was made — there
+> were no false positives, which is unusual for a cross-model review.
+>
+> The fixes and their regression tests are in the commit that follows this file.
+> In short: refresh tokens now have durable per-generation lineage with an atomic
+> consume (F1); consent is the only writer of grant scopes and codes are bound to
+> a grant epoch (F2); every approval starts a clean epoch that destroys prior
+> credentials (F3); loopback redirects match exactly apart from the port (F4);
+> and bar counts are gated on `bar:read` (F5).
+>
+> Q1's advice was taken: lineage first, no refresh grace window for now.
+> Q1 also corrected a claim made when the OAuth server shipped — reuse detection
+> covered only one generation back, not the whole family. That was the single
+> most valuable output of this audit.
+>
+> This document is kept as the record of what was reviewed. The findings below
+> describe the code as it was, not as it is.
+
+## Summary
+
+The MCP surface has a strong overall tenant-isolation posture: all 22 tools pass through required bearer authentication and per-tool scope enforcement, their primary queries are scoped to the authenticated `userId`, foreign-key inputs are ownership-checked, and the bearer verifier checks token revocation, expiry, grant revocation, and account suspension from the database on every request. I found no cross-user row access and no unauthenticated path into an MCP tool. The OAuth implementation's main weakness is state lineage rather than basic PKCE: refresh rotation remembers only one prior hash and performs validation, access-token issuance, and rotation as separate state transitions. A thief can rotate a stolen refresh token twice so the legitimate client's later replay is no longer associated with the grant and does not revoke the attacker's token family. Fixing durable, atomic refresh-token lineage is the single most important change.
+
+## Findings
+
+### 1. High — Refresh-token replay detection forgets the token family
+
+- **Severity:** High
+- **Confidence:** Confirmed
+- **Files:** `src/app/api/oauth/token/route.ts:152-170`, `src/app/api/oauth/token/route.ts:181-193`, `src/app/api/oauth/revoke/route.ts:28-47`, `src/lib/mcp/oauth.ts:243-255`, `prisma/schema.prisma:237-241`
+
+Only `refreshHash` and the immediately preceding `refreshPrevHash` are retained. Every successful rotation overwrites the prior predecessor. A refresh token therefore identifies its grant for only one subsequent generation, not for the lifetime of the token family.
+
+Concrete exploit path:
+
+1. An attacker steals the grant's current refresh token `R0`.
+2. The attacker sends `R0` to `POST /api/oauth/token` and receives `R1`; the row now stores `current=R1, previous=R0`.
+3. Before the legitimate client refreshes, the attacker sends `R1` and receives `R2`; the row now stores `current=R2, previous=R1`. The only database reference to `R0` has been erased.
+4. The legitimate client later sends `R0`. The query at `token/route.ts:152-159` finds no grant, returns `invalid_grant`, and never calls `revokeGrant`.
+5. The attacker retains `R2` and can continue rotating it for the sliding 90-day lifetime. The event that should expose theft instead looks like an unknown token and leaves the attacker's family active.
+
+The same loss of lineage defeats RFC 7009 revocation by token value. If the legitimate client posts the now-aged-out `R0` to `/api/oauth/revoke`, that endpoint cannot associate it with the grant, still returns its mandatory 200 response, and leaves `R2` active. Settings-based revocation by grant ID does still terminate the family.
+
+There is also an atomicity failure in the same path. Two simultaneous requests with the same current token can both read it as current before either reaches `storeRefreshToken`. Both can issue token responses, and the last unconditional update at `oauth.ts:249-255` wins. Neither request is treated as replay. This also means the current zero-grace policy is not actually strict under concurrency.
+
+**Specific fix:** Model refresh tokens as a lineage, for example an `OAuthRefreshToken` row per issued hash with `grantId`, generation, `usedAt`, and expiry. Retain consumed hashes until the family can no longer be used. In one database transaction, atomically change the presented current token from unused to used, create its successor, revoke/replace access tokens, and update grant activity. If the hash exists but is already consumed outside any configured grace interval, revoke the grant and all descendants. Use a guarded update/CAS, a row lock, or serializable transaction so only one non-grace exchange of a generation succeeds. An authenticated family/grant identifier inside the otherwise opaque token can make lookup efficient, but it does not remove the need to retain consumed generations.
+
+This behavior falls short of [RFC 9700 section 4.14.2](https://www.rfc-editor.org/rfc/rfc9700.html#section-4.14.2), which describes retaining the relationship between rotated tokens so presentation of an invalidated family member can revoke the active token.
+
+### 2. Medium — Stale code and refresh snapshots can restore broader scope than the latest consent
+
+- **Severity:** Medium
+- **Confidence:** Confirmed
+- **Files:** `src/app/oauth/authorize/page.tsx:84-91`, `src/app/oauth/authorize/page.tsx:139-160`, `src/app/api/oauth/authorize/route.ts:68-88`, `src/app/api/oauth/token/route.ts:99-126`, `src/app/api/oauth/token/route.ts:152-185`, `src/lib/mcp/oauth.ts:217-231`
+
+Authorization codes carry a scope snapshot, but they are bound only to `(client, user)`, not to a particular grant version or consent epoch. `exchangeCode` checks only that some current grant for that pair is active, then issues the code's stored scopes. `issueAccessToken` also writes those scopes back onto the standing grant. Refresh similarly uses a grant object read before issuance and writes that snapshot back.
+
+Concrete exploit path:
+
+1. A malicious public client has an existing broad grant and obtains an auto-approved broad authorization code `C-old` with its own valid PKCE challenge.
+2. The user disconnects it, then reconnects the same client read-only. The approval upsert reuses the same grant row, stores the narrow scopes, and clears `revokedAt`.
+3. Before `C-old`'s 60-second expiry, the client exchanges it with the correct verifier.
+4. `exchangeCode` finds the newly active grant, mints an access token using `C-old`'s broad scopes, and `issueAccessToken` writes those broad scopes back to the grant. The client gets more privilege than the user's latest read-only consent.
+
+A concurrent version does not require a stale code: a refresh request can read broad `grant.scopes`, a narrower consent can be stored, and the refresh can then mint and write the stale broad snapshot. Two different authorization codes can likewise be exchanged in an order chosen by the client, with the last exchange redefining the standing grant.
+
+No path was found that invents an unsupported scope or grants a scope the user never approved at any point. The defect is that an older approval can override the latest effective consent.
+
+**Specific fix:** Make the consent endpoint the only writer of standing grant scopes; remove the scope update from `issueAccessToken`. Add a monotonically increasing consent/grant version and bind every authorization code to the grant ID and version that authorized it. At exchange, require the same active version and require the current grant to cover every code scope; otherwise return `invalid_grant`. Read refresh scopes and atomically rotate the token inside the same transaction so a stale pre-consent snapshot cannot be written back.
+
+### 3. Medium — An in-flight exchange can repopulate a revoked grant and later revive a stolen credential
+
+- **Severity:** Medium
+- **Confidence:** Confirmed
+- **Files:** `src/app/api/oauth/token/route.ts:99-135`, `src/app/api/oauth/token/route.ts:146-193`, `src/lib/mcp/oauth.ts:212-275`, `src/app/api/oauth/authorize/route.ts:68-74`, `src/lib/mcp/auth.ts:58-66`
+
+Grant validation, access-token issuance, refresh-token storage, and grant revocation are separate transactions. Neither `issueAccessToken` nor `storeRefreshToken` conditionally requires `revokedAt` to remain null. Re-consent then clears only `revokedAt` and changes scopes; it does not clear refresh state or invalidate access tokens that may have been created after the revocation transaction scanned them.
+
+Concrete exploit path:
+
+1. A malicious client or refresh-token thief starts a valid refresh request and passes the active-grant checks.
+2. The user disconnects the client. `revokeGrant` marks the grant revoked, clears the two stored refresh hashes, and revokes access tokens visible to that transaction.
+3. The already in-flight request resumes. It can create an unrevoked access-token row and/or unconditionally write its newly returned refresh hash onto the now-revoked grant.
+4. While `revokedAt` remains set, `verifyMcpToken` correctly rejects the access token and refresh rejects the grant. There is no immediate bypass; this check was traced and is why the finding is not rated High.
+5. If the user later reconnects the same client, the approval upsert sets `revokedAt: null` without resetting those raced credentials. The attacker's refresh token becomes usable again for up to 90 days; a raced access token also becomes usable if its one-hour lifetime has not elapsed.
+
+There is a related non-racing state inconsistency. If a grant already has a refresh token and a later authorization code omits `offline_access`, `exchangeCode` skips `storeRefreshToken` but does not clear the existing refresh hashes. The grant scopes can therefore say that offline access is absent while the old refresh token remains accepted. The refresh handler never checks for `offline_access` and returns another refresh token on every successful use. Thus a previously issued refresh credential can continue persistent access after the standing scope array no longer contains the permission that authorized it.
+
+**Specific fix:** Serialize refresh/code issuance against revocation on the grant row and make the active-grant predicate part of the write, not an earlier read. A revoke should advance a grant epoch. Every token/code write should be conditional on the epoch it read, so it fails if revocation won the race. Re-consenting a revoked grant should atomically start a clean epoch: clear all refresh-token state and revoke/delete every old access token before setting the grant active. Also clear refresh state whenever the effective grant drops `offline_access`, and reject refresh grants whose current scope array lacks it. Creating a new grant row is viable if codes are bound to its exact ID/version.
+
+### 4. Medium — Loopback redirect matching ignores every component except scheme, host, and path
+
+- **Severity:** Medium
+- **Confidence:** Suspected
+- **Files:** `src/lib/mcp/oauth.ts:137-152`, `src/lib/mcp/authorize-request.ts:65-81`
+
+For loopback redirects, `redirectUriMatches` intentionally ignores the port, but it also ignores the query string, fragment, username, and password. A requested loopback URI can therefore differ from its registered value in security-relevant components and still become the actual redirect target.
+
+Concrete conditional exploit path: suppose a legitimate native client's registered loopback callback uses a query parameter to select or forward the post-callback destination. An attacker starts authorization with that legitimate public `client_id`, an attacker-controlled PKCE challenge, and the same loopback scheme/host/path but changes the query to an attacker destination. The server accepts it. If the victim has a covering standing grant, authorization is automatic; otherwise the legitimate client name is shown. A callback handler that forwards the altered query and authorization response can deliver the code to the attacker, who knows the verifier and exchanges it for the victim's token.
+
+The comparison defect is confirmed, but exploitability is marked **Suspected** because the database was deliberately not queried and the external clients' loopback callback behavior is not in this repository. I could not establish that a currently registered callback forwards attacker-controlled query data.
+
+**Specific fix:** For a supported loopback URI, compare the complete registered and requested URI after removing only the port. Query, fragment, path, userinfo, scheme, and host must otherwise match exactly; reject a requested fragment even if the registered URI has none. Apply the same rule at authorization and token exchange. [RFC 9700 section 2.1](https://www.rfc-editor.org/rfc/rfc9700.html#section-2.1) requires exact redirect matching except for the native-loopback port exception and identifies authorization-code leakage as the consequence of loose matching.
+
+### 5. Low — `drinks:read` discloses bar and recipe counts
+
+- **Severity:** Low
+- **Confidence:** Confirmed
+- **Files:** `src/lib/mcp/tools/drinks.ts:368-400`, `src/lib/mcp/tools/drinks.ts:417-436`, `src/lib/mcp/oauth.ts:27-32`
+
+`get_collection_stats` requires only `drinks:read`, but queries and returns the number of non-empty bar items and saved recipes. The advertised scope descriptions put bar inventory and recipes behind `bar:read`.
+
+Concrete exploit path: a client requests and receives only `drinks:read`, then calls `get_collection_stats`. It receives `barItemCount`, `recipeCount`, and the corresponding text despite the user never granting `bar:read`. This is same-user metadata rather than row contents, so the impact is Low.
+
+**Specific fix:** Remove the bar/recipe counts from this tool, return them only when `caller.scopes` includes `bar:read`, or require both `drinks:read` and `bar:read` for the entire tool. If more cross-axis tools are expected, extend `ToolSpec` to accept an array of required scopes rather than relying on ad hoc checks.
+
+## Material negative results
+
+- **No cross-tenant MCP access found.** All 22 tools are registered through `defineTool`; all 22 declare a scope. Reads use `caller.userId`, creates set it server-side, and updates/deletes first resolve ownership. `rate_drink` verifies the supplied drink belongs to the caller, and `save_recipe` does the same for `sourceDrinkId`.
+- **No unauthenticated MCP operation found.** `withMcpAuth` is configured with `required: true`; the verifier accepts only the MCP token shape; no route calls `auth()` as a fallback; cookies cannot authenticate `/api/mcp`. `OPTIONS` exposes no data or state change.
+- **Revocation, expiry, and suspension are checked live.** `verifyMcpToken` checks the token row, access-token expiry, OAuth grant revocation, and the joined user's current status on every request. There is no token cache. Finding 3 requires a later re-consent because the live grant check correctly blocks the raced token while the grant remains revoked.
+- **PKCE and same-code concurrency are sound.** S256 is mandatory, verifier syntax is constrained, comparison is timing-safe, and the guarded code consume permits only one exchange of a given code. The consume-before-validation ordering can burn a code or let a bad verifier revoke its grant, but that is availability/DoS and is outside the stated threat model.
+- **Dynamic registration did not expose SSRF or client secrets.** Registration performs no outbound fetch, generates the client ID server-side, accepts only public clients, and restricts redirects to HTTPS or loopback. Untrusted client metadata is rendered as escaped text in the reviewed UI.
+- **Settings management is tenant-scoped.** PAT and OAuth-grant listing/revocation use the cookie-authenticated user's ID in their reads and writes.
+- **The deliberate wildcard MCP CORS policy does not combine with cookie auth.** No `Access-Control-Allow-Credentials` header or session-cookie authentication path was found.
+
+## Q1. Should refresh-token replay have a grace window?
+
+Zero grace is security-correct but operationally harsh; it is not required to be zero by the OAuth security BCP. [RFC 9700 section 4.14.2](https://www.rfc-editor.org/rfc/rfc9700.html#section-4.14.2) requires public clients to use sender-constrained refresh tokens or rotation with retained family relationships and describes family revocation when an invalidated token reveals reuse. It does not prescribe a grace duration.
+
+Mature implementations expose both choices. [Auth0 offers a configurable rotation overlap](https://dev.auth0.com/docs/secure/tokens/refresh-tokens/configure-refresh-token-rotation): its default leeway is disabled, but a configured window allows only the immediately previous generation and older generations still trigger family invalidation. [Okta defaults to a 30-second grace period configurable from 0 to 60 seconds](https://developer.okta.com/docs/guides/refresh-tokens/main/#grace-period-for-token-rotation), explicitly to handle a lost response or poor network.
+
+For this app, the present zero-grace behavior is defensible if forced reauthorization after a lost response is acceptable. A short 5–10 second grace window would be a reasonable reliability choice, limited to the immediately preceding generation; any older generation or a replay outside the window should revoke the family. Do **not** add grace to the current implementation first. Finding 1 means old generations are forgotten, and simultaneous current-token requests are not atomically resolved. Implement durable lineage and atomic rotation, then choose grace. For robust retry semantics, either retain enough protected successor state to return the same result idempotently or use generation-aware rotation that cannot let late responses overwrite newer client state.
+
+One correction to the question's premise: the implementation does not revoke the grant whenever a previous token is “ever” presented. It recognizes only the immediately preceding hash until the next successful rotation overwrites it.
+
+## Q2. Can a token receive more scope than the user consented to?
+
+**Normal, non-racing path:** no problem found. `validateAuthorizeRequest` parses the requested scopes; explicit approval writes the same array to both grant and code; auto-approval requires `coversScopes(existing.scopes, requested)` and puts only the requested subset on the code; access-token creation removes only the `offline_access` marker. Unsupported scopes are never written to an access token.
+
+**Across consent changes and concurrency:** yes, a token can exceed the user's latest effective consent, as detailed in Finding 2. A broad code from an older consent can be exchanged after the same grant row is revoked and reactivated with narrow scopes. A refresh that read broad scopes before the consent change can also issue and write those scopes afterward. The code/refresh never creates a scope the user did not historically approve, but it can override a later narrowing or revoke-and-reconsent decision. The root causes are the absence of a consent epoch and `issueAccessToken` treating token issuance as authority to rewrite standing consent.
+
+Two concurrent exchanges of different valid codes can both succeed and whichever token issuance writes last determines the grant's stored scopes. That does not by itself invent scope, but it makes request ordering—not the latest consent action—the final authority.
+
+`offline_access` has a separate persistence defect rather than an access-token-scope escalation: removing it from the effective grant does not clear a previously issued refresh token, and refresh does not require the marker. That path is covered in Finding 3.
+
+## Q3. Is consume-before-grant/PKCE safe, and what happens under concurrency?
+
+The exact ordering is:
+
+| Stage | Code behavior | Security/concurrency result |
+|---|---|---|
+| 1 | Hash lookup loads the code plus client and user snapshot (`token/route.ts:65-71`). | No lock is held; expiry is not accepted here. |
+| 2 | `updateMany` sets `consumedAt` only where it is null and `expiresAt` is still in the future (`:74-87`). | For the **same code**, exactly one concurrent request gets `count=1`; all others fail. Expiry between lookup and consume also fails closed. |
+| 3 | Client ID and redirect are checked (`:90-97`). | A wrong request burns the code but gets no token. This is availability, not token theft. |
+| 4 | The current grant is loaded by `(client row ID, user ID)` and must be active (`:99-105`). | A missing or already-revoked grant fails. The code is already consumed. |
+| 5 | PKCE is verified (`:107-116`). | A mismatch cannot mint a token and revokes the current grant. The code remains consumed. |
+| 6 | The user snapshot's status is checked (`:118-120`). | A suspended account gets no token; MCP also rechecks live status on use. |
+| 7 | `issueAccessToken` revokes visible access tokens, creates one, and updates grant scopes/activity in a transaction (`oauth.ts:212-233`). | The transaction does not condition on the grant still being active or on a grant version. A concurrent revoke can win without preventing this row from being created, although live grant verification blocks it while revoked. |
+| 8 | If requested, the first refresh token is stored in a separate write (`token/route.ts:129-134`). | A revoke can interleave before this write; re-consent can later revive the raced state (Finding 3). |
+
+Direct answers:
+
+- **Can a code outlive its grant?** Yes in the logical authorization sense. `OAuthAuthCode` has foreign keys to client and user, not to `OAuthGrant`, and soft revocation does not consume/delete codes. A code cannot exchange while the grant is revoked, but if the same `(client,user)` row is reactivated before the code's 60-second expiry, the old code becomes usable again. It can carry stale scopes (Finding 2).
+- **Can a revoked grant be resurrected?** `exchangeCode` and `issueAccessToken` do not directly clear `revokedAt`, and `verifyMcpToken` checks the grant, so a concurrent exchange does not immediately bypass a grant that stays revoked. The approval upsert intentionally reactivates the row. Because that reactivation has no epoch/reset, it can also reactivate pre-revocation codes or credentials written by an in-flight request (Findings 2 and 3).
+- **Can two concurrent exchanges both succeed?** Two exchanges of the **same authorization code cannot both succeed** because of the guarded consume. Two different valid codes for the same grant can both succeed. Two concurrent refresh requests using the same current refresh token can also both succeed because validation and rotation are not one guarded transaction (Finding 1).
+
+Consume-before-PKCE is therefore not the source of a duplicate same-code mint. Its security cost is code burning and grant revocation on a bad verifier; under the stated exclusion of DoS, that ordering is acceptable. The material problems are the missing grant/consent version and the non-atomic writes after the consume.
+
+## Coverage
+
+| Requested file | Coverage | Result |
+|---|---|---|
+| `src/lib/mcp/oauth.ts` | Fully reviewed | Findings 1–4; PKCE primitive itself clean. |
+| `src/app/api/oauth/token/route.ts` | Fully reviewed | Findings 1–3; same-code consume is concurrency-safe. |
+| `src/lib/mcp/authorize-request.ts` | Fully reviewed | Fatal-before-redirect validation order is sound; Finding 4 inherits the loopback matcher. |
+| `src/app/oauth/authorize/page.tsx` | Fully reviewed | Auto-approve correctly uses `coversScopes`; stale-consent issue is Finding 2. |
+| `src/app/api/oauth/authorize/route.ts` | Fully reviewed | Cookie auth and request revalidation are present; Findings 2–3 concern grant reuse/versioning. |
+| `src/lib/mcp/auth.ts` | Fully reviewed | Per-request token, expiry, grant, and user-status checks are sound. |
+| `src/app/api/mcp/route.ts` | Fully reviewed | Bearer auth is required; no cookie fallback; CORS decision is safe under current wiring. |
+| `src/app/api/oauth/register/route.ts` | Fully reviewed | No SSRF or secret storage found; redirect registration restrictions present. |
+| `src/lib/mcp/tools/bar.ts` | Fully reviewed | No IDOR or scope bypass found. |
+| `src/lib/mcp/tools/chatgpt.ts` | Fully reviewed | No IDOR or scope bypass found. |
+| `src/lib/mcp/tools/drinks.ts` | Fully reviewed | No IDOR; Finding 5 is a cross-scope count disclosure. |
+| `src/lib/mcp/tools/meta.ts` | Fully reviewed | No IDOR or scope bypass found. |
+| `src/lib/mcp/tools/recipes.ts` | Fully reviewed | `sourceDrinkId` ownership is checked; no IDOR found. |
+| `src/lib/mcp/tools/wishlist.ts` | Fully reviewed | No IDOR or scope bypass found. |
+| `src/lib/mcp/context.ts` | Fully reviewed | Missing auth fails closed; scope check is exact. |
+| `src/lib/mcp/define.ts` | Fully reviewed | Every tool goes through auth/scope/error plumbing. |
+| `src/lib/mcp/schemas.ts` | Fully reviewed | Image URL omitted; IDs and write schemas traced to validators. |
+| `src/lib/auth.ts` | Fully reviewed | `/oauth/authorize` is not public; JWT revalidates active user. |
+| `prisma/schema.prisma` | Fully reviewed | OAuth uniqueness/cascades and all related user-owned relationships traced. |
+| `src/app/api/settings/mcp-tokens/route.ts` | Fully reviewed | User-scoped listing/creation; no token secret stored. |
+| `src/app/api/settings/mcp-tokens/[id]/route.ts` | Fully reviewed | Guarded user-scoped revocation. |
+| `src/app/api/settings/mcp-grants/route.ts` | Fully reviewed | User-scoped listing. |
+| `src/app/api/settings/mcp-grants/[id]/route.ts` | Fully reviewed | User-scoped lookup before grant revocation. |
+
+Supporting paths fully reviewed: `src/app/api/oauth/revoke/route.ts`, all three OAuth/protected-resource discovery routes under `src/app/.well-known`, `src/middleware.ts`, `src/lib/authz.ts`, `src/components/oauth/consent-form.tsx`, `src/lib/validators.ts`, `src/lib/mcp/{config,tokens,server,audit}.ts`, and the relevant `mcp-handler` authentication wrapper. Relevant ownership invariants were cross-checked in the existing ratings and recipes API routes. The `mcp-handler` package as a whole and unrelated application features were not audited.
+
+Validation: `./node_modules/.bin/tsc --noEmit --incremental false` completed successfully. The application was not run, and no database or service was contacted.

deploy/README.md

@@ -26,9 +26,13 @@ The app runs **natively under systemd — there is no Docker on this host.**
 
 **`drinktracker.tenseconddelay.net` resolves to the reverse proxy (192.168.2.172), not the container.** `NEXTAUTH_URL` points at the public name, so resolving it and SSHing there fails in a way that looks like a rejected key but is simply a different machine.
 
-**`next.config.mjs` evaluates `rewrites()` at BUILD time.** The `/minio-images/:path*` destination is baked into `routes-manifest.json` as `http://localhost:9000/drink-images/:path*`. Changing `MINIO_ENDPOINT` in the env file appears to work and does nothing — images break only after the *next* rebuild, so the change and the breakage are separated in time. MinIO must stay on `localhost:9000` serving `drink-images`.
+**Images are served by an authenticated route, not a rewrite.** `next.config.mjs` has no `rewrites()` block at all. `/minio-images/:path*` is a route handler (`src/app/minio-images/[...key]/route.ts`) that requires a session, rejects `..` in the key, and enforces per-user ownership by key prefix (`<userId>/` or `scans/<userId>/`). It reads `MINIO_ENDPOINT` at request time, so changing it takes effect on restart with no rebuild.
 
-**The image proxy is an unauthenticated GET.** Next rewrites don't sign requests, so the bucket's anonymous-download policy is load-bearing. Verify with `mc anonymous get local/drink-images` → must say `download`.
+> This replaced an unauthenticated rewrite that, combined with an anonymous-download bucket policy, exposed every stored image to the public internet. **The bucket must not have an anonymous policy, and MinIO must stay bound to loopback** (`127.0.0.1:9000`, verify with `ss -lntp | grep 9000`). `mc anonymous get local/drink-images` is not a usable check here — the `local` alias on this host has no working credentials and returns `Access Denied` regardless. Check from outside instead: `curl -si https://drinktracker.tenseconddelay.net/minio-images?list-type=2` must **not** return a `200` with an XML key listing.
+
+**Never use `next/image` for stored images.** The optimizer fetches them server-side without the session cookie, so every image would 401. All renders are plain `<img>`, and there is deliberately no `images.remotePatterns` config.
+
+**`NEXTAUTH_URL` must exactly equal the public origin, with no trailing slash.** Beyond login redirects it now drives the OAuth `issuer` and the MCP `resource` identifier, both of which clients compare byte-for-byte. A mismatch does not error — connectors just fail to complete with an unhelpful "couldn't reach the server".
 
 **`prisma db push`, not `migrate deploy`.** Production has no `_prisma_migrations` table, so `migrate deploy` would try to apply the init migration against populated tables and fail. `--accept-data-loss` means removing a field from `schema.prisma` **drops the column and its data** — the deploy script dumps first for this reason.
 
@@ -36,13 +40,15 @@ The app runs **natively under systemd — there is no Docker on this host.**
 
 Build happens in `/opt/drinktracker/repo` and is assembled **out-of-place** into `releases/<sha>`, so the ~3 minute build runs while the current release keeps serving. Only the symlink swap and `systemctl restart` cause downtime, about 3 seconds. Rollback is the same swap in reverse and needs no rebuild.
 
-The build deliberately does **not** source the env file — same as the Dockerfile — because of the build-time rewrite baking described above.
+The build deliberately does **not** source the env file, same as the Dockerfile. Nothing in this app needs configuration baked into the bundle: `NEXTAUTH_URL`, `MINIO_ENDPOINT` and the rest are read at request time by dynamic routes, so the build stays identical regardless of which host it runs on. (This originally guarded against a `rewrites()` block that baked a MinIO URL into `routes-manifest.json`; that block is gone, but building without secrets is still the right default.)
 
 ## Docker still works, just not here
 
 `Dockerfile` and `docker-compose.prod.yml` are maintained for running this app elsewhere (a VPS). `docker compose build && docker compose up -d` works on any normal Docker host.
 
-It does **not** work on this LXC, which is why the app runs natively. Two independent problems, both from unprivileged-LXC nesting:
+It does **not** work on this LXC, which is why the app runs natively. Two independent problems, both from running a container runtime nested inside an LXC:
+
+> An earlier version of this note blamed *unprivileged*-LXC nesting. That is wrong: `/proc/self/uid_map` reads `0 0 4294967295`, an identity mapping, so **this container is privileged**. The two failures below were reproduced and are real; the root cause was never actually established, so don't treat "because unprivileged" as a diagnosis if you revisit this.
 
 1. AppArmor — the container can't load the `docker-default` profile (`apparmor_parser: Access denied`), so any container without `--security-opt apparmor=unconfined` fails.
 2. runc can't set `net.ipv4.ip_unprivileged_port_start`, so containers only work with `--network host`.
@@ -56,16 +62,59 @@ ssh -i ~/.ssh/drinktracker_ed25519 drinkadmin@192.168.2.169
 systemctl is-active postgresql@16-main minio drinktracker
 sudo journalctl -u drinktracker -n 50
 sudo journalctl -u drinktracker | grep '\[switchboard\]'    # AI routing + cost per call
+sudo journalctl -u drinktracker | grep '\[mcp\]'            # one line per MCP tool call
+sudo journalctl -u drinktracker | grep '\[oauth\]'          # connector grants and token issuance
 readlink /opt/drinktracker/current                          # which release is live
 ```
 
 Each AI call logs one `[switchboard] feature=… model=… cost=… latency_ms=…` line. `FAILOVER` warnings are expected — the gateway's `:batch` model variants currently fail on every request and fall back. `CONTEXT OVERFLOW` is worth investigating.
 
+MCP tool calls log `[mcp] user=… tool=… ok=… ms=…`, and the same events are rows in `McpAuditLog` (arguments are deliberately never recorded). `[oauth]` lines cover consent grants and token issuance; a `PKCE mismatch` or `refresh token replay` warning there means a grant was revoked defensively and is worth looking at.
+
+Discovery can be checked without credentials:
+
+```bash
+curl -s https://drinktracker.tenseconddelay.net/.well-known/oauth-protected-resource | jq
+curl -si https://drinktracker.tenseconddelay.net/api/mcp -X POST \
+  -H 'content-type: application/json' -H 'accept: application/json, text/event-stream' \
+  -H 'MCP-Protocol-Version: 2026-07-28' -H 'Mcp-Method: tools/list' \
+  -d '{"jsonrpc":"2.0","id":1,"method":"tools/list","params":{"_meta":{"io.modelcontextprotocol/protocolVersion":"2026-07-28","io.modelcontextprotocol/clientCapabilities":{}}}}'
+```
+
+The second must return **401** with a `WWW-Authenticate: Bearer … resource_metadata="…"` header. A `307` there means `/.well-known` fell out of `PUBLIC_ROUTES` in `src/lib/auth.ts` and discovery is broken.
+
 ## Backups
 
 `drinktracker-backup.timer` runs nightly at 03:25: `pg_dump` + a tar of `/var/lib/minio/data` + a copy of the env file, into `/var/backups/drinktracker`, retained 14 days (~1 GB).
 
-**TODO — the chain has no off-site leg.** `/usr/local/bin/drinktracker-backup` supports `OFFSITE_DEST` (an scp target) and `OFFSITE_KEY`; set them via `Environment=` in `drinktracker-backup.service`. Until then everything lives on one LXC on one Proxmox host, and the nightly job prints a warning saying so.
+Each run also copies the dump and the MinIO tarball off-site to TrueNAS:
+
+| | |
+|---|---|
+| Destination | `jpscott84@192.168.2.196:/mnt/Beefcake/backup/drinktracker` (dataset `Beefcake/backup`) |
+| Transport | `scp` over SSH — **not** NFS, so there is no mount to go stale and silently swallow a night's backup into the LXC's own disk |
+| Key | `/root/.ssh/drinktracker_backup_ed25519`, a dedicated backup-only keypair |
+| Config | `Environment=` lines in `/etc/systemd/system/drinktracker-backup.service.d/offsite.conf` |
+
+Set as a **drop-in** rather than an edit to the unit, so re-installing the service cannot silently drop the off-site leg and return to local-only backups.
+
+A successful run logs `offsite copy ok -> …`. If it ever logs `NOTE: OFFSITE_DEST unset` again, the drop-in has gone missing. A failed copy logs `WARNING: offsite copy FAILED` and **keeps the local copy** — the job still exits 0, so watch the log rather than the exit status.
+
+```bash
+sudo systemctl start drinktracker-backup.service       # run it now
+sudo journalctl -u drinktracker-backup -n 20           # look for "offsite copy ok"
+```
+
+Verify what landed is actually restorable, not merely present:
+
+```bash
+sudo ssh -i /root/.ssh/drinktracker_backup_ed25519 jpscott84@192.168.2.196 \
+  'ls -lh /mnt/Beefcake/backup/drinktracker && gzip -t /mnt/Beefcake/backup/drinktracker/pg-*.sql.gz && echo OK'
+```
+
+**Two gaps that remain.** The dumps are **unencrypted** and contain every user's data plus bcrypt password hashes — acceptable writing to a NAS on the same LAN, but encrypt before this ever leaves the network. And TrueNAS is in the same building, so this survives losing the LXC or the Proxmox host, not fire or theft.
+
+> If SSH key auth to TrueNAS ever breaks: sshd refuses `authorized_keys` when the user's **home directory** is group- or world-writable, which TrueNAS datasets often are by default. `/mnt/local/jpscott84` must not be `777`. The symptom is indistinguishable from a missing key — the key is offered and silently rejected — so check `journalctl -u ssh | grep 'bad ownership'` on the NAS first.
 
 Restore:
 

deploy/deploy.sh

@@ -109,9 +109,10 @@ git -C "$APP_DIR/repo" log --oneline -1
 
 cd "$APP_DIR/repo"
 export npm_config_cache="$APP_DIR/.npm" NODE_OPTIONS=--max-old-space-size=3072
-# Built WITHOUT the env file, deliberately - same as the Dockerfile. next.config.mjs
-# evaluates rewrites() at build time, so sourcing the env here would bake a different
-# /minio-images destination into routes-manifest.json.
+# Built WITHOUT the env file, deliberately - same as the Dockerfile. Every setting
+# this app reads (NEXTAUTH_URL, MINIO_ENDPOINT, ...) is read at request time by a
+# dynamic route, so nothing needs baking in and the bundle stays host-independent.
+# Keep it that way: a build that depends on secrets stops being reproducible.
 npm ci --prefer-offline --no-audit --fund=false
 npx prisma generate
 npm run build

prisma/schema.prisma

@@ -208,6 +208,12 @@ model OAuthAuthCode {
   codeHash            String    @unique
   clientId            String
   userId              String
+  /// The grant, and the epoch of that grant, this code was authorized against.
+  /// Without both, a code is bound only to (client, user): one minted under a
+  /// broad consent stays redeemable after the user narrows or revokes and
+  /// re-grants, carrying the old scopes with it.
+  grantId             String
+  grantEpoch          Int       @default(0)
   /// Exactly what was sent to /oauth/authorize - re-checked at the token endpoint
   redirectUri         String
   /// RFC 8707 audience binding
@@ -221,8 +227,10 @@ model OAuthAuthCode {
 
   client OAuthClient @relation(fields: [clientId], references: [id], onDelete: Cascade)
   user   User        @relation(fields: [userId], references: [id], onDelete: Cascade)
+  grant  OAuthGrant  @relation(fields: [grantId], references: [id], onDelete: Cascade)
 
   @@index([userId])
+  @@index([grantId])
   @@index([expiresAt])
 }
 
@@ -232,27 +240,58 @@ model OAuthGrant {
   id       String   @id @default(cuid())
   clientId String
   userId   String
+  /// Written ONLY by the consent endpoint. Token issuance must never rewrite
+  /// standing consent - doing so let a stale authorization code redefine what
+  /// the user had most recently agreed to.
   scopes   String[]
 
-  refreshHash      String?   @unique
-  /// One generation back. A hit here means a rotated token was replayed, which
-  /// is a theft signal - revoke the whole grant rather than issuing again.
-  refreshPrevHash  String?
-  refreshExpiresAt DateTime?
+  /// Bumped every time consent is re-granted after a revocation. Codes and
+  /// refresh tokens carry the epoch they were issued under, so anything minted
+  /// before a revoke is dead even if the grant row is later reactivated.
+  epoch Int @default(0)
 
   revokedAt  DateTime?
   lastUsedAt DateTime?
   createdAt  DateTime  @default(now())
   updatedAt  DateTime  @updatedAt
 
-  client       OAuthClient      @relation(fields: [clientId], references: [id], onDelete: Cascade)
-  user         User             @relation(fields: [userId], references: [id], onDelete: Cascade)
-  accessTokens McpAccessToken[]
+  client        OAuthClient         @relation(fields: [clientId], references: [id], onDelete: Cascade)
+  user          User                @relation(fields: [userId], references: [id], onDelete: Cascade)
+  accessTokens  McpAccessToken[]
+  authCodes     OAuthAuthCode[]
+  refreshTokens OAuthRefreshToken[]
 
   @@unique([clientId, userId])
   @@index([userId])
 }
 
+/// One row per issued refresh token, retained after use.
+///
+/// The previous design kept only the current hash and one predecessor on the
+/// grant, so each rotation erased the generation before last. A thief who
+/// rotated a stolen token twice made the victim's original token unrecognisable:
+/// replaying it returned "unknown token" instead of revoking the family, and the
+/// thief kept working. Keeping every generation is what makes reuse detectable
+/// for the life of the family (RFC 9700 4.14.2).
+///
+/// Consumed with a guarded update on usedAt, so two concurrent uses of the same
+/// token cannot both succeed.
+model OAuthRefreshToken {
+  id         String    @id @default(cuid())
+  grantId    String
+  tokenHash  String    @unique
+  generation Int       @default(0)
+  /// Set the moment it is redeemed. A second presentation is reuse.
+  usedAt     DateTime?
+  expiresAt  DateTime
+  createdAt  DateTime  @default(now())
+
+  grant OAuthGrant @relation(fields: [grantId], references: [id], onDelete: Cascade)
+
+  @@index([grantId])
+  @@index([expiresAt])
+}
+
 model Account {
   id                String  @id @default(cuid())
   userId            String

src/app/api/oauth/authorize/route.ts

@@ -65,28 +65,49 @@ export async function POST(request: Request) {
     })
   }
 
-  // One standing grant per (client, user). Re-approving widens the scopes and
-  // clears a previous revocation rather than piling up rows.
-  const grant = await prisma.oAuthGrant.upsert({
-    where: { clientId_userId: { clientId: client.id, userId } },
-    update: { scopes: params.scopes, revokedAt: null },
-    create: { clientId: client.id, userId, scopes: params.scopes },
-    select: { id: true },
-  })
-
   const { code, hash } = generateAuthCode()
-  await prisma.oAuthAuthCode.create({
-    data: {
-      codeHash: hash,
-      clientId: client.id,
-      userId,
-      redirectUri: params.redirectUri,
-      resource: params.resource,
-      scopes: params.scopes,
-      codeChallenge: params.codeChallenge,
-      codeChallengeMethod: "S256",
-      expiresAt: new Date(Date.now() + AUTH_CODE_TTL_MS),
-    },
+
+  // One standing grant per (client, user), but every explicit approval starts a
+  // CLEAN EPOCH: the epoch counter advances and everything issued under the
+  // previous one is destroyed.
+  //
+  // Without this, reusing the row made a revocation reversible. Disconnecting an
+  // app then reconnecting it cleared revokedAt and left any credential that had
+  // raced the revoke - or any code minted under the older, broader consent -
+  // live again. Consent is also the only place standing scopes are written; the
+  // token endpoint no longer rewrites them.
+  const grant = await prisma.$transaction(async (tx) => {
+    const g = await tx.oAuthGrant.upsert({
+      where: { clientId_userId: { clientId: client.id, userId } },
+      update: { scopes: params.scopes, revokedAt: null, epoch: { increment: 1 } },
+      create: { clientId: client.id, userId, scopes: params.scopes },
+      select: { id: true, epoch: true },
+    })
+
+    await tx.mcpAccessToken.updateMany({
+      where: { grantId: g.id, revokedAt: null },
+      data: { revokedAt: new Date() },
+    })
+    await tx.oAuthRefreshToken.deleteMany({ where: { grantId: g.id } })
+    await tx.oAuthAuthCode.deleteMany({ where: { grantId: g.id, consumedAt: null } })
+
+    await tx.oAuthAuthCode.create({
+      data: {
+        codeHash: hash,
+        clientId: client.id,
+        userId,
+        grantId: g.id,
+        grantEpoch: g.epoch,
+        redirectUri: params.redirectUri,
+        resource: params.resource,
+        scopes: params.scopes,
+        codeChallenge: params.codeChallenge,
+        codeChallengeMethod: "S256",
+        expiresAt: new Date(Date.now() + AUTH_CODE_TTL_MS),
+      },
+    })
+
+    return g
   })
 
   console.log(

src/app/api/oauth/revoke/route.ts

@@ -29,13 +29,15 @@ export async function POST(request: Request) {
 
   try {
     // A refresh token identifies the whole grant, so revoking it ends the
-    // connection outright.
-    const grant = await prisma.oAuthGrant.findFirst({
-      where: { OR: [{ refreshHash: hash }, { refreshPrevHash: hash }] },
-      select: { id: true },
+    // connection outright. Every generation is retained, so this works for any
+    // member of the family - previously only the newest two were recognised,
+    // which meant revoking an older token silently did nothing.
+    const refresh = await prisma.oAuthRefreshToken.findUnique({
+      where: { tokenHash: hash },
+      select: { grantId: true },
     })
-    if (grant) {
-      await revokeGrant(grant.id)
+    if (refresh) {
+      await revokeGrant(refresh.grantId)
       return ok
     }
 

src/app/api/oauth/token/route.ts

@@ -1,14 +1,16 @@
 import { NextResponse } from "next/server"
 import { prisma } from "@/lib/prisma"
 import {
+  GrantStateError,
   OFFLINE_ACCESS,
-  generateRefreshToken,
+  consumeRefreshToken,
+  coversScopes,
   issueAccessToken,
+  issueRefreshToken,
   oauthError,
   redirectUriMatches,
   revokeGrant,
   sha256,
-  storeRefreshToken,
   verifyPkce,
 } from "@/lib/mcp/oauth"
 
@@ -40,8 +42,16 @@ export async function POST(request: Request) {
   const form = await readForm(request)
   const grantType = form.get("grant_type")
 
-  if (grantType === "authorization_code") return exchangeCode(form)
-  if (grantType === "refresh_token") return refresh(form)
+  try {
+    if (grantType === "authorization_code") return await exchangeCode(form)
+    if (grantType === "refresh_token") return await refresh(form)
+  } catch (error) {
+    // Raised when a revoke or a fresh consent won a race against this request.
+    if (error instanceof GrantStateError) {
+      return oauthError("invalid_grant", error.message)
+    }
+    throw error
+  }
 
   return oauthError(
     "unsupported_grant_type",
@@ -96,13 +106,27 @@ async function exchangeCode(form: URLSearchParams) {
     return oauthError("invalid_grant", "redirect_uri does not match the authorization request.")
   }
 
+  // Resolved by the code's own grantId, not by (client, user). Those can point
+  // at the same row across a revoke-and-re-consent, which is how a code minted
+  // under an old, broader consent used to stay redeemable afterwards.
   const grant = await prisma.oAuthGrant.findUnique({
-    where: { clientId_userId: { clientId: row.client.id, userId: row.userId } },
-    select: { id: true, revokedAt: true },
+    where: { id: row.grantId },
+    select: { id: true, epoch: true, scopes: true, revokedAt: true },
   })
   if (!grant || grant.revokedAt) {
     return oauthError("invalid_grant", "This authorization is no longer valid.")
   }
+  if (grant.epoch !== row.grantEpoch) {
+    return oauthError(
+      "invalid_grant",
+      "Consent has changed since this code was issued. Start the authorization again."
+    )
+  }
+  // Defence in depth behind the epoch check: never issue more than the standing
+  // consent currently allows, whatever the code happens to carry.
+  if (!coversScopes(grant.scopes, row.scopes)) {
+    return oauthError("invalid_grant", "This code requests more than the current consent allows.")
+  }
 
   if (!verifyPkce(verifier, row.codeChallenge)) {
     // A verifier that does not match means whoever is holding this code is
@@ -119,23 +143,15 @@ async function exchangeCode(form: URLSearchParams) {
     return oauthError("invalid_grant", "This account is not active.")
   }
 
-  const tokens = await issueAccessToken(
-    grant.id,
-    row.userId,
-    row.scopes,
-    row.client.clientName
-  )
+  const tokens = await issueAccessToken(grant, row.userId, row.scopes, row.client.clientName)
 
-  // A refresh token only exists if offline_access was actually granted.
   let refreshToken: string | undefined
   if (row.scopes.includes(OFFLINE_ACCESS)) {
-    const minted = generateRefreshToken()
-    await storeRefreshToken(grant.id, minted.hash, null)
-    refreshToken = minted.token
+    refreshToken = await issueRefreshToken(grant, 0)
   }
 
   console.log(
-    `[oauth] token issued user=${row.userId} client=${row.client.clientId} grant=${grant.id}`
+    `[oauth] token issued user=${row.userId} client=${row.client.clientId} grant=${grant.id} epoch=${grant.epoch}`
   )
   return NextResponse.json(
     { ...tokens, ...(refreshToken ? { refresh_token: refreshToken } : {}) },
@@ -147,53 +163,55 @@ async function refresh(form: URLSearchParams) {
   const token = form.get("refresh_token")
   if (!token) return oauthError("invalid_request", "refresh_token is required.")
 
-  const hash = sha256(token)
+  const outcome = await consumeRefreshToken(sha256(token))
 
-  const grant = await prisma.oAuthGrant.findFirst({
-    where: { OR: [{ refreshHash: hash }, { refreshPrevHash: hash }] },
-    include: {
+  if (outcome.kind === "unknown") {
+    return oauthError("invalid_grant", "Unknown refresh token.")
+  }
+  if (outcome.kind === "expired") {
+    return oauthError("invalid_grant", "This refresh token has expired.")
+  }
+  if (outcome.kind === "reuse") {
+    // Every generation is retained, so this fires for any member of the family,
+    // not just the one immediately previous. Reuse means either theft or a
+    // client that cannot hold state; both warrant ending the family.
+    await revokeGrant(outcome.grantId)
+    console.warn(`[oauth] refresh token reuse on grant=${outcome.grantId}; family revoked`)
+    return oauthError("invalid_grant", "This refresh token has already been used.")
+  }
+
+  const grant = await prisma.oAuthGrant.findUnique({
+    where: { id: outcome.grantId },
+    select: {
+      id: true,
+      epoch: true,
+      scopes: true,
+      revokedAt: true,
+      userId: true,
       client: { select: { clientId: true, clientName: true } },
       user: { select: { status: true } },
     },
   })
-  if (!grant) return oauthError("invalid_grant", "Unknown refresh token.")
-
-  // Presenting the previous generation means the current one was rotated out
-  // from under this caller - either a replay of a stolen token or a client
-  // holding a stale copy. Either way the safe move is to end the grant and make
-  // everyone re-authorize.
-  if (grant.refreshPrevHash === hash && grant.refreshHash !== hash) {
-    await revokeGrant(grant.id)
-    console.warn(
-      `[oauth] refresh token replay for client=${grant.client.clientId} user=${grant.userId}; grant revoked`
-    )
-    return oauthError("invalid_grant", "This refresh token has already been used.")
+  if (!grant || grant.revokedAt) {
+    return oauthError("invalid_grant", "This authorization was revoked.")
   }
-
-  if (grant.revokedAt) return oauthError("invalid_grant", "This authorization was revoked.")
-  if (grant.refreshExpiresAt && grant.refreshExpiresAt <= new Date()) {
-    return oauthError("invalid_grant", "This refresh token has expired.")
+  // A grant whose standing consent no longer includes offline_access must not
+  // keep minting refresh tokens off a credential issued when it did.
+  if (!grant.scopes.includes(OFFLINE_ACCESS)) {
+    await revokeGrant(grant.id)
+    return oauthError("invalid_grant", "Offline access is no longer granted.")
   }
   if (grant.user.status !== "ACTIVE") {
     return oauthError("invalid_grant", "This account is not active.")
   }
 
   const tokens = await issueAccessToken(
-    grant.id,
+    grant,
     grant.userId,
     grant.scopes,
     grant.client.clientName
   )
+  const next = await issueRefreshToken(grant, outcome.generation + 1)
 
-  // Rotate. OAuth 2.1 requires it for public clients, and keeping the old hash
-  // in refreshPrevHash is what makes the replay check above possible. Done
-  // after the access token is minted so a failure there leaves the caller's
-  // existing refresh token still usable.
-  const next = generateRefreshToken()
-  await storeRefreshToken(grant.id, next.hash, hash)
-
-  return NextResponse.json(
-    { ...tokens, refresh_token: next.token },
-    { headers: NO_STORE }
-  )
+  return NextResponse.json({ ...tokens, refresh_token: next }, { headers: NO_STORE })
 }

src/app/api/recipes/[id]/promote/route.ts

@@ -0,0 +1,53 @@
+import { NextResponse } from "next/server"
+import { z } from "zod"
+import { requireUser } from "@/lib/authz"
+import { promoteRecipe } from "@/lib/recipe-promotion"
+
+/**
+ * Promote a saved recipe into the collection so it can be rated, or onto the
+ * wishlist to try later.
+ *
+ * All the logic lives in src/lib/recipe-promotion.ts, shared with the MCP tool.
+ */
+const bodySchema = z.object({
+  target: z.enum(["drink", "wishlist"]),
+})
+
+export async function POST(
+  request: Request,
+  { params }: { params: { id: string } }
+) {
+  const session = await requireUser()
+  if (session instanceof NextResponse) return session
+
+  const parsed = bodySchema.safeParse(await request.json().catch(() => null))
+  if (!parsed.success) {
+    return NextResponse.json(
+      { error: "target must be \"drink\" or \"wishlist\"" },
+      { status: 400 }
+    )
+  }
+
+  const result = await promoteRecipe(
+    session.user.id,
+    params.id,
+    parsed.data.target
+  )
+
+  if (result.kind === "not_found") {
+    return NextResponse.json({ error: "Not found" }, { status: 404 })
+  }
+
+  if (result.kind === "drink") {
+    return NextResponse.json(
+      { target: "drink", drink: result.drink, existing: result.existing },
+      // 200 rather than 201 when it was already linked - nothing was created.
+      { status: result.existing ? 200 : 201 }
+    )
+  }
+
+  return NextResponse.json(
+    { target: "wishlist", item: result.item, existing: result.existing },
+    { status: result.existing ? 200 : 201 }
+  )
+}

src/app/api/recipes/route.ts

@@ -18,7 +18,9 @@ export async function GET() {
         orderBy: { createdAt: "desc" },
         include: {
           sourceDrink: {
-            select: { name: true, type: true },
+            // id included so the UI can link straight to the drink (and its
+            // rating page) for a recipe that has already been promoted.
+            select: { id: true, name: true, type: true },
           },
         },
       }),

src/app/oauth/authorize/page.tsx

@@ -83,11 +83,13 @@ export default async function AuthorizePage({
   // round of clicking Allow.
   const existing = await prisma.oAuthGrant.findUnique({
     where: { clientId_userId: { clientId: client.id, userId } },
-    select: { id: true, scopes: true, revokedAt: true },
+    select: { id: true, epoch: true, scopes: true, revokedAt: true },
   })
 
   if (existing && !existing.revokedAt && coversScopes(existing.scopes, params.scopes)) {
-    const code = await createAuthCode(client.id, userId, params)
+    // Auto-approval changes no state: same grant, same epoch, narrower-or-equal
+    // scopes. Only an explicit approval advances the epoch.
+    const code = await createAuthCode(client.id, userId, existing, params)
     redirect(buildRedirect(params.redirectUri, { code, state: params.state }))
   }
 
@@ -139,6 +141,7 @@ export default async function AuthorizePage({
 async function createAuthCode(
   clientRowId: string,
   userId: string,
+  grant: { id: string; epoch: number },
   params: {
     redirectUri: string
     scopes: string[]
@@ -152,6 +155,11 @@ async function createAuthCode(
       codeHash: hash,
       clientId: clientRowId,
       userId,
+      // Bound to the grant AND its epoch, so the code dies the moment consent
+      // is revoked or re-granted rather than outliving the decision that
+      // authorized it.
+      grantId: grant.id,
+      grantEpoch: grant.epoch,
       redirectUri: params.redirectUri,
       resource: params.resource,
       scopes: params.scopes,

src/components/bartender/recipe-card.tsx

@@ -1,6 +1,7 @@
 "use client"
 
 import { useState } from "react"
+import Link from "next/link"
 import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card"
 import { Badge } from "@/components/ui/badge"
 import { Button } from "@/components/ui/button"
@@ -13,6 +14,8 @@ import {
   Trash2,
   GlassWater,
   Loader2,
+  Star,
+  Clock,
 } from "lucide-react"
 import { cn } from "@/lib/utils"
 import type { RecipeIngredient } from "@/hooks/use-bartender"
@@ -26,15 +29,20 @@ export interface RecipeCardData {
   glassware?: string | null
   notes?: string | null
   missingCount?: number
-  sourceDrink?: { name: string; type: string } | null
+  sourceDrink?: { id: string; name: string; type: string } | null
 }
 
+export type PromoteTarget = "drink" | "wishlist"
+
 interface RecipeCardProps {
   recipe: RecipeCardData
   onSave?: (recipe: RecipeCardData) => void
   onDelete?: (id: string) => void
+  onPromote?: (id: string, target: PromoteTarget) => void
   isSaving?: boolean
   isDeleting?: boolean
+  /** Which promotion is in flight for this card, if any. */
+  promoting?: PromoteTarget | null
   saved?: boolean
 }
 
@@ -42,8 +50,10 @@ export function RecipeCard({
   recipe,
   onSave,
   onDelete,
+  onPromote,
   isSaving,
   isDeleting,
+  promoting,
   saved,
 }: RecipeCardProps) {
   const [expanded, setExpanded] = useState(false)
@@ -189,6 +199,64 @@ export function RecipeCard({
             <p className="text-sm text-muted-foreground">{recipe.notes}</p>
           </div>
         )}
+
+        {/*
+          A recipe is instructions; a drink is the thing you have an opinion
+          about. Promoting links the two rather than converting, so the recipe
+          stays here and also shows on the drink's page.
+        */}
+        {saved && recipe.id && onPromote && (
+          <div className="flex flex-wrap gap-2 border-t pt-3">
+            {recipe.sourceDrink ? (
+              <Button asChild variant="secondary" size="sm">
+                <Link href={`/rate/${recipe.sourceDrink.id}`}>
+                  <Star className="mr-1.5 h-4 w-4" />
+                  Rate this
+                </Link>
+              </Button>
+            ) : (
+              <Button
+                variant="secondary"
+                size="sm"
+                disabled={!!promoting}
+                onClick={() => onPromote(recipe.id!, "drink")}
+              >
+                {promoting === "drink" ? (
+                  <Loader2 className="mr-1.5 h-4 w-4 animate-spin" />
+                ) : (
+                  <GlassWater className="mr-1.5 h-4 w-4" />
+                )}
+                Add to my drinks
+              </Button>
+            )}
+
+            <Button
+              variant="outline"
+              size="sm"
+              disabled={!!promoting}
+              onClick={() => onPromote(recipe.id!, "wishlist")}
+            >
+              {promoting === "wishlist" ? (
+                <Loader2 className="mr-1.5 h-4 w-4 animate-spin" />
+              ) : (
+                <Clock className="mr-1.5 h-4 w-4" />
+              )}
+              Try later
+            </Button>
+          </div>
+        )}
+
+        {recipe.sourceDrink && (
+          <p className="text-xs text-muted-foreground">
+            In your drinks as{" "}
+            <Link
+              href={`/drinks/${recipe.sourceDrink.id}`}
+              className="text-primary underline-offset-4 hover:underline"
+            >
+              {recipe.sourceDrink.name}
+            </Link>
+          </p>
+        )}
       </CardContent>
     </Card>
   )

src/components/bartender/saved-recipes-tab.tsx

@@ -1,15 +1,27 @@
 "use client"
 
-import { Skeleton } from "@/components/ui/skeleton"
-import { RecipeCard } from "./recipe-card"
-import { useRecipes, useDeleteRecipe } from "@/hooks/use-bartender"
-import { BookOpen } from "lucide-react"
 import { useState } from "react"
+import Link from "next/link"
+import { useRouter } from "next/navigation"
+import { Skeleton } from "@/components/ui/skeleton"
+import { RecipeCard, type PromoteTarget } from "./recipe-card"
+import {
+  useRecipes,
+  useDeleteRecipe,
+  usePromoteRecipe,
+} from "@/hooks/use-bartender"
+import { BookOpen } from "lucide-react"
 
 export function SavedRecipesTab() {
+  const router = useRouter()
   const { data, isLoading, error } = useRecipes()
   const deleteRecipe = useDeleteRecipe()
+  const promoteRecipe = usePromoteRecipe()
   const [deletingId, setDeletingId] = useState<string | null>(null)
+  const [promoting, setPromoting] = useState<{ id: string; target: PromoteTarget } | null>(null)
+  // There is no Toaster mounted in this app, so feedback is inline.
+  const [notice, setNotice] = useState<string | null>(null)
+  const [promoteError, setPromoteError] = useState<string | null>(null)
 
   const recipes = data?.recipes || []
 
@@ -23,6 +35,36 @@ export function SavedRecipesTab() {
     })
   }
 
+  function handlePromote(id: string, target: PromoteTarget) {
+    setNotice(null)
+    setPromoteError(null)
+    setPromoting({ id, target })
+
+    promoteRecipe.mutate(
+      { id, target },
+      {
+        onSuccess: (result) => {
+          if (result.target === "drink" && result.drink) {
+            // Straight to the rating page - being able to rate it is the whole
+            // point of adding it, and the drink is otherwise unrated and easy
+            // to lose track of.
+            router.push(`/rate/${result.drink.id}`)
+            return
+          }
+          setNotice(
+            result.existing
+              ? `${result.item?.name} is already on your wishlist.`
+              : `Added ${result.item?.name} to your wishlist.`
+          )
+        },
+        onError: (err: Error) => {
+          setPromoteError(err.message || "Could not add that. Please try again.")
+        },
+        onSettled: () => setPromoting(null),
+      }
+    )
+  }
+
   if (isLoading) {
     return (
       <div className="grid grid-cols-1 md:grid-cols-2 gap-4">
@@ -58,25 +100,46 @@ export function SavedRecipesTab() {
   }
 
   return (
-    <div className="grid grid-cols-1 md:grid-cols-2 gap-4">
-      {recipes.map((recipe) => (
-        <RecipeCard
-          key={recipe.id}
-          recipe={{
-            id: recipe.id,
-            title: recipe.title,
-            ingredients: recipe.ingredients,
-            steps: recipe.steps,
-            garnish: recipe.garnish,
-            glassware: recipe.glassware,
-            notes: recipe.notes,
-            sourceDrink: recipe.sourceDrink,
-          }}
-          onDelete={handleDelete}
-          isDeleting={deletingId === recipe.id}
-          saved
-        />
-      ))}
+    <div className="space-y-4">
+      {notice && (
+        <div className="rounded-md bg-primary/10 p-3 text-sm">
+          {notice}{" "}
+          <Link
+            href="/wishlist"
+            className="text-primary underline-offset-4 hover:underline"
+          >
+            View wishlist
+          </Link>
+        </div>
+      )}
+      {promoteError && (
+        <div className="rounded-md bg-destructive/10 p-3">
+          <p className="text-sm text-destructive">{promoteError}</p>
+        </div>
+      )}
+
+      <div className="grid grid-cols-1 md:grid-cols-2 gap-4">
+        {recipes.map((recipe) => (
+          <RecipeCard
+            key={recipe.id}
+            recipe={{
+              id: recipe.id,
+              title: recipe.title,
+              ingredients: recipe.ingredients,
+              steps: recipe.steps,
+              garnish: recipe.garnish,
+              glassware: recipe.glassware,
+              notes: recipe.notes,
+              sourceDrink: recipe.sourceDrink,
+            }}
+            onDelete={handleDelete}
+            onPromote={handlePromote}
+            isDeleting={deletingId === recipe.id}
+            promoting={promoting?.id === recipe.id ? promoting.target : null}
+            saved
+          />
+        ))}
+      </div>
     </div>
   )
 }

src/hooks/use-bartender.ts

@@ -21,7 +21,7 @@ export interface Recipe {
   notes: string | null
   createdAt: string
   updatedAt: string
-  sourceDrink?: { name: string; type: string } | null
+  sourceDrink?: { id: string; name: string; type: string } | null
 }
 
 export interface SuggestedCocktail {
@@ -100,3 +100,41 @@ export function useDeleteRecipe() {
     },
   })
 }
+
+export interface PromotionResponse {
+  target: "drink" | "wishlist"
+  drink?: { id: string; name: string }
+  item?: { id: string; name: string }
+  /** True when it was already there - nothing new was created. */
+  existing: boolean
+}
+
+/**
+ * Adds a saved recipe to the collection (so it can be rated) or to the wishlist.
+ * The recipe itself is kept either way; promoting to a drink links the two.
+ */
+export function usePromoteRecipe() {
+  const queryClient = useQueryClient()
+
+  return useMutation({
+    mutationFn: ({
+      id,
+      target,
+    }: {
+      id: string
+      target: "drink" | "wishlist"
+    }): Promise<PromotionResponse> =>
+      fetchWithError(`/api/recipes/${id}/promote`, {
+        method: "POST",
+        headers: { "content-type": "application/json" },
+        body: JSON.stringify({ target }),
+      }),
+    onSuccess: (_data, variables) => {
+      // The recipe list carries the sourceDrink link, so it goes stale too.
+      queryClient.invalidateQueries({ queryKey: ["recipes"] })
+      queryClient.invalidateQueries({
+        queryKey: [variables.target === "drink" ? "drinks" : "wishlist"],
+      })
+    },
+  })
+}

src/lib/mcp/oauth.ts

@@ -126,13 +126,18 @@ export function isRegisterableRedirectUri(value: string): boolean {
 /**
  * Matching a redirect at authorize time.
  *
- * Exact string equality, except for loopback addresses where the port is
- * ignored. This is the single most common cause of a silently broken native
- * client: Claude Code registers http://localhost/callback and
- * http://127.0.0.1/callback, then redirects to an ephemeral port such as
- * http://localhost:51234/callback. RFC 8252 section 7.3 requires the port to be
+ * Exact string equality, except for loopback addresses where the PORT ALONE is
+ * ignored. Claude Code registers http://localhost/callback and then redirects to
+ * an ephemeral port such as http://localhost:51234/callback, so exact matching
+ * would reject every native client. RFC 8252 section 7.3 requires the port to be
  * ignored for the IP-literal form; the same is applied to localhost so Claude
  * Code works at all.
+ *
+ * Everything else must match exactly. An earlier version compared only scheme,
+ * host and path, which silently accepted a request that differed in query,
+ * fragment or userinfo - and a callback that forwards its query is then a route
+ * for delivering the authorization code somewhere the client never registered.
+ * RFC 9700 section 2.1 requires exact matching apart from the loopback port.
  */
 export function redirectUriMatches(registered: string, requested: string): boolean {
   if (registered === requested) return true
@@ -145,11 +150,19 @@ export function redirectUriMatches(registered: string, requested: string): boole
     return false
   }
   if (!isLoopback(a) || !isLoopback(b)) return false
-  return (
-    a.protocol === b.protocol &&
-    a.hostname === b.hostname &&
-    a.pathname === b.pathname
-  )
+
+  // A fragment is never legal on a redirect URI (RFC 6749 3.1.2).
+  if (a.hash || b.hash) return false
+  // Credentials in a redirect target are a spoofing aid and never needed here.
+  if (a.username || a.password || b.username || b.password) return false
+
+  // Normalise away only the port, then require the rest to be identical.
+  const strip = (u: URL) => {
+    const c = new URL(u.toString())
+    c.port = ""
+    return c.toString()
+  }
+  return strip(a) === strip(b)
 }
 
 export function findMatchingRedirectUri(
@@ -187,22 +200,31 @@ export interface IssuedTokens {
   refresh_token?: string
 }
 
+/** Thrown when a grant was revoked or re-consented while a request was in flight. */
+export class GrantStateError extends Error {
+  constructor(message = "This authorization is no longer valid.") {
+    super(message)
+    this.name = "GrantStateError"
+  }
+}
+
 /**
  * Mints an access token against a grant.
  *
- * Refresh tokens are deliberately NOT handled here. The two call sites need
- * different behaviour - the code exchange creates the first one, a refresh
- * rotates the existing one and has to record the previous hash for replay
- * detection - and doing it in here once meant the rotation was silently
- * overwritten and the token handed back matched nothing on file.
+ * Two things this deliberately does NOT do:
  *
- * The access token is an ordinary McpAccessToken row with source "oauth", the
- * same shape a hand-minted one has, so verifyMcpToken needs no OAuth awareness.
- * Previously outstanding tokens on the grant are revoked, so a refresh replaces
- * rather than accumulates.
+ *  - It does not write scopes back to the grant. Token issuance is not consent,
+ *    and treating it as such let a stale authorization code redefine what the
+ *    user had most recently agreed to. The consent endpoint is the only writer.
+ *  - It does not touch refresh tokens. Those have their own lineage.
+ *
+ * The whole write is conditional on the grant still being active AT THE EPOCH
+ * the caller validated. A revoke racing this call makes the guarded update
+ * affect zero rows and the transaction aborts, rather than leaving a live token
+ * hanging off a revoked grant to be resurrected by a later re-consent.
  */
 export async function issueAccessToken(
-  grantId: string,
+  grant: { id: string; epoch: number },
   userId: string,
   scopes: string[],
   clientName: string
@@ -210,8 +232,14 @@ export async function issueAccessToken(
   const access = generateMcpToken()
 
   await prisma.$transaction(async (tx) => {
+    const { count } = await tx.oAuthGrant.updateMany({
+      where: { id: grant.id, epoch: grant.epoch, revokedAt: null },
+      data: { lastUsedAt: new Date() },
+    })
+    if (count !== 1) throw new GrantStateError()
+
     await tx.mcpAccessToken.updateMany({
-      where: { grantId, revokedAt: null },
+      where: { grantId: grant.id, revokedAt: null },
       data: { revokedAt: new Date() },
     })
     await tx.mcpAccessToken.create({
@@ -222,14 +250,10 @@ export async function issueAccessToken(
         name: clientName,
         scopes: accessScopes(scopes),
         source: "oauth",
-        grantId,
+        grantId: grant.id,
         expiresAt: new Date(Date.now() + ACCESS_TOKEN_TTL_MS),
       },
     })
-    await tx.oAuthGrant.update({
-      where: { id: grantId },
-      data: { scopes, lastUsedAt: new Date() },
-    })
   })
 
   return {
@@ -240,23 +264,70 @@ export async function issueAccessToken(
   }
 }
 
-/** Stores a freshly minted refresh token on the grant, keeping the old hash for replay detection. */
-export async function storeRefreshToken(
-  grantId: string,
-  hash: string,
-  previousHash: string | null
-): Promise<void> {
-  await prisma.oAuthGrant.update({
-    where: { id: grantId },
-    data: {
-      refreshHash: hash,
-      refreshPrevHash: previousHash,
-      refreshExpiresAt: new Date(Date.now() + REFRESH_TOKEN_TTL_MS),
-    },
+/**
+ * Mints the next refresh token in a family, guarded on the grant's epoch for the
+ * same reason as above.
+ */
+export async function issueRefreshToken(
+  grant: { id: string; epoch: number },
+  generation: number
+): Promise<string> {
+  const { token, hash } = generateRefreshToken()
+
+  await prisma.$transaction(async (tx) => {
+    const active = await tx.oAuthGrant.count({
+      where: { id: grant.id, epoch: grant.epoch, revokedAt: null },
+    })
+    if (active !== 1) throw new GrantStateError()
+
+    await tx.oAuthRefreshToken.create({
+      data: {
+        grantId: grant.id,
+        tokenHash: hash,
+        generation,
+        expiresAt: new Date(Date.now() + REFRESH_TOKEN_TTL_MS),
+      },
+    })
   })
+
+  return token
 }
 
-/** Kills a grant and every access token it issued. */
+export type RefreshConsumption =
+  | { kind: "unknown" }
+  | { kind: "expired" }
+  | { kind: "reuse"; grantId: string }
+  | { kind: "ok"; grantId: string; generation: number }
+
+/**
+ * Redeems a refresh token exactly once.
+ *
+ * Every generation is retained, so a token from any point in the family's life
+ * is still recognisable as reuse - which is the whole point. The guarded update
+ * on `usedAt` means two concurrent presentations of the same token cannot both
+ * win: the loser is reported as reuse, which is indistinguishable from theft and
+ * treated the same way.
+ */
+export async function consumeRefreshToken(hash: string): Promise<RefreshConsumption> {
+  const row = await prisma.oAuthRefreshToken.findUnique({
+    where: { tokenHash: hash },
+    select: { id: true, grantId: true, generation: true, usedAt: true, expiresAt: true },
+  })
+
+  if (!row) return { kind: "unknown" }
+  if (row.usedAt) return { kind: "reuse", grantId: row.grantId }
+  if (row.expiresAt <= new Date()) return { kind: "expired" }
+
+  const { count } = await prisma.oAuthRefreshToken.updateMany({
+    where: { id: row.id, usedAt: null },
+    data: { usedAt: new Date() },
+  })
+  if (count !== 1) return { kind: "reuse", grantId: row.grantId }
+
+  return { kind: "ok", grantId: row.grantId, generation: row.generation }
+}
+
+/** Kills a grant, every access token it issued, and its whole refresh family. */
 export async function revokeGrant(grantId: string): Promise<void> {
   const now = new Date()
   await prisma.$transaction([
@@ -264,13 +335,13 @@ export async function revokeGrant(grantId: string): Promise<void> {
       where: { grantId, revokedAt: null },
       data: { revokedAt: now },
     }),
+    prisma.oAuthRefreshToken.deleteMany({ where: { grantId } }),
+    // Outstanding codes die too, so one minted before the revoke cannot be
+    // redeemed against a later re-consent.
+    prisma.oAuthAuthCode.deleteMany({ where: { grantId, consumedAt: null } }),
     prisma.oAuthGrant.update({
       where: { id: grantId },
-      data: {
-        revokedAt: now,
-        refreshHash: null,
-        refreshPrevHash: null,
-      },
+      data: { revokedAt: now },
     }),
   ])
 }

src/lib/mcp/schemas.ts

@@ -120,6 +120,20 @@ export const saveRecipeSchema = recipeCreateSchema
 
 export const deleteRecipeSchema = z.object({ id: idField })
 
+/**
+ * One tool with a target rather than two tools. The tool list is already at the
+ * point where selection quality starts to suffer, and "add it to my drinks" and
+ * "add it to try later" are the same gesture with a different destination.
+ */
+export const promoteRecipeSchema = z.object({
+  id: idField,
+  target: z
+    .enum(["drink", "wishlist"])
+    .describe(
+      "\"drink\" adds it to the collection so it can be rated; \"wishlist\" notes it to try later"
+    ),
+})
+
 // ─── Wishlist ────────────────────────────────────────────────────
 
 export const listWishlistSchema = z.object({})

src/lib/mcp/tools/drinks.ts

@@ -371,7 +371,7 @@ export function registerDrinkTools(server: McpServer): void {
       name: "get_collection_stats",
       title: "Collection summary",
       description:
-        "Counts and averages across the whole collection: drinks by type, rating activity, top rated drinks, bar size and recipe count. Prefer this over several list calls when the question is about the collection as a whole.",
+        "Counts and averages across the whole collection: drinks by type, rating activity and top rated drinks, plus bar size and recipe count when the connection also has bar access. Prefer this over several list calls when the question is about the collection as a whole.",
       inputSchema: emptySchema,
       scope: "drinks:read",
       readOnly: true,
@@ -379,6 +379,11 @@ export function registerDrinkTools(server: McpServer): void {
     async (_args, caller) => {
       const userId = caller.userId
       const thirtyDaysAgo = new Date(Date.now() - 30 * 24 * 60 * 60 * 1000)
+      // This tool sits behind drinks:read, but two of its numbers describe the
+      // bar. Returning them regardless leaked across the scope boundary the
+      // consent screen promises, so they are gated on the caller actually
+      // holding bar:read rather than on which tool happened to be called.
+      const canReadBar = caller.scopes.includes("bar:read")
 
       const [byType, drinkCount, ratingAgg, recentRatings, barCount, recipeCount, top] =
         await Promise.all([
@@ -396,8 +401,12 @@ export function registerDrinkTools(server: McpServer): void {
           prisma.rating.count({
             where: { userId, createdAt: { gte: thirtyDaysAgo } },
           }),
-          prisma.barItem.count({ where: { userId, quantity: { not: "EMPTY" } } }),
-          prisma.recipe.count({ where: { userId } }),
+          canReadBar
+            ? prisma.barItem.count({ where: { userId, quantity: { not: "EMPTY" } } })
+            : Promise.resolve(null),
+          canReadBar
+            ? prisma.recipe.count({ where: { userId } })
+            : Promise.resolve(null),
           prisma.rating.findMany({
             where: { userId },
             orderBy: [{ score: "desc" }, { createdAt: "desc" }],
@@ -419,7 +428,9 @@ export function registerDrinkTools(server: McpServer): void {
         typeLines || "  (none)",
         `${ratingAgg._count._all} ratings, average ${avg ? avg.toFixed(2) : "n/a"}`,
         `${recentRatings} ratings in the last 30 days`,
-        `${barCount} bottles in the bar, ${recipeCount} saved recipes`,
+        canReadBar
+          ? `${barCount} bottles in the bar, ${recipeCount} saved recipes`
+          : "",
         top.length ? "top rated:" : "",
         topLines,
       ]
@@ -432,8 +443,7 @@ export function registerDrinkTools(server: McpServer): void {
         ratingCount: ratingAgg._count._all,
         averageScore: avg,
         ratingsLast30Days: recentRatings,
-        barItemCount: barCount,
-        recipeCount,
+        ...(canReadBar ? { barItemCount: barCount, recipeCount } : {}),
         topRated: top.map((r) => ({
           drinkName: r.drink.name,
           type: r.drink.type,

src/lib/mcp/tools/recipes.ts

@@ -10,8 +10,10 @@ import { McpToolError } from "@/lib/mcp/context"
 import {
   deleteRecipeSchema,
   listRecipesSchema,
+  promoteRecipeSchema,
   saveRecipeSchema,
 } from "@/lib/mcp/schemas"
+import { promoteRecipe } from "@/lib/recipe-promotion"
 import { formatRecipeLine, ok } from "@/lib/mcp/render"
 
 type StoredIngredient = { name: string; amount: string; available: boolean }
@@ -132,6 +134,48 @@ export function registerRecipeTools(server: McpServer): void {
     }
   )
 
+  defineTool(
+    server,
+    {
+      name: "promote_recipe",
+      title: "Add a recipe to drinks or the wishlist",
+      description:
+        "Turn a saved recipe into something the user can rate. target=\"drink\" adds it to their collection and links the recipe to it, so the recipe then shows on that drink's page; target=\"wishlist\" notes it to try later. The recipe is kept either way. Safe to call twice - if it is already there, the existing entry is returned rather than a duplicate created.",
+      inputSchema: promoteRecipeSchema,
+      // Creating a drink or wishlist entry is a change to the collection, not
+      // to the bar, so this is gated on drinks:write despite acting on a recipe.
+      scope: "drinks:write",
+    },
+    async ({ id, target }, caller) => {
+      const result = await promoteRecipe(caller.userId, id, target)
+
+      if (result.kind === "not_found") {
+        throw new McpToolError("No recipe with that id.", "not_found")
+      }
+
+      if (result.kind === "drink") {
+        const verb = result.existing ? "was already in" : "added to"
+        return {
+          result: ok(
+            `[${result.drink.id}] ${result.drink.name} ${verb} the collection. Use rate_drink with this id to record what they thought.`,
+            { target: "drink", id: result.drink.id, existing: result.existing }
+          ),
+          recordId: result.drink.id,
+        }
+      }
+
+      const verb = result.existing ? "was already on" : "added to"
+      return {
+        result: ok(`[${result.item.id}] ${result.item.name} ${verb} the wishlist.`, {
+          target: "wishlist",
+          id: result.item.id,
+          existing: result.existing,
+        }),
+        recordId: result.item.id,
+      }
+    }
+  )
+
   defineTool(
     server,
     {

src/lib/recipe-promotion.ts

@@ -0,0 +1,122 @@
+import type { Drink, WishlistItem } from "@prisma/client"
+import { prisma } from "@/lib/prisma"
+
+/**
+ * Turning a saved recipe into something rateable.
+ *
+ * A recipe is instructions; a Drink is the thing you form an opinion about. The
+ * two are related but not the same, so promoting **links** them via
+ * Recipe.sourceDrinkId rather than converting one into the other - the recipe
+ * survives, and it then renders on the drink's page through the relation the
+ * drink detail view already reads.
+ *
+ * This is deliberately shared between the REST route and the MCP tool. The
+ * ownership check, the idempotency rule and the generated description all have
+ * to be identical, and the wishlist promote route is a standing example of that
+ * logic existing in exactly one place and being copied by nobody.
+ */
+
+export type PromotionTarget = "drink" | "wishlist"
+
+export type PromotionResult =
+  | { kind: "not_found" }
+  /** `existing` is true when the recipe was already linked - nothing was created. */
+  | { kind: "drink"; drink: Drink; existing: boolean }
+  | { kind: "wishlist"; item: WishlistItem; existing: boolean }
+
+interface StoredIngredient {
+  name: string
+  amount: string
+  available: boolean
+}
+
+function ingredientsOf(value: unknown): StoredIngredient[] {
+  return Array.isArray(value) ? (value as StoredIngredient[]) : []
+}
+
+/**
+ * A short human description, since a Drink has no ingredients of its own and a
+ * bare title would tell the user nothing on the drinks list a month from now.
+ */
+export function describeRecipe(recipe: {
+  ingredients: unknown
+  garnish: string | null
+  glassware: string | null
+  notes: string | null
+}): string {
+  const names = ingredientsOf(recipe.ingredients).map((i) => i.name).filter(Boolean)
+
+  const parts = [
+    names.length ? `Made with ${names.join(", ")}.` : null,
+    recipe.glassware ? `Served in a ${recipe.glassware}.` : null,
+    recipe.garnish ? `Garnish: ${recipe.garnish}.` : null,
+    recipe.notes,
+  ].filter(Boolean) as string[]
+
+  // description is capped at 2000 by drinkCreateSchema.
+  return parts.join(" ").slice(0, 2000)
+}
+
+export async function promoteRecipe(
+  userId: string,
+  recipeId: string,
+  target: PromotionTarget
+): Promise<PromotionResult> {
+  const recipe = await prisma.recipe.findFirst({
+    where: { id: recipeId, userId },
+  })
+  if (!recipe) return { kind: "not_found" }
+
+  const description = describeRecipe(recipe)
+
+  if (target === "drink") {
+    // Already linked? Hand back the existing drink rather than creating a second
+    // one - promoting twice is a double click, not a request for a duplicate.
+    if (recipe.sourceDrinkId) {
+      const linked = await prisma.drink.findFirst({
+        where: { id: recipe.sourceDrinkId, userId },
+      })
+      if (linked) return { kind: "drink", drink: linked, existing: true }
+    }
+
+    // One transaction: a drink that exists but is not linked back would show no
+    // recipe on its page and would be promoted again on the next attempt.
+    const drink = await prisma.$transaction(async (tx) => {
+      const created = await tx.drink.create({
+        data: {
+          userId,
+          name: recipe.title,
+          type: "COCKTAIL",
+          description: description || null,
+        },
+      })
+      await tx.recipe.update({
+        where: { id: recipe.id },
+        data: { sourceDrinkId: created.id },
+      })
+      return created
+    })
+
+    return { kind: "drink", drink, existing: false }
+  }
+
+  // Wishlist. The recipe is untouched - "try later" is a note to self, not a
+  // change to the recipe itself.
+  const duplicate = await prisma.wishlistItem.findFirst({
+    where: { userId, source: "recipe", name: { equals: recipe.title, mode: "insensitive" } },
+  })
+  if (duplicate) return { kind: "wishlist", item: duplicate, existing: true }
+
+  const item = await prisma.wishlistItem.create({
+    data: {
+      userId,
+      name: recipe.title,
+      type: "COCKTAIL",
+      description: description || null,
+      notes: recipe.notes,
+      source: "recipe",
+    },
+  })
+
+  return { kind: "wishlist", item, existing: false }
+}