From c7799854b9942a1e95d0bbad8dc5c476e4fc11e6 Mon Sep 17 00:00:00 2001 From: naiba Date: Mon, 20 Jul 2026 17:02:21 +0000 Subject: [PATCH] fix(agentcompat): validate supervised executable paths Co-authored-by: naiba/CloudCode --- .../internal/process/supervisor.go | 23 ++-------- .../internal/process/supervisor_spec.go | 46 +++++++++++++++++++ .../internal/process/supervisor_test.go | 25 ++++++++++ 3 files changed, 74 insertions(+), 20 deletions(-) create mode 100644 integration/agentcompat/internal/process/supervisor_spec.go diff --git a/integration/agentcompat/internal/process/supervisor.go b/integration/agentcompat/internal/process/supervisor.go index edef60f0..9492c171 100644 --- a/integration/agentcompat/internal/process/supervisor.go +++ b/integration/agentcompat/internal/process/supervisor.go @@ -6,7 +6,6 @@ import ( "context" "errors" "fmt" - "io" "os" "os/exec" "sync" @@ -21,22 +20,6 @@ const ( Stderr Stream = "stderr" ) -type Spec struct { - Name string - Path string - Args []string - Dir string - Env []string - ExtraFiles []*os.File - Stdout io.Writer - Stderr io.Writer - MaxLogBytes int - TerminateTimeout time.Duration - KillTimeout time.Duration - Readiness func(Stream, string) bool - Credential *syscall.Credential -} - type Supervisor struct { ctx context.Context spec Spec @@ -62,10 +45,10 @@ func NewSupervisor(ctx context.Context, spec Spec) *Supervisor { } func (supervisor *Supervisor) Start() error { - if supervisor.spec.Name == "" || supervisor.spec.Path == "" || supervisor.spec.MaxLogBytes < 1 || supervisor.spec.TerminateTimeout <= 0 || supervisor.spec.KillTimeout <= 0 { - return errors.New("invalid process specification") + if err := supervisor.spec.validate(); err != nil { + return err } - command := exec.Command(supervisor.spec.Path, supervisor.spec.Args...) + command := exec.Command(supervisor.spec.Path, supervisor.spec.Args...) // #nosec G204 -- Absolute executable regular-file path is validated before fixed argv execution; no shell is invoked. command.Dir = supervisor.spec.Dir command.Env = supervisor.spec.Env if command.Env == nil { diff --git a/integration/agentcompat/internal/process/supervisor_spec.go b/integration/agentcompat/internal/process/supervisor_spec.go new file mode 100644 index 00000000..3a0863be --- /dev/null +++ b/integration/agentcompat/internal/process/supervisor_spec.go @@ -0,0 +1,46 @@ +//go:build linux + +package process + +import ( + "errors" + "fmt" + "io" + "os" + "path/filepath" + "syscall" + "time" +) + +type Spec struct { + Name string + Path string + Args []string + Dir string + Env []string + ExtraFiles []*os.File + Stdout io.Writer + Stderr io.Writer + MaxLogBytes int + TerminateTimeout time.Duration + KillTimeout time.Duration + Readiness func(Stream, string) bool + Credential *syscall.Credential +} + +func (spec Spec) validate() error { + if spec.Name == "" || spec.Path == "" || spec.MaxLogBytes < 1 || spec.TerminateTimeout <= 0 || spec.KillTimeout <= 0 { + return errors.New("invalid process specification") + } + if !filepath.IsAbs(spec.Path) { + return errors.New("process path must be absolute") + } + info, err := os.Stat(spec.Path) + if err != nil { + return fmt.Errorf("stat process path: %w", err) + } + if !info.Mode().IsRegular() || info.Mode()&0o111 == 0 { + return errors.New("process path must be an executable regular file") + } + return nil +} diff --git a/integration/agentcompat/internal/process/supervisor_test.go b/integration/agentcompat/internal/process/supervisor_test.go index 1fa46387..a3403df6 100644 --- a/integration/agentcompat/internal/process/supervisor_test.go +++ b/integration/agentcompat/internal/process/supervisor_test.go @@ -29,6 +29,31 @@ func TestSupervisor_CleanExit(t *testing.T) { requireNoError(t, supervisor.Wait(t.Context())) } +func TestSupervisor_StartRejectsUntrustedExecutablePaths(t *testing.T) { + tests := []struct { + name string + path string + }{ + {name: "relative", path: "relative-helper"}, + {name: "directory", path: t.TempDir()}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + // Given + supervisor := newHelperSupervisor(t.Context(), "clean", nil) + supervisor.spec.Path = test.path + + // When + err := supervisor.Start() + + // Then + if err == nil { + t.Fatal("untrusted process path was accepted") + } + }) + } +} + func TestSupervisor_RunsChildWithConfiguredCredential(t *testing.T) { // Given credentialDirectory, err := os.MkdirTemp("/tmp", "agentcompat-credential-")