diff --git a/CHANGELOG.md b/CHANGELOG.md index 88d16ec..0cab54d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,182 @@ All notable changes to QRAMM CryptoDeps will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +Release-blocking defects found by a fresh-user release test on the 1.3.0 +candidate. 1.3.0 was never tagged. + +### Fixed + +- **Every machine-readable output reported the wrong tool version.** One binary + gave four answers: `version` said 1.3.0 while SARIF and CBOM both claimed + 1.0.0 and JSON carried no version at all. SARIF and CBOM are provenance + artifacts, so a stale literal there is a false record of what produced the + document. A new `pkg/version` package is now the single source of truth, fed + once from the values GoReleaser injects into `main`. SARIF also gained + `semanticVersion`, and JSON gained a top-level `tool` object. + +- **A manifest that could not be parsed was dropped silently and the scan still + reported a clean summary.** A tree containing a good and a corrupt + `package.json` scanned only the good one and never mentioned the other, so a + manifest broken by a bad merge became invisible and CI went green. The cause + was not the parse-error path: discovery rejected the file before any parser + ran. Unreadable manifests are now listed by name with the reason in the table, + JSON, markdown and SARIF output, and always exit 2. A tree whose only manifest + is corrupt no longer claims that no manifest was found. + +- **A scan where every dependency was unknown reported "No cryptographic usage + detected".** Nothing had been examined. The three cases (no dependencies, all + dependencies unknown, and dependencies analyzed with no findings) are now + worded differently, and the `--deep` hints that the analyzer had always + generated are finally printed. + +- **`--risk` and `--min-severity` did nothing.** Both were stored and never + read, so every value, including a misspelt one, produced byte-identical + output. They now filter, and the summary and exit code are computed from what + survives so that every number describes the same set of findings. Unknown + values are rejected instead of ignored. When a filter removes every finding, + the report says so rather than reporting a clean scan. + +- **Every SARIF result pointed at a literal path `"multiple"`.** Multi-project + runs flattened all projects into one synthetic result, discarding the real + manifest paths, so every alert landed on a file that does not exist. Results + now carry their own project's manifest, relative to the scan root and declared + through `SRCROOT` in `originalUriBaseIds`. + +- **Output order shuffled between runs of the same scan.** `package.json` + dependency blocks were read by ranging over maps, and findings were sorted on + risk alone with a non-stable sort. The finding set was stable but the order + was not, which breaks golden-file CI and reproducible SBOMs. Discovery, + parsing and rendering are now fully ordered, so ordering is stable across runs + in all five formats. The JSON scan timestamp and the CBOM serial number + necessarily differ between runs; every other byte is identical. + +- **A repository containing an unsupported manifest type always exited 2.** + Discovery recognised `Cargo.toml`, `Gemfile`, `composer.json` and the Gradle + files, but no parser exists for any of them, so each became a reported skip, + and a skip forces exit 2. A tree with a `go.mod` beside a `Cargo.toml` + reported an analysis error instead of the exit 1 its real quantum-vulnerable + findings had earned, so the CI signal the tool exists to emit was replaced by + an error about a file cryptodeps never claimed to read. Such files are now + reported as an unsupported ecosystem rather than an unread manifest, and only + a manifest that should have been readable and was not marks the scan + incomplete. + +- **A filtered scan reported clean in every format except the table.** The + verdict that distinguishes "nothing was found" from "nothing was examined" + from "everything was withheld" reached the table only. Markdown still printed + "No cryptographic usage detected in dependencies.", SARIF asserted + `executionSuccessful` over an empty result set, and CBOM emitted no + components and said nothing, so a consumer of any of them read a clean bill + of health. All five formats now classify through one shared function, SARIF + records coverage as `toolExecutionNotifications`, and CBOM records it as + `metadata.properties`. + +- **The aggregate summary of a workspace scan omitted the withheld count.** + `AggregateResults` summed nine fields and not `filteredOut`, so + `totalSummary.filteredOut` stayed absent while the per-project summaries + reported dozens. The totals a reader actually looks at described a filtered + scan as a complete one. + +- **`--min-severity` discarded findings whose severity was not upper case.** + Severity ranking is keyed by the upper-case constants and a Go map returns + zero for an absent key, so an unrecognised severity ranked as `INFO` and any + higher threshold dropped it, uncounted. Database records arrive from a remote + feed with no normalisation, so a record carrying `critical` was discarded by + the very filter a user reaches for to see critical findings. Ranking is now + case-insensitive, and a severity that cannot be ranked is reported rather + than withheld. + +- **`analyze ` emitted SARIF pointing at nothing.** Passing a + file rather than a directory made the `SRCROOT` base the manifest itself, so + every result resolved to the literal `"."`. The base is now the containing + directory. + +- **The markdown remediation table shuffled between runs.** It was the one + format still ranging over a map after the determinism work, so ten runs of + the same scan produced ten different documents. + +- **The GitHub Action published zero counts and could not upload SARIF.** It + read `.summary` from JSON, but workspace discovery is the default and a + multi-project document carries `.totalSummary`, so `vulnerable-count` was + always 0. Its SARIF step also treated any non-zero exit as a step failure, + which skipped the upload for exactly the incomplete scans most worth + reporting. + +- **A CBOM published the operator's filesystem layout.** Manifest paths were + rendered relative to the scan root so that a shared bill of materials carries + the repository layout and not a home directory or a CI runner's workspace + path, but the comparison used the scan root exactly as it was typed while the + manifest paths had already been absolutized. `cryptodeps analyze /abs/path` + produced relative paths and `cryptodeps analyze .`, which is the default and + what the Action runs, emitted absolute ones. SARIF had always normalized the + root; both formats now share one implementation, so they cannot disagree again. + +- **A scanned repository could write its own lines into the report.** Every + filesystem path reached the table and markdown reports uninterpolated, and a + path is attacker-controlled: a directory named with embedded newlines and the + text `## Scan result: CLEAN` put exactly that heading in the markdown report, + above the corrupt manifest the report exists to disclose. A directory named + `a|b` shifted a column out of the "Not analyzed" table, because + GitHub-flavoured markdown splits a cell on an unescaped pipe even inside a + code span. Every path, skip reason and dependency string in the markdown report is now + rendered inside a code span, where only a backtick and a pipe are active, rather than + escaped character by character: the first attempt at this escaped control + characters and left a bare `##` heading, so a directory named `**CLEAN**` or + `[no findings](https://...)` still rendered as markup. In the plain-text + report, and inside the code spans, anything carrying a control character, a + Unicode line or paragraph separator, a bidi override, a zero-width character, + a backtick or a pipe is rendered as a quoted string: single-line, reversible, + and still naming the file. JSON, CBOM and SARIF were never affected by the + line-injection vector, because `encoding/json` escapes what it emits. + +- **A dependency string could write its own lines into the report too.** The + path fix did not cover the other channel into the same document: a dependency + name and version come from the manifest under scan, and both were interpolated + bare into the markdown findings tables and the table report. It needs no + filesystem access at all, which makes it easier to reach than the directory + name that was fixed first: the database lookup falls back from "name@version" + to the name alone, so a real package with a version of + "1.3.1\n\n## Scan result: CLEAN" still resolved, reached the findings table, + and put that heading in the report seven times. Now rendered through the same + code spans every path uses. Present in 1.2.2 as well; the fix is not a + regression repair. + +- **The table and markdown reports named manifests differently from the CBOM and + SARIF for the same scan.** Only the two machine-readable formats expressed a + manifest relative to the scan root; the table used a string-prefix test that + compared the root as typed against absolutized paths, and markdown did not + relativize at all. Every surface of one run now names a manifest the same way, + with one absolute anchor per document. `getRelativePath` also returned a path + that does not exist when the root was a string prefix of a sibling directory, + so `("/repo", "/repository/go.mod")` gave `./sitory/go.mod`. + +- **`cryptodeps status` reported roughly twice the packages the database holds.** + It announced 1731 packages, and every per-ecosystem number was wrong the same + way, for a database of 901. The index files each package under two keys, + `name@version` and `name`, so a lookup succeeds with or without a version, and + the count was of index entries rather than packages. `status` now reports 901, + matching the database's own stats block and a direct count of its records, and + it lists the ecosystems in a fixed order instead of the order the map happened + to iterate in. + +### Changed + +- `output.PrintSkipped` takes the scan root as its second argument, so it can + name a manifest the same way the rest of the report does. This is a breaking + change to an exported function in an importable package. + +- Coloured emoji in the table output are replaced by the ASCII markers the + section headers already use: `[!]` vulnerable, `[~]` partial, `[OK]` safe, + `[?]` unknown. They need no legend, and unlike the emoji they survive a pipe + into a file, a terminal without an emoji font, and a screen reader. This also + brings the tool in line with the CSNP no-emoji standard. + +- A runtime failure no longer prints the full flag list after the error. The + message that explains the failure was being pushed off the top of the + terminal. Usage is still shown for genuine flag mistakes, where it helps. + ## [1.3.0] - 2026-07-27 Fixes both open community issues, plus a silent false negative found while @@ -85,9 +261,9 @@ reproducing them. ### Changed - **Output formatting**: Clean, professional terminal design with colored status indicators - - 🔴 Vulnerable (quantum-broken by Shor's algorithm) - - 🟡 Partial risk (weakened by Grover's algorithm) - - 🟢 Safe (quantum-resistant) + - Vulnerable (quantum-broken by Shor's algorithm) + - Partial risk (weakened by Grover's algorithm) + - Safe (quantum-resistant) - Improved remediation guidance layout with aligned fields - Call trace formatting now uses `>` prefix for cleaner output diff --git a/LICENSE b/LICENSE index a1d84c0..d1e437e 100644 --- a/LICENSE +++ b/LICENSE @@ -175,7 +175,7 @@ END OF TERMS AND CONDITIONS - Copyright 2024 CSNP (csnp.org) + Copyright 2025-2026 CyberSecurity NonProfit (CSNP) Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. diff --git a/README.md b/README.md index 59a1123..abe8cf3 100644 --- a/README.md +++ b/README.md @@ -174,11 +174,12 @@ cryptodeps analyze /path/to/monorepo --no-workspaces Every finding is classified by quantum computing threat level: -| Symbol | Risk Level | Quantum Threat | Examples | +| Marker | Risk Level | Quantum Threat | Examples | |--------|------------|----------------|----------| -| 🔴 | VULNERABLE | Shor's algorithm | RSA, ECDSA, Ed25519, ECDH, DH, DSA | -| 🟡 | PARTIAL | Grover's algorithm | AES-128, SHA-256, HMAC-SHA256 | -| 🟢 | SAFE | Resistant | AES-256, SHA-384+, ChaCha20, Argon2 | +| `[!]` | VULNERABLE | Shor's algorithm | RSA, ECDSA, Ed25519, ECDH, DH, DSA | +| `[~]` | PARTIAL | Grover's algorithm | AES-128, SHA-256, HMAC-SHA256 | +| `[OK]` | SAFE | Resistant | AES-256, SHA-384+, ChaCha20, Argon2 | +| `[?]` | UNKNOWN | Not classified | Algorithms absent from the database | ### Smart Remediation @@ -227,11 +228,29 @@ Analyze Flags: --deep Force AST analysis for packages not in database --offline Use only local database, skip auto-updates --no-workspaces Disable workspace discovery (scan single manifest only) - --risk string Filter by risk: vulnerable, partial, all - --min-severity string Minimum severity to report + --risk string Report only this risk level: vulnerable, partial, safe, unknown, all + --min-severity string Report only findings at or above: info, low, medium, high, critical -h, --help Show help ``` +`--risk` and `--min-severity` filter the report. The summary and the exit code +are computed from what the filters leave, so every number in the output +describes the same set of findings. When a filter removes everything, the tool +says so rather than reporting a clean scan. + +### Exit codes + +| Code | Meaning | +|------|---------| +| 0 | No quantum-vulnerable findings | +| 1 | Quantum-vulnerable findings detected | +| 2 | Analysis error, including any manifest that was found but could not be read | +| 3 | Partial-risk findings detected (with `--fail-on partial`) | + +A manifest that cannot be parsed is always reported by name, with the reason, +and always exits 2. A scan that silently omitted a dependency file would let a +build pass on a report that never read it. + ### Common Workflows ```bash @@ -262,38 +281,38 @@ cryptodeps status ## Sample Output ``` -[*] Scanning go.mod... found 36 dependencies +[*] Scanning go.mod... found 2 dependencies [!] CONFIRMED - Actually used by your code (requires action): ────────────────────────────────────────────────────────────────────────────────────────── - 🔴 Ed25519 VULNERABLE 1-2yr low + [!] Ed25519 VULNERABLE 1-2yr low └─ golang.org/x/crypto@v0.31.0 > Called from: crypto.GenerateEd25519KeyPair > Called from: crypto.SignMessage - - 🟡 HS256 PARTIAL - low + [~] HS256 PARTIAL - low └─ github.com/golang-jwt/jwt/v5@v5.3.0 > Called from: auth.JWTService.GenerateAccessToken - - 🟢 bcrypt SAFE - - + [OK] bcrypt SAFE - - └─ golang.org/x/crypto@v0.31.0 > Called from: auth.HashPassword [.] AVAILABLE - In dependencies but not called (lower priority): ────────────────────────────────────────────────────────────────────────────────────────── + github.com/golang-jwt/jwt/v5@v5.3.0 + └─ [!] ES256, [!] ES384, [!] ES512, [!] RS256, [!] RS384, [!] RS512, [~] HS384, [OK] HS512 golang.org/x/crypto@v0.31.0 - └─ 🔴 X25519, 🟢 ChaCha20-Poly1305, 🟢 Argon2 + └─ [!] X25519, [OK] Argon2, [OK] ChaCha20-Poly1305 ══════════════════════════════════════════════════════════════════════════════════════════ -SUMMARY: 36 deps | 2 with crypto | 8 vulnerable | 2 partial +SUMMARY: 2 deps | 2 with crypto | 8 vulnerable | 2 partial REACHABILITY: 3 confirmed | 0 reachable | 11 available-only REMEDIATION GUIDANCE: ══════════════════════════════════════════════════════════════════════════════════════════ -🔴 Ed25519 [PRIORITY] +[!] Ed25519 [PRIORITY] ────────────────────────────────────────────────── - Action: Plan migration to ML-DSA; prioritize if signing long-lived data + Action: Plan migration to ML-DSA; prioritize if signing long-lived data or certificates Replace with: ML-DSA-65 (FIPS 204) NIST: FIPS 204 Timeline: Short-term (1-2 years) @@ -439,7 +458,7 @@ qramm-cryptodeps/ │ ├── crypto/ # Algorithm patterns & remediation │ ├── output/ # Formatters (table, JSON, CBOM, SARIF) │ └── types/ # Shared type definitions -├── data/ # Curated crypto database (1,100+ packages) +├── data/ # Crypto database (901 packages) └── examples/ # Sample projects for testing ``` @@ -447,7 +466,7 @@ qramm-cryptodeps/ ## Roadmap -### v1.2 (Current Release) +### v1.2 (Released) - [x] Multi-ecosystem dependency scanning (Go, npm, Python, Maven) - [x] Reachability analysis for Go projects @@ -455,11 +474,24 @@ qramm-cryptodeps/ - [x] Quantum risk classification with CNSA 2.0 timeline - [x] Smart remediation guidance with NIST references - [x] GitHub repository URL scanning -- [x] Curated database of 1,100+ packages +- [x] Crypto database of 901 packages (69 verified, 832 inferred) - [x] Workspace & monorepo support (npm, pnpm, Go workspaces) - [x] Multi-project aggregated results -### v1.3 (Next) +### v1.3 (Next release) + +Correctness of what the tool reports, rather than new surfaces. + +- [x] Every machine-readable output reports the tool version that produced it +- [x] Manifests that cannot be read are named, with the reason, and exit 2 +- [x] Manifests for unsupported ecosystems are reported without failing the scan +- [x] A scan that examined nothing no longer reports a clean result +- [x] `--risk` and `--min-severity` filter, and say how much they withheld +- [x] SARIF results point at the manifest they came from, relative to the root +- [x] Stable ordering across runs of the same scan, in all five formats + (the scan timestamp and the CBOM serial number necessarily differ) + +### v1.4 (Planned) - [ ] Improved reachability for npm/Python projects - [ ] Transitive dependency crypto inheritance diff --git a/action.yml b/action.yml index f8da5e3..b80f217 100644 --- a/action.yml +++ b/action.yml @@ -87,9 +87,14 @@ runs: # Parse summary for outputs (extract from table or JSON) if [ "${{ inputs.format }}" = "json" ]; then - VULNERABLE=$(echo "$OUTPUT" | jq -r '.summary.quantumVulnerable // 0') - PARTIAL=$(echo "$OUTPUT" | jq -r '.summary.quantumPartial // 0') - CRYPTO=$(echo "$OUTPUT" | jq -r '.summary.withCrypto // 0') + # Workspace discovery is the default, and a multi-project document + # carries totalSummary rather than summary. Reading only .summary made + # every published count 0 on the default path, so the action reported + # "0 vulnerable" for trees that were full of findings. + SUMMARY=$(echo "$OUTPUT" | jq -r '(.totalSummary // .summary) // {}') + VULNERABLE=$(echo "$SUMMARY" | jq -r '.quantumVulnerable // 0') + PARTIAL=$(echo "$SUMMARY" | jq -r '.quantumPartial // 0') + CRYPTO=$(echo "$SUMMARY" | jq -r '.withCrypto // 0') else # Default counts if not parseable VULNERABLE=0 @@ -105,16 +110,39 @@ runs: exit $EXIT_CODE - name: Generate SARIF Report - if: inputs.sarif-file != '' + # !cancelled(), not success(): the analysis step above ends with + # `exit $EXIT_CODE` and its default threshold exits 1 on any vulnerable + # finding, so the implicit success() skipped the SARIF steps for every + # repository that had something to report. Not always() either, which + # GitHub documents as running even on cancellation: a cancelled run would + # then still push a report into the Security tab. + if: ${{ !cancelled() && inputs.sarif-file != '' }} shell: bash run: | + # A non-zero exit here must not stop the upload. --fail-on none silences + # the finding-based exit codes, but exit 2 still reports an analysis + # error such as a manifest that could not be read, and that report is + # precisely the one worth sending to the Security tab. Composite bash + # steps run under -e, so without this the step failed and the upload + # below never ran. Only a missing or empty document is a real failure. + set +e cryptodeps analyze "${{ inputs.path }}" \ --format sarif \ --fail-on none \ --offline > "${{ inputs.sarif-file }}" + EXIT_CODE=$? + set -e + + if [ ! -s "${{ inputs.sarif-file }}" ]; then + echo "::error::cryptodeps wrote no SARIF output (exit $EXIT_CODE)" + exit 1 + fi + if [ "$EXIT_CODE" -ne 0 ]; then + echo "::warning::cryptodeps exited $EXIT_CODE. The SARIF report may be incomplete; see runs[].invocations[].toolExecutionNotifications for what was not analyzed." + fi - name: Upload SARIF to GitHub Security - if: inputs.sarif-file != '' + if: ${{ !cancelled() && inputs.sarif-file != '' }} uses: github/codeql-action/upload-sarif@v3 with: sarif_file: ${{ inputs.sarif-file }} diff --git a/cmd/cryptodeps/main.go b/cmd/cryptodeps/main.go index 664cf0e..090b6cf 100644 --- a/cmd/cryptodeps/main.go +++ b/cmd/cryptodeps/main.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 // CryptoDeps analyzes software dependencies for cryptographic usage and quantum vulnerability. @@ -7,6 +7,7 @@ package main import ( "fmt" "os" + "sort" "strings" "github.com/spf13/cobra" @@ -15,22 +16,31 @@ import ( "github.com/csnp/qramm-cryptodeps/internal/database" "github.com/csnp/qramm-cryptodeps/pkg/output" "github.com/csnp/qramm-cryptodeps/pkg/types" + buildinfo "github.com/csnp/qramm-cryptodeps/pkg/version" ) // Exit codes for CI/CD integration const ( - ExitSuccess = 0 // No issues found - ExitVulnerable = 1 // Quantum-vulnerable findings detected - ExitError = 2 // Analysis error occurred - ExitPartial = 3 // Partial-risk findings detected (when --fail-on=partial) + ExitSuccess = 0 // No issues found + ExitVulnerable = 1 // Quantum-vulnerable findings detected + ExitError = 2 // Analysis error occurred + ExitPartial = 3 // Partial-risk findings detected (when --fail-on=partial) ) +// Build identity. GoReleaser injects these via -X main.version and friends, so +// they have to live here under these exact names. They are handed straight to +// pkg/version, which is what the rest of the tool reads. Keep them var, not +// const: -X is silently ignored on a const and the build still succeeds. var ( version = "dev" commit = "none" date = "unknown" ) +func init() { + buildinfo.Set(version, commit, date) +} + // CLI flags var ( formatFlag string @@ -47,8 +57,10 @@ var ( ) func main() { + // Cobra has already written the error to stderr, and for a flag mistake it + // writes the message before the usage block, which is the order a reader + // needs. Printing it again here would duplicate every message. if err := rootCmd.Execute(); err != nil { - fmt.Fprintln(os.Stderr, err) os.Exit(ExitError) } // Exit with appropriate code for CI/CD @@ -77,9 +89,11 @@ var versionCmd = &cobra.Command{ Use: "version", Short: "Print version information", Run: func(cmd *cobra.Command, args []string) { - fmt.Printf("cryptodeps %s\n", version) - fmt.Printf(" commit: %s\n", commit) - fmt.Printf(" built: %s\n", date) + // Read through pkg/version rather than the locals, so that this command + // and every machine-readable emitter provably agree. + fmt.Printf("%s %s\n", buildinfo.Name, buildinfo.Version()) + fmt.Printf(" commit: %s\n", buildinfo.Commit()) + fmt.Printf(" built: %s\n", buildinfo.Date()) }, } @@ -99,9 +113,13 @@ The path can be: Exit codes (for CI/CD): 0 - No quantum-vulnerable findings 1 - Quantum-vulnerable findings detected - 2 - Analysis error + 2 - Analysis error, including any manifest that was found but could not be read 3 - Partial-risk findings detected (with --fail-on=partial) +--risk and --min-severity filter the report. The summary and the exit code are +computed from what the filters leave, so that every number in the output +describes the same set of findings. + Examples: cryptodeps analyze . cryptodeps analyze ./go.mod @@ -139,8 +157,8 @@ func init() { analyzeCmd.Flags().BoolVar(&deepFlag, "deep", false, "Force on-demand analysis for unknown packages") analyzeCmd.Flags().BoolVar(&reachabilityFlag, "reachability", true, "Analyze call graph to find actually-used crypto (Go only, use --reachability=false to disable)") analyzeCmd.Flags().BoolVar(&noWorkspacesFlag, "no-workspaces", false, "Disable workspace/monorepo discovery (scan single manifest only)") - analyzeCmd.Flags().StringVar(&riskFilter, "risk", "", "Filter by risk level (vulnerable, partial, all)") - analyzeCmd.Flags().StringVar(&minSeverity, "min-severity", "", "Minimum severity to report") + analyzeCmd.Flags().StringVar(&riskFilter, "risk", "", "Report only this risk level (vulnerable, partial, safe, unknown, all)") + analyzeCmd.Flags().StringVar(&minSeverity, "min-severity", "", "Report only findings at or above this severity (info, low, medium, high, critical)") analyzeCmd.Flags().StringVar(&failOn, "fail-on", "vulnerable", "Exit non-zero when risk found (vulnerable, partial, any, none)") // Update command flags @@ -155,6 +173,12 @@ func init() { } func runAnalyze(cmd *cobra.Command, args []string) error { + // From here on, a failure is a runtime failure rather than a usage mistake, + // so do not follow it with the whole flag list. Setting this inside RunE + // rather than on the command keeps usage where it helps: an unknown flag is + // rejected during parsing, before this line runs. + cmd.SilenceUsage = true + // Default to current directory path := "." if len(args) > 0 { @@ -178,6 +202,15 @@ func runAnalyze(cmd *cobra.Command, args []string) error { path = tempDir } + // Reject unknown filter values before doing any work. Silently ignoring + // them produced an unfiltered report that the user believed was filtered. + if err := analyzer.ValidateRiskFilter(riskFilter); err != nil { + return err + } + if err := analyzer.ValidateMinSeverity(minSeverity); err != nil { + return err + } + // Parse output format format, err := output.ParseFormat(formatFlag) if err != nil { @@ -228,6 +261,19 @@ func runAnalyze(cmd *cobra.Command, args []string) error { } exitCode = determineExitCodeMulti(multiResult, failOn) + + // An unread manifest means the scan is incomplete, whatever the + // findings say. That is an analysis error, so it takes precedence over + // the finding-based codes and over --fail-on none: a build must not go + // green on a report that silently omits a dependency file. + // + // A manifest for an ecosystem cryptodeps has no parser for is reported + // but does not count. It is not a failure to read something; it is a + // declared limit of the tool, and treating it as an error made every + // repository holding a Cargo.toml or a build.gradle exit 2. + if types.IncompleteScan(multiResult.Skipped) { + exitCode = ExitError + } } return nil @@ -327,8 +373,16 @@ func runStatus(cmd *cobra.Command, args []string) error { fmt.Printf("Total packages: %d\n", stats.TotalPackages) fmt.Println() fmt.Println("By ecosystem:") - for ecosystem, count := range stats.ByEcosystem { - fmt.Printf(" %-8s %d packages\n", ecosystem+":", count) + // Sorted. Ranging over the map printed the ecosystems in a different order + // on almost every run, which is the same nondeterminism the five output + // formats were fixed for: it defeats diffing two runs and any golden file. + ecosystems := make([]string, 0, len(stats.ByEcosystem)) + for ecosystem := range stats.ByEcosystem { + ecosystems = append(ecosystems, string(ecosystem)) + } + sort.Strings(ecosystems) + for _, ecosystem := range ecosystems { + fmt.Printf(" %-8s %d packages\n", ecosystem+":", stats.ByEcosystem[types.Ecosystem(ecosystem)]) } fmt.Println() diff --git a/cmd/gendb/main.go b/cmd/gendb/main.go index 42cb9d3..fc1a77d 100644 --- a/cmd/gendb/main.go +++ b/cmd/gendb/main.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 // Command gendb generates a JSON database file from the embedded database @@ -34,11 +34,11 @@ type DatabaseExport struct { // DatabaseStats contains statistics about the database. type DatabaseStats struct { - TotalPackages int `json:"totalPackages"` - VerifiedPackages int `json:"verifiedPackages"` - InferredPackages int `json:"inferredPackages"` - ByEcosystem map[types.Ecosystem]int `json:"byEcosystem"` - ByConfidence map[types.Confidence]int `json:"byConfidence"` + TotalPackages int `json:"totalPackages"` + VerifiedPackages int `json:"verifiedPackages"` + InferredPackages int `json:"inferredPackages"` + ByEcosystem map[types.Ecosystem]int `json:"byEcosystem"` + ByConfidence map[types.Confidence]int `json:"byConfidence"` } var ( @@ -81,8 +81,15 @@ func main() { fmt.Fprintf(os.Stderr, " Verified: %d\n", stats.VerifiedPackages) fmt.Fprintf(os.Stderr, " Inferred: %d\n", stats.InferredPackages) fmt.Fprintf(os.Stderr, " By ecosystem:\n") - for eco, count := range stats.ByEcosystem { - fmt.Fprintf(os.Stderr, " %s: %d\n", eco, count) + // Sorted, for the same reason the status command is: ranging over the map + // printed the ecosystems in a different order on almost every run. + ecosystems := make([]string, 0, len(stats.ByEcosystem)) + for eco := range stats.ByEcosystem { + ecosystems = append(ecosystems, string(eco)) + } + sort.Strings(ecosystems) + for _, eco := range ecosystems { + fmt.Fprintf(os.Stderr, " %s: %d\n", eco, stats.ByEcosystem[types.Ecosystem(eco)]) } // Marshal and output to stdout diff --git a/examples/vulnerable-demo/main.go b/examples/vulnerable-demo/main.go index 1f2daf1..454f5d9 100644 --- a/examples/vulnerable-demo/main.go +++ b/examples/vulnerable-demo/main.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 // Package main demonstrates a typical application with mixed quantum vulnerability levels. diff --git a/internal/analyzer/analyzer.go b/internal/analyzer/analyzer.go index b0375d0..1fc1ceb 100644 --- a/internal/analyzer/analyzer.go +++ b/internal/analyzer/analyzer.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 // Package analyzer provides the core dependency analysis functionality. @@ -8,6 +8,7 @@ import ( "fmt" "os" "path/filepath" + "strings" "time" "github.com/csnp/qramm-cryptodeps/internal/analyzer/ondemand" @@ -29,8 +30,87 @@ type Options struct { Offline bool // Only use database, no on-demand analysis Deep bool // Force on-demand analysis for all packages Reachability bool // Perform reachability analysis to determine actual crypto usage - RiskFilter string // Filter by risk level (vulnerable, partial, all) - MinSeverity string // Minimum severity to report + RiskFilter string // Filter by risk level (vulnerable, partial, safe, unknown, all) + MinSeverity string // Minimum severity to report (info, low, medium, high, critical) +} + +// Risk filter values accepted by --risk. +const ( + RiskFilterAll = "all" +) + +// severityRank orders severities so that --min-severity can act as a threshold. +var severityRank = map[types.Severity]int{ + types.SeverityInfo: 0, + types.SeverityLow: 1, + types.SeverityMedium: 2, + types.SeverityHigh: 3, + types.SeverityCritical: 4, +} + +// ValidateRiskFilter checks a --risk value. An unrecognised value used to be +// accepted and then ignored, so a typo produced a full report that the user +// believed was filtered. +func ValidateRiskFilter(s string) error { + switch strings.ToLower(strings.TrimSpace(s)) { + case "", RiskFilterAll, + strings.ToLower(string(types.RiskVulnerable)), + strings.ToLower(string(types.RiskPartial)), + strings.ToLower(string(types.RiskSafe)), + strings.ToLower(string(types.RiskUnknown)): + return nil + default: + return fmt.Errorf("invalid --risk value %q: expected one of vulnerable, partial, safe, unknown, all", s) + } +} + +// ValidateMinSeverity checks a --min-severity value. +func ValidateMinSeverity(s string) error { + if strings.TrimSpace(s) == "" { + return nil + } + if _, ok := severityRank[types.Severity(strings.ToUpper(strings.TrimSpace(s)))]; ok { + return nil + } + return fmt.Errorf("invalid --min-severity value %q: expected one of info, low, medium, high, critical", s) +} + +// keepCrypto reports whether a finding survives the configured filters. +func (a *Analyzer) keepCrypto(c types.CryptoUsage) bool { + risk := strings.ToLower(strings.TrimSpace(a.options.RiskFilter)) + if risk != "" && risk != RiskFilterAll { + if !strings.EqualFold(string(c.QuantumRisk), risk) { + return false + } + } + + min := strings.ToUpper(strings.TrimSpace(a.options.MinSeverity)) + if min != "" { + threshold, ok := severityRank[types.Severity(min)] + // A severity this build does not recognise is reported, not withheld. + // Ranking an unmapped key gave 0, which is INFO, so any threshold above + // INFO silently dropped it. Database records arrive from a remote feed + // and are unmarshalled without normalising case, so a record carrying + // "critical" rather than "CRITICAL" was discarded by the very filter a + // user reaches for to see critical findings. A finding whose severity + // cannot be ranked has not been shown to be below the threshold. + if rank, known := severityRank[normalizeSeverity(c.Severity)]; ok && known && rank < threshold { + return false + } + } + + return true +} + +// normalizeSeverity puts a severity into the case severityRank is keyed by. +func normalizeSeverity(s types.Severity) types.Severity { + return types.Severity(strings.ToUpper(strings.TrimSpace(string(s)))) +} + +// filtersActive reports whether any reporting filter is set. +func (a *Analyzer) filtersActive() bool { + risk := strings.ToLower(strings.TrimSpace(a.options.RiskFilter)) + return (risk != "" && risk != RiskFilterAll) || strings.TrimSpace(a.options.MinSeverity) != "" } // New creates a new analyzer with the given database and options. @@ -60,9 +140,12 @@ func (a *Analyzer) Analyze(path string) (*types.ScanResult, error) { } // AnalyzeAll discovers and analyzes all manifests in a directory (including workspaces). +// +// Manifests that could not be read are carried on the result rather than +// dropped, so that the report can say what was not looked at. func (a *Analyzer) AnalyzeAll(path string) (*types.MultiProjectResult, error) { // Discover and parse all manifests - manifests, err := manifest.DetectAndParseAll(path) + manifests, skipped, err := manifest.DetectAndParseAll(path) if err != nil { return nil, err } @@ -71,17 +154,22 @@ func (a *Analyzer) AnalyzeAll(path string) (*types.MultiProjectResult, error) { for _, m := range manifests { result, err := a.analyzeManifest(m, filepath.Dir(m.Path)) if err != nil { - fmt.Fprintf(os.Stderr, "Warning: failed to analyze %s: %v\n", m.Path, err) + skipped = append(skipped, types.SkippedManifest{Path: m.Path, Reason: err.Error()}) continue } results = append(results, result) } if len(results) == 0 { + if len(skipped) > 0 { + return nil, fmt.Errorf("no manifests could be analyzed: %d found, all unreadable", len(skipped)) + } return nil, fmt.Errorf("no manifests could be analyzed") } - return types.AggregateResults(path, results), nil + multi := types.AggregateResults(path, results) + multi.Skipped = skipped + return multi, nil } // analyzeManifest analyzes a single parsed manifest. @@ -98,6 +186,29 @@ func (a *Analyzer) analyzeManifest(m *manifest.Manifest, projectPath string) (*t // Analyze each dependency for _, dep := range m.Dependencies { depResult := a.analyzeDependency(dep) + + // Apply the reporting filters before the summary is accumulated, so + // that the counts, the table and the exit code all describe the same + // set of findings. --risk and --min-severity were previously stored and + // never read, so every value including a misspelt one produced the full + // report. + if a.filtersActive() && depResult.Analysis != nil { + kept := make([]types.CryptoUsage, 0, len(depResult.Analysis.Crypto)) + for _, c := range depResult.Analysis.Crypto { + if a.keepCrypto(c) { + kept = append(kept, c) + } else { + result.Summary.FilteredOut++ + } + } + // Copy before mutating: Analysis points into the shared database, + // so writing through it would corrupt the entry for every other + // dependency that resolves to the same package. + filtered := *depResult.Analysis + filtered.Crypto = kept + depResult.Analysis = &filtered + } + result.Dependencies = append(result.Dependencies, depResult) // Update summary diff --git a/internal/analyzer/analyzer_test.go b/internal/analyzer/analyzer_test.go index 3320c1d..482cf4d 100644 --- a/internal/analyzer/analyzer_test.go +++ b/internal/analyzer/analyzer_test.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 package analyzer diff --git a/internal/analyzer/ast/go.go b/internal/analyzer/ast/go.go index ece26c3..35549ed 100644 --- a/internal/analyzer/ast/go.go +++ b/internal/analyzer/ast/go.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 // Package ast provides AST-based analysis for crypto detection. @@ -112,9 +112,9 @@ type cryptoVisitor struct { // funcContext tracks a function context during AST traversal. type funcContext struct { - Name string // function or method name - Receiver string // receiver type for methods (empty for functions) - IsExported bool // whether the function is exported (uppercase first letter) + Name string // function or method name + Receiver string // receiver type for methods (empty for functions) + IsExported bool // whether the function is exported (uppercase first letter) EndPos token.Pos // position where function ends } diff --git a/internal/analyzer/ast/go_test.go b/internal/analyzer/ast/go_test.go index c6b0a8e..5aa42e5 100644 --- a/internal/analyzer/ast/go_test.go +++ b/internal/analyzer/ast/go_test.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 package ast diff --git a/internal/analyzer/ast/java.go b/internal/analyzer/ast/java.go index 5a5d97a..2fe9816 100644 --- a/internal/analyzer/ast/java.go +++ b/internal/analyzer/ast/java.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 // Package ast provides AST-based analysis for crypto detection. @@ -29,24 +29,24 @@ var ( javaImportPattern = regexp.MustCompile(`^\s*import\s+([a-zA-Z0-9_.]+(?:\.\*)?)\s*;`) // JCA/JCE patterns (Java Cryptography Architecture) - with string literals - cipherPattern = regexp.MustCompile(`Cipher\.getInstance\s*\(\s*["']([^"']+)["']`) - messageDigestPattern = regexp.MustCompile(`MessageDigest\.getInstance\s*\(\s*["']([^"']+)["']`) - keyGeneratorPattern = regexp.MustCompile(`KeyGenerator\.getInstance\s*\(\s*["']([^"']+)["']`) - keyPairGenPattern = regexp.MustCompile(`KeyPairGenerator\.getInstance\s*\(\s*["']([^"']+)["']`) - signaturePattern = regexp.MustCompile(`Signature\.getInstance\s*\(\s*["']([^"']+)["']`) - macPattern = regexp.MustCompile(`Mac\.getInstance\s*\(\s*["']([^"']+)["']`) - keyFactoryPattern = regexp.MustCompile(`KeyFactory\.getInstance\s*\(\s*["']([^"']+)["']`) - keyAgreementPattern = regexp.MustCompile(`KeyAgreement\.getInstance\s*\(\s*["']([^"']+)["']`) + cipherPattern = regexp.MustCompile(`Cipher\.getInstance\s*\(\s*["']([^"']+)["']`) + messageDigestPattern = regexp.MustCompile(`MessageDigest\.getInstance\s*\(\s*["']([^"']+)["']`) + keyGeneratorPattern = regexp.MustCompile(`KeyGenerator\.getInstance\s*\(\s*["']([^"']+)["']`) + keyPairGenPattern = regexp.MustCompile(`KeyPairGenerator\.getInstance\s*\(\s*["']([^"']+)["']`) + signaturePattern = regexp.MustCompile(`Signature\.getInstance\s*\(\s*["']([^"']+)["']`) + macPattern = regexp.MustCompile(`Mac\.getInstance\s*\(\s*["']([^"']+)["']`) + keyFactoryPattern = regexp.MustCompile(`KeyFactory\.getInstance\s*\(\s*["']([^"']+)["']`) + keyAgreementPattern = regexp.MustCompile(`KeyAgreement\.getInstance\s*\(\s*["']([^"']+)["']`) secretKeyFactoryPattern = regexp.MustCompile(`SecretKeyFactory\.getInstance\s*\(\s*["']([^"']+)["']`) // JCA/JCE patterns - variable-based (detects crypto API usage without literal) - cipherVarPattern = regexp.MustCompile(`(?:javax\.crypto\.)?Cipher\.getInstance\s*\(`) + cipherVarPattern = regexp.MustCompile(`(?:javax\.crypto\.)?Cipher\.getInstance\s*\(`) messageDigestVarPattern = regexp.MustCompile(`(?:java\.security\.)?MessageDigest\.getInstance\s*\(`) - keyGenVarPattern = regexp.MustCompile(`(?:javax\.crypto\.)?KeyGenerator\.getInstance\s*\(`) - keyPairGenVarPattern = regexp.MustCompile(`KeyPairGenerator\.getInstance\s*\(`) - signatureVarPattern = regexp.MustCompile(`(?:java\.security\.)?Signature\.getInstance\s*\(`) - macVarPattern = regexp.MustCompile(`(?:javax\.crypto\.)?Mac\.getInstance\s*\(`) - secureRandomPattern = regexp.MustCompile(`SecureRandom\.getInstance\s*\(`) + keyGenVarPattern = regexp.MustCompile(`(?:javax\.crypto\.)?KeyGenerator\.getInstance\s*\(`) + keyPairGenVarPattern = regexp.MustCompile(`KeyPairGenerator\.getInstance\s*\(`) + signatureVarPattern = regexp.MustCompile(`(?:java\.security\.)?Signature\.getInstance\s*\(`) + macVarPattern = regexp.MustCompile(`(?:javax\.crypto\.)?Mac\.getInstance\s*\(`) + secureRandomPattern = regexp.MustCompile(`SecureRandom\.getInstance\s*\(`) // SecretKeySpec usage (indicates symmetric encryption) secretKeySpecPattern = regexp.MustCompile(`new\s+SecretKeySpec\s*\(`) @@ -68,46 +68,46 @@ var ( // Java crypto algorithm mappings var javaCryptoAlgorithms = map[string]string{ // Ciphers - "AES": "AES", - "AES/CBC/PKCS5Padding": "AES", - "AES/GCM/NoPadding": "AES-GCM", - "AES/ECB/PKCS5Padding": "AES-ECB", - "AES/CTR/NoPadding": "AES", - "DES": "DES", - "DES/CBC/PKCS5Padding": "DES", - "DESede": "3DES", - "DESede/CBC/PKCS5Padding": "3DES", - "RSA": "RSA", - "RSA/ECB/PKCS1Padding": "RSA", + "AES": "AES", + "AES/CBC/PKCS5Padding": "AES", + "AES/GCM/NoPadding": "AES-GCM", + "AES/ECB/PKCS5Padding": "AES-ECB", + "AES/CTR/NoPadding": "AES", + "DES": "DES", + "DES/CBC/PKCS5Padding": "DES", + "DESede": "3DES", + "DESede/CBC/PKCS5Padding": "3DES", + "RSA": "RSA", + "RSA/ECB/PKCS1Padding": "RSA", "RSA/ECB/OAEPWithSHA-256AndMGF1Padding": "RSA-OAEP", - "Blowfish": "Blowfish", - "RC4": "RC4", - "ChaCha20": "ChaCha20", - "ChaCha20-Poly1305": "ChaCha20-Poly1305", + "Blowfish": "Blowfish", + "RC4": "RC4", + "ChaCha20": "ChaCha20", + "ChaCha20-Poly1305": "ChaCha20-Poly1305", // Message Digests - "MD5": "MD5", - "SHA-1": "SHA-1", - "SHA1": "SHA-1", - "SHA-256": "SHA-256", - "SHA256": "SHA-256", - "SHA-384": "SHA-384", - "SHA384": "SHA-384", - "SHA-512": "SHA-512", - "SHA512": "SHA-512", - "SHA3-256": "SHA3-256", - "SHA3-512": "SHA3-512", + "MD5": "MD5", + "SHA-1": "SHA-1", + "SHA1": "SHA-1", + "SHA-256": "SHA-256", + "SHA256": "SHA-256", + "SHA-384": "SHA-384", + "SHA384": "SHA-384", + "SHA-512": "SHA-512", + "SHA512": "SHA-512", + "SHA3-256": "SHA3-256", + "SHA3-512": "SHA3-512", // Signatures - "SHA256withRSA": "RSA", - "SHA384withRSA": "RSA", - "SHA512withRSA": "RSA", - "SHA256withECDSA": "ECDSA", - "SHA384withECDSA": "ECDSA", - "SHA512withECDSA": "ECDSA", - "SHA256withDSA": "DSA", - "Ed25519": "Ed25519", - "Ed448": "Ed448", + "SHA256withRSA": "RSA", + "SHA384withRSA": "RSA", + "SHA512withRSA": "RSA", + "SHA256withECDSA": "ECDSA", + "SHA384withECDSA": "ECDSA", + "SHA512withECDSA": "ECDSA", + "SHA256withDSA": "DSA", + "Ed25519": "Ed25519", + "Ed448": "Ed448", // MACs "HmacMD5": "HMAC-MD5", @@ -117,15 +117,15 @@ var javaCryptoAlgorithms = map[string]string{ "HmacSHA512": "HMAC-SHA512", // Key Agreement - "DH": "DH", - "ECDH": "ECDH", - "X25519": "X25519", - "X448": "X448", - "XDH": "X25519", + "DH": "DH", + "ECDH": "ECDH", + "X25519": "X25519", + "X448": "X448", + "XDH": "X25519", // Key Factories - "EC": "ECDSA", - "DSA": "DSA", + "EC": "ECDSA", + "DSA": "DSA", // Password-based "PBKDF2WithHmacSHA256": "PBKDF2", @@ -136,9 +136,9 @@ var javaCryptoAlgorithms = map[string]string{ // javaFuncContext tracks function context during Java file analysis. type javaFuncContext struct { - Name string - ClassName string - IsPublic bool + Name string + ClassName string + IsPublic bool BraceDepth int } @@ -306,7 +306,7 @@ func (a *JavaAnalyzer) checkCryptoUsageWithContext(line string, lineNum int, fil // Check JCA/JCE patterns with string literals patterns := []struct { - pattern *regexp.Regexp + pattern *regexp.Regexp cryptoType string }{ {cipherPattern, "encryption"}, @@ -338,14 +338,14 @@ func (a *JavaAnalyzer) checkCryptoUsageWithContext(line string, lineNum int, fil cryptoType string fallback string }{ - {cipherVarPattern, "encryption", "AES"}, // Most common cipher - {messageDigestVarPattern, "hash", "SHA-256"}, // Most common hash - {keyGenVarPattern, "key-generation", "AES"}, // Most common symmetric key - {keyPairGenVarPattern, "key-generation", "RSA"}, // Most common asymmetric - {signatureVarPattern, "signature", "RSA"}, // Most common signature - {macVarPattern, "mac", "HMAC"}, // HMAC is the standard - {secureRandomPattern, "random", "SecureRandom"}, // Track secure random usage - {secretKeySpecPattern, "encryption", "AES"}, // Most common symmetric + {cipherVarPattern, "encryption", "AES"}, // Most common cipher + {messageDigestVarPattern, "hash", "SHA-256"}, // Most common hash + {keyGenVarPattern, "key-generation", "AES"}, // Most common symmetric key + {keyPairGenVarPattern, "key-generation", "RSA"}, // Most common asymmetric + {signatureVarPattern, "signature", "RSA"}, // Most common signature + {macVarPattern, "mac", "HMAC"}, // HMAC is the standard + {secureRandomPattern, "random", "SecureRandom"}, // Track secure random usage + {secretKeySpecPattern, "encryption", "AES"}, // Most common symmetric } for _, p := range varPatterns { diff --git a/internal/analyzer/ast/java_test.go b/internal/analyzer/ast/java_test.go index 74c3e8f..6555dcf 100644 --- a/internal/analyzer/ast/java_test.go +++ b/internal/analyzer/ast/java_test.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 package ast diff --git a/internal/analyzer/ast/javascript.go b/internal/analyzer/ast/javascript.go index 1a26d70..b7c27b0 100644 --- a/internal/analyzer/ast/javascript.go +++ b/internal/analyzer/ast/javascript.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 // Package ast provides AST-based analysis for crypto detection. @@ -26,25 +26,25 @@ func NewJavaScriptAnalyzer() *JavaScriptAnalyzer { // Common patterns for detecting crypto in JavaScript var ( // Import patterns - requirePattern = regexp.MustCompile(`require\s*\(\s*['"]([^'"]+)['"]\s*\)`) - importPattern = regexp.MustCompile(`import\s+.*?\s+from\s+['"]([^'"]+)['"]`) - importDynPattern = regexp.MustCompile(`import\s*\(\s*['"]([^'"]+)['"]\s*\)`) + requirePattern = regexp.MustCompile(`require\s*\(\s*['"]([^'"]+)['"]\s*\)`) + importPattern = regexp.MustCompile(`import\s+.*?\s+from\s+['"]([^'"]+)['"]`) + importDynPattern = regexp.MustCompile(`import\s*\(\s*['"]([^'"]+)['"]\s*\)`) // Function detection patterns - jsFuncDeclPattern = regexp.MustCompile(`(?:async\s+)?function\s+(\w+)\s*\(`) - jsArrowFuncPattern = regexp.MustCompile(`(?:const|let|var)\s+(\w+)\s*=\s*(?:async\s*)?\([^)]*\)\s*=>`) + jsFuncDeclPattern = regexp.MustCompile(`(?:async\s+)?function\s+(\w+)\s*\(`) + jsArrowFuncPattern = regexp.MustCompile(`(?:const|let|var)\s+(\w+)\s*=\s*(?:async\s*)?\([^)]*\)\s*=>`) jsClassMethodPattern = regexp.MustCompile(`(?:async\s+)?(\w+)\s*\([^)]*\)\s*{`) - jsExportPattern = regexp.MustCompile(`^(?:export\s+(?:default\s+)?(?:async\s+)?(?:function|class|const|let|var)\s+(\w+)|module\.exports\s*[.=]|exports\.(\w+))`) - jsClassPattern = regexp.MustCompile(`class\s+(\w+)`) + jsExportPattern = regexp.MustCompile(`^(?:export\s+(?:default\s+)?(?:async\s+)?(?:function|class|const|let|var)\s+(\w+)|module\.exports\s*[.=]|exports\.(\w+))`) + jsClassPattern = regexp.MustCompile(`class\s+(\w+)`) // Crypto function call patterns cryptoMethodPattern = regexp.MustCompile(`crypto\.(createCipher|createDecipher|createCipheriv|createDecipheriv|createSign|createVerify|createHash|createHmac|generateKeyPair|generateKeyPairSync|randomBytes|scrypt|scryptSync|pbkdf2|pbkdf2Sync)\s*\(`) // Common crypto library function patterns - bcryptPattern = regexp.MustCompile(`bcrypt\.(hash|compare|genSalt|hashSync|compareSync|genSaltSync)\s*\(`) - jwtPattern = regexp.MustCompile(`(jwt|jsonwebtoken)\.(sign|verify|decode)\s*\(`) - cryptoJSPattern = regexp.MustCompile(`CryptoJS\.(AES|DES|TripleDES|Rabbit|RC4|MD5|SHA1|SHA256|SHA512|SHA3|RIPEMD160|HmacMD5|HmacSHA1|HmacSHA256|HmacSHA512)\.(encrypt|decrypt|hash)\s*\(`) - nodeForgePattern = regexp.MustCompile(`forge\.(pki|cipher|md|hmac|random|util)\.(rsa|aes|des|md5|sha1|sha256|sha512)`) + bcryptPattern = regexp.MustCompile(`bcrypt\.(hash|compare|genSalt|hashSync|compareSync|genSaltSync)\s*\(`) + jwtPattern = regexp.MustCompile(`(jwt|jsonwebtoken)\.(sign|verify|decode)\s*\(`) + cryptoJSPattern = regexp.MustCompile(`CryptoJS\.(AES|DES|TripleDES|Rabbit|RC4|MD5|SHA1|SHA256|SHA512|SHA3|RIPEMD160|HmacMD5|HmacSHA1|HmacSHA256|HmacSHA512)\.(encrypt|decrypt|hash)\s*\(`) + nodeForgePattern = regexp.MustCompile(`forge\.(pki|cipher|md|hmac|random|util)\.(rsa|aes|des|md5|sha1|sha256|sha512)`) // Algorithm string patterns in crypto calls algStringPattern = regexp.MustCompile(`['"]([a-zA-Z0-9-]+)['"]`) @@ -52,46 +52,46 @@ var ( // Known crypto algorithms in JavaScript var jsAlgorithmMap = map[string]string{ - "aes-128-cbc": "AES-128", - "aes-192-cbc": "AES-192", - "aes-256-cbc": "AES-256", - "aes-128-gcm": "AES-128-GCM", - "aes-256-gcm": "AES-256-GCM", - "aes-128-ctr": "AES-128", - "aes-256-ctr": "AES-256", - "des": "DES", - "des-ede3": "3DES", - "des-ede3-cbc": "3DES", - "rc4": "RC4", - "md5": "MD5", - "sha1": "SHA-1", - "sha256": "SHA-256", - "sha384": "SHA-384", - "sha512": "SHA-512", - "sha3-256": "SHA3-256", - "sha3-512": "SHA3-512", - "rsa": "RSA", - "rsa-sha256": "RSA", - "ecdsa": "ECDSA", - "ed25519": "Ed25519", - "x25519": "X25519", - "curve25519": "X25519", - "secp256k1": "ECDSA", - "prime256v1": "P-256", - "secp384r1": "P-384", - "secp521r1": "P-521", - "hs256": "HMAC-SHA256", - "hs384": "HMAC-SHA384", - "hs512": "HMAC-SHA512", - "rs256": "RS256", - "rs384": "RS384", - "rs512": "RS512", - "es256": "ES256", - "es384": "ES384", - "es512": "ES512", - "ps256": "PS256", - "ps384": "PS384", - "ps512": "PS512", + "aes-128-cbc": "AES-128", + "aes-192-cbc": "AES-192", + "aes-256-cbc": "AES-256", + "aes-128-gcm": "AES-128-GCM", + "aes-256-gcm": "AES-256-GCM", + "aes-128-ctr": "AES-128", + "aes-256-ctr": "AES-256", + "des": "DES", + "des-ede3": "3DES", + "des-ede3-cbc": "3DES", + "rc4": "RC4", + "md5": "MD5", + "sha1": "SHA-1", + "sha256": "SHA-256", + "sha384": "SHA-384", + "sha512": "SHA-512", + "sha3-256": "SHA3-256", + "sha3-512": "SHA3-512", + "rsa": "RSA", + "rsa-sha256": "RSA", + "ecdsa": "ECDSA", + "ed25519": "Ed25519", + "x25519": "X25519", + "curve25519": "X25519", + "secp256k1": "ECDSA", + "prime256v1": "P-256", + "secp384r1": "P-384", + "secp521r1": "P-521", + "hs256": "HMAC-SHA256", + "hs384": "HMAC-SHA384", + "hs512": "HMAC-SHA512", + "rs256": "RS256", + "rs384": "RS384", + "rs512": "RS512", + "es256": "ES256", + "es384": "ES384", + "es512": "ES512", + "ps256": "PS256", + "ps384": "PS384", + "ps512": "PS512", } // AnalyzeDirectory analyzes all JavaScript files in a directory. diff --git a/internal/analyzer/ast/javascript_test.go b/internal/analyzer/ast/javascript_test.go index eac2485..5f666b1 100644 --- a/internal/analyzer/ast/javascript_test.go +++ b/internal/analyzer/ast/javascript_test.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 package ast diff --git a/internal/analyzer/ast/python.go b/internal/analyzer/ast/python.go index 2e9f5f0..ccc0bc5 100644 --- a/internal/analyzer/ast/python.go +++ b/internal/analyzer/ast/python.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 // Package ast provides AST-based analysis for crypto detection. @@ -63,27 +63,27 @@ var ( // Python crypto package mappings var pythonCryptoPackages = map[string][]string{ - "cryptography": {"RSA", "ECDSA", "Ed25519", "AES", "3DES", "ChaCha20"}, - "Crypto": {"RSA", "AES", "DES", "3DES", "SHA-256"}, - "Cryptodome": {"RSA", "AES", "DES", "3DES", "SHA-256"}, - "pycryptodome": {"RSA", "AES", "DES", "3DES", "SHA-256"}, - "hashlib": {"SHA-256", "SHA-512", "MD5", "SHA-1"}, - "hmac": {"HMAC"}, - "bcrypt": {"bcrypt"}, - "argon2": {"Argon2"}, - "scrypt": {"scrypt"}, - "nacl": {"X25519", "Ed25519", "XSalsa20"}, - "PyNaCl": {"X25519", "Ed25519", "XSalsa20"}, - "jwt": {"RS256", "ES256", "HS256"}, - "jose": {"RS256", "ES256", "HS256"}, - "python-jose": {"RS256", "ES256", "HS256"}, - "passlib": {"bcrypt", "Argon2", "PBKDF2"}, - "paramiko": {"RSA", "ECDSA", "Ed25519", "AES"}, - "pyOpenSSL": {"RSA", "ECDSA", "AES", "3DES"}, - "M2Crypto": {"RSA", "AES", "DES"}, - "ecdsa": {"ECDSA"}, - "ed25519": {"Ed25519"}, - "rsa": {"RSA"}, + "cryptography": {"RSA", "ECDSA", "Ed25519", "AES", "3DES", "ChaCha20"}, + "Crypto": {"RSA", "AES", "DES", "3DES", "SHA-256"}, + "Cryptodome": {"RSA", "AES", "DES", "3DES", "SHA-256"}, + "pycryptodome": {"RSA", "AES", "DES", "3DES", "SHA-256"}, + "hashlib": {"SHA-256", "SHA-512", "MD5", "SHA-1"}, + "hmac": {"HMAC"}, + "bcrypt": {"bcrypt"}, + "argon2": {"Argon2"}, + "scrypt": {"scrypt"}, + "nacl": {"X25519", "Ed25519", "XSalsa20"}, + "PyNaCl": {"X25519", "Ed25519", "XSalsa20"}, + "jwt": {"RS256", "ES256", "HS256"}, + "jose": {"RS256", "ES256", "HS256"}, + "python-jose": {"RS256", "ES256", "HS256"}, + "passlib": {"bcrypt", "Argon2", "PBKDF2"}, + "paramiko": {"RSA", "ECDSA", "Ed25519", "AES"}, + "pyOpenSSL": {"RSA", "ECDSA", "AES", "3DES"}, + "M2Crypto": {"RSA", "AES", "DES"}, + "ecdsa": {"ECDSA"}, + "ed25519": {"Ed25519"}, + "rsa": {"RSA"}, } // AnalyzeDirectory analyzes all Python files in a directory. @@ -134,10 +134,10 @@ func (a *PythonAnalyzer) AnalyzeDirectory(dir string) ([]types.CryptoUsage, erro // pyFuncContext tracks function context during Python file analysis. type pyFuncContext struct { - Name string - ClassName string // for methods within classes - IsPublic bool // Python uses leading underscore convention for private - Indent int // indentation level where function starts + Name string + ClassName string // for methods within classes + IsPublic bool // Python uses leading underscore convention for private + Indent int // indentation level where function starts } // AnalyzeFile analyzes a single Python file for cryptographic usage. diff --git a/internal/analyzer/ast/python_test.go b/internal/analyzer/ast/python_test.go index d544bf2..7a22c5a 100644 --- a/internal/analyzer/ast/python_test.go +++ b/internal/analyzer/ast/python_test.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 package ast diff --git a/internal/analyzer/filter_test.go b/internal/analyzer/filter_test.go new file mode 100644 index 0000000..3d8598a --- /dev/null +++ b/internal/analyzer/filter_test.go @@ -0,0 +1,332 @@ +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) +// SPDX-License-Identifier: Apache-2.0 + +package analyzer + +import ( + "fmt" + "os" + "path/filepath" + "sort" + "strings" + "testing" + + "github.com/csnp/qramm-cryptodeps/internal/database" + "github.com/csnp/qramm-cryptodeps/pkg/types" +) + +// filterFixture writes a package.json whose dependencies are in the embedded +// database and therefore produce findings across several risk levels. +func filterFixture(t *testing.T) string { + t.Helper() + dir := t.TempDir() + path := filepath.Join(dir, "package.json") + content := `{"name":"fx","version":"1.0.0","dependencies":{"node-forge":"1.3.1","crypto-js":"4.2.0"}}` + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatalf("write fixture: %v", err) + } + return dir +} + +// findingKeys renders a scan's findings as a comparable set. Comparing sets +// rather than counts is deliberate: a count can match exactly while the filter +// is swapping one finding for another. +func findingKeys(result *types.ScanResult) map[string]bool { + keys := make(map[string]bool) + for _, dep := range result.Dependencies { + if dep.Analysis == nil { + continue + } + for _, c := range dep.Analysis.Crypto { + keys[fmt.Sprintf("%s|%s|%s|%s", + dep.Dependency.Name, c.Algorithm, c.QuantumRisk, c.Severity)] = true + } + } + return keys +} + +func sortedSet(set map[string]bool) []string { + out := make([]string, 0, len(set)) + for k := range set { + out = append(out, k) + } + sort.Strings(out) + return out +} + +// analyzeWith runs a scan with the given options against the fixture. +func analyzeWith(t *testing.T, dir string, opts Options) *types.ScanResult { + t.Helper() + opts.Offline = true + a := New(database.NewWithCachedData(), opts) + result, err := a.Analyze(dir) + if err != nil { + t.Fatalf("analyze: %v", err) + } + return result +} + +// TestRiskFilterActuallyFilters is the regression test for two flags that were +// stored in Options and never read. +// +// Every value, including a misspelt one, produced byte-identical output, so a +// user who asked for a filtered report got an unfiltered one and had no way to +// tell. +func TestRiskFilterActuallyFilters(t *testing.T) { + dir := filterFixture(t) + + unfiltered := findingKeys(analyzeWith(t, dir, Options{})) + if len(unfiltered) == 0 { + t.Fatal("fixture produced no findings, so this test proves nothing") + } + + var haveVulnerable, havePartial bool + for k := range unfiltered { + if strings.Contains(k, string(types.RiskVulnerable)) { + haveVulnerable = true + } + if strings.Contains(k, string(types.RiskPartial)) { + havePartial = true + } + } + if !haveVulnerable || !havePartial { + t.Fatalf("fixture must contain both vulnerable and partial findings to exercise the filter; got %v", + sortedSet(unfiltered)) + } + + filtered := findingKeys(analyzeWith(t, dir, Options{RiskFilter: "vulnerable"})) + if len(filtered) == 0 { + t.Fatal("--risk vulnerable removed everything") + } + if len(filtered) == len(unfiltered) { + t.Errorf("--risk vulnerable changed nothing: %d findings either way", len(filtered)) + } + + // Every surviving finding must match the filter. + for k := range filtered { + if !strings.Contains(k, string(types.RiskVulnerable)) { + t.Errorf("--risk vulnerable kept a non-vulnerable finding: %s", k) + } + } + // The filtered set must be a subset of the unfiltered one. A filter that + // adds or alters findings is a different bug from one that does nothing. + for k := range filtered { + if !unfiltered[k] { + t.Errorf("--risk vulnerable produced a finding absent from the full scan: %s", k) + } + } +} + +// TestMinSeverityActuallyFilters checks the severity threshold. +func TestMinSeverityActuallyFilters(t *testing.T) { + dir := filterFixture(t) + + unfiltered := findingKeys(analyzeWith(t, dir, Options{})) + filtered := findingKeys(analyzeWith(t, dir, Options{MinSeverity: "critical"})) + + if len(unfiltered) == 0 { + t.Fatal("fixture produced no findings") + } + if len(filtered) == len(unfiltered) { + t.Errorf("--min-severity critical changed nothing: %d findings either way", len(filtered)) + } + for k := range filtered { + if !unfiltered[k] { + t.Errorf("--min-severity produced a finding absent from the full scan: %s", k) + } + if !strings.Contains(k, string(types.SeverityCritical)) { + t.Errorf("--min-severity critical kept a lower severity finding: %s", k) + } + } +} + +// TestFilteredOutIsCounted checks that withheld findings are counted, which is +// what lets the report say "filtered" instead of "clean". +func TestFilteredOutIsCounted(t *testing.T) { + dir := filterFixture(t) + + full := analyzeWith(t, dir, Options{}) + filtered := analyzeWith(t, dir, Options{RiskFilter: "vulnerable"}) + + fullCount := 0 + for _, dep := range full.Dependencies { + if dep.Analysis != nil { + fullCount += len(dep.Analysis.Crypto) + } + } + keptCount := 0 + for _, dep := range filtered.Dependencies { + if dep.Analysis != nil { + keptCount += len(dep.Analysis.Crypto) + } + } + + if got, want := filtered.Summary.FilteredOut, fullCount-keptCount; got != want { + t.Errorf("FilteredOut = %d, want %d (full %d, kept %d)", got, want, fullCount, keptCount) + } + if filtered.Summary.FilteredOut == 0 { + t.Error("filter withheld nothing, so the fixture does not exercise the counter") + } + if full.Summary.FilteredOut != 0 { + t.Errorf("unfiltered scan reports %d filtered out, want 0", full.Summary.FilteredOut) + } +} + +// TestNoFilterIsUnchanged guards the default path. A filter that silently +// applies when unset would be a suppression bug. +func TestNoFilterIsUnchanged(t *testing.T) { + dir := filterFixture(t) + + base := findingKeys(analyzeWith(t, dir, Options{})) + for _, value := range []string{"", "all", "ALL", " "} { + got := findingKeys(analyzeWith(t, dir, Options{RiskFilter: value})) + if len(got) != len(base) { + t.Errorf("--risk %q changed the finding count: %d vs %d", value, len(got), len(base)) + } + for k := range base { + if !got[k] { + t.Errorf("--risk %q dropped finding %s", value, k) + } + } + } +} + +// TestFilteringDoesNotMutateTheDatabase catches a filter that writes through the +// shared analysis pointer. Two dependencies resolving to the same database entry +// would then see each other's filtered results. +func TestFilteringDoesNotMutateTheDatabase(t *testing.T) { + dir := filterFixture(t) + db := database.NewWithCachedData() + + filtered, err := New(db, Options{Offline: true, RiskFilter: "vulnerable"}).Analyze(dir) + if err != nil { + t.Fatalf("filtered analyze: %v", err) + } + if filtered.Summary.FilteredOut == 0 { + t.Fatal("filter withheld nothing, so mutation could not be observed") + } + + // Same database instance, no filter. If the filtered run had written + // through the shared pointer, findings would now be missing. + after, err := New(db, Options{Offline: true}).Analyze(dir) + if err != nil { + t.Fatalf("second analyze: %v", err) + } + + fresh, err := New(database.NewWithCachedData(), Options{Offline: true}).Analyze(dir) + if err != nil { + t.Fatalf("fresh analyze: %v", err) + } + + afterKeys, freshKeys := findingKeys(after), findingKeys(fresh) + if len(afterKeys) != len(freshKeys) { + t.Errorf("filtering corrupted the shared database: %d findings after a filtered run, %d on a fresh one", + len(afterKeys), len(freshKeys)) + } + for k := range freshKeys { + if !afterKeys[k] { + t.Errorf("finding %s lost from the shared database after a filtered run", k) + } + } +} + +// TestFilterValidationRejectsUnknownValues checks that a typo is refused rather +// than ignored. Silently accepting one is how a user ends up trusting a report +// they believe was filtered. +func TestFilterValidationRejectsUnknownValues(t *testing.T) { + for _, tc := range []struct { + name string + fn func(string) error + good []string + bad []string + wantMsg string + }{ + { + name: "risk", + fn: ValidateRiskFilter, + good: []string{"", "all", "vulnerable", "VULNERABLE", "partial", "safe", "unknown"}, + bad: []string{"banana", "vuln", "high", "critical"}, + wantMsg: "--risk", + }, + { + name: "min-severity", + fn: ValidateMinSeverity, + good: []string{"", "info", "low", "medium", "high", "critical", "CRITICAL"}, + bad: []string{"banana", "vulnerable", "sev1"}, + wantMsg: "--min-severity", + }, + } { + t.Run(tc.name, func(t *testing.T) { + for _, v := range tc.good { + if err := tc.fn(v); err != nil { + t.Errorf("%q rejected: %v", v, err) + } + } + for _, v := range tc.bad { + err := tc.fn(v) + if err == nil { + t.Errorf("%q accepted; a typo produces an unfiltered report the user believes is filtered", v) + continue + } + if !strings.Contains(err.Error(), tc.wantMsg) { + t.Errorf("error for %q does not name the flag: %v", v, err) + } + } + }) + } +} + +// TestSeverityRankingIsCaseInsensitive guards --min-severity against a database +// record whose severity is not upper case. +// +// severityRank is keyed by the upper-case types.Severity constants, and a Go map +// returns the zero value for an absent key. Ranking an unrecognised severity +// therefore gave 0, which is INFO, so every threshold above INFO discarded the +// finding. Database records arrive from a remote feed and are unmarshalled with +// no normalisation, so a record carrying "critical" was thrown away by the exact +// filter a user reaches for to see critical findings, and it was not even +// counted as withheld. +func TestSeverityRankingIsCaseInsensitive(t *testing.T) { + for _, severity := range []types.Severity{"critical", "CRITICAL", "Critical", "cRiTiCaL"} { + for _, threshold := range []string{"info", "low", "medium", "high", "critical"} { + a := &Analyzer{options: Options{MinSeverity: threshold}} + if !a.keepCrypto(types.CryptoUsage{Algorithm: "DES", Severity: severity}) { + t.Errorf("severity %q was withheld by --min-severity %s; a CRITICAL finding "+ + "must not be discarded because of its case", severity, threshold) + } + } + } +} + +// TestUnrankableSeverityIsReportedNotWithheld pins the fail-safe direction. +// +// A severity this build cannot rank has not been shown to be below the +// threshold, so it is reported. Withholding it would let an unrecognised value +// in remote data silently suppress a finding. +func TestUnrankableSeverityIsReportedNotWithheld(t *testing.T) { + for _, severity := range []types.Severity{"", "SEV1", "urgent", " "} { + a := &Analyzer{options: Options{MinSeverity: "critical"}} + if !a.keepCrypto(types.CryptoUsage{Algorithm: "DES", Severity: severity}) { + t.Errorf("severity %q was withheld; an unrankable severity must fail towards "+ + "reporting, not towards silence", severity) + } + } +} + +// TestRankableSeveritiesBelowThresholdAreStillWithheld proves the two tests +// above did not simply disable the filter. +// +// Without this, making keepCrypto always return true would pass them both. +func TestRankableSeveritiesBelowThresholdAreStillWithheld(t *testing.T) { + a := &Analyzer{options: Options{MinSeverity: "high"}} + for _, severity := range []types.Severity{types.SeverityInfo, types.SeverityLow, types.SeverityMedium, "low", "medium"} { + if a.keepCrypto(types.CryptoUsage{Algorithm: "AES", Severity: severity}) { + t.Errorf("severity %q survived --min-severity high; the filter is not filtering", severity) + } + } + for _, severity := range []types.Severity{types.SeverityHigh, types.SeverityCritical} { + if !a.keepCrypto(types.CryptoUsage{Algorithm: "DES", Severity: severity}) { + t.Errorf("severity %q was withheld by --min-severity high", severity) + } + } +} diff --git a/internal/analyzer/github.go b/internal/analyzer/github.go index 5e6d65f..fc871ff 100644 --- a/internal/analyzer/github.go +++ b/internal/analyzer/github.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 package analyzer diff --git a/internal/analyzer/ondemand/analyzer.go b/internal/analyzer/ondemand/analyzer.go index a27e6a4..1353a1b 100644 --- a/internal/analyzer/ondemand/analyzer.go +++ b/internal/analyzer/ondemand/analyzer.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 // Package ondemand provides on-demand cryptographic analysis of packages. diff --git a/internal/analyzer/ondemand/analyzer_test.go b/internal/analyzer/ondemand/analyzer_test.go index bdfe486..78c42ea 100644 --- a/internal/analyzer/ondemand/analyzer_test.go +++ b/internal/analyzer/ondemand/analyzer_test.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 package ondemand diff --git a/internal/analyzer/reachability/reachability.go b/internal/analyzer/reachability/reachability.go index b524842..baa8538 100644 --- a/internal/analyzer/reachability/reachability.go +++ b/internal/analyzer/reachability/reachability.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 // Package reachability provides call graph analysis to determine if crypto diff --git a/internal/analyzer/reachability/reachability_test.go b/internal/analyzer/reachability/reachability_test.go index ce783ab..f7c9c4b 100644 --- a/internal/analyzer/reachability/reachability_test.go +++ b/internal/analyzer/reachability/reachability_test.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 package reachability @@ -397,13 +397,13 @@ func TestJWTSigningMethods(t *testing.T) { func TestKnownCryptoTargets(t *testing.T) { // Verify essential crypto targets are present expectedTargets := map[string]string{ - "crypto/rsa": "RSA", - "crypto/ecdsa": "ECDSA", - "crypto/ed25519": "Ed25519", - "crypto/aes": "AES", - "crypto/sha256": "SHA-256", - "golang.org/x/crypto/bcrypt": "bcrypt", - "golang.org/x/crypto/argon2": "Argon2", + "crypto/rsa": "RSA", + "crypto/ecdsa": "ECDSA", + "crypto/ed25519": "Ed25519", + "crypto/aes": "AES", + "crypto/sha256": "SHA-256", + "golang.org/x/crypto/bcrypt": "bcrypt", + "golang.org/x/crypto/argon2": "Argon2", "golang.org/x/crypto/chacha20poly1305": "ChaCha20-Poly1305", } diff --git a/internal/analyzer/source/fetcher.go b/internal/analyzer/source/fetcher.go index d0b47d8..de9ef5b 100644 --- a/internal/analyzer/source/fetcher.go +++ b/internal/analyzer/source/fetcher.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 // Package source provides functionality for fetching package source code. diff --git a/internal/analyzer/source/fetcher_test.go b/internal/analyzer/source/fetcher_test.go index 320cad7..94821fe 100644 --- a/internal/analyzer/source/fetcher_test.go +++ b/internal/analyzer/source/fetcher_test.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 package source diff --git a/internal/database/database.go b/internal/database/database.go index cd633da..418a0c6 100644 --- a/internal/database/database.go +++ b/internal/database/database.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 // Package database provides the crypto knowledge base lookup functionality. @@ -121,14 +121,33 @@ func enrichWithRemediation(analysis *types.PackageAnalysis, ecosystem types.Ecos } // Stats returns statistics about the database. +// +// It counts distinct database records, not index entries. addToIndex deliberately files +// every package under two keys, "name@version" and "name", so that a lookup +// succeeds with or without a version. Reporting len(index) as a package count +// therefore counted every versioned package twice: `cryptodeps status` announced +// 1731 packages for a database holding 901, and every per-ecosystem number was +// wrong in the same way. The two keys collapse for a package with no version, +// which is why the inflation was not a clean doubling and why the number looked +// plausible enough to survive. +// +// Records, not names: the database can hold two records for one package under +// different spellings of its name, as it does for PGPy/pgpy and PyNaCl/pynacl, +// and counting records is what makes this agree with the stats block the +// database publishes about itself. A name-based count would report 899 where +// the file says 901. func (db *Database) Stats() DatabaseStats { stats := DatabaseStats{ ByEcosystem: make(map[types.Ecosystem]int), } for ecosystem, pkgs := range db.index { - stats.ByEcosystem[ecosystem] = len(pkgs) - stats.TotalPackages += len(pkgs) + distinct := make(map[*types.PackageAnalysis]bool, len(pkgs)) + for _, pkg := range pkgs { + distinct[pkg] = true + } + stats.ByEcosystem[ecosystem] = len(distinct) + stats.TotalPackages += len(distinct) } return stats @@ -195,10 +214,14 @@ func (db *Database) loadEmbeddedData() { Version: "", Ecosystem: types.EcosystemGo, Analysis: types.AnalysisMetadata{ - Date: time.Now(), - Method: "embedded", - Tool: "cryptodeps", - ToolVersion: "1.0.0", + Date: time.Now(), + Method: "embedded", + Tool: "cryptodeps", + // No ToolVersion: the embedded records are curated by hand, so + // attributing them to any tool version is a false provenance + // claim. The scanner's own version is reported once, at the top + // of each output document. This matches the other 69 embedded + // records, which have never set the field. }, Crypto: []types.CryptoUsage{ {Algorithm: "Ed25519", Type: "signature", QuantumRisk: types.RiskVulnerable, Severity: types.SeverityHigh, Remediation: "Migrate to ML-DSA (FIPS 204) for signatures"}, diff --git a/internal/database/database_test.go b/internal/database/database_test.go index 9e196f6..2543b21 100644 --- a/internal/database/database_test.go +++ b/internal/database/database_test.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 package database @@ -360,3 +360,50 @@ func TestLookupWithExactVersion(t *testing.T) { t.Error("Should find package with fallback to no-version key") } } + +// TestStatsCountsPackagesNotIndexEntries is the regression test for a number the +// tool announced on a user-facing surface and got wrong by about 90 per cent. +// +// addToIndex deliberately files every package under two keys, "name@version" and +// "name", so a lookup succeeds with or without a version. Stats reported +// len(index), so every versioned package counted twice: `cryptodeps status` +// announced 1731 packages for a database of 901. The two keys collapse into one +// for a package with no version, which is why the inflation was not a clean +// doubling and why the wrong number looked plausible. +func TestStatsCountsPackagesNotIndexEntries(t *testing.T) { + db := New("") + pkgs := []*types.PackageAnalysis{ + {Package: "node-forge", Version: "1.3.1", Ecosystem: types.EcosystemNPM}, + {Package: "jsonwebtoken", Version: "9.0.0", Ecosystem: types.EcosystemNPM}, + // No version: both index keys collapse to one, so this package is the + // reason a naive len(index) is not simply double the truth. + {Package: "left-pad", Ecosystem: types.EcosystemNPM}, + {Package: "cryptography", Version: "42.0.0", Ecosystem: types.EcosystemPyPI}, + } + for _, p := range pkgs { + db.addToIndex(p) + } + + // Guard the fixture: the index really must hold more entries than packages, + // or this passes against the broken implementation too. + entries := 0 + for _, byEcosystem := range db.index { + entries += len(byEcosystem) + } + if entries <= len(pkgs) { + t.Fatalf("index holds %d entries for %d packages; the fixture does not exercise "+ + "the double-keying this test exists for", entries, len(pkgs)) + } + + stats := db.Stats() + if stats.TotalPackages != len(pkgs) { + t.Errorf("Stats().TotalPackages = %d, want %d; the index holds %d entries and the "+ + "count must be of packages, not entries", stats.TotalPackages, len(pkgs), entries) + } + if got, want := stats.ByEcosystem[types.EcosystemNPM], 3; got != want { + t.Errorf("Stats().ByEcosystem[npm] = %d, want %d", got, want) + } + if got, want := stats.ByEcosystem[types.EcosystemPyPI], 1; got != want { + t.Errorf("Stats().ByEcosystem[pypi] = %d, want %d", got, want) + } +} diff --git a/internal/database/updater.go b/internal/database/updater.go index 65c9db0..bca4bbf 100644 --- a/internal/database/updater.go +++ b/internal/database/updater.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 package database diff --git a/internal/database/updater_test.go b/internal/database/updater_test.go index 57cbd4e..9cdfef4 100644 --- a/internal/database/updater_test.go +++ b/internal/database/updater_test.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 package database diff --git a/internal/manifest/gomod.go b/internal/manifest/gomod.go index 9a7bbd6..2fa1ed0 100644 --- a/internal/manifest/gomod.go +++ b/internal/manifest/gomod.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 package manifest diff --git a/internal/manifest/maven.go b/internal/manifest/maven.go index e3185aa..54c301e 100644 --- a/internal/manifest/maven.go +++ b/internal/manifest/maven.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 package manifest diff --git a/internal/manifest/npm.go b/internal/manifest/npm.go index 2100047..cc10540 100644 --- a/internal/manifest/npm.go +++ b/internal/manifest/npm.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 package manifest @@ -6,6 +6,7 @@ package manifest import ( "encoding/json" "os" + "sort" "strings" "github.com/csnp/qramm-cryptodeps/pkg/types" @@ -26,11 +27,11 @@ func (p *NPMParser) Filenames() []string { // packageJSON represents the structure of a package.json file. type packageJSON struct { - Name string `json:"name"` - Version string `json:"version"` - Dependencies map[string]string `json:"dependencies"` - DevDependencies map[string]string `json:"devDependencies"` - PeerDependencies map[string]string `json:"peerDependencies"` + Name string `json:"name"` + Version string `json:"version"` + Dependencies map[string]string `json:"dependencies"` + DevDependencies map[string]string `json:"devDependencies"` + PeerDependencies map[string]string `json:"peerDependencies"` OptionalDependencies map[string]string `json:"optionalDependencies"` } @@ -48,47 +49,37 @@ func (p *NPMParser) Parse(path string) ([]types.Dependency, error) { var deps []types.Dependency - // Parse production dependencies (direct) - for name, version := range pkg.Dependencies { - deps = append(deps, types.Dependency{ - Name: name, - Version: cleanNPMVersion(version), - Ecosystem: types.EcosystemNPM, - Direct: true, - }) + // Each block is emitted in sorted order. Go randomises map iteration, so + // ranging over these maps directly made the whole report shuffle between + // runs of the same scan, which breaks diffable CI output and reproducible + // SBOMs even though the finding set itself was stable. + for _, block := range []map[string]string{ + pkg.Dependencies, + pkg.DevDependencies, + pkg.PeerDependencies, + pkg.OptionalDependencies, + } { + for _, name := range sortedKeys(block) { + deps = append(deps, types.Dependency{ + Name: name, + Version: cleanNPMVersion(block[name]), + Ecosystem: types.EcosystemNPM, + Direct: true, + }) + } } - // Parse dev dependencies (direct but dev-only) - for name, version := range pkg.DevDependencies { - deps = append(deps, types.Dependency{ - Name: name, - Version: cleanNPMVersion(version), - Ecosystem: types.EcosystemNPM, - Direct: true, - }) - } - - // Parse peer dependencies - for name, version := range pkg.PeerDependencies { - deps = append(deps, types.Dependency{ - Name: name, - Version: cleanNPMVersion(version), - Ecosystem: types.EcosystemNPM, - Direct: true, - }) - } + return deps, nil +} - // Parse optional dependencies - for name, version := range pkg.OptionalDependencies { - deps = append(deps, types.Dependency{ - Name: name, - Version: cleanNPMVersion(version), - Ecosystem: types.EcosystemNPM, - Direct: true, - }) +// sortedKeys returns a map's keys in a stable order. +func sortedKeys(m map[string]string) []string { + keys := make([]string, 0, len(m)) + for k := range m { + keys = append(keys, k) } - - return deps, nil + sort.Strings(keys) + return keys } // cleanNPMVersion normalizes npm version strings. diff --git a/internal/manifest/npm_order_test.go b/internal/manifest/npm_order_test.go new file mode 100644 index 0000000..3399401 --- /dev/null +++ b/internal/manifest/npm_order_test.go @@ -0,0 +1,66 @@ +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) +// SPDX-License-Identifier: Apache-2.0 + +package manifest + +import ( + "path/filepath" + "strings" + "testing" +) + +// TestNPMParseOrderIsStable is the regression test for output that shuffled +// between runs of the same scan. +// +// package.json dependency blocks unmarshal into maps, and Go randomises map +// iteration, so ranging over them directly made every downstream format reorder +// itself run to run. The finding set was stable; only the order moved, which is +// enough to break golden-file CI and reproducible SBOMs. +// +// Ten dependency names are used because the randomisation is per-iteration: with +// two or three names, a stable-looking result is likely by chance. +func TestNPMParseOrderIsStable(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "package.json") + writeFile(t, path, `{ + "name": "ordering", + "dependencies": { + "zeta": "1.0.0", "alpha": "1.0.0", "mike": "1.0.0", "bravo": "1.0.0", + "yankee": "1.0.0", "charlie": "1.0.0", "xray": "1.0.0", "delta": "1.0.0", + "whisky": "1.0.0", "echo": "1.0.0" + }, + "devDependencies": {"tango": "1.0.0", "foxtrot": "1.0.0"} + }`) + + parser := &NPMParser{} + var first []string + for i := 0; i < 20; i++ { + deps, err := parser.Parse(path) + if err != nil { + t.Fatalf("parse: %v", err) + } + names := make([]string, 0, len(deps)) + for _, d := range deps { + names = append(names, d.Name) + } + if i == 0 { + first = names + continue + } + if strings.Join(names, ",") != strings.Join(first, ",") { + t.Fatalf("dependency order changed between parses of the same file:\nfirst: %v\nnow: %v", + first, names) + } + } + + if len(first) != 12 { + t.Fatalf("got %d dependencies, want 12", len(first)) + } + // Production dependencies come before dev dependencies, each sorted. + if first[0] != "alpha" || first[9] != "zeta" { + t.Errorf("production block is not sorted: %v", first[:10]) + } + if first[10] != "foxtrot" || first[11] != "tango" { + t.Errorf("dev block is not sorted or not last: %v", first[10:]) + } +} diff --git a/internal/manifest/parser.go b/internal/manifest/parser.go index 062e69f..e31bc42 100644 --- a/internal/manifest/parser.go +++ b/internal/manifest/parser.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 // Package manifest provides parsers for dependency manifest files. @@ -142,46 +142,56 @@ func SupportedManifests() []string { } // DetectAndParseAll discovers all manifests in a directory (including workspaces) -// and parses each one. Returns a slice of parsed manifests. -func DetectAndParseAll(path string) ([]*Manifest, error) { +// and parses each one. +// +// It returns the parsed manifests and every manifest that was found but could +// not be used, so that the caller can report the skips rather than hiding them. +// Both a validation rejection during discovery and a parse failure here produce +// a SkippedManifest. +func DetectAndParseAll(path string) ([]*Manifest, []types.SkippedManifest, error) { // Check if path is a directory info, err := os.Stat(path) if err != nil { - return nil, fmt.Errorf("cannot access path: %w", err) + return nil, nil, fmt.Errorf("cannot access path: %w", err) } // If it's a file, just parse that single file if !info.IsDir() { manifest, err := DetectAndParse(path) if err != nil { - return nil, err + return nil, nil, err } - return []*Manifest{manifest}, nil + return []*Manifest{manifest}, nil, nil } // Discover all manifests in the directory tree - manifestPaths, err := DiscoverManifests(path) + manifestPaths, skipped, err := DiscoverManifests(path) if err != nil { - return nil, fmt.Errorf("failed to discover manifests: %w", err) + return nil, nil, fmt.Errorf("failed to discover manifests: %w", err) } - if len(manifestPaths) == 0 { - return nil, fmt.Errorf("no supported manifest files found in %s", path) + if len(manifestPaths) == 0 && len(skipped) == 0 { + return nil, nil, fmt.Errorf("no supported manifest files found in %s", path) } var manifests []*Manifest - var parseErrors []string for _, manifestPath := range manifestPaths { parser, err := getParserForPath(manifestPath) if err != nil { - // Skip unsupported files silently (they might have been picked up by glob) + // Recognised by discovery but not by any parser. Report it, because + // the user is entitled to know a file that looks like a manifest was + // not read, but mark it unsupported so it does not make the scan + // look incomplete. cryptodeps never claimed to read Cargo.toml, and + // erroring on one turned every polyglot repository into a build + // failure. + skipped = append(skipped, newSkip(manifestPath, "no parser for this manifest type")) continue } deps, err := parser.Parse(manifestPath) if err != nil { - parseErrors = append(parseErrors, fmt.Sprintf("%s: %v", manifestPath, err)) + skipped = append(skipped, newSkip(manifestPath, err.Error())) continue } @@ -192,9 +202,27 @@ func DetectAndParseAll(path string) ([]*Manifest, error) { }) } - if len(manifests) == 0 && len(parseErrors) > 0 { - return nil, fmt.Errorf("failed to parse any manifests: %s", strings.Join(parseErrors, "; ")) + if len(manifests) == 0 && len(skipped) > 0 { + // "could not be read" is only true of a manifest that should have been + // readable. A tree holding nothing but a Cargo.toml is not a broken + // tree, it is an ecosystem cryptodeps does not support, and saying + // otherwise sends the user to look for a defect in a healthy file. + if !types.IncompleteScan(skipped) { + return nil, skipped, fmt.Errorf("no supported manifest files found in %s: %s", + path, describeSkipped(skipped)) + } + return nil, skipped, fmt.Errorf("found %d manifest file(s) but none could be read: %s", + len(skipped), describeSkipped(skipped)) } - return manifests, nil + return manifests, skipped, nil +} + +// describeSkipped renders skipped manifests for an error message. +func describeSkipped(skipped []types.SkippedManifest) string { + parts := make([]string, 0, len(skipped)) + for _, s := range skipped { + parts = append(parts, fmt.Sprintf("%s (%s)", s.Path, s.Reason)) + } + return strings.Join(parts, "; ") } diff --git a/internal/manifest/parser_test.go b/internal/manifest/parser_test.go index 121f31e..5c9ac2c 100644 --- a/internal/manifest/parser_test.go +++ b/internal/manifest/parser_test.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 package manifest diff --git a/internal/manifest/python.go b/internal/manifest/python.go index 02724bb..5b27b1f 100644 --- a/internal/manifest/python.go +++ b/internal/manifest/python.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 package manifest diff --git a/internal/manifest/python_formats_test.go b/internal/manifest/python_formats_test.go index ce77c57..9d3de7d 100644 --- a/internal/manifest/python_formats_test.go +++ b/internal/manifest/python_formats_test.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 package manifest @@ -72,10 +72,13 @@ func TestDiscoverFindsRequirementsFamily(t *testing.T) { writeFile(t, filepath.Join(dir, "requirements-prod.txt"), "pyjwt>=2.0\n") writeFile(t, filepath.Join(dir, "requirements", "base.txt"), "pycryptodome==3.19.0\n") - found, err := DiscoverManifests(dir) + found, skipped, err := DiscoverManifests(dir) if err != nil { t.Fatalf("discover: %v", err) } + if len(skipped) != 0 { + t.Errorf("no manifest should have been skipped, got %v", skipped) + } seen := make(map[string]bool) for _, path := range found { diff --git a/internal/manifest/skipped_test.go b/internal/manifest/skipped_test.go new file mode 100644 index 0000000..ba15641 --- /dev/null +++ b/internal/manifest/skipped_test.go @@ -0,0 +1,306 @@ +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) +// SPDX-License-Identifier: Apache-2.0 + +package manifest + +import ( + "path/filepath" + "sort" + "strings" + "testing" + + "github.com/csnp/qramm-cryptodeps/pkg/types" +) + +// TestCorruptManifestIsReportedNotDropped is the regression test for a scanner +// that skipped input silently. +// +// A tree with a good and a corrupt package.json scanned only the good one, never +// mentioned the corrupt one, and reported a clean summary. The cause was not the +// parse-error path: validateManifest rejected the file during discovery, before +// any parser ran, so no parse error was ever produced. +func TestCorruptManifestIsReportedNotDropped(t *testing.T) { + root := t.TempDir() + writeFile(t, filepath.Join(root, "good", "package.json"), + `{"name":"good","dependencies":{"node-forge":"1.3.1"}}`) + // Truncated mid-object, the shape a bad merge leaves behind. + writeFile(t, filepath.Join(root, "corrupt", "package.json"), + `{"name":"corrupt","dependencies":{"node-forge":`) + + manifests, skipped, err := DetectAndParseAll(root) + if err != nil { + t.Fatalf("DetectAndParseAll: %v", err) + } + + if len(manifests) != 1 { + t.Fatalf("got %d parsed manifests, want 1 (the good one)", len(manifests)) + } + if !strings.Contains(manifests[0].Path, "good") { + t.Errorf("parsed the wrong manifest: %s", manifests[0].Path) + } + + if len(skipped) != 1 { + t.Fatalf("got %d skipped manifests, want 1; a corrupt manifest was dropped silently", len(skipped)) + } + if !strings.Contains(skipped[0].Path, "corrupt") { + t.Errorf("skipped the wrong file: %s", skipped[0].Path) + } + if skipped[0].Reason == "" { + t.Error("skipped manifest carries no reason, so the user cannot tell what is wrong with the file") + } + if !strings.Contains(skipped[0].Reason, "JSON") { + t.Errorf("reason %q does not identify the defect", skipped[0].Reason) + } +} + +// TestOnlyCorruptManifestDoesNotClaimNoneExist checks the error message. +// +// A tree whose only manifest is corrupt reported "no supported manifest files +// found", sending the user to look for a file that is right there. +func TestOnlyCorruptManifestDoesNotClaimNoneExist(t *testing.T) { + root := t.TempDir() + writeFile(t, filepath.Join(root, "package.json"), `{"name":"x","dependencies":{`) + + _, skipped, err := DetectAndParseAll(root) + if err == nil { + t.Fatal("expected an error when no manifest could be read") + } + if strings.Contains(err.Error(), "no supported manifest files found") { + t.Errorf("error claims no manifest exists when one is present: %v", err) + } + if !strings.Contains(err.Error(), "package.json") { + t.Errorf("error does not name the file that failed: %v", err) + } + if len(skipped) != 1 { + t.Errorf("got %d skipped, want 1", len(skipped)) + } +} + +// TestEmptyManifestIsReported covers the other validation rejection. +func TestEmptyManifestIsReported(t *testing.T) { + root := t.TempDir() + writeFile(t, filepath.Join(root, "good", "package.json"), `{"name":"good"}`) + writeFile(t, filepath.Join(root, "empty", "package.json"), ``) + + _, skipped, err := DetectAndParseAll(root) + if err != nil { + t.Fatalf("DetectAndParseAll: %v", err) + } + if len(skipped) != 1 { + t.Fatalf("got %d skipped, want 1; an empty manifest was dropped silently", len(skipped)) + } + if !strings.Contains(skipped[0].Reason, "empty") { + t.Errorf("reason %q does not say the file is empty", skipped[0].Reason) + } +} + +// TestValidTreeSkipsNothing guards against a change that reports everything as +// skipped, which would make the notice meaningless. +func TestValidTreeSkipsNothing(t *testing.T) { + root := t.TempDir() + writeFile(t, filepath.Join(root, "a", "package.json"), `{"name":"a","dependencies":{"left-pad":"1.3.0"}}`) + writeFile(t, filepath.Join(root, "b", "requirements.txt"), "requests==2.31.0\n") + writeFile(t, filepath.Join(root, "c", "go.mod"), "module example.com/c\n\ngo 1.21\n") + + manifests, skipped, err := DetectAndParseAll(root) + if err != nil { + t.Fatalf("DetectAndParseAll: %v", err) + } + if len(skipped) != 0 { + t.Errorf("a valid tree reported skipped manifests: %v", skipped) + } + if len(manifests) != 3 { + t.Errorf("got %d manifests, want 3", len(manifests)) + } +} + +// TestDiscoveryOrderIsStable checks that repeated discovery returns the same +// order. +// +// This is a preservation guard, not a regression test, and the distinction was +// previously misstated here. The old comment claimed discovery "merged two +// layers without sorting" so the order depended on how they interleaved. That +// was wrong on both counts: filepath.Glob returns sorted matches and +// filepath.Walk is lexical, so the pre-fix code was already stable, and the old +// fixture declared no workspaces at all so it never engaged the workspace layer +// it claimed to test. Measured against the pre-fix build, the order was +// identical across 50 runs. +// +// The fixture below does declare a workspace, so both layers contribute and the +// sort actually has something to order. What the added sort changed is where the +// root package.json lands, not whether the order is stable. This test fails if +// anyone reintroduces map iteration into discovery; it does not claim to prove +// the sort fixed a nondeterminism that existed. +func TestDiscoveryOrderIsStable(t *testing.T) { + root := t.TempDir() + writeFile(t, filepath.Join(root, "package.json"), + `{"name":"root","private":true,"workspaces":["zeta","alpha","mid","beta"],`+ + `"dependencies":{"left-pad":"1.3.0"}}`) + for _, name := range []string{"zeta", "alpha", "mid", "beta"} { + writeFile(t, filepath.Join(root, name, "package.json"), + `{"name":"`+name+`","dependencies":{"left-pad":"1.3.0"}}`) + } + + var first []string + for i := 0; i < 10; i++ { + found, _, err := DiscoverManifests(root) + if err != nil { + t.Fatalf("discover: %v", err) + } + if i == 0 { + first = found + continue + } + if strings.Join(found, "\n") != strings.Join(first, "\n") { + t.Fatalf("discovery order changed between runs:\nfirst: %v\nnow: %v", first, found) + } + } + if len(first) != 5 { + t.Fatalf("got %d manifests, want 5 (root plus four workspace members); "+ + "a fixture that does not engage the workspace layer cannot guard its ordering", len(first)) + } + if !sort.StringsAreSorted(first) { + t.Errorf("discovery returned an unsorted list, so the two layers are being "+ + "concatenated rather than ordered: %v", first) + } +} + +// TestSupportedManifestsAllHaveParsers is the structural version of the test +// above: it fails by construction if the parsable set and the parsers disagree, +// rather than waiting for someone to notice an exit code. +// +// Both directions, because the first version checked only one. A parsable name +// with no parser forces exit 2 on every scan that meets the file. A parser that +// is no longer reachable from discovery silently stops scanning an entire +// ecosystem, which is the more dangerous of the two: flipping "pom.xml" to false +// disabled all Maven scanning and left the whole suite green. +func TestSupportedManifestsAllHaveParsers(t *testing.T) { + for name, parsable := range ManifestFiles { + _, err := getParser(name) + switch { + case parsable && err != nil: + t.Errorf("%q is marked parsable but has no parser (%v); it would be discovered, "+ + "fail to parse, and be reported as unsupported on every scan that meets one", name, err) + case !parsable && err == nil: + t.Errorf("%q has a parser but is marked unparsable, so discovery routes it to the "+ + "unsupported path and that ecosystem is never scanned", name) + } + } + + // Every manifest the tool advertises must be reachable from discovery. + for _, name := range SupportedManifests() { + if !IsParsableManifest(name) { + t.Errorf("SupportedManifests advertises %q but discovery does not treat it as "+ + "parsable, so a documented ecosystem is silently never scanned", name) + } + } +} + +// TestUnsupportedManifestsAreStillReported is the other half of the polyglot +// fix, and the half that was got wrong first. +// +// Narrowing discovery stopped the spurious exit 2 by making these files vanish +// from every output format at exit 0. A build.gradle full of crypto dependencies +// became invisible, which is exactly the silent skip this branch exists to +// remove, and it contradicted the tool's own message that "a silently skipped +// manifest is how a scanner reports a clean tree it never read". They must be +// reported AND must not force exit 2. +func TestUnsupportedManifestsAreStillReported(t *testing.T) { + for _, name := range []string{"Cargo.toml", "Gemfile", "composer.json", "build.gradle", "build.gradle.kts"} { + t.Run(name, func(t *testing.T) { + root := t.TempDir() + writeFile(t, filepath.Join(root, "go.mod"), "module example.com/x\n\ngo 1.21\n") + writeFile(t, filepath.Join(root, name), "placeholder\n") + + _, skipped, err := DetectAndParseAll(root) + if err != nil { + t.Fatalf("DetectAndParseAll: %v", err) + } + + var found *types.SkippedManifest + for i := range skipped { + if filepath.Base(skipped[i].Path) == name { + found = &skipped[i] + } + } + if found == nil { + t.Fatalf("%s was not reported at all; a manifest the tool cannot read must "+ + "never be silently dropped, whatever the reason", name) + } + if !found.Unsupported { + t.Errorf("%s is reported as an unread manifest rather than an unsupported "+ + "ecosystem, so it forces exit 2 and masks the real finding-based code", name) + } + }) + } +} + +// TestUnreadableManifestStillMarksTheScanIncomplete is the paired guard: the +// Unsupported flag must not become a way for a genuinely broken manifest to stop +// counting. +func TestUnreadableManifestStillMarksTheScanIncomplete(t *testing.T) { + root := t.TempDir() + writeFile(t, filepath.Join(root, "good", "package.json"), `{"name":"g","dependencies":{"left-pad":"1.3.0"}}`) + writeFile(t, filepath.Join(root, "bad", "package.json"), `{"name":"b","dependencies":`) + + _, skipped, err := DetectAndParseAll(root) + if err != nil { + t.Fatalf("DetectAndParseAll: %v", err) + } + if len(skipped) != 1 { + t.Fatalf("got %d skipped, want 1: %+v", len(skipped), skipped) + } + if skipped[0].Unsupported { + t.Error("a corrupt package.json was marked unsupported, so the scan would report " + + "itself complete while a dependency file went unread") + } + if !types.IncompleteScan(skipped) { + t.Error("IncompleteScan is false for an unreadable manifest, so the scan exits 0") + } +} + +// TestUnsupportedOnlyTreeIsNotCalledUnreadable guards the wording a user of a +// Rust or Ruby repository actually sees. +// +// When nothing parseable is found, the error explains why. Reporting a healthy +// Cargo.toml as a file that "could not be read" sends the user to look for a +// defect in a file that has none, and contradicts the tool's own classification +// of the same skip as a declared limit rather than an incomplete scan. +func TestUnsupportedOnlyTreeIsNotCalledUnreadable(t *testing.T) { + root := t.TempDir() + writeFile(t, filepath.Join(root, "Cargo.toml"), "[package]\nname = \"x\"\n") + + _, skipped, err := DetectAndParseAll(root) + if err == nil { + t.Fatal("a tree with no parseable manifest reported success") + } + if !types.IncompleteScan(skipped) && strings.Contains(err.Error(), "none could be read") { + t.Errorf("error says the manifest could not be read, which IncompleteScan says is "+ + "false for it: %v", err) + } + if !strings.Contains(err.Error(), "no supported manifest files found") { + t.Errorf("error does not say the ecosystem is unsupported: %v", err) + } + if len(skipped) != 1 || !skipped[0].Unsupported { + t.Errorf("skipped = %+v, want one unsupported entry so the caller can report the "+ + "file rather than only the failure", skipped) + } +} + +// TestUnreadableOnlyTreeStillSaysItCouldNotBeRead is the paired direction. The +// wording split must not make a genuinely broken manifest sound supported. +func TestUnreadableOnlyTreeStillSaysItCouldNotBeRead(t *testing.T) { + root := t.TempDir() + writeFile(t, filepath.Join(root, "package.json"), `{"name":"b","dependencies":`) + + _, skipped, err := DetectAndParseAll(root) + if err == nil { + t.Fatal("a tree whose only manifest is corrupt reported success") + } + if !strings.Contains(err.Error(), "none could be read") { + t.Errorf("error does not say the manifest could not be read: %v", err) + } + if !types.IncompleteScan(skipped) { + t.Errorf("skipped = %+v, want an incomplete scan", skipped) + } +} diff --git a/internal/manifest/workspace.go b/internal/manifest/workspace.go index 2a80a73..54fd871 100644 --- a/internal/manifest/workspace.go +++ b/internal/manifest/workspace.go @@ -1,16 +1,21 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 package manifest import ( "encoding/json" + "errors" + "fmt" "os" "path/filepath" "regexp" + "sort" "strings" "gopkg.in/yaml.v3" + + "github.com/csnp/qramm-cryptodeps/pkg/types" ) // DefaultSkipDirs contains directories that should be skipped during manifest discovery. @@ -42,20 +47,61 @@ var DefaultSkipDirs = map[string]bool{ "bower_components": true, } -// ManifestFiles contains filenames that indicate a project manifest. +// ManifestFiles maps a manifest filename to whether cryptodeps has a parser for +// it. Every name here is discovered; the value decides what happens next. +// +// A name mapped to false is still found and still reported, as an unsupported +// ecosystem rather than an unread file, and it does not affect the exit code. +// Both halves of that matter, and getting either wrong has already shipped a +// defect. Treating these as ordinary skips made every polyglot repository exit 2 +// and masked the exit 1 that real findings had earned. Dropping them from +// discovery to fix that made a build.gradle full of crypto dependencies vanish +// from all five output formats at exit 0, which is precisely the silent skip +// this branch exists to remove. +// +// go.work is false because workspace membership is resolved by parseGoWorkspace, +// which reads it directly and contributes the member go.mod files. The workspace +// file declares no dependencies of its own. var ManifestFiles = map[string]bool{ "go.mod": true, - "go.work": true, "package.json": true, "requirements.txt": true, "pyproject.toml": true, "Pipfile": true, "pom.xml": true, - "build.gradle": true, - "build.gradle.kts": true, - "Cargo.toml": true, - "Gemfile": true, - "composer.json": true, + + "go.work": false, + "build.gradle": false, + "build.gradle.kts": false, + "Cargo.toml": false, + "Gemfile": false, + "composer.json": false, +} + +// IsParsableManifest reports whether a path has a parser behind it. +// +// It asks getParserForPath, the same function discovery uses, rather than +// reimplementing the rule. A parallel predicate would drift, and it would get +// the requirements/*.txt layout wrong: requirements/base.txt is named base.txt +// and is parsable only because of its parent directory. +func IsParsableManifest(path string) bool { + _, err := getParserForPath(path) + return err == nil +} + +// newSkip records a manifest that was found but not analyzed. +// +// Whether a skip is "unsupported" is a property of the FILE, not of the stage +// that happened to reject it. Deciding it at the parser-lookup site instead left +// an empty Cargo.toml, rejected earlier by validateManifest, classified as an +// unread manifest, so it forced exit 2 and undid the polyglot fix for any tree +// whose unsupported manifest was also empty or unreadable. +func newSkip(path, reason string) types.SkippedManifest { + return types.SkippedManifest{ + Path: path, + Reason: reason, + Unsupported: !IsParsableManifest(path), + } } // DiscoverManifests finds all manifest files in a directory tree. @@ -63,23 +109,33 @@ var ManifestFiles = map[string]bool{ // 1. Parse workspace configuration files (package.json workspaces, go.work, pnpm-workspace.yaml) // 2. Recursively walk the directory tree for any manifests not covered by workspace config // 3. Deduplicate and validate results -func DiscoverManifests(root string) ([]string, error) { +// +// It returns the usable manifests and, separately, every file that was +// recognised as a manifest by name but rejected by validation. The second +// return used to be discarded, which is what made a corrupt package.json +// invisible: it was dropped here, before any parser ran, so no parse error was +// ever produced and the scan reported a clean summary for the files that +// happened to survive. +func DiscoverManifests(root string) ([]string, []types.SkippedManifest, error) { root, err := filepath.Abs(root) if err != nil { - return nil, err + return nil, nil, err } info, err := os.Stat(root) if err != nil { - return nil, err + return nil, nil, err } // If it's a file, return just that file if it's a manifest if !info.IsDir() { if isManifestPath(root) { - return []string{root}, nil + if err := validateManifest(root); err != nil { + return nil, []types.SkippedManifest{newSkip(root, err.Error())}, nil + } + return []string{root}, nil, nil } - return nil, nil + return nil, nil, nil } seen := make(map[string]bool) @@ -109,15 +165,21 @@ func DiscoverManifests(root string) ([]string, error) { } } - // Layer 3: Validate and filter + // Layer 3: Validate, keeping the rejects so the caller can report them. + // Sorted so that discovery order does not depend on how the two layers + // above happened to interleave. + sort.Strings(manifests) var validated []string + var skipped []types.SkippedManifest for _, m := range manifests { - if isValidManifest(m) { - validated = append(validated, m) + if err := validateManifest(m); err != nil { + skipped = append(skipped, newSkip(m, err.Error())) + continue } + validated = append(validated, m) } - return validated, nil + return validated, skipped, nil } // parseWorkspaceConfigs detects and parses workspace configuration files. @@ -354,8 +416,13 @@ func isRequirementsFile(name string) bool { } // isManifestFile checks if a filename is a recognized manifest file. +// +// Membership, not the mapped value: a name cryptodeps cannot parse is still +// discovered so that it can be reported as an unsupported ecosystem. Testing the +// value here instead is what made those files vanish from every output format. func isManifestFile(name string) bool { - return ManifestFiles[name] || isRequirementsFile(name) + _, known := ManifestFiles[name] + return known || isRequirementsFile(name) } // isManifestPath checks whether a path is a manifest, including layouts that @@ -374,22 +441,22 @@ func isManifestPath(path string) bool { return false } -// isValidManifest checks if a manifest file is valid and parseable. -func isValidManifest(path string) bool { +// validateManifest reports whether a manifest file is usable, and returns the +// reason when it is not. The reason is shown to the user, so it names the defect +// rather than just saying the file was skipped. +func validateManifest(path string) error { info, err := os.Stat(path) if err != nil { - return false + return fmt.Errorf("cannot read file: %w", err) } - // Skip empty files if info.Size() == 0 { - return false + return errors.New("file is empty") } - // Try to read the file data, err := os.ReadFile(path) if err != nil { - return false + return fmt.Errorf("cannot read file: %w", err) } filename := filepath.Base(path) @@ -398,21 +465,25 @@ func isValidManifest(path string) bool { switch filename { case "package.json": var pkg map[string]interface{} - return json.Unmarshal(data, &pkg) == nil + if err := json.Unmarshal(data, &pkg); err != nil { + return fmt.Errorf("not valid JSON: %w", err) + } + return nil case "go.mod": - // Must contain "module" directive - return strings.Contains(string(data), "module ") + if !strings.Contains(string(data), "module ") { + return errors.New("no module directive") + } + return nil case "pom.xml": - // Must contain project tag - return strings.Contains(string(data), " element") + } + return nil default: - return true + // requirements.txt family, pyproject.toml, Pipfile and anything else + // recognised by name: non-empty is all we can check cheaply. Real + // defects surface as parse errors, which are reported the same way. + return nil } } diff --git a/internal/registry/fetcher.go b/internal/registry/fetcher.go index a867dab..b1e1e44 100644 --- a/internal/registry/fetcher.go +++ b/internal/registry/fetcher.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 // Package registry fetches crypto package information from package registries. diff --git a/internal/registry/golang.go b/internal/registry/golang.go index 58b4b31..412a809 100644 --- a/internal/registry/golang.go +++ b/internal/registry/golang.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 package registry diff --git a/internal/registry/inference.go b/internal/registry/inference.go index 9f33dfe..aab9e61 100644 --- a/internal/registry/inference.go +++ b/internal/registry/inference.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 package registry @@ -8,6 +8,7 @@ import ( "github.com/csnp/qramm-cryptodeps/pkg/crypto" "github.com/csnp/qramm-cryptodeps/pkg/types" + "github.com/csnp/qramm-cryptodeps/pkg/version" ) // InferredAlgorithm represents an algorithm inferred from package metadata. @@ -359,10 +360,12 @@ func ToPackageAnalysis(pkg PackageInfo, algorithms []InferredAlgorithm) types.Pa Version: pkg.Version, Ecosystem: pkg.Ecosystem, Analysis: types.AnalysisMetadata{ - Date: pkg.UpdatedAt, + Date: pkg.UpdatedAt, + // This record is produced by the running binary, so it is the one + // place where the scanner's own version is the honest provenance. Method: "inferred", - Tool: "cryptodeps", - ToolVersion: "", + Tool: version.Name, + ToolVersion: version.Version(), }, Crypto: cryptoUsages, QuantumSummary: summary, diff --git a/internal/registry/maven.go b/internal/registry/maven.go index 1fac49b..746c6c1 100644 --- a/internal/registry/maven.go +++ b/internal/registry/maven.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 package registry @@ -41,14 +41,14 @@ type mavenSearchResponse struct { Response struct { NumFound int `json:"numFound"` Docs []struct { - ID string `json:"id"` - Group string `json:"g"` - Artifact string `json:"a"` - LatestVersion string `json:"latestVersion"` - RepositoryID string `json:"repositoryId"` - Timestamp int64 `json:"timestamp"` - VersionCount int `json:"versionCount"` - Text []string `json:"text"` + ID string `json:"id"` + Group string `json:"g"` + Artifact string `json:"a"` + LatestVersion string `json:"latestVersion"` + RepositoryID string `json:"repositoryId"` + Timestamp int64 `json:"timestamp"` + VersionCount int `json:"versionCount"` + Text []string `json:"text"` } `json:"docs"` } `json:"response"` } diff --git a/internal/registry/merger.go b/internal/registry/merger.go index 5b930ca..a58e504 100644 --- a/internal/registry/merger.go +++ b/internal/registry/merger.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 package registry diff --git a/internal/registry/npm.go b/internal/registry/npm.go index 88da42d..8e17987 100644 --- a/internal/registry/npm.go +++ b/internal/registry/npm.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 package registry diff --git a/internal/registry/pypi.go b/internal/registry/pypi.go index a0ea018..a5d50f5 100644 --- a/internal/registry/pypi.go +++ b/internal/registry/pypi.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 package registry diff --git a/pkg/crypto/patterns.go b/pkg/crypto/patterns.go index 44fd4db..4201e0e 100644 --- a/pkg/crypto/patterns.go +++ b/pkg/crypto/patterns.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 package crypto @@ -7,7 +7,7 @@ import "github.com/csnp/qramm-cryptodeps/pkg/types" // ImportPattern represents a crypto library import pattern for an ecosystem. type ImportPattern struct { - Pattern string // Import path or module pattern + Pattern string // Import path or module pattern Ecosystem types.Ecosystem Description string Algorithms []string // Known algorithms this library provides diff --git a/pkg/crypto/patterns_test.go b/pkg/crypto/patterns_test.go index fdae7d9..2c714d8 100644 --- a/pkg/crypto/patterns_test.go +++ b/pkg/crypto/patterns_test.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 package crypto diff --git a/pkg/crypto/quantum.go b/pkg/crypto/quantum.go index f171032..d746dc1 100644 --- a/pkg/crypto/quantum.go +++ b/pkg/crypto/quantum.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 // Package crypto provides cryptographic algorithm classification and quantum risk assessment. @@ -51,15 +51,15 @@ var algorithmDatabase = map[string]AlgorithmInfo{ "ps512": {Name: "PS512", Type: "signature", QuantumRisk: types.RiskVulnerable, Severity: types.SeverityHigh, Description: "RSASSA-PSS with SHA-512", Remediation: "Use HS512 for symmetric signing, or wait for PQ-JWT standards"}, // Symmetric - PARTIAL (Grover's reduces security by half) - "aes": {Name: "AES", Type: "encryption", QuantumRisk: types.RiskPartial, Severity: types.SeverityInfo, Description: "Advanced Encryption Standard", Remediation: "Use AES-256 for 128-bit post-quantum security"}, - "aes-128": {Name: "AES-128", Type: "encryption", QuantumRisk: types.RiskPartial, Severity: types.SeverityLow, Description: "AES with 128-bit key (64-bit post-quantum)", Remediation: "Upgrade to AES-256 for 128-bit post-quantum security"}, - "aes-192": {Name: "AES-192", Type: "encryption", QuantumRisk: types.RiskPartial, Severity: types.SeverityInfo, Description: "AES with 192-bit key", Remediation: "Consider upgrading to AES-256"}, - "aes-256": {Name: "AES-256", Type: "encryption", QuantumRisk: types.RiskSafe, Severity: types.SeverityInfo, Description: "AES with 256-bit key (128-bit post-quantum)", Remediation: "No action needed - quantum safe"}, - "aes-gcm": {Name: "AES-GCM", Type: "encryption", QuantumRisk: types.RiskPartial, Severity: types.SeverityInfo, Description: "AES Galois/Counter Mode", Remediation: "Use AES-256-GCM for post-quantum security"}, - "aes-cbc": {Name: "AES-CBC", Type: "encryption", QuantumRisk: types.RiskPartial, Severity: types.SeverityInfo, Description: "AES Cipher Block Chaining", Remediation: "Consider AES-256-GCM for authenticated encryption"}, - "chacha20": {Name: "ChaCha20", Type: "encryption", QuantumRisk: types.RiskSafe, Severity: types.SeverityInfo, Description: "ChaCha20 stream cipher", Remediation: "No action needed - quantum safe"}, - "xchacha20": {Name: "XChaCha20", Type: "encryption", QuantumRisk: types.RiskSafe, Severity: types.SeverityInfo, Description: "Extended nonce ChaCha20", Remediation: "No action needed - quantum safe"}, - "poly1305": {Name: "Poly1305", Type: "hash", QuantumRisk: types.RiskSafe, Severity: types.SeverityInfo, Description: "Poly1305 MAC", Remediation: "No action needed - quantum safe"}, + "aes": {Name: "AES", Type: "encryption", QuantumRisk: types.RiskPartial, Severity: types.SeverityInfo, Description: "Advanced Encryption Standard", Remediation: "Use AES-256 for 128-bit post-quantum security"}, + "aes-128": {Name: "AES-128", Type: "encryption", QuantumRisk: types.RiskPartial, Severity: types.SeverityLow, Description: "AES with 128-bit key (64-bit post-quantum)", Remediation: "Upgrade to AES-256 for 128-bit post-quantum security"}, + "aes-192": {Name: "AES-192", Type: "encryption", QuantumRisk: types.RiskPartial, Severity: types.SeverityInfo, Description: "AES with 192-bit key", Remediation: "Consider upgrading to AES-256"}, + "aes-256": {Name: "AES-256", Type: "encryption", QuantumRisk: types.RiskSafe, Severity: types.SeverityInfo, Description: "AES with 256-bit key (128-bit post-quantum)", Remediation: "No action needed - quantum safe"}, + "aes-gcm": {Name: "AES-GCM", Type: "encryption", QuantumRisk: types.RiskPartial, Severity: types.SeverityInfo, Description: "AES Galois/Counter Mode", Remediation: "Use AES-256-GCM for post-quantum security"}, + "aes-cbc": {Name: "AES-CBC", Type: "encryption", QuantumRisk: types.RiskPartial, Severity: types.SeverityInfo, Description: "AES Cipher Block Chaining", Remediation: "Consider AES-256-GCM for authenticated encryption"}, + "chacha20": {Name: "ChaCha20", Type: "encryption", QuantumRisk: types.RiskSafe, Severity: types.SeverityInfo, Description: "ChaCha20 stream cipher", Remediation: "No action needed - quantum safe"}, + "xchacha20": {Name: "XChaCha20", Type: "encryption", QuantumRisk: types.RiskSafe, Severity: types.SeverityInfo, Description: "Extended nonce ChaCha20", Remediation: "No action needed - quantum safe"}, + "poly1305": {Name: "Poly1305", Type: "hash", QuantumRisk: types.RiskSafe, Severity: types.SeverityInfo, Description: "Poly1305 MAC", Remediation: "No action needed - quantum safe"}, // HMAC algorithms "hs256": {Name: "HS256", Type: "signature", QuantumRisk: types.RiskPartial, Severity: types.SeverityInfo, Description: "HMAC with SHA-256", Remediation: "Consider HS512 for stronger post-quantum security"}, @@ -87,18 +87,18 @@ var algorithmDatabase = map[string]AlgorithmInfo{ "blake3": {Name: "BLAKE3", Type: "hash", QuantumRisk: types.RiskSafe, Severity: types.SeverityInfo, Description: "BLAKE3 hash", Remediation: "No action needed - quantum safe"}, // Broken classical algorithms - "des": {Name: "DES", Type: "encryption", QuantumRisk: types.RiskVulnerable, Severity: types.SeverityCritical, Description: "DES (56-bit, broken)", Remediation: "Replace immediately with AES-256"}, - "3des": {Name: "3DES", Type: "encryption", QuantumRisk: types.RiskVulnerable, Severity: types.SeverityHigh, Description: "Triple DES", Remediation: "Replace with AES-256"}, - "rc4": {Name: "RC4", Type: "encryption", QuantumRisk: types.RiskVulnerable, Severity: types.SeverityCritical, Description: "RC4 (broken)", Remediation: "Replace immediately with AES-256 or ChaCha20"}, - "rc2": {Name: "RC2", Type: "encryption", QuantumRisk: types.RiskVulnerable, Severity: types.SeverityCritical, Description: "RC2 (weak)", Remediation: "Replace immediately with AES-256"}, + "des": {Name: "DES", Type: "encryption", QuantumRisk: types.RiskVulnerable, Severity: types.SeverityCritical, Description: "DES (56-bit, broken)", Remediation: "Replace immediately with AES-256"}, + "3des": {Name: "3DES", Type: "encryption", QuantumRisk: types.RiskVulnerable, Severity: types.SeverityHigh, Description: "Triple DES", Remediation: "Replace with AES-256"}, + "rc4": {Name: "RC4", Type: "encryption", QuantumRisk: types.RiskVulnerable, Severity: types.SeverityCritical, Description: "RC4 (broken)", Remediation: "Replace immediately with AES-256 or ChaCha20"}, + "rc2": {Name: "RC2", Type: "encryption", QuantumRisk: types.RiskVulnerable, Severity: types.SeverityCritical, Description: "RC2 (weak)", Remediation: "Replace immediately with AES-256"}, // Post-Quantum - SAFE - "ml-kem": {Name: "ML-KEM", Type: "key-exchange", QuantumRisk: types.RiskSafe, Severity: types.SeverityInfo, Description: "NIST FIPS 203 (Kyber)", Remediation: "No action needed - quantum safe"}, - "ml-dsa": {Name: "ML-DSA", Type: "signature", QuantumRisk: types.RiskSafe, Severity: types.SeverityInfo, Description: "NIST FIPS 204 (Dilithium)", Remediation: "No action needed - quantum safe"}, - "slh-dsa": {Name: "SLH-DSA", Type: "signature", QuantumRisk: types.RiskSafe, Severity: types.SeverityInfo, Description: "NIST FIPS 205 (SPHINCS+)", Remediation: "No action needed - quantum safe"}, - "kyber": {Name: "Kyber", Type: "key-exchange", QuantumRisk: types.RiskSafe, Severity: types.SeverityInfo, Description: "Kyber KEM", Remediation: "No action needed - quantum safe"}, - "dilithium": {Name: "Dilithium", Type: "signature", QuantumRisk: types.RiskSafe, Severity: types.SeverityInfo, Description: "Dilithium signature", Remediation: "No action needed - quantum safe"}, - "sphincs": {Name: "SPHINCS+", Type: "signature", QuantumRisk: types.RiskSafe, Severity: types.SeverityInfo, Description: "SPHINCS+ signature", Remediation: "No action needed - quantum safe"}, + "ml-kem": {Name: "ML-KEM", Type: "key-exchange", QuantumRisk: types.RiskSafe, Severity: types.SeverityInfo, Description: "NIST FIPS 203 (Kyber)", Remediation: "No action needed - quantum safe"}, + "ml-dsa": {Name: "ML-DSA", Type: "signature", QuantumRisk: types.RiskSafe, Severity: types.SeverityInfo, Description: "NIST FIPS 204 (Dilithium)", Remediation: "No action needed - quantum safe"}, + "slh-dsa": {Name: "SLH-DSA", Type: "signature", QuantumRisk: types.RiskSafe, Severity: types.SeverityInfo, Description: "NIST FIPS 205 (SPHINCS+)", Remediation: "No action needed - quantum safe"}, + "kyber": {Name: "Kyber", Type: "key-exchange", QuantumRisk: types.RiskSafe, Severity: types.SeverityInfo, Description: "Kyber KEM", Remediation: "No action needed - quantum safe"}, + "dilithium": {Name: "Dilithium", Type: "signature", QuantumRisk: types.RiskSafe, Severity: types.SeverityInfo, Description: "Dilithium signature", Remediation: "No action needed - quantum safe"}, + "sphincs": {Name: "SPHINCS+", Type: "signature", QuantumRisk: types.RiskSafe, Severity: types.SeverityInfo, Description: "SPHINCS+ signature", Remediation: "No action needed - quantum safe"}, } // ClassifyAlgorithm returns information about an algorithm. diff --git a/pkg/crypto/quantum_test.go b/pkg/crypto/quantum_test.go index 62b3e6c..02bb7a9 100644 --- a/pkg/crypto/quantum_test.go +++ b/pkg/crypto/quantum_test.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 package crypto diff --git a/pkg/crypto/remediation.go b/pkg/crypto/remediation.go index 6508446..f325c8b 100644 --- a/pkg/crypto/remediation.go +++ b/pkg/crypto/remediation.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 // Package crypto provides cryptographic algorithm classification and remediation guidance. diff --git a/pkg/crypto/remediation_test.go b/pkg/crypto/remediation_test.go index 7e428dc..4209121 100644 --- a/pkg/crypto/remediation_test.go +++ b/pkg/crypto/remediation_test.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 package crypto diff --git a/pkg/output/cbom.go b/pkg/output/cbom.go index 7fef17b..99582f1 100644 --- a/pkg/output/cbom.go +++ b/pkg/output/cbom.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 package output @@ -15,6 +15,7 @@ import ( "time" "github.com/csnp/qramm-cryptodeps/pkg/types" + "github.com/csnp/qramm-cryptodeps/pkg/version" ) // CBOMFormatter formats scan results as CycloneDX CBOM (JSON). @@ -32,8 +33,22 @@ type cycloneDXBOM struct { } type cycloneDXMetadata struct { - Timestamp string `json:"timestamp"` - Tools []cycloneDXTool `json:"tools"` + Timestamp string `json:"timestamp"` + Tools []cycloneDXTool `json:"tools"` + Properties []cycloneDXProperty `json:"properties,omitempty"` +} + +// cycloneDXProperty is the spec's name/value pair, used here to record what the +// scan did not cover. +// +// A CBOM asserts a cryptographic bill of materials. Emitting one from a scan +// that could not read a manifest, or that withheld findings behind a filter, +// with nothing to say so, is the same false-completeness claim that unreadable +// manifests used to produce in the table. The component list alone cannot +// express an absence. +type cycloneDXProperty struct { + Name string `json:"name"` + Value string `json:"value"` } type cycloneDXTool struct { @@ -74,6 +89,17 @@ type cycloneDXAlgorithmProperties struct { // Format writes the scan result as CycloneDX CBOM. func (f *CBOMFormatter) Format(result *types.ScanResult, w io.Writer) error { + return f.format(result, []*types.ScanResult{result}, nil, w) +} + +// format writes a CBOM, recording any manifest that was not read and anything +// the scan did not establish. +// +// projects is the per-project view, kept separate from the merged result because +// coverage has to be judged per project. Judging it on the merged summary let a +// workspace where one project was entirely unexamined emit a document that said +// nothing about it. +func (f *CBOMFormatter) format(result *types.ScanResult, projects []*types.ScanResult, skipped []types.SkippedManifest, w io.Writer) error { if result == nil { return errors.New("result cannot be nil") } @@ -89,14 +115,15 @@ func (f *CBOMFormatter) Format(result *types.ScanResult, w io.Writer) error { Timestamp: time.Now().UTC().Format(time.RFC3339), Tools: []cycloneDXTool{ { - Vendor: "CSNP", - Name: "cryptodeps", - Version: "1.0.0", + Vendor: version.Vendor, + Name: version.Name, + Version: version.Version(), }, }, }, Components: make([]cycloneDXComponent, 0), } + bom.Metadata.Properties = cbomCoverageProperties(result.Project, projects, skipped) // Emit each dependency as a component, then each algorithm it provides as a // cryptographic asset, and link the two through the dependencies graph. @@ -303,5 +330,66 @@ func (f *CBOMFormatter) FormatMulti(result *types.MultiProjectResult, w io.Write merged.Dependencies = append(merged.Dependencies, project.Dependencies...) } - return f.Format(merged, w) + return f.format(merged, result.Projects, result.Skipped, w) +} + +// cbomCoverageProperties records what this bill of materials does not cover. +// +// Named under a cryptodeps: prefix because CycloneDX property names are +// namespaced by convention and these are tool-specific, not spec fields. +// Coverage is judged per project through the shared classifier, so this cannot +// disagree with what the table and markdown reports say. +func cbomCoverageProperties(root string, projects []*types.ScanResult, skipped []types.SkippedManifest) []cycloneDXProperty { + var props []cycloneDXProperty + + // Normalized once. Comparing a manifest path against the raw root is what + // made the relativization a no-op for `cryptodeps analyze .`, which is the + // invocation the GitHub Action uses. + absRoot := scanRootDir(root) + + var unread int + for _, s := range skipped { + name := "cryptodeps:manifestNotAnalyzed" + if s.Unsupported { + name = "cryptodeps:manifestNotSupported" + } else { + unread++ + } + props = append(props, cycloneDXProperty{Name: name, Value: relativeManifest(absRoot, s.Path) + ": " + s.Reason}) + } + if unread > 0 { + props = append(props, cycloneDXProperty{ + Name: "cryptodeps:coverage", + Value: fmt.Sprintf("incomplete: %d manifest(s) were found but could not be read, "+ + "so the dependencies they declare are absent from this document", unread), + }) + } + + for _, note := range coverageNotes(projects) { + name := "cryptodeps:coverage" + if note.Case == caseFiltered { + name = "cryptodeps:findingsWithheld" + } + value := note.Text() + if note.Manifest != "" && len(projects) > 1 { + // Relative to the scan root. Emitting the absolute path published + // the operator's home directory, or a CI runner's workspace path, + // into a document meant to be shared. + value = relativeManifest(absRoot, note.Manifest) + ": " + value + } + props = append(props, cycloneDXProperty{Name: name, Value: value}) + } + + return props +} + +// relativeManifest renders a manifest path relative to an already-normalized +// scan root, so that a shared document carries no local filesystem layout. +// +// absRoot must come from scanRootDir. The first version took the raw root and +// compared it against absolute manifest paths, so it returned the absolute path +// unchanged for every relative root, which is every default invocation. +func relativeManifest(absRoot, manifest string) string { + rel, _ := relativeToRoot(absRoot, manifest) + return rel } diff --git a/pkg/output/cbom_attribution_test.go b/pkg/output/cbom_attribution_test.go index c80e7bf..df14a8a 100644 --- a/pkg/output/cbom_attribution_test.go +++ b/pkg/output/cbom_attribution_test.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 package output diff --git a/pkg/output/coverage_test.go b/pkg/output/coverage_test.go new file mode 100644 index 0000000..f66922d --- /dev/null +++ b/pkg/output/coverage_test.go @@ -0,0 +1,692 @@ +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) +// SPDX-License-Identifier: Apache-2.0 + +package output + +import ( + "bytes" + "encoding/json" + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/csnp/qramm-cryptodeps/pkg/types" +) + +// These tests exist because the first version of the false-clean fix was +// table-only. +// +// The table learned to distinguish "we looked and found nothing" from "we +// looked at nothing" and from "we withheld everything", while markdown, CBOM and +// SARIF kept reporting the clean result. Markdown still printed the exact +// sentence the table fix removed, and SARIF asserted executionSuccessful with an +// empty result set, which a code-scanning consumer reads as a clean bill of +// health. A user who asks for --format markdown is not reading the table, so a +// fix that only reaches one of five formats has not fixed the defect. +// +// Every case below is asserted against every format that can express it, so a +// future formatter cannot quietly opt out. + +// writeTestFile writes a fixture file, failing the test if it cannot. +func writeTestFile(t *testing.T, path, content string) { + t.Helper() + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatalf("write %s: %v", path, err) + } +} + +// renderMulti formats a multi-project result in the named format. +func renderMulti(t *testing.T, format Format, result *types.MultiProjectResult) string { + t.Helper() + f, err := GetFormatter(format) + if err != nil { + t.Fatalf("GetFormatter(%s): %v", format, err) + } + var buf bytes.Buffer + if err := f.FormatMulti(result, &buf); err != nil { + t.Fatalf("FormatMulti(%s): %v", format, err) + } + return buf.String() +} + +// filteredScan is a scan where every finding was detected and then withheld by a +// filter. Nothing survives to be reported, but the tree is not clean. +func filteredScan() *types.MultiProjectResult { + return types.AggregateResults("/repo", []*types.ScanResult{{ + Project: "/repo/a", + Manifest: "/repo/a/package.json", + Ecosystem: types.EcosystemNPM, + Dependencies: []types.DependencyResult{{ + Dependency: types.Dependency{Name: "node-forge", Version: "1.3.1"}, + InDatabase: true, + Analysis: &types.PackageAnalysis{Package: "node-forge"}, + }}, + Summary: types.ScanSummary{TotalDependencies: 1, DirectDependencies: 1, FilteredOut: 9}, + }}) +} + +// nothingExaminedScan is a scan where no dependency was in the database, so no +// conclusion about cryptographic usage was drawn. +func nothingExaminedScan() *types.MultiProjectResult { + return types.AggregateResults("/repo", []*types.ScanResult{{ + Project: "/repo/a", + Manifest: "/repo/a/requirements.txt", + Ecosystem: types.EcosystemPyPI, + Dependencies: []types.DependencyResult{ + {Dependency: types.Dependency{Name: "rsa", Version: "4.9"}}, + {Dependency: types.Dependency{Name: "certifi"}}, + }, + Summary: types.ScanSummary{TotalDependencies: 2, DirectDependencies: 2, NotInDatabase: 2}, + }}) +} + +// withheldAssertion checks, per format, that the withheld findings are actually +// reported in a way a consumer of THAT format can act on. +// +// Each assertion names the concrete field or sentence, never a bare substring. +// The first version of this test asserted strings.Contains(out, "9"), which the +// CBOM's random v4 serialNumber and the JSON scanDate satisfy by accident: it +// passed against an implementation with the CBOM property and the JSON +// aggregation both deliberately disabled, and was flaky besides. A count is the +// wrong assertion, and so is a digit. +var withheldAssertion = map[Format]func(t *testing.T, out string, want int){ + FormatTable: func(t *testing.T, out string, want int) { + assertWithheldSentence(t, FormatTable, out, want, emptiedByFilter(FormatTable, want)...) + }, + FormatMarkdown: func(t *testing.T, out string, want int) { + assertWithheldSentence(t, FormatMarkdown, out, want, emptiedByFilter(FormatMarkdown, want)...) + }, + FormatJSON: func(t *testing.T, out string, want int) { + var doc struct { + TotalSummary struct { + FilteredOut int `json:"filteredOut"` + } `json:"totalSummary"` + } + if err := json.Unmarshal([]byte(out), &doc); err != nil { + t.Fatalf("JSON is not valid: %v", err) + } + if doc.TotalSummary.FilteredOut != want { + t.Errorf("totalSummary.filteredOut = %d, want %d; a consumer reading the "+ + "aggregate cannot tell this filtered scan from a clean one", + doc.TotalSummary.FilteredOut, want) + } + }, + FormatCBOM: func(t *testing.T, out string, want int) { + var doc struct { + Metadata struct { + Properties []struct { + Name string `json:"name"` + Value string `json:"value"` + } `json:"properties"` + } `json:"metadata"` + } + if err := json.Unmarshal([]byte(out), &doc); err != nil { + t.Fatalf("CBOM is not valid JSON: %v", err) + } + for _, p := range doc.Metadata.Properties { + if p.Name == "cryptodeps:findingsWithheld" { + // The exact count, at the start of the named property. + // Contains(value, "9") passed against an implementation + // multiplying the count by 100, and the CBOM's random v4 + // serialNumber contains any given digit most runs anyway. + if !strings.HasPrefix(p.Value, fmt.Sprintf("%d finding(s)", want)) { + t.Errorf("cryptodeps:findingsWithheld does not state %d withheld: %q", want, p.Value) + } + return + } + } + t.Errorf("CBOM has no cryptodeps:findingsWithheld property, so it asserts a "+ + "complete bill of materials for a filtered scan: %+v", doc.Metadata.Properties) + }, + FormatSARIF: func(t *testing.T, out string, want int) { + var doc struct { + Runs []struct { + Invocations []struct { + ToolExecutionNotifications []struct { + Message struct { + Text string `json:"text"` + } `json:"message"` + } `json:"toolExecutionNotifications"` + } `json:"invocations"` + } `json:"runs"` + } + if err := json.Unmarshal([]byte(out), &doc); err != nil { + t.Fatalf("SARIF is not valid JSON: %v", err) + } + // Parsed, not grepped: the notification text is the field a consumer + // reads, and a bare substring match over the whole document is + // satisfied by a timestamp, a version or a path. + wantText := fmt.Sprintf("%d finding(s) were detected and withheld", want) + for _, run := range doc.Runs { + for _, inv := range run.Invocations { + for _, n := range inv.ToolExecutionNotifications { + if strings.Contains(n.Message.Text, wantText) { + return + } + } + } + } + t.Errorf("no SARIF toolExecutionNotification states %q:\n%s", wantText, out) + }, +} + +// assertWithheldSentence checks that a human format states the withheld count, +// in every statement that format is supposed to make about it. +// +// Not an any-of list. Markdown states it twice in a workspace report, once per +// project and once in the Overview table, so accepting either one alone left +// both deletable with the suite green, and the single-project renderer that +// --no-workspaces uses has only the first. The two scenarios genuinely word it +// differently: a scan filtered to nothing explains itself where the verdict +// would go, while a scan that still has something to show annotates the summary. +func assertWithheldSentence(t *testing.T, format Format, out string, want int, sentences ...string) { + t.Helper() + if len(sentences) == 0 { + t.Fatalf("%s: no sentence supplied, so this asserts nothing", format) + } + for _, sentence := range sentences { + if !strings.Contains(out, sentence) { + t.Errorf("%s does not state %q, so it does not say that %d finding(s) were "+ + "withheld:\n%s", format, sentence, want, out) + } + } +} + +// emptiedByFilter is what the table and markdown say when a filter withheld +// everything, and partiallyFiltered is what they say when something survived. +func emptiedByFilter(format Format, n int) []string { + switch format { + case FormatTable: + return []string{fmt.Sprintf("%d finding(s) were detected and", n), + "excluded by --risk or --min-severity"} + case FormatMarkdown: + return []string{fmt.Sprintf("%d finding(s) were detected and "+ + "excluded by `--risk` or `--min-severity`", n)} + } + return nil +} + +func partiallyFiltered(format Format, n int) []string { + switch format { + case FormatTable: + return []string{fmt.Sprintf("FILTERED: %d further finding(s) excluded by "+ + "--risk or --min-severity", n)} + case FormatMarkdown: + // Both. Each is the only statement on some rendering path: the first is + // all the single-project report has, the second is what a reader of the + // workspace Overview takes away. + return []string{ + fmt.Sprintf("> %d further finding(s) were excluded by `--risk` or `--min-severity`", n), + fmt.Sprintf("| **Withheld by filter** | %d |", n), + } + } + return nil +} + +// TestEveryFormatSaysFindingsWereWithheld covers the filtered-to-empty case. +func TestEveryFormatSaysFindingsWereWithheld(t *testing.T) { + for _, format := range []Format{FormatTable, FormatMarkdown, FormatJSON, FormatCBOM, FormatSARIF} { + t.Run(string(format), func(t *testing.T) { + out := renderMulti(t, format, filteredScan()) + + if strings.Contains(out, "No cryptographic usage detected in dependencies.") { + t.Errorf("%s reports a clean scan while 9 findings were withheld by a filter:\n%s", + format, out) + } + withheldAssertion[format](t, out, 9) + }) + } +} + +// TestCoverageNotesDoNotContradictResults is the guard for a statement that was +// false on the face of the document that carried it. +// +// SARIF and CBOM evaluated "nothing was examined" over the whole run and without +// checking whether findings existed, so a --deep scan that found two CRITICAL +// algorithms in packages absent from the database emitted both those findings +// AND a notice saying no conclusion about cryptographic usage could be drawn. +// A project with findings gets no coverage note. +func TestCoverageNotesDoNotContradictResults(t *testing.T) { + withFindings := &types.ScanResult{ + Manifest: "/repo/a/go.mod", + Dependencies: []types.DependencyResult{{ + Dependency: types.Dependency{Name: "github.com/google/uuid", Version: "v1.6.0"}, + Analysis: &types.PackageAnalysis{Package: "github.com/google/uuid", Crypto: []types.CryptoUsage{ + {Algorithm: "MD5", QuantumRisk: types.RiskVulnerable, Severity: types.SeverityCritical}, + }}, + }}, + // Deliberately the shape --deep produces: findings exist even though + // every dependency is absent from the database. + Summary: types.ScanSummary{TotalDependencies: 1, DirectDependencies: 1, WithCrypto: 1, + QuantumVulnerable: 1, NotInDatabase: 1}, + } + + if notes := coverageNotes([]*types.ScanResult{withFindings}); len(notes) != 0 { + t.Fatalf("a project with findings produced coverage notes %+v; the document would "+ + "assert that nothing was examined beside the findings it just reported", notes) + } + + for _, format := range []Format{FormatSARIF, FormatCBOM, FormatTable, FormatMarkdown} { + t.Run(string(format), func(t *testing.T) { + out := renderMulti(t, format, types.AggregateResults("/repo", []*types.ScanResult{withFindings})) + if strings.Contains(out, "nothing was examined") || + strings.Contains(out, "no conclusion about cryptographic usage") { + t.Errorf("%s claims nothing was examined while reporting findings:\n%s", format, out) + } + if !strings.Contains(out, "MD5") { + t.Fatalf("fixture produced no MD5 finding in %s, so it cannot detect the "+ + "contradiction:\n%s", format, out) + } + }) + } +} + +// TestCoverageIsJudgedPerProject guards the opposite direction of the same bug. +// +// Summing notInDatabase and totalDependencies across a whole workspace hid a +// project that was entirely unexamined behind a sibling that was fully analyzed: +// 21 of 24 dependencies went unexamined and no machine-readable format said so, +// while the table said it plainly. +func TestCoverageIsJudgedPerProject(t *testing.T) { + unexamined := &types.ScanResult{ + Manifest: "/repo/unknown/package.json", + Dependencies: []types.DependencyResult{ + {Dependency: types.Dependency{Name: "left-pad"}}, + {Dependency: types.Dependency{Name: "is-odd"}}, + }, + Summary: types.ScanSummary{TotalDependencies: 2, DirectDependencies: 2, NotInDatabase: 2}, + } + analyzed := &types.ScanResult{ + Manifest: "/repo/known/go.mod", + Dependencies: []types.DependencyResult{{ + Dependency: types.Dependency{Name: "golang.org/x/crypto", Version: "v0.31.0"}, + InDatabase: true, + Analysis: &types.PackageAnalysis{Package: "golang.org/x/crypto", Crypto: []types.CryptoUsage{ + {Algorithm: "RSA", QuantumRisk: types.RiskVulnerable, Severity: types.SeverityHigh}, + }}, + }}, + Summary: types.ScanSummary{TotalDependencies: 1, DirectDependencies: 1, WithCrypto: 1, QuantumVulnerable: 1}, + } + multi := types.AggregateResults("/repo", []*types.ScanResult{unexamined, analyzed}) + + // Guard the fixture: the aggregate must NOT satisfy notInDatabase == total, + // or the old whole-run test would have caught this and there is no bug. + if multi.TotalSummary.NotInDatabase >= multi.TotalSummary.TotalDependencies { + t.Fatalf("fixture does not mix examined and unexamined projects: %+v", multi.TotalSummary) + } + + notes := coverageNotes(multi.Projects) + if len(notes) != 1 || notes[0].Case != caseNothingExamined { + t.Fatalf("expected exactly one nothing-examined note for the unexamined project, got %+v", notes) + } + + for _, format := range []Format{FormatSARIF, FormatCBOM} { + t.Run(string(format), func(t *testing.T) { + out := renderMulti(t, format, multi) + if !strings.Contains(out, "nothing was examined") { + t.Errorf("%s does not report the project where no dependency was examined:\n%s", + format, out) + } + }) + } +} + +// TestClassifyNoFindingsChecksFilterFirst pins the ordering the package +// documents as its safety property. +// +// Nothing else in the suite fails if the cases are reordered, yet a summary of +// {Total: 2, NotInDatabase: 2, FilteredOut: 2} is reachable whenever --deep finds +// crypto in packages absent from the database and a filter withholds it. Ordered +// wrongly, that scan reports "not analyzed" and never mentions the two withheld +// findings. +func TestClassifyNoFindingsChecksFilterFirst(t *testing.T) { + both := types.ScanSummary{TotalDependencies: 2, NotInDatabase: 2, FilteredOut: 2} + if got := classifyNoFindings(both); got != caseFiltered { + t.Errorf("classifyNoFindings(%+v) = %v, want caseFiltered; withheld findings must "+ + "outrank every other explanation for an empty report", both, got) + } + + // And each other case in isolation, so the test above cannot be satisfied by + // always returning caseFiltered. + for _, tc := range []struct { + name string + summary types.ScanSummary + want noFindingsCase + }{ + {"no dependencies", types.ScanSummary{}, caseNoDependencies}, + {"all unknown", types.ScanSummary{TotalDependencies: 3, NotInDatabase: 3}, caseNothingExamined}, + {"examined and clean", types.ScanSummary{TotalDependencies: 3, NotInDatabase: 1}, caseGenuinelyClean}, + } { + if got := classifyNoFindings(tc.summary); got != tc.want { + t.Errorf("%s: classifyNoFindings(%+v) = %v, want %v", tc.name, tc.summary, got, tc.want) + } + } +} + +// TestEveryFormatSaysNothingWasExamined covers the all-unknown case, which is +// the defect the table verdict was originally written for. +func TestEveryFormatSaysNothingWasExamined(t *testing.T) { + for _, format := range []Format{FormatTable, FormatMarkdown, FormatJSON, FormatCBOM, FormatSARIF} { + t.Run(string(format), func(t *testing.T) { + out := renderMulti(t, format, nothingExaminedScan()) + + if strings.Contains(out, "No cryptographic usage detected in dependencies.") { + t.Errorf("%s reports a clean scan when no dependency was examined:\n%s", format, out) + } + }) + } +} + +// TestFilteredScanIsNotAssertedAsAFullySuccessfulRun checks the SARIF signal a +// code-scanning consumer actually reads. +func TestFilteredScanIsNotAssertedAsAFullySuccessfulRun(t *testing.T) { + var doc struct { + Runs []struct { + Results []any `json:"results"` + Invocations []struct { + ExecutionSuccessful bool `json:"executionSuccessful"` + ToolExecutionNotifications []struct { + Level string `json:"level"` + Message struct { + Text string `json:"text"` + } `json:"message"` + } `json:"toolExecutionNotifications"` + } `json:"invocations"` + } `json:"runs"` + } + out := renderMulti(t, FormatSARIF, filteredScan()) + if err := json.Unmarshal([]byte(out), &doc); err != nil { + t.Fatalf("SARIF is not valid JSON: %v", err) + } + if len(doc.Runs) != 1 || len(doc.Runs[0].Invocations) != 1 { + t.Fatalf("expected one run with one invocation, got %+v", doc) + } + if len(doc.Runs[0].Results) != 0 { + t.Fatalf("fixture did not filter everything away: %d results", len(doc.Runs[0].Results)) + } + notifications := doc.Runs[0].Invocations[0].ToolExecutionNotifications + if len(notifications) == 0 { + t.Fatal("SARIF emitted zero results and zero notifications, so a consumer cannot " + + "tell a filtered run from a clean one") + } + var mentionsWithheld bool + for _, n := range notifications { + if strings.Contains(n.Message.Text, "withheld") { + mentionsWithheld = true + } + } + if !mentionsWithheld { + t.Errorf("no notification says findings were withheld: %+v", notifications) + } +} + +// TestSARIFBaseIDIsADirectoryWhenRootIsAFile guards the documented +// `cryptodeps analyze ./package.json` invocation. +// +// A uriBaseId names a directory. Passing the manifest file through unchanged +// declared a base of "file:///.../package.json/" and made every result relative +// to itself, so each one resolved to the literal ".". That is the same +// unusable-literal defect as the "multiple" path it replaced: a real string in a +// well-formed document that points at nothing. +func TestSARIFBaseIDIsADirectoryWhenRootIsAFile(t *testing.T) { + dir := t.TempDir() + manifest := filepath.Join(dir, "package.json") + writeTestFile(t, manifest, `{"name":"x","dependencies":{"node-forge":"1.3.1"}}`) + + result := &types.ScanResult{ + Project: manifest, + Manifest: manifest, + Ecosystem: types.EcosystemNPM, + Dependencies: []types.DependencyResult{{ + Dependency: types.Dependency{Name: "node-forge", Version: "1.3.1"}, + InDatabase: true, + Analysis: &types.PackageAnalysis{Package: "node-forge", Crypto: []types.CryptoUsage{ + {Algorithm: "DES", QuantumRisk: types.RiskVulnerable, Severity: types.SeverityCritical}, + }}, + }}, + Summary: types.ScanSummary{TotalDependencies: 1, DirectDependencies: 1, WithCrypto: 1}, + } + + var buf bytes.Buffer + if err := (&SARIFFormatter{Options: DefaultOptions()}).Format(result, &buf); err != nil { + t.Fatalf("format: %v", err) + } + + var doc struct { + Runs []struct { + OriginalURIBaseIDs map[string]struct { + URI string `json:"uri"` + } `json:"originalUriBaseIds"` + Results []struct { + Locations []struct { + PhysicalLocation struct { + ArtifactLocation struct { + URI string `json:"uri"` + } `json:"artifactLocation"` + } `json:"physicalLocation"` + } `json:"locations"` + } `json:"results"` + } `json:"runs"` + } + if err := json.Unmarshal(buf.Bytes(), &doc); err != nil { + t.Fatalf("SARIF is not valid JSON: %v", err) + } + if len(doc.Runs[0].Results) == 0 { + t.Fatal("fixture produced no results, so it cannot guard result locations") + } + + base := doc.Runs[0].OriginalURIBaseIDs["SRCROOT"].URI + if strings.HasSuffix(base, "package.json/") { + t.Errorf("SRCROOT names a file, not a directory: %s", base) + } + for _, res := range doc.Runs[0].Results { + uri := res.Locations[0].PhysicalLocation.ArtifactLocation.URI + if uri == "." || uri == "" { + t.Errorf("result location is the literal %q, which points at no file", uri) + } + if uri != "package.json" { + t.Errorf("result location = %q, want %q relative to SRCROOT", uri, "package.json") + } + } +} + +// TestMarkdownRemediationOrderIsStable covers the one format still shuffling +// after the determinism sweep. +// +// The other four were made deterministic, and the change was described as +// covering all five, but the markdown remediation table ranged over a map. Ten +// runs of the same scan produced ten different documents. +func TestMarkdownRemediationOrderIsStable(t *testing.T) { + result := &types.ScanResult{ + Project: "/repo", + Manifest: "/repo/package.json", + Ecosystem: types.EcosystemNPM, + Dependencies: []types.DependencyResult{{ + Dependency: types.Dependency{Name: "node-forge", Version: "1.3.1"}, + InDatabase: true, + Analysis: &types.PackageAnalysis{Package: "node-forge", Crypto: []types.CryptoUsage{ + {Algorithm: "DES", QuantumRisk: types.RiskVulnerable, Severity: types.SeverityCritical, Remediation: "Replace with AES-256"}, + {Algorithm: "MD5", QuantumRisk: types.RiskVulnerable, Severity: types.SeverityCritical, Remediation: "Replace with SHA-256"}, + {Algorithm: "RSA", QuantumRisk: types.RiskVulnerable, Severity: types.SeverityHigh, Remediation: "Migrate to ML-KEM"}, + {Algorithm: "SHA-1", QuantumRisk: types.RiskVulnerable, Severity: types.SeverityHigh, Remediation: "Replace with SHA-256"}, + {Algorithm: "3DES", QuantumRisk: types.RiskVulnerable, Severity: types.SeverityHigh, Remediation: "Replace with AES-256"}, + {Algorithm: "AES", QuantumRisk: types.RiskPartial, Severity: types.SeverityInfo, Remediation: "Use AES-256"}, + }}, + }}, + Summary: types.ScanSummary{TotalDependencies: 1, DirectDependencies: 1, WithCrypto: 1, QuantumVulnerable: 5, QuantumPartial: 1}, + } + + var first string + for i := 0; i < 20; i++ { + var buf bytes.Buffer + if err := (&MarkdownFormatter{Options: DefaultOptions()}).Format(result, &buf); err != nil { + t.Fatalf("format: %v", err) + } + if i == 0 { + first = buf.String() + continue + } + if buf.String() != first { + t.Fatalf("markdown output changed between runs of the same scan; " + + "reproducible reports and golden-file CI both depend on it being stable") + } + } + // Guard the fixture: without a remediation table there is nothing to shuffle. + if !strings.Contains(first, "Remediation Guidance") { + t.Fatal("fixture produced no remediation table, so it cannot guard its ordering") + } +} + +// TestWithheldFindingsAreReportedEvenWhenSomeSurvive is the regression test for +// the case the whole filtered-scan fix missed. +// +// Withholding findings is a property of the scan, not of an empty report, but it +// was routed through classifyNoFindings, which by contract only speaks about +// scans that produced nothing. So the moment one finding survived the filter, +// SARIF and CBOM stopped mentioning the withheld ones entirely: 28 withheld +// beside 38 reported, and a machine consumer read the run as complete. The table +// and JSON said it plainly, which is what made the divergence invisible in +// review. +func TestWithheldFindingsAreReportedEvenWhenSomeSurvive(t *testing.T) { + partial := types.AggregateResults("/repo", []*types.ScanResult{{ + Project: "/repo/a", + Manifest: "/repo/a/package.json", + Ecosystem: types.EcosystemNPM, + Dependencies: []types.DependencyResult{{ + Dependency: types.Dependency{Name: "node-forge", Version: "1.3.1"}, + InDatabase: true, + Analysis: &types.PackageAnalysis{Package: "node-forge", Crypto: []types.CryptoUsage{ + {Algorithm: "DES", QuantumRisk: types.RiskVulnerable, Severity: types.SeverityCritical}, + }}, + }}, + Summary: types.ScanSummary{TotalDependencies: 1, DirectDependencies: 1, WithCrypto: 1, + QuantumVulnerable: 1, FilteredOut: 7}, + }}) + + // Guard the fixture: findings must SURVIVE, or this is the filtered-to-empty + // case that was already covered and the test proves nothing. + if !hasAnyCrypto(partial.Projects[0].Dependencies) { + t.Fatal("fixture has no surviving finding, so it cannot exercise the partial-filter case") + } + + for _, format := range []Format{FormatTable, FormatMarkdown, FormatJSON, FormatCBOM, FormatSARIF} { + t.Run(string(format), func(t *testing.T) { + out := renderMulti(t, format, partial) + // The named field, per format. The first version of this test + // asserted strings.Contains(out, "7"), which the scan timestamp and + // the CBOM's random serial number satisfy on their own: it passed + // against an implementation that reported no withheld findings at + // all, which is the entire defect it was written for. + switch format { + case FormatTable, FormatMarkdown: + assertWithheldSentence(t, format, out, 7, partiallyFiltered(format, 7)...) + default: + withheldAssertion[format](t, out, 7) + } + if !strings.Contains(out, "DES") { + t.Fatalf("fixture produced no surviving finding in %s:\n%s", format, out) + } + }) + } +} + +// TestCoverageNotesAreEmittedPerProjectNotJustFirst pins the plural. +// +// The previous per-project test had exactly one note-producing project, so an +// implementation returning after the first note passed it. +func TestCoverageNotesAreEmittedPerProjectNotJustFirst(t *testing.T) { + mk := func(manifest string) *types.ScanResult { + return &types.ScanResult{ + Manifest: manifest, + Dependencies: []types.DependencyResult{ + {Dependency: types.Dependency{Name: "left-pad"}}, + }, + Summary: types.ScanSummary{TotalDependencies: 1, DirectDependencies: 1, NotInDatabase: 1}, + } + } + multi := types.AggregateResults("/repo", []*types.ScanResult{ + mk("/repo/a/package.json"), mk("/repo/b/package.json"), mk("/repo/c/package.json"), + }) + + notes := coverageNotes(multi.Projects) + if len(notes) != 3 { + t.Fatalf("got %d coverage notes, want 3 (one per unexamined project); an "+ + "implementation that stops after the first would satisfy a single-project fixture", len(notes)) + } + for _, format := range []Format{FormatSARIF, FormatCBOM} { + t.Run(string(format), func(t *testing.T) { + out := renderMulti(t, format, multi) + for _, name := range []string{"a/package.json", "b/package.json", "c/package.json"} { + if !strings.Contains(out, name) { + t.Errorf("%s does not attribute a coverage note to %s:\n%s", format, name, out) + } + } + }) + } +} + +// renderSingle formats one project the way `--no-workspaces` does. +func renderSingle(t *testing.T, format Format, result *types.ScanResult) string { + t.Helper() + f, err := GetFormatter(format) + if err != nil { + t.Fatalf("GetFormatter(%s): %v", format, err) + } + var buf bytes.Buffer + if err := f.Format(result, &buf); err != nil { + t.Fatalf("Format(%s): %v", format, err) + } + return buf.String() +} + +// TestSingleProjectReportsAlsoStateWithheldFindings covers the rendering path +// that had no test at all. +// +// `cryptodeps analyze --no-workspaces` calls Format, not FormatMulti, so +// none of the workspace-level statements exist on it: the markdown Overview +// table with its "Withheld by filter" row is not rendered, and the one line that +// says findings were withheld is the only one there is. Deleting it left the +// whole suite green, because the workspace test was satisfied by the Overview row +// instead. +func TestSingleProjectReportsAlsoStateWithheldFindings(t *testing.T) { + partial := &types.ScanResult{ + Project: "/repo/a", + Manifest: "/repo/a/package.json", + Ecosystem: types.EcosystemNPM, + Dependencies: []types.DependencyResult{{ + Dependency: types.Dependency{Name: "node-forge", Version: "1.3.1"}, + InDatabase: true, + Analysis: &types.PackageAnalysis{Package: "node-forge", Crypto: []types.CryptoUsage{ + {Algorithm: "DES", QuantumRisk: types.RiskVulnerable, Severity: types.SeverityCritical}, + }}, + }}, + Summary: types.ScanSummary{TotalDependencies: 1, DirectDependencies: 1, WithCrypto: 1, + QuantumVulnerable: 1, FilteredOut: 7}, + } + // Guard the fixture: a finding must survive, or this is the filtered-to-empty + // case that the verdict already covers. + if !hasAnyCrypto(partial.Dependencies) { + t.Fatal("fixture has no surviving finding, so it cannot exercise the partial-filter case") + } + + for _, format := range []Format{FormatTable, FormatMarkdown} { + t.Run(string(format), func(t *testing.T) { + out := renderSingle(t, format, partial) + if !strings.Contains(out, "DES") { + t.Fatalf("fixture produced no surviving finding in %s:\n%s", format, out) + } + // The single-project wording, which is the first of the two + // statements the workspace report makes. + want := "FILTERED: 7 further finding(s) excluded by --risk or --min-severity" + if format == FormatMarkdown { + want = "> 7 further finding(s) were excluded by `--risk` or `--min-severity`" + } + if !strings.Contains(out, want) { + t.Errorf("%s single-project report does not state %q, so a --no-workspaces "+ + "scan reads as complete while 7 findings were withheld:\n%s", format, want, out) + } + }) + } +} diff --git a/pkg/output/json.go b/pkg/output/json.go index 963e240..3f6aa4a 100644 --- a/pkg/output/json.go +++ b/pkg/output/json.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 package output @@ -9,6 +9,7 @@ import ( "io" "github.com/csnp/qramm-cryptodeps/pkg/types" + "github.com/csnp/qramm-cryptodeps/pkg/version" ) // JSONFormatter formats scan results as JSON. @@ -16,6 +17,36 @@ type JSONFormatter struct { Indent bool } +// jsonTool identifies the build that produced a document. Consumers need this +// to attribute a result to a scanner version; without it a stored report cannot +// be told apart from one produced by a build with different detection rules. +// +// This is deliberately not the same field as a dependency's +// analysis.toolVersion, which records who produced that database record, not who +// ran the scan. +type jsonTool struct { + Name string `json:"name"` + Version string `json:"version"` +} + +func currentTool() jsonTool { + return jsonTool{Name: version.Name, Version: version.Version()} +} + +// jsonScanDocument is a ScanResult with the emitting tool's identity attached. +// The embedded pointer inlines the scan result's own fields, so the shape is +// the previous one plus a "tool" object. +type jsonScanDocument struct { + Tool jsonTool `json:"tool"` + *types.ScanResult +} + +// jsonMultiDocument is the same wrapper for multi-project results. +type jsonMultiDocument struct { + Tool jsonTool `json:"tool"` + *types.MultiProjectResult +} + // Format writes the scan result as JSON. func (f *JSONFormatter) Format(result *types.ScanResult, w io.Writer) error { if result == nil { @@ -24,11 +55,7 @@ func (f *JSONFormatter) Format(result *types.ScanResult, w io.Writer) error { if w == nil { return errors.New("writer cannot be nil") } - encoder := json.NewEncoder(w) - if f.Indent { - encoder.SetIndent("", " ") - } - return encoder.Encode(result) + return f.encode(w, jsonScanDocument{Tool: currentTool(), ScanResult: result}) } // FormatMulti writes multi-project scan results as JSON. @@ -39,9 +66,13 @@ func (f *JSONFormatter) FormatMulti(result *types.MultiProjectResult, w io.Write if w == nil { return errors.New("writer cannot be nil") } + return f.encode(w, jsonMultiDocument{Tool: currentTool(), MultiProjectResult: result}) +} + +func (f *JSONFormatter) encode(w io.Writer, doc any) error { encoder := json.NewEncoder(w) if f.Indent { encoder.SetIndent("", " ") } - return encoder.Encode(result) + return encoder.Encode(doc) } diff --git a/pkg/output/markdown.go b/pkg/output/markdown.go index 6a53b93..2062566 100644 --- a/pkg/output/markdown.go +++ b/pkg/output/markdown.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 package output @@ -7,6 +7,7 @@ import ( "errors" "fmt" "io" + "sort" "github.com/csnp/qramm-cryptodeps/pkg/types" ) @@ -18,6 +19,15 @@ type MarkdownFormatter struct { // Format writes the scan result as Markdown. func (f *MarkdownFormatter) Format(result *types.ScanResult, w io.Writer) error { + return f.formatProject(result, "", w) +} + +// formatProject renders one project. root is the scan root when this render is +// part of a workspace report and empty when it stands alone, so that a manifest +// is named the same way here as in the project list above it. Threaded as an +// argument rather than held on the formatter, which is what cbom.go does, so the +// formatter stays stateless. +func (f *MarkdownFormatter) formatProject(result *types.ScanResult, root string, w io.Writer) error { if result == nil { return errors.New("result cannot be nil") } @@ -31,7 +41,7 @@ func (f *MarkdownFormatter) Format(result *types.ScanResult, w io.Writer) error fmt.Fprintf(w, "## Summary\n\n") fmt.Fprintf(w, "| Metric | Value |\n") fmt.Fprintf(w, "|--------|-------|\n") - fmt.Fprintf(w, "| **Manifest** | `%s` |\n", result.Manifest) + fmt.Fprintf(w, "| **Manifest** | `%s` |\n", markdownCell(manifestForReport(root, result.Manifest))) fmt.Fprintf(w, "| **Ecosystem** | %s |\n", result.Ecosystem) fmt.Fprintf(w, "| **Total Dependencies** | %d |\n", result.Summary.TotalDependencies) fmt.Fprintf(w, "| **Using Crypto** | %d |\n", result.Summary.WithCrypto) @@ -40,18 +50,16 @@ func (f *MarkdownFormatter) Format(result *types.ScanResult, w io.Writer) error fmt.Fprintf(w, "| **Not in Database** | %d |\n", result.Summary.NotInDatabase) fmt.Fprintf(w, "\n") - // Check if there are any crypto findings - hasCrypto := false - for _, dep := range result.Dependencies { - if dep.Analysis != nil && len(dep.Analysis.Crypto) > 0 { - hasCrypto = true - break - } + if !hasAnyCrypto(result.Dependencies) { + writeMarkdownNoFindingsVerdict(w, result.Summary) + return nil } - if !hasCrypto { - fmt.Fprintf(w, "No cryptographic usage detected in dependencies.\n") - return nil + // A filtered report describes only what survived the filter, so say so next + // to the numbers rather than letting the table above read as complete. + if result.Summary.FilteredOut > 0 { + fmt.Fprintf(w, "> %d further finding(s) were excluded by `--risk` or `--min-severity`.\n\n", + result.Summary.FilteredOut) } // Findings by risk level @@ -74,9 +82,14 @@ func (f *MarkdownFormatter) Format(result *types.ScanResult, w io.Writer) error fmt.Fprintf(w, "|------------|-----------|------|----------|\n") hasVulnerable = true } - fmt.Fprintf(w, "| %s@%s | %s | %s | %s |\n", - dep.Dependency.Name, - dep.Dependency.Version, + // The name and the version come from the manifest being + // scanned, so they are attacker-controlled in exactly the way a + // path is, and they need no exotic filesystem to deliver: a + // dependency entry in a pull request is enough. A version of + // "1.3.1\n\n## Scan result: CLEAN" put that heading in this + // report, in a table row, seven times. + fmt.Fprintf(w, "| `%s` | %s | %s | %s |\n", + markdownCell(dependencyLabel(dep.Dependency.Name, dep.Dependency.Version)), crypto.Algorithm, crypto.Type, crypto.Severity, @@ -106,9 +119,14 @@ func (f *MarkdownFormatter) Format(result *types.ScanResult, w io.Writer) error fmt.Fprintf(w, "|------------|-----------|------|----------|\n") hasPartial = true } - fmt.Fprintf(w, "| %s@%s | %s | %s | %s |\n", - dep.Dependency.Name, - dep.Dependency.Version, + // The name and the version come from the manifest being + // scanned, so they are attacker-controlled in exactly the way a + // path is, and they need no exotic filesystem to deliver: a + // dependency entry in a pull request is enough. A version of + // "1.3.1\n\n## Scan result: CLEAN" put that heading in this + // report, in a table row, seven times. + fmt.Fprintf(w, "| `%s` | %s | %s | %s |\n", + markdownCell(dependencyLabel(dep.Dependency.Name, dep.Dependency.Version)), crypto.Algorithm, crypto.Type, crypto.Severity, @@ -129,8 +147,16 @@ func (f *MarkdownFormatter) Format(result *types.ScanResult, w io.Writer) error fmt.Fprintf(w, "## Remediation Guidance\n\n") fmt.Fprintf(w, "| Algorithm | Recommended Action |\n") fmt.Fprintf(w, "|-----------|--------------------|\n") - for algo, remediation := range remediationMap { - fmt.Fprintf(w, "| **%s** | %s |\n", algo, remediation) + // Sorted, not ranged: Go randomises map iteration, so this table was the + // one part of the report that still shuffled between runs of the same + // scan after the other four formats were made deterministic. + algos := make([]string, 0, len(remediationMap)) + for algo := range remediationMap { + algos = append(algos, algo) + } + sort.Strings(algos) + for _, algo := range algos { + fmt.Fprintf(w, "| **%s** | %s |\n", algo, remediationMap[algo]) } fmt.Fprintf(w, "\n") } @@ -159,6 +185,34 @@ func (f *MarkdownFormatter) Format(result *types.ScanResult, w io.Writer) error return nil } +// writeMarkdownNoFindingsVerdict states what a findings-free scan established. +// It renders the shared classification in markdown prose; the wording differs +// from the table's, the meaning must not. +func writeMarkdownNoFindingsVerdict(w io.Writer, s types.ScanSummary) { + switch classifyNoFindings(s) { + case caseFiltered: + fmt.Fprintf(w, "**No findings matched the active filter.** %d finding(s) were detected and "+ + "excluded by `--risk` or `--min-severity`. This is not a clean result. "+ + "Re-run without the filter to see them.\n", s.FilteredOut) + + case caseNoDependencies: + fmt.Fprintf(w, "**No dependencies found in this manifest.** Nothing to analyze.\n") + + case caseNothingExamined: + fmt.Fprintf(w, "**Not analyzed.** All %d dependencies are absent from the crypto database, "+ + "so no conclusion about cryptographic usage can be drawn from this scan. "+ + "Run with `--deep` to analyze package source code directly.\n", s.TotalDependencies) + + default: + fmt.Fprintf(w, "No cryptographic usage detected in the %d of %d dependencies that were analyzed.\n", + s.TotalDependencies-s.NotInDatabase, s.TotalDependencies) + if s.NotInDatabase > 0 { + fmt.Fprintf(w, "\n%d not in database, so they were not examined (use `--deep` to analyze).\n", + s.NotInDatabase) + } + } +} + // FormatMulti writes multi-project scan results as Markdown. func (f *MarkdownFormatter) FormatMulti(result *types.MultiProjectResult, w io.Writer) error { if result == nil { @@ -171,32 +225,81 @@ func (f *MarkdownFormatter) FormatMulti(result *types.MultiProjectResult, w io.W // Title fmt.Fprintf(w, "# CryptoDeps Multi-Project Scan Report\n\n") + // Manifests that were found but not read change how every number below + // should be read, so they are stated before the overview rather than in a + // footnote. + // Split by kind, for the same reason the table does: counting an unreadable + // manifest and an unsupported ecosystem together produced a number that + // disagreed with the CBOM for the same run, and asserted that a healthy + // Cargo.toml "could not be read". + var unread, unsupported []types.SkippedManifest + for _, s := range result.Skipped { + if s.Unsupported { + unsupported = append(unsupported, s) + } else { + unread = append(unread, s) + } + } + if len(unread) > 0 { + fmt.Fprintf(w, "## Not analyzed\n\n") + fmt.Fprintf(w, "%d manifest file(s) were found but could not be read. "+ + "The dependencies they declare are missing from this report.\n\n", len(unread)) + fmt.Fprintf(w, "| Manifest | Reason |\n") + fmt.Fprintf(w, "|----------|--------|\n") + for _, s := range unread { + // The reason is a code span too: it is an error string that quotes the + // path back, so it carries the same untrusted bytes as the cell beside it. + fmt.Fprintf(w, "| `%s` | `%s` |\n", markdownCell(getRelativePath(result.RootPath, s.Path)), markdownCell(s.Reason)) + } + fmt.Fprintf(w, "\n") + } + if len(unsupported) > 0 { + fmt.Fprintf(w, "## Unsupported ecosystems\n\n") + fmt.Fprintf(w, "%d manifest file(s) belong to ecosystems cryptodeps does not parse. "+ + "Their dependencies were not analyzed, and this does not affect the exit code.\n\n", + len(unsupported)) + for _, s := range unsupported { + fmt.Fprintf(w, "- `%s`\n", markdownCode(getRelativePath(result.RootPath, s.Path))) + } + fmt.Fprintf(w, "\n") + } + // Overview fmt.Fprintf(w, "## Overview\n\n") fmt.Fprintf(w, "| Metric | Value |\n") fmt.Fprintf(w, "|--------|-------|\n") - fmt.Fprintf(w, "| **Root Path** | `%s` |\n", result.RootPath) + fmt.Fprintf(w, "| **Root Path** | `%s` |\n", markdownCell(scanRootDir(result.RootPath))) fmt.Fprintf(w, "| **Projects Scanned** | %d |\n", len(result.Projects)) fmt.Fprintf(w, "| **Total Dependencies** | %d |\n", result.TotalSummary.TotalDependencies) fmt.Fprintf(w, "| **Using Crypto** | %d |\n", result.TotalSummary.WithCrypto) fmt.Fprintf(w, "| **Quantum Vulnerable** | %d |\n", result.TotalSummary.QuantumVulnerable) fmt.Fprintf(w, "| **Quantum Partial** | %d |\n", result.TotalSummary.QuantumPartial) + if result.TotalSummary.FilteredOut > 0 { + fmt.Fprintf(w, "| **Withheld by filter** | %d |\n", result.TotalSummary.FilteredOut) + } fmt.Fprintf(w, "\n") + if result.TotalSummary.FilteredOut > 0 { + fmt.Fprintf(w, "> Every number above describes only the findings that survived "+ + "`--risk` or `--min-severity`. %d were withheld.\n\n", result.TotalSummary.FilteredOut) + } // Project list fmt.Fprintf(w, "### Projects\n\n") for _, project := range result.Projects { - fmt.Fprintf(w, "- `%s` (%s)\n", project.Manifest, project.Ecosystem) + fmt.Fprintf(w, "- `%s` (%s)\n", markdownCode(getRelativePath(result.RootPath, project.Manifest)), project.Ecosystem) } fmt.Fprintf(w, "\n") // Individual project reports for _, project := range result.Projects { fmt.Fprintf(w, "---\n\n") - fmt.Fprintf(w, "## %s\n\n", project.Manifest) + // In a code span, like every other path in this document. A bare heading + // renders whatever the name contains, and a directory can be named + // "**CLEAN**" or "[no findings](https://...)". + fmt.Fprintf(w, "## `%s`\n\n", markdownCode(getRelativePath(result.RootPath, project.Manifest))) // Use the single-project formatter for detailed output - if err := f.Format(project, w); err != nil { + if err := f.formatProject(project, result.RootPath, w); err != nil { return err } } diff --git a/pkg/output/ordering_test.go b/pkg/output/ordering_test.go new file mode 100644 index 0000000..5700b63 --- /dev/null +++ b/pkg/output/ordering_test.go @@ -0,0 +1,76 @@ +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) +// SPDX-License-Identifier: Apache-2.0 + +package output + +import ( + "bytes" + "testing" + + "github.com/csnp/qramm-cryptodeps/pkg/types" +) + +// resultWithCrypto builds a scan result whose findings are supplied in the given +// order, all at the same risk level. +func resultWithCrypto(algorithms []string, risk types.QuantumRisk) *types.ScanResult { + crypto := make([]types.CryptoUsage, 0, len(algorithms)) + for _, a := range algorithms { + crypto = append(crypto, types.CryptoUsage{ + Algorithm: a, + QuantumRisk: risk, + Severity: types.SeverityHigh, + }) + } + return &types.ScanResult{ + Manifest: "package.json", + Ecosystem: types.EcosystemNPM, + Dependencies: []types.DependencyResult{{ + Dependency: types.Dependency{Name: "node-forge", Version: "1.3.1"}, + InDatabase: true, + Analysis: &types.PackageAnalysis{Package: "node-forge", Crypto: crypto}, + }}, + Summary: types.ScanSummary{ + TotalDependencies: 1, WithCrypto: 1, QuantumVulnerable: len(algorithms), + }, + } +} + +// TestTableOrderDoesNotDependOnInputOrder is the regression test for a sort that +// was not total. +// +// Findings were ordered on risk alone, and sort.Slice is not stable, so every +// same-risk finding sat in an arbitrary relative position. Two scans that found +// the same things in a different order printed different reports, which makes +// the text report's numbering meaningless and breaks any diff-based CI check. +func TestTableOrderDoesNotDependOnInputOrder(t *testing.T) { + forward := []string{"RSA", "DES", "3DES", "MD5", "SHA-1", "DSA", "ECDSA"} + reversed := make([]string, len(forward)) + for i, a := range forward { + reversed[len(forward)-1-i] = a + } + + render := func(order []string) string { + var buf bytes.Buffer + result := resultWithCrypto(order, types.RiskVulnerable) + if err := (&TableFormatter{Options: DefaultOptions()}).Format(result, &buf); err != nil { + t.Fatalf("format: %v", err) + } + return buf.String() + } + + first := render(forward) + if first == "" { + t.Fatal("fixture rendered nothing") + } + if got := render(reversed); got != first { + t.Errorf("report depends on the order findings arrived in.\n--- forward ---\n%s\n--- reversed ---\n%s", + first, got) + } + + // Rendering the same input repeatedly must also be stable. + for i := 0; i < 10; i++ { + if got := render(forward); got != first { + t.Fatalf("repeated render of identical input differs on iteration %d", i) + } + } +} diff --git a/pkg/output/output.go b/pkg/output/output.go index f0abcc3..87917ed 100644 --- a/pkg/output/output.go +++ b/pkg/output/output.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 // Package output provides formatters for scan results. diff --git a/pkg/output/output_test.go b/pkg/output/output_test.go index 4972bd0..ddc19c3 100644 --- a/pkg/output/output_test.go +++ b/pkg/output/output_test.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 package output diff --git a/pkg/output/paths.go b/pkg/output/paths.go new file mode 100644 index 0000000..f380a43 --- /dev/null +++ b/pkg/output/paths.go @@ -0,0 +1,164 @@ +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) +// SPDX-License-Identifier: Apache-2.0 + +package output + +import ( + "os" + "path/filepath" + "strconv" + "strings" +) + +// scanRootDir normalizes a scan root into the absolute directory that manifest +// paths are expressed relative to. +// +// Two shapes reach the formatters, and both have to be handled before any +// comparison against a manifest path is meaningful. Discovery absolutizes every +// manifest path, while the scan root is whatever the user typed: `cryptodeps +// analyze .` leaves the literal "." in RootPath, so relativizing against it +// silently produced no relative path at all and the absolute one was emitted +// instead. And `cryptodeps analyze ./package.json` puts a FILE in the root, +// which as a base directory made every path relative to itself. +// +// SARIF handled both and CBOM handled neither, which is why this lives in one +// place that both call rather than in each formatter. +func scanRootDir(root string) string { + abs, err := filepath.Abs(root) + if err != nil { + return root + } + if info, statErr := os.Stat(abs); statErr == nil && !info.IsDir() { + abs = filepath.Dir(abs) + } + return abs +} + +// relativeToRoot expresses a manifest path relative to an absolute scan root, so +// that a document leaving this machine carries the repository layout rather than +// the operator's home directory or a CI runner's workspace path. +// +// absRoot must come from scanRootDir. The second return reports whether the +// manifest actually sits under the root; when it does not, the caller gets the +// absolute path, because a relative path would be a lie. Callers that need a URI +// add their own scheme: this returns a plain path. +func relativeToRoot(absRoot, manifest string) (path string, underRoot bool) { + if manifest == "" { + return "", false + } + absManifest, err := filepath.Abs(manifest) + if err != nil { + return filepath.ToSlash(manifest), false + } + rel, err := filepath.Rel(absRoot, absManifest) + // rel == ".." and the "../" prefix mean the manifest is outside the root. A + // bare HasPrefix(rel, "..") would also reject a directory legitimately named + // something like "..config". + if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + return filepath.ToSlash(absManifest), false + } + return filepath.ToSlash(rel), true +} + +// reportSafe renders a filesystem path, or an error string quoting one, for a +// plain-text report. +// +// Both are attacker-controlled. Any repository can hold a directory whose name +// carries newlines, and the scan report is published by the bundled GitHub +// Action, so a scanned repository could write its own lines into the document +// that judges it: a directory named with an embedded "## Scan result: CLEAN" +// put exactly that heading in the markdown report, above the real findings. +// Anything carrying a control character, a backtick or a pipe is rendered as a +// Go-quoted string, which is single-line, unambiguous and reversible. A path +// with none of those, which is every real one, is returned unchanged. +func reportSafe(s string) string { + if !needsEscaping(s) { + return s + } + return strconv.Quote(s) +} + +// markdownCode renders a string for the inside of a markdown code span that is +// not in a table. +// +// Its callers must put the result in one. That is the whole defence, and it is +// why this is not a list of markdown metacharacters to escape: inside a code +// span only a backtick is active, and it would close the span early. Everywhere +// else every markdown construct is live, which is how the first version of this +// missed a path named "**CLEAN**": it had no trigger character, so it reached a +// bare `##` heading unescaped and rendered as bold. +func markdownCode(s string) string { + return strings.ReplaceAll(reportSafe(s), "`", `\x60`) +} + +// markdownCell renders a string for a code span inside a GitHub-flavoured +// markdown table cell, where a pipe splits the cell even inside the span. +// +// Separate from markdownCode because the pipe escape is a table rule and nothing +// else consumes it: applied to a bullet or a heading, the backslash renders +// literally, so the report displayed a name the filesystem does not have and the +// quoted form no longer round-tripped through strconv.Unquote. +func markdownCell(s string) string { + return strings.ReplaceAll(markdownCode(s), "|", `\|`) +} + +// needsEscaping reports whether a string can break out of the line, the code +// span or the table cell it is about to be rendered into, or misrepresent what +// it names. +// +// ASCII control characters are not the whole set. U+2028 and U+2029 are line +// breaks to a renderer, so they inject lines exactly as \n does. The bidi +// controls reverse the visible order of a name, which is the Trojan Source +// trick: a report can be made to display a filename that is not the one it is +// talking about. The zero-width characters make two different paths render +// identically. U+FFFD is what invalid UTF-8 in a filename decodes to, and a +// filesystem does not require valid UTF-8. +func needsEscaping(s string) bool { + for _, r := range s { + switch { + // Everything Go does not consider printable: the ASCII controls, DEL, + // U+0085, the Unicode line and paragraph separators, every bidi and + // format control, the zero-width set, and the non-ASCII spaces that + // render as an ordinary one. Enumerating these by hand missed U+061C, + // U+2060, U+180E, the tag block and U+00A0 on the first attempt, all of + // which belong to the classes the enumeration claimed to cover. + case !strconv.IsPrint(r): + return true + // Printable, but active in the context this is rendered into. + case r == '`', r == '|': + return true + // Printable, and what invalid UTF-8 in a filename decodes to. It injects + // nothing; quoting marks the name as a rendering rather than the literal + // bytes, which cannot be recovered. + case r == 0xfffd: + return true + } + } + return false +} + +// manifestForReport names a manifest inside a report. +// +// With a scan root it is expressed relative to it, matching every other surface +// of the same run. Standalone, which is what `--no-workspaces` and a +// single-manifest scan produce, there is no root to express it against and the +// absolute path is the only anchor the reader has. +func manifestForReport(root, manifest string) string { + if root == "" { + return manifest + } + return getRelativePath(root, manifest) +} + +// dependencyLabel renders "name@version", or just the name when no version was +// declared. +// +// One place, because both the table and markdown build this string and both had +// it interpolated raw. The pieces come from the manifest under scan, so they +// carry whatever the author of that manifest put in them. +func dependencyLabel(name, version string) string { + if version == "" { + return name + } + return name + "@" + version +} diff --git a/pkg/output/paths_test.go b/pkg/output/paths_test.go new file mode 100644 index 0000000..046cc57 --- /dev/null +++ b/pkg/output/paths_test.go @@ -0,0 +1,227 @@ +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) +// SPDX-License-Identifier: Apache-2.0 + +package output + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/csnp/qramm-cryptodeps/pkg/types" +) + +// chdir moves into dir for the duration of the test and returns the working +// directory as the OS reports it, which on macOS is the resolved path behind +// /var. Both sides of a path comparison have to come from the same place or the +// assertion measures the symlink rather than the code. +func chdir(t *testing.T, dir string) string { + t.Helper() + previous, err := os.Getwd() + if err != nil { + t.Fatalf("getwd: %v", err) + } + if err := os.Chdir(dir); err != nil { + t.Fatalf("chdir %s: %v", dir, err) + } + t.Cleanup(func() { + if err := os.Chdir(previous); err != nil { + t.Fatalf("restore cwd: %v", err) + } + }) + cwd, err := os.Getwd() + if err != nil { + t.Fatalf("getwd after chdir: %v", err) + } + return cwd +} + +// skippedUnderRelativeRoot builds the scan a user gets from `cryptodeps analyze .`: +// a root exactly as typed, and manifest paths absolutized by discovery. +func skippedUnderRelativeRoot(root string) *types.MultiProjectResult { + return &types.MultiProjectResult{ + RootPath: ".", + Projects: []*types.ScanResult{{ + Project: filepath.Join(root, "good"), + Manifest: filepath.Join(root, "good", "package.json"), + Ecosystem: types.EcosystemNPM, + Dependencies: []types.DependencyResult{{ + Dependency: types.Dependency{Name: "node-forge", Version: "1.3.1"}, + InDatabase: true, + Analysis: &types.PackageAnalysis{Package: "node-forge", Crypto: []types.CryptoUsage{ + {Algorithm: "DES", QuantumRisk: types.RiskVulnerable, Severity: types.SeverityCritical}, + }}, + }}, + Summary: types.ScanSummary{TotalDependencies: 1, DirectDependencies: 1, WithCrypto: 1, + QuantumVulnerable: 1}, + }}, + Skipped: []types.SkippedManifest{ + {Path: filepath.Join(root, "corrupt", "package.json"), + Reason: "not valid JSON: unexpected end of JSON input"}, + }, + } +} + +// TestCBOMPathsAreRelativeUnderARelativeScanRoot is the regression test for a +// privacy fix that only worked for the invocation nobody uses. +// +// The relativization compared an absolute manifest path against the scan root +// exactly as the user typed it. `cryptodeps analyze /abs/path` therefore +// produced repository-relative paths, while `cryptodeps analyze .`, which is the +// default invocation and the one the GitHub Action runs, fell through to the +// absolute path and published the operator's home directory or the CI runner's +// workspace layout inside a document meant to be shared. +func TestCBOMPathsAreRelativeUnderARelativeScanRoot(t *testing.T) { + root := chdir(t, t.TempDir()) + out := renderMulti(t, FormatCBOM, skippedUnderRelativeRoot(root)) + + var doc struct { + Metadata struct { + Properties []struct { + Name string `json:"name"` + Value string `json:"value"` + } `json:"properties"` + } `json:"metadata"` + } + if err := json.Unmarshal([]byte(out), &doc); err != nil { + t.Fatalf("CBOM is not valid JSON: %v", err) + } + + var found bool + for _, p := range doc.Metadata.Properties { + if p.Name != "cryptodeps:manifestNotAnalyzed" { + continue + } + found = true + if !strings.HasPrefix(p.Value, "corrupt/package.json:") { + t.Errorf("cryptodeps:manifestNotAnalyzed = %q, want a path relative to the scan "+ + "root; an absolute path publishes the local filesystem layout", p.Value) + } + if strings.Contains(p.Value, root) { + t.Errorf("cryptodeps:manifestNotAnalyzed carries the absolute scan root %q: %q", root, p.Value) + } + } + // Without this the test passes on an implementation that emits no coverage + // properties at all, which is the defect the properties exist to prevent. + if !found { + t.Fatalf("CBOM has no cryptodeps:manifestNotAnalyzed property, so this cannot "+ + "guard how one is rendered: %+v", doc.Metadata.Properties) + } +} + +// TestSARIFPathsAreRelativeUnderARelativeScanRoot is a preservation guard, not a +// regression test: SARIF absolutized the root before comparing and was correct +// throughout. It exists so the shared helper both formats now call cannot be +// changed to fix CBOM at SARIF's expense. +// +// The absolute root legitimately appears once, in originalUriBaseIds, which is +// what a consumer needs to resolve the relative uris. The uris themselves must +// not repeat it. +func TestSARIFPathsAreRelativeUnderARelativeScanRoot(t *testing.T) { + root := chdir(t, t.TempDir()) + out := renderMulti(t, FormatSARIF, skippedUnderRelativeRoot(root)) + + var doc struct { + Runs []struct { + Invocations []struct { + ToolExecutionNotifications []struct { + Locations []struct { + PhysicalLocation struct { + ArtifactLocation struct { + URI string `json:"uri"` + URIBaseID string `json:"uriBaseId"` + } `json:"artifactLocation"` + } `json:"physicalLocation"` + } `json:"locations"` + } `json:"toolExecutionNotifications"` + } `json:"invocations"` + Results []struct { + Locations []struct { + PhysicalLocation struct { + ArtifactLocation struct { + URI string `json:"uri"` + URIBaseID string `json:"uriBaseId"` + } `json:"artifactLocation"` + } `json:"physicalLocation"` + } `json:"locations"` + } `json:"results"` + } `json:"runs"` + } + if err := json.Unmarshal([]byte(out), &doc); err != nil { + t.Fatalf("SARIF is not valid JSON: %v", err) + } + if len(doc.Runs) != 1 { + t.Fatalf("got %d runs, want 1", len(doc.Runs)) + } + + var checked int + for _, n := range doc.Runs[0].Invocations[0].ToolExecutionNotifications { + for _, l := range n.Locations { + a := l.PhysicalLocation.ArtifactLocation + checked++ + if a.URIBaseID != sarifURIBaseID || strings.Contains(a.URI, root) { + t.Errorf("notification uri %q (base %q) is not relative to the scan root", + a.URI, a.URIBaseID) + } + } + } + for _, r := range doc.Runs[0].Results { + for _, l := range r.Locations { + a := l.PhysicalLocation.ArtifactLocation + checked++ + if a.URIBaseID != sarifURIBaseID || strings.Contains(a.URI, root) { + t.Errorf("result uri %q (base %q) is not relative to the scan root", + a.URI, a.URIBaseID) + } + } + } + if checked == 0 { + t.Fatal("SARIF carried no located notification or result, so this asserts nothing") + } +} + +// TestRelativeToRootKeepsPathsOutsideTheRootAbsolute pins the other half of the +// contract. A manifest that does not sit under the scan root cannot be described +// relative to it, and inventing a "../../.." path would be a lie a consumer +// would then try to resolve. +func TestRelativeToRootKeepsPathsOutsideTheRootAbsolute(t *testing.T) { + root := chdir(t, t.TempDir()) + outside := filepath.Join(filepath.Dir(root), "elsewhere", "package.json") + + got, underRoot := relativeToRoot(root, outside) + if underRoot { + t.Errorf("relativeToRoot(%q, %q) claims the manifest is under the root", root, outside) + } + if !filepath.IsAbs(got) { + t.Errorf("relativeToRoot(%q, %q) = %q, want the absolute path", root, outside, got) + } + + // A directory whose name merely starts with ".." is under the root and must + // stay relative. A HasPrefix(rel, "..") test gets this wrong. + dotted := filepath.Join(root, "..config", "package.json") + got, underRoot = relativeToRoot(root, dotted) + if !underRoot || got != "..config/package.json" { + t.Errorf("relativeToRoot(%q, %q) = %q, %v; want \"..config/package.json\", true", + root, dotted, got, underRoot) + } +} + +// TestScanRootDirTreatsAManifestFileAsItsDirectory guards the second +// normalization. `cryptodeps analyze ./package.json` passes a file as the scan +// root, and using it as a base directory made every path relative to itself. +func TestScanRootDirTreatsAManifestFileAsItsDirectory(t *testing.T) { + dir := chdir(t, t.TempDir()) + manifest := filepath.Join(dir, "package.json") + if err := os.WriteFile(manifest, []byte(`{"name":"x"}`), 0o644); err != nil { + t.Fatalf("write fixture: %v", err) + } + + if got := scanRootDir(manifest); got != dir { + t.Errorf("scanRootDir(%q) = %q, want the containing directory %q", manifest, got, dir) + } + if got := scanRootDir("."); got != dir { + t.Errorf("scanRootDir(\".\") = %q, want the absolute working directory %q", got, dir) + } +} diff --git a/pkg/output/provenance_test.go b/pkg/output/provenance_test.go new file mode 100644 index 0000000..12aa773 --- /dev/null +++ b/pkg/output/provenance_test.go @@ -0,0 +1,271 @@ +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) +// SPDX-License-Identifier: Apache-2.0 + +package output + +import ( + "bytes" + "encoding/json" + "path/filepath" + "strings" + "testing" + + "github.com/csnp/qramm-cryptodeps/pkg/types" + "github.com/csnp/qramm-cryptodeps/pkg/version" +) + +// withVersion sets the reported build version for the duration of a test. +// +// The value is deliberately not any version this tool has ever shipped, so a +// formatter that carries its own literal cannot accidentally agree with it. +func withVersion(t *testing.T, v string) { + t.Helper() + original := version.Version() + version.Set(v, "testcommit", "testdate") + t.Cleanup(func() { version.Set(original, "", "") }) +} + +// sampleResult builds a scan result with one vulnerable finding. +func sampleResult(projectDir, manifestPath string) *types.ScanResult { + return &types.ScanResult{ + Project: projectDir, + Manifest: manifestPath, + Ecosystem: types.EcosystemNPM, + Dependencies: []types.DependencyResult{ + { + Dependency: types.Dependency{ + Name: "node-forge", Version: "1.3.1", + Ecosystem: types.EcosystemNPM, Direct: true, + }, + InDatabase: true, + Analysis: &types.PackageAnalysis{ + Package: "node-forge", + Crypto: []types.CryptoUsage{ + { + Algorithm: "RSA", + Type: "key-exchange", + QuantumRisk: types.RiskVulnerable, + Severity: types.SeverityHigh, + }, + }, + }, + }, + }, + Summary: types.ScanSummary{TotalDependencies: 1, WithCrypto: 1, QuantumVulnerable: 1}, + } +} + +// TestEveryEmitterReportsTheRunningVersion pins the single source of truth. +// +// Before pkg/version existed, one binary gave four answers: the version command +// said 1.3.0, SARIF said 1.0.0, CBOM said 1.0.0 and JSON carried no version at +// all. SARIF and CBOM are provenance artifacts, so a stale literal there is a +// false record of what produced the document. +func TestEveryEmitterReportsTheRunningVersion(t *testing.T) { + const want = "9.9.9-provenance-test" + withVersion(t, want) + + dir := t.TempDir() + result := sampleResult(dir, filepath.Join(dir, "package.json")) + + t.Run("sarif", func(t *testing.T) { + var buf bytes.Buffer + if err := (&SARIFFormatter{}).Format(result, &buf); err != nil { + t.Fatalf("format: %v", err) + } + var doc struct { + Runs []struct { + Tool struct { + Driver struct { + Version string `json:"version"` + SemanticVersion string `json:"semanticVersion"` + } `json:"driver"` + } `json:"tool"` + } `json:"runs"` + } + if err := json.Unmarshal(buf.Bytes(), &doc); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if got := doc.Runs[0].Tool.Driver.Version; got != want { + t.Errorf("sarif driver.version = %q, want %q", got, want) + } + if got := doc.Runs[0].Tool.Driver.SemanticVersion; got != want { + t.Errorf("sarif driver.semanticVersion = %q, want %q", got, want) + } + }) + + t.Run("cbom", func(t *testing.T) { + var buf bytes.Buffer + if err := (&CBOMFormatter{}).Format(result, &buf); err != nil { + t.Fatalf("format: %v", err) + } + var doc struct { + Metadata struct { + Tools []struct { + Version string `json:"version"` + } `json:"tools"` + } `json:"metadata"` + } + if err := json.Unmarshal(buf.Bytes(), &doc); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if len(doc.Metadata.Tools) == 0 { + t.Fatal("cbom declared no tools") + } + if got := doc.Metadata.Tools[0].Version; got != want { + t.Errorf("cbom metadata.tools[0].version = %q, want %q", got, want) + } + }) + + t.Run("json", func(t *testing.T) { + var buf bytes.Buffer + if err := (&JSONFormatter{}).Format(result, &buf); err != nil { + t.Fatalf("format: %v", err) + } + var doc struct { + Tool struct { + Name string `json:"name"` + Version string `json:"version"` + } `json:"tool"` + Manifest string `json:"manifest"` + } + if err := json.Unmarshal(buf.Bytes(), &doc); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if got := doc.Tool.Version; got != want { + t.Errorf("json tool.version = %q, want %q", got, want) + } + if doc.Tool.Name == "" { + t.Error("json tool.name is empty") + } + // The wrapper must not shadow the scan result's own fields. + if doc.Manifest == "" { + t.Error("json lost the manifest field when the tool object was added") + } + }) +} + +// TestSARIFLocationsPointAtRealManifests guards the "multiple" literal. +// +// Multi-project runs flattened every project into one synthetic result whose +// manifest was the string "multiple", so every alert in the file pointed at a +// path that does not exist and nothing could be ingested. +func TestSARIFLocationsPointAtRealManifests(t *testing.T) { + root := t.TempDir() + projectA := filepath.Join(root, "a") + projectB := filepath.Join(root, "b") + + multi := &types.MultiProjectResult{ + RootPath: root, + Projects: []*types.ScanResult{ + sampleResult(projectA, filepath.Join(projectA, "package.json")), + sampleResult(projectB, filepath.Join(projectB, "package.json")), + }, + } + + var buf bytes.Buffer + if err := (&SARIFFormatter{}).FormatMulti(multi, &buf); err != nil { + t.Fatalf("format: %v", err) + } + + if strings.Contains(buf.String(), `"multiple"`) { + t.Error(`SARIF still contains the placeholder location "multiple"`) + } + + var doc struct { + Runs []struct { + OriginalURIBaseIDs map[string]struct { + URI string `json:"uri"` + } `json:"originalUriBaseIds"` + Results []struct { + Locations []struct { + PhysicalLocation struct { + ArtifactLocation struct { + URI string `json:"uri"` + URIBaseID string `json:"uriBaseId"` + } `json:"artifactLocation"` + } `json:"physicalLocation"` + } `json:"locations"` + } `json:"results"` + } `json:"runs"` + } + if err := json.Unmarshal(buf.Bytes(), &doc); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if len(doc.Runs[0].Results) == 0 { + t.Fatal("fixture produced no SARIF results, so this test proves nothing") + } + + got := make(map[string]bool) + for _, r := range doc.Runs[0].Results { + if len(r.Locations) == 0 { + t.Fatal("result carries no location") + } + loc := r.Locations[0].PhysicalLocation.ArtifactLocation + if loc.URIBaseID != sarifURIBaseID { + t.Errorf("uriBaseId = %q, want %q", loc.URIBaseID, sarifURIBaseID) + } + got[loc.URI] = true + } + + for _, want := range []string{"a/package.json", "b/package.json"} { + if !got[want] { + t.Errorf("no result located at %q; got %v", want, got) + } + } + if _, ok := doc.Runs[0].OriginalURIBaseIDs[sarifURIBaseID]; !ok { + t.Errorf("run does not declare %s, so the relative uris cannot be resolved", sarifURIBaseID) + } +} + +// TestSARIFReportsSkippedManifests checks that an unread manifest reaches SARIF +// and clears executionSuccessful, so a consumer can tell an incomplete scan from +// a complete one. +func TestSARIFReportsSkippedManifests(t *testing.T) { + root := t.TempDir() + project := filepath.Join(root, "good") + + multi := &types.MultiProjectResult{ + RootPath: root, + Projects: []*types.ScanResult{sampleResult(project, filepath.Join(project, "package.json"))}, + Skipped: []types.SkippedManifest{ + {Path: filepath.Join(root, "broken", "package.json"), Reason: "not valid JSON"}, + }, + } + + var buf bytes.Buffer + if err := (&SARIFFormatter{}).FormatMulti(multi, &buf); err != nil { + t.Fatalf("format: %v", err) + } + + var doc struct { + Runs []struct { + Invocations []struct { + ExecutionSuccessful bool `json:"executionSuccessful"` + ToolExecutionNotifications []struct { + Level string `json:"level"` + Message struct { + Text string `json:"text"` + } `json:"message"` + } `json:"toolExecutionNotifications"` + } `json:"invocations"` + } `json:"runs"` + } + if err := json.Unmarshal(buf.Bytes(), &doc); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if len(doc.Runs[0].Invocations) == 0 { + t.Fatal("no invocation recorded, so a skipped manifest is invisible to SARIF consumers") + } + inv := doc.Runs[0].Invocations[0] + if inv.ExecutionSuccessful { + t.Error("executionSuccessful is true even though a manifest was not read") + } + if len(inv.ToolExecutionNotifications) != 1 { + t.Fatalf("got %d notifications, want 1", len(inv.ToolExecutionNotifications)) + } + if !strings.Contains(inv.ToolExecutionNotifications[0].Message.Text, "not valid JSON") { + t.Errorf("notification does not carry the reason: %q", + inv.ToolExecutionNotifications[0].Message.Text) + } +} diff --git a/pkg/output/reportsafe_test.go b/pkg/output/reportsafe_test.go new file mode 100644 index 0000000..93eb538 --- /dev/null +++ b/pkg/output/reportsafe_test.go @@ -0,0 +1,373 @@ +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) +// SPDX-License-Identifier: Apache-2.0 + +package output + +import ( + "strconv" + "strings" + "testing" + + "github.com/csnp/qramm-cryptodeps/pkg/types" +) + +// injectedHeading is what a hostile directory name tries to put in the report. +// A filesystem path may contain any byte except NUL and the separator, so a +// repository can name a directory with embedded newlines and markdown. +const injectedHeading = "## Scan result: CLEAN" + +// hostilePathScan is a scan of a tree holding a directory whose name carries +// newlines and markdown, plus one whose name carries a pipe and a backtick. +func hostilePathScan() *types.MultiProjectResult { + // The pipe and the backtick are in the SKIPPED path, because that is the one + // the "Not analyzed" table renders and the one the table-cell test inspects. + // They were on a project manifest before, which no table row carries, so the + // pipe assertion ran against a row that had no pipe in it. + injected := "/repo/x\n\n" + injectedHeading + "\n\nNo issues found.\n\nig|nore`x/package.json" + return &types.MultiProjectResult{ + RootPath: "/repo", + Projects: []*types.ScanResult{{ + Project: "/repo/a|b`c", + Manifest: "/repo/a|b`c/package.json", + Ecosystem: types.EcosystemNPM, + Dependencies: []types.DependencyResult{{ + Dependency: types.Dependency{Name: "node-forge", Version: "1.3.1"}, + InDatabase: true, + Analysis: &types.PackageAnalysis{Package: "node-forge", Crypto: []types.CryptoUsage{ + {Algorithm: "DES", QuantumRisk: types.RiskVulnerable, Severity: types.SeverityCritical}, + }}, + }}, + Summary: types.ScanSummary{TotalDependencies: 1, DirectDependencies: 1, WithCrypto: 1, + QuantumVulnerable: 1}, + }}, + Skipped: []types.SkippedManifest{ + {Path: injected, Reason: "not valid JSON: unexpected end of JSON input"}, + }, + } +} + +// TestAScannedRepositoryCannotWriteItsOwnReport is the regression test for a +// report-forgery vector. +// +// A path is attacker-controlled input: the tool is pointed at repositories it +// does not trust, and the bundled GitHub Action publishes the markdown report. +// Every path was interpolated raw, so a directory named with an embedded +// "## Scan result: CLEAN" put that heading in the report, above the findings +// that contradict it. Reproduced end to end before this fix: the injected +// heading appeared in the markdown report for a tree containing one corrupt +// manifest, and in the table report as free-standing lines. +// +// The three machine-readable formats were never affected, because encoding/json +// escapes what it emits. They are asserted here anyway, so that a future change +// away from the encoder cannot reintroduce it quietly. +func TestAScannedRepositoryCannotWriteItsOwnReport(t *testing.T) { + result := hostilePathScan() + + for _, format := range []Format{FormatTable, FormatMarkdown, FormatJSON, FormatCBOM, FormatSARIF} { + t.Run(string(format), func(t *testing.T) { + out := renderMulti(t, format, result) + + // The heading must never appear at the start of a line, which is + // the only position markdown reads it as a heading, and the only + // position it is forged from. + for _, line := range strings.Split(out, "\n") { + if strings.HasPrefix(strings.TrimSpace(line), injectedHeading) { + t.Errorf("%s carries a heading forged by the scanned tree:\n%s", format, out) + break + } + } + // Guard the fixture: the hostile path must actually have reached + // the document, or this asserts nothing. + if !strings.Contains(out, "nore") { + t.Fatalf("%s does not report the hostile manifest at all, so this test "+ + "cannot show how it is rendered:\n%s", format, out) + } + }) + } +} + +// TestMarkdownTableCellsSurviveAPipeInAPath pins the GitHub-flavoured markdown +// rule that a pipe splits a table cell even inside a code span, so a directory +// named "a|b" silently shifted every column after it. +// +// The assertion is the invariant itself, not a cell count: no code span in a +// table row may contain an unescaped pipe. Counting cells broke the moment a +// four-column findings table also began wrapping its first cell in a code span, +// and a count would not have caught a pipe in the fourth column anyway. +func TestMarkdownTableCellsSurviveAPipeInAPath(t *testing.T) { + out := renderMulti(t, FormatMarkdown, hostilePathScan()) + + var spansChecked, withPipe int + for _, line := range strings.Split(out, "\n") { + if !strings.HasPrefix(line, "|") { + continue + } + for _, span := range codeSpans(line) { + spansChecked++ + if strings.Contains(span, "|") { + withPipe++ + if !strings.Contains(span, `\|`) { + t.Errorf("code span %q in a table row carries an unescaped pipe, which "+ + "splits the cell:\n%s", span, line) + } + } + } + } + if spansChecked == 0 { + t.Fatal("no table row carried a code span, so this asserts nothing") + } + // The fixture must actually deliver a pipe into a table row, or the check + // above never runs on the input it exists for. + if withPipe == 0 { + t.Fatalf("no table-row code span carried a pipe, so the fixture does not exercise "+ + "the rule this test is named for; %d spans checked", spansChecked) + } +} + +// codeSpans returns the contents of each backtick-delimited span in a line. +func codeSpans(line string) []string { + var out []string + parts := strings.Split(line, "`") + // Odd indices are inside a span when the backticks are balanced. + for i := 1; i < len(parts); i += 2 { + out = append(out, parts[i]) + } + return out +} + +// TestOrdinaryPathsAreRenderedUnchanged is the paired guard. Escaping must be +// invisible for every real path, or it would churn the output of every scan and +// make the reports harder to read to fix a case that does not occur. +func TestOrdinaryPathsAreRenderedUnchanged(t *testing.T) { + for _, p := range []string{ + "/repo/a/package.json", + "relative/path/go.mod", + "/repo/with space/pom.xml", + "/repo/dot.dir/requirements.txt", + `C:\repo\package.json`, + } { + if got := reportSafe(p); got != p { + t.Errorf("reportSafe(%q) = %q, want it unchanged", p, got) + } + if got := markdownCell(p); got != p { + t.Errorf("markdownCell(%q) = %q, want it unchanged", p, got) + } + } +} + +// TestHostilePathsAreEscapedNotDropped checks the escaping is reversible. A +// scanner that silently dropped or truncated the name would be hiding the file +// it is reporting, which is the silent skip this branch exists to remove. +func TestHostilePathsAreEscapedNotDropped(t *testing.T) { + for _, tc := range []struct{ name, in string }{ + {"newline", "/repo/x\n## heading/package.json"}, + {"carriage return", "/repo/x\r## heading/package.json"}, + {"pipe", "/repo/a|b/package.json"}, + {"backtick", "/repo/a`b/package.json"}, + {"tab", "/repo/a\tb/package.json"}, + {"del", "/repo/a\x7fb/package.json"}, + } { + t.Run(tc.name, func(t *testing.T) { + got := reportSafe(tc.in) + if got == tc.in { + t.Fatalf("reportSafe(%q) returned it unchanged", tc.in) + } + if strings.ContainsAny(got, "\n\r\t\x7f") { + t.Errorf("reportSafe(%q) = %q still carries a control character", tc.in, got) + } + // Reversible: the quoted form unquotes back to the original, so the + // user can still identify the file. strconv, not encoding/json: + // Go's quoting renders DEL as \x7f, which JSON does not accept. + back, err := strconv.Unquote(got) + if err != nil { + t.Fatalf("reportSafe(%q) = %q is not a decodable quoted string: %v", tc.in, got, err) + } + if back != tc.in { + t.Errorf("reportSafe(%q) decodes to %q, so the real path is lost", tc.in, back) + } + + md := markdownCell(tc.in) + if strings.Contains(md, "`") { + t.Errorf("markdownCell(%q) = %q carries a backtick and would close the code span", + tc.in, md) + } + if strings.Count(md, "|") != strings.Count(md, `\|`) { + t.Errorf("markdownCell(%q) = %q carries an unescaped pipe", tc.in, md) + } + }) + } +} + +// TestMarkdownRendersUntrustedPathsInertly is the regression test for the half +// of the injection fix that the first attempt missed. +// +// Escaping characters one at a time only works if you enumerate every character +// the surrounding context makes active. The first version escaped control +// characters, backticks and pipes, which is the right set for the inside of a +// code span, and then interpolated the result into a bare "##" heading and a +// bare table cell, where every markdown construct is live. A directory named +// "**CLEAN**" rendered as bold, and one named "[no findings](https://...)" +// rendered as a link, in the report the bundled Action publishes. +// +// The assertion works the other way round from the first version of this test. +// That one selected lines by the code span it was supposed to be testing for +// ("| `" and "- `"), so removing a code span removed the line from the sample +// and the assertion never ran: five of six mutations survived it. This one finds +// the hostile bytes wherever they landed and requires them to be inside a span. +func TestMarkdownRendersUntrustedPathsInertly(t *testing.T) { + // The link has a single slash: a path goes through filepath.Clean, which + // collapses "https://" to "https:/", so a marker containing "//" would never + // match the rendered form and the coverage guard below would misreport. + const bold, link = "**CLEAN**", "[no findings](evil.invalid)" + + result := mixedSkips() + result.Projects[0].Manifest = "/repo/" + bold + "/package.json" + result.Projects[0].Project = "/repo/" + bold + result.Skipped[0].Path = "/repo/" + link + "/package.json" + result.Skipped[0].Reason = "not valid JSON, near " + bold + result.Skipped[1].Path = "/repo/" + bold + "-unsupported/Cargo.toml" + + out := renderMulti(t, FormatMarkdown, result) + + var found int + for _, line := range strings.Split(out, "\n") { + for _, marker := range []string{bold, link} { + if !strings.Contains(line, marker) { + continue + } + found++ + inSpan := false + for _, span := range codeSpans(line) { + if strings.Contains(span, marker) { + inSpan = true + } + } + if !inSpan { + t.Errorf("markdown renders untrusted text outside a code span, so the "+ + "scanned tree controls the markup:\n%s", line) + } + } + } + // Every context the fixture reaches must have been inspected: the Summary + // Manifest row, the project list, the project heading, the "Not analyzed" + // path cell and its Reason cell, and the unsupported bullet. Six. + if found < 6 { + t.Fatalf("only %d lines carried the hostile markers, so some of the six markdown "+ + "contexts were never rendered by this fixture:\n%s", found, out) + } +} + +// TestNeedsEscapingCoversNonASCIILineAndBidiControls pins the set. ASCII control +// characters are not the whole vocabulary a filename can use against a report: +// U+2028 and U+2029 are line breaks to a renderer, the bidi overrides reverse the +// visible order of a name so the report displays a file it is not talking about, +// and the zero-width characters make two different paths look identical. +func TestNeedsEscapingCoversNonASCIILineAndBidiControls(t *testing.T) { + for _, tc := range []struct { + name string + r rune + }{ + {"line separator", 0x2028}, + {"paragraph separator", 0x2029}, + {"next line", 0x85}, + {"right-to-left override", 0x202e}, + {"left-to-right embedding", 0x202a}, + {"first strong isolate", 0x2068}, + {"zero width space", 0x200b}, + {"zero width joiner", 0x200d}, + {"byte order mark", 0xfeff}, + } { + t.Run(tc.name, func(t *testing.T) { + path := "/repo/a" + string(tc.r) + "b/package.json" + if !needsEscaping(path) { + t.Fatalf("needsEscaping(%q) is false, so U+%04X reaches the report raw", path, tc.r) + } + if got := reportSafe(path); strings.ContainsRune(got, tc.r) { + t.Errorf("reportSafe(%q) = %q still carries U+%04X", path, got, tc.r) + } + }) + } +} + +// TestInvalidUTF8InAPathIsFlagged is the U+FFFD case, which is different in kind +// from the rest of the set. +// +// A filesystem does not require valid UTF-8, and Go decodes an invalid byte to +// U+FFFD. That character injects nothing and reverses nothing, so it is not +// removed: it is printable, and strconv.Quote leaves it alone. What matters is +// that the path is marked as an escaped rendering rather than presented as the +// literal name, because the bytes behind it cannot be recovered from the report. +func TestInvalidUTF8InAPathIsFlagged(t *testing.T) { + path := "/repo/a\xff" + "b/package.json" + decoded := string([]rune(path)) // what a reader of the report sees + + if !needsEscaping(decoded) { + t.Fatalf("needsEscaping(%q) is false, so an unrepresentable filename is presented "+ + "as though it were the literal name", decoded) + } + got := reportSafe(decoded) + if !strings.HasPrefix(got, `"`) { + t.Errorf("reportSafe(%q) = %q, want it quoted so the reader knows the name was "+ + "not rendered literally", decoded, got) + } +} + +// hostileDependencyScan is a scan of a manifest whose dependency version carries +// a forged report heading. Nothing about the filesystem is unusual: the payload +// is a string in a package.json, which is all a pull request needs. +func hostileDependencyScan() *types.MultiProjectResult { + return types.AggregateResults("/repo", []*types.ScanResult{{ + Project: "/repo", + Manifest: "/repo/package.json", + Ecosystem: types.EcosystemNPM, + Dependencies: []types.DependencyResult{{ + Dependency: types.Dependency{ + Name: "node-forge", + Version: "1.3.1\n\n" + injectedHeading + "\n\nNo issues found.\n\n| x ", + }, + InDatabase: true, + Analysis: &types.PackageAnalysis{Package: "node-forge", Crypto: []types.CryptoUsage{ + {Algorithm: "DES", QuantumRisk: types.RiskVulnerable, Severity: types.SeverityCritical}, + }}, + }}, + Summary: types.ScanSummary{TotalDependencies: 1, DirectDependencies: 1, WithCrypto: 1, + QuantumVulnerable: 1}, + }}) +} + +// TestADependencyStringCannotWriteItsOwnReport is the regression test for the +// channel the path fix did not cover. +// +// A dependency name and version come from the manifest under scan, so they are +// attacker-controlled in exactly the way a path is, and they are a strictly +// easier channel: no filesystem write, no directory named with embedded +// newlines, just an entry in a package.json. The database lookup falls back from +// "name@version" to the name alone, so a real package with an arbitrary version +// still resolves and reaches the findings table. A version of +// "1.3.1\n\n## Scan result: CLEAN" put that heading in the markdown report seven +// times, in the middle of the table of findings that contradicts it. +func TestADependencyStringCannotWriteItsOwnReport(t *testing.T) { + result := hostileDependencyScan() + + for _, format := range []Format{FormatTable, FormatMarkdown, FormatJSON, FormatCBOM, FormatSARIF} { + t.Run(string(format), func(t *testing.T) { + out := renderMulti(t, format, result) + + for _, line := range strings.Split(out, "\n") { + if strings.HasPrefix(strings.TrimSpace(line), injectedHeading) { + t.Errorf("%s carries a heading forged by a dependency version:\n%s", format, out) + break + } + } + // Guard the fixture: the finding must have reached the document, or + // the dependency string was never rendered and this proves nothing. + if !strings.Contains(out, "node-forge") { + t.Fatalf("%s does not name the dependency, so the hostile version was never "+ + "rendered:\n%s", format, out) + } + if !strings.Contains(out, "DES") { + t.Fatalf("%s produced no finding for the fixture:\n%s", format, out) + } + }) + } +} diff --git a/pkg/output/sarif.go b/pkg/output/sarif.go index 161872d..30b78bd 100644 --- a/pkg/output/sarif.go +++ b/pkg/output/sarif.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 package output @@ -7,8 +7,10 @@ import ( "encoding/json" "errors" "io" + "path/filepath" "github.com/csnp/qramm-cryptodeps/pkg/types" + "github.com/csnp/qramm-cryptodeps/pkg/version" ) // SARIFFormatter formats scan results as SARIF for GitHub Security integration. @@ -18,14 +20,37 @@ type SARIFFormatter struct { // sarifLog represents a SARIF log structure. type sarifLog struct { - Schema string `json:"$schema"` - Version string `json:"version"` - Runs []sarifRun `json:"runs"` + Schema string `json:"$schema"` + Version string `json:"version"` + Runs []sarifRun `json:"runs"` } type sarifRun struct { - Tool sarifTool `json:"tool"` - Results []sarifResult `json:"results"` + Tool sarifTool `json:"tool"` + OriginalURIBaseIDs map[string]sarifArtifactBase `json:"originalUriBaseIds,omitempty"` + Invocations []sarifInvocation `json:"invocations,omitempty"` + Results []sarifResult `json:"results"` +} + +// sarifInvocation carries whether the run was complete. A manifest that could +// not be read is reported here as a tool execution notification and clears +// executionSuccessful, which is how a SARIF consumer learns the scan did not +// cover everything it was pointed at. +type sarifInvocation struct { + ExecutionSuccessful bool `json:"executionSuccessful"` + ToolExecutionNotifications []sarifNotification `json:"toolExecutionNotifications,omitempty"` +} + +type sarifNotification struct { + Level string `json:"level"` + Message sarifMessage `json:"message"` + Locations []sarifLocation `json:"locations,omitempty"` +} + +// sarifArtifactBase declares what a uriBaseId resolves to, so a consumer can +// turn the repository-relative uri on each result back into a real file. +type sarifArtifactBase struct { + URI string `json:"uri"` } type sarifTool struct { @@ -33,10 +58,11 @@ type sarifTool struct { } type sarifDriver struct { - Name string `json:"name"` - Version string `json:"version"` - InformationURI string `json:"informationUri"` - Rules []sarifRule `json:"rules"` + Name string `json:"name"` + Version string `json:"version"` + SemanticVersion string `json:"semanticVersion,omitempty"` + InformationURI string `json:"informationUri"` + Rules []sarifRule `json:"rules"` } type sarifRule struct { @@ -57,10 +83,10 @@ type sarifDefaultConfig struct { } type sarifResult struct { - RuleID string `json:"ruleId"` - Level string `json:"level"` - Message sarifMessage `json:"message"` - Locations []sarifLocation `json:"locations"` + RuleID string `json:"ruleId"` + Level string `json:"level"` + Message sarifMessage `json:"message"` + Locations []sarifLocation `json:"locations"` } type sarifLocation struct { @@ -72,9 +98,13 @@ type sarifPhysicalLocation struct { } type sarifArtifactLocation struct { - URI string `json:"uri"` + URI string `json:"uri"` + URIBaseID string `json:"uriBaseId,omitempty"` } +// sarifURIBaseID names the base that every result uri is relative to. +const sarifURIBaseID = "SRCROOT" + // Format writes the scan result as SARIF. func (f *SARIFFormatter) Format(result *types.ScanResult, w io.Writer) error { if result == nil { @@ -83,6 +113,36 @@ func (f *SARIFFormatter) Format(result *types.ScanResult, w io.Writer) error { if w == nil { return errors.New("writer cannot be nil") } + root := result.Project + if root == "" { + root = filepath.Dir(result.Manifest) + } + return f.write(w, root, []*types.ScanResult{result}, nil) +} + +// FormatMulti writes multi-project scan results as SARIF. +// Every project contributes its own results, each located at that project's own +// manifest. An earlier version merged all projects into one synthetic result +// whose manifest was the literal string "multiple", so every alert in the file +// pointed at a path that does not exist and nothing could be ingested. +func (f *SARIFFormatter) FormatMulti(result *types.MultiProjectResult, w io.Writer) error { + if result == nil { + return errors.New("result cannot be nil") + } + if w == nil { + return errors.New("writer cannot be nil") + } + return f.write(w, result.RootPath, result.Projects, result.Skipped) +} + +// write emits one SARIF run covering every supplied project. +func (f *SARIFFormatter) write(w io.Writer, root string, projects []*types.ScanResult, skipped []types.SkippedManifest) error { + // A uriBaseId names a directory, and the root has to be absolute before any + // manifest path can be expressed relative to it. Both normalizations live in + // scanRootDir, which CBOM calls too: when SARIF did this inline and CBOM did + // not, the same run produced repository-relative SARIF and absolute CBOM. + absRoot := scanRootDir(root) + log := sarifLog{ Schema: "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/Schemata/sarif-schema-2.1.0.json", Version: "2.1.0", @@ -90,70 +150,128 @@ func (f *SARIFFormatter) Format(result *types.ScanResult, w io.Writer) error { { Tool: sarifTool{ Driver: sarifDriver{ - Name: "CryptoDeps", - Version: "1.0.0", - InformationURI: "https://github.com/csnp/qramm-cryptodeps", - Rules: make([]sarifRule, 0), + Name: version.DisplayName, + Version: version.Version(), + SemanticVersion: version.Version(), + InformationURI: version.InformationURI, + Rules: make([]sarifRule, 0), }, }, + OriginalURIBaseIDs: map[string]sarifArtifactBase{ + sarifURIBaseID: {URI: "file://" + filepath.ToSlash(absRoot) + "/"}, + }, Results: make([]sarifResult, 0), }, }, } - // Add rules and results for each finding + // Record unread manifests as execution notifications. Emitting results + // without saying that part of the input was never read would let a consumer + // treat an incomplete scan as a complete one. + // An unsupported ecosystem is reported but does not clear + // executionSuccessful: the tool did not fail to read the file, it has no + // parser for it, which is a declared limit rather than an incomplete run. + invocation := sarifInvocation{ExecutionSuccessful: !types.IncompleteScan(skipped)} + for _, s := range skipped { + uri, baseID := sarifArtifactURI(absRoot, s.Path) + level, prefix := "error", "manifest found but not analyzed: " + if s.Unsupported { + level, prefix = "warning", "manifest found but not supported: " + } + invocation.ToolExecutionNotifications = append(invocation.ToolExecutionNotifications, sarifNotification{ + Level: level, + Message: sarifMessage{Text: prefix + s.Reason}, + Locations: []sarifLocation{ + {PhysicalLocation: sarifPhysicalLocation{ + ArtifactLocation: sarifArtifactLocation{URI: uri, URIBaseID: baseID}, + }}, + }, + }) + } + // Say what the run did not cover, for the same reason the table does, and + // through the same classification so the two cannot disagree. A consumer + // reading only `results` cannot tell an empty array produced by a clean tree + // from one produced by a filter that withheld everything, or by a scan where + // no dependency was in the database. + // + // Per project, and only for projects that produced no findings. Evaluated + // over the whole run instead, these notes contradicted the results beside + // them: a scan with two CRITICAL findings carried a notification saying no + // conclusion could be drawn, and a workspace where one project of two went + // entirely unexamined carried no note at all. These are coverage statements + // rather than tool failures, so they do not clear executionSuccessful. + for _, note := range coverageNotes(projects) { + message := note.Text() + if note.Manifest != "" && len(projects) > 1 { + uri, _ := sarifArtifactURI(absRoot, note.Manifest) + message = uri + ": " + message + } + invocation.ToolExecutionNotifications = append(invocation.ToolExecutionNotifications, sarifNotification{ + Level: note.Level(), + Message: sarifMessage{Text: message}, + }) + } + log.Runs[0].Invocations = []sarifInvocation{invocation} + rulesMap := make(map[string]bool) - for _, dep := range result.Dependencies { - if dep.Analysis == nil || len(dep.Analysis.Crypto) == 0 { + for _, result := range projects { + if result == nil { continue } + uri, baseID := sarifArtifactURI(absRoot, result.Manifest) - for _, crypto := range dep.Analysis.Crypto { - ruleID := "CRYPTO-" + crypto.Algorithm + for _, dep := range result.Dependencies { + if dep.Analysis == nil || len(dep.Analysis.Crypto) == 0 { + continue + } - // Add rule if not already added - if !rulesMap[ruleID] { - rule := sarifRule{ - ID: ruleID, - Name: crypto.Algorithm + " Usage", - ShortDescription: sarifMessage{ - Text: "Dependency uses " + crypto.Algorithm, - }, - FullDescription: sarifMessage{ - Text: "The dependency " + dep.Dependency.Name + " uses " + crypto.Algorithm + " which has quantum risk: " + string(crypto.QuantumRisk), - }, - DefaultConfig: sarifDefaultConfig{ - Level: severityToSARIFLevel(crypto.Severity), - }, + for _, crypto := range dep.Analysis.Crypto { + ruleID := "CRYPTO-" + crypto.Algorithm + + // Add rule if not already added + if !rulesMap[ruleID] { + rule := sarifRule{ + ID: ruleID, + Name: crypto.Algorithm + " Usage", + ShortDescription: sarifMessage{ + Text: "Dependency uses " + crypto.Algorithm, + }, + FullDescription: sarifMessage{ + Text: "A dependency uses " + crypto.Algorithm + " which has quantum risk: " + string(crypto.QuantumRisk), + }, + DefaultConfig: sarifDefaultConfig{ + Level: severityToSARIFLevel(crypto.Severity), + }, + } + log.Runs[0].Tool.Driver.Rules = append(log.Runs[0].Tool.Driver.Rules, rule) + rulesMap[ruleID] = true } - log.Runs[0].Tool.Driver.Rules = append(log.Runs[0].Tool.Driver.Rules, rule) - rulesMap[ruleID] = true - } - // Build message with remediation if available - msgText := dep.Dependency.Name + "@" + dep.Dependency.Version + " uses " + crypto.Algorithm + " (Quantum Risk: " + string(crypto.QuantumRisk) + ")" - if f.Options.ShowRemediation && crypto.Remediation != "" { - msgText += ". Remediation: " + crypto.Remediation - } + // Build message with remediation if available + msgText := dep.Dependency.Name + "@" + dep.Dependency.Version + " uses " + crypto.Algorithm + " (Quantum Risk: " + string(crypto.QuantumRisk) + ")" + if f.Options.ShowRemediation && crypto.Remediation != "" { + msgText += ". Remediation: " + crypto.Remediation + } - // Add result - res := sarifResult{ - RuleID: ruleID, - Level: severityToSARIFLevel(crypto.Severity), - Message: sarifMessage{ - Text: msgText, - }, - Locations: []sarifLocation{ - { - PhysicalLocation: sarifPhysicalLocation{ - ArtifactLocation: sarifArtifactLocation{ - URI: result.Manifest, + res := sarifResult{ + RuleID: ruleID, + Level: severityToSARIFLevel(crypto.Severity), + Message: sarifMessage{ + Text: msgText, + }, + Locations: []sarifLocation{ + { + PhysicalLocation: sarifPhysicalLocation{ + ArtifactLocation: sarifArtifactLocation{ + URI: uri, + URIBaseID: baseID, + }, }, }, }, - }, + } + log.Runs[0].Results = append(log.Runs[0].Results, res) } - log.Runs[0].Results = append(log.Runs[0].Results, res) } } @@ -162,6 +280,22 @@ func (f *SARIFFormatter) Format(result *types.ScanResult, w io.Writer) error { return encoder.Encode(log) } +// sarifArtifactURI expresses a manifest path relative to the scan root, which is +// the form SARIF consumers such as GitHub code scanning need in order to attach +// an alert to a file in the repository. A manifest that does not sit under the +// root (which a caller can produce by passing an explicit file path) falls back +// to an absolute file URI with no base, since a relative path would be a lie. +func sarifArtifactURI(absRoot, manifestPath string) (uri string, baseID string) { + path, underRoot := relativeToRoot(absRoot, manifestPath) + if path == "" { + return "", "" + } + if !underRoot { + return "file://" + path, "" + } + return path, sarifURIBaseID +} + // severityToSARIFLevel converts a severity to SARIF level. func severityToSARIFLevel(severity types.Severity) string { switch severity { @@ -173,30 +307,3 @@ func severityToSARIFLevel(severity types.Severity) string { return "note" } } - -// FormatMulti writes multi-project scan results as SARIF. -// It merges all findings from all projects into a single SARIF log. -func (f *SARIFFormatter) FormatMulti(result *types.MultiProjectResult, w io.Writer) error { - if result == nil { - return errors.New("result cannot be nil") - } - if w == nil { - return errors.New("writer cannot be nil") - } - - // Create a merged scan result for SARIF output - merged := &types.ScanResult{ - Project: result.RootPath, - Manifest: "multiple", - Ecosystem: types.EcosystemUnknown, - ScanDate: result.ScanDate, - Summary: result.TotalSummary, - } - - // Collect all dependencies from all projects - for _, project := range result.Projects { - merged.Dependencies = append(merged.Dependencies, project.Dependencies...) - } - - return f.Format(merged, w) -} diff --git a/pkg/output/skipkind_test.go b/pkg/output/skipkind_test.go new file mode 100644 index 0000000..c5d8f8c --- /dev/null +++ b/pkg/output/skipkind_test.go @@ -0,0 +1,334 @@ +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) +// SPDX-License-Identifier: Apache-2.0 + +package output + +import ( + "encoding/json" + "strings" + "testing" + + "github.com/csnp/qramm-cryptodeps/pkg/types" +) + +// mixedSkips is a scan holding one of each kind of skip: a manifest that should +// have been readable and was not, and a manifest for an ecosystem cryptodeps has +// no parser for. +// +// The two are genuinely different and the difference is load-bearing. Only the +// first means the scan failed to cover its input, so only the first forces exit +// 2. Counting them together made the table say "2 manifest file(s) found but NOT +// analyzed" about a corrupt package.json and a healthy Cargo.toml in the same +// breath, and asserted that the Cargo.toml could not be read. +func mixedSkips() *types.MultiProjectResult { + return &types.MultiProjectResult{ + RootPath: "/repo", + Projects: []*types.ScanResult{{ + Project: "/repo/good", + Manifest: "/repo/good/package.json", + Ecosystem: types.EcosystemNPM, + Dependencies: []types.DependencyResult{{ + Dependency: types.Dependency{Name: "node-forge", Version: "1.3.1"}, + InDatabase: true, + Analysis: &types.PackageAnalysis{Package: "node-forge", Crypto: []types.CryptoUsage{ + {Algorithm: "DES", QuantumRisk: types.RiskVulnerable, Severity: types.SeverityCritical}, + }}, + }}, + Summary: types.ScanSummary{TotalDependencies: 1, DirectDependencies: 1, WithCrypto: 1, + QuantumVulnerable: 1}, + }}, + Skipped: []types.SkippedManifest{ + {Path: "/repo/corrupt/package.json", Reason: "not valid JSON: unexpected end of JSON input"}, + {Path: "/repo/Cargo.toml", Reason: "no parser for this manifest type", Unsupported: true}, + }, + } +} + +// TestUnsupportedManifestsAreReportedSeparatelyInEveryFormat is the guard for +// the reporting half of the unread/unsupported split. +// +// The exit-code half is guarded in internal/manifest. This half had nothing: +// deleting the unsupported section from the table and from markdown outright +// left the whole suite green, which is how a build.gradle full of crypto +// dependencies could go back to vanishing from the report at exit 0. That silent +// skip is the defect this branch exists to remove. +// +// Every count below is asserted with its number, because a conflated count is +// exactly what the split fixes and a bare "the file is named somewhere" check +// passes on the conflated implementation too. +func TestUnsupportedManifestsAreReportedSeparatelyInEveryFormat(t *testing.T) { + result := mixedSkips() + + t.Run("table", func(t *testing.T) { + out := renderMulti(t, FormatTable, result) + if !strings.Contains(out, "1 manifest file(s) found but NOT analyzed") { + t.Errorf("table does not report exactly 1 unread manifest, so it counts the "+ + "unsupported one as a gap in the scan:\n%s", out) + } + if !strings.Contains(out, "1 manifest file(s) found for ecosystems cryptodeps does not support") { + t.Errorf("table does not report the unsupported manifest, so a Cargo.toml "+ + "vanishes from the report entirely:\n%s", out) + } + if !strings.Contains(out, "This does not affect the exit code.") { + t.Errorf("table does not say an unsupported ecosystem is not a failure:\n%s", out) + } + for _, path := range []string{"./corrupt/package.json", "./Cargo.toml"} { + if !strings.Contains(out, path) { + t.Errorf("table does not name %s:\n%s", path, out) + } + } + }) + + t.Run("markdown", func(t *testing.T) { + out := renderMulti(t, FormatMarkdown, result) + if !strings.Contains(out, "1 manifest file(s) were found but could not be read") { + t.Errorf("markdown does not report exactly 1 unread manifest:\n%s", out) + } + if !strings.Contains(out, "## Unsupported ecosystems") || + !strings.Contains(out, "1 manifest file(s) belong to ecosystems cryptodeps does not parse") { + t.Errorf("markdown does not report the unsupported manifest:\n%s", out) + } + for _, path := range []string{"./corrupt/package.json", "./Cargo.toml"} { + if !strings.Contains(out, path) { + t.Errorf("markdown does not name %s:\n%s", path, out) + } + } + }) + + t.Run("cbom", func(t *testing.T) { + out := renderMulti(t, FormatCBOM, result) + var doc struct { + Metadata struct { + Properties []struct { + Name string `json:"name"` + Value string `json:"value"` + } `json:"properties"` + } `json:"metadata"` + } + if err := json.Unmarshal([]byte(out), &doc); err != nil { + t.Fatalf("CBOM is not valid JSON: %v", err) + } + byName := map[string]string{} + for _, p := range doc.Metadata.Properties { + byName[p.Name] = p.Value + } + if got := byName["cryptodeps:manifestNotAnalyzed"]; !strings.HasPrefix(got, "corrupt/package.json:") { + t.Errorf("cryptodeps:manifestNotAnalyzed = %q, want the corrupt package.json", got) + } + if got := byName["cryptodeps:manifestNotSupported"]; !strings.HasPrefix(got, "Cargo.toml:") { + t.Errorf("cryptodeps:manifestNotSupported = %q, want the Cargo.toml", got) + } + if got := byName["cryptodeps:coverage"]; !strings.Contains(got, "1 manifest(s) were found but could not be read") { + t.Errorf("cryptodeps:coverage = %q, want it to count 1 unread manifest; counting "+ + "the unsupported one makes this document disagree with the table for the same run", got) + } + }) + + t.Run("sarif", func(t *testing.T) { + out := renderMulti(t, FormatSARIF, result) + var doc struct { + Runs []struct { + Invocations []struct { + ExecutionSuccessful bool `json:"executionSuccessful"` + ToolExecutionNotifications []struct { + Level string `json:"level"` + Message struct { + Text string `json:"text"` + } `json:"message"` + } `json:"toolExecutionNotifications"` + } `json:"invocations"` + } `json:"runs"` + } + if err := json.Unmarshal([]byte(out), &doc); err != nil { + t.Fatalf("SARIF is not valid JSON: %v", err) + } + inv := doc.Runs[0].Invocations[0] + if inv.ExecutionSuccessful { + t.Error("executionSuccessful is true while a manifest could not be read, so a " + + "code-scanning consumer reads an incomplete scan as a complete one") + } + levels := map[string]string{} + for _, n := range inv.ToolExecutionNotifications { + switch { + case strings.HasPrefix(n.Message.Text, "manifest found but not analyzed:"): + levels["unread"] = n.Level + case strings.HasPrefix(n.Message.Text, "manifest found but not supported:"): + levels["unsupported"] = n.Level + } + } + if levels["unread"] != "error" { + t.Errorf("unread manifest notification level = %q, want error", levels["unread"]) + } + if levels["unsupported"] != "warning" { + t.Errorf("unsupported manifest notification level = %q, want warning; it is a "+ + "declared limit of the tool, not a failure to read the file", levels["unsupported"]) + } + }) +} + +// TestOnlyUnsupportedSkipsLeaveTheScanComplete pins the direction of the split +// that decides the exit code, from the formatter's side. +func TestOnlyUnsupportedSkipsLeaveTheScanComplete(t *testing.T) { + unsupportedOnly := []types.SkippedManifest{ + {Path: "/repo/Cargo.toml", Reason: "no parser for this manifest type", Unsupported: true}, + {Path: "/repo/Gemfile", Reason: "no parser for this manifest type", Unsupported: true}, + } + if types.IncompleteScan(unsupportedOnly) { + t.Error("a tree whose only skips are unsupported ecosystems reports an incomplete " + + "scan, which is the exit 2 that made every polyglot repository a build failure") + } + + result := mixedSkips() + result.Skipped = unsupportedOnly + out := renderMulti(t, FormatTable, result) + if strings.Contains(out, "found but NOT analyzed") { + t.Errorf("table calls an unsupported ecosystem an unread manifest:\n%s", out) + } + if !strings.Contains(out, "2 manifest file(s) found for ecosystems cryptodeps does not support") { + t.Errorf("table does not report both unsupported manifests:\n%s", out) + } +} + +// TestCoverageNoteLevels pins which coverage statements warn. +// +// Every note used to be a warning, so a healthy npm workspace produced three +// warnings saying "this package.json declares no dependencies" and buried the +// one that matters. A filtered report and an empty manifest are complete and +// correct states that need explaining; a scan that drew no conclusion is not. +func TestCoverageNoteLevels(t *testing.T) { + for _, tc := range []struct { + name string + note coverageNote + want string + }{ + {"filtered", coverageNote{Case: caseFiltered}, "note"}, + {"no dependencies", coverageNote{Case: caseNoDependencies}, "note"}, + {"nothing examined", coverageNote{Case: caseNothingExamined}, "warning"}, + } { + if got := tc.note.Level(); got != tc.want { + t.Errorf("%s: Level() = %q, want %q", tc.name, got, tc.want) + } + } +} + +// TestEveryFormatNamesTheSameManifestPath is the guard for an inconsistency the +// path consolidation claimed to have removed and had not. +// +// scanRootDir and relativeToRoot were shared by CBOM and SARIF only. The table +// kept a third implementation, getRelativePath, which compared the raw scan root +// against absolutized manifest paths with a string prefix, and markdown did not +// relativize at all. So one scan of one tree, published two ways, disagreed about +// where a file is: `cryptodeps analyze .` put "corrupt/package.json" in the CBOM +// and the operator's absolute home directory in the table and the markdown for +// the same manifest. The privacy rationale for the CBOM change applies to the +// markdown report too, which the README documents and the Action publishes. +func TestEveryFormatNamesTheSameManifestPath(t *testing.T) { + result := mixedSkips() + + for _, format := range []Format{FormatTable, FormatMarkdown, FormatCBOM, FormatSARIF} { + t.Run(string(format), func(t *testing.T) { + out := renderMulti(t, format, result) + if !strings.Contains(out, "corrupt/package.json") { + t.Fatalf("%s does not name the skipped manifest at all:\n%s", format, out) + } + // The scan root is /repo, so the manifest is corrupt/package.json. + // Any format still carrying the absolute form is publishing the + // local layout its siblings deliberately stopped publishing. + if strings.Contains(out, "/repo/corrupt/package.json") { + t.Errorf("%s renders the skipped manifest as an absolute path while other "+ + "formats render it relative to the scan root:\n%s", format, out) + } + }) + } +} + +// TestGetRelativePathDoesNotInventPathsAcrossASharedPrefix pins the bug the +// string-prefix implementation had. "/repository" is not inside "/repo". +func TestGetRelativePathDoesNotInventPathsAcrossASharedPrefix(t *testing.T) { + if got := getRelativePath("/repo", "/repository/go.mod"); got != "/repository/go.mod" { + t.Errorf("getRelativePath(\"/repo\", \"/repository/go.mod\") = %q, want the path "+ + "unchanged; a shared string prefix is not containment", got) + } + if got := getRelativePath("/repo", "/repo/a/go.mod"); got != "./a/go.mod" { + t.Errorf("getRelativePath(\"/repo\", \"/repo/a/go.mod\") = %q, want \"./a/go.mod\"", got) + } +} + +// analyzedPathScan is a two-project workspace, so the per-project renders run +// through the workspace path rather than the single-project short-circuit. +func analyzedPathScan() *types.MultiProjectResult { + mk := func(dir string) *types.ScanResult { + return &types.ScanResult{ + Project: "/repo/" + dir, + Manifest: "/repo/" + dir + "/package.json", + Ecosystem: types.EcosystemNPM, + Dependencies: []types.DependencyResult{{ + Dependency: types.Dependency{Name: "node-forge", Version: "1.3.1"}, + InDatabase: true, + Analysis: &types.PackageAnalysis{Package: "node-forge", Crypto: []types.CryptoUsage{ + {Algorithm: "DES", QuantumRisk: types.RiskVulnerable, Severity: types.SeverityCritical}, + }}, + }}, + Summary: types.ScanSummary{TotalDependencies: 1, DirectDependencies: 1, WithCrypto: 1, + QuantumVulnerable: 1}, + } + } + return types.AggregateResults("/repo", []*types.ScanResult{mk("a"), mk("b")}) +} + +// TestAnalyzedManifestsAreNamedRelativeToTheRoot guards the headline behaviour of +// the path consolidation, which had no test that failed when it was removed. +// +// Only the SKIPPED manifests were guarded. Deleting the relativization from the +// per-project table header, the markdown Summary row, the markdown project list +// or the markdown project heading left the whole suite green, so the absolute +// paths those surfaces used to publish could come straight back. +func TestAnalyzedManifestsAreNamedRelativeToTheRoot(t *testing.T) { + for _, format := range []Format{FormatTable, FormatMarkdown} { + t.Run(string(format), func(t *testing.T) { + out := renderMulti(t, format, analyzedPathScan()) + if !strings.Contains(out, "a/package.json") || !strings.Contains(out, "b/package.json") { + t.Fatalf("%s does not name both analyzed manifests, so this asserts nothing:\n%s", + format, out) + } + for _, line := range strings.Split(out, "\n") { + if strings.Contains(line, "/repo/a/package.json") || + strings.Contains(line, "/repo/b/package.json") { + t.Errorf("%s names an analyzed manifest by its absolute path while the "+ + "skip list and the machine formats use the relative one:\n%s", format, line) + } + } + }) + } +} + +// TestASingleProjectTreeIsNamedLikeAnyOther pins the short-circuit. +// +// table.go's FormatMulti delegates a one-project workspace straight to the +// single-project renderer. That renderer takes no scan root, so a repository +// with exactly one manifest, which is the most common shape there is, printed an +// absolute path in the table while markdown printed a relative one for the same +// run, and the skip list printed above it in the same document was relative too. +func TestASingleProjectTreeIsNamedLikeAnyOther(t *testing.T) { + one := types.AggregateResults("/repo", []*types.ScanResult{analyzedPathScan().Projects[0]}) + one.Skipped = []types.SkippedManifest{ + {Path: "/repo/corrupt/package.json", Reason: "not valid JSON: unexpected end of JSON input"}, + } + if len(one.Projects) != 1 { + t.Fatalf("fixture has %d projects, so it does not exercise the short-circuit", + len(one.Projects)) + } + + for _, format := range []Format{FormatTable, FormatMarkdown} { + t.Run(string(format), func(t *testing.T) { + out := renderMulti(t, format, one) + if !strings.Contains(out, "a/package.json") { + t.Fatalf("%s does not name the manifest:\n%s", format, out) + } + if strings.Contains(out, "/repo/a/package.json") { + t.Errorf("%s names the only manifest by its absolute path, while the skipped "+ + "one in the same document is relative:\n%s", format, out) + } + }) + } +} diff --git a/pkg/output/table.go b/pkg/output/table.go index e713b10..6d69755 100644 --- a/pkg/output/table.go +++ b/pkg/output/table.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 package output @@ -31,6 +31,13 @@ type cryptoDetail struct { // Format writes the scan result as a table. func (f *TableFormatter) Format(result *types.ScanResult, w io.Writer) error { + return f.formatProject(result, "", w) +} + +// formatProject renders one project. root is the scan root when this render is +// part of a workspace report and empty when it stands alone, so the manifest is +// named the same way here as in the project list above it. +func (f *TableFormatter) formatProject(result *types.ScanResult, root string, w io.Writer) error { if result == nil { return errors.New("result cannot be nil") } @@ -38,7 +45,7 @@ func (f *TableFormatter) Format(result *types.ScanResult, w io.Writer) error { return errors.New("writer cannot be nil") } // Header - fmt.Fprintf(w, "\n[*] Scanning %s... found %d dependencies\n\n", result.Manifest, result.Summary.TotalDependencies) + fmt.Fprintf(w, "\n[*] Scanning %s... found %d dependencies\n\n", reportSafe(manifestForReport(root, result.Manifest)), result.Summary.TotalDependencies) // Check if there are any crypto findings hasCrypto := false @@ -50,8 +57,7 @@ func (f *TableFormatter) Format(result *types.ScanResult, w io.Writer) error { } if !hasCrypto { - fmt.Fprintln(w, "[OK] No cryptographic usage detected in dependencies.") - fmt.Fprintln(w) + f.printNoFindingsVerdict(w, result) return nil } @@ -65,10 +71,10 @@ func (f *TableFormatter) Format(result *types.ScanResult, w io.Writer) error { continue } - depName := dep.Dependency.Name - if dep.Dependency.Version != "" { - depName = fmt.Sprintf("%s@%s", dep.Dependency.Name, dep.Dependency.Version) - } + // Escaped here, once, because this string is built from the manifest + // under scan and is printed on several rows below. A version of + // "1.3.1\n\n## Scan result: CLEAN" wrote that line into this report. + depName := reportSafe(dependencyLabel(dep.Dependency.Name, dep.Dependency.Version)) for _, c := range dep.Analysis.Crypto { allCrypto = append(allCrypto, cryptoDetail{ @@ -104,12 +110,20 @@ func (f *TableFormatter) Format(result *types.ScanResult, w io.Writer) error { result.Summary.AvailableCrypto, ) } else { - fmt.Fprintf(w, "SUMMARY: %d deps | %d with crypto | %d vulnerable | %d partial\n\n", + fmt.Fprintf(w, "SUMMARY: %d deps | %d with crypto | %d vulnerable | %d partial\n", result.Summary.TotalDependencies, result.Summary.WithCrypto, result.Summary.QuantumVulnerable, result.Summary.QuantumPartial, ) + fmt.Fprintln(w) + } + + // A filtered report is a partial view, and every number above describes only + // what survived the filter. Say so, next to the numbers. + if result.Summary.FilteredOut > 0 { + fmt.Fprintf(w, "FILTERED: %d further finding(s) excluded by --risk or --min-severity.\n\n", + result.Summary.FilteredOut) } // Display detailed remediation for vulnerable findings @@ -145,13 +159,104 @@ func (f *TableFormatter) Format(result *types.ScanResult, w io.Writer) error { fmt.Fprintf(w, "[!] %d packages not in database (use --deep to analyze)\n", notAnalyzed) } - if deepAnalyzed > 0 || notAnalyzed > 0 { + f.printHints(w, result) + + if deepAnalyzed > 0 || notAnalyzed > 0 || len(result.Hints) > 0 { fmt.Fprintln(w) } return nil } +// printNoFindingsVerdict states what a scan with no crypto findings actually +// established. +// +// "No cryptographic usage detected" is only true if something was examined. When +// every dependency is unknown to the database, nothing was examined, and +// reporting a clean result is a false negative on the tool's core question. The +// three cases below are genuinely different and are worded differently. +func (f *TableFormatter) printNoFindingsVerdict(w io.Writer, result *types.ScanResult) { + total := result.Summary.TotalDependencies + unknown := result.Summary.NotInDatabase + + switch classifyNoFindings(result.Summary) { + case caseFiltered: + fmt.Fprintf(w, "[?] No findings matched the active filter. %d finding(s) were detected and\n", + result.Summary.FilteredOut) + fmt.Fprintln(w, " excluded by --risk or --min-severity. This is not a clean result.") + fmt.Fprintln(w, " Re-run without the filter to see them.") + + case caseNoDependencies: + fmt.Fprintln(w, "[?] No dependencies found in this manifest. Nothing to analyze.") + + case caseNothingExamined: + fmt.Fprintf(w, "[?] Not analyzed. All %d dependencies are absent from the crypto database,\n", total) + fmt.Fprintln(w, " so no conclusion about cryptographic usage can be drawn from this scan.") + fmt.Fprintln(w, " Run with --deep to analyze package source code directly.") + // The hints for this case say the same thing in other words. Printing + // both reads as three separate problems. + + default: + fmt.Fprintf(w, "[OK] No cryptographic usage detected in the %d of %d dependencies that were analyzed.\n", + total-unknown, total) + if unknown > 0 { + fmt.Fprintf(w, "[!] %d not in database, so they were not examined (use --deep to analyze).\n", unknown) + } + f.printHints(w, result) + } + + fmt.Fprintln(w) +} + +// printHints prints the analyzer's suggestions. They were generated on every +// scan but never reached the terminal. +func (f *TableFormatter) printHints(w io.Writer, result *types.ScanResult) { + for _, hint := range result.Hints { + fmt.Fprintf(w, " %s\n", hint) + } +} + +// PrintSkipped reports manifests that were found but not analyzed. It is +// deliberately loud: a silently skipped manifest is how a scanner reports a +// clean tree it never read. +func PrintSkipped(w io.Writer, root string, skipped []types.SkippedManifest) { + if len(skipped) == 0 { + return + } + // Split by kind. Counting them together made this report say "2 manifest + // file(s) found but NOT analyzed" about a corrupt package.json and a + // Cargo.toml in the same breath, while the CBOM for the same run said one. + // Only the first is a gap in the scan; the second is a limit of the tool. + var unread, unsupported []types.SkippedManifest + for _, s := range skipped { + if s.Unsupported { + unsupported = append(unsupported, s) + } else { + unread = append(unread, s) + } + } + + if len(unread) > 0 { + fmt.Fprintf(w, "[!] %d manifest file(s) found but NOT analyzed:\n", len(unread)) + for _, s := range unread { + fmt.Fprintf(w, " %s\n", reportSafe(getRelativePath(root, s.Path))) + fmt.Fprintf(w, " reason: %s\n", reportSafe(s.Reason)) + } + fmt.Fprintln(w, " These dependencies are missing from the results below.") + fmt.Fprintln(w) + } + + if len(unsupported) > 0 { + fmt.Fprintf(w, "[?] %d manifest file(s) found for ecosystems cryptodeps does not support:\n", + len(unsupported)) + for _, s := range unsupported { + fmt.Fprintf(w, " %s\n", reportSafe(getRelativePath(root, s.Path))) + } + fmt.Fprintln(w, " Their dependencies were not analyzed. This does not affect the exit code.") + fmt.Fprintln(w) + } +} + // printReachabilityBreakdown prints crypto grouped by reachability status. func (f *TableFormatter) printReachabilityBreakdown(w io.Writer, allCrypto []cryptoDetail) { // Group by reachability @@ -172,7 +277,7 @@ func (f *TableFormatter) printReachabilityBreakdown(w io.Writer, allCrypto []cry icon := riskIcon(c.risk) timeline := formatTimelineShort(getTimeline(c.algorithm)) effort := formatEffortShort(getEffort(c.algorithm)) - fmt.Fprintf(w, " %s %-14s %-12s %-12s %s\n", + fmt.Fprintf(w, " %-4s %-14s %-12s %-12s %s\n", icon, c.algorithm, formatRisk(c.risk), timeline, effort) fmt.Fprintf(w, " └─ %s\n", c.dependency) @@ -196,7 +301,7 @@ func (f *TableFormatter) printReachabilityBreakdown(w io.Writer, allCrypto []cry fmt.Fprintln(w, strings.Repeat("─", 90)) for _, c := range reachable { icon := riskIcon(c.risk) - fmt.Fprintf(w, " %s %-14s %-12s %s\n", icon, c.algorithm, formatRisk(c.risk), c.dependency) + fmt.Fprintf(w, " %-4s %-14s %-12s %s\n", icon, c.algorithm, formatRisk(c.risk), c.dependency) } fmt.Fprintln(w) } @@ -206,15 +311,22 @@ func (f *TableFormatter) printReachabilityBreakdown(w io.Writer, allCrypto []cry fmt.Fprintln(w, "[.] AVAILABLE - In dependencies but not called (lower priority):") fmt.Fprintln(w, strings.Repeat("─", 90)) - // Group available by dependency for cleaner output + // Group available by dependency for cleaner output. Iterate the names in + // sorted order rather than ranging over the map, whose order Go + // randomises. byDep := make(map[string][]cryptoDetail) for _, c := range available { byDep[c.dependency] = append(byDep[c.dependency], c) } + depNames := make([]string, 0, len(byDep)) + for dep := range byDep { + depNames = append(depNames, dep) + } + sort.Strings(depNames) - for dep, algos := range byDep { + for _, dep := range depNames { var algoStrs []string - for _, a := range algos { + for _, a := range byDep[dep] { algoStrs = append(algoStrs, fmt.Sprintf("%s %s", riskIcon(a.risk), a.algorithm)) } fmt.Fprintf(w, " %s\n", dep) @@ -239,7 +351,7 @@ func (f *TableFormatter) printSimpleBreakdown(w io.Writer, allCrypto []cryptoDet for _, c := range vulnerable { timeline := formatTimelineShort(getTimeline(c.algorithm)) effort := formatEffortShort(getEffort(c.algorithm)) - fmt.Fprintf(w, " 🔴 %-14s %-12s %-12s %s\n", c.algorithm, timeline, effort, c.dependency) + fmt.Fprintf(w, " %-4s %-14s %-12s %-12s %s\n", riskIcon(c.risk), c.algorithm, timeline, effort, c.dependency) } fmt.Fprintln(w) } @@ -250,7 +362,7 @@ func (f *TableFormatter) printSimpleBreakdown(w io.Writer, allCrypto []cryptoDet for _, c := range partial { timeline := formatTimelineShort(getTimeline(c.algorithm)) effort := formatEffortShort(getEffort(c.algorithm)) - fmt.Fprintf(w, " 🟡 %-14s %-12s %-12s %s\n", c.algorithm, timeline, effort, c.dependency) + fmt.Fprintf(w, " %-4s %-14s %-12s %-12s %s\n", riskIcon(c.risk), c.algorithm, timeline, effort, c.dependency) } fmt.Fprintln(w) } @@ -259,7 +371,7 @@ func (f *TableFormatter) printSimpleBreakdown(w io.Writer, allCrypto []cryptoDet fmt.Fprintln(w, "[OK] QUANTUM SAFE:") fmt.Fprintln(w, strings.Repeat("─", 90)) for _, c := range safe { - fmt.Fprintf(w, " 🟢 %-14s %s\n", c.algorithm, c.dependency) + fmt.Fprintf(w, " %-4s %-14s %s\n", riskIcon(c.risk), c.algorithm, c.dependency) } fmt.Fprintln(w) } @@ -270,7 +382,7 @@ func (f *TableFormatter) printSimpleBreakdown(w io.Writer, allCrypto []cryptoDet fmt.Fprintln(w, "[?] UNKNOWN RISK:") fmt.Fprintln(w, strings.Repeat("─", 90)) for _, c := range unknown { - fmt.Fprintf(w, " ⚪ %-14s %s\n", c.algorithm, c.dependency) + fmt.Fprintf(w, " %-4s %-14s %s\n", riskIcon(c.risk), c.algorithm, c.dependency) } fmt.Fprintln(w) } @@ -440,22 +552,42 @@ func filterByRisk(crypto []cryptoDetail, risk types.QuantumRisk) []cryptoDetail return result } +// sortByRisk orders findings highest risk first. +// +// The comparison has to be total. Ordering on risk alone left every same-risk +// finding in an arbitrary relative position, and since sort.Slice is not stable +// the same scan printed its rows in a different order between runs. func sortByRisk(crypto []cryptoDetail) { sort.Slice(crypto, func(i, j int) bool { - return riskPriority(crypto[i].risk) > riskPriority(crypto[j].risk) + a, b := crypto[i], crypto[j] + if pa, pb := riskPriority(a.risk), riskPriority(b.risk); pa != pb { + return pa > pb + } + if a.algorithm != b.algorithm { + return a.algorithm < b.algorithm + } + return a.dependency < b.dependency }) } +// riskIcon maps a risk level to the marker shown beside a finding. It is the +// single place that decision is made. +// +// These are the same ASCII tokens the section headers already use, so a reader +// never needs a legend to connect a row to its section. They also survive a pipe +// into a file, a screen reader, a terminal without an emoji font, and a +// fixed-width column, none of which was true of the coloured circles that used +// to be here. CSNP output carries no emoji. func riskIcon(risk types.QuantumRisk) string { switch risk { case types.RiskVulnerable: - return "🔴" + return "[!]" case types.RiskPartial: - return "🟡" + return "[~]" case types.RiskSafe: - return "🟢" + return "[OK]" default: - return "⚪" + return "[?]" } } @@ -579,27 +711,40 @@ func (f *TableFormatter) FormatMulti(result *types.MultiProjectResult, w io.Writ return errors.New("writer cannot be nil") } - // If there's only one project, just format it normally + // Report unread manifests first. They change how every number below should + // be read, so they cannot go in a footer. + PrintSkipped(w, result.RootPath, result.Skipped) + + // If there's only one project, just format it normally. It still gets the + // scan root: without it, a single-manifest repository, which is the most + // common shape there is, printed an absolute manifest path in the table while + // markdown printed a relative one for the same run, and the skip list printed + // above it by PrintSkipped was relative in the same document. if len(result.Projects) == 1 { - return f.Format(result.Projects[0], w) + // The root, once, before the report. Relativizing the manifest without + // it left this document with no absolute path anywhere, so a reader had + // nothing to resolve "./package.json" against. Every other format + // declares its root; this one had stopped. + fmt.Fprintf(w, "\nScanning %s...\n", reportSafe(scanRootDir(result.RootPath))) + return f.formatProject(result.Projects[0], result.RootPath, w) } // Header showing discovered projects - fmt.Fprintf(w, "\nScanning %s...\n", result.RootPath) + fmt.Fprintf(w, "\nScanning %s...\n", reportSafe(scanRootDir(result.RootPath))) fmt.Fprintf(w, "Found %d projects:\n", len(result.Projects)) for _, p := range result.Projects { relPath := getRelativePath(result.RootPath, p.Manifest) - fmt.Fprintf(w, " - %s (%s)\n", relPath, p.Ecosystem) + fmt.Fprintf(w, " - %s (%s)\n", reportSafe(relPath), p.Ecosystem) } fmt.Fprintln(w) // Format each project for i, project := range result.Projects { relPath := getRelativePath(result.RootPath, project.Manifest) - fmt.Fprintf(w, "=== %s (%s) ===\n", relPath, project.Ecosystem) + fmt.Fprintf(w, "=== %s (%s) ===\n", reportSafe(relPath), project.Ecosystem) // Use the single-project formatter for each project - if err := f.Format(project, w); err != nil { + if err := f.formatProject(project, result.RootPath, w); err != nil { return err } @@ -618,6 +763,13 @@ func (f *TableFormatter) FormatMulti(result *types.MultiProjectResult, w io.Writ result.TotalSummary.QuantumVulnerable, result.TotalSummary.QuantumPartial, ) + // The aggregate is the line a reader takes away, so it carries the filter + // annotation too. Printing it only under each project left the TOTAL row + // describing a filtered scan as though it were complete. + if result.TotalSummary.FilteredOut > 0 { + fmt.Fprintf(w, "FILTERED: %d further finding(s) excluded by --risk or --min-severity.\n", + result.TotalSummary.FilteredOut) + } if result.TotalSummary.ReachabilityAnalyzed { fmt.Fprintf(w, "REACHABILITY: %d confirmed | %d reachable | %d available-only\n", result.TotalSummary.ConfirmedCrypto, @@ -630,16 +782,23 @@ func (f *TableFormatter) FormatMulti(result *types.MultiProjectResult, w io.Writ return nil } -// getRelativePath returns a relative path from root to target. +// getRelativePath renders target relative to the scan root, in the "./x" form +// the project list uses. +// +// It asks relativeToRoot, the same helper CBOM and SARIF use. The string-prefix +// test it replaced was a third implementation of that comparison, and it was +// wrong in both directions: it compared the raw root against absolutized +// manifest paths, so `cryptodeps analyze .` printed absolute paths while +// `cryptodeps analyze /abs/path` printed relative ones for the same tree; and a +// shared string prefix produced a path that does not exist, with +// getRelativePath("/repo", "/repository/go.mod") returning "./sitory/go.mod". func getRelativePath(root, target string) string { - // Simple approach: remove root prefix if present - if strings.HasPrefix(target, root) { - rel := strings.TrimPrefix(target, root) - rel = strings.TrimPrefix(rel, "/") - if rel == "" { - return "." - } - return "./" + rel + rel, underRoot := relativeToRoot(scanRootDir(root), target) + if !underRoot { + return rel + } + if rel == "." { + return "." } - return target + return "./" + rel } diff --git a/pkg/output/verdict.go b/pkg/output/verdict.go new file mode 100644 index 0000000..9d641d3 --- /dev/null +++ b/pkg/output/verdict.go @@ -0,0 +1,159 @@ +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) +// SPDX-License-Identifier: Apache-2.0 + +package output + +import ( + "fmt" + + "github.com/csnp/qramm-cryptodeps/pkg/types" +) + +// noFindingsCase says why a scan produced no findings. +// +// This lives in one place because the first version of the fix did not. The +// table formatter learned to distinguish these cases while markdown, JSON, CBOM +// and SARIF kept reporting a clean scan, so the same false-clean verdict the fix +// was written to remove survived in four of the five formats a user can ask for. +// Every formatter now classifies through this function and only chooses its own +// wording, so a new case cannot reach one format and miss the others. +type noFindingsCase int + +const ( + // caseFiltered means findings exist and a filter withheld them. It is + // deliberately first: an absence produced by a filter must never be + // reported as an absence of findings. Order is the safety property, so the + // question "did we withhold anything" is asked before any conclusion about + // what is present. + caseFiltered noFindingsCase = iota + // caseNoDependencies means the manifest declared nothing to analyze. + caseNoDependencies + // caseNothingExamined means every dependency was absent from the database, + // so the scan drew no conclusion. Reporting this as clean is a false + // negative on the tool's core question. + caseNothingExamined + // caseGenuinelyClean means dependencies were examined and carried no + // cryptography. + caseGenuinelyClean +) + +// classifyNoFindings decides which case a findings-free summary falls into. +// Callers must only reach it when no finding survived to be reported. +func classifyNoFindings(s types.ScanSummary) noFindingsCase { + switch { + case s.FilteredOut > 0: + return caseFiltered + case s.TotalDependencies == 0: + return caseNoDependencies + case s.NotInDatabase >= s.TotalDependencies: + return caseNothingExamined + default: + return caseGenuinelyClean + } +} + +// hasAnyCrypto reports whether any dependency carried a surviving finding. +func hasAnyCrypto(deps []types.DependencyResult) bool { + for _, dep := range deps { + if dep.Analysis != nil && len(dep.Analysis.Crypto) > 0 { + return true + } + } + return false +} + +// coverageNote is what one project's scan failed to establish. +// +// The machine-readable formats need this as data rather than prose, but they +// must reach it through the same classification the human formats use. The first +// attempt at this re-derived the conditions inline in sarif.go and cbom.go +// instead, with the test applied to the whole run rather than per project and +// without checking whether the project had findings at all. The result was a +// SARIF document carrying two CRITICAL findings alongside a notification saying +// no conclusion about cryptographic usage could be drawn, and a mixed workspace +// where 21 of 24 dependencies went unexamined without either format saying so. +type coverageNote struct { + // Manifest is the project the note is about, empty for a single-project run. + Manifest string + // Case is the shared classification. + Case noFindingsCase + // Summary is that project's summary, for rendering the numbers. + Summary types.ScanSummary +} + +// coverageNotes returns a note for each project that produced no findings and +// whose emptiness therefore needs explaining. +// +// A project with findings gets no note: its results speak for it, and saying +// "nothing was examined" beside a populated result set is simply false. +// caseGenuinelyClean also gets no note, because an empty result set is exactly +// what it means. +func coverageNotes(projects []*types.ScanResult) []coverageNote { + var notes []coverageNote + for _, p := range projects { + if p == nil { + continue + } + + // Withheld findings are a property of the SCAN, not of an empty report, + // so this is asked before and independently of the no-findings + // classification. Routing it through classifyNoFindings, which by + // contract only speaks about scans that produced nothing, meant a + // partially filtered scan said nothing at all: 28 findings withheld + // beside 38 reported, and neither SARIF nor CBOM mentioned it, because + // one finding had survived. + if p.Summary.FilteredOut > 0 { + notes = append(notes, coverageNote{ + Manifest: p.Manifest, + Case: caseFiltered, + Summary: p.Summary, + }) + } + + if hasAnyCrypto(p.Dependencies) { + continue + } + c := classifyNoFindings(p.Summary) + // caseFiltered is already handled above, and caseGenuinelyClean needs no + // explanation: an empty result set is exactly what it means. + if c == caseGenuinelyClean || c == caseFiltered { + continue + } + notes = append(notes, coverageNote{Manifest: p.Manifest, Case: c, Summary: p.Summary}) + } + return notes +} + +// Text renders a note for a machine-readable consumer. +func (n coverageNote) Text() string { + switch n.Case { + case caseFiltered: + return fmt.Sprintf("%d finding(s) were detected and withheld by --risk or --min-severity. "+ + "This is a filtered subset, not every finding.", n.Summary.FilteredOut) + case caseNoDependencies: + return "No dependencies were declared, so nothing was analyzed." + case caseNothingExamined: + return fmt.Sprintf("None of the %d dependencies are present in the crypto database, so no "+ + "conclusion about cryptographic usage was drawn. An empty result set here means "+ + "nothing was examined, not that nothing was found.", n.Summary.TotalDependencies) + default: + return "" + } +} + +// Level maps a note to a SARIF notification level. +// +// Only a scan that failed to establish something warns. A filtered report and a +// manifest that declared no dependencies are both complete and correct states +// that merely need explaining, so they are notes. Warning on every one of them +// buried the signal: a healthy npm workspace produced three warnings saying +// "this package.json has no dependencies" and none saying findings were +// withheld. +func (n coverageNote) Level() string { + switch n.Case { + case caseFiltered, caseNoDependencies: + return "note" + default: + return "warning" + } +} diff --git a/pkg/output/verdict_test.go b/pkg/output/verdict_test.go new file mode 100644 index 0000000..c603706 --- /dev/null +++ b/pkg/output/verdict_test.go @@ -0,0 +1,229 @@ +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) +// SPDX-License-Identifier: Apache-2.0 + +package output + +import ( + "bytes" + "strings" + "testing" + "unicode" + + "github.com/csnp/qramm-cryptodeps/pkg/types" +) + +// renderTable formats a scan result the way the CLI does. +func renderTable(t *testing.T, result *types.ScanResult) string { + t.Helper() + var buf bytes.Buffer + if err := (&TableFormatter{Options: DefaultOptions()}).Format(result, &buf); err != nil { + t.Fatalf("format: %v", err) + } + return buf.String() +} + +// cleanVerdict is the phrase the tool uses when it has looked and found nothing. +// It must not appear when the tool has not looked. +const cleanVerdict = "No cryptographic usage detected" + +// TestAllUnknownIsNotReportedAsClean covers the case where every dependency is +// absent from the database. +// +// The scan examined nothing, and reporting "No cryptographic usage detected" is +// a false negative on the tool's core question. The JSON for the same scan said +// notInDatabase: 3 while the table said the project was clean. +func TestAllUnknownIsNotReportedAsClean(t *testing.T) { + result := &types.ScanResult{ + Manifest: "requirements.txt", + Ecosystem: types.EcosystemPyPI, + Dependencies: []types.DependencyResult{ + {Dependency: types.Dependency{Name: "rsa", Version: "4.9"}}, + {Dependency: types.Dependency{Name: "requests"}}, + {Dependency: types.Dependency{Name: "certifi"}}, + }, + Summary: types.ScanSummary{TotalDependencies: 3, DirectDependencies: 3, NotInDatabase: 3}, + } + + out := renderTable(t, result) + + if strings.Contains(out, cleanVerdict) { + t.Errorf("scan that examined nothing reported a clean result:\n%s", out) + } + if !strings.Contains(out, "--deep") { + t.Errorf("verdict does not tell the user how to actually analyze these packages:\n%s", out) + } +} + +// TestFilteredToEmptyIsNotReportedAsClean covers findings that exist but were +// withheld by --risk or --min-severity. +// +// This is the same false-clean verdict as the all-unknown case reached from a +// different direction, and it was introduced by the fix that made those flags +// work at all. +func TestFilteredToEmptyIsNotReportedAsClean(t *testing.T) { + result := &types.ScanResult{ + Manifest: "package.json", + Ecosystem: types.EcosystemNPM, + Dependencies: []types.DependencyResult{ + { + Dependency: types.Dependency{Name: "node-forge", Version: "1.3.1"}, + InDatabase: true, + Analysis: &types.PackageAnalysis{Package: "node-forge"}, + }, + }, + Summary: types.ScanSummary{TotalDependencies: 1, FilteredOut: 13}, + } + + out := renderTable(t, result) + + if strings.Contains(out, cleanVerdict) { + t.Errorf("filtered-away findings reported as a clean result:\n%s", out) + } + if !strings.Contains(out, "13") { + t.Errorf("verdict does not say how many findings were withheld:\n%s", out) + } + if !strings.Contains(out, "--risk") { + t.Errorf("verdict does not name the filter responsible:\n%s", out) + } +} + +// TestGenuinelyCleanScanStillSaysSo guards the opposite direction: the honest +// clean verdict must survive. Without this, a test suite could be satisfied by a +// tool that never reports anything as clean. +func TestGenuinelyCleanScanStillSaysSo(t *testing.T) { + result := &types.ScanResult{ + Manifest: "package.json", + Ecosystem: types.EcosystemNPM, + Dependencies: []types.DependencyResult{ + { + Dependency: types.Dependency{Name: "left-pad", Version: "1.3.0"}, + InDatabase: true, + Analysis: &types.PackageAnalysis{Package: "left-pad"}, + }, + }, + Summary: types.ScanSummary{TotalDependencies: 1, DirectDependencies: 1}, + } + + out := renderTable(t, result) + + if !strings.Contains(out, cleanVerdict) { + t.Errorf("a genuinely clean scan no longer reports a clean result:\n%s", out) + } +} + +// TestSkippedManifestsAreReported checks the skip notice reaches the reader and +// names both the file and the reason. +func TestSkippedManifestsAreReported(t *testing.T) { + multi := &types.MultiProjectResult{ + RootPath: "/repo", + Projects: []*types.ScanResult{{ + Manifest: "/repo/good/package.json", + Summary: types.ScanSummary{TotalDependencies: 1, DirectDependencies: 1}, + Dependencies: []types.DependencyResult{{ + Dependency: types.Dependency{Name: "left-pad"}, + InDatabase: true, + Analysis: &types.PackageAnalysis{Package: "left-pad"}, + }}, + }}, + Skipped: []types.SkippedManifest{ + {Path: "/repo/broken/package.json", Reason: "not valid JSON: unexpected end of JSON input"}, + }, + } + + var buf bytes.Buffer + if err := (&TableFormatter{Options: DefaultOptions()}).FormatMulti(multi, &buf); err != nil { + t.Fatalf("format: %v", err) + } + out := buf.String() + + if !strings.Contains(out, "./broken/package.json") { + t.Errorf("skipped manifest is not named in the output:\n%s", out) + } + if !strings.Contains(out, "not valid JSON") { + t.Errorf("skipped manifest carries no reason:\n%s", out) + } +} + +// TestTableOutputCarriesNoEmoji enforces the CSNP no-emoji standard on the one +// format that had them. +// +// The markers also have to survive a pipe into a file, a terminal without an +// emoji font, and a screen reader, none of which was true of the coloured +// circles. Box-drawing characters used for rules are not emoji and are allowed. +func TestTableOutputCarriesNoEmoji(t *testing.T) { + result := &types.ScanResult{ + Manifest: "package.json", + Ecosystem: types.EcosystemNPM, + Dependencies: []types.DependencyResult{{ + Dependency: types.Dependency{Name: "node-forge", Version: "1.3.1"}, + InDatabase: true, + Analysis: &types.PackageAnalysis{ + Package: "node-forge", + Crypto: []types.CryptoUsage{ + {Algorithm: "RSA", QuantumRisk: types.RiskVulnerable, Severity: types.SeverityHigh}, + {Algorithm: "AES", QuantumRisk: types.RiskPartial, Severity: types.SeverityMedium}, + {Algorithm: "Ed25519", QuantumRisk: types.RiskSafe, Severity: types.SeverityInfo}, + {Algorithm: "Mystery", QuantumRisk: types.RiskUnknown, Severity: types.SeverityInfo}, + }, + }, + }}, + Summary: types.ScanSummary{TotalDependencies: 1, WithCrypto: 1, QuantumVulnerable: 1, QuantumPartial: 1}, + } + + out := renderTable(t, result) + + // Confirm the fixture actually reached the rows under test, so this cannot + // pass by rendering nothing. + for _, section := range []string{"VULNERABLE", "PARTIAL RISK", "QUANTUM SAFE", "UNKNOWN RISK"} { + if !strings.Contains(out, section) { + t.Fatalf("fixture did not reach the %s section, so the emoji check proves nothing:\n%s", section, out) + } + } + + for _, r := range out { + if isEmoji(r) { + t.Errorf("emoji %U (%q) present in table output", r, string(r)) + } + } + + for _, want := range []string{"[!]", "[~]", "[OK]", "[?]"} { + if !strings.Contains(out, want) { + t.Errorf("ASCII risk token %q missing from output:\n%s", want, out) + } + } +} + +// isEmoji reports whether a rune is a pictographic character. +func isEmoji(r rune) bool { + switch { + case r >= 0x1F300 && r <= 0x1FAFF: // pictographs, symbols, supplemental + return true + case r >= 0x2600 && r <= 0x27BF: // misc symbols and dingbats + return true + case r == 0xFE0F: // variation selector 16, the emoji presentation marker + return true + case r >= 0x2B00 && r <= 0x2BFF: // arrows and geometric shapes used as emoji + return true + default: + return false + } +} + +// TestRiskIconIsASCIIAndUnique checks the single mapping point. +func TestRiskIconIsASCIIAndUnique(t *testing.T) { + seen := make(map[string]types.QuantumRisk) + for _, risk := range []types.QuantumRisk{ + types.RiskVulnerable, types.RiskPartial, types.RiskSafe, types.RiskUnknown, + } { + token := riskIcon(risk) + for _, r := range token { + if r > unicode.MaxASCII { + t.Errorf("riskIcon(%s) = %q contains non-ASCII rune %U", risk, token, r) + } + } + if prev, dup := seen[token]; dup { + t.Errorf("riskIcon(%s) and riskIcon(%s) both return %q", risk, prev, token) + } + seen[token] = risk + } +} diff --git a/pkg/types/types.go b/pkg/types/types.go index 1b1caa5..2ece379 100644 --- a/pkg/types/types.go +++ b/pkg/types/types.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 // Package types defines the core data structures used throughout CryptoDeps. @@ -10,10 +10,10 @@ import "time" type Ecosystem string const ( - EcosystemGo Ecosystem = "go" - EcosystemNPM Ecosystem = "npm" - EcosystemPyPI Ecosystem = "pypi" - EcosystemMaven Ecosystem = "maven" + EcosystemGo Ecosystem = "go" + EcosystemNPM Ecosystem = "npm" + EcosystemPyPI Ecosystem = "pypi" + EcosystemMaven Ecosystem = "maven" EcosystemUnknown Ecosystem = "unknown" ) @@ -47,7 +47,7 @@ type Dependency struct { Name string `json:"name" yaml:"name"` Version string `json:"version" yaml:"version"` Ecosystem Ecosystem `json:"ecosystem" yaml:"ecosystem"` - Direct bool `json:"direct" yaml:"direct"` // true if direct dependency, false if transitive + Direct bool `json:"direct" yaml:"direct"` // true if direct dependency, false if transitive Parent string `json:"parent,omitempty" yaml:"parent,omitempty"` // parent dependency (for transitive) } @@ -101,13 +101,13 @@ type CryptoUsage struct { QuantumRisk QuantumRisk `json:"quantumRisk" yaml:"quantumRisk"` Severity Severity `json:"severity" yaml:"severity"` Location Location `json:"location" yaml:"location"` - CallPath []string `json:"callPath,omitempty" yaml:"callPath,omitempty"` // trace from public API to crypto - InExported bool `json:"inExported,omitempty" yaml:"inExported,omitempty"` // whether in exported/public function - Function string `json:"function,omitempty" yaml:"function,omitempty"` // containing function name - Remediation string `json:"remediation,omitempty" yaml:"remediation,omitempty"` // migration guidance - Confidence Confidence `json:"confidence,omitempty" yaml:"confidence,omitempty"` // verified, high, medium, low + CallPath []string `json:"callPath,omitempty" yaml:"callPath,omitempty"` // trace from public API to crypto + InExported bool `json:"inExported,omitempty" yaml:"inExported,omitempty"` // whether in exported/public function + Function string `json:"function,omitempty" yaml:"function,omitempty"` // containing function name + Remediation string `json:"remediation,omitempty" yaml:"remediation,omitempty"` // migration guidance + Confidence Confidence `json:"confidence,omitempty" yaml:"confidence,omitempty"` // verified, high, medium, low Reachability Reachability `json:"reachability,omitempty" yaml:"reachability,omitempty"` // CONFIRMED, REACHABLE, AVAILABLE - Traces []CallTrace `json:"traces,omitempty" yaml:"traces,omitempty"` // paths from user code to this crypto + Traces []CallTrace `json:"traces,omitempty" yaml:"traces,omitempty"` // paths from user code to this crypto } // AnalysisMetadata contains information about how the analysis was performed. @@ -161,25 +161,65 @@ type ScanResult struct { // ScanSummary provides aggregate statistics for a scan. type ScanSummary struct { - TotalDependencies int `json:"totalDependencies" yaml:"totalDependencies"` - DirectDependencies int `json:"directDependencies" yaml:"directDependencies"` - WithCrypto int `json:"withCrypto" yaml:"withCrypto"` - QuantumVulnerable int `json:"quantumVulnerable" yaml:"quantumVulnerable"` - QuantumPartial int `json:"quantumPartial" yaml:"quantumPartial"` - NotInDatabase int `json:"notInDatabase" yaml:"notInDatabase"` + TotalDependencies int `json:"totalDependencies" yaml:"totalDependencies"` + DirectDependencies int `json:"directDependencies" yaml:"directDependencies"` + WithCrypto int `json:"withCrypto" yaml:"withCrypto"` + QuantumVulnerable int `json:"quantumVulnerable" yaml:"quantumVulnerable"` + QuantumPartial int `json:"quantumPartial" yaml:"quantumPartial"` + NotInDatabase int `json:"notInDatabase" yaml:"notInDatabase"` + // FilteredOut counts findings that were detected and then withheld by + // --risk or --min-severity. Without it, a filter that matches nothing is + // indistinguishable from a project with no cryptography, and the report + // would state the second while the first is true. + FilteredOut int `json:"filteredOut,omitempty" yaml:"filteredOut,omitempty"` // Reachability stats (only populated when reachability analysis is enabled) ReachabilityAnalyzed bool `json:"reachabilityAnalyzed,omitempty" yaml:"reachabilityAnalyzed,omitempty"` - ConfirmedCrypto int `json:"confirmedCrypto,omitempty" yaml:"confirmedCrypto,omitempty"` // Direct calls from user code - ReachableCrypto int `json:"reachableCrypto,omitempty" yaml:"reachableCrypto,omitempty"` // In call graph - AvailableCrypto int `json:"availableCrypto,omitempty" yaml:"availableCrypto,omitempty"` // In deps but not called + ConfirmedCrypto int `json:"confirmedCrypto,omitempty" yaml:"confirmedCrypto,omitempty"` // Direct calls from user code + ReachableCrypto int `json:"reachableCrypto,omitempty" yaml:"reachableCrypto,omitempty"` // In call graph + AvailableCrypto int `json:"availableCrypto,omitempty" yaml:"availableCrypto,omitempty"` // In deps but not called +} + +// SkippedManifest records a file that was recognised as a manifest but could not +// be analyzed, and why. +// +// A scanner may skip input. It must never skip it silently: a manifest broken by +// a bad merge would otherwise vanish from the report while the summary still +// reads clean and CI still goes green. +type SkippedManifest struct { + Path string `json:"path" yaml:"path"` + Reason string `json:"reason" yaml:"reason"` + // Unsupported separates "cryptodeps has no parser for this ecosystem" from + // "this file should have been readable and was not". + // + // Both are reported, because a file that looks like a manifest and was not + // read is something the user is entitled to know either way. Only the second + // means the scan is incomplete, so only the second forces exit 2. Collapsing + // the two made every polyglot repository an analysis error, and then + // dropping the unsupported ones from discovery to fix that made them + // invisible instead, which is the failure this type exists to prevent. + Unsupported bool `json:"unsupported,omitempty" yaml:"unsupported,omitempty"` +} + +// IncompleteScan reports whether any skip means the scan failed to cover input +// it should have covered. An unsupported ecosystem is not such a case. +func IncompleteScan(skipped []SkippedManifest) bool { + for _, s := range skipped { + if !s.Unsupported { + return true + } + } + return false } // MultiProjectResult represents the result of scanning multiple projects/manifests. type MultiProjectResult struct { - RootPath string `json:"rootPath" yaml:"rootPath"` - ScanDate time.Time `json:"scanDate" yaml:"scanDate"` - Projects []*ScanResult `json:"projects" yaml:"projects"` - TotalSummary ScanSummary `json:"totalSummary" yaml:"totalSummary"` + RootPath string `json:"rootPath" yaml:"rootPath"` + ScanDate time.Time `json:"scanDate" yaml:"scanDate"` + Projects []*ScanResult `json:"projects" yaml:"projects"` + // Skipped lists manifests that were found but could not be analyzed. An + // empty scan with a non-empty Skipped is an incomplete scan, not a clean one. + Skipped []SkippedManifest `json:"skipped,omitempty" yaml:"skipped,omitempty"` + TotalSummary ScanSummary `json:"totalSummary" yaml:"totalSummary"` } // AggregateResults combines multiple scan results into a single multi-project result. @@ -198,6 +238,10 @@ func AggregateResults(rootPath string, results []*ScanResult) *MultiProjectResul multi.TotalSummary.QuantumVulnerable += r.Summary.QuantumVulnerable multi.TotalSummary.QuantumPartial += r.Summary.QuantumPartial multi.TotalSummary.NotInDatabase += r.Summary.NotInDatabase + // Without this the aggregate reported zero withheld findings while the + // per-project summaries reported dozens, so the totals a reader + // actually looks at described a filtered scan as a complete one. + multi.TotalSummary.FilteredOut += r.Summary.FilteredOut multi.TotalSummary.ConfirmedCrypto += r.Summary.ConfirmedCrypto multi.TotalSummary.ReachableCrypto += r.Summary.ReachableCrypto multi.TotalSummary.AvailableCrypto += r.Summary.AvailableCrypto diff --git a/pkg/types/types_test.go b/pkg/types/types_test.go index 8aa6b64..cf9c6ab 100644 --- a/pkg/types/types_test.go +++ b/pkg/types/types_test.go @@ -1,4 +1,4 @@ -// Copyright 2024-2025 CSNP (csnp.org) +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) // SPDX-License-Identifier: Apache-2.0 package types @@ -335,3 +335,43 @@ func TestSeverityConstants(t *testing.T) { }) } } + +// TestAggregateSumsFilteredOut guards the number a reader of a workspace scan +// actually looks at. +// +// AggregateResults sums nine summary fields, and FilteredOut was not one of +// them, so TotalSummary.FilteredOut stayed 0 while the per-project summaries +// reported dozens of withheld findings. The aggregate row and the multi-project +// JSON therefore described a filtered scan as though it were complete, which is +// the same false-clean claim the per-project verdict was rewritten to avoid. +func TestAggregateSumsFilteredOut(t *testing.T) { + multi := AggregateResults("/repo", []*ScanResult{ + {Summary: ScanSummary{TotalDependencies: 3, FilteredOut: 4}}, + {Summary: ScanSummary{TotalDependencies: 2, FilteredOut: 6}}, + {Summary: ScanSummary{TotalDependencies: 1}}, + }) + + if got, want := multi.TotalSummary.FilteredOut, 10; got != want { + t.Errorf("TotalSummary.FilteredOut = %d, want %d; a workspace scan reports the "+ + "aggregate, so withheld findings that are not summed are invisible", got, want) + } + // Guard the fixture: if the projects carried no withheld findings the + // assertion above would pass against an implementation that never sums. + if multi.TotalSummary.TotalDependencies != 6 { + t.Fatalf("fixture did not aggregate at all: TotalDependencies = %d, want 6", + multi.TotalSummary.TotalDependencies) + } +} + +// TestAggregateReportsNoFilterWhenNoneWasApplied is the paired guard: the field +// must stay absent from an unfiltered scan rather than acquiring a stray count. +func TestAggregateReportsNoFilterWhenNoneWasApplied(t *testing.T) { + multi := AggregateResults("/repo", []*ScanResult{ + {Summary: ScanSummary{TotalDependencies: 3, QuantumVulnerable: 2}}, + {Summary: ScanSummary{TotalDependencies: 2, QuantumVulnerable: 1}}, + }) + if multi.TotalSummary.FilteredOut != 0 { + t.Errorf("TotalSummary.FilteredOut = %d on an unfiltered scan, want 0", + multi.TotalSummary.FilteredOut) + } +} diff --git a/pkg/version/version.go b/pkg/version/version.go new file mode 100644 index 0000000..4d8abd5 --- /dev/null +++ b/pkg/version/version.go @@ -0,0 +1,66 @@ +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) +// SPDX-License-Identifier: Apache-2.0 + +// Package version holds the single source of truth for the running build's +// identity. +// +// GoReleaser injects the real values into main via -X main.version, so main is +// the only place that learns them from the linker. Every other package reads +// them from here. Before this existed each output format carried its own +// literal, and they drifted: a 1.3.0 binary emitted SARIF claiming 1.0.0, a CBOM +// claiming 1.0.0 and JSON claiming nothing at all. SARIF and CBOM are provenance +// artifacts, so a stale literal there is a false provenance record, not a +// cosmetic bug. +package version + +const ( + // devVersion is what an un-injected build reports. It matches the default + // in main so a plain `go build` is honest about not being a release. + devVersion = "dev" + unknown = "unknown" + noCommit = "none" +) + +var ( + version = devVersion + commit = noCommit + date = unknown +) + +// Set records the build identity. main calls this once at startup with the +// values the linker injected. Empty arguments are ignored so that a build +// without -X keeps the honest defaults rather than reporting empty strings. +func Set(v, c, d string) { + if v != "" { + version = v + } + if c != "" { + commit = c + } + if d != "" { + date = d + } +} + +// Version returns the tool version, for example "1.3.0" for a release build or +// "dev" for a local one. +func Version() string { return version } + +// Commit returns the commit the binary was built from. +func Commit() string { return commit } + +// Date returns the build date. +func Date() string { return date } + +// Name is the tool name reported in machine-readable output. It is lower case +// to match the binary and the purl/package identity, not the display name. +const Name = "cryptodeps" + +// DisplayName is the tool name for human-facing and SARIF driver use. +const DisplayName = "CryptoDeps" + +// InformationURI is the tool's home, reported in SARIF. +const InformationURI = "https://github.com/csnp/qramm-cryptodeps" + +// Vendor is the publishing organisation, reported in CBOM. +const Vendor = "CSNP"