Merge staging into rfc-2991-pr-1 to clear BEHIND state

This commit is contained in:
Hongming Wang 2026-05-05 20:50:55 -07:00
commit 1b29b24e83
16 changed files with 1805 additions and 277 deletions

View File

@ -193,7 +193,47 @@ jobs:
# sweeper is best-effort. Next hourly tick re-attempts. We
# only fail loud at the safety-cap gate above.
- name: Sweep orphan tunnels
# Stale-org cleanup deletes the org (which cascades to tunnel
# delete inside the CP). But when that cascade fails partway —
# CP transient 5xx after the org row is deleted but before the
# CF tunnel delete completes — the tunnel persists with no
# matching org row. The reconciler in internal/sweep flags this
# as `cf_tunnel kind=orphan`, but nothing automatically reaps it.
#
# `/cp/admin/orphan-tunnels/cleanup` is the operator-triggered
# reaper. Calling it here at the end of every sweep tick
# converges the staging CF account to clean even when CP
# cascades half-fail.
#
# PR #492 made the underlying DeleteTunnel actually check
# status — pre-fix it silent-succeeded on CF code 1022
# ("active connections"), so this step would have been a no-op
# against stuck connectors. Post-fix the cleanup invokes
# CleanupTunnelConnections + retry, which actually clears the
# 1022 case. (#2987)
#
# Best-effort. Failure here doesn't fail the workflow — next
# tick re-attempts. Errors flow to step output for ops review.
if: env.DRY_RUN != 'true'
run: |
set +e
curl -sS -o /tmp/cleanup_resp -w "%{http_code}" \
--max-time 60 \
-X POST "$MOLECULE_CP_URL/cp/admin/orphan-tunnels/cleanup" \
-H "Authorization: Bearer $ADMIN_TOKEN" >/tmp/cleanup_code
set -e
http_code=$(cat /tmp/cleanup_code 2>/dev/null || echo "000")
body=$(cat /tmp/cleanup_resp 2>/dev/null | head -c 500)
if [ "$http_code" = "200" ]; then
count=$(echo "$body" | python3 -c "import sys,json; d=json.loads(sys.stdin.read() or '{}'); print(d.get('deleted_count', 0))" 2>/dev/null || echo "0")
failed_n=$(echo "$body" | python3 -c "import sys,json; d=json.loads(sys.stdin.read() or '{}'); print(len(d.get('failed') or {}))" 2>/dev/null || echo "0")
echo "Orphan-tunnel sweep: deleted=$count failed=$failed_n"
else
echo "::warning::orphan-tunnels cleanup returned HTTP $http_code — body: $body"
fi
- name: Dry-run summary
if: env.DRY_RUN == 'true'
run: |
echo "DRY RUN — would have deleted ${{ steps.identify.outputs.count }} org(s). Re-run with dry_run=false to actually delete."
echo "DRY RUN — would have deleted ${{ steps.identify.outputs.count }} org(s) AND triggered orphan-tunnels cleanup. Re-run with dry_run=false to actually delete."

View File

@ -287,7 +287,7 @@ export function SidePanel() {
{panelTab === "config" && <ConfigTab key={selectedNodeId} workspaceId={selectedNodeId} />}
{panelTab === "schedule" && <ScheduleTab key={selectedNodeId} workspaceId={selectedNodeId} />}
{panelTab === "channels" && <ChannelsTab key={selectedNodeId} workspaceId={selectedNodeId} />}
{panelTab === "files" && <FilesTab key={selectedNodeId} workspaceId={selectedNodeId} />}
{panelTab === "files" && <FilesTab key={selectedNodeId} workspaceId={selectedNodeId} data={node.data} />}
{panelTab === "memory" && <MemoryInspectorPanel key={selectedNodeId} workspaceId={selectedNodeId} />}
{panelTab === "traces" && <TracesTab key={selectedNodeId} workspaceId={selectedNodeId} />}
{panelTab === "events" && <EventsTab key={selectedNodeId} workspaceId={selectedNodeId} />}

View File

@ -2,9 +2,11 @@
import { useState, useEffect, useRef, useMemo } from "react";
import { showToast } from "../Toaster";
import type { WorkspaceNodeData } from "@/store/canvas";
import { FilesToolbar } from "./FilesTab/FilesToolbar";
import { FileTree } from "./FilesTab/FileTree";
import { FileEditor } from "./FilesTab/FileEditor";
import { NotAvailablePanel } from "./FilesTab/NotAvailablePanel";
import { useFilesApi } from "./FilesTab/useFilesApi";
import { buildTree } from "./FilesTab/tree";
@ -14,9 +16,40 @@ export type { TreeNode } from "./FilesTab/tree";
interface Props {
workspaceId: string;
/** Workspace metadata from the canvas store. Optional for back-compat
* with any caller that still mounts <FilesTab workspaceId=.../> without
* threading data through (legacy tests). When present, runtime gates
* the early-return below. Mirrors TerminalTab's prop shape (#2830). */
data?: WorkspaceNodeData;
}
export function FilesTab({ workspaceId }: Props) {
/** Runtimes whose filesystem the platform doesn't own. The canvas can't
* list/read/write files on these the agent runs on the user's own
* hardware (mac laptop, mac mini, hermes-on-home-server) and reaches
* the platform via the heartbeat-based polling Phase 30 layer.
*
* Keep narrow only add a runtime here when its provisioner genuinely
* has no platform-owned filesystem. Otherwise the user loses access to
* a real surface (e.g. claude-code SaaS workspaces have files served
* by ListFiles via EIC; they belong on the rendering path, not here). */
const RUNTIMES_WITHOUT_FILES = new Set(["external"]);
export function FilesTab({ workspaceId, data }: Props) {
// Early-return for runtimes whose filesystem is not platform-owned.
// Skips the whole useFilesApi hook + tree render below — without this,
// mounting the tab for an external workspace would issue a GET that
// the platform can technically answer (it reads its own DB row, not
// the user's machine), but every result row is fictional. Showing
// "0 files / No config files yet" reads as a bug. The placeholder
// makes the absence intentional and points the user at the right
// surface (Chat).
if (data && RUNTIMES_WITHOUT_FILES.has(data.runtime)) {
return <NotAvailablePanel runtime={data.runtime} />;
}
return <PlatformOwnedFilesTab workspaceId={workspaceId} />;
}
function PlatformOwnedFilesTab({ workspaceId }: { workspaceId: string }) {
const [root, setRoot] = useState("/configs");
const [selectedFile, setSelectedFile] = useState<string | null>(null);
const [fileContent, setFileContent] = useState("");
@ -45,6 +78,7 @@ export function FilesTab({ workspaceId }: Props) {
readFile,
writeFile,
deleteFile,
downloadFileByPath,
downloadAllFiles,
uploadFiles,
deleteAllFiles,
@ -216,7 +250,15 @@ export function FilesTab({ workspaceId }: Props) {
nodes={tree}
selectedPath={selectedFile}
onSelect={openFile}
// Delete is currently gated to /configs to match the
// toolbar's New / Upload / Clear affordances. Context
// menu and inline ✕ both honour the gate. PR-A made the
// backend EIC delete work on all roots — keeping the
// canvas gate conservative until we want to expose
// /home /workspace deletion intentionally.
onDelete={root === "/configs" ? setConfirmDelete : () => {}}
onDownload={downloadFileByPath}
canDelete={root === "/configs"}
expandedDirs={expandedDirs}
onToggleDir={toggleDir}
loadingDir={loadingDir}

View File

@ -1,41 +1,108 @@
"use client";
import { useState } from "react";
import { type TreeNode, getIcon } from "./tree";
import { FileTreeContextMenu, type MenuItem } from "./FileTreeContextMenu";
interface TreeCallbacks {
selectedPath: string | null;
onSelect: (path: string) => void;
onDelete: (path: string) => void;
/** PR-C: right-click → Download. Files only — directories ignore. */
onDownload: (path: string) => void;
/** Whether the active root permits delete. Wire into the Delete
* context-menu item's `disabled` flag so the user gets the same
* affordance as the toolbar (which gates Clear/New on /configs). */
canDelete: boolean;
expandedDirs: Set<string>;
onToggleDir: (path: string) => void;
loadingDir: string | null;
}
/**
* FileTree renders the workspace tree + owns the right-click
* context-menu state. Lifting the menu state to the tree (vs each
* row) means only one menu is open at a time opening a new row's
* menu auto-closes the prior one. Same UX as VSCode / Theia.
*/
export function FileTree({
nodes,
selectedPath,
onSelect,
onDelete,
onDownload,
canDelete,
expandedDirs,
onToggleDir,
loadingDir,
depth = 0,
}: TreeCallbacks & { nodes: TreeNode[]; depth?: number }) {
const [menu, setMenu] = useState<{
x: number;
y: number;
items: MenuItem[];
} | null>(null);
const openContextMenu = (e: React.MouseEvent, node: TreeNode) => {
e.preventDefault();
// Items composed per-row so the available actions reflect the
// node type (files get Download; directories don't have a
// useful per-tree download — the Export toolbar covers bulk).
const items: MenuItem[] = [];
if (!node.isDir) {
items.push({
id: "open",
label: "Open",
icon: "⤴",
onClick: () => onSelect(node.path),
});
items.push({
id: "download",
label: "Download",
icon: "↓",
onClick: () => onDownload(node.path),
});
}
items.push({
id: "delete",
label: "Delete",
icon: "✕",
destructive: true,
disabled: !canDelete,
onClick: () => onDelete(node.path),
});
setMenu({ x: e.clientX, y: e.clientY, items });
};
// Single state lifted to the top-level tree; nested <FileTree>s
// (rendered for expanded directories below) do NOT instantiate
// their own menus — they call the SAME openContextMenu via prop
// drilling. This keeps "only one menu open" the structural
// invariant rather than a render-order coincidence.
const childCallbacks: TreeCallbacks = {
selectedPath, onSelect, onDelete, onDownload, canDelete,
expandedDirs, onToggleDir, loadingDir,
};
return (
<div>
{nodes.map((node) => (
<TreeItem
key={`${node.path}:${node.isDir ? "dir" : "file"}`}
node={node}
selectedPath={selectedPath}
onSelect={onSelect}
onDelete={onDelete}
expandedDirs={expandedDirs}
onToggleDir={onToggleDir}
loadingDir={loadingDir}
openContextMenu={openContextMenu}
depth={depth}
{...childCallbacks}
/>
))}
{menu && (
<FileTreeContextMenu
x={menu.x}
y={menu.y}
items={menu.items}
onClose={() => setMenu(null)}
/>
)}
</div>
);
}
@ -45,11 +112,18 @@ function TreeItem({
selectedPath,
onSelect,
onDelete,
onDownload,
canDelete,
expandedDirs,
onToggleDir,
loadingDir,
depth,
}: TreeCallbacks & { node: TreeNode; depth: number }) {
openContextMenu,
}: TreeCallbacks & {
node: TreeNode;
depth: number;
openContextMenu: (e: React.MouseEvent, node: TreeNode) => void;
}) {
const isSelected = selectedPath === node.path;
const expanded = expandedDirs.has(node.path);
const isLoading = loadingDir === node.path;
@ -61,6 +135,7 @@ function TreeItem({
className="group w-full flex items-center gap-1 px-2 py-0.5 text-left hover:bg-surface-card/40 transition-colors cursor-pointer"
style={{ paddingLeft: `${depth * 12 + 8}px` }}
onClick={() => onToggleDir(node.path)}
onContextMenu={(e) => openContextMenu(e, node)}
>
<span className="text-[9px] text-ink-soft w-3">{isLoading ? "…" : expanded ? "▼" : "▶"}</span>
<span className="text-[10px]">📁</span>
@ -82,6 +157,8 @@ function TreeItem({
selectedPath={selectedPath}
onSelect={onSelect}
onDelete={onDelete}
onDownload={onDownload}
canDelete={canDelete}
expandedDirs={expandedDirs}
onToggleDir={onToggleDir}
loadingDir={loadingDir}
@ -99,6 +176,7 @@ function TreeItem({
}`}
style={{ paddingLeft: `${depth * 12 + 20}px` }}
onClick={() => onSelect(node.path)}
onContextMenu={(e) => openContextMenu(e, node)}
>
<span className="text-[9px]">{getIcon(node.name, false)}</span>
<span className="text-[10px] flex-1 truncate font-mono">{node.name}</span>

View File

@ -0,0 +1,141 @@
"use client";
import { useEffect, useRef } from "react";
/**
* FileTreeContextMenu VSCode-style right-click menu for a single
* file-tree row. Pops at the cursor's viewport coords; dismisses on
* outside-click, Esc, blur, or scroll.
*
* Why a custom component (no library): the menu is one of several
* "small popovers" in canvas; pulling in a dnd / popover lib for one
* surface adds 10x the bytes of this implementation. The patterns
* (outside-click + Esc + portal-free fixed position) match the
* ContextMenu used in canvas/Toolbar so the keyboard-nav muscle
* memory is uniform.
*
* Items are rendered from a `MenuItem[]` so callers can add/remove
* actions without touching this component (e.g. PR-D will add an
* "Upload to this folder" item for directory rows).
*
* Accessibility:
* - role="menu" + role="menuitem" so screen readers announce the
* surface as a menu, not a generic div.
* - First item gets autofocus so keyboard users can //Enter without
* reaching for the mouse.
* - Esc + outside-click + Tab dismisses; behaves like every other
* menu the user has touched on the canvas.
*/
export interface MenuItem {
/** Stable identifier for testing + analytics. */
id: string;
label: string;
/** Optional left icon glyph; not load-bearing. */
icon?: string;
/** Destructive (rendered in red) — for Delete-class actions. */
destructive?: boolean;
/** Item-specific click handler. The menu auto-closes after onClick
* fires so handlers don't have to call onClose themselves. */
onClick: () => void;
/** Disabled items render but don't fire onClick (useful for
* Delete-on-non-/configs case where the caller wants to surface
* the item but explain it's gated). Currently unused placeholder
* for future options. */
disabled?: boolean;
}
interface Props {
/** Viewport-coordinate position of the cursor that opened the menu. */
x: number;
y: number;
items: MenuItem[];
onClose: () => void;
}
export function FileTreeContextMenu({ x, y, items, onClose }: Props) {
const ref = useRef<HTMLDivElement>(null);
// First item gets initial focus for keyboard ↓/↑/Enter nav.
const firstItemRef = useRef<HTMLButtonElement>(null);
useEffect(() => {
firstItemRef.current?.focus();
}, []);
// Outside-click + Esc dismiss. Per memory
// (feedback_abort_controller_for_rerendered_listeners), use an
// AbortController so re-mounts (caller toggles the menu) don't leak
// listeners.
useEffect(() => {
const ctrl = new AbortController();
const onPointerDown = (e: MouseEvent) => {
if (ref.current && !ref.current.contains(e.target as Node)) onClose();
};
const onKeyDown = (e: KeyboardEvent) => {
if (e.key === "Escape") {
e.preventDefault();
onClose();
} else if (e.key === "ArrowDown" || e.key === "ArrowUp") {
// Roving focus across .menuitem buttons. Doing this with
// tabindex management because Tab / Shift+Tab leave the menu
// (which is the right thing — the user is escaping the menu).
e.preventDefault();
const buttons = ref.current?.querySelectorAll<HTMLButtonElement>(
"[role='menuitem']:not([disabled])",
);
if (!buttons || buttons.length === 0) return;
const arr = Array.from(buttons);
const cur = arr.indexOf(document.activeElement as HTMLButtonElement);
const next =
e.key === "ArrowDown"
? (cur + 1) % arr.length
: (cur - 1 + arr.length) % arr.length;
arr[next].focus();
}
};
// `mousedown` (not `click`) so the menu dismisses BEFORE the
// tree-row's click handler would fire — otherwise clicking
// outside also selects a different row, which is not what the
// user expected when "outside-click closes the menu".
document.addEventListener("mousedown", onPointerDown, { signal: ctrl.signal });
document.addEventListener("keydown", onKeyDown, { signal: ctrl.signal });
// Scroll inside any ancestor also dismisses — the fixed-position
// menu would otherwise stay anchored to viewport coords while the
// row it points at scrolled away. Use capture so we catch scroll
// on inner panels (FileTree's overflow-y-auto wrapper).
document.addEventListener("scroll", onClose, { signal: ctrl.signal, capture: true });
return () => ctrl.abort();
}, [onClose]);
return (
<div
ref={ref}
role="menu"
aria-label="File actions"
className="fixed z-[1000] min-w-[140px] py-1 bg-surface-elevated border border-line/60 rounded-md shadow-xl shadow-black/30 text-[11px]"
style={{ left: x, top: y }}
>
{items.map((item, i) => (
<button
key={item.id}
ref={i === 0 ? firstItemRef : undefined}
type="button"
role="menuitem"
disabled={item.disabled}
onClick={() => {
if (item.disabled) return;
item.onClick();
onClose();
}}
className={
item.destructive
? "w-full text-left px-3 py-1 text-bad hover:bg-red-900/30 focus:bg-red-900/30 focus:outline-none disabled:opacity-40 disabled:pointer-events-none transition-colors"
: "w-full text-left px-3 py-1 text-ink-mid hover:bg-surface-card hover:text-ink focus:bg-surface-card focus:text-ink focus:outline-none disabled:opacity-40 disabled:pointer-events-none transition-colors"
}
>
{item.icon && <span className="inline-block w-4 mr-1.5 text-ink-soft">{item.icon}</span>}
{item.label}
</button>
))}
</div>
);
}

View File

@ -0,0 +1,58 @@
"use client";
/**
* NotAvailablePanel full-tab placeholder for runtimes whose filesystem
* the platform doesn't own (today: runtime === "external").
*
* Pre-fix the FilesTab tried to GET /workspaces/<id>/files for these
* workspaces. The platform answered with [] (no rows in workspace_files
* for an external workspace by definition), but the canvas rendered
* "0 files / No config files yet" which reads identically to the SaaS
* empty-listing bug fixed in PR-A. Showing an explicit placeholder
* makes the absence intentional and routes the user toward the
* supported surface (Chat) for these workspaces.
*
* Mirrors the same affordance TerminalTab adopted for runtimes without
* a TTY in PR #2830 uniform "feature-not-applicable" UX across tabs.
*/
export function NotAvailablePanel({ runtime }: { runtime: string }) {
return (
<div className="flex flex-col items-center justify-center h-full p-8 text-center bg-surface-sunken/30">
{/* Folder-with-slash icon. Custom inline SVG so we don't depend
on an icon set being present at canvas build-time (matches
TerminalTab's NotAvailablePanel pattern). */}
<svg
width="72"
height="72"
viewBox="0 0 72 72"
fill="none"
aria-hidden="true"
className="text-ink-soft mb-4"
>
{/* Folder body */}
<path
d="M10 22 L10 56 a4 4 0 0 0 4 4 L58 60 a4 4 0 0 0 4 -4 L62 26 a4 4 0 0 0 -4 -4 L34 22 L28 16 L14 16 a4 4 0 0 0 -4 4 Z"
stroke="currentColor"
strokeWidth="2.5"
strokeLinejoin="round"
fill="none"
opacity="0.6"
/>
{/* Diagonal cancel slash */}
<path
d="M14 14 L58 58"
stroke="currentColor"
strokeWidth="3"
strokeLinecap="round"
/>
</svg>
<h3 className="text-sm font-medium text-ink mb-1.5">Files not available</h3>
<p className="text-[11px] text-ink-soft max-w-xs leading-relaxed">
This workspace runs the{" "}
<span className="font-mono text-ink-mid">{runtime}</span> runtime,
whose filesystem isn't owned by the platform. Use the Chat tab to
interact with the agent directly.
</p>
</div>
);
}

View File

@ -0,0 +1,135 @@
// @vitest-environment jsdom
//
// Pins the right-click context menu added in PR-C of issue #2999.
// VSCode-style affordance: Open / Download / Delete on file rows,
// Delete on directory rows. Delete is gated by `canDelete` (parent
// only enables on /configs root, matching the toolbar's gate).
//
// Pinned branches:
// 1. Right-click on a file row opens the menu at the click coords
// with Open + Download + Delete items.
// 2. Right-click on a directory row opens the menu with Delete
// only (no Open/Download — directories don't have one-click
// semantics in this surface).
// 3. Clicking Download fires the onDownload callback with the
// row's path.
// 4. Clicking Delete fires onDelete with the row's path (when
// canDelete=true).
// 5. Delete is disabled in the rendered menu when canDelete=false
// and clicking it does NOT fire onDelete (gate is real).
// 6. Esc dismisses the menu.
// 7. Click outside the menu dismisses it.
import { describe, it, expect, vi, afterEach } from "vitest";
import { render, screen, cleanup, fireEvent, act } from "@testing-library/react";
import React from "react";
import { FileTree } from "../FileTree";
import type { TreeNode } from "../tree";
afterEach(cleanup);
const file: TreeNode = { name: "config.yaml", path: "config.yaml", isDir: false, children: [] };
const dir: TreeNode = {
name: "skills",
path: "skills",
isDir: true,
children: [],
};
function renderTree(props: Partial<React.ComponentProps<typeof FileTree>> = {}) {
const defaults = {
nodes: [file, dir],
selectedPath: null,
onSelect: vi.fn(),
onDelete: vi.fn(),
onDownload: vi.fn(),
canDelete: true,
expandedDirs: new Set<string>(),
onToggleDir: vi.fn(),
loadingDir: null,
};
const merged = { ...defaults, ...props };
return { ...render(<FileTree {...merged} />), props: merged };
}
describe("FileTree right-click context menu", () => {
it("right-click on a file row opens menu with Open/Download/Delete", () => {
renderTree();
fireEvent.contextMenu(screen.getByText("config.yaml"), {
clientX: 50,
clientY: 100,
});
expect(screen.getByRole("menu")).not.toBeNull();
expect(screen.getByRole("menuitem", { name: /Open/i })).not.toBeNull();
expect(screen.getByRole("menuitem", { name: /Download/i })).not.toBeNull();
expect(screen.getByRole("menuitem", { name: /Delete/i })).not.toBeNull();
});
it("right-click on a directory row opens menu with Delete only (no Open/Download)", () => {
renderTree();
fireEvent.contextMenu(screen.getByText("skills"), { clientX: 60, clientY: 120 });
expect(screen.getByRole("menu")).not.toBeNull();
expect(screen.queryByRole("menuitem", { name: /Open/i })).toBeNull();
expect(screen.queryByRole("menuitem", { name: /Download/i })).toBeNull();
expect(screen.getByRole("menuitem", { name: /Delete/i })).not.toBeNull();
});
it("clicking Download fires onDownload with the row's path", () => {
const { props } = renderTree();
fireEvent.contextMenu(screen.getByText("config.yaml"), { clientX: 0, clientY: 0 });
fireEvent.click(screen.getByRole("menuitem", { name: /Download/i }));
expect(props.onDownload).toHaveBeenCalledWith("config.yaml");
// Menu auto-closes after click.
expect(screen.queryByRole("menu")).toBeNull();
});
it("clicking Delete fires onDelete with the row's path when canDelete=true", () => {
const { props } = renderTree({ canDelete: true });
fireEvent.contextMenu(screen.getByText("config.yaml"), { clientX: 0, clientY: 0 });
fireEvent.click(screen.getByRole("menuitem", { name: /Delete/i }));
expect(props.onDelete).toHaveBeenCalledWith("config.yaml");
});
it("Delete is disabled when canDelete=false; clicking does not fire onDelete", () => {
const { props } = renderTree({ canDelete: false });
fireEvent.contextMenu(screen.getByText("config.yaml"), { clientX: 0, clientY: 0 });
const del = screen.getByRole("menuitem", { name: /Delete/i }) as HTMLButtonElement;
expect(del.disabled).toBe(true);
fireEvent.click(del);
expect(props.onDelete).not.toHaveBeenCalled();
// Menu stays open on disabled click — same as VSCode (the user
// can read the disabled-state hint without losing the menu).
expect(screen.getByRole("menu")).not.toBeNull();
});
it("Esc dismisses the menu", () => {
renderTree();
fireEvent.contextMenu(screen.getByText("config.yaml"), { clientX: 0, clientY: 0 });
expect(screen.getByRole("menu")).not.toBeNull();
act(() => {
fireEvent.keyDown(document, { key: "Escape" });
});
expect(screen.queryByRole("menu")).toBeNull();
});
it("click outside the menu dismisses it", () => {
renderTree();
fireEvent.contextMenu(screen.getByText("config.yaml"), { clientX: 0, clientY: 0 });
expect(screen.getByRole("menu")).not.toBeNull();
// mousedown on document.body — outside the menu.
act(() => {
fireEvent.mouseDown(document.body);
});
expect(screen.queryByRole("menu")).toBeNull();
});
it("opening a second context menu replaces the first (only one open at a time)", () => {
renderTree();
fireEvent.contextMenu(screen.getByText("config.yaml"), { clientX: 10, clientY: 10 });
fireEvent.contextMenu(screen.getByText("skills"), { clientX: 20, clientY: 20 });
// Only one menu in the DOM. The second open replaced the first
// because the menu state is lifted to the FileTree, not per-row.
const menus = screen.getAllByRole("menu");
expect(menus.length).toBe(1);
});
});

View File

@ -90,6 +90,43 @@ export function useFilesApi(workspaceId: string, root: string) {
[workspaceId]
);
/**
* Fetch a file's content from the server and trigger a browser
* download. Used by the right-click "Download" context-menu item
* (PR-C of issue #2999) distinct from `handleDownloadFile` in
* FilesTab which downloads the CURRENTLY-OPEN-IN-EDITOR file from
* the in-memory `editContent` buffer (so unsaved edits round-trip
* to disk). This helper downloads the on-server content, suitable
* for arbitrary tree rows the user hasn't opened.
*/
const downloadFileByPath = useCallback(
async (path: string) => {
try {
const res = await api.get<{ content: string }>(
`/workspaces/${workspaceId}/files/${path}?root=${encodeURIComponent(root)}`,
);
// text/plain is correct for the canvas's text-only file
// surface (config.yaml, prompts, skill markdown). Binary
// files would need an Accept-arraybuffer path; the API
// returns string today so this matches the wire shape.
const blob = new Blob([res.content], { type: "text/plain" });
const url = URL.createObjectURL(blob);
const a = document.createElement("a");
a.href = url;
a.download = path.split("/").pop() || "file";
a.click();
URL.revokeObjectURL(url);
showToast(`Downloaded ${a.download}`, "success");
} catch (e) {
showToast(
`Download failed: ${e instanceof Error ? e.message : "unknown error"}`,
"error",
);
}
},
[workspaceId, root],
);
const downloadAllFiles = useCallback(async () => {
const fileEntries = files.filter((f) => !f.dir);
const results = await Promise.allSettled(
@ -165,6 +202,7 @@ export function useFilesApi(workspaceId: string, root: string) {
readFile,
writeFile,
deleteFile,
downloadFileByPath,
downloadAllFiles,
uploadFiles,
deleteAllFiles,

View File

@ -0,0 +1,119 @@
// @vitest-environment jsdom
//
// Pins the "Files not available" early-return for runtimes whose
// filesystem the platform doesn't own (today: runtime === "external").
//
// Pre-fix: FilesTab issued a GET /workspaces/<id>/files for every
// workspace. The platform's response for an external workspace is
// always [] (no rows in workspace_files), but the canvas rendered
// "0 files / No config files yet" — visually identical to the SaaS
// empty-listing bug fixed in PR-A. The placeholder makes the absence
// intentional.
//
// Pinned branches:
// 1. external runtime → "Files not available" banner renders,
// runtime name surfaces in the body so user knows WHY.
// 2. external runtime → useFilesApi is NOT invoked. Verified by
// asserting the mocked api.get was never called.
// 3. claude-code (or any other runtime) → no banner, normal mount
// proceeds (`/configs` toolbar visible). Pre-fix regression cover.
// 4. data prop omitted (legacy callers) → no early-return, falls
// through to normal mount.
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
import { render, screen, cleanup, waitFor } from "@testing-library/react";
import React from "react";
afterEach(cleanup);
// Mock the api module so the normal-mount branches don't try to
// fetch against a real backend — and so we can assert the
// external-runtime branch never fires a request.
const apiCalls: string[] = [];
vi.mock("@/lib/api", () => ({
api: {
get: vi.fn((path: string) => {
apiCalls.push(path);
return Promise.resolve([]);
}),
put: vi.fn(() => Promise.resolve()),
del: vi.fn(() => Promise.resolve()),
},
}));
// useCanvasStore is referenced by useFilesApi for the needsRestart
// flag. The Toaster import inside FilesTab also pulls the store
// indirectly. Stub minimally to satisfy the import chain.
vi.mock("@/store/canvas", async () => {
const actual = await vi.importActual<typeof import("@/store/canvas")>(
"@/store/canvas",
);
return {
...actual,
useCanvasStore: {
getState: () => ({
updateNodeData: vi.fn(),
}),
},
};
});
vi.mock("../Toaster", () => ({
showToast: vi.fn(),
}));
beforeEach(() => {
apiCalls.length = 0;
});
import { FilesTab } from "../FilesTab";
const externalData = { runtime: "external", status: "online" } as unknown as Parameters<
typeof FilesTab
>[0]["data"];
const claudeData = { runtime: "claude-code", status: "online" } as unknown as Parameters<
typeof FilesTab
>[0]["data"];
describe("FilesTab not-available early-return for runtimes without platform-owned filesystem", () => {
it("external runtime renders the not-available banner with runtime name", () => {
render(<FilesTab workspaceId="ws-ext" data={externalData} />);
expect(screen.getByText(/Files not available/i)).not.toBeNull();
// Runtime name must surface so the user understands WHY — without
// it the placeholder reads as a generic error.
expect(screen.getByText(/external/)).not.toBeNull();
// Chat tab is the recommended alternative — flagged in copy so the
// user knows where to go next instead of bouncing tabs.
expect(screen.getByText(/Chat tab/i)).not.toBeNull();
});
it("external runtime does NOT issue any /files API call", async () => {
render(<FilesTab workspaceId="ws-ext" data={externalData} />);
// Tolerate one microtask boundary in case useEffect schedules.
await new Promise((r) => setTimeout(r, 0));
const filesCalls = apiCalls.filter((p) => p.includes("/files"));
expect(filesCalls).toEqual([]);
});
it("claude-code runtime does NOT render the banner (normal mount)", async () => {
render(<FilesTab workspaceId="ws-claude" data={claudeData} />);
// The normal-mount path renders the FilesToolbar with the root
// selector. Wait for it (useEffect → loadFiles → setLoading false).
await waitFor(() => {
expect(screen.queryByText(/Files not available/i)).toBeNull();
});
// Toolbar's root selector confirms we're on the platform-owned
// rendering path, not the placeholder.
expect(screen.getByLabelText(/File root directory/i)).not.toBeNull();
});
it("data prop omitted falls through to normal mount (back-compat)", async () => {
render(<FilesTab workspaceId="ws-no-data" />);
await waitFor(() => {
expect(screen.queryByText(/Files not available/i)).toBeNull();
});
// Without data we can't gate on runtime — must mount normally.
expect(screen.getByLabelText(/File root directory/i)).not.toBeNull();
});
});

View File

@ -1,23 +1,20 @@
package handlers
// template_files_eic.go — SSH-backed file write for SaaS workspaces
// (EC2-per-workspace). Pairs with the existing Docker-path in templates.go
// (WriteFile) and template_import.go (ReplaceFiles).
// template_files_eic.go — SSH-backed file operations for SaaS workspaces
// (EC2-per-workspace). Pairs with the local-Docker path in templates.go
// (List/Read/Write/Delete) and template_import.go (ReplaceFiles).
//
// Flow for a single file write:
// 1. Generate ephemeral ed25519 keypair (on-disk for ≤ write duration).
// 2. Push the public key via `aws ec2-instance-connect send-ssh-public-key`
// so the target sshd accepts it for the next 60s.
// 3. Open a TLS-tunnelled TCP port via `aws ec2-instance-connect open-tunnel`
// from a local free port → workspace's sshd on 22.
// 4. Pipe content to `ssh ... "install -D -m 0644 /dev/stdin <abs path>"`.
// `install -D` creates any missing parent dirs atomically. File is owned
// by whichever $OSUser we authenticated as (ubuntu by default).
// 5. Close tunnel + wipe keydir.
// Architecture note: every operation goes through `withEICTunnel`, which
// owns the ephemeral-keypair → key-push → tunnel → port-wait dance. Per-
// op helpers (list/read/write/delete) only carry the remote command +
// stdin/stdout shape. This keeps the EIC connection logic in one place
// so a fix to the dance — e.g. PR #2822's `LogLevel=ERROR` shim — only
// touches one helper.
//
// All the AWS calls + ssh tunnel exec go through the same package-level
// func vars defined in terminal.go (openTunnelCmd, sendSSHPublicKey) so
// tests can stub them the same way the terminal tests do.
// Path translation rules: see resolveWorkspaceFilePath. `/configs`
// is the per-runtime managed-config indirection (claude-code → /configs,
// hermes → /home/ubuntu/.hermes); other allow-listed roots (`/home`,
// `/workspace`, `/plugins`) pass through literally.
import (
"bytes"
@ -32,8 +29,7 @@ import (
)
// workspaceFilePathPrefix maps a runtime name to the absolute base path on
// the workspace EC2 where the Files API's relative paths land. New runtimes
// can be added here without touching handler code.
// the workspace EC2 where the runtime's managed-config dir lives.
//
// Keep these stable — changing the base path for an existing runtime
// without a migration shim will make previously-saved files disappear from
@ -60,41 +56,104 @@ var workspaceFilePathPrefix = map[string]string{
// those runtimes actually have on disk.
}
func resolveWorkspaceFilePath(runtime, relPath string) (string, error) {
// resolveWorkspaceFilePath translates (runtime, root, relPath) into an
// absolute path on the workspace EC2.
//
// `root="/configs"` (or empty / unrecognized) is treated as the
// runtime's MANAGED-config dir via workspaceFilePathPrefix —
// /home/ubuntu/.hermes for hermes, /configs for claude-code, etc.
// This preserves the v1 ReadFile/WriteFile behavior where the canvas's
// Config tab GETs/PUTs "config.yaml" without specifying a root and
// lands in the runtime's own config dir, even though that dir's
// absolute path differs per runtime.
//
// Any other allow-listed root (`/home`, `/workspace`, `/plugins`) is
// treated as a LITERAL absolute path on the EC2 host. Those roots are
// universal Linux paths that don't need per-runtime indirection.
//
// Restricting the literal pass-through to allowedRoots is the
// security boundary — the handler also gates this same set, so the
// resolver is defence-in-depth: even if a future caller forgets the
// handler-side check, the resolver won't translate `?root=/etc` into
// a real absolute path.
//
// relPath is sanitised by validateRelPath (no absolute, no `..`).
func resolveWorkspaceFilePath(runtime, root, relPath string) (string, error) {
if err := validateRelPath(relPath); err != nil {
return "", err
}
base, ok := workspaceFilePathPrefix[strings.ToLower(strings.TrimSpace(runtime))]
if !ok {
base = "/configs"
}
base := resolveWorkspaceRootPath(runtime, root)
return filepath.Join(base, filepath.Clean(relPath)), nil
}
// eicFileWriteTimeout bounds the whole dance. Key push is <500ms, tunnel
// is 1-2s, ssh + write is <2s. 30s gives headroom for slow pulls without
// hanging the Files API forever under EIC misconfiguration.
const eicFileWriteTimeout = 30 * time.Second
// writeFileViaEIC writes a single file to the workspace EC2 at the
// absolute path that resolveWorkspaceFilePath computed. On success,
// optionally invokes the runtime's reload hook (not implemented yet —
// tracked as follow-up; for today the canvas issues a separate Restart
// after Save).
// resolveWorkspaceRootPath returns the absolute base directory on the
// workspace EC2 for a given (runtime, root) pair, without touching a
// relative file path. Used by listFilesViaEIC to compute the directory
// to walk; resolveWorkspaceFilePath joins this with relPath.
//
// instanceID: AWS EC2 instance id from workspaces.instance_id.
// runtime: used only for path-prefix resolution.
// relPath: the relative path the caller validated (no /, no ..).
// content: file body bytes.
func writeFileViaEIC(ctx context.Context, instanceID, runtime, relPath string, content []byte) error {
// Centralising the runtime-vs-literal indirection here means
// list/read/write/delete agree on what `?root=/configs` means for
// hermes vs claude-code vs an unknown runtime — otherwise list could
// show one directory while read/write target another.
func resolveWorkspaceRootPath(runtime, root string) string {
root = strings.TrimSpace(root)
// "/configs" + empty + unrecognized → runtime's managed-config dir.
// The runtime prefix map is the SSOT for that translation.
if root == "" || root == "/configs" || !allowedRoots[root] {
base, ok := workspaceFilePathPrefix[strings.ToLower(strings.TrimSpace(runtime))]
if !ok {
base = "/configs"
}
return base
}
// Literal universal path (`/home`, `/workspace`, `/plugins`).
return root
}
// eicFileOpTimeout bounds the whole tunnel + ssh dance. Key push is
// <500ms, tunnel is 1-2s, ssh + remote command is <2s for read/write.
// 30s gives headroom for slow EIC pulls + the larger `find` walk that
// listFilesViaEIC issues, without hanging the Files API forever under
// EIC misconfiguration.
const eicFileOpTimeout = 30 * time.Second
// eicFileOpTimeout was historically named eicFileWriteTimeout when the
// only EIC op was writeFile. Keep an alias so any external test that
// pinned the old name still compiles; rename can land as a follow-up
// once we've gone a release without the alias being touched.
//
//nolint:revive // intentional alias for back-compat with prior tests.
const eicFileWriteTimeout = eicFileOpTimeout
// eicSSHSession describes an open EIC tunnel ready for an ssh subprocess.
// Only valid inside the closure passed to withEICTunnel — the underlying
// keypair + tunnel are torn down when the closure returns.
type eicSSHSession struct {
keyPath string
localPort int
osUser string
instanceID string
}
// withEICTunnel sets up an EIC SSH session (ephemeral keypair → push
// → AWS open-tunnel → wait-for-port), invokes fn with a session handle,
// and tears everything down on return. The caller is responsible for
// applying the per-op context.WithTimeout before calling — this helper
// only owns the EIC dance, not the operation budget, so a caller that
// needs a different timeout (e.g. a large bulk import) doesn't have to
// fight a hard-coded one.
//
// All AWS calls go through the package-level func vars in terminal.go
// (sendSSHPublicKey, openTunnelCmd) so tests can stub them the same way
// terminal_test.go does. The whole helper is also assigned to a
// `var` (`withEICTunnel`) so handler-dispatch tests can stub the entire
// dance instead of having to wire up a fake tunnel + fake ssh server.
var withEICTunnel = realWithEICTunnel
func realWithEICTunnel(ctx context.Context, instanceID string, fn func(s eicSSHSession) error) error {
if instanceID == "" {
return fmt.Errorf("workspace has no instance_id — not a SaaS EC2 workspace")
}
absPath, err := resolveWorkspaceFilePath(runtime, relPath)
if err != nil {
return fmt.Errorf("invalid path: %w", err)
}
osUser := os.Getenv("WORKSPACE_EC2_OS_USER")
if osUser == "" {
osUser = "ubuntu"
@ -104,11 +163,7 @@ func writeFileViaEIC(ctx context.Context, instanceID, runtime, relPath string, c
region = "us-east-2"
}
ctx, cancel := context.WithTimeout(ctx, eicFileWriteTimeout)
defer cancel()
// Ephemeral keypair.
keyDir, err := os.MkdirTemp("", "molecule-filewrite-*")
keyDir, err := os.MkdirTemp("", "molecule-eic-*")
if err != nil {
return fmt.Errorf("keydir mkdir: %w", err)
}
@ -116,7 +171,7 @@ func writeFileViaEIC(ctx context.Context, instanceID, runtime, relPath string, c
keyPath := keyDir + "/id"
if out, kerr := exec.CommandContext(ctx, "ssh-keygen",
"-t", "ed25519", "-f", keyPath, "-N", "", "-q",
"-C", "molecule-filewrite",
"-C", "molecule-eic",
).CombinedOutput(); kerr != nil {
return fmt.Errorf("ssh-keygen: %w (%s)", kerr, strings.TrimSpace(string(out)))
}
@ -125,24 +180,21 @@ func writeFileViaEIC(ctx context.Context, instanceID, runtime, relPath string, c
return fmt.Errorf("read pubkey: %w", err)
}
// 1. Push key.
if err := sendSSHPublicKey(ctx, region, instanceID, osUser, strings.TrimSpace(string(pubKey))); err != nil {
return fmt.Errorf("send-ssh-public-key: %w", err)
}
// 2. Open tunnel on an OS-picked free port.
localPort, err := pickFreePort()
if err != nil {
return fmt.Errorf("pick free port: %w", err)
}
opts := eicSSHOptions{
tunnel := openTunnelCmd(eicSSHOptions{
InstanceID: instanceID,
OSUser: osUser,
Region: region,
LocalPort: localPort,
PrivateKeyPath: keyPath,
}
tunnel := openTunnelCmd(opts)
})
tunnel.Env = os.Environ()
if err := tunnel.Start(); err != nil {
return fmt.Errorf("open-tunnel start: %w", err)
@ -157,183 +209,330 @@ func writeFileViaEIC(ctx context.Context, instanceID, runtime, relPath string, c
return fmt.Errorf("tunnel never listened: %w", err)
}
// 3. SSH + install -D. `install` creates any missing parent dirs and
// writes the file atomically via temp-file-rename. Permissions 0644
// match the existing tar-unpack defaults on the Docker path.
//
// `sudo -n` (non-interactive) prefix: the canonical containerized
// workspace layout puts /configs at the root, owned by root because
// cloud-init runs as root (see
// molecule-controlplane/internal/provisioner/userdata_containerized.go).
// SSH-as-ubuntu can't write into /configs without escalation.
// Ubuntu has passwordless sudo on EC2 by default; sudo -n fails fast
// (no prompt) if that ever changes, surfacing a clean error instead
// of a hang. The hermes path /home/ubuntu/.hermes is ubuntu-owned
// and doesn't strictly need sudo, but using it uniformly avoids
// per-runtime branching here.
//
// The remote command is fully deterministic — no user-controlled
// input reaches a shell eval (absPath is built from a map + Clean()).
sshArgs := []string{
"-i", keyPath,
return fn(eicSSHSession{
keyPath: keyPath,
localPort: localPort,
osUser: osUser,
instanceID: instanceID,
})
}
// sshArgs returns the standard ssh CLI args for an EIC session pointed
// at the local tunnel port + a single remote command string.
//
// `LogLevel=ERROR` silences the benign "Warning: Permanently added
// '[127.0.0.1]:NNNNN' to known hosts" notice that ssh emits on every
// fresh tunnel connection. Without this, the notice lands on stderr
// and fools the read/list "empty stdout + empty stderr → not found"
// classifiers into thinking the warning is a real ssh-layer error → 500
// instead of 404 (Hermes config.yaml load, hongming tenant, 2026-05-05
// 02:38; PR #2822). Real auth/tunnel errors stay visible because they're
// emitted at ERROR level.
//
// Originally each helper assembled its own ssh args inline, so PR #2822's
// LogLevel=ERROR fix had to be applied to every copy. Centralising here
// means future ssh-option tweaks only land in one place.
func (s eicSSHSession) sshArgs(remoteCommand string) []string {
return []string{
"-i", s.keyPath,
"-o", "StrictHostKeyChecking=no",
"-o", "UserKnownHostsFile=/dev/null",
// LogLevel=ERROR silences the benign "Warning: Permanently
// added '[127.0.0.1]:NNNNN' to known hosts" notice that ssh
// emits on every fresh tunnel connection. Without this, the
// notice lands on stderr and fools readFileViaEIC's "empty
// stdout + empty stderr → file not found" classifier into
// thinking the warning is a real ssh-layer error → 500
// instead of 404 (Hermes config.yaml load, hongming tenant,
// 2026-05-05 02:38). Real auth/tunnel errors stay visible
// because they're emitted at ERROR level.
"-o", "LogLevel=ERROR",
"-o", "ServerAliveInterval=15",
"-p", fmt.Sprintf("%d", localPort),
fmt.Sprintf("%s@127.0.0.1", osUser),
fmt.Sprintf("sudo -n install -D -m 0644 /dev/stdin %s", shellQuote(absPath)),
"-p", fmt.Sprintf("%d", s.localPort),
fmt.Sprintf("%s@127.0.0.1", s.osUser),
remoteCommand,
}
sshCmd := exec.CommandContext(ctx, "ssh", sshArgs...)
sshCmd.Env = os.Environ()
sshCmd.Stdin = bytes.NewReader(content)
var stderr bytes.Buffer
sshCmd.Stderr = &stderr
if err := sshCmd.Run(); err != nil {
return fmt.Errorf("ssh install: %w (%s)", err, strings.TrimSpace(stderr.String()))
}
// buildInstallShell returns the remote command for atomically writing
// `/dev/stdin` to absPath with mode 0644 via `sudo -n install -D`.
// `install -D` creates any missing parent dirs and writes via
// temp-file-rename (atomic). Pure function for direct testability —
// the only variable input (absPath) is shellQuote-wrapped to defeat
// any shell metachar in a future caller's path.
func buildInstallShell(absPath string) string {
return fmt.Sprintf("sudo -n install -D -m 0644 /dev/stdin %s", shellQuote(absPath))
}
// buildCatShell returns the remote command for reading absPath and
// swallowing missing-file stderr (so the empty-stdout + non-zero-exit
// case is unambiguous → os.ErrNotExist at the caller).
func buildCatShell(absPath string) string {
return fmt.Sprintf("sudo -n cat %s 2>/dev/null", shellQuote(absPath))
}
// buildRmShell returns the remote command for `sudo -n rm -f` against
// absPath. `-f` (not `-rf`) is intentional — directory removal needs
// its own explicit endpoint if/when the canvas grows that affordance,
// and `-rf` would let a misclassified directory entry trigger a
// recursive delete.
func buildRmShell(absPath string) string {
return fmt.Sprintf("sudo -n rm -f %s", shellQuote(absPath))
}
// buildFindShell returns the remote command for enumerating files
// under listPath up to maxDepth, emitting `TYPE|SIZE|REL_PATH` lines
// (matches the local-Docker container path's parser exactly).
//
// `2>/dev/null` swallows find's "No such file" error so a missing
// listing root surfaces as empty stdout (handler returns []) rather
// than 500.
//
// `stat -c %s` is GNU coreutils; `stat -f %z` is BSD. Try GNU first,
// fall back to BSD, then 0 — same shape the local-Docker `sh -c`
// version uses so a future cross-runtime fleet (Alpine vs Ubuntu)
// doesn't regress.
//
// Hidden / cache dir pruning matches the container path: .git,
// __pycache__, node_modules, .DS_Store. Without these the tree drowns
// in transient artefacts on a /workspace listing.
func buildFindShell(listPath string, maxDepth int) string {
return fmt.Sprintf(
`sudo -n find %s -maxdepth %d -not -path '*/.git/*' -not -path '*/__pycache__/*' -not -path '*/node_modules/*' -not -name .DS_Store 2>/dev/null | while IFS= read -r f; do `+
`rel="${f#%s/}"; [ "$rel" = %s ] && continue; [ -z "$rel" ] && continue; `+
`if [ -d "$f" ]; then echo "d|0|$rel"; else `+
`s=$(stat -c %%s "$f" 2>/dev/null || stat -f %%z "$f" 2>/dev/null || echo 0); echo "f|$s|$rel"; `+
`fi; done`,
shellQuote(listPath), maxDepth, shellQuote(listPath), shellQuote(listPath),
)
}
// parseFindOutput parses TYPE|SIZE|REL_PATH lines emitted by
// buildFindShell into eicFileEntry rows. Whitespace-only lines and
// malformed rows are silently skipped — the same behaviour as the
// local-Docker container parser for symmetric output.
func parseFindOutput(raw []byte) []eicFileEntry {
files := make([]eicFileEntry, 0)
for _, line := range strings.Split(string(raw), "\n") {
parts := strings.SplitN(line, "|", 3)
if len(parts) != 3 || parts[2] == "" {
continue
}
var size int64
fmt.Sscanf(parts[1], "%d", &size)
files = append(files, eicFileEntry{
Path: parts[2],
Size: size,
Dir: parts[0] == "d",
})
}
log.Printf("writeFileViaEIC: ws instance=%s runtime=%s wrote %d bytes → %s",
instanceID, runtime, len(content), absPath)
return nil
return files
}
// shellQuote wraps a value in single quotes + escapes embedded single
// quotes for POSIX sh. Used for the sole piece of variable data in the
// remote ssh command. (absPath is already built from a map + Clean() so
// traversal is blocked regardless; this is defence-in-depth against
// future refactor that might accept user paths here.)
// quotes for POSIX sh. Used for the variable parts of remote ssh
// commands (absolute paths). The paths are already built from a
// validated allowlist + Clean(), so traversal is blocked regardless;
// this is defence-in-depth against a future refactor that might accept
// user paths directly here.
func shellQuote(s string) string {
return "'" + strings.ReplaceAll(s, "'", `'\''`) + "'"
}
// writeFileViaEIC writes a single file to the workspace EC2 at the
// absolute path that resolveWorkspaceFilePath computed. On success,
// optionally invokes the runtime's reload hook (not implemented yet —
// tracked as follow-up; for today the canvas issues a separate Restart
// after Save).
//
// `install -D` creates any missing parent dirs and writes atomically
// via temp-file-rename. Permissions 0644 match the existing tar-unpack
// defaults on the Docker path.
//
// `sudo -n` (non-interactive) prefix: the canonical containerized
// workspace layout puts /configs at the root, owned by root because
// cloud-init runs as root (see
// molecule-controlplane/internal/provisioner/userdata_containerized.go).
// SSH-as-ubuntu can't write into /configs without escalation. Ubuntu
// has passwordless sudo on EC2 by default; sudo -n fails fast (no
// prompt) if that ever changes, surfacing a clean error instead of a
// hang. The hermes path /home/ubuntu/.hermes is ubuntu-owned and
// doesn't strictly need sudo, but using it uniformly avoids per-runtime
// branching here.
func writeFileViaEIC(ctx context.Context, instanceID, runtime, root, relPath string, content []byte) error {
absPath, err := resolveWorkspaceFilePath(runtime, root, relPath)
if err != nil {
return fmt.Errorf("invalid path: %w", err)
}
ctx, cancel := context.WithTimeout(ctx, eicFileOpTimeout)
defer cancel()
return withEICTunnel(ctx, instanceID, func(s eicSSHSession) error {
sshCmd := exec.CommandContext(ctx, "ssh", s.sshArgs(buildInstallShell(absPath))...)
sshCmd.Env = os.Environ()
sshCmd.Stdin = bytes.NewReader(content)
var stderr bytes.Buffer
sshCmd.Stderr = &stderr
if err := sshCmd.Run(); err != nil {
return fmt.Errorf("ssh install: %w (%s)", err, strings.TrimSpace(stderr.String()))
}
log.Printf("writeFileViaEIC: ws instance=%s runtime=%s root=%s wrote %d bytes → %s",
instanceID, runtime, root, len(content), absPath)
return nil
})
}
// readFileViaEIC reads a single file from the workspace EC2 at the
// absolute path that resolveWorkspaceFilePath computes. Mirrors
// writeFileViaEIC end-to-end (ephemeral keypair, EIC tunnel, ssh) so
// canvas's Config tab can GET back what it just PUT. Pre-fix the GET
// path (templates.go ReadFile) only handled local Docker containers
// + a host-side template fallback; SaaS workspaces (EC2-per-workspace)
// always 404'd because neither handles their on-EC2 layout.
// writeFileViaEIC (ephemeral keypair, EIC tunnel, ssh) so the canvas's
// Config tab can GET back what it just PUT.
//
// Returns ("", os.ErrNotExist) when the remote path doesn't exist so
// the handler can map it to HTTP 404 cleanly. Other errors propagate.
func readFileViaEIC(ctx context.Context, instanceID, runtime, relPath string) ([]byte, error) {
if instanceID == "" {
return nil, fmt.Errorf("workspace has no instance_id — not a SaaS EC2 workspace")
}
absPath, err := resolveWorkspaceFilePath(runtime, relPath)
//
// `sudo -n cat`: /configs is root-owned (same reason writeFileViaEIC
// needs sudo). The path is built from a validated map + Clean(), so no
// user-controlled string reaches the shell here. `2>/dev/null` swallows
// `cat: ...: No such file` so the missing-file case returns empty
// stdout + non-zero exit, which we translate to os.ErrNotExist.
func readFileViaEIC(ctx context.Context, instanceID, runtime, root, relPath string) ([]byte, error) {
absPath, err := resolveWorkspaceFilePath(runtime, root, relPath)
if err != nil {
return nil, fmt.Errorf("invalid path: %w", err)
}
osUser := os.Getenv("WORKSPACE_EC2_OS_USER")
if osUser == "" {
osUser = "ubuntu"
}
region := os.Getenv("AWS_REGION")
if region == "" {
region = "us-east-2"
}
ctx, cancel := context.WithTimeout(ctx, eicFileWriteTimeout)
ctx, cancel := context.WithTimeout(ctx, eicFileOpTimeout)
defer cancel()
keyDir, err := os.MkdirTemp("", "molecule-fileread-*")
if err != nil {
return nil, fmt.Errorf("keydir mkdir: %w", err)
}
defer func() { _ = os.RemoveAll(keyDir) }()
keyPath := keyDir + "/id"
if out, kerr := exec.CommandContext(ctx, "ssh-keygen",
"-t", "ed25519", "-f", keyPath, "-N", "", "-q",
"-C", "molecule-fileread",
).CombinedOutput(); kerr != nil {
return nil, fmt.Errorf("ssh-keygen: %w (%s)", kerr, strings.TrimSpace(string(out)))
}
pubKey, err := os.ReadFile(keyPath + ".pub")
if err != nil {
return nil, fmt.Errorf("read pubkey: %w", err)
}
if err := sendSSHPublicKey(ctx, region, instanceID, osUser, strings.TrimSpace(string(pubKey))); err != nil {
return nil, fmt.Errorf("send-ssh-public-key: %w", err)
}
localPort, err := pickFreePort()
if err != nil {
return nil, fmt.Errorf("pick free port: %w", err)
}
tunnel := openTunnelCmd(eicSSHOptions{
InstanceID: instanceID,
OSUser: osUser,
Region: region,
LocalPort: localPort,
PrivateKeyPath: keyPath,
var out []byte
runErr := withEICTunnel(ctx, instanceID, func(s eicSSHSession) error {
sshCmd := exec.CommandContext(ctx, "ssh", s.sshArgs(buildCatShell(absPath))...)
sshCmd.Env = os.Environ()
var stdout, stderr bytes.Buffer
sshCmd.Stdout = &stdout
sshCmd.Stderr = &stderr
err := sshCmd.Run()
out = stdout.Bytes()
if err != nil {
// `cat` returns 1 on missing file; with 2>/dev/null we have no
// stderr distinguisher. Treat empty-stdout + empty-stderr +
// non-zero exit as not-found rather than a tunnel/auth error
// (those usually produce stderr from ssh itself, not from the
// remote command).
if len(out) == 0 && stderr.Len() == 0 {
return os.ErrNotExist
}
return fmt.Errorf("ssh cat: %w (%s)", err, strings.TrimSpace(stderr.String()))
}
log.Printf("readFileViaEIC: ws instance=%s runtime=%s root=%s read %d bytes ← %s",
instanceID, runtime, root, len(out), absPath)
return nil
})
tunnel.Env = os.Environ()
if err := tunnel.Start(); err != nil {
return nil, fmt.Errorf("open-tunnel start: %w", err)
}
defer func() {
if tunnel.Process != nil {
_ = tunnel.Process.Kill()
}
_ = tunnel.Wait()
}()
if err := waitForPort(ctx, "127.0.0.1", localPort, 10*time.Second); err != nil {
return nil, fmt.Errorf("tunnel never listened: %w", err)
}
// `sudo -n cat`: /configs is root-owned by cloud-init (same reason
// writeFileViaEIC needs sudo to install). The path is built from a
// validated map + Clean(), so no user-controlled string reaches the
// shell here. `2>/dev/null` swallows `cat: ...: No such file` so
// the missing-file case returns empty stdout + non-zero exit, which
// we translate to os.ErrNotExist below.
sshCmd := exec.CommandContext(ctx, "ssh",
"-i", keyPath,
"-o", "StrictHostKeyChecking=no",
"-o", "UserKnownHostsFile=/dev/null",
// LogLevel=ERROR silences the benign "Warning: Permanently
// added '[127.0.0.1]:NNNNN' to known hosts" notice that ssh
// emits on every fresh tunnel connection. Without this, the
// notice lands on stderr and fools readFileViaEIC's "empty
// stdout + empty stderr → file not found" classifier into
// thinking the warning is a real ssh-layer error → 500
// instead of 404 (Hermes config.yaml load, hongming tenant,
// 2026-05-05 02:38). Real auth/tunnel errors stay visible
// because they're emitted at ERROR level.
"-o", "LogLevel=ERROR",
"-o", "ServerAliveInterval=15",
"-p", fmt.Sprintf("%d", localPort),
fmt.Sprintf("%s@127.0.0.1", osUser),
fmt.Sprintf("sudo -n cat %s 2>/dev/null", shellQuote(absPath)),
)
sshCmd.Env = os.Environ()
var stdout, stderr bytes.Buffer
sshCmd.Stdout = &stdout
sshCmd.Stderr = &stderr
runErr := sshCmd.Run()
out := stdout.Bytes()
if runErr != nil {
// `cat` returns 1 on missing file; with 2>/dev/null we have no
// stderr distinguisher. Treat empty-stdout + non-zero exit as
// not-found rather than a tunnel/auth error (those usually
// produce stderr from ssh itself, not from the remote command).
if len(out) == 0 && stderr.Len() == 0 {
return nil, os.ErrNotExist
}
return nil, fmt.Errorf("ssh cat: %w (%s)", runErr, strings.TrimSpace(stderr.String()))
return nil, runErr
}
log.Printf("readFileViaEIC: ws instance=%s runtime=%s read %d bytes ← %s",
instanceID, runtime, len(out), absPath)
return out, nil
}
// eicFileEntry is the wire shape returned by listFilesViaEIC. It
// matches the inline `fileEntry` in templates.go::ListFiles so the
// handler can emit either path's output without a translation layer.
type eicFileEntry struct {
Path string `json:"path"`
Size int64 `json:"size"`
Dir bool `json:"dir"`
}
// listFilesViaEIC enumerates files under <root>/<sub> on the workspace
// EC2 host, up to the given depth, returning entries with paths
// relative to the listing root (matching the local-Docker path's
// output). Closes the symmetry gap that left ListFiles silently
// returning [] for SaaS workspaces — see issue #2999.
//
// Output line format: TYPE|SIZE|REL_PATH (matches the container's find
// shell so the parser is identical). `find -maxdepth N` traverses up
// to N levels; the canvas requests depth=1 by default and re-fetches
// when the user expands a directory.
//
// Pruning: same hidden / cache dirs as the container path (.git,
// __pycache__, node_modules, .DS_Store) so the canvas's tree doesn't
// drown in transient artefacts.
//
// `sudo -n` matches the read/write paths — even though the universal
// roots (/home, /workspace, /plugins) are typically ubuntu-owned and
// don't need it, /configs and runtime-prefix dirs do (root-owned by
// cloud-init), and using sudo uniformly avoids per-root branching.
func listFilesViaEIC(ctx context.Context, instanceID, runtime, root, sub string, depth int) ([]eicFileEntry, error) {
if sub != "" {
if err := validateRelPath(sub); err != nil {
return nil, fmt.Errorf("invalid sub: %w", err)
}
}
if depth < 1 {
depth = 1
}
if depth > 5 {
depth = 5
}
listPath := resolveWorkspaceRootPath(runtime, root)
if sub != "" {
listPath = filepath.Join(listPath, filepath.Clean(sub))
}
ctx, cancel := context.WithTimeout(ctx, eicFileOpTimeout)
defer cancel()
var rawOutput []byte
runErr := withEICTunnel(ctx, instanceID, func(s eicSSHSession) error {
sshCmd := exec.CommandContext(ctx, "ssh", s.sshArgs(buildFindShell(listPath, depth))...)
sshCmd.Env = os.Environ()
var stdout, stderr bytes.Buffer
sshCmd.Stdout = &stdout
sshCmd.Stderr = &stderr
if err := sshCmd.Run(); err != nil {
// Empty stdout + empty stderr after we swallowed find's
// own error stream means the listing root genuinely
// doesn't exist on this workspace — return an empty
// slice rather than a 500. Real ssh/tunnel errors emit
// to stderr at LogLevel=ERROR.
if stdout.Len() == 0 && stderr.Len() == 0 {
rawOutput = nil
return nil
}
return fmt.Errorf("ssh find: %w (%s)", err, strings.TrimSpace(stderr.String()))
}
rawOutput = stdout.Bytes()
return nil
})
if runErr != nil {
return nil, runErr
}
files := parseFindOutput(rawOutput)
log.Printf("listFilesViaEIC: ws instance=%s runtime=%s root=%s sub=%s depth=%d → %d entries from %s",
instanceID, runtime, root, sub, depth, len(files), listPath)
return files, nil
}
// deleteFileViaEIC removes a single file from the workspace EC2.
// Returns nil for both "deleted" and "didn't exist" — `rm -f` doesn't
// distinguish, and the canvas's delete-then-refresh flow doesn't need
// it to.
//
// Symmetry note: pre-fix DeleteFile (templates.go:514) had no EIC
// branch, so right-click delete on a SaaS workspace would fall through
// to the local-Docker path, find no container (dockerCli is nil on
// SaaS), and try the ephemeral-volume path which itself only handles
// local Docker volumes. Net effect: silent no-op. Closing this gap is
// part of issue #2999.
func deleteFileViaEIC(ctx context.Context, instanceID, runtime, root, relPath string) error {
absPath, err := resolveWorkspaceFilePath(runtime, root, relPath)
if err != nil {
return fmt.Errorf("invalid path: %w", err)
}
ctx, cancel := context.WithTimeout(ctx, eicFileOpTimeout)
defer cancel()
return withEICTunnel(ctx, instanceID, func(s eicSSHSession) error {
sshCmd := exec.CommandContext(ctx, "ssh", s.sshArgs(buildRmShell(absPath))...)
sshCmd.Env = os.Environ()
var stderr bytes.Buffer
sshCmd.Stderr = &stderr
if err := sshCmd.Run(); err != nil {
return fmt.Errorf("ssh rm: %w (%s)", err, strings.TrimSpace(stderr.String()))
}
log.Printf("deleteFileViaEIC: ws instance=%s runtime=%s root=%s removed %s",
instanceID, runtime, root, absPath)
return nil
})
}

View File

@ -0,0 +1,303 @@
package handlers
// template_files_eic_dispatch_test.go — handler-level tests for the
// EIC dispatch added in PR-A of issue #2999. Pre-PR-A, ListFiles and
// DeleteFile silently fell through to the local-Docker path on SaaS
// workspaces (where dockerCli is nil) and returned [] / silent no-op.
// These tests pin the new behavior:
//
// 1. instance_id != "" → handler invokes the EIC helper
// 2. EIC success → 200 with the helper's payload
// 3. EIC error → 500 (does NOT fall through to local-Docker /
// template-dir, which would mask the real failure)
// 4. instance_id == "" → existing local-Docker / template-dir
// fallback (back-compat with self-hosted operators)
//
// Stubs `withEICTunnel` so the entire EIC dance (keypair, AWS calls,
// tunnel, ssh) is replaced with a fake closure that yields a captured
// session — lets the test capture what the inner closure would have
// done without spinning up a real sshd. The test for the actual
// remote shell shapes lives in template_files_eic_shells_test.go
// (pure-function tests on buildFindShell / buildInstallShell etc).
import (
"context"
"encoding/json"
"errors"
"net/http"
"net/http/httptest"
"strings"
"testing"
"github.com/DATA-DOG/go-sqlmock"
"github.com/gin-gonic/gin"
)
// stubWithEICTunnel replaces the package-level withEICTunnel with a
// closure that records its inputs and runs fn against a fake session,
// returning fnErr from the inner fn if non-nil. Restores the original
// on test cleanup.
func stubWithEICTunnel(t *testing.T, fnErr error) (calls *[]string) {
t.Helper()
captured := []string{}
calls = &captured
prev := withEICTunnel
withEICTunnel = func(ctx context.Context, instanceID string, fn func(s eicSSHSession) error) error {
captured = append(captured, instanceID)
// Hand the closure a sentinel session so any code that pulls
// session fields gets deterministic non-empty values. The
// closure's exec.Command call will fail at runtime because no
// real ssh exists for instanceID="i-test"; but most
// dispatch-tests inject fnErr directly to skip that.
return fnErr
}
t.Cleanup(func() { withEICTunnel = prev })
return calls
}
// stubWithEICTunnelReturning is like stubWithEICTunnel but lets the
// test substitute the inner fn entirely so it can populate `out` /
// return shaped errors without invoking the real ssh closure.
func stubWithEICTunnelReturning(t *testing.T, replacement func(s eicSSHSession) error) (calls *[]string) {
t.Helper()
captured := []string{}
calls = &captured
prev := withEICTunnel
withEICTunnel = func(ctx context.Context, instanceID string, _ func(s eicSSHSession) error) error {
captured = append(captured, instanceID)
return replacement(eicSSHSession{instanceID: instanceID, osUser: "ubuntu", localPort: 12345, keyPath: "/tmp/k"})
}
t.Cleanup(func() { withEICTunnel = prev })
return calls
}
// ---- ListFiles EIC dispatch ----
// TestListFiles_EICDispatch_Success: a workspace with instance_id set
// must route to listFilesViaEIC, NOT to local-Docker / template-dir.
// Verifies the handler hands the EIC helper's output back as JSON.
//
// Until PR-A this test would fail no matter what mocks were in place —
// the dispatch branch did not exist.
func TestListFiles_EICDispatch_Success(t *testing.T) {
mock := setupTestDB(t)
setupTestRedis(t)
mock.ExpectQuery(`SELECT name, COALESCE\(instance_id, ''\), COALESCE\(runtime, ''\) FROM workspaces WHERE id =`).
WithArgs("ws-eic").
WillReturnRows(sqlmock.NewRows([]string{"name", "instance_id", "runtime"}).
AddRow("My Agent", "i-test", "claude-code"))
// The package-level withEICTunnel stub doesn't get to set the
// listFilesViaEIC outparam, so we have to override the helper at
// a higher level. Instead, we stub withEICTunnel to *return* the
// inner closure's err — but we can't reach the byte-output path.
// Use the dedicated stubWithEICTunnelReturning + intercept ssh:
// since the tunnel stub doesn't run the closure's ssh exec at all
// when we replace the inner fn, the helper's `rawOutput` stays
// nil and parseFindOutput returns []. Sufficient for "200 + empty"
// dispatch verification.
stubWithEICTunnelReturning(t, func(s eicSSHSession) error {
return nil // skip the real ssh; outer rawOutput stays nil → []
})
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Params = gin.Params{{Key: "id", Value: "ws-eic"}}
c.Request = httptest.NewRequest("GET", "/workspaces/ws-eic/files?root=/configs", nil)
(&TemplatesHandler{}).ListFiles(c)
if w.Code != http.StatusOK {
t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String())
}
var got []map[string]interface{}
if err := json.Unmarshal(w.Body.Bytes(), &got); err != nil {
t.Fatalf("response not JSON array: %v (body=%s)", err, w.Body.String())
}
// EIC stub returned no output → empty list. The point of this
// assertion is "200 with [] from EIC", not "fell through to host
// template fallback which would 200 with []" — to discriminate,
// we ALSO assert mock expectations were met (proving the new SQL
// shape was queried) AND the local-Docker fallback path can't
// have run (handler.docker is nil here, so findContainer returns
// "" and the only paths that reach 200 are EIC or template-dir;
// template-dir requires a non-empty configsDir which we left at
// "" via the zero-value handler).
if got == nil {
t.Errorf("expected JSON array (even if empty); got null")
}
if err := mock.ExpectationsWereMet(); err != nil {
t.Errorf("unmet sqlmock expectations: %v", err)
}
}
// TestListFiles_EICDispatch_Error: a real EIC failure (network blip,
// AWS API throttle, sshd down) must surface as 500, NOT silently fall
// through to the local-Docker path which would mask the failure as
// "0 files" — which is the exact UX symptom the PR-A bug report cites.
func TestListFiles_EICDispatch_Error(t *testing.T) {
mock := setupTestDB(t)
setupTestRedis(t)
mock.ExpectQuery(`SELECT name, COALESCE\(instance_id, ''\), COALESCE\(runtime, ''\) FROM workspaces WHERE id =`).
WithArgs("ws-eic-err").
WillReturnRows(sqlmock.NewRows([]string{"name", "instance_id", "runtime"}).
AddRow("My Agent", "i-test", "claude-code"))
stubWithEICTunnel(t, errors.New("eic open-tunnel: timeout"))
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Params = gin.Params{{Key: "id", Value: "ws-eic-err"}}
c.Request = httptest.NewRequest("GET", "/workspaces/ws-eic-err/files?root=/home", nil)
(&TemplatesHandler{}).ListFiles(c)
if w.Code != http.StatusInternalServerError {
t.Fatalf("expected 500, got %d: %s", w.Code, w.Body.String())
}
if !strings.Contains(w.Body.String(), "failed to list files") {
t.Errorf("error body should describe ListFiles failure; got %s", w.Body.String())
}
}
// TestListFiles_EICBranch_NotTakenForSelfHosted: workspaces with no
// instance_id (self-hosted, local-Docker path) MUST NOT enter the EIC
// branch. Stubs withEICTunnel to fail loudly if it's called — the
// stub being invoked is itself the assertion failure.
func TestListFiles_EICBranch_NotTakenForSelfHosted(t *testing.T) {
mock := setupTestDB(t)
setupTestRedis(t)
mock.ExpectQuery(`SELECT name, COALESCE\(instance_id, ''\), COALESCE\(runtime, ''\) FROM workspaces WHERE id =`).
WithArgs("ws-local").
WillReturnRows(sqlmock.NewRows([]string{"name", "instance_id", "runtime"}).
AddRow("Local Agent", "", ""))
prev := withEICTunnel
withEICTunnel = func(ctx context.Context, instanceID string, fn func(s eicSSHSession) error) error {
t.Errorf("withEICTunnel called for self-hosted workspace (instance_id=''); EIC branch must be gated on non-empty instance_id")
return errors.New("should not be called")
}
defer func() { withEICTunnel = prev }()
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Params = gin.Params{{Key: "id", Value: "ws-local"}}
c.Request = httptest.NewRequest("GET", "/workspaces/ws-local/files", nil)
(&TemplatesHandler{configsDir: t.TempDir()}).ListFiles(c)
// Don't pin the response code here — the local path's behavior is
// covered by TestListFiles_FallbackToHost_NoTemplate. Just confirm
// EIC wasn't called.
}
// ---- DeleteFile EIC dispatch ----
// TestDeleteFile_EICDispatch_Success: same shape as ListFiles —
// instance_id != "" routes to deleteFileViaEIC and returns 200 on
// success. Pre-PR-A right-click delete on a SaaS workspace silently
// no-op'd because findContainer returned "" and the ephemeral-volume
// fallback only handles local Docker volumes.
func TestDeleteFile_EICDispatch_Success(t *testing.T) {
mock := setupTestDB(t)
setupTestRedis(t)
mock.ExpectQuery(`SELECT name, COALESCE\(instance_id, ''\), COALESCE\(runtime, ''\) FROM workspaces WHERE id =`).
WithArgs("ws-eic-del").
WillReturnRows(sqlmock.NewRows([]string{"name", "instance_id", "runtime"}).
AddRow("My Agent", "i-test", "claude-code"))
stubWithEICTunnel(t, nil)
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Params = gin.Params{
{Key: "id", Value: "ws-eic-del"},
{Key: "path", Value: "old.txt"},
}
c.Request = httptest.NewRequest("DELETE", "/workspaces/ws-eic-del/files/old.txt", nil)
(&TemplatesHandler{}).DeleteFile(c)
if w.Code != http.StatusOK {
t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String())
}
if !strings.Contains(w.Body.String(), `"deleted"`) {
t.Errorf("expected status:deleted; got %s", w.Body.String())
}
}
func TestDeleteFile_EICDispatch_Error(t *testing.T) {
mock := setupTestDB(t)
setupTestRedis(t)
mock.ExpectQuery(`SELECT name, COALESCE\(instance_id, ''\), COALESCE\(runtime, ''\) FROM workspaces WHERE id =`).
WithArgs("ws-eic-del-err").
WillReturnRows(sqlmock.NewRows([]string{"name", "instance_id", "runtime"}).
AddRow("My Agent", "i-test", "hermes"))
stubWithEICTunnel(t, errors.New("ssh rm: connection refused"))
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Params = gin.Params{
{Key: "id", Value: "ws-eic-del-err"},
{Key: "path", Value: "old.txt"},
}
c.Request = httptest.NewRequest("DELETE", "/workspaces/ws-eic-del-err/files/old.txt", nil)
(&TemplatesHandler{}).DeleteFile(c)
if w.Code != http.StatusInternalServerError {
t.Fatalf("expected 500, got %d: %s", w.Code, w.Body.String())
}
}
// TestListFiles_RootValidation: the handler must reject roots outside
// the allowlist BEFORE any DB query (otherwise a bad root would burn
// a tunnel + EIC call to discover what a 400 already knows). Critical
// security guard — without it `?root=/etc` would translate via the
// resolver's literal-pass-through. Let me prove the gate exists by
// driving an out-of-allowlist root and asserting 400 + no DB query.
func TestListFiles_RootValidation(t *testing.T) {
setupTestDB(t)
setupTestRedis(t)
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Params = gin.Params{{Key: "id", Value: "ws-x"}}
c.Request = httptest.NewRequest("GET", "/workspaces/ws-x/files?root=/etc", nil)
(&TemplatesHandler{}).ListFiles(c)
if w.Code != http.StatusBadRequest {
t.Errorf("expected 400 for /etc root, got %d: %s", w.Code, w.Body.String())
}
}
// TestDeleteFile_RootValidation mirrors the ListFiles guard. PR-A
// added ?root= handling to DeleteFile so the canvas's right-click
// delete works on any root (not just /configs) — that means the
// allowlist guard has to be present here too, otherwise an unsafe
// root flows straight into the resolver.
func TestDeleteFile_RootValidation(t *testing.T) {
setupTestDB(t)
setupTestRedis(t)
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Params = gin.Params{
{Key: "id", Value: "ws-x"},
{Key: "path", Value: "f.txt"},
}
c.Request = httptest.NewRequest("DELETE", "/workspaces/ws-x/files/f.txt?root=/etc", nil)
(&TemplatesHandler{}).DeleteFile(c)
if w.Code != http.StatusBadRequest {
t.Errorf("expected 400 for /etc root, got %d: %s", w.Code, w.Body.String())
}
}

View File

@ -0,0 +1,200 @@
package handlers
// template_files_eic_shells_test.go — pure-function tests for the
// remote shell builders + parser. Factored out of the EIC helpers so
// the wire shape can be pinned without standing up a real EIC tunnel
// or sshd. If a future edit changes the find/install/cat/rm shell in
// a way that drifts from the local-Docker container path, these tests
// catch it before staging.
import (
"strings"
"testing"
)
// TestBuildInstallShell pins the write-side remote command. `install`
// (not `cp`/`tee`) is load-bearing — it creates parent dirs (-D) and
// writes atomically via temp-file-rename. Permissions 0644 match the
// local-Docker tar-unpack defaults so a save → restart → save → restart
// cycle doesn't flip-flop file modes per backend.
func TestBuildInstallShell(t *testing.T) {
got := buildInstallShell("/configs/config.yaml")
wants := []string{
"sudo -n", // privilege escalation for root-owned /configs
"install -D", // creates parent dirs
"-m 0644", // permission contract
"/dev/stdin", // pipe-from-ssh source
"'/configs/config.yaml'", // shell-quoted destination
}
for _, w := range wants {
if !strings.Contains(got, w) {
t.Errorf("buildInstallShell missing %q in: %s", w, got)
}
}
}
// TestBuildCatShell pins the read-side remote command. `2>/dev/null`
// is load-bearing: without it the missing-file case emits "cat: ...:
// No such file" to stderr, and the helper's "empty stdout + empty
// stderr → os.ErrNotExist" classifier fires the wrong branch (500
// instead of 404). The tunnel-warning silencer (LogLevel=ERROR in
// sshArgs) handles the ssh side; this one handles the remote-cmd side.
func TestBuildCatShell(t *testing.T) {
got := buildCatShell("/home/ubuntu/.hermes/config.yaml")
wants := []string{
"sudo -n",
"cat",
"'/home/ubuntu/.hermes/config.yaml'",
"2>/dev/null", // missing-file → empty stdout + non-zero exit
}
for _, w := range wants {
if !strings.Contains(got, w) {
t.Errorf("buildCatShell missing %q in: %s", w, got)
}
}
}
// TestBuildRmShell pins `rm -f`, NOT `rm -rf`. A misclassified
// directory entry passing through the validator must NOT trigger a
// recursive delete. Directory removal needs its own explicit endpoint
// when/if the canvas grows that affordance.
func TestBuildRmShell(t *testing.T) {
got := buildRmShell("/configs/dead.yaml")
wants := []string{"sudo -n", "rm -f", "'/configs/dead.yaml'"}
for _, w := range wants {
if !strings.Contains(got, w) {
t.Errorf("buildRmShell missing %q in: %s", w, got)
}
}
// Negative assertion: NEVER emit -rf.
if strings.Contains(got, "rm -rf") {
t.Errorf("buildRmShell uses -rf, must use -f only: %s", got)
}
}
// TestBuildFindShell pins the listing-side remote command — it must
// match the local-Docker path's parser shape (TYPE|SIZE|REL_PATH per
// line) AND prune the same hidden / cache directories. If either
// side drifts, a /workspace listing on EC2 either drowns in node_modules
// noise (pruning regression) or drops files entirely (parser shape
// regression).
func TestBuildFindShell(t *testing.T) {
got := buildFindShell("/workspace", 2)
wants := []string{
"sudo -n find",
"'/workspace'",
"-maxdepth 2",
// Matches local-Docker container path; without these the EC2
// listing fills with VCS/build artefacts.
"-not -path '*/.git/*'",
"-not -path '*/__pycache__/*'",
"-not -path '*/node_modules/*'",
"-not -name .DS_Store",
"2>/dev/null", // missing-root → empty stdout + non-zero exit
// Wire shape — emit "TYPE|SIZE|REL_PATH" so parseFindOutput
// (and the canvas tree builder) can decode each line.
"d|0|",
"f|",
// Portable stat: GNU first, BSD fallback, then 0.
"stat -c %s",
"stat -f %z",
}
for _, w := range wants {
if !strings.Contains(got, w) {
t.Errorf("buildFindShell missing %q in: %s", w, got)
}
}
}
// TestBuildFindShell_DepthForwarding catches a regression where the
// helper hard-codes a depth instead of using the caller's value.
// `?depth=` on the canvas side controls how many levels expand on
// load — losing it means the file tree is either empty (depth=0) or
// the network blows up on a top-level /home with everyone's $HOME
// (uncapped).
func TestBuildFindShell_DepthForwarding(t *testing.T) {
for _, d := range []int{1, 3, 5} {
got := buildFindShell("/configs", d)
want := "-maxdepth " + intToStr(d)
if !strings.Contains(got, want) {
t.Errorf("buildFindShell depth=%d output missing %q: %s", d, want, got)
}
}
}
// intToStr avoids pulling strconv into a one-liner; matches the shell
// builder's fmt.Sprintf %d output exactly.
func intToStr(n int) string {
if n == 0 {
return "0"
}
neg := n < 0
if neg {
n = -n
}
var buf [20]byte
i := len(buf)
for n > 0 {
i--
buf[i] = byte('0' + n%10)
n /= 10
}
s := string(buf[i:])
if neg {
return "-" + s
}
return s
}
// TestParseFindOutput pins the parser. Each line is TYPE|SIZE|REL,
// blank/short lines silently skipped. Pre-PR-A this logic was inlined
// in the handler with the same shape; extracting + testing separately
// removes the "regex passes against the inline parser but a future
// refactor of the handler subtly changes the parse" failure mode.
func TestParseFindOutput(t *testing.T) {
in := []byte(`d|0|nested
f|123|nested/a.yaml
f|45|README.md
invalid-line
f||no-size
d|0|
`)
got := parseFindOutput(in)
// Want 4 entries: nested(d), nested/a.yaml(f,123), README.md(f,45),
// no-size(f,0). Blank lines, "invalid-line" (no pipes), and
// `d|0|` (empty rel) are skipped.
wantPaths := []string{"nested", "nested/a.yaml", "README.md", "no-size"}
if len(got) != len(wantPaths) {
t.Fatalf("got %d entries, want %d: %+v", len(got), len(wantPaths), got)
}
for i, w := range wantPaths {
if got[i].Path != w {
t.Errorf("entry[%d].Path = %q, want %q", i, got[i].Path, w)
}
}
if !got[0].Dir {
t.Errorf("entry[0] should be Dir")
}
if got[1].Size != 123 {
t.Errorf("entry[1].Size = %d, want 123", got[1].Size)
}
if got[3].Size != 0 {
t.Errorf("entry[3].Size on missing-size line = %d, want 0", got[3].Size)
}
}
// TestParseFindOutput_EmptyInput — a missing listing root yields
// empty stdout (find swallows the "No such file" via 2>/dev/null),
// which must round-trip to a JSON `[]`, not null. The handler does
// `make([]eicFileEntry, 0)` to enforce this; the test pins the
// helper-level guarantee independently.
func TestParseFindOutput_EmptyInput(t *testing.T) {
got := parseFindOutput([]byte(""))
if got == nil {
t.Errorf("parseFindOutput(\"\") returned nil; want empty slice for JSON []")
}
if len(got) != 0 {
t.Errorf("parseFindOutput(\"\") = %+v; want []", got)
}
}

View File

@ -7,39 +7,112 @@ import (
"testing"
)
// TestResolveWorkspaceFilePath_KnownRuntimes — the runtime → base-path
// map is the source of truth for where saved files land on the workspace
// EC2. Changing a base path without a migration shim silently orphans
// previously-saved files; this test pins the current contract.
func TestResolveWorkspaceFilePath_KnownRuntimes(t *testing.T) {
// TestResolveWorkspaceFilePath_RuntimeIndirection pins the
// `?root="/configs"` (or empty / unrecognized) → runtime managed-config
// dir behavior. Hermes uses /home/ubuntu/.hermes; claude-code uses
// /configs; unknowns fall back to /configs. This indirection is the
// reason hermes Config-tab edits land in the right place even though
// the canvas only ever sends `?root=/configs`. Changing it without a
// migration shim silently orphans previously-saved files.
func TestResolveWorkspaceFilePath_RuntimeIndirection(t *testing.T) {
cases := []struct {
runtime string
root string
relPath string
want string
}{
{"hermes", "config.yaml", "/home/ubuntu/.hermes/config.yaml"},
{"HERMES", "config.yaml", "/home/ubuntu/.hermes/config.yaml"}, // case-insensitive
{"hermes", "nested/a.yaml", "/home/ubuntu/.hermes/nested/a.yaml"},
{"hermes", "/configs", "config.yaml", "/home/ubuntu/.hermes/config.yaml"},
{"HERMES", "/configs", "config.yaml", "/home/ubuntu/.hermes/config.yaml"}, // case-insensitive
{"hermes", "/configs", "nested/a.yaml", "/home/ubuntu/.hermes/nested/a.yaml"},
{"hermes", "", "config.yaml", "/home/ubuntu/.hermes/config.yaml"}, // empty root → runtime indirection
{"hermes", "/etc", "config.yaml", "/home/ubuntu/.hermes/config.yaml"}, // out-of-allowlist → runtime indirection
// claude-code (and any future containerized runtime) lands at /configs —
// the path user-data creates and bind-mounts into the container. Pre-fix
// this fell through to /opt/configs which doesn't exist on workspace EC2s
// and would 500 with EACCES on save (the bug that motivated this gate).
{"claude-code", "config.yaml", "/configs/config.yaml"},
{"CLAUDE-CODE", "config.yaml", "/configs/config.yaml"}, // case-insensitive
{"langgraph", "config.yaml", "/opt/configs/config.yaml"},
{"external", "skills.json", "/opt/configs/skills.json"},
{"", "config.yaml", "/configs/config.yaml"}, // empty → default
{"unknown", "config.yaml", "/configs/config.yaml"}, // unknown → default
{"claude-code", "/configs", "config.yaml", "/configs/config.yaml"},
{"CLAUDE-CODE", "/configs", "config.yaml", "/configs/config.yaml"}, // case-insensitive
{"langgraph", "/configs", "config.yaml", "/opt/configs/config.yaml"},
{"external", "/configs", "skills.json", "/opt/configs/skills.json"},
{"", "/configs", "config.yaml", "/configs/config.yaml"}, // empty runtime → default
{"unknown", "/configs", "config.yaml", "/configs/config.yaml"}, // unknown → default
}
for _, tc := range cases {
t.Run(tc.runtime+"/"+tc.relPath, func(t *testing.T) {
got, err := resolveWorkspaceFilePath(tc.runtime, tc.relPath)
t.Run(tc.runtime+"+"+tc.root+"/"+tc.relPath, func(t *testing.T) {
got, err := resolveWorkspaceFilePath(tc.runtime, tc.root, tc.relPath)
if err != nil {
t.Fatalf("unexpected err: %v", err)
}
if got != tc.want {
t.Errorf("resolveWorkspaceFilePath(%q,%q) = %q, want %q",
tc.runtime, tc.relPath, got, tc.want)
t.Errorf("resolveWorkspaceFilePath(%q,%q,%q) = %q, want %q",
tc.runtime, tc.root, tc.relPath, got, tc.want)
}
})
}
}
// TestResolveWorkspaceFilePath_LiteralRoots pins that the universal
// allow-listed roots (`/home`, `/workspace`, `/plugins`) pass through
// LITERALLY rather than getting rewritten to the runtime prefix. This
// is the half of the resolver that the FilesTab "/home" selector
// depends on — without it, picking /home on a hermes workspace would
// route to /home/ubuntu/.hermes (the runtime indirection) and the
// canvas's tree row would never line up with what the user sees on
// the EC2 host.
func TestResolveWorkspaceFilePath_LiteralRoots(t *testing.T) {
cases := []struct {
runtime string
root string
relPath string
want string
}{
// /home is always literal regardless of runtime — it's a
// universal Linux path, not a managed-config indirection.
{"hermes", "/home", "ubuntu/.bashrc", "/home/ubuntu/.bashrc"},
{"claude-code", "/home", "ubuntu/notes.md", "/home/ubuntu/notes.md"},
{"langgraph", "/home", "ubuntu/x", "/home/ubuntu/x"},
// /workspace and /plugins are also literal — runtime is ignored.
{"hermes", "/workspace", "src/main.go", "/workspace/src/main.go"},
{"claude-code", "/plugins", "p/manifest.yaml", "/plugins/p/manifest.yaml"},
}
for _, tc := range cases {
t.Run(tc.runtime+"+"+tc.root+"/"+tc.relPath, func(t *testing.T) {
got, err := resolveWorkspaceFilePath(tc.runtime, tc.root, tc.relPath)
if err != nil {
t.Fatalf("unexpected err: %v", err)
}
if got != tc.want {
t.Errorf("resolveWorkspaceFilePath(%q,%q,%q) = %q, want %q",
tc.runtime, tc.root, tc.relPath, got, tc.want)
}
})
}
}
// TestResolveWorkspaceRootPath pins the directory-only translation
// used by listFilesViaEIC. Same indirection rules as
// resolveWorkspaceFilePath but without joining a relative path.
func TestResolveWorkspaceRootPath(t *testing.T) {
cases := []struct {
runtime string
root string
want string
}{
{"hermes", "/configs", "/home/ubuntu/.hermes"},
{"claude-code", "/configs", "/configs"},
{"hermes", "", "/home/ubuntu/.hermes"},
{"hermes", "/home", "/home"},
{"claude-code", "/workspace", "/workspace"},
{"hermes", "/plugins", "/plugins"},
{"unknown", "/configs", "/configs"},
{"hermes", "/etc", "/home/ubuntu/.hermes"}, // not allowlisted → runtime indirection
}
for _, tc := range cases {
t.Run(tc.runtime+"+"+tc.root, func(t *testing.T) {
got := resolveWorkspaceRootPath(tc.runtime, tc.root)
if got != tc.want {
t.Errorf("resolveWorkspaceRootPath(%q,%q) = %q, want %q",
tc.runtime, tc.root, got, tc.want)
}
})
}
@ -53,48 +126,80 @@ func TestResolveWorkspaceFilePath_KnownRuntimes(t *testing.T) {
// We only assert the cases that Clean() can't rescue.
func TestResolveWorkspaceFilePath_RejectsTraversal(t *testing.T) {
bad := []string{
"../etc/shadow", // escapes base via ..
"/etc/shadow", // absolute path
"./../../etc", // multiple ..
"a/../../etc", // escapes via deeper ..
"../etc/shadow", // escapes base via ..
"/etc/shadow", // absolute path
"./../../etc", // multiple ..
"a/../../etc", // escapes via deeper ..
}
for _, rel := range bad {
t.Run(rel, func(t *testing.T) {
_, err := resolveWorkspaceFilePath("hermes", rel)
_, err := resolveWorkspaceFilePath("hermes", "/configs", rel)
if err == nil {
t.Errorf("resolveWorkspaceFilePath(hermes, %q) should have errored, got nil", rel)
t.Errorf("resolveWorkspaceFilePath(hermes,/configs,%q) should have errored, got nil", rel)
}
})
}
}
// TestSSHArgs_LogLevelErrorBothSites pins that BOTH ssh invocations
// (writeFileViaEIC + readFileViaEIC) include `-o LogLevel=ERROR`.
// TestSSHArgs_HardenedFlags pins the ssh option set returned by
// eicSSHSession.sshArgs(). Centralising the args was deliberate so a
// fix like PR #2822's `LogLevel=ERROR` (silences the benign
// known-hosts warning that fooled the read/list "empty stderr → not
// found" classifier) only needs to land in one place.
//
// Without that flag, ssh emits a "Warning: Permanently added
// '[127.0.0.1]:NNNNN' (ED25519) to the list of known hosts." line on
// every fresh tunnel connection (even with UserKnownHostsFile=/dev/null
// — that prevents persistence, not the warning). The warning lands on
// stderr, which fools readFileViaEIC's "empty stdout + empty stderr →
// file not found" classifier into thinking the warning is a real
// ssh-layer error and returning 500 instead of 404.
//
// Caught 2026-05-05 02:38 on hongming.moleculesai.app: opening Hermes
// workspace's Config tab returned 500 with body
// Caught 2026-05-05 02:38 on hongming.moleculesai.app: opening
// Hermes workspace's Config tab returned 500 with body
// `ssh cat: exit status 1 (Warning: Permanently added '[127.0.0.1]:37951'…)`.
//
// LogLevel=ERROR silences info+warning while keeping real auth/tunnel
// errors visible. This test reads the source and asserts the flag
// appears at least twice (one per ssh block) — fires if a future edit
// removes it from either site.
func TestSSHArgs_LogLevelErrorBothSites(t *testing.T) {
// Asserts each load-bearing flag appears in the args slice — fires if
// a future edit removes any of them.
func TestSSHArgs_HardenedFlags(t *testing.T) {
s := eicSSHSession{keyPath: "/tmp/k", localPort: 12345, osUser: "ubuntu", instanceID: "i-x"}
got := s.sshArgs("echo hi")
wantFragments := [][]string{
{"-i", "/tmp/k"},
{"-o", "StrictHostKeyChecking=no"},
{"-o", "UserKnownHostsFile=/dev/null"},
{"-o", "LogLevel=ERROR"},
{"-o", "ServerAliveInterval=15"},
{"-p", "12345"},
}
joined := strings.Join(got, " ")
for _, frag := range wantFragments {
if !strings.Contains(joined, strings.Join(frag, " ")) {
t.Errorf("sshArgs() missing fragment %v; got: %v", frag, got)
}
}
// Last two args must be `<user>@127.0.0.1` then the remote command.
if got[len(got)-2] != "ubuntu@127.0.0.1" {
t.Errorf("sshArgs() second-last must be user@127.0.0.1; got: %q", got[len(got)-2])
}
if got[len(got)-1] != "echo hi" {
t.Errorf("sshArgs() last must be the remote command; got: %q", got[len(got)-1])
}
}
// TestEicSSHSessionSingleSourceForSSHFlags is a structural guard: the
// production EIC source must invoke s.sshArgs() exclusively for ssh
// invocations — direct ssh args inlined in any helper would re-open
// the regression that PR #2822 closed (LogLevel=ERROR drift between
// helpers). Counts `s.sshArgs(` occurrences (one per file op) and
// fails if anyone copy-pastes a raw ssh args slice.
func TestEicSSHSessionSingleSourceForSSHFlags(t *testing.T) {
src, err := os.ReadFile("template_files_eic.go")
if err != nil {
t.Fatalf("read source: %v", err)
}
matches := regexp.MustCompile(`"-o", "LogLevel=ERROR"`).FindAllIndex(src, -1)
if len(matches) < 2 {
t.Errorf("expected LogLevel=ERROR in BOTH ssh blocks (write + read); found %d occurrences", len(matches))
// Each of write/read/list/delete should call s.sshArgs once.
matches := regexp.MustCompile(`s\.sshArgs\(`).FindAllIndex(src, -1)
if len(matches) < 4 {
t.Errorf("expected ≥4 s.sshArgs() callers (write/read/list/delete); found %d", len(matches))
}
// Belt and braces: no helper should be assembling its own
// `LogLevel=ERROR` literal outside of sshArgs.
literal := regexp.MustCompile(`"-o", "LogLevel=ERROR"`).FindAllIndex(src, -1)
if len(literal) != 1 {
t.Errorf("LogLevel=ERROR must appear exactly once (in sshArgs); found %d occurrences — drift risk", len(literal))
}
}

View File

@ -216,7 +216,12 @@ func (h *TemplatesHandler) ReplaceFiles(c *gin.Context) {
// as a follow-up.
if instanceID != "" {
for relPath, content := range body.Files {
if err := writeFileViaEIC(ctx, instanceID, runtime, relPath, []byte(content)); err != nil {
// ReplaceFiles is a bulk template-import endpoint — files
// always land in the runtime's managed-config dir. Pass
// "/configs" so resolveWorkspaceFilePath routes through the
// runtime prefix map (matches the local-Docker arm below
// which always copies to /configs).
if err := writeFileViaEIC(ctx, instanceID, runtime, "/configs", relPath, []byte(content)); err != nil {
log.Printf("ReplaceFiles EIC for %s path=%s: %v", workspaceID, relPath, err)
c.JSON(http.StatusInternalServerError, gin.H{"error": fmt.Sprintf("failed to write file %s: %v", relPath, err)})
return

View File

@ -243,8 +243,11 @@ func (h *TemplatesHandler) ListFiles(c *gin.Context) {
listPath = rootPath + "/" + subPath
}
var wsName string
if err := db.DB.QueryRowContext(ctx, `SELECT name FROM workspaces WHERE id = $1`, workspaceID).Scan(&wsName); err != nil {
var wsName, instanceID, runtime string
if err := db.DB.QueryRowContext(ctx,
`SELECT name, COALESCE(instance_id, ''), COALESCE(runtime, '') FROM workspaces WHERE id = $1`,
workspaceID,
).Scan(&wsName, &instanceID, &runtime); err != nil {
c.JSON(http.StatusNotFound, gin.H{"error": "workspace not found"})
return
}
@ -255,6 +258,32 @@ func (h *TemplatesHandler) ListFiles(c *gin.Context) {
Dir bool `json:"dir"`
}
// SaaS workspace (EC2-per-workspace) — no Docker on this tenant. List
// via SSH through the EIC endpoint, mirroring ReadFile/WriteFile's
// dispatch. Pre-fix this branch was missing and SaaS workspaces
// always fell through to local-Docker check (finds nothing on a SaaS
// tenant) + template-dir fallback (returns the seed template, not
// the persisted state, and almost never matches on user-named
// workspaces). Net effect: the canvas Files tab always rendered "0
// files / No config files yet" for SaaS workspaces, regardless of
// what was actually on disk. See issue #2999.
if instanceID != "" {
entries, err := listFilesViaEIC(ctx, instanceID, runtime, rootPath, subPath, depth)
if err != nil {
log.Printf("ListFiles EIC for %s root=%s sub=%s: %v", workspaceID, rootPath, subPath, err)
c.JSON(http.StatusInternalServerError, gin.H{"error": fmt.Sprintf("failed to list files: %v", err)})
return
}
// Translate to the handler's wire shape (the field names match
// 1:1, but Go can't implicit-convert named struct types).
out := make([]fileEntry, 0, len(entries))
for _, e := range entries {
out = append(out, fileEntry{Path: e.Path, Size: e.Size, Dir: e.Dir})
}
c.JSON(http.StatusOK, out)
return
}
// Try container filesystem first
if containerName := h.findContainer(ctx, workspaceID); containerName != "" {
// Portable file listing: works on both GNU and BusyBox/Alpine.
@ -378,12 +407,13 @@ func (h *TemplatesHandler) ReadFile(c *gin.Context) {
// canvas Config tab always 404'd for SaaS workspaces — visible to
// users after #2781 added the "no config.yaml" error UX.
//
// The ?root= query param is intentionally ignored on the SaaS path —
// it's a local-Docker concept (arbitrary container roots). The
// runtime → base-path map (workspaceFilePathPrefix in
// template_files_eic.go) is the SaaS source of truth.
// `?root=` flows through resolveWorkspaceFilePath: "/configs" stays
// the per-runtime managed-config indirection (claude-code → /configs,
// hermes → /home/ubuntu/.hermes); other allow-listed roots
// (`/home`, `/workspace`, `/plugins`) pass through literally so
// list/read/write/delete agree on what file a tree row points to.
if instanceID != "" {
content, err := readFileViaEIC(ctx, instanceID, runtime, filePath)
content, err := readFileViaEIC(ctx, instanceID, runtime, rootPath, filePath)
if err == nil {
c.JSON(http.StatusOK, gin.H{
"path": filePath,
@ -468,6 +498,11 @@ func (h *TemplatesHandler) WriteFile(c *gin.Context) {
}
ctx := c.Request.Context()
rootPath := c.DefaultQuery("root", "/configs")
if !allowedRoots[rootPath] {
c.JSON(http.StatusBadRequest, gin.H{"error": "root must be one of: /configs, /workspace, /home, /plugins"})
return
}
var wsName, instanceID, runtime string
if err := db.DB.QueryRowContext(ctx,
`SELECT name, COALESCE(instance_id, ''), COALESCE(runtime, '') FROM workspaces WHERE id = $1`,
@ -479,8 +514,11 @@ func (h *TemplatesHandler) WriteFile(c *gin.Context) {
// SaaS workspace (EC2-per-workspace) — no Docker on this tenant. Write
// via SSH through the EIC endpoint to the runtime-specific path.
// `?root=` flows through the same per-runtime / literal indirection
// as ReadFile so list/read/write/delete agree on what file a tree
// row points to.
if instanceID != "" {
if err := writeFileViaEIC(ctx, instanceID, runtime, filePath, []byte(body.Content)); err != nil {
if err := writeFileViaEIC(ctx, instanceID, runtime, rootPath, filePath, []byte(body.Content)); err != nil {
log.Printf("WriteFile EIC for %s path=%s: %v", workspaceID, filePath, err)
c.JSON(http.StatusInternalServerError, gin.H{"error": fmt.Sprintf("failed to write file: %v", err)})
return
@ -528,12 +566,35 @@ func (h *TemplatesHandler) DeleteFile(c *gin.Context) {
}
ctx := c.Request.Context()
var wsName string
if err := db.DB.QueryRowContext(ctx, `SELECT name FROM workspaces WHERE id = $1`, workspaceID).Scan(&wsName); err != nil {
rootPath := c.DefaultQuery("root", "/configs")
if !allowedRoots[rootPath] {
c.JSON(http.StatusBadRequest, gin.H{"error": "root must be one of: /configs, /workspace, /home, /plugins"})
return
}
var wsName, instanceID, runtime string
if err := db.DB.QueryRowContext(ctx,
`SELECT name, COALESCE(instance_id, ''), COALESCE(runtime, '') FROM workspaces WHERE id = $1`,
workspaceID,
).Scan(&wsName, &instanceID, &runtime); err != nil {
c.JSON(http.StatusNotFound, gin.H{"error": "workspace not found"})
return
}
// SaaS workspace (EC2-per-workspace) — no Docker on this tenant. Delete
// via SSH through the EIC endpoint, mirroring ReadFile/WriteFile's
// dispatch. Pre-fix this branch was missing — DeleteFile fell through
// to local-Docker (no container) + ephemeral-volume (no Docker) and
// silently 500'd. See issue #2999.
if instanceID != "" {
if err := deleteFileViaEIC(ctx, instanceID, runtime, rootPath, filePath); err != nil {
log.Printf("DeleteFile EIC for %s root=%s path=%s: %v", workspaceID, rootPath, filePath, err)
c.JSON(http.StatusInternalServerError, gin.H{"error": fmt.Sprintf("failed to delete file: %v", err)})
return
}
c.JSON(http.StatusOK, gin.H{"status": "deleted", "path": filePath})
return
}
// Delete via docker exec when container is running
if containerName := h.findContainer(ctx, workspaceID); containerName != "" {
// CWE-78: use filepath.Join instead of string concat to prevent path

View File

@ -750,7 +750,11 @@ func TestListFiles_WorkspaceNotFound(t *testing.T) {
handler := NewTemplatesHandler(t.TempDir(), nil, nil)
mock.ExpectQuery("SELECT name FROM workspaces WHERE id =").
// SQL shape: SELECT name, COALESCE(instance_id, ''), COALESCE(runtime, '') FROM workspaces WHERE id = $1
// (matches the L/R/W/D unified shape so dispatchers can branch on
// instance_id; sqlmock matches via QueryMatcherRegexp so the parens
// need escaping.)
mock.ExpectQuery(`SELECT name, COALESCE\(instance_id, ''\), COALESCE\(runtime, ''\) FROM workspaces WHERE id =`).
WithArgs("ws-nonexist").
WillReturnError(sql.ErrNoRows)
@ -777,9 +781,9 @@ func TestListFiles_FallbackToHost_NoTemplate(t *testing.T) {
tmpDir := t.TempDir()
handler := NewTemplatesHandler(tmpDir, nil, nil) // nil docker = no container
mock.ExpectQuery("SELECT name FROM workspaces WHERE id =").
mock.ExpectQuery(`SELECT name, COALESCE\(instance_id, ''\), COALESCE\(runtime, ''\) FROM workspaces WHERE id =`).
WithArgs("ws-fallback").
WillReturnRows(sqlmock.NewRows([]string{"name"}).AddRow("Unknown Agent"))
WillReturnRows(sqlmock.NewRows([]string{"name", "instance_id", "runtime"}).AddRow("Unknown Agent", "", ""))
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
@ -817,9 +821,9 @@ func TestListFiles_FallbackToHost_WithTemplate(t *testing.T) {
handler := NewTemplatesHandler(tmpDir, nil, nil)
mock.ExpectQuery("SELECT name FROM workspaces WHERE id =").
mock.ExpectQuery(`SELECT name, COALESCE\(instance_id, ''\), COALESCE\(runtime, ''\) FROM workspaces WHERE id =`).
WithArgs("ws-tmpl").
WillReturnRows(sqlmock.NewRows([]string{"name"}).AddRow("Test Agent"))
WillReturnRows(sqlmock.NewRows([]string{"name", "instance_id", "runtime"}).AddRow("Test Agent", "", ""))
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
@ -1103,7 +1107,7 @@ func TestDeleteFile_WorkspaceNotFound(t *testing.T) {
handler := NewTemplatesHandler(t.TempDir(), nil, nil)
mock.ExpectQuery("SELECT name FROM workspaces WHERE id =").
mock.ExpectQuery(`SELECT name, COALESCE\(instance_id, ''\), COALESCE\(runtime, ''\) FROM workspaces WHERE id =`).
WithArgs("ws-del-nf").
WillReturnError(sql.ErrNoRows)