mirror of
https://github.com/Studio-Saelix/sencho.git
synced 2026-08-17 05:58:37 +00:00
feat(files): move files and folders across directories in the stack explorer (#1373)
* feat(files): move files and folders across directories in the stack explorer Add a cross-directory move to the stack file explorer. Files and folders can be relocated either through a "Move to..." context-menu item that opens a folder-picker dialog, or by dragging an entry onto a folder node (or onto the root area to move it to the stack root). The backend reuses the existing rename endpoint: renameStackPath now resolves both ends through the leaf helper, so a symlink moves as the link entry rather than its target, and it guards against moving a directory into its own subtree. A cross-filesystem rename surfaces as a clean 409 instead of a 500. Protected root files (compose / docker-compose / .env) stay put. Moving the open file, or a folder containing it, deselects the viewer; a move that would discard unsaved edits is blocked with a clear message. * fix(files): fold case in move guards and keep the move dialog open on failure Harden the cross-directory move against case-insensitive filesystems and fix a dialog dismissal edge: - Protected root files (compose / docker-compose / .env) were gated by an exact, lowercase name match. On a case-insensitive filesystem a request like COMPOSE.YAML resolves to the real compose.yaml and slipped past the gate, so a protected file could be moved out of the stack root via the API. The gate now folds case on case-insensitive platforms; Linux stays case-sensitive, where a differently-cased name is a distinct, unprotected file. - The directory-into-descendant guard compared resolved paths case-sensitively, so a source supplied with non-disk casing skipped the guard and fell through to an opaque OS error (500) instead of a clean 400. The comparison now folds case the same way. - The move dialog closed after awaiting the move regardless of outcome, so a blocked move (unsaved edits) or a failed move dismissed the picker as if it had succeeded. The shared handler now reports success and the dialog only closes on an actual move.
This commit is contained in:
Anso
@@ -416,6 +416,127 @@ describe('FileSystemService stack methods', () => {
|
||||
});
|
||||
});
|
||||
|
||||
// ── renameStackPath (rename + cross-directory move) ──────────────────────
|
||||
|
||||
describe('renameStackPath', () => {
|
||||
it('moves a file from the stack root into a subdirectory', async () => {
|
||||
await fs.writeFile(path.join(stackDir, 'app.conf'), 'data');
|
||||
await fs.mkdir(path.join(stackDir, 'configs'));
|
||||
|
||||
const service = FileSystemService.getInstance();
|
||||
await service.renameStackPath(STACK, 'app.conf', 'configs/app.conf');
|
||||
|
||||
await expect(fs.access(path.join(stackDir, 'app.conf'))).rejects.toMatchObject({ code: 'ENOENT' });
|
||||
expect(await fs.readFile(path.join(stackDir, 'configs', 'app.conf'), 'utf-8')).toBe('data');
|
||||
});
|
||||
|
||||
it('moves a file from a subdirectory back to the stack root', async () => {
|
||||
await fs.mkdir(path.join(stackDir, 'configs'));
|
||||
await fs.writeFile(path.join(stackDir, 'configs', 'app.conf'), 'data');
|
||||
|
||||
const service = FileSystemService.getInstance();
|
||||
await service.renameStackPath(STACK, 'configs/app.conf', 'app.conf');
|
||||
|
||||
await expect(fs.access(path.join(stackDir, 'configs', 'app.conf'))).rejects.toMatchObject({ code: 'ENOENT' });
|
||||
expect(await fs.readFile(path.join(stackDir, 'app.conf'), 'utf-8')).toBe('data');
|
||||
});
|
||||
|
||||
it('moves a directory into another directory', async () => {
|
||||
await fs.mkdir(path.join(stackDir, 'src'));
|
||||
await fs.writeFile(path.join(stackDir, 'src', 'child.txt'), 'inner');
|
||||
await fs.mkdir(path.join(stackDir, 'dest'));
|
||||
|
||||
const service = FileSystemService.getInstance();
|
||||
await service.renameStackPath(STACK, 'src', 'dest/src');
|
||||
|
||||
await expect(fs.access(path.join(stackDir, 'src'))).rejects.toMatchObject({ code: 'ENOENT' });
|
||||
expect(await fs.readFile(path.join(stackDir, 'dest', 'src', 'child.txt'), 'utf-8')).toBe('inner');
|
||||
});
|
||||
|
||||
it('renames a file in place (same directory)', async () => {
|
||||
await fs.writeFile(path.join(stackDir, 'old.txt'), 'x');
|
||||
|
||||
const service = FileSystemService.getInstance();
|
||||
await service.renameStackPath(STACK, 'old.txt', 'new.txt');
|
||||
|
||||
await expect(fs.access(path.join(stackDir, 'old.txt'))).rejects.toMatchObject({ code: 'ENOENT' });
|
||||
expect(await fs.readFile(path.join(stackDir, 'new.txt'), 'utf-8')).toBe('x');
|
||||
});
|
||||
|
||||
it('rejects moving a directory into its own descendant', async () => {
|
||||
await fs.mkdir(path.join(stackDir, 'parent', 'child'), { recursive: true });
|
||||
|
||||
const service = FileSystemService.getInstance();
|
||||
await expect(service.renameStackPath(STACK, 'parent', 'parent/child/parent')).rejects.toMatchObject({
|
||||
code: 'INVALID_PATH',
|
||||
});
|
||||
});
|
||||
|
||||
it('rejects overwriting an existing destination', async () => {
|
||||
await fs.writeFile(path.join(stackDir, 'source.txt'), 'a');
|
||||
await fs.mkdir(path.join(stackDir, 'sub'));
|
||||
await fs.writeFile(path.join(stackDir, 'sub', 'source.txt'), 'b');
|
||||
|
||||
const service = FileSystemService.getInstance();
|
||||
await expect(service.renameStackPath(STACK, 'source.txt', 'sub/source.txt')).rejects.toMatchObject({
|
||||
code: 'EEXIST',
|
||||
});
|
||||
});
|
||||
|
||||
it('rejects an in-place rename onto an existing sibling name', async () => {
|
||||
await fs.writeFile(path.join(stackDir, 'old.txt'), 'a');
|
||||
await fs.writeFile(path.join(stackDir, 'existing.txt'), 'b');
|
||||
|
||||
const service = FileSystemService.getInstance();
|
||||
await expect(service.renameStackPath(STACK, 'old.txt', 'existing.txt')).rejects.toMatchObject({
|
||||
code: 'EEXIST',
|
||||
});
|
||||
});
|
||||
|
||||
it('rejects moving a protected root file out of the stack root', async () => {
|
||||
await fs.writeFile(path.join(stackDir, 'compose.yaml'), 'services: {}\n');
|
||||
await fs.mkdir(path.join(stackDir, 'sub'));
|
||||
|
||||
const service = FileSystemService.getInstance();
|
||||
await expect(service.renameStackPath(STACK, 'compose.yaml', 'sub/compose.yaml')).rejects.toMatchObject({
|
||||
code: 'PROTECTED_FILE',
|
||||
});
|
||||
});
|
||||
|
||||
it('rejects a destination that becomes a protected root name', async () => {
|
||||
await fs.mkdir(path.join(stackDir, 'sub'));
|
||||
await fs.writeFile(path.join(stackDir, 'sub', 'compose.yaml'), 'services: {}\n');
|
||||
|
||||
const service = FileSystemService.getInstance();
|
||||
await expect(service.renameStackPath(STACK, 'sub/compose.yaml', 'compose.yaml')).rejects.toMatchObject({
|
||||
code: 'PROTECTED_FILE',
|
||||
});
|
||||
});
|
||||
|
||||
// On a case-insensitive filesystem a differently-cased request resolves to the
|
||||
// real protected file, so the gate must fold case. These only reproduce the
|
||||
// bypass on Windows/macOS; on Linux the cased name is a distinct, unprotected
|
||||
// file and the scenario does not arise.
|
||||
it.skipIf(!isWindows)('rejects moving a protected root file referenced by a different case', async () => {
|
||||
await fs.writeFile(path.join(stackDir, 'compose.yaml'), 'services: {}\n');
|
||||
await fs.mkdir(path.join(stackDir, 'sub'));
|
||||
|
||||
const service = FileSystemService.getInstance();
|
||||
await expect(service.renameStackPath(STACK, 'COMPOSE.YAML', 'sub/COMPOSE.YAML')).rejects.toMatchObject({
|
||||
code: 'PROTECTED_FILE',
|
||||
});
|
||||
});
|
||||
|
||||
it.skipIf(!isWindows)('rejects moving a directory into its own descendant when the source case differs', async () => {
|
||||
await fs.mkdir(path.join(stackDir, 'parent', 'child'), { recursive: true });
|
||||
|
||||
const service = FileSystemService.getInstance();
|
||||
await expect(service.renameStackPath(STACK, 'Parent', 'parent/child/x')).rejects.toMatchObject({
|
||||
code: 'INVALID_PATH',
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
// ── path traversal ──────────────────────────────────────────────────────
|
||||
|
||||
describe('path traversal protection', () => {
|
||||
@@ -553,5 +674,52 @@ describe('FileSystemService stack methods', () => {
|
||||
const kept = await fs.readFile(path.join(targetDir, 'kept.txt'), 'utf-8');
|
||||
expect(kept).toBe('preserve');
|
||||
});
|
||||
|
||||
it('move on a symlink relocates the link entry and leaves an internal target intact', async () => {
|
||||
const targetPath = path.join(stackDir, 'target.txt');
|
||||
const linkPath = path.join(stackDir, 'link.txt');
|
||||
await fs.writeFile(targetPath, 'payload');
|
||||
await fs.symlink(targetPath, linkPath);
|
||||
await fs.mkdir(path.join(stackDir, 'sub'));
|
||||
|
||||
const service = FileSystemService.getInstance();
|
||||
await service.renameStackPath(STACK, 'link.txt', 'sub/link.txt');
|
||||
|
||||
// The link entry moved; the old location is gone and the target is untouched.
|
||||
await expect(fs.lstat(linkPath)).rejects.toMatchObject({ code: 'ENOENT' });
|
||||
const movedLink = await fs.lstat(path.join(stackDir, 'sub', 'link.txt'));
|
||||
expect(movedLink.isSymbolicLink()).toBe(true);
|
||||
expect(await fs.readFile(targetPath, 'utf-8')).toBe('payload');
|
||||
// Following the moved link still resolves to the original target content.
|
||||
expect(await fs.readFile(path.join(stackDir, 'sub', 'link.txt'), 'utf-8')).toBe('payload');
|
||||
});
|
||||
|
||||
it('rejects a move whose destination is occupied by a dangling symlink', async () => {
|
||||
await fs.writeFile(path.join(stackDir, 'real.txt'), 'payload');
|
||||
await fs.mkdir(path.join(stackDir, 'sub'));
|
||||
// A symlink to a now-removed target: lstat sees the link, so the slot is occupied.
|
||||
await fs.symlink(path.join(stackDir, 'sub', 'gone-target'), path.join(stackDir, 'sub', 'real.txt'));
|
||||
|
||||
const service = FileSystemService.getInstance();
|
||||
await expect(service.renameStackPath(STACK, 'real.txt', 'sub/real.txt')).rejects.toMatchObject({
|
||||
code: 'EEXIST',
|
||||
});
|
||||
});
|
||||
|
||||
it('move on a symlink whose target is outside the stack relocates only the link, not the target', async () => {
|
||||
const externalFile = path.join(tmpBase, 'outside.txt');
|
||||
await fs.writeFile(externalFile, 'external');
|
||||
const linkPath = path.join(stackDir, 'escape-link');
|
||||
await fs.symlink(externalFile, linkPath);
|
||||
await fs.mkdir(path.join(stackDir, 'sub'));
|
||||
|
||||
const service = FileSystemService.getInstance();
|
||||
await service.renameStackPath(STACK, 'escape-link', 'sub/escape-link');
|
||||
|
||||
await expect(fs.lstat(linkPath)).rejects.toMatchObject({ code: 'ENOENT' });
|
||||
const moved = await fs.lstat(path.join(stackDir, 'sub', 'escape-link'));
|
||||
expect(moved.isSymbolicLink()).toBe(true);
|
||||
expect(await fs.readFile(externalFile, 'utf-8')).toBe('external');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1129,6 +1129,59 @@ describe('PATCH /api/stacks/:stackName/files/rename', () => {
|
||||
const moved = await fs.readFile(path.join(stacksDir, STACK, 'community-rename-to.txt'), 'utf-8');
|
||||
expect(moved).toBe('src');
|
||||
});
|
||||
|
||||
it('moves a file into a subdirectory (cross-directory move) with 204', async () => {
|
||||
await fs.writeFile(path.join(stacksDir, STACK, 'move-me.txt'), 'payload');
|
||||
await fs.mkdir(path.join(stacksDir, STACK, 'movedest'), { recursive: true });
|
||||
|
||||
const res = await request(app)
|
||||
.patch(`/api/stacks/${STACK}/files/rename`)
|
||||
.set('Cookie', adminCookie)
|
||||
.send({ from: 'move-me.txt', to: 'movedest/move-me.txt' });
|
||||
expect(res.status).toBe(204);
|
||||
|
||||
await expect(fs.access(path.join(stacksDir, STACK, 'move-me.txt'))).rejects.toMatchObject({ code: 'ENOENT' });
|
||||
expect(await fs.readFile(path.join(stacksDir, STACK, 'movedest', 'move-me.txt'), 'utf-8')).toBe('payload');
|
||||
});
|
||||
|
||||
it('returns 400 INVALID_PATH when moving a directory into its own descendant', async () => {
|
||||
await fs.mkdir(path.join(stacksDir, STACK, 'movparent', 'movchild'), { recursive: true });
|
||||
|
||||
const res = await request(app)
|
||||
.patch(`/api/stacks/${STACK}/files/rename`)
|
||||
.set('Cookie', adminCookie)
|
||||
.send({ from: 'movparent', to: 'movparent/movchild/movparent' });
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body.code).toBe('INVALID_PATH');
|
||||
});
|
||||
|
||||
it('returns 409 PROTECTED_FILE when moving a nested compose file to the stack root', async () => {
|
||||
await fs.mkdir(path.join(stacksDir, STACK, 'nested'), { recursive: true });
|
||||
await fs.writeFile(path.join(stacksDir, STACK, 'nested', 'compose.yaml'), 'services: {}\n');
|
||||
|
||||
const res = await request(app)
|
||||
.patch(`/api/stacks/${STACK}/files/rename`)
|
||||
.set('Cookie', adminCookie)
|
||||
.send({ from: 'nested/compose.yaml', to: 'compose.yaml' });
|
||||
expect(res.status).toBe(409);
|
||||
expect(res.body.code).toBe('PROTECTED_FILE');
|
||||
});
|
||||
|
||||
it('maps a cross-device EXDEV rename failure to 409 EXDEV', async () => {
|
||||
await fs.writeFile(path.join(stacksDir, STACK, 'exdev-src.txt'), 'data');
|
||||
await fs.mkdir(path.join(stacksDir, STACK, 'exdev-dest'), { recursive: true });
|
||||
const renameSpy = vi.spyOn(fs, 'rename').mockRejectedValueOnce(
|
||||
Object.assign(new Error('cross-device link not permitted'), { code: 'EXDEV' }),
|
||||
);
|
||||
|
||||
const res = await request(app)
|
||||
.patch(`/api/stacks/${STACK}/files/rename`)
|
||||
.set('Cookie', adminCookie)
|
||||
.send({ from: 'exdev-src.txt', to: 'exdev-dest/exdev-src.txt' });
|
||||
expect(res.status).toBe(409);
|
||||
expect(res.body.code).toBe('EXDEV');
|
||||
renameSpy.mockRestore();
|
||||
});
|
||||
});
|
||||
|
||||
// ── PUT /:stackName/files/permissions ────────────────────────────────────────
|
||||
|
||||
@@ -1703,7 +1703,8 @@ type FsErrorCode =
|
||||
| 'FILE_EXISTS'
|
||||
| 'DIR_EXISTS'
|
||||
| 'PROTECTED_FILE'
|
||||
| 'LINK_CHMOD_UNSUPPORTED';
|
||||
| 'LINK_CHMOD_UNSUPPORTED'
|
||||
| 'EXDEV';
|
||||
|
||||
function sendFsError(
|
||||
res: Response,
|
||||
@@ -1730,6 +1731,9 @@ function sendFsError(
|
||||
if (e.code === 'EEXIST') {
|
||||
return res.status(409).json({ error: e.message, code: 'ALREADY_EXISTS' satisfies FsErrorCode });
|
||||
}
|
||||
if (e.code === 'EXDEV') {
|
||||
return res.status(409).json({ error: 'Cannot move across a storage boundary', code: 'EXDEV' satisfies FsErrorCode });
|
||||
}
|
||||
if (e.code === 'ENOTDIR') {
|
||||
return res.status(400).json({ error: 'Target path is not a directory', code: 'INVALID_PATH' satisfies FsErrorCode });
|
||||
}
|
||||
|
||||
@@ -77,6 +77,14 @@ function stripTrailingSlash(s: string): string {
|
||||
return s.endsWith('/') ? s.slice(0, -1) : s;
|
||||
}
|
||||
|
||||
// On a case-insensitive filesystem (Windows, default macOS) two paths that differ
|
||||
// only in case point at the same entry, so comparisons that gate filesystem
|
||||
// mutations must fold case to stay authoritative. On Linux (where Sencho runs in
|
||||
// production) paths are case-sensitive and this returns the input unchanged.
|
||||
function fsCaseKey(s: string): string {
|
||||
return process.platform === 'win32' || process.platform === 'darwin' ? s.toLowerCase() : s;
|
||||
}
|
||||
|
||||
function isProtectedRelPath(relPath: string): boolean {
|
||||
if (!relPath) return false;
|
||||
const normalized = stripTrailingSlash(relPath);
|
||||
@@ -84,7 +92,9 @@ function isProtectedRelPath(relPath: string): boolean {
|
||||
// the stack directory itself, so a subdirectory entry named compose.yaml is just
|
||||
// an arbitrary file and the user may want to delete it.
|
||||
if (normalized.includes('/')) return false;
|
||||
return PROTECTED_STACK_FILES.has(normalized);
|
||||
// Fold case so e.g. a request for COMPOSE.YAML cannot dodge the gate on a
|
||||
// case-insensitive filesystem where it resolves to the real compose.yaml.
|
||||
return PROTECTED_STACK_FILES.has(fsCaseKey(normalized));
|
||||
}
|
||||
|
||||
function protectedFileError(relPath: string): Error & { code: string } {
|
||||
@@ -1385,22 +1395,40 @@ export class FileSystemService {
|
||||
await fsPromises.mkdir(safePath, { recursive: true });
|
||||
}
|
||||
|
||||
/**
|
||||
* Renames or moves an entry within a stack. The source and destination may sit
|
||||
* in different directories (a cross-directory move), since fs.rename relocates
|
||||
* natively. Both paths resolve through the leaf helper so a symlink source is
|
||||
* moved as the link entry itself rather than followed to its target, matching
|
||||
* the delete/chmod policy. fs.rename fails with EXDEV across a filesystem
|
||||
* boundary (e.g. a bind-mounted subdirectory); the route surfaces that as a 409.
|
||||
*/
|
||||
async renameStackPath(stackName: string, fromRel: string, toRel: string): Promise<void> {
|
||||
if (isProtectedRelPath(fromRel)) throw protectedFileError(fromRel);
|
||||
if (isProtectedRelPath(toRel)) throw protectedFileError(toRel);
|
||||
const fromPath = await this.resolveSafeStackPath(stackName, fromRel);
|
||||
// toRel must resolve to the same parent directory (rename only, no cross-dir move).
|
||||
const toPath = await this.resolveSafeStackPath(stackName, toRel);
|
||||
if (path.dirname(fromPath) !== path.dirname(toPath)) {
|
||||
throw Object.assign(new Error('Cross-directory rename is not supported'), { code: 'INVALID_PATH' });
|
||||
}
|
||||
const fromPath = await this.resolveSafeStackLeafPath(stackName, fromRel);
|
||||
const toPath = await this.resolveSafeStackLeafPath(stackName, toRel);
|
||||
const toName = path.basename(toPath);
|
||||
if (!toName || toName === '.' || toName === '..') {
|
||||
throw Object.assign(new Error('Invalid destination name'), { code: 'INVALID_PATH' });
|
||||
}
|
||||
// Prevent overwriting an existing path.
|
||||
// Block moving a directory into itself or one of its own descendants; fs.rename
|
||||
// would otherwise fail with an opaque EINVAL/EPERM. Compare case-folded so the
|
||||
// guard stays authoritative when the source is supplied with non-disk casing on
|
||||
// a case-insensitive filesystem.
|
||||
const fromStat = await fsPromises.lstat(fromPath);
|
||||
if (fromStat.isDirectory()) {
|
||||
const fromKey = fsCaseKey(fromPath);
|
||||
const fromKeyWithSep = fromKey.endsWith(path.sep) ? fromKey : fromKey + path.sep;
|
||||
const toKey = fsCaseKey(toPath);
|
||||
if (toKey === fromKey || toKey.startsWith(fromKeyWithSep)) {
|
||||
throw Object.assign(new Error('Cannot move a folder into itself'), { code: 'INVALID_PATH' });
|
||||
}
|
||||
}
|
||||
// Prevent overwriting an existing path. lstat (not access) so a dangling
|
||||
// symlink already at the destination still counts as occupied.
|
||||
try {
|
||||
await fsPromises.access(toPath);
|
||||
await fsPromises.lstat(toPath);
|
||||
throw Object.assign(new Error('A file or folder with that name already exists'), { code: 'EEXIST' });
|
||||
} catch (e: unknown) {
|
||||
const fe = e as NodeJS.ErrnoException;
|
||||
|
||||
Reference in New Issue
Block a user