-
Notifications
You must be signed in to change notification settings - Fork 0
fix(guard): scan PR titles and commit messages, not just bodies and files #73
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -12,6 +12,15 @@ name: public-repo-guard-body | |||||
| # for naming a private repo in wrangler.toml while the very same name, with more | ||||||
| # operational detail attached, sat unchallenged in its body. | ||||||
| # | ||||||
| # THREE SURFACES, ONE RULE TABLE. This job scans the PR TITLE, the PR/issue/comment | ||||||
| # BODY, and — added here — EVERY COMMIT MESSAGE in the pull request, all through the | ||||||
| # same body-policy.sh. The commit-message half closes the last text surface a public | ||||||
| # repo publishes that nothing read: on 2026-09-10 an internal tracking id inside a | ||||||
| # conventional-commit scope ("fix(<ID>): …") reached a public repo in a PR title AND | ||||||
| # in the commit messages beneath it, because the only gate in front of it read FILE | ||||||
| # CONTENT. A title and a commit message are as permanent as a body — `git log` keeps | ||||||
| # the message even after a body is edited — so they are held to the same table. | ||||||
| # | ||||||
| # This job's check-run name ("Body content policy") is NOT a required status | ||||||
| # context in this repo's ruleset, so it can safely trigger on every comment/review | ||||||
| # event without any risk of masking or wedging the required tree-scan context — | ||||||
|
|
@@ -42,8 +51,16 @@ on: | |||||
|
|
||||||
| # `pull_request`, deliberately NOT `pull_request_target`: a fork PR must never get | ||||||
| # a write token or repo secrets just because a gate wanted to read its body. | ||||||
| # | ||||||
| # `pull-requests: read` is the ONLY addition, and it is read-only: the commit-message | ||||||
| # step below lists the PR's commits through the REST API rather than deepening the | ||||||
| # checkout. Reading them from the API keeps this job's `sparse-checkout` of the | ||||||
| # gate's own scripts intact (no full history, no full tree on every comment) and | ||||||
| # keeps the token read-only — a gate that needed write scope to read text would be a | ||||||
| # worse trade than the gap it closes. | ||||||
| permissions: | ||||||
| contents: read | ||||||
| pull-requests: read | ||||||
|
|
||||||
| jobs: | ||||||
| body-guard: | ||||||
|
|
@@ -116,7 +133,48 @@ jobs: | |||||
| "$GITHUB_EVENT_PATH" > "$RUNNER_TEMP/bodyscan/body.txt" | ||||||
| echo "scanning $(wc -l < "$RUNNER_TEMP/bodyscan/body.txt") line(s) of body text" | ||||||
|
|
||||||
| - name: body policy (PR / issue / comment text) | ||||||
| - name: body policy (PR title / issue / comment text) | ||||||
| env: | ||||||
| GUARD_PRIVATE_REPOS: ${{ vars.GUARD_PRIVATE_REPOS }} | ||||||
| run: bash scripts/public-repo-guard/body-policy.sh "$RUNNER_TEMP/bodyscan/body.txt" | ||||||
|
|
||||||
| # COMMIT MESSAGES — the surface nothing read until now. Same discipline as the | ||||||
| # title/body step above: the untrusted text goes API -> FILE -> script path, and | ||||||
| # is never interpolated into a run: block, never placed in an environment | ||||||
| # variable. The two values that ARE interpolated are the repository slug and the | ||||||
| # PR number, both produced by GitHub and neither author-controlled. | ||||||
| # | ||||||
| # Read from the API, not from `git log`: this job checks out only | ||||||
| # scripts/public-repo-guard at depth 1, so there is no base..head range on disk, | ||||||
| # and a fetch-depth-0 checkout on every comment event would be a much larger bill | ||||||
| # than one paginated read. | ||||||
| # | ||||||
| # --paginate because a PR is not always small; the endpoint caps at 250 commits, | ||||||
| # and past that a PR is being asked to do a branch's job — the guard still reads | ||||||
| # the first 250 and the tree scan is unaffected. | ||||||
| - name: Materialize the PR commit messages to a file | ||||||
| if: github.event_name == 'pull_request' | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: Custom agent: Enforce Pragmatic Test Coverage The new workflow-level commit scan has no success or empty-result failure test. Existing fixtures exercise Prompt for AI agents |
||||||
| env: | ||||||
| GH_TOKEN: ${{ github.token }} | ||||||
| PR_NUMBER: ${{ github.event.pull_request.number }} | ||||||
| run: | | ||||||
| set -euo pipefail | ||||||
| mkdir -p "$RUNNER_TEMP/bodyscan" | ||||||
| gh api --paginate "repos/${GITHUB_REPOSITORY}/pulls/${PR_NUMBER}/commits" \ | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P1: Replace the pull-request commits REST request with a complete commit enumeration. GitHub caps this endpoint at 250 commits even with Prompt for AI agents |
||||||
| --jq '.[].commit.message' > "$RUNNER_TEMP/bodyscan/commits.txt" | ||||||
|
Comment on lines
+163
to
+164
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🛡️ Analyzed with Security Review | 🟠 Major | 🏗️ Heavy lift 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- workflow context ---'
sed -n '120,205p' .github/workflows/public-repo-guard-body.yml
printf '%s\n' '--- related policy references ---'
rg -n -C 3 'bodyscan|commits\.txt|commit.message|blocked|secret|identifier|pull_request' .github/workflows/public-repo-guard-body.yml .github/workflows/public-repo-guard.ymlRepository: wave-av/cli Length of output: 21377 🤖 get_repo_knowledge executed:
Length of output: 913 🌐 Web query:
💡 Result: <search_synthesis> <source_evidence> Citations:
🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- workflow permissions and triggers ---'
sed -n '1,120p' .github/workflows/public-repo-guard-body.yml
printf '%s\n' '--- policy implementation ---'
sed -n '1,240p' scripts/public-repo-guard/body-policy.sh
printf '%s\n' '--- commit-scanning references ---'
rg -n -C 4 'commit|private|internal|GUARD_PRIVATE_REPOS|body-policy' scripts/public-repo-guard .github/workflows/public-repo-guard-body.ymlRepository: wave-av/cli Length of output: 50367 🌐 Web query:
💡 Result: <search_synthesis> <source_evidence> Citations:
Information Disclosure Reachability: External Retrieve all pull request commits before scanning
Use the paginated GraphQL 🤖 Prompt for AI AgentsThere was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: When the API response omits Prompt for AI agents
Suggested change
|
||||||
| # EMPTY IS A FAILURE, never a pass. Every pull request has at least one | ||||||
| # commit, so an empty file means the API shape moved, the token lost read | ||||||
| # access, or pagination returned nothing — and a gate that reports success | ||||||
| # over text it never read is worse than no gate (the same argument the | ||||||
| # title/body step makes about an unrecognized event payload). | ||||||
| if [ ! -s "$RUNNER_TEMP/bodyscan/commits.txt" ]; then | ||||||
| echo "::error title=public-repo-guard-body::Listed 0 commit messages for PR #${PR_NUMBER} — a pull request always has at least one. Refusing to report a pass on unscanned commit messages." | ||||||
| exit 1 | ||||||
| fi | ||||||
| echo "scanning $(wc -l < "$RUNNER_TEMP/bodyscan/commits.txt") line(s) of commit-message text" | ||||||
|
|
||||||
| - name: body policy (PR commit messages) | ||||||
| if: github.event_name == 'pull_request' | ||||||
| env: | ||||||
| GUARD_PRIVATE_REPOS: ${{ vars.GUARD_PRIVATE_REPOS }} | ||||||
| run: bash scripts/public-repo-guard/body-policy.sh "$RUNNER_TEMP/bodyscan/commits.txt" | ||||||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -2,19 +2,29 @@ | |||||||||
| # WAVE public-repo BODY policy — the internal-leak gate for PR/issue/comment text. | ||||||||||
| # | ||||||||||
| # Companion to content-policy.sh. That script scans the published working TREE; | ||||||||||
| # this one scans the other half of a public repo's surface: pull-request titles | ||||||||||
| # and bodies, issue bodies, and comment bodies. Those are equally world-readable | ||||||||||
| # and, until this script existed, were scanned by NOTHING server-side. That gap | ||||||||||
| # was not theoretical — a PR was merged whose wrangler.toml was correctly BLOCKED | ||||||||||
| # for naming a private repo while the PR body named the same repo, with more | ||||||||||
| # operational detail attached, and sailed through. | ||||||||||
| # this one scans the other half of a public repo's surface: pull-request TITLES | ||||||||||
| # and bodies, every COMMIT MESSAGE in a pull request, issue bodies, and comment | ||||||||||
| # bodies. Those are equally world-readable and, until this script existed, were | ||||||||||
| # scanned by NOTHING server-side. That gap was not theoretical — a PR was merged | ||||||||||
| # whose wrangler.toml was correctly BLOCKED for naming a private repo while the PR | ||||||||||
| # body named the same repo, with more operational detail attached, and sailed | ||||||||||
| # through. | ||||||||||
| # | ||||||||||
| # Usage: scripts/public-repo-guard/body-policy.sh <file> | ||||||||||
| # <file> holds the untrusted text, already materialized to disk. It is passed as | ||||||||||
| # a PATH and only ever read — the body is never interpolated into a command line | ||||||||||
| # or an environment variable, so no amount of shell metacharacters in a PR body | ||||||||||
| # can influence what runs here. | ||||||||||
| # | ||||||||||
| # The workflow calls this script once per SURFACE, each with its own file: the | ||||||||||
| # title+body payload, and the concatenated commit messages of the pull request. | ||||||||||
| # One script, one rule table, three surfaces — so a title and a commit message | ||||||||||
| # can never be held to a weaker standard than a body. A title-shaped leak is not | ||||||||||
| # hypothetical: a tracking id inside a conventional-commit scope | ||||||||||
| # ("fix(<ID>): …") reached a public repo on 2026-09-10 because the only gate in | ||||||||||
| # front of it read FILE CONTENT, and neither the title nor the commit messages | ||||||||||
| # that carried the same string were read by anything. | ||||||||||
| # | ||||||||||
| # Exit: 0 clean · 1 blocking violation · 2 scanner error (fail closed). | ||||||||||
| # | ||||||||||
| # Allowlisting: unlike the tree scanner, where a `guard:allow` marker lands in a | ||||||||||
|
|
@@ -147,6 +157,59 @@ check BLOCK abs-user-path '/(Users|home)/(?!runner/)[a-z][a-z0-9._-]+/' 'O | |||||||||
| # `guard:allow <reason>` already exists as the honest, visible one. | ||||||||||
| check BLOCK internal-marker '(?<![“"'"'"'`])\b(internal[- ]only|do\s+not\s+(share|publish|distribute)|for\s+internal\s+use)\b(?![”"'"'"'`])' 'Text self-identifies as not-for-public' about-the-control-exempt | ||||||||||
|
|
||||||||||
| # --- Internal tracking ids and internal document paths ----------------------- | ||||||||||
| # THE TITLE / COMMIT-MESSAGE CLASS. A conventional-commit scope is the single most | ||||||||||
| # likely place for an internal id to reach a public repo: the id is how the work is | ||||||||||
| # tracked internally, the scope is where a habit puts it, and a file-content scan | ||||||||||
| # never reads a title or a commit message at all. One walked through on 2026-09-10. | ||||||||||
| # | ||||||||||
| # GENERIC SHAPES ONLY — this file is itself world-readable, so every pattern below | ||||||||||
| # is a CLASS (letter/digit silhouettes, a directory prefix, a wikilink form). Not | ||||||||||
| # one private repo name, product name, partner name or real id appears here; the | ||||||||||
| # repo-name half of the policy stays where it belongs, in the run-time | ||||||||||
| # GUARD_PRIVATE_REPOS variable used by the rule further down. | ||||||||||
| # | ||||||||||
| # The four regexes are kept in LOCKSTEP with the client-side pre-write gate's | ||||||||||
| # equivalent table, which was measured against real merged public PRs before it | ||||||||||
| # shipped. Keeping them byte-identical is the point: two gates that disagree about | ||||||||||
| # the same policy is how a leak lands in the gap between them (exactly the | ||||||||||
| # body-vs-wrangler.toml disagreement documented at the top of this file). If you | ||||||||||
| # tune one, tune both. | ||||||||||
| # | ||||||||||
| # All four are `about-the-control-exempt`: a PR that CHANGES this gate has to be | ||||||||||
| # able to describe what it now blocks, and a body that names the gate is prose | ||||||||||
| # about the control, not a leak through it. Same use-vs-mention trade as | ||||||||||
| # internal-marker above, and the same reason — a gate that blocks its own pull | ||||||||||
| # requests gets switched off. A credential rule above still gets no such escape. | ||||||||||
|
|
||||||||||
| # Internal criterion / ticket id: the XX-### tracking silhouette. The lookahead | ||||||||||
| # exempts standards, algorithms and CVE-style names that share the shape | ||||||||||
| # (SHA-256, PEP-503, ISO-8601, CWE-200) and the lowercase path/branch words | ||||||||||
| # (fix/issue-123, step-001); the lookbehind exempts an id embedded in a path or a | ||||||||||
| # dotted name. Three digits exactly, so CVE-2025-12345 and RFC-7231 stay free. | ||||||||||
| check BLOCK internal-id \ | ||||||||||
| '(?<![\w/.-])(?!(?:SHA|AES|HMAC|RSA|ECDSA|ECDH|CRC|NIST|RFC|PEP|IEEE|ISO|IEC|UTF|SMPTE|EBU|ANSI|MPEG|HEVC|BCP|ITU|IETF|FIPS|OWASP|CWE|issue|issues|pr|pull|fix|bug|task|step|test|tests|node|port|run|job|item|part|page|line|v|rev|build)-)[A-Z]{2,8}-\d{3}(?![\w-])' \ | ||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: In Prompt for AI agents
Suggested change
|
||||||||||
| 'Internal criterion / ticket id (the XX-### tracking shape) — internal tracking state, not public product detail' \ | ||||||||||
| about-the-control-exempt | ||||||||||
|
Comment on lines
+190
to
+193
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: The generic pattern blocks ordinary public identifiers such as Assessment: 🟠 Prompt for AI Agent 🤖This is a comment left during a code review.
**Path:** scripts/public-repo-guard/body-policy.sh
**Line:** 190:193
**Comment:**
*Incorrect Condition Logic: The generic pattern blocks ordinary public identifiers such as `API-123` or `SDK-001`, causing legitimate titles and commit messages to fail the publication gate.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fixThere was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🛡️ Analyzed with Security Review | 🟠 Major | 🏗️ Heavy lift 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- target policy structure ---'
sed -n '1,260p' scripts/public-repo-guard/body-policy.sh
printf '%s\n' '--- related policy references ---'
rg -n -C 3 'about-the-control-exempt|body-policy|content-policy|allowlist|exempt|internal|document|path' scripts/public-repo-guardRepository: wave-av/cli Length of output: 50367 🤖 get_repo_knowledge executed:
Length of output: 963 Information Disclosure Reachability: External Do not let control prose suppress internal-identifier and document-path matches
Restrict this exemption to trusted fixtures, or match only the exact explanatory prose and scan the remaining content on the line. 🤖 Prompt for AI AgentsThere was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P1: Do not apply Prompt for AI agents |
||||||||||
|
|
||||||||||
| # Decision-record id: who decided what, and when, in one token. | ||||||||||
| check BLOCK internal-decision-id '\bIGV-[A-Z]-\d{3}\b' \ | ||||||||||
| 'Internal decision-record id — the record of who decided what is not public' \ | ||||||||||
| about-the-control-exempt | ||||||||||
|
|
||||||||||
| # Epic / plan / workstream id (E4-SOME-THING): names an internal workstream. | ||||||||||
| check BLOCK internal-plan-id '\bE\d{1,2}-[A-Z]{3,}(?:-[A-Z]{3,})+\b' \ | ||||||||||
| 'Internal plan / workstream id — names an internal programme of work' \ | ||||||||||
| about-the-control-exempt | ||||||||||
|
|
||||||||||
|
Comment on lines
+190
to
+204
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||||||||
| # Internal document paths: an internal-process directory, a long-hyphenated rule | ||||||||||
| # filename, or a [[wikilink]] to one. The four-plus-word rule-file shape keeps an | ||||||||||
| # eslint-style rules/no-unused-vars.md clean. | ||||||||||
| check BLOCK internal-doc-path \ | ||||||||||
| '(?<![\w-])governance/(?:bin|lib|plans|rules|sources|data|test|vendor-bundles)/|\brules/[a-z0-9]+(?:-[a-z0-9]+){3,}\.md\b|\[\[[a-z0-9]+(?:-[a-z0-9]+){2,}\]\]' \ | ||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: The four-component filename heuristic blocks public ESLint paths such as Prompt for AI agents
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: A legitimate reference such as Prompt for AI agents
Suggested change
|
||||||||||
| 'Internal process document path or wikilink — internal document layout is not public' \ | ||||||||||
| about-the-control-exempt | ||||||||||
|
Comment on lines
+208
to
+211
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: The directory alternative blocks any public reference under Assessment: 🟠 Prompt for AI Agent 🤖This is a comment left during a code review.
**Path:** scripts/public-repo-guard/body-policy.sh
**Line:** 208:211
**Comment:**
*Logic Error: The directory alternative blocks any public reference under `governance/plans`, `governance/rules`, or similar paths, including legitimate project documentation.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix |
||||||||||
|
|
||||||||||
| # --- Private repo + operational detail (PROXIMITY, not bare name) ------------ | ||||||||||
| # The BODY profile deliberately DIVERGES from the FILE profile here, and the | ||||||||||
| # divergence is the whole design. content-policy.sh blocks a bare private-repo | ||||||||||
|
|
||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: The pull-request commits endpoint is limited to 250 commits, so commits beyond that limit are never scanned despite the workflow claiming to scan every commit message. [incomplete implementation]
Assessment: 🔴
Critical· 🔁Occurrence: RarelyPrompt for AI Agent 🤖