Skip to content

Commit dcbeb91

Browse files
committed
fix(vscode-extension): ask before running a write, as the web editor does
Run sent the target straight to query/run, so DELETE FROM orders ran on the first click while the web editor asks first. query/target now also answers dangerous: true when isDangerousQuery flags the target under the connection's type, and Run shows a modal warning with the statement and a Run Anyway button before anything is sent. Cancel, Escape and a cancelled run send nothing. Select Rows (a generated SELECT) and Load More (pages of a statement already confirmed) ask nothing, as in the web editor. The AI risk analysis and the typed confirmation some engines require are not reproduced. Closes #1935
1 parent 732e91f commit dcbeb91

8 files changed

Lines changed: 158 additions & 12 deletions

File tree

‎docs/VSCODE_EXTENSION.md‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ The webview build swaps three imports for shims through `shimPlugin` in `scripts
8888
| Connection profiles and secrets | `src/host/connections/profile.ts` (`readProfiles`, `toEntry`, `toWireConnection`), `connection-store.ts` (`ConnectionStore`), `secret-keys.ts` (`secretKey`, `SECRET_PATHS`) | `src/lib/storage/secret-fields.ts` (`secretFieldValues`, `withoutSecretFields`, `withSecretFieldValues` and its field classification maps), `EXTERNAL_DATABASE_TYPES` in `src/lib/db/compatibility.ts` | The profile sits in the `libredb.connections` setting; every secret sits in SecretStorage |
8989
| Add Connection | `src/host/connections/add-connection-flow.ts` (`AddConnectionFlow`, a pure state machine), `add-connection-ui.ts` (`addConnection`), `field-prompts.ts` (`FIELD_PROMPTS`), `src/backend/engine-catalog.ts` (`engineCatalog`) | `CONNECTION_FORM`, `CONNECTION_PICKER_ORDER` and the `connectionField*` helpers in `src/lib/db/connection-form.ts`, `CONNECTION_FIELDS` and `PLACEABLE_FIELDS` in `src/lib/db/connection-fields.ts`, `CONNECTION_STRING` in `src/lib/db/connection-string`, `parseConnectionString` in `src/lib/connection-string-parser.ts`, `testConnection` in `src/lib/api/test-connection.ts`, `newLocalId` in `src/lib/ids.ts` | The flow names no engine: every per-engine fact comes from the `engines/list` catalogue |
9090
| Connections tree | `src/host/tree/connections-tree.ts` (`ConnectionsTree`), `nodes.ts`, `tree-items.ts`, `icons.ts` | The kind helpers in `src/lib/db/object-kinds.ts` (`containerDepth`, `enumerableKinds`, `findKind`, `kindGroup`, ...), and in the child the provider methods `listContainers`, `countObjects`, `listObjects`, `describeObject` | Levels, folders and labels come from the provider's capabilities, recorded in `OpenConnections` |
91-
| Editor binding, Run, Cancel, status bar | `src/host/editor/` (`BindingStore`, `RunRegistry`, `RunService`, `runQuery`, `newQuery`, `selectRows`, `cancelQuery`, `changeConnection`, `ConnectionStatusBar`, `ContextKeys`, `DocumentLifecycle`) | In the child: `runTarget` in `src/lib/sql/run-target.ts`, `resolveSqlGrammar`, `countCodeStatements`, `generateTableQuery` in `src/lib/query-generators.ts`, `editorLanguageForTabType` in `src/lib/editor/tab-language.ts` | Bindings live in memory only, so a window reload forgets them |
91+
| Editor binding, Run, Cancel, status bar | `src/host/editor/` (`BindingStore`, `RunRegistry`, `RunService`, `runQuery`, `newQuery`, `selectRows`, `cancelQuery`, `changeConnection`, `ConnectionStatusBar`, `ContextKeys`, `DocumentLifecycle`) | In the child: `runTarget` in `src/lib/sql/run-target.ts`, `isDangerousQuery` in `src/lib/sql/dangerous-query.ts`, `resolveSqlGrammar`, `countCodeStatements`, `generateTableQuery` in `src/lib/query-generators.ts`, `editorLanguageForTabType` in `src/lib/editor/tab-language.ts` | Bindings live in memory only, so a window reload forgets them |
9292
| Backend handlers and registry | `src/backend/handlers.ts` (`createHandlers`, `registerHandlers`, `toResponseError`), `registry.ts` (`ConnectionRegistry`), `start.ts` (`startBackend`), `main.ts` | `getOrCreateProvider`, `removeProvider`, `clearProviderCache`, `enabledProviderTypes` in `src/lib/db/factory.ts`; `checkEditorQuery` and `runEditorQuery` in `src/lib/api/editor-query.ts`; `databaseErrorBody` and `internalErrorBody` in `src/lib/api/error-body.ts`; `supportsQueryCancel`, `scopeToCatalog`, `executionUiFor` | `query/run` re-does the steps of `POST /api/db/query` for one statement through the same functions |
9393
| Backend lifecycle | `src/host/backend-supervisor.ts` (`BackendSupervisor`), `fork-options.ts`, `channel.ts` (`ENABLED_PROVIDERS`) | None; `RESTART_DELAYS_MS` copies `desktop/src-tauri/src/backoff.rs` | |
9494
| JSON-RPC protocol | `src/protocol.ts` (`BackendApi`, `METHODS`, `PROTOCOL_VERSION`), `src/host/backend-client.ts` (`BackendClient`, `BackendError`) | Types only (`DatabaseConnection`, `ProviderCapabilities`, `EditorQueryBody`, `ApiErrorCode`, ...) | Every Studio import in `protocol.ts` is type-only, so it adds no provider code to the host |
@@ -173,7 +173,8 @@ sequenceDiagram
173173
B-->>C: ProviderDescription
174174
R->>C: runTarget
175175
C->>B: query/target with text, selection, cursor
176-
B-->>R: statement and range
176+
B-->>R: statement, range and dangerous
177+
R->>U: confirm, when dangerous
177178
R->>S: execute
178179
S->>V: runStarted
179180
V->>W: runStarted
@@ -195,7 +196,8 @@ Step by step, with the code that does it:
195196
2. `RunRegistry.prepare` registers a preparing run with its own cancellation token, so Cancel works even while the connection opens.
196197
3. `BackendClient.ensureOpen` returns the connection's `ProviderDescription`, opening it first when needed; where the capabilities declare catalog sessions, the person picks a catalog once per binding.
197198
4. `query/target` sends the whole text, the selection and the cursor; the child applies the web editor's run target rule (`runTarget`) with the engine's grammar, and an empty target becomes "Nothing to run.".
198-
The range that will run is highlighted for one second.
199+
The answer also says whether the web editor would ask before running that statement: the child reads it with `isDangerousQuery` under the connection's type, the gate behind the web editor's Query Safety dialog.
200+
The range that will run is highlighted for one second, and a statement the gate flags then asks in a modal `showWarningMessage` that shows it and offers Run Anyway; Cancel or Escape runs nothing.
199201
5. `RunService.execute` mints a fresh `queryId`, turns the preparing run into a running `RunEntry`, tells the presenter, and sends `query/run` inside a progress shown on the Results view.
200202
6. In the child, `createHandlers().runQuery` checks the params, runs `checkEditorQuery` (the page bounds, and the console text bound, which answers `TEXT_TOO_LARGE`), resolves the provider through `getOrCreateProvider` and `scopeToCatalog`, and calls `runEditorQuery`, the same function `POST /api/db/query` uses.
201203
7. The answer is a `QueryPage`: the route's `EditorQueryBody` (first result set, `pagination` always present), plus `resultSetCount` when the text produced more than one result set.
@@ -367,7 +369,8 @@ Select Rows asks the child for `objects/selectStatement`, which writes the state
367369

368370
- **No login or roles.** The extension serves one person's VS Code: no host or backend code signs anyone in or checks a role, and the host bundle check refuses `src/lib/config/auth-env.ts`.
369371
- **No storage layer or sync.** Connections are the setting plus SecretStorage, not `src/lib/storage`; the webview's `@/lib/storage` shim throws on any use; editor bindings are in memory only.
370-
- **No dangerous-query confirmation.** The web editor's safety check in `src/hooks/use-query-execution.ts` has no counterpart; `query/run` sends the target as written, held only to `checkEditorQuery`'s page bounds and console text bound.
372+
- **A plainer dangerous-query confirmation.** The statement is read by the web editor's own `isDangerousQuery`, but the question is one modal button: no AI risk analysis, and no typed confirmation where the web editor asks for one (an etcd range delete or lease revoke).
373+
Select Rows and Load More ask nothing: the first runs a generated SELECT and the second reads more pages of a statement already confirmed.
371374
- **No query events or governance.** The child bundle refuses `src/lib/governance/start.ts` and `src/lib/query-events/sink.ts`, and `query/run` runs no admission gate.
372375
- **Masking starts off.** The view's saved state starts with `maskingEnabled: false` (`INITIAL_VIEW_STATE`); the person can switch it on and reveal values, since `EXTENSION_MASKING_CONFIG` gives the `user` role both rights.
373376
- **Some engines are held back.** `ENABLED_PROVIDERS` in `src/host/channel.ts` is every engine of `EXTERNAL_DATABASE_TYPES` except the ones `HELD_BACK` lists with a reason; the catalogue and the factory both follow it.

‎vscode-extension/README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ Rows live only in the window's memory, so reloading the window empties the view.
3939
## Running and cancelling
4040

4141
**Run Query** sends the selected text, or the statement at the cursor when nothing is selected.
42-
There is no confirmation before a statement runs, including one that changes data.
42+
A statement that changes data, database objects or permissions asks for confirmation first, as it does in the web editor.
4343
**Cancel Query** stops a running statement on an engine that can cancel one, such as PostgreSQL, SQL Server and MySQL.
4444
A MongoDB command cannot be cancelled once it is sent.
4545

‎vscode-extension/src/backend/handlers.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import type {
2525
} from "@/lib/db/types";
2626
import { type EditorLanguage, editorLanguageForTabType, resolveTabType } from "@/lib/editor/tab-language";
2727
import { generateTableQuery } from "@/lib/query-generators";
28+
import { isDangerousQuery } from "@/lib/sql/dangerous-query";
2829
import { resolveSqlGrammar } from "@/lib/sql/grammar";
2930
import { runTarget } from "@/lib/sql/run-target";
3031
import { countCodeStatements } from "@/lib/sql/statement-splitter";
@@ -448,7 +449,8 @@ export function createHandlers(
448449
},
449450

450451
// The web editor's run target rule on the offsets the host's editor reports; a target with no code under the sql
451-
// editor language answers "", so the host's one blank check says "Nothing to run." (builder's call 4).
452+
// editor language answers "", so the host's one blank check says "Nothing to run." (builder's call 4). The target
453+
// also carries the web editor's confirmation gate's verdict, read under the connection's type, for the host to ask on.
452454
async runTarget(params) {
453455
const connectionId = connectionIdOf(params);
454456
const text = aString(fieldOf(params, "text"), "text");
@@ -476,7 +478,7 @@ export function createHandlers(
476478
if (rule.editorLanguage === "sql" && countCodeStatements(target.statement, rule.grammar) === 0) {
477479
return { statement: "" };
478480
}
479-
return target;
481+
return isDangerousQuery(target.statement, record.connection.type) ? { ...target, dangerous: true } : target;
480482
},
481483
};
482484
}

‎vscode-extension/src/host/editor/run-query.ts‎

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import type { RunRequest, RunService } from "./run-service";
1515
export const NOTHING_TO_RUN = "Nothing to run.";
1616
export const EDITOR_NOT_VISIBLE = "The editor to run is not visible.";
1717
export const NO_ACTIVE_EDITOR = "Open an editor to run a query.";
18+
export const RUN_ANYWAY = "Run Anyway";
1819
export const HIGHLIGHT_MS = 1000;
1920
export const ROW_LIMIT_MAX = 10_000;
2021

@@ -41,7 +42,10 @@ export function readRowLimit(workspace: Pick<typeof vscode.workspace, "getConfig
4142
export interface RunQueryDeps extends PickerDeps, FailureDeps {
4243
readonly window: PickerDeps["window"] &
4344
FailureDeps["window"] &
44-
Pick<typeof vscode.window, "activeTextEditor" | "visibleTextEditors" | "createTextEditorDecorationType">;
45+
Pick<
46+
typeof vscode.window,
47+
"activeTextEditor" | "visibleTextEditors" | "createTextEditorDecorationType" | "showWarningMessage"
48+
>;
4549
readonly workspace: Pick<typeof vscode.workspace, "getConfiguration">;
4650
readonly store: PickerDeps["store"] & Pick<ConnectionStore, "get">;
4751
readonly client: Pick<BackendClient, "ensureOpen" | "runTarget" | "listContainers">;
@@ -130,8 +134,9 @@ export function showRowLimitError(deps: Pick<RunQueryDeps, "window" | "openUserS
130134
}
131135

132136
/**
133-
* Steps 7 to 11 under the prepared run's token: the open, the catalog, query/target and the highlight. Answers the
134-
* request, or undefined when the run was cancelled, the catalog pick dismissed, or the target holds no code.
137+
* Steps 7 to 11 under the prepared run's token: the open, the catalog, query/target, the highlight and, for a target the
138+
* web editor would ask about, the confirmation. Answers the request, or undefined when the run was cancelled, the
139+
* catalog pick dismissed, the target holds no code or the confirmation declined.
135140
*/
136141
async function prepare(
137142
deps: RunQueryDeps,
@@ -173,7 +178,7 @@ async function prepare(
173178
token,
174179
);
175180
if (target.cancelled) return undefined;
176-
const { statement, range } = target.value;
181+
const { statement, range, dangerous } = target.value;
177182
if (statement.trim() === "") {
178183
void deps.window.showInformationMessage(NOTHING_TO_RUN);
179184
return undefined;
@@ -190,6 +195,10 @@ async function prepare(
190195
}
191196
const entry = deps.store.get(connectionId);
192197
if (entry === undefined) throw new UnknownConnectionError(connectionId);
198+
if (dangerous === true) {
199+
const answer = await untilCancelled(confirmDangerous(deps, entry.name, statement), token);
200+
if (answer.cancelled || !answer.value) return undefined;
201+
}
193202
return {
194203
documentLabel: documentLabel(document),
195204
connectionName: entry.name,
@@ -201,6 +210,23 @@ async function prepare(
201210
};
202211
}
203212

213+
/**
214+
* The web editor's dialog before a write or a drop, as a modal that shows the statement: true once the person chooses
215+
* Run Anyway, false on Cancel or Escape.
216+
*/
217+
async function confirmDangerous(
218+
deps: Pick<RunQueryDeps, "window">,
219+
connectionName: string,
220+
statement: string,
221+
): Promise<boolean> {
222+
const choice = await deps.window.showWarningMessage(
223+
`This statement may change data, database objects, or permissions. Review it before running it on ${connectionName}.`,
224+
{ modal: true, detail: statement },
225+
RUN_ANYWAY,
226+
);
227+
return choice === RUN_ANYWAY;
228+
}
229+
204230
/**
205231
* Binds the document under the key its run is registered under now, so a rename or Save As during the open or the
206232
* catalog pick does not recreate a binding under the old key.

‎vscode-extension/src/protocol.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,10 +118,14 @@ export interface RunTargetParams extends ConnectionIdParams {
118118
cursor: number;
119119
}
120120

121-
/** range is absent when the statement is the whole text; statement is "" when the target holds no code. */
121+
/**
122+
* range is absent when the statement is the whole text; statement is "" when the target holds no code; dangerous is
123+
* present, as true, when the web editor would ask before running it (isDangerousQuery under the connection's type).
124+
*/
122125
export interface RunTargetResult {
123126
statement: string;
124127
range?: OffsetRange;
128+
dangerous?: true;
125129
}
126130

127131
export interface RunQueryParams {

‎vscode-extension/test/integration/sqlite-child.test.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
METHODS,
1515
type OpenResult,
1616
type QueryPage,
17+
type RunTargetResult,
1718
type TestConnectionResult,
1819
type WireConnection,
1920
} from "../../src/protocol";
@@ -89,6 +90,22 @@ describe("SQLite in the backend child", () => {
8990
assert.doesNotMatch(session.stderr(), /SQLite worker exited/);
9091
});
9192

93+
it("flags a write on query/target and leaves a read unflagged, as the bundled gate reads them", async () => {
94+
await session.connection.sendRequest<OpenResult>(
95+
METHODS.openConnection,
96+
params(sqlite("flag", join(dir, "flag.db"))),
97+
);
98+
const target = (text: string) =>
99+
session.connection.sendRequest<RunTargetResult>(METHODS.runTarget, {
100+
connectionId: "flag",
101+
text,
102+
selection: { start: 0, end: 0 },
103+
cursor: 0,
104+
});
105+
assert.equal((await target("DELETE FROM t")).dangerous, true);
106+
assert.equal((await target("SELECT 1")).dangerous, undefined);
107+
});
108+
92109
it("refuses a relative path before the provider builds, and leaves no file in the extension's folder", async () => {
93110
const refused = await errorOf(
94111
session.connection.sendRequest(METHODS.testConnection, params(sqlite("relative", "it-relative.db"))),

‎vscode-extension/test/unit/backend-handlers.test.ts‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -960,6 +960,47 @@ describe("query/target", () => {
960960
assert.deepEqual(await target(handlers, text, 12), { statement: text });
961961
});
962962

963+
it("flags a target the web editor would ask about, and only that one", async () => {
964+
const { handlers } = await on();
965+
const text = "select 1;\ndelete from t;";
966+
assert.deepEqual(await target(handlers, text, 3), { statement: "select 1", range: { start: 0, end: 8 } });
967+
assert.deepEqual(await target(handlers, text, 12), {
968+
statement: "delete from t",
969+
range: { start: 10, end: 23 },
970+
dangerous: true,
971+
});
972+
});
973+
974+
it("flags a selection that holds a write behind a read, which the gate splits as the runner does", async () => {
975+
const { handlers } = await on();
976+
assert.deepEqual(await target(handlers, "select 1; drop table t", 0, { start: 0, end: 22 }), {
977+
statement: "select 1; drop table t",
978+
range: { start: 0, end: 22 },
979+
dangerous: true,
980+
});
981+
});
982+
983+
it("reads the target under the connection's type", async () => {
984+
// A `#` starts a comment in MySQL and is code in PostgreSQL, so only there does the DELETE after it run.
985+
const text = "select 1 # ; delete from t";
986+
const whole = { start: 0, end: text.length };
987+
const postgres = await on();
988+
const mysql = await openedOn(fakeProvider({ getCapabilities: () => SQL_CAPS }), { ...WIRE, type: "mysql" });
989+
assert.equal((await target(postgres.handlers, text, 0, whole)).dangerous, true);
990+
assert.equal((await target(mysql.handlers, text, 0, whole)).dangerous, undefined);
991+
});
992+
993+
it("flags a non-SQL target by its engine's own vocabulary", async () => {
994+
const mongo = await openedOn(
995+
fakeProvider({ getCapabilities: () => ({ ...SQL_CAPS, queryLanguage: "json" }) as ProviderCapabilities }),
996+
{ ...WIRE, type: "mongodb" },
997+
);
998+
const drop = '{ "collection": "orders", "operation": "deleteMany", "filter": {} }';
999+
assert.deepEqual(await target(mongo.handlers, drop, 0), { statement: drop, dangerous: true });
1000+
const find = '{ "collection": "orders", "operation": "find" }';
1001+
assert.deepEqual(await target(mongo.handlers, find, 0), { statement: find });
1002+
});
1003+
9631004
it("answers runTarget's RangeError as InvalidParams with its sentence", async () => {
9641005
const { handlers } = await on();
9651006
const offsets = [

‎vscode-extension/test/unit/editor/run-query.test.ts‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
NO_ACTIVE_EDITOR,
1313
NOTHING_TO_RUN,
1414
ROW_LIMIT_MAX,
15+
RUN_ANYWAY,
1516
RowLimitError,
1617
readRowLimit,
1718
runQuery,
@@ -552,6 +553,58 @@ describe("runQuery", () => {
552553
});
553554
});
554555

556+
describe("step 11b: the confirmation", () => {
557+
const DELETE = "delete from t";
558+
const asks = () => {
559+
const env = bound(DELETE);
560+
env.scripts.runTarget = () => Promise.resolve({ statement: DELETE, dangerous: true });
561+
return env;
562+
};
563+
564+
it("asks in a modal that shows the statement, and runs it once the person chooses Run Anyway", async () => {
565+
const { deps, executed } = asks();
566+
answerMessages(() => RUN_ANYWAY);
567+
await runQuery(deps);
568+
assert.deepEqual(shownMessages, [
569+
{
570+
level: "warning",
571+
message:
572+
"This statement may change data, database objects, or permissions. Review it before running it on shop.",
573+
options: { modal: true, detail: DELETE },
574+
items: ["Run Anyway"],
575+
},
576+
]);
577+
assert.equal(executed.length, 1);
578+
assert.equal(executed[0][1].statement, DELETE);
579+
});
580+
581+
it("sends nothing and settles the run when the person dismisses it", async () => {
582+
const { deps, runs, executed } = asks();
583+
await runQuery(deps);
584+
assert.equal(shownMessages.length, 1);
585+
assert.deepEqual(executed, []);
586+
assert.deepEqual(runs.running(), []);
587+
});
588+
589+
it("sends nothing when the run is cancelled during the question, whatever the person then chose", async () => {
590+
const { deps, runs, executed } = asks();
591+
answerMessages(() => {
592+
runs.of(UNTITLED)?.tokenSource.cancel();
593+
return RUN_ANYWAY;
594+
});
595+
await runQuery(deps);
596+
assert.deepEqual(executed, []);
597+
assert.deepEqual(runs.running(), []);
598+
});
599+
600+
it("asks nothing for a target the child did not flag", async () => {
601+
const { deps, executed } = bound("select 1;");
602+
await runQuery(deps);
603+
assert.deepEqual(shownMessages, []);
604+
assert.equal(executed.length, 1);
605+
});
606+
});
607+
555608
describe("step 12: the run", () => {
556609
it("hands the prepared run and its request to the service, and resolves once it settled", async () => {
557610
const { deps, scripts, executed, runs } = bound();

0 commit comments

Comments
 (0)