fix(redis): skip comment lines when executing editor commands

This commit is contained in:
Loc Nguyen
2026-09-20 01:48:20 +08:00
committed by GitHub
parent 01f8326575
commit c5312553f1
10 changed files with 110 additions and 18 deletions
@@ -1540,6 +1540,33 @@ SELECT @value AS Message;`;
expect(executeCurrentSql).toHaveBeenCalledWith(sql, { tabId: "tab-1", skipRedisSafetyCheck: false, openInNewResultTab: true });
});
it("executes Redis batches with comment lines when the dangerous-command guard is on by default", async () => {
const sql = "# warm the cache\nGET user:1\n-- then ping\nPING";
const activeTab = ref<QueryTab | undefined>({ ...queryTab("0"), sql });
const activeConnection = ref<ConnectionConfig | undefined>(connection("redis"));
const activeOutputView = ref<"result" | "summary" | "explain" | "chart">("result");
const queryStore = useQueryStore();
const executeCurrentSql = vi.spyOn(queryStore, "executeCurrentSql").mockImplementation(async () => {
if (activeTab.value) activeTab.value.result = { columns: [], rows: [], affected_rows: 0, execution_time_ms: 1 };
});
vi.spyOn(useHistoryStore(), "add").mockResolvedValue(undefined);
const execution = useSqlExecution({
activeTab: computed(() => activeTab.value),
activeConnection: computed(() => activeConnection.value),
executableSql: computed(() => sql),
activeOutputView,
});
await execution.tryExecute();
// A comment line never reaches the safety pre-scan, so a safe batch is not
// blocked by the fail-closed classifier.
expect(execution.showDangerDialog.value).toBe(false);
expect(executeCurrentSql).toHaveBeenCalledTimes(1);
expect(executeCurrentSql).toHaveBeenCalledWith(sql, { tabId: "tab-1", skipRedisSafetyCheck: false });
});
it("distinguishes read-only and mutating Meilisearch REST requests", () => {
expect(isDangerousSql("GET /health", "meilisearch")).toBe(false);
expect(isDangerousSql('POST /indexes/movies/documents/fetch\n{"limit":10}', "meilisearch")).toBe(false);
@@ -14,6 +14,7 @@ import { invalidateObjectMetadataCache } from "@/lib/metadata/objectMetadataCach
import { defaultViewForResult } from "@/lib/query/queryResultDefaultView";
import { isQueryExecutionErrorResult } from "@/lib/query/queryResultError";
import { classifyRedisCommandSafety } from "@/lib/redis/redisCommandSafety";
import { isRedisCommentLine } from "@/lib/redis/redisCommandTokenizer";
import { isSqlExecutionSnapshot, resolveExecutableSql, type SqlExecutionOverride, type SqlExecutionSnapshot } from "@/lib/sql/sqlExecutionTarget";
import { isElasticsearchRestRequestText, parseElasticsearchRestRequestTarget, splitSqlStatementRanges, sqlStatementParameterOptionsForCompatibility } from "@/lib/sql/sqlStatementRanges";
import { extractSqlParameterDescriptors, type SqlParameterDescriptor, type SqlParameterSyntax } from "@/lib/sql/sqlParameters";
@@ -253,7 +254,7 @@ export function useSqlExecution(deps: {
const commands = sql
.split("\n")
.map((line) => line.trim())
.filter((line) => line.length > 0);
.filter((line) => line.length > 0 && !isRedisCommentLine(line));
let highestSafety: "allowed" | "write" | "confirm" | "blocked" = "allowed";
for (const cmd of commands) {
const safety = classifyRedisCommandSafety(cmd);
@@ -489,7 +490,7 @@ export function useSqlExecution(deps: {
const commands = sql
.split("\n")
.map((line) => line.trim())
.filter((line) => line.length > 0);
.filter((line) => line.length > 0 && !isRedisCommentLine(line));
let highestSafety: "allowed" | "confirm" | "blocked" = "allowed";
for (const command of commands) {
const safety = classifyRedisCommandSafety(command);
@@ -1541,10 +1541,10 @@ DISTRIBUTED BY HASH(product_id) BUCKETS 1`;
});
it("returns Redis executable command lines", () => {
const sql = "GET user:1\n# comment\n DEL user:2 ";
const sql = "GET user:1\n# comment\n DEL user:2 \n-- another note\nPING";
const ranges = executableStatementRanges(sql, "redis");
expect(rangeSqlTexts(ranges)).toEqual(["GET user:1", "DEL user:2"]);
expect(ranges.map((range) => range.from)).toEqual([0, sql.indexOf("DEL")]);
expect(rangeSqlTexts(ranges)).toEqual(["GET user:1", "DEL user:2", "PING"]);
expect(ranges.map((range) => range.from)).toEqual([0, sql.indexOf("DEL"), sql.indexOf("PING")]);
});
it("keeps MySQL REPLACE INTO as an executable statement start", () => {
@@ -1618,10 +1618,11 @@ describe("currentExecutableStatementRange", () => {
});
it("uses the current Redis command line", () => {
const sql = "GET user:1\n DEL user:2\n# comment";
const sql = "GET user:1\n DEL user:2\n# comment\n-- note";
expect(currentExecutableStatementRange(sql, indexOf(sql, "DEL"), "redis")?.sql).toBe("DEL user:2");
expect(currentExecutableStatementRange(sql, indexOf(sql, "comment"), "redis")).toBeNull();
expect(currentExecutableStatementRange(sql, indexOf(sql, "note"), "redis")).toBeNull();
});
it("uses the current MongoDB command range", () => {
@@ -44,6 +44,8 @@ describe("read-only write unlock", () => {
expect(sqlLooksLikeMutation("CREATE TABLE t (id INT)", "mysql")).toBe(true);
expect(sqlLooksLikeMutation("GET k", "redis")).toBe(false);
expect(sqlLooksLikeMutation("SET k v", "redis")).toBe(true);
expect(sqlLooksLikeMutation("# note\nGET k", "redis")).toBe(false);
expect(sqlLooksLikeMutation("-- note\nSET k v", "redis")).toBe(true);
});
it("keeps the persistent read-only flag on while a window is active", async () => {
@@ -1,6 +1,7 @@
import { getActivePinia } from "pinia";
import { useReadOnlyUnlockStore, type WriteUnlockDurationSecs } from "@/stores/readOnlyUnlockStore";
import { classifyRedisCommandSafety } from "@/lib/redis/redisCommandSafety";
import { isRedisCommentLine } from "@/lib/redis/redisCommandTokenizer";
import { classifySqlRisk, isSqlRiskMutation } from "@/lib/sql/sqlRisk";
import type { DatabaseType } from "@/types/database";
@@ -39,7 +40,7 @@ export function sqlLooksLikeMutation(sql: string | undefined, databaseType?: Dat
if (databaseType === "redis") {
return sql.split("\n").some((line) => {
const command = line.trim();
return command.length > 0 && classifyRedisCommandSafety(command) !== "allowed";
return command.length > 0 && !isRedisCommentLine(command) && classifyRedisCommandSafety(command) !== "allowed";
});
}
return isSqlRiskMutation(classifySqlRisk(sql, { dialect: databaseType }).risk);
@@ -12,6 +12,15 @@ export interface RedisArgvResult {
unclosedQuoteStart?: number;
}
/**
* Redis has no comment syntax, but `#` and `--` lines are common as notes in saved
* scripts. The editor treats them as comments everywhere — diagnostics, statement
* ranges and execution — so a note never reaches the server as a command.
*/
export function isRedisCommentLine(line: string): boolean {
return /^\s*(#|--)/.test(line);
}
function escapedCharacter(value: string): string {
return value === "n" ? "\n" : value === "r" ? "\r" : value === "t" ? "\t" : value;
}
@@ -9,7 +9,7 @@
import type { SqlTextSpan } from "@/types/database";
import type { SqlSemanticDiagnostic } from "@/lib/sql/semantic/diagnostics";
import { resolveRedisCommandSpec } from "@/lib/redis/redisCommandTable";
import { tokenizeRedisLine } from "@/lib/redis/redisCommandTokenizer";
import { isRedisCommentLine, tokenizeRedisLine } from "@/lib/redis/redisCommandTokenizer";
export { tokenizeRedisLine, type RedisArgvResult, type RedisArgvToken } from "@/lib/redis/redisCommandTokenizer";
@@ -47,9 +47,7 @@ export function buildRedisSyntaxDiagnostics(source: string): SqlSemanticDiagnost
const rawLine = lines[lineIndex];
const lineNo = lineIndex + 1; // 1-based line number for SqlTextSpan
if (!rawLine || !rawLine.trim()) continue;
// Skip comment-ish lines (Redis has no official comments, but `#`/`--` are
// commonly used by users as notes).
if (/^\s*(#|--)/.test(rawLine)) continue;
if (isRedisCommentLine(rawLine)) continue;
const { argv, unclosedQuote, unclosedQuoteStart } = tokenizeRedisLine(rawLine);
@@ -1,6 +1,7 @@
import type { SqlExecutionCandidate } from "@/lib/sql/sqlExecutionTarget";
import { cursorBelongsToTrailingStatementDelimiter } from "@/lib/sql/statementDelimiter";
import { splitMongoCommandRanges } from "@/lib/mongo/mongoShellCommand";
import { isRedisCommentLine } from "@/lib/redis/redisCommandTokenizer";
import { readSqlBracedParameterAt, type SqlParameterOptions } from "@/lib/sql/sqlParameters";
import { isElasticsearchCompatibleDatabaseType, isMeilisearchDatabaseType, type DatabaseType } from "@/types/database";
@@ -2364,7 +2365,7 @@ function redisExecutableCommandRanges(sql: string): SqlTextRange[] {
const leadingWhitespace = rawLine.length - rawLine.trimStart().length;
const trailingWhitespace = rawLine.length - rawLine.trimEnd().length;
const trimmedLine = rawLine.trim();
if (trimmedLine && !trimmedLine.startsWith("#")) {
if (trimmedLine && !isRedisCommentLine(trimmedLine)) {
const from = lineStart + leadingWhitespace;
const to = lineStart + rawLine.length - trailingWhitespace;
ranges.push({ from, to, sql: sql.slice(from, to) });
@@ -2386,7 +2387,7 @@ function redisCommandRangeAtCursor(sql: string, cursorPos: number): SqlTextRange
const rawLine = sql.slice(lineStart, lineEnd);
const leadingWhitespace = rawLine.length - rawLine.trimStart().length;
const trimmedLine = rawLine.trim();
if (!trimmedLine || trimmedLine.startsWith("#")) return null;
if (!trimmedLine || isRedisCommentLine(trimmedLine)) return null;
const from = lineStart + leadingWhitespace;
const to = lineStart + rawLine.length - (rawLine.length - rawLine.trimEnd().length);
+4 -5
View File
@@ -6511,10 +6511,10 @@ export const useQueryStore = defineStore("query", () => {
if (conn?.db_type === "redis") {
await connStore.ensureConnected(executionConnectionId);
let currentDb = Number(executionDatabase) || Number(tab.database) || 0;
const commands = sql
.split("\n")
.map((line) => line.trim())
.filter((line) => line.length > 0);
// One command per line, skipping blank and comment lines — the same split that
// gives each result its source range, so the two can never drift apart.
const commandRanges = executableStatementRanges(sql, "redis");
const commands = commandRanges.map((range) => range.sql);
if (commands.length === 0) return false;
if (commands.length === 1 && isRedisMonitorCommand(commands[0])) {
const monitor = startRedisMonitor(
@@ -6556,7 +6556,6 @@ export const useQueryStore = defineStore("query", () => {
queryExecutionLog("info", "redis:start", { traceId, db: currentDb, commandCount: commands.length, sqlLength: sql.length });
const allResults: QueryResult[] = [];
const commandRanges = executableStatementRanges(sql, "redis");
const skipSafety = options?.skipRedisSafetyCheck;
let hadMutatingCommand = false;
for (const [commandIndex, command] of commands.entries()) {
+53
View File
@@ -6721,6 +6721,59 @@ test("redis multi-command execution records source statements for each result",
}
});
test("redis execution skips comment lines and keeps results aligned with their source ranges", async () => {
const restoreStorage = installMemoryStorage();
setActivePinia(createPinia());
const connectionStore = useConnectionStore();
const store = useQueryStore();
const originalFetch = globalThis.fetch;
const sentCommands: string[] = [];
connectionStore.addEphemeralConnection({
...conn("redis-1"),
db_type: "redis",
port: 6379,
});
globalThis.fetch = withConnectionHealthMock(async (input, init) => {
const url = String(input);
if (url === "/api/redis/execute-command") {
const body = JSON.parse(String(init?.body ?? "{}"));
sentCommands.push(body.command);
return new Response(JSON.stringify({ command: body.command, safety: "allowed", value: "OK" }), {
status: 200,
headers: { "Content-Type": "application/json" },
});
}
return new Response("unexpected request", { status: 500 });
});
try {
const sql = "# warm the cache\nGET user:1\n-- then check\n PING ";
const tabId = store.createTab("redis-1", "0", "Redis", "query", sql);
await store.executeTabSql(tabId, sql, { sourceOffset: 0 });
const tab = store.tabs.find((item) => item.id === tabId);
// A note is never sent to the server as a command.
assert.deepEqual(sentCommands, ["GET user:1", "PING"]);
assert.deepEqual(
tab?.results?.map((result) => result.sourceStatement),
["GET user:1", "PING"],
);
// Each result points at its own line, not the line of the comment above it.
assert.deepEqual(
tab?.results?.map((result) => [result.sourceFrom, result.sourceTo]),
[
[sql.indexOf("GET"), sql.indexOf("GET") + "GET user:1".length],
[sql.indexOf("PING"), sql.indexOf("PING") + "PING".length],
],
);
} finally {
globalThis.fetch = originalFetch;
restoreStorage();
}
});
test("mongo multi-command execution records source statements for error results", async () => {
const restoreStorage = installMemoryStorage();
setActivePinia(createPinia());