Optimize security-review workflow: ~67% cost reduction, 55% fewer LLM turns#1717
Optimize security-review workflow: ~67% cost reduction, 55% fewer LLM turns#1717
Conversation
Agent-Logs-Url: https://github.com/github/gh-aw-firewall/sessions/a0f9cc8b-f124-4758-8972-07df1661dffc Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw-firewall/sessions/a0f9cc8b-f124-4758-8972-07df1661dffc Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR optimizes the daily security-review agentic workflow to reduce token usage and LLM turns by pruning unused tool schemas, moving deterministic data-fetching into a pre-agent steps: block, and collapsing multi-phase analysis into fewer prompts.
Changes:
- Remove unused
agentic-workflows/web-fetchtooling and restrict GitHub MCP toolsets to[repos, code_security]. - Add a pre-agent step that fetches the latest escape test logs into
/tmp/gh-aw/escape-test-summary.txtfor the agent to read locally. - Consolidate evidence gathering into a single bash block and merge phases 3–5 into a single synthesis phase.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/security-review.md |
Workflow prompt/frontmatter updates: tool pruning, new pre-agent log fetch step, batched evidence collection, and merged synthesis phase. |
.github/workflows/security-review.lock.yml |
Regenerated compiled workflow reflecting the prompt/tooling changes and removal of the agentic-workflows MCP server wiring. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 3
.github/workflows/security-review.md
Outdated
| - name: Fetch latest escape test results | ||
| run: | | ||
| mkdir -p /tmp/gh-aw | ||
| RUN_ID=$(gh run list --workflow "firewall-escape-test.lock.yml" \ |
There was a problem hiding this comment.
The pre-step queries gh run list --workflow "firewall-escape-test.lock.yml", but there is no workflow file with that name in this repository’s .github/workflows/ directory. As written, this will consistently produce an empty RUN_ID and Phase 1 will always read the fallback “No recent successful…” message. Update the --workflow argument to the actual workflow file name or workflow display name used by the escape test workflow in this repo (and consider documenting it here).
| RUN_ID=$(gh run list --workflow "firewall-escape-test.lock.yml" \ | |
| # Keep this workflow identifier aligned with the escape test workflow's file name or display name. | |
| RUN_ID=$(gh run list --workflow "firewall-escape-test" \ |
.github/workflows/security-review.md
Outdated
| mkdir -p /tmp/gh-aw | ||
| RUN_ID=$(gh run list --workflow "firewall-escape-test.lock.yml" \ | ||
| --status success --limit 1 --json databaseId --jq '.[0].databaseId' 2>/dev/null || echo "") | ||
| if [ -n "$RUN_ID" ]; then |
There was a problem hiding this comment.
gh run list ... --jq '.[0].databaseId' can output the literal string null (jq returns null with exit code 0 when the array is empty). Because the script only checks -n "$RUN_ID", it may attempt gh run view "null" and then report a fetch failure instead of the clearer “no run found” path. Consider validating RUN_ID is a numeric id (or non-null) before calling gh run view.
| if [ -n "$RUN_ID" ]; then | |
| if [ -n "$RUN_ID" ] && [ "$RUN_ID" != "null" ] && [[ "$RUN_ID" =~ ^[0-9]+$ ]]; then |
.github/workflows/security-review.md
Outdated
| grep -r "network" src/ --include="*.ts" -l | ||
|
|
||
| echo "=== CONTAINER SECURITY ===" && \ | ||
| grep -rn "cap_drop\|capabilities\|NET_ADMIN\|NET_RAW" src/ containers/ && echo "---" && \ | ||
| cat containers/agent/seccomp-profile.json && echo "---" && \ | ||
| grep -rn "privilege\|root\|user\|uid" containers/ | ||
|
|
||
| echo "=== DOMAIN PATTERNS ===" && \ | ||
| cat src/domain-patterns.ts && echo "---" && \ | ||
| grep -rn "domain\|wildcard\|pattern" src/ --include="*.ts" -l | ||
|
|
||
| echo "=== INJECTION RISKS ===" && \ | ||
| grep -rn "exec\|spawn\|shell\|command" src/ --include="*.ts" -l && echo "---" && \ | ||
| grep -rn '\$\{' containers/ --include="*.sh" && echo "---" && \ |
There was a problem hiding this comment.
Several grep -r... src/ --include="*.ts" -l / grep ... containers/ --include="*.sh" commands place --include after the path argument. In GNU grep, options after a file/dir operand can be treated as additional filenames, causing the search to fail (and undermining the “single evidence-gathering block” goal). Reorder these greps so all options (including --include and -l) come before the pattern/path operands.
|
@copilot apply changes based on the comments in this thread |
…ption ordering Agent-Logs-Url: https://github.com/github/gh-aw-firewall/sessions/04d0ba5b-f85d-4910-afe2-8e06f6511880 Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Applied all three review fixes in commit 2b02de8:
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (1 files)
Coverage comparison generated by |
|
Smoke test results (run 24048360915) ✅ GitHub MCP: #1704 "Fix Secret Digger (Copilot)…", #1701 "⚡ pelis-agent-factory-advisor…" Overall: PASS
|
🔥 Smoke Test Results — PASS
PR: "Optimize security-review workflow: ~67% cost reduction, 55% fewer LLM turns"
|
|
Smoke test results:
|
Security ReviewThis PR modifies CI workflow files only — no changes to AWF core security components (iptables, squid config, container hardening, domain patterns, seccomp profile). ✅ Security Improvements
|
Smoke Test: GitHub Actions Services Connectivity
|
The
security-review.mdworkflow was generating ~5.1M tokens per run ($8.91) due to 33 LLM turns driven by sequential phase structure and overly broad tool loading.Tool/schema pruning
agentic-workflowsandweb-fetchtools (neither needed in final form)[default, actions, code_security]→[repos, code_security]— drops ~15 unused tool schemas (~297K tokens across turns)Phase 1: pre-step replaces agentic-workflows tool
Add a
steps:pre-step that fetches the latestfirewall-escape-testrun logs viaghCLI and writes them to/tmp/gh-aw/escape-test-summary.txtbefore the agent starts. Phase 1 now just doescat /tmp/gh-aw/escape-test-summary.txt— eliminates 3–4 agentic-workflows turns.Phase 2: batch evidence gathering
7 separate bash blocks (one per sub-section) collapsed into a single block:
Reduces ~15 evidence-gathering turns to 2 (gather + analyze).
Phases 3–5: single synthesis phase
Merged STRIDE threat modeling, attack surface mapping, and CIS/NIST comparison into one prompt section — 1 synthesis turn instead of 9–10.
Projected impact