From 8b5b8a000bb305e1166e0b7da08f8e9132a26bea Mon Sep 17 00:00:00 2001 From: Abdel Fane Date: Mon, 27 Jul 2026 13:55:17 -0600 Subject: [PATCH] Scan the requirements.txt family and attribute CBOM algorithms to dependencies Manifest discovery matched the exact filename requirements.txt, so the requirements-dev.txt and requirements-prod.txt split that most Python projects use was never scanned. Match the requirements*.txt family and the requirements/*.txt directory layout instead. CBOM output emitted only the algorithm, carrying the dependency's version but never its name, so a component reading {"name": "RSA", "version": "1.3.2"} could not be traced to the library that provided it. Emit each dependency as its own library component with a bom-ref and link algorithms to it through the CycloneDX dependencies graph. Reproducing those two also surfaced a silent false negative. pyproject.toml and Pipfile were advertised as supported, but parsing fell through to the requirements.txt line parser behind a TODO. A pyproject.toml produced dependencies named after TOML keys, and because the real packages were never identified, a project depending on cryptography reported no cryptographic usage at all. Both formats now have real TOML parsers covering PEP 621, Poetry and Pipfile layouts. Also fixed: generateUUID returned a hardcoded all-zero UUID, so every CBOM shared a serial number that did not match the CycloneDX urn:uuid pattern; unresolved Maven properties such as ${java-jwt.version} and version ranges such as >=2.0 were emitted where CycloneDX expects a concrete version; and primitive values were outside the CycloneDX enum, failing schema validation for the whole document. Verified the emitted CBOM against the official CycloneDX 1.6 schema. Fixes #1 Fixes #2 --- CHANGELOG.md | 49 +++++ README.md | 2 +- docs/testing/release-smoke.md | 55 +++++ go.mod | 1 + go.sum | 2 + internal/manifest/parser.go | 27 ++- internal/manifest/python.go | 206 +++++++++++++++++- internal/manifest/python_formats_test.go | 258 +++++++++++++++++++++++ internal/manifest/workspace.go | 83 +++++--- pkg/output/cbom.go | 230 +++++++++++++++++--- pkg/output/cbom_attribution_test.go | 228 ++++++++++++++++++++ 11 files changed, 1072 insertions(+), 69 deletions(-) create mode 100644 docs/testing/release-smoke.md create mode 100644 internal/manifest/python_formats_test.go create mode 100644 pkg/output/cbom_attribution_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index d5ab4a4..88d16ec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,55 @@ All notable changes to QRAMM CryptoDeps will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [1.3.0] - 2026-07-27 + +Fixes both open community issues, plus a silent false negative found while +reproducing them. + +### Fixed + +- **`requirements-*.txt` files were skipped** + ([#1](https://github.com/csnp/cryptodeps/issues/1)). Manifest discovery + matched the exact filename `requirements.txt`, so the + `requirements-dev.txt` and `requirements-prod.txt` split that most Python + projects use was never scanned. The `requirements*.txt` family and the + `requirements/*.txt` directory layout are now discovered and parsed. +- **CBOM did not identify which dependency an algorithm came from** + ([#2](https://github.com/csnp/cryptodeps/issues/2)). Output emitted only the + algorithm, carrying the dependency's version but never its name, so a + component reading `{"name": "RSA", "version": "1.3.2"}` was unattributable. + Each dependency is now emitted as its own `library` component with a + `bom-ref`, and the CycloneDX `dependencies` graph links each algorithm to the + library that provides it. +- **`pyproject.toml` and `Pipfile` produced fabricated dependencies.** Both were + advertised as supported, but parsing fell through to the `requirements.txt` + line parser behind a `TODO`. A `pyproject.toml` yielded entries named after + TOML keys (`name`, `dependencies`, `requires-python`), and because the real + packages were never identified, a project depending on `cryptography` reported + "No cryptographic usage detected". Both formats now have real TOML parsers + covering PEP 621, Poetry, and Pipfile layouts. +- **Every CBOM shared one serial number.** `generateUUID` returned a hardcoded + all-zero UUID, which also failed the CycloneDX `urn:uuid` pattern. Serial + numbers are now random version 4 UUIDs. +- **Unresolved build properties were emitted as versions.** A Maven dependency + declared as `${java-jwt.version}` appeared in the CBOM with that literal as + its version. Version ranges such as `>=2.0` were likewise emitted where + CycloneDX expects a concrete version. Only pinned versions are now reported as + versions; the declared constraint is preserved in the component description. +- **CBOM `primitive` values were outside the CycloneDX enum**, which failed + schema validation for the whole document. Categories now map onto the + permitted enum, resolving `encryption` by algorithm where possible. + +### Added + +- Regression tests for manifest discovery, the three Python formats, PEP 508 + requirement parsing, CBOM dependency attribution, version resolution, serial + number uniqueness, and primitive enum conformance. + +### Dependencies + +- Added `github.com/BurntSushi/toml` for pyproject.toml and Pipfile parsing. + ## [1.2.1] - 2025-12-27 ### Added diff --git a/README.md b/README.md index dba8934..59a1123 100644 --- a/README.md +++ b/README.md @@ -150,7 +150,7 @@ CryptoDeps goes beyond simple dependency scanning by analyzing your code's call |-----------|----------------| | Go | `go.mod`, `go.sum` | | npm | `package.json`, `package-lock.json` | -| Python | `requirements.txt`, `pyproject.toml`, `Pipfile` | +| Python | `requirements.txt` and the `requirements-*.txt` family, `requirements/*.txt`, `pyproject.toml` (PEP 621 and Poetry), `Pipfile` | | Maven | `pom.xml` | ### Workspace & Monorepo Support diff --git a/docs/testing/release-smoke.md b/docs/testing/release-smoke.md new file mode 100644 index 0000000..a84fea7 --- /dev/null +++ b/docs/testing/release-smoke.md @@ -0,0 +1,55 @@ +# Release Smoke Test: qramm-cryptodeps + +Manual pre-release walkthrough. Run this before every deploy or publish. +Use real keyboard and mouse input, not synthetic events. Record the actual +output, not a summary. + +## 1. Build and start clean + +- [ ] Fresh install succeeds (`npm ci` or equivalent) with no errors. +- [ ] Build succeeds (`npm run build`). +- [ ] App starts locally (`npm run dev`) with no console errors. + +## 2. Core user paths + +List the three most important things a real visitor does on this site or +with this tool, then walk each one by hand. + +- [ ] Path 1: +- [ ] Path 2: +- [ ] Path 3: + +## 3. Accessibility and audience fit + +CSNP serves non-technical and vulnerable users. Verify the experience holds +for them. + +- [ ] Plain language: no unexplained jargon in user-facing copy. +- [ ] Keyboard navigable: every interactive element reachable by Tab. +- [ ] Screen-reader labels present on buttons, links, and form fields. +- [ ] Color contrast meets WCAG AA. + +## 4. Responsive parity + +- [ ] Mobile (375px): nav, menus, and layout all work. +- [ ] Desktop (1280px): no overflow or broken grids. +- [ ] Dark mode (if supported) renders correctly. + +## 5. Data and links + +- [ ] Every download link resolves (no 404s). +- [ ] Any displayed number or statistic traces to a real source. +- [ ] No placeholder or fabricated content shipped. + +## 6. Security + +- [ ] No secrets in the bundle or committed files. +- [ ] `.env` and credential files are gitignored and excluded. +- [ ] Forms validate and sanitize input. + +## Result + +- Date: +- Tester: +- Verdict: PASS / FAIL +- Notes: diff --git a/go.mod b/go.mod index f2f4309..8143673 100644 --- a/go.mod +++ b/go.mod @@ -3,6 +3,7 @@ module github.com/csnp/qramm-cryptodeps go 1.21 require ( + github.com/BurntSushi/toml v1.6.0 github.com/spf13/cobra v1.8.0 gopkg.in/yaml.v3 v3.0.1 ) diff --git a/go.sum b/go.sum index 3926859..bb77c0a 100644 --- a/go.sum +++ b/go.sum @@ -1,3 +1,5 @@ +github.com/BurntSushi/toml v1.6.0 h1:dRaEfpa2VI55EwlIW72hMRHdWouJeRF7TPYhI+AUQjk= +github.com/BurntSushi/toml v1.6.0/go.mod h1:ukJfTF/6rtPPRCnwkur4qwRxa8vTRFBF0uk2lLoLwho= github.com/cpuguy83/go-md2man/v2 v2.0.3/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= diff --git a/internal/manifest/parser.go b/internal/manifest/parser.go index 01839cb..062e69f 100644 --- a/internal/manifest/parser.go +++ b/internal/manifest/parser.go @@ -104,9 +104,30 @@ func getParser(filename string) (Parser, error) { return &PythonParser{}, nil case "pom.xml": return &MavenParser{}, nil - default: - return nil, fmt.Errorf("unsupported manifest file: %s", filename) } + + // The requirements.txt family carries no fixed name. Matching only the exact + // filename silently skipped requirements-dev.txt, requirements-prod.txt and + // the requirements/*.txt layout. + if isRequirementsFile(filename) { + return &PythonParser{}, nil + } + + return nil, fmt.Errorf("unsupported manifest file: %s", filename) +} + +// getParserForPath selects a parser using the whole path, so that layouts whose +// meaning depends on the parent directory resolve correctly. A pip requirements +// file at requirements/base.txt is named base.txt and is indistinguishable from +// any other text file by name alone. +func getParserForPath(path string) (Parser, error) { + if parser, err := getParser(filepath.Base(path)); err == nil { + return parser, nil + } + if isManifestPath(path) && strings.EqualFold(filepath.Ext(path), ".txt") { + return &PythonParser{}, nil + } + return nil, fmt.Errorf("unsupported manifest file: %s", path) } // SupportedManifests returns a list of supported manifest filenames. @@ -152,7 +173,7 @@ func DetectAndParseAll(path string) ([]*Manifest, error) { var parseErrors []string for _, manifestPath := range manifestPaths { - parser, err := getParser(filepath.Base(manifestPath)) + parser, err := getParserForPath(manifestPath) if err != nil { // Skip unsupported files silently (they might have been picked up by glob) continue diff --git a/internal/manifest/python.go b/internal/manifest/python.go index bf754fa..02724bb 100644 --- a/internal/manifest/python.go +++ b/internal/manifest/python.go @@ -5,10 +5,14 @@ package manifest import ( "bufio" + "fmt" "os" + "path/filepath" "regexp" + "sort" "strings" + "github.com/BurntSushi/toml" "github.com/csnp/qramm-cryptodeps/pkg/types" ) @@ -25,16 +29,208 @@ func (p *PythonParser) Filenames() []string { return []string{"requirements.txt", "pyproject.toml", "Pipfile"} } -// Parse parses a Python requirements file and returns the list of dependencies. +// Parse parses a Python dependency file and returns the list of dependencies. +// +// Each format is parsed by its own reader. Falling back to the requirements.txt +// line parser for TOML files produced entries named after TOML keys, so a +// pyproject.toml declaring cryptography as a dependency yielded a dependency +// literally named "dependencies" and the real package was never analyzed. func (p *PythonParser) Parse(path string) ([]types.Dependency, error) { - // Determine file type - if strings.HasSuffix(path, "requirements.txt") { + switch base := strings.ToLower(filepath.Base(path)); { + case base == "pyproject.toml": + return p.parsePyProject(path) + case base == "pipfile": + return p.parsePipfile(path) + default: return p.parseRequirementsTxt(path) } - // TODO: Implement pyproject.toml and Pipfile parsing - return p.parseRequirementsTxt(path) } +// pyProject models the dependency-bearing sections of pyproject.toml, covering +// both the PEP 621 standard layout and Poetry's own layout. +type pyProject struct { + Project struct { + Dependencies []string `toml:"dependencies"` + OptionalDependencies map[string][]string `toml:"optional-dependencies"` + } `toml:"project"` + Tool struct { + Poetry struct { + Dependencies map[string]any `toml:"dependencies"` + DevDependencies map[string]any `toml:"dev-dependencies"` + Group map[string]struct { + Dependencies map[string]any `toml:"dependencies"` + } `toml:"group"` + } `toml:"poetry"` + } `toml:"tool"` +} + +// parsePyProject reads PEP 621 and Poetry dependency declarations. +func (p *PythonParser) parsePyProject(path string) ([]types.Dependency, error) { + var doc pyProject + if _, err := toml.DecodeFile(path, &doc); err != nil { + return nil, fmt.Errorf("parse pyproject.toml: %w", err) + } + + var deps []types.Dependency + seen := make(map[string]bool) + + add := func(name, version string, direct bool) { + name = normalizePyPIName(name) + if name == "" || seen[name] { + return + } + seen[name] = true + deps = append(deps, types.Dependency{ + Name: name, + Version: strings.TrimSpace(version), + Ecosystem: types.EcosystemPyPI, + Direct: direct, + }) + } + + // PEP 621: dependencies are PEP 508 requirement strings. + for _, spec := range doc.Project.Dependencies { + name, version := splitRequirement(spec) + add(name, version, true) + } + for _, group := range doc.Project.OptionalDependencies { + for _, spec := range group { + name, version := splitRequirement(spec) + add(name, version, false) + } + } + + // Poetry: dependencies are a table keyed by package name, whose value is + // either a version string or an inline table carrying a version field. + poetry := doc.Tool.Poetry + for name, constraint := range poetry.Dependencies { + if strings.EqualFold(name, "python") { + continue // the interpreter requirement, not a package + } + add(name, poetryVersion(constraint), true) + } + for name, constraint := range poetry.DevDependencies { + add(name, poetryVersion(constraint), false) + } + for _, group := range poetry.Group { + for name, constraint := range group.Dependencies { + add(name, poetryVersion(constraint), false) + } + } + + sort.Slice(deps, func(i, j int) bool { return deps[i].Name < deps[j].Name }) + return deps, nil +} + +// pipfile models the dependency sections of a Pipfile, which is also TOML. +type pipfile struct { + Packages map[string]any `toml:"packages"` + DevPackages map[string]any `toml:"dev-packages"` +} + +// parsePipfile reads a Pipfile. +func (p *PythonParser) parsePipfile(path string) ([]types.Dependency, error) { + var doc pipfile + if _, err := toml.DecodeFile(path, &doc); err != nil { + return nil, fmt.Errorf("parse Pipfile: %w", err) + } + + var deps []types.Dependency + seen := make(map[string]bool) + + add := func(name string, constraint any, direct bool) { + name = normalizePyPIName(name) + if name == "" || seen[name] { + return + } + seen[name] = true + version := poetryVersion(constraint) + if version == "*" { + version = "" // Pipfile wildcard means unpinned + } + deps = append(deps, types.Dependency{ + Name: name, + Version: strings.TrimSpace(version), + Ecosystem: types.EcosystemPyPI, + Direct: direct, + }) + } + + for name, constraint := range doc.Packages { + add(name, constraint, true) + } + for name, constraint := range doc.DevPackages { + add(name, constraint, false) + } + + sort.Slice(deps, func(i, j int) bool { return deps[i].Name < deps[j].Name }) + return deps, nil +} + +// poetryVersion extracts a version constraint from a Poetry or Pipfile value, +// which may be a bare string or an inline table such as +// {version = "^1.0", optional = true}. +func poetryVersion(constraint any) string { + switch v := constraint.(type) { + case string: + return v + case map[string]any: + if version, ok := v["version"].(string); ok { + return version + } + // Git and path dependencies carry no version. + return "" + default: + return "" + } +} + +// requirementNamePattern captures the package name and the remainder of a +// PEP 508 requirement string such as "cryptography[ssh]>=41.0.0; python_version<'4'". +var requirementNamePattern = regexp.MustCompile(`^([A-Za-z0-9][A-Za-z0-9._-]*)`) + +// splitRequirement separates a PEP 508 requirement into name and version +// constraint, discarding extras and environment markers. +func splitRequirement(spec string) (name, version string) { + spec = strings.TrimSpace(spec) + if spec == "" { + return "", "" + } + + // Drop environment markers, which follow a semicolon. + if idx := strings.Index(spec, ";"); idx != -1 { + spec = strings.TrimSpace(spec[:idx]) + } + + match := requirementNamePattern.FindStringSubmatch(spec) + if match == nil { + return "", "" + } + name = match[1] + + rest := strings.TrimSpace(spec[len(name):]) + // Drop extras such as [ssh]. + if strings.HasPrefix(rest, "[") { + if idx := strings.Index(rest, "]"); idx != -1 { + rest = strings.TrimSpace(rest[idx+1:]) + } + } + return name, strings.TrimSpace(rest) +} + +// normalizePyPIName applies PEP 503 normalization so that the same package +// written as Foo.Bar, foo-bar or foo_bar resolves to one entry. +func normalizePyPIName(name string) string { + name = strings.TrimSpace(name) + if name == "" { + return "" + } + name = pypiSeparatorPattern.ReplaceAllString(name, "-") + return strings.ToLower(name) +} + +var pypiSeparatorPattern = regexp.MustCompile(`[-_.]+`) + // parseRequirementsTxt parses a requirements.txt file. func (p *PythonParser) parseRequirementsTxt(path string) ([]types.Dependency, error) { file, err := os.Open(path) diff --git a/internal/manifest/python_formats_test.go b/internal/manifest/python_formats_test.go new file mode 100644 index 0000000..ce77c57 --- /dev/null +++ b/internal/manifest/python_formats_test.go @@ -0,0 +1,258 @@ +// Copyright 2024-2025 CSNP (csnp.org) +// SPDX-License-Identifier: Apache-2.0 + +package manifest + +import ( + "os" + "path/filepath" + "testing" +) + +func writeFile(t *testing.T, path, content string) { + t.Helper() + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatalf("write %s: %v", path, err) + } +} + +// TestRequirementsFamilyIsRecognized is the regression guard for issue #1. +// Matching only the exact name "requirements.txt" silently skipped the +// requirements-dev.txt and requirements-prod.txt files that most Python +// projects actually use. +func TestRequirementsFamilyIsRecognized(t *testing.T) { + tests := []struct { + name string + want bool + }{ + {"requirements.txt", true}, + {"requirements-dev.txt", true}, + {"requirements-prod.txt", true}, + {"requirements_test.txt", true}, + {"requirements.dev.txt", true}, + {"REQUIREMENTS-DEV.TXT", true}, + {"notrequirements.txt", false}, + {"requirements.md", false}, + {"readme.txt", false}, + {"requirements", false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := isRequirementsFile(tt.name); got != tt.want { + t.Errorf("isRequirementsFile(%q) = %v, want %v", tt.name, got, tt.want) + } + }) + } +} + +// TestRequirementsDirectoryLayoutIsRecognized covers the requirements/base.txt +// layout, where the filename alone carries no hint of its contents. +func TestRequirementsDirectoryLayoutIsRecognized(t *testing.T) { + if !isManifestPath(filepath.Join("proj", "requirements", "base.txt")) { + t.Error("requirements/base.txt not recognized as a manifest") + } + if !isManifestPath(filepath.Join("proj", "requirements", "dev.txt")) { + t.Error("requirements/dev.txt not recognized as a manifest") + } + if isManifestPath(filepath.Join("proj", "docs", "notes.txt")) { + t.Error("docs/notes.txt wrongly recognized as a manifest") + } +} + +// TestDiscoverFindsRequirementsFamily walks a real tree, which is what the +// reporter was doing. +func TestDiscoverFindsRequirementsFamily(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "requirements.txt"), "requests==2.31.0\n") + writeFile(t, filepath.Join(dir, "requirements-dev.txt"), "cryptography==41.0.0\n") + writeFile(t, filepath.Join(dir, "requirements-prod.txt"), "pyjwt>=2.0\n") + writeFile(t, filepath.Join(dir, "requirements", "base.txt"), "pycryptodome==3.19.0\n") + + found, err := DiscoverManifests(dir) + if err != nil { + t.Fatalf("discover: %v", err) + } + + seen := make(map[string]bool) + for _, path := range found { + seen[filepath.Base(path)] = true + } + for _, want := range []string{ + "requirements.txt", "requirements-dev.txt", "requirements-prod.txt", "base.txt", + } { + if !seen[want] { + t.Errorf("%s was not discovered; found %v", want, found) + } + } +} + +// TestPyProjectIsNotParsedAsRequirements guards a silent data-integrity defect. +// pyproject.toml was advertised as supported but fell through to the +// requirements.txt line parser, which produced dependencies named after TOML +// keys. A project depending on cryptography reported no cryptographic usage at +// all, which is a false negative on this tool's core purpose. +func TestPyProjectIsNotParsedAsRequirements(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "pyproject.toml") + writeFile(t, path, ` +[project] +name = "demo" +requires-python = ">=3.11" +dependencies = ["cryptography==41.0.0", "pyjwt>=2.0"] + +[project.optional-dependencies] +dev = ["pytest>=7.0"] + +[build-system] +requires = ["setuptools>=61"] +`) + + deps, err := (&PythonParser{}).Parse(path) + if err != nil { + t.Fatalf("parse: %v", err) + } + + byName := make(map[string]string) + for _, d := range deps { + byName[d.Name] = d.Version + } + + // The real dependencies must be present. + if _, ok := byName["cryptography"]; !ok { + t.Errorf("cryptography not parsed; got %v", byName) + } + if _, ok := byName["pyjwt"]; !ok { + t.Errorf("pyjwt not parsed; got %v", byName) + } + if _, ok := byName["pytest"]; !ok { + t.Errorf("optional dependency pytest not parsed; got %v", byName) + } + + // TOML keys must not appear as dependencies. + for _, bogus := range []string{"name", "dependencies", "requires-python", "requires", "build-system"} { + if _, ok := byName[bogus]; ok { + t.Errorf("TOML key %q was parsed as a dependency; got %v", bogus, byName) + } + } + + // setuptools is a build requirement, not a project dependency. + if _, ok := byName["setuptools"]; ok { + t.Errorf("build-system requirement leaked into dependencies; got %v", byName) + } +} + +// TestPyProjectPoetryLayout covers Poetry projects, which declare dependencies +// as a TOML table rather than PEP 508 strings. +func TestPyProjectPoetryLayout(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "pyproject.toml") + writeFile(t, path, ` +[tool.poetry] +name = "demo" + +[tool.poetry.dependencies] +python = "^3.11" +cryptography = "41.0.0" +requests = {version = "^2.31", optional = true} + +[tool.poetry.group.dev.dependencies] +pytest = "^7.0" +`) + + deps, err := (&PythonParser{}).Parse(path) + if err != nil { + t.Fatalf("parse: %v", err) + } + + byName := make(map[string]string) + for _, d := range deps { + byName[d.Name] = d.Version + } + + if got := byName["cryptography"]; got != "41.0.0" { + t.Errorf("cryptography version = %q, want 41.0.0; got all %v", got, byName) + } + if got, ok := byName["requests"]; !ok || got != "^2.31" { + t.Errorf("requests from inline table = %q (present=%v), want ^2.31", got, ok) + } + if _, ok := byName["pytest"]; !ok { + t.Errorf("dev group dependency pytest missing; got %v", byName) + } + // The interpreter constraint is not a package. + if _, ok := byName["python"]; ok { + t.Errorf("python interpreter constraint parsed as a dependency; got %v", byName) + } +} + +// TestPipfileIsParsedAsTOML covers Pipfile, which was also falling through to +// the requirements parser. +func TestPipfileIsParsedAsTOML(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "Pipfile") + writeFile(t, path, ` +[[source]] +url = "https://pypi.org/simple" + +[packages] +cryptography = "==41.0.0" +requests = "*" + +[dev-packages] +pytest = ">=7.0" +`) + + deps, err := (&PythonParser{}).Parse(path) + if err != nil { + t.Fatalf("parse: %v", err) + } + + byName := make(map[string]string) + for _, d := range deps { + byName[d.Name] = d.Version + } + + if got := byName["cryptography"]; got != "==41.0.0" { + t.Errorf("cryptography = %q, want ==41.0.0; got all %v", got, byName) + } + if got, ok := byName["requests"]; !ok || got != "" { + t.Errorf("wildcard requests = %q (present=%v), want an empty version", got, ok) + } + if _, ok := byName["pytest"]; !ok { + t.Errorf("dev package pytest missing; got %v", byName) + } + if _, ok := byName["url"]; ok { + t.Errorf("source url parsed as a dependency; got %v", byName) + } +} + +// TestSplitRequirementHandlesPEP508 checks extras and environment markers are +// stripped rather than becoming part of the package name. +func TestSplitRequirementHandlesPEP508(t *testing.T) { + tests := []struct { + spec string + wantName string + wantVersion string + }{ + {"cryptography==41.0.0", "cryptography", "==41.0.0"}, + {"cryptography[ssh]>=41.0.0", "cryptography", ">=41.0.0"}, + {`pyjwt>=2.0; python_version<"4"`, "pyjwt", ">=2.0"}, + {"requests", "requests", ""}, + {" flask == 3.0 ", "flask", "== 3.0"}, + } + + for _, tt := range tests { + t.Run(tt.spec, func(t *testing.T) { + name, version := splitRequirement(tt.spec) + if name != tt.wantName { + t.Errorf("name = %q, want %q", name, tt.wantName) + } + if version != tt.wantVersion { + t.Errorf("version = %q, want %q", version, tt.wantVersion) + } + }) + } +} diff --git a/internal/manifest/workspace.go b/internal/manifest/workspace.go index 2f4aa56..2a80a73 100644 --- a/internal/manifest/workspace.go +++ b/internal/manifest/workspace.go @@ -7,6 +7,7 @@ import ( "encoding/json" "os" "path/filepath" + "regexp" "strings" "gopkg.in/yaml.v3" @@ -14,30 +15,30 @@ import ( // DefaultSkipDirs contains directories that should be skipped during manifest discovery. var DefaultSkipDirs = map[string]bool{ - "node_modules": true, - "vendor": true, - ".git": true, - ".svn": true, - ".hg": true, - "dist": true, - "build": true, - ".next": true, - "__pycache__": true, - ".venv": true, - "venv": true, - ".tox": true, - "target": true, // Maven/Rust - "bin": true, - "obj": true, // .NET - ".idea": true, - ".vscode": true, - "coverage": true, - ".nyc_output": true, - ".pytest_cache": true, - ".mypy_cache": true, - ".ruff_cache": true, - ".gradle": true, - ".m2": true, + "node_modules": true, + "vendor": true, + ".git": true, + ".svn": true, + ".hg": true, + "dist": true, + "build": true, + ".next": true, + "__pycache__": true, + ".venv": true, + "venv": true, + ".tox": true, + "target": true, // Maven/Rust + "bin": true, + "obj": true, // .NET + ".idea": true, + ".vscode": true, + "coverage": true, + ".nyc_output": true, + ".pytest_cache": true, + ".mypy_cache": true, + ".ruff_cache": true, + ".gradle": true, + ".m2": true, "bower_components": true, } @@ -75,7 +76,7 @@ func DiscoverManifests(root string) ([]string, error) { // If it's a file, return just that file if it's a manifest if !info.IsDir() { - if isManifestFile(filepath.Base(root)) { + if isManifestPath(root) { return []string{root}, nil } return nil, nil @@ -326,7 +327,7 @@ func walkForManifests(root string) ([]string, error) { } // Check if this is a manifest file - if isManifestFile(name) { + if isManifestPath(path) { // Skip go.work files in the recursive walk (handled separately) if name == "go.work" { return nil @@ -340,9 +341,37 @@ func walkForManifests(root string) ([]string, error) { return manifests, err } +// requirementsFilePattern matches the requirements.txt family. Python projects +// routinely split dependencies across requirements-dev.txt, requirements-prod.txt, +// requirements_test.txt and similar, and an exact match on "requirements.txt" +// silently skips all of them. +var requirementsFilePattern = regexp.MustCompile(`^requirements([-_.][^/]*)?\.txt$`) + +// isRequirementsFile reports whether a filename belongs to the requirements.txt +// family. +func isRequirementsFile(name string) bool { + return requirementsFilePattern.MatchString(strings.ToLower(name)) +} + // isManifestFile checks if a filename is a recognized manifest file. func isManifestFile(name string) bool { - return ManifestFiles[name] + return ManifestFiles[name] || isRequirementsFile(name) +} + +// isManifestPath checks whether a path is a manifest, including layouts that +// only make sense with the parent directory in hand. The common +// requirements/base.txt and requirements/dev.txt layout puts pip requirements in +// files whose own names carry no hint of their contents. +func isManifestPath(path string) bool { + name := filepath.Base(path) + if isManifestFile(name) { + return true + } + if strings.EqualFold(filepath.Base(filepath.Dir(path)), "requirements") && + strings.EqualFold(filepath.Ext(name), ".txt") { + return true + } + return false } // isValidManifest checks if a manifest file is valid and parseable. diff --git a/pkg/output/cbom.go b/pkg/output/cbom.go index b6da484..7fef17b 100644 --- a/pkg/output/cbom.go +++ b/pkg/output/cbom.go @@ -4,9 +4,14 @@ package output import ( + "crypto/rand" "encoding/json" "errors" + "fmt" "io" + "net/url" + "regexp" + "strings" "time" "github.com/csnp/qramm-cryptodeps/pkg/types" @@ -17,17 +22,18 @@ type CBOMFormatter struct{} // cycloneDXBOM represents a CycloneDX BOM structure. type cycloneDXBOM struct { - BOMFormat string `json:"bomFormat"` - SpecVersion string `json:"specVersion"` - SerialNumber string `json:"serialNumber"` - Version int `json:"version"` - Metadata cycloneDXMetadata `json:"metadata"` - Components []cycloneDXComponent `json:"components"` + BOMFormat string `json:"bomFormat"` + SpecVersion string `json:"specVersion"` + SerialNumber string `json:"serialNumber"` + Version int `json:"version"` + Metadata cycloneDXMetadata `json:"metadata"` + Components []cycloneDXComponent `json:"components"` + Dependencies []cycloneDXDependency `json:"dependencies,omitempty"` } type cycloneDXMetadata struct { - Timestamp string `json:"timestamp"` - Tools []cycloneDXTool `json:"tools"` + Timestamp string `json:"timestamp"` + Tools []cycloneDXTool `json:"tools"` } type cycloneDXTool struct { @@ -37,23 +43,33 @@ type cycloneDXTool struct { } type cycloneDXComponent struct { - Type string `json:"type"` - Name string `json:"name"` - Version string `json:"version"` - CryptoProperties *cycloneDXCryptoProperties `json:"cryptoProperties,omitempty"` + Type string `json:"type"` + BOMRef string `json:"bom-ref"` + Name string `json:"name"` + Version string `json:"version,omitempty"` + Purl string `json:"purl,omitempty"` + Description string `json:"description,omitempty"` + CryptoProperties *cycloneDXCryptoProperties `json:"cryptoProperties,omitempty"` +} + +// cycloneDXDependency links a component to the components it depends on. This +// is how a cryptographic asset is attributed to the library that provides it. +type cycloneDXDependency struct { + Ref string `json:"ref"` + DependsOn []string `json:"dependsOn"` } type cycloneDXCryptoProperties struct { - AssetType string `json:"assetType"` - AlgorithmProperties *cycloneDXAlgorithmProperties `json:"algorithmProperties,omitempty"` + AssetType string `json:"assetType"` + AlgorithmProperties *cycloneDXAlgorithmProperties `json:"algorithmProperties,omitempty"` } type cycloneDXAlgorithmProperties struct { - Primitive string `json:"primitive"` - ParameterSetIdentifier string `json:"parameterSetIdentifier,omitempty"` - ExecutionEnvironment string `json:"executionEnvironment,omitempty"` - ImplementationPlatform string `json:"implementationPlatform,omitempty"` - CryptoFunctions []string `json:"cryptoFunctions,omitempty"` + Primitive string `json:"primitive"` + ParameterSetIdentifier string `json:"parameterSetIdentifier,omitempty"` + ExecutionEnvironment string `json:"executionEnvironment,omitempty"` + ImplementationPlatform string `json:"implementationPlatform,omitempty"` + CryptoFunctions []string `json:"cryptoFunctions,omitempty"` } // Format writes the scan result as CycloneDX CBOM. @@ -82,37 +98,185 @@ func (f *CBOMFormatter) Format(result *types.ScanResult, w io.Writer) error { Components: make([]cycloneDXComponent, 0), } - // Convert scan results to CycloneDX components + // Emit each dependency as a component, then each algorithm it provides as a + // cryptographic asset, and link the two through the dependencies graph. + // + // Previously only the algorithm was emitted, carrying the dependency's + // version but never its name, so a component reading {"name": "RSA", + // "version": "1.3.2"} could not be traced back to the library it came from. + seenAlgorithm := make(map[string]bool) + dependsOn := make(map[string][]string) + for _, dep := range result.Dependencies { if dep.Analysis == nil || len(dep.Analysis.Crypto) == 0 { continue } + depName := dep.Dependency.Name + depVersion := resolveVersion(dep.Dependency.Version) + purl := "pkg:" + string(dep.Dependency.Ecosystem) + "/" + depName + if depVersion != "" { + purl += "@" + depVersion + } + // The bom-ref must stay unique per declared dependency even when two + // entries resolve to the same purl because neither pinned a version. + depRef := purl + if depVersion == "" && strings.TrimSpace(dep.Dependency.Version) != "" { + depRef = purl + "?constraint=" + url.QueryEscape(strings.TrimSpace(dep.Dependency.Version)) + } + + if _, exists := dependsOn[depRef]; !exists { + bom.Components = append(bom.Components, cycloneDXComponent{ + Type: "library", + BOMRef: depRef, + Name: depName, + Version: depVersion, + Purl: purl, + Description: describeConstraint(dep.Dependency.Version), + }) + dependsOn[depRef] = nil + } + for _, crypto := range dep.Analysis.Crypto { - component := cycloneDXComponent{ - Type: "cryptographic-asset", - Name: crypto.Algorithm, - Version: dep.Dependency.Version, - CryptoProperties: &cycloneDXCryptoProperties{ - AssetType: "algorithm", - AlgorithmProperties: &cycloneDXAlgorithmProperties{ - Primitive: crypto.Type, + algorithmRef := "crypto:" + crypto.Algorithm + + if !seenAlgorithm[algorithmRef] { + seenAlgorithm[algorithmRef] = true + bom.Components = append(bom.Components, cycloneDXComponent{ + Type: "cryptographic-asset", + BOMRef: algorithmRef, + Name: crypto.Algorithm, + CryptoProperties: &cycloneDXCryptoProperties{ + AssetType: "algorithm", + AlgorithmProperties: &cycloneDXAlgorithmProperties{ + Primitive: cycloneDXPrimitive(crypto.Type, crypto.Algorithm), + }, }, - }, + }) } - bom.Components = append(bom.Components, component) + + dependsOn[depRef] = append(dependsOn[depRef], algorithmRef) } } + // Emit the graph in component order so output is deterministic. + for _, component := range bom.Components { + refs, ok := dependsOn[component.BOMRef] + if !ok || len(refs) == 0 { + continue + } + bom.Dependencies = append(bom.Dependencies, cycloneDXDependency{ + Ref: component.BOMRef, + DependsOn: dedupeStrings(refs), + }) + } + encoder := json.NewEncoder(w) encoder.SetIndent("", " ") return encoder.Encode(bom) } -// generateUUID generates a simple UUID (placeholder implementation). +// mavenPropertyPattern matches an unresolved build property such as +// ${java-jwt.version}. +var mavenPropertyPattern = regexp.MustCompile(`^\$\{[^}]*\}$`) + +// pinnedVersionPattern matches a constraint that names exactly one version, +// either bare (1.2.3) or pinned with == or === as pip and Maven write it. +var pinnedVersionPattern = regexp.MustCompile(`^(?:={2,3})?\s*v?(\d[\w.+-]*)$`) + +// resolveVersion converts a declared dependency constraint into a concrete +// version, or an empty string when the constraint does not name one. +// +// CycloneDX `version` and the version segment of a purl are concrete versions, +// not ranges. Emitting ">=2.0" or "^1.0" there states a version that was never +// resolved, and emitting a Maven placeholder such as ${java-jwt.version} states +// one that does not exist at all. An absent version is honest; either of those +// is not. The original constraint is preserved separately in the component +// description so nothing is lost. +func resolveVersion(constraint string) string { + constraint = strings.TrimSpace(constraint) + if constraint == "" || mavenPropertyPattern.MatchString(constraint) { + return "" + } + if match := pinnedVersionPattern.FindStringSubmatch(constraint); match != nil { + return match[1] + } + return "" +} + +// describeConstraint records the constraint exactly as the manifest declared it, +// for the cases where it could not be resolved to a single version. +func describeConstraint(constraint string) string { + constraint = strings.TrimSpace(constraint) + if constraint == "" || resolveVersion(constraint) != "" { + return "" + } + if mavenPropertyPattern.MatchString(constraint) { + return "Declared version is the unresolved build property " + constraint + } + return "Declared version constraint: " + constraint +} + +// cycloneDXPrimitive maps this tool's algorithm categories onto the closed +// enum CycloneDX 1.6 allows for algorithmProperties.primitive. Emitting a value +// outside that enum makes the whole document fail schema validation. +func cycloneDXPrimitive(cryptoType, algorithm string) string { + switch strings.ToLower(cryptoType) { + case "key-exchange": + return "key-agree" + case "signature": + return "signature" + case "hash": + return "hash" + case "mac": + return "mac" + case "encryption": + // "encryption" covers several CycloneDX primitives, so resolve it by + // algorithm where the algorithm is recognized. + switch strings.ToUpper(algorithm) { + case "AES", "3DES", "DES", "BLOWFISH", "CAMELLIA", "IDEA", "CAST5", "SEED", "ARIA": + return "block-cipher" + case "CHACHA20", "RC4", "SALSA20": + return "stream-cipher" + case "RSA", "ELGAMAL": + return "pke" + case "ML-KEM", "KYBER": + return "kem" + } + return "other" + default: + // An unrecognized category is reported as unknown rather than guessed. + return "unknown" + } +} + +func dedupeStrings(values []string) []string { + seen := make(map[string]bool, len(values)) + out := make([]string, 0, len(values)) + for _, v := range values { + if seen[v] { + continue + } + seen[v] = true + out = append(out, v) + } + return out +} + +// generateUUID returns a random RFC 4122 version 4 UUID. Every CBOM previously +// carried the same all-zero serial number, so documents from different scans +// were indistinguishable and the value did not match the CycloneDX urn:uuid +// pattern. func generateUUID() string { - // In production, use a proper UUID library - return "00000000-0000-0000-0000-000000000000" + var b [16]byte + if _, err := rand.Read(b[:]); err != nil { + // crypto/rand failing is not recoverable here, and emitting a fixed + // serial number would reintroduce the collision this replaces. + panic("cbom: cannot read random bytes for serial number: " + err.Error()) + } + b[6] = (b[6] & 0x0f) | 0x40 // version 4 + b[8] = (b[8] & 0x3f) | 0x80 // RFC 4122 variant + return fmt.Sprintf("%x-%x-%x-%x-%x", b[0:4], b[4:6], b[6:8], b[8:10], b[10:16]) } // FormatMulti writes multi-project scan results as CBOM. diff --git a/pkg/output/cbom_attribution_test.go b/pkg/output/cbom_attribution_test.go new file mode 100644 index 0000000..c80e7bf --- /dev/null +++ b/pkg/output/cbom_attribution_test.go @@ -0,0 +1,228 @@ +// Copyright 2024-2025 CSNP (csnp.org) +// SPDX-License-Identifier: Apache-2.0 + +package output + +import ( + "bytes" + "encoding/json" + "regexp" + "testing" + + "github.com/csnp/qramm-cryptodeps/pkg/types" +) + +// cycloneDXPrimitiveEnum is the closed set CycloneDX 1.6 allows for +// cryptoProperties.algorithmProperties.primitive. +var cycloneDXPrimitiveEnum = map[string]bool{ + "drbg": true, "mac": true, "block-cipher": true, "stream-cipher": true, + "signature": true, "hash": true, "pke": true, "xof": true, "kdf": true, + "key-agree": true, "kem": true, "ae": true, "combiner": true, + "other": true, "unknown": true, +} + +var uuidURNPattern = regexp.MustCompile( + `^urn:uuid:[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$`) + +type decodedBOM struct { + SerialNumber string `json:"serialNumber"` + Components []struct { + Type string `json:"type"` + BOMRef string `json:"bom-ref"` + Name string `json:"name"` + Version string `json:"version"` + Purl string `json:"purl"` + Description string `json:"description"` + CryptoProperties *struct { + AssetType string `json:"assetType"` + AlgorithmProperties *struct { + Primitive string `json:"primitive"` + } `json:"algorithmProperties"` + } `json:"cryptoProperties"` + } `json:"components"` + Dependencies []struct { + Ref string `json:"ref"` + DependsOn []string `json:"dependsOn"` + } `json:"dependencies"` +} + +func renderCBOM(t *testing.T, result *types.ScanResult) decodedBOM { + t.Helper() + + var buf bytes.Buffer + if err := (&CBOMFormatter{}).Format(result, &buf); err != nil { + t.Fatalf("format: %v", err) + } + var doc decodedBOM + if err := json.Unmarshal(buf.Bytes(), &doc); err != nil { + t.Fatalf("CBOM is not valid JSON: %v", err) + } + return doc +} + +func scanResultWithCrypto(depName, depVersion string, algorithms ...string) *types.ScanResult { + crypto := make([]types.CryptoUsage, 0, len(algorithms)) + for _, algo := range algorithms { + crypto = append(crypto, types.CryptoUsage{Algorithm: algo, Type: "signature"}) + } + return &types.ScanResult{ + Project: "testproject", + Ecosystem: types.EcosystemPyPI, + Dependencies: []types.DependencyResult{{ + Dependency: types.Dependency{ + Name: depName, + Version: depVersion, + Ecosystem: types.EcosystemPyPI, + }, + Analysis: &types.PackageAnalysis{Crypto: crypto}, + }}, + } +} + +// TestCBOMAttributesAlgorithmToDependency is the regression guard for issue #2. +// The previous output emitted only the algorithm, carrying the dependency's +// version but never its name, so {"name": "RSA", "version": "1.3.2"} could not +// be traced to the library that provided it. +func TestCBOMAttributesAlgorithmToDependency(t *testing.T) { + doc := renderCBOM(t, scanResultWithCrypto("java-jwt", "4.4.0", "RS384")) + + var libRef, algoRef string + for _, c := range doc.Components { + switch c.Type { + case "library": + if c.Name != "java-jwt" { + t.Errorf("library component name = %q, want java-jwt", c.Name) + } + libRef = c.BOMRef + case "cryptographic-asset": + if c.Name != "RS384" { + t.Errorf("crypto asset name = %q, want RS384", c.Name) + } + algoRef = c.BOMRef + } + } + + if libRef == "" { + t.Fatal("no library component emitted; the algorithm is unattributable") + } + if algoRef == "" { + t.Fatal("no cryptographic-asset component emitted") + } + + var linked bool + for _, d := range doc.Dependencies { + if d.Ref != libRef { + continue + } + for _, on := range d.DependsOn { + if on == algoRef { + linked = true + } + } + } + if !linked { + t.Errorf("dependency graph does not link %s to %s", libRef, algoRef) + } +} + +// TestCBOMDoesNotEmitUnresolvedMavenProperty covers the reporter's second +// example, where the algorithm component carried the literal build property +// ${java-jwt.version} as though it were a version. +func TestCBOMDoesNotEmitUnresolvedMavenProperty(t *testing.T) { + doc := renderCBOM(t, scanResultWithCrypto("java-jwt", "${java-jwt.version}", "RS384")) + + for _, c := range doc.Components { + if c.Version == "${java-jwt.version}" { + t.Errorf("component %q emitted an unresolved build property as its version", c.Name) + } + if c.Purl == "pkg:pypi/java-jwt@${java-jwt.version}" { + t.Errorf("purl %q embeds an unresolved build property", c.Purl) + } + } + + // The information must still be reachable rather than silently dropped. + var described bool + for _, c := range doc.Components { + if c.Type == "library" && c.Description != "" { + described = true + } + } + if !described { + t.Error("unresolved version constraint was dropped without explanation") + } +} + +// TestCBOMVersionIsConcreteNotAConstraint checks that range constraints do not +// masquerade as resolved versions. +func TestCBOMVersionIsConcreteNotAConstraint(t *testing.T) { + tests := []struct { + constraint string + wantVersion string + }{ + {"==41.0.0", "41.0.0"}, + {"41.0.0", "41.0.0"}, + {"===1.2.3", "1.2.3"}, + {">=2.0", ""}, + {"^1.0", ""}, + {"~=3.1", ""}, + {"${java-jwt.version}", ""}, + {"", ""}, + } + + for _, tt := range tests { + t.Run(tt.constraint, func(t *testing.T) { + if got := resolveVersion(tt.constraint); got != tt.wantVersion { + t.Errorf("resolveVersion(%q) = %q, want %q", + tt.constraint, got, tt.wantVersion) + } + }) + } +} + +// TestCBOMSerialNumberIsARealUUID guards the hardcoded all-zero serial number, +// which made every document share one identifier and did not match the +// CycloneDX urn:uuid pattern. +func TestCBOMSerialNumberIsARealUUID(t *testing.T) { + first := renderCBOM(t, scanResultWithCrypto("cryptography", "41.0.0", "RSA")) + second := renderCBOM(t, scanResultWithCrypto("cryptography", "41.0.0", "RSA")) + + if !uuidURNPattern.MatchString(first.SerialNumber) { + t.Errorf("serialNumber %q does not match the CycloneDX urn:uuid pattern", + first.SerialNumber) + } + if first.SerialNumber == "urn:uuid:00000000-0000-0000-0000-000000000000" { + t.Error("serialNumber is the placeholder all-zero UUID") + } + if first.SerialNumber == second.SerialNumber { + t.Error("two separate CBOMs share a serial number") + } +} + +// TestCBOMPrimitivesAreInSchemaEnum checks every emitted primitive against the +// CycloneDX enum, since a value outside it fails validation for the whole +// document. +func TestCBOMPrimitivesAreInSchemaEnum(t *testing.T) { + categories := []string{"key-exchange", "encryption", "hash", "signature", "mac", "surprise"} + algorithms := []string{"AES", "ChaCha20", "RSA", "ML-KEM", "SHA-256", "HMAC", "Whatever"} + + for _, category := range categories { + for _, algorithm := range algorithms { + got := cycloneDXPrimitive(category, algorithm) + if !cycloneDXPrimitiveEnum[got] { + t.Errorf("cycloneDXPrimitive(%q, %q) = %q, which is not in the CycloneDX enum", + category, algorithm, got) + } + } + } + + // Spot-check that the mapping is meaningful rather than uniformly "unknown". + if got := cycloneDXPrimitive("key-exchange", "ECDH"); got != "key-agree" { + t.Errorf("key-exchange mapped to %q, want key-agree", got) + } + if got := cycloneDXPrimitive("encryption", "AES"); got != "block-cipher" { + t.Errorf("AES mapped to %q, want block-cipher", got) + } + if got := cycloneDXPrimitive("encryption", "RSA"); got != "pke" { + t.Errorf("RSA encryption mapped to %q, want pke", got) + } +}