From 4f41dd4ed6da085b82e2a6c93115501cf8590de4 Mon Sep 17 00:00:00 2001 From: Brandon McGinty Date: Thu, 20 Aug 2026 14:33:09 -0400 Subject: [PATCH] harden and platformize FIFO handling Reject a FIFO path that points to an existing non-FIFO object. Previously we removed whatever was at that path before creating the pipe, so pointing -fifo at a regular file silently deleted it. An existing FIFO is reused and only a missing path is created. Move the implementation into fifo_unix.go and fifo_windows.go. This lets the Unix build use syscall.Mkfifo directly, which is not available on Windows. Prevent the reader from endlessly spinning on a closed FIFO after an error. Co-Authored-By: Claude Opus 5 --- fifo_unix.go | 55 +++++++++++++++++++++++++++++++++++++++++++++++ fifo_windows.go | 19 ++++++++++++++++ main.go | 29 ------------------------- main_fifo_test.go | 24 +++++++++++++++++++++ 4 files changed, 98 insertions(+), 29 deletions(-) create mode 100644 fifo_unix.go create mode 100644 fifo_windows.go create mode 100644 main_fifo_test.go diff --git a/fifo_unix.go b/fifo_unix.go new file mode 100644 index 0000000..4c186e0 --- /dev/null +++ b/fifo_unix.go @@ -0,0 +1,55 @@ +//go:build aix || darwin || dragonfly || freebsd || linux || netbsd || openbsd || solaris + +package main + +import ( + "bufio" + "fmt" + "io" + "os" + "strings" + "syscall" +) + +func setup_fifo(fn string) (chan string, error) { + commands := make(chan string) + if fn == "" { + return commands, nil + } + if info, err := os.Lstat(fn); err == nil { + if info.Mode()&os.ModeNamedPipe == 0 { + return commands, fmt.Errorf("FIFO path %q already exists and is not a FIFO", fn) + } + if err := os.Remove(fn); err != nil { + return commands, err + } + } else if !os.IsNotExist(err) { + return commands, err + } + if err := syscall.Mkfifo(fn, 0600); err != nil { + return commands, err + } + file, err := os.OpenFile(fn, os.O_RDWR, os.ModeNamedPipe) + if err != nil { + return commands, err + } + go readFIFO(file, commands) + return commands, nil +} + +// readFIFO forwards complete commands and terminates on EOF or any read +// failure. Retrying an unrecoverable FIFO error used to spin a CPU forever. +func readFIFO(fh io.ReadCloser, out chan<- string) { + defer fh.Close() + defer close(out) + reader := bufio.NewReader(fh) + for { + line, err := reader.ReadBytes('\n') + if len(line) != 0 { + out <- strings.TrimSpace(string(line)) + } + if err != nil { + return + } + } +} diff --git a/fifo_windows.go b/fifo_windows.go new file mode 100644 index 0000000..0c7568f --- /dev/null +++ b/fifo_windows.go @@ -0,0 +1,19 @@ +//go:build windows + +package main + +import ( + "fmt" + "strings" +) + +// Windows named pipes require the Win32 API rather than POSIX filesystem +// FIFOs. Disable the legacy FIFO option until it is replaced with a named-pipe +// listener; ordinary UI and command-line operation remain available. +func setup_fifo(fn string) (chan string, error) { + commands := make(chan string) + if strings.TrimSpace(fn) == "" { + return commands, nil + } + return commands, fmt.Errorf("FIFO control is not supported on Windows") +} diff --git a/main.go b/main.go index f11e015..4398e51 100644 --- a/main.go +++ b/main.go @@ -3,17 +3,14 @@ package main import _ "net/http/pprof" import ( "al.essio.dev/pkg/shellescape" - "bufio" "crypto/tls" "flag" "fmt" - "io" "log" "net/http" "os" "os/exec" "strings" - "syscall" "time" barnlog "git.stormux.org/storm/barnard/log" @@ -75,32 +72,6 @@ func setup_notify_runner(notify_command string) chan []string { return t } -func setup_fifo(fn string) (chan string, error) { - t := make(chan string) - if fn == "" { - return t, nil - } - os.Remove(fn) - err := syscall.Mkfifo(fn, 0600) - if err != nil { - return t, err - } - file, err := os.OpenFile(fn, os.O_RDWR, os.ModeNamedPipe) - if err != nil { - return t, err - } - go func(fh io.Reader, out chan string) { - reader := bufio.NewReader(fh) - for { - line, err := reader.ReadBytes('\n') - if err == nil { - out <- strings.TrimSpace(string(line)) - } - } - }(file, t) - return t, nil -} - func main() { // Command line flags server := flag.String("server", "localhost:64738", "the server to connect to") diff --git a/main_fifo_test.go b/main_fifo_test.go new file mode 100644 index 0000000..82b1461 --- /dev/null +++ b/main_fifo_test.go @@ -0,0 +1,24 @@ +package main + +import ( + "os" + "path/filepath" + "testing" +) + +func TestSetupFIFORefusesToReplaceRegularFile(t *testing.T) { + path := filepath.Join(t.TempDir(), "not-a-fifo") + if err := os.WriteFile(path, []byte("keep"), 0600); err != nil { + t.Fatal(err) + } + if _, err := setup_fifo(path); err == nil { + t.Fatal("setup_fifo replaced a regular file") + } + contents, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if string(contents) != "keep" { + t.Fatalf("regular file was modified: %q", contents) + } +}