Merge pull request #355 from PR0M3TH3AN/refactor-governance-target-validation-5425533680072569362

🧹 refactor(governance): extract duplicated target validation logic

src/services/governance/index.js

@@ -15,6 +15,12 @@ async function ensureDirs() {
   await fs.mkdir(HISTORY_DIR, { recursive: true });
 }
 
+function resolveAndValidateTarget(target) {
+  const absoluteTarget = path.resolve(process.cwd(), target);
+  const isAllowed = ALLOWED_TARGET_DIRS.some(allowed => absoluteTarget.startsWith(allowed));
+  return { absoluteTarget, isAllowed };
+}
+
 export async function listProposals() {
   await ensureDirs();
   const entries = await fs.readdir(PROPOSALS_DIR, { withFileTypes: true });
@@ -70,8 +76,7 @@ export async function createProposal({ agent, target, newContent, reason }) {
   await ensureDirs();
 
   // Validate target path is allowed
-  const absoluteTarget = path.resolve(process.cwd(), target);
-  const isAllowed = ALLOWED_TARGET_DIRS.some(allowed => absoluteTarget.startsWith(allowed));
+  const { absoluteTarget, isAllowed } = resolveAndValidateTarget(target);
 
   if (!isAllowed) {
     throw new Error(`Target ${target} is not in an allowed directory (src/prompts/daily or src/prompts/weekly).`);
@@ -131,8 +136,7 @@ export async function validateProposal(id) {
   const { meta, newContent } = await getProposal(id);
 
   // 1. Allowlist check (redundant but safe)
-  const absoluteTarget = path.resolve(process.cwd(), meta.target);
-  const isAllowed = ALLOWED_TARGET_DIRS.some(allowed => absoluteTarget.startsWith(allowed));
+  const { isAllowed } = resolveAndValidateTarget(meta.target);
   if (!isAllowed) {
     return { valid: false, reason: 'Target not in allowed directories.' };
   }
@@ -169,7 +173,7 @@ export async function applyProposal(id) {
     throw new Error(`Validation failed: ${validation.reason}`);
   }
 
-  const absoluteTarget = path.resolve(process.cwd(), meta.target);
+  const { absoluteTarget } = resolveAndValidateTarget(meta.target);
 
   // Archive old
   try {
@@ -311,7 +315,7 @@ export async function listPromptVersions(target) {
 export async function rollbackPrompt(target, hashOrStrategy = 'latest') {
   // target is relative path e.g. src/prompts/daily/agent.md
   const archiveDir = path.join(HISTORY_DIR, path.dirname(target));
-  const absoluteTarget = path.resolve(process.cwd(), target);
+  const { absoluteTarget } = resolveAndValidateTarget(target);
 
   let sourceContent = null;
   let sourceName = null;

test/governance-health.test.mjs

@@ -0,0 +1,69 @@
+import { test } from 'node:test';
+import assert from 'node:assert/strict';
+import { createProposal, validateProposal } from '../src/services/governance/index.js';
+import path from 'node:path';
+import fs from 'node:fs/promises';
+
+test('createProposal should throw error for disallowed target', async () => {
+  const agent = 'test-agent';
+  const target = 'secrets/passwords.txt';
+  const newContent = 'new content';
+
+  await assert.rejects(
+    createProposal({ agent, target, newContent, reason: 'test' }),
+    {
+      message: /Target secrets\/passwords.txt is not in an allowed directory/
+    }
+  );
+});
+
+test('validateProposal should return invalid for disallowed target', async () => {
+  const id = 'test-disallowed-proposal';
+  const dir = path.resolve(process.cwd(), 'src/proposals', id);
+  await fs.mkdir(dir, { recursive: true });
+
+  const meta = {
+    id,
+    author: 'test-agent',
+    target: 'secrets/passwords.txt',
+    reason: 'test',
+    timestamp: new Date().toISOString(),
+    status: 'pending'
+  };
+
+  await fs.writeFile(path.join(dir, 'meta.json'), JSON.stringify(meta));
+  await fs.writeFile(path.join(dir, 'new.md'), 'Shared contract (required):\nRequired startup + artifacts + memory + issue capture');
+
+  try {
+    const result = await validateProposal(id);
+    assert.strictEqual(result.valid, false);
+    assert.strictEqual(result.reason, 'Target not in allowed directories.');
+  } finally {
+    await fs.rm(dir, { recursive: true, force: true });
+  }
+});
+
+test('validateProposal should return valid for allowed target', async () => {
+  const id = 'test-allowed-proposal';
+  const dir = path.resolve(process.cwd(), 'src/proposals', id);
+  await fs.mkdir(dir, { recursive: true });
+
+  const meta = {
+    id,
+    author: 'test-agent',
+    target: 'src/prompts/daily/test.md',
+    reason: 'test',
+    timestamp: new Date().toISOString(),
+    status: 'pending'
+  };
+
+  await fs.writeFile(path.join(dir, 'meta.json'), JSON.stringify(meta));
+  await fs.writeFile(path.join(dir, 'new.md'), 'Shared contract (required):\nRequired startup + artifacts + memory + issue capture');
+
+  try {
+    const result = await validateProposal(id);
+    assert.strictEqual(result.valid, true);
+  } finally {
+    await fs.rm(dir, { recursive: true, force: true });
+  }
+});