Summary
PR #842 fixed a stored XSS in the OAuth approval dialog where client_uri, policy_uri, and tos_uri fields were rendered as clickable href attributes without validating the URI scheme. javascript: URIs bypassed HTML escaping entirely. The fix added an allow-list (sanitizeHrefURL) restricting schemes to http/https.
This class of bug — untrusted string → href without scheme validation — is easy to miss in review and should be caught automatically via a Warden rule.
Current behavior
No static analysis or Warden rule exists to flag patterns like:
// dangerous: href value from external input without scheme validation
`<a href="${uri}">`Gap
HTML-escaping does not protect against javascript: URIs. Any place where user-controlled or API-sourced strings are interpolated directly into href, src, or action attributes is a latent XSS vector that code review can silently miss.
Options
- Warden semgrep rule — pattern-match template literals or string concatenation that feeds into href/src/action without passing through an allow-list sanitizer. Flag for human review.
- ESLint plugin (e.g.
eslint-plugin-no-unsanitized) — enforce at lint time in CI; blocks merges rather than async Warden alerts.
- Both — Warden for existing codebase sweep + ESLint for ongoing enforcement.
Recommendation
Ship a Warden rule first for a broad sweep across the codebase, then layer in an ESLint rule (or extend the existing config) so new instances are caught at PR time before merge.
Action taken on behalf of David Cramer.
Summary
PR #842 fixed a stored XSS in the OAuth approval dialog where
client_uri,policy_uri, andtos_urifields were rendered as clickablehrefattributes without validating the URI scheme.javascript:URIs bypassed HTML escaping entirely. The fix added an allow-list (sanitizeHrefURL) restricting schemes tohttp/https.This class of bug — untrusted string → href without scheme validation — is easy to miss in review and should be caught automatically via a Warden rule.
Current behavior
No static analysis or Warden rule exists to flag patterns like:
Gap
HTML-escaping does not protect against
javascript:URIs. Any place where user-controlled or API-sourced strings are interpolated directly intohref,src, oractionattributes is a latent XSS vector that code review can silently miss.Options
eslint-plugin-no-unsanitized) — enforce at lint time in CI; blocks merges rather than async Warden alerts.Recommendation
Ship a Warden rule first for a broad sweep across the codebase, then layer in an ESLint rule (or extend the existing config) so new instances are caught at PR time before merge.
Action taken on behalf of David Cramer.