refactor: move transition hooks config behind storage (#1202)

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",

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",

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<typeof import('node:fs/promises')>();
+  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<typeof import('node:fs/promises')>('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);
+  });
+});

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<TransitionHooksConfig> {
-  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<void> {
-  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, 'project' | 'type'>
+  task: Pick<Task, 'project' | 'type'>,
+  loadedConfig?: TransitionHooksConfig
 ): Promise<TransitionRule[]> {
-  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[] = [];

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 {

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<TransitionHooksConfig | null>;
+  write(config: TransitionHooksConfig): Promise<void>;
+}
+
+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<TransitionHooksConfig | null> {
+    let handle: Awaited<ReturnType<typeof open>> | 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<void> {
+    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'));
+  }
+}