mirror of
https://github.com/JuliusBrussee/caveman.git
synced 2026-08-11 13:21:09 +02:00
fix(hooks): /caveman arg whitelist + symlinked-parent ~/.claude support
Two converging hook fixes:
* /caveman arg parser uses VALID_MODES whitelist (#314 by @jgttech).
Drops the silent fallback to getDefaultMode() on unknown args. Three
bugs collapsed: /caveman full was impossible if defaultMode was set
to anything else, /caveman off had no effect, and typos like
/caveman fulll silently clobbered state. Now: bare /caveman activates
default; /caveman off|stop|disable removes flag; unknown args leave
flag untouched (no silent overwrite); independent modes (commit,
review, compress) cannot be selected as args.
* safeWriteFlag through symlinked ~/.claude (#224 by @voidborne-d,
closes #207). Earlier hardening (5ad8f6d) refused every symlinked
parent including legitimate ~/.claude → /opt/shared-claude or
/mnt/d/claude-config patterns. Now resolves the parent via
realpathSync, verifies ownership on Unix (uid match) or under-home
on Windows, and uses the resolved path for the atomic write. Flag
file itself still must not be a symlink — that's the actual clobber
vector. CAVEMAN_DEBUG=1 emits stderr diagnostics on refusal.
12 regression tests in tests/test_symlink_flag.js — all pass.
Verified:
/caveman / lite / full / ultra / wenyan{,-lite,-full,-ultra} → mode set
/caveman off / stop / disable → flag deleted
/caveman commit → no flag (rejected)
/caveman fulll (typo) → no flag (rejected)
symlink ~/.claude → owned dir → write succeeds in real dir
symlink ~/.claude → other-user dir → refused
Co-Authored-By: Jonathon Tech <jgttech@users.noreply.github.com>
Co-Authored-By: voidborne-d <voidborne-d@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Jonathon Tech
voidborne-d
Claude Opus 4.7
parent
b24b915e0a
commit
a660db02c9
+50
-12
@@ -59,37 +59,75 @@ function getDefaultMode() {
|
||||
}
|
||||
|
||||
// Symlink-safe flag file write.
|
||||
// Refuses symlinks at the target file and at the immediate parent directory,
|
||||
// uses O_NOFOLLOW where available, writes atomically via temp + rename with
|
||||
// Uses O_NOFOLLOW where available, writes atomically via temp + rename with
|
||||
// 0600 permissions. Protects against local attackers replacing the predictable
|
||||
// flag path (~/.claude/.caveman-active) with a symlink to clobber other files.
|
||||
//
|
||||
// Does NOT walk the full ancestor chain — macOS has /tmp -> /private/tmp and
|
||||
// many legitimate setups route through symlinked home dirs, so a full walk
|
||||
// produces false positives. The attack surface requires write access to the
|
||||
// immediate parent, which is what we check.
|
||||
// When the parent directory is itself a symlink (legitimate pattern: ~/.claude
|
||||
// symlinked to another drive or shared config dir), resolves through to the
|
||||
// real path and verifies ownership on Unix (uid match). This allows e.g.
|
||||
// ln -s /opt/shared-claude-config ~/.claude
|
||||
// while still refusing attacker-planted symlinks pointing to dirs owned by
|
||||
// another user.
|
||||
//
|
||||
// On Windows, uid checks are unavailable — falls back to verifying the resolved
|
||||
// path lives under the user's home directory.
|
||||
//
|
||||
// The flag file itself must never be a symlink (that's the actual clobber vector).
|
||||
//
|
||||
// Set CAVEMAN_DEBUG=1 to emit stderr diagnostics when flag writes are refused.
|
||||
//
|
||||
// Silent-fails on any filesystem error — the flag is best-effort.
|
||||
function safeWriteFlag(flagPath, content) {
|
||||
const debug = process.env.CAVEMAN_DEBUG === '1';
|
||||
try {
|
||||
const flagDir = path.dirname(flagPath);
|
||||
fs.mkdirSync(flagDir, { recursive: true });
|
||||
|
||||
// Refuse if the parent directory itself is a symlink (attacker redirect).
|
||||
// When the parent directory is a symlink, resolve it and verify ownership.
|
||||
// This allows legitimate symlinked ~/.claude dirs while still refusing
|
||||
// attacker-planted symlinks pointing at dirs owned by another user.
|
||||
let realFlagDir;
|
||||
try {
|
||||
if (fs.lstatSync(flagDir).isSymbolicLink()) return;
|
||||
const lstat = fs.lstatSync(flagDir);
|
||||
if (lstat.isSymbolicLink()) {
|
||||
realFlagDir = fs.realpathSync(flagDir);
|
||||
const realStat = fs.statSync(realFlagDir);
|
||||
if (!realStat.isDirectory()) {
|
||||
if (debug) process.stderr.write(`[caveman] safeWriteFlag: symlink target ${realFlagDir} is not a directory\n`);
|
||||
return;
|
||||
}
|
||||
if (typeof process.getuid === 'function') {
|
||||
if (realStat.uid !== process.getuid()) {
|
||||
if (debug) process.stderr.write(`[caveman] safeWriteFlag: symlink target ${realFlagDir} owned by uid ${realStat.uid}, not current user ${process.getuid()}\n`);
|
||||
return;
|
||||
}
|
||||
} else {
|
||||
const home = os.homedir();
|
||||
const normalizedReal = path.resolve(realFlagDir);
|
||||
const normalizedHome = path.resolve(home);
|
||||
if (!normalizedReal.toLowerCase().startsWith(normalizedHome.toLowerCase() + path.sep) &&
|
||||
normalizedReal.toLowerCase() !== normalizedHome.toLowerCase()) {
|
||||
if (debug) process.stderr.write(`[caveman] safeWriteFlag: symlink target ${normalizedReal} is outside home directory ${normalizedHome}\n`);
|
||||
return;
|
||||
}
|
||||
}
|
||||
} else {
|
||||
realFlagDir = flagDir;
|
||||
}
|
||||
} catch (e) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Refuse if the target already exists as a symlink.
|
||||
// The flag file itself must never be a symlink (that's the actual clobber vector).
|
||||
const realFlagPath = path.join(realFlagDir, path.basename(flagPath));
|
||||
try {
|
||||
if (fs.lstatSync(flagPath).isSymbolicLink()) return;
|
||||
if (fs.lstatSync(realFlagPath).isSymbolicLink()) return;
|
||||
} catch (e) {
|
||||
if (e.code !== 'ENOENT') return;
|
||||
}
|
||||
|
||||
const tempPath = path.join(flagDir, `.caveman-active.${process.pid}.${Date.now()}`);
|
||||
const tempPath = path.join(realFlagDir, `.caveman-active.${process.pid}.${Date.now()}`);
|
||||
const O_NOFOLLOW = typeof fs.constants.O_NOFOLLOW === 'number' ? fs.constants.O_NOFOLLOW : 0;
|
||||
const flags = fs.constants.O_WRONLY | fs.constants.O_CREAT | fs.constants.O_EXCL | O_NOFOLLOW;
|
||||
let fd;
|
||||
@@ -100,7 +138,7 @@ function safeWriteFlag(flagPath, content) {
|
||||
} finally {
|
||||
if (fd !== undefined) fs.closeSync(fd);
|
||||
}
|
||||
fs.renameSync(tempPath, flagPath);
|
||||
fs.renameSync(tempPath, realFlagPath);
|
||||
} catch (e) {
|
||||
// Silent fail — flag is best-effort
|
||||
}
|
||||
|
||||
@@ -5,7 +5,11 @@
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
const os = require('os');
|
||||
const { getDefaultMode, safeWriteFlag, readFlag } = require('./caveman-config');
|
||||
const { getDefaultMode, safeWriteFlag, readFlag, VALID_MODES } = require('./caveman-config');
|
||||
|
||||
// Modes handled by their own slash commands (/caveman-commit, etc.) — not
|
||||
// selectable via /caveman <arg>.
|
||||
const INDEPENDENT_MODES = new Set(['commit', 'review', 'compress']);
|
||||
|
||||
const claudeDir = process.env.CLAUDE_CONFIG_DIR || path.join(os.homedir(), '.claude');
|
||||
const flagPath = path.join(claudeDir, '.caveman-active');
|
||||
@@ -45,12 +49,18 @@ process.stdin.on('end', () => {
|
||||
} else if (cmd === '/caveman-compress' || cmd === '/caveman:caveman-compress') {
|
||||
mode = 'compress';
|
||||
} else if (cmd === '/caveman' || cmd === '/caveman:caveman') {
|
||||
if (arg === 'lite') mode = 'lite';
|
||||
else if (arg === 'ultra') mode = 'ultra';
|
||||
else if (arg === 'wenyan-lite') mode = 'wenyan-lite';
|
||||
else if (arg === 'wenyan' || arg === 'wenyan-full') mode = 'wenyan';
|
||||
else if (arg === 'wenyan-ultra') mode = 'wenyan-ultra';
|
||||
else mode = getDefaultMode();
|
||||
// Bare /caveman → activate at configured default
|
||||
if (!arg) {
|
||||
mode = getDefaultMode();
|
||||
} else if (arg === 'off' || arg === 'stop' || arg === 'disable') {
|
||||
mode = 'off';
|
||||
} else if (arg === 'wenyan-full') {
|
||||
// Canonical alias — config stores as 'wenyan'
|
||||
mode = 'wenyan';
|
||||
} else if (VALID_MODES.includes(arg) && !INDEPENDENT_MODES.has(arg)) {
|
||||
mode = arg;
|
||||
}
|
||||
// Unknown arg → mode stays null, flag untouched (no silent overwrite)
|
||||
}
|
||||
|
||||
if (mode && mode !== 'off') {
|
||||
@@ -78,7 +88,6 @@ process.stdin.on('end', () => {
|
||||
// If the flag is missing, corrupted, oversized, or a symlink pointing at
|
||||
// something like ~/.ssh/id_rsa, readFlag returns null and we emit nothing
|
||||
// — never inject untrusted bytes into model context.
|
||||
const INDEPENDENT_MODES = new Set(['commit', 'review', 'compress']);
|
||||
const activeMode = readFlag(flagPath);
|
||||
if (activeMode && !INDEPENDENT_MODES.has(activeMode)) {
|
||||
process.stdout.write(JSON.stringify({
|
||||
|
||||
@@ -0,0 +1,223 @@
|
||||
#!/usr/bin/env node
|
||||
// Tests for safeWriteFlag / readFlag behavior with symlinked parent directories.
|
||||
// Covers fix for issue #207: safeWriteFlag refuses flag writes when ~/.claude
|
||||
// is a symlink.
|
||||
//
|
||||
// Run: node tests/test_symlink_flag.js
|
||||
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
const os = require('os');
|
||||
const assert = require('assert');
|
||||
|
||||
const { safeWriteFlag, readFlag, VALID_MODES } = require('../hooks/caveman-config');
|
||||
|
||||
let passed = 0;
|
||||
let failed = 0;
|
||||
|
||||
function test(name, fn) {
|
||||
const tmpBase = fs.mkdtempSync(path.join(os.tmpdir(), 'caveman-symlink-test-'));
|
||||
try {
|
||||
fn(tmpBase);
|
||||
passed++;
|
||||
console.log(` ✓ ${name}`);
|
||||
} catch (e) {
|
||||
failed++;
|
||||
console.error(` ✗ ${name}`);
|
||||
console.error(` ${e.message}`);
|
||||
} finally {
|
||||
fs.rmSync(tmpBase, { recursive: true, force: true });
|
||||
}
|
||||
}
|
||||
|
||||
console.log('safeWriteFlag + readFlag symlink tests\n');
|
||||
|
||||
// ---------- safeWriteFlag ----------
|
||||
|
||||
test('writes flag in normal (non-symlinked) directory', (tmp) => {
|
||||
const flagDir = path.join(tmp, 'claude-config');
|
||||
fs.mkdirSync(flagDir, { recursive: true });
|
||||
const flagPath = path.join(flagDir, '.caveman-active');
|
||||
|
||||
safeWriteFlag(flagPath, 'full');
|
||||
|
||||
assert.strictEqual(fs.readFileSync(flagPath, 'utf8'), 'full');
|
||||
});
|
||||
|
||||
test('writes flag when parent directory is a symlink owned by current user', (tmp) => {
|
||||
// Create real directory and symlink to it (simulating ~/.claude -> /real/path)
|
||||
const realDir = path.join(tmp, 'real-claude-config');
|
||||
fs.mkdirSync(realDir, { recursive: true });
|
||||
const symlinkDir = path.join(tmp, 'claude-symlink');
|
||||
fs.symlinkSync(realDir, symlinkDir);
|
||||
|
||||
const flagPath = path.join(symlinkDir, '.caveman-active');
|
||||
safeWriteFlag(flagPath, 'ultra');
|
||||
|
||||
// Flag should exist in the real directory
|
||||
const realFlagPath = path.join(realDir, '.caveman-active');
|
||||
assert.strictEqual(fs.existsSync(realFlagPath), true, 'flag file should exist in resolved dir');
|
||||
assert.strictEqual(fs.readFileSync(realFlagPath, 'utf8'), 'ultra');
|
||||
});
|
||||
|
||||
test('readFlag works through symlinked parent directory', (tmp) => {
|
||||
const realDir = path.join(tmp, 'real-claude-config');
|
||||
fs.mkdirSync(realDir, { recursive: true });
|
||||
const symlinkDir = path.join(tmp, 'claude-symlink');
|
||||
fs.symlinkSync(realDir, symlinkDir);
|
||||
|
||||
// Write directly to real path, then read through symlink path
|
||||
const realFlagPath = path.join(realDir, '.caveman-active');
|
||||
fs.writeFileSync(realFlagPath, 'lite', { mode: 0o600 });
|
||||
|
||||
const result = readFlag(path.join(symlinkDir, '.caveman-active'));
|
||||
assert.strictEqual(result, 'lite');
|
||||
});
|
||||
|
||||
test('safeWriteFlag then readFlag round-trip through symlink', (tmp) => {
|
||||
const realDir = path.join(tmp, 'real-config');
|
||||
fs.mkdirSync(realDir, { recursive: true });
|
||||
const symlinkDir = path.join(tmp, 'link-config');
|
||||
fs.symlinkSync(realDir, symlinkDir);
|
||||
|
||||
const flagPath = path.join(symlinkDir, '.caveman-active');
|
||||
safeWriteFlag(flagPath, 'wenyan-ultra');
|
||||
|
||||
// Read back through the same symlink path
|
||||
const result = readFlag(flagPath);
|
||||
assert.strictEqual(result, 'wenyan-ultra');
|
||||
});
|
||||
|
||||
test('refuses flag file that is itself a symlink (even through symlinked parent)', (tmp) => {
|
||||
const realDir = path.join(tmp, 'real-config');
|
||||
fs.mkdirSync(realDir, { recursive: true });
|
||||
const symlinkDir = path.join(tmp, 'link-config');
|
||||
fs.symlinkSync(realDir, symlinkDir);
|
||||
|
||||
// Create a symlink at the flag file location pointing to some other file
|
||||
const decoyFile = path.join(tmp, 'decoy.txt');
|
||||
fs.writeFileSync(decoyFile, 'ATTACK');
|
||||
const realFlagPath = path.join(realDir, '.caveman-active');
|
||||
fs.symlinkSync(decoyFile, realFlagPath);
|
||||
|
||||
// safeWriteFlag should refuse (flag file is a symlink)
|
||||
safeWriteFlag(path.join(symlinkDir, '.caveman-active'), 'full');
|
||||
// The decoy should NOT have been overwritten
|
||||
assert.strictEqual(fs.readFileSync(decoyFile, 'utf8'), 'ATTACK');
|
||||
});
|
||||
|
||||
test('readFlag refuses flag file that is a symlink', (tmp) => {
|
||||
const realDir = path.join(tmp, 'real-config');
|
||||
fs.mkdirSync(realDir, { recursive: true });
|
||||
|
||||
const secretFile = path.join(tmp, 'secret.txt');
|
||||
fs.writeFileSync(secretFile, 'SSH_PRIVATE_KEY_CONTENT');
|
||||
fs.symlinkSync(secretFile, path.join(realDir, '.caveman-active'));
|
||||
|
||||
const result = readFlag(path.join(realDir, '.caveman-active'));
|
||||
assert.strictEqual(result, null, 'should refuse symlinked flag file');
|
||||
});
|
||||
|
||||
test('flag file permissions are 0600 when written through symlink', (tmp) => {
|
||||
if (process.platform === 'win32') return; // skip on Windows
|
||||
|
||||
const realDir = path.join(tmp, 'real-config');
|
||||
fs.mkdirSync(realDir, { recursive: true });
|
||||
const symlinkDir = path.join(tmp, 'link-config');
|
||||
fs.symlinkSync(realDir, symlinkDir);
|
||||
|
||||
safeWriteFlag(path.join(symlinkDir, '.caveman-active'), 'full');
|
||||
|
||||
const realFlagPath = path.join(realDir, '.caveman-active');
|
||||
const stat = fs.statSync(realFlagPath);
|
||||
const mode = stat.mode & 0o777;
|
||||
assert.strictEqual(mode, 0o600, `expected 0600, got 0${mode.toString(8)}`);
|
||||
});
|
||||
|
||||
test('overwrites existing flag through symlinked parent', (tmp) => {
|
||||
const realDir = path.join(tmp, 'real-config');
|
||||
fs.mkdirSync(realDir, { recursive: true });
|
||||
const symlinkDir = path.join(tmp, 'link-config');
|
||||
fs.symlinkSync(realDir, symlinkDir);
|
||||
|
||||
const flagPath = path.join(symlinkDir, '.caveman-active');
|
||||
|
||||
safeWriteFlag(flagPath, 'lite');
|
||||
assert.strictEqual(readFlag(flagPath), 'lite');
|
||||
|
||||
safeWriteFlag(flagPath, 'ultra');
|
||||
assert.strictEqual(readFlag(flagPath), 'ultra');
|
||||
});
|
||||
|
||||
test('creates parent directory via mkdirSync even when it does not exist yet', (tmp) => {
|
||||
const flagDir = path.join(tmp, 'nonexistent', 'nested');
|
||||
const flagPath = path.join(flagDir, '.caveman-active');
|
||||
|
||||
safeWriteFlag(flagPath, 'full');
|
||||
|
||||
assert.strictEqual(fs.existsSync(flagPath), true);
|
||||
assert.strictEqual(fs.readFileSync(flagPath, 'utf8'), 'full');
|
||||
});
|
||||
|
||||
test('symlink to nonexistent target silently fails', (tmp) => {
|
||||
const symlinkDir = path.join(tmp, 'broken-link');
|
||||
try {
|
||||
fs.symlinkSync('/nonexistent/path/that/does/not/exist', symlinkDir);
|
||||
} catch (e) {
|
||||
// Can't create symlink — skip
|
||||
return;
|
||||
}
|
||||
|
||||
const flagPath = path.join(symlinkDir, '.caveman-active');
|
||||
// Should not throw
|
||||
safeWriteFlag(flagPath, 'full');
|
||||
// Flag should not exist (target doesn't exist)
|
||||
assert.strictEqual(fs.existsSync(path.join(symlinkDir, '.caveman-active')), false);
|
||||
});
|
||||
|
||||
test('all valid modes round-trip through symlinked parent', (tmp) => {
|
||||
const realDir = path.join(tmp, 'real-config');
|
||||
fs.mkdirSync(realDir, { recursive: true });
|
||||
const symlinkDir = path.join(tmp, 'link-config');
|
||||
fs.symlinkSync(realDir, symlinkDir);
|
||||
|
||||
const flagPath = path.join(symlinkDir, '.caveman-active');
|
||||
|
||||
for (const mode of VALID_MODES) {
|
||||
safeWriteFlag(flagPath, mode);
|
||||
const read = readFlag(flagPath);
|
||||
assert.strictEqual(read, mode, `mode '${mode}' did not round-trip`);
|
||||
}
|
||||
});
|
||||
|
||||
// ---------- Source code audit ----------
|
||||
|
||||
test('safeWriteFlag no longer has blanket symlink parent refusal', (tmp) => {
|
||||
// Verify the old pattern "if (fs.lstatSync(flagDir).isSymbolicLink()) return;"
|
||||
// without ownership check is no longer present
|
||||
const source = fs.readFileSync(
|
||||
path.join(__dirname, '..', 'hooks', 'caveman-config.js'), 'utf8'
|
||||
);
|
||||
|
||||
// The old pattern: check isSymbolicLink on flagDir and immediately return
|
||||
// New pattern: check isSymbolicLink, then realpathSync + ownership verification
|
||||
const lines = source.split('\n');
|
||||
let foundSymlinkCheck = false;
|
||||
let foundOwnershipCheck = false;
|
||||
for (const line of lines) {
|
||||
if (line.includes('isSymbolicLink()') && line.includes('flagDir')) {
|
||||
// This is the lstat check on the parent dir — should NOT be a blanket return
|
||||
foundSymlinkCheck = true;
|
||||
}
|
||||
if (line.includes('realpathSync') || line.includes('getuid') || line.includes('normalizedHome')) {
|
||||
foundOwnershipCheck = true;
|
||||
}
|
||||
}
|
||||
|
||||
assert.ok(foundOwnershipCheck, 'safeWriteFlag should include ownership/home-dir verification');
|
||||
});
|
||||
|
||||
// ---------- Summary ----------
|
||||
|
||||
console.log(`\n${passed} passed, ${failed} failed`);
|
||||
if (failed > 0) process.exit(1);
|
||||
Reference in New Issue
Block a user