Skip to content

Commit 1efa082

Browse files
AriehSchneierAriehSchneier
committed
Add CodeQL query to detect unclosed Repository and Storage instances
Implements static analysis to detect resource leaks in go-git usage where Repository or Storage instances are created but never closed. This provides compile-time detection of file handle leaks as an alternative to runtime leak detection. The query detects: - Repository creation via PlainOpen, Init, Clone, and related functions - Storage creation via NewStorage and NewStorageWithOptions - Submodule and worktree operations that return repositories - Missing Close() calls or defer cleanup patterns Includes a GitHub Actions workflow that: - Runs on pushes, PRs, and weekly schedule - Can be manually triggered with custom go-git branch/ref - Uses latest actions/checkout@v6.0.2 and codeql-action@v4.35.3 Assisted-by: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Arieh Schneier <15041913+AriehSchneier@users.noreply.github.com>
1 parent e7fc238 commit 1efa082

5 files changed

Lines changed: 228 additions & 0 deletions

File tree

‎.github/workflows/codeql.yml‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
name: CodeQL
2+
3+
on:
4+
workflow_dispatch:
5+
inputs:
6+
go-git-ref:
7+
description: 'go-git branch, tag, or commit to analyze'
8+
required: false
9+
default: 'main'
10+
type: string
11+
push:
12+
branches: [main]
13+
pull_request:
14+
branches: [main]
15+
schedule:
16+
# Run weekly on Mondays at 00:00 UTC
17+
- cron: '0 0 * * 1'
18+
19+
permissions:
20+
contents: read
21+
security-events: write
22+
23+
jobs:
24+
analyze:
25+
name: Analyze go-git with custom queries
26+
runs-on: ubuntu-latest
27+
28+
steps:
29+
- name: Checkout x repository
30+
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
31+
with:
32+
fetch-depth: 0
33+
34+
- name: Checkout go-git repository
35+
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
36+
with:
37+
repository: go-git/go-git
38+
ref: ${{ inputs.go-git-ref || 'main' }}
39+
path: go-git
40+
fetch-depth: 0
41+
42+
- name: Copy CodeQL queries to go-git
43+
run: cp -r codeql go-git/
44+
45+
- name: Initialize CodeQL
46+
uses: github/codeql-action/init@c3f298df8c1fea2fefe20c785e6aa00f32df8260 # v4.35.3
47+
with:
48+
languages: go
49+
source-root: go-git
50+
config-file: go-git/codeql/codeql-config.yml
51+
checkout-path: go-git
52+
53+
- name: Perform CodeQL Analysis
54+
uses: github/codeql-action/analyze@c3f298df8c1fea2fefe20c785e6aa00f32df8260 # v4.35.3
55+
with:
56+
category: go-git-custom-queries
57+
checkout-path: go-git

‎codeql/README.md‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
# CodeQL Queries for go-git
2+
3+
This directory contains CodeQL queries for detecting common issues in go-git usage.
4+
5+
## Queries
6+
7+
### Unclosed Resources (`unclosed-resources.ql`)
8+
9+
Detects instances where `Repository` or `Storage` objects are created but never closed, which can lead to file handle leaks, particularly on Windows.
10+
11+
**What it detects:**
12+
- Calls to `PlainOpen`, `Init`, `Clone`, and other repository creation functions
13+
- Calls to `NewStorage` and `NewStorageWithOptions`
14+
- Submodule and worktree operations that return repositories
15+
- Missing `Close()` calls or `defer` cleanup patterns
16+
17+
**Example violations:**
18+
19+
```go
20+
// BAD: No Close() call
21+
func bad() error {
22+
r, err := git.PlainOpen("/path/to/repo")
23+
if err != nil {
24+
return err
25+
}
26+
// Missing: defer func() { _ = r.Close() }()
27+
_, err = r.Head()
28+
return err
29+
}
30+
31+
// GOOD: Proper cleanup
32+
func good() error {
33+
r, err := git.PlainOpen("/path/to/repo")
34+
if err != nil {
35+
return err
36+
}
37+
defer func() { _ = r.Close() }()
38+
_, err = r.Head()
39+
return err
40+
}
41+
```
42+
43+
## Running the queries
44+
45+
### Using CodeQL CLI
46+
47+
```bash
48+
codeql database create /tmp/go-git-db --language=go --source-root=/path/to/go-git
49+
codeql query run codeql/queries/unclosed-resources.ql --database=/tmp/go-git-db
50+
```
51+
52+
### Using GitHub Actions
53+
54+
The queries run automatically via the CodeQL workflow on pull requests.
55+
56+
## Contributing
57+
58+
To add a new query:
59+
60+
1. Create a `.ql` file in `codeql/queries/`
61+
2. Include proper metadata (name, description, severity, tags)
62+
3. Test the query against go-git codebase
63+
4. Document the query in this README

‎codeql/codeql-config.yml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
name: "go-git resource leak detection"
2+
3+
queries:
4+
- uses: ./queries/unclosed-resources.ql

‎codeql/qlpack.yml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
name: go-git/codeql-queries
2+
version: 0.0.1
3+
library: false
4+
dependencies:
5+
codeql/go-all: "*"

‎codeql/queries/unclosed-resources.ql‎

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
/**
2+
* @name Unclosed Repository or Storage
3+
* @description Detects Repository or Storage instances that are created but never closed,
4+
* which can lead to file handle leaks on Windows.
5+
* @kind problem
6+
* @problem.severity warning
7+
* @id go-git/unclosed-resources
8+
* @tags reliability
9+
* resource-management
10+
*/
11+
12+
import go
13+
14+
/**
15+
* A type that represents either `*Repository` or a Storage type that needs closing.
16+
*/
17+
class ResourceType extends Type {
18+
ResourceType() {
19+
// Repository type
20+
this.(PointerType).getBaseType().hasQualifiedName("github.com/go-git/go-git/v6", "Repository")
21+
or
22+
// filesystem.Storage
23+
this.(PointerType).getBaseType().hasQualifiedName("github.com/go-git/go-git/v6/storage/filesystem", "Storage")
24+
or
25+
// transactional.Storage
26+
this.(PointerType).getBaseType().hasQualifiedName("github.com/go-git/go-git/v6/storage/transactional", "Storage")
27+
}
28+
}
29+
30+
/**
31+
* A function call that creates a resource (Repository or Storage).
32+
*/
33+
class ResourceCreation extends CallExpr {
34+
ResourceCreation() {
35+
exists(Function f | f = this.getTarget() |
36+
// Repository creation functions
37+
f.hasQualifiedName("github.com/go-git/go-git/v6", ["PlainOpen", "PlainOpenWithOptions", "PlainClone", "PlainCloneContext", "PlainInit", "Init", "Clone", "CloneContext", "Open"])
38+
or
39+
// Storage creation functions
40+
f.hasQualifiedName("github.com/go-git/go-git/v6/storage/filesystem", ["NewStorage", "NewStorageWithOptions"])
41+
or
42+
f.hasQualifiedName("github.com/go-git/go-git/v6/storage/transactional", "NewStorage")
43+
or
44+
// Submodule.Repository() method
45+
f.hasQualifiedName("github.com/go-git/go-git/v6", "Submodule", "Repository")
46+
or
47+
// Worktree.Repository() method
48+
f.hasQualifiedName("github.com/go-git/go-git/v6", "Worktree", "Repository")
49+
or
50+
// Repository.Worktree() method returns a Worktree with repository field
51+
f.hasQualifiedName("github.com/go-git/go-git/v6", "Repository", "Worktree")
52+
)
53+
}
54+
}
55+
56+
/**
57+
* A call to Close() method on a resource.
58+
*/
59+
class CloseCall extends MethodCall {
60+
CloseCall() {
61+
this.getTarget().getName() = "Close" and
62+
this.getReceiver().getType() instanceof ResourceType
63+
}
64+
}
65+
66+
/**
67+
* Checks if a variable has a Close() call (direct or in defer) in the same function.
68+
*/
69+
predicate hasCloseCall(SsaVariable v) {
70+
exists(CloseCall close |
71+
close.getReceiver() = v.getAUse()
72+
)
73+
or
74+
// Check for defer Close() patterns
75+
exists(DeferStmt defer, CloseCall close |
76+
defer.getCall() = close and
77+
close.getReceiver() = v.getAUse()
78+
)
79+
or
80+
// Check for defer func() { _ = x.Close() }() patterns
81+
exists(DeferStmt defer, FuncLit fn, AssignStmt assign, CloseCall close |
82+
defer.getCall().(CallExpr).getCalleeExpr() = fn and
83+
fn.getBody().getAStmt() = assign and
84+
assign.getRhs(0) = close and
85+
close.getReceiver() = v.getAUse()
86+
)
87+
}
88+
89+
from ResourceCreation create, SsaVariable v
90+
where
91+
// The resource is assigned to a variable
92+
v.getDefinition().(SsaExplicitDefinition).getInstruction().getNode() = create and
93+
// The variable is not closed
94+
not hasCloseCall(v) and
95+
// The variable is not assigned to a field (which might be closed elsewhere)
96+
not exists(Field f | v.getAUse() = f.getAWrite().getRhs()) and
97+
// The variable is not returned (caller's responsibility)
98+
not exists(ReturnStmt ret | v.getAUse() = ret.getExpr())
99+
select create, "This resource is created but never closed, which may cause file handle leaks."

0 commit comments

Comments
 (0)