Skip to content

Fix public-only token enforcement for user/org APIs and notifications#37118

Open
lunny wants to merge 4 commits intogo-gitea:mainfrom
lunny:lunny/fix_public_only_list_org
Open

Fix public-only token enforcement for user/org APIs and notifications#37118
lunny wants to merge 4 commits intogo-gitea:mainfrom
lunny:lunny/fix_public_only_list_org

Conversation

@lunny
Copy link
Copy Markdown
Member

@lunny lunny commented Apr 6, 2026

  • Ensure public-only scoped tokens never return private orgs, repos, activity feeds, or repo-by-id details by enforcing visibility in handlers and routes.
  • Add a notifications middleware to block public-only tokens at the router level.
  • Add integration tests covering public-only behavior for user/org repos, repo-by-id access, activity feeds, stars, and notifications.

Generated by a coding agent with Codex 5.2

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 6, 2026
@github-actions github-actions bot added modifies/api This PR adds API routes or modifies them modifies/go Pull requests that update Go code labels Apr 6, 2026
@lunny lunny marked this pull request as ready for review April 6, 2026 19:39
@silverwind
Copy link
Copy Markdown
Member

silverwind commented Apr 7, 2026

A few points worth addressing:

  • The scattered if ctx.PublicOnly { private = false } checks across 6+ handlers are fragile and easy to miss when adding new endpoints. A cleaner approach would be to push PublicOnly into the query layer: add a PublicOnly bool field to the search/find option structs (SearchRepoOptions, FindOrgOptions, StarredReposOptions, WatchedReposOptions, etc.), set it once from ctx.PublicOnly, and let the database queries handle the exclusion. For entity-level checks (like GetByID), a centralized tokenCanAccessRepo(ctx, repo) called from repoAssignment() would also reduce the surface area for missed checks.

  • rejectPublicOnly() hardcodes "token scope is limited to public notifications" — should use a generic message like "this endpoint is not available for public-only tokens" or accept a parameter.

  • The explanatory comment about notifications is placed after the middleware registration — should go before it.

  • TestAPIActivityFeedsPublicOnly asserts Empty(t, activities), which only works because all test fixture activity happens to be on private repos. If someone adds public activity for user2/org3, the test would still pass but no longer actually test the filtering. Better to assert that no returned activity references a private repo.

The switchfor fix and the notification rejection approach are solid as-is.


This comment was written with the help of Claude.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. modifies/api This PR adds API routes or modifies them modifies/go Pull requests that update Go code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants