Repository navigation
feat(scopes): publish a machine-readable scope catalogue (#615) - #628
Merged
robertocarlous merged 2 commits intoOct 19, 2026
Merged
Conversation
…urowealth#614) The limits lived only in code: every limiter was constructed inline and mounted by hand, so "what is the budget on POST /api/v1/deposit, and does it apply to a session or an API key?" had no answer that was not an archaeology exercise across three files — and no check that the answer stayed true. This adds the missing single source of truth and makes CI enforce it. - src/middleware/rateLimitTiers.ts — the tier catalogue (window, burst, principal, auth types, env vars) and the route → tier matrix, plus the resolver and the Markdown renderers the doc is generated from. - src/middleware/rateLimiter.ts — every limiter now carries the tier it enforces, and buildRateLimiter requires one, so a limiter the matrix cannot describe is a compile error rather than a silent gap. - src/routes/registry.ts — the route table, extracted from src/index.ts so the check walks the routes the server actually serves instead of a hand-written list. - src/middleware/rateLimiterChain.ts — the app-wide global + tiered chain, named so the check can assert that no route is ever unlimited. - docs/RATE_LIMITING.md — the generated matrix, the auth-type mapping and the fail-open/fail-closed decision. - tests/unit/middleware/rateLimitTierMatrix.test.ts — the CI half: an unmapped route, a matrix that disagrees with the mounted limiter, a matrix entry that no longer matches a real route, an undescribed limiter, or a stale doc each fail the build. The fail-open / fail-closed split is deliberate and now stated in one place. At runtime an unmapped route still gets the global + tiered budgets, so it is never unlimited; in CI a gap is a hard failure, because the matrix is the contract and the contract is enforced. Fixes the erasure limiter's metrics label, which read "admin" and so was indistinguishable from the general admin budget it is deliberately held far below. Closes Neurowealth#614
…#615) The valid scopes were two bare string arrays and the meaning of each scope lived in a comment or a doc line. That left three gaps: - a `requireScope('...')` / `requireAdminScope('...')` call site naming a scope that was not in either list was a runtime 403 on a live route rather than a build failure; - a key-creation UI could not show an operator what a scope actually unlocks, or how much damage it could do; - nothing connected a scope to the endpoints it gated, so the scope tables in docs/API_KEYS.md were hand-maintained and had already drifted (they were missing `portfolio:write`, `loans:read` and `loans:write` entirely). This adds the catalogue and makes CI enforce it. - src/auth/scopeCatalog.ts — the shared entry shape: description, risk level, enforcement status, implied scopes, guardrail notes, and the Markdown renderer the doc is generated from. - src/auth/scopes.ts — the user key catalogue. Typed as a total map over USER_SCOPES, so a new scope cannot be added without describing it. - src/middleware/adminAuth.ts — the admin key catalogue, same shape. - src/auth/scopeCoverage.ts — derives the endpoints each scope covers by reading the scope tag off each route's middleware chain, so the "endpoints covered" list cannot drift from the route table. - GET /api/admin/keys/scopes — serves the catalogue for the key-creation UI, with the endpoints each scope actually unlocks. - docs/API_KEYS.md — the generated scope tables, the enforcement model and the endpoint's contract. - tests/unit/auth/scopeCatalog.test.ts — the CI half. ## The enforcement model The catalogue says whether a scope actually gates anything, because a scope that gates nothing is a promise the platform does not keep: enforced — a route requires it. Granting it is meaningful. declared-only — declared and reservable, but no route requires it today. `super` and `agent` also carry an `implies` list, and `hasAdminScope` is now driven by that same data, so the runtime behaviour and the published catalogue cannot drift apart. The check verifies the catalogue's `implies` list against `hasAdminScope` directly. ## A hole this surfaced, and closes `portfolio:read`, `transactions:read` and `vault:read` were declared and granted to new keys by default, but NO route required them. The portfolio, transaction and vault read endpoints used `requireAuth` alone, and `requireAuth` accepts API keys — so a key scoped to nothing but `alerts:manage` could read the user's entire portfolio, transaction history, tax report and vault balance. This PR enforces them. Sessions are unaffected (`authScopes` is `*` for a session); only API keys that lack the read scope are now refused, with the usual 403 `insufficient_scope`. **This is a behaviour change.** An existing API key that was created without the read scopes will lose read access to those endpoints. New keys are unaffected — the default read set is unchanged and the key-creation UI should offer the read scopes alongside the write ones. ## Acceptance criteria - [x] Catalog exported — machine-readable, with name, description, risk level and the endpoints covered - [x] Admin API `GET /api/admin/keys/scopes` for the key-creation UX - [x] Every `requireScope` / `requireAdminScope` call site references a catalog entry — enforced in CI - [x] Docs list all scopes with descriptions — generated tables in docs/API_KEYS.md Closes Neurowealth#615
|
@johnsmccain Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #615
Closes #616
Closes #614
Closes #617
Context
The valid scopes were two bare string arrays (
USER_SCOPES,ADMIN_SCOPES) and the meaning of each scope lived in a comment or a doc line. That left three gaps:requireScope('...')/requireAdminScope('...')call site naming a scope that was in neither list was a runtime 403 on a live route rather than a build failure.docs/API_KEYS.mdwere hand-maintained and had already drifted — they were missingportfolio:write,loans:readandloans:writeentirely.Proposal
GET /api/admin/keys/scopesfor the key-creation UX.requireScope/requireAdminScopecall site references a catalogue entry — no ad-hoc strings.What is in the PR
src/auth/scopeCatalog.tssrc/auth/scopes.tsUSER_SCOPESso a new scope cannot be added without describing itsrc/middleware/adminAuth.tssrc/auth/scopeCoverage.tssrc/routes/admin.tsGET /api/admin/keys/scopes— serves the catalogue for the key-creation UIdocs/API_KEYS.mdtests/unit/auth/scopeCatalog.test.tsThe enforcement model
The catalogue says whether a scope actually gates anything, because a scope that gates nothing is a promise the platform does not keep:
enforceddeclared-onlysuperandagentalso carry animplieslist, andhasAdminScopeis now driven by that sameADMIN_GRANTOR_SCOPESdata — so the runtime behaviour and the published catalogue cannot drift apart. The check verifies the catalogue'simplieslist againsthasAdminScopedirectly.Five admin scopes turn out to be
declared-only(wallet,erasure:read,risk-metadata:read,risk-metadata:write,admin:impersonate): they are declared and reservable, but no route requires them today. The catalogue says so explicitly rather than implying they restrict something.A hole this surfaced, and closes
portfolio:read,transactions:readandvault:readwere declared and granted to new keys by default, but no route required them. The portfolio, transaction and vault read endpoints usedrequireAuthalone — andrequireAuthaccepts API keys.So a key scoped to nothing but
alerts:managecould read the user's entire portfolio, transaction history, tax report and vault balance.This PR enforces them:
GET /api/v1/portfolio/:userIdand itssuggestions/history/earnings/tax-reportvariants →portfolio:readGET /api/v1/transactions/:userId,/detail/:txHash,/:id/events→transactions:readGET /api/v1/vault/balance→vault:readThis is a behaviour change. Sessions are unaffected (
authScopesis*for a session). An existing API key created without the read scopes will now get a 403insufficient_scopeon those endpoints. New keys are unaffected — the default read set is unchanged, and the key-creation UI should offer the read scopes alongside the write ones.Acceptance criteria
GET /api/admin/keys/scopesfor the key-creation UXrequireScope/requireAdminScopecall site references a catalogue entry — enforced in CI, no ad-hoc stringsdocs/API_KEYS.mdTesting
tests/unit/auth/scopeCatalog.test.ts, plus 7 behavioural cases intests/unit/middleware/apiKeyAuth.test.tscovering the newly-enforced read scopestsc --noEmitandeslintcleanmainare unchanged by this PR (baseline onmain: 9 failed suites / 64 failed tests; on this branch: 9 failed / 57 failed, and 2618 passing vs 2564)