Skip to content

Commit 0be2913

Browse files
committed
feat: add allow_flags whitelist and rename decision constants for clarity
Rename NoOpinion->Prompt and DenyFlags->PromptFlags so names match actual behavior, add AllowFlags whitelist to prompt on any unrecognized flag, and add gofmt/gofumpt/goimports rules using the new whitelist.
1 parent 548b864 commit 0be2913

11 files changed

Lines changed: 361 additions & 172 deletions

File tree

‎AGENTS.md‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -33,28 +33,28 @@ stdin (hook JSON, Crush or Claude Code format)
3333
| `internal/bash` | Tree-sitter–based bash parser. Extracts `Command` structs from a bash AST. Detects "complex" constructs (command substitution `$()`, subshells `()`, process substitution `<()`, arithmetic `$(())`) and parse errors, both will not auto-allow. |
3434
| `internal/checker` | Orchestrates parse + rule check. Tracks `cd` to ensure the working directory stays within `RootDir`. Resolves `~/...` paths against `HomeDir`. Any `cd` that would escape the root causes a deny. |
3535
| `internal/config` | Layered config loading. Reads global (`$XDG_CONFIG_HOME/crushout/crushout.{yml,yaml}`) and repo (`<rootDir>/.crushout.{yml,yaml}`) config, deep-merges both over built-in defaults. Repo config wins over global. Supports `overwrite_defaults: true` per-layer to replace the accumulated base entirely. |
36-
| `internal/rules` | Recursive rule engine (`Rule` struct with `Subcommands`, `DenyFlags`, `Default`). `defaults.go` contains the full built-in ruleset mapping command names to their allow/deny rules. |
36+
| `internal/rules` | Recursive rule engine (`Rule` struct with `Subcommands`, `PromptFlags`, `AllowFlags`, `Default`). `defaults.go` contains the full built-in ruleset mapping command names to their allow/deny rules. |
3737

3838
## Key Design Decisions
3939

40-
- **Fail-closed**: anything ambiguous or unknown falls through to the normal permission prompt. Parse errors, complex bash constructs, unknown commands, commands with `$` in the name, and missing rules all produce a "no opinion" / "ask" result.
41-
- **Three outcomes**: `allow` (auto-approve), `deny` (hard-block with reason), and the default "no opinion" (fall through to normal permission flow). The built-in rules never deny; deny rules come from `.crushout.yml` config only.
40+
- **Fail-closed**: anything ambiguous or unknown falls through to the normal permission prompt. Parse errors, complex bash constructs, unknown commands, commands with `$` in the name, and missing rules all produce a `prompt` result.
41+
- **Three outcomes**: `allow` (auto-approve), `deny` (hard-block with reason), and `prompt` (fall through to normal permission flow). The built-in rules never deny; deny rules come from `.crushout.yml` config only.
4242
- **"Complex" bash is rejected outright**: command substitution, process substitution, subshells, and arithmetic expansion set `IsComplex=true` and skip command extraction entirely.
4343
- **Layered config**: crushout reads two config layers merged in order — global (`$XDG_CONFIG_HOME/crushout/crushout.{yml,yaml}`) then repo (`<rootDir>/.crushout.{yml,yaml}`). Each layer is deep-merged over the accumulated base, with later layers winning. `overwrite_defaults: true` in a layer replaces the accumulated base entirely (so a global `overwrite_defaults: true` drops built-ins that no later layer can recover). Scalar fields like `rtk_rewrite` use `*bool` internally so an unset value is distinguishable from an explicit `false`.
4444
- **`cd` tracking**: the checker tracks the current working directory across `cd` commands in a chain. `cd` with `$VAR`, `~` (if home is outside root), `-`, or any path resolving outside `RootDir` denies the whole command.
45-
- **Output convention**: returning a protocol-specific "no opinion" payload (`{}` for Crush, `ask` for Claude Code) means fall through to normal prompt. `allow` auto-approves. `deny` hard-blocks with a reason string.
45+
- **Output convention**: returning a protocol-specific `prompt` payload (`{}` for Crush, `ask` for Claude Code) means fall through to normal prompt. `allow` auto-approves. `deny` hard-blocks with a reason string.
4646

4747
## Testing Patterns
4848

4949
- Standard `testing` package only, no assertion libraries. Tests define local helpers (`assertNoError`, `assertCommand`).
5050
- `checker_test.go` uses `newTestChecker()` with `RootDir: "/home/user/project"` and `HomeDir: "/home/user"`.
5151
- Config tests in `config_test.go` cover YAML parsing, shorthand syntax, single-layer loading (`loadFirst`), layered merging (`load`), `resolveRtkRewrite`, and `applyLayer`/`buildRules` overwrite matrix.
52-
- Rule tests in `rule_test.go` construct minimal `Rule` trees to test subcommand resolution, deny flags, and nesting, then also test the full `Default` ruleset.
52+
- Rule tests in `rule_test.go` construct minimal `Rule` trees to test subcommand resolution, `PromptFlags`, `AllowFlags`, and nesting, then also test the full `Default` ruleset.
5353
- E2E tests in `tests/e2e/` run the full binary against JSONL test cases (`cases_crush.jsonl`, `cases_claude.jsonl`) via `run.sh`.
5454
- The `cmd/crushout` and `internal/hook` packages have no unit tests.
5555

5656
## Gotchas
5757

58-
- The tree-sitter bash grammar treats `git -C /tmp status` with `-C` as an anonymous (un-named) child node. This means `-C` does **not** appear in `cmd.Args`. The deny works via `DenyFlags` on the rule which checks the raw args, but the tree-sitter parse won't include it in the structured args. This is why `checker.isReadOnly` uses `filepath.Base` on names with `/` and rejects names containing `$` or backticks.
59-
- `Rule.resolve` walks args as a subcommand chain. The first arg matching a subcommand key descends into that sub-rule, consuming the arg. Remaining args are then checked against `DenyFlags` at the new level. This means flag position matters: `git -C /tmp status` has `-C` checked at the top-level git rule, but `git branch -l` descends into the `branch` sub-rule.
58+
- The tree-sitter bash grammar treats `git -C /tmp status` with `-C` as an anonymous (un-named) child node. This means `-C` does **not** appear in `cmd.Args`. The deny works via `PromptFlags` on the rule which checks the raw args, but the tree-sitter parse won't include it in the structured args. This is why `checker.isReadOnly` uses `filepath.Base` on names with `/` and rejects names containing `$` or backticks.
59+
- `Rule.resolve` walks args as a subcommand chain. The first arg matching a subcommand key descends into that sub-rule, consuming the arg. Remaining args are then checked against `PromptFlags` and `AllowFlags` at the new level. This means flag position matters: `git -C /tmp status` has `-C` checked at the top-level git rule, but `git branch -l` descends into the `branch` sub-rule.
6060
- `cd` with no arguments (bare `cd`) resolves to `$HOME`. It's only allowed if `HomeDir` is within `RootDir`.

‎README.md‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ Or use a full path in either:
146146
| `git push` | 🔒 prompt | mutable subcommand |
147147
| `git commit -m 'fix'` | 🔒 prompt | mutable subcommand |
148148
| `git branch new-feature` | 🔒 prompt | `branch` without list flag |
149-
| `git branch -D old` | 🔒 prompt | `-D` in deny flags |
149+
| `git branch -D old` | 🔒 prompt | `-D` in prompt flags |
150150
| `git tag v1.0.0` | 🔒 prompt | `tag` without `-l` |
151151
| `git stash` | 🔒 prompt | bare `stash` = push |
152152
| `git -C /tmp status` | 🔒 prompt | `-C` is denied |
@@ -189,15 +189,16 @@ Edit `internal/rules/defaults.go`. The rule type is recursive:
189189
```go
190190
var Default = map[string]*Rule{
191191
"my-tool": {
192-
Default: rules.Allow, // allow unknown subcommands
193-
DenyFlags: []string{"--dangerous"}, // prompt on these flags
192+
Default: rules.Allow, // allow unknown subcommands
193+
PromptFlags: []string{"--dangerous"}, // prompt on these flags
194+
AllowFlags: []string{"-l", "-d", "-s"}, // only these flags are safe
194195
Subcommands: map[string]*Rule{
195196
"read": {Default: rules.Allow},
196-
"write": {Default: rules.NoOpinion}, // prompt
197+
"write": {Default: rules.Prompt},
197198
"db": {
198199
Subcommands: map[string]*Rule{
199-
"migrate": {Default: rules.NoOpinion},
200-
"seed": {Default: rules.NoOpinion},
200+
"migrate": {Default: rules.Prompt},
201+
"seed": {Default: rules.Prompt},
201202
},
202203
},
203204
},
@@ -207,9 +208,10 @@ var Default = map[string]*Rule{
207208

208209
Resolution walks arguments left-to-right:
209210

210-
1. `DenyFlags` are checked at each level against all remaining args
211-
2. If an arg matches a `Subcommands` key, descend into that rule
212-
3. When no deeper match is found, use `Default`
211+
1. `PromptFlags` are checked at each level against all remaining args. If any arg matches, the result is `prompt`.
212+
2. `AllowFlags` are checked at each level against all remaining args starting with `-`. If any flag is not in the allow list, the result is `prompt`.
213+
3. If an arg matches a `Subcommands` key, descend into that rule
214+
4. When no deeper match is found, use `Default`
213215

214216
## Config files
215217

@@ -273,7 +275,8 @@ Because layers build on each other, a `overwrite_defaults: true` in the global l
273275
| `rules` | map | Map of command name → rule. |
274276
| `rules.*` | string or map | Shorthand (`allow`, `deny`, `prompt`) or full rule mapping. |
275277
| `rules.*.decision` | string | Decision for unknown subcommands: `allow`, `deny`, or `prompt`. Defaults to `prompt` if not set. |
276-
| `rules.*.deny_flags` | []string | Flags that always require confirmation. |
278+
| `rules.*.prompt_flags` | []string | Flags that always trigger a prompt (blacklist). |
279+
| `rules.*.allow_flags` | []string | If set, any flag-like arg (`-` prefix) not in this list triggers a prompt (whitelist). |
277280
| `rules.*.message` | string | Custom message shown when denied. Only used with `decision: deny`. |
278281
| `rules.*.subcommands` | map | Recursive map of subcommand name → rule. |
279282

‎cmd/crushout/main.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ func main() {
2020

2121
// If tool is not bash we just skip it
2222
if !hook.IsBashTool(input) {
23-
out, err := input.FormatDecision(rules.NoOpinion, "", "")
23+
out, err := input.FormatDecision(rules.Prompt, "", "")
2424
if err != nil {
2525
fmt.Fprintf(os.Stderr, "could not serialize output: %v\n", err)
2626
os.Exit(1)

‎default.nix‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
buildGoApplication,
44
}:
55
let
6-
version = "0.7.0";
6+
version = "0.8.0";
77
in
88
buildGoApplication {
99
inherit version;

‎internal/checker/checker.go‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,24 +16,24 @@ type Checker struct {
1616
}
1717

1818
// Check evaluates the input command string and returns a Decision.
19-
// Allow means auto-approve, Deny means hard block, NoOpinion means
19+
// Allow means auto-approve, Deny means hard block, Prompt means
2020
// let the normal permission prompt handle it.
2121
func (c *Checker) Check(input string) (rules.Decision, string, error) {
2222
result, err := bash.Parse(input)
2323
if err != nil {
24-
return rules.NoOpinion, "", nil
24+
return rules.Prompt, "", nil
2525
}
2626

2727
if result.HasError || result.IsComplex || result.HasRedirect || len(result.Commands) == 0 {
28-
return rules.NoOpinion, "", nil
28+
return rules.Prompt, "", nil
2929
}
3030

3131
final := rules.Allow
3232
cwd := c.RootDir
3333
for _, cmd := range result.Commands {
3434
if cmd.Name == "cd" {
3535
if !c.isSafeCD(cmd, &cwd) {
36-
return rules.NoOpinion, "", nil
36+
return rules.Prompt, "", nil
3737
}
3838
continue
3939
} else if cmd.Name == "rtk" && len(cmd.Args) > 0 {
@@ -51,8 +51,8 @@ func (c *Checker) Check(input string) (rules.Decision, string, error) {
5151
switch d {
5252
case rules.Deny:
5353
return rules.Deny, msg, nil
54-
case rules.NoOpinion:
55-
final = rules.NoOpinion
54+
case rules.Prompt:
55+
final = rules.Prompt
5656
}
5757
}
5858

@@ -65,12 +65,12 @@ func (c *Checker) checkCommand(cmd bash.Command) (rules.Decision, string) {
6565
name = filepath.Base(name)
6666
}
6767
if strings.Contains(name, "$") || strings.Contains(name, "`") {
68-
return rules.NoOpinion, ""
68+
return rules.Prompt, ""
6969
}
7070

7171
rule, exists := c.Rules[name]
7272
if !exists {
73-
return rules.NoOpinion, ""
73+
return rules.Prompt, ""
7474
}
7575

7676
d, msg := rule.Resolve(cmd.Args)

0 commit comments

Comments
 (0)