fix: tighten up fs perm strings (#3860)

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

@@ -3852,6 +3852,74 @@ describe('FSService permission rules', () => {
         );
         expect(higher).toContain(`manage:fs:${file.uuid}`);
     });
+
+    it('never offers the bare entry as a parent of a moded permission', async () => {
+        const higher = await server.services.permission.getHigherPermissions(
+            `fs:${file.uuid}:write`,
+        );
+        expect(higher).not.toContain(`fs:${file.uuid}`);
+    });
+
+    it('refuses to grant an entry with no mode', async () => {
+        const stranger = await makeUser();
+        for (const specifier of [file.path, file.uuid]) {
+            const error = await caught(() =>
+                server.services.permission.grantUserUserPermission(
+                    user.actor,
+                    stranger.username,
+                    `fs:${specifier}`,
+                ),
+            );
+            expect(error.statusCode).toBe(400);
+            expect(error.legacyCode).toBe('bad_request');
+        }
+    });
+
+    // A row an earlier bug could still have written: it names the entry and no
+    // mode, so the parent walk once let it answer read, write and delete.
+    it('resolves nothing for a stored permission with no mode', async () => {
+        const app = (await server.stores.app.create(
+            {
+                name: `perm-${uuidv4()}`,
+                title: 'FS permission test',
+                index_url: 'https://perm.test/',
+            },
+            { ownerUserId: user.userId },
+        )) as { id: number; uid: string };
+        const appActor = makeActor({
+            user: user.actor.user,
+            app: { uid: app.uid, id: app.id },
+        });
+
+        await server.stores.permission.upsertUserAppPerm(
+            user.userId,
+            app.id,
+            `fs:${file.uuid}`,
+            {},
+        );
+
+        for (const mode of ['see', 'list', 'read', 'write']) {
+            await expect(
+                server.services.permission.check(
+                    appActor,
+                    `fs:${file.uuid}:${mode}`,
+                ),
+            ).resolves.toBe(false);
+        }
+
+        // The same row with a mode on it still resolves, and only that far.
+        await server.services.permission.grantUserAppPermission(
+            user.actor,
+            app.uid,
+            `fs:${file.uuid}:read`,
+        );
+        await expect(
+            server.services.permission.check(appActor, `fs:${file.uuid}:read`),
+        ).resolves.toBe(true);
+        await expect(
+            server.services.permission.check(appActor, `fs:${file.uuid}:write`),
+        ).resolves.toBe(false);
+    });
 });
 
 // -- Cross-app AppData (app-data:<uid>:fs:<class>) ----------------------

src/backend/services/permission/PermissionService.test.ts

@@ -161,6 +161,14 @@ describe('PermissionService.getHigherPermissions', () => {
         expect(higher).toEqual(expect.arrayContaining(['a:b:c', 'a:b', 'a']));
     });
 
+    it('drops the bare fs parents, which no grant may hold', async () => {
+        const service = createPermissionService();
+        const higher = await service.getHigherPermissions('fs:some-uuid:write');
+        expect(higher).toContain('fs:some-uuid:write');
+        expect(higher).not.toContain('fs:some-uuid');
+        expect(higher).not.toContain('fs');
+    });
+
     it('expands via registered exploders when the parent matches', async () => {
         const service = createPermissionService();
         service.registerExploder({

src/backend/services/permission/PermissionService.ts

@@ -30,6 +30,7 @@ import {
     PERMISSION_SCAN_CACHE_TTL_SECONDS,
 } from './consts';
 import {
+    isBareFsPermission,
     PermissionUtil,
     readingHasTerminal,
     type PermissionExploder,
@@ -126,7 +127,9 @@ export class PermissionService extends PuterService {
                 for (const p of more) higher.add(p);
             }
         }
-        return [...higher];
+        // The parent walk reaches `fs:<uid>`, which no grant is allowed to
+        // hold; drop it so a stored one can't answer a check for any mode.
+        return [...higher].filter((p) => !isBareFsPermission(p));
     }
 
     getParentPermissions(permission: string): string[] {
@@ -140,6 +143,22 @@ export class PermissionService extends PuterService {
         return parents;
     }
 
+    /**
+     * Grants only: an `fs:` permission has to name a mode. Without one the row
+     * sits above every mode, and a request that simply omits it reads to the
+     * user as a narrower grant than it is. Revokes stay unguarded so an
+     * existing bare row can still be withdrawn.
+     */
+    assertGrantableFsPermission(permission: string): void {
+        if (isBareFsPermission(permission)) {
+            throw new HttpError(
+                400,
+                'Invalid `permission`: `fs` requires an access mode',
+                { legacyCode: 'bad_request' },
+            );
+        }
+    }
+
     // -- Public check / scan API --------------------------------------
 
     async check(
@@ -825,6 +844,7 @@ export class PermissionService extends PuterService {
         meta: GrantMeta = {},
     ): Promise<void> {
         permission = await this.rewritePermission(permission);
+        this.assertGrantableFsPermission(permission);
         const user = await this.stores.user.getByUsername(username);
         if (!user)
             throw new HttpError(404, `user_does_not_exist: ${username}`, {
@@ -933,6 +953,7 @@ export class PermissionService extends PuterService {
     ): Promise<void> {
         // First: the rewrite decides the row's width and what a revoke matches.
         permission = await this.rewritePermission(permission);
+        this.assertGrantableFsPermission(permission);
         if (permission.length > PERMISSION_MAX_LEN) {
             throw new HttpError(400, 'permission is too long', {
                 legacyCode: 'bad_request',
@@ -1287,6 +1308,7 @@ export class PermissionService extends PuterService {
      */
     async assertUserAppPermissionWritable(permission: string): Promise<void> {
         const rewritten = await this.#rewriteForUserAppWrite(permission);
+        this.assertGrantableFsPermission(rewritten);
         if (rewritten.length > PERMISSION_MAX_LEN) {
             throw new HttpError(400, 'Invalid `permission`', {
                 legacyCode: 'bad_request',
@@ -1302,6 +1324,7 @@ export class PermissionService extends PuterService {
         meta: GrantMeta = {},
     ): Promise<void> {
         permission = await this.#rewriteForUserAppWrite(permission);
+        this.assertGrantableFsPermission(permission);
         // Checked after the rewrite, because the rewrite is what decides how
         // wide the row actually is: `fs:/deep/path:read` collapses to
         // `fs:<uuid>:read`. Reject here rather than let an oversized string
@@ -1448,6 +1471,7 @@ export class PermissionService extends PuterService {
         meta: GrantMeta = {},
     ): Promise<void> {
         permission = await this.rewritePermission(permission);
+        this.assertGrantableFsPermission(permission);
         const app = await this.stores.app.resolveApp(appIdentifier);
         if (!app)
             throw new HttpError(404, `entity_not_found: app:${appIdentifier}`, {

src/backend/services/permission/permissionUtil.ts

@@ -165,6 +165,17 @@ export const PermissionUtil = {
     },
 };
 
+/**
+ * Whether a permission names an fs entry but no access mode. `fs:<uid>` is a
+ * parent of `fs:<uid>:<mode>`, so one answers every mode over the entry and
+ * everything under it. Nothing grants one deliberately, so both the grant path
+ * and the check path treat it as invalid rather than as a wildcard.
+ */
+export const isBareFsPermission = (permission: string): boolean => {
+    const parts = PermissionUtil.split(permission);
+    return parts[0] === 'fs' && parts.length < 3;
+};
+
 /**
  * Check whether a reading includes any terminal node (an `option`, or a `path`
  * that itself transitively terminates).

src/gui/src/UI/UIPermissionDialog.js

@@ -552,6 +552,9 @@ function permission_icon_svg (icon) {
     return icons[icon] ?? icons.shield;
 }
 
+/** Access modes an `fs:` permission can ask for; anything else is not shown. */
+const FS_ACCESS_MODES = ['see', 'list', 'read', 'write'];
+
 /**
  * Generates a user-friendly description of a permission string.
  *
@@ -568,6 +571,11 @@ async function get_permission_description (permission, options = {}) {
         const [resource_type, resource_id, action, interface_name = null] = parts;
 
         if ( resource_type === 'fs' ) {
+            // No mode means no verb to put in front of the user, and a
+            // modeless `fs:` grant is one the backend refuses anyway.
+            if ( ! FS_ACCESS_MODES.includes(action) ) {
+                return null;
+            }
             // Check for standard folders using whoami().directories
             const standard_folder_description = await get_standard_folder_description(resource_id, action);
             if ( standard_folder_description ) {