Skip to content

refactor(orpc): shared error mapping, workspace-access gate, /api-internal router - #1068

Open
realcodesiman wants to merge 4 commits into
mainfrom
refactor/orpc-error-mapping-and-gates
Open

refactor(orpc): shared error mapping, workspace-access gate, /api-internal router#1068
realcodesiman wants to merge 4 commits into
mainfrom
refactor/orpc-error-mapping-and-gates

Conversation

@realcodesiman

Copy link
Copy Markdown
Contributor

Summary

  • Consolidates the three oRPC auth-stack exports (authorizedAPI, workspaceTokenAuthAPI, channelApiTokenAPI) onto one shared instanceof-based error mapper, replacing three duplicated error.name === checks. Adds an ActionValidationError → 422 mapping.
  • Wires the owner-quota/trial gate (checkWorkspaceOwnerAccess, from authorize-workspace-access.ts) into workspaceAuthorizedMidddleware so an oRPC mutation can't bypass the same gate a server action already enforces for the identical operation. Mutations only — GET/HEAD/DELETE stay open so an expired workspace keeps its read/delete-only mode (invariant [Settings] General setting page #14).
  • Splits the OpenAPI REST surface: /api/[[...rest]] now serves only publicRouter (workspace-token / channel-token authed procedures) instead of the full session-authed router. A procedure missing from publicRouter now 404s instead of silently answering to a session cookie. A dev-only /api-internal mirrors the old full-router behavior for local Scalar debugging and 404s in production.
  • Wires two previously-unregistered workspace-token API modules (externalWebhooksWorkspaceTokenAPIs, refLinksWorkspaceTokenAPIs) into publicRouter.

⚠️ Branch dependency

This branch is stacked on #1065 (PR-1, rename) and #1066 (PR-2, token hash) — it needs authorize-workspace-access.ts from PR-2 and the api/private.ts naming from PR-1. Merge #1065 and #1066 first, then this PR's diff against main will shrink to just its own ~550 lines. Until then the diff shown here includes their changes too.

Notes for reviewers

  • Important catch during extraction: main had independently evolved orpc.ts and safe-action.ts since the source migration branch diverged — both gained SdkException/toPublicErrorMessage channel-error-message handling from an already-merged, unrelated PR. A naive copy of the migration branch's version of these two files would have silently deleted that feature. Both files were hand-merged to preserve it; orpc-error-mapping.test.ts doesn't exercise the SdkException path (it predates that feature), so I verified it manually — toKnownOrpcError still maps SdkExceptionBAD_REQUEST with the provider's own message.
  • The builder UI is unaffected by the /apipublicRouter restriction: it calls the untouched /rpc/[[...rest]]/route.ts, which still serves the full router. /api is the external-integration/OpenAPI surface (MCP, CLI, third-party tools).
  • apps/builder/__tests__/public-router-boundary.test.ts dynamically scans the filesystem for every features/**/api/workspace-token.ts and fails if any aren't wired into routers/public.ts — this is a regression guard the branch introduces, not something I hand-verified once and hoped stays true.
  • Deliberately did not touch @/features/tags/api/token-auth.ts — the source branch's tags/api/workspace-token.ts replacement depends on @chatbotx.io/api-contract, a package that doesn't exist until PR-4. Renaming/rewriting it here would break the build.

Test plan

  • pnpm lint — clean
  • pnpm --filter builder check-types — clean
  • pnpm --filter builder test — 2028/2028 passing (2020 carried over from PR-1/PR-2 + 8 new)
  • pnpm check:circular — no new circular deps (69 pre-existing, unchanged)
  • invariant-guard agent review — PASS (specifically verified the SdkException merge survived correctly in both files)
  • Manual security review — no findings (verified /api-internal's production gate, the public/private router boundary, and that /api-internal in proxy.ts's allowlist only bypasses the middleware redirect, not per-procedure auth)

…s to api/private.ts

Pure mechanical rename across ~61 feature directories, extracted fresh off
main instead of cherry-picked from the migration branch since many of those
paths also carry unrelated logic changes there. Excludes ads-campaign,
which no longer exists on the source branch.
Adds a tokenHash column and moves workspace bearer-token lookup to hash-first
with a plaintext fallback for the deploy-to-migration gap. The fallback and
the token column removal are deliberately deferred to a follow-up once the
legacy-plaintext warning log has been silent for a release.

Also generalizes channel-api-rate-limit.ts into api-rate-limit.ts (adds a
scope param) and pulls authorize-workspace-access.ts forward as shared
owner-quota/trial-gate infrastructure, since the token-auth middleware needs
both.
…ernal router

Consolidates the three oRPC auth-stack exports onto one instanceof-based error
mapper (was three duplicated error.name checks), adding an ActionValidationError
-> 422 mapping. Wires the owner-quota/trial gate from authorize-workspace-access.ts
into workspaceAuthorizedMidddleware so an oRPC mutation can't bypass the gate a
server action already enforces for the same operation (invariant #14: read/delete
stays open on an expired workspace).

Splits the OpenAPI REST surface so /api/[[...rest]] only ever serves publicRouter
(workspace-token / channel-token authed procedures) instead of the full
session-authed router. A procedure missing from publicRouter now 404s instead of
silently answering to a session cookie. A dev-only /api-internal mirrors the old
full-router behavior for local Scalar debugging and 404s in production; the
builder UI is unaffected since it calls the untouched /rpc route, not /api.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement Refactor or performance improvement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant