Skip to content

fix: require an explicit workspace for admin credential issuance#520

Merged
Zach Dunn (zachdunn) merged 1 commit into
mainfrom
fix/admin-routes-require-explicit-workspace
Jul 24, 2026
Merged

fix: require an explicit workspace for admin credential issuance#520
Zach Dunn (zachdunn) merged 1 commit into
mainfrom
fix/admin-routes-require-explicit-workspace

Conversation

@zachdunn

Copy link
Copy Markdown
Member

Closes the one path that could put unrelated users in the same workspace without anyone choosing it.

The problem

Four ADMIN_TOKEN-gated routes defaulted an omitted workspace to the communal default workspace, and uploads admin invite create hardcoded the same fallback:

Surface Old behavior when workspace was omitted
POST /admin/tokens minted a token for default
POST /admin/enrollments created an invite granting access to default
GET /admin/tokens listed default's tokens
DELETE /admin/tokens revoked against default
uploads admin invite create --workspace defaulted to default
/console invite form field prefilled default, and blank fell back to it

So an operator inviting someone "to their workspace" and forgetting the field handed out files:read/files:write on the shared tenant instead of getting an error.

What makes this worse than a mis-scoped grant: enrollment redemption mints a token without creating an org membership (redeemEnrollment, apps/api/src/auth-db.ts). The recipient could read and write default's objects while appearing in no member list and in no account UI — invisible co-tenancy.

The DELETE case fails the other way: intending to revoke in workspace X, omitting it, and revoking in default instead — or believing you revoked something when you hadn't.

The fix

workspace is required everywhere. Omitted or blank is now 400 workspace_required; malformed slugs keep the distinct invalid_workspace code, so "you forgot the field" doesn't read as "you typo'd the slug". Naming default explicitly still works — this gates omission, not the value.

The CLI change matters independently: because it hardcoded the string "default" rather than reading the shared constant, it would have kept sending default explicitly and sailed past the API guard.

Production state

Checked before changing anything (aggregates only):

  • default has one member — the operator.
  • Every other workspace has one member, except the team org.
  • Zero pending invites, zero enrollment rows.

No accidental co-tenancy exists today. This closes the path before it gets used rather than cleaning up after it — which is also why there's no migration or backfill here.

Scope

Deliberately narrow. This does not retire the communal workspace, move objects, change any public URL, or touch the communal flag / member-cap exemption / reserved slug. Those are the rest of the de-privileging cleanup discussed in #505.

Verification

  • pnpm test — 3115/3115 (240 files)
  • pnpm typecheck — clean
  • Changed files format-clean under oxfmt

New apps/api/src/routes/admin-workspace-required.test.ts pins the rule per route (omitted and blank, both codes) so the default can't be reintroduced one handler at a time, plus a CLI test asserting the guard fires before any network call.

Six tests in routes-auth.test.ts relied on the old default while testing scopes and email behavior; each now passes an explicit workspace. One of them (label too long) would otherwise have kept passing for the wrong reason — workspace_required fires before label validation — so it needed the fix to keep testing what it claims to.

Refs #505.

The four ADMIN_TOKEN-gated token/enrollment routes defaulted an omitted
`workspace` to the communal `default` workspace, and `uploads admin invite
create` hardcoded the same fallback. A forgotten field therefore issued a
credential against the shared tenant instead of erroring.

That is the accidental-co-tenancy path: enrollment redemption mints a token
without creating an org membership (`redeemEnrollment`), so the recipient got
files:read/files:write on `default`'s objects while appearing in no member
list and in no account UI.

- apps/api: `POST/GET/DELETE /admin/tokens` and `POST /admin/enrollments` now
  400 `workspace_required` when the field is absent or blank. Malformed slugs
  keep the distinct `invalid_workspace` code so "you forgot it" doesn't read
  as "you typo'd it". Naming `default` explicitly still works.
- CLI: `uploads admin invite create` requires `--workspace`.
- /console: the workspace field no longer prefills `default`, and the invite
  handler refuses to submit a blank one rather than falling back.
- Docs: admin-tokens.md documented the old default; ops.md's example now uses
  a named workspace.

Prod state at time of change: no accidental co-tenancy exists — `default` has
one member (the operator), every other workspace has one member except the
team org, and there are zero pending invites and zero enrollment rows. This
closes the path before it is used, rather than cleaning up after it.

Verification: pnpm test 3115/3115, pnpm typecheck clean, changed files
format-clean.

Refs #505
@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are limited based on label configuration.

🏷️ Required labels (at least one) (2)
  • coderabbit:review
  • review
🚫 Excluded labels (none allowed) (1)
  • wip

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: eaf6c10a-33de-4843-b0d7-ee80d260c30a

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/admin-routes-require-explicit-workspace

Comment @coderabbitai help to get the list of available commands.

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
uploads-auth fbfd0cc Commit Preview URL

Branch Preview URL
Jul 24 2026, 07:10 PM

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
uploads-api fbfd0cc Commit Preview URL

Branch Preview URL
Jul 24 2026, 07:11 PM

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
uploads-web fbfd0cc Commit Preview URL

Branch Preview URL
Jul 24 2026, 07:11 PM

@zachdunn
Zach Dunn (zachdunn) merged commit 2a5a28e into main Jul 24, 2026
6 checks passed
@zachdunn
Zach Dunn (zachdunn) deleted the fix/admin-routes-require-explicit-workspace branch July 24, 2026 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant