Repository navigation
fix(api): answer 400 or 404, not 500, for a repo claim or key with a NUL byte - #792
Conversation
A repo claim with a NUL byte answered 500 on PostgreSQL for every repository route, because PostgreSQL refuses such text. SQLite stored it. The verifier now refuses such a claim, as it already refuses an iss with a NUL. _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_ Change-Id: I62ffb423af933aa2a01f855f593e69f7996bae59 Signed-off-by: Thomas Kosiewski <tk@coder.com>
A path name such as /api/repos/a%00b reached PostgreSQL and answered 500. No repository can have such a key, so ResolveRepoKey answers ErrNotFound without a store call, on every backend. _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_ Change-Id: Ic6e1c02e480c4f801a0fe56d73185fc83c7bd56d Signed-off-by: Thomas Kosiewski <tk@coder.com>
Move the NUL and UTF-8 guard from ResolveRepoKey into the PostgreSQL GetRepoByURL and GetRepoByID. SQLite and S3-WAL can hold such a url from before the verifier refused NUL claims, so they keep resolving it. _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_ Change-Id: I8094fe392a757991b0bcea3523b8b1bb160b5b3e Signed-off-by: Thomas Kosiewski <tk@coder.com>
…legacy rows _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_ Change-Id: Ibfaf7345017de243987b93541ce228737d1b4547 Signed-off-by: Thomas Kosiewski <tk@coder.com>
|
@codex review |
|
@codex security review |
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
… the legacy note _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_ Change-Id: I03de578b58b843f4745bd88a41b727e300f1110a Signed-off-by: Thomas Kosiewski <tk@coder.com>
The verifier refusal locked out repositories that SQLite and S3-WAL had stored with such a url, and broke their upstream sync (the loopback token carries the url). Refuse the claim only where it would name a new repository, and answer not found on PostgreSQL for lookups and delete. _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_ Change-Id: I419eb5f0f6d7bfad44f18414b9031160ad72c4b1 Signed-off-by: Thomas Kosiewski <tk@coder.com>
|
@codex review |
|
@codex security review |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
…tabase access _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_ Change-Id: Ib423276f53400246bee1713f592c15ba02dd92e8 Signed-off-by: Thomas Kosiewski <tk@coder.com>
…or valid unusual claims _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_ Change-Id: I6956696b39f508fb439b0287a74e40acfac74e44 Signed-off-by: Thomas Kosiewski <tk@coder.com>
|
@codex review |
|
@codex security review |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
Why
A request whose repository name or id held a NUL byte answered 500 on PostgreSQL. PostgreSQL refuses a NUL byte or invalid UTF-8 in a text parameter (SQLSTATE 22021). Two inputs carried such text into a repository lookup or create:
repoclaim with a NUL. On PostgreSQL,POST /api/v1/repos,GET /api/v1/repo,GET /api/v1/repos/branchesandDELETE /api/v1/repos/deleteeach answered 500. SQLite created such a repository (201).GET /api/repos/a%00borGET /api/repo-urls/a%00b. PostgreSQL answered 500 and SQLite 404.A claim-checking lane found the first while #790 (issue #788) was in review. That PR's advisor found the second.
What changes
POST /repos(both paths, every body form) answers 400bad_request"invalid repo claim (want UTF-8 text without NUL)" when therepoclaim is notmeta.ValidText. The claim becomes the new repository's url, so no backend creates a repository with such a url. The check is inforkAwareCreateRepo, which every create form passes through. JSON decoding already makes a claim valid UTF-8, so in practice this refuses a NUL.GetRepoByURL,GetRepoByIDandSoftDeleteRepoanswerErrNotFoundfor a key thatmeta.ValidTextrefuses, before any query: PostgreSQL holds no such row. Every route that looks a repository up by the claim or by a path name or id then answers its existing 404 on REST instead of 500. The git surface already answered 403 (claim mismatch) and still does.meta.ValidText(new,internal/meta/text.go): valid UTF-8 with no NUL.POST /reposwith that claim changes: it answered by the row's state (200 for a finished repository, 409, 503 or 404 otherwise) and now answers 400.docs/reference/authentication.mdstates the create rule. Theinternal/apiandinternal/metaREADMEs state the create rule and the PostgreSQL rule. Theinternal/authREADME drops a stalesub == SubjectLiteralinvariant: any non-emptysubverifies since fix(auth): accept any non-empty sub and treat a token without repo as org-wide #666.Other request text with a NUL byte (webhook and git-credential fields and ids, cursors, sync bodies) is #793.
Verification
TestRepoClaimWithNUL(PostgreSQL and SQLite): create answers 400 "invalid repo claim (want UTF-8 text without NUL)" on/api/v1/repos,/api/reposand with a fork body and a copy body (the same detail, so the check runs before the fork dispatch). A claim with control characters,éand U+FFFD still creates (201).GET /repo,GET /repos/branchesandDELETE /repos/deleteanswer 404. On SQLite, a repository stored with such a url still answers 200 toGET /repoand to delete.TestAliasNotFoundgainsGET /api/repos/a%00banda%FFb(404).TestRepoURLByIDgainsGET /api/repo-urls/a%00banda%FFb(404).TestResolveRepoKeygainsteam/a\x00bandteam/a\xffb(ErrNotFound on every backend), and resolves urls with control characters,éand U+FFFD on every backend.TestPGRepoLookupsRefuseInvalidTextWithoutQuerycalls the three PostgreSQL methods on aPGwith no pool, so any database access panics: such keys answer ErrNotFound.SoftDeleteRepoguard, delete answers 500 on PostgreSQL. Without theGetRepoByURLorGetRepoByIDguard,TestResolveRepoKey/pgfails with SQLSTATE 22021, andTestAliasNotFound/pg(both) andTestRepoURLByID/pg(id) fail. With theGetRepoByURLguard afterlookupOrgID, even before its error check, that test panics. AValidTextthat also refuses\x01oréfailsTestResolveRepoKey/pg.go vet,scripts/check-doc-links.py(broken links: 0) and both--fingerprints-onlychecks pass.Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:high