Skip to content

Commit 92089bf

Browse files
authored
Merge pull request #1 from StudioLambda/fix/audit-high-severity-issues
Fix high-severity audit findings across matcher, router, CI, and build
2 parents 0f5ae06 + 403c3da commit 92089bf

10 files changed

Lines changed: 496 additions & 322 deletions

File tree

.github/workflows/ci.yml

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,7 @@ concurrency:
1111
cancel-in-progress: true
1212

1313
permissions:
14-
contents: write
15-
id-token: write
14+
contents: read
1615

1716
jobs:
1817
check:
@@ -74,6 +73,10 @@ jobs:
7473
runs-on: ubuntu-latest
7574
needs: [build]
7675

76+
permissions:
77+
contents: write
78+
id-token: write
79+
7780
# Only release on push to main, not on PRs
7881
if: ${{ github.event_name == 'push' }}
7982

package-lock.json

Lines changed: 333 additions & 311 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@
100100
"react": "^19.2.0",
101101
"react-dom": "^19.2.0",
102102
"typescript": "^6.0.2",
103-
"vite": "^8.0.3",
103+
"vite": "^8.0.8",
104104
"vite-plugin-dts": "^4.5.4",
105105
"vitest": "^4.0.7"
106106
}

src/react/components/Router.tsx

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,18 @@ export interface RouterProps {
145145
*/
146146
export function Router(options: RouterProps) {
147147
const contextNavigation = use(NavigationContext)
148-
const navigation: Navigation = options.navigation ?? contextNavigation ?? window.navigation
148+
const navigation: Navigation =
149+
options.navigation ??
150+
contextNavigation ??
151+
(typeof window !== 'undefined' ? window.navigation : undefined)!
152+
153+
if (navigation === undefined || navigation === null) {
154+
throw new Error(
155+
'Router requires a navigation prop, NavigationContext provider, ' +
156+
'or browser Navigation API support. ' +
157+
'Use createMemoryNavigation() for SSR or non-browser environments.'
158+
)
159+
}
149160
const matcher: Matcher<Handler> = options.matcher ?? use(MatcherContext)
150161
const internalTransition = useTransition()
151162
const transition = options.transition ?? internalTransition

src/react/createRouter.test.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -394,6 +394,33 @@ describe('createRouter', { concurrent: true }, function () {
394394
expect(receivedUrl?.search).toBe('?q=test')
395395
expect(context.controller.redirect).toHaveBeenCalledWith('/new-search?q=test')
396396
})
397+
398+
it('does not inherit middleware from parent groups', function ({ expect }) {
399+
const Auth = createMiddleware()
400+
401+
const router = createRouter(function (route) {
402+
const authed = route().middleware([Auth]).group()
403+
404+
authed('/old').redirect('/new')
405+
})
406+
407+
const match = router.match('/old')
408+
409+
expect(match).not.toBeNull()
410+
expect(match?.handler.middlewares).toBeUndefined()
411+
})
412+
413+
it('does not inherit scroll or focusReset configuration', function ({ expect }) {
414+
const router = createRouter(function (route) {
415+
route('/old').scroll('manual').focusReset('manual').redirect('/new')
416+
})
417+
418+
const match = router.match('/old')
419+
420+
expect(match).not.toBeNull()
421+
expect(match?.handler.scroll).toBeUndefined()
422+
expect(match?.handler.focusReset).toBeUndefined()
423+
})
397424
})
398425

399426
describe('groups', { concurrent: true }, function () {

src/react/createRouter.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -412,14 +412,11 @@ function createRouteFactory(matcher: Matcher<Handler>, inherited: InheritedConfi
412412

413413
const handler: Handler = {
414414
component: RedirectFallback,
415-
middlewares: resolveMiddlewares(),
416415
prefetch: function (context) {
417416
const resolved = typeof target === 'function' ? target(context) : target
418417

419418
context.controller.redirect(resolved)
420419
},
421-
scroll: state.scroll,
422-
focusReset: state.focusReset,
423420
}
424421

425422
matcher.register(fullPath, handler)

src/react/hooks/useNextMatch.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,6 @@ export function useNextMatch(options?: NextMatchOptions) {
4949
return fallbackResolved
5050
}
5151

52-
return matcher.match(new URL(destination).pathname) ?? fallbackResolved
52+
return matcher.match(new URL(destination, 'http://localhost').pathname) ?? fallbackResolved
5353
}
5454
}

src/router/matcher.test.ts

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,5 +175,103 @@ describe('router', function () {
175175
expect(route).not.toBeNull()
176176
expect(route?.params).toStrictEqual({ id: '42', path: 'docs/readme.md' })
177177
})
178+
179+
it('matches a route with handler value 0', function ({ expect }) {
180+
const router = createMatcher<number>()
181+
182+
router.register('/zero', 0)
183+
184+
const route = router.match('/zero')
185+
186+
expect(route).not.toBeNull()
187+
expect(route?.handler).toBe(0)
188+
})
189+
190+
it('matches a route with handler value empty string', function ({ expect }) {
191+
const router = createMatcher<string>()
192+
193+
router.register('/empty', '')
194+
195+
const route = router.match('/empty')
196+
197+
expect(route).not.toBeNull()
198+
expect(route?.handler).toBe('')
199+
})
200+
201+
it('matches a route with handler value false', function ({ expect }) {
202+
const router = createMatcher<boolean>()
203+
204+
router.register('/false', false)
205+
206+
const route = router.match('/false')
207+
208+
expect(route).not.toBeNull()
209+
expect(route?.handler).toBe(false)
210+
})
211+
212+
it('matches a wildcard route with handler value 0', function ({ expect }) {
213+
const router = createMatcher<number>()
214+
215+
router.register('/files/*path', 0)
216+
217+
const route = router.match('/files/readme.md')
218+
219+
expect(route).not.toBeNull()
220+
expect(route?.handler).toBe(0)
221+
expect(route?.params).toStrictEqual({ path: 'readme.md' })
222+
})
223+
224+
it('matches root handler', function ({ expect }) {
225+
const router = createMatcher<number>()
226+
227+
router.register('/', 1)
228+
229+
const route = router.match('/')
230+
231+
expect(route).not.toBeNull()
232+
expect(route?.handler).toBe(1)
233+
})
234+
})
235+
236+
describe('param name conflicts', function () {
237+
it('throws on conflicting dynamic param names at the same level', function ({ expect }) {
238+
const router = createMatcher<number>()
239+
240+
router.register('/user/:id/profile', 1)
241+
242+
expect(function () {
243+
router.register('/user/:name/settings', 2)
244+
}).toThrow('conflicting dynamic param name')
245+
})
246+
247+
it('throws on conflicting wildcard param names at the same level', function ({ expect }) {
248+
const router = createMatcher<number>()
249+
250+
router.register('/files/*path', 1)
251+
252+
expect(function () {
253+
router.register('/files/*filepath', 2)
254+
}).toThrow('conflicting wildcard param name')
255+
})
256+
257+
it('allows the same dynamic param name at the same level', function ({ expect }) {
258+
const router = createMatcher<number>()
259+
260+
router.register('/user/:id/profile', 1)
261+
262+
expect(function () {
263+
router.register('/user/:id/settings', 2)
264+
}).not.toThrow()
265+
})
266+
267+
it('allows the same wildcard param name at the same level', function ({ expect }) {
268+
const router = createMatcher<number>()
269+
270+
router.register('/files/*path', 1)
271+
272+
expect(function () {
273+
router.register('/other/*path', 2)
274+
}).not.toThrow()
275+
})
178276
})
179277
})

src/router/matcher.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,11 @@ export function createMatcher<T>(options?: Options<T>): Matcher<T> {
156156

157157
if (!node.wildcard) {
158158
node.wildcard = { children: new Map(), name }
159+
} else if (node.wildcard.name !== name) {
160+
throw new Error(
161+
`conflicting wildcard param name at "${pattern}": ` +
162+
`existing "*${node.wildcard.name}" vs new "*${name}"`
163+
)
159164
}
160165

161166
node = node.wildcard
@@ -167,6 +172,11 @@ export function createMatcher<T>(options?: Options<T>): Matcher<T> {
167172

168173
if (!node.child) {
169174
node.child = { children: new Map(), name }
175+
} else if (node.child.name !== name) {
176+
throw new Error(
177+
`conflicting dynamic param name at "${pattern}": ` +
178+
`existing ":${node.child.name}" vs new ":${name}"`
179+
)
170180
}
171181

172182
node = node.child
@@ -216,7 +226,7 @@ export function createMatcher<T>(options?: Options<T>): Matcher<T> {
216226
params: Record<string, string>
217227
): Resolved<T> | null {
218228
if (index === segments.length) {
219-
return node.handler ? { handler: node.handler, params } : null
229+
return node.handler !== undefined ? { handler: node.handler, params } : null
220230
}
221231

222232
const segment = segments[index]
@@ -242,7 +252,7 @@ export function createMatcher<T>(options?: Options<T>): Matcher<T> {
242252
}
243253
}
244254

245-
if (node.wildcard && node.wildcard.name && node.wildcard.handler) {
255+
if (node.wildcard && node.wildcard.name && node.wildcard.handler !== undefined) {
246256
const rest = segments.slice(index).join('/')
247257

248258
return {

vite.config.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,13 @@ export default defineConfig({
4646
}),
4747
dts({
4848
include: ['**/*.ts*'],
49-
exclude: ['**/*.test.ts*'],
49+
exclude: [
50+
'**/*.test.ts*',
51+
'**/example.tsx',
52+
'**/ExampleMain.tsx',
53+
'**/examples/**',
54+
'**/test-helpers.ts',
55+
],
5056
outDir: '../../dist',
5157
root: './src/react',
5258
}),

0 commit comments

Comments
 (0)