From 3e5d6a91b52f3d805415f45f3bdf0257991ac8ec Mon Sep 17 00:00:00 2001 From: 7ttp <117663341+7ttp@users.noreply.github.com> Date: Fri, 23 Oct 2026 14:54:47 +0530 Subject: [PATCH 1/3] fix(cli): stop db advisors --remote minting a login role --- .../commands/db/advisors/advisors.command.ts | 4 +- .../commands/db/advisors/advisors.handler.ts | 20 ++------- .../db/advisors/advisors.integration.test.ts | 43 +++++++------------ 3 files changed, 22 insertions(+), 45 deletions(-) diff --git a/apps/cli/src/commands/db/advisors/advisors.command.ts b/apps/cli/src/commands/db/advisors/advisors.command.ts index d1ad2c6b80..911ea71cc5 100644 --- a/apps/cli/src/commands/db/advisors/advisors.command.ts +++ b/apps/cli/src/commands/db/advisors/advisors.command.ts @@ -2,7 +2,7 @@ import { Command, Flag } from "effect/cli"; import type * as CliCommand from "effect/cli/Command"; import { withPermissions } from "../../../command-internal/command-permissions/command-permissions.annotation.ts"; import { compose, withCondition } from "../../../command-internal/command-permissions/model.ts"; -import { dbConfig } from "../../../command-internal/db-config.layer.ts"; +import { projectRefPicker } from "../../../config/project-ref.layer.ts"; import { linkedProjectCache } from "../../../telemetry/linked-project-cache.layer.ts"; import { withJsonErrorHandling } from "../../../shared/output/json-error-handling.ts"; import { withCommandTelemetry } from "../../../telemetry/command-telemetry.ts"; @@ -62,7 +62,7 @@ export const dbAdvisorsCommand = Command.make("advisors", config).pipe( ], noApiEffectFlags: ["level", "fail-on"], }, - dbConfig, + withCondition(projectRefPicker, { dbTarget: ["linked"] }), withCondition(linkedProjectCache, { dbTarget: ["linked"] }), branchResolution, ), diff --git a/apps/cli/src/commands/db/advisors/advisors.handler.ts b/apps/cli/src/commands/db/advisors/advisors.handler.ts index 53158c599c..6f1202dde8 100644 --- a/apps/cli/src/commands/db/advisors/advisors.handler.ts +++ b/apps/cli/src/commands/db/advisors/advisors.handler.ts @@ -103,32 +103,20 @@ const runLocal = Effect.fn("db.advisors.runLocal")(function* ( return filterAdvisorLints(lints, advisorType, level); }); -/** Gathers lints from the Management API for the linked project. */ +/** Gathers the linked project's lints from the Management API, never from its database. */ const runLinked = Effect.fn("db.advisors.runLinked")(function* ( target: Extract, - dnsResolver: "native" | "https", advisorType: string, level: string, ) { - const resolver = yield* DbConfigResolver; const credentials = yield* CommandCredentials; const linkedProjectCache = yield* LinkedProjectCache; const { ref } = target; - // The linked-project cache is written whenever the project ref was resolved, even when the - // connection resolve below fails. + // The linked-project cache is written whenever the project ref was resolved, even when a later + // step fails. return yield* Effect.gen(function* () { - // The host probe / login-role mint / pooler / IPv6 fallback. The linked lint-gathering path - // ignores the resolved config — this runs purely for the side effects and early-failure - // ordering (before the token gate). - yield* resolver.resolveConnection({ - target, - dnsResolver, - password: Option.none(), - networkBanRemoval: yield* hostedMutationPreapproval, - }); - // The access token is validated against the `sbp_` pattern before calling the API: a // malformed token maps to the invalid-token error, an absent token to missing. const tokenOpt = yield* credentials.getAccessToken.pipe( @@ -217,7 +205,7 @@ const runAdvisors = Effect.fnUntraced(function* ( // Linked goes through the Management API; local and `--db-url` read the database directly. const filtered = target.kind === "linked" - ? yield* runLinked(target, dnsResolver, advisorType, level) + ? yield* runLinked(target, advisorType, level) : yield* runLocal(target, dnsResolver, advisorType, level); yield* Effect.annotateCurrentSpan("advisor.reported_count", filtered.length); diff --git a/apps/cli/src/commands/db/advisors/advisors.integration.test.ts b/apps/cli/src/commands/db/advisors/advisors.integration.test.ts index cd0b1e50b3..e44c89947d 100644 --- a/apps/cli/src/commands/db/advisors/advisors.integration.test.ts +++ b/apps/cli/src/commands/db/advisors/advisors.integration.test.ts @@ -490,7 +490,7 @@ describe("db advisors — linked", () => { }; it.effect( - "mints a login role, fetches both advisors and fills the cache through the real layers", + "fetches both advisors and fills the cache through the real layers without minting a login role", () => { const { layer, api } = setup({ securityLints: [securityLint], @@ -504,10 +504,7 @@ describe("db advisors — linked", () => { yield* dbAdvisors( linkedFlags({ remote: Option.some({ _tag: "Ref", ref: VALID_REF } as const) }), ); - expect(resolverRequests.map(({ method, path }) => `${method} ${path}`)).toEqual([ - `GET /v1/projects/${VALID_REF}/config/database/pooler`, - `POST /v1/projects/${VALID_REF}/cli/login-role`, - ]); + expect(resolverRequests).toEqual([]); expect(api.requests.map(({ method, url }) => `${method} ${url}`)).toEqual([ `GET https://api.supabase.com/v1/projects/${VALID_REF}/advisors/security`, `GET https://api.supabase.com/v1/projects/${VALID_REF}/advisors/performance`, @@ -564,7 +561,7 @@ describe("db advisors — linked", () => { it.effect( "fails on a 403 from the security advisors without asking for the performance ones", () => { - const { layer, api } = setup({ securityStatus: 403 }); + const { layer, api, cache } = setup({ securityStatus: 403 }); return Effect.gen(function* () { const exit = yield* Effect.exit(dbAdvisors(linkedFlags({ type: Option.some("all") }))); expect(Exit.isFailure(exit)).toBe(true); @@ -574,6 +571,7 @@ describe("db advisors — linked", () => { expect(api.requests.map(({ method, url }) => `${method} ${url}`)).toEqual([ `GET https://api.supabase.com/v1/projects/${VALID_REF}/advisors/security`, ]); + expect(cache.cached).toBe(true); assertPermissionDrift({ command: "db advisors", activeFlags: { remote: true, type: "all" }, @@ -631,31 +629,15 @@ describe("db advisors — linked", () => { }, ); - it.effect("resolves the linked DB config before fetching advisors", () => { - // Resolved even though the linked lint-gathering path discards the connection. - const { layer, resolver, api } = setup({ + it.effect("never resolves a database connection for linked advisors", () => { + const { layer, resolver, api, cache } = setup({ + ipv6Error: true, securityLints: [securityLint], }); return Effect.gen(function* () { yield* dbAdvisors(linkedFlags({ type: Option.some("security") })); - expect(resolver.resolveInputs.some((input) => input.target.kind === "linked")).toBe(true); + expect(resolver.resolveInputs).toEqual([]); expect(api.requests.some((r) => r.url.includes("/advisors/security"))).toBe(true); - }).pipe(Effect.provide(layer)); - }); - - it.effect("fails on the linked DB-config error before any advisor API call", () => { - // The DB-config resolve fails before the linked lint-gathering path runs, so the advisors - // API is never reached — but the ref was already loaded and cached unconditionally on the - // error path. - const { layer, api, cache } = setup({ ipv6Error: true }); - return Effect.gen(function* () { - const exit = yield* Effect.exit(dbAdvisors(linkedFlags())); - expect(Exit.isFailure(exit)).toBe(true); - if (Exit.isFailure(exit)) { - const causeText = Cause.pretty(exit.cause); - expect(causeText).toContain("IPv6 is not supported"); - } - expect(api.requests).toHaveLength(0); expect(cache.cached).toBe(true); }).pipe(Effect.provide(layer)); }); @@ -820,7 +802,14 @@ describeDbTargetSelection("db advisors", { layer: ctx.layer, out: ctx.out, run: dbAdvisors(flags(target)), - target: () => ctx.resolver.resolveInputs[0]?.target, + target: () => { + const hostedRef = ctx.api.requests + .map(({ url }) => /\/v1\/projects\/([^/]+)\/advisors\//u.exec(url)?.[1]) + .find((ref) => ref !== undefined); + return hostedRef === undefined + ? ctx.resolver.resolveInputs[0]?.target + : { kind: "linked", ref: hostedRef }; + }, cachedRef: () => ctx.cache.cachedRef, branchLookups: () => ctx.api.requests.filter(({ url }) => url.endsWith("/branches")), apiRequests: () => ctx.api.requests, From 3796394e0462f5200921779730a6f19c15f19863 Mon Sep 17 00:00:00 2001 From: 7ttp <117663341+7ttp@users.noreply.github.com> Date: Mon, 26 Oct 2026 17:32:08 +0530 Subject: [PATCH 2/3] nit --- apps/cli/src/commands/db/advisors/SIDE_EFFECTS.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/cli/src/commands/db/advisors/SIDE_EFFECTS.md b/apps/cli/src/commands/db/advisors/SIDE_EFFECTS.md index 84aa5b959d..60b500fa69 100644 --- a/apps/cli/src/commands/db/advisors/SIDE_EFFECTS.md +++ b/apps/cli/src/commands/db/advisors/SIDE_EFFECTS.md @@ -13,7 +13,7 @@ Management API. | `/supabase/config.toml` | TOML | local / `--remote=` — to resolve the DB connection config | | `~/.supabase/access-token` | plain text | a remote project or branch only, when `SUPABASE_ACCESS_TOKEN` unset (keyring → file) | | `/supabase/.temp/project-ref` | plain text | bare `--remote` — to resolve the project ref, skipped when `SUPABASE_PROJECT_ID` is set; with no selector — only to name the project in the `Target:` hint | -| `PGSSL*` PEM, `pg_service.conf` | PEM, libpq | remote project: `pg_service.conf` if `PGSERVICE` is set; PEM files only on the pooler temp-role probe; see `db push` | +| `PGSSL*` PEM, `pg_service.conf` | PEM, libpq | `--remote=`, or local on the experimental stack (shell env only), when set; see `db push` | ## Files Written @@ -52,7 +52,7 @@ One connection. Within one transaction: `BEGIN` → `set local search_path = ''` | `SUPABASE_PROJECT_ID` | linked project ref override, and the parent that `--remote=` is resolved against (superseded by `--remote=`) | no | | `SUPABASE_PROFILE` | API profile (built-in name or YAML path) | no | | `PGHOST` / `PGPORT` / … | connection overrides (local / `--remote=`) | no | -| `PG*` libpq | remote project: validated; used only by the temp-role probe; see `db push` | no | +| `PG*` libpq | `--remote=` TLS, timeout, service, and local on the experimental stack (shell env only); see `db push` | no | The API base URL is derived from `SUPABASE_PROFILE`; `SUPABASE_API_URL` is **not** honored (see `command-settings.layer.unit.test.ts`). From ff833eb301ae907e921249b8ebfdb9483b6bc28f Mon Sep 17 00:00:00 2001 From: 7ttp <117663341+7ttp@users.noreply.github.com> Date: Mon, 26 Oct 2026 17:54:39 +0530 Subject: [PATCH 3/3] nit --- apps/cli/src/commands/db/advisors/SIDE_EFFECTS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/cli/src/commands/db/advisors/SIDE_EFFECTS.md b/apps/cli/src/commands/db/advisors/SIDE_EFFECTS.md index 60b500fa69..94c4f93204 100644 --- a/apps/cli/src/commands/db/advisors/SIDE_EFFECTS.md +++ b/apps/cli/src/commands/db/advisors/SIDE_EFFECTS.md @@ -13,7 +13,7 @@ Management API. | `/supabase/config.toml` | TOML | local / `--remote=` — to resolve the DB connection config | | `~/.supabase/access-token` | plain text | a remote project or branch only, when `SUPABASE_ACCESS_TOKEN` unset (keyring → file) | | `/supabase/.temp/project-ref` | plain text | bare `--remote` — to resolve the project ref, skipped when `SUPABASE_PROJECT_ID` is set; with no selector — only to name the project in the `Target:` hint | -| `PGSSL*` PEM, `pg_service.conf` | PEM, libpq | `--remote=`, or local on the experimental stack (shell env only), when set; see `db push` | +| `PGSSL*` PEM, `pg_service.conf` | PEM, libpq | `--remote=` or local on the experimental stack (shell env only), when set; client cert and key never for the local stack database; see `db push` | ## Files Written