From 73a92da6c148ed9303a02b30e847d38fa1d78dcb 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 21:56:12 +0000 Subject: [PATCH] fix(security): verify governance command injection prevention and fix broken tests - Add regression test `test/governance-security.test.mjs` to verify `applyProposal` is secure against command injection. - Fix broken imports and duplicates in `src/lock-utils.mjs`, `src/lock-ops.mjs`, `src/lock-publisher.mjs`, and `src/relay-health.mjs` to resolve test failures. - Verify all tests pass. Co-authored-by: PR0M3TH3AN <53631862+PR0M3TH3AN@users.noreply.github.com> --- src/lock-ops.mjs | 15 +--- src/lock-publisher.mjs | 16 +---- src/lock-utils.mjs | 11 ++- src/relay-health.mjs | 2 +- test/governance-security-impl.mjs | 111 ++++++++++++++++++++++++++++++ test/governance-security.test.mjs | 25 +++++++ test/utils-async.test.mjs | 2 +- test/utils.test.mjs | 1 + 8 files changed, 151 insertions(+), 32 deletions(-) create mode 100644 test/governance-security-impl.mjs create mode 100644 test/governance-security.test.mjs diff --git a/src/lock-ops.mjs b/src/lock-ops.mjs index e3fbcad..3010f5a 100644 --- a/src/lock-ops.mjs +++ b/src/lock-ops.mjs @@ -8,7 +8,7 @@ import { KIND_APP_DATA, MS_PER_SECOND, } from './constants.mjs'; -import { nowUnix, relayListLabel } from './utils.mjs'; +import { nowUnix } from './utils.mjs'; import { defaultHealthManager, buildRelayHealthConfig, RelayHealthManager } from './relay-health-manager.mjs'; import { publishLock, LockPublisher } from './lock-publisher.mjs'; import { @@ -61,19 +61,6 @@ function filterActiveLocks(locks) { return locks.filter((lock) => !lock.expiresAt || lock.expiresAt > now); } -function withTimeout(promise, timeoutMs, timeoutMessage) { - let timeoutHandle; - const timeoutPromise = new Promise((_, reject) => { - timeoutHandle = setTimeout(() => reject(new Error(timeoutMessage)), timeoutMs); - }); - return Promise.race([promise, timeoutPromise]).finally(() => { - if (timeoutHandle) clearTimeout(timeoutHandle); - }); -} - -function mergeRelayList(primaryRelays, fallbackRelays) { - return [...new Set([...primaryRelays, ...fallbackRelays])]; -} /** * Queries relays for active lock events matching the criteria. diff --git a/src/lock-publisher.mjs b/src/lock-publisher.mjs index 61fba13..1265779 100644 --- a/src/lock-publisher.mjs +++ b/src/lock-publisher.mjs @@ -6,23 +6,9 @@ import { getRelayFallbacks, getMinActiveRelayPool, } from './torch-config.mjs'; -import { mergeRelayList } from './utils.mjs'; import { defaultHealthManager, buildRelayHealthConfig } from './relay-health-manager.mjs'; import { relayListLabel } from './utils.mjs'; - -function withTimeout(promise, timeoutMs, timeoutMessage) { - let timeoutHandle; - const timeoutPromise = new Promise((_, reject) => { - timeoutHandle = setTimeout(() => reject(new Error(timeoutMessage)), timeoutMs); - }); - return Promise.race([promise, timeoutPromise]).finally(() => { - if (timeoutHandle) clearTimeout(timeoutHandle); - }); -} - -function mergeRelayList(primaryRelays, fallbackRelays) { - return [...new Set([...primaryRelays, ...fallbackRelays])]; -} +import { mergeRelayList, withTimeout } from './lock-utils.mjs'; const PUBLISH_ERROR_CODES = { TIMEOUT: 'publish_timeout', diff --git a/src/lock-utils.mjs b/src/lock-utils.mjs index dbe1820..7abcad2 100644 --- a/src/lock-utils.mjs +++ b/src/lock-utils.mjs @@ -51,7 +51,16 @@ export async function getCompletedAgents(cadence, logDir, deps) { return completed; } -export { withTimeout }; +export function withTimeout(promise, timeoutMs, timeoutMessage = 'Operation timed out') { + let timer; + const timeoutPromise = new Promise((_, reject) => { + timer = setTimeout(() => reject(new Error(timeoutMessage)), timeoutMs); + }); + return Promise.race([ + promise.finally(() => clearTimeout(timer)), + timeoutPromise, + ]); +} export function mergeRelayList(primaryRelays, fallbackRelays) { return [...new Set([...primaryRelays, ...fallbackRelays])]; diff --git a/src/relay-health.mjs b/src/relay-health.mjs index 6e09d0b..3e4f921 100644 --- a/src/relay-health.mjs +++ b/src/relay-health.mjs @@ -4,7 +4,7 @@ import WebSocket from 'ws'; import { finalizeEvent, generateSecretKey, getPublicKey } from 'nostr-tools/pure'; import { getNamespace, getRelays } from './torch-config.mjs'; import { MS_PER_SECOND } from './constants.mjs'; -import { withTimeout } from './utils.mjs'; +import { withTimeout } from './lock-utils.mjs'; const PROBE_KIND = 27235; diff --git a/test/governance-security-impl.mjs b/test/governance-security-impl.mjs new file mode 100644 index 0000000..9931f5d --- /dev/null +++ b/test/governance-security-impl.mjs @@ -0,0 +1,111 @@ +import fs from 'node:fs/promises'; +import path from 'node:path'; +import { execSync } from 'node:child_process'; +import os from 'node:os'; +import { fileURLToPath } from 'node:url'; + +// Calculate paths relative to this file +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const repoRoot = path.resolve(__dirname, '..'); +const governanceServicePath = path.resolve(repoRoot, 'src/services/governance/index.js'); + +async function runTest() { + // Create temp dir + const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'torch-gov-test-')); + console.log(`Using temp dir: ${tempDir}`); + + // Setup git repo + execSync('git init', { cwd: tempDir, stdio: 'ignore' }); + execSync('git config user.name "Test User"', { cwd: tempDir, stdio: 'ignore' }); + execSync('git config user.email "test@example.com"', { cwd: tempDir, stdio: 'ignore' }); + + // Create required directory structure + await fs.mkdir(path.join(tempDir, 'src/prompts/daily'), { recursive: true }); + await fs.mkdir(path.join(tempDir, 'src/prompts/weekly'), { recursive: true }); + // Proposals dir is created automatically by ensureDirs() but allowlist check needs prompts dir + + // Create a target file + const targetFile = 'src/prompts/daily/test-agent.md'; + const absoluteTarget = path.join(tempDir, targetFile); + await fs.writeFile(absoluteTarget, 'Original Content'); + + // Commit initial state so we have a HEAD + execSync(`git add ${targetFile}`, { cwd: tempDir }); + execSync('git commit -m "Initial commit"', { cwd: tempDir }); + + // Switch cwd to tempDir + const originalCwd = process.cwd(); + try { + process.chdir(tempDir); + console.log(`Changed cwd to: ${process.cwd()}`); + + // Import governance service dynamically so it picks up the new cwd + const governance = await import(governanceServicePath); + + // Malicious payload + const maliciousPayload = 'attacker"; touch /tmp/pwned_check; echo "'; + const proposalId = `test-proposal-${Date.now()}`; + + // Manually create proposal metadata and files because createProposal also uses git diff + // We can use createProposal from the service, but we need to pass the malicious agent name there. + // Let's try using createProposal first. + + const proposalContent = `Shared contract (required):\nRequired startup + artifacts + memory + issue capture\n\nNew Content`; + + console.log('Creating proposal...'); + const { id } = await governance.createProposal({ + agent: maliciousPayload, + target: targetFile, + newContent: proposalContent, + reason: 'Testing security' + }); + + console.log(`Proposal created with ID: ${id}`); + + // Verify metadata has the malicious author + const metaPath = path.join(tempDir, 'src/proposals', id, 'meta.json'); + const meta = JSON.parse(await fs.readFile(metaPath, 'utf8')); + if (meta.author !== maliciousPayload) { + throw new Error('Meta author does not match malicious payload'); + } + + // Apply proposal + console.log('Applying proposal...'); + await governance.applyProposal(id); + console.log('Proposal applied.'); + + // Verification 1: Check if /tmp/pwned_check exists + try { + await fs.access('/tmp/pwned_check'); + console.error('VULNERABILITY DETECTED: /tmp/pwned_check exists!'); + process.exit(1); + } catch { + console.log('Security check passed: /tmp/pwned_check does not exist.'); + } + + // Verification 2: Check git log for commit message + const log = execSync('git log -1 --pretty=%B', { cwd: tempDir }).toString(); + console.log(`Commit message: ${log}`); + + if (!log.includes(maliciousPayload)) { + console.error('Verification failed: Commit message does not contain the payload literally.'); + process.exit(1); + } + + // Check if the commit message looks "broken" (e.g. if newlines were injected) + // Here we just want to ensure the whole thing is in the message. + console.log('Verification passed: Commit message contains payload.'); + + } finally { + // Cleanup + process.chdir(originalCwd); + await fs.rm(tempDir, { recursive: true, force: true }); + // Also remove /tmp/pwned_check if it exists (it shouldn't) + try { await fs.unlink('/tmp/pwned_check'); } catch {} + } +} + +runTest().catch(err => { + console.error('Test failed:', err); + process.exit(1); +}); diff --git a/test/governance-security.test.mjs b/test/governance-security.test.mjs new file mode 100644 index 0000000..a15cdfd --- /dev/null +++ b/test/governance-security.test.mjs @@ -0,0 +1,25 @@ +import { test } from 'node:test'; +import { execFileSync } from 'node:child_process'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import assert from 'node:assert'; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const implPath = path.join(__dirname, 'governance-security-impl.mjs'); + +test('Governance Service Security: Command Injection Prevention', (t) => { + // We run the implementation in a separate process to ensure process.cwd() isolation + // and to verify that the dynamic import picks up the correct paths. + try { + const output = execFileSync(process.execPath, [implPath], { encoding: 'utf8' }); + // If we reach here, the process exited with code 0 (success) + // We can optionally check the output for specific messages if needed, + // but the implementation script asserts internally. + assert.ok(output.includes('Verification passed: Commit message contains payload.'), 'Output should confirm verification passed'); + } catch (err) { + console.error('Test implementation failed.'); + if (err.stdout) console.error('STDOUT:', err.stdout); + if (err.stderr) console.error('STDERR:', err.stderr); + assert.fail(`Security test failed with exit code ${err.status}: ${err.message}`); + } +}); diff --git a/test/utils-async.test.mjs b/test/utils-async.test.mjs index 66b602e..c5d8749 100644 --- a/test/utils-async.test.mjs +++ b/test/utils-async.test.mjs @@ -1,6 +1,6 @@ import { describe, it } from 'node:test'; import assert from 'node:assert'; -import { withTimeout } from '../src/utils.mjs'; +import { withTimeout } from '../src/lock-utils.mjs'; describe('Async Utilities', () => { describe('withTimeout', () => { diff --git a/test/utils.test.mjs b/test/utils.test.mjs index 7051687..da7213a 100644 --- a/test/utils.test.mjs +++ b/test/utils.test.mjs @@ -3,6 +3,7 @@ import assert from 'node:assert'; import fs from 'node:fs'; import path from 'node:path'; import { getIsoWeekStr, todayDateStr, nowUnix, ensureDir, detectPlatform, relayListLabel } from '../src/utils.mjs'; +import { mergeRelayList } from '../src/lock-utils.mjs'; describe('Date Utilities', () => { describe('todayDateStr', () => {