Skip to content

Commit 3c8a70d

Browse files
lpcoxCopilotCopilot
authored
perf(security-guard): prioritize security-relevant files in PR diff (#5329)
* perf(security-guard): prioritize security-relevant files in PR diff The Security Guard agent's pre-fetched diff dumped full patches for every changed file, so large refactor PRs sent oversized prompts (cost, latency, and a larger first-request that can trip the upstream Copilot provider). Rework the pre-fetch so it includes full patches only for security-relevant files (matching the same path regex the relevance gate uses), largest first, and lists every other changed file by name only. A non-security file's patch no longer bloats the prompt (e.g. a workflow/test-only PR drops from ~22 KB to a few hundred bytes). Also stop instructing the agent to re-fetch the entire PR diff on truncation: security-relevant patches are shown first, so it should only fetch a still-missing security-relevant file via get_pull_request_diff. Prompt copy and the truncation note updated to match. Recompiled the lock (security-relevant patch prioritization in the pr-diff step); the COPILOT_DUMMY_BYOK placeholder fix from #5306 and the gh-aw-mcpg image pin are preserved. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(security-guard): address review feedback on diff scope and wording --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
1 parent 10d3a2e commit 3c8a70d

2 files changed

Lines changed: 26 additions & 12 deletions

File tree

.github/workflows/security-guard.lock.yml

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

.github/workflows/security-guard.md

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -67,16 +67,30 @@ steps:
6767
run: |
6868
DELIM="GHAW_PR_FILES_$(date +%s)"
6969
DIFF_LIMIT=100000
70+
SECURITY_RE='host-iptables|setup-iptables|squid-config|docker-manager|seccomp-profile|domain-patterns|entrypoint\.sh|Dockerfile|(^|/)containers/'
71+
TEST_EXCLUDE_RE='(^|/)tests?/|\.test\.'
7072
DIFF_TMP="$(mktemp)"
73+
# Include full patches only for security-relevant files (largest first);
74+
# list every other changed file by name so large non-security refactors
75+
# don't bloat the prompt.
76+
gh api "repos/${GH_REPO}/pulls/${PR_NUMBER}/files" --paginate --slurp \
77+
| jq -r --arg re "$SECURITY_RE" --arg test_ex "$TEST_EXCLUDE_RE" '
78+
([.[][]]) as $files
79+
| ([$files[] | select((.filename | test($re)) and (.filename | test($test_ex) | not))] | sort_by(-(.additions + .deletions))) as $sec
80+
| ([$files[] | select(((.filename | test($re)) and (.filename | test($test_ex) | not)) | not)]) as $other
81+
| ( $sec[]
82+
| "### " + .filename + " (+" + (.additions|tostring) + "/-" + (.deletions|tostring) + ") [security-relevant]\n" + (.patch // "(binary or no textual patch)") + "\n" ),
83+
( if ($other | length) > 0
84+
then "\n### Other changed files (not security-relevant — patches omitted to save context):\n"
85+
+ ([$other[] | "- " + .filename + " (+" + (.additions|tostring) + "/-" + (.deletions|tostring) + ")"] | join("\n")) + "\n"
86+
else empty end )
87+
' > "$DIFF_TMP" || true
88+
DIFF_SIZE="$(wc -c < "$DIFF_TMP" | tr -d ' ')"
7189
{
7290
echo "PR_FILES<<${DELIM}"
73-
gh api "repos/${GH_REPO}/pulls/${PR_NUMBER}/files" \
74-
--paginate --jq '.[] | "### " + .filename + " (+" + (.additions|tostring) + "/-" + (.deletions|tostring) + ")\n" + (.patch // "") + "\n"' \
75-
> "$DIFF_TMP" || true
76-
DIFF_SIZE="$(wc -c < "$DIFF_TMP" | tr -d ' ')"
7791
head -c "$DIFF_LIMIT" "$DIFF_TMP" || true
7892
if [ "$DIFF_SIZE" -gt "$DIFF_LIMIT" ]; then
79-
echo -e "\n[DIFF TRUNCATED at ${DIFF_LIMIT} bytes — use mcp__github__get_pull_request_diff for full context]"
93+
echo -e "\n[DIFF TRUNCATED at ${DIFF_LIMIT} bytes — security-relevant patches are shown first; if one is still missing, fetch the full PR diff once via mcp__github__get_pull_request_diff and locate that file section]"
8094
fi
8195
echo ""
8296
echo "${DELIM}"
@@ -126,7 +140,7 @@ steps:
126140
127141
## ⚡ Fast Path
128142

129-
Read the pre-fetched diff below first. If you see `[DIFF TRUNCATED ...]`, fetch full context once with `mcp__github__get_pull_request_diff` before deciding to noop. Only use the fast path when the full diff contains **no** security-weakening changes: no weakened DROP/REJECT or expanded ACCEPT, no egress/domain allowlist expansion, no firewall chain changes, no capability additions, no ACL regressions, no seccomp relaxations, no DNS/wildcard bypass, no input validation weakening, and no secrets. Then call `safeoutputs noop` immediately — do not read additional files or make further tool calls.
143+
Read the pre-fetched diff below first. Security-relevant files are included in full; other changed files are listed by name only. If you see `[DIFF TRUNCATED ...]` and a **security-relevant** patch is missing, fetch the full PR diff once with `mcp__github__get_pull_request_diff` and locate that file section before deciding to noop. Only use the fast path when the security-relevant changes contain **no** security-weakening changes: no weakened DROP/REJECT or expanded ACCEPT, no egress/domain allowlist expansion, no firewall chain changes, no capability additions, no ACL regressions, no seccomp relaxations, no DNS/wildcard bypass, no input validation weakening, and no secrets. Then call `safeoutputs noop` immediately — do not read additional files or make further tool calls.
130144

131145
## Repository Context
132146

@@ -137,9 +151,9 @@ Security-critical files: `src/host-iptables.ts`, `containers/agent/setup-iptable
137151

138152
Analyze PR #${{ github.event.pull_request.number }} in repository ${{ github.repository }}.
139153

140-
1. **Review the pre-fetched diff below** (up to 100 KB of changes are included)
154+
1. **Review the pre-fetched diff below** (security-relevant files in full; other files listed by name)
141155
2. **Batch all independent reads** in a single tool-use block rather than making sequential calls
142-
3. **Use ONLY the pre-fetched diff below.** Do NOT call `gh pr diff`, `gh pr view`, `gh api`, `git diff`, `git log`, or `git show`. Do NOT read files from the checkout. If `[DIFF TRUNCATED ...]` appears, call `mcp__github__get_pull_request_diff` once then stop making tool calls and analyze inline.
156+
3. **Use ONLY the pre-fetched diff below.** Do NOT call `gh pr diff`, `gh pr view`, `gh api`, `git diff`, `git log`, or `git show`. Do NOT read files from the checkout. If `[DIFF TRUNCATED ...]` appears and a security-relevant patch is missing, call `mcp__github__get_pull_request_diff` once (it returns the full PR diff), locate the missing security-relevant file section, then stop making tool calls and analyze inline.
143157
4. **Collect evidence** with specific file names, line numbers, and code snippets
144158

145159
## Security Checks
@@ -149,7 +163,7 @@ Focus: weakened DROP/REJECT, added capabilities (SYS_ADMIN/NET_RAW), expanded AC
149163
## Output Format
150164

151165
**IMPORTANT: Be concise.** Report each security finding in ≤ 150 words. Maximum 5 findings total.
152-
If `[DIFF TRUNCATED ...]` is present, fetch full context once with `mcp__github__get_pull_request_diff` before deciding to noop.
166+
If `[DIFF TRUNCATED ...]` is present and a security-relevant patch is missing, fetch the full PR diff once with `mcp__github__get_pull_request_diff`, locate that file section, then decide whether to noop.
153167

154168
If you find security concerns:
155169
1. Add a comment to the PR explaining each concern
@@ -165,7 +179,7 @@ If no security issues are found:
165179

166180
**SECURITY**: Be thorough but avoid false positives. Focus on actual security weakening, not code style or refactoring that maintains the same security level.
167181

168-
## Changed Files (Pre-fetched, up to 100 KB)
182+
## Changed Files (Pre-fetched; security-relevant patches in full)
169183

170184
The following PR diff has been pre-computed. Focus your security analysis on these changes:
171185

0 commit comments

Comments
 (0)