From 390f7b0c6320b5f28b41876d29559e9c3a8bef36 Mon Sep 17 00:00:00 2001 From: kartik Date: Thu, 24 Sep 2026 21:19:09 +0530 Subject: [PATCH 1/2] fix(cli): accept the ./... package pattern in scan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `sqlguard scan ./...` — the form the README quick start, the getting-started guide, the scan page, the intro table and the CI snippet all use — failed with `scan failed: lstat ./...: no such file or directory`. The argument reached filepath.Abs verbatim, so the pattern became a directory that does not exist; packages.Load then failed and the AST fallback tried to walk it. The scan was already recursive on both paths (scanViaPackages loads "./..." relative to the target itself), so `dir/...` selects exactly what `dir` does and trimming the suffix is the whole fix. Pinned by an equivalence test that asserts both spellings produce identical output, and by a nested-file test that proves the pattern still reaches subdirectories. The trim is separator-anchored. A directory really named `weird...` is a legal path, and an unanchored trim would point the scan at a sibling `weird` instead — reporting that tree's findings, or a clean exit, for a directory the user never named. --- CHANGELOG.md | 4 ++ cmd/sqlguard/scan.go | 31 +++++++-- cmd/sqlguard/scan_test.go | 133 +++++++++++++++++++++++++++++++++++++- website/docs/scan.md | 8 +++ 4 files changed, 171 insertions(+), 5 deletions(-) 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..d7d44c1 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,27 @@ 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. +// +// 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. +func trimPatternSuffix(path string) string { + if path != "..." && !strings.HasSuffix(path, "/...") { + return path + } + trimmed := strings.TrimSuffix(path, "...") + switch trimmed { + case "": + return "." + case "/": + return "/" + } + return strings.TrimSuffix(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..a1b9f75 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,127 @@ func TestScanCommand_NoUsageDumpOnIssues(t *testing.T) { t.Errorf("expected the finding in output, got:\n%s", out) } } + +func TestTrimPatternSuffix(t *testing.T) { + cases := []struct{ in, want string }{ + {"./...", "."}, + {"...", "."}, + {"/...", "/"}, + {"./pkg/...", "./pkg"}, + {"pkg/...", "pkg"}, + {"/abs/pkg/...", "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/abs/pkg"}, + {".", "."}, + {"./pkg", "./pkg"}, + {"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/abs/pkg", "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/abs/pkg"}, + {"", ""}, + // Separator-anchored: these are directory names, not patterns. + {"weird...", "weird..."}, + {"./weird...", "./weird..."}, + {"....", "...."}, + {"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/abs/weird...", "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/abs/weird..."}, + } + for _, c := range cases { + if got := trimPatternSuffix(c.in); got != c.want { + t.Errorf("trimPatternSuffix(%q) = %q, want %q", c.in, 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}`. | From e4606b80b0039e874ad0894fdd75c7358e1d7e6f Mon Sep 17 00:00:00 2001 From: kartik Date: Thu, 24 Sep 2026 21:38:16 +0530 Subject: [PATCH 2/2] fix(cli): treat `.\...` as a pattern on Windows only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The go command rewrites `\` to `/` in relative arguments "as a courtesy to Windows developers" (cmd/go/internal/search), so `.\...` is a spelling Windows users do type — and the `/...`-only anchor left it failing exactly as `./...` used to. The trim is guarded on the OS separator rather than applied unconditionally: on Unix a backslash is an ordinary filename byte, so a directory named `weird\...` there must reach the filesystem intact. Trimming it on both platforms would reintroduce the wrong-directory redirect the anchor was added to prevent, just under a different spelling. trimPatternSuffixSep takes the separator so both platforms are covered by the table from either host. Both failure directions are pinned: an unconditional trim fails the three Unix backslash cases, and dropping the branch fails the five Windows ones. --- cmd/sqlguard/scan.go | 34 +++++++++++++++------ cmd/sqlguard/scan_test.go | 62 +++++++++++++++++++++++++++------------ 2 files changed, 69 insertions(+), 27 deletions(-) diff --git a/cmd/sqlguard/scan.go b/cmd/sqlguard/scan.go index d7d44c1..b4efd6b 100644 --- a/cmd/sqlguard/scan.go +++ b/cmd/sqlguard/scan.go @@ -102,22 +102,38 @@ func runScan(cmd *cobra.Command, args []string) error { // 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. -func trimPatternSuffix(path string) string { - if path != "..." && !strings.HasSuffix(path, "/...") { +// +// `\...` 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 } - trimmed := strings.TrimSuffix(path, "...") - switch trimmed { - case "": - return "." - case "/": - return "/" + // A bare root ("/...", or "C:\..." on Windows) loses its separator above. + if trimmed == "" || strings.HasSuffix(trimmed, ":") { + return trimmed + string(sep) } - return strings.TrimSuffix(trimmed, "/") + return trimmed } func newReporter(format string) (reporter.Reporter, error) { diff --git a/cmd/sqlguard/scan_test.go b/cmd/sqlguard/scan_test.go index a1b9f75..a330b30 100644 --- a/cmd/sqlguard/scan_test.go +++ b/cmd/sqlguard/scan_test.go @@ -406,26 +406,52 @@ func TestScanCommand_NoUsageDumpOnIssues(t *testing.T) { } func TestTrimPatternSuffix(t *testing.T) { - cases := []struct{ in, want string }{ - {"./...", "."}, - {"...", "."}, - {"/...", "/"}, - {"./pkg/...", "./pkg"}, - {"pkg/...", "pkg"}, - {"/abs/pkg/...", "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/abs/pkg"}, - {".", "."}, - {"./pkg", "./pkg"}, - {"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/abs/pkg", "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/abs/pkg"}, - {"", ""}, - // Separator-anchored: these are directory names, not patterns. - {"weird...", "weird..."}, - {"./weird...", "./weird..."}, - {"....", "...."}, - {"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/abs/weird...", "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/abs/weird..."}, + // 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 := trimPatternSuffix(c.in); got != c.want { - t.Errorf("trimPatternSuffix(%q) = %q, want %q", c.in, got, c.want) + 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) } } }