Skip to content

fix: add extra validation for vercel sync#6094

Open
maidul98 wants to merge 1 commit intomainfrom
vercel-sync-add-validation
Open

fix: add extra validation for vercel sync#6094
maidul98 wants to merge 1 commit intomainfrom
vercel-sync-add-validation

Conversation

@maidul98
Copy link
Copy Markdown
Collaborator

Prevent team scope from marking secret as sensitive when only dev env is selected

Prevent team scope from marking secret as sensitive when only dev env  is selected
@maidul98
Copy link
Copy Markdown
Collaborator Author

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

Comment on lines +54 to 63
ctx.addIssue({
code: z.ZodIssueCode.custom,
message:
"Marking secrets as sensitive in Vercel is not supported for development environments. Add another target environment or disable Sensitive.",
path: ["sensitive"]
});
}
});

const VercelSyncOptionsConfig: TSyncOptionsConfig = { canImportSecrets: true };
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 When targetEnvironments is empty and sensitive is true, the new Team-scope check config.targetEnvironments.every(...) fires vacuously ([].every(fn) === true in JavaScript), producing the misleading error "not supported for development environments" alongside the correct "At least one environment is required" error. Fix: add config.targetEnvironments.length > 0 && before the .every() call in both the backend schema (vercel-sync-schemas.ts line 54) and the frontend schema (vercel-sync-destination-schema.ts line 53).

Extended reasoning...

What the bug is and how it manifests

Both the backend (backend/src/services/secret-sync/vercel/vercel-sync-schemas.ts, lines 54–63) and frontend (frontend/src/components/secret-syncs/forms/schemas/vercel-sync-destination-schema.ts, lines 53–63) Vercel sync schemas have a new superRefine check that fires when scope === Team and every selected environment is Development. The intent is to prevent marking a Team-scoped sync as sensitive when it only targets dev environments. However, the guard omits a length check, making it susceptible to JavaScript's vacuous truth behavior.

The specific code path that triggers it

if (
  config.scope === VercelSyncScope.Team &&
  config.targetEnvironments.every((env) => env === VercelEnvironmentType.Development)
) { ctx.addIssue(...) }

When targetEnvironments is an empty array [], [].every(fn) unconditionally returns true in JavaScript (vacuous truth — there are no elements that violate the predicate). This means the entire condition evaluates to true whenever scope === Team, sensitive === true, and targetEnvironments is empty.

Why existing code doesn't prevent it

In Zod v3, superRefine runs even after inner field refinements (such as .min(1)) have already added validation errors. The .min(1) constraint on targetEnvironments is a refinement, not a type narrowing — it adds an issue but does not abort parsing. The empty array [] still satisfies the TypeScript type VercelEnvironmentType[], so the discriminated union branch matches and the value is passed to superRefine.

Impact

A user who submits the form (or makes a direct API call) with scope=Team, targetEnvironments=[], and sensitive=true will see two error messages simultaneously:

  1. ✅ "At least one environment is required" — correct and actionable
  2. ❌ "Marking secrets as sensitive in Vercel is not supported for development environments. Add another target environment or disable Sensitive." — misleading, since no environments were selected at all; the message implies development environments exist and should be replaced

The second message actively misdirects the user.

Step-by-step proof

  1. User sets scope = Team, checks sensitive = true, and leaves targetEnvironments as [] (empty).
  2. Zod evaluates the discriminated union — scope === Team matches the second branch.
  3. .min(1) refinement on targetEnvironments adds: "At least one environment is required".
  4. superRefine is called with { scope: 'team', targetEnvironments: [], sensitive: true, ... }.
  5. !config.sensitive is false, so the early return is skipped.
  6. config.scope === VercelSyncScope.Teamtrue.
  7. [].every(env => env === VercelEnvironmentType.Development)true (vacuous truth).
  8. ctx.addIssue() fires, adding the misleading message.
  9. Both errors are returned to the caller.

How to fix it

Add a length guard before the .every() call in both files:

if (
  config.scope === VercelSyncScope.Team &&
  config.targetEnvironments.length > 0 &&  // <-- add this
  config.targetEnvironments.every((env) => env === VercelEnvironmentType.Development)
) {
  ctx.addIssue({ ... });
}

This ensures the sensitive/dev-environment error only fires when at least one environment has actually been selected and all of them are Development.

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.

2 participants