From 5342002d77ed730a9d391cb296b98c7bb4723ec6 Mon Sep 17 00:00:00 2001 From: Johannes Edmeier Date: Thu, 12 Feb 2026 21:01:50 +0100 Subject: [PATCH 1/2] fix: make mount failure a hard error in diskfill createBundle A silent mount failure causes the fill to write to the sidecar's own filesystem instead of the target, which is wrong behavior that should fail fast. --- go/action_kit_commons/diskfill/diskfill_runc.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/action_kit_commons/diskfill/diskfill_runc.go b/go/action_kit_commons/diskfill/diskfill_runc.go index 87ad31a7..784c88c3 100644 --- a/go/action_kit_commons/diskfill/diskfill_runc.go +++ b/go/action_kit_commons/diskfill/diskfill_runc.go @@ -142,7 +142,7 @@ func createBundle(ctx context.Context, r ociruntime.OciRuntime, sidecar SidecarO if targetPath != "" { if err := bundle.MountFromProcess(ctx, sidecar.TargetProcess.Pid, targetPath, mountpointInContainer); err != nil { - log.Warn().Err(err).Msgf("failed to mount %s", targetPath) + return nil, fmt.Errorf("failed to mount %s: %w", targetPath, err) } } From ef8962a90e4c2ea6b7b0f57243f9f6a40cb60d70 Mon Sep 17 00:00:00 2001 From: Johannes Edmeier Date: Thu, 12 Feb 2026 21:01:56 +0100 Subject: [PATCH 2/2] feat: add target path validation for fill disk attacks Add CheckPathWritableRunc and CheckPathWritableProcess functions that verify the target directory exists and is writable before starting the fill. The runc variant creates a short-lived sidecar in the target's mount namespace; the process variant uses direct commands. Both clean up after themselves. --- go/action_kit_commons/CHANGELOG.md | 4 +++ .../diskfill/diskfill_process.go | 19 +++++++++-- .../diskfill/diskfill_runc.go | 27 ++++++++++++++- .../diskfill/read_diskspace.go | 4 +-- go/action_kit_commons/network/dig_runner.go | 2 -- .../ociruntime/container_bundle.go | 33 +++++++------------ go/action_kit_commons/ociruntime/utils.go | 15 ++++----- 7 files changed, 65 insertions(+), 39 deletions(-) diff --git a/go/action_kit_commons/CHANGELOG.md b/go/action_kit_commons/CHANGELOG.md index 9315b08d..a099043b 100644 --- a/go/action_kit_commons/CHANGELOG.md +++ b/go/action_kit_commons/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## 1.5.15 + +- Add filldisk helpers to validate target directory + ## 1.5.14 - Set OOM score adjustment in disc fill command diff --git a/go/action_kit_commons/diskfill/diskfill_process.go b/go/action_kit_commons/diskfill/diskfill_process.go index 41a3101e..fd2c87c0 100644 --- a/go/action_kit_commons/diskfill/diskfill_process.go +++ b/go/action_kit_commons/diskfill/diskfill_process.go @@ -7,13 +7,14 @@ package diskfill import ( "context" "fmt" - "github.com/rs/zerolog/log" - "github.com/steadybit/action-kit/go/action_kit_commons/utils" "os/exec" "path/filepath" "strconv" "strings" "time" + + "github.com/rs/zerolog/log" + "github.com/steadybit/action-kit/go/action_kit_commons/utils" ) type diskfillProcess struct { @@ -92,3 +93,17 @@ func (df *diskfillProcess) Args() []string { func (df *diskfillProcess) Noop() bool { return df.cmd.Args[0] == "echo" && df.cmd.Args[1] == "noop" } + +func CheckPathWritableProcess(ctx context.Context, targetPath string) error { + if out, err := utils.RootCommandContext(ctx, "test", "-d", targetPath).CombinedOutput(); err != nil { + return fmt.Errorf("target path %q does not exist: %w: %s", targetPath, err, string(out)) + } + + checkFile := filepath.Join(targetPath, ".steadybit-diskfill-check") + if out, err := utils.RootCommandContext(ctx, "touch", checkFile).CombinedOutput(); err != nil { + return fmt.Errorf("target path %q is not writable: %w: %s", targetPath, err, string(out)) + } + + _ = utils.RootCommandContext(ctx, "rm", "-f", checkFile).Run() + return nil +} diff --git a/go/action_kit_commons/diskfill/diskfill_runc.go b/go/action_kit_commons/diskfill/diskfill_runc.go index 784c88c3..7d933c30 100644 --- a/go/action_kit_commons/diskfill/diskfill_runc.go +++ b/go/action_kit_commons/diskfill/diskfill_runc.go @@ -5,6 +5,7 @@ package diskfill import ( + "bytes" "context" "fmt" "path/filepath" @@ -142,7 +143,7 @@ func createBundle(ctx context.Context, r ociruntime.OciRuntime, sidecar SidecarO if targetPath != "" { if err := bundle.MountFromProcess(ctx, sidecar.TargetProcess.Pid, targetPath, mountpointInContainer); err != nil { - return nil, fmt.Errorf("failed to mount %s: %w", targetPath, err) + return nil, err } } @@ -183,6 +184,30 @@ func createBundle(ctx context.Context, r ociruntime.OciRuntime, sidecar SidecarO return bundle, nil } +func CheckPathWritableRunc(ctx context.Context, r ociruntime.OciRuntime, sidecar SidecarOpts, targetPath string) error { + bundle, err := createBundle(ctx, r, sidecar, targetPath, "sh", "-c", fmt.Sprintf("test -d %s && touch %s/.steadybit-diskfill-check && rm -f %s/.steadybit-diskfill-check", mountpointInContainer, mountpointInContainer, mountpointInContainer)) + if err != nil { + return fmt.Errorf("target path %q is not accessible: %w", targetPath, err) + } + defer func() { + if err := bundle.Remove(); err != nil { + log.Warn().Str("id", bundle.ContainerId()).Err(err).Msg("failed to remove bundle") + } + }() + + var errb bytes.Buffer + err = r.Run(ctx, bundle, ociruntime.IoOpts{Stderr: &errb}) + defer func() { + if err := r.Delete(context.Background(), bundle.ContainerId(), true); err != nil { + log.Debug().Str("id", bundle.ContainerId()).Err(err).Msg("failed to delete check container") + } + }() + if err != nil { + return fmt.Errorf("target path %q does not exist or is not writable: %w: %s", targetPath, err, errb.String()) + } + return nil +} + func getNextContainerId(executionId uuid.UUID, suffix string) string { return fmt.Sprintf("sb-diskfill-%d-%s-%s", time.Now().UnixMilli(), utils.ShortenUUID(executionId), suffix) } diff --git a/go/action_kit_commons/diskfill/read_diskspace.go b/go/action_kit_commons/diskfill/read_diskspace.go index 01ec5a3d..2b0b5386 100644 --- a/go/action_kit_commons/diskfill/read_diskspace.go +++ b/go/action_kit_commons/diskfill/read_diskspace.go @@ -68,9 +68,7 @@ func readDiskUsageProcess(ctx context.Context, path string) (*DiskUsage, error) var outb, errb bytes.Buffer cmd.Stdout = &outb cmd.Stderr = &errb - - err := cmd.Run() - if err != nil { + if err := cmd.Run(); err != nil { return nil, fmt.Errorf("failed to read disk usage: %w: %s", err, errb.String()) } diff --git a/go/action_kit_commons/network/dig_runner.go b/go/action_kit_commons/network/dig_runner.go index 8229eda6..722c0050 100644 --- a/go/action_kit_commons/network/dig_runner.go +++ b/go/action_kit_commons/network/dig_runner.go @@ -16,7 +16,6 @@ import ( "github.com/steadybit/action-kit/go/action_kit_commons/utils" "io" "os/exec" - "runtime/trace" ) type DigRunner interface { @@ -50,7 +49,6 @@ type RuncDigRunner struct { var _ DigRunner = (*RuncDigRunner)(nil) func (r *RuncDigRunner) Run(ctx context.Context, arg []string, stdin io.Reader) ([]byte, error) { - defer trace.StartRegion(ctx, "RuncDigRunner.Run").End() id := getNextContainerId(r.Sidecar.ExecutionId, "dig", r.Sidecar.IdSuffix) bundle, err := r.Runc.Create(ctx, "/", id) diff --git a/go/action_kit_commons/ociruntime/container_bundle.go b/go/action_kit_commons/ociruntime/container_bundle.go index 09a8884c..17358af9 100644 --- a/go/action_kit_commons/ociruntime/container_bundle.go +++ b/go/action_kit_commons/ociruntime/container_bundle.go @@ -5,16 +5,15 @@ package ociruntime import ( - "bytes" "context" "errors" "fmt" - "github.com/rs/zerolog/log" - "github.com/steadybit/action-kit/go/action_kit_commons/utils" "os" "path/filepath" - "runtime/trace" "strconv" + + "github.com/rs/zerolog/log" + "github.com/steadybit/action-kit/go/action_kit_commons/utils" ) type containerBundle struct { @@ -86,38 +85,28 @@ func (b *containerBundle) mountRootfsOverlay(ctx context.Context, image string) } func (b *containerBundle) CopyFileFromProcess(ctx context.Context, pid int, fromPath, toPath string) error { - defer trace.StartRegion(ctx, "utils.CopyFileFromProcessToBundle").End() - var out bytes.Buffer - cmd := utils.RootCommandContext(ctx, "cat", filepath.Join("/proc", strconv.Itoa(pid), "root", fromPath)) - cmd.Stdout = &out - cmd.Stderr = &out - if err := cmd.Run(); err != nil { - return fmt.Errorf("%w: %s", err, out.String()) + out, err := utils.RootCommandContext(ctx, "cat", filepath.Join("/proc", strconv.Itoa(pid), "root", fromPath)).CombinedOutput() + if err != nil { + return fmt.Errorf("failed to mount %s (%w): %s", fromPath, err, out) } - return os.WriteFile(filepath.Join(b.path, "rootfs", toPath), out.Bytes(), 0644) + return os.WriteFile(filepath.Join(b.path, "rootfs", toPath), out, 0644) } func (b *containerBundle) MountFromProcess(ctx context.Context, fromPid int, fromPath, toPath string) error { - defer trace.StartRegion(ctx, "utils.MountFromProcessToBundle").End() - mountpoint := filepath.Join(b.path, "rootfs", toPath) log.Trace(). Int("fromPid", fromPid). Str("fromPath", fromPath). - Str("mountpoint", mountpoint). + Str("mount-point", mountpoint). Msg("mount from process to bundle") if err := os.Mkdir(mountpoint, 0755); err != nil && !os.IsExist(err) { - return fmt.Errorf("failed to create mountpoint %s: %w", mountpoint, err) + return fmt.Errorf("failed to create mount point %s: %w", mountpoint, err) } - var out bytes.Buffer - cmd := utils.RootCommandContext(ctx, nsmountPath, strconv.Itoa(fromPid), fromPath, strconv.Itoa(os.Getpid()), mountpoint) - cmd.Stdout = &out - cmd.Stderr = &out - if err := cmd.Run(); err != nil { - return fmt.Errorf("%w: %s", err, out.String()) + if out, err := utils.RootCommandContext(ctx, nsmountPath, strconv.Itoa(fromPid), fromPath, strconv.Itoa(os.Getpid()), mountpoint).CombinedOutput(); err != nil { + return fmt.Errorf("failed to mount %s (%w): %s", fromPath, err, out) } b.addFinalizer(func() error { return unmount(context.Background(), mountpoint) diff --git a/go/action_kit_commons/ociruntime/utils.go b/go/action_kit_commons/ociruntime/utils.go index 66495dd1..6f43f775 100644 --- a/go/action_kit_commons/ociruntime/utils.go +++ b/go/action_kit_commons/ociruntime/utils.go @@ -515,19 +515,16 @@ func searchNamespacePathInProcess(ctx context.Context, inode uint64, nsType spec } func readCgroupPath(ctx context.Context, pid int) (string, error) { - var out bytes.Buffer - cmd := utils.RootCommandContext(ctx, nsenterPath, "-t", "1", "-C", "--", "cat", filepath.Join("/proc", strconv.Itoa(pid), "cgroup")) - cmd.Stdout = &out - cmd.Stderr = &out - if err := cmd.Run(); err != nil { - return "", fmt.Errorf("%w: %s", err, out.String()) + out, err := utils.RootCommandContext(ctx, nsenterPath, "-t", "1", "-C", "--", "cat", filepath.Join("/proc", strconv.Itoa(pid), "cgroup")).CombinedOutput() + if err != nil { + return "", fmt.Errorf("%w: %s", err, out) } - if cgroup := parseProcCgroupFile(out.String()); cgroup != "" { + if cgroup := parseProcCgroupFile(string(out)); cgroup != "" { return cgroup, nil - } else { - return "", fmt.Errorf("failed to read cgroup for pid %d\n%s", pid, out.String()) } + + return "", fmt.Errorf("failed to read cgroup for pid %d\n%s", pid, out) } func parseProcCgroupFile(s string) string {