From adb9c6818504ee46dfead1360c1f0324eaf59877 Mon Sep 17 00:00:00 2001 From: Molecule AI Core Platform Lead Date: Fri, 24 Apr 2026 11:07:43 +0000 Subject: [PATCH] fix(tests): path validation before docker check + a2a queue mock in tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - container_files.go: move validateRelPath before h.docker==nil check in deleteViaEphemeral so F1085 traversal tests fire even when Docker is absent in CI (fixes TestDeleteViaEphemeral_F1085_RejectsTraversal) - a2a_proxy_test.go: add EnqueueA2A mock expectation in TestHandleA2ADispatchError_ContextDeadline — DeadlineExceeded now triggers the #1870 queue path; mock the INSERT to return an error so the test correctly falls through to the expected 503 Retry-After shape Co-Authored-By: Claude Sonnet 4.6 --- .../internal/handlers/a2a_proxy_test.go | 15 ++++++++++----- .../internal/handlers/container_files.go | 11 ++++++++--- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/workspace-server/internal/handlers/a2a_proxy_test.go b/workspace-server/internal/handlers/a2a_proxy_test.go index 89ca8029..dcad98e2 100644 --- a/workspace-server/internal/handlers/a2a_proxy_test.go +++ b/workspace-server/internal/handlers/a2a_proxy_test.go @@ -1158,13 +1158,18 @@ func TestDispatchA2A_ContextDeadline_NoCancelAdded(t *testing.T) { // --- handleA2ADispatchError --- func TestHandleA2ADispatchError_ContextDeadline(t *testing.T) { - setupTestDB(t) + mock := setupTestDB(t) setupTestRedis(t) handler := NewWorkspaceHandler(newTestBroadcaster(), nil, "http://localhost:8080", t.TempDir()) - // No workspace row expected — maybeMarkContainerDead with nil - // provisioner short-circuits, and activity-log insert is suppressed - // (logActivity=false). + // maybeMarkContainerDead with nil provisioner short-circuits (no DB call). + // activity-log insert is suppressed (logActivity=false). + // DeadlineExceeded → isUpstreamBusyError=true → EnqueueA2A attempted. + // Mock the INSERT INTO a2a_queue to fail so we fall through to 503. + mock.ExpectQuery(`INSERT INTO a2a_queue`). + WithArgs("ws-dl", nil, PriorityTask, "{}", "message/send", nil). + WillReturnError(fmt.Errorf("test: queue unavailable")) + _, _, perr := handler.handleA2ADispatchError( context.Background(), "ws-dl", "", []byte("{}"), "message/send", context.DeadlineExceeded, 1, false, @@ -1172,7 +1177,7 @@ func TestHandleA2ADispatchError_ContextDeadline(t *testing.T) { if perr == nil { t.Fatal("expected error, got nil") } - // DeadlineExceeded is classified as upstream-busy → 503 with Retry-After. + // EnqueueA2A failed → falls through to legacy 503 with Retry-After. if perr.Status != http.StatusServiceUnavailable { t.Errorf("got status %d, want 503", perr.Status) } diff --git a/workspace-server/internal/handlers/container_files.go b/workspace-server/internal/handlers/container_files.go index ad06924f..290bd5f7 100644 --- a/workspace-server/internal/handlers/container_files.go +++ b/workspace-server/internal/handlers/container_files.go @@ -159,9 +159,6 @@ func (h *TemplatesHandler) writeViaEphemeral(ctx context.Context, volumeName str // deleteViaEphemeral deletes a file from a named volume using an ephemeral container. func (h *TemplatesHandler) deleteViaEphemeral(ctx context.Context, volumeName, filePath string) error { - if h.docker == nil { - return fmt.Errorf("docker not available") - } // CWE-78/CWE-22: exec form binds rm to the /configs volume regardless // of path traversal in filePath. The bind mount volumeName:/configs // constrains rm; exec form prevents shell interpolation. @@ -169,9 +166,17 @@ func (h *TemplatesHandler) deleteViaEphemeral(ctx context.Context, volumeName, f // The concat form is the critical fix: rm receives ONE path argument // so ".." is processed literally — rm -rf /configs/foo/../bar resolves // to /configs/bar (inside volume), not bar (outside volume). + // + // Path validation MUST come before the docker-available check so that + // traversal inputs are rejected even in test/CI environments where + // Docker is absent. This ensures F1085 regression tests catch real + // violations rather than short-circuiting on "docker not available". if err := validateRelPath(filePath); err != nil { return err } + if h.docker == nil { + return fmt.Errorf("docker not available") + } resp, err := h.docker.ContainerCreate(ctx, &container.Config{ Image: "alpine:latest",