Skip to content

fix: UNSAFE_TOOL_OUTPUT_PATH false positive on prefix boundaries#8

Merged
dmchaledev merged 1 commit into
mainfrom
fix/unsafe-output-path-boundary
May 29, 2026
Merged

fix: UNSAFE_TOOL_OUTPUT_PATH false positive on prefix boundaries#8
dmchaledev merged 1 commit into
mainfrom
fix/unsafe-output-path-boundary

Conversation

@dmchaledev

Copy link
Copy Markdown
Contributor

Summary

The UNSAFE_TOOL_OUTPUT_PATH rule used String.startsWith() to check whether a tool's output path falls under a system directory (/etc, /proc, /sys, /boot, /root, /dev). This caused false positives for legitimate paths that merely start with the same characters as a system directory:

Path Before (wrong) After (correct)
/etcfoo/bar ❌ Fired ✅ Safe
/procrastinate/file ❌ Fired ✅ Safe
/device/some-path ❌ Fired ✅ Safe
/rootfs/data ❌ Fired ✅ Safe
/etc/passwd ✅ Fired ✅ Fired
/proc/1/mem ✅ Fired ✅ Fired
/etc (exact) ✅ Fired ✅ Fired

Fix

Replace bare startsWith(dir) with a proper path-boundary check:

const isUnsafe = UNSAFE_OUTPUT_DIRS.some((dir) => {
  const p = tool.outputPath!;
  return p === dir || p.startsWith(dir + '/');
});

This ensures only exact matches or proper subdirectories trigger the rule.

Tests Added

5 new test cases in src/__tests__/scanner.test.ts:

  • /etcfoo — prefix boundary guard (should NOT fire)
  • /procrastinate — prefix boundary guard (should NOT fire)
  • /device — prefix boundary guard (should NOT fire)
  • /rootfs — prefix boundary guard (should NOT fire)
  • /etc exact match (should fire)

Verification

  • npx tsc --noEmit — type checking passes
  • npm test — 75/75 tests pass (was 70)
  • npm run lint — 0 warnings

The UNSAFE_TOOL_OUTPUT_PATH rule used startsWith() to check if a tool's
output path falls under a system directory (/etc, /proc, /sys, /boot,
/root, /dev). This caused false positives for paths like /etcfoo,
/procrastinate, /device, /rootfs — none of which are actually system
directories.

Fix: check for exact match (path === dir) or proper subdirectory
(path.startsWith(dir + '/')).

Added 5 new test cases covering:
- /etcfoo (should NOT fire)
- /procrastinate (should NOT fire)
- /device (should NOT fire)
- /rootfs (should NOT fire)
- /etc exact match (should fire)
@dmchaledev dmchaledev merged commit 0030394 into main May 29, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant