diff --git a/internal/tools/credentialed_exec.go b/internal/tools/credentialed_exec.go index aa5e47ab..74fd2fbf 100644 --- a/internal/tools/credentialed_exec.go +++ b/internal/tools/credentialed_exec.go @@ -140,6 +140,7 @@ func resolveAndMatchBinary(binaryName string, configPath *string) (string, error } // matchesBinaryDeny checks if the joined args string matches any per-binary deny pattern. +// Used for deny_args where patterns span multiple args (e.g. `auth\s+login`, `repo\s+delete`). // Returns the matched pattern string, or empty if allowed. func matchesBinaryDeny(args []string, denyPatternsJSON json.RawMessage) string { if len(denyPatternsJSON) == 0 { @@ -163,6 +164,40 @@ func matchesBinaryDeny(args []string, denyPatternsJSON json.RawMessage) string { return "" } +// matchesBinaryVerbose checks each arg token against verbose/debug flag patterns. +// Patterns are anchored at the START of each arg (but not the end), which allows: +// - `-v` to match `-v`, `-vv`, `-vvv` (verbosity escalation), `-v=1`, `-vq` (combined flags) +// - `--verbose` to match `--verbose`, `--verbose=true` +// - `-v` to NOT match `--version` (char 1 is `-`, not `v`) +// - `--verbose` to NOT match `--version` (diverges at char 5) +// +// This is intentional: verbose flags leak sensitive output (tokens in HTTP headers, +// API response bodies, OAuth flows). Start-anchored per-arg matching catches the +// real verbose family without false-positive on safe flags like `--version`. +// Returns the matched pattern string, or empty if allowed. +func matchesBinaryVerbose(args []string, denyPatternsJSON json.RawMessage) string { + if len(denyPatternsJSON) == 0 { + return "" + } + var patterns []string + if err := json.Unmarshal(denyPatternsJSON, &patterns); err != nil || len(patterns) == 0 { + return "" + } + for _, p := range patterns { + re, err := regexp.Compile("^(?:" + p + ")") + if err != nil { + slog.Warn("secure_cli.invalid_deny_pattern", "pattern", p, "error", err) + continue + } + for _, arg := range args { + if re.MatchString(arg) { + return p + } + } + } + return "" +} + // executeCredentialed runs a CLI command in Direct Exec Mode (no shell). // Credentials are injected as env vars into the child process only. // rawCommand is the original command string before shell-word parsing (preserves quoting). @@ -196,8 +231,9 @@ func (t *ExecTool) executeCredentialed(ctx context.Context, cred *store.SecureCL if p := matchesBinaryDeny(args, cred.DenyArgs); p != "" { return credentialedDenyError(binary, args, p) } - // Per-binary verbose deny check (deny_verbose) - if p := matchesBinaryDeny(args, cred.DenyVerbose); p != "" { + // Per-binary verbose deny check (deny_verbose) — per-arg start-anchored match + // so `-v` blocks `-v`/`-vv`/`-v=1` but not `--version`. + if p := matchesBinaryVerbose(args, cred.DenyVerbose); p != "" { return credentialedDenyError(binary, args, p) } diff --git a/internal/tools/credentialed_exec_test.go b/internal/tools/credentialed_exec_test.go index 6e950867..209f27af 100644 --- a/internal/tools/credentialed_exec_test.go +++ b/internal/tools/credentialed_exec_test.go @@ -1,6 +1,9 @@ package tools -import "testing" +import ( + "encoding/json" + "testing" +) func TestDetectShellOperators(t *testing.T) { tests := []struct { @@ -120,3 +123,85 @@ func TestParseCommandBinary(t *testing.T) { }) } } + +// TestMatchesBinaryVerbose verifies start-anchored per-arg matching for +// deny_verbose patterns. Regression guard: `-v` must NOT false-positive on +// `--version` (used by the system to probe CLI availability), but MUST still +// block real verbose flags (`-v`, `-vv`, `-v=1`, `--verbose=true`) to prevent +// leakage of tokens/request bodies via verbose output. +func TestMatchesBinaryVerbose(t *testing.T) { + ghPatterns, _ := json.Marshal([]string{"--verbose", "-v"}) + gcloudPatterns, _ := json.Marshal([]string{"--verbosity=debug", "--log-http"}) + awsPatterns, _ := json.Marshal([]string{"--debug"}) + + tests := []struct { + name string + patterns json.RawMessage + args []string + wantHit bool + }{ + // --- regression: safe flags must pass --- + {"gh --version not blocked", ghPatterns, []string{"--version"}, false}, + {"gh version subcmd not blocked", ghPatterns, []string{"version"}, false}, + {"gh --help not blocked", ghPatterns, []string{"--help"}, false}, + {"gh api repos/x not blocked", ghPatterns, []string{"api", "repos/x"}, false}, + + // --- real verbose flags still blocked --- + {"gh -v blocked", ghPatterns, []string{"-v"}, true}, + {"gh --verbose blocked", ghPatterns, []string{"--verbose"}, true}, + {"gh -vv blocked (escalation)", ghPatterns, []string{"-vv"}, true}, + {"gh -vvv blocked (escalation)", ghPatterns, []string{"-vvv"}, true}, + {"gh --verbose=true blocked (equals form)", ghPatterns, []string{"--verbose=true"}, true}, + {"gh -v in middle of args blocked", ghPatterns, []string{"api", "-v", "repos/x"}, true}, + + // --- gcloud patterns: exact flag=value --- + {"gcloud --verbosity=debug blocked", gcloudPatterns, []string{"--verbosity=debug"}, true}, + {"gcloud --verbosity=info not blocked", gcloudPatterns, []string{"--verbosity=info"}, false}, + {"gcloud --log-http blocked", gcloudPatterns, []string{"--log-http"}, true}, + {"gcloud version not blocked", gcloudPatterns, []string{"version"}, false}, + + // --- aws --- + {"aws --debug blocked", awsPatterns, []string{"--debug"}, true}, + {"aws --debugger not blocked-worthy (prefix match)", awsPatterns, []string{"--debugger"}, true}, // acceptable: still debug family + {"aws --version not blocked", awsPatterns, []string{"--version"}, false}, + + // --- empty / no patterns --- + {"empty patterns", json.RawMessage(nil), []string{"--verbose"}, false}, + {"empty args", ghPatterns, []string{}, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := matchesBinaryVerbose(tt.args, tt.patterns) + if (got != "") != tt.wantHit { + t.Errorf("matchesBinaryVerbose(%v) = %q, wantHit=%v", tt.args, got, tt.wantHit) + } + }) + } +} + +// TestMatchesBinaryDenyJoinedArgs verifies deny_args keeps joined-string +// matching so multi-token patterns like `auth\s+login` and `repo\s+delete` +// still work. +func TestMatchesBinaryDenyJoinedArgs(t *testing.T) { + ghPatterns, _ := json.Marshal([]string{`auth\s+`, `repo\s+delete`, `secret\s+`}) + + tests := []struct { + name string + args []string + wantHit bool + }{ + {"gh auth login blocked", []string{"auth", "login"}, true}, + {"gh repo delete blocked", []string{"repo", "delete", "foo/bar"}, true}, + {"gh secret set blocked", []string{"secret", "set", "TOKEN"}, true}, + {"gh api repos allowed", []string{"api", "repos/x"}, false}, + {"gh repo view allowed", []string{"repo", "view", "foo/bar"}, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := matchesBinaryDeny(tt.args, ghPatterns) + if (got != "") != tt.wantHit { + t.Errorf("matchesBinaryDeny(%v) = %q, wantHit=%v", tt.args, got, tt.wantHit) + } + }) + } +}