Add static analysis with golangci-lint to prevent nil pointer exceptions

.circleci/config.yml

@@ -13,6 +13,19 @@ defaults: &defaults
     - image: 087285199408.dkr.ecr.us-east-1.amazonaws.com/circle-ci-test-image-base:go1.22.6-tf1.5-tg58.8-pck1.8-ci56.0
 version: 2.1
 jobs:
+  lint:
+    <<: *defaults
+    steps:
+      - checkout
+      - run:
+          name: Install golangci-lint
+          command: |
+            curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.61.0
+      - run:
+          name: Run golangci-lint
+          command: |
+            golangci-lint run --new-from-rev=origin/master ./...
+
   test:
     <<: *defaults
     steps:
@@ -259,7 +272,16 @@ workflows:
       not:
         equal: [ scheduled_pipeline, << pipeline.trigger_source >> ]
     jobs:
+      - lint:
+          filters:
+            tags:
+              only: /^v.*/
+          context:
+            - AWS__PHXDEVOPS__circle-ci-test
+            - GITHUB__PAT__gruntwork-ci
       - test:
+          requires:
+            - lint
           filters:
             tags:
               only: /^v.*/

.golangci.yml

@@ -0,0 +1,90 @@
+run:
+  timeout: 10m
+  tests: true
+  modules-download-mode: readonly
+  go: "1.23"
+
+linters:
+  enable:
+    # Default linters (these come with golangci-lint)
+    - errcheck      # Checking for unchecked errors - critical for nil safety
+    - gosimple      # Simplify code  
+    - govet         # Reports suspicious constructs including nil issues
+    - ineffassign   # Detects when assignments to existing variables are not used
+    - staticcheck   # Comprehensive static analysis including nil checks (SA5011)
+    - typecheck     # Like the front-end of a Go compiler, parses and type-checks
+    - unused        # Checks for unused constants, variables, functions and types
+
+    # Critical nil safety linters
+    - nilnil        # Checks that there is no simultaneous return of nil error and an invalid value
+    - nilerr        # Finds the code that returns nil even if it checks that the error is not nil
+
+    # Code quality linters
+    - bodyclose     # Checks whether HTTP response body is closed successfully
+    - durationcheck # Check for two common problems with time.Duration
+    - errorlint     # Find code that will cause problems with error handling
+    - copyloopvar   # Checks for pointers to enclosing loop variables (replaces exportloopref)
+    - noctx         # Finds sending http request without context.Context
+
+    # Style linters
+    - gofmt         # Checks whether code was gofmt-ed
+    - goimports     # Check import statements are formatted
+    - misspell      # Finds commonly misspelled English words in comments
+    - whitespace    # Detection of leading and trailing whitespace
+
+  disable:
+    - depguard      # Too restrictive for dependencies
+    - exhaustive    # Too strict for switch statements
+    - gochecknoglobals # We use some globals
+    - lll           # Line length limit is too strict
+    - wsl           # Whitespace linter is too opinionated
+
+linters-settings:
+  errcheck:
+    check-type-assertions: true
+    check-blank: true
+
+  govet:
+    enable:
+      - shadow  # Check for shadowed variables
+
+  staticcheck:
+    checks: ["all", "-ST1000", "-ST1003"]  # Disable some stylistic checks
+
+  nilnil:
+    checked-types:
+      - ptr
+      - func
+      - iface
+      - map
+      - chan
+
+issues:
+  # Only show issues on new/changed lines
+  # This makes adoption easier for existing codebases
+  new: true
+
+  # Exclude directories from all linters
+  exclude-dirs:
+    - vendor
+    - third_party
+    - generated
+
+  exclude-rules:
+    # Exclude test files from some linters
+    - path: _test\.go
+      linters:
+        - errcheck
+        - noctx
+
+    # Exclude generated files
+    - path: "generated"
+      linters:
+        - staticcheck
+        - govet
+
+  # Maximum issues count per one linter
+  max-issues-per-linter: 0
+
+  # Maximum count of issues with the same text
+  max-same-issues: 0

.pre-commit-config.yaml

@@ -11,3 +11,8 @@ repos:
   hooks:
     - id: shellcheck
     - id: gofmt
+- repo: https://github.com/golangci/golangci-lint
+  rev: v1.55.2
+  hooks:
+    - id: golangci-lint
+      args: ['--config=.golangci.yml']

docs/STATIC_ANALYSIS.md

@@ -0,0 +1,50 @@
+# Static Analysis
+
+This project uses **golangci-lint** to prevent nil pointer dereferences and other common Go bugs.
+
+## Key Linters for Nil Safety
+- **staticcheck**: Comprehensive analysis including nil checks (SA5011)
+- **errcheck**: Ensures errors are checked before using returns
+- **nilnil**: Prevents returning nil error with nil value
+- **nilerr**: Catches returning nil when error is not nil
+
+## Local Development
+
+```bash
+# Install and run pre-commit hooks (recommended)
+pip install pre-commit
+pre-commit install
+pre-commit run --all-files
+
+# Or run golangci-lint directly
+golangci-lint run --new-from-rev=origin/master ./...
+```
+
+## CI/CD
+
+CircleCI runs linting on all PRs, checking only new/modified code (`--new-from-rev=origin/master`).
+
+## Common Fixes
+
+### AWS SDK nil responses
+```go
+// ❌ Bad
+output, err := client.DescribeResourcePolicy(ctx, input)
+if err != nil {
+    return err
+}
+policy := output.Policy // Potential nil panic
+
+// ✅ Good
+output, err := client.DescribeResourcePolicy(ctx, input)
+if err != nil {
+    return err
+}
+if output != nil && output.Policy != nil {
+    policy := output.Policy
+}
+```
+
+## Configuration
+
+See `.golangci.yml`. Currently configured to check only new code for easier adoption.