Skip to content

fix(security): anchor rm-deletion regex with \b to stop false positives#1374

Open
IanGordonOne wants to merge 2 commits into
danielmiessler:mainfrom
IanGordonOne:fix/rm-protection-regex-word-boundary
Open

fix(security): anchor rm-deletion regex with \b to stop false positives#1374
IanGordonOne wants to merge 2 commits into
danielmiessler:mainfrom
IanGordonOne:fix/rm-protection-regex-word-boundary

Conversation

@IanGordonOne

Copy link
Copy Markdown

Problem

The bash.blocked rm-protection rules in the SecurityPipeline use an unanchored rm\s..., e.g.:

- pattern: rm\s.*-\w*r.*\s+/(\s|$)
  reason: Recursive deletion of system root (/).

Because rm has no word boundary and the .* wildcards span the whole command, this matches the rm inside ordinary words like confirm, then any hyphenated word (matching -\w*r), then a spaced / in prose — silently blocking entirely benign commands.

Reproduction (both blocked by the current pattern)

echo "confirm the fire-water plan / setback"

Real-world: routine note/issue text that mentions "confirm", a hyphenated term, and a "/" path trips it. (Hit this repeatedly running bd/git commands with descriptive text.)

Fix

Add a \b word boundary before rm. In "confirm" the r is preceded by a word char (no boundary) so it no longer matches; a real rm (start of command / after a space / ;) still does. Real-threat coverage is unchanged.

Verified in Node:

/rm\s.*-\w*r.*\s+\/(\s|$)/    -> matches "rm -rf /"  AND  "confirm ... fire-water ... / ..."   (the bug)
/\brm\s.*-\w*r.*\s+\/(\s|$)/  -> matches "rm -rf /"  but NOT the confirm/fire-water/"/" string

rm -rf /, rm -fr /, sudo rm -rf /, and rm -r --no-preserve-root / all still block.

Scope

Anchors the six -\w*r patterns in Releases/v5.0.0/.claude/PAI/USER/SECURITY/PATTERNS.yaml and the two (root + home) in Releases/v5.0.0/.claude/PAI/DOCUMENTATION/Security/Patterns.example.yaml.

The four path-specific rm\s.*/<file> patterns (PATTERNS.yaml ~lines 54-60) share the same unanchored-rm weakness but require a specific path token, so they false-positive far less — left out to keep this focused; happy to add \b there too if you'd like.

Applied and live-verified on my own PAI instance before submitting.

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