fix(security): fix trust level mapping and implement get_agent#53
fix(security): fix trust level mapping and implement get_agent#53
Conversation
GA-007: Fix stubs and trust level mapping - TRUST_LEVEL_UNSPECIFIED (proto enum 0) now maps to '0' (none) instead of '1' (DV). Previous mapping silently elevated unverified badges to domain-verified trust level. - Implement RegistryClient.get_agent() — was silently returning (None, 'not yet implemented'). Now returns agent dict or error.
|
✅ Documentation validation passed!
|
|
✅ All checks passed! Ready for review. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
✅ SDK server contract tests passed (test_server_integration.py). Cross-product scenarios are validated in capiscio-e2e-tests. |
There was a problem hiding this comment.
Pull request overview
This PR addresses two security/registry-client correctness issues in the SDK’s gRPC client layer: preventing silent trust-level elevation when converting badge claims, and replacing a stubbed RegistryClient.get_agent() with a real response-to-dict mapping.
Changes:
- Fix
_claims_to_dict()trust-level mapping soTRUST_LEVEL_UNSPECIFIED (0)maps to"0"instead of"1". - Implement
RegistryClient.get_agent()to return agent data rather than a(None, "not yet implemented")stub.
| # SECURITY: UNSPECIFIED (0) MUST map to "0" (none), never to a verified level. | ||
| # Mapping UNSPECIFIED to DV would silently elevate unverified badges. | ||
| trust_level_map = { | ||
| 0: "1", # TRUST_LEVEL_UNSPECIFIED -> default to DV (1) | ||
| 0: "0", # TRUST_LEVEL_UNSPECIFIED -> none/unverified | ||
| 1: "0", # TRUST_LEVEL_SELF_SIGNED (Level 0) |
There was a problem hiding this comment.
The comment says trust level string "0" means "none/unverified", but elsewhere in this file "0" is used for Level 0 / self-signed (see _trust_level_to_string). To avoid confusion for callers, consider adjusting the wording to clarify that UNSPECIFIED is coerced to the lowest trust level (Level 0) rather than introducing a new semantic meaning for "0".
| return { | ||
| "id": agent.id, | ||
| "did": agent.did, | ||
| "name": agent.name, | ||
| "domain": agent.domain, | ||
| "status": agent.status, |
There was a problem hiding this comment.
RegisteredAgent protobuf (capiscio_sdk/_rpc/gen/capiscio/v1/registry_pb2.py) does not define id or domain fields. Accessing agent.id / agent.domain here will raise AttributeError at runtime. Consider returning only fields that exist on RegisteredAgent (e.g., did, name, description, agent_card_json, status, etc.), and if you need a domain, derive it from agent.badge.domain (and set include_badge=True in the request) when available.
| agent = response.agent | ||
| if not agent or not agent.did: | ||
| return None, "agent not found" |
There was a problem hiding this comment.
GetAgentRequest supports include_badge / verify_badge, but this method currently doesn’t set them while attempting to return badge-derived details (e.g., domain). If the intent is to expose badge-derived fields, set include_badge=True (and optionally verify_badge=True) and handle the case where the badge is absent/empty.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
capiscio_sdk/_rpc/client.py:1790
- For maintainability and consistency with
_trust_level_to_string()(which usesbadge_pb2.TrustLevel.*constants), consider using the enum constants as keys intrust_level_mapinstead of raw integers. This reduces the chance of accidental mis-mapping if the proto enum changes.
trust_level_map = {
0: "0", # TRUST_LEVEL_UNSPECIFIED -> none/unverified
1: "0", # TRUST_LEVEL_SELF_SIGNED (Level 0)
2: "1", # TRUST_LEVEL_DV (Level 1)
3: "2", # TRUST_LEVEL_OV (Level 2)
4: "3", # TRUST_LEVEL_EV (Level 3)
5: "4", # TRUST_LEVEL_CV (Level 4)
| if response.error_message: | ||
| return None, response.error_message | ||
|
|
||
| # TODO: Convert response.agent to dict | ||
| return None, "not yet implemented" | ||
| agent = response.agent | ||
| if not agent or not agent.did: | ||
| return None, "agent not found" | ||
|
|
There was a problem hiding this comment.
get_agent() now has non-trivial response-shaping logic (including “not found” handling and field projection), but there’s no automated coverage around these branches. Add a unit test with a mocked RegistryServiceStub that returns: (1) error_message set, (2) an empty agent message, and (3) a populated RegisteredAgent, and assert the method’s (dict, error) outputs for each case.
- get_agent(): use actual RegisteredAgent proto fields (did, name, description, status, agent_card_json, capabilities, tags) instead of non-existent 'id' and 'domain' fields - get_agent(): set include_badge=True and derive domain from badge - _claims_to_dict(): clarify UNSPECIFIED is coerced to Level 0 (lowest trust level), not a separate 'none' concept
|
✅ Documentation validation passed!
|
|
✅ All checks passed! Ready for review. |
|
✅ SDK server contract tests passed (test_server_integration.py). Cross-product scenarios are validated in capiscio-e2e-tests. |
GA-007: Fix trust level mapping and get_agent stub
Priority: P0 — silent trust elevation + API failure
Problem 1: Trust Level Elevation
_claims_to_dict()mappedTRUST_LEVEL_UNSPECIFIED(proto enum0) to trust level"1"(DV/domain-verified). This silently elevated unverified badges to domain-verified status.Fix 1
Map
TRUST_LEVEL_UNSPECIFIED→"0"(none/unverified). An unspecified trust level must never promote to a verified level.Problem 2: Silent API Failure
RegistryClient.get_agent()returned(None, "not yet implemented")— a tuple that callers would interpret as "agent not found" rather than "this method doesn't work".Fix 2
Implemented
get_agent()properly — convertsresponse.agent(RegisteredAgent protobuf message) to a dict withid,did,name,domain,statusfields.Testing