Skip to content

fix: resolve install binary verification, uninstall, and version prefix bugs #535

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
khaliqgant merged 3 commits into from
Mar 10, 2026
Merged

fix: resolve install binary verification, uninstall, and version prefix bugs #535

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d3f2bf7
fix: resolve install script binary verification, uninstall, and versi…
khaliqgant Mar 10, 2026
768241f
fix: remove shell-dependent 2>/dev/null from npm uninstall execCommand
khaliqgant Mar 10, 2026
476a44a
fix: only remove ~/.agent-relay/bin/ not entire ~/.agent-relay directory
khaliqgant Mar 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions install.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ download_broker_binary() {

if curl -fsSL "$download_url" -o "$target_path" 2>/dev/null; then
chmod +x "$target_path"
strip_quarantine "$target_path"
# Verify binary works (Rust clap binary supports --help)
if "$target_path" --help &>/dev/null; then
success "Downloaded broker binary (workflow agent spawning)"
Expand Down Expand Up @@ -217,6 +218,7 @@ download_dashboard_binary() {
if gunzip -c "${temp_file}.gz" > "$target_path" 2>/dev/null; then
rm -f "${temp_file}.gz"
chmod +x "$target_path"
strip_quarantine "$target_path"

if "$target_path" --version &>/dev/null; then
success "Downloaded standalone dashboard-server binary"
Expand Down Expand Up @@ -244,6 +246,7 @@ download_dashboard_binary() {

if [ "$file_size" -gt 1000000 ]; then
chmod +x "$target_path"
strip_quarantine "$target_path"

if "$target_path" --version &>/dev/null; then
success "Downloaded standalone dashboard-server binary"
Expand Down Expand Up @@ -325,6 +328,13 @@ has_command() {
command -v "$1" &> /dev/null
}

# Strip macOS Gatekeeper quarantine attribute from a binary
strip_quarantine() {
if [ "$OS" = "darwin" ] && has_command xattr; then
xattr -d com.apple.quarantine "$1" 2>/dev/null || true
fi
}

# Download relay-acp binary for Zed editor integration
download_relay_acp() {
step "Downloading relay-acp binary (Zed editor integration)..."
Expand Down Expand Up @@ -354,6 +364,7 @@ download_relay_acp() {
if gunzip -c "${temp_file}.gz" > "$target_path" 2>/dev/null; then
rm -f "${temp_file}.gz"
chmod +x "$target_path"
strip_quarantine "$target_path"

if "$target_path" --help &>/dev/null; then
success "Downloaded relay-acp binary (Zed ACP bridge)"
Expand All @@ -379,6 +390,7 @@ download_relay_acp() {

if [ "$file_size" -gt 1000000 ]; then
chmod +x "$target_path"
strip_quarantine "$target_path"

if "$target_path" --help &>/dev/null; then
success "Downloaded relay-acp binary (Zed ACP bridge)"
Expand Down Expand Up @@ -465,6 +477,7 @@ download_standalone_binary() {
if gunzip -c "${temp_file}.gz" > "$target_path" 2>/dev/null; then
rm -f "${temp_file}.gz"
chmod +x "$target_path"
strip_quarantine "$target_path"

# Verify the binary works
if "$target_path" --version &>/dev/null; then
Expand Down Expand Up @@ -501,6 +514,7 @@ download_standalone_binary() {

if [ "$file_size" -gt 1000000 ]; then
chmod +x "$target_path"
strip_quarantine "$target_path"

# Verify the binary works
if "$target_path" --version &>/dev/null; then
Expand Down Expand Up @@ -686,6 +700,18 @@ verify_installation() {
if command -v agent-relay &> /dev/null; then
local installed_version=$(agent-relay --version 2>/dev/null || echo "unknown")
success "agent-relay $installed_version installed successfully!"

# Warn if another version shadows the one we just installed
local which_path=$(command -v agent-relay)
if [ -x "$BIN_DIR/agent-relay" ] && [ "$which_path" != "$BIN_DIR/agent-relay" ]; then
local other_version=$("$BIN_DIR/agent-relay" --version 2>/dev/null || echo "unknown")
if [ "$installed_version" != "$other_version" ]; then
warn "Another agent-relay ($installed_version) at $which_path shadows the newly installed $other_version at $BIN_DIR/agent-relay"
echo " To fix, either:"
echo " 1. Uninstall the old version: npm uninstall -g agent-relay"
echo " 2. Or ensure $BIN_DIR is earlier in your PATH"
fi
fi
elif [ -x "$BIN_DIR/agent-relay" ]; then
local installed_version=$("$BIN_DIR/agent-relay" --version 2>/dev/null || echo "unknown")
success "agent-relay $installed_version installed to $BIN_DIR"
Expand Down
16 changes: 13 additions & 3 deletions packages/sdk/src/client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -743,8 +743,10 @@
timeout: 15_000,
stdio: ['pipe', 'pipe', 'pipe'],
}).toString();
const match = result.match(/"tag_name"\s*:\s*"v?([^"]+)"/);
return match?.[1] ?? null;
const match = result.match(/"tag_name"\s*:\s*"([^"]+)"/);
if (!match?.[1]) return null;
// Strip tag prefixes: "openclaw-v3.1.18" -> "3.1.18", "v3.1.18" -> "3.1.18"
return match[1].replace(/^openclaw-/, '').replace(/^v/, '');
} catch {
return null;
}
Expand Down Expand Up @@ -784,8 +786,16 @@
});
fs.chmodSync(targetPath, 0o755);

// macOS: re-sign to avoid Gatekeeper issues
// macOS: strip quarantine attribute and re-sign to avoid Gatekeeper issues
if (process.platform === 'darwin') {
try {
execSync(`xattr -d com.apple.quarantine "${targetPath}" 2>/dev/null || true`, {
timeout: 10_000,
stdio: ['pipe', 'pipe', 'pipe'],
});
} catch {

Check warning

Code scanning / CodeQL

Indirect uncontrolled command line Medium

This command depends on an unsanitized
environment variable
Loading
.
This command depends on an unsanitized
environment variable
Loading
.

Copilot Autofix

AI about 2 months ago

In general, the safest fix is to avoid exec/execSync with a single shell command string when any part of that string comes from untrusted or tainted data. Instead, use execFile/execFileSync (or spawn/spawnSync) and pass arguments as an array of strings; this avoids shell parsing, redirection, and environment-variable expansion. For cases where you truly need shell features like redirection (2>/dev/null) or || true, wrap that in a small, fixed shell snippet and keep all untrusted data passed as separate arguments, or emulate the behavior in JavaScript (e.g., ignore errors instead of || true, discard stderr instead of 2>/dev/null).

Here, we only need to (a) run xattr -d com.apple.quarantine <targetPath> and ignore any errors, and (b) run codesign --force --sign - <targetPath> and ignore errors. Both can be implemented with execFileSync without any shell metacharacters, and the “ignore failure” behavior can be preserved by wrapping each call in a try/catch as already done. So the single best fix is:

  • Add execFileSync to the existing import from node:child_process.
  • Replace:
    • execSync(\xattr -d com.apple.quarantine "${targetPath}" 2>/dev/null || true`, ...)withexecFileSync('xattr', ['-d', 'com.apple.quarantine', targetPath], ...). The previous 2>/dev/null || true` is unnecessary because:
      • stdio: ['pipe','pipe','pipe'] already prevents stderr from cluttering the console.
      • The try/catch around the call already makes errors non-fatal.
  • Replace:
    • execSync(\codesign --force --sign - "${targetPath}"`, ...)withexecFileSync('codesign', ['--force', '--sign', '-', targetPath], ...)`.

This preserves all semantics (timeout, stdio behavior, non-fatal nature of failures), removes any use of an interpolated shell command string, and thus addresses all variants of the alert at this location.

Suggested changeset 1
packages/sdk/src/client.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/sdk/src/client.ts b/packages/sdk/src/client.ts
--- a/packages/sdk/src/client.ts
+++ b/packages/sdk/src/client.ts
@@ -1,5 +1,5 @@
 import { once } from 'node:events';
-import { execSync, spawn, type ChildProcessWithoutNullStreams } from 'node:child_process';
+import { execFileSync, execSync, spawn, type ChildProcessWithoutNullStreams } from 'node:child_process';
 import { createInterface, type Interface as ReadlineInterface } from 'node:readline';
 import fs from 'node:fs';
 import os from 'node:os';
@@ -793,7 +793,7 @@
     // macOS: strip quarantine attribute and re-sign to avoid Gatekeeper issues
     if (process.platform === 'darwin') {
       try {
-        execSync(`xattr -d com.apple.quarantine "${targetPath}" 2>/dev/null || true`, {
+        execFileSync('xattr', ['-d', 'com.apple.quarantine', targetPath], {
           timeout: 10_000,
           stdio: ['pipe', 'pipe', 'pipe'],
         });
@@ -801,7 +801,7 @@
         // Non-fatal
       }
       try {
-        execSync(`codesign --force --sign - "${targetPath}"`, {
+        execFileSync('codesign', ['--force', '--sign', '-', targetPath], {
           timeout: 10_000,
           stdio: ['pipe', 'pipe', 'pipe'],
         });
EOF
@@ -1,5 +1,5 @@
import { once } from 'node:events';
import { execSync, spawn, type ChildProcessWithoutNullStreams } from 'node:child_process';
import { execFileSync, execSync, spawn, type ChildProcessWithoutNullStreams } from 'node:child_process';
import { createInterface, type Interface as ReadlineInterface } from 'node:readline';
import fs from 'node:fs';
import os from 'node:os';
@@ -793,7 +793,7 @@
// macOS: strip quarantine attribute and re-sign to avoid Gatekeeper issues
if (process.platform === 'darwin') {
try {
execSync(`xattr -d com.apple.quarantine "${targetPath}" 2>/dev/null || true`, {
execFileSync('xattr', ['-d', 'com.apple.quarantine', targetPath], {
timeout: 10_000,
stdio: ['pipe', 'pipe', 'pipe'],
});
@@ -801,7 +801,7 @@
// Non-fatal
}
try {
execSync(`codesign --force --sign - "${targetPath}"`, {
execFileSync('codesign', ['--force', '--sign', '-', targetPath], {
timeout: 10_000,
stdio: ['pipe', 'pipe', 'pipe'],
});
Copilot is powered by AI and may make mistakes. Always verify output.
// Non-fatal
}
try {
execSync(`codesign --force --sign - "${targetPath}"`, {
timeout: 10_000,
Expand Down
50 changes: 50 additions & 0 deletions src/cli/lib/core-maintenance.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,56 @@ export async function runUninstallCommand(
removeZedConfig(serverName, deps.fs, isDryRun, deps.log);
}

// --- Binary removal (standalone binaries + npm packages) ---
const homeDir = os.homedir();
const standaloneBinDir = path.join(homeDir, '.local', 'bin');
const installBinDir = path.join(homeDir, '.agent-relay', 'bin');

// Remove standalone binaries from ~/.local/bin
for (const binaryName of ['agent-relay', 'relay-dashboard-server', 'relay-acp']) {
const binPath = path.join(standaloneBinDir, binaryName);
if (deps.fs.existsSync(binPath)) {
if (isDryRun) {
deps.log(`[dry-run] Would remove binary: ${binPath}`);
} else {
try {
deps.fs.unlinkSync(binPath);
deps.log(`Removed ${binPath}`);
} catch {
// Best-effort.
}
}
}
}

// Remove broker binary from ~/.agent-relay/bin/ (not the parent dir which stores global data)
if (deps.fs.existsSync(installBinDir)) {
if (isDryRun) {
deps.log(`[dry-run] Would remove directory: ${installBinDir}`);
} else {
try {
deps.fs.rmSync(installBinDir, { recursive: true, force: true });
deps.log(`Removed ${installBinDir}`);
} catch {
// Best-effort.
}
}
}

// Remove npm-installed packages
if (!isDryRun) {
for (const pkg of ['agent-relay', '@agent-relay/dashboard-server', '@agent-relay/acp-bridge']) {
try {
await deps.execCommand(`npm uninstall -g ${pkg}`);
deps.log(`Uninstalled npm package: ${pkg}`);
} catch {
// Package may not be installed via npm — that's fine.
}
}
} else {
deps.log('[dry-run] Would run: npm uninstall -g agent-relay @agent-relay/dashboard-server @agent-relay/acp-bridge');
}

// --- Snippet cleanup (CLAUDE.md, GEMINI.md, AGENTS.md) ---
if (options.snippets) {
for (const fileName of SNIPPET_TARGET_FILES) {
Expand Down
Loading