fix(cli): comma-split network-bans and network-restrictions slice flags like pflag (CLI-1983) by Coly010 · Pull Request #5975 · supabase/cli

Coly010 · 2026-07-28T14:20:13Z

What changed

Go declares both --db-unban-ip (network-bans remove, cmd/bans.go:48) and --db-allow-cidr (network-restrictions update, cmd/restrictions.go:40) with pflag's StringSliceVar, which CSV-splits every occurrence (--db-unban-ip=1.2.3.4,5.6.7.8 → two IPs) and appends across repeated flags. The TS legacy ports used plain Flag.string(...).pipe(Flag.atLeast(0)) with no splitting, so a comma-separated value stayed one token and failed validation (invalid IP address: 1.2.3.4,5.6.7.8 / failed to parse IP: 1.2.3.0/24,5.6.7.0/24) where the Go CLI accepts both values.

Both flags now route through the existing parity-reviewed legacyParseStringSliceFlag helper (the same encoding/csv-faithful pipeline already used by sso --domains, postgres-config --config, start, and status), using the established Flag.mapTryCatch pattern. This restores Go-identical semantics: comma-split per occurrence, append across repeats, CSV quoting for embedded commas, no trimming, and parse-time rejection of malformed CSV (which, as in Go, precedes the --experimental gate and handler validation).

Scope check: these are the only two StringSliceVar flags in either command family (--bypass-cidr-checks and --append are booleans, --project-ref is a plain string), so no sibling flags needed the same fix. Flag names, defaults, and help text are unchanged, so docs/go-cli-porting-status.md needs no update.

New coverage: flag-parse unit tests per command (comma split, quoted embedded comma, unset default, malformed-CSV rejection) plus integration scenarios that run the real flag pipeline into the handler and assert the resulting request bodies (comma form, repeated append, mixed, single-value regression, and an invalid post-split element failing before any API call). The new tests fail without the fix.

Review notes (deliberately left open)

The rendered flag-error wording for malformed CSV (Go's invalid argument "..." for "--db-unban-ip" flag: parse error ... wrapper) is asserted nowhere in the repo — a pre-existing gap shared with the sso/postgres-config slice-flag tests; this PR matches the existing pattern rather than solving it unilaterally.
In the pathological double-error case (-o bad plus malformed CSV in one invocation), TS surfaces the CSV parse error while Go's winner depends on argv order; both exit non-zero. No action taken.

Fixes CLI-1983
https://linear.app/supabase/issue/CLI-1983/db-unban-ip-and-db-allow-cidr-dont-comma-split-like-gos-stringslicevar

…gs like pflag (CLI-1983)

Coly010 · 2026-07-28T14:20:23Z

@codex review

chatgpt-codex-connector · 2026-07-28T14:23:20Z

Codex Review: Didn't find any major issues. 🚀

Reviewed commit: 09ab038073

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 09ab038073

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

github-actions · 2026-07-29T09:53:31Z

Supabase CLI preview

npx --yes https://pkg.pr.new/supabase/cli/supabase@774b8be5cfcf19dfec3da5eb0fcff50d226f845c

Preview package for commit 774b8be.

…S to match Go pflag ordering (review: #5975)

Coly010 · 2026-07-29T11:11:01Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3f46eb3b27

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

… to match Go stderr (review: #5975)

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a4caf575ef

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

…encoding/csv (review: #5975)

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b75ed7276c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

kanadgupta

Two asks before merge (but I'll approve so as to unblock this):

The PR description is stale: the "Review notes" still say the rendered flag-error wording is asserted nowhere and that the PR "matches the existing pattern rather than solving it unilaterally" — the later commits (a4caf57, b75ed72) do solve it, assert it, and replace the plain Flag.mapTryCatch pattern with a new builder plus a renderer passthrough. Please update the body so it describes what is actually being merged.
See the inline comment about migrating the seven remaining hand-rolled slice-flag sites to legacyStringSliceFlag in a follow-up.

The unresolved %q-vs-JSON.stringify quoting thread (U+2028/NBSP/DEL residual) is a reasonable accepted divergence given the Bun argv lossy-decode ceiling; fine to leave as-is with a human sign-off on that thread.

kanadgupta · 2026-07-29T17:04:44Z

+ * including `\n`/`\r` escapes in multiline values (same precedent as
+ * `sso.format.ts`).
+ */
+export function legacyStringSliceFlag(name: string, description: string) {


DRY follow-up: this builder now supersedes the hand-rolled Flag.mapTryCatch((raw) => legacyParseStringSliceFlag(raw), err => err.message) pattern still used by the seven other slice-flag sites (sso add/update x4, postgres-config update/delete, start, status). Those sites map only err.message, so their malformed-CSV stderr misses the pflag invalid argument %q for %q flag: ... framing this PR establishes as the Go-parity contract — and the parser rewrite in this file also silently changed their multiline/blank-line semantics (first-record-only, EOF on blank-only values). Keeping the migration out of this PR is the right call for diff size, but it might be worthwhile to file a follow-up to move those consumers onto legacyStringSliceFlag so the near-duplicate pattern (and the parity gap it carries) does not persist.

Agreed, and filed as a follow-up: CLI-2005. Keeping this PR scoped to the two flags in CLI-1983.

kanadgupta · 2026-07-29T17:04:44Z

+
+// 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)", () => {


Optional diff-trim: these four tests (and the mirror file in network-restrictions/update) mostly re-cover behavior that already has stronger homes — comma split / quoted comma / malformed CSV live in the parser suite (legacy-string-slice-flag.unit.test.ts), the integration tests run the same .parse wiring via parseDbUnbanIp, and the experimental-gate integration test asserts the full rendered pflag diagnostic through the real command path. Per the repo testing pyramid (flag parsing coverage belongs in integration by default), the two *.command.unit.test.ts files could be dropped without losing any unique assertion. Fine to keep if you want the flag-level anchor, but they are ~140 lines of duplicate coverage.

Good call — dropped both files in 774b8be. Confirmed the parser suite, the commands' own integration tests, and the experimental-gate integration test cover every scenario these had (comma split, quoted comma, unset default, malformed CSV), matching the repo's testing-pyramid policy that flag-parsing coverage belongs in integration by default.

…ork-restrictions (review: kanadgupta) The four comma-split/quoted-comma/unset-default/malformed-CSV scenarios in remove.command.unit.test.ts and update.command.unit.test.ts are already covered by legacy-string-slice-flag.unit.test.ts (parser suite), the commands' own integration tests (comma form, repeated append, mixed, single-value regression, invalid-post-split-element), and the experimental-gate integration test (full rendered pflag diagnostic through the real command path) — matching the repo's testing-pyramid policy that flag-parsing coverage belongs in integration by default.

fix(cli): comma-split network-bans and network-restrictions slice fla…

09ab038

…gs like pflag (CLI-1983)

Coly010 mentioned this pull request Jul 28, 2026

fix(cli): edge and cosmetic parity sweep from the 2026-07-24 audit (CLI-1990) #5978

Open

Coly010 self-assigned this Jul 29, 2026

Coly010 marked this pull request as ready for review July 29, 2026 09:46

Coly010 requested a review from a team as a code owner July 29, 2026 09:46

chatgpt-codex-connector Bot reviewed Jul 29, 2026

View reviewed changes

Comment thread apps/cli/src/legacy/commands/network-bans/remove/remove.command.ts Outdated

docs(cli): document pre-handler CSV flag-parse failure in SIDE_EFFECT…

3f46eb3

…S to match Go pflag ordering (review: #5975)

chatgpt-codex-connector Bot reviewed Jul 29, 2026

View reviewed changes

Comment thread apps/cli/src/legacy/commands/network-bans/remove/remove.command.ts Outdated

fix(cli): emit pflag's exact malformed-CSV diagnostic for slice flags…

a4caf57

… to match Go stderr (review: #5975)

chatgpt-codex-connector Bot reviewed Jul 29, 2026

View reviewed changes

Comment thread apps/cli/src/legacy/shared/legacy-string-slice-flag.ts Outdated

fix(cli): track physical lines in slice-flag CSV parsing to match Go …

b75ed72

…encoding/csv (review: #5975)

chatgpt-codex-connector Bot reviewed Jul 29, 2026

View reviewed changes

Comment thread apps/cli/src/legacy/shared/legacy-string-slice-flag.ts

kanadgupta approved these changes Jul 30, 2026

View reviewed changes

Uh oh!

Conversation

Coly010 commented Jul 28, 2026

What changed

Review notes (deliberately left open)

Uh oh!

Coly010 commented Jul 28, 2026

Uh oh!

chatgpt-codex-connector Bot commented Jul 28, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

github-actions Bot commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Supabase CLI preview

Uh oh!

Coly010 commented Jul 29, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

kanadgupta left a comment

Choose a reason for hiding this comment

Uh oh!

kanadgupta Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

Coly010 Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

kanadgupta Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

Coly010 Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions Bot commented Jul 29, 2026 •

edited

Loading