llm-security/tests/scanners/auto-cleaner-traversal.test.mjs
Kjell Tore Guttormsen dfb663fa61 fix(llm-security): F-2/F-3 — contain path traversal + kill npm-view shell injection
Session B of the security-fix track. Both sinks trusted untrusted strings at a
filesystem/subprocess boundary; same fix family as F-1.

F-2 (HIGH, arbitrary file write) — scanners/auto-cleaner.mjs: applyFixes() did
resolve(targetPath, f.file) with no containment, then wrote the cleaned content
back. f.file is untrusted (scanned-repo filenames, or a fully attacker-chosen
--findings file), so file: "../../.claude/settings.json" let the cleaner modify
files OUTSIDE the scanned tree. Add a prefix-containment check before grouping:
absPath must equal targetPath or start with targetPath + sep, else the finding
is refused and reported as skipped. (Documented residual gap: prefix containment
does not stop a symlink inside the tree pointing out — noted inline.)

F-3 (HIGH, command injection, pre-confirmation) — hooks/scripts/
pre-install-supply-chain.mjs: inspectNpmPackage ran execSafe(`npm view ${spec}
--json`), a shell string. spec derives from package tokens parsed out of the
scanned Bash command, so a metachar-bearing token reached the shell on
PreToolUse(Bash) — BEFORE the install, so it ran even if the user then denied
the command. Switch to spawnSync('npm', ['view', spec, '--json']) (no shell);
spec is passed as one argv element. The static `npm audit --json` execSafe call
is left as-is (no interpolation).

TDD (repro -> red -> green):
- tests/scanners/auto-cleaner-traversal.test.mjs: a "../secret.txt" finding must
  not rewrite the outside file; contained files still get cleaned.
- tests/hooks/supply-chain-injection.test.mjs: `npm install $(>/abs/PWNED)`
  (redirect-only $(...) survives normalizeBashExpansion + the whitespace split)
  must not create the sentinel.

Closing gates: full node --test suite 1863/0 (was 1860; +3); gitleaks clean;
F-1 regression re-run GREEN (sink still closed).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01V3s6WnubSSrFjAQTLQdVbG
2026-06-20 11:09:13 +02:00

96 lines
4.1 KiB
JavaScript
Raw Permalink Blame History

This file contains invisible Unicode characters

This file contains invisible Unicode characters that are indistinguishable to humans but may be processed differently by a computer. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

// auto-cleaner-traversal.test.mjs — Security regression for F-2 (HIGH, arbitrary file write).
//
// auto-cleaner's applyFixes() historically did `resolve(targetPath, f.file)` with no
// containment check, then wrote the modified content back to that path. `f.file` comes
// from findings JSON — untrusted scanned-repo filenames, or a fully attacker-chosen
// `--findings` file. A finding with `file: "../secret.txt"` therefore let the cleaner
// modify (overwrite) a file OUTSIDE the scanned tree.
//
// This test places an auto-fixable file outside the target dir and points an auto-tier
// finding at it via "../secret.txt". It must FAIL while the sink is uncontained (the
// outside file gets rewritten) and PASS once applyFixes refuses paths that escape the
// target (prefix containment), reporting them as skipped instead of writing.
import { describe, it } from 'node:test';
import assert from 'node:assert/strict';
import { mkdtempSync, mkdirSync, writeFileSync, readFileSync, rmSync } from 'node:fs';
import { join } from 'node:path';
import { tmpdir } from 'node:os';
import { resetCounter } from '../../scanners/lib/output.mjs';
import { applyFixes } from '../../scanners/auto-cleaner.mjs';
const ZW = ''; // zero-width space — strip_zero_width will remove it
describe('auto-cleaner path-traversal regression (F-2)', () => {
it('refuses to write a finding whose file path escapes the target directory', async () => {
// root/ <- mkdtemp parent
// secret.txt <- the escape target, OUTSIDE the scanned tree
// scan-target/ <- the directory actually passed to the cleaner
const root = mkdtempSync(join(tmpdir(), 'auto-cleaner-traversal-'));
const target = join(root, 'scan-target');
const escape = join(root, 'secret.txt');
try {
mkdirSync(target, { recursive: true });
// The escape file carries a zero-width char so the (auto-tier) strip_zero_width
// op WOULD change it — proving a write actually reaches outside the tree if
// containment is missing.
const original = `secret${ZW}value\n`;
writeFileSync(escape, original, 'utf-8');
// Untrusted finding: classified 'auto' (UNI zero-width), file traverses up and out.
const findings = [{
id: 'TRAVERSAL-1',
scanner: 'UNI',
title: 'Zero-width characters detected',
severity: 'high',
file: '../secret.txt',
}];
resetCounter();
const { fixes } = await applyFixes(target, findings, /* dryRun */ false);
// The file outside the target must be byte-for-byte untouched.
assert.equal(
readFileSync(escape, 'utf-8'),
original,
'PATH TRAVERSAL: auto-cleaner rewrote a file outside the scanned target directory',
);
// And the escaping finding must be surfaced as refused/skipped, never applied.
const escaped = fixes.find(f => f.file === '../secret.txt');
assert.ok(escaped, 'the escaping finding should be reported');
assert.notEqual(escaped.status, 'applied', 'escaping finding must not be applied');
} finally {
rmSync(root, { recursive: true, force: true });
}
});
it('still applies fixes to files contained within the target directory', async () => {
const target = mkdtempSync(join(tmpdir(), 'auto-cleaner-contained-'));
try {
const inside = join(target, 'inside.md');
writeFileSync(inside, `---\ntitle: x${ZW}y\n---\nbody\n`, 'utf-8');
const findings = [{
id: 'CONTAINED-1',
scanner: 'UNI',
title: 'Zero-width characters detected',
severity: 'high',
file: 'inside.md',
}];
resetCounter();
const { fixes } = await applyFixes(target, findings, /* dryRun */ false);
assert.ok(
!readFileSync(inside, 'utf-8').includes(ZW),
'contained file should have been cleaned',
);
const applied = fixes.find(f => f.file === 'inside.md' && f.status === 'applied');
assert.ok(applied, 'a contained auto-fix should be applied');
} finally {
rmSync(target, { recursive: true, force: true });
}
});
});