feat: P1 hardening suite — approval gate, audit anchoring, egress, pre-flight, key-strength#10
Merged
Merged
Conversation
…e-flight, key-strength Implements the remaining roadmap issues (#4 partial, #5, #6, #7, #8). #6 approval gate (approval.py): --sandbox local now requires a parameter-bound, fail-closed approval token (HMAC over mode + canonical-sorted-targets + expiry, keyed by the audit key). Mint with `aegis approve local <targets> [--ttl N]`, pass via --approval / ARGUS_APPROVAL_TOKEN. Cannot be replayed against other targets or after expiry; wildcard grants are impossible. The banner no longer "authorizes" — no token, no run. #5 audit anchoring (anchor.py): AuditLog mirrors the chain tip {seq, tip, ts} to an out-of-band anchor after every entry; `aegis audit` cross-checks it. Detects a full-log rewrite that re-verifies under a leaked key (HMAC-VALID but anchor MISMATCH). Enabled via audit.anchor_path (point at S3 Object Lock / WORM in prod). #7 egress control (egress.py): `aegis egress-rules` emits a default-drop nftables allow-list from the scope policy (denied ranges carved out first) for the disposable recon host — the missing network-layer second barrier for local mode. #8 pre-flight (preflight.py): warns on public-IP targets / over-broad scope / resolve_dns before any packet on the un-isolated path. #4 (short-term): AuditLog fails closed on a weak audit key (<32 chars) so a placeholder key can't sign a "tamper-evident" chain. Full out-of-band signer remains tracked. Config: Policy.audit_anchor_path; commented anchor_path example in scope-policy.yaml. Tests: +14 (test_p1_hardening.py); test fixtures bumped to 32-char keys. 95 passed, ruff + Bandit(medium+) clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PH7w1wBoUxFESq9gh6yjAQ
Reviewer's GuideImplements a P1 hardening suite around local sandbox runs and audit logging: adds parameter-bound approval tokens and CLI, external audit-chain anchoring with cross-checks, nftables egress rules generation from policy, pre-flight configuration checks for risky local runs, and a minimum audit-key length, plus wiring into policy, CLI, scope-policy, docs, and tests. Sequence diagram for approval gate and pre-flight in aegis scansequenceDiagram
actor Operator
participant cli_cmd_scan as cmd_scan
participant Guard as Guardrail
participant Preflight as preflight
participant Approval as approval
Operator->>cli_cmd_scan: aegis scan --sandbox local --approval TOKEN
cli_cmd_scan->>Guard: check_target(t)
Guard-->>cli_cmd_scan: Decision.allowed / reason
cli_cmd_scan->>Preflight: check(args.targets, guard.policy)
Preflight-->>cli_cmd_scan: warnings
cli_cmd_scan->>Approval: verify(args.approval, required, args.targets, audit_key)
Approval-->>cli_cmd_scan: ok / ApprovalError
cli_cmd_scan-->>Operator: REFUSED or proceed with sandbox
Sequence diagram for audit verification with external anchor cross-checksequenceDiagram
actor Operator
participant cli_cmd_audit as cmd_audit
participant Guard as Guardrail
participant Audit as AuditLog
participant Anchor as anchor
Operator->>cli_cmd_audit: aegis audit
cli_cmd_audit->>Guard: _guard(args)
Guard-->>cli_cmd_audit: guard
cli_cmd_audit->>Audit: verify()
Audit-->>cli_cmd_audit: ok
cli_cmd_audit->>Audit: cross_check_anchor()
Audit->>Anchor: read_anchor(audit_anchor_path)
Anchor-->>Audit: anchor_record
Audit-->>cli_cmd_audit: anchored, reason
cli_cmd_audit-->>Operator: audit chain VALID/TAMPERED and anchor OK/MISMATCH
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Per review feedback: #6 scoped the gate to --sandbox local AND --arm, but the first pass covered only local. Generalize the approval token to authorize a SET of modes ({local, arm}); cmd_scan now requires: - a `local`-scoped token for --sandbox local (un-isolated host exec), - an `arm`-scoped token for --arm (exploit-capable tools) in any sandbox, - a token authorizing BOTH when used together. Tokens are exact-match on the canonical mode set (a {local,arm} token does not satisfy a {local}-only requirement), fail closed, and dry-runs need none. `aegis approve <targets> [--mode local] [--mode arm] [--ttl N]` mints them. Adds combined-mode + arm-mode tests. 97 passed; ruff + Bandit clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PH7w1wBoUxFESq9gh6yjAQ
gesh75
marked this pull request as ready for review
June 29, 2026 02:00
![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 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="aegis/aegis/cli.py" line_range="216-217" />
<code_context>
+def cmd_approve(args) -> int:
+ """Mint a parameter-bound approval token for a high-risk run mode (#6)."""
+ policy = Policy.load(args.policy)
+ key = os.environ.get(policy.audit_key_env)
+ if not key:
+ print(f"REFUSED: audit key env {policy.audit_key_env} unset", file=sys.stderr)
+ return 2
</code_context>
<issue_to_address>
**🚨 suggestion (security):** Approval minting only checks that the audit key env is non-empty, not that it meets the minimum key-length requirement enforced elsewhere.
AuditLog enforces `MIN_AUDIT_KEY_LEN`, but `cmd_approve` only checks that `audit_key_env` is non-empty. This lets operators mint approval tokens with weak or placeholder keys, breaking the security assumptions shared with the audit chain. Please reuse the same minimum-length check (or initialize AuditLog here) so approval minting fails on weak keys.
Suggested implementation:
```python
def cmd_approve(args) -> int:
"""Mint a parameter-bound approval token for a high-risk run mode (#6)."""
policy = Policy.load(args.policy)
# Ensure the audit key meets the same minimum-length requirement as the audit log.
audit_key = os.environ.get(policy.audit_key_env)
if not audit_key:
print(f"REFUSED: audit key env {policy.audit_key_env} unset", file=sys.stderr)
return 2
if len(audit_key) < AuditLog.MIN_AUDIT_KEY_LEN:
print(
f"REFUSED: audit key env {policy.audit_key_env} too short "
f"(len={len(audit_key)}, min={AuditLog.MIN_AUDIT_KEY_LEN})",
file=sys.stderr,
)
return 2
# Fail-closed, parameter-bound approval gate for high-risk modes — banners don't
# enforce (#6). `local` = un-isolated host exec; `arm` = exploit-capable tools.
# Dry-run executes nothing, so it needs no approval.
required = []
```
To fully implement this change you will also need to:
1. Ensure `os`, `sys`, and `AuditLog` are imported at the top of `aegis/aegis/cli.py`. For example, add:
- `import os`
- `import sys`
- `from aegis.audit import AuditLog` (or the correct module path where `AuditLog` is defined).
2. If `AuditLog.MIN_AUDIT_KEY_LEN` is not a public attribute, expose or import the constant used by `AuditLog` for the minimum key length and reference that instead.
3. If `cmd_approve` needs the actual key material later (e.g., to mint the token), reuse the validated `audit_key` variable rather than re-reading from the environment.
</issue_to_address>
### Comment 2
<location path="aegis/aegis/egress.py" line_range="26-34" />
<code_context>
+ "#!/usr/sbin/nft -f",
+ "# Argus egress allow-list — generated from the scope policy. Apply on the",
+ "# disposable recon host before `--sandbox local`. Fail-closed: default drop.",
+ f"add table inet {_TABLE}",
+ f"delete table inet {_TABLE}",
+ f"table inet {_TABLE} {{",
+ " chain output {",
+ " type filter hook output priority 0; policy drop;",
</code_context>
<issue_to_address>
**suggestion (bug_risk):** The generated nftables script adds and then deletes the table before redefining it, which is confusing and potentially error-prone.
The `add` → `delete` → `table` sequence is redundant and may have unintended effects depending on nftables semantics. A clearer, safer pattern is to (optionally) `delete` the existing table first, then define it once with the `table inet ... {}` block.
```suggestion
lines = [
"#!/usr/sbin/nft -f",
"# Argus egress allow-list — generated from the scope policy. Apply on the",
"# disposable recon host before `--sandbox local`. Fail-closed: default drop.",
f"delete table inet {_TABLE}",
f"table inet {_TABLE} {{",
" chain output {",
" type filter hook output priority 0; policy drop;",
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+216
to
+217
| key = os.environ.get(policy.audit_key_env) | ||
| if not key: |
There was a problem hiding this comment.
🚨 suggestion (security): Approval minting only checks that the audit key env is non-empty, not that it meets the minimum key-length requirement enforced elsewhere.
AuditLog enforces MIN_AUDIT_KEY_LEN, but cmd_approve only checks that audit_key_env is non-empty. This lets operators mint approval tokens with weak or placeholder keys, breaking the security assumptions shared with the audit chain. Please reuse the same minimum-length check (or initialize AuditLog here) so approval minting fails on weak keys.
Suggested implementation:
def cmd_approve(args) -> int:
"""Mint a parameter-bound approval token for a high-risk run mode (#6)."""
policy = Policy.load(args.policy)
# Ensure the audit key meets the same minimum-length requirement as the audit log.
audit_key = os.environ.get(policy.audit_key_env)
if not audit_key:
print(f"REFUSED: audit key env {policy.audit_key_env} unset", file=sys.stderr)
return 2
if len(audit_key) < AuditLog.MIN_AUDIT_KEY_LEN:
print(
f"REFUSED: audit key env {policy.audit_key_env} too short "
f"(len={len(audit_key)}, min={AuditLog.MIN_AUDIT_KEY_LEN})",
file=sys.stderr,
)
return 2
# Fail-closed, parameter-bound approval gate for high-risk modes — banners don't
# enforce (#6). `local` = un-isolated host exec; `arm` = exploit-capable tools.
# Dry-run executes nothing, so it needs no approval.
required = []To fully implement this change you will also need to:
-
Ensure
os,sys, andAuditLogare imported at the top ofaegis/aegis/cli.py. For example, add:import osimport sysfrom aegis.audit import AuditLog(or the correct module path whereAuditLogis defined).
-
If
AuditLog.MIN_AUDIT_KEY_LENis not a public attribute, expose or import the constant used byAuditLogfor the minimum key length and reference that instead. -
If
cmd_approveneeds the actual key material later (e.g., to mint the token), reuse the validatedaudit_keyvariable rather than re-reading from the environment.
Comment on lines
+26
to
+34
| lines = [ | ||
| "#!/usr/sbin/nft -f", | ||
| "# Argus egress allow-list — generated from the scope policy. Apply on the", | ||
| "# disposable recon host before `--sandbox local`. Fail-closed: default drop.", | ||
| f"add table inet {_TABLE}", | ||
| f"delete table inet {_TABLE}", | ||
| f"table inet {_TABLE} {{", | ||
| " chain output {", | ||
| " type filter hook output priority 0; policy drop;", |
There was a problem hiding this comment.
suggestion (bug_risk): The generated nftables script adds and then deletes the table before redefining it, which is confusing and potentially error-prone.
The add → delete → table sequence is redundant and may have unintended effects depending on nftables semantics. A clearer, safer pattern is to (optionally) delete the existing table first, then define it once with the table inet ... {} block.
Suggested change
| lines = [ | |
| "#!/usr/sbin/nft -f", | |
| "# Argus egress allow-list — generated from the scope policy. Apply on the", | |
| "# disposable recon host before `--sandbox local`. Fail-closed: default drop.", | |
| f"add table inet {_TABLE}", | |
| f"delete table inet {_TABLE}", | |
| f"table inet {_TABLE} {{", | |
| " chain output {", | |
| " type filter hook output priority 0; policy drop;", | |
| lines = [ | |
| "#!/usr/sbin/nft -f", | |
| "# Argus egress allow-list — generated from the scope policy. Apply on the", | |
| "# disposable recon host before `--sandbox local`. Fail-closed: default drop.", | |
| f"delete table inet {_TABLE}", | |
| f"table inet {_TABLE} {{", | |
| " chain output {", | |
| " type filter hook output priority 0; policy drop;", |
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
2 participants
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
Implements the remaining roadmap items in one suite. Closes #5, closes #6, closes #7, closes #8, and lands the short-term half of #4.
What's in this PR
#6 — Risk-tiered approval gate (
approval.py)--sandbox localnow requires a parameter-bound, fail-closed approval token — an HMAC over(mode, canonical-sorted-targets, expiry)keyed by the audit signing key. Mint withaegis approve local <targets> [--ttl N]; pass via--approval/ARGUS_APPROVAL_TOKEN. A token cannot be replayed against different targets or after expiry, and wildcard grants are impossible (every token names its targets). The warning banner no longer "authorizes" anything — no valid token, no run.#5 — External anchoring + WORM cross-check (
anchor.py)AuditLogmirrors the chain tip{seq, tip, ts}to an out-of-band anchor file after every entry;aegis auditcross-checks the live chain against it. This catches a full-log rewrite that re-verifies under a leaked key (demonstrated: chainVALID ✅but anchorMISMATCH ❌, exit 1). Enabled viaaudit.anchor_path; point it at S3 Object Lock / a WORM volume in production (format is ready).#7 — Network-layer egress control (
egress.py)aegis egress-rulesemits a default-drop nftables allow-list derived from the scope policy (denied ranges carved out first). Apply withnft -fon the disposable recon host before--sandbox local— the missing second barrier now that the container boundary is gone.#8 — Pre-flight checks (
preflight.py)Before an un-isolated local run, warns on public-IP targets, an over-broad allow-list (> a /24), or
resolve_dnsenabled — catching misconfiguration before any packet.#4 (short-term) — Audit-key strength
AuditLognow fails closed on a weak audit key (< 32chars) so a placeholder key can't sign a "tamper-evident" chain. The full out-of-band signer process remains tracked under #4.Testing
tests/test_p1_hardening.py; covers approval roundtrip/replay/expiry/wrong-key, anchor tracking + rewrite detection, egress ruleset, pre-flight, short-key refusal).approve→token, scan-local without token → REFUSED, valid token passes, wrong-target token rejected,egress-rulesvalid nft, audit+anchor tamper detection.Follow-up (still tracked under #4)
Full out-of-band signer process (orchestrator never holds the key) + wiring
audit.anchor_pathto an actual S3 Object Lock / KMS store — operational, environment-specific.🤖 Generated with Claude Code
Generated by Claude Code
Summary by Sourcery
Introduce a P1 hardening suite that adds an approval gate for high‑risk modes, external audit anchoring, network egress controls, pre‑flight checks, and an audit-key strength guardrail.
New Features:
aegis approveCLI for--sandbox localand--armhigh-risk modes.aegis auditcross-check support.aegis egress-rulesCLI that generates an nftables egress allow-list from the scope policy.Enhancements:
Documentation:
Tests: