# 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.
- 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.

**Question for bajdev: `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.

To test the code anyway, I ran a throwaway 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: Pad, PadHasNotes, Sequences, PoolFiles, tune, mute group, copy, move, swap, clear, sequence edits, and Encode.
- Result: 12,726 cases, 0 panics. It takes about 10 minutes.

Options:
1. Keep `FuzzLoad` as it is (it meets the plan).
2. Also commit the sweep as a test, cut down to run in under a minute, e.g. the first pad and sequence only.
3. Replace `FuzzLoad` with a fuzz target that changes the decoded tree rather than the gzip bytes.

I'd suggest 2. It is a few lines in a follow-up PR or in this one if you prefer.

## 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.
- `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`. 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.