Validate required owner/name at tool layer — empty repo arg leaks a bare 404 #37

Closed
opened 2026-06-10 08:45:19 +00:00 by mathias · 1 comment
Owner

Follow-up to #36. The mid-path // guard in the gitea client catches most empty-segment cases, but a call that omits the repo identifier entirely (e.g. repo_get{owner} with no name/repo) builds a trailing empty segment /api/v1/repos/{owner}/ and still leaks gitea's plain 404 page not found instead of a clean CodeValidation.

The client guard cannot blanket-reject trailing slashes — legit roots like /contents/ end that way — so the fix belongs at the tool layer: reject empty required string fields (owner, name, …) in parseArgs/per-tool before calling the client, returning CodeValidation with the missing field name.

Low priority: name is schema-required so a compliant caller never hits this, and the result is now a benign 404, not the misleading swagger-404. Defense-in-depth + clearer error.

Follow-up to #36. The mid-path `//` guard in the gitea client catches most empty-segment cases, but a call that omits the repo identifier *entirely* (e.g. `repo_get{owner}` with no `name`/`repo`) builds a **trailing** empty segment `/api/v1/repos/{owner}/` and still leaks gitea's plain `404 page not found` instead of a clean `CodeValidation`. The client guard cannot blanket-reject trailing slashes — legit roots like `/contents/` end that way — so the fix belongs at the tool layer: reject empty required string fields (`owner`, `name`, …) in `parseArgs`/per-tool before calling the client, returning `CodeValidation` with the missing field name. Low priority: `name` is schema-required so a compliant caller never hits this, and the result is now a benign 404, not the misleading swagger-404. Defense-in-depth + clearer error.
Author
Owner

Done in v0.4.2 (commit on main).

parseArgs now validates, via reflection, that repo/name string args are non-empty and returns a typed ErrValidation naming the field — before the client builds a trailing-empty /api/v1/repos/{owner}/ segment. Optional identifiers opt out with ,omitempty (only code_search's owner-wide fan-out repo today). owner was already enforced by allowlist.Check (returns "owner required" on empty).

Test TestEmptyRepoRejectedAsValidation: empty repo on an identifier tool and empty name on a create tool both return ErrValidation. task check green.

Done in v0.4.2 (commit on `main`). `parseArgs` now validates, via reflection, that `repo`/`name` string args are non-empty and returns a typed `ErrValidation` naming the field — before the client builds a trailing-empty `/api/v1/repos/{owner}/` segment. Optional identifiers opt out with `,omitempty` (only `code_search`'s owner-wide fan-out `repo` today). `owner` was already enforced by `allowlist.Check` (returns "owner required" on empty). Test `TestEmptyRepoRejectedAsValidation`: empty `repo` on an identifier tool and empty `name` on a create tool both return `ErrValidation`. `task check` green.
Sign in to join this conversation.
No labels
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: mathias/gitea-mcp#37