Repository navigation
fix(auth): the agent-key self-gate is one helper the route census recognises (abilityai/trinity-enterprise#840) - #3929
Conversation
…ognises (Abilityai/trinity-enterprise#840) An agent-scoped key resolves to its owner carrying the owner's role, so the access check answers "may this key reach that agent" (its ent#629 reach), never "is it acting on its own agent". Nine modules hand-copied that second check, invisible to CI; seat_decisions shipped without it (ent#839). - dependencies.assert_own_agent(current_user, agent_name, *, detail, refusal=None): 403 with the route's own message when an agent key names another agent; a no-op for every other principal, like the copies. - 18 routes converted, each keeping its status and message: reminders x3, seat decisions x2, user memory, metric points, reports, canvas writes and deletes x4, canon roles, objectives, shared files, voice reply, outbound A2A call and poll. Canvas helpers keep their full rule. - Route census: agent_callable splits into agent_callable_self (the gate as the first statement, or Depends(get_self_acting_agent), recognised AND listed with why it is a use, not a grant) and agent_callable_any (listed with why a sibling may reach it). Baseline 348 -> 331. - test_1310 refuses a hand-copied gate anywhere under src/backend/. - MCP: agent_name on write_user_memory, the reminders tools and send_voice_reply is described as user-key-only; get_a2a_task matches call_a2a_agent. Behaviour changes, each for a caller refused either way: user memory refuses an out-of-reach agent key with its self message (was "Not authorized", both 403); the A2A kill switch is a dependency, so with it off a schema-invalid or missing body is 404 (was 422); a principal whose agent_name is not a string, or absent, is not treated as an agent key (test doubles only). Mutation: 27 call-site mutants killed. Removing, nesting, misplacing or misaiming a gate reds census rule 8 and that route's sibling case in test_ent840_agent_self_gate.py; a changed message reds its pin; dropping the A2A switch dependency reds test_736's malformed-body test; reverting the helper predicate, the census recognition or a ratchet branch reds the matching self-tests; restoring an override wording reds self-fenced-overrides.test.ts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Abilityai/trinity-enterprise#840) /review on #3929: architecture/security.md (Container Security) still named the AGENT_CALLABLE table that this change splits into AGENT_CALLABLE_SELF / AGENT_CALLABLE_ANY. POST /api/skill-gate/check takes the agent from the key, so it is an ANY entry with no sibling to name. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@AndriiPasternak31, could you review this? It's the route-census class for the agent-key self-gate (abilityai/trinity-enterprise#840, under epic abilityai/trinity-enterprise#318), and it preserves behaviour. One shared |
|
Resolve by running |
|
Resolve by merging |
AndriiPasternak31
left a comment
There was a problem hiding this comment.
Reviewed at 86e97f7. The conversion is right and the census has teeth. One blocker, and it comes from the merge, not the diff.
I ran the 18 routes over HTTP on the merge base (66a8600) and on this head. It's a real FastAPI app; only get_current_user and the reach lookups are faked. Each route got 14 principals: own key, a sibling inside reach, a sibling outside reach, an agent key on a missing agent, JWT owner/shared/stranger, a user key, the system key, a connector bound to the agent, a connector bound to another agent, and the event loopback. 298 of 308 cases match, status and message. The other 10 are the two changes the body lists and nothing else:
POST …/user-memory: an out-of-reach sibling and a missing agent get 403 with the self message instead of 403 "Not authorized" (public_memory.py:71).POST …/a2a/callwith{}and outbound off: 422 becomes 404 for every principal that passes access, not just agent keys (a2a.py:1964).
Mutation check over test_ent840, test_2996_human_only_routes, test_1310, test_2996_route_census_runtime and test_736: red in 10 of 13. All of these go red:
- deleting the gate, moving it off the first line, or aiming it at
reminder_id; - an aliased import;
- a new unlisted route, gated or not;
- a hand copy listed as ANY, or one in a helper;
- moving a self route to ANY;
- dropping the a2a switch dependency.
The three greens are under non-blocking below.
Blocking:
-
The conflict isn't registry-only.
devaddednevermined_delegation.py::get_agent_delegation(#3914), and its self-gate is a hand copy (nevermined_delegation.py:161-163, in the oldAGENT_CALLABLE). I mergeddevinto this head locally:- Taking this side of
_route_census.pyfails rule 1 andtest_no_hand_copied_agent_self_gate. - Moving the entry to ANY still fails the ratchet.
- What passes (166/166 across the three census files) is making
assert_own_agent(current_user, agent_name, refusal=lambda: HTTPException(status_code=404, detail="Agent not found"))the first statement, plus anAGENT_CALLABLE_SELFentry.
Please keep
refusal=there: withdetail=, a sibling inside reach gets 403 where dev answers 404. Theagent_principal_nameimport in that file goes unused after the change.tests/registry.jsonis a plain take-both. Once it's pushed I'll re-check that one route. - Taking this side of
Non-blocking:
- "A self-scoped grant cannot classify itself" only holds until it's listed. A new route with
assert_own_agentfirst and anAGENT_CALLABLE_SELFentry passes everything (249/249). That matches the oldAGENT_CALLABLE, where the reason in review was the guard, so it's fine. I'd word it that way insecurity.md§6 and the census docstring so nobody reads it as a code check. - Ratchet gap outside the stated limit:
if (current_user.agent_name or name) != name: raiseisn't caught, because_agent_name_of(_route_census.py:520) doesn't look inside aBoolOp. The{agent}-parameter miss is the stated limit, so that one's expected. uilabel: nosrc/frontendchanges, so the design-system contract doesn't apply. It's the e2e opt-in, and e2e is green.
I didn't run the MCP tests locally. CI's typecheck + unit run is green.
Conflicts: - tests/registry.json: both sides appended at the tail; kept both (text edit, every entry three-way checked). - tests/unit/_route_census.py: dev (#3914) added routers/nevermined_delegation.py::get_agent_delegation to AGENT_CALLABLE, which this branch splits; it is now an AGENT_CALLABLE_SELF entry. Semantic, from #3914 (merged cleanly, would fail the census): - routers/nevermined_delegation.py: the read's hand-copied agent-key check is now assert_own_agent(..., refusal=...) as its first statement, building the access dependency's uniform 404, so a sibling inside the caller's reach still answers exactly like a missing agent; the unused agent_principal_name import is gone. - tests/unit/test_ent966_delegation_routes.py: reads AGENT_CALLABLE_SELF and asserts the recognised gate carries refusal=. - tests/unit/test_ent840_agent_self_gate.py: pins the route's message. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nary, the census wording names its real guard (Abilityai/trinity-enterprise#840) From the #3929 review: - The ratchet missed a principal's agent name inside an expression that can evaluate to it: `(current_user.agent_name or name) != name`, and the `… if … else …` form. `_agent_name_of` now reads BoolOp operands and IfExp branches; two new spellings in test_1310 (red with the branch removed). All 19 earlier spellings are still found, the 9 benign shapes still spared, and the live tree has no new hit. - "A self-scoped grant cannot classify itself" held only until someone lists it. The census, security.md §6 and auth.md §2.11 now say what is true: the gate alone never classifies a route, so every self-gated route reaches review with a reason, and the reviewed reason is the guard, as it was for the old AGENT_CALLABLE table. - Counts after the dev merge: ten modules carried a hand copy; the Nevermined delegation read joins the converted list. - MCP get_agent_spending: its agent_name says an agent-scoped key may only pass its own name, like the a2a tools. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks, all addressed in
CI is green on |
dolho
left a comment
There was a problem hiding this comment.
Re-reviewed at 3ca51df53. All three items from the previous review are addressed:
- Delegation route:
get_agent_delegationnow opens withassert_own_agent(..., refusal=lambda: HTTPException(404, "Agent not found"))and has anAGENT_CALLABLE_SELFentry. I swappedrefusal=fordetail=locally, andtest_ent966went red on both the in-reach sibling 404 and the census pin, so the uniform 404 is protected. - Wording:
security.md§6,auth.md§2.11 and the census docstring now say the reviewed reason is the guard, not the gate. - Ratchet: I planted
if (current_user.agent_name or name) != name: raiseand agetattr-alias copy that returns a 403.test_no_hand_copied_agent_self_gatecaught both.
The 10 affected unit files pass locally (526 tests), and CI is green.
One nit you can take or leave: docs/security-reports/cso-diff-2026-10-23-840-agent-self-gate.md line 12 still says "A self-scoped grant cannot classify itself" and counts 18 routes and 6 tools. You could update it or add a short note that it predates the review round.
Approving. Before merging, note that dev-ci on dev is currently red on frontend-e2e.
Conflict: tests/registry.json, both sides appended at the tail (dev: test_3940_cron_dst_fold); kept both, every entry three-way checked. The two dev commits are scheduler-only (#3940), so nothing else interacts with the census. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
AndriiPasternak31
left a comment
There was a problem hiding this comment.
Re-reviewed at f4d8ba3f6. All three points are fixed, so I'm approving. GitHub shows a new conflict with dev, and it's bookkeeping only, see below.
Delegation route. get_agent_delegation now opens with assert_own_agent(..., refusal=lambda: HTTPException(status_code=404, detail="Agent not found")) and has an AGENT_CALLABLE_SELF entry. I ran a mutation check over test_ent840, test_2996_human_only_routes, test_1310, test_2996_route_census_runtime and test_ent966. The control run passes (275 passed). Every planted change goes red, 12 of 12:
- dropping
refusal=, which gives a 403; - a
refusalthat returns 403, or 404 with a different detail; - deleting the gate, or moving it below the rate limit;
- going back to
dev's hand copy; - hand copies spelled with
or, a ternary,getattrinsideor, andand; - deleting the census entry;
- reverting the new
BoolOp/IfExpbranches in_agent_name_of.
Wording. The census docstring, security.md §6 and auth.md §2.11 now say the gate alone never classifies a route, and that the reviewed reason is the guard. That's right.
At this head, the 13 auth and census unit files pass (642 passed).
The new conflict. dev moved after your merge. #3935 (ent#717, the email-whitelist gate) took one route out of the frozen baseline, 348 → 347, and edited the same security.md §6 paragraph and the feature-flows.md table. I resolved it locally like this:
_route_census.py:FROZEN_BASELINE_COUNT = 330. The mergedhuman_only_route_baseline.jsonhas 330 entries: your 331 minus the whitelist route. With 331 or 347,test_rule4_the_baseline_count_is_frozengoes red, so the count checks itself.security.md§6: takedev's "Two human-only rules" paragraph, change itsagent_callabletoagent_callable_self/agent_callable_any, then add your new ent#840 paragraph. The two sides changed different words.feature-flows.md: keep both rows.
On that merge, the same 13 files plus test_ent717_whitelist_session_only.py pass (728 passed).
Optional: dolho's nit still applies. docs/security-reports/cso-diff-2026-10-23-840-agent-self-gate.md:12 says "A self-scoped grant cannot classify itself".
I didn't run the MCP tests locally. CI's typecheck and unit run is green.
Conflicts: - docs/memory/architecture/security.md: dev (#3935) extended the "two human-only rules" paragraph; kept dev's paragraph with this branch's class rename (agent_callable -> agent_callable_self / agent_callable_any), then the self-gate paragraph. - docs/memory/feature-flows.md: both sides added an index row; kept both, newest first. - tests/unit/_route_census.py: both sides removed baseline rows (17 here, the whitelist add in #3935); FROZEN_BASELINE_COUNT = 330, the merged baseline's exact size. Checked: #3913's routers/a2a.py change is inbound-path attribution; both outbound handlers still open with assert_own_agent and keep the switch dependency, and no new hand-copied self-gate arrived. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Conflicts (both sides appended): docs/memory/feature-flows.md (kept both index rows, newest first) and tests/registry.json (kept both; every entry three-way checked). #3936 and #3946 add no route this census sees as a hand-copied self-gate and do not reference the old AGENT_CALLABLE table. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…eally is, and what changed after the audit Review nit (both reviewers): "A self-scoped grant cannot classify itself" was an overclaim; the gate alone never classifies a route, and the reviewed reason is the guard. A dated note records the review round's additions (the 19th route, the seventh MCP description, the or/and/ternary ratchet); the audited counts stay as they were at 66a8600. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ec40) (#3995) Three enterprise commits, in order: - Abilityai/trinity-enterprise#963: a retention fix (#3089). - Abilityai/trinity-enterprise#786: a CI workflow change; no product code. - Abilityai/trinity-enterprise#970: the private half of Abilityai/trinity-enterprise#840 — the private tree's one hand-copied agent-key self-gate now calls dependencies.assert_own_agent (#3929), with a private twin of the census guards. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Summary
An agent-scoped MCP key resolves to its owner, carrying the owner's role. So
AuthorizedAgentandassert_agent_accessanswer "may this key reach that agent", which since ent#629 means itself, its permission-edge targets and the agents it spawned. They never answer "is this key acting on its own agent". Ten modules copied that second check by hand, invisible to CI (the tenth, the Nevermined delegation read, arrived fromdevduring review), andseat_decisionsshipped without it (ent#839).dependencies.assert_own_agent(current_user, agent_name, *, detail, refusal=None)returns 403 with the route's own message when an agent key names another agent. It is a no-op for every other principal, exactly like the copies it replaces.refusal=builds the exception instead, for a router whose access dependency answers a uniform 404.dev; it keeps its uniform 404 throughrefusal=).agent_callable.agent_callable_selfis recognised AND listed, theadmin_widenedshape. The handler's first statement isassert_own_agent(<get_current_user principal>, <the path's name|agent_name>), or the signature takesDepends(get_self_acting_agent). Each route also needs anAGENT_CALLABLE_SELFentry saying why it is a use, not a grant. The gate alone never classifies a route, so every self-gated route reaches review with a reason; that reviewed reason, not a code check, keeps a self-scoped grant out, as it did for the oldAGENT_CALLABLE.agent_callable_anyentries now answer, in their own field, why an agent other than the one in the path may reach the route.test_1310_auth_wiring.pyrefuses an inline self-gate anywhere undersrc/backend/. It reads the principal's agent name as an attribute, agetattr,agent_principal_nameor a local bound from one (also insideor/and/… if … else …), on a parameter annotatedUser/CurrentUser(any form), aDependsdefault or an unannotatedcurrent_user. It catches!=,not in, a negated==and== … else: raise, guarding anyraiseor a returned 401/403/404, in handlers and helpers alike. The stated limit: the comparison must be againstname/agent_nameor an alias of one.agent_nameparameter ofwrite_user_memory, the three reminders tools andsend_voice_replyis now described as for a user-scoped key only, thedecisions.tswording. It is no longer advertised as an override a prompt-injected agent may take.get_a2a_tasknow says, likecall_a2a_agent, that an agent key may only pass its own name.Behaviour changes (all for a caller who is refused either way)
POST /api/agents/{name}/user-memory. The self-gate now runs beforeassert_agent_access. An agent key naming an agent outside its reach, or one that doesn't exist, gets 403 "Agent-scoped key may only write its own agent's user memory" instead of 403 "Not authorized". The status is the same, and the reach/existence differential is gone.POST /api/agents/{name}/a2a/calland/a2a/task. The kill switch is now a route dependency, theaauth_person.require_liveshape. With the switch off, a schema-invalid or missing body answers 404 instead of 422, so a disabled route no longer reveals itself that way. JSON that doesn't parse is still 422, because FastAPI decodes the body before solving dependencies. Valid bodies are unchanged.agent_principal_name, the oneagent_may_reachuses. A principal whoseagent_nameis not a string, such as a bare MagicMock, or that has noagent_nameat all, is no longer treated as an agent key. A realUseralways declares the field, as a string orNone.Findings (not fixed here; ruling: behaviour-preserving)
system-scoped key and the event loopback carry no agent name, so they pass every self-gate while resolving to the owner's role. This is for trinity-enterprise#719 to rule on.list_canvases,get_canvas,canvas_context),GET …/reportsand the metrics cross-read (that one is ent#727's deliberate edge rule).share_agent_file. It mints a public download URL and is agent-callable for the agent's own publish dir (FILES-001), listed self with that reason. It belongs to ent#719's sharing cluster.event_subscriptions.emit_event_for_agent. A sibling can emit events as another agent; only the attribution is narrowed.Changes
src/backend/dependencies.py:assert_own_agent, beside the refactor(auth): consolidate duplicated auth/dependency patterns behind factories (INV-8, split from #654) #1310 imperative family.src/backend/routers/{reminders,seat_decisions,public_memory,metric_points,reports,canvas,agent_files,agents,a2a,nevermined_delegation}.py: the conversions.tests/unit/_route_census.py:SelfGate,_self_gate,_current_user_params);AGENT_CALLABLE_SELFandAGENT_CALLABLE_ANYtables;hand_copied_self_gates.tests/unit/fixtures/human_only_route_baseline.json: 17 lines deleted, nothing else (FROZEN_BASELINE_COUNT347 → 330 against today'sdev).tests/unit/test_ent966_delegation_routes.py(fromdev): readsAGENT_CALLABLE_SELFand asserts the recognised gate carriesrefusal=.src/mcp-server/src/tools/{memory,reminders,voice,a2a_call,nevermined}.ts,src/mcp-server/src/access{,.test}.ts: wording, and the owner string that named the deleted_self_gate. Newsrc/mcp-server/src/tools/self-fenced-overrides.test.tspins the wording on the seven tools that takeagent_name.docs/memory/requirements/auth.md: §2.11 (new), §2.7 row, §2.8;docs/memory/architecture/security.md§6, and the census table the skill-gate check names (Container Security);api-endpoints.md;Test Plan
cd tests && pytest unit/test_ent840_agent_self_gate.py unit/test_2996_human_only_routes.py unit/test_1310_auth_wiring.py -v:AGENT_CALLABLE_SELFroute (22 cases), driven off the census, with the caller's own name in every other argument so a gate that compares the wrong parameter fails;unit/test_736_a2a_outbound_call.py::test_with_the_switch_off_a_malformed_body_is_the_same_404.dev(bbd0d4095):unit/test_ent966_delegation_routes.pypasses on the merged tree, including its in-reach sibling 404 over HTTP;test_2996_route_census_runtime.py3/3 against the real app.cd src/mcp-server && node --import tsx --test src/*.test.ts src/tools/*.test.ts: 984 pass on the merged tree,tsc --noEmitclean.66a860094and head fail identically (21 pre-existing on this host:import mainneedsopentelemetry.exporter, plus the knownconftestsubset errors); head adds 69 passing tests.test_2996_route_census_runtime.py: 3/3 in the/verify-localvenv (realimport main). Every storedMETHOD /pathmatches the live app, the two new tables included./verify-local --skip-agent: PASS.test_3262cases caused byTRINITY_DEFAULT_SYSTEM_MANIFEST=disabled, which pass 10/10 without it.import mainin the image: OK. Boot: OK.Eyeball (local stack, on
314cfe527)A is an agent whose own key makes the calls. B is a second agent inside A's reach (a temporary A→B permission edge, removed afterwards). C is an agent outside A's reach.
devboth were 403 "Not authorized".POST …/a2a/callwith{}and outbound off → 404 "Not found"; ondevit was 422 "Field required". Unparseable JSON is still 422json_invalid.self-fenced-overrides.test.tscovers it.Mutation: 30 call-site mutants, all killed (18 before the code-review round, 9 after it, and 3 in the review round). Each was applied to the call site, then reverted with a byte-identical restore:
test_736::test_with_the_switch_off_a_malformed_body_is_the_same_404red;not in, the refusingelse, unannotated principals, anyraise) turns the spelling tests red;test_1310ortest_ent840red;self-fenced-overrides.test.tsred;refusal=fordetail=turnstest_ent966's in-reach sibling 404 red, and deleting its gate also turns census rule 8 and its sibling case red;or/and/ ternary branch turns the two new spellings red.A private submodule PR, abilityai/trinity-enterprise#970 (draft), follows this one. It moves the private tree's one hand-copied self-gate onto the helper and adds a private twin of these guards. Order: this PR, then the private PR, then a gitlink bump.
Fixes abilityai/trinity-enterprise#840
🤖 Generated with Claude Code