diff --git a/src/management/shared-manager.ts b/src/management/shared-manager.ts index bb8cc7ac..e468c2d6 100644 --- a/src/management/shared-manager.ts +++ b/src/management/shared-manager.ts @@ -150,11 +150,6 @@ class SharedManager { * Detect circular symlink before creation */ private detectCircularSymlink(target: string): boolean { - // Check if target exists and is symlink - if (!fs.existsSync(target)) { - return false; - } - try { const stats = fs.lstatSync(target); if (!stats.isSymbolicLink()) { @@ -164,20 +159,31 @@ class SharedManager { // Resolve target's link const targetLink = fs.readlinkSync(target); const resolvedTarget = path.resolve(path.dirname(target), targetLink); + const sharedDirPath = path.resolve(this.sharedDir); + + // A raw target path pointing back into ~/.ccs/shared is already unsafe. + // Re-pointing ~/.ccs/shared/* to ~/.claude/* would turn it into a real loop, + // even if the current ~/.ccs/shared entry ultimately resolves to an external path. + if (this.isPathWithinDirectory(resolvedTarget, sharedDirPath)) { + console.log(warn(`Circular symlink detected: ${target} → ${resolvedTarget}`)); + return true; + } // Only treat targets inside the managed shared root as circular. // Existing shared symlinks may already resolve through ~/.claude/ to an // external repo, which is a supported upgrade path rather than a loop. - const sharedDir = this.resolveCanonicalPath(path.join(getCcsDir(), 'shared')); + const sharedDir = this.resolveCanonicalPath(sharedDirPath); const canonicalResolvedTarget = this.resolveCanonicalPath(resolvedTarget); if (this.isPathWithinDirectory(canonicalResolvedTarget, sharedDir)) { console.log(warn(`Circular symlink detected: ${target} → ${resolvedTarget}`)); return true; } - } catch (_err) { - // If can't read, assume not circular - return false; + } catch (err) { + if ((err as NodeJS.ErrnoException).code === 'ENOENT') { + return false; + } + throw err; } return false; @@ -189,13 +195,13 @@ class SharedManager { */ ensureSharedDirectories(): void { // Create ~/.claude/ if missing - if (!fs.existsSync(this.claudeDir)) { + if (!this.getLstatSync(this.claudeDir)) { console.log(info('Creating ~/.claude/ directory structure')); fs.mkdirSync(this.claudeDir, { recursive: true, mode: 0o700 }); } // Create shared directory - if (!fs.existsSync(this.sharedDir)) { + if (!this.getLstatSync(this.sharedDir)) { fs.mkdirSync(this.sharedDir, { recursive: true, mode: 0o700 }); } @@ -207,7 +213,7 @@ class SharedManager { const sharedPath = path.join(this.sharedDir, item.name); // Create in ~/.claude/ if missing - if (!fs.existsSync(claudePath)) { + if (!this.getLstatSync(claudePath)) { if (item.type === 'directory') { fs.mkdirSync(claudePath, { recursive: true, mode: 0o700 }); } else if (item.type === 'file') { @@ -223,7 +229,7 @@ class SharedManager { } // If already a symlink pointing to correct target, skip - if (fs.existsSync(sharedPath)) { + if (this.getLstatSync(sharedPath)) { try { const stats = fs.lstatSync(sharedPath); if (stats.isSymbolicLink()) { @@ -1639,6 +1645,17 @@ class SharedManager { } } + private getLstatSync(targetPath: string): fs.Stats | null { + try { + return fs.lstatSync(targetPath); + } catch (err) { + if ((err as NodeJS.ErrnoException).code === 'ENOENT') { + return null; + } + throw err; + } + } + /** * Copy directory as fallback (Windows without Developer Mode) */ diff --git a/tests/unit/shared-manager.test.ts b/tests/unit/shared-manager.test.ts index dee54d4f..240769c6 100644 --- a/tests/unit/shared-manager.test.ts +++ b/tests/unit/shared-manager.test.ts @@ -155,6 +155,36 @@ describe('SharedManager', () => { }); describe('shared symlink lifecycle', () => { + it('does not rewrite inverse shared symlink chains into a real loop', () => { + const manager = new SharedManager(); + const externalCommandsDir = path.join(tempRoot, 'Documents', 'claude-config', 'commands'); + const claudeCommandsPath = path.join(claudeDir(), 'commands'); + const sharedCommandsPath = path.join(ccsDir(), 'shared', 'commands'); + const logSpy = spyOn(console, 'log').mockImplementation(() => {}); + + fs.mkdirSync(externalCommandsDir, { recursive: true }); + fs.mkdirSync(claudeDir(), { recursive: true }); + fs.mkdirSync(path.join(ccsDir(), 'shared'), { recursive: true }); + fs.symlinkSync(sharedCommandsPath, claudeCommandsPath, 'dir'); + fs.symlinkSync(externalCommandsDir, sharedCommandsPath, 'dir'); + + manager.ensureSharedDirectories(); + + expect( + logSpy.mock.calls.some(([message]) => + String(message).includes('Skipping commands: circular symlink detected') + ) + ).toBe(true); + expect(fs.lstatSync(claudeCommandsPath).isSymbolicLink()).toBe(true); + expect( + path.resolve(path.dirname(claudeCommandsPath), fs.readlinkSync(claudeCommandsPath)) + ).toBe(sharedCommandsPath); + expect(fs.lstatSync(sharedCommandsPath).isSymbolicLink()).toBe(true); + expect( + path.resolve(path.dirname(sharedCommandsPath), fs.readlinkSync(sharedCommandsPath)) + ).toBe(externalCommandsDir); + }); + it('preserves external ~/.claude symlinks during upgrade reconciliation', () => { const manager = new SharedManager(); const externalCommandsDir = path.join(tempRoot, 'Documents', 'claude-config', 'commands'); @@ -183,9 +213,10 @@ describe('SharedManager', () => { manager.ensureSharedDirectories(); expect( - logSpy.mock.calls.some(([message]) => - String(message).includes('Skipping commands: circular symlink detected') || - String(message).includes('Skipping settings.json: circular symlink detected') + logSpy.mock.calls.some( + ([message]) => + String(message).includes('Skipping commands: circular symlink detected') || + String(message).includes('Skipping settings.json: circular symlink detected') ) ).toBe(false); expect(fs.lstatSync(sharedCommandsPath).isSymbolicLink()).toBe(true); @@ -217,6 +248,31 @@ describe('SharedManager', () => { ).toBe(true); expect(fs.lstatSync(sharedCommandsPath).isDirectory()).toBe(true); }); + + it('does not materialize dangling external settings symlinks', () => { + const manager = new SharedManager(); + const externalSettingsPath = path.join( + tempRoot, + 'Documents', + 'claude-config', + 'settings.json' + ); + const claudeSettingsPath = path.join(claudeDir(), 'settings.json'); + const sharedSettingsPath = path.join(ccsDir(), 'shared', 'settings.json'); + + fs.mkdirSync(path.dirname(externalSettingsPath), { recursive: true }); + fs.mkdirSync(claudeDir(), { recursive: true }); + fs.symlinkSync(externalSettingsPath, claudeSettingsPath, 'file'); + + manager.ensureSharedDirectories(); + + expect(fs.lstatSync(claudeSettingsPath).isSymbolicLink()).toBe(true); + expect(fs.existsSync(externalSettingsPath)).toBe(false); + expect(fs.lstatSync(sharedSettingsPath).isSymbolicLink()).toBe(true); + expect( + path.resolve(path.dirname(sharedSettingsPath), fs.readlinkSync(sharedSettingsPath)) + ).toBe(claudeSettingsPath); + }); }); describe('marketplace registry ownership', () => {