Skip to content

RI-8187 Workbench dangerous-command gating via type-to-confirm#5964

Merged
KrumTy merged 5 commits into
mainfrom
fe/feature/RI-8187/workbench-dangerous-type-to-confirm
May 22, 2026
Merged

RI-8187 Workbench dangerous-command gating via type-to-confirm#5964
KrumTy merged 5 commits into
mainfrom
fe/feature/RI-8187/workbench-dangerous-type-to-confirm

Conversation

@KrumTy

@KrumTy KrumTy commented May 22, 2026

Copy link
Copy Markdown
Contributor

What

Gates dangerous Redis commands (ACL CAT dangerous) in the Workbench multi-command runner behind a TypeToConfirmModal on production-environment connections. The dangerous-commands list is already populated on connect by #5962 (RI-8186) — this PR just consumes it via useDatabaseEnvironment().isDangerousCommand.

When any command in the submitted batch is dangerous, WBViewWrapper.sourceValueSubmit holds the batch and opens the modal. The user types the database name (or host:port fallback) to confirm; cancel aborts the batch; confirm dispatches the original sendWbQueryAction unchanged. Non-production passes through with zero behavior change because the hook short-circuits to false.

The modal body lists the dangerous commands found in the batch (comma-joined) so users see exactly what they're authorizing, e.g.:

"You're about to run FLUSHALL, FLUSHDB against the production database prod-db."

Screenshot 2026-05-22 at 12 42 11 (copy is subject to change)

Design notes

  • One modal per batch (vs one-per-dangerous-command in the original ticket text). Simpler UX, listing the commands in the body keeps the user informed.
  • Modal lives in WBViewWrapper, matching the sibling pattern in #5962 (CLI gating in CliBodyWrapper) and #5963 (bulk delete gating in BulkDeleteFooter). Each surface has different post-confirm wiring, so a shared component would add indirection without payoff.
  • No FE telemetry — BE WORKBENCH_COMMAND_EXECUTED event enriched by RI-8196 (#5930, merged) carries environment + dangerous flags.

Dependencies / siblings

  • Depends on: RI-8198 (hook + slice action), RI-8186 / #5962 (on-connect dangerous-commands fetch)
  • Sibling: #5962 (CLI gating — same hook, same modal), #5963 (bulk delete gating)

Testing

  • node node_modules/.bin/jest redisinsight/ui/src/pages/workbench/components/wb-view/WBViewWrapper.spec.tsx -c jest.config.cjs — 11/11 pass (4 new gating tests + the host:port fallback case + existing).
  • Broader workbench area: 170/172 pass (2 pre-existing skips).
  • yarn lint:ui — clean.

Manual smoke (yarn dev:desktop)

  • Connect to a DB marked Production (requires devProdMode flag on). Open Workbench:
    • PING → runs immediately, no modal.
    • FLUSHALL → modal opens, names the command and the DB; cancel → not dispatched; confirm with DB name → runs.
    • PING\nFLUSHDB\nSET x 1 → one modal listing FLUSHDB; cancel → no commands run; confirm → whole batch dispatched.
    • FLUSHALL ASYNC → still gated (verb extracted before dangerous-command check).
  • Connect to a Development-marked database → dangerous commands run without modal.
  • With devProdMode flag off → dangerous commands run without modal regardless of environment.

Closes #RI-8187


Note

Medium Risk
Changes Workbench query submission flow to defer execution behind a confirmation modal when potentially destructive commands are detected, which could block or alter multi-command execution behavior if parsing/gating is incorrect.

Overview
Adds dangerous-command gating to the Workbench multi-command runner: WBViewWrapper now parses the submitted batch (getCommandsForExecution) and, if any command verb is flagged by useDatabaseEnvironment().isDangerousCommand, defers dispatch and shows a TypeToConfirmModal requiring the user to type the DB name (or host:port) to proceed.

On confirm, the original batch is dispatched unchanged; on cancel, nothing is sent. Updates WBViewWrapper.spec.tsx with new tests covering non-dangerous pass-through, modal gating for multi-command and Monaco continuation cases, confirm/cancel behavior, and the host:port fallback.

Reviewed by Cursor Bugbot for commit 72757c0. Bugbot is set up for automated code reviews on this repo. Configure here.

@KrumTy KrumTy requested a review from a team as a code owner May 22, 2026 09:27
@KrumTy KrumTy added the AI-Made label May 22, 2026
@jit-ci

jit-ci Bot commented May 22, 2026

Copy link
Copy Markdown

🛡️ Jit Security Scan Results

CRITICAL HIGH MEDIUM

✅ No security findings were detected in this PR


Security scan by Jit

KrumTy added 2 commits May 22, 2026 12:36
Wires the Workbench multi-command runner through `TypeToConfirmModal`
when the connected database is classified as `Environment.Production`
(via `useDatabaseEnvironment`). If any command in the submitted batch
is dangerous, the batch is held and the modal opens; the user types
the database name (or `host:port`) to confirm. Cancel aborts the
batch; confirm dispatches the original `sendWbQueryAction` unchanged.
Non-production environments pass through with no behavior change
because `isDangerousCommand` short-circuits to `false`.

The dangerous commands found in the batch are listed (comma-joined)
in the modal body so users see exactly what they're authorizing.

No FE telemetry — BE `WORKBENCH_COMMAND_EXECUTED` event enriched by
RI-8196 (PR #5930) carries the `environment` and `dangerous` flags.

References: #RI-8187
Hoists the name-or-host:port expression so the modal's confirmationText
and actionDescription reference a single source. Mirrors the pattern in
PR #5962 (CliBodyWrapper).

References: #RI-8187
@KrumTy KrumTy force-pushed the fe/feature/RI-8187/workbench-dangerous-type-to-confirm branch from 2bcd631 to f2eb3ad Compare May 22, 2026 09:38
Comment thread redisinsight/ui/src/pages/workbench/components/wb-view/WBViewWrapper.tsx Outdated
@github-actions

github-actions Bot commented May 22, 2026

Copy link
Copy Markdown
Contributor

Code Coverage - Frontend unit tests

St.
Category Percentage Covered / Total
🟢 Statements 82.89% 24745/29854
🟡 Branches 68.13% 10396/15258
🟡 Functions 78.11% 6686/8560
🟢 Lines 83.35% 24162/28990

Test suite run success

6976 tests passing in 802 suites.

Report generated by 🧪jest coverage report action from 72757c0

sourceValueSubmit was checking dangerousness against effectiveValue
(value || script) but passing the raw value to runSubmission on the
safe path. If value was an empty string while script held content,
the gate would correctly run on script but the safe path would
silently drop the commands via handleSubmit's !commandInit?.length
guard. Now both branches use effectiveValue.

References: #RI-8187

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit ad44c72. Configure here.

cmd.split(' ')[0] only handles single-space separation. getMonacoLines
splits on \n(?=[^\s]), so multi-line continuation commands like
'FLUSHALL\n  ASYNC' arrive as a single entry with embedded newlines.
Splitting on ' ' yielded 'FLUSHALL\n' (with trailing newline), which
failed the isDangerousCommand set lookup and bypassed the gate.
Using /\s+/ extracts the bare verb.

References: #RI-8187
Comment thread redisinsight/ui/src/pages/workbench/components/wb-view/WBViewWrapper.spec.tsx Outdated
Replaces ;(X as jest.Mock).foo() lines with named mock-handle consts
(sendWbQueryActionMock, useDatabaseEnvironmentMock,
connectedInstanceSelectorMock) so call sites no longer need the
leading-semicolon dance to avoid ASI ambiguity.

References: #RI-8187
@KrumTy KrumTy requested a review from valkirilov May 22, 2026 12:11
@KrumTy KrumTy merged commit 8c2cf11 into main May 22, 2026
20 checks passed
@KrumTy KrumTy deleted the fe/feature/RI-8187/workbench-dangerous-type-to-confirm branch May 22, 2026 12:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants