fix: add input validation, date range checks, and security hardening

- validate-agents.js: reject invalid model names in agent frontmatter
- package-manager.js: validate script/binary names against shell injection
- session-manager.js: reject impossible month/day values in filenames
- utils.js: support options.all for replaceInFile string patterns
- strategic-compact/SKILL.md: fix hook matcher syntax and script reference
- install.sh: warn when overwriting existing rule customizations
- Add 24 new tests covering all validation and edge cases
This commit is contained in:
Affaan Mustafa
2026-02-12 17:32:04 -08:00
parent 35aed05903
commit 926eba97c5
10 changed files with 312 additions and 10 deletions

View File

@@ -70,6 +70,12 @@ fi
if [[ "$TARGET" == "claude" ]]; then
DEST_DIR="${CLAUDE_RULES_DIR:-$HOME/.claude/rules}"
# Warn if destination already exists (user may have local customizations)
if [[ -d "$DEST_DIR" ]] && [[ "$(ls -A "$DEST_DIR" 2>/dev/null)" ]]; then
echo "Note: $DEST_DIR/ already exists. Existing files will be overwritten."
echo " Back up any local customizations before proceeding."
fi
# Always install common rules
echo "Installing common rules -> $DEST_DIR/common/"
mkdir -p "$DEST_DIR/common"

View File

@@ -8,6 +8,7 @@ const path = require('path');
const AGENTS_DIR = path.join(__dirname, '../../agents');
const REQUIRED_FIELDS = ['model', 'tools'];
const VALID_MODELS = ['haiku', 'sonnet', 'opus'];
function extractFrontmatter(content) {
// Strip BOM if present (UTF-8 BOM: \uFEFF)
@@ -62,6 +63,12 @@ function validateAgents() {
hasErrors = true;
}
}
// Validate model is a known value
if (frontmatter.model && !VALID_MODELS.includes(frontmatter.model)) {
console.error(`ERROR: ${file} - Invalid model '${frontmatter.model}'. Must be one of: ${VALID_MODELS.join(', ')}`);
hasErrors = true;
}
}
if (hasErrors) {

View File

@@ -280,12 +280,24 @@ function setProjectPackageManager(pmName, projectDir = process.cwd()) {
return config;
}
// Allowed characters in script/binary names: alphanumeric, dash, underscore, dot, slash, @
// This prevents shell metacharacter injection while allowing scoped packages (e.g., @scope/pkg)
const SAFE_NAME_REGEX = /^[@a-zA-Z0-9_.\/-]+$/;
/**
* Get the command to run a script
* @param {string} script - Script name (e.g., "dev", "build", "test")
* @param {object} options - { projectDir }
* @throws {Error} If script name contains unsafe characters
*/
function getRunCommand(script, options = {}) {
if (!script || typeof script !== 'string') {
throw new Error('Script name must be a non-empty string');
}
if (!SAFE_NAME_REGEX.test(script)) {
throw new Error(`Script name contains unsafe characters: ${script}`);
}
const pm = getPackageManager(options);
switch (script) {
@@ -306,8 +318,16 @@ function getRunCommand(script, options = {}) {
* Get the command to execute a package binary
* @param {string} binary - Binary name (e.g., "prettier", "eslint")
* @param {string} args - Arguments to pass
* @throws {Error} If binary name contains unsafe characters
*/
function getExecCommand(binary, args = '', options = {}) {
if (!binary || typeof binary !== 'string') {
throw new Error('Binary name must be a non-empty string');
}
if (!SAFE_NAME_REGEX.test(binary)) {
throw new Error(`Binary name contains unsafe characters: ${binary}`);
}
const pm = getPackageManager(options);
return `${pm.config.execCmd} ${binary}${args ? ' ' + args : ''}`;
}

View File

@@ -31,6 +31,11 @@ function parseSessionFilename(filename) {
if (!match) return null;
const dateStr = match[1];
// Validate date components are in valid ranges (not just format)
const [year, month, day] = dateStr.split('-').map(Number);
if (month < 1 || month > 12 || day < 1 || day > 31) return null;
// match[2] is undefined for old format (no ID)
const shortId = match[2] || 'no-id';

View File

@@ -388,13 +388,26 @@ function getGitModifiedFiles(patterns = []) {
/**
* Replace text in a file (cross-platform sed alternative)
* @param {string} filePath - Path to the file
* @param {string|RegExp} search - Pattern to search for. String patterns replace
* the FIRST occurrence only; use a RegExp with the `g` flag for global replacement.
* @param {string} replace - Replacement string
* @param {object} options - Options
* @param {boolean} options.all - When true and search is a string, replaces ALL
* occurrences (uses String.replaceAll). Ignored for RegExp patterns.
* @returns {boolean} true if file was written, false on error
*/
function replaceInFile(filePath, search, replace) {
function replaceInFile(filePath, search, replace, options = {}) {
const content = readFile(filePath);
if (content === null) return false;
try {
const newContent = content.replace(search, replace);
let newContent;
if (options.all && typeof search === 'string') {
newContent = content.replaceAll(search, replace);
} else {
newContent = content.replace(search, replace);
}
writeFile(filePath, newContent);
return true;
} catch (err) {

View File

@@ -29,7 +29,7 @@ Strategic compaction at logical boundaries:
## How It Works
The `suggest-compact.sh` script runs on PreToolUse (Edit/Write) and:
The `suggest-compact.js` script runs on PreToolUse (Edit/Write) and:
1. **Tracks tool calls** — Counts tool invocations in session
2. **Threshold detection** — Suggests at configurable threshold (default: 50 calls)
@@ -42,13 +42,16 @@ Add to your `~/.claude/settings.json`:
```json
{
"hooks": {
"PreToolUse": [{
"matcher": "tool == \"Edit\" || tool == \"Write\"",
"hooks": [{
"type": "command",
"command": "~/.claude/skills/strategic-compact/suggest-compact.sh"
}]
}]
"PreToolUse": [
{
"matcher": "Edit",
"hooks": [{ "type": "command", "command": "node ~/.claude/skills/strategic-compact/suggest-compact.js" }]
},
{
"matcher": "Write",
"hooks": [{ "type": "command", "command": "node ~/.claude/skills/strategic-compact/suggest-compact.js" }]
}
]
}
}
```

View File

@@ -445,6 +445,64 @@ async function runTests() {
assert.ok(elapsed < 5000, `Should complete in <5s, took ${elapsed}ms`);
})) passed++; else failed++;
if (await asyncTest('hooks survive stdin exceeding 1MB limit', async () => {
// The post-edit-console-warn hook reads stdin up to 1MB then passes through
// Send > 1MB to verify truncation doesn't crash the hook
const oversizedInput = JSON.stringify({
tool_input: { file_path: '/test.js' },
tool_output: { output: 'x'.repeat(1200000) } // ~1.2MB
});
const proc = spawn('node', [path.join(scriptsDir, 'post-edit-console-warn.js')], {
stdio: ['pipe', 'pipe', 'pipe']
});
let code = null;
// MUST drain stdout/stderr to prevent backpressure blocking the child process
proc.stdout.on('data', () => {});
proc.stderr.on('data', () => {});
proc.stdin.on('error', (err) => {
if (err.code !== 'EPIPE' && err.code !== 'EOF') throw err;
});
proc.stdin.write(oversizedInput);
proc.stdin.end();
await new Promise(resolve => {
proc.on('close', (c) => { code = c; resolve(); });
});
assert.strictEqual(code, 0, 'Should exit 0 despite oversized input');
})) passed++; else failed++;
if (await asyncTest('hooks handle truncated JSON from overflow gracefully', async () => {
// session-end parses stdin JSON. If input is > 1MB and truncated mid-JSON,
// JSON.parse should fail and fall back to env var
const proc = spawn('node', [path.join(scriptsDir, 'session-end.js')], {
stdio: ['pipe', 'pipe', 'pipe']
});
let code = null;
let stderr = '';
// MUST drain stdout to prevent backpressure blocking the child process
proc.stdout.on('data', () => {});
proc.stderr.on('data', data => stderr += data);
proc.stdin.on('error', (err) => {
if (err.code !== 'EPIPE' && err.code !== 'EOF') throw err;
});
// Build a string that will be truncated mid-JSON at 1MB
const bigValue = 'x'.repeat(1200000);
proc.stdin.write(`{"transcript_path":"/tmp/none","padding":"${bigValue}"}`);
proc.stdin.end();
await new Promise(resolve => {
proc.on('close', (c) => { code = c; resolve(); });
});
// Should exit 0 even if JSON parse fails (falls back to env var or null)
assert.strictEqual(code, 0, 'Should not crash on truncated JSON');
})) passed++; else failed++;
// Summary
console.log('\n=== Test Results ===');
console.log(`Passed: ${passed}`);

View File

@@ -498,6 +498,131 @@ function runTests() {
assert.ok(regex.test('bun run lint'), 'Should match bun run lint');
})) passed++; else failed++;
// getPackageManager robustness tests
console.log('\ngetPackageManager (robustness):');
if (test('falls through on corrupted project config JSON', () => {
const testDir = fs.mkdtempSync(path.join(os.tmpdir(), 'pm-robust-'));
const claudeDir = path.join(testDir, '.claude');
fs.mkdirSync(claudeDir, { recursive: true });
fs.writeFileSync(path.join(claudeDir, 'package-manager.json'), '{not valid json!!!');
const originalEnv = process.env.CLAUDE_PACKAGE_MANAGER;
try {
delete process.env.CLAUDE_PACKAGE_MANAGER;
const result = pm.getPackageManager({ projectDir: testDir });
// Should fall through to default (npm) since project config is corrupt
assert.ok(result.name, 'Should return a package manager');
assert.ok(result.source !== 'project-config', 'Should not use corrupt project config');
} finally {
if (originalEnv !== undefined) {
process.env.CLAUDE_PACKAGE_MANAGER = originalEnv;
}
fs.rmSync(testDir, { recursive: true, force: true });
}
})) passed++; else failed++;
if (test('falls through on project config with unknown PM', () => {
const testDir = fs.mkdtempSync(path.join(os.tmpdir(), 'pm-robust-'));
const claudeDir = path.join(testDir, '.claude');
fs.mkdirSync(claudeDir, { recursive: true });
fs.writeFileSync(path.join(claudeDir, 'package-manager.json'),
JSON.stringify({ packageManager: 'nonexistent-pm' }));
const originalEnv = process.env.CLAUDE_PACKAGE_MANAGER;
try {
delete process.env.CLAUDE_PACKAGE_MANAGER;
const result = pm.getPackageManager({ projectDir: testDir });
assert.ok(result.name, 'Should return a package manager');
assert.ok(result.source !== 'project-config', 'Should not use unknown PM config');
} finally {
if (originalEnv !== undefined) {
process.env.CLAUDE_PACKAGE_MANAGER = originalEnv;
}
fs.rmSync(testDir, { recursive: true, force: true });
}
})) passed++; else failed++;
// getRunCommand validation tests
console.log('\ngetRunCommand (validation):');
if (test('rejects empty script name', () => {
assert.throws(() => pm.getRunCommand(''), /non-empty string/);
})) passed++; else failed++;
if (test('rejects null script name', () => {
assert.throws(() => pm.getRunCommand(null), /non-empty string/);
})) passed++; else failed++;
if (test('rejects script name with shell metacharacters', () => {
assert.throws(() => pm.getRunCommand('test; rm -rf /'), /unsafe characters/);
})) passed++; else failed++;
if (test('rejects script name with backticks', () => {
assert.throws(() => pm.getRunCommand('test`whoami`'), /unsafe characters/);
})) passed++; else failed++;
if (test('accepts scoped package names', () => {
const originalEnv = process.env.CLAUDE_PACKAGE_MANAGER;
try {
process.env.CLAUDE_PACKAGE_MANAGER = 'npm';
const cmd = pm.getRunCommand('@scope/my-script');
assert.strictEqual(cmd, 'npm run @scope/my-script');
} finally {
if (originalEnv !== undefined) {
process.env.CLAUDE_PACKAGE_MANAGER = originalEnv;
} else {
delete process.env.CLAUDE_PACKAGE_MANAGER;
}
}
})) passed++; else failed++;
// getExecCommand validation tests
console.log('\ngetExecCommand (validation):');
if (test('rejects empty binary name', () => {
assert.throws(() => pm.getExecCommand(''), /non-empty string/);
})) passed++; else failed++;
if (test('rejects null binary name', () => {
assert.throws(() => pm.getExecCommand(null), /non-empty string/);
})) passed++; else failed++;
if (test('rejects binary name with shell metacharacters', () => {
assert.throws(() => pm.getExecCommand('prettier; cat /etc/passwd'), /unsafe characters/);
})) passed++; else failed++;
if (test('accepts dotted binary names like tsc', () => {
const originalEnv = process.env.CLAUDE_PACKAGE_MANAGER;
try {
process.env.CLAUDE_PACKAGE_MANAGER = 'npm';
const cmd = pm.getExecCommand('tsc');
assert.strictEqual(cmd, 'npx tsc');
} finally {
if (originalEnv !== undefined) {
process.env.CLAUDE_PACKAGE_MANAGER = originalEnv;
} else {
delete process.env.CLAUDE_PACKAGE_MANAGER;
}
}
})) passed++; else failed++;
if (test('ignores unknown env var package manager', () => {
const originalEnv = process.env.CLAUDE_PACKAGE_MANAGER;
try {
process.env.CLAUDE_PACKAGE_MANAGER = 'totally-fake-pm';
const result = pm.getPackageManager();
// Should ignore invalid env var and fall through
assert.notStrictEqual(result.name, 'totally-fake-pm', 'Should not use unknown PM');
} finally {
if (originalEnv !== undefined) {
process.env.CLAUDE_PACKAGE_MANAGER = originalEnv;
} else {
delete process.env.CLAUDE_PACKAGE_MANAGER;
}
}
})) passed++; else failed++;
// Summary
console.log('\n=== Test Results ===');
console.log(`Passed: ${passed}`);

View File

@@ -465,6 +465,20 @@ src/main.ts
assert.ok(result, 'Should find old-format session by filename');
})) passed++; else failed++;
if (test('getSessionById returns null for empty string', () => {
const result = sessionManager.getSessionById('');
assert.strictEqual(result, null, 'Empty string should not match any session');
})) passed++; else failed++;
if (test('getSessionById metadata and stats populated when includeContent=true', () => {
const result = sessionManager.getSessionById('abcd1234', true);
assert.ok(result, 'Should find session');
assert.ok(result.metadata, 'Should have metadata');
assert.ok(result.stats, 'Should have stats');
assert.strictEqual(typeof result.stats.totalItems, 'number', 'stats.totalItems should be number');
assert.strictEqual(typeof result.stats.lineCount, 'number', 'stats.lineCount should be number');
})) passed++; else failed++;
// parseSessionMetadata edge cases
console.log('\nparseSessionMetadata (edge cases):');
@@ -574,6 +588,32 @@ src/main.ts
assert.strictEqual(result, null, 'Extra segments should be rejected');
})) passed++; else failed++;
if (test('rejects impossible month (13)', () => {
const result = sessionManager.parseSessionFilename('2026-13-01-abcd1234-session.tmp');
assert.strictEqual(result, null, 'Month 13 should be rejected');
})) passed++; else failed++;
if (test('rejects impossible day (32)', () => {
const result = sessionManager.parseSessionFilename('2026-01-32-abcd1234-session.tmp');
assert.strictEqual(result, null, 'Day 32 should be rejected');
})) passed++; else failed++;
if (test('rejects month 00', () => {
const result = sessionManager.parseSessionFilename('2026-00-15-abcd1234-session.tmp');
assert.strictEqual(result, null, 'Month 00 should be rejected');
})) passed++; else failed++;
if (test('rejects day 00', () => {
const result = sessionManager.parseSessionFilename('2026-01-00-abcd1234-session.tmp');
assert.strictEqual(result, null, 'Day 00 should be rejected');
})) passed++; else failed++;
if (test('accepts valid edge date (month 12, day 31)', () => {
const result = sessionManager.parseSessionFilename('2026-12-31-abcd1234-session.tmp');
assert.ok(result, 'Month 12, day 31 should be accepted');
assert.strictEqual(result.date, '2026-12-31');
})) passed++; else failed++;
if (test('datetime field is a Date object', () => {
const result = sessionManager.parseSessionFilename('2026-06-15-abcdef12-session.tmp');
assert.ok(result);

View File

@@ -546,6 +546,31 @@ function runTests() {
}
})) passed++; else failed++;
if (test('replaces all occurrences with string when options.all is true', () => {
const testFile = path.join(utils.getTempDir(), `utils-test-${Date.now()}.txt`);
try {
utils.writeFile(testFile, 'hello world hello again hello');
utils.replaceInFile(testFile, 'hello', 'goodbye', { all: true });
const content = utils.readFile(testFile);
assert.strictEqual(content, 'goodbye world goodbye again goodbye');
} finally {
fs.unlinkSync(testFile);
}
})) passed++; else failed++;
if (test('options.all is ignored for regex patterns', () => {
const testFile = path.join(utils.getTempDir(), `utils-test-${Date.now()}.txt`);
try {
utils.writeFile(testFile, 'foo bar foo');
// all option should be ignored for regex; only g flag matters
utils.replaceInFile(testFile, /foo/, 'qux', { all: true });
const content = utils.readFile(testFile);
assert.strictEqual(content, 'qux bar foo', 'Regex without g should still replace first only');
} finally {
fs.unlinkSync(testFile);
}
})) passed++; else failed++;
if (test('replaces with capture groups', () => {
const testFile = path.join(utils.getTempDir(), `utils-test-${Date.now()}.txt`);
try {