Claude/system audit complete 9czpyd - #1887
Merged
Merged
Conversation
…een) The `Security Audit` CI job (`npm audit --omit=dev --audit-level=critical`) has been red since May: the prod dependency tree pulls a vulnerable `tar` (node-ble → dbus-next → usocket → node-gyp → tar) flagged by GHSA-vmf3-w455-68vh (and related) for versions <= 7.5.20. Pin `tar` to >= 7.5.21 via `overrides` (resolves to 7.5.22). This clears the only CRITICAL prod advisory, so the audit job passes again. No runtime code depends on tar directly — it is reached only through node-gyp's build path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UjWnM91xMywEtcw7Eu3YUE
…absent CLAUDE.md documents that `jest.config.cjs` "automatically skips SQLite-dependent suites" so `npm test` still runs the pure-JS suites after `npm install --ignore-scripts` (the documented container/CI setup). In practice the config only skipped a single hard-coded file (`midi-filter.test.js`), so six other suites that construct a real better-sqlite3 database failed with a bindings error — `npm test` was red in the documented dev workflow (27 test failures). Detect the SQLite-backed suites from their source (they import `better-sqlite3`, construct the top-level Database/DatabaseManager, or run the migration runner) and add them to `testPathIgnorePatterns` only when the native binding probe fails. This is self-maintaining: new SQLite suites are picked up automatically instead of silently failing once a hand-maintained list drifts. With bindings present all 96 suites still run. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UjWnM91xMywEtcw7Eu3YUE
Fixes verified bugs found while auditing every subsystem. Each fix is minimal and covered by existing or new regression tests. No behavioural features added. Persistence - Database façade was missing three instrument methods (findInstrumentByNormalizedName / reconcileDeviceId / deduplicateByUsbSerial) that InstrumentRepository delegates to, so device reconciliation threw and was silently swallowed — saved settings appeared lost after a USB re-enumerate. Add the delegations; guard the whole façade surface with a prototype test. - runMigrations sorted files lexicographically; sort by numeric version prefix so a future non-zero-padded/4-digit migration can't run out of order. Playback - Seek state reconstruction called this.deviceManager (undefined) — threw on every seek past a SysEx, skipping bank/program/CC/pitch-bend restore. Use this._deps.deviceManager. - Seeking while paused stopped the MIDI clock permanently (resumePlayback became a no-op); only stop the clock on active-play seeks. - pause() now resets the scheduler's per-note tracking after All Notes Off so a polyphony-limited instrument doesn't drop notes after resume. - All Notes Off now resolves omni-fallback channels to the omni device. - Live transposition changes now release sounding notes to avoid stuck notes. Routing / devices / transports - MidiRouter relative-compensation cache was keyed `source|channel` but invalidated by bare `source`, so route add/remove/enable never cleared it. - RTP-MIDI parser decoded delta-time as a proper VLQ (respecting the Z bit) instead of skipping every high-bit-clear byte, which had swallowed running-status commands from real peers. - File routing status counted split-segment rows as routed channels, reporting a partly-unrouted file as fully playable; count distinct channels. - A device disabled via enableDevice(false) was re-enabled by any device-map rebuild; preserve the disabled flag across the rebuild. - Inbound network SysEx / System Real-Time were dropped; forward raw bytes. - NobleBleAdapter's unexpected-disconnect handler leaked itself across reconnects. Adaptation - compressNoteToRange folded to the wrong pitch class (reflection instead of octave folding): note 49 mapped to 71 (B) instead of 61 (C#). - Capability validator flagged note_range_min === 0 as missing and accepted an inverted range (min > max) that later marked every melodic channel incompatible. - DrumNoteMapper never reserved the side-stick (37) or tom→latin fallback pads, allowing double-assignment collisions. API / core - WebSocket connection-limit reject path attached no error listener → an errored over-limit socket raised an unhandled 'error' that shut the server down (remote DoS). Attach a listener before closing. - CommandRegistry/WebSocketServer dereferenced a null frame in their error/log paths (client can send literal JSON `null`); use optional chaining. Lighting - sACN packet length was 126+slotCount (off by one), inflating all three PDU length fields and the trailing byte; correct to 125+slotCount. - _applyBrightness now clamps to [0,255] so the schema-less lighting API can't overflow a DMX/packed-RGB byte; hsvToRgb normalises negative hue and clamps. - Fade tracking keys collided within a millisecond, orphaning intervals through a blackout; append a monotonic counter. Frontend - WebSocket auto-reconnect died after the first failed retry (_reconnecting was never cleared on close); reset it so the retry loop survives outages. - Command responses with falsy data leaked the raw envelope; use `'data' in`. - EventBus once+debounce listeners were never removed and off() left a pending debounce timer that fired after teardown. - MidiEditorPlayableNotes guarded a null routing value; FilterManager subfolder filter now requires a path-separator boundary (/Rock no longer matches /Rockabilly). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UjWnM91xMywEtcw7Eu3YUE
The `Lint & Format` CI job's `npm run format:check` step has been red since May: 352 source/test files did not match the pinned Prettier (3.8.x) output (mostly assignment/ternary line-break changes), so the whole job failed on every push. Run `prettier --write` across src/, public/js/ and tests/. This commit is formatting-only EXCEPT for one test fix: gm-coverage.test.js matched KeyboardModal.js source with a regex that assumed a single-line array literal; Prettier legitimately wrapped that array, so the regex is now whitespace/ newline tolerant (it validates the dispatched view-kind list, not the layout). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UjWnM91xMywEtcw7Eu3YUE
…to CI The 26 non-en/fr locales were each missing ~250 keys added with the lighting, instrument-settings and playlist UI (the runtime silently fell back to French for those panels). The `tests/audit-i18n.test.js` guard that should have caught this ran under NEITHER runner — Jest ignores it and Vitest didn't include it — so the drift went unnoticed. - Deep-merge the missing keys from en.json into every locale (English placeholders; existing translations untouched, order preserved) so the key structure is complete and non-fr users get a real string in these panels. Human translation of the placeholders can follow. - Fill loopCreator.statusIdle in en/fr (it was an empty string while every sibling status and every other locale had text). - Add tests/audit-i18n.test.js to the Vitest include so the guard runs in the frontend test job and future drift fails CI. All 28 locales now match en.json (2685 keys, no empty strings, 128 GM instrument names each). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UjWnM91xMywEtcw7Eu3YUE
…risk Bound the worst-case memory footprint for the 1 GB Raspberry Pi target. - MAX_SF2_FILE_SIZE 500 MB -> 160 MB. The upload path buffers the whole file and parsing allocates a full-file copy, so the old cap could OOM on a single upload; 160 MB still fits common GM soundfonts (FluidR3_GM ~140 MB). - SF2InstanceCache default capacity 3 -> 2 (holds a melodic + drum instance without re-parse thrash, but caps resident parsed soundfonts). - UploadQueue: default depth 100 -> 8 AND a new 64 MB in-flight BYTE budget (100 x 10 MB MIDI could pin ~1 GB). A lone task is always admitted so a legal large file can't deadlock the queue; the MIDI upload route now charges req.body.length against the budget. Also document (per maintainer decision to keep current behaviour) that the trusted-lan same-origin/Sec-Fetch header bypass is forgeable by a non-browser WAN client, and that security.mode="secure" is the fix when exposing the box. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UjWnM91xMywEtcw7Eu3YUE
… persistence) Files - FileManager.handleUpload wrapped the post-parse analysis (validate + metadata /tempo/text extraction) in try/catch so a parseable-but-pathological AST that throws there cleans up its orphan blob instead of leaking it. - Enforce MAX_MIDI_FILE_SIZE on every user-supplied write path (saveFile, replaceFileBytes, createDerivedFile) via a shared _assertMidiSizeLimit; only the upload route checked it before. bakeAndSave is exempt (internal transform of already-bounded content). - BlobStore.write now fsyncs the staged file and the directory before/after the rename, so a power loss on the Pi SD card can't leave a truncated blob whose content no longer matches its content_hash. Playback - stop() during a no-gap queue advance no longer restarts playback: a monotonic play token lets the in-flight playQueueItem detect it was superseded while awaiting loadFile and bail before start(); stop() finalizes cleanly during the advance window (when `playing` is briefly false). - PlaybackStateMachine docs corrected — it is advisory state-tracking, not an enforcing guard (the real guards are the playing/paused flag checks). Gating callers on tryTransition would regress seek-while-stopped. Lighting - note_led note-off now decrements the active-note counter (was leaked to the stale-note safety sweep). Transport - Inbound RTP-MIDI responder sessions get a 10 s establishment watchdog; a peer that never completes the data-port invitation no longer lingers in rtpSessions forever. Persistence - Database.restoreFromBackup reopens the connection and rebuilds the per-domain sub-modules (extracted into _initSubModules), so the manager is usable immediately instead of leaving every sub-module on a closed handle. Deferred (documented in code): the lighting wildcard-rule double-fire needs a shared per-event id plumbed through the router (risky without rule-engine tests); the WS same-origin port-match check is left as-is per the maintainer's decision to keep current auth behaviour. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UjWnM91xMywEtcw7Eu3YUE
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.