fix: enforce public-only token scope and harden push options / locale parsing#38323
Draft
bircni wants to merge 4 commits into
Draft
fix: enforce public-only token scope and harden push options / locale parsing#38323bircni wants to merge 4 commits into
bircni wants to merge 4 commits into
Conversation
The Locale middleware runs on every unauthenticated request and passed the raw Accept-Language header to language.ParseAcceptLanguage. That parser has quadratic-time behavior on long malformed inputs; its guard only counts "-" separators while the scanner aliases "_" to "-", so a single unauthenticated request with a large "_"-separated header burns seconds of CPU. Bound the header length before parsing (only the leading languages are used anyway).
…endpoints
A public-only access token could still read private data through routes whose
handlers did not apply the restriction:
- GET /api/v1/teams/{id}/repos and .../repos/{org}/{repo} returned private team
repository metadata (the doer's own access was checked, but not the token's
public-only scope).
- GET /api/v1/teams/{id}/activities/feeds returned private activity entries.
- GET /api/v1/users/{username}/orgs/{org}/permissions disclosed membership
permissions for a non-public org.
Filter repos via TokenCanAccessRepo, apply ApplyPublicOnly to the team feed, and
reject public-only access to non-public org permissions.
…-to-create The post-receive hook applied the repo.private and repo.template git push options to any existing repository, letting a repo owner or admin collaborator silently toggle visibility with a bare "git push -o repo.private=..." while bypassing the audit trail, webhook, and notifications a settings change fires. These options are only meant to set the initial state during push-to-create, so skip them once the repository is populated.
GiteaBot
added
the
lgtm/need 2
lafriks
approved these changes
Jul 5, 2026
GiteaBot
added
lgtm/need 1
wxiaoguang
reviewed
Jul 5, 2026
| } | ||
|
|
||
| // FIXME: these options are not quite right, for example: changing visibility should do more works than just setting the is_private flag | ||
| // These options should only be used for "push-to-create" |
wxiaoguang
reviewed
Jul 5, 2026
Comment on lines
165
to
185
Contributor
There was a problem hiding this comment.
If "push-to-create" only, do these "if" blocks still make sense?
wxiaoguang
reviewed
Jul 5, 2026
Comment on lines
165
to
185
Contributor
There was a problem hiding this comment.
If "push-to-create" only, do these "if" blocks still make sense?
wxiaoguang
reviewed
Jul 5, 2026
Comment on lines
+124
to
+125
| var repos []api.Repository | ||
| DecodeJSON(t, resp, &repos) |
Contributor
There was a problem hiding this comment.
Can AI learn correct code pattern?
wxiaoguang
reviewed
Jul 5, 2026
Comment on lines
+155
to
+157
| defer func() { | ||
| require.NoError(t, repo_service.DeleteRepositoryDirectly(t.Context(), repo.ID)) | ||
| }() |
wxiaoguang
reviewed
Jul 5, 2026
Comment on lines
+163
to
+167
| // These options are only meant for "push-to-create": setting the initial | ||
| // visibility/template flags on a repository created by the very first push. | ||
| // On an already-populated repository a bare "git push -o repo.private=..." | ||
| // would silently flip visibility without an audit entry, webhook, or | ||
| // notification, so restrict the options to the push-to-create case. |
Contributor
There was a problem hiding this comment.
The comment seems not quite right.
It not only works for "push-to-create", it also works for all empty repo?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three independent security hardening fixes, each with a regression test:
Localemiddleware passed the rawAccept-Languageheader toParseAcceptLanguage, whose guard only counts-while the scanner aliases_to-— a large_-separated header on an unauthenticated request burned CPU. The header is now length-bounded before parsing.GET /teams/{id}/repos,.../repos/{org}/{repo},/teams/{id}/activities/feeds, and/users/{username}/orgs/{org}/permissionsstill returned private repo/activity/permission data to a public-only token. They now filter viaTokenCanAccessRepo/ApplyPublicOnlyand reject non-public org permissions.repo.private/repo.templatepush options were applied to any existing repo, letting an owner/admin silently flip visibility bypassing audit, webhooks, and notifications. They are now honored only on push-to-create.