From 09ab038073ffd20e9f3f25a51bff361a3ea3aaf5 Mon Sep 17 00:00:00 2001 From: Colum Ferry Date: Tue, 28 Jul 2026 15:19:34 +0100 Subject: [PATCH] fix(cli): comma-split network-bans and network-restrictions slice flags like pflag (CLI-1983) --- .../network-bans/remove/remove.command.ts | 18 ++++- .../remove/remove.command.unit.test.ts | 61 +++++++++++++++ .../remove/remove.integration.test.ts | 75 +++++++++++++++++++ .../update/update.command.ts | 18 ++++- .../update/update.command.unit.test.ts | 62 +++++++++++++++ .../update/update.integration.test.ts | 75 +++++++++++++++++++ 6 files changed, 301 insertions(+), 8 deletions(-) create mode 100644 apps/cli/src/legacy/commands/network-bans/remove/remove.command.unit.test.ts create mode 100644 apps/cli/src/legacy/commands/network-restrictions/update/update.command.unit.test.ts diff --git a/apps/cli/src/legacy/commands/network-bans/remove/remove.command.ts b/apps/cli/src/legacy/commands/network-bans/remove/remove.command.ts index 02beb81d15..fdf3da1b2b 100644 --- a/apps/cli/src/legacy/commands/network-bans/remove/remove.command.ts +++ b/apps/cli/src/legacy/commands/network-bans/remove/remove.command.ts @@ -6,21 +6,31 @@ import { withJsonErrorHandling } from "../../../../shared/output/json-error-hand import { legacyRequireExperimental } from "../../../shared/legacy-experimental-gate.ts"; import { LEGACY_RESOURCE_OUTPUT_FORMATS } from "../../../shared/legacy-go-output-flag.ts"; import { legacyManagementApiRuntimeLayer } from "../../../shared/legacy-management-api-runtime.layer.ts"; +import { legacyParseStringSliceFlag } from "../../../shared/legacy-string-slice-flag.ts"; import { legacyValidateOutputFormat, withLegacyCommandInstrumentation, } from "../../../telemetry/legacy-command-instrumentation.ts"; import { legacyNetworkBansRemove } from "./remove.handler.ts"; +// Go declares `--db-unban-ip` with pflag's `StringSliceVar` (`cmd/bans.go:48`), +// which CSV-splits each occurrence (`--db-unban-ip=1.2.3.4,5.6.7.8` → two IPs) +// and appends across repeats. +export const legacyNetworkBansRemoveDbUnbanIpFlag = Flag.string("db-unban-ip").pipe( + Flag.withDescription("IP to allow DB connections from."), + Flag.atLeast(0), + Flag.mapTryCatch( + (rawValues) => legacyParseStringSliceFlag(rawValues), + (err) => (err instanceof Error ? err.message : String(err)), + ), +); + const config = { projectRef: Flag.string("project-ref").pipe( Flag.withDescription("Project ref of the Supabase project."), Flag.optional, ), - dbUnbanIp: Flag.string("db-unban-ip").pipe( - Flag.withDescription("IP to allow DB connections from."), - Flag.atLeast(0), - ), + dbUnbanIp: legacyNetworkBansRemoveDbUnbanIpFlag, } as const; export type LegacyNetworkBansRemoveFlags = CliCommand.Command.Config.Infer; diff --git a/apps/cli/src/legacy/commands/network-bans/remove/remove.command.unit.test.ts b/apps/cli/src/legacy/commands/network-bans/remove/remove.command.unit.test.ts new file mode 100644 index 0000000000..aaffba6c54 --- /dev/null +++ b/apps/cli/src/legacy/commands/network-bans/remove/remove.command.unit.test.ts @@ -0,0 +1,61 @@ +import { BunServices } from "@effect/platform-bun"; +import { Effect, Exit } from "effect"; +import { describe, expect, test } from "vitest"; +import { legacyNetworkBansRemoveDbUnbanIpFlag } from "./remove.command.ts"; + +// Go declares `--db-unban-ip` with pflag's `StringSliceVar` (`cmd/bans.go:48`), +// which CSV-splits each occurrence and appends across repeats. +describe("legacy network-bans remove --db-unban-ip flag (pflag StringSlice parity)", () => { + test("splits a comma-separated value into multiple IPs", async () => { + const [, ips] = await Effect.runPromise( + legacyNetworkBansRemoveDbUnbanIpFlag + .parse({ + flags: { "db-unban-ip": ["12.3.4.5,5.6.7.8"] }, + arguments: [], + }) + .pipe(Effect.provide(BunServices.layer)), + ); + + expect(ips).toEqual(["12.3.4.5", "5.6.7.8"]); + }); + + test("keeps a quoted value with embedded comma as a single element", async () => { + const [, ips] = await Effect.runPromise( + legacyNetworkBansRemoveDbUnbanIpFlag + .parse({ + flags: { "db-unban-ip": ['"12.3.4.5,5.6.7.8"'] }, + arguments: [], + }) + .pipe(Effect.provide(BunServices.layer)), + ); + + expect(ips).toEqual(["12.3.4.5,5.6.7.8"]); + }); + + test("defaults to an empty array when unset", async () => { + const [, ips] = await Effect.runPromise( + legacyNetworkBansRemoveDbUnbanIpFlag + .parse({ + flags: {}, + arguments: [], + }) + .pipe(Effect.provide(BunServices.layer)), + ); + + expect(ips).toEqual([]); + }); + + test("rejects malformed CSV (unterminated quote)", async () => { + const exit = await Effect.runPromise( + legacyNetworkBansRemoveDbUnbanIpFlag + .parse({ + flags: { "db-unban-ip": ['"12.3.4.5'] }, + arguments: [], + }) + .pipe(Effect.provide(BunServices.layer)) + .pipe(Effect.exit), + ); + + expect(Exit.isFailure(exit)).toBe(true); + }); +}); diff --git a/apps/cli/src/legacy/commands/network-bans/remove/remove.integration.test.ts b/apps/cli/src/legacy/commands/network-bans/remove/remove.integration.test.ts index 47e9bf7185..d091f30e69 100644 --- a/apps/cli/src/legacy/commands/network-bans/remove/remove.integration.test.ts +++ b/apps/cli/src/legacy/commands/network-bans/remove/remove.integration.test.ts @@ -1,3 +1,4 @@ +import { BunServices } from "@effect/platform-bun"; import { describe, expect, it } from "@effect/vitest"; import { Effect, Exit, Option } from "effect"; @@ -12,8 +13,19 @@ import { mockLegacyTelemetryStateTracked, useLegacyTempWorkdir, } from "../../../../../tests/helpers/legacy-mocks.ts"; +import { legacyNetworkBansRemoveDbUnbanIpFlag } from "./remove.command.ts"; import { legacyNetworkBansRemove } from "./remove.handler.ts"; +// Runs the real `--db-unban-ip` flag pipeline (pflag StringSlice CSV parity — +// `cmd/bans.go:48`) so these scenarios cover raw CLI values → request body. +const parseDbUnbanIp = (rawValues: ReadonlyArray) => + legacyNetworkBansRemoveDbUnbanIpFlag + .parse({ flags: { "db-unban-ip": rawValues }, arguments: [] }) + .pipe( + Effect.map(([, values]) => values), + Effect.provide(BunServices.layer), + ); + interface SetupOpts { format?: "text" | "json" | "stream-json"; legacyOutput?: "env" | "pretty" | "json" | "toml" | "yaml"; @@ -88,6 +100,69 @@ describe("legacy network-bans remove integration", () => { }).pipe(Effect.provide(layer)); }); + it.live("unbans every IP in a comma-separated --db-unban-ip value (pflag CSV parity)", () => { + const { layer, api } = setup(); + return Effect.gen(function* () { + const dbUnbanIp = yield* parseDbUnbanIp(["12.3.4.5,5.6.7.8"]); + yield* legacyNetworkBansRemove({ projectRef: Option.none(), dbUnbanIp }); + expect(api.requests[0]?.body).toEqual({ + ipv4_addresses: ["12.3.4.5", "5.6.7.8"], + requester_ip: false, + }); + }).pipe(Effect.provide(layer)); + }); + + it.live("appends IPs across repeated --db-unban-ip flags", () => { + const { layer, api } = setup(); + return Effect.gen(function* () { + const dbUnbanIp = yield* parseDbUnbanIp(["12.3.4.5", "5.6.7.8"]); + yield* legacyNetworkBansRemove({ projectRef: Option.none(), dbUnbanIp }); + expect(api.requests[0]?.body).toEqual({ + ipv4_addresses: ["12.3.4.5", "5.6.7.8"], + requester_ip: false, + }); + }).pipe(Effect.provide(layer)); + }); + + it.live("combines comma-separated and repeated --db-unban-ip occurrences", () => { + const { layer, api } = setup(); + return Effect.gen(function* () { + const dbUnbanIp = yield* parseDbUnbanIp(["12.3.4.5,5.6.7.8", "9.9.9.9"]); + yield* legacyNetworkBansRemove({ projectRef: Option.none(), dbUnbanIp }); + expect(api.requests[0]?.body).toEqual({ + ipv4_addresses: ["12.3.4.5", "5.6.7.8", "9.9.9.9"], + requester_ip: false, + }); + }).pipe(Effect.provide(layer)); + }); + + it.live("still unbans a single --db-unban-ip value", () => { + const { layer, api } = setup(); + return Effect.gen(function* () { + const dbUnbanIp = yield* parseDbUnbanIp(["12.3.4.5"]); + yield* legacyNetworkBansRemove({ projectRef: Option.none(), dbUnbanIp }); + expect(api.requests[0]?.body).toEqual({ + ipv4_addresses: ["12.3.4.5"], + requester_ip: false, + }); + }).pipe(Effect.provide(layer)); + }); + + it.live("rejects an invalid IP produced by a comma split before any API call", () => { + const { layer, api } = setup(); + return Effect.gen(function* () { + const dbUnbanIp = yield* parseDbUnbanIp(["12.3.4.5,notanip"]); + const exit = yield* Effect.exit( + legacyNetworkBansRemove({ projectRef: Option.none(), dbUnbanIp }), + ); + expect(Exit.isFailure(exit)).toBe(true); + expect(api.requests).toHaveLength(0); + if (Exit.isFailure(exit)) { + expect(JSON.stringify(exit.cause)).toContain("invalid IP address: notanip"); + } + }).pipe(Effect.provide(layer)); + }); + it.live("ignores legacy --output values and still prints the success line", () => { const { layer, out } = setup({ legacyOutput: "json" }); return Effect.gen(function* () { diff --git a/apps/cli/src/legacy/commands/network-restrictions/update/update.command.ts b/apps/cli/src/legacy/commands/network-restrictions/update/update.command.ts index a2e3cc0667..7fcd877db8 100644 --- a/apps/cli/src/legacy/commands/network-restrictions/update/update.command.ts +++ b/apps/cli/src/legacy/commands/network-restrictions/update/update.command.ts @@ -6,21 +6,31 @@ import { withJsonErrorHandling } from "../../../../shared/output/json-error-hand import { legacyRequireExperimental } from "../../../shared/legacy-experimental-gate.ts"; import { LEGACY_RESOURCE_OUTPUT_FORMATS } from "../../../shared/legacy-go-output-flag.ts"; import { legacyManagementApiRuntimeLayer } from "../../../shared/legacy-management-api-runtime.layer.ts"; +import { legacyParseStringSliceFlag } from "../../../shared/legacy-string-slice-flag.ts"; import { legacyValidateOutputFormat, withLegacyCommandInstrumentation, } from "../../../telemetry/legacy-command-instrumentation.ts"; import { legacyNetworkRestrictionsUpdate } from "./update.handler.ts"; +// Go declares `--db-allow-cidr` with pflag's `StringSliceVar` (`cmd/restrictions.go:40`), +// which CSV-splits each occurrence (`--db-allow-cidr=1.2.3.0/24,5.6.7.0/24` → two +// CIDRs) and appends across repeats. +export const legacyNetworkRestrictionsUpdateDbAllowCidrFlag = Flag.string("db-allow-cidr").pipe( + Flag.withDescription("CIDR to allow DB connections from."), + Flag.atLeast(0), + Flag.mapTryCatch( + (rawValues) => legacyParseStringSliceFlag(rawValues), + (err) => (err instanceof Error ? err.message : String(err)), + ), +); + const config = { projectRef: Flag.string("project-ref").pipe( Flag.withDescription("Project ref of the Supabase project."), Flag.optional, ), - dbAllowCidr: Flag.string("db-allow-cidr").pipe( - Flag.withDescription("CIDR to allow DB connections from."), - Flag.atLeast(0), - ), + dbAllowCidr: legacyNetworkRestrictionsUpdateDbAllowCidrFlag, bypassCidrChecks: Flag.boolean("bypass-cidr-checks").pipe( Flag.withDescription("Bypass some of the CIDR validation checks."), ), diff --git a/apps/cli/src/legacy/commands/network-restrictions/update/update.command.unit.test.ts b/apps/cli/src/legacy/commands/network-restrictions/update/update.command.unit.test.ts new file mode 100644 index 0000000000..c4e403aa90 --- /dev/null +++ b/apps/cli/src/legacy/commands/network-restrictions/update/update.command.unit.test.ts @@ -0,0 +1,62 @@ +import { BunServices } from "@effect/platform-bun"; +import { Effect, Exit } from "effect"; +import { describe, expect, test } from "vitest"; +import { legacyNetworkRestrictionsUpdateDbAllowCidrFlag } from "./update.command.ts"; + +// Go declares `--db-allow-cidr` with pflag's `StringSliceVar` +// (`cmd/restrictions.go:40`), which CSV-splits each occurrence and appends +// across repeats. +describe("legacy network-restrictions update --db-allow-cidr flag (pflag StringSlice parity)", () => { + test("splits a comma-separated value into multiple CIDRs", async () => { + const [, cidrs] = await Effect.runPromise( + legacyNetworkRestrictionsUpdateDbAllowCidrFlag + .parse({ + flags: { "db-allow-cidr": ["1.2.3.0/24,5.6.7.0/24"] }, + arguments: [], + }) + .pipe(Effect.provide(BunServices.layer)), + ); + + expect(cidrs).toEqual(["1.2.3.0/24", "5.6.7.0/24"]); + }); + + test("keeps a quoted value with embedded comma as a single element", async () => { + const [, cidrs] = await Effect.runPromise( + legacyNetworkRestrictionsUpdateDbAllowCidrFlag + .parse({ + flags: { "db-allow-cidr": ['"1.2.3.0/24,5.6.7.0/24"'] }, + arguments: [], + }) + .pipe(Effect.provide(BunServices.layer)), + ); + + expect(cidrs).toEqual(["1.2.3.0/24,5.6.7.0/24"]); + }); + + test("defaults to an empty array when unset", async () => { + const [, cidrs] = await Effect.runPromise( + legacyNetworkRestrictionsUpdateDbAllowCidrFlag + .parse({ + flags: {}, + arguments: [], + }) + .pipe(Effect.provide(BunServices.layer)), + ); + + expect(cidrs).toEqual([]); + }); + + test("rejects malformed CSV (unterminated quote)", async () => { + const exit = await Effect.runPromise( + legacyNetworkRestrictionsUpdateDbAllowCidrFlag + .parse({ + flags: { "db-allow-cidr": ['"1.2.3.0/24'] }, + arguments: [], + }) + .pipe(Effect.provide(BunServices.layer)) + .pipe(Effect.exit), + ); + + expect(Exit.isFailure(exit)).toBe(true); + }); +}); diff --git a/apps/cli/src/legacy/commands/network-restrictions/update/update.integration.test.ts b/apps/cli/src/legacy/commands/network-restrictions/update/update.integration.test.ts index 71b4b5bc59..eb5b7b1c36 100644 --- a/apps/cli/src/legacy/commands/network-restrictions/update/update.integration.test.ts +++ b/apps/cli/src/legacy/commands/network-restrictions/update/update.integration.test.ts @@ -1,3 +1,4 @@ +import { BunServices } from "@effect/platform-bun"; import { type V1PatchNetworkRestrictionsOutput, type V1UpdateNetworkRestrictionsOutput, @@ -17,8 +18,19 @@ import { mockLegacyTelemetryStateTracked, useLegacyTempWorkdir, } from "../../../../../tests/helpers/legacy-mocks.ts"; +import { legacyNetworkRestrictionsUpdateDbAllowCidrFlag } from "./update.command.ts"; import { legacyNetworkRestrictionsUpdate } from "./update.handler.ts"; +// Runs the real `--db-allow-cidr` flag pipeline (pflag StringSlice CSV parity — +// `cmd/restrictions.go:40`) so these scenarios cover raw CLI values → request body. +const parseDbAllowCidr = (rawValues: ReadonlyArray) => + legacyNetworkRestrictionsUpdateDbAllowCidrFlag + .parse({ flags: { "db-allow-cidr": rawValues }, arguments: [] }) + .pipe( + Effect.map(([, values]) => values), + Effect.provide(BunServices.layer), + ); + // --------------------------------------------------------------------------- // Fixtures // --------------------------------------------------------------------------- @@ -193,6 +205,69 @@ describe("legacy network-restrictions update integration", () => { }).pipe(Effect.provide(layer)); }); + it.live("allows every CIDR in a comma-separated --db-allow-cidr value (pflag CSV parity)", () => { + const { layer, api } = setup(); + return Effect.gen(function* () { + const dbAllowCidr = yield* parseDbAllowCidr(["1.2.3.0/24,5.6.7.0/24"]); + yield* legacyNetworkRestrictionsUpdate({ ...baseFlags, dbAllowCidr }); + expect(api.requests[0]?.body).toEqual({ + dbAllowedCidrs: ["1.2.3.0/24", "5.6.7.0/24"], + dbAllowedCidrsV6: [], + }); + }).pipe(Effect.provide(layer)); + }); + + it.live("appends CIDRs across repeated --db-allow-cidr flags", () => { + const { layer, api } = setup(); + return Effect.gen(function* () { + const dbAllowCidr = yield* parseDbAllowCidr(["1.2.3.0/24", "5.6.7.0/24"]); + yield* legacyNetworkRestrictionsUpdate({ ...baseFlags, dbAllowCidr }); + expect(api.requests[0]?.body).toEqual({ + dbAllowedCidrs: ["1.2.3.0/24", "5.6.7.0/24"], + dbAllowedCidrsV6: [], + }); + }).pipe(Effect.provide(layer)); + }); + + it.live("combines comma-separated and repeated --db-allow-cidr occurrences", () => { + const { layer, api } = setup(); + return Effect.gen(function* () { + const dbAllowCidr = yield* parseDbAllowCidr(["1.2.3.0/24,5.6.7.0/24", "9.9.9.0/24"]); + yield* legacyNetworkRestrictionsUpdate({ ...baseFlags, dbAllowCidr }); + expect(api.requests[0]?.body).toEqual({ + dbAllowedCidrs: ["1.2.3.0/24", "5.6.7.0/24", "9.9.9.0/24"], + dbAllowedCidrsV6: [], + }); + }).pipe(Effect.provide(layer)); + }); + + it.live("still allows a single --db-allow-cidr value", () => { + const { layer, api } = setup(); + return Effect.gen(function* () { + const dbAllowCidr = yield* parseDbAllowCidr(["1.2.3.0/24"]); + yield* legacyNetworkRestrictionsUpdate({ ...baseFlags, dbAllowCidr }); + expect(api.requests[0]?.body).toEqual({ + dbAllowedCidrs: ["1.2.3.0/24"], + dbAllowedCidrsV6: [], + }); + }).pipe(Effect.provide(layer)); + }); + + it.live("rejects an invalid CIDR produced by a comma split before any API call", () => { + const { layer, api } = setup(); + return Effect.gen(function* () { + const dbAllowCidr = yield* parseDbAllowCidr(["1.2.3.0/24,notacidr"]); + const exit = yield* Effect.exit( + legacyNetworkRestrictionsUpdate({ ...baseFlags, dbAllowCidr }), + ); + expect(Exit.isFailure(exit)).toBe(true); + expect(api.requests).toHaveLength(0); + if (Exit.isFailure(exit)) { + expect(JSON.stringify(exit.cause)).toContain("failed to parse IP: notacidr"); + } + }).pipe(Effect.provide(layer)); + }); + // ------------------------------------------------------------------------- // Append mode (PATCH /network-restrictions) // -------------------------------------------------------------------------