diff --git a/.github/workflows/sarif-upload-check.yml b/.github/workflows/sarif-upload-check.yml new file mode 100644 index 0000000..4abf930 --- /dev/null +++ b/.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 diff --git a/README.md b/README.md index 0d980d3..195016e 100644 --- a/README.md +++ b/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 diff --git a/cmd/decolint/config.go b/cmd/decolint/config.go index e332a85..85fc947 100644 --- a/cmd/decolint/config.go +++ b/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 diff --git a/cmd/decolint/format.go b/cmd/decolint/format.go index 00b3810..36a9e68 100644 --- a/cmd/decolint/format.go +++ b/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 +} diff --git a/cmd/decolint/format_test.go b/cmd/decolint/format_test.go index df7ad9c..05e6789 100644 --- a/cmd/decolint/format_test.go +++ b/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) } }) } diff --git a/cmd/decolint/init.go b/cmd/decolint/init.go index c6c1f57..f4a4c45 100644 --- a/cmd/decolint/init.go +++ b/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 diff --git a/cmd/decolint/main_test.go b/cmd/decolint/main_test.go index 706aa1e..7b74891 100644 --- a/cmd/decolint/main_test.go +++ b/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() diff --git a/cmd/decolint/opts.go b/cmd/decolint/opts.go index d6e75d3..4e71840 100644 --- a/cmd/decolint/opts.go +++ b/cmd/decolint/opts.go @@ -37,8 +37,8 @@ type Options struct { // "merge": true). mergeSet bool // Format is the raw -format flag value ("" if not given), naming how lint issues are written to - // stdout: "text", "json", or "github". A non-empty value replaces the config file's "format" - // member; it is resolved into a Format by parseFormat in runLint. + // stdout: "text", "json", "github", or "sarif". A non-empty value replaces the config file's + // "format" member; it is resolved into a Format by parseFormat in runLint. Format string // Version, when set, causes the program to print its version and exit. Version bool @@ -60,7 +60,7 @@ func parseOptions(args []string, output io.Writer) (Options, error) { fs.BoolVar(&opts.DenyWarnings, "deny-warnings", false, "treat warnings as failures (exit code 1); overrides the config file's \"denyWarnings\" member") fs.StringVar(&opts.ConfigPath, "config", "", "path to a config file (default: auto-discover .decolint.jsonc or .decolint.json in the current directory)") fs.StringVar(&platformFlag, "platform", "", "comma-separated target platforms to include in addition to \"all\" (vscode, codespaces); overrides the config file's \"platforms\" member") - fs.StringVar(&formatFlag, "format", "", "output format: text (default), json, or github; overrides the config file's \"format\" member") + fs.StringVar(&formatFlag, "format", "", "output format: text (default), json, github, or sarif; overrides the config file's \"format\" member") fs.BoolVar(&opts.Merge, "merge", false, "lint the merged (effective) configuration, including referenced Features and base image metadata; overrides the config file's \"merge\" member") fs.BoolVar(&opts.Version, "version", false, "print version information and exit") fs.BoolVar(&opts.ListRules, "rules", false, "print the built-in rules as a Markdown table (category, target platforms, current severity), then exit") diff --git a/format/doc.go b/format/doc.go index 6fb2c6b..b858a1e 100644 --- a/format/doc.go +++ b/format/doc.go @@ -1,3 +1,3 @@ // Package format implements the output formats decolint can write lint issues in: one-line-per-issue -// text, a JSON array, and GitHub Actions workflow command annotations. +// text, a JSON array, GitHub Actions workflow command annotations, and a SARIF 2.1.0 log. package format diff --git a/format/sarif.go b/format/sarif.go new file mode 100644 index 0000000..8bc5045 --- /dev/null +++ b/format/sarif.go @@ -0,0 +1,197 @@ +package format + +import ( + "bytes" + "encoding/json/v2" + "fmt" + "io" + "maps" + "net/url" + "path/filepath" + "slices" + "strings" + + "github.com/bare-devcontainer/decolint/linter" +) + +// SARIFRule describes one rule for the SARIF rule catalog. It mirrors the [linter.Rule] fields the +// SARIF output needs, so this package does not depend on the rules package. +type SARIFRule struct { + ID string + Description string + Category string +} + +// SARIFFormat prints a SARIF 2.1.0 log, suitable for upload to GitHub Code Scanning. +type SARIFFormat struct { + // Version is the tool version recorded in the run. + Version string + // Rules is the rule catalog used to describe the rules referenced by issues. + Rules []SARIFRule +} + +const ( + sarifSchemaURI = "https://json.schemastore.org/sarif-2.1.0.json" + sarifVersion = "2.1.0" + informationURI = "https://github.com/bare-devcontainer/decolint" +) + +// The wire structs cover the subset of SARIF 2.1.0 decolint emits. Fields marshal in declaration +// order, so the output is deterministic. + +type sarifLog struct { + Schema string `json:"$schema"` + Version string `json:"version"` + Runs []sarifRun `json:"runs"` +} + +type sarifRun struct { + Tool sarifTool `json:"tool"` + Results []sarifResult `json:"results"` +} + +type sarifTool struct { + Driver sarifDriver `json:"driver"` +} + +type sarifDriver struct { + Name string `json:"name"` + Version string `json:"version"` + InformationURI string `json:"informationUri"` + Rules []sarifRuleDescriptor `json:"rules"` +} + +type sarifRuleDescriptor struct { + ID string `json:"id"` + ShortDescription sarifMessage `json:"shortDescription,omitzero"` + Properties sarifProperties `json:"properties,omitzero"` +} + +type sarifProperties struct { + Tags []string `json:"tags"` +} + +type sarifMessage struct { + Text string `json:"text"` +} + +type sarifResult struct { + RuleID string `json:"ruleId"` + RuleIndex int `json:"ruleIndex"` + Level string `json:"level"` + Message sarifMessage `json:"message"` + Locations []sarifLocation `json:"locations"` +} + +type sarifLocation struct { + PhysicalLocation sarifPhysicalLocation `json:"physicalLocation"` +} + +type sarifPhysicalLocation struct { + ArtifactLocation sarifArtifactLocation `json:"artifactLocation"` + Region sarifRegion `json:"region"` +} + +type sarifArtifactLocation struct { + URI string `json:"uri"` +} + +type sarifRegion struct { + StartLine int `json:"startLine"` + StartColumn int `json:"startColumn"` +} + +// WriteIssues writes issues to w as a SARIF 2.1.0 log with a single run. It marshals into an +// in-memory buffer first so that a failure never leaves partial output on w. +// +// The run's rule catalog lists only the rules referenced by issues, sorted by rule ID; a rule +// missing from f.Rules is listed with its ID alone. Issue paths are reported as URIs; see +// [artifactURIFor]. +func (f SARIFFormat) WriteIssues(w io.Writer, issues []linter.Issue) error { + catalog := make(map[string]SARIFRule, len(f.Rules)) + for _, r := range f.Rules { + catalog[r.ID] = r + } + + referenced := make(map[string]bool) + for _, issue := range issues { + referenced[issue.RuleID] = true + } + ids := slices.Sorted(maps.Keys(referenced)) + + descriptors := make([]sarifRuleDescriptor, 0, len(ids)) + index := make(map[string]int, len(ids)) + for i, id := range ids { + index[id] = i + desc := sarifRuleDescriptor{ID: id} + if r, ok := catalog[id]; ok { + desc.ShortDescription = sarifMessage{Text: r.Description} + desc.Properties = sarifProperties{Tags: []string{r.Category}} + } + descriptors = append(descriptors, desc) + } + + results := make([]sarifResult, 0, len(issues)) + for _, issue := range issues { + level := "error" + if issue.Severity == linter.SeverityWarn { + level = "warning" + } + // startColumn is emitted as the issue's byte column even though SARIF defaults to UTF-16 + // code units; the two agree on ASCII lines, and alerts anchor by line regardless. + results = append(results, sarifResult{ + RuleID: issue.RuleID, + RuleIndex: index[issue.RuleID], + Level: level, + Message: sarifMessage{Text: issue.Message}, + Locations: []sarifLocation{{ + PhysicalLocation: sarifPhysicalLocation{ + ArtifactLocation: sarifArtifactLocation{URI: artifactURIFor(issue.Path)}, + Region: sarifRegion{StartLine: issue.Line, StartColumn: issue.Col}, + }, + }}, + }) + } + + log := sarifLog{ + Schema: sarifSchemaURI, + Version: sarifVersion, + Runs: []sarifRun{{ + Tool: sarifTool{Driver: sarifDriver{ + Name: "decolint", + Version: f.Version, + InformationURI: informationURI, + Rules: descriptors, + }}, + Results: results, + }}, + } + + var buf bytes.Buffer + if err := json.MarshalWrite(&buf, log); err != nil { + return fmt.Errorf("marshal sarif: %w", err) + } + buf.WriteByte('\n') + if _, err := w.Write(buf.Bytes()); err != nil { + return fmt.Errorf("write issues: %w", err) + } + return nil +} + +// artifactURIFor returns the SARIF location of the file at path, which must be a URI rather than a +// filesystem path. A relative path stays relative, for the consumer to resolve against the root of +// the analyzed project; an absolute one, which decolint reports for a file outside the directory it +// runs in, becomes an absolute file URI, since resolving it that way would name a different file. +func artifactURIFor(path string) string { + slashed := filepath.ToSlash(path) + if !filepath.IsAbs(path) { + return (&url.URL{Path: slashed}).String() + } + // A UNC path names a host, which a file URI carries as its authority rather than in its path. + if rest, ok := strings.CutPrefix(slashed, "//"); ok { + host, share, _ := strings.Cut(rest, "/") + return (&url.URL{Scheme: "file", Host: host, Path: "/" + share}).String() + } + // A Windows drive path is absolute without a leading "/", which a file URI's path needs. + return (&url.URL{Scheme: "file", Path: "/" + strings.TrimPrefix(slashed, "/")}).String() +} diff --git a/format/sarif_test.go b/format/sarif_test.go new file mode 100644 index 0000000..3276a18 --- /dev/null +++ b/format/sarif_test.go @@ -0,0 +1,133 @@ +package format + +import ( + "errors" + "path/filepath" + "strings" + "testing" +) + +// testSARIFFormat returns a SARIFFormat whose catalog knows only one of the two rules in +// testIssues, so both the known- and unknown-rule descriptor paths are exercised. +func testSARIFFormat() SARIFFormat { + return SARIFFormat{ + Version: "1.2.3", + Rules: []SARIFRule{ + { + ID: "no-image-latest", + Description: "images should be pinned to a specific version", + Category: "reproducibility", + }, + }, + } +} + +func TestSARIFWriteIssues(t *testing.T) { + t.Parallel() + + var sb strings.Builder + if err := testSARIFFormat().WriteIssues(&sb, testIssues()); err != nil { + t.Fatalf("WriteIssues: %v", err) + } + + want := `{"$schema":"https://json.schemastore.org/sarif-2.1.0.json","version":"2.1.0","runs":[` + + `{"tool":{"driver":{"name":"decolint","version":"1.2.3","informationUri":"https://github.com/bare-devcontainer/decolint","rules":[` + + `{"id":"no-image-latest","shortDescription":{"text":"images should be pinned to a specific version"},"properties":{"tags":["reproducibility"]}},` + + `{"id":"some-error-rule"}]}},"results":[` + + `{"ruleId":"no-image-latest","ruleIndex":0,"level":"warning","message":{"text":"image \"ubuntu:latest\" uses the \"latest\" tag; pin a specific version"},` + + `"locations":[{"physicalLocation":{"artifactLocation":{"uri":".devcontainer/devcontainer.json"},"region":{"startLine":4,"startColumn":12}}}]},` + + `{"ruleId":"some-error-rule","ruleIndex":1,"level":"error","message":{"text":"something is broken"},` + + `"locations":[{"physicalLocation":{"artifactLocation":{"uri":".devcontainer/devcontainer.json"},"region":{"startLine":8,"startColumn":3}}}]}]}]}` + + "\n" + if sb.String() != want { + t.Errorf("WriteIssues sarif = %q, want %q", sb.String(), want) + } +} + +// TestSARIFWriteIssues_ArtifactLocation checks how a finding's path becomes a SARIF artifact +// location: a path built with the host's separators stays relative but is written with "/" ones, +// while an absolute path — which decolint reports for a file outside the directory it runs in — is +// written as a file URI. +func TestSARIFWriteIssues_ArtifactLocation(t *testing.T) { + t.Parallel() + + t.Run("relative path", func(t *testing.T) { + t.Parallel() + + issues := testIssues()[:1] + issues[0].Path = filepath.Join(".devcontainer", "go", "devcontainer.json") + + var sb strings.Builder + if err := testSARIFFormat().WriteIssues(&sb, issues); err != nil { + t.Fatalf("WriteIssues: %v", err) + } + + want := `"artifactLocation":{"uri":".devcontainer/go/devcontainer.json"}` + if !strings.Contains(sb.String(), want) { + t.Errorf("WriteIssues sarif = %q, want it to contain %q", sb.String(), want) + } + }) + + t.Run("UNC path", func(t *testing.T) { + t.Parallel() + + // A UNC path names its host in the leading "//" segment, which belongs in the URI's authority + // rather than its path. The path is given in the form filepath.ToSlash yields for it on + // Windows, which is absolute on every host, so this holds off Windows too. + issues := testIssues()[:1] + issues[0].Path = "//server/share/devcontainer.json" + + var sb strings.Builder + if err := testSARIFFormat().WriteIssues(&sb, issues); err != nil { + t.Fatalf("WriteIssues: %v", err) + } + + want := `"artifactLocation":{"uri":"file://server/share/devcontainer.json"}` + if !strings.Contains(sb.String(), want) { + t.Errorf("WriteIssues sarif = %q, want it to contain %q", sb.String(), want) + } + }) + + t.Run("absolute path", func(t *testing.T) { + t.Parallel() + + issues := testIssues()[:1] + issues[0].Path = filepath.Join(t.TempDir(), "devcontainer.json") + + var sb strings.Builder + if err := testSARIFFormat().WriteIssues(&sb, issues); err != nil { + t.Fatalf("WriteIssues: %v", err) + } + + // The URI's own path varies with the host, so only its form is asserted: a file URI with an + // empty authority. + out := sb.String() + if !strings.Contains(out, `"artifactLocation":{"uri":"file:///`) { + t.Errorf("WriteIssues sarif = %q, want an absolute file URI", out) + } + }) +} + +func TestSARIFWriteIssues_Empty(t *testing.T) { + t.Parallel() + + var sb strings.Builder + if err := testSARIFFormat().WriteIssues(&sb, nil); err != nil { + t.Fatalf("WriteIssues: %v", err) + } + + want := `{"$schema":"https://json.schemastore.org/sarif-2.1.0.json","version":"2.1.0","runs":[` + + `{"tool":{"driver":{"name":"decolint","version":"1.2.3","informationUri":"https://github.com/bare-devcontainer/decolint","rules":[]}},"results":[]}]}` + + "\n" + if sb.String() != want { + t.Errorf("WriteIssues sarif (empty) = %q, want %q", sb.String(), want) + } +} + +func TestSARIFWriteIssues_WriteError(t *testing.T) { + t.Parallel() + + if err := testSARIFFormat().WriteIssues(errWriter{}, testIssues()); !errors.Is(err, errWrite) { + t.Errorf("WriteIssues error = %v, want %v", err, errWrite) + } +}