Validate Firebase deploy config by rissrice2105-agent · Pull Request #649 · profullstack/sh1pt

rissrice2105-agent · 2026-06-05T23:18:35Z

Fixes #648.

Changes:

validate projectId as a Firebase-safe identifier
reject blank config and message values
validate only entries and reject commas before joining targets
normalize config before plan rendering and Firebase CLI args
add regression tests for invalid config before CLI work

Validation:

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

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

Greptile Summary

This PR adds an input-validation layer to the Firebase deploy target, rejecting blank values, comma-containing only entries, and unsafe projectId strings before any plan file is written or CLI command is run.

A normalizedConfig helper centralises all validation and is applied at the start of build, ship, configPath, deployArgs, and renderPlan.
New regression tests cover bad/project projectId, commas in only[], blank only[] entries, and blank message.

Confidence Score: 4/5

Safe to merge; the validation logic is sound and tests cover the main failure modes.

The projectId guard allows uppercase letters, dots, and underscores that the Firebase CLI will reject, so a misconfigured project ID can still reach the CLI and produce a confusing error instead of the helpful message introduced here. Normalization also runs 3–4 times per build/ship call due to each helper re-normalizing, which is wasteful but doesn't break anything.

packages/targets/deploy-firebase/src/index.ts — the projectId regex and the repeated normalizedConfig calls in the helper chain.

Important Files Changed

Filename	Overview
packages/targets/deploy-firebase/src/index.ts	Adds input-validation layer (normalizedConfig + helpers) and applies it at every entry/helper point; projectId regex is more permissive than actual Firebase constraints, and normalizedConfig is invoked redundantly 3-4× per build/ship call.
packages/targets/deploy-firebase/src/index.test.ts	Adds regression tests for invalid config cases (bad projectId, comma in only[], blank only[] entry, blank message); missing a test for blank/empty projectId itself.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[build / ship called] --> B[normalizedConfig]
    B --> C{validation}
    C -- projectId invalid --> E[throw Error]
    C -- only entry blank/comma --> E
    C -- message/config blank --> E
    C -- valid --> D[normalized Config]
    D --> F[renderPlan / deployArgs]
    F --> G[normalizedConfig called again]
    G --> H[configPath]
    H --> I[normalizedConfig called again]
    I --> J[write plan / exec firebase CLI]

Comments Outside Diff (1)

packages/targets/deploy-firebase/src/index.ts, line 48-74 (link)

Redundant normalization on every call path

normalizedConfig is now called at both the public entry points (build, ship) and inside every helper (configPath, deployArgs, renderPlan). Because build calls renderPlan which calls both deployArgs and configPath, a single build invocation triggers validation 4× on the same config object. The helpers should trust that their callers already normalized the config, or the helpers should be restructured to accept a pre-normalized type so the duplication is caught by the type system.

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 Firebase deploy config" | Re-trigger Greptile}

greptile-apps · 2026-06-05T23:20:50Z

+  if (!/^[A-Za-z0-9._-]+$/.test(id)) {
+    throw new Error('deploy-firebase projectId must contain only letters, numbers, dots, underscores, or hyphens');
+  }


The projectId regex ^[A-Za-z0-9._-]+$ is more permissive than Firebase's actual constraints. Real Firebase project IDs allow only lowercase letters, digits, and hyphens (6–30 characters, must start with a letter). This means values like 'My_Project', 'proj.name', or 'AB' all pass this guard but are rejected by the Firebase CLI — potentially surfacing a confusing CLI error rather than the clear validation message added here.

Suggested change

if (!/^[A-Za-z0-9._-]+$/.test(id)) {

throw new Error('deploy-firebase projectId must contain only letters, numbers, dots, underscores, or hyphens');

}

if (!/^[a-z][a-z0-9-]{5,29}$/.test(id)) {

throw new Error('deploy-firebase projectId must be 6–30 lowercase letters, digits, or hyphens, and start with a letter');

}

github-actions · 2026-06-06T07:07:15Z