-
Notifications
You must be signed in to change notification settings - Fork 1
Add CodeQL query to detect unclosed Repository and Storage instances #8
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.
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
pjbgf
merged 2 commits into
go-git:main
from
AriehSchneier:add-codeql-resource-leak-detection
May 7, 2026
Merged
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| name: CodeQL | ||
|
|
||
| on: | ||
| workflow_dispatch: | ||
| inputs: | ||
| go-git-ref: | ||
| description: 'go-git branch, tag, or commit to analyze' | ||
| required: false | ||
| default: 'main' | ||
| type: string | ||
| push: | ||
| branches: [main] | ||
| schedule: | ||
| # Run weekly on Mondays at 00:00 UTC | ||
| - cron: '0 0 * * 1' | ||
|
|
||
| permissions: | ||
| contents: read | ||
| security-events: write | ||
|
|
||
| jobs: | ||
| analyze: | ||
| name: Analyze go-git with custom queries | ||
| runs-on: ubuntu-latest | ||
|
|
||
| steps: | ||
| - name: Checkout x repository | ||
| uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 | ||
| with: | ||
| fetch-depth: 0 | ||
|
|
||
| - name: Checkout go-git repository | ||
| uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 | ||
| with: | ||
| repository: go-git/go-git | ||
| ref: ${{ inputs.go-git-ref || 'main' }} | ||
| path: go-git | ||
| fetch-depth: 0 | ||
|
|
||
| - name: Copy CodeQL queries to go-git | ||
| run: cp -r codeql go-git/ | ||
|
|
||
| - name: Initialize CodeQL | ||
| uses: github/codeql-action/init@c3f298df8c1fea2fefe20c785e6aa00f32df8260 # v4.35.3 | ||
|
pjbgf marked this conversation as resolved.
Outdated
|
||
| with: | ||
| languages: go | ||
| source-root: go-git | ||
| config-file: go-git/codeql/codeql-config.yml | ||
| checkout-path: go-git | ||
|
|
||
| - name: Perform CodeQL Analysis | ||
| uses: github/codeql-action/analyze@c3f298df8c1fea2fefe20c785e6aa00f32df8260 # v4.35.3 | ||
|
pjbgf marked this conversation as resolved.
Outdated
|
||
| with: | ||
| category: go-git-custom-queries | ||
| checkout-path: go-git | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| # CodeQL Queries for go-git | ||
|
|
||
| This directory contains CodeQL queries for detecting common issues in go-git usage. | ||
|
|
||
| ## Queries | ||
|
|
||
| ### Unclosed Resources (`unclosed-resources.ql`) | ||
|
|
||
| Detects instances where `Repository` or `Storage` objects are created but never closed, which can lead to file handle leaks, particularly on Windows. | ||
|
|
||
| **What it detects:** | ||
| - Calls to `PlainOpen`, `Init`, `Clone`, and other repository creation functions | ||
| - Calls to `NewStorage` and `NewStorageWithOptions` | ||
| - Submodule and worktree operations that return repositories | ||
| - Missing `Close()` calls or `defer` cleanup patterns | ||
|
|
||
| **Example violations:** | ||
|
|
||
| ```go | ||
| // BAD: No Close() call | ||
| func bad() error { | ||
| r, err := git.PlainOpen("/path/to/repo") | ||
| if err != nil { | ||
| return err | ||
| } | ||
| // Missing: defer func() { _ = r.Close() }() | ||
| _, err = r.Head() | ||
| return err | ||
| } | ||
|
|
||
| // GOOD: Proper cleanup | ||
| func good() error { | ||
| r, err := git.PlainOpen("/path/to/repo") | ||
| if err != nil { | ||
| return err | ||
| } | ||
| defer func() { _ = r.Close() }() | ||
| _, err = r.Head() | ||
| return err | ||
| } | ||
| ``` | ||
|
|
||
| ## Running the queries | ||
|
|
||
| ### Using CodeQL CLI | ||
|
|
||
| ```bash | ||
| codeql database create /tmp/go-git-db --language=go --source-root=/path/to/go-git | ||
| codeql query run codeql/queries/unclosed-resources.ql --database=/tmp/go-git-db | ||
| ``` | ||
|
|
||
| ### Using GitHub Actions | ||
|
|
||
| The queries run automatically via the CodeQL workflow on pull requests. | ||
|
|
||
| ## Contributing | ||
|
|
||
| To add a new query: | ||
|
|
||
| 1. Create a `.ql` file in `codeql/queries/` | ||
| 2. Include proper metadata (name, description, severity, tags) | ||
| 3. Test the query against go-git codebase | ||
| 4. Document the query in this README |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| name: "go-git resource leak detection" | ||
|
|
||
| queries: | ||
| - uses: ./queries/unclosed-resources.ql |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| name: go-git/codeql-queries | ||
| version: 0.0.1 | ||
| library: false | ||
| dependencies: | ||
| codeql/go-all: "*" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,99 @@ | ||
| /** | ||
| * @name Unclosed Repository or Storage | ||
| * @description Detects Repository or Storage instances that are created but never closed, | ||
| * which can lead to file handle leaks on Windows. | ||
| * @kind problem | ||
| * @problem.severity warning | ||
| * @id go-git/unclosed-resources | ||
| * @tags reliability | ||
| * resource-management | ||
| */ | ||
|
|
||
| import go | ||
|
|
||
| /** | ||
| * A type that represents either `*Repository` or a Storage type that needs closing. | ||
| */ | ||
| class ResourceType extends Type { | ||
| ResourceType() { | ||
| // Repository type | ||
| this.(PointerType).getBaseType().hasQualifiedName("github.com/go-git/go-git/v6", "Repository") | ||
| or | ||
| // filesystem.Storage | ||
| this.(PointerType).getBaseType().hasQualifiedName("github.com/go-git/go-git/v6/storage/filesystem", "Storage") | ||
| or | ||
| // transactional.Storage | ||
| this.(PointerType).getBaseType().hasQualifiedName("github.com/go-git/go-git/v6/storage/transactional", "Storage") | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * A function call that creates a resource (Repository or Storage). | ||
| */ | ||
| class ResourceCreation extends CallExpr { | ||
| ResourceCreation() { | ||
| exists(Function f | f = this.getTarget() | | ||
| // Repository creation functions | ||
| f.hasQualifiedName("github.com/go-git/go-git/v6", ["PlainOpen", "PlainOpenWithOptions", "PlainClone", "PlainCloneContext", "PlainInit", "Init", "Clone", "CloneContext", "Open"]) | ||
| or | ||
| // Storage creation functions | ||
| f.hasQualifiedName("github.com/go-git/go-git/v6/storage/filesystem", ["NewStorage", "NewStorageWithOptions"]) | ||
| or | ||
| f.hasQualifiedName("github.com/go-git/go-git/v6/storage/transactional", "NewStorage") | ||
| or | ||
| // Submodule.Repository() method | ||
| f.hasQualifiedName("github.com/go-git/go-git/v6", "Submodule", "Repository") | ||
| or | ||
| // Worktree.Repository() method | ||
| f.hasQualifiedName("github.com/go-git/go-git/v6", "Worktree", "Repository") | ||
| or | ||
| // Repository.Worktree() method returns a Worktree with repository field | ||
| f.hasQualifiedName("github.com/go-git/go-git/v6", "Repository", "Worktree") | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * A call to Close() method on a resource. | ||
| */ | ||
| class CloseCall extends MethodCall { | ||
| CloseCall() { | ||
| this.getTarget().getName() = "Close" and | ||
| this.getReceiver().getType() instanceof ResourceType | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Checks if a variable has a Close() call (direct or in defer) in the same function. | ||
| */ | ||
| predicate hasCloseCall(SsaVariable v) { | ||
| exists(CloseCall close | | ||
| close.getReceiver() = v.getAUse() | ||
| ) | ||
| or | ||
| // Check for defer Close() patterns | ||
| exists(DeferStmt defer, CloseCall close | | ||
| defer.getCall() = close and | ||
| close.getReceiver() = v.getAUse() | ||
| ) | ||
| or | ||
| // Check for defer func() { _ = x.Close() }() patterns | ||
| exists(DeferStmt defer, FuncLit fn, AssignStmt assign, CloseCall close | | ||
| defer.getCall().(CallExpr).getCalleeExpr() = fn and | ||
| fn.getBody().getAStmt() = assign and | ||
| assign.getRhs(0) = close and | ||
| close.getReceiver() = v.getAUse() | ||
| ) | ||
| } | ||
|
|
||
| from ResourceCreation create, SsaVariable v | ||
| where | ||
| // The resource is assigned to a variable | ||
| v.getDefinition().(SsaExplicitDefinition).getInstruction().getNode() = create and | ||
| // The variable is not closed | ||
| not hasCloseCall(v) and | ||
| // The variable is not assigned to a field (which might be closed elsewhere) | ||
| not exists(Field f | v.getAUse() = f.getAWrite().getRhs()) and | ||
| // The variable is not returned (caller's responsibility) | ||
| not exists(ReturnStmt ret | v.getAUse() = ret.getExpr()) | ||
| select create, "This resource is created but never closed, which may cause file handle leaks." |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.