From ae91479f0beb555bc4cd3bb698a76a85522b5c3d Mon Sep 17 00:00:00 2001 From: Gary Date: Tue, 22 Sep 2026 10:00:25 +0200 Subject: [PATCH 1/4] fix(command): run start scripts with their interpreter, a timeout and logs ExecuteScripts read the shebang after closing the file, so every script ran with /bin/sh, and it passed the whole shebang as the program name, so '#!/usr/bin/env bash' could not have worked either. It threw the output away, stopped at the first failing script and had no time limit, so a hung script held the start of CasaOS until systemd killed it, again at every restart. Now each regular file of the directory runs in name order with the program and optional argument of its shebang line, for at most ScriptTimeout (60 seconds). On Linux the script gets its own process group and a timeout kills the whole group; elsewhere only the script's process is killed and WaitDelay stops waiting for pipes its children keep open. Each run is logged with its name, interpreter, exit code, duration and the last 4 KB of its output, at Info or Error. A failure no longer stops the next scripts; the failures come back joined. A missing directory is logged and is not an error. --- utils/command/command.go | 184 +++++++++++++++++++++++++++------ utils/command/command_linux.go | 15 +++ utils/command/command_other.go | 10 ++ utils/command/command_test.go | 176 ++++++++++++++++++++++++++----- 4 files changed, 327 insertions(+), 58 deletions(-) create mode 100644 utils/command/command_linux.go create mode 100644 utils/command/command_other.go diff --git a/utils/command/command.go b/utils/command/command.go index 35f1048..fb28240 100644 --- a/utils/command/command.go +++ b/utils/command/command.go @@ -3,13 +3,19 @@ package command import ( "bufio" "bytes" + "context" + "errors" "fmt" + "io/fs" "os" "os/exec" "path/filepath" "strings" + "time" exec2 "github.com/ReCasaOS/CasaOS-Common/utils/exec" + "github.com/ReCasaOS/CasaOS-Common/utils/logger" + "go.uber.org/zap" ) // Deprecated: This method is not safe, sould have ensure input. @@ -40,55 +46,171 @@ func ExecResultStrArray(cmdStr string) ([]string, error) { return result, nil } +// ScriptTimeout bounds each script ExecuteScripts runs: a hung script would +// otherwise hold the start of the service until systemd kills it, and then +// again at every restart. A variable so tests can shorten it. +var ScriptTimeout = 60 * time.Second + +const ( + // scriptOutputLimit is how much of a script's output goes to the log: the + // end of it, where an error usually is. + scriptOutputLimit = 4 << 10 + + // scriptWaitDelay is how long a script that has exited, or been killed, may + // keep its output open through something it started, such as a daemon. + scriptWaitDelay = 2 * time.Second +) + +// ExecuteScripts runs every regular file of scriptDirectory, in name order, +// each with the interpreter of its shebang line (/bin/sh without one) and for +// at most ScriptTimeout. Every run is logged with its output. A failing script +// does not stop the next ones; the failures come back joined. A missing +// directory is not an error. func ExecuteScripts(scriptDirectory string) error { - if _, err := os.Stat(scriptDirectory); os.IsNotExist(err) { - fmt.Printf("No post-start scripts at %s\n", scriptDirectory) - return err + entries, err := os.ReadDir(scriptDirectory) + if errors.Is(err, fs.ErrNotExist) { + logger.Info("no start scripts to run", zap.String("directory", scriptDirectory)) + return nil } - - files, err := os.ReadDir(scriptDirectory) if err != nil { - fmt.Printf("Failed to read from script directory %s: %s\n", scriptDirectory, err.Error()) + logger.Error("cannot read the start scripts", zap.String("directory", scriptDirectory), zap.Error(err)) return err } - for _, file := range files { - if file.IsDir() { + var failures []error + for _, entry := range entries { + path := filepath.Join(scriptDirectory, entry.Name()) + if info, err := os.Stat(path); err != nil || !info.Mode().IsRegular() { continue } - scriptFilepath := filepath.Join(scriptDirectory, file.Name()) - - f, err := os.Open(scriptFilepath) - if err != nil { - fmt.Printf("Failed to open script file %s: %s\n", scriptFilepath, err.Error()) - continue + result := runScript(path) + reportScript(result) + if result.err != nil { + failures = append(failures, fmt.Errorf("start script %s: %w", result.name, result.err)) } - f.Close() + } + + return errors.Join(failures...) +} - scanner := bufio.NewScanner(f) - scanner.Scan() - shebang := scanner.Text() +type scriptResult struct { + name string + interpreter string + exitCode int + duration time.Duration + output string + err error +} - interpreter := "/bin/sh" - if strings.HasPrefix(shebang, "#!") { - interpreter = shebang[2:] - } +// reportScript logs what a script did; tests replace it. +var reportScript = func(r scriptResult) { + fields := []zap.Field{ + zap.String("script", r.name), + zap.String("interpreter", r.interpreter), + zap.Int("exit_code", r.exitCode), + zap.Duration("duration", r.duration), + zap.String("output", r.output), + } + if r.err != nil { + logger.Error("start script failed", append(fields, zap.Error(r.err))...) + return + } + logger.Info("start script ran", fields...) +} - cmd := exec2.Command(interpreter, scriptFilepath) +func runScript(path string) scriptResult { + result := scriptResult{name: filepath.Base(path), exitCode: -1} - fmt.Printf("Executing post-start script %s using %s\n", scriptFilepath, interpreter) + line, err := firstLine(path) + if err != nil { + result.err = err + return result + } + argv := interpreter(line) + result.interpreter = strings.Join(argv, " ") + + ctx, cancel := context.WithTimeout(context.Background(), ScriptTimeout) + defer cancel() + + var output tail + cmd := exec2.CommandContext(ctx, argv[0], append(argv[1:], path)...) + cmd.Stdout, cmd.Stderr = &output, &output + cmd.WaitDelay = scriptWaitDelay + killGroupOnCancel(cmd.Cmd) + + start := time.Now() + err = cmd.Run() + result.duration = time.Since(start) + result.exitCode = cmd.ProcessState.ExitCode() + result.output = output.String() + + switch { + case err == nil, errors.Is(err, exec.ErrWaitDelay): + // ErrWaitDelay: the script exited well, but something it started still + // held its output. Starting a daemon is not a failure. + case ctx.Err() != nil: + result.err = fmt.Errorf("timed out after %s: %w", ScriptTimeout, err) + default: + result.err = err + } - _, err = cmd.CombinedOutput() - if err != nil { - fmt.Printf("Failed to execute post-start script %s: %s\n", scriptFilepath, err.Error()) - return err - } + return result +} + +func firstLine(path string) (string, error) { + f, err := os.Open(path) + if err != nil { + return "", err + } + defer f.Close() + + scanner := bufio.NewScanner(f) + scanner.Scan() + + return scanner.Text(), scanner.Err() +} + +// interpreter reads a shebang line as the kernel does: a program and at most +// one argument, so "#!/usr/bin/env bash" is /usr/bin/env with the argument +// bash. A script without one runs with /bin/sh. +func interpreter(line string) []string { + rest, ok := strings.CutPrefix(line, "#!") + rest = strings.TrimSpace(rest) + if !ok || rest == "" { + return []string{"/bin/sh"} + } + + i := strings.IndexAny(rest, " \t") + if i < 0 { + return []string{rest} + } + + return []string{rest[:i], strings.TrimSpace(rest[i+1:])} +} + +// tail keeps the last scriptOutputLimit bytes written to it. +type tail struct { + buf []byte + truncated bool +} + +func (t *tail) Write(p []byte) (int, error) { + t.buf = append(t.buf, p...) + if over := len(t.buf) - scriptOutputLimit; over > 0 { + t.buf, t.truncated = t.buf[over:], true + } + + return len(p), nil +} +func (t *tail) String() string { + output := strings.TrimSpace(string(t.buf)) + if t.truncated { + return "..." + output } - fmt.Println("Finished executing post-start scripts.") - return nil + return output } func ExecStdin(stdinStr string, name string, args ...string) (string, error) { diff --git a/utils/command/command_linux.go b/utils/command/command_linux.go new file mode 100644 index 0000000..c42c954 --- /dev/null +++ b/utils/command/command_linux.go @@ -0,0 +1,15 @@ +package command + +import ( + "os/exec" + "syscall" +) + +// killGroupOnCancel runs a script in its own process group, so that its +// timeout kills whatever the script started along with it. +func killGroupOnCancel(cmd *exec.Cmd) { + cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} + cmd.Cancel = func() error { + return syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL) + } +} diff --git a/utils/command/command_other.go b/utils/command/command_other.go new file mode 100644 index 0000000..490133b --- /dev/null +++ b/utils/command/command_other.go @@ -0,0 +1,10 @@ +//go:build !linux + +package command + +import "os/exec" + +// killGroupOnCancel keeps the default of os/exec: a timeout kills the script's +// own process only, and WaitDelay stops waiting for the output its children +// may keep open. Process groups are a Linux affair here; CasaOS runs there. +func killGroupOnCancel(*exec.Cmd) {} diff --git a/utils/command/command_test.go b/utils/command/command_test.go index 0d075d0..6feb2a7 100644 --- a/utils/command/command_test.go +++ b/utils/command/command_test.go @@ -2,7 +2,14 @@ package command import ( "os" + "path/filepath" + "reflect" + "runtime" + "strings" "testing" + "time" + + "github.com/ReCasaOS/CasaOS-Common/utils/logger" ) func TestCommand(t *testing.T) { @@ -66,33 +73,6 @@ func TestCommand(t *testing.T) { }, } - t.Run("TestExecuteScripts", func(t *testing.T) { - // make a temp directory - tmpDir, err := os.MkdirTemp("", "casaos-test-*") - if err != nil { - t.Error(err) - } - defer os.RemoveAll(tmpDir) - - ExecuteScripts(tmpDir) - - // create a sample script under tmpDir - script := tmpDir + "/test.sh" - f, err := os.Create(script) - if err != nil { - t.Error(err) - } - defer f.Close() - - // write a sample script - _, err = f.WriteString("#!/bin/bash\necho 123") - if err != nil { - t.Error(err) - } - - ExecuteScripts(tmpDir) - }) - for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { if output, err := ExecResultStr(tt.cmdStr); tt.noErr != (err == nil) { @@ -111,3 +91,145 @@ func TestCommand(t *testing.T) { }) } } + +func TestTheShebangIsReadLikeTheKernelDoes(t *testing.T) { + for line, want := range map[string][]string{ + "#!/bin/sh": {"/bin/sh"}, + "#!/bin/bash\r": {"/bin/bash"}, + "#! /bin/bash -e": {"/bin/bash", "-e"}, + "#!/usr/bin/env bash": {"/usr/bin/env", "bash"}, + "#!/usr/bin/env\tpython3 -u": {"/usr/bin/env", "python3 -u"}, + "#!": {"/bin/sh"}, + "echo no shebang": {"/bin/sh"}, + "": {"/bin/sh"}, + } { + if got := interpreter(line); !reflect.DeepEqual(got, want) { + t.Errorf("%q: got %q, want %q", line, got, want) + } + } +} + +func TestOnlyTheEndOfALongOutputIsKept(t *testing.T) { + var output tail + output.Write([]byte(strings.Repeat("a", scriptOutputLimit))) + output.Write([]byte(strings.Repeat("b", 100) + "the error\n")) + + got := output.String() + if len(got) > scriptOutputLimit+3 || !strings.HasPrefix(got, "...a") || !strings.HasSuffix(got, "bthe error") { + t.Fatalf("got %d bytes ending in %q", len(got), got[len(got)-20:]) + } +} + +func TestAMissingScriptDirectoryIsNotAnError(t *testing.T) { + logger.LogInitConsoleOnly() + + if err := ExecuteScripts(filepath.Join(t.TempDir(), "start.d")); err != nil { + t.Fatal(err) + } + + // The default report logs through zap, success and failure alike. + reportScript(scriptResult{name: "10-ok.sh", interpreter: "/bin/sh", output: "done"}) + reportScript(scriptResult{name: "20-ko.sh", interpreter: "/bin/sh", exitCode: 1, err: os.ErrInvalid}) +} + +// runScripts writes the scripts to a directory, runs it and returns what was +// reported for each script, in order. +func runScripts(t *testing.T, scripts map[string]string) ([]scriptResult, error) { + t.Helper() + if runtime.GOOS == "windows" { + t.Skip("the scripts need /bin/sh and /usr/bin/env; CI runs this on Linux") + } + + dir := t.TempDir() + for name, body := range scripts { + if err := os.WriteFile(filepath.Join(dir, name), []byte(body), 0o755); err != nil { + t.Fatal(err) + } + } + if err := os.Mkdir(filepath.Join(dir, "00-a-directory"), 0o755); err != nil { + t.Fatal(err) + } + + var results []scriptResult + previous := reportScript + reportScript = func(r scriptResult) { results = append(results, r) } + t.Cleanup(func() { reportScript = previous }) + + err := ExecuteScripts(dir) + + return results, err +} + +func TestEachScriptRunsWithItsOwnInterpreter(t *testing.T) { + results, err := runScripts(t, map[string]string{ + "10-sh.sh": "#!/bin/sh\necho plain sh\n", + "20-env.sh": "#!/usr/bin/env sh\necho through env >&2\n", + "30-none.sh": "echo no shebang\n", + }) + if err != nil { + t.Fatal(err) + } + + want := [][3]string{ + {"10-sh.sh", "/bin/sh", "plain sh"}, + {"20-env.sh", "/usr/bin/env sh", "through env"}, + {"30-none.sh", "/bin/sh", "no shebang"}, + } + if len(results) != len(want) { + t.Fatalf("got %d scripts, want %d: %+v", len(results), len(want), results) + } + for i, r := range results { + if got := [3]string{r.name, r.interpreter, r.output}; got != want[i] || r.exitCode != 0 || r.err != nil { + t.Errorf("got %q (exit %d, %v), want %q", got, r.exitCode, r.err, want[i]) + } + } +} + +func TestAFailingScriptDoesNotStopTheNextOnes(t *testing.T) { + results, err := runScripts(t, map[string]string{ + "10-fails.sh": "#!/bin/sh\necho broken >&2\nexit 3\n", + "20-next.sh": "#!/bin/sh\necho still ran\n", + }) + if err == nil || !strings.Contains(err.Error(), "10-fails.sh") { + t.Fatalf("the failure must come back, got %v", err) + } + + if len(results) != 2 { + t.Fatalf("got %d scripts, want 2: %+v", len(results), results) + } + if r := results[0]; r.err == nil || r.exitCode != 3 || r.output != "broken" { + t.Errorf("failing script: got exit %d, output %q, %v", r.exitCode, r.output, r.err) + } + if r := results[1]; r.err != nil || r.output != "still ran" { + t.Errorf("next script: got output %q, %v", r.output, r.err) + } +} + +func TestAHungScriptIsKilledAtItsTimeout(t *testing.T) { + previous := ScriptTimeout + ScriptTimeout = 200 * time.Millisecond + t.Cleanup(func() { ScriptTimeout = previous }) + + results, err := runScripts(t, map[string]string{ + "10-hangs.sh": "#!/bin/sh\necho started\nsleep 30\n", + "20-next.sh": "#!/bin/sh\necho still ran\n", + }) + if err == nil || !strings.Contains(err.Error(), "10-hangs.sh: timed out") { + t.Fatalf("the timeout must come back, got %v", err) + } + + if len(results) != 2 { + t.Fatalf("got %d scripts, want 2: %+v", len(results), results) + } + hung := results[0] + if hung.err == nil || hung.output != "started" { + t.Errorf("hung script: got output %q, %v", hung.output, hung.err) + } + // Past scriptWaitDelay, the sleep outlived the shell: the group was not killed. + if hung.duration >= scriptWaitDelay { + t.Errorf("the hung script held the start for %s", hung.duration) + } + if r := results[1]; r.err != nil || r.output != "still ran" { + t.Errorf("next script: got output %q, %v", r.output, r.err) + } +} From 3863d275998dee81ab4a0ebd0f4cb11dd124f0e5 Mon Sep 17 00:00:00 2001 From: Gary Date: Tue, 22 Sep 2026 10:01:19 +0200 Subject: [PATCH 2/4] fix(external): move the message bus socket out of /tmp PublishEventInSocket posted to /tmp/message-bus.sock. /tmp is world-writable, so any local user could create that path first and keep the message bus from listening, or answer in its place. The socket now lives in the runtime path, which only root can write: MessageBusSocketPath(runtimePath) returns it, and PublishEventInSocket takes the runtime path as its second argument. The message bus must listen on the same path. The dial and the request now follow the caller's context, and keep-alive is off: each call builds its own transport, and the idle connection it kept was never reused nor closed. --- external/message_bus.go | 25 +++++++++++--- external/message_bus_test.go | 65 ++++++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+), 5 deletions(-) create mode 100644 external/message_bus_test.go diff --git a/external/message_bus.go b/external/message_bus.go index 16db7f6..e5ec665 100644 --- a/external/message_bus.go +++ b/external/message_bus.go @@ -16,8 +16,19 @@ import ( const ( MessageBusAddressFilename = "message-bus.url" APIMessageBus = "/v2/message_bus" + + // MessageBusSocketFilename is the unix socket, in the runtime path, where + // the message bus takes events. + MessageBusSocketFilename = "message-bus.sock" ) +// MessageBusSocketPath is where the message bus listens for events over a unix +// socket: in the runtime path, which only root can write. It used to be +// /tmp/message-bus.sock, a name any local user could take first. +func MessageBusSocketPath(runtimePath string) string { + return filepath.Join(runtimePath, MessageBusSocketFilename) +} + type EventType struct { Name string SourceID string @@ -70,13 +81,17 @@ func GetMessageBusAddress(runtimePath string) (string, error) { return strings.TrimRight(address, "/") + APIMessageBus, nil } -func PublishEventInSocket(ctx context.Context, SourceID string, Name string, properties map[string]string) (*http.Response, error) { - socketPath := "/tmp/message-bus.sock" +// PublishEventInSocket posts an event to the message bus through its unix +// socket in runtimePath. The response comes back with its body closed. +func PublishEventInSocket(ctx context.Context, runtimePath, sourceID, name string, properties map[string]string) (*http.Response, error) { + socketPath := MessageBusSocketPath(runtimePath) httpClient := &http.Client{ Transport: &http.Transport{ DialContext: func(ctx context.Context, _, _ string) (net.Conn, error) { - return net.Dial("unix", socketPath) + return (&net.Dialer{}).DialContext(ctx, "unix", socketPath) }, + // A transport per call: an idle connection kept alive would be leaked. + DisableKeepAlives: true, }, } @@ -85,8 +100,8 @@ func PublishEventInSocket(ctx context.Context, SourceID string, Name string, pro return nil, err } - req, err := http.NewRequest("POST", - fmt.Sprintf("http://unix/v2/message_bus/event/%s/%s", SourceID, Name), + req, err := http.NewRequestWithContext(ctx, http.MethodPost, + fmt.Sprintf("http://unix/v2/message_bus/event/%s/%s", sourceID, name), bytes.NewBuffer(body), ) if err != nil { diff --git a/external/message_bus_test.go b/external/message_bus_test.go new file mode 100644 index 0000000..15e8660 --- /dev/null +++ b/external/message_bus_test.go @@ -0,0 +1,65 @@ +package external + +import ( + "context" + "encoding/json" + "net" + "net/http" + "os" + "path/filepath" + "testing" +) + +func TestTheMessageBusSocketIsInTheRuntimePath(t *testing.T) { + if got, want := MessageBusSocketPath("/var/run/casaos"), filepath.Join("/var/run/casaos", "message-bus.sock"); got != want { + t.Fatalf("got %q, want %q", got, want) + } +} + +func TestPublishEventInSocketPostsToTheRuntimeSocket(t *testing.T) { + // Short: a unix socket path is limited to about a hundred bytes. + runtimePath, err := os.MkdirTemp("", "mb") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { os.RemoveAll(runtimePath) }) + + listener, err := net.Listen("unix", MessageBusSocketPath(runtimePath)) + if err != nil { + t.Skipf("no unix socket here: %v", err) + } + + type request struct { + method, path, contentType string + properties map[string]string + } + received := make(chan request, 1) + server := &http.Server{Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + got := request{method: r.Method, path: r.URL.Path, contentType: r.Header.Get("Content-Type")} + json.NewDecoder(r.Body).Decode(&got.properties) + received <- got + w.WriteHeader(http.StatusAccepted) + })} + go server.Serve(listener) + t.Cleanup(func() { server.Close() }) + + response, err := PublishEventInSocket(context.Background(), runtimePath, "app-management", "app:install-end", map[string]string{"app:name": "nextcloud"}) + if err != nil { + t.Fatal(err) + } + if response.StatusCode != http.StatusAccepted { + t.Fatalf("status %d, want %d", response.StatusCode, http.StatusAccepted) + } + + got := <-received + if got.method != http.MethodPost || got.path != "/v2/message_bus/event/app-management/app:install-end" || got.contentType != "application/json" { + t.Errorf("got %s %s (%s)", got.method, got.path, got.contentType) + } + if got.properties["app:name"] != "nextcloud" { + t.Errorf("got properties %v", got.properties) + } + + if _, err := PublishEventInSocket(context.Background(), t.TempDir(), "app-management", "app:install-end", nil); err == nil { + t.Fatal("a runtime path without the message bus must fail") + } +} From 9bc0de50608431f910209cfa43a0b931befbaa98 Mon Sep 17 00:00:00 2001 From: Gary Date: Tue, 22 Sep 2026 10:01:52 +0200 Subject: [PATCH 3/4] feat(external): expose the internal Authorization value A websocket handshake from x/net/websocket takes its headers from Config.Header, where a request editor does not fit. InternalAuthorization(runtimePath) returns "Internal " when the secret file is readable and an empty string otherwise, so such a caller can set the header itself. InternalRequestEditor and the HTTP helpers now go through it; their behaviour does not change. --- external/internal_auth.go | 24 +++++++++++++++--------- external/internal_auth_test.go | 19 +++++++++++++++++++ 2 files changed, 34 insertions(+), 9 deletions(-) diff --git a/external/internal_auth.go b/external/internal_auth.go index dce84e0..4651cad 100644 --- a/external/internal_auth.go +++ b/external/internal_auth.go @@ -93,14 +93,20 @@ func isLoopbackIP(ip string) bool { // request to a loopback address. The file is read at each request, so a secret // rewritten by a restarted gateway is picked up without anybody restarting. func useInternalSecret(runtimePath string) { - http2.SetInternalAuthorization(func() string { - secret, err := readInternalSecret(runtimePath) - if err != nil { - return "" - } + http2.SetInternalAuthorization(func() string { return InternalAuthorization(runtimePath) }) +} + +// InternalAuthorization returns the Authorization value that makes a request +// internal, "Internal ", or "" when the secret cannot be read. It is for +// what a request editor does not fit, such as the Config.Header of a websocket +// handshake. Send it to loopback addresses only. +func InternalAuthorization(runtimePath string) string { + secret, err := readInternalSecret(runtimePath) + if err != nil { + return "" + } - return internalScheme + secret - }) + return internalScheme + secret } // InternalRequestEditor is for the API clients oapi-codegen generates, which do @@ -114,8 +120,8 @@ func InternalRequestEditor(runtimePath string) func(ctx context.Context, req *ht if req.Header.Get("Authorization") != "" || !isLoopbackHost(req.URL.Hostname()) { return nil } - if secret, err := readInternalSecret(runtimePath); err == nil { - req.Header.Set("Authorization", internalScheme+secret) + if authorization := InternalAuthorization(runtimePath); authorization != "" { + req.Header.Set("Authorization", authorization) } return nil diff --git a/external/internal_auth_test.go b/external/internal_auth_test.go index 520e1f4..da8cd50 100644 --- a/external/internal_auth_test.go +++ b/external/internal_auth_test.go @@ -141,3 +141,22 @@ func TestTheRequestEditorSendsTheSecretToLoopbackOnly(t *testing.T) { t.Fatalf("with no secret file the request goes without one, got %q (%v)", missing.Header.Get("Authorization"), err) } } + +func TestInternalAuthorizationIsTheHeaderValueOrNothing(t *testing.T) { + dir, secret := writeSecretFor(t) + if got := InternalAuthorization(dir); got != internalScheme+secret { + t.Fatalf("got %q, want %q", got, internalScheme+secret) + } + + if got := InternalAuthorization(t.TempDir()); got != "" { + t.Fatalf("with no secret file, got %q", got) + } + + empty := t.TempDir() + if err := os.WriteFile(filepath.Join(empty, InternalSecretFilename), []byte("\n"), 0o600); err != nil { + t.Fatal(err) + } + if got := InternalAuthorization(empty); got != "" { + t.Fatalf("with an empty secret file, got %q", got) + } +} From af9a6b6ffccb6a14151d507b1167f54208e2ee99 Mon Sep 17 00:00:00 2001 From: Gary Date: Tue, 22 Sep 2026 10:20:16 +0200 Subject: [PATCH 4/4] fix(command): start scripts run through os/exec, read like the kernel, and warn about kept pipes utils/exec's injection check refused shebang arguments with a space and script names with one, though no shell ever sees them: scripts now go straight to os/exec. The first line is read the way the kernel reads it, at most 256 bytes, so a long first line no longer fails the script. A start.d entry that cannot be stat'ed is logged instead of skipped silently, and a script whose daemon kept its output open is logged as a warning: that daemon dies of SIGPIPE on its next write. A #!/bin/cat case proves the shebang's program is the one that runs. --- utils/command/command.go | 45 +++++++++++++++++++++++++++-------- utils/command/command_test.go | 3 +++ 2 files changed, 38 insertions(+), 10 deletions(-) diff --git a/utils/command/command.go b/utils/command/command.go index fb28240..97ff390 100644 --- a/utils/command/command.go +++ b/utils/command/command.go @@ -1,11 +1,11 @@ package command import ( - "bufio" "bytes" "context" "errors" "fmt" + "io" "io/fs" "os" "os/exec" @@ -66,6 +66,10 @@ const ( // at most ScriptTimeout. Every run is logged with its output. A failing script // does not stop the next ones; the failures come back joined. A missing // directory is not an error. +// +// A script that starts a daemon must redirect the daemon's output: once the +// script has exited, its pipes are closed after a short delay, and a daemon +// still writing to them is killed by SIGPIPE. func ExecuteScripts(scriptDirectory string) error { entries, err := os.ReadDir(scriptDirectory) if errors.Is(err, fs.ErrNotExist) { @@ -80,7 +84,12 @@ func ExecuteScripts(scriptDirectory string) error { var failures []error for _, entry := range entries { path := filepath.Join(scriptDirectory, entry.Name()) - if info, err := os.Stat(path); err != nil || !info.Mode().IsRegular() { + info, err := os.Stat(path) + if err != nil { + logger.Warn("start script skipped", zap.String("script", entry.Name()), zap.Error(err)) + continue + } + if !info.Mode().IsRegular() { continue } @@ -100,6 +109,7 @@ type scriptResult struct { exitCode int duration time.Duration output string + warning string err error } @@ -116,6 +126,10 @@ var reportScript = func(r scriptResult) { logger.Error("start script failed", append(fields, zap.Error(r.err))...) return } + if r.warning != "" { + logger.Warn("start script ran", append(fields, zap.String("warning", r.warning))...) + return + } logger.Info("start script ran", fields...) } @@ -133,11 +147,14 @@ func runScript(path string) scriptResult { ctx, cancel := context.WithTimeout(context.Background(), ScriptTimeout) defer cancel() + // os/exec directly: the arguments go to execve, no shell ever sees them, and + // utils/exec's injection check would refuse a legitimate shebang argument + // such as "-S sh -e" or a script name with a space in it. var output tail - cmd := exec2.CommandContext(ctx, argv[0], append(argv[1:], path)...) + cmd := exec.CommandContext(ctx, argv[0], append(argv[1:], path)...) cmd.Stdout, cmd.Stderr = &output, &output cmd.WaitDelay = scriptWaitDelay - killGroupOnCancel(cmd.Cmd) + killGroupOnCancel(cmd) start := time.Now() err = cmd.Run() @@ -146,9 +163,11 @@ func runScript(path string) scriptResult { result.output = output.String() switch { - case err == nil, errors.Is(err, exec.ErrWaitDelay): - // ErrWaitDelay: the script exited well, but something it started still - // held its output. Starting a daemon is not a failure. + case err == nil: + case errors.Is(err, exec.ErrWaitDelay): + // the script exited well, but something it started still held its + // output: starting a daemon is not a failure, writing there again kills it + result.warning = "a process the script started kept its output open; it is killed by SIGPIPE if it writes again: redirect its output" case ctx.Err() != nil: result.err = fmt.Errorf("timed out after %s: %w", ScriptTimeout, err) default: @@ -158,6 +177,8 @@ func runScript(path string) scriptResult { return result } +// firstLine reads what the kernel reads of a script: at most its first 256 +// bytes, up to the first newline. func firstLine(path string) (string, error) { f, err := os.Open(path) if err != nil { @@ -165,10 +186,14 @@ func firstLine(path string) (string, error) { } defer f.Close() - scanner := bufio.NewScanner(f) - scanner.Scan() + head := make([]byte, 256) + n, err := io.ReadFull(f, head) + if err != nil && !errors.Is(err, io.EOF) && !errors.Is(err, io.ErrUnexpectedEOF) { + return "", err + } + line, _, _ := bytes.Cut(head[:n], []byte("\n")) - return scanner.Text(), scanner.Err() + return strings.TrimRight(string(line), "\r"), nil } // interpreter reads a shebang line as the kernel does: a program and at most diff --git a/utils/command/command_test.go b/utils/command/command_test.go index 6feb2a7..ed47757 100644 --- a/utils/command/command_test.go +++ b/utils/command/command_test.go @@ -165,6 +165,8 @@ func TestEachScriptRunsWithItsOwnInterpreter(t *testing.T) { "10-sh.sh": "#!/bin/sh\necho plain sh\n", "20-env.sh": "#!/usr/bin/env sh\necho through env >&2\n", "30-none.sh": "echo no shebang\n", + // cat prints the script itself: proof the shebang's program is what ran + "40-cat.txt": "#!/bin/cat\nhello from cat\n", }) if err != nil { t.Fatal(err) @@ -174,6 +176,7 @@ func TestEachScriptRunsWithItsOwnInterpreter(t *testing.T) { {"10-sh.sh", "/bin/sh", "plain sh"}, {"20-env.sh", "/usr/bin/env sh", "through env"}, {"30-none.sh", "/bin/sh", "no shebang"}, + {"40-cat.txt", "/bin/cat", "#!/bin/cat\nhello from cat"}, } if len(results) != len(want) { t.Fatalf("got %d scripts, want %d: %+v", len(results), len(want), results)