fix(stack-files): symlink-aware delete and chmod (#1214)

deleteStackPath now lstats the leaf and unlinks the link entry itself
when it is a symbolic link, so the file the user clicked on in the tree
is what gets removed (the linked target stays intact). chmodStackPath
rejects with LINK_CHMOD_UNSUPPORTED on a symlink rather than silently
mutating the target's permissions; Node's lchmod is macOS-only and
following the link is the bug being fixed here.

Path-component symlinks are still resolved via the existing
resolveSafeStackPath, so a symlinked parent that escapes the stack dir
still surfaces SYMLINK_ESCAPE before the leaf is inspected.

Service-level tests cover delete on internal-target / external-target /
broken / dir-target symlinks, chmod rejection on symlinks (including
the broken case), and non-symlink regression checks. Route-level tests
pin the 409 LINK_CHMOD_UNSUPPORTED mapping and the link-only-delete
behaviour. The describe blocks are platform-gated; Windows symlink
creation needs admin/developer-mode and is skipped along with the
existing SYMLINK_ESCAPE test.

Docs updated to describe both behaviours in plain product terms.
This commit is contained in:
Anso
2026-05-25 01:30:00 -04:00
committed by GitHub
parent ba4de2e004
commit c2357ec534
5 changed files with 222 additions and 9 deletions
+51 -6
View File
@@ -877,21 +877,53 @@ export class FileSystemService {
return { ok: true, mtimeMs: newStat.mtimeMs };
}
/**
* Like resolveSafeStackPath but does NOT follow a symlink at the leaf.
* Path-component symlinks are still resolved and validated (so a symlinked
* parent that escapes the stack dir still throws SYMLINK_ESCAPE), but the
* final entry stays as the link path the user sees in the tree. Callers
* use this to act on the link entry itself (unlink the link, not the
* target) for operations where following would mutate a file other than
* the one the user clicked on.
*/
private async resolveSafeStackLeafPath(stackName: string, relPath: string): Promise<string> {
if (relPath === '' || relPath === '.') {
return this.resolveSafeStackPath(stackName, '');
}
const parentRel = path.dirname(relPath);
const baseName = path.basename(relPath);
if (!baseName || baseName === '.' || baseName === '..') {
throw Object.assign(new Error('Invalid path'), { code: 'INVALID_PATH' });
}
const safeParent = await this.resolveSafeStackPath(stackName, parentRel === '.' ? '' : parentRel);
return path.join(safeParent, baseName);
}
async deleteStackPath(stackName: string, relPath: string, recursive: boolean = false): Promise<void> {
if (isProtectedRelPath(relPath)) throw protectedFileError(relPath);
const safePath = await this.resolveSafeStackPath(stackName, relPath);
const leafPath = await this.resolveSafeStackLeafPath(stackName, relPath);
// Branch on whether the leaf is a symlink BEFORE following it. Deleting
// a symlink should remove the link entry the user clicked on; following
// through to the target would silently delete a file with a different
// name and leave the link entry dangling.
const leafStat = await fsPromises.lstat(leafPath);
if (leafStat.isSymbolicLink()) {
await fsPromises.unlink(leafPath);
return;
}
if (recursive) {
await fsPromises.rm(safePath, { recursive: true, force: true });
await fsPromises.rm(leafPath, { recursive: true, force: true });
return;
}
try {
await fsPromises.unlink(safePath);
await fsPromises.unlink(leafPath);
} catch (err: unknown) {
const e = err as NodeJS.ErrnoException;
if (e.code === 'EISDIR') {
try {
await fsPromises.rmdir(safePath);
await fsPromises.rmdir(leafPath);
} catch (inner: unknown) {
const ie = inner as NodeJS.ErrnoException;
if (ie.code === 'ENOTEMPTY' || ie.code === 'EEXIST') {
@@ -946,8 +978,21 @@ export class FileSystemService {
throw Object.assign(new Error('Invalid permission bits'), { code: 'INVALID_PATH' });
}
if (isProtectedRelPath(relPath)) throw protectedFileError(relPath);
const safePath = await this.resolveSafeStackPath(stackName, relPath);
await fsPromises.chmod(safePath, mode);
const leafPath = await this.resolveSafeStackLeafPath(stackName, relPath);
// chmod on a symlink is rejected. Following the link would silently
// mutate permissions on a file with a different name than the entry the
// user clicked on. Node's fsPromises.lchmod is macOS-only, so for the
// common Linux/Windows case there is no safe in-place alternative; we
// surface a clear error so the user edits the target file directly.
const leafStat = await fsPromises.lstat(leafPath);
if (leafStat.isSymbolicLink()) {
throw Object.assign(
new Error('Cannot change permissions of a symlink. Edit the target file directly.'),
{ code: 'LINK_CHMOD_UNSUPPORTED' as const },
);
}
await fsPromises.chmod(leafPath, mode);
}
async statStackEntry(stackName: string, relPath: string): Promise<FileEntry> {