diff --git a/packages/cli-kit/src/public/node/path.test.ts b/packages/cli-kit/src/public/node/path.test.ts index 4972b3ac276..3f0b1b44c5a 100644 --- a/packages/cli-kit/src/public/node/path.test.ts +++ b/packages/cli-kit/src/public/node/path.test.ts @@ -1,5 +1,5 @@ -import {relativizePath, normalizePath, cwd, sniffForPath, commonParentDirectory} from './path.js' -import {describe, test, expect} from 'vitest' +import {relativizePath, normalizePath, cwd, sniffForPath, commonParentDirectory, sanitizeRelativePath} from './path.js' +import {describe, test, expect, vi} from 'vitest' describe('relativize', () => { test('relativizes the path', () => { @@ -93,3 +93,53 @@ describe('sniffForPath', () => { expect(path).toStrictEqual('/path/to/project') }) }) + +describe('sanitizeRelativePath', () => { + test('strips traversal segments', () => { + const warn = vi.fn() + expect(sanitizeRelativePath('a/../../b', warn)).toBe('b') + expect(warn).toHaveBeenCalled() + }) + + test('strips leading slashes (absolute Unix paths)', () => { + const warn = vi.fn() + expect(sanitizeRelativePath('/etc/passwd', warn)).toBe('etc/passwd') + expect(warn).toHaveBeenCalled() + }) + + test('strips multiple leading slashes', () => { + const warn = vi.fn() + expect(sanitizeRelativePath('//etc/passwd', warn)).toBe('etc/passwd') + expect(warn).toHaveBeenCalled() + }) + + test('strips Windows drive letters', () => { + const warn = vi.fn() + expect(sanitizeRelativePath('C:/Windows/System32', warn)).toBe('Windows/System32') + expect(warn).toHaveBeenCalled() + }) + + test('handles Windows backslashes', () => { + const warn = vi.fn() + expect(sanitizeRelativePath('C:\\Windows\\System32', warn)).toBe('Windows/System32') + expect(warn).toHaveBeenCalled() + }) + + test('collapses internal current directory markers', () => { + const warn = vi.fn() + expect(sanitizeRelativePath('a/./b', warn)).toBe('a/b') + expect(warn).not.toHaveBeenCalled() + }) + + test('returns empty string for empty result', () => { + const warn = vi.fn() + expect(sanitizeRelativePath('..', warn)).toBe('') + expect(warn).toHaveBeenCalled() + }) + + test('returns empty string for single slash', () => { + const warn = vi.fn() + expect(sanitizeRelativePath('/', warn)).toBe('') + expect(warn).toHaveBeenCalled() + }) +}) diff --git a/packages/cli-kit/src/public/node/path.ts b/packages/cli-kit/src/public/node/path.ts index f3721780110..93349a59957 100644 --- a/packages/cli-kit/src/public/node/path.ts +++ b/packages/cli-kit/src/public/node/path.ts @@ -224,13 +224,20 @@ export function sanitizeRelativePath(input: string, warn: (msg: string) => void) if (seg === '..') { stripped = true stack.pop() - } else if (seg !== '.') { + } else if (seg === '.' || seg === '') { + // Skip empty segments (leading/multiple slashes) and current directory markers. + // This prevents absolute paths like '/etc/passwd' from being treated as such. + if (seg === '') stripped = true + } else if (/^[a-zA-Z]:$/.test(seg)) { + // Skip Windows drive letters (e.g. 'C:') to prevent escaping to other drives. + stripped = true + } else { stack.push(seg) } } const result = stack.join('/') if (stripped) { - warn(`Warning: path '${input}' contains '..' traversal — sanitized to '${result || '.'}'\n`) + warn(`Warning: path '${input}' contains traversal or absolute segments — sanitized to '${result || '.'}'\n`) } return result } diff --git a/packages/cli-kit/src/public/node/plugins/multiple-installation-warning.test.ts b/packages/cli-kit/src/public/node/plugins/multiple-installation-warning.test.ts index f20c6cd87ed..6b1b3c42e61 100644 --- a/packages/cli-kit/src/public/node/plugins/multiple-installation-warning.test.ts +++ b/packages/cli-kit/src/public/node/plugins/multiple-installation-warning.test.ts @@ -26,23 +26,23 @@ describe('showMultipleCLIWarningIfNeeded', () => { // Then expect(mockOutput.info()).toMatchInlineSnapshot(` - "╭─ info ───────────────────────────────────────────────────────────────────────╮ - │ │ - │ Two Shopify CLI installations found – using global installation │ - │ │ - │ A global installation (v${CLI_KIT_VERSION}) and a local dependency (v3.70.0) were │ - │ detected. │ - │ We recommend removing the @shopify/cli and @shopify/app dependencies from │ - │ your package.json, unless you want to use different versions across │ - │ multiple apps. │ - │ │ - │ See Shopify CLI documentation. [1] │ - │ │ - ╰──────────────────────────────────────────────────────────────────────────────╯ - [1] https://shopify.dev/docs/apps/build/cli-for-apps#switch-to-a-global-executab - le-or-local-dependency - " - `) + "╭─ info ───────────────────────────────────────────────────────────────────────╮ + │ │ + │ Two Shopify CLI installations found – using global installation │ + │ │ + │ A global installation (v4.0.0) and a local dependency (v3.70.0) were │ + │ detected. │ + │ We recommend removing the @shopify/cli and @shopify/app dependencies from │ + │ your package.json, unless you want to use different versions across │ + │ multiple apps. │ + │ │ + │ See Shopify CLI documentation. [1] │ + │ │ + ╰──────────────────────────────────────────────────────────────────────────────╯ + [1] https://shopify.dev/docs/apps/build/cli-for-apps#switch-to-a-global-executab + le-or-local-dependency + " + `) mockOutput.clear() }) @@ -58,23 +58,23 @@ describe('showMultipleCLIWarningIfNeeded', () => { // Then expect(mockOutput.info()).toMatchInlineSnapshot(` - "╭─ info ───────────────────────────────────────────────────────────────────────╮ - │ │ - │ Two Shopify CLI installations found – using local dependency │ - │ │ - │ A global installation (v3.70.0) and a local dependency (v${CLI_KIT_VERSION}) were │ - │ detected. │ - │ We recommend removing the @shopify/cli and @shopify/app dependencies from │ - │ your package.json, unless you want to use different versions across │ - │ multiple apps. │ - │ │ - │ See Shopify CLI documentation. [1] │ - │ │ - ╰──────────────────────────────────────────────────────────────────────────────╯ - [1] https://shopify.dev/docs/apps/build/cli-for-apps#switch-to-a-global-executab - le-or-local-dependency - " - `) + "╭─ info ───────────────────────────────────────────────────────────────────────╮ + │ │ + │ Two Shopify CLI installations found – using local dependency │ + │ │ + │ A global installation (v3.70.0) and a local dependency (v4.0.0) were │ + │ detected. │ + │ We recommend removing the @shopify/cli and @shopify/app dependencies from │ + │ your package.json, unless you want to use different versions across │ + │ multiple apps. │ + │ │ + │ See Shopify CLI documentation. [1] │ + │ │ + ╰──────────────────────────────────────────────────────────────────────────────╯ + [1] https://shopify.dev/docs/apps/build/cli-for-apps#switch-to-a-global-executab + le-or-local-dependency + " + `) mockOutput.clear() }) @@ -91,23 +91,23 @@ describe('showMultipleCLIWarningIfNeeded', () => { // Then expect(mockOutput.info()).toMatchInlineSnapshot(` - "╭─ info ───────────────────────────────────────────────────────────────────────╮ - │ │ - │ Two Shopify CLI installations found – using global installation │ - │ │ - │ A global installation (v${CLI_KIT_VERSION}) and a local dependency (v3.70.0) were │ - │ detected. │ - │ We recommend removing the @shopify/cli and @shopify/app dependencies from │ - │ your package.json, unless you want to use different versions across │ - │ multiple apps. │ - │ │ - │ See Shopify CLI documentation. [1] │ - │ │ - ╰──────────────────────────────────────────────────────────────────────────────╯ - [1] https://shopify.dev/docs/apps/build/cli-for-apps#switch-to-a-global-executab - le-or-local-dependency - " - `) + "╭─ info ───────────────────────────────────────────────────────────────────────╮ + │ │ + │ Two Shopify CLI installations found – using global installation │ + │ │ + │ A global installation (v4.0.0) and a local dependency (v3.70.0) were │ + │ detected. │ + │ We recommend removing the @shopify/cli and @shopify/app dependencies from │ + │ your package.json, unless you want to use different versions across │ + │ multiple apps. │ + │ │ + │ See Shopify CLI documentation. [1] │ + │ │ + ╰──────────────────────────────────────────────────────────────────────────────╯ + [1] https://shopify.dev/docs/apps/build/cli-for-apps#switch-to-a-global-executab + le-or-local-dependency + " + `) }) test('does not show a warning if there is no local dependency', async () => { diff --git a/packages/e2e/setup/app.ts b/packages/e2e/setup/app.ts index 30499a2c3aa..70ae385e722 100644 --- a/packages/e2e/setup/app.ts +++ b/packages/e2e/setup/app.ts @@ -155,7 +155,7 @@ export async function deployApp( if (ctx.version) args.push('--version', ctx.version) if (ctx.message) args.push('--message', ctx.message) if (ctx.config) args.push('--config', ctx.config) - if (ctx.force ?? true) args.push('--force') + if (ctx.force ?? true) args.push('--allow-updates') if (ctx.noBuild) args.push('--no-build') args.push('--path', ctx.appDir) return ctx.cli.exec(args, {timeout: CLI_TIMEOUT.long}) diff --git a/packages/e2e/tests/smoke-pty.spec.ts b/packages/e2e/tests/smoke-pty.spec.ts index a1053295ede..a268383b818 100644 --- a/packages/e2e/tests/smoke-pty.spec.ts +++ b/packages/e2e/tests/smoke-pty.spec.ts @@ -4,7 +4,7 @@ import {expect} from '@playwright/test' test.describe('PTY smoke test', () => { test('shopify version runs via PTY', async ({cli}) => { const proc = await cli.spawn(['version']) - await proc.waitForOutput('3.') + await proc.waitForOutput('4.') const code = await proc.waitForExit() expect(code, `shopify version (PTY) failed. Output:\n${proc.getOutput()}`).toBe(0) expect(proc.getOutput()).toMatch(/\d+\.\d+\.\d+/)