diff --git a/apps/desktop/src/components/redis/RedisKeyBrowser.vue b/apps/desktop/src/components/redis/RedisKeyBrowser.vue index 9d258955a..657eb6ec5 100644 --- a/apps/desktop/src/components/redis/RedisKeyBrowser.vue +++ b/apps/desktop/src/components/redis/RedisKeyBrowser.vue @@ -151,6 +151,11 @@ const dangerConfirmLabel = computed(() => { if (pendingDanger.value?.kind === "command") return t("dangerDialog.confirm"); return t("dangerDialog.deleteConfirm"); }); +const dangerMessage = computed(() => { + // Redis write commands such as SET/HSET are mutating but not necessarily delete operations. + if (pendingDanger.value?.kind === "command") return t("dangerDialog.redisCommandMessage"); + return t("dangerDialog.deleteMessage"); +}); const commandPrompt = computed(() => `db${commandDb.value}>`); const createKeyTypeOptions = computed<{ value: RedisCreateKeyType; label: string }[]>(() => [ { value: "string", label: "String" }, @@ -518,12 +523,11 @@ async function runRedisCommand(command: string) { // The db this command ran on — capture before nextRedisCommandDb() advances it. const executedDb = commandDb.value; commandDb.value = nextRedisCommandDb(commandDb.value, command, result.value); - if (result.safety === "confirm") { - await loadKeys(); - } // Drop the cached key-name completion for this db so the editor's autocomplete // reflects keys added/removed/renamed by SET/DEL/RENAME/... - if (isRedisMutatingCommand(command)) { + const mutatesKeys = isRedisMutatingCommand(command); + if (mutatesKeys) { + await loadKeys(); connectionStore.invalidateCompletionCache(props.connectionId, String(executedDb)); // Refresh the sidebar db key counts (INFO keyspace) so `dbN (count)` stays accurate // after the write. Fire-and-forget so the terminal stays responsive. @@ -1224,7 +1228,7 @@ defineExpose({ focusSearch }); - + diff --git a/apps/desktop/src/i18n/locales/en.ts b/apps/desktop/src/i18n/locales/en.ts index a2237ae2c..fd5b9008c 100644 --- a/apps/desktop/src/i18n/locales/en.ts +++ b/apps/desktop/src/i18n/locales/en.ts @@ -2223,6 +2223,7 @@ export default { dangerDialog: { title: "Dangerous Operation", message: "This SQL statement may modify or delete data irreversibly. Are you sure you want to execute it?", + redisCommandMessage: "This Redis command may modify data and cannot be undone automatically. Continue?", suppressFuturePrompts: "Do not ask again for dangerous SQL", wrapLines: "Toggle word wrap", deleteMessage: "This delete operation may be irreversible. Continue?", diff --git a/apps/desktop/src/i18n/locales/es.ts b/apps/desktop/src/i18n/locales/es.ts index 783afeba3..412f1739c 100644 --- a/apps/desktop/src/i18n/locales/es.ts +++ b/apps/desktop/src/i18n/locales/es.ts @@ -2155,6 +2155,7 @@ export default withEnglishFallback({ dangerDialog: { title: "Operación peligrosa", message: "Esta sentencia SQL puede modificar o eliminar datos de forma irreversible. ¿Estás seguro de que deseas ejecutarla?", + redisCommandMessage: "Este comando de Redis puede modificar datos y no se puede deshacer automáticamente. ¿Continuar?", suppressFuturePrompts: "No volver a preguntar para SQL peligroso", wrapLines: "Alternar ajuste de línea", deleteMessage: "Esta operación de eliminación puede ser irreversible. ¿Continuar?", diff --git a/apps/desktop/src/i18n/locales/it.ts b/apps/desktop/src/i18n/locales/it.ts index db7d82595..72ea71e38 100644 --- a/apps/desktop/src/i18n/locales/it.ts +++ b/apps/desktop/src/i18n/locales/it.ts @@ -2153,6 +2153,7 @@ export default withEnglishFallback({ dangerDialog: { title: "Operazione Pericolosa", message: "Questa istruzione SQL potrebbe modificare o eliminare i dati in modo irreversibile. Sei sicuro di volerla eseguire?", + redisCommandMessage: "Questo comando Redis può modificare i dati e non può essere annullato automaticamente. Continuare?", suppressFuturePrompts: "Non chiedere più per SQL pericolosi", wrapLines: "Attiva/disattiva ritorno a capo", deleteMessage: "Questa operazione di eliminazione potrebbe essere irreversibile. Continuare?", diff --git a/apps/desktop/src/i18n/locales/ja.ts b/apps/desktop/src/i18n/locales/ja.ts index fee2f0cf8..5650349bc 100644 --- a/apps/desktop/src/i18n/locales/ja.ts +++ b/apps/desktop/src/i18n/locales/ja.ts @@ -2153,6 +2153,7 @@ export default withEnglishFallback({ dangerDialog: { title: "危険な操作", message: "このSQL文はデータを不可逆的に変更または削除する可能性があります。実行してもよろしいですか?", + redisCommandMessage: "このRedisコマンドはデータを変更し、自動的に元に戻せない可能性があります。続行しますか?", suppressFuturePrompts: "危険なSQLの確認を今後表示しない", wrapLines: "折り返し表示を切り替え", deleteMessage: "この削除操作は元に戻せない可能性があります。続行しますか?", diff --git a/apps/desktop/src/i18n/locales/pt-BR.ts b/apps/desktop/src/i18n/locales/pt-BR.ts index d7c0f7a51..c7fa7c362 100644 --- a/apps/desktop/src/i18n/locales/pt-BR.ts +++ b/apps/desktop/src/i18n/locales/pt-BR.ts @@ -2154,6 +2154,7 @@ export default withEnglishFallback({ dangerDialog: { title: "Operação perigosa", message: "Esta instrução SQL pode modificar ou excluir dados de forma irreversível. Tem certeza de que deseja executá-la?", + redisCommandMessage: "Este comando Redis pode modificar dados e não pode ser desfeito automaticamente. Continuar?", suppressFuturePrompts: "Não perguntar novamente para SQL perigoso", wrapLines: "Alternar quebra de linha", deleteMessage: "Esta operação de exclusão pode ser irreversível. Continuar?", diff --git a/apps/desktop/src/i18n/locales/zh-CN.ts b/apps/desktop/src/i18n/locales/zh-CN.ts index 619c6d7eb..6a3a5958d 100644 --- a/apps/desktop/src/i18n/locales/zh-CN.ts +++ b/apps/desktop/src/i18n/locales/zh-CN.ts @@ -2223,6 +2223,7 @@ export default withEnglishFallback({ dangerDialog: { title: "危险操作", message: "此 SQL 语句可能不可逆地修改或删除数据,确认要执行吗?", + redisCommandMessage: "此 Redis 命令可能会修改数据,且无法自动撤销,确认要继续吗?", suppressFuturePrompts: "以后执行危险 SQL 不再提示", wrapLines: "切换自动换行", deleteMessage: "此删除操作可能不可逆,确认要继续吗?", diff --git a/apps/desktop/src/i18n/locales/zh-TW.ts b/apps/desktop/src/i18n/locales/zh-TW.ts index 40999e2d6..f56644cb0 100644 --- a/apps/desktop/src/i18n/locales/zh-TW.ts +++ b/apps/desktop/src/i18n/locales/zh-TW.ts @@ -2010,6 +2010,7 @@ export default withEnglishFallback({ dangerDialog: { title: "危險操作", message: "此 SQL 語句可能不可逆地修改或刪除資料,確認要執行嗎?", + redisCommandMessage: "此 Redis 命令可能會修改資料,且無法自動復原,確認要繼續嗎?", suppressFuturePrompts: "之後執行危險 SQL 不再提示", wrapLines: "切換自動換行", deleteMessage: "此刪除操作可能不可逆,確認要繼續嗎?", diff --git a/apps/desktop/src/lib/backend/tauri.ts b/apps/desktop/src/lib/backend/tauri.ts index 42d6353d7..bb212d765 100644 --- a/apps/desktop/src/lib/backend/tauri.ts +++ b/apps/desktop/src/lib/backend/tauri.ts @@ -1391,7 +1391,7 @@ export interface RedisScanResult { total_keys: number; } -export type RedisCommandSafety = "allowed" | "confirm" | "blocked"; +export type RedisCommandSafety = "allowed" | "write" | "confirm" | "blocked"; export interface RedisCommandResult { command: string; diff --git a/apps/desktop/src/lib/redis/redisCommandSafety.ts b/apps/desktop/src/lib/redis/redisCommandSafety.ts index 4d12c1827..88222e3b5 100644 --- a/apps/desktop/src/lib/redis/redisCommandSafety.ts +++ b/apps/desktop/src/lib/redis/redisCommandSafety.ts @@ -1,4 +1,4 @@ -export type RedisCommandSafety = "allowed" | "confirm" | "blocked"; +export type RedisCommandSafety = "allowed" | "write" | "confirm" | "blocked"; const BLOCKED_COMMANDS = new Set(["KEYS", "FLUSHALL", "SHUTDOWN", "CONFIG", "SAVE", "BGSAVE", "SLAVEOF", "REPLICAOF", "MIGRATE", "MODULE", "SCRIPT", "EVAL", "EVALSHA"]); @@ -9,30 +9,84 @@ const CONFIRM_COMMANDS = new Set([ "EXPIREAT", "PEXPIRE", "PEXPIREAT", - "PERSIST", "RENAME", "RENAMENX", + "GETDEL", + "HDEL", + "LPOP", + "RPOP", + "LREM", + "LTRIM", + "SPOP", + "SREM", + "ZREM", + "ZPOPMAX", + "ZPOPMIN", + "ZMPOP", + "ZREMRANGEBYLEX", + "ZREMRANGEBYRANK", + "ZREMRANGEBYSCORE", + "XDEL", + "XTRIM", + "MOVE", + "SORT", + "SDIFFSTORE", + "SINTERSTORE", + "SUNIONSTORE", + "ZDIFFSTORE", + "ZINTERSTORE", + "ZRANGESTORE", + "ZUNIONSTORE", + "PFMERGE", + "GEOSEARCHSTORE", + "FLUSHDB", +]); + +const WRITE_COMMANDS = new Set([ + "APPEND", + "BITFIELD", + "BITOP", + "COPY", + "DECR", + "DECRBY", + "GEOADD", + "GEORADIUS", + "GEORADIUSBYMEMBER", + "GETSET", + "INCR", + "INCRBY", + "INCRBYFLOAT", "SET", "SETEX", "PSETEX", "SETNX", + "SETRANGE", "MSET", "MSETNX", + "PERSIST", "HSET", - "HDEL", - "LPUSH", - "RPUSH", - "LPOP", - "RPOP", + "HMSET", + "HINCRBY", + "HINCRBYFLOAT", + "HSETNX", + "LINSERT", "LSET", - "LREM", + "LMOVE", + "LPUSH", + "LPUSHX", + "PFADD", + "RPUSH", + "RPUSHX", + "RESTORE", "SADD", - "SREM", "ZADD", - "ZREM", + "ZINCRBY", + "SETBIT", "XADD", - "XDEL", - "FLUSHDB", + "XACK", + "XAUTOCLAIM", + "XCLAIM", + "XSETID", ]); export function firstRedisCommandToken(command: string): string { @@ -64,5 +118,6 @@ export function classifyRedisCommandSafety(command: string): RedisCommandSafety const token = firstRedisCommandToken(command); if (BLOCKED_COMMANDS.has(token)) return "blocked"; if (CONFIRM_COMMANDS.has(token)) return "confirm"; + if (WRITE_COMMANDS.has(token)) return "write"; return "allowed"; } diff --git a/apps/desktop/src/lib/redis/redisCommandTable.ts b/apps/desktop/src/lib/redis/redisCommandTable.ts index 12373a5ca..81656fd7b 100644 --- a/apps/desktop/src/lib/redis/redisCommandTable.ts +++ b/apps/desktop/src/lib/redis/redisCommandTable.ts @@ -2,15 +2,17 @@ * Static Redis command metadata, distilled from the official Redis command table * (redis-doc `commands.json` / the `COMMAND` output). Kept offline and lightweight: * we only retain what the editor syntax diagnostics need — `arity` and `group` — - * plus a `safety` hint for danger/write-command highlighting. + * plus a `safety` hint for command gating and write-command highlighting. * * Arity semantics (matches the Redis `COMMAND` reply, command name INCLUDED in the count): * arity > 0 → the command takes exactly `arity` tokens (e.g. GET key → arity 2). * arity < 0 → the command takes AT LEAST `-arity` tokens (e.g. MSET k v [k v ...] → arity -3). * * `safety` is aligned with the token-level sets in `redisCommandSafety.ts` (used to gate - * execution). Here it is recorded per-command (including subcommands) so diagnostics can - * be more precise than the first-token classification. + * execution). `write` commands mutate data without a confirmation prompt; `confirm` + * commands are destructive/structural enough to ask first. Here it is recorded + * per-command (including subcommands) so diagnostics can be more precise than the + * first-token classification. * * Subcommands are keyed as `"MAIN SUB"` in UPPER CASE (e.g. `"CONFIG GET"`, `"XGROUP CREATE"`). */ @@ -27,6 +29,57 @@ export interface RedisCommandSpec { type Spec = [arity: number, group: string, safety?: RedisCommandSafety]; +const WRITE_WITHOUT_CONFIRM = new Set([ + "APPEND", + "BITFIELD", + "BITOP", + "CLIENT SETINFO", + "CLIENT SETNAME", + "COPY", + "DECR", + "DECRBY", + "GEOADD", + "GEORADIUS", + "GEORADIUSBYMEMBER", + "GETSET", + "HINCRBY", + "HINCRBYFLOAT", + "HMSET", + "HSET", + "HSETNX", + "INCR", + "INCRBY", + "INCRBYFLOAT", + "LINSERT", + "LMOVE", + "LPUSH", + "LPUSHX", + "MSET", + "MSETNX", + "PERSIST", + "PFADD", + "PSETEX", + "RESTORE", + "RPUSH", + "RPUSHX", + "SADD", + "SET", + "SETBIT", + "SETEX", + "SETNX", + "SETRANGE", + "XACK", + "XADD", + "XAUTOCLAIM", + "XCLAIM", + "XGROUP CREATE", + "XGROUP CREATECONSUMER", + "XGROUP SETID", + "XSETID", + "ZADD", + "ZINCRBY", +]); + // Compact tuple form → expanded into the record below. const RAW_COMMANDS: Record = { // ---- String ---- @@ -367,7 +420,12 @@ const RAW_COMMANDS: Record = { READWRITE: [1, "cluster"], }; -export const REDIS_COMMAND_TABLE: Record = Object.fromEntries(Object.entries(RAW_COMMANDS).map(([name, [arity, group, safety]]) => [name.toUpperCase(), { arity, group, safety: safety ?? "allowed" }])); +function commandTableSafety(name: string, safety?: RedisCommandSafety): RedisCommandSafety { + if (safety === "confirm" && WRITE_WITHOUT_CONFIRM.has(name.toUpperCase())) return "write"; + return safety ?? "allowed"; +} + +export const REDIS_COMMAND_TABLE: Record = Object.fromEntries(Object.entries(RAW_COMMANDS).map(([name, [arity, group, safety]]) => [name.toUpperCase(), { arity, group, safety: commandTableSafety(name, safety) }])); /** * Resolve a command spec. Handles two-token subcommands (e.g. `XGROUP CREATE`, @@ -442,7 +500,7 @@ const NON_MUTATING_BLOCKED = new Set(["KEYS", "BGSAVE", "SAVE", "SHUTDOWN", "REP * completion for that db is potentially stale and should be dropped. * * Mapping from the diagnostic `safety` field: - * - `confirm` → every write command mutates keys (SET/DEL/INCR/HSET/...). + * - `write`/`confirm` → every write command mutates keys (SET/DEL/INCR/HSET/...). * - `blocked` → mostly destructive/admin; we keep the ones that may touch keys * (FLUSHALL, MIGRATE, EVAL[ESHA]) and exclude the read-only/admin * ones in `NON_MUTATING_BLOCKED`. @@ -455,7 +513,7 @@ export function isRedisMutatingCommand(command: string): boolean { const argv = firstRedisArgvUpper(command); const spec = resolveRedisCommandSpec(argv); if (!spec) return false; - if (spec.safety === "confirm") return true; + if (spec.safety === "write" || spec.safety === "confirm") return true; if (spec.safety === "blocked") { return !NON_MUTATING_BLOCKED.has(argv[0]); } diff --git a/apps/desktop/src/lib/redis/redisSyntaxDiagnostics.ts b/apps/desktop/src/lib/redis/redisSyntaxDiagnostics.ts index 9500d4690..688e5cbc5 100644 --- a/apps/desktop/src/lib/redis/redisSyntaxDiagnostics.ts +++ b/apps/desktop/src/lib/redis/redisSyntaxDiagnostics.ts @@ -196,7 +196,7 @@ export function buildRedisSyntaxDiagnostics(source: string): SqlSemanticDiagnost } else if (spec.safety === "confirm") { diagnostics.push({ span: lineSpan(lineNo, commandToken.startColumn, commandToken.endColumn), - message: `Write command '${argv[0].value}' — will modify data`, + message: `Dangerous command '${argv[0].value}' — confirmation recommended`, severity: "warning", }); } diff --git a/crates/dbx-core/src/db/redis_driver.rs b/crates/dbx-core/src/db/redis_driver.rs index 8b6b585f6..2b150c26d 100644 --- a/crates/dbx-core/src/db/redis_driver.rs +++ b/crates/dbx-core/src/db/redis_driver.rs @@ -185,6 +185,7 @@ pub enum RedisCollectionPage { #[serde(rename_all = "snake_case")] pub enum RedisCommandSafety { Allowed, + Write, Confirm, Blocked, } @@ -1331,11 +1332,16 @@ pub fn classify_command(command: &str) -> RedisCommandSafety { match command.to_ascii_uppercase().as_str() { "KEYS" | "FLUSHALL" | "SHUTDOWN" | "CONFIG" | "SAVE" | "BGSAVE" | "SLAVEOF" | "REPLICAOF" | "MIGRATE" | "MODULE" | "SCRIPT" | "EVAL" | "EVALSHA" => RedisCommandSafety::Blocked, - "DEL" | "UNLINK" | "EXPIRE" | "EXPIREAT" | "PEXPIRE" | "PEXPIREAT" | "PERSIST" | "RENAME" | "RENAMENX" - | "SET" | "SETEX" | "PSETEX" | "SETNX" | "MSET" | "MSETNX" | "HSET" | "HDEL" | "LPUSH" | "RPUSH" | "LPOP" - | "RPOP" | "LSET" | "LREM" | "SADD" | "SREM" | "ZADD" | "ZREM" | "XADD" | "XDEL" | "FLUSHDB" => { - RedisCommandSafety::Confirm - } + "DEL" | "UNLINK" | "EXPIRE" | "EXPIREAT" | "PEXPIRE" | "PEXPIREAT" | "RENAME" | "RENAMENX" | "GETDEL" + | "HDEL" | "LPOP" | "RPOP" | "LREM" | "LTRIM" | "SPOP" | "SREM" | "ZREM" | "ZPOPMAX" | "ZPOPMIN" | "ZMPOP" + | "ZREMRANGEBYLEX" | "ZREMRANGEBYRANK" | "ZREMRANGEBYSCORE" | "XDEL" | "XTRIM" | "MOVE" | "SORT" + | "SDIFFSTORE" | "SINTERSTORE" | "SUNIONSTORE" | "ZDIFFSTORE" | "ZINTERSTORE" | "ZRANGESTORE" + | "ZUNIONSTORE" | "PFMERGE" | "GEOSEARCHSTORE" | "FLUSHDB" => RedisCommandSafety::Confirm, + "APPEND" | "BITFIELD" | "BITOP" | "COPY" | "DECR" | "DECRBY" | "GEOADD" | "GEORADIUS" | "GEORADIUSBYMEMBER" + | "GETSET" | "INCR" | "INCRBY" | "INCRBYFLOAT" | "SET" | "SETEX" | "PSETEX" | "SETNX" | "SETRANGE" | "MSET" + | "MSETNX" | "PERSIST" | "HSET" | "HMSET" | "HINCRBY" | "HINCRBYFLOAT" | "HSETNX" | "LINSERT" | "LSET" + | "LMOVE" | "LPUSH" | "LPUSHX" | "PFADD" | "RPUSH" | "RPUSHX" | "RESTORE" | "SADD" | "ZADD" | "ZINCRBY" + | "SETBIT" | "XADD" | "XACK" | "XAUTOCLAIM" | "XCLAIM" | "XSETID" => RedisCommandSafety::Write, _ => RedisCommandSafety::Allowed, } } @@ -2909,7 +2915,9 @@ mod tests { #[test] fn classifies_safe_confirmed_and_blocked_commands() { assert_eq!(classify_command("GET"), RedisCommandSafety::Allowed); - assert_eq!(classify_command("set"), RedisCommandSafety::Confirm); + assert_eq!(classify_command("set"), RedisCommandSafety::Write); + assert_eq!(classify_command("hset"), RedisCommandSafety::Write); + assert_eq!(classify_command("del"), RedisCommandSafety::Confirm); assert_eq!(classify_command("flushdb"), RedisCommandSafety::Confirm); assert_eq!(classify_command("KEYS"), RedisCommandSafety::Blocked); assert_eq!(classify_command("flushall"), RedisCommandSafety::Blocked); diff --git a/packages/app-tests/redisCommandSafety.test.ts b/packages/app-tests/redisCommandSafety.test.ts new file mode 100644 index 000000000..0b4e8b776 --- /dev/null +++ b/packages/app-tests/redisCommandSafety.test.ts @@ -0,0 +1,13 @@ +import assert from "node:assert/strict"; +import { test } from "vitest"; +import { classifyRedisCommandSafety } from "../../apps/desktop/src/lib/redis/redisCommandSafety.ts"; + +test("classifies normal Redis writes separately from destructive commands", () => { + assert.equal(classifyRedisCommandSafety("SET session:1 value"), "write"); + assert.equal(classifyRedisCommandSafety("HSET hash field value"), "write"); + assert.equal(classifyRedisCommandSafety("LPUSH queue value"), "write"); + assert.equal(classifyRedisCommandSafety("DEL session:1"), "confirm"); + assert.equal(classifyRedisCommandSafety("FLUSHDB"), "confirm"); + assert.equal(classifyRedisCommandSafety("KEYS *"), "blocked"); + assert.equal(classifyRedisCommandSafety("GET session:1"), "allowed"); +}); diff --git a/packages/app-tests/redisCommandTable.test.ts b/packages/app-tests/redisCommandTable.test.ts index ab3825ec8..8be1ef303 100644 --- a/packages/app-tests/redisCommandTable.test.ts +++ b/packages/app-tests/redisCommandTable.test.ts @@ -53,8 +53,16 @@ test("unknown / empty commands are treated as non-mutating (no cache thrash)", ( test("resolveRedisCommandSpec resolves subcommand then main", () => { const sub = resolveRedisCommandSpec(["XGROUP", "CREATE"]); assert.ok(sub); - assert.equal(sub?.safety, "confirm"); + assert.equal(sub?.safety, "write"); const main = resolveRedisCommandSpec(["GET"]); assert.ok(main); assert.equal(main?.group, "string"); }); + +test("normal writes do not require confirmation but destructive commands do", () => { + assert.equal(resolveRedisCommandSpec(["SET"])?.safety, "write"); + assert.equal(resolveRedisCommandSpec(["HSET"])?.safety, "write"); + assert.equal(resolveRedisCommandSpec(["LPUSH"])?.safety, "write"); + assert.equal(resolveRedisCommandSpec(["DEL"])?.safety, "confirm"); + assert.equal(resolveRedisCommandSpec(["FLUSHDB"])?.safety, "confirm"); +}); diff --git a/packages/app-tests/redisSyntaxDiagnostics.test.ts b/packages/app-tests/redisSyntaxDiagnostics.test.ts index 578d87f09..a04799504 100644 --- a/packages/app-tests/redisSyntaxDiagnostics.test.ts +++ b/packages/app-tests/redisSyntaxDiagnostics.test.ts @@ -37,13 +37,11 @@ test("flags wrong arity (too many arguments)", () => { }); test("respects variable arity (negative arity is a minimum)", () => { - // MSET arity -3 → at least 2 args; 2 args (3 tokens) is valid (still flagged write-warning). + // MSET arity -3 → at least 2 args; 2 args (3 tokens) is valid. const ok3 = buildRedisSyntaxDiagnostics("MSET k1 v1"); - assert.equal(ok3.length, 1); - assert.equal(ok3[0].severity, "warning"); + assert.equal(ok3.length, 0); const ok4 = buildRedisSyntaxDiagnostics("MSET k1 v1 k2 v2"); - assert.equal(ok4.length, 1); - assert.equal(ok4[0].severity, "warning"); + assert.equal(ok4.length, 0); // Too few → arity error takes precedence. const diags = buildRedisSyntaxDiagnostics("MSET k1"); assert.equal(diags.length, 1); @@ -58,11 +56,11 @@ test("flags unclosed quote and spans to end of line", () => { assert.equal(diags[0].span.start_column, 5); // quote starts at column 5 }); -test("highlights write commands as warning", () => { +test("highlights destructive commands as warning", () => { const diags = buildRedisSyntaxDiagnostics("DEL x"); assert.equal(diags.length, 1); assert.equal(diags[0].severity, "warning"); - assert.match(diags[0].message, /Write command 'DEL'/); + assert.match(diags[0].message, /Dangerous command 'DEL'/); }); test("highlights flushall as blocked error but flushdb as confirm warning", () => { @@ -75,22 +73,19 @@ test("highlights flushall as blocked error but flushdb as confirm warning", () = const db = buildRedisSyntaxDiagnostics("FLUSHDB"); assert.equal(db.length, 1); assert.equal(db[0].severity, "warning"); - assert.match(db[0].message, /Write command 'FLUSHDB'/); + assert.match(db[0].message, /Dangerous command 'FLUSHDB'/); }); test("subcommands resolve via MAIN SUB key", () => { - // XGROUP CREATE stream group id MKSTREAM → 6 tokens, satisfies arity -6, confirm → warning. + // XGROUP CREATE stream group id MKSTREAM → 6 tokens, satisfies arity -6. const diags = buildRedisSyntaxDiagnostics("XGROUP CREATE stream group $ MKSTREAM"); - assert.equal(diags.length, 1); - assert.equal(diags[0].severity, "warning"); - assert.match(diags[0].message, /Write command 'XGROUP'/); + assert.equal(diags.length, 0); }); test("treats command names case-insensitively", () => { assert.deepEqual(messages("get foo"), []); const setDiag = buildRedisSyntaxDiagnostics("Set a b"); - assert.equal(setDiag.length, 1); - assert.equal(setDiag[0].severity, "warning"); + assert.equal(setDiag.length, 0); const diags = buildRedisSyntaxDiagnostics("flushall"); assert.match(diags[0].message, /Blocked command 'flushall'/); }); @@ -127,11 +122,17 @@ test("shouldRunRedisDiagnostics waits while typing the command name", () => { test("tokenizeRedisLine preserves quoted values with spaces", () => { const { argv, unclosedQuote } = tokenizeRedisLine('SET mykey "hello world"'); assert.equal(unclosedQuote, false); - assert.deepEqual(argv.map((t) => t.value), ["SET", "mykey", "hello world"]); + assert.deepEqual( + argv.map((t) => t.value), + ["SET", "mykey", "hello world"], + ); assert.equal(argv[2]!.startColumn, 11); }); test("tokenizeRedisLine handles backslash escapes", () => { - const { argv } = tokenizeRedisLine('SET k a\\ b'); - assert.deepEqual(argv.map((t) => t.value), ["SET", "k", "a b"]); + const { argv } = tokenizeRedisLine("SET k a\\ b"); + assert.deepEqual( + argv.map((t) => t.value), + ["SET", "k", "a b"], + ); }); diff --git a/packages/mcp-server/tests/server.test.ts b/packages/mcp-server/tests/server.test.ts index 9127d8c24..18ca73b3d 100644 --- a/packages/mcp-server/tests/server.test.ts +++ b/packages/mcp-server/tests/server.test.ts @@ -245,7 +245,7 @@ test("redis command tool blocks write commands in read-only MCP sessions", async findConnection: async () => redisConnection, executeRedisCommand: async () => { executed = true; - return { command: "SET", safety: "confirm", value: "OK" }; + return { command: "SET", safety: "write", value: "OK" }; }, }; diff --git a/packages/node-core/src/redis-command.ts b/packages/node-core/src/redis-command.ts index 443bd3cc5..e5801eb00 100644 --- a/packages/node-core/src/redis-command.ts +++ b/packages/node-core/src/redis-command.ts @@ -1,6 +1,6 @@ import type { SqlSafetyOptions } from "./sql-safety.js"; -export type RedisCommandSafety = "allowed" | "confirm" | "blocked"; +export type RedisCommandSafety = "allowed" | "write" | "confirm" | "blocked"; export interface RedisCommandResult { command: string; @@ -21,21 +21,7 @@ export interface RedisCommandSafetyDecision { skipSafetyCheck?: boolean; } -const BLOCKED_REDIS_COMMANDS = new Set([ - "KEYS", - "FLUSHALL", - "SHUTDOWN", - "CONFIG", - "SAVE", - "BGSAVE", - "SLAVEOF", - "REPLICAOF", - "MIGRATE", - "MODULE", - "SCRIPT", - "EVAL", - "EVALSHA", -]); +const BLOCKED_REDIS_COMMANDS = new Set(["KEYS", "FLUSHALL", "SHUTDOWN", "CONFIG", "SAVE", "BGSAVE", "SLAVEOF", "REPLICAOF", "MIGRATE", "MODULE", "SCRIPT", "EVAL", "EVALSHA"]); const CONFIRM_REDIS_COMMANDS = new Set([ "DEL", @@ -44,30 +30,84 @@ const CONFIRM_REDIS_COMMANDS = new Set([ "EXPIREAT", "PEXPIRE", "PEXPIREAT", - "PERSIST", "RENAME", "RENAMENX", + "GETDEL", + "HDEL", + "LPOP", + "RPOP", + "LREM", + "LTRIM", + "SPOP", + "SREM", + "ZREM", + "ZPOPMAX", + "ZPOPMIN", + "ZMPOP", + "ZREMRANGEBYLEX", + "ZREMRANGEBYRANK", + "ZREMRANGEBYSCORE", + "XDEL", + "XTRIM", + "MOVE", + "SORT", + "SDIFFSTORE", + "SINTERSTORE", + "SUNIONSTORE", + "ZDIFFSTORE", + "ZINTERSTORE", + "ZRANGESTORE", + "ZUNIONSTORE", + "PFMERGE", + "GEOSEARCHSTORE", + "FLUSHDB", +]); + +const WRITE_REDIS_COMMANDS = new Set([ + "APPEND", + "BITFIELD", + "BITOP", + "COPY", + "DECR", + "DECRBY", + "GEOADD", + "GEORADIUS", + "GEORADIUSBYMEMBER", + "GETSET", + "INCR", + "INCRBY", + "INCRBYFLOAT", "SET", "SETEX", "PSETEX", "SETNX", + "SETRANGE", "MSET", "MSETNX", + "PERSIST", "HSET", - "HDEL", - "LPUSH", - "RPUSH", - "LPOP", - "RPOP", + "HMSET", + "HINCRBY", + "HINCRBYFLOAT", + "HSETNX", + "LINSERT", "LSET", - "LREM", + "LMOVE", + "LPUSH", + "LPUSHX", + "PFADD", + "RPUSH", + "RPUSHX", + "RESTORE", "SADD", - "SREM", "ZADD", - "ZREM", + "ZINCRBY", + "SETBIT", "XADD", - "XDEL", - "FLUSHDB", + "XACK", + "XAUTOCLAIM", + "XCLAIM", + "XSETID", ]); export function firstRedisCommandToken(commandText: string): string | undefined { @@ -84,6 +124,7 @@ export function classifyRedisCommand(commandText: string): RedisCommandSafety { if (!command) return "blocked"; if (BLOCKED_REDIS_COMMANDS.has(command)) return "blocked"; if (CONFIRM_REDIS_COMMANDS.has(command)) return "confirm"; + if (WRITE_REDIS_COMMANDS.has(command)) return "write"; return "allowed"; } @@ -124,7 +165,7 @@ export function parseRedisCommandArgv(commandText: string): string[] { const trimmed = commandText.trimEnd().replace(/;+$/, ""); const argv: string[] = []; let current = ""; - let quote: "\"" | "'" | undefined; + let quote: '"' | "'" | undefined; let escaping = false; for (const ch of trimmed) { @@ -148,7 +189,7 @@ export function parseRedisCommandArgv(commandText: string): string[] { continue; } - if (ch === "\"" || ch === "'") { + if (ch === '"' || ch === "'") { quote = ch; continue; } diff --git a/packages/node-core/tests/redis-command.test.ts b/packages/node-core/tests/redis-command.test.ts index 28c497eac..cf1cff18f 100644 --- a/packages/node-core/tests/redis-command.test.ts +++ b/packages/node-core/tests/redis-command.test.ts @@ -14,7 +14,8 @@ test("parseRedisCommandArgv handles quoted values and escapes", () => { test("classifyRedisCommand mirrors DBX redis command safety classes", () => { assert.equal(classifyRedisCommand("GET session:1"), "allowed"); - assert.equal(classifyRedisCommand("SET session:1 value"), "confirm"); + assert.equal(classifyRedisCommand("SET session:1 value"), "write"); + assert.equal(classifyRedisCommand("DEL session:1"), "confirm"); assert.equal(classifyRedisCommand("KEYS *"), "blocked"); }); diff --git a/packages/node-core/tests/redis-direct.test.ts b/packages/node-core/tests/redis-direct.test.ts index a16350293..c34ec4a9c 100644 --- a/packages/node-core/tests/redis-direct.test.ts +++ b/packages/node-core/tests/redis-direct.test.ts @@ -99,7 +99,10 @@ test("executeRedisCommand runs standalone redis commands without the DBX bridge" assert.deepEqual(result, { command: "GET", safety: "allowed", value: "value-1" }); const dataCommands = seen.filter((request) => request.command !== "CLIENT"); - assert.deepEqual(dataCommands.map((request) => request.command), ["SELECT", "GET"]); + assert.deepEqual( + dataCommands.map((request) => request.command), + ["SELECT", "GET"], + ); assert.deepEqual(dataCommands[0].args, ["2"]); }, ); @@ -113,13 +116,13 @@ test("executeRedisCommand parses quoted arguments and JSON bulk replies", async assert.deepEqual(request.args, ["session:1", "hello world"]); return "+OK\r\n"; } - return bulk("{\"ok\":true}"); + return bulk('{"ok":true}'); }, async (port) => { const set = await executeRedisCommand(redisConnection(port), 0, 'SET session:1 "hello world"'); const get = await executeRedisCommand(redisConnection(port), 0, "GET session:1"); - assert.deepEqual(set, { command: "SET", safety: "confirm", value: "OK" }); + assert.deepEqual(set, { command: "SET", safety: "write", value: "OK" }); assert.deepEqual(get, { command: "GET", safety: "allowed", value: { ok: true } }); }, ); @@ -129,7 +132,7 @@ test("executeRedisCommand keeps blocked redis commands behind skipSafetyCheck", await withRedisServer( (request) => { if (request.command === "CLIENT") return "+OK\r\n"; - return bulk("[\"session:1\"]"); + return bulk('["session:1"]'); }, async (port) => { await assert.rejects(() => executeRedisCommand(redisConnection(port), 0, "KEYS *"), /blocked for safety/);