fix(hardening): prerelease peer versions, asset-stub escaping, Flow-strip guard#71
Open
danfry1 wants to merge 2 commits into
Open
fix(hardening): prerelease peer versions, asset-stub escaping, Flow-strip guard#71danfry1 wants to merge 2 commits into
danfry1 wants to merge 2 commits into
Conversation
…trip guard - validate.ts: strip prerelease/build metadata before version comparison — "4.0.0-beta.3" parsed to [4, NaN, …] and failed the minimum check, throwing the unsupported-peer startup error for installs running betas/RCs (exactly the early-adopter cohort). A prerelease of the minimum itself is accepted. - plugin.ts asset stubs: case-insensitive extension matching (the native loader already lowercases; the engines must agree on what is an asset), regex-escaped user-supplied assetExts entries, and JSON-stringified basenames so filenames containing quotes emit valid JS (mirrors the native loader, which already stringifies). - plugin.ts Flow-strip: the "@flow" content filter is a heuristic — the marker can appear inside a string or comment of a file flow-remove-types cannot parse; that parse error previously propagated and killed the whole transform pipeline. Unparseable files now pass through untouched (a genuine Flow file that fails here would fail Vite's own parse next with a clearer error).
…-bug description, property-based test assertions Pre-PR review findings: the silent catch now warns under diagnostics with the file id (a genuine Flow file the stripper chokes on otherwise surfaces only as a downstream Vite parse error with no hint the strip was attempted); the comment/changeset overstated the old parse bug (NaN landed in the patch slot, so only prereleases sharing the minimum's major.minor failed, not all prereleases); the quote-escaping test now asserts the emitted literal round-trips rather than mirroring the implementation expression.
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.
Problem
Three unrelated hardening gaps, all found in a code audit:
satisfiesMinimumsplit versions on.only, so a prerelease sharing the minimum's major.minor — e.g. vitest4.0.0-beta.3against the4.0.0floor — parsed withNaNin the patch slot and failed the check, throwing the unsupported-peer error for exactly the early-adopter installs that run betas/RCs.LOGO.PNGfell through and broke parsing) while the native loader lowercases; user-suppliedassetExtsweren't regex-escaped (png|jswould asset-stub every.jsfile); the stubbed basename was raw-interpolated into the emitted module, so a filename containing a quote emitted broken JS.@flowcontent filter is a heuristic — the marker can appear inside a string or comment of a fileflow-remove-typescannot parse; that parse error previously propagated out of the transform hook.Change
diagnosticswith the file id.Review
Independently adversarially reviewed pre-PR: the new tests were verified to fail against main's source (real regressions, not tautologies), the native-loader parity claims were checked against
loader.mjs, and the prerelease-of-minimum trade-off was assessed against npm publishing reality. Review findings (diagnostics warning, overstated bug description, property-based assertion) are incorporated.Full gate: mock suite (incl. 16 new tests), native suites, crosscheck 75/75, lint/format/typecheck clean.