diff --git a/src/backend/controllers/fs/FSController.ts b/src/backend/controllers/fs/FSController.ts index 3db7e81bb..0a675329e 100644 --- a/src/backend/controllers/fs/FSController.ts +++ b/src/backend/controllers/fs/FSController.ts @@ -1466,7 +1466,8 @@ export class FSController extends PuterController { const entry = await this.#resolveEntryForRequest(body); await this.#assertAccess(actor, entry.path, 'write'); - const renamed = await this.services.fs.rename(entry, newName); + const userId = this.#getActorUserId(req); + const renamed = await this.services.fs.rename(userId, entry, newName); this.#emitGuiItemUpdated(renamed); res.json(this.#toClientEntry(renamed)); } diff --git a/src/backend/controllers/fs/LegacyFSController.ts b/src/backend/controllers/fs/LegacyFSController.ts index d4e1ac734..c89d74229 100644 --- a/src/backend/controllers/fs/LegacyFSController.ts +++ b/src/backend/controllers/fs/LegacyFSController.ts @@ -946,7 +946,8 @@ export class LegacyFSController extends PuterController { 'write', ); - const renamed = await this.services.fs.rename(entry, newName); + const userId = this.#getActorUserId(req); + const renamed = await this.services.fs.rename(userId, entry, newName); await this.#emitGuiEvent('outer.gui.item.updated', renamed); res.json(await toLegacyEntry(this.clients.event, renamed)); }; @@ -1522,7 +1523,11 @@ export class LegacyFSController extends PuterController { targetEntry.path, 'write', ); - const renamed = await this.services.fs.rename(targetEntry, newName); + const renamed = await this.services.fs.rename( + userId, + targetEntry, + newName, + ); await this.#emitGuiEvent('outer.gui.item.updated', renamed); res.json({ ...signEntry(renamed, signingCfg, { diff --git a/src/backend/services/fs/FSService.test.ts b/src/backend/services/fs/FSService.test.ts index eef0a94f1..61889d65a 100644 --- a/src/backend/services/fs/FSService.test.ts +++ b/src/backend/services/fs/FSService.test.ts @@ -2111,7 +2111,7 @@ describe('FSService mkdir, touch, rename and shortcuts', () => { `${user.home}/Documents/before.txt`, 'x', ); - const renamed = await fs.rename(entry, 'after.txt'); + const renamed = await fs.rename(user.userId, entry, 'after.txt'); expect(renamed.name).toBe('after.txt'); expect(renamed.path).toBe(`${user.home}/Documents/after.txt`); @@ -2124,7 +2124,7 @@ describe('FSService mkdir, touch, rename and shortcuts', () => { }); await writeFile(user, `${user.home}/Documents/olddir/inner.txt`, 'x'); - await fs.rename(dir, 'newdir'); + await fs.rename(user.userId, dir, 'newdir'); expect( await entryAt(user, '/Documents/newdir/inner.txt'), @@ -2140,18 +2140,34 @@ describe('FSService mkdir, touch, rename and shortcuts', () => { ); await writeFile(user, `${user.home}/Documents/taken.txt`, 'x'); - expect((await caught(() => fs.rename(entry, 'a/b'))).message).toBe( + expect((await caught(() => fs.rename(user.userId, entry, 'a/b'))).message).toBe( 'Name cannot contain a slash', ); - expect((await caught(() => fs.rename(entry, ' '))).message).toBe( + expect((await caught(() => fs.rename(user.userId, entry, ' '))).message).toBe( 'Name cannot be empty', ); expect( - (await caught(() => fs.rename(entry, 'taken.txt'))).statusCode, + (await caught(() => fs.rename(user.userId, entry, 'taken.txt'))).statusCode, ).toBe(409); // Renaming to the current name is a no-op that returns the same row. - await expect(fs.rename(entry, 'ren.txt')).resolves.toBe(entry); + await expect(fs.rename(user.userId, entry, 'ren.txt')).resolves.toBe(entry); + }); + + it("refuses to rename another user's entry, matching remove and move", async () => { + const other = await makeUser(); + const entry = await writeFile( + user, + `${user.home}/Documents/theirs.txt`, + 'x', + ); + + // A write-mode share recipient passes the ACL but must not be able to + // restructure the owner's tree — the same policy remove/move enforce. + await expect( + fs.rename(other.userId, entry, 'renamed.txt'), + ).rejects.toMatchObject({ statusCode: 403 }); + expect(await entryAt(user, '/Documents/theirs.txt')).not.toBeNull(); }); it('creates a shortcut, conflicts on a taken name and dedupes on request', async () => { @@ -2707,7 +2723,7 @@ describe('FSService copy', () => { expect(first.path).toBe(`${user.home}/Desktop/phantom.txt`); // Renaming the occupant frees the path... - await fs.rename(first, 'phantom-renamed.txt'); + await fs.rename(user.userId, first, 'phantom-renamed.txt'); // ...so an immediate re-copy must succeed. A stale path-cache entry // for the old name used to surface a phantom conflict here — and a @@ -3081,7 +3097,7 @@ describe('FSService — cross-app AppData access', () => { asCalendar(() => fs.remove(owner.userId, { entry: contactsFile })), ).rejects.toMatchObject({ statusCode: 403 }); await expect( - asCalendar(() => fs.rename(contactsFile, 'renamed.json')), + asCalendar(() => fs.rename(owner.userId, contactsFile, 'renamed.json')), ).rejects.toMatchObject({ statusCode: 403 }); const desktop = (await server.stores.fsEntry.getEntryByPath( @@ -3110,7 +3126,7 @@ describe('FSService — cross-app AppData access', () => { it('allows rename once the delete class is granted', async () => { await grant(appDataPermission(contacts.uid, 'fs', 'delete')); const renamed = await asCalendar(() => - fs.rename(contactsFile, 'renamed.json'), + fs.rename(owner.userId, contactsFile, 'renamed.json'), ); expect(renamed.name).toBe('renamed.json'); }); diff --git a/src/backend/services/fs/FSService.ts b/src/backend/services/fs/FSService.ts index de6f5332f..b23ba30a5 100644 --- a/src/backend/services/fs/FSService.ts +++ b/src/backend/services/fs/FSService.ts @@ -3246,7 +3246,20 @@ export class FSService extends PuterService { * Rename an entry in place. The name changes and path rewrites; if the * entry is a directory, descendant paths are rewritten too. */ - async rename(entry: FSEntry, newName: string): Promise { + async rename( + userId: number, + entry: FSEntry, + newName: string, + ): Promise { + if (entry.userId !== userId) { + // Same policy as remove/move: ACL write on a shared entry does not + // extend to restructuring the owner's tree. + throw new HttpError( + 403, + 'Cannot rename an entry owned by another user', + { legacyCode: 'forbidden' }, + ); + } if (newName.includes('/')) throw new HttpError(400, 'Name cannot contain a slash', { legacyCode: 'bad_request',