From 2943202170a2971257553168820aea68e0640957 Mon Sep 17 00:00:00 2001 From: synsoftworks Date: Wed, 15 Apr 2026 13:36:18 -0700 Subject: [PATCH] fix(store): harden persisted saved-role loading Validate persisted saved-role data at load time so malformed or partially corrupted role files fail clearly and consistently. Also: - add `InvalidSavedRoleDataError` in `src/adapters/role-store.ts` - reject invalid JSON in the saved roles file - reject the wrong top-level shape - reject malformed role entries and bad field types - reject invalid persisted role names - fail the entire load on mixed valid/invalid entries - keep the user-facing error message consistent: `saved role data is invalid; fix or recreate the roles file` - add adapter and CLI coverage for the invalid persisted-data cases Verified with: - npm run build - npm test - npm run test:e2e - npm run test:release --- src/adapters/role-store.ts | 83 ++++++++++++++++++++++++++++++++++++-- test/cli.test.ts | 49 +++++++++++++++++++++- test/role-store.test.ts | 68 ++++++++++++++++++++++++++++++- 3 files changed, 194 insertions(+), 6 deletions(-) diff --git a/src/adapters/role-store.ts b/src/adapters/role-store.ts index 8bc442e..4aabde4 100644 --- a/src/adapters/role-store.ts +++ b/src/adapters/role-store.ts @@ -11,11 +11,21 @@ interface StoredRoles { roles: Role[]; } +const invalidSavedRoleDataMessage = + 'saved role data is invalid; fix or recreate the roles file'; + export interface RoleStoreOptions { configFilePath?: string; env?: NodeJS.ProcessEnv; } +export class InvalidSavedRoleDataError extends Error { + constructor() { + super(invalidSavedRoleDataMessage); + this.name = 'InvalidSavedRoleDataError'; + } +} + export class FileRoleStore { private readonly configFilePath: string; @@ -70,10 +80,15 @@ export class FileRoleStore { await this.ensureFile(); const raw = await readFile(this.configFilePath, 'utf8'); - const parsed = JSON.parse(raw) as Partial; - const roles = Array.isArray(parsed.roles) ? parsed.roles.map(normalizeRole) : []; + let parsed: unknown; - return { roles }; + try { + parsed = JSON.parse(raw); + } catch { + throw new InvalidSavedRoleDataError(); + } + + return parseStoredRoles(parsed); } private async writeData(data: StoredRoles): Promise { @@ -111,3 +126,65 @@ export function resolveRolesFilePath(env: NodeJS.ProcessEnv = process.env): stri return path.join(configHome, 'gitrole', 'roles.json'); } + +function parseStoredRoles(input: unknown): StoredRoles { + if (!isRecord(input) || !Array.isArray(input.roles)) { + throw new InvalidSavedRoleDataError(); + } + + return { + roles: input.roles.map((role) => parseStoredRole(role)) + }; +} + +function parseStoredRole(input: unknown): Role { + if (!isRecord(input)) { + throw new InvalidSavedRoleDataError(); + } + + const role: Role = { + name: readRequiredString(input, 'name'), + fullName: readRequiredString(input, 'fullName'), + email: readRequiredString(input, 'email'), + sshKeyPath: readOptionalString(input, 'sshKeyPath'), + githubUser: readOptionalString(input, 'githubUser'), + githubHost: readOptionalString(input, 'githubHost') + }; + + try { + return normalizeRole(role); + } catch { + throw new InvalidSavedRoleDataError(); + } +} + +function isRecord(input: unknown): input is Record { + return typeof input === 'object' && input !== null && !Array.isArray(input); +} + +function readRequiredString(input: Record, key: string): string { + const value = input[key]; + + if (typeof value !== 'string') { + throw new InvalidSavedRoleDataError(); + } + + return value; +} + +function readOptionalString( + input: Record, + key: string +): string | undefined { + const value = input[key]; + + if (value === undefined) { + return undefined; + } + + if (typeof value !== 'string') { + throw new InvalidSavedRoleDataError(); + } + + return value; +} diff --git a/test/cli.test.ts b/test/cli.test.ts index 9dcedde..3412cf7 100644 --- a/test/cli.test.ts +++ b/test/cli.test.ts @@ -2015,7 +2015,54 @@ process.exit(1); assert.equal(result.stdout, ''); assert.match( result.stderr, - /error: invalid role name "client acme"; use lowercase letters, numbers, "-" or "_"/ + /error: saved role data is invalid; fix or recreate the roles file/ + ); +}); + +test('cli list fails clearly and does not silently skip mixed valid and invalid stored roles', async () => { + const tempDir = await mkdtemp(path.join(os.tmpdir(), 'gitrole-cli-list-invalid-stored-roles-')); + const configHome = path.join(tempDir, 'config'); + + await mkdir(path.join(configHome, 'gitrole'), { recursive: true }); + await writeFile( + path.join(configHome, 'gitrole', 'roles.json'), + JSON.stringify( + { + roles: [ + { + name: 'work', + fullName: 'Alex Developer', + email: 'alex@work.example' + }, + { + name: 'client acme', + fullName: 'Pat Person', + email: 'pat@example.com' + } + ] + }, + null, + 2 + ), + 'utf8' + ); + + const env = { + ...process.env, + HOME: tempDir, + XDG_CONFIG_HOME: configHome + }; + + const result = spawnSync(process.execPath, [cliPath, 'list'], { + encoding: 'utf8', + env + }); + + assert.equal(result.status, 1); + assert.equal(result.stdout, ''); + assert.match( + result.stderr, + /error: saved role data is invalid; fix or recreate the roles file/ ); }); diff --git a/test/role-store.test.ts b/test/role-store.test.ts index 0c39e15..4e80b5f 100644 --- a/test/role-store.test.ts +++ b/test/role-store.test.ts @@ -3,11 +3,15 @@ */ import test from 'node:test'; import assert from 'node:assert/strict'; -import { mkdtemp, readFile, readdir } from 'node:fs/promises'; +import { mkdir, mkdtemp, readFile, readdir, writeFile } from 'node:fs/promises'; import os from 'node:os'; import path from 'node:path'; -import { FileRoleStore, resolveRolesFilePath } from '../src/adapters/role-store.js'; +import { + FileRoleStore, + InvalidSavedRoleDataError, + resolveRolesFilePath +} from '../src/adapters/role-store.js'; test('role store adds, lists, gets, and removes roles', async () => { const tempDir = await mkdtemp(path.join(os.tmpdir(), 'gitrole-role-store-')); @@ -79,3 +83,63 @@ test('role store resolves the XDG path', () => { assert.equal(rolesFilePath, '/tmp/config-home/gitrole/roles.json'); }); + +test('role store rejects invalid JSON in the persisted roles file', async () => { + const store = await createStoreWithRawRolesFile('{not valid json'); + + await assert.rejects(() => store.list(), { + name: 'InvalidSavedRoleDataError', + message: 'saved role data is invalid; fix or recreate the roles file' + }); +}); + +test('role store rejects the wrong top-level shape', async () => { + const store = await createStoreWithRawRolesFile( + JSON.stringify([{ name: 'work', fullName: 'Alex Developer', email: 'alex@work.example' }]) + ); + + await assert.rejects(() => store.list(), InvalidSavedRoleDataError); +}); + +test('role store rejects invalid role entry shapes', async () => { + const store = await createStoreWithRawRolesFile( + JSON.stringify({ + roles: [{ name: 'work', fullName: 'Alex Developer' }] + }) + ); + + await assert.rejects(() => store.list(), InvalidSavedRoleDataError); +}); + +test('role store rejects invalid persisted role names', async () => { + const store = await createStoreWithRawRolesFile( + JSON.stringify({ + roles: [{ name: 'client acme', fullName: 'Alex Developer', email: 'alex@work.example' }] + }) + ); + + await assert.rejects(() => store.list(), InvalidSavedRoleDataError); +}); + +test('role store rejects mixed valid and invalid persisted roles', async () => { + const store = await createStoreWithRawRolesFile( + JSON.stringify({ + roles: [ + { name: 'work', fullName: 'Alex Developer', email: 'alex@work.example' }, + { name: 'client acme', fullName: 'Pat Person', email: 'pat@example.com' } + ] + }) + ); + + await assert.rejects(() => store.list(), InvalidSavedRoleDataError); +}); + +async function createStoreWithRawRolesFile(raw: string): Promise { + const tempDir = await mkdtemp(path.join(os.tmpdir(), 'gitrole-role-store-invalid-')); + const configFilePath = path.join(tempDir, 'gitrole', 'roles.json'); + + await mkdir(path.dirname(configFilePath), { recursive: true }); + await writeFile(configFilePath, raw, 'utf8'); + + return new FileRoleStore({ configFilePath }); +}