feat: remote code review for Warpified SSH sessions + security hardening#317
Open
TranscriptionFactory wants to merge 17 commits into
Open
Conversation
Backport upstream's shell_quote_arg() (a thin wrapper over the existing shell_escape_single_quotes) plus shared_tests.rs coverage, and re-export it through command_executor so session::shell_quote_arg resolves. Prerequisite for the display-chip RCE (warpdotdev #25398) and the is_file_path/is_git_repository escaping (warpdotdev #26138) fixes, which need shell-aware argument quoting threaded through the session shell type. No telemetry/auth involved. (cherry picked from commit f889750)
Agent Mode's is_file_path and is_git_repository built shell commands by
interpolating untrusted path strings into double-quoted "{path}", which
allowed command substitution / quote-breakout injection on the session's
shell (test -f, Test-Path, git -C).
Extract build_is_file_path_command / build_is_git_repository_command and
quote the path via shell_quote_arg(path, shell_type) so path-like input is
treated as a single argument across POSIX shells, PowerShell, and fish.
Port: warpdotdev/master b6caa95. Tests ported to execute_tests.rs
(external test-file convention matches this module). No telemetry/auth.
(cherry picked from commit 0ee900d)
Remote SSH session flows interpolated untrusted, parsed paths into shell
commands without escaping single quotes, letting a malicious path inject
commands that Warp auto-runs:
* read_history_from_file built "cat {history_file}" (unquoted)
* RemoteCommandExecutor built "cd '{current_directory_path}' && "
without escaping embedded single quotes
Quote both via the shell-aware shell_escape_single_quotes(.., shell_type).
The history command is extracted into Session::build_read_history_command
so the escaping has a unit-tested seam.
Port: warpdotdev/master 88c344e. Upstream's integration test relied on
SessionInfo builders absent in this fork, so the regression coverage was
adapted to session_test.rs targeting the extracted helper. No telemetry/auth.
Advisories: GHSA-xpw4-xmjx-f6gc, GHSA-qqpc-wvvw-4269, GHSA-9w2v-jhww-vm85
(cherry picked from commit 57fc910)
…CE (#25398) Display chips built shell commands by interpolating untrusted values straight into command strings: git checkout was fully unquoted, nvm use was unquoted, and cd used a bash-only escape — so a crafted branch name, node version, or directory could inject arbitrary shell commands (RCE). Replace eager String rendering with a PromptChipShellCommand intent enum (GitCheckout / ChangeDirectory / NvmUse / NvmInstallLatestNode) that is rendered to a concrete command only in Input::handle_prompt_event, where the active session's shell type is known. render_prompt_chip_shell_command quotes every interpolated argument via shell_quote_arg, so values are shell-aware single arguments across bash/zsh/fish/PowerShell. Removed the now-unused format_git_branch_command / format_change_directory_command. Port: warpdotdev/master 4295ec0, adapted to this fork (no worktree / CreateGitBranch / Echo paths exist here, so those variants are omitted). Tests ported to input_test.rs. Advisory: GHSA-hgvx-4xvm-39pw. No telemetry/auth. (cherry picked from commit 4608456)
… (#25311, #26091)
Incoming URIs routed to the app can carry secrets in their query string
(e.g. a Firebase refresh_token on warp://auth/desktop_redirect?...). Two
sites logged them:
* UriServiceImpl::handle_request logged the full Vec<Url> via log::info!
on every channel (Windows URI service) — deleted.
* handle_incoming_uri logged scheme/host/path via safe_url_log_fields on
release; tightened to log no URL fields at all (full URL stays dogfood-
only), and removed the now-unused safe_url_log_fields helper.
Port: warpdotdev/master e566a6c + a7f668e. The auth-UI log deletions in
#26091 are N-A here (this fork removed the cloud login surfaces). No
telemetry/auth gating added.
(cherry picked from commit b69d525)
…#25261) The iTerm2 File=/MultipartFile= OSC handler wrote non-inline payloads to disk under the active block's CWD using an attacker-controlled filename (end_iterm_image_receiving -> save_as_file), letting a malicious server overwrite arbitrary files in the user's working directory. The ITermImages feature flag only gates inline-image rendering and does NOT cover this write path. Remove the disk-write branch: non-inline iTerm file payloads are now ignored with a warning; only inline images are still decoded/rendered. Port: warpdotdev/master f3b9ce1. Tests ported to terminal_model_test.rs (single-part + multipart non-inline payloads don't overwrite a CWD file; inline image still decodes). No telemetry/auth. (cherry picked from commit 0a5df5b)
…knowns (#25353) Clicking a link in rendered markdown could open files via SystemDefault/ SystemGeneric, handing extensionless/unknown files (e.g. a disguised executable) to the OS default handler — arbitrary code execution. open_file now: keeps inline images on SystemGeneric; emits OpenFileWithTarget only for safe in-app/editor targets (MarkdownViewer, CodeEditor, ImageViewer, ExternalEditor, EnvEditor); and for SystemGeneric/SystemDefault reveals the file in Finder/Explorer instead of opening it. Port: warpdotdev/master 7f0c4dd, adapted to this fork's FileTarget (adds the fork-only ImageViewer safe target). Ported the extensionless-file regression test; the upstream markdown-viewer-preference test is N-A here (this fork routes .md through OpenFileNotebook, not open_file). No telemetry/auth. (cherry picked from commit 7397278)
…g (#25258) Commands with leading env-var assignments (e.g. `X=1 rm file.txt`) were matched against the execution denylist raw, so a denylist entry like `rm .*` (anchored to ^...$) failed to match and the blocked command ran. Add warp_completer::command_without_leading_env_vars (reusing the existing Command::remove_leading_env_vars) and a command_for_execution_predicates helper, then strip leading env vars from each subcommand before denylist matching only. Allowlist matching deliberately keeps the raw command, so a `PATH=/tmp <allowlisted>` form is not auto-allowed via stripping. Port: warpdotdev/master 0c1e243. Ported the parser unit test and the env-prefixed-denylist regression test (to parser_test.rs / permissions_test.rs, this fork's singular naming). No telemetry/auth. (cherry picked from commit 09171d8)
…rget
Introduce GitExecTarget { Local, Remote } as the single chokepoint for the
code-review diff engine. The Local arm delegates to the existing git
subprocess; the Remote arm ships commands over the remote_server channel
(RemoteServerClient::run_command) with POSIX shell-quoted args, GIT_OPTIONAL_LOCKS=0,
and the same exit-code -> Ok/Err contract.
Make the util/git helpers (detect_current_branch, detect_main_branch,
detect_fork_point, get_unpushed_commits) and all ~29 diff_state.rs git call
sites target-aware. FS-bound bits (untracked line counting, post-discard file
removal) branch on local vs remote. No behavior change for local workspaces.
Engine layer only; remote enablement (gate flip + repo-root sourcing) follows.
Verified: warp lib compiles; 10 git unit tests pass (shell-quote POSIX
round-trip, command build, output mapping incl. binary/-z payloads).
(cherry picked from commit 77df95e)
Add diff_state tests proving a -z NUL-delimited status payload and a unified diff payload, as they arrive over the remote-server channel, decode and parse identically to the local subprocess path. Document the remaining remote-enablement work (Phase B) in docs/2026-06-28_code-review-remote-diffs-phase-b.md: the panel lifecycle is keyed on a local PathBuf end to end, so wiring remote requires teaching it a remote repo identity + async repo-root resolution (git rev-parse on the client), plus the gate flip. Needs a live SSH host to E2E-verify. (cherry picked from commit 24e1a2b)
Resolve the warpified-SSH repo root via 'git rev-parse --show-toplevel' over the remote server client and cache it as current_remote_repo, since local-FS detection can't see the remote working copy. Emits RepoChanged so the code review panel re-initializes (Phase B step 1). (cherry picked from commit 4187760)
Add get_or_create_remote_diff_state_model keyed by (host, root) and a remote_code_review_context helper so setup_code_review_panel falls back to a warpified remote repo when no local repo is detected (Phase B 2-3). (cherry picked from commit 27ed07d)
Report has_remote_server truthfully (connected remote client) in both the CodeReviewView session_env and the right-panel session env. Warpified SSH now renders diffs / the normal no-repo state; only non-warpified SSH keeps the 'local workspaces only' message, reworded to say so. (cherry picked from commit 23dfe14)
The panel-level no-repo gate filters selected_repo_path against available_repos, which is built purely from local detection and never contains a warpified-remote root. So a warpified SSH session resolved a remote DiffStateModel and set the CodeReviewView's active_repo, but the right panel short-circuited to "Diffs only work for git repositories" before rendering it (and set_available_repos cleared the remote selection on the next refresh). Track the active remote root on CodeReviewState and merge it into available_repos so the dropdown, selection clear-logic, and render gate all accept it. setup_code_review_panel now passes the resolved remote root (or None for local) to RightPanelView::set_remote_repo_root. Verified: cargo check -p warp --lib (local_fs on) clean; diff_state unit tests (22) pass. Manual E2E on a live warpified SSH host still needed to confirm diffs render end to end. (cherry picked from commit 4c2e3bf)
create_code_review_view early-returned when the pane group had no local repository_roots, so a warpified SSH session never built a CodeReviewView. render_panel_content then fell through to render_loading_state, leaving the panel stuck on the "Loading open changes…" placeholder — the diff load was never even started because on_open was never called. Add DiffStateModel::is_remote() and admit the remote-backed model past the gate (!has_active_repos && !is_remote). The view is now created and on_open drives the remote diff load. Verified: cargo check -p warp --lib --features local_fs clean; diff_state unit tests (22) pass. Manual E2E on a live warpified SSH host still needed. (cherry picked from commit ce8fe2d)
远端 SSH Markdown 文件的 Rendered 预览会在文档末尾重复渲染一份、 每行只有一个字符宽。原因:set_remote_markdown_rendered 先调 set_interaction_state(Selectable) 再 reset_with_markdown。前者发出 延迟的 InteractionStateModelEvent,后者同步改 buffer;effect flush 时 状态变更处理器触发 Mermaid relayout,读到已填充的 buffer 并在首帧真实 宽度已知前(viewport 宽度 0)再插一次整篇布局编辑,经 insert-before- first-block 分支把前一棵窄树当 suffix 保留 → 文档渲染两份。 改为先 reset_with_markdown 再 set_interaction_state,使内容编辑先入队, 只保留单次布局编辑;首帧后按正常 resize 流程重排到真实宽度。 (cherry picked from commit 6e0a86e)
上一版(6e0a86e)仅调换 reset_with_markdown 与 set_interaction_state 的 顺序,依赖两条整篇布局动作在 FIFO 布局通道里的到达次序恰好"愈合"而非重复, 脆弱且未经真实构建验证。 真正根因:Editable→Selectable 触发 Mermaid 开关翻转(仅 MarkdownMermaid 特性开启时),其延迟处理器调用 rebuild_layout。只要内容已同步填充 buffer, 这次 relayout 就对整篇已填充文档发起失效,与内容插入竞争同一布局通道。 改为:先在**空 buffer** 上切到 Selectable(该次 relayout 成为不可能产生 窄树的空失效),再用 ctx.spawn 把 reset_with_markdown 推迟到**下一个**事件 循环周期,使内容作为唯一一条布局动作干净插入空树——与通道时序无关,且与本地 文件路径(内容在异步加载后的较晚周期到达)行为一致。 注:因缺少 Metal Toolchain(xcodebuild -downloadComponent MetalToolchain), warpui build.rs 无法编译着色器,本环境仍无法完成 cargo check;改动经人工逐项 核对类型与借用,与 view.rs:1350 现有 ctx.spawn 模式一致。 (cherry picked from commit 14e9013)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two feature sets from the OpenWarp fork, contributed back upstream. Code and tests only — no branding, docs, or workflow changes.
Remote code review (9 commits)
Extends the Code Review panel to work inside Warpified SSH sessions:
refactor: git operations routed through a transport-agnosticGitExecTarget, so diff/status commands can execute on the remote hostfeat: remote repo root tracked inTerminalView; remote-keyedDiffStateModelcache with panel wiring; remote-diffs gate enabled with updated messaging (Diffs only work for local workspaces and Warpified SSH sessions.)fix: remote repo root registered as a selectable repo;CodeReviewViewcreated past the local-repo gate; two fixes for duplicated remote Markdown preview rendering (defer content writes until the buffer state switch)Includes unit tests for remote-sourced diff/status parsing, git exec targeting, and diff-state caching.
Security hardening (8 commits)
shell_quote_arg: shell-aware quoting helper (used by the fixes below)is_file_path/is_git_repositorychecksVAR=valueassignments before command-denylist matching, so the denylist can't be bypassed via env-var prefixesEach fix ships with unit tests.
Note: our webfetch SSRF guard and macOS relaunch shell-escaping were dropped from this PR since upstream already has equivalent implementations.
Testing
cargo check --workspacepasses on this branch (one pre-existing warning from 5490588, untouched)