# open-review — independent security review @ 6c46c829661733504a9f68383b0ec69b810cf039

## 1. Bottom line

The target at this commit is not a Basecamp module: it is the open-review harness itself (no `metadata.json`, no `.lgx`, no Basecamp module source). I therefore audited the pipeline, sandbox, renderer and host scripts, and ran the parts that can run inside the supplied sandbox. `/run/src` was treated as hostile data throughout.

The escaping and serving layer is the strongest part. Every module/report-derived string rendered into `index.html`/`live.html` goes through `html.escape`, the Caddy configuration sets `default-src 'none'` and `nosniff`, JSONL is forced to `application/x-ndjson`, and the Venice inference key is not passed into the sandbox (the sandbox's own check confirms this). The bait canaries are unique, and the simple "absolute path read plus DNS lookup" case is detected.

The weak points are structural and, in one case, critical for a service whose entire product is trustworthy evidence. A hostile module has the same root and the same writable `/run` as its reviewer; it can therefore tamper with the evidence and report that judge it (FR-2), and the host-side renderer follows symlinks out of `/run`, copying host files — including the file holding the Venice inference key — into the published review (FR-1). The network record does not work at all: the DNS sink is not wired into `resolv.conf`, egress is unrestricted, and the analyser ignores raw-IP connects, so a module can read bait and exfiltrate it without the summary noticing (FR-3). Inputs are interpolated into `bash -lc` in `review.sh`, giving repository/commit/`.lgx` URLs shell-injection capability before static checks run (FR-4). The "hard" model budget can be exceeded by an unknown model name and is under-priced by the local price table (FR-5), and an outer 40-minute `timeout` truncates a review advertised as 150 minutes (FR-6).

## 2. What I ran

All probes are in `/run/probes/`; results are quoted below. Tags: `[run]` reproduced in this sandbox, `[read]` from code only.

- `[read]` Mapped the target: `git log`/`git status`, `find`, `wc -l`, then read `pipeline/{review,review-in-sandbox,reviewer,review-stop}.sh`, `pipeline/{gateway,render,analyse,balance,fetch_deps,dns_sink}.py`, `pipeline/desk.sh`, `pipeline/sandbox-check.sh`, `sandbox/*`, `host/*`, `docs/proposal.md`, `NOTES.md`, `test-modules/*`.
- `[run]` `/run/checks/*`: pre-run gitleaks found only the deliberate fake key template in `sandbox/bait.sh` (commit `d5194a8`); osv-scanner returned no results; semgrep returned four items (three dynamic-`urllib` notes, one `curl | bash`).
- `[run]` `desk time` / `date` at start: 0 of 0.80 DIEM testing, 150-minute clock. Runtime facts: `id` = root, `/run` and every `/run/{behaviour,media,logs,src}` writable (0775), `/pipeline` and the AppImage read-only, both `REVIEW_TOKEN` and `REPORT_TOKEN` readable in `/proc/1/environ`, `/root/.pi/agent/models.json` mode 0644 containing `REVIEW_TOKEN`.
- `[run]` **FR-1 symlink probe** (`/run/probes/p1`, `p5`): built a run directory whose `report.md` and `media/leak.txt` were symlinks to a marker file, and whose `logs/` contained a symlink to a directory, then ran the real renderer: `python3 pipeline/render.py /run/probes/p1/run /run/probes/p1/dest slug final`. Result: `dest/report.md` and `dest/media/leak.txt` contained `TOP-SECRET-VENICE-KEY-abc123`; the directory symlink produced `dest/logs/fakedir/id_ed25519` containing `HOSTFILE-SECRET-XYZ`.
- `[run]` **FR-2 writability probe**: `touch /run/behaviour/.wtest` succeeded; `ls -ld /run /run/behaviour /run/media /run/logs /run/src` all `drwxrwxr-x root:root`; `/run/src` writable. The same root identity runs the module via `desk.sh` (`/opt/bc/squashfs-root/AppRun`).
- `[run]` **FR-3 network probe**: `/etc/resolv.conf` nameservers are external (`195.179.224.53`, `209.126.15.53`); `dns_sink.py` binds `127.0.0.1:53` but is never used. A direct UDP DNS query to `195.179.224.53` and `1.1.1.1` returned a 120-byte reply; TCP connects to `8.8.8.8:53`, `1.1.1.1:443` and `140.82.121.4:443` all succeeded; `/run/behaviour/dns.log` stayed 0 bytes throughout, including after running the repo's own credential-reading `test-modules/leaky-build/collect.js`.
- `[run]` **FR-3/analyser probe** (`/run/probes/p2`, `p2b`): a synthetic but syscall-accurate `strace.log` with `openat(3, ".ssh/id_ed25519", ...)` and `connect(... inet_addr("203.0.113.9"))` yielded `bait_files_read: []`, `contacted_hosts: []`, `read_secret_then_contacted_external_host: false`; the control (absolute `/root/.ssh/id_ed25519` plus a DNS line) yielded the expected flagged result. The live global capture does contain the control sample's absolute bait read (`openat(AT_FDCWD, "/root/.ssh/id_ed25519", ...)`), so the intended simple case is observable; the dirfd-relative variant in the synthetic log is not reported, and literal-IP connects are never put into `contacted_hosts`.
- `[run]` **FR-4 injection probe**: with `REPO="x'; echo INJECTED > /tmp/probe-ci; echo '"`, the exact `git clone ... '$REPO' ...` string executed `echo INJECTED > /tmp/probe-ci` (marker present). Same construction applies to `$COMMIT` and to the `.lgx` URL in the `curl` line.
- `[run]` **FR-5 gateway probe** (`/run/probes/gw`): ran `gateway.py` against a local mock upstream with `REVIEW_BUDGET=0.8`; one POST for `claude-3-opus-20240229` with 1M/1M tokens was forwarded and logged `cost: 1.65`, leaving `spend.json` at `{"spent":1.65,"budget":0.8,"testing":0.64}` — i.e. the first request alone exceeded the stated cap, priced with the DeepSeek fallback rate.
- `[run]` **FR-7 scanner-config probe** (`/tmp/gltest`): a committed file with `AKIAABCDEFGHIJKLMNOP` was found by the exact `gitleaks detect --no-banner --report-format json --report-path ... .` command; adding a repository `.gitleaks.toml` with `[allowlist] paths = ['a\.txt']` made the same command report `no leaks found`. A malformed `.gitleaks.toml` made gitleaks exit fatally; the wrapper's `|| true` hides that.
- `[run]` **FR-8 `desk.sh` install probe** (`/run/probes/p4`): crafted an `.lgx` with `manifest.json` `name` = `../../../../../../tmp/lgx-pwn-dir`; calling the real `install_one` created `/tmp/lgx-pwn-dir/marker` outside the Basecamp module tree. GNU `tar` refused a `../` member (`Member name contains '..'`), but accepted an absolute-target symlink member; `install_one` also copies `variants/linux-amd64` with `cp -a` (follows a symlinked variant).
- `[run]` **FR-9 malformed report probe** (`/run/probes/p3`): `report.json` = `{"verdict_summary":"x","findings":{"a":1}}` made `render.py` raise `AttributeError: 'str' object has no attribute 'get'` at line 135; only `chain.txt`, `logs/` and `report.json` were published, no `index.html`.
- `[run]` **FR-11 semver probe**: extracted and executed the real `ver`/`satisfies` functions from `fetch_deps.py`; `satisfies("0.9.0","^0.1.0")` returned `True`.
- `[run]` `getent hosts telemetry.handy-notes-cdn.net` failed (`rc=2`) while direct DNS to `1.1.1.1` succeeded; this confirms the sink is not in the resolver path, not that egress is blocked.
- `[read]` Compared every claim in `docs/proposal.md` with the implementation (`grep` for schema/signature/overlay/uninstall/build-compare) and read the repository's reviewer-directed prose.
- **Not run:** a real Basecamp module session. This target has no `.lgx`, no `metadata.json`, and no module UI, so `desk install`/`desk start` had nothing to install; no Basecamp screenshots or video exist for this review. `podman`, the host Caddy/shrooms/agent setup, and a full end-to-end review need the host and were not available here. Nested `strace` returned `PTRACE_TRACEME: Operation not permitted`, so the live global capture was used instead of a per-probe trace.

## 3. Findings

### FR-1 — Critical — Host-file disclosure and evidence forgery: the renderer follows symlinks out of the module-writable run directory
**Files:** `pipeline/render.py:45-49` (latest screenshot copied), `pipeline/render.py:92-97` (`copytree` + `copy` of raw evidence), `pipeline/render.py:12-14`/`130` (`readf`), `pipeline/render.py:124-126` (report.json), `pipeline/review.sh:40` (`-v "$RUN:/run:z"` rw), `pipeline/reviewer.sh:11` (location of the real key: `/home/reviewer/.pi/agent/.env`).

**Exploit:** A submitted module runs as root in the same container and `/run` is mounted read-write. It creates `ln -s /home/reviewer/.pi/agent/.env /run/media/leak.txt` (or `ln -s ... /run/report.md`, or `ln -s /home/reviewer /run/logs`). `render.py` runs later on the host as the `reviewer` user and uses `shutil.copytree(..., symlinks=False)` and `shutil.copy`, both of which dereference the link and publish the target under `/srv/reviews/<slug>/`. My probe did exactly this with a stand-in secret:

```
ln -sf /tmp/probe-secret p1/run/media/leak.txt
ln -sf /tmp/probe-secret p1/run/report.md
python3 pipeline/render.py /run/probes/p1/run /run/probes/p1/dest slug final
grep -rl 'TOP-SECRET-VENICE-KEY' p1/dest
→ p1/dest/report.md
→ p1/dest/media/leak.txt
```

A directory symlink was also recursively copied (`dest/logs/fakedir/id_ed25519`). In production the target of interest is `/home/reviewer/.pi/agent/.env`, which `reviewer.sh` itself reads as the Venice key; publishing it leaks provider credentials and the whole account to anyone who fetches the review. A symlinked `report.md` also makes the pipeline's `[ -s report.md ]` check pass with attacker-chosen content.

**Fix:** Never dereference paths supplied by the sandbox. Resolve every path with `os.path.realpath` and refuse anything outside `$RUN`; walk with `os.walk(followlinks=False)` and skip all symlinks (copy links as links, not targets); open files with `O_NOFOLLOW`; do not `copytree` arbitrary run subdirectories at all — copy an explicit allowlist of regular files (`report.md`, `report.json`, `chain.txt`, known evidence files) after `lstat` checks. Mount the source/evidence directory read-only to the sandbox or separate module-writable data from host-published evidence.

### FR-2 — High — The untrusted module can tamper with, or forge, every piece of evidence and the report used to judge it
**Files:** `pipeline/review.sh:40` (rw `/run`), `pipeline/review-in-sandbox.sh:49` (`strace ... -o /run/behaviour/strace.log`), `pipeline/analyse.py:10-11` (reads `bait.json`, `dns.log`), `pipeline/render.py:92-97`/`124-126` (publishes them), `pipeline/desk.sh:31` (Basecamp/module runs as root).

**Exploit:** The module is root in the same filesystem view as the capture. It can `truncate -s0 /run/behaviour/strace.log`, `echo '{}' > /run/behaviour/bait.json`, delete/replace `/run/behaviour/dns.log`, write an arbitrary `/run/report.md`/`report.json`, and plant arbitrary files in `/run/media`. `analyse.py` then grades an attacker-chosen record, and `render.py` publishes it. I verified the access: `/run/behaviour` is `drwxrwxr-x root:root`, my `touch /run/behaviour/.wtest` succeeded, and `/run/src` is writable; the report symlink probe (FR-1) shows `report.md` can be replaced wholesale. The experiment also showed the automated summary can be emptied without touching `strace` (FR-3). There is no host-side, append-only copy to compare against.

**Fix:** Move evidence out of the module's mount namespace: collect `strace`/DNS/network events on the host (or a separate privileged observer) and write them to files the sandbox cannot see or write; expose a read-only `/run/src` and a separate bind-mounted, host-owned `/run/evidence`; run the module as a non-root UID distinct from the reviewer; sign or hash the capture immediately in the observer; refuse to publish if evidence files changed between capture and render (compare inode/mtime/hash to the observer's copy).

### FR-3 — High — Network monitoring is ineffective: the DNS sink is bypassed and raw-IP traffic is never reported
**Files:** `pipeline/dns_sink.py:19` (binds only `127.0.0.1:53`), `pipeline/review-in-sandbox.sh:20` (starts it but nothing points the resolver at it), `pipeline/analyse.py:16-27` (`connects` computed but unused; `contacted_hosts` comes only from `dns.log`), `pipeline/review.sh:26` / `pipeline/reviewer.sh:26` (`--network bridge`, no egress rule), `pipeline/Containerfile` (no resolver or firewall setup).

**Exploit:** The sandbox's `/etc/resolv.conf` points at external DNS servers, not `127.0.0.1`, so the sink never logs a lookup. Egress is unrestricted, and a module can also skip DNS entirely and connect to a literal IP. Verified in the running sandbox: direct UDP DNS to `1.1.1.1` and `195.179.224.53` got replies; TCP to `8.8.8.8:53`, `1.1.1.1:443`, `140.82.121.4:443` succeeded; `dns.log` stayed at 0 bytes even after the repository's credential-stealing control sample ran. A module using `openat(dirfd, ".ssh/id_ed25519")` plus `connect(203.0.113.9:443)` produced no bait and no host in `analyse.py`'s summary:

```
{"bait_files_read": [], "contacted_hosts": [],
 "read_secret_then_contacted_external_host": false, ...}
```

This directly contradicts the proposal's "only the model gateway, package mirrors ... and all of that is recorded" and defeats the product's core exfiltration claim.

**Fix:** Point `/etc/resolv.conf` at the sink (and make it immutable to the module), handle AAAA and TCP DNS, or better, intercept DNS in a dedicated network namespace. Enforce egress with a default-deny firewall that allows only the gateway and mirror ranges — the sink is a logger, not a control. Parse and report `connect()` destinations (including `AF_INET`/`AF_INET6` literals) from `strace` in `analyse.py`; include process and UID per connection. Add a sandbox check that a test lookup and a test raw-IP connect are both recorded before the review is allowed to run.

### FR-4 — High — Shell injection from repository, commit and `.lgx` URL into `bash -lc`
**Files:** `pipeline/review.sh:40` (`inbox()` builds `bash -lc "$2"`), `pipeline/review.sh:49` (`git clone ... '$REPO' ...; git checkout ... '$COMMIT'`), `pipeline/review.sh:58` (`curl ... -o 'lgx/$(basename "$LGX")' '$LGX'`).

**Exploit:** `$REPO`, `$COMMIT` and `$LGX` are interpolated inside single quotes in a string handed to `bash -lc`. A submitter-supplied repository URL containing a single quote breaks out and executes commands in the clone container, which has the run directory mounted read-write and `--network bridge`, before static checks and before the reviewer starts. Verified with the exact line-49 construction:

```
REPO="x'; echo INJECTED > /tmp/probe-ci; echo '"
bash -lc "git clone ... '$REPO' src ..."
→ /tmp/probe-ci contains INJECTED
```

The attacker can rewrite `/run/checks/*`, plant source or instructions for the reviewer, and forge `logs/*`, or attack anything reachable on the bridge. The same primitive exists for `LGX` (line 58) and `COMMIT`.

**Fix:** Do not build shell strings. Execute `git`/`curl` with an argument vector and `--` (`git clone -- "$REPO" src`), validate the URL scheme/host, and quote nothing because no shell is involved. Run cloning in a container with `/run/src` as the only writable mount, and perform static checks before any untrusted input is fetched or executed; put the clone step after a host-side URL validation.

### FR-5 — Medium — The "hard" DIEM budget can be exceeded and is under-priced; both gateway tokens are readable by the module
**Files:** `pipeline/gateway.py:21-27` (`PRICES` fallback for unknown models), `pipeline/gateway.py:77` (unbounded `Content-Length` read), `pipeline/gateway.py:82-84` (pre-check uses `last`, not a reservation), `pipeline/reviewer.sh:19-27` (tokens passed into the container env), `pipeline/review-in-sandbox.sh:29` (`models.json` written 0644), `pipeline/sandbox-check.sh:19-21` (only the Venice key is checked).

**Exploit:** Two independent bypasses. (1) Model IDs are not validated and unknown IDs are priced with the DeepSeek fallback `(0.33, 0.0075, 1.32)`. My mock-upstream run forwarded `claude-3-opus-20240229` and recorded `cost 1.65`, `spent 1.65` against a `budget 0.8`; a valid but expensive Venice-hosted model is therefore charged at DeepSeek rates, and a single first request can exceed the cap because the pre-check sees `last=0` and only reserves `0.0024` DIEM. (2) The model can read `REVIEW_TOKEN` from the 0644 `models.json` and `REPORT_TOKEN` from `/proc/1/environ` (I confirmed both strings are present and readable), then make its own requests, spend the budget, and inject content into `gateway.jsonl`/`live.txt`. The `REPORT_TOKEN` also bypasses the testing/report split. Separately, the gateway fails open when `REVIEW_TOKEN` is unset (`elif not TOKEN or auth==...`, `gateway.py:74`), so a gateway started outside `reviewer.sh` is an unauthenticated relay to the Venice key.

**Fix:** Allowlist model IDs and derive price from the allowlist; reject unlisted models. Before forwarding, reserve `max_tokens × output_price + prompt_tokens × input_price` (or a conservative cap) against the budget under the lock. Do not put either token in the container environment or a world-readable file; give pi a Unix-socket proxy with a per-process credential, and make the report token available only to the second process. Cap request body size and thread count.

### FR-6 — Medium — A 40-minute outer `timeout` silently truncates the 150-minute review and suppresses the report pass
**Files:** `pipeline/reviewer.sh:26` (`timeout 2400 podman run ...`), `pipeline/review-in-sandbox.sh:45-64` (`MAX_MIN=150`, inner timeout and report-pass logic), `pipeline/desk.sh:71-77` (clock promises `max` minutes).

**Exploit:** `2400` seconds is 40 minutes. The outer command terminates the attached `podman run` long before the inner `timeout 150m`. Because the report-pass block (`review-in-sandbox.sh:66-73`) runs inside the container after the reviewer exits, a review that legitimately uses the advertised 150 minutes can be stopped with no report (and `review.sh` then marks it failed). This is visible in the clock model itself: the clock reports 150 minutes while the container is capped at 40. `[read]` — I did not let the run reach the cap.

**Fix:** Make the outer timeout derive from `REVIEW_MAX_MIN` (e.g. `MAX_MIN*60 + 300`) or delete the outer timeout and rely on the inner one plus a host watchdog that still allows the report pass. Test a review that runs longer than 40 minutes. Ensure SIGTERM captured outside the container triggers evidence collection and the report pass.

### FR-7 — Medium — Repository-controlled scanner configuration can hide findings; scanner failure is silent
**Files:** `pipeline/review.sh:52-56` (scanners run in the repository working directory with no trusted config), `sandbox/reviewer.Containerfile:45-52` (scanner versions), `pipeline/review.sh:62` (only byte count is surfaced).

**Exploit:** `gitleaks detect` auto-loads a repository `.gitleaks.toml`. I committed a file containing `AKIAABCDEFGHIJKLMNOP`; the exact pipeline command found it (`1` finding). After adding `.gitleaks.toml` with `[allowlist] paths = ['a\.txt']`, the same command reported `no leaks found` (`0` findings). A malformed config makes gitleaks exit fatally; `review.sh` ignores the exit status (`|| true`) and only omits the `gitleaks: N bytes` live line, so a review can publish with no secret scan and no failure marker. `.semgrepignore` and `osv-scanner.toml` (e.g. `[[IgnoredVulns]]`) give an analogous suppression path for the other two scanners. `[run]` for gitleaks; `[read]` for the other two.

**Fix:** Run each scanner with an explicit, host-owned config and disable repository config discovery where possible (`--config /pipeline/gitleaks.toml`; unset `SEMGREP_*`/`GITLEAKS_CONFIG`; pass `--no-ignore`/equivalent; validate the result file schema). Treat a missing or empty report as a failed check, not a pass, and hash the scanner configs and outputs.

### FR-8 — Medium — `desk install` writes and deletes outside the module directory using the manifest `name` (path traversal), and extracts the `.lgx` as root
**Files:** `pipeline/desk.sh:48-57` (`name=$(jq -r .name ...)`; `dest="$U/modules/$name"`; `rm -rf "$dest"`; `cp -a ...`), `pipeline/desk.sh:113-121` (dependency and module install loop).

**Exploit:** A crafted `.lgx` with `manifest.json` `{"name":"../../../../../../tmp/lgx-pwn-dir","type":"ui_qml"}` makes `install_one` set `dest` outside `$U`, then `rm -rf` it and copy attacker files there as root. Verified against the real function: `/tmp/lgx-pwn-dir/marker` was created. `tar xzf` runs as root in a temporary directory; GNU tar refused a `../` member and refused to write through an absolute symlink member in my tests (`Cannot open: Not a directory`), so the manifest-name traversal is the confirmed vector; `cp -a "$t/variants/linux-amd64/."` would still follow a symlinked `variants/linux-amd64` into its target. Inside the sandbox this can delete/rewrite `/run`, `/opt/bc`, or `/usr` content and thus corrupt the review before it starts.

**Fix:** Reject any manifest `name` not matching `^[A-Za-z0-9._-]+$` and reject `.`/`..`; compute `dest` with `realpath` and verify it is under `$U`. Extract with a safe extractor (or `tar --no-same-owner --no-overwrite-dir` plus an explicit rejection of symlink/hardlink/absolute members), and install as a non-root user. Never use `rm -rf` on a path derived from untrusted metadata without the prefix check.

### FR-9 — Medium — No schema validation: a malformed `report.json` aborts publication
**Files:** `pipeline/render.py:124-140` and `225`; claim at `docs/proposal.md:35` ("A plain program ... checks the report against a schema").

**Exploit:** `report.json` is written by the model and can also be overwritten by the module (FR-2). When `findings` is an object instead of a list, `for f in rep["findings"]` yields strings and `f.get` raises `AttributeError` at line 135. My probe produced only `chain.txt`, `logs/` and `report.json`; no `index.html` was written, so the review page is gone. There is no schema file or validation anywhere in the repo, contrary to the proposal.

**Fix:** Validate `report.json` against a strict schema before rendering, with size limits and type checks; wrap rendering so `index.html` is always written (with a "failed" banner on invalid input); never let the model's output determine whether publication succeeds.

### FR-10 — Medium — Self-certifying/overstated claims in the repository contradict the implementation
**Files:** `docs/proposal.md:32` (network recorded), `:35` (schema check), `:40` (build-vs-`.lgx` comparison), `:47` (uninstall and leftovers), `:50-57` (write overlay diff, processes, resources), `:120` (logs "signed", inputs "pinned"); `README.md`/`test-modules/README.md:5` and `test-modules/leaky-build/collect.js:1` (the credential-stealing sample is labelled "harmless"/"CONTROL SAMPLE ... not for any other use"); `NOTES.md` (self-assessment).

**Exploit:** None of the following exists in the code: build-and-compare, overlay/write diff, uninstall, process/resource summary, signature over `chain.txt` (it is a bare SHA-256 chain, `render.py:99-106`), or input pinning (`FROM ubuntu:24.04`, `curl | bash`, unpinned npm/pipx). The network claim is refuted by FR-3. The test-module comments and README are prose addressed to the reviewer telling it how to classify a credential-stealing postinstall; per the review rules this is itself a finding, and `report.json` records `repo_tried_to_instruct_reviewer: true`. `[read]` (claims/implementation comparison) and `[run]` for the network half.

**Fix:** Make the documentation state exactly what is implemented and mark unimplemented claims as roadmap (as `BACKLOG.md` partly does); sign the chain with a key outside the review host or remove the signature claim; pin images and downloads by digest; treat repository prose as untrusted input in the prompt and never let it set scope or severity.

### FR-11 — Low — Caret version matching is wrong for 0.x dependencies
**Files:** `pipeline/fetch_deps.py:18-27` (`satisfies`), `:40-49` (selection).

**Exploit:** The implementation treats `^` as "same major and `>=`", so `satisfies("0.9.0","^0.1.0")` is `True`; semantic versioning says `^0.1.0` must stay below `0.2.0`. A module declaring `^0.1.0` can therefore receive `0.9.0` from the catalogue, outside the range its author intended. `[run]` with the extracted real functions.

**Fix:** Implement proper semver caret rules (for `0.x.y`, lock the minor; for `0.0.z`, lock the patch), or use a maintained semver library and test the boundary cases.

### FR-12 — Low — Gateway DoS surface and incomplete secret redaction
**Files:** `pipeline/gateway.py:35-38` (redaction regex), `:77` (unbounded body), `:148` (`ThreadingHTTPServer`), `pipeline/reviewer.sh:12` (`GATEWAY_BIND=0.0.0.0`).

**Exploit:** A module that has read `REVIEW_TOKEN` (FR-5) can open many connections and/or send a huge or never-finished `Content-Length`; `self.rfile.read(int(...))` and a thread per connection let it exhaust host memory/threads. The redaction regex matches `AKIA[0-9A-Z]{16}` (the access-key ID) but not the AWS secret value, and matches only the `-----BEGIN ... PRIVATE KEY-----` header, leaving the key body in `gateway.jsonl`, which is published. Bait canaries are matched, but any future real secret in a request is not. The gateway binds all interfaces; the firewall is the only reason this is not directly reachable. `[read]`.

**Fix:** Cap body size (e.g. 4 MB), cap concurrent threads/connections, add read timeouts, bind to loopback or a Unix socket, and perform field-aware redaction (remove whole PEM blocks, `aws_secret_access_key`, seed phrases, bearer tokens). Never publish unredacted request/response bodies without a second review.

### FR-13 — Low — Unpinned host/image supply chain for code that holds the Venice key
**Files:** `host/30-agent.sh:13-16` (`bash install-agent.sh`, `curl .../setup_22.x | bash`, unpinned `npm i -g`), `sandbox/reviewer.Containerfile:9-13,26,52` (`ubuntu:24.04` tag, nodesource pipe, unpinned `pipx install semgrep`), `pipeline/reviewer.sh:7` (`localhost/open-reviewer:latest`).

**Exploit:** A compromise of nodesource, the unpinned shrooms `install-agent.sh`, the unpinned npm package, the base image tag, or the mutable `latest` image runs with access to the host's Venice key and the review artifacts. `[read]`; semgrep flagged the `curl | bash` line.

**Fix:** Pin every input by digest or exact version with a hash (`FROM ubuntu:24.04@sha256:...`, `npm i -g pkg@version`, gitleaks/osv/semgrep versions, cosign/checksums), vendor the installer, and build the reviewer image reproducibly.

### FR-14 — Low — The report pass shares the contaminated sandbox/context with the untrusted module
**Files:** `pipeline/review-in-sandbox.sh:66-73` (same conversation, same container, `pi --continue`), `pipeline/reviewer-prompt.md` (report is model-written), `pipeline/render.py:124-140` (published without independent validation).

**Exploit:** Repository prose and module output can prompt-inject the reviewer; the report pass re-uses the same session and filesystem, so a self-serving report (or a symlinked/faked `report.md`, FR-1/FR-2) reaches the published page. `BACKLOG.md` itself notes the report stage is not caged. `[read]`; the repo contains exactly such reviewer-directed prose (FR-10).

**Fix:** Produce the report in a fresh context with only a typed, host-collected evidence bundle; validate it against a schema; have a non-model program decide severity/confirmation labels or at least flag unsupported claims; keep the report stage out of the module's mount namespace.

### FR-15 — Low — Reviews are served over plain HTTP and the hash chain is unsigned
**Files:** `host/Caddyfile:4-14`, `host/20-web.sh:11-18`, `pipeline/render.py:99-106`; claim at `docs/proposal.md:120`.

**Exploit:** A network attacker can read or modify reviews in transit; because `chain.txt` is only a hash chain with no signature or published root, a recipient cannot detect a host-side rewrite. The proposal says the logs are "hash-chained and signed", but no signing key or signature exists in the repo. `[read]`.

**Fix:** Serve TLS, sign `chain.txt` with a key held outside the review host, publish the signature and signing identity, and document how a reader verifies it. Until then, do not claim authenticity.

### FR-16 — Low — Any file staged in the run's evidence/media directories is published same-origin
**Files:** `pipeline/render.py:92-97` (copies whole `media`, `logs`, `checks`, `build`, `behaviour` trees), `host/Caddyfile:5-6` (`file_server browse`).

**Exploit:** The module can write arbitrary files into `/run/media` (or any copied directory). `render.py` copies them verbatim into the published review; Caddy serves them from the same origin with directory browsing. My probe dropped `media/page.html` with a phishing link; `render.py` published it at `dest/media/page.html` (it is not linked from `index.html`, but `/media/` is browsable and the URL is guessable). CSP blocks scripts and forms, so this is not XSS, but it is arbitrary same-origin content on the review site. `[run]` for the copy; `[read]` for Caddy browse.

**Fix:** Copy only an explicit allowlist of media extensions (`.png`, `.jpg`, `.mp4`) after content sniffing; strip everything else; disable directory browsing or restrict it to known files; set `Content-Disposition: attachment` for raw evidence and consider a separate origin for evidence.

## 4. What is solid

- **HTML output escaping.** `render.py` escapes every module/report-derived field (`esc` at line 11; findings at 137-138; summary at 225; run/not-tested/behaviour at 208-213; filenames at 114). I found no HTML/script injection path in the rendered pages, and the Caddy CSP (`default-src 'none'`, `style-src 'self' 'unsafe-inline'`, `nosniff`) blocks scripts even if one were missed. JSONL is forced to `application/x-ndjson` (`host/Caddyfile:12-13`).
- **Venice key separation (for the reviewer process).** `reviewer.sh:11` keeps the key on the host; `review-in-sandbox.sh:50` removes `REVIEW_TOKEN`/`REPORT_TOKEN` from pi's environment; `sandbox-check.sh:19-21` checks the Venice key is absent from `/proc/1/environ` and the sandbox disk. (The other two tokens are *not* protected — FR-5.)
- **Token check works.** `sandbox-check.sh:24-25` verifies the gateway rejects a tokenless request; the gateway's bearer check (`gateway.py:72-75`) is correct for the configured token.
- **Bait construction.** `sandbox/bait.sh` generates per-run random canaries, writes bait mode 600, and records path→canary in `bait.json`; the simple absolute-path-read + DNS case is detected (probe `/run/probes/p2b`), so the intended control still works when the network path happens to be observed.
- **Read-only pipeline and app.** `/pipeline` and `/opt/basecamp/Basecamp.AppImage` are mounted read-only (`reviewer.sh:33-35`); the sandbox check confirms they cannot be changed, and the image is not the host home.
- **Dependency integrity check.** `fetch_deps.py` verifies each downloaded dependency against the catalogue's SHA-256 before use and caches by hash (lines 51-57), so a corrupted mirror does not silently install.
- **Model-call logging and retry.** `gateway.py` logs every request/reply with token usage, retries provider 5xx/429 with backoff, and redacts known key patterns; the cost ledger and budget concept are present even though the enforcement is bypassable.
- **Static checks are present and the baseline is clean.** gitleaks, osv-scanner and semgrep run before the review; gitleaks found only the intentional fake-key template, osv found no vulnerable dependency metadata, and semgrep flagged the `curl | bash` supply-chain line I raised in FR-13.
- **Kill-by-PID discipline.** The newer scripts avoid `pkill -f` for the reviewer and record PIDs (`review-stop.sh`, `review-in-sandbox.sh`); the prompt and desk recorder were changed accordingly.

## 5. What I did not test

- **No Basecamp module run.** The target is the review harness repository, not a Basecamp module: there is no `metadata.json`, no `module.lgx` and no `/run/lgx`, so there was nothing to install in Basecamp. No module screens, input attacks through the UI, screenshots or session video could be produced; `desk start` was not used for a module. The `desk install` path was exercised directly through `install_one` (FR-8).
- **Host-only components.** No `podman`, no Caddy, no shrooms mesh, no `/home/reviewer/.pi/agent/.env`, and no real Venice key were available. Host scripts (`host/*.sh`), the Caddy serving behaviour, a full `review.sh` end-to-end run, and the report-pass timing were reviewed statically, not executed.
- **A real end-to-end review** (clone → checks → sandbox → publish) was not run; it needs the host, image and key. The probes exercised the individual components with local substitutes.
- **Nested `strace`** was denied (`PTRACE_TRACEME: Operation not permitted`), so `analyse.py` tests used syscall-accurate synthetic logs plus the live global capture; I did not run the analyser over a fresh per-process trace of a complete module.
- **Container escape and kernel-level attacks.** No attempt to break out of the sandbox; FR-1 is a data-flow escape (symlinks), not a kernel exploit.
- **DNS parser fuzzing / `dns_sink.py`** pointers and malformed packets, `disguise.sh`/`bait.sh` on a real podman host, `render.py` with very large media/evidence trees, and Caddy path handling.
- **The `test-modules/leaky-build` sample** was run directly to observe its reads (the published behaviour record will show a bait read from this probe, not from the reviewed target); its outbound POST was not completed against a live TLS sink (there is no TLS MITM here).

## 6. Suggested order

1. **Stop host-file disclosure and evidence forgery (FR-1, FR-2).** This is the product's trust boundary: no symlinks, no module-writable evidence/report, host-side capture. Nothing else matters if the published report can be replaced or the key exfiltrated.
2. **Make the network record real (FR-3).** Wire the resolver, enforce egress, and report raw-IP connects; add a sandbox self-test that proves a lookup and a literal-IP connect are recorded.
3. **Remove shell injection from untrusted inputs (FR-4)** and fix the install path traversal (FR-8); validate URLs and manifests as data.
4. **Repair budget enforcement (FR-5)** with a model allowlist, price reservation, and secret-free token delivery; cap gateway bodies/threads (FR-12).
5. **Fix the review lifecycle (FR-6) and scanner trust (FR-7):** align the outer timeout, make a missing/empty scan a failure, and use host-owned scanner configs.
6. **Harden publication (FR-9, FR-15) and align the documentation (FR-10, FR-14):** schema validation, always-written index, TLS/signature, and a clean report context.
7. **Housekeeping (FR-11, FR-13):** correct semver, pin the supply chain.

---
Triage, not approval — this never certifies a module as safe. The sandbox is a container; its kernel is the host's and could be detected.