From af90d8e2be93b7e008884b7b726c5e707ea0c792 Mon Sep 17 00:00:00 2001 From: Jay Date: Fri, 14 Aug 2026 23:39:21 +0530 Subject: [PATCH] fix(agent/agentscripts): create missing log_path parent directory (#28166) Previously, a `coder_script` whose `log_path` pointed under a directory that did not yet exist failed before the script ran, with no per-script log output. `OpenFile(logPath, O_CREATE|O_RDWR, 0o600)` creates the log file but not its parent directories, so the open returned `ENOENT`. The failure only surfaced in the agent log (`startup script(s) failed` / `shutdown script(s) failed`) and never reached the script's own UI logs, which made it look like a silent failure. This creates the resolved parent directory with `MkdirAll(filepath.Dir(logPath), 0o700)` before opening the log file, so the script runs and its log is written. `0o700` matches the existing script data-dir and secret-file directory conventions in this package. Resolution of `~`, environment variables, and paths relative to `LogDir` is unchanged; only the parent directory is now created. Fixes coder/coder#21986
Implementation notes and validation **Change** * `agent/agentscripts/agentscripts.go`: in `(*Runner).run`, after the full `logPath` resolution and before `OpenFile`, create the parent directory: ```go logDir := filepath.Dir(logPath) if err = r.Filesystem.MkdirAll(logDir, 0o700); err != nil { return xerrors.Errorf("create script log file directory %q: %w", logDir, err) } ``` **Regression test** * `agent/agentscripts/agentscripts_test.go`: `TestExecuteCreatesMissingLogDir` runs a script with a nested, nonexistent `LogPath` and asserts the streamed output and that the log file is created. * The test uses `afero.NewOsFs()` on purpose: `afero.NewMemMapFs()` auto-creates parent directories on `OpenFile`, so it cannot reproduce the reported failure. * Verified red without the fix (`open .../does/not/exist/install.log: no such file or directory`) and green with it. **Local validation** * `gofmt` clean, `go vet`, `go build`, `golangci-lint run` on the package, and `go test -race ./agent/agentscripts/` all pass. **End-to-end** * Validated on a dev instance with a template whose `coder_script.log_path` targets a nested directory that does not exist. The agent created the parents with mode `0700` and wrote the log file; the workspace agent reported healthy. **Prior attempts** * [#22796]() and [#25545]() proposed the same directory-creation approach. Both were closed for non-technical reasons (a low-effort AI PR and a stale community PR), not rejected on the merits. This supersedes them, authored by the issue owner, using `0o700` and adding a regression test.
--- *Raised on behalf of* @35C4n0r *by Coder Agents.* --- agent/agentscripts/agentscripts.go | 5 +++ agent/agentscripts/agentscripts_test.go | 44 +++++++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/agent/agentscripts/agentscripts.go b/agent/agentscripts/agentscripts.go index 153bbaa51a..81fc50a1eb 100644 --- a/agent/agentscripts/agentscripts.go +++ b/agent/agentscripts/agentscripts.go @@ -283,6 +283,11 @@ func (r *Runner) run(ctx context.Context, script codersdk.WorkspaceAgentScript, ) logger.Info(ctx, "running agent script", slog.F("script", script.Script)) + logDir := filepath.Dir(logPath) + if err = r.Filesystem.MkdirAll(logDir, 0o700); err != nil { + return xerrors.Errorf("create script log file directory %q: %w", logDir, err) + } + fileWriter, err := r.Filesystem.OpenFile(logPath, os.O_CREATE|os.O_RDWR, 0o600) if err != nil { return xerrors.Errorf("open %s script log file: %w", logPath, err) diff --git a/agent/agentscripts/agentscripts_test.go b/agent/agentscripts/agentscripts_test.go index c032ea1f83..ed8cc5ffc6 100644 --- a/agent/agentscripts/agentscripts_test.go +++ b/agent/agentscripts/agentscripts_test.go @@ -47,6 +47,50 @@ func TestExecuteBasic(t *testing.T) { require.Equal(t, "hello", log.Output) } +func TestExecuteCreatesMissingLogDir(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitShort) + + fs := afero.NewOsFs() + logger := testutil.Logger(t) + s, err := agentssh.NewServer(context.Background(), logger, prometheus.NewRegistry(), fs, agentexec.DefaultExecer, nil) + require.NoError(t, err) + t.Cleanup(func() { + _ = s.Close() + }) + + fLogger := newFakeScriptLogger() + runner := agentscripts.New(agentscripts.Options{ + LogDir: t.TempDir(), + DataDirBase: t.TempDir(), + Logger: logger, + SSHServer: s, + Filesystem: fs, + GetScriptLogger: func(uuid.UUID) agentscripts.ScriptLogger { + return fLogger + }, + }) + defer runner.Close() + + logPath := filepath.Join(t.TempDir(), "does", "not", "exist", "install.log") + + aAPI := agenttest.NewFakeAgentAPI(t, logger, nil, nil) + err = runner.Init([]codersdk.WorkspaceAgentScript{{ + LogSourceID: uuid.New(), + LogPath: logPath, + Script: "echo hello", + }}, aAPI.ScriptCompleted) + require.NoError(t, err) + require.NoError(t, runner.Execute(ctx, agentscripts.ExecuteAllScripts)) + + log := testutil.TryReceive(ctx, t, fLogger.logs) + require.Equal(t, "hello", log.Output) + + exists, err := afero.Exists(fs, logPath) + require.NoError(t, err) + require.True(t, exists, "expected log file to be created at %s", logPath) +} + func TestEnv(t *testing.T) { t.Parallel() fLogger := newFakeScriptLogger()