fix: events hardening (#3814)

src/backend/services/events/EventsService.ts

@@ -184,6 +184,7 @@ import {
     assertShareableAppUid,
     assertShareablePermission,
     assertShareablePrefix,
+    kvShareAppDelegateImplicator,
     kvShareGrantCovers,
     kvShareManageNamespaceRoot,
     kvShareManagePermission,
@@ -1298,6 +1299,15 @@ export class EventsService extends PuterService {
         // which is what lets its owner mint a handle on their own data.
         this.services.permission.registerImplicator(kvShareOwnerImplicator());
 
+        // Lets an app-under-user actor exercise a share grant its user holds,
+        // scoped to the app the grant's namespace names.
+        this.services.permission.registerImplicator(
+            kvShareAppDelegateImplicator({
+                userHolds: (actor, permission) =>
+                    this.services.permission.check(actor, permission),
+            }),
+        );
+
         this.#armExpirySweep();
         this.#armPendingSweep();
         this.#armCreditSweep();

src/backend/services/events/kvHandleDelegation.integration.test.ts

@@ -18,14 +18,17 @@
  */
 
 /**
- * An app minting a share handle on its user's data.
+ * An app minting a share handle on its user's data, and a grantee reading one
+ * through their own app.
  *
  * The bounds are the ones sharing already puts on an app handing out its
  * user's files: the authority is the user's, the consent is a `manage:` grant
  * the user gave this app on this region, and the reach is whatever the
  * credential structurally holds — for key-value that is one namespace. What
  * these cases pin is that each of those is actually load-bearing, and that a
- * handle minted this way is in every other respect an ordinary one.
+ * handle minted this way is in every other respect an ordinary one — including
+ * for a grantee who exercises it while running as an app, which only works
+ * bound to the same app the region was shared to.
  */
 
 import { v4 as uuidv4 } from 'uuid';
@@ -54,6 +57,7 @@ let env: PuterTestEnv;
 let owner: TestUser;
 let guest: TestUser;
 let appUid: string;
+let appId: number;
 let appActor: Actor;
 
 const events = () => env.server.services.events;
@@ -146,6 +150,7 @@ beforeAll(async () => {
         { ownerUserId: owner.id },
     );
     appUid = app.uid;
+    appId = app.id;
     appActor = makeActor({
         user: owner.actor.user as never,
         app: { uid: app.uid, id: app.id },
@@ -448,3 +453,79 @@ describe('a handle an app minted', () => {
         expect(delivered).toEqual([]);
     });
 });
+
+describe('a grantee subscribing through their own app actor', () => {
+    beforeAll(async () => {
+        await delegate();
+    });
+
+    it('receives the writes when it runs as the region’s own app', async () => {
+        await clearRows();
+        const { handle } = await mint();
+        const guestAppActor = makeActor({
+            user: guest.actor.user as never,
+            app: { uid: appUid, id: appId },
+        });
+
+        const { sub } = await events().subscribe(guestAppActor, SOCKET_ID, {
+            subject: `kv:${handle}:*`,
+        });
+        delivered.length = 0;
+
+        await appWrites(`${PREFIX}messages:1`, { body: 'hello' });
+        await settled();
+
+        expect(delivered).toHaveLength(1);
+        expect(delivered[0].subId).toBe(sub.subId);
+    });
+
+    it('is refused when it runs as a different app than the one the region was shared to', async () => {
+        await clearRows();
+        const { handle } = await mint();
+        // A real app row with an id, so the user-to-app scanner actually runs
+        // and the refusal is the app binding rather than a lookup finding
+        // nothing to read.
+        const name = `kv-other-${uuidv4().slice(0, 8)}`;
+        const other = await env.server.stores.app.create(
+            {
+                name,
+                title: 'Another App',
+                index_url: `https://${name}.example.test/index.html`,
+            },
+            { ownerUserId: guest.id },
+        );
+        const otherAppActor = makeActor({
+            user: guest.actor.user as never,
+            app: { uid: other.uid, id: other.id },
+        });
+
+        await expect(
+            events().subscribe(otherAppActor, SOCKET_ID, {
+                subject: `kv:${handle}:*`,
+            }),
+        ).rejects.toMatchObject({ legacyCode: 'subject_does_not_exist' });
+    });
+
+    it('stops receiving deliveries once the owner revokes the handle', async () => {
+        await clearRows();
+        const { handle } = await mint();
+        const guestAppActor = makeActor({
+            user: guest.actor.user as never,
+            app: { uid: appUid, id: appId },
+        });
+        const { sub } = await events().subscribe(guestAppActor, SOCKET_ID, {
+            subject: `kv:${handle}:*`,
+        });
+
+        await appWrites(`${PREFIX}live:1`, 1);
+        await settled();
+        expect(delivered[0].subId).toBe(sub.subId);
+
+        await events().revokeKvHandle(owner.actor, handle);
+        delivered.length = 0;
+
+        await appWrites(`${PREFIX}live:2`, 2);
+        await quiet();
+        expect(delivered).toEqual([]);
+    });
+});

src/backend/services/events/kvShares.test.ts

@@ -18,7 +18,7 @@
  */
 
 import { v4 as uuidv4 } from 'uuid';
-import { describe, expect, it } from 'vitest';
+import { describe, expect, it, vi } from 'vitest';
 import type { Actor } from '../../core/actor.js';
 import { HttpError } from '../../core/http/HttpError.js';
 import { PermissionUtil } from '../permission/permissionUtil.js';
@@ -28,6 +28,7 @@ import {
     assertShareablePermission,
     assertShareablePrefix,
     keyPrefixSegments,
+    kvShareAppDelegateImplicator,
     kvShareGrantCovers,
     kvShareManagePermission,
     kvShareOwnerImplicator,
@@ -41,10 +42,18 @@ import { isKvHandleId, kvHandleFromSubject } from './subjects.js';
 const OWNER = '2a1b0c9d-0000-4000-8000-000000000001';
 const OTHER = '2a1b0c9d-0000-4000-8000-000000000002';
 const APP = 'app-1234';
+const OTHER_APP = 'app-5678';
 
 const userActor = (uuid: string): Actor =>
     ({ user: { uuid }, effectiveApp: null }) as unknown as Actor;
 
+const appActor = (uuid: string, appUid: string): Actor =>
+    ({
+        user: { uuid },
+        app: { uid: appUid },
+        effectiveApp: { uid: appUid },
+    }) as unknown as Actor;
+
 describe('the share permission family', () => {
     it('names the owner, the app and the granted prefix', () => {
         expect(kvSharePermission(OWNER, APP, 'workspace:abc:')).toBe(
@@ -331,3 +340,100 @@ describe('the owner implicator', () => {
         ).toBeUndefined();
     });
 });
+
+describe('the app delegate implicator', () => {
+    const permission = kvSharePermission(OWNER, APP, 'workspace:abc:');
+
+    it('answers the share family, never its manage arm, nor other families', () => {
+        const implicator = kvShareAppDelegateImplicator({
+            userHolds: vi.fn(),
+        });
+        expect(implicator.matches(permission)).toBe(true);
+        expect(implicator.matches(`manage:${permission}`)).toBe(false);
+        expect(implicator.matches(`fs:${OWNER}:read`)).toBe(false);
+    });
+
+    it('grants when the actor\'s own app matches the namespace app and the user holds the grant', async () => {
+        const userHolds = vi.fn().mockResolvedValue(true);
+        const implicator = kvShareAppDelegateImplicator({ userHolds });
+        const actor = appActor(OWNER, APP);
+
+        await expect(
+            implicator.check({ actor, permission }),
+        ).resolves.toEqual({});
+
+        expect(userHolds).toHaveBeenCalledTimes(1);
+        const [calledActor, calledPermission] = userHolds.mock.calls[0];
+        expect(calledActor.app).toBeUndefined();
+        expect(calledActor.effectiveApp).toBeNull();
+        expect(calledActor.user).toEqual({ uuid: OWNER });
+        expect(calledPermission).toBe(permission);
+    });
+
+    it('refuses when the user does not hold the grant', async () => {
+        const implicator = kvShareAppDelegateImplicator({
+            userHolds: vi.fn().mockResolvedValue(false),
+        });
+        await expect(
+            implicator.check({ actor: appActor(OWNER, APP), permission }),
+        ).resolves.toBeUndefined();
+    });
+
+    it('refuses when the actor\'s app differs from the namespace app', async () => {
+        const userHolds = vi.fn().mockResolvedValue(true);
+        const implicator = kvShareAppDelegateImplicator({ userHolds });
+
+        await expect(
+            implicator.check({
+                actor: appActor(OWNER, OTHER_APP),
+                permission,
+            }),
+        ).resolves.toBeUndefined();
+        expect(userHolds).not.toHaveBeenCalled();
+    });
+
+    it('refuses a plain user actor with no app', async () => {
+        const userHolds = vi.fn().mockResolvedValue(true);
+        const implicator = kvShareAppDelegateImplicator({ userHolds });
+
+        await expect(
+            implicator.check({ actor: userActor(OWNER), permission }),
+        ).resolves.toBeUndefined();
+        expect(userHolds).not.toHaveBeenCalled();
+    });
+
+    it('refuses an access-token actor', async () => {
+        const userHolds = vi.fn().mockResolvedValue(true);
+        const implicator = kvShareAppDelegateImplicator({ userHolds });
+        const actor = {
+            user: { uuid: OWNER },
+            app: { uid: APP },
+            accessToken: { uid: 't' },
+        } as unknown as Actor;
+
+        await expect(
+            implicator.check({ actor, permission }),
+        ).resolves.toBeUndefined();
+        expect(userHolds).not.toHaveBeenCalled();
+    });
+
+    it('refuses a permission with no key segments or no app', async () => {
+        const userHolds = vi.fn().mockResolvedValue(true);
+        const implicator = kvShareAppDelegateImplicator({ userHolds });
+        const actor = appActor(OWNER, APP);
+
+        await expect(
+            implicator.check({
+                actor,
+                permission: PermissionUtil.join('kv-share', OWNER, APP),
+            }),
+        ).resolves.toBeUndefined();
+        await expect(
+            implicator.check({
+                actor,
+                permission: PermissionUtil.join('kv-share', OWNER),
+            }),
+        ).resolves.toBeUndefined();
+        expect(userHolds).not.toHaveBeenCalled();
+    });
+});

src/backend/services/events/kvShares.ts

@@ -18,6 +18,7 @@
  */
 
 import { randomUUID } from 'node:crypto';
+import { type Actor, userRelatedActor } from '../../core/actor.js';
 import { HttpError } from '../../core/http/HttpError.js';
 import { KV_GLOBAL_APP_KEY } from '../../stores/systemKv/SystemKVStore.js';
 import {
@@ -281,3 +282,35 @@ export const kvShareOwnerImplicator = (): PermissionImplicator => ({
         return owner === uuid ? {} : undefined;
     },
 });
+
+/**
+ * Letting an app-under-user actor exercise a `kv-share:` grant its user holds,
+ * bounded to the app the grant's namespace names.
+ *
+ * Only the read arm is matched — never `manage:kv-share:…` — because answering
+ * the manage arm here would read as the unbounded namespace-root delegation and
+ * break minting for everyone (see `kvShareManageNamespaceRoot`). The
+ * namespace-app check keeps one app from reading a region shared with another
+ * app of the same user.
+ */
+export const kvShareAppDelegateImplicator = (deps: {
+    userHolds: (actor: Actor, permission: string) => Promise<boolean>;
+}): PermissionImplicator => ({
+    id: 'kv-share-app-delegate',
+    shortcut: true,
+    matches: (permission: string): boolean => isKvSharePermission(permission),
+    check: async ({ actor, permission }): Promise<unknown> => {
+        if (actor.accessToken) return undefined;
+        const app = actor.app;
+        if (!app?.uid) return undefined;
+
+        const [, owner, namespaceApp, ...segments] =
+            PermissionUtil.split(permission);
+        if (!owner || !namespaceApp || segments.length === 0) return undefined;
+        if (namespaceApp !== app.uid) return undefined;
+
+        return (await deps.userHolds(userRelatedActor(actor), permission))
+            ? {}
+            : undefined;
+    },
+});

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

@@ -2180,6 +2180,50 @@ describe('FSService mkdir, touch, rename and shortcuts', () => {
         expect(await entryAt(user, '/Documents/before.txt')).toBeNull();
     });
 
+    it('rename returns the new path even when the replica lags behind the update', async () => {
+        const entry = await writeFile(
+            user,
+            `${user.home}/Documents/stale-rename.txt`,
+            'x',
+        );
+
+        const db = server.clients.db;
+        const staleRows = (await db.read(
+            'SELECT * FROM fsentries WHERE uuid = ?',
+            [entry.uuid],
+        )) as Array<Record<string, unknown>>;
+        for (const row of staleRows) row.subdomains_agg = null;
+
+        const originalTryHardRead = (Object.getPrototypeOf(db) as typeof db)
+            .tryHardRead;
+        const tryHardReadSpy = vi
+            .spyOn(db, 'tryHardRead')
+            .mockImplementation(async (query: string, params: unknown[] = []) => {
+                if (
+                    query.includes('WHERE uuid = ? LIMIT 1') &&
+                    params[0] === entry.uuid
+                ) {
+                    return staleRows;
+                }
+                return originalTryHardRead.call(db, query, params);
+            });
+        try {
+
+            const renamed = await fs.rename(
+                user.userId,
+                entry,
+                'stale-rename-2.txt',
+            );
+
+            expect(renamed.path).toBe(
+                `${user.home}/Documents/stale-rename-2.txt`,
+            );
+
+        } finally {
+            tryHardReadSpy.mockRestore();
+        }
+    });
+
     it('rewrites descendant paths when a directory is renamed', async () => {
         const dir = await fs.mkdir(user.userId, {
             path: `${user.home}/Documents/olddir`,