fix(agent-mesh): preserve full sub-resource path in capability parsing#3246
Draft
liamcrumm wants to merge 2 commits into
Draft
fix(agent-mesh): preserve full sub-resource path in capability parsing#3246liamcrumm wants to merge 2 commits into
liamcrumm wants to merge 2 commits into
Conversation
CapabilityGrant.parse_capability truncated the qualifier to a single segment (parts[2]), silently dropping parts[3:]. A leaf grant such as write:database:table_users:row_1 was stored/compared as write:database:table_users, so it authorized its parent (write:database:table_users) and its siblings (write:database:table_users:row_2) -- a privilege escalation (#3180). This is the 4+-segment variant and is independent of #3176 (which fixed the 3-segment matcher under-restriction); it lives in the parser. Fix: - parse_capability now preserves the full remainder: qualifier = ":".join(parts[2:]). matches() needs no change because it already exact-matches qualifier and handles broad->narrow via the requested.startswith(capability + ":") branch. - Add a model_validator(mode="before") that re-derives action/resource/qualifier from `capability` on every construction and revalidation, so a grant deserialized/constructed with a stale truncated qualifier cannot re-open the escalation. create() no longer passes the derived fields. - Spec AGENTMESH-TRUST-COORDINATION-1.0 sec 8.2/8.5 updated: qualifier is the full sub-resource path compared as one opaque token. Behavior (before -> after) for a write:database:table_users:row_1 grant: - exact leaf : True -> True - parent : True -> False - sibling : True -> False Tests: dedicated 4+-segment matrix in test_coverage_boost.py (exact leaf, parent, siblings, deeper nesting, resource_ids, delegation, registry check, get_capabilities/deny, wildcard + empty-segment grammar pins, validator self-heal) plus retained #3176 regressions. Full agent-mesh suite green (pre-existing agentrust_trace env failures unrelated). Out of scope: integrations/mcp/_check_capability is a separate, stricter matcher (exact + trailing wildcard) and is not vulnerable. Closes #3180 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Liam Crumm <liamcrumm@gmail.com>
PR Review Summary
Verdict: AI review comments are untrusted advisory output. The summary reports workflow-generated completion status only, not model-authored pass/fail claims. |
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
…rity audit doc Follow-up hardening from a deep-code-review pass on the #3180 fix: - Switch CapabilityGrant's derive validator from mode="before" to mode="after" and enable ConfigDict(validate_assignment=True). The previous mode="before"/dict-only guard left action/resource/qualifier stale when a grant was built from a non-dict mapping (UserDict) or mutated via attribute assignment, so matches() could re-authorize the parent of a leaf grant. action/resource now carry defaults and are always re-derived from `capability`. Documented residual: pydantic model_copy(update=) does not run validators; callers must use create() to re-scope. - Add docs/security/audits/2026-07-02-capability-subresource-path- preservation.md, required by scripts/ci/security-audit-required.sh for any change under agentmesh/trust/ (matches the #3176 precedent). - Add regression tests for the reassignment and non-dict-mapping re-heal paths. Full agent-mesh suite: 3426 passed, 73 skipped (4 pre-existing agentrust_trace env failures unrelated). ruff clean on changed source. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Liam Crumm <liamcrumm@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
CapabilityGrant.parse_capabilitytruncated the qualifier to a single segment, so a leaf capability grant authorized its parent and its siblings. This preserves the full sub-resource path so a narrow grant only authorizes what it names. Closes #3180.Problem
parse_capabilitydidqualifier = parts[2] if len(parts) > 2 else None, discardingparts[3:]. A grant scoped towrite:database:table_users:row_1was stored and compared aswrite:database:table_users, so it authorized:write:database:table_users, andwrite:database:table_users:row_2.This is the 4+-segment variant and is independent of #3176 (which fixed the 3-segment matcher under-restriction). It lives in the parser, not the matcher, and reproduced on the pulled tree.
Reproduction (before -> after)
For a
write:database:table_users:row_1grant:write:database:table_users:row_1TrueTruewrite:database:table_usersTrueFalsewrite:database:table_users:row_2TrueFalseSecurity model (blast radius)
This is a capability data-model change; the chosen approach and its blast radius:
parse_capabilitynow preserves the full remainder viaqualifier = ":".join(parts[2:]).matches()is unchanged because it already treatsqualifieras an opaque exact-match token, and the broad-satisfies-narrower direction is handled by the separaterequested.startswith(capability + ":")branch (which reads the untruncatedcapabilitystring).model_validator(mode="before")re-derivesaction/resource/qualifierfromcapabilityon every construction and revalidation (includingmodel_validate/deserialization). Without it, a grant built asCapabilityGrant(capability="a:b:c:d", qualifier="c")would keep a stale truncated qualifier and re-open the escalation, sincematches()trusts the stored qualifier. No such path exists in the tree today, but this makes the model correct-by-construction and satisfies the "no half-migrated comparison paths" requirement.*(action/resource) and a trailing:*are wildcards. A*in the middle of the remainder (e.g.write:database:*:row) is now a literal qualifier segment (stricter / fail-closed vs the old accidentally-permissive parse). Empty/trailing-segment behavior (write:database:) is pinned.matches()to a variable-length component list (larger blast radius on the security-critical matcher, no behavioral gain).integrations/mcp/__init__.py::_check_capabilityis a separate, stricter matcher (exact + trailing wildcard only), never callsparse_capability, and is not vulnerable.Maintainer decision points
action:resource[:qualifier]spec grammar to treat the qualifier as a full colon-containing remainder. Spec sec 8.2/8.5 were updated to document this. Alternative (b) would instead reject >3 segments.Changes
agent-governance-python/agent-mesh/src/agentmesh/trust/capability.pyparse_capabilitypreserves full remainder; addmodel_validatorre-deriving components fromcapability;create()no longer passes derived fields; docstrings updatedagent-governance-python/agent-mesh/tests/test_coverage_boost.pyTestCapabilityFourSegmentEscalationmatrix: exact leaf / parent / siblings, deeper nesting,resource_ids, delegation (require_grantor_capability), registrycheck(),get_capabilities/deny, wildcard + empty-segment grammar pins, validator self-heal, retained #3176 regressionsdocs/specs/AGENTMESH-TRUST-COORDINATION-1.0.mdTesting
True -> Falsewhile exact-leaf staysTrue.pytestfull agent-mesh suite: 3424 passed, 73 skipped; the only failures are 4 pre-existingModuleNotFoundError: agentrust_traceenv failures intests/governance/test_trace_sink.py, unrelated to this change.test_trust.py+test_spec_mesh_trust_conformance.py: all green; fix(capability): reject narrow grants satisfying broad capability checks #3176 and spec S8.1-S8.3 preserved.ruff check --select E,F,W --ignore E501clean on changed lines.