Skip to content

fix: validate server fn id during dev #6087

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
schiller-manuel merged 1 commit into from
Dec 13, 2025
Merged

fix: validate server fn id during dev #6087

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 36 additions & 5 deletions packages/server-functions-plugin/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
/// <reference types="vite/client" />
import crypto from 'node:crypto'
import assert from 'node:assert'
import { TanStackDirectiveFunctionsPluginEnv } from '@tanstack/directive-functions-plugin'
import type { DevEnvironment, Plugin } from 'vite'
import type { Plugin } from 'vite'
import type {
DirectiveFn,
GenerateFunctionIdFn,
Expand Down Expand Up @@ -39,16 +40,26 @@ const debug =
process.env.TSR_VITE_DEBUG &&
['true', 'server-functions-plugin'].includes(process.env.TSR_VITE_DEBUG)

const validateServerFnIdVirtualModule = `virtual:tanstack-start-validate-server-fn-id`

function parseIdQuery(id: string): {
filename: string
query: {
[k: string]: string
}
} {
if (!id.includes('?')) return { filename: id, query: {} }
const [filename, rawQuery] = id.split(`?`, 2) as [string, string]
const query = Object.fromEntries(new URLSearchParams(rawQuery))
return { filename, query }
}
Comment on lines +43 to +55
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

parseIdQuery is fine, but it makes “missing id” look like an assertion failure.

parseIdQuery never returns falsy, so assert(parsed) (Line 177) is redundant, and assert(parsed.query.id) will throw an AssertionError instead of a clear Vite error. Prefer an explicit check and this.error('Missing ...') for consistency with the invalid-id path.

🤖 Prompt for AI Agents 
In packages/server-functions-plugin/src/index.ts around lines 43 to 55, remove
the redundant assert(parsed) usage and replace the assertion that
parsed.query.id exists with an explicit runtime check that calls
this.error('Missing id in virtual module request') (or a message matching the
invalid-id path) when parsed.query.id is absent; since parseIdQuery always
returns an object, rely on that and use this.error to raise a clear Vite error
instead of throwing an AssertionError.


export function TanStackServerFnPlugin(
opts: TanStackServerFnPluginOpts,
): Array<Plugin> {
const directiveFnsById: Record<string, DirectiveFn> = {}
let serverDevEnv: DevEnvironment | undefined

const onDirectiveFnsById = (d: Record<string, DirectiveFn>) => {
if (serverDevEnv) {
return
}
if (debug) {
console.info(`onDirectiveFnsById received: `, d)
}
Expand Down Expand Up @@ -152,6 +163,24 @@ export function TanStackServerFnPlugin(
provider: opts.provider,
callers: opts.callers,
}),
{
name: 'tanstack-start-server-fn-vite-plugin-validate-serverfn-id',
apply: 'serve',
load: {
filter: {
id: new RegExp(resolveViteId(validateServerFnIdVirtualModule)),
},
handler(id) {
const parsed = parseIdQuery(id)
assert(parsed)
assert(parsed.query.id)
if (directiveFnsById[parsed.query.id]) {
return `export {}`
}
this.error(`Invalid server function ID: ${parsed.query.id}`)
},
},
},
Comment on lines 166 to 183
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Fix the load.filter.id regex pattern: the quantifier ? is positioned incorrectly and unescaped strings are unsafe in RegExp.

The current pattern `^${resolveViteId(validateServerFnIdVirtualModule)}?.+` makes the last character of the resolved ID optional rather than making the query string optional. This is a logic error that can silently match unintended IDs or fail to match valid ones. The unescaped dynamic string is also brittle; if special regex characters appear, the pattern breaks silently.

Additionally, the assertions at lines 177–178 (assert(parsed) and assert(parsed.query.id)) will throw cryptic errors instead of returning proper validation failures. Use explicit error checks instead:

 id: new RegExp(
-  `^${resolveViteId(validateServerFnIdVirtualModule)}?.+`,
+  `^${escapeRegExp(resolveViteId(validateServerFnIdVirtualModule))}\\?id=.+$`,
 ),
 
 handler(id) {
   const parsed = parseIdQuery(id)
-  assert(parsed)
-  assert(parsed.query.id)
-  if (directiveFnsById[parsed.query.id]) {
+  const serverFnId = parsed?.query?.id
+  if (!serverFnId) {
+    this.error(`Missing server function ID in: ${id}`)
+  }
+  if (directiveFnsById[serverFnId]) {
     return `export {}`
   }
-  this.error(`Invalid server function ID: ${parsed.query.id}`)
+  this.error(`Invalid server function ID: ${serverFnId}`)
 }

Also add the escaping helper at the top of the file:

+function escapeRegExp(s: string) {
+  return s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')
+}

Finally: verify that directiveFnsById is populated before this load hook can run in dev mode. If a module's load hook executes before its corresponding resolveId hook completes, all validations will fail. If that's possible, add a guard to detect uninitialized state.

🧰 Tools
🪛 ast-grep (0.40.0)

[warning] 170-172: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(
^${resolveViteId(validateServerFnIdVirtualModule)}?.+,
)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html

(regexp-from-variable)

🤖 Prompt for AI Agents 
In packages/server-functions-plugin/src/index.ts around lines 166 to 185, the
load.filter.id RegExp is incorrect and unsafe: the `?` is applied to the last
character of the resolved ID instead of making the query part optional, and the
dynamic string must be escaped before insertion into a RegExp to avoid
special-character injection; replace the pattern with a RegExp that escapes the
resolved ID and matches an optional query suffix (e.g. make the query portion
optional as a whole, not the last character). Replace assert(parsed) and
assert(parsed.query.id) with explicit runtime checks that call this.error with a
clear message when parsed or parsed.query.id are missing. Finally add a guard
that ensures directiveFnsById is initialized/populated before validating IDs in
the load hook and, if not ready, either skip validation or return a safe
fallback to avoid false negatives during dev-mode load ordering.

{
// On the server, we need to be able to read the server-function manifest from the client build.
// This is likely used in the handler for server functions, so we can find the server function
Expand Down Expand Up @@ -191,6 +220,8 @@ export function TanStackServerFnPlugin(
if (this.environment.mode !== 'build') {
const mod = `
export async function getServerFnById(id) {
const validateIdImport = ${JSON.stringify(validateServerFnIdVirtualModule)} + '?id=' + id
await import(/* @vite-ignore */ '/@id/__x00__' + validateIdImport)
const decoded = Buffer.from(id, 'base64url').toString('utf8')
const devServerFn = JSON.parse(decoded)
const mod = await import(/* @vite-ignore */ devServerFn.file)
Expand Down
Loading