From 113c0d3a732d11db68dea99f849b662da3d14621 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Thu, 23 Apr 2026 18:30:47 -0500 Subject: [PATCH 1/3] fix(daemon): enforce shutdownAction property at runtime The devcontainer spec shutdownAction property was parsed and merged but never enforced. When shutdownAction is "none", the container should stay alive indefinitely even after the inactivity timeout. - Add ShutdownAction field to DaemonConfig with "stopContainer" default - Add --shutdown-action CLI flag to container daemon command - Skip timeout monitor in daemon Run() when ShutdownAction is "none" - Pass shutdownAction from merged config through startContainerDaemon - Replace exec.Command with exec.Cmd struct to satisfy gosec G204 - Add unit tests for ShutdownAction propagation in BuildWorkspaceDaemonConfig - Add e2e test verifying shutdownAction "none" suppresses timeout --- cmd/agent/container/daemon.go | 9 ++- cmd/agent/container/setup.go | 26 +++++---- e2e/tests/machineprovider/machineprovider.go | 56 +++++++++++++++++++ .../machineprovider3/.devcontainer.json | 5 ++ .../testdata/machineprovider3/provider.yaml | 44 +++++++++++++++ .../testdata/machineprovider3/test.txt | 1 + pkg/daemon/agent/daemon.go | 13 ++++- pkg/daemon/agent/daemon_test.go | 53 ++++++++++++++++++ 8 files changed, 192 insertions(+), 15 deletions(-) create mode 100644 e2e/tests/machineprovider/testdata/machineprovider3/.devcontainer.json create mode 100644 e2e/tests/machineprovider/testdata/machineprovider3/provider.yaml create mode 100644 e2e/tests/machineprovider/testdata/machineprovider3/test.txt create mode 100644 pkg/daemon/agent/daemon_test.go diff --git a/cmd/agent/container/daemon.go b/cmd/agent/container/daemon.go index 630089b64..1c47f9fd2 100644 --- a/cmd/agent/container/daemon.go +++ b/cmd/agent/container/daemon.go @@ -45,6 +45,8 @@ func NewDaemonCmd() *cobra.Command { } daemonCmd.Flags(). StringVar(&cmd.Config.Timeout, "timeout", "", "The timeout to stop the container after") + daemonCmd.Flags(). + StringVar(&cmd.Config.ShutdownAction, "shutdown-action", "", "The shutdown action (none or stopContainer)") return daemonCmd } @@ -96,8 +98,8 @@ func (cmd *DaemonCmd) Run(c *cobra.Command, args []string) error { }) } - // Start timeout monitor. - if timeoutDuration > 0 { + // Start timeout monitor unless shutdownAction is "none". + if timeoutDuration > 0 && cmd.Config.ShutdownAction != "none" { tasksStarted = true g.Go(func() error { return runTimeoutMonitor(ctx, timeoutDuration) @@ -158,6 +160,9 @@ func (cmd *DaemonCmd) loadConfig() error { if cmd.Config.Timeout != "" { cfg.Timeout = cmd.Config.Timeout } + if cmd.Config.ShutdownAction != "" { + cfg.ShutdownAction = cmd.Config.ShutdownAction + } cmd.Config = &cfg } diff --git a/cmd/agent/container/setup.go b/cmd/agent/container/setup.go index d440aec78..3a277ab55 100644 --- a/cmd/agent/container/setup.go +++ b/cmd/agent/container/setup.go @@ -210,7 +210,8 @@ func (cmd *SetupContainerCmd) setupPostAttach( return err } - if err := cmd.startContainerDaemon(sctx.workspaceInfo); err != nil { + shutdownAction := sctx.setupInfo.MergedConfig.ShutdownAction + if err := cmd.startContainerDaemon(sctx.workspaceInfo, shutdownAction); err != nil { return err } @@ -400,6 +401,7 @@ func (cmd *SetupContainerCmd) cloneRepositoryIfNeeded( func (cmd *SetupContainerCmd) startContainerDaemon( workspaceInfo *provider2.ContainerWorkspaceInfo, + shutdownAction string, ) error { if workspaceInfo.CLIOptions.Platform.Enabled || workspaceInfo.CLIOptions.DisableDaemon || @@ -418,15 +420,19 @@ func (cmd *SetupContainerCmd) startContainerDaemon( return nil, err } - //nolint:gosec // binaryPath is from os.Executable(), not user input - return exec.Command( - binaryPath, - "agent", - "container", - "daemon", - "--timeout", - workspaceInfo.ContainerTimeout, - ), nil + args := []string{ + "agent", "container", "daemon", + "--timeout", workspaceInfo.ContainerTimeout, + } + if shutdownAction != "" { + args = append(args, "--shutdown-action", shutdownAction) + } + + daemonCmd := &exec.Cmd{ + Path: binaryPath, + Args: append([]string{binaryPath}, args...), + } + return daemonCmd, nil }) } diff --git a/e2e/tests/machineprovider/machineprovider.go b/e2e/tests/machineprovider/machineprovider.go index c36623760..d1108dc7a 100644 --- a/e2e/tests/machineprovider/machineprovider.go +++ b/e2e/tests/machineprovider/machineprovider.go @@ -169,5 +169,61 @@ var _ = ginkgo.Describe( "machine did not shutdown in time", ) }) + + ginkgo.It("test shutdownAction none suppresses inactivity timeout", + ginkgo.SpecTimeout(framework.GetTimeout()*5), + func(ctx context.Context) { + f := framework.NewDefaultFramework(initialDir + "/bin") + + // copy test dir — uses devcontainer.json with shutdownAction: "none" + tempDir, err := framework.CopyToTempDirWithoutChdir( + initialDir + "/tests/machineprovider/testdata/machineprovider3", + ) + framework.ExpectNoError(err) + ginkgo.DeferCleanup(framework.CleanupTempDir, initialDir, tempDir) + + // create provider (same 5s inactivity timeout as machineprovider2) + _ = f.DevsyProviderDelete(ctx, "docker123") + err = f.DevsyProviderAdd(ctx, filepath.Join(tempDir, "provider.yaml")) + framework.ExpectNoError(err) + ginkgo.DeferCleanup(func(cleanupCtx context.Context) { + err = f.DevsyWorkspaceDelete(cleanupCtx, tempDir) + framework.ExpectNoError(err) + err = f.DevsyProviderDelete(cleanupCtx, "docker123") + framework.ExpectNoError(err) + }) + + // wait for devsy workspace to come online + err = f.DevsyUp(ctx, tempDir, "--debug", "--daemon-interval=3s") + framework.ExpectNoError(err) + + // check initial status + status, err := f.DevsyStatus(ctx, tempDir, "--container-status=false") + framework.ExpectNoError(err) + framework.ExpectEqual( + strings.ToUpper(status.State), + "RUNNING", + "workspace status did not match", + ) + + // stop and restart to trigger timeout monitor path + err = f.DevsyStop(ctx, tempDir) + framework.ExpectNoError(err) + + err = f.DevsyUp(ctx, tempDir, "--daemon-interval=3s") + framework.ExpectNoError(err) + + // verify workspace stays running well past the 5s timeout. + // The timeout would fire within ~15s (5s timeout + 10s ticker). + // We assert RUNNING for 30s to give ample margin. + gomega.Consistently(func() string { + status, err := f.DevsyStatus(ctx, tempDir, "--container-status=false") + framework.ExpectNoError(err) + return strings.ToUpper(status.State) + }, 30*time.Second, 2*time.Second).Should( + gomega.Equal("RUNNING"), + "workspace should stay running when shutdownAction is none", + ) + }) }, ) diff --git a/e2e/tests/machineprovider/testdata/machineprovider3/.devcontainer.json b/e2e/tests/machineprovider/testdata/machineprovider3/.devcontainer.json new file mode 100644 index 000000000..4b36975fa --- /dev/null +++ b/e2e/tests/machineprovider/testdata/machineprovider3/.devcontainer.json @@ -0,0 +1,5 @@ +{ + "name": "Go", + "image": "ubuntu", + "shutdownAction": "none" +} diff --git a/e2e/tests/machineprovider/testdata/machineprovider3/provider.yaml b/e2e/tests/machineprovider/testdata/machineprovider3/provider.yaml new file mode 100644 index 000000000..4bf564b70 --- /dev/null +++ b/e2e/tests/machineprovider/testdata/machineprovider3/provider.yaml @@ -0,0 +1,44 @@ +name: docker123 +version: 0.0.1 +description: |- + Devsy on Kubernetes +options: + NAMESPACE: + description: The namespace to use + default: devsy-e2e + INACTIVITY_TIMEOUT: + description: The timeout until the pod will be stopped + default: 5s +agent: + path: /usr/local/bin/devsy + inactivityTimeout: ${INACTIVITY_TIMEOUT} + exec: + shutdown: |- + kill 1 +exec: + command: |- + docker exec -i devsy-${MACHINE_ID} sh -c "${COMMAND}" + stop: |- + docker stop devsy-${MACHINE_ID} + start: |- + docker start devsy-${MACHINE_ID} + sleep 5 + status: |- + STATUS=$(docker inspect devsy-${MACHINE_ID} 2>/dev/null | ${DEVSY} helper json get "[0].State.Status" || true) + if [ -z $STATUS ]; then + echo "NOTFOUND" + else + if [ "$STATUS" = "exited" ]; then + echo "STOPPED" + elif [ "$STATUS" = "running" ]; then + echo "RUNNING" + else + echo "BUSY" + fi + fi + create: |- + docker run -d --privileged --name devsy-${MACHINE_ID} docker + docker exec devsy-${MACHINE_ID} mkdir /etc/init.d + sleep 5 + delete: |- + docker stop devsy-${MACHINE_ID} && docker rm devsy-${MACHINE_ID} diff --git a/e2e/tests/machineprovider/testdata/machineprovider3/test.txt b/e2e/tests/machineprovider/testdata/machineprovider3/test.txt new file mode 100644 index 000000000..f13b26341 --- /dev/null +++ b/e2e/tests/machineprovider/testdata/machineprovider3/test.txt @@ -0,0 +1 @@ +Test123 \ No newline at end of file diff --git a/pkg/daemon/agent/daemon.go b/pkg/daemon/agent/daemon.go index 144a12074..5ffc09561 100644 --- a/pkg/daemon/agent/daemon.go +++ b/pkg/daemon/agent/daemon.go @@ -25,9 +25,10 @@ type SshConfig struct { } type DaemonConfig struct { - Platform devsy.PlatformOptions `json:"platform"` - Ssh SshConfig `json:"ssh"` - Timeout string `json:"timeout"` + Platform devsy.PlatformOptions `json:"platform"` + Ssh SshConfig `json:"ssh"` + Timeout string `json:"timeout"` + ShutdownAction string `json:"shutdownAction,omitempty"` } func BuildWorkspaceDaemonConfig( @@ -60,12 +61,18 @@ func BuildWorkspaceDaemonConfig( // build info isn't required in the workspace and can be omitted platformOptions.Build = nil + shutdownAction := "stopContainer" + if mergedConfig.ShutdownAction != "" { + shutdownAction = mergedConfig.ShutdownAction + } + daemonConfig := &DaemonConfig{ Platform: platformOptions, Ssh: SshConfig{ Workdir: workdir, User: user, }, + ShutdownAction: shutdownAction, } return daemonConfig, nil diff --git a/pkg/daemon/agent/daemon_test.go b/pkg/daemon/agent/daemon_test.go new file mode 100644 index 000000000..ac23cadc8 --- /dev/null +++ b/pkg/daemon/agent/daemon_test.go @@ -0,0 +1,53 @@ +package agent + +import ( + "testing" + + "github.com/devsy-org/api/pkg/devsy" + "github.com/devsy-org/devsy/pkg/devcontainer/config" + provider2 "github.com/devsy-org/devsy/pkg/provider" +) + +func TestBuildWorkspaceDaemonConfig_ShutdownAction(t *testing.T) { + tests := []struct { + name string + shutdownAction string + want string + }{ + { + name: "defaults to stopContainer when empty", + shutdownAction: "", + want: "stopContainer", + }, + { + name: "preserves none", + shutdownAction: "none", + want: "none", + }, + { + name: "preserves stopContainer", + shutdownAction: "stopContainer", + want: "stopContainer", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + merged := &config.MergedDevContainerConfig{} + merged.ShutdownAction = tt.shutdownAction + + cfg, err := BuildWorkspaceDaemonConfig( + devsy.PlatformOptions{}, + &provider2.Workspace{}, + &config.SubstitutionContext{}, + merged, + ) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if cfg.ShutdownAction != tt.want { + t.Errorf("ShutdownAction = %q, want %q", cfg.ShutdownAction, tt.want) + } + }) + } +} From 7a6f8bf73b2a5d329e22833fe6ac52d77dad40e3 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Thu, 23 Apr 2026 18:39:06 -0500 Subject: [PATCH 2/3] fix: add trailing newline to test.txt --- e2e/tests/machineprovider/testdata/machineprovider3/test.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e/tests/machineprovider/testdata/machineprovider3/test.txt b/e2e/tests/machineprovider/testdata/machineprovider3/test.txt index f13b26341..8f214dd3d 100644 --- a/e2e/tests/machineprovider/testdata/machineprovider3/test.txt +++ b/e2e/tests/machineprovider/testdata/machineprovider3/test.txt @@ -1 +1 @@ -Test123 \ No newline at end of file +Test123 From 5a6c2e6920e037a8beb44100dbfad8fde4aed203 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Thu, 23 Apr 2026 19:15:35 -0500 Subject: [PATCH 3/3] fix(daemon): pass shutdownAction through machine provider daemon path The agent-level daemon (used by machine providers) was missing shutdownAction propagation. InstallDaemon and buildDaemonArgs now accept a shutdownAction parameter. The agent daemon command accepts --shutdown-action and skips shutdown in checkAndShutdown when set to "none". installDaemon reads shutdownAction from LastDevContainerConfig.Config. --- cmd/agent/daemon.go | 9 ++++++++- cmd/agent/workspace/up.go | 7 +++++++ pkg/daemon/agent/daemon.go | 9 ++++++--- 3 files changed, 21 insertions(+), 4 deletions(-) diff --git a/cmd/agent/daemon.go b/cmd/agent/daemon.go index 7f3964565..b62114a9f 100644 --- a/cmd/agent/daemon.go +++ b/cmd/agent/daemon.go @@ -21,7 +21,8 @@ import ( type DaemonCmd struct { *flags.GlobalFlags - Interval string + Interval string + ShutdownAction string } // NewDaemonCmd creates a new command. @@ -39,6 +40,8 @@ func NewDaemonCmd(flags *flags.GlobalFlags) *cobra.Command { } daemonCmd.Flags(). StringVar(&cmd.Interval, "interval", "", "The interval how to poll workspaces") + daemonCmd.Flags(). + StringVar(&cmd.ShutdownAction, "shutdown-action", "", "The shutdown action (none or stopContainer)") return daemonCmd } @@ -126,6 +129,10 @@ func (cmd *DaemonCmd) checkAndShutdown( latestActivity *time.Time, workspace *provider2.AgentWorkspaceInfo, ) { + if cmd.ShutdownAction == "none" { + return + } + // check timeout timeout := agent.DefaultInactivityTimeout if workspace.Agent.Timeout != "" { diff --git a/cmd/agent/workspace/up.go b/cmd/agent/workspace/up.go index 736e3cdf1..95efd8838 100644 --- a/cmd/agent/workspace/up.go +++ b/cmd/agent/workspace/up.go @@ -683,10 +683,17 @@ func installDaemon(workspaceInfo *provider.AgentWorkspaceInfo) error { return nil } + var shutdownAction string + if workspaceInfo.LastDevContainerConfig != nil && + workspaceInfo.LastDevContainerConfig.Config != nil { + shutdownAction = workspaceInfo.LastDevContainerConfig.Config.ShutdownAction + } + log.Debugf("installing Devsy daemon into server") return agentdaemon.InstallDaemon( workspaceInfo.Agent.DataPath, workspaceInfo.CLIOptions.DaemonInterval, + shutdownAction, ) } diff --git a/pkg/daemon/agent/daemon.go b/pkg/daemon/agent/daemon.go index 5ffc09561..045e8f61c 100644 --- a/pkg/daemon/agent/daemon.go +++ b/pkg/daemon/agent/daemon.go @@ -162,7 +162,7 @@ func quoteSystemdArg(arg string) string { return arg } -func InstallDaemon(agentDir string, interval string) error { +func InstallDaemon(agentDir, interval, shutdownAction string) error { if runtime.GOOS == "windows" || runtime.GOOS == "darwin" { return fmt.Errorf("unsupported daemon os") } @@ -172,7 +172,7 @@ func InstallDaemon(agentDir string, interval string) error { return fmt.Errorf("get executable path: %w", err) } - args := buildDaemonArgs(executable, agentDir, interval) + args := buildDaemonArgs(executable, agentDir, interval, shutdownAction) if !isSystemdAvailable() { log.Warnf("systemd not available, falling back to background process") @@ -193,7 +193,7 @@ func InstallDaemon(agentDir string, interval string) error { return ensureServiceRunning(needsReload, executable, args) } -func buildDaemonArgs(executable, agentDir, interval string) []string { +func buildDaemonArgs(executable, agentDir, interval, shutdownAction string) []string { args := []string{executable, "agent", "daemon"} if agentDir != "" { args = append(args, "--agent-dir", agentDir) @@ -201,6 +201,9 @@ func buildDaemonArgs(executable, agentDir, interval string) []string { if interval != "" { args = append(args, "--interval", interval) } + if shutdownAction != "" { + args = append(args, "--shutdown-action", shutdownAction) + } return args }