Repository navigation
feat(auth): ops_write — a bounded machine credential for four operational writes (#3180) - #4024
AndriiPasternak31 wants to merge 32 commits into
Conversation
) A bounded write tier beside the read-only `ops` scope. An `ops_write` key is admitted at the single auth entry point only when three predicates hold: (method, routed path) full-matches `_OPS_WRITE_ALLOWED_ROUTES` (circuit-breaker reset, agent start, agent stop, execution terminate); the routed handler carries `@ops_write_route("<reason>")`, read from `scope["route"].endpoint`; and the key owner's row is still `role == "admin"`, so the key dies with its owner on every route, including the ones with no admin gate. `enforce_agent_spawn_scope` gains the target policy for this scope: start and stop refuse the system agent (by name or `is_system`) and ephemeral agents, and fail closed when that lookup raises or cannot be proven. `ADMIN_GATE_SCOPES` is unchanged; nothing is opted in by this commit, so the scope reaches no route yet.
Marks circuit-breaker reset, agent start, agent stop and execution terminate
with `@ops_write_route`, below each route decorator. Reset's admin gate becomes
`require_admin_allowing("ops_write")`: the same gate with one bounded scope
admitted, `role == "admin"` still enforced afterwards. The now-unused
`require_admin` import in routers/agents.py is dropped.
Guard change, and why: the #2996 route census requires every widened admin
gate to have a reviewed `ADMIN_WIDENED` entry, so reset gains one row in
tests/unit/_route_census.py. The frozen baseline is not edited and its count
is unchanged; start, stop and terminate stay where they were.
…rite (#3180) `POST /api/mcp/keys` accepts `scope: "ops_write"` behind the same ladder as `ops`: route-level `require_interactive`, then `reject_non_interactive_principal`, `reject_agent_principal` and `assert_admin`. The DB layer adds it to `_USER_CREATABLE_SCOPES`; the row is written with `agent_name` NULL as for every creatable scope. `PERSON_KEY_SCOPES` gains the scope, so a password change with "revoke keys" revokes an ops_write key after a suspected leak; a test pins it equal to the mintable set. An ops_write key found in an agent's `.mcp.json` reports its own verify verdict, `foreign_ops_write_key`, instead of "a personal key".
Admins get an "Ops (operational writes)" scope choice beside "Ops (read-only)", whose hint names the four actions, the system/ephemeral refusal, the missing reads and the owner binding. A minted ops_write key is badged "Ops (writes)" so it never reads as a Standard key, and the agent key panel labels the `foreign_ops_write_key` verdict. Semantic tokens only. tests/unit/mcpKeysOpsWriteScope.mount.spec.js mounts the tab: the badge, the radio beside the read-only one, the exact `scope: "ops_write"` it sends, and no bounded scope offered to a non-admin.
`OPERATOR_SCOPES` stays {user, agent, system} and its pin is untouched. The
auth-context scope union gains "ops_write" (it is kept exhaustive against the
column), and the call-time gate suite boots a real FastMCP server for an
`ops_write` and an `ops` session: nothing is advertised, and calling the
operator tool by name is a not-found that never runs its body.
Real keys minted through POST /api/mcp/keys on the real schema, the real resolver and routers; only Docker and Redis are faked, each named where it is installed. Pins the four pass paths (start through the real start_agent_internal, including the #1854 self-heal of a revoked baked key: nothing reaches the caller and its reach is unchanged), start/stop audit attribution, the target policy, demotion and suspension on all four routes, password-change revoke and its parity with the mintable set, the mint matrix (composed, and each redundant rung in isolation), the ops key refused on the four writes, A2A answering 403 with no dispatch and no anonymous fallback, enumeration safety, the verify verdict, and a crafted Host (#3102) through the routers with the desync asserted.
) The marked set is a frozen literal; marks are unaliased, carry a reason and sit below their route decorator; nothing else references the mark or its attribute; every marked handler authenticates through get_current_user; every path entry resolves to exactly one live, marked route. The fence/gate agreement pair from test_2323 is mirrored for reset. _ops_write_sweep.py imports the real main.app in a subprocess and drives every OSS route with real keys, classified by what its credential dependency does: unlisted routes refuse, the four do not; with the path list holed the mark still refuses every unmarked handler; with every handler marked the path list still refuses; a demoted owner's key is refused and a suspended owner's key is 401 everywhere; the ops key reaches no write; the named grant families refuse, and with the fence call removed each shows its own gate or, for the owner-tier routes, that the fence is their only layer. Routes reading no Trinity credential are listed by name. OSS routes only.
) Guard change, and why: test_3102 drives every scope fence in the resolver with a real Starlette request whose Host desyncs `url.path` from the routed path. A new fence must join that list or the guard silently stops covering the resolver. The new case passes the real marked start handler as the route, so only the path decides, and the section heading now counts six fences.
requirements/security.md §20.10 gains the ops_write block (shape, the four measured routes, the three admission predicates, the start/stop target policy, start's self-heal as a use, the owner binding, the mint ladder, password-change revoke, the allowlists that exclude it, attribution) and its "not delivered" line now says the write tier covers four routes only and which refusals rely on the route fence alone. The PERSON table, the architecture scope table and the routed-path fence list name the scope; the MCP keys flow gains MCP-008 and the write path; the user docs gain the scope row, the owner-binding paragraph, the admin-gate and mint lines, the /ws/events row and the POST /api/mcp/keys row.
The target policy fails closed when the system/ephemeral lookup raises, and the caller sees only the protected-target refusal. Log the cause, so a DB fault that refuses a durable agent is diagnosable instead of silent.
Under the full unit tier a sibling test leaves CREDENTIAL_ENCRYPTION_KEY in os.environ, so the subprocess inherited it, the admin's `user` key registered a subscription, and the later "assign" family found it — a comparison against the fence-removed run that depended on test order. The subprocess now gets only PATH/HOME/TMPDIR/LANG, and the `user`-key run covers only the fence-only families, so no gated family can change what a later one finds.
assert_admin runs the same agent rejection, so the isolated test for the in-handler reject_agent_principal stayed green with that rung deleted. Neutralize assert_admin too, so only the rung under test can refuse.
…are (#3180) Two proofs in the ops_write sweep could not fail (independent review): - the pull seams refuse a non-pilot agent before they read the key, so the ops_write 403 there said nothing about the key check. The sweep now makes the seeded agent a pilot and drives every self-validating door with the agent's own key too; that key must get past the check (200, or 404 for the missing execution), so the ops_write 403 is the key check's answer; - the get_optional_user door was probed with a token matching no share, where every principal gets the same 404. The sweep now seeds a live `authorized` canvas share: the owner's key and session get the canvas, anonymous and an unknown key are asked to sign in, and with the fence call removed the ops_write key would get the canvas.
… says valid (#3180) Introspection validates the key itself and checks neither the owner's role nor suspension, so the universal "refused on every route" wording was not true of it. Qualify it, and say plainly that a valid answer there is no evidence the owner binding failed.
… demote (#3180) Two security statements were false. §20.10 said every audit row an ops_write key causes carries its mcp_key_id, but the #1854 self-heal a start may run writes its agent_key_self_heal row as `system`, without the key, so an operator scoping a leaked key by mcp_key_id would miss the one credential mint and delete the key caused. And both §20.10 and the API authentication page advised containing an agent by taking it "out of the key owner's reach", which cannot be done: the fence admits the key only while its owner is an admin, and an admin reaches every agent. The remedies are deleting the agent, revoking the ops_write keys, or demoting their owner.
The FAQ said admins see two bounded scope choices and that every key carries one of six scopes, listed the bounded scopes and the scope badges without ops_write, and called portal_delegate "the other bounded scope". With ops_write there are three admin choices, seven scopes and an "Ops (writes)" badge.
…#3180) The OpenAPI description of GET /api/audit-log's mcp_scope filter enumerated the credential kinds without the new scope.
…gh the probe (#3180) The sweep opened /ws?token=<key>, but /ws reads only `ticket`, so it closed 4001 for any input and the assertion could not go red. The key's door to /ws is POST /api/ws/ticket, which the families sweep already shows refused; the socket test now asserts /ws/events only and says why. The verify-verdict test called _entry_verdict on a row it had read itself. It now goes through interpret_probe_payload with the stored key_hash as the container's digest, so the hash lookup is in the path (verdict branch removed: 1/38 red). A list-membership test named as if it checked a refusal is renamed; the refusal is the next test's.
…ency (#3180) test_an_authorization_refusal_records_the_client overrode main.get_current_user. The unit conftest evicts `dependencies` after every test, so when a test file has imported a router before `main` is first imported (the ops_write suites import routers.agents at module level), `main` binds a newer copy of get_current_user than the one the POST /api/agents route was built with. The override was then never consulted and the request answered 401 instead of 403: red whenever one of those suites ran first on the same xdist worker, green alone. Take the dependency from the route's own globals instead, through the route_globals helper the file already uses for the sign-in tests. The assertion is unchanged.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
AndriiPasternak31
added
the
ui
webmixgamer
left a comment
webmixgamer
left a comment
There was a problem hiding this comment.
Review via /review (pre-landing structural review) on the draft head 7ab23eac0.
/review Report
Branch: AndriiPasternak31/issue-3180-s1 → dev (merge-base 0f689db80)
Files Changed: 29 (+2190/−38): backend 10, frontend 3, MCP server 2, tests 7, docs 8
Scope: CLEAN. Every hunk serves the ops_write scope, apart from one test made order-independent (test_ent873_audit_client_ip.py, explained in the body).
Plan Completion (#3180 proposal): 5 done, 1 changed, 1 partial, 1 not done
- DONE: an explicit fail-closed allowlist beside
_OPS_ALLOWED_ROUTES(dependencies.py_OPS_WRITE_ALLOWED_ROUTES); never-grantable families refused (the fence + the 684-route live census); minted only from an interactive admin session (mcp_keys.py:require_interactive→reject_non_interactive_principal→reject_agent_principal→assert_admin); dies with its owner (_enforce_ops_write_key_fenceowner check, suspension in the resolver,PERSON_KEY_SCOPES); no MCP tools (OPERATOR_SCOPESunchanged, pinned bytool-visibility.test.ts). - CHANGED: candidate routes cut to the four measured ones. Restart/recreate, fleet restart, emergency stop, cleanup sweeps, retention acknowledge and schedule toggle are listed under "Not delivered".
- PARTIAL: attribution. Start and stop audit rows carry
mcp_key_id/mcp_scope; reset and terminate rows land in #4035. - NOT DONE: "optionally expiring". Scope cut, low impact, and the follow-up is not filed yet (I2).
Execution coverage (Step 2.5)
The PR's own mutation list covers every gate: the fence call, path, mark, owner check, reset opt-in, the four marks, the target policy and its fail-closed arms, the mint rungs, _USER_CREATABLE_SCOPES / PERSON_KEY_SCOPES, the verify verdict, the UI and OPERATOR_SCOPES. I added three it does not list:
| changed symbol / wiring | executed by | live consumer | verdict |
|---|---|---|---|
WS_EVENT_STREAM_SCOPES keeps ops_write out (N1: added it) |
test_3180_ops_write_route_census::test_the_event_stream_socket_refuses_the_key (red: 1) |
main.py /ws/events |
✅ executed |
_refuse_ops_write_protected_target name arm (N2: dropped target_agent == SYSTEM_AGENT_NAME) |
— (green) | start/stop | ✅ equivalent mutant: db.is_system_agent already returns True on the name (db/agents.py:696) |
missing is_ephemeral key fails closed (N3: info.get(...)) |
TestTargetPolicy::test_an_unprovable_ghost_lookup_refuses[sqlite-no-flag] (red: 1) |
start/stop | ✅ executed |
No changed test asserts source text. The census drives the real main.app with real keys.
Critical Findings (block merge)
None found.
Informational Findings (review required)
[I1] Design / Product Quality: an ops_write start can undo a person's containment by re-minting the agent's key (Confidence: 7/10)
Files: services/agent_service/lifecycle.py (start path), docs/memory/requirements/security.md (§ops_write)
Evidence:
mcp_key_match = check_agent_mcp_key_matches(container, agent_name)…if not mcp_key_match: … mcp_env_overrides = await heal_agent_mcp_key_env(agent_name)- The PR's own requirement text: "Consequence: an agent stopped and key-revoked for containment can be started again by this key … to contain an agent, delete it, revoke the
ops_writekeys, or demote their owner."
Issue: the scope exists to be bounded, yet its one documented failure mode is the one that matters most: a person stops an agent and revokes its key, and an automation whose job is "start agents that are down" brings it back with a freshly minted credential. Every remedy offered is fleet-wide (revoke every ops_write key, demote the owner) or destructive (delete the agent). Minting a credential is the grant side of Invariant #8's grant-vs-use line, even when the platform mints it for the agent rather than for the caller.
Suggestion: for mcp_scope == "ops_write", refuse a start whose MCP-key predicate is unsatisfied with a named 409 (e.g. ops_write_would_mint_key, "a person must start this agent"). On a healthy agent the predicate matches, so automation loses nothing, and the containment path stays a person's decision. If that is declined, the requirement should say why that trade was made.
[I2] Process: the follow-ups the body says are "to be filed" are not filed (Confidence: 9/10)
Searching both trackers for 3180 returns only #3180 itself. Missing:
- Key expiry: the issue's "optionally expiring".
- The
opsdocs-vs-code claim:architecture/security.md'sopsrow says "demotion 403s it", while the body reports that a demoted-but-operator owner'sopskey still reads the fence-only reads (from reading the resolver; I did not run it either). This is a security doc an operator would act on, so it is the most important of the four. - The second layer for terminate and for key revoke/delete (today only the fence holds them).
- The incomplete user docs (FAQ admin-gate, event-stream and
/wsanswers, the mint row).
Suggestion: file them before leaving draft and link them here. Alternatively, fold the ops doc correction in, since it is diagnosed and doc-only.
Low confidence (appendix)
- (5/10) "Dies with its owner" reads as permanent, but demotion only suspends the key: it stays
is_active, and re-promoting the owner makes it work again. If that is intended, one sentence in §ops_write would stop an operator from treating demotion as revocation.
Clean Categories
- 4.1 SQL: no new SQL.
_USER_CREATABLE_SCOPESis a tuple check before the insert. - 4.2 Concurrency: the fence is pure (method, routed path, mark, an owner row already loaded); no shared state.
- 4.3 Auth:
- The fence runs inside
resolve_platform_user_unfloored, so it coversget_current_userand the three Workspace doors. - Refusals precede the uniform-404 agent lookup and do not depend on agent existence (#186).
- Reset keeps
require_admin_allowing("ops_write"); nothing writesrequire_role("admin"). - The side doors are allowlists that exclude the scope:
/ws/events(WS_EVENT_STREAM_SCOPES, pinned by N1), heartbeat / #1083 callback / pull seams (authorize_heartbeatneedsagent_nameto match, and it is NULL on these keys), and MCP tools (OPERATOR_SCOPES). - Paths read
request.scope["path"], neverrequest.url(#3102; the fence is added totest_3102).
- The fence runs inside
- 4.4 Credentials: start returns
credentials_resultwith file names and a count only (inject_assigned_credentials→{"credential_count", "files": list(files.keys())}), never values. - 4.5 Enterprise docs: the docs describe the scope and the OSS-core ruling only; no private module named.
- 4.10 Frontend: the badge uses semantic
status-warning-*tokens; the radio sits insidev-if="isAdmin"; the raw-color and loading-gate ratchets are green. - 4.12 Enum completeness:
foreign_ops_write_keyis handled in_entry_verdict,_verdict_message,AgentMcpKeyPanel.vueVERIFY_LABELS, and both model comments. The scope is in thetypes.tsunion, the audit-log filter text,PERSON_KEY_SCOPESand_USER_CREATABLE_SCOPES(the last two pinned equal).
Tests run (head 7ab23eac0)
- Backend, in a clean venv from
tests/requirements-test.txt+ CI's plugins (fastapi 0.115.14 / starlette 0.46.2, matching CI rather than the image pins). The 3 PR suites,test_3102,test_ent873and 26 pinned auth guards (2323, 2389, 293, 2996 + runtime census, 1310 ×2, 837, 3825, 163, mcp-key-session, 186, 2094, ent629, 1890, 995, 1854): 947 passed on seeds 12345 and 67890 (-n auto --dist loadfile -p randomly). - Frontend:
mcpKeysOpsWriteScope.mount.spec.js+ the raw-color and loading-gate ratchets: 23 passed. - MCP server:
tool-visibility.test.ts27/29 on my host. The 2 failures are the new ops / ops_write cases, which fail only because my host hasfastmcp4.16.13 against the lockfile's 4.22.4 (listTools→-32601). CI'sMCP Server Testsjob passes on this PR. Not attributed. - Not run by me: PostgreSQL, the real container, and a real browser (still the draft gate).
Summary
- Critical: none found
- Informational: 2 (I1 is a design decision for the author; I2 is filing work before leaving draft)
- Scope: clean
|
Resolve by merging |
|
Resolve by running |
Brings the branch onto origin/dev b900357. Conflicts resolved by keeping both sides: - dependencies.py: the ops_write fence call stays where it was, ahead of dev's new `principal = User(...)` + `record_acting_turn` (ent#926). - routers/agents.py: the dependencies import keeps ops_write_route and require_admin_allowing (require_admin is no longer used here) and adds dev's assert_not_share_viewer. - routers/chat.py: the dependencies import keeps ops_write_route and adds dev's row_is_viewers and viewer_row_scope. - tests/registry.json: both sides' new entries kept (596 entries, no duplicate file, every entry equal to its source side). Dev's ent#875 viewer tier classifies ops_write as "other", which narrows its terminate to rows its owner started; that semantic conflict is fixed in the next commit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HWna79vVmnezQdhpaGRKbw
Dev's ent#875 viewer tier is an allowlist: a scope it does not name is "other" and is narrowed to the rows its principal started. ops_write was not named, so after the dev merge its terminate refused every execution its admin owner had not started (404 "Execution not found in agent"), which is the stuck-turn case the scope exists for. ops_write now classifies as "key", like ops. Its fence admits one execution route (terminate), so nothing else widens. The ent#875 classification, tier and row-scope tests pin it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HWna79vVmnezQdhpaGRKbw
…ment (#3180) Review finding I1 on #4024: the requirement said an agent stopped and key-revoked for containment could be started again by an ops_write key, because the start's #1854 self-heal re-mints the agent's key. The decision (2026-10-28) reverses that: a start that would mint the agent a key is refused for this scope with 409 ops_write_would_mint_key, the message saying a person has to start the agent. A person's start is unchanged. The refusal does not try to tell a person's revoke from benign drift. A revoke leaves an inactive row, but a person's delete leaves no row, which is also what the platform's own prune leaves. Treating "no row" as benign would leave delete as a containment a machine could undo. So every minting start is refused: the drift heal (reason key_drift) and the #1559 missing-container rebuild (reason container_missing). It costs availability only: on a healthy agent the predicate matches, and a drifted agent needs one person's start, after which the key starts it again. Updated where the consequence was documented: requirements/security.md (including the attribution bullet, since the key no longer causes an agent_key_self_heal row), the MCP keys feature flow, the user API authentication page and the architecture/security.md scope row. Appendix finding: demotion suspends the key rather than revoking it. Re-promoting the owner makes it work again. Both security.md and the user page now say so and say to revoke it to end it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HWna79vVmnezQdhpaGRKbw
Review finding I1 on #4024. When a person had revoked the agent's platform key to contain it, a start made with an ops_write key ran the #1854 self-heal and minted a fresh key, undoing the containment. start_agent_internal takes allow_key_mint (default True, today's behaviour). With False it raises 409 {code: ops_write_would_mint_key, reason, message} before any side effect, so nothing is minted, pruned, recreated, started or cleared. That happens in two places: - reason key_drift: check_agent_mcp_key_matches is unsatisfied, which is exactly when the heal mints. That covers a revoked, deleted, missing or foreign baked key, on a stopped or a running agent. - reason container_missing: the #1559 rebuild, which also mints. The start route passes allow_key_mint=False for an ops_write principal. A person's start is unchanged. Tests: TestContainment drives the real resolver, route and start_agent_internal with a key revoked, deleted, missing, URL-less or foreign, and with no container. Each answers 409 and leaves the key rows, container, breakers and audit log as they were. The same contained agent, and a missing container, started by a person still heal and rebuild. The old test that pinned the heal under ops_write is replaced: a healthy agent's ops_write start keeps the pass path and the key attribution. Without the change, the 7 refusal cases fail (200 with a re-minted key). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HWna79vVmnezQdhpaGRKbw
… it (#3180) Appendix finding on #4024: "dies with its owner" read as permanent. This pins what the docs now say. A demoted owner's key is refused (403 ops_write_owner_not_admin) while its row stays active, and promoting the owner back makes it work again. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HWna79vVmnezQdhpaGRKbw
tests/registry.json: both sides add entries; kept both, this slice's three first, dev's side unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HWna79vVmnezQdhpaGRKbw
…r docs name ops_write (#3180) The docs say demoting an ops key's owner stops the key, but only the reads behind an admin gate did: the `ops` fence reads the method and path only, so the reads it admits with no gate of their own (version, monitoring, telemetry, the agent roster, the execution reads, subscriptions; 10 of 17 probed) kept answering a demoted owner's key. The resolver now checks the owner's role for `ops` ahead of the fence, as it does for `ops_write` (403 `ops_owner_not_admin`). Not an escalation (a demoted owner could read the same endpoints with their own session), but the documented control did not hold. This changes the shipped `ops` tier (#2323): a key whose owner was already demoted stops working on upgrade, as its docs promise. `TestTheReadTierDiesWithItsOwnerToo` drives every path the fence admits through the real resolver on a probe app: 200 with an admin owner, 403 after demotion (red before the change), restored on re-promotion. User docs: `ops_write` is named in the FAQ's `/ws/events` and admin-gate answers and gets a row in the roles-and-permissions mint table (review I2). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HWna79vVmnezQdhpaGRKbw
|
Thanks for the review, @webmixgamer. Here is what changed for each point. The PR is out of draft now. I1 (an An
A person's start is unchanged. It does not try to tell a person's action from benign drift. A revoke is identifiable (an inactive row matching the container's key), but a person's Tests ( Separately, merging dev surfaced an ent#875 interaction. The viewer tier is an allowlist, and I2 (follow-ups not filed). Two are fixed here and two are filed.
Appendix (5/10, "dies with its owner" reads as permanent). Fixed in the docs. It is intended, and now it is said. §ops_write and the user authentication page state that demotion suspends the key (the row stays active, and re-promoting the owner restores it) and that you revoke it to end it. Draft gate: the browser sign-off is done. A real Chrome, light and dark, against a local stack of this branch merged with current dev:
Dev @webmixgamer @dolho could you take another look? The I1 refusal is the behaviour change to check. |
dolho
left a comment
dolho
left a comment
There was a problem hiding this comment.
Reviewed at bf13b18e7.
I found nothing blocking. An ops_write key gets past the single auth entry point only when three things hold: the (method, routed path) is one of four, the routed handler carries @ops_write_route, and the key's owner is still an admin. Both places where a start mints the agent a key are now refused for this scope. Minting the key needs an interactive admin session. Agent, connector, portal_delegate and ops principals cannot mint one, because the route fence refuses them and the mint handler's own rungs refuse them again. Every gate I mutated turned at least one test red. Everything webmixgamer raised is either fixed in the code or filed as a follow-up. The CodeQL alert was dismissed correctly. The open items are known and tracked: reset and terminate write no audit row yet (#4035), terminate and the owner-tier grant routes have no second layer behind the fence (#4095), and keys cannot expire yet (#4094).
Earlier review (webmixgamer, 2026-10-28, at 7ab23eac0)
- I1: an
ops_writestart could undo containment by minting the agent a new key. Fixed, and the fix is wider than the suggestion.start_agent_internalnow takesallow_key_mint, and the route passesFalsefor this scope (routers/agents.py:903:allow_key_mint=getattr(current_user, "mcp_scope", None) != OPS_WRITE_SCOPE,). A start refuses with 409ops_write_would_mint_keybefore any side effect, on both paths that mint a key. One is the #1559 missing-container rebuild (lifecycle.py:414:_refuse_key_mint("container_missing")). The other is the #1854 drift heal (lifecycle.py:433:_refuse_key_mint("key_drift")). I checked for other mint sites:db.create_agent_mcp_api_keyappears in the lifecycle module only insiderecreate_missing_container(lifecycle.py:1782), andheal_agent_mcp_key_envonly behindif not mcp_key_match. Both are now gated. Mutation M4 below turns 7 tests red. - I2: follow-ups not filed. Addressed.
- The
opsdemotion docs-vs-code gap is fixed in code.dependencies.py:941adds_enforce_ops_key_owner(user), which runs ahead of theopsfence. Mutation M3 turns 3 tests red. - The user-doc gaps (FAQ,
/ws/eventsrow, mint row) are fixed. - Key expiry is filed as #4094 (open).
- The second layer is filed as #4095 (open).
- The
- Appendix (demotion suspends the key, it does not revoke it). The docs now say so.
test_demotion_suspends_the_key_and_re_promotion_restores_itpins the behaviour. - CodeQL alert 460 (
py/weak-sensitive-data-hashing,tests/unit/test_3180_ops_write_scope.py:435) is dismissed as a false positive, and I agree. The line isdigest = hashlib.sha256(fresh.encode()).hexdigest(). It hashes a freshly minted MCP key to find itsmcp_api_keysrow, the same lookup production uses. It is not a password.
What I ran
All runs used a detached worktree at bf13b18e7, with tests/.venv and -p no:randomly. The merge-base is 2d8b2c385. git merge-tree against the current origin/dev (6c7818cb8) merges cleanly. The PR has no schema change, so neither migration track nor the Alembic head is involved.
unit/test_3180_ops_write_scope.py+unit/test_3180_ops_write_route_census.py: 81 passed (serial, 11 min).- Auth guards:
test_293_admin_gate_rejects_agent_keys,test_1310_auth_consolidation,test_1310_auth_wiring,test_2323_machine_identities,test_2996_human_only_routes,test_2996_route_census_runtime,test_3102_host_header_fences,test_ent875_viewer_tier,test_186_enumeration_uniformity,test_mcp_key_creation_requires_session,test_ent873_audit_client_ip,test_2094_dependency_path_param_pairing: 407 passed (-n 8 --dist loadfile). - Frontend:
mcpKeysOpsWriteScope.mount.spec.jswith the raw-color and loading-gate ratchets: 23 passed. The spec mounts the component under jsdom. ItstoContainassertions read rendered text, not source. - MCP server:
src/tool-visibility.test.ts: 29 passed. This includes the new cases checking that anops_writesession and anopssession can neither list nor call an operator tool.
Mutations. Each was applied alone in a second worktree, which was restored after every run and is clean at the end.
| # | Mutation | Suite | Red |
|---|---|---|---|
| M1 | drop the _enforce_ops_write_key_fence(request, user) call |
scope | 6/51 |
| M2 | drop the owner role != "admin" check in that fence |
scope | 3/51 |
| M3 | drop _enforce_ops_key_owner(user) (the new ops owner check) |
scope | 3/51 |
| M4 | start always passes allow_key_mint=True |
scope | 7/51 |
| M5 | reset back to require_admin_allowing() (no ops_write opt-in) |
scope | 3/51 |
| M6 | drop the target-policy call in enforce_agent_spawn_scope |
scope | 10/51 |
| M7 | viewer_principal_kind without OPS_WRITE_SCOPE (my addition) |
ent875 + scope | 5/110 |
| M8 | the mark check in the fence never refuses | scope 0/51; census + 3102 | 2/54 |
M8 survives the scope suite, which is expected: the census catches it, through test_with_the_path_list_holed_every_unmarked_handler_still_refuses and test_nothing_marks_a_handler_any_other_way.
Execution coverage
| changed symbol | executed by | live consumer | verdict |
|---|---|---|---|
_enforce_ops_write_key_fence |
scope + census (real main.app, real keys) |
dependencies.py:946, in the resolver |
executed |
_enforce_ops_key_owner |
TestTheReadTierDiesWithItsOwnerToo |
dependencies.py:941 |
executed |
_refuse_ops_write_protected_target |
TestTargetPolicy |
dependencies.py:1781 (start/stop) |
executed |
allow_key_mint / _refuse_key_mint |
TestContainment |
routers/agents.py:903 |
executed |
require_admin_allowing("ops_write") on reset |
TestPassPaths::test_reset |
routers/agents.py:1151 |
executed |
viewer_principal_kind (ops_write counted as a key) |
ent875 + terminate pass path | routers/chat.py terminate |
executed |
census inspect.getsource over the admitted handlers |
n/a | the route table | guard (paired with the live sweep) |
Checks against the auth invariants
- Reach beyond the four writes. The path list (
_OPS_WRITE_ALLOWED_ROUTES) is anchored andfullmatched againstrequest.scope["path"]. The mark is read fromscope["route"].endpoint; FastAPI'sAPIRoute.matchessetschild_scope["route"] = self. The doors that check a key themselves, not through the resolver, are allowlists that leave this scope out:/ws/events:WS_EVENT_STREAM_SCOPES = frozenset({None, "user", "agent", "system"}).- heartbeat, the #1083 result callback and the pull seams:
authorize_heartbeatrequires a matchingagent_name. - MCP tools:
OPERATOR_SCOPESis unchanged.
- Admin gates.
ADMIN_GATE_SCOPESis unchanged. Reset is the only opt-in, andassert_adminstill checkscurrent_user.role != "admin". The diff adds norequire_role("admin"). - Mint is grant-side and human-only.
POST /api/mcp/keysisDepends(require_interactive). Forops/ops_writethe handler then callsreject_non_interactive_principal,reject_agent_principalandassert_admin._USER_CREATABLE_SCOPESandPERSON_KEY_SCOPESboth gainops_write, and a test pins the two sets equal. Revoke and delete of a key are held only by the fence today; that is #4095. - Idempotency (Invariant #18). Not applicable: none of the four routes creates an execution.
# mcp:headers. No new router. The changed routers keep their existing headers.
Findings
Blocking
None.
Medium
None.
Low
L1. Two of the four writes are unattributed until #4035 lands (Confidence: 9/10)
In routers/agents.py, start (:912, event_action="start") and stop (:1007, event_action="stop") write audit rows. reset_circuit_breaker_endpoint and terminate_agent_execution write none, for any principal. #3180 asks for writes "Attributed in audit_log". So if this PR merges first and the Fixes #3180 keyword is left in place, the issue closes at the next release cut with two of the key's four writes leaving no mcp_key_id trail. The body already commits to moving the closing keyword. The fix is to do that before merge: either land #4035 first, or switch this PR's first line to Related to #3180 and let #4035 carry Fixes.
L2. The ops owner check changes a shipped tier and needs a release note (Confidence: 7/10)
dependencies.py:941, _enforce_ops_key_owner(user). After upgrade, an existing ops key whose owner is no longer an admin starts returning 403 ops_owner_not_admin on the 10 reads that the fence alone used to answer. That is the documented behaviour and not an escalation. But an operator whose monitoring runs on such a key loses it silently at upgrade, unless the release notes say so. Name it in the next release's notes, and say what to do: re-mint the key under a current admin.
tests/registry.json: both sides add entries; kept both, this slice's three first, dev's side unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HWna79vVmnezQdhpaGRKbw
…in (#3180) The ops owner check in the resolver changes the shipped ops tier (#2323): after upgrade, an active ops key whose owner is no longer an admin gets 403 ops_owner_not_admin on the reads the route fence alone used to answer. Operator note in docs/migrations, the form the release notes' upgrade section links at the cut: what changes, how to list the affected keys on SQLite and PostgreSQL, and the fix (re-mint under a current admin, then revoke the old key). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HWna79vVmnezQdhpaGRKbw
|
Thanks for the review, @dolho. Both Low items are addressed. Here is what changed for each. L1 (two of the four writes are unattributed until #4035 lands). Fixed: no slice closes #3180 now, and the merge order is stated. This PR's first line is now L2 (the Release notes are written at the cut, and nothing in
The link from the release notes is still the cut's job. The bullet I would add under "Behaviour that changes by default" in the next
The query was checked against a scratch SQLite with the real column names. The PostgreSQL spelling is the same SQL, and Dev |
Part of #3180
This is slice S1 of two. Neither slice closes #3180: with both on
dev, the issue's "optionally expiring" key is still missing, and that is #4094 (open). The audit slice, #4035, is where circuit-breaker reset and execution terminate write their audit rows, so it merges before this PR or in the same train; otherwise two of this key's four writes leave no trail. The two slices share onlytests/registry.json.Journey Impact: none: adds an admin-minted machine key scope that reaches four existing routes; no first-run, sign-in or end-user journey changes path
Gating: OSS-core, nothing behind the enterprise seam, by the ruling recorded on #2323 for the same problem: the credential is the edition-agnostic core primitive and the two-factor policy is the enterprise knob, the shape of
users.suspended_at(#995) and the connector scope. The read tier (#2389) shipped that way on this same code, and #3180 was filed on the public tracker as its write tier. If any part is meant as a paid module, say so in review.Browser sign-off (2026-10-30), done: a real Chrome, light and dark, against a local stack of this branch merged with dev
2d8b2c385. Settings → MCP Keys: the Ops (operational writes) choice in the create form and the Ops (writes) badge on two minted keys. An agent's Trinity access key panel, with its container's.mcp.jsonauthenticating with anops_writekey: the verify verdictforeign_ops_write_keyand its label, as a warning, with no "person's key" wording, also on re-verify.What it does
ops_write, the write sibling of the read-onlyopsscope. It is admitted at the single auth entry point only when the (method, routed path) is one of four, the routed handler carries@ops_write_route("<why it is a use>"), and the key's owner is still an admin:POST /api/agents/{name}/circuit-breaker/reset,POST /api/agents/{name}/start,POST /api/agents/{name}/stop,POST /api/agents/{name}/executions/{id}/terminate. They are the measured, executed operational writes of the ops toolkit that are uses rather than grants.require_admin_allowing("ops_write")(oneADMIN_WIDENEDcensus row);role == "admin"still runs.ADMIN_GATE_SCOPESis unchanged and nothing saysrequire_role("admin").opskey now dies with its owner on every route too. The docs said demotion stops it, but theopsfence reads only the method and path, so the reads with no admin gate of their own (version, monitoring, telemetry, the agent roster, the execution reads, subscriptions: 10 of the 17 probed) kept answering a demoted owner's key. The resolver now checks the owner's role foropsahead of the fence, as it does forops_write(403ops_owner_not_admin). This changes the shippedopstier (feat: machine identities for admin/ops APIs — service credentials that survive enforced 2FA #2323): a key whose owner was already demoted stops working on upgrade, which is what its docs promise. A demoted owner could still read those same endpoints with their own session, so this was a docs-vs-behaviour gap, not an escalation. The upgrade note isdocs/migrations/OPS_KEY_OWNER_ADMIN_2026-10.md: how to list the affected keys on SQLite and PostgreSQL, and the fix, which is to re-mint each one under a current admin and revoke the old one. The next release's upgrade notes link it.reason: key_drift, on a stopped or running agent; the bug(recovery): soft-delete recovery dead-ends —recovertells you tostart, butstart404s when the container is gone #1559 rebuild of a missing container,reason: container_missing) is refused with 409ops_write_would_mint_keybefore any side effect, and the message says a person has to start the agent. A person's start is unchanged. A person's delete of the key cannot be told from the platform's own prune (both leave no row), so every minting start is refused, benign drift included. That costs availability only: a healthy agent's predicate matches, and a drifted agent needs one person's start.opsladder). Settings → MCP Keys offers "Ops (operational writes)" and badges the key "Ops (writes)". No MCP tools, no event stream.requirements/security.md§20.10,requirements/auth.md,architecture/security.md, the MCP keys feature flow and index, the user API authentication page, and the two user FAQ pages whose scope counts and lists this change made false (faq/mcp-and-api.md,faq/security.md). An operator note for theopschange on upgrade:docs/migrations/OPS_KEY_OWNER_ADMIN_2026-10.md. The audit-logmcp_scopefilter's OpenAPI description lists the new value.tests/unit/test_ent873_audit_client_ip.py, fromdev). Its agent-refusal case overrodemain.get_current_user. The unit conftest evictsdependenciesafter each test, so once this PR's suites have importedrouters.agentsbeforemain,mainholds a newer copy than the one the route was built with. The override was then never consulted, and the call answered 401 instead of 403. That made the case red whenever one of this PR's suites ran first on the same worker; it was green alone. It now takes the dependency from the route's own globals, and its assertion is unchanged.Not delivered
user-scoped admin key: fleet restart; retention acknowledge; schedule edit and toggle; schedule trigger; agent resources, capacity and timeout; every platform setting; subscriptions; user roles; the login whitelist; capability grants and skill managers; agent MCP-key regeneration; credential injection; creating and deleting agents; skill-library sources; chat and task dispatch; system-agent restart (and start/stop of the system agent); start/stop of ephemeral agents; emergency stop, fleet stop, schedule pause/resume; monitoring enable/disable; notification acknowledge/dismiss; health-check trigger. The ops toolkit'supdateandsubscriptionflows therefore keep a user-scoped key.ops,ops_write), the "optionally expiring" part of #3180 #4094.userkey gets there. A second layer there, or a resolver that refuses any scope without a registered fence, is a follow-up:ops_write: a second layer on terminate and the owner-tier grant routes, beside the route fence #4095./api/mcp/validatereports the key as valid (introspection) even after its owner is demoted or suspended; it grants nothing.mcp_key_idandmcp_scope=ops_write).Collisions
#3762 (
controllerscope, draft) and #3778 edit the same anchors (the resolver's fence block,_USER_CREATABLE_SCOPES, the mint scope check,McpKeysTab.vue,terminate_agent_execution,stop_agent_endpoint). Whichever lands second re-merges and re-runs both never-grantable sweeps: a sibling fence that returns early must not skip this one.Dev's ent#875 viewer tier (#3777) is an allowlist that classified
ops_writeas "other", so terminate 404'd on any turn its owner had not started. Fixed ina4b07a715by classifyingops_writeas a key tier, pinned intest_ent875_viewer_tier.py.The audit slice, #4035, shares one set of lines with this PR:
tests/registry.json, where each slice prepends its own entry. Whichever merges second re-merges and keeps both entries. A local integration branch of both slices onorigin/devconflicted there and nowhere else, resolved by keeping both.Validation: at 9d599ff, full backend unit tier as CI runs it (
pytest unit/ -m "not slow" -n auto --dist loadfile -p randomly) on seeds 12345 / 67890 / 99999: 29,396 / 29,397 / 29,396 passed; every #3180 test green on all three. The reds are not this slice's: fourtest_ent679_payments_pin_paritycases (local venv package versions vs image pins; the same four red on pristine f754dfb) andtest_2915 … test_mark_unconfirmed_sweeps_only_the_named_scopeon two seeds (green alone, green alongside these suites in one process, on head and base; cause traced to xdist workers sharing one unit SQLite file, follow-up drafted). New suites:test_3180_ops_write_scope.py+test_3180_ops_write_route_census.py68 passed (the census drives every live OSS route of the realmain.appwith real keys, 684 routes classified by credential dependency); the guard files (test_2323, test_293, the #2996 census and its runtime sibling, test_1310_auth_wiring/consolidation, test_3102, test_837 ×2, test_3825, test_163, test_mcp_key_creation_requires_session) 449 passed; frontendnpm run test:unit412 files / 6,446 tests incl. the raw-colour and loading-gate ratchets; MCP server 29 passed andtsc --noEmitclean;tests/lint_sys_modules.pyno new violations; gitleaks over the branch: no leaks.Re-run at e5c21b2 (after the fresh-review fixes):
test_3180_ops_write_scope.py+test_3180_ops_write_route_census.py68 passed; the audit-log suites 61 passed.Then
origin/dev0f689db was merged into the branch, cleanly and as a merge rather than a rebase. Dev's enterprise submodule pointer bump (#3995) had made this branch's unchanged pointer read as a change. On the merged head 7ab23ea, 65 files ran under-n 4 --dist loadfile -p randomlyon seeds 12345 and 67890: the two suites, the pinned guards, the terminate/reset neighbours, and dev's new sign-in and client-address files. Both seeds: 1,540 passed, 3 skipped.tests/lint_sys_modules.py: no new violations.Batch: both slices of #3180 merged with
--no-ffonto one integration branch offorigin/devf754dfb (the only conflict:tests/registry.json, both entries kept, 573 entries, no duplicate). Full backend unit tier (tests/unit -n 4 --dist loadfile -p randomly) on seeds 12345 / 67890 / 99999: 29,425 / 29,425 / 29,426 passed, 5 failed each; all 5 reds reproduce on pristineorigin/devwith the same seed (fourtest_ent679_payments_pin_paritycases: local venv package versions vs the image pins;test_2850_skill_allowed_tools_forms::test_warning_truncates_the_offending_value: a log-line length that embeds the xdist tmp path), none introduced.tests/lint_sys_modules.py: no new violations. Frontendnpm run test:unit: 412 files / 6,446 tests, ratchets included. MCP server: 1,002 passed,tsc --noEmitclean. The tier ran with this slice at 9d599ff; the four review-fix commits after it (docs, one OpenAPI string, two test files) re-merged cleanly and were re-run with both slices' suites, the guards and the terminate suites on the integration head: 1,158 passed, 2 skipped on seeds 12345 and 67890 (57 files under-n 4 --dist loadfile -p randomly; the three #3180 suites 38 / 30 / 34 and every pinned guard counted in the junit),tests/lint_sys_modules.pyno new violations. The full tier, the frontend and the MCP suites were not re-run after the fixes: no frontend or TypeScript file changed, and the one behaviour change in the batch since the tier (the audit slice's container-arm CAS gate) is covered by the terminate suites of all three of its callers in that set. After dev moved to 0f689db, this branch's merged head was merged into the same integration branch, cleanly, giving dev plus both slices. The same targeted set plus the audit slice's suite (66 files) passed 1,574, with 3 skipped, on seeds 12345 and 67890. The three #3180 suites counted 38 / 30 / 34 in the junit and the ent#873 file 7, none failed.Mutation: each gate removed in an isolated copy of HEAD, red-in-K/N of the named files, then the restored tree (byte-identical to HEAD) 198/198 green: fence call 15/68; path check 8/68; mark check 2/68; owner-admin check 4/68; reset's
allow_scopes3/157; the mark on reset 7/68, start 9/68, stop 12/68, terminate 7/68; target policy call 10/38, its fail-closed lookup 1/38, itsis_systemarm 2/38;_USER_CREATABLE_SCOPESentry 28/38;PERSON_KEY_SCOPESentry 2/38; mint rungs in isolation — in-handler interactive 1/38, agent 1/38, admin 3/38, router scope admission 30/38 (the route-levelrequire_interactiveon the mint route predates this slice and is pinned statically bytest_2996_human_only_routes; a behavioural mutation of it goes red here only through the test's dependency override, so it is not counted); verify verdict 1/38 (re-run at e5c21b2 through the probe path: 1/38); fence readingrequest.url.path3/62; theADMIN_WIDENEDrow 2/89; an overbroad path entry 7/30; pull-seam key checks 1/30 and 1/30. UI: badge, radio and scope string each 1/3 red, restored 3/3. MCP:ops_writeadded toOPERATOR_SCOPES2 red, restored 29/29.Since the draft.
origin/devb900357 was merged (890d8bdb4; conflicts independencies.py,routers/agents.py,routers/chat.pyandtests/registry.json, both sides kept; the ent#875 semantic conflict fixed ina4b07a715), the containment fix landed (d845cc57fdocs,cba065883fix,45bd068cbtest), and dev2d8b2c385was merged (0618ea368; onlytests/registry.json, both sides kept). Then theopsowner check and the user-doc gaps (bf13b18e7):TestTheReadTierDiesWithItsOwnerToodrives the real resolver over a probe app that mounts every path theopsfence admits; before the change a demoted owner's key got 200 there, after it 403 on every one, and re-promotion restores it. The new refusal was red before the change: 7 failed, each a 200 with a re-minted key. Per-arm mutants, each alone:container_missingarm removed → 1 failed;key_driftarm removed → 6 failed; route alwaysallow_key_mint=True→ 7 failed; alwaysFalse→ 2 failed (a person is never refused); restored → 48/48. At45bd068cb, the full unit tier (seed 12345): 30625 passed, 1 failed, a load-induced timeout intest_1898_sys_modules_isolationthat passes alone in 3.46 s and touches no changed code. At0618ea368, the 90 files that reach the changed code (the three #3180 suites, everystart_agent_internalcaller, the pinned auth guards, ent875, ent926, the terminate/reset neighbours; seed 12345): 2165 passed, 4 skipped, 0 failed; frontendmcpKeysOpsWriteScope+ both ratchets: 23 passed. Atbf13b18e7, the same set widened to every file naming theopsscope (115 files, seed 67890): 3533 passed, 4 skipped, 0 failed. Then dolho's review: the closing keyword moved to #4035 (L1), and theopsupgrade note landed inb07fa2917(L2). Dev0b11bd501is merged (8ae68ae50; onlytests/registry.jsonconflicted, both sides kept, 619 entries, no duplicate). On that head, the two #3180 suites, the files that read the route census (_route_census.pygained dev's retrieval-memory rows), the files that cover dev'srouters/chat.pychange (#3998, ent875),test_3102and the registry's one reader (test_2338_journey_catalog), 10 files under-n 3 --dist loadfile: 526 passed, 0 failed (scope 51, census 30).Fresh review: an independent reviewer (Claude Opus 5.5, extra-high effort, briefed with the slice's goal, the plan's proof list and the bar only) re-ran the gate mutations at 9d599ff and every one went red (fence call 15/68, path check 8/68, owner check 4/68, target policy 10/38,
_USER_CREATABLE_SCOPES28/38, among them). The code and tests held. Its verdict was REVISE for docs: one blocking and two should-fix findings, plus four nits. All seven are closed. The blocking one: the requirements doc said every audit row the key causes carries its id, but the start-time self-heal row is written assystem; this is fixed in 4a11d88. The containment advice could not be followed, since an admin reaches every agent; it now says delete, revoke or demote, also in 4a11d88. The FAQ scope counts and lists are fixed in 891b3d3. Of the nits, the OpenAPI filter text is fixed in b1efd08. A/wsprobe that could not fail is dropped, and the verdict test now goes through the probe path; both are in e5c21b2. The mint mutation that went red only through a test override is no longer counted (above).Unverified: enterprise routes and any enterprise-only scope (submodule not mounted: the never-grantable proof is OSS-only); PostgreSQL (the suites ran on SQLite only, and they are not
requires_postgres, so CI's PostgreSQL tier does not run them either); the four handlers against a real container (Docker faked; on start the config predicates other than the MCP-key one were pinned to 'matches'); the pinned FastAPI 0.115.6 / Starlette 0.41.3 forscope["route"]in the real app (the unit tier ran 0.115.14 / 0.46.2; the plan's probe covered the pins on a scratch app); the real MCP server's authenticate →/api/mcp/validate→ session-scope path end to end (the refused-call proof boots a real FastMCP server with the real predicates); the consumer's measured write counts (regex counts over deploy records, 3–4 per route read by hand); whether any automation revokes keys or injects credentials with asystemkey; whether #3762 or #3778 merges first; whether any part is meant as a paid module (inferred from the #2323 ruling and the public filing, not asked separately); the inline sign-in paths' MFA/suspension gap (out of scope); whether a started agent's enabled schedules then fire on their own; the dev-tooling test catalog entry (lives in another repository).