diff --git a/doc/embedding-app-browser.md b/doc/embedding-app-browser.md index ac25dc6a1..a2a834b28 100644 --- a/doc/embedding-app-browser.md +++ b/doc/embedding-app-browser.md @@ -27,11 +27,13 @@ alone does not populate the main GUI origin's storage; the main GUI session must already exist. The route's `Content-Security-Policy: frame-ancestors` allows the GUI origin and -the embedding page's origin when it is a subdomain of `config.domain` with the -scheme and port from `config.origin`, and not under one of the user-content -hosting domains (`static_hosting_domain`, `private_app_hosting_domain`, and their -`_alt` variants). For example, `https://puter.com` permits -`https://spreadsheet.puter.com`, and `http://puter.localhost:4100` permits +the embedding page's origin when it is exactly one label under `config.domain` +with the scheme and port from `config.origin`. Subdomains the server answers +itself (`api`, `dav`, `js`, `www`, the native-app hosts, and the host of +`api_base_url`) and the user-content hosting domains (`static_hosting_domain`, +`private_app_hosting_domain`, and their `_alt` variants) are refused. For +example, `https://puter.com` permits `https://spreadsheet.puter.com` but not +`https://api.puter.com`, and `http://puter.localhost:4100` permits `http://spreadsheet.puter.localhost:4100` but not `http://someone.site.puter.localhost:4100`. The embedding page is read from the iframe request's referrer, so the host must not suppress it (the browser default diff --git a/src/backend/controllers/homepage/HomepageController.test.ts b/src/backend/controllers/homepage/HomepageController.test.ts index bd7224299..488a13c19 100644 --- a/src/backend/controllers/homepage/HomepageController.test.ts +++ b/src/backend/controllers/homepage/HomepageController.test.ts @@ -27,6 +27,7 @@ import type { Actor } from '../../core/actor.js'; import { PuterRouter } from '../../core/http/PuterRouter.js'; import { PuterServer } from '../../server.js'; import { setupTestServer } from '../../testUtil.js'; +import { embedFrameAncestors } from './HomepageController.js'; // ── Test harness ──────────────────────────────────────────────────── // @@ -529,3 +530,23 @@ describe('HomepageController GET /sw.js', () => { expect(route?.options.subdomain).toBe(''); }); }); + +describe('embedFrameAncestors', () => { + const config = { + origin: 'https://example.test', + domain: 'example.test', + api_base_url: 'https://backend.example.test', + }; + + it('names a sibling subdomain that embeds the browser', () => { + expect( + embedFrameAncestors(config, 'https://sheets.example.test/doc/1'), + ).toBe("'self' https://sheets.example.test"); + }); + + it('refuses the configured API host even off the api label', () => { + expect( + embedFrameAncestors(config, 'https://backend.example.test/x'), + ).toBe("'self'"); + }); +}); diff --git a/src/backend/controllers/homepage/HomepageController.ts b/src/backend/controllers/homepage/HomepageController.ts index 286cdf271..d994b99cd 100644 --- a/src/backend/controllers/homepage/HomepageController.ts +++ b/src/backend/controllers/homepage/HomepageController.ts @@ -22,6 +22,10 @@ import path from 'node:path'; import { PuterController } from '../types.js'; import { toAppShellView } from '../../util/appShellView.js'; import type { PuterRouter } from '../../core/http/PuterRouter'; +import { + isReservedSubdomain, + subdomainLabelOf, +} from '../../core/http/middleware/hostRedirects.js'; import type { IConfig } from '../../types'; import type { PuterHomepageService, @@ -59,14 +63,17 @@ export function appMetaDescription(text: string): string { /** * `frame-ancestors` for the app browser embed. A wildcard over `*.` - * would also admit user-hosted sites whenever the hosting domains sit under - * it, so the embedding page is named exactly, from the request's referrer. + * would also admit user-hosted sites whenever the hosting domains sit under it, + * so the embedding page is named exactly, from the request's referrer. Only a + * single label under `` qualifies, and not one this server answers + * itself (the API, WebDAV, SDK and native-app hosts). */ export function embedFrameAncestors( config: Pick< IConfig, | 'origin' | 'domain' + | 'api_base_url' | 'static_hosting_domain' | 'static_hosting_domain_alt' | 'private_app_hosting_domain' @@ -82,6 +89,7 @@ export function embedFrameAncestors( return "'self'"; } const host = parent.hostname.toLowerCase(); + const label = subdomainLabelOf(host, config.domain); const hostingDomains = [ config.static_hosting_domain, config.static_hosting_domain_alt, @@ -93,7 +101,9 @@ export function embedFrameAncestors( const allowed = parent.protocol === gui.protocol && parent.port === gui.port && - host.endsWith(`.${config.domain.toLowerCase()}`) && + label !== undefined && + !isReservedSubdomain(label) && + host !== new URL(config.api_base_url).hostname && !hostingDomains.some((d) => host === d || host.endsWith(`.${d}`)); return allowed ? `'self' ${parent.origin}` : "'self'"; } @@ -149,7 +159,8 @@ export class HomepageController extends PuterController { router.get('/embed/apps', {}, (req, res) => { // SDK bootstrap parameters must not change this page's account or API origin. - if (Object.keys(req.query).length) { + // Checked on the raw URL; `req.query` drops pairs past the parser's limit. + if (req.originalUrl.includes('?')) { res.redirect('/embed/apps'); return; } diff --git a/src/backend/core/http/middleware/hostRedirects.ts b/src/backend/core/http/middleware/hostRedirects.ts index 2b532d235..2cad4e0ca 100644 --- a/src/backend/core/http/middleware/hostRedirects.ts +++ b/src/backend/core/http/middleware/hostRedirects.ts @@ -111,6 +111,10 @@ export const subdomainLabelOf = ( return label && !label.includes('.') ? label : undefined; }; +/** Whether core itself answers `.` (`www` redirects to the root). */ +export const isReservedSubdomain = (name: string): boolean => + name === 'www' || RESERVED_SUBDOMAINS.has(name); + /** * Record a claim on `.`. Throws on a malformed or uppercase * label, a subdomain core already handles, or a name that is already claimed. @@ -123,7 +127,7 @@ export const addClaimedSubdomain = ( if (typeof name !== 'string' || !SUBDOMAIN_LABEL.test(name)) { throw new Error(`Invalid subdomain label: ${JSON.stringify(name)}`); } - if (name === 'www' || RESERVED_SUBDOMAINS.has(name)) { + if (isReservedSubdomain(name)) { throw new Error(`Subdomain '${name}' is reserved`); } if (claims.has(name)) { diff --git a/src/backend/server.test.ts b/src/backend/server.test.ts index 469aa2e75..eed087b09 100644 --- a/src/backend/server.test.ts +++ b/src/backend/server.test.ts @@ -367,6 +367,10 @@ describe('PuterServer host header validation', () => { it.each([ ['a user-hosted site', 'http://someone.site.puter.localhost:PORT/'], ['a private app host', 'http://someone.app.puter.localhost:PORT/'], + ['the API host', 'http://api.puter.localhost:PORT/'], + ['the WebDAV host', 'http://dav.puter.localhost:PORT/'], + ['a native app host', 'http://editor.puter.localhost:PORT/'], + ['a nested subdomain', 'http://a.spreadsheet.puter.localhost:PORT/'], ['another site', 'http://evil.test:PORT/'], ['another port', 'http://spreadsheet.puter.localhost:1/'], ['a suppressed referrer', undefined], @@ -379,8 +383,12 @@ describe('PuterServer host header validation', () => { expect(res.headers['content-security-policy']).toBe("frame-ancestors 'self'"); }); - it('strips SDK bootstrap query parameters from the embed route', async () => { - const res = await request('/embed/apps?puter.app_instance_id=other&puter.api_origin=https://other.test', { + it.each([ + ['plain', ''], + // Past the parser's 1000-pair limit, so `req.query` comes back empty. + ['padded', '&'.repeat(1000)], + ])('strips %s SDK bootstrap query parameters from the embed route', async (_label, padding) => { + const res = await request(`/embed/apps?${padding}puter.app_instance_id=other&puter.api_origin=https://other.test`, { host: `puter.localhost:${port}`, }); expect(res.status).toBe(302);