From ce052dbd7e21b9940ddbba7f34e2feba7e31494a Mon Sep 17 00:00:00 2001 From: Dicha Zelianivan Arkana <51877647+elianiva@users.noreply.github.com> Date: Thu, 21 Aug 2025 16:15:52 +0700 Subject: [PATCH] refactor(storage): better fs check (#7164) * refactor(storage): better fs check * fix(storage): fs check inconsistencies * test(storage): more thorough testing --- src/utils/__tests__/storage.spec.ts | 155 ++++++++++++++++++++++++++++ src/utils/storage.ts | 8 +- 2 files changed, 159 insertions(+), 4 deletions(-) create mode 100644 src/utils/__tests__/storage.spec.ts diff --git a/src/utils/__tests__/storage.spec.ts b/src/utils/__tests__/storage.spec.ts new file mode 100644 index 0000000000..e5e1586dc6 --- /dev/null +++ b/src/utils/__tests__/storage.spec.ts @@ -0,0 +1,155 @@ +import * as vscode from "vscode" + +vi.mock("fs/promises", async () => { + const mod = await import("../../__mocks__/fs/promises") + return (mod as any).default ?? mod +}) + +describe("getStorageBasePath - customStoragePath", () => { + const defaultPath = "/test/global-storage" + + beforeEach(() => { + vi.clearAllMocks() + }) + + afterEach(() => { + vi.restoreAllMocks() + }) + + it("returns the configured custom path when it is writable", async () => { + const customPath = "/test/storage/path" + vi.spyOn(vscode.workspace, "getConfiguration").mockReturnValue({ + get: vi.fn().mockReturnValue(customPath), + } as any) + + const fsPromises = await import("fs/promises") + const { getStorageBasePath } = await import("../storage") + + const result = await getStorageBasePath(defaultPath) + + expect(result).toBe(customPath) + expect((fsPromises as any).mkdir).toHaveBeenCalledWith(customPath, { recursive: true }) + expect((fsPromises as any).access).toHaveBeenCalledWith(customPath, 7) // 7 = R_OK(4) | W_OK(2) | X_OK(1) + }) + + it("falls back to default and shows an error when custom path is not writable", async () => { + const customPath = "/test/storage/unwritable" + + vi.spyOn(vscode.workspace, "getConfiguration").mockReturnValue({ + get: vi.fn().mockReturnValue(customPath), + } as any) + + const showErrorSpy = vi.spyOn(vscode.window, "showErrorMessage").mockResolvedValue(undefined as any) + + const fsPromises = await import("fs/promises") + const { getStorageBasePath } = await import("../storage") + + await (fsPromises as any).mkdir(customPath, { recursive: true }) + + const accessMock = (fsPromises as any).access as ReturnType + accessMock.mockImplementationOnce(async (p: string) => { + if (p === customPath) { + const err: any = new Error("EACCES: permission denied") + err.code = "EACCES" + throw err + } + return Promise.resolve() + }) + + const result = await getStorageBasePath(defaultPath) + + expect(result).toBe(defaultPath) + expect(showErrorSpy).toHaveBeenCalledTimes(1) + const firstArg = showErrorSpy.mock.calls[0][0] + expect(typeof firstArg).toBe("string") + }) + it("returns the default path when customStoragePath is an empty string and does not touch fs", async () => { + vi.spyOn(vscode.workspace, "getConfiguration").mockReturnValue({ + get: vi.fn().mockReturnValue(""), + } as any) + + const fsPromises = await import("fs/promises") + const { getStorageBasePath } = await import("../storage") + + const result = await getStorageBasePath(defaultPath) + + expect(result).toBe(defaultPath) + expect((fsPromises as any).mkdir).not.toHaveBeenCalled() + expect((fsPromises as any).access).not.toHaveBeenCalled() + }) + + it("falls back to default when mkdir fails and does not attempt access", async () => { + const customPath = "/test/storage/failmkdir" + + vi.spyOn(vscode.workspace, "getConfiguration").mockReturnValue({ + get: vi.fn().mockReturnValue(customPath), + } as any) + + const showErrorSpy = vi.spyOn(vscode.window, "showErrorMessage").mockResolvedValue(undefined as any) + + const fsPromises = await import("fs/promises") + const { getStorageBasePath } = await import("../storage") + + const mkdirMock = (fsPromises as any).mkdir as ReturnType + mkdirMock.mockImplementationOnce(async (p: string) => { + if (p === customPath) { + const err: any = new Error("EACCES: permission denied") + err.code = "EACCES" + throw err + } + return Promise.resolve() + }) + + const result = await getStorageBasePath(defaultPath) + + expect(result).toBe(defaultPath) + expect((fsPromises as any).access).not.toHaveBeenCalled() + expect(showErrorSpy).toHaveBeenCalledTimes(1) + }) + + it("passes the correct permission flags (R_OK | W_OK | X_OK) to fs.access", async () => { + const customPath = "/test/storage/path" + vi.spyOn(vscode.workspace, "getConfiguration").mockReturnValue({ + get: vi.fn().mockReturnValue(customPath), + } as any) + + const fsPromises = await import("fs/promises") + const { getStorageBasePath } = await import("../storage") + + await getStorageBasePath(defaultPath) + + const constants = (fsPromises as any).constants + const expectedFlags = constants.R_OK | constants.W_OK | constants.X_OK + + expect((fsPromises as any).access).toHaveBeenCalledWith(customPath, expectedFlags) + }) + + it("falls back when directory is readable but not writable (partial permissions)", async () => { + const customPath = "/test/storage/readonly" + vi.spyOn(vscode.workspace, "getConfiguration").mockReturnValue({ + get: vi.fn().mockReturnValue(customPath), + } as any) + + const showErrorSpy = vi.spyOn(vscode.window, "showErrorMessage").mockResolvedValue(undefined as any) + + const fsPromises = await import("fs/promises") + const { getStorageBasePath } = await import("../storage") + + const accessMock = (fsPromises as any).access as ReturnType + const constants = (fsPromises as any).constants + accessMock.mockImplementationOnce(async (p: string, mode?: number) => { + // Simulate readable (R_OK) but not writable/executable (W_OK | X_OK) + if (p === customPath && mode && mode & (constants.W_OK | constants.X_OK)) { + const err: any = new Error("EACCES: permission denied") + err.code = "EACCES" + throw err + } + return Promise.resolve() + }) + + const result = await getStorageBasePath(defaultPath) + + expect(result).toBe(defaultPath) + expect(showErrorSpy).toHaveBeenCalledTimes(1) + }) +}) diff --git a/src/utils/storage.ts b/src/utils/storage.ts index 8240588794..5125d6cf44 100644 --- a/src/utils/storage.ts +++ b/src/utils/storage.ts @@ -1,6 +1,7 @@ import * as vscode from "vscode" import * as path from "path" import * as fs from "fs/promises" +import { constants as fsConstants } from "fs" import { Package } from "../shared/package" import { t } from "../i18n" @@ -32,10 +33,8 @@ export async function getStorageBasePath(defaultPath: string): Promise { // Ensure custom path exists await fs.mkdir(customStoragePath, { recursive: true }) - // Test if path is writable - const testFile = path.join(customStoragePath, ".write_test") - await fs.writeFile(testFile, "test") - await fs.rm(testFile) + // Check directory write permission without creating temp files + await fs.access(customStoragePath, fsConstants.R_OK | fsConstants.W_OK | fsConstants.X_OK) return customStoragePath } catch (error) { @@ -132,6 +131,7 @@ export async function promptForCustomStoragePath(): Promise { try { // Test if path is accessible await fs.mkdir(result, { recursive: true }) + await fs.access(result, fsConstants.R_OK | fsConstants.W_OK | fsConstants.X_OK) vscode.window.showInformationMessage(t("common:info.custom_storage_path_set", { path: result })) } catch (error) { vscode.window.showErrorMessage(