Validate Netlify deploy config by rissrice2105-agent · Pull Request #641 · profullstack/sh1pt

rissrice2105-agent · 2026-06-05T23:04:58Z

Fixes #640.

Changes:

validate siteId as a single URL path segment
reject blank dir and message values before plan or CLI work
normalize deploy config before rendering plans and shipping
keep dry-run output side-effect free while still validating input
add regression tests for invalid config

Validation:

vitest run packages/targets/deploy-netlify/src/index.test.ts
tsc -p packages/targets/deploy-netlify/tsconfig.json --noEmit

greptile-apps · 2026-06-05T23:07:42Z

Greptile Summary

This PR adds input validation to the Netlify deploy target, normalizing and rejecting blank dir/message values and validating siteId as a single URL path segment, plus regression tests covering invalid configs.

Adds normalizedConfig, requireText, optionalText, and optionalSiteId helpers that validate and trim config fields before use; ship correctly normalizes at entry before any side effects.
Adds three test cases covering blank dir, slash-containing siteId, and blank message — all of which now reject before CLI work in ship, though build still runs mkdir before validation fires (pre-existing gap flagged in an earlier review).

Confidence Score: 4/5

Safe to merge with awareness that build still creates the output directory before validation runs — an issue flagged in a prior review that remains unaddressed.

The ship path is correctly hardened: normalizedConfig fires at entry before any log, token fetch, or exec call. The build path still runs ctx.log and mkdir before renderPlan triggers validation, so a blank dir causes the output directory to be created on disk before the error is thrown — contradicting the PR's stated goal of rejecting invalid input before any side-effect work.

packages/targets/deploy-netlify/src/index.ts — specifically the build function, which needs normalizedConfig called before the mkdir and ctx.log calls.

Important Files Changed

Filename	Overview
packages/targets/deploy-netlify/src/index.ts	Adds validation helpers and normalizedConfig; ship correctly guards at entry, but build still runs ctx.log and mkdir before normalizedConfig fires (unaddressed from prior review)
packages/targets/deploy-netlify/src/index.test.ts	Adds three regression tests for invalid config; tests pass but the build assertion does not verify absence of side effects before the error

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[build / ship called] --> B{which entry?}
    B -->|ship| S1[normalizedConfig]
    S1 -->|invalid| SE[throw Error]
    S1 -->|valid| S2[log + dryRun check]
    S2 -->|dryRun| S3[return dry-run result]
    S2 -->|real deploy| S4[get NETLIFY_AUTH_TOKEN]
    S4 --> S5[exec npx netlify-cli]
    S5 --> S6[return]
    B -->|build| B1[join planPath]
    B1 --> B2[ctx.log side effect]
    B2 --> B3[mkdir side effect]
    B3 --> B4[renderPlan normalizedConfig]
    B4 -->|invalid| BE[throw after mkdir ran]
    B4 -->|valid| B5[writeFile plan]
    B5 --> B6[return artifact]
    style BE fill:#f88,stroke:#c00
    style SE fill:#8f8,stroke:#080

_{Reviews (2): Last reviewed commit: "Validate Netlify deploy config" | Re-trigger Greptile}

greptile-apps · 2026-06-05T23:07:46Z

+
+function optionalSiteId(value: string | undefined): string | undefined {
+  const id = optionalText(value, 'siteId');
+  if (id && /[\\/?#\x00-\x1F\x7F]/.test(id)) {


The siteId regex does not reject embedded spaces. A value like 'my site' trims cleanly, passes the regex, and is accepted as valid. It is a semantically invalid Netlify site ID that silently slips through the stated "single URL path segment" guard. Adding \s to the character class would close the gap.

Suggested change

if (id && /[\\/?#\x00-\x1F\x7F]/.test(id)) {

if (id && /[\s\\/?#\x00-\x1F\x7F]/.test(id)) {

greptile-apps · 2026-06-05T23:07:47Z

@@ -26,6 +55,7 @@ function deployArgs(ctx: { channel: string; projectDir: string; version: string
 }


Redundant normalizedConfig calls in inner helpers

deployDir and deployArgs both call normalizedConfig internally, and both are called from renderPlan and ship after those callers have already normalized the config. This means normalizedConfig — and its validation logic — runs 3–4 times for a single ship or renderPlan invocation. While idempotent today, any future validator with observable side-effects (e.g., logging, counters) would fire multiple times unexpectedly. Consider removing the normalizedConfig calls from deployDir and deployArgs and relying solely on callers to normalize before calling these helpers.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

github-actions · 2026-06-06T07:09:13Z