Files
claude-engineering-plugin/docs/solutions/skill-design/claude-permissions-optimizer-classification-fix.md
2026-03-18 01:18:27 -07:00

16 KiB

title, category, date, severity, tags, component, symptoms
title category date severity tags component symptoms
Classification bugs in claude-permissions-optimizer extract-commands script logic-errors 2026-03-18 high
security
classification
normalization
permissions
command-extraction
destructive-commands
dcg
claude-permissions-optimizer
Dangerous commands (find -delete, git push -f) recommended as safe to auto-allow
Safe/common commands (git blame, gh CLI) invisible or misclassified in output
632 commands reported as below-threshold noise due to filtering before normalization
git restore -S (safe unstage) incorrectly classified as red (destructive)

Classification Bugs in claude-permissions-optimizer

Problem

The extract-commands.mjs script in the claude-permissions-optimizer skill had three categories of bugs that affected both security and UX of permission recommendations.

Symptoms observed: Running the skill across 200 sessions reported 632 commands as "below threshold noise" -- suspiciously high. Cross-referencing against the Destructive Command Guard (DCG) project confirmed classification gaps on both spectrums.

Root Cause

1. Threshold before normalization (architectural ordering)

The min-count filter was applied to each raw command before normalization and grouping. Hundreds of variants of the same logical command (e.g., git log --oneline src/foo.ts, git log --oneline src/bar.ts) were each discarded individually for falling below the threshold of 5, even though their normalized form (git log *) had 200+ total uses.

2. Normalization broadens classification

Safety classification happened on the raw command, but the result was carried forward to the normalized pattern. node --version (green via --version$ regex) would normalize to the dangerously broad node *, inheriting the green classification despite node being a yellow-tier base command.

3. Compound command classification leak

Classify ran on the full raw command string, but normalize only used the first command in a compound chain. So cd /dir && git branch -D feature was classified as RED (from the git branch -D part) but normalized to cd *. The red classification from the second command leaked into the first command's pattern, causing cd * to appear in the blocked list.

4. Global risk flags causing false fragmentation

Risk flags (-f, -v) were preserved globally during normalization to keep dangerous variants separate. But -f means "force" in git push -f and "pattern file" in grep -f, while -v means "remove volumes" in docker-compose down -v and "verbose/invert" everywhere else. Global preservation fragmented green patterns unnecessarily (grep -v * separate from grep *) and contaminated benign patterns with wrong risk reasons.

5. Allowlist glob broader than classification intent

Commands with mode-switching flags (sed -i, find -delete, ast-grep --rewrite) were classified green without the flag but normalized to a broad pattern like sed *. The resulting allowlist rule Bash(sed *) would auto-allow the destructive form too, since Claude Code's glob matching treats * as matching everything. The classification was correct for the individual command but the recommended pattern was unsafe.

6. Classification gaps (found via DCG cross-reference)

Security bugs (dangerous classified as green):

  • find unconditionally in GREEN_BASES -- find -delete and find -exec rm passed as safe
  • git push -f regex required -f after other args, missed -f immediately after push
  • git restore -S falsely red (lookahead only checked --staged, not the -S alias)
  • git clean -fd regex required f at end of flag group, missed -fd (f then d)
  • git checkout HEAD -- file pattern didn't allow a ref between checkout and --
  • git branch --force not caught alongside -D
  • Missing RED patterns: npm unpublish, cargo yank, dd of=, mkfs, pip uninstall, apt remove/purge, brew uninstall, git reset --merge

UX bugs (safe commands misclassified):

  • git blame, git shortlog -> unknown (missing from GREEN_COMPOUND)
  • git tag -l, git stash list/show -> yellow instead of green
  • git clone -> unknown (not in any YELLOW pattern)
  • All gh CLI commands -> unknown (no patterns at all)
  • git restore --staged/-S -> red instead of yellow

Solution

Fix 1: Reorder the pipeline

Normalize and group commands first, then apply the min-count threshold to the grouped totals:

// Group ALL non-allowed commands by normalized pattern first
for (const [command, data] of commands) {
  if (isAllowed(command)) { alreadyCovered++; continue; }
  const pattern = "Bash(" + normalize(command) + ")";
  // ... group by pattern, merge sessions, escalate tiers
}

// THEN filter by min-count on GROUPED totals
for (const [pattern, data] of patternGroups) {
  if (data.totalCount < minCount) {
    belowThreshold += data.rawCommands.length;
    patternGroups.delete(pattern);
  }
}

Fix 2: Post-grouping safety reclassification

After grouping, re-classify the normalized pattern itself. If the broader form maps to a more restrictive tier, escalate:

for (const [pattern, data] of patternGroups) {
  if (data.tier !== "green") continue;
  if (!pattern.includes("*")) continue;
  const cmd = pattern.replace(/^Bash\(|\)$/g, "");
  const { tier, reason } = classify(cmd);
  if (tier === "red") { data.tier = "red"; data.reason = reason; }
  else if (tier === "yellow") { data.tier = "yellow"; }
  else if (tier === "unknown") { data.tier = "unknown"; }
}

Fix 3: Classify must match normalize's scope

Classify now extracts the first command from compound chains (&&, ||, ;) and pipe chains before checking patterns, matching what normalize does. Pipe-to-shell (| bash) is excluded from stripping since the pipe itself is the danger.

function classify(command) {
  const compoundMatch = command.match(/^(.+?)\s*(&&|\|\||;)\s*(.+)$/);
  if (compoundMatch) return classify(compoundMatch[1].trim());
  const pipeMatch = command.match(/^(.+?)\s*\|\s*(.+)$/);
  if (pipeMatch && !/\|\s*(sh|bash|zsh)\b/.test(command)) {
    return classify(pipeMatch[1].trim());
  }
  // ... RED/GREEN/YELLOW checks on the first command only
}

Fix 4: Context-specific risk flags

Replaced global -f/-v risk flags with a contextual system. Flags are only preserved during normalization when they're risky for the specific base command:

const CONTEXTUAL_RISK_FLAGS = {
  "-f": new Set(["git", "docker", "rm"]),
  "-v": new Set(["docker", "docker-compose"]),
};

function isRiskFlag(token, base) {
  if (GLOBAL_RISK_FLAGS.has(token)) return true;
  const contexts = CONTEXTUAL_RISK_FLAGS[token];
  if (contexts && base && contexts.has(base)) return true;
  // ...
}

Risk flags are a presentation improvement, not a safety mechanism. Classification + tier escalation handles safety regardless. The contextual approach prevents fragmentation of green patterns (grep -v * merges with grep *) while keeping dangerous variants visible in the blocked table (git push -f * stays separate from git push *).

Commands with mode-switching flags (sed -i, ast-grep --rewrite) are handled via dedicated normalization rules rather than risk flags, since their safe and dangerous forms need entirely different classification.

Fix 5: Mode-preserving normalization

Commands with mode-switching flags get dedicated normalization rules that preserve the safe/dangerous mode flag, producing narrow patterns safe to recommend:

// sed: preserve the mode flag
if (/^sed\s/.test(command)) {
  if (/\s-i\b/.test(command)) return "sed -i *";
  const sedFlag = command.match(/^sed\s+(-[a-zA-Z])\s/);
  return sedFlag ? "sed " + sedFlag[1] + " *" : "sed *";
}

// find: preserve the predicate/action flag
if (/^find\s/.test(command)) {
  if (/\s-delete\b/.test(command)) return "find -delete *";
  if (/\s-exec\s/.test(command)) return "find -exec *";
  const findFlag = command.match(/\s(-(?:name|type|path|iname))\s/);
  return findFlag ? "find " + findFlag[1] + " *" : "find *";
}

GREEN_COMPOUND then matches the narrow normalized forms:

/^sed\s+-(?!i\b)[a-zA-Z]\s/   // sed -n *, sed -e * (not sed -i *)
/^find\s+-(?:name|type|path|iname)\s/  // find -name *, find -type *
/^(ast-grep|sg)\b(?!.*--rewrite)/      // ast-grep * (not ast-grep --rewrite *)

Bare forms without a mode flag (sed *, find *) fall to yellow/unknown since Bash(sed *) would match the destructive variant.

Fix 6: Patch classification gaps

Key regex fixes:

// find: removed from GREEN_BASES; destructive forms caught by RED
{ test: /\bfind\b.*\s-delete\b/, reason: "find -delete permanently removes files" },
{ test: /\bfind\b.*\s-exec\s+rm\b/, reason: "find -exec rm permanently removes files" },
// Safe find via GREEN_COMPOUND:
/^find\b(?!.*(-delete|-exec))/

// git push -f: catch -f in any position
{ test: /git\s+(?:\S+\s+)*push\s+.*-f\b/ },
{ test: /git\s+(?:\S+\s+)*push\s+-f\b/ },

// git restore: exclude both --staged and -S from red
{ test: /git\s+restore\s+(?!.*(-S\b|--staged\b))/ },
// And add yellow pattern for the safe form:
/^git\s+restore\s+.*(-S\b|--staged\b)/

// git clean: match f anywhere in combined flags
{ test: /git\s+clean\s+.*(-[a-z]*f[a-z]*\b|--force\b)/ },

// git branch: catch both -D and --force
{ test: /git\s+branch\s+.*(-D\b|--force\b)/ },

New GREEN_COMPOUND patterns for safe commands:

/^git\s+(status|log|diff|show|blame|shortlog|...)\b/  // added blame, shortlog
/^git\s+tag\s+(-l\b|--list\b)/                         // tag listing
/^git\s+stash\s+(list|show)\b/                          // stash read-only
/^gh\s+(pr|issue|run)\s+(view|list|status|diff|checks)\b/  // gh read-only
/^gh\s+repo\s+(view|list|clone)\b/
/^gh\s+api\b/

New YELLOW_COMPOUND patterns:

/^git\s+(...|clone)\b/           // added clone
/^gh\s+(pr|issue)\s+(create|edit|comment|close|reopen|merge)\b/  // gh write ops

Verification

  • Built a test suite of 70+ commands across both spectrums (dangerous and safe)
  • Cross-referenced against DCG rule packs: core/git, core/filesystem, package_managers
  • Final result: 0 dangerous commands classified as green, 0 safe commands misclassified
  • Repo test suite: 344 tests pass

Prevention Strategies

Pipeline ordering is an architectural invariant

The correct pipeline order is:

filter(allowlist) -> normalize -> group -> threshold -> re-classify(normalized) -> output

The post-grouping safety check that re-classifies normalized patterns containing wildcards is load-bearing. It must never be removed or moved before the grouping step.

The allowlist pattern is the product, not the classification

The skill's output is an allowlist glob like Bash(sed *), not a safety tier. Classification determines whether to recommend a pattern, but the pattern itself must be safe to auto-allow. This creates a critical constraint: commands with mode-switching flags that change safety profile need normalization that preserves the safe mode flag, so the resulting glob can't match the destructive form.

Example: sed -n 's/foo/bar/' file is read-only and safe. But normalizing it to sed * produces Bash(sed *) which also matches sed -i 's/foo/bar/' file (destructive in-place edit). The fix is mode-preserving normalization: sed -n * produces Bash(sed -n *) which is narrow enough to be safe.

This applies to any command where a flag changes the safety profile:

  • sed -n * (green) vs sed -i * (red) -- -n is read-only, -i edits in place
  • find -name * (green) vs find -delete * (red) -- -name is a predicate, -delete removes files
  • ast-grep * (green) vs ast-grep --rewrite * (red) -- default is search, --rewrite modifies files

Commands like these should NOT go in GREEN_BASES (which produces the blanket X * pattern). They need dedicated normalization rules that preserve the mode flag, and GREEN_COMPOUND patterns that match the narrower normalized form.

GREEN_BASES requires proof of no destructive subcommands

Before adding any command to GREEN_BASES, verify it has NO destructive flags or modes. If in doubt, use GREEN_COMPOUND with explicit negative lookaheads. Commands that should never be in GREEN_BASES: find, xargs, sed, awk, curl, wget.

Regex negative lookaheads must enumerate ALL flag aliases

Every flag exclusion must cover both long and short forms. For git, consult git <subcmd> --help for every alias. Example: (?!.*(-S\b|--staged\b)) not just (?!.*--staged\b).

Classify and normalize must operate on the same scope

If normalize extracts the first command from compound chains, classify must do the same. Otherwise a dangerous second command (git branch -D) contaminates the first command's pattern (cd *). Any future change to normalize's scoping logic must be mirrored in classify.

Risk flags are contextual, not global

Short flags like -f and -v mean different things for different commands. Adding a short flag to GLOBAL_RISK_FLAGS will fragment every green command that uses it innocently. Use CONTEXTUAL_RISK_FLAGS with explicit base-command sets instead. For commands where a flag completely changes the safety profile (sed -i, ast-grep --rewrite), use a dedicated normalization rule rather than a risk flag.

GREEN_BASES must exclude commands useless as allowlist rules

Commands like cd and cal are technically safe but useless as standalone allowlist rules in agent contexts (shell state doesn't persist, novelty commands never used). Including them creates noise in recommendations. Before adding to GREEN_BASES, ask: would a user actually benefit from Bash(X *) in their allowlist?

RISK_FLAGS must stay synchronized with RED_PATTERNS

Every flag in a RED_PATTERNS regex must have a corresponding entry in GLOBAL_RISK_FLAGS or CONTEXTUAL_RISK_FLAGS so normalization preserves it.

External References

Destructive Command Guard (DCG)

Repository: https://github.com/Dicklesworthstone/destructive_command_guard

DCG is a Rust-based security hook with 49+ modular security packs that classify destructive commands. Its pack-based architecture maps well to the classifier's rule sections:

DCG Pack Classifier Section
core/filesystem RED_PATTERNS (rm, find -delete, chmod, chown)
core/git RED_PATTERNS (force push, reset --hard, clean -f, filter-branch)
strict_git Additional git patterns (rebase, amend, worktree remove)
package_managers RED_PATTERNS (publish, unpublish, uninstall)
system RED_PATTERNS (sudo, reboot, kill -9, dd, mkfs)
containers RED_PATTERNS (--privileged, system prune, volume rm)

DCG's rule packs are a goldmine for validating classifier completeness. When adding new command categories or modifying rules, cross-reference the corresponding DCG pack. Key packs not yet fully cross-referenced: database, kubernetes, cloud, infrastructure, secrets.

DCG also demonstrates smart detection patterns worth studying:

  • Scans heredocs and inline scripts (python -c, bash -c)
  • Context-aware (won't block grep "rm -rf" in string literals)
  • Explicit safe-listing of temp directory operations (rm -rf /tmp/*)

Testing Recommendations

Future work should add a dedicated classification test suite covering:

  1. Red boundary tests: Every RED_PATTERNS entry with positive match AND safe variant
  2. Green boundary tests: Every GREEN_BASES/COMPOUND with destructive flag variants
  3. Normalization safety tests: Verify that classify(normalize(cmd)) never returns a lower tier than classify(cmd)
  4. DCG cross-reference tests: Data-driven test with one entry per DCG pack rule, asserting never-green
  5. Broadening audit: For each green rule, generate variants with destructive flags and assert they are NOT green
  6. Compound command tests: Verify that cd /dir && git branch -D feat classifies as green (cd), not red
  7. Contextual flag tests: Verify grep -v pattern normalizes to grep * (not grep -v *), while docker-compose down -v preserves -v
  8. Allowlist safety tests: For every green pattern containing *, verify that the glob cannot match a known destructive variant (e.g., Bash(sed -n *) must not match sed -i)