Skip to content

docs(standards): propose push protection standard#95

Merged
don-petry merged 7 commits intomainfrom
claude/push-protection-standard-2GjYF
Apr 9, 2026
Merged

docs(standards): propose push protection standard#95
don-petry merged 7 commits intomainfrom
claude/push-protection-standard-2GjYF

Conversation

@don-petry
Copy link
Copy Markdown
Contributor

@don-petry don-petry commented Apr 8, 2026

Summary

  • Add standards/push-protection.md proposing 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 CI secret-scan job, and an incident response procedure
  • Cross-reference the new standard from AGENTS.md and add a Security & Analysis block to standards/github-settings.md documenting the required repo-level security_and_analysis settings

What's covered in the new standard

  • Scope — every repo regardless of visibility; source, configs, workflows, agent files, fixtures, notebooks
  • What counts as a secret — cloud creds, API tokens, DB strings, private keys, OAuth secrets, signing keys
  • Layer 1 — GitHub push protection (primary enforcement): required org + repo security_and_analysis settings, recommended custom secret-scanning patterns, admin-only bypass with audit
  • Layer 2 — Local pre-commit prevention: gitleaks via pre-commit, agent workstation rules
  • Layer 3 — CI secret scanning (secondary defense): required secret-scan job in ci.yml running gitleaks in full-history mode, coordination with AgentShield
  • Developer practices: handling real secrets, required .gitignore entries, fixture conventions, branch cleanup
  • Push protection block response — what to do when GitHub blocks your push (no bypass without justification)
  • Incident response — rotate first, history rewrite later; 7-step procedure with owners
  • Application to a repo and compliance audit checks the weekly audit should add

Test plan

  • markdownlint-cli2 passes on all changed .md files
  • shellcheck --severity=warning -x scripts/**/*.sh passes (CI globbing also fixed to recurse into scripts/lib/)
  • scripts/apply-repo-settings.sh enforces the new security_and_analysis flags via pp_apply_security_and_analysis from scripts/lib/push-protection.sh
  • scripts/compliance-audit.sh runs the 5 push-protection checks via pp_run_all_checks from the same shared lib
  • Review the illustrative custom secret-scanning regexes before enabling them org-wide
  • Follow-up: add the secret-scan job to the code-quality ruleset's required checks (still needs detect_required_checks() update in scripts/apply-rulesets.sh)
  • Smoke-test the apply script in DRY_RUN=true mode against a single repo before running --all

Summary by CodeRabbit

  • Documentation
    • Added an org-wide push-protection standard covering prohibited secret types, enforcement layers, developer rules, CI secret-scan requirements, incident response, onboarding checklist, and compliance audit matrix.
    • Updated repository security guidance to require secret scanning, push protection (including AI detection and custom/non-provider patterns), and Dependabot security updates.
    • Added a secrets-focused .gitignore to ignore common credential and token artifacts.
    • Updated organization standards index to reference the new push-protection standard.

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.
Copilot AI review requested due to automatic review settings April 8, 2026 16:48
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 8, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds an organization-wide push-protection standard: AGENTS.md now references standards/push-protection.md; standards/github-settings.md requires security & analysis controls; new standards/push-protection.md and a secrets-focused .gitignore define enforcement layers, CI secret-scan job, local tooling, incident response, and audit checks.

Changes

Cohort / File(s) Summary
Organization Standards Mapping
AGENTS.md
Added an entry referencing standards/push-protection.md to the organization standards table.
Repository Security Settings
standards/github-settings.md
Added a "Security & Analysis" section requiring Secret scanning, Secret scanning push protection, Secret scanning AI detection, non-provider patterns, and Dependabot security updates; links to standards/push-protection.md for full requirements.
Push Protection Standard
standards/push-protection.md
New comprehensive standard covering scope, prohibited secret categories, org-level secret scanning & push-protection settings (custom patterns, admin-bypass rules), required repo settings, mandated CI secret-scan job (gitleaks, full-history, --redact, fail-on-findings), local/pre-commit guidance, incident response (rotate-first), onboarding checklist, and compliance/audit matrix.
Secrets-only ignores
.gitignore
Added a 381-line .gitignore focusing on secret/artifact patterns (dotenv, cloud CLI caches, key formats, tfstate/tfvars, secret-manager artifacts, DB dumps, tooling auth files, editor credential files) with explicit re-allow rules for public/template/encrypted variants.

Sequence Diagram(s)

mermaid
sequenceDiagram
participant Dev as Developer
participant Git as GitHub Push Protection
participant CI as CI (gitleaks)
participant Admin as Org Admin
Dev->>Git: Push / Open PR with commit(s)
Note over Git,CI: Org-level secret scanning & push protection enabled + org patterns
Git->>Git: Scan push with org + custom patterns
alt Secret detected by push protection
Git--xDev: Block push / fail push protection
Dev->>Dev: Remove secret, rotate credentials, update commit
Dev->>Git: Re-push / update PR
Admin->>Git: (optional) Approve admin bypass with justification
else No secret detected or after remediation
Git->>CI: Trigger secret-scan workflow on PR/main
CI->>CI: Run gitleaks (full-history, --redact); fail on findings
alt CI findings
CI--xDev: Fail required code-quality check; surface findings
Dev->>CI: Fix secrets, rotate, update PR
else No findings
CI->>Git: Pass required code-quality check
end
end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: proposing a push protection standard. It directly reflects the primary addition of standards/push-protection.md and related documentation updates.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/push-protection-standard-2GjYF

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds an org standard for secret-leak prevention via GitHub secret scanning + push protection, plus guidance for local and CI-based scanning, and wires that standard into the existing org standards docs.

Changes:

  • Add new standards/push-protection.md defining a defense-in-depth push protection standard (GitHub settings, local hooks, CI scan job, incident response).
  • Extend standards/github-settings.md with a “Security & Analysis” block documenting required security_and_analysis settings.
  • Cross-reference the new standard from AGENTS.md.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
standards/push-protection.md New push-protection/secret-scanning standard with org/repo settings, CI job guidance, and audit/IR procedures
standards/github-settings.md Documents required Security & Analysis repo settings and links to the new standard
AGENTS.md Adds the push protection standard to the org standards index

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread standards/push-protection.md Outdated
Comment thread standards/push-protection.md Outdated
Comment thread standards/push-protection.md
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

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 359-362: The repo onboarding automation doesn't add the new
required check, so update the detect_required_checks() implementation in
scripts/apply-rulesets.sh to recognize and insert the "secret-scan" check into
the code-quality required checks list (ensure the ruleset payload includes
"secret-scan" the same way other checks are handled), and also adjust any
mapping/validation logic that filters allowed checks so "secret-scan" isn't
rejected; after that, ensure github-settings.md (the Code Quality required
checks section) is updated if the ruleset template now needs an explicit entry.
- Around line 370-384: The policy lists seven mandatory audit 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) but the implementation
in scripts/compliance-audit.sh currently only validates ruleset existence;
either implement the missing checks in scripts/compliance-audit.sh (add
verification for security_and_analysis fields, GET
/repos/.../secret-scanning/alerts for open alerts, parse
.github/workflows/ci.yml for a gitleaks job, inspect .gitignore entries, and
check bypasses within 30 days) or update standards/push-protection.md to
accurately reflect what scripts/compliance-audit.sh actually enforces so the doc
and script do not diverge.
- Around line 223-224: Update the documentation text to remove the incorrect
severity qualifier for Gitleaks v8.24.0: replace "Fail the build (`--exit-code
1`) when any finding at or above severity `medium` is detected" with "Fail the
build (`--exit-code 1`) when any finding is detected" to reflect that
`--exit-code 1` fails on any finding; alternatively, document the recommended
approach using `--exit-code 0 --report-format json` plus a custom
post-processing step to map severities and fail CI accordingly.
🪄 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: 985a6a7a-6ad4-4f58-9846-2d37d191c98e

📥 Commits

Reviewing files that changed from the base of the PR and between 67cb057 and ee1e71e.

📒 Files selected for processing (3)
  • AGENTS.md
  • standards/github-settings.md
  • standards/push-protection.md

Comment thread standards/push-protection.md Outdated
Comment thread standards/push-protection.md
Comment thread standards/push-protection.md
@don-petry
Copy link
Copy Markdown
Contributor Author

@claude - review and resolve the active comments and findings

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Apr 8, 2026

Claude finished @don-petry's task in 2m 31s —— View job


Resolved active review comments

  • Gather context and read changed files
  • Fix: Add dependabot_security_updates to repo-level PATCH example (~line 108)
  • Fix: Pin action references to commit SHAs in workflow snippet (~line 211)
  • Fix: Correct gitleaks severity exit-code claim (~line 224)
  • Fix: Clarify secret-scan ruleset automation gap (~line 362)
  • Fix: Update compliance audit section — add missing checks + caveat about implementation gap (~lines 383–384)
  • Commit and push changes

Summary of changes (standards/push-protection.md)

Issue Fix
Missing dependabot_security_updates in PATCH example (line ~108) Added "dependabot_security_updates": {"status": "enabled"} to the API payload
Tag-based action refs violate Action Pinning Policy (line ~211) Pinned actions/checkout to de0fac2e... (v6.0.2, matching org standard); added lookup comment for gitleaks-action@v2 with # TODO: replace with pinned SHA before merging
Incorrect severity qualifier for gitleaks exit code (line ~224) Changed "at or above severity medium" → "when any finding is detected" (gitleaks v8.24.0 has no per-severity exit threshold)
Automation gap for secret-scan required check (line ~362) Step 4 now calls out that detect_required_checks() in scripts/apply-rulesets.sh must be updated before applying the ruleset org-wide
Missing audit checks + doc/implementation mismatch (lines ~383–384) Added ai_detection_enabled and dependabot_security_updates_enabled rows to the audit table; reworded intro to "target state" with a caveat that scripts/compliance-audit.sh needs extension before these checks are automated

Note on gitleaks-action SHA: The org's Action Pinning Policy says not to fabricate SHAs. Since I can't make outbound API calls, I've left a # TODO comment with the lookup command. The gitleaks-action SHA should be resolved before merging: gh api repos/gitleaks/gitleaks-action/git/refs/tags/v2 --jq '.object.sha'
|

- Add dependabot_security_updates to repo-level PATCH example payload
- Pin actions/checkout to SHA (v6.0.2) and add lookup comment for gitleaks-action v2
- Fix incorrect gitleaks severity exit-code claim (any finding, not medium+)
- Clarify secret-scan ruleset automation gap; note apply-rulesets.sh needs update
- Update compliance audit section: add ai_detection + dependabot checks, caveat about implementation gap

Co-authored-by: don-petry <don-petry@users.noreply.github.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 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 169-170: The doc incorrectly labels the shown commands
("pre-commit run gitleaks --all-files" and "gitleaks detect --source .") as
“history” scans; update the text to state these commands only scan the current
working tree / files, not the full git history, and replace or augment the
guidance with the correct history-scan command for gitleaks (e.g., call out
using gitleaks detect with the flag to scan all commits such as --all-commits or
the repo-history mode) and a short note that pre-commit --all-files is a
one-time working-tree scan rather than a history scan; ensure you update both
occurrences (the current lines and the other instance mentioned).
- Line 216: The workflow uses an unpinned action reference
"gitleaks/gitleaks-action@v2" which violates the Action Pinning Policy; replace
that tag with a specific commit SHA (e.g.,
"gitleaks/gitleaks-action@<full-commit-sha>") so the job is pinned, update any
comment TODO accordingly, and ensure the new SHA is documented in the standards
text to satisfy the policy.
🪄 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: 31ed9b31-7e87-4caf-80b2-10f469ad73a5

📥 Commits

Reviewing files that changed from the base of the PR and between ee1e71e and 33dd94d.

📒 Files selected for processing (1)
  • standards/push-protection.md

Comment thread standards/push-protection.md Outdated
Comment thread standards/push-protection.md Outdated
- Clarify that `pre-commit run gitleaks --all-files` and `gitleaks detect
  --source .` only scan the current working tree, not git history. Use
  `gitleaks git --redact --exit-code 1` for full-history scans (both in
  the local-tooling section and in the repo onboarding step).
- Pin `gitleaks/gitleaks-action` to commit SHA
  ff98106e4c7b2bc287b24eaf42907196329070c7 (v2) per the Action Pinning
  Policy in ci-standards.md, removing the pre-merge TODO.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 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 113-115: The script scripts/apply-repo-settings.sh does not apply
the security_and_analysis configuration described in the standard; update the
apply_settings() function to include and enforce the security_and_analysis block
(the same keys defined in lines 102–111 of the standard) when calling the GitHub
API (or gh repo edit/REST payload), ensure you construct the JSON payload to
include security_and_analysis alongside existing merge/label settings, make the
operation idempotent and log success/failure, and reference the
security_and_analysis keys and apply_settings() so reviewers can find and verify
the change.
- Line 224: Update the inline comment next to the pinned action reference to
reflect the exact release tag: change the comment on the gitleaks action
reference "uses:
gitleaks/gitleaks-action@ff98106e4c7b2bc287b24eaf42907196329070c7" from "# v2"
to "# v2.3.9" so the comment matches the pinned SHA.
🪄 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: 3406d587-c6c6-4a8a-91a2-b9dbdc9f1936

📥 Commits

Reviewing files that changed from the base of the PR and between 33dd94d and af906f3.

📒 Files selected for processing (1)
  • standards/push-protection.md

Comment thread standards/push-protection.md Outdated
Comment thread standards/push-protection.md Outdated
claude added 2 commits April 8, 2026 20:08
Adds a top-level `.gitignore` that every petry-projects repo MUST start
from. The file is intentionally **secrets-only** and language-agnostic
so it composes with per-repo language ignores from github/gitignore.

Coverage (12 categories):

1.  Dotenv family (with `.env.*.example` negations)
2.  Cloud provider credential files — AWS, GCP, Azure, DO, Linode,
    Hetzner, Fly, Vercel, Netlify, Cloudflare, Wrangler
3.  Kubernetes / Helm secrets — kubeconfigs, .docker/config.json,
    helm secrets values, with sops/age `.enc.yaml` re-allowed
4.  SSH / TLS / GPG key material — *.pem, *.key, *.p12, *.pfx, *.jks,
    id_rsa*, GPG keys, with `*.pub` / `*.crt` / `ca.crt` re-allowed
5.  Terraform / IaC state and tfvars — *.tfstate*, *.tfvars (allow
    *.tfvars.example), Pulumi.*.yaml, ansible vault password files
6.  Secret manager local caches — sops, age, vault, doppler, 1password,
    infisical, bitwarden, chamber, teller
7.  Database dumps and DB client dotfiles — .pgpass, .my.cnf,
    .mongorc.js, *.sql.gz dumps
8.  Package registry credential dotfiles — .npmrc, .yarnrc.yml,
    .pypirc, .cargo/credentials, nuget.config, .m2/settings.xml,
    gradle.properties, .netrc, gh tokens
9.  Cloud CLI session/token caches — aws sso cache, gcloud, .boto,
    .s3cfg, rclone, oci, ibmcloud, snowflake, databricks
10. IDE files known to cache credentials — JetBrains workspace.xml,
    dataSources.local.xml, VS Code sftp.json, Cursor mcp.json,
    Windsurf, Zed
11. Generic secret/credential/private filename conventions, with
    `*.example` and SOPS/age `*.enc.*` / `*.sops.*` re-allowed
12. Modern (2024-2026) leak hotspots — LLM/AI tooling configs
    (.anthropic, .openai, .continue, .aider), edge platform CLIs
    (Supabase, PlanetScale, Neon, Turso, Railway, Render),
    Stripe/Twilio CLIs, Temporal/Dagger/Earthly

Negations are placed immediately after the broad pattern they carve
out of, with a header comment documenting why ordering matters.

Updates `standards/push-protection.md` to:

- Replace the inline 11-line minimal example with a link to the new
  baseline at `/.gitignore`.
- Document that per-repo overrides must append (not replace), must not
  re-ignore the negated files, and must negate by file path (never by
  directory) — a negation inside an ignored directory does not
  re-include the file.
- Note that repos copying the org baseline verbatim automatically
  satisfy the `gitignore_secrets_block` audit check.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Soften the apply-repo-settings.sh wording at lines 113-115 to a
  "target state" with an explicit caveat that the script's
  apply_settings() function does not yet apply a security_and_analysis
  payload, so the settings must be applied manually (or via a follow-up
  PR against the script) until then. Mirrors the same pattern already
  used in the Compliance Audit Checks section.
- Update the inline comment for gitleaks-action from "# v2" to
  "# v2.3.9" to match the actual release tag for the pinned SHA
  ff98106e4c7b2bc287b24eaf42907196329070c7.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
standards/push-protection.md (1)

113-115: ⚠️ Potential issue | 🟠 Major

Automation claim is ahead of current implementation.

The text says scripts/apply-repo-settings.sh enforces security_and_analysis, but current apply_settings() only patches merge/label-related repository settings. This creates policy/automation drift.

Also applies to: 374-377

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@standards/push-protection.md` around lines 113 - 115, The documentation
claims scripts/apply-repo-settings.sh enforces security_and_analysis, but
apply_settings() currently only updates merge/label settings; update
apply_settings() (in scripts/apply-repo-settings.sh) to include the
security_and_analysis block and ensure the same PATCH/PUT call that writes
merge/label settings also sends the security_and_analysis payload (e.g., include
keys like "security_and_analysis" with the desired subfields) and handle API
response/errors consistently with existing settings application logic so the
script truly enforces those values.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.gitignore:
- Around line 1-381: The .gitignore baseline is missing worktree ignore entries;
add the patterns ".claude/worktrees/" and ".worktrees/" to the file (preferably
under a relevant section such as near "Modern (2024-2026) credential-leak
hotspots" or the top-level secrets list) so temporary agent worktrees are always
ignored and cannot be committed accidentally; ensure the entries are
directory-form (trailing slash) and placed before any negations that might
re-include those paths.

---

Duplicate comments:
In `@standards/push-protection.md`:
- Around line 113-115: The documentation claims scripts/apply-repo-settings.sh
enforces security_and_analysis, but apply_settings() currently only updates
merge/label settings; update apply_settings() (in
scripts/apply-repo-settings.sh) to include the security_and_analysis block and
ensure the same PATCH/PUT call that writes merge/label settings also sends the
security_and_analysis payload (e.g., include keys like "security_and_analysis"
with the desired subfields) and handle API response/errors consistently with
existing settings application logic so the script truly enforces those values.
🪄 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: 87a9ac66-1a28-4664-a88d-b29495543281

📥 Commits

Reviewing files that changed from the base of the PR and between af906f3 and 76b5588.

📒 Files selected for processing (2)
  • .gitignore
  • standards/push-protection.md

Comment thread .gitignore
Adds a new section 13 to the secrets baseline covering Claude Code /
agent worktree directories:

- .claude/worktrees/
- .worktrees/

These are not strictly secret material, but the petry-projects coding
guidelines require every repo to ignore them so an agent's scratch
worktree cannot be committed accidentally. Addresses CodeRabbit
finding on commit 7893f9b.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

♻️ Duplicate comments (2)
standards/push-protection.md (2)

378-380: ⚠️ Potential issue | 🟠 Major

Contradicts earlier caveat about apply-repo-settings.sh enforcement.

This says scripts/apply-repo-settings.sh “enforces this on every run,” but Lines 113-119 explicitly say it does not yet apply security_and_analysis. Keep one consistent “target state + current gap” message here too.

Suggested doc fix
-1. **Enable secret scanning + push protection** via the API call in
-   [Required repo-level settings](`#required-repo-level-settings`). `scripts/apply-repo-settings.sh`
-   enforces this on every run.
+1. **Enable secret scanning + push protection** via the API call in
+   [Required repo-level settings](`#required-repo-level-settings`).
+   Target state: `scripts/apply-repo-settings.sh` enforces this on every run.
+   Current state: until `apply_settings()` includes a `security_and_analysis`
+   payload, apply these settings manually (or via a follow-up PR to the script).

Based on learnings: In scripts/apply-repo-settings.sh, apply_settings() currently applies merge/label settings and does not yet enforce a security_and_analysis payload.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@standards/push-protection.md` around lines 378 - 380, The sentence claiming
"scripts/apply-repo-settings.sh enforces this on every run" conflicts with the
note that apply_settings() does not yet apply security_and_analysis; update the
wording to state the desired target (enable secret scanning + push protection
via the API) and explicitly call out the current gap
(scripts/apply-repo-settings.sh's apply_settings() does not yet enforce the
security_and_analysis payload), e.g., rephrase to indicate it's a required
repo-level setting that the script intends to enforce but currently only applies
merge/label settings and not security_and_analysis until apply_settings() is
extended.

205-207: ⚠️ Potential issue | 🟠 Major

CI example does not actually run a full-history scan.

The section requires full-history scanning, but args: detect --source . scans repository content from the working tree/path, not commit history. Use a history-aware invocation in the required example.

Suggested doc fix
-      with:
-        args: detect --source . --redact --verbose --exit-code 1
+      with:
+        args: git --redact --verbose --exit-code 1
In gitleaks v8.24.0 and gitleaks-action v2, does `detect --source .` scan full git history, or only files at the provided source path? What is the recommended command for full-history scanning in CI?

Also applies to: 230-230

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@standards/push-protection.md` around lines 205 - 207, The CI example's
secret-scan job in ci.yml currently uses "args: detect --source ." which only
scans the working-tree files; change the secret-scan job that calls gitleaks
(the entry referenced by the secret-scan job and the args: detect --source .
invocation) to invoke gitleaks in full-git-history mode instead of
filesystem-only mode: replace the filesystem scan invocation with the
history-aware gitleaks detect invocation per gitleaks v8 docs (do not pass
filesystem-only flags like --no-git) so the action walks commit history (update
both occurrences noted in the comment).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@standards/push-protection.md`:
- Around line 378-380: The sentence claiming "scripts/apply-repo-settings.sh
enforces this on every run" conflicts with the note that apply_settings() does
not yet apply security_and_analysis; update the wording to state the desired
target (enable secret scanning + push protection via the API) and explicitly
call out the current gap (scripts/apply-repo-settings.sh's apply_settings() does
not yet enforce the security_and_analysis payload), e.g., rephrase to indicate
it's a required repo-level setting that the script intends to enforce but
currently only applies merge/label settings and not security_and_analysis until
apply_settings() is extended.
- Around line 205-207: The CI example's secret-scan job in ci.yml currently uses
"args: detect --source ." which only scans the working-tree files; change the
secret-scan job that calls gitleaks (the entry referenced by the secret-scan job
and the args: detect --source . invocation) to invoke gitleaks in
full-git-history mode instead of filesystem-only mode: replace the filesystem
scan invocation with the history-aware gitleaks detect invocation per gitleaks
v8 docs (do not pass filesystem-only flags like --no-git) so the action walks
commit history (update both occurrences noted in the comment).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 4ea4e28a-946f-446a-959c-6a7abce5a424

📥 Commits

Reviewing files that changed from the base of the PR and between 76b5588 and 7893f9b.

📒 Files selected for processing (1)
  • standards/push-protection.md

coderabbitai[bot]
coderabbitai Bot previously approved these changes Apr 9, 2026
…red lib

Adds the apply and compliance-audit logic for the Push Protection
Standard, factored into a single shared library so both scripts
read from one source of truth.

New file: scripts/lib/push-protection.sh

Sourceable Bash library that exposes:

  Apply (used by apply-repo-settings.sh):
    pp_apply_security_and_analysis <repo>
      - Idempotent PATCH of the org-required security_and_analysis flags
      - Honors $DRY_RUN and the existing info/ok/err/skip log helpers
      - Sends a single JSON payload via `gh api --input -` (gh's -F form
        does not accept nested objects)

  Check (used by compliance-audit.sh):
    pp_check_security_and_analysis <repo>
      - Verifies the 5 SA flags via the same PP_REQUIRED_SA_SETTINGS list
        the apply function uses, so the audit cannot drift
    pp_check_open_secret_alerts <repo>
      - error finding for any open secret-scanning alert
    pp_check_secret_scan_ci_job <repo>
      - Fetches .github/workflows/ci.yml and verifies a gitleaks job
    pp_check_gitignore_secrets_block <repo>
      - Fetches .gitignore and verifies the baseline patterns from
        PP_REQUIRED_GITIGNORE_PATTERNS, ignoring `!`-negation lines
    pp_check_push_protection_bypasses <repo>
      - Queries secret-scanning alerts for push_protection_bypassed
        events in the last $PP_BYPASS_LOOKBACK_DAYS days (default: 30)

  Convenience:
    pp_run_all_checks <repo>
      - Single entry point the audit's per-repo loop calls

Single-source-of-truth design

The PP_REQUIRED_SA_SETTINGS array (key:expected:severity:detail) is
the canonical list of required security_and_analysis flags. Adding a
new required flag there extends BOTH the apply script and the weekly
audit on the next run. Same for PP_REQUIRED_GITIGNORE_PATTERNS.

Wiring

- scripts/apply-repo-settings.sh sources the lib at the top and calls
  pp_apply_security_and_analysis after apply_settings/apply_labels in
  both the single-repo and --all code paths.
- scripts/compliance-audit.sh sources the lib AFTER its gh_api() and
  add_finding() helpers are defined (the lib's check functions call
  them by name) and calls pp_run_all_checks at the tail of the
  per-repo audit loop in main().

CI shellcheck glob fix

The existing ShellCheck step in .github/workflows/ci.yml only globbed
scripts/*.sh, so the new scripts/lib/push-protection.sh would have
been silently skipped. Updated the step to recurse via globstar and
to pass -x so `# shellcheck source=...` directives are followed.

Standard wording

Reverted the two "target state" softenings added in earlier commits
of this PR (apply-repo-settings drift caveat and the
"until compliance-audit.sh is extended" caveat in the audit section)
to assertive MUST language. Both scripts now actually enforce.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Apr 9, 2026

@don-petry don-petry merged commit d1ac0ee into main Apr 9, 2026
19 checks passed
@don-petry don-petry deleted the claude/push-protection-standard-2GjYF branch April 9, 2026 11:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants