Found by the skills#52 gatekeeper review (round 2), verified against gitea-mcp 0e2bd3d and a scratch Gitea 1.27.1 (same version as git.d-ma.be).
1. Case-variant duplicate keys override silently
Arguments are decoded with Go encoding/json, which matches keys case-insensitively, and the last one wins. normalizeAliases usually re-marshals with sorted keys, which puts lowercase last. It skips that step when nothing changes, and passing name alongside repo makes nothing change.
Repro: file_write_branch with {"owner":"probe","repo":"t","name":"t","path":"case8.txt","content":"x","message":"m","branch":"feat/y8","Branch":"main"}. Result: OK branch=main, and the file is on main.
Impact: any client-side check that reads the lowercase key, such as the skills mcp-destructive-guard hook (infra ADR-0027), sees a different value from the one the server acts on. The hook now asks on case-variant duplicates, but the server should not accept ambiguous input in the first place.
Fix: in parseArgs, reject any object with two keys that are equal under strings.EqualFold, and reject keys whose case doesn't match the schema exactly. Test with the repro above.
After that, every API write to main returns 500 (dst refspec refs/heads/main matches more than one) until someone deletes the ref by hand. A single tool call can block writes to a deploy branch.
Fix: validate branch names. Reject a refs/ prefix, .., whitespace and a trailing /, using the same shape the hook now enforces: ^[A-Za-z0-9][A-Za-z0-9._/-]*$.
Done when
Both repros return a validation error, each with a test.
Found by the skills#52 gatekeeper review (round 2), verified against gitea-mcp `0e2bd3d` and a scratch Gitea 1.27.1 (same version as git.d-ma.be).
## 1. Case-variant duplicate keys override silently
Arguments are decoded with Go `encoding/json`, which matches keys **case-insensitively, and the last one wins**. `normalizeAliases` usually re-marshals with sorted keys, which puts lowercase last. It skips that step when nothing changes, and passing `name` alongside `repo` makes nothing change.
Repro: `file_write_branch` with `{"owner":"probe","repo":"t","name":"t","path":"case8.txt","content":"x","message":"m","branch":"feat/y8","Branch":"main"}`. Result: `OK branch=main`, and the file is on main.
Impact: any client-side check that reads the lowercase key, such as the skills `mcp-destructive-guard` hook (infra ADR-0027), sees a different value from the one the server acts on. The hook now asks on case-variant duplicates, but the server should not accept ambiguous input in the first place.
**Fix:** in `parseArgs`, reject any object with two keys that are equal under `strings.EqualFold`, and reject keys whose case doesn't match the schema exactly. Test with the repro above.
## 2. `branch: "refs/heads/main"` creates `refs/heads/refs/heads/main`
After that, every API write to main returns 500 (`dst refspec refs/heads/main matches more than one`) until someone deletes the ref by hand. A single tool call can block writes to a deploy branch.
**Fix:** validate branch names. Reject a `refs/` prefix, `..`, whitespace and a trailing `/`, using the same shape the hook now enforces: `^[A-Za-z0-9][A-Za-z0-9._/-]*$`.
## Done when
Both repros return a validation error, each with a test.
---
_Created via git-mcp on behalf of @mathias_
Both repros now return ErrValidation, and no request reaches Gitea:
{"branch":"feat/y8","Branch":"main", ...} is refused.
branch: "refs/heads/main" is refused. The same rule covers .., whitespace, a trailing / or ., //, /., .lock, HEAD and a leading - or .. It applies to file_write_branch.branch and to tbd_ship.branch and tbd_ship.base.
The test is internal/tools/strict_args_test.go.
Gatekeeper, 3 rounds (verdicts are on the PR: #issuecomment-15184, -15232, -15322):
Round 1, BLOCKED. A "z":1e999 decoy key ended the scan early, so the #65 repro still wrote to main. Fixed with UseNumber, and the scan now fails closed on input it can't parse.
Round 1, also fixed. The fold used ToLower; it now matches strings.EqualFold, so long-s and the Kelvin sign are caught.
Round 2. The pairwise duplicate check was quadratic: 7s of CPU on 90k keys. It now uses a unicode.SimpleFold orbit key. The reviewer brute-forced 75.6M pairs against EqualFold with 0 mismatches.
Round 3. Approved and merged.
CI and deploy. This repo has no PR CI: the workflow runs only on push to main and on tags. So the pre-merge gate was a local task check (exit 0), and the merge went ahead on that, on my instruction.
## Shipped in v0.16.2 (PR #66, merge dc127a0)
**Both repros now return ErrValidation, and no request reaches Gitea:**
- `{"branch":"feat/y8","Branch":"main", ...}` is refused.
- `branch: "refs/heads/main"` is refused. The same rule covers `..`, whitespace, a trailing `/` or `.`, `//`, `/.`, `.lock`, `HEAD` and a leading `-` or `.`. It applies to `file_write_branch.branch` and to `tbd_ship.branch` and `tbd_ship.base`.
The test is `internal/tools/strict_args_test.go`.
**Gatekeeper, 3 rounds** (verdicts are on the PR: #issuecomment-15184, -15232, -15322):
- **Round 1, BLOCKED.** A `"z":1e999` decoy key ended the scan early, so the #65 repro still wrote to main. Fixed with `UseNumber`, and the scan now fails closed on input it can't parse.
- **Round 1, also fixed.** The fold used ToLower; it now matches `strings.EqualFold`, so long-s and the Kelvin sign are caught.
- **Round 2.** The pairwise duplicate check was quadratic: 7s of CPU on 90k keys. It now uses a `unicode.SimpleFold` orbit key. The reviewer brute-forced 75.6M pairs against EqualFold with 0 mismatches.
- **Round 3.** Approved and merged.
**CI and deploy.** This repo has no PR CI: the workflow runs only on push to main and on tags. So the pre-merge gate was a local `task check` (exit 0), and the merge went ahead on that, on my instruction.
- Post-merge run 9316 on dc127a0: **success**.
- The deploy runs `localhost:5000/gitea-mcp:dc127a0`; the pod is Running and Ready.
- Tag `v0.16.2` is pushed.
**Follow-up:** #67 covers tag names in tag_create and release_create, plus the remaining ref arguments.
---
_Created via git-mcp on behalf of @mathias_
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Found by the skills#52 gatekeeper review (round 2), verified against gitea-mcp
0e2bd3dand a scratch Gitea 1.27.1 (same version as git.d-ma.be).1. Case-variant duplicate keys override silently
Arguments are decoded with Go
encoding/json, which matches keys case-insensitively, and the last one wins.normalizeAliasesusually re-marshals with sorted keys, which puts lowercase last. It skips that step when nothing changes, and passingnamealongsiderepomakes nothing change.Repro:
file_write_branchwith{"owner":"probe","repo":"t","name":"t","path":"case8.txt","content":"x","message":"m","branch":"feat/y8","Branch":"main"}. Result:OK branch=main, and the file is on main.Impact: any client-side check that reads the lowercase key, such as the skills
mcp-destructive-guardhook (infra ADR-0027), sees a different value from the one the server acts on. The hook now asks on case-variant duplicates, but the server should not accept ambiguous input in the first place.Fix: in
parseArgs, reject any object with two keys that are equal understrings.EqualFold, and reject keys whose case doesn't match the schema exactly. Test with the repro above.2.
branch: "refs/heads/main"createsrefs/heads/refs/heads/mainAfter that, every API write to main returns 500 (
dst refspec refs/heads/main matches more than one) until someone deletes the ref by hand. A single tool call can block writes to a deploy branch.Fix: validate branch names. Reject a
refs/prefix,.., whitespace and a trailing/, using the same shape the hook now enforces:^[A-Za-z0-9][A-Za-z0-9._/-]*$.Done when
Both repros return a validation error, each with a test.
Created via git-mcp on behalf of @mathias
Shipped in v0.16.2 (PR #66, merge
dc127a0)Both repros now return ErrValidation, and no request reaches Gitea:
{"branch":"feat/y8","Branch":"main", ...}is refused.branch: "refs/heads/main"is refused. The same rule covers.., whitespace, a trailing/or.,//,/.,.lock,HEADand a leading-or.. It applies tofile_write_branch.branchand totbd_ship.branchandtbd_ship.base.The test is
internal/tools/strict_args_test.go.Gatekeeper, 3 rounds (verdicts are on the PR: #issuecomment-15184, -15232, -15322):
"z":1e999decoy key ended the scan early, so the #65 repro still wrote to main. Fixed withUseNumber, and the scan now fails closed on input it can't parse.strings.EqualFold, so long-s and the Kelvin sign are caught.unicode.SimpleFoldorbit key. The reviewer brute-forced 75.6M pairs against EqualFold with 0 mismatches.CI and deploy. This repo has no PR CI: the workflow runs only on push to main and on tags. So the pre-merge gate was a local
task check(exit 0), and the merge went ahead on that, on my instruction.dc127a0: success.localhost:5000/gitea-mcp:dc127a0; the pod is Running and Ready.v0.16.2is pushed.Follow-up: #67 covers tag names in tag_create and release_create, plus the remaining ref arguments.
Created via git-mcp on behalf of @mathias