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

workspace-server/internal/handlers/templates.go

@@ -410,9 +410,13 @@ func (h *TemplatesHandler) WriteFile(c *gin.Context) {
 func (h *TemplatesHandler) DeleteFile(c *gin.Context) {
 	workspaceID := c.Param("id")
 	filePath := c.Param("path")
-	if strings.HasPrefix(filePath, "/") {
-		filePath = filePath[1:]
+	// Reject absolute paths before stripping the leading slash — this check
+	// must come before the strip so that "/etc/passwd" is not silently accepted.
+	if filepath.IsAbs(filePath) {
+		c.JSON(http.StatusBadRequest, gin.H{"error": "absolute paths not permitted"})
+		return
 	}
+	filePath = strings.TrimPrefix(filePath, "/")
 
 	if err := validateRelPath(filePath); err != nil {
 		c.JSON(http.StatusBadRequest, gin.H{"error": "invalid path"})
@@ -428,8 +432,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 +492,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

@@ -649,7 +649,7 @@ func TestDeleteFile_WorkspaceNotFound(t *testing.T) {
 	c, _ := gin.CreateTestContext(w)
 	c.Params = gin.Params{
 		{Key: "id", Value: "ws-del-nf"},
-		{Key: "path", Value: "/old-file.txt"},
+		{Key: "path", Value: "old-file.txt"},
 	}
 	c.Request = httptest.NewRequest("DELETE", "/workspaces/ws-del-nf/files/old-file.txt", nil)
 
@@ -789,3 +789,107 @@ 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"},
+		{"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)
+	}
+}