# Reject malformed projects at load

Branch: `harden-load` → `main`

PR 1 of the review fixes plan (`docs/review-fixes-plan.md`). Part of 0.5.2; the version does not change in this PR.

## What changed

**Sample names stay inside the project (S1, load side).** Loading a project now refuses it if any sample file name in the pool is not a plain file name. A plain name is not empty, not `.` or `..`, and has no `/`, `\` or control characters. The same rule applies to each pad layer's `sampleFile`, which must be text or missing. Before, a pool `path` of `../../../secret.txt` was joined to the data folder as it was. The error names the bad entry, e.g. `samples[0].path: file name "../x.wav" contains a folder separator`. The rule is `project.PlainFileName`, which PR 3 reuses at export.

**Missing fields no longer crash (S3).**
- Load refuses a pool entry without a text `path`. It also refuses a sequence without a whole-number `key`, an object `value`, a number `bpm`, a whole-number `lengthBars` or a text `name`. Each error names the entry, e.g. `sequences[0]: bpm is missing or not a number`. Before, the first two cases were nil-pointer panics.
- `xpj.Node.Int` and `Float` return an error on a nil node, and `Get` on a nil node returns an error. `Lookup` was already nil-safe. In `project`, the remaining direct `.Text` and `.Float()` uses on lookups now go through `text()`, `num()` and a new `float()`.
- A missing `data.version` now gives the warning "project data version is missing" instead of "version 0".
- `main.go` sets Wails's `PanicHandler`. A panic is logged with its stack and the window shows "Something went wrong inside MPCkit, and the last action may not have finished." Without the handler, Wails exits the app on a panic in an event handler such as file drop.
- The preview audio handler now releases the project lock with `defer`, so a panic there cannot leave the lock held and freeze every later call. This was not in the plan, but it follows from the same finding.

**Main nodes found once (M2).** `wrap` finds `samples`, `tracks[0].samples`, `tracks[0].program.drum.instruments` and `sequences` once and stores them on `Project`. `pool()`, `trackPool()`, `pads()` and `sequences()` return the stored nodes instead of looking them up again with the error dropped.

**Tests.**
- `TestLoadRejectsMalformed` builds 16 broken copies of DemoFile in `t.TempDir()` and checks each error. The cases cover pool paths `../x.wav`, `a/b.wav`, `..\b.wav`, `..`, an empty name, a control character, a bad track pool entry, and a missing or non-text `path`. They also cover a bad layer `sampleFile`, and a sequence missing `bpm`, `key`, `value` or `lengthBars`, or with a non-text name.
- Other new tests cover a layer with no `sampleFile` (still loads), a missing `data.version` (warning), `PlainFileName`, and `xpj` getters on a nil node.
- `FuzzLoad` is the fuzz target the plan describes.
- `TestBrokenFieldsDoNotPanic` breaks DemoFile's fields one at a time, then runs every read and most edits on the result. None may panic. It covers 162 fields and 1,134 cases, and takes about 30 seconds on the server; `go test -short` skips it. See "Fuzzing" below for why it was added.
- Against the old code, the path cases load without error and the missing-`path` case panics.

## Notes

Choices the plan left open. Say if any should change:
- **The track pool is checked too.** `tracks[0].samples` holds the same entries as `samples`, so its `path`s get the same check.
- **Empty `sampleFile` is allowed.** The device writes `""` on every empty pad layer (1,018 of them in DemoFile), so an empty name means "no sample" and only non-empty names must be plain.
- **Whole numbers only.** `key` and `lengthBars` must be whole numbers, since both are used as integers. `bpm` may be any number.
- **Not checked, because the plan doesn't ask and neither can panic:** a sequence `key` outside 0–127, duplicate keys, and duplicate pool names.
- **Two messages after a panic in a bound method.** Wails already turns such a panic into a failed call, and the frontend shows that error. With the handler set, the handler's message is also sent, and both go to the same banner, so the last one shows.
- **The fuzzer's progress line is wrong for this target.** It shows `execs: 9 (0/sec)` for most of the run. The fuzz function itself counted 42,097 calls in 15 seconds, so the 2-minute run made about 300,000.

**Fuzzing.** `FuzzLoad` barely reaches the project code. The seeds are gzip files, and gzip checks a CRC at the end of the stream, so almost every mutated input fails at decompression. In a 15-second sample:
- 5,885 inputs ran.
- 5,882 failed in gzip or flate.
- Only the 3 unchanged seeds loaded.

Fuzzing the decompressed JSON does not help either: each project is about 3 MB uncompressed, too big for Go's fuzzer to mutate usefully.

So I first ran a full sweep, not committed:
- It changed each of 1,818 fields that `project` reads in DemoFile in 7 ways: null, text, -1, 1000, an empty list, an empty object, or deleted.
- After each change, it ran every read and edit.
- Result: 12,726 cases, 0 panics, in about 10 minutes.

bajdev chose to keep `FuzzLoad` and also commit a cut-down sweep that runs in under a minute: `TestBrokenFieldsDoNotPanic`.
- It changes only the first item of each list. For pads it covers the first pad's own fields, its layers and `sliceInfo`, rather than every field below the pad.
- It checks that nothing panics, not that the file is refused. Some changes load and are handled as an empty value.
- With the load checks and nil guards turned off, it reports 67 panics; with them, none.

## Testing

**On the server:**
- `go test ./...` passes, including the round-trip and golden tests, unchanged.
- `go vet ./...` and `staticcheck ./...` are clean.
- `govulncheck ./...` reports only GO-2026-6859 in the standard library's `net/http`. That needs Go 1.27.4 (PR 4).
- `go test -fuzz=FuzzLoad -fuzztime=2m ./internal/project` found nothing.
- `TestBrokenFieldsDoNotPanic` passes in about 30 seconds.
- `npm install`, `npm run check` (0 errors, 0 warnings) and `npm run build` pass. The lockfile `npm install` wrote was not committed.
- `wails3 build` builds.
- `timeout 15 xvfb-run -a bin/mpckit` starts and runs. The log holds only the usual headless GTK warnings.

**On the laptop (PowerShell, in the repo):**
1. `git fetch; git checkout harden-load`
2. `go test ./internal/project ./internal/xpj`: both `ok` (the project tests take about half a minute). On Windows, `filepath` also treats `\` and drive letters as path parts, so this checks the name rule there.
3. `wails3 dev`: load `testdata\device\DemoFile.xpj`. It opens as before: pads A1–A4, B1 and B2 filled; sequences Intro and Sequence 02.
4. Load `testdata\accepted\MpckitT1.xpj`, then `MpckitT3.xpj`. Both open with no error or warning.
5. Drop a WAV on a pad, change its tune and mute group, then add and remove a sequence. All work as before.