Skip to content

feat: add large file detection to pre-commit hook #1011

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rjmurillo merged 1 commit into from
Mar 7, 2026
Merged

feat: add large file detection to pre-commit hook #1011

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .githooks/hooks/Invoke-PreCommit.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ if ($LASTEXITCODE -ne 0 -or -not $repoRoot) {
. "$PSScriptRoot/../lib/LintHelpers.ps1"

try {
# Large file detection (file size and line count)
Test-LargeFiles

# C# formatting (auto-fix + re-stage)
$csFiles = Get-StagedFiles -Extensions @('.cs')
if ($csFiles.Count -gt 0) {
Expand Down
59 changes: 59 additions & 0 deletions .githooks/lib/LintHelpers.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,3 +97,62 @@
Write-Host "FAIL: $Check" -ForegroundColor Red
$script:HookExitCode = 1
}

function Test-LargeFiles {

Check notice on line 101 in .githooks/lib/LintHelpers.ps1

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

.githooks/lib/LintHelpers.ps1#L101 

The cmdlet 'Test-LargeFiles' uses a plural noun. A singular noun should be used instead.
<#
.SYNOPSIS
Checks staged files against size and line-count thresholds.
.DESCRIPTION
Prevents accidentally committing oversized files or source files
that have grown beyond a maintainable line count. Binary and
generated files are excluded from the line-count check.
Comment on lines +106 to +108
Copy link

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

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

Doc comment says "Binary and generated files are excluded from the line-count check", but the implementation only gates line counting by extension ($SourceExtensions). Either adjust the description to match the actual behavior, or add explicit exclusion logic for generated/binary cases (e.g., known generated suffixes/paths).

Copilot uses AI. Check for mistakes.
.PARAMETER MaxFileSizeKB
Maximum file size in kilobytes. Default 500 KB.
.PARAMETER MaxLineCount
Maximum number of lines for source files. Default 1000 lines.
.PARAMETER SourceExtensions
File extensions treated as source code for line-count checks.
#>
[CmdletBinding()]
param(
[int]$MaxFileSizeKB = 500,
[int]$MaxLineCount = 1000,
[string[]]$SourceExtensions = @('.cs', '.ps1', '.sh', '.yaml', '.yml', '.json', '.xml', '.md')
)

$repoRoot = git rev-parse --show-toplevel
$stagedFiles = git diff --cached --name-only --diff-filter=d
if ($LASTEXITCODE -ne 0 -or -not $stagedFiles) {
return
}
Copy link
Copy Markdown

@cursor cursor Bot Mar 7, 2026

Choose a reason for hiding this comment

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

Unchecked git rev-parse exit code before overwrite

Low Severity

$LASTEXITCODE from git rev-parse on line 123 is silently overwritten by git diff on line 124. The check on line 125 only validates the git diff result, so a git rev-parse failure goes undetected. If $repoRoot ends up $null, subsequent Join-Path calls will error or resolve against the wrong directory. The caller script correctly checks $LASTEXITCODE immediately after git rev-parse (line 4–5 of Invoke-PreCommit.ps1), but Test-LargeFiles doesn't follow this pattern for its own local $repoRoot.

Fix in Cursor Fix in Web

Copy link
Copy Markdown

@cursor cursor Bot Mar 7, 2026

Choose a reason for hiding this comment

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

Git failure silently skips large file check

Medium Severity

When git diff --cached fails ($LASTEXITCODE -ne 0), Test-LargeFiles silently returns without warning. This is inconsistent with Get-StagedFiles, which throws an exception on the same failure, allowing the try-catch in Invoke-PreCommit.ps1 to report the error. Combining the error condition with the "no staged files" condition in one check means a git failure is indistinguishable from an empty staging area, so the large file safety check can be silently bypassed.

Fix in Cursor Fix in Web


Comment on lines +125 to +128
Copy link

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

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

If git diff --cached ... fails (non-zero $LASTEXITCODE), the function silently returns and the hook will allow the commit without running this check. This is inconsistent with Get-StagedFiles, which throws on git failures. Consider failing the hook (throw or Set-HookFailed + warning) when git commands fail, and only return early when there are genuinely no staged files.

Suggested change
if ($LASTEXITCODE -ne 0 -or -not $stagedFiles) {
return
}
if ($LASTEXITCODE -ne 0) {
Write-Warning "git diff --cached failed with exit code $LASTEXITCODE. Large-file check cannot be completed."
Set-HookFailed -Check "large-file-detection"
return
}
if (-not $stagedFiles) {
return
}

Copilot uses AI. Check for mistakes.
$maxBytes = $MaxFileSizeKB * 1024
$violations = @()

foreach ($file in $stagedFiles) {
$fullPath = Join-Path $repoRoot $file
if (-not (Test-Path $fullPath)) {
continue
}
Comment on lines +123 to +136
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Mar 7, 2026

Choose a reason for hiding this comment

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

medium

This function can be simplified by reusing existing functionality and context from the calling script. Specifically:

  1. Reuse Get-StagedFiles: Instead of duplicating git diff logic, use the existing Get-StagedFiles helper function to promote code reuse (DRY principle).
  2. Simplify Path Handling: The Invoke-PreCommit.ps1 script already sets the working directory to the repository root. This makes the git rev-parse call and Join-Path logic redundant. Relative paths can be used directly.

This refactoring improves maintainability and consistency with other hooks.

    $stagedFiles = Get-StagedFiles -DiffFilter 'd'
    if (-not $stagedFiles) {
        return
    }

    $maxBytes = $MaxFileSizeKB * 1024
    $violations = @()

    foreach ($file in $stagedFiles) {
        $fullPath = $file
        if (-not (Test-Path $fullPath)) {
            continue
        }


$fileInfo = Get-Item $fullPath
if ($fileInfo.Length -gt $maxBytes) {
$sizeKB = [math]::Round($fileInfo.Length / 1024, 1)
$violations += " $file ($($sizeKB) KB exceeds $($MaxFileSizeKB) KB limit)"
}

$ext = [System.IO.Path]::GetExtension($file)
if ($ext -and $SourceExtensions -contains $ext) {
$lineCount = (Get-Content $fullPath | Measure-Object -Line).Lines
Comment on lines +133 to +146
Copy link

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

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

This check reads file size/contents from the working tree (via Get-Item/Get-Content on $fullPath), not from the staged index. With partial staging it can miss oversized staged content (e.g., stage a 2MB file, then edit it down locally without re-staging; the hook will see the smaller working-tree file but the commit will still include the large staged blob). Consider computing size/line count from the index instead (e.g., git cat-file -s ":$file" for size and git show ":$file" for line counting).

Suggested change
$fullPath = Join-Path $repoRoot $file
if (-not (Test-Path $fullPath)) {
continue
}
$fileInfo = Get-Item $fullPath
if ($fileInfo.Length -gt $maxBytes) {
$sizeKB = [math]::Round($fileInfo.Length / 1024, 1)
$violations += " $file ($($sizeKB) KB exceeds $($MaxFileSizeKB) KB limit)"
}
$ext = [System.IO.Path]::GetExtension($file)
if ($ext -and $SourceExtensions -contains $ext) {
$lineCount = (Get-Content $fullPath | Measure-Object -Line).Lines
# Get size of the staged blob from the index, not the working tree.
$sizeOutput = git cat-file -s ":$file" 2>$null
if ($LASTEXITCODE -ne 0 -or -not $sizeOutput) {
continue
}
$fileBytes = [int64]$sizeOutput
if ($fileBytes -gt $maxBytes) {
$sizeKB = [math]::Round($fileBytes / 1024, 1)
$violations += " $file ($($sizeKB) KB exceeds $($MaxFileSizeKB) KB limit)"
}
$ext = [System.IO.Path]::GetExtension($file)
if ($ext -and $SourceExtensions -contains $ext) {
# Count lines from the staged content in the index.
$stagedContent = git show ":$file" 2>$null
if ($LASTEXITCODE -ne 0) {
continue
}
$lineCount = ($stagedContent | Measure-Object -Line).Lines

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Mar 7, 2026

Choose a reason for hiding this comment

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

medium

Get-Content loads the entire file into memory before counting lines, which can be inefficient for large text files. Using .NET's [System.IO.File]::ReadLines() provides a more memory-efficient, streaming approach to reading lines.

            $lineCount = ([System.IO.File]::ReadLines($fullPath) | Measure-Object).Count

if ($lineCount -gt $MaxLineCount) {
$violations += " $file ($lineCount lines exceeds $MaxLineCount line limit)"
Comment on lines +144 to +148
Copy link

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

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

The line-count check reads and allocates the entire file content even if the size check already flagged the file, and Get-Content | Measure-Object -Line can be slow for large files. To keep the hook fast, consider skipping line counting once a file already exceeds the size threshold, and using a more streaming-friendly approach to count lines (avoid materializing all lines into memory).

Copilot uses AI. Check for mistakes.
}
}
}

if ($violations.Count -gt 0) {
Write-Host "Large file(s) detected:" -ForegroundColor Yellow

Check warning on line 154 in .githooks/lib/LintHelpers.ps1

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

.githooks/lib/LintHelpers.ps1#L154 

File 'LintHelpers.ps1' uses Write-Host. Avoid using Write-Host because it might not work in all hosts, does not work when there is no host, and (prior to PS 5.0) cannot be suppressed, captured, or redirected. Instead, use Write-Output, Write-Verbose, or Write-Information.
$violations | ForEach-Object { Write-Host $_ -ForegroundColor Yellow }

Check warning on line 155 in .githooks/lib/LintHelpers.ps1

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

.githooks/lib/LintHelpers.ps1#L155 

File 'LintHelpers.ps1' uses Write-Host. Avoid using Write-Host because it might not work in all hosts, does not work when there is no host, and (prior to PS 5.0) cannot be suppressed, captured, or redirected. Instead, use Write-Output, Write-Verbose, or Write-Information.
Set-HookFailed -Check "large-file-detection"
}
}
1 change: 1 addition & 0 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ This project adheres to the [Contributor Covenant Code of Conduct](CODE-OF-CONDU

| Hook | Check | Mode | Tool |
| ------ | ------- | ------ | ------ |
| pre-commit | Large file detection | Fail on error | Built-in (500 KB / 1000 lines) |
| pre-commit | C# formatting | Auto-fix + re-stage | `dotnet format` |
| pre-commit | Markdown lint | Auto-fix + re-stage | `markdownlint-cli2` |
| pre-commit | YAML lint | Lint only | `yamllint` |
Expand Down
Loading