security: fail closed for unavailable Runtime capabilities - #44
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughThe Runtime now supports closed Acquire, Continue, and OpenCitation requests, gates unavailable capabilities before I/O, returns typed generic refusal outcomes, records restricted internal audit data, and validates the behavior through HTTP, runtime, OpenAPI, and security-catalog tests. ChangesRuntime capability fail-closed flow
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant Client
participant ResolveEndpoint
participant Runtime
participant CapabilityGate
participant ContentIO
Client->>ResolveEndpoint: submit closed ResolveWire
ResolveEndpoint->>Runtime: resolve RuntimeRequest
Runtime->>CapabilityGate: require capability
CapabilityGate-->>Runtime: available or unsupported
alt capability available
Runtime->>ContentIO: perform runtime I/O
ContentIO-->>Runtime: resolution data
Runtime-->>ResolveEndpoint: Resolved
else capability unavailable
Runtime-->>ResolveEndpoint: RequestNotAvailable or CitationNotAvailable
end
ResolveEndpoint-->>Client: ResolutionOutcomeWire
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
engine/runtime/__init__.py (1)
65-94: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
RuntimeCapabilityisn't re-exported despite being a publicRuntime.__init__parameter type.
Runtime(..., acquire_capability: RuntimeCapability = ...)is a public constructor parameter, butRuntimeCapabilityis never added to this__all__/import list, so callers must reach intoengine.runtime.capabilitiesdirectly instead of the package surface implied by this file. Confirm if that's intentional (e.g., capability selection is meant to stay internal/test-only for now); if not, add it here for surface completeness.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@engine/runtime/__init__.py` around lines 65 - 94, Re-export the public RuntimeCapability type through engine.runtime by adding it to the import and __all__ list in __init__.py. Keep the existing Runtime constructor API unchanged and use the defining capabilities module as the source.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/unit/test_runtime_authorized_evidence.py`:
- Line 405: Remove the redundant type assertions in
tests/unit/test_runtime_authorized_evidence.py at lines 405-405 and 436-436:
each duplicates an earlier assert type(outcome) is Resolved within its test
block. Delete both assertions, or replace them only if a distinct attribute
check was intended.
---
Nitpick comments:
In `@engine/runtime/__init__.py`:
- Around line 65-94: Re-export the public RuntimeCapability type through
engine.runtime by adding it to the import and __all__ list in __init__.py. Keep
the existing Runtime constructor API unchanged and use the defining capabilities
module as the source.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9c0392b3-2cea-4198-851d-b3947e44ae08
📒 Files selected for processing (20)
CONTEXT.mdREADME.mdadapters/http/app.pyadapters/http/contracts.pydocs/decisions/0028-fail-closed-unavailable-runtime-capabilities.mddocs/decisions/README.mdengine/runtime/__init__.pyengine/runtime/capabilities.pyengine/runtime/construction.pyengine/runtime/contracts.pyeval/catalogs/security-catalog.schema.jsoneval/catalogs/security-invariants.yamlscripts/validate_security_catalog.pytests/catalog/test_validate_security_catalog.pytests/unit/test_effective_scope_runtime.pytests/unit/test_http_trust_boundary.pytests/unit/test_http_unavailable_capabilities.pytests/unit/test_runtime_authorized_evidence.pytests/unit/test_runtime_empty_package.pytests/unit/test_runtime_unavailable_capabilities.py
| assert outcome.scope_decision.is_empty is True | ||
| assert outcome.scope_decision.target_count == 0 | ||
| assert outcome.package.blocks == () | ||
| assert type(outcome) is Resolved |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the duplicate assert type(outcome) is Resolved. Each of these two tests asserts the same outcome type twice within the same block. It's harmless but redundant; if a distinct second check was intended (e.g. a different attribute), please add that instead.
tests/unit/test_runtime_authorized_evidence.py#L405-L405: this assert duplicates the one already at L401; drop it (or replace with the intended distinct assertion).tests/unit/test_runtime_authorized_evidence.py#L436-L436: this assert duplicates the one already at L430; drop it (or replace with the intended distinct assertion).
📍 Affects 1 file
tests/unit/test_runtime_authorized_evidence.py#L405-L405(this comment)tests/unit/test_runtime_authorized_evidence.py#L436-L436
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/unit/test_runtime_authorized_evidence.py` at line 405, Remove the
redundant type assertions in tests/unit/test_runtime_authorized_evidence.py at
lines 405-405 and 436-436: each duplicates an earlier assert type(outcome) is
Resolved within its test block. Delete both assertions, or replace them only if
a distinct attribute check was intended.
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
1 participant
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.
Closes #16
Outcome
Acceptance evidence
request_not_availableand OpenCitation returnscitation_not_available; neither response contains token, locator, carrier, category, Source, Resource, or audit detail.request_not_available.422 {"code":"invalid_request"}before Runtime.UNSUPPORTED_CAPABILITYremains internal: the mandatory audit gate retains only a typed count and zero detail; public outcomes expose no audit receipt.tests/.Verification
Fresh local CI-equivalent evidence with the PostgreSQL 17 + pgvector harness running:
make checkScope boundaries
No continuation/citation issuance or redemption, federated Provider, source-native ACL carrier, File publication, durable DecisionAudit, or product capability UI is introduced.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation