mirror of
https://github.com/PR0M3TH3AN/TORCH.git
synced 2026-11-04 11:51:53 +00:00
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 <[email protected]>
This commit is contained in:
1 parent
8d43fc1be8
commit
73a92da6c1
8 files changed
+151
-32
No files matched your search
+1
-14
@@ -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.
|
||||
|
||||
+1
-15
@@ -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',
|
||||
|
||||
+10
-1
@@ -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])];
|
||||
|
||||
@@ -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;
|
||||
|
||||
|
||||
@@ -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 "[email protected]"', { 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);
|
||||
});
|
||||
@@ -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}`);
|
||||
}
|
||||
});
|
||||
@@ -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', () => {
|
||||
|
||||
@@ -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', () => {
|
||||
|
||||
Reference in new issue
Block a user