From 74a044e4ab02114ebb2d41c26691ec43ce52b0a3 Mon Sep 17 00:00:00 2001 From: Abdel Fane <32418586+thebenignhacker@users.noreply.github.com> Date: Tue, 20 Oct 2026 23:30:46 -0600 Subject: [PATCH 1/3] Write JSON severity by name, fix CBOM primitives and asset types, name skipped node_modules, cap the streamed list and plain help wording (#7) Refs #7 Remaining, as each changes detection, an exit code or the README capture: hybrid classification of mlkem768x25519-sha256; a distinct exit code for tool errors; the score box border; repo-relative finding paths, URL scans included; the migration score framed as a path forward; the recommendation line; RC2 named by algorithm; assert messages and comments counted as withheld narrative; jsonwebtoken npm dependency inventory. --- CHANGELOG.md | 59 +++++++++ docs/cbom-fields.md | 82 ++++++------ docs/commands.md | 12 +- docs/how-it-works.md | 2 +- internal/cli/help_stream_test.go | 189 +++++++++++++++++++++++++++ internal/cli/json_shape_test.go | 104 +++++++++++++++ internal/cli/pqc.go | 6 +- internal/cli/root.go | 6 +- internal/cli/scan.go | 86 +++++++++++-- pkg/reporter/cbom.go | 93 ++++++++++++-- pkg/reporter/cbom_assets_test.go | 205 ++++++++++++++++++++++++++++++ pkg/reporter/text.go | 43 ++++++- pkg/reporter/text_wording_test.go | 121 ++++++++++++++++++ pkg/scanner/scanner.go | 23 ++++ pkg/scanner/skipped_test.go | 40 ++++++ pkg/types/types.go | 63 +++++++++ pkg/types/types_test.go | 96 ++++++++++++++ 17 files changed, 1148 insertions(+), 82 deletions(-) create mode 100644 internal/cli/help_stream_test.go create mode 100644 internal/cli/json_shape_test.go create mode 100644 pkg/reporter/cbom_assets_test.go create mode 100644 pkg/reporter/text_wording_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 3cf2567..609a686 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,65 @@ All notable changes to CryptoScan are recorded here. This project follows ## [Unreleased] +### Added + +- **A scan names the `node_modules` directories it did not enter.** Installed + packages were left out of every scan without a word. The text report of + `scan` now lists each `node_modules` directory the walk passed over, + relative to the scan root, under + `N dependency directories were not scanned, so the packages installed there are not in this report:`, + and says that passing a package's directory to `cryptoscan scan` scans it. + JSON output carries the list as `skippedDependencyDirs`, an empty array when + there is none, and JSON, CSV, SARIF and CBOM output print it on stderr. A + `node_modules` directory inside another one is not reached, so it is not + listed. + +### Changed + +- **A JSON finding names its severity.** `--format json` wrote each finding's + severity as a number from 0 to 4, as in `"severity": 3`, while + `summary.bySeverity` is keyed by name. Each finding now carries the name, as + in `"severity": "HIGH"`, the form `summary.bySeverity`, the text report, CSV + and `cryptoscan patterns list --format json` use. A reader that compared the + number has to compare the name. A JSON report written by an earlier release, + with numbers, still loads as a `--baseline`. +- **Every JSON finding has `algorithm` and `keySize`.** Both keys were left out + of a finding that had no value for them. They are now on every finding, as + `null` when the pattern names no algorithm or records no key size, and they + are the last two keys of the finding object. +- **CBOM components describe more of what was found.** In `--format cbom` and + `cryptoscan cbom` output, AES-GCM and ChaCha20-Poly1305 have the primitive + `ae`, the CycloneDX value for authenticated encryption, where they had + `other` and `mac`. TLS findings are `protocol` assets with + `protocolProperties` of type `tls` and the TLS version their match names, + one component per version, where they were algorithm assets with primitive + `other`. Findings in the `Configuration`, `Library Import` and + `PQC Library Import` categories that name no algorithm, such as + `Hardcoded Key Size` and `Post-Quantum Crypto Library`, are left out of the + CBOM rather than written as algorithm components with no properties; the + other formats still report them. A finding with a recorded key size is a + component of its own, named with the size, such as `RSA-1024` with + `classicalSecurityLevel` 80. One `RSA` component used to hold every RSA + finding with the level of the first one, which on `crypto-samples` gave 80 + to all 15 occurrences; RSA findings with no recorded size now carry no + level. [CBOM fields](docs/cbom-fields.md) lists each field as it is now. +- **The text report.** The footer no longer ends with the CSNP mission + statement. A file with one finding reads `(1 finding)` in the TOP PRIORITY + FILES rows and in the `--group-by file` headers, where it read + `(1 findings)`. The list printed while the scan runs stops after 200 + matches with the line + `Matches after the first 200 are not listed while the scan runs; the report lists every finding.`, + and the stream footer says how many matches the scan found; the report + after it still lists every finding. +- **Help text.** `cryptoscan --help` opens with what the tool finds, where it + opened with `A powerful tool for discovering cryptographic algorithms`. + `scan --help` says that the report classes SHA-2 and AES as PARTIAL and that + `--include-quantum-safe` lists the SHA-2 findings, and the AES findings at + INFO severity other than AES-GCM and AES-ECB, that a default scan leaves + out, where it called them quantum-safe; `pqc --help` calls them PARTIAL too. The `--fail-on` help says + that, when neither the flag nor `failOn` in the config file is set, the scan + exits 1 when any finding is CRITICAL. + ### Fixed - **A scan that read no files no longer reports a clean result.** A scan of an diff --git a/docs/cbom-fields.md b/docs/cbom-fields.md index 327db9c..7577b69 100644 --- a/docs/cbom-fields.md +++ b/docs/cbom-fields.md @@ -48,19 +48,19 @@ reference. | JSON path | Status | Written when | CycloneDX 1.6 | Set in `cbom.go` | |---|---|---|---|---| -| `bomFormat` | emitted | Every CBOM. Value `CycloneDX`. | [bomFormat](https://cyclonedx.org/docs/1.6/json/#bomFormat) | 523 | -| `specVersion` | emitted | Every CBOM. Value `1.6`. | [specVersion](https://cyclonedx.org/docs/1.6/json/#specVersion) | 524 | -| `serialNumber` | emitted | Every CBOM. A random `urn:uuid` value, new on each run. | [serialNumber](https://cyclonedx.org/docs/1.6/json/#serialNumber) | 525, 651-659 | -| `version` | emitted | Every CBOM. Value `1`. | [version](https://cyclonedx.org/docs/1.6/json/#version) | 526 | -| `metadata.timestamp` | emitted | Every CBOM. The time of the run in UTC, RFC 3339. | [metadata.timestamp](https://cyclonedx.org/docs/1.6/json/#metadata_timestamp) | 528 | -| `metadata.tools[].vendor` | emitted | Every CBOM. Value `CSNP`. | [tools, legacy form](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i1_items_vendor) | 531 | -| `metadata.tools[].name` | emitted | Every CBOM. Value `CryptoScan`. | [tools, legacy form](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i1_items_name) | 532 | -| `metadata.tools[].version` | emitted | Every CBOM. The CryptoScan version. | [tools, legacy form](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i1_items_version) | 533 | -| `metadata.component` | emitted | Every CBOM. It holds a summary named "Cryptographic Inventory Summary" with the migration readiness score, not the scanned project. See F1. | [metadata.component](https://cyclonedx.org/docs/1.6/json/#metadata_component) | 512-520, 536 | +| `bomFormat` | emitted | Every CBOM. Value `CycloneDX`. | [bomFormat](https://cyclonedx.org/docs/1.6/json/#bomFormat) | 592 | +| `specVersion` | emitted | Every CBOM. Value `1.6`. | [specVersion](https://cyclonedx.org/docs/1.6/json/#specVersion) | 593 | +| `serialNumber` | emitted | Every CBOM. A random `urn:uuid` value, new on each run. | [serialNumber](https://cyclonedx.org/docs/1.6/json/#serialNumber) | 594, 722-730 | +| `version` | emitted | Every CBOM. Value `1`. | [version](https://cyclonedx.org/docs/1.6/json/#version) | 595 | +| `metadata.timestamp` | emitted | Every CBOM. The time of the run in UTC, RFC 3339. | [metadata.timestamp](https://cyclonedx.org/docs/1.6/json/#metadata_timestamp) | 597 | +| `metadata.tools[].vendor` | emitted | Every CBOM. Value `CSNP`. | [tools, legacy form](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i1_items_vendor) | 600 | +| `metadata.tools[].name` | emitted | Every CBOM. Value `CryptoScan`. | [tools, legacy form](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i1_items_name) | 601 | +| `metadata.tools[].version` | emitted | Every CBOM. The CryptoScan version. | [tools, legacy form](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i1_items_version) | 602 | +| `metadata.component` | emitted | Every CBOM. It holds a summary named "Cryptographic Inventory Summary" with the migration readiness score, not the scanned project. See F1. | [metadata.component](https://cyclonedx.org/docs/1.6/json/#metadata_component) | 581-589, 605 | | `metadata.authors` | not emitted | The person or organization that ran the scan is not recorded. See F7. | [metadata.authors](https://cyclonedx.org/docs/1.6/json/#metadata_authors) | not in the reporter | | `metadata.manufacturer` | not emitted | See F7. | [metadata.manufacturer](https://cyclonedx.org/docs/1.6/json/#metadata_manufacturer) | not in the reporter | -| `services[]` | not emitted | Declared and never assigned. See F8. | [services](https://cyclonedx.org/docs/1.6/json/#services) | declared at 42 | -| `dependencies[]` | not emitted | The code links a hybrid construction to its parts only when the algorithm name carries both of them. No detection pattern produces such a name, so no dependency is written. See F3. | [dependencies](https://cyclonedx.org/docs/1.6/json/#dependencies) | 452-461, 491-509 | +| `services[]` | not emitted | Declared and never assigned. See F8. | [services](https://cyclonedx.org/docs/1.6/json/#services) | declared at 43 | +| `dependencies[]` | not emitted | The code links a hybrid construction to its parts only when the algorithm name carries both of them. No detection pattern produces such a name, so no dependency is written. See F3. | [dependencies](https://cyclonedx.org/docs/1.6/json/#dependencies) | 521-530, 560-578 | The reporter writes `metadata.tools` in the legacy form, an array of vendor, name and version. CycloneDX 1.6 still accepts that form and marks it @@ -70,32 +70,35 @@ deprecated. | JSON path | Status | Written when | CycloneDX 1.6 | Set in `cbom.go` | |---|---|---|---|---| -| `components[].type` | emitted | Every component. Value `cryptographic-asset`. | [type](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_type) | 392 | -| `components[].bom-ref` | emitted | Every component. `crypto--`, unique within one CBOM. | [bom-ref](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_bom-ref) | 388, 393 | -| `components[].name` | emitted | Every component. The algorithm name, or the detection pattern's name when the finding names no algorithm, as for key material and certificates. | [name](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_name) | 382-385, 394 | -| `components[].description` | emitted | Every component. The detection pattern's description. | [description](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_description) | 395 | -| `components[].version` | not emitted | Declared and never assigned. See F2. | [version](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_version) | declared at 62 | +| `components[].type` | emitted | Every component. Value `cryptographic-asset`. | [type](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_type) | 461 | +| `components[].bom-ref` | emitted | Every component. `crypto--`, unique within one CBOM. | [bom-ref](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_bom-ref) | 457, 462 | +| `components[].name` | emitted | Every component. The algorithm name, or the detection pattern's name when the finding names no algorithm, as for key material and certificates. When the pattern records a key size, the size is added to the name, as in `RSA-1024`, and each size is a component of its own. A protocol is a component per version, so two TLS components can share the name `TLS`. | [name](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_name) | 218-233, 454, 463 | +| `components[].description` | emitted | Every component. The detection pattern's description. | [description](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_description) | 464 | +| `components[].version` | not emitted | Declared and never assigned. See F2. | [version](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_version) | declared at 63 | | `components[].supplier` | not emitted | See F2. | [supplier](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_supplier) | not in the reporter | -| `components[].cryptoProperties.assetType` | emitted | Every component. Findings in the `Certificate` category are written as `certificate`, findings in the `Secret Detection` category as `related-crypto-material`, and everything else as `algorithm`, including TLS findings. | [assetType](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_cryptoProperties_assetType) | 396-398, 156-172 | -| `components[].cryptoProperties.algorithmProperties.primitive` | emitted | Algorithm assets whose finding names an algorithm. | [primitive](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_cryptoProperties_algorithmProperties_primitive) | 427-431, 239-292 | -| `components[].cryptoProperties.algorithmProperties.parameterSetIdentifier` | not emitted | Filled only when the algorithm name carries a parameter set, such as `ML-KEM-768`. The detection patterns report family names such as `ML-KEM`, so no scan reaches it. See F6. | [parameterSetIdentifier](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_cryptoProperties_algorithmProperties_parameterSetIdentifier) | 433-436 | -| `components[].cryptoProperties.algorithmProperties.classicalSecurityLevel` | partial | Algorithm assets where the pattern records a key size, or where a level can be estimated from the name. AES is written as 256 whatever key length the code uses. Omitted when neither gives a value. See F6. | [classicalSecurityLevel](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_cryptoProperties_algorithmProperties_classicalSecurityLevel) | 438-444, 355-371, 585-609 | -| `components[].cryptoProperties.algorithmProperties.nistQuantumSecurityLevel` | partial | Quantum-safe algorithm assets. When the name carries no parameter set, the value written is 1, which can understate a stronger parameter set. Omitted for algorithms that are not quantum-safe. See F6. | [nistQuantumSecurityLevel](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_cryptoProperties_algorithmProperties_nistQuantumSecurityLevel) | 446-450, 306-353 | -| `components[].cryptoProperties.algorithmProperties.executionEnvironment` | not emitted | Declared and never assigned. See F8. | [executionEnvironment](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_cryptoProperties_algorithmProperties_executionEnvironment) | declared at 89 | -| `components[].cryptoProperties.algorithmProperties.implementationPlatform` | not emitted | Declared and never assigned. See F8. | [implementationPlatform](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_cryptoProperties_algorithmProperties_implementationPlatform) | declared at 90 | -| `components[].cryptoProperties.algorithmProperties.certificationLevel` | not emitted | Declared and never assigned. See F8. | [certificationLevel](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_cryptoProperties_algorithmProperties_certificationLevel) | declared at 91 | -| `components[].cryptoProperties.algorithmProperties.mode` | not emitted | Declared and never assigned. See F8. | [mode](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_cryptoProperties_algorithmProperties_mode) | declared at 92 | -| `components[].cryptoProperties.algorithmProperties.padding` | not emitted | Declared and never assigned. See F8. | [padding](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_cryptoProperties_algorithmProperties_padding) | declared at 93 | -| `components[].cryptoProperties.algorithmProperties.cryptoFunctions` | not emitted | Declared and never assigned. See F8. | [cryptoFunctions](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_cryptoProperties_algorithmProperties_cryptoFunctions) | declared at 94 | -| `components[].cryptoProperties.certificateProperties` | not emitted | Certificate findings are written with `assetType` `certificate` and no properties block. See F5. | [certificateProperties](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_cryptoProperties_certificateProperties) | declared at 71 and 99-105 | -| `components[].cryptoProperties.protocolProperties` | not emitted | The code looks for the categories `tls` and `protocol`, but the TLS patterns use `TLS/SSL` and `Deprecated Protocol`, so TLS findings are written as algorithm assets with primitive `other`. See F4. | [protocolProperties](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_cryptoProperties_protocolProperties) | 464-473 | -| `components[].cryptoProperties.relatedCryptoMaterialProperties.type` | emitted | Key material, that is findings in the `Secret Detection` category such as a private key header. Values come from the CycloneDX enum, and anything unrecognized is written as `unknown`. | [relatedCryptoMaterialProperties.type](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_cryptoProperties_relatedCryptoMaterialProperties_type) | 418-423, 210-237 | -| `components[].cryptoProperties.relatedCryptoMaterialProperties.size` | not emitted | Copied from the finding's key size, which no key material pattern records. See F6. | [relatedCryptoMaterialProperties.size](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_cryptoProperties_relatedCryptoMaterialProperties_size) | 421 | -| `components[].cryptoProperties.relatedCryptoMaterialProperties.state` | not emitted | Declared and never assigned. See F8. | [relatedCryptoMaterialProperties.state](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_cryptoProperties_relatedCryptoMaterialProperties_state) | declared at 82 | -| `components[].cryptoProperties.oid` | partial | Algorithms whose name is in the OID table in `pkg/types/types.go`. Of the algorithm names the detection patterns report, only `RSA` is in that table today. See F6. | [oid](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_cryptoProperties_oid) | 410-413, 294-304 | -| `components[].evidence.occurrences[].location` | emitted | Every component, once per occurrence. The file path relative to the scan root. | [location](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_evidence_occurrences_items_location) | 402, 480 | -| `components[].evidence.occurrences[].line` | emitted | Every component, once per occurrence. | [line](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_evidence_occurrences_items_line) | 403, 481 | -| `components[].evidence.occurrences[].symbol` | emitted | Every component, once per occurrence. The matched text. | [symbol](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_evidence_occurrences_items_symbol) | 404, 482 | +| `components[].cryptoProperties.assetType` | emitted | Every component. Findings in the `Certificate` category are written as `certificate`, findings in the `Secret Detection` category as `related-crypto-material`, findings in the `TLS/SSL` and `Deprecated Protocol` categories as `protocol`, and everything else as `algorithm`. A finding in the `Configuration`, `Library Import` or `PQC Library Import` category that names no algorithm, such as `Hardcoded Key Size` or `Post-Quantum Crypto Library`, is not a cryptographic asset and is not written as a component; the other formats still report it. | [assetType](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_cryptoProperties_assetType) | 448-453, 465-467, 162-196 | +| `components[].cryptoProperties.algorithmProperties.primitive` | emitted | Algorithm assets whose finding names an algorithm. AES-GCM and ChaCha20-Poly1305 are written as `ae`, the CycloneDX value for authenticated encryption. | [primitive](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_cryptoProperties_algorithmProperties_primitive) | 496-500, 300-359 | +| `components[].cryptoProperties.algorithmProperties.parameterSetIdentifier` | not emitted | Filled only when the algorithm name carries a parameter set, such as `ML-KEM-768`. The detection patterns report family names such as `ML-KEM`, so no scan reaches it. See F6. | [parameterSetIdentifier](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_cryptoProperties_algorithmProperties_parameterSetIdentifier) | 502-505 | +| `components[].cryptoProperties.algorithmProperties.classicalSecurityLevel` | partial | Algorithm assets where the pattern records a key size, or where a level can be estimated from the name. Each recorded key size is a component of its own, so `RSA-1024` carries 80 and `RSA-2048` carries 112, and RSA findings with no recorded size carry no level. AES is written as 256 whatever key length the code uses. Omitted when neither gives a value. See F6. | [classicalSecurityLevel](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_cryptoProperties_algorithmProperties_classicalSecurityLevel) | 507-513, 422-438, 655-678 | +| `components[].cryptoProperties.algorithmProperties.nistQuantumSecurityLevel` | partial | Quantum-safe algorithm assets. When the name carries no parameter set, the value written is 1, which can understate a stronger parameter set. Omitted for algorithms that are not quantum-safe. See F6. | [nistQuantumSecurityLevel](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_cryptoProperties_algorithmProperties_nistQuantumSecurityLevel) | 515-519, 374-420 | +| `components[].cryptoProperties.algorithmProperties.executionEnvironment` | not emitted | Declared and never assigned. See F8. | [executionEnvironment](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_cryptoProperties_algorithmProperties_executionEnvironment) | declared at 90 | +| `components[].cryptoProperties.algorithmProperties.implementationPlatform` | not emitted | Declared and never assigned. See F8. | [implementationPlatform](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_cryptoProperties_algorithmProperties_implementationPlatform) | declared at 91 | +| `components[].cryptoProperties.algorithmProperties.certificationLevel` | not emitted | Declared and never assigned. See F8. | [certificationLevel](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_cryptoProperties_algorithmProperties_certificationLevel) | declared at 92 | +| `components[].cryptoProperties.algorithmProperties.mode` | not emitted | Declared and never assigned. See F8. | [mode](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_cryptoProperties_algorithmProperties_mode) | declared at 93 | +| `components[].cryptoProperties.algorithmProperties.padding` | not emitted | Declared and never assigned. See F8. | [padding](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_cryptoProperties_algorithmProperties_padding) | declared at 94 | +| `components[].cryptoProperties.algorithmProperties.cryptoFunctions` | not emitted | Declared and never assigned. See F8. | [cryptoFunctions](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_cryptoProperties_algorithmProperties_cryptoFunctions) | declared at 95 | +| `components[].cryptoProperties.certificateProperties` | not emitted | Certificate findings are written with `assetType` `certificate` and no properties block. See F5. | [certificateProperties](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_cryptoProperties_certificateProperties) | declared at 72 and 100-106 | +| `components[].cryptoProperties.protocolProperties` | emitted | Protocol assets, that is TLS findings. | [protocolProperties](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_cryptoProperties_protocolProperties) | 533-542 | +| `components[].cryptoProperties.protocolProperties.type` | emitted | Protocol assets. Value `tls`; the TLS patterns also match SSL versions, which are written as `tls` too. | [protocolProperties.type](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_cryptoProperties_protocolProperties_type) | 535-537, 201-210 | +| `components[].cryptoProperties.protocolProperties.version` | partial | Protocol assets whose match names TLS 1.0, 1.1, 1.2 or 1.3, written as `TLSv1.2`, `TLS_1_3`, `TLS-1.2` or `TLS13`. A match that names an SSL version gets no version. See F4. | [protocolProperties.version](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_cryptoProperties_protocolProperties_version) | 538-541, 699-714 | +| `components[].cryptoProperties.protocolProperties.cipherSuites` | not emitted | Declared and never assigned. See F4. | [protocolProperties.cipherSuites](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_cryptoProperties_protocolProperties_cipherSuites) | declared at 111 | +| `components[].cryptoProperties.relatedCryptoMaterialProperties.type` | emitted | Key material, that is findings in the `Secret Detection` category such as a private key header. Values come from the CycloneDX enum, and anything unrecognized is written as `unknown`. | [relatedCryptoMaterialProperties.type](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_cryptoProperties_relatedCryptoMaterialProperties_type) | 487-492, 271-298 | +| `components[].cryptoProperties.relatedCryptoMaterialProperties.size` | not emitted | Copied from the finding's key size, which no key material pattern records. See F6. | [relatedCryptoMaterialProperties.size](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_cryptoProperties_relatedCryptoMaterialProperties_size) | 490 | +| `components[].cryptoProperties.relatedCryptoMaterialProperties.state` | not emitted | Declared and never assigned. See F8. | [relatedCryptoMaterialProperties.state](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_cryptoProperties_relatedCryptoMaterialProperties_state) | declared at 83 | +| `components[].cryptoProperties.oid` | partial | Algorithms whose name is in the OID table in `pkg/types/types.go`. Of the algorithm names the detection patterns report, only `RSA` is in that table today. See F6. | [oid](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_cryptoProperties_oid) | 479-482, 362-371 | +| `components[].evidence.occurrences[].location` | emitted | Every component, once per occurrence. The file path relative to the scan root. | [location](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_evidence_occurrences_items_location) | 471, 549 | +| `components[].evidence.occurrences[].line` | emitted | Every component, once per occurrence. | [line](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_evidence_occurrences_items_line) | 472, 550 | +| `components[].evidence.occurrences[].symbol` | emitted | Every component, once per occurrence. The matched text. | [symbol](https://cyclonedx.org/docs/1.6/json/#metadata_tools_oneOf_i0_components_items_evidence_occurrences_items_symbol) | 473, 551 | ## NTIA minimum elements @@ -157,9 +160,10 @@ them has a date. name carries both, for example `X25519` and `MLKEM`. The hybrid patterns report `Hybrid-KEM`, `Hybrid-TLS` and `Composite-Signature`, so no link is written. -- **F4. Protocol properties for TLS findings.** Match the categories the TLS - patterns use, so that TLS findings carry `protocolProperties` with the - protocol version. +- **F4. Cipher suites and SSL versions for TLS findings.** TLS findings are + written as protocol assets with the TLS version their match names. They do + not carry `cipherSuites`, and a match that names an SSL version, such as + `SSLv3`, gets no version. - **F5. Certificate properties for certificate findings.** Subject, issuer, validity dates and signature algorithm, where the certificate can be read. - **F6. Parameter sets, key sizes and identifiers.** The patterns report diff --git a/docs/commands.md b/docs/commands.md index 4db058a..4a8a22f 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -38,12 +38,12 @@ The descriptions below are the ones `cryptoscan scan --help` prints. | `-g, --group-by string` | Group the text report by: file | | `-c, --context int` | Lines of source to show before and after each finding (default 3) | | `-p, --progress` | Show scan progress: the number of files scanned so far, updated in place (text output on a terminal) | -| `--stream` | Show findings as they are discovered (default true) | +| `--stream` | Show findings as they are discovered, up to the first 200 matches (default true) | | `--min-severity string` | Minimum severity to report: info, low, medium, high, critical (default "info") | | `--include-imports` | Include library import findings (normally suppressed as low-value) | -| `--include-quantum-safe` | Include quantum-safe algorithm findings (SHA-256, AES-256) | +| `--include-quantum-safe` | Include the SHA-2 and AES findings left out by default, which the report classes as PARTIAL | | `--include-narrative` | Include algorithms named in prose, log messages, documentation and config keys | -| `-v, --verbose` | Show all findings including imports, quantum-safe algorithms and narrative mentions | +| `-v, --verbose` | Show all findings including imports, the SHA-2 and AES findings left out by default, and narrative mentions | | `--no-color` | Disable colored output | | `--pretty` | Pretty print JSON output | | `-h, --help` | help for scan | @@ -54,7 +54,7 @@ CI/CD flags: |------|-------------| | `--ignore string` | Pattern IDs to ignore (comma-separated, e.g., "RSA-001,CERT-*") | | `--ignore-category string` | Categories to ignore (comma-separated, e.g., "Certificate,Library Import") | -| `--fail-on string` | Exit non-zero if findings at this severity or higher (info, low, medium, high, critical), or if any file could not be analyzed | +| `--fail-on string` | Exit non-zero if findings at this severity or higher (info, low, medium, high, critical), or if any file could not be analyzed. When neither this flag nor failOn in the config file is set, the scan exits 1 when any finding is CRITICAL | | `--baseline string` | Baseline JSON file - only report new findings | | `--config string` | Config file path (default: auto-detect .cryptoscan.yaml) | | `--no-config` | Do not read a config file; use only command-line options | @@ -114,8 +114,8 @@ Shows post-quantum readiness: the post-quantum algorithms found (ML-KEM, ML-DSA, SLH-DSA, FN-DSA, XMSS, LMS, library imports) with up to three file:line locations each, the hybrid constructions found, the quantum-vulnerable algorithms with a count each, then the migration score and -the QRAMM Dimension 1 practice levels. Quantum-safe findings are always -included. Exits 0 whatever it finds; non-zero only when the scan fails. To +the QRAMM Dimension 1 practice levels. The SHA-2 and AES findings that scan +leaves out by default are always included. Exits 0 whatever it finds; non-zero only when the scan fails. To fail a build on findings, use cryptoscan gate. Flags: diff --git a/docs/how-it-works.md b/docs/how-it-works.md index af72e10..d765a58 100644 --- a/docs/how-it-works.md +++ b/docs/how-it-works.md @@ -40,7 +40,7 @@ Scanning a repository that ships cryptographic reference data (a database of packages and the algorithms they use, for example) will report a finding per row. Use `--exclude` to skip those paths. -Library imports and quantum-safe algorithms are also left out of the default report. `--include-imports`, `--include-quantum-safe` and `--include-narrative` bring each group back, and `--verbose` brings back all three. +Library imports, SHA-2 findings, and AES findings at INFO severity other than AES-GCM and AES-ECB are also left out of the default report; the report classes SHA-2 and AES as PARTIAL, weakened but not broken by a quantum computer. `--include-imports`, `--include-quantum-safe` and `--include-narrative` bring each group back, and `--verbose` brings back all three. ## Detection coverage diff --git a/internal/cli/help_stream_test.go b/internal/cli/help_stream_test.go new file mode 100644 index 0000000..5d923fd --- /dev/null +++ b/internal/cli/help_stream_test.go @@ -0,0 +1,189 @@ +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) +// SPDX-License-Identifier: Apache-2.0 + +package cli + +import ( + "bytes" + "encoding/json" + "fmt" + "path/filepath" + "regexp" + "strings" + "testing" + "time" + + "github.com/csnp/cryptoscan/pkg/scanner" +) + +// cryptoscan --help opens by saying what the tool does, in plain words. +func TestRootHelpOpensWithAPlainDescription(t *testing.T) { + paragraphs := strings.Split(rootCmd.Long, "\n\n") + if len(paragraphs) < 2 { + t.Fatalf("root help has %d paragraphs, want a title and a description", len(paragraphs)) + } + opening := paragraphs[1] + if !strings.HasPrefix(opening, "CryptoScan finds the cryptographic algorithms") { + t.Errorf("root help opens with %q", firstLine(opening)) + } + for _, word := range []string{"powerful", "enterprise-grade", "empower", "cutting-edge", "world-class"} { + if strings.Contains(strings.ToLower(rootCmd.Long), word) { + t.Errorf("root help contains %q", word) + } + } +} + +// The scan help uses the report's words for SHA-2 and AES: the report classes +// them PARTIAL, not quantum-safe. A default scan leaves a SHA-256 call and an +// AES cipher out, and --include-quantum-safe reports both as PARTIAL. +func TestScanHelpUsesTheReportWordsForSHA2AndAES(t *testing.T) { + if !strings.Contains(scanCmd.Long, "The report classes SHA-2 and AES as PARTIAL") { + t.Error("scan --help does not say that the report classes SHA-2 and AES as PARTIAL") + } + if strings.Contains(scanCmd.Long, "Quantum-safe algorithms, including AES") { + t.Error("scan --help still calls AES and SHA-2 quantum-safe") + } + usage := scanCmd.Flags().Lookup("include-quantum-safe").Usage + if !strings.Contains(usage, "SHA-2 and AES") || !strings.Contains(usage, "PARTIAL") { + t.Errorf("--include-quantum-safe help reads %q", usage) + } + + dir := t.TempDir() + writeFixtureFile(t, filepath.Join(dir, "digest.py"), "def digest(data):\n return hashlib.sha256(data).hexdigest()\n\n"+ + "def seal(key):\n return AES.new(key, AES.MODE_CBC)\n") + partial := func(args ...string) map[string][]scanner.Finding { + out := runScanWith(t, dir, append([]string{"--stream=false", "--format", "json"}, args...)...) + var report scanner.Results + if err := json.Unmarshal([]byte(out.stdout), &report); err != nil { + t.Fatalf("scan output is not JSON: %v\n%s", err, out.stdout) + } + found := map[string][]scanner.Finding{} + for _, f := range report.Findings { + if f.Algorithm == "SHA-2" || f.Algorithm == "AES" { + found[f.Algorithm] = append(found[f.Algorithm], f) + } + } + return found + } + if got := partial(); len(got) != 0 { + t.Errorf("a default scan reported %v, which the help says are left out", got) + } + got := partial("--include-quantum-safe") + for _, algo := range []string{"SHA-2", "AES"} { + if len(got[algo]) == 0 { + t.Errorf("--include-quantum-safe did not report the %s line", algo) + } + for _, f := range got[algo] { + if f.Quantum != scanner.QuantumPartial { + t.Errorf("%s finding at line %d is classed %s, and the help says PARTIAL", algo, f.Line, f.Quantum) + } + } + } +} + +// The --fail-on help names what a scan does without it: a CRITICAL finding +// exits 1. +func TestFailOnHelpNamesTheCriticalDefault(t *testing.T) { + usage := scanCmd.Flags().Lookup("fail-on").Usage + if !strings.Contains(usage, "exits 1 when any finding is CRITICAL") { + t.Errorf("--fail-on help reads %q, which does not name the default", usage) + } + if !strings.Contains(scanCmd.Long, "the scan exits 1 when any finding is CRITICAL") { + t.Error("the CI/CD section of scan --help does not name the --fail-on default") + } + critical := &scanner.Results{Findings: []scanner.Finding{{Severity: scanner.SeverityCritical}}} + critical.Summary = calculateSummary(critical.Findings) + if code := exitCode(critical, "", true); code != 1 { + t.Errorf("without --fail-on a CRITICAL finding exits %d, and the help says 1", code) + } +} + +// streamedMatch is one numbered line of the list printed while the scan runs. +var streamedMatch = regexp.MustCompile(`(?m)^ \[\w+\] #\d+`) + +// The list printed while the scan runs stops after maxStreamedMatches lines +// and says that the report lists the rest. The report still counts every +// finding. +func TestStreamListsAtMostTheFirstMatches(t *testing.T) { + const lines = maxStreamedMatches + 30 + var body strings.Builder + for i := 0; i < lines; i++ { + fmt.Fprintf(&body, "h%d = hashlib.md5(data)\n", i) + } + dir := t.TempDir() + writeFixtureFile(t, filepath.Join(dir, "digests.py"), body.String()) + + out := runScanWith(t, dir, "--stream").stdout + + if got := len(streamedMatch.FindAllString(out, -1)); got != maxStreamedMatches { + t.Errorf("the stream listed %d matches, want %d", got, maxStreamedMatches) + } + notice := fmt.Sprintf(" Matches after the first %d are not listed while the scan runs; the report lists every finding.\n", maxStreamedMatches) + if n := strings.Count(out, notice); n != 1 { + t.Errorf("the stream printed the notice %d times, want once:\n%s", n, firstLines(out, maxStreamedMatches+8)) + } + footer := fmt.Sprintf("The scan found %d matches; the list above shows the first %d.", lines, maxStreamedMatches) + if !strings.Contains(out, footer) { + t.Errorf("the stream footer does not say %q", footer) + } + if want := fmt.Sprintf("Scan complete: %d findings", lines); !strings.Contains(out, want) { + t.Errorf("the stream footer does not say %q", want) + } + if want := fmt.Sprintf("Total Findings: %d ", lines); !strings.Contains(out, want) { + t.Errorf("the report does not say %q", want) + } +} + +// The footer of a capped stream says how many matches were found and how many +// were listed, and still accounts for the repeats. +func TestPrintScanningFooterNamesTheCap(t *testing.T) { + out := captureStdout(func() { + printScanningFooter(streamTally{listed: 250, hidden: 50, unique: 240, reported: 240, files: 3}, time.Millisecond, false) + }) + for _, want := range []string{ + "Scan complete: 240 findings in 3 files", + "The scan found 250 matches; the list above shows the first 200.", + "10 repeat a match of the same kind on the same line and are counted with it.", + } { + if !strings.Contains(out, want) { + t.Errorf("footer should contain %q, got:\n%s", want, out) + } + } + if strings.Contains(out, "The list above has") { + t.Errorf("footer of a capped stream says how many matches the list has:\n%s", out) + } +} + +// JSON, CSV, SARIF and CBOM output names the node_modules directories the scan +// did not enter on stderr; the text report lists them itself. +func TestSkippedDependencyNoticeOnStderr(t *testing.T) { + var buf bytes.Buffer + writeSkippedDependencyNotice(&buf, "json", []string{"node_modules", "web/node_modules"}) + want := "cryptoscan: 2 dependency directories were not scanned, so the packages installed there are not in this report:\n" + + " node_modules\n web/node_modules\n" + if buf.String() != want { + t.Errorf("notice reads\n%s\nwant\n%s", buf.String(), want) + } + for _, tc := range []struct { + format string + dirs []string + }{ + {"text", []string{"node_modules"}}, + {"sarif", nil}, + } { + buf.Reset() + writeSkippedDependencyNotice(&buf, tc.format, tc.dirs) + if buf.Len() != 0 { + t.Errorf("format %s with %d directories printed %q", tc.format, len(tc.dirs), buf.String()) + } + } +} + +// firstLines returns the first n lines of s. +func firstLines(s string, n int) string { + lines := strings.SplitN(s, "\n", n+1) + if len(lines) > n { + lines = lines[:n] + } + return strings.Join(lines, "\n") +} diff --git a/internal/cli/json_shape_test.go b/internal/cli/json_shape_test.go new file mode 100644 index 0000000..36f0098 --- /dev/null +++ b/internal/cli/json_shape_test.go @@ -0,0 +1,104 @@ +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) +// SPDX-License-Identifier: Apache-2.0 + +package cli + +import ( + "encoding/json" + "os" + "path/filepath" + "regexp" + "strconv" + "testing" + + "github.com/csnp/cryptoscan/pkg/scanner" +) + +// A JSON report names each finding's severity the way summary.bySeverity keys +// on it, and every finding has the algorithm and keySize keys, null when the +// pattern gives no value. +func TestJSONFindingsShareTheSummarySeverityNamesAndKeys(t *testing.T) { + dir := t.TempDir() + writeReviewedTree(t, dir, reviewedApp) + writeFixtureFile(t, filepath.Join(dir, "src", "keys.py"), "key_size = 2048\n") + + out := runScanWith(t, dir, "--stream=false", "--format", "json") + var report struct { + Findings []map[string]any `json:"findings"` + Summary struct { + BySeverity map[string]int `json:"bySeverity"` + } `json:"summary"` + } + if err := json.Unmarshal([]byte(out.stdout), &report); err != nil { + t.Fatalf("scan output is not JSON: %v\n%s", err, out.stdout) + } + if len(report.Findings) == 0 { + t.Fatal("the fixture produced no findings") + } + + counted := map[string]int{} + withoutAlgorithm := 0 + for _, f := range report.Findings { + name, ok := f["severity"].(string) + if !ok { + t.Errorf("finding %v has severity %#v, want a name", f["id"], f["severity"]) + continue + } + counted[name]++ + for _, key := range []string{"algorithm", "keySize"} { + if _, ok := f[key]; !ok { + t.Errorf("finding %v has no %s key", f["id"], key) + } + } + if f["algorithm"] == nil { + withoutAlgorithm++ + } + } + for name, n := range counted { + if report.Summary.BySeverity[name] != n { + t.Errorf("%d findings have severity %q, summary.bySeverity says %d", n, name, report.Summary.BySeverity[name]) + } + } + if withoutAlgorithm == 0 { + t.Error("no finding in the fixture names no algorithm, so the null case is not checked") + } +} + +// A baseline written by an earlier release carries each severity as a number. +// It still loads, and its findings still match the scan. +func TestBaselineWithNumericSeveritiesStillLoads(t *testing.T) { + a := t.TempDir() + writeReviewedTree(t, a, reviewedApp) + baseline := writeBaseline(t, a) + + data, err := os.ReadFile(baseline) + if err != nil { + t.Fatal(err) + } + named := regexp.MustCompile(`"severity":\s*"([A-Z]+)"`) + if len(named.FindAll(data, -1)) == 0 { + t.Fatalf("the baseline has no severity names to turn into numbers:\n%s", data) + } + numbers := named.ReplaceAllFunc(data, func(m []byte) []byte { + name := string(named.FindSubmatch(m)[1]) + for level := scanner.SeverityInfo; level <= scanner.SeverityCritical; level++ { + if level.String() == name { + return []byte(`"severity": ` + strconv.Itoa(int(level))) + } + } + t.Fatalf("unknown severity %q in the baseline", name) + return m + }) + if named.Match(numbers) || !regexp.MustCompile(`"severity": [0-4]`).Match(numbers) { + t.Fatalf("the baseline still names its severities:\n%s", numbers) + } + if err := os.WriteFile(baseline, numbers, 0o644); err != nil { + t.Fatal(err) + } + + b := t.TempDir() + writeReviewedTree(t, b, reviewedApp) + if got := scanTree(t, b, baseline, filepath.Join(t.TempDir(), "out.json")); len(got) != 0 { + t.Errorf("a baseline with numeric severities left %d findings new:\n %s", len(got), describe(got)) + } +} diff --git a/internal/cli/pqc.go b/internal/cli/pqc.go index 0e25471..4f75337 100644 --- a/internal/cli/pqc.go +++ b/internal/cli/pqc.go @@ -32,9 +32,9 @@ The view lists, in this order: - The migration readiness score and the QRAMM Dimension 1 practice levels, as the scan report prints them -pqc always includes the quantum-safe findings, such as SHA-256 and AES, that -scan withholds unless you pass --include-quantum-safe. Its score counts them, -so it can differ from the score a default scan prints. +pqc always includes the SHA-2 and AES findings, classed PARTIAL, that scan +leaves out unless you pass --include-quantum-safe. Its score counts them, so +it can differ from the score a default scan prints. A readiness view is information, not a verdict, so pqc exits 0 whatever it finds. It exits non-zero only when the scan fails. A baseline or minimum diff --git a/internal/cli/root.go b/internal/cli/root.go index 8415051..52765f2 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -24,9 +24,9 @@ var rootCmd = &cobra.Command{ Version: version.Get(), Long: `CryptoScan - QRAMM Cryptographic Discovery Scanner -A powerful tool for discovering cryptographic algorithms, key sizes, and -quantum-vulnerable patterns in your codebase. Part of the QRAMM (Quantum -Readiness Assurance Maturity Model) toolkit by CSNP. +CryptoScan finds the cryptographic algorithms, key sizes and quantum-vulnerable +patterns in source code and configuration files. It is part of the QRAMM +(Quantum Readiness Assurance Maturity Model) toolkit by CSNP. CryptoScan helps organizations prepare for post-quantum cryptography by: - Discovering hardcoded cryptographic algorithms (RSA, ECC, AES, etc.) diff --git a/internal/cli/scan.go b/internal/cli/scan.go index dcdd778..38f2ff0 100644 --- a/internal/cli/scan.go +++ b/internal/cli/scan.go @@ -12,6 +12,7 @@ import ( "os/exec" "path/filepath" "slices" + "strconv" "strings" "sync" "time" @@ -79,9 +80,12 @@ The scanner detects: - TLS/SSL settings - Crypto library imports -Quantum-safe algorithms, including AES and the SHA-2 family, are detected but -withheld from the default report because they need no migration. Pass ---include-quantum-safe to see them. +The report classes SHA-2 and AES as PARTIAL: a quantum computer weakens them +but does not break them, so they need no post-quantum migration. SHA-2 +findings, and AES findings at INFO severity other than AES-GCM and AES-ECB, +are left out of the default report; pass --include-quantum-safe to list +them. Post-quantum algorithms such as ML-KEM are classed SAFE and are +reported without that flag. Not detected: DSA. Standalone DSA key generation is not matched by any pattern, so a codebase using it will not be told that it is quantum @@ -99,7 +103,8 @@ CI/CD Integration: --ignore Suppress specific pattern IDs (e.g., "RSA-001,CERT-*") --ignore-category Suppress entire categories (e.g., "Certificate,Library Import") --fail-on Exit non-zero if findings at this severity or higher, - or if any file could not be analyzed + or if any file could not be analyzed. Without it, + the scan exits 1 when any finding is CRITICAL --baseline Only report new findings compared to baseline JSON --config Path to .cryptoscan.yaml config file --no-config Do not read a config file; use only command-line options @@ -137,16 +142,16 @@ func init() { scanCmd.Flags().BoolVar(&jsonPretty, "pretty", false, "Pretty print JSON output") scanCmd.Flags().StringVarP(&groupBy, "group-by", "g", "", "Group the text report by: "+strings.Join(groupByNames, ", ")) scanCmd.Flags().IntVarP(&contextLines, "context", "c", 3, "Lines of source to show before and after each finding") - scanCmd.Flags().BoolVar(&streamFindings, "stream", true, "Show findings as they are discovered") + scanCmd.Flags().BoolVar(&streamFindings, "stream", true, "Show findings as they are discovered, up to the first "+strconv.Itoa(maxStreamedMatches)+" matches") scanCmd.Flags().BoolVar(&includeImports, "include-imports", false, "Include library import findings (normally suppressed as low-value)") - scanCmd.Flags().BoolVar(&includeQuantumSafe, "include-quantum-safe", false, "Include quantum-safe algorithm findings (SHA-256, AES-256)") + scanCmd.Flags().BoolVar(&includeQuantumSafe, "include-quantum-safe", false, "Include the SHA-2 and AES findings left out by default, which the report classes as PARTIAL") scanCmd.Flags().BoolVar(&includeNarrative, "include-narrative", false, "Include algorithms named in prose, log messages, documentation and config keys") - scanCmd.Flags().BoolVarP(&verbose, "verbose", "v", false, "Show all findings including imports, quantum-safe algorithms and narrative mentions") + scanCmd.Flags().BoolVarP(&verbose, "verbose", "v", false, "Show all findings including imports, the SHA-2 and AES findings left out by default, and narrative mentions") // CI/CD flexibility flags scanCmd.Flags().StringVar(&ignorePatterns, "ignore", "", "Pattern IDs to ignore (comma-separated, e.g., \"RSA-001,CERT-*\")") scanCmd.Flags().StringVar(&ignoreCategories, "ignore-category", "", "Categories to ignore (comma-separated, e.g., \"Certificate,Library Import\")") - scanCmd.Flags().StringVar(&failOn, "fail-on", "", "Exit non-zero if findings at this severity or higher ("+strings.Join(severityNames, ", ")+"), or if any file could not be analyzed") + scanCmd.Flags().StringVar(&failOn, "fail-on", "", "Exit non-zero if findings at this severity or higher ("+strings.Join(severityNames, ", ")+"), or if any file could not be analyzed. When neither this flag nor failOn in the config file is set, the scan exits 1 when any finding is CRITICAL") scanCmd.Flags().StringVar(&baselineFile, "baseline", "", "Baseline JSON file - only report new findings") scanCmd.Flags().StringVar(&configFile, "config", "", "Config file path (default: auto-detect .cryptoscan.yaml)") scanCmd.Flags().BoolVar(&noConfig, "no-config", false, "Do not read a config file; use only command-line options") @@ -358,9 +363,18 @@ func scanPipeline(cmd *cobra.Command, target string, opts pipelineOptions) (*sca outputMu.Lock() listedCount++ num := listedCount - // Clear the progress line before printing finding - clearProgressLine() - printStreamFinding(f, num, !noColor) + // Print the first matches, then one line saying the rest are in + // the report. Listing every match put the summary below all of + // them, however many a large tree gave. The progress line is + // cleared before either is printed. + switch { + case num <= maxStreamedMatches: + clearProgressLine() + printStreamFinding(f, num, !noColor) + case num == maxStreamedMatches+1: + clearProgressLine() + printStreamCapNotice(!noColor) + } outputMu.Unlock() } } @@ -454,6 +468,7 @@ func scanPipeline(cmd *cobra.Command, target string, opts pipelineOptions) (*sca if streamFindings && textReport { printScanningFooter(streamTally{ listed: listedCount, + hidden: max(listedCount-maxStreamedMatches, 0), unique: uniqueCount, reported: len(results.Findings), files: results.FilesScanned, @@ -510,6 +525,7 @@ func scanPipeline(cmd *cobra.Command, target string, opts pipelineOptions) (*sca writeUnanalyzedNotice(os.Stderr, outputFormat, results.UnanalyzedFiles) writeSkippedNotice(os.Stderr, outputFormat, results.SkippedFiles) + writeSkippedDependencyNotice(os.Stderr, outputFormat, results.SkippedDependencyDirs) return results, effectiveFailOn, nil } @@ -591,6 +607,27 @@ func writeSkippedNotice(w io.Writer, format string, files []scanner.SkippedFile) } } +// writeSkippedDependencyNotice names the node_modules directories the scan +// did not enter, for the same formats as writeUnanalyzedNotice. The text +// report lists them itself. +func writeSkippedDependencyNotice(w io.Writer, format string, dirs []string) { + if format == "text" || len(dirs) == 0 { + return + } + head := fmt.Sprintf("%d dependency directories were", len(dirs)) + if len(dirs) == 1 { + head = "1 dependency directory was" + } + fmt.Fprintf(w, "cryptoscan: %s not scanned, so the packages installed there are not in this report:\n", head) + for i, d := range dirs { + if i == maxSkippedNamed { + fmt.Fprintf(w, " and %d more, listed under skippedDependencyDirs in --format json\n", len(dirs)-maxSkippedNamed) + break + } + fmt.Fprintf(w, " %s\n", d) + } +} + func printBanner() { const ( colorCyan = "\033[36m" @@ -691,7 +728,8 @@ func printScanningHeader(useColor bool) { // matches of the same category on the same line are merged into one finding, // and a baseline removes findings that were already known. type streamTally struct { - listed int // matches printed while scanning + listed int // matches found while scanning + hidden int // matches found after the first maxStreamedMatches, not printed unique int // findings left after deduplication reported int // findings the report counts as Total Findings files int // files scanned @@ -731,9 +769,15 @@ func printScanningFooter(tally streamTally, duration time.Duration, useColor boo // The listed matches are numbered, so when their count differs from the // report's, say where the difference went. var notes []string + if tally.hidden > 0 { + notes = append(notes, fmt.Sprintf("The scan found %d matches; the list above shows the first %d.", + tally.listed, tally.listed-tally.hidden)) + } if tally.listed != tally.reported { - notes = append(notes, fmt.Sprintf("The list above has %d %s.", - tally.listed, plural(tally.listed, "match", "matches"))) + if tally.hidden == 0 { + notes = append(notes, fmt.Sprintf("The list above has %d %s.", + tally.listed, plural(tally.listed, "match", "matches"))) + } if merged := tally.listed - tally.unique; merged > 0 { notes = append(notes, fmt.Sprintf("%d %s a match of the same kind on the same line and %s counted with it.", merged, plural(merged, "repeats", "repeat"), plural(merged, "is", "are"))) @@ -760,6 +804,20 @@ func plural(n int, one, many string) string { return many } +// maxStreamedMatches is the number of matches the text report lists while +// the scan runs. The report after the stream lists every finding. +const maxStreamedMatches = 200 + +// printStreamCapNotice prints the line that ends the streamed list once +// maxStreamedMatches matches have been printed. +func printStreamCapNotice(useColor bool) { + line := fmt.Sprintf(" Matches after the first %d are not listed while the scan runs; the report lists every finding.", maxStreamedMatches) + if useColor { + line = "\033[2m" + line + "\033[0m" + } + fmt.Println(line) +} + // printStreamFinding prints a compact finding line during scanning func printStreamFinding(f scanner.Finding, num int, useColor bool) { const ( diff --git a/pkg/reporter/cbom.go b/pkg/reporter/cbom.go index df85681..3bc96ba 100644 --- a/pkg/reporter/cbom.go +++ b/pkg/reporter/cbom.go @@ -9,6 +9,7 @@ import ( "fmt" "os" "path/filepath" + "strconv" "strings" "time" @@ -143,6 +144,7 @@ type cbomDependency struct { // TestPrimitivesAreInCycloneDXEnum. const ( primitiveCombiner = "combiner" + primitiveAE = "ae" ) // categoryToAssetType maps a pattern category onto a CycloneDX asset type. @@ -153,12 +155,16 @@ const ( // was reported as an algorithm. A detected private key was emitted as // assetType "algorithm" named "Private Key Header", which is what the reporter // of issue #1 meant by incomplete key material representation. +// +// The TLS patterns use the categories "TLS/SSL" and "Deprecated Protocol", +// which are protocols in CycloneDX; matched only as "tls" and "protocol", a +// TLS finding was written as an algorithm with primitive "other". func categoryToAssetType(category string) string { switch strings.ToLower(strings.TrimSpace(category)) { case "asymmetric", "key-exchange", "symmetric", "hash", "mac", "kdf", "post-quantum", "signature", "random": return "algorithm" - case "tls", "protocol", "ssh", "ipsec": + case "tls", "tls/ssl", "deprecated protocol", "protocol", "ssh", "ipsec": return "protocol" case "certificate": return "certificate" @@ -171,6 +177,61 @@ func categoryToAssetType(category string) string { } } +// nonAssetCategories are the categories, in lower case, of findings that +// describe something other than a cryptographic asset: an import of a crypto +// library, or a setting such as a hard-coded key size. CycloneDX has no asset +// type for either, and written as algorithm components they carried a pattern +// name, such as "Post-Quantum Crypto Library", and no algorithm properties. +var nonAssetCategories = map[string]bool{ + "configuration": true, + "library import": true, + "pqc library import": true, +} + +// isCryptoAsset reports whether a finding becomes a CBOM component. A finding +// that names an algorithm always does. The other report formats list every +// finding whatever this returns. +func isCryptoAsset(f scanner.Finding) bool { + return f.Algorithm != "" || !nonAssetCategories[strings.ToLower(strings.TrimSpace(f.Category))] +} + +// protocolType maps a protocol finding's category onto the CycloneDX +// protocolProperties.type enum. The TLS patterns also match SSL versions, +// which CycloneDX counts as TLS. +func protocolType(category string) string { + switch strings.ToLower(strings.TrimSpace(category)) { + case "ssh": + return "ssh" + case "ipsec": + return "ipsec" + default: + return "tls" + } +} + +// componentName returns the component a finding belongs to and the key that +// merges findings into it. An algorithm with a key size is a component of its +// own, such as RSA-1024 beside RSA-2048, so that each carries the security +// level of its size; merged under one name, every RSA finding took the level +// of whichever was found first. A protocol is keyed on its version as well, +// so TLS 1.2 and TLS 1.3 each carry their own version. +func componentName(f scanner.Finding, assetType string) (name, key string) { + name = f.Algorithm + if name == "" { + name = f.Type + } + switch assetType { + case "algorithm": + if size := strconv.Itoa(f.KeySize); f.KeySize > 0 && !strings.HasSuffix(name, size) { + name += "-" + size + } + return name, name + case "protocol": + return name, strings.TrimSpace(name + " " + extractTLSVersion(f.Match)) + } + return name, name +} + // relativeLocation expresses a finding's file relative to the scan root. // // A CBOM is an artifact that gets committed, attached to releases and shared @@ -239,6 +300,12 @@ func relatedCryptoMaterialType(patternID, name string) string { func algorithmToPrimitive(algo string) string { algoUpper := strings.ToUpper(algo) switch { + // Authenticated encryption, which CycloneDX calls "ae". Checked before + // the MACs because ChaCha20-Poly1305 carries the name of its MAC, which + // made it a "mac"; AES-GCM matched no case and was written as "other". + case strings.Contains(algoUpper, "GCM"), strings.Contains(algoUpper, "CCM"), + strings.Contains(algoUpper, "CHACHA") && strings.Contains(algoUpper, "POLY1305"): + return primitiveAE // MACs - check before hashes because HMAC-SHA256 contains SHA case strings.Contains(algoUpper, "HMAC"), strings.Contains(algoUpper, "KMAC"), strings.Contains(algoUpper, "CMAC"), strings.Contains(algoUpper, "GMAC"), @@ -378,12 +445,14 @@ func (r *CBOMReporter) Generate(results *scanner.Results) (string, error) { hybridComponents := make(map[string][]string) // Track hybrid -> component dependencies for _, f := range results.Findings { - // Create unique component key - compKey := f.Algorithm - if compKey == "" { - compKey = f.Type + if !isCryptoAsset(f) { + continue } + // Create unique component key + assetType := categoryToAssetType(f.Category) + name, compKey := componentName(f, assetType) + // Generate a stable BOM reference bomRef := fmt.Sprintf("crypto-%s-%d", sanitizeBOMRef(compKey), len(componentMap)) @@ -391,10 +460,10 @@ func (r *CBOMReporter) Generate(results *scanner.Results) (string, error) { comp := cbomComponent{ Type: "cryptographic-asset", BOMRef: bomRef, - Name: compKey, + Name: name, Description: f.Description, CryptoProperties: &cbomCryptoProperties{ - AssetType: categoryToAssetType(f.Category), + AssetType: assetType, }, Evidence: &cbomEvidence{ Occurrences: []cbomOccurrence{ @@ -417,7 +486,7 @@ func (r *CBOMReporter) Generate(results *scanner.Results) (string, error) { // being flattened into a component named after the PEM header. if comp.CryptoProperties.AssetType == "related-crypto-material" { comp.CryptoProperties.RelatedCryptoMaterialProperties = &cbomRelatedCryptoMaterial{ - Type: relatedCryptoMaterialType(f.ID, compKey), + Type: relatedCryptoMaterialType(f.ID, name), Size: f.KeySize, } } @@ -462,9 +531,9 @@ func (r *CBOMReporter) Generate(results *scanner.Results) (string, error) { } // Add protocol properties for TLS findings - if f.Category == "tls" || f.Category == "protocol" { + if assetType == "protocol" { comp.CryptoProperties.ProtocolProperties = &cbomProtocol{ - Type: "tls", + Type: protocolType(f.Category), } // Extract TLS version if present if version := extractTLSVersion(f.Match); version != "" { @@ -628,7 +697,9 @@ func extractHybridComponents(algo string) (classical string, pqc string) { // extractTLSVersion extracts the TLS version from a match string func extractTLSVersion(match string) string { - matchUpper := strings.ToUpper(match) + // The TLS patterns also match TLS_1_2, TLSv1_2 and TLS-1.2, so a version + // written with "_" or "-" is read as one written with ".". + matchUpper := strings.NewReplacer("_", ".", "-", ".").Replace(strings.ToUpper(match)) switch { case strings.Contains(matchUpper, "1.3") || strings.Contains(matchUpper, "TLS13"): return "1.3" diff --git a/pkg/reporter/cbom_assets_test.go b/pkg/reporter/cbom_assets_test.go new file mode 100644 index 0000000..e2038ba --- /dev/null +++ b/pkg/reporter/cbom_assets_test.go @@ -0,0 +1,205 @@ +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) +// SPDX-License-Identifier: Apache-2.0 + +package reporter + +import ( + "encoding/json" + "testing" + + "github.com/csnp/cryptoscan/pkg/scanner" +) + +// cbomOf generates the CBOM for results and reads it back into the +// reporter's own types. +func cbomOf(t *testing.T, results *scanner.Results) cbomReport { + t.Helper() + out, err := NewCBOMReporter().Generate(results) + if err != nil { + t.Fatalf("generate CBOM: %v", err) + } + var bom cbomReport + if err := json.Unmarshal([]byte(out), &bom); err != nil { + t.Fatalf("CBOM is not valid JSON: %v", err) + } + return bom +} + +// componentsNamed returns the components called name. +func componentsNamed(bom cbomReport, name string) []cbomComponent { + var out []cbomComponent + for _, c := range bom.Components { + if c.Name == name { + out = append(out, c) + } + } + return out +} + +// AES-GCM and ChaCha20-Poly1305 are authenticated encryption, which CycloneDX +// calls "ae". ChaCha20-Poly1305 was written as a MAC, after the Poly1305 in its +// name, and AES-GCM as "other". Poly1305 on its own is still a MAC. +func TestCBOMWritesAuthenticatedEncryptionAsAE(t *testing.T) { + for algo, want := range map[string]string{ + "AES-GCM": "ae", + "AES-256-GCM": "ae", + "AES-CCM": "ae", + "ChaCha20-Poly1305": "ae", + "Poly1305": "mac", + "ChaCha20": "stream-cipher", + "AES": "block-cipher", + } { + if got := algorithmToPrimitive(algo); got != want { + t.Errorf("algorithmToPrimitive(%q) = %q, want %q", algo, got, want) + } + } + + bom := cbomOf(t, scanCryptoSamples(t)) + for _, name := range []string{"AES-GCM", "ChaCha20-Poly1305"} { + comps := componentsNamed(bom, name) + if len(comps) == 0 { + t.Fatalf("the CBOM of crypto-samples has no %s component", name) + } + for _, c := range comps { + p := c.CryptoProperties.AlgorithmProperties + if p == nil || p.Primitive != "ae" { + t.Errorf("%s is written with algorithm properties %+v, want primitive ae", name, p) + } + } + } +} + +// A TLS finding is a protocol asset with protocol properties, not an +// algorithm with primitive "other". +func TestCBOMWritesTLSAsAProtocol(t *testing.T) { + for _, category := range []string{"TLS/SSL", "Deprecated Protocol"} { + if got := categoryToAssetType(category); got != "protocol" { + t.Errorf("categoryToAssetType(%q) = %q, want protocol", category, got) + } + } + + bom := cbomOf(t, scanCryptoSamples(t)) + comps := componentsNamed(bom, "TLS") + if len(comps) == 0 { + t.Fatal("the CBOM of crypto-samples has no TLS component") + } + for _, c := range comps { + cp := c.CryptoProperties + if cp.AssetType != "protocol" { + t.Errorf("TLS is written with assetType %q, want protocol", cp.AssetType) + } + if cp.AlgorithmProperties != nil { + t.Errorf("TLS carries algorithm properties %+v", cp.AlgorithmProperties) + } + if cp.ProtocolProperties == nil || cp.ProtocolProperties.Type != "tls" || cp.ProtocolProperties.Version != "1.3" { + t.Errorf("TLS is written with protocol properties %+v, want type tls and version 1.3 (the samples name TLSv1.3)", cp.ProtocolProperties) + } + } +} + +// TLS 1.2 and TLS 1.3 are separate components, each with its own version, and +// a version written with "_" is read. +func TestCBOMKeepsOneProtocolComponentPerVersion(t *testing.T) { + for match, want := range map[string]string{ + "TLSv1.3": "1.3", + "TLSv1_2": "1.2", + "TLS_1_3": "1.3", + "TLS-1.2": "1.2", + "TLS13": "1.3", + "TLS": "", + } { + if got := extractTLSVersion(match); got != want { + t.Errorf("extractTLSVersion(%q) = %q, want %q", match, got, want) + } + } + + bom := cbomOf(t, resultsWith("/tmp/project", + scanner.Finding{ID: "TLS-002", Algorithm: "TLS", Category: "TLS/SSL", File: "/tmp/project/a.py", Line: 1, Match: "TLSv1_2"}, + scanner.Finding{ID: "TLS-002", Algorithm: "TLS", Category: "TLS/SSL", File: "/tmp/project/b.py", Line: 2, Match: "TLSv1.3"}, + scanner.Finding{ID: "TLS-002", Algorithm: "TLS", Category: "TLS/SSL", File: "/tmp/project/c.py", Line: 3, Match: "TLS_1_3"}, + )) + versions := map[string]int{} + for _, c := range componentsNamed(bom, "TLS") { + versions[c.CryptoProperties.ProtocolProperties.Version] = len(c.Evidence.Occurrences) + } + if len(versions) != 2 || versions["1.2"] != 1 || versions["1.3"] != 2 { + t.Errorf("TLS components by version and occurrence count: %v, want map[1.2:1 1.3:2]", versions) + } +} + +// A hard-coded key size and a post-quantum library import are not +// cryptographic assets, so they are not written as algorithm components with +// no properties. A finding that names an algorithm is always written. +func TestCBOMLeavesOutFindingsThatAreNotAssets(t *testing.T) { + results := scanCryptoSamples(t) + present := map[string]bool{} + for _, f := range results.Findings { + present[f.Type] = true + } + notAssets := []string{"Hardcoded Key Size", "Post-Quantum Crypto Library"} + for _, name := range notAssets { + if !present[name] { + t.Fatalf("the scan of crypto-samples no longer finds %q, so this test checks nothing for it", name) + } + } + + bom := cbomOf(t, results) + for _, name := range notAssets { + if comps := componentsNamed(bom, name); len(comps) > 0 { + t.Errorf("the CBOM writes %d component(s) named %q", len(comps), name) + } + } + + if !isCryptoAsset(scanner.Finding{Category: "Configuration", Algorithm: "RSA"}) { + t.Error("a Configuration finding that names an algorithm is left out of the CBOM") + } + for _, category := range []string{"Configuration", "Library Import", "PQC Library Import"} { + if isCryptoAsset(scanner.Finding{Category: category}) { + t.Errorf("a %s finding that names no algorithm is written to the CBOM", category) + } + } + for _, category := range []string{"Weak Cipher", "Secret Detection", "Certificate", "dependency"} { + if !isCryptoAsset(scanner.Finding{Category: category}) { + t.Errorf("a %s finding is left out of the CBOM", category) + } + } +} + +// Each RSA key size is a component of its own with the security level of that +// size. One RSA component took the level of whichever finding came first, so +// a 1024-bit key gave every RSA occurrence a classical level of 80. +func TestCBOMKeepsOneComponentPerKeySize(t *testing.T) { + rsa := func(line, size int) scanner.Finding { + return scanner.Finding{ + ID: "RSA-001", Algorithm: "RSA", KeySize: size, Category: "Asymmetric Encryption", + File: "/tmp/project/keys.go", Line: line, Match: "rsa.GenerateKey", + } + } + bom := cbomOf(t, resultsWith("/tmp/project", rsa(1, 1024), rsa(2, 0), rsa(3, 2048), rsa(4, 1024), rsa(5, 0))) + + want := map[string]struct{ level, occurrences int }{ + "RSA-1024": {80, 2}, + "RSA-2048": {112, 1}, + "RSA": {0, 2}, + } + if len(bom.Components) != len(want) { + t.Errorf("%d components, want %d", len(bom.Components), len(want)) + } + for name, w := range want { + comps := componentsNamed(bom, name) + if len(comps) != 1 { + t.Errorf("%d components named %s, want 1", len(comps), name) + continue + } + c := comps[0] + if got := c.CryptoProperties.AlgorithmProperties.ClassicalSecurityLevel; got != w.level { + t.Errorf("%s classicalSecurityLevel = %d, want %d", name, got, w.level) + } + if got := len(c.Evidence.Occurrences); got != w.occurrences { + t.Errorf("%s has %d occurrences, want %d", name, got, w.occurrences) + } + if got := c.CryptoProperties.OID; got != "1.2.840.113549.1.1.1" { + t.Errorf("%s oid = %q, want the RSA OID", name, got) + } + } +} diff --git a/pkg/reporter/text.go b/pkg/reporter/text.go index d1fd9d2..721e4a5 100644 --- a/pkg/reporter/text.go +++ b/pkg/reporter/text.go @@ -182,6 +182,9 @@ func (r *TextReporter) Generate(results *scanner.Results) (string, error) { if len(results.SkippedFiles) > 0 { r.writeSkippedFiles(&b, results.SkippedFiles) } + if len(results.SkippedDependencyDirs) > 0 { + r.writeSkippedDependencyDirs(&b, results.SkippedDependencyDirs) + } if results.Config != nil { r.writeConfigSummary(&b, results) } @@ -424,7 +427,7 @@ func (r *TextReporter) generateGroupedByFile(results *scanner.Results, b *string // File header b.WriteString(r.color(colorBold+colorCyan, fmt.Sprintf(" ━━━ %s ", file))) - b.WriteString(r.color(colorYellow, fmt.Sprintf("(%d findings)", len(findings)))) + b.WriteString(r.color(colorYellow, "("+findingCount(len(findings))+")")) b.WriteString(r.color(colorBold+colorCyan, " ━━━\n")) b.WriteString("\n") @@ -525,6 +528,27 @@ func (r *TextReporter) writeSkippedFiles(b *strings.Builder, files []scanner.Ski } } +// writeSkippedDependencyDirs names the node_modules directories the scan did +// not enter. The packages installed there are not in the report, and a report +// that did not say so read as one that had covered them. +func (r *TextReporter) writeSkippedDependencyDirs(b *strings.Builder, dirs []string) { + head := fmt.Sprintf("%d dependency directories were", len(dirs)) + if len(dirs) == 1 { + head = "1 dependency directory was" + } + b.WriteString(r.color(colorYellow, fmt.Sprintf( + "\n %s not scanned, so the packages installed there are not in this report:\n", head))) + for i, d := range dirs { + if i == maxSkippedListed { + fmt.Fprintf(b, " and %d more, listed under skippedDependencyDirs in --format json\n", len(dirs)-maxSkippedListed) + break + } + fmt.Fprintf(b, " %s\n", d) + } + b.WriteString(" Dependency manifests such as package.json are read for known crypto libraries.\n") + b.WriteString(" To scan one installed package, pass its directory to cryptoscan scan.\n") +} + // writeConfigSummary names the config file the scan applied and what each of // its settings removed. The file may sit in a directory above the scan target, // where a reader of the report would not think to look. @@ -577,7 +601,10 @@ func (r *TextReporter) writeMigrationScore(b *strings.Builder, score *scanner.Mi break } truncFile := truncatePath(f.File, 40) - b.WriteString(r.color(colorCyan, "│") + fmt.Sprintf(" %d. %-42s (%d findings)", i+1, truncFile, f.TotalFindings) + r.color(colorCyan, " │\n")) + // "(1 finding)" is padded to the width "(1 findings)" had, so + // the border stays where it was. + count := fmt.Sprintf("%-*s", len(fmt.Sprintf("(%d findings)", f.TotalFindings)), "("+findingCount(f.TotalFindings)+")") + b.WriteString(r.color(colorCyan, "│") + fmt.Sprintf(" %d. %-42s %s", i+1, truncFile, count) + r.color(colorCyan, " │\n")) } } @@ -746,12 +773,18 @@ func (r *TextReporter) writeFooter(b *strings.Builder) { b.WriteString(" Part of the QRAMM Toolkit: https://qramm.org\n") b.WriteString(" Assess your organisation: https://qramm.org/quick-assessment.html\n") b.WriteString(" Developed by CSNP: https://csnp.org\n") - b.WriteString("\n") - b.WriteString(r.color(colorGreen, " CSNP Mission: ") + "Advancing cybersecurity through education,\n") - b.WriteString(" research, and open-source tools that empower organizations.\n") b.WriteString(r.color(colorBold, "═══════════════════════════════════════════════════════════════\n")) } +// findingCount returns "1 finding" for one finding and "N findings" for any +// other count. +func findingCount(n int) string { + if n == 1 { + return "1 finding" + } + return fmt.Sprintf("%d findings", n) +} + func min(a, b int) int { if a < b { return a diff --git a/pkg/reporter/text_wording_test.go b/pkg/reporter/text_wording_test.go new file mode 100644 index 0000000..300935c --- /dev/null +++ b/pkg/reporter/text_wording_test.go @@ -0,0 +1,121 @@ +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) +// SPDX-License-Identifier: Apache-2.0 + +package reporter + +import ( + "strings" + "testing" + "unicode/utf8" + + "github.com/csnp/cryptoscan/pkg/scanner" + "github.com/csnp/cryptoscan/pkg/types" +) + +// A file with one finding reads "1 finding", in the TOP PRIORITY FILES rows +// of the readiness box and in the file headers of --group-by file. The row +// keeps the width it had, so the box border does not move. +func TestTextReportSaysOneFinding(t *testing.T) { + results := createTestResults() + results.MigrationScore = &scanner.MigrationScore{ + Score: 10, Level: "HIGH", TotalCount: 3, + TopRiskFiles: []types.FileRiskScore{ + {File: "/test/crypto.go", TotalFindings: 2, VulnerableCount: 2, RiskScore: 20}, + {File: "/test/encrypt.go", TotalFindings: 1, VulnerableCount: 1, RiskScore: 10}, + }, + } + + text, err := NewTextReporter(false).Generate(results) + if err != nil { + t.Fatalf("generate text: %v", err) + } + two := lineWith(text, "1. /test/crypto.go") + one := lineWith(text, "2. /test/encrypt.go") + if !strings.Contains(two, "(2 findings)") { + t.Errorf("row for 2 findings reads %q", two) + } + if !strings.Contains(one, "(1 finding) ") || strings.Contains(one, "1 findings") { + t.Errorf("row for 1 finding reads %q, want (1 finding) followed by the padding", one) + } + if utf8.RuneCountInString(one) != utf8.RuneCountInString(two) { + t.Errorf("the row for 1 finding is %d columns wide and the row for 2 is %d:\n%s\n%s", + utf8.RuneCountInString(one), utf8.RuneCountInString(two), one, two) + } + + grouped := NewTextReporter(false) + grouped.SetGroupBy("file") + text, err = grouped.Generate(createTestResults()) + if err != nil { + t.Fatalf("generate grouped text: %v", err) + } + for _, file := range []string{"/test/crypto.go", "/test/encrypt.go"} { + header := lineWith(text, "━━━ "+file) + if !strings.Contains(header, "(1 finding)") { + t.Errorf("header for %s reads %q, want (1 finding)", file, header) + } + } + if strings.Contains(text, "1 findings") { + t.Error("the grouped report says 1 findings") + } +} + +// The footer names the tool and its links. It carries no mission statement. +func TestFooterCarriesNoMissionStatement(t *testing.T) { + text, err := NewTextReporter(false).Generate(createTestResults()) + if err != nil { + t.Fatalf("generate text: %v", err) + } + footer := text[strings.LastIndex(text, " QRAMM CryptoScan - "):] + for _, phrase := range []string{"Mission", "empower", "Advancing cybersecurity"} { + if strings.Contains(footer, phrase) { + t.Errorf("the footer contains %q:\n%s", phrase, footer) + } + } + for _, line := range []string{ + " Part of the QRAMM Toolkit: https://qramm.org\n", + " Developed by CSNP: https://csnp.org\n", + } { + if !strings.Contains(footer, line) { + t.Errorf("the footer lost %q:\n%s", strings.TrimSpace(line), footer) + } + } +} + +// The text report names the node_modules directories the scan did not enter, +// and says nothing about them when there were none. +func TestTextReportListsSkippedDependencyDirs(t *testing.T) { + results := createTestResults() + results.SkippedDependencyDirs = []string{"node_modules", "web/node_modules"} + text, err := NewTextReporter(false).Generate(results) + if err != nil { + t.Fatalf("generate text: %v", err) + } + for _, want := range []string{ + " 2 dependency directories were not scanned, so the packages installed there are not in this report:\n" + + " node_modules\n" + + " web/node_modules\n", + " To scan one installed package, pass its directory to cryptoscan scan.\n", + } { + if !strings.Contains(text, want) { + t.Errorf("text report does not contain %q", want) + } + } + + results.SkippedDependencyDirs = []string{"node_modules"} + text, err = NewTextReporter(false).Generate(results) + if err != nil { + t.Fatalf("generate text: %v", err) + } + if !strings.Contains(text, " 1 dependency directory was not scanned,") { + t.Error("text report does not use the singular for one directory") + } + + results.SkippedDependencyDirs = []string{} + text, err = NewTextReporter(false).Generate(results) + if err != nil { + t.Fatalf("generate text: %v", err) + } + if strings.Contains(text, "dependency director") { + t.Error("text report mentions dependency directories when none were skipped") + } +} diff --git a/pkg/scanner/scanner.go b/pkg/scanner/scanner.go index 600b684..b0f291f 100644 --- a/pkg/scanner/scanner.go +++ b/pkg/scanner/scanner.go @@ -194,6 +194,12 @@ type Results struct { // so they are named rather than dropped without a word. SkippedFiles []SkippedFile `json:"skippedFiles"` + // SkippedDependencyDirs lists the node_modules directories the scan did + // not enter, relative to the scan root and slash-separated. Installed + // packages are left out of every scan; this list says where they were, + // so that a report without their findings is not read as a full one. + SkippedDependencyDirs []string `json:"skippedDependencyDirs"` + // ScanRoot is the directory finding paths were walked from (a temporary // clone for a Git URL). It is used to recompute the migration score and // is not written to any report. @@ -331,6 +337,10 @@ type Scanner struct { // skipped holds files left out because they look minified or generated. skipped []SkippedFile + // dependencyDirs holds the node_modules directories the walk did not + // enter, relative to the scan root. + dependencyDirs []string + // configWithheld holds, per configuration-file setting, the matches that // setting kept out of a report that would otherwise have contained them. // configSkipped counts the paths ignore.files kept out of the scan. @@ -466,6 +476,9 @@ func (s *Scanner) Scan() (*Results, error) { sort.Slice(skipped, func(i, j int) bool { return skipped[i].File < skipped[j].File }) + dependencyDirs := make([]string, len(s.dependencyDirs)) + copy(dependencyDirs, s.dependencyDirs) + sort.Strings(dependencyDirs) // Build results results := &Results{ @@ -481,6 +494,8 @@ func (s *Scanner) Scan() (*Results, error) { Config: configReport, UnanalyzedFiles: unanalyzed, SkippedFiles: skipped, + + SkippedDependencyDirs: dependencyDirs, } results.Summary = s.calculateSummary() @@ -691,6 +706,14 @@ func (s *Scanner) shouldSkipDir(root, path, name string) error { ".terraform": true, } if !isRoot && skipDirs[name] { + // An installed dependency tree holds the code of every package the + // project uses, and the report names it so that its absence is not + // read as an absence of cryptography. + if name == "node_modules" { + s.mu.Lock() + s.dependencyDirs = append(s.dependencyDirs, filepath.ToSlash(relPath)) + s.mu.Unlock() + } return filepath.SkipDir } diff --git a/pkg/scanner/skipped_test.go b/pkg/scanner/skipped_test.go index 562b427..2f4e938 100644 --- a/pkg/scanner/skipped_test.go +++ b/pkg/scanner/skipped_test.go @@ -189,3 +189,43 @@ func TestSkippedFilesAreSortedAndAlwaysWritten(t *testing.T) { t.Errorf("expected an empty skippedFiles array in JSON, got %s", data) } } + +// A node_modules directory is never walked, and the results name each one the +// walk passed over, relative to the scan root. One inside another is not +// reached, so it is not listed. A scan with none lists an empty array. +func TestScanListsNodeModulesDirectoriesItDidNotEnter(t *testing.T) { + dir := t.TempDir() + for _, sub := range []string{ + filepath.Join("node_modules", "jsonwebtoken", "node_modules", "jws"), + filepath.Join("web", "node_modules", "left"), + "src", + } { + if err := os.MkdirAll(filepath.Join(dir, sub), 0o755); err != nil { + t.Fatal(err) + } + } + md5 := "const h = crypto.createHash('md5');\n" + writeTestFile(t, dir, filepath.Join("node_modules", "jsonwebtoken", "index.js"), md5) + writeTestFile(t, dir, filepath.Join("node_modules", "jsonwebtoken", "node_modules", "jws", "index.js"), md5) + writeTestFile(t, dir, filepath.Join("web", "node_modules", "left", "index.js"), md5) + writeTestFile(t, dir, filepath.Join("src", "app.js"), md5) + + results := scanWithConfig(t, Config{Target: dir, ExcludeGlobs: DefaultExcludeGlobs}) + want := []string{"node_modules", "web/node_modules"} + if !reflect.DeepEqual(results.SkippedDependencyDirs, want) { + t.Errorf("SkippedDependencyDirs = %q, want %q", results.SkippedDependencyDirs, want) + } + for _, f := range results.Findings { + if strings.Contains(filepath.ToSlash(f.File), "/node_modules/") { + t.Errorf("a finding from %s, which is inside a node_modules directory", f.File) + } + } + + data, err := json.Marshal(scanWithConfig(t, Config{Target: filepath.Join(dir, "src")})) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(data), `"skippedDependencyDirs":[]`) { + t.Errorf("expected an empty skippedDependencyDirs array in JSON, got %s", data) + } +} diff --git a/pkg/types/types.go b/pkg/types/types.go index 82908e2..27b2069 100644 --- a/pkg/types/types.go +++ b/pkg/types/types.go @@ -4,6 +4,12 @@ // Package types contains shared type definitions package types +import ( + "encoding/json" + "fmt" + "strings" +) + // Severity levels for findings type Severity int @@ -92,6 +98,40 @@ func (s Severity) String() string { } } +// MarshalJSON writes a severity by name, such as "HIGH", which is how +// summary.bySeverity, the text report, CSV and the patterns list name it. +func (s Severity) MarshalJSON() ([]byte, error) { + return json.Marshal(s.String()) +} + +// UnmarshalJSON reads a severity by name, in any case. It also reads the +// number 0 (INFO) to 4 (CRITICAL) that JSON reports from earlier releases +// carry, so that such a report still loads as a baseline. +func (s *Severity) UnmarshalJSON(data []byte) error { + if string(data) == "null" { + return nil + } + var name string + if err := json.Unmarshal(data, &name); err == nil { + for level := SeverityInfo; level <= SeverityCritical; level++ { + if strings.EqualFold(name, level.String()) { + *s = level + return nil + } + } + return fmt.Errorf("unknown severity %q", name) + } + var level int + if err := json.Unmarshal(data, &level); err != nil { + return fmt.Errorf("severity must be a name such as \"HIGH\" or a number from 0 to 4, not %s", data) + } + if level < int(SeverityInfo) || level > int(SeverityCritical) { + return fmt.Errorf("severity %d is outside 0 (INFO) to 4 (CRITICAL)", level) + } + *s = Severity(level) + return nil +} + // QuantumRisk indicates quantum vulnerability status type QuantumRisk string @@ -186,6 +226,29 @@ type Finding struct { EvidenceKind string `json:"-"` } +// MarshalJSON writes the finding with the "algorithm" and "keySize" keys on +// every finding: null when the pattern names no algorithm or records no key +// size, so that a reader can index both keys without checking for them. +// Reading such a finding back with encoding/json gives "" and 0. +func (f Finding) MarshalJSON() ([]byte, error) { + // plain has Finding's fields and none of its methods, so marshaling it + // does not call this method again. The two fields below are less deeply + // nested than plain's fields of the same JSON name, so they replace them. + type plain Finding + out := struct { + plain + Algorithm *string `json:"algorithm"` + KeySize *int `json:"keySize"` + }{plain: plain(f)} + if f.Algorithm != "" { + out.Algorithm = &f.Algorithm + } + if f.KeySize != 0 { + out.KeySize = &f.KeySize + } + return json.Marshal(out) +} + // Priority calculates a priority score for sorting (higher = more urgent) func (f *Finding) Priority() int { score := 0 diff --git a/pkg/types/types_test.go b/pkg/types/types_test.go index b5c3fa9..f7c4329 100644 --- a/pkg/types/types_test.go +++ b/pkg/types/types_test.go @@ -4,6 +4,7 @@ package types import ( + "encoding/json" "testing" ) @@ -317,3 +318,98 @@ func TestFindingFields(t *testing.T) { t.Error("Metadata nist_status should be deprecated") } } + +// A finding's severity is written by name, the form summary.bySeverity keys +// on, rather than as the number 0 to 4. +func TestFindingSeverityIsWrittenByName(t *testing.T) { + for _, sev := range []Severity{SeverityInfo, SeverityLow, SeverityMedium, SeverityHigh, SeverityCritical} { + data, err := json.Marshal(Finding{ID: "X-001", Severity: sev}) + if err != nil { + t.Fatalf("marshal: %v", err) + } + var doc map[string]any + if err := json.Unmarshal(data, &doc); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if got := doc["severity"]; got != sev.String() { + t.Errorf("severity %d is written as %#v, want %q", int(sev), got, sev.String()) + } + } +} + +// A severity is read by name in any case, and as the number an earlier +// release wrote, so that an older JSON report still loads as a baseline. +func TestSeverityReadsNameOrNumber(t *testing.T) { + for _, tc := range []struct { + in string + want Severity + }{ + {`"HIGH"`, SeverityHigh}, + {`"critical"`, SeverityCritical}, + {`"Info"`, SeverityInfo}, + {`3`, SeverityHigh}, + {`0`, SeverityInfo}, + {`4`, SeverityCritical}, + } { + var got Severity + if err := json.Unmarshal([]byte(tc.in), &got); err != nil { + t.Errorf("unmarshal %s: %v", tc.in, err) + continue + } + if got != tc.want { + t.Errorf("unmarshal %s = %s, want %s", tc.in, got, tc.want) + } + } + for _, bad := range []string{`"SEVERE"`, `5`, `-1`, `true`} { + var got Severity + if err := json.Unmarshal([]byte(bad), &got); err == nil { + t.Errorf("unmarshal %s = %s, want an error", bad, got) + } + } +} + +// Every finding carries the algorithm and keySize keys: null when the pattern +// names no algorithm or records no key size, the value otherwise. A finding +// written this way reads back unchanged. +func TestFindingAlwaysHasAlgorithmAndKeySizeKeys(t *testing.T) { + for _, f := range []Finding{ + {ID: "KEYSIZE-001", Type: "Hardcoded Key Size", Severity: SeverityLow}, + {ID: "RSA-001", Algorithm: "RSA", Severity: SeverityHigh}, + {ID: "RSA-1024", Algorithm: "RSA", KeySize: 1024, Severity: SeverityCritical}, + } { + data, err := json.Marshal(f) + if err != nil { + t.Fatalf("marshal %s: %v", f.ID, err) + } + var doc map[string]any + if err := json.Unmarshal(data, &doc); err != nil { + t.Fatalf("unmarshal %s: %v", f.ID, err) + } + algorithm, ok := doc["algorithm"] + if !ok { + t.Errorf("%s: no algorithm key in %s", f.ID, data) + } + keySize, ok := doc["keySize"] + if !ok { + t.Errorf("%s: no keySize key in %s", f.ID, data) + } + var wantAlgorithm, wantKeySize any + if f.Algorithm != "" { + wantAlgorithm = f.Algorithm + } + if f.KeySize != 0 { + wantKeySize = float64(f.KeySize) + } + if algorithm != wantAlgorithm || keySize != wantKeySize { + t.Errorf("%s: algorithm %#v, keySize %#v; want %#v and %#v", f.ID, algorithm, keySize, wantAlgorithm, wantKeySize) + } + + var back Finding + if err := json.Unmarshal(data, &back); err != nil { + t.Fatalf("read back %s: %v", f.ID, err) + } + if back.ID != f.ID || back.Algorithm != f.Algorithm || back.KeySize != f.KeySize || back.Severity != f.Severity { + t.Errorf("%s read back as %+v", f.ID, back) + } + } +} From 1ad2b363a5025192974e0012bce09099223ca7a0 Mon Sep 17 00:00:00 2001 From: Abdel Fane <32418586+thebenignhacker@users.noreply.github.com> Date: Tue, 20 Oct 2026 23:40:08 -0600 Subject: [PATCH 2/3] Refuse an -update-capture directory whose last path element is not cryptoscan, and state the rule in the flag description (#31) Fixes #31 --- internal/cli/readme_capture_test.go | 97 ++++++++++++++++++++++++++--- 1 file changed, 88 insertions(+), 9 deletions(-) diff --git a/internal/cli/readme_capture_test.go b/internal/cli/readme_capture_test.go index 1a1877c..f5fe7c8 100644 --- a/internal/cli/readme_capture_test.go +++ b/internal/cli/readme_capture_test.go @@ -33,7 +33,10 @@ import ( // go test -ldflags "-X github.com/csnp/cryptoscan/pkg/version.value=" \ // ./internal/cli -run TestReadmeExcerptIsCurrent -update-capture=/tmp/cryptoscan // -// The directory must be empty or absent; its path appears in the capture. +// The directory's last path element must be cryptoscan, the name the test +// scans from without -update-capture, since the report abbreviates paths to +// their last characters; another name is refused. The directory must be empty +// or absent; its path appears in the capture. const ( readmePath = "../../README.md" captureMetaPath = "../../docs/captures/scan-crypto-samples.json" @@ -41,10 +44,17 @@ const ( // readmeExcerptLead begins the README line that introduces the block. readmeExcerptLead = "What `cryptoscan scan crypto-samples` prints" + + // captureRootName is the last path element of the directory the scan runs + // in. The report abbreviates paths to their last characters, and the + // README shows ...scan/crypto-samples/. + captureRootName = "cryptoscan" ) var updateCapture = flag.String("update-capture", "", - "directory in which to scan a copy of crypto-samples; rewrites docs/captures/scan-crypto-samples.{txt,json} and the README block from that scan") + "directory in which to scan a copy of crypto-samples; the directory must be empty or absent and have "+captureRootName+ + " as its last path element, as in /tmp/"+captureRootName+ + "; rewrites docs/captures/scan-crypto-samples.{txt,json} and the README block from that scan") // captureMeta is scan-crypto-samples.json, in the order the file keeps. type captureMeta struct { @@ -430,13 +440,35 @@ func writeCapture(t *testing.T, out string, code int, root string, at time.Time) t.Logf("wrote %s, %s and the README block from a %d-line scan", meta.Capture, filepath.Base(captureMetaPath), len(lines)) } +// updateCaptureRoot returns the absolute path of the -update-capture +// directory, or an error saying why the capture cannot be made there. Its last +// path element must be captureRootName, the name TestReadmeExcerptIsCurrent +// scans from without -update-capture, and it must be empty or absent. +func updateCaptureRoot(dir string) (string, error) { + abs, err := filepath.Abs(dir) + if err != nil { + return "", err + } + if name := filepath.Base(abs); name != captureRootName { + return "", fmt.Errorf("-update-capture=%s: the last path element is %q, and it must be %s: "+ + "the report abbreviates paths to their last characters, as in ...scan/crypto-samples/, "+ + "and the README lines are checked against a scan from a directory named %s; use one such as /tmp/%s", + dir, name, captureRootName, captureRootName, captureRootName) + } + if entries, err := os.ReadDir(abs); err == nil && len(entries) > 0 { + return "", fmt.Errorf("%s is not empty; the capture must come from a directory holding only crypto-samples", abs) + } + return abs, nil +} + // TestReadmeExcerptIsCurrent scans crypto-samples the way the capture was // made, from a directory named cryptoscan that holds only the fixture, and // checks that every line the README shows is still printed. Streamed findings // are matched without their discovery number and in any order; the other // lines must appear in the README's order. Durations and the clock are -// ignored. With -update-capture, the scan runs in that directory and the -// capture, its metadata and the README block are rewritten from it first. +// ignored. With -update-capture, the scan runs in that directory, which must be +// named cryptoscan, and the capture, its metadata and the README block are +// rewritten from it first. func TestReadmeExcerptIsCurrent(t *testing.T) { meta := loadCaptureMeta(t) if meta.Command != "cryptoscan scan crypto-samples" { @@ -445,13 +477,10 @@ func TestReadmeExcerptIsCurrent(t *testing.T) { var root string if *updateCapture != "" { - abs, err := filepath.Abs(*updateCapture) + abs, err := updateCaptureRoot(*updateCapture) if err != nil { t.Fatal(err) } - if entries, err := os.ReadDir(abs); err == nil && len(entries) > 0 { - t.Fatalf("%s is not empty; the capture must come from a directory holding only crypto-samples", abs) - } root = abs } else { // os.MkdirTemp rather than t.TempDir, whose path carries the test @@ -462,7 +491,7 @@ func TestReadmeExcerptIsCurrent(t *testing.T) { t.Fatalf("create fixture directory: %v", err) } t.Cleanup(func() { os.RemoveAll(dir) }) - root = filepath.Join(dir, "cryptoscan") + root = filepath.Join(dir, captureRootName) } at := time.Now() @@ -516,3 +545,53 @@ func TestReadmeExcerptIsCurrent(t *testing.T) { len(missing), len(excerpt), meta.Capture, strings.Join(missing, "\n")) } } + +// TestUpdateCaptureRefusesOtherDirectoryNames checks the directories +// -update-capture accepts. A capture made in a directory with another last +// path element, such as other, writes README lines that +// TestReadmeExcerptIsCurrent then reports as missing from a fresh scan, so a +// directory not named cryptoscan is refused with a message that names the rule. +func TestUpdateCaptureRefusesOtherDirectoryNames(t *testing.T) { + base := t.TempDir() + absent := filepath.Join(base, "absent", captureRootName) + empty := filepath.Join(base, "empty", captureRootName) + full := filepath.Join(base, "full", captureRootName) + other := filepath.Join(base, "other") + for _, dir := range []string{empty, filepath.Join(full, "crypto-samples"), other} { + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + } + sep := string(filepath.Separator) + + for _, tc := range []struct{ dir, want string }{ + {absent, absent}, + {absent + sep, absent}, + {empty, empty}, + } { + if got, err := updateCaptureRoot(tc.dir); err != nil || got != tc.want { + t.Errorf("updateCaptureRoot(%q) = %q, %v; want %q, nil", tc.dir, got, err, tc.want) + } + } + + for _, dir := range []string{ + other, // empty + filepath.Join(base, "new"), // absent + filepath.Join(base, "CryptoScan"), // the name in another case + filepath.Join(base, "cryptoscan-old"), + empty + sep + "..", // the parent of a directory named cryptoscan + } { + _, err := updateCaptureRoot(dir) + if err == nil { + t.Errorf("updateCaptureRoot(%q) accepted a directory whose last path element is not %s", dir, captureRootName) + continue + } + if msg := err.Error(); !strings.Contains(msg, "last path element") || !strings.Contains(msg, "must be "+captureRootName) { + t.Errorf("updateCaptureRoot(%q) refused it with %q, which does not name the rule", dir, msg) + } + } + + if _, err := updateCaptureRoot(full); err == nil || !strings.Contains(err.Error(), "is not empty") { + t.Errorf("updateCaptureRoot(%q) = %v; want the refusal of a directory that is not empty", full, err) + } +} From 4104b1814b5a86c692002ed93753b18c743c336e Mon Sep 17 00:00:00 2001 From: Abdel Fane <32418586+thebenignhacker@users.noreply.github.com> Date: Tue, 20 Oct 2026 23:42:50 -0600 Subject: [PATCH 3/3] List CryptoScan's own reports as not scanned, name a change in each dependency fix, and use current GoReleaser keys (#23) Refs #23 Still open in #23: certificate context for CERT-EXPIRY-001 and CERT-SUBJECT-001 and one finding for an RSA use and its key size (both detection changes), and a pinned golangci-lint version (needs a .github/ change). --- .goreleaser.yaml | 15 +- CHANGELOG.md | 55 ++++++ README.md | 2 +- docs/ci.md | 19 +- internal/cli/report_skip_test.go | 195 +++++++++++++++++++ internal/cli/scan.go | 10 +- pkg/analyzer/dependencies.go | 40 ++-- pkg/analyzer/dependency_fix_test.go | 101 ++++++++++ pkg/reporter/report_skip_test.go | 119 ++++++++++++ pkg/reporter/text.go | 15 +- pkg/scanner/report.go | 176 +++++++++++++++++ pkg/scanner/report_test.go | 284 ++++++++++++++++++++++++++++ pkg/scanner/scanner.go | 53 +++++- 13 files changed, 1041 insertions(+), 43 deletions(-) create mode 100644 internal/cli/report_skip_test.go create mode 100644 pkg/analyzer/dependency_fix_test.go create mode 100644 pkg/reporter/report_skip_test.go create mode 100644 pkg/scanner/report.go create mode 100644 pkg/scanner/report_test.go diff --git a/.goreleaser.yaml b/.goreleaser.yaml index b20ea3e..77f17a6 100644 --- a/.goreleaser.yaml +++ b/.goreleaser.yaml @@ -32,12 +32,12 @@ builds: archives: - id: default - format: tar.gz + formats: [tar.gz] name_template: >- {{ .ProjectName }}_{{ .Version }}_{{ .Os }}_{{ .Arch }} format_overrides: - goos: windows - format: zip + formats: [zip] files: - LICENSE - README.md @@ -102,21 +102,20 @@ release: Verify your download with the checksums.txt file. -brews: +# A cask has no install or test block: its binary stanza installs cryptoscan. +homebrew_casks: - name: cryptoscan repository: owner: csnp name: homebrew-tap token: "{{ .Env.HOMEBREW_TAP_TOKEN }}" skip_upload: true - directory: Formula + directory: Casks homepage: https://qramm.org description: Cryptographic discovery tool for the post-quantum era license: Apache-2.0 - install: | - bin.install "cryptoscan" - test: | - system "#{bin}/cryptoscan", "--version" + binaries: + - cryptoscan nfpms: - id: packages diff --git a/CHANGELOG.md b/CHANGELOG.md index 609a686..030bccc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -81,6 +81,61 @@ All notable changes to CryptoScan are recorded here. This project follows check mark line now appears only when at least one file was scanned. JSON output is unchanged and already records `filesScanned` as 0. +- **Reports CryptoScan wrote into the scanned tree are listed as not + scanned, instead of being read as source.** The + README and `docs/ci.md` write reports into the tree they scan, for example + with `cryptoscan scan . --format sarif --output results.sarif`, and the next + scan read them as source. Run twice in a copy of `crypto-samples`, that + command wrote 110 results and then 903. A directory holding only the pretty + JSON report of `crypto-samples` gave 1313 findings, its SARIF report 793 and + its CBOM 184, and each of those scans exited 1. A scan now leaves out a file + that starts like a CryptoScan report: JSON from `scan`, `gate` or `pqc` + (`tool.name` is `CryptoScan`), SARIF (`runs[].tool.driver.name`), a + CycloneDX CBOM (`metadata.tools[].name`), a CSV report (its header row), or + a text report (its title box, or the `Scanning...` line of streamed + output). It also leaves out the file `--output` names, whatever that file + holds. Each such file is named as not scanned, with a reason such as + `CryptoScan report: SARIF` or + `CryptoScan report: the --output file of this scan`, in the text summary, + on stderr for the other formats, and under `skippedFiles` in JSON. A report + saved as documentation, such as `report.txt` or a file under `docs/`, was + already left out of the scan and is still not listed. The text summary adds + `To leave CryptoScan reports off this list, write them outside the scanned folder or pass their names to --exclude.` + The same command now writes 110 results on both runs, and each of the + three report directories gives 0 findings and exits 0. The CSV report and + the single-line JSON report of `crypto-samples` were already left out, as + minified; they are now named as CryptoScan reports. + +- **Dependency findings for libraries without post-quantum algorithms name a + change.** Eleven of the libraries the scanner recognizes in dependency + manifests, among them `node-forge`, `tweetnacl`, `jose` and `pycryptodome`, + printed `Fix: Plan migration to a PQC-capable library or enable hybrid mode when available.`, + which names no change to make. Each library without post-quantum + algorithms now names what to move and what to move it to and, for + public-key algorithms in a Python, JavaScript, Go, Java or Rust library, + the post-quantum library the remediation engine names for that language, + for example + `Add liboqs-node and move RSA from node-forge to ML-KEM-768 for encryption and key exchange and ML-DSA-65 for signatures`. + The `cryptography` entry, which said + `Add liboqs-python for PQC support alongside cryptography`, now names what + to move as well. The text is the `Fix:` line of the text report and the + `fix` and `remediation` fields of JSON findings; SARIF writes it as the + `help.text` of the finding's rule, and CSV in its Remediation column. + +### Changed + +- **The release configuration uses the current GoReleaser keys.** + `goreleaser check` (GoReleaser 2.17.1) reported `archives.format`, + `archives.format_overrides.format` and `brews` as deprecated and exited 2. + The archives now set `formats`, and the Homebrew definition is a cask + (`homebrew_casks`), written to `dist/homebrew/Casks/cryptoscan.rb` in place + of the formula in `dist/homebrew/Formula/cryptoscan.rb`. It is still not + uploaded to the tap (`skip_upload: true`). A cask has no `install` or `test` + block, so the formula's `bin.install` and `--version` test are gone; the + cask's `binary "cryptoscan"` stanza installs the binary. `goreleaser check` + now exits 0, and a snapshot build writes the same six archives as before: + tar.gz for Linux and macOS, zip for Windows. + ## [1.5.0] - 2026-10-18 ### Added diff --git a/README.md b/README.md index 09f7607..422a467 100644 --- a/README.md +++ b/README.md @@ -138,7 +138,7 @@ CryptoScan goes beyond simple pattern matching: - **Confidence scoring**: Findings in comments, logs, or docs are marked low confidence - **File type awareness**: Different severity for code vs. test vs. documentation - **Language detection**: 15+ programming languages recognized -- **Noise reduction**: Automatically filters minified files, lock files, and build artifacts. The scan summary counts the minified files it left out and names up to ten; JSON output lists them under `skippedFiles` +- **Noise reduction**: Automatically filters minified files, lock files, build artifacts, and the reports CryptoScan writes (JSON, SARIF, CBOM, CSV and text), so a report saved in the project is not scanned as source. The scan summary counts the minified files and reports it left out and names up to ten; JSON output lists them under `skippedFiles` Algorithm names that appear only in prose, logs or comments are held back from the default report, and the scan summary says how many. [How CryptoScan works](docs/how-it-works.md) explains the rule. diff --git a/docs/ci.md b/docs/ci.md index 1d0fbd8..f72faa3 100644 --- a/docs/ci.md +++ b/docs/ci.md @@ -44,11 +44,20 @@ as an error in the SARIF run's `invocations[].toolExecutionNotifications`. Without `--fail-on`, such a file does not change the exit code. A file skipped because it looks minified or generated, such as `app.min.js`, -or because it is a dependency manifest the scanner has no reader for, such as -`build.gradle` or `Pipfile`, is named in the text summary or, for the other -formats, on stderr (up to ten files, with a count of the rest), and JSON -output also carries it under `skippedFiles`. It does not change the exit code, -with or without `--fail-on`. +because it is a dependency manifest the scanner has no reader for, such as +`build.gradle` or `Pipfile`, or because it is a report CryptoScan wrote, such +as the `baseline.json` or `results.sarif` that the commands on this page write +into the project, is named in the text summary or, for the other formats, on +stderr (up to ten files, with a count of the rest), and JSON output also +carries it under `skippedFiles`. It does not change the exit code, with or +without `--fail-on`. + +CryptoScan recognizes its reports by how they start: the tool name in a JSON, +SARIF or CBOM report, the header row of a CSV report, and the title box of a +text report or the `Scanning...` line that starts a scan's streamed output. +The file `--output` names is left out of the scan that writes it, whatever it +holds. To keep reports off the list, write them outside the scanned directory +or name them in `--exclude`. Without `--fail-on`, the scan exits 1 on any CRITICAL finding. The full table is in [Exit codes](commands.md#exit-codes). diff --git a/internal/cli/report_skip_test.go b/internal/cli/report_skip_test.go new file mode 100644 index 0000000..1e2908f --- /dev/null +++ b/internal/cli/report_skip_test.go @@ -0,0 +1,195 @@ +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) +// SPDX-License-Identifier: Apache-2.0 + +package cli + +import ( + "encoding/json" + "os" + "path/filepath" + "reflect" + "strings" + "testing" + + "github.com/csnp/cryptoscan/pkg/scanner" +) + +// chdirForTest changes the working directory to dir until the test ends. +func chdirForTest(t *testing.T, dir string) { + t.Helper() + wd, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + if err := os.Chdir(dir); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + if err := os.Chdir(wd); err != nil { + t.Fatal(err) + } + }) +} + +// scanHere runs "cryptoscan scan . --format --output +// --exclude " from the working directory, the way the README and +// docs/ci.md run it from a project root, and returns the exit code and what +// the scan wrote to stderr. +func scanHere(t *testing.T, format, output, exclude string) (int, string) { + t.Helper() + resetPipelineFlags(t) + outputFormat, outputFile, excludeGlobs = format, output, exclude + var code int + stderr := captureStderr(func() { + var err error + code, err = scanTarget(scanCmd, ".") + if err != nil { + t.Errorf("scan . --format %s --output %s: %v", format, output, err) + } + }) + return code, stderr +} + +// reportSummary is what a JSON report says about which files were read: the +// base name of each file a finding is in, and the base name and reason of +// each file listed under skippedFiles. +type reportSummary struct { + findingFiles map[string]int + skipped map[string]string +} + +func readReportSummary(t *testing.T, path string) reportSummary { + t.Helper() + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("no report written: %v", err) + } + var report scanner.Results + if err := json.Unmarshal(data, &report); err != nil { + t.Fatalf("invalid JSON report: %v", err) + } + s := reportSummary{findingFiles: map[string]int{}, skipped: map[string]string{}} + for _, f := range report.Findings { + s.findingFiles[filepath.Base(f.File)]++ + } + for _, sk := range report.SkippedFiles { + s.skipped[filepath.Base(sk.File)] = sk.Reason + } + return s +} + +// sarifResultFiles returns the base name of the file each result in the SARIF +// report at path points at. +func sarifResultFiles(t *testing.T, path string) []string { + t.Helper() + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("no SARIF report written: %v", err) + } + var report struct { + Runs []struct { + 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(data, &report); err != nil { + t.Fatalf("invalid SARIF report: %v", err) + } + var files []string + for _, run := range report.Runs { + for _, result := range run.Results { + for _, loc := range result.Locations { + files = append(files, filepath.Base(loc.PhysicalLocation.ArtifactLocation.URI)) + } + } + } + return files +} + +// The reproduction from the report, through the scan command: run from the +// project root, "scan . --format sarif --output results.sarif" writes its +// report into the tree it scans, and the next run read that report as +// source. The reports earlier runs wrote, in any format and from scan or +// pqc, are now named as not scanned, and the findings come from the source +// file alone. Naming the reports in --exclude leaves them off the list. +func TestScanDoesNotReadTheReportsEarlierRunsWrote(t *testing.T) { + savedPretty, savedStream := jsonPretty, streamFindings + t.Cleanup(func() { jsonPretty, streamFindings = savedPretty, savedStream }) + jsonPretty, streamFindings = true, false + + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "a.py"), []byte("import hashlib\nh = hashlib.md5(b\"a\")\n"), 0o644); err != nil { + t.Fatal(err) + } + chdirForTest(t, dir) + + first, _ := scanHere(t, "sarif", "results.sarif", "") + firstFiles := sarifResultFiles(t, "results.sarif") + second, stderr := scanHere(t, "sarif", "results.sarif", "") + if second != first { + t.Errorf("exit code = %d on the second run and %d on the first; the first run's report changed the result", second, first) + } + if want := "results.sarif CryptoScan report: the --output file of this scan\n"; !strings.Contains(stderr, want) { + t.Errorf("stderr of the second run has no line ending %q:\n%s", want, stderr) + } + secondFiles := sarifResultFiles(t, "results.sarif") + if len(firstFiles) == 0 || !reflect.DeepEqual(secondFiles, firstFiles) { + t.Errorf("SARIF results are in %q on the second run and %q on the first; want the same non-empty list", secondFiles, firstFiles) + } + + // The pqc JSON view and a scan's streamed text output, saved in the tree. + resetPipelineFlags(t) + outputFormat = "json" + pqcJSON := captureStdout(func() { + if _, err := pqcTarget(pqcCmd, "."); err != nil { + t.Errorf("pqc . --format json: %v", err) + } + }) + if err := os.WriteFile("pqc.json", []byte(pqcJSON), 0o644); err != nil { + t.Fatal(err) + } + resetPipelineFlags(t) + streamFindings = true + streamed := captureStdout(func() { + if _, err := scanTarget(scanCmd, "a.py"); err != nil { + t.Errorf("scan a.py: %v", err) + } + }) + streamFindings = false + if !strings.Contains(streamed, "MD5") { + t.Fatalf("streamed output names no MD5 finding:\n%s", streamed) + } + if err := os.WriteFile("scan-output.log", []byte(streamed), 0o644); err != nil { + t.Fatal(err) + } + + scanHere(t, "json", "report.json", "") + scanHere(t, "json", "report.json", "") + got := readReportSummary(t, "report.json") + wantSkipped := map[string]string{ + "pqc.json": "CryptoScan report: JSON", + "report.json": "CryptoScan report: the --output file of this scan", + "results.sarif": "CryptoScan report: SARIF", + "scan-output.log": "CryptoScan report: text", + } + if !reflect.DeepEqual(got.skipped, wantSkipped) { + t.Errorf("skippedFiles = %v, want %v", got.skipped, wantSkipped) + } + if len(got.findingFiles) != 1 || got.findingFiles["a.py"] == 0 { + t.Errorf("findings are in %v, want a.py alone", got.findingFiles) + } + + scanHere(t, "json", "report.json", "report.json,results.sarif,pqc.json,scan-output.log") + excluded := readReportSummary(t, "report.json") + if len(excluded.skipped) != 0 || len(excluded.findingFiles) != 1 || excluded.findingFiles["a.py"] == 0 { + t.Errorf("with the reports in --exclude: skippedFiles = %v, findings in %v; want none, and a.py alone", + excluded.skipped, excluded.findingFiles) + } +} diff --git a/internal/cli/scan.go b/internal/cli/scan.go index 38f2ff0..ac17e5f 100644 --- a/internal/cli/scan.go +++ b/internal/cli/scan.go @@ -350,6 +350,10 @@ func scanPipeline(cmd *cobra.Command, target string, opts pipelineOptions) (*sca ConfigFile: cfgRules, } + // The file --output names holds an earlier report, if anything, and this + // run replaces it, so the scan does not read it. + cfg.ReportFile = outputFile + // The banner and streamed findings belong to the scan's text report, not // to a view. textReport := outputFormat == "text" && opts.view == nil @@ -590,9 +594,9 @@ func writeUnanalyzedNotice(w io.Writer, format string, files []scanner.Unanalyze const maxSkippedNamed = 10 // writeSkippedNotice names the files the scan left out as minified or -// generated, or as dependency manifests it has no reader for, for the same -// formats and reasons as writeUnanalyzedNotice. The exit code does not depend -// on them. +// generated, as dependency manifests it has no reader for, or as reports +// CryptoScan wrote, for the same formats and reasons as writeUnanalyzedNotice. +// The exit code does not depend on them. func writeSkippedNotice(w io.Writer, format string, files []scanner.SkippedFile) { if format == "text" || len(files) == 0 { return diff --git a/pkg/analyzer/dependencies.go b/pkg/analyzer/dependencies.go index 91c0046..72b8247 100644 --- a/pkg/analyzer/dependencies.go +++ b/pkg/analyzer/dependencies.go @@ -38,19 +38,24 @@ type DependencyFinding struct { Remediation string } -// KnownCryptoLibraries contains well-known cryptographic libraries +// KnownCryptoLibraries contains well-known cryptographic libraries. For a +// library without post-quantum algorithms, Migration names the algorithms to +// move and their replacements, and the post-quantum library the remediation +// engine names for that language, where it names one. var KnownCryptoLibraries = []CryptoLibrary{ // Python {Name: "cryptography", Package: "cryptography", Language: LangPython, Algorithms: []string{"RSA", "ECC", "AES", "ChaCha20"}, QuantumSafe: false, Description: "Python cryptographic recipes and primitives", - Migration: "Add liboqs-python for PQC support alongside cryptography"}, + Migration: "Add liboqs-python alongside cryptography and move RSA and ECC to ML-KEM-768 for key exchange and ML-DSA-65 for signatures"}, {Name: "PyCryptodome", Package: "pycryptodome", Language: LangPython, Algorithms: []string{"RSA", "ECC", "AES", "DES", "3DES"}, QuantumSafe: false, - Description: "Self-contained Python crypto library"}, + Description: "Self-contained Python crypto library", + Migration: "Add liboqs-python and move RSA and ECC from pycryptodome to ML-KEM-768 for key exchange and ML-DSA-65 for signatures"}, {Name: "PyNaCl", Package: "pynacl", Language: LangPython, Algorithms: []string{"X25519", "Ed25519", "ChaCha20"}, QuantumSafe: false, - Description: "Python binding to libsodium"}, + Description: "Python binding to libsodium", + Migration: "Add liboqs-python and move X25519 key exchange from pynacl to ML-KEM-768 and Ed25519 signatures to ML-DSA-65"}, {Name: "liboqs-python", Package: "liboqs-python", Language: LangPython, Algorithms: []string{"ML-KEM", "ML-DSA", "SLH-DSA"}, QuantumSafe: true, Description: "Python bindings for liboqs (post-quantum algorithms)"}, @@ -62,7 +67,8 @@ var KnownCryptoLibraries = []CryptoLibrary{ Migration: "Replace crypto-js with node:crypto in Node.js or the Web Crypto API (crypto.subtle) in the browser, then remove crypto-js from package.json"}, {Name: "node-forge", Package: "node-forge", Language: LangJavaScript, Algorithms: []string{"RSA", "AES", "DES", "SHA", "MD5"}, QuantumSafe: false, - Description: "JavaScript implementation of TLS and crypto"}, + Description: "JavaScript implementation of TLS and crypto", + Migration: "Add liboqs-node and move RSA from node-forge to ML-KEM-768 for encryption and key exchange and ML-DSA-65 for signatures"}, {Name: "bcrypt", Package: "bcrypt", Language: LangJavaScript, Algorithms: []string{"bcrypt"}, QuantumSafe: true, Description: "bcrypt password hashing (quantum-resistant for passwords)"}, @@ -71,10 +77,12 @@ var KnownCryptoLibraries = []CryptoLibrary{ Description: "Argon2 password hashing (quantum-resistant)"}, {Name: "tweetnacl", Package: "tweetnacl", Language: LangJavaScript, Algorithms: []string{"X25519", "Ed25519", "XSalsa20"}, QuantumSafe: false, - Description: "Port of TweetNaCl cryptographic library"}, + Description: "Port of TweetNaCl cryptographic library", + Migration: "Add liboqs-node and move X25519 key exchange from tweetnacl to ML-KEM-768 and Ed25519 signatures to ML-DSA-65"}, {Name: "jose", Package: "jose", Language: LangJavaScript, Algorithms: []string{"RSA", "ECDSA", "EdDSA", "AES"}, QuantumSafe: false, - Description: "JavaScript Object Signing and Encryption"}, + Description: "JavaScript Object Signing and Encryption", + Migration: "Add liboqs-node and move RSA, ECDSA and EdDSA signatures made with jose to ML-DSA-65, and RSA key encryption to ML-KEM-768"}, // Go {Name: "circl", Package: "github.com/cloudflare/circl", Language: LangGo, @@ -82,7 +90,8 @@ var KnownCryptoLibraries = []CryptoLibrary{ Description: "Cloudflare Interoperable Reusable Cryptographic Library with PQC"}, {Name: "go-jose", Package: "github.com/go-jose/go-jose", Language: LangGo, Algorithms: []string{"RSA", "ECDSA", "EdDSA"}, QuantumSafe: false, - Description: "Go implementation of JOSE standards"}, + Description: "Go implementation of JOSE standards", + Migration: "Add cloudflare/circl and move RSA, ECDSA and EdDSA signatures made with go-jose to ML-DSA-65, and RSA key encryption to ML-KEM-768"}, // Java {Name: "Bouncy Castle", Package: "org.bouncycastle", Language: LangJava, @@ -91,15 +100,18 @@ var KnownCryptoLibraries = []CryptoLibrary{ Migration: "Upgrade to BC 1.78+ for ML-KEM and ML-DSA support"}, {Name: "Google Tink", Package: "com.google.crypto.tink", Language: LangJava, Algorithms: []string{"AES-GCM", "ECDSA", "Ed25519"}, QuantumSafe: false, - Description: "Multi-language, cross-platform crypto library"}, + Description: "Multi-language, cross-platform crypto library", + Migration: "Add BouncyCastle and move ECDSA and Ed25519 signatures from Google Tink to ML-DSA-65"}, // Rust {Name: "ring", Package: "ring", Language: LangRust, Algorithms: []string{"AES-GCM", "ChaCha20", "RSA", "ECDSA"}, QuantumSafe: false, - Description: "Safe, fast, small crypto using Rust"}, + Description: "Safe, fast, small crypto using Rust", + Migration: "Add pqcrypto and move RSA and ECDSA signatures from ring to ML-DSA-65"}, {Name: "rustcrypto", Package: "aes", Language: LangRust, Algorithms: []string{"AES", "ChaCha20"}, QuantumSafe: false, - Description: "Pure Rust implementation of crypto algorithms"}, + Description: "Pure Rust implementation of crypto algorithms", + Migration: "Use Aes256, a 256-bit key, in place of Aes128 and Aes192 from the aes crate"}, {Name: "pqcrypto", Package: "pqcrypto", Language: LangRust, Algorithms: []string{"ML-KEM", "ML-DSA", "SLH-DSA"}, QuantumSafe: true, Description: "Post-quantum cryptography for Rust"}, @@ -107,10 +119,12 @@ var KnownCryptoLibraries = []CryptoLibrary{ // Ruby {Name: "OpenSSL Ruby", Package: "openssl", Language: LangRuby, Algorithms: []string{"RSA", "ECC", "AES"}, QuantumSafe: false, - Description: "Ruby OpenSSL bindings"}, + Description: "Ruby OpenSSL bindings", + Migration: "Move RSA and ECC keys made with the openssl gem to ML-KEM-768 for key exchange and ML-DSA-65 for signatures"}, {Name: "RbNaCl", Package: "rbnacl", Language: LangRuby, Algorithms: []string{"X25519", "Ed25519", "XSalsa20"}, QuantumSafe: false, - Description: "Ruby binding to libsodium"}, + Description: "Ruby binding to libsodium", + Migration: "Move X25519 key exchange from rbnacl to ML-KEM-768 and Ed25519 signatures to ML-DSA-65"}, } // utf8BOM is the UTF-8 byte order mark. It marks the file's encoding and is diff --git a/pkg/analyzer/dependency_fix_test.go b/pkg/analyzer/dependency_fix_test.go new file mode 100644 index 0000000..2f42f77 --- /dev/null +++ b/pkg/analyzer/dependency_fix_test.go @@ -0,0 +1,101 @@ +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) +// SPDX-License-Identifier: Apache-2.0 + +package analyzer + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/csnp/cryptoscan/pkg/types" +) + +// applyToDependency runs the remediation engine over the finding the scanner +// builds from dep, and returns its Fix and Guidance. +func applyToDependency(dep DependencyFinding) (fix, guidance string) { + f := types.Finding{ + FindingType: types.FindingTypeDependency, File: dep.File, Line: 1, Match: dep.Library.Package, + Severity: dep.Severity, Quantum: dep.Quantum, Remediation: dep.Remediation, + } + NewRemediationEngine().Apply(&f) + return f.Fix, f.Guidance +} + +// Eleven libraries without post-quantum algorithms printed "Fix: Plan +// migration to a PQC-capable library or enable hybrid mode when available.", +// which names no change to make. Every such library now has a fix that names +// it and the replacement for what it provides. +func TestDependencyFixNamesAReplacementForEveryLibraryWithoutPQC(t *testing.T) { + for _, lib := range KnownCryptoLibraries { + if lib.QuantumSafe { + continue + } + t.Run(lib.Name, func(t *testing.T) { + fix, guidance := applyToDependency(createDependencyFinding(lib, "1.0.0", "manifest")) + if fix == "" || guidance != "" { + t.Fatalf("Fix = %q, Guidance = %q; want a fix and no guidance", fix, guidance) + } + for _, vague := range []string{"Plan migration", "when available", "Consider"} { + if strings.Contains(fix, vague) { + t.Errorf("Fix %q says %q, which names no change", fix, vague) + } + } + if !strings.Contains(fix, lib.Package) && !strings.Contains(fix, lib.Name) { + t.Errorf("Fix %q names neither the package %q nor the library %q", fix, lib.Package, lib.Name) + } + named := strings.HasPrefix(fix, "Replace ") + for _, replacement := range []string{"ML-KEM-768", "ML-DSA-65", "256-bit"} { + named = named || strings.Contains(fix, replacement) + } + if !named { + t.Errorf("Fix %q names no replacement", fix) + } + }) + } +} + +// The libraries from the report's ecosystems, read from real manifests: each +// fix names the algorithms to move, their replacements and the library to add. +func TestDependencyFindingsNameTheirReplacement(t *testing.T) { + dir := t.TempDir() + manifests := map[string]string{ + "package.json": `{"dependencies": {"node-forge": "^1.3.1", "tweetnacl": "^1.0.3", "jose": "^5.9.0"}}`, + "requirements.txt": "pycryptodome==3.20.0\npynacl==1.5.0\ncryptography==43.0.0\n", + } + want := map[string]string{ + "node-forge": "Add liboqs-node and move RSA from node-forge to ML-KEM-768 for encryption and key exchange and ML-DSA-65 for signatures", + "tweetnacl": "Add liboqs-node and move X25519 key exchange from tweetnacl to ML-KEM-768 and Ed25519 signatures to ML-DSA-65", + "jose": "Add liboqs-node and move RSA, ECDSA and EdDSA signatures made with jose to ML-DSA-65, and RSA key encryption to ML-KEM-768", + "pycryptodome": "Add liboqs-python and move RSA and ECC from pycryptodome to ML-KEM-768 for key exchange and ML-DSA-65 for signatures", + "pynacl": "Add liboqs-python and move X25519 key exchange from pynacl to ML-KEM-768 and Ed25519 signatures to ML-DSA-65", + "cryptography": "Add liboqs-python alongside cryptography and move RSA and ECC to ML-KEM-768 for key exchange and ML-DSA-65 for signatures", + } + got := map[string]string{} + for name, content := range manifests { + path := filepath.Join(dir, name) + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatal(err) + } + deps, err := ScanDependencies(path) + if err != nil { + t.Fatalf("%s: %v", name, err) + } + for _, dep := range deps { + fix, guidance := applyToDependency(dep) + if guidance != "" { + t.Errorf("%s: Guidance = %q, want none", dep.Library.Package, guidance) + } + got[dep.Library.Package] = fix + } + } + for pkg, wantFix := range want { + if got[pkg] != wantFix { + t.Errorf("%s: Fix = %q\nwant %q", pkg, got[pkg], wantFix) + } + } + if len(got) != len(want) { + t.Errorf("findings for %d packages, want %d: %v", len(got), len(want), got) + } +} diff --git a/pkg/reporter/report_skip_test.go b/pkg/reporter/report_skip_test.go new file mode 100644 index 0000000..5e73cb7 --- /dev/null +++ b/pkg/reporter/report_skip_test.go @@ -0,0 +1,119 @@ +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) +// SPDX-License-Identifier: Apache-2.0 + +package reporter + +import ( + "os" + "path/filepath" + "reflect" + "strings" + "testing" + + "github.com/csnp/cryptoscan/pkg/scanner" +) + +// The reproduction from the report: the README and docs/ci.md write reports +// into the tree they scan, and the next scan read them as source. A directory +// holding only the pretty JSON report of crypto-samples gave 1313 findings, +// the SARIF report 793 and the CBOM 184, and each scan failed --fail-on high. +// Every report the reporters write is now named as not scanned, and none of +// them gives a finding. The text reports are saved under a .log name here, +// since a .txt file is documentation and is not scanned by default anyway. +func TestScanLeavesOutTheReportsCryptoScanWrites(t *testing.T) { + samples, err := scanner.New(scanner.Config{Target: "../../crypto-samples", MinSeverity: scanner.SeverityInfo}).Scan() + if err != nil { + t.Fatalf("scan crypto-samples: %v", err) + } + if len(samples.Findings) == 0 { + t.Fatal("crypto-samples gave no findings, so its reports would prove nothing") + } + + type report struct { + name string + rep Reporter + reason string + } + reports := []report{ + {"baseline.json", NewJSONReporter(false), "CryptoScan report: JSON"}, + {"crypto-bom.json", NewCBOMReporter(), "CryptoScan report: CycloneDX CBOM"}, + {"findings.csv", NewCSVReporter(), "CryptoScan report: CSV"}, + {"findings.json", NewJSONReporter(true), "CryptoScan report: JSON"}, + {"pqc-report.log", NewPQCReporter(false), "CryptoScan report: text"}, + {"results.sarif", NewSARIFReporter(), "CryptoScan report: SARIF"}, + {"scan-report-color.log", NewTextReporter(true), "CryptoScan report: text"}, + {"scan-report.log", NewTextReporter(false), "CryptoScan report: text"}, + } + dir := t.TempDir() + var want []scanner.SkippedFile + for _, r := range reports { + out, err := r.rep.Generate(samples) + if err != nil { + t.Fatalf("generate %s: %v", r.name, err) + } + path := filepath.Join(dir, r.name) + if err := os.WriteFile(path, []byte(out), 0o644); err != nil { + t.Fatal(err) + } + want = append(want, scanner.SkippedFile{File: path, Reason: r.reason}) + } + + results, err := scanner.New(scanner.Config{Target: dir, MinSeverity: scanner.SeverityInfo}).Scan() + if err != nil { + t.Fatalf("scan the reports: %v", err) + } + if len(results.Findings) != 0 || results.FilesScanned != 0 { + t.Errorf("findings = %d, filesScanned = %d; want 0 and 0", len(results.Findings), results.FilesScanned) + } + if !reflect.DeepEqual(results.SkippedFiles, want) { + t.Errorf("skippedFiles = %+v\nwant %+v", results.SkippedFiles, want) + } + if results.HasFindingsAtOrAbove(scanner.SeverityHigh) { + t.Error("a directory of reports fails --fail-on high") + } +} + +// A report CryptoScan wrote is listed with the files that were not scanned. +// The advice for minified files does not fit it, so it has its own, and a list +// holding both kinds gets both. +func TestTextReportAdvisesOnCryptoScanReports(t *testing.T) { + const ( + reportAdvice = "\n To leave CryptoScan reports off this list, write them outside the scanned folder or pass their names to --exclude.\n" + minifiedAdvice = "\n Scan the source these files were built from, or search them for algorithm names.\n" + ) + report := scanner.SkippedFile{File: "/test/path/results.sarif", Reason: "CryptoScan report: SARIF"} + output := scanner.SkippedFile{File: "/test/path/scan-report", Reason: "CryptoScan report: the --output file of this scan"} + minified := scanner.SkippedFile{File: "/test/path/app.min.js", Reason: "minified: the file name ends in .min.js"} + + tests := []struct { + name string + files []scanner.SkippedFile + wantReport bool + wantMinified bool + }{ + {"reports only", []scanner.SkippedFile{report, output}, true, false}, + {"minified file only", []scanner.SkippedFile{minified}, false, true}, + {"minified file and report", []scanner.SkippedFile{minified, report}, true, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + results := createTestResults() + results.SkippedFiles = tt.files + text, err := NewTextReporter(false).Generate(results) + if err != nil { + t.Fatalf("Generate failed: %v", err) + } + for _, sk := range tt.files { + if want := "\n " + sk.File + " " + sk.Reason + "\n"; !strings.Contains(text, want) { + t.Errorf("text report is missing %q", want) + } + } + if got := strings.Contains(text, reportAdvice); got != tt.wantReport { + t.Errorf("text report has %q: %v, want %v", reportAdvice, got, tt.wantReport) + } + if got := strings.Contains(text, minifiedAdvice); got != tt.wantMinified { + t.Errorf("text report has %q: %v, want %v", minifiedAdvice, got, tt.wantMinified) + } + }) + } +} diff --git a/pkg/reporter/text.go b/pkg/reporter/text.go index 721e4a5..77be7c0 100644 --- a/pkg/reporter/text.go +++ b/pkg/reporter/text.go @@ -500,7 +500,8 @@ func (r *TextReporter) generateGroupedByFile(results *scanner.Results, b *string const maxSkippedListed = 10 // writeSkippedFiles names the files the scan left out as minified or -// generated, or as dependency manifests it has no reader for, and why. +// generated, as dependency manifests it has no reader for, or as reports +// CryptoScan wrote, and why. func (r *TextReporter) writeSkippedFiles(b *strings.Builder, files []scanner.SkippedFile) { b.WriteString(r.color(colorYellow, fmt.Sprintf( "\n %d file(s) were not scanned, so cryptography in them is not in this report:\n", @@ -512,11 +513,14 @@ func (r *TextReporter) writeSkippedFiles(b *strings.Builder, files []scanner.Ski } fmt.Fprintf(b, " %s %s\n", sk.File, sk.Reason) } - minified, manifests := false, false + minified, manifests, reports := false, false, false for _, sk := range files { - if sk.IsUnreadManifest() { + switch { + case sk.IsUnreadManifest(): manifests = true - } else { + case sk.IsCryptoScanReport(): + reports = true + default: minified = true } } @@ -526,6 +530,9 @@ func (r *TextReporter) writeSkippedFiles(b *strings.Builder, files []scanner.Ski if manifests { b.WriteString(" Check the dependency manifests for cryptography libraries by hand.\n") } + if reports { + b.WriteString(" To leave CryptoScan reports off this list, write them outside the scanned folder or pass their names to --exclude.\n") + } } // writeSkippedDependencyDirs names the node_modules directories the scan did diff --git a/pkg/scanner/report.go b/pkg/scanner/report.go new file mode 100644 index 0000000..28cd8fa --- /dev/null +++ b/pkg/scanner/report.go @@ -0,0 +1,176 @@ +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) +// SPDX-License-Identifier: Apache-2.0 + +package scanner + +import ( + "bytes" + "encoding/json" + "errors" + "io" + "regexp" + "slices" + "strings" +) + +// toolName is the name CryptoScan gives itself in its reports: the tool of a +// JSON report, the SARIF driver and the tool of a CBOM. +const toolName = "CryptoScan" + +// reportReasonPrefix starts the Reason of a file left out of the scan because +// it is a report CryptoScan wrote. A report quotes the algorithm names and the +// code it found, so reading one as source reports those findings again, as +// findings in the report. +const reportReasonPrefix = "CryptoScan report: " + +// reportFileReason is the Reason of the file Config.ReportFile names. +const reportFileReason = reportReasonPrefix + "the --output file of this scan" + +// reportHeadSize is how much of a file reportFormat reads. CryptoScan writes +// the fields that name it at the start of each report, ahead of the findings. +const reportHeadSize = 8 << 10 + +// csvReportHeader is the first line of a CSV report. +const csvReportHeader = "ID,Severity,Type,Category,Algorithm,Key Size,Quantum Risk,Confidence,File,Line,Column,Match,Language,File Type,Purpose,Description,Remediation,Impact,Effort,Tags,Priority Score" + +// byteOrderMark is the UTF-8 byte order mark. It marks a file's encoding and +// is not part of its content. +var byteOrderMark = []byte("\xef\xbb\xbf") + +// textReportTitle is the line in the box at the top of a text report from +// scan, gate or pqc. +const textReportTitle = "QRAMM CryptoScan" + +// textStreamHeader ends the line a scan prints on standard output before the +// findings it lists as it finds them. +const textStreamHeader = "Scanning... (findings appear as discovered)" + +// ansiEscape matches the colour and style sequences a text report carries +// when it is written while standard output is a terminal. +var ansiEscape = regexp.MustCompile("\x1b\\[[0-9;]*m") + +// reportFormat returns the format of the CryptoScan report r holds, "JSON", +// "SARIF", "CycloneDX CBOM", "CSV" or "text", or "" when r does not start +// like one. It reads at most reportHeadSize bytes. +func reportFormat(r io.Reader) string { + head, err := io.ReadAll(io.LimitReader(r, reportHeadSize)) + if err != nil { + return "" + } + head = bytes.TrimPrefix(head, byteOrderMark) + if trimmed := bytes.TrimLeft(head, " \t\r\n"); len(trimmed) > 0 && trimmed[0] == '{' { + return jsonReportFormat(trimmed) + } + firstLine, _, _ := bytes.Cut(head, []byte("\n")) + if string(bytes.TrimSuffix(firstLine, []byte("\r"))) == csvReportHeader { + return "CSV" + } + if isTextReport(head) { + return "text" + } + return "" +} + +// isTextReport reports whether head starts like a text report: with the box +// whose third line is textReportTitle, or with textStreamHeader. +func isTextReport(head []byte) bool { + var lines []string + rest := ansiEscape.ReplaceAllString(string(head), "") + for len(lines) < 3 && rest != "" { + var line string + line, rest, _ = strings.Cut(rest, "\n") + if line = strings.TrimSpace(line); line != "" { + lines = append(lines, line) + } + } + switch { + case len(lines) > 0 && strings.HasSuffix(lines[0], textStreamHeader): + return true + case len(lines) == 3 && strings.HasPrefix(lines[0], "╔"): + return strings.Trim(lines[2], "║ ") == textReportTitle + } + return false +} + +// jsonReportFormat reads the JSON object at the start of head, which may stop +// part way through, and returns the report format that its fields name: +// tool.name for a JSON report (from scan, gate or pqc), runs[].tool.driver.name +// for SARIF, and metadata.tools[].name, with bomFormat "CycloneDX", for a CBOM. +func jsonReportFormat(head []byte) string { + format := "" + cycloneDX, cbomTool := false, false + visit := func(path []string, value any) bool { + s, _ := value.(string) + switch { + case pathIs(path, "tool", "name"): + if s == toolName { + format = "JSON" + } + case pathIs(path, "runs", "[]", "tool", "driver", "name"): + if s == toolName { + format = "SARIF" + } + case pathIs(path, "bomFormat"): + cycloneDX = s == "CycloneDX" + case pathIs(path, "metadata", "tools", "[]", "name"), + pathIs(path, "metadata", "tools", "components", "[]", "name"): + cbomTool = cbomTool || s == toolName + } + if cycloneDX && cbomTool { + format = "CycloneDX CBOM" + } + return format != "" + } + // The walk ends with an error where head stops, which is expected: what + // was read up to there decides. + _ = walkJSON(json.NewDecoder(bytes.NewReader(head)), nil, visit) + return format +} + +// pathIs reports whether path, as walkJSON builds it, is want. +func pathIs(path []string, want ...string) bool { + return slices.Equal(path, want) +} + +// errStopWalk ends walkJSON early. +var errStopWalk = errors.New("stop walking") + +// maxJSONDepth is how deep walkJSON reads. The fields jsonReportFormat looks +// for are at most five levels down. +const maxJSONDepth = 8 + +// walkJSON reads one JSON value from dec and calls visit with each scalar in +// it and the path to that scalar: the object keys, with "[]" for an array +// element. It stops at the first error, at a container nested deeper than +// maxJSONDepth, or when visit returns true. +func walkJSON(dec *json.Decoder, path []string, visit func(path []string, value any) bool) error { + tok, err := dec.Token() + if err != nil { + return err + } + delim, isContainer := tok.(json.Delim) + if !isContainer { + if visit(path, tok) { + return errStopWalk + } + return nil + } + if len(path) >= maxJSONDepth { + return errStopWalk + } + for dec.More() { + elem := "[]" + if delim == '{' { + key, err := dec.Token() + if err != nil { + return err + } + elem, _ = key.(string) + } + if err := walkJSON(dec, append(path, elem), visit); err != nil { + return err + } + } + _, err = dec.Token() // the closing '}' or ']' + return err +} diff --git a/pkg/scanner/report_test.go b/pkg/scanner/report_test.go new file mode 100644 index 0000000..591a998 --- /dev/null +++ b/pkg/scanner/report_test.go @@ -0,0 +1,284 @@ +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) +// SPDX-License-Identifier: Apache-2.0 + +package scanner + +import ( + "path/filepath" + "reflect" + "strings" + "testing" +) + +// Reports as CryptoScan writes them, cut down to one finding that names MD5. +// Read as source, each of them gives findings of its own. +const ( + prettyJSONReport = `{ + "tool": { + "name": "CryptoScan", + "version": "1.5.0" + }, + "findings": [ + { + "id": "MD5-001-2-5", + "type": "MD5 Hash Function", + "algorithm": "MD5", + "context": "h = hashlib.md5(b\"a\")" + } + ] +} +` + compactJSONReport = `{"tool":{"name":"CryptoScan","version":"1.5.0"},"findings":[{"id":"MD5-001-2-5","type":"MD5 Hash Function","algorithm":"MD5","context":"h = hashlib.md5(b\"a\")"}]}` + + sarifReport = `{ + "$schema": "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/Schemata/sarif-schema-2.1.0.json", + "version": "2.1.0", + "runs": [ + { + "tool": { + "driver": { + "name": "CryptoScan", + "version": "1.5.0", + "rules": [ + { + "id": "MD5-001-2-5", + "name": "MD5 Hash Function", + "help": { + "text": "Replace hashlib.md5() with hashlib.sha256()" + } + } + ] + } + }, + "results": [] + } + ] +} +` + + cbomReport = `{ + "bomFormat": "CycloneDX", + "specVersion": "1.6", + "serialNumber": "urn:uuid:00000000-0000-4000-8000-000000000000", + "version": 1, + "metadata": { + "timestamp": "2026-10-20T00:00:00Z", + "tools": [ + { + "vendor": "CSNP", + "name": "CryptoScan", + "version": "1.5.0" + } + ] + }, + "components": [ + { + "type": "cryptographic-asset", + "name": "MD5", + "description": "h = hashlib.md5(b\"a\")" + } + ] +} +` + + csvReport = "ID,Severity,Type,Category,Algorithm,Key Size,Quantum Risk,Confidence,File,Line,Column,Match,Language,File Type,Purpose,Description,Remediation,Impact,Effort,Tags,Priority Score\n" + + "MD5-001-2-5,MEDIUM,MD5 Hash Function,Hash Function,MD5,,PARTIAL,HIGH,a.py,2,5,hashlib.md5,python,code,,MD5 is broken,Replace hashlib.md5() with hashlib.sha256(),,Low,,40\n" + + textReport = "\n" + + "╔═══════════════════════════════════════════════════════════════╗\n" + + "║ CRYPTOGRAPHIC SCAN RESULTS ║\n" + + "║ QRAMM CryptoScan ║\n" + + "╚═══════════════════════════════════════════════════════════════╝\n" + + "\n" + + " [MEDIUM] MD5 Hash Function\n" + + " │ h = hashlib.md5(b\"a\")\n" + + " │ Fix: Replace hashlib.md5() with hashlib.sha256()\n" + + // streamedText is what a scan prints on standard output when that is not + // a terminal, as in "cryptoscan scan . > scan-output.log". + streamedText = " Scanning... (findings appear as discovered)\n" + + "\n" + + " [MEDIUM] #1 MD5 Hash Function a.py:2 [!]\n" + + " │ h = hashlib.md5(b\"a\")\n" +) + +// coloredText is textReport as the text reporter writes it with colour on, +// each box line wrapped in a bold and a reset sequence. +var coloredText = strings.NewReplacer( + "╔", "\x1b[1m╔", "╗\n", "╗\n\x1b[0m", + "║ ", "\x1b[1m║ ", "║\n", "║\n\x1b[0m", +).Replace(textReport) + +func TestReportFormat(t *testing.T) { + tests := []struct { + name string + content string + want string + }{ + {"pretty JSON report", prettyJSONReport, "JSON"}, + {"compact JSON report", compactJSONReport, "JSON"}, + {"pqc JSON view", `{"tool":{"name":"CryptoScan","version":"1.5.0"},"migrationScore":null,"pqcFindings":[]}`, "JSON"}, + {"JSON report with a byte order mark", "\xef\xbb\xbf" + prettyJSONReport, "JSON"}, + {"SARIF report", sarifReport, "SARIF"}, + {"CBOM", cbomReport, "CycloneDX CBOM"}, + {"CBOM listing tools as components", + `{"bomFormat": "CycloneDX", "metadata": {"tools": {"components": [{"type": "application", "name": "CryptoScan"}]}}}`, + "CycloneDX CBOM"}, + {"CSV report", csvReport, "CSV"}, + {"CSV report with CRLF line ends", strings.ReplaceAll(csvReport, "\n", "\r\n"), "CSV"}, + {"CSV report with no findings", strings.SplitAfter(csvReport, "\n")[0], "CSV"}, + {"text report", textReport, "text"}, + {"text report in colour", coloredText, "text"}, + {"pqc text view", strings.Replace(textReport, "CRYPTOGRAPHIC SCAN RESULTS", "POST-QUANTUM READINESS ", 1), "text"}, + {"streamed scan output", streamedText, "text"}, + + // Files that are not CryptoScan reports are read as before. + {"SARIF from another tool", + strings.Replace(sarifReport, `"name": "CryptoScan"`, `"name": "OtherScanner"`, 1), ""}, + {"CycloneDX SBOM from another tool", + strings.Replace(cbomReport, `"name": "CryptoScan"`, `"name": "cyclonedx-gomod"`, 1), ""}, + {"CryptoScan named as a tool outside a CycloneDX document", + `{"metadata": {"tools": [{"name": "CryptoScan"}]}}`, ""}, + {"a package named CryptoScan", `{"name": "CryptoScan", "version": "1.0.0", "dependencies": {}}`, ""}, + {"another tool", `{"tool": {"name": "OtherScanner"}, "findings": []}`, ""}, + {"tool.name below the top level", `{"settings": {"tool": {"name": "CryptoScan"}}}`, ""}, + {"a JSON array", `[{"tool": {"name": "CryptoScan"}}]`, ""}, + {"another CSV", "ID,Severity,Type\n1,HIGH,md5\n", ""}, + {"a box with another title", strings.Replace(textReport, "QRAMM CryptoScan", "Release checklist", 1), ""}, + {"text naming the report title", "Notes\n\nThe report title reads QRAMM CryptoScan.\nmd5 is broken\n", ""}, + {"source code", "import hashlib\nh = hashlib.md5(b\"a\")\n", ""}, + {"an empty file", "", ""}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := reportFormat(strings.NewReader(tt.content)); got != tt.want { + t.Errorf("reportFormat = %q, want %q", got, tt.want) + } + }) + } +} + +// Only the start of a file is read. A report names its tool before its +// findings, so one longer than the part read is still recognised, and a JSON +// file nested deeper than the fields looked for ends the walk without them. +func TestReportFormatReadsTheStartOfTheFile(t *testing.T) { + long := `{"tool":{"name":"CryptoScan","version":"1.5.0"},"findings":[` + + strings.Repeat(`{"id":"MD5-001-2-5","algorithm":"MD5"},`, 2*reportHeadSize/38) + + `{"id":"MD5-001-2-5","algorithm":"MD5"}]}` + if len(long) <= reportHeadSize { + t.Fatalf("fixture is %d bytes, want more than %d", len(long), reportHeadSize) + } + if got := reportFormat(strings.NewReader(long)); got != "JSON" { + t.Errorf("reportFormat on a %d-byte JSON report = %q, want %q", len(long), got, "JSON") + } + + nested := `{"a":` + strings.Repeat("[", 4000) + strings.Repeat("]", 4000) + `,"tool":{"name":"CryptoScan"}}` + if got := reportFormat(strings.NewReader(nested)); got != "" { + t.Errorf("reportFormat on deeply nested JSON = %q, want \"\"", got) + } +} + +// The reproduction from the report: a report CryptoScan wrote into the tree +// was read as source on the next scan, and every algorithm it named became a +// finding. Each report is now listed as not scanned, and only the source file +// gives findings. +func TestScanLeavesOutCryptoScanReports(t *testing.T) { + dir := t.TempDir() + source := filepath.Join(dir, "a.py") + writeFixture(t, source, md5Source) + reports := map[string]string{ + "findings.json": prettyJSONReport, + "baseline.json": compactJSONReport, + "results.sarif": sarifReport, + "crypto-bom.json": cbomReport, + "findings.csv": csvReport, + "scan-output.log": streamedText, + "scan-report.log": coloredText, + } + for name, content := range reports { + writeFixture(t, filepath.Join(dir, "reports", name), content) + } + + results := scanTarget(t, dir) + + for _, f := range results.Findings { + if f.File != source { + t.Errorf("finding %s in %s: only %s is source", f.ID, f.File, source) + } + } + if len(md5Findings(results)) != 1 || results.FilesScanned != 1 { + t.Errorf("md5Findings = %d, filesScanned = %d; want 1 and 1", + len(md5Findings(results)), results.FilesScanned) + } + want := []SkippedFile{ + {File: filepath.Join(dir, "reports", "baseline.json"), Reason: "CryptoScan report: JSON"}, + {File: filepath.Join(dir, "reports", "crypto-bom.json"), Reason: "CryptoScan report: CycloneDX CBOM"}, + {File: filepath.Join(dir, "reports", "findings.csv"), Reason: "CryptoScan report: CSV"}, + {File: filepath.Join(dir, "reports", "findings.json"), Reason: "CryptoScan report: JSON"}, + {File: filepath.Join(dir, "reports", "results.sarif"), Reason: "CryptoScan report: SARIF"}, + {File: filepath.Join(dir, "reports", "scan-output.log"), Reason: "CryptoScan report: text"}, + {File: filepath.Join(dir, "reports", "scan-report.log"), Reason: "CryptoScan report: text"}, + } + if !reflect.DeepEqual(results.SkippedFiles, want) { + t.Errorf("skippedFiles = %+v\nwant %+v", results.SkippedFiles, want) + } + for _, sk := range results.SkippedFiles { + if !sk.IsCryptoScanReport() || sk.IsUnreadManifest() { + t.Errorf("%s: IsCryptoScanReport = %v, IsUnreadManifest = %v; want true and false", + sk.File, sk.IsCryptoScanReport(), sk.IsUnreadManifest()) + } + } +} + +// The file the scan's report goes to is left out whatever it holds, since +// the report replaces it. Without ReportFile, the same file is scanned. +func TestScanLeavesOutTheReportFile(t *testing.T) { + dir := t.TempDir() + report := filepath.Join(dir, "scan-report") + writeFixture(t, report, md5Source) + + control := scanTarget(t, dir) + if len(md5Findings(control)) != 1 || len(control.SkippedFiles) != 0 { + t.Fatalf("control scan: md5Findings = %d, skippedFiles = %+v; want 1 and none", + len(md5Findings(control)), control.SkippedFiles) + } + + results, err := New(Config{Target: dir, MinSeverity: SeverityInfo, ReportFile: report}).Scan() + if err != nil { + t.Fatal(err) + } + want := []SkippedFile{{File: report, Reason: "CryptoScan report: the --output file of this scan"}} + if !reflect.DeepEqual(results.SkippedFiles, want) { + t.Errorf("skippedFiles = %+v, want %+v", results.SkippedFiles, want) + } + if len(results.Findings) != 0 || results.FilesScanned != 0 { + t.Errorf("findings = %d, filesScanned = %d; want 0 and 0", len(results.Findings), results.FilesScanned) + } + + // A report file that does not exist yet leaves the scan as it was. + missing, err := New(Config{Target: dir, MinSeverity: SeverityInfo, ReportFile: filepath.Join(dir, "new-report")}).Scan() + if err != nil { + t.Fatal(err) + } + if len(md5Findings(missing)) != 1 || len(missing.SkippedFiles) != 0 { + t.Errorf("scan with a new report file: md5Findings = %d, skippedFiles = %+v; want 1 and none", + len(md5Findings(missing)), missing.SkippedFiles) + } +} + +func TestIsCryptoScanReport(t *testing.T) { + for _, tt := range []struct { + reason string + want bool + }{ + {"CryptoScan report: SARIF", true}, + {"CryptoScan report: the --output file of this scan", true}, + {"minified: the file name ends in .min.js", false}, + {"minified or generated: its lines average 674 bytes", false}, + {"dependency manifest: the scanner does not read this format", false}, + } { + if got := (SkippedFile{File: "f", Reason: tt.reason}).IsCryptoScanReport(); got != tt.want { + t.Errorf("IsCryptoScanReport for %q = %v, want %v", tt.reason, got, tt.want) + } + } +} diff --git a/pkg/scanner/scanner.go b/pkg/scanner/scanner.go index b0f291f..5778163 100644 --- a/pkg/scanner/scanner.go +++ b/pkg/scanner/scanner.go @@ -77,6 +77,11 @@ type Config struct { // that what each setting removes can be counted and reported. Checking // them separately does not change which findings are reported. ConfigFile *ConfigFileRules + + // ReportFile is the file the scan's report is written to, if any. A file + // already at that path, which the scan would otherwise read, is left out + // and listed as skipped, whatever it holds, since the report replaces it. + ReportFile string } // Where a configuration file was found, as recorded in ConfigReport.Source. @@ -189,9 +194,10 @@ type Results struct { UnanalyzedFiles []UnanalyzedFile `json:"unanalyzedFiles"` // SkippedFiles lists files left out of the scan because they look - // minified or generated, or because they are dependency manifests the - // scanner has no reader for. Cryptography in them is not in the report, - // so they are named rather than dropped without a word. + // minified or generated, because they are dependency manifests the + // scanner has no reader for, or because they are reports CryptoScan + // wrote. Cryptography in them is not in the report, so they are named + // rather than dropped without a word. SkippedFiles []SkippedFile `json:"skippedFiles"` // SkippedDependencyDirs lists the node_modules directories the scan did @@ -230,8 +236,9 @@ func (u UnanalyzedFile) IsInternalError() bool { } // SkippedFile is a file the scanner did not search for cryptography because -// its name or its line lengths mark it as minified or generated code, or -// because it is a dependency manifest the scanner has no reader for. +// its name or its line lengths mark it as minified or generated code, because +// it is a dependency manifest the scanner has no reader for, or because it is +// a report CryptoScan wrote. type SkippedFile struct { File string `json:"file"` Reason string `json:"reason"` @@ -248,6 +255,12 @@ func (sk SkippedFile) IsUnreadManifest() bool { return sk.Reason == unreadManifestReason } +// IsCryptoScanReport reports whether the file was skipped because it is a +// report CryptoScan wrote, or the file the scan's own report replaces. +func (sk SkippedFile) IsCryptoScanReport() bool { + return strings.HasPrefix(sk.Reason, reportReasonPrefix) +} + // MetadataScanRoot is the Results.Metadata entry holding the directory the // scan's file paths are under. ScanTarget is the target as it was given, such // as "." or a repository URL, so a saved report needs this to turn each @@ -334,13 +347,18 @@ type Scanner struct { // unanalyzed holds files whose analysis stopped on an internal error. unanalyzed []UnanalyzedFile - // skipped holds files left out because they look minified or generated. + // skipped holds files left out because they look minified or generated, + // are dependency manifests the scanner does not read, or are reports + // CryptoScan wrote. skipped []SkippedFile // dependencyDirs holds the node_modules directories the walk did not // enter, relative to the scan root. dependencyDirs []string + // reportFile is the file at Config.ReportFile when the scan began, or nil. + reportFile os.FileInfo + // configWithheld holds, per configuration-file setting, the matches that // setting kept out of a report that would otherwise have contained them. // configSkipped counts the paths ignore.files kept out of the scan. @@ -390,6 +408,12 @@ func (s *Scanner) Scan() (*Results, error) { return nil, fmt.Errorf("cannot access target: %w", err) } + if s.config.ReportFile != "" { + if reportInfo, err := os.Stat(s.config.ReportFile); err == nil { + s.reportFile = reportInfo + } + } + if info.IsDir() { s.root = target err = s.scanDirectory(target) @@ -482,7 +506,7 @@ func (s *Scanner) Scan() (*Results, error) { // Build results results := &Results{ - Tool: ToolInfo{Name: "CryptoScan", Version: version.Get()}, + Tool: ToolInfo{Name: toolName, Version: version.Get()}, Findings: s.findings, MigrationScore: migrationScore, ScanRoot: root, @@ -974,6 +998,17 @@ func (s *Scanner) scanFile(path string) error { } fileSize := fileInfo.Size() + // A report CryptoScan wrote is not source, and it is not read as a + // dependency manifest either. + if s.reportFile != nil && os.SameFile(s.reportFile, fileInfo) { + s.skip(path, reportFileReason) + return nil + } + if format := reportFormat(file); format != "" { + s.skip(path, reportReasonPrefix+format) + return nil + } + // Analyze file context fileCtx := s.analyzeFile(path) @@ -1764,8 +1799,8 @@ func betterRepresentative(a, b Finding) bool { } // skip records a file left out of the scan because it looks minified or -// generated, or because it is a dependency manifest the scanner has no reader -// for, with the reason given. +// generated, because it is a dependency manifest the scanner has no reader +// for, or because it is a report CryptoScan wrote, with the reason given. func (s *Scanner) skip(path, reason string) { s.mu.Lock() s.skipped = append(s.skipped, SkippedFile{File: path, Reason: reason})