Repository navigation
fix(cli): lint config-exposed schemas in local db advisors (CLI-2824) #7379
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.
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
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -54,6 +54,8 @@ export interface DbTomlValues { | |
| */ | ||
| readonly envLookup: (name: EnvOverrideName) => string | undefined; | ||
| readonly apiSchemas: ReadonlyArray<string>; | ||
| /** Schemas the Data API serves; empty when `[api].enabled` is false. */ | ||
| readonly exposedSchemas: ReadonlyArray<string>; | ||
| /** `[db] port`, default 54322 (`packages/config/src/db.ts`). */ | ||
| readonly port: number; | ||
| /** `[db] shadow_port`, default 54320. */ | ||
|
|
@@ -1066,6 +1068,7 @@ const ENV_OVERRIDE_NAMES = [ | |
| "SUPABASE_DB_MIGRATIONS_SCHEMA_PATHS", | ||
| "SUPABASE_API_AUTO_EXPOSE_NEW_TABLES", | ||
| "SUPABASE_API_SCHEMAS", | ||
| "SUPABASE_API_ENABLED", | ||
| "SUPABASE_EXPERIMENTAL_PG_DELTA", | ||
| ] as const; | ||
|
|
||
|
|
@@ -2187,10 +2190,22 @@ const readDbTomlCore = Effect.fnUntraced(function* ( | |
| }); | ||
| } | ||
|
|
||
| const apiEnabled = yield* resolveBoolOrFail( | ||
| "api.enabled", | ||
| apiRaw?.["enabled"], | ||
| true, | ||
| lookup, | ||
| remoteWins("api.enabled") ? undefined : envOverride("SUPABASE_API_ENABLED"), | ||
| ); | ||
|
Comment on lines
+2193
to
+2199
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ⚪ NIT · Malformed SUPABASE_API_ENABLED also fails --linked commands unless a matching remote block overrides api.enabled, but the query documentation describes this failure only for --local and --db-url. Evidence: db-config.toml-read.ts:2193-2199 always resolves api.enabled; :889-892 rejects malformed overrides. db-config.layer.ts:734-736 invokes this reader on the linked branch. query.handler.ts:269-275 and advisors.handler.ts:152-158 invoke that resolver for linked commands. commands/db/query/SIDE_EFFECTS.md:38 limits the documented failure scope to --local / --db-url. Suggested fix: Document that malformed SUPABASE_API_ENABLED can fail linked commands too, including the matching remote-override exception.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed: both |
||
| const exposedSchemas = apiEnabled | ||
| ? [...new Set(apiSchemas.map((schema) => schema.trim()).filter((schema) => schema.length > 0))] | ||
| : []; | ||
|
|
||
| const values: DbTomlValues = { | ||
| projectEnv, | ||
| envLookup: envOverride, | ||
| apiSchemas, | ||
| exposedSchemas, | ||
| port, | ||
| shadowPort, | ||
| password: passwordRaw !== undefined ? expandEnv(passwordRaw, lookup) : DEFAULT_PASSWORD, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 MINOR ·
correctness· source: claudeRemote --db-url advisor runs overwrite the database's exposure setting with local config, which can misclassify exposed schemas or suppress exposure-dependent security findings when the local Data API is disabled. The output does not identify this source.
Evidence: db-config.layer.ts:658-660 reads base local config, and :704-708 attaches its exposedSchemas regardless of isLocal. advisors.handler.ts:70 writes those schemas to pgrst.db_schemas, and :113-115 applies them unconditionally when defined. The embedded rls_disabled_in_public query in advisors.lints-sql.ts:11 filters by that setting.
Suggested fix: Apply local exposure automatically only to local targets. For remote URLs, preserve database-derived exposure or require an explicit exposure override and report its source.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keeping local
[api]exposure for--db-url, including remote URLs. This is the case the ticket targets:--db-urlthrough Supavisor afterdb push, where a backend left withpgrst.db_schemas = ''made the CLI report "No issues found". Limiting exposure to local targets would leave that case broken, and a raw URL does not identify a project whose Data API settings the CLI could read.SIDE_EFFECTS.mdnow documents that--db-urluses local[api](orSUPABASE_API_SCHEMAS/SUPABASE_API_ENABLED) and points to--linkedfor the platform-derived exposure.