Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -450,6 +450,10 @@ The aggregator that mounts every org-scoped sub-router lives at
Org slugs are **immutable** — `ORGANIZATION_UPDATE` rejects slug changes — so URLs remain
stable.

Instance-level routes (no org context, e.g. the deployment-admin surface) use an
underscore-prefixed namespace like `/api/_admin/...`, mounted before the `/api/:org`
catch-all so the static segment wins over the slug param.

## License

Sustainable Use License (SUL):
Expand Down
26 changes: 25 additions & 1 deletion apps/mesh/src/api/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,11 @@ import {
} from "../observability";
import { posthog } from "../posthog";
import authRoutes from "./routes/auth";
import {
ADMIN_API_PREFIX,
createAdminRoutes,
fenceRawAdminSurface,
} from "./routes/admin";
import { createSsoRoutes } from "./routes/org-sso";
import { createDecopilotRoutes } from "./routes/decopilot";
import { createDownstreamTokenRoutes } from "./routes/downstream-token";
Expand Down Expand Up @@ -1226,6 +1231,11 @@ export async function createApp(options: CreateAppOptions = {}) {
// Auth routes (API key management via web UI)
app.route("/api/auth/custom", authRoutes);

// Fence off the raw Better Auth admin plugin (/api/auth/admin/*) from
// external callers — see fenceRawAdminSurface's doc in routes/admin.ts for
// why this must exist whenever deploymentAdminUserIds does.
app.all("/api/auth/admin/*", fenceRawAdminSurface);

// All Better Auth routes (OAuth, session management, etc.)
app.all("/api/auth/*", async (c) => {
return await auth.handler(c.req.raw);
Expand Down Expand Up @@ -1688,7 +1698,11 @@ export async function createApp(options: CreateAppOptions = {}) {
path.startsWith("/api/org-sso/") ||
path.startsWith("/api/auth/") ||
path.startsWith("/api/tools/management") ||
path.startsWith("/oauth-proxy/")
path.startsWith("/oauth-proxy/") ||
// Instance-level operator surface — not governed by any single org's SSO
// policy. Without this, an admin whose active org enforces SSO gets 403'd
// off the whole dashboard, and the UI reads that as "not an admin".
path.startsWith(`${ADMIN_API_PREFIX}/`)
) {
return next();
}
Expand Down Expand Up @@ -2006,6 +2020,16 @@ export async function createApp(options: CreateAppOptions = {}) {
app.post("/api/:org/registry/publish-request", publishRequestHandler);
app.all("/api/:org/registry/*", publicMCPHandler);

// Deployment admin dashboard. Static segment — must register before the
// `:org` catch-all below, same trick as the registry mounts above. That
// registration order is the real no-collision guarantee: ORGANIZATION_CREATE
// rejects slugs outside `^[a-z0-9-]+$`, but the raw better-auth
// organization/create endpoint enforces no charset, so an `_admin`-slugged
// org CAN exist — mounting first means such an org gets shadowed, never the
// admin surface. The `_` prefix just keeps well-behaved slugs from ever
// wanting the name (a bare `admin` is a legal, live slug).
app.route(ADMIN_API_PREFIX, createAdminRoutes());

// New canonical org-scoped API surface — all routes that depend on org context
// live here. Old routes still work (with deprecation logs) until the cleanup
// PR removes them after the deprecation window.
Expand Down
72 changes: 72 additions & 0 deletions apps/mesh/src/api/error-handler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,75 @@ describe("handleApiError", () => {
expect(res.status).toBe(403);
});
});

/** Stand-in for a better-call APIError: an Error whose `name` is "APIError"
* carrying `statusCode` + `body` (matches error.mjs in every bundled copy). */
function apiError(
statusCode: number,
body?: { message?: string; code?: string; cause?: unknown },
): Error {
const err = new Error(body?.message ?? "api error");
err.name = "APIError";
Object.assign(err, { statusCode, body });
return err;
}

describe("handleApiError APIError shape-mapping", () => {
it("forwards a better-call APIError's status, message, and code", async () => {
const res = handleApiError(
apiError(404, {
message: "Organization not found",
code: "ORG_NOT_FOUND",
}),
fakeCtx,
);
expect(res.status).toBe(404);
expect(await res.json()).toEqual({
error: "Organization not found",
code: "ORG_NOT_FOUND",
});
});

it("never forwards body.cause (may hold internals)", async () => {
const res = handleApiError(
apiError(400, { message: "bad", code: "X", cause: "secret-internal" }),
fakeCtx,
);
const json = (await res.json()) as Record<string, unknown>;
expect(JSON.stringify(json)).not.toContain("secret-internal");
expect(json).toEqual({ error: "bad", code: "X" });
});

it("forwards a 5xx APIError's status (does not flatten to 500-generic)", () => {
const res = handleApiError(apiError(503, { message: "down" }), fakeCtx);
expect(res.status).toBe(503);
});

it("falls back to err.message and omits code when body is absent", async () => {
const res = handleApiError(apiError(404), fakeCtx);
expect(res.status).toBe(404);
expect(await res.json()).toEqual({ error: "api error" });
});

it("does NOT match a foreign Error with statusCode but a different name", async () => {
// e.g. the AI SDK's AI_APICallError carries statusCode but is not ours —
// must 500, not relabel an upstream failure as a client-facing 4xx.
const foreign = new Error("Incorrect API key provided");
foreign.name = "AI_APICallError";
Object.assign(foreign, { statusCode: 401 });
const res = handleApiError(foreign, fakeCtx);
expect(res.status).toBe(500);
expect(await res.json()).toEqual({
error: "Internal Server Error",
message: "Incorrect API key provided",
});
});

it("does not match out-of-range or non-numeric statusCodes (falls to 500)", () => {
expect(handleApiError(apiError(399), fakeCtx).status).toBe(500);
expect(handleApiError(apiError(600), fakeCtx).status).toBe(500);
const stringCode = apiError(400);
Object.assign(stringCode, { statusCode: "400" });
expect(handleApiError(stringCode, fakeCtx).status).toBe(500);
});
});
48 changes: 46 additions & 2 deletions apps/mesh/src/api/error-handler.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,29 @@
import type { Context } from "hono";
import { HTTPException } from "hono/http-exception";

/**
* A better-auth / better-call APIError, matched by SHAPE not `instanceof`.
* better-call is present in several copies (better-auth bundles its own), so the
* APIError class thrown by the auth plugins isn't identical to one we'd import —
* `instanceof` silently misses and the error 500s. Every copy's constructor sets
* `this.name = "APIError"` (verified in better-call error.mjs, both 1.x and 2.x),
* so `name === "APIError"` + a numeric 4xx/5xx `statusCode` is the reliable
* cross-copy discriminator. The name check is what keeps this from misfiring on
* foreign errors that merely happen to carry a `statusCode` (e.g. the AI SDK's
* `AI_APICallError`), relabeling an upstream 5xx as our own client-facing 4xx.
*/
function asApiError(
err: unknown,
): { statusCode: number; body?: { message?: string; code?: string } } | null {
if (!(err instanceof Error) || err.name !== "APIError") return null;
const code = (err as { statusCode?: unknown }).statusCode;
if (typeof code !== "number" || code < 400 || code >= 600) return null;
return err as unknown as {
statusCode: number;
body?: { message?: string; code?: string };
};
}

/**
* Global API error handler (wired via `app.onError`).
*
Expand All @@ -9,14 +32,35 @@ import { HTTPException } from "hono/http-exception";
* flattening every error to 500. Before this, the handler 500'd *everything*
* thrown — including HTTPException — so routes like files/file-uploads/
* thread-outputs that `throw new HTTPException(401|404|...)` silently returned
* 500. Anything that isn't an HTTPException is a genuine unexpected failure →
* logged + 500 with the same JSON shape as before.
* 500.
*
* Better Auth `APIError`s likewise carry an intended status + body. Routes that
* call `auth.api.*` in-process (e.g. the deployment-admin addMember on a bad
* orgId) let these propagate; honoring `statusCode` turns them into the right
* 4xx instead of a misleading 500. Only `message`/`code` are forwarded — never
* `body.cause`, which can hold internals.
*
* NOTE this mapping is GLOBAL (app.onError): every route on the API surface
* that lets an APIError bubble now returns better-auth's real status and
* message text to the client, where it previously flattened to an opaque 500.
* That exposure is deliberate — these are user-facing auth errors by design.
*
* Anything else is a genuine unexpected failure → logged + 500.
*/
export function handleApiError(err: unknown, c: Context): Response {
if (err instanceof HTTPException) {
return err.getResponse();
}

const apiErr = asApiError(err);
if (apiErr) {
const body = apiErr.body ?? {};
return Response.json(
{ error: body.message ?? (err as Error).message, code: body.code },
{ status: apiErr.statusCode },
);
}

console.error("Server error :", err);
const message = err instanceof Error ? err.message : "Unknown error";
return c.json({ error: "Internal Server Error", message }, 500);
Expand Down
Loading
Loading