|
| 1 | +# RustySNES `v1.26.0` "Bulwark" |
| 2 | + |
| 3 | +**Released:** 2026-07-31 · **Tag commit:** `ff9379e` · **Previous release:** [`v1.25.0` "Workbench"](https://github.com/doublegate/RustySNES/releases/tag/v1.25.0) |
| 4 | + |
| 5 | +> The first rung after the frontend-parity program closed. Nothing here changes what the emulator |
| 6 | +> computes — every change is to the machinery that proves it, protects it, or measures it. |
| 7 | +> **Zero accuracy risk by construction:** no chip model was touched. |
| 8 | +
|
| 9 | +--- |
| 10 | + |
| 11 | +## Executive summary |
| 12 | + |
| 13 | +`v1.26.0` builds the defences the project had specified but never built. Three of them: |
| 14 | + |
| 15 | +1. **A fuzzing layer that did not exist.** `docs/testing-strategy.md` had named Layer 1 |
| 16 | + fuzzability as an intended property since it was written, and no `fuzz/` directory had ever |
| 17 | + existed. Fourteen `cargo-fuzz` targets now cover every untrusted-input boundary in the tree. |
| 18 | + The first campaign found a **real, reachable heap-exhaustion defect in ROM header parsing**. |
| 19 | +2. **CI supply-chain hardening.** 15 of 17 `actions/checkout` sites stopped persisting the |
| 20 | + workflow token; all 34 third-party action references are pinned to immutable commit SHAs; and |
| 21 | + the auto-release tag-existence check now fails closed instead of treating a network blip as |
| 22 | + "not yet released". |
| 23 | +3. **The golden vectors were reachable from no pull-request job.** 53 rendered-scene goldens plus |
| 24 | + every framebuffer golden ran only on `main`. A PR could move a golden and hear nothing about it |
| 25 | + until after merge. |
| 26 | + |
| 27 | +Plus one measurement that produced a *negative* result and is published as such: run-ahead stays |
| 28 | +opt-in, and the reason is now a number rather than an assumption. |
| 29 | + |
| 30 | +| | | |
| 31 | +|---|---| |
| 32 | +| Commits | 5 (PRs #273–#277) | |
| 33 | +| Diff | 46 files changed, +6,319 / −70 | |
| 34 | +| AccuracySNES coverage | **344 of 443** (291 on-cart + 53 scene) — unchanged, by design | |
| 35 | +| Save-state format version | unchanged | |
| 36 | +| Public API | unchanged | |
| 37 | + |
| 38 | +--- |
| 39 | + |
| 40 | +## Headline 1 — the fuzzing layer, and the defect it found within twenty seconds |
| 41 | + |
| 42 | +### The infrastructure |
| 43 | + |
| 44 | +`fuzz/` is a **separate cargo workspace** (its own `Cargo.toml` with an empty `[workspace]` table), |
| 45 | +so cargo-fuzz's nightly requirement never leaks into an ordinary `cargo check --workspace` and the |
| 46 | +targets never appear in the default build graph. |
| 47 | + |
| 48 | +Fourteen targets, one per untrusted-input boundary: |
| 49 | + |
| 50 | +| Target | Entry point under test | |
| 51 | +|---|---| |
| 52 | +| `rom_header` | `Header::detect` | |
| 53 | +| `rom_load` | `EmuCore::load_rom`, including the third-party `zip` container path | |
| 54 | +| `save_state` | `System::load_state` via `rustysnes-savestate`'s tagged/length-prefixed reader | |
| 55 | +| `movie` | `Movie::deserialize` | |
| 56 | +| `netplay_message` | `NetMessage::decode` | |
| 57 | +| `patch` | IPS / UPS / BPS application | |
| 58 | +| `cheat_code` | Game Genie + Pro Action Replay decoding | |
| 59 | +| `hd_pack_manifest` | manifest TOML, tile-path resolution, RGBA conversion | |
| 60 | +| `slang_preset` | preset parsing + the GLSL→WGSL bridge and its string rewriters | |
| 61 | +| `config_toml` | `Config` serde-derive recursion | |
| 62 | +| `symbols` | `SymbolMap::load` — returns no `Result` at all by design, so panic-freedom *is* the contract | |
| 63 | +| `coproc_firmware` | user-supplied DSP-1 / CX4 chip-ROM dumps | |
| 64 | +| `cpu_step`, `apu_reg_io` | seeded chip state driven by a register-write stream | |
| 65 | + |
| 66 | +Every one of those entry points already returns a `Result`. What is under test is therefore |
| 67 | +**panic-freedom, unbounded allocation, and slice-index arithmetic** — not missing error handling. |
| 68 | + |
| 69 | +**CI split, deliberately.** A *compile gate* runs on every PR (`ci.yml`'s `lint` job) so a target |
| 70 | +cannot silently rot out of buildability as the API it calls moves. The *campaign* is weekly |
| 71 | +(`security.yml`, scheduled only), because per-commit fuzzing mostly re-treads a corpus it already |
| 72 | +has. |
| 73 | + |
| 74 | +### The defect: `$xFD8` shifted unbounded |
| 75 | + |
| 76 | +`Header::detect` computed SRAM size as `1024 << N`, with `N` an arbitrary byte lifted straight out |
| 77 | +of an untrusted image. |
| 78 | + |
| 79 | +- **Debug builds panicked outright** (`attempt to shift left with overflow`) for `N >= 64`. |
| 80 | +- **Release builds masked the shift instead** — so `N = 22` silently handed `board::select` a |
| 81 | + **4 GiB zeroed allocation** at `vec![0u8; header.sram_size]`. Reachable from any downloaded ROM, |
| 82 | + any fan hack, any bad dump. |
| 83 | +- **`wasm32` was worse on both counts:** `usize` is 32 bits there, so the panic started at |
| 84 | + `N >= 32` and `N = 21` already exceeded `isize::MAX`. |
| 85 | + |
| 86 | +The fix clamps to `MAX_SRAM_SIZE` (512 KiB) **with the shift amount capped first** — both bounds |
| 87 | +are required, since a `min` on the *result* cannot run until the shift has already happened. |
| 88 | + |
| 89 | +512 KiB is the smallest value that cannot refuse a real cartridge: LoROM's SRAM window reaches |
| 90 | +448 KiB, HiROM's 256 KiB, and SA-1 BW-RAM tops out at 256 KiB. A header claiming more is describing |
| 91 | +memory the console cannot address. **Clamping rather than rejecting is deliberate** — the field is |
| 92 | +an allocation hint, every board already wraps its accesses to `sram_size`, and real dumps do carry |
| 93 | +garbage there. |
| 94 | + |
| 95 | +Pinned by `a_forged_ram_size_byte_is_clamped_not_shifted_unbounded` (all 256 byte values) with |
| 96 | +`the_sram_clamp_leaves_every_real_cartridge_size_untouched` as its negative control, and verified by |
| 97 | +re-injecting the bug and confirming *those* tests fail and no others. |
| 98 | + |
| 99 | +**How it was reached matters as much as the fix.** The defect is invisible without a seeded corpus. |
| 100 | +Header detection scores candidate offsets, so a random image essentially never scores above zero — |
| 101 | +unseeded, `rom_header` plateaus around 29 edges and never reaches this code at all. Seeded from the |
| 102 | +committed permissive ROM corpus, it surfaced in under twenty seconds. |
| 103 | + |
| 104 | +--- |
| 105 | + |
| 106 | +## Headline 2 — CI supply-chain hardening |
| 107 | + |
| 108 | +Three items. The first two carry over from the sibling project's `v2.2.2` audit; the third was |
| 109 | +raised in review of the fuzzing PR. |
| 110 | + |
| 111 | +### `persist-credentials: false` on 15 of 17 checkouts |
| 112 | + |
| 113 | +`actions/checkout` writes the workflow `GITHUB_TOKEN` into `.git/config`, where **anything the job |
| 114 | +then executes from the tree** — build scripts, proc macros, test binaries, `scripts/*.sh`, the |
| 115 | +MkDocs build — can read it. On a pull request that tree is by definition unreviewed code, and |
| 116 | +nearly every job here compiles or runs it. |
| 117 | + |
| 118 | +Highest exposure was `web.yml`'s `build`, which holds `pages: write` + `id-token: write` while |
| 119 | +running `trunk`, `cargo doc`, `pip install`, and `mkdocs build`. |
| 120 | + |
| 121 | +**Audited per site rather than applied blanket, and that mattered.** `release-auto.yml`'s `prepare` |
| 122 | +job *genuinely needs* the credential: it creates an annotated tag and runs `git push origin "$TAG"`, |
| 123 | +which authenticates through exactly that token. The sibling's audit concluded no job needed it; |
| 124 | +that conclusion does not transfer, and a blanket sweep would have broken every future release. It |
| 125 | +is now the only checkout in the repository that keeps the token, with the reason recorded inline, |
| 126 | +and its exposure is bounded by its trigger (`workflow_run` on a completed CI run of `main`, never a |
| 127 | +pull request). |
| 128 | + |
| 129 | +### The release-tag existence check now fails closed |
| 130 | + |
| 131 | +It was: |
| 132 | + |
| 133 | +```bash |
| 134 | +git ls-remote --exit-code --tags origin "refs/tags/${tag}" |
| 135 | +``` |
| 136 | + |
| 137 | +which collapses **three** outcomes into two — tag present, tag absent, and *lookup failed* — reading |
| 138 | +any non-zero exit as "absent". A transient network blip, auth hiccup, or rate limit therefore sent |
| 139 | +an **already-released version** down the `should_release=true` path, re-tagging a release the |
| 140 | +ceremony treats as immutable. |
| 141 | + |
| 142 | +It is now `gh api git/matching-refs/tags/<tag>`, chosen over `git/ref/tags/<tag>` because it answers |
| 143 | +"absent" with HTTP 200 and an empty array — so a genuine miss can never be confused with an error, |
| 144 | +and no error-body parsing is needed. That endpoint matches by **prefix**, so the exact ref is |
| 145 | +compared in `jq`. Verified necessary, not theoretical: **`v1.2` prefix-matches 7 real tags while |
| 146 | +exact-matching none**, and a naive `length > 0` would have reported it as already released. Every |
| 147 | +failure path aborts the job under `set -euo pipefail`. |
| 148 | + |
| 149 | +### All 34 third-party action references pinned to commit SHAs |
| 150 | + |
| 151 | +Across all 8 workflows plus the composite `rust-setup` action. An upstream tag move could |
| 152 | +previously change what CI executes without any repository review. Each pin carries its original tag |
| 153 | +as a trailing comment so the intent stays readable. One stray `actions/upload-artifact@v4` |
| 154 | +(introduced with the fuzz job) was aligned to the `@v7` the rest of the repo already used. |
| 155 | + |
| 156 | +Verified: `actionlint` reports the **same three findings before and after** (a self-hosted runner |
| 157 | +label and two pre-existing shellcheck notes) — no new lint surface; all nine files parse. |
| 158 | + |
| 159 | +--- |
| 160 | + |
| 161 | +## Headline 3 — the golden vectors ran in no pull-request job |
| 162 | + |
| 163 | +Only `lint`, `test-light`, and `accuracysnes` run on a PR. `test-light` never passes |
| 164 | +`--features test-roms`, and the `accuracysnes` job scoped itself to `--test accuracysnes`. So the |
| 165 | +**53 rendered-scene goldens and every framebuffer golden** were reached only by `full-test`, which |
| 166 | +runs on `main`, on tags, and on the weekly cron. |
| 167 | + |
| 168 | +A PR could shift PPU behaviour, move a golden, and hear nothing about it until merge-to-main. |
| 169 | + |
| 170 | +That is the same structure behind this project's own coprocessor-golden staleness (goldens left |
| 171 | +stale by post-bless PPU accuracy fixes): **a golden vector nothing executes only accumulates |
| 172 | +drift.** Here it was not that nothing ran them ever, but that nothing ran them while it was still |
| 173 | +cheap to fix. |
| 174 | + |
| 175 | +The `accuracysnes` job now also runs the scene, undisbeliever, rainwarrior, coprocessor, and |
| 176 | +save-state suites — roughly 5 minutes for the three with a committed corpus. The `*_oncart` suites |
| 177 | +need gitignored dumps and self-skip for free; they are listed anyway so the intent is explicit. |
| 178 | + |
| 179 | +**Found while looking for something else.** The search was for RustySNES's analogue of the sibling's |
| 180 | +`expansion_level_tripwire`. That mechanism does *not* port: the only `commercial-roms`-gated suite |
| 181 | +here (`commercial_screenshots`) is a screenshot generator that asserts nothing, so it holds no |
| 182 | +golden that can go stale, and PPU accuracy is not parameterised by a small constant set the way |
| 183 | +expansion-audio levels are. Looking for it surfaced a real and larger gap instead. |
| 184 | + |
| 185 | +--- |
| 186 | + |
| 187 | +## Measured, and the answer was no: run-ahead stays opt-in |
| 188 | + |
| 189 | +`docs/frontend.md` recorded the per-frame save-state allocation as *the* blocker on making |
| 190 | +run-ahead default-on. `v1.25.0` removed that allocation, so the question was re-measured. |
| 191 | + |
| 192 | +Absolute timings are from one development machine and will differ elsewhere. They are recorded |
| 193 | +because the **ratios** are what decide this, and a reader can only check a ratio by re-running the |
| 194 | +same two benches (`save_state_cost`, `headless_frame`) and comparing. The percentages are the |
| 195 | +portable part; the microseconds are the working. |
| 196 | + |
| 197 | +| | cost | |
| 198 | +|---|---:| |
| 199 | +| `save_state` | ~119 µs | |
| 200 | +| `load_state` | ~285 µs | |
| 201 | +| **save/load round trip** | **~0.40 ms** | |
| 202 | +| one emulated frame (`headless_frame_steady_state`) | **6.39 ms** | |
| 203 | +| NTSC frame budget | 16.64 ms | |
| 204 | + |
| 205 | +The round trip is **2.4% of the budget** — it was never the dominant cost, and removing the |
| 206 | +allocation did not move the decision. What run-ahead costs is the **extra frame of emulation**, |
| 207 | +which is inherent: `frames = 1` needs **79%** of the NTSC budget on a fast development machine, |
| 208 | +leaving ~3.5 ms for present, UI, and audio; `frames = 2` needs **118%** and cannot hold 60 fps at |
| 209 | +all. Defaulting that on would miss deadlines on ordinary hardware to buy latency nobody asked for. |
| 210 | + |
| 211 | +**One real bug fell out of the measurement.** `RunAheadConfig`'s `Default` is now hand-written so |
| 212 | +the throttle is armed. A derived `Default` gave `throttle_ms: 0.0`, which *disables* the throttle — |
| 213 | +leaving the safety net off for exactly the user who had just enabled run-ahead from Settings and had |
| 214 | +no `throttle_ms` line in their `config.toml`. It now defaults to 14 ms (below the 16.64 ms NTSC |
| 215 | +deadline with headroom, and conservative against PAL's 20 ms). An existing config that spells the |
| 216 | +field out keeps its value; `#[serde(default)]` fills only absent fields, pinned by a negative-control |
| 217 | +test. |
| 218 | + |
| 219 | +--- |
| 220 | + |
| 221 | +## Compatibility and upgrade notes |
| 222 | + |
| 223 | +- **Save states:** format version unchanged. States written by `v1.25.0` load unmodified. |
| 224 | +- **Config:** `run_ahead.throttle_ms` now defaults to `14.0` when the key is **absent**. A config |
| 225 | + that already spells the field out is untouched. If you had deliberately relied on the absent-key |
| 226 | + behaviour to disable the throttle, set `throttle_ms = 0.0` explicitly. |
| 227 | +- **Emulation behaviour:** unchanged. No chip model, timing constant, or golden vector moved. |
| 228 | +- **Build:** `fuzz/` is a separate workspace and does not affect `cargo build --workspace`. Running |
| 229 | + the targets yourself needs a nightly toolchain and `cargo-fuzz`. |
| 230 | + |
| 231 | +## Verification |
| 232 | + |
| 233 | +```bash |
| 234 | +cargo fmt --check |
| 235 | +cargo clippy --workspace --all-targets -- -D warnings |
| 236 | +cargo test --workspace |
| 237 | +cargo test --workspace --features test-roms |
| 238 | +RUSTDOCFLAGS="-D warnings" cargo doc --workspace --no-deps |
| 239 | +cargo build -p rustysnes-core --target thumbv7em-none-eabihf --no-default-features |
| 240 | +cargo +nightly fuzz build --manifest-path fuzz/Cargo.toml # all 14 targets |
| 241 | +``` |
| 242 | + |
| 243 | +- All 14 fuzz targets build; each run to a bounded `-max_total_time` locally with zero crashes |
| 244 | + beyond the `rom_header` finding fixed in this release. |
| 245 | +- `actionlint` finding count unchanged (3 pre-existing) across all 9 workflow files. |
| 246 | +- AccuracySNES battery and all framebuffer goldens unmoved. |
| 247 | + |
| 248 | +## Included changes |
| 249 | + |
| 250 | +| PR | Commit | Summary | |
| 251 | +|---|---|---| |
| 252 | +| #273 | `55bea3c` | `test(reviewer)`: guard the comment-selection filter that has broken twice | |
| 253 | +| #274 | `55f3831` | `docs(readme)`: enumerate the 17 releases the Roadmap had collapsed | |
| 254 | +| #275 | `0b18e60` | `test(fuzz)`: build the fuzzing infrastructure and fix the defect it found | |
| 255 | +| #276 | `81baef5` | `ci(security)`: harden checkout credentials, pin actions, fail-closed tag check | |
| 256 | +| #277 | `ff9379e` | `ci(test)`: gate the goldens on PRs; keep run-ahead opt-in on measurement | |
| 257 | + |
| 258 | +Full per-entry detail: [`CHANGELOG.md` → `[1.26.0]`](../../CHANGELOG.md). |
0 commit comments