fix(server): return 404 instead of 500 for non-UUID company refs (#9959)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - The board REST API is how operators and integrations read company
state; `GET /api/companies/:companyId` is one of its most basic reads
> - The route passes the raw path param into `companyService.getById`,
which queries the uuid-typed `companies.id` column directly
> - Any non-UUID ref — a slug, a typo, a stale bookmark — makes Postgres
throw `invalid input syntax for type uuid`, which surfaces as an HTTP
500 with a stack trace in the server log instead of a clean client error
> - A 500 for malformed client input is miscategorized: it pages
operators, pollutes error budgets, and hides the actual problem ("that
ref doesn't exist") from the caller
> - This pull request guards `getById` with a UUID check so non-UUID
refs resolve to `null` and the route returns its existing 404 path
> - The benefit is correct HTTP semantics for bad input, quieter logs,
and one less misleading 500 for self-hosters to chase
## Linked Issues or Issue Description
Fixes #9962 — `GET /api/companies/:companyId` returns 500 (`invalid
input syntax for type uuid`) for non-UUID refs instead of 404. Full
repro and log excerpt in the issue.
## What Changed
- `server/src/services/companies.ts`: `getById` returns `null` early for
non-UUID refs instead of passing them to the uuid-typed query.
- `server/src/__tests__/companies-service.test.ts`: regression test —
non-UUID refs (`"tumbly-haus-creative"`, `"not-a-uuid"`, `""`) resolve
to `null` without a query error.
## Verification
- `npx vitest run src/__tests__/companies-service.test.ts` — 12/12 pass
(new test included, embedded-postgres suite).
- Manual: `curl -i /api/companies/not-a-uuid` → 404 (was 500); `curl -i
/api/companies/<real-uuid>` → 200 unchanged.
## Risks
- Low. Pure input-validation guard on one read path; UUID lookups are
byte-for-byte unchanged. Only behavioral shift is 500→404 for refs that
could never have matched a row.
## Model Used
Claude Fable 5 (`claude-fable-5`, Anthropic) via Claude Code — diagnosis
from server logs, patch, and test authored with extended thinking and
tool use; human-reviewed and submitted by @christianlappin.
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links
- [x] My branch name describes the change and contains no internal
ticket id
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes (n/a —
no doc references this error path)
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green (pending first CI run)
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
(pending)
- [x] I will address all Greptile and reviewer comments before
requesting merge
This commit is contained in:
parent
0db8480b19
commit
fc5c6ffed2
|
|
@ -869,4 +869,12 @@ describeEmbeddedPostgres("companyService", () => {
|
|||
details: { agentsPaused: 1, runsCancelled: 1 },
|
||||
});
|
||||
});
|
||||
|
||||
it("getById returns null (not a query error) for non-UUID refs", async () => {
|
||||
const svc = companyService(db);
|
||||
await expect(svc.getById("tumbly-haus-creative")).resolves.toBeNull();
|
||||
await expect(svc.getById("not-a-uuid")).resolves.toBeNull();
|
||||
await expect(svc.getById("")).resolves.toBeNull();
|
||||
});
|
||||
|
||||
});
|
||||
|
|
|
|||
|
|
@ -39,6 +39,8 @@ import { heartbeatService } from "./heartbeat.js";
|
|||
import { logActivity } from "./activity-log.js";
|
||||
import { builtInAgentService } from "./built-in-agents.js";
|
||||
|
||||
|
||||
const UUID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i;
|
||||
export interface CompanyActivityActor {
|
||||
actorType: "user" | "agent" | "system" | "plugin";
|
||||
actorId: string;
|
||||
|
|
@ -259,6 +261,11 @@ export function companyService(db: Db) {
|
|||
},
|
||||
|
||||
getById: async (id: string) => {
|
||||
// Non-UUID refs previously reached the uuid-typed query and threw a
|
||||
// DrizzleQueryError ("invalid input syntax for type uuid"), surfacing
|
||||
// as HTTP 500 from GET /api/companies/:companyId. Treat them as
|
||||
// not-found so the route returns 404.
|
||||
if (!UUID_RE.test(id)) return null;
|
||||
const row = await getCompanyQuery(db)
|
||||
.where(eq(companies.id, id))
|
||||
.then((rows) => rows[0] ?? null);
|
||||
|
|
|
|||
Loading…
Reference in New Issue