Skip to content

Actually disable rc sourcing in the SSH wrapper's remote zsh env script#13407

Open
maxmilian wants to merge 1 commit into
warpdotdev:masterfrom
maxmilian:maxmilian/fix-6244-wrapper-norcs
Open

Actually disable rc sourcing in the SSH wrapper's remote zsh env script#13407
maxmilian wants to merge 1 commit into
warpdotdev:masterfrom
maxmilian:maxmilian/fix-6244-wrapper-norcs

Conversation

@maxmilian

@maxmilian maxmilian commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Description

Fixes #13408 (one verified failure mode in the #6244 family).

The SSH wrapper's zsh_env_script (written to a temp $ZDOTDIR/.zshenv on the remote host) intends to start the remote zsh with minimal startup, and let the bootstrap body re-source rc files itself — zsh_body.sh explicitly re-sources /etc/zprofile → .zprofile → /etc/zshrc → .zshrc → /etc/zlogin → .zlogin with timing brackets once it's injected.

But unset RCS; unset GLOBAL_RCS removes shell variables named RCS/GLOBAL_RCS — it does not touch the zsh options, so rc sourcing stays fully enabled:

% T=$(mktemp -d)
% echo 'unset RCS; unset GLOBAL_RCS' > $T/.zshenv
% ZDOTDIR=$T zsh -i -c 'echo rcs=$options[rcs] globalrcs=$options[globalrcs]'
rcs=on globalrcs=on        # ← options untouched; a $ZDOTDIR/.zshrc still gets sourced

Consequences on every wrapper-warpified remote zsh session:

  • Global rc files execute twice — once bare before Warp's InitShell handshake, then again when the body re-sources them. (e.g. sshing into a macOS host runs path_helper twice, reshuffling PATH.)
  • Anything slow, interactive, or output-producing in global rc runs before the handshake, where it can consume or garble the injected bootstrap — the failure family tracked in .bashrc addition to automatically warpify a SSH shell prints out bash code #6244.

Fix: fold RCS/GLOBAL_RCS into the existing unsetopt. 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).

Testing

  • zsh -n app/assets/bundled/bootstrap/zsh_body.sh

  • bash -n app/assets/bundled/bootstrap/bash_body.sh

  • Behavioral red/green (local zsh 5.9): with unset RCS; unset GLOBAL_RCS in a $ZDOTDIR/.zshenv, an interactive zsh reports rcs=on globalrcs=on and a planted $ZDOTDIR/.zshrc is sourced; with the new payload prefix unsetopt ZLE RCS GLOBAL_RCS, it reports rcs=off globalrcs=off and 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 marker echo planted in the remote /etc/zsh/zshrc:

    ════ BEFORE (current payload prefix: unset = no-op) ════
    [OLD] GLOBAL-ZSHRC occurrences before handshake = 1
    [OLD]   WARP_ENV_RAN
    [OLD]   [GLOBAL-ZSHRC] executed 23:29:00.694   ← global rc ran pre-handshake
    [OLD]   SHELL_UP
    
    ════ AFTER (this PR's payload prefix) ════
    [NEW] GLOBAL-ZSHRC occurrences before handshake = 0
    [NEW]   WARP_ENV_RAN                            ← env script itself still runs
    [NEW]   SHELL_UP                                ← shell still comes up normally
    
  • 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 ssh flow 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

  • Warp Agent Mode - This PR was created via Warp's AI Agent Mode

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.
@cla-bot cla-bot Bot added the cla-signed label Jul 6, 2026
@github-actions github-actions Bot added the external-contributor Indicates that a PR has been opened by someone outside the Warp team. label Jul 6, 2026
@maxmilian maxmilian marked this pull request as ready for review July 6, 2026 15:30
@oz-for-oss

oz-for-oss Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

@maxmilian

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-for-oss[bot]
oz-for-oss Bot previously requested changes Jul 6, 2026

@oz-for-oss oz-for-oss Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@maxmilian

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

@zachbai

zachbai commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

/oz-review

@maxmilian maxmilian marked this pull request as ready for review July 11, 2026 00:51
@maxmilian

Copy link
Copy Markdown
Contributor Author

/oz-review

@oz-for-oss

oz-for-oss Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

@maxmilian

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: @acarl005.

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

@oz-for-oss

oz-for-oss Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

@maxmilian

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: @acarl005.

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

@oz-for-oss oz-for-oss Bot dismissed their stale review July 11, 2026 00:54

Oz no longer requests changes for this pull request after the latest automated review.

@oz-for-oss oz-for-oss Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

@oz-for-oss oz-for-oss Bot requested a review from acarl005 July 11, 2026 00:54

@oz-for-oss oz-for-oss Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed external-contributor Indicates that a PR has been opened by someone outside the Warp team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SSH wrapper's remote zsh env script fails to disable rc sourcing (unset vs unsetopt)

2 participants