Skip to content

Commit 15e6e98

Browse files
fix(codebuddy): resolve installer path issues, unused vars, and uninstall safety
1 parent fd0877c commit 15e6e98

6 files changed

Lines changed: 97 additions & 62 deletions

File tree

.codebuddy/README.zh-CN.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ cd /path/to/your/project
7272
├── ecc-install-state.json # 安装状态跟踪
7373
├── install.sh # 旧版安装脚本
7474
├── uninstall.sh # 旧版卸载脚本
75-
└── README.md # 此文件
75+
└── README.zh-CN.md # 此文件
7676
```
7777

7878
## Target Adapter 安装的优势

.codebuddy/install.js

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,6 @@ function doInstall() {
192192
let agents = 0;
193193
let skills = 0;
194194
let rules = 0;
195-
let other = 0;
196195

197196
// Copy commands
198197
const commandsDir = path.join(repoRoot, 'commands');
@@ -268,27 +267,14 @@ function doInstall() {
268267
}
269268
}
270269

271-
// Copy README files
270+
// Copy README files (skip install/uninstall scripts to avoid broken
271+
// path references when the copied script runs from the target directory)
272272
const readmeFiles = ['README.md', 'README.zh-CN.md'];
273273
for (const readmeFile of readmeFiles) {
274274
const sourcePath = path.join(scriptDir, readmeFile);
275275
if (fs.existsSync(sourcePath)) {
276276
const targetPath = path.join(codebuddyFullPath, readmeFile);
277-
if (copyManagedFile(sourcePath, targetPath, manifest, readmeFile)) {
278-
other += 1;
279-
}
280-
}
281-
}
282-
283-
// Copy install and uninstall scripts
284-
const scriptFiles = ['install.sh', 'uninstall.sh', 'install.js', 'uninstall.js'];
285-
for (const scriptFile of scriptFiles) {
286-
const sourcePath = path.join(scriptDir, scriptFile);
287-
if (fs.existsSync(sourcePath)) {
288-
const targetPath = path.join(codebuddyFullPath, scriptFile);
289-
if (copyManagedFile(sourcePath, targetPath, manifest, scriptFile, true)) {
290-
other += 1;
291-
}
277+
copyManagedFile(sourcePath, targetPath, manifest, readmeFile);
292278
}
293279
}
294280

.codebuddy/install.sh

Lines changed: 25 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,29 @@ set -euo pipefail
1313
# When globs match nothing, expand to empty list instead of the literal pattern
1414
shopt -s nullglob
1515

16-
# Resolve the directory where this script lives (the repo root)
16+
# Resolve the directory where this script lives
1717
SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
18-
REPO_ROOT="$(dirname "$SCRIPT_DIR")"
18+
19+
# Locate the ECC repo root by walking up from SCRIPT_DIR to find the marker
20+
# file (VERSION). This keeps the script working even when it has been copied
21+
# into a target project's .codebuddy/ directory.
22+
find_repo_root() {
23+
local dir="$(dirname "$SCRIPT_DIR")"
24+
# First try the parent of SCRIPT_DIR (original layout: .codebuddy/ lives in repo root)
25+
if [ -f "$dir/VERSION" ] && [ -d "$dir/commands" ] && [ -d "$dir/agents" ]; then
26+
echo "$dir"
27+
return 0
28+
fi
29+
echo ""
30+
return 1
31+
}
32+
33+
REPO_ROOT="$(find_repo_root)"
34+
if [ -z "$REPO_ROOT" ]; then
35+
echo "Error: Cannot locate the ECC repository root."
36+
echo "This script must be run from within the ECC repository's .codebuddy/ directory."
37+
exit 1
38+
fi
1939

2040
# CodeBuddy directory name
2141
CODEBUDDY_DIR=".codebuddy"
@@ -111,7 +131,6 @@ do_install() {
111131
agents=0
112132
skills=0
113133
rules=0
114-
other=0
115134

116135
# Copy commands from repo root
117136
if [ -d "$REPO_ROOT/commands" ]; then
@@ -174,25 +193,13 @@ do_install() {
174193
done < <(find "$REPO_ROOT/rules" -type f | sort)
175194
fi
176195

177-
# Copy README files from this directory
196+
# Copy README files (skip install/uninstall scripts to avoid broken
197+
# path references when the copied script runs from the target directory)
178198
for readme_file in "$SCRIPT_DIR/README.md" "$SCRIPT_DIR/README.zh-CN.md"; do
179199
if [ -f "$readme_file" ]; then
180200
local_name=$(basename "$readme_file")
181201
target_path="$codebuddy_full_path/$local_name"
182-
if copy_managed_file "$readme_file" "$target_path" "$MANIFEST" "$local_name"; then
183-
other=$((other + 1))
184-
fi
185-
fi
186-
done
187-
188-
# Copy install and uninstall scripts
189-
for script_file in "$SCRIPT_DIR/install.sh" "$SCRIPT_DIR/uninstall.sh"; do
190-
if [ -f "$script_file" ]; then
191-
local_name=$(basename "$script_file")
192-
target_path="$codebuddy_full_path/$local_name"
193-
if copy_managed_file "$script_file" "$target_path" "$MANIFEST" "$local_name" 1; then
194-
other=$((other + 1))
195-
fi
202+
copy_managed_file "$readme_file" "$target_path" "$MANIFEST" "$local_name" || true
196203
fi
197204
done
198205

.codebuddy/uninstall.js

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,6 @@ const path = require('path');
1313
const os = require('os');
1414
const readline = require('readline');
1515

16-
// Platform detection
17-
const isWindows = process.platform === 'win32';
18-
1916
/**
2017
* Get home directory cross-platform
2118
*/
@@ -120,8 +117,6 @@ async function promptConfirm(question) {
120117
* Main uninstall function
121118
*/
122119
async function doUninstall() {
123-
// Resolve script directory
124-
const scriptDir = path.dirname(path.resolve(__filename));
125120
const codebuddyDirName = '.codebuddy';
126121

127122
// Parse arguments
@@ -211,27 +206,29 @@ async function doUninstall() {
211206
}
212207

213208
const fullPath = path.join(codebuddyFullPath, filePath);
214-
const resolvedFull = resolvePath(fullPath);
215209

216-
// Security check: ensure resolved path is within codebuddy directory
217-
if (!resolvedFull.startsWith(codebuddyRootResolved)) {
218-
console.log(`Skipped: ${filePath} (invalid manifest entry)`);
210+
// Security check: use path.relative() to ensure the manifest entry
211+
// resolves inside the codebuddy directory. This is stricter than
212+
// startsWith and correctly handles edge-cases with symlinks.
213+
const relative = path.relative(codebuddyRootResolved, path.resolve(fullPath));
214+
if (relative.startsWith('..') || path.isAbsolute(relative)) {
215+
console.log(`Skipped: ${filePath} (outside target directory)`);
219216
skipped += 1;
220217
continue;
221218
}
222219

223220
try {
224-
const stats = fs.statSync(resolvedFull);
221+
const stats = fs.lstatSync(fullPath);
225222

226-
if (stats.isFile()) {
227-
fs.unlinkSync(resolvedFull);
223+
if (stats.isFile() || stats.isSymbolicLink()) {
224+
fs.unlinkSync(fullPath);
228225
console.log(`Removed: ${filePath}`);
229226
removed += 1;
230227
} else if (stats.isDirectory()) {
231228
try {
232-
const files = fs.readdirSync(resolvedFull);
229+
const files = fs.readdirSync(fullPath);
233230
if (files.length === 0) {
234-
fs.rmdirSync(resolvedFull);
231+
fs.rmdirSync(fullPath);
235232
console.log(`Removed: ${filePath}/`);
236233
removed += 1;
237234
} else {

.codebuddy/uninstall.sh

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -115,27 +115,28 @@ do_uninstall() {
115115
fi
116116

117117
full_path="$codebuddy_full_path/$file_path"
118-
resolved_full="$(resolve_path "$full_path")"
119118

120-
case "$resolved_full" in
121-
"$codebuddy_root_resolved"|"$codebuddy_root_resolved"/*)
122-
;;
123-
*)
124-
echo "Skipped: $file_path (invalid manifest entry)"
119+
# Security check: ensure the path resolves inside the target directory.
120+
# Use Python to compute a reliable relative path so symlinks cannot
121+
# escape the boundary.
122+
relative="$(python3 -c 'import os,sys; print(os.path.relpath(os.path.abspath(sys.argv[1]), sys.argv[2]))' "$full_path" "$codebuddy_root_resolved")"
123+
case "$relative" in
124+
../*|..)
125+
echo "Skipped: $file_path (outside target directory)"
125126
skipped=$((skipped + 1))
126127
continue
127128
;;
128129
esac
129130

130-
if [ -f "$resolved_full" ]; then
131-
rm -f "$resolved_full"
131+
if [ -L "$full_path" ] || [ -f "$full_path" ]; then
132+
rm -f "$full_path"
132133
echo "Removed: $file_path"
133134
removed=$((removed + 1))
134-
elif [ -d "$resolved_full" ]; then
135+
elif [ -d "$full_path" ]; then
135136
# Only remove directory if it's empty
136-
if [ -z "$(ls -A "$resolved_full" 2>/dev/null)" ]; then
137-
rmdir "$resolved_full" 2>/dev/null || true
138-
if [ ! -d "$resolved_full" ]; then
137+
if [ -z "$(ls -A "$full_path" 2>/dev/null)" ]; then
138+
rmdir "$full_path" 2>/dev/null || true
139+
if [ ! -d "$full_path" ]; then
139140
echo "Removed: $file_path/"
140141
removed=$((removed + 1))
141142
fi

tests/lib/install-targets.test.js

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,50 @@ function runTests() {
286286
);
287287
})) passed++; else failed++;
288288

289+
if (test('every schema target enum value has a matching adapter (regression guard)', () => {
290+
const schemaPath = path.join(__dirname, '..', '..', 'schemas', 'ecc-install-config.schema.json');
291+
const schema = JSON.parse(require('fs').readFileSync(schemaPath, 'utf8'));
292+
const schemaTargets = schema.properties.target.enum;
293+
const adapters = listInstallTargetAdapters();
294+
const adapterTargets = adapters.map(a => a.target);
295+
296+
for (const target of schemaTargets) {
297+
assert.ok(
298+
adapterTargets.includes(target),
299+
`Schema target "${target}" has no matching adapter. ` +
300+
`Available adapter targets: ${adapterTargets.join(', ')}`
301+
);
302+
}
303+
})) passed++; else failed++;
304+
305+
if (test('every adapter target is listed in the schema enum (regression guard)', () => {
306+
const schemaPath = path.join(__dirname, '..', '..', 'schemas', 'ecc-install-config.schema.json');
307+
const schema = JSON.parse(require('fs').readFileSync(schemaPath, 'utf8'));
308+
const schemaTargets = schema.properties.target.enum;
309+
const adapters = listInstallTargetAdapters();
310+
311+
for (const adapter of adapters) {
312+
assert.ok(
313+
schemaTargets.includes(adapter.target),
314+
`Adapter target "${adapter.target}" is not in schema enum. ` +
315+
`Schema targets: ${schemaTargets.join(', ')}`
316+
);
317+
}
318+
})) passed++; else failed++;
319+
320+
if (test('every adapter target is in SUPPORTED_INSTALL_TARGETS (regression guard)', () => {
321+
const { SUPPORTED_INSTALL_TARGETS } = require('../../scripts/lib/install-manifests');
322+
const adapters = listInstallTargetAdapters();
323+
324+
for (const adapter of adapters) {
325+
assert.ok(
326+
SUPPORTED_INSTALL_TARGETS.includes(adapter.target),
327+
`Adapter target "${adapter.target}" is not in SUPPORTED_INSTALL_TARGETS. ` +
328+
`Supported: ${SUPPORTED_INSTALL_TARGETS.join(', ')}`
329+
);
330+
}
331+
})) passed++; else failed++;
332+
289333
console.log(`\nResults: Passed: ${passed}, Failed: ${failed}`);
290334
process.exit(failed > 0 ? 1 : 0);
291335
}

0 commit comments

Comments
 (0)