docs(standards): propose push protection standard#134
Conversation
Add a dedicated standard defining a defense-in-depth approach for preventing secrets, keys, and sensitive values from being pushed to any petry-projects repo. Covers GitHub native secret scanning + push protection (primary enforcement), local gitleaks pre-commit hooks, complementary CI secret-scan job, agent hygiene, incident response, and compliance audit checks. Cross-reference the new standard from AGENTS.md and add a "Security & Analysis" block to github-settings.md documenting the required repo-level security_and_analysis settings.
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 22 minutes and 31 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughAdds an organization-wide push-protection standard and enforces it via documentation and automation: new Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(220,220,255,0.5)
participant Dev as Developer (local)
end
rect rgba(200,255,200,0.5)
participant PreCommit as Local pre-commit (gitleaks)
end
rect rgba(255,220,200,0.5)
participant GH as GitHub (push protection & secret scanning)
end
rect rgba(255,255,200,0.5)
participant CI as CI (secret-scan job)
end
rect rgba(200,255,255,0.5)
participant Audit as Compliance Audit Script
end
rect rgba(255,200,255,0.5)
participant IR as Incident Response Workflow
end
Dev->>PreCommit: commit/push (pre-commit runs gitleaks)
PreCommit-->>Dev: block or allow commit
Dev->>GH: push
GH-->>Dev: accept or block push (push protection)
GH->>CI: on push, run secret-scan job (gitleaks)
CI-->>GH: report status (required check)
Audit->>GH: fetch repo JSON/settings
Audit->>GH: evaluate push-protection and required checks
Audit-->>IR: emit findings if failures detected
IR-->>Dev: remediation steps (rotate/revoke, advisory, post-mortem)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Adds an organization-wide documentation standard for preventing secrets from being committed/pushed, centered on GitHub secret scanning + push protection, with local and CI scanning as defense-in-depth.
Changes:
- Add a new Push Protection standard (
standards/push-protection.md) covering scope, enforcement layers, incident response, and audit expectations. - Document required repository
security_and_analysissettings instandards/github-settings.md. - Cross-reference the new standard from
AGENTS.md.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| standards/push-protection.md | Introduces the proposed push-protection standard, including settings, CI job guidance, and response procedures. |
| standards/github-settings.md | Adds a “Security & Analysis” block tying repo settings to the push-protection standard. |
| AGENTS.md | Adds the push-protection standard to the org standards index for agents. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ```bash | ||
| pip install pre-commit | ||
| pre-commit install | ||
| pre-commit run gitleaks --all-files # one-off scan of existing history |
There was a problem hiding this comment.
pre-commit run gitleaks --all-files runs the hook against all tracked files, but it doesn’t scan git history. The inline comment says “one-off scan of existing history”, which is misleading—either change the comment to reflect what it does, or replace with a gitleaks detect invocation that actually scans commit history.
| pre-commit run gitleaks --all-files # one-off scan of existing history | |
| pre-commit run gitleaks --all-files # one-off scan of all tracked files |
| uses: actions/checkout@v5 | ||
| with: | ||
| fetch-depth: 0 | ||
|
|
||
| - name: Run gitleaks | ||
| uses: gitleaks/gitleaks-action@v2 |
There was a problem hiding this comment.
The CI job example uses tag-based action refs (actions/checkout@v5, gitleaks/gitleaks-action@v2), but the repo’s Action Pinning Policy requires SHA pinning (see standards/ci-standards.md:587-595). Update the example to use SHA-pinned uses: lines so readers don’t copy a pattern that violates the org standard.
| uses: actions/checkout@v5 | |
| with: | |
| fetch-depth: 0 | |
| - name: Run gitleaks | |
| uses: gitleaks/gitleaks-action@v2 | |
| uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5 | |
| with: | |
| fetch-depth: 0 | |
| - name: Run gitleaks | |
| uses: gitleaks/gitleaks-action@44c9b0a5f4a6d0f8a6c3b1d2e7f9a0b1c2d3e4f5 # v2 |
| - Fail the build (`--exit-code 1`) when any finding at or above severity | ||
| `medium` is detected |
There was a problem hiding this comment.
The bullets say the job should fail only when a finding at/above severity medium is detected, but the provided args (gitleaks detect ... --exit-code 1) don’t encode that threshold and will typically fail on any finding. Either adjust the wording to match the command’s behavior, or document the config/flags needed to enforce a medium+ threshold.
| - Fail the build (`--exit-code 1`) when any finding at or above severity | |
| `medium` is detected | |
| - Fail the build (`--exit-code 1`) when `gitleaks` reports any finding |
| Every repository MUST have the following security features turned on. The | ||
| compliance audit checks these flags via `GET /repos/{owner}/{repo}`: |
There was a problem hiding this comment.
The doc states “the compliance audit checks these flags via GET /repos/{owner}/{repo}”, but scripts/compliance-audit.sh currently doesn’t check any security_and_analysis.secret_scanning* fields. Consider rephrasing to avoid implying this is already implemented (or add a pointer that the audit script must be updated as a follow-up).
| Every repository MUST have the following security features turned on. The | |
| compliance audit checks these flags via `GET /repos/{owner}/{repo}`: | |
| Every repository MUST have the following security features turned on. These | |
| flags are exposed via `GET /repos/{owner}/{repo}` and SHOULD be included in the | |
| compliance audit: |
| "secret_scanning": {"status": "enabled"}, | ||
| "secret_scanning_push_protection": {"status": "enabled"}, | ||
| "secret_scanning_ai_detection": {"status": "enabled"}, | ||
| "secret_scanning_non_provider_patterns": {"status": "enabled"} |
There was a problem hiding this comment.
The required repo-level settings table includes security_and_analysis.dependabot_security_updates.status, but the example gh api ... -F security_and_analysis=... payload doesn’t set dependabot_security_updates. Update the example to include it (or remove it from the required settings list if it’s out of scope for this call).
| "secret_scanning_non_provider_patterns": {"status": "enabled"} | |
| "secret_scanning_non_provider_patterns": {"status": "enabled"}, | |
| "dependabot_security_updates": {"status": "enabled"} |
| `scripts/apply-repo-settings.sh` MUST enforce these values alongside the | ||
| existing merge and label settings — see |
There was a problem hiding this comment.
scripts/apply-repo-settings.sh is referenced as already enforcing these security_and_analysis values, but the current script doesn’t set any security_and_analysis fields. Either update the script in this PR, or adjust the wording to make this an explicit future requirement (with a follow-up reference) rather than current behavior.
| `scripts/apply-repo-settings.sh` MUST enforce these values alongside the | |
| existing merge and label settings — see | |
| `scripts/apply-repo-settings.sh` does not yet enforce these | |
| `security_and_analysis` values. A follow-up change MUST add that enforcement | |
| alongside the existing merge and label settings; until then, apply these | |
| settings using the API example above. See |
| | Pattern name | Regex (illustrative) | Rationale | | ||
| |--------------|----------------------|-----------| | ||
| | `petry-internal-webhook` | `https://hooks\.petry-projects\.internal/[A-Za-z0-9/_-]{20,}` | Internal webhook URLs | | ||
| | `claude-oauth-token` | `sk-ant-oat01-[A-Za-z0-9_-]{40,}` | Anthropic OAuth tokens | | ||
| | `gha-pat-scoped` | `github_pat_[A-Za-z0-9_]{82}` | Fine-grained GitHub PATs (provider pattern supplements) | | ||
| | `generic-high-entropy` | High-entropy strings assigned to `*_TOKEN`, `*_SECRET`, `*_KEY` env vars | Catches untyped long strings in YAML and `.env` files | |
There was a problem hiding this comment.
In the custom patterns table, the “Regex (illustrative)” column contains a non-regex description for generic-high-entropy. Consider either providing an actual illustrative regex/pattern definition, or rename the column to something like “Pattern (illustrative)” so readers don’t try to paste a non-regex into GitHub’s custom pattern UI.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@standards/push-protection.md`:
- Around line 92-110: The PATCH example omits setting
security_and_analysis.dependabot_security_updates, causing a mismatch with the
table; update the gh api -X PATCH "repos/petry-projects/<repo>" example payload
to include "dependabot_security_updates": {"status": "enabled"} inside the
security_and_analysis object so the command sets
dependabot_security_updates.status to "enabled" along with the other
secret_scanning* keys.
- Around line 112-114: The script currently claims to “MUST enforce”
push-protection but scripts/apply-repo-settings.sh does not patch
security_and_analysis; either add code to the existing block that applies repo
settings (the logic around the merge/label enforcement in
scripts/apply-repo-settings.sh, currently handling repo patching in the 77-161
region) to also PATCH the repository via the GitHub API (or gh api) with a
security_and_analysis payload (setting advanced_security, secret_scanning,
secret_scanning_push_protection as required), or if you cannot implement it now,
update the standard text to mark enforcement as pending/future enforcement to
avoid a false claim; locate the repo-patching logic in
scripts/apply-repo-settings.sh (the function/section that constructs the repo
settings payload and calls gh api or curl) and extend it to include the
security_and_analysis fields or change the standard statement accordingly.
- Around line 218-224: The gitleaks invocation currently uses only --exit-code
and will fail on low-severity findings; either change the command to enforce a
medium+ threshold by pointing gitleaks to a config that disables low-severity
rules (create a gitleaks config with disabledRules or rule severity set and pass
it via --config) or selectively enable only medium+ rules via --enable-rule
flags in the same gitleaks detect call; alternatively, update the prose to state
that --exit-code 1 fails on any finding (including low) if you do not add a
severity-filtering config or flags.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 80612379-0991-4eab-86e7-53ff519ff729
📒 Files selected for processing (3)
AGENTS.mdstandards/github-settings.mdstandards/push-protection.md
- Fix misleading comment: pre-commit scans tracked files, not history - SHA-pin actions/checkout, add placeholder note for gitleaks-action SHA - Fix severity wording: --exit-code 1 fails on any finding, not medium+ - Rephrase compliance audit reference as aspirational (not yet implemented) - Add dependabot_security_updates to the API example payload - Mark apply-repo-settings.sh enforcement as a follow-up requirement - Rename "Regex (illustrative)" column to "Pattern (illustrative)" and provide an actual regex for generic-high-entropy
- apply-repo-settings.sh: add apply_security_analysis() to enforce secret_scanning, push_protection, ai_detection, non_provider_patterns, and dependabot_security_updates on every repo - compliance-audit.sh: add check_push_protection() with 7 checks: secret_scanning_enabled, push_protection_enabled, non_provider_patterns_enabled, open_secret_alerts, secret_scan_ci_job_present, gitignore_secrets_block, push_protection_bypasses_recent - github-settings.md: add Secret Scan (gitleaks) as the 6th required check in the code-quality ruleset - push-protection.md: pin gitleaks/gitleaks-action to SHA ff98106e4c7b2bc287b24eaf42907196329070c7 (v2), update enforcement wording to reflect implemented state
- Deduplicate API calls: fetch repo JSON once in the per-repo loop and
pass it to both apply_settings/apply_security_analysis (apply script)
and check_repo_settings/check_push_protection (audit script), halving
API usage per repo
- Fix audit drift bug: check_push_protection now audits all 5
security_and_analysis settings (was missing ai_detection and
dependabot_security_updates)
- Build JSON payload with jq instead of string concatenation
- Fix error handling: apply_security_analysis returns 1 on PATCH failure
- Remove fragile hardcoded check count ("six") from github-settings.md
- Fix ShellCheck SC2168: remove `local` outside functions in main block
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
standards/github-settings.md (1)
145-159:⚠️ Potential issue | 🟠 MajorDon’t mark
Secret Scanas universally enforced until ruleset automation can add it.
scripts/apply-rulesets.sh:48-140still only detects SonarCloud, CodeQL, Claude, Coverage, and the general CI pipeline when building thecode-qualityruleset. Repos provisioned from the current automation will therefore miss this new required check while the standard says it is mandatory everywhere.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@standards/github-settings.md` around lines 145 - 159, The code-quality ruleset builder in scripts/apply-rulesets.sh currently only adds SonarCloud, CodeQL, Claude, Coverage, and CI pipeline checks; update the ruleset construction logic (the block that builds the code-quality ruleset around lines 48–140) to also detect and include the Secret Scan check by name "Secret scan (gitleaks)" (and/or add it to the checks array/required_checks list used when provisioning repos), ensure the detection logic that scans repo capabilities recognizes gitleaks/full-history secret-scan support, and add the Secret Scan entry to any emitted ruleset JSON/YAML so newly provisioned repos get the required check configured.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/apply-repo-settings.sh`:
- Around line 220-231: The gh api call is sending the jq-built JSON payload
incorrectly with -F; change the request to send the pre-built JSON body via
--input (and include Content-Type: application/json) instead of -F; locate the
payload variable and the gh api PATCH call that currently uses -F
"security_and_analysis=$payload" and update it to pass the payload as the
request body with --input (using the payload variable) so the
security_and_analysis object is sent as JSON rather than as a literal string.
In `@scripts/compliance-audit.sh`:
- Around line 373-380: The script treats failures from gh_api as zero findings
by using fallbacks like alert_count=$(... || echo "0"); instead, detect and
handle API or jq failures: check the exit status or validate that alert_raw
contains valid JSON before parsing (referencing alert_raw, gh_api, alert_count
and the add_finding call), and if the API call or jq parsing fails do not set
alert_count to "0" — either add an explicit error/unknown finding (e.g., call
add_finding with a distinct severity/message indicating the API check failed) or
exit with a non-zero status after logging the failure so the report doesn't
silently claim "no findings." Ensure both the secret-scanning alerts block
(alert_raw/alert_count) and the bypasses block use the same failure-detection
and reporting behavior.
- Around line 1150-1158: The script currently assigns repo_json=$(gh_api
"repos/$ORG/$repo" ... || echo "{}") which lets check_repo_settings and
check_push_protection silently no-op on failure; update the logic so that if
gh_api failed (repo_json is empty, "{}", or gh_api returned non-zero) the script
fails closed by either logging an explicit error and exiting non-zero or setting
a global failure flag before calling check_repo_settings/check_push_protection;
specifically detect the gh_api failure after the repo_json assignment (using the
repo_json value or gh_api exit status), and ensure check_repo_settings and
check_push_protection are not invoked with the placeholder "{}" but instead the
script aborts or marks the repo as failing so audits aren't falsely reported
clean.
- Around line 382-393: The current heuristic uses a bare substring match on
ci_decoded for 'gitleaks', which can match comments or unrelated text; update
the check in the ci.yml handling (variables ci_decoded and the grep 'gitleaks'
usage that triggers add_finding for secret_scan_ci_job_present) to look for
actual job steps that invoke gitleaks (e.g., lines like "uses: .*gitleaks" or
"run: .*gitleaks" and common action identifiers such as
"zricethezav/gitleaks-action") and ignore commented lines first (strip lines
starting with # or require matches not preceded by #); replace the single grep
-q 'gitleaks' with a more specific grep -E that detects "uses:.*gitleaks" or
"run:.*gitleaks" (or use yq to parse jobs and steps) so add_finding only fires
when a real secret-scan job/step is present.
In `@standards/push-protection.md`:
- Around line 376-385: The checklist in standards/push-protection.md is missing
the mandatory repo settings secret_scanning_ai_detection and
dependabot_security_updates, causing it to diverge from the repo-level settings
and scripts/compliance-audit.sh; update the table to add rows for
`secret_scanning_ai_detection` (error) and `dependabot_security_updates` (error)
with the appropriate required expressions (matching the earlier settings style),
and ensure any descriptions or examples in the file reference the same checks
used by scripts/compliance-audit.sh so the documented audit contract matches the
actual checks.
- Around line 103-112: The example uses gh api with -F
security_and_analysis='{...}' which sends a string literal instead of nested
JSON; update the example to use either a heredoc/--input approach or
bracket-style form fields so GitHub CLI sends nested objects—e.g., replace the
-F security_and_analysis='{...}' usage in the shown gh api command with an
--input heredoc containing the JSON payload or use -F
security_and_analysis[secret_scanning][status]=enabled (and similarly for other
nested keys) so the settings (secret_scanning, secret_scanning_push_protection,
secret_scanning_ai_detection, secret_scanning_non_provider_patterns,
dependabot_security_updates) are correctly applied.
---
Outside diff comments:
In `@standards/github-settings.md`:
- Around line 145-159: The code-quality ruleset builder in
scripts/apply-rulesets.sh currently only adds SonarCloud, CodeQL, Claude,
Coverage, and CI pipeline checks; update the ruleset construction logic (the
block that builds the code-quality ruleset around lines 48–140) to also detect
and include the Secret Scan check by name "Secret scan (gitleaks)" (and/or add
it to the checks array/required_checks list used when provisioning repos),
ensure the detection logic that scans repo capabilities recognizes
gitleaks/full-history secret-scan support, and add the Secret Scan entry to any
emitted ruleset JSON/YAML so newly provisioned repos get the required check
configured.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0eb130db-687d-4827-b5e8-f199c668cf15
📒 Files selected for processing (4)
scripts/apply-repo-settings.shscripts/compliance-audit.shstandards/github-settings.mdstandards/push-protection.md
…r heuristics
- apply-repo-settings.sh: wrap security_and_analysis in full request body and
send via --input instead of -F (which cannot send nested JSON objects)
- compliance-audit.sh: fail closed when repo metadata fetch returns {}; add
explicit warning findings when secret-scanning alerts or bypass queries fail
rather than silently treating failures as zero findings; tighten gitleaks CI
check to match actual 'uses: gitleaks-action@' steps, not any gitleaks mention
- apply-rulesets.sh: detect gitleaks-action in ci.yml and add "Secret scan
(gitleaks)" to the code-quality required checks so newly provisioned repos
include it automatically
- push-protection.md: fix repo PATCH example to use --input heredoc; add
ai_detection_enabled and dependabot_security_updates_enabled rows to the
compliance audit checklist
https://claude.ai/code/session_01EoMazC6Mn4fjcUvkik7Epm
- push-protection.md: keep --input heredoc fix (correct gh api syntax), take main's shared library references, expanded pre-commit docs, detailed SHA comments, expanded gitignore section, and align severity levels with the PP_REQUIRED_SA_SETTINGS library definition - apply-repo-settings.sh: remove duplicate apply_security_analysis() (superseded by pp_apply_security_and_analysis from shared library) - compliance-audit.sh: remove duplicate check_push_protection() (superseded by pp_run_all_checks from shared library) - lib/push-protection.sh: tighten pp_check_secret_scan_ci_job heuristic to match actual gitleaks-action uses: refs, not bare substring mentions https://claude.ai/code/session_01EoMazC6Mn4fjcUvkik7Epm
|
Summary
standards/push-protection.mdproposing a defense-in-depth approach to prevent secrets, keys, and credentials from being accidentally pushed to any petry-projects repo — GitHub native secret scanning + push protection as primary enforcement, local gitleaks hooks, a complementary CIsecret-scanjob, and an incident response procedureAGENTS.mdand add aSecurity & Analysisblock tostandards/github-settings.mddocumenting the required repo-levelsecurity_and_analysissettingsWhat's covered in the new standard
security_and_analysissettings, recommended custom secret-scanning patterns, admin-only bypass with auditsecret-scanjob inci.ymlrunning gitleaks in full-history mode, coordination with AgentShield.gitignoreentries, fixture conventions, branch cleanupTest plan
markdownlint-cli2passes on all three changed filesscripts/apply-repo-settings.shto enforce the newsecurity_and_analysisflagsscripts/compliance-audit.shwith the checks listed in the "Compliance Audit Checks" section of the new standardsecret-scanjob to thecode-qualityruleset's required checksSummary by CodeRabbit