From 263dedc58e85c7916dabeff708f181d972976832 Mon Sep 17 00:00:00 2001 From: dawn Date: Wed, 21 Oct 2026 22:01:18 +0300 Subject: [PATCH] spindle: refuse a workflow dir with too many files Signed-off-by: dawn --- spindle/notify_event.go | 4 ++-- spindle/server.go | 3 +++ spindle/workflow_limits.go | 8 +++++++ spindle/workflow_limits_test.go | 39 +++++++++++++++++++++++++++++++++ 4 files changed, 52 insertions(+), 2 deletions(-) create mode 100644 spindle/workflow_limits.go create mode 100644 spindle/workflow_limits_test.go diff --git a/spindle/notify_event.go b/spindle/notify_event.go index 58f05218e..abeb6cab9 100644 --- a/spindle/notify_event.go +++ b/spindle/notify_event.go @@ -355,9 +355,9 @@ func (s *Spindle) repoForPush(ctx context.Context, k knotclient.Knot, repoDid sy return repo, nil } -// the knot drops what a 4xx refuses, so a refusal that wraps errRevRefused gets one +// the knot drops what a 4xx refuses, so a refusal a retry would only repeat gets one func refusedAsStatus(err error) error { - if errors.Is(err, errRevRefused) { + if errors.Is(err, errRevRefused) || errors.Is(err, errTooManyWorkflowFiles) { return notifyStatus{http.StatusUnprocessableEntity, err} } return err diff --git a/spindle/server.go b/spindle/server.go index 4fe978a9c..7352011e9 100644 --- a/spindle/server.go +++ b/spindle/server.go @@ -1270,6 +1270,9 @@ func (s *Spindle) loadPipeline(ctx context.Context, repoUri, repoPath, rev strin if !ok || len(fields) != 3 || (fields[0] != "100644" && fields[0] != "100755") { continue } + if len(rawPipeline) >= maxWorkflowFiles { + return nil, "", fmt.Errorf("%w: more than %d files under %s", errTooManyWorkflowFiles, maxWorkflowFiles, workflow.WorkflowDir) + } contents, err := gitutil.Output(ctx, "-C", repoPath, "cat-file", "blob", fields[2]) if err != nil { return nil, "", fmt.Errorf("reading raw content of '%s': %w", path, err) diff --git a/spindle/workflow_limits.go b/spindle/workflow_limits.go new file mode 100644 index 000000000..ad075f3df --- /dev/null +++ b/spindle/workflow_limits.go @@ -0,0 +1,8 @@ +package spindle + +import "errors" + +// every workflow file costs a goroutine, a log file and some status rows +const maxWorkflowFiles = 32 + +var errTooManyWorkflowFiles = errors.New("too many workflow files") diff --git a/spindle/workflow_limits_test.go b/spindle/workflow_limits_test.go new file mode 100644 index 000000000..91fe528a9 --- /dev/null +++ b/spindle/workflow_limits_test.go @@ -0,0 +1,39 @@ +package spindle + +import ( + "context" + "errors" + "fmt" + "net/http" + "path/filepath" + "testing" +) + +func loadWorkflowFiles(t *testing.T, files int) error { + t.Helper() + src := newSourceRepo(t, "sha1") + for i := range files { + writeTestFile(t, filepath.Join(src, fmt.Sprintf(".tangled/workflows/w%03d.yaml", i)), "when: []\n") + } + commitEverything(t, src) + sha := gitInDir(t, src, "rev-parse", "HEAD") + _, _, err := (&Spindle{}).loadPipeline(context.Background(), src, filepath.Join(t.TempDir(), "checkout"), sha) + return err +} + +func TestLoadPipelineRefusesMoreWorkflowFilesThanItRuns(t *testing.T) { + if err := loadWorkflowFiles(t, maxWorkflowFiles); err != nil { + t.Fatalf("a repo with %d workflow files was refused: %v", maxWorkflowFiles, err) + } + if err := loadWorkflowFiles(t, maxWorkflowFiles+1); !errors.Is(err, errTooManyWorkflowFiles) { + t.Fatalf("a repo with %d workflow files loaded with err = %v, want errTooManyWorkflowFiles", maxWorkflowFiles+1, err) + } +} + +func TestTooManyWorkflowFilesCarryAStatusTheKnotDrops(t *testing.T) { + err := refusedAsStatus(fmt.Errorf("loading pipeline: %w", errTooManyWorkflowFiles)) + var coded interface{ HTTPStatus() int } + if !errors.As(err, &coded) || coded.HTTPStatus() != http.StatusUnprocessableEntity { + t.Fatalf("err = %v, want a 422", err) + } +} -- 2.54.0