Repository navigation
feat: PIV step-0, audited SSH/NETCONF connector, Batfish sidecar - #27
Merged
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideImplements the three roadmap foundations as guarded, fail-closed paths: Step-0 PIV trust and attestation scaffolding, explicitly gated SSH/NETCONF connectivity, and an optional loopback-only Batfish sidecar. The supplied tests cover the PIV, connector, Batfish, and seal behavior, while hardware, production devices, and a running Batfish container were not exercised. Sequence diagram for guarded SSH and NETCONF connector promotionsequenceDiagram
participant Caller
participant ConnectorFactory
participant Device
Caller->>ConnectorFactory: get_connector(protocol)
ConnectorFactory-->>Caller: live connector
Caller->>Device: connect()
alt dry-run or live gates not satisfied
Device-->>Caller: no socket opened
else AEGIS_PROMOTE_ALLOW_LIVE=1 and AEGIS_SSH_ENABLE=1 and allowlisted
Device-->>Caller: audited SSH or NETCONF session
end
Sequence diagram for fail-closed Batfish analysissequenceDiagram
participant Simulator
participant BatfishSidecar
participant Batfish
Simulator->>BatfishSidecar: analyze()
BatfishSidecar->>Batfish: analyze()
alt sidecar unavailable
Batfish-->>BatfishSidecar: errors>=1
BatfishSidecar-->>Simulator: passed=0
else sidecar available
Batfish-->>BatfishSidecar: analysis result
BatfishSidecar-->>Simulator: analysis result
end
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Approved. Cursor Bugbot was not running after the first check poll, no approval policy required human review, and no unresolved automated findings were present. Reviewers were not assigned.
Sent by Cursor Approval Agent: Pull Request Router and Approver
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="core/seal/pins.py" line_range="11-13" />
<code_context>
+# SHA-256 of the DER encoding of "Yubico PIV Root CA Serial 263751"
+# Source: https://developers.yubico.com/PKI/yubico-piv-ca-1.pem
+# Verified 2026-08-30 against the published PEM (795-byte DER).
+YUBICO_PIV_ROOT_CA_DER_SHA256 = (
+ "63ece914e54dd87915f34033c85af4c0696ba1512f8add66ced738331207b546"
+)
+
+PIV_SERIAL_ALLOWLIST: frozenset[str] = frozenset()
</code_context>
<issue_to_address>
**🚨 issue (security):** The compiled-in root hash is never imported or consumed anywhere in the repository, so no attestation or certificate-validation path compares a presented PIV chain against this pin. A non-Yubico or otherwise untrusted chain therefore receives no rejection from this change.
**Triggers:** When a PIV attestation path is used.
**Suggested fix:** Wire the pin into the attestation verifier and fail closed unless the DER SHA-256 exactly matches it.
</issue_to_address>Sourcery assessment
Approval pending. 1 finding to address first.
Blocking findings: core/seal/pins.py:13
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+11
to
+13
| YUBICO_PIV_ROOT_CA_DER_SHA256 = ( | ||
| "63ece914e54dd87915f34033c85af4c0696ba1512f8add66ced738331207b546" | ||
| ) |
There was a problem hiding this comment.
🚨 issue (security): The compiled-in root hash is never imported or consumed anywhere in the repository, so no attestation or certificate-validation path compares a presented PIV chain against this pin. A non-Yubico or otherwise untrusted chain therefore receives no rejection from this change.
Triggers: When a PIV attestation path is used.
Suggested fix: Wire the pin into the attestation verifier and fail closed unless the DER SHA-256 exactly matches it.
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.
Why
Closes #26. Implements the three LATER roadmap cards without pretending hardware or a live fabric is present.
What
1. YubiKey PIV signer — Step 0 (no token)
63ece914e54dd87915f34033c85af4c0696ba1512f8add66ced738331207b546(checked against the official PEM fixture).r‖s↔ DER helpers.PivSignerrefuses to mintpiv-ecdsa-p256withoutAEGIS_PIV_MODULE+ token. Software P-256 isecdsa-p256-softwareonly.C_Signagainst libykcs11 is not wired. That is plan step 2 and needs the key in slot 9c.2. Audited SSH/NETCONF behind G4
get_connector("ssh"|"netconf")returns live=True connectors.AEGIS_PROMOTE_ALLOW_LIVE=1andAEGIS_SSH_ENABLE=1and the device is on the allowlist.shell=True. Allowlist isdevice hostnamelines.3. Batfish allinone sidecar
docker-compose.batfish.ymlbinds 8888/9996/9997 to loopback.BatfishSidecar.analyze()fail-closes when the sidecar is down (errors>=1,passed=0).Tests run locally
python -m aegis.tests.piv_test— PASS 13python -m aegis.tests.connector_test— PASS 13python -m aegis.tests.batfish_sidecar_test— PASS 6python -m aegis.tests.seal_test— PASS 15/15Honesty
No YubiKey was present. No production SSH push was executed. No Batfish container was started. Those paths fail closed.
Summary by Sourcery
Introduce fail-closed foundations for hardware-backed PIV signing, audited network connectivity, and Batfish analysis without requiring production hardware or live infrastructure.
New Features:
Enhancements:
Deployment:
Tests: