Skip to content

Commit 8ce5cfa

Browse files
avalletecursoragent
andcommitted
fix(cli): reject sub-second gen types timeouts that disable the bound
`--query-timeout 1ms` rounded to 0 and silently dropped both timeout guards. Parse through the shared Go duration helper, refuse rounded-to-0 except explicit disable, and document native TS/Python shape diffs. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent abdb6aa commit 8ce5cfa

5 files changed

Lines changed: 136 additions & 128 deletions

File tree

apps/cli/src/legacy/commands/gen/types/SIDE_EFFECTS.md

Lines changed: 27 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -54,11 +54,13 @@ remote targets and the `--dns-resolver` DoH mode), runs the package's
5454
introspection queries against `pg_catalog`/`information_schema`, and renders the
5555
requested language locally. `--query-timeout` is applied as the session's
5656
`statement_timeout` (the flag wins over a DSN `statement_timeout`) and as a
57-
client-side bound around `introspect()`; `0` disables both. When the connection
58-
string carries no explicit `connect_timeout`, a positive `--query-timeout` is
59-
also used as the connect timeout — `0` leaves the driver's default (10s remote,
60-
2s local). `--local` connects to the host-mapped database port from
61-
`supabase/config.toml` (`db.port`).
57+
client-side bound around `introspect()`; `0` / `0s` / `0ms` disable both. A
58+
positive duration that rounds below 1s (`1ms`, `400ms`) is rejected rather than
59+
treated as disable. When the connection string carries no explicit
60+
`connect_timeout`, a positive `--query-timeout` is also used as the connect
61+
timeout — `0` leaves the driver's default (10s remote, 2s local). `--local`
62+
connects to the host-mapped database port from `supabase/config.toml`
63+
(`db.port`).
6264

6365
For a remote target whose DSN carries no explicit `sslmode`, a raw TCP
6466
`SSLRequest` probe (the shared pg-delta probe, default 10s timeout) is opened
@@ -171,21 +173,32 @@ go`/`--lang swift`/`--lang python` — the defaults-only claim above holds only
171173
intentionally). `--local` still forces v9 compat when the local PostgREST image tag
172174
contains `v9`.
173175
- `--query-timeout` sets the maximum timeout for the introspection queries (default
174-
15s). It is mutually exclusive with an _explicit_ `--linked`/`--project-id`; on
175-
the implicit linked fallback it is accepted, and honored for
176-
`--lang go`/`--lang swift`/`--lang python` (silently unused only for the implicit
177-
linked TypeScript case, since that path never opens a database connection).
176+
15s). Parsed as a Go `time.Duration` (bare `0` is valid and disables). A
177+
positive value that rounds to 0 seconds is rejected — use `0` to disable, or
178+
at least `500ms`. It is mutually exclusive with an _explicit_ `--linked`/
179+
`--project-id`; on the implicit linked fallback it is accepted, and honored
180+
for `--lang go`/`--lang swift`/`--lang python` (silently unused only for the
181+
implicit linked TypeScript case, since that path never opens a database
182+
connection).
178183
- `--db-url` is parsed by the shared connection resolver (libpq keywords, `PG*` env
179184
fallbacks, `options=reference=<ref>` pooler tenants, `sslmode`), matching every
180185
other `--db-url` command. An absent dbname follows libpq (`PGDATABASE`, then the
181186
connection user) rather than forcing `postgres`.
182187
- The legacy positional language argument (`supabase gen types typescript`) is still accepted;
183188
any other positional language requires an explicit `--lang` flag.
184-
- Go and Python output now lists entities in the canonical sorted order
189+
- Go output lists entities in the canonical sorted order
185190
(`sortGeneratorMetadata`) instead of pg-meta's environment-dependent SQL row
186-
order; the rendered content is otherwise identical (Swift verified
187-
byte-identical — its template sorts internally). TypeScript is formatted by
188-
oxfmt (postgrest-typegen ≥ 0.2.0) instead of pg-meta's prettier: content is
189-
identical, with minor whitespace differences in how long union types wrap.
191+
order; the rendered structs are otherwise the same. Swift is byte-identical
192+
(its template sorts internally). Python is not a drop-in of pg-meta: it emits
193+
`JsonValue` instead of pydantic `Json`, and imports `NotRequired`/`TypeAlias`
194+
from `typing` rather than `typing_extensions`. TypeScript is formatted by
195+
oxfmt (postgrest-typegen ≥ 0.2.0) instead of pg-meta's prettier, and native
196+
generation differs in shape from both pg-meta and the Management API
197+
`--linked`/`--project-id` TypeScript path: empty-arg RPCs are
198+
`Args: Record<PropertyKey, never>` rather than `never`; some `Json` fields
199+
become `NonNullable<Json>`; generated `path_tokens` is `never` rather than
200+
`string[] | null`. The Management API payload also injects
201+
`__InternalSupabase.PostgrestVersion`, which native `--local`/`--db-url`
202+
generation omits.
190203
- The linked-project telemetry cache is written only when a project ref is resolved
191204
(`--linked`/`--project-id`/fallback) — it's skipped when no ref is available.

apps/cli/src/legacy/commands/gen/types/types.handler.ts

Lines changed: 7 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { loadCliConfig } from "@supabase/config/internal";
22
import { ChildProcessSpawner } from "effect/unstable/process";
3-
import { Effect, FileSystem, Option, Path, Stdio, Stream } from "effect";
3+
import { Effect, FileSystem, Option, Path, Stdio } from "effect";
44
import { LegacyDnsResolverFlag } from "../../../../shared/legacy/global-flags.ts";
55
import { Output } from "../../../../shared/output/output.service.ts";
66
import {
@@ -15,7 +15,7 @@ import {
1515
LegacyProjectRefResolver,
1616
PROJECT_NOT_LINKED_MESSAGE,
1717
} from "../../../config/legacy-project-ref.service.ts";
18-
import { spawnContainerCli } from "../../../shared/legacy-container-cli.ts";
18+
import { legacyCollectText, spawnContainerCli } from "../../../shared/legacy-container-cli.ts";
1919
import { legacyIsIPv6ConnectivityErrorCause } from "../../../shared/legacy-connect-errors.ts";
2020
import { mapLegacyHttpError } from "../../../shared/legacy-http-errors.ts";
2121
import { LegacyDbConfigResolver } from "../../../shared/legacy-db-config.service.ts";
@@ -123,38 +123,10 @@ const GEN_TYPES_SCAN_SPEC = {
123123
valueFlagShorthands: new Map([["s", "schema"], ...PERSISTENT_VALUE_FLAG_SHORTHANDS]),
124124
} as const;
125125

126-
function collectByteStream(stream: Stream.Stream<Uint8Array, unknown>) {
127-
const decoder = new TextDecoder();
128-
return Stream.runFold(
129-
stream,
130-
() => "",
131-
(text, chunk) => text + decoder.decode(chunk, { stream: true }),
132-
).pipe(Effect.map((text) => text + decoder.decode()));
133-
}
134-
135-
// Keep these two sets in sync with the value-bearing flags on the root command
136-
// (shared/legacy/global-flags.ts) and the `gen types` command (types.command.ts).
137-
// They let `findLegacyPositionalLanguage` skip a flag's value so it is not
138-
// mistaken for the legacy positional language argument (e.g. `gen types typescript`).
139-
const LONG_FLAGS_WITH_VALUES = new Set([
140-
"db-url",
141-
"project-id",
142-
"lang",
143-
"schema",
144-
"swift-access-control",
145-
"query-timeout",
146-
"profile",
147-
"workdir",
148-
"network-id",
149-
"dns-resolver",
150-
"output",
151-
"output-format",
152-
"log-level",
153-
"completions",
154-
"agent",
155-
]);
156-
157-
const SHORT_FLAGS_WITH_VALUES = new Set(["s", "o"]);
126+
// Positional `typescript` scanning must skip the same value tokens pflag
127+
// consumes — derive from the scan spec so a new value flag cannot drift.
128+
const LONG_FLAGS_WITH_VALUES = GEN_TYPES_SCAN_SPEC.valueFlagNames;
129+
const SHORT_FLAGS_WITH_VALUES = new Set(GEN_TYPES_SCAN_SPEC.valueFlagShorthands.keys());
158130

159131
function findLegacyPositionalLanguage(rawArgs: ReadonlyArray<string>): Option.Option<string> {
160132
const commandIndex = rawArgs.findIndex(
@@ -409,7 +381,7 @@ export const legacyGenTypes = Effect.fn("legacy.gen.types")(function* (flags: Le
409381
);
410382
const [exitCode, stderr] = yield* Effect.all([
411383
child.exitCode.pipe(Effect.map(Number)),
412-
collectByteStream(child.stderr),
384+
legacyCollectText(child.stderr),
413385
]);
414386
if (exitCode !== 0) {
415387
const message = stderr.trim();

apps/cli/src/legacy/commands/gen/types/types.integration.test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -806,6 +806,28 @@ describe("legacy gen types", () => {
806806
});
807807
});
808808

809+
it.live("rejects a sub-second --query-timeout that would disable the bound", () => {
810+
const { layer, telemetry } = setup({
811+
args: ["gen", "types", "--db-url", "postgresql://postgres@127.0.0.1:5432/postgres"],
812+
});
813+
814+
return Effect.gen(function* () {
815+
const exit = yield* legacyGenTypes(
816+
defaultFlags({
817+
dbUrl: Option.some("postgresql://postgres@127.0.0.1:5432/postgres"),
818+
queryTimeout: "1ms",
819+
}),
820+
).pipe(Effect.provide(layer), Effect.exit);
821+
822+
expect(Exit.isFailure(exit)).toBe(true);
823+
if (Exit.isFailure(exit)) {
824+
expect(String(exit.cause)).toContain('invalid duration "1ms"');
825+
expect(String(exit.cause)).toContain("use 0 to disable, or at least 500ms");
826+
}
827+
expect(telemetry.flushed).toBe(false);
828+
});
829+
});
830+
809831
it.live("silently ignores --query-timeout for implicit linked TypeScript generation", () => {
810832
const { layer, out, api } = setup({
811833
args: ["gen", "types", "--query-timeout", "20s"],

apps/cli/src/legacy/commands/gen/types/types.shared.ts

Lines changed: 26 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -1,81 +1,47 @@
11
import { Effect } from "effect";
22

33
import type { LegacyPgConnInput } from "../../../shared/legacy-db-connection.service.ts";
4+
import { legacyParseGoDuration } from "../../../shared/legacy-go-duration.ts";
45
import { LegacyInvalidGenTypesDurationError } from "./types.errors.ts";
56

67
// The local Docker container id is hoisted to `legacy/shared` so the declarative
78
// seam can derive the same `supabase_db_<id>` name when checking the local stack.
89
export { localDbContainerId } from "../../../shared/legacy-docker-ids.ts";
910

10-
const DURATION_UNITS_TO_MILLIS = {
11-
ns: 1 / 1_000_000,
12-
us: 1 / 1_000,
13-
"\u00b5s": 1 / 1_000,
14-
"\u03bcs": 1 / 1_000,
15-
ms: 1,
16-
s: 1_000,
17-
m: 60_000,
18-
h: 3_600_000,
19-
} as const;
20-
21-
const DURATION_PART_PATTERN = new RegExp(
22-
String.raw`([+-]?(?:\d+\.?\d*|\.\d+))(ns|us|\u00b5s|\u03bcs|ms|s|m|h)`,
23-
"g",
24-
);
25-
2611
export function defaultSchemas(extraSchemas: ReadonlyArray<string> = []) {
2712
return [...new Set(["public", ...extraSchemas])];
2813
}
2914

15+
function invalidQueryTimeout(raw: string, detail?: string) {
16+
return new LegacyInvalidGenTypesDurationError({
17+
message:
18+
detail === undefined
19+
? `invalid duration ${JSON.stringify(raw)}`
20+
: `invalid duration ${JSON.stringify(raw)}: ${detail}`,
21+
});
22+
}
23+
3024
export function parseQueryTimeoutSeconds(
3125
raw: string,
3226
): Effect.Effect<number, LegacyInvalidGenTypesDurationError> {
33-
return Effect.gen(function* () {
34-
const input = raw.trim();
35-
if (input.length === 0) {
36-
return yield* Effect.fail(
37-
new LegacyInvalidGenTypesDurationError({
38-
message: `invalid duration ${JSON.stringify(raw)}`,
39-
}),
40-
);
41-
}
42-
43-
let totalMillis = 0;
44-
let consumed = 0;
45-
DURATION_PART_PATTERN.lastIndex = 0;
46-
for (const match of input.matchAll(DURATION_PART_PATTERN)) {
47-
const [token, rawNumber, rawUnit] = match;
48-
if (
49-
token === undefined ||
50-
rawNumber === undefined ||
51-
rawUnit === undefined ||
52-
match.index === undefined
53-
) {
54-
continue;
27+
return Effect.try({
28+
try: () => legacyParseGoDuration(raw),
29+
catch: () => invalidQueryTimeout(raw),
30+
}).pipe(
31+
Effect.flatMap((nanos) => {
32+
if (nanos < 0) {
33+
return Effect.fail(invalidQueryTimeout(raw));
5534
}
56-
if (match.index !== consumed) {
57-
return yield* Effect.fail(
58-
new LegacyInvalidGenTypesDurationError({
59-
message: `invalid duration ${JSON.stringify(raw)}`,
60-
}),
61-
);
35+
// Whole-second `statement_timeout` / client bound. `0` is the disable
36+
// sentinel — a positive duration that rounds into it would silently
37+
// drop the user's requested cap.
38+
const seconds = Math.round(nanos / 1_000_000_000);
39+
if (seconds === 0 && nanos !== 0) {
40+
return Effect.fail(invalidQueryTimeout(raw, "use 0 to disable, or at least 500ms"));
6241
}
63-
const amount = Number.parseFloat(rawNumber);
64-
const unitMillis = DURATION_UNITS_TO_MILLIS[rawUnit as keyof typeof DURATION_UNITS_TO_MILLIS];
65-
totalMillis += amount * unitMillis;
66-
consumed += token.length;
67-
}
68-
69-
if (!Number.isFinite(totalMillis) || consumed !== input.length || totalMillis < 0) {
70-
return yield* Effect.fail(
71-
new LegacyInvalidGenTypesDurationError({
72-
message: `invalid duration ${JSON.stringify(raw)}`,
73-
}),
74-
);
75-
}
76-
77-
return Math.round(totalMillis / 1_000);
78-
});
42+
return Effect.succeed(seconds);
43+
}),
44+
);
7945
}
8046

8147
export function localDbPassword() {

apps/cli/src/legacy/commands/gen/types/types.unit.test.ts

Lines changed: 54 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
import { createRequire } from "node:module";
2+
import { readFileSync } from "node:fs";
3+
import { fileURLToPath } from "node:url";
14
import { describe, expect, it } from "@effect/vitest";
25
import { Effect, Exit } from "effect";
36
import {
@@ -8,6 +11,13 @@ import {
811
parseQueryTimeoutSeconds,
912
} from "./types.shared.ts";
1013

14+
function expectInvalidDuration(exit: Exit.Exit<unknown, unknown>, raw: string) {
15+
expect(Exit.isFailure(exit)).toBe(true);
16+
if (Exit.isFailure(exit)) {
17+
expect(String(exit.cause)).toContain(`invalid duration ${JSON.stringify(raw)}`);
18+
}
19+
}
20+
1121
const BASE_CONN = {
1222
host: "db.example.com",
1323
port: 5432,
@@ -44,52 +54,55 @@ describe("parseQueryTimeoutSeconds", () => {
4454
}),
4555
);
4656

47-
it.effect("rounds sub-second durations to whole seconds", () =>
57+
it.effect("accepts Go's bare 0 as disable", () =>
58+
Effect.gen(function* () {
59+
expect(yield* parseQueryTimeoutSeconds("0")).toBe(0);
60+
expect(yield* parseQueryTimeoutSeconds("0s")).toBe(0);
61+
expect(yield* parseQueryTimeoutSeconds("0ms")).toBe(0);
62+
}),
63+
);
64+
65+
it.effect("rounds 500ms up to a still-applied 1s bound", () =>
4866
Effect.gen(function* () {
4967
expect(yield* parseQueryTimeoutSeconds("500ms")).toBe(1);
50-
expect(yield* parseQueryTimeoutSeconds("400ms")).toBe(0);
68+
}),
69+
);
70+
71+
it.effect("rejects a positive duration that would disable the timeout", () =>
72+
Effect.gen(function* () {
73+
for (const raw of ["1ms", "400ms", `15${"µ"}s`, `15${"μ"}s`]) {
74+
expectInvalidDuration(yield* parseQueryTimeoutSeconds(raw).pipe(Effect.exit), raw);
75+
}
5176
}),
5277
);
5378

5479
it.effect("rejects an empty duration", () =>
5580
Effect.gen(function* () {
56-
const exit = yield* parseQueryTimeoutSeconds(" ").pipe(Effect.exit);
57-
expect(Exit.isFailure(exit)).toBe(true);
81+
expectInvalidDuration(yield* parseQueryTimeoutSeconds(" ").pipe(Effect.exit), " ");
5882
}),
5983
);
6084

6185
it.effect("rejects a duration with a leading non-duration prefix", () =>
6286
Effect.gen(function* () {
63-
const exit = yield* parseQueryTimeoutSeconds("x15s").pipe(Effect.exit);
64-
expect(Exit.isFailure(exit)).toBe(true);
87+
expectInvalidDuration(yield* parseQueryTimeoutSeconds("x15s").pipe(Effect.exit), "x15s");
6588
}),
6689
);
6790

6891
it.effect("rejects a duration with trailing junk", () =>
6992
Effect.gen(function* () {
70-
const exit = yield* parseQueryTimeoutSeconds("15s30").pipe(Effect.exit);
71-
expect(Exit.isFailure(exit)).toBe(true);
93+
expectInvalidDuration(yield* parseQueryTimeoutSeconds("15s30").pipe(Effect.exit), "15s30");
7294
}),
7395
);
7496

7597
it.effect("rejects a string with no recognizable units", () =>
7698
Effect.gen(function* () {
77-
const exit = yield* parseQueryTimeoutSeconds("abc").pipe(Effect.exit);
78-
expect(Exit.isFailure(exit)).toBe(true);
99+
expectInvalidDuration(yield* parseQueryTimeoutSeconds("abc").pipe(Effect.exit), "abc");
79100
}),
80101
);
81102

82103
it.effect("rejects a negative duration", () =>
83104
Effect.gen(function* () {
84-
const exit = yield* parseQueryTimeoutSeconds("-5s").pipe(Effect.exit);
85-
expect(Exit.isFailure(exit)).toBe(true);
86-
}),
87-
);
88-
89-
it.effect("accepts Go-style microsecond duration aliases", () =>
90-
Effect.gen(function* () {
91-
expect(yield* parseQueryTimeoutSeconds(`15${"µ"}s`)).toBe(0);
92-
expect(yield* parseQueryTimeoutSeconds(`15${"μ"}s`)).toBe(0);
105+
expectInvalidDuration(yield* parseQueryTimeoutSeconds("-5s").pipe(Effect.exit), "-5s");
93106
}),
94107
);
95108
});
@@ -148,3 +161,25 @@ describe("schema and password helpers", () => {
148161
expect(withEnv("SUPABASE_DB_PASSWORD", "secret", () => localDbPassword())).toBe("secret");
149162
});
150163
});
164+
165+
describe("oxfmt binding pin", () => {
166+
it("stays on the oxfmt version postgrest-typegen resolves", () => {
167+
const cliPackageJson = fileURLToPath(new URL("../../../../../package.json", import.meta.url));
168+
const cliPkg = JSON.parse(readFileSync(cliPackageJson, "utf8")) as {
169+
readonly devDependencies: Readonly<Record<string, string>>;
170+
};
171+
const typegenReq = createRequire(cliPackageJson);
172+
const typegenPkg = typegenReq("@supabase/postgrest-typegen/package.json") as {
173+
readonly dependencies: Readonly<Record<string, string>>;
174+
};
175+
const oxfmtVersion = typegenPkg.dependencies["oxfmt"];
176+
expect(oxfmtVersion).toEqual(expect.stringMatching(/^\d+\.\d+\.\d+/));
177+
const bindingPins = Object.entries(cliPkg.devDependencies).filter(([name]) =>
178+
name.startsWith("@oxfmt/binding-"),
179+
);
180+
expect(bindingPins.length).toBeGreaterThan(0);
181+
for (const [, version] of bindingPins) {
182+
expect(version).toBe(oxfmtVersion);
183+
}
184+
});
185+
});

0 commit comments

Comments
 (0)