fix(share): walk a directory revoke by parent linkage, not path prefix

listByFsentrySubtree matched descendants with fsentry_id = ? OR path
LIKE ?, which has two problems: fsentries.path is lazily backfilled
and NULL on old rows, so those descendants' shares silently survived a
directory revoke, and the OR'd predicates forced a scan of every
active share. A recursive CTE over parent_id — the same shape the
lineage resolver already uses — covers every descendant and runs on
idx_parentId_name.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Juan Castro
2026-08-14 14:38:11 -04:00
co-authored by Claude Fable 5
parent 68f34f0f0c
commit 058f41145b
4 changed files with 63 additions and 15 deletions
@@ -82,9 +82,18 @@ describe('ShareService', () => {
'INSERT INTO `fsentries` (`uuid`, `name`, `path`, `user_id`, `is_dir`, `modified`) VALUES (?, ?, ?, ?, 1, ?)',
[dirUuid, dirName, dirPath, owner.id, now],
);
const dirRow = await server.stores.fsEntry.getEntryByPath(dirPath);
await server.clients.db.write(
'INSERT INTO `fsentries` (`uuid`, `name`, `path`, `user_id`, `is_dir`, `modified`) VALUES (?, ?, ?, ?, 0, ?)',
[fileUuid, fileName, `${dirPath}/${fileName}`, owner.id, now],
'INSERT INTO `fsentries` (`uuid`, `name`, `path`, `user_id`, `is_dir`, `modified`, `parent_id`, `parent_uid`) VALUES (?, ?, ?, ?, 0, ?, ?, ?)',
[
fileUuid,
fileName,
`${dirPath}/${fileName}`,
owner.id,
now,
dirRow!.id,
dirUuid,
],
);
const dir = await server.stores.fsEntry.getEntryByPath(dirPath);
+1 -1
View File
@@ -356,7 +356,7 @@ export class ShareService extends PuterService {
// The whole subtree, not just this node: `manage` inherits downwards,
// so a grant on a descendant can rest on authority held here.
const rows = (
await this.stores.share.listByFsentrySubtree(entry.id, entry.path)
await this.stores.share.listByFsentrySubtree(entry.id)
).filter(
(row: { issuer_user_id: number }) =>
Number(row.issuer_user_id) === issuerId,
+14 -12
View File
@@ -111,23 +111,25 @@ export class ShareStore extends PuterStore {
}
/**
* Active shares on a directory and everything beneath it. `manage` inherits
* downwards, so a revoke here has to see what rests on it.
* Active shares on a directory and everything beneath it. Walks by parent
* linkage, not path prefix `fsentries.path` is lazily backfilled and NULL
* on old rows, so a LIKE would skip those descendants' shares.
*
* @param {number} fsentryId
* @param {string} path Directory path, used to match descendants.
*/
async listByFsentrySubtree(fsentryId, path) {
// `!` escapes the LIKE wildcards so a directory named with `%` or `_`
// cannot widen the match into siblings.
const prefix = `${String(path).replace(/([!%_])/g, '!$1')}/%`;
async listByFsentrySubtree(fsentryId) {
const rows = await this.clients.db.read(
'SELECT `share`.* FROM `share` ' +
'JOIN `fsentries` ON `fsentries`.`id` = `share`.`fsentry_id` ' +
'WHERE `share`.`holder_user_id` IS NOT NULL AND ' +
"(`share`.`fsentry_id` = ? OR `fsentries`.`path` LIKE ? ESCAPE '!') " +
'WITH RECURSIVE `subtree`(`id`) AS (' +
'SELECT `id` FROM `fsentries` WHERE `id` = ? ' +
'UNION ALL ' +
'SELECT `f`.`id` FROM `fsentries` `f` ' +
'JOIN `subtree` `s` ON `f`.`parent_id` = `s`.`id`' +
') ' +
'SELECT `share`.* FROM `share` ' +
'JOIN `subtree` ON `share`.`fsentry_id` = `subtree`.`id` ' +
'WHERE `share`.`holder_user_id` IS NOT NULL ' +
'ORDER BY `share`.`id`',
[fsentryId, prefix],
[fsentryId],
);
return rows.map((r) => this.#normalizeRow(r));
}
@@ -262,6 +262,43 @@ describe('ShareStore', () => {
holder = await makeUser();
});
it('finds a subtree share even when the descendant has no path yet', async () => {
const now = Math.floor(Date.now() / 1000);
const dirUuid = uuidv4();
await server.clients.db.write(
'INSERT INTO `fsentries` (`uuid`, `name`, `path`, `user_id`, `is_dir`, `modified`) VALUES (?, ?, ?, ?, 1, ?)',
[dirUuid, `d-${dirUuid.slice(0, 8)}`, `/x/${dirUuid}`, issuer.id, now],
);
const dirRows = await server.clients.db.read(
'SELECT `id` FROM `fsentries` WHERE `uuid` = ?',
[dirUuid],
);
const dirId = Number(dirRows[0].id);
const childUuid = uuidv4();
await server.clients.db.write(
'INSERT INTO `fsentries` (`uuid`, `name`, `path`, `user_id`, `is_dir`, `modified`, `parent_id`, `parent_uid`) VALUES (?, ?, NULL, ?, 0, ?, ?, ?)',
[childUuid, `f-${childUuid.slice(0, 8)}`, issuer.id, now, dirId, dirUuid],
);
const childRows = await server.clients.db.read(
'SELECT `id` FROM `fsentries` WHERE `uuid` = ?',
[childUuid],
);
const childId = Number(childRows[0].id);
await store.upsertActive({
issuerUserId: issuer.id,
holderUserId: holder.id,
fsentryId: childId,
mode: 'read',
});
const rows = await store.listByFsentrySubtree(dirId);
expect(
rows.some((r) => Number(r.fsentry_id) === childId),
).toBe(true);
});
it('records an active share and lists it for the holder', async () => {
const entry = await makeEntry(issuer);
const created = await store.upsertActive({