diff --git a/apps/cli/src/legacy/commands/network-bans/network-bans.experimental-gate.integration.test.ts b/apps/cli/src/legacy/commands/network-bans/network-bans.experimental-gate.integration.test.ts index 867514f1df..958e97e256 100644 --- a/apps/cli/src/legacy/commands/network-bans/network-bans.experimental-gate.integration.test.ts +++ b/apps/cli/src/legacy/commands/network-bans/network-bans.experimental-gate.integration.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from "@effect/vitest"; import { Effect, Exit, Layer } from "effect"; import { CliOutput, Command } from "effect/unstable/cli"; +import { normalizeCause } from "../../../shared/output/normalize-error.ts"; import { textCliOutputFormatter } from "../../../shared/output/text-formatter.ts"; import { LEGACY_GLOBAL_FLAGS } from "../../../shared/legacy/global-flags.ts"; import { TelemetryRuntime } from "../../../shared/telemetry/runtime.service.ts"; @@ -118,4 +119,36 @@ describe("legacy network-bans experimental gate (Go PersistentPreRunE parity)", }).pipe(Effect.provide(layer)); }); } + + it.live( + "remove: malformed --db-unban-ip CSV fails at parse time with pflag's exact diagnostic, before the gate", + () => { + // Go parity (CLI-1983): pflag's `readAsCSV` error aborts cobra's + // `ParseFlags` BEFORE `PersistentPreRunE`'s experimental-gate check, so + // the parse error must win even with `--experimental` unset. The + // rendered line — what `runCli`'s `handledProgram` writes to stderr via + // `normalizeCause` — byte-matches the real Go CLI (pflag v1.0.10 + // `errors.go:116` wrapping `encoding/csv`; `"1.2.3.4` is 8 bytes → EOF + // at column 9). + const { layer, api } = setup(); + return Effect.gen(function* () { + const exit = yield* Effect.exit( + Command.runWith(testRoot, { version: "0.0.0-test" })([ + "network-bans", + "remove", + "--db-unban-ip", + '"1.2.3.4', + ]), + ); + expect(Exit.isFailure(exit)).toBe(true); + if (Exit.isFailure(exit)) { + expect(JSON.stringify(exit.cause)).not.toContain("LegacyExperimentalRequiredError"); + expect(normalizeCause(exit.cause).message).toBe( + 'invalid argument "\\"1.2.3.4" for "--db-unban-ip" flag: parse error on line 1, column 9: extraneous or missing " in quoted-field', + ); + } + expect(api.requests).toHaveLength(0); + }).pipe(Effect.provide(layer)); + }, + ); }); diff --git a/apps/cli/src/legacy/commands/network-bans/remove/SIDE_EFFECTS.md b/apps/cli/src/legacy/commands/network-bans/remove/SIDE_EFFECTS.md index 937970c60d..15a89c610e 100644 --- a/apps/cli/src/legacy/commands/network-bans/remove/SIDE_EFFECTS.md +++ b/apps/cli/src/legacy/commands/network-bans/remove/SIDE_EFFECTS.md @@ -9,10 +9,10 @@ ## Files Written -| Path | Format | When | -| ------------------------------------------------ | ------ | -------------------------------------------------------------------------------------------------------------------------- | -| `~/.supabase//linked-project.json` | JSON | once the `--experimental` gate is open, after ref resolution, via `Effect.ensuring` — on success and failure | -| `~/.supabase/telemetry.json` | JSON | once the `--experimental` gate is open, via `Effect.ensuring` — on success and failure. Not written if the gate is closed. | +| Path | Format | When | +| ------------------------------------------------ | ------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `~/.supabase//linked-project.json` | JSON | once the `--experimental` gate is open, after ref resolution, via `Effect.ensuring` — on success and failure | +| `~/.supabase/telemetry.json` | JSON | once the `--experimental` gate is open, via `Effect.ensuring` — on success and failure. Not written if the gate is closed or if flag parsing fails (malformed `--db-unban-ip` CSV). | ## API Routes @@ -33,14 +33,15 @@ ## Exit Codes -| Code | Condition | -| ---- | ----------------------------------------------------------------------------------------------------------------------------------------------- | -| `0` | success — network ban removed | -| `1` | `--experimental` not passed and `SUPABASE_EXPERIMENTAL` unset (`LegacyExperimentalRequiredError`) — checked before ref resolution/API/telemetry | -| `1` | invalid IP supplied via `--db-unban-ip` (`LegacyNetworkBansInvalidIpError`) | -| `1` | project ref unresolved (`LegacyProjectNotLinkedError` / `LegacyInvalidProjectRefError`) | -| `1` | API non-2xx (`LegacyNetworkBansRemoveUnexpectedStatusError`) | -| `1` | transport failure (`LegacyNetworkBansRemoveNetworkError`) | +| Code | Condition | +| ---- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `0` | success — network ban removed | +| `1` | malformed CSV in a `--db-unban-ip` value, e.g. unterminated quote — stderr byte-matches pflag's diagnostic (`invalid argument "\"1.2.3.4" for "--db-unban-ip" flag: parse error on line 1, column 9: extraneous or missing " in quoted-field`; columns are 1-based byte offsets, per Go `encoding/csv`) — fails during flag parsing, before the `--experimental` gate, the handler, the `linked-project.json`/`telemetry.json` writes, and the `cli_command_executed` event | +| `1` | `--experimental` not passed and `SUPABASE_EXPERIMENTAL` unset (`LegacyExperimentalRequiredError`) — checked before ref resolution/API/telemetry | +| `1` | invalid IP supplied via `--db-unban-ip` (`LegacyNetworkBansInvalidIpError`) | +| `1` | project ref unresolved (`LegacyProjectNotLinkedError` / `LegacyInvalidProjectRefError`) | +| `1` | API non-2xx (`LegacyNetworkBansRemoveUnexpectedStatusError`) | +| `1` | transport failure (`LegacyNetworkBansRemoveNetworkError`) | ## Telemetry Events Fired @@ -80,7 +81,7 @@ One `result` event on success when the Go `--output` flag is unset. - Requires `--db-unban-ip` flag to specify IP(s) to unban (repeatable). When omitted, the caller's own IP is unbanned (`requester_ip: true`). - Requires `--project-ref` or a linked project (`.supabase/config.json`). - `linked-project.json` is written **after** the project ref is resolved, regardless of whether the subsequent API call succeeds (mirrors Go's `PersistentPostRun`). -- `telemetry.json` is written on every invocation, including failures, but only once the `--experimental` gate is open. +- `telemetry.json` is written on every invocation that reaches the handler, including failures, but only once the `--experimental` gate is open. A malformed `--db-unban-ip` CSV value (e.g. an unterminated quote) fails during flag parsing — before the gate, the handler, both file writes, and the `cli_command_executed` event — even when `--experimental` is passed. This matches Go: pflag's `readAsCSV` error aborts cobra's `ParseFlags` before `PersistentPreRunE` ever creates the telemetry service (`root.go:131-142`), so `Execute()`'s post-run capture and `ensureProjectGroupsCached` (`root.go:171-181`) never fire. - `network-bans` is an experimental command (Go `root.go:63`, `bansCmd`): `remove` requires `--experimental` (or `SUPABASE_EXPERIMENTAL`), matching Go's root-level `PersistentPreRunE` gate (`root.go:91-96`), which runs before the `IsManagementAPI` login check (`root.go:105-109`). 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..e56f26c640 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,33 @@ 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 { legacyStringSliceFlag } 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. Malformed CSV fails at parse time with pflag's +// exact diagnostic (see `legacyStringSliceFlag`). Accepted approximation: +// given an invalid `-o` AND malformed CSV together, Go fails on whichever bad +// flag comes first in argv (pflag parses left-to-right); here the CSV error +// always wins, because the global `-o` is validated in-handler +// (`legacyValidateOutputFormat`) — same divergence class as the `-o` vs +// `--experimental` ordering note in the handler below. +export const legacyNetworkBansRemoveDbUnbanIpFlag = legacyStringSliceFlag( + "db-unban-ip", + "IP to allow DB connections from.", +); + 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..b2d07f841a --- /dev/null +++ b/apps/cli/src/legacy/commands/network-bans/remove/remove.command.unit.test.ts @@ -0,0 +1,70 @@ +import { BunServices } from "@effect/platform-bun"; +import { Cause, 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) with pflag's exact diagnostic", 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); + if (!Exit.isFailure(exit)) return; + // Byte-matches the Go CLI (pflag v1.0.10 `errors.go:116` wrapping + // `encoding/csv`'s error; `"12.3.4.5` is 9 bytes → EOF at column 10). + const error = Cause.squash(exit.cause); + expect(error).toMatchObject({ + _tag: "InvalidValue", + expected: + 'invalid argument "\\"12.3.4.5" for "--db-unban-ip" flag: parse error on line 1, column 10: extraneous or missing " in quoted-field', + }); + }); +}); 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/network-restrictions.experimental-gate.integration.test.ts b/apps/cli/src/legacy/commands/network-restrictions/network-restrictions.experimental-gate.integration.test.ts index 57acaad0b9..22f0e3649f 100644 --- a/apps/cli/src/legacy/commands/network-restrictions/network-restrictions.experimental-gate.integration.test.ts +++ b/apps/cli/src/legacy/commands/network-restrictions/network-restrictions.experimental-gate.integration.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from "@effect/vitest"; import { Effect, Exit, Layer } from "effect"; import { CliOutput, Command } from "effect/unstable/cli"; +import { normalizeCause } from "../../../shared/output/normalize-error.ts"; import { textCliOutputFormatter } from "../../../shared/output/text-formatter.ts"; import { LEGACY_GLOBAL_FLAGS } from "../../../shared/legacy/global-flags.ts"; import { TelemetryRuntime } from "../../../shared/telemetry/runtime.service.ts"; @@ -118,4 +119,36 @@ describe("legacy network-restrictions experimental gate (Go PersistentPreRunE pa }).pipe(Effect.provide(layer)); }); } + + it.live( + "update: malformed --db-allow-cidr CSV fails at parse time with pflag's exact diagnostic, before the gate", + () => { + // Go parity (CLI-1983): pflag's `readAsCSV` error aborts cobra's + // `ParseFlags` BEFORE `PersistentPreRunE`'s experimental-gate check, so + // the parse error must win even with `--experimental` unset. The + // rendered line — what `runCli`'s `handledProgram` writes to stderr via + // `normalizeCause` — byte-matches the real Go CLI (pflag v1.0.10 + // `errors.go:116` wrapping `encoding/csv`; `"1.2.3.0/24` is 11 bytes → + // EOF at column 12). + const { layer, api } = setup(); + return Effect.gen(function* () { + const exit = yield* Effect.exit( + Command.runWith(testRoot, { version: "0.0.0-test" })([ + "network-restrictions", + "update", + "--db-allow-cidr", + '"1.2.3.0/24', + ]), + ); + expect(Exit.isFailure(exit)).toBe(true); + if (Exit.isFailure(exit)) { + expect(JSON.stringify(exit.cause)).not.toContain("LegacyExperimentalRequiredError"); + expect(normalizeCause(exit.cause).message).toBe( + 'invalid argument "\\"1.2.3.0/24" for "--db-allow-cidr" flag: parse error on line 1, column 12: extraneous or missing " in quoted-field', + ); + } + expect(api.requests).toHaveLength(0); + }).pipe(Effect.provide(layer)); + }, + ); }); diff --git a/apps/cli/src/legacy/commands/network-restrictions/update/SIDE_EFFECTS.md b/apps/cli/src/legacy/commands/network-restrictions/update/SIDE_EFFECTS.md index 3eb0cd2630..9bc0e16cd5 100644 --- a/apps/cli/src/legacy/commands/network-restrictions/update/SIDE_EFFECTS.md +++ b/apps/cli/src/legacy/commands/network-restrictions/update/SIDE_EFFECTS.md @@ -9,10 +9,10 @@ ## Files Written -| Path | Format | When | -| ------------------------------------------------ | ------ | ------------------------------------------------------------------------------------------------------------------------------------ | -| `~/.supabase//linked-project.json` | JSON | once the `--experimental` gate is open, after ref resolution, via `Effect.ensuring` — success and HTTP failure | -| `~/.supabase/telemetry.json` | JSON | once the `--experimental` gate is open, via outermost `Effect.ensuring` — including CIDR validation failures. Not written if closed. | +| Path | Format | When | +| ------------------------------------------------ | ------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `~/.supabase//linked-project.json` | JSON | once the `--experimental` gate is open, after ref resolution, via `Effect.ensuring` — success and HTTP failure | +| `~/.supabase/telemetry.json` | JSON | once the `--experimental` gate is open, via outermost `Effect.ensuring` — including CIDR validation failures. Not written if closed or if flag parsing fails (malformed `--db-allow-cidr` CSV). | ## API Routes @@ -37,21 +37,22 @@ when no `--db-allow-cidr` was supplied), matching Go's `&[]string{}` initializat ## Exit Codes -| Code | Condition | -| ---- | ------------------------------------------------------------------------------------------------------------------------------------------ | -| `0` | success — network restrictions updated and status printed to stdout | -| `1` | `--experimental` not passed and `SUPABASE_EXPERIMENTAL` unset (`LegacyExperimentalRequiredError`) — checked before CIDR validation/ref/API | -| `1` | CIDR parse failure — `LegacyNetworkRestrictionsInvalidCidrError` (`failed to parse IP: `) | -| `1` | private-IP rejection — `LegacyNetworkRestrictionsPrivateIpError` (`private IP provided: `) | -| `1` | project ref unresolved (`LegacyProjectNotLinkedError` / `LegacyInvalidProjectRefError`) | -| `1` | API non-201 (POST) / non-200 (PATCH) — `LegacyNetworkRestrictionsUpdateUnexpectedStatusError` | -| `1` | transport failure — `LegacyNetworkRestrictionsUpdateNetworkError` | +| Code | Condition | +| ---- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `0` | success — network restrictions updated and status printed to stdout | +| `1` | malformed CSV in a `--db-allow-cidr` value, e.g. unterminated quote — stderr byte-matches pflag's diagnostic (`invalid argument "\"1.2.3.0/24" for "--db-allow-cidr" flag: parse error on line 1, column 12: extraneous or missing " in quoted-field`; columns are 1-based byte offsets, per Go `encoding/csv`) — fails during flag parsing, before the `--experimental` gate, CIDR validation, the `linked-project.json`/`telemetry.json` writes, and the `cli_command_executed` event | +| `1` | `--experimental` not passed and `SUPABASE_EXPERIMENTAL` unset (`LegacyExperimentalRequiredError`) — checked before CIDR validation/ref/API | +| `1` | CIDR parse failure — `LegacyNetworkRestrictionsInvalidCidrError` (`failed to parse IP: `) | +| `1` | private-IP rejection — `LegacyNetworkRestrictionsPrivateIpError` (`private IP provided: `) | +| `1` | project ref unresolved (`LegacyProjectNotLinkedError` / `LegacyInvalidProjectRefError`) | +| `1` | API non-201 (POST) / non-200 (PATCH) — `LegacyNetworkRestrictionsUpdateUnexpectedStatusError` | +| `1` | transport failure — `LegacyNetworkRestrictionsUpdateNetworkError` | ## Telemetry Events Fired -| Event | When | Notable properties / groups | -| ---------------------- | ---------------------------------------------------------------------------------------------- | -------------------------------------------------------------------- | -| `cli_command_executed` | post-run, success or failure (via wrapper); not fired when the `--experimental` gate is closed | `exit_code`, `duration_ms`, `flags` (`--project-ref` → ``) | +| Event | When | Notable properties / groups | +| ---------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------- | +| `cli_command_executed` | post-run, success or failure (via wrapper); not fired when the `--experimental` gate is closed or when flag parsing fails (malformed `--db-allow-cidr` CSV) | `exit_code`, `duration_ms`, `flags` (`--project-ref` → ``) | Matches `apps/cli-go/internal/restrictions/update/`. Go does not fire any custom telemetry event for this command. @@ -114,5 +115,10 @@ One `result` event whose `data` is the full response object. - `telemetry.json` writes on every invocation past the `--experimental` gate, including CIDR validation failures, ref resolution failures, and API failures. A closed gate writes nothing (Go's `PersistentPreRunE` fails before `PersistentPostRun` runs). + A malformed `--db-allow-cidr` CSV value (e.g. an unterminated quote) also writes nothing + and fires no telemetry, even with `--experimental` set — it fails during flag parsing, + before the gate and the handler. This matches Go: pflag's `readAsCSV` error aborts + cobra's `ParseFlags` before `PersistentPreRunE` ever creates the telemetry service + (`root.go:131-142`), so `Execute()`'s post-run capture (`root.go:171-181`) never fires. - Go's `restrictions/update` itself does not honor `--output`. The legacy TS port honors both `--output` and `--output-format` per the legacy CLAUDE.md output-parity rules. 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..86c533b5e4 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,33 @@ 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 { legacyStringSliceFlag } 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. Malformed CSV fails at parse time with +// pflag's exact diagnostic (see `legacyStringSliceFlag`). Accepted +// approximation: given an invalid `-o` AND malformed CSV together, Go fails on +// whichever bad flag comes first in argv (pflag parses left-to-right); here +// the CSV error always wins, because the global `-o` is validated in-handler +// (`legacyValidateOutputFormat`) — same divergence class as the `-o` vs +// `--experimental` ordering note in the handler below. +export const legacyNetworkRestrictionsUpdateDbAllowCidrFlag = legacyStringSliceFlag( + "db-allow-cidr", + "CIDR to allow DB connections from.", +); + 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..9095b564fc --- /dev/null +++ b/apps/cli/src/legacy/commands/network-restrictions/update/update.command.unit.test.ts @@ -0,0 +1,71 @@ +import { BunServices } from "@effect/platform-bun"; +import { Cause, 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) with pflag's exact diagnostic", 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); + if (!Exit.isFailure(exit)) return; + // Byte-matches the Go CLI (pflag v1.0.10 `errors.go:116` wrapping + // `encoding/csv`'s error; `"1.2.3.0/24` is 11 bytes → EOF at column 12). + const error = Cause.squash(exit.cause); + expect(error).toMatchObject({ + _tag: "InvalidValue", + expected: + 'invalid argument "\\"1.2.3.0/24" for "--db-allow-cidr" flag: parse error on line 1, column 12: extraneous or missing " in quoted-field', + }); + }); +}); 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) // ------------------------------------------------------------------------- diff --git a/apps/cli/src/legacy/shared/legacy-string-slice-flag.ts b/apps/cli/src/legacy/shared/legacy-string-slice-flag.ts index dafb3df022..6c45e4cb45 100644 --- a/apps/cli/src/legacy/shared/legacy-string-slice-flag.ts +++ b/apps/cli/src/legacy/shared/legacy-string-slice-flag.ts @@ -9,15 +9,27 @@ * Whitespace is NOT trimmed and empty fields are NOT dropped: Go's csv.Reader * returns raw field values; pflag appends them directly to the slice. */ +import { Flag } from "effect/unstable/cli"; + +/** + * Go's `encoding/csv` reports `ParseError.Column` as a **1-based byte offset** + * within the input line (`reader.go` tracks `pos.col` in bytes, not runes) — + * verified against the real Go CLI: `é"x` fails at column 3 (`é` is 2 UTF-8 + * bytes), not 2. + */ +const utf8ByteOffset = (val: string, index: number): number => + new TextEncoder().encode(val.slice(0, index)).length + 1; /** Thrown by `legacyParseStringSliceFlag` when a value is not valid CSV. */ export class LegacyStringSliceFlagParseError extends Error { readonly value: string; + readonly column: number; readonly detail: string; - constructor(value: string, detail: string) { - super(`parse error on line 1, column 0: ${detail}`); + constructor(value: string, column: number, detail: string) { + super(`parse error on line 1, column ${column}: ${detail}`); this.name = "LegacyStringSliceFlagParseError"; this.value = value; + this.column = column; this.detail = detail; } } @@ -60,20 +72,36 @@ function readAsCSVStrict(val: string): string[] { } } if (!closed) { - // Ran off the end without finding a closing quote. - throw new LegacyStringSliceFlagParseError(val, `extraneous or missing " in quoted-field`); + // Ran off the end without finding a closing quote. Go's csv.Reader + // hits EOF here, so the reported column is one past the final byte. + throw new LegacyStringSliceFlagParseError( + val, + utf8ByteOffset(val, val.length), + `extraneous or missing " in quoted-field`, + ); } // After the closing quote only a comma or end-of-string is allowed. + // Go reports the byte position of the closing quote itself + // (`reader.go`: `pos.col - quoteLen`) — `i` already skipped past it. if (i < val.length && val[i] !== ",") { - throw new LegacyStringSliceFlagParseError(val, `extraneous or missing " in quoted-field`); + throw new LegacyStringSliceFlagParseError( + val, + utf8ByteOffset(val, i - 1), + `extraneous or missing " in quoted-field`, + ); } fields.push(field); } else { - // Unquoted field: a bare `"` anywhere inside is illegal. + // Unquoted field: a bare `"` anywhere inside is illegal. Go reports + // the byte position of the offending quote. const start = i; while (i < val.length && val[i] !== ",") { if (val[i] === '"') { - throw new LegacyStringSliceFlagParseError(val, `bare " in non-quoted-field`); + throw new LegacyStringSliceFlagParseError( + val, + utf8ByteOffset(val, i), + `bare " in non-quoted-field`, + ); } i++; } @@ -114,3 +142,34 @@ export function legacyParseStringSliceFlag( } return values; } + +/** + * Builds a legacy flag that ports a shorthand-less pflag `StringSliceVar`: + * repeatable, CSV-split per occurrence, accumulated across repeats. + * + * On malformed CSV it fails at parse time — matching Go, where pflag's + * `readAsCSV` error aborts cobra's `ParseFlags` before `PersistentPreRunE` + * (so before the `--experimental` gate, telemetry, and the handler) — with + * pflag's exact diagnostic: `invalid argument %q for %q flag: %v` + * (pflag v1.0.10 `errors.go:116`, wrapped per occurrence in `flag.go:493`). + * The full Go message is emitted as the failure's `expected` text so the + * renderer's pflag passthrough (`formatInvalidValueMessage`) prints it + * verbatim, byte-matching the Go CLI's stderr line. `JSON.stringify` mirrors + * Go's `%q` for the ASCII/printable-Unicode values these flags carry (same + * precedent as `sso.format.ts`). + */ +export function legacyStringSliceFlag(name: string, description: string) { + return Flag.string(name).pipe( + Flag.withDescription(description), + Flag.atLeast(0), + Flag.mapTryCatch( + (rawValues) => legacyParseStringSliceFlag(rawValues), + (err) => + err instanceof LegacyStringSliceFlagParseError + ? `invalid argument ${JSON.stringify(err.value)} for "--${name}" flag: ${err.message}` + : err instanceof Error + ? err.message + : String(err), + ), + ); +} diff --git a/apps/cli/src/legacy/shared/legacy-string-slice-flag.unit.test.ts b/apps/cli/src/legacy/shared/legacy-string-slice-flag.unit.test.ts index af6fb4f41c..67c708c37c 100644 --- a/apps/cli/src/legacy/shared/legacy-string-slice-flag.unit.test.ts +++ b/apps/cli/src/legacy/shared/legacy-string-slice-flag.unit.test.ts @@ -44,28 +44,75 @@ describe("legacyParseStringSliceFlag (pflag StringSlice CSV parity)", () => { expect(legacyParseStringSliceFlag([" public , private "])).toEqual([" public ", " private "]); }); - // --- malformed inputs: must THROW --- + // --- malformed inputs: must THROW with Go's exact message --- + // + // Columns are 1-based BYTE offsets, matching Go `encoding/csv`'s + // `ParseError.Column`. Every vector below was verified against the real Go + // CLI (`apps/cli-go`, pflag v1.0.10 → encoding/csv, Go 1.26). - it("throws on an unterminated quoted field", () => { - // `"tenant` — opening quote but no closing quote + it("throws on an unterminated quoted field (column = byte length + 1, Go hits EOF)", () => { + // `"tenant` — opening quote but no closing quote; 7 bytes → column 8 expect(() => legacyParseStringSliceFlag(['"tenant'])).toThrow(LegacyStringSliceFlagParseError); expect(() => legacyParseStringSliceFlag(['"tenant'])).toThrow( - /extraneous or missing " in quoted-field/, + 'parse error on line 1, column 8: extraneous or missing " in quoted-field', + ); + // `"1.2.3.4` — 8 bytes → column 9 + expect(() => legacyParseStringSliceFlag(['"1.2.3.4'])).toThrow( + 'parse error on line 1, column 9: extraneous or missing " in quoted-field', + ); + // `a,"b` — the unterminated quote opens the SECOND field, but the column + // still counts from the start of the whole value; 4 bytes → column 5 + expect(() => legacyParseStringSliceFlag(['a,"b'])).toThrow( + 'parse error on line 1, column 5: extraneous or missing " in quoted-field', ); }); - it("throws on extra bytes after a closing quote", () => { - // `"a"b` — closing quote followed by a non-comma character + it("throws on extra bytes after a closing quote (column = byte position of the closing quote)", () => { + // `"a"b` — closing quote at byte 3 expect(() => legacyParseStringSliceFlag(['"a"b'])).toThrow(LegacyStringSliceFlagParseError); expect(() => legacyParseStringSliceFlag(['"a"b'])).toThrow( - /extraneous or missing " in quoted-field/, + 'parse error on line 1, column 3: extraneous or missing " in quoted-field', + ); + // `aa,"b"x` — closing quote of the second field at byte 6 + expect(() => legacyParseStringSliceFlag(['aa,"b"x'])).toThrow( + 'parse error on line 1, column 6: extraneous or missing " in quoted-field', ); }); - it("throws on a bare quote inside an unquoted field", () => { - // `a"b` — bare " in a field that did not start with a quote + it("throws on a bare quote inside an unquoted field (column = byte position of the quote)", () => { + // `a"b` — bare " at byte 2 expect(() => legacyParseStringSliceFlag(['a"b'])).toThrow(LegacyStringSliceFlagParseError); - expect(() => legacyParseStringSliceFlag(['a"b'])).toThrow(/bare " in non-quoted-field/); + expect(() => legacyParseStringSliceFlag(['a"b'])).toThrow( + 'parse error on line 1, column 2: bare " in non-quoted-field', + ); + // `1.2.3.4,5"6` — bare " in the second field, at byte 10 of the value + expect(() => legacyParseStringSliceFlag(['1.2.3.4,5"6'])).toThrow( + 'parse error on line 1, column 10: bare " in non-quoted-field', + ); + }); + + it("counts columns in bytes, not code points (Go csv tracks byte offsets)", () => { + // `é"x` — é is 2 UTF-8 bytes, so the bare quote sits at byte 3 + expect(() => legacyParseStringSliceFlag(['é"x'])).toThrow( + 'parse error on line 1, column 3: bare " in non-quoted-field', + ); + // `"é` — 3 bytes total, EOF in a quoted field → column 4 + expect(() => legacyParseStringSliceFlag(['"é'])).toThrow( + 'parse error on line 1, column 4: extraneous or missing " in quoted-field', + ); + }); + + it("carries the offending occurrence and column on the error for pflag framing", () => { + try { + legacyParseStringSliceFlag(["public", '"broken']); + expect.unreachable("expected legacyParseStringSliceFlag to throw"); + } catch (err) { + if (!(err instanceof LegacyStringSliceFlagParseError)) throw err; + // pflag wraps the csv error PER OCCURRENCE (`flag.go` `Set`), quoting + // only the malformed value — not the accumulated list. + expect(err.value).toBe('"broken'); + expect(err.column).toBe(8); + } }); it("throws on the first malformed value in a multi-value list", () => { diff --git a/apps/cli/src/shared/cli/invalid-value-message.ts b/apps/cli/src/shared/cli/invalid-value-message.ts index ff895e2a94..b3eeb376fb 100644 --- a/apps/cli/src/shared/cli/invalid-value-message.ts +++ b/apps/cli/src/shared/cli/invalid-value-message.ts @@ -36,6 +36,16 @@ // instead. const EXPECTED_PREFIX = "Expected "; +// Go-parity passthrough (CLI-1983): legacy flags that byte-match Go pflag's +// parse-time diagnostics (`legacyStringSliceFlag`'s malformed-CSV failure, +// `migration down --last`) fail with the COMPLETE Go message as `expected` — +// pflag's `invalid argument %q for %q flag: %v` (pflag v1.0.10 +// `errors.go:116`). Wrapping that in `CliError.InvalidValue`'s own +// `Invalid value for flag --X: "V". Expected: ...` template would +// double-frame it and break the legacy shell's stderr contract, so render it +// verbatim instead. +const PFLAG_INVALID_ARGUMENT_PREFIX = "invalid argument "; + export interface InvalidValueMessageFields { readonly option: string; readonly value: string; @@ -45,11 +55,13 @@ export interface InvalidValueMessageFields { /** * Rebuilds a `CliError.InvalidValue` message from its own template when - * `expected` carries the doubled "Expected" prefix. Returns `undefined` when - * `expected` is unaffected, so callers can fall back to the error's own - * untouched `message`. + * `expected` carries the doubled "Expected" prefix, or passes `expected` + * through verbatim when it is a complete pflag-format diagnostic. Returns + * `undefined` when `expected` is unaffected, so callers can fall back to the + * error's own untouched `message`. */ export function formatInvalidValueMessage(error: InvalidValueMessageFields): string | undefined { + if (error.expected.startsWith(PFLAG_INVALID_ARGUMENT_PREFIX)) return error.expected; if (!error.expected.startsWith(EXPECTED_PREFIX)) return undefined; return error.kind === "argument" ? `Invalid value for argument <${error.option}>: "${error.value}". ${error.expected}` diff --git a/apps/cli/src/shared/cli/subcommand-flag-suggestions.unit.test.ts b/apps/cli/src/shared/cli/subcommand-flag-suggestions.unit.test.ts index 595b0969c6..c86fdfe188 100644 --- a/apps/cli/src/shared/cli/subcommand-flag-suggestions.unit.test.ts +++ b/apps/cli/src/shared/cli/subcommand-flag-suggestions.unit.test.ts @@ -198,6 +198,26 @@ describe("subcommand flag placement suggestions", () => { ); }); + it("passes a complete pflag-format diagnostic through verbatim (Go stderr parity, CLI-1983)", () => { + // Legacy flags that byte-match Go pflag's parse-time diagnostics + // (`legacyStringSliceFlag`'s malformed-CSV failure) emit the COMPLETE Go + // message as `expected`. Wrapping it in the `Invalid value for flag ...` + // template would double-frame it — pflag prints the bare line. + const pflagMessage = + 'invalid argument "\\"1.2.3.4" for "--db-unban-ip" flag: parse error on line 1, column 9: extraneous or missing " in quoted-field'; + const errors = formatCliErrorsForDisplay([ + new CliError.InvalidValue({ + option: "db-unban-ip", + value: '"1.2.3.4', + expected: pflagMessage, + kind: "flag", + }), + ]); + + expect(errors.changed).toBe(true); + expect(errors.errors[0]?.message).toBe(pflagMessage); + }); + it("does not corrupt a value that itself contains the literal 'Expected: Expected' text", () => { // Regression test: the fix must anchor on `error.expected` (the field the // buggy primitive actually populates) rather than searching the fully diff --git a/apps/cli/src/shared/output/normalize-error.unit.test.ts b/apps/cli/src/shared/output/normalize-error.unit.test.ts index 5f7c122ae3..feb74a74bc 100644 --- a/apps/cli/src/shared/output/normalize-error.unit.test.ts +++ b/apps/cli/src/shared/output/normalize-error.unit.test.ts @@ -114,6 +114,49 @@ describe("normalizeCliError", () => { }); }); + test("InvalidValue passes a complete pflag-format diagnostic through verbatim (Go stderr parity, CLI-1983)", () => { + // Legacy flags that byte-match Go pflag's parse-time diagnostics + // (`legacyStringSliceFlag`'s malformed-CSV failure, `migration down + // --last`) emit the COMPLETE Go message as `expected`. Wrapping it in + // Effect's `Invalid value for flag ...: Expected: ...` template would + // double-frame it — Go prints the bare pflag line. + const pflagMessage = + 'invalid argument "\\"1.2.3.4" for "--db-unban-ip" flag: parse error on line 1, column 9: extraneous or missing " in quoted-field'; + const error = new CliError.InvalidValue({ + option: "db-unban-ip", + value: '"1.2.3.4', + expected: pflagMessage, + kind: "flag", + }); + + expect(normalizeCliError(error)).toEqual({ + code: "InvalidValue", + message: pflagMessage, + }); + }); + + test("ShowHelp envelope unwraps a single InvalidValue carrying a pflag-format diagnostic verbatim", () => { + const pflagMessage = + 'invalid argument "\\"1.2.3.0/24" for "--db-allow-cidr" flag: parse error on line 1, column 12: extraneous or missing " in quoted-field'; + const error = { + _tag: "ShowHelp", + commandPath: ["network-restrictions", "update"], + errors: [ + new CliError.InvalidValue({ + option: "db-allow-cidr", + value: '"1.2.3.0/24', + expected: pflagMessage, + kind: "flag", + }), + ], + }; + + expect(normalizeCliError(error)).toEqual({ + code: "InvalidValue", + message: pflagMessage, + }); + }); + test("InvalidValue leaves an already-clean 'expected' message untouched", () => { const error = new CliError.InvalidValue({ option: "define",