diff --git a/CHANGELOG.md b/CHANGELOG.md index 072fdc8..a82aee6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -81,6 +81,39 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 backslash is not joined to the next line. The dependencies, findings and exit code for that file are unchanged. +- **A `requirements/*.txt` file passed as the argument was refused.** A scan + of a directory reads `requirements/dev.txt` as a requirements file, but + `cryptodeps analyze requirements/dev.txt --offline` exited 2 with `Error: + analysis failed: unsupported manifest file: dev.txt`, with and without + `--no-workspaces`, because the parser for a file argument was chosen by the + file's name alone. It is now chosen by the file's path, as in a scan of a + directory. For a `requirements/dev.txt` holding `-r requirements.txt` and + `paramiko>=3.0`, the scan of the file now lists `paramiko` and the unread + `-r requirements.txt` line, its JSON report differs from the report for a + scan of the directory only in timestamps and paths, and both exit 1. Other + files keep their message: `docs/notes.txt` and `x/Cargo.toml` are refused + with `unsupported manifest file: notes.txt` and `unsupported manifest file: + Cargo.toml`, as before. + +- **A requirements.txt pin continued with a backslash kept the backslash in + its version.** pip reads a line that ends in a backslash together with the + next line, so `cryptography==42.0.0 \` with `--hash=sha256:...` on the next + line pins cryptography 42.0.0. The dependency parser read one physical line + at a time, so `analyze --offline --format json` reported + `"version": "42.0.0 \\"` and the table printed `cryptography@42.0.0 \`. + Dependencies are now read with a continued line joined to the next one as + pip joins it, and a requirement ends at its first per-requirement option, + such as `--hash`, so that file reports cryptography `42.0.0` at line 1. The + JSON output for that file differs only in the version and timestamps, and + the exit code is 1 before and after. A `--hash` on the requirement's own + line, which was reported as part of the version + (`42.0.0 --hash=sha256:...`), is left out of it the same way. An `-r` line + or a `name @ url` line continued onto the next line is now skipped as a + whole: `-r \` followed by `extra.txt` was also read as a dependency named + `extra` with the version `.txt`, and `paramiko @ \` followed by a URL as + `paramiko` with the version `@ \`. Each is still listed, by its first line, + with the lines the scan did not read. + - **A scan of a GitHub repository reported reachability for source code it never fetched.** A GitHub URL or `owner/repo` argument downloads the repository's manifest files only, and reachability then ran over the diff --git a/internal/manifest/parser.go b/internal/manifest/parser.go index c148055..f1863d8 100644 --- a/internal/manifest/parser.go +++ b/internal/manifest/parser.go @@ -54,8 +54,11 @@ func DetectAndParse(path string) (*Manifest, error) { manifestPath = path } - // Detect ecosystem from filename - parser, err := getParser(filepath.Base(manifestPath)) + // Detect the ecosystem from the path, with the lookup DetectAndParseAll + // uses. Looked up by filename alone, requirements/dev.txt was refused as + // "unsupported manifest file: dev.txt", though a scan of its directory + // reads it as a requirements file. + parser, err := getParserForPath(manifestPath) if err != nil { if lockErr := lockFileError(manifestPath); lockErr != nil { return nil, lockErr @@ -128,15 +131,18 @@ func getParser(filename string) (Parser, error) { // 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. +// any other text file by name alone. When no parser matches, it returns +// getParser's error, which names the file without its directory, as in +// "unsupported manifest file: Cargo.toml" for x/Cargo.toml. func getParserForPath(path string) (Parser, error) { - if parser, err := getParser(filepath.Base(path)); err == nil { + parser, err := getParser(filepath.Base(path)) + if 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) + return nil, err } // lockFileManifests maps a lock file, by its lower-case name, to the manifest diff --git a/internal/manifest/parser_test.go b/internal/manifest/parser_test.go index b8ce457..5f01ea0 100644 --- a/internal/manifest/parser_test.go +++ b/internal/manifest/parser_test.go @@ -6,6 +6,7 @@ package manifest import ( "os" "path/filepath" + "reflect" "strings" "testing" @@ -528,6 +529,51 @@ func TestDetectAndParseOtherUnsupportedKeepsMessage(t *testing.T) { } } +// A requirements/*.txt file passed as the argument was refused as +// "unsupported manifest file: dev.txt": DetectAndParse looked its parser up by +// filename, while a scan of the directory looks it up by path and reads the +// file. Given as the argument, the file is now read with the dependencies and +// unread lines the scan of its directory lists for it. +func TestDetectAndParseRequirementsDirectoryFile(t *testing.T) { + dir := writeFiles(t, map[string]string{ + filepath.Join("requirements", "dev.txt"): "-r requirements.txt\nparamiko>=3.0\n", + }) + path := filepath.Join(dir, "requirements", "dev.txt") + + scanned, skipped, err := DetectAndParseAll(dir) + if err != nil || len(skipped) != 0 || len(scanned) != 1 { + t.Fatalf("DetectAndParseAll(dir) = %d manifests, skipped %+v, error %v; want 1 manifest", len(scanned), skipped, err) + } + want := scanned[0] + if len(want.Dependencies) != 1 || want.Dependencies[0].Name != "paramiko" || + len(want.Unread) != 1 || want.Unread[0].Line != 1 || want.Unread[0].Name != "requirements.txt" { + t.Fatalf("scan of the directory: dependencies %+v, unread %+v; want paramiko and the -r include on line 1", + want.Dependencies, want.Unread) + } + + // Both entry points: analyze uses DetectAndParseAll by default and + // DetectAndParse under --no-workspaces. + single, err := DetectAndParse(path) + if err != nil { + t.Fatalf("DetectAndParse(%s): %v", path, err) + } + all, skipped, err := DetectAndParseAll(path) + if err != nil || len(skipped) != 0 || len(all) != 1 { + t.Fatalf("DetectAndParseAll(%s) = %d manifests, skipped %+v, error %v; want 1 manifest", path, len(all), skipped, err) + } + for name, got := range map[string]*Manifest{"DetectAndParse": single, "DetectAndParseAll": all[0]} { + if got.Ecosystem != types.EcosystemPyPI { + t.Errorf("%s: Ecosystem = %q, want %q", name, got.Ecosystem, types.EcosystemPyPI) + } + if !reflect.DeepEqual(got.Dependencies, want.Dependencies) { + t.Errorf("%s: Dependencies = %+v, want %+v", name, got.Dependencies, want.Dependencies) + } + if !reflect.DeepEqual(got.Unread, want.Unread) { + t.Errorf("%s: Unread = %+v, want %+v", name, got.Unread, want.Unread) + } + } +} + func TestDetectAndParseNonexistent(t *testing.T) { _, err := DetectAndParse("/nonexistent/path") if err == nil { diff --git a/internal/manifest/python.go b/internal/manifest/python.go index 684eef0..a7d69a0 100644 --- a/internal/manifest/python.go +++ b/internal/manifest/python.go @@ -631,10 +631,9 @@ func localPathRequirementName(line string) string { // readAsRequirements reports whether the scan reads path with // parseRequirementsTxt, as it does requirements.txt, the requirements-*.txt // family and the requirements/*.txt layout. It asks getParserForPath, the -// lookup DetectAndParseAll picks parsers with, rather than repeating its rule, -// and then leaves out pyproject.toml and Pipfile, which Parse reads with -// readers of their own. For a file DetectAndParse accepts, getParserForPath -// returns the parser getParser chose. +// lookup DetectAndParse and DetectAndParseAll pick parsers with, rather than +// repeating its rule, and then leaves out pyproject.toml and Pipfile, which +// Parse reads with readers of their own. // // unreadLines used to test for the name "requirements.txt" alone, so an "-r" // include, an "-e" line or a URL requirement in requirements-dev.txt or @@ -722,7 +721,24 @@ func joinContinuedLines(r io.Reader) ([]requirementsLine, error) { return lines, scanner.Err() } +// perRequirementOptionPattern finds the first per-requirement option on a +// requirements.txt line, such as --hash: whitespace followed by "-". pip ends +// the requirement before the first space-separated token that begins with "-" +// and reads the rest of the line as options. A tab before such a token ends +// the requirement here too, where pip splits on spaces only. +var perRequirementOptionPattern = regexp.MustCompile(`\s-`) + // parseRequirementsTxt parses a requirements.txt file. +// +// Lines are read as pip reads them, with a line that ends in a backslash +// joined to the next one, and a requirement ends at its first per-requirement +// option. Read one physical line at a time, a pin with its hash on the next +// line, +// +// cryptography==42.0.0 \ +// --hash=sha256:... +// +// gave cryptography the version "42.0.0 \". func (p *PythonParser) parseRequirementsTxt(path string) ([]types.Dependency, error) { file, err := os.Open(path) if err != nil { @@ -730,6 +746,11 @@ func (p *PythonParser) parseRequirementsTxt(path string) ([]types.Dependency, er } defer file.Close() + lines, err := joinContinuedLines(file) + if err != nil { + return nil, err + } + var deps []types.Dependency // Regex patterns for requirements.txt @@ -739,11 +760,8 @@ func (p *PythonParser) parseRequirementsTxt(path string) ([]types.Dependency, er // package[extra]==1.0.0 requirementPattern := regexp.MustCompile(`^([a-zA-Z0-9_-]+)(?:\[[^\]]+\])?([<>=!~]+)?(.+)?`) - scanner := bufio.NewScanner(file) - lineNo := 0 - for scanner.Scan() { - lineNo++ - line := strings.TrimSpace(scanner.Text()) + for _, l := range lines { + line := strings.TrimSpace(l.text) // Skip empty lines and comments if line == "" || strings.HasPrefix(line, "#") { @@ -766,7 +784,11 @@ func (p *PythonParser) parseRequirementsTxt(path string) ([]types.Dependency, er continue } - // Parse requirement line + // Parse requirement line, which ends at its first per-requirement + // option + if loc := perRequirementOptionPattern.FindStringIndex(line); loc != nil { + line = line[:loc[0]] + } if matches := requirementPattern.FindStringSubmatch(line); matches != nil { name := matches[1] version := "" @@ -787,14 +809,10 @@ func (p *PythonParser) parseRequirementsTxt(path string) ([]types.Dependency, er Version: version, Ecosystem: types.EcosystemPyPI, Direct: true, - Line: lineNo, + Line: l.number, }) } } - if err := scanner.Err(); err != nil { - return nil, err - } - return deps, nil } diff --git a/internal/manifest/requirements_continued_test.go b/internal/manifest/requirements_continued_test.go new file mode 100644 index 0000000..7e90e6c --- /dev/null +++ b/internal/manifest/requirements_continued_test.go @@ -0,0 +1,86 @@ +// Copyright 2025-2026 CyberSecurity NonProfit (CSNP) +// SPDX-License-Identifier: Apache-2.0 + +package manifest + +import ( + "path/filepath" + "strings" + "testing" +) + +// pip reads a requirements.txt line that ends in a backslash together with the +// next one, so a pin can carry its hashes on the lines after it: +// +// cryptography==42.0.0 \ +// --hash=sha256:... +// +// The parser read one physical line at a time, so the version kept the +// backslash and was reported as "42.0.0 \". It now reads the lines unreadLines +// reads, with continued lines joined and numbered by their first line, and +// ends a requirement at its first per-requirement option, such as --hash, as +// pip does. An "-r" include or a name @ URL reference continued onto the next +// line is then skipped as a whole, where its second line was read as a +// dependency of its own or its first line gave a version of "@ \". +func TestRequirementsTxtJoinsContinuedLines(t *testing.T) { + type dep struct { + name string + version string + line int + } + cases := []struct { + name string + lines []string + want []dep + }{ + {"hash on the next line", []string{`cryptography==42.0.0 \`, " --hash=sha256:FAKE"}, + []dep{{"cryptography", "42.0.0", 1}}}, + {"several pins, hashes and comment lines", []string{ + "# pinned with hashes", + `cryptography==42.0.0 \`, + ` --hash=sha256:FAKE1 \`, + " --hash=sha256:FAKE2", + " # via paramiko", + `paramiko==3.4.0 \`, + " --hash=sha256:FAKE3", + " # via -r requirements.in", + }, []dep{{"cryptography", "42.0.0", 2}, {"paramiko", "3.4.0", 6}}}, + {"hash on the same line", []string{"cryptography==42.0.0 --hash=sha256:FAKE"}, + []dep{{"cryptography", "42.0.0", 1}}}, + {"tab before the hash", []string{"cryptography==42.0.0\t--hash=sha256:FAKE"}, + []dep{{"cryptography", "42.0.0", 1}}}, + {"unpinned with a hash", []string{`cryptography \`, " --hash=sha256:FAKE"}, + []dep{{"cryptography", "", 1}}}, + {"environment marker on the next line", []string{`pyjwt==2.8.0 \`, ` ; python_version >= "3.8"`}, + []dep{{"pyjwt", "2.8.0", 1}}}, + {"CRLF line endings", []string{"cryptography==42.0.0 \\\r", " --hash=sha256:FAKE\r"}, + []dep{{"cryptography", "42.0.0", 1}}}, + {"backslash on the last line", []string{"requests==2.31.0", `cryptography==42.0.0 \`}, + []dep{{"requests", "2.31.0", 1}, {"cryptography", "42.0.0", 2}}}, + {"include continued onto the next line", []string{`-r \`, "extra.txt", "cryptography==42.0.0"}, + []dep{{"cryptography", "42.0.0", 3}}}, + {"name @ URL continued onto the next line", []string{`paramiko @ \`, "https://example.invalid/p", "cryptography==42.0.0"}, + []dep{{"cryptography", "42.0.0", 3}}}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + path := filepath.Join(t.TempDir(), "requirements.txt") + writeFile(t, path, strings.Join(c.lines, "\n")+"\n") + + deps, err := (&PythonParser{}).Parse(path) + if err != nil { + t.Fatalf("parse: %v", err) + } + if len(deps) != len(c.want) { + t.Fatalf("dependencies = %+v, want %d", deps, len(c.want)) + } + for i, w := range c.want { + g := deps[i] + if g.Name != w.name || g.Version != w.version || g.Line != w.line { + t.Errorf("dependencies[%d] = %s %q at line %d, want %s %q at line %d", + i, g.Name, g.Version, g.Line, w.name, w.version, w.line) + } + } + }) + } +} diff --git a/internal/manifest/requirements_unread_test.go b/internal/manifest/requirements_unread_test.go index 29ddf4f..138eea2 100644 --- a/internal/manifest/requirements_unread_test.go +++ b/internal/manifest/requirements_unread_test.go @@ -127,8 +127,7 @@ func TestRequirementsUnreadLocalPathLines(t *testing.T) { // as requirements.txt, but listed unread lines only for a file named // requirements.txt, so the same lines in the others were dropped with no // signal. Each now lists what requirements.txt lists for the same content, in -// a scan of its directory and, for a file the scan accepts as its argument, in -// a scan of the file itself. +// a scan of its directory and in a scan of the file itself. func TestUnreadLinesForEveryRequirementsFile(t *testing.T) { content := strings.Join([]string{ "cryptography==42.0.0", // 1 read @@ -149,19 +148,14 @@ func TestUnreadLinesForEveryRequirementsFile(t *testing.T) { t.Fatalf("requirements.txt: Unread = %+v, want lines 2 to 6", reference.Unread) } - for _, tc := range []struct { - name string - // argument is whether the scan accepts the file as its argument. A - // requirements/*.txt file is read in a scan of a directory only. - argument bool - }{ - {"requirements-dev.txt", true}, - {"requirements_test.txt", true}, - {"requirements.prod.txt", true}, - {filepath.Join("requirements", "dev.txt"), false}, + for _, name := range []string{ + "requirements-dev.txt", + "requirements_test.txt", + "requirements.prod.txt", + filepath.Join("requirements", "dev.txt"), } { - t.Run(tc.name, func(t *testing.T) { - dir := writeFiles(t, map[string]string{tc.name: content}) + t.Run(name, func(t *testing.T) { + dir := writeFiles(t, map[string]string{name: content}) manifests, skipped, err := DetectAndParseAll(dir) if err != nil || len(skipped) != 0 || len(manifests) != 1 { t.Fatalf("DetectAndParseAll = %d manifests, skipped %+v, error %v; want 1 manifest", len(manifests), skipped, err) @@ -170,10 +164,7 @@ func TestUnreadLinesForEveryRequirementsFile(t *testing.T) { t.Errorf("scan of the directory: Unread = %+v, want %+v", manifests[0].Unread, reference.Unread) } - if !tc.argument { - return - } - m, err := DetectAndParse(filepath.Join(dir, tc.name)) + m, err := DetectAndParse(filepath.Join(dir, name)) if err != nil { t.Fatal(err) }