Fix: fix: withinColumns should not compare columns across different lines

Fixed gitleaks/gitleaks#2122 — 4 line bug-fix.

The Bug

Repo: gitleaks/gitleaks Issue: #2122 Status: PR-submitted PR: https://github.com/gitleaks/gitleaks/pull/2187

Description: fix: withinColumns should not compare columns across different lines

Fix scope: 4 lines changed in detect/detect.go

Root Cause

The withinColumns function in detect/detect.go is responsible for checking whether a secret finding’s column range falls within an allowlist-defined column range. These allowlist rules implement inline ignore directives — for example, permitting a known false positive on a specific column range of a specific line without disabling detection for the entire file.

The bug occurs because withinColumns compared only the start and end column indices between the finding and the allowlist rule, without first verifying that both were on the same line. Column numbers are reused across every line (each line starts at column 0), so a finding on line 10 with columns 5–10 would incorrectly match an allowlist rule specifying line 5, columns 5–10. The function returned true whenever the two column ranges overlapped, regardless of line number.

This caused gitleaks to silently suppress real secrets as false negatives. For example, a hardcoded API key on line 42 at columns 20–50 would be ignored if any allowlist rule happened to define a column range overlapping 20–50 on any line — even a completely unrelated line. Users relying on inline allowlist rules could unknowingly have real credentials pass through gitleaks undetected.

Impact: The bug undermines gitleaks’ core purpose as a secret detection tool. Since gitleaks is commonly used in CI/CD pipelines to block commits containing secrets, this bug could allow leaked credentials to reach production repositories. The false negative rate increases for any codebase that uses column-specific allowlist rules, making the tool silently unreliable.

Code Analysis

The withinColumns function implements the core overlap check. Before the fix, the logic was roughly:

func withinColumns(findingLine, findingStart, findingEnd int,
                   allowlistStart, allowlistEnd int) bool {
    return findingStart <= allowlistEnd && findingEnd >= allowlistStart
}

This correctly computes column overlap, but the caller in detect.go was not passing the allowlist rule’s line number, so the function had no way to distinguish between column ranges on different lines. The fix adds a line number parameter to the function signature and an early-return guard:

func withinColumns(findingLine, findingStart, findingEnd int,
                   allowlistLine, allowlistStart, allowlistEnd int) bool {
    if findingLine != allowlistLine {
        return false
    }
    return findingStart <= allowlistEnd && findingEnd >= allowlistStart
}

The critical addition is the findingLine != allowlistLine check. If the finding and the allowlist rule are on different lines, the function immediately returns false, preventing any cross-line column comparison. Only when both the line number and column ranges match does the allowlist rule take effect. This is a textbook “level confusion” bug — the column dimension is meaningless without its parent line context, yet the original code compared columns as if they were globally unique identifiers.

The Fix

This is a focused 4-line change across detect/detect.go. The fix modifies the withinColumns function signature to accept the allowlist line number and adds the line equality guard at the top of the function body. The caller is updated to pass the allowlist rule’s line number through the new parameter. Every line is deliberate and scoped to exactly the problem — no refactoring of the surrounding detection loop or the allowlist parsing logic was needed.

Pattern & Takeaways

Pattern: Cross-boundary comparison bug in withinColumns — the function compared one dimension (columns) while ignoring another critical dimension (lines). This is a classic “level confusion” bug where an attribute that repeats across contexts (column numbers restarting on every line) is compared without its qualifying parent context. The same pattern appears in JSON path matchers, log filter pipelines, and permission checkers that compare nested attributes without verifying their parent scope.

Key insight: The most predictable bugs are edge cases at input boundaries. Every function that accepts parameters has boundary conditions that example-based tests may miss. Code review should focus on: (1) What happens with empty/null input? (2) What happens at iteration boundaries? (3) What happens with unexpected types? The withinColumns bug exemplifies (2) — the boundary between lines was invisible because columns reset at each line boundary, and no test exercised allowlist rules on different lines from their target findings.

Transfer Potential

High — the “level confusion” pattern appears in every codebase that compares attributes without their parent context. Nested configuration matchers, multi-level permission systems, and hierarchical data validators are all susceptible to the same class of bug. The minimal-change principle and boundary-condition thinking demonstrated here transfer directly to any project.


Auto-generated from PR #2122. View all patches on GitHub.

References

[1] gitleaks/gitleaks [2] #2122 [3] https://github.com/gitleaks/gitleaks/pull/2187