mirror of
https://github.com/openswarm-ai/openswarm.git
synced 2026-10-01 05:54:56 +02:00
[eric] outputs: HITL preview for risky backend.py + workspace path-traversal hardening
This commit is contained in:
ciregenz
@@ -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`).
|
||||
|
||||
@@ -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"
|
||||
|
||||
@@ -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):
|
||||
|
||||
@@ -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()
|
||||
|
||||
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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`.
|
||||
|
||||
@@ -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<Props> = ({ output, onClose }) => {
|
||||
const [result, setResult] = useState<OutputExecuteResult | null>(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<Props> = ({ output, onClose }) => {
|
||||
}
|
||||
};
|
||||
|
||||
const handleRun = () => runWith(false);
|
||||
const handleRunAnyway = () => runWith(true);
|
||||
|
||||
return (
|
||||
<Dialog open onClose={onClose} maxWidth="lg" fullWidth>
|
||||
<DialogTitle sx={{ fontWeight: 600, color: c.text.primary }}>
|
||||
@@ -107,7 +114,47 @@ const ViewRunDialog: React.FC<Props> = ({ output, onClose }) => {
|
||||
<CircularProgress size={28} />
|
||||
</Box>
|
||||
)}
|
||||
{result ? (
|
||||
{warnings && codePreview ? (
|
||||
<Box sx={{ p: 2, overflow: 'auto', height: '100%', display: 'flex', flexDirection: 'column', gap: 1.5 }}>
|
||||
<Box sx={{ display: 'flex', alignItems: 'center', gap: 1 }}>
|
||||
<WarningAmberIcon sx={{ color: c.status.warning, fontSize: 22 }} />
|
||||
<Typography sx={{ fontSize: '0.95rem', fontWeight: 600, color: c.text.primary }}>
|
||||
Review before running
|
||||
</Typography>
|
||||
</Box>
|
||||
<Typography sx={{ fontSize: '0.8rem', color: c.text.secondary }}>
|
||||
This Output's backend code does things outside the safe
|
||||
data-shaping allowlist. Read it and decide whether to run.
|
||||
</Typography>
|
||||
<Box
|
||||
component="ul"
|
||||
sx={{ m: 0, pl: 2.5, color: c.text.secondary, fontSize: '0.78rem', lineHeight: 1.55 }}
|
||||
>
|
||||
{warnings.map((w, i) => (
|
||||
<li key={i}>{w}</li>
|
||||
))}
|
||||
</Box>
|
||||
<Box
|
||||
sx={{
|
||||
flex: 1,
|
||||
minHeight: 120,
|
||||
mt: 0.5,
|
||||
p: 1.25,
|
||||
borderRadius: 1,
|
||||
border: `1px solid ${c.border.subtle}`,
|
||||
bgcolor: c.bg.secondary,
|
||||
fontFamily: 'ui-monospace, SFMono-Regular, Menlo, monospace',
|
||||
fontSize: '0.74rem',
|
||||
lineHeight: 1.5,
|
||||
color: c.text.primary,
|
||||
whiteSpace: 'pre',
|
||||
overflow: 'auto',
|
||||
}}
|
||||
>
|
||||
{codePreview}
|
||||
</Box>
|
||||
</Box>
|
||||
) : result ? (
|
||||
<ViewPreview
|
||||
serveUrl={`${SERVE_BASE}/${output.id}/serve/index.html`}
|
||||
frontendCode={result.frontend_code}
|
||||
@@ -127,14 +174,25 @@ const ViewRunDialog: React.FC<Props> = ({ output, onClose }) => {
|
||||
</DialogContent>
|
||||
<DialogActions sx={{ px: 3, pb: 2 }}>
|
||||
<Button onClick={onClose} sx={{ color: c.text.muted }}>Close</Button>
|
||||
<Button
|
||||
variant="contained"
|
||||
onClick={handleRun}
|
||||
disabled={running}
|
||||
sx={{ bgcolor: c.accent.primary, '&:hover': { bgcolor: c.accent.hover } }}
|
||||
>
|
||||
{running ? 'Running...' : getBackendCode(output) ? 'Execute & Preview' : 'Preview'}
|
||||
</Button>
|
||||
{warnings ? (
|
||||
<Button
|
||||
variant="contained"
|
||||
onClick={handleRunAnyway}
|
||||
disabled={running}
|
||||
sx={{ bgcolor: c.status.warning, '&:hover': { bgcolor: c.status.warning } }}
|
||||
>
|
||||
{running ? 'Running...' : 'Run anyway'}
|
||||
</Button>
|
||||
) : (
|
||||
<Button
|
||||
variant="contained"
|
||||
onClick={handleRun}
|
||||
disabled={running}
|
||||
sx={{ bgcolor: c.accent.primary, '&:hover': { bgcolor: c.accent.hover } }}
|
||||
>
|
||||
{running ? 'Running...' : getBackendCode(output) ? 'Execute & Preview' : 'Preview'}
|
||||
</Button>
|
||||
)}
|
||||
</DialogActions>
|
||||
</Dialog>
|
||||
);
|
||||
|
||||
@@ -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<string, any> }) => {
|
||||
// `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<string, any>; force?: boolean }) => {
|
||||
const res = await fetch(`${OUTPUTS_API}/execute`, {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
|
||||
Reference in New Issue
Block a user