From 6705946940fa8143c760cf95c91f2edf3dc65763 Mon Sep 17 00:00:00 2001 From: prode Date: Sun, 2 Aug 2026 20:57:32 -0300 Subject: [PATCH] feat(validate): read Python, Rust, PHP and Java dependency files The stack check only knew go.mod and package.json, so a Python or Java project got no findings at all: dependencies() came back empty and Stack returned before it ever looked for docs/stack.md. Silence is the right answer for an ecosystem scc cannot read, but it also meant the rule the whole knowledge base is built on held in exactly two ecosystems. Five readers, each with a parse that cannot be wrong: requirements.txt one requirement per line, PEP 508 names; options, markers, extras and direct URLs all fall out pyproject.toml PEP 621, PEP 735 and Poetry's own tables Cargo.toml dependencies, dev-, build-, per-target, workspace, and the [dependencies.] long form composer.json require and require-dev, minus the platform keys pom.xml project/dependencies/dependency and nothing else, so dependencyManagement and profiles stay out The two TOML files share a reader that recognizes exactly two shapes, a table header and a key = value line, and ignores everything else. It is not a TOML parser and the comment says so: an array that never closes yields nothing rather than half a dependency list. Also: a dependency documented with the other separator now counts. PyPI treats Flask-SQLAlchemy and flask_sqlalchemy as one project, and an extra spelling can only remove a finding, never add one. The readers move to stack_manifests.go. The rule is fixed; the readers grow whenever somebody uses an ecosystem scc cannot see. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01GchWF6iL5PjTdXAT8H3mhb --- CLAUDE.md | 2 +- design/orchestration.md | 15 +- internal/validate/stack.go | 130 ++----- internal/validate/stack_manifests.go | 448 ++++++++++++++++++++++ internal/validate/stack_manifests_test.go | 255 ++++++++++++ 5 files changed, 751 insertions(+), 99 deletions(-) create mode 100644 internal/validate/stack_manifests.go create mode 100644 internal/validate/stack_manifests_test.go diff --git a/CLAUDE.md b/CLAUDE.md index e73e41e..71ad1da 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -78,7 +78,7 @@ cmd/scc/main.go os.Exit(cli.Run(os.Args[1:])) | `internal/scaffold` | Applies the template set to a root (`Apply`) and brings an existing one current (`PlanUpdate`/`ApplyUpdate`). Idempotent, never overwrites without being told to, manifest written last. | | `internal/mdscan` | The only Markdown parser: fence- and HTML-comment-aware headings, checkboxes, links, wikilinks, slugs, plus a small frontmatter reader. `Body` is the comment/fence-stripped text every validator applies its grammar to. | | `internal/ears` | EARS requirement parsing, all five patterns plus complex. | -| `internal/validate` | The eight validators, one file each, sharing `mdscan` and `finding`. | +| `internal/validate` | The eight validators, one file each, sharing `mdscan` and `finding`. The exception is `stack_manifests.go`: the seven dependency-file readers age on their own schedule, so they sit beside the rule rather than inside it. | | `internal/cli` | The dispatcher and every command handler. | `go.mod` is stdlib-only. Keep it that way unless a dependency earns its place — the binary is distributed to six platforms and every dep is a supply-chain surface. diff --git a/design/orchestration.md b/design/orchestration.md index 4f84158..9e82462 100644 --- a/design/orchestration.md +++ b/design/orchestration.md @@ -297,7 +297,7 @@ a shape is checkable. Findings exit `2`. | **codewiki** | at `docs/codewiki/`, one page per area: every `[path:start-end]()` citation resolves against the checkout · slugs unique and derived from their headings · no section citing nothing | | **adr** | numbering contiguous · superseded records marked rather than edited · `adr:` citations resolve | | **glossary** | each concept has one canonical term · an avoided synonym used as a whole token is a finding | -| **stack** | every dependency in the project's manifest (`go.mod`, `package.json`, …) appears in `docs/stack.md` — a manifest is structured data, not source, so this is checkable without reading code | +| **stack** | every direct dependency the project declares appears in `docs/stack.md` — a dependency file is structured data, not source, so this is checkable without reading code. Seven readers today: `go.mod`, `package.json`, `requirements.txt`, `pyproject.toml`, `Cargo.toml`, `composer.json`, `pom.xml` | Two of these are worth calling out because they are stronger than they look. @@ -314,9 +314,16 @@ guess is exactly the false positive that would cost the user's trust in the othe The three above are unambiguous. **`stack` turns a rule into a gate.** "Technology not listed in `docs/stack.md` is an open -decision, never adopted silently" reads like an unenforceable principle, but dependency -manifests are structured files. Diffing the manifest against `stack.md` catches the silent -dependency, and does it in any ecosystem that has a manifest. +decision, never adopted silently" reads like an unenforceable principle, but a project's +dependency file is a structured file. Diffing it against `stack.md` catches the silent +dependency, and does it in any ecosystem that declares its dependencies as data. + +**An ecosystem with no reader is not checked, never failed.** No declared dependencies +means Stack returns before it asks whether `stack.md` exists, so a Ruby, Elixir or +Gradle project passes `scc validate` untouched rather than failing on a file scc never +understood. That is the same rule as "a manifest scc cannot parse produces no findings", +and it is why those three are absent: their manifests are executable code, and reading +one honestly would mean evaluating it. ### The discipline eight validators require diff --git a/internal/validate/stack.go b/internal/validate/stack.go index 71a05d8..4ee1a65 100644 --- a/internal/validate/stack.go +++ b/internal/validate/stack.go @@ -1,34 +1,33 @@ package validate import ( - "encoding/json" - "os" - "path/filepath" "regexp" "sort" "strings" "github.com/protonspy/spec-claude-code/internal/finding" "github.com/protonspy/spec-claude-code/internal/paths" - "github.com/protonspy/spec-claude-code/internal/textutil" ) // Stack turns a rule into a gate. // // "Technology not listed in docs/stack.md is an open decision, never adopted silently" -// reads like an unenforceable principle — but a dependency manifest is structured data, -// not source, so diffing the manifest against stack.md catches the silent dependency -// without reading a line of code. That is the whole trick, and it works in any ecosystem -// that has a manifest. +// reads like an unenforceable principle — but a project's dependency file is structured +// data, not source, so diffing it against stack.md catches the silent dependency without +// reading a line of code. That is the whole trick, and it works in any ecosystem that +// declares its dependencies as data. The readers live in stack_manifests.go. // -// Two deliberate limits: +// Three deliberate limits: // // - **Direct dependencies only.** An indirect dependency is not a decision anybody // made; reporting the transitive closure would bury the one line that matters. -// - **An unparsed manifest produces no findings.** Ecosystems whose manifests cannot -// be read confidently without a dependency (TOML, for one) wait for a safe parse -// rather than getting a naive one. Silence is the correct behavior when a check -// cannot be sure. +// - **An unparsed manifest produces no findings.** A file scc cannot read +// confidently, or a shape it does not recognize, yields no dependency rather than a +// guessed one. Silence is the correct behavior when a check cannot be sure. +// - **An ecosystem with no reader is not checked at all.** No declared dependencies +// means no expectation of a stack.md and no finding of any kind, so a project scc +// cannot read the manifest of passes rather than failing on a file it never +// understood. func Stack(root string) (*finding.Set, error) { set := &finding.Set{} deps, err := dependencies(root) @@ -63,10 +62,7 @@ func Stack(root string) (*finding.Set, error) { } sort.Strings(names) for _, name := range names { - // The full path, or the last segment — someone writing "chi" for - // github.com/go-chi/chi has documented the decision, and insisting on the - // module path would be the validator enforcing a spelling nobody agreed to. - if strings.Contains(listed, strings.ToLower(name)) || strings.Contains(listed, strings.ToLower(baseName(name))) { + if documented(listed, name) { continue } set.Addf(file, 0, "stack.undocumented-dependency", @@ -76,86 +72,32 @@ func Stack(root string) (*finding.Set, error) { return set, nil } -// dependencies collects direct dependencies, mapping each to the manifest that declared -// it. Ecosystems are added here one at a time, each with a parse that cannot be wrong. -func dependencies(root string) (map[string]string, error) { - deps := map[string]string{} - if err := goModDependencies(root, deps); err != nil { - return nil, err - } - if err := packageJSONDependencies(root, deps); err != nil { - return nil, err - } - return deps, nil -} - -// goModDependencies reads the require blocks of go.mod by hand. +// documented reports whether stack.md names this dependency in any spelling somebody +// would plausibly have written it down as. // -// By hand rather than with golang.org/x/mod: scc is stdlib-only, the grammar in play -// here is two forms of one directive, and a dependency added to read a dependency file -// would be its own punchline. -func goModDependencies(root string, deps map[string]string) error { - path := filepath.Join(root, "go.mod") - b, err := os.ReadFile(path) - if err != nil { - if os.IsNotExist(err) { - return nil - } - return err - } - inBlock := false - for _, raw := range strings.Split(textutil.NormalizeNewlines(string(b)), "\n") { - line := strings.TrimSpace(raw) - // An indirect dependency is not a decision anybody made. - if line == "" || strings.HasPrefix(line, "//") || strings.Contains(line, "// indirect") { - continue - } - switch { - case line == "require (": - inBlock = true - continue - case inBlock && line == ")": - inBlock = false - continue - } - fields := strings.Fields(line) - switch { - case inBlock && len(fields) >= 2: - deps[fields[0]] = "go.mod" - case !inBlock && len(fields) >= 3 && fields[0] == "require": - deps[fields[1]] = "go.mod" - } - } - return nil -} - -// packageJSONDependencies reads dependencies and devDependencies with encoding/json. -// Both count: a test framework is as much an adopted decision as a web server, and it is -// as much of a supply-chain surface. -func packageJSONDependencies(root string, deps map[string]string) error { - path := filepath.Join(root, "package.json") - b, err := os.ReadFile(path) - if err != nil { - if os.IsNotExist(err) { - return nil - } - return err - } - var manifest struct { - Dependencies map[string]string `json:"dependencies"` - DevDependencies map[string]string `json:"devDependencies"` - } - if err := json.Unmarshal(b, &manifest); err != nil { - // A manifest scc cannot parse produces no findings. It is not scc's place to - // report on the syntax of somebody else's build file. - return nil - } - for _, group := range []map[string]string{manifest.Dependencies, manifest.DevDependencies} { - for name := range group { - deps[name] = "package.json" +// The full path or its last segment: writing "chi" for github.com/go-chi/chi documents +// the decision, and insisting on the module path would be the validator enforcing a +// spelling nobody agreed to. The same goes for the separator — PyPI treats +// "Flask-SQLAlchemy" and "flask_sqlalchemy" as one project, and Maven artifacts are +// written both ways — so each candidate is also tried with the separators swapped. +// +// Every extra spelling can only remove a finding, never add one. That asymmetry is the +// reason this is allowed to be generous: the cost of a miss here is a dependency +// nobody was reminded to document, and the cost of a false positive is the user +// learning to disbelieve the validator. +func documented(listed, name string) bool { + for _, candidate := range []string{name, baseName(name)} { + for _, spelling := range []string{ + candidate, + strings.ReplaceAll(candidate, "_", "-"), + strings.ReplaceAll(candidate, "-", "_"), + } { + if spelling != "" && strings.Contains(listed, strings.ToLower(spelling)) { + return true + } } } - return nil + return false } // baseName is the name a human would write a dependency down as: the last path segment, diff --git a/internal/validate/stack_manifests.go b/internal/validate/stack_manifests.go new file mode 100644 index 0000000..60eb399 --- /dev/null +++ b/internal/validate/stack_manifests.go @@ -0,0 +1,448 @@ +package validate + +import ( + "encoding/json" + "encoding/xml" + "os" + "path/filepath" + "regexp" + "strings" + + "github.com/protonspy/spec-claude-code/internal/textutil" +) + +// This file is the answer to one question: what does scc consider a *declared +// dependency*? It sits beside stack.go rather than inside it because the rule and the +// readers age differently — the rule is fixed, and an ecosystem is added here roughly +// whenever somebody uses one scc cannot see. +// +// Every reader in here obeys the same three rules, and they are what keep the stack +// check from becoming a source of wrong findings: +// +// 1. **An absent manifest is not an error.** Most projects have exactly one of these +// files, so every reader returns nil when its file is not there. +// 2. **Direct dependencies only.** A transitive dependency is not a decision anybody +// made, and reporting the closure would bury the one line that matters. +// 3. **What cannot be read confidently is not read at all.** A malformed file, an +// unrecognized shape, an unresolved variable — each produces no dependency rather +// than a guessed one. Silence is the correct output of a check that cannot be +// sure, and it is also why a project in an ecosystem with no reader here is +// simply not checked instead of being reported as undocumented. +// +// Rule 3 is why Gemfile, mix.exs, Package.swift and build.gradle are absent: those +// manifests are executable code, and reading them honestly would mean evaluating +// them. + +// dependencies collects direct dependencies, mapping each to the manifest that +// declared it. A project may legitimately have several — a Python service with a +// package.json for its front end is one repo with two decisions to record. +func dependencies(root string) (map[string]string, error) { + deps := map[string]string{} + for _, read := range []func(string, map[string]string) error{ + goModDependencies, + packageJSONDependencies, + requirementsDependencies, + pyprojectDependencies, + cargoDependencies, + composerDependencies, + pomDependencies, + } { + if err := read(root, deps); err != nil { + return nil, err + } + } + return deps, nil +} + +// manifest reads one dependency file, reporting whether it is there at all. The +// content is LF-normalized, so a checkout with CRLF endings parses identically. +func manifest(root, name string) (string, bool, error) { + b, err := os.ReadFile(filepath.Join(root, name)) + if err != nil { + if os.IsNotExist(err) { + return "", false, nil + } + return "", false, err + } + return textutil.NormalizeNewlines(string(b)), true, nil +} + +// goModDependencies reads the require blocks of go.mod by hand. +// +// By hand rather than with golang.org/x/mod: scc is stdlib-only, the grammar in play +// here is two forms of one directive, and a dependency added to read a dependency file +// would be its own punchline. +func goModDependencies(root string, deps map[string]string) error { + text, ok, err := manifest(root, "go.mod") + if err != nil || !ok { + return err + } + inBlock := false + for _, raw := range strings.Split(text, "\n") { + line := strings.TrimSpace(raw) + // An indirect dependency is not a decision anybody made. + if line == "" || strings.HasPrefix(line, "//") || strings.Contains(line, "// indirect") { + continue + } + switch { + case line == "require (": + inBlock = true + continue + case inBlock && line == ")": + inBlock = false + continue + } + fields := strings.Fields(line) + switch { + case inBlock && len(fields) >= 2: + deps[fields[0]] = "go.mod" + case !inBlock && len(fields) >= 3 && fields[0] == "require": + deps[fields[1]] = "go.mod" + } + } + return nil +} + +// packageJSONDependencies reads dependencies and devDependencies with encoding/json. +// Both count: a test framework is as much an adopted decision as a web server, and it is +// as much of a supply-chain surface. +func packageJSONDependencies(root string, deps map[string]string) error { + text, ok, err := manifest(root, "package.json") + if err != nil || !ok { + return err + } + var doc struct { + Dependencies map[string]string `json:"dependencies"` + DevDependencies map[string]string `json:"devDependencies"` + } + if err := json.Unmarshal([]byte(text), &doc); err != nil { + // A manifest scc cannot parse produces no findings. It is not scc's place to + // report on the syntax of somebody else's build file. + return nil + } + for _, group := range []map[string]string{doc.Dependencies, doc.DevDependencies} { + for name := range group { + deps[name] = "package.json" + } + } + return nil +} + +// composerDependencies reads require and require-dev from composer.json, on the same +// terms as package.json. +// +// Platform requirements are dropped: "php", "hhvm", and the ext-/lib- prefixes are +// statements about the interpreter the code runs on, not packages anybody adopted, and +// a finding demanding a stack.md entry for "ext-json" is noise. +func composerDependencies(root string, deps map[string]string) error { + text, ok, err := manifest(root, "composer.json") + if err != nil || !ok { + return err + } + var doc struct { + Require map[string]string `json:"require"` + RequireDev map[string]string `json:"require-dev"` + } + if err := json.Unmarshal([]byte(text), &doc); err != nil { + return nil + } + for _, group := range []map[string]string{doc.Require, doc.RequireDev} { + for name := range group { + if isPlatformRequirement(name) { + continue + } + deps[name] = "composer.json" + } + } + return nil +} + +func isPlatformRequirement(name string) bool { + switch strings.ToLower(name) { + case "php", "php-64bit", "php-ipv6", "hhvm", "composer", "composer-plugin-api", "composer-runtime-api": + return true + } + lower := strings.ToLower(name) + return strings.HasPrefix(lower, "ext-") || strings.HasPrefix(lower, "lib-") +} + +// requirementsDependencies reads requirements.txt, one requirement per line. +// +// Only the file with that exact name, at the root. requirements-dev.txt, +// requirements/base.txt, and a dozen other layouts all exist; globbing for them would +// be scc guessing which files in somebody's repo are dependency manifests, and a wrong +// guess produces findings about a file that was never a manifest. +func requirementsDependencies(root string, deps map[string]string) error { + text, ok, err := manifest(root, "requirements.txt") + if err != nil || !ok { + return err + } + // A trailing backslash continues the requirement on the next line; joining first + // means a continuation is never mistaken for a requirement of its own. + for _, raw := range strings.Split(strings.ReplaceAll(text, "\\\n", " "), "\n") { + line := strings.TrimSpace(stripHashComment(raw)) + // An option line: -r, -e, -c, --index-url, --hash. Not a requirement, and -r + // deliberately does not recurse — that file is a manifest scc was not pointed at. + if line == "" || strings.HasPrefix(line, "-") { + continue + } + if name := requirementName(line); name != "" { + deps[name] = "requirements.txt" + } + } + return nil +} + +// requirementName pulls the project name off a PEP 508 requirement, or returns "" for +// anything that is not one — a bare URL, a VCS reference, a line scc does not +// recognize. Everything after the name is a version specifier, an extras list, a +// marker, or a direct URL, and all four start with a character that cannot appear in a +// name. +func requirementName(spec string) string { + if i := strings.IndexAny(spec, ";@[("); i >= 0 { + spec = spec[:i] + } + if i := strings.IndexAny(spec, "=<>!~ \t"); i >= 0 { + spec = spec[:i] + } + spec = strings.TrimSpace(spec) + if !pep508NameRe.MatchString(spec) { + return "" + } + return spec +} + +// pep508NameRe is the name grammar PyPI actually enforces: letters, digits, and the +// three separators, starting with an alphanumeric. +var pep508NameRe = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9._-]*$`) + +// pyprojectDependencies reads the four places a Python project declares a direct +// dependency: PEP 621's [project] dependencies and optional-dependencies, PEP 735's +// dependency-groups, and Poetry's own tables. +// +// Poetry's "python" key is skipped — it constrains the interpreter, not the project's +// technology choices, and it is the one key in that table that is never a package. +func pyprojectDependencies(root string, deps map[string]string) error { + text, ok, err := manifest(root, "pyproject.toml") + if err != nil || !ok { + return err + } + tables := tomlTables(strings.Split(text, "\n")) + add := func(specs []string) { + for _, spec := range specs { + if name := requirementName(spec); name != "" { + deps[name] = "pyproject.toml" + } + } + } + add(tomlArray(tables["project"], "dependencies")) + for _, group := range []string{"project.optional-dependencies", "dependency-groups"} { + for _, key := range tomlKeys(tables[group]) { + add(tomlArray(tables[group], key)) + } + } + for table, lines := range tables { + if !isPoetryDependencyTable(table) { + continue + } + for _, key := range tomlKeys(lines) { + if key == "python" || !pep508NameRe.MatchString(key) { + continue + } + deps[key] = "pyproject.toml" + } + } + return nil +} + +// isPoetryDependencyTable matches [tool.poetry.dependencies] and the per-group +// [tool.poetry.group..dependencies]. +func isPoetryDependencyTable(table string) bool { + return table == "tool.poetry.dependencies" || + (strings.HasPrefix(table, "tool.poetry.group.") && strings.HasSuffix(table, ".dependencies")) +} + +// cargoDependencies reads Cargo's dependency tables: the three kinds, their +// per-target variants, a workspace's shared table, and the [dependencies.] +// long form. +func cargoDependencies(root string, deps map[string]string) error { + text, ok, err := manifest(root, "Cargo.toml") + if err != nil || !ok { + return err + } + for table, lines := range tomlTables(strings.Split(text, "\n")) { + kind, crate := cargoTable(table) + if !kind { + continue + } + // [dependencies.serde] names the crate in the header; a plain table names one + // per line. + if crate != "" { + if isCrateName(crate) { + deps[crate] = "Cargo.toml" + } + continue + } + for _, key := range tomlKeys(lines) { + if isCrateName(key) { + deps[key] = "Cargo.toml" + } + } + } + return nil +} + +// cargoTable reports whether a table header declares dependencies, and the single +// crate it names when it is the [dependencies.] long form. +func cargoTable(table string) (bool, string) { + // [target.'cfg(unix)'.dependencies] and [workspace.dependencies] both end in a + // dependency table; what precedes it only says when it applies. + for _, kind := range []string{"dependencies", "dev-dependencies", "build-dependencies"} { + switch { + case table == kind, strings.HasSuffix(table, "."+kind): + return true, "" + } + if i := strings.Index(table, kind+"."); i >= 0 && (i == 0 || table[i-1] == '.') { + crate := table[i+len(kind)+1:] + // A crate's own sub-table, e.g. [dependencies.serde.features], names the + // crate first. + if j := strings.Index(crate, "."); j >= 0 { + crate = crate[:j] + } + return true, crate + } + } + return false, "" +} + +var crateNameRe = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9_-]*$`) + +func isCrateName(s string) bool { return crateNameRe.MatchString(s) } + +// pomDependencies reads Maven's direct dependencies with encoding/xml. +// +// The path is deliberately project/dependencies/dependency and nothing else, which +// excludes two things on purpose: , which pins versions for +// modules rather than adopting anything, and , whose dependencies apply only +// when a profile the build was not necessarily run with is active. An artifactId that +// still carries an unresolved ${property} is skipped for the same reason a malformed +// manifest is — scc does not resolve Maven properties, so it does not know the name. +func pomDependencies(root string, deps map[string]string) error { + text, ok, err := manifest(root, "pom.xml") + if err != nil || !ok { + return err + } + var pom struct { + Dependencies []struct { + ArtifactID string `xml:"artifactId"` + } `xml:"dependencies>dependency"` + } + if err := xml.Unmarshal([]byte(text), &pom); err != nil { + return nil + } + for _, d := range pom.Dependencies { + name := strings.TrimSpace(d.ArtifactID) + if name == "" || strings.Contains(name, "${") { + continue + } + deps[name] = "pom.xml" + } + return nil +} + +// tomlTables groups a TOML file's lines by the table they are under, keyed by the +// header without its brackets. Lines before the first header belong to "". +// +// **This is not a TOML parser and must not become one.** It recognizes exactly two +// shapes — a table header alone on a line, and `key = value` under it — which is how +// every dependency table in the wild is written. Everything else passes through as a +// line nothing asks a question about, so an exotic file yields no dependencies rather +// than wrong ones. Array-of-table headers ([[x]]) are treated as their table, which is +// right for the only thing that matters here: they never hold dependencies. +func tomlTables(lines []string) map[string][]string { + tables := map[string][]string{} + current := "" + for _, raw := range lines { + line := strings.TrimSpace(stripHashComment(raw)) + if line == "" { + continue + } + if strings.HasPrefix(line, "[") && strings.HasSuffix(line, "]") { + current = strings.Trim(line, "[]") + // A quoted key in a header — [target.'cfg(unix)'.dependencies] — keeps its + // quotes, which is fine: nothing here matches on that segment. + continue + } + tables[current] = append(tables[current], line) + } + return tables +} + +// tomlKeys returns the left-hand side of every `key = value` line in a table, with +// quotes stripped. +func tomlKeys(lines []string) []string { + var keys []string + for _, line := range lines { + key, _, ok := strings.Cut(line, "=") + if !ok { + continue + } + key = strings.Trim(strings.TrimSpace(key), `"'`) + if key != "" { + keys = append(keys, key) + } + } + return keys +} + +// tomlArray returns the quoted strings of `key = [...]`, spanning lines until the +// bracket closes. An array that never closes yields nothing — the file is not one this +// reader understands, and half of a dependency list is worse than none. +func tomlArray(lines []string, key string) []string { + for i, line := range lines { + name, value, ok := strings.Cut(line, "=") + if !ok || strings.Trim(strings.TrimSpace(name), `"'`) != key { + continue + } + value = strings.TrimSpace(value) + if !strings.HasPrefix(value, "[") { + return nil + } + var out []string + for j := i; j < len(lines); j++ { + text := lines[j] + if j == i { + text = value + } + out = append(out, tomlStringRe.FindAllString(text, -1)...) + if strings.Contains(text, "]") { + return unquote(out) + } + } + return nil + } + return nil +} + +// tomlStringRe matches a basic or literal TOML string. Dependency specifiers contain +// neither escapes nor newlines, which is what makes this safe to match with a regexp +// instead of a scanner. +var tomlStringRe = regexp.MustCompile(`"[^"]*"|'[^']*'`) + +func unquote(in []string) []string { + out := make([]string, 0, len(in)) + for _, s := range in { + out = append(out, strings.Trim(s, `"'`)) + } + return out +} + +// stripHashComment drops a # comment. A # inside a quoted string would be cut too, +// which is why this is only ever applied to dependency declarations: a version +// specifier, a crate name, and a PEP 508 requirement all cannot contain one. +func stripHashComment(line string) string { + if i := strings.Index(line, "#"); i >= 0 { + return line[:i] + } + return line +} diff --git a/internal/validate/stack_manifests_test.go b/internal/validate/stack_manifests_test.go new file mode 100644 index 0000000..6729df0 --- /dev/null +++ b/internal/validate/stack_manifests_test.go @@ -0,0 +1,255 @@ +package validate + +import ( + "path/filepath" + "sort" + "strings" + "testing" +) + +// declared runs the readers over a temp root holding one manifest and returns the +// dependency names it found, sorted. Asserting on the reader rather than on Stack's +// findings keeps these tests about the one thing that can be wrong here: what counts +// as a declared dependency. +func declared(t *testing.T, name, content string) []string { + t.Helper() + root := t.TempDir() + write(t, filepath.Join(root, name), content) + deps, err := dependencies(root) + if err != nil { + t.Fatalf("dependencies: %v", err) + } + var got []string + for dep, from := range deps { + if from != name { + t.Errorf("%s was attributed to %q, want %q", dep, from, name) + } + got = append(got, dep) + } + sort.Strings(got) + return got +} + +func want(t *testing.T, got []string, want ...string) { + t.Helper() + if strings.Join(got, ",") != strings.Join(want, ",") { + t.Errorf("dependencies = %v, want %v", got, want) + } +} + +// A requirements.txt carries more non-requirements than requirements: options, +// comments, markers, extras, and direct URLs. Every one of them has to fall out, or the +// validator reports a finding about "-r" and is never trusted again. +func TestRequirementsTxt(t *testing.T) { + got := declared(t, "requirements.txt", `# production +requests>=2.31.0 +Flask-SQLAlchemy==3.1.1 +uvicorn[standard]~=0.30 # extras are not part of the name +pytest ; python_version >= "3.9" +django \ + >= 5.0 +-r dev-requirements.txt +-e . +--index-url https://example.invalid/simple +https://example.invalid/pkg.tar.gz +mypkg @ git+https://example.invalid/mypkg.git@v1 +`) + want(t, got, "Flask-SQLAlchemy", "django", "mypkg", "pytest", "requests", "uvicorn") +} + +// PEP 621, PEP 735 and Poetry are three ways of saying the same thing, and a project +// uses whichever its tooling chose. Reading only one of them would leave a whole +// dependency list unchecked while reporting nothing, which is the worst shape of miss. +func TestPyprojectPEP621(t *testing.T) { + got := declared(t, "pyproject.toml", `[build-system] +requires = ["hatchling"] + +[project] +name = "app" +dependencies = [ + "httpx>=0.27", # multi-line array + "pydantic[email]>=2", +] + +[project.optional-dependencies] +dev = ["pytest", "ruff"] + +[dependency-groups] +docs = ["mkdocs"] +`) + // hatchling is under [build-system], which builds the package rather than being a + // dependency of it — and reading it would put every build backend in stack.md. + want(t, got, "httpx", "mkdocs", "pydantic", "pytest", "ruff") +} + +func TestPyprojectPoetry(t *testing.T) { + got := declared(t, "pyproject.toml", `[tool.poetry] +name = "app" + +[tool.poetry.dependencies] +python = "^3.12" +requests = "^2.31" +sqlalchemy = { version = "^2.0", extras = ["asyncio"] } + +[tool.poetry.group.dev.dependencies] +pytest = "^8.0" + +[tool.poetry.scripts] +app = "app.main:run" +`) + // python constrains the interpreter and [tool.poetry.scripts] declares an entry + // point; neither is a package anybody adopted. + want(t, got, "pytest", "requests", "sqlalchemy") +} + +func TestCargoToml(t *testing.T) { + got := declared(t, "Cargo.toml", `[package] +name = "app" + +[dependencies] +serde = { version = "1", features = ["derive"] } +tokio = "1" + +[dev-dependencies] +criterion = "0.5" + +[build-dependencies] +cc = "1" + +[target.'cfg(unix)'.dependencies] +nix = "0.29" + +[dependencies.reqwest] +version = "0.12" +features = ["json"] +`) + want(t, got, "cc", "criterion", "nix", "reqwest", "serde", "tokio") +} + +// The long form's sub-tables name the crate once, in the header. Reading their keys as +// crates would turn every feature list into a dependency. +func TestCargoLongFormSubTableIsNotACrate(t *testing.T) { + got := declared(t, "Cargo.toml", `[dependencies.serde] +version = "1" + +[dependencies.serde.features] +derive = true +`) + want(t, got, "serde") +} + +func TestComposerJSON(t *testing.T) { + got := declared(t, "composer.json", `{ + "require": { + "php": ">=8.2", + "ext-json": "*", + "monolog/monolog": "^3.0" + }, + "require-dev": {"phpunit/phpunit": "^11.0"} +}`) + // The interpreter and its extensions are what the code runs on, not decisions to + // record — a finding demanding a stack.md entry for ext-json is noise. + want(t, got, "monolog/monolog", "phpunit/phpunit") +} + +// dependencyManagement pins versions for modules that may never depend on the artifact, +// and a profile's dependencies apply only when that profile is active. Reporting either +// would demand stack.md entries for technology the build does not necessarily use. +func TestPomXMLReadsOnlyTheDirectDependencies(t *testing.T) { + got := declared(t, "pom.xml", ` + + + + com.fasterxml.jackson + jackson-bom + + + + + + org.springframework + spring-core + + + org.junit.jupiter + junit-jupiter + test + + + com.example + ${unresolved.artifact} + + + + + + + com.example + only-under-a-profile + + + + + +`) + // A test-scoped dependency stays: it is as much an adopted decision, and as much of + // a supply-chain surface, as anything that ships. + want(t, got, "junit-jupiter", "spring-core") +} + +// Rule 3 of the readers, one case per shape: a file scc cannot read produces no +// dependencies rather than guessed ones. +func TestManifestsThatCannotBeReadProduceNothing(t *testing.T) { + cases := map[string]string{ + "composer.json": "{ not json", + "pom.xml": "", + "pyproject.toml": "[project]\ndependencies = [\n \"httpx\",\n", // array never closes + } + for name, content := range cases { + if got := declared(t, name, content); len(got) != 0 { + t.Errorf("%s: dependencies = %v, want none", name, got) + } + } +} + +// The whole reason a language without a reader is safe: nothing is declared, so Stack +// returns before it ever asks whether docs/stack.md exists. A Ruby, Elixir or Swift +// project must pass `scc validate` untouched, not fail on a manifest scc never +// understood. +func TestAnEcosystemWithNoReaderIsNotChecked(t *testing.T) { + root := t.TempDir() + write(t, filepath.Join(root, "Gemfile"), "source 'https://rubygems.org'\ngem 'rails', '~> 7.1'\n") + write(t, filepath.Join(root, "mix.exs"), "defp deps do\n [{:phoenix, \"~> 1.7\"}]\nend\n") + write(t, filepath.Join(root, "build.gradle"), "dependencies { implementation 'com.google.guava:guava:33.0.0' }\n") + + if got := runValidator(t, Stack, root); len(got) != 0 { + t.Errorf("rules = %v, want silence for an ecosystem with no reader", got) + } +} + +// A repo can hold two ecosystems, and each dependency has to be reported against the +// file that declared it — "requests is declared in package.json" would send the reader +// to the wrong file. +func TestSeveralManifestsInOneRepo(t *testing.T) { + root := t.TempDir() + write(t, filepath.Join(root, "requirements.txt"), "requests\n") + write(t, filepath.Join(root, "package.json"), `{"dependencies": {"react": "^18"}}`) + deps, err := dependencies(root) + if err != nil { + t.Fatalf("dependencies: %v", err) + } + if deps["requests"] != "requirements.txt" || deps["react"] != "package.json" { + t.Errorf("dependencies = %v, want each attributed to its own manifest", deps) + } +} + +// PyPI treats these as one project name, so a glossary of spellings is not the user's +// problem to solve before the validator will believe them. +func TestStackAcceptsEitherSeparatorSpelling(t *testing.T) { + root := t.TempDir() + write(t, filepath.Join(root, "requirements.txt"), "Flask-SQLAlchemy==3.1.1\n") + write(t, filepath.Join(root, "docs", "stack.md"), "# Stack\n\n- flask_sqlalchemy — the ORM binding\n") + if got := runValidator(t, Stack, root); len(got) != 0 { + t.Errorf("rules = %v, want the underscore spelling to count as documented", got) + } +}