fix: upgrade Next.js to 16.3.4, fixing 3 high-severity Snyk findings - #1766
Open
magic-peach wants to merge 1 commit into
Open
magic-peach wants to merge 1 commit into
magic-peach wants to merge 1 commit into
Conversation
Snyk flagged 3 high-severity CVEs traced to [email protected]: two Directory Traversal issues in its [email protected] dependency, and one Allocation of Resources Without Limits or Throttling in next itself. All three are fixed by [email protected]+; this goes to the latest 16.3.4. ## The real risk: Turbopack Next 16 defaults to Turbopack, which silently ignores next.config.ts's custom webpack() config — the fs:false fallback this app relies on for FFmpeg.wasm to load correctly. Confirmed this isn't hypothetical: an unmodified `next build` fails outright ("This build is using Turbopack, with a webpack config and no turbopack config"). Rather than translate the fix into Turbopack's different config surface and hope it behaves identically for WASM loading, dev/build scripts now pin --webpack explicitly, preserving the exact proven bundler behavior. Verified end-to-end in a real browser (not just a clean build exit code): uploaded a synthetic video, triggered export, confirmed via network trace that the FFmpeg worker chunk, ffmpeg-core.js, and ffmpeg-core.wasm all load and the export completes with zero console errors. ## ESLint: pinned to 9.x, not 10.x The upgrade codemod defaulted eslint to 10.9.1, which cascades into real breakage unrelated to the Next.js CVE fix: eslint-plugin-jsx-a11y tops out at eslint ^9 (no compatible release exists yet), and eslint-plugin-react's react/display-name rule crashes outright under ESLint 10's context API. Pinned to 9.39.5, the latest 9.x, which eslint-config-next's peerDependency (>=9.0.0) is fully satisfied by. ## eslint.config.mjs rewritten, not patched The existing config used a FlatCompat bridge (compat.extends("plugin: jsx-a11y/recommended")) to pull in jsx-a11y's legacy .eslintrc-style config. That bridge crashes on ESLint 10's minimatch resolution, and is unnecessary: jsx-a11y ships a native flat-config export (flatConfigs.recommended, 34 rules) that eslint-config-next's own core-web-vitals preset already partially registers. Fixed to merge just the rules (avoiding the plugin double-registration this causes) with no compat layer. Also fixes: `next lint` is removed in Next 16 (migrated to plain `eslint .` per Next's own codemod), and the migration didn't carry over next lint's automatic exclusion of build output — eslint.config.mjs now explicitly ignores .next/out/storybook-static/coverage, since eslint . was otherwise linting into storybook-static's bundled/minified JS as if it were source. Removed the now-dead legacy .eslintrc.json. ## 18 pre-existing lint findings, now fixed eslint-config-next 16 pulls in eslint-plugin-react-hooks@^7 (from an older major under Next 15), which adds new React-Compiler-era rules that caught 18 pre-existing issues across 16 files, mostly react-hooks/set-state-in-effect (calling a setState setter synchronously in a useEffect body). None of these are caused by this upgrade — the patterns were always there; the stricter linter just started checking for them. Each was reviewed individually rather than blanket-suppressed: - Genuine external-system synchronization (browser-only APIs unavailable during SSR, object-URL/timer/DOM lifecycles, video-element imperative control) kept as effects with a scoped eslint-disable and a comment explaining why: PrivacyBanner, ThemeToggle, ThemeProvider, NativeShareButton, VideoEditor, ExportOverlay, ImageOverlay, ThumbnailStrip, VideoPreview, OnboardingTour (dismiss), useVideoEditor. - Prop-mirrored-into-local-state cases (AudioSpeedControl, TrimControl) rewritten using React's own documented "adjust state during render" pattern instead of an effect — https://react.dev/learn/you-might-not- need-an-effect#adjusting-some-state-when-a-prop-changes. - ThemeProvider had a genuinely redundant duplicate setThemeState call immediately superseded by applyTheme() two lines later — removed rather than suppressed. - OnboardingTour's react-hooks/refs finding (reading tooltipRef.current during render to self-position a tooltip) fixed properly: the pure positioning math now takes plain width/height instead of a ref, with the real DOM measurement moved into useLayoutEffect and the result stored in state. This also improves actual behavior — previously the tooltip always used hardcoded 320x140 fallback dimensions since the ref was read during the same render it was attached in, never reflecting the tooltip's real measured size. - FileUpload.tsx restructured: `handleFile` was referenced (in the drag-and-drop effect) before its declaration further down the file. That effect had `[]` deps with exhaustive-deps silenced, so it captured whatever handleFile was on mount and would never pick up a new one if onFileSelect's identity ever changed — a real stale-closure bug, not just an ordering nitpick. Reordered handleFile before its use and added it to the effect's deps. Hoisting FileInfo/DropZone out of the component body (react-hooks/static-components: they were recreated every render, forcing React to remount their subtree) also surfaced and fixed a second latent bug: both FileInfo and the outer return rendered their own <input type="file"> bound to the same inputRef, so only the last-rendered one actually got the ref — verified via a real browser test that the "Change" button's click now reliably reaches the single remaining input. - Handful of stale eslint-disable comments removed (ComparisonPreview, LottiePlayer, VideoPreview) where the underlying issue no longer applied. Co-Authored-By: Claude Sonnet 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01L5gqyU5QKQtLAza6hL6dXv
Contributor
👋 Thanks for your PR, @magic-peach!Welcome to Reframe — a browser-based video editor built for everyone 🎬 What happens next
Quick checklist
Useful links
Happy coding! 🎉 |
✅ Deploy Preview for reframe-os ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
github-actions
Bot
added
level:advanced
Contributor
|
Contributor
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
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
Snyk flagged 3 high-severity CVEs on
[email protected]: two Directory Traversal issues in its[email protected]dependency, and one Allocation of Resources Without Limits or Throttling innextitself. All fixed by upgrading to[email protected].The real risk, and how it was handled
Next 16 defaults to Turbopack, which silently ignores this repo's custom
webpack()config innext.config.ts— thefs: falsefallback FFmpeg.wasm relies on to load correctly. Confirmed non-hypothetical: an unmodifiednext buildfails outright with an explicit Turbopack/webpack config conflict error. Rather than translate the fix into Turbopack's config surface and hope it behaves identically for WASM loading,dev/buildnow pin--webpackexplicitly — same proven bundler, still gets every other Next 16 fix.Verified in a real headless browser, not just a clean build exit code: uploaded a synthetic video, triggered export, confirmed via network trace that the FFmpeg worker chunk,
ffmpeg-core.js, andffmpeg-core.wasmall load and export completes with zero console errors.Also in this PR
eslint-plugin-jsx-a11yhas no ESLint-10-compatible release yet, andeslint-plugin-react'sreact/display-namerule crashes outright under ESLint 10's context API. 9.x fully satisfieseslint-config-next's>=9.0.0peer requirement.eslint.config.mjsrewritten, not patched — the oldFlatCompatbridge for jsx-a11y crashes on ESLint 10'sminimatchresolution and is unnecessary;jsx-a11yships its own native flat-config export. Also added explicitignoresfor build output, since migrating offnext lint(removed in Next 16) to plaineslint .lost that automatic exclusion — it was linting intostorybook-static's bundled JS as if it were source.eslint-config-next16 pulls in a majoreslint-plugin-react-hooksbump with new React-Compiler-era rules that caught real (if usually benign) patterns across 16 files. Each was reviewed on its own merits: genuine effect-appropriate cases (SSR-unsafe browser APIs, timer/object-URL lifecycles) got a scoped disable + comment explaining why; prop-mirrored-into-state cases were rewritten using React's own documented pattern; a couple were genuine bugs — see commit message for the full breakdown, including two real (not just lint-shaped) bugs this surfaced and fixed:ThemeProviderhad a redundant duplicatesetThemeStatecall.FileUpload.tsxhad two<input type="file">elements bound to the same ref (only the last-rendered one actually worked) — verified fixed via a real browser click test.Test plan
bunx tsc --noEmitcleanbun run lintclean (was 18 errors + 6 warnings)bun run buildsucceeds, static export intact🤖 Generated with Claude Code
https://claude.ai/code/session_01L5gqyU5QKQtLAza6hL6dXv