Show MotD over Warp SSH for bash and zsh (#1160)#9586
Show MotD over Warp SSH for bash and zsh (#1160)#9586tarun-khatri wants to merge 3 commits intowarpdotdev:masterfrom
Conversation
The MotD-emulation block in each shell-bootstrap body
(`bash_body.sh`, `zsh_body.sh`, `fish.sh`) was nested inside an
`if test "${SHELL##*/}" != "bash" -a "${SHELL##*/}" != "zsh"`
guard, with a comment claiming "For bash and zsh, this is instead
handled by our bootstrap script." That assumption was wrong: sshd
skips MotD when invoked with a command (Warp's wrapper passes one),
and our bash/zsh rcfile bootstrap doesn't reintroduce it. So bash
and zsh users — the vast majority — silently lost the MotD over
Warp SSH while csh/tcsh/ksh users got it.
Hoist the MotD block out of the shell-type guard so it runs
unconditionally, before the bash/zsh dispatch. The /etc/profile
sourcing remains gated to the non-bash/non-zsh path because bash and
zsh do their own profile/rcfile loading.
Also harden the MotD probe to handle modern setups:
* Use `test -f /etc/motd` (regular file) before `cat` so the
"/etc/motd is a directory" failure mode (some debconf setups)
is skipped instead of erroring.
* Concatenate `/etc/motd.d/*` fragments when present (Fedora / RHEL
style).
* Try `/run/motd.dynamic` (Ubuntu / Debian's pre-rendered MotD)
before falling back to the legacy paths.
Adds a regression test in `bootstrap_test.rs` that `include_str!`s
each of the three bootstrap bodies and asserts the MotD block
appears BEFORE the non-bash/non-zsh guard so a future refactor can't
silently re-nest it.
Live before/after demo (BEFORE: silent, AFTER: MotD printed) included
in the PR description.
Fixes warpdotdev#1160
|
I'm starting a first review of this pull request. I reviewed this pull request and requested human review from: @zachbai. Comment I'm re-reviewing this pull request in response to a review request. I reviewed this pull request and requested human review from: @zachbai. Comment I completed the review and posted feedback on this pull request. Comment I completed the review and posted feedback on this pull request. Comment I reviewed this pull request and requested human review from: @zachbai. Comment You can view the conversation on Warp. I completed the review and posted feedback on this pull request. Comment Powered by Oz |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This PR hoists SSH MotD emulation out of the non-bash/non-zsh guard across the bundled shell bootstraps and adds coverage to keep bash/zsh from regressing.
Concerns
- The regression test searches for
/etc/motd, which can match explanatory comments instead of the actual MotD branch; tightening the marker would make the test protect the intended invariant.
Verdict
Found: 0 critical, 0 important, 1 suggestions
Approve with nits
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| // Marker the heredoc uses to print MotD. | ||
| let motd_marker = "/etc/motd"; | ||
| // Marker for the non-bash/non-zsh guard. Both `!= "bash" -a` and | ||
| // `!= \"bash\"` show up in the file as a string segment depending on | ||
| // the surrounding heredoc quoting, but the `!= "bash" -a` substring | ||
| // is stable across the three files. | ||
| let guard_marker = "!= \"bash\" -a"; |
There was a problem hiding this comment.
💡 [SUGGESTION] This marker can match comments that mention /etc/motd, so the test could still pass if the real print branch moved back under the guard while a comment stayed above it.
| // Marker the heredoc uses to print MotD. | |
| let motd_marker = "/etc/motd"; | |
| // Marker for the non-bash/non-zsh guard. Both `!= "bash" -a` and | |
| // `!= \"bash\"` show up in the file as a string segment depending on | |
| // the surrounding heredoc quoting, but the `!= "bash" -a` substring | |
| // is stable across the three files. | |
| let guard_marker = "!= \"bash\" -a"; | |
| // Marker for the actual MotD branch, not a comment that mentions MotD. | |
| let motd_marker = "if test -f /etc/motd && test -r /etc/motd; then"; | |
| // Marker for the non-bash/non-zsh guard. | |
| let guard_marker = "!= \"bash\" -a"; |
captainsafia
added
the
external-contributor
Warp Dev Github Integration
) Per Oz review feedback on PR warpdotdev#9586: the previous marker `/etc/motd` also matched explanatory comments in the surrounding block, so a file that kept the comments but removed the executable probe would still pass the test. Anchor on `test -f /etc/motd && test -r /etc/motd` instead — the actual conditional, unambiguously distinct from prose.
|
/oz-review |
There was a problem hiding this comment.
Overview
This PR moves the SSH MotD emulation outside the bash/zsh shell-type guard in the three bootstrap heredocs, hardens /etc/motd probing, adds /etc/motd.d handling, and adds a structural regression test.
Concerns
- The new
/etc/motd.d/*loops concatenate regular files without checking readability first, so an unreadable fragment can surfacecatpermission errors during SSH bootstrap. Apply the same readability check in each copied bootstrap loop. - Supplemental security pass: no command-injection, secret exposure, or privilege-boundary issue was identified in the changed lines; the expanded fragment path is quoted before use.
Verdict
Found: 0 critical, 0 important, 3 suggestions
Approve with nits
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| command -p cat /etc/motd | ||
| elif test -d /etc/motd.d; then | ||
| for motd_fragment in /etc/motd.d/*; do | ||
| test -f "$motd_fragment" && command -p cat "$motd_fragment" |
There was a problem hiding this comment.
💡 [SUGGESTION] Check fragment readability before cat so unreadable /etc/motd.d entries do not print permission errors during SSH bootstrap.
| test -f "$motd_fragment" && command -p cat "$motd_fragment" | |
| test -f "$motd_fragment" && test -r "$motd_fragment" && command -p cat "$motd_fragment" |
| cat /etc/motd | ||
| elif test -d /etc/motd.d; then | ||
| for motd_fragment in /etc/motd.d/*; do | ||
| test -f "$motd_fragment" && cat "$motd_fragment" |
There was a problem hiding this comment.
💡 [SUGGESTION] Check fragment readability before cat so unreadable /etc/motd.d entries do not print permission errors during SSH bootstrap.
| test -f "$motd_fragment" && cat "$motd_fragment" | |
| test -f "$motd_fragment" && test -r "$motd_fragment" && cat "$motd_fragment" |
| command -p cat /etc/motd | ||
| elif test -d /etc/motd.d; then | ||
| for motd_fragment in /etc/motd.d/*; do | ||
| test -f "$motd_fragment" && command -p cat "$motd_fragment" |
There was a problem hiding this comment.
💡 [SUGGESTION] Check fragment readability before cat so unreadable /etc/motd.d entries do not print permission errors during SSH bootstrap.
| test -f "$motd_fragment" && command -p cat "$motd_fragment" | |
| test -f "$motd_fragment" && test -r "$motd_fragment" && command -p cat "$motd_fragment" |
There was a problem hiding this comment.
Overview
This PR hoists remote SSH MotD emulation for bash/zsh and adds /etc/motd.d handling across the bash, zsh, and fish bootstrap wrappers.
Concerns
- The new
/etc/motd.dloop references$motd_fragmentwithout escaping it out of the local SSH command string, so the local shell expands it before the script reaches the remote host; when unset this can turn the remote command intotest -f && cat, which can hang on hosts with/etc/motd.d. - The
/etc/motd.dbranch is terminal even when the directory is empty or has no readable fragments, so later fallbacks such as/run/motd.dynamicare skipped.
Verdict
Found: 0 critical, 4 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| # `cat /etc/motd` would error. | ||
| if test -f /etc/motd && test -r /etc/motd; then | ||
| command -p cat /etc/motd | ||
| elif test -d /etc/motd.d; then |
There was a problem hiding this comment.
/etc/motd.d directory as a terminal elif suppresses later fallbacks when the directory is empty or has no readable fragments, so /run/motd.dynamic will never print on those hosts. Track whether a fragment was printed, or only skip the remaining probes when a readable fragment exists; the same branch exists in the fish/zsh copies.
| command -p cat /etc/motd | ||
| elif test -d /etc/motd.d; then | ||
| for motd_fragment in /etc/motd.d/*; do | ||
| test -f "$motd_fragment" && command -p cat "$motd_fragment" |
There was a problem hiding this comment.
$motd_fragment is inside the local SSH command string, so the local shell expands it before the remote script runs; when unset the remote line becomes test -f && command -p cat and can hang. Escape it like the other remote variables so expansion happens on the remote host.
| test -f "$motd_fragment" && command -p cat "$motd_fragment" | |
| test -f \"\$motd_fragment\" && command -p cat \"\$motd_fragment\" |
| cat /etc/motd | ||
| elif test -d /etc/motd.d; then | ||
| for motd_fragment in /etc/motd.d/*; do | ||
| test -f "$motd_fragment" && cat "$motd_fragment" |
There was a problem hiding this comment.
$motd_fragment is inside the local SSH command string, so fish expands it before the remote script runs; when unset the remote line becomes test -f && cat and can hang. Escape it like the other remote variables so expansion happens on the remote host.
| test -f "$motd_fragment" && cat "$motd_fragment" | |
| test -f \"\$motd_fragment\" && cat \"\$motd_fragment\" |
| command -p cat /etc/motd | ||
| elif test -d /etc/motd.d; then | ||
| for motd_fragment in /etc/motd.d/*; do | ||
| test -f "$motd_fragment" && command -p cat "$motd_fragment" |
There was a problem hiding this comment.
$motd_fragment is inside the local SSH command string, so the local shell expands it before the remote script runs; when unset the remote line becomes test -f && command -p cat and can hang. Escape it like the other remote variables so expansion happens on the remote host.
| test -f "$motd_fragment" && command -p cat "$motd_fragment" | |
| test -f \"\$motd_fragment\" && command -p cat \"\$motd_fragment\" |
|
/oz-review |
…warpdotdev#1160) Two follow-up fixes per Oz feedback: * Heredoc escape: the for-loop iterator was bare $motd_fragment inside the outer double-quoted heredoc the local shell builds before sending to the remote. Local bash had no motd_fragment set, so the dollar expanded to empty before the heredoc ever reached the remote -- every cat/test on "$motd_fragment" was actually cat "" / test "". Net effect: the /etc/motd.d branch silently emitted nothing for every Fedora/RHEL user. Use the \ escape pattern (matches the existing zshenv decode loop further down) so the dollar reaches the remote literally and the remote shell expands it on each iteration. * Fallthrough on empty /etc/motd.d: the previous elif chain treated /etc/motd.d existing as terminal -- if the directory was present but empty (or only had unreadable fragments), nothing was emitted and /run/motd.dynamic etc. were skipped. Track an emitted flag and only enter the next fallback when nothing has been printed yet, matching the original elif-chain semantics while staying safe for the directory case. Same change applied identically to bash_body.sh, zsh_body.sh, fish.sh. Tracks warpdotdev#1160.
|
/oz-review |
There was a problem hiding this comment.
Overview
This PR hoists SSH MotD emulation out of the bash/zsh guard across the bootstrap scripts, hardens /etc/motd handling, and adds a structural regression test.
Concerns
- The new
/etc/motd.dbranch is anelifthat stops probing later MotD sources even if the directory is empty, unreadable, or has no readable regular fragments. This can still drop/run/motd.dynamicor/run/motdon systems where both are present. The same pattern appears in all three bootstrap scripts.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| # | ||
| # Quoting note: this whole block runs on the *remote* shell but is embedded | ||
| # in a double-quoted heredoc that the local shell parses first. The remote | ||
| # for-loop variable and emit-tracker must be referenced as \$VAR so the |
There was a problem hiding this comment.
elif treats the presence of /etc/motd.d as sufficient, so an empty, unreadable, or fragment-less directory prevents /run/motd.dynamic and /run/motd from ever being considered. Track whether a readable fragment was actually printed before suppressing the later fallbacks; the same issue exists in fish.sh and zsh_body.sh.
There was a problem hiding this comment.
Overview
This PR moves the SSH MotD emulation outside the non-bash/non-zsh guard for the bundled bash, zsh, and fish bootstrap wrappers, hardens the /etc/motd probe, adds /etc/motd.d handling, and adds a regression test that checks the MotD probe appears before the shell-type guard.
Concerns
- No blocking correctness or security concerns found in the changed hunks.
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 hoists MotD emulation out of the non-bash/non-zsh guard in the SSH bootstrap snippets and adds a structural regression test to keep MotD printing available for bash and zsh.
Concerns
- The added explanatory comments in all three generated SSH command strings contain shell metacharacters that are parsed by the local shell before the remote command is sent. In bash/zsh wrappers, backticks in comments execute locally; in all wrappers, the unescaped quoted phrase splits the SSH command construction.
Verdict
Found: 0 critical, 3 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| # `test -f` follows symlinks and only succeeds for regular files, which | ||
| # rules out the "/etc/motd is a directory" failure mode where the old | ||
| # `cat /etc/motd` would error. |
There was a problem hiding this comment.
cat /etc/motd can run locally and the quoted phrase can split the SSH arguments. Remove the shell metacharacters from the comment.
| # `test -f` follows symlinks and only succeeds for regular files, which | |
| # rules out the "/etc/motd is a directory" failure mode where the old | |
| # `cat /etc/motd` would error. | |
| # test -f follows symlinks and only succeeds for regular files, which | |
| # rules out the case where /etc/motd is a directory and the old | |
| # cat /etc/motd would error. |
| # `test -f` follows symlinks and only succeeds for regular files, which | ||
| # rules out the "/etc/motd is a directory" failure mode where the old | ||
| # `cat /etc/motd` would error. |
There was a problem hiding this comment.
| # `test -f` follows symlinks and only succeeds for regular files, which | |
| # rules out the "/etc/motd is a directory" failure mode where the old | |
| # `cat /etc/motd` would error. | |
| # test -f follows symlinks and only succeeds for regular files, which | |
| # rules out the case where /etc/motd is a directory and the old | |
| # cat /etc/motd would error. |
| # `test -f` follows symlinks and only succeeds for regular files, which | ||
| # rules out the "/etc/motd is a directory" failure mode where the old | ||
| # `cat /etc/motd` would error. |
There was a problem hiding this comment.
cat /etc/motd can run locally and the quoted phrase can split the SSH arguments. Remove the shell metacharacters from the comment.
| # `test -f` follows symlinks and only succeeds for regular files, which | |
| # rules out the "/etc/motd is a directory" failure mode where the old | |
| # `cat /etc/motd` would error. | |
| # test -f follows symlinks and only succeeds for regular files, which | |
| # rules out the case where /etc/motd is a directory and the old | |
| # cat /etc/motd would error. |
2 participants
Description
Fixes #1160.
When you
ssh user@hostfrom Warp, the host's MotD (/etc/motd,/etc/motd.d/*,/run/motd.dynamic, etc.) does not appear. Plainsshfrom a regular terminal shows it. Warp's wrapper swallows it.Root cause
The MotD-emulation block in each shell-bootstrap body (
bash_body.sh,zsh_body.sh,fish.sh) was nested inside anif test "${SHELL##*/}" != "bash" -a "${SHELL##*/}" != "zsh"guard, with a comment claiming "For bash and zsh, this is instead handled by our bootstrap script."That assumption was wrong:
sshdskips MotD when invoked with a command — and Warp's wrapper passes one (the bootstrap script).So bash and zsh users (the vast majority of Linux users) silently lost the MotD over Warp SSH, while csh/tcsh/ksh users got it. A 4-year-old, much-reported bug — last user comment 2025-09-17: "Just observed this bug today." Issue #1160 also included a smoking gun from another user: "I just noticed
/etc/motd is a directorywhen Warp's SSH wrapper is on", which surfaces because the old code doestest -r /etc/motd(true for a directory) followed bycat /etc/motd(errors).Fix
/etc/profilesourcing remains gated to non-bash/non-zsh because bash and zsh do their own profile/rcfile loading.test -f /etc/motd && test -r /etc/motd— a regular-file (or symlink-to-regular-file) MotD; using-fskips the "directory" failure mode entirely.test -d /etc/motd.d— concatenate fragments (Fedora / RHEL)./run/motd.dynamic— Ubuntu / Debian's pre-rendered dynamic MotD./run/motd,/usr/lib/motd,/usr/lib/motd.dynamicfor older systems.Same change applied to all three shell bootstraps to keep the heredoc structure consistent.
Testing
Live BEFORE/AFTER demo
Driven by a self-contained POSIX-shell repro that exercises the exact
if test … != "bash" -a … != "zsh"branch withSHELL=/bin/bash(the case the bug report covers). No Warp build required.BEFORE FIX — upstream master heredoc structure,
SHELL=/bin/bash:(MotD silently dropped — exactly the user-reported behavior.)
AFTER FIX — same repro with the new heredoc structure:
Regression test
test_motd_emulation_is_not_gated_on_shell_typeinapp/src/terminal/bootstrap_test.rsinclude_str!s each of the three real bootstrap files and asserts the MotD-print block (/etc/motd) appears before the!= "bash" -ashell-type guard so a future refactor can't silently re-nest the MotD block and regress GH-1160.Not run locally
script/presubmitand the fullcargo nextest runwere not run on the host this PR was developed on. The change is shell-script-only plus one self-contained Rust unit test that usesinclude_str!and&str::find; CI will run the full presubmit on PR submission. Same posture as the PR-template-recommended workflow.Server API dependencies
This PR has no server dependencies.
Changelog Entries for Stable
CHANGELOG-BUG-FIX: Show the remote host's MotD when SSHing from Warp with bash or zsh. Previously the MotD was silently dropped for bash/zsh users; now it's printed for every shell, with hardened probing for
/etc/motd.d/*fragments and/run/motd.dynamic(GH-1160).