fix(workflow-code): parse code via AST allow-list instead of exec()

[sebastionoss]

Summary

WorkflowCodeHandler.parse_code() executes user-submitted Python via exec() when a request hits the /api/code_convert/* endpoints. Because the backend has no authentication layer, this is unauthenticated remote code execution on the host running PySpur.

• CWE-95: Improper neutralization of directives in dynamically evaluated code (Eval Injection)
• Affected file: backend/pyspur/workflow_code_handler.py — WorkflowCodeHandler.parse_code()
• Reached from: backend/pyspur/api/workflow_code_convert.py at three routes:
  • POST /api/code_convert/create_from_code (line 111)
  • PUT /api/code_convert/{workflow_id} (line 199)
  • the parse route at line 277
• Severity: Critical (unauthenticated RCE)

Data flow

1. HTTP client posts JSON { "code": "..." } to /api/code_convert/create_from_code.
2. FastAPI deserializes into a Pydantic model and calls WorkflowCodeHandler.parse_code(request.code).
3. parse_code previously called exec(code, {"WorkflowBuilder": WorkflowBuilder}, local_vars) on the raw string.

The {"WorkflowBuilder": WorkflowBuilder} globals dict is not a sandbox — Python builtins remain reachable through object traversal (e.g. ().__class__.__mro__[1].__subclasses__()), and __import__ / open / os are trivially recoverable inside exec.

I checked the routing layer for auth gates before submitting: api_app.include_router(workflow_code_router, prefix="/code_convert", tags=["workflow code (beta)"]) has no dependencies=[...], there is no app-wide auth middleware, and CORS is allow_origins=["*"] with allow_credentials=True. Every route on this app is reachable without credentials.

Fix

Replace exec() with a strict AST allow-list walker. The parser now:

• Parses the submitted code with ast.parse and walks the tree explicitly.
• Allows only:
  • The single WorkflowBuilder import.
  • Assignments whose RHS is a call on a WorkflowBuilder() instance or a chained method call on it.
  • Literal arguments (ast.Constant), name references to previously-bound locals, and simple containers (list/tuple/dict of literals).
• Rejects: For, While, FunctionDef, ClassDef, Lambda, Import of anything else, attribute chains that escape the builder namespace, dunder attribute access, and any reference to os, sys, __import__, open, eval, exec, compile, globals, locals, getattr, setattr.
• Executes the reconstructed calls against a real WorkflowBuilder instance via normal Python attribute lookup — no exec, no eval.

Only the one handler file changed (185 insertions, 22 deletions). No routes, models, or callers were altered — behavior for well-formed workflow definitions is preserved.

Proof of concept

Against a default docker-compose up PySpur deployment (backend on :8000):

curl -X POST http://TARGET:8000/api/code_convert/create_from_code \
  -H 'Content-Type: application/json' \
  -d '{
    "code": "import os\nos.system(\"id > /tmp/pwned\")\nbuilder = WorkflowBuilder()\n",
    "name": "poc"
  }'

Before the fix: /tmp/pwned appears on the server containing the output of id — arbitrary shell command execution as the backend process user.

A sandbox-escape variant that does not use a top-level import (to show the old "restricted globals" argument didn't hold):

{
  "code": "builder = WorkflowBuilder()\n[c for c in ().__class__.__mro__[1].__subclasses__() if c.__name__=='Popen'][0](['id'])\n",
  "name": "poc2"
}

After the fix both requests are rejected at parse time with a ValueError describing the disallowed construct (import os, subscript/attribute traversal, list comprehension, etc.), and no execution occurs.

Testing

• Ran the existing handler test suite: 9/9 pass.
• Added negative cases covering: import os, __import__, open(...), for loop, def inside submitted code, lambda, attribute traversal through __class__, and getattr(builder, "…"). All rejected.
• Positive cases: representative WorkflowBuilder().add_node(...).add_edge(...).build() chains parse and produce equivalent workflow_def objects to the previous implementation.

Adversarial review

Before submitting I tried to disprove this: I checked whether a router-level Depends, an app-wide auth middleware, a reverse-proxy assumption in the docs, or a feature flag gating /code_convert might mean the endpoint isn't actually reachable by an unauthenticated attacker in real deployments. None of those exist — the api_app mounts the router without dependencies, there is no auth middleware, CORS is fully permissive, and the docker-compose quickstart in the README exposes the backend directly. I also checked whether the "restricted globals" pattern in the old code offered meaningful protection and confirmed it does not, since builtins remain reachable via object-graph traversal inside exec.

---

_{Discovered by the Sebastion AI GitHub App.}

[rajeev]

@sebastiondev there is similar code in backend/pyspur/nodes/python/python_func.py. Can you update this PR to include that as well?

[sebastionoss]

Thanks @rajeev — you're right that backend/pyspur/nodes/python/python_func.py also passes user-controlled strings to exec(), and it's reachable via the same unauthenticated API surface (a workflow that contains a PythonFuncNode will exec config.code at run time).

Before I push a change, one design difference I want to flag so we agree on the fix — the two call sites aren't quite the same problem:

• workflow_code_handler.parse_code (this PR): the input is workflow-builder DSL that the backend itself generates — a small allow-list of builder.add_node(...), builder.add_link(...), literals, etc. An AST allow-list works cleanly because no legitimate caller ever needs import os or a for loop there.
• PythonFuncNode.run: the input is arbitrary user Python by design. The whole point of the node is "run this Python on the inputs and return a dict." An AST allow-list can't be applied without removing the feature.

So the meaningful mitigations for python_func.py are different:

1. Sandboxed execution — RestrictedPython, a subprocess with dropped privileges / seccomp, or an external runner (nsjail, gVisor, Firecracker). This is the correct fix but it's a substantial change and needs a policy decision from you about the target isolation level.
2. Auth gate — require an authenticated user with an explicit python:execute capability before a workflow containing a PythonFuncNode can be run, i.e. treat the node as an admin-only primitive. Cheap, doesn't need a sandbox, but depends on the auth layer you want to adopt.
3. In-process hardening only — dropping __builtins__, stripping dangerous names, etc. I want to be honest that this is not a real fix: builtins remain reachable through object-graph traversal (().__class__.__mro__[1].__subclasses__()), so it only stops the laziest payloads and can create a false sense of safety.

I'd rather not ship (3) alone in this PR. My preference is to keep this PR focused on the workflow_code_handler AST fix (which is a complete fix for its threat model) and open a separate issue for PythonFuncNode so we can pick between (1) and (2) with your input.

Happy to do whichever you prefer — could you confirm:

• (a) land this PR as-is and I'll open a follow-up issue for PythonFuncNode describing the sandbox/auth options, or
• (b) you want a specific mitigation added to PythonFuncNode in this PR — if so, which of (1)/(2), and I'll implement it here.