diff --git a/.changeset/d1-binding-config-validation.md b/.changeset/d1-binding-config-validation.md new file mode 100644 index 0000000000..d78efcfed4 --- /dev/null +++ b/.changeset/d1-binding-config-validation.md @@ -0,0 +1,7 @@ +--- +"@cloudflare/workers-utils": patch +--- + +Validate optional configuration fields for D1 database bindings + +Enforce type checks for the optional D1 database properties `database_name`, `migrations_dir`, `migrations_table`, and `database_internal_env` to ensure consistency with other binding types. diff --git a/.changeset/types-ignore-parent-directory-dts.md b/.changeset/types-ignore-parent-directory-dts.md new file mode 100644 index 0000000000..efc7126013 --- /dev/null +++ b/.changeset/types-ignore-parent-directory-dts.md @@ -0,0 +1,5 @@ +--- +"wrangler": patch +--- + +Fix false-positive validation error in `wrangler types` when a custom `worker-configuration.d.ts` file exists in a parent directory. diff --git a/packages/workers-utils/src/config/validation.ts b/packages/workers-utils/src/config/validation.ts index 289f581455..f5239e1cc9 100644 --- a/packages/workers-utils/src/config/validation.ts +++ b/packages/workers-utils/src/config/validation.ts @@ -4153,6 +4153,42 @@ const validateD1Binding: ValidatorFn = (diagnostics, field, value) => { isValid = false; } + if (!isOptionalProperty(value, "database_name", "string")) { + diagnostics.errors.push( + `"${field}" bindings should, optionally, have a string "database_name" field but got ${JSON.stringify( + value + )}.` + ); + isValid = false; + } + + if (!isOptionalProperty(value, "migrations_dir", "string")) { + diagnostics.errors.push( + `"${field}" bindings should, optionally, have a string "migrations_dir" field but got ${JSON.stringify( + value + )}.` + ); + isValid = false; + } + + if (!isOptionalProperty(value, "migrations_table", "string")) { + diagnostics.errors.push( + `"${field}" bindings should, optionally, have a string "migrations_table" field but got ${JSON.stringify( + value + )}.` + ); + isValid = false; + } + + if (!isOptionalProperty(value, "database_internal_env", "string")) { + diagnostics.errors.push( + `"${field}" bindings should, optionally, have a string "database_internal_env" field but got ${JSON.stringify( + value + )}.` + ); + isValid = false; + } + validateAdditionalProperties(diagnostics, field, Object.keys(value), [ "binding", "database_id", diff --git a/packages/workers-utils/tests/config/validation/normalize-and-validate-config.test.ts b/packages/workers-utils/tests/config/validation/normalize-and-validate-config.test.ts index f220e28eb4..e39799f111 100644 --- a/packages/workers-utils/tests/config/validation/normalize-and-validate-config.test.ts +++ b/packages/workers-utils/tests/config/validation/normalize-and-validate-config.test.ts @@ -3700,6 +3700,106 @@ describe("normalizeAndValidateConfig()", () => { expect(diagnostics.hasWarnings()).toBe(false); expect(diagnostics.hasErrors()).toBe(false); }); + + it("should error if D1 database database_name has incorrect type", ({ + expect, + }) => { + const { diagnostics } = normalizeAndValidateConfig( + { + d1_databases: [ + { + binding: "DB", + database_name: true, + }, + ], + } as unknown as RawConfig, + undefined, + undefined, + { env: undefined } + ); + + expect(diagnostics.hasWarnings()).toBe(false); + expect(diagnostics.hasErrors()).toBe(true); + expect(diagnostics.renderErrors()).toMatchInlineSnapshot(` + "Processing wrangler configuration: + - "d1_databases[0]" bindings should, optionally, have a string "database_name" field but got {"binding":"DB","database_name":true}." + `); + }); + + it("should error if D1 database migrations_dir has incorrect type", ({ + expect, + }) => { + const { diagnostics } = normalizeAndValidateConfig( + { + d1_databases: [ + { + binding: "DB", + migrations_dir: 123, + }, + ], + } as unknown as RawConfig, + undefined, + undefined, + { env: undefined } + ); + + expect(diagnostics.hasWarnings()).toBe(false); + expect(diagnostics.hasErrors()).toBe(true); + expect(diagnostics.renderErrors()).toMatchInlineSnapshot(` + "Processing wrangler configuration: + - "d1_databases[0]" bindings should, optionally, have a string "migrations_dir" field but got {"binding":"DB","migrations_dir":123}." + `); + }); + + it("should error if D1 database migrations_table has incorrect type", ({ + expect, + }) => { + const { diagnostics } = normalizeAndValidateConfig( + { + d1_databases: [ + { + binding: "DB", + migrations_table: {}, + }, + ], + } as unknown as RawConfig, + undefined, + undefined, + { env: undefined } + ); + + expect(diagnostics.hasWarnings()).toBe(false); + expect(diagnostics.hasErrors()).toBe(true); + expect(diagnostics.renderErrors()).toMatchInlineSnapshot(` + "Processing wrangler configuration: + - "d1_databases[0]" bindings should, optionally, have a string "migrations_table" field but got {"binding":"DB","migrations_table":{}}." + `); + }); + + it("should error if D1 database database_internal_env has incorrect type", ({ + expect, + }) => { + const { diagnostics } = normalizeAndValidateConfig( + { + d1_databases: [ + { + binding: "DB", + database_internal_env: false, + }, + ], + } as unknown as RawConfig, + undefined, + undefined, + { env: undefined } + ); + + expect(diagnostics.hasWarnings()).toBe(false); + expect(diagnostics.hasErrors()).toBe(true); + expect(diagnostics.renderErrors()).toMatchInlineSnapshot(` + "Processing wrangler configuration: + - "d1_databases[0]" bindings should, optionally, have a string "database_internal_env" field but got {"binding":"DB","database_internal_env":false}." + `); + }); }); describe("[hyperdrive]", () => { diff --git a/packages/wrangler/src/__tests__/type-generation.test.ts b/packages/wrangler/src/__tests__/type-generation.test.ts index d8d0a9bff8..3219e7dd99 100644 --- a/packages/wrangler/src/__tests__/type-generation.test.ts +++ b/packages/wrangler/src/__tests__/type-generation.test.ts @@ -1,4 +1,5 @@ import * as fs from "node:fs"; +import path from "node:path"; import { runInTempDir } from "@cloudflare/workers-utils/test-helpers"; import { http, HttpResponse } from "msw"; import { afterAll, beforeAll, beforeEach, describe, it, vi } from "vitest"; @@ -3429,6 +3430,42 @@ describe("generate types - CLI", () => { "At least one of --include-env or --include-runtime must be enabled." ); }); + + it("should ignore a non-Wrangler types file in a parent directory", async ({ + expect, + }) => { + const parentDir = path.resolve(".."); + const parentTypesPath = path.join( + parentDir, + "worker-configuration.d.ts" + ); + + fs.writeFileSync(parentTypesPath, "export interface CustomEnv {}"); + + try { + fs.writeFileSync( + "./index.ts", + "export default { async fetch() {} };" + ); + fs.writeFileSync( + "./wrangler.jsonc", + JSON.stringify({ + compatibility_date: "2022-01-12", + name: "test-name", + main: "./index.ts", + }), + "utf-8" + ); + + await runWrangler("types"); + + expect(fs.existsSync("./worker-configuration.d.ts")).toBe(true); + } finally { + try { + fs.unlinkSync(parentTypesPath); + } catch {} + } + }); }); it("should allow multiple customizations to be applied together", async ({ diff --git a/packages/wrangler/src/type-generation/index.ts b/packages/wrangler/src/type-generation/index.ts index c1b18f343e..f39dcf761e 100644 --- a/packages/wrangler/src/type-generation/index.ts +++ b/packages/wrangler/src/type-generation/index.ts @@ -1781,13 +1781,16 @@ function generatePerEnvTypeStrings( * @throws {UserError} If a non-Wrangler .d.ts file already exists at the given path. */ const validateTypesFile = (path: string): void => { - const wranglerOverrideDTSPath = find.file(path); - if (wranglerOverrideDTSPath === undefined) { + if (!fs.existsSync(path)) { + return; + } + const stats = fs.statSync(path); + if (!stats.isFile()) { return; } try { - const fileContent = fs.readFileSync(wranglerOverrideDTSPath, "utf8"); + const fileContent = fs.readFileSync(path, "utf8"); if ( !fileContent.includes("Generated by Wrangler") && !fileContent.includes("Runtime types generated with workerd")