Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 54 additions & 0 deletions src/platform/adapters/fs/integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down Expand Up @@ -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");
});
});
});
51 changes: 43 additions & 8 deletions src/platform/adapters/fs/integration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string | symbol, unknown> = {};
const seen = new Set<string | symbol>();

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,
Expand Down Expand Up @@ -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,
Expand Down
39 changes: 39 additions & 0 deletions src/proxy/proxy-access-control.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
35 changes: 34 additions & 1 deletion src/proxy/proxy-access-control.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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(
Expand Down