From cf766fd6f8cc05a68c89f26877131b7a58a2fac6 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Thu, 10 Sep 2026 15:21:24 +1000 Subject: [PATCH] fix(marketplace): prevent corrupt registry data loss --- src/core/marketplace.ts | 53 +++++++++++--- tests/unit/core/marketplace-scope.test.ts | 88 +++++++++++++++++++---- 2 files changed, 119 insertions(+), 22 deletions(-) diff --git a/src/core/marketplace.ts b/src/core/marketplace.ts index e6900deb..df94ed63 100644 --- a/src/core/marketplace.ts +++ b/src/core/marketplace.ts @@ -1,6 +1,15 @@ import { randomUUID } from 'node:crypto'; import { existsSync, lstatSync, realpathSync } from 'node:fs'; -import { mkdir, readFile, readdir, rename, rm, writeFile } from 'node:fs/promises'; +import { + chmod, + mkdir, + readFile, + readdir, + rename, + rm, + stat, + writeFile, +} from 'node:fs/promises'; import { basename, dirname, join, resolve } from 'node:path'; import simpleGit from 'simple-git'; import { getHomeDir } from '../constants.js'; @@ -304,15 +313,19 @@ export function getProjectRegistryPath(workspacePath: string): string { export async function loadRegistryFromPath( registryPath: string, ): Promise { - if (!existsSync(registryPath)) { - return { version: 1, marketplaces: {} }; - } - try { const content = await readFile(registryPath, 'utf-8'); return JSON.parse(content) as MarketplaceRegistry; - } catch { - return { version: 1, marketplaces: {} }; + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') { + return { version: 1, marketplaces: {} }; + } + + const detail = error instanceof Error ? error.message : String(error); + throw new Error( + `Marketplace registry at ${registryPath} is unreadable: ${detail}. Refusing to overwrite it; fix or delete the file to continue.`, + { cause: error }, + ); } } @@ -324,12 +337,32 @@ export async function saveRegistryToPath( registryPath: string, ): Promise { const dir = dirname(registryPath); + await mkdir(dir, { recursive: true }); - if (!existsSync(dir)) { - await mkdir(dir, { recursive: true }); + let mode: number | undefined; + try { + mode = (await stat(registryPath)).mode; + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error; } - await writeFile(registryPath, `${JSON.stringify(registry, null, 2)}\n`); + const temporaryPath = join( + dir, + `.${basename(registryPath)}.${randomUUID()}.tmp`, + ); + try { + await writeFile(temporaryPath, `${JSON.stringify(registry, null, 2)}\n`, { + encoding: 'utf-8', + flag: 'wx', + ...(mode !== undefined && { mode }), + }); + if (mode !== undefined) { + await chmod(temporaryPath, mode); + } + await rename(temporaryPath, registryPath); + } finally { + await rm(temporaryPath, { force: true }).catch(() => {}); + } } /** diff --git a/tests/unit/core/marketplace-scope.test.ts b/tests/unit/core/marketplace-scope.test.ts index c607709e..67c24e44 100644 --- a/tests/unit/core/marketplace-scope.test.ts +++ b/tests/unit/core/marketplace-scope.test.ts @@ -1,5 +1,16 @@ import { describe, it, expect, beforeEach, afterEach, mock } from 'bun:test'; -import { mkdirSync, writeFileSync, rmSync, readFileSync, existsSync } from 'node:fs'; +import { + chmodSync, + closeSync, + existsSync, + mkdirSync, + openSync, + readFileSync, + readdirSync, + rmSync, + statSync, + writeFileSync, +} from 'node:fs'; import { join } from 'node:path'; import { tmpdir } from 'node:os'; import { stubHomeDir } from '../../helpers/env.js'; @@ -91,18 +102,36 @@ describe('scope-aware registry loading and saving', () => { expect(loaded).toEqual({ version: 1, marketplaces: {} }); }); - it('returns empty registry for invalid JSON', async () => { + it('rejects invalid JSON instead of treating the registry as empty', async () => { const registryPath = join(tmpDir, 'bad.json'); writeFileSync(registryPath, 'not valid json {{{'); - const loaded = await loadRegistryFromPath(registryPath); - expect(loaded).toEqual({ version: 1, marketplaces: {} }); + await expect(loadRegistryFromPath(registryPath)).rejects.toThrow( + `Marketplace registry at ${registryPath} is unreadable`, + ); + }); + + it('rejects filesystem read errors instead of treating the registry as absent', async () => { + const registryPath = join(tmpDir, 'directory-registry'); + mkdirSync(registryPath); + + await expect(loadRegistryFromPath(registryPath)).rejects.toThrow( + `Marketplace registry at ${registryPath} is unreadable`, + ); }); }); describe('saveRegistryToPath', () => { - it('writes registry to specified path and creates parent dirs', async () => { - const nestedPath = join(tmpDir, 'deep', 'nested', 'dir', 'marketplaces.json'); + it('atomically replaces an existing registry without leaving temporary files', async () => { + const registryPath = join(tmpDir, 'marketplaces.json'); + const originalContent = JSON.stringify({ + version: 1, + marketplaces: { stale: {} }, + }); + writeFileSync(registryPath, originalContent); + chmodSync(registryPath, 0o660); + const originalMode = statSync(registryPath).mode; + const originalDescriptor = openSync(registryPath, 'r'); const registry: MarketplaceRegistry = { version: 1, marketplaces: { @@ -114,13 +143,31 @@ describe('scope-aware registry loading and saving', () => { }, }; - await saveRegistryToPath(registry, nestedPath); + try { + await saveRegistryToPath(registry, registryPath); + + const content = readFileSync(registryPath, 'utf-8'); + expect(JSON.parse(content)).toEqual(registry); + expect(content.endsWith('\n')).toBe(true); + expect(readFileSync(originalDescriptor, 'utf-8')).toBe(originalContent); + expect(statSync(registryPath).mode).toBe(originalMode); + expect(readdirSync(tmpDir)).toEqual(['marketplaces.json']); + } finally { + closeSync(originalDescriptor); + } + }); + + it('cleans up the temporary file when replacement fails', async () => { + const registryPath = join(tmpDir, 'marketplaces.json'); + mkdirSync(registryPath); + writeFileSync(join(registryPath, 'keep'), 'original'); + + await expect( + saveRegistryToPath({ version: 1, marketplaces: {} }, registryPath), + ).rejects.toThrow(); - expect(existsSync(nestedPath)).toBe(true); - const content = readFileSync(nestedPath, 'utf-8'); - expect(JSON.parse(content)).toEqual(registry); - // Verify trailing newline - expect(content.endsWith('\n')).toBe(true); + expect(readFileSync(join(registryPath, 'keep'), 'utf-8')).toBe('original'); + expect(readdirSync(tmpDir)).toEqual(['marketplaces.json']); }); }); @@ -421,6 +468,23 @@ describe('addMarketplace with scope', () => { expect(existsSync(userRegistryPath)).toBe(false); }); + it('refuses to overwrite a corrupt project registry', async () => { + const localMarketplace = join(tmpProject, 'replacement-marketplace'); + const projectRegistryPath = getProjectRegistryPath(tmpProject); + const corruptContent = '{"version":1,"marketplaces":'; + mkdirSync(localMarketplace); + writeFileSync(projectRegistryPath, corruptContent); + + await expect( + addMarketplace(localMarketplace, undefined, undefined, false, { + scope: 'project', + workspacePath: tmpProject, + }), + ).rejects.toThrow(`Marketplace registry at ${projectRegistryPath} is unreadable`); + + expect(readFileSync(projectRegistryPath, 'utf-8')).toBe(corruptContent); + }); + it('should default to user scope when no scope provided', async () => { // Create a local marketplace directory const localMarketplace = join(testHome, 'default-scope-marketplace');