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
2 changes: 2 additions & 0 deletions packages/app/src/cli/models/app/config-file-naming.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ describe('isValidFormatAppConfigurationFileName', () => {
test('returns false for invalid filenames', () => {
expect(isValidFormatAppConfigurationFileName('production')).toBe(false)
expect(isValidFormatAppConfigurationFileName('shopify.web.toml')).toBe(false)
expect(isValidFormatAppConfigurationFileName('shopify.application.toml')).toBe(false)
expect(isValidFormatAppConfigurationFileName('shopify.app.foo.bar.toml')).toBe(false)
expect(isValidFormatAppConfigurationFileName('shopify.app..toml')).toBe(false)
expect(isValidFormatAppConfigurationFileName('')).toBe(false)
})
Expand Down
3 changes: 3 additions & 0 deletions packages/app/src/cli/models/app/config-file-naming.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ import {basename} from '@shopify/cli-kit/node/path'
const appConfigurationFileNameRegex = /^shopify\.app(\.[-\w]+)?\.toml$/
export type AppConfigurationFileName = 'shopify.app.toml' | `shopify.app.${string}.toml`

/** Glob used to discover app configuration files. Always filter matches with `isValidFormatAppConfigurationFileName`. */
export const APP_CONFIG_FILE_GLOB = 'shopify.app*.toml'

/**
* Gets the name of the app configuration file (e.g. `shopify.app.production.toml`) based on a provided config name.
*
Expand Down
11 changes: 5 additions & 6 deletions packages/app/src/cli/models/project/project.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {configurationFileNames} from '../../constants.js'
import {APP_CONFIG_FILE_GLOB, isValidFormatAppConfigurationFileName} from '../app/config-file-naming.js'
import {TomlFile, TomlFileError} from '@shopify/cli-kit/node/toml/toml-file'
import {readAndParseDotEnv, DotEnvFile} from '@shopify/cli-kit/node/dot-env'
import {fileExists, glob, findPathUp, readFile} from '@shopify/cli-kit/node/fs'
Expand All @@ -12,8 +13,6 @@ import {joinPath, basename} from '@shopify/cli-kit/node/path'
import {AbortError} from '@shopify/cli-kit/node/error'
import {JsonMapType} from '@shopify/cli-kit/node/toml'

const APP_CONFIG_GLOB = 'shopify.app*.toml'
const APP_CONFIG_REGEX = /^shopify\.app(\.[-\w]+)?\.toml$/
const EXTENSION_TOML = '*.extension.toml'
const WEB_TOML = 'shopify.web.toml'
const DEFAULT_EXTENSION_DIR = 'extensions/*'
Expand Down Expand Up @@ -164,8 +163,8 @@ export class Project {
async function findProjectRoot(startDirectory: string): Promise<string> {
const found = await findPathUp(
async (directory) => {
const matches = await glob(joinPath(directory, APP_CONFIG_GLOB))
if (matches.length > 0) return directory
const matches = await glob(joinPath(directory, APP_CONFIG_FILE_GLOB))
if (matches.some((path) => isValidFormatAppConfigurationFileName(basename(path)))) return directory
},
{
cwd: startDirectory,
Expand All @@ -181,9 +180,9 @@ async function findProjectRoot(startDirectory: string): Promise<string> {
}

async function discoverAppConfigFiles(directory: string, errors: TomlFileError[]): Promise<TomlFile[]> {
const pattern = joinPath(directory, APP_CONFIG_GLOB)
const pattern = joinPath(directory, APP_CONFIG_FILE_GLOB)
const paths = await glob(pattern)
const validPaths = paths.filter((filePath) => APP_CONFIG_REGEX.test(basename(filePath)))
const validPaths = paths.filter((filePath) => isValidFormatAppConfigurationFileName(basename(filePath)))
return readTomlFilesCollectingErrors(validPaths, errors)
}

Expand Down
6 changes: 4 additions & 2 deletions packages/app/src/cli/prompts/config.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/* eslint-disable no-await-in-loop */
import {AppConfigurationFileName} from '../models/app/loader.js'
import {APP_CONFIG_FILE_GLOB} from '../models/app/config-file-naming.js'
import {AppConfigurationFileName, isValidFormatAppConfigurationFileName} from '../models/app/loader.js'
import {
RenderTextPromptOptions,
renderConfirmationPrompt,
Expand Down Expand Up @@ -43,7 +44,8 @@ function filenameFromName(name: string, highlight = false): AppConfigurationFile
}

export async function findConfigFiles(directory: string): Promise<string[]> {
return glob(joinPath(directory, 'shopify.app*.toml'))
const files = await glob(joinPath(directory, APP_CONFIG_FILE_GLOB))
return files.filter((path) => isValidFormatAppConfigurationFileName(basename(path)))
}

export async function selectConfigFile(directory: string): Promise<Result<string, string>> {
Expand Down
108 changes: 56 additions & 52 deletions packages/app/src/cli/services/app-doctor-engine/scanners/discover.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import {APP_CONFIG_FILE_GLOB, isValidFormatAppConfigurationFileName} from '../../../models/app/config-file-naming.js'
import {AppAccessScopesSchema, AppAuthSchema} from '../../../models/extensions/specifications/app_config_app_access.js'
import {WebhookSubscriptionSchema} from '../../../models/extensions/specifications/app_config_webhook_schemas/webhook_subscription_schema.js'
import {removeTrailingSlash} from '../../../models/extensions/specifications/validation/common.js'
import {fileExistsSync, fileSizeSync, globSync, readFileSync} from '@shopify/cli-kit/node/fs'
import {cwd, dirname, extname, joinPath, relativePath, resolvePath} from '@shopify/cli-kit/node/path'
import {basename, cwd, dirname, extname, joinPath, relativePath, resolvePath} from '@shopify/cli-kit/node/path'
import {zod} from '@shopify/cli-kit/node/schema'
import {decodeToml} from '@shopify/cli-kit/node/toml/codec'
import {lstatSync} from 'node:fs'
Expand All @@ -19,11 +21,12 @@ export class AppRootDiscoveryError extends Error {
/**
* Find the nearest app root without ever substituting CWD for a bad explicit path.
*
* This is intentionally not `Project.load()`. That loader also reads package,
* environment, and hidden configuration, and it does not keep the bounded raw
* bytes App Doctor hashes and reports as coverage. App Doctor reuses the same
* `shopify.app*.toml` candidate shape, then reads those files through its own
* repository boundary.
* App identity matches the rest of Shopify CLI: walk up for
* `shopify.app.toml` / `shopify.app.<name>.toml` using
* `isValidFormatAppConfigurationFileName`. This is not `Project.load()` — that
* loader also reads package, environment, and hidden configuration, follows
* different symlink policy, and does not keep the bounded raw bytes App Doctor
* hashes and reports as coverage.
*/
export function findAppRoot(startPath?: string): string {
const requestedPath = resolvePath(startPath ?? cwd())
Expand All @@ -33,8 +36,8 @@ export function findAppRoot(startPath?: string): string {

let directory = requestedPath
if (startPath && lstatSync(requestedPath).isFile()) {
if (!requestedPath.endsWith('.toml')) {
throw new AppRootDiscoveryError(`App path is not a directory or TOML file: ${startPath}`)
if (!isValidFormatAppConfigurationFileName(basename(requestedPath))) {
throw new AppRootDiscoveryError(`App path is not a directory or Shopify app configuration file: ${startPath}`)
}
return dirname(requestedPath)
}
Expand All @@ -43,14 +46,7 @@ export function findAppRoot(startPath?: string): string {
}

while (true) {
const tomls = globSync('shopify.app*.toml', {
cwd: directory,
deep: 1,
dot: false,
onlyFiles: false,
followSymbolicLinks: false,
})
if (tomls.length > 0) return directory
if (listAppConfigFiles(directory).length > 0) return directory

const parent = dirname(directory)
if (parent === directory) break
Expand All @@ -60,22 +56,25 @@ export function findAppRoot(startPath?: string): string {
throw new AppRootDiscoveryError(`Could not find a shopify.app*.toml from: ${startPath ?? cwd()}`)
}

/**
* Find and parse all shopify.app.*.toml files in the app root.
*
* Candidate discovery matches `Project.load()`, but parsing stays on bounded
* raw reads so unreadable files become coverage gaps instead of loader errors.
*/
export function findAppTomls(appRoot: string): AppTomlContent[] {
const files = globSync('shopify.app*.toml', {
cwd: appRoot,
function listAppConfigFiles(directory: string): string[] {
return globSync(APP_CONFIG_FILE_GLOB, {
cwd: directory,
deep: 1,
dot: false,
onlyFiles: false,
followSymbolicLinks: false,
})
}).filter((file) => isValidFormatAppConfigurationFileName(basename(file)))
}

return files.flatMap((file) => {
/**
* Find and parse all shopify.app.*.toml files in the app root.
*
* Candidate discovery uses the same config-file identity as `Project.load()`,
* but parsing stays on bounded raw reads so unreadable files become coverage
* gaps instead of loader errors.
*/
export function findAppTomls(appRoot: string): AppTomlContent[] {
return listAppConfigFiles(appRoot).flatMap((file) => {
const path = joinPath(appRoot, file)
const content = readRepositoryText(appRoot, path)
if (content === undefined) return []
Expand Down Expand Up @@ -116,6 +115,7 @@ export function loadAppToml(tomlPath: string, appRoot = dirname(tomlPath)): AppT
}
}

/** CLI webhook section shape. URI fields stay strings so INSECURE_WEBHOOK_URL can report http. */
const WebhooksSectionSchema = zod.object({
api_version: zod.string().optional(),
privacy_compliance: zod
Expand All @@ -128,10 +128,11 @@ const WebhooksSectionSchema = zod.object({
subscriptions: zod.array(zod.unknown()).optional(),
})

const EvidenceWebhookSubscriptionSchema = zod.object({
uri: zod.string(),
topics: zod.array(zod.string()).optional(),
compliance_topics: zod.array(zod.string()).optional(),
const SecurityWebhookSubscriptionSchema = WebhookSubscriptionSchema.extend({
uri: zod.preprocess(
(arg) => removeTrailingSlash(arg as string),
zod.string({invalid_type_error: 'Value must be string'}),
),
})

export function parseAppToml(
Expand Down Expand Up @@ -170,14 +171,11 @@ function projectAccessScopes(value: unknown, path: string, appRoot?: string): st
function projectRedirectUrls(value: unknown, path: string, appRoot?: string): string[] {
if (value === undefined) return []
const parsed = AppAuthSchema.safeParse(value)
if (parsed.success) return parsed.data.redirect_urls

const fallback = zod.object({redirect_urls: zod.array(zod.string())}).safeParse(value)
if (!fallback.success) {
if (!parsed.success) {
recordSectionGap(appRoot, path, 'auth section could not be parsed')
return []
}
return fallback.data.redirect_urls
return parsed.data.redirect_urls
}

function projectWebhooks(
Expand All @@ -192,7 +190,9 @@ function projectWebhooks(
return {subscriptions: []}
}

const webhookSubscriptions = (parsed.data.subscriptions ?? []).flatMap(projectWebhookSubscription)
const webhookSubscriptions = (parsed.data.subscriptions ?? []).flatMap((subscription) =>
projectWebhookSubscription(subscription, path, appRoot),
)
const privacyCompliance = parsed.data.privacy_compliance
const privacyComplianceWebhooks = [
{topic: 'customers/redact', uri: privacyCompliance?.customer_deletion_url},
Expand All @@ -206,25 +206,28 @@ function projectWebhooks(
}
}

function projectWebhookSubscription(value: unknown): WebhookSubscription[] {
const strict = WebhookSubscriptionSchema.safeParse(value)
if (strict.success) {
const WebhookUriOnlySchema = zod.object({
uri: zod.preprocess(
(arg) => removeTrailingSlash(arg as string),
zod.string({invalid_type_error: 'Value must be string'}),
),
})

function projectWebhookSubscription(value: unknown, path: string, appRoot?: string): WebhookSubscription[] {
const parsed = SecurityWebhookSubscriptionSchema.safeParse(value)
if (parsed.success) {
return [
{
topics: [...(strict.data.topics ?? []), ...(strict.data.compliance_topics ?? [])],
uri: strict.data.uri,
topics: [...(parsed.data.topics ?? []), ...(parsed.data.compliance_topics ?? [])],
uri: parsed.data.uri,
},
]
}

const evidence = EvidenceWebhookSubscriptionSchema.safeParse(value)
if (!evidence.success) return []
return [
{
topics: [...(evidence.data.topics ?? []), ...(evidence.data.compliance_topics ?? [])],
uri: evidence.data.uri,
},
]
recordSectionGap(appRoot, path, 'webhook subscription could not be parsed')
const uriOnly = WebhookUriOnlySchema.safeParse(value)
if (!uriOnly.success) return []
return [{topics: [], uri: uriOnly.data.uri}]
}

function recordSectionGap(appRoot: string | undefined, path: string, detail: string): void {
Expand Down Expand Up @@ -274,14 +277,15 @@ function normalizePath(path: string): string {
function findNestedAppDirectories(appRoot: string): string[] {
return [
...new Set(
globSync('**/shopify.app*.toml', {
globSync(`**/${APP_CONFIG_FILE_GLOB}`, {
followSymbolicLinks: false,
cwd: appRoot,
ignore: IGNORED_DIRECTORIES,
absolute: false,
dot: false,
onlyFiles: false,
})
.filter((path) => isValidFormatAppConfigurationFileName(basename(path)))
.map((path) => normalizePath(dirname(path)))
.filter((path) => path !== '.' && path.length > 0),
),
Expand All @@ -300,7 +304,7 @@ function discoveryIgnores(directory: string, projectRoot: string): string[] {
* Find extension-like repository content under the app root.
*
* `Project.load()` only considers paths in each app configuration's
* `extension_directories`. App Doctor scans every `shopify.extension.toml`
* `extension_directories`. App Doctor still scans every `shopify.extension.toml`
* inside the repository boundary, including unconfigured extensions, because
* those files can still contain secrets, XSS, and other security evidence.
* Nested apps, generated output, and test trees remain excluded.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ describe('deterministic rules product contract', () => {
expect(parsed.apiVersion).toBeUndefined()
})

test('keeps insecure webhook URIs that the CLI schema rejects', () => {
test('keeps insecure webhook URIs that the CLI URI validator rejects', () => {
const parsed = parseAppToml(
{
webhooks: {
Expand All @@ -119,6 +119,32 @@ describe('deterministic rules product contract', () => {
)
expect(parsed.webhooks).toEqual([{topics: ['orders/create'], uri: 'http://insecure.example/webhooks'}])
})

test('drops webhook subscriptions that are not CLI-shaped', () => {
const parsed = parseAppToml(
{
webhooks: {
api_version: '2023-07',
subscriptions: [{not: 'a-subscription'}, {topics: ['orders/create']}],
},
},
'/app/shopify.app.toml',
)
expect(parsed.webhooks).toEqual([])
})

test('keeps an insecure URI when another subscription field is invalid', () => {
const parsed = parseAppToml(
{
webhooks: {
api_version: '2023-07',
subscriptions: [{topics: ['orders/create'], uri: 'http://insecure.example/webhooks', filter: 42}],
},
},
'/app/shopify.app.toml',
)
expect(parsed.webhooks).toEqual([{topics: [], uri: 'http://insecure.example/webhooks'}])
})
})

describe('JavaScript regex mode', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,23 @@ describe.sequential('app root discovery', () => {
expect(() => findAppRoot(missing)).toThrow(AppRootDiscoveryError)
expect(() => findAppRoot(missing)).toThrow(`App path does not exist: ${missing}`)
})

test('ignores files that match the glob but are not CLI app configuration names', async () => {
const root = await makeDirectory()
await writeFile(join(root, 'shopify.application.toml'), appConfiguration)
await writeFile(join(root, 'shopify.app.foo.bar.toml'), appConfiguration)
expect(() => findAppRoot(root)).toThrow(AppRootDiscoveryError)
})

test('rejects an explicit non-app TOML path with a configuration-file error', async () => {
const root = await makeDirectory()
const webToml = join(root, 'shopify.web.toml')
await writeFile(webToml, 'type = "frontend"\n')
expect(() => findAppRoot(webToml)).toThrow(AppRootDiscoveryError)
expect(() => findAppRoot(webToml)).toThrow(
`App path is not a directory or Shopify app configuration file: ${webToml}`,
)
})
})

describe('repository discovery exclusions', () => {
Expand Down
Loading