chore: security CI + lint/pre-commit config + NEXT_STEPS roadmap (+ exit-127 fix)#2
Merged
Merged
Conversation
…-fix Implements the EXA-researched hardening roadmap (aegis/docs/NEXT_STEPS.md). CI / supply-chain (P0): - .github/workflows/ci.yml — pytest + Bandit(medium+) blocking; pip-audit + detect-secrets informational. Every Action pinned to a full commit SHA, least-privilege permissions (OpenSSF post-tj-actions guidance). - aegis/pyproject.toml — central Bandit/Ruff/pytest config (high-signal rule set). - .pre-commit-config.yaml — shift-left mirror of the CI gates. Correctness (P2.8): orchestrator no longer conflates "binary missing" with "tool ran and exited 127". ExecResult gains a tool_missing flag the sandbox sets only when a binary was never launched; DockerSandbox keeps its 127 heuristic, LocalSandbox flags synthesized-missing only. Covered by new tests. Lint hygiene to make the gates pass on existing code: justified # nosec on the vetted urlopen (fixed scheme) and defusedxml call sites, # noqa for the lab default cred and best-effort error-body read, and ruff --fix import/pyupgrade cleanups. No behavior change. Larger architectural items (out-of-band signer, WORM anchoring, approval gate, network egress, dependency hash-pinning) are tracked as issues per NEXT_STEPS. Full suite: 81 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 GuideAdds a hardened security-focused CI pipeline and shift-left linting config, introduces a tool_missing flag to fix exit-code 127 handling in sandboxes and orchestrator, and updates documentation and minor code hygiene to satisfy new security lint rules. Sequence diagram for updated sandbox exit-127 handlingsequenceDiagram
participant Orchestrator as Orchestrator
participant SandboxLocal as LocalSandbox
participant SandboxDocker as DockerSandbox
participant Subprocess as subprocess.Popen
Orchestrator->>SandboxLocal: run(argv, timeout)
alt [empty argv]
SandboxLocal-->>Orchestrator: ExecResult(exit_code=127, tool_missing=True)
else [argv[0] not on PATH]
SandboxLocal-->>Orchestrator: ExecResult(exit_code=127, tool_missing=True)
else [binary found]
SandboxLocal->>Subprocess: Popen(argv, env=_safe_env)
Subprocess-->>SandboxLocal: returncode, stdout, stderr
SandboxLocal-->>Orchestrator: ExecResult(exit_code, tool_missing=False)
end
Orchestrator->>SandboxDocker: run(argv, timeout)
SandboxDocker->>Subprocess: Popen(docker compose exec ...)
Subprocess-->>SandboxDocker: returncode, stdout, stderr
SandboxDocker->>SandboxDocker: [exit_code == 127 and not timed_out] set tool_missing=True
SandboxDocker-->>Orchestrator: ExecResult(exit_code, tool_missing)
Orchestrator->>Orchestrator: guard.record(tool.binary, exit_code, summary)
alt [ex.tool_missing]
Orchestrator->>Orchestrator: result.errors.append("tool unavailable in sandbox")
else [not ex.tool_missing]
Orchestrator->>Orchestrator: tool.parse(stdout, target)
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
gesh75
marked this pull request as ready for review
June 28, 2026 19:05
![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, and left some high level feedback:
- In the
secretsjob, thedetect-secrets scan aegis targetsinvocation assumes atargetsdirectory exists; consider scoping the scan to actual repo paths (e.g. justaegisor using a glob) to avoid noisy failures or scanning non-existent paths. - The DockerSandbox heuristic sets
tool_missing=Truefor every non-timeout 127 fromdocker exec, which means genuine in-container exit 127 is still surfaced as "tool unavailable"; if possible, consider tightening this by checking for known missing-binary patterns or adding an escape hatch for tools that legitimately use 127.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the `secrets` job, the `detect-secrets scan aegis targets` invocation assumes a `targets` directory exists; consider scoping the scan to actual repo paths (e.g. just `aegis` or using a glob) to avoid noisy failures or scanning non-existent paths.
- The DockerSandbox heuristic sets `tool_missing=True` for every non-timeout 127 from `docker exec`, which means genuine in-container exit 127 is still surfaced as "tool unavailable"; if possible, consider tightening this by checking for known missing-binary patterns or adding an escape hatch for tools that legitimately use 127.
## Individual Comments
### Comment 1
<location path=".github/workflows/ci.yml" line_range="77-78" />
<code_context>
+ python-version: "3.12"
+ - name: Install detect-secrets
+ run: pip install detect-secrets==1.5.0
+ - name: Scan working tree for secrets
+ run: detect-secrets scan aegis targets
</code_context>
<issue_to_address>
**🚨 suggestion (security):** The paths passed to `detect-secrets` depend on the workflow-level working directory and may not cover the whole repo as intended.
With `defaults.run.working-directory` set to `aegis`, `detect-secrets scan aegis targets` actually scans `aegis/aegis` and `aegis/targets` from the repo root, and skips top-level paths like `.github` or any non-`aegis` directories. If you want full-repo coverage, consider removing the workflow-level `working-directory` for this job or adjusting the command to scan from the repo root (e.g., `detect-secrets scan . ..` or explicitly listing root-level directories).
</issue_to_address>
### Comment 2
<location path="aegis/docs/NEXT_STEPS.md" line_range="26" />
<code_context>
+
+> *Best practice:* the 4-gate Python DevSecOps pattern (SAST + secret-scan + AST + dep-audit)
+> is the consensus baseline; CI is "the authoritative gate" because pre-commit can be skipped
+> with `--no-verify`. — `thunderstornX/secure-python-pipeline-template`,
+> `developmentseed/action-python-security-auditing`.
+
</code_context>
<issue_to_address>
**suggestion (typo):** Consider double-checking the spelling of `thunderstornX` in this reference.
The identifier `thunderstornX` appears to be misspelled (likely `thunderstormX` or similar). Since this references a GitHub repo/template, please confirm the exact name so readers can locate the correct resource.
Suggested implementation:
```
> *Best practice:* the 4-gate Python DevSecOps pattern (SAST + secret-scan + AST + dep-audit)
> is the consensus baseline; CI is "the authoritative gate" because pre-commit can be skipped
> with `--no-verify`. — `secure-python-pipeline-template`,
> `developmentseed/action-python-security-auditing`.
```
If the intention is to point to a specific GitHub repository owner, replace `secure-python-pipeline-template` with an explicit Markdown link once the exact `owner/repo` is confirmed, for example:
- \[`secure-python-pipeline-template`](https://github.com/OWNER/secure-python-pipeline-template)
This ensures readers can directly navigate to the referenced template without ambiguity.
</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
+77
to
+78
| - name: Scan working tree for secrets | ||
| run: detect-secrets scan aegis targets |
There was a problem hiding this comment.
🚨 suggestion (security): The paths passed to detect-secrets depend on the workflow-level working directory and may not cover the whole repo as intended.
With defaults.run.working-directory set to aegis, detect-secrets scan aegis targets actually scans aegis/aegis and aegis/targets from the repo root, and skips top-level paths like .github or any non-aegis directories. If you want full-repo coverage, consider removing the workflow-level working-directory for this job or adjusting the command to scan from the repo root (e.g., detect-secrets scan . .. or explicitly listing root-level directories).
|
|
||
| > *Best practice:* the 4-gate Python DevSecOps pattern (SAST + secret-scan + AST + dep-audit) | ||
| > is the consensus baseline; CI is "the authoritative gate" because pre-commit can be skipped | ||
| > with `--no-verify`. — `thunderstornX/secure-python-pipeline-template`, |
There was a problem hiding this comment.
suggestion (typo): Consider double-checking the spelling of thunderstornX in this reference.
The identifier thunderstornX appears to be misspelled (likely thunderstormX or similar). Since this references a GitHub repo/template, please confirm the exact name so readers can locate the correct resource.
Suggested implementation:
> *Best practice:* the 4-gate Python DevSecOps pattern (SAST + secret-scan + AST + dep-audit)
> is the consensus baseline; CI is "the authoritative gate" because pre-commit can be skipped
> with `--no-verify`. — `secure-python-pipeline-template`,
> `developmentseed/action-python-security-auditing`.
If the intention is to point to a specific GitHub repository owner, replace secure-python-pipeline-template with an explicit Markdown link once the exact owner/repo is confirmed, for example:
- [
secure-python-pipeline-template](https://github.com/OWNER/secure-python-pipeline-template)
This ensures readers can directly navigate to the referenced template without ambiguity.
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 immediate, fully-verifiable items from the EXA-researched hardening roadmap and lands the roadmap itself as
aegis/docs/NEXT_STEPS.md. Larger architectural items are tracked as issues (linked below).What's in this PR
P0 — Supply-chain & CI (the repo had no CI at all)
.github/workflows/ci.yml—pytest+ Bandit (medium+) as blocking gates;pip-audit+detect-secretsinformational. Every Action pinned to a full commit SHA, least-privilegepermissions: contents: read(OpenSSF post-tj-actionsguidance).aegis/pyproject.toml— central Bandit / Ruff / pytest config (high-signal rule set:F,I,B,UP,S)..pre-commit-config.yaml— shift-left mirror of the CI gates.P2.8 — Correctness fix (deferred from the previous review)
ExecResultgains atool_missingflag the sandbox sets only when a binary was never launched;DockerSandboxkeeps its 127 heuristic,LocalSandboxflags synthesized-missing only. New tests intests/test_sandbox.py.Lint hygiene (to make the new gates pass on existing code, no behavior change)
# nosecon the vettedurlopen(fixed scheme) anddefusedxmlcall sites;# noqafor the lab default cred and best-effort error-body read.ruff --fiximport-sorting / pyupgrade cleanups across a few modules.Documentation
aegis/docs/NEXT_STEPS.md— the full prioritized roadmap with best-practice citations (OWASP AI Agent Security, ROE Gate, Aikido, IntegSec, OpenSSF, tamper-evident-audit-log sources).Tracked as follow-up issues (not in this PR)
pip-auditto blocking--sandbox local/--armTesting
🤖 Generated with Claude Code
Generated by Claude Code
Summary by Sourcery
Introduce security-focused CI, linting, and pre-commit configuration, add a hardening roadmap doc, and fix orchestrator handling of missing tools vs real exit-127 failures.
New Features:
Bug Fixes:
Enhancements:
CI:
Documentation:
Tests:
Chores: