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
4 changes: 4 additions & 0 deletions src/server/runtime-handler/adapter-factory.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1069,6 +1069,9 @@ describe("adapter-factory", () => {

// Defaults, not the caller's config.
assertEquals(result.config, undefined);
// Downstream substitutes the process-wide security config for an absent
// project config, so the reason it is absent has to survive the return.
assertEquals(result.configOutcome, "hosted-absent");
});

it("uses defaults when the 404 arrives wrapped rather than at the top level", async () => {
Expand Down Expand Up @@ -1108,6 +1111,7 @@ describe("adapter-factory", () => {
});

assertEquals(result.config, undefined);
assertEquals(result.configOutcome, "hosted-absent");
});

it("still fails when a 404 comes from something other than the config read", async () => {
Expand Down
48 changes: 47 additions & 1 deletion src/server/runtime-handler/adapter-factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,40 @@ const baseLogger = getBaseLogger("SERVER");

const logger = baseLogger.component("adapter-factory");

/**
* Which path produced `config`, so a caller that degrades on a missing config
* can say why it is missing.
*
* `config` being `undefined` is reached from four unrelated places -- an
* inherited caller config, a deliberate defer, a published project that has no
* config file, and a hosted 404 -- and the result alone cannot tell them
* apart. Downstream, `resolveProjectRuntimeContext` silently substitutes the
* process-wide security config for an absent project config, which serves a
* correct-looking 200 carrying the platform-default CSP instead of the
* project's. That degradation was undiagnosable from production logs because
* every branch that reaches it logs at debug.
*/
export type ConfigResolutionOutcome =
/** No project-specific load ran; whatever the caller passed through stands. */
| "inherited"
/** Loaded from a local project directory. */
| "local"
/** Deliberately skipped: see `shouldDeferConfigLoad`. */
| "deferred"
/** Loaded from the control plane for this project. */
| "hosted"
/** The control plane answered 404: the project publishes no config file. */
| "hosted-absent";

interface AdapterResolutionResult {
/** The effective project directory to use */
projectDir: string;
/** The adapter to use for this request */
adapter: RuntimeAdapter;
/** The config for this project */
config: VeryfrontConfig | undefined;
/** Which branch produced `config`. */
configOutcome: ConfigResolutionOutcome;
/** Whether this is a local project (filesystem-first) */
isLocalProject: boolean;
}
Expand Down Expand Up @@ -163,6 +190,7 @@ export async function resolveAdapter(
let effectiveProjectDir = opts.projectDir;
let effectiveAdapter = opts.adapter;
let effectiveConfig = opts.config;
let configOutcome: ConfigResolutionOutcome = "inherited";

// Check if this is a local project.
// In proxy mode, skip local discovery unless there's an explicit header path override —
Expand Down Expand Up @@ -208,6 +236,7 @@ export async function resolveAdapter(

if (shouldDeferConfigLoad(opts)) {
effectiveConfig = undefined;
configOutcome = "deferred";
} else if (opts.isProxyMode) {
const hosted = await prepareProxyConfigLoad(opts, true);
effectiveConfig = await timeAsync(
Expand All @@ -218,11 +247,13 @@ export async function resolveAdapter(
signal: opts.req.signal,
}),
);
configOutcome = "hosted";
} else {
effectiveConfig = await timeAsync(
"config:load-project",
() => getConfig(effectiveProjectDir, effectiveAdapter),
);
configOutcome = "local";

logger.debug("Loaded project-specific config", {
projectSlug: opts.projectSlug,
Expand All @@ -243,6 +274,7 @@ export async function resolveAdapter(
projectDir: effectiveProjectDir,
adapter: effectiveAdapter,
config: effectiveConfig,
configOutcome: "deferred",
isLocalProject,
};
}
Expand Down Expand Up @@ -290,6 +322,8 @@ export async function resolveAdapter(
return loadCurrentConfig();
});

configOutcome = "hosted";

logger.debug("Loaded config in proxy mode", {
projectSlug: opts.projectSlug,
hasConfig: !!effectiveConfig,
Expand All @@ -306,10 +340,21 @@ export async function resolveAdapter(
// Defaults, not whatever a caller happened to pass in: a project with no
// published config must not silently inherit another config's routes.
effectiveConfig = undefined;
logger.debug("No hosted config for this release; using defaults", {
configOutcome = "hosted-absent";
// Warn, not debug. For a project that publishes no config at all this
// is routine, but it is indistinguishable here from a project whose
// config momentarily 404s -- and the two produce the same silently
// degraded response downstream (platform-default security headers in
// place of the project's). At debug it was invisible in production
// while a preview served the wrong CSP on a third of its renders.
logger.warn("No hosted config for this release; using defaults", {
projectSlug: opts.projectSlug,
projectId: opts.projectId,
releaseId: opts.releaseId,
proxyEnv: opts.proxyEnv,
branch: opts.branch ?? null,
environmentName: opts.environmentName ?? null,
pathname: opts.pathname ?? null,
});
} else {
// Log at error level — this is a real failure that will affect rendering.
Expand All @@ -333,6 +378,7 @@ export async function resolveAdapter(
projectDir: effectiveProjectDir,
adapter: effectiveAdapter,
config: effectiveConfig,
configOutcome,
isLocalProject,
};
}
73 changes: 73 additions & 0 deletions src/server/runtime-handler/project-runtime-context.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1323,6 +1323,9 @@ describe("resolveProjectRuntimeContext", () => {
});

it("keeps exact-source control-plane config undefined at the runtime-context boundary", async () => {
__resetLoggerConfigForTests();
const entries: LogEntry[] = [];
__registerLogRecordEmitter((entry) => entries.push(entry));
let outerContextCalls = 0;
const adapter = createExtendedMockAdapter({
onRunWithContext: () => {
Expand Down Expand Up @@ -1359,6 +1362,76 @@ describe("resolveProjectRuntimeContext", () => {
assertEquals(outerContextCalls, 0);
assertEquals(result.adapter.config, undefined);
assertEquals(result.handlerContext?.config, undefined);
// A config-less control-plane request is the intended shape, so it must
// stay distinguishable from a project whose config failed to resolve.
assertEquals(result.adapter.configOutcome, "deferred");
// And it must stay silent. The exclusion is the whole reason the outcome
// is threaded through: without it this path would warn on every
// control-plane request and drown the signal it exists to carry.
assertEquals(
entries.filter((entry) => entry.message.includes("serving platform-default security headers"))
.length,
0,
);
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.

it("records the security fallback when a proxied request resolves no project config", async () => {
// No proxy token, so no project-specific config load runs at all and the
// caller's (absent) config stands. The response still gets security
// headers -- from the process-wide config rather than the project's.
const adapter = createExtendedMockAdapter();
const req = new Request("http://localhost/page", {
headers: {
"x-project-slug": "proxy-project",
"x-project-id": "proj-proxy",
},
});
const url = new URL(req.url);
const securityConfig = { allowedOrigins: ["*"] };
__resetLoggerConfigForTests();
const entries: LogEntry[] = [];
__registerLogRecordEmitter((entry) => entries.push(entry));

const result = await resolveProjectRuntimeContext(makeRuntimeContextInput({
req,
url,
adapter,
config: undefined,
securityConfig,
headers: extractRequestHeaders(req, url),
requestContext: createRequestContext(req),
isProxyMode: true,
projectIdentity: {
projectSlug: "proxy-project",
projectId: "proj-proxy",
releaseId: "rel-proxy",
environmentName: "Preview",
proxyEnv: "preview",
parsedDomain: defaultParsedDomain,
},
}));

assertEquals(result.adapter.config, undefined);
// The outcome names the branch, which is what makes the accompanying warn
// actionable: several unrelated paths leave `config` undefined and are
// otherwise indistinguishable at the point of the fallback.
assertEquals(result.adapter.configOutcome, "inherited");
// And the degradation this records: the request falls back to the
// process-wide security config, so the response carries platform-default
// headers in place of the project's policy.
assertStrictEquals(result.handlerContext?.securityConfig, securityConfig);

// The whole point of the change: this is visible above debug level, and
// carries the branch that produced the absent config.
const warning = entries.find((entry) =>
entry.level === "warn" &&
entry.message.includes("serving platform-default security headers")
);
assertExists(warning);
assertEquals(
(warning.context as Record<string, unknown> | undefined)?.configOutcome,
"inherited",
);
});

it("rejects proxy config load failures at the runtime-context boundary", async () => {
Expand Down
29 changes: 29 additions & 0 deletions src/server/runtime-handler/project-runtime-context.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { getBaseLogger } from "#veryfront/utils";
import { getHostEnv } from "#veryfront/platform/compat/process.ts";
import type { VeryfrontConfig } from "#veryfront/config";
import { prepareDeclarativeConfigContext } from "#veryfront/config/declarative-evaluator.ts";
Expand All @@ -17,6 +18,8 @@ import { buildHandlerContext } from "./handler-context-builder.ts";
import { extractRequestHeaders, resolveProject } from "./project-resolution.ts";
import { shouldSkipEnrichedContext } from "./request-utils.ts";

const logger = getBaseLogger("SERVER").component("project-runtime-context");

type ProxyTrustVerifier = (req: Request) => Promise<boolean>;

export interface PrepareProjectRequestInput {
Expand Down Expand Up @@ -322,6 +325,32 @@ export async function resolveProjectRuntimeContext(
})
: undefined;

// Falling back here substitutes the process-wide security config for the
// project's, which serves a 200 whose CSP is the platform floor rather than
// the project's policy. The response looks correct, so nothing downstream
// can notice. Record it where the substitution happens, with the branch that
// produced the absent config, so an intermittent config-resolution failure
// is legible from logs instead of only from diffing served headers.
//
// `deferred` is excluded: those control-plane endpoints authenticate a
// signed operation envelope and expose no browser surface, so a config-less
// security context is their intended shape, not a degradation.
if (
input.isProxyMode && requestSecurity === undefined &&
adapterRes.configOutcome !== "deferred"
) {
logger.warn("No project config for this request; serving platform-default security headers", {
projectSlug: projectRes.projectSlug,
projectId: projectRes.projectId,
configOutcome: adapterRes.configOutcome,
releaseId: envRes.releaseId ?? null,
branch: reqCtx.branch ?? null,
environmentName: projectRes.environmentName ?? null,
resolvedEnvironment: envRes.resolvedEnvironment ?? null,
pathname: input.url.pathname,
Comment on lines +342 to +350

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Remove raw project and request metadata from both warnings.

Both warning paths log tenant identifiers and request-derived values. These values can contain customer data or private infrastructure details. Use redacted values or an approved correlation identifier.

  • src/server/runtime-handler/project-runtime-context.ts#L342-L350: remove or redact project, release, branch, environment, and pathname fields.
  • src/server/runtime-handler/adapter-factory.ts#L350-L357: remove or redact project, release, branch, environment, and pathname fields.
📍 Affects 2 files
  • src/server/runtime-handler/project-runtime-context.ts#L342-L350 (this comment)
  • src/server/runtime-handler/adapter-factory.ts#L350-L357
🤖 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/server/runtime-handler/project-runtime-context.ts` around lines 342 -
350, Remove or redact the raw project and request metadata from both warning
calls: the warning in src/server/runtime-handler/project-runtime-context.ts
lines 342-350 and the corresponding warning in
src/server/runtime-handler/adapter-factory.ts lines 350-357. Eliminate project,
release, branch, environment, and pathname fields, or replace them with an
approved correlation identifier while preserving the warnings themselves.

Source: Coding guidelines

});
}

const handlerContext = buildHandlerContext({
projectDir: adapterRes.projectDir,
adapter: adapterRes.adapter,
Expand Down