diff --git a/CHANGELOG.md b/CHANGELOG.md index 030bccc..acddf94 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -63,9 +63,29 @@ All notable changes to CryptoScan are recorded here. This project follows out, where it called them quantum-safe; `pqc --help` calls them PARTIAL too. The `--fail-on` help says that, when neither the flag nor `failOn` in the config file is set, the scan exits 1 when any finding is CRITICAL. +- **The release configuration uses the current GoReleaser keys.** + `goreleaser check` (GoReleaser 2.17.1) reported `archives.format`, + `archives.format_overrides.format` and `brews` as deprecated and exited 2. + The archives now set `formats`, and the Homebrew definition is a cask + (`homebrew_casks`), written to `dist/homebrew/Casks/cryptoscan.rb` in place + of the formula in `dist/homebrew/Formula/cryptoscan.rb`. It is still not + uploaded to the tap (`skip_upload: true`). A cask has no `install` or `test` + block, so the formula's `bin.install` and `--version` test are gone; the + cask's `binary "cryptoscan"` stanza installs the binary. `goreleaser check` + now exits 0, and a snapshot build writes the same six archives as before: + tar.gz for Linux and macOS, zip for Windows. ### Fixed +- **Files with the same risk score are listed in the same order on every + run.** `migrationScore.topRiskFiles` in JSON output, and the + `TOP PRIORITY FILES` box of the text report, ordered files by risk score + alone, so files that tied came out in a different order from run to run. + When the tie fell at the fifth place, which files made the list changed + too. With three copies of `crypto-samples` in one directory, six runs of + `cryptoscan scan --format json --no-config` gave six different + lists. Files that tie are now ordered by path. + - **A scan that read no files no longer reports a clean result.** A scan of an empty directory, or of one where every file was left out, printed `Scanned: 0 files • 0 lines • 0 B` and then a check mark with @@ -122,19 +142,17 @@ All notable changes to CryptoScan are recorded here. This project follows `fix` and `remediation` fields of JSON findings; SARIF writes it as the `help.text` of the finding's rule, and CSV in its Remediation column. -### Changed - -- **The release configuration uses the current GoReleaser keys.** - `goreleaser check` (GoReleaser 2.17.1) reported `archives.format`, - `archives.format_overrides.format` and `brews` as deprecated and exited 2. - The archives now set `formats`, and the Homebrew definition is a cask - (`homebrew_casks`), written to `dist/homebrew/Casks/cryptoscan.rb` in place - of the formula in `dist/homebrew/Formula/cryptoscan.rb`. It is still not - uploaded to the tap (`skip_upload: true`). A cask has no `install` or `test` - block, so the formula's `bin.install` and `--version` test are gone; the - cask's `binary "cryptoscan"` stanza installs the binary. `goreleaser check` - now exits 0, and a snapshot build writes the same six archives as before: - tar.gz for Linux and macOS, zip for Windows. +- **The dependency insight lists its libraries in name order.** In + `--format json` output, the `description` of the `dependency_update` + insight names the quantum-vulnerable libraries found in dependency + manifests after `Libraries detected:`. That list followed the iteration + order of a Go map, which changes from run to run, so scans of the same tree + could list the libraries in different orders. Eight scans of a directory + whose `package.json` depends on `node-forge`, `tweetnacl` and `jose` and + whose `requirements.txt` lists `pycryptodome`, `pynacl` and `cryptography` + gave four different orders. The list is now sorted by name, as in + `Libraries detected: cryptography, jose, node-forge, pycryptodome, pynacl, tweetnacl.`, + and the same eight scans give that one order. ## [1.5.0] - 2026-10-18 diff --git a/internal/cli/pqc_test.go b/internal/cli/pqc_test.go index faa8145..208f312 100644 --- a/internal/cli/pqc_test.go +++ b/internal/cli/pqc_test.go @@ -261,26 +261,34 @@ func TestPqcOutputIsDeterministic(t *testing.T) { } } - // The JSON check covers pqcFindings, the part pqc adds. The embedded - // migrationScore is the object scan reports, and its topRiskFiles order - // among files with equal risk scores is not yet fixed from run to run. - var first string + // The JSON check covers pqcFindings, the part pqc adds, and the embedded + // migrationScore, whose topRiskFiles lists files the six fixture files + // tie on. + var firstFindings, firstScore string for n := 0; n < 3; n++ { resetPipelineFlags(t) outputFormat = "json" out, _ := runPqcCapture(t, dir) var view struct { - PQCFindings json.RawMessage `json:"pqcFindings"` + PQCFindings json.RawMessage `json:"pqcFindings"` + MigrationScore json.RawMessage `json:"migrationScore"` } if err := json.Unmarshal([]byte(out), &view); err != nil { t.Fatalf("pqc --format json is not JSON: %v", err) } if n == 0 { - first = string(view.PQCFindings) + firstFindings = string(view.PQCFindings) + firstScore = string(view.MigrationScore) + if firstScore == "" { + t.Fatalf("pqc --format json has no migrationScore:\n%s", out) + } continue } - if string(view.PQCFindings) != first { + if string(view.PQCFindings) != firstFindings { t.Fatalf("pqc --format json: pqcFindings in run %d differs from run 0", n) } + if string(view.MigrationScore) != firstScore { + t.Fatalf("pqc --format json: migrationScore in run %d differs from run 0", n) + } } } diff --git a/pkg/analyzer/scoring.go b/pkg/analyzer/scoring.go index 756d7ad..a7119e3 100644 --- a/pkg/analyzer/scoring.go +++ b/pkg/analyzer/scoring.go @@ -129,8 +129,13 @@ func CalculateMigrationScoreForRoot(findings []types.Finding, root string) *type fileList = append(fileList, *fr) } } + // fileList comes from a map, so files with the same risk score are + // ordered by path to give the same list on every run. sort.Slice(fileList, func(i, j int) bool { - return fileList[i].RiskScore > fileList[j].RiskScore + if fileList[i].RiskScore != fileList[j].RiskScore { + return fileList[i].RiskScore > fileList[j].RiskScore + } + return fileList[i].File < fileList[j].File }) if len(fileList) > 5 { fileList = fileList[:5] diff --git a/pkg/analyzer/scoring_test.go b/pkg/analyzer/scoring_test.go index 5e6ee53..8519baa 100644 --- a/pkg/analyzer/scoring_test.go +++ b/pkg/analyzer/scoring_test.go @@ -101,6 +101,43 @@ func TestCalculateMigrationScoreForRootIgnoresDirectoriesAboveTheTarget(t *testi } } +func TestTopRiskFilesOrdersTiedFilesByPath(t *testing.T) { + // src/high.go has two findings and the seven other files one each, so + // six files tie at the second place and only four of them fit in the + // list of five. + findings := []types.Finding{ + scoringFinding("src/g.go", types.QuantumVulnerable), + scoringFinding("src/c.go", types.QuantumVulnerable), + scoringFinding("src/high.go", types.QuantumVulnerable), + scoringFinding("src/e.go", types.QuantumVulnerable), + scoringFinding("src/a.go", types.QuantumVulnerable), + scoringFinding("src/high.go", types.QuantumVulnerable), + scoringFinding("src/f.go", types.QuantumVulnerable), + scoringFinding("src/b.go", types.QuantumVulnerable), + scoringFinding("src/d.go", types.QuantumVulnerable), + } + want := []string{"src/high.go", "src/a.go", "src/b.go", "src/c.go", "src/d.go"} + + // The files are collected in a map, so one run can match by chance. + for run := 0; run < 20; run++ { + input := make([]types.Finding, len(findings)) + copy(input, findings) + score := CalculateMigrationScore(input) + + var got []string + for _, fr := range score.TopRiskFiles { + got = append(got, fr.File) + } + if strings.Join(got, " ") != strings.Join(want, " ") { + t.Fatalf("run %d: TopRiskFiles = %v, want %v", run, got, want) + } + if score.TopRiskFiles[0].RiskScore != 2*score.TopRiskFiles[1].RiskScore { + t.Fatalf("run %d: RiskScore of %s = %d, want twice the %d of %s", run, + want[0], score.TopRiskFiles[0].RiskScore, score.TopRiskFiles[1].RiskScore, want[1]) + } + } +} + func TestNoQuantumSafeRecommendationRespectsHybridFindings(t *testing.T) { const noSafe = "No quantum-safe algorithms detected" diff --git a/pkg/reporter/changelog_test.go b/pkg/reporter/changelog_test.go index 8476b80..664f4c5 100644 --- a/pkg/reporter/changelog_test.go +++ b/pkg/reporter/changelog_test.go @@ -6,6 +6,7 @@ package reporter import ( "encoding/csv" "encoding/json" + "fmt" "os" "path/filepath" "strings" @@ -15,7 +16,7 @@ import ( ) // changelogFile is the release history. Its entries quote text that the -// reporters write, and the test in this file holds such a quote to the SARIF +// reporters write, and a test in this file holds such a quote to the SARIF // and CSV output of a real scan. const changelogFile = "../../CHANGELOG.md" @@ -80,6 +81,52 @@ func TestChangelogEntries(t *testing.T) { } } +// repeatedChangelogHeadings returns one line for each "### " heading of +// markdown that appears again under the same "## " release, naming the +// heading, the release and the line numbers of both. +func repeatedChangelogHeadings(markdown string) []string { + var repeats []string + var release string + seen := map[string]int{} + for i, line := range strings.Split(markdown, "\n") { + switch { + case strings.HasPrefix(line, "## "): + release = strings.TrimPrefix(line, "## ") + seen = map[string]int{} + case strings.HasPrefix(line, "### "): + if first, ok := seen[line]; ok { + repeats = append(repeats, fmt.Sprintf("%q under %q is at lines %d and %d", line, release, first, i+1)) + } else { + seen[line] = i + 1 + } + } + } + return repeats +} + +func TestRepeatedChangelogHeadings(t *testing.T) { + markdown := "# Changelog\n\n## [Unreleased]\n\n### Changed\n\n- **A.**\n\n### Fixed\n\n- **B.**\n\n" + + "### Changed\n\n- **C.**\n\n## [1.0.0]\n\n### Changed\n\n- **D.**\n" + got := repeatedChangelogHeadings(markdown) + want := []string{`"### Changed" under "[Unreleased]" is at lines 5 and 13`} + if len(got) != len(want) || got[0] != want[0] { + t.Fatalf("got %q, want %q", got, want) + } +} + +// Each release lists one heading per kind of change. The [Unreleased] section +// had a second "### Changed" after "### Fixed", holding an entry that belongs +// under the first one. +func TestChangelogReleasesHaveOneHeadingPerKind(t *testing.T) { + raw, err := os.ReadFile(changelogFile) + if err != nil { + t.Fatal(err) + } + for _, repeat := range repeatedChangelogHeadings(string(raw)) { + t.Errorf("%s: %s; move the entries under the second one to the first", changelogFile, repeat) + } +} + // scanCryptoJSDependency runs the real scanner over a directory whose // package.json lists crypto-js, and returns the ID and help text of the // crypto-js dependency rule in the SARIF output, and the Remediation column diff --git a/pkg/scanner/determinism_test.go b/pkg/scanner/determinism_test.go index 8feee80..05c5e2c 100644 --- a/pkg/scanner/determinism_test.go +++ b/pkg/scanner/determinism_test.go @@ -89,3 +89,46 @@ d = 1 # sha1 stays until the legacy peers migrate } } } + +// TestDependencyInsightListsLibrariesByName covers the "Libraries detected:" +// list of the dependency_update insight. The list was built by ranging over a +// map, and Go randomises map iteration order, so two scans of the same tree +// could list the libraries in different orders. It is sorted by name now. +func TestDependencyInsightListsLibrariesByName(t *testing.T) { + dir := t.TempDir() + write(t, filepath.Join(dir, "package.json"), `{ + "name": "fixture", + "version": "1.0.0", + "dependencies": { + "node-forge": "^1.3.1", + "tweetnacl": "^1.0.3", + "jose": "^5.2.0" + } +} +`) + write(t, filepath.Join(dir, "requirements.txt"), `pycryptodome==3.20.0 +pynacl==1.5.0 +cryptography==42.0.5 +`) + + const want = "Libraries detected: cryptography, jose, node-forge, pycryptodome, pynacl, tweetnacl. " + + "These provide quantum-vulnerable algorithms." + for run := 0; run < 5; run++ { + results, err := New(Config{Target: dir, MinSeverity: types.SeverityInfo}).Scan() + if err != nil { + t.Fatalf("scan: %v", err) + } + var got []string + for _, insight := range results.Insights { + if insight.Type == "dependency_update" { + got = append(got, insight.Description) + } + } + if len(got) != 1 { + t.Fatalf("run %d: want one dependency_update insight, got %d: %q", run, len(got), got) + } + if got[0] != want { + t.Fatalf("run %d: description = %q, want %q", run, got[0], want) + } + } +} diff --git a/pkg/scanner/scanner.go b/pkg/scanner/scanner.go index 5778163..bbb4f1a 100644 --- a/pkg/scanner/scanner.go +++ b/pkg/scanner/scanner.go @@ -1693,6 +1693,9 @@ func (s *Scanner) generateInsights() []Insight { for lib := range libs { libList = append(libList, lib) } + // Go randomises map iteration order, so without this sort two scans + // of the same tree could list the libraries in different orders. + sort.Strings(libList) insights = append(insights, Insight{ Type: "dependency_update", Title: fmt.Sprintf("Crypto Libraries Need PQC Upgrade (%d libraries)", len(libs)),