Address PR review feedback

- Check `..` as a path segment (not prefix) in assertPathWithinAppDir so a sibling named `..cache` isn't false-rejected.
- Round up the displayed size in the bundle limit error via Math.ceil so a size just over the cap can't display as the cap.
- Mock fileSize in the over-limit test to avoid allocating ~101MB in CI memory.
- Move the app-directory boundary check before glob in copy-by-pattern so an out-of-app sourceDir fails fast; preserve missing-dir behavior via fileExists short-circuit.

Author: isaacroldan

packages/app/src/cli/services/build/steps/include-assets/assert-path-within-app.test.ts

@@ -78,6 +78,17 @@ describe('assertPathWithinAppDir', () => {
     })
   })
 
+  test('allows a sibling whose name starts with two dots (e.g. ..cache)', async () => {
+    await inTemporaryDirectory(async (appDir) => {
+      const dotdotDir = joinPath(appDir, '..cache')
+      await mkdir(dotdotDir)
+      await writeFile(joinPath(dotdotDir, 'file.txt'), 'x')
+      await expect(
+        assertPathWithinAppDir(joinPath(dotdotDir, 'file.txt'), appDir, '..cache/file.txt'),
+      ).resolves.toBeUndefined()
+    })
+  })
+
   test('includes the original config value in the error for debuggability', async () => {
     await inTemporaryDirectory(async (parent) => {
       const appDir = joinPath(parent, 'app')

packages/app/src/cli/services/build/steps/include-assets/assert-path-within-app.ts

@@ -24,7 +24,11 @@ export async function assertPathWithinAppDir(
 ): Promise<void> {
   const [realSource, realAppDir] = await Promise.all([fileRealPath(resolvedPath), fileRealPath(appDirectory)])
   const relative = relativePath(realAppDir, realSource)
-  if (relative.startsWith('..') || isAbsolutePath(relative)) {
+  // Check `..` as a path segment, not just a prefix. A sibling-style name like
+  // `..cache` is a valid in-app entry whose relative path begins with `..` but
+  // does not escape.
+  const firstSegment = relative.split(/[/\\]/, 1)[0]
+  if (firstSegment === '..' || isAbsolutePath(relative)) {
     throw new AbortError(
       `Asset path '${configValue}' resolves outside the app directory.`,
       `Asset sources must be inside the app folder. Resolved to: ${realSource}`,

packages/app/src/cli/services/build/steps/include-assets/copy-by-pattern.test.ts

@@ -9,6 +9,7 @@ describe('copyByPattern', () => {
 
   beforeEach(() => {
     mockStdout = {write: vi.fn()}
+    vi.mocked(fs.fileExists).mockResolvedValue(true)
   })
 
   test('copies matched files preserving relative paths', async () => {

packages/app/src/cli/services/build/steps/include-assets/copy-by-pattern.ts

@@ -1,6 +1,6 @@
 import {assertPathWithinAppDir} from './assert-path-within-app.js'
 import {joinPath, dirname, relativePath} from '@shopify/cli-kit/node/path'
-import {glob, copyFile, mkdir} from '@shopify/cli-kit/node/fs'
+import {glob, copyFile, mkdir, fileExists} from '@shopify/cli-kit/node/fs'
 
 /**
  * Pattern strategy: glob-based file selection.
@@ -17,6 +17,15 @@ export async function copyByPattern(
   options: {stdout: NodeJS.WritableStream},
 ): Promise<{filesCopied: number; outputPaths: string[]}> {
   const {sourceDir, outputDir, patterns, ignore, appDirectory, sourceDirConfigValue} = config
+
+  // Validate the boundary up front, before touching the filesystem. Realpath
+  // would throw on a missing sourceDir, so preserve the existing "missing dir
+  // = no files" behavior by short-circuiting first.
+  if (!(await fileExists(sourceDir))) {
+    return {filesCopied: 0, outputPaths: []}
+  }
+  await assertPathWithinAppDir(sourceDir, appDirectory, sourceDirConfigValue)
+
   const files = await glob(patterns, {
     absolute: true,
     cwd: sourceDir,
@@ -26,9 +35,6 @@ export async function copyByPattern(
   if (files.length === 0) {
     return {filesCopied: 0, outputPaths: []}
   }
-  // Realpath on sourceDir would throw on a missing dir; we only get here when
-  // glob found files, so sourceDir exists.
-  await assertPathWithinAppDir(sourceDir, appDirectory, sourceDirConfigValue)
 
   await mkdir(outputDir)
 

packages/app/src/cli/services/bundle.test.ts

@@ -2,7 +2,7 @@ import {writeManifestToBundle, compressBundle, uploadToGCS, BUNDLE_EXCLUSION_PAT
 import {AppInterface} from '../models/app/app.js'
 import {describe, test, expect, vi} from 'vitest'
 import {joinPath} from '@shopify/cli-kit/node/path'
-import {inTemporaryDirectory, mkdir, writeFile, readFile} from '@shopify/cli-kit/node/fs'
+import {inTemporaryDirectory, mkdir, writeFile, readFile, fileSize} from '@shopify/cli-kit/node/fs'
 import {brotliCompress, zip} from '@shopify/cli-kit/node/archiver'
 import {AbortError} from '@shopify/cli-kit/node/error'
 import {fetch} from '@shopify/cli-kit/node/http'
@@ -19,6 +19,11 @@ vi.mock('@shopify/cli-kit/node/http', async (importActual) => {
   return {...actual, fetch: vi.fn()}
 })
 
+vi.mock('@shopify/cli-kit/node/fs', async (importActual) => {
+  const actual: any = await importActual()
+  return {...actual, fileSize: vi.fn(actual.fileSize)}
+})
+
 describe('writeManifestToBundle', () => {
   test('writes manifest.json to the specified directory', async () => {
     await inTemporaryDirectory(async (tmpDir) => {
@@ -222,11 +227,11 @@ describe('uploadToGCS', () => {
 
   test('aborts with a helpful error when the bundle exceeds the 100 MB upload limit', async () => {
     await inTemporaryDirectory(async (tmpDir) => {
-      // Given — write a sparse-ish file larger than 100 MB without allocating it fully
+      // Given — mock the reported size so CI doesn't have to allocate 101 MB on disk.
       const bundlePath = joinPath(tmpDir, 'huge.zip')
-      const oneMb = Buffer.alloc(1024 * 1024, 'x')
-      // 101 MB to exceed the cap
-      await writeFile(bundlePath, Buffer.concat(Array.from({length: 101}, () => oneMb)))
+      await writeFile(bundlePath, 'placeholder')
+      const oneHundredOneMb = 101 * 1024 * 1024
+      vi.mocked(fileSize).mockResolvedValueOnce(oneHundredOneMb).mockResolvedValueOnce(oneHundredOneMb)
       vi.mocked(fetch).mockResolvedValue({} as never)
 
       // When / Then

packages/app/src/cli/services/bundle.ts

@@ -37,7 +37,8 @@ export async function compressBundle(inputDirectory: string, outputPath: string,
 export async function uploadToGCS(signedURL: string, filePath: string) {
   const size = await fileSize(filePath)
   if (size > MAX_BUNDLE_SIZE_BYTES) {
-    const humanSize = `${(size / MEGABYTE).toFixed(2)} MB`
+    // Round up so a size that barely exceeds the cap never displays as the cap.
+    const humanSize = `${(Math.ceil((size / MEGABYTE) * 100) / 100).toFixed(2)} MB`
     throw new AbortError(
       `Your app bundle exceeds the ${MAX_BUNDLE_SIZE_MB} MB upload limit (it is ${humanSize}).`,
       `Check the asset paths in your extension configuration — a misconfigured source can pull in much more than intended. Exclude large files or directories from your bundle, then try again.`,