vikingowl
c44db99b41
Plugin loader resolves HookSpec.Exec as a relative path joined to the plugin directory, and manifest.checkSafePath rejects absolute paths and '..' traversal — Exec was always meant to be an executable path. The hook executor was wrapping it in 'sh -c', adding a redundant shell interpretation step that turned any space, quote, or metacharacter in the path into command-injection surface. Switch to exec.Command(path) with no shell wrapping. Closes audit finding C3. Adds a regression test that fails under the old 'sh -c' code path: a canary file created via shell sequencing remains absent when the executor treats Exec as a literal filename. Hook command tests now write small /bin/sh scripts to t.TempDir and point Exec at those — matching production semantics (resolved binary path) rather than inline shell strings.
73 lines
2.0 KiB
Go
73 lines
2.0 KiB
Go
package hook
|
|
|
|
import (
|
|
"bytes"
|
|
"context"
|
|
"fmt"
|
|
"os/exec"
|
|
"time"
|
|
)
|
|
|
|
// CommandExecutor runs a shell command and interprets its stdin/stdout.
|
|
type CommandExecutor struct {
|
|
def HookDef
|
|
}
|
|
|
|
// NewCommandExecutor constructs a CommandExecutor for the given definition.
|
|
func NewCommandExecutor(def HookDef) *CommandExecutor {
|
|
return &CommandExecutor{def: def}
|
|
}
|
|
|
|
// Execute runs the hook command, pipes payload to stdin, and reads stdout.
|
|
func (c *CommandExecutor) Execute(ctx context.Context, payload []byte) (HookResult, error) {
|
|
timeout := c.def.timeout()
|
|
ctx, cancel := context.WithTimeout(ctx, timeout)
|
|
defer cancel()
|
|
|
|
start := time.Now()
|
|
// Exec is a resolved binary path (see plugin/loader.go). No shell wrapping —
|
|
// shell metacharacters in the path are treated as literal filename bytes.
|
|
cmd := exec.CommandContext(ctx, c.def.Exec)
|
|
cmd.Stdin = bytes.NewReader(payload)
|
|
|
|
var stdout bytes.Buffer
|
|
cmd.Stdout = &stdout
|
|
|
|
runErr := cmd.Run()
|
|
duration := time.Since(start)
|
|
|
|
// Determine exit code and whether it was a timeout.
|
|
exitCode := 0
|
|
if runErr != nil {
|
|
if ctx.Err() != nil {
|
|
// Context deadline exceeded — apply fail_open policy.
|
|
action := Deny
|
|
if c.def.FailOpen {
|
|
action = Allow
|
|
}
|
|
return HookResult{Action: action, Duration: duration}, fmt.Errorf("hook %q: timed out after %v", c.def.Name, timeout)
|
|
}
|
|
if exitErr, ok := runErr.(*exec.ExitError); ok {
|
|
exitCode = exitErr.ExitCode()
|
|
} else {
|
|
// Unexpected error launching the process.
|
|
action := Deny
|
|
if c.def.FailOpen {
|
|
action = Allow
|
|
}
|
|
return HookResult{Action: action, Duration: duration}, fmt.Errorf("hook %q: %w", c.def.Name, runErr)
|
|
}
|
|
}
|
|
|
|
action, transformed, err := ParseHookOutput(stdout.Bytes(), exitCode)
|
|
if err != nil {
|
|
failAction := Deny
|
|
if c.def.FailOpen {
|
|
failAction = Allow
|
|
}
|
|
return HookResult{Action: failAction, Duration: duration}, fmt.Errorf("hook %q: %w", c.def.Name, err)
|
|
}
|
|
|
|
return HookResult{Action: action, Output: transformed, Duration: duration}, nil
|
|
}
|