diff --git a/src/backend/controllers/auth/AuthController.test.ts b/src/backend/controllers/auth/AuthController.test.ts index 7d468b6af..417d1f35e 100644 --- a/src/backend/controllers/auth/AuthController.test.ts +++ b/src/backend/controllers/auth/AuthController.test.ts @@ -5423,6 +5423,35 @@ describe('AuthController user-protected mutations (validation paths)', () => { } }); + it('change-username: 400 when the home path is already occupied, before the rename', async () => { + const { user, actor } = await makeUserAndActor(); + // A name no account holds, whose home path a stray row does. This is + // the shape of legacy drift; it used to leave two roots at one path. + const parked = `p_${uniq()}`; + const root = (await server.stores.fsEntry.getRootEntryForUser( + user.id, + ))!; + await server.clients.db.write( + 'UPDATE fsentries SET path = ? WHERE id = ?', + [`/${parked}`, root.id], + ); + const { user: mover, actor: moverActor } = await makeUserAndActor(); + + await expect( + controller.handleChangeUsername( + makeReq({ new_username: parked }, { actor: moverActor }), + makeRes(), + ), + ).rejects.toMatchObject({ statusCode: 400 }); + + // The username claim is refused whole: the user row keeps its name. + const after = await server.stores.user.getById(mover.id, { + force: true, + }); + expect(after!.username).toBe(mover.username); + void actor; + }); + it('change-email: 400 on missing/invalid email and on a confirmed-account collision', async () => { const { actor } = await makeUserAndActor(); await expect( diff --git a/src/backend/controllers/auth/AuthController.ts b/src/backend/controllers/auth/AuthController.ts index 2acdc7ff8..0d10c5c39 100644 --- a/src/backend/controllers/auth/AuthController.ts +++ b/src/backend/controllers/auth/AuthController.ts @@ -836,6 +836,15 @@ export class AuthController extends PuterController { ); } + // ...and the same against the filesystem: a free username whose home + // path is occupied would provision a second root there, and the two + // trees then resolve interchangeably. + if (await this.stores.fsEntry.findHomePathConflict(body.username)) { + throw new HttpError(400, 'This username is not available.', { + legacyCode: 'bad_request', + }); + } + // Duplicate confirmed-email check. A confirmed account (any // credential type — password OR OIDC) on this email → reject. // @@ -2587,6 +2596,19 @@ export class AuthController extends PuterController { legacyCode: 'username_already_in_use', }); } + // Before the username is written, not after: the rename below is what + // keeps the account's files reachable, and it can't run onto a taken + // path. + if ( + await this.stores.fsEntry.findHomePathConflict( + new_username, + req.actor!.user.id!, + ) + ) { + throw new HttpError(400, 'This username is not available.', { + legacyCode: 'username_already_in_use', + }); + } await this.stores.user.update(req.actor!.user.id!, { username: new_username, @@ -2911,6 +2933,11 @@ export class AuthController extends PuterController { legacyCode: 'username_already_in_use', }); } + if (await this.stores.fsEntry.findHomePathConflict(username, user.id)) { + throw new HttpError(400, 'This username is not available.', { + legacyCode: 'username_already_in_use', + }); + } // Match raw + canonical to catch gmail-alias collisions, and // reject on ANY confirmed account (OIDC accounts have // password=null but are real) — not just password-holders. diff --git a/src/backend/services/fs/FSService.test.ts b/src/backend/services/fs/FSService.test.ts index bd14c2210..02cfc42d2 100644 --- a/src/backend/services/fs/FSService.test.ts +++ b/src/backend/services/fs/FSService.test.ts @@ -2489,6 +2489,65 @@ describe('FSService remove', () => { }); }); +describe('FSService home directory guard', () => { + // A home free to be renamed or moved can be parked on a name another + // account later claims, and from then on both trees resolve at one path — + // whichever row has the lower id answers, and new entries there inherit + // ITS owner. Only `renameUserHome` writes a home's name. + it('refuses to rename a home directory', async () => { + const user = await makeUser(); + const root = (await server.stores.fsEntry.getRootEntryForUser( + user.userId, + ))!; + + const error = await caught(() => + fs.rename(user.userId, root, 'some-other-name'), + ); + + expect(error.statusCode).toBe(403); + expect(error.message).toContain('home directory'); + const unchanged = await server.stores.fsEntry.getRootEntryForUser( + user.userId, + ); + expect(unchanged?.path).toBe(user.home); + }); + + it('refuses to move a home directory into another tree', async () => { + const user = await makeUser(); + const other = await makeUser(); + const root = (await server.stores.fsEntry.getRootEntryForUser( + user.userId, + ))!; + const destination = (await entryAt(other, '/Documents'))!; + + const error = await caught(() => + runWithContext({ actor: user.actor }, () => + fs.move(user.userId, { + source: root, + destinationParent: destination, + }), + ), + ); + + expect(error.statusCode).toBe(403); + const unchanged = await server.stores.fsEntry.getRootEntryForUser( + user.userId, + ); + expect(unchanged?.path).toBe(user.home); + }); + + it('still renames an ordinary directory at the top of a home', async () => { + const user = await makeUser(); + const dir = await fs.mkdir(user.userId, { + path: `${user.home}/notahome`, + }); + + const renamed = await fs.rename(user.userId, dir, 'renamed'); + + expect(renamed.path).toBe(`${user.home}/renamed`); + }); +}); + describe('FSService move', () => { let user: TestUser; beforeAll(async () => { diff --git a/src/backend/services/fs/FSService.ts b/src/backend/services/fs/FSService.ts index 9be272687..b576402f2 100644 --- a/src/backend/services/fs/FSService.ts +++ b/src/backend/services/fs/FSService.ts @@ -3464,6 +3464,7 @@ export class FSService extends PuterService { entry: FSEntry, newName: string, ): Promise { + this.#assertNotUserRoot(entry, 'rename'); await this.#assertCanRename(entry, userId); this.#assertUsableName(newName); if (entry.name === newName) return entry; @@ -3603,6 +3604,25 @@ export class FSService extends PuterService { }); } + /** + * A home's name is the account's username, and ACL authorizes by matching + * the first path segment against it — so a root free to be renamed or moved + * can be parked on a name another account later claims, leaving two trees + * at one path. `renameUserHome` is the only writer of a home's name. + * + * Both parent columns have to be NULL: `parent_uid` alone is NULL on legacy + * rows that only ever carried `parent_id`. + */ + #assertNotUserRoot(entry: FSEntry, verb: string): void { + if (entry.parentUid === null && entry.parentId === null) { + throw new HttpError( + 403, + `Cannot ${verb} a home directory — its name follows the account username.`, + { legacyCode: 'forbidden' }, + ); + } + } + /** * A FILE shared directly with you is renameable with `write` on it — the * name is the file's own, and rename stays in place. A folder's name is @@ -3996,6 +4016,7 @@ export class FSService extends PuterService { }, ): Promise { const { source, destinationParent } = input; + this.#assertNotUserRoot(source, 'move'); // The source only: moving *into* another app's AppData is a write, and // ACL plus the fs:write class already cover that. await this.#assertCrossAppDeleteAllowed(source.path); diff --git a/src/backend/stores/fs/FSEntryStore.test.ts b/src/backend/stores/fs/FSEntryStore.test.ts index 40e399222..9e99ad9db 100644 --- a/src/backend/stores/fs/FSEntryStore.test.ts +++ b/src/backend/stores/fs/FSEntryStore.test.ts @@ -1459,6 +1459,34 @@ describe('FSEntryStore home and prefix rewrites', () => { ).resolves.toBeNull(); }); + it('refuses to heal a home onto a path another user holds', async () => { + const occupant = await makeUser(); + const mover = await makeUser(); + + const conflict = await caught(() => + store.renameUserHome(mover.userId, occupant.username), + ); + expect(conflict.statusCode).toBe(409); + + // The mover's home is untouched — no second row at that path. + const root = await store.getRootEntryForUser(mover.userId); + expect(root?.path).toBe(mover.home); + }); + + it('reports a home path conflict only for a foreign owner', async () => { + const owner = await makeUser(); + + const foreign = await store.findHomePathConflict(owner.username); + expect(foreign?.userId).toBe(owner.userId); + + await expect( + store.findHomePathConflict(owner.username, owner.userId), + ).resolves.toBeNull(); + await expect( + store.findHomePathConflict('nobody-has-this-name'), + ).resolves.toBeNull(); + }); + it('rewrites a path prefix and reports how many rows moved', async () => { const user = await makeUser(); await store.ensureDirectoriesForUser(user.userId, [ diff --git a/src/backend/stores/fs/FSEntryStore.ts b/src/backend/stores/fs/FSEntryStore.ts index 280e78341..3fcf007dc 100644 --- a/src/backend/stores/fs/FSEntryStore.ts +++ b/src/backend/stores/fs/FSEntryStore.ts @@ -2608,7 +2608,8 @@ export class FSEntryStore extends PuterStore { } = {}, ): Promise<{ entries: FSEntry[]; cursor?: string }> { const payload = decodeCursor(options.cursor) as - { v: unknown; id: number; s?: string; o?: string } | undefined; + | { v: unknown; id: number; s?: string; o?: string } + | undefined; const requestedSort = options.sortBy ?? null; const requestedOrder = options.sortOrder ?? null; @@ -3074,6 +3075,32 @@ export class FSEntryStore extends PuterStore { return entry; } + /** + * A row already sitting at `/{username}` that `ownerUserId` doesn't own. + * Two rows at one home path make each account's tree answer for the + * other's, so every point that claims a username checks this first. Reads + * the primary: a racing signup has to see the row just written. + */ + async findHomePathConflict( + username: string, + ownerUserId?: number, + ): Promise { + const path = this.#normalizePath(`/${username}`); + const rows = (await this.clients.db.pread( + `SELECT ${this.#selectFsentriesColumns()} FROM fsentries + WHERE path = ? ORDER BY id ASC`, + [path], + )) as unknown as FSEntryRow[]; + for (const row of rows) { + const entry = this.#mapFSEntryRow(row); + if (ownerUserId !== undefined && entry.userId === ownerUserId) { + continue; + } + return entry; + } + return null; + } + // Heal a user's home tree to `/{username}`: if the root entry's path/name // already match, no-op; otherwise rewrite the root row and cascade the // prefix to descendants. Used by the username change flow AND by the @@ -3091,6 +3118,15 @@ export class FSEntryStore extends PuterStore { return root; } + // Never heal onto a path someone else's row holds — the two would + // resolve interchangeably from then on. + const conflict = await this.findHomePathConflict(newUsername, userId); + if (conflict) { + throw new HttpError(409, `An entry already exists at ${newPath}`, { + legacyCode: 'conflict', + }); + } + const oldPath = root.path; const now = Math.floor(Date.now() / 1000); diff --git a/src/backend/util/userProvisioning.ts b/src/backend/util/userProvisioning.ts index 2c7f1f660..fc33e8917 100644 --- a/src/backend/util/userProvisioning.ts +++ b/src/backend/util/userProvisioning.ts @@ -19,6 +19,7 @@ import { v4 as uuidv4 } from 'uuid'; import type { AbstractDatabaseClient } from '../clients/database/DatabaseClient'; +import { HttpError } from '../core/http/HttpError'; import type { GroupStore } from '../stores/group/GroupStore'; import type { UserRow, UserStore } from '../stores/user/UserStore'; import type { IConfig } from '../types'; @@ -58,6 +59,19 @@ export async function generateDefaultFsentries( // Cheap check vs. a redundant INSERT + UPDATE on retries / re-runs. if (user.trash_uuid) return; + // Callers check the name is free before provisioning, so this only fires + // on drift that predates those checks. A home written on top of another + // row gives both accounts a tree answering to one path. + const occupied = (await db.pread( + 'SELECT id FROM fsentries WHERE path = ? LIMIT 1', + [`/${user.username}`], + )) as Array<{ id: number }>; + if (occupied.length > 0) { + throw new HttpError(400, 'This username is not available.', { + legacyCode: 'username_already_in_use', + }); + } + const home_uuid = uuidv4(); const folderUuids: Record = { Trash: uuidv4(),