Skip to content

Commit bf518df

Browse files
committed
chore: apply AI code review suggestions
Apply suggestions from cubic, gemini, and copilot review threads on PR #364: - Use `Object.hasOwn(vercelJson, key)` instead of `key in vercelJson` so prototype-chain properties cannot leak into projectSettings even if upstream code mutates Object.prototype (cubic, copilot). - Type-guard whitelisted vercel.json values: only string and null pass through to projectSettings. Other types surface a local warning instead of producing a 400 from the Vercel API (gemini). Add tests for both behaviors: explicit null sentinel preservation and rejection of non-string/non-null values. Refs #359, PR #364
1 parent f7cc7b0 commit bf518df

4 files changed

Lines changed: 67 additions & 13 deletions

File tree

dist/index.js

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -102556,12 +102556,19 @@ function buildProjectConfig(config) {
102556102556
const projectSettings = {};
102557102557
if (vercelJson) {
102558102558
for (const key of PROJECT_SETTINGS_KEYS) {
102559-
if (key in vercelJson) {
102560-
// The Vercel REST API validates each value type itself. We only
102561-
// copy whitelisted keys, so prototype-pollution gadgets in
102562-
// vercel.json (`__proto__`, `constructor`, ...) cannot reach
102563-
// projectSettings.
102564-
projectSettings[key] = vercelJson[key];
102559+
if (Object.hasOwn(vercelJson, key)) {
102560+
// Whitelist + own-property check keeps prototype-pollution gadgets
102561+
// (`__proto__`, `constructor`, ...) out of projectSettings. The
102562+
// value-type guard rejects unexpected shapes (numbers, booleans,
102563+
// arrays) before the Vercel API validator sees them, surfacing a
102564+
// local warning instead of a 400 from the deployment endpoint.
102565+
const value = vercelJson[key];
102566+
if (typeof value === 'string' || value === null) {
102567+
projectSettings[key] = value;
102568+
}
102569+
else {
102570+
core.warning(`Ignoring vercel.json "${key}" — expected string or null, got ${typeof value === 'object' ? (Array.isArray(value) ? 'array' : 'object') : typeof value}.`);
102571+
}
102565102572
}
102566102573
}
102567102574
}

dist/index.js.map

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/__tests__/project-config.test.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,44 @@ describe('buildProjectConfig', () => {
269269
expect(result.projectSettings?.sourceFilesOutsideRootDirectory).toBeUndefined()
270270
})
271271

272+
it('preserves explicit null values for whitelisted keys (Vercel "use default" sentinel)', () => {
273+
// The Vercel REST API treats `null` and `undefined` differently: omission
274+
// means "keep current project setting", null means "explicit override to
275+
// no command". The action must preserve null verbatim.
276+
writeFileSync(
277+
path.join(tmpDir, 'vercel.json'),
278+
JSON.stringify({ buildCommand: null, framework: 'hugo' }),
279+
)
280+
281+
const result = buildProjectConfig(createConfig({ workingDirectory: tmpDir }))
282+
283+
expect(result.projectSettings?.buildCommand).toBeNull()
284+
expect(result.projectSettings?.framework).toBe('hugo')
285+
})
286+
287+
it('rejects non-string/non-null values for whitelisted keys without sending them to the API', () => {
288+
// Defensive guard: a user with a malformed vercel.json (e.g. `framework: 42`)
289+
// should get a local warning rather than a 400 from the deployment endpoint.
290+
writeFileSync(
291+
path.join(tmpDir, 'vercel.json'),
292+
JSON.stringify({
293+
buildCommand: 42,
294+
installCommand: true,
295+
outputDirectory: ['dist'],
296+
framework: { name: 'nextjs' },
297+
devCommand: 'dev',
298+
}),
299+
)
300+
301+
const result = buildProjectConfig(createConfig({ workingDirectory: tmpDir }))
302+
303+
expect(result.projectSettings).not.toHaveProperty('buildCommand')
304+
expect(result.projectSettings).not.toHaveProperty('installCommand')
305+
expect(result.projectSettings).not.toHaveProperty('outputDirectory')
306+
expect(result.projectSettings).not.toHaveProperty('framework')
307+
expect(result.projectSettings?.devCommand).toBe('dev')
308+
})
309+
272310
it('does not copy images, redirects, or other non-whitelisted vercel.json keys into projectSettings', () => {
273311
writeFileSync(
274312
path.join(tmpDir, 'vercel.json'),

src/project-config.ts

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -130,12 +130,21 @@ export function buildProjectConfig(config: ActionConfig): ProjectConfig {
130130

131131
if (vercelJson) {
132132
for (const key of PROJECT_SETTINGS_KEYS) {
133-
if (key in vercelJson) {
134-
// The Vercel REST API validates each value type itself. We only
135-
// copy whitelisted keys, so prototype-pollution gadgets in
136-
// vercel.json (`__proto__`, `constructor`, ...) cannot reach
137-
// projectSettings.
138-
projectSettings[key] = vercelJson[key] as string | null
133+
if (Object.hasOwn(vercelJson, key)) {
134+
// Whitelist + own-property check keeps prototype-pollution gadgets
135+
// (`__proto__`, `constructor`, ...) out of projectSettings. The
136+
// value-type guard rejects unexpected shapes (numbers, booleans,
137+
// arrays) before the Vercel API validator sees them, surfacing a
138+
// local warning instead of a 400 from the deployment endpoint.
139+
const value = vercelJson[key]
140+
if (typeof value === 'string' || value === null) {
141+
projectSettings[key] = value
142+
}
143+
else {
144+
core.warning(
145+
`Ignoring vercel.json "${key}" — expected string or null, got ${typeof value === 'object' ? (Array.isArray(value) ? 'array' : 'object') : typeof value}.`,
146+
)
147+
}
139148
}
140149
}
141150
}

0 commit comments

Comments
 (0)