fix: inspect multi-arch index children in deploy image conformance check

nicktrn · nicktrn · commit 81ec73dc97cc · 2026-07-07T16:33:42.000+01:00
diff --git a/apps/webapp/app/v3/services/verifyDeploymentImage.server.ts b/apps/webapp/app/v3/services/verifyDeploymentImage.server.ts
@@ -25,30 +25,76 @@ export type ImageLookupResult = "found" | "missing" | "unknown" | "nonconformant
 // can emit it when reusing zstd layers from a prior build or the registry cache.
 const UNPULLABLE_LAYER_MEDIA_TYPE = "application/vnd.docker.image.rootfs.diff.tar.zstd";
 
-// Lenient: we only need layer media types. A manifest list / OCI index has no top-level
-// layers[], so it just parses to `layers: undefined` and is treated as conformant.
-const ImageManifestSchema = z.object({
+// Nested indexes are exotic (index -> per-platform image manifests is depth 1); cap the
+// walk so a pathological manifest can't fan out unbounded.
+const MAX_MANIFEST_DEPTH = 3;
+
+// Lenient: we only read what we need. An image manifest carries layers[]; a manifest list
+// / OCI index carries manifests[] (per-platform child pointers). Both optional so either
+// shape parses cleanly.
+const ManifestSchema = z.object({
   layers: z.array(z.object({ mediaType: z.string().optional() })).optional(),
+  manifests: z.array(z.object({ digest: z.string() })).optional(),
 });
 
-function manifestHasUnpullableLayers(imageManifest: string | undefined): boolean {
-  if (!imageManifest) {
-    return false;
+// Inspect one raw manifest: does it directly carry an unpullable layer, and (if it's an
+// index) which child manifests should be walked. Fails open to empty on anything we can't
+// parse, so an unreadable manifest never blocks a deploy.
+export function inspectManifest(rawManifest: string | undefined): {
+  hasUnpullableLayer: boolean;
+  childDigests: string[];
+} {
+  const empty = { hasUnpullableLayer: false, childDigests: [] };
+
+  if (!rawManifest) {
+    return empty;
   }
 
   let json: unknown;
   try {
-    json = JSON.parse(imageManifest);
+    json = JSON.parse(rawManifest);
   } catch {
-    return false; // fail open on a manifest we can't read
+    return empty;
   }
 
-  const parsed = ImageManifestSchema.safeParse(json);
+  const parsed = ManifestSchema.safeParse(json);
   if (!parsed.success) {
-    return false; // fail open
+    return empty;
+  }
+
+  return {
+    hasUnpullableLayer:
+      parsed.data.layers?.some((layer) => layer.mediaType === UNPULLABLE_LAYER_MEDIA_TYPE) ?? false,
+    childDigests: parsed.data.manifests?.map((child) => child.digest) ?? [],
+  };
+}
+
+// Walk a manifest and, for a multi-arch index, its child manifests (fetched by digest).
+// Returns true if any layer anywhere in the tree uses the unpullable media type. A child
+// that can't be fetched (undefined) is treated as conformant - fail open.
+export async function treeHasUnpullableLayer(
+  rawManifest: string | undefined,
+  fetchManifest: (digest: string) => Promise<string | undefined>,
+  depth = 0
+): Promise<boolean> {
+  const { hasUnpullableLayer, childDigests } = inspectManifest(rawManifest);
+
+  if (hasUnpullableLayer) {
+    return true;
+  }
+
+  if (depth >= MAX_MANIFEST_DEPTH) {
+    return false;
+  }
+
+  for (const digest of childDigests) {
+    const childManifest = await fetchManifest(digest);
+    if (await treeHasUnpullableLayer(childManifest, fetchManifest, depth + 1)) {
+      return true;
+    }
   }
 
-  return parsed.data.layers?.some((layer) => layer.mediaType === UNPULLABLE_LAYER_MEDIA_TYPE) ?? false;
+  return false;
 }
 
 /**
@@ -88,13 +134,7 @@ export function parseEcrImageReference(
 export function interpretBatchGetImageResponse(
   response: BatchGetImageCommandOutput
 ): ImageLookupResult {
-  const image = response.images?.[0];
-  if (image) {
-    // Present but built with a runtime-incompatible layer media type - promoting it
-    // would 100%-fail every run at pull time, so treat it as a distinct failure.
-    if (manifestHasUnpullableLayers(image.imageManifest)) {
-      return "nonconformant";
-    }
+  if (response.images && response.images.length > 0) {
     return "found";
   }
 
@@ -125,8 +165,10 @@ const sendBatchGetImage: BatchGetImageSender = async ({
   imageIds,
 }) => {
   const ecr = await createEcrClient({ region, assumeRole });
-  // No acceptedMediaTypes: only the single-manifest types are valid enum values, and
-  // we only care whether the image exists, not its manifest format.
+  // Intentionally no acceptedMediaTypes: ECR returns the manifest as stored - which we
+  // rely on for the layer-media-type check - and omitting it avoids a multi-arch index
+  // being reported as a failure (i.e. misread as missing). BatchGetImage populates
+  // imageManifest by default when the image exists; the check fails open if it's absent.
   return ecr.send(new BatchGetImageCommand({ repositoryName, registryId, imageIds }));
 };
 
@@ -224,12 +266,44 @@ export async function ecrImageExists(
 
   const result = interpretBatchGetImageResponse(response);
 
-  if (result === "nonconformant") {
+  if (result !== "found") {
+    return result;
+  }
+
+  // Image exists - now confirm the runtime can actually pull it. Follow index children by
+  // digest so multi-arch deploys are covered, not just single image manifests.
+  const fetchManifest = async (digest: string): Promise<string | undefined> => {
+    const [fetchError, childResponse] = await tryCatch(
+      _send({
+        region,
+        assumeRole,
+        registryId: accountId,
+        repositoryName: parsed.repositoryName,
+        imageIds: [{ imageDigest: digest }],
+      })
+    );
+
+    if (fetchError) {
+      logger.warn("Could not fetch child manifest for conformance check", {
+        imageReference,
+        digest,
+        error: fetchError.message,
+      });
+      return undefined; // fail open on this child
+    }
+
+    return childResponse.images?.[0]?.imageManifest;
+  };
+
+  const topManifest = response.images?.[0]?.imageManifest;
+
+  if (await treeHasUnpullableLayer(topManifest, fetchManifest)) {
     logger.error("Deployment image has a runtime-incompatible layer media type", {
       imageReference,
       repositoryName: parsed.repositoryName,
     });
+    return "nonconformant";
   }
 
-  return result;
+  return "found";
 }
diff --git a/apps/webapp/test/verifyDeploymentImage.test.ts b/apps/webapp/test/verifyDeploymentImage.test.ts
@@ -2,14 +2,37 @@ import { RepositoryNotFoundException } from "@aws-sdk/client-ecr";
 import { describe, expect, it } from "vitest";
 import {
   ecrImageExists,
+  inspectManifest,
   interpretBatchGetImageResponse,
   parseEcrImageReference,
+  treeHasUnpullableLayer,
 } from "~/v3/services/verifyDeploymentImage.server";
 import { type RegistryConfig } from "~/v3/registryConfig.server";
 
 const ECR_HOST = "123456789012.dkr.ecr.us-east-1.amazonaws.com";
 const ecrConfig: RegistryConfig = { host: ECR_HOST, namespace: "deployments-test" };
 
+const DIGEST_A = `sha256:${"a".repeat(64)}`;
+const DIGEST_B = `sha256:${"b".repeat(64)}`;
+const ZSTD_DOCKER = "application/vnd.docker.image.rootfs.diff.tar.zstd";
+
+const imageManifest = (layerMediaTypes: string[]) =>
+  JSON.stringify({
+    schemaVersion: 2,
+    mediaType: "application/vnd.docker.distribution.manifest.v2+json",
+    layers: layerMediaTypes.map((mediaType) => ({ mediaType, digest: DIGEST_A })),
+  });
+
+const indexManifest = (childDigests: string[]) =>
+  JSON.stringify({
+    schemaVersion: 2,
+    mediaType: "application/vnd.oci.image.index.v1+json",
+    manifests: childDigests.map((digest) => ({
+      digest,
+      mediaType: "application/vnd.oci.image.manifest.v1+json",
+    })),
+  });
+
 describe("parseEcrImageReference", () => {
   it("splits repository and tag for a ref under the configured host", () => {
     const ref = `${ECR_HOST}/deployments-test/proj_abc:20240101.1.prod.a1b2c3d4`;
@@ -59,41 +82,72 @@ describe("interpretBatchGetImageResponse", () => {
     );
     expect(interpretBatchGetImageResponse({} as any)).toBe("unknown");
   });
+});
 
-  const manifestWith = (layerMediaTypes: string[]) =>
-    JSON.stringify({
-      schemaVersion: 2,
-      mediaType: "application/vnd.docker.distribution.manifest.v2+json",
-      layers: layerMediaTypes.map((mediaType) => ({ mediaType, digest: `sha256:${"a".repeat(64)}` })),
-    });
+describe("inspectManifest", () => {
+  it("flags an unpullable zstd layer in an image manifest", () => {
+    const result = inspectManifest(
+      imageManifest(["application/vnd.docker.image.rootfs.diff.tar.gzip", ZSTD_DOCKER])
+    );
+    expect(result.hasUnpullableLayer).toBe(true);
+    expect(result.childDigests).toEqual([]);
+  });
 
-  it("returns nonconformant when any layer is a zstd layer in a Docker manifest", () => {
-    const imageManifest = manifestWith([
-      "application/vnd.docker.image.rootfs.diff.tar.gzip",
-      "application/vnd.docker.image.rootfs.diff.tar.zstd",
-    ]);
-    expect(interpretBatchGetImageResponse({ images: [{ imageManifest }] } as any)).toBe(
-      "nonconformant"
+  it("passes OCI zstd and gzip layers (runtime-supported media types)", () => {
+    const result = inspectManifest(
+      imageManifest([
+        "application/vnd.oci.image.layer.v1.tar+gzip",
+        "application/vnd.oci.image.layer.v1.tar+zstd",
+      ])
     );
+    expect(result.hasUnpullableLayer).toBe(false);
   });
 
-  it("returns found for OCI zstd layers (the runtime-supported media type)", () => {
-    const imageManifest = manifestWith([
-      "application/vnd.oci.image.layer.v1.tar+gzip",
-      "application/vnd.oci.image.layer.v1.tar+zstd",
-    ]);
-    expect(interpretBatchGetImageResponse({ images: [{ imageManifest }] } as any)).toBe("found");
+  it("returns child digests for an index and does not flag it directly", () => {
+    const result = inspectManifest(indexManifest([DIGEST_A, DIGEST_B]));
+    expect(result.hasUnpullableLayer).toBe(false);
+    expect(result.childDigests).toEqual([DIGEST_A, DIGEST_B]);
   });
 
-  it("returns found (does not block) when the manifest is absent, unparseable, or an index", () => {
-    expect(interpretBatchGetImageResponse({ images: [{}] } as any)).toBe("found");
-    expect(interpretBatchGetImageResponse({ images: [{ imageManifest: "not json" }] } as any)).toBe(
-      "found"
+  it("fails open (empty) when the manifest is absent or unparseable", () => {
+    expect(inspectManifest(undefined)).toEqual({ hasUnpullableLayer: false, childDigests: [] });
+    expect(inspectManifest("not json")).toEqual({ hasUnpullableLayer: false, childDigests: [] });
+  });
+});
+
+describe("treeHasUnpullableLayer", () => {
+  const neverFetch = async () => undefined;
+
+  it("detects an unpullable layer in a flat image manifest", async () => {
+    expect(await treeHasUnpullableLayer(imageManifest([ZSTD_DOCKER]), neverFetch)).toBe(true);
+  });
+
+  it("follows an index and detects an unpullable layer in a child", async () => {
+    const children: Record<string, string> = {
+      [DIGEST_A]: imageManifest(["application/vnd.oci.image.layer.v1.tar+gzip"]),
+      [DIGEST_B]: imageManifest([ZSTD_DOCKER]),
+    };
+    const result = await treeHasUnpullableLayer(
+      indexManifest([DIGEST_A, DIGEST_B]),
+      async (digest) => children[digest]
     );
-    const index = JSON.stringify({ schemaVersion: 2, manifests: [{ digest: "sha256:x" }] });
-    expect(interpretBatchGetImageResponse({ images: [{ imageManifest: index }] } as any)).toBe(
-      "found"
+    expect(result).toBe(true);
+  });
+
+  it("returns false for an index whose children are all conformant", async () => {
+    const children: Record<string, string> = {
+      [DIGEST_A]: imageManifest(["application/vnd.oci.image.layer.v1.tar+zstd"]),
+      [DIGEST_B]: imageManifest(["application/vnd.docker.image.rootfs.diff.tar.gzip"]),
+    };
+    const result = await treeHasUnpullableLayer(
+      indexManifest([DIGEST_A, DIGEST_B]),
+      async (digest) => children[digest]
     );
+    expect(result).toBe(false);
+  });
+
+  it("fails open when a child manifest can't be fetched", async () => {
+    expect(await treeHasUnpullableLayer(indexManifest([DIGEST_A]), neverFetch)).toBe(false);
   });
 });
 
@@ -210,4 +264,54 @@ describe("ecrImageExists", () => {
     );
     expect(seen.imageIds).toEqual([{ imageTag: "v1.prod.a1b2c3d4" }]);
   });
+
+  // Resolve a manifest by tag or digest against a fixture map, mimicking BatchGetImage.
+  const sendFrom =
+    (byRef: Record<string, string>) =>
+    async (input: any): Promise<any> => {
+      const id = input.imageIds[0];
+      const manifest = byRef[id.imageDigest ?? id.imageTag];
+      return manifest ? { images: [{ imageManifest: manifest }] } : { images: [{}] };
+    };
+
+  it("returns nonconformant for a single-arch image with an unpullable zstd layer", async () => {
+    const result = await ecrImageExists(
+      {
+        imageReference: `${ECR_HOST}/deployments-test/proj_abc:v1.prod.a1b2c3d4`,
+        registryConfig: ecrConfig,
+      },
+      sendFrom({ "v1.prod.a1b2c3d4": imageManifest([ZSTD_DOCKER]) })
+    );
+    expect(result).toBe("nonconformant");
+  });
+
+  it("returns nonconformant when a multi-arch index has an unpullable child", async () => {
+    const result = await ecrImageExists(
+      {
+        imageReference: `${ECR_HOST}/deployments-test/proj_abc:v1.prod.a1b2c3d4`,
+        registryConfig: ecrConfig,
+      },
+      sendFrom({
+        "v1.prod.a1b2c3d4": indexManifest([DIGEST_A, DIGEST_B]),
+        [DIGEST_A]: imageManifest(["application/vnd.oci.image.layer.v1.tar+gzip"]),
+        [DIGEST_B]: imageManifest([ZSTD_DOCKER]),
+      })
+    );
+    expect(result).toBe("nonconformant");
+  });
+
+  it("returns found when a multi-arch index's children are all conformant", async () => {
+    const result = await ecrImageExists(
+      {
+        imageReference: `${ECR_HOST}/deployments-test/proj_abc:v1.prod.a1b2c3d4`,
+        registryConfig: ecrConfig,
+      },
+      sendFrom({
+        "v1.prod.a1b2c3d4": indexManifest([DIGEST_A, DIGEST_B]),
+        [DIGEST_A]: imageManifest(["application/vnd.oci.image.layer.v1.tar+zstd"]),
+        [DIGEST_B]: imageManifest(["application/vnd.docker.image.rootfs.diff.tar.gzip"]),
+      })
+    );
+    expect(result).toBe("found");
+  });
 });
