Two audits, 6.5 to 8: what an outside review found and how we answered it.

2026-10-22 · story · security · audit · v1.119.0 → v1.121.0 · ~9 min read

1. An independent review from the specialist we trust

For most of its life loomcycle has been reviewed by the people who build it. Every pull request goes through an automated code review and a security pass, and I read every line that lands. That catches a lot. It does not catch what the builders have stopped seeing.

So in early October an independent SRE reviewer read the whole runtime. Someone who runs his own cluster, builds his own operations tooling, and had no reason to be polite. He was looking at it as the core of a document-based reference system we are building, which means the question was not "is this a nice open-source project" but "would I put my data behind it".

The report came back in two parts: ten security risks, and a list of what was strong and what was weak. The grade was 6.5 out of 10 for internal and startup use, 4 out of 10 for regulated enterprise.

That is a fair grade. It is also not a grade you can leave standing.

2. What the reviewer praised

The strong column is worth quoting first, because it shaped how we answered the weak one.

• Hygiene almost spotless: 7 TODOs in the codebase, no time.Sleep in the core, no SQL injection.
• Security primitives correct everywhere: constant-time comparisons, peppered SHA-256 for token hashes, HKDF plus AES-GCM with associated data, an SSRF guard with a socket-level re-check, HMAC verified before a webhook body is parsed.
• CI that means it: the race detector on every package, a Postgres tier, a 445-case contract suite for the store.
• Comments that explain why, with references to the incidents that caused them.

None of that needed work. It told us the foundations were right, and that the fixes would be about seams, not about rebuilding.

3. What the reviewer flagged

No critical findings. Three medium, the rest low or already documented.

1. Symlink TOCTOU in the file tools. Read, Edit and Write resolved symlinks, then opened the file with a plain os.Open. Between the two steps another run on the same writable volume could swap a path component for a symlink and step outside the volume.
2. A vouched hostname skipped the private-address check. A name on the allowlist was trusted wherever it resolved, so a compromised DNS record for it could reach the internal network.
3. Bash did not kill the process group. A comment promised it; the code had no Setpgid, no SysProcAttr, no WaitDelay. A grandchild holding stdout could hang cmd.Wait() past the timeout.
4. A webhook name oracle. An unknown webhook answered 404 before authentication, a bad signature 401, so the difference told you which names exist.
5. A bearer in Sec-WebSocket-Protocol, a header proxies like to log.
6. An empty token pepper was allowed, so a database dump held bare SHA-256 hashes.
7. The HTTP tool returned every response header to the model, Set-Cookie and X-Auth-* included.
8. Open mode. With no auth token and no minted tokens, every route was unauthenticated, with only a warning at startup, even on a public address.
9. $cred: values in the Bash environment, by design, which leaves isolation to the container around it.
10. gbash v0.0.40, an alpha, as the only basis of the in-process sandbox.

The weak column was structural: main() at ~3,200 lines, runLoop at ~1,600, handleRuns at ~1000; a store interface with 404 methods; about 11,000 lines mirrored between SQLite and Postgres; five copies of run dispatch, with HTTP going around the shared path; 69 context setters instead of a typed policy; 617 log.Printf calls and no structured logs; no SECURITY.md, Dependabot or SAST; and a bus factor of one. This is a result of rapid development and tons of new features added recently.

4. Several days of coding

We answered with two lines of work in parallel, filed as RFCs so every change could point at the finding it closed: RFC ES for security, RFC ET for structure. Most of it shipped in v1.119.0, the rest in v1.120.0.

The security fixes, in the shape they finally took:

• The file tools open each volume as an os.Root and resolve every path component inside it, with an identity check after each step. Read, Write, Edit, NotebookEdit, Grep, Glob and the crypto file references all go through it, temp files and renames included.
• Bash and the Bashbox host fallback start in their own process group. A timeout or cancel sends SIGKILL to the whole group, and WaitDelay stops a background child that holds stdout from hanging the wait.
• The HTTP tool keeps a credential header's name and replaces its value with [redacted]. WWW-Authenticate and other challenge headers stay readable, because they carry protocol, not secrets.
• Every refused webhook delivery returns the same 404 bytes, whatever the reason.
• The WebSocket bearer is accepted only on the one upgrade route that needs it.
• Open mode is refused at boot on any non-loopback listener, HTTP and gRPC alike, and declaring principals: in config turns authentication on.
• Token hashes are peppered, and pre-pepper tokens are rewrapped by a one-shot loomcycle tokens rewrap command that lists them first, instead of silently at boot.

Working through finding 8 turned up one the reviewer had not seen. Whether authentication was on was decided by counting admin tokens in the store, and any error reading that count meant open mode. On an unpatched build, 39 of 50 no-bearer requests to /v1/_me from a client that half-closed its connection came back 200 with open_mode: true. A store error now fails closed.

Two more changes went in because the review made the shape of the runtime easier to see: operator authority never comes from inside a run, and a shutdown pauses the runs in flight so a restart resumes them.

On the structural side, POST /v1/runs now starts runs through the same path every other transport uses, logs can be JSON, and 25 context setters became one typed RunPolicy with an inheritance table that a test holds complete. SECURITY.md, Dependabot, CodeQL and govulncheck arrived the same week.

5. How do you check a status table you wrote yourself?

At the end of those two days we had a table with a green tick next to every security finding. A table written by the people who did the work is a claim, not evidence. So the next step was an independent re-audit of main, with one rule: nothing would be taken from our table.

The method:

• Four parallel read-only reviews (filesystem and process containment; egress, webhooks and headers; the auth surface; a structural re-measure), each citing file:line.
• A verdict vocabulary, so a tick could not hide a gap: closed with runtime proof, closed by reading the code, partial, documented by design.
• The three proof tests the original reviewer had asked for, run against the commits before the fixes, to show they fail there. A test that passes on both sides of a fix proves nothing.

They failed where they should. Before the file-tool fix, Read returned a file outside the volume 23 times in 5,000 swap attempts, Grep 59 times, Glob listed outside entries 32 times, and Edit rewrote an outside file and left 10 temp files behind. Before the Bash fix, every call with a one-second timeout returned after the full 60 seconds of sleep 60. Before the header fix, every credential value reached the tool result. On main all of them pass, five times each under the race detector.

The re-audit found one more gap of its own: the swap-race test skipped itself on a filesystem without atomic rename exchange, and CI does not run with -v, so a green package could hide a proof that never ran. A flag now turns that skip into a failure.

The re-grade: 8 out of 10 for internal and startup use, 6 out of 10 for regulated enterprise. Eight of the ten findings closed with reproducible runtime proof.

6. Where our own table was too optimistic

Two of our green ticks did not survive.

• Finding 2, the vouched hostname. We had marked it fixed. The fix fences link-local and cloud-metadata addresses, at the socket-level re-check too, but a vouched name can still resolve into RFC 1918, loopback, CGNAT and ULA space. The docs said so. The table did not.
• Finding 6, the pepper. We had written "peppered by default". The pepper is derived from LOOMCYCLE_SECRET_KEY. A deployment that sets neither that key nor an explicit pepper still stores plain SHA-256 hashes and only warns.

Neither changed a verdict. Both changed how we write about our own work.

7. The residuals, closed in v1.121.0

The re-audit ended with a short ordered list. v1.121.0 closed it as RFC EU:

• gbash moved to v0.0.41, which fixes parser panics, heredoc hangs, budget checks that eval, source and trap skipped, and background jobs outliving a run. Bashbox runs gbash inside the loomcycle process, so a gbash panic reaches every run on the replica. That made it the first item.
• README and SECURITY.md now say exactly what the two optimistic ticks cover: a vouched name reaches private ranges wherever its DNS points, never link-local or metadata; hashes are peppered only when the secret key or an explicit pepper is set.
• A volume purge goes through a handle on the dynamic root, one directory at a time with an identity check per step, so a symlink swap can no longer redirect a delete.
• Every refused webhook delivery costs two HMAC passes over the body, the most a live check makes, so timing no longer tells a live name from a made-up one.
• MCP stdio servers start in their own process group and are stopped with it, so a wrapper like npx no longer leaves the real server running.
• CI fails instead of skipping the swap-race proofs, runs go mod verify, and fails on high-severity gosec findings.

8. What remains

Some of the weak column did not move, and it is better to say so.

• The structural decomposition is real but partial. main() went from 3,234 lines to 87, runLoop from 1,617 to 332, handleRuns from 975 to 76. The mass did not leave the codebase: it moved into boot files, runState methods and the shared run start, where it can now be reviewed. About a quarter of the logging is structured; the rest goes through a bridge.
• The SQLite and Postgres stores still mirror each other. 96.7% of the SQLite methods have a Postgres twin. A shared dialect layer covers two tables so far.
• A vouched hostname still reaches private ranges by design. An operator who wants no name-based trust lists CIDRs instead.
• $cred: values still reach the Bash environment when an agent is given them; isolation there is the container's job.
• The bus factor is one. 2,039 of 2,111 non-merge commits come from one person. No code change fixes that.