Skip to content

fix: improve error handling in datasource URL parsing #18

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
jiashengguo merged 1 commit into from
Jan 10, 2026
Merged

fix: improve error handling in datasource URL parsing #18

Show file tree
Hide file tree
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
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@zenstackhq/proxy",
"version": "0.2.2",
"version": "0.2.3",
"description": "A CLI tool to run an Express server that proxies CRUD requests to a ZenStack backend",
"main": "index.js",
"publishConfig": {
Expand Down
57 changes: 30 additions & 27 deletions src/zmodel-parser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,15 +71,9 @@ function loadPrismaConfig(schemaDir: string): string | null {

// Use Function constructor to safely evaluate the object literal
// This is safer than eval as it doesn't have access to the local scope
try {
const configFn = new Function('env', `return ${configObjectStr}`)
const config = configFn(env)

return config?.datasource?.url || null
} catch (evalError) {
console.warn(`Warning: Could not evaluate config object: ${evalError}`)
return null
}
const configFn = new Function('env', `return ${configObjectStr}`)
const config = configFn(env)
return config?.datasource?.url
Copy link

Copilot AI Jan 10, 2026

Choose a reason for hiding this comment

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

The return type signature of loadPrismaConfig is string | null, but line 76 can potentially return undefined if config?.datasource?.url is undefined. While functionally similar, this creates a type inconsistency. Consider explicitly handling the undefined case by changing to return config?.datasource?.url || null or return config?.datasource?.url ?? null to ensure the return value strictly matches the declared type signature.

Suggested change
return config?.datasource?.url
return config?.datasource?.url ?? null

Copilot uses AI. Check for mistakes.
Comment on lines +74 to +76
Copy link

Copilot AI Jan 10, 2026

Choose a reason for hiding this comment

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

The removal of the inner try-catch block around the Function constructor evaluation in loadPrismaConfig means that any errors during evaluation (other than environment variable errors) will now propagate up and be caught by the outer catch block at line 77. However, this changes the behavior: previously, evaluation errors would result in returning null with a warning, now they will log a warning and return null (same outcome). But if the Function constructor itself throws an error during construction (syntax error in the config object string), this will now be handled differently. Ensure this behavioral change is intentional and doesn't break existing error handling expectations.

Suggested change
const configFn = new Function('env', `return ${configObjectStr}`)
const config = configFn(env)
return config?.datasource?.url
try {
const configFn = new Function('env', `return ${configObjectStr}`)
const config = configFn(env)
return config?.datasource?.url
} catch (error) {
if (error instanceof Error && error.message.includes('Environment variable')) {
// Propagate environment variable errors so they can be handled upstream
throw error
}
console.warn(`Warning: Failed to parse prisma.config.ts: ${error}`)
return null
}

Copilot uses AI. Check for mistakes.
} catch (error) {
if (error instanceof Error && error.message.includes('Environment variable')) {
throw error
Expand Down Expand Up @@ -117,34 +111,43 @@ function parseDatasource(
return { provider, url: datasourceUrlOverride }
}

// Extract url from schema
let urlMatch = datasourceBlock.match(/url\s*=\s*['"]([^'"]+)['"]/)
// Extract url value using single regex (could be string literal, env() call, or expression)
const urlMatch = datasourceBlock.match(/url\s*=\s*([^\n]+)/)
Copy link

Copilot AI Jan 10, 2026

Choose a reason for hiding this comment

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

The regex pattern ([^\n]+) will capture everything up to the newline, including trailing whitespace, semicolons, and potentially inline comments. For example, with url = "postgres://..." // comment, this will capture "postgres://..." // comment. While the .trim() on line 119 removes leading/trailing whitespace, it won't handle inline comments or trailing semicolons that might be part of urlValueStr before evaluation. This could cause the Function constructor evaluation to fail unexpectedly. Consider using a more specific pattern like ([^\n;/]+) to stop at semicolons or comment markers, or add additional sanitization of the captured value.

Copilot uses AI. Check for mistakes.
let url: string | null = null

if (urlMatch) {
url = urlMatch[1]
} else {
// Try to match env("xxx")
const envMatch = datasourceBlock.match(/url\s*=\s*env\(\s*['"]([^'"]+)['"]\s*\)/)
if (envMatch) {
const envVar = envMatch[1]
const envValue = process.env[envVar]
if (envValue) {
url = envValue
const urlValueStr = urlMatch[1].trim()

// Create env helper function
const env = (varName: string) => {
const value = process.env[varName]
if (!value) {
throw new CliError(`Environment variable ${varName} is not set`)
}
return value
}
}

// If no URL found in schema, try prisma.config.ts (Prisma 7)
if (!url) {
try {
// Use Function constructor to evaluate the url value
const urlFn = new Function('env', `return ${urlValueStr}`)
url = urlFn(env)
Comment on lines +130 to +133
Copy link

Copilot AI Jan 10, 2026

Choose a reason for hiding this comment

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

Using the Function constructor with user-controlled input (schema file content) poses a security risk. While the code attempts to sandbox by only passing the env function, the evaluated code can still execute arbitrary JavaScript. An attacker with control over the schema file could potentially inject malicious code. Consider adding input validation to ensure urlValueStr only contains expected patterns (quotes, env() calls, etc.) before evaluation, or use a safer alternative like a purpose-built parser for Prisma schema syntax.

Suggested change
try {
// Use Function constructor to evaluate the url value
const urlFn = new Function('env', `return ${urlValueStr}`)
url = urlFn(env)
// Safely evaluate limited URL expressions without using the Function constructor.
// Supported forms:
// - String literal: "postgres://..." or 'postgres://...'
// - env("VAR") or env('VAR')
// - Concatenations using + of the above, e.g. env("URL") + "/shadow"
const evaluateUrlExpression = (expr: string): string => {
const parts = expr.split('+').map((p) => p.trim()).filter((p) => p.length > 0)
if (parts.length === 0) {
throw new CliError(
'Could not evaluate datasource url from schema, you could provide it via -d option.'
)
}
let result = ''
for (const part of parts) {
// Match simple string literal
const stringMatch = part.match(/^(['"])(.*)\1$/)
if (stringMatch) {
result += stringMatch[2]
continue
}
// Match env("VAR") or env('VAR')
const envMatch = part.match(/^env\(\s*(['"])([A-Za-z_][A-Za-z0-9_]*)\1\s*\)$/)
if (envMatch) {
result += env(envMatch[2])
continue
}
// Any other pattern is considered unsupported for security reasons
throw new CliError(
'Unsupported datasource URL expression in schema; only string literals, env() calls, and "+" concatenations are allowed.'
)
}
return result
}
try {
url = evaluateUrlExpression(urlValueStr)

Copilot uses AI. Check for mistakes.
} catch (evalError) {
throw new CliError(
'Could not evaluate datasource url from schema, you could provide it via -d option.'
)
}
} else {
url = loadPrismaConfig(schemaDir)
// If still no URL found, throw error
if (url == null) {
throw new CliError(
'No datasource URL found. For Prisma 7, ensure prisma.config.ts exists with datasource configuration, or provide the URL via -d option.'
Comment on lines +136 to +144
Copy link

Copilot AI Jan 10, 2026

Choose a reason for hiding this comment

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

Inconsistent error message format and option naming. Line 136 uses "-d option" while line 144 uses "-d option" but previously the code mentioned "--datasource-url option" (visible in the diff context). The error messages should consistently reference the same option format. Additionally, line 136 says "Could not evaluate datasource url" while line 144 says "No datasource URL found" - these should follow a consistent capitalization style for "datasource URL" vs "datasource url".

Copilot uses AI. Check for mistakes.
)
}
}

Comment on lines +141 to 148
Copy link

Copilot AI Jan 10, 2026

Choose a reason for hiding this comment

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

There is a logic issue with the dual error checking. When urlMatch succeeds (line 118), url is assigned from the evaluation (line 133), then the check at line 149 validates if it's truthy. However, when urlMatch fails (line 139), the code loads from loadPrismaConfig, checks if it's null at line 142, but then checks again at line 149. This means when coming from the else branch, if url is null, it throws at line 142, making the check at line 149 unreachable for that path. If coming from the if branch, the evaluation could theoretically return an empty string or other falsy value that's not caught by the try-catch. The logic should be restructured to have a single validation point after both paths complete, or the check at line 149 should be removed if line 142 is sufficient for all cases.

Suggested change
// If still no URL found, throw error
if (url == null) {
throw new CliError(
'No datasource URL found. For Prisma 7, ensure prisma.config.ts exists with datasource configuration, or provide the URL via -d option.'
)
}
}
}
if (url == null) {
throw new CliError(
'No datasource URL found. For Prisma 7, ensure prisma.config.ts exists with datasource configuration, or provide the URL via -d option.'
)
}

Copilot uses AI. Check for mistakes.
// If still no URL found, throw error
if (!url) {
throw new CliError(
'No datasource URL found. For Prisma 7, ensure prisma.config.ts exists with datasource configuration, or provide the URL via --datasource-url option.'
)
throw new CliError('datasource url has no value, you could provide it via -d option.')
Copy link

Copilot AI Jan 10, 2026

Choose a reason for hiding this comment

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

The error message uses inconsistent capitalization and grammar. It should be "Datasource URL has no value, you could provide it via -d option." with "Datasource" capitalized to match the style of other error messages, or follow a consistent lowercase style throughout. Additionally, consider adding a period after "option" for consistency with other error messages in the codebase.

Suggested change
throw new CliError('datasource url has no value, you could provide it via -d option.')
throw new CliError('Datasource URL has no value, you could provide it via -d option.')

Copilot uses AI. Check for mistakes.
}

return { provider, url }
Expand Down
Loading