My Security Product's Docs Said "Sandboxed." One Line Read Every Secret.
TopFlow's docs promised workflow code had no access to globals. One line of JavaScript returned every server secret. How we found it, contained it within hours, and what we got wrong along the way.
The Sentence That Was Wrong
TopFlow lets you add a JavaScript step to an AI workflow: write a few lines, and the server runs them between the other steps. Our documentation filed that step under "sandboxed" and described it like this:
"JavaScript code runs in a limited scope with no access to global objects, file system, or network."
It read well. It sounded like the kind of thing a security-focused product should say. It was also untested.
While auditing our own blog posts and docs against the code, a sentence at a time, we got to that one and did the obvious thing: we tried it. One line was enough:
return process.envThe step returned every environment variable on the server. On the live service, that includes our rate limiter's IP-hashing key and, where configured, the credentials for its Redis store. And fetch was sitting right there to send them anywhere. No account was needed to submit a workflow.
This post is about how that happened, how we contained it within hours, the one mistake we made during the response, and the fix we're building.
Why new Function Feels Safe
The code path was short. Each JavaScript and Tool node ran like this:
const fn = new Function(...Object.keys(inputs), code)
return fn(...Object.values(inputs))The file, lib/topflow-execution-engine.ts, even carried a comment above those lines calling them sandboxed.
Here's why that's an easy mistake. Code created with new Function really can't see the local variables around it: try to read one and you get a ReferenceError. So it feels scoped. But it sees every global in the process: process, process.env, fetch, globalThis. Hiding local variables isn't isolation. For anything that matters, new Function is eval with better manners.
Conditional nodes had the same shape: their condition string was evaluated with new Function(..., "return " + condition). So the question wasn't "can a JavaScript node do this?" It was "which three node types can?"
Proving It Without Touching Real Secrets
The temptation in a moment like this is to confirm the problem in production. We didn't. Instead:
- On a local production build, we started the server with a fake secret in its environment, then ran a workflow whose JavaScript node returned only names and types, never values. It reported
envVarCount: 121,sawTestSecretName: true,hasFetch: "function". - Against production we used a harmless probe: a custom node whose code is just
return 1. If it returns1, custom code runs; if it's refused, it doesn't. That answers the only question that matters without reading anything.
Containment in Hours, Not a Rewrite
The proper fix is real isolation, and that takes time to build and review. The hole was open now, so we contained it first, without breaking the product. Two changes:
1. Only code we ship can run
The server now runs JavaScript or Tool code only if it's byte-identical to code in one of TopFlow's built-in templates (lib/security/trusted-code.ts):
case 'javascript':
if (!isTrustedCode(data.code)) throw new Error(UNTRUSTED_CODE_MESSAGE)
return super.executeNode(node, inputs, context)Users' inputs still flow into that code as function arguments, so every template, including the GitHub security scanner, which uses JavaScript steps, keeps working. Custom code gets a clear refusal explaining why.
2. Conditions stopped being code
Instead of evaluating the condition string, the server now parses it with a small interpreter (lib/conditions/safe-evaluate.ts) that understands input variables, comparisons, &&, ||, ! and a few string methods, and nothing else. There's no eval left to escape from. We tested it against JavaScript's own answers for every condition our templates use, and against the classic escapes: constructor.constructor("return process")() is simply a syntax the parser doesn't accept.
We chose these over the obvious alternatives on purpose:
- Block all JavaScript nodes: would have broken the flagship scanner.
- Delete secrets from
process.envfirst:fetchand every other global would remain. - Node's
vmmodule: Node's own documentation says it "is not a security mechanism."
A Test That Fails for the Right Reason
The regression test (h17-user-code.test.ts) plants a secret in process.env, runs the real route and engine, and tries to read it three ways: a JavaScript node, a Tool node, and a condition. Against the fixed code, the secret never appears. Against the old code, the tests fail, and we checked why they fail: the secret was right there in the response. A security test that fails for some other reason proves nothing.
The Mistake: "Merged" Is Not "Live"
Here's the part we'd rather not write, which is why it's worth writing.
The fix was reviewed and merged into our development branch. The secrets were rotated. Then we ran the harmless probe against production, and it returned 1. Production deploys from a different branch, and the fix hadn't been released yet. The hole was still open, which meant the new secrets had been exposed to it too.
We released, re-ran the probe until production refused custom code, confirmed the templates and ordinary conditions still worked, and rotated every secret again. That second rotation is the one that counts.
The order that matters, which we now follow as a rule:
- Reproduce safely (no real secrets).
- Contain.
- Release, and verify in production with a harmless probe.
- Then rotate credentials.
What We Can't Tell You
We don't log workflow content (a deliberate privacy choice), so our logs can't say whether anyone ever used this. We assume the worst: every secret that existed while the hole was open has been replaced.
Custom JavaScript and Tool code is disabled on the hosted service until real isolation ships. Exported code (which runs on your own infrastructure) is unaffected.
The Real Fix: Isolation, Not Trust
The plan is to run custom code in QuickJS compiled to WebAssembly, a separate JavaScript engine with no process, no fetch, no require, inside a worker thread per run, with an empty environment, a memory limit and a hard time limit.
In a spike, every escape we tried inside QuickJS saw nothing of the host. The spike also taught us why the worker layer matters: on its own, QuickJS took six seconds to hit its memory limit and then crashed the whole process when cleaned up. Inside a worker, the same attack is killed after two seconds and the server doesn't notice. The full design, including what we rejected and why, is public: js-node-isolation-design.md.
Key Takeaways
eval,new Functionandvmare not sandboxes. Code inside your process can reach everything the process can.- Test your security claims, especially the ones in your docs. Ours was never tested; it was just believed.
- Prove problems without reading real secrets: fake secrets locally, harmless probes in production.
- Contain first, fix properly second, and design containment so the product keeps working.
- "Merged" isn't "live." Verify in production, then rotate credentials.
- Say what you don't know. We can't prove the hole was never used, so we act as if it was.
Go Deeper
- The isolation design (QuickJS + worker threads)
- The JavaScript node documentation, now accurate.
- 5 Layers of Security (its A03 section now describes this issue) and Preventing SSRF Attacks:
fetchfrom user code would have bypassed the SSRF guard entirely.