Skip to content

AUDIT_2026-09-01.md

Latest commit

 

History

History
490 lines (376 loc) · 27.8 KB

AUDIT_2026-09-01.md

File metadata and controls

490 lines (376 loc) · 27.8 KB
Raw

Audit — repository, CI and defects (2026-09-01)

Run against 15b0fb8 (main, working tree clean at the start). Three questions:

  1. Is what the project says about itself true?
  2. What is actually broken in the code?
  3. Does this repository read like a project a stranger would trust their only Walkman to?

Every claim below was executed, not inferred, unless it says otherwise. Where a defect's mechanism is certain but its trigger is unmeasured, the row says which is which — the distinction this project already makes between device-verified and device-unverified, applied to desk findings.

Summary
main went red TWICE today 15b0fb8 (pushed direct, -Wformat-truncation) and b22202b (a dependabot PR merged over a failing check, cinder-ffi would not compile). Both fixed here.
The gate exists and is not required CI ran on both and said no. Nothing on main requires a passing check — that is the root cause of both, and it is §D5.
2 real defects found Both in the guard/watchdog machinery added today; one could hang the app permanently. Fixed, with a self-test that fails on the old code.
2 documentation claims were false The README promised a Linux installer that had not shipped since v0.1.2 and could not have completed an install. Now published, and honest about where it stops.
1 structural problem, half fixed .git is 1.3 GB, 863 MB of it committed ARM binaries. dist/dev/ is untracked from here (74 of the 117 revisions); recovering the existing 863 MB needs a history rewrite, which is the maintainer's call — §D1.
Everything is green now 429 Rust tests, 46 launcher-recovery cases, 9 guard self-tests, all harness scenarios, shellcheck clean, zero dependency vulnerabilities — and a full ARM cross-build at GLIBC ≤ 2.18, which is the gate no runner can run.

Part A — Defects

A1. main was red on GitHub — a truncated buffer in the fault path — FIXED

cinder-home/src/main.cpp:325. The guard-recovery message was widened today from "GUARD RECOVERED: %s" to a 209-byte string, and its buffer was widened from 128 to 192. So every one of these lines was truncated mid-sentence:

error: 'snprintf' will always be truncated; specified size is 192,
       but format string expands to at least 208 [-Werror,-Wformat-truncation]

This is not a lint nit. It is the single most important line the fault path prints — the one that tells you Sony IPC has died for the boot and that audio and Bluetooth need a restart — and its tail was being cut off. tools/host_syntax_check.sh caught it; the GitHub run for 15b0fb8 failed on it.

Fixed: buffer to 320, with a comment saying why the number is what it is. All 20 files parse.

A2. run_guarded wrapped work that is not Sony IPC, and a timeout there leaked a locked Rust mutex — FIXED

run_guarded exists for one failure mode: a Sony IPC call that never returns. Its recovery is a siglongjmp out of the call, and — as of today's commit — permanently setting g_ipc_dead, on the correct reasoning that the abandoned call left a half-built std::string inside a closed client.

Four call sites are not Sony IPC:

Site What it actually does
main.cpp:612 cinder_db_open — SQLite over ~3,500 tracks. 25 s. Boot path.
main.cpp:9192 reclaim_contents — system("cinder-msc off"), a mount, then cinder_db_open again. 30 s.
main.cpp:785 statvfs. The comment says "guarded for parity", which is honest and is the smell.
main.cpp:812, :846 system() shell-outs, dev channel only.

Two of those take the Rust render lock. cinder_db_open holds cell().lock().unwrap() — a std::sync::Mutex<Option<Render>> — across Db::open, build_library, the playlist store, the liked list and the whole likes import (lib.rs:4562-4630).

A siglongjmp past a MutexGuard does not run its destructor. The mutex stays locked, with no owner, for the life of the process. The next frame calls into cinder-ffi, blocks on it, and never comes back — so the per-frame watchdog fires, latches the bad-boot counter and _exit(42). On the boot-path site that is a boot to stock, caused by a library that was merely slow.

Mechanism: certain. siglongjmp skipping Rust destructors is not in question, and the guard is what performs it. Trigger: unmeasured. Nothing has timed cinder_db_open against a cold vfat /contents under USB-MSC contention, which is exactly the condition site 9192 runs in. The 25 s and 30 s budgets may be comfortable — nobody has checked, and the file's own note says the art cache's first build across a 304-album library is unmeasured.

Fixed by splitting the guard three ways, on the question a call site actually has to answer — not "might this be slow" but "if this call is abandoned mid-flight, what does it leave behind?"

Kind Leaves behind Behaviour Sites
run_guarded (GUARD_IPC) a half-built container in a closed Sony client unchanged: recover once, then g_ipc_dead 102
run_guarded_local (GUARD_LOCAL) nothing recover and carry on; never touches g_ipc_dead in either direction 1 (statvfs)
run_watchdog_only (GUARD_FATAL) a held Rust mutex; an unreaped child; system()'s swapped SIGINT/SIGQUIT no sigsetjmp at all — a hang falls through fault_handler's un-guarded path: name the call, latch, _exit(42), escape ladder 4

GUARD_FATAL is the part worth dwelling on. Not unwinding looks like the weaker option and is the stronger one: the old path ended in _exit(42) anyway, just two watchdog cycles later, with g_ipc_dead set as a red herring and the wrong thread's stack in the log. A clean, immediate, correctly-labelled exit into a ladder built to absorb exactly this is strictly better than a silent freeze. fault_handler now prints the stuck call's label via write(2) before the fault dump.

The DB budget went 25 s → 45 s and the reclaim's 30 s → 60 s, because the consequence of overrunning is now a hard exit rather than a skipped subsystem, and neither has ever been timed. The retry ladders are unaffected: they are driven by the return code — a missing /contents makes cinder_db_open return non-zero promptly — not by the timeout.

This is the rule already written down in this project as "an escape must depend on less than what it rescues", applied one level down: a guard must not be able to break more than the call it is guarding.

Tested. The harness cannot reach this — its clock is virtual and alarm() is wall-clock, so no scenario can make a real recovery happen. It is pure logic, so it went into cinder-home/tools/guard_selftest.cpp, which CI already runs. That file went from 4 tests to 9; the 5 new ones produce 6 failures against the pre-split logic and none against the new.

A3. g_ipc_dead refused work that has nothing to do with Sony IPC — FIXED

The mirror of A2, and it follows from the same conflation. run_guarded returns -1 immediately when g_ipc_dead is set. That flag is now permanent for the boot. So after any guard recovery — one slow NextTrack is enough — the following can never run again:

  • /contents can never be reclaimed (site 9192). Sony's stack unmounts the music volume when a cable appears; the reclaim that undoes this is refused. The user gets an empty library, grey album art and a scrobbler writing to an unlinked inode, for the rest of the boot, with no way back except a reboot.
  • Storage figures in Settings stop updating (site 785).

The user-visible outcome is "I plugged it into my PC and all my music vanished", reached from a transport timeout that the guard successfully recovered from.

Fixed by the same split (A2): only GUARD_IPC consults g_ipc_dead. Self-tests 6 and 7 pin it — with Sony IPC dead, statvfs and the /contents reclaim both still run.

A4. The README promised a Linux installer that does not exist — FIXED

Two claims, both false:

  • "There's a Linux build too." — no Linux artifact has been attached to a release since v0.1.2 (2026-08-20). release.yml has a single windows-latest build job.
  • "release.yml … builds the Windows and Linux installers" — it does not.

And it could not usefully, as written: run_sony_updater() is #[cfg(not(windows))] → Err(Unsupported), so finish_install stages the payload, prints ERROR: could not start the Sony updater, and exit(1). A published Linux binary would report failure on every successful run.

Fixed by making the build honest and then publishing it, rather than by withdrawing the promise — because ci.yml had been building and testing the Linux installer on every push the whole time. The only things ever missing were an honest exit code and an upload step.

  • run_sony_updater() off Windows now returns Ok(()) instead of Err(Unsupported). The staging is the useful half and it always worked; only the handoff is impossible.
  • print_next_steps() has a #[cfg(not(windows))] arm that says plainly this is as far as a non-Windows build can go, then gives the three manual steps that finish the job (eject cleanly, then Settings ▸ Device Settings ▸ Update on the player), and points at the Windows one-click build as the easier path.
  • release.yml gains a build-linux job; release now needs both. The release body and the README describe exactly where it stops.

An installer that stops halfway and does not say so is worse than one that never ran. This one now says so.

A5. The dependency gate added in this pass immediately broke main — FIXED, and it is the best evidence in this document

Worth reading as a whole, because it demonstrates §D5 better than any argument could.

.github/dependabot.yml was added in this pass (§C3), committed, and pushed. Dependabot picked it up within minutes and opened two PRs. Both were merged the same hour. The second, #8, bumped six player crates — three of them across a semver-major boundary, straight through the device's own code paths:

Crate From To Reaches
zune-jpeg 0.4.21 0.5.15 album-art JPEG decode
png 0.17.16 0.18.1 album-art PNG decode
rusqlite 0.32.1 0.40.2 the media library — and its bundled SQLite C source
fontdue / libc / minifb semver-compatible

CI ran on that PR and went red. Both decoders changed their reader traits — zune-jpeg 0.5 takes T: ZByteReaderTrait where 0.4 took &[u8], png 0.18 requires Read + Seek — and cinder-ffi would not compile. Eight errors. It was merged anyway, and main did not build for the next hour.

Two distinct failures, and the second is the serious one:

  1. A red check did not stop a merge. That is §D5, and it is the same root cause as A1.
  2. rusqlite is a change no gate in this repository can validate. cinder-home/build.sh is the only thing on earth that checks this tree against glibc 2.23. The device is glibc 2.23 (2016); rusqlite's bundled feature compiles SQLite from vendored C; and build.sh's own header warns that C must be built against the 2.23 headers with LFS off or it emits *_time64/stat64/ fcntl64 references the device's ld-2.23 refuses to resolve. No hosted runner does that link — deliberately; it needs a cross toolchain and a xenial armhf sysroot. So an automated rusqlite bump can only fail on a maintainer's machine, or on the player.

Resolved, and the bump kept — because it was actually verified rather than assumed:

  • The API breaks are fixed (ZCursor for zune-jpeg, io::Cursor for png). One of them is a genuine improvement worth noting: png 0.18's output_buffer_size() returns Option<usize> because it now reports overflow instead of wrapping; ? on it is a second net under the dimension gate, and 0.17 would have allocated on the wrapped value.
  • cinder-home/build.sh stable was run in full, which is the gate CI cannot be: OK: cinder-home GLIBC needs = GLIBC_2.4 2.9 2.12 2.15 2.17 2.18 — every one under 2.23. The qemu construction preflight passed, the 46-case launcher matrix passed, and dist/stable/ was restaged so the committed payload matches the source it was built from.

dependabot.yml was then rewritten so it can only propose what CI can judge: semver-major updates are ignored for every crate the device links (rusqlite, png, zune-jpeg, fontdue, embedded-graphics, libc), with the reasoning written into the file. Actions keep majors, because there CI genuinely does judge the result — a broken action turns its own workflow red.

The lesson generalises past dependencies: an automated gate that proposes changes must be scoped to what the automated gates can check. This repository's most important gate runs on a laptop.

Part B — What is green

Run locally, in full, after the fixes in this pass:

Gate Result
player test suite 432 passed, 0 failed (323 cinder-ui + 82 cinder-ffi + 17 cinder-db + 10 integration)
installer test suite 7 passed
Clippy correctness + suspicious 0 errors
tools/host_syntax_check.sh 20 files, all parse
C++ self-tests 7/7
tools/shell_check.sh 47 scripts, bash -n + shellcheck 0.11.0 clean
cinder-home/harness/run.sh all scenarios passed
cinder-home/tools/test_launcher.sh 46 passed, 0 failed
cargo audit (new) 0 vulnerabilities, both workspaces

Part B2 — verified on the device

Not desk work. The NW-A55 was on adb, so the fixes were built for ARM, pushed and booted.

Before: dev-channel build from 13:22 (the pre-fix binary), bootcount 0, escape ladder intact (launcher present, MAXBAD and cable_escape_off both live, no kill switch armed). The current binary was copied to /data/cinder/backup/ first, so recovery is a push-back rather than a flash — the existing cinder-home.prev from 2026-08-16 was left alone rather than overwritten.

After — dist/dev/cinder-home, 3,653,580 bytes, GLIBC_2.4 … 2.18, pushed and rebooted:
Boot completed, StopBootAnimation at 27.4 s, bootcount stayed 0
Library 2560 tracks, 256 albums, 166 artists — identical to the pre-reboot baseline, so rusqlite 0.40's bundled SQLite reads the media DB correctly on the device
Guard recoveries 0
Fatal signals / watchdogs 0
Art decode failures 0
Stability 8½ minutes' uptime, clean

The /contents reclaim ran, and this is the useful part. At 13.343 s:

usb-msc: /contents was unmounted by something other than us — reclaimed it
(cinder-msc off rc=0). Library and album art are back.

That is the exact call site moved to run_watchdog_only in §A2 — a system() mount followed by a full cinder_db_open — executing on hardware, completing, and the library loading normally afterwards with Sony IPC untouched. The cable being in for adb is what makes Sony's stack unmount the volume, so the adb session reproduces the condition for free.

What the boot did not prove, and how it was covered instead. The log said art cache: 256 cached, 0 to decode — the cache was built by the old binary, so the new decoders never ran. Clearing it was the obvious next step and was not worth the device state, so the same question was answered read-only: the actual embedded cover was pulled off the player with dd (Aña — Anymore, 40,338 bytes, magic=FFD8FFE0) and decoded on the host through decode(). It came back at the right dimensions with w*h*3 bytes.

That exposed the real gap: decode_jpeg had no test at all, and JPEG is what every cover in this library actually is. Three were added with hand-built 8×8 fixtures embedded as byte literals — RGB channel order, the grayscale out.len() == px fan-out branch (which real covers hit, and which would otherwise be logged as an "unrecognized layout" and dropped), and a truncated file, which is the realistic corruption when art is read from a FLAC at a recorded offset and length.

Things that are quietly excellent and should not be lost

  • The installer has zero dependencies. installer/Cargo.lock contains exactly one package: the installer itself. A signed-by-nobody .exe that people run against their only Walkman, with no supply chain at all, is a genuinely strong position and is now protected by a CI gate (§C3).
  • The setuid helpers take fixed verbs, never paths. cinder-power accepts off/restart; cinder-msc accepts five literals; cinder-clock accepts set plus a digit string it validates character by character. There is no path or format string reachable from an unprivileged caller into a root process. For twelve chmod 4755 installs this is the right design and it is held consistently.
  • Six TODO markers in ~60,000 lines, five of them one cluster in ldac-bridge naming vtable indices that are genuinely not yet recovered. There is no hidden backlog in the comments.

Part C — Repository and release process

C1. Community health was at 71% — FIXED

GitHub's own community profile reported code_of_conduct, issue_template and pull_request_template missing. Added:

  • CODE_OF_CONDUCT.md — Contributor Covenant 2.1, with one project-specific clause: understating the risk of a build or a procedure is a conduct problem here, not merely a technical disagreement. Enforcement routes to the private security advisory form, the only private channel this repo has.
  • .github/ISSUE_TEMPLATE/bug_report.yml — asks for channel, model, Sony firmware version, the log path, and whether the escape ladder brought the device back. That last field makes every crash report double as a test of the recovery mechanism.
  • .github/ISSUE_TEMPLATE/device_report.yml — for reporting a DEVICE_CHECKLIST.md item run on real hardware, pass or fail. This project's bottleneck is device time from people who own the player; there was no route for a stranger to contribute one.
  • .github/ISSUE_TEMPLATE/feature_request.yml, config.yml (blank issues off; RECOVERY.md, SECURITY.md and Discussions surfaced at the point of filing).
  • .github/PULL_REQUEST_TEMPLATE.md — with a blast-radius section that separates "UI, cannot brick anything" from "runs as root / boot path / Sony IPC / USB-MSC ordering", and the six local check commands as a checklist.

C2. There was no changelog — FIXED

CHANGELOG.md, Keep a Changelog format, backfilled from tags v0.1.0–v0.1.5 and linked from the README and the release body. Entries carry the project's own device-verified / device-unverified marking, because for this codebase "what changed" and "was it ever run on hardware" are different questions and the second one matters more.

C3. Nothing checked the dependency tree — FIXED

122 crates reach the binary that runs as the device's Home app, and no gate looked at any of them. Added an audit job to ci.yml running cargo audit over both workspaces, plus a weekly cron so an advisory published against unchanged code is found by the calendar rather than by the next unrelated commit.

Verified locally before wiring, per this repo's own rule about CI steps: 0 vulnerabilities, exit 0 on both. Two unmaintained-crate warnings (instant, ttf-parser, both transitive via fontdue) are reported and do not fail the build, which is the right split — a vulnerability is a stop, an unmaintained transitive dependency is a fact to know.

.github/dependabot.yml added: monthly, grouped, low PR limit. Monthly on purpose — this repo ships binaries built by a maintainer and committed, so a dependency bump landing unattended is a bump whose output nobody rebuilt.

C4. 26 documents in docs/ with no index — FIXED

docs/README.md says which document answers which question and, more usefully, which ones are history. PRODUCTION_READINESS.md and FLASH_NEXT.md were accurate on 2026-07-28 and are now read as current by anyone who opens them cold.

C5. The README's front door — FIXED

Added CI / release / downloads / licence / device badges, a Documentation section pointing at the new index, and a Contributing section that says plainly that the most useful contribution is a device report, because it is, and nothing said so.

Part D — What is left, and who has to do it

D2 and D3 were fixed in this pass and are kept here with their outcomes, because the reasoning is the useful part. D1, D5 and D6 are open. D5 is the one that matters: it is the root cause of both of today's breakages, and it needs an authenticated maintainer — nothing in this pass could touch it.

D1. .git is 1.3 GB, and 863 MB of it is committed ARM binaries — half fixed
.git total 1.3 GB (418 MB in July — it has tripled)
Blobs under cinder-home/dist/ 863 MB, 66% of the repository
Revisions of dist/dev/cinder-home 38
Revisions of dist/dev/cinder-probe 36
Revisions of dist/stable/* 22 and 21

117 copies of ~3.5 MB ARM ELFs, every one of them rewritten in full because a stripped binary shares no deltas with the last build. A git clone of a ~60,000-line project transfers 1.3 GB, and that is the first thing anyone evaluating this repository experiences.

The binaries are tracked for a good reason — they are the flashable deliverable, the ARM toolchain is not reproducible on a hosted runner, and tools/release.sh verifies the committed payload byte-for-byte before it will tag. That reason justifies shipping them; it does not justify keeping every intermediate build forever. Three options, cheapest first:

  1. Stop committing dist/dev/. It is 74 of the 117 revisions and it is not what users install. Publish dev builds as CI artefacts or pre-release assets instead. Costs nothing, halves the bleeding, does not touch history.
  2. Commit dist/stable/ only on release commits. A binary per tag, not per push — six objects a year instead of twenty.
  3. Rewrite history (git filter-repo) to drop superseded dist/ blobs, keeping the ones tagged releases point at. Recovers most of the 863 MB. This rewrites every SHA, so it needs a force-push and it invalidates every existing clone and every commit hash quoted in docs/ — of which there are many, deliberately. Only worth it alongside (1) and (2), or the growth resumes the next day.

Recommended: (1) now, (2) next release, and treat (3) as a separate decision made once the bleeding has stopped.

2026-09-11 — the 1.3 GB was one working copy's .git, not the clone. GitHub reports the repository at 124 MB, and a fresh clone downloads about that; the .git that was measured held 1.09 GB of loose objects that git gc had never packed. The committed binaries are still most of the history, and option 3 is still how to drop them — now as one step of a rewrite that also takes Sony's files out, rehearsed in HISTORY_REWRITE.md.

D2. cinder-home/ROADMAP.md was five weeks stale and the README pointed at it — FIXED

Its header says "Last audited: 2026-07-28". STATUS.md carries entries through 2026-08-31. Between them sit the FM tuner, the clock helper, Bluetooth, NFC, playlists, the on-screen keyboard, liked-songs sync, the device settings page and six audits — none of which the roadmap knows about. Its P1 list still describes Bluetooth as "P2, backend not wired".

The README calls it "Forward plan" and docs/README.md cannot fix this by describing it, because unlike the dated audits a roadmap is supposed to be current. Either re-audit it, or demote it: put the forward plan in DEVICE_CHECKLIST.md (which is current) and keep ROADMAP.md as a dated snapshot like the audits, with the date in its filename.

D3. Small tracked clutter at the repository root — FIXED

battery_track.tsv (7.7 KB of session data) and cinder_screen_20260826_113143.png (468 KB, a timestamped screenshot) were tracked at the top level. .gitignore already excluded rendered screen previews on the grounds that they are regenerable; these two slipped past.

Fixed: both moved to artifacts/session/ (ignored wholesale) and untracked, with /battery_track.tsv and /cinder_screen_*.png added to .gitignore so they cannot drift back. The repository root is now thirteen files, every one a document or a manifest.

D5. main accepts commits that fail CI — the root cause of both of today's breakages

This is the most important item in this document, and it is the one that cannot be fixed from a shell. It needs an authenticated maintainer.

Two red commits reached main on 2026-09-01:

Commit How What CI said
15b0fb8 pushed directly to main failure at 12:25 UTC (§A1)
b22202b dependabot PR #8 merged failure on the PR run for 92fd6b2 (§A5)

The gate worked both times. It ran, it was correct, and it was not required. Everything else in this audit — 429 tests, the launcher matrix, the harness, the self-tests, the advisory scan — is only worth what the merge rule makes it worth, and right now that is nothing.

The fix is one API call, and it also makes the dependabot.yml caution in §A5 mean something, because a red PR merged is a red main either way:

gh api -X PUT repos/superwilso/Cinder/branches/main/protection \
  --input - <<'JSON'
{
  "required_status_checks": {
    "strict": true,
    "contexts": [
      "player (host tests)",
      "installer (linux)",
      "installer (windows)",
      "C/C++ syntax + self-tests + shell + harness + launcher",
      "dependency advisories",
      "dist payload is complete"
    ]
  },
  "enforce_admins": false,
  "required_pull_request_reviews": null,
  "restrictions": null,
  "allow_force_pushes": false,
  "allow_deletions": false
}
JSON

enforce_admins: false and required_pull_request_reviews: null are deliberate for a single-maintainer project: this should stop a red merge, not require a second person who does not exist. allow_force_pushes: false is worth having regardless — and note it would have to be lifted deliberately for the history rewrite in §D1, which is the right amount of friction for that.

Check what is set today with:

gh api repos/superwilso/Cinder/branches/main/protection

D6. Repository metadata (needs auth)

Topics are nw-a30, nw-a40, nw-a50, walkman, walkmanone — every one about the device, none about what this repository is, so it does not surface for anyone searching the technology:

gh api -X PUT repos/superwilso/Cinder/topics -f names='["walkman","nw-a50","nw-a40","nw-a30",
  "walkmanone","rust","reverse-engineering","firmware","embedded","custom-firmware","ldac",
  "bluetooth","music-player","digital-audio-player","hi-res-audio"]'

gh api -X PATCH repos/superwilso/Cinder \
  -f homepage='https://github.com/superwilso/Cinder/releases/latest'

Wiki is off and Discussions is on, which is the right way round for a project whose documentation is this good in the tree.

Method

Every check in Part B was executed on this machine at the tree described. GitHub state was read from the public API (/repos, /releases, /actions/runs, /pulls, /community/profile) — gh is not authenticated here, so nothing was written to GitHub in this pass: no issue, release, merge or setting was changed, and D5 and D6 are open for exactly that reason. The ARM figures in Part B come from a full cinder-home/build.sh stable run on this machine, which has the cross toolchain and the xenial armhf sysroot. Repository sizes come from git rev-list --objects --all piped through git cat-file --batch-check, not from estimates.