From 5ad9d185c58a826201a1a6085deb58995decff09 Mon Sep 17 00:00:00 2001 From: Jintin Date: Mon, 3 Aug 2026 00:12:23 +0800 Subject: [PATCH 1/2] fix: zsh portability, store integrity, and test coverage Storage was written with echo, which expands backslash escapes under zsh. Adding `printf "[%s]\n"` wrote a real newline into ~/.aliasme/cmd, breaking the two-lines-per-record format and orphaning every entry after it. All writes and all rendering of stored text now use printf. Also fixes two zsh-only execution bugs and one variable leak: - A stored command containing an unmatched glob character aborted under zsh with "no matches found" (any URL with a query string). _excute now sets no_nomatch locally; intentional globs still expand. - _excute's locals were visible to the eval'd command, so an alias referencing $name or $value saw aliasme's internals. They are now _al_* prefixed. The command also runs after the storage fd is closed. Tests: run against a mktemp store instead of the real ~/.aliasme, exit non-zero on failure (the suite previously printed failures and exited 0, so CI was green regardless), and cover the name/command collision, exit code propagation, escape handling, and variable isolation cases. CI now runs them on bash and zsh across ubuntu and macos, with ShellCheck split into its own job. Co-Authored-By: Claude Fable 5 --- .github/workflows/ci.yml | 21 +++++- aliasme.sh | 45 +++++++------ test/aliastest.sh | 135 +++++++++++++++++++++++++++++---------- 3 files changed, 149 insertions(+), 52 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5bcfe6f..73fe13b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -7,7 +7,7 @@ on: branches: [ master, main ] jobs: - test: + lint: runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 @@ -18,5 +18,22 @@ jobs: - name: Run ShellCheck run: shellcheck aliasme.sh test/*.sh + test: + # The script is sourced into the user's shell, so it has to be exercised + # under both supported shells; most users install via Homebrew on macOS, + # where zsh is the default. + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, macos-latest] + shell: [bash, zsh] + runs-on: ${{ matrix.os }} + steps: + - uses: actions/checkout@v4 + + - name: Install zsh + if: matrix.shell == 'zsh' && runner.os == 'Linux' + run: sudo apt-get install -y zsh + - name: Run Tests - run: make test + run: ${{ matrix.shell }} test/aliastest.sh diff --git a/aliasme.sh b/aliasme.sh index cc728fa..73a584c 100644 --- a/aliasme.sh +++ b/aliasme.sh @@ -10,7 +10,7 @@ _list() { while IFS= read -r name do if ! IFS= read -r value; then break; fi - echo "$name : $value" + printf '%s : %s\n' "$name" "$value" done < "$ALIASME_CMD" fi } @@ -52,13 +52,12 @@ _add() { fi if _find "$name"; then - echo "$name already exists, remove it first: al rm $name" + printf '%s already exists, remove it first: al rm %s\n' "$name" "$name" return 1 fi - echo "$name" >> "$ALIASME_CMD" - echo "$cmd" >> "$ALIASME_CMD" - echo "add: $name -> $cmd" + printf '%s\n%s\n' "$name" "$cmd" >> "$ALIASME_CMD" + printf 'add: %s -> %s\n' "$name" "$cmd" _autocomplete } @@ -78,34 +77,44 @@ _remove() { do if ! IFS= read -r value; then break; fi if [ "$line" = "$name" ]; then - echo "remove $name" + printf 'remove %s\n' "$name" found=0 else - echo "$line" >> "$ALIASME_DIR/cmdtemp" - echo "$value" >> "$ALIASME_DIR/cmdtemp" + printf '%s\n%s\n' "$line" "$value" >> "$ALIASME_DIR/cmdtemp" fi done < "$ALIASME_CMD" mv "$ALIASME_DIR/cmdtemp" "$ALIASME_CMD" fi if [ "$found" -ne 0 ]; then - echo "not found: $name" + printf 'not found: %s\n' "$name" fi _autocomplete return "$found" } _excute() { - local name value + # Prefixed names: the stored command is eval'd in this scope, so plain + # names like "name" would shadow the user's own variables. + local _al_name _al_value _al_found + # zsh aborts on a glob that matches nothing, which would break any stored + # command containing a literal ? or * (a URL query string, typically). + if [ -n "$ZSH_VERSION" ]; then + setopt local_options no_nomatch + fi + _al_found=1 if [ -s "$ALIASME_CMD" ];then - while IFS= read -u9 -r name; do - if ! IFS= read -u9 -r value; then break; fi - if [ "$1" = "$name" ]; then - eval "$value" - return $? + while IFS= read -u9 -r _al_name; do + if ! IFS= read -u9 -r _al_value; then break; fi + if [ "$1" = "$_al_name" ]; then + _al_found=0 + break fi done 9< "$ALIASME_CMD" fi - return 1 + [ "$_al_found" -eq 0 ] || return 1 + + # Run with the storage file closed so the command cannot inherit fd 9. + eval "$_al_value" } _bashauto() @@ -167,13 +176,13 @@ al(){ echo "al -v # version information" echo "al -h # help" elif [ "$1" = "-v" ]; then - echo "aliasme 3.1.0" + echo "aliasme 3.1.1" echo "visit https://github.com/Jintin/aliasme for more information" else if _find "$1"; then _excute "$1" else - echo "not found: $1" + printf 'not found: %s\n' "$1" return 1 fi fi diff --git a/test/aliastest.sh b/test/aliastest.sh index abd081e..2e08147 100755 --- a/test/aliastest.sh +++ b/test/aliastest.sh @@ -1,45 +1,116 @@ #!/bin/bash -. test/assert.sh -. aliasme.sh +. ./test/assert.sh -testInit() { - if [[ ! -f ~/.aliasme/path ]]; then - mkdir -p ~/.aliasme && touch ~/.aliasme/path - fi +# Run against a throwaway store so the suite never touches the real ~/.aliasme +ALIASME_DIR=$(mktemp -d "${TMPDIR:-/tmp}/aliasme_test.XXXXXX") +export ALIASME_DIR +cleanup() { + rm -rf "$ALIASME_DIR" } +trap cleanup EXIT -testAlias() { - - name1=testaaa - cmd1=cmdaaa - testAdd "$name1" "$cmd1" - - name2=testbbb - cmd2=cmdbbb - testAdd "$name2" "$cmd2" +# zsh's "." does not search the current directory, so keep the "./" prefix +. ./aliasme.sh - testRemove "$name1" - testRemove "$name2" -} +failures=0 -testAdd() { - _add "$1" "$2" - if [[ $(_list) = *"$1 : $2"* ]]; then - log_success "path test success" +check() { + local desc="$1" expected="$2" actual="$3" + if [ "$expected" = "$actual" ]; then + log_success "$desc" else - log_failure "path test failure" + log_failure "$desc -- expected [$expected] got [$actual]" + failures=$((failures + 1)) fi } -testRemove() { - _remove "$1" - if [[ $(_list) = *"$1"* ]]; then - log_failure "remove test failure" - else - log_success "remove test success" - fi +reset_store() { + rm -f "$ALIASME_CMD" } -testInit -testAlias +log_header "Add and list" + +reset_store +_add hello "echo hello" > /dev/null +_add world "echo world" > /dev/null +check "list renders both entries" \ + "hello : echo hello +world : echo world" "$(_list)" + +check "duplicate name is rejected" "1" "$(_add hello "echo other" > /dev/null 2>&1; echo $?)" +check "empty name is rejected" "1" "$(_add "" "" > /dev/null 2>&1; echo $?)" + +log_header "Remove" + +reset_store +_add keep "echo keep" > /dev/null +_add drop "echo drop" > /dev/null +_remove drop > /dev/null +check "removed entry is gone" "keep : echo keep" "$(_list)" +check "removing an unknown name reports failure" "1" "$(_remove ghost > /dev/null 2>&1; echo $?)" + +log_header "Execute" + +reset_store +_add greet "echo hi" > /dev/null +check "alias runs its command" "hi" "$(al greet)" +check "unknown alias reports not found" "not found: nosuch" "$(al nosuch 2>&1)" +check "unknown alias exits non-zero" "1" "$(al nosuch > /dev/null 2>&1; echo $?)" + +reset_store +_add fails "false" > /dev/null +check "failing command propagates its exit code" "1" "$(al fails > /dev/null 2>&1; echo $?)" +_add code "return 3" > /dev/null +check "exit code is passed through verbatim" "3" "$(al code > /dev/null 2>&1; echo $?)" + +log_header "Name and command collision" + +# A command line whose text equals another alias's name must never be +# mistaken for that name when executing or removing. +reset_store +_add x deploy > /dev/null +_add deploy "echo deploying" > /dev/null +check "lookup skips command lines" "deploying" "$(al deploy)" + +_remove deploy > /dev/null +check "removal skips command lines" "x : deploy" "$(_list)" + +log_header "Escapes and special characters" + +reset_store +_add box 'printf "[%s]\n" done' > /dev/null +_add after "echo STILL_HERE" > /dev/null +check "backslash escapes are stored verbatim" \ + 'box : printf "[%s]\n" done +after : echo STILL_HERE' "$(_list)" +check "entry after an escaped command is still reachable" "STILL_HERE" "$(al after)" +check "escaped command runs correctly" "[done]" "$(al box)" + +reset_store +_add url "echo http://api.com/x?key=1" > /dev/null +check "literal ? in a command survives" "http://api.com/x?key=1" "$(al url)" +check "literal ? does not fail the command" "0" "$(al url > /dev/null 2>&1; echo $?)" + +# Suppressing the "no matches" error must not disable globbing itself. +reset_store +touch "$ALIASME_DIR/globbed.txt" +_add glob "echo $ALIASME_DIR/*.txt" > /dev/null +check "an intentional glob still expands" "$ALIASME_DIR/globbed.txt" "$(al glob)" + +log_header "Variable isolation" + +reset_store +_add show 'echo "[$name][$value][$line]"' > /dev/null +name=OUTER +value=OUTER +line=OUTER +check "aliasme internals do not shadow user variables" \ + "[OUTER][OUTER][OUTER]" "$(al show)" + +if [ "$failures" -gt 0 ]; then + log_failure "$failures test(s) failed" + exit 1 +fi + +log_success "All tests passed!" From c97be24b0340ee6cd96f727764f15c44e448f0fa Mon Sep 17 00:00:00 2001 From: Jintin Date: Mon, 3 Aug 2026 08:11:11 +0800 Subject: [PATCH 2/2] test: fix shellcheck name collisions and a blocking prompt assert.sh declares expected/actual as arrays, so reusing those names for scalars in check() triggered SC2178/SC2128. The empty-name case calls _add with no arguments, which prompts interactively; without stdin redirected the suite blocked forever when run from a terminal. Co-Authored-By: Claude Fable 5 --- test/aliastest.sh | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/test/aliastest.sh b/test/aliastest.sh index 2e08147..7ccde0d 100755 --- a/test/aliastest.sh +++ b/test/aliastest.sh @@ -15,12 +15,13 @@ trap cleanup EXIT failures=0 +# Names are prefixed: assert.sh declares "expected"/"actual" as arrays. check() { - local desc="$1" expected="$2" actual="$3" - if [ "$expected" = "$actual" ]; then - log_success "$desc" + local _desc="$1" _want="$2" _got="$3" + if [ "$_want" = "$_got" ]; then + log_success "$_desc" else - log_failure "$desc -- expected [$expected] got [$actual]" + log_failure "$_desc -- expected [$_want] got [$_got]" failures=$((failures + 1)) fi } @@ -39,7 +40,9 @@ check "list renders both entries" \ world : echo world" "$(_list)" check "duplicate name is rejected" "1" "$(_add hello "echo other" > /dev/null 2>&1; echo $?)" -check "empty name is rejected" "1" "$(_add "" "" > /dev/null 2>&1; echo $?)" +# stdin is closed: with no arguments _add prompts for them interactively, +# and the suite must not block waiting on a terminal. +check "empty name is rejected" "1" "$(_add "" "" > /dev/null 2>&1 < /dev/null; echo $?)" log_header "Remove" @@ -101,6 +104,7 @@ check "an intentional glob still expands" "$ALIASME_DIR/globbed.txt" "$(al glob) log_header "Variable isolation" reset_store +# shellcheck disable=SC2016 # the command is stored literally, on purpose _add show 'echo "[$name][$value][$line]"' > /dev/null name=OUTER value=OUTER