Skip to content

Commit ed28e01

Browse files
Ebonsignoriclaude
andcommitted
fix: address PR review comments for alias system
Use yq strenv() for safe variable passing instead of string interpolation to prevent injection via alias names/values. Validate alias names against ^[a-zA-Z0-9_-]+$. Use (.value | tostring) for robust alias listing. Fix glob expansion in alias resolution with read -r -a. Add e2e test coverage for alias set, list, remove, and invalid name rejection. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 6a9e3aa commit ed28e01

2 files changed

Lines changed: 65 additions & 8 deletions

File tree

src/crabcode

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,7 @@ resolve_command_aliases() {
309309
fi
310310

311311
local resolved
312-
resolved=$(yq -r ".aliases.\"$cmd\" // \"\"" "$GLOBAL_CONFIG" 2>/dev/null)
312+
resolved=$(cmd="$cmd" yq -r '.aliases.[strenv(cmd)] // ""' "$GLOBAL_CONFIG" 2>/dev/null)
313313
if [ -n "$resolved" ] && [ "$resolved" != "null" ]; then
314314
echo "$resolved"
315315
return 0
@@ -8047,7 +8047,7 @@ handle_alias_command() {
80478047
fi
80488048

80498049
local aliases
8050-
aliases=$(yq -r '.aliases // {} | to_entries[] | .key + " → " + .value' "$GLOBAL_CONFIG" 2>/dev/null)
8050+
aliases=$(yq -r '.aliases // {} | to_entries[] | .key + " → " + (.value | tostring)' "$GLOBAL_CONFIG" 2>/dev/null)
80518051
if [ -z "$aliases" ]; then
80528052
echo -e "${YELLOW}No aliases configured.${NC}"
80538053
echo ""
@@ -8071,6 +8071,12 @@ handle_alias_command() {
80718071
error "Usage: crab alias set <name> <command...>"
80728072
exit 1
80738073
fi
8074+
# Validate alias name
8075+
if ! [[ "$name" =~ ^[a-zA-Z0-9_-]+$ ]]; then
8076+
error "Invalid alias name '$name'. Use only letters, numbers, hyphens, and underscores."
8077+
exit 1
8078+
fi
8079+
80748080
shift 2
80758081
local value="$*"
80768082
if [ -z "$value" ]; then
@@ -8084,7 +8090,7 @@ handle_alias_command() {
80848090
echo "{}" > "$GLOBAL_CONFIG"
80858091
fi
80868092

8087-
yq -i ".aliases.\"$name\" = \"$value\"" "$GLOBAL_CONFIG"
8093+
NAME="$name" VALUE="$value" yq -i '.aliases.[strenv(NAME)] = strenv(VALUE)' "$GLOBAL_CONFIG"
80888094
echo -e "${GREEN}Alias set:${NC} $name$value"
80898095
;;
80908096
"rm"|"remove"|"delete")
@@ -8100,13 +8106,13 @@ handle_alias_command() {
81008106
fi
81018107

81028108
local existing
8103-
existing=$(yq -r ".aliases.\"$name\" // \"\"" "$GLOBAL_CONFIG" 2>/dev/null)
8109+
existing=$(NAME="$name" yq -r '.aliases.[strenv(NAME)] // ""' "$GLOBAL_CONFIG" 2>/dev/null)
81048110
if [ -z "$existing" ] || [ "$existing" = "null" ]; then
81058111
error "Alias '$name' not found"
81068112
exit 1
81078113
fi
81088114

8109-
yq -i "del(.aliases.\"$name\")" "$GLOBAL_CONFIG"
8115+
NAME="$name" yq -i 'del(.aliases.[strenv(NAME)])' "$GLOBAL_CONFIG"
81108116

81118117
# Clean up empty aliases map
81128118
local remaining
@@ -8144,9 +8150,9 @@ main() {
81448150
# Resolve user-configured command aliases (before project + command dispatch)
81458151
if resolved=$(resolve_command_aliases "${1:-}"); then
81468152
shift
8147-
# Split resolved alias into words and prepend to remaining args
8148-
# shellcheck disable=SC2086
8149-
set -- $resolved "$@"
8153+
# Split resolved alias into words without glob expansion
8154+
read -r -a resolved_parts <<<"$resolved"
8155+
set -- "${resolved_parts[@]}" "$@"
81508156
fi
81518157

81528158
# For project-aware commands, resolve project (cwd-first, then default)

tests/e2e/run_e2e.sh

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -423,6 +423,57 @@ rm -rf test-share-dir
423423

424424
echo ""
425425

426+
# =============================================================================
427+
# Test 12: Command Aliases
428+
# =============================================================================
429+
430+
log "Testing command aliases..."
431+
432+
# List aliases (should be empty initially)
433+
run_test "crab alias (empty)" \
434+
"crab alias" \
435+
"No aliases configured"
436+
437+
# Set a single-word alias
438+
run_test "crab alias set (single word)" \
439+
"crab alias set cu cleanup" \
440+
"Alias set.*cu.*cleanup"
441+
442+
# List aliases (should show the alias)
443+
run_test "crab alias (lists alias)" \
444+
"crab alias" \
445+
"cu.*cleanup"
446+
447+
# Set a multi-word alias
448+
run_test "crab alias set (multi-word)" \
449+
"crab alias set wn 'ws new'" \
450+
"Alias set.*wn.*ws new"
451+
452+
# List aliases (should show both)
453+
run_test "crab alias (lists multiple)" \
454+
"crab alias" \
455+
"cu.*cleanup"
456+
457+
# Remove an alias
458+
run_test "crab alias rm" \
459+
"crab alias rm cu" \
460+
"Removed alias.*cu"
461+
462+
# Verify removed alias is gone
463+
run_test "crab alias rm (alias removed)" \
464+
"crab alias rm cu 2>&1" \
465+
"not found"
466+
467+
# Invalid alias name rejected
468+
run_test "crab alias set (invalid name rejected)" \
469+
"crab alias set 'bad name!' cleanup 2>&1" \
470+
"Invalid alias name"
471+
472+
# Clean up remaining test alias
473+
crab alias rm wn 2>/dev/null || true
474+
475+
echo ""
476+
426477
# =============================================================================
427478
# Summary
428479
# =============================================================================

0 commit comments

Comments
 (0)