Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
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
2 changes: 2 additions & 0 deletions .github/workflows/codeql.yml
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ jobs:
- go-git
queries:
- uses: ./codeql/queries/unclosed-resources.ql
env:
CODEQL_EXTRACTOR_GO_OPTION_EXTRACT_TESTS: true

Comment on lines 44 to 55
- name: Perform CodeQL Analysis
uses: github/codeql-action/analyze@68bde559dea0fdcac2102bfdf6230c5f70eb485e # v4.35.4
Expand Down
39 changes: 35 additions & 4 deletions codeql/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,29 @@ Detects instances where `Repository` or `Storage` objects are created but never
**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
- Missing `Close()` calls on created resources

**What it excludes (to avoid false positives):**
- Factory functions that return Repository/Storage to the caller
- Resources assigned to struct fields (managed by struct lifecycle)
- Direct calls to `memory.NewStorage` (no cleanup needed)
- Factory functions that only create memory storage
- Resources passed to wrapper types that are properly closed
- Resources cleaned up via returned callback functions
- Resources closed via type assertions: `if c, ok := st.(io.Closer); ok { c.Close() }`

**Detection techniques:**
- Precise variable name tracking for defer statements
- Tuple assignment support: `r, err := git.Clone(...)`
- Type assertion pattern recognition
- Wrapper pattern detection (e.g., transactional storage wrapping filesystem storage)
- Returned callback function analysis
- Testing cleanup via `t.Cleanup()` or `b.Cleanup()`

**Known limitations:**
- Does not track inter-procedural dataflow beyond one level (e.g., resources passed to helper functions)
- Complex ownership transfer patterns may require manual verification
- Designed for high precision (minimal false positives) while maintaining full leak detection coverage

**Example violations:**

Expand All @@ -38,20 +59,30 @@ func good() error {
_, err = r.Head()
return err
}

// ALSO GOOD: Factory function (caller's responsibility)
func createRepo() (*git.Repository, error) {
return git.PlainOpen("/path/to/repo")
}
```

## Running the queries

### Using CodeQL CLI

To include test files in the analysis (recommended):

```bash
codeql database create /tmp/go-git-db --language=go --source-root=/path/to/go-git
CODEQL_EXTRACTOR_GO_OPTION_EXTRACT_TESTS=true \
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
```

Without the environment variable, `*_test.go` files are excluded by default.

### Using GitHub Actions

The queries run automatically via the CodeQL workflow on pull requests.
The queries run automatically via the CodeQL workflow on pull requests and include test files.

## Contributing

Expand Down
Loading
Loading