Skip to content

fix(handler): validate skill id UUID at request boundary#3025

Open
tomqiaozc wants to merge 1 commit into
multica-ai:mainfrom
tomqiaozc:security/skill-loader-uuid-validation
Open

fix(handler): validate skill id UUID at request boundary#3025
tomqiaozc wants to merge 1 commit into
multica-ai:mainfrom
tomqiaozc:security/skill-loader-uuid-validation

Conversation

@tomqiaozc
Copy link
Copy Markdown
Contributor

Summary

Fixes #3024.

loadSkillForUser was passing the raw chi.URLParam("id") into parseUUID, the panic-on-invalid helper reserved for trusted UUID round-trips. A malformed /api/skills/{notuuid} panicked in util.MustParseUUID; chi's middleware.Recoverer turned the panic into a 500 instead of a 400.

This violates CLAUDE.mdBackend Handler UUID Parsing Convention: pure-UUID request inputs must use parseUUIDOrBadRequest. Switching to it makes the behavior:

  • Valid UUID: unchanged.
  • Malformed UUID: 400 Bad Request with {"error":"invalid skill id"} and no panic log.

Test plan

  • New TestGetSkill_MalformedUUIDReturns400 asserts GET /api/skills/not-a-uuid returns 400.
  • Existing TestGetSkill_IncludesContent and TestListSkills_OmitsContent still pass (valid UUID path unchanged).
  • make test (reviewer-side; my local environment lacks Go).

loadSkillForUser was passing chi.URLParam(r, "id") directly into
parseUUID, the panic-on-invalid helper reserved for trusted UUID
round-trips. A malformed `/api/skills/{notuuid}` request panicked
in util.MustParseUUID; chi's middleware.Recoverer turned it into a
500 instead of a 400.

This violates the documented convention (CLAUDE.md → "Backend Handler
UUID Parsing Convention"): pure-UUID request inputs must use
parseUUIDOrBadRequest, which writes a 400 and short-circuits.

Switch loadSkillForUser to parseUUIDOrBadRequest. Behaviour for valid
UUIDs is unchanged; malformed input now returns 400 with a clear
"invalid skill id" message.

Test:
- TestGetSkill_MalformedUUIDReturns400 asserts GET /api/skills/not-a-uuid
  returns 400.
@vercel
Copy link
Copy Markdown

vercel Bot commented May 21, 2026

@tomqiaozc is attempting to deploy a commit to the IndexLabs Team on Vercel.

A member of the Team first needs to authorize it.

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.

Handler convention: loadSkillForUser panics on malformed skill id (500 instead of 400)

1 participant