fix(handlers): CWE-78 hardening for DeleteFile and SharedContext (#2011)

Replace string concatenation with safe exec-form path construction in
two remaining locations in templates.go:

1. DeleteFile (container-running path):
   - Before: `containerPath := "/configs/" + filePath` → `rm -rf containerPath`
   - After:  `rm -f filepath.Join("/configs", filePath)`
   - Also tightens rm flag from -rf to -f (no recursive delete on a file endpoint)

2. SharedContext (container-running path, per-file cat loop):
   - Before: `[]string{"cat", "/configs/" + relPath}`
   - After:  `[]string{"cat", "/configs", relPath}` (separate args, no shell join)

In both cases validateRelPath is already the primary guard (rejects traversal
inputs before reaching exec). filepath.Join / separate args is defence-in-depth
so that a bypass of validateRelPath cannot produce a dangerous concatenated path
in the exec argument list.

ReadFile was already fixed (PR #1885, merged to main at 12:08Z).

Regression tests added:
- TestCWE78_DeleteFile_TraversalVariants: 7 traversal patterns all → 400
- TestCWE78_SharedContext_SkipsTraversalPaths: traversal paths in
  shared_context config are silently skipped, only safe files returned

Fixes: #2011

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

workspace-server/internal/handlers/templates.go

@@ -428,8 +428,11 @@ func (h *TemplatesHandler) DeleteFile(c *gin.Context) {
 
 	// Delete via docker exec when container is running
 	if containerName := h.findContainer(ctx, workspaceID); containerName != "" {
-		containerPath := "/configs/" + filePath
-		_, err := h.execInContainer(ctx, containerName, []string{"rm", "-rf", containerPath})
+		// CWE-78: use filepath.Join instead of string concat to prevent path
+		// injection into the exec argument. validateRelPath above is the primary
+		// guard; filepath.Join is defence-in-depth. Use -f (not -rf) to avoid
+		// recursive deletion of an entire directory via traversal.
+		_, err := h.execInContainer(ctx, containerName, []string{"rm", "-f", filepath.Join("/configs", filePath)})
 		if err != nil {
 			c.JSON(http.StatusInternalServerError, gin.H{"error": fmt.Sprintf("failed to delete: %v", err)})
 			return
@@ -485,7 +488,11 @@ func (h *TemplatesHandler) SharedContext(c *gin.Context) {
 			if err := validateRelPath(relPath); err != nil {
 				continue
 			}
-			content, err := h.execInContainer(ctx, containerName, []string{"cat", "/configs/" + relPath})
+			// CWE-78: pass path components as separate exec args instead of
+			// concatenating into a single string. validateRelPath above is the
+			// primary guard; separate args is defence-in-depth (no shell
+			// interpolation possible in exec form).
+			content, err := h.execInContainer(ctx, containerName, []string{"cat", "/configs", relPath})
 			if err != nil {
 				continue
 			}

workspace-server/internal/handlers/templates_test.go

@@ -789,3 +789,108 @@ func TestResolveTemplateDir_NotFound(t *testing.T) {
 		t.Errorf("expected empty string, got %q", result)
 	}
 }
+
+// ==================== CWE-78 hardening regression (issue #2011) ====================
+// These tests lock in the defence-in-depth guards for DeleteFile and SharedContext.
+// The primary guard is validateRelPath (fires before any exec/file-read path);
+// the exec-form path construction (filepath.Join / separate args) is defence-in-depth.
+
+// TestCWE78_DeleteFile_TraversalVariants asserts that a range of traversal patterns
+// are all rejected with 400 before any Docker exec or ephemeral container operation.
+// This covers the validateRelPath guard that sits at the entry of DeleteFile.
+func TestCWE78_DeleteFile_TraversalVariants(t *testing.T) {
+	cases := []struct {
+		name string
+		path string
+	}{
+		{"double dotdot", "/../../../etc/passwd"},
+		{"leading dotdot", "/../secret"},
+		{"mid-path traversal", "/valid/../../../etc/shadow"},
+		{"absolute path", "/etc/passwd"},
+		{"null byte", "/foo\x00../../etc/passwd"},
+		{"encoded dotdot raw", "..%2F..%2Fetc%2Fpasswd"},
+		{"triple dotdot", "/../../.."},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			setupTestDB(t)
+			setupTestRedis(t)
+
+			handler := NewTemplatesHandler(t.TempDir(), nil)
+
+			w := httptest.NewRecorder()
+			c, _ := gin.CreateTestContext(w)
+			c.Params = gin.Params{
+				{Key: "id", Value: "ws-cwe78"},
+				{Key: "path", Value: tc.path},
+			}
+			c.Request = httptest.NewRequest("DELETE", "/workspaces/ws-cwe78/files"+tc.path, nil)
+
+			handler.DeleteFile(c)
+
+			if w.Code != http.StatusBadRequest {
+				t.Errorf("path %q: expected 400 (traversal blocked), got %d: %s",
+					tc.path, w.Code, w.Body.String())
+			}
+		})
+	}
+}
+
+// TestCWE78_SharedContext_SkipsTraversalPaths asserts that when a workspace's
+// config.yaml lists traversal paths in shared_context, SharedContext skips them
+// via validateRelPath rather than passing them to exec or os.ReadFile.
+// Uses the filesystem fallback path (no docker client) so no container mock needed.
+func TestCWE78_SharedContext_SkipsTraversalPaths(t *testing.T) {
+	mock := setupTestDB(t)
+	setupTestRedis(t)
+
+	tmpDir := t.TempDir()
+	// Create a template directory that SharedContext will resolve for "Cwe Agent".
+	tmplDir := filepath.Join(tmpDir, "cwe-agent")
+	os.MkdirAll(tmplDir, 0755)
+	// config.yaml with a mix of safe and traversal-attack paths.
+	configYAML := "name: Cwe Agent\nshared_context:\n  - safe-file.md\n  - ../../etc/passwd\n  - ../shadow\n  - another-safe.md\n"
+	os.WriteFile(filepath.Join(tmplDir, "config.yaml"), []byte(configYAML), 0644)
+	// Only write the safe files — traversal paths must not be reachable.
+	os.WriteFile(filepath.Join(tmplDir, "safe-file.md"), []byte("# safe"), 0644)
+	os.WriteFile(filepath.Join(tmplDir, "another-safe.md"), []byte("# also safe"), 0644)
+
+	mock.ExpectQuery("SELECT name FROM workspaces WHERE id =").
+		WithArgs("ws-cwe78-sc").
+		WillReturnRows(sqlmock.NewRows([]string{"name"}).AddRow("Cwe Agent"))
+
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	c.Params = gin.Params{{Key: "id", Value: "ws-cwe78-sc"}}
+	c.Request = httptest.NewRequest("GET", "/workspaces/ws-cwe78-sc/shared-context", nil)
+
+	handler := NewTemplatesHandler(tmpDir, nil) // nil docker → filesystem fallback
+	handler.SharedContext(c)
+
+	if w.Code != http.StatusOK {
+		t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String())
+	}
+
+	var files []struct {
+		Path    string `json:"path"`
+		Content string `json:"content"`
+	}
+	if err := json.Unmarshal(w.Body.Bytes(), &files); err != nil {
+		t.Fatalf("failed to decode response: %v", err)
+	}
+
+	// Only the two safe files must appear; traversal paths must be absent.
+	if len(files) != 2 {
+		t.Errorf("expected 2 safe files, got %d: %v", len(files), files)
+	}
+	for _, f := range files {
+		if strings.Contains(f.Path, "..") || strings.Contains(f.Path, "etc") || strings.Contains(f.Path, "shadow") {
+			t.Errorf("traversal path %q must not appear in shared-context response", f.Path)
+		}
+	}
+
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Errorf("unmet sqlmock expectations: %v", err)
+	}
+}