From 6f13b1f4573a55bfb42f6f29eff50bced86ee708 Mon Sep 17 00:00:00 2001 From: Daniel Salazar Date: Mon, 26 Oct 2026 22:05:21 -0700 Subject: [PATCH] fix(apps): compare normalized index URLs before update checks (#4370) --- src/backend/drivers/apps/AppDriver.js | 20 +- .../drivers/apps/AppDriver.legacyUrl.test.ts | 233 ++++++++++++++++++ src/docs/src/Apps/update.md | 2 + 3 files changed, 248 insertions(+), 7 deletions(-) create mode 100644 src/backend/drivers/apps/AppDriver.legacyUrl.test.ts diff --git a/src/backend/drivers/apps/AppDriver.js b/src/backend/drivers/apps/AppDriver.js index 606f22aa5..8c242366c 100644 --- a/src/backend/drivers/apps/AppDriver.js +++ b/src/backend/drivers/apps/AppDriver.js @@ -424,10 +424,10 @@ export class AppDriver extends PuterDriver { existing: app, }); - // Puter-hosted index_url handling on update — same flow as create - // but only when the index_url is actually changing. Self-app is + // Puter-hosted index_url handling on update — same flow as create. + // #validateInput already dropped an unchanged index_url. Self-app is // excluded from the conflict search via `excludeAppId`. - if (fields.index_url && fields.index_url !== app.index_url) { + if (fields.index_url) { await this.#ensurePuterSiteSubdomainIsOwned( fields.index_url, actor.user, @@ -620,13 +620,19 @@ export class AppDriver extends PuterDriver { protocols: WEB_AND_EXTENSION_PROTOCOLS, }); if (out.index_url) out.index_url = new URL(out.index_url).href; - // Only enforce on new/changed values so rows that already - // carry a reserved host (migration-seeded builtins) can still - // have their other fields updated. + // A value equal to the stored one after normalization is + // unchanged: drop it so legacy spellings stay as stored and + // skip the host checks (migration-seeded builtins carry + // reserved hosts). Rewriting it would make a dormant + // mixed-case/default-port row match again without checks. if ( + existing && out.index_url !== undefined && - out.index_url !== existing?.index_url + out.index_url === + (URL.parse(existing.index_url)?.href ?? existing.index_url) ) { + delete out.index_url; + } else if (out.index_url !== undefined) { this.#assertIndexUrlHostAllowed(out.index_url); } } diff --git a/src/backend/drivers/apps/AppDriver.legacyUrl.test.ts b/src/backend/drivers/apps/AppDriver.legacyUrl.test.ts new file mode 100644 index 000000000..8988b97f9 --- /dev/null +++ b/src/backend/drivers/apps/AppDriver.legacyUrl.test.ts @@ -0,0 +1,233 @@ +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { v4 as uuidv4 } from 'uuid'; +import { makeActor } from '../../core/actor.js'; +import { runWithContext } from '../../core/context.js'; +import { PuterServer } from '../../server.js'; +import { setupTestServer } from '../../testUtil.js'; +import type { IConfig } from '../../types.js'; + +let server: PuterServer; + +beforeAll(async () => { + server = await setupTestServer({ + domain: 'puter.localhost', + static_hosting_domain: 'site.puter.localhost', + protocol: 'http', + } as unknown as IConfig); +}); + +afterAll(async () => { + await server?.shutdown(); +}); + +type AppsDriver = { + update: (args: { + id: { name: string }; + object: Record; + }) => Promise>; +}; + +describe('saving app settings with an unchanged legacy index_url', () => { + it('allows saving an unchanged URL after its site is gone', async () => { + const username = `li-${Math.random().toString(36).slice(2, 10)}`; + const user = await server.stores.user.create({ + username, + uuid: uuidv4(), + password: null, + email: `${username}@test.local`, + free_storage: 100 * 1024 * 1024, + requires_email_confirmation: false, + } as Parameters[0]); + const actor = makeActor({ + user: { ...user, email_confirmed: true } as never, + }); + const sub = `li-${Math.random().toString(36).slice(2, 8)}`; + const legacyUrl = `http://${sub}.site.puter.localhost`; + const name = `li-app-${Math.random().toString(36).slice(2, 8)}`; + // Row as Dev Center wrote it before normalization: no trailing slash. + await server.clients.db.write( + 'INSERT INTO `apps` (`uid`, `name`, `title`, `index_url`, `owner_user_id`) VALUES (?, ?, ?, ?, ?)', + [`app-${uuidv4()}`, name, 'Old title', legacyUrl, user.id], + ); + const apps = server.drivers.apps as unknown as AppsDriver; + + // The site it pointed at has since been deleted. + expect(await server.stores.subdomain.getBySubdomain(sub)).toBeFalsy(); + + // Title-only save works. + await runWithContext({ actor }, () => + apps.update({ id: { name }, object: { title: 'Title only' } }), + ); + + // Dev Center's save sends the stored index_url back as it was. + await expect( + runWithContext({ actor }, () => + apps.update({ + id: { name }, + object: { title: 'New title', index_url: legacyUrl }, + }), + ), + ).resolves.toMatchObject({ title: 'New title' }); + await expect( + runWithContext({ actor }, () => + apps.update({ + id: { name }, + object: { + index_url: `http://different-${sub}.site.puter.localhost/`, + }, + }), + ), + ).rejects.toMatchObject({ + statusCode: 400, + legacyCode: 'subdomain_not_owned', + }); + }); + + it('normalizes a stored reserved host before comparing unchanged values', async () => { + const user = await server.stores.user.create({ + username: `legacy-${uuidv4().slice(0, 8)}`, + uuid: uuidv4(), + password: null, + }); + const actor = makeActor({ user }); + const name = `legacy-${uuidv4()}`; + const indexUrl = 'https://builtins.namespaces.puter.com'; + await server.clients.db.write( + 'INSERT INTO `apps` (`uid`, `name`, `title`, `index_url`, `owner_user_id`) VALUES (?, ?, ?, ?, ?)', + [`app-${uuidv4()}`, name, 'Builtin', indexUrl, user.id], + ); + const apps = server.drivers.apps as unknown as AppsDriver; + await expect( + runWithContext({ actor }, () => + apps.update({ + id: { name }, + object: { index_url: indexUrl, title: 'Saved' }, + }), + ), + ).resolves.toMatchObject({ title: 'Saved' }); + await expect( + runWithContext({ actor }, () => + apps.update({ + id: { name }, + object: { index_url: `${indexUrl}/different` }, + }), + ), + ).rejects.toMatchObject({ statusCode: 400 }); + }); + + it('leaves a legacy mixed-case row as stored when re-sent normalized', async () => { + const mkUser = async () => { + const username = `lc-${Math.random().toString(36).slice(2, 10)}`; + const user = await server.stores.user.create({ + username, + uuid: uuidv4(), + password: null, + email: `${username}@test.local`, + free_storage: 100 * 1024 * 1024, + requires_email_confirmation: false, + } as Parameters[0]); + return { + user, + actor: makeActor({ + user: { ...user, email_confirmed: true } as never, + }), + }; + }; + const owner = await mkUser(); + const siteOwner = await mkUser(); + const sub = `lc${Math.random().toString(36).slice(2, 8)}`; + const otherSub = `lc${Math.random().toString(36).slice(2, 8)}`; + for (const s of [sub, otherSub]) { + const home = await server.services.fs.mkdir(siteOwner.user.id, { + path: `/${siteOwner.user.username}/${s}`, + createMissingParents: true, + }); + await server.stores.subdomain.create({ + userId: siteOwner.user.id, + subdomain: s, + rootDirId: home.id, + }); + } + // Pre-normalization row pointing at a site now owned by someone else. + const legacyUrl = `http://${sub.toUpperCase()}.site.puter.localhost`; + const name = `lc-app-${uuidv4().slice(0, 8)}`; + await server.clients.db.write( + 'INSERT INTO `apps` (`uid`, `name`, `title`, `index_url`, `owner_user_id`) VALUES (?, ?, ?, ?, ?)', + [`app-${uuidv4()}`, name, 'Old', legacyUrl, owner.user.id], + ); + const apps = server.drivers.apps as unknown as AppsDriver; + const readStored = async () => { + const [row] = await server.clients.db.read( + 'SELECT index_url, title FROM apps WHERE name = ?', + [name], + ); + return row; + }; + + await runWithContext({ actor: owner.actor }, () => + apps.update({ id: { name }, object: { title: 'Title only' } }), + ); + expect(await readStored()).toMatchObject({ + index_url: legacyUrl, + title: 'Title only', + }); + + await runWithContext({ actor: owner.actor }, () => + apps.update({ + id: { name }, + object: { + title: 'Resent', + index_url: `http://${sub}.site.puter.localhost/`, + }, + }), + ); + expect(await readStored()).toMatchObject({ + index_url: legacyUrl, + title: 'Resent', + }); + + await expect( + runWithContext({ actor: owner.actor }, () => + apps.update({ + id: { name }, + object: { + index_url: `http://${otherSub}.site.puter.localhost/`, + }, + }), + ), + ).rejects.toMatchObject({ + statusCode: 400, + legacyCode: 'subdomain_not_owned', + }); + expect((await readStored()).index_url).toBe(legacyUrl); + }); + + it('leaves a legacy default-port reserved host as stored', async () => { + const user = await server.stores.user.create({ + username: `legacy-${uuidv4().slice(0, 8)}`, + uuid: uuidv4(), + password: null, + }); + const actor = makeActor({ user }); + const name = `legacy-${uuidv4()}`; + const indexUrl = 'https://PUTER.COM:443'; + await server.clients.db.write( + 'INSERT INTO `apps` (`uid`, `name`, `title`, `index_url`, `owner_user_id`) VALUES (?, ?, ?, ?, ?)', + [`app-${uuidv4()}`, name, 'Builtin', indexUrl, user.id], + ); + const apps = server.drivers.apps as unknown as AppsDriver; + await expect( + runWithContext({ actor }, () => + apps.update({ + id: { name }, + object: { index_url: 'https://puter.com/', title: 'Saved' }, + }), + ), + ).resolves.toMatchObject({ title: 'Saved' }); + const [row] = await server.clients.db.read( + 'SELECT index_url FROM apps WHERE name = ?', + [name], + ); + expect(row.index_url).toBe(indexUrl); + }); +}); diff --git a/src/docs/src/Apps/update.md b/src/docs/src/Apps/update.md index 9c574c47a..c8abf84f1 100755 --- a/src/docs/src/Apps/update.md +++ b/src/docs/src/Apps/update.md @@ -28,6 +28,8 @@ An object containing the attributes to update. The object can contain the follow - `filetypeAssociations` (optional): An array of strings representing the filetypes that the app can open. Defaults to `[]`. File extentions and MIME types are supported; For example, `[".txt", ".md", "application/pdf"]` would allow the app to open `.txt`, `.md`, and PDF files. At most 200 entries, each at most 60 characters. - `metadata` (optional): An object containing custom metadata for the app. This can be used to store arbitrary key-value pairs associated with the app. At most 16 KiB when serialized as JSON. +Changing `indexURL` can merge the app into an existing app for that hosted origin. The returned app can have a different UID. The deleted source app's KV data and KV shares are removed. Data already belonging to the surviving app is retained. + ## Return value A `Promise` that will resolve to the [`App`](/Objects/app/) object that was updated.