Fix local storage initialization permission errors

Author: gonzaloriestra

.changeset/local-storage-constructor-permissions.md

@@ -0,0 +1,5 @@
+---
+'@shopify/cli-kit': patch
+---
+
+Show recovery guidance instead of crashing when Shopify CLI cannot create a local storage directory

packages/cli-kit/src/public/node/local-storage.test.ts

@@ -1,8 +1,9 @@
 import {LocalStorage} from './local-storage.js'
-import {inTemporaryDirectory, readFile, writeFile} from './fs.js'
-import {AbortError} from './error.js'
+import {chmod, inTemporaryDirectory, mkdir, readFile, writeFile} from './fs.js'
+import {AbortError, BugError} from './error.js'
 import {joinPath} from './path.js'
 import * as fs from './fs.js'
+import Config from 'conf'
 import {describe, expect, test, vi} from 'vitest'
 
 interface TestSchema {
@@ -96,6 +97,112 @@ describe('storage', () => {
 })
 
 describe('error handling', () => {
+  test('throws AbortError for a structured EPERM during initialization', async () => {
+    await inTemporaryDirectory((cwd) => {
+      // Given
+      const permissionError = Object.assign(new Error('Permission denied'), {
+        code: 'EPERM',
+        syscall: 'mkdir',
+        path: joinPath(cwd, 'shopify-cli-test-nodejs'),
+      })
+      const storeSpy = vi.spyOn(Config.prototype, 'store', 'get').mockImplementationOnce(() => {
+        throw permissionError
+      })
+
+      try {
+        // When
+        const storage = new LocalStorage<TestSchema>({cwd})
+        expect(storage).toBeDefined()
+        expect.fail('Should have thrown')
+      } catch (error) {
+        // Then
+        if (!(error instanceof AbortError)) throw error
+        expect(error.message).toContain('Failed to access local storage (initialize)')
+      } finally {
+        storeSpy.mockRestore()
+      }
+    })
+  })
+
+  test.skipIf(process.platform === 'win32')(
+    'throws AbortError when the config directory cannot be created',
+    async () => {
+      await inTemporaryDirectory(async (cwd) => {
+        // Given
+        const readOnlyDirectory = joinPath(cwd, 'read-only')
+        const configDirectory = joinPath(readOnlyDirectory, 'shopify-cli-test-nodejs')
+        const configPath = joinPath(configDirectory, 'config.json')
+        await mkdir(readOnlyDirectory)
+        await chmod(readOnlyDirectory, 0o555)
+
+        try {
+          // When
+          const storage = new LocalStorage<TestSchema>({cwd: configDirectory})
+          expect(storage).toBeDefined()
+          expect.fail('Should have thrown')
+        } catch (error) {
+          // Then
+          expect(error).toBeInstanceOf(AbortError)
+          if (!(error instanceof AbortError)) throw error
+
+          expect(error.message).toContain('Failed to access local storage (initialize)')
+          const tryMessage = JSON.stringify(error.tryMessage)
+          expect(error.tryMessage).toContainEqual({filePath: readOnlyDirectory})
+          expect(tryMessage).not.toContain(configPath)
+          expect(error.tryMessage).not.toContainEqual({command: `rm -rf ${configDirectory}`})
+        } finally {
+          await chmod(readOnlyDirectory, 0o755)
+        }
+      })
+    },
+  )
+
+  test.skipIf(process.platform === 'win32')('throws AbortError when the config file cannot be read', async () => {
+    await inTemporaryDirectory(async (cwd) => {
+      // Given
+      const configPath = joinPath(cwd, 'config.json')
+      await writeFile(configPath, '{"testValue":"test"}')
+      await chmod(configPath, 0o200)
+
+      try {
+        // When
+        const storage = new LocalStorage<TestSchema>({cwd})
+        expect(storage).toBeDefined()
+        expect.fail('Should have thrown')
+      } catch (error) {
+        // Then
+        expect(error).toBeInstanceOf(AbortError)
+        if (!(error instanceof AbortError)) throw error
+
+        expect(error.message).toContain('Failed to access local storage (initialize)')
+        expect(error.tryMessage).toContainEqual({filePath: configPath})
+        expect(error.tryMessage).toContainEqual({filePath: cwd})
+        expect(JSON.stringify(error.tryMessage)).not.toContain('rm -rf')
+      } finally {
+        await chmod(configPath, 0o600)
+      }
+    })
+  })
+
+  test('preserves initialization errors that are unrelated to permissions', async () => {
+    await inTemporaryDirectory(async (cwd) => {
+      // Given
+      const filePath = joinPath(cwd, 'file')
+      await writeFile(filePath, 'content')
+
+      try {
+        // When
+        const storage = new LocalStorage<TestSchema>({cwd: joinPath(filePath, 'config')})
+        expect(storage).toBeDefined()
+        expect.fail('Should have thrown')
+      } catch (error) {
+        // Then
+        if ((error as NodeJS.ErrnoException).code !== 'ENOTDIR') throw error
+        expect(error).not.toBeInstanceOf(BugError)
+      }
+    })
+  })
+
   test('throws AbortError when file lacks write permissions', async () => {
     await inTemporaryDirectory(async (cwd) => {
       // Given

packages/cli-kit/src/public/node/local-storage.ts

@@ -1,8 +1,28 @@
 import {AbortError, BugError} from './error.js'
-import {fileHasWritePermissions, unixFileIsOwnedByCurrentUser} from './fs.js'
-import {dirname} from './path.js'
+import {fileExistsSync, fileHasWritePermissions, findPathUpSync, unixFileIsOwnedByCurrentUser} from './fs.js'
+import {dirname, resolvePath} from './path.js'
 import {TokenItem} from './ui.js'
 import Config from 'conf'
+import envPaths from 'env-paths'
+
+function isFileSystemPermissionError(error: unknown): error is NodeJS.ErrnoException {
+  if (!(error instanceof Error)) return false
+  const errorCode = (error as NodeJS.ErrnoException).code
+  return errorCode === 'EACCES' || errorCode === 'EPERM'
+}
+
+function configPathFromInitializationError(
+  options: {projectName?: string; cwd?: string},
+  error: NodeJS.ErrnoException,
+): string | undefined {
+  if (typeof error.path === 'string') {
+    return error.syscall === 'mkdir' ? resolvePath(error.path, 'config.json') : resolvePath(error.path)
+  }
+
+  const configDirectory =
+    options.cwd ?? (options.projectName ? envPaths(options.projectName, {suffix: 'nodejs'}).config : undefined)
+  return configDirectory ? resolvePath(configDirectory, 'config.json') : undefined
+}
 
 function deserializeJson<T>(value: string): T {
   // Some Windows editors encode UTF-8 files with a byte order mark, which JSON.parse does not accept.
@@ -19,11 +39,20 @@ export class LocalStorage<T extends Record<string, any>> {
   private readonly config: Config<T>
 
   constructor(options: {projectName?: string; cwd?: string}) {
-    this.config = new Config<T>({
-      ...options,
-      clearInvalidConfig: true,
-      deserialize: deserializeJson<T>,
-    })
+    try {
+      this.config = new Config<T>({
+        ...options,
+        clearInvalidConfig: true,
+        deserialize: deserializeJson<T>,
+      })
+    } catch (error) {
+      if (!isFileSystemPermissionError(error)) throw error
+
+      const configPath = configPathFromInitializationError(options, error)
+      if (configPath) this.handleError(error, 'initialize', configPath)
+
+      throw new AbortError(`Failed to access local storage (initialize): ${error}`)
+    }
   }
 
   /**
@@ -98,40 +127,47 @@ export class LocalStorage<T extends Record<string, any>> {
    *
    * @param error - The error that occurred.
    * @param operation - The operation that failed.
+   * @param configPath - The local storage configuration file path.
    * @throws AbortError if the error is permission-related.
    * @throws BugError if the error is not permission-related.
    */
-  private handleError(error: unknown, operation: string): never {
-    if (this.isPermissionError()) {
-      throw new AbortError(`Failed to access local storage (${operation}): ${error}`, this.tryMessage())
+  private handleError(error: unknown, operation: string, configPath = this.config.path): never {
+    if (isFileSystemPermissionError(error) || this.isPermissionError(configPath)) {
+      throw new AbortError(`Failed to access local storage (${operation}): ${error}`, this.tryMessage(configPath))
     } else {
-      throw new BugError(
-        `Unexpected error while accessing local storage at ${this.config.path} (${operation}): ${error}`,
-      )
+      throw new BugError(`Unexpected error while accessing local storage at ${configPath} (${operation}): ${error}`)
     }
   }
 
-  private isPermissionError(): boolean {
-    const canAccessFile = fileHasWritePermissions(this.config.path)
-    const canAccessFolder = fileHasWritePermissions(dirname(this.config.path))
-    const ownsFile = unixFileIsOwnedByCurrentUser(this.config.path)
+  private isPermissionError(configPath: string): boolean {
+    const canAccessFile = fileHasWritePermissions(configPath)
+    const canAccessFolder = fileHasWritePermissions(dirname(configPath))
+    const ownsFile = unixFileIsOwnedByCurrentUser(configPath)
 
     return !canAccessFile || !canAccessFolder || ownsFile === false
   }
 
-  private tryMessage() {
-    const ownsFile = unixFileIsOwnedByCurrentUser(this.config.path)
-    const ownsFolder = unixFileIsOwnedByCurrentUser(dirname(this.config.path))
+  private tryMessage(configPath: string) {
+    const configDirectory = dirname(configPath)
+    const configDirectoryExists = fileExistsSync(configDirectory)
+    const permissionsPath = configDirectoryExists
+      ? configPath
+      : (findPathUpSync('.', {cwd: configDirectory, type: 'directory'}) ?? configDirectory)
+    const ownsFile = fileExistsSync(configPath) ? unixFileIsOwnedByCurrentUser(configPath) : undefined
+    const ownershipDirectory = configDirectoryExists ? configDirectory : permissionsPath
+    const ownsFolder = unixFileIsOwnedByCurrentUser(ownershipDirectory)
 
-    const message: TokenItem = [`Check that you have write permissions for`, {filePath: this.config.path}]
+    const message: TokenItem = [`Check that you have write permissions for`, {filePath: permissionsPath}]
     if (ownsFile === false || ownsFolder === false) {
       message.push(
         '- The file is owned by a different user. This typically happens when Shopify CLI was previously run with elevated permissions (e.g., sudo).',
       )
     }
 
-    message.push('\n\nTo resolve this, remove the Shopify CLI preferences folder:')
-    message.push({command: `rm -rf ${dirname(this.config.path)}`})
+    if (configDirectoryExists) {
+      message.push('\n\nTo resolve this, remove the Shopify CLI preferences folder:')
+      message.push({filePath: configDirectory})
+    }
 
     return message
   }