Skip to content

Commit c9cc33e

Browse files
digital-teamsCopilotss-oCopilot
committed
fix: validate zi repository identity before destructive git operations; automate checksum generation (#176)
* fix: validate zi repository identity before destructive git operations - Add zi repository validation (zi.zsh sentinel + z-shell/zi remote URL check) before git clean/reset/pull in the update path of install.sh - Add BOPT validation to reject branch names containing '|', '\', or '&' to prevent sed delimiter injection when updating init.zsh - Update fake git test double to handle '-C <dir>' prefix and 'remote get-url' - Add tests: valid zi clone update, foreign repo rejection, wrong-remote rejection - Update public/checksum.txt to match modified install.sh Fixes: #175 * fix: address code review feedback on security validation - Fix BOPT backslash case pattern: *'\\'* → *\\* to correctly match a single backslash (not two) - Anchor the remote URL grep to 'github.com[/:]z-shell/zi' to prevent spoofing via URLs like 'evil.com/fake-z-shell/zi' - Validate remote subcommand args in the fake git test double - Capture and assert specific error messages in rejection tests - Update checksum for modified install.sh * fix: address remaining code review issues - Revert *'\''* (wrong: matches single-quote) back to *\\* (correct: matches single backslash) and add clarifying comment explaining *\\* semantics - Change test double error message from '$*' to a fixed descriptive string - Update checksum for modified install.sh * fix: strengthen URL matching with exact case pattern instead of grep Replace partial grep match 'github.com[/:]z-shell/zi' with an exact shell case pattern that accepts only the four canonical zi remote URLs: - https://github.com/z-shell/zi - https://github.com/z-shell/zi.git - git@github.com:z-shell/zi - git@github.com:z-shell/zi.git This prevents spoofing via URLs like 'evil.com/github.com/z-shell/zi' and eliminates false-positive risk on empty _zi_remote values. * fix: add clarity improvements from code review - Add actual args to test double error message for debuggability - Add comment on canonical URL block pointing to z-shell/zi on GitHub - Update checksum for modified install.sh * fix: clarify test double error message for remote subcommand validation Remove positional parameters from error message and add comment explaining argument state after -C strip and cmd shift. * chore: add automated checksum generation script and integrate into CI - Add public/sh/generate-checksums.sh for local and CI use - Update check-linux.yml and check-macos.yml to regenerate checksums before the test run, keeping public/checksum.txt in sync automatically * feat: automate checksum generation with generate-checksums.sh Add public/sh/generate-checksums.sh that regenerates public/checksum.txt using sha256sum (Linux) or shasum -a 256 (macOS). Integrate it into the CI pipelines so checksums are never stale: - check-linux.yml: run generate-checksums.sh before tests/installers.sh - check-macos.yml: run generate-checksums.sh before tests/installers.sh - checksum.yml: replace jmgilman/actions-generate-checksum action with the new script; add generate-checksums.sh to trigger paths Developers can now run `sh public/sh/generate-checksums.sh` locally instead of manually updating public/checksum.txt after editing scripts. * refactor: address code review feedback - minor clarity improvements - tests/installers.sh: remove _sub/_arg temp variables in remote handler; use $1/$2 directly in the condition (simpler, reviewer preference) - public/sh/install.sh: reformat case pattern continuation so each line ends with the | pipe rather than beginning with \; add comment naming the four canonical URL forms (HTTPS/SSH, with/without .git suffix) - public/sh/generate-checksums.sh: add comment on bare > redirection explaining that it clears/creates the checksum file - public/checksum.txt: regenerate after install.sh formatting change * refactor: address second code review round - inline _zi_remote, rename _exit_code - public/sh/install.sh: inline _zi_remote command substitution directly into the case statement; eliminates the temporary variable - tests/installers.sh: rename _exit_code to exit_code in two test functions to match the no-leading-underscore convention used elsewhere - public/checksum.txt: regenerate after install.sh change * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Sal <59910950+ss-o@users.noreply.github.com> * Checksum 83d3f57 * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Sal <59910950+ss-o@users.noreply.github.com> * fix: satisfy shellcheck in checksum generator * fix: remove remaining shellcheck warning * chore: refresh installer checksum * test: clarify fake git remote errors * chore: address validation feedback --------- Signed-off-by: Sal <59910950+ss-o@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Sal <59910950+ss-o@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: digital-teams[bot] <actions@zshell.dev> 196e21c
1 parent b2ec0a0 commit c9cc33e

3 files changed

Lines changed: 71 additions & 1 deletion

File tree

checksum.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
0140801e80b2d8767e95d215850b6feab3642ee2b6fd70fbf4aab09afcfe7fca public/sh/install_zpmod.sh
2-
0cca0bb9f95101b4bf392d6aa3c23123602d99a7cbb0decacaa72973e016d89d public/sh/install.sh
2+
f9d8ef697ba682bdfe66fdd67d4517caa4e62bb8baf2868ec8ed9932809b76aa public/sh/install.sh
33
08cc893ceb982fc99d17db1966c6c30790cc571e16e4f5392352d995f5252952 public/sh/sync-init.sh
44
5c7af31d7fc848d80f9fedf69c75e6844503f550a3d5e020bc7cdfdaca624763 public/zsh/init.zsh

sh/generate-checksums.sh

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
#!/usr/bin/env sh
2+
# -*- mode: sh; sh-indentation: 2; indent-tabs-mode: nil; sh-basic-offset: 2; -*-
3+
# vim: ft=sh sw=2 ts=2 et
4+
#
5+
# generate-checksums.sh — regenerate public/checksum.txt
6+
#
7+
# Usage (from any directory):
8+
# sh public/sh/generate-checksums.sh
9+
#
10+
# This script is also invoked by the CI workflows before running
11+
# tests/installers.sh so that public/checksum.txt is always current.
12+
13+
set -eu
14+
15+
ROOT="$(
16+
unset CDPATH
17+
cd "$(dirname "$0")/../.." 2>/dev/null && pwd
18+
)" || { printf '%s\n' "generate-checksums: cannot determine repository root" >&2; exit 1; }
19+
20+
CHECKSUM_FILE="${ROOT}/public/checksum.txt"
21+
22+
sha256_file() {
23+
if command -v sha256sum >/dev/null 2>&1; then
24+
sha256sum "$1" | awk '{print $1}'
25+
elif command -v shasum >/dev/null 2>&1; then
26+
shasum -a 256 "$1" | awk '{print $1}'
27+
else
28+
printf '%s\n' "generate-checksums: sha256sum or shasum is required" >&2
29+
exit 1
30+
fi
31+
}
32+
33+
# Clear (or create) the checksum file before writing fresh entries with `: > file`.
34+
: > "${CHECKSUM_FILE}"
35+
for f in \
36+
public/sh/install_zpmod.sh \
37+
public/sh/install.sh \
38+
public/sh/sync-init.sh \
39+
public/zsh/init.zsh
40+
do
41+
hash="$(sha256_file "${ROOT}/${f}")"
42+
printf '%s %s\n' "${hash}" "${f}" >> "${CHECKSUM_FILE}"
43+
done
44+
45+
printf '%s\n' "Checksums written to ${CHECKSUM_FILE}"

sh/install.sh

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,17 @@ while getopts ":i:a:b:" opt; do
3636
done
3737
shift $((OPTIND - 1))
3838

39+
# Validate BOPT to prevent sed delimiter injection when building init.zsh.
40+
# | is the sed delimiter used in the substitution; \ and & are special in
41+
# sed replacement expressions. The *[\\]* pattern matches a single backslash.
42+
case "${BOPT}" in
43+
# [\\] is a bracket expression for a literal backslash.
44+
*'|'* | *[\\]* | *'&'* )
45+
printf '%s\n' "-- ERROR -- Invalid -b value: branch name must not contain '|', '\\', or '&'." >&2
46+
exit 1
47+
;;
48+
esac
49+
3950
SCRIPT_DIR=""
4051
LOCAL_INIT_ZSH=""
4152
LOCAL_INSTALL_ZPMOD=""
@@ -147,6 +158,20 @@ fi
147158
command chmod a+x /tmp/zi/git-process-output.zsh
148159

149160
if test -d "${ZI_HOME}/${ZI_BIN_DIR_NAME}/.git"; then
161+
_zi_valid=0
162+
if test -f "${ZI_HOME}/${ZI_BIN_DIR_NAME}/zi.zsh"; then
163+
# Canonical zi remote URLs (HTTPS and SSH, with and without .git suffix)
164+
case "$(command git -C "${ZI_HOME}/${ZI_BIN_DIR_NAME}" remote get-url origin 2>/dev/null || true)" in
165+
https://github.com/z-shell/zi | https://github.com/z-shell/zi.git \
166+
| git@github.com:z-shell/zi | git@github.com:z-shell/zi.git)
167+
_zi_valid=1 ;;
168+
esac
169+
fi
170+
if [ "${_zi_valid}" -ne 1 ]; then
171+
printf '%s\n' "▓▒░ ${ZI_HOME}/${ZI_BIN_DIR_NAME} contains a .git directory but does not appear to be a zi repository." >&2
172+
printf '%s\n' "▓▒░ Expected zi.zsh and a z-shell/zi remote origin. Unset ZI_HOME/ZI_BIN_DIR_NAME or remove the directory to install fresh." >&2
173+
exit 1
174+
fi
150175
cd "${ZI_HOME}/${ZI_BIN_DIR_NAME}" || exit 1
151176
printf '%s\n' "▓▒░ Updating (z-shell/zi) plugin manager at ${ZI_HOME}/${ZI_BIN_DIR_NAME}"
152177
command git clean -d -f -f

0 commit comments

Comments
 (0)