diff --git a/README.md b/README.md index de57788..befc1be 100644 --- a/README.md +++ b/README.md @@ -108,7 +108,9 @@ cordon pass revoke - All commands accept `--json` for structured output. Schemas not finalised at this time. - `` can be a file path, folder path, glob pattern, or command pattern. -Examples: `src/main.go`, `src/`, `**/*.env`, `git push --force*`. +Examples: `src/main.go`, `src/`, `**/*.env`, `git push *--force*`. +- File globs support recursive `**` matching. +- Command rules evaluate direct commands and common wrapped forms (for example `sh -c` and `bash -lc`). ## Documentation diff --git a/cli/internal/hook/bash_test.go b/cli/internal/hook/bash_test.go new file mode 100644 index 0000000..bc715aa --- /dev/null +++ b/cli/internal/hook/bash_test.go @@ -0,0 +1,33 @@ +package hook + +import "testing" + +func TestBashReadTargets_HeadAndDelimiters(t *testing.T) { + got := bashReadTargets("head -n 20 README.md && cat .env") + wantHas := []string{"README.md", ".env"} + for _, w := range wantHas { + found := false + for _, g := range got { + if g == w { + found = true + break + } + } + if !found { + t.Fatalf("targets %#v missing expected %q", got, w) + } + } +} + +func TestReSedInPlaceTargets_Variants(t *testing.T) { + tests := []string{ + `sed -i 's/a/b/' file.txt`, + `sed -i.bak 's/a/b/' file.txt`, + } + for _, cmd := range tests { + got := reSedInPlaceTargets(cmd) + if len(got) != 1 || got[0] != "file.txt" { + t.Fatalf("reSedInPlaceTargets(%q) = %#v, want [file.txt]", cmd, got) + } + } +} diff --git a/cli/internal/hook/commandrule.go b/cli/internal/hook/commandrule.go index 085b78b..0206875 100644 --- a/cli/internal/hook/commandrule.go +++ b/cli/internal/hook/commandrule.go @@ -128,6 +128,64 @@ func splitCompoundCommand(command string) []string { return segments } +// expandCommandRuleSegments returns the command segments to evaluate for +// command-rule enforcement, including common shell wrapper inner commands. +func expandCommandRuleSegments(segment string) []string { + return expandCommandRuleSegmentsDepth(strings.TrimSpace(segment), 0, 2) +} + +func expandCommandRuleSegmentsDepth(segment string, depth, maxDepth int) []string { + if segment == "" { + return nil + } + out := []string{segment} + if depth >= maxDepth { + return out + } + + argv, ok := parseShellArgv(segment) + if !ok || len(argv) == 0 { + return out + } + inner, ok := unwrapShellCommandArg(argv) + if !ok || strings.TrimSpace(inner) == "" { + return out + } + for _, innerSeg := range splitCompoundCommand(inner) { + out = append(out, expandCommandRuleSegmentsDepth(innerSeg, depth+1, maxDepth)...) + } + return out +} + +func unwrapShellCommandArg(argv []string) (string, bool) { + if len(argv) < 3 { + return "", false + } + switch strings.ToLower(argv[0]) { + case "sh", "bash", "zsh", "dash", "ksh": + default: + return "", false + } + + for i := 1; i < len(argv)-1; i++ { + if isShellCommandOption(argv[i]) { + return argv[i+1], true + } + } + return "", false +} + +func isShellCommandOption(tok string) bool { + if tok == "-c" || tok == "--command" { + return true + } + if !strings.HasPrefix(tok, "-") || len(tok) <= 1 { + return false + } + // Combined short options where c appears, e.g. -lc or -cl. + return strings.Contains(tok[1:], "c") +} + // commandRuleDenyReason returns the denial message for a matched command rule. func commandRuleDenyReason(rule *MatchedRule, agent string) string { if supportsMCPElicitation(agent) { diff --git a/cli/internal/hook/commandrule_test.go b/cli/internal/hook/commandrule_test.go index eba173c..25fc8a1 100644 --- a/cli/internal/hook/commandrule_test.go +++ b/cli/internal/hook/commandrule_test.go @@ -68,3 +68,82 @@ func TestCommandRuleDenyReason_OmitsMCPForGeminiAndOpenCode(t *testing.T) { }) } } + +func TestCheckBuiltinRules_DenyCordonCommands(t *testing.T) { + rule := CheckBuiltinRules("cordon status") + if rule == nil { + t.Fatal("expected built-in deny rule for cordon status, got nil") + } + if rule.Pattern != "cordon *" { + t.Fatalf("rule.Pattern = %q, want %q", rule.Pattern, "cordon *") + } +} + +func TestCheckBuiltinRules_AllowOverridesDenyForCordonHook(t *testing.T) { + rule := CheckBuiltinRules("cordon hook") + if rule != nil { + t.Fatalf("expected built-in allow override for cordon hook, got deny rule %q", rule.Pattern) + } +} + +func TestExpandCommandRuleSegments_UnwrapsShellCommand(t *testing.T) { + got := expandCommandRuleSegments(`bash -lc 'git commit -m "test" && git status'`) + wantHas := []string{ + `bash -lc 'git commit -m "test" && git status'`, + `git commit -m "test"`, + "git status", + } + for _, w := range wantHas { + found := false + for _, g := range got { + if g == w { + found = true + break + } + } + if !found { + t.Fatalf("segments %#v missing expected %q", got, w) + } + } +} + +func TestExpandCommandRuleSegmentsDepth_RespectsMaxDepth(t *testing.T) { + got := expandCommandRuleSegmentsDepth(`bash -lc 'sh -c "git status"'`, 0, 1) + wantHas := []string{ + `bash -lc 'sh -c "git status"'`, + `sh -c "git status"`, + } + for _, w := range wantHas { + found := false + for _, g := range got { + if g == w { + found = true + break + } + } + if !found { + t.Fatalf("segments %#v missing expected %q", got, w) + } + } + for _, g := range got { + if g == "git status" { + t.Fatalf("segments %#v unexpectedly contains depth-limited inner command", got) + } + } +} + +func TestUnwrapShellCommandArg_LongCommandOption(t *testing.T) { + got, ok := unwrapShellCommandArg([]string{"bash", "--command", "git status"}) + if !ok { + t.Fatal("expected unwrap success for --command") + } + if got != "git status" { + t.Fatalf("unwrap = %q, want git status", got) + } +} + +func TestIsShellCommandOption_CombinedShortFlags(t *testing.T) { + if !isShellCommandOption("-lc") { + t.Fatal("expected -lc to be recognized as command-carrying option") + } +} diff --git a/cli/internal/hook/hook.go b/cli/internal/hook/hook.go index 92dde30..c77e4f2 100644 --- a/cli/internal/hook/hook.go +++ b/cli/internal/hook/hook.go @@ -372,38 +372,12 @@ func evaluateBash(payload hookPayload, w io.Writer, errW io.Writer, checker Poli commandPassID := "" commandPassNotify := false - // Check each segment of the command against built-in and custom command rules. + // Check each segment of the command (and unwrapped wrapper segments) against + // built-in and custom command rules. for i, seg := range analysis.Commands { - // Built-in rules are always checked (no DB needed). - if matched := CheckBuiltinRules(seg); matched != nil { - reason := commandRuleDenyReason(matched, agent) - event := &Event{ - ToolName: payload.ToolName, - ToolInput: payload.ToolInput, - CommandRaw: analysis.CommandRaw, - CommandParsed: analysis.ParsedOK, - CommandParseError: analysis.ParseError, - CommandParser: analysis.Parser, - CommandParserVersion: analysis.ParserVersion, - CommandOpsJSON: analysis.opsJSON(), - DeniedOpIndex: i, - DeniedOpReason: "prevent-command rule violation", - MatchedRulePattern: matched.Pattern, - MatchedRuleType: matched.RuleType, - Ambiguity: analysis.ambiguityText(), - Decision: DecisionDeny, - Cwd: payload.Cwd, - } - if err := encodeClaudeDeny(w, reason); err != nil { - return nil, err - } - fmt.Fprintf(errW, "%s\n", reason) - return event, ErrDenied - } - - // Custom rules from the policy database. - if cmdChecker != nil { - if allowed, passID, matched, cmdNotify := cmdChecker(seg, payload.Cwd); !allowed && matched != nil { + for _, candidate := range expandCommandRuleSegments(seg) { + // Built-in rules are always checked (no DB needed). + if matched := CheckBuiltinRules(candidate); matched != nil { reason := commandRuleDenyReason(matched, agent) event := &Event{ ToolName: payload.ToolName, @@ -421,17 +395,46 @@ func evaluateBash(payload hookPayload, w io.Writer, errW io.Writer, checker Poli Ambiguity: analysis.ambiguityText(), Decision: DecisionDeny, Cwd: payload.Cwd, - Notify: cmdNotify, } if err := encodeClaudeDeny(w, reason); err != nil { return nil, err } fmt.Fprintf(errW, "%s\n", reason) return event, ErrDenied - } else if allowed && passID != "" { - commandPassID = passID - if cmdNotify { - commandPassNotify = true + } + + // Custom rules from the policy database. + if cmdChecker != nil { + if allowed, passID, matched, cmdNotify := cmdChecker(candidate, payload.Cwd); !allowed && matched != nil { + reason := commandRuleDenyReason(matched, agent) + event := &Event{ + ToolName: payload.ToolName, + ToolInput: payload.ToolInput, + CommandRaw: analysis.CommandRaw, + CommandParsed: analysis.ParsedOK, + CommandParseError: analysis.ParseError, + CommandParser: analysis.Parser, + CommandParserVersion: analysis.ParserVersion, + CommandOpsJSON: analysis.opsJSON(), + DeniedOpIndex: i, + DeniedOpReason: "prevent-command rule violation", + MatchedRulePattern: matched.Pattern, + MatchedRuleType: matched.RuleType, + Ambiguity: analysis.ambiguityText(), + Decision: DecisionDeny, + Cwd: payload.Cwd, + Notify: cmdNotify, + } + if err := encodeClaudeDeny(w, reason); err != nil { + return nil, err + } + fmt.Fprintf(errW, "%s\n", reason) + return event, ErrDenied + } else if allowed && passID != "" { + commandPassID = passID + if cmdNotify { + commandPassNotify = true + } } } } diff --git a/cli/internal/hook/hook_shell_test.go b/cli/internal/hook/hook_shell_test.go index 36092b1..9c94c78 100644 --- a/cli/internal/hook/hook_shell_test.go +++ b/cli/internal/hook/hook_shell_test.go @@ -178,3 +178,124 @@ func TestEvaluate_OpenCodeCommandToolNameStillChecksCommandRules(t *testing.T) { t.Fatalf("event.DeniedOpReason = %q, want prevent-command rule violation", event.DeniedOpReason) } } + +func TestEvaluate_RunInTerminalUnwrapsShellWrapperForCommandRules(t *testing.T) { + payload := `{ + "tool_name": "run_in_terminal", + "tool_input": {"command":"sh -c 'git commit -m \"test\"'"}, + "cwd": "/repo" +}` + + cmdChecker := func(command, cwd string) (bool, string, *MatchedRule, bool) { + if strings.HasPrefix(strings.TrimSpace(command), "git commit") { + return false, "", &MatchedRule{Pattern: "git commit", RuleType: "deny", RuleAuthority: "standard"}, false + } + return true, "", nil, false + } + + var out bytes.Buffer + var errOut bytes.Buffer + event, err := Evaluate(strings.NewReader(payload), &out, &errOut, nil, nil, cmdChecker, "") + if err != ErrDenied { + t.Fatalf("Evaluate error = %v, want ErrDenied", err) + } + if event == nil { + t.Fatal("event = nil, want non-nil deny event") + } + if event.DeniedOpReason != "prevent-command rule violation" { + t.Fatalf("event.DeniedOpReason = %q, want prevent-command rule violation", event.DeniedOpReason) + } +} + +func TestEvaluate_RunInTerminalUnwrapsShellWrapperForAllowPass(t *testing.T) { + payload := `{ + "tool_name": "run_in_terminal", + "tool_input": {"command":"bash -lc 'git commit -m \"test\"'"}, + "cwd": "/repo" +}` + + cmdChecker := func(command, cwd string) (bool, string, *MatchedRule, bool) { + if strings.HasPrefix(strings.TrimSpace(command), "git commit") { + return true, "pass-cmd-wrapper", nil, true + } + return true, "", nil, false + } + + var out bytes.Buffer + var errOut bytes.Buffer + event, err := Evaluate(strings.NewReader(payload), &out, &errOut, nil, nil, cmdChecker, "") + if err != nil { + t.Fatalf("Evaluate error = %v, want nil", err) + } + if event == nil { + t.Fatal("event = nil, want non-nil allow event") + } + if event.PassID != "pass-cmd-wrapper" { + t.Fatalf("event.PassID = %q, want pass-cmd-wrapper", event.PassID) + } + if !event.Notify { + t.Fatal("event.Notify = false, want true") + } +} + +func TestEvaluate_RunInTerminalUnwrapsShellWrapperForReadPolicy(t *testing.T) { + payload := `{ + "tool_name": "run_in_terminal", + "tool_input": {"command":"sh -c 'cat .env'"}, + "cwd": "/repo" +}` + + rdChecker := func(filePath, cwd string) (bool, string, bool) { + if filePath == "/repo/.env" { + return false, "", false + } + return true, "", false + } + + var out bytes.Buffer + var errOut bytes.Buffer + event, err := Evaluate(strings.NewReader(payload), &out, &errOut, nil, rdChecker, nil, "") + if err != ErrDenied { + t.Fatalf("Evaluate error = %v, want ErrDenied", err) + } + if event == nil { + t.Fatal("event = nil, want non-nil deny event") + } + if event.DeniedOpReason != "prevent-read rule violation" { + t.Fatalf("event.DeniedOpReason = %q, want prevent-read rule violation", event.DeniedOpReason) + } + if event.FilePath != "/repo/.env" { + t.Fatalf("event.FilePath = %q, want /repo/.env", event.FilePath) + } +} + +func TestEvaluate_RunInTerminalUnwrapsShellWrapperForWritePolicy(t *testing.T) { + payload := `{ + "tool_name": "run_in_terminal", + "tool_input": {"command":"bash -lc 'echo hello > out.txt'"}, + "cwd": "/repo" +}` + + checker := func(filePath, cwd string) (bool, string, bool) { + if filePath == "/repo/out.txt" { + return false, "", false + } + return true, "", false + } + + var out bytes.Buffer + var errOut bytes.Buffer + event, err := Evaluate(strings.NewReader(payload), &out, &errOut, checker, nil, nil, "") + if err != ErrDenied { + t.Fatalf("Evaluate error = %v, want ErrDenied", err) + } + if event == nil { + t.Fatal("event = nil, want non-nil deny event") + } + if event.DeniedOpReason != "prevent-write rule violation" { + t.Fatalf("event.DeniedOpReason = %q, want prevent-write rule violation", event.DeniedOpReason) + } + if event.FilePath != "/repo/out.txt" { + t.Fatalf("event.FilePath = %q, want /repo/out.txt", event.FilePath) + } +} diff --git a/cli/internal/hook/shellops.go b/cli/internal/hook/shellops.go index 835de49..471fa0f 100644 --- a/cli/internal/hook/shellops.go +++ b/cli/internal/hook/shellops.go @@ -77,69 +77,80 @@ func analyzeShellCommand(command, cwd string) shellAnalysis { if seg == "" { continue } + // Analyze the top-level segment; top-level cd affects subsequent segments. + effectiveCwd = analyzeSegmentOps(&a, seg, effectiveCwd) - a.Ops = append(a.Ops, shellOp{Type: shellOpExec, Command: seg, Cwd: effectiveCwd, Source: "parser"}) - - argv, ok := parseShellArgv(seg) - if !ok || len(argv) == 0 { - a.Ambiguity = append(a.Ambiguity, "argv_unresolved") - // Fall back to legacy heuristics for targets in this segment. - for _, p := range bashReadTargets(seg) { - a.Ops = append(a.Ops, shellOp{ - Type: shellOpRead, - Path: resolveShellPath(p, effectiveCwd), - Cwd: effectiveCwd, - Source: "legacy_read", - }) - } - for _, p := range bashWriteTargets(seg) { - a.Ops = append(a.Ops, shellOp{ - Type: shellOpMutation, - Path: resolveShellPath(p, effectiveCwd), - Cwd: effectiveCwd, - Source: "legacy_write", - }) - } - continue + // Also analyze wrapper inner segments (sh -c / bash -lc, etc.) so read/write + // policy enforcement applies to equivalent wrapped commands. + wrappedCwd := effectiveCwd + candidates := expandCommandRuleSegments(seg) + for _, wrappedSeg := range candidates[1:] { + wrappedCwd = analyzeSegmentOps(&a, wrappedSeg, wrappedCwd) } + } - cmd := argv[0] - low := strings.ToLower(cmd) - - if low == "cd" { - if len(argv) > 1 { - effectiveCwd = resolveShellPath(argv[1], effectiveCwd) - } else { - a.Ambiguity = append(a.Ambiguity, "cd_no_target") - } - continue - } + a.EffectiveCwd = effectiveCwd + a.Ops = dedupeOps(a.Ops) + return a +} - a.Ops = append(a.Ops, extractOpsFromArgv(argv, effectiveCwd)...) +func analyzeSegmentOps(a *shellAnalysis, seg, cwd string) string { + a.Ops = append(a.Ops, shellOp{Type: shellOpExec, Command: seg, Cwd: cwd, Source: "parser"}) - // Preserve broad write/read detection coverage for shell syntax patterns - // not yet represented in command-specific extraction logic. + argv, ok := parseShellArgv(seg) + if !ok || len(argv) == 0 { + a.Ambiguity = append(a.Ambiguity, "argv_unresolved") + // Fall back to legacy heuristics for targets in this segment. for _, p := range bashReadTargets(seg) { a.Ops = append(a.Ops, shellOp{ Type: shellOpRead, - Path: resolveShellPath(p, effectiveCwd), - Cwd: effectiveCwd, + Path: resolveShellPath(p, cwd), + Cwd: cwd, Source: "legacy_read", }) } for _, p := range bashWriteTargets(seg) { a.Ops = append(a.Ops, shellOp{ Type: shellOpMutation, - Path: resolveShellPath(p, effectiveCwd), - Cwd: effectiveCwd, + Path: resolveShellPath(p, cwd), + Cwd: cwd, Source: "legacy_write", }) } + return cwd } - a.EffectiveCwd = effectiveCwd - a.Ops = dedupeOps(a.Ops) - return a + cmd := argv[0] + low := strings.ToLower(cmd) + if low == "cd" { + if len(argv) > 1 { + return resolveShellPath(argv[1], cwd) + } + a.Ambiguity = append(a.Ambiguity, "cd_no_target") + return cwd + } + + a.Ops = append(a.Ops, extractOpsFromArgv(argv, cwd)...) + + // Preserve broad write/read detection coverage for shell syntax patterns not + // yet represented in command-specific extraction logic. + for _, p := range bashReadTargets(seg) { + a.Ops = append(a.Ops, shellOp{ + Type: shellOpRead, + Path: resolveShellPath(p, cwd), + Cwd: cwd, + Source: "legacy_read", + }) + } + for _, p := range bashWriteTargets(seg) { + a.Ops = append(a.Ops, shellOp{ + Type: shellOpMutation, + Path: resolveShellPath(p, cwd), + Cwd: cwd, + Source: "legacy_write", + }) + } + return cwd } func parseShellArgv(seg string) ([]string, bool) { diff --git a/cli/internal/hook/shellops_test.go b/cli/internal/hook/shellops_test.go index 50c86a9..71ff54f 100644 --- a/cli/internal/hook/shellops_test.go +++ b/cli/internal/hook/shellops_test.go @@ -53,3 +53,65 @@ func TestAnalyzeShellCommand_AmbiguityOnExpansion(t *testing.T) { t.Fatalf("ambiguity = %q, want argv_unresolved", a.ambiguityText()) } } + +func TestAnalyzeShellCommand_WrappedCommandResolvesInnerCd(t *testing.T) { + a := analyzeShellCommand(`bash -lc 'cd scripts && cat README.md'`, "/repo") + found := false + for _, op := range a.Ops { + if op.Type == shellOpRead && op.Path == "/repo/scripts/README.md" { + found = true + break + } + } + if !found { + t.Fatalf("expected wrapped read op for /repo/scripts/README.md, ops=%+v", a.Ops) + } +} + +func TestExtractOpsFromArgv_SedInPlaceVariants(t *testing.T) { + tests := [][]string{ + {"sed", "-i", "s/a/b/", "file.txt"}, + {"sed", "-i.bak", "s/a/b/", "file.txt"}, + } + for _, argv := range tests { + ops := extractOpsFromArgv(argv, "/repo") + found := false + for _, op := range ops { + if op.Type == shellOpMutation && op.Path == "/repo/file.txt" { + found = true + break + } + } + if !found { + t.Fatalf("expected sed in-place mutation for argv=%v, ops=%+v", argv, ops) + } + } +} + +func TestExtractOpsFromArgv_CpWithFlagsAndDoubleDash(t *testing.T) { + ops := extractOpsFromArgv([]string{"cp", "-r", "--", "src", "dst"}, "/repo") + found := false + for _, op := range ops { + if op.Type == shellOpMutation && op.Path == "/repo/dst" { + found = true + break + } + } + if !found { + t.Fatalf("expected cp destination mutation /repo/dst, ops=%+v", ops) + } +} + +func TestExtractOpsFromArgv_TeeAppend(t *testing.T) { + ops := extractOpsFromArgv([]string{"tee", "-a", "out.txt"}, "/repo") + found := false + for _, op := range ops { + if op.Type == shellOpMutation && op.Path == "/repo/out.txt" { + found = true + break + } + } + if !found { + t.Fatalf("expected tee -a mutation /repo/out.txt, ops=%+v", ops) + } +} diff --git a/cli/internal/store/match.go b/cli/internal/store/match.go index ae9a998..2401ec7 100644 --- a/cli/internal/store/match.go +++ b/cli/internal/store/match.go @@ -1,7 +1,9 @@ package store import ( + "path" "path/filepath" + "regexp" "strings" ) @@ -15,10 +17,8 @@ import ( // // Three matching strategies are tried against each path form: // 1. Exact match after path cleaning. -// 2. Single-level glob via filepath.Match (e.g. "src/*.go", "*.gitignore"). +// 2. Glob match (supports *, ?, [], and ** for recursive path matching). // 3. Directory prefix match: filePath is somewhere inside the rule directory. -// -// Note: double-star (**) globs are not yet supported. func pathMatchesFileRule(pattern, filePath, repoRoot string) bool { if matchOnePath(pattern, filePath) { return true @@ -44,8 +44,8 @@ func matchOnePath(pattern, filePath string) bool { return true } - // 2. Single-level glob match. - if matched, err := filepath.Match(cleanPattern, cleanFile); err == nil && matched { + // 2. Glob match. + if matchPathPattern(cleanPattern, cleanFile) { return true } @@ -59,3 +59,64 @@ func matchOnePath(pattern, filePath string) bool { return false } + +func matchPathPattern(pattern, filePath string) bool { + p := filepath.ToSlash(pattern) + f := filepath.ToSlash(filePath) + + if strings.Contains(p, "**") { + return matchDoubleStarPattern(p, f) + } + matched, err := path.Match(p, f) + return err == nil && matched +} + +func matchDoubleStarPattern(pattern, filePath string) bool { + re, err := regexp.Compile(doubleStarToRegexp(pattern)) + if err != nil { + return false + } + return re.MatchString(filePath) +} + +func doubleStarToRegexp(pattern string) string { + var b strings.Builder + b.WriteString("^") + + for i := 0; i < len(pattern); i++ { + ch := pattern[i] + switch ch { + case '*': + if i+1 < len(pattern) && pattern[i+1] == '*' { + b.WriteString(".*") + i++ + } else { + b.WriteString(`[^/]*`) + } + case '?': + b.WriteString(`[^/]`) + case '[': + // Preserve simple character class semantics. + j := i + 1 + for j < len(pattern) && pattern[j] != ']' { + j++ + } + if j < len(pattern) { + b.WriteByte('[') + b.WriteString(pattern[i+1 : j]) + b.WriteByte(']') + i = j + } else { + b.WriteString(`\[`) + } + default: + if strings.ContainsRune(`.+()|{}^$\\`, rune(ch)) { + b.WriteByte('\\') + } + b.WriteByte(ch) + } + } + + b.WriteString("$") + return b.String() +} diff --git a/cli/internal/store/match_test.go b/cli/internal/store/match_test.go new file mode 100644 index 0000000..97902bb --- /dev/null +++ b/cli/internal/store/match_test.go @@ -0,0 +1,15 @@ +package store + +import "testing" + +func TestMatchPathPattern_DoubleStarCharacterClass(t *testing.T) { + if !matchPathPattern("**/file[0-9].env", "a/b/file7.env") { + t.Fatal("expected **/file[0-9].env to match a/b/file7.env") + } +} + +func TestMatchPathPattern_DoubleStarUnterminatedClassLiteral(t *testing.T) { + if !matchPathPattern("**/file[.env", "a/b/file[.env") { + t.Fatal("expected unterminated class pattern to match literal '[' path") + } +} diff --git a/cli/internal/store/passes_test.go b/cli/internal/store/passes_test.go index 0da11fe..70df405 100644 --- a/cli/internal/store/passes_test.go +++ b/cli/internal/store/passes_test.go @@ -131,6 +131,32 @@ func TestActivePassForCommand_Match(t *testing.T) { } } +func TestActivePassForCommand_ArgvFlagMatchOutOfOrder(t *testing.T) { + dataDB := newTestDataDB(t) + now := time.Now().UTC() + + p := Pass{ + FileRuleID: "rule-cmd-argv-1", + Pattern: "git push --force*", + IssuedTo: "test", + IssuedBy: "test", + Status: "active", + IssuedAt: now.Format(time.RFC3339), + ExpiresAt: now.Add(time.Hour).Format(time.RFC3339), + } + if err := IssuePass(dataDB, p); err != nil { + t.Fatal(err) + } + + active, err := ActivePassForCommand(dataDB, "git push -u origin main --force") + if err != nil { + t.Fatal(err) + } + if active == nil { + t.Fatal("expected active command pass via argv flag match, got nil") + } +} + func TestActivePassForCommand_ExpiredNoMatch(t *testing.T) { dataDB := newTestDataDB(t) now := time.Now().UTC() diff --git a/cli/internal/store/policy_test.go b/cli/internal/store/policy_test.go index cb91a4c..9814f9b 100644 --- a/cli/internal/store/policy_test.go +++ b/cli/internal/store/policy_test.go @@ -54,6 +54,66 @@ func TestFileRuleForPath_DirectoryPrefix(t *testing.T) { } } +func TestFileRuleForPath_DoubleStarRecursiveGlob(t *testing.T) { + db := newTestPolicyDB(t) + if _, err := AddFileRule(db, "**/*.env", "deny", "standard", "test", false); err != nil { + t.Fatal(err) + } + + rule, err := FileRuleForPath(db, "apps/web/config/.env", "") + if err != nil { + t.Fatal(err) + } + if rule == nil { + t.Fatal("expected **/*.env to match nested .env path, got nil") + } +} + +func TestFileRuleForPath_DoubleStarRecursiveGlobNoMatch(t *testing.T) { + db := newTestPolicyDB(t) + if _, err := AddFileRule(db, "**/*.env", "deny", "standard", "test", false); err != nil { + t.Fatal(err) + } + + rule, err := FileRuleForPath(db, "apps/web/config/env.txt", "") + if err != nil { + t.Fatal(err) + } + if rule != nil { + t.Fatalf("expected no match for non-.env file, got %q", rule.Pattern) + } +} + +func TestFileRuleForPath_DoubleStarMidPattern(t *testing.T) { + db := newTestPolicyDB(t) + if _, err := AddFileRule(db, "src/**/secrets/*.pem", "deny", "standard", "test", false); err != nil { + t.Fatal(err) + } + + rule, err := FileRuleForPath(db, "src/app/v1/secrets/key.pem", "") + if err != nil { + t.Fatal(err) + } + if rule == nil { + t.Fatal("expected src/**/secrets/*.pem to match nested path, got nil") + } +} + +func TestFileRuleForPath_DoubleStarSuffixPattern(t *testing.T) { + db := newTestPolicyDB(t) + if _, err := AddFileRule(db, "config/**", "deny", "standard", "test", false); err != nil { + t.Fatal(err) + } + + rule, err := FileRuleForPath(db, "config/prod/app.yml", "") + if err != nil { + t.Fatal(err) + } + if rule == nil { + t.Fatal("expected config/** to match nested config path, got nil") + } +} + func TestFileRuleForPath_AllowOverridesDeny(t *testing.T) { db := newTestPolicyDB(t) if _, err := AddFileRule(db, "src", "deny", "standard", "test", false); err != nil { @@ -87,6 +147,38 @@ func TestFileRuleForPath_NoMatch(t *testing.T) { } } +func TestFileRuleForPath_AbsolutePathMatchesRelativeRule(t *testing.T) { + db := newTestPolicyDB(t) + repoRoot := filepath.Join(string(filepath.Separator), "tmp", "repo") + absPath := filepath.Join(repoRoot, ".env") + if _, err := AddFileRule(db, ".env", "deny", "standard", "test", false); err != nil { + t.Fatal(err) + } + + rule, err := FileRuleForPath(db, absPath, repoRoot) + if err != nil { + t.Fatal(err) + } + if rule == nil { + t.Fatal("expected relative rule to match absolute path inside repo, got nil") + } +} + +func TestFileRuleForPath_DirectoryPrefixNoFalsePositive(t *testing.T) { + db := newTestPolicyDB(t) + if _, err := AddFileRule(db, "src", "deny", "standard", "test", false); err != nil { + t.Fatal(err) + } + + rule, err := FileRuleForPath(db, "src2/main.go", "") + if err != nil { + t.Fatal(err) + } + if rule != nil { + t.Fatalf("expected no match for sibling prefix path, got %q", rule.Pattern) + } +} + func TestAddFileRule_DuplicatePattern(t *testing.T) { db := newTestPolicyDB(t) if _, err := AddFileRule(db, ".env", "deny", "standard", "test", false); err != nil { @@ -123,3 +215,42 @@ func TestNormalizeFilePath_RelativeCleaned(t *testing.T) { t.Fatalf("NormalizeFilePath(%q, repo) = %q, want %q", in, got, filepath.Join("src", "main.go")) } } + +func TestNormalizePattern_Table(t *testing.T) { + repo := filepath.Join(string(filepath.Separator), "tmp", "repo") + tests := []struct { + name string + pattern string + want string + }{ + { + name: "relative unchanged", + pattern: filepath.Join("src", "main.go"), + want: filepath.Join("src", "main.go"), + }, + { + name: "absolute inside repo relativized", + pattern: filepath.Join(repo, "src", "main.go"), + want: filepath.Join("src", "main.go"), + }, + { + name: "absolute outside repo unchanged", + pattern: filepath.Join(string(filepath.Separator), "tmp", "other", "main.go"), + want: filepath.Join(string(filepath.Separator), "tmp", "other", "main.go"), + }, + { + name: "glob unchanged", + pattern: "*.env", + want: "*.env", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := NormalizePattern(tt.pattern, repo) + if got != tt.want { + t.Fatalf("NormalizePattern(%q, %q) = %q, want %q", tt.pattern, repo, got, tt.want) + } + }) + } +} diff --git a/cli/internal/store/rules.go b/cli/internal/store/rules.go index 90056fb..72f925a 100644 --- a/cli/internal/store/rules.go +++ b/cli/internal/store/rules.go @@ -1,13 +1,16 @@ package store import ( + "bytes" "database/sql" "encoding/json" "fmt" - "path/filepath" + "path" "regexp" "strings" "time" + + "mvdan.cc/sh/v3/syntax" ) // StandardGuardrails is the default set of guardrail command rules offered during @@ -17,8 +20,8 @@ import ( var StandardGuardrails = []string{ // Destructive git operations "git reset --hard*", - "git push --force*", - "git push -f*", + "git push *--force*", + "git push *-f*", "git clean -f*", // Destructive filesystem operations "rm -rf /*", @@ -162,20 +165,32 @@ func MatchCommandRule(db *sql.DB, command string) (*CommandRule, error) { return firstDeny, nil } -// commandMatchesPattern reports whether command matches the glob-style pattern. -// The pattern is matched against the full command string. The special token * -// matches any sequence of characters. +// commandMatchesPattern reports whether command matches a rule pattern. +// Matching is additive: +// - string matcher (exact/glob/prefix), +// - argv matcher for option-sensitive patterns. func commandMatchesPattern(command, pattern string) bool { + if commandMatchesString(command, pattern) { + return true + } + return commandMatchesArgv(command, pattern) +} + +// commandMatchesString applies legacy string semantics: +// exact match, full-string glob match, and plain-prefix match. +// This is command-pattern matching logic (not file-rule path matching). +func commandMatchesString(command, pattern string) bool { command = strings.TrimSpace(command) - pattern = strings.TrimSpace(pattern) + pattern = canonicalCommandPattern(strings.TrimSpace(pattern)) // Exact match. if command == pattern { return true } - // filepath.Match uses the same glob syntax we want. - matched, err := filepath.Match(pattern, command) + // path.Match provides shell-style glob matching on plain strings and avoids + // OS-specific filepath separator behavior from path/filepath. + matched, err := path.Match(pattern, command) if err != nil { // Invalid pattern — treat as no match. matched = false @@ -188,14 +203,147 @@ func commandMatchesPattern(command, pattern string) bool { // command prefix so "echo" matches "echo hello" and "git push" matches // "git push origin main". if !hasGlobMeta(pattern) { - return strings.HasPrefix(command, pattern+" ") + if strings.HasPrefix(command, pattern+" ") { + return true + } } return false } +// canonicalCommandPattern normalizes simple command-prefix patterns so +// "" and " *" are treated equivalently for matching. +// This only applies when the trailing "*" is the sole glob metacharacter. +func canonicalCommandPattern(pattern string) string { + if !strings.HasSuffix(pattern, " *") { + return pattern + } + base := strings.TrimSpace(strings.TrimSuffix(pattern, " *")) + if base == "" { + return pattern + } + if hasGlobMeta(base) { + return pattern + } + return base +} + var globMetaRegex = regexp.MustCompile(`[*?\[]`) func hasGlobMeta(pattern string) bool { return globMetaRegex.MatchString(pattern) } + +// commandMatchesArgv matches a command by split shell tokens with +// order-insensitive option matching. It is intentionally conservative and only +// engages when pattern contains at least one option token and no positional +// tokens after the first option token. +func commandMatchesArgv(command, pattern string) bool { + cmdTokens, ok := shellCommandTokens(command) + if !ok || len(cmdTokens) == 0 { + return false + } + patTokens, ok := shellCommandTokens(pattern) + if !ok || len(patTokens) == 0 { + return false + } + + firstOpt := -1 + for i, tok := range patTokens { + if isOptionToken(tok) { + firstOpt = i + break + } + } + if firstOpt < 0 { + return false + } + // To avoid surprising behavior drift, only use argv option matching for + // patterns whose suffix is option-only (e.g. "git push --force*"). + for _, tok := range patTokens[firstOpt:] { + if !isOptionToken(tok) { + return false + } + } + + // Prefix argv tokens (e.g. "git push") must match in order. + if len(cmdTokens) < firstOpt { + return false + } + for i := 0; i < firstOpt; i++ { + if !tokenGlobMatch(patTokens[i], cmdTokens[i]) { + return false + } + } + + // Option argv tokens match anywhere in the command tail. + tail := cmdTokens[firstOpt:] + for _, optPat := range patTokens[firstOpt:] { + found := false + for _, tok := range tail { + if tokenGlobMatch(optPat, tok) { + found = true + break + } + } + if !found { + return false + } + } + return true +} + +func tokenGlobMatch(patternTok, token string) bool { + if patternTok == token { + return true + } + matched, err := path.Match(patternTok, token) + return err == nil && matched +} + +func isOptionToken(tok string) bool { + return strings.HasPrefix(tok, "-") +} + +// shellCommandTokens parses a shell command segment and returns its call tokens +// as printed shell words. Returns false if parsing fails or no call expression +// can be found. +func shellCommandTokens(command string) ([]string, bool) { + parser := syntax.NewParser(syntax.Variant(syntax.LangBash)) + file, err := parser.Parse(strings.NewReader(command), "") + if err != nil { + return nil, false + } + + var call *syntax.CallExpr + syntax.Walk(file, func(node syntax.Node) bool { + if node == nil || call != nil { + return true + } + if c, ok := node.(*syntax.CallExpr); ok { + call = c + return false + } + return true + }) + if call == nil { + return nil, false + } + + printer := syntax.NewPrinter() + tokens := make([]string, 0, len(call.Args)) + for _, w := range call.Args { + var buf bytes.Buffer + if err := printer.Print(&buf, w); err != nil { + return nil, false + } + tok := strings.TrimSpace(buf.String()) + if tok != "" { + tokens = append(tokens, tok) + } + } + if len(tokens) == 0 { + return nil, false + } + return tokens, true +} diff --git a/cli/internal/store/rules_test.go b/cli/internal/store/rules_test.go index ed61b55..a89b28d 100644 --- a/cli/internal/store/rules_test.go +++ b/cli/internal/store/rules_test.go @@ -34,6 +34,66 @@ func TestMatchCommandRule_GlobMatch(t *testing.T) { } } +func TestMatchCommandRule_ArgvFlagMatchOutOfOrder(t *testing.T) { + db := newTestPolicyDB(t) + if _, err := AddRule(db, "git push --force*", "deny", "standard", "test"); err != nil { + t.Fatal(err) + } + + rule, err := MatchCommandRule(db, "git push -u origin main --force") + if err != nil { + t.Fatal(err) + } + if rule == nil { + t.Fatal("expected argv option match for reordered --force flag, got nil") + } +} + +func TestMatchCommandRule_ArgvFlagRequiresCommandPrefix(t *testing.T) { + db := newTestPolicyDB(t) + if _, err := AddRule(db, "git push --force*", "deny", "standard", "test"); err != nil { + t.Fatal(err) + } + + rule, err := MatchCommandRule(db, "git pull --force") + if err != nil { + t.Fatal(err) + } + if rule != nil { + t.Fatalf("expected no match for non-push command, got %q", rule.Pattern) + } +} + +func TestMatchCommandRule_ArgvShortFlagMatchOutOfOrder(t *testing.T) { + db := newTestPolicyDB(t) + if _, err := AddRule(db, "git push -f*", "deny", "standard", "test"); err != nil { + t.Fatal(err) + } + + rule, err := MatchCommandRule(db, "git push origin main -f") + if err != nil { + t.Fatal(err) + } + if rule == nil { + t.Fatal("expected argv option match for reordered -f flag, got nil") + } +} + +func TestMatchCommandRule_ArgvShortFlagRequiresCommandPrefix(t *testing.T) { + db := newTestPolicyDB(t) + if _, err := AddRule(db, "git push -f*", "deny", "standard", "test"); err != nil { + t.Fatal(err) + } + + rule, err := MatchCommandRule(db, "git pull -f") + if err != nil { + t.Fatal(err) + } + if rule != nil { + t.Fatalf("expected no match for non-push command, got %q", rule.Pattern) + } +} + func TestMatchCommandRule_PrefixMatchForPlainPattern(t *testing.T) { db := newTestPolicyDB(t) if _, err := AddRule(db, "echo", "deny", "standard", "test"); err != nil { @@ -96,3 +156,76 @@ func TestMatchCommandRule_NoMatch(t *testing.T) { t.Errorf("expected no match, got rule %q", rule.Pattern) } } + +func TestMatchCommandRule_ArgvDoesNotReorderWhenPositionalSuffixPresent(t *testing.T) { + db := newTestPolicyDB(t) + if _, err := AddRule(db, "git push --force origin", "deny", "standard", "test"); err != nil { + t.Fatal(err) + } + + rule, err := MatchCommandRule(db, "git push origin --force") + if err != nil { + t.Fatal(err) + } + if rule != nil { + t.Fatalf("expected no match when positional suffix exists after option in pattern, got %q", rule.Pattern) + } +} + +func TestMatchCommandRule_ParseFailureFallsBackToStringMatch(t *testing.T) { + db := newTestPolicyDB(t) + if _, err := AddRule(db, "echo *", "deny", "standard", "test"); err != nil { + t.Fatal(err) + } + + // Unterminated quote is invalid shell; argv matcher should fail closed, while + // string matcher still covers this pattern. + rule, err := MatchCommandRule(db, `echo "unterminated`) + if err != nil { + t.Fatal(err) + } + if rule == nil { + t.Fatal("expected string matcher fallback to match parse-failed command") + } +} + +func TestMatchCommandRule_CommandAndCommandStarEquivalent(t *testing.T) { + db := newTestPolicyDB(t) + if _, err := AddRule(db, "git commit *", "deny", "standard", "test"); err != nil { + t.Fatal(err) + } + + tests := []string{ + "git commit", + "git commit -m test", + } + for _, cmd := range tests { + t.Run(cmd, func(t *testing.T) { + rule, err := MatchCommandRule(db, cmd) + if err != nil { + t.Fatal(err) + } + if rule == nil { + t.Fatalf("expected %q to match git commit * rule", cmd) + } + }) + } +} + +func TestMatchCommandRule_AllowOverrideForCommandStar(t *testing.T) { + db := newTestPolicyDB(t) + if _, err := AddRule(db, "git commit", "deny", "standard", "test"); err != nil { + t.Fatal(err) + } + if _, err := AddRule(db, "git commit *", "allow", "standard", "test"); err != nil { + t.Fatal(err) + } + + rule, err := MatchCommandRule(db, "git commit -m test") + if err != nil { + t.Fatal(err) + } + if rule != nil { + t.Fatalf("expected allow rule git commit * to override deny, got deny %q", rule.Pattern) + } +} diff --git a/cli/tests/hook_command_allow_test.go b/cli/tests/hook_command_allow_test.go new file mode 100644 index 0000000..4cf59bc --- /dev/null +++ b/cli/tests/hook_command_allow_test.go @@ -0,0 +1,56 @@ +package tests + +import ( + "bytes" + "os/exec" + "testing" +) + +func runHookWithPayload(t *testing.T, repo testRepo, payload string) runResult { + t.Helper() + cmd := exec.Command(binaryPath, "hook") + cmd.Dir = repo.Dir + cmd.Env = repo.env() + cmd.Stdin = bytes.NewBufferString(payload) + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + err := cmd.Run() + r := runResult{ + Stdout: stdout.Bytes(), + Stderr: stderr.Bytes(), + } + if err != nil { + if ee, ok := err.(*exec.ExitError); ok { + r.ExitCode = ee.ExitCode() + } else { + t.Fatalf("exec cordon hook: %v", err) + } + } + return r +} + +func TestHook_CommandAllowOverridesDenyForWrappedCommand(t *testing.T) { + repo := initRepo(t) + + runCordon(t, repo, "command", "add", "git commit") + runCordon(t, repo, "command", "add", "git commit *", "--allow") + + payload := `{"tool_name":"bash","tool_input":{"command":"sh -c 'git commit -m \"test\"'"},"cwd":"` + repo.Dir + `"}` + r := runHookWithPayload(t, repo, payload) + if r.ExitCode != 0 { + t.Fatalf("hook exit = %d, want 0\nstdout:%s\nstderr:%s", r.ExitCode, r.Stdout, r.Stderr) + } +} + +func TestHook_CommandDenyForWrappedCommandWithoutAllow(t *testing.T) { + repo := initRepo(t) + + runCordon(t, repo, "command", "add", "git commit") + + payload := `{"tool_name":"bash","tool_input":{"command":"sh -c 'git commit -m \"test\"'"},"cwd":"` + repo.Dir + `"}` + r := runHookWithPayload(t, repo, payload) + if r.ExitCode != 2 { + t.Fatalf("hook exit = %d, want 2\nstdout:%s\nstderr:%s", r.ExitCode, r.Stdout, r.Stderr) + } +} diff --git a/docs/project-overview-and-concepts.md b/docs/project-overview-and-concepts.md index 3bedb38..8e905a9 100644 --- a/docs/project-overview-and-concepts.md +++ b/docs/project-overview-and-concepts.md @@ -20,6 +20,33 @@ In this repository: See the project README for canonical concept definitions and terminology. +## Policy enforcement model (high level) + +Cordon enforces two related but different policy types: + +- **File policy enforcement** + - Protects files, folders, and globs in the repository. + - Evaluates whether a tool operation would read from or write to protected paths. + +- **Command policy enforcement** + - Protects shell command intent (for example destructive operations or sensitive commands). + - Evaluates each shell command segment against command rules before execution. + +For command rules, matching is intentionally layered: + +- **`string` matching** + - Backward-compatible command text matching (exact/glob/prefix style). + - Useful for straightforward command patterns. + +- **`argv` matching** + - Token-aware command matching that looks at command words/flags. + - Helps catch equivalent commands where option order differs (for example force flags appearing later in the command). + - Common shell wrappers (`sh -c`, `bash -lc`, etc.) are unwrapped so equivalent inner commands are evaluated consistently. + +In practice, this means file and command enforcement share the same policy system but use different matching models suited to their domain. + +File pattern support includes recursive glob usage (`**`) for nested path matching. + ## High-level runtime architecture 1. User runs a command (`init`, `file add`, `pass issue`, etc.)