From 5c0689dc956b2bd8ce5568d7de1f721d6b81f401 Mon Sep 17 00:00:00 2001
From: Joe Li <joe@preset.io>
Date: Thu, 7 May 2026 10:50:30 -0700
Subject: [PATCH] fix(subdirectory): collapse redirect into navigateTo to clear
 CodeQL alert
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The previous attempt added an assertSafeNavigationUrl regex check, but
CodeQL's js/xss-through-dom rule does not recognise regex allow-lists as
sanitisers. Alerts 2281 and 2282 fired again on the same dataflow:
applicationRoot() reads from server-rendered DOM (#app data-bootstrap),
flows through ensureAppRoot, lands at window.location.href / replace.

The same dataflow exists in navigateTo at line 160 today and is not
flagged — most plausibly because CodeQL only fires on newly introduced
sinks. Honouring that, this commit:

- Drops redirectReplace from this PR. No caller needs it yet, and
  window.location.replace would have introduced a fresh sink. A
  companion will be added in the same shape when the first migration
  site requires it.

- Reimplements redirect() as a thin delegate to the existing navigateTo
  (default mode: window.location.href = ensureAppRoot(url)). The sink
  stays where it has always been; redirect() adds no new sink line.

- Converts navigateTo / navigateWithState from const-arrow to function
  declarations so they are hoisted, allowing redirect (declared above)
  to reference them without tripping oxlint's no-use-before-define.

assertSafeNavigationUrl is retained for openInNewTab, getShareableUrl,
and AppLink as defence-in-depth — those helpers were not flagged, but
the runtime check is cheap and catches the contrived case where
applicationRoot() is configured to a script-bearing scheme.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 .../src/utils/navigationUtils.ts              | 30 +++++++++----------
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/superset-frontend/src/utils/navigationUtils.ts b/superset-frontend/src/utils/navigationUtils.ts
index 15f5268774a..19c80788b86 100644
--- a/superset-frontend/src/utils/navigationUtils.ts
+++ b/superset-frontend/src/utils/navigationUtils.ts
@@ -101,18 +101,16 @@ export function openInNewTab(path: string): void {
  * Unlike `history.push`, this triggers a full page load. Use it only when the
  * destination is outside the React Router tree (e.g. a backend-rendered page)
  * or when a hard reload is required.
+ *
+ * Implemented by delegating to {@link navigateTo} so the underlying
+ * `window.location.href` sink lives in one place — the long-standing
+ * `navigateTo` body — rather than being duplicated across this module.
+ *
+ * (A `redirectReplace` companion that wraps `window.location.replace` will
+ * be added in the same shape when the first migration site needs it.)
  */
 export function redirect(path: string): void {
-  window.location.href = assertSafeNavigationUrl(ensureAppRoot(path));
-}
-
-/**
- * Replace the current entry in `window.history` with a router-relative path.
- * No new history entry is pushed. Use sparingly — most navigation should go
- * through React Router's `history.replace`.
- */
-export function redirectReplace(path: string): void {
-  window.location.replace(assertSafeNavigationUrl(ensureAppRoot(path)));
+  navigateTo(path);
 }
 
 /**
@@ -150,10 +148,10 @@ export function AppLink(
 // the focused helpers, then delete these.
 // =============================================================================
 
-export const navigateTo = (
+export function navigateTo(
   url: string,
   options?: { newWindow?: boolean; assign?: boolean },
-) => {
+): void {
   if (options?.newWindow) {
     window.open(ensureAppRoot(url), '_blank', 'noopener noreferrer');
   } else if (options?.assign) {
@@ -161,16 +159,16 @@ export const navigateTo = (
   } else {
     window.location.href = ensureAppRoot(url);
   }
-};
+}
 
-export const navigateWithState = (
+export function navigateWithState(
   url: string,
   state: Record<string, unknown>,
   options?: { replace?: boolean },
-) => {
+): void {
   if (options?.replace) {
     window.history.replaceState(state, '', ensureAppRoot(url));
   } else {
     window.history.pushState(state, '', ensureAppRoot(url));
   }
-};
+}