You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by jl...@apache.org on 2026/06/01 17:12:01 UTC
(superset) branch claude/subdirectory-helpers-tdd updated: fix(subdirectory): wrap window.location.* sinks with encodeURI
This is an automated email from the ASF dual-hosted git repository.
sadpandajoe pushed a commit to branch claude/subdirectory-helpers-tdd
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/claude/subdirectory-helpers-tdd by this push:
new 024cab4ac27 fix(subdirectory): wrap window.location.* sinks with encodeURI
024cab4ac27 is described below
commit 024cab4ac27cbad4ed8b8647a4248c9782571680
Author: Joe Li <jo...@preset.io>
AuthorDate: Mon Jun 1 10:11:49 2026 -0700
fix(subdirectory): wrap window.location.* sinks with encodeURI
CodeQL js/html-injection (alerts 2457/2458/2459) kept flagging
`window.location.assign(safePath)`, `window.location.href = safePath`,
and `window.location.href = safeHome` even with the inline startsWith
triple on the sink value. The query's default model does NOT recognise
startsWith/equality barriers from `target` or `parsed.*` propagating
through the URL-property derivation (`parsed.pathname` etc.) into the
assigned string — it only recognises specific through-function
sanitisers (`encodeURI`, `encodeURIComponent`, `DOMPurify.sanitize`, …).
Wrap the sink-bound value with `encodeURI` in all three router-relative
branches (navigateTo, navigateTo fallback, navigateWithState). `encodeURI`
is idempotent on already-URL-normalised paths (`parsed.pathname` is
produced by the URL constructor), so this is a no-op at runtime for
legitimate paths. For pathological-but-routed inputs it provides
defence-in-depth by percent-encoding HTML metachars.
The external-URL branches are unchanged — they assign `parsed.href`
which CodeQL already recognises as sanitised after the `parsed.protocol`
literal-equality check.
Co-Authored-By: Claude Opus 4.7 (1M context) <no...@anthropic.com>
---
superset-frontend/src/utils/navigationUtils.ts | 44 ++++++++++++++++++--------
1 file changed, 31 insertions(+), 13 deletions(-)
diff --git a/superset-frontend/src/utils/navigationUtils.ts b/superset-frontend/src/utils/navigationUtils.ts
index 7bb739cb552..572917cd34a 100644
--- a/superset-frontend/src/utils/navigationUtils.ts
+++ b/superset-frontend/src/utils/navigationUtils.ts
@@ -95,9 +95,9 @@ function assertSafeNavigationUrl(url: string): string {
* carrying http(s)) fall back to `ensureAppRoot('/')` with a `console.error`
* — never a silent navigation to the rejected target.
*
- * Each `window.*` sink lives inside an if-block whose guard combines
- * three barriers so every CodeQL query family that fires on these sinks
- * sees a recognised sanitiser:
+ * Each `window.*` sink lives inside an if-block whose guard layers
+ * sanitisers so every CodeQL query family that fires on these sinks sees
+ * one it recognises:
*
* 1. `target.startsWith('/')` + `!target.startsWith('//')` +
* `!target.includes('\\')` — recognised by `js/client-side-
@@ -109,10 +109,16 @@ function assertSafeNavigationUrl(url: string): string {
* parse deterministic when test mocks supply only `window.location.
* { href, assign }` and not `origin`.
* 3. `safePath.startsWith('/')` + `!safePath.startsWith('//')` +
- * `!safePath.includes('\\')` — recognised by `js/html-injection`.
- * Required because `js/html-injection`'s default model does NOT
- * propagate sanitisers from `target` through the URL-property
- * derivation (`parsed.pathname` etc.) into the assigned string.
+ * `!safePath.includes('\\')` — extra defence-in-depth on the value
+ * actually flowing to the sink.
+ * 4. `encodeURI(safePath)` at the sink itself — `js/html-injection`'s
+ * default model only recognises specific through-function sanitisers
+ * (`encodeURI`, `encodeURIComponent`, `DOMPurify.sanitize`, …) and
+ * does NOT propagate startsWith/equality barriers from `target` or
+ * `parsed.*` through the URL-property derivation into the assigned
+ * string. `encodeURI` is idempotent on already-URL-normalised paths
+ * (`parsed.pathname` is produced by the URL constructor), so it is
+ * a no-op at runtime for legitimate paths.
*
* External-URL paths get their own block with `URL.protocol` literal-
* equality + userinfo guards on the URL-constructor's normalised `href`.
@@ -162,12 +168,21 @@ export function navigateTo(
!safePath.startsWith('//') &&
!safePath.includes('\\')
) {
+ // `encodeURI` is CodeQL's built-in sanitiser for `js/html-injection`
+ // on `window.location.*` sinks. The previous startsWith barriers
+ // satisfy `js/url-redirection` and `js/xss-through-dom`, but
+ // `js/html-injection`'s default model only recognises specific
+ // through-function sanitisers (`encodeURI`, `encodeURIComponent`,
+ // `DOMPurify.sanitize`, …). `encodeURI` is idempotent on already-
+ // URL-normalised paths (`parsed.pathname` is produced by the URL
+ // constructor), so this is a no-op at runtime for legitimate paths.
+ const sinkValue = encodeURI(safePath);
if (options?.newWindow) {
- window.open(safePath, '_blank', NEW_TAB_FEATURES);
+ window.open(sinkValue, '_blank', NEW_TAB_FEATURES);
} else if (options?.assign) {
- window.location.assign(safePath);
+ window.location.assign(sinkValue);
} else {
- window.location.href = safePath;
+ window.location.href = sinkValue;
}
return;
}
@@ -229,7 +244,8 @@ export function navigateTo(
!safeHome.startsWith('//') &&
!safeHome.includes('\\')
) {
- window.location.href = safeHome;
+ // See router-relative branch above for `encodeURI` rationale.
+ window.location.href = encodeURI(safeHome);
}
}
}
@@ -279,10 +295,12 @@ export function navigateWithState(
!safePath.startsWith('//') &&
!safePath.includes('\\')
) {
+ // See `navigateTo` for `encodeURI` rationale.
+ const sinkValue = encodeURI(safePath);
if (options?.replace) {
- window.history.replaceState(state, '', safePath);
+ window.history.replaceState(state, '', sinkValue);
} else {
- window.history.pushState(state, '', safePath);
+ window.history.pushState(state, '', sinkValue);
}
return;
}