From 29bd11561079c3126ac8e6ffb05c8a2053a4f966 Mon Sep 17 00:00:00 2001 From: Nick Hayden Date: Tue, 21 Jul 2026 11:41:43 -0400 Subject: [PATCH 01/11] Write TOML updates atomically --- packages/cli-kit/src/public/node/fs.test.ts | 61 +++++++++++++++++++ packages/cli-kit/src/public/node/fs.ts | 20 ++++++ .../src/public/node/toml/toml-file.test.ts | 28 ++++++++- .../cli-kit/src/public/node/toml/toml-file.ts | 10 +-- 4 files changed, 112 insertions(+), 7 deletions(-) diff --git a/packages/cli-kit/src/public/node/fs.test.ts b/packages/cli-kit/src/public/node/fs.test.ts index d30612b5b96..09054ad923d 100644 --- a/packages/cli-kit/src/public/node/fs.test.ts +++ b/packages/cli-kit/src/public/node/fs.test.ts @@ -22,10 +22,71 @@ import { copyDirectoryContents, symlink, fileRealPath, + readdir, + writeFileAtomically, } from './fs.js' import {joinPath, normalizePath} from './path.js' import * as array from '../common/array.js' import {describe, expect, test, vi} from 'vitest' +import {rename as fsRename, stat} from 'node:fs/promises' + +vi.mock('node:fs/promises', async (importOriginal) => { + const actual = await importOriginal() + return {...actual, rename: vi.fn(actual.rename)} +}) + +describe('writeFileAtomically', () => { + test('replaces existing file content', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const path = joinPath(tmpDir, 'test-file') + await writeFile(path, 'original content') + + await writeFileAtomically(path, 'replacement content') + + await expect(readFile(path)).resolves.toBe('replacement content') + }) + }) + + test('removes the temporary sibling after success', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const fileName = 'test-file' + const path = joinPath(tmpDir, fileName) + + await writeFileAtomically(path, 'content') + + const entries = await readdir(tmpDir) + expect(entries.filter((entry) => entry.startsWith(`${fileName}.`) && entry.endsWith('.tmp'))).toStrictEqual([]) + }) + }) + + test.skipIf(process.platform === 'win32')('preserves the mode of an existing file', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const path = joinPath(tmpDir, 'test-file') + await writeFile(path, 'original content') + await chmod(path, 0o754) + const originalMode = (await stat(path)).mode + + await writeFileAtomically(path, 'replacement content') + + expect((await stat(path)).mode).toBe(originalMode) + }) + }) + + test('leaves the original unchanged and removes the temporary sibling when replacement fails', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const fileName = 'test-file' + const path = joinPath(tmpDir, fileName) + await writeFile(path, 'original content') + vi.mocked(fsRename).mockRejectedValueOnce(new Error('replacement failed')) + + await expect(writeFileAtomically(path, 'replacement content')).rejects.toThrow('replacement failed') + + await expect(readFile(path)).resolves.toBe('original content') + const entries = await readdir(tmpDir) + expect(entries.filter((entry) => entry.startsWith(`${fileName}.`) && entry.endsWith('.tmp'))).toStrictEqual([]) + }) + }) +}) describe('inTemporaryDirectory', () => { test('ties the lifecycle of the temporary directory to the lifecycle of the callback', async () => { diff --git a/packages/cli-kit/src/public/node/fs.ts b/packages/cli-kit/src/public/node/fs.ts index 5948c9f6bb6..709d8088cf0 100644 --- a/packages/cli-kit/src/public/node/fs.ts +++ b/packages/cli-kit/src/public/node/fs.ts @@ -54,6 +54,7 @@ import { symlink as fsSymlink, } from 'fs/promises' import {pathToFileURL as pathToFile} from 'url' +import {randomUUID} from 'node:crypto' import * as os from 'os' import type {Pattern, Options as GlobOptions} from 'fast-glob' @@ -229,6 +230,25 @@ export async function writeFile( await fsWriteFile(path, data, options) } +/** + * Atomically writes text content to a file through a sibling temporary file. + * + * @param path - Path to the file to be written. + * @param data - Text content to be written. + */ +export async function writeFileAtomically(path: string, data: string): Promise { + const temporaryPath = `${path}.${process.pid}.${randomUUID()}.tmp` + const existingMode = (await fileExists(path)) ? (await fsStat(path)).mode : undefined + + try { + await writeFile(temporaryPath, data) + if (existingMode !== undefined) await chmod(temporaryPath, existingMode) + await renameFile(temporaryPath, path) + } finally { + await removeFile(temporaryPath) + } +} + /** * Synchronously writes content to file at path. * diff --git a/packages/cli-kit/src/public/node/toml/toml-file.test.ts b/packages/cli-kit/src/public/node/toml/toml-file.test.ts index 9e507c60df0..e9f8515a6b6 100644 --- a/packages/cli-kit/src/public/node/toml/toml-file.test.ts +++ b/packages/cli-kit/src/public/node/toml/toml-file.test.ts @@ -1,8 +1,14 @@ import {TomlFile, TomlFileError} from './toml-file.js' import {shouldReportErrorAsUnexpected} from '../error.js' -import {writeFile, readFile, inTemporaryDirectory} from '../fs.js' +import {writeFile, readFile, inTemporaryDirectory, readdir} from '../fs.js' import {joinPath} from '../path.js' -import {describe, expect, test} from 'vitest' +import {describe, expect, test, vi} from 'vitest' +import {rename as fsRename} from 'node:fs/promises' + +vi.mock('node:fs/promises', async (importOriginal) => { + const actual = await importOriginal() + return {...actual, rename: vi.fn(actual.rename)} +}) describe('TomlFile', () => { describe('read', () => { @@ -137,6 +143,24 @@ describe('TomlFile', () => { expect(content.auth.redirect_urls).toStrictEqual(['https://new.com', 'https://other.com']) }) }) + + test('leaves the original unchanged when atomic replacement fails', async () => { + await inTemporaryDirectory(async (dir) => { + const fileName = 'test.toml' + const path = joinPath(dir, fileName) + const originalContent = 'name = "old"\n' + await writeFile(path, originalContent) + const file = await TomlFile.read(path) + vi.mocked(fsRename).mockRejectedValueOnce(new Error('replacement failed')) + + await expect(file.patch({name: 'new'})).rejects.toThrow('replacement failed') + + await expect(readFile(path)).resolves.toBe(originalContent) + expect(file.content).toStrictEqual({name: 'old'}) + const entries = await readdir(dir) + expect(entries.filter((entry) => entry.startsWith(`${fileName}.`) && entry.endsWith('.tmp'))).toStrictEqual([]) + }) + }) }) describe('remove', () => { diff --git a/packages/cli-kit/src/public/node/toml/toml-file.ts b/packages/cli-kit/src/public/node/toml/toml-file.ts index a2ee81b3236..7e9287ae76c 100644 --- a/packages/cli-kit/src/public/node/toml/toml-file.ts +++ b/packages/cli-kit/src/public/node/toml/toml-file.ts @@ -1,6 +1,6 @@ import {JsonMapType, decodeToml, encodeToml} from './codec.js' import {AbortError} from '../error.js' -import {fileExists, readFile, writeFile} from '../fs.js' +import {fileExists, readFile, writeFileAtomically} from '../fs.js' import {updateTomlValues} from '@shopify/toml-patch' type TomlPatchValue = string | number | boolean | undefined | (string | number | boolean)[] @@ -76,7 +76,7 @@ export class TomlFile { const raw = await readFile(this.path) const updated = updateTomlValues(raw, patches) const parsed = this.decode(updated) - await writeFile(this.path, updated) + await writeFileAtomically(this.path, updated) this.content = parsed } @@ -94,7 +94,7 @@ export class TomlFile { const raw = await readFile(this.path) const updated = updateTomlValues(raw, [[keys, undefined]]) const parsed = this.decode(updated) - await writeFile(this.path, updated) + await writeFileAtomically(this.path, updated) this.content = parsed } @@ -111,7 +111,7 @@ export class TomlFile { async replace(content: JsonMapType): Promise { const encoded = encodeToml(content) this.decode(encoded) - await writeFile(this.path, encoded) + await writeFileAtomically(this.path, encoded) this.content = content } @@ -133,7 +133,7 @@ export class TomlFile { const raw = await readFile(this.path) const transformed = transform(raw) const parsed = this.decode(transformed) - await writeFile(this.path, transformed) + await writeFileAtomically(this.path, transformed) this.content = parsed } From 7743769f836c9c64c70e689f6e65e1d8eb5370aa Mon Sep 17 00:00:00 2001 From: Nick Hayden Date: Wed, 22 Jul 2026 11:01:22 -0400 Subject: [PATCH 02/11] Harden atomic writes --- packages/cli-kit/src/public/node/fs.test.ts | 59 ++++++++++++++++++++- packages/cli-kit/src/public/node/fs.ts | 28 ++++++++-- 2 files changed, 81 insertions(+), 6 deletions(-) diff --git a/packages/cli-kit/src/public/node/fs.test.ts b/packages/cli-kit/src/public/node/fs.test.ts index 09054ad923d..8e0f76c383a 100644 --- a/packages/cli-kit/src/public/node/fs.test.ts +++ b/packages/cli-kit/src/public/node/fs.test.ts @@ -28,11 +28,11 @@ import { import {joinPath, normalizePath} from './path.js' import * as array from '../common/array.js' import {describe, expect, test, vi} from 'vitest' -import {rename as fsRename, stat} from 'node:fs/promises' +import {rename as fsRename, stat, lstat, writeFile as fsWriteFile} from 'node:fs/promises' vi.mock('node:fs/promises', async (importOriginal) => { const actual = await importOriginal() - return {...actual, rename: vi.fn(actual.rename)} + return {...actual, rename: vi.fn(actual.rename), writeFile: vi.fn(actual.writeFile)} }) describe('writeFileAtomically', () => { @@ -59,15 +59,40 @@ describe('writeFileAtomically', () => { }) }) + test('passes a restrictive mode when creating the temporary sibling', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const path = joinPath(tmpDir, 'test-file') + vi.mocked(fsWriteFile).mockClear() + + await writeFileAtomically(path, 'content') + + const temporaryWrite = vi.mocked(fsWriteFile).mock.calls.find(([writePath]) => String(writePath).endsWith('.tmp')) + expect(temporaryWrite?.[2]).toMatchObject({encoding: 'utf8', mode: 0o600}) + }) + }) + + test.skipIf(process.platform === 'win32')('uses restrictive permissions for a new file', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const path = joinPath(tmpDir, 'test-file') + + await writeFileAtomically(path, 'content') + + expect((await stat(path)).mode & 0o777).toBe(0o600) + }) + }) + test.skipIf(process.platform === 'win32')('preserves the mode of an existing file', async () => { await inTemporaryDirectory(async (tmpDir) => { const path = joinPath(tmpDir, 'test-file') await writeFile(path, 'original content') await chmod(path, 0o754) const originalMode = (await stat(path)).mode + vi.mocked(fsWriteFile).mockClear() await writeFileAtomically(path, 'replacement content') + const temporaryWrite = vi.mocked(fsWriteFile).mock.calls.find(([writePath]) => String(writePath).endsWith('.tmp')) + expect(temporaryWrite?.[2]).toMatchObject({mode: originalMode & 0o7777}) expect((await stat(path)).mode).toBe(originalMode) }) }) @@ -86,6 +111,36 @@ describe('writeFileAtomically', () => { expect(entries.filter((entry) => entry.startsWith(`${fileName}.`) && entry.endsWith('.tmp'))).toStrictEqual([]) }) }) + + test('writes through an existing file symlink and preserves the link', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const targetPath = joinPath(tmpDir, 'target-file') + const linkPath = joinPath(tmpDir, 'linked-file') + await writeFile(targetPath, 'original content') + await symlink(targetPath, linkPath) + + await writeFileAtomically(linkPath, 'replacement content') + + await expect(readFile(targetPath)).resolves.toBe('replacement content') + expect((await lstat(linkPath)).isSymbolicLink()).toBe(true) + const entries = await readdir(tmpDir) + expect(entries.filter((entry) => entry.endsWith('.tmp'))).toStrictEqual([]) + }) + }) + + test('rejects a dangling symlink without replacing it', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const linkPath = joinPath(tmpDir, 'linked-file') + await symlink(joinPath(tmpDir, 'missing-target'), linkPath) + + await expect(writeFileAtomically(linkPath, 'replacement content')).rejects.toThrow( + `Unable to atomically write ${linkPath}: destination is a dangling symlink`, + ) + + expect((await lstat(linkPath)).isSymbolicLink()).toBe(true) + await expect(readdir(tmpDir)).resolves.toEqual(['linked-file']) + }) + }) }) describe('inTemporaryDirectory', () => { diff --git a/packages/cli-kit/src/public/node/fs.ts b/packages/cli-kit/src/public/node/fs.ts index 709d8088cf0..b05e8b55128 100644 --- a/packages/cli-kit/src/public/node/fs.ts +++ b/packages/cli-kit/src/public/node/fs.ts @@ -212,6 +212,7 @@ export function appendFileSync(path: string, data: string): void { export interface WriteOptions { encoding: BufferEncoding + mode?: number } /** @@ -237,13 +238,32 @@ export async function writeFile( * @param data - Text content to be written. */ export async function writeFileAtomically(path: string, data: string): Promise { - const temporaryPath = `${path}.${process.pid}.${randomUUID()}.tmp` - const existingMode = (await fileExists(path)) ? (await fsStat(path)).mode : undefined + let destinationStats + try { + destinationStats = await fsLstat(path) + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error + } + + let destinationPath = path + if (destinationStats) { + try { + destinationPath = await fileRealPath(path) + } catch (error) { + if (destinationStats.isSymbolicLink() && (error as NodeJS.ErrnoException).code === 'ENOENT') { + throw new Error(`Unable to atomically write ${path}: destination is a dangling symlink.`) + } + throw error + } + } + + const existingMode = destinationStats ? (await fsStat(destinationPath)).mode & 0o7777 : undefined + const temporaryPath = `${destinationPath}.${process.pid}.${randomUUID()}.tmp` try { - await writeFile(temporaryPath, data) + await writeFile(temporaryPath, data, {encoding: 'utf8', mode: existingMode ?? 0o600}) if (existingMode !== undefined) await chmod(temporaryPath, existingMode) - await renameFile(temporaryPath, path) + await renameFile(temporaryPath, destinationPath) } finally { await removeFile(temporaryPath) } From 8ccf3e34a37e77690b043ef058ef3cdb287d0888 Mon Sep 17 00:00:00 2001 From: Nick Hayden Date: Wed, 22 Jul 2026 11:01:22 -0400 Subject: [PATCH 03/11] Add atomic write changeset --- .changeset/theme-airlock-atomic-toml-writes.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/theme-airlock-atomic-toml-writes.md diff --git a/.changeset/theme-airlock-atomic-toml-writes.md b/.changeset/theme-airlock-atomic-toml-writes.md new file mode 100644 index 00000000000..f84973ec8e4 --- /dev/null +++ b/.changeset/theme-airlock-atomic-toml-writes.md @@ -0,0 +1,5 @@ +--- +'@shopify/cli-kit': minor +--- + +Add atomic text writes and use them for TOML configuration updates. From a9b957f272f5ce41aa27a3591993bb4df3b679a0 Mon Sep 17 00:00:00 2001 From: Nick Hayden Date: Thu, 23 Jul 2026 11:52:38 -0400 Subject: [PATCH 04/11] Harden atomic temporary file handling --- .../theme-airlock-atomic-toml-writes.md | 2 +- packages/cli-kit/src/public/node/fs.test.ts | 31 ++++++++++++++++++- packages/cli-kit/src/public/node/fs.ts | 21 +++++++++++-- 3 files changed, 50 insertions(+), 4 deletions(-) diff --git a/.changeset/theme-airlock-atomic-toml-writes.md b/.changeset/theme-airlock-atomic-toml-writes.md index f84973ec8e4..af0f40cacf5 100644 --- a/.changeset/theme-airlock-atomic-toml-writes.md +++ b/.changeset/theme-airlock-atomic-toml-writes.md @@ -2,4 +2,4 @@ '@shopify/cli-kit': minor --- -Add atomic text writes and use them for TOML configuration updates. +Write TOML configuration updates atomically while preserving existing file permissions and creating new files securely. diff --git a/packages/cli-kit/src/public/node/fs.test.ts b/packages/cli-kit/src/public/node/fs.test.ts index 8e0f76c383a..ef3f74d3b4a 100644 --- a/packages/cli-kit/src/public/node/fs.test.ts +++ b/packages/cli-kit/src/public/node/fs.test.ts @@ -28,6 +28,9 @@ import { import {joinPath, normalizePath} from './path.js' import * as array from '../common/array.js' import {describe, expect, test, vi} from 'vitest' +// eslint-disable-next-line @typescript-eslint/ban-ts-comment +// @ts-ignore +import {remove as fsRemove} from 'fs-extra/esm' import {rename as fsRename, stat, lstat, writeFile as fsWriteFile} from 'node:fs/promises' vi.mock('node:fs/promises', async (importOriginal) => { @@ -35,6 +38,11 @@ vi.mock('node:fs/promises', async (importOriginal) => { return {...actual, rename: vi.fn(actual.rename), writeFile: vi.fn(actual.writeFile)} }) +vi.mock('fs-extra/esm', async (importOriginal) => { + const actual = await importOriginal<{remove: typeof fsRemove}>() + return {...actual, remove: vi.fn(actual.remove)} +}) + describe('writeFileAtomically', () => { test('replaces existing file content', async () => { await inTemporaryDirectory(async (tmpDir) => { @@ -92,7 +100,7 @@ describe('writeFileAtomically', () => { await writeFileAtomically(path, 'replacement content') const temporaryWrite = vi.mocked(fsWriteFile).mock.calls.find(([writePath]) => String(writePath).endsWith('.tmp')) - expect(temporaryWrite?.[2]).toMatchObject({mode: originalMode & 0o7777}) + expect(temporaryWrite?.[2]).toMatchObject({encoding: 'utf8', mode: 0o600}) expect((await stat(path)).mode).toBe(originalMode) }) }) @@ -112,6 +120,27 @@ describe('writeFileAtomically', () => { }) }) + test('rejects the cleanup error after a successful replacement', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const path = joinPath(tmpDir, 'test-file') + vi.mocked(fsRemove).mockRejectedValueOnce(new Error('cleanup failed')) + + await expect(writeFileAtomically(path, 'replacement content')).rejects.toThrow('cleanup failed') + await expect(readFile(path)).resolves.toBe('replacement content') + }) + }) + + test('preserves the replacement error when temporary cleanup also fails', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const path = joinPath(tmpDir, 'test-file') + await writeFile(path, 'original content') + vi.mocked(fsRename).mockRejectedValueOnce(new Error('replacement failed')) + vi.mocked(fsRemove).mockRejectedValueOnce(new Error('cleanup failed')) + + await expect(writeFileAtomically(path, 'replacement content')).rejects.toThrow('replacement failed') + }) + }) + test('writes through an existing file symlink and preserves the link', async () => { await inTemporaryDirectory(async (tmpDir) => { const targetPath = joinPath(tmpDir, 'target-file') diff --git a/packages/cli-kit/src/public/node/fs.ts b/packages/cli-kit/src/public/node/fs.ts index b05e8b55128..e5a15817115 100644 --- a/packages/cli-kit/src/public/node/fs.ts +++ b/packages/cli-kit/src/public/node/fs.ts @@ -260,13 +260,30 @@ export async function writeFileAtomically(path: string, data: string): Promise Date: Tue, 21 Jul 2026 12:13:19 -0400 Subject: [PATCH 05/11] Load project-scoped store trust --- .../utilities/theme-airlock/config.test.ts | 227 ++++++++++++++++++ .../src/cli/utilities/theme-airlock/config.ts | 78 ++++++ .../src/cli/utilities/theme-airlock/types.ts | 37 +++ 3 files changed, 342 insertions(+) create mode 100644 packages/theme/src/cli/utilities/theme-airlock/config.test.ts create mode 100644 packages/theme/src/cli/utilities/theme-airlock/config.ts create mode 100644 packages/theme/src/cli/utilities/theme-airlock/types.ts diff --git a/packages/theme/src/cli/utilities/theme-airlock/config.test.ts b/packages/theme/src/cli/utilities/theme-airlock/config.test.ts new file mode 100644 index 00000000000..b8fe456b303 --- /dev/null +++ b/packages/theme/src/cli/utilities/theme-airlock/config.test.ts @@ -0,0 +1,227 @@ +import {loadThemeProjectTrust} from './config.js' +import {ThemeAirlockError} from './types.js' +import {configurationFileName} from '../../constants.js' +import {describe, expect, test} from 'vitest' +import {inTemporaryDirectory, mkdir, readFile, writeFile} from '@shopify/cli-kit/node/fs' +import {joinPath} from '@shopify/cli-kit/node/path' +import {TomlFileError} from '@shopify/cli-kit/node/toml/toml-file' + +async function writeConfiguration(directory: string, content: string): Promise { + await mkdir(directory) + const configurationPath = joinPath(directory, configurationFileName) + await writeFile(configurationPath, content) + return configurationPath +} + +async function captureError(operation: () => Promise): Promise { + try { + await operation() + } catch (error) { + if (error instanceof Error) return error + throw error + } + throw new Error('Expected operation to throw') +} + +describe('loadThemeProjectTrust', () => { + test('returns unconfigured when no configuration file exists', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const themePath = joinPath(tmpDir, 'theme') + await mkdir(themePath) + + await expect(loadThemeProjectTrust(themePath)).resolves.toEqual({state: 'unconfigured', themePath}) + }) + }) + + test('returns unconfigured with the discovered path when the file has no environments', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const themePath = joinPath(tmpDir, 'theme') + const configurationPath = await writeConfiguration(tmpDir, 'name = "theme-project"\n') + await mkdir(themePath) + + await expect(loadThemeProjectTrust(themePath)).resolves.toEqual({ + state: 'unconfigured', + path: configurationPath, + themePath, + }) + }) + }) + + test.each(['environments = "preview"\n', 'environments = []\n'])( + 'returns unconfigured when environments is not an object: %s', + async (content) => { + await inTemporaryDirectory(async (tmpDir) => { + const themePath = joinPath(tmpDir, 'theme') + const configurationPath = await writeConfiguration(tmpDir, content) + await mkdir(themePath) + + await expect(loadThemeProjectTrust(themePath)).resolves.toEqual({ + state: 'unconfigured', + path: configurationPath, + themePath, + }) + }) + }, + ) + + test('uses the nearest ancestor configuration', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const projectPath = joinPath(tmpDir, 'project') + const themePath = joinPath(projectPath, 'themes', 'dawn') + await writeConfiguration(tmpDir, '[environments.default]\nstore = "root-store"\n') + const configurationPath = await writeConfiguration( + projectPath, + '[environments.default]\nstore = "project-store"\n', + ) + await mkdir(themePath) + + await expect(loadThemeProjectTrust(themePath)).resolves.toEqual({ + state: 'configured', + path: configurationPath, + themePath, + environments: [{name: 'default', store: 'project-store.myshopify.com'}], + }) + }) + }) + + test('ignores configuration files in sibling directories', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const themePath = joinPath(tmpDir, 'theme') + await writeConfiguration(joinPath(tmpDir, 'sibling'), '[environments.default]\nstore = "sibling-store"\n') + await mkdir(themePath) + + await expect(loadThemeProjectTrust(themePath)).resolves.toEqual({state: 'unconfigured', themePath}) + }) + }) + + test('normalizes default and named stores in TOML iteration order', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const themePath = joinPath(tmpDir, 'theme') + const configurationPath = await writeConfiguration( + tmpDir, + [ + '[environments.default]', + 'store = "default-store"', + '', + '[environments.preview]', + 'store = "https://Named-Store.myshopify.com/admin/"', + '', + ].join('\n'), + ) + await mkdir(themePath) + + await expect(loadThemeProjectTrust(themePath)).resolves.toEqual({ + state: 'configured', + path: configurationPath, + themePath, + environments: [ + {name: 'default', store: 'default-store.myshopify.com'}, + {name: 'preview', store: 'named-store.myshopify.com'}, + ], + }) + }) + }) + + test('does not trust environment entries without stores', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const themePath = joinPath(tmpDir, 'theme') + const configurationPath = await writeConfiguration( + tmpDir, + '[environments.default]\ntheme = "123456789"\n\n[environments.preview]\npassword = "secret"\n', + ) + await mkdir(themePath) + + await expect(loadThemeProjectTrust(themePath)).resolves.toEqual({ + state: 'unconfigured', + path: configurationPath, + themePath, + }) + }) + }) + + test('throws for a non-string store without modifying the configuration', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const themePath = joinPath(tmpDir, 'theme') + const originalContent = '[environments.production]\nstore = 123\n' + const configurationPath = await writeConfiguration(tmpDir, originalContent) + await mkdir(themePath) + + const error = await captureError(() => loadThemeProjectTrust(themePath)) + + expect(error).toBeInstanceOf(ThemeAirlockError) + if (!(error instanceof ThemeAirlockError)) throw error + expect(error.reason).toBe('malformed-configuration') + expect(error.targets).toEqual([]) + expect(error.tryMessage).toBe('No files were uploaded.') + expect(error.message).toContain(configurationPath) + expect(error.message).toContain('production') + await expect(readFile(configurationPath)).resolves.toBe(originalContent) + }) + }) + + test('throws for an invalid string store without modifying the configuration', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const themePath = joinPath(tmpDir, 'theme') + const originalContent = '[environments.preview]\nstore = ""\n' + const configurationPath = await writeConfiguration(tmpDir, originalContent) + await mkdir(themePath) + + const error = await captureError(() => loadThemeProjectTrust(themePath)) + + expect(error).toBeInstanceOf(ThemeAirlockError) + if (!(error instanceof ThemeAirlockError)) throw error + expect(error.reason).toBe('malformed-configuration') + expect(error.targets).toEqual([]) + expect(error.tryMessage).toBe('No files were uploaded.') + expect(error.message).toContain(configurationPath) + expect(error.message).toContain('preview') + await expect(readFile(configurationPath)).resolves.toBe(originalContent) + }) + }) + + test('throws when two environment names normalize to the same store without modifying the configuration', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const themePath = joinPath(tmpDir, 'theme') + const originalContent = [ + '[environments.default]', + 'store = "shared-store"', + '', + '[environments.production]', + 'store = "shared-store.myshopify.com"', + '', + ].join('\n') + const configurationPath = await writeConfiguration(tmpDir, originalContent) + await mkdir(themePath) + + const error = await captureError(() => loadThemeProjectTrust(themePath)) + + expect(error).toBeInstanceOf(ThemeAirlockError) + if (!(error instanceof ThemeAirlockError)) throw error + expect(error.reason).toBe('ambiguous-configuration') + expect(error.targets).toEqual([]) + expect(error.tryMessage).toBe('No files were uploaded.') + expect(error.message).toContain(configurationPath) + expect(error.message).toContain('default') + expect(error.message).toContain('production') + expect(error.message).toContain('shared-store.myshopify.com') + await expect(readFile(configurationPath)).resolves.toBe(originalContent) + }) + }) + + test('reports malformed TOML with its exact path without modifying the configuration', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const themePath = joinPath(tmpDir, 'theme') + const originalContent = '[environments.default\nstore = "broken"\n' + const configurationPath = await writeConfiguration(tmpDir, originalContent) + await mkdir(themePath) + + const error = await captureError(() => loadThemeProjectTrust(themePath)) + + expect(error).toBeInstanceOf(TomlFileError) + if (!(error instanceof TomlFileError)) throw error + expect(error.path).toBe(configurationPath) + expect(error.message).toContain(configurationPath) + await expect(readFile(configurationPath)).resolves.toBe(originalContent) + }) + }) +}) diff --git a/packages/theme/src/cli/utilities/theme-airlock/config.ts b/packages/theme/src/cli/utilities/theme-airlock/config.ts new file mode 100644 index 00000000000..19d3164d505 --- /dev/null +++ b/packages/theme/src/cli/utilities/theme-airlock/config.ts @@ -0,0 +1,78 @@ +import {ThemeAirlockError} from './types.js' +import {configurationFileName} from '../../constants.js' +import {environmentFilePath} from '@shopify/cli-kit/node/environments' +import {AbortError} from '@shopify/cli-kit/node/error' +import {normalizeStoreFqdn} from '@shopify/cli-kit/node/context/fqdn' +import {TomlFile, TomlFileError} from '@shopify/cli-kit/node/toml/toml-file' + +import type {ThemeProjectTrust, TrustedThemeEnvironment} from './types.js' + +function isObject(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value) +} + +async function readConfiguration(configurationPath: string): Promise { + try { + return await TomlFile.read(configurationPath) + } catch (error) { + if (error instanceof TomlFileError) { + throw new TomlFileError( + configurationPath, + `Unable to parse theme configuration at ${configurationPath}: ${error.message}`, + ) + } + throw error + } +} + +export async function loadThemeProjectTrust(themePath: string): Promise { + const configurationPath = await environmentFilePath(configurationFileName, {from: themePath}) + if (!configurationPath) return {state: 'unconfigured', themePath} + + const configuration = await readConfiguration(configurationPath) + const environmentsValue: unknown = configuration.content.environments + if (!isObject(environmentsValue)) { + return {state: 'unconfigured', path: configurationPath, themePath} + } + + const environments: TrustedThemeEnvironment[] = [] + const environmentNameByStore = new Map() + + for (const [name, environmentValue] of Object.entries(environmentsValue)) { + if (!isObject(environmentValue) || environmentValue.store === undefined) continue + if (typeof environmentValue.store !== 'string') { + throw new ThemeAirlockError( + `Invalid store in ${configurationPath} for environment "${name}": expected a string.`, + 'malformed-configuration', + ) + } + + let store: string + try { + store = normalizeStoreFqdn(environmentValue.store) + } catch (error) { + if (!(error instanceof AbortError)) throw error + throw new ThemeAirlockError( + `Invalid store in ${configurationPath} for environment "${name}": expected a valid Shopify store handle or domain.`, + 'malformed-configuration', + ) + } + + if (environmentNameByStore.has(store)) { + const existingEnvironmentName = environmentNameByStore.get(store) as string + throw new ThemeAirlockError( + `Theme configuration ${configurationPath} maps environments "${existingEnvironmentName}" and "${name}" to the same store ${store}.`, + 'ambiguous-configuration', + ) + } + + environmentNameByStore.set(store, name) + environments.push({name, store}) + } + + if (environments.length === 0) { + return {state: 'unconfigured', path: configurationPath, themePath} + } + + return {state: 'configured', path: configurationPath, themePath, environments} +} diff --git a/packages/theme/src/cli/utilities/theme-airlock/types.ts b/packages/theme/src/cli/utilities/theme-airlock/types.ts new file mode 100644 index 00000000000..2d9b22b7869 --- /dev/null +++ b/packages/theme/src/cli/utilities/theme-airlock/types.ts @@ -0,0 +1,37 @@ +import {AbortError} from '@shopify/cli-kit/node/error' + +export type StoreSelectionSource = + | 'explicit-environment' + | 'explicit-store' + | 'environment-variable' + | 'default' + | 'sole-store' + | 'bootstrap' + +export interface TrustedThemeEnvironment { + name: string + store: string +} + +export type ThemeProjectTrust = + | {state: 'unconfigured'; path?: string; themePath: string} + | {state: 'configured'; path: string; themePath: string; environments: TrustedThemeEnvironment[]} + +export interface AirlockTarget { + environment?: string + store: string + source: StoreSelectionSource + implicit: boolean +} + +export class ThemeAirlockError extends AbortError { + readonly targets: AirlockTarget[] + readonly reason: string + + constructor(message: string, reason: string, targets: AirlockTarget[] = []) { + super(message, 'No files were uploaded.') + this.name = 'ThemeAirlockError' + this.targets = targets + this.reason = reason + } +} From 091d7dc0b72e83696c44e21e778fed4980569dbe Mon Sep 17 00:00:00 2001 From: Nick Hayden Date: Tue, 21 Jul 2026 12:42:36 -0400 Subject: [PATCH 06/11] Resolve trusted Airlock targets --- .../utilities/theme-airlock/resolver.test.ts | 615 ++++++++++++++++++ .../cli/utilities/theme-airlock/resolver.ts | 302 +++++++++ 2 files changed, 917 insertions(+) create mode 100644 packages/theme/src/cli/utilities/theme-airlock/resolver.test.ts create mode 100644 packages/theme/src/cli/utilities/theme-airlock/resolver.ts diff --git a/packages/theme/src/cli/utilities/theme-airlock/resolver.test.ts b/packages/theme/src/cli/utilities/theme-airlock/resolver.test.ts new file mode 100644 index 00000000000..221200c117a --- /dev/null +++ b/packages/theme/src/cli/utilities/theme-airlock/resolver.test.ts @@ -0,0 +1,615 @@ +import {resolveBatchAirlockTargets, resolveSingleAirlockTarget} from './resolver.js' +import {ThemeAirlockError} from './types.js' +import {describe, expect, test, vi} from 'vitest' + +import type {ThemeProjectTrust} from './types.js' + +const configuredTrust: ThemeProjectTrust = { + state: 'configured', + path: '/theme/shopify.theme.toml', + themePath: '/theme', + environments: [ + {name: 'default', store: 'default-store.myshopify.com'}, + {name: 'preview', store: 'preview-store.myshopify.com'}, + {name: 'production', store: 'production-store.myshopify.com'}, + ], +} + +const unconfiguredTrust: ThemeProjectTrust = { + state: 'unconfigured', + themePath: '/theme', +} + +function captureAirlockError(operation: () => unknown): ThemeAirlockError { + try { + operation() + } catch (error) { + expect(error).toBeInstanceOf(ThemeAirlockError) + if (error instanceof ThemeAirlockError) return error + throw error + } + throw new Error('Expected operation to throw') +} + +function resolveSingle({ + trust = configuredTrust, + flags = {}, + argv = [], + env = {}, +}: { + trust?: ThemeProjectTrust + flags?: Record + argv?: string[] + env?: NodeJS.ProcessEnv +} = {}) { + return resolveSingleAirlockTarget({trust, flags, argv, env}) +} + +describe('resolveSingleAirlockTarget', () => { + test.each([ + { + name: 'a long CLI environment with a separate value', + flags: {environment: 'preview'}, + argv: ['--environment', 'preview'], + }, + { + name: 'a short CLI environment with an equals value', + flags: {environment: 'preview'}, + argv: ['-e=preview'], + }, + { + name: 'a short CLI environment with an attached value', + flags: {environment: 'preview'}, + argv: ['-epreview'], + }, + ])('resolves $name', ({flags, argv}) => { + expect(resolveSingle({flags, argv})).toEqual({ + environment: 'preview', + store: 'preview-store.myshopify.com', + source: 'explicit-environment', + implicit: false, + }) + }) + + test('resolves an environment from SHOPIFY_FLAG_ENVIRONMENT', () => { + expect( + resolveSingle({ + flags: {environment: 'production'}, + env: {SHOPIFY_FLAG_ENVIRONMENT: 'production'}, + }), + ).toEqual({ + environment: 'production', + store: 'production-store.myshopify.com', + source: 'explicit-environment', + implicit: false, + }) + }) + + test.each([ + { + name: 'a long CLI store with an equals value', + flags: {store: 'preview-store.myshopify.com'}, + argv: ['--store=preview-store'], + }, + { + name: 'a short CLI store with a separate value', + flags: {store: 'preview-store.myshopify.com'}, + argv: ['-s', 'preview-store'], + }, + { + name: 'a short CLI store with an attached value', + flags: {store: 'preview-store.myshopify.com'}, + argv: ['-spreview-store'], + }, + ])('resolves $name', ({flags, argv}) => { + expect(resolveSingle({flags, argv})).toEqual({ + environment: 'preview', + store: 'preview-store.myshopify.com', + source: 'explicit-store', + implicit: false, + }) + }) + + test('ignores unrelated flags that contain selector names', () => { + expect(resolveSingle({argv: ['--storefront', 'preview-store', '--environmental', 'preview', '-xpreview']})).toEqual( + { + environment: 'default', + store: 'default-store.myshopify.com', + source: 'default', + implicit: true, + }, + ) + }) + + test('resolves a store selected only by SHOPIFY_FLAG_STORE', () => { + expect( + resolveSingle({ + flags: {store: 'preview-store.myshopify.com'}, + env: {SHOPIFY_FLAG_STORE: 'preview-store'}, + }), + ).toEqual({ + environment: 'preview', + store: 'preview-store.myshopify.com', + source: 'environment-variable', + implicit: false, + }) + }) + + test.each([ + { + name: '--store=', + argv: ['--store='], + env: {}, + reason: 'unknown-store' as const, + message: '--store requires a value.', + }, + { + name: "SHOPIFY_FLAG_STORE: ''", + argv: [], + env: {SHOPIFY_FLAG_STORE: ''}, + reason: 'unknown-store' as const, + message: 'SHOPIFY_FLAG_STORE requires a value.', + }, + { + name: '--environment=', + argv: ['--environment='], + env: {}, + reason: 'unknown-environment' as const, + message: '--environment requires a value.', + }, + { + name: "SHOPIFY_FLAG_ENVIRONMENT: ''", + argv: [], + env: {SHOPIFY_FLAG_ENVIRONMENT: ''}, + reason: 'unknown-environment' as const, + message: 'SHOPIFY_FLAG_ENVIRONMENT requires a value.', + }, + ])('rejects an empty explicit selection from $name', ({argv, env, reason, message}) => { + const error = captureAirlockError(() => resolveSingle({argv, env})) + + expect(error.reason).toBe(reason) + expect(error.message).toBe(message) + }) + + test.each([ + { + name: 'repeated long store selectors', + argv: ['--store', 'preview-store', '--store', 'production-store'], + message: 'Multiple --store selections were provided. Provide only one.', + }, + { + name: 'repeated short store selectors', + argv: ['-spreview-store', '-sproduction-store'], + message: 'Multiple --store selections were provided. Provide only one.', + }, + ])('rejects $name', ({argv, message}) => { + const error = captureAirlockError(() => resolveSingle({argv})) + + expect(error.reason).toBe('conflicting-selection') + expect(error.targets).toEqual([]) + expect(error.message).toBe(message) + }) + + test.each([ + { + name: 'repeated long environment selectors', + argv: ['--environment', 'preview', '--environment', 'production'], + message: 'Multiple --environment selections were provided. Provide only one.', + }, + { + name: 'repeated short environment selectors', + argv: ['-epreview', '-eproduction'], + message: 'Multiple --environment selections were provided. Provide only one.', + }, + ])('rejects $name', ({argv, message}) => { + const error = captureAirlockError(() => resolveSingle({argv})) + + expect(error.reason).toBe('conflicting-selection') + expect(error.targets).toEqual([]) + expect(error.message).toBe(message) + }) + + test.each([ + { + name: 'CLI store', + argv: ['--store', 'not a store'], + env: {}, + message: 'Invalid store value for --store: not a store.', + }, + { + name: 'environment-variable store', + argv: [], + env: {SHOPIFY_FLAG_STORE: 'not a store'}, + message: 'Invalid store value for SHOPIFY_FLAG_STORE: not a store.', + }, + ])('rejects a malformed $name', ({argv, env, message}) => { + const error = captureAirlockError(() => resolveSingle({argv, env})) + + expect(error.reason).toBe('invalid-store') + expect(error.targets).toEqual([]) + expect(error.message).toBe(message) + }) + + test('attributes a matching CLI and environment-variable store to the CLI', () => { + expect( + resolveSingle({ + flags: {store: 'preview-store.myshopify.com'}, + argv: ['--store', 'https://PREVIEW-STORE.myshopify.com/admin/'], + env: {SHOPIFY_FLAG_STORE: 'preview-store'}, + }), + ).toEqual({ + environment: 'preview', + store: 'preview-store.myshopify.com', + source: 'explicit-store', + implicit: false, + }) + }) + + test('rejects mismatching CLI and environment-variable stores', () => { + const error = captureAirlockError(() => + resolveSingle({ + flags: {store: 'preview-store.myshopify.com'}, + argv: ['--store', 'preview-store'], + env: {SHOPIFY_FLAG_STORE: 'production-store'}, + }), + ) + + expect(error.reason).toBe('conflicting-selection') + expect(error.targets).toEqual([]) + expect(error.message).toBe( + 'Store selections conflict: --store selects preview-store.myshopify.com, while SHOPIFY_FLAG_STORE selects production-store.myshopify.com.', + ) + }) + + test('rejects conflicting CLI and environment-variable stores before resolving an explicit environment', () => { + const error = captureAirlockError(() => + resolveSingle({ + argv: ['--environment', 'unknown', '--store', 'preview-store'], + env: {SHOPIFY_FLAG_STORE: 'production-store'}, + }), + ) + + expect(error.reason).toBe('conflicting-selection') + expect(error.targets).toEqual([]) + expect(error.message).toBe( + 'Store selections conflict: --store selects preview-store.myshopify.com, while SHOPIFY_FLAG_STORE selects production-store.myshopify.com.', + ) + }) + + test.each([ + { + name: 'a matching CLI store', + argv: ['--environment', 'preview', '--store', 'https://PREVIEW-STORE.myshopify.com/admin/'], + env: {}, + }, + { + name: 'a matching environment-variable store', + argv: ['--environment=preview'], + env: {SHOPIFY_FLAG_STORE: 'preview-store'}, + }, + ])('resolves an explicit environment combined with $name', ({argv, env}) => { + expect(resolveSingle({argv, env})).toEqual({ + environment: 'preview', + store: 'preview-store.myshopify.com', + source: 'explicit-environment', + implicit: false, + }) + }) + + test.each([ + { + name: 'a mismatching CLI store', + argv: ['--environment', 'preview', '--store', 'production-store'], + env: {}, + }, + { + name: 'a mismatching environment-variable store', + argv: ['--environment=preview'], + env: {SHOPIFY_FLAG_STORE: 'production-store'}, + }, + ])('rejects an explicit environment combined with $name', ({argv, env}) => { + const error = captureAirlockError(() => resolveSingle({argv, env})) + + expect(error.reason).toBe('conflicting-selection') + expect(error.targets).toEqual([]) + expect(error.message).toBe( + 'Environment "preview" selects preview-store.myshopify.com, but the store selection resolves to production-store.myshopify.com.', + ) + }) + + test('uses the CLI environment instead of SHOPIFY_FLAG_ENVIRONMENT', () => { + expect( + resolveSingle({ + flags: {environment: 'preview'}, + argv: ['--environment', 'preview'], + env: {SHOPIFY_FLAG_ENVIRONMENT: 'production'}, + }), + ).toEqual({ + environment: 'preview', + store: 'preview-store.myshopify.com', + source: 'explicit-environment', + implicit: false, + }) + }) + + test('resolves the configured default implicitly', () => { + expect(resolveSingle()).toEqual({ + environment: 'default', + store: 'default-store.myshopify.com', + source: 'default', + implicit: true, + }) + }) + + test('resolves the sole trusted store implicitly', () => { + const trust: ThemeProjectTrust = { + state: 'configured', + path: '/theme/shopify.theme.toml', + themePath: '/theme', + environments: [{name: 'preview', store: 'preview-store'}], + } + + expect(resolveSingle({trust})).toEqual({ + environment: 'preview', + store: 'preview-store.myshopify.com', + source: 'sole-store', + implicit: true, + }) + }) + + test.each([{flags: {}}, {flags: {force: true}}, {flags: {yes: true}}])( + 'rejects multiple trusted stores without a default regardless of non-selection flags: $flags', + ({flags}) => { + const trust: ThemeProjectTrust = { + state: 'configured', + path: '/theme/shopify.theme.toml', + themePath: '/theme', + environments: [ + {name: 'preview', store: 'preview-store'}, + {name: 'production', store: 'production-store'}, + ], + } + + const error = captureAirlockError(() => resolveSingle({trust, flags})) + + expect(error.reason).toBe('ambiguous-selection') + expect(error.targets).toEqual([]) + expect(error.message).toBe('Multiple trusted stores are configured. Use --environment or --store to select one.') + }, + ) + + test('rejects an unknown environment', () => { + const error = captureAirlockError(() => + resolveSingle({flags: {environment: 'staging'}, argv: ['--environment=staging']}), + ) + + expect(error.reason).toBe('unknown-environment') + expect(error.targets).toEqual([]) + expect(error.message).toBe('Environment "staging" is not configured for this theme project.') + }) + + test.each([ + { + name: 'CLI store', + flags: {store: 'unknown-store.myshopify.com'}, + argv: ['--store', 'unknown-store'], + env: {}, + source: 'explicit-store' as const, + }, + { + name: 'environment-variable store', + flags: {store: 'unknown-store.myshopify.com'}, + argv: [], + env: {SHOPIFY_FLAG_STORE: 'unknown-store'}, + source: 'environment-variable' as const, + }, + ])('rejects an unknown $name and exposes the attempted target', ({flags, argv, env, source}) => { + const error = captureAirlockError(() => resolveSingle({flags, argv, env})) + + expect(error.reason).toBe('unknown-store') + expect(error.targets).toEqual([ + { + store: 'unknown-store.myshopify.com', + source, + implicit: false, + }, + ]) + expect(error.message).toBe('Store unknown-store.myshopify.com is not configured for this theme project.') + }) + + test.each([ + { + name: 'CLI store', + flags: {store: 'candidate-store.myshopify.com'}, + argv: ['--store', 'candidate-store'], + env: {}, + }, + { + name: 'environment-variable store', + flags: {store: 'candidate-store.myshopify.com'}, + argv: [], + env: {SHOPIFY_FLAG_STORE: 'https://CANDIDATE-STORE.myshopify.com/admin/'}, + }, + ])('returns an untrusted normalized bootstrap candidate from a $name', ({flags, argv, env}) => { + expect(resolveSingle({trust: unconfiguredTrust, flags, argv, env})).toEqual({ + bootstrap: true, + candidate: 'candidate-store.myshopify.com', + allowRememberedCandidate: false, + }) + }) + + test('allows a remembered candidate for an unconfigured bare command without returning one', () => { + expect(resolveSingle({trust: unconfiguredTrust})).toEqual({ + bootstrap: true, + allowRememberedCandidate: true, + }) + }) + + test.each([ + { + name: 'CLI environment', + flags: {environment: 'staging'}, + argv: ['-e', 'staging'], + env: {}, + }, + { + name: 'environment-variable environment', + flags: {environment: 'staging'}, + argv: [], + env: {SHOPIFY_FLAG_ENVIRONMENT: 'staging'}, + }, + ])('proposes an explicit $name during bootstrap', ({flags, argv, env}) => { + expect(resolveSingle({trust: unconfiguredTrust, flags, argv, env})).toEqual({ + bootstrap: true, + proposedEnvironment: 'staging', + allowRememberedCandidate: false, + }) + }) + + test('normalizes selection and configured trust stores before comparing and returning them', () => { + const trust: ThemeProjectTrust = { + state: 'configured', + path: '/theme/shopify.theme.toml', + themePath: '/theme', + environments: [{name: 'preview', store: 'https://PREVIEW-STORE.myshopify.com/admin/'}], + } + + expect( + resolveSingle({ + trust, + flags: {store: 'preview-store.myshopify.com'}, + argv: ['--store', 'preview-store'], + }), + ).toEqual({ + environment: 'preview', + store: 'preview-store.myshopify.com', + source: 'explicit-store', + implicit: false, + }) + }) + + test('uses only the supplied env object', () => { + vi.stubEnv('SHOPIFY_FLAG_STORE', 'production-store') + vi.stubEnv('SHOPIFY_FLAG_ENVIRONMENT', 'production') + + try { + expect(resolveSingle({env: {}})).toEqual({ + environment: 'default', + store: 'default-store.myshopify.com', + source: 'default', + implicit: true, + }) + } finally { + vi.unstubAllEnvs() + } + }) + + test('does not mutate any single-target input', () => { + const trust: ThemeProjectTrust = { + state: 'configured', + path: '/theme/shopify.theme.toml', + themePath: '/theme', + environments: [{name: 'preview', store: 'PREVIEW-STORE'}], + } + const flags = {store: 'preview-store.myshopify.com', force: true, files: ['layout/theme.liquid']} + const argv = ['--store', 'preview-store'] + const env = {SHOPIFY_FLAG_STORE: 'preview-store'} + const originalInputs = structuredClone({trust, flags, argv, env}) + + resolveSingleAirlockTarget({trust, flags, argv, env}) + + expect({trust, flags, argv, env}).toEqual(originalInputs) + }) +}) + +describe('resolveBatchAirlockTargets', () => { + test('resolves multiple environments in request order after normalizing both sides', () => { + const trust: ThemeProjectTrust = { + state: 'configured', + path: '/theme/shopify.theme.toml', + themePath: '/theme', + environments: [ + {name: 'preview', store: 'https://PREVIEW-STORE.myshopify.com/admin/'}, + {name: 'production', store: 'PRODUCTION-STORE'}, + ], + } + const environments = [ + {name: 'production', store: 'https://production-store.myshopify.com/admin/'}, + {name: 'preview', store: 'preview-store'}, + ] + + expect(resolveBatchAirlockTargets({trust, environments})).toEqual([ + { + environment: 'production', + store: 'production-store.myshopify.com', + source: 'explicit-environment', + implicit: false, + }, + { + environment: 'preview', + store: 'preview-store.myshopify.com', + source: 'explicit-environment', + implicit: false, + }, + ]) + }) + + test.each([ + { + name: 'an unknown environment', + environments: [ + {name: 'preview', store: 'preview-store'}, + {name: 'staging', store: 'staging-store'}, + ], + message: 'Invalid batch environment "staging": it is not configured for this theme project.', + }, + { + name: 'a missing requested store', + environments: [{name: 'preview', store: 'preview-store'}, {name: 'production'}], + message: 'Invalid batch environment "production": a store is required.', + }, + { + name: 'a mismatching requested store', + environments: [ + {name: 'preview', store: 'preview-store'}, + {name: 'production', store: 'other-store'}, + ], + message: + 'Invalid batch environment "production": other-store.myshopify.com does not match configured store production-store.myshopify.com.', + }, + ])('rejects the whole batch when it contains $name', ({environments, message}) => { + const error = captureAirlockError(() => resolveBatchAirlockTargets({trust: configuredTrust, environments})) + + expect(error.reason).toBe('invalid-batch') + expect(error.targets).toEqual([]) + expect(error.message).toBe(message) + }) + + test('rejects a batch for an unconfigured project', () => { + const error = captureAirlockError(() => + resolveBatchAirlockTargets({ + trust: unconfiguredTrust, + environments: [{name: 'preview', store: 'preview-store'}], + }), + ) + + expect(error.reason).toBe('unconfigured-project') + expect(error.targets).toEqual([]) + expect(error.message).toBe("Can't resolve batch environments for an unconfigured theme project.") + }) + + test('does not mutate trust or requested environments', () => { + const trust: ThemeProjectTrust = { + state: 'configured', + path: '/theme/shopify.theme.toml', + themePath: '/theme', + environments: [{name: 'preview', store: 'PREVIEW-STORE'}], + } + const environments = [{name: 'preview', store: 'https://preview-store.myshopify.com/admin/'}] + const originalInputs = structuredClone({trust, environments}) + + resolveBatchAirlockTargets({trust, environments}) + + expect({trust, environments}).toEqual(originalInputs) + }) +}) diff --git a/packages/theme/src/cli/utilities/theme-airlock/resolver.ts b/packages/theme/src/cli/utilities/theme-airlock/resolver.ts new file mode 100644 index 00000000000..11e7d7a9e9b --- /dev/null +++ b/packages/theme/src/cli/utilities/theme-airlock/resolver.ts @@ -0,0 +1,302 @@ +import {ThemeAirlockError} from './types.js' +import {normalizeStoreFqdn} from '@shopify/cli-kit/node/context/fqdn' + +import type {AirlockTarget, StoreSelectionSource, ThemeProjectTrust, TrustedThemeEnvironment} from './types.js' + +export type FlagValues = Record + +interface SuppliedValue { + supplied: boolean + value?: string + occurrences: number +} + +interface RawSelections { + cliStore: SuppliedValue + cliEnvironment: SuppliedValue + environmentStore: SuppliedValue + environmentName: SuppliedValue +} + +interface NormalizedTrust { + environmentsByName: Map + environmentsByStore: Map +} + +function argvValue(argv: string[], longName: string, shortName: string): SuppliedValue { + let selection: SuppliedValue = {supplied: false, occurrences: 0} + + for (let index = 0; index < argv.length; index++) { + const argument = argv[index] + if (argument === undefined || argument === '--') break + + let value: string | undefined + let matched = false + if (argument === longName || argument === shortName) { + value = argv[index + 1] + index++ + matched = true + } else { + const matchingName = [longName, shortName].find((name) => argument.startsWith(`${name}=`)) + if (matchingName) { + value = argument.slice(matchingName.length + 1) + matched = true + } else if (argument.startsWith(shortName) && argument.length > shortName.length) { + value = argument.slice(shortName.length) + matched = true + } + } + + if (matched) { + selection = {supplied: true, value, occurrences: selection.occurrences + 1} + } + } + + return selection +} + +function environmentValue(env: NodeJS.ProcessEnv, name: string): SuppliedValue { + const value = env[name] + return value === undefined ? {supplied: false, occurrences: 0} : {supplied: true, value, occurrences: 1} +} + +function flagString(flags: FlagValues, name: string): string | undefined { + const value = flags[name] + return typeof value === 'string' ? value : undefined +} + +function rawSelections(flags: FlagValues, argv: string[], env: NodeJS.ProcessEnv): RawSelections { + const cliStore = argvValue(argv, '--store', '-s') + const cliEnvironment = argvValue(argv, '--environment', '-e') + + return { + cliStore: { + supplied: cliStore.supplied, + value: cliStore.value ?? (cliStore.supplied ? flagString(flags, 'store') : undefined), + occurrences: cliStore.occurrences, + }, + cliEnvironment: { + supplied: cliEnvironment.supplied, + value: cliEnvironment.value ?? (cliEnvironment.supplied ? flagString(flags, 'environment') : undefined), + occurrences: cliEnvironment.occurrences, + }, + environmentStore: environmentValue(env, 'SHOPIFY_FLAG_STORE'), + environmentName: environmentValue(env, 'SHOPIFY_FLAG_ENVIRONMENT'), + } +} + +function normalizeTrust(trust: Extract): NormalizedTrust { + const environmentsByName = new Map() + const environmentsByStore = new Map() + + for (const environment of trust.environments) { + const normalizedEnvironment = {...environment, store: normalizeStoreFqdn(environment.store)} + environmentsByName.set(environment.name, normalizedEnvironment) + environmentsByStore.set(normalizedEnvironment.store, normalizedEnvironment) + } + + return {environmentsByName, environmentsByStore} +} + +function targetForEnvironment( + environment: TrustedThemeEnvironment, + source: StoreSelectionSource, + implicit: boolean, +): AirlockTarget { + return { + environment: environment.name, + store: environment.store, + source, + implicit, + } +} + +function unknownStoreError(store: string, source: StoreSelectionSource): ThemeAirlockError { + return new ThemeAirlockError(`Store ${store} is not configured for this theme project.`, 'unknown-store', [ + {store, source, implicit: false}, + ]) +} + +function invalidStoreError(value: string, source: string): ThemeAirlockError { + return new ThemeAirlockError(`Invalid store value for ${source}: ${value}.`, 'invalid-store') +} + +function assertSelectionHasValue( + selection: SuppliedValue, + source: string, + reason: 'unknown-store' | 'unknown-environment', +): void { + if (!selection.supplied || (selection.value !== undefined && selection.value.length > 0)) return + + throw new ThemeAirlockError(`${source} requires a value.`, reason) +} + +function normalizeSelectedStore(value: string | undefined, sourceName: string): string | undefined { + if (value === undefined) return undefined + + try { + return normalizeStoreFqdn(value) + } catch { + throw invalidStoreError(value, sourceName) + } +} + +function assertNoRepeatedSelection(selection: SuppliedValue, name: string): void { + if (selection.occurrences < 2) return + + throw new ThemeAirlockError(`Multiple ${name} selections were provided. Provide only one.`, 'conflicting-selection') +} + +function assertCompatibleStoreSelections(cliStore?: string, environmentStore?: string): void { + if (cliStore === undefined || environmentStore === undefined || cliStore === environmentStore) return + + throw new ThemeAirlockError( + `Store selections conflict: --store selects ${cliStore}, while SHOPIFY_FLAG_STORE selects ${environmentStore}.`, + 'conflicting-selection', + ) +} + +function selectedEnvironmentName(selections: RawSelections): string | undefined { + return selections.cliEnvironment.supplied ? selections.cliEnvironment.value : selections.environmentName.value +} + +export function resolveSingleAirlockTarget(options: { + trust: ThemeProjectTrust + flags: FlagValues + argv: string[] + env: NodeJS.ProcessEnv +}): + | AirlockTarget + | { + bootstrap: true + candidate?: string + proposedEnvironment?: string + allowRememberedCandidate: boolean + } { + const selections = rawSelections(options.flags, options.argv, options.env) + assertNoRepeatedSelection(selections.cliStore, '--store') + assertNoRepeatedSelection(selections.cliEnvironment, '--environment') + assertSelectionHasValue(selections.cliStore, '--store', 'unknown-store') + assertSelectionHasValue(selections.environmentStore, 'SHOPIFY_FLAG_STORE', 'unknown-store') + + const cliStore = normalizeSelectedStore(selections.cliStore.value, '--store') + const environmentStore = normalizeSelectedStore(selections.environmentStore.value, 'SHOPIFY_FLAG_STORE') + assertCompatibleStoreSelections(cliStore, environmentStore) + + assertSelectionHasValue(selections.cliEnvironment, '--environment', 'unknown-environment') + assertSelectionHasValue(selections.environmentName, 'SHOPIFY_FLAG_ENVIRONMENT', 'unknown-environment') + + const environmentName = selectedEnvironmentName(selections) + const selectedStore = selections.cliStore.supplied ? cliStore : environmentStore + + if (options.trust.state === 'unconfigured') { + return { + bootstrap: true, + ...(selectedStore === undefined ? {} : {candidate: selectedStore}), + ...(environmentName === undefined ? {} : {proposedEnvironment: environmentName}), + allowRememberedCandidate: + !selections.cliStore.supplied && + !selections.environmentStore.supplied && + !selections.cliEnvironment.supplied && + !selections.environmentName.supplied, + } + } + + const {environmentsByName, environmentsByStore} = normalizeTrust(options.trust) + + if (environmentName !== undefined) { + const environment = environmentsByName.get(environmentName) + if (!environment) { + throw new ThemeAirlockError( + `Environment "${environmentName}" is not configured for this theme project.`, + 'unknown-environment', + ) + } + + if (selectedStore !== undefined && selectedStore !== environment.store) { + throw new ThemeAirlockError( + `Environment "${environmentName}" selects ${environment.store}, but the store selection resolves to ${selectedStore}.`, + 'conflicting-selection', + ) + } + + return targetForEnvironment(environment, 'explicit-environment', false) + } + + if (selectedStore !== undefined) { + const source = selections.cliStore.supplied ? 'explicit-store' : 'environment-variable' + const environment = environmentsByStore.get(selectedStore) + if (!environment) throw unknownStoreError(selectedStore, source) + return targetForEnvironment(environment, source, false) + } + + const defaultEnvironment = environmentsByName.get('default') + if (defaultEnvironment) return targetForEnvironment(defaultEnvironment, 'default', true) + + if (environmentsByStore.size === 1) { + const soleEnvironment = environmentsByStore.values().next().value as TrustedThemeEnvironment + return targetForEnvironment(soleEnvironment, 'sole-store', true) + } + + throw new ThemeAirlockError( + 'Multiple trusted stores are configured. Use --environment or --store to select one.', + 'ambiguous-selection', + ) +} + +function normalizeBatchStore(environment: {name: string; store?: string}): string { + if (environment.store === undefined) { + throw new ThemeAirlockError( + `Invalid batch environment "${environment.name}": a store is required.`, + 'invalid-batch', + ) + } + + try { + return normalizeStoreFqdn(environment.store) + } catch { + throw new ThemeAirlockError( + `Invalid batch environment "${environment.name}": ${environment.store} is not a valid store.`, + 'invalid-batch', + ) + } +} + +export function resolveBatchAirlockTargets(options: { + trust: ThemeProjectTrust + environments: {name: string; store?: string}[] +}): AirlockTarget[] { + if (options.trust.state === 'unconfigured') { + throw new ThemeAirlockError( + "Can't resolve batch environments for an unconfigured theme project.", + 'unconfigured-project', + ) + } + + const {environmentsByName} = normalizeTrust(options.trust) + const normalizedRequests = options.environments.map((environment) => ({ + name: environment.name, + store: normalizeBatchStore(environment), + })) + + const validatedEnvironments = normalizedRequests.map((requestedEnvironment) => { + const configuredEnvironment = environmentsByName.get(requestedEnvironment.name) + if (!configuredEnvironment) { + throw new ThemeAirlockError( + `Invalid batch environment "${requestedEnvironment.name}": it is not configured for this theme project.`, + 'invalid-batch', + ) + } + + if (requestedEnvironment.store !== configuredEnvironment.store) { + throw new ThemeAirlockError( + `Invalid batch environment "${requestedEnvironment.name}": ${requestedEnvironment.store} does not match configured store ${configuredEnvironment.store}.`, + 'invalid-batch', + ) + } + + return configuredEnvironment + }) + + return validatedEnvironments.map((environment) => targetForEnvironment(environment, 'explicit-environment', false)) +} From 95ff21933ae321b14b90ec9cfbeb435d414fd5d9 Mon Sep 17 00:00:00 2001 From: Nick Hayden Date: Tue, 21 Jul 2026 14:53:49 -0400 Subject: [PATCH 07/11] Bootstrap trusted theme stores --- packages/theme/package.json | 2 + .../utilities/theme-airlock/bootstrap.test.ts | 304 ++++++++++++++++ .../cli/utilities/theme-airlock/bootstrap.ts | 107 ++++++ .../utilities/theme-airlock/writer.test.ts | 324 ++++++++++++++++++ .../src/cli/utilities/theme-airlock/writer.ts | 157 +++++++++ pnpm-lock.yaml | 6 + 6 files changed, 900 insertions(+) create mode 100644 packages/theme/src/cli/utilities/theme-airlock/bootstrap.test.ts create mode 100644 packages/theme/src/cli/utilities/theme-airlock/bootstrap.ts create mode 100644 packages/theme/src/cli/utilities/theme-airlock/writer.test.ts create mode 100644 packages/theme/src/cli/utilities/theme-airlock/writer.ts diff --git a/packages/theme/package.json b/packages/theme/package.json index 636ac3c3273..cf1dcf14b25 100644 --- a/packages/theme/package.json +++ b/packages/theme/package.json @@ -46,10 +46,12 @@ "@shopify/theme-language-server-node": "2.21.4", "chokidar": "3.6.0", "h3": "1.15.11", + "proper-lockfile": "4.1.2", "yaml": "2.9.0" }, "devDependencies": { "@shopify/theme-hot-reload": "^0.0.22", + "@types/proper-lockfile": "4.1.4", "@vitest/coverage-istanbul": "^3.2.7", "node-stream-zip": "^1.15.0" }, diff --git a/packages/theme/src/cli/utilities/theme-airlock/bootstrap.test.ts b/packages/theme/src/cli/utilities/theme-airlock/bootstrap.test.ts new file mode 100644 index 00000000000..447396a37f5 --- /dev/null +++ b/packages/theme/src/cli/utilities/theme-airlock/bootstrap.test.ts @@ -0,0 +1,304 @@ +import {bootstrapThemeAirlock} from './bootstrap.js' +import {ThemeAirlockError} from './types.js' +import {configurationFileName} from '../../constants.js' +import {describe, expect, test} from 'vitest' +import {fileExists, inTemporaryDirectory, mkdir, readFile, writeFile} from '@shopify/cli-kit/node/fs' +import {joinPath} from '@shopify/cli-kit/node/path' + +interface Session { + token: string +} + +async function createTheme(tmpDir: string): Promise { + const themePath = joinPath(tmpDir, 'theme') + await mkdir(themePath) + return themePath +} + +async function captureError(operation: () => Promise): Promise { + try { + await operation() + } catch (error) { + if (error instanceof Error) return error + throw error + } + throw new Error('Expected operation to throw') +} + +function optionsFor(themePath: string, overrides: Partial>[0]> = {}) { + return { + themePath, + confirmStore: async () => 'trust' as const, + promptStore: async () => 'prompted-store', + promptEnvironment: async () => 'prompted-environment', + authenticate: async (_store: string) => ({token: 'session'}) satisfies Session, + ...overrides, + } +} + +describe('bootstrapThemeAirlock', () => { + test('uses an explicit candidate instead of an unrelated remembered store', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const themePath = await createTheme(tmpDir) + const confirmed: string[] = [] + const remembered = async () => 'remembered-store' + const result = await bootstrapThemeAirlock( + optionsFor(themePath, { + candidate: 'candidate-store', + rememberedStore: 'remembered-store', + confirmStore: async (store) => { + confirmed.push(store) + return 'trust' + }, + promptStore: remembered, + }), + ) + + expect(confirmed).toEqual(['candidate-store']) + expect(result.target).toEqual({ + environment: 'prompted-environment', + store: 'candidate-store.myshopify.com', + source: 'bootstrap', + implicit: false, + }) + }) + }) + + test('continues with a candidate when confirmation trusts it', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const themePath = await createTheme(tmpDir) + const authenticatedStores: string[] = [] + + await bootstrapThemeAirlock( + optionsFor(themePath, { + candidate: 'candidate-store', + confirmStore: async (store) => { + expect(store).toBe('candidate-store') + return 'trust' + }, + authenticate: async (store) => { + authenticatedStores.push(store) + return {token: 'trusted'} + }, + }), + ) + + expect(authenticatedStores).toEqual(['candidate-store.myshopify.com']) + }) + }) + + test('chooses a new store after declining to trust a candidate', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const themePath = await createTheme(tmpDir) + const result = await bootstrapThemeAirlock( + optionsFor(themePath, { + candidate: 'candidate-store', + confirmStore: async () => 'choose', + promptStore: async () => 'chosen-store', + }), + ) + + expect(result.target.store).toBe('chosen-store.myshopify.com') + }) + }) + + test('presents a remembered store only when explicitly supplied', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const themePath = await createTheme(tmpDir) + const confirmed: string[] = [] + await bootstrapThemeAirlock( + optionsFor(themePath, { + rememberedStore: 'remembered-store', + confirmStore: async (store) => { + confirmed.push(store) + return 'trust' + }, + }), + ) + + expect(confirmed).toEqual(['remembered-store']) + }) + }) + + test('prompts for a store when no candidate is supplied', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const themePath = await createTheme(tmpDir) + const prompted: string[] = [] + const result = await bootstrapThemeAirlock( + optionsFor(themePath, { + promptStore: async () => { + prompted.push('store') + return 'prompted-store' + }, + }), + ) + + expect(prompted).toEqual(['store']) + expect(result.target.store).toBe('prompted-store.myshopify.com') + }) + }) + + test.each([ + {name: 'confirmation', overrides: {candidate: 'candidate-store', confirmStore: async () => 'cancel' as const}}, + {name: 'store prompt', overrides: {promptStore: async () => undefined}}, + {name: 'environment prompt', overrides: {promptEnvironment: async () => undefined}}, + ])('cancelling at the $name stage does not authenticate or write', async ({overrides}) => { + await inTemporaryDirectory(async (tmpDir) => { + const themePath = await createTheme(tmpDir) + let authenticated = false + const error = await captureError(() => + bootstrapThemeAirlock( + optionsFor(themePath, { + ...overrides, + authenticate: async () => { + authenticated = true + return {token: 'never'} + }, + }), + ), + ) + + expect(error).toBeInstanceOf(ThemeAirlockError) + expect((error as ThemeAirlockError).reason).toBe('bootstrap-cancelled') + expect(authenticated).toBe(false) + await expect(readFile(joinPath(themePath, configurationFileName))).rejects.toThrow() + }) + }) + + test('rejects an invalid selected store before authentication or writing', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const themePath = await createTheme(tmpDir) + let authenticated = false + const error = await captureError(() => + bootstrapThemeAirlock( + optionsFor(themePath, { + candidate: 'not a store', + authenticate: async () => { + authenticated = true + return {token: 'never'} + }, + }), + ), + ) + + expect(error).toBeInstanceOf(ThemeAirlockError) + expect((error as ThemeAirlockError).reason).toBe('invalid-store') + expect(authenticated).toBe(false) + await expect(readFile(joinPath(themePath, configurationFileName))).rejects.toThrow() + }) + }) + + test('authentication failure does not write configuration', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const themePath = await createTheme(tmpDir) + + await expect( + bootstrapThemeAirlock( + optionsFor(themePath, { + authenticate: async () => { + throw new Error('authentication failed') + }, + }), + ), + ).rejects.toThrow('authentication failed') + await expect(readFile(joinPath(themePath, configurationFileName))).rejects.toThrow() + }) + }) + + test('authenticates before writing and returns the same session', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const themePath = await createTheme(tmpDir) + const session = {token: 'same-session'} + let configurationDuringAuthentication: string | undefined + const result = await bootstrapThemeAirlock( + optionsFor(themePath, { + authenticate: async () => { + await expect(fileExists(joinPath(themePath, configurationFileName))).resolves.toBe(false) + configurationDuringAuthentication = undefined + return session + }, + }), + ) + + expect(configurationDuringAuthentication).toBeUndefined() + expect(result.session).toBe(session) + expect(result.configurationPath).toBe(joinPath(themePath, configurationFileName)) + await expect(readFile(result.configurationPath)).resolves.toContain('[environments.prompted-environment]') + }) + }) + + test('preserves an existing configuration while authenticating', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const themePath = await createTheme(tmpDir) + const configurationPath = joinPath(themePath, configurationFileName) + const original = '[environments.default]\nstore = "existing-store"\n' + await writeFile(configurationPath, original) + + await bootstrapThemeAirlock( + optionsFor(themePath, { + candidate: 'new-store', + authenticate: async () => { + await expect(readFile(configurationPath)).resolves.toBe(original) + return {token: 'session'} + }, + promptEnvironment: async () => 'preview', + }), + ) + + await expect(readFile(configurationPath)).resolves.toContain('preview.store = "new-store.myshopify.com"') + }) + }) + + test('does not prompt for environment when a proposed environment is supplied', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const themePath = await createTheme(tmpDir) + let promptedForStore = false + let promptedForEnvironment = false + const result = await bootstrapThemeAirlock( + optionsFor(themePath, { + proposedEnvironment: 'explicit-environment', + promptStore: async () => { + promptedForStore = true + return 'explicit-store' + }, + promptEnvironment: async () => { + promptedForEnvironment = true + return 'wrong-environment' + }, + }), + ) + + expect(promptedForStore).toBe(true) + expect(promptedForEnvironment).toBe(false) + expect(result.target.environment).toBe('explicit-environment') + }) + }) + + test('propagates writer conflicts after authentication without changing configuration', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const themePath = await createTheme(tmpDir) + const configurationPath = joinPath(themePath, configurationFileName) + const original = '[environments.preview]\nstore = "existing-store"\n' + await writeFile(configurationPath, original) + let authenticated = false + + const error = await captureError(() => + bootstrapThemeAirlock( + optionsFor(themePath, { + candidate: 'new-store', + proposedEnvironment: 'preview', + authenticate: async () => { + authenticated = true + return {token: 'session'} + }, + }), + ), + ) + + expect(authenticated).toBe(true) + expect(error).toBeInstanceOf(ThemeAirlockError) + expect((error as ThemeAirlockError).reason).toBe('environment-conflict') + await expect(readFile(configurationPath)).resolves.toBe(original) + }) + }) +}) diff --git a/packages/theme/src/cli/utilities/theme-airlock/bootstrap.ts b/packages/theme/src/cli/utilities/theme-airlock/bootstrap.ts new file mode 100644 index 00000000000..989f1ba9357 --- /dev/null +++ b/packages/theme/src/cli/utilities/theme-airlock/bootstrap.ts @@ -0,0 +1,107 @@ +import {addTrustedThemeEnvironment} from './writer.js' +import {ThemeAirlockError} from './types.js' +import {normalizeStoreFqdn} from '@shopify/cli-kit/node/context/fqdn' +import {renderSelectPrompt, renderTextPrompt} from '@shopify/cli-kit/node/ui' + +import type {AirlockTarget} from './types.js' + +type BootstrapUI = Pick, 'confirmStore' | 'promptStore' | 'promptEnvironment'> + +export interface BootstrapOptions { + themePath: string + candidate?: string + rememberedStore?: string + proposedEnvironment?: string + confirmStore: (store: string) => Promise<'trust' | 'choose' | 'cancel'> + promptStore: () => Promise + promptEnvironment: () => Promise + authenticate: (store: string) => Promise +} + +function bootstrapCancelled(message: string): ThemeAirlockError { + return new ThemeAirlockError(message, 'bootstrap-cancelled') +} + +function hasValue(value: string | undefined): value is string { + return value !== undefined && value.trim().length > 0 +} + +function normalizeBootstrapStore(store: string): string { + try { + return normalizeStoreFqdn(store) + } catch { + throw new ThemeAirlockError(`Invalid store value during theme bootstrap: ${store}.`, 'invalid-store') + } +} + +function requiredBootstrapValue(value: string | undefined, message: string): string { + if (hasValue(value)) return value + throw bootstrapCancelled(message) +} + +export function interactiveBootstrapUI(): BootstrapUI { + return { + confirmStore: async (store) => + renderSelectPrompt({ + message: `The store ${store} is untrusted. Choose how to continue.`, + choices: [ + {label: `Trust ${store}`, value: 'trust' as const}, + {label: 'Choose a different store', value: 'choose' as const}, + {label: 'Cancel', value: 'cancel' as const}, + ], + }), + promptStore: async () => { + const store = await renderTextPrompt({message: 'Enter the Shopify store to trust'}) + return hasValue(store) ? store : undefined + }, + promptEnvironment: async () => { + const environment = await renderTextPrompt({message: 'Enter a name for this theme environment'}) + return hasValue(environment) ? environment : undefined + }, + } +} + +export async function bootstrapThemeAirlock( + options: BootstrapOptions, +): Promise<{target: AirlockTarget; session: TSession; configurationPath: string}> { + let selectedStore: string | undefined + const suppliedStore = options.candidate ?? options.rememberedStore + + if (suppliedStore === undefined) { + selectedStore = await options.promptStore() + } else { + const confirmation = await options.confirmStore(suppliedStore) + if (confirmation === 'cancel') { + throw bootstrapCancelled('Theme bootstrap was cancelled while selecting a store.') + } + selectedStore = confirmation === 'trust' ? suppliedStore : await options.promptStore() + } + + const selectedStoreValue = requiredBootstrapValue( + selectedStore, + 'Theme bootstrap was cancelled without selecting a store.', + ) + const normalizedStore = normalizeBootstrapStore(selectedStoreValue) + const environment = requiredBootstrapValue( + options.proposedEnvironment ?? (await options.promptEnvironment()), + 'Theme bootstrap was cancelled without selecting an environment.', + ) + + const session = await options.authenticate(normalizedStore) + const configuration = await addTrustedThemeEnvironment({ + themePath: options.themePath, + environment, + store: normalizedStore, + }) + + return { + target: { + environment, + store: configuration.store, + source: 'bootstrap', + implicit: false, + }, + session, + configurationPath: configuration.path, + } +} diff --git a/packages/theme/src/cli/utilities/theme-airlock/writer.test.ts b/packages/theme/src/cli/utilities/theme-airlock/writer.test.ts new file mode 100644 index 00000000000..c0facfe06d4 --- /dev/null +++ b/packages/theme/src/cli/utilities/theme-airlock/writer.test.ts @@ -0,0 +1,324 @@ +import {addTrustedThemeEnvironment} from './writer.js' +import {loadThemeProjectTrust} from './config.js' +import {ThemeAirlockError} from './types.js' +import {configurationFileName} from '../../constants.js' +import {describe, expect, test, vi} from 'vitest' +import {chmod, inTemporaryDirectory, mkdir, readFile, readdir, writeFile} from '@shopify/cli-kit/node/fs' +import {joinPath} from '@shopify/cli-kit/node/path' +import {rename as fsRename} from 'node:fs/promises' + +vi.mock('node:fs/promises', async (importOriginal) => { + const actual = await importOriginal() + return {...actual, rename: vi.fn(actual.rename)} +}) + +async function createTheme(tmpDir: string): Promise { + const themePath = joinPath(tmpDir, 'theme') + await mkdir(themePath) + return themePath +} + +async function writeConfiguration(directory: string, content: string): Promise { + const configurationPath = joinPath(directory, configurationFileName) + await writeFile(configurationPath, content) + return configurationPath +} + +async function captureError(operation: () => Promise): Promise { + try { + await operation() + } catch (error) { + if (error instanceof Error) return error + throw error + } + throw new Error('Expected operation to throw') +} + +async function coordinateConcurrentRenames(): Promise { + const renameSpy = vi.mocked(fsRename) + const {rename: originalRename} = await vi.importActual('node:fs/promises') + let renameCount = 0 + let releaseSecondRename!: () => void + const secondRename = new Promise((resolve) => { + releaseSecondRename = resolve + }) + renameSpy.mockImplementation(async (...arguments_: Parameters) => { + renameCount += 1 + if (renameCount === 2) releaseSecondRename() + if (renameCount === 1) { + await Promise.race([secondRename, new Promise((resolve) => setTimeout(resolve, 250))]) + } + return originalRename(...arguments_) + }) +} + +describe('addTrustedThemeEnvironment', () => { + test('creates a new configuration in the theme directory', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const themePath = await createTheme(tmpDir) + + await expect( + addTrustedThemeEnvironment({themePath, environment: 'preview', store: 'preview-store'}), + ).resolves.toEqual({path: joinPath(themePath, configurationFileName), store: 'preview-store.myshopify.com'}) + + await expect(readFile(joinPath(themePath, configurationFileName))).resolves.toContain( + '[environments.preview]\nstore = "preview-store.myshopify.com"', + ) + }) + }) + + test('patches the nearest existing ancestor configuration', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const projectPath = joinPath(tmpDir, 'project') + const themePath = joinPath(projectPath, 'themes', 'dawn') + await mkdir(themePath) + const configurationPath = await writeConfiguration(projectPath, 'name = "project"\n') + + const result = await addTrustedThemeEnvironment({themePath, environment: 'preview', store: 'preview-store'}) + + expect(result.path).toBe(configurationPath) + await expect(readFile(configurationPath)).resolves.toContain('[environments.preview]') + await expect(readdir(joinPath(themePath))).resolves.not.toContain(configurationFileName) + }) + }) + + test('patches an existing valid configuration without environments', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const themePath = await createTheme(tmpDir) + const original = '# Keep this comment\nname = "theme"\n\n[other]\nvalue = true\n' + const configurationPath = await writeConfiguration(tmpDir, original) + + await addTrustedThemeEnvironment({themePath, environment: 'preview', store: 'preview-store'}) + + const updated = await readFile(configurationPath) + expect(updated).toContain('# Keep this comment') + expect(updated).toContain('name = "theme"') + expect(updated).toContain('[other]\nvalue = true') + expect(updated).toContain('[environments.preview]\nstore = "preview-store.myshopify.com"') + }) + }) + + test('preserves comments, unrelated keys, and ordering while patching', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const themePath = await createTheme(tmpDir) + const original = [ + '# Header', + 'name = "theme"', + '', + '[other]', + 'value = "before"', + '', + '# Existing environment', + '[environments.default]', + 'store = "default-store"', + '', + ].join('\n') + const configurationPath = await writeConfiguration(tmpDir, original) + + await addTrustedThemeEnvironment({themePath, environment: 'preview', store: 'preview-store'}) + + const expected = [ + '# Header', + 'name = "theme"', + '', + '[other]', + 'value = "before"', + '', + '[environments]', + 'preview.store = "preview-store.myshopify.com"', + '', + '# Existing environment', + '[environments.default]', + 'store = "default-store"', + '', + ].join('\n') + + await expect(readFile(configurationPath)).resolves.toBe(expected) + + const trust = await loadThemeProjectTrust(themePath) + expect(trust).toMatchObject({ + state: 'configured', + environments: [ + {name: 'preview', store: 'preview-store.myshopify.com'}, + {name: 'default', store: 'default-store.myshopify.com'}, + ], + }) + }) + }) + + test('normalizes the requested store', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const themePath = await createTheme(tmpDir) + + await expect( + addTrustedThemeEnvironment({ + themePath, + environment: 'preview', + store: 'https://PREVIEW-STORE.myshopify.com/admin/', + }), + ).resolves.toMatchObject({store: 'preview-store.myshopify.com'}) + }) + }) + + test('is idempotent for the same environment and normalized store without writing', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const themePath = await createTheme(tmpDir) + const configurationPath = await writeConfiguration(tmpDir, '[environments.preview]\nstore = "preview-store"\n') + const original = await readFile(configurationPath) + const renameSpy = vi.mocked(fsRename) + + await expect( + addTrustedThemeEnvironment({ + themePath, + environment: 'preview', + store: 'https://PREVIEW-STORE.myshopify.com/admin/', + }), + ).resolves.toEqual({path: configurationPath, store: 'preview-store.myshopify.com'}) + + await expect(readFile(configurationPath)).resolves.toBe(original) + expect(renameSpy).not.toHaveBeenCalled() + }) + }) + + test('rejects an environment name conflict without writing', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const themePath = await createTheme(tmpDir) + const configurationPath = await writeConfiguration(tmpDir, '[environments.preview]\nstore = "other-store"\n') + const original = await readFile(configurationPath) + + const error = await captureError(() => + addTrustedThemeEnvironment({themePath, environment: 'preview', store: 'preview-store'}), + ) + + expect(error).toBeInstanceOf(ThemeAirlockError) + expect((error as ThemeAirlockError).reason).toBe('environment-conflict') + await expect(readFile(configurationPath)).resolves.toBe(original) + }) + }) + + test('rejects a normalized store conflict under another environment without writing', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const themePath = await createTheme(tmpDir) + const configurationPath = await writeConfiguration(tmpDir, '[environments.default]\nstore = "shared-store"\n') + const original = await readFile(configurationPath) + + const error = await captureError(() => + addTrustedThemeEnvironment({themePath, environment: 'preview', store: 'https://SHARED-STORE.myshopify.com/'}), + ) + + expect(error).toBeInstanceOf(ThemeAirlockError) + expect((error as ThemeAirlockError).reason).toBe('store-conflict') + await expect(readFile(configurationPath)).resolves.toBe(original) + }) + }) + + test('rejects invalid requested stores contextually without writing', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const themePath = await createTheme(tmpDir) + const configurationPath = await writeConfiguration(tmpDir, 'name = "theme"\n') + const original = await readFile(configurationPath) + + const error = await captureError(() => + addTrustedThemeEnvironment({themePath, environment: 'preview', store: 'not a store'}), + ) + + expect(error).toBeInstanceOf(ThemeAirlockError) + expect((error as ThemeAirlockError).reason).toBe('invalid-store') + await expect(readFile(configurationPath)).resolves.toBe(original) + }) + }) + + test('leaves malformed configuration unchanged', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const themePath = await createTheme(tmpDir) + const original = '[environments.preview\nstore = "broken"\n' + const configurationPath = await writeConfiguration(tmpDir, original) + + await expect( + addTrustedThemeEnvironment({themePath, environment: 'preview', store: 'preview-store'}), + ).rejects.toThrow(configurationPath) + await expect(readFile(configurationPath)).resolves.toBe(original) + }) + }) + + test.skipIf(process.platform === 'win32' || process.getuid?.() === 0)( + 'rejects a read-only existing file', + async () => { + await inTemporaryDirectory(async (tmpDir) => { + const themePath = await createTheme(tmpDir) + const configurationPath = await writeConfiguration(tmpDir, 'name = "theme"\n') + await chmod(configurationPath, 0o444) + const original = await readFile(configurationPath) + + const error = await captureError(() => + addTrustedThemeEnvironment({themePath, environment: 'preview', store: 'preview-store'}), + ) + + expect(error).toBeInstanceOf(ThemeAirlockError) + expect((error as ThemeAirlockError).reason).toBe('permission-denied') + await expect(readFile(configurationPath)).resolves.toBe(original) + }) + }, + ) + + test('serializes concurrent additions to preserve both environments', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const themePath = await createTheme(tmpDir) + await coordinateConcurrentRenames() + + const results = await Promise.all([ + addTrustedThemeEnvironment({themePath, environment: 'preview', store: 'preview-store'}), + addTrustedThemeEnvironment({themePath, environment: 'production', store: 'production-store'}), + ]) + + expect(results).toHaveLength(2) + await expect(loadThemeProjectTrust(themePath)).resolves.toMatchObject({ + state: 'configured', + environments: expect.arrayContaining([ + {name: 'preview', store: 'preview-store.myshopify.com'}, + {name: 'production', store: 'production-store.myshopify.com'}, + ]), + }) + await expect(readdir(themePath)).resolves.not.toContain(`${configurationFileName}.lock`) + }) + }) + + test('revalidates a concurrent environment conflict under the lock', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const themePath = await createTheme(tmpDir) + const configurationPath = await writeConfiguration(tmpDir, 'name = "theme"\n') + await coordinateConcurrentRenames() + + const results = await Promise.allSettled([ + addTrustedThemeEnvironment({themePath, environment: 'preview', store: 'first-store'}), + addTrustedThemeEnvironment({themePath, environment: 'preview', store: 'second-store'}), + ]) + + expect(results.filter(({status}) => status === 'fulfilled')).toHaveLength(1) + const rejected = results.filter(({status}) => status === 'rejected') + expect(rejected).toHaveLength(1) + expect((rejected[0] as PromiseRejectedResult).reason).toMatchObject({reason: 'environment-conflict'}) + await expect(readFile(configurationPath)).resolves.toMatch(/store = "(?:first|second)-store\.myshopify\.com"/) + await expect(readdir(tmpDir)).resolves.not.toContain(`${configurationFileName}.lock`) + }) + }) + + test('cleans up the temporary sibling and preserves the original on native rename failure', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const themePath = await createTheme(tmpDir) + const configurationPath = await writeConfiguration(tmpDir, 'name = "theme"\n') + const original = await readFile(configurationPath) + vi.mocked(fsRename).mockRejectedValueOnce(new Error('injected rename failure')) + + await expect( + addTrustedThemeEnvironment({themePath, environment: 'preview', store: 'preview-store'}), + ).rejects.toThrow('injected rename failure') + + await expect(readFile(configurationPath)).resolves.toBe(original) + const entries = await readdir(tmpDir) + expect( + entries.filter((entry) => entry.startsWith(`${configurationFileName}.`) && entry.endsWith('.tmp')), + ).toEqual([]) + }) + }) +}) diff --git a/packages/theme/src/cli/utilities/theme-airlock/writer.ts b/packages/theme/src/cli/utilities/theme-airlock/writer.ts new file mode 100644 index 00000000000..d2c5b10a78c --- /dev/null +++ b/packages/theme/src/cli/utilities/theme-airlock/writer.ts @@ -0,0 +1,157 @@ +import {loadThemeProjectTrust} from './config.js' +import {ThemeAirlockError} from './types.js' +import {configurationFileName} from '../../constants.js' +import {fileHasWritePermissions} from '@shopify/cli-kit/node/fs' +import {joinPath, dirname, resolvePath} from '@shopify/cli-kit/node/path' +import {normalizeStoreFqdn} from '@shopify/cli-kit/node/context/fqdn' +import {TomlFile} from '@shopify/cli-kit/node/toml/toml-file' +import lockfile from 'proper-lockfile' + +const configurationLockOptions = { + realpath: false, + retries: {retries: 10, factor: 1, minTimeout: 25, maxTimeout: 100}, +} +const maximumConfigurationPathRedirects = 3 + +type TrustedThemeTrust = Awaited> +interface TrustedEnvironmentResult { + path: string + store: string +} +type LockedWriteResult = {kind: 'redirect'; path: string} | {kind: 'written'; value: TrustedEnvironmentResult} + +function normalizeRequestedStore(store: string): string { + try { + return normalizeStoreFqdn(store) + } catch { + throw new ThemeAirlockError(`Invalid store value for theme trust: ${store}.`, 'invalid-store') + } +} + +function conflictError(message: string, reason: 'environment-conflict' | 'store-conflict'): ThemeAirlockError { + return new ThemeAirlockError(message, reason) +} + +async function assertWritable(path: string, description: string): Promise { + if (await fileHasWritePermissions(path)) return + + throw new ThemeAirlockError(`Unable to write ${description} at ${path}: permission denied.`, 'permission-denied') +} + +async function writeTrustedThemeEnvironment(options: { + configurationPath: string + trust: TrustedThemeTrust + environment: string + normalizedStore: string +}): Promise { + const {configurationPath, trust, environment, normalizedStore} = options + + if (trust.state === 'configured') { + const existingEnvironment = trust.environments.find(({name}) => name === environment) + if (existingEnvironment) { + if (existingEnvironment.store === normalizedStore) return {path: configurationPath, store: normalizedStore} + + throw conflictError( + `Environment "${environment}" is already trusted for ${existingEnvironment.store}, not ${normalizedStore}.`, + 'environment-conflict', + ) + } + + const existingStore = trust.environments.find(({store}) => store === normalizedStore) + if (existingStore) { + throw conflictError( + `Store ${normalizedStore} is already trusted under environment "${existingStore.name}".`, + 'store-conflict', + ) + } + } + + const changes = {environments: {[environment]: {store: normalizedStore}}} + + if (trust.path) { + await assertWritable(configurationPath, 'theme configuration') + const configuration = await TomlFile.read(configurationPath) + await configuration.patch(changes) + } else { + await assertWritable(dirname(configurationPath), 'theme configuration directory') + const configuration = new TomlFile(configurationPath, {}) + await configuration.replace(changes) + } + + return {path: configurationPath, store: normalizedStore} +} + +function configurationLockError(configurationPath: string): ThemeAirlockError { + return new ThemeAirlockError( + `Unable to acquire the theme configuration lock at ${configurationPath}.`, + 'configuration-lock-failed', + ) +} + +async function lockAndWriteConfiguration(options: { + themePath: string + configurationPath: string + environment: string + normalizedStore: string +}): Promise { + let releaseLock: (() => Promise) | undefined + try { + try { + releaseLock = await lockfile.lock(options.configurationPath, configurationLockOptions) + } catch { + throw configurationLockError(options.configurationPath) + } + + const freshTrust = await loadThemeProjectTrust(options.themePath) + const freshConfigurationPath = resolvePath(freshTrust.path ?? joinPath(options.themePath, configurationFileName)) + if (freshConfigurationPath !== options.configurationPath) { + return {kind: 'redirect', path: freshConfigurationPath} + } + + const value = await writeTrustedThemeEnvironment({ + configurationPath: options.configurationPath, + trust: freshTrust, + environment: options.environment, + normalizedStore: options.normalizedStore, + }) + return {kind: 'written', value} + } finally { + if (releaseLock) await releaseLock() + } +} + +async function writeWithConfigurationRedirects(options: { + themePath: string + configurationPath: string + environment: string + normalizedStore: string + redirects: number +}): Promise { + const result = await lockAndWriteConfiguration(options) + if (result.kind === 'written') return result.value + if (options.redirects >= maximumConfigurationPathRedirects) throw configurationLockError(result.path) + + return writeWithConfigurationRedirects({ + ...options, + configurationPath: result.path, + redirects: options.redirects + 1, + }) +} + +export async function addTrustedThemeEnvironment(options: { + themePath: string + environment: string + store: string +}): Promise { + const normalizedStore = normalizeRequestedStore(options.store) + const initialTrust = await loadThemeProjectTrust(options.themePath) + const configurationPath = resolvePath(initialTrust.path ?? joinPath(options.themePath, configurationFileName)) + + return writeWithConfigurationRedirects({ + themePath: options.themePath, + configurationPath, + environment: options.environment, + normalizedStore, + redirects: 0, + }) +} diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 90357c9c908..393a008903e 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -704,6 +704,9 @@ importers: h3: specifier: 1.15.11 version: 1.15.11 + proper-lockfile: + specifier: 4.1.2 + version: 4.1.2 yaml: specifier: 2.9.0 version: 2.9.0 @@ -711,6 +714,9 @@ importers: '@shopify/theme-hot-reload': specifier: ^0.0.22 version: 0.0.22 + '@types/proper-lockfile': + specifier: 4.1.4 + version: 4.1.4 '@vitest/coverage-istanbul': specifier: ^3.2.7 version: 3.2.7(vitest@4.1.10) From e7585ef248ce18750a87ed12f72586247437c0b8 Mon Sep 17 00:00:00 2001 From: Nick Hayden Date: Wed, 22 Jul 2026 11:01:39 -0400 Subject: [PATCH 08/11] Harden Airlock trust writes --- .../utilities/theme-airlock/writer.test.ts | 43 ++++++++++++++++++- .../src/cli/utilities/theme-airlock/writer.ts | 35 +++++++++++---- 2 files changed, 68 insertions(+), 10 deletions(-) diff --git a/packages/theme/src/cli/utilities/theme-airlock/writer.test.ts b/packages/theme/src/cli/utilities/theme-airlock/writer.test.ts index c0facfe06d4..4f71294144c 100644 --- a/packages/theme/src/cli/utilities/theme-airlock/writer.test.ts +++ b/packages/theme/src/cli/utilities/theme-airlock/writer.test.ts @@ -3,9 +3,9 @@ import {loadThemeProjectTrust} from './config.js' import {ThemeAirlockError} from './types.js' import {configurationFileName} from '../../constants.js' import {describe, expect, test, vi} from 'vitest' -import {chmod, inTemporaryDirectory, mkdir, readFile, readdir, writeFile} from '@shopify/cli-kit/node/fs' +import {chmod, inTemporaryDirectory, mkdir, readFile, readdir, symlink, writeFile} from '@shopify/cli-kit/node/fs' import {joinPath} from '@shopify/cli-kit/node/path' -import {rename as fsRename} from 'node:fs/promises' +import {rename as fsRename, lstat} from 'node:fs/promises' vi.mock('node:fs/promises', async (importOriginal) => { const actual = await importOriginal() @@ -283,6 +283,45 @@ describe('addTrustedThemeEnvironment', () => { }) }) + test.skipIf(process.platform === 'win32')( + 'serializes distinct symlinked configurations on their shared target', + async () => { + await inTemporaryDirectory(async (tmpDir) => { + const firstThemePath = await createTheme(joinPath(tmpDir, 'first')) + const secondThemePath = await createTheme(joinPath(tmpDir, 'second')) + const targetPath = joinPath(tmpDir, 'shared', configurationFileName) + await mkdir(joinPath(tmpDir, 'shared')) + await writeFile(targetPath, 'name = "shared"\n') + const firstConfigurationPath = joinPath(firstThemePath, configurationFileName) + const secondConfigurationPath = joinPath(secondThemePath, configurationFileName) + await symlink(targetPath, firstConfigurationPath) + await symlink(targetPath, secondConfigurationPath) + + const results = await Promise.all([ + addTrustedThemeEnvironment({themePath: firstThemePath, environment: 'preview', store: 'preview-store'}), + addTrustedThemeEnvironment({ + themePath: secondThemePath, + environment: 'production', + store: 'production-store', + }), + ]) + + expect(results).toEqual([ + {path: firstConfigurationPath, store: 'preview-store.myshopify.com'}, + {path: secondConfigurationPath, store: 'production-store.myshopify.com'}, + ]) + expect((await lstat(firstConfigurationPath)).isSymbolicLink()).toBe(true) + expect((await lstat(secondConfigurationPath)).isSymbolicLink()).toBe(true) + const targetContent = await readFile(targetPath) + expect(targetContent).toContain('preview-store.myshopify.com') + expect(targetContent).toContain('production-store.myshopify.com') + await expect(readdir(joinPath(tmpDir, 'shared'))).resolves.toEqual([configurationFileName]) + await expect(readdir(firstThemePath)).resolves.toEqual([configurationFileName]) + await expect(readdir(secondThemePath)).resolves.toEqual([configurationFileName]) + }) + }, + ) + test('revalidates a concurrent environment conflict under the lock', async () => { await inTemporaryDirectory(async (tmpDir) => { const themePath = await createTheme(tmpDir) diff --git a/packages/theme/src/cli/utilities/theme-airlock/writer.ts b/packages/theme/src/cli/utilities/theme-airlock/writer.ts index d2c5b10a78c..723780e79b6 100644 --- a/packages/theme/src/cli/utilities/theme-airlock/writer.ts +++ b/packages/theme/src/cli/utilities/theme-airlock/writer.ts @@ -1,7 +1,7 @@ import {loadThemeProjectTrust} from './config.js' import {ThemeAirlockError} from './types.js' import {configurationFileName} from '../../constants.js' -import {fileHasWritePermissions} from '@shopify/cli-kit/node/fs' +import {fileHasWritePermissions, fileRealPath} from '@shopify/cli-kit/node/fs' import {joinPath, dirname, resolvePath} from '@shopify/cli-kit/node/path' import {normalizeStoreFqdn} from '@shopify/cli-kit/node/context/fqdn' import {TomlFile} from '@shopify/cli-kit/node/toml/toml-file' @@ -18,7 +18,18 @@ interface TrustedEnvironmentResult { path: string store: string } -type LockedWriteResult = {kind: 'redirect'; path: string} | {kind: 'written'; value: TrustedEnvironmentResult} +type LockedWriteResult = + | {kind: 'redirect'; path: string; displayPath: string} + | {kind: 'written'; value: TrustedEnvironmentResult} + +async function canonicalConfigurationPath(path: string): Promise { + try { + return await fileRealPath(path) + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') return path + throw error + } +} function normalizeRequestedStore(store: string): string { try { @@ -40,16 +51,17 @@ async function assertWritable(path: string, description: string): Promise async function writeTrustedThemeEnvironment(options: { configurationPath: string + displayPath: string trust: TrustedThemeTrust environment: string normalizedStore: string }): Promise { - const {configurationPath, trust, environment, normalizedStore} = options + const {configurationPath, displayPath, trust, environment, normalizedStore} = options if (trust.state === 'configured') { const existingEnvironment = trust.environments.find(({name}) => name === environment) if (existingEnvironment) { - if (existingEnvironment.store === normalizedStore) return {path: configurationPath, store: normalizedStore} + if (existingEnvironment.store === normalizedStore) return {path: displayPath, store: normalizedStore} throw conflictError( `Environment "${environment}" is already trusted for ${existingEnvironment.store}, not ${normalizedStore}.`, @@ -78,7 +90,7 @@ async function writeTrustedThemeEnvironment(options: { await configuration.replace(changes) } - return {path: configurationPath, store: normalizedStore} + return {path: displayPath, store: normalizedStore} } function configurationLockError(configurationPath: string): ThemeAirlockError { @@ -91,6 +103,7 @@ function configurationLockError(configurationPath: string): ThemeAirlockError { async function lockAndWriteConfiguration(options: { themePath: string configurationPath: string + displayPath: string environment: string normalizedStore: string }): Promise { @@ -103,13 +116,15 @@ async function lockAndWriteConfiguration(options: { } const freshTrust = await loadThemeProjectTrust(options.themePath) - const freshConfigurationPath = resolvePath(freshTrust.path ?? joinPath(options.themePath, configurationFileName)) + const freshDisplayPath = resolvePath(freshTrust.path ?? joinPath(options.themePath, configurationFileName)) + const freshConfigurationPath = await canonicalConfigurationPath(freshDisplayPath) if (freshConfigurationPath !== options.configurationPath) { - return {kind: 'redirect', path: freshConfigurationPath} + return {kind: 'redirect', path: freshConfigurationPath, displayPath: freshDisplayPath} } const value = await writeTrustedThemeEnvironment({ configurationPath: options.configurationPath, + displayPath: freshDisplayPath, trust: freshTrust, environment: options.environment, normalizedStore: options.normalizedStore, @@ -126,6 +141,7 @@ async function writeWithConfigurationRedirects(options: { environment: string normalizedStore: string redirects: number + displayPath: string }): Promise { const result = await lockAndWriteConfiguration(options) if (result.kind === 'written') return result.value @@ -134,6 +150,7 @@ async function writeWithConfigurationRedirects(options: { return writeWithConfigurationRedirects({ ...options, configurationPath: result.path, + displayPath: result.displayPath, redirects: options.redirects + 1, }) } @@ -145,11 +162,13 @@ export async function addTrustedThemeEnvironment(options: { }): Promise { const normalizedStore = normalizeRequestedStore(options.store) const initialTrust = await loadThemeProjectTrust(options.themePath) - const configurationPath = resolvePath(initialTrust.path ?? joinPath(options.themePath, configurationFileName)) + const displayPath = resolvePath(initialTrust.path ?? joinPath(options.themePath, configurationFileName)) + const configurationPath = await canonicalConfigurationPath(displayPath) return writeWithConfigurationRedirects({ themePath: options.themePath, configurationPath, + displayPath, environment: options.environment, normalizedStore, redirects: 0, From 4ebf2ac454e360e7b8b4be8ed869bd22de3df80e Mon Sep 17 00:00:00 2001 From: Nick Hayden Date: Wed, 22 Jul 2026 10:47:59 -0400 Subject: [PATCH 09/11] Wait for concurrent Airlock writers --- packages/theme/src/cli/utilities/theme-airlock/writer.test.ts | 2 +- packages/theme/src/cli/utilities/theme-airlock/writer.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/theme/src/cli/utilities/theme-airlock/writer.test.ts b/packages/theme/src/cli/utilities/theme-airlock/writer.test.ts index 4f71294144c..9a7a39266c0 100644 --- a/packages/theme/src/cli/utilities/theme-airlock/writer.test.ts +++ b/packages/theme/src/cli/utilities/theme-airlock/writer.test.ts @@ -46,7 +46,7 @@ async function coordinateConcurrentRenames(): Promise { renameCount += 1 if (renameCount === 2) releaseSecondRename() if (renameCount === 1) { - await Promise.race([secondRename, new Promise((resolve) => setTimeout(resolve, 250))]) + await Promise.race([secondRename, new Promise((resolve) => setTimeout(resolve, 350))]) } return originalRename(...arguments_) }) diff --git a/packages/theme/src/cli/utilities/theme-airlock/writer.ts b/packages/theme/src/cli/utilities/theme-airlock/writer.ts index 723780e79b6..cbf8d04a2bd 100644 --- a/packages/theme/src/cli/utilities/theme-airlock/writer.ts +++ b/packages/theme/src/cli/utilities/theme-airlock/writer.ts @@ -9,7 +9,7 @@ import lockfile from 'proper-lockfile' const configurationLockOptions = { realpath: false, - retries: {retries: 10, factor: 1, minTimeout: 25, maxTimeout: 100}, + retries: {retries: 40, factor: 1, minTimeout: 50, maxTimeout: 50}, } const maximumConfigurationPathRedirects = 3 From 8ee3ee2ac8cbb307790739c895fbcdbac64dca62 Mon Sep 17 00:00:00 2001 From: Nick Hayden Date: Wed, 22 Jul 2026 13:55:43 -0400 Subject: [PATCH 10/11] Keep Airlock implementation details private --- .../cli/utilities/theme-airlock/bootstrap.ts | 27 +------------------ .../cli/utilities/theme-airlock/resolver.ts | 2 +- 2 files changed, 2 insertions(+), 27 deletions(-) diff --git a/packages/theme/src/cli/utilities/theme-airlock/bootstrap.ts b/packages/theme/src/cli/utilities/theme-airlock/bootstrap.ts index 989f1ba9357..a4628bd2df3 100644 --- a/packages/theme/src/cli/utilities/theme-airlock/bootstrap.ts +++ b/packages/theme/src/cli/utilities/theme-airlock/bootstrap.ts @@ -1,13 +1,10 @@ import {addTrustedThemeEnvironment} from './writer.js' import {ThemeAirlockError} from './types.js' import {normalizeStoreFqdn} from '@shopify/cli-kit/node/context/fqdn' -import {renderSelectPrompt, renderTextPrompt} from '@shopify/cli-kit/node/ui' import type {AirlockTarget} from './types.js' -type BootstrapUI = Pick, 'confirmStore' | 'promptStore' | 'promptEnvironment'> - -export interface BootstrapOptions { +interface BootstrapOptions { themePath: string candidate?: string rememberedStore?: string @@ -39,28 +36,6 @@ function requiredBootstrapValue(value: string | undefined, message: string): str throw bootstrapCancelled(message) } -export function interactiveBootstrapUI(): BootstrapUI { - return { - confirmStore: async (store) => - renderSelectPrompt({ - message: `The store ${store} is untrusted. Choose how to continue.`, - choices: [ - {label: `Trust ${store}`, value: 'trust' as const}, - {label: 'Choose a different store', value: 'choose' as const}, - {label: 'Cancel', value: 'cancel' as const}, - ], - }), - promptStore: async () => { - const store = await renderTextPrompt({message: 'Enter the Shopify store to trust'}) - return hasValue(store) ? store : undefined - }, - promptEnvironment: async () => { - const environment = await renderTextPrompt({message: 'Enter a name for this theme environment'}) - return hasValue(environment) ? environment : undefined - }, - } -} - export async function bootstrapThemeAirlock( options: BootstrapOptions, ): Promise<{target: AirlockTarget; session: TSession; configurationPath: string}> { diff --git a/packages/theme/src/cli/utilities/theme-airlock/resolver.ts b/packages/theme/src/cli/utilities/theme-airlock/resolver.ts index 11e7d7a9e9b..053fcf9bb85 100644 --- a/packages/theme/src/cli/utilities/theme-airlock/resolver.ts +++ b/packages/theme/src/cli/utilities/theme-airlock/resolver.ts @@ -3,7 +3,7 @@ import {normalizeStoreFqdn} from '@shopify/cli-kit/node/context/fqdn' import type {AirlockTarget, StoreSelectionSource, ThemeProjectTrust, TrustedThemeEnvironment} from './types.js' -export type FlagValues = Record +type FlagValues = Record interface SuppliedValue { supplied: boolean From 97f3a6ccf2fded1664478c76f88dfb387c07f4d4 Mon Sep 17 00:00:00 2001 From: Nick Hayden Date: Thu, 23 Jul 2026 12:13:31 -0400 Subject: [PATCH 11/11] Reject malformed Airlock environment tables --- .../src/cli/utilities/theme-airlock/config.test.ts | 11 +++++------ .../theme/src/cli/utilities/theme-airlock/config.ts | 8 +++++++- .../src/cli/utilities/theme-airlock/writer.test.ts | 13 +++++++++++++ 3 files changed, 25 insertions(+), 7 deletions(-) diff --git a/packages/theme/src/cli/utilities/theme-airlock/config.test.ts b/packages/theme/src/cli/utilities/theme-airlock/config.test.ts index b8fe456b303..2cfc2098c7a 100644 --- a/packages/theme/src/cli/utilities/theme-airlock/config.test.ts +++ b/packages/theme/src/cli/utilities/theme-airlock/config.test.ts @@ -48,17 +48,16 @@ describe('loadThemeProjectTrust', () => { }) test.each(['environments = "preview"\n', 'environments = []\n'])( - 'returns unconfigured when environments is not an object: %s', + 'throws when environments is not a table: %s', async (content) => { await inTemporaryDirectory(async (tmpDir) => { const themePath = joinPath(tmpDir, 'theme') - const configurationPath = await writeConfiguration(tmpDir, content) + await writeConfiguration(tmpDir, content) await mkdir(themePath) - await expect(loadThemeProjectTrust(themePath)).resolves.toEqual({ - state: 'unconfigured', - path: configurationPath, - themePath, + await expect(loadThemeProjectTrust(themePath)).rejects.toMatchObject({ + reason: 'malformed-configuration', + message: expect.stringContaining('environments'), }) }) }, diff --git a/packages/theme/src/cli/utilities/theme-airlock/config.ts b/packages/theme/src/cli/utilities/theme-airlock/config.ts index 19d3164d505..520697d1287 100644 --- a/packages/theme/src/cli/utilities/theme-airlock/config.ts +++ b/packages/theme/src/cli/utilities/theme-airlock/config.ts @@ -31,9 +31,15 @@ export async function loadThemeProjectTrust(themePath: string): Promise() diff --git a/packages/theme/src/cli/utilities/theme-airlock/writer.test.ts b/packages/theme/src/cli/utilities/theme-airlock/writer.test.ts index 9a7a39266c0..4aee0917a39 100644 --- a/packages/theme/src/cli/utilities/theme-airlock/writer.test.ts +++ b/packages/theme/src/cli/utilities/theme-airlock/writer.test.ts @@ -228,6 +228,19 @@ describe('addTrustedThemeEnvironment', () => { }) }) + test('rejects a non-table environments value without modifying the configuration', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const themePath = await createTheme(tmpDir) + const original = '# important note\nenvironments = ["a", "b"]\nname = "keepme"\n' + const configurationPath = await writeConfiguration(tmpDir, original) + + await expect( + addTrustedThemeEnvironment({themePath, environment: 'preview', store: 'preview-store'}), + ).rejects.toMatchObject({reason: 'malformed-configuration'}) + await expect(readFile(configurationPath)).resolves.toBe(original) + }) + }) + test('leaves malformed configuration unchanged', async () => { await inTemporaryDirectory(async (tmpDir) => { const themePath = await createTheme(tmpDir)