Require operator credentials on the admin surface (REST + MCP) - #39
Open
therevoltingx wants to merge 2 commits into
Open
Require operator credentials on the admin surface (REST + MCP)#39therevoltingx wants to merge 2 commits into
therevoltingx wants to merge 2 commits into
Conversation
therevoltingx
marked this pull request as draft
July 29, 2026 21:31
therevoltingx
commented
Jul 29, 2026
| storage = await get_storage() | ||
| service = ApiKeyService(storage) | ||
| return await service.create_key( | ||
| ApiKeyCreateRequest( |
Contributor
Author
There was a problem hiding this comment.
Doesn't seem right to overload the api key, think a new object should be created instead of having all these non-sensical field values
therevoltingx
marked this pull request as ready for review
July 29, 2026 21:51
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
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.
Require operator credentials on the admin surface (REST + MCP)
Problem
Nearly all admin endpoints were publicly accessible: anyone who could reach the
server could mint API keys (
POST /auth/api-keys), read the event log, rewritethe rate card, change buyer-agent trust levels, mutate packages, trigger
inventory syncs, and push/distribute deals. There was also no way to
distinguish an operator credential from a buyer credential —
ApiKeyRecordhadno role concept — so the existing auth dependencies couldn't have gated these
routes correctly even if applied.
What this PR does
1. API keys now carry a role
ApiKeyRoleenum (buyer|operator) onApiKeyRecord, the createrequest/response, and
ApiKeyInfo.buyer, so pre-existing stored keys deserialize as buyers — oldkeys are never silently promoted.
POST /auth/api-keysaccepts"role": "operator", so an existing operatorcan mint additional operator keys (invalid roles → 400).
2. Operator enforcement
require_operator_keydependency: anonymous/invalid/revoked/expired→ 401, valid buyer key → 403, operator key → allowed.
/auth/api-keys(create, list, get, revoke)/events,/events/{id}PUT /api/v1/rate-cardPOST /api/v1/inventory-sync/triggerGET /gam/orders,GET /gam/report(previously optional auth)POST/PUT/DELETE /packages,/packages/assemble,/packages/syncPOST /api/v1/curators,POST /api/v1/deals/push,POST /api/v1/deals/distributeGET /packages,GET /registry/agents,GET /api/v1/rate-card, media-kit search, etc.).set_agent_trust,register_buyer_agent,sync_inventory,create_package,update_rate_card,set_publisher_identity,set_approval_gates,approve_or_reject, deal push/distribute, GAM reporting,list_sessions,inbound queue, buyer activity). Over HTTP transports the tool validates an
operator key from
Authorization: Bearer/X-Api-Key(mcp-remote:
--header "Authorization: Bearer <key>"). Local stdio accessis trusted, same model as the CLI.
3. Bootstrap
ad-seller create-operator-key [--label ...] [--expires-in-days N]. Mints an operator key by writing directly to storage(no network surface), prints the key exactly once. This is how the first
operator key is created; subsequent keys go through the now-gated REST/MCP
surface.
ApiKeyServiceinstead of POSTing to the now-gated endpoint.Bug fix found along the way
app.dependency_overridesnever worked for router-mounted routes:main.pyappends router routes directly and set
dependency_overrides_providerafterward, but
APIRoute.__init__had already compiled the request handlerwith a
Noneprovider, so every test override in the codebase was a silentno-op. The routes' handlers are now rebuilt after wiring (using FastAPI's
request_response, which also sets up the dependency exit stacks). Threetests in
test_trust_tier_verification.pywere unknowingly relying on theiroverride not applying and were adjusted to drop it where they genuinely
need real key validation.
Tests
tests/unit/test_operator_auth.py(21 tests): legacy records default tobuyer role, 401/403/allowed matrix across the REST surface, operator keys
minting buyer + operator keys, CLI bootstrap, and MCP gating (stdio allowed;
HTTP without key / with buyer key denied, with operator key allowed).
dependency via
app.dependency_overrides(they test catalog/flow behavior,not auth).