diff --git a/cli/shared/deployment/deploy-project.test.ts b/cli/shared/deployment/deploy-project.test.ts index f3543f496e..b502cf807b 100644 --- a/cli/shared/deployment/deploy-project.test.ts +++ b/cli/shared/deployment/deploy-project.test.ts @@ -1533,7 +1533,7 @@ describe("release asset manifest", () => { polling, ), Error, - "identifies a different release than the one being deployed", + "identifies a different release than the one requested", ); }); diff --git a/docs/api-reference/veryfront/release-assets.md b/docs/api-reference/veryfront/release-assets.md index baf623c8fa..18557c4740 100644 --- a/docs/api-reference/veryfront/release-assets.md +++ b/docs/api-reference/veryfront/release-assets.md @@ -66,10 +66,10 @@ const url = releaseAssetUrl("a".repeat(64), "js"); | Name | Description | Source | | -------------------------------------------- | -------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------- | -| `clearCachedReleaseAssetManifests` | Clear cached manifest bodies while keeping registered fetchers intact. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/release-assets/manifest-cache.ts#L587) | -| `clearReleaseAssetManifestCache` | Clear the cache and fetcher registry (tests / adapter teardown). | [source](https://github.com/veryfront/veryfront-code/blob/main/src/release-assets/manifest-cache.ts#L597) | +| `clearCachedReleaseAssetManifests` | Clear cached manifest bodies while keeping registered fetchers intact. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/release-assets/manifest-cache.ts#L594) | +| `clearReleaseAssetManifestCache` | Clear the cache and fetcher registry (tests / adapter teardown). | [source](https://github.com/veryfront/veryfront-code/blob/main/src/release-assets/manifest-cache.ts#L604) | | `contentTypeForExtension` | Resolve the content type for an extension, or null if not allowed. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/release-assets/constants.ts#L96) | -| `describeReadyReleaseAssetManifestRejection` | Explain why a ready manifest response was rejected. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/release-assets/manifest-schema.ts#L403) | +| `describeReadyReleaseAssetManifestRejection` | Explain why a ready manifest response was rejected. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/release-assets/manifest-schema.ts#L425) | | `getReadyManifestForRender` | Return a ready manifest for `releaseId` if one is cached, else null. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/release-assets/manifest-cache.ts#L258) | | `getReadyManifestForRenderAsync` | Await a ready manifest for rendering when release-manifest consumption is enabled. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/release-assets/manifest-cache.ts#L359) | | `hasImmutableReleaseAssetDependencies` | True only when manifest dependency entries are safe immutable rewrite targets. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/release-assets/manifest-schema.ts#L305) | @@ -78,9 +78,9 @@ const url = releaseAssetUrl("a".repeat(64), "js"); | `isSafeBoundedText` | Check that an untrusted value is a non-empty, trimmed string within `maxLength` that contains no control characters. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/release-assets/manifest-schema.ts#L102) | | `isValidContentHash` | Validate a content hash is exactly 64 lowercase hex characters. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/release-assets/constants.ts#L114) | | `normalizeManifestModuleKey` | Normalize a logical module path to the manifest's key convention. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/release-assets/html-consumption.ts#L27) | -| `parseReadyReleaseAssetManifestResponse` | Parse an untrusted ready response without executing accessors. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/release-assets/manifest-schema.ts#L359) | -| `parseReleaseAssetManifest` | Parse an untrusted manifest without requiring a registered schema extension. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/release-assets/manifest-schema.ts#L344) | -| `readUntrustedOwnDataProperty` | Read an own data property from an untrusted value without invoking accessors. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/release-assets/manifest-schema.ts#L447) | +| `parseReadyReleaseAssetManifestResponse` | Parse an untrusted ready response without executing accessors. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/release-assets/manifest-schema.ts#L380) | +| `parseReleaseAssetManifest` | Parse an untrusted manifest without requiring a registered schema extension. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/release-assets/manifest-schema.ts#L362) | +| `readUntrustedOwnDataProperty` | Read an own data property from an untrusted value without invoking accessors. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/release-assets/manifest-schema.ts#L480) | | `registerManifestFetcherForRelease` | Register a project-scoped manifest fetcher for the given releaseId. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/release-assets/manifest-cache.ts#L159) | | `releaseAssetUrl` | Map a 64-hex content hash + extension to its public asset URL. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/release-assets/constants.ts#L85) | | `resolveManifestModuleUrl` | Resolve a module URL through the manifest. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/release-assets/html-consumption.ts#L42) | @@ -104,6 +104,7 @@ const url = releaseAssetUrl("a".repeat(64), "js"); | `ReleaseAssetManifestFetchContext` | Cancellation context passed to a release-scoped manifest fetcher. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/release-assets/manifest-cache.ts#L92) | | `ReleaseAssetManifestFetcher` | Fetcher used to retrieve a manifest for a release. Registered per-releaseId by the runtime adapter that owns that release, so the correct project-scoped token is always used. Returns null when the manifest is unavailable. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/release-assets/manifest-cache.ts#L109) | | `ReleaseAssetManifestFetcherCleanup` | Idempotent cleanup for one fetcher registration. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/release-assets/manifest-cache.ts#L117) | +| `ReleaseAssetManifestParseOptions` | Options shared by the dependency-free consumption parsers. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/release-assets/manifest-schema.ts#L339) | | `ReleaseAssetManifestResponse` | Response shape for the GET asset-manifest endpoint. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/release-assets/manifest-schema.ts#L321) | | `ReleaseAssetManifestState` | Manifest lifecycle states (DB-owned; mirrored here for runtime checks). | [source](https://github.com/veryfront/veryfront-code/blob/main/src/release-assets/manifest-schema.ts#L312) | | `ReleaseAssetRouteEntry` | Per-route module and CSS closure. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/release-assets/manifest-schema.ts#L296) | diff --git a/src/release-assets/index.ts b/src/release-assets/index.ts index 56bb21027e..362455472c 100644 --- a/src/release-assets/index.ts +++ b/src/release-assets/index.ts @@ -60,6 +60,7 @@ export { type ReleaseAssetDependencyMode, type ReleaseAssetEntry, type ReleaseAssetManifest, + type ReleaseAssetManifestParseOptions, type ReleaseAssetManifestResponse, type ReleaseAssetManifestState, type ReleaseAssetRouteEntry, diff --git a/src/release-assets/manifest-cache.ts b/src/release-assets/manifest-cache.ts index d27610a08b..e06a978f85 100644 --- a/src/release-assets/manifest-cache.ts +++ b/src/release-assets/manifest-cache.ts @@ -428,7 +428,10 @@ function fetchManifest(releaseId: string): Promise : "invalid"; const manifestState = normalizeManifestState(state); const readyResponse = isUsableManifestState(state) - ? parseReadyReleaseAssetManifestResponse(result, releaseId) + // Runtime reads serve releases published before the v2 move, so they + // must accept the v1 body still in storage. Producer-side callers + // (build executor, CLI deploy wait) deliberately do not. + ? parseReadyReleaseAssetManifestResponse(result, releaseId, { acceptLegacyV1: true }) : null; const manifest = readyResponse?.manifest ?? null; @@ -465,7 +468,11 @@ function fetchManifest(releaseId: string): Promise // framework version skew from a corrupt payload. logger.error("Release manifest is ready upstream but failed validation", { releaseId, - reason: describeReadyReleaseAssetManifestRejection(result, releaseId), + // Same acceptance as the parse above, so the reason describes what + // this caller actually rejected. + reason: describeReadyReleaseAssetManifestRejection(result, releaseId, { + acceptLegacyV1: true, + }), }); } else { markManifestDecision(`fetch_${manifestState}`); diff --git a/src/release-assets/manifest-schema.test.ts b/src/release-assets/manifest-schema.test.ts index 8f36158ceb..9203a7cdf8 100644 --- a/src/release-assets/manifest-schema.test.ts +++ b/src/release-assets/manifest-schema.test.ts @@ -93,9 +93,11 @@ function legacyV1Manifest(): Record { }; } +const LEGACY = { acceptLegacyV1: true } as const; + describe("legacy v1 manifest consumption", () => { it("admits modules from a stored v1 body", () => { - const manifest = parseReleaseAssetManifest(legacyV1Manifest()); + const manifest = parseReleaseAssetManifest(legacyV1Manifest(), LEGACY); assertExists(manifest); assertEquals(manifest.schemaVersion, RELEASE_ASSET_MANIFEST_SCHEMA_VERSION); assertEquals(manifest.modules["pages/index.tsx"]?.contentHash, "a".repeat(64)); @@ -106,14 +108,14 @@ describe("legacy v1 manifest consumption", () => { // v1 CSS carries no `cssPipelineIdentity` and a non-sha256 profile hash. // Synthesizing either would fabricate a cache-correctness key, so the // adapter reports no manifest CSS and the renderer keeps its own pipeline. - const manifest = parseReleaseAssetManifest(legacyV1Manifest()); + const manifest = parseReleaseAssetManifest(legacyV1Manifest(), LEGACY); assertExists(manifest); assertEquals(manifest.css, []); assertEquals(manifest.routes["/"]?.css, []); }); it("reports source dependency mode for a v1 body", () => { - const manifest = parseReleaseAssetManifest(legacyV1Manifest()); + const manifest = parseReleaseAssetManifest(legacyV1Manifest(), LEGACY); assertExists(manifest); assertEquals(manifest.dependencyMode, "source"); }); @@ -123,11 +125,39 @@ describe("legacy v1 manifest consumption", () => { corruptModuleKey.modules = { "../escape.tsx": { contentHash: "a".repeat(64), size: 1, contentType: "text/javascript" }, }; - assertEquals(parseReleaseAssetManifest(corruptModuleKey), null); + assertEquals(parseReleaseAssetManifest(corruptModuleKey, LEGACY), null); const danglingRoute = legacyV1Manifest(); danglingRoute.routes = { "/": { modules: ["pages/missing.tsx"], css: [] } }; - assertEquals(parseReleaseAssetManifest(danglingRoute), null); + assertEquals(parseReleaseAssetManifest(danglingRoute, LEGACY), null); + }); + + it("rejects a __proto__ route key instead of silently dropping it", () => { + // Route keys are untrusted. The adapter accumulates them on a + // null-prototype object so `__proto__` arrives at the validator as an + // ordinary own property and is rejected for not being a canonical route + // path. On a plain `{}` it would hit the prototype setter instead, which + // swallows the key and reshapes the accumulator -- a different route to + // the same rejection, but one that hides which key was at fault. + const hostile = legacyV1Manifest(); + const routes: Record = Object.create(null); + routes["/"] = { modules: ["pages/index.tsx"], css: [] }; + routes["__proto__"] = { modules: ["pages/index.tsx"], css: [] }; + hostile.routes = routes; + + assertEquals(parseReleaseAssetManifest(hostile, LEGACY), null); + // Pollution shows up as a property reachable from an unrelated object, not + // as a changed prototype identity, so probe for the injected value itself. + assertEquals( + ({} as Record).modules, + undefined, + "a route entry leaked onto Object.prototype while parsing", + ); + assertEquals( + ({} as Record).css, + undefined, + "a route entry leaked onto Object.prototype while parsing", + ); }); it("accepts a ready response carrying a v1 body", () => { @@ -139,6 +169,7 @@ describe("legacy v1 manifest consumption", () => { const parsed = parseReadyReleaseAssetManifestResponse( response, "22222222-2222-2222-2222-222222222222", + LEGACY, ); assertExists(parsed); assertEquals(parsed.manifest.modules["pages/index.tsx"]?.size, 1234); @@ -147,6 +178,24 @@ describe("legacy v1 manifest consumption", () => { it("keeps the strict validator v2-only so builds cannot emit v1", () => { assertEquals(getReleaseAssetManifestSchema().safeParse(legacyV1Manifest()).success, false); }); + + it("rejects a v1 body unless the caller opts in", () => { + // The default has to stay strict. Producer-side callers -- the build + // executor verifying what it just emitted, the CLI waiting on a deploy -- + // rely on it to surface a builder/framework skew instead of absorbing it. + assertEquals(parseReleaseAssetManifest(legacyV1Manifest()), null); + assertEquals( + parseReleaseAssetManifest(legacyV1Manifest(), { acceptLegacyV1: false }), + null, + ); + assertEquals( + parseReadyReleaseAssetManifestResponse( + { state: "ready", manifest_version: 1, manifest: legacyV1Manifest() }, + "22222222-2222-2222-2222-222222222222", + ), + null, + ); + }); }); describe("release asset manifest schema", () => { @@ -399,7 +448,7 @@ describe("release asset manifest schema", () => { throw new Error("hostile ownKeys"); }, }); - assertEquals(parseReleaseAssetManifest(hostile), null); + assertEquals(parseReleaseAssetManifest(hostile, LEGACY), null); }); it("rejects accessor-backed input without executing accessors", () => { @@ -470,6 +519,30 @@ describe("describeReadyReleaseAssetManifestRejection", () => { assertStringIncludes(reason, "different framework version"); }); + it("does not call a malformed v1 body a skew for a caller that reads v1", () => { + // A runtime read accepts v1, so a v1 body that still fails is corrupt. + // Reporting skew would send operators to upgrade the builder for something + // an upgrade cannot fix. + const reason = describeReadyReleaseAssetManifestRejection( + { state: "ready", manifest_version: 1, manifest: { schemaVersion: 1, releaseId: "r1" } }, + "r1", + { acceptLegacyV1: true }, + ); + + assertEquals(reason, "the manifest body did not match the expected schema"); + }); + + it("still names a skew for a version no caller reads", () => { + const reason = describeReadyReleaseAssetManifestRejection( + { state: "ready", manifest_version: 1, manifest: { schemaVersion: 3, releaseId: "r1" } }, + "r1", + { acceptLegacyV1: true }, + ); + + assertStringIncludes(reason, "schema version 3"); + assertStringIncludes(reason, `versions 1 and ${RELEASE_ASSET_MANIFEST_SCHEMA_VERSION}`); + }); + it("distinguishes the other rejection paths", () => { assertStringIncludes( describeReadyReleaseAssetManifestRejection("not-an-object", "r1"), diff --git a/src/release-assets/manifest-schema.ts b/src/release-assets/manifest-schema.ts index 061690536b..fcf43decbf 100644 --- a/src/release-assets/manifest-schema.ts +++ b/src/release-assets/manifest-schema.ts @@ -335,15 +335,36 @@ export interface ReadyReleaseAssetManifestResponse { // Dependency-free consumption parser // --------------------------------------------------------------------------- +/** Options shared by the dependency-free consumption parsers. */ +export interface ReleaseAssetManifestParseOptions { + /** + * Accept the v1 body still held for releases published before the v2 move. + * + * Off by default, and deliberately opt-in per call site. + * + * Runtime reads must set it, or every release published before the v2 move + * loses its browser modules. Producer-side callers must not: for the build + * executor verifying what it just emitted, the CLI waiting on a deploy, or a + * locally built bundle, a v1 body means the builder and this framework are + * skewed, and accepting it would hide that skew instead of naming it. + * + * @default false + */ + readonly acceptLegacyV1?: boolean; +} + /** * Parse an untrusted manifest without requiring a registered schema extension. * * The parser is non-throwing, applies explicit work and memory bounds, validates * route references, and returns a detached deeply frozen snapshot. */ -export function parseReleaseAssetManifest(value: unknown): ReleaseAssetManifest | null { +export function parseReleaseAssetManifest( + value: unknown, + options: ReleaseAssetManifestParseOptions = {}, +): ReleaseAssetManifest | null { try { - return parseReleaseAssetManifestImpl(value); + return parseReleaseAssetManifestImpl(value, options.acceptLegacyV1 === true); } catch { return null; } @@ -359,6 +380,7 @@ export function parseReleaseAssetManifest(value: unknown): ReleaseAssetManifest export function parseReadyReleaseAssetManifestResponse( value: unknown, expectedReleaseId: string, + options: ReleaseAssetManifestParseOptions = {}, ): ReadyReleaseAssetManifestResponse | null { try { if (!isSafeBoundedText(expectedReleaseId, MAX_IDENTIFIER_LENGTH)) return null; @@ -366,7 +388,7 @@ export function parseReadyReleaseAssetManifestResponse( const state = readOwnDataProperty(value, "state"); const manifestVersion = readOwnDataProperty(value, "manifest_version"); - const manifest = parseReleaseAssetManifest(readOwnDataProperty(value, "manifest")); + const manifest = parseReleaseAssetManifest(readOwnDataProperty(value, "manifest"), options); if ( state !== "ready" || !isSafeIntegerInRange(manifestVersion, Number.MAX_SAFE_INTEGER) || @@ -403,6 +425,7 @@ export function parseReadyReleaseAssetManifestResponse( export function describeReadyReleaseAssetManifestRejection( value: unknown, expectedReleaseId: string, + options: ReleaseAssetManifestParseOptions = {}, ): string { if (typeof value !== "object" || value === null || Array.isArray(value)) { return "the response envelope was not an object"; @@ -414,22 +437,32 @@ export function describeReadyReleaseAssetManifestRejection( } const body = readUntrustedOwnDataProperty(value, "manifest"); - const manifest = parseReleaseAssetManifest(body); + const manifest = parseReleaseAssetManifest(body, options); if (!manifest) { const schemaVersion = readUntrustedOwnDataProperty(body, "schemaVersion"); + // Which versions count as skew depends on what this caller reads. A + // producer-side caller reads v2 only, so a v1 body there is a genuine + // framework skew. A runtime read also accepts v1, so a v1 body that still + // fails is corrupt -- calling that a skew would send operators to upgrade + // the builder for something an upgrade cannot fix. + const acceptsLegacyV1 = options.acceptLegacyV1 === true; if ( isSafeIntegerInRange(schemaVersion, Number.MAX_SAFE_INTEGER) && - schemaVersion !== RELEASE_ASSET_MANIFEST_SCHEMA_VERSION + schemaVersion !== RELEASE_ASSET_MANIFEST_SCHEMA_VERSION && + !(acceptsLegacyV1 && schemaVersion === LEGACY_V1_SCHEMA_VERSION) ) { + const readable = acceptsLegacyV1 + ? `versions ${LEGACY_V1_SCHEMA_VERSION} and ${RELEASE_ASSET_MANIFEST_SCHEMA_VERSION}` + : `version ${RELEASE_ASSET_MANIFEST_SCHEMA_VERSION}`; return `the release assets declare manifest schema version ${schemaVersion}, but this ` + - `build reads version ${RELEASE_ASSET_MANIFEST_SCHEMA_VERSION}. The assets were built ` + - `by a different framework version than the one running this deploy`; + `build reads ${readable}. The assets were built ` + + `by a different framework version than the one reading them`; } return "the manifest body did not match the expected schema"; } if (manifest.releaseId !== expectedReleaseId) { - return "the manifest identifies a different release than the one being deployed"; + return "the manifest identifies a different release than the one requested"; } if (manifest.manifestVersion !== manifestVersion) { return "the envelope and manifest body disagree on the manifest version"; @@ -463,9 +496,12 @@ export function readUntrustedOwnDataProperty(value: unknown, key: PropertyKey): * written — while reads adapt the old shape and then apply the full v2 * validator to it. The adapter only reshapes; it never validates. */ -function parseReleaseAssetManifestImpl(value: unknown): ReleaseAssetManifest | null { +function parseReleaseAssetManifestImpl( + value: unknown, + acceptLegacyV1: boolean, +): ReleaseAssetManifest | null { const current = parseCurrentManifestBody(value); - if (current) return current; + if (current || !acceptLegacyV1) return current; const adapted = adaptLegacyV1ManifestBody(value); return adapted ? parseCurrentManifestBody(adapted) : null; @@ -516,7 +552,10 @@ function adaptLegacyV1Routes(value: unknown): Record | null { const keys = Object.keys(value); if (keys.length > MAX_ROUTE_ENTRIES) return null; - const routes: Record = {}; + // Null-prototype: route keys are untrusted, and assigning a key of + // `__proto__` onto a plain object hits the prototype setter instead of + // creating an own property. `snapshotRoutes` uses the same guard. + const routes: Record = Object.create(null); for (const key of keys) { const entry = readUntrustedOwnDataProperty(value, key); if (!isPlainRecord(entry)) return null;