Audit hardening: persistence durability, Electron security, perf, deps - #65
Open
aagentah wants to merge 27 commits into
Open
Audit hardening: persistence durability, Electron security, perf, deps#65aagentah wants to merge 27 commits into
aagentah wants to merge 27 commits into
Conversation
… audit vulns - @types/node ^25 → ^20 to match node>=20 engine and Electron's bundled Node (ends types/runtime skew) - eslint ^8 → ^9 (9.39.2 already installed; flat config is already v9); add @eslint/js + globals as explicit devDeps since eslint.config.mjs imports them - electron ^39.2.7 → ^39.8.10 (Chromium security patches within the same major) - @playwright/test + playwright 1.57.0 → 1.60.0 - add overrides.ws ^8.18.1: fixes the ws advisory pulled via osc (the only shipped-runtime vuln) without downgrading osc - npm update on within-range build/lint tooling: @babel/preset-env ≥7.29.4 fixes GHSA-fv7c-fp4j-7gwp; webpack, postcss, ts-eslint, etc. - npm audit fix (non-breaking): 38 → 12 vulns; remaining are the electron-builder 25.x cluster and dev-only webpack-dev-server transitives, handled/deferred separately Note: legacy-peer-deps kept intentionally (load-bearing for CI npm ci / lockfile compatibility per release.yml). React @types already realigned to 18.x on develop. Verified green: typecheck:all, 170/170 unit tests, lint (no new warnings). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ling backup atomicWrite did writeFile→rename with no fsync, so a power loss within the OS writeback window could leave a truncated JSON primary. The .backup the reader (readJsonWithBackup) relies on was only created inside the Windows EEXIST/EPERM rename fallback, which POSIX rename never triggers, so on macOS/Linux no backup was ever written and corrupt-primary recovery had nothing to restore from. Now both async and sync paths: - write the temp file and fsync it before touching the primary - copy the current good primary to <file>.backup before the atomic rename - fsync the containing directory after the rename Directory fsync and the backup copy are best-effort (never abort the write); the Windows rename fallback is preserved. TDD: added 4 tests (rolling backup + fsync, async and sync). Full suite 174/174, typecheck:all + lint green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…on privileged windows The dashboard and projector windows hold the full nwWrldBridge (workspace fs, JSON store, openExternal) but had no setWindowOpenHandler and no will-navigate guard, so a stray or injected link/navigation could load remote content into a filesystem-capable renderer. Both windows now: - deny window.open, routing genuine http(s) links through the validated normalizeOpenExternalUrl + shell.openExternal path (same allow-list as os IPC) - prevent will-navigate / will-redirect away from bundled local content (file:/nw-sandbox:/nw-assets:/devtools:/about:blank), which a file:// SPA using hash/history routing never triggers, so no in-app flow regresses. TDD: extended the windows runtime harness (webContents open-handler + nav listeners + shell) and added a test asserting deny + external-nav block + file:// allow on both windows. Full suite 175/175, typecheck:all + lint green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…en overlay is closed addDebugLog (and the from-projector debug-log path) called setDebugLogs on every inbound MIDI/OSC event regardless of whether the debug overlay was open, forcing a re-render of every debugLogs consumer on the live input hot path. Entries are now always retained in a ring buffer (cheap, no re-render) and a re-render is only triggered while the overlay is open; the buffer is flushed into state when the overlay opens. Observable behaviour is unchanged (the overlay still shows the last 200 entries); only the wasted re-renders while closed are removed. Verified: typecheck:all + lint green (no new warnings). Dashboard hook is webpack-lane (not node-testable); logic preserves the existing last-200 semantics. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…nder loop getSequencerForTrack (which walks recordingData) was called inside every one of the 16 step buttons for every channel row, i.e. 16xN times per NoteSelector render - and NoteSelector re-renders every step during playback (the playhead prop). The lookup depends only on recordingData + track.id, so it is now memoized once per render and channelPattern is derived from it. Pure refactor: identical values, no behaviour change. Verified: typecheck:all + lint green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…hreeBase destroy BaseThreeJsModule added a window 'resize' listener in its constructor but never removed it in destroy(), and disposed the renderer without forcing context loss. Across same-sandbox instance churn (e.g. matrix re-configuration, which destroys and recreates instances without recycling the sandbox process), each dead instance stayed pinned by its resize closure and its WebGL context lingered until GC. destroy() now removes the resize listener and calls renderer.forceContextLoss() before dispose(). Verified: typecheck:all green; full webpack production build compiles (dashboard, projector, moduleSandbox bundles all emit). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The release-publishing action ran from the mutable @v2 tag while holding the contents:write token, so a repointed tag (maintainer or attacker) could tamper with published artifacts or exfiltrate the token. Pinned to the commit SHA the v2 tag currently resolves to (3bb12739...), which is behaviourally identical to @v2. First-party actions/* remain on @v4 (lower risk); pinning those is a follow-up. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…aged builds The signed app shipped with Electron's RunAsNode, EnableNodeCliInspectArguments and EnableNodeOptionsEnvironmentVariable fuses enabled, so a local process could relaunch the notarized binary as a generic Node runtime (ELECTRON_RUN_AS_NODE), attach a debugger (--inspect), or inject NODE_OPTIONS, running arbitrary code inside the app's code-signing identity and entitlements. Added an electron-builder afterPack hook (scripts/afterPack.js) that flips these three fuses off before signing, via @electron/fuses. OnlyLoadAppFromAsar / asar-integrity fuses intentionally deferred (they interact with asar signing and need their own test). Verified: unsigned pack:mac:arm64 --dir succeeds and afterPack flips the fuses on the packaged binary. NEEDS a signed+notarized build test before release (confirm launch, and that ELECTRON_RUN_AS_NODE no longer works). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The hardened-runtime entitlements granted com.apple.security.cs.disable-library-validation, which lets the signed app load unsigned/third-party dylibs and weakens the hardened runtime. Removed it from both entitlement plists; kept allow-jit and allow-unsigned-executable-memory (required for V8 JIT) and device.audio-input (the mic capture feature). No dependency appears to load external unsigned dylibs. Verified: plists remain valid (packagingConfig test green); unsigned --dir pack succeeds. NEEDS a signed+notarized build launch test (confirm the app starts and audio capture works) before release. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… build CVEs electron-builder 25.x pulled ~9 high-severity advisories via app-builder-lib, @electron/rebuild, node-gyp, tar, etc. (build-time only, not shipped, but flagged by npm audit). Bumped electron-builder, dmg-builder and electron-builder-squirrel-windows together to 26.8.1. npm audit: 12 to 3 (the remaining 3 are dev-only webpack-dev-server transitives needing a breaking major). Verified: typecheck:all + 175 unit tests + lint green; unsigned pack:mac:arm64 --dir packages successfully on eb 26 with the afterPack fuses applied. NEEDS a full signed+notarized release build (dist:mac/win/linux) before release: eb 26 changed parts of the signing/notarization path that --dir does not exercise. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…nal rejection Adversarial review of the branch surfaced two issues: - afterPack used productFilename (nw_wrld) for the Linux binary, but electron-builder names the Linux executable from the package name (nw-wrld), so flipFuses hit ENOENT and would abort every Linux build and the Linux release CI job. The Linux branch now uses packager.executableName (sanitizedName.toLowerCase()). macOS/Windows paths were already correct and verified. - the window-open handler caught only synchronous throws from shell.openExternal; its promise rejection is now swallowed too (mirrors registerOsBridge). Verified: typecheck:all + 175 unit tests + lint green; unsigned --dir packs now succeed on macOS AND Linux, afterPack flipping fuses on the correct binary (release/linux-unpacked/nw-wrld). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
eslint.config.mjs already runs on eslint 8 with both packages resolved transitively, so the 8->9 bump was currency-only and added two avoidable direct deps. Revert to eslint ^8 and remove the companions; lint stays green (0 errors). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codebase is near-zero comments (sandbox.ts: 633 lines, 0; all untouched test files: 0). Reduce the multi-line explanatory blocks to terse one-liners, keeping only genuine gotchas (Windows rename, Linux exec name, dir fsync). threeBase.ts keeps short notes matching the projector layer's local style. atomicWrite.ts is comments-only; logic unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Creating or switching to a set with no selected track sends reload-data with trackName: null. The projector fell back to the previously active track's name (props.trackName || this.activeTrack?.name), then tried to select that track in the new set, surfacing "Track <name> not found" on the projector. Now a null/empty trackName clears lastRequestedTrackName, deactivates, and shows "No tracks in this set" / "No track selected". Decision extracted to a pure, unit-tested helper (resolveReloadTarget, 5 tests). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
AsteroidGraph's loadMeteors (executeOnLoad) awaits an asset load, but unlike the working PerlinBlob it had no 'destroyed' guard. During the projector's deactivate/re-init churn the async load resolves on an already-destroyed instance, orphaning a p5 canvas (flaky removal) and leaving the live instance's meteors unpopulated (blank on add); preview/refresh create a clean single instance so they render. Mirror PerlinBlob's lifecycle: set destroyed=false in the constructor, bail after the await and in p5 setup/draw, and set destroyed=true first in destroy(). Unit test (RED->GREEN) covers the post-destroy guard. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…from the saved constructor Confirmed from the real save folder: set2/aaa AsteroidGraph was saved with constructor [matrix, show] - loadMeteors (its executeOnLoad data loader) was missing, so the sandbox (which only ran the saved constructor methods) never populated this.meteors and drew nothing. handleAddToTrack drops executeOnLoad methods when a module is added before its introspection has populated module.methods; only AsteroidGraph showed it because its dropped method loads render DATA (other modules' dropped executeOnLoad methods are cosmetic, so they still draw). Fix: at initTrack and setMatrixForInstance, merge the module class's declared executeOnLoad methods (with default options) into the run list when absent from the saved constructor, running loaders first. Pure helper resolveConstructorRunList with 5 tests; verified against the real save ([matrix, show] -> runs [loadMeteors, show]). Fixes existing saves with no recreation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…c init" This reverts commit 77b4cd6.
Remove modules, exports, types and assets confirmed unused by repo-wide grep (no runtime, test or build references): - delete orphaned files ThreeTemplate.ts, SequencerGrid.tsx (and its dead helpText key) and ProgressBar.tsx - drop the unused helpTextAtom (recordingStateAtom is still used, kept) - drop dead midiUtils exports (MIDI_INPUT_NAME, NOTE_TO_CHANNEL, noteNameToNumber, buildChannelNotesMap, buildTrackNotesMapFromTracks) and the now-orphaned CHANNEL_NOTES table - drop the unwired per-track signal-settings type island in userData.ts - delete unused assets (low-earth-orbits-objects.json, three-font.json) and the unreferenced Roboto Mono static font weights No behaviour change. typecheck, test:unit, lint and build all pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… style - ErrorBoundary: default export to named export, matching the rest of the component layer; update the single import site - audio/device/file dashboard hooks: export function to export const, to match the other hooks - ipcFromDashboard and methodExecutor: route console.* through the projector logger (silences debug logs in packaged builds; logger.error keeps the same always-on behaviour as console.error) No behaviour change. typecheck, test:unit, lint and build all pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Correct inaccuracies found by auditing the docs against the code, and trim duplication: - starter modules: 16 to 22, with correct names and categories - README: fix the Electron badge (39.8.10) and source tree (.ts paths, drop src/renderer.ts and the deleted template, describe the real main process), complete the allowed-imports list, fix the built-in-methods anchor; de-duplicate the project tree, DAW and external-modes sections - GETTING_STARTED: rewrite the non-existent "Monaco" editor step to match the read-only viewer, fill the libasound stub, point install at the README - CONTRIBUTING: Node v20, double quotes and semicolons (Prettier), target develop, add the automated test commands, split validated vs recommended module rules, version 0.5.0-beta - MODULE_DEVELOPMENT: add listAssets, correct the seeded-assets list, condense the generic performance section to nw_wrld specifics - RUNTIME_TS_TESTING: repoint IPC-handler wiring to src/main/mainProcess/ipcBridge/*; consolidate the dist/runtime vs src rule - add E2E_TESTING_GUIDELINES to the PR and copilot context lists British English, no em dashes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… cached Every module method trigger paid an extra sandbox:ensure IPC invoke, an fs.statSync, and a BrowserView re-attach + setBounds in main before the actual sandbox:request. Cache the token in TrackSandboxHost and only ensure when absent; on an out-of-band token invalidation (sandbox crash/respawn) re-ensure and resend the same payload once, which lands in the same INSTANCE_NOT_FOUND end state the per-request ensure produced. Resize re-attach is unaffected: the projector window's resize handler already calls updateSandboxViewBounds directly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…three.js renders buildMidiConfig walked every track and channel mapping on every input event, before the sequencer-mode and source early-outs. Add createMidiConfigCache (reference-keyed on userData/config/input type, both reassigned wholesale in loadUserData) and build only after the drop checks. threeBase rendered the scene 2-4 times per rAF tick while a camera animation or damping was active: updateCameraAnimation ran its own controls.update() + render() microseconds before animate() ran the same pair, and the OrbitControls "change" listener rendered again from inside controls.update(). Intermediate renders in the same tick are never composited, so drop the duplicate pair and gate the change listener to the pre-loop phase (modules that never call setModel keep rendering on interaction). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The sequencer playhead, flash setters, projector perf stats, and audio levels all lived in (or subscribed from) the root Dashboard component, so every 16th-note step, channel flash, 1Hz perf heartbeat, and 100ms audio-level update re-rendered the entire dashboard, including the eagerly-evaluated bodies of every closed modal. - sequencerCurrentStep moves to an atom written by the playback hook and read only by a new memoised SequencerStepRow, dropping the prop from the Dashboard -> Body -> TrackItem -> SortableModuleItem -> NoteSelector chain (which also restores TrackItem's memo during playback). - useFlashingChannels becomes setter-only (no caller used the value) and setter-only useAtom subscriptions switch to useSetAtom in Dashboard and TrackItem, including TrackItem's unused userDataAtom value. - Projector perf stats park in a ref while the debug overlay is closed and seed the state on open. - Audio capture/file level updates bail with the previous state when all six values are unchanged (kills steady churn during silence). - SettingsModal (1001 lines) returns null before building its JSX when closed; DashboardFooter drops a recordingData subscription whose value was never read. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A pure track or set switch re-fired the debounced userData save with an unchanged reference, paying stringify + atomic write + fsync + backup copy for identical bytes, then a reload-data broadcast that made the projector sync-read the whole file on its render thread. Skip the save (and broadcast) when the userData reference matches the last save; all mutations flow through immer so changed data always has a new reference, and the existing post-save reload-data recovery for freshly created tracks is unaffected. set-activate now only fires when the set actually changed; same-set track switches use track-activate alone, the exact flow MIDI-driven track selection has always used, removing a second synchronous userData.json read per switch. The appState read-modify-write per switch/mute now reads disk once and spreads from the in-memory copy (this hook and the unload flush are the only writers). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
In any external input mode the footer's bottom right now shows the last matched track selection and method trigger (TRACK / METHOD). Values are written from useInputEvents only when a trigger actually fires and are read by a memoised leaf component, so per-event updates re-render the readout alone. Covered by footer-input-activity e2e (mock MIDI). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Clone icons next to the edit/delete controls in the set and track modals, and next to the eye toggle on each module. Copies get a "(Copy)" name with case-insensitive collision handling, fresh ids throughout (set, track, module instance, with modulesData remapped), the next free track slot for same-set track copies (alert when full), and duplicated recordingData entries so sequencer patterns and recordings carry over. Pure helpers in shared/utils/duplicateUtils.ts (runtime lane, 8 unit tests) plus duplicate-set-track-module e2e. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- remove unused DraftFloatInput, FileInput, cleanupStaleTempFiles, getJsonDir - drop leftover viewport-line console.log; fix stale JSDoc/header in moduleBase - correct BPM range claim in README/GETTING_STARTED (no 60-130 clamp exists) - rename optionValidator -> optionValidation to match validation/ convention - route useDashboardUpdateConfig through the updateUserData wrapper - rename Modal onCloseHandler -> onOverlayClick; NewModuleDialog local Modal -> DialogShell - rename generic SortableItem -> SortableMethodItem; handleVisibilityChange naming - add parseInt radix at 5 sites; replace deprecated substr; flatten else-after-return Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Summary
Hardening from a full codebase audit (security, performance, persistence, dependencies). 11 commits, each verified independently.
Regression-proof set (verified: typecheck:all, 175/175 unit tests, lint, webpack prod build, unsigned --dir packs on macOS + Linux)
.backupon every write. Previously it didwriteFile+renamewith no fsync, and the.backupthe reader relies on was only written on the Windows EEXIST/EPERM path, so on macOS/Linux it never existed and corrupt-primary recovery had nothing to restore from. (4 new TDD tests)window.openand block navigation away from bundled local content, routing genuine http(s) links through the validatednormalizeOpenExternalUrl+shell.openExternalpath. (1 new TDD test)BaseThreeJsModule.destroy()now removes itswindowresize listener and callsrenderer.forceContextLoss()before dispose.@types/node^25 to ^20, eslint 8 to 9 alignment (+@eslint/js,globals), electron to 39.8.10, playwright 1.60,overrides.ws ^8.18.1, within-range tooling bumps, non-breakingnpm audit fix.Packaging hardening (committed; REQUIRES a signed + notarized build test before release)
disable-library-validationfrom the macOS entitlements (kept allow-jit, allow-unsigned-executable-memory, audio-input).softprops/action-gh-releaseto a commit SHA.Deliberately NOT changed (would have regressed)
developalready setsbackgroundThrottling:falseon both windows; the audit's Tone.Draw change would make visuals lead audio by the look-ahead.script-src 'self'would likely blank them (opaque file origin) and could break Tone's AudioWorklet. Deferred to a custom-protocol migration + smoke test.Test plan
pack:mac:arm64 --dirandpack:linux --dir(afterPack fuses applied to the correct per-platform binary)npm run dist:mac(signed + notarized): app launches, mic/MIDI input works,ELECTRON_RUN_AS_NODE=1 .../nw_wrld -e "..."is blockednpm run dist:winandnpm run dist:linuxpackage end-to-end (electron-builder 26 is a major bump)Deferred follow-ups (need a product/behaviour decision)
OSC default bind 0.0.0.0 to 127.0.0.1 + rate limit; recordingData growth cap; quit-time write flush; CSP; first-party action SHA pins; asar-integrity fuses.
🤖 Generated with Claude Code