Skip to content

Improve password rules validation errors#1094

Merged
rmondello merged 1 commit into
apple:mainfrom
LeSingh1:improve-password-rules-validation
May 18, 2026
Merged

Improve password rules validation errors#1094
rmondello merged 1 commit into
apple:mainfrom
LeSingh1:improve-password-rules-validation

Conversation

@LeSingh1

Copy link
Copy Markdown
Contributor

Summary

Adds a CI lint step that parses every "password-rules" value in quirks/password-rules.json using the existing tools/PasswordRulesParser.js. Any parse error is reported with the offending domain and the parser's error message, causing the password-rules lint job to fail.

This addresses #907, where a small formatting error (a comma instead of a semicolon between two required: properties for hetzner.com) landed without CI catching it and only surfaced later as a downstream parser exception.

The new check piggybacks on the existing password-rules job (sort-order check) and adds two steps: actions/setup-node@v6 and node .github/workflows/lint-scripts/password-rules-parse.js. No other lint behavior changes.

How it works

password-rules-parse.js loads tools/PasswordRulesParser.js via node:vm into a sandbox whose console.error is captured. It iterates every domain in password-rules.json, calls parsePasswordRules(), 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

  • Run node .github/workflows/lint-scripts/password-rules-parse.js on mainOK: 419 password-rules entries parsed cleanly. (exit 0)
  • Reintroduce the original hetzner.com comma typo from Improve password-rules.json error catching in CI #907 → script fails with 1 domain(s) in password-rules.json failed to parse. and identifies hetzner.com (exit 1)
  • Restore correct value → clean again (exit 0)
  • python -c "import yaml; yaml.safe_load(open('.github/workflows/lint.yml'))" → YAML valid

Risks / follow-ups

  • The script only fails on console.error from the parser; recoverable warnings (e.g. the console.warn about a - inside a character class) are intentionally ignored to avoid noisy false positives. If desired, a follow-up could promote those to errors.
  • This does not introduce any runtime dependencies — it uses only Node built-ins (fs, path, vm).

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 rmondello merged commit 1e4d28f into apple:main May 18, 2026
5 checks passed
@rmondello

Copy link
Copy Markdown
Contributor

@LeSingh1 Awesome contribution! Thank you!

I think the next step here is to ensure that we "fail" in the CI if - is in a character class, but isn't the first character of the class and/or if ] is in a character class, but isn't the last character of the class. That's a mistake that people make a lot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants