mirror of
https://github.com/vxcontrol/pentagi.git
synced 2026-08-24 03:56:32 +00:00
test(docker,flows): cover the listing cap paths, bound the container-list path count
The truncation slice and the demux stdout byte-cap had no docker-layer tests — only the handler's Truncated wiring was exercised through the fake, so a mis-slice or a dropped cap would have gone unnoticed. Extract find-output parsing into a pure parseFindEntries and take the byte cap as a demuxExecStdout parameter, then unit-test both boundaries (at cap / cap+1 / over-limit stream). Also bound how many paths one container-files request may list (maxContainerListPaths), so the per-path entry cap can't be multiplied by an attacker-chosen path count into a large fan-out or response body. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
18234f05a3
commit
7cd22ccbc9
@@ -640,7 +640,7 @@ func (dc *dockerClient) ListContainerDir(
|
||||
if err != nil {
|
||||
return ContainerDirListing{}, fmt.Errorf("failed to attach list exec for '%s': %w", dirPath, err)
|
||||
}
|
||||
output, readErr := demuxExecStdout(resp.Reader)
|
||||
output, readErr := demuxExecStdout(resp.Reader, maxListStdoutBytes)
|
||||
resp.Close()
|
||||
if readErr != nil {
|
||||
return ContainerDirListing{}, fmt.Errorf("failed to read list output for '%s': %w", dirPath, readErr)
|
||||
@@ -654,22 +654,7 @@ func (dc *dockerClient) ListContainerDir(
|
||||
return ContainerDirListing{}, fmt.Errorf("list command failed for '%s' with exit code %d: %s", dirPath, inspect.ExitCode, string(output))
|
||||
}
|
||||
|
||||
// find -print0 emits `<absolute-path>\0<absolute-path>\0…`.
|
||||
entryPaths := make([]string, 0)
|
||||
for _, tok := range strings.Split(string(output), "\x00") {
|
||||
if tok == "" {
|
||||
continue
|
||||
}
|
||||
entryPaths = append(entryPaths, tok)
|
||||
}
|
||||
|
||||
// Over the cap, list the first maxListEntries and flag the truncation rather
|
||||
// than failing the whole listing — the caller reports the partiality.
|
||||
truncated := false
|
||||
if len(entryPaths) > maxListEntries {
|
||||
entryPaths = entryPaths[:maxListEntries]
|
||||
truncated = true
|
||||
}
|
||||
entryPaths, truncated := parseFindEntries(output)
|
||||
|
||||
stats, failures := statContainerEntries(ctx, entryPaths, containerListWorkers, func(ctx context.Context, entryPath string) (container.PathStat, error) {
|
||||
return dc.ContainerStatPath(ctx, containerID, entryPath)
|
||||
@@ -695,10 +680,27 @@ func (dc *dockerClient) ListContainerDir(
|
||||
return listing, nil
|
||||
}
|
||||
|
||||
// parseFindEntries splits `find -print0` output (NUL-delimited absolute paths),
|
||||
// dropping empties, and caps the result at maxListEntries — returning truncated=true
|
||||
// so the caller reports the partiality instead of failing or silently dropping.
|
||||
func parseFindEntries(output []byte) (entries []string, truncated bool) {
|
||||
for _, tok := range strings.Split(string(output), "\x00") {
|
||||
if tok == "" {
|
||||
continue
|
||||
}
|
||||
entries = append(entries, tok)
|
||||
}
|
||||
if len(entries) > maxListEntries {
|
||||
return entries[:maxListEntries], true
|
||||
}
|
||||
return entries, false
|
||||
}
|
||||
|
||||
// demuxExecStdout reads a non-TTY Docker exec stream — stdout and stderr
|
||||
// interleaved as frames with an 8-byte header (stream id + big-endian size) —
|
||||
// and returns only the stdout bytes.
|
||||
func demuxExecStdout(r io.Reader) ([]byte, error) {
|
||||
// and returns only the stdout bytes, erroring if stdout exceeds maxStdout so a
|
||||
// compromised sandbox can't stream unbounded output into memory.
|
||||
func demuxExecStdout(r io.Reader, maxStdout int) ([]byte, error) {
|
||||
var stdout bytes.Buffer
|
||||
header := make([]byte, 8)
|
||||
for {
|
||||
@@ -719,8 +721,8 @@ func demuxExecStdout(r io.Reader) ([]byte, error) {
|
||||
if _, err := io.CopyN(sink, r, size); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if stdout.Len() > maxListStdoutBytes {
|
||||
return nil, fmt.Errorf("listing output exceeded %d bytes", maxListStdoutBytes)
|
||||
if stdout.Len() > maxStdout {
|
||||
return nil, fmt.Errorf("listing output exceeded %d bytes", maxStdout)
|
||||
}
|
||||
}
|
||||
return stdout.Bytes(), nil
|
||||
|
||||
@@ -0,0 +1,55 @@
|
||||
package docker
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"encoding/binary"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func listingFrame(streamID byte, payload string) []byte {
|
||||
h := make([]byte, 8)
|
||||
h[0] = streamID
|
||||
binary.BigEndian.PutUint32(h[4:8], uint32(len(payload)))
|
||||
return append(h, []byte(payload)...)
|
||||
}
|
||||
|
||||
func TestParseFindEntries(t *testing.T) {
|
||||
entries, truncated := parseFindEntries([]byte("a\x00b\x00\x00c\x00"))
|
||||
if truncated {
|
||||
t.Error("small input must not truncate")
|
||||
}
|
||||
if len(entries) != 3 || entries[0] != "a" || entries[1] != "b" || entries[2] != "c" {
|
||||
t.Fatalf("split/skip-empty wrong: %q", entries)
|
||||
}
|
||||
|
||||
// exactly at the cap → not truncated
|
||||
e, tr := parseFindEntries([]byte(strings.Repeat("x\x00", maxListEntries)))
|
||||
if tr || len(e) != maxListEntries {
|
||||
t.Fatalf("at cap: truncated=%v len=%d (want false, %d)", tr, len(e), maxListEntries)
|
||||
}
|
||||
|
||||
// cap+1 → truncated, capped to maxListEntries
|
||||
e, tr = parseFindEntries([]byte(strings.Repeat("x\x00", maxListEntries+1)))
|
||||
if !tr || len(e) != maxListEntries {
|
||||
t.Fatalf("over cap: truncated=%v len=%d (want true, %d)", tr, len(e), maxListEntries)
|
||||
}
|
||||
}
|
||||
|
||||
func TestDemuxExecStdout_StdoutOnly_AndByteCap(t *testing.T) {
|
||||
// stdout is returned; stderr (id 2) is discarded
|
||||
in := append(listingFrame(1, "hello"), listingFrame(2, "diagnostic")...)
|
||||
out, err := demuxExecStdout(bytes.NewReader(in), 1<<20)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected err: %v", err)
|
||||
}
|
||||
if string(out) != "hello" {
|
||||
t.Fatalf("want stdout only, got %q", out)
|
||||
}
|
||||
|
||||
// stdout exceeding the cap → error before materializing the full buffer
|
||||
_, err = demuxExecStdout(bytes.NewReader(listingFrame(1, strings.Repeat("x", 50))), 10)
|
||||
if err == nil || !strings.Contains(err.Error(), "listing output exceeded") {
|
||||
t.Fatalf("want cap error, got %v", err)
|
||||
}
|
||||
}
|
||||
@@ -38,6 +38,11 @@ import (
|
||||
// resources/ ← user resources copied from user_resources table; also pushed to container /work/resources/
|
||||
// container/ ← files synced from the container via pull; never sent back to container
|
||||
|
||||
// maxContainerListPaths bounds how many directory paths one container-files
|
||||
// request may list, so the per-path entry cap can't be multiplied into an
|
||||
// unbounded fan-out or response.
|
||||
const maxContainerListPaths = 128
|
||||
|
||||
type pendingUpload struct {
|
||||
fileName string
|
||||
dstPath string
|
||||
@@ -940,6 +945,14 @@ func (s *FlowFileService) GetFlowContainerFiles(c *gin.Context) {
|
||||
response.Error(c, response.ErrFlowFilesInvalidRequest, err)
|
||||
return
|
||||
}
|
||||
// Bound the number of paths per request so the per-path entry cap can't be
|
||||
// multiplied by an attacker-chosen path count into a huge fan-out / response.
|
||||
if len(containerPaths) > maxContainerListPaths {
|
||||
err = fmt.Errorf("too many paths requested (%d, limit %d)", len(containerPaths), maxContainerListPaths)
|
||||
logger.FromContext(c).WithError(err).WithField("flow_id", flowID).Error("too many container paths")
|
||||
response.Error(c, response.ErrFlowFilesInvalidRequest, err)
|
||||
return
|
||||
}
|
||||
if len(containerPaths) == 0 {
|
||||
containerPaths = []string{docker.WorkFolderPathInContainer}
|
||||
}
|
||||
|
||||
@@ -4,6 +4,8 @@ import (
|
||||
"fmt"
|
||||
"net/http"
|
||||
"os"
|
||||
"strconv"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"pentagi/pkg/docker"
|
||||
@@ -132,3 +134,29 @@ func TestGetFlowContainerFiles_TruncatedFlagSurfaced(t *testing.T) {
|
||||
require.Equal(t, http.StatusOK, *code)
|
||||
assert.True(t, resp.Truncated)
|
||||
}
|
||||
|
||||
// The per-request path count is bounded so the per-path entry cap can't be
|
||||
// multiplied by an attacker-chosen number of paths.
|
||||
func TestGetFlowContainerFiles_TooManyPathsRejected(t *testing.T) {
|
||||
buildQuery := func(n int) string {
|
||||
parts := make([]string, n)
|
||||
for i := range parts {
|
||||
parts[i] = "paths[]=/p" + strconv.Itoa(i)
|
||||
}
|
||||
return strings.Join(parts, "&")
|
||||
}
|
||||
|
||||
// One over the cap → 400 before any docker call.
|
||||
db := setupFlowFileServiceTestDB(t)
|
||||
seedFlow(t, db, 1, 1)
|
||||
svc := NewFlowFileService(db, t.TempDir(), &fakeDockerClient{running: true}, nil)
|
||||
c, w := newFlowFileTestContext(http.MethodGet,
|
||||
"/flows/1/files/container?"+buildQuery(maxContainerListPaths+1), nil,
|
||||
[]string{"flow_files.view", "containers.view"}, 1, 1)
|
||||
svc.GetFlowContainerFiles(c)
|
||||
require.Equal(t, http.StatusBadRequest, w.Code)
|
||||
|
||||
// Exactly at the cap is allowed.
|
||||
code, _ := listContainerFiles(t, &fakeDockerClient{running: true}, buildQuery(maxContainerListPaths))
|
||||
require.Equal(t, http.StatusOK, *code)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user