Skip to content

fix(cli): comma-split network-bans and network-restrictions slice flags like pflag (CLI-1983) #5975

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
Coly010 wants to merge 1 commit into
base: develop
Choose a base branch
from
+301 −8
Draft

fix(cli): comma-split network-bans and network-restrictions slice flags like pflag (CLI-1983) #5975

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions apps/cli/src/legacy/commands/network-bans/remove/remove.command.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<typeof config>;
Expand Down
61 changes: 61 additions & 0 deletions apps/cli/src/legacy/commands/network-bans/remove/remove.command.unit.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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);
});
});
75 changes: 75 additions & 0 deletions apps/cli/src/legacy/commands/network-bans/remove/remove.integration.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { BunServices } from "@effect/platform-bun";
import { describe, expect, it } from "@effect/vitest";
import { Effect, Exit, Option } from "effect";

Expand All @@ -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<string>) =>
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";
Expand Down Expand Up @@ -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* () {
Expand Down
18 changes: 14 additions & 4 deletions apps/cli/src/legacy/commands/network-restrictions/update/update.command.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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."),
),
Expand Down
62 changes: 62 additions & 0 deletions apps/cli/src/legacy/commands/network-restrictions/update/update.command.unit.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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);
});
});
75 changes: 75 additions & 0 deletions apps/cli/src/legacy/commands/network-restrictions/update/update.integration.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { BunServices } from "@effect/platform-bun";
import {
type V1PatchNetworkRestrictionsOutput,
type V1UpdateNetworkRestrictionsOutput,
Expand All @@ -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<string>) =>
legacyNetworkRestrictionsUpdateDbAllowCidrFlag
.parse({ flags: { "db-allow-cidr": rawValues }, arguments: [] })
.pipe(
Effect.map(([, values]) => values),
Effect.provide(BunServices.layer),
);

// ---------------------------------------------------------------------------
// Fixtures
// ---------------------------------------------------------------------------
Expand Down Expand Up @@ -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)
// -------------------------------------------------------------------------
Expand Down
Loading