Skip to content

Commit cd5116b

Browse files
wgsimclaude
andcommitted
security: Fix 3 critical issues from external review round 4 (v2.0.8)
This commit addresses all FAIL/CRITICAL issues identified by Codex and Gemini verification reviews of v2.0.6 and v2.0.7. CRITICAL SECURITY FIX #1: Command substitution after secret export ────────────────────────────────────────────────────────────────────── Issue: External dirname command executed after API key fetch/export, enabling PATH poisoning to leak credentials Codex PoC: Successfully leaked LINUX_FETCHED_SECRET_789 via malicious dirname in PATH Fix: Replaced $(dirname "$var") with ${var%/*} shell builtin - bin/glm-mcp-wrapper:161 - npx_dir="${npx_bin%/*}" - bin/claude-by-glm:303 - claude_dir="${claude_bin%/*}" Impact: Eliminates credential leak surface completely by avoiding external command execution after secrets are in environment CRITICAL PORTABILITY FIX #2: realpath -m regression ────────────────────────────────────────────────────────────────────── Issue: Plain realpath requires paths to exist, breaking: - Fresh installations (install dir doesn't exist yet) - Clean home scenarios (sessions dir not created yet) - First-run validation (paths validated before creation) Affected: scripts/install.sh:43, scripts/uninstall.sh:46, bin/glm-cleanup-sessions:121,125,242,243 (6 locations) Fix: Added canonicalize_path() function to scripts/common-utils.sh - Portable to both GNU (Linux) and BSD (macOS) - Works with non-existent paths by canonicalizing parent + basename - Falls back gracefully when realpath unavailable - Handles recursive parent canonicalization Impact: Restores installation and cleanup functionality on macOS and in all first-run scenarios CRITICAL FUNCTIONALITY FIX #3: Incomplete nvm/volta support ────────────────────────────────────────────────────────────────────── Issue: bin/glm-mcp-wrapper was updated for nvm/volta in v2.0.6, but bin/claude-by-glm validation was not, blocking nvm/volta users Fix: Added nvm/volta paths to claude validation trust pattern - bin/claude-by-glm:291 - Added "$HOME"/.nvm/*/bin/claude - bin/claude-by-glm:291 - Added "$HOME"/.volta/bin/claude - Updated error message to include new trusted paths Impact: nvm/volta users can now use claude-by-glm wrapper without "Untrusted claude path" errors Verification: ───────────── ✅ Syntax checks: PASS (all 7 modified files) ✅ Security scan: PASS (gitleaks found no issues) ✅ Shellcheck: PASS (no new warnings) Security Properties Maintained: ──────────────────────────────── ✅ PATH hardening intact (trusted paths only) ✅ Binary validation enforced (nvm/volta now trusted) ✅ Config isolation preserved (CLAUDE_CONFIG_DIR separation) ✅ No command substitution after secret operations ✅ Path canonicalization prevents traversal attacks External Review Status: ─────────────────────── Round 4 identified 3 CRITICAL issues in v2.0.6/v2.0.7 All 3 issues now resolved in v2.0.8 Ready for verification review round 5 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent e155735 commit cd5116b

7 files changed

Lines changed: 69 additions & 27 deletions

File tree

VERSION

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
2.0.7
1+
2.0.8

bin/claude-by-glm

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -287,20 +287,22 @@ EOF
287287
claude_bin="$(command -v claude)" || handle_error "claude binary not found in PATH"
288288

289289
# Validate claude is in a trusted location to prevent PATH poisoning
290+
# Support nvm/volta installations as well
290291
case "$claude_bin" in
291-
/usr/bin/claude|/usr/local/bin/claude|/opt/homebrew/bin/claude|"$HOME/.claude-glm-mcp/bin/claude")
292+
/usr/bin/claude|/usr/local/bin/claude|/opt/homebrew/bin/claude|"$HOME/.claude-glm-mcp/bin/claude"|"$HOME"/.nvm/*/bin/claude|"$HOME"/.volta/bin/claude)
292293
# Trusted path - continue
293294
;;
294295
*)
295-
handle_error "Untrusted claude path: $claude_bin (expected in /usr/bin, /usr/local/bin, /opt/homebrew/bin, or ~/.claude-glm-mcp/bin)"
296+
handle_error "Untrusted claude path: $claude_bin (expected in /usr/bin, /usr/local/bin, /opt/homebrew/bin, ~/.claude-glm-mcp/bin, ~/.nvm/*/bin, or ~/.volta/bin)"
296297
;;
297298
esac
298299

299300
# Use minimal trusted PATH to prevent PATH poisoning attacks
300301
# Include claude_bin directory to support nvm/volta users
301302
# Don't use exec - let trap cleanup run on exit
303+
# Use shell builtin to avoid command execution after secret fetch
302304
local claude_dir
303-
claude_dir="$(dirname "$claude_bin")"
305+
claude_dir="${claude_bin%/*}"
304306

305307
env -i \
306308
PATH="/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin:/opt/homebrew/bin:$HOME/.claude-glm-mcp/bin:$claude_dir" \

bin/glm-cleanup-sessions

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -117,12 +117,13 @@ EXPECTED_BASE="$HOME/.claude-glm"
117117
SESSIONS_DIR="${CLAUDE_CONFIG_DIR:-$EXPECTED_BASE}/glm-sessions"
118118

119119
# Validate sessions directory is within expected location (prevent path manipulation)
120+
# Use portable canonicalization that works with non-existent directories
120121
if command -v realpath &>/dev/null; then
121-
canonical_dir="$(realpath "$SESSIONS_DIR" 2>/dev/null)" || {
122+
canonical_dir="$(canonicalize_path "$SESSIONS_DIR")" || {
122123
print_error "Cannot resolve sessions directory: $SESSIONS_DIR"
123124
exit 1
124125
}
125-
canonical_expected="$(realpath "$EXPECTED_BASE/glm-sessions" 2>/dev/null)" || {
126+
canonical_expected="$(canonicalize_path "$EXPECTED_BASE/glm-sessions")" || {
126127
print_error "Cannot resolve expected directory"
127128
exit 1
128129
}
@@ -237,9 +238,10 @@ if [[ ${#DELETE_SESSIONS[@]} -gt 0 ]]; then
237238
session_file="$SESSIONS_DIR/$session_id.json"
238239

239240
# Canonicalize path and verify it's within SESSIONS_DIR
241+
# Use portable canonicalization that works with non-existent files
240242
if command -v realpath &>/dev/null; then
241-
canonical_path="$(realpath "$session_file" 2>/dev/null || echo "$session_file")"
242-
canonical_dir="$(realpath "$SESSIONS_DIR" 2>/dev/null || echo "$SESSIONS_DIR")"
243+
canonical_path="$(canonicalize_path "$session_file" || echo "$session_file")"
244+
canonical_dir="$(canonicalize_path "$SESSIONS_DIR" || echo "$SESSIONS_DIR")"
243245

244246
if [[ "$canonical_path" != "$canonical_dir"/* ]]; then
245247
print_error "Session path outside sessions directory: $session_id"

bin/glm-mcp-wrapper

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,8 +157,9 @@ main() {
157157
# Execute the MCP server with sanitized environment
158158
# Only pass essential variables to prevent leakage
159159
# Note: npx_bin already validated, so we include its directory in PATH
160+
# Use shell builtin to avoid command execution after secret export
160161
local npx_dir
161-
npx_dir="$(dirname "$npx_bin")"
162+
npx_dir="${npx_bin%/*}"
162163

163164
exec env -i \
164165
PATH="/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin:/opt/homebrew/bin:$npx_dir" \

scripts/common-utils.sh

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,51 @@ handle_error() {
5454
exit 1
5555
}
5656

57+
# Portable path canonicalization that works with non-existent paths
58+
# Works on both GNU (Linux) and BSD (macOS) systems
59+
# Args: $1 - path to canonicalize (may or may not exist)
60+
# Returns: canonical absolute path via stdout, or empty on error
61+
canonicalize_path() {
62+
local target="$1"
63+
64+
# Empty path is invalid
65+
[[ -n "$target" ]] || return 1
66+
67+
# If realpath not available, return as-is
68+
if ! command -v realpath &>/dev/null; then
69+
echo "$target"
70+
return 0
71+
fi
72+
73+
# If path exists, use realpath directly
74+
if [[ -e "$target" ]]; then
75+
realpath "$target" 2>/dev/null || return 1
76+
return 0
77+
fi
78+
79+
# Path doesn't exist - canonicalize parent + basename
80+
# This is portable to both GNU and BSD realpath
81+
local parent
82+
local basename
83+
parent="$(dirname "$target")"
84+
basename="$(basename "$target")"
85+
86+
# Canonicalize parent directory (which should exist or be /)
87+
local canonical_parent
88+
if [[ "$parent" == "." ]]; then
89+
canonical_parent="$(pwd)"
90+
elif [[ -e "$parent" ]]; then
91+
canonical_parent="$(realpath "$parent" 2>/dev/null)" || return 1
92+
else
93+
# Parent doesn't exist either - recursively canonicalize
94+
canonical_parent="$(canonicalize_path "$parent")" || return 1
95+
fi
96+
97+
# Combine canonical parent with basename
98+
echo "${canonical_parent%/}/$basename"
99+
return 0
100+
}
101+
57102
# ============================================================================
58103
# OS Detection
59104
# ============================================================================

scripts/install.sh

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -37,16 +37,12 @@ validate_install_dir() {
3737
return 1
3838
fi
3939

40-
# Canonicalize if realpath available
40+
# Canonicalize path (works with non-existent paths on both GNU/BSD)
4141
local canonical_dir
42-
if command -v realpath &>/dev/null; then
43-
canonical_dir="$(realpath "$raw")" || {
44-
print_error "Cannot resolve INSTALL_DIR: $raw"
45-
return 1
46-
}
47-
else
48-
canonical_dir="$raw"
49-
fi
42+
canonical_dir="$(canonicalize_path "$raw")" || {
43+
print_error "Cannot resolve INSTALL_DIR: $raw"
44+
return 1
45+
}
5046

5147
# Reject unsafe directories
5248
local canonical_home

scripts/uninstall.sh

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -40,16 +40,12 @@ validate_install_dir() {
4040
return 1
4141
fi
4242

43-
# Canonicalize if realpath available
43+
# Canonicalize path (works with non-existent paths on both GNU/BSD)
4444
local canonical_dir
45-
if command -v realpath &>/dev/null; then
46-
canonical_dir="$(realpath "$raw")" || {
47-
print_error "Cannot resolve INSTALL_DIR: $raw"
48-
return 1
49-
}
50-
else
51-
canonical_dir="$raw"
52-
fi
45+
canonical_dir="$(canonicalize_path "$raw")" || {
46+
print_error "Cannot resolve INSTALL_DIR: $raw"
47+
return 1
48+
}
5349

5450
# Reject unsafe directories
5551
local canonical_home

0 commit comments

Comments
 (0)