Merge pull request #2 from StudioLambda/fix/medium-severity-audit-findings

Fix medium-severity audit findings across router, hooks, and matcher

Author: ConsoleTVs

src/react/components/Router.tsx

@@ -22,6 +22,7 @@ import { useNavigationEvents } from 'router/react:hooks/useNavigationEvents'
 import { MatcherContext } from 'router/react:context/MatcherContext'
 import { TransitionContext } from 'router/react:context/TransitionContext'
 import { PathnameContext } from 'router/react:context/PathnameContext'
+import { UrlContext } from 'router/react:context/UrlContext'
 import { extractPathname } from 'router/react:extractPathname'
 
 /**
@@ -57,6 +58,14 @@ interface CurrentState {
    * active links and read the current location.
    */
   pathname: string
+
+  /**
+   * The full destination URL string for this navigation.
+   * Used by `useSearchParams` to derive search parameters
+   * from React state rather than reading the mutable
+   * `navigation.currentEntry` during render.
+   */
+  url: string | null
 }
 
 /**
@@ -171,6 +180,7 @@ export function Router(options: RouterProps) {
       signal: null,
       navigationType: null,
       pathname: extractPathname(url),
+      url,
     }
   })
 
@@ -223,6 +233,7 @@ export function Router(options: RouterProps) {
         signal: event.signal,
         navigationType: event.navigationType,
         pathname: extractPathname(event.destination.url),
+        url: event.destination.url,
       })
     })
 
@@ -250,13 +261,15 @@ export function Router(options: RouterProps) {
           <NavigationTypeContext value={current.navigationType}>
             <NavigationSignalContext value={current.signal}>
               <PathnameContext value={current.pathname}>
-                <ParamsContext value={current.match.params}>
-                  <Suspense fallback={options.fallback}>
-                    <Middlewares value={middlewares}>
-                      <CurrentComponent />
-                    </Middlewares>
-                  </Suspense>
-                </ParamsContext>
+                <UrlContext value={current.url}>
+                  <ParamsContext value={current.match.params}>
+                    <Suspense fallback={options.fallback}>
+                      <Middlewares value={middlewares}>
+                        <CurrentComponent />
+                      </Middlewares>
+                    </Suspense>
+                  </ParamsContext>
+                </UrlContext>
               </PathnameContext>
             </NavigationSignalContext>
           </NavigationTypeContext>

src/react/context/NavigationContext.ts

@@ -10,4 +10,4 @@ import { createContext } from 'react'
  * The `useNavigation` hook throws a descriptive error when consumed
  * without a provider.
  */
-export const NavigationContext = createContext<Navigation>(null as unknown as Navigation)
+export const NavigationContext = createContext<Navigation | null>(null)

src/react/context/UrlContext.ts

@@ -0,0 +1,14 @@
+import { createContext } from 'react'
+
+/**
+ * Provides the full committed URL string to descendant
+ * components. Updated by the Router on every navigation
+ * with the destination URL. Defaults to `null` when no
+ * Router is present in the tree.
+ *
+ * Consumed by `useSearchParams` to derive search parameters
+ * from React state rather than reading the mutable
+ * `navigation.currentEntry` during render — preventing
+ * subscription tearing in concurrent mode.
+ */
+export const UrlContext = createContext<string | null>(null)

src/react/createRouter.test.ts

@@ -838,4 +838,205 @@ describe('createRouter', { concurrent: true }, function () {
       expect(router.match('/dashboard/settings')?.handler.component).toBe(DashSettings)
     })
   })
+
+  describe('redirect cycle detection', { concurrent: true }, function () {
+    it('throws on a self-redirect', function ({ expect }) {
+      expect(function () {
+        createRouter(function (route) {
+          route('/loop').redirect('/loop')
+        })
+      }).toThrow('redirect cycle detected')
+    })
+
+    it('throws on a two-hop redirect cycle', function ({ expect }) {
+      expect(function () {
+        createRouter(function (route) {
+          route('/a').redirect('/b')
+          route('/b').redirect('/a')
+        })
+      }).toThrow('redirect cycle detected')
+    })
+
+    it('throws on a multi-hop redirect cycle', function ({ expect }) {
+      expect(function () {
+        createRouter(function (route) {
+          route('/a').redirect('/b')
+          route('/b').redirect('/c')
+          route('/c').redirect('/a')
+        })
+      }).toThrow('redirect cycle detected')
+    })
+
+    it('allows non-cyclic redirect chains', function ({ expect }) {
+      expect(function () {
+        createRouter(function (route) {
+          route('/a').redirect('/b')
+          route('/b').redirect('/c')
+          route('/c').render(Stub)
+        })
+      }).not.toThrow()
+    })
+
+    it('skips cycle detection for callback redirects', function ({ expect }) {
+      expect(function () {
+        createRouter(function (route) {
+          route('/a').redirect(function () {
+            return '/a'
+          })
+        })
+      }).not.toThrow()
+    })
+  })
+
+  describe('builder consumed guard', { concurrent: true }, function () {
+    it('throws when calling middleware after render', function ({ expect }) {
+      expect(function () {
+        createRouter(function (route) {
+          const builder = route('/page')
+
+          builder.render(Stub)
+          builder.middleware([createMiddleware()])
+        })
+      }).toThrow('cannot call .middleware() on a route builder')
+    })
+
+    it('throws when calling prefetch after render', function ({ expect }) {
+      expect(function () {
+        createRouter(function (route) {
+          const builder = route('/page')
+
+          builder.render(Stub)
+          builder.prefetch(vi.fn())
+        })
+      }).toThrow('cannot call .prefetch() on a route builder')
+    })
+
+    it('throws when calling scroll after render', function ({ expect }) {
+      expect(function () {
+        createRouter(function (route) {
+          const builder = route('/page')
+
+          builder.render(Stub)
+          builder.scroll('manual')
+        })
+      }).toThrow('cannot call .scroll() on a route builder')
+    })
+
+    it('throws when calling focusReset after render', function ({ expect }) {
+      expect(function () {
+        createRouter(function (route) {
+          const builder = route('/page')
+
+          builder.render(Stub)
+          builder.focusReset('manual')
+        })
+      }).toThrow('cannot call .focusReset() on a route builder')
+    })
+
+    it('throws when calling formHandler after render', function ({ expect }) {
+      expect(function () {
+        createRouter(function (route) {
+          const builder = route('/page')
+
+          builder.render(Stub)
+          builder.formHandler(vi.fn())
+        })
+      }).toThrow('cannot call .formHandler() on a route builder')
+    })
+
+    it('throws when calling render twice', function ({ expect }) {
+      expect(function () {
+        createRouter(function (route) {
+          const builder = route('/page')
+
+          builder.render(Stub)
+          builder.render(createStub())
+        })
+      }).toThrow('cannot call .render() on a route builder')
+    })
+
+    it('throws when calling redirect after render', function ({ expect }) {
+      expect(function () {
+        createRouter(function (route) {
+          const builder = route('/page')
+
+          builder.render(Stub)
+          builder.redirect('/other')
+        })
+      }).toThrow('cannot call .redirect() on a route builder')
+    })
+
+    it('throws when calling group after render', function ({ expect }) {
+      expect(function () {
+        createRouter(function (route) {
+          const builder = route('/page')
+
+          builder.render(Stub)
+          builder.group()
+        })
+      }).toThrow('cannot call .group() on a route builder')
+    })
+
+    it('throws when calling render after redirect', function ({ expect }) {
+      expect(function () {
+        createRouter(function (route) {
+          const builder = route('/page')
+
+          builder.redirect('/other')
+          builder.render(Stub)
+        })
+      }).toThrow('cannot call .render() on a route builder')
+    })
+
+    it('throws when calling render after group', function ({ expect }) {
+      expect(function () {
+        createRouter(function (route) {
+          const builder = route('/page')
+
+          builder.group()
+          builder.render(Stub)
+        })
+      }).toThrow('cannot call .render() on a route builder')
+    })
+  })
+
+  describe('duplicate route registration', { concurrent: true }, function () {
+    it('throws when registering the same path twice with render', function ({ expect }) {
+      expect(function () {
+        createRouter(function (route) {
+          route('/page').render(Stub)
+          route('/page').render(createStub())
+        })
+      }).toThrow('duplicate route registration')
+    })
+
+    it('throws when registering the same path with render and redirect', function ({ expect }) {
+      expect(function () {
+        createRouter(function (route) {
+          route('/page').render(Stub)
+          route('/page').redirect('/other')
+        })
+      }).toThrow('duplicate route registration')
+    })
+
+    it('throws when registering the same nested path twice', function ({ expect }) {
+      expect(function () {
+        createRouter(function (route) {
+          const app = route('/app').group()
+
+          app('/page').render(Stub)
+          app('/page').render(createStub())
+        })
+      }).toThrow('duplicate route registration')
+    })
+
+    it('allows registering different paths', function ({ expect }) {
+      expect(function () {
+        createRouter(function (route) {
+          route('/a').render(Stub)
+          route('/b').render(createStub())
+        })
+      }).not.toThrow()
+    })
+  })
 })