Fix registration token API

[lunny]

Another possible resolution is to remove GET method.

[Zettat123]

Overall LGTM. However, I would prefer to remove the GET method because:

• it's not a real GET method
• GH doesn't support GET and only POST is allowed

[Copilot]

Pull request overview

This PR tightens authorization for Actions runner registration token API endpoints, ensuring that retrieving a registration token via GET requires write token scopes (since the operation can mint/refresh tokens).

Changes:

• Refactors token-scope middleware to share logic via checkTokenScopes and introduces tokenRequiresScopesLevel(...).
• Updates runner registration-token GET routes (admin/user/org/repo contexts) to require write scope categories.
• Extends integration tests to assert GET .../registration-token is forbidden when using read-only scoped tokens.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
tests/integration/api_actions_runner_test.go	Adds integration coverage that read-scoped tokens are rejected for GET .../registration-token endpoints.
routers/api/v1/api.go	Introduces tokenRequiresScopesLevel and applies it to GET .../registration-token routes to require write scopes.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

addActionsRoutes now takes requiredScopeCategories as a variadic argument, but checkTokenScopes becomes a no-op when the slice is empty. That makes it easy for a future call site to omit the category and accidentally reintroduce the original scope-bypass on GET .../registration-token. Consider making this a required (non-variadic) parameter for addActionsRoutes, or defensively failing fast when it’s empty for endpoints that must enforce scopes.

Suggested change

	requiredScopeCategories ...auth_model.AccessTokenScopeCategory,
	requiredScopeCategories []auth_model.AccessTokenScopeCategory,

[Copilot]

The PR description mentions removing the GET method as a resolution, but the route is still present (now with stricter scope enforcement). If the intended fix is to eliminate GET entirely, this should be removed/disabled (and related swagger/docs/tests updated) rather than kept.

[Copilot]

GET .../registration-token still maps to shared.GetRegistrationToken, which can create a new runner token when none exists or the latest is inactive. Even with the write-scope requirement, using GET for a potentially state-changing operation is problematic (cache/prefetch/logging semantics) and differs from GitHub’s POST-only behavior. Consider making GET strictly read-only (never creating) or returning 405 and requiring POST to create tokens.

Suggested change

	m.Get("/registration-token", reqToken(), reqChecker, tokenRequiresScopesLevel(auth_model.Write, requiredScopeCategories...), act.GetRegistrationToken)
	m.Get("/registration-token", reqToken(), reqChecker, tokenRequiresScopesLevel(auth_model.Write, requiredScopeCategories...), func(ctx *context.APIContext) {
	// For security and HTTP semantics reasons, registration token creation
	// must use POST. GET is not allowed to perform this state-changing action.
	ctx.Status(http.StatusMethodNotAllowed)
	})

[Copilot]

The new assertions cover that read-scoped tokens are forbidden for the GET registration-token endpoint, but there’s no corresponding positive test that a write-scoped token can still successfully GET a registration token. Adding a success-path assertion here (or in one of the other contexts) would help catch route/middleware ordering mistakes that could inadvertently block the endpoint entirely.