diff --git a/src/services/governance/index.js b/src/services/governance/index.js index cce30d0..22cc434 100644 --- a/src/services/governance/index.js +++ b/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; diff --git a/test/governance-health.test.mjs b/test/governance-health.test.mjs new file mode 100644 index 0000000..b23bb9a --- /dev/null +++ b/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 }); + } +});