fix(dev-env): tighten option URL validation#2862
Open
sjinks wants to merge 8 commits into
Open
Conversation
Purpose and Context Dev-env SQL validation should warn about siteurl and home values only when those values come from WordPress options rows. Broad string matching can produce misleading search-replace recommendations when similar text appears in unrelated SQL data. Key Changes - Track active options-table INSERT and REPLACE statements while scanning SQL dumps. - Parse option rows by column name or default wp_options order before checking siteurl and home URL values. - Add regression coverage for supported statement variants, multiline rows, comments, and unrelated option-like data. Impact and Considerations This narrows validation warnings without changing import execution. No migrations, configuration changes, or deployment steps are required. The scanner remains purpose-built for supported dump shapes rather than a full SQL parser. Testing and Validation - npx jest --runTestsByPath __tests__/lib/validations/sql.js --runInBand - npx eslint src/lib/validations/sql.ts __tests__/lib/validations/sql.js - npm run check-types - git diff --check -- src/lib/validations/sql.ts __fixtures__/validations/bad-sql-dev-env.sql __tests__/lib/validations/sql.js
Contributor
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR tightens dev-env SQL validation so siteurl and home domain warnings are produced from parsed wp_options insert rows instead of broad string matching.
Changes:
- Adds stateful parsing for
INSERT/REPLACEstatements targetingwp_optionsand numeric multisite options tables. - Extracts
option_name/option_valueusing explicit column lists or default options-table ordering. - Expands fixtures and Jest coverage for comments, multiline statements, reordered columns, qualified tables, and non-options tables.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/lib/validations/sql.ts |
Replaces broad siteurl/home matching with parsed options-table row detection. |
__tests__/lib/validations/sql.js |
Adds assertions for supported SQL shapes and ignored false-positive cases. |
__fixtures__/validations/bad-sql-dev-env.sql |
Adds SQL examples covering parser edge cases and regression scenarios. |
Purpose and Context The dev-env SQL validator added a small INSERT/REPLACE parser inside sql.ts. The pure helpers are large enough to warrant a dedicated module and dedicated unit coverage. Keeping them inline mixes a stateful check-runner with a self-contained parser, which makes the parser harder to understand and harder to test. Key Changes - Move the pure (state-free) helpers, constants, and interfaces to src/lib/validations/sql-insert-parser.ts. - Replace inline definitions in src/lib/validations/sql.ts with a named import from the new module. - Leave the six module-level currentInsertStatement* variables and the five stateful helpers (collectInsertColumnList, isSqlCommentOnlyLine, collectOptionUrlMatchesFromRows, collectOptionsInsertRows, collectOptionsInsertMatches) in sql.ts so this commit is purely a code move. Impact and Considerations No behavior change. The extracted helpers have byte-identical bodies modulo prettier's printWidth: 100 wrap on three signature lines that crossed the limit once `export ` was prepended. The existing integration suite at __tests__/lib/validations/sql.js remains the authoritative behavior-preservation evidence and passes unchanged (26/26). Testing and Validation - npx jest --runTestsByPath __tests__/lib/validations/sql.js --runInBand - npx eslint src/lib/validations/sql.ts src/lib/validations/sql-insert-parser.ts - npm run check-types
Purpose and Context The pure helpers extracted into sql-insert-parser.ts were previously verified only through fixture-driven integration tests. A dedicated unit-test layer localizes regressions to the specific helper rather than reporting them as missing warning text downstream. Key Changes - Add __tests__/lib/validations/sql-insert-parser.js with one describe per helper (14 helpers + constants block). - Cover edge cases for SQL quoting (single, double, doubled, backslash-escaped), comment styles (--, #, block, inline, inside-string), qualified and backtick-quoted table names, INSERT modifiers, multisite numeric options tables, reordered column lists, and multiline tuple buffers. - Lock the parseSqlTupleRows contract that values keep their surrounding quotes (unquoteSqlValue is the separate unwrapper). Impact and Considerations Tests only. No production-code change. Total 98 assertions across 15 describe blocks. Testing and Validation - npx jest --runTestsByPath __tests__/lib/validations/sql-insert-parser.js --runInBand - npx eslint __tests__/lib/validations/sql-insert-parser.js - npm run check-types
Purpose and Context A review comment identified that multiline block comments could still allow commented options-table rows to be parsed as real siteurl/home rows. The parser stripped an unterminated block comment on the current line, but insert detection did not carry comment state across the SQL stream. Key Changes - Add stream-level SQL comment stripping state for insert detection. - Reuse the stripped physical line for insert detection, option-row collection, and statement reset. - Rewrite the comment scanner as an explicit while loop so it no longer assigns to a for-loop counter. - Add regression coverage for top-level and active-insert multiline block comments containing commented options rows. Impact and Considerations This narrows dev-env SQL validation warnings by ignoring options rows inside multiline block comments. Generic non-insert validation checks continue to read the raw line, preserving the existing validation surface outside options-row URL suggestions. No migrations, configuration changes, or deployment steps are required. Testing and Validation - npx jest --runTestsByPath __tests__/lib/validations/sql-insert-parser.js --runInBand - npx jest --runTestsByPath __tests__/lib/validations/sql.js --runInBand - npx eslint src/lib/validations/sql-insert-parser.ts src/lib/validations/sql.ts __tests__/lib/validations/sql-insert-parser.js __tests__/lib/validations/sql.js - npm run check-types - npm test
Purpose and Context MySQL accepts the singular VALUE keyword for single-row inserts. The parsed options-row validation path only recognized VALUES, so valid wp_options rows using VALUE were skipped and dev-env validation missed siteurl/home URL warnings that the previous broad matcher would have seen. Key Changes - Add centralized VALUE/VALUES keyword detection with matched bounds. - Use the matched keyword end index when option-row tuple parsing starts, instead of assuming the plural VALUES length. - Keep parser keyword boundaries aligned with the SQL identifier character set, including underscores and dollar signs. - Add unit and fixture-backed coverage for singular VALUE siteurl and home rows, column-list disambiguation, and false-positive tokens. Impact and Considerations This extends the supported options-row dump shapes without changing warning copy or validation policy. Existing plural VALUES behavior is preserved, and larger tokens such as NOVALUES, value_backup, and VALUE$backup are not treated as SQL value-list keywords. Testing and Validation - npx jest --runTestsByPath __tests__/lib/validations/sql-insert-parser.js --runInBand - npx jest --runTestsByPath __tests__/lib/validations/sql.js --runInBand - npx eslint src/lib/validations/sql-insert-parser.ts src/lib/validations/sql.ts __tests__/lib/validations/sql-insert-parser.js __tests__/lib/validations/sql.js - npm run check-types - npm test
|
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
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.
Description
Tightens dev-env SQL validation so
siteurlandhomeURL warnings are based on parsed WordPress options-table rows instead of broad string matches. This avoids misleading search-replace recommendations when option-like text appears in unrelated SQL data, comments, or non-options tables.The scanner now tracks active
INSERTandREPLACEstatements forwp_optionsand numeric multisite options tables, pairsoption_namewithoption_valueby explicit column list or defaultwp_optionsorder, and handles common dump shapes such as modifiers, qualified table names, multiline column lists, multiline tuples, and comments.Changelog Description
Fixed
siteurlandhomesearch-replace recommendations from unrelated SQL data.Pull request checklist
New release checklist
Steps to Test
npx jest --runTestsByPath __tests__/lib/validations/sql.js --runInBand.npx eslint src/lib/validations/sql.ts __tests__/lib/validations/sql.js.npm run check-types.siteurl/homewarnings only for parsedwp_optionsrows and ignores unrelated table rows, comments, non-URL values, and pseudo options tables.