Skip to content

Validate Vercel deploy config - #639

Merged
ralyodio merged 1 commit into
profullstack:masterfrom
rissrice2105-agent:codex/vercel-config-validation
Jun 6, 2026
Merged

Validate Vercel deploy config#639
ralyodio merged 1 commit into
profullstack:masterfrom
rissrice2105-agent:codex/vercel-config-validation

Conversation

@rissrice2105-agent

Copy link
Copy Markdown
Contributor

Fixes #638.

Changes:

  • validate optional project and org values as Vercel-safe slugs
  • reject blank dir 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-vercel/src/index.test.ts
  • tsc -p packages/targets/deploy-vercel/tsconfig.json --noEmit

@greptile-apps

greptile-apps Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds input validation to the Vercel deploy target, rejecting blank dir values and invalid project/org slugs before plan generation or CLI invocation, and adds regression tests covering all three cases.

  • normalizedConfig is introduced as a shared validation helper called at each public entry point (build, ship, renderPlan, deployDir, deployArgs), trimming whitespace and checking slugs against a regex before any I/O.
  • ship correctly places normalizedConfig as its first operation, but build calls mkdir(ctx.outDir) before renderPlan runs the validation, so an invalid config still creates the output directory as a side effect before throwing.
  • The slug regex (/^[A-Za-z0-9._-]+$/) is more permissive than Vercel's actual constraints (lowercase, alphanumeric + hyphens only), so values like My_App.v2 pass local validation but will be rejected by the Vercel CLI at runtime.

Confidence Score: 3/5

The validation logic is sound and the tests pass, but build() creates the output directory before the new guards run, leaving a silent side effect that contradicts the stated goal of the PR.

The build() method calls mkdir() before renderPlan() triggers normalizedConfig(), so an invalid dir/project/org config still creates the output directory before the error is raised. This is exactly the scenario the PR set out to prevent, and ship() was fixed correctly but build() was missed. The slug regex also admits characters Vercel rejects, giving users false confidence that a name is valid when the CLI will still fail.

packages/targets/deploy-vercel/src/index.ts — specifically the build() method and the optionalSlug regex.

Important Files Changed

Filename Overview
packages/targets/deploy-vercel/src/index.ts Adds config validation helpers and calls normalizedConfig at entry points, but build() validates after mkdir(), and normalizedConfig is invoked 3-4x per operation due to nested calls.
packages/targets/deploy-vercel/src/index.test.ts Adds regression tests for blank dir, invalid project slug, and invalid org slug; tests use partial substring matching which aligns with actual error messages.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[build / ship called] --> B{Entry point}
    B -->|ship| C[normalizedConfig first]
    B -->|build| D[ctx.log + mkdir runs first]
    D --> E[renderPlan normalizedConfig]
    C --> F{valid?}
    E --> F
    F -->|no| G[throw Error]
    F -->|yes| H[normalized Config]
    H --> I[deployArgs]
    H --> J[deployDir]
    I --> K[normalizedConfig again]
    J --> L[normalizedConfig again]
    I --> J
    K --> M[build CLI args]
    L --> N[resolve dir path]
    M --> O[exec / dry-run return]
    N --> O
Loading

Comments Outside Diff (2)

  1. packages/targets/deploy-vercel/src/index.ts, line 76-81 (link)

    P1 Validation runs after mkdir in build

    build calls mkdir(ctx.outDir, { recursive: true }) before renderPlan (which is where normalizedConfig is first invoked). An invalid config — e.g. dir: ' ' — still creates the output directory before throwing. This is inconsistent with ship, which calls normalizedConfig as its very first line, and with the PR goal of rejecting invalid config "before plan or CLI work." Moving config = normalizedConfig(config) to the top of build (before the log and mkdir) would make the guard consistent.

  2. packages/targets/deploy-vercel/src/index.ts, line 39-65 (link)

    P2 normalizedConfig is called 3–4 times per renderPlan invocation

    renderPlan calls normalizedConfig, then passes the result to deployDir (which calls normalizedConfig again) and deployArgs (which calls normalizedConfig, then calls deployDir which calls it a third time). The validation is idempotent so this produces no wrong output, but each normalized call re-validates and re-constructs the config object unnecessarily. Since ship already reassigns config before calling the helpers, deployDir and deployArgs could trust the incoming value and skip the internal normalizedConfig call — or those inner calls could simply be removed once the public entry points guarantee normalization.

    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!

Reviews (1): Last reviewed commit: "Validate Vercel deploy config" | Re-trigger Greptile

Comment on lines +22 to +27
function optionalSlug(value: string | undefined, field: string): string | undefined {
const slug = optionalText(value, field);
if (slug && !/^[A-Za-z0-9._-]+$/.test(slug)) {
throw new Error(`deploy-vercel ${field} must contain only letters, numbers, dots, underscores, or hyphens`);
}
return slug;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Slug regex is more permissive than Vercel actually allows

The pattern /^[A-Za-z0-9._-]+$/ permits uppercase letters, dots, and underscores. Vercel project and org slugs are lowercase alphanumeric with hyphens only (no dots, no underscores, no uppercase). Passing a slug like My_Project.v2 will not be caught here, but the Vercel CLI will reject it at runtime. This won't cause silent data loss but does mean the local validation gives a false "clean" result for names that will ultimately fail.

@github-actions

github-actions Bot commented Jun 6, 2026

Copy link
Copy Markdown

🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: git fetch upstream master && git rebase upstream/master.

11 similar comments
@github-actions

github-actions Bot commented Jun 6, 2026

Copy link
Copy Markdown

🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: git fetch upstream master && git rebase upstream/master.

@github-actions

github-actions Bot commented Jun 6, 2026

Copy link
Copy Markdown

🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: git fetch upstream master && git rebase upstream/master.

@github-actions

github-actions Bot commented Jun 6, 2026

Copy link
Copy Markdown

🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: git fetch upstream master && git rebase upstream/master.

@github-actions

github-actions Bot commented Jun 6, 2026

Copy link
Copy Markdown

🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: git fetch upstream master && git rebase upstream/master.

@github-actions

github-actions Bot commented Jun 6, 2026

Copy link
Copy Markdown

🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: git fetch upstream master && git rebase upstream/master.

@github-actions

github-actions Bot commented Jun 6, 2026

Copy link
Copy Markdown

🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: git fetch upstream master && git rebase upstream/master.

@github-actions

github-actions Bot commented Jun 6, 2026

Copy link
Copy Markdown

🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: git fetch upstream master && git rebase upstream/master.

@github-actions

github-actions Bot commented Jun 6, 2026

Copy link
Copy Markdown

🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: git fetch upstream master && git rebase upstream/master.

@github-actions

github-actions Bot commented Jun 6, 2026

Copy link
Copy Markdown

🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: git fetch upstream master && git rebase upstream/master.

@github-actions

github-actions Bot commented Jun 6, 2026

Copy link
Copy Markdown

🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: git fetch upstream master && git rebase upstream/master.

@github-actions

github-actions Bot commented Jun 6, 2026

Copy link
Copy Markdown

🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: git fetch upstream master && git rebase upstream/master.

@ralyodio
ralyodio merged commit c06a404 into profullstack:master Jun 6, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

deploy-vercel accepts invalid deploy config

2 participants