feat(api): workspace-governance token scopes for the org-admin tier (#262)#264
Conversation
Add WORKSPACE_SCOPES (workspace:invite, workspace:manage) alongside
OPERATOR_SCOPES in auth-db.ts, with validateScopes(..., { allowOperator,
allowWorkspace }) gating each independently. The token mint route now
resolves the caller's org role in the target workspace (from the
membership lookup already used for the workspace_forbidden check) and
requires admin/owner before accepting any workspace:* scope in a
grant — platform-admin role does not bypass this. parseScopes (file
routes) is unaffected: a governance-scoped token still has zero file
access.
Add workspaceGovernanceAuth(scope) — a D1-backed workspace:*-scoped bearer token guard that rejects revoked/expired/foreign-workspace/file-only/ operator-only tokens — and mount POST /v1/workspaces/:name/invites on it. The route acts as the token's minting_user_id; the auth worker's existing POST /internal/invite already re-checks that user's org role server-side, so no apps/auth changes were needed.
Add GET/DELETE /v1/workspaces/:name/tokens, dual-authed via a session user with org admin/owner role in :name or a workspace:manage-scoped D1 token bound to :name. Mirrors the redacted GET/DELETE /admin/tokens contract, scoped to one workspace's active tokens. Also removes an unused beforeAll import and adds coverage for the workspaceGovernanceAuth guard's no_minting_user 403 branch, both deferred from Task 2.
…262) Review follow-up: document that an up_-shaped bearer is authoritative (no session fallback when invalid) and add a regression test pinning shaped-but-invalid bearer + valid admin session -> 401.
#262) Add workspace:invite/workspace:manage to the exported TokenScope alias, patch changeset scoped to @buildinternet/uploads only, and document the new scopes' minting gate, workspace-boundedness, and zero file-route access alongside the existing operator-scopes docs.
Zach Dunn (zachdunn)
added
the
coderabbit:review
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
uploads-api | 23cb998 | Commit Preview URL Branch Preview URL |
Jul 18 2026, 09:39 PM |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
uploads-auth | 01b1447 | Commit Preview URL Branch Preview URL |
Jul 18 2026, 09:27 PM |
|
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:
📝 WalkthroughWalkthroughWorkspace governance scopes are added to token minting, with workspace-bounded authentication for invites and token management. Routes support invite creation, active-token listing, and revocation, with fail-closed parsing and expanded authorization tests and documentation. ChangesWorkspace governance
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant Client
participant GovernanceAuth
participant TokenStore
participant WorkspaceRoutes
participant AuthService
Client->>GovernanceAuth: Send workspace bearer token
GovernanceAuth->>TokenStore: Load active token and scopes
TokenStore-->>GovernanceAuth: Return workspace-bound governance token
GovernanceAuth->>WorkspaceRoutes: Set minting user context
WorkspaceRoutes->>AuthService: Create workspace invitation
AuthService-->>WorkspaceRoutes: Return invitation data
WorkspaceRoutes-->>Client: Return invitation response
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/workspaces.ts`:
- Around line 417-419: Update the token filter in the active-token listing
around listTokens to exclude expired records using the same expiry condition as
findActiveToken, while retaining the existing revoked_at check. Ensure only
non-revoked, non-expired tokens reach the mapping step.
- Around line 441-445: Validate that body.hashPrefix and body.label are strings
before invoking trim in the workspace route request parsing flow; non-string or
absent values should be treated as invalid or omitted according to the route’s
existing validation behavior, returning a validation error rather than throwing.
Update the selectors derived from the parsed body while preserving the JSON
parse fallback.
- Around line 76-78: Update the authorization logic around authHeader, rawToken,
and workspaceNameFromToken so any Bearer token beginning with “up_” is treated
as authoritative, even when parsing fails; return 401 instead of falling back to
session authentication, while preserving existing behavior for valid tokens and
non-up_ bearer values.
- Around line 437-452: Apply the workspace-keyed writeRateLimit middleware to
the DELETE “/:name/tokens” route alongside workspaceManageAuth, using the
route’s workspace name as the limiter key. Keep the existing selector validation
and revokeToken flow unchanged.
🪄 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: 10524426-e7c3-49fa-8199-cc5114f234ef
📒 Files selected for processing (12)
.changeset/workspace-governance-scope-widening.mdapps/api/src/auth-db.test.tsapps/api/src/auth-db.tsapps/api/src/routes/me.tsapps/api/src/routes/tokens.test.tsapps/api/src/routes/tokens.tsapps/api/src/routes/workspace-governance-invite.test.tsapps/api/src/routes/workspace-governance-tokens.test.tsapps/api/src/routes/workspaces.tsapps/api/src/workspace.tsdocs/admin-tokens.mdpackages/uploads/src/client.ts
| const authHeader = c.req.header("Authorization"); | ||
| const rawToken = authHeader?.startsWith("Bearer ") ? authHeader.slice(7) : undefined; | ||
| if (rawToken && workspaceNameFromToken(rawToken) !== undefined) { |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Treat every up_ bearer as authoritative.
Line 78 falls back to session auth when an explicitly supplied up_ token is malformed and cannot be parsed. A valid cookie could therefore authorize the request instead of returning 401, contrary to the documented fail-closed precedence.
Proposed fix
- if (rawToken && workspaceNameFromToken(rawToken) !== undefined) {
+ if (rawToken?.startsWith("up_")) {Add a valid-session test using a malformed bearer such as up_.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const authHeader = c.req.header("Authorization"); | |
| const rawToken = authHeader?.startsWith("Bearer ") ? authHeader.slice(7) : undefined; | |
| if (rawToken && workspaceNameFromToken(rawToken) !== undefined) { | |
| const authHeader = c.req.header("Authorization"); | |
| const rawToken = authHeader?.startsWith("Bearer ") ? authHeader.slice(7) : undefined; | |
| if (rawToken?.startsWith("up_")) { |
🤖 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/workspaces.ts` around lines 76 - 78, Update the
authorization logic around authHeader, rawToken, and workspaceNameFromToken so
any Bearer token beginning with “up_” is treated as authoritative, even when
parsing fails; return 401 instead of falling back to session authentication,
while preserving existing behavior for valid tokens and non-up_ bearer values.
| const tokens = (await listTokens(c.env.DB, name)) | ||
| .filter((token) => token.revoked_at === null) | ||
| .map((token) => ({ |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Exclude expired tokens from the active-token listing.
Lines 417–419 only filter revoked records, so expired tokens are still returned as active. Apply the same expiry condition used by findActiveToken.
Proposed fix
+ const now = new Date().toISOString();
const tokens = (await listTokens(c.env.DB, name))
- .filter((token) => token.revoked_at === null)
+ .filter(
+ (token) =>
+ token.revoked_at === null &&
+ (token.expires_at === null || token.expires_at > now),
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const tokens = (await listTokens(c.env.DB, name)) | |
| .filter((token) => token.revoked_at === null) | |
| .map((token) => ({ | |
| const now = new Date().toISOString(); | |
| const tokens = (await listTokens(c.env.DB, name)) | |
| .filter( | |
| (token) => | |
| token.revoked_at === null && | |
| (token.expires_at === null || token.expires_at > now), | |
| ) | |
| .map((token) => ({ |
🤖 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/workspaces.ts` around lines 417 - 419, Update the token
filter in the active-token listing around listTokens to exclude expired records
using the same expiry condition as findActiveToken, while retaining the existing
revoked_at check. Ensure only non-revoked, non-expired tokens reach the mapping
step.
| workspaces.delete("/:name/tokens", workspaceManageAuth(), async (c) => { | ||
| const name = c.req.param("name"); | ||
| requireWorkspaceName(name); | ||
|
|
||
| const body = await c.req | ||
| .json<{ hashPrefix?: string; label?: string }>() | ||
| .catch(() => ({}) as { hashPrefix?: string; label?: string }); | ||
| const hashPrefix = body.hashPrefix?.trim(); | ||
| const label = body.label?.trim(); | ||
| if (!hashPrefix && !label) { | ||
| throw new ValidationError("hashPrefix or label required", { | ||
| code: "hash_prefix_or_label_required", | ||
| }); | ||
| } | ||
|
|
||
| const result = await revokeToken(c.env.DB, name, { hashPrefix, label }); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Rate-limit token revocation by workspace.
This mutating endpoint reaches D1 without the workspace write limiter, allowing unbounded selector probes and revoke attempts.
Proposed fix
const name = c.req.param("name");
requireWorkspaceName(name);
+ if (!(await allowWrite(c.env, name))) {
+ throw new RateLimitedError("rate limit exceeded");
+ }As per coding guidelines, “Mutating routes must use the writeRateLimit middleware keyed by workspace.”
🤖 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/workspaces.ts` around lines 437 - 452, Apply the
workspace-keyed writeRateLimit middleware to the DELETE “/:name/tokens” route
alongside workspaceManageAuth, using the route’s workspace name as the limiter
key. Keep the existing selector validation and revokeToken flow unchanged.
Source: Coding guidelines
| const body = await c.req | ||
| .json<{ hashPrefix?: string; label?: string }>() | ||
| .catch(() => ({}) as { hashPrefix?: string; label?: string }); | ||
| const hashPrefix = body.hashPrefix?.trim(); | ||
| const label = body.label?.trim(); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Validate selector types before calling .trim().
JSON is untrusted despite the generic annotation. Values such as { "hashPrefix": 123 } throw at Line 444 and return a 500 instead of a validation error.
Proposed fix
- const hashPrefix = body.hashPrefix?.trim();
- const label = body.label?.trim();
+ const hashPrefix =
+ typeof body.hashPrefix === "string" ? body.hashPrefix.trim() : undefined;
+ const label = typeof body.label === "string" ? body.label.trim() : undefined;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const body = await c.req | |
| .json<{ hashPrefix?: string; label?: string }>() | |
| .catch(() => ({}) as { hashPrefix?: string; label?: string }); | |
| const hashPrefix = body.hashPrefix?.trim(); | |
| const label = body.label?.trim(); | |
| const body = await c.req | |
| .json<{ hashPrefix?: string; label?: string }>() | |
| .catch(() => ({}) as { hashPrefix?: string; label?: string }); | |
| const hashPrefix = | |
| typeof body.hashPrefix === "string" ? body.hashPrefix.trim() : undefined; | |
| const label = typeof body.label === "string" ? body.label.trim() : undefined; |
🤖 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/workspaces.ts` around lines 441 - 445, Validate that
body.hashPrefix and body.label are strings before invoking trim in the workspace
route request parsing flow; non-string or absent values should be treated as
invalid or omitted according to the route’s existing validation behavior,
returning a validation error rather than throwing. Update the selectors derived
from the parsed body while preserving the JSON parse fallback.
Excludes expired-but-unrevoked tokens from self-serve GET /:name/tokens, validates DELETE /:name/tokens body fields as strings before trimming (avoids a 500 on non-string input), treats any up_-shaped bearer as authoritative even when it fails to parse (no session fallback), and adds the missing write rate limiter to the token-revoke route.
1 participant
Implements #262: gives the org-admin tier its own credential — workspace-bounded governance scopes
workspace:inviteandworkspace:manage— completing the scope taxonomy started in #257/#261 (files:*for members,workspace:*for org admins,operator:*for platform operators).What changed
apps/api/src/routes/tokens.ts,auth-db.ts) —POST /v1/tokensaccepts opt-inworkspace:invite/workspace:manageonly when the minting session user holds org roleadmin/ownerin the target workspace; otherwise 400invalid_scopes. Platform-admin/operator status does not bypass the org-role gate (operators already have/admin-ui). Mixed requests (operator:*+workspace:*) must pass both gates independently. Defaults unchanged.workspace.tsworkspaceGovernanceAuth) — unlikeoperator:*(global by design), aworkspace:*token authorizes actions only on the workspace embedded in it; foreign-workspace, revoked, expired, file-only, and operator-only tokens are rejected fail-closed.POST /v1/workspaces/:name/invites(requiresworkspace:invite), mirroring the session invite endpoint. Invites are attributed to the token'sminting_user_id, and the auth worker re-checks that user's current org role at invite time — a token minted by a since-demoted admin can no longer invite. No auth-worker changes were needed (its internal invite route already took an inviter id).GET/DELETE /v1/workspaces/:name/tokens(redacted listing / revoke by hashPrefix or label, mirroring the/admin/tokenscontracts), dual-authed: orgadmin/ownersession or aworkspace:managetoken bound to:name. Closes the gap where only platform operators could list/revoke a workspace's tokens.TokenScopealias widened in the published package (patch changeset targeting only@buildinternet/uploads);docs/admin-tokens.mdgains a "Workspace-governance scopes" section.Deliberate choices
up_-shaped bearer is authoritative — an invalid one is rejected without falling back to a valid session cookie (documented at the decision point and pinned by a regression test).operator:*, any token carrying a governance scope gets no file-route access (file-route scope parsing fails closed) — mint separate tokens per plane.apps/authhas a zero diff — governance scopes never enterOAUTH_SCOPESor any DCR-registerable set.Testing
pnpm testat root: 134 files / 1519 tests passing;tsc --noEmitclean. New coverage includes: org-role mint gating (member/admin/owner/platform-admin-no-bypass), both-gates mixed requests, foreign-workspace rejection, demoted-minter invite rejection, no-minting-user branch, shaped-but-invalid-bearer-with-valid-session composition, revocation flippingrevoked_at, and redacted listing never exposing token values.Closes #262
Summary by CodeRabbit
New Features
Documentation
Bug Fixes