fix(mcp-keys): listing keys requires a signed-in session - #3045
Merged
Merged
Conversation
Records the rule in security.md §20.10 ahead of the change: POST /api/mcp/keys and POST /api/mcp/keys/ensure-default take a signed-in (JWT) session, and ensure-default writes the same key_create audit row as the create route.
…person, require_interactive Two allowlist rules over mcp_scope, both fail-closed on a principal without one: PERSON (a JWT session or the person's own user-scoped key, via is_person_principal) and INTERACTIVE (a JWT session only). The Depends forms make a route's rule a one-line signature change an AST guard can see. Both also refuse a principal carrying vouched_source_agent. The ent#611 ask-endings detail and predicate are unchanged, and pinned so.
POST /api/mcp/keys (every scope) and POST /api/mcp/keys/ensure-default now take Depends(require_interactive): a credential minter is at least as strict as the principal it produces. The existing per-scope checks stay as they were. ensure-default also writes the same key_create audit row as the create route, so every created key is attributable. Tests go through the real get_current_user with seeded key rows and assert the mcp_api_keys row count is unchanged on every refusal.
…t registry mcp-api-keys.md: POST /keys and ensure-default are signed-in-session only; ensure-default is audited as key_create. Adds the feature-flows index row, a learnings fragment and the two new test files to tests/registry.json.
…he route census requirements/auth.md §2.8 (and the §2.7 helper table): the PERSON and INTERACTIVE rules, which routes take which, the primitives, and the route census with its shrink-only baseline. architecture/security.md §5 and §6, and one sentence on Invariant #8. Refs #2996, trinity-enterprise#711
PUT /api/agents/{name}/autonomy takes Depends(require_person): a JWT
session or the person's own user-scoped key. Agent-scoped keys (on
their own agent or any other), the system key and every other scope get
a 403 with the human-only detail, before the owner check, so the
refusal discloses nothing about whether the agent exists.
Autonomy decides whether an agent's cron schedules fire unattended; it
is a grant, so the agent's own key must not be able to switch it on.
Tests go through the real get_current_user with seeded key rows and
assert the stored autonomy flag before and after.
Refs #2996
…2996) PUT api-key-setting, read-only, resources, capabilities, capacity, timeout, public-channel-model and guardrails take Depends(require_person), the same rule as autonomy: the same file, the same owner configuration surface, and no agent or system caller. Each write is tested from its own non-ambient stored value: machine keys through the real get_current_user leave it unchanged, a person changes it. A router-level check pins that these and autonomy are every PUT in the file. Refs #2996
…a signed-in session PUT /api/users/me/email and PUT/DELETE /api/users/me/github-pat take Depends(require_interactive). The email is the account's sign-in identity and the PAT is the credential future agent creations inherit; both are credential/identity bindings, so no MCP key may change them, the person's own user key included. Tests: every key kind through the real get_current_user, with the stored email and encrypted PAT checked before and after; the session path (400 on a bad shape, 409 on a taken address, set and clear) unchanged; and a record that email sign-in resolves the account by the email column alone. Refs trinity-enterprise#711
…2996) Every OSS backend route, every method, must resolve to exactly one class: gated in code (interactive / person / admin tier / widened admin / portal), listed with a reason (agent-callable / own-auth / delegated), or in the frozen baseline, which only shrinks. A new route that is neither gated nor classified fails the build, so the next human-only route inherits the rule instead of relying on someone remembering it. The AST side is import-free (rglob over src/backend, minus the private submodule). The runtime side imports main in a subprocess and checks every stored METHOD /path against the live table; it fails, never skips.
The human-only rule covers the autonomy grant, not the schedule tools it bounds. Pins create/enable/disable with the agent's own key through the real principal resolution, asserting the stored state moves.
Autonomy, read-only, resources, capabilities, capacity, guardrails, public-channel model and the API-key setting flows note the person-only rule on their writes; email-authentication, first-time-setup and github-sync note the session-only rule on the sign-in email and the personal GitHub PAT. Index rows, a learnings fragment, and the schedule regression test in auth.md §2.8. Refs #2996, trinity-enterprise#711
…tes call (#2996) The read-only route imports its logic lazily from sys.modules, and a sibling unit file evicts that entry at import, so under some orders the route ran an unpatched copy and 404'd. The fixture now resolves and pins the real module, and patches the bound logic through the globals of the functions the router holds.
Drop the sign-in resolution test class and its registry note: they pinned behaviour outside this change. Reword the census and MCP-boundary notes neutrally; the baseline is a to-do marker, not a judgement.
… only (#2996) An inherited PYTHONPATH (the verify-local unit stage sets one) let the subprocess resolve `main` from another tree, so the fail-loud check on a missing app could not fail.
GET /api/mcp/keys now takes the interactive rule: only a JWT session may list keys. The Settings tab is unchanged; a signed-in admin still sees every key with its owner's email, and a signed-in user sees their own. Refs trinity-enterprise#712
Refs trinity-enterprise#712
The real connector key is refused at the entry point, so add a principal that reaches the route. Route labels no longer depend on other changes. Refs trinity-enterprise#712
Switch GET /api/mcp/keys to Depends(require_interactive), like its POST siblings, and take it out of the route-census baseline (count 360) now that it is classified. Refs trinity-enterprise#712
This was referenced Sep 28, 2026
Contributor
Author
|
✅ Nightly unit-suite clean when this PR is merged into |
|
Resolve by merging |
…e-2996 # Conflicts: # tests/registry.json
…sus (#2996) Routes that landed on dev after the branch was cut were unclassified. Skill-set assign/unassign are the USE of the ent#596 skills-manage capability (fenced by get_skill_managed_agent_by_name; the grant is the admin+interactive /skill-manager route), the two skill-set reads are open reads, and raise_my_ask is MCP ask_operator, self-only via get_self_acting_agent. All five go in AGENT_CALLABLE with a reason.
An agent key can no longer PUT /api/agents/{name}/timeout, so the cap
refusal on schedule and loop timeouts now says the owner sets it.
…to AndriiPasternak31/ent712-on-2996
…12-on-2996 # Conflicts: # docs/memory/feature-flows.md # docs/memory/requirements/auth.md # tests/registry.json # tests/unit/_route_census.py # tests/unit/fixtures/human_only_route_baseline.json # tests/unit/test_2996_human_only_routes.py
AndriiPasternak31
marked this pull request as ready for review
September 29, 2026 22:07
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
2 participants
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.
Refs trinity-enterprise#712
Pre-merge gate: waived
The evidence-preservation gate on trinity-enterprise#712 was waived by @vybe at merge-train Gate 1 on 2026-09-29 (waiver on ent#713, ent#712 note that it unblocks this PR). The post-merge obligation to notify the reporter still stands.
Ship in the same release as #3040 (already in
dev), so the release doesn't carry half of the fix.Summary
GET /api/mcp/keys(the key inventory) now takesDepends(require_interactive), the same rule as creating a key: a signed-in session only. The Settings → MCP Keys tab is unchanged: a signed-in admin still sees every key with its owner's email, and a signed-in user sees their own. Nothing returns the secret. The list is only this route's rule;db.list_*is untouched.This PR also takes the route out of #2996's census baseline (count 360) and adds its row to
requirements/auth.md§2.8.Base
Base is
dev. PR M (#3043) and PR A (#3044) have merged, so the diff is this PR's changes only: 9 files, +270/−9.Journey Impact: none: the UI lists keys with a JWT; no key-authenticated caller exists
Type of Change
Testing
tests/unit/test_mcp_key_listing_requires_session.py: every principal kind, plus every route inmcp_keys.pyread off the router with its rule.dev(7d73ac5ae): census,test_2996_*and the mcp-key session tests 227 passed (no randomization), and 275 passed withtest_mcp_key_track_usageandtest_1854on seed 99999.After merge
Close
trinity-enterprise#712by hand at the release cut.🤖 Generated with Claude Code