Conversation
Mirror the MCP Guard naming pattern: - Title: 'CapiscIO SDK (Python)' → 'CapiscIO Agent Guard' - Description: introduce Agent Guard as the product name - Keep all class names (SimpleGuard, CapiscioMiddleware) unchanged
… and fastapi integration - Remove agent_card_json parameter from connect flow - Refactor simple_guard to use badge-based identity - Update fastapi integration for new identity model - Add/update unit tests for all changes
|
✅ 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. |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
This PR removes the agent_card_json dependency from the SDK’s identity flow, shifting SimpleGuard/connect/FastAPI integration toward badge-based identity and adding lazy guard binding support in the middleware.
Changes:
- Refactors
SimpleGuardto resolve identity via explicit params (preferred) or legacyagent-card.json(deprecated), and removes auto-writing ofagent-card.json. - Updates FastAPI middleware to support lazy guard binding via callable factories and
set_guard(). - Updates/extends unit tests to cover the new identity and middleware behaviors.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| capiscio_sdk/simple_guard.py | Identity resolution refactor; deprecates agent-card.json loading and removes agent-card generation. |
| capiscio_sdk/connect.py | Connect flow now stores DID and wires SimpleGuard using preloaded keys. |
| capiscio_sdk/integrations/fastapi.py | Adds lazy guard binding (callable guard + set_guard) and allows guard to be unset. |
| tests/unit/test_simple_guard.py | Updates tests to assert no agent-card.json creation and covers legacy deprecation warning. |
| tests/unit/test_fastapi_integration.py | Adds tests for callable/None guard behavior in middleware. |
| tests/unit/test_connect.py | Adjusts badge setup test to account for stored DID. |
| README.md | Updates top-level branding copy. |
You can also share your feedback on Copilot code review. Take the survey.
capiscio_sdk/connect.py
Outdated
| # from _init_identity(), so skip file-based PEM loading | ||
| guard = SimpleGuard( | ||
| base_dir=str(self.keys_dir.parent), | ||
| agent_id=self.agent_id, |
There was a problem hiding this comment.
In _setup_badge(), SimpleGuard is initialized with agent_id=self.agent_id (the server agent UUID). SimpleGuard uses agent_id as the issuer DID (it injects it into the JWS 'iss' header/claim), so this will produce tokens with an invalid issuer and can break downstream verification. Pass the connector's DID (self.did / local identity) as SimpleGuard.agent_id instead of the agent UUID.
| agent_id=self.agent_id, | |
| agent_id=self.did, |
capiscio_sdk/integrations/fastapi.py
Outdated
| # Treat as factory only if it's a plain function/lambda (not a guard-like object) | ||
| if guard is not None and callable(guard) and not hasattr(guard, 'verify_inbound'): | ||
| self._guard_factory = guard | ||
| self._guard: Optional[SimpleGuard] = None | ||
| else: | ||
| self._guard = guard |
There was a problem hiding this comment.
The guard/factory detection will mis-handle callables like the SimpleGuard class itself: guard=SimpleGuard is callable and has verify_inbound, so it gets stored as the guard instance. Later self.guard.verify_inbound(...) will call the unbound method and crash with a TypeError. Consider using isinstance(guard, SimpleGuard) (instance check) to detect real guards, and treat any other callable as a factory.
| # Treat as factory only if it's a plain function/lambda (not a guard-like object) | |
| if guard is not None and callable(guard) and not hasattr(guard, 'verify_inbound'): | |
| self._guard_factory = guard | |
| self._guard: Optional[SimpleGuard] = None | |
| else: | |
| self._guard = guard | |
| self._guard: Optional[SimpleGuard] = None | |
| # Treat any SimpleGuard instance or non-callable guard-like object as a guard; | |
| # treat other callables (including the SimpleGuard class itself) as factories. | |
| if guard is None: | |
| self._guard = None | |
| elif isinstance(guard, SimpleGuard) or not callable(guard): | |
| self._guard = guard | |
| else: | |
| # Callable but not a SimpleGuard instance → resolve lazily via factory | |
| self._guard_factory = guard |
capiscio_sdk/integrations/fastapi.py
Outdated
| # If guard is not yet bound (lazy binding), pass through as unverified | ||
| if self.guard is None: | ||
| logger.debug("CapiscioMiddleware: guard not yet bound, passing through as unverified") | ||
| request.state.agent = None | ||
| request.state.agent_id = None | ||
| return await call_next(request) |
There was a problem hiding this comment.
When guard is None (or the factory returns None), the middleware currently passes requests through as unverified even when running in the default strict mode (require_signatures=True, fail_mode='block'). This effectively disables enforcement if the guard is never bound and can lead to protected endpoints being unintentionally exposed. Consider blocking with a 503/500 until a guard is available (or require an explicit opt-in like allow_unverified_when_guard_missing=True), and document the behavior clearly.
| # If guard is not yet bound (lazy binding), pass through as unverified | |
| if self.guard is None: | |
| logger.debug("CapiscioMiddleware: guard not yet bound, passing through as unverified") | |
| request.state.agent = None | |
| request.state.agent_id = None | |
| return await call_next(request) | |
| # If guard is not yet bound (lazy binding), treat as guard unavailable and fail closed | |
| if self.guard is None: | |
| logger.error("CapiscioMiddleware: guard not bound or unavailable; blocking request to avoid unverified access") | |
| # Mark request as unauthenticated | |
| request.state.agent = None | |
| request.state.agent_id = None | |
| # Auto-event: verification.failed (guard unavailable) | |
| self._auto_emit(EventEmitter.EVENT_VERIFICATION_FAILED, { | |
| "method": request.method, | |
| "path": path, | |
| "reason": "guard_unavailable", | |
| "duration_ms": 0.0, | |
| }) | |
| return JSONResponse( | |
| {"error": "CapiscIO guard is not available. Requests cannot be verified at this time."}, | |
| status_code=503, | |
| ) |
- connect.py: Use self.did (DID) instead of self.agent_id (UUID) for SimpleGuard agent_id, fixing invalid JWS issuer claims - fastapi.py: Add isinstance(guard, type) check to guard detection, preventing SimpleGuard class from being used as a guard instance - fastapi.py: Fail closed with 503 when guard is None instead of passing through as unverified (security: prevents unintended access) - Updated tests for fail-closed behavior (guard=None -> 503)
|
✅ 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. |
|
✅ Documentation validation passed!
|
|
✅ All checks passed! Ready for review. |
The workflow builds Go from source (capiscio-core + capiscio-server Docker) plus a Python test runner image. 15 minutes is too tight for shared runners.
|
✅ 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. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
You can also share your feedback on Copilot code review. Take the survey.
| def test_middleware_with_none_guard_passes_through(self): | ||
| """Test that None guard returns 503 (fail closed).""" | ||
| app = FastAPI() | ||
| app.add_middleware( | ||
| CapiscioMiddleware, | ||
| guard=None, | ||
| exclude_paths=["/health"], | ||
| ) | ||
|
|
||
| @app.post("/test") | ||
| async def test_endpoint(request: Request): | ||
| return { | ||
| "agent": getattr(request.state, 'agent', 'not-set'), | ||
| "agent_id": getattr(request.state, 'agent_id', 'not-set'), | ||
| } | ||
|
|
||
| client = TestClient(app) | ||
| response = client.post("/test", json={}) | ||
| assert response.status_code == 503 | ||
| assert "guard is not available" in response.json()["error"] | ||
|
|
There was a problem hiding this comment.
Test name says it "passes_through" but the assertions verify the middleware fails closed with a 503 when guard=None. Please rename the test (and/or docstring) to reflect the actual behavior to avoid confusion when reading failures.
| if not keys_preloaded: | ||
| self._load_or_generate_keys() | ||
| else: | ||
| logger.info(f"Keys preloaded in gRPC server, skipping file-based key loading") |
There was a problem hiding this comment.
This log call uses an f-string but doesn't interpolate anything. Consider using a plain string to avoid unnecessary formatting overhead and keep logging consistent.
| logger.info(f"Keys preloaded in gRPC server, skipping file-based key loading") | |
| logger.info("Keys preloaded in gRPC server, skipping file-based key loading") |
Summary
Removes the
agent_card_jsonparameter dependency from the connect flow, simple_guard, and FastAPI integration. Agents now use badge-based identity exclusively.Changes
agent_card_jsonparameter from connect flowContext
Part of the ongoing effort to align the SDK with the Agent Guard product direction, where badges are the primary identity primitive rather than agent card JSON files.