From 170d798e439507534b880d12ec85eabf92ccf166 Mon Sep 17 00:00:00 2001 From: Arieh Schneier <15041913+AriehSchneier@users.noreply.github.com> Date: Fri, 8 May 2026 00:52:44 +1000 Subject: [PATCH 1/2] 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 Signed-off-by: Arieh Schneier <15041913+AriehSchneier@users.noreply.github.com> --- .github/workflows/codeql.yml | 55 ++++++++++++++++ codeql/README.md | 63 ++++++++++++++++++ codeql/codeql-config.yml | 4 ++ codeql/qlpack.yml | 5 ++ codeql/queries/unclosed-resources.ql | 99 ++++++++++++++++++++++++++++ 5 files changed, 226 insertions(+) create mode 100644 .github/workflows/codeql.yml create mode 100644 codeql/README.md create mode 100644 codeql/codeql-config.yml create mode 100644 codeql/qlpack.yml create mode 100644 codeql/queries/unclosed-resources.ql diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml new file mode 100644 index 0000000..ca12a84 --- /dev/null +++ b/.github/workflows/codeql.yml @@ -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 + 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 + with: + category: go-git-custom-queries + checkout-path: go-git diff --git a/codeql/README.md b/codeql/README.md new file mode 100644 index 0000000..aa48a4a --- /dev/null +++ b/codeql/README.md @@ -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 diff --git a/codeql/codeql-config.yml b/codeql/codeql-config.yml new file mode 100644 index 0000000..7ec60cf --- /dev/null +++ b/codeql/codeql-config.yml @@ -0,0 +1,4 @@ +name: "go-git resource leak detection" + +queries: + - uses: ./queries/unclosed-resources.ql diff --git a/codeql/qlpack.yml b/codeql/qlpack.yml new file mode 100644 index 0000000..fb9976a --- /dev/null +++ b/codeql/qlpack.yml @@ -0,0 +1,5 @@ +name: go-git/codeql-queries +version: 0.0.1 +library: false +dependencies: + codeql/go-all: "*" diff --git a/codeql/queries/unclosed-resources.ql b/codeql/queries/unclosed-resources.ql new file mode 100644 index 0000000..c3fa602 --- /dev/null +++ b/codeql/queries/unclosed-resources.ql @@ -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." From 7fa77d4cafb28d41b7a3f88825d0d9a6260fb35a Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Thu, 7 May 2026 22:26:42 +0100 Subject: [PATCH 2/2] Apply suggestions from code review Co-authored-by: Paulo Gomes Signed-off-by: Paulo Gomes --- .github/workflows/codeql.yml | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index ca12a84..5b07c23 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -10,9 +10,6 @@ on: type: string push: branches: [main] - schedule: - # Run weekly on Mondays at 00:00 UTC - - cron: '0 0 * * 1' permissions: contents: read @@ -41,7 +38,7 @@ jobs: run: cp -r codeql go-git/ - name: Initialize CodeQL - uses: github/codeql-action/init@c3f298df8c1fea2fefe20c785e6aa00f32df8260 # v4.35.3 + uses: github/codeql-action/init@68bde559dea0fdcac2102bfdf6230c5f70eb485e # v4.35.4 with: languages: go source-root: go-git @@ -49,7 +46,7 @@ jobs: checkout-path: go-git - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@c3f298df8c1fea2fefe20c785e6aa00f32df8260 # v4.35.3 + uses: github/codeql-action/analyze@68bde559dea0fdcac2102bfdf6230c5f70eb485e # v4.35.4 with: category: go-git-custom-queries checkout-path: go-git