canvas: fix orgs-page + contextmenu test regressions from PR #1243

.github/workflows/ci.yml

@@ -6,12 +6,12 @@ on:
   pull_request:
     branches: [main, staging]
 
-# Queue new CI runs when a commit arrives on the same ref.
-# New runs queue instead of cancelling each other — prevents
-# the single self-hosted macOS arm64 runner from being monopolised.
+# Cancel in-progress CI runs when a new commit arrives on the same ref.
+# This prevents multiple stale runs from queuing behind each other and
+# monopolising the self-hosted macOS arm64 runner.
 concurrency:
   group: ci-${{ github.ref }}
-  cancel-in-progress: false
+  cancel-in-progress: true
 
 jobs:
   # Detect which paths changed so downstream jobs can skip when only

canvas/src/app/__tests__/orgs-page.test.tsx

@@ -15,7 +15,7 @@
  *   - Polling: provisioning orgs schedule a 5s refresh (fake timers)
  */
 import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
-import { render, screen, cleanup } from "@testing-library/react";
+import { render, screen, cleanup, waitFor } from "@testing-library/react";
 
 // ── Hoisted mocks ────────────────────────────────────────────────────────────
 // vi.mock factories are hoisted above imports; any captured references must
@@ -112,10 +112,12 @@ afterEach(() => {
 
 describe("/orgs — auth guard", () => {
   it("redirects to login when session is null", async () => {
+    vi.useRealTimers();
     mockFetchSession.mockResolvedValueOnce(null);
     render(<OrgsPage />);
-    await vi.advanceTimersByTimeAsync(50);
-    expect(mockRedirectToLogin).toHaveBeenCalled();
+    await waitFor(() => {
+      expect(mockRedirectToLogin).toHaveBeenCalled();
+    });
     // Must not attempt to fetch /cp/orgs before auth is established
     expect(mockFetch).not.toHaveBeenCalledWith(
       expect.stringContaining("/cp/orgs"),
@@ -126,22 +128,26 @@ describe("/orgs — auth guard", () => {
 
 describe("/orgs — error state", () => {
   it("shows error + Retry button when /cp/orgs fails", async () => {
-    mockFetchSession.mockResolvedValue({ userId: "u-1" });
+    vi.useRealTimers();
+    mockFetchSession.mockResolvedValueOnce({ userId: "u-1" });
     mockFetch.mockResolvedValueOnce(notOk(500, "db down"));
     render(<OrgsPage />);
-    await vi.advanceTimersByTimeAsync(50);
-    expect(screen.getByText(/Error:/)).toBeTruthy();
+    await waitFor(() => {
+      expect(screen.getByText(/Error:/)).toBeTruthy();
+    });
     expect(screen.getByRole("button", { name: /retry/i })).toBeTruthy();
   });
 });
 
 describe("/orgs — empty list", () => {
   it("renders EmptyState with CreateOrgForm when user has zero orgs", async () => {
-    mockFetchSession.mockResolvedValue({ userId: "u-1" });
+    vi.useRealTimers();
+    mockFetchSession.mockResolvedValueOnce({ userId: "u-1" });
     mockFetch.mockResolvedValueOnce(okJson({ orgs: [] }));
     render(<OrgsPage />);
-    await vi.advanceTimersByTimeAsync(50);
-    expect(screen.getByText(/don't have any organizations/i)).toBeTruthy();
+    await waitFor(() => {
+      expect(screen.getByText(/don't have any organizations/i)).toBeTruthy();
+    });
     expect(screen.getByRole("button", { name: /create organization/i })).toBeTruthy();
   });
 });
@@ -150,7 +156,8 @@ describe("/orgs — CTAs by status", () => {
   const session = { userId: "u-1" };
 
   it("running → Open link targets {slug}.moleculesai.app", async () => {
-    mockFetchSession.mockResolvedValue(session);
+    vi.useRealTimers();
+    mockFetchSession.mockResolvedValueOnce(session);
     mockFetch.mockResolvedValueOnce(
       okJson({
         orgs: [
@@ -167,13 +174,15 @@ describe("/orgs — CTAs by status", () => {
       })
     );
     render(<OrgsPage />);
-    await vi.advanceTimersByTimeAsync(50);
-    const link = screen.getByRole("link", { name: /open/i }) as HTMLAnchorElement;
-    expect(link.href).toBe("https://acme.moleculesai.app/");
+    await waitFor(() => {
+      const link = screen.getByRole("link", { name: /open/i });
+      expect(link.getAttribute("href")).toBe("https://acme.moleculesai.app/");
+    });
   });
 
   it("awaiting_payment → Complete payment link to /pricing?org=<slug>", async () => {
-    mockFetchSession.mockResolvedValue(session);
+    vi.useRealTimers();
+    mockFetchSession.mockResolvedValueOnce(session);
     mockFetch.mockResolvedValueOnce(
       okJson({
         orgs: [
@@ -190,15 +199,15 @@ describe("/orgs — CTAs by status", () => {
       })
     );
     render(<OrgsPage />);
-    await vi.advanceTimersByTimeAsync(50);
-    const link = screen.getByRole("link", {
-      name: /complete payment/i,
-    }) as HTMLAnchorElement;
-    expect(link.getAttribute("href")).toBe("/pricing?org=beta-co");
+    await waitFor(() => {
+      const link = screen.getByRole("link", { name: /complete payment/i });
+      expect(link.getAttribute("href")).toBe("/pricing?org=beta-co");
+    });
   });
 
   it("failed → mailto support link", async () => {
-    mockFetchSession.mockResolvedValue(session);
+    vi.useRealTimers();
+    mockFetchSession.mockResolvedValueOnce(session);
     mockFetch.mockResolvedValueOnce(
       okJson({
         orgs: [
@@ -215,19 +224,19 @@ describe("/orgs — CTAs by status", () => {
       })
     );
     render(<OrgsPage />);
-    await vi.advanceTimersByTimeAsync(50);
-    const link = screen.getByRole("link", {
-      name: /contact support/i,
-    }) as HTMLAnchorElement;
-    expect(link.getAttribute("href")).toBe("mailto:support@moleculesai.app");
+    await waitFor(() => {
+      const link = screen.getByRole("link", { name: /contact support/i });
+      expect(link.getAttribute("href")).toBe("mailto:support@moleculesai.app");
+    });
   });
 });
 
 describe("/orgs — post-checkout banner", () => {
   it("renders CheckoutBanner when ?checkout=success and scrubs the URL", async () => {
+    vi.useRealTimers();
     setLocation("https://moleculesai.app/orgs?checkout=success");
     const replaceState = vi.spyOn(window.history, "replaceState");
-    mockFetchSession.mockResolvedValue({ userId: "u-1" });
+    mockFetchSession.mockResolvedValueOnce({ userId: "u-1" });
     mockFetch.mockResolvedValueOnce(
       okJson({
         orgs: [
@@ -244,36 +253,41 @@ describe("/orgs — post-checkout banner", () => {
       })
     );
     render(<OrgsPage />);
-    await vi.advanceTimersByTimeAsync(50);
-    expect(screen.getByText(/Payment confirmed/i)).toBeTruthy();
+    await waitFor(() => {
+      expect(screen.getByText(/Payment confirmed/i)).toBeTruthy();
+    });
     // URL must be rewritten to drop the ?checkout flag so reload doesn't re-show the banner
     expect(replaceState).toHaveBeenCalled();
     const callArgs = replaceState.mock.calls[0];
     expect(callArgs[2]).toBe("/orgs");
   });
 
   it("does NOT render CheckoutBanner without ?checkout=success", async () => {
-    mockFetchSession.mockResolvedValue({ userId: "u-1" });
+    vi.useRealTimers();
+    mockFetchSession.mockResolvedValueOnce({ userId: "u-1" });
     mockFetch.mockResolvedValueOnce(okJson({ orgs: [] }));
     render(<OrgsPage />);
-    await vi.advanceTimersByTimeAsync(50);
-    expect(screen.getByText(/don't have any organizations/i)).toBeTruthy();
+    await waitFor(() => {
+      expect(screen.getByText(/don't have any organizations/i)).toBeTruthy();
+    });
     expect(screen.queryByText(/Payment confirmed/i)).toBeNull();
   });
 });
 
 describe("/orgs — fetch includes credentials + timeout signal", () => {
   it("/cp/orgs fetch is called with credentials:include and an AbortSignal", async () => {
-    mockFetchSession.mockResolvedValue({ userId: "u-1" });
+    vi.useRealTimers();
+    mockFetchSession.mockResolvedValueOnce({ userId: "u-1" });
     mockFetch.mockResolvedValueOnce(okJson({ orgs: [] }));
     render(<OrgsPage />);
-    await vi.advanceTimersByTimeAsync(50);
-    const callArgs = mockFetch.mock.calls.find((c) =>
-      String(c[0]).includes("/cp/orgs")
-    );
-    expect(callArgs).toBeDefined();
-    expect(callArgs![1]).toMatchObject({ credentials: "include" });
-    expect(callArgs![1].signal).toBeInstanceOf(AbortSignal);
+    await waitFor(() => {
+      const callArgs = mockFetch.mock.calls.find((c) =>
+        String(c[0]).includes("/cp/orgs")
+      );
+      expect(callArgs).toBeDefined();
+      expect(callArgs![1]).toMatchObject({ credentials: "include" });
+      expect(callArgs![1].signal).toBeInstanceOf(AbortSignal);
+    });
   });
 });
 

canvas/src/components/__tests__/ContextMenu.keyboard.test.tsx

@@ -225,6 +225,7 @@ describe("ContextMenu — keyboard accessibility", () => {
     expect(mockStore.setPendingDelete).toHaveBeenCalledWith({
       id: "ws-1",
       name: "Alpha Workspace",
+      hasChildren: false,
     });
     expect(closeContextMenu).toHaveBeenCalled();
   });

workspace-server/internal/handlers/container_files.go

@@ -73,9 +73,19 @@ func (h *TemplatesHandler) copyFilesToContainer(ctx context.Context, containerNa
 
 	createdDirs := map[string]bool{}
 	for name, content := range files {
+		// CWE-22: reject absolute paths and path-traversal sequences
+		// before using the name in the tar header.
+		clean := filepath.Clean(name)
+		if filepath.IsAbs(clean) || strings.Contains(clean, "..") {
+			return fmt.Errorf("path traversal blocked: %s", name)
+		}
+		// Use the safe, cleaned name joined with destPath so the tar
+		// header Name is always a relative path inside destPath.
+		safeName := filepath.Join(destPath, clean)
+
 		// Create parent directories in tar (deduplicated)
-		dir := filepath.Dir(name)
-		if dir != "." && !createdDirs[dir] {
+		dir := filepath.Dir(safeName)
+		if dir != destPath && !createdDirs[dir] {
 			tw.WriteHeader(&tar.Header{
 				Typeflag: tar.TypeDir,
 				Name:     dir + "/",
@@ -86,7 +96,7 @@ func (h *TemplatesHandler) copyFilesToContainer(ctx context.Context, containerNa
 
 		data := []byte(content)
 		header := &tar.Header{
-			Name: name,
+			Name: safeName,
 			Mode: 0644,
 			Size: int64(len(data)),
 		}
@@ -143,6 +153,12 @@ func (h *TemplatesHandler) deleteViaEphemeral(ctx context.Context, volumeName, f
 		return fmt.Errorf("docker not available")
 	}
 
+	// CWE-22: validate filePath before constructing the rm command so
+	// a path-traversal sequence cannot escape /configs.
+	if err := validateRelPath(filePath); err != nil {
+		return err
+	}
+
 	resp, err := h.docker.ContainerCreate(ctx, &container.Config{
 		Image: "alpine:latest",
 		Cmd:   []string{"rm", "-rf", "/configs/" + filePath},

workspace-server/internal/handlers/plugins_install_pipeline.go

@@ -144,6 +144,8 @@ func (h *PluginsHandler) resolveAndStage(ctx context.Context, req installRequest
 	}
 	resolver, err := h.sources.Resolve(source)
 	if err != nil {
+		// F1086 / #1206: include schemes so the caller can self-diagnose
+		// the fix, but never the raw error message.
 		return nil, newHTTPErr(http.StatusBadRequest, gin.H{
 			"error":             "failed to resolve plugin source",
 			"available_schemes": h.sources.Schemes(),

workspace-server/internal/handlers/workspace_provision.go

@@ -103,12 +103,16 @@ func (h *WorkspaceHandler) provisionWorkspaceOpts(workspaceID, templatePath stri
 	// never recovers. Failing fast here surfaces the cause to the operator.
 	if err := h.envMutators.Run(ctx, workspaceID, envVars); err != nil {
 		log.Printf("Provisioner: env mutator chain failed for %s: %v", workspaceID, err)
+		// F1086 / #1206: broadcast and db last_sample_error use generic messages —
+		// env mutator errors (missing tokens, vault paths, etc.) can include
+		// internal credential URIs and file paths that must not reach the caller.
 		h.broadcaster.RecordAndBroadcast(ctx, "WORKSPACE_PROVISION_FAILED", workspaceID, map[string]interface{}{
 			"error": "plugin env mutator chain failed",
 		})
 		if _, dbErr := db.DB.ExecContext(ctx,
 			`UPDATE workspaces SET status = 'failed', last_sample_error = $2, updated_at = now() WHERE id = $1`,
 			workspaceID, "plugin env mutator chain failed"); dbErr != nil {
+
 			log.Printf("Provisioner: failed to mark workspace %s as failed after mutator error: %v", workspaceID, dbErr)
 		}
 		return
@@ -175,11 +179,11 @@ func (h *WorkspaceHandler) provisionWorkspaceOpts(workspaceID, templatePath stri
 
 	url, err := h.provisioner.Start(ctx, cfg)
 	if err != nil {
-		// Persist the error text to last_sample_error so the canvas and
-		// GET /workspaces/:id expose something actionable — previously the
-		// provision failure was only logged + broadcast, leaving the DB
-		// row with an empty last_sample_error. Issue #117.
-		log.Printf("Provisioner: failed to start workspace %s: %v", workspaceID, err)
+		// F1086 / #1206: persist a generic message so the canvas and
+		// GET /workspaces/:id expose something actionable without leaking
+		// docker/error internals (image pull messages, volume paths, etc.).
+		errMsg := "workspace start failed"
+		log.Printf("Provisioner: %s for %s: %v", errMsg, workspaceID, err)
 		if _, dbErr := db.DB.ExecContext(ctx,
 			`UPDATE workspaces SET status = 'failed', last_sample_error = $2, updated_at = now() WHERE id = $1`,
 			workspaceID, "workspace start failed"); dbErr != nil {
@@ -581,6 +585,8 @@ func (h *WorkspaceHandler) provisionWorkspaceCP(workspaceID, templatePath string
 	applyAgentGitIdentity(envVars, payload.Name)
 	if err := h.envMutators.Run(ctx, workspaceID, envVars); err != nil {
 		log.Printf("CPProvisioner: env mutator failed for %s: %v", workspaceID, err)
+		// F1086 / #1206: env mutator errors (missing tokens, vault paths) must not
+		// leak into last_sample_error — use generic message.
 		db.DB.ExecContext(ctx, `UPDATE workspaces SET status = 'failed', last_sample_error = $2, updated_at = now() WHERE id = $1`,
 				workspaceID, "plugin env mutator chain failed")
 		return
@@ -596,7 +602,10 @@ func (h *WorkspaceHandler) provisionWorkspaceCP(workspaceID, templatePath string
 
 	machineID, err := h.cpProv.Start(ctx, cfg)
 	if err != nil {
-		log.Printf("CPProvisioner: failed to start workspace %s: %v", workspaceID, err)
+		// F1086 / #1206: CP errors can include machine type, AMI IDs, VPC
+		// paths — use generic message for broadcast and last_sample_error.
+		errMsg := "workspace start failed"
+		log.Printf("CPProvisioner: %s for %s: %v", errMsg, workspaceID, err)
 		h.broadcaster.RecordAndBroadcast(ctx, "WORKSPACE_PROVISION_FAILED", workspaceID, map[string]interface{}{
 			"error": "provisioning failed",
 		})