Skip to content

refactor(security): consolidate the eight private MCP in-tool permission-gate copies and add a generic denial sweep #3374

Open

Description

@vybe

Deferred-debt entrys (verbatim from the trinity-dev DEBT_INBOX.md)

debt:2026-09-15-mcp-in-tool-gate-rows-not-consolidated

What: After ent#628 the MCP permission gate has ONE canonical implementation (src/mcp-server/src/access.ts::checkAgentEdge, applied by the registration-time enforce wrapper) — but the eight modules that gated before it (schedules.ts with its read/write split, reports.ts, operator_queue.ts, git.ts via its run wrapper, executions.ts, a2a.ts, agents.ts's three inline getPermittedAgents checks, chat.ts's user branch) still carry their own copy and sit in TOOL_ACCESS_POLICY as in-tool rows. An in-tool row is a claim the table cannot verify; only enforce rows are enforced by the mechanism.
Why deferred: each copy encodes a module-specific rule (schedules: writes are self-only; a2a_call: self-only by design; reports/operator_queue: {self} ∪ permitted on a resolved resource, not a parameter), so consolidation is a per-module semantics decision, not a mechanical rewrite — and ent#628 is a P2 bug fix whose scope is the loop tools.
Context: src/mcp-server/src/access.ts (TOOL_ACCESS_POLICY, withAgentAccess, policyFor), src/mcp-server/src/server.ts::addToolWithAudit. Flip a row from in-tool to enforce:{param} once the module's parameter-addressed tools are confirmed to want {self} ∪ permitted; resource-keyed tools (item/report/loop ids) stay in-tool by shape. Chat's user branch is separately suspect (see the bug filed from the ent#628 gate: literal "admin" username test, global is_shared vs per-user grant).
Interest: the audit table over-states coverage while in-tool rows exist; a regression inside one of the eight copies is invisible to the ratchet, exactly the class ent#628 was.
Effort (AI): M (one small PR per module, or one PR for the parameter-addressed subset)
Source: abilityai/trinity-enterprise#628, /autoplan gate (deferred item 1; both independent reviewers flagged the copies)

debt:2026-09-15-mcp-in-tool-rows-have-no-behavioural-sweep

What: enforce rows are behaviourally tested generically (the wrapper test drives an unpermitted agent session and asserts the wrapped execute never ran). in-tool rows rely on each module's own tests — and executions.ts, reports.ts, operator_queue.ts, a2a.ts have no denial test that asserts the side-effecting client method was NOT called; a gate that denies after the side effect, or whose result is ignored, would stay green.
Why deferred: a generic sweep needs a fake TrinityClient that stubs every mutating method the 30-odd in-tool tools call (or a Proxy that records any call), plus per-tool minimal valid params — a test-infrastructure PR, not part of the loop fix.
Context: src/mcp-server/src/tools/dynamic-agents.test.ts::makeFakeClient is the idiom; a recording Proxy over TrinityClient would generalise it. The sweep iterates TOOL_ACCESS_POLICY rows of kind in-tool, builds each tool via its factory, drives execute with {session: {scope:"agent", agentName:"caller"}} and an unpermitted target, and asserts the proxy recorded no call other than getPermittedAgents/isAgentPermitted/getAgentAccessInfo.
Interest: the 2026-09-15 ledger class ("a per-tool gate is a discipline") remains live for every in-tool row until each has a no-side-effect denial test.
Effort (AI): M
Source: abilityai/trinity-enterprise#628, /autoplan gate (deferred item 2; engineering reviewer F10)


Filed by trinity-pm /groom 2026-10-08 (Debt Health & Inbox Triage). Grouped: 2 inbox entries fold into this one issue — see the debt: keys above.

Activity

  1. added
    complexity-mediumComplexity: medium (board points 5-8)
    status-readyGreenlit and ready for development (vetted; counterpart to status-incubating)
    on Oct 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    complexity-mediumComplexity: medium (board points 5-8)priority-p2Importantstatus-readyGreenlit and ready for development (vetted; counterpart to status-incubating)theme-securityTheme: Securitytype-refactorCode improvement

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions