Improve password rules validation errors#1094
Merged
Merged
Conversation
Adds a Node-based lint step that loads tools/PasswordRulesParser.js and calls parsePasswordRules() on every "password-rules" string in quirks/password-rules.json. Any parse error (such as the comma-instead-of- semicolon hetzner.com regression referenced in apple#907) is reported with the offending domain and causes the lint job to fail. Fixes apple#907
rmondello
approved these changes
May 18, 2026
erynofwales
approved these changes
May 18, 2026
Contributor
|
@LeSingh1 Awesome contribution! Thank you! I think the next step here is to ensure that we "fail" in the CI if |
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.
Summary
Adds a CI lint step that parses every
"password-rules"value inquirks/password-rules.jsonusing the existingtools/PasswordRulesParser.js. Any parse error is reported with the offending domain and the parser's error message, causing thepassword-ruleslint job to fail.This addresses #907, where a small formatting error (a comma instead of a semicolon between two
required:properties forhetzner.com) landed without CI catching it and only surfaced later as a downstream parser exception.The new check piggybacks on the existing
password-rulesjob (sort-order check) and adds two steps:actions/setup-node@v6andnode .github/workflows/lint-scripts/password-rules-parse.js. No other lint behavior changes.How it works
password-rules-parse.jsloadstools/PasswordRulesParser.jsvianode:vminto a sandbox whoseconsole.erroris captured. It iterates every domain inpassword-rules.json, callsparsePasswordRules(), and exits non-zero if any rule produced a parser error. The parser is loaded unchanged — it remains usable by Safari/WebKit consumers that expect a plain script.Test plan
node .github/workflows/lint-scripts/password-rules-parse.jsonmain→OK: 419 password-rules entries parsed cleanly.(exit 0)hetzner.comcomma typo from Improve password-rules.json error catching in CI #907 → script fails with1 domain(s) in password-rules.json failed to parse.and identifieshetzner.com(exit 1)python -c "import yaml; yaml.safe_load(open('.github/workflows/lint.yml'))"→ YAML validRisks / follow-ups
console.errorfrom the parser; recoverable warnings (e.g. theconsole.warnabout a-inside a character class) are intentionally ignored to avoid noisy false positives. If desired, a follow-up could promote those to errors.fs,path,vm).