mptyl/ThothII
1
0
Fork 0
Code Issues 25 Pull Requests Actions Packages Projects Releases 1 Wiki Activity

fix(auth): harden unified diagnostic execution

Browse Source
This commit is contained in:
marcopan
2026-08-17 17:25:26 +02:00
parent 30ee9433dc
commit ef244ab56d
18 changed files with 670 additions and 68 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tools/tht/internal/compose/process_unix.go
+1 -2
View File
@@ -3,7 +3,6 @@
package compose
import (
"errors"
"os/exec"
"syscall"
"time"
@@ -32,6 +31,6 @@ func terminateProcess(command *exec.Cmd, done <-chan error) error {
case <-done:
return nil
case <-time.After(finalTerminationBound):
return errors.New("Docker command could not be reaped")
return ErrProcessReap
}
}
tools/tht/internal/compose/process_windows.go
+1 -2
View File
@@ -3,7 +3,6 @@
package compose
import (
"errors"
"os/exec"
"syscall"
"time"
@@ -25,6 +24,6 @@ func terminateProcess(command *exec.Cmd, done <-chan error) error {
case <-done:
return nil
case <-time.After(finalTerminationBound):
return errors.New("Docker command could not be reaped")
return ErrProcessReap
}
}
tools/tht/internal/compose/runner.go
+175 -15
View File
@@ -3,22 +3,36 @@ package compose
import (
"context"
"crypto/rand"
"encoding/hex"
"errors"
"fmt"
"io"
"os"
"os/exec"
"strings"
"sync"
"time"
"github.com/aritmolab/thothii/tools/tht/internal/config"
)
const defaultCaptureBytes = 4 * 1024 * 1024
const maximumCaptureBytes = 64 * 1024 * 1024
const cleanupCaptureBytes = 4 * 1024
const containerCleanupBound = 2 * time.Second
// ErrOutputLimit reports that a child exceeded one of its capture limits.
var ErrOutputLimit = errors.New("Docker command output limit exceeded")
// ErrProcessReap reports that a cancelled Docker CLI could not be reaped in its final bound.
var ErrProcessReap = errors.New("Docker command process could not be reaped")
// ErrContainerCleanup reports that Docker could not confirm removal of a named one-shot container.
var ErrContainerCleanup = errors.New("Docker one-shot container cleanup failed")
var terminateProcessForRunner = terminateProcess
// CaptureLimits bounds each captured stream while the child is running.
type CaptureLimits struct {
StdoutBytes int
@@ -64,13 +78,26 @@ func (r execRunner) Run(ctx context.Context, args []string, stdin io.Reader) (Re
// RunBounded invokes Docker while enforcing both stream limits during capture.
func (r execRunner) RunBounded(ctx context.Context, args []string, stdin io.Reader, limits CaptureLimits) (Result, error) {
return r.runBounded(ctx, args, stdin, limits, true)
}
func (r execRunner) runBounded(ctx context.Context, args []string, stdin io.Reader, limits CaptureLimits, manageOneShot bool) (Result, error) {
if err := validCaptureLimits(limits); err != nil {
return Result{}, err
}
if err := ctx.Err(); err != nil {
return Result{}, err
}
command := exec.Command(r.binary, args...)
preparedArgs := append([]string(nil), args...)
containerName := ""
if manageOneShot {
var err error
preparedArgs, containerName, err = prepareOneShot(preparedArgs)
if err != nil {
return Result{}, err
}
}
command := exec.Command(r.binary, preparedArgs...)
configureProcess(command)
command.Stdin = stdin
overflow := make(chan struct{}, 1)
@@ -83,35 +110,168 @@ func (r execRunner) RunBounded(ctx context.Context, args []string, stdin io.Read
}
done := make(chan error, 1)
go func() { done <- command.Wait() }()
var err error
var processErr error
var lifecycleErr error
interrupted := false
select {
case err = <-done:
case processErr = <-done:
case <-ctx.Done():
_ = terminateProcess(command, done)
err = ctx.Err()
interrupted = true
processErr = ctx.Err()
if err := terminateProcessForRunner(command, done); err != nil {
lifecycleErr = errors.Join(lifecycleErr, ErrProcessReap)
}
case <-overflow:
_ = terminateProcess(command, done)
err = ErrOutputLimit
interrupted = true
processErr = ErrOutputLimit
if err := terminateProcessForRunner(command, done); err != nil {
lifecycleErr = errors.Join(lifecycleErr, ErrProcessReap)
}
}
if interrupted && containerName != "" {
if err := r.cleanupOneShotContainer(containerName); err != nil {
lifecycleErr = errors.Join(lifecycleErr, ErrContainerCleanup)
}
}
result := Result{Stdout: stdout.String(), Stderr: stderr.String()}
if command.ProcessState != nil {
result.ExitCode = command.ProcessState.ExitCode()
}
if stdout.Overflowed() || stderr.Overflowed() {
return result, ErrOutputLimit
return result, errors.Join(ErrOutputLimit, lifecycleErr)
}
if err == nil {
if interrupted {
return result, errors.Join(processErr, lifecycleErr)
}
if processErr == nil {
return result, nil
}
if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) || errors.Is(err, ErrOutputLimit) {
return result, err
}
var exitError *exec.ExitError
if errors.As(err, &exitError) {
if errors.As(processErr, &exitError) {
result.ExitCode = exitError.ExitCode()
return result, err
return result, processErr
}
return result, err
return result, processErr
}
// NewOneShotContainerName returns a Docker-safe, cross-process unique name with a bounded prefix.
func NewOneShotContainerName(prefix string) (string, error) {
if len(prefix) < 1 || len(prefix) > 96 || !safeContainerName(prefix) {
return "", errors.New("Docker one-shot container name is invalid")
}
random := make([]byte, 12)
if _, err := rand.Read(random); err != nil {
return "", errors.New("Docker one-shot container name is unavailable")
}
return prefix + "-" + hex.EncodeToString(random), nil
}
func prepareOneShot(args []string) ([]string, string, error) {
for runIndex, arg := range args {
if arg != "run" || !(runIndex == 0 || args[0] == "compose") {
continue
}
_, name, valid := oneShotRunOptions(args, runIndex)
if !valid {
continue
}
if name != "" {
return args, name, nil
}
return args, "", nil
}
return args, "", nil
}
func oneShotRunOptions(args []string, runIndex int) (rmIndex int, name string, valid bool) {
rmIndex = -1
valueOptions := map[string]struct{}{
"--name": {}, "--entrypoint": {}, "--network": {}, "--mount": {}, "--env": {}, "-e": {},
"--user": {}, "-u": {}, "--volume": {}, "-v": {}, "--workdir": {}, "-w": {},
"--label": {}, "-l": {}, "--pull": {}, "--cap-add": {}, "--cap-drop": {},
}
for index := runIndex + 1; index < len(args); index++ {
arg := args[index]
if arg == "--rm" {
rmIndex = index
continue
}
if _, expectsValue := valueOptions[arg]; expectsValue {
if index+1 >= len(args) {
return -1, "", false
}
if arg == "--name" {
if !safeContainerName(args[index+1]) {
return -1, "", false
}
name = args[index+1]
}
index++
continue
}
if strings.HasPrefix(arg, "--name=") {
name = strings.TrimPrefix(arg, "--name=")
if !safeContainerName(name) {
return -1, "", false
}
continue
}
if strings.HasPrefix(arg, "-") {
continue
}
return rmIndex, name, rmIndex >= 0
}
return -1, "", false
}
func safeContainerName(value string) bool {
if len(value) < 1 || len(value) > 128 {
return false
}
for index, character := range value {
letter := character >= 'a' && character <= 'z' || character >= 'A' && character <= 'Z'
digit := character >= '0' && character <= '9'
if letter || digit || index > 0 && (character == '-' || character == '_' || character == '.') {
continue
}
return false
}
return true
}
func (r execRunner) cleanupOneShotContainer(name string) error {
if !safeContainerName(name) {
return ErrContainerCleanup
}
ctx, cancel := context.WithTimeout(context.Background(), containerCleanupBound)
defer cancel()
exists, err := r.oneShotContainerExists(ctx, name)
if err != nil || !exists {
if err != nil {
return ErrContainerCleanup
}
return nil
}
limits := CaptureLimits{StdoutBytes: cleanupCaptureBytes, StderrBytes: cleanupCaptureBytes}
if _, err := r.runBounded(ctx, []string{"container", "rm", "-f", name}, nil, limits, false); err != nil {
return ErrContainerCleanup
}
exists, err = r.oneShotContainerExists(ctx, name)
if err != nil || exists {
return ErrContainerCleanup
}
return nil
}
func (r execRunner) oneShotContainerExists(ctx context.Context, name string) (bool, error) {
limits := CaptureLimits{StdoutBytes: cleanupCaptureBytes, StderrBytes: cleanupCaptureBytes}
result, err := r.runBounded(ctx, []string{
"container", "ls", "--all", "--quiet", "--filter", "name=^/" + name + "$",
}, nil, limits, false)
if err != nil {
return false, ErrContainerCleanup
}
return strings.TrimSpace(result.Stdout) != "", nil
}
// RunBounded uses the production runner's during-capture limits while retaining compatibility
tools/tht/internal/compose/runner_test.go
+141 -1
View File
@@ -55,7 +55,7 @@ func TestRunnerCancelsAndReapsAHangingChildWithinFinalBound(t *testing.T) {
t.Parallel()
runner := NewRunner(writeExecutable(t, "#!/bin/sh\ntrap '' TERM INT\nwhile :; do sleep 1; done\n"))
ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond)
ctx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond)
defer cancel()
started := time.Now()
_, err := RunBounded(runner, ctx, []string{"compose", "run", "--rm", "core"}, nil, CaptureLimits{
@@ -70,6 +70,98 @@ func TestRunnerCancelsAndReapsAHangingChildWithinFinalBound(t *testing.T) {
}
}
func TestRunnerCleansUpNamedComposeContainerAfterOverflow(t *testing.T) {
logFile := filepath.Join(t.TempDir(), "calls.log")
marker := filepath.Join(t.TempDir(), "container-present")
if err := os.WriteFile(marker, []byte("present"), 0o600); err != nil {
t.Fatal(err)
}
t.Setenv("THT_RUNNER_TEST_LOG", logFile)
t.Setenv("THT_RUNNER_TEST_MARKER", marker)
t.Setenv("THT_RUNNER_TEST_MODE", "flood")
runner := NewRunner(writeExecutable(t, cleanupAwareDockerScript))
name := "thothii-cleanup-overflow-sentinel"
_, err := RunBounded(runner, context.Background(), []string{
"compose", "run", "--rm", "--name", name, "core",
}, nil, CaptureLimits{StdoutBytes: 1024, StderrBytes: 1024})
if !errors.Is(err, ErrOutputLimit) {
t.Fatalf("RunBounded() error = %v, want ErrOutputLimit", err)
}
assertContainerCleanup(t, logFile, marker, name)
}
func TestRunnerCleansUpNamedComposeContainerAfterCancellation(t *testing.T) {
logFile := filepath.Join(t.TempDir(), "calls.log")
marker := filepath.Join(t.TempDir(), "container-present")
if err := os.WriteFile(marker, []byte("present"), 0o600); err != nil {
t.Fatal(err)
}
t.Setenv("THT_RUNNER_TEST_LOG", logFile)
t.Setenv("THT_RUNNER_TEST_MARKER", marker)
t.Setenv("THT_RUNNER_TEST_MODE", "hang")
runner := NewRunner(writeExecutable(t, cleanupAwareDockerScript))
name := "thothii-cleanup-cancel-sentinel"
ctx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond)
defer cancel()
_, err := RunBounded(runner, ctx, []string{
"compose", "run", "--rm", "--name", name, "core",
}, nil, CaptureLimits{StdoutBytes: 1024, StderrBytes: 1024})
if !errors.Is(err, context.DeadlineExceeded) {
t.Fatalf("RunBounded() error = %v, want deadline exceeded", err)
}
assertContainerCleanup(t, logFile, marker, name)
}
func TestRunnerSurfacesCleanupFailureWithoutContainerName(t *testing.T) {
logFile := filepath.Join(t.TempDir(), "calls.log")
marker := filepath.Join(t.TempDir(), "container-present")
if err := os.WriteFile(marker, []byte("present"), 0o600); err != nil {
t.Fatal(err)
}
t.Setenv("THT_RUNNER_TEST_LOG", logFile)
t.Setenv("THT_RUNNER_TEST_MARKER", marker)
t.Setenv("THT_RUNNER_TEST_MODE", "cleanup-fails")
runner := NewRunner(writeExecutable(t, cleanupAwareDockerScript))
name := "thothii-cleanup-secret-sentinel"
ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond)
defer cancel()
_, err := RunBounded(runner, ctx, []string{
"compose", "run", "--rm", "--name", name, "core",
}, nil, CaptureLimits{StdoutBytes: 1024, StderrBytes: 1024})
if err == nil || !strings.Contains(err.Error(), "one-shot container cleanup failed") {
t.Fatalf("RunBounded() error = %v, want safe cleanup failure", err)
}
if strings.Contains(err.Error(), name) {
t.Fatalf("RunBounded() exposed the container name: %v", err)
}
}
func TestRunnerPropagatesAReapingFailure(t *testing.T) {
original := terminateProcessForRunner
terminateProcessForRunner = func(command *exec.Cmd, done <-chan error) error {
_ = terminateProcess(command, done)
return ErrProcessReap
}
t.Cleanup(func() { terminateProcessForRunner = original })
runner := NewRunner(writeExecutable(t, "#!/bin/sh\ntrap '' TERM INT\nwhile :; do sleep 1; done\n"))
ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond)
defer cancel()
_, err := RunBounded(runner, ctx, []string{"version"}, nil, CaptureLimits{
StdoutBytes: 1024, StderrBytes: 1024,
})
if !errors.Is(err, context.DeadlineExceeded) || !errors.Is(err, ErrProcessReap) {
t.Fatalf("RunBounded() error = %v, want deadline and reap failure", err)
}
}
func TestRunnerReturnsTheChildExitCode(t *testing.T) {
t.Parallel()
@@ -86,6 +178,22 @@ func TestRunnerReturnsTheChildExitCode(t *testing.T) {
}
}
func TestRunnerRetainsTheExactNormalRmInvocationForAnUnnamedOneShot(t *testing.T) {
t.Parallel()
runner := NewRunner(writeExecutable(t, "#!/bin/sh\nprintf '%s\\n' \"$@\"\n"))
result, err := runner.Run(context.Background(), []string{
"compose", "run", "--rm", "--no-deps", "core", "node", "diagnostic.js",
}, nil)
if err != nil {
t.Fatal(err)
}
got := strings.Fields(result.Stdout)
if strings.Join(got, " ") != "compose run --rm --no-deps core node diagnostic.js" {
t.Fatalf("one-shot argv = %#v, want exact original --rm invocation", got)
}
}
func TestRunnerReportsMissingDocker(t *testing.T) {
t.Parallel()
@@ -140,3 +248,35 @@ func writeExecutable(t *testing.T, contents string) string {
}
return path
}
func assertContainerCleanup(t *testing.T, logFile, marker, name string) {
t.Helper()
calls, err := os.ReadFile(logFile)
if err != nil {
t.Fatal(err)
}
if !strings.Contains(string(calls), "container rm -f "+name) {
t.Fatalf("Docker calls = %q, want bounded container cleanup", calls)
}
if _, err := os.Stat(marker); !os.IsNotExist(err) {
t.Fatalf("one-shot container marker still exists: %v", err)
}
}
const cleanupAwareDockerScript = `#!/bin/sh
printf '%s\n' "$*" >> "$THT_RUNNER_TEST_LOG"
if [ "$1" = "container" ] && [ "$2" = "ls" ]; then
if [ -f "$THT_RUNNER_TEST_MARKER" ]; then printf '%s\n' container-id; fi
exit 0
fi
if [ "$1" = "container" ] && [ "$2" = "rm" ]; then
if [ "$THT_RUNNER_TEST_MODE" = "cleanup-fails" ]; then exit 9; fi
rm -f "$THT_RUNNER_TEST_MARKER"
exit 0
fi
if [ "$THT_RUNNER_TEST_MODE" = "flood" ]; then
while :; do printf '0123456789abcdef'; printf 'fedcba9876543210' >&2; done
fi
trap '' TERM INT
while :; do sleep 1; done
`