feat(api): scoped admin/operator token minting for CLI admin operations (#257)#261
Merged
Merged
Conversation
…ting (#257) Adds ADMIN_SCOPES/isAdminScope and an allowAdmin option to validateScopes, fixes requireAdminUser's multi-role rejection bug via a shared userHasAdminRole helper, and wires POST /v1/tokens to accept admin:* scopes only when the minting session user holds the admin role (400 invalid_scopes otherwise). Default mint scopes are unchanged.
adminAuth now falls through from the static ADMIN_TOKEN check (unchanged, still fail-closed when unset) to a D1 auth_tokens lookup for bearer tokens shaped like up_<workspace>_<random>. admin:write is a read+write superset; admin:read only covers GET/HEAD. Scoped-token auth works even without ADMIN_TOKEN configured, since the fail-closed rule is specific to the static secret.
Allow packages/uploads client.ts scope unions to accept admin:read / admin:write alongside file scopes so CLI/SDK callers can request operator scopes minted server-side; no new commands or flags. Adds patch changeset for the published package.
… scopes (#257) Operator admin scopes grant global /admin/* authority regardless of the workspace a token is minted against; docs now say so explicitly. Adds a routes-files.test.ts regression proving a token stored with ["files:read","admin:write"] gets no file-route access at all (parseScopes fails the whole array when it contains a non-file scope, so the admin scope doesn't silently downgrade into extra file permissions).
Zach Dunn (zachdunn)
added
the
coderabbit:review
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdmin-scoped workspace tokens can be minted by admin-role users, stored with existing token records, and used on ChangesAdmin operator token flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Admin
participant TokenAPI
participant AuthDB
participant AdminAPI
participant D1
Admin->>TokenAPI: POST /v1/tokens with admin scope
TokenAPI->>AuthDB: validate scopes using admin role
AuthDB-->>TokenAPI: validated admin-scoped token
TokenAPI-->>Admin: workspace token
Admin->>AdminAPI: /admin/* with bearer token
AdminAPI->>D1: find active token and scopes
D1-->>AdminAPI: active token record
AdminAPI-->>Admin: authorize read/write request
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
uploads-api | 4b6336f | Commit Preview URL Branch Preview URL |
Jul 18 2026, 08:51 PM |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
uploads-auth | 4b6336f | Commit Preview URL Branch Preview URL |
Jul 18 2026, 08:51 PM |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/uploads/src/client.ts (1)
247-247: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract a shared scope type alias to avoid repeating the 5-element union.
The literal union
"files:read" | "files:write" | "files:delete" | "admin:read" | "admin:write"is duplicated across four locations. A single type alias would prevent drift if scopes change again.♻️ Proposed refactor
+export type TokenScope = "files:read" | "files:write" | "files:delete" | "admin:read" | "admin:write"; + export interface EnrollmentExchangeResult { apiUrl?: string; workspace: string; token: string; - scopes?: Array<"files:read" | "files:write" | "files:delete" | "admin:read" | "admin:write">; + scopes?: TokenScope[]; expiresAt?: string; }Then apply the same substitution at lines 294, 489, and 533.
Also applies to: 294-294, 489-489, 533-533
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/uploads/src/client.ts` at line 247, Extract the repeated five-value scope union in client.ts into a shared type alias, then replace the inline unions at the visible scopes declaration and the corresponding declarations near lines 294, 489, and 533 with that alias. Use the alias consistently so future scope changes require updating only one definition.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/api/src/routes/admin-scoped-token.test.ts`:
- Around line 38-44: Update appWith so an explicitly provided undefined
adminToken remains undefined instead of being replaced by ADMIN_TOKEN; use a
presence-aware defaulting approach while retaining ADMIN_TOKEN only when the
option is omitted. Ensure the unset-secret test exercises an absent ADMIN_TOKEN
binding.
---
Nitpick comments:
In `@packages/uploads/src/client.ts`:
- Line 247: Extract the repeated five-value scope union in client.ts into a
shared type alias, then replace the inline unions at the visible scopes
declaration and the corresponding declarations near lines 294, 489, and 533 with
that alias. Use the alias consistently so future scope changes require updating
only one definition.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6f4e7537-c68f-4739-9279-3cd0937996ee
📒 Files selected for processing (12)
.changeset/admin-scope-widening.mdapps/api/src/admin.tsapps/api/src/auth-db.tsapps/api/src/index.tsapps/api/src/routes/admin-scoped-token.test.tsapps/api/src/routes/tokens.test.tsapps/api/src/routes/tokens.tsapps/api/src/session-auth.test.tsapps/api/src/session-auth.tsapps/api/test/routes-files.test.tsdocs/admin-tokens.mdpackages/uploads/src/client.ts
Comment on lines
+38
to
+44
| function appWith(opts: { adminToken?: string | undefined; db: SqliteD1 }) { | ||
| const { adminToken = ADMIN_TOKEN, db } = opts; | ||
| const app = new Hono<{ Bindings: Env }>() | ||
| .route("/admin", admin) | ||
| .onError((err, c) => respondError(c, err)); | ||
| const env = { | ||
| ADMIN_TOKEN: adminToken, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Actually remove ADMIN_TOKEN in the unset-secret test.
Line 39’s destructuring default converts the explicit undefined at Line 156 back to ADMIN_TOKEN, so this test never exercises the unset binding.
Proposed fix
function appWith(opts: { adminToken?: string | undefined; db: SqliteD1 }) {
- const { adminToken = ADMIN_TOKEN, db } = opts;
+ const { db } = opts;
+ const adminToken = Object.hasOwn(opts, "adminToken") ? opts.adminToken : ADMIN_TOKEN;
const app = new Hono<{ Bindings: Env }>()
.route("/admin", admin)
.onError((err, c) => respondError(c, err));
const env = {
- ADMIN_TOKEN: adminToken,
+ ...(adminToken === undefined ? {} : { ADMIN_TOKEN: adminToken }),
REGISTRY: fakeKv({ "ws:acme": RECORD }),
DB: database(db),
} as unknown as Env;Also applies to: 150-158
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/api/src/routes/admin-scoped-token.test.ts` around lines 38 - 44, Update
appWith so an explicitly provided undefined adminToken remains undefined instead
of being replaced by ADMIN_TOKEN; use a presence-aware defaulting approach while
retaining ADMIN_TOKEN only when the option is omitted. Ensure the unset-secret
test exercises an absent ADMIN_TOKEN binding.
Avoid collision with the weaker workspace-admin org role; these scopes express platform-wide operator authority. Renames ADMIN_SCOPES/AdminScope/ isAdminScope/allowAdmin to their Operator equivalents across apps/api, packages/uploads, docs, and the changeset.
… TokenScope type (#257) - appWith() now distinguishes an explicitly-passed undefined adminToken from an omitted one, so the "no ADMIN_TOKEN set" test truly exercises an absent binding instead of falling back to the default token. - Extract the repeated five-value scope union in client.ts into a single exported TokenScope type used at all four call sites.
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
1 participant
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.
Implements #257: admins can mint workspace tokens carrying opt-in operator scopes (
operator:read,operator:write) viaPOST /v1/tokens, and/admin/*accepts those tokens alongside the staticADMIN_TOKEN— giving per-token revocation, labels, and attribution (minting_user_id) instead of only the shared break-glass secret.What changed
apps/api/src/auth-db.ts— newOPERATOR_SCOPES(operator:read,operator:write),isOperatorScope, and avalidateScopes(..., { allowOperator })overload. All existing callers (/admin/tokens, enrollments, admin-ui invite links) stay file-scopes-only.apps/api/src/routes/tokens.ts— mint requests that include admin scopes require the session user to hold the better-authadminrole; non-admins get 400invalid_scopes. Default mint scopes are unchanged (files:read,files:write) — admin scopes are never implicit.apps/api/src/session-auth.ts— newuserHasAdminRoleusing better-auth comma-split multi-role semantics;requireAdminUsernow uses it (fixes a pre-existing bug where a"admin,support"role was rejected).apps/api/src/admin.ts—adminAuthnow accepts D1-backed operator tokens after the timing-safeADMIN_TOKENcheck:operator:readgrants GET/HEAD,operator:writegrants everything (superset). Revoked/expired/files-only/legacy-null-scope tokens are rejected fail-closed.ADMIN_TOKENbehavior is unchanged and remains break-glass; scoped tokens also work whenADMIN_TOKENis unset.packages/uploads— client scope unions widened to allow the new scopes (no new commands/flags); patch changeset included.docs/admin-tokens.md— operator-scopes section, including two sharp edges: operator authority is global across/admin/*(the minted workspace only anchors storage/revocation), and a token carrying an admin scope gets no file-route access (file-route scope parsing fails closed on any non-file scope) — mint separate tokens.Guardrails (per issue)
apps/authis completely untouched — nothing here lands inOAUTH_SCOPES, so DCR-registered MCP clients cannot request admin scopes./admin/*is bearer-token-only and deliberately untouched" boundary noted inapps/api/src/index.ts; the comment there is updated.Testing
pnpm testat repo root: 131 files / 1477 tests passing (includes new coverage: role gating incl. multi-role, default-mint safety, read/write scope mapping on/admin/*, revoked/expired/files-only rejection, no-ADMIN_TOKENoperation, and a regression pinning mixed-scope tokens to zero file-route access).Closes #257