From 70c9dfa85c8da56fcb6152a6466d004372de3f2f Mon Sep 17 00:00:00 2001 From: terry-writer Date: Sun, 16 Aug 2026 12:10:15 +0900 Subject: [PATCH 1/4] fix: skip non-regular files matched by glob in model test A glob match that is a FIFO with no writer causes os.ReadFile to block forever. filepath.Glob matches are now filtered to regular files only; explicitly named paths are left untouched. Closes #739 --- cmd/model/test.go | 30 ++++++++++++- cmd/model/test_test.go | 98 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 127 insertions(+), 1 deletion(-) create mode 100644 cmd/model/test_test.go diff --git a/cmd/model/test.go b/cmd/model/test.go index 1ac74956..6b119e36 100644 --- a/cmd/model/test.go +++ b/cmd/model/test.go @@ -87,7 +87,6 @@ var modelTestCmd = &cobra.Command{ } multipleFiles := len(fileNames) > 1 - clientConfig := cmdutils.GetClientConfig(cmd) fgaClient, err := clientConfig.GetFgaClient() @@ -169,6 +168,35 @@ var modelTestCmd = &cobra.Command{ }, } +// resolveTestFiles expands testsPattern via filepath.Glob and filters out any matches that +// are not regular files (e.g. FIFOs, devices, sockets). Reading a FIFO with no writer via +// os.ReadFile blocks forever, so non-regular glob matches are silently skipped rather than +// read. This filtering only applies to glob matches: if the pattern matches nothing, the +// pattern itself is treated as a literal path and returned as-is, untouched by the regular +// file check (e.g. to keep process substitution like --tests <(...) working). + +func resolveTestFiles(testsPattern string) ([]string, error) { + fileNames, err := filepath.Glob(testsPattern) + if err != nil { + return nil, fmt.Errorf("invalid tests pattern %s due to %w", testsPattern, err) + } + + regularFileNames := fileNames[:0] + + for _, name := range fileNames { + info, statErr := os.Stat(name) + if statErr != nil { + return nil, fmt.Errorf("failed to stat test file %s: %w", name, statErr) + } + + if info.Mode().IsRegular() { + regularFileNames = append(regularFileNames, name) + } + } + + return regularFileNames, nil +} + func init() { modelTestCmd.Flags().String("store-id", "", "Store ID") modelTestCmd.Flags().String("model-id", "", "Model ID") diff --git a/cmd/model/test_test.go b/cmd/model/test_test.go new file mode 100644 index 00000000..926143a0 --- /dev/null +++ b/cmd/model/test_test.go @@ -0,0 +1,98 @@ +package model + +import ( + "os" + "path/filepath" + "sort" + "syscall" + "testing" +) + +// Regression test for https://github.com/openfga/cli/issues/739 +// A glob match that is a FIFO with no writer must never be handed to +// storetest.ReadFromFile, since os.ReadFile on such a FIFO blocks forever. +func TestResolveTestFilesSkipsNonRegularGlobMatches(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + + regularPath := filepath.Join(dir, "ok.fga.yaml") + if err := os.WriteFile(regularPath, []byte("name: ok\n"), 0o600); err != nil { + t.Fatalf("failed to write regular test file: %v", err) + } + + fifoPath := filepath.Join(dir, "evil.fga.yaml") + if err := syscall.Mkfifo(fifoPath, 0o600); err != nil { + t.Fatalf("failed to create fifo: %v", err) + } + + pattern := filepath.Join(dir, "*.fga.yaml") + + // This call must return promptly. If the FIFO were not filtered out, resolveTestFiles + // itself doesn't read file contents, but a regression that moved the read here would hang + // the test; test-unit's overall timeout is the real backstop for that case. + fileNames, err := resolveTestFiles(pattern) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(fileNames) != 1 { + t.Fatalf("expected exactly 1 regular file, got %d: %v", len(fileNames), fileNames) + } + + if fileNames[0] != regularPath { + t.Fatalf("expected %s, got %s", regularPath, fileNames[0]) + } +} + +// A pattern with no glob matches at all is treated as a literal path by the caller, not by +// resolveTestFiles itself - so an empty result here is expected and not an error. +func TestResolveTestFilesNoMatches(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + pattern := filepath.Join(dir, "*.fga.yaml") + + fileNames, err := resolveTestFiles(pattern) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(fileNames) != 0 { + t.Fatalf("expected no matches, got %v", fileNames) + } +} + +// Multiple regular files matched by the glob should all be returned, in addition to any +// non-regular files being dropped. +func TestResolveTestFilesMultipleRegularFiles(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + + first := filepath.Join(dir, "a.fga.yaml") + second := filepath.Join(dir, "b.fga.yaml") + + for _, p := range []string{first, second} { + if err := os.WriteFile(p, []byte("name: ok\n"), 0o600); err != nil { + t.Fatalf("failed to write regular test file: %v", err) + } + } + + if err := syscall.Mkfifo(filepath.Join(dir, "c.fga.yaml"), 0o600); err != nil { + t.Fatalf("failed to create fifo: %v", err) + } + + pattern := filepath.Join(dir, "*.fga.yaml") + + fileNames, err := resolveTestFiles(pattern) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + sort.Strings(fileNames) + + if len(fileNames) != 2 || fileNames[0] != first || fileNames[1] != second { + t.Fatalf("expected [%s %s], got %v", first, second, fileNames) + } +} From 78ef08d227e0c17d7a3a08bcbaa2a39020f0b2a1 Mon Sep 17 00:00:00 2001 From: terry-writer Date: Mon, 17 Aug 2026 19:26:19 +0900 Subject: [PATCH 2/4] fix: address review feedback on glob FIFO fix - Wire RunE to actually call resolveTestFiles instead of calling filepath.Glob directly, so the fix takes effect at runtime. - Fold the literal-path fallback into resolveTestFiles and distinguish 'glob matched nothing' from 'glob matched only non-regular files', avoiding double-wrapped errors and a confusing fallback-to-literal-path error message. - Move FIFO-based tests into test_unix_test.go behind //go:build unix, since syscall.Mkfifo does not exist on Windows. --- cmd/model/test.go | 49 +++++++++++------- cmd/model/test_test.go | 88 +++++++++------------------------ cmd/model/test_unix_test.go | 99 +++++++++++++++++++++++++++++++++++++ 3 files changed, 154 insertions(+), 82 deletions(-) create mode 100644 cmd/model/test_unix_test.go diff --git a/cmd/model/test.go b/cmd/model/test.go index 6b119e36..d3b2e8ad 100644 --- a/cmd/model/test.go +++ b/cmd/model/test.go @@ -17,6 +17,7 @@ limitations under the License. package model import ( + "errors" "fmt" "os" "path/filepath" @@ -30,6 +31,11 @@ import ( "github.com/openfga/cli/internal/storetest" ) +// errAllTestFilesNonRegular is returned when a tests glob pattern matches only non-regular +// files (e.g. FIFOs), so there is nothing safe to read. +var errAllTestFilesNonRegular = errors.New("tests pattern matched only non-regular files (e.g. FIFOs); " + + "pass an explicit regular file path instead") + // modelTestCmd represents the test command. var modelTestCmd = &cobra.Command{ Use: "test", @@ -72,21 +78,13 @@ var modelTestCmd = &cobra.Command{ MaxTypesPerAuthorizationModel: maxTypes, } - fileNames, err := filepath.Glob(testsFileName) + fileNames, err := resolveTestFiles(testsFileName) if err != nil { - return fmt.Errorf("invalid tests pattern %s due to %w", testsFileName, err) - } - - if len(fileNames) == 0 { - // Check if the literal path exists - if _, err := os.Stat(testsFileName); err != nil { - return fmt.Errorf("test file %s does not exist: %w", testsFileName, err) - } - - fileNames = []string{testsFileName} + return err } multipleFiles := len(fileNames) > 1 + clientConfig := cmdutils.GetClientConfig(cmd) fgaClient, err := clientConfig.GetFgaClient() @@ -171,19 +169,30 @@ var modelTestCmd = &cobra.Command{ // resolveTestFiles expands testsPattern via filepath.Glob and filters out any matches that // are not regular files (e.g. FIFOs, devices, sockets). Reading a FIFO with no writer via // os.ReadFile blocks forever, so non-regular glob matches are silently skipped rather than -// read. This filtering only applies to glob matches: if the pattern matches nothing, the -// pattern itself is treated as a literal path and returned as-is, untouched by the regular -// file check (e.g. to keep process substitution like --tests <(...) working). - +// read. +// +// If the pattern matches nothing at all, it is treated as a literal path (e.g. to keep +// process substitution like --tests <(...) working) and is not subject to the regular-file +// check. If the pattern matches only non-regular files, that is treated as an error rather +// than silently falling back to the literal-path behavior, since testsPattern itself +// (e.g. "*.fga.yaml") is very unlikely to also be a valid literal path. func resolveTestFiles(testsPattern string) ([]string, error) { - fileNames, err := filepath.Glob(testsPattern) + rawMatches, err := filepath.Glob(testsPattern) if err != nil { return nil, fmt.Errorf("invalid tests pattern %s due to %w", testsPattern, err) } - regularFileNames := fileNames[:0] + if len(rawMatches) == 0 { + if _, statErr := os.Stat(testsPattern); statErr != nil { + return nil, fmt.Errorf("test file %s does not exist: %w", testsPattern, statErr) + } + + return []string{testsPattern}, nil + } - for _, name := range fileNames { + regularFileNames := rawMatches[:0] + + for _, name := range rawMatches { info, statErr := os.Stat(name) if statErr != nil { return nil, fmt.Errorf("failed to stat test file %s: %w", name, statErr) @@ -194,6 +203,10 @@ func resolveTestFiles(testsPattern string) ([]string, error) { } } + if len(regularFileNames) == 0 { + return nil, fmt.Errorf("%w: %s", errAllTestFilesNonRegular, testsPattern) + } + return regularFileNames, nil } diff --git a/cmd/model/test_test.go b/cmd/model/test_test.go index 926143a0..479d6391 100644 --- a/cmd/model/test_test.go +++ b/cmd/model/test_test.go @@ -1,98 +1,58 @@ package model import ( + "fmt" "os" "path/filepath" - "sort" - "syscall" "testing" ) -// Regression test for https://github.com/openfga/cli/issues/739 -// A glob match that is a FIFO with no writer must never be handed to -// storetest.ReadFromFile, since os.ReadFile on such a FIFO blocks forever. -func TestResolveTestFilesSkipsNonRegularGlobMatches(t *testing.T) { - t.Parallel() - - dir := t.TempDir() - - regularPath := filepath.Join(dir, "ok.fga.yaml") - if err := os.WriteFile(regularPath, []byte("name: ok\n"), 0o600); err != nil { - t.Fatalf("failed to write regular test file: %v", err) - } - - fifoPath := filepath.Join(dir, "evil.fga.yaml") - if err := syscall.Mkfifo(fifoPath, 0o600); err != nil { - t.Fatalf("failed to create fifo: %v", err) - } - - pattern := filepath.Join(dir, "*.fga.yaml") - - // This call must return promptly. If the FIFO were not filtered out, resolveTestFiles - // itself doesn't read file contents, but a regression that moved the read here would hang - // the test; test-unit's overall timeout is the real backstop for that case. - fileNames, err := resolveTestFiles(pattern) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } +// writeRegularFile writes a minimal regular test file at path. Shared by both this file and +// test_unix_test.go. +func writeRegularFile(t *testing.T, path string) error { + t.Helper() - if len(fileNames) != 1 { - t.Fatalf("expected exactly 1 regular file, got %d: %v", len(fileNames), fileNames) + if err := os.WriteFile(path, []byte("name: ok\n"), 0o600); err != nil { + return fmt.Errorf("failed to write test file %s: %w", path, err) } - if fileNames[0] != regularPath { - t.Fatalf("expected %s, got %s", regularPath, fileNames[0]) - } + return nil } -// A pattern with no glob matches at all is treated as a literal path by the caller, not by -// resolveTestFiles itself - so an empty result here is expected and not an error. -func TestResolveTestFilesNoMatches(t *testing.T) { +// A pattern with no glob matches at all is treated as a literal path. Since the pattern here +// contains a wildcard and no such literal file exists, resolution should fail with a +// not-found error rather than silently returning an empty result. +func TestResolveTestFilesNoMatchesTreatedAsLiteralPath(t *testing.T) { t.Parallel() dir := t.TempDir() pattern := filepath.Join(dir, "*.fga.yaml") fileNames, err := resolveTestFiles(pattern) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - if len(fileNames) != 0 { - t.Fatalf("expected no matches, got %v", fileNames) + if err == nil { + t.Fatalf("expected an error, got fileNames=%v", fileNames) } } -// Multiple regular files matched by the glob should all be returned, in addition to any -// non-regular files being dropped. -func TestResolveTestFilesMultipleRegularFiles(t *testing.T) { +// An existing literal path (no glob metacharacters, so filepath.Glob returns it as a single +// match) is returned as-is, without the regular-file check rejecting it - this preserves +// existing behavior for explicitly named paths. +func TestResolveTestFilesLiteralExistingPath(t *testing.T) { t.Parallel() dir := t.TempDir() + path := filepath.Join(dir, "ok.fga.yaml") - first := filepath.Join(dir, "a.fga.yaml") - second := filepath.Join(dir, "b.fga.yaml") - - for _, p := range []string{first, second} { - if err := os.WriteFile(p, []byte("name: ok\n"), 0o600); err != nil { - t.Fatalf("failed to write regular test file: %v", err) - } + if err := writeRegularFile(t, path); err != nil { + t.Fatalf("failed to write test file: %v", err) } - if err := syscall.Mkfifo(filepath.Join(dir, "c.fga.yaml"), 0o600); err != nil { - t.Fatalf("failed to create fifo: %v", err) - } - - pattern := filepath.Join(dir, "*.fga.yaml") - - fileNames, err := resolveTestFiles(pattern) + fileNames, err := resolveTestFiles(path) if err != nil { t.Fatalf("unexpected error: %v", err) } - sort.Strings(fileNames) - - if len(fileNames) != 2 || fileNames[0] != first || fileNames[1] != second { - t.Fatalf("expected [%s %s], got %v", first, second, fileNames) + if len(fileNames) != 1 || fileNames[0] != path { + t.Fatalf("expected [%s], got %v", path, fileNames) } } diff --git a/cmd/model/test_unix_test.go b/cmd/model/test_unix_test.go new file mode 100644 index 00000000..0ebd6d11 --- /dev/null +++ b/cmd/model/test_unix_test.go @@ -0,0 +1,99 @@ +//go:build unix + +package model + +import ( + "path/filepath" + "sort" + "syscall" + "testing" +) + +// Regression test for https://github.com/openfga/cli/issues/739 +// A glob match that is a FIFO with no writer must never be handed to +// storetest.ReadFromFile, since os.ReadFile on such a FIFO blocks forever. +// +// This lives in a Unix-only file because syscall.Mkfifo does not exist on Windows, so a +// runtime GOOS skip would not prevent a compile failure there. +func TestResolveTestFilesSkipsNonRegularGlobMatches(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + + regularPath := filepath.Join(dir, "ok.fga.yaml") + if err := writeRegularFile(t, regularPath); err != nil { + t.Fatalf("failed to write regular test file: %v", err) + } + + fifoPath := filepath.Join(dir, "evil.fga.yaml") + if err := syscall.Mkfifo(fifoPath, 0o600); err != nil { + t.Skipf("unable to create FIFO: %v", err) + } + + pattern := filepath.Join(dir, "*.fga.yaml") + + // This call must return promptly. If the FIFO were not filtered out, resolveTestFiles + // itself doesn't read file contents, but a regression that moved the read here would hang + // the test; test-unit's overall timeout is the real backstop for that case. + fileNames, err := resolveTestFiles(pattern) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(fileNames) != 1 || fileNames[0] != regularPath { + t.Fatalf("expected [%s], got %v", regularPath, fileNames) + } +} + +// Multiple regular files matched by the glob should all be returned, in addition to any +// non-regular files being dropped. +func TestResolveTestFilesMultipleRegularFiles(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + + first := filepath.Join(dir, "a.fga.yaml") + second := filepath.Join(dir, "b.fga.yaml") + + for _, p := range []string{first, second} { + if err := writeRegularFile(t, p); err != nil { + t.Fatalf("failed to write regular test file: %v", err) + } + } + + if err := syscall.Mkfifo(filepath.Join(dir, "c.fga.yaml"), 0o600); err != nil { + t.Skipf("unable to create FIFO: %v", err) + } + + pattern := filepath.Join(dir, "*.fga.yaml") + + fileNames, err := resolveTestFiles(pattern) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + sort.Strings(fileNames) + + if len(fileNames) != 2 || fileNames[0] != first || fileNames[1] != second { + t.Fatalf("expected [%s %s], got %v", first, second, fileNames) + } +} + +// If every glob match is non-regular, resolution must fail with a clear error rather than +// silently falling back to treating the glob pattern string itself as a literal path. +func TestResolveTestFilesAllMatchesNonRegular(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + + if err := syscall.Mkfifo(filepath.Join(dir, "evil.fga.yaml"), 0o600); err != nil { + t.Skipf("unable to create FIFO: %v", err) + } + + pattern := filepath.Join(dir, "*.fga.yaml") + + fileNames, err := resolveTestFiles(pattern) + if err == nil { + t.Fatalf("expected an error, got fileNames=%v", fileNames) + } +} From 447f110f90f85c024298e83f65cff16c3bbcd733 Mon Sep 17 00:00:00 2001 From: terry-writer Date: Wed, 19 Aug 2026 22:32:40 +0900 Subject: [PATCH 3/4] test: exercise the real command path and address lint - Add TestModelTestCmdDoesNotHangOnFifoGlobMatch, which runs modelTestCmd.Execute() with a FIFO glob match and asserts it returns within a deadline instead of hanging (per review). - Suppress paralleltest on that test since it mutates the shared global modelTestCmd and must not run in parallel. --- cmd/model/test_unix_test.go | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/cmd/model/test_unix_test.go b/cmd/model/test_unix_test.go index 0ebd6d11..563083c3 100644 --- a/cmd/model/test_unix_test.go +++ b/cmd/model/test_unix_test.go @@ -7,6 +7,7 @@ import ( "sort" "syscall" "testing" + "time" ) // Regression test for https://github.com/openfga/cli/issues/739 @@ -97,3 +98,39 @@ func TestResolveTestFilesAllMatchesNonRegular(t *testing.T) { t.Fatalf("expected an error, got fileNames=%v", fileNames) } } + +// End-to-end regression test through the actual modelTestCmd path (not resolveTestFiles +// directly), per review feedback. Before the fix, a FIFO picked up by the --tests glob made +// the command block forever in os.ReadFile. Here the command must return within a short +// deadline instead of hanging; whatever error it returns afterwards (e.g. from running the +// tests) is irrelevant to this regression. +func TestModelTestCmdDoesNotHangOnFifoGlobMatch(t *testing.T) { //nolint:paralleltest // mutates the shared global modelTestCmd, so it must not run in parallel // Not parallel: modelTestCmd is a shared global and this test sets flags/args on it. + dir := t.TempDir() + + regularPath := filepath.Join(dir, "ok.fga.yaml") + if err := writeRegularFile(t, regularPath); err != nil { + t.Fatalf("failed to write regular test file: %v", err) + } + + if err := syscall.Mkfifo(filepath.Join(dir, "evil.fga.yaml"), 0o600); err != nil { + t.Skipf("unable to create FIFO: %v", err) + } + + modelTestCmd.SetArgs([]string{"--tests", filepath.Join(dir, "*.fga.yaml")}) + + done := make(chan struct{}) + + go func() { + // The command may return an error (e.g. no reachable FGA server); we only care that + // it returns at all rather than blocking on the FIFO read. + _ = modelTestCmd.Execute() + + close(done) + }() + + select { + case <-done: + case <-time.After(10 * time.Second): + t.Fatal("modelTestCmd hung on a FIFO glob match instead of skipping it") + } +} From b4ac9418cd35034bfbe63554eae4cef24d443652 Mon Sep 17 00:00:00 2001 From: terry-writer Date: Wed, 19 Aug 2026 22:48:55 +0900 Subject: [PATCH 4/4] fix: honor explicitly named non-regular paths in model test Branch on glob metacharacters up front: a pattern without *, ?, or [ is treated as an explicit literal path and honored as-is, so process substitution (--tests <(...), a FIFO like /dev/fd/11) works again. Only patterns with metacharacters go through glob expansion and the regular-file filter, so a FIFO literally named '*.fga.yaml' is still filtered rather than read. Add tests for an explicit FIFO path being honored and a FIFO named like a glob still being filtered. Update the resolveTestFiles doc comment to match the new behavior. --- cmd/model/test.go | 40 +++++++++++++++++++----------- cmd/model/test_test.go | 13 +++++----- cmd/model/test_unix_test.go | 49 ++++++++++++++++++++++++++++++++++--- 3 files changed, 77 insertions(+), 25 deletions(-) diff --git a/cmd/model/test.go b/cmd/model/test.go index d3b2e8ad..82d5a37f 100644 --- a/cmd/model/test.go +++ b/cmd/model/test.go @@ -36,6 +36,9 @@ import ( var errAllTestFilesNonRegular = errors.New("tests pattern matched only non-regular files (e.g. FIFOs); " + "pass an explicit regular file path instead") +// errNoTestFilesMatched is returned when a tests glob pattern matches no files at all. +var errNoTestFilesMatched = errors.New("no test files matched pattern") + // modelTestCmd represents the test command. var modelTestCmd = &cobra.Command{ Use: "test", @@ -166,28 +169,37 @@ var modelTestCmd = &cobra.Command{ }, } -// resolveTestFiles expands testsPattern via filepath.Glob and filters out any matches that -// are not regular files (e.g. FIFOs, devices, sockets). Reading a FIFO with no writer via -// os.ReadFile blocks forever, so non-regular glob matches are silently skipped rather than -// read. +// resolveTestFiles turns testsPattern into the list of test files to read. +// +// A pattern with no glob metacharacters (*, ?, [) is treated as an explicit, literal path +// and honored as-is - including non-regular paths such as process substitution +// (--tests <(...)), which resolves to a FIFO like /dev/fd/11. The caller asked for that +// exact path, so it is not subject to the regular-file filter below. // -// If the pattern matches nothing at all, it is treated as a literal path (e.g. to keep -// process substitution like --tests <(...) working) and is not subject to the regular-file -// check. If the pattern matches only non-regular files, that is treated as an error rather -// than silently falling back to the literal-path behavior, since testsPattern itself -// (e.g. "*.fga.yaml") is very unlikely to also be a valid literal path. +// A pattern with glob metacharacters is expanded via filepath.Glob, and non-regular matches +// (FIFOs, devices, sockets) are dropped: a glob is not an explicit request for any single +// file, and reading a FIFO with no writer via os.ReadFile blocks forever. If the glob matches +// only non-regular files, that is an error rather than something to read. +// +// The metacharacter check (rather than, say, comparing the single glob match against the +// pattern) is deliberate: a FIFO literally named "*.fga.yaml" must still be filtered out, not +// mistaken for a literal path. func resolveTestFiles(testsPattern string) ([]string, error) { + if !strings.ContainsAny(testsPattern, `*?[`) { + if _, statErr := os.Stat(testsPattern); statErr != nil { + return nil, fmt.Errorf("test file %s does not exist: %w", testsPattern, statErr) + } + + return []string{testsPattern}, nil + } + rawMatches, err := filepath.Glob(testsPattern) if err != nil { return nil, fmt.Errorf("invalid tests pattern %s due to %w", testsPattern, err) } if len(rawMatches) == 0 { - if _, statErr := os.Stat(testsPattern); statErr != nil { - return nil, fmt.Errorf("test file %s does not exist: %w", testsPattern, statErr) - } - - return []string{testsPattern}, nil + return nil, fmt.Errorf("%w: %s", errNoTestFilesMatched, testsPattern) } regularFileNames := rawMatches[:0] diff --git a/cmd/model/test_test.go b/cmd/model/test_test.go index 479d6391..79afdd20 100644 --- a/cmd/model/test_test.go +++ b/cmd/model/test_test.go @@ -19,10 +19,10 @@ func writeRegularFile(t *testing.T, path string) error { return nil } -// A pattern with no glob matches at all is treated as a literal path. Since the pattern here -// contains a wildcard and no such literal file exists, resolution should fail with a -// not-found error rather than silently returning an empty result. -func TestResolveTestFilesNoMatchesTreatedAsLiteralPath(t *testing.T) { +// A glob pattern (contains metacharacters) that matches no files should fail with a +// not-found/no-match error rather than being treated as a literal path or returning an empty +// result. +func TestResolveTestFilesNoMatchesFails(t *testing.T) { t.Parallel() dir := t.TempDir() @@ -34,9 +34,8 @@ func TestResolveTestFilesNoMatchesTreatedAsLiteralPath(t *testing.T) { } } -// An existing literal path (no glob metacharacters, so filepath.Glob returns it as a single -// match) is returned as-is, without the regular-file check rejecting it - this preserves -// existing behavior for explicitly named paths. +// An explicitly named, existing regular file (no glob metacharacters) is honored as-is - this +// preserves existing behavior for explicitly named paths. func TestResolveTestFilesLiteralExistingPath(t *testing.T) { t.Parallel() diff --git a/cmd/model/test_unix_test.go b/cmd/model/test_unix_test.go index 563083c3..26f19034 100644 --- a/cmd/model/test_unix_test.go +++ b/cmd/model/test_unix_test.go @@ -33,9 +33,6 @@ func TestResolveTestFilesSkipsNonRegularGlobMatches(t *testing.T) { pattern := filepath.Join(dir, "*.fga.yaml") - // This call must return promptly. If the FIFO were not filtered out, resolveTestFiles - // itself doesn't read file contents, but a regression that moved the read here would hang - // the test; test-unit's overall timeout is the real backstop for that case. fileNames, err := resolveTestFiles(pattern) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -104,7 +101,7 @@ func TestResolveTestFilesAllMatchesNonRegular(t *testing.T) { // the command block forever in os.ReadFile. Here the command must return within a short // deadline instead of hanging; whatever error it returns afterwards (e.g. from running the // tests) is irrelevant to this regression. -func TestModelTestCmdDoesNotHangOnFifoGlobMatch(t *testing.T) { //nolint:paralleltest // mutates the shared global modelTestCmd, so it must not run in parallel // Not parallel: modelTestCmd is a shared global and this test sets flags/args on it. +func TestModelTestCmdDoesNotHangOnFifoGlobMatch(t *testing.T) { //nolint:paralleltest // mutates the shared global modelTestCmd, so it must not run in parallel dir := t.TempDir() regularPath := filepath.Join(dir, "ok.fga.yaml") @@ -134,3 +131,47 @@ func TestModelTestCmdDoesNotHangOnFifoGlobMatch(t *testing.T) { //nolint:paralle t.Fatal("modelTestCmd hung on a FIFO glob match instead of skipping it") } } + +// An explicitly named non-regular path (no glob metacharacters) must be honored as-is, not +// rejected by the regular-file filter. This is what keeps process substitution +// (--tests <(...), which the shell turns into a FIFO like /dev/fd/11) working. The literal- +// path test in test_test.go uses a regular file, so it cannot catch this case. +func TestResolveTestFilesLiteralFifoPathHonored(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + fifoPath := filepath.Join(dir, "pipe.fga.yaml") + + if err := syscall.Mkfifo(fifoPath, 0o600); err != nil { + t.Skipf("unable to create FIFO: %v", err) + } + + fileNames, err := resolveTestFiles(fifoPath) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(fileNames) != 1 || fileNames[0] != fifoPath { + t.Fatalf("expected [%s], got %v", fifoPath, fileNames) + } +} + +// A FIFO literally named "*.fga.yaml" must be treated as a glob match and filtered out, not +// mistaken for a literal path - otherwise it would be read and hang. This guards the +// metacharacter-based branch against a naive "single match equals the pattern" shortcut. +func TestResolveTestFilesFifoNamedLikeGlobStillFiltered(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + + if err := syscall.Mkfifo(filepath.Join(dir, "*.fga.yaml"), 0o600); err != nil { + t.Skipf("unable to create FIFO: %v", err) + } + + pattern := filepath.Join(dir, "*.fga.yaml") + + fileNames, err := resolveTestFiles(pattern) + if err == nil { + t.Fatalf("expected an error, got fileNames=%v", fileNames) + } +}