mirror of
https://github.com/HeyPuter/puter.git
synced 2026-11-14 09:50:49 +00:00
fix(embed): only sibling subdomains not served by core can frame the app browser (#4515)
/embed/apps now names the referring origin in frame-ancestors only when it is exactly one label under the domain and that label is not one the server answers itself (api, dav, js, www, native-app hosts) or the host of api_base_url. Nested subdomains are refused too. The query-string redirect now checks the raw URL instead of the parsed query, so every request carrying a query string is redirected.
This commit is contained in:
1 parent
9d22f39928
commit
7f0cd1689b
5 files changed
+58
-12
No files matched your search
@@ -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
|
||||
|
||||
@@ -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'");
|
||||
});
|
||||
});
|
||||
@@ -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 `*.<domain>`
|
||||
* 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 `<domain>` 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;
|
||||
}
|
||||
|
||||
@@ -111,6 +111,10 @@ export const subdomainLabelOf = (
|
||||
return label && !label.includes('.') ? label : undefined;
|
||||
};
|
||||
|
||||
/** Whether core itself answers `<name>.<domain>` (`www` redirects to the root). */
|
||||
export const isReservedSubdomain = (name: string): boolean =>
|
||||
name === 'www' || RESERVED_SUBDOMAINS.has(name);
|
||||
|
||||
/**
|
||||
* Record a claim on `<name>.<domain>`. 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)) {
|
||||
|
||||
@@ -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);
|
||||
|
||||
Reference in new issue
Block a user