feat(cli): add sarif output format

.github/workflows/sarif-upload-check.yml

@@ -0,0 +1,132 @@
+name: SARIF Upload Check
+
+# Uploads a real decolint SARIF log to code scanning to prove GitHub accepts it and resolves the
+# reported paths to files in the repository, then deletes the analysis so the Security tab is left
+# as it was found. Runs on demand, and on a pull request that edits this workflow, since a workflow
+# nothing triggers automatically is otherwise only proven broken the next time someone runs it.
+on:
+  workflow_dispatch:
+  pull_request:
+    paths:
+      - .github/workflows/sarif-upload-check.yml
+
+permissions: {}
+
+env:
+  # A category of its own puts this check in an analysis set of its own, so it can neither replace
+  # nor be replaced by a real analysis, and the cleanup can target it exactly.
+  CATEGORY: decolint-sarif-upload-check
+  TOOL_NAME: decolint
+  FIXTURE: cmd/decolint/testdata/e2e/violations
+
+jobs:
+  upload-check:
+    runs-on: ubuntu-latest
+    # A pull request from a fork is given a read-only token however this workflow declares its
+    # permissions, so the upload could only fail. Skip it rather than report a failure.
+    if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository
+    permissions:
+      contents: read
+      security-events: write
+    steps:
+      - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
+        with:
+          persist-credentials: false
+      - uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6.5.0
+        with:
+          go-version-file: go.mod
+      - run: make build
+
+      - name: Generate a SARIF log
+        # decolint exits 1 when it reports findings, which is what this check wants; any other
+        # non-zero status is a real failure. It runs from the repository root so that the reported
+        # paths are relative to it, which is how code scanning resolves them. Merging is off so the
+        # run needs no registry access and cannot fail for reasons unrelated to the upload.
+        run: ./bin/decolint -format=sarif -merge=false "$FIXTURE" > decolint.sarif || [ "$?" -eq 1 ]
+
+      - name: Upload the SARIF log
+        id: upload
+        # wait-for-processing defaults to true, so this step fails if code scanning rejects the log.
+        uses: github/codeql-action/upload-sarif@99df26d4f13ea111d4ec1a7dddef6063f76b97e9 # v4.37.0
+        with:
+          sarif_file: decolint.sarif
+          category: ${{ env.CATEGORY }}
+
+      - name: Check the analysis
+        env:
+          GH_TOKEN: ${{ github.token }}
+          SARIF_ID: ${{ steps.upload.outputs.sarif-id }}
+        run: |
+          analyses=$(gh api "repos/$GITHUB_REPOSITORY/code-scanning/analyses?sarif_id=$SARIF_ID")
+          echo "$analyses" | jq .
+          # The category is stored as the automation id, which carries a trailing slash.
+          echo "$analyses" | jq -e --arg tool "$TOOL_NAME" --arg category "$CATEGORY" '
+            length == 1
+              and .[0].tool.name == $tool
+              and (.[0].category // "" | rtrimstr("/")) == $category
+              and .[0].results_count > 0
+          ' > /dev/null
+          # The upload picks the ref the analysis lands on, which for a pull request is a refs/pull
+          # ref rather than GITHUB_REF, so take it from the analysis instead of assuming it.
+          echo "ANALYSIS_REF=$(echo "$analyses" | jq -r '.[0].ref')" >> "$GITHUB_ENV"
+
+      - name: Check the findings resolve to repository files
+        env:
+          GH_TOKEN: ${{ github.token }}
+        run: |
+          want=$(jq -nc --arg path "$FIXTURE/.devcontainer/devcontainer.json" '[$path]')
+          # Alerts can lag the analysis, so give them a bounded chance to appear.
+          for _ in $(seq 1 10); do
+            got=$(gh api "repos/$GITHUB_REPOSITORY/code-scanning/alerts?tool_name=$TOOL_NAME&ref=$ANALYSIS_REF&per_page=100" 2>/dev/null || echo '[]')
+            got=$(printf '%s' "$got" | jq -c '[.[] | .most_recent_instance.location.path] | unique')
+            if [ "$got" = "$want" ]; then
+              echo "alerts on $ANALYSIS_REF resolve to $got"
+              exit 0
+            fi
+            sleep 5
+          done
+          echo "::error::alert paths are $got, want $want"
+          exit 1
+
+      - name: Delete the uploaded analysis
+        # Runs even when a check above failed, so a failure never leaves an analysis behind.
+        if: always()
+        env:
+          GH_TOKEN: ${{ github.token }}
+        run: |
+          ref="${ANALYSIS_REF:-$GITHUB_REF}"
+          api() { gh api "repos/$GITHUB_REPOSITORY/$1" 2>/dev/null || echo '[]'; }
+          # Not filtered by ref: the category belongs to this workflow alone, so an analysis carrying
+          # it is one of ours whichever ref it landed on, including one an earlier run left behind.
+          analyses() {
+            api "code-scanning/analyses?tool_name=$TOOL_NAME&per_page=100" \
+              | jq -c --arg category "$CATEGORY" '[.[] | select((.category // "" | rtrimstr("/")) == $category)]'
+          }
+          # The alerts endpoint cannot filter by category, so this sweeps every decolint alert on the
+          # ref. That is exact while this workflow is decolint's only uploader; should a real
+          # decolint analysis ever be added, narrow it, or it will report that alert as left behind.
+          alerts() { api "code-scanning/alerts?tool_name=$TOOL_NAME&ref=$ref&per_page=100"; }
+
+          # Only the most recent analysis of a set is deletable, so deleting a set takes one request
+          # per analysis. confirm_delete is required for the last one, which also discards its
+          # alerts. The loop is bounded so an analysis that refuses to go cannot hang the job.
+          for _ in $(seq 1 20); do
+            id=$(analyses | jq -r '[.[] | select(.deletable)] | .[0].id // empty')
+            if [ -z "$id" ]; then
+              break
+            fi
+            echo "deleting analysis $id"
+            gh api --method DELETE "repos/$GITHUB_REPOSITORY/code-scanning/analyses/$id?confirm_delete=true" --silent
+          done
+
+          for _ in $(seq 1 10); do
+            left_analyses=$(analyses | jq 'length')
+            left_alerts=$(alerts | jq 'length')
+            if [ "$left_analyses" -eq 0 ] && [ "$left_alerts" -eq 0 ]; then
+              echo "nothing was left behind"
+              exit 0
+            fi
+            sleep 3
+          done
+          echo "::error::$left_analyses analyses and $left_alerts alerts were left behind"
+          exit 1

README.md

@@ -187,6 +187,19 @@ Select a different output format to change this:
   ```
   ::warning file=.devcontainer/devcontainer.json,line=4,col=12,title=no-image-latest::image "ubuntu:latest" uses the "latest" tag; pin a specific version
   ```
+- `sarif` — a [SARIF 2.1.0](https://docs.oasis-open.org/sarif/sarif/v2.1.0/sarif-v2.1.0.html)
+  log, for upload to [GitHub Code
+  Scanning](https://docs.github.com/en/code-security/code-scanning/integrating-with-code-scanning/uploading-a-sarif-file-to-github)
+  so findings appear as alerts in the repository's Security tab:
+  ```yaml
+  - run: decolint -merge -format=sarif . > decolint.sarif
+    continue-on-error: true # decolint exits 1 on findings; the upload must still run
+  - uses: github/codeql-action/upload-sarif@v3
+    with:
+      sarif_file: decolint.sarif
+  ```
+  Run `decolint` from the repository root, so that the paths in the log
+  resolve to files in the repository.
 
 Every format reports paths relative to the directory `decolint` runs in,
 whichever way the linted directory was named on the command line. A file

cmd/decolint/config.go

@@ -24,7 +24,7 @@ type Config struct {
 	// also cause exit code 1. The -deny-warnings flag can enable it as well.
 	DenyWarnings bool `json:"denyWarnings"`
 	// Format selects how lint issues are written to stdout: "text" (the default when empty), "json",
-	// or "github". The -format flag takes precedence.
+	// "github", or "sarif". The -format flag takes precedence.
 	Format string `json:"format"`
 	// LocalEnv maps names to the values "${localEnv:NAME}" (and "${env:NAME}") resolve to during
 	// variable substitution, which runs with Merge; see [substitute.Context.LocalEnv]. It is also

cmd/decolint/format.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/bare-devcontainer/decolint/format"
 	"github.com/bare-devcontainer/decolint/linter"
+	"github.com/bare-devcontainer/decolint/rules"
 )
 
 // Format identifies how lint issues are written to stdout.
@@ -25,7 +26,24 @@ func parseFormat(name string) (Format, error) {
 		return format.JSONFormat{}, nil
 	case "github":
 		return format.GitHubFormat{}, nil
+	case "sarif":
+		return format.SARIFFormat{Version: version, Rules: sarifRules()}, nil
 	default:
-		return nil, fmt.Errorf("unknown format %q (want one of: text, json, github)", name)
+		return nil, fmt.Errorf("unknown format %q (want one of: text, json, github, sarif)", name)
 	}
 }
+
+// sarifRules adapts the built-in rule catalog into the shape [format.SARIFFormat] consumes, so the
+// format package does not depend on the rules package.
+func sarifRules() []format.SARIFRule {
+	regs := rules.Builtin()
+	out := make([]format.SARIFRule, len(regs))
+	for i, reg := range regs {
+		out[i] = format.SARIFRule{
+			ID:          reg.Rule.ID,
+			Description: reg.Rule.Description,
+			Category:    reg.Rule.Category.String(),
+		}
+	}
+	return out
+}

cmd/decolint/format_test.go

@@ -4,6 +4,7 @@ import (
 	"testing"
 
 	"github.com/bare-devcontainer/decolint/format"
+	"github.com/google/go-cmp/cmp"
 )
 
 func TestParseFormat(t *testing.T) {
@@ -19,6 +20,7 @@ func TestParseFormat(t *testing.T) {
 		{"text", "text", format.TextFormat{}, false},
 		{"json", "json", format.JSONFormat{}, false},
 		{"github", "github", format.GitHubFormat{}, false},
+		{"sarif", "sarif", format.SARIFFormat{Version: version, Rules: sarifRules()}, false},
 		{"unknown", "bogus", nil, true},
 	}
 	for _, tt := range tests {
@@ -31,8 +33,10 @@ func TestParseFormat(t *testing.T) {
 			if err != nil {
 				return
 			}
-			if got != tt.want {
-				t.Errorf("parseFormat(%q) = %v, want %v", tt.in, got, tt.want)
+			// cmp.Diff rather than ==: SARIFFormat is not comparable, so an interface comparison
+			// would panic.
+			if diff := cmp.Diff(tt.want, got); diff != "" {
+				t.Errorf("parseFormat(%q) mismatch (-want +got):\n%s", tt.in, diff)
 			}
 		})
 	}

cmd/decolint/init.go

@@ -43,8 +43,8 @@ func initConfigFile(output io.Writer) error {
 // "denyWarnings", when true, treats warnings as failures (exit code 1);
 // the -deny-warnings flag takes precedence, e.g.:
 //   "denyWarnings": true
-// "format" selects the output format ("text", "json", or "github"); the
-// -format flag takes precedence, e.g.:
+// "format" selects the output format ("text", "json", "github", or
+// "sarif"); the -format flag takes precedence, e.g.:
 //   "format": "github"
 // "localEnv" supplies the values ${localEnv:NAME} resolves to when merging,
 // and the environment Compose-file interpolation reads; environment

cmd/decolint/main_test.go

@@ -9,6 +9,7 @@ import (
 	"io"
 	"io/fs"
 	"os"
+	"path"
 	"path/filepath"
 	"strings"
 	"testing"
@@ -516,6 +517,45 @@ func TestRun_Flags(t *testing.T) {
 	})
 }
 
+// The sarif* types decode the structural subset of a SARIF log the "sarif log" test asserts. The
+// message texts and rule descriptions the format carries are left out on purpose: they are owned by
+// the rule tests and the format package's own tests.
+type (
+	sarifLog struct {
+		Version string     `json:"version"`
+		Runs    []sarifRun `json:"runs"`
+	}
+	sarifRun struct {
+		Tool    sarifTool     `json:"tool"`
+		Results []sarifResult `json:"results"`
+	}
+	sarifTool struct {
+		Driver sarifDriver `json:"driver"`
+	}
+	sarifDriver struct {
+		Name  string      `json:"name"`
+		Rules []sarifRule `json:"rules"`
+	}
+	sarifRule struct {
+		ID string `json:"id"`
+	}
+	sarifResult struct {
+		RuleID    string          `json:"ruleId"`
+		RuleIndex int             `json:"ruleIndex"`
+		Level     string          `json:"level"`
+		Locations []sarifLocation `json:"locations"`
+	}
+	sarifLocation struct {
+		PhysicalLocation sarifPhysicalLocation `json:"physicalLocation"`
+	}
+	sarifPhysicalLocation struct {
+		ArtifactLocation sarifArtifactLocation `json:"artifactLocation"`
+	}
+	sarifArtifactLocation struct {
+		URI string `json:"uri"`
+	}
+)
+
 func TestRun_OutputFormat(t *testing.T) {
 	t.Parallel()
 
@@ -565,6 +605,92 @@ func TestRun_OutputFormat(t *testing.T) {
 		}
 	})
 
+	t.Run("sarif log", func(t *testing.T) {
+		t.Parallel()
+
+		var stdout, stderr bytes.Buffer
+		exitCode := run(t.Context(), []string{"-format=sarif", "-platform=vscode,codespaces", violationsDir}, &stdout, &stderr)
+		if exitCode != 1 {
+			t.Fatalf("exit code = %d, want 1; stderr: %s", exitCode, stderr.String())
+		}
+
+		var log sarifLog
+		if err := json.Unmarshal(stdout.Bytes(), &log); err != nil {
+			t.Fatalf("output is not a SARIF log: %v\noutput: %s", err, stdout.String())
+		}
+
+		// The fixture is inside the working directory, so it is reported relative to it.
+		locAt := func(uri string) []sarifLocation {
+			return []sarifLocation{{PhysicalLocation: sarifPhysicalLocation{
+				ArtifactLocation: sarifArtifactLocation{URI: uri},
+			}}}
+		}
+		// The catalog lists only referenced rules, sorted by ID, so each result's ruleIndex points at
+		// its like-named catalog entry.
+		want := sarifLog{
+			Version: "2.1.0",
+			Runs: []sarifRun{{
+				Tool: sarifTool{Driver: sarifDriver{
+					Name:  "decolint",
+					Rules: []sarifRule{{ID: "no-bind-mount"}, {ID: "no-host-port-format"}},
+				}},
+				Results: []sarifResult{
+					{RuleID: "no-bind-mount", RuleIndex: 0, Level: "error", Locations: locAt(violationsFile)},
+					{RuleID: "no-host-port-format", RuleIndex: 1, Level: "error", Locations: locAt(violationsFile)},
+				},
+			}},
+		}
+		sortResults := cmpopts.SortSlices(func(a, b sarifResult) bool { return a.RuleID < b.RuleID })
+		if diff := cmp.Diff(want, log, sortResults); diff != "" {
+			t.Errorf("sarif log mismatch (-want +got):\n%s", diff)
+		}
+	})
+
+	t.Run("sarif log outside the working directory", func(t *testing.T) {
+		t.Parallel()
+
+		// A directory outside the working directory is reported absolutely, which SARIF can only
+		// express as an absolute file URI. The fixture trips missing-container-def, an error by
+		// default and free of any platform or fetch.
+		dir := writeDevcontainer(t, `{}`)
+
+		var stdout, stderr bytes.Buffer
+		exitCode := run(t.Context(), []string{"-format=sarif", dir}, &stdout, &stderr)
+		if exitCode != 1 {
+			t.Fatalf("exit code = %d, want 1; stderr: %s", exitCode, stderr.String())
+		}
+
+		var log sarifLog
+		if err := json.Unmarshal(stdout.Bytes(), &log); err != nil {
+			t.Fatalf("output is not a SARIF log: %v\noutput: %s", err, stdout.String())
+		}
+
+		// A file URI's path carries exactly one leading slash, which a POSIX path already has and a
+		// Windows one does not.
+		config := filepath.Join(dir, ".devcontainer", "devcontainer.json")
+		uri := "file://" + path.Join("/", filepath.ToSlash(config))
+		want := sarifLog{
+			Version: "2.1.0",
+			Runs: []sarifRun{{
+				Tool: sarifTool{Driver: sarifDriver{
+					Name:  "decolint",
+					Rules: []sarifRule{{ID: "missing-container-def"}},
+				}},
+				Results: []sarifResult{{
+					RuleID:    "missing-container-def",
+					RuleIndex: 0,
+					Level:     "error",
+					Locations: []sarifLocation{{PhysicalLocation: sarifPhysicalLocation{
+						ArtifactLocation: sarifArtifactLocation{URI: uri},
+					}}},
+				}},
+			}},
+		}
+		if diff := cmp.Diff(want, log); diff != "" {
+			t.Errorf("sarif log mismatch (-want +got):\n%s", diff)
+		}
+	})
+
 	t.Run("format selected by config file", func(t *testing.T) {
 		t.Parallel()