Actually disable rc sourcing in the SSH wrapper's remote zsh env script#13407
Actually disable rc sourcing in the SSH wrapper's remote zsh env script#13407maxmilian wants to merge 1 commit into
Conversation
The wrapper's zsh_env_script intends to start the remote zsh with minimal startup and let the bootstrap body re-source rc files itself (with proper bracketing and timing). But 'unset RCS; unset GLOBAL_RCS' removes shell *variables* named RCS/GLOBAL_RCS — it does not touch the zsh *options*, so rc sourcing stays enabled: % echo 'unset RCS; unset GLOBAL_RCS' > $T/.zshenv % ZDOTDIR=$T zsh -i -c 'echo rcs=$options[rcs] globalrcs=$options[globalrcs]' rcs=on globalrcs=on Consequences on every wrapper-warpified remote zsh session: - global rc files (/etc/zprofile, /etc/zshrc, /etc/zlogin) execute twice: once bare before Warp's InitShell handshake, then again when the body re-sources them (zsh_body.sh sources /etc/z* explicitly) - anything slow, interactive, or output-producing in global rc runs before the handshake, where it can consume or garble the injected bootstrap Fix: fold RCS and GLOBAL_RCS into the existing unsetopt. After the change the same probe reports rcs=off globalrcs=off and $ZDOTDIR/.zshrc is not sourced. The same payload string is duplicated in the bash and fish SSH wrappers; all three copies updated.
|
Every PR must be linked to a same-repo issue before Oz can review it. Next step: open or find a same-repo issue describing this change, then link it to this PR by adding See the contribution guidelines for the full readiness model. Powered by Oz |
There was a problem hiding this comment.
Every PR must be linked to a same-repo issue before Oz can review it.
Next step: open or find a same-repo issue describing this change, then link it to this PR by adding Closes #123 to the PR description (or using the "Development" sidebar on GitHub). A maintainer will mark the issue ready-to-implement when it is ready. Once it is marked, comment /oz-review to re-trigger review.
See the contribution guidelines for the full readiness model.
Powered by Oz
|
/oz-review |
|
/oz-review |
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I reviewed this pull request and requested human review from: Comment Powered by Oz |
|
I'm re-reviewing this pull request in response to a review request. You can view the conversation on Warp. I reviewed this pull request and requested human review from: Comment Powered by Oz |
Oz no longer requests changes for this pull request after the latest automated review.
There was a problem hiding this comment.
Overview
This PR fixes the SSH wrapper's remote zsh environment script so it disables zsh's RCS and GLOBAL_RCS options with unsetopt, rather than unsetting unrelated shell variables, across the bash, fish, and zsh wrapper copies.
Concerns
- No blocking concerns found. The change is scoped to the annotated bootstrap payload lines, has terminal-text behavioral evidence, and does not drift from any approved spec context.
Verdict
Found: 0 critical, 0 important, 0 suggestions
Approve
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
There was a problem hiding this comment.
Overview
This PR updates the duplicated SSH wrapper zsh environment payloads in the bash, fish, and zsh bootstrap scripts so the remote zsh startup disables the RCS and GLOBAL_RCS options via unsetopt instead of unsetting shell variables. That matches the intended pre-handshake rc-suppression behavior and leaves the existing bootstrap flow intact.
Concerns
- No blocking correctness, security, or spec-alignment concerns found in the changed lines.
Verdict
Found: 0 critical, 0 important, 0 suggestions
Approve
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
Description
Fixes #13408 (one verified failure mode in the #6244 family).
The SSH wrapper's
zsh_env_script(written to a temp$ZDOTDIR/.zshenvon the remote host) intends to start the remote zsh with minimal startup, and let the bootstrap body re-source rc files itself —zsh_body.shexplicitly re-sources/etc/zprofile → .zprofile → /etc/zshrc → .zshrc → /etc/zlogin → .zloginwith timing brackets once it's injected.But
unset RCS; unset GLOBAL_RCSremoves shell variables namedRCS/GLOBAL_RCS— it does not touch the zsh options, so rc sourcing stays fully enabled:Consequences on every wrapper-warpified remote zsh session:
InitShellhandshake, then again when the body re-sources them. (e.g. sshing into a macOS host runspath_helpertwice, reshufflingPATH.)Fix: fold
RCS/GLOBAL_RCSinto the existingunsetopt. The same payload string is duplicated in the bash and fish SSH wrappers; all three copies updated identically.Scope note: this fixes the verified pre-handshake rc-suppression defect in the legacy SSH wrapper. It does not address the separate app-side issue where a failed warpification unhides the long-running block with the echoed bootstrap left in place — I have a live repro of that and will file details on #6244 for a follow-up.
Linked Issue
Closes #13408
Related to #6244 (this fixes one concrete, verified failure mode in that family — not the whole issue, so it deliberately does not close it).
ready-to-specorready-to-implement. (SSH wrapper's remote zsh env script fails to disable rc sourcing (unset vs unsetopt) #13408 is newly filed and awaiting maintainer triage; will comment/oz-reviewonce labeled.)Testing
zsh -n app/assets/bundled/bootstrap/zsh_body.shbash -n app/assets/bundled/bootstrap/bash_body.shBehavioral red/green (local zsh 5.9): with
unset RCS; unset GLOBAL_RCSin a$ZDOTDIR/.zshenv, an interactive zsh reportsrcs=on globalrcs=onand a planted$ZDOTDIR/.zshrcis sourced; with the new payload prefixunsetopt ZLE RCS GLOBAL_RCS, it reportsrcs=off globalrcs=offand the planted rc is not sourced.End-to-end against a live SSH host (Ubuntu 24.04, zsh 5.9), exercising the wrapper's exact delivery mechanism — payload prefix written to a temp
$ZDOTDIR/.zshenv, then an interactive zsh started against it — with a markerechoplanted in the remote/etc/zsh/zshrc:I have manually tested my changes locally with
./script/run(dev build used to drive the SSH sessions used for the investigation).Screenshots / Videos
The behavior change is in a non-UI remote bootstrap payload; the terminal-text evidence above captures it end-to-end. One caveat surfaced during testing: on my test host the interactive
sshflow engages the newer remote-server extension rather than the legacy wrapper, so the payload-level test above exercises the changed path directly instead of going through the full GUI flow.Agent Mode