From 15134c99bf106e3e9a2ff6a69c80e3ab047c48bc Mon Sep 17 00:00:00 2001 From: Sourabh Sharma Date: Mon, 8 Jun 2026 01:19:00 +0530 Subject: [PATCH] Fix mkdir dedupeName for existing directories (#3215) --- .../controllers/fs/FSController.test.ts | 76 +++++++++++++++++++ src/backend/controllers/fs/FSController.ts | 17 ++++- .../controllers/fs/LegacyFSController.test.ts | 72 ++++++++++++++++++ .../controllers/fs/LegacyFSController.ts | 18 ++++- src/backend/services/fs/FSService.ts | 11 ++- 5 files changed, 187 insertions(+), 7 deletions(-) diff --git a/src/backend/controllers/fs/FSController.test.ts b/src/backend/controllers/fs/FSController.test.ts index 84207fa97..ee84b571a 100644 --- a/src/backend/controllers/fs/FSController.test.ts +++ b/src/backend/controllers/fs/FSController.test.ts @@ -953,6 +953,82 @@ describe('FSController.mkdirEntry', () => { expect(body.isDir).toBe(true); }); + it('dedupes an existing directory when dedupe_name is true', async () => { + const { actor } = await makeUser(); + const username = actor.user!.username!; + const target = `/${username}/Documents/hello`; + + await withActor(actor, () => + controller.mkdirEntry( + makeReq({ + body: { path: target }, + actor, + }), + makeRes().res, + ), + ); + + const { res, captured } = makeRes(); + await withActor(actor, () => + controller.mkdirEntry( + makeReq({ + body: { path: target, dedupe_name: true }, + actor, + }), + res, + ), + ); + + const body = captured.body as { + path: string; + name: string; + isDir: boolean; + }; + expect(body.path).toBe(`/${username}/Documents/hello (1)`); + expect(body.name).toBe('hello (1)'); + expect(body.isDir).toBe(true); + expect( + await server.stores.fsEntry.getEntryByPath( + `/${username}/Documents/hello (1)`, + ), + ).toMatchObject({ isDir: true }); + }); + + it('requires parent write when deduping an existing directory', async () => { + const { actor: userActor } = await makeUser(); + const username = userActor.user!.username!; + const appUid = `app-mkdir-${uuidv4()}`; + const appActor: Actor = { ...userActor, app: { uid: appUid } }; + const target = `/${username}/AppData/${appUid}`; + + await withActor(userActor, () => + controller.mkdirEntry( + makeReq({ + body: { path: target }, + actor: userActor, + }), + makeRes().res, + ), + ); + + await expect( + withActor(appActor, () => + controller.mkdirEntry( + makeReq({ + body: { path: target, dedupe_name: true }, + actor: appActor, + }), + makeRes().res, + ), + ), + ).rejects.toMatchObject({ statusCode: 404 }); + expect( + await server.stores.fsEntry.getEntryByPath( + `/${username}/AppData/${appUid} (1)`, + ), + ).toBeNull(); + }); + it('expands ~/ in the path to the user home', async () => { const { actor } = await makeUser(); const username = actor.user!.username!; diff --git a/src/backend/controllers/fs/FSController.ts b/src/backend/controllers/fs/FSController.ts index f9aebe3da..139a37682 100644 --- a/src/backend/controllers/fs/FSController.ts +++ b/src/backend/controllers/fs/FSController.ts @@ -1039,13 +1039,15 @@ export class FSController extends PuterController { legacyCode: 'bad_request', }); + const dedupeName = + this.#toBoolean(body.dedupe_name ?? body.dedupeName) ?? false; await this.#assertCanCreate(actor, path); + if (dedupeName) await this.#assertCanDedupeCreate(actor, path); const entry = await this.services.fs.mkdir(userId, { path, overwrite: this.#toBoolean(body.overwrite) ?? false, - dedupeName: - this.#toBoolean(body.dedupe_name ?? body.dedupeName) ?? false, + dedupeName, createMissingParents: this.#toBoolean( body.create_missing_parents ?? @@ -1310,6 +1312,17 @@ export class FSController extends PuterController { await this.#assertAccess(actor, parentForCheck, 'write'); } + async #assertCanDedupeCreate(actor: Actor, targetPath: string) { + const existing = await this.stores.fsEntry.getEntryByPath(targetPath); + if (!existing) return; + const parent = pathPosix.dirname(targetPath); + await this.#assertAccess( + actor, + parent === '/' ? targetPath : parent, + 'write', + ); + } + async #assertAccess( actor: Actor, path: string, diff --git a/src/backend/controllers/fs/LegacyFSController.test.ts b/src/backend/controllers/fs/LegacyFSController.test.ts index 73d375672..27e2407b0 100644 --- a/src/backend/controllers/fs/LegacyFSController.test.ts +++ b/src/backend/controllers/fs/LegacyFSController.test.ts @@ -206,6 +206,78 @@ describe('LegacyFSController.mkdir', () => { expect(fetched?.isDir).toBe(true); }); + it('dedupes an existing directory when dedupe_name is true', async () => { + const { actor } = await makeUser(); + const username = actor.user!.username!; + const parent = `/${username}/Documents`; + + await withActor(actor, () => + controller.mkdir( + makeReq({ + body: { parent, path: 'hello' }, + actor, + }), + makeRes().res, + ), + ); + + const { res, captured } = makeRes(); + await withActor(actor, () => + controller.mkdir( + makeReq({ + body: { parent, path: 'hello', dedupe_name: true }, + actor, + }), + res, + ), + ); + + const body = captured.body as Record; + expect(body).toMatchObject({ + path: `${parent}/hello (1)`, + name: 'hello (1)', + is_dir: true, + }); + expect( + await server.stores.fsEntry.getEntryByPath(`${parent}/hello (1)`), + ).toMatchObject({ isDir: true }); + }); + + it('requires parent write when deduping an existing directory', async () => { + const { actor: userActor } = await makeUser(); + const username = userActor.user!.username!; + const appUid = `app-legacy-mkdir-${uuidv4()}`; + const appActor: Actor = { ...userActor, app: { uid: appUid } }; + const parent = `/${username}/AppData`; + + await withActor(userActor, () => + controller.mkdir( + makeReq({ + body: { parent, path: appUid }, + actor: userActor, + }), + makeRes().res, + ), + ); + + await expect( + withActor(appActor, () => + controller.mkdir( + makeReq({ + body: { parent, path: appUid, dedupe_name: true }, + actor: appActor, + }), + makeRes().res, + ), + ), + ).rejects.toMatchObject({ statusCode: 404 }); + expect( + await server.stores.fsEntry.getEntryByPath( + `${parent}/${appUid} (1)`, + ), + ).toBeNull(); + }); + it("rejects writing into another user's home with a 4xx", async () => { const a = await makeUser(); const b = await makeUser(); diff --git a/src/backend/controllers/fs/LegacyFSController.ts b/src/backend/controllers/fs/LegacyFSController.ts index fb8cf5308..fd96014dd 100644 --- a/src/backend/controllers/fs/LegacyFSController.ts +++ b/src/backend/controllers/fs/LegacyFSController.ts @@ -427,17 +427,33 @@ export class LegacyFSController extends PuterController { const normalizedTarget = targetPath.startsWith('/') ? targetPath : `/${targetPath}`; + const dedupeName = + getBoolean(body, 'dedupe_name', 'change_name') ?? false; await assertCanCreate( this.services.acl, this.services.fs, actor, normalizedTarget, ); + if (dedupeName) { + const existing = + await this.stores.fsEntry.getEntryByPath(normalizedTarget); + if (existing) { + const parent = pathPosix.dirname(normalizedTarget); + await assertAccess( + this.services.acl, + this.services.fs, + actor, + parent === '/' ? normalizedTarget : parent, + 'write', + ); + } + } const entry = await this.services.fs.mkdir(userId, { path: targetPath, overwrite: getBoolean(body, 'overwrite') ?? false, - dedupeName: getBoolean(body, 'dedupe_name', 'change_name') ?? false, + dedupeName, createMissingParents: getBoolean( body, diff --git a/src/backend/services/fs/FSService.ts b/src/backend/services/fs/FSService.ts index cab7bdf9e..9324c9d13 100644 --- a/src/backend/services/fs/FSService.ts +++ b/src/backend/services/fs/FSService.ts @@ -2840,10 +2840,13 @@ export class FSService extends PuterService { const existing = await this.stores.fsEntry.getEntryByPath(targetPath); if (existing) { if (existing.isDir) { - // A directory already exists at path: idempotent success. - return existing; - } - if (input.overwrite) { + if (input.dedupeName) { + name = await this.#findDedupedName(parent, name); + } else { + // A directory already exists at path: idempotent success. + return existing; + } + } else if (input.overwrite) { // Remove the non-directory occupant then create the dir. await this.remove(userId, { entry: existing,