Copilot commented on code in PR #1341:
URL: https://github.com/apache/knox/pull/1341#discussion_r3739624770
##########
gateway-applications/src/main/resources/applications/knoxauth/app/login.html:
##########
@@ -36,40 +36,83 @@
<!-- Knox Theme Loader (Inline & Blocking to prevent FOUC) -->
<script type="text/javascript">
(function() {
+ // Theme names are attacker-influenced (URL parameter and
localStorage), so
+ // every candidate is validated before it is stored or used to
build a URL.
+ // Rejecting quotes, angle brackets, dots and path separators
means the only
+ // URL this can ever produce is styles/themes/<name>/theme.css
- so the set
+ // of themes actually installed on the server is the effective
allowlist,
+ // and an unknown name simply 404s and leaves the base styles
in place.
+ var THEME_PATTERN = /^[a-zA-Z0-9_-]{1,64}$/;
+
+ function sanitize(candidate) {
+ return (candidate && THEME_PATTERN.test(candidate)) ?
candidate : null;
+ }
+
// Load theme based on: deployment lock > URL parameter >
localStorage > deployment default
- var urlParams = new URLSearchParams(window.location.search);
var theme;
+ var fromSavedPreference = false;
// Check if theme is locked (admin-enforced theme)
if (typeof KNOX_THEME_LOCKED !== 'undefined' &&
KNOX_THEME_LOCKED === true) {
// Theme is locked, use deployment default only
- theme = KNOX_DEFAULT_THEME || 'default';
+ theme = sanitize(KNOX_DEFAULT_THEME) || 'default';
} else {
// Normal mode: URL parameter > localStorage > deployment
default
- theme = urlParams.get('theme');
+ var urlParams = new URLSearchParams(window.location.search);
+ var requested = sanitize(urlParams.get('theme'));
- // If theme parameter exists, save to localStorage for
persistence
- if (theme) {
+ if (requested) {
+ // Only a validated theme is persisted for future visits
try {
- localStorage.setItem('knox-auth-theme', theme);
+ localStorage.setItem('knox-auth-theme', requested);
} catch (e) {
Review Comment:
When the theme comes from the `?theme=` URL parameter, it is persisted to
localStorage, but `fromSavedPreference` remains false. That means
`link.onerror` is not attached on the first visit, so a non-existent theme
(404) will still be saved and replayed on the next visit before being cleared.
If the intent is to discard a saved preference that does not resolve to an
installed theme (as described in the PR), set the flag when successfully
persisting the URL-provided theme so the onerror cleanup runs immediately.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
