From ea450853a8980b4246fc7d8e8162bf05f2eb45a0 Mon Sep 17 00:00:00 2001 From: Affaan Mustafa Date: Sun, 29 Mar 2026 21:34:36 -0400 Subject: [PATCH] fix: harden trae install ownership --- .trae/install.sh | 67 ++++++----- tests/scripts/trae-install.test.js | 179 +++++++++++++++++++++++++++++ 2 files changed, 219 insertions(+), 27 deletions(-) create mode 100644 tests/scripts/trae-install.test.js diff --git a/.trae/install.sh b/.trae/install.sh index da0d860c..959579e7 100755 --- a/.trae/install.sh +++ b/.trae/install.sh @@ -39,6 +39,40 @@ ensure_manifest_entry() { fi } +manifest_has_entry() { + local manifest="$1" + local entry="$2" + + [ -f "$manifest" ] && grep -Fqx "$entry" "$manifest" +} + +copy_managed_file() { + local source_path="$1" + local target_path="$2" + local manifest="$3" + local manifest_entry="$4" + local make_executable="${5:-0}" + + local already_managed=0 + if manifest_has_entry "$manifest" "$manifest_entry"; then + already_managed=1 + fi + + if [ -f "$target_path" ]; then + if [ "$already_managed" -eq 1 ]; then + ensure_manifest_entry "$manifest" "$manifest_entry" + fi + return 1 + fi + + cp "$source_path" "$target_path" + if [ "$make_executable" -eq 1 ]; then + chmod +x "$target_path" + fi + ensure_manifest_entry "$manifest" "$manifest_entry" + return 0 +} + # Install function do_install() { local target_dir="$PWD" @@ -95,12 +129,8 @@ do_install() { [ -f "$f" ] || continue local_name=$(basename "$f") target_path="$trae_full_path/commands/$local_name" - if [ ! -f "$target_path" ]; then - cp "$f" "$target_path" - ensure_manifest_entry "$MANIFEST" "commands/$local_name" + if copy_managed_file "$f" "$target_path" "$MANIFEST" "commands/$local_name"; then commands=$((commands + 1)) - else - ensure_manifest_entry "$MANIFEST" "commands/$local_name" fi done fi @@ -111,12 +141,8 @@ do_install() { [ -f "$f" ] || continue local_name=$(basename "$f") target_path="$trae_full_path/agents/$local_name" - if [ ! -f "$target_path" ]; then - cp "$f" "$target_path" - ensure_manifest_entry "$MANIFEST" "agents/$local_name" + if copy_managed_file "$f" "$target_path" "$MANIFEST" "agents/$local_name"; then agents=$((agents + 1)) - else - ensure_manifest_entry "$MANIFEST" "agents/$local_name" fi done fi @@ -134,11 +160,9 @@ do_install() { target_path="$target_skill_dir/$relative_path" mkdir -p "$(dirname "$target_path")" - if [ ! -f "$target_path" ]; then - cp "$source_file" "$target_path" + if copy_managed_file "$source_file" "$target_path" "$MANIFEST" "skills/$skill_name/$relative_path"; then skill_copied=1 fi - ensure_manifest_entry "$MANIFEST" "skills/$skill_name/$relative_path" done < <(find "$d" -type f | sort) if [ "$skill_copied" -eq 1 ]; then @@ -154,11 +178,9 @@ do_install() { target_path="$trae_full_path/rules/$relative_path" mkdir -p "$(dirname "$target_path")" - if [ ! -f "$target_path" ]; then - cp "$rule_file" "$target_path" + if copy_managed_file "$rule_file" "$target_path" "$MANIFEST" "rules/$relative_path"; then rules=$((rules + 1)) fi - ensure_manifest_entry "$MANIFEST" "rules/$relative_path" done < <(find "$REPO_ROOT/rules" -type f | sort) fi @@ -167,12 +189,8 @@ do_install() { if [ -f "$readme_file" ]; then local_name=$(basename "$readme_file") target_path="$trae_full_path/$local_name" - if [ ! -f "$target_path" ]; then - cp "$readme_file" "$target_path" - ensure_manifest_entry "$MANIFEST" "$local_name" + if copy_managed_file "$readme_file" "$target_path" "$MANIFEST" "$local_name"; then other=$((other + 1)) - else - ensure_manifest_entry "$MANIFEST" "$local_name" fi fi done @@ -182,13 +200,8 @@ do_install() { if [ -f "$script_file" ]; then local_name=$(basename "$script_file") target_path="$trae_full_path/$local_name" - if [ ! -f "$target_path" ]; then - cp "$script_file" "$target_path" - chmod +x "$target_path" - ensure_manifest_entry "$MANIFEST" "$local_name" + if copy_managed_file "$script_file" "$target_path" "$MANIFEST" "$local_name" 1; then other=$((other + 1)) - else - ensure_manifest_entry "$MANIFEST" "$local_name" fi fi done diff --git a/tests/scripts/trae-install.test.js b/tests/scripts/trae-install.test.js new file mode 100644 index 00000000..8328e81b --- /dev/null +++ b/tests/scripts/trae-install.test.js @@ -0,0 +1,179 @@ +/** + * Tests for .trae/install.sh and .trae/uninstall.sh + */ + +const assert = require('assert'); +const fs = require('fs'); +const os = require('os'); +const path = require('path'); +const { execFileSync } = require('child_process'); + +const REPO_ROOT = path.join(__dirname, '..', '..'); +const INSTALL_SCRIPT = path.join(REPO_ROOT, '.trae', 'install.sh'); +const UNINSTALL_SCRIPT = path.join(REPO_ROOT, '.trae', 'uninstall.sh'); + +function createTempDir(prefix) { + return fs.mkdtempSync(path.join(os.tmpdir(), prefix)); +} + +function cleanup(dirPath) { + fs.rmSync(dirPath, { recursive: true, force: true }); +} + +function runInstall(options = {}) { + return execFileSync('bash', [INSTALL_SCRIPT, ...(options.args || [])], { + cwd: options.cwd, + env: { + ...process.env, + HOME: options.homeDir || process.env.HOME, + }, + encoding: 'utf8', + stdio: ['pipe', 'pipe', 'pipe'], + timeout: 20000, + }); +} + +function runUninstall(options = {}) { + return execFileSync('bash', [UNINSTALL_SCRIPT, ...(options.args || [])], { + cwd: options.cwd, + env: { + ...process.env, + HOME: options.homeDir || process.env.HOME, + }, + encoding: 'utf8', + input: options.input || 'y\n', + stdio: ['pipe', 'pipe', 'pipe'], + timeout: 20000, + }); +} + +function readManifestLines(projectRoot) { + const manifestPath = path.join(projectRoot, '.trae', '.ecc-manifest'); + return fs.readFileSync(manifestPath, 'utf8') + .split(/\r?\n/) + .map((line) => line.trim()) + .filter(Boolean); +} + +function test(name, fn) { + try { + fn(); + console.log(` \u2713 ${name}`); + return true; + } catch (error) { + console.log(` \u2717 ${name}`); + console.log(` Error: ${error.message}`); + return false; + } +} + +function runTests() { + console.log('\n=== Testing Trae install/uninstall scripts ===\n'); + + let passed = 0; + let failed = 0; + + if (process.platform === 'win32') { + console.log(' - skipped on Windows; Trae shell scripts are Unix-only'); + console.log(`\nResults: Passed: ${passed}, Failed: ${failed}`); + process.exit(0); + } + + if (test('does not claim ownership of preexisting target files', () => { + const homeDir = createTempDir('trae-home-'); + const projectRoot = createTempDir('trae-project-'); + + try { + const preexistingCommandPath = path.join(projectRoot, '.trae', 'commands', 'e2e.md'); + fs.mkdirSync(path.dirname(preexistingCommandPath), { recursive: true }); + fs.writeFileSync(preexistingCommandPath, 'user owned command\n'); + + runInstall({ cwd: projectRoot, homeDir }); + + const manifestLines = readManifestLines(projectRoot); + assert.ok(!manifestLines.includes('commands/e2e.md'), 'Preexisting file should not be recorded in manifest'); + + runUninstall({ cwd: projectRoot, homeDir }); + + assert.strictEqual(fs.readFileSync(preexistingCommandPath, 'utf8'), 'user owned command\n'); + } finally { + cleanup(homeDir); + cleanup(projectRoot); + } + })) passed++; else failed++; + + if (test('records nested skill files and the full rules tree in the manifest', () => { + const homeDir = createTempDir('trae-home-'); + const projectRoot = createTempDir('trae-project-'); + + try { + runInstall({ cwd: projectRoot, homeDir }); + + const manifestLines = readManifestLines(projectRoot); + assert.ok(manifestLines.includes('skills/skill-comply/pyproject.toml')); + assert.ok(manifestLines.includes('rules/common/code-review.md')); + assert.ok(manifestLines.includes('rules/python/coding-style.md')); + assert.ok(manifestLines.includes('rules/zh/README.md')); + + assert.ok(fs.existsSync(path.join(projectRoot, '.trae', 'skills', 'skill-comply', 'pyproject.toml'))); + assert.ok(fs.existsSync(path.join(projectRoot, '.trae', 'rules', 'python', 'coding-style.md'))); + assert.ok(fs.existsSync(path.join(projectRoot, '.trae', 'rules', 'zh', 'README.md'))); + } finally { + cleanup(homeDir); + cleanup(projectRoot); + } + })) passed++; else failed++; + + if (test('reinstall preserves managed manifest coverage without duplicate entries', () => { + const homeDir = createTempDir('trae-home-'); + const projectRoot = createTempDir('trae-project-'); + + try { + runInstall({ cwd: projectRoot, homeDir }); + + const managedCommandPath = path.join(projectRoot, '.trae', 'commands', 'e2e.md'); + fs.rmSync(managedCommandPath); + + runInstall({ cwd: projectRoot, homeDir }); + + const manifestLines = readManifestLines(projectRoot); + const entryCount = manifestLines.filter((line) => line === 'commands/e2e.md').length; + + assert.strictEqual(entryCount, 1, 'Managed file should appear once in manifest after reinstall'); + assert.ok(fs.existsSync(managedCommandPath), 'Managed file should be recreated on reinstall'); + } finally { + cleanup(homeDir); + cleanup(projectRoot); + } + })) passed++; else failed++; + + if (test('uninstall rejects manifest entries that escape the Trae root via symlink traversal', () => { + const homeDir = createTempDir('trae-home-'); + const projectRoot = createTempDir('trae-project-'); + const externalRoot = createTempDir('trae-outside-'); + + try { + const traeRoot = path.join(projectRoot, '.trae'); + fs.mkdirSync(traeRoot, { recursive: true }); + + const outsideSecretPath = path.join(externalRoot, 'secret.txt'); + fs.writeFileSync(outsideSecretPath, 'do not remove\n'); + fs.symlinkSync(externalRoot, path.join(traeRoot, 'escape-link')); + fs.writeFileSync(path.join(traeRoot, '.ecc-manifest'), 'escape-link/secret.txt\n.ecc-manifest\n'); + + const stdout = runUninstall({ cwd: projectRoot, homeDir }); + + assert.ok(stdout.includes('Skipped: escape-link/secret.txt (invalid manifest entry)')); + assert.strictEqual(fs.readFileSync(outsideSecretPath, 'utf8'), 'do not remove\n'); + } finally { + cleanup(homeDir); + cleanup(projectRoot); + cleanup(externalRoot); + } + })) passed++; else failed++; + + console.log(`\nResults: Passed: ${passed}, Failed: ${failed}`); + process.exit(failed > 0 ? 1 : 0); +} + +runTests();