Skip to content

fix(security): derive CSP origins on release-backed content #3487

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kojiwakayama merged 2 commits into from
Aug 8, 2026
Merged

fix(security): derive CSP origins on release-backed content #3487

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
30cefed
fix(security): derive CSP origins on release-backed content
kojiwakayama Aug 8, 2026
20fa4b3
fix(security): do not make derivation depend on a cache write
kojiwakayama Aug 8, 2026
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
45 changes: 39 additions & 6 deletions src/platform/adapters/fs/veryfront/adapter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,11 @@ export class VeryfrontFSAdapter implements FSAdapter {
/** Resolves when file list initialization is complete (for coordinating reads) */
private fileListReadyResolve: (() => void) | null = null;
/** Single-flight background rewarm when the file list cache disappears */
private fileListWarmupPromise: Promise<void> | null = null;
// Resolves with the files it fetched, so a caller that waited does not have
// to depend on the cache write having succeeded -- writes are skipped
// entirely when caching is disabled, and can fail on a backend cache.
private fileListWarmupPromise: Promise<Array<{ path: string; content?: string }> | null> | null =
null;
private fileListWarmupKey: string | null = null;
/** Single-flight foreground refresh when a branch preview read misses a newly pushed file. */
private branchMissRecoveryPromise: Promise<void> | null = null;
Expand Down Expand Up @@ -663,7 +667,7 @@ export class VeryfrontFSAdapter implements FSAdapter {
}

const warmupContext = this.contentContext;
let warmupPromise: Promise<void> | null = null;
let warmupPromise: Promise<Array<{ path: string; content?: string }> | null> | null = null;
warmupPromise = (async () => {
try {
const existing = await this.cache.getAsync<Array<{ path: string; content?: string }>>(
Expand All @@ -676,7 +680,7 @@ export class VeryfrontFSAdapter implements FSAdapter {
cacheKey: effectiveCacheKey,
fileCount: existing.length,
});
return;
return existing;
}

logger.debug("Starting file list warmup", {
Expand Down Expand Up @@ -704,12 +708,16 @@ export class VeryfrontFSAdapter implements FSAdapter {
totalFiles: files.length,
filesWithContent: files.filter((file) => file.content).length,
});

return files;
} catch (error) {
logger.warn("File list warmup failed", {
reason,
cacheKey: effectiveCacheKey,
error: error instanceof Error ? error.message : String(error),
});

return null;
Comment on lines +711 to +720

@coderabbitai coderabbitai Bot Aug 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Preserve fetched files when the cache write fails.

If this.cache.setAsync rejects on Line 696, this catch returns null after a successful fetch. getAllSourceFiles then cannot use the fetched files and can only retry the failed cache read. CSP derivation remains empty in the cache-write-failure case that this change must support.

Catch and log cache-write failures separately. Continue with files. Add a focused test with a rejecting cache write.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/platform/adapters/fs/veryfront/adapter.ts` around lines 711 - 720, Update
getAllSourceFiles so failures from this.cache.setAsync are caught and logged
separately while preserving and returning the successfully fetched files. Keep
the existing warmup/read-failure handling that returns null, and add a focused
test covering a rejecting cache write and verifying the fetched files are still
returned.

} finally {
if (warmupPromise && this.fileListWarmupPromise === warmupPromise) {
this.fileListWarmupPromise = null;
Expand All @@ -720,7 +728,8 @@ export class VeryfrontFSAdapter implements FSAdapter {

this.fileListWarmupPromise = warmupPromise;
this.fileListWarmupKey = effectiveCacheKey;
this.readOps.setFileListReadyPromise(warmupPromise);
// That collaborator only needs completion, not the payload.
this.readOps.setFileListReadyPromise(warmupPromise.then(() => {}));
}

private markSourceSnapshotChanged(
Expand Down Expand Up @@ -1040,7 +1049,16 @@ export class VeryfrontFSAdapter implements FSAdapter {
return this.projectData;
}

async getAllSourceFiles(): Promise<Array<{ path: string; content?: string }>> {
/**
* @param options.waitForWarmup wait for an in-flight file-list fetch instead
* of answering empty. Off by default: most callers can proceed without the
* list and must not pay for the fetch, but a caller that has no other way to
* obtain it -- CSP derivation on a release-backed context, where nothing else
* populates the cache -- would otherwise read empty on every request forever.
*/
async getAllSourceFiles(
options: { waitForWarmup?: boolean } = {},
): Promise<Array<{ path: string; content?: string }>> {
if (!this.contentContext) {
logger.debug("getAllSourceFiles called without contentContext", {
initialized: this.initialized,
Expand All @@ -1055,7 +1073,22 @@ export class VeryfrontFSAdapter implements FSAdapter {
"getAllSourceFiles miss",
);
const cacheKey = cached?.cacheKey;
const files = cached?.files;
let files = cached?.files;

// A miss schedules a warmup and returns immediately, which is right for
// callers that can proceed without the list. This one cannot: nothing else
// populates it for a release-backed context, so returning early meant the
// list was empty on every request for the life of the process. Wait for the
// fetch this read just started, then look again.
if (options.waitForWarmup && cacheKey && !files?.length && this.fileListWarmupPromise) {
// Take what the fetch returned rather than re-reading the cache: with
// caching disabled, or a failed backend write, the cache keeps nothing
// and correctness would depend on a write that never happened.
const fetched = await this.fileListWarmupPromise;
files = fetched?.length
? fetched
: await this.cache.getAsync<{ path: string; content?: string }[]>(cacheKey);
}
Comment on lines +1076 to +1091

@coderabbitai coderabbitai Bot Aug 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Bind the waited warmup to cacheKey.

cacheKey is captured before this block, but this.fileListWarmupPromise is a mutable singleton. If another content context starts a warmup before Line 1087, this call can await that other warmup and return its files for the original cache key. This can derive CSP origins from the wrong source snapshot.

Store warmups by cache key, or return a key-tagged warmup handle from scheduling and consume it only when its key matches cacheKey. Add an interleaved context-switch test.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/platform/adapters/fs/veryfront/adapter.ts` around lines 1076 - 1091, Bind
the warmup awaited in the file-list read path to the captured cacheKey instead
of using the mutable singleton this.fileListWarmupPromise. Update warmup
scheduling and consumption around the relevant file-list methods to store
per-key promises or use a key-tagged handle, and only apply fetched files when
the key matches; otherwise read the requested key’s cache. Add a test covering
an interleaved context switch and verifying CSP origins use the original source
snapshot.


if (!cacheKey || !files?.length) {
logger.debug("getAllSourceFiles cache miss or empty", {
Expand Down
6 changes: 4 additions & 2 deletions src/platform/adapters/fs/veryfront/multi-project-adapter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,10 +316,12 @@ export class MultiProjectFSAdapter implements FSAdapter {
}
}

async getAllSourceFiles(): Promise<Array<{ path: string; content?: string }>> {
async getAllSourceFiles(
options: { waitForWarmup?: boolean } = {},
): Promise<Array<{ path: string; content?: string }>> {
try {
const adapter = await this.getAdapter();
const files = (await adapter.getAllSourceFiles?.()) ?? [];
const files = (await adapter.getAllSourceFiles?.(options)) ?? [];

if (files.length === 0) {
logger.debug("getAllSourceFiles returned empty", {
Expand Down
46 changes: 45 additions & 1 deletion src/server/runtime-handler/derive-project-csp.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ function createHostedAdapter(options: { requireInitialization: boolean }) {
return Promise.resolve(SOURCE);
},
getContentContext: () => null,
getSourceSnapshotVersion: () => 7,
// Async, like MultiProjectFSAdapter's.
getSourceSnapshotVersion: () => Promise.resolve(7),
};

const fs = {
Expand Down Expand Up @@ -93,6 +94,49 @@ describe("server/runtime-handler/deriveProjectCspOrigins", () => {
assertEquals(derived?.["img-src"], ["https://images.unsplash.com"]);
});

it("re-derives when the snapshot moves under a fixed release", async () => {
// The wrapper's `getSourceSnapshotVersion` is async, and template-stringifying
// it wrote the literal "[object Promise]" into every key. Two releases would
// still differ by their id prefix, so only a moving snapshot under one fixed
// identity can see this: with the promise stringified, both calls share a key
// and the second is served from cache instead of re-reading the source.
__clearDerivedCspCacheForTests();

let snapshot = 1;
let reads = 0;
let source = SOURCE;
const underlying = {
ensureSourceSnapshotFresh: () => Promise.resolve(),
getAllSourceFiles: () => {
reads += 1;
return Promise.resolve(source);
},
getContentContext: () => null,
getSourceSnapshotVersion: () => Promise.resolve(snapshot),
};
const adapter = {
fs: {
isVeryfrontAdapter: true,
isMultiProjectMode: true,
getUnderlyingAdapter: () => underlying,
ensureSourceSnapshotFresh: () => Promise.resolve(),
runWithContext: (_s: string, _t: string, run: () => Promise<unknown>) => run(),
},
} as unknown as RuntimeAdapter;

const first = await deriveProjectCspOrigins({ ...PRODUCTION, adapter });
assertEquals(first?.["img-src"], ["https://images.unsplash.com"]);
assertEquals(reads, 1);

// Same release, new content pushed under it.
snapshot = 2;
source = [{ path: "pages/index.tsx", content: '<img src="https://cdn.example.com/a.png" />' }];

const second = await deriveProjectCspOrigins({ ...PRODUCTION, adapter });
assertEquals(reads, 2, "a moved snapshot must not be served from the previous key");
assertEquals(second?.["img-src"], ["https://cdn.example.com"]);
});

it("returns nothing rather than throwing when the adapter cannot host a tenant", async () => {
__clearDerivedCspCacheForTests();
const derived = await deriveProjectCspOrigins({
Expand Down
15 changes: 11 additions & 4 deletions src/server/runtime-handler/project-runtime-context.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -595,9 +595,11 @@ export async function deriveProjectCspOrigins(args: {

const underlying = typeof fs.getUnderlyingAdapter === "function"
? fs.getUnderlyingAdapter() as {
getAllSourceFiles?: () => Promise<Array<{ path: string; content?: string }>>;
getAllSourceFiles?: (
options?: { waitForWarmup?: boolean },
) => Promise<Array<{ path: string; content?: string }>>;
getContentContext?: () => ResolvedContentContext | null;
getSourceSnapshotVersion?: () => number;
getSourceSnapshotVersion?: () => number | Promise<number | undefined>;
ensureSourceSnapshotFresh?: (reason?: string) => Promise<void>;
}
: undefined;
Expand All @@ -617,8 +619,11 @@ export async function deriveProjectCspOrigins(args: {
// under them changes, so the adapter's snapshot generation is what actually
// moves when a preview is pushed to. Without it a preview would serve a
// derivation from before the push until the entry is evicted.
// Awaited: the multi-project wrapper's version of this is async, and
// template-stringifying the promise put the literal "[object Promise]" in
// every key, collapsing all snapshots to one value.
const snapshot = typeof underlying.getSourceSnapshotVersion === "function"
? underlying.getSourceSnapshotVersion()
? await underlying.getSourceSnapshotVersion()
: 0;
const contentVersion = `${
resolveStyleContentVersion(underlying.getContentContext?.() ?? null, {
Expand All @@ -631,7 +636,9 @@ export async function deriveProjectCspOrigins(args: {
return await getDerivedCspOrigins({
projectScope: args.projectSlug,
contentVersion,
loadSourceFiles: () => underlying.getAllSourceFiles!(),
// Nothing else populates the file list for a release-backed context, so
// this read must wait for the fetch rather than answer empty forever.
loadSourceFiles: () => underlying.getAllSourceFiles!({ waitForWarmup: true }),
});
};

Expand Down