Skip to content

Commit 23a598e

Browse files
committed
fix(cli): match Go's config validation for inspect report rules
Three parity gaps in how custom `[experimental.inspect.rules]` are loaded, all grounded in Go's `UnmarshalExact` (config.go:579) under viper's default `WeaklyTypedInput: true` and its `PersistentPreRunE` config load: - Validate the rule config BEFORE connecting or writing CSVs. Go loads/validates the whole config in PersistentPreRunE (via ParseDatabaseConfig → LoadConfig), so a malformed config aborts with zero side effects. The reader now runs at the top of the handler, before mkdir/connect/COPY (rules are still applied in the summary step). (review: validate-before-COPY) - Reject a non-array `rules` value the way Go's decodeSlice does under weak typing: a scalar (`rules = "foo"`) aborts ("expected a map or struct"); a single inline table is wrapped into one rule; an empty table yields no custom rules. Previously any non-array silently fell through to the defaults. (review: non-array rules) - Reject unknown/misspelled keys in a rule table. UnmarshalExact sets mapstructure ErrorUnused per-struct, so `fails = "bad"` aborts the load; the reader now fails with LegacyDbConfigLoadError instead of silently ignoring extra keys. (review: unknown keys) Adds unit coverage (unknown key, single inline table, scalar rules) and an integration test asserting a malformed config aborts before any connection or CSV.
1 parent 32524fe commit 23a598e

4 files changed

Lines changed: 140 additions & 9 deletions

File tree

apps/cli/src/legacy/commands/inspect/report/report.config.ts

Lines changed: 45 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -85,27 +85,65 @@ export const legacyReadInspectRules = Effect.fnUntraced(function* (
8585

8686
const inspect = asRecord(asRecord(doc?.["experimental"])?.["inspect"]);
8787
const rawRules = inspect?.["rules"];
88-
if (!Array.isArray(rawRules)) return [] as ReadonlyArray<LegacyInspectRule>;
88+
89+
// Normalize `rules` into the list of entries to decode, mirroring Go's
90+
// `decodeSlice` under viper's `WeaklyTypedInput: true` (which is NOT disabled in
91+
// `config.Load`, `apps/cli-go/pkg/config/config.go:579-584`):
92+
// - absent → no custom rules (defaults apply)
93+
// - array-of-tables → decode each element as a rule
94+
// - a single table → weak-typing wraps it into a 1-element slice → one rule
95+
// - an EMPTY table → wraps into an empty slice → no custom rules (defaults)
96+
// - a scalar (string/number/…) → wrapped into `[scalar]`, then decoding a scalar
97+
// into a rule struct aborts ("expected a map or struct") — surfaced below.
98+
let entries: ReadonlyArray<unknown>;
99+
if (rawRules === undefined) {
100+
return [] as ReadonlyArray<LegacyInspectRule>;
101+
} else if (Array.isArray(rawRules)) {
102+
entries = rawRules;
103+
} else {
104+
const asMap = asRecord(rawRules);
105+
if (asMap !== undefined && Object.keys(asMap).length === 0) {
106+
return [] as ReadonlyArray<LegacyInspectRule>;
107+
}
108+
entries = [rawRules];
109+
}
110+
if (entries.length === 0) return [] as ReadonlyArray<LegacyInspectRule>;
111+
112+
const RULE_FIELDS = ["query", "name", "pass", "fail"] as const;
89113

90114
// Resolve `env(VAR)` against the shell env first, then the project `.env` files.
91115
const projectEnv = yield* legacyLoadProjectEnv(fs, path, workdir);
92116
const lookup = (name: string): string | undefined => process.env[name] ?? projectEnv[name];
93117

94118
const rules: Array<LegacyInspectRule> = [];
95-
for (let index = 0; index < rawRules.length; index++) {
96-
const record = asRecord(rawRules[index]);
97-
// A non-table array entry (e.g. `rules = ["foo"]`) is rejected by Go: mapstructure
98-
// routes the element into `decodeStruct`, whose default branch returns "expected a
99-
// map or struct", aborting `config.Load`. Match that rather than silently skipping.
119+
for (let index = 0; index < entries.length; index++) {
120+
const record = asRecord(entries[index]);
121+
// A non-table entry (e.g. `rules = ["foo"]` or `rules = "foo"`) is rejected by Go:
122+
// mapstructure routes it into `decodeStruct`, whose default branch returns
123+
// "expected a map or struct", aborting `config.Load`. Match that, not silent skip.
100124
if (record === undefined) {
101125
return yield* Effect.fail(
102126
new LegacyDbConfigLoadError({
103127
message: `failed to load config: experimental.inspect.rules[${index}] expected a map or struct`,
104128
}),
105129
);
106130
}
131+
// Go decodes with `UnmarshalExact` (`config.go:579`), which sets mapstructure's
132+
// `ErrorUnused` per-struct: an unknown/misspelled key in a rule table (e.g.
133+
// `fails = "bad"`) aborts the whole config load. The `rule` struct has no
134+
// `,remain` field, so there is no escape hatch.
135+
const unknownKeys = Object.keys(record).filter(
136+
(key) => !(RULE_FIELDS as ReadonlyArray<string>).includes(key),
137+
);
138+
if (unknownKeys.length > 0) {
139+
return yield* Effect.fail(
140+
new LegacyDbConfigLoadError({
141+
message: `failed to load config: experimental.inspect.rules[${index}] has invalid keys: ${unknownKeys.join(", ")}`,
142+
}),
143+
);
144+
}
107145
const fields: Record<string, string> = {};
108-
for (const field of ["query", "name", "pass", "fail"] as const) {
146+
for (const field of RULE_FIELDS) {
109147
const coerced = coerceRuleField(record[field]);
110148
// A non-coercible field type (nested table/array/datetime) aborts in Go too.
111149
if (coerced === undefined) {

apps/cli/src/legacy/commands/inspect/report/report.config.unit.test.ts

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,60 @@ describe("legacyReadInspectRules", () => {
121121
}),
122122
);
123123

124+
it.effect("rejects unknown keys in a rule table (Go's UnmarshalExact ErrorUnused)", () =>
125+
Effect.gen(function* () {
126+
const exit = yield* Effect.exit(
127+
readRules(
128+
makeWorkdir(
129+
[
130+
"[[experimental.inspect.rules]]",
131+
'query = "SELECT 1"',
132+
'name = "r"',
133+
'pass = "ok"',
134+
'fail = "bad"',
135+
'fails = "typo"',
136+
"",
137+
].join("\n"),
138+
),
139+
),
140+
);
141+
expect(exit._tag).toBe("Failure");
142+
if (exit._tag === "Failure") {
143+
expect(JSON.stringify(exit.cause)).toContain("invalid keys: fails");
144+
}
145+
}),
146+
);
147+
148+
it.effect("accepts a single inline rules table as one rule (Go weak-typing wrap)", () =>
149+
Effect.gen(function* () {
150+
const rules = yield* readRules(
151+
makeWorkdir(
152+
[
153+
"[experimental.inspect.rules]",
154+
'query = "SELECT 1"',
155+
'name = "solo"',
156+
'pass = "ok"',
157+
'fail = "bad"',
158+
"",
159+
].join("\n"),
160+
),
161+
);
162+
expect(rules).toEqual([{ query: "SELECT 1", name: "solo", pass: "ok", fail: "bad" }]);
163+
}),
164+
);
165+
166+
it.effect("fails when rules is a scalar string (Go aborts)", () =>
167+
Effect.gen(function* () {
168+
const exit = yield* Effect.exit(
169+
readRules(makeWorkdir('[experimental.inspect]\nrules = "oops"\n')),
170+
);
171+
expect(exit._tag).toBe("Failure");
172+
if (exit._tag === "Failure") {
173+
expect(JSON.stringify(exit.cause)).toContain("expected a map or struct");
174+
}
175+
}),
176+
);
177+
124178
it.effect("fails when a rule field is a non-coercible type (nested table)", () =>
125179
Effect.gen(function* () {
126180
const exit = yield* Effect.exit(

apps/cli/src/legacy/commands/inspect/report/report.handler.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,13 @@ const legacyRunInspectReport = Effect.fnUntraced(function* (
8585
);
8686
}
8787

88+
// Read + validate the custom `[experimental.inspect.rules]` BEFORE any DB work.
89+
// Go loads and validates the whole config in `PersistentPreRunE` (via
90+
// `flags.ParseDatabaseConfig` → `LoadConfig`, `cmd/root.go:118`), so a malformed
91+
// `inspect.rules` config aborts before connecting or writing any CSV files. They
92+
// are applied later (in `printSummary`), but validated here for parity.
93+
const configRules = yield* legacyReadInspectRules(fs, path, cliConfig.workdir);
94+
8895
// Go's `--linked` defaults to true, so absence of the others resolves to linked.
8996
const linked = flags.linked || (Option.isNone(flags.dbUrl) && !flags.local);
9097
const cfg = yield* resolver.resolve({
@@ -146,8 +153,8 @@ const legacyRunInspectReport = Effect.fnUntraced(function* (
146153
yield* output.raw(`Reports saved to ${legacyBold(outDir, tty.stdoutIsTty)}\n`, "stderr");
147154
}
148155

149-
// Custom `[experimental.inspect.rules]` replace the 7 defaults when present.
150-
const configRules = yield* legacyReadInspectRules(fs, path, cliConfig.workdir);
156+
// Custom `[experimental.inspect.rules]` (read + validated up front) replace the 7
157+
// defaults when present.
151158
const rules = configRules.length > 0 ? configRules : LEGACY_DEFAULT_INSPECT_RULES;
152159
if (configRules.length === 0 && isText) {
153160
yield* output.raw("Loading default rules...\n", "stderr");

apps/cli/src/legacy/commands/inspect/report/report.integration.test.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,38 @@ describe("legacy inspect report", () => {
340340
}).pipe(Effect.provide(layer));
341341
});
342342

343+
it.live("aborts on a malformed config.toml before connecting or writing any CSV", () => {
344+
const base = tempDir("supabase-report-out-");
345+
const workdir = tempDir("supabase-report-workdir-");
346+
mkdirSync(join(workdir, "supabase"), { recursive: true });
347+
// An invalid rule config (unknown key) — Go loads config in PersistentPreRun, so
348+
// it must abort before the DB connection and before any CSV files are written.
349+
writeFileSync(
350+
join(workdir, "supabase", "config.toml"),
351+
[
352+
"[[experimental.inspect.rules]]",
353+
'query = "SELECT 1"',
354+
'name = "r"',
355+
'pass = "ok"',
356+
'fail = "bad"',
357+
'typo = "x"',
358+
"",
359+
].join("\n"),
360+
);
361+
const { layer, connection } = setupLegacyReport({ workdir });
362+
return Effect.gen(function* () {
363+
const exit = yield* Effect.exit(legacyInspectReport(flags({ outputDir: base })));
364+
expect(Exit.isFailure(exit)).toBe(true);
365+
if (Exit.isFailure(exit)) {
366+
expect(JSON.stringify(exit.cause)).toContain("invalid keys: typo");
367+
}
368+
// No connection and no dated output folder — config validation ran first,
369+
// before mkdir / connect / COPY (base itself is the pre-created temp dir).
370+
expect(connection.copiedSql.length).toBe(0);
371+
expect(readdirSync(base).length).toBe(0);
372+
}).pipe(Effect.provide(layer));
373+
});
374+
343375
it.live("emits a structured result and writes CSVs but no table in json mode", () => {
344376
const base = tempDir("supabase-report-out-");
345377
const { layer, out } = setupLegacyReport({ format: "json", csvs: DEFAULT_RULE_CSVS });

0 commit comments

Comments
 (0)