From cd744982a11e23437862a0fcb65c39caad74c131 Mon Sep 17 00:00:00 2001
From: "google-labs-jules[bot]"
 <161369871+google-labs-jules[bot]@users.noreply.github.com>
Date: Fri, 20 Feb 2026 03:50:36 +0000
Subject: [PATCH] refactor(governance): extract duplicated target validation
 logic

Extracted duplicated target path resolution and allowlist validation logic
into a shared `resolveAndValidateTarget` helper function in
`src/services/governance/index.js`.

Updated `createProposal`, `validateProposal`, `applyProposal`, and
`rollbackPrompt` to use the new helper, ensuring consistent validation
and path handling across the service.

Added `test/governance-health.test.mjs` to verify validation logic.

Co-authored-by: PR0M3TH3AN <53631862+PR0M3TH3AN@users.noreply.github.com>
---
 src/services/governance/index.js | 16 +++++---
 test/governance-health.test.mjs  | 69 ++++++++++++++++++++++++++++++++
 2 files changed, 79 insertions(+), 6 deletions(-)
 create mode 100644 test/governance-health.test.mjs

diff --git a/src/services/governance/index.js b/src/services/governance/index.js
index f88ccce..f18e8d9 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 });
+  }
+});