feat(auth): one permission vocabulary, key scopes as bundles, /me permissions, and API-key scope enforcement (#16270, #16040)

[mrveiss]

Closes #16270
Refs #16040. This PR delivers AC2, AC5 and AC7, AC4's mechanism (require_key_permission) and AC6's test of the decision. AC4's routes that declare a scope, and AC6's route-level test, land with #16294, because no route accepts keys until then.

Thinking Path

• One change, two issues. API key scopes are stored and displayed but never enforced — keys inherit full user authority #16040's AC2 intersects a key's scopes with its owner's permissions, and that cannot be computed until scopes and Permission share one vocabulary. auth: extend Permission to cover chat, teams and webhooks, define key scopes as permission bundles, and return effective permissions from /me #16270 builds that vocabulary. The standing batching goal therefore puts both in one PR.
• Owner rulings, recorded on auth: extend Permission to cover chat, teams and webhooks, define key scopes as permission bundles, and return effective permissions from /me #16270 and API key scopes are stored and displayed but never enforced — keys inherit full user authority #16040:
  • Chat, teams and webhooks get dedicated members, rather than migration 062's fold into api.* / admin.users.*.
  • The four flagged mappings adopt 062's choices. settings:* maps to admin.config.* and users:* to admin.users.*. audit:write and admin:organization are frontend-only strings with no key scope; tech-debt(frontend): usePermissions' permission strings share nothing with the backend's Permission vocabulary #16243 retires them.
  • Teams are admin-only.
  • For API keys, this PR builds the mechanism and a test route. No route accepts a key today; feat(auth): decide which routes accept API keys, and wire X-API-Key through the #16040 mechanism #16294 decides which will.
  • The extraction rides in this same PR.
  • Keys created before enforcement get a 90-day grace period.
• The ratchets decided two shapes.
  • permissions.py was frozen at 631 lines. Its seeding builders moved to a pure permission_seed.py, which imports nothing back, so there is no cycle. The file is now 595 lines and off the ratchet list.
  • api/auth.py was frozen at 770 lines. /me moved to api/auth_me.py, leaving 734.
• There is one scope matcher. repo_tests/api_key_scopes_are_enforced_16040_test.py forbids a second implementation. So scope_granted in key_scopes.py is the rule, APIKey.has_scope delegates to it, and key_permissions is built on it.
• The permission decision is pure. SLM tests cannot import services/auth.py in isolation: autobot-slm-backend is off pythonpath (tech-debt(tests): cross-backend + conftest-stub namespace pollution causes ~92% of full-suite collection errors and ~1400+ runtime failures (prereq for #10691) #13084) and services is stubbed. So the whole decision moved to a FastAPI-free services/api_key_authority.py, which the conftest loads for real. services/auth.py only turns a refusal into a 403.

What Changed

Commit	Change
c496187	Refactor: the seeding builders move to permission_seed.py, with no behaviour change. The ratchet entry is deleted, since the file is under MAX_LINES.
816ae02	Seven new Permission members and their grants. key_scopes.py holds all 14 scope bundles.
6d8a801	GET /me returns permissions (as role_has_permission answers them) and is_admin, from api/auth_me.py.
c94b9f9	One scope matcher, shared by has_scope and key_permissions.
e0b1dc6	#16040: owner role ∩ key scopes, require_key_permission, the grace period for pre-enforcement keys, and the reason /api-keys/scopes stays open.

Grant record (#16270 AC4):

permission	granted to	kind	evidence
chat.use, chat.history	admin, operator, analyst, editor, user, readonly	behaviour-preserving	10 of the 11 backend chat route files are login-only. The one admin-only route in chat_shared_links.py manages share links.
teams.read / create / manage / delete	admin	policy choice	the #16276 ruling
webhooks.trigger	admin	policy choice	It gates nothing today; inbound webhooks authenticate with provider secrets.

Superadmin holds none of these, by design (#13854). /me reports is_admin so the frontend still admits a superadmin.

#16040's acceptance criteria:

• AC2: permission_allowed grants a permission only if the owner's role holds it AND key_permissions(scopes) carries it.
• AC4: require_key_permission(permission) answers 403 when a key lacks the scope.
• AC5: A key created before AUTOBOT_API_KEY_SCOPES_ENFORCED_FROM (default 2026-09-11) keeps its owner's authority, with a warning on every use, until AUTOBOT_API_KEY_LEGACY_GRACE_DAYS (90) have passed. After that it gets 401 until it is re-issued.
• AC6: api_key_authority_test.py shows a narrow-scope key being refused an out-of-scope permission in the full decision.
• AC7: GET /api-keys/scopes stays open. Its docstring gives the reason: it returns a static catalogue of scopes, not keys, users or tenant data.

Verification

• Local checks: ruff, black (line length 120) and isort pass on every changed file. Every commit passed pre-commit, including strict mypy on autobot_shared, the hardcoded-values hook and the 600-line guard. I did not run tests locally; the repo verifies through CI.
• The existing API key scopes are stored and displayed but never enforced — keys inherit full user authority #16040 guards still hold. get_api_key_user still returns a dict literal, its admin still comes from has_scope("admin:*"), it still carries scopes, and no scope matching exists outside the single implementation.
• The generated api.ts for /me is regenerated by the auto-fix-generated-types workflow.
• Stated gaps:
  • No HTTP-level test through require_key_permission. SLM tests cannot import services/auth.py in isolation. permission_allowed is the whole decision and is tested directly; the dependency only converts False into 403. Route-level tests come with feat(auth): decide which routes accept API keys, and wire X-API-Key through the #16040 mechanism #16294's wiring.
  • New members are not seeded into the permissions table on existing deployments. This gap predates this PR (feat(llc): who may edit a card — the creator, their manager, and that manager's manager #15765's member has it too) and is filed as bug(auth): new Permission members never reach the permissions table on existing deployments, so LLC roles cannot be granted them #16295.
  • AC5's cutoff defaults to this PR's date. If enforcement reaches a deployment later than that, set AUTOBOT_API_KEY_SCOPES_ENFORCED_FROM to the actual deploy date.
• Follow-ups: feat(auth): decide which routes accept API keys, and wire X-API-Key through the #16040 mechanism #16294 (which routes accept keys) and bug(auth): new Permission members never reach the permissions table on existing deployments, so LLC roles cannot be granted them #16295 (the database seeding drift). This PR unblocks tech-debt(frontend): usePermissions' permission strings share nothing with the backend's Permission vocabulary #16243, the frontend reading /me.

Model Used

Claude Opus 5

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 6ac49a58-d5bc-407d-aeb3-a767570cd5dc

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[mrveiss]

Security review at e0b1dc6a4a: approved. It merges on a settled green at that SHA. One closure ask: #16040 can't close with this PR, so please change Closes #16270, #16040 to Closes #16270 plus Refs #16040.

Commit	Check	Evidence
c49618761 move	Behaviour-preserving	permission_seed.py carries the secrets-vault rows unchanged. Base gave admin extras to Role.ADMIN, user extras to Role.USER and [] to everything else. _SECRETS_EXTRA_BY_ROLE.get(role.value, []) with keys "admin" and "user" is the same mapping. getattr(p, "value", p) equals p.value if isinstance(p, Permission) else p for Permission and str items. The builders take their sources as arguments, so there's no import cycle.
816ae0284 vocabulary (#16270)	AC1–AC4	Seven members: chat.use, chat.history, teams.read/create/manage/delete, webhooks.trigger. test_team_and_webhook_permissions_are_admin_only, test_superadmin_gains_none_of_them and test_every_role_that_can_log_in_keeps_chat. key_scopes.py holds 14 bundles, test_no_scope_maps_to_an_empty_bundle, and admin:* equals exactly what Role.ADMIN holds. The body records each grant as behaviour-preserving or a policy choice, and the four decisions are recorded on the issue (06:26Z). roles_are_canonical_test is untouched.
6d8a8014c /me (#16270 AC5)	permissions and is_admin	test_a_superadmin_gets_no_granular_permissions_and_is_admin, test_an_admin_gets_every_permission, test_an_unknown_role_gets_nothing and test_no_session_is_a_401. auth.py 770 → 734, matching the file.
c94b9f99b one matcher	scope_granted	APIKey.has_scope delegates to it, and key_permissions is built on it. It fails closed on a non-list scopes, because a string would substring-match "*" and grant everything. That's tested for "*", "admin:*", None, a dict and 7, and test_a_resource_wildcard_does_not_leak_into_other_resources.
e0b1dc6a4 #16040	AC1–AC3, AC5, AC7	get_api_key_user returns the owner's role (same rule as sessions), scopes, api_key_id, legacy_full_scope, and "admin" only when the owner is a platform admin and the key holds admin:*. permission_allowed requires the role to grant and, for a key caller, the scopes to carry it (test_a_key_never_exceeds_its_owners_role, test_an_admin_scope_on_a_non_admin_owners_key_grants_no_admin_permission). The AC5 cutoff is a fixed "2026-09-11" (env-overridable, not computed at runtime), and a key with no created_at is held to its scopes (fails closed). After grace, _legacy_grace_or_401 refuses the key; within it, every use is logged. legacy_full_scope counts only as a literal True.

Why #16040 stays open. Its AC4 is "routes declare the scope they require, and a key lacking it gets 403". require_key_permission exists, but, as the body says, no route accepts a key yet (#16294). AC6 asks for a route-level test, and the refusal is tested at the decision level only, because SLM tests can't import services/auth.py in isolation, as stated. Both land with #16294.

Note, non-blocking but worth doing before #16294 wires any route: permission_allowed applies the key half only when "api_key_id" in caller. The real payload sets it today, but no test pins that get_api_key_user's return carries it (every test builds its own dict). A refactor that dropped the key would give every key its owner's full role, with no test failing. An AST pin on the returned dict's keys, the way #16292 pinned main.py, would close that.

[mrveiss]

CI red at 083f9e8344, caused by this PR, so it is out of the merge train. There are two reds.

1. SSOT Configuration Compliance (detect-hardcoded-values.sh --audit-baseline)

STALE  other|autobot-backend/api/auth.py|"user"

This PR moves code out of api/auth.py into api/auth_me.py, so the baseline line no longer matches anything. To fix:

1. Run ./pipeline-scripts/detect-hardcoded-values.sh --prune-baseline.
2. Commit the pruned baseline.
3. Confirm that the moved "user" literal does not now fire in api/auth_me.py. The no-growth check blocks that direction independently.

2. CodeQL: 1 new high alert, "Clear-text logging of sensitive information"

• Where: autobot-infrastructure/shared/scripts/migrations/migrate_config_users.py:150, i.e. logger.debug("Created permission: %s", perm_data["name"]).
• That file is not in this diff. I have not traced which changed file its source (1) points at; the alert's source link names it.

Once both are green, this PR merges on its own after the train.

[mrveiss]

Delta review of adda1a5 from a second session (the coordinator): needs one fix. This combines my reviewer's check with 90's pass.

• The CodeQL sink fix (bccc1c4) is correct. It removes exactly the flagged logger.debug("Created permission: %s", ...) line. No other logger.* call in the file prints a permission name, and the count line stays. The script can't run today anyway (bug(infra): migrate_config_users.py reads SYSTEM_PERMISSIONS and SYSTEM_ROLES as lists of dicts, so both seeding loops raise TypeError #16327), so that line was never reached.
• The SSOT baseline fix deletes too much. Base's entry was 2|other|autobot-backend/api/auth.py|"user", covering two occurrences. Moving code to auth_me.py took one of them, so the audit flagged the count as too high; it didn't flag the entry as dead. d630882 deletes the whole line, but at this head auth.py:136 still reads "role": "admin" if user.is_platform_admin else "user", now with no baseline entry. The SSOT scan will fail on it. I verified this at review/16298; the SSOT check at this head hasn't reported yet.
• Fix: replace the literals with the enum auth.py already imports (line 39): role_value(Role.ADMIN) if user.is_platform_admin else role_value(Role.USER). Role.USER = "user" is defined in autobot_shared/auth/permissions.py, and role_value is the sanctioned stringifier (bug(auth): is_admin_role(Role.ADMIN) returns False — str() on a (str, Enum) member yields 'Role.ADMIN' #14944). That leaves this file needing no baseline entry at all, which beats restoring the entry at count 1.

[mrveiss]

Delta review of 219fd8f from a second session (the coordinator): approve.

The one commit replaces auth.py:136's literals with role_value(Role.ADMIN) if user.is_platform_admin else role_value(Role.USER), using the imports already at :39. No "user" literal is left, so no baseline entry is needed. The CodeQL sink fix (bccc1c4) was approved in the previous round.

My owner-approved cancellation matched on branch rather than head, so it caught this head's CI. I've re-run those runs (20 re-run). Once they're green at this head, blocked can come off.

[mrveiss]

Delta review adda1a523→219fd8f87: the SSOT red is resolved at its source, so this delta passes.

• It's one line, autobot-backend/api/auth.py:136: "admin" if … else "user" becomes role_value(Role.ADMIN) if … else role_value(Role.USER). There are no trailers.
• The claim value is unchanged:
  • Role.ADMIN = "admin" and Role.USER = "user" (autobot_shared/auth/permissions.py:174, :179);
  • role_value returns role.value for a Role (:252-253);
  • both names were already imported (auth.py:39).
• auth.py now has no "user" literal. The remaining "admin" at :280 is a dict key (current_user.get("admin")), not a role literal. So deleting the baseline entry in d630882 is now correct, because of this commit.

blocked comes off once CI is green at 219fd8f87 and f2's delta review approves. The misleading audit wording behind the first attempt is filed as #16334.

[mrveiss]

Content review: approve. (Prioritized per the #16493 dependency.)

Verified the core decision chain end-to-end:

• key_scopes.scope_granted: correctly guards against the string/list membership pitfall (isinstance(scopes, list) before any in check) — a scalar scope string would otherwise pass a substring test against "*" and silently grant everything. One matcher, APIKey.has_scope and key_permissions both delegate to it.
• services/api_key_authority.permission_allowed: role-grants-permission AND (no api_key_id OR key_scope_allows) — a key can't exceed its owner's role or its own scopes. FastAPI-free, directly unit-tested (17 cases covering wildcard scopes, malformed scopes, non-admin-owner-with-admin-scope, session-vs-key contrast).
• AC5 grace period wired correctly end-to-end: services/auth.py's _legacy_grace_or_401 computes legacy_grace_deadline(api_key.created_at), raises 401 past the deadline, warns and sets legacy_full_scope: True on the caller dict otherwise; key_scope_allows honors that flag for full owner authority. Fails closed on unknown key age (created_at is None → not legacy).
• require_key_permission reuses the same _require_permission_or_403 as session-based require_permission — no route wires it yet (correctly deferred to feat(auth): decide which routes accept API keys, and wire X-API-Key through the #16040 mechanism #16294), but the mechanism is real and tested.

Stated gaps (no HTTP-level test through require_key_permission, permission-table seeding drift #16295) are honestly disclosed, not hidden, and don't block this PR's own scope.

closingIssuesReferences=[16270], matches its own Closes #16270. CI green (fail=0, pending=0), no missing required contexts. Ready — and unblocks #16493 per the dependency note.