fix(auth): an agent key reaches only itself, its edges and the agents it spawned — on the raw backend routes too (trinity-enterprise#629)

[dolho]

Summary

An agent-scoped MCP key resolves to its owner with the owner's role (Invariant #8), so every access and owner check said yes for any sibling the owner holds. On an admin-owned install, that's every agent. The agent_permissions edge the MCP layer checks (P-02) wasn't enforced one hop down. With its injected key and TRINITY_BACKEND_URL, agent A could, for any sibling B: chat with, task, fan out to, or loop it; read its history and logs; replace its git token. The public guardrails page (#3280) says an agent can call only the agents you allow. Operator ruling 2026-10-06: fix the hole.

Narrowed in the shared gate, not per endpoint (the #1890 lesson). dependencies.agent_may_reach(current_user, target, manage=):

Tier	Where	An agent key reaches
use	get_authorized_agent[_by_name], assert_agent_access, A2A inbound	itself · an agent it has an edge to · an agent it spawned with its current key
manage	get_owned_agent[_by_name], assert_agent_owner	itself · an agent it spawned. An edge means "may call", never "may manage".

• Refusals outside the reach get the uniform 404 (SEC: User and Agent Enumeration via differential API responses #186). Humans, user/system keys and connectors are unchanged.
• Capabilities (ent#164 / ent#596) earn reach per request. A capability fence (capability_fence, require_person_or_capability, the skills fence) admits a holder only after the grant check and owner_owns_agent (owner_username equality, never the admin short-circuit). It records the target on the request's principal (User._capability_reach), and agent_may_reach honours that record. So a schedules.manage holder reaches a sibling's schedule routes, an agents.manage holder its reconfigure routes, a skills.manage holder its skills routes, and none of them reaches anything else or another user's agent. Holding a grant is never reach by itself. The files tab applies the same bound per file class (skills / instructions).
• Inline checks: every inline can_user_access_agent / can_user_share_agent call ANDs the reach (AST-guarded). nevermined's admin shortcut no longer skips it. Loops by id check reach before the admin/initiator shortcut. Credential export/import refuse an agent key by name before the owner fence (human_only).
• Operator queue: an admin-owned agent key gets {self} ∪ edges ∪ spawned, not every agent's asks.
• Chat history/sessions: an agent key never gets the admin "every user" view.
• MCP: every baselined: ENT629 row (33 tools) is now enforce. The edge is decided by GET /api/agents/{caller}/reach/{target}: the edge or the spawn record, from the database, never the container list. So a ghost with no container, a child being recreated, and a Docker read fault no longer read as "not permitted".

Behaviour change to know before merging

Any agent that today reaches a sibling without an edge through the raw backend or a now-enforce MCP tool will get 404 / "Permission denied". That's the purpose of the ruling. Spawned children are unaffected. None of the public abilities skills call the backend raw. Private orchestration skills are not audited: they will need an edge for each sibling they act on.

Not in this PR

Fleet-level routes that take no single agent (collection reads such as the agent list, event subscriptions and the activity timeline, plus some fleet-level writes) still answer an agent key with its owner's reach. Some return task text and emails. They're tracked privately as a follow-up (#3346 review); security.md says so.

Test Plan

• tests/unit/test_ent629_agent_reach.py, 41 tests on a real (temp) schema:
  • the reach truth table, and a rotated parent key losing spawn reach;
  • all path helpers and imperative helpers, A2A inbound, and the operator queue (admin- and non-admin-owned);
  • the per-request capability record, including through a real route under TestClient;
  • another user's agent on every capability path;
  • credential human_only, loops by id, the spawned-set query (soft-deleted excluded), the reach endpoint, the chat-history rule;
  • an AST guard on inline checks.
• Mutation checks:
  • reach always true → 14 red; an edge counted as manage → red;
  • each of the review's four surviving mutations (human_only, the non-admin queue bound, deleted_at, the skills owner bound) → red.
• Related suites (ent164, ent596, ent753, 2996, 1310, 186, 293, 2806, 2973, 3435, 3052, loops, operator queue, facade, models): 777 passed + 7 xfailed. MCP: 885/885, tsc clean.
• CI green on the previous head, including journey-smoke (J10 loop refusal) and regression diff.

Fixes abilityai/trinity-enterprise#629

🤖 Generated with Claude Code

[dolho]

Local regression run

I ran 257 unit files: every file that imports or exercises a module this PR changes.

• First pass: 7015 passed, 41 failed. I re-ran the same 41 against this PR's parent commit on dev, and 27 fail there too (ent#679 payments, ent#751 gate callers and others), so those predate this PR. The other 14 were new, and all of them are fixed in 47909a4:

New failure	Cause	Fix
test_ent596_skill_manager ×5	Its recorder patched get_owned_agent_by_name as the skill fence's owner check.	The owner check that a capability dependency uses is its own named function again, get_capability_owned_agent_by_name, and it is not narrowed. The test now patches that name.
the same, files-tab case	A skills.manage holder editing a sibling's skills through the files tab was refused, because there's no edge.	files._may_reach_for_path: a holder reaches a sibling's skills path only. Every other path is narrowed. A test is added for it.
test_1310…anti_exfil_sibling	Still a 403, but the narrowed owner gate fired before the specific message.	share_agent_file checks "an agent key shares only its own volume" first.
test_2806 ×4, test_2973 ×3	Agent A calls sibling B with no edge, which is exactly what is now refused.	Their fixtures seed the A→B edge, as a real deployment needs.
test_ent549 ×1	Flaky: I'd mounted the enterprise module partway through the run.	Passes on re-run.

After the fixes: every affected file passes (ent#596, ent#629, ent#549, #1310, #2806, #2973: 235 passed). The enterprise assignment suites pass too (112). The edge read fails closed, so a fixture schema with no agent_permissions table gets a refusal instead of an exception.

🤖 Generated with Claude Code

[vybe]

merge-train: ejected from this batch — rides the next train once fixed. It conflicts with dev in src/backend/routers/agents.py, src/backend/routers/schedules.py, src/backend/services/agent_service/files.py (and docs/memory/architecture/security.md) — most likely #3236 (ent#164) touching the same handlers. That's a re-apply over someone else's change, so it needs the author rather than a mechanical merge.

[dolho]

Merged dev in c0356f0ba. This answers the merge-train ejection.

Textual conflicts, with both sides kept: the dependencies imports in agents.py and schedules.py, files.py (ours _may_reach_for_path, dev's _touches_instructions), the access.ts table (get_agent_skills → enforce, plus dev's three skill-gate rows), and security.md.

Semantic conflict. The textual merge left 7 tests red. ent#629's narrowed owner gate refused two things that #3236 and #3208 grant:

• test_2996_owner_config_person_only::test_an_agent_holding_agents_manage_changes_the_stored_value ×5: an agents.manage holder reconfiguring its owner's sibling.
• test_ent753_gate_routes ×2: a skills.manage holder setting a sibling's skill gate.

Fix: a capability whose grant is the reach keeps that reach, but only to agents its owner owns (owner_username equality, never the admin short-circuit). That is the same bound _refuse_unless_owners_agent applies:

• agent_may_reach(manage=True) now admits an agents.manage holder on an owner-owned target (_manages_owners_agent, fail-closed). Reads are not widened.
• The skill-gate writes resolve the agent with get_capability_owned_agent, the path-dependency form of the capability owner fence. It runs after require_person_or_capability has already refused a non-holder.
• security.md gets one line stating the rule.

Local: ent164, ent629, ent596, 2996 ×2, ent753 routes, 1310 ×2, 2806, 186 and 293 give 449 passed. MCP access.test.ts + access-wiring.test.ts: 33 passed.

🤖 Generated with Claude Code

[vybe]

merge-train: ejected from this batch — rides the next train once fixed.

The narrowed reach removes cross-agent reach from capabilities that dev grants for exactly that. agent_may_reach (src/backend/dependencies.py:1358) admits a non-self, non-spawned target on the manage tier only via _manages_owners_agent, which honours agents.manage alone — and on the use tier, only via an edge. Verified against a real temp schema (admin-owned holder, sibling with no edge):

• schedules.manage holder → schedule create (routers/schedules.py:218, assert_agent_access) 403; enable/disable/trigger (schedules.py:473/496/519, OwnedAgent) 404. db/capability_grants.py documents schedules.manage as what an agent needs "to change ANOTHER agent's schedules".
• instructions.manage holder → git reset-to-main (routers/git.py:779) and instruction-file writes (services/agent_service/files.py:475/552) 404.
• agents.manage holder → PUT /{name}/model (routers/chat.py:767, use tier) 404, though agents.manage explicitly covers models.
• Edge holders can create a sibling's schedule but not enable it.

CI is green because no test drives these holders against a sibling. The fix is a design call — how a passed capability fence tells the reach gate to stand down (agent_may_reach never sees which capability the route checked; widening _manages_owners_agent to any capability would be wrong) — so it's yours.

Mechanical, same push:

• routers/loops.py:120-122 _check_loop_access returns early on role == "admin" / started_by_user_id == user.id before any reach check — an agent key reads/stops a sibling's loop by id with no edge (every sibling's loop is "started by" the shared owner). Same class as the nevermined fix in this PR; check agent_may_reach(user, loop["agent_name"]) first.
• files.py:157-168 _may_reach_for_path gives a skills.manage holder any agent the owner can access (other users' agents on an admin-owned install), while the new security.md line says "agents its owner OWNS". Pre-existing on dev; fix the code or the doc.
• operator_queue.py:98 does ~2 lookups per held agent on every queue call by an agent key; one "children spawned by my current key" query would do.

Lane C also wants frontend-e2e green on the PR (skipped — no ui label). Everything else held up: heartbeat / result callback / reports / ghost spawning / fan-out / A2A / trinity-system unaffected; 547 tests pass on the PR merged with dev.

[vybe]

merge-train (2026-10-10): ejected — the branch has not moved since the 2026-10-08 ejection and both criticals reproduce on the merged tree: (1) dependencies.py:1346 agent_may_reach / :1363 _manages_owners_agent revoke reach that ent#164 capabilities grant — a schedules.manage holder gets 404 from get_owned_agent (schedules.py:473/496) and 403 from assert_agent_access (schedules.py:218) on a sibling; an agents.manage holder gets 404 on PUT /{name}/model (chat.py:767); (2) routers/loops.py:118 _check_loop_access returns on role == "admin" before any reach check, so an admin-owned agent key reads/stops any sibling's loop by id. Rides the next train once fixed.

[github-actions]

⚠️ Live-instance suite skipped — merge conflict against dev.

Resolve by merging dev locally and pushing the result; the next nightly re-tests.

[dolho]

Answering the 2026-10-08 and 2026-10-10 ejections. dev merged (e6bfacd3b), fixes in 89eb4339e.

Critical 1: capabilities lost their reach. I went with a per-request record instead of widening the gate. capability_fence and require_person_or_capability now record the target they admitted on the request's principal (User._capability_reach, a private attribute), after the grant check and the owner bound (owner_owns_agent: owner_username equality, never the admin short-circuit). agent_may_reach honours that record on both tiers. So:

• schedules.manage holder → a sibling's create (assert_agent_access) and enable/disable (OwnedAgent) pass;
• instructions.manage holder → git/reset-to-main-preserve-state passes;
• agents.manage holder → PUT /{name}/model and the agent_config reconfigure routes pass;
• the reach is for that target on that request only. Holding a grant is never reach by itself, so _manages_owners_agent (which widened every owner check for an agents.manage holder, on every route) is gone.

Manual schedule trigger has no capability fence on dev, so it stays narrowed to self and spawned. If it should count as schedules.manage, that means adding the fence there, which is a separate call.

Critical 2: _check_loop_access. It now checks reach before the admin and initiator shortcuts.

Mechanical items:

• files._may_reach_for_path is bounded to agents the owner OWNS (code fixed to match the doc). It also covers instructions.manage now, needing the grant for each file class a path touches. That was the instruction-file 404 at files.py:475/552.
• Operator queue: the spawned set is one query (db.list_agents_spawned_by_key), using the same key-id rule as the gate.
• Merge: the agents.py activity routes stay deleted (refactor(api): remove nine uncalled routes and the orphan observability store (#3434) #3483). access.ts takes dev's CREDENTIAL_HUMAN_ONLY / KEY_ADMIN_ONLY plus this PR's two enforce rows. The live /api/activities/timeline is a list read, so it's in the existing list-read follow-up.

Tests: 8 new in test_ent629_agent_reach.py:

• fenced holder → sibling reachable on both tiers;
• same target only, same request only;
• agents.manage alone is no longer reach;
• require_person_or_capability records;
• a fence never admits another user's agent;
• a refused non-holder records nothing;
• the loop-by-id refusal;
• the owner bound on the file path for both classes;
• the spawned-set query.

Mutation check: dropping the record in capability_fence turns the first test red. The ent#596 files fixture now seeds the owner row a real deployment has.

Locally, 950 passed and 7 xfailed across ent164, ent629, ent596, 2996 ×4, ent753 routes, 1310 ×2, 186, 293, 2806, 2973, operator-queue, loops, file-path, facade-parity and models-centralized. MCP access + access-wiring: 36 passed, tsc clean.

🤖 Generated with Claude Code

[webmixgamer]

@dolho, I reviewed f9cd83859 as part of 1.0 release testing (trinity-enterprise#847, the agent-reach row).

Verdict: it does what trinity-enterprise#629 asks. The six per-agent reads my release probe made with a sibling's key now all go through the narrowed checks: agent detail, schedules, files, skills, and capability grants on two agents. So do the sibling file downloads. Nothing here blocks the merge. There are three things I'd fix first, some test gaps, a design question, and the description to refresh.

What I ran

• The unit tests around this change: ent629, ent596, ent164, ent753 routes, 2996 ×5, 1310 ×2, 186, 293, 2806, 2973, ent715, loops, 2094 ×2 and the 837 floor. 739 passed. The two test_2996_route_census_runtime errors come from my local venv, which lacks opentelemetry; they pass on CI.
• MCP access, access-wiring and loops tests: 54/54. tsc is clean.
• 17 hand mutations: 13 caught. Making agent_may_reach always return True turns 14 tests red, so the fix has a real regression test. The 4 that survive are in point 4.

1. skills.manage still reaches another user's agent on an admin-owned install

• The path: get_skill_managed_agent_by_name (dependencies.py:2446) calls get_capability_owned_agent_by_name, which uses narrow=False and so falls back to can_user_share_agent. That returns True for any agent when the owner is admin (db/agent_settings/sharing.py:304).
• I ran a probe on this head with an admin-owned agent key holding skills.manage:
  • get_skill_managed_agent_by_name(<another user's agent>) returns that agent;
  • can_manage_agent_skills (:2269) returns True;
  • files._may_reach_for_path refuses the same target.
• So one capability has two bounds. The dependency's docstring says the grant "never widens reach beyond the owner's own agents", and security.md says the fences record only after owner_owns_agent.
• vybe raised the same point about files.py on 10-08, and it was fixed there only. The gap predates this PR (dev went through get_owned_agent_by_name), but this PR is where the bound is now defined.
• Suggested fix:
  • In get_skill_managed_agent_by_name and can_manage_agent_skills, for an agent principal, call _refuse_unless_owners_agent and then _admit_capability_reach after the grant check, and return the narrowed get_owned_agent_by_name.
  • The skill-gate writes should then pass with get_owned_agent_by_name as well, because _gate_writer resolves first and records the target.
  • That would let get_capability_owned_agent[_by_name] and _owned_agent_or_404(narrow=False) go: one mechanism, and no unnarrowed path dependency left for someone to reuse.
  • A FOREIGN case next to test_a_fence_never_admits_another_users_agent would pin it.

2. MCP enforce reads a Docker-built list, so a target with no container reads as "not permitted" (traced in code, not run)

• checkAgentEdge (access.ts:162) calls isAgentPermitted, which reads the lenient GET /api/agents/{caller}/permissions.
  • The backend builds permitted_agents from get_accessible_agents(), which comes from list_all_agents_fast() (services/agent_service/permissions.py:50).
  • getPermittedAgents (client.ts:984) turns any error into [].
  • Stopped containers are listed; missing ones are not.
• Where it bites:
  • A parent can't delete_agent its own crash-residue ghost (routers/agents.py:744: "must be force-discardable, never 404").
  • It can't start_agent a recovered child (lifecycle.py:394-403 recreates the container).
  • Both tools were baselined before this PR and worked.
• The same path also refuses:
  • a spawned child whose edge was removed, although the backend admits it through spawn provenance (dependencies.py:1547);
  • every one of the 33 rows during a Docker read fault, with "Permission denied … Configure permissions in the Trinity UI".
• Suggested fix: decide the MCP edge from the database (the edge plus the spawn record, i.e. what agent_may_reach answers), not from the container-filtered list.

3. The new security.md line understates what is still open
security.md:59 says: "Collection reads (the agent list, the event-subscription list) are not narrowed: they disclose names, never act."

• Outside this diff, I found fleet-level routes where an agent key still reaches its owner's other agents. Two of them write. Some of the reads return task text and requester emails, not just names.
• I'll file these privately as a follow-up and link it here.
• For this PR: please reword that line so it doesn't promise more than the code holds.

4. Test gaps (mutations that survive)

• Removing dependencies=[Depends(human_only)] from credentials export or import (routers/credentials.py:457, :532) leaves everything green. Only the refusal message changes, since the narrowed owner fence still returns 404 for a sibling, but the last commit has no test.
• Removing reach &= owner_names | {me} (routers/operator_queue.py:96) stays green. The fixture only uses an admin owner, so the non-admin bound is untested.
• Removing deleted_at.is_(None) from list_agents_spawned_by_key (db/agents.py:470) stays green.
• No real-route test sends a capability holder through a capability_fence route (schedules, model, git reset) to a sibling; the fence is only called directly.
  • The per-request record relies on FastAPI handing the same User object to the fence and to the path dependency.
  • One TestClient case would pin that, e.g. a schedules.manage holder enabling a sibling's schedule.

5. Design question: an edge also lets an agent read everyone's chats with the target

• routers/chat.py:829 and :867 drop the per-user filter for role == "admin". So an admin-owned agent key with an edge to B reads every user's history and sessions with B.
• Before this PR that applied to any sibling; now it applies to edge targets only.
• If "may call" shouldn't include "may read everyone's conversations with it", those two lines need the same agent_name check _accessible_set got.

6. PR description

• The stacking note says feat(auth): an agent changes its own shape only with a grant (ent#164) #3236 isn't on dev yet and prescribes narrow=False. feat(auth): an agent changes its own shape only with a grant (ent#164) #3236 is merged, and the design is now the per-request record (89eb433).
• "19 tests" is now 23.
• Two test-plan boxes are unchecked.
• The squash message comes from the description, so it's worth refreshing before merge.

Learnings fragment for the "bound fixed on one of two paths" class. Suggested file: docs/memory/learnings/2026-10-12-capability-bound-one-helper.md:

## 2026-10-12 — pitfall — a capability's owner bound must live in ONE helper, not in each path that honours the grant
**Context**: abilityai/trinity#3346 (trinity-enterprise#629 × #596/#164). `skills.manage` is honoured on two paths: the skills routes (`dependencies.get_skill_managed_agent_by_name`) and the files tab (`files._may_reach_for_path`). A review asked for the owner bound on the files path; it was added there only, so on an admin-owned install the skills routes still admitted another user's agent — `can_user_share_agent` short-circuits True for `role == "admin"`, and an agent key carries its owner's role.
**Lesson**: when a grant is honoured on more than one path, put its bound (grant check → `owner_owns_agent` → record) in one helper every path calls, and test each path with an agent owned by ANOTHER user under an admin owner. `can_user_share_agent` / `can_user_access_agent` are never an owner bound for an agent principal.

Minor: agent_may_reach costs 2–3 queries per call for an agent key, so per-agent loops (autonomy.py:181, evaluations._operator_for) cost N×3 queries for agent keys. Fine as is.

[dolho]

@webmixgamer thanks, all six are addressed in 5af5897d0.

1. skills.manage and another user's agent. Fixed as you suggested:
   • get_skill_managed_agent_by_name does grant, then _refuse_unless_owners_agent, then _admit_capability_reach, and returns the narrowed get_owned_agent_by_name. can_manage_agent_skills takes the same owner_owns_agent bound.
   • The skill-gate writes use get_owned_agent_by_name, because _gate_writer records first.
   • get_capability_owned_agent[_by_name] and _owned_agent_or_404(narrow=False) are deleted: one mechanism.
   • New FOREIGN cases are in ent629 and ent596.
2. MCP edge from a Docker-built list. New GET /api/agents/{caller}/reach/{target} decides from the edge or the spawn record in the database, with no container read. TrinityClient.isAgentPermitted reads it and fails closed. A ghost with no container, a recreated child, an edge-less spawned child and a Docker fault no longer read as "not permitted". It's in the route census and the endpoint catalog.
3. security.md line. Reworded. It now says fleet-level routes still answer with the owner's reach, some write, and some return task text and emails, and that a follow-up tracks them. Thanks for filing that privately.
4. Surviving mutations. Tests added for credential human_only (agent key on a sibling → named 403), the queue's non-admin bound, deleted_at in the spawned set, and a schedules.manage holder enabling a sibling's schedule through the real route under TestClient, which pins the same-principal-object assumption. I re-ran your four mutations; each now goes red.
5. Edge → everyone's chats. I took the narrow reading. An agent key never gets the admin "every user" view on /chat/history/persistent or /chat/sessions, so it reads only its own user's conversations with the target.
6. Description refreshed: per-request record, 41 tests, test plan ticked.

I added the learnings fragment as you proposed (docs/memory/learnings/2026-10-12-capability-bound-one-helper.md).

Locally: ent629 41/41; related suites 777 passed + 7 xfailed; MCP 885/885, tsc clean.

🤖 Generated with Claude Code

[webmixgamer]

@dolho thanks, I checked 5af5897d0:

• Backend: 759 passed. MCP: 121/121, tsc clean.
• My skills probe now refuses another user's agent, on both the skills routes and the files path.
• The mutations for credential export's human_only, the queue's non-admin bound and deleted_at now each go red.
• Dropping the fence's reach record turns 2 tests red, one of them through the real route.

Three things left, the first one red on CI:

1. regression diff fails on this head with one new failure.
   • The test is test_ent754_gate_map_viewer_and_hook::test_probe_is_a_persons_even_for_a_key_that_may_manage_skills[agent_key_with_skills_manage].
   • Its precondition at :276, can_manage_agent_skills(principal, AGENT) is True, is now False. The new owner bound reads db.get_agent_owner, and that fixture only stubs can_user_share_agent (:117).
   • The product behaviour is right; it's the same fixture gap the ent596 files fixture had.
   • I checked locally: as is, 1 failed and 29 passed. With monkeypatch.setattr(db, "get_agent_owner", lambda name: {"owner_username": "owner"} if name == AGENT else None) added to the fixture, all 30 pass.
2. isAgentPermitted swallows the Workspace-only refusal.
   • client.ts:1017 now ends in catch { return false; }, so a workspace_only 403 reads as "not permitted".
   • Its siblings getPermittedAgents and getAgentAccessInfo rethrow it, and access.test.ts:229 pins that.
   • A probe on this head: isAgentPermitted resolves false where getPermittedAgents rejects with workspace_only.
   • So an agent whose owner is a Workspace-only member is told "Permission denied … Configure permissions in the Trinity UI" instead of the account's real situation.
   • Fix: catch (err) { if (isWorkspaceOnlyRefusal(err)) throw err; return false; }, plus a third case in that describe.
3. Credential import's human_only is still unpinned. Dropping it from routers/credentials.py:532 stays green. The new test covers export only.

Optional: /reach/{target} answers true for an edge to an agent the owner can't access, where the old list only held agents the owner could access. The backend still refuses those calls, so only the message differs. Adding and db.can_user_access_agent(current_user.username, target) would keep the old answer.

I'll approve once CI is green. The fleet-level follow-up is filed privately as trinity-enterprise#872.

[webmixgamer]

Approving 2cc17964e: CI is green and the ent754 fixture is fixed. Points 2 and 3 from my last comment are still open: isAgentPermitted drops the workspace_only refusal, and the credential-import guard has no test like the export one. Both are small, and fine as a quick follow-up.

[vybe]

merge-train: batch validated on train/20261012-2333 (#3639)