Repository navigation
Conversation
Every leaf command carries a withTargetPolicy annotation with its target group and hosted-mutation consent default. A tree guard checks the group against each leaf's flags, database-target default and consent.
…LI-2695) Add resolveTarget for sync and selectable commands and DbConfigResolver.resolveConnection. It takes an already-resolved target, rejects --password unless the target is linked, announces the local and --db-url targets, and only removes network bans when the caller holds an approval. The existing resolve method keeps its behaviour.
requireHostedMutationConsent grants on --yes or a shell SUPABASE_YES, asks only an interactive text-mode terminal, and otherwise fails with HostedMutationConsentRequiredError; stdin is never read. A local target needs no consent, and hostedMutationPreapproval lets reads clear a network ban only under --yes.
planDbSeed returns the seed input and a detail line for the single hosted consent. The existing resolveDbSeedInput and confirmSeedIntoMatchedRemote stay until the integration shard removes them.
describeHostedConsent runs the six consent cases against a leaf: JSON and piped text without --yes, a TTY decline, --yes, a shell SUPABASE_YES, and a project .env that must not count. A synthetic leaf proves the helper.
…ith hosted consent
CLI-2695) Encryption, network bans/restrictions, SSL enforcement, Postgres config, SSO and notebooks push ask for consent (default No) before their first API write, or accept --yes. Reads never prompt.
…he local target first
…s after the retries run out
…r can match A nested comment closed by */ directly followed by * re-used the slash as the start of /*, so the splitter stayed inside a comment PostgreSQL had already closed. /*/ also no longer closes the comment it opens. Adds an unterminated-input flag for callers that must refuse such input.
…ck waits --read-only now accepts only statements that start with SELECT, WITH, VALUES, TABLE, SHOW or EXPLAIN, refuses input that ends inside a comment, quote or dollar quote or contains BEGIN ATOMIC, and sets a 5s lock_timeout inside the read-only transaction on direct connections. Docs and SIDE_EFFECTS state what the flag does and does not guarantee.
…-project-id removal
…tions - Hosted changes accept only `--yes` or a shell-exported SUPABASE_YES; a value that a cwd `.env`, `.env.local` or `.env.$NODE_ENV[.local]` also defines counts as file-sourced. - The confirmation echo names the real source (`--yes` or `SUPABASE_YES`). - The consent-required error names the action, target and reason, with suggestions for `--yes`, `--read-only` (db query), ignored dotenv files and agent runs. - Action strings are imperative sentence-case phrases at every call site. - `canPrompt` refuses when an agent is detected; `notebooks pull` reconcile uses it. - Global `--yes` help describes the hosted rule. - A sync or selectable leaf with `consent: "none"` must declare `consentExemptReason` (type and tree guard); notebooks pull's create operation is exempt for that leaf only. - ADR 0033 matches the code: groups, approval sources, agent rule, exemptions.
# Conflicts: # apps/cli/src/commands/gen/types/SIDE_EFFECTS.md
…default-local hint Show "branch" only on positive evidence, name a ref that is not the linked project, append a --linked hint to the default-local banner, add the port to the db-url banner, encode every JSON target through a CommandTargetJson schema, share withAnnouncedTarget, and build the linked ResolvedTarget in resolveTarget. config push reads its JSON target environment from the live probe and prints its plan lines in plan wording.
Every loader reports a malformed config file through describeConfigParseError, resolveTarget hints at --linked only for a default-local run on a linked checkout, resolveConfigTarget has integration coverage for its linked target, and the command target banner and JSON shape live in one shared doc that the SIDE_EFFECTS files link.
# Conflicts: # apps/cli/src/commands/config/push/push.integration.test.ts # apps/cli/src/commands/secrets/set/set.integration.test.ts
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🤖 AI Review
Verified all four findings against the PR code. Confirmed the inaccurate reset plan, failing read-only live-test assertion, and misleading linked-target hint. Refuted the reported consent-bypass example: parseDotEnv expands the same-file reference to "1", so the consent check recognizes it as dotenv-derived. Focused Bun checks verified the parser behavior and SQL rejection; the live suite was not run.
Findings
| Severity | Location | Category | Sources | Claim |
|---|---|---|---|---|
| 🟡 MINOR | apps/cli/src/commands/db/reset/reset.handler.ts:243 |
ux |
claude | The remote reset confirmation promises to apply local migrations even when migrations are disabled in config. |
| 🟡 MINOR | apps/cli/src/commands/db/query/query.live.test.ts:48 |
test-correctness |
codex | The new read-only live test expects a database error, but its CREATE statement is rejected by the client-side filter before execution. |
| 🟡 MINOR | apps/cli/src/command-internal/target/resolve-target.ts:75 |
target-selection |
codex | The default-local hint can name a different project from the project that --linked selects when SUPABASE_PROJECT_ID overrides the saved link. |
Refuted findings (kept for transparency, not posted as review comments)
apps/cli/src/command-internal/hosted-consent/hosted-consent.yes.ts:74(security): Dotenv variable expansion can silently approve hosted mutations because APPROVAL=1 followed by SUPABASE_YES=$APPROVAL allegedly parses as a literal reference while Bun supplies "1".
Refuted: Running the actual parseDotEnv implementation on the reported input returns SUPABASE_YES="1", not "$APPROVAL". That equals the reported Bun process value, so hosted-consent.yes.ts:74-77 returns no approval. The alleged literal-versus-expanded mismatch does not occur for this example.
Stats
Claude findings: 1 · Codex findings: 3 · Confirmed: 3 · Refuted: 1 · Uncertain: 0
Models: claude-opus-5-5 + gpt-6.1-sol · Trigger: auto · Workflow run
This review runs once per PR. A maintainer can request another with a /ai-review comment.
| useSchemaFiles | ||
| ? "apply the declarative schema files" | ||
| : `apply local migrations${resolvedVersion === "" ? "" : ` up to version ${resolvedVersion}`}`, |
There was a problem hiding this comment.
🟡 MINOR · ux · source: claude
The remote reset confirmation promises to apply local migrations even when migrations are disabled in config.
Evidence: reset.handler.ts:243-245 includes the migrations plan line whenever useSchemaFiles is false, whereas lines 270-279 apply migrations only when toml.migrationsEnabled is true. db-config.toml-read.ts:718 obtains that boolean from config.db.migrations.enabled.
Suggested fix: Include the migrations plan line only when toml.migrationsEnabled is true; retain the separate declarative-schema plan line.
| `create table ${table} (id int)`, | ||
| "--db-url", | ||
| project.dbUrl, | ||
| "--read-only", | ||
| ]); | ||
| expect(result.exitCode).not.toBe(0); | ||
| expect(result.stderr).toContain("read-only transaction"); |
There was a problem hiding this comment.
🟡 MINOR · test-correctness · source: codex
The new read-only live test expects a database error, but its CREATE statement is rejected by the client-side filter before execution.
Evidence: query.live.test.ts:48 submits CREATE TABLE with --read-only, and line 54 expects "read-only transaction". query.read-only.ts:77-87 classifies CREATE as a violation; query.handler.ts:364-371 returns the statement-filter error before reaching execution at lines 384-391.
Suggested fix: Test database enforcement with an allowlisted statement that attempts a write, such as SELECT INTO a uniquely named table, and clean up that table if enforcement regresses.
| const linkedRef = yield* readProjectRefFile(fs, path, workdir).pipe( | ||
| Effect.map(Option.filter((ref) => PROJECT_REF_PATTERN.test(ref))), | ||
| Effect.orElseSucceed(() => Option.none<string>()), | ||
| ); | ||
| if (Option.isNone(linkedRef)) return; | ||
| const cell = yield* Effect.serviceOption(CommandTarget); | ||
| if (Option.isNone(cell)) return; | ||
| const cache = yield* readCachedLinkedProject(fs, path, workdir); | ||
| yield* cell.value.hintLocal(linkedAlternativeHint({ ref: linkedRef.value, cache })); |
There was a problem hiding this comment.
🟡 MINOR · target-selection · source: codex
The default-local hint can name a different project from the project that --linked selects when SUPABASE_PROJECT_ID overrides the saved link.
Evidence: resolve-target.ts:75-83 builds the hint exclusively from readProjectRefFile. target-identity.ts:76 renders "default; pass --linked for" that ref. In contrast, project-ref.layer.ts:115-122 prioritizes cliSettings.projectId over the saved ref, and command-settings.layer.ts:148-149 loads the environment project ID.
Suggested fix: Resolve the hint's project using the same environment-before-file precedence as linked selection, without prompting or making network requests.
This branch has not been deployed
1 participant
Summary
Database and storage commands that can run locally didn't agree on what they target by default. Some went to the local stack, some to the linked project, and
gen typesopened a picker. Hosted mutations also each had their own idea of when to ask, what the default answer was, and whetheryon stdin orSUPABASE_YESinsupabase/.envcounted. This makes both consistent.Targets
withTargetPolicy(local-only, account, hosted, sync, selectable). A tree test fails if a leaf doesn't.--linked,--project-refor--db-urlpick something else, or set the new workspacetarget = "remote"insupabase/config.toml(orSUPABASE_TARGET=remote) to make the linked project the default.--local,--linkedand--db-urlare mutually exclusive everywhere,--project-refimplies--linked, and--passwordonly applies to a linked target.Target: …line on stderr, and JSON results carry atargetobject.Hosted consent
--yesor a shell-exportedSUPABASE_YES. ASUPABASE_YESfromsupabase/.envor a working-directory.envdoesn't count, stdin is never read as an answer, and agents never get a prompt.--yes.db pullorpullwhen it would update the remote migration history now aborts before any file is written.New
db query --read-onlyaccepts only read statements (SELECT,WITH,VALUES,TABLE,SHOW,EXPLAINor a parenthesised query), runs them in a read-only transaction with a short lock timeout (or with the Management API'sread_onlymode on--linked), and skips consent. It's meant for unattended reads against hosted databases. Functions with side effects called from aSELECTstill run, and the docs say so.The design and its enforcement are in ADR 0033, which also replaces ADR 0032's seed consent section and updates ADR 0004's safety model. A companion PR to
develop, #7186, adds thetargetkey early and warns on commands whose default flips here.Linked issue
Closes CLI-2695
🤖 Generated with Claude Code
BREAKING CHANGE: Target defaults and hosted-mutation consent are now consistent across the CLI.
db dump,migration list,migration repair,inspect db *,inspect report,gen typesandstorage ls/cp/mv/rmnow use the workspace target, which defaults to the local stack, instead of the linked project. To keep the old behaviour, pass--linked, settarget = "remote"insupabase/config.toml, or exportSUPABASE_TARGET=remote.SUPABASE_PROJECT_IDon its own no longer selects a hosted project for these commands.gen typeswith no selector generates from the local database instead of opening a project picker or failing.--project-idis deprecated in favour of--project-ref.db schema declarative generateno longer asks which target to use. It uses the workspace target, or--linked/--db-url.--local=falseand--linked=falsenow mean "not set", sodb reset --linked=falseresets the local database.--local,--linkedand--db-urlare mutually exclusive on every command.--project-refimplies--linkedand is rejected with--localor--db-url.--passwordis rejected unless the target is the linked project, before any local stack is started.HostedMutationConsentRequiredErrorin non-interactive, JSON, stream-json, agent or-o json|yaml|toml|envruns unless--yes(orSUPABASE_YESexported in the shell) is set. Commands that newly need this for unattended runs:--read-onlyfor reads)ypiped on stdin no longer confirms a hosted mutation.SUPABASE_YESinsupabase/.env, or in a.env,.env.localor.env.<NODE_ENV>[.local]file in the working directory, no longer confirms hosted mutations. When one of those files sets it, a shell export of the same value doesn't count either. Use--yes.--prunenow runs even when every function is already up to date.secrets unset --alllists the secrets before asking.HostedMutationDeclinedError("context canceled"). The command-specific cancelled error codes are removed. Decliningdb pullorpullaborts before any file is written; there's no longer a files-only path.--yes. Otherwise the command fails and points tosupabase network-bans remove.migration downandmigration repair --allon the local database no longer prompt.Target: …once on stderr, including read-only commands against a hosted project.config pushno longer prints its own "Pushing config to" line. JSON object results and error envelopes gain atargetobject;db reset'stargetchanges from a string to that object, andconfig pushJSON no longer reportsdeclined_addonsor a skipped status. Arrays and generated artifacts are unchanged.migration upagainst a hosted target reports "Remote database is up to date." instead of "Local database is up to date.", andmigration listlabels the history column "Local DB" when the target is local.--yesfor hosted mutations.