Skip to content

fix(kits): restore the extensions' param validation - #3198

Open
CorieW wants to merge 1 commit into
kitsfrom
invertase/kits-param-validation-parity
Open

CorieW wants to merge 1 commit into
kitsfrom
invertase/kits-param-validation-parity

Conversation

@CorieW

@CorieW CorieW commented Sep 15, 2026

Copy link
Copy Markdown
Member

Restores the param validation the kits dropped from their predecessor extensions. Reported in #3192, which is correct and wider than the example given: firebase deploy accepted an empty answer for 21 params their extension marks required: true, and skipped five regexes the extension applied.

Changes

  • Add input.text.nonEmpty to the 21 params their extension marks required: true (fix(kits): reject required params set to an empty value in .env #3193 has since landed two of them, BACKUP_INSTANCE_ID and SELECTED_DATABASE_INSTANCE; rebased onto it), across delete-user-data, firestore-bigquery-export, firestore-bundle-builder, firestore-genai-chatbot, firestore-incremental-capture, firestore-send-email, firestore-translate-text, firestore-vector-search, rtdb-limit-child-nodes and speech-to-text.
  • Restore five dropped validationRegex values with their validationErrorMessage: firestore-bundle-builder/BUNDLE_STORAGE_BUCKET, firestore-genai-chatbot/MODEL, and firestore-incremental-capture/SYNC_COLLECTION_PATH, SYNC_DATASET, BACKUP_INSTANCE_ID.
  • delete-user-data/AUTO_DISCOVERY_SEARCH_DEPTH uses validationRegex: /.+/ instead: nonEmpty is typed for string params only, and an empty answer resolved to 0 rather than the declared default of 3.
  • Add regression tests per kit asserting the declared nonEmpty and regexes, plus a new config.test.ts for firestore-bundle-builder and config-runtime.test.ts for firestore-bigquery-export, whose config.test.ts fakes the params module and cannot see declaration options.

Not changed

  • The (?:…|) widening on 14 optional params is correct parity and stays: askUserForParam only applies a regex to non-empty input when the param is optional (src/extensions/askUserForParam.ts:57 in firebase-tools).
  • speech-to-text/EXTENSION_BUCKET and storage-resize-images/IMG_BUCKET, the other two regexes 🐛 [All Kits] Kit Params are not specifying when they require a non-empty value. #3192 counts, use BUCKET_PICKER. A ResourceInput carries no text validation, and the picker only offers buckets that exist in the project, so it is at least as strict as the regex.

Caveat

firebase deploy validates only at the interactive prompt: resolveParams partitions out any param already present in .env and never validates it (src/deploy/functions/params.ts:167). The Extensions backend validated at instance create regardless of source, so a migrated .env still bypasses these declarations. Re-validate stored values when migrating an instance. #3193's module-scope assertRequiredParams guard catches the blank-in-.env case at discovery; this PR covers the prompt.

Verification

npm ci && npm run build && npm test in each of the 10 kits: all green.

@gemini-code-assist gemini-code-assist Bot left a comment

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.

Code Review

This pull request updates configuration parameters across multiple Firebase Extension kits to enforce required fields and input validation rules (such as nonEmpty: true and validationRegex) that match their extension.yaml definitions, preventing the CLI from accepting empty values. It also introduces corresponding unit tests to verify these constraints. The review feedback recommends tightening the validation regex for AUTO_DISCOVERY_SEARCH_DEPTH in the delete-user-data kit to restrict input to positive integers instead of any non-empty string, and updating the test assertions to match.

Comment thread kits/delete-user-data/src/config.ts
Comment thread kits/delete-user-data/tests/config.test.ts
The kits dropped `required: true` and several `validationRegex` values from
the predecessor extensions, so `firebase deploy` accepted an empty answer for
21 params the extension installer refuses, and skipped validation the
extension applied.

Add `nonEmpty` to the 21 params their extension marks required, and restore
the five dropped regexes with their error messages.

Fixes #3192
@CorieW
CorieW force-pushed the invertase/kits-param-validation-parity branch from 0afea6f to 2dcbb64 Compare September 15, 2026 15:28

@IzaakGough IzaakGough left a comment

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.

lgtm!

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.

3 participants