Skip to content

fix(danger): close pipe-fed xargs bypass, gate root-level and git data-loss verbs#103

Merged
jkyberneees merged 1 commit into
mainfrom
fix/danger-classifier-xargs-chmod-git
Jul 26, 2026
Merged

fix(danger): close pipe-fed xargs bypass, gate root-level and git data-loss verbs#103
jkyberneees merged 1 commit into
mainfrom
fix/danger-classifier-xargs-chmod-git

Conversation

@jkyberneees

Copy link
Copy Markdown
Contributor

Summary

Fixes three danger-classifier findings, all verified against the current code before fixing:

1. Destructive-command bypass via pipe-fed xargs — HIGH

echo "/" | xargs rm -rf was classified local_writeallow, executing a root-destructive command with no prompt and no deny. The classifier composed here-strings and command substitutions into the downstream command, but did not model piped stdin as arguments to xargs.

Fix: when a pipeline's sink is xargs invoking a destructive/system verb, statically determinable upstream literal payloads (echo / printf args) are composed onto the inner command before classification — echo "/" | xargs rm -rf now classifies exactly like rm -rf / (destructive → deny). When the payload is not statically determinable (cat file | …, find … | …, $VARS, multi-stage producers) and the inner verb is dangerous-capable (rm, shred, dd, chmod, chown, chattr, mv, cp, ln, install, tee, mkfs.* family), the pipeline fails closed as unknown (deny-by-default). Benign sinks (… | xargs grep, … | xargs wc -l) and benign payloads (echo ./tmpfile | xargs rm) keep their original classification.

2. Recursive chmod at system roots not treated as system_write — MEDIUM

chmod -R 777 / was local_write → allow, while the equivalent chown -R nobody / was system_write → prompt. Related: mv / /tmp/x was local_write while mv ~/.ssh /tmp/stolen prompted.

Fix: ClassifyPath("/") now returns system_write, so mutations aimed at the filesystem root can never downgrade to local_write, regardless of verb. chattr was added to writePrefixes so recursive attribute flips (chattr -R +i /) get the same operand scan as chmod/chown (it previously fell through to unknown).

3. Irreversible git data-loss verbs classified safe — MEDIUM

git clean -fdx, git reset --hard, git checkout -- ., and git branch -D were safe → allow, so a prompt-injection payload (or a confused model) could wipe a working tree with zero friction.

Fix: these now classify as system_writeprompt by default (policy change — affects agent ergonomics, documented):

  • git clean -f* / --force (unless -n / --dry-run is also given)
  • git reset --hard
  • git checkout -f / git checkout -- <path> / git checkout .
  • git restore <pathspec> (worktree; --staged-only stays safe)
  • git branch -D / --delete --force
  • git stash drop / git stash clear
  • git reflog expire

Non-destructive forms stay safe: git clean -n, git checkout main, git checkout -b, git branch -d, git stash, git stash pop, git reflog, git restore --staged.

Tests

  • New regression suites in internal/danger/hardening_test.go:
    • TestHardening_XargsPipedPayload — composed-payload variants (echo "/" | xargs rm -rf, -0, -I P, wrapper chains), fail-closed variants (cat, find, yes, $VAR, multi-stage upstream), and benign no-regression cases; pins the default-policy action (deny).
    • TestHardening_RootLevelMutationTargetschmod -R 777 /, chattr -R +i /, mv / /tmp/x, ClassifyPath("/"), plus benign chmod/chattr unchanged.
    • TestHardening_GitDataLossVerbs — every reclassified verb plus safe-form no-regression cases; pins the default-policy action (prompt).
  • go test ./... -count=1 ✅, go test -race ./internal/danger/ ✅, golangci-lint run internal/danger/... → 0 issues ✅

Docs updated: docs/SECURITY.md, docs/CLI.md, AGENTS.md.

…a-loss verbs

Fixes three classifier findings:

1. Destructive-command bypass via pipe-fed xargs (HIGH): echo "/" | xargs
   rm -rf classified as local_write (auto-allow) because piped stdin was
   never modelled as xargs arguments. When a pipeline's sink is xargs
   invoking a destructive/system verb, statically determinable upstream
   payloads (echo/printf args) are now composed onto the inner command
   before classification; when the payload is not statically determinable
   and the inner verb is dangerous-capable, the pipeline fails closed
   (unknown -> deny).

2. Recursive chmod at system roots not treated as system_write (MEDIUM):
   ClassifyPath("/") fell through to local_write, so chmod -R 777 / and
   mv / /tmp/x ran without approval while chown -R nobody / prompted.
   The filesystem root now classifies as system_write, and chattr is
   handled with the same operand scan as chmod.

3. Irreversible git data-loss verbs classified safe (MEDIUM): git
   clean -f*, reset --hard, checkout -f/--/<pathspec>, restore (worktree),
   branch -D / --delete --force, stash drop|clear, and reflog expire now
   classify as system_write (prompt-by-default) instead of safe. Dry-run
   and non-destructive forms (clean -n, checkout <branch>, branch -d,
   restore --staged, stash pop) stay safe.

Regression tests pin the new classifications and the default-policy
actions; docs updated (SECURITY.md, CLI.md, AGENTS.md).
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 26, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
odek 1afb04d Commit Preview URL

Branch Preview URL
Jul 26 2026, 08:13 AM

@jkyberneees
jkyberneees merged commit 6265f39 into main Jul 26, 2026
8 checks 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