diff --git a/src/platform/adapters/fs/integration.test.ts b/src/platform/adapters/fs/integration.test.ts index b857033edd..32632a36b9 100644 --- a/src/platform/adapters/fs/integration.test.ts +++ b/src/platform/adapters/fs/integration.test.ts @@ -16,6 +16,7 @@ import { } from "./integration.ts"; import { denoAdapter } from "../deno.ts"; import { VeryfrontError } from "#veryfront/errors/types.ts"; +import { isProxyWithoutHooks } from "#veryfront/platform/compat/error-introspection.ts"; describe("integration.ts", () => { it("should export enhanceAdapterWithFS function", () => { @@ -313,4 +314,57 @@ describe("integration.ts", () => { ); }); }); + + describe("enhanced adapter shape", () => { + function enhanceWithRemoteFs() { + // The GitHub adapter fetches and schema-validates a repository tree at + // construction, so the mock has to satisfy that shape. + return withMockFetch( + () => + Promise.resolve( + new Response( + JSON.stringify({ sha: "deadbeef", tree: [], truncated: false }), + { status: 200, headers: { "content-type": "application/json" } }, + ), + ), + () => + enhanceAdapterWithFS(denoAdapter, { + fs: { + type: "github", + github: { token: "test-token", owner: "owner", repo: "repo" }, + }, + }), + ); + } + + it("is not a Proxy, because security consumers refuse one outright", async () => { + // A Proxy here failed every hosted render using a remote filesystem with + // "SecureFs runtime adapter cannot be a Proxy". + const enhanced = await enhanceWithRemoteFs(); + assertEquals(enhanced === denoAdapter, false); + assertEquals(isProxyWithoutHooks(enhanced), false); + }); + + it("exposes the remote filesystem as an own data property", async () => { + // SecureFs resolves the filesystem through getOwnPropertyDescriptor. A + // Proxy carrying only a get trap forwarded that to the target and handed + // back the host filesystem, silently serving the wrong source. + const enhanced = await enhanceWithRemoteFs(); + const descriptor = Object.getOwnPropertyDescriptor(enhanced, "fs"); + assertExists(descriptor); + assertEquals("value" in descriptor, true); + assertEquals(typeof descriptor.value, "object"); + assertEquals(descriptor.value === denoAdapter.fs, false); + }); + + it("keeps the rest of the adapter, with methods bound to the original", async () => { + const enhanced = await enhanceWithRemoteFs(); + assertEquals(enhanced.id, denoAdapter.id); + assertEquals(enhanced.name, denoAdapter.name); + assertEquals(enhanced.capabilities, denoAdapter.capabilities); + // `shutdown` lives on the prototype and closes over instance state, so it + // must survive materialization already bound to the source adapter. + assertEquals(typeof enhanced.shutdown, "function"); + }); + }); }); diff --git a/src/platform/adapters/fs/integration.ts b/src/platform/adapters/fs/integration.ts index e1b241ab12..63bab312b3 100644 --- a/src/platform/adapters/fs/integration.ts +++ b/src/platform/adapters/fs/integration.ts @@ -19,6 +19,48 @@ function isLocalFS(config: FSIntegrationConfig): boolean { return !config.fs?.type || config.fs.type === "local"; } +/** + * Materialize an adapter that serves `wrappedFS` as its filesystem. + * + * This deliberately builds a plain object rather than wrapping the adapter in a + * Proxy. Security-sensitive consumers refuse a Proxy adapter outright, because a + * Proxy can intercept the reads they rely on, so a Proxy here made every hosted + * project using a remote filesystem fail its render with + * "SecureFs runtime adapter cannot be a Proxy". + * + * A Proxy was also quietly wrong for those consumers even where it was allowed: + * they resolve the filesystem through `getOwnPropertyDescriptor`, and a Proxy + * with only a `get` trap forwards that to the target, handing back the *host* + * filesystem instead of the remote one. + * + * Adapters are class instances whose methods close over instance state, so + * functions stay bound to the original adapter, exactly as the previous `get` + * trap did. Prototype methods are captured by walking the chain; the runtime + * adapters carry no accessors, so materializing eagerly evaluates nothing that + * a property read would not have. + */ +function materializeAdapterWithFS( + adapter: RuntimeAdapter, + wrappedFS: RuntimeAdapter["fs"], +): RuntimeAdapter { + const enhanced: Record = {}; + const seen = new Set(); + + let current: object | null = adapter; + while (current !== null && current !== Object.prototype) { + for (const key of Reflect.ownKeys(current)) { + if (key === "constructor" || key === "fs" || seen.has(key)) continue; + seen.add(key); + const value = Reflect.get(adapter, key) as unknown; + enhanced[key] = typeof value === "function" ? value.bind(adapter) : value; + } + current = Object.getPrototypeOf(current); + } + + enhanced.fs = wrappedFS; + return enhanced as unknown as RuntimeAdapter; +} + export function enhanceAdapterWithFS( adapter: RuntimeAdapter, config: FSIntegrationConfig, @@ -50,14 +92,7 @@ export function enhanceAdapterWithFS( const fsAdapter = await createFSAdapter(fsAdapterConfig); const wrappedFS = wrapFSAdapter(fsAdapter); - const enhancedAdapter: RuntimeAdapter = new Proxy(adapter, { - get(target, prop, receiver) { - if (prop === "fs") return wrappedFS; - - const value = Reflect.get(target, prop, receiver); - return typeof value === "function" ? value.bind(target) : value; - }, - }); + const enhancedAdapter = materializeAdapterWithFS(adapter, wrappedFS); logger.debug("FSAdapter initialized successfully", { type: fsType, diff --git a/src/proxy/proxy-access-control.test.ts b/src/proxy/proxy-access-control.test.ts index 3adbfa54a4..79867dfc92 100644 --- a/src/proxy/proxy-access-control.test.ts +++ b/src/proxy/proxy-access-control.test.ts @@ -99,6 +99,45 @@ describe("proxy/proxy-access-control", () => { ); }); + it("signs in on the apex the request arrived on", () => { + // Sending a staging visitor to veryfront.com mints a cookie for a domain + // that a veryfront.org host never receives, so the redirect loop cannot + // close and staging previews stay unreachable while signed in. + assertEquals( + buildProxyAuthRedirectUrl(new URL("https://app.preview.veryfront.org/dashboard?a=1")), + "https://veryfront.org/sign-in?from=%2Fdashboard%3Fa%3D1", + ); + // Production-mode deployments keep the default apex, unchanged since #1827. + assertEquals( + buildProxyAuthRedirectUrl( + new URL("https://app.production.veryfront.org/dashboard?a=1"), + ), + "https://veryfront.com/sign-in?from=https%3A%2F%2Fapp.production.veryfront.org%2Fdashboard%3Fa%3D1", + ); + assertEquals( + buildProxyAuthRedirectUrl(new URL("https://veryfront.org/dashboard")), + "https://veryfront.org/sign-in?from=%2Fdashboard", + ); + }); + + it("never takes the sign-in host from an unrecognized request host", () => { + // The apex is chosen from a fixed allowlist, so a forged Host header cannot + // point the sign-in redirect off-platform. + for ( + const hostname of [ + "evil.com", + "veryfront.org.evil.com", + "notveryfront.org", + "app.preview.veryfront.io", + ] + ) { + assertEquals( + buildProxyAuthRedirectUrl(new URL(`https://${hostname}/dashboard`)), + "https://veryfront.com/sign-in?from=%2Fdashboard", + ); + } + }); + it("checks project membership by user id", () => { assertEquals(isProjectMember([{ id: "user-1" }], "user-1"), true); assertEquals(isProjectMember([{ id: "user-1" }], "user-2"), false); diff --git a/src/proxy/proxy-access-control.ts b/src/proxy/proxy-access-control.ts index 5d6c19e262..506263ea26 100644 --- a/src/proxy/proxy-access-control.ts +++ b/src/proxy/proxy-access-control.ts @@ -167,6 +167,38 @@ export async function extractUserIdFromToken( } } +/** + * Apex domains that may host the sign-in page, most specific first. + * + * The result is always one of these constants, never a value taken from the + * request, so a forged Host header cannot redirect a user off-platform. + */ +const SIGN_IN_APEX_DOMAINS = ["veryfront.org", "veryfront.com"] as const; +const DEFAULT_SIGN_IN_APEX = "veryfront.com"; + +/** + * Pick the sign-in host matching the environment the request arrived on. + * + * This used to be hardcoded to production. A staging visitor was therefore sent + * to veryfront.com to sign in, received a cookie scoped to that domain, and + * returned to a veryfront.org host that never receives it, so the redirect loop + * could not close and staging previews were unreachable while signed in. + */ +function resolveSignInApex(hostname: string, isHostedProductionDeployment: boolean): string { + // Production-mode deployments keep the default apex. `*.production.veryfront.org` + // has signed in at veryfront.com since #1827, and which environment owns that + // hostname is not derivable from the code, so it is left alone rather than + // changed on an assumption. Only preview hosts, which the cluster shows split + // cleanly (production serves *.preview.veryfront.com, staging serves + // *.preview.veryfront.org), are routed by apex here. + if (isHostedProductionDeployment) return DEFAULT_SIGN_IN_APEX; + + for (const apex of SIGN_IN_APEX_DOMAINS) { + if (hostname === apex || hostname.endsWith(`.${apex}`)) return apex; + } + return DEFAULT_SIGN_IN_APEX; +} + export function buildProxyAuthRedirectUrl(url: URL): string { const safePath = normalizeProxyOriginFormPath(url.pathname); const returnPath = safePath + url.search; @@ -182,7 +214,8 @@ export function buildProxyAuthRedirectUrl(url: URL): string { ? `https://${url.hostname}${returnPath}` : returnPath; - return `https://veryfront.com/sign-in?from=${encodeURIComponent(returnTarget)}`; + const signInApex = resolveSignInApex(url.hostname, isHostedProductionDeployment); + return `https://${signInApex}/sign-in?from=${encodeURIComponent(returnTarget)}`; } export function isProjectMember(