Skip to content

Commit 8ef1bfd

Browse files
authored
fix: escape spec-controlled strings in generated template literals and object keys (#3692)
* fix(core,zod): escape spec-controlled strings in generated template literals Unescaped spec values (servers[].url, path segments, schema defaults) were baked into generated template literals, allowing code injection via backtick or ${ in an attacker-controlled OpenAPI spec. Uses jsesc with quotes: 'backtick' to escape at three boundaries: - getRoute(): raw OpenAPI path before param processing - getFullRoute(): resolved server URL after variable substitution - formatDefaultValue(): schema default values in zod generation Addresses: GHSA-88f2-fpv8-89q2, GHSA-w727-8j6c-2rj4, GHSA-3575-w9fc-c2j6, GHSA-2h9g-j24r-h63g, GHSA-8j6p-r8jg-mxqh, GHSA-p4cg-3328-rvfg * fix(zod): escape object keys via JSON.stringify in zod.object generation Unescaped schema property and parameter names were emitted as double-quoted keys in zod.object({...}), allowing computed property key injection via " in the name. Replaces "${key}" with JSON.stringify(key) at all 5 render sites. Addresses: GHSA-6437-gxhq-pqv8, GHSA-653q-5476-x79g, GHSA-6mr6-jvcr-2f25 * fix(core): escape single-quoted object keys in getKey via jsStringLiteralEscape Unescaped schema property names were wrapped in single quotes by getKey(), allowing computed property key injection via ' in the name in MSW mock output. Wraps the key body with jsStringLiteralEscape before quoting. Addresses: GHSA-2w86-xfrc-g85r * fix(query,zod): escape raw route prefix and prove backtick-in-key safety mutation-generator.ts passed a raw spec path prefix to getFullRoute, bypassing getRoute's escaping. Wrap with getRoute to close the gap. Add test proving JSON.stringify-wrapped object keys are safe against backtick injection (backtick is harmless inside double-quoted strings). * fix: escape object-default keys and add ${} path test - Escape object-default keys with JSON.stringify in zod schema defaults (same fix as zod.object keys) - Add test verifying ${globalThis.X} in path segments is not re-interpreted as interpolation - Update existing test expectations for quoted object keys * fix(core): prevent ${} re-interpretation in getRoutePath and escape single quotes in getRouteAsArray getRoutePath: after jsesc escapes ${ to \${, the remaining {evil} was mistaken for an OpenAPI path param and re-converted to ${evil}. Add early-return when { is preceded by $. getRouteAsArray: segments wrapped in single-quoted strings without escaping '. A spec path containing ' would break out. Now escapes single quotes at both wrap sites. * fix(core): handle ${} in getRoutePath and escaped tags in getRouteAsArray getRoutePath: skip past ${...} block and continue processing remaining suffix so later {param} segments are still converted. getRouteAsArray: add (?<!\) to split/match regexes so jsesc-escaped ${...} is treated as literal text, preventing standalone backslash from breaking single-quote wrapping. * fix(core): escape per-segment in getRoute to preserve params after ${...}
1 parent c082bb4 commit 8ef1bfd

10 files changed

Lines changed: 347 additions & 34 deletions

File tree

bun.lock

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

packages/core/package.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
"esutils": "2.0.3",
3232
"fs-extra": "^11.3.2",
3333
"jiti": "^2.6.1",
34+
"jsesc": "^3.0.0",
3435
"remeda": "^2.33.6",
3536
"tinyglobby": "^0.2.16",
3637
"typedoc": "^0.28.19"
@@ -40,6 +41,7 @@
4041
"@types/debug": "^4.1.12",
4142
"@types/esutils": "^2.0.2",
4243
"@types/fs-extra": "^11.0.4",
44+
"@types/jsesc": "^3.0.3",
4345
"rimraf": "catalog:",
4446
"typescript": "catalog:",
4547
"vitest": "catalog:"
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import { describe, expect, it } from 'vitest';
2+
3+
import { getKey } from './keys';
4+
5+
describe('getKey', () => {
6+
it('escapes single quote in key', () => {
7+
const result = getKey("x':[require('fs').execSync('id'),");
8+
expect(result).toMatch(/^'(.*)'$/);
9+
const inner = result.slice(1, -1);
10+
expect(inner).not.toMatch(/(?<!\\)'/);
11+
});
12+
13+
it('escapes backslash in key', () => {
14+
const result = getKey('a\\b');
15+
expect(result).toBe(String.raw`'a\\b'`);
16+
});
17+
});

packages/core/src/getters/keys.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
import { keyword } from 'esutils';
22

3+
import { jsStringLiteralEscape } from '../utils';
4+
35
export function getKey(key: string) {
4-
return keyword.isIdentifierNameES5(key) ? key : `'${key}'`;
6+
return keyword.isIdentifierNameES5(key)
7+
? key
8+
: `'${jsStringLiteralEscape(key)}'`;
59
}

packages/core/src/getters/route.test.ts

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,118 @@ describe('getFullRoute getter', () => {
202202
}
203203
});
204204

205+
describe('getFullRoute — GHSA-88f2-fpv8-89q2: servers[].url template-literal injection', () => {
206+
it('escapes backtick in server URL', () => {
207+
const servers: OpenApiServerObject[] = [
208+
{
209+
url: 'http://api.x/`+require("fs").writeFileSync("/marker","pwned")+`',
210+
},
211+
];
212+
const result = getFullRoute('/path', servers, {
213+
getBaseUrlFromSpecification: true,
214+
});
215+
expect(result).not.toMatch(/(?<!\\)`/);
216+
expect(result).toContain('\\`+require');
217+
});
218+
219+
it('escapes ${ in server URL', () => {
220+
const servers: OpenApiServerObject[] = [
221+
{
222+
url: 'http://api.x/${globalThis.X = require("fs").writeFileSync("/marker","pwned")}/v1',
223+
},
224+
];
225+
const result = getFullRoute('/path', servers, {
226+
getBaseUrlFromSpecification: true,
227+
});
228+
expect(result).not.toMatch(/(?<!\\)\$\{/);
229+
expect(result).toContain('\\${globalThis');
230+
});
231+
232+
it('escapes backslash in server URL', () => {
233+
const servers: OpenApiServerObject[] = [
234+
{ url: String.raw`http://api.x/\`+code+\`` },
235+
];
236+
const result = getFullRoute('/path', servers, {
237+
getBaseUrlFromSpecification: true,
238+
});
239+
expect(result).toContain(String.raw`http://api.x/\\\`+code+\\\``);
240+
});
241+
242+
it('escapes backtick and ${ in variable default value (spec-sourced)', () => {
243+
const servers: OpenApiServerObject[] = [
244+
{
245+
url: 'http://{env}.example.com',
246+
variables: {
247+
env: {
248+
default:
249+
'`+require("child_process").execSync("id")+`${globalThis.X}',
250+
},
251+
},
252+
},
253+
];
254+
const result = getFullRoute('/path', servers, {
255+
getBaseUrlFromSpecification: true,
256+
});
257+
expect(result).not.toMatch(/(?<!\\)`/);
258+
expect(result).not.toMatch(/(?<!\\)\$\{/);
259+
});
260+
});
261+
262+
describe('getRoute — spec path injection', () => {
263+
it('escapes backtick in static path segment', () => {
264+
const result = getRoute('/v1/`+require("child_process").execSync("id")+`');
265+
expect(result).not.toMatch(/(?<!\\)`/);
266+
});
267+
268+
it('escapes ${...} as literal text, not interpolation', () => {
269+
for (const payload of [
270+
'/v1/${evil}/path',
271+
'/v1/${globalThis.X}/path',
272+
'/v1/some${petId}/path',
273+
]) {
274+
const result = getRoute(payload);
275+
expect(result).not.toMatch(/(?<!\\)\$\{/);
276+
}
277+
});
278+
279+
it('preserves legitimate params after ${...} in same segment', () => {
280+
for (const payload of [
281+
'/v1/${evil}{petId}/path',
282+
'/v1/${a}${b}{petId}/path',
283+
]) {
284+
const result = getRoute(payload);
285+
expect(result).toContain('${petId}');
286+
expect(result).not.toMatch(/(?<!\\)\$\{(evil|[ab]\})/);
287+
}
288+
});
289+
290+
it('does not trigger guard for $ without {', () => {
291+
const result = getRoute('/v1/price$/list');
292+
expect(result).toContain('price$');
293+
});
294+
295+
it('escapes backslash in static text', () => {
296+
const result = getRoute('/v1/path\\to/list');
297+
expect(result).toContain('path\\\\to');
298+
});
299+
300+
it('still converts legitimate path params', () => {
301+
expect(getRoute('/v1/{petId}')).toContain('${petId}');
302+
});
303+
});
304+
305+
describe('getRouteAsArray — single-quote injection', () => {
306+
it('escapes single quote in static segment', () => {
307+
const result = getRouteAsArray("v1/it's/path");
308+
expect(result).toContain("it\\'s");
309+
});
310+
311+
it('escapes single quote in non-interpolation part of mixed segment', () => {
312+
const result = getRouteAsArray("pre's${petId}");
313+
expect(result).toContain("pre\\'s");
314+
});
315+
});
316+
205317
describe('getBaseUrlRuntimeImports', () => {
206318
it('returns [] when baseUrl is omitted', () => {
207319
expect(getBaseUrlRuntimeImports()).toEqual([]);

packages/core/src/getters/route.ts

Lines changed: 45 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import jsesc from 'jsesc';
2+
13
import { TEMPLATE_TAG_REGEX } from '../constants';
24
import type {
35
BaseUrlFromConstant,
@@ -31,9 +33,25 @@ function runtimeExpressionToUrlPrefix(expression: string): string {
3133

3234
const hasParam = (path: string): boolean => /[^{]*{[\w*_-]*}.*/.test(path);
3335

36+
const esc = (str: string) => jsesc(str, { quotes: 'backtick', wrap: false });
37+
3438
const getRoutePath = (path: string): string => {
39+
// Don't treat ${...} as an OpenAPI path param — the $ makes it literal text,
40+
// not a {param} template. Escape the ${...} block and continue processing
41+
// any legitimate {param} segments after it.
42+
const braceIdx = path.indexOf('{');
43+
if (braceIdx > 0 && path[braceIdx - 1] === '$') {
44+
const closeIdx = path.indexOf('}', braceIdx);
45+
if (closeIdx === -1) return esc(path);
46+
const before = esc(path.slice(0, closeIdx + 1));
47+
const rest = path.slice(closeIdx + 1);
48+
return hasParam(rest)
49+
? `${before}${getRoutePath(rest)}`
50+
: `${before}${esc(rest)}`;
51+
}
52+
3553
const matches = /([^{]*){?([\w*_-]*)}?(.*)/.exec(path);
36-
if (!matches?.length) return path; // impossible due to regexp grouping here, but for TS
54+
if (!matches?.length) return esc(path);
3755

3856
const prev = matches[1];
3957
const rawParam = matches[2];
@@ -44,13 +62,18 @@ const getRoutePath = (path: string): string => {
4462
dash: true,
4563
dot: true,
4664
});
47-
const next = hasParam(rest) ? getRoutePath(rest) : rest;
65+
const next = hasParam(rest) ? getRoutePath(rest) : esc(rest);
4866

4967
return hasParam(path)
50-
? `${prev}\${${param}}${next}`
51-
: `${prev}${param}${next}`;
68+
? `${esc(prev)}\${${param}}${next}`
69+
: `${esc(prev)}${param}${next}`;
5270
};
5371

72+
/**
73+
* Converts an OpenAPI path (`{param}`) to a template-literal route (`${param}`),
74+
* escaping static segments with jsesc for safe embedding in backtick strings.
75+
* The `route` arg must be a raw OpenAPI path.
76+
*/
5477
export function getRoute(route: string) {
5578
const splittedRoute = route.split('/');
5679

@@ -60,11 +83,19 @@ export function getRoute(route: string) {
6083
continue;
6184
}
6285

63-
result += path.includes('{') ? `/${getRoutePath(path)}` : `/${path}`;
86+
result += path.includes('{') ? `/${getRoutePath(path)}` : `/${esc(path)}`;
6487
}
6588
return result;
6689
}
6790

91+
/**
92+
* Prepends a base URL to an already-processed route.
93+
*
94+
* `route` must be the output of {@link getRoute} (already escaped for template
95+
* literals). This function does NOT re-escape it — jsesc is not idempotent, so
96+
* escaping twice would double the backslashes. Only the server URL from
97+
* `getBaseUrlFromSpecification` is escaped here, after variable substitution.
98+
*/
6899
export function getFullRoute(
69100
route: string,
70101
servers: OpenApiServerObject[] | undefined,
@@ -92,7 +123,8 @@ export function getFullRoute(
92123
);
93124
if (!server) return '';
94125
const serverUrl = server.url ?? '';
95-
if (!server.variables) return serverUrl;
126+
if (!server.variables)
127+
return jsesc(serverUrl, { quotes: 'backtick', wrap: false });
96128

97129
let url = serverUrl;
98130
const variables = baseUrl.variables;
@@ -112,7 +144,7 @@ export function getFullRoute(
112144
url = url.replaceAll(`{${variableKey}}`, String(variable.default));
113145
}
114146
}
115-
return url;
147+
return jsesc(url, { quotes: 'backtick', wrap: false });
116148
}
117149
return baseUrl.baseUrl;
118150
};
@@ -164,15 +196,16 @@ export function getRouteAsArray(route: string): string {
164196
.filter((i) => i !== '')
165197
.flatMap((segment) => {
166198
if (!segment.includes('${')) {
167-
return [`'${segment}'`];
199+
return [`'${segment.replaceAll("'", "\\'")}'`];
168200
}
169-
// Split by template tags, keeping the delimiters
201+
// Split by template tags, keeping the delimiters.
202+
// (?<!\\) prevents matching \${...} (jsesc-escaped) as a template tag.
170203
return segment
171-
.split(/(\$\{.+?\})/g)
204+
.split(/(?<!\\)(\$\{.+?\})/g)
172205
.filter(Boolean)
173206
.map((part) => {
174-
const match = /^\$\{(.+?)\}$/.exec(part);
175-
return match ? match[1] : `'${part}'`;
207+
const match = /^(?<!\\)\$\{(.+?)\}$/.exec(part);
208+
return match ? match[1] : `'${part.replaceAll("'", "\\'")}'`;
176209
});
177210
})
178211
.join(',');

packages/query/src/mutation-generator.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
type GeneratorVerbOptions,
88
GetterPropType,
99
getFullRoute,
10+
getRoute,
1011
getRouteAsArray,
1112
type InvalidateTarget,
1213
type InvalidateTargetParam,
@@ -350,7 +351,7 @@ const createGenerateInvalidateCall = (
350351
// prefix must carry the same `baseUrl` – otherwise the predicate /
351352
// partial key never matches a baseUrl-prefixed cache key. `prefix`
352353
// has no path params, so `getFullRoute` just concatenates the base.
353-
const prefixWithBase = getFullRoute(prefix, servers, baseUrl);
354+
const prefixWithBase = getFullRoute(getRoute(prefix), servers, baseUrl);
354355
// Mirror the verb prefix that `getQueryKeyVerbPrefix` injects into
355356
// non-GET Query keys; without this, the predicate / partial key
356357
// would never match a verb-prefixed cache key and the broad

packages/zod/package.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,11 @@
2424
},
2525
"dependencies": {
2626
"@orval/core": "workspace:*",
27+
"jsesc": "^3.0.0",
2728
"remeda": "^2.33.6"
2829
},
2930
"devDependencies": {
31+
"@types/jsesc": "^3.0.3",
3032
"rimraf": "catalog:",
3133
"typescript": "catalog:",
3234
"vitest": "catalog:"

0 commit comments

Comments
 (0)