Skip to content

Add route authorization manifest and CI check (#598) - #1208

Open
ptone wants to merge 5 commits into
scion/ci-linter-frameworkfrom
scion/ci-linter-route-authz
Open

Add route authorization manifest and CI check (#598)#1208
ptone wants to merge 5 commits into
scion/ci-linter-frameworkfrom
scion/ci-linter-route-authz

Conversation

@ptone

@ptone ptone commented Aug 22, 2026

Copy link
Copy Markdown
Owner

Summary

Closes #598. Part of #1193 (CI custom linter framework, Phase 3).

Every route registered in registerRoutes() must now appear in a manifest declaring its authorization posture. This closes the gap identified in the authz-guards check (hack/check-authz-guards.sh lines 55-63): a handler with no authorization at all was invisible to any lexical rule because there was no guard to key on.

Deliverables

  • pkg/hub/route_authz_manifest.go — Go map covering all 147 routes with their verified authorization posture (public, auth-flow, authenticated, admin, workstation, broker-hmac, webhook, agent-token, oidc-public)
  • hack/check-route-authz-manifest.sh — security-grade lint script (exit 3 on missing tools, not exit 0) with --self-test mode
  • Makefilecheck-route-authz-manifest target added as dependency of check-custom
  • CI — dedicated workflow step with distinct ::error title= annotation
  • hack/check-project-compat-literals.sh — allowlist entry for manifest file (references /api/v1/groves/* paths as registered routes)

Authorization posture verification

Each route's posture was verified by reading the handler implementation, not just the registration pattern:

  • Routes in isUnauthenticatedEndpoint() (auth.go:426) confirmed as auth-flow/public/webhook
  • Admin handlers confirmed to check user.Role() != "admin"
  • Workstation handlers wrapped with requireWorkstation() middleware
  • Broker handlers check GetBrokerIdentityFromContext()
  • Agent-token handlers check GetAgentFromContext()
  • Notable: /metrics, /api/v1/settings/public, /api/v1/auth/me, /api/v1/auth/tokens[/], /api/v1/auth/invite/redeem require auth despite names/comments suggesting otherwise — classified as authenticated
  • /api/v1/metrics/ and /api/v1/admin/metrics-dashboard intentionally open to all authenticated users (not admin-only)

Test plan

  • ./hack/check-route-authz-manifest.sh --self-test passes
  • ./hack/check-route-authz-manifest.sh reports 0 violations (all 147 routes covered)
  • Removing a manifest entry and running the script catches the gap (exit 1)
  • make check-custom passes
  • make ci passes

Scion Agent (cl-dev-3) added 5 commits August 22, 2026 13:07
Every route registered in registerRoutes() must now appear in
pkg/hub/route_authz_manifest.go declaring its authorization posture
(public, auth-flow, authenticated, admin, workstation, broker-hmac,
webhook, agent-token, oidc-public). This closes the gap identified in
the authz-guards check (hack/check-authz-guards.sh lines 55-63):
a handler with no authorization at all was invisible to any lexical rule.

Deliverables:
- pkg/hub/route_authz_manifest.go: manifest covering all 147 routes
- hack/check-route-authz-manifest.sh: security-grade lint with --self-test
- Makefile: check-route-authz-manifest target, added to check-custom
- CI: dedicated workflow step with distinct error annotations
- hack/check-project-compat-literals.sh: allowlist entry for manifest file

Closes #598.
Part of #1193 (CI custom linter framework).
- CI step: match authz-guards precedent with explanatory comment and
  detailed NOTHING WAS ANALYSED annotation for the * case
- Replace grep -P (GNU Perl regex) with POSIX sed in all 4 extraction
  sites (self-test and main script) for macOS compatibility
- Add lint-only artifact comment to routeAuthzManifest var declaration
Add nolint:unused directive to routeAuthzManifest var in
route_authz_manifest.go — the var is a lint-only artifact consumed by
hack/check-route-authz-manifest.sh, not Go code.

Add shellcheck disable=SC2001 before two sed 's/^/  /' lines in
check-route-authz-manifest.sh — prepending to each line of a multiline
string requires sed; bash parameter expansion cannot do this.
gofmt requires an empty comment line between a doc comment block and
a //nolint directive. Without it, gofmt treats the nolint line as part
of the doc comment and reformats it, causing CI to fail.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant