codeql: Improve unclosed resources query with better cleanup detection

.github/workflows/codeql.yml

@@ -10,29 +10,36 @@ on:
         type: string
   push:
     branches: [main]
+    paths:
+      - 'codeql/**'
+      - '.github/workflows/codeql.yml'
+  pull_request:
+    paths:
+      - 'codeql/**'
+      - '.github/workflows/codeql.yml'
 
-permissions:
-  contents: read
-  security-events: write
+permissions: {}
 
 jobs:
   analyze:
     name: Analyze go-git with custom queries
     runs-on: ubuntu-latest
 
+    permissions:
+      actions: read
+      contents: read
+      security-events: write
+
     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: Initialize CodeQL
         uses: github/codeql-action/init@68bde559dea0fdcac2102bfdf6230c5f70eb485e # v4.35.4
@@ -43,8 +50,57 @@ jobs:
               - go-git
             queries:
               - uses: ./codeql/queries/unclosed-resources.ql
+        env:
+          CODEQL_EXTRACTOR_GO_OPTION_EXTRACT_TESTS: true
+
+      - name: Manual Build
+        working-directory: go-git
+        run: |
+          go build ./...
+          # Compile test files for CodeQL analysis
+          mkdir -p /tmp/codeql-build
+          for pkg in $(go list ./...); do
+            go test -c "$pkg" -o "/tmp/codeql-build/$(basename $pkg).test" || true
+          done
+          # Compile examples (ignored by ./... due to _ prefix)
+          # Build from example directories to ensure proper module context
+          find _examples -type d -mindepth 1 -maxdepth 2 | while read dir; do
+            if [ -f "$dir/main.go" ]; then
+              echo "Building example: $dir"
+              (cd "$dir" && go build -o "/tmp/codeql-build/$(basename $dir)" .) || true
+            fi
+          done
 
       - name: Perform CodeQL Analysis
         uses: github/codeql-action/analyze@68bde559dea0fdcac2102bfdf6230c5f70eb485e # v4.35.4
         with:
           category: go-git-custom-queries
+          output: sarif-results
+          upload: never
+
+      - name: Display CodeQL Results
+        if: always()
+        run: |
+          echo "=== CodeQL Analysis Results ==="
+          if [ -d "sarif-results" ]; then
+            result_count=0
+            for sarif in sarif-results/*.sarif; do
+              if [ -f "$sarif" ]; then
+                echo "Processing $sarif..."
+                # Output as GitHub Actions annotations and echo location to logs
+                jq -r '.runs[].results[] |
+                  "::\(.level // "warning") file=\(.locations[0].physicalLocation.artifactLocation.uri),line=\(.locations[0].physicalLocation.region.startLine),title=\(.ruleId)::\(.message.text | gsub("%"; "%25") | gsub("\r"; "%0D") | gsub("\n"; "%0A"))\n" +
+                  "at \(.locations[0].physicalLocation.artifactLocation.uri):\(.locations[0].physicalLocation.region.startLine)"' "$sarif" 2>/dev/null || echo "No results in $sarif"
+                # Count results
+                count=$(jq '[.runs[].results[]] | length' "$sarif" 2>/dev/null || echo "0")
+                result_count=$((result_count + count))
+              fi
+            done
+            if [ $result_count -gt 10 ]; then
+              echo "::notice::Found $result_count CodeQL result(s) - GitHub Actions limits annotations to 10, see logs for complete results"
+            else
+              echo "::notice::Found $result_count CodeQL result(s)"
+            fi
+          else
+            echo "::warning::No SARIF results directory found"
+          fi

codeql/README.md

@@ -11,8 +11,38 @@ 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 stored in struct literals (managed by struct lifecycle)
+- Direct calls to `memory.NewStorage` (no cleanup needed)
+- Factory functions that only create memory storage
+- Factory functions that return existing resources without creating new ones (getters)
+- Factory functions that register their own cleanup (cache-or-create patterns)
+- Resources passed to wrapper types that are properly closed (including inline arguments)
+- Resources cleaned up via returned callback functions
+- Resources closed via type assertions: `if c, ok := st.(io.Closer); ok { c.Close() }`
+- Error path tests where creation is followed by error assertions (`Error`, `ErrorIs`, `NotNil`, etc.)
+
+**Detection techniques:**
+- Precise variable name tracking for defer statements and cleanup callbacks
+- Tuple assignment support: `r, err := git.Clone(...)`
+- Variable declaration support: `var sto storage.Storer = filesystem.NewStorage(...)`
+- Type assertion pattern recognition
+- Wrapper pattern detection (e.g., transactional storage wrapping filesystem storage)
+- Inline argument detection: `r := Open(NewStorage(...))`
+- Struct field cleanup patterns: `s.Field = Create(); r := s.Field; t.Cleanup(func() { r.Close() })`
+- Returned callback function analysis
+- Testing cleanup via `t.Cleanup()` or `b.Cleanup()` (with dataflow tracking)
+- Error path detection to filter out tests that expect creation to fail
+
+**Known limitations:**
+- Does not track inter-procedural dataflow beyond one level (e.g., resources passed to helper functions)
+- Suite-level cleanup in test lifecycle methods (e.g., `TearDownTest()`) may not be detected
+- Complex ownership transfer patterns may require manual verification
+- Local dataflow only - does not track through struct fields in general (exception: specific cleanup patterns)
+- Designed for high precision (minimal false positives) while maintaining high recall for actual resource leaks
 
 **Example violations:**
 
@@ -38,20 +68,55 @@ 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")
+}
+
+// ALSO GOOD: Error path test (expects creation to fail)
+func TestOpenInvalid(t *testing.T) {
+    r, err := git.PlainOpen("/invalid/path")
+    require.Error(t, err)  // Test expects error
+    require.Nil(t, r)
+}
+
+// ALSO GOOD: Testing cleanup callback
+func TestWithCleanup(t *testing.T) {
+    r, err := git.PlainOpen("/path/to/repo")
+    require.NoError(t, err)
+    t.Cleanup(func() { _ = r.Close() })
+    // ... use r
+}
+
+// ALSO GOOD: Inline wrapper pattern
+func example() error {
+    r, err := git.Open(filesystem.NewStorage(...), fs)  // Storage passed inline
+    if err != nil {
+        return err
+    }
+    defer func() { _ = r.Close() }()  // Closing r also closes storage
+    return nil
+}
 ```
 
 ## 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 pushes to main and can be manually triggered via workflow_dispatch. Test files are included in the analysis.
 
 ## Contributing