Skip to content

The field routes ignore the signup id in their URL, so a request naming one signup can change another's field #554

Closed

Description

@richshaw

What happens

PATCH and DELETE /api/signups/[id]/fields/[fieldId] (src/app/api/signups/[id]/fields/[fieldId]/route.ts) never read [id]. They call updateField(db, actor, fieldId, body) and deleteField(db, actor, fieldId) in src/services/slot-fields.ts, which load the field by its id alone and then lock and change the field's own signup.

So PATCH /api/signups/<B>/fields/<a field of A> edits signup A's field and answers 200.

This is hardening, not a hole. The policy guard runs on the field's own workspace, so the caller must already be able to write to A. A field from another workspace is refused with 403. The builder always builds the URL from the same signup's field list (useBuildState), so no screen sends a mismatched pair. But a nested route whose parent id means nothing is the shape that turns into a real hole if a guard ever moves, and it's the only nested route under src/app/api/signups/[id]/ with a child id.

Fix, in outline

  • Give updateField and deleteField an optional expectedSignupId.
  • Straight after the field is read, if expectedSignupId is set and differs from the field's signupId, return not_found (the same answer as a field that doesn't exist, so the response doesn't say the field lives elsewhere).
  • Pass params.id from both route handlers.
  • The MCP tools (src/mcp/tools/fields.ts) take only a field id by design and keep calling without it.

Watch for

Done when

  • A db test in src/services/slot-fields.db.test.ts (or a route test): updating and deleting signup A's field with signup B's id (same workspace) returns not_found, and A's field is unchanged.
  • Existing field tests and the MCP field tools still pass unchanged.
  • pnpm lint && pnpm typecheck && pnpm knip && pnpm test && pnpm test:db pass.

Found by the pipeline's security scout in src/app/api/signups/[id]/fields/[fieldId] and src/services/slot-fields.ts, at 15deb24. Checked by an independent trace of the route, services and every caller; no upstream check compares the two ids. https://claude.ai/code/session_01HCHmgfKauTAjkwmocXZQAR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions