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
38 changes: 37 additions & 1 deletion src/security/http/derived-csp-cache.test.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,49 @@
import "#veryfront/schemas/_test-setup.ts";
import { afterEach, describe, it } from "#veryfront/testing/bdd.ts";
import { assert, assertEquals } from "#veryfront/testing/assert.ts";
import { __clearDerivedCspCacheForTests, getDerivedCspOrigins } from "./derived-csp-cache.ts";
import {
__clearDerivedCspCacheForTests,
getDerivedCspOrigins,
shouldWarnOnceForKey,
} from "./derived-csp-cache.ts";

const IMG = `<img src="https://cdn.example.com/a.png" />`;

afterEach(() => __clearDerivedCspCacheForTests());

describe("security/http/derived-csp-cache", () => {
it("reports an underivable content version once, not once per request", async () => {
// The failure paths deliberately do not cache, so the read is retried every
// request. Without a guard the diagnostic would be emitted every request
// too, on every pod, for as long as the failure lasts -- turning a
// one-line-per-release signal into log flooding.
const lookup = {
projectScope: "proj",
contentVersion: "release:persistent-failure",
loadSourceFiles: () => Promise.resolve([]),
};

for (let i = 0; i < 5; i += 1) await getDerivedCspOrigins(lookup);

// The guard is what the log is gated on, so ask it directly: after those
// calls the key must already be spent.
const key = `${lookup.projectScope}\u0000${lookup.contentVersion}`;
assertEquals(shouldWarnOnceForKey(key), false, "the key was reported during the calls above");
});

it("reports each content version separately", async () => {
assertEquals(shouldWarnOnceForKey("proj\u0000a"), true);
assertEquals(shouldWarnOnceForKey("proj\u0000a"), false, "same key stays spent");
assertEquals(shouldWarnOnceForKey("proj\u0000b"), true, "a new release is still worth a line");
});

it("bounds the set of reported keys", () => {
for (let i = 0; i < 260; i += 1) shouldWarnOnceForKey(`proj\u0000bounded-${i}`);
// The earliest key was evicted, so it would be reported again rather than
// being remembered for the life of the pod.
assertEquals(shouldWarnOnceForKey("proj\u0000bounded-0"), true);
});

it("retries after a source read that came back empty", async () => {
// `getAllSourceFiles` returns [] while its own file list is cold and warms
// it asynchronously. Remembering that emptiness pinned a release to the
Expand Down
73 changes: 61 additions & 12 deletions src/security/http/derived-csp-cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,30 @@ registerCache("derived-csp-origins", () => ({
maxEntries: MAX_ENTRIES,
}));

/**
* Content versions already reported as underivable.
*
* The two failure paths deliberately do not `remember`, so the read is retried
* on the next request -- which means without this they would warn on every
* request for as long as the failure lasts, on every pod. The diagnostic is
* worth one line per content version, not one per request.
*
* Bounded like the cache, and for the same reason.
*/
const warned = new Set<string>();

/** @returns whether this key's diagnostic has not been emitted yet */
export function shouldWarnOnceForKey(key: string): boolean {
if (warned.has(key)) return false;
while (warned.size >= MAX_ENTRIES) {
const oldest = warned.values().next().value as string | undefined;
if (oldest === undefined) break;
warned.delete(oldest);
}
warned.add(key);
return true;
}

function remember(key: string, value: DerivedCspOrigins): DerivedCspOrigins {
// Insertion-ordered eviction: the oldest content version is the one least
// likely to still be serving traffic.
Expand Down Expand Up @@ -104,10 +128,17 @@ async function deriveOnce(
try {
files = await lookup.loadSourceFiles();
} catch (error) {
logger.debug("Could not read sources for CSP derivation", {
projectScope: lookup.projectScope,
error: error instanceof Error ? error.message : String(error),
});
// Warn, not debug. Every path out of this function is a silent `EMPTY`, so
// a derivation that never works looks exactly like a project that
// references no external origins. That is how this shipped doing nothing
// in production while reading as healthy.
if (shouldWarnOnceForKey(key)) {
logger.warn("CSP derivation could not read project sources", {
projectScope: lookup.projectScope,
contentVersion: lookup.contentVersion,
error: error instanceof Error ? error.message : String(error),
});
}
// Deliberately not remembered. See below.
return EMPTY;
}
Expand All @@ -126,22 +157,40 @@ async function deriveOnce(
// So distinguish the two cases: files read and no origins found is immutable
// for the content version and worth caching, while nothing read is a race and
// must be retried.
if (!files || files.length === 0) return EMPTY;
if (!files || files.length === 0) {
if (shouldWarnOnceForKey(key)) {
logger.warn("CSP derivation read no project sources", {
projectScope: lookup.projectScope,
contentVersion: lookup.contentVersion,
});
}
return EMPTY;
}

const derived = deriveCspOriginsFromSource(files);
const count = derived["img-src"]?.length ?? 0;
if (count > 0) {
logger.debug("Derived CSP origins from project source", {
projectScope: lookup.projectScope,
contentVersion: lookup.contentVersion,
originCount: count,
});
}

// Logged once per content version, because that is exactly what the cache key
// is, so this cannot grow with traffic. Both outcomes are recorded: "read 40
// files, derived 0 origins" is the shape of a broken derivation, and it is
// indistinguishable from a correct one unless the file count is stated.
logger.info("Derived CSP origins from project source", {
projectScope: lookup.projectScope,
contentVersion: lookup.contentVersion,
fileCount: files.length,
filesWithContent: files.filter((file) =>
typeof file.content === "string" && file.content !== ""
)
.length,
originCount: count,
});

return remember(key, derived);
}

/** @internal Test seam. */
export function __clearDerivedCspCacheForTests(): void {
cache.clear();
inFlight.clear();
warned.clear();
}
27 changes: 23 additions & 4 deletions src/server/runtime-handler/project-runtime-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -562,7 +562,15 @@ async function deriveProjectCspOrigins(args: {
branch: string | null | undefined;
environmentName: string | undefined;
}): Promise<SecurityConfig["derivedCsp"]> {
if (!isExtendedFSAdapter(args.adapter.fs) || !args.adapter.fs.runWithContext) return undefined;
// Each early return below is indistinguishable, from the served header, from
// a project that references no external origins. Naming which one fired is
// the difference between diagnosing this from logs and needing a live probe.
if (!isExtendedFSAdapter(args.adapter.fs) || !args.adapter.fs.runWithContext) {
logger.warn("CSP derivation skipped: adapter cannot run in a tenant context", {
projectSlug: args.projectSlug,
});
return undefined;
}
const fs = args.adapter.fs;

const run = async (): Promise<SecurityConfig["derivedCsp"]> => {
Expand All @@ -573,7 +581,13 @@ async function deriveProjectCspOrigins(args: {
getSourceSnapshotVersion?: () => number;
}
: undefined;
if (!underlying || typeof underlying.getAllSourceFiles !== "function") return undefined;
if (!underlying || typeof underlying.getAllSourceFiles !== "function") {
logger.warn("CSP derivation skipped: adapter exposes no source listing", {
projectSlug: args.projectSlug,
hasUnderlying: Boolean(underlying),
});
return undefined;
}

// Branch and environment content versions are stable while the content
// under them changes, so the adapter's snapshot generation is what actually
Expand Down Expand Up @@ -610,8 +624,13 @@ async function deriveProjectCspOrigins(args: {
environmentName: args.environmentName ?? null,
},
) as SecurityConfig["derivedCsp"];
} catch {
// Never fail a response over a CSP nicety.
} catch (error) {
// Never fail a response over a CSP nicety, but do not swallow it either.
logger.warn("CSP derivation failed", {
projectSlug: args.projectSlug,
releaseId: args.releaseId,
error: error instanceof Error ? error.message : String(error),
});
return undefined;
}
}