fix(fs): authorize restructuring by write on the parent

src/backend/services/fs/FSService.test.ts

@@ -2762,6 +2762,120 @@ describe('FSService copy', () => {
     });
 });
 
+describe('FSService restructuring a shared tree', () => {
+    let owner: TestUser;
+    let holder: TestUser;
+    let shared: FSEntry;
+
+    const shareWrite = (entry: FSEntry) =>
+        server.services.acl.setUserUser(
+            owner.actor,
+            holder.actor,
+            {
+                path: entry.path,
+                resolveAncestors: () => fs.getAncestorChain(entry.path),
+            },
+            'write',
+        );
+
+    const asHolder = <T>(run: () => Promise<T>): Promise<T> =>
+        runWithContext({ actor: holder.actor }, run);
+
+    const stillThere = async (path: string) =>
+        (await server.stores.fsEntry.getEntryByPath(path, {
+            skipCache: true,
+        })) !== null;
+
+    beforeAll(async () => {
+        owner = await makeUser();
+        holder = await makeUser();
+        shared = await fs.mkdir(owner.userId, {
+            path: `${owner.home}/Documents/Contents`,
+        });
+        await shareWrite(shared);
+    });
+
+    it('lets a recipient delete a file inside the shared folder', async () => {
+        const file = await writeFile(owner, `${shared.path}/gone.txt`, 'x');
+
+        await asHolder(() => fs.remove(holder.userId, { entry: file }));
+
+        expect(await stillThere(file.path)).toBe(false);
+    });
+
+    it('lets a recipient delete a subfolder of the shared folder', async () => {
+        const sub = await fs.mkdir(owner.userId, {
+            path: `${shared.path}/sub`,
+        });
+        await writeFile(owner, `${sub.path}/deep.txt`, 'x');
+
+        await asHolder(() =>
+            fs.remove(holder.userId, { entry: sub, recursive: true }),
+        );
+
+        expect(await stillThere(sub.path)).toBe(false);
+        expect(await stillThere(`${sub.path}/deep.txt`)).toBe(false);
+    });
+
+    it('lets a recipient rename a file inside the shared folder', async () => {
+        const file = await writeFile(owner, `${shared.path}/before.txt`, 'x');
+
+        const renamed = await asHolder(() =>
+            fs.rename(holder.userId, file, 'after.txt'),
+        );
+
+        expect(renamed.path).toBe(`${shared.path}/after.txt`);
+        expect(renamed.userId).toBe(owner.userId);
+    });
+
+    it('refuses to let a recipient delete the shared folder itself', async () => {
+        const error = await caught(() =>
+            asHolder(() =>
+                fs.remove(holder.userId, { entry: shared, recursive: true }),
+            ),
+        );
+
+        expect(error.statusCode).toBe(403);
+        expect(error.legacyCode).toBe('forbidden');
+        expect(await stillThere(shared.path)).toBe(true);
+    });
+
+    it('refuses to let a recipient rename the shared folder itself', async () => {
+        const error = await caught(() =>
+            asHolder(() => fs.rename(holder.userId, shared, 'Renamed')),
+        );
+
+        expect(error.statusCode).toBe(403);
+        expect(await stillThere(shared.path)).toBe(true);
+    });
+
+    it('refuses to let a recipient rename a file shared directly with them', async () => {
+        const file = await writeFile(owner, `${owner.home}/direct.txt`, 'x');
+        await shareWrite(file);
+
+        const error = await caught(() =>
+            asHolder(() => fs.rename(holder.userId, file, 'mine.txt')),
+        );
+
+        expect(error.statusCode).toBe(403);
+        expect(await stillThere(file.path)).toBe(true);
+    });
+
+    it('refuses a stranger with no share at all', async () => {
+        const stranger = await makeUser();
+        const file = await writeFile(owner, `${shared.path}/private.txt`, 'x');
+
+        const error = await caught(() =>
+            runWithContext({ actor: stranger.actor }, () =>
+                fs.remove(stranger.userId, { entry: file }),
+            ),
+        );
+
+        expect(error.statusCode).toBe(403);
+        expect(await stillThere(file.path)).toBe(true);
+    });
+});
+
 describe('FSService access checks', () => {
     let owner: TestUser;
     let stranger: TestUser;

src/backend/services/fs/FSService.ts

@@ -3253,15 +3253,7 @@ export class FSService extends PuterService {
         entry: FSEntry,
         newName: string,
     ): Promise<FSEntry> {
-        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' },
-            );
-        }
+        await this.#assertCanRestructure(entry, userId);
         if (newName.includes('/'))
             throw new HttpError(400, 'Name cannot contain a slash', {
                 legacyCode: 'bad_request',
@@ -3389,6 +3381,35 @@ export class FSService extends PuterService {
         }
     }
 
+    /**
+     * Rename, move and delete are authorized by `write` on the parent, not on
+     * the entry — which is what lets a share recipient reorganize inside a
+     * shared folder without reaching the shared folder itself.
+     */
+    async #assertCanRestructure(entry: FSEntry, userId: number): Promise<void> {
+        if (entry.userId === userId) return;
+
+        const actor = Context.get('actor') as Actor | undefined;
+        const parentPath = pathPosix.dirname(entry.path);
+        if (actor && parentPath !== '/') {
+            const allowed = await this.services.acl.check(
+                actor,
+                {
+                    path: parentPath,
+                    resolveAncestors: () => this.getAncestorChain(parentPath),
+                },
+                'write',
+            );
+            if (allowed) return;
+        }
+
+        throw new HttpError(
+            403,
+            'Cannot restructure an entry owned by another user',
+            { legacyCode: 'forbidden' },
+        );
+    }
+
     async remove(
         userId: number,
         input: {
@@ -3407,20 +3428,11 @@ export class FSService extends PuterService {
         if (!input.systemInitiated) {
             await this.#assertCrossAppDeleteAllowed(entry.path);
         }
-        if (entry.userId !== userId) {
-            // Defensive — only the owner should be hitting this path; higher
-            // layers grant access via ACL, not raw ownership, but we still
-            // want to avoid a misrouted call taking out someone else's tree.
-            throw new HttpError(
-                403,
-                'Cannot remove an entry owned by another user',
-                { legacyCode: 'forbidden' },
-            );
-        }
+        await this.#assertCanRestructure(entry, userId);
 
         if (entry.isDir) {
             const descendants = await this.stores.fsEntry.listDescendantsByPath(
-                userId,
+                entry.userId,
                 entry.path,
             );
             if (descendants.length > 0 && !input.recursive) {
@@ -3642,13 +3654,7 @@ export class FSService extends PuterService {
         // The source only: moving *into* another app's AppData is a write, and
         // ACL plus the fs:write class already cover that.
         await this.#assertCrossAppDeleteAllowed(source.path);
-        if (source.userId !== userId) {
-            throw new HttpError(
-                403,
-                'Cannot move an entry owned by another user',
-                { legacyCode: 'forbidden' },
-            );
-        }
+        await this.#assertCanRestructure(source, userId);
         if (!destinationParent.isDir) {
             throw new HttpError(400, 'Destination parent is not a directory', {
                 legacyCode: 'dest_is_not_a_directory',
@@ -3715,7 +3721,7 @@ export class FSService extends PuterService {
 
         if (source.isDir && source.path !== finalPath) {
             await this.stores.fsEntry.updatePathPrefixForUser(
-                userId,
+                source.userId,
                 source.path,
                 finalPath,
             );