fix(webapp): fail closed on image verification + address review

Author: nicktrn

apps/webapp/app/v3/services/finalizeDeploymentV2.server.ts

@@ -169,6 +169,11 @@ export class FinalizeDeploymentV2Service extends BaseService {
 
     const registryConfig = getRegistryConfig(deployment.type === "MANAGED");
 
+    // ECR-only: non-ECR (self-hosted) registries can't be checked this way, so skip.
+    if (!isEcrRegistry(registryConfig.host)) {
+      return;
+    }
+
     const result = await ecrImageExists({
       imageReference: deployment.imageReference,
       imageDigest: body.imageDigest,
@@ -181,9 +186,13 @@ export class FinalizeDeploymentV2Service extends BaseService {
       );
     }
 
-    // "unknown" (non-ECR registry, unparseable ref, or an API/permission error)
-    // is logged inside ecrImageExists - proceed, since a verifier failure must
-    // not become a deploy outage.
+    // Fail closed: if we can't confirm the image is present, don't promote a version
+    // that might not start. Set DEPLOY_IMAGE_VERIFICATION_ENABLED=0 for out-of-band pushes.
+    if (result === "unknown") {
+      throw new ServiceValidationError(
+        "Could not verify the deployment image exists in the registry. Aborting the deploy."
+      );
+    }
   }
 
   async #createTemplateIfNeeded(

apps/webapp/app/v3/services/verifyDeploymentImage.server.ts

@@ -1,4 +1,8 @@
-import { BatchGetImageCommand, type BatchGetImageCommandOutput } from "@aws-sdk/client-ecr";
+import {
+  BatchGetImageCommand,
+  type BatchGetImageCommandOutput,
+  RepositoryNotFoundException,
+} from "@aws-sdk/client-ecr";
 import { tryCatch } from "@trigger.dev/core";
 import { logger } from "~/services/logger.server";
 import {
@@ -81,28 +85,17 @@ const sendBatchGetImage: BatchGetImageSender = async ({
   imageIds,
 }) => {
   const ecr = await createEcrClient({ region, assumeRole });
-  return ecr.send(
-    new BatchGetImageCommand({
-      repositoryName,
-      registryId,
-      imageIds,
-      // We only care whether the manifest exists, not its contents.
-      acceptedMediaTypes: [
-        "application/vnd.docker.distribution.manifest.v2+json",
-        "application/vnd.oci.image.manifest.v1+json",
-        "application/vnd.oci.image.index.v1+json",
-        "application/vnd.docker.distribution.manifest.list.v2+json",
-      ],
-    })
-  );
+  // No acceptedMediaTypes: only the single-manifest types are valid enum values, and
+  // we only care whether the image exists, not its manifest format.
+  return ecr.send(new BatchGetImageCommand({ repositoryName, registryId, imageIds }));
 };
 
 /**
  * Pre-promotion backstop: check the deployment image actually exists in ECR.
  *
- * Returns "unknown" for non-ECR registries or any error we can't read as a
- * definitive miss - callers treat "unknown" as "proceed", so a verifier failure
- * never becomes a deploy outage. `_send` is a test seam.
+ * "found"/"missing" are definitive (a nonexistent repo counts as missing).
+ * "unknown" means we couldn't determine it - non-ECR registry, unparseable ref, or
+ * an API error; the caller decides what to do with each. `_send` is a test seam.
  */
 export async function ecrImageExists(
   {
@@ -152,6 +145,11 @@ export async function ecrImageExists(
   );
 
   if (error) {
+    // A missing repo is a definitive miss, not an ambiguous error.
+    if (error instanceof RepositoryNotFoundException) {
+      return "missing";
+    }
+
     logger.error("Failed to verify deployment image in ECR", {
       imageReference,
       repositoryName: parsed.repositoryName,

apps/webapp/test/verifyDeploymentImage.test.ts

@@ -1,3 +1,4 @@
+import { RepositoryNotFoundException } from "@aws-sdk/client-ecr";
 import { describe, expect, it } from "vitest";
 import {
   ecrImageExists,
@@ -77,6 +78,22 @@ describe("ecrImageExists", () => {
     expect(called).toBe(false);
   });
 
+  it("returns unknown for an unparseable ECR ref without calling the registry", async () => {
+    let called = false;
+    const result = await ecrImageExists(
+      {
+        imageReference: `${ECR_HOST}/deployments-test/proj_abc`,
+        registryConfig: ecrConfig,
+      },
+      async () => {
+        called = true;
+        return {} as any;
+      }
+    );
+    expect(result).toBe("unknown");
+    expect(called).toBe(false);
+  });
+
   it("returns found when the image exists", async () => {
     const result = await ecrImageExists(
       {
@@ -99,7 +116,7 @@ describe("ecrImageExists", () => {
     expect(result).toBe("missing");
   });
 
-  it("returns unknown (fails open) when the registry call throws", async () => {
+  it("returns unknown when the registry call throws an ambiguous error", async () => {
     const result = await ecrImageExists(
       {
         imageReference: `${ECR_HOST}/deployments-test/proj_abc:v1.prod.a1b2c3d4`,
@@ -112,6 +129,19 @@ describe("ecrImageExists", () => {
     expect(result).toBe("unknown");
   });
 
+  it("returns missing when the repository does not exist", async () => {
+    const result = await ecrImageExists(
+      {
+        imageReference: `${ECR_HOST}/deployments-test/proj_abc:v1.prod.a1b2c3d4`,
+        registryConfig: ecrConfig,
+      },
+      async () => {
+        throw new RepositoryNotFoundException({ message: "not found", $metadata: {} });
+      }
+    );
+    expect(result).toBe("missing");
+  });
+
   it("queries by digest when a valid digest is supplied", async () => {
     const digest = `sha256:${"b".repeat(64)}`;
     let seen: any;