Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 11 additions & 5 deletions internal/manifest/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
46 changes: 46 additions & 0 deletions internal/manifest/parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package manifest
import (
"os"
"path/filepath"
"reflect"
"strings"
"testing"

Expand Down Expand Up @@ -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 {
Expand Down
48 changes: 33 additions & 15 deletions internal/manifest/python.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -722,14 +721,36 @@ 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 {
return nil, err
}
defer file.Close()

lines, err := joinContinuedLines(file)
if err != nil {
return nil, err
}

var deps []types.Dependency

// Regex patterns for requirements.txt
Expand All @@ -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, "#") {
Expand All @@ -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 := ""
Expand All @@ -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
}
86 changes: 86 additions & 0 deletions internal/manifest/requirements_continued_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
}
})
}
}
27 changes: 9 additions & 18 deletions internal/manifest/requirements_unread_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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)
}
Expand Down
Loading