Skip to content

OSAC-2465: Add gitleaks pre-commit hook, remove per-PR workflow - #140

Merged
openshift-merge-bot[bot] merged 4 commits into
osac-project:mainfrom
minmzzhang:feat/OSAC-2465-secret-scanning-precommit
Aug 6, 2026
Merged

OSAC-2465: Add gitleaks pre-commit hook, remove per-PR workflow#140
openshift-merge-bot[bot] merged 4 commits into
osac-project:mainfrom
minmzzhang:feat/OSAC-2465-secret-scanning-precommit

Conversation

@minmzzhang

@minmzzhang minmzzhang commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add a gitleaks pre-commit hook to catch secrets at commit time before they reach the repository
  • Remove the standalone per-PR secret-scanning.yaml CI workflow; fold its gitleaks scan into pre-commit.yaml to reduce attack surface on fork PRs
  • Harden pre-commit.yaml: explicit least-privilege permissions, SHA-pinned actions, persist-credentials: false, if: always() so lint failures cannot mask a secret leak
  • CI loads .gitleaks.toml and .gitleaksignore from the base branch (not the PR checkout) and passes --ignore-gitleaks-allow so PR authors cannot weaken the scan
  • Migrate .gitleaks.toml from deprecated [allowlist] to [[allowlists]] (gitleaks ≥ v8.25)

Port of osac-installer#451 into the monorepo.

How to suppress a false positive

  1. Locally: add # gitleaks:allow as an inline comment on the flagged line (pre-commit only).
  2. In CI: inline allows are ignored (--ignore-gitleaks-allow). Add a narrowly-scoped [[allowlists]] entry to .gitleaks.toml and merge it to the base branch first.

Never allowlist a real secret — rotate it at its source immediately instead.

Test plan

  • Run pre-commit install && pre-commit run gitleaks on staged files locally
  • Confirm CI pre-commit job passes (hardened workflow + gitleaks diff scan)
  • Verify secret-scanning.yaml no longer runs on PRs

Summary by CodeRabbit

  • New Features

    • Added secret scanning for staged files and pull-request changes.
    • Failed scans now provide guidance to rotate exposed credentials and manage false positives.
    • Added documented templates for narrowly scoped false-positive exceptions.
  • Documentation

    • Clarified that local scanning covers staged files, while CI scans pull-request changes.
    • Updated workflow documentation to describe pull-request secret scanning.
  • Bug Fixes

    • Improved scan reliability with pinned tool versions, read-only access, and base-to-head diff scanning.

@openshift-ci-robot

openshift-ci-robot commented Aug 4, 2026

Copy link
Copy Markdown

@minmzzhang: This pull request references OSAC-2465 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "5.0.0" version, but no target version was set.

Details

In response to this:

Summary

  • Add a gitleaks pre-commit hook to catch secrets at commit time before they reach the repository
  • Remove the standalone per-PR secret-scanning.yaml CI workflow; fold its gitleaks scan into pre-commit.yaml to reduce attack surface on fork PRs
  • Harden pre-commit.yaml: explicit least-privilege permissions, SHA-pinned actions, persist-credentials: false, if: always() so lint failures cannot mask a secret leak
  • CI loads .gitleaks.toml and .gitleaksignore from the base branch (not the PR checkout) and passes --ignore-gitleaks-allow so PR authors cannot weaken the scan
  • Migrate .gitleaks.toml from deprecated [allowlist] to [[allowlists]] (gitleaks ≥ v8.25)

Port of osac-installer#451 into the monorepo.

How to suppress a false positive

  1. Locally: add # gitleaks:allow as an inline comment on the flagged line (pre-commit only).
  2. In CI: inline allows are ignored (--ignore-gitleaks-allow). Add a narrowly-scoped [[allowlists]] entry to .gitleaks.toml and merge it to the base branch first.

Never allowlist a real secret — rotate it at its source immediately instead.

Test plan

  • Run pre-commit install && pre-commit run gitleaks on staged files locally
  • Confirm CI pre-commit job passes (hardened workflow + gitleaks diff scan)
  • Verify secret-scanning.yaml no longer runs on PRs

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown

Review Change Stack

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

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: osac-project/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 7934a25d-b11e-491e-be7c-d9ef8ed934a4

📥 Commits

Reviewing files that changed from the base of the PR and between cab5b6c and ed5cc4a.

📒 Files selected for processing (7)
  • .github/workflows/pre-commit.yaml
  • .github/workflows/secret-scanning.yaml
  • .gitleaks.toml
  • .pre-commit-config.yaml
  • bare-metal-fulfillment-operator/AGENTS.md
  • osac-aap/AGENTS.md
  • osac-installer/AGENTS.md
💤 Files with no reviewable changes (1)
  • .github/workflows/secret-scanning.yaml
🚧 Files skipped from review as they are similar to previous changes (6)
  • osac-installer/AGENTS.md
  • bare-metal-fulfillment-operator/AGENTS.md
  • osac-aap/AGENTS.md
  • .gitleaks.toml
  • .pre-commit-config.yaml
  • .github/workflows/pre-commit.yaml

Walkthrough

The pull request adds local Gitleaks checks and moves pull-request diff scanning into the pre-commit workflow. CI uses base-branch rules and posts rotation guidance for findings. Documentation and allowlist guidance are updated.

Changes

Gitleaks secret scanning

Layer / File(s) Summary
Local Gitleaks hook and configuration
.gitleaks.toml, .pre-commit-config.yaml, osac-installer/AGENTS.md
Adds the staged-file Gitleaks hook at v8.30.1. Updates allowlist guidance and documents local execution behavior.
Workflow permissions and action setup
.github/workflows/pre-commit.yaml, bare-metal-fulfillment-operator/AGENTS.md, osac-aap/AGENTS.md
Pins workflow actions, sets repository read and Issues write permissions, disables persisted credentials, and skips the staged-only Gitleaks hook in CI.
Pull-request scan and rotation comment
.github/workflows/pre-commit.yaml, .github/workflows/secret-scanning.yaml
Loads Gitleaks rules from the PR base revision, scans the base-to-head diff, and creates or updates a rotation reminder comment. Removes the standalone secret-scanning workflow.

Estimated code review effort: 4 (Complex) | ~45 minutes

Suggested reviewers: eliorerz, larsks, omer-vishlitzky

Sequence Diagram(s)

sequenceDiagram
  participant PullRequest
  participant GitHubActions
  participant BaseRevision
  participant GitleaksContainer
  participant GitHubPRComment
  PullRequest->>GitHubActions: Trigger pre-commit workflow
  GitHubActions->>BaseRevision: Load Gitleaks configuration and ignore rules
  GitHubActions->>GitleaksContainer: Scan the base-to-head diff
  GitleaksContainer-->>GitHubActions: Return scan result
  GitHubActions->>GitHubPRComment: Create or update rotation reminder
Loading

Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

  • Ignore

❌ Failed checks (1 error)

Check name Status Explanation Resolution
Container-Privileges ❌ Error The new gitleaks Docker invocation omits a user setting; the pinned image Dockerfile declares no USER, so the container runs as root without justification. Run gitleaks as a non-root UID/GID, such as --user "$(id -u):$(id -g)", or document and justify why root is required.
✅ Passed checks (10 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main changes: adding a gitleaks pre-commit hook and removing the per-PR secret-scanning workflow.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
No-Hardcoded-Secrets ✅ Passed The PR adds only workflow/configuration text, public action/image SHA pins, and gitleaks guidance; no credential assignments, private-key material, embedded-credential URLs, or secret-shaped litera...
No-Weak-Crypto ✅ Passed The feature commit adds no MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB, custom crypto, or secret-comparison code; hashes are only pinned action/image references.
No-Injection-Vectors ✅ Passed The exact PR diff adds no listed injection sink; BASE_SHA/HEAD_SHA are quoted in git/docker arguments, and the comment job uses static GitHub API objects.
No-Sensitive-Data-In-Logs ✅ Passed The hook and CI scan use --redact and omit --verbose; the failure comment contains no finding values or credentials, so the added logging paths do not expose sensitive data.
Ai-Attribution ✅ Passed The feature commit identifies Cursor and includes an Assisted-by: Cursor <cursoragent@cursor.com> trailer; no feature commit contains an AI Co-Authored-By trailer.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/pre-commit.yaml:
- Around line 9-18: Move issues: write out of the scan job’s permissions,
leaving that job with only contents: read. Isolate the comment step in a
separate job that receives issues: write and does not check out or execute pull
request-controlled content; preserve the existing scan and commenting behavior.
🪄 Autofix

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: Repository: osac-project/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: cf46f07d-819f-4c21-ab42-fa7037b61a85

📥 Commits

Reviewing files that changed from the base of the PR and between 64376c3 and 5bee6ee.

📒 Files selected for processing (7)
  • .github/workflows/pre-commit.yaml
  • .github/workflows/secret-scanning.yaml
  • .gitleaks.toml
  • .pre-commit-config.yaml
  • bare-metal-fulfillment-operator/AGENTS.md
  • osac-aap/AGENTS.md
  • osac-installer/AGENTS.md
💤 Files with no reviewable changes (1)
  • .github/workflows/secret-scanning.yaml

Comment thread .github/workflows/pre-commit.yaml Outdated
Catch secrets at commit time and fold the CI scan into pre-commit.yaml
so fork PRs no longer trigger a separate secret-scanning workflow.
CI loads .gitleaks.toml/.gitleaksignore from the base branch and runs
even when lint fails so a broken check cannot mask a leak.

Harden further: drop gitleaks --verbose (secrets can leak before
redaction), and isolate issues: write in a comment-only job that does
not check out PR content.

Signed-off-by: Min Zhang <minzhang@redhat.com>
Assisted-by: Cursor <cursoragent@cursor.com>
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@omer-vishlitzky

Copy link
Copy Markdown
Contributor

/lgtm
/approve

@openshift-ci

openshift-ci Bot commented Aug 6, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: minmzzhang, omer-vishlitzky

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci Bot added the approved label Aug 6, 2026
@openshift-merge-bot
openshift-merge-bot Bot merged commit 40b9eea into osac-project:main Aug 6, 2026
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants