Skip to content

Commit bb2e8cd

Browse files
pjbgfpjbgf
authored
Merge pull request #8 from AriehSchneier/add-codeql-resource-leak-detection
Add CodeQL query to detect unclosed Repository and Storage instances
2 parents e7fc238 + 7fa77d4 commit bb2e8cd

5 files changed

Lines changed: 223 additions & 0 deletions

File tree

‎.github/workflows/codeql.yml‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
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+
14+
permissions:
15+
contents: read
16+
security-events: write
17+
18+
jobs:
19+
analyze:
20+
name: Analyze go-git with custom queries
21+
runs-on: ubuntu-latest
22+
23+
steps:
24+
- name: Checkout x repository
25+
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
26+
with:
27+
fetch-depth: 0
28+
29+
- name: Checkout go-git repository
30+
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
31+
with:
32+
repository: go-git/go-git
33+
ref: ${{ inputs.go-git-ref || 'main' }}
34+
path: go-git
35+
fetch-depth: 0
36+
37+
- name: Copy CodeQL queries to go-git
38+
run: cp -r codeql go-git/
39+
40+
- name: Initialize CodeQL
41+
uses: github/codeql-action/init@68bde559dea0fdcac2102bfdf6230c5f70eb485e # v4.35.4
42+
with:
43+
languages: go
44+
source-root: go-git
45+
config-file: go-git/codeql/codeql-config.yml
46+
checkout-path: go-git
47+
48+
- name: Perform CodeQL Analysis
49+
uses: github/codeql-action/analyze@68bde559dea0fdcac2102bfdf6230c5f70eb485e # v4.35.4
50+
with:
51+
category: go-git-custom-queries
52+
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)