diff --git a/docs/architecture/service-filesystem-boundary.json b/docs/architecture/service-filesystem-boundary.json index 39d9fc45..1709615f 100644 --- a/docs/architecture/service-filesystem-boundary.json +++ b/docs/architecture/service-filesystem-boundary.json @@ -1,6 +1,6 @@ { "schemaVersion": 1, - "maximumEntries": 42, + "maximumEntries": 41, "entries": [ { "path": "server/src/services/agent-health-service.ts", @@ -230,12 +230,6 @@ "owner": "#1187", "rationale": "Operational evidence storage migration is tracked in issue #1187." }, - { - "path": "server/src/services/transition-hooks-service.ts", - "category": "authoritative-persistence", - "owner": "#1186", - "rationale": "Coordination-state storage migration is tracked in issue #1186." - }, { "path": "server/src/services/work-product-service.ts", "category": "authoritative-persistence", diff --git a/docs/testing/critical-path-coverage.json b/docs/testing/critical-path-coverage.json index 5d73681a..a523eb88 100644 --- a/docs/testing/critical-path-coverage.json +++ b/docs/testing/critical-path-coverage.json @@ -87,6 +87,7 @@ "src/__tests__/storage/sqlite-workflow-repositories.test.ts", "src/__tests__/storage/sqlite-workflow-run-execution.test.ts", "src/__tests__/task-service-sqlite.test.ts", + "src/__tests__/transition-hooks-config-repository.test.ts", "src/__tests__/work-product-run-launch-manifest.test.ts", "src/__tests__/work-product-schemas.test.ts", "src/__tests__/workflow-definition-repository.test.ts", diff --git a/server/src/__tests__/transition-hooks-config-repository.test.ts b/server/src/__tests__/transition-hooks-config-repository.test.ts new file mode 100644 index 00000000..3f00d2b7 --- /dev/null +++ b/server/src/__tests__/transition-hooks-config-repository.test.ts @@ -0,0 +1,88 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { lstat, mkdir, mkdtemp, rm, symlink, writeFile } from 'node:fs/promises'; +import path from 'node:path'; +import type { TransitionHooksConfig } from '@veritas-kanban/shared'; +import { FileTransitionHooksConfigRepository } from '../storage/transition-hooks-config-repository.js'; + +vi.mock('node:fs/promises', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, lstat: vi.fn(actual.lstat) }; +}); + +function config(enabled = true): TransitionHooksConfig { + return { version: 1, enabled, rules: [] }; +} + +describe('FileTransitionHooksConfigRepository', () => { + let root: string; + let runtimeDir: string; + let repository: FileTransitionHooksConfigRepository; + + beforeEach(async () => { + root = await mkdtemp(path.join(process.cwd(), '.veritas-transition-hooks-config-')); + runtimeDir = path.join(root, 'runtime'); + repository = new FileTransitionHooksConfigRepository(runtimeDir); + }); + + afterEach(async () => { + await rm(root, { recursive: true, force: true }); + }); + + it('reads missing state and atomically replaces configuration', async () => { + await expect(repository.read()).resolves.toBeNull(); + await repository.write(config()); + await expect(repository.read()).resolves.toEqual(config()); + await repository.write(config(false)); + await expect(repository.read()).resolves.toEqual(config(false)); + }); + + it('rejects symbolic links, changed files, and non-file paths', async () => { + await mkdir(runtimeDir, { recursive: true }); + const configFile = path.join(runtimeDir, 'transition-hooks.json'); + const target = path.join(root, 'outside.json'); + await writeFile(target, JSON.stringify(config()), 'utf8'); + await symlink(target, configFile); + await expect(repository.read()).rejects.toThrow(/symbolic link/i); + + await rm(configFile); + await writeFile(configFile, JSON.stringify(config()), 'utf8'); + const actual = await vi.importActual('node:fs/promises'); + vi.mocked(lstat).mockImplementationOnce(async (filePath) => { + const stats = await actual.lstat(filePath); + return Object.assign(Object.create(Object.getPrototypeOf(stats)), stats, { + ino: stats.ino + 1, + }); + }); + await expect(repository.read()).rejects.toThrow(/changed file/i); + + await rm(configFile); + await mkdir(configFile); + await expect(repository.read()).rejects.toThrow(/bounded regular file/i); + }); + + it('rejects symbolic-link directories and oversized configuration', async () => { + const realDirectory = path.join(root, 'real-runtime'); + const linkedDirectory = path.join(root, 'linked-runtime'); + await mkdir(realDirectory); + await symlink(realDirectory, linkedDirectory, 'dir'); + const linkedRepository = new FileTransitionHooksConfigRepository(linkedDirectory); + await expect(linkedRepository.write(config())).rejects.toThrow(/regular directory/i); + + await expect( + repository.write({ + ...config(), + rules: [ + { + id: 'large', + name: 'x'.repeat(4 * 1024 * 1024), + enabled: true, + from: '*', + to: '*', + gates: [], + actions: [], + }, + ], + }) + ).rejects.toThrow(/4 MiB/i); + }); +}); diff --git a/server/src/services/transition-hooks-service.ts b/server/src/services/transition-hooks-service.ts index eac7a43c..e3fcec01 100644 --- a/server/src/services/transition-hooks-service.ts +++ b/server/src/services/transition-hooks-service.ts @@ -8,8 +8,6 @@ * Extends the basic hook-service with sophisticated quality gate logic. */ -import fs from 'fs/promises'; -import path from 'path'; import { createLogger } from '../lib/logger.js'; import { getOutboundIntegrationService } from './outbound-integration-service.js'; import type { Task, TaskStatus } from '@veritas-kanban/shared'; @@ -22,7 +20,7 @@ import type { TransitionValidationResult, } from '@veritas-kanban/shared'; import { DEFAULT_TRANSITION_HOOKS_CONFIG } from '@veritas-kanban/shared'; -import { getRuntimeDir } from '../utils/paths.js'; +import { FileTransitionHooksConfigRepository } from '../storage/transition-hooks-config-repository.js'; const log = createLogger('transition-hooks'); @@ -30,45 +28,29 @@ const log = createLogger('transition-hooks'); // Configuration Storage // --------------------------------------------------------------------------- -const CONFIG_PATH = path.join(getRuntimeDir(), 'transition-hooks.json'); - -let cachedConfig: TransitionHooksConfig | null = null; +const configRepository = new FileTransitionHooksConfigRepository(); /** * Load transition hooks configuration from disk. */ export async function loadTransitionHooksConfig(): Promise { - if (cachedConfig) { - return cachedConfig; - } - - try { - const content = await fs.readFile(CONFIG_PATH, 'utf-8'); - cachedConfig = JSON.parse(content) as TransitionHooksConfig; + const config = await configRepository.read(); + if (config) { log.info( - { enabled: cachedConfig.enabled, ruleCount: cachedConfig.rules.length }, + { enabled: config.enabled, ruleCount: config.rules.length }, 'Loaded transition hooks config' ); - return cachedConfig; - } catch (err: unknown) { - if ((err as NodeJS.ErrnoException).code === 'ENOENT') { - // File doesn't exist, use defaults - cachedConfig = { ...DEFAULT_TRANSITION_HOOKS_CONFIG }; - log.info('Using default transition hooks config'); - return cachedConfig; - } - throw err; + return config; } + log.info('Using default transition hooks config'); + return { ...DEFAULT_TRANSITION_HOOKS_CONFIG }; } /** * Save transition hooks configuration to disk. */ export async function saveTransitionHooksConfig(config: TransitionHooksConfig): Promise { - const dir = path.dirname(CONFIG_PATH); - await fs.mkdir(dir, { recursive: true }); - await fs.writeFile(CONFIG_PATH, JSON.stringify(config, null, 2), 'utf-8'); - cachedConfig = config; + await configRepository.write(config); log.info( { enabled: config.enabled, ruleCount: config.rules.length }, 'Saved transition hooks config' @@ -96,7 +78,7 @@ export async function updateTransitionHooksConfig( * Clear the cached configuration (for testing). */ export function clearConfigCache(): void { - cachedConfig = null; + // Retained for API compatibility. Configuration reads are no longer cached. } // --------------------------------------------------------------------------- @@ -142,9 +124,10 @@ function taskMatchesFilters( export async function findApplicableRules( fromStatus: TaskStatus | undefined, toStatus: TaskStatus, - task: Pick + task: Pick, + loadedConfig?: TransitionHooksConfig ): Promise { - const config = await getTransitionHooksConfig(); + const config = loadedConfig ?? (await getTransitionHooksConfig()); if (!config.enabled) { return []; @@ -259,7 +242,7 @@ export async function validateTransition( } // Find applicable rules - const rules = await findApplicableRules(fromStatus, toStatus, task); + const rules = await findApplicableRules(fromStatus, toStatus, task, config); // Collect all gates from applicable rules const allGates: TransitionGate[] = []; @@ -462,7 +445,7 @@ export async function executePostTransitionActions( } // Find applicable rules - const rules = await findApplicableRules(fromStatus, toStatus, task); + const rules = await findApplicableRules(fromStatus, toStatus, task, config); // Collect all actions from applicable rules const allActions: TransitionAction[] = []; diff --git a/server/src/storage/index.ts b/server/src/storage/index.ts index eea94b0b..b37bf710 100644 --- a/server/src/storage/index.ts +++ b/server/src/storage/index.ts @@ -41,6 +41,10 @@ export type { export { LocalWorkspaceFileRepository } from './workspace-file-repository.js'; export { FileProgressRepository, type ProgressRepository } from './progress-repository.js'; export { FileStatusHistoryStore } from './status-history-repository.js'; +export { + FileTransitionHooksConfigRepository, + type TransitionHooksConfigRepository, +} from './transition-hooks-config-repository.js'; export { FileScheduledDeliverablesStore } from './scheduled-deliverables-repository.js'; export { FileBroadcastRepository } from './broadcast-repository.js'; export { diff --git a/server/src/storage/transition-hooks-config-repository.ts b/server/src/storage/transition-hooks-config-repository.ts new file mode 100644 index 00000000..5eb4c76e --- /dev/null +++ b/server/src/storage/transition-hooks-config-repository.ts @@ -0,0 +1,69 @@ +import { constants } from 'node:fs'; +import { lstat, mkdir, open } from 'node:fs/promises'; +import path from 'node:path'; +import type { TransitionHooksConfig } from '@veritas-kanban/shared'; +import { withFileLock } from '../services/file-lock.js'; +import { getRuntimeDir } from '../utils/paths.js'; +import { ensureWithinBase } from '../utils/sanitize.js'; +import { atomicWriteFile } from './fs-helpers.js'; + +const MAX_TRANSITION_HOOKS_CONFIG_BYTES = 4 * 1024 * 1024; + +export interface TransitionHooksConfigRepository { + read(): Promise; + write(config: TransitionHooksConfig): Promise; +} + +export class FileTransitionHooksConfigRepository implements TransitionHooksConfigRepository { + private readonly runtimeDir: string; + private readonly configFile: string; + + constructor(runtimeDir = getRuntimeDir()) { + this.runtimeDir = path.resolve(runtimeDir); + this.configFile = ensureWithinBase( + this.runtimeDir, + path.join(this.runtimeDir, 'transition-hooks.json') + ); + } + + async read(): Promise { + let handle: Awaited> | undefined; + try { + handle = await open(this.configFile, constants.O_RDONLY | (constants.O_NOFOLLOW ?? 0)); + const [pathStats, stats] = await Promise.all([lstat(this.configFile), handle.stat()]); + if ( + pathStats.isSymbolicLink() || + pathStats.dev !== stats.dev || + pathStats.ino !== stats.ino + ) { + throw new Error('Transition hooks config must not use a symbolic link or changed file'); + } + if (!stats.isFile() || stats.size > MAX_TRANSITION_HOOKS_CONFIG_BYTES) { + throw new Error('Transition hooks config must use a bounded regular file'); + } + return JSON.parse(await handle.readFile({ encoding: 'utf8' })) as TransitionHooksConfig; + } catch (error) { + const errorCode = (error as NodeJS.ErrnoException).code; + if (errorCode === 'ENOENT') return null; + if (errorCode === 'ELOOP') { + throw new Error('Transition hooks config must not use a symbolic link', { cause: error }); + } + throw error; + } finally { + await handle?.close(); + } + } + + async write(config: TransitionHooksConfig): Promise { + await mkdir(this.runtimeDir, { recursive: true, mode: 0o700 }); + const directoryStats = await lstat(this.runtimeDir); + if (!directoryStats.isDirectory() || directoryStats.isSymbolicLink()) { + throw new Error('Transition hooks config path must use a regular directory'); + } + const content = JSON.stringify(config, null, 2); + if (Buffer.byteLength(content, 'utf8') > MAX_TRANSITION_HOOKS_CONFIG_BYTES) { + throw new Error('Transition hooks config exceeds the 4 MiB storage limit'); + } + await withFileLock(this.configFile, () => atomicWriteFile(this.configFile, content, 'utf8')); + } +}