diff --git a/backend/CLAUDE.md b/backend/CLAUDE.md index edd44a1a..1260c8e0 100644 --- a/backend/CLAUDE.md +++ b/backend/CLAUDE.md @@ -2,6 +2,10 @@ FastAPI orchestrator. Entry: `backend/main.py` (uvicorn `:8324`, REST `/api/*`, WS `/ws/*`, Swagger `/docs`). See root `CLAUDE.md` for repo-wide constraints. +## Coding precedences + +Full precedences live in root [CLAUDE.md](../.claude/CLAUDE.md). Always: **understand the end goal before coding** (what does the user actually need?); **reuse before you write** (grep existing routes / SubApps / helpers — most needs already have one); ~300 LOC/file ceiling; downward-tree imports; no comments except WHY-non-obvious; test after meaningful changes; weigh speed, efficiency, robustness, UX, security on every change. + ## Run / test - Dev: `bash backend/run.sh` (creates `.venv/`, installs `requirements.txt`, runs uvicorn with `--reload`). diff --git a/backend/apps/outputs/executor.py b/backend/apps/outputs/executor.py index 20e919d7..eca24320 100644 --- a/backend/apps/outputs/executor.py +++ b/backend/apps/outputs/executor.py @@ -39,33 +39,61 @@ class UnsafeCodeError(Exception): """Raised when AST validation rejects user-supplied backend code.""" -def _validate_code_safety(code: str) -> None: +def get_code_warnings(code: str) -> list[str]: + """Return human-readable warnings for AST-visible risks, without raising. + + Used by `/api/outputs/execute` to surface risks to the user in the run + dialog before executing — so a legit Output that needs `pandas` doesn't + silently 500 with "import not allowed," it gets a "this Output uses + unsafe imports — review and click Run Anyway" affordance. + + Returns [] for code that's fully inside the allowlist. A syntax error + is reported as a single warning rather than raised so the dialog can + show it next to the code. + """ try: tree = ast.parse(code) except SyntaxError as e: - raise UnsafeCodeError(f"Backend code has a syntax error: {e}") + return [f"Syntax error: {e}"] + warnings: list[str] = [] + seen: set[str] = set() for node in ast.walk(tree): if isinstance(node, ast.Import): for alias in node.names: root = alias.name.split(".")[0] if root not in _ALLOWED_MODULES: - raise UnsafeCodeError( - f"import of '{alias.name}' is not allowed in backend code " - f"(allowed: {sorted(_ALLOWED_MODULES)})" - ) + msg = f"Imports '{alias.name}' (outside the safe-data-shaping allowlist)" + if msg not in seen: + seen.add(msg) + warnings.append(msg) elif isinstance(node, ast.ImportFrom): if node.module: root = node.module.split(".")[0] if root not in _ALLOWED_MODULES: - raise UnsafeCodeError( - f"from '{node.module}' import ... is not allowed in backend code" - ) + msg = f"Imports from '{node.module}' (outside the safe-data-shaping allowlist)" + if msg not in seen: + seen.add(msg) + warnings.append(msg) elif isinstance(node, ast.Call): if isinstance(node.func, ast.Name) and node.func.id in _BLOCKED_BUILTINS: - raise UnsafeCodeError( - f"call to builtin '{node.func.id}()' is not allowed in backend code" - ) + msg = f"Calls builtin '{node.func.id}()' which can escape the sandbox" + if msg not in seen: + seen.add(msg) + warnings.append(msg) + return warnings + + +def _validate_code_safety(code: str) -> None: + """Raise UnsafeCodeError on the first AST-visible risk. Thin wrapper + around get_code_warnings for callers that want the strict-reject + behavior (the default `execute_backend_code` path). Callers that want + to show warnings to a user and let them override should call + get_code_warnings directly and pass `skip_validation=True` to + execute_backend_code.""" + warnings = get_code_warnings(code) + if warnings: + raise UnsafeCodeError(warnings[0]) def _minimal_env() -> dict: @@ -95,7 +123,9 @@ class BackendExecResult: stderr: str -async def execute_backend_code(code: str, input_data: dict) -> BackendExecResult: +async def execute_backend_code( + code: str, input_data: dict, *, skip_validation: bool = False +) -> BackendExecResult: """Execute user-provided Python code in a subprocess. The code receives ``input_data`` as a global dict and must assign its @@ -109,9 +139,14 @@ async def execute_backend_code(code: str, input_data: dict) -> BackendExecResult 4. Preamble scrubs dangerous attrs off `builtins` inside the subprocess to catch AST-bypass tricks (e.g. metaclass shenanigans). 5. 30s wall-clock timeout, killed on overrun. + + `skip_validation=True` bypasses #1 — intended ONLY for callers that + have already surfaced the warnings to a user and gotten explicit + consent (the `/api/outputs/execute` HITL flow). #2–#5 always run. """ - _validate_code_safety(code) + if not skip_validation: + _validate_code_safety(code) preamble = ( "import json, sys, io, builtins\n" diff --git a/backend/apps/outputs/models.py b/backend/apps/outputs/models.py index 09878160..b0ed9eec 100644 --- a/backend/apps/outputs/models.py +++ b/backend/apps/outputs/models.py @@ -123,6 +123,14 @@ class OutputUpdate(BaseModel): class OutputExecute(BaseModel): output_id: str input_data: dict[str, Any] = Field(default_factory=dict) + # When False (default), `/execute` returns AST warnings instead of + # running if the backend code touches anything outside the safe + # data-shaping allowlist. The UI shows those warnings to the user and + # re-submits with force=True after they click "Run Anyway." This is + # a UX gate, not a security one — anyone holding the auth token can + # set force=True; the value is providing the user explicit visibility + # of what's about to execute. + force: bool = False class OutputExecuteResult(BaseModel): @@ -134,6 +142,11 @@ class OutputExecuteResult(BaseModel): stdout: Optional[str] = None stderr: Optional[str] = None error: Optional[str] = None + # Populated when the AST validator flagged risky constructs and the + # caller didn't set force=True. When present, `backend_result` is null + # because execution was deferred pending user consent. + warnings: Optional[list[str]] = None + code_preview: Optional[str] = None class WorkspaceSeedRequest(BaseModel): diff --git a/backend/apps/outputs/outputs.py b/backend/apps/outputs/outputs.py index 9910b141..15f5b369 100644 --- a/backend/apps/outputs/outputs.py +++ b/backend/apps/outputs/outputs.py @@ -16,7 +16,7 @@ from backend.apps.outputs.models import ( Output, OutputCreate, OutputUpdate, OutputExecute, OutputExecuteResult, VibeCodeRequest, WorkspaceSeedRequest, ) -from backend.apps.outputs.executor import execute_backend_code +from backend.apps.outputs.executor import execute_backend_code, get_code_warnings from backend.apps.outputs.view_builder_templates import ( VIEW_TEMPLATE_FILES, load_app_builder_skill, @@ -586,8 +586,14 @@ async def write_workspace_file(workspace_id: str, filepath: str, body: dict): folder = os.path.join(WORKSPACE_DIR, workspace_id) if not os.path.isdir(folder): raise HTTPException(status_code=404, detail="Workspace not found") + folder_norm = os.path.normpath(folder) full_path = os.path.normpath(os.path.join(folder, filepath)) - if not full_path.startswith(os.path.normpath(folder)): + # `startswith(folder_norm + os.sep)` (not just folder_norm) so a workspace + # `abc-123` can't be tricked into writing into a sibling `abc-1234-evil` — + # prefix-string collision rather than path-component containment. Today's + # UUID-format ids make the collision unlikely in practice, but the check + # is one character and immunizes future id schemes. + if full_path != folder_norm and not full_path.startswith(folder_norm + os.sep): raise HTTPException(status_code=403, detail="Path traversal not allowed") os.makedirs(os.path.dirname(full_path), exist_ok=True) with open(full_path, "w") as f: @@ -601,8 +607,9 @@ async def delete_workspace_file(workspace_id: str, filepath: str): folder = os.path.join(WORKSPACE_DIR, workspace_id) if not os.path.isdir(folder): raise HTTPException(status_code=404, detail="Workspace not found") + folder_norm = os.path.normpath(folder) full_path = os.path.normpath(os.path.join(folder, filepath)) - if not full_path.startswith(os.path.normpath(folder)): + if full_path != folder_norm and not full_path.startswith(folder_norm + os.sep): raise HTTPException(status_code=403, detail="Path traversal not allowed") if os.path.isfile(full_path): os.remove(full_path) @@ -782,16 +789,33 @@ async def execute_output(body: OutputExecute): stdout_text = None stderr_text = None error = None + warnings_out: Optional[list[str]] = None + code_preview: Optional[str] = None if output.backend_code: - try: - exec_result = await execute_backend_code( - output.backend_code, body.input_data - ) - backend_result = exec_result.result - stdout_text = exec_result.stdout - stderr_text = exec_result.stderr - except Exception as e: - error = str(e) + # HITL gate: collect warnings up front. If the caller hasn't opted + # in via force=True AND the code touches anything outside the safe + # allowlist, return the warnings + the code itself so the UI can + # show a preview dialog. No subprocess is spawned on this path — + # zero-cost when warnings exist, identical-to-before when they + # don't. + if not body.force: + warnings_out = get_code_warnings(output.backend_code) + if warnings_out: + code_preview = output.backend_code + if not warnings_out: + try: + # We've either already vetted (no warnings above) or the + # user explicitly opted in with force=True. Pass + # skip_validation=True so we don't pay for a redundant + # AST walk inside execute_backend_code. + exec_result = await execute_backend_code( + output.backend_code, body.input_data, skip_validation=True + ) + backend_result = exec_result.result + stdout_text = exec_result.stdout + stderr_text = exec_result.stderr + except Exception as e: + error = str(e) return OutputExecuteResult( output_id=output.id, @@ -802,6 +826,8 @@ async def execute_output(body: OutputExecute): stdout=stdout_text, stderr=stderr_text, error=error, + warnings=warnings_out if warnings_out else None, + code_preview=code_preview, ).model_dump() diff --git a/electron/CLAUDE.md b/electron/CLAUDE.md index 7ddf0cf4..44024ea0 100644 --- a/electron/CLAUDE.md +++ b/electron/CLAUDE.md @@ -2,6 +2,10 @@ Electron 40.x (CastLabs DRM build) desktop shell + auto-updater via GitHub Releases. Entry: `main.js`. Version is in `package.json`. See root `CLAUDE.md` for repo-wide constraints. +## Coding precedences + +Full precedences live in root [CLAUDE.md](../.claude/CLAUDE.md). Always: **understand the end goal before coding** (what does the user actually need?); **reuse before you write** (grep existing IPC handlers / helpers in `main.js` — most needs already have one); ~300 LOC/file ceiling; downward-tree imports; no comments except WHY-non-obvious; test the packaged build path after meaningful changes (not just dev); weigh speed (startup time), efficiency (memory), robustness (auto-updater, OAuth windows), UX, security (signed binaries, no plaintext secrets) on every change. + ## Build / release - Local build: `npm run build` → produces `build-staging/` containing the frontend dist, backend bundle, standalone Python 3.13, and the 9router binary. Build artifacts are ephemeral; not git-tracked. diff --git a/frontend/CLAUDE.md b/frontend/CLAUDE.md index 1442801a..5c628864 100644 --- a/frontend/CLAUDE.md +++ b/frontend/CLAUDE.md @@ -2,6 +2,10 @@ React 18 + TypeScript + webpack 5 + Redux. Entry: `src/app/Main.tsx`. Dev server on `:3000` proxies REST and WebSocket to backend on `:8324`. See root `CLAUDE.md` for repo-wide constraints. +## Coding precedences + +Full precedences live in root [CLAUDE.md](../.claude/CLAUDE.md). Always: **understand the end goal before coding** (what does the user actually need?); **reuse before you write** (grep existing components / hooks / Redux slices — most needs already have one); ~300 LOC/file ceiling; downward-tree imports (`shared/` → `app/components/` → `pages/`); no comments except WHY-non-obvious; manually exercise the UI after meaningful changes; weigh speed (no double renders), efficiency, robustness, UX (loading/error/animation states), security on every change. + ## Run - Dev (full stack): `bash run.sh`. diff --git a/frontend/src/app/pages/Views/ViewRunDialog.tsx b/frontend/src/app/pages/Views/ViewRunDialog.tsx index adacfa16..28f43e84 100644 --- a/frontend/src/app/pages/Views/ViewRunDialog.tsx +++ b/frontend/src/app/pages/Views/ViewRunDialog.tsx @@ -7,6 +7,7 @@ import Button from '@mui/material/Button'; import Box from '@mui/material/Box'; import Typography from '@mui/material/Typography'; import CircularProgress from '@mui/material/CircularProgress'; +import WarningAmberIcon from '@mui/icons-material/WarningAmber'; import { Output, executeOutput, OutputExecuteResult, getFrontendCode, getBackendCode, buildServeUrl, SERVE_BASE } from '@/shared/state/outputsSlice'; import { useAppDispatch } from '@/shared/hooks'; import { useClaudeTokens } from '@/shared/styles/ThemeContext'; @@ -27,11 +28,14 @@ const ViewRunDialog: React.FC = ({ output, onClose }) => { const [result, setResult] = useState(null); const [running, setRunning] = useState(false); - const handleRun = async () => { + const warnings = result?.warnings && result.warnings.length > 0 ? result.warnings : null; + const codePreview = result?.code_preview || null; + + const runWith = async (force: boolean) => { setRunning(true); try { const res = await dispatch( - executeOutput({ output_id: output.id, input_data: inputData }) + executeOutput({ output_id: output.id, input_data: inputData, force }) ).unwrap(); setResult(res); } finally { @@ -39,6 +43,9 @@ const ViewRunDialog: React.FC = ({ output, onClose }) => { } }; + const handleRun = () => runWith(false); + const handleRunAnyway = () => runWith(true); + return ( @@ -107,7 +114,47 @@ const ViewRunDialog: React.FC = ({ output, onClose }) => { )} - {result ? ( + {warnings && codePreview ? ( + + + + + Review before running + + + + This Output's backend code does things outside the safe + data-shaping allowlist. Read it and decide whether to run. + + + {warnings.map((w, i) => ( +
  • {w}
    • + ))} +     + + {codePreview} + +     + ) : result ? ( = ({ output, onClose }) => { - + {warnings ? ( + + ) : ( + + )}     ); diff --git a/frontend/src/shared/state/outputsSlice.ts b/frontend/src/shared/state/outputsSlice.ts index 5837208a..8901416f 100644 --- a/frontend/src/shared/state/outputsSlice.ts +++ b/frontend/src/shared/state/outputsSlice.ts @@ -60,6 +60,12 @@ export interface OutputExecuteResult { stdout: string | null; stderr: string | null; error: string | null; + // Present when the backend AST validator flagged risky imports/calls and + // the caller didn't pass force=true. UI shows these alongside `code_preview` + // in a "review and Run Anyway" dialog; resubmitting with force:true bypasses + // the gate. Absent (undefined) on the happy path. + warnings?: string[] | null; + code_preview?: string | null; } interface OutputsState { @@ -115,7 +121,9 @@ export const deleteOutput = createAsyncThunk('outputs/delete', async (id: string export const executeOutput = createAsyncThunk( 'outputs/execute', - async (body: { output_id: string; input_data: Record }) => { + // `force` opts past the AST warnings gate — only set after the user has + // seen the code preview in the run dialog and clicked Run Anyway. + async (body: { output_id: string; input_data: Record; force?: boolean }) => { const res = await fetch(`${OUTPUTS_API}/execute`, { method: 'POST', headers: { 'Content-Type': 'application/json' },