Handle invalid remote contract schemas

.changeset/graceful-remote-contracts.md

@@ -0,0 +1,5 @@
+---
+'@shopify/app': patch
+---
+
+Skip invalid remote contract schemas without aborting app commands.

packages/app/src/cli/services/generate/fetch-extension-specifications.test.ts

@@ -1,8 +1,66 @@
 import {fetchSpecifications} from './fetch-extension-specifications.js'
 import {testDeveloperPlatformClient, testOrganizationApp} from '../../models/app/app.test-data.js'
-import {describe, expect, test} from 'vitest'
+import {RemoteSpecification} from '../../api/graphql/extension_specifications.js'
+import {afterEach, describe, expect, test} from 'vitest'
+import {mockAndCaptureOutput} from '@shopify/cli-kit/node/testing/output'
 
 describe('fetchExtensionSpecifications', () => {
+  afterEach(() => {
+    mockAndCaptureOutput().clear()
+  })
+
+  test('skips a remote-only specification with an invalid contract schema', async () => {
+    // Given
+    const invalidRemoteSpecification: RemoteSpecification = {
+      name: 'Invalid remote extension',
+      externalName: 'Invalid Remote Extension',
+      identifier: 'invalid_remote_extension',
+      externalIdentifier: 'invalid_remote_extension',
+      gated: false,
+      experience: 'extension',
+      managementExperience: 'cli',
+      registrationLimit: 1,
+      uidStrategy: 'uuid',
+      validationSchema: {
+        jsonSchema: '{"type":"object","properties":{"name":{"$ref":"#/definitions/missing"}}}',
+      },
+    }
+    const validRemoteSpecification: RemoteSpecification = {
+      name: 'Valid remote extension',
+      externalName: 'Valid Remote Extension',
+      identifier: 'valid_remote_extension',
+      externalIdentifier: 'valid_remote_extension',
+      gated: false,
+      experience: 'extension',
+      managementExperience: 'cli',
+      registrationLimit: 1,
+      uidStrategy: 'uuid',
+      validationSchema: {
+        jsonSchema: '{"type":"object","properties":{"name":{"type":"string"}}}',
+      },
+    }
+    const outputMock = mockAndCaptureOutput()
+    const developerPlatformClient = testDeveloperPlatformClient({
+      specifications: () => Promise.resolve([invalidRemoteSpecification, validRemoteSpecification]),
+    })
+
+    // When
+    const specifications = await fetchSpecifications({
+      developerPlatformClient,
+      app: testOrganizationApp(),
+    })
+
+    // Then
+    expect(specifications).not.toContainEqual(
+      expect.objectContaining({identifier: invalidRemoteSpecification.identifier}),
+    )
+    expect(specifications).toContainEqual(expect.objectContaining({identifier: validRemoteSpecification.identifier}))
+    expect(outputMock.warn()).toContain(
+      `Remote contract validation for "${invalidRemoteSpecification.identifier}" is skipped`,
+    )
+    expect(outputMock.warn()).toContain('Server-side validation remains the authority.')
+  })
+
   test('returns the filtered and mapped results including theme', async () => {
     // Given/When
     const got = await fetchSpecifications({

packages/app/src/cli/services/generate/fetch-extension-specifications.ts

@@ -7,7 +7,7 @@ import {
 } from '../../models/extensions/specification.js'
 import {DeveloperPlatformClient} from '../../utilities/developer-platform-client.js'
 import {MinimalAppIdentifiers} from '../../models/organization.js'
-import {unifiedConfigurationParserFactory} from '../../utilities/json-schema.js'
+import {unifiedConfigurationParserFactory, warnRemoteContractValidationSkipped} from '../../utilities/json-schema.js'
 import {getArrayRejectingUndefined} from '@shopify/cli-kit/common/array'
 import {outputDebug} from '@shopify/cli-kit/node/output'
 import {normaliseJsonSchema} from '@shopify/cli-kit/node/json-schema'
@@ -137,8 +137,16 @@ function mergeLocalAndRemoteSpec(
 async function createRemoteOnlySpecification(
   remoteSpec: RemoteSpecification,
   validationSchema: {jsonSchema: string},
-): Promise<RemoteAwareExtensionSpecification> {
-  const normalisedSchema = await normaliseJsonSchema(validationSchema.jsonSchema)
+): Promise<RemoteAwareExtensionSpecification | undefined> {
+  let normalisedSchema
+  try {
+    normalisedSchema = await normaliseJsonSchema(validationSchema.jsonSchema)
+    // eslint-disable-next-line no-catch-all/no-catch-all
+  } catch {
+    warnRemoteContractValidationSkipped(remoteSpec.identifier)
+    return undefined
+  }
+
   const hasLocalization = normalisedSchema.properties?.localization !== undefined
   const localSpec = createContractBasedModuleSpecification({
     identifier: remoteSpec.identifier,

packages/app/src/cli/utilities/json-schema.test.ts

@@ -1,6 +1,7 @@
 import {unifiedConfigurationParserFactory} from './json-schema.js'
-import {describe, test, expect} from 'vitest'
+import {afterEach, describe, test, expect} from 'vitest'
 import {randomUUID} from '@shopify/cli-kit/node/crypto'
+import {mockAndCaptureOutput} from '@shopify/cli-kit/node/testing/output'
 
 describe('unifiedConfigurationParserFactory', () => {
   const mockParseConfigurationObject = (config: any) => {
@@ -18,6 +19,10 @@ describe('unifiedConfigurationParserFactory', () => {
     }
   }
 
+  afterEach(() => {
+    mockAndCaptureOutput().clear()
+  })
+
   test('falls back to zod parser when no JSON schema is provided', async () => {
     // Given
     const merged = {
@@ -60,6 +65,81 @@ describe('unifiedConfigurationParserFactory', () => {
     })
   })
 
+  test('falls back to zod parser when JSON schema has an invalid reference', async () => {
+    // Given
+    const merged = {
+      identifier: randomUUID(),
+      parseConfigurationObject: mockParseConfigurationObject,
+      validationSchema: {
+        jsonSchema: '{"type":"object","properties":{"name":{"$ref":"#/definitions/missing"}}}',
+      },
+    }
+    const outputMock = mockAndCaptureOutput()
+
+    // When
+    const parser = await unifiedConfigurationParserFactory(merged as any, merged.validationSchema)
+    const result = parser({type: 'product_subscription'})
+
+    // Then
+    expect(result).toEqual({
+      state: 'ok',
+      data: {type: 'product_subscription'},
+      errors: undefined,
+    })
+    expect(outputMock.warn()).toContain(`Remote contract validation for "${merged.identifier}" is skipped`)
+    expect(outputMock.warn()).toContain('Server-side validation remains the authority.')
+  })
+
+  test('falls back to zod parser when JSON schema is invalid', async () => {
+    // Given
+    const merged = {
+      identifier: randomUUID(),
+      parseConfigurationObject: mockParseConfigurationObject,
+      validationSchema: {
+        jsonSchema: '{',
+      },
+    }
+    const outputMock = mockAndCaptureOutput()
+
+    // When
+    const parser = await unifiedConfigurationParserFactory(merged as any, merged.validationSchema)
+    const result = parser({type: 'product_subscription'})
+
+    // Then
+    expect(result).toEqual({
+      state: 'ok',
+      data: {type: 'product_subscription'},
+      errors: undefined,
+    })
+    expect(outputMock.warn()).toContain(`Remote contract validation for "${merged.identifier}" is skipped`)
+    expect(outputMock.warn()).toContain('Server-side validation remains the authority.')
+  })
+
+  test('falls back to zod parser when JSON schema fails AJV compilation', async () => {
+    // Given
+    const merged = {
+      identifier: randomUUID(),
+      parseConfigurationObject: mockParseConfigurationObject,
+      validationSchema: {
+        jsonSchema: '{"type":"object","properties":{"name":{"type":"not-a-real-type"}}}',
+      },
+    }
+    const outputMock = mockAndCaptureOutput()
+    const config = {type: 'product_subscription', localOnly: 'preserved'}
+
+    // When
+    const parser = await unifiedConfigurationParserFactory(merged as any, merged.validationSchema)
+    const firstResult = parser(config)
+    const secondResult = parser(config)
+
+    // Then
+    expect(firstResult).toEqual({state: 'ok', data: config, errors: undefined})
+    expect(secondResult).toEqual({state: 'ok', data: config, errors: undefined})
+    expect(outputMock.warn()).toBe(
+      `Remote contract validation for "${merged.identifier}" is skipped because its schema is invalid. Server-side validation remains the authority.`,
+    )
+  })
+
   test('validates with both zod and JSON schema when both succeed', async () => {
     // Given
     const merged = {

packages/app/src/cli/utilities/json-schema.ts

@@ -8,6 +8,7 @@ import {
 } from '@shopify/cli-kit/node/json-schema'
 import {isEmpty} from '@shopify/cli-kit/common/object'
 import {JsonMapType} from '@shopify/cli-kit/node/toml'
+import {outputWarn} from '@shopify/cli-kit/node/output'
 
 /**
  * The base properties that are added to all JSON Schema contracts.
@@ -36,27 +37,49 @@ export async function unifiedConfigurationParserFactory(
   handleInvalidAdditionalProperties: HandleInvalidAdditionalProperties = 'strip',
 ) {
   const contractJsonSchema = validationSchema?.jsonSchema
-  if (contractJsonSchema === undefined || isEmpty(JSON.parse(contractJsonSchema))) {
+  if (contractJsonSchema === undefined) {
     return merged.parseConfigurationObject
   }
-  const contract = await normaliseJsonSchema(contractJsonSchema)
+
+  let contract
+  try {
+    if (isEmpty(JSON.parse(contractJsonSchema))) {
+      return merged.parseConfigurationObject
+    }
+    contract = await normaliseJsonSchema(contractJsonSchema)
+    // eslint-disable-next-line no-catch-all/no-catch-all
+  } catch {
+    warnRemoteContractValidationSkipped(merged.identifier)
+    return merged.parseConfigurationObject
+  }
+
   contract.properties = {...JsonSchemaBaseProperties, ...contract.properties}
   const extensionIdentifier = merged.identifier
+  let remoteContractValidationEnabled = true
 
   const parseConfigurationObject = (config: object): ParseConfigurationResult<BaseConfigType> => {
     // First we parse with zod. This may also change the format of the data.
     const zodParse = merged.parseConfigurationObject(config)
+    if (!remoteContractValidationEnabled) return zodParse
 
     // Then, even if this failed, we try to validate against the contract.
     const zodValidatedData = zodParse.state === 'ok' ? zodParse.data : undefined
     const subjectForAjv = zodValidatedData ?? (config as JsonMapType)
 
-    const jsonSchemaParse = jsonSchemaValidate(
-      subjectForAjv,
-      contract,
-      handleInvalidAdditionalProperties,
-      extensionIdentifier,
-    )
+    let jsonSchemaParse: ReturnType<typeof jsonSchemaValidate>
+    try {
+      jsonSchemaParse = jsonSchemaValidate(
+        subjectForAjv,
+        contract,
+        handleInvalidAdditionalProperties,
+        extensionIdentifier,
+      )
+      // eslint-disable-next-line no-catch-all/no-catch-all
+    } catch {
+      remoteContractValidationEnabled = false
+      warnRemoteContractValidationSkipped(extensionIdentifier)
+      return zodParse
+    }
 
     // Finally, we de-duplicate the error set from both validations -- identical messages for identical paths are removed
     let errors = zodParse.errors ?? []
@@ -88,3 +111,9 @@ export async function unifiedConfigurationParserFactory(
   }
   return parseConfigurationObject
 }
+
+export function warnRemoteContractValidationSkipped(identifier: string) {
+  outputWarn(
+    `Remote contract validation for "${identifier}" is skipped because its schema is invalid. Server-side validation remains the authority.`,
+  )
+}