Skip to content

enh(secu): manage invalid-revision range error - #64

Open
sc979 wants to merge 13 commits into
mainfrom
SECU-gitleaks-custom
Open

enh(secu): manage invalid-revision range error#64
sc979 wants to merge 13 commits into
mainfrom
SECU-gitleaks-custom

Conversation

@sc979

@sc979 sc979 commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Description

What are your needs or what are you planning to do in this PR?

enh(secu): manage invalid-revision range error

Resolves the "Invalid revision range" failures on the gitleaks job by resolving the
scan range defensively (retry + re-fetch), narrows the scan so it only covers commits
that actually belong to the change under test, and makes the build fail only on a
real secret finding so the alert isn't raised on tooling errors.

Scan-range behavior

compute_range() picks the git log range gitleaks scans (--log-opts), with an
empty value meaning a full-history scan.

  • pull_requestmerge-base(base, head)..head — only the PR's own commits.
  • push, existing branchbefore..after — only the commits the push added.
  • push, new branch / empty before / rewritten historymerge-base(origin/<default>, after)..after
    — only the commits unique to this branch since it diverged from the default branch.
    The shared base history is already scanned on the default branch's own pushes, so
    re-scanning it here would just re-report pre-existing, unrelated findings.
  • fallbacks (default branch not resolvable, unrelated history, other events) →
    full scan. Over-scan is the safe direction — no path silently scans nothing.

New-branch fix: previously a new branch fell back to a full scan, which
re-flagged secrets already living in the default branch's history (not introduced
by the branch). The empty-before case now follows the same path as a real
new branch, so it is consistent and no longer reports unrelated secrets.

Build outcome & alerting

The alert is wired to the build failure, so the job fails only when it must:

gitleaks result Job outcome Alert
No secrets (exit 0) ✅ success
Secrets found (exit 2) fail (exit 1) 🔔 raised
Unexpected tool error (any other exit) ✅ success (exit 0) + ::error:: log
Range unresolvable after all retries ❌ fail (exit 1) 🔔 raised (transient — re-run the job)

A gitleaks tool/internal error no longer fails the build, so it can't raise a
false alert; it is still surfaced as an ::error:: annotation for visibility. Only a
genuine secret finding (or an unrecoverable range after every retry) fails the job.
The range-resolved log is displayed in the logs

compute_range() test coverage

Verified verbatim against a throwaway repo mimicking the runner layout (default
branch present as refs/remotes/origin/<default>, detached HEAD). Each case asserts
the resulting --log-opts (empty = full scan) and the return code.

A -- B -- C          refs/remotes/origin/main  (default)
      \
       D -- E         pushed branch tip
 \
  F -- G              branch forked off an OLD main commit (A)

X -- Y                unrelated history (no common ancestor)

push — existing branch

Scenario Mocked env --log-opts rc
Existing branch (old tip present) PUSH_BEFORE=A, PUSH_AFTER=C --no-merges A..C 0

push — new branch / empty / rewritten → unique commits only

Scenario Mocked env --log-opts rc
New branch off main PUSH_BEFORE=0000…, PUSH_AFTER=E --no-merges B..E 0
Empty before (now consistent) PUSH_BEFORE="", PUSH_AFTER=E --no-merges B..E 0
Force-push / rewritten (old tip absent) PUSH_BEFORE=<missing>, PUSH_AFTER=E --no-merges B..E 0
Branch forked off an old main commit PUSH_BEFORE=0000…, PUSH_AFTER=G --no-merges A..G 0

push — fail-safe fallbacks (must be full scan, never empty)

Scenario Mocked env --log-opts rc
Default branch ref missing DEFAULT_BRANCH=does-not-exist full scan 0
Unrelated history (no merge-base) PUSH_AFTER=Y full scan 0
Branch == default tip (nothing new) PUSH_AFTER=C --no-merges C..C (empty range, correct) 0

pull_request

Scenario Mocked env --log-opts rc
Normal PR HEAD_SHA=E, PR_BASE_SHA=C --no-merges --first-parent B..E 0
Head missing (force-push race) HEAD_SHA=<missing> empty 1 → triggers refetch loop

other events → full scan

Scenario --log-opts rc
workflow_dispatch full scan 0
schedule (cron) full scan 0

Result: 12 / 12 passed.

Scope: covers the pure range-selection logic (git-only, no gitleaks binary needed).
Does not exercise the refetch retry loop or the gitleaks invocation itself.

@sc979
sc979 requested a review from a team as a code owner July 28, 2026 09:26
@sc979
sc979 requested review from Tpo76 and kduret July 28, 2026 09:26
Comment thread .github/workflows/gitleaks-analysis.yml Outdated
Comment thread .github/workflows/gitleaks-analysis.yml Outdated
Comment thread .github/workflows/gitleaks-analysis.yml Outdated
Comment thread .github/workflows/gitleaks-analysis.yml Outdated
Co-authored-by: Stéphane Chapron <34628915+sc979@users.noreply.github.com>
Signed-off-by: Stéphane Chapron <34628915+sc979@users.noreply.github.com>
Comment thread .github/workflows/gitleaks-analysis.yml
Signed-off-by: Stéphane Chapron <34628915+sc979@users.noreply.github.com>
Comment thread .github/workflows/gitleaks-analysis.yml Outdated
@sc979
sc979 marked this pull request as draft July 28, 2026 12:18
Comment thread .github/workflows/gitleaks-analysis.yml Outdated
Comment thread .github/workflows/gitleaks-analysis.yml Outdated
Comment thread .github/workflows/gitleaks-analysis.yml Outdated
Comment thread .github/workflows/gitleaks-analysis.yml Outdated
Comment thread .github/workflows/gitleaks-analysis.yml Outdated
Co-authored-by: Stéphane Chapron <34628915+sc979@users.noreply.github.com>
Signed-off-by: Stéphane Chapron <34628915+sc979@users.noreply.github.com>
@sc979
sc979 marked this pull request as ready for review July 29, 2026 09:25
Comment thread .github/workflows/gitleaks-analysis.yml Outdated
sc979 added 2 commits July 29, 2026 11:27
Signed-off-by: Stéphane Chapron <34628915+sc979@users.noreply.github.com>
Signed-off-by: Stéphane Chapron <34628915+sc979@users.noreply.github.com>
Comment thread .github/workflows/gitleaks-analysis.yml Outdated
Signed-off-by: Stéphane Chapron <34628915+sc979@users.noreply.github.com>
Comment thread .github/workflows/gitleaks-analysis.yml Outdated
sc979 added 2 commits July 29, 2026 12:03
Signed-off-by: Stéphane Chapron <34628915+sc979@users.noreply.github.com>
already managed by the triggering pipeline. causing failing loop

Signed-off-by: Stéphane Chapron <34628915+sc979@users.noreply.github.com>
sc979 and others added 2 commits July 29, 2026 12:11
Signed-off-by: Stéphane Chapron <34628915+sc979@users.noreply.github.com>
On a push that creates a new branch (or rewrites history), the
gitleaks range fell back to a full-history scan. Because a branch
usually forks off the default branch, that re-scanned commits already
living in the default branch and re-reported pre-existing secrets that
the branch never introduced.

Resolve the range against the default branch instead: scan only
merge-base(origin/<default>, after)..after, i.e. the commits unique to
the branch. The empty-"before" case now follows the same path, so it is
consistent with a real new branch. When the default branch cannot be
resolved, fall back to a full scan (over-scan is the safe direction).

Also correct the range-resolved log to ::notice:: (::info:: is not a
valid workflow command).

Assisted-by: Claude Code (claude-opus-4-8)
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.

1 participant