Skip to content

Fix: [for cherry-picking] Removed the allowlist filter pipeline's fail- #23

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
qodo-code-review wants to merge 1 commit into from
Closed

Fix: [for cherry-picking] Removed the allowlist filter pipeline's fail- #23

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions scripts/public-repo-guard/body-policy.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,15 @@ check() {
# Filter with rg, not grep: BSD/macOS grep has no -P, so a `grep -P` allowlist
# silently errors out locally while working on GNU/CI — the gate would then
# disagree with itself depending on where it ran. rg is already required above.
local matches
local matches filter_rc
matches="$(printf '%s' "$raw" \
| rg -vN -- 'guard:allow[[:space:]]+[^[:space:]]' \
| rg -vNiP -- "$ABOUT_THE_CONTROL" || true)"
| rg -vNiP -- "$ABOUT_THE_CONTROL")"
filter_rc=$?
if (( filter_rc >= 2 )); then
echo "::error title=public-repo-guard ($name)::ripgrep failed (exit $filter_rc) while applying allowlists — failing closed."
exit 2
fi
Comment on lines 53 to +60

@devin-ai-integration devin-ai-integration Bot Aug 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Scanner failure in the first allowlist step can still go unnoticed and let text pass unchecked

The new failure check reads only the combined result of the two-step allowlist filter (filter_rc=$? at scripts/public-repo-guard/body-policy.sh:56) instead of each step's own result, so a crash in the first step is hidden by the second step's normal "nothing left" result and the text is declared clean.
Impact: If part of the text scanner breaks, the gate can silently report success instead of blocking, allowing leaked internal text to be published.

How bash pipefail masks the earlier command's exit code

With set -o pipefail, the pipeline status is the exit status of the rightmost command that exited non-zero, not the maximum. In the pipeline at scripts/public-repo-guard/body-policy.sh:53-55, if the first rg -vN (guard:allow filter) fails with exit 2, it emits no output; the second rg -vNiP then reads empty input and exits 1 (no lines selected). The pipeline status is therefore 1, filter_rc is 1, the >= 2 guard at line 57 does not trigger, matches is empty, and check returns 0 as clean — exactly the fail-open the change intends to prevent. Only a failure in the last stage is currently caught. Using ${PIPESTATUS[@]} (captured immediately after the assignment) and failing if any element is >= 2 closes the gap.

Suggested change
matches="$(printf '%s' "$raw" \
| rg -vN -- 'guard:allow[[:space:]]+[^[:space:]]' \
| rg -vNiP -- "$ABOUT_THE_CONTROL" || true)"
| rg -vNiP -- "$ABOUT_THE_CONTROL")"
filter_rc=$?
if (( filter_rc >= 2 )); then
echo "::error title=public-repo-guard ($name)::ripgrep failed (exit $filter_rc) while applying allowlists — failing closed."
exit 2
fi
matches="$(printf '%s' "$raw" \
| rg -vN -- 'guard:allow[[:space:]]+[^[:space:]]' \
| rg -vNiP -- "$ABOUT_THE_CONTROL")"
local -a filter_st=( "${PIPESTATUS[@]}" )
for filter_rc in "${filter_st[@]}"; do
if (( filter_rc >= 2 )); then
echo "::error title=public-repo-guard ($name)::ripgrep failed (exit $filter_rc) while applying allowlists — failing closed."
exit 2
fi
done
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +55 to +60

@devin-ai-integration devin-ai-integration Bot Aug 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 Sibling scanner content-policy.sh still swallows allowlist-filter errors

The same fail-open pattern the PR fixes here still exists in the companion script: scripts/public-repo-guard/content-policy.sh:60 uses grep -vE ... || true for its guard:allow filter, so a grep failure there yields empty matches and a clean result. Worth applying the same treatment for consistency between the two gates.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines 53 to +60

@devin-ai-integration devin-ai-integration Bot Aug 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟨 Leak gate can silently pass when the first allowlist filter errors

The new fail-closed check inspects only the pipeline's aggregate status (filter_rc=$? at scripts/public-repo-guard/body-policy.sh:56). Under bash pipefail the status is the rightmost non-zero exit, so an error (exit 2) in the first rg -vN allowlist filter (scripts/public-repo-guard/body-policy.sh:54) is masked by the second filter's normal exit 1, leaving matches empty and the body reported as clean. This is a fail-open in a security gate whose purpose is to block secrets and internal detail from being published.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

[[ -z "$matches" ]] && return 0
local count; count="$(printf '%s\n' "$matches" | grep -c '')"
# Print the LINE NUMBER only — never the matched text. This annotation is itself
Expand Down
Loading