diff --git a/CHANGELOG.md b/CHANGELOG.md index a9c1813..77de154 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -67,6 +67,10 @@ the same version in lockstep. ### Fixed +- **CLI `scan` rejected the `./...` path every doc example uses**, failing with + `scan failed: lstat ./...: no such file or directory`. The scan has always + been recursive, so the pattern suffix is now trimmed and `./pkg/...` selects + exactly what `./pkg` does. - **CLI `explain` could not connect to any database**: the binary linked no SQL driver, so every invocation failed with `sql: unknown driver "postgres" (forgotten import?)`. `cmd/sqlguard` now diff --git a/cmd/sqlguard/scan.go b/cmd/sqlguard/scan.go index a6c42b5..b4efd6b 100644 --- a/cmd/sqlguard/scan.go +++ b/cmd/sqlguard/scan.go @@ -37,9 +37,11 @@ var formatFlag string var scanCmd = &cobra.Command{ Use: "scan [path]", Short: "Scan Go source files for SQL query issues", - Long: "Statically analyzes Go source files to find SQL queries and check them for common issues.", - Args: cobra.MaximumNArgs(1), - RunE: runScan, + Long: "Statically analyzes Go source files to find SQL queries and check them for common issues.\n\n" + + "The scan is always recursive. A path may be written either plainly (./pkg)\n" + + "or with the Go package-pattern suffix (./pkg/...); both select the same files.", + Args: cobra.MaximumNArgs(1), + RunE: runScan, } func init() { @@ -56,7 +58,7 @@ func runScan(cmd *cobra.Command, args []string) error { dir := "." if len(args) > 0 { - dir = args[0] + dir = trimPatternSuffix(args[0]) } rep, err := newReporter(formatFlag) @@ -97,6 +99,43 @@ func runScan(cmd *cobra.Command, args []string) error { return nil } +// trimPatternSuffix accepts the `./...` spelling every Go tool takes. The scan +// is already recursive, so `dir/...` selects exactly what `dir` does and the +// suffix only has to be removed before the path reaches the filesystem. +func trimPatternSuffix(path string) string { + return trimPatternSuffixSep(path, filepath.Separator) +} + +// trimPatternSuffixSep takes the separator explicitly so both platforms' +// behavior is testable from either one. +// +// The suffix must be separator-anchored: a directory really named `weird...` +// is a legal path, and trimming it unanchored would silently scan `weird` +// instead and report a clean exit for a tree that was never looked at. +// +// `\...` counts only where the OS separator is a backslash. The go command +// rewrites `\` to `/` in relative arguments "as a courtesy to Windows +// developers" (cmd/go/internal/search), so `.\...` is a spelling users do +// type — but on Unix a backslash is an ordinary filename byte, and a directory +// named `weird\...` there must reach the filesystem intact. +func trimPatternSuffixSep(path string, sep rune) string { + if path == "..." { + return "." + } + trimmed, ok := strings.CutSuffix(path, "/...") + if !ok && sep == '\\' { + trimmed, ok = strings.CutSuffix(path, `\...`) + } + if !ok { + return path + } + // A bare root ("/...", or "C:\..." on Windows) loses its separator above. + if trimmed == "" || strings.HasSuffix(trimmed, ":") { + return trimmed + string(sep) + } + return trimmed +} + func newReporter(format string) (reporter.Reporter, error) { switch format { case "json": diff --git a/cmd/sqlguard/scan_test.go b/cmd/sqlguard/scan_test.go index 7cbafc8..a330b30 100644 --- a/cmd/sqlguard/scan_test.go +++ b/cmd/sqlguard/scan_test.go @@ -336,6 +336,13 @@ func f(db *sql.DB) { // Returns the output and the error (errIssuesFound if issues were found). func captureScanOutput(t *testing.T, dir string) (string, error) { t.Helper() + return captureScanTarget(t, dir) +} + +// captureScanTarget is captureScanOutput for a target that is not a plain +// directory path, such as the `./...` package-pattern spelling. +func captureScanTarget(t *testing.T, target string) (string, error) { + t.Helper() // Reset format flag to default for each test formatFlag = "console" @@ -344,7 +351,7 @@ func captureScanOutput(t *testing.T, dir string) (string, error) { r, w, _ := os.Pipe() os.Stderr = w - err := runScan(&cobra.Command{}, []string{dir}) + err := runScan(&cobra.Command{}, []string{target}) w.Close() os.Stderr = old @@ -397,3 +404,153 @@ func TestScanCommand_NoUsageDumpOnIssues(t *testing.T) { t.Errorf("expected the finding in output, got:\n%s", out) } } + +func TestTrimPatternSuffix(t *testing.T) { + // sep is the OS separator the call is evaluated under, so Windows + // behavior is covered from a Unix host and vice versa. + cases := []struct { + sep rune + in, want string + }{ + // Slash patterns work on every platform. + {'/', "./...", "."}, + {'/', "...", "."}, + {'/', "/...", "/"}, + {'/', "./pkg/...", "./pkg"}, + {'/', "pkg/...", "pkg"}, + {'/', "/abs/pkg/...", "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/abs/pkg"}, + {'\\', "./...", "."}, + {'\\', "...", "."}, + {'\\', "./pkg/...", "./pkg"}, + + // Plain paths are untouched. + {'/', ".", "."}, + {'/', "./pkg", "./pkg"}, + {'/', "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/abs/pkg", "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/abs/pkg"}, + {'/', "", ""}, + + // Separator-anchored: directory names, not patterns. + {'/', "weird...", "weird..."}, + {'/', "./weird...", "./weird..."}, + {'/', "....", "...."}, + {'/', "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/abs/weird...", "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/abs/weird..."}, + + // A backslash is an ordinary filename byte on Unix, so `weird\...` + // and `.\...` are directory names there — but patterns on Windows. + {'/', `.\...`, `.\...`}, + {'/', `weird\...`, `weird\...`}, + {'/', `./a\...`, `./a\...`}, + {'\\', `.\...`, "."}, + {'\\', `.\pkg\...`, `.\pkg`}, + {'\\', `pkg\...`, "pkg"}, + {'\\', `weird...`, `weird...`}, + + // Bare roots keep their separator. + {'\\', `C:\...`, `C:\`}, + {'\\', `\...`, `\`}, + } + for _, c := range cases { + if got := trimPatternSuffixSep(c.in, c.sep); got != c.want { + t.Errorf("trimPatternSuffixSep(%q, %q) = %q, want %q", c.in, c.sep, got, c.want) + } + } +} + +// TestScan_AcceptsPackagePattern pins the `./...` spelling the README and the +// docs site use. It used to reach filepath.Abs verbatim and fail with +// "lstat ./...: no such file or directory", so the documented invocation was +// the one invocation that did not work. The nested file also proves the +// pattern still reaches subdirectories rather than silently scanning one level. +func TestScan_AcceptsPackagePattern(t *testing.T) { + dir := t.TempDir() + createTestFile(t, dir, "top.go", `package example +import "database/sql" +func f(db *sql.DB) { + db.Query("SELECT * FROM users") +} +`) + sub := filepath.Join(dir, "nested") + if err := os.Mkdir(sub, 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + createTestFile(t, sub, "deep.go", `package nested +import "database/sql" +func g(db *sql.DB) { + db.Exec("DELETE FROM sessions") +} +`) + + out, err := captureScanTarget(t, filepath.Join(dir, "...")) + + if !errors.Is(err, errIssuesFound) { + t.Fatalf("expected errIssuesFound for ./... target, got %v\noutput:\n%s", err, out) + } + if !strings.Contains(out, "select-star") { + t.Errorf("pattern target missed the top-level file, got:\n%s", out) + } + if !strings.Contains(out, "delete-without-where") { + t.Errorf("pattern target did not recurse into the subdirectory, got:\n%s", out) + } +} + +// TestScan_PatternMatchesPlainPath is the equivalence the fix rests on: the +// scan is already recursive, so `dir/...` must select exactly what `dir` does. +func TestScan_PatternMatchesPlainPath(t *testing.T) { + dir := t.TempDir() + createTestFile(t, dir, "a.go", `package example +import "database/sql" +func f(db *sql.DB) { + db.Query("SELECT * FROM users") +} +`) + + plain, errPlain := captureScanTarget(t, dir) + pattern, errPattern := captureScanTarget(t, filepath.Join(dir, "...")) + + if !errors.Is(errPlain, errIssuesFound) || !errors.Is(errPattern, errIssuesFound) { + t.Fatalf("both spellings should report issues: plain=%v pattern=%v", errPlain, errPattern) + } + if plain != pattern { + t.Errorf("plain and pattern targets disagree:\nplain:\n%s\npattern:\n%s", plain, pattern) + } +} + +// TestScan_DottedDirectoryIsNotAPattern guards the separator anchor. A +// directory named `weird...` is a legal path; trimming the suffix unanchored +// pointed the scan at a sibling `weird` instead, which reported that +// directory's findings under the name the user did not ask for — and would +// have exited clean had the sibling been clean. +func TestScan_DottedDirectoryIsNotAPattern(t *testing.T) { + root := t.TempDir() + dotted := filepath.Join(root, "weird...") + plain := filepath.Join(root, "weird") + for _, d := range []string{dotted, plain} { + if err := os.Mkdir(d, 0o755); err != nil { + t.Fatalf("mkdir %s: %v", d, err) + } + } + createTestFile(t, dotted, "a.go", `package weird +import "database/sql" +func f(db *sql.DB) { + db.Query("SELECT * FROM users") +} +`) + createTestFile(t, plain, "b.go", `package weird +import "database/sql" +func g(db *sql.DB) { + db.Exec("DELETE FROM sessions") +} +`) + + out, err := captureScanTarget(t, dotted) + + if !errors.Is(err, errIssuesFound) { + t.Fatalf("expected the dotted directory to be scanned, got %v\noutput:\n%s", err, out) + } + if !strings.Contains(out, "select-star") { + t.Errorf("did not scan the directory that was named, got:\n%s", out) + } + if strings.Contains(out, "delete-without-where") { + t.Errorf("scanned the sibling directory instead of the one named, got:\n%s", out) + } +} diff --git a/website/docs/scan.md b/website/docs/scan.md index f52b227..654eb76 100644 --- a/website/docs/scan.md +++ b/website/docs/scan.md @@ -21,6 +21,14 @@ sqlguard scan ./internal/repository # one package tree sqlguard scan --format json ./... # machine-readable ``` +The scan is always recursive, so a path may be written plainly (`./internal`) +or with the Go package-pattern suffix (`./internal/...`); both select the same +files. With no path at all it scans the current directory. + +_Changed in 0.3._ In 0.2 the pattern spelling was rejected outright — +`sqlguard scan ./...` failed with `lstat ./...: no such file or directory` — +so the form used throughout these docs had to be written as `sqlguard scan .`. + | Flag | Default | Effect | | --- | --- | --- | | `--format console\|json` | `console` | Output shape. JSON is an array of `{rule, severity, query, fingerprint, message, suggestion, file, line}`. |