lee/puter
1
0
Fork 0
mirror of https://github.com/HeyPuter/puter.git synced 2026-08-28 08:56:58 +00:00
Code Issues Actions 6 Packages Projects Releases Wiki Activity

fix: tell every recipient when a shared file disappears

Browse Source 
Both ways a file can vanish were silent for anyone holding the folder
above it, so a third party's window kept showing a file that was gone
and 404'd on click.

A delete built its audience from the permission rows it removed, and a
file inside a shared folder has no grant of its own — only the folder
does — so the audience was empty. A move resolved its audience from the
entry's new path, and the GUI's Delete is a move to the owner's Trash,
where no recipient has a share.

Resolve the audience from where the entry was rather than from the
grants that went with it:

- Deletes also fan out to holders reaching the entry through an
  ancestor, coalesced by parent so a subtree stays a couple of queries.
  A holder covered by both passes is told once.
- Moves resolve both ends. Reaching both is item.moved, only the
  destination item.added, only the origin item.removed.

Recipients are named by the path they knew, masked through their own
share rather than the owner's tree.
This commit is contained in:
Juan Castro
2026-08-26 14:54:06 -04:00
parent cf00ebb9d4
commit 8e622189ca
2 changed files with 397 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/backend/services/share/ShareService.test.ts
+257 -1
View File
@@ -1519,6 +1519,202 @@ describe('ShareService', () => {
expect(audiences.flat()).toContain(recipient.user.id);
});
it('tells a folder recipient when a file inside it is deleted', async () => {
const owner = await makeUser();
const recipient = await makeUser();
const { dir, file } = await makeDirWithFile(owner.user);
await share(owner.actor, {
uid: dir.uuid,
recipient: { email: recipient.email },
mode: 'read',
});
// Nothing was shared at the file, so the revoke reports no holder
// for it — the audience has to be found through the folder.
const payload = await capturePayload(
'outer.gui.item.removed',
recipient.user.id,
async () => {
await server.services.fs.remove(owner.user.id, {
entry: file,
});
},
);
// Named by the path they knew, masked through the folder they hold.
expect(payload?.path).toBe(
`/${owner.user.username}/${dir.uuid}/${dir.name}/${file.name}`,
);
});
it('tells a recipient once when they hold both the file and its folder', async () => {
const owner = await makeUser();
const recipient = await makeUser();
const { dir, file } = await makeDirWithFile(owner.user);
for (const uid of [dir.uuid, file.uuid]) {
await share(owner.actor, {
uid,
recipient: { email: recipient.email },
mode: 'read',
});
}
// The revoke reports them for the file, and the folder reaches it
// too — one delete must not arrive as two removals.
const audiences = await captureAudiences(
'outer.gui.item.removed',
file.uuid,
async () => {
await server.services.fs.remove(owner.user.id, {
entry: file,
});
},
);
const told = audiences
.flat()
.filter((id) => id === recipient.user.id);
expect(told).toHaveLength(1);
});
it('tells a folder recipient when a file is moved out of it', async () => {
const owner = await makeUser();
const recipient = await makeUser();
const { dir, file } = await makeDirWithFile(owner.user);
const before = file.path;
const after = `/${owner.user.username}/Trash/${file.name}`;
await share(owner.actor, {
uid: dir.uuid,
recipient: { email: recipient.email },
mode: 'read',
});
// What the GUI's Delete does. `item.moved` would name a path the
// recipient cannot see, leaving the file on their screen.
const audiences = await captureAudiences(
'outer.gui.item.removed',
file.uuid,
async () => {
await server.clients.event.emitAndWait(
'fs.move.node',
{
node: { ...file, path: after },
fromPath: before,
toPath: after,
},
{},
);
},
);
expect(audiences.flat()).toContain(recipient.user.id);
});
it('names a moved-out file by where the recipient last saw it', async () => {
const owner = await makeUser();
const recipient = await makeUser();
const { dir, file } = await makeDirWithFile(owner.user);
const before = file.path;
const after = `/${owner.user.username}/Trash/${file.name}`;
await share(owner.actor, {
uid: dir.uuid,
recipient: { email: recipient.email },
mode: 'read',
});
const payload = await capturePayload(
'outer.gui.item.removed',
recipient.user.id,
async () => {
await server.clients.event.emitAndWait(
'fs.move.node',
{
node: { ...file, path: after },
fromPath: before,
toPath: after,
},
{},
);
},
);
// Masked through the folder they hold, not the owner's Trash.
expect(payload?.path).toBe(
`/${owner.user.username}/${dir.uuid}/${dir.name}/${file.name}`,
);
});
it('tells a folder recipient when a file is moved into it', async () => {
const owner = await makeUser();
const recipient = await makeUser();
const { dir } = await makeDirWithFile(owner.user);
const loose = await makeFile(owner.user);
const after = `${dir.path}/${loose.name}`;
await share(owner.actor, {
uid: dir.uuid,
recipient: { email: recipient.email },
mode: 'read',
});
// They never had it, so there is nothing for `item.moved` to move.
const audiences = await captureAudiences(
'outer.gui.item.added',
loose.uuid,
async () => {
await server.clients.event.emitAndWait(
'fs.move.node',
{
node: { ...loose, path: after },
fromPath: loose.path,
toPath: after,
},
{},
);
},
);
expect(audiences.flat()).toContain(recipient.user.id);
});
it('still reports a move within the shared folder as a move', async () => {
const owner = await makeUser();
const recipient = await makeUser();
const { dir, file } = await makeDirWithFile(owner.user);
const before = file.path;
const after = `${dir.path}/renamed-${file.name}`;
await share(owner.actor, {
uid: dir.uuid,
recipient: { email: recipient.email },
mode: 'read',
});
const payload = await capturePayload(
'outer.gui.item.moved',
recipient.user.id,
async () => {
await server.clients.event.emitAndWait(
'fs.move.node',
{
node: { ...file, path: after },
fromPath: before,
toPath: after,
},
{},
);
},
);
const masked = `/${owner.user.username}/${dir.uuid}/${dir.name}`;
expect(payload?.from_path).toBe(`${masked}/${file.name}`);
expect(payload?.path).toBe(`${masked}/renamed-${file.name}`);
});
it('tells a folder recipient when a file inside it changes', async () => {
const owner = await makeUser();
const recipient = await makeUser();
@@ -1638,7 +1834,10 @@ describe('ShareService', () => {
/** The one payload sent to `holder` for `event`, or undefined. */
const capturePayload = async (
event: 'outer.gui.item.moved' | 'outer.gui.item.renamed',
event:
| 'outer.gui.item.moved'
| 'outer.gui.item.removed'
| 'outer.gui.item.renamed',
holderId: number,
fn: () => Promise<void>,
) => {
@@ -1703,6 +1902,63 @@ describe('ShareService', () => {
expect(lookups).toBe(1);
});
it('asks once per folder when a whole subtree is deleted', async () => {
const owner = await makeUser();
const recipient = await makeUser();
const { dir } = await makeDirWithFile(owner.user);
await share(owner.actor, {
uid: dir.uuid,
recipient: { email: recipient.email },
mode: 'read',
});
await Promise.all(
Array.from({ length: 8 }, (_, i) =>
server.services.fs.touch(owner.user.id, {
path: `${dir.path}/doomed-${i}.txt`,
}),
),
);
const files = await Promise.all(
Array.from({ length: 8 }, (_, i) =>
server.stores.fsEntry.getEntryByPath(
`${dir.path}/doomed-${i}.txt`,
),
),
);
const seen: number[][] = [];
const listener = (_key: string, data: unknown) => {
const payload = data as { user_id_list?: number[] };
if (!payload.user_id_list?.includes(recipient.user.id)) return;
seen.push(payload.user_id_list);
};
server.clients.event.on('outer.gui.item.removed', listener);
const reaching = vi.spyOn(server.stores.share, 'listReaching');
let lookups = -1;
try {
await Promise.all(
files.map((entry) =>
server.services.fs.remove(owner.user.id, { entry }),
),
);
for (let i = 0; i < 50 && seen.length < 8; i++) {
await new Promise((resolve) => setTimeout(resolve, 20));
}
lookups = reaching.mock.calls.length;
} finally {
server.clients.event.off('outer.gui.item.removed', listener);
reaching.mockRestore();
}
// Siblings share a parent, so they share the audience lookup —
// a burst settles in one or two flushes, never one per file.
expect(seen).toHaveLength(8);
expect(lookups).toBeLessThanOrEqual(2);
});
it('stays quiet for an event another node already handled', async () => {
const owner = await makeUser();
const recipient = await makeUser();
src/backend/services/share/ShareService.ts
+140 -15
View File
@@ -205,9 +205,11 @@ const holderPayload = (
entry: FSEntry,
/** Defaults to the entry itself, for the paths with no root to mask by. */
root: FSEntry = entry,
/** Where the entry was; differs from `entry.path` once it has moved. */
realPath: string = entry.path,
): Record<string, unknown> => {
const path =
maskedPathVia(root, entry.path) ?? maskedSelfPath(entry, entry.path);
maskedPathVia(root, realPath) ?? maskedSelfPath(entry, realPath);
return {
uid: entry.uuid,
uuid: entry.uuid,
@@ -320,17 +322,7 @@ export class ShareService extends PuterService {
fromUserId?: number;
};
if (!node?.uuid) return;
const notify = this.#fanOutToHolders(
node,
'outer.gui.item.moved',
(root) => {
// Omitted when the move started outside this share.
const from = fromPath
? maskedFormerPath(root, node, fromPath)
: null;
return from ? { from_path: from } : {};
},
).catch(() => {
const notify = this.#fanOutMove(node, fromPath).catch(() => {
// A stale window is better than a failed move.
});
@@ -499,6 +491,7 @@ export class ShareService extends PuterService {
*/
async #flushRetire(entries: FSEntry[]): Promise<void> {
const ownerOf = new Map(entries.map((e) => [e.uuid, e]));
const notified = new Map<string, Set<number>>();
for (let i = 0; i < entries.length; i += RETIRE_CHUNK_SIZE) {
const chunk = entries.slice(i, i + RETIRE_CHUNK_SIZE);
@@ -526,8 +519,135 @@ export class ShareService extends PuterService {
[...holders],
holderPayload(entry),
);
notified.set(uuid, holders);
}
}
await this.#fanOutRetiredToAncestors(entries, notified);
}
/**
* Tell whoever reached these through a folder above them. Their grant is on
* that folder, not on what was deleted, so the revoke reports no holder for
* them — and the file stays in their window, 404ing when they open it.
* Coalesced by parent like creates, so a subtree stays a few queries.
*/
async #fanOutRetiredToAncestors(
entries: FSEntry[],
notified: Map<string, Set<number>>,
): Promise<void> {
const byParent = new Map<string, FSEntry[]>();
for (const entry of entries) {
const parent = pathPosix.dirname(entry.path);
byParent.set(parent, [...(byParent.get(parent) ?? []), entry]);
}
for (const siblings of byParent.values()) {
const first = siblings[0];
if (!first) continue;
// Ancestors only — a share on the entry itself is the revoke's to
// report, and it already has.
const groups = (await this.#reachingRoots(first)).filter(
({ root }) => root.uuid !== first.uuid,
);
for (const { root, holders } of groups) {
for (const entry of siblings) {
const unheard = holders.filter(
(holder) => !notified.get(entry.uuid)?.has(holder),
);
if (!unheard.length) continue;
await this.#emitGui(
'outer.gui.item.removed',
unheard,
holderPayload(entry, root),
);
}
}
}
}
/**
* A move seen from both ends, because it can take an item out of someone's
* share as easily as into it. Reaching both ends is a move, only the
* destination an arrival, only the origin a removal — that last being the
* GUI's Delete, a move to the owner's Trash where no recipient has a
* share.
*/
async #fanOutMove(entry: FSEntry, fromPath?: string): Promise<void> {
const destination = await this.#reachingRoots(entry);
const origin =
fromPath && fromPath !== entry.path
? await this.#reachingRoots(entry, fromPath)
: destination;
const rootsBySide = (
groups: Array<{ root: FSEntry; holders: number[] }>,
) =>
new Map<number, FSEntry>(
groups.flatMap(({ root, holders }) =>
holders.map((holder) => [holder, root] as const),
),
);
const originRootOf = rootsBySide(origin);
const destinationRootOf = rootsBySide(destination);
// Keyed on both shares: the paths a holder is told depend on the one
// they saw each end through, and those need not be the same share.
const batches = new Map<
string,
{
event: HolderGuiEvent;
from?: FSEntry;
to?: FSEntry;
holders: number[];
}
>();
const place = (
holder: number,
event: HolderGuiEvent,
from?: FSEntry,
to?: FSEntry,
) => {
const key = `${event}|${from?.id ?? ''}|${to?.id ?? ''}`;
const batch = batches.get(key) ?? { event, from, to, holders: [] };
batch.holders.push(holder);
batches.set(key, batch);
};
for (const [holder, to] of destinationRootOf) {
const from = originRootOf.get(holder);
place(
holder,
from ? 'outer.gui.item.moved' : 'outer.gui.item.added',
from,
to,
);
}
for (const [holder, from] of originRootOf) {
if (destinationRootOf.has(holder)) continue;
place(holder, 'outer.gui.item.removed', from);
}
for (const { event, from, to, holders } of batches.values()) {
if (event === 'outer.gui.item.removed') {
// Named by where they last saw it, which is all they have.
await this.#emitGui(
event,
holders,
holderPayload(entry, from as FSEntry, fromPath),
);
continue;
}
const formerPath =
from && fromPath
? maskedFormerPath(from, entry, fromPath)
: null;
await this.#emitGui(event, holders, {
...holderPayload(entry, to as FSEntry),
// The GUI rewrites the item it already has by this.
...(formerPath ? { from_path: formerPath } : {}),
});
}
}
async #fanOutToHolders(
@@ -550,8 +670,10 @@ export class ShareService extends PuterService {
/** Holders who can reach `entry`, grouped by the share they came through. */
async #reachingRoots(
entry: FSEntry,
/** Where to look from; the former path once the entry has moved. */
realPath: string = entry.path,
): Promise<Array<{ root: FSEntry; holders: number[] }>> {
const { rows, nodesById } = await this.#sharesReaching(entry);
const { rows, nodesById } = await this.#sharesReaching(entry, realPath);
// Deepest root wins, so a holder with nested shares is told once.
const rootByHolder = new Map<number, FSEntry>();
@@ -588,13 +710,16 @@ export class ShareService extends PuterService {
* share table to join fsentries and match `path IN (...)` instead put an
* un-indexable OR on the write path.
*/
async #sharesReaching(entry: FSEntry): Promise<{
async #sharesReaching(
entry: FSEntry,
realPath: string = entry.path,
): Promise<{
rows: Array<{ holder_user_id: number; fsentry_id: number }>;
nodesById: Map<number, FSEntry>;
}> {
const ancestorPaths: string[] = [];
for (
let cursor = pathPosix.dirname(entry.path);
let cursor = pathPosix.dirname(realPath);
cursor !== '/' && cursor !== '.';
cursor = pathPosix.dirname(cursor)
) {