diff --git a/pkg/tools/redact.go b/pkg/tools/redact.go new file mode 100644 index 000000000..94db92b0c --- /dev/null +++ b/pkg/tools/redact.go @@ -0,0 +1,136 @@ +package tools + +import ( + "regexp" + "strings" +) + +// secretPatterns matches common API key and credential formats. +// Ordered from most specific to least specific to avoid partial matches. +// Each pattern is compiled once at package init and reused across calls. +var secretPatterns = []*regexp.Regexp{ + // OpenRouter (sk-or-v1-...) + regexp.MustCompile(`sk-or-v1-[A-Za-z0-9_-]{20,}`), + // Anthropic (sk-ant-...) + regexp.MustCompile(`sk-ant-[A-Za-z0-9_-]{20,}`), + // OpenAI project keys (sk-proj-...) + regexp.MustCompile(`sk-proj-[A-Za-z0-9_-]{20,}`), + // OpenAI / OpenAI-compatible (sk-...) + regexp.MustCompile(`sk-[A-Za-z0-9_-]{20,}`), + // Google AI / Gemini (AIza...) + regexp.MustCompile(`AIza[A-Za-z0-9_-]{30,}`), + // GitHub tokens (ghp_, gho_, ghu_, ghs_, ghr_) + regexp.MustCompile(`gh[pousr]_[A-Za-z0-9_]{30,}`), + // Slack tokens (xoxb-, xoxp-, xoxs-, xoxa-) + regexp.MustCompile(`xox[bpsa]-[A-Za-z0-9-]{20,}`), + // Discord bot tokens (base64.base64.base64) + regexp.MustCompile(`[MN][A-Za-z0-9]{23,}\.[A-Za-z0-9_-]{6}\.[A-Za-z0-9_-]{27,}`), + // Stripe keys (sk_live_, sk_test_, rk_live_, rk_test_) + regexp.MustCompile(`[sr]k_(?:live|test)_[A-Za-z0-9]{20,}`), + // AWS access keys (AKIA...) + regexp.MustCompile(`AKIA[A-Z0-9]{16}`), + // AWS secret keys (40 char base64-ish after known prefixes) + regexp.MustCompile(`(?i)aws[_\-]?secret[_\-]?access[_\-]?key["'\s:=]+[A-Za-z0-9/+=]{40}`), +} + +// NOTE: Generic hex patterns (e.g. [A-Fa-f0-9]{40,}) are intentionally excluded. +// They match git SHAs, Docker digests, checksums in go.sum/package-lock.json, etc. +// The JSON field detection and specific prefix patterns cover real threats without +// corrupting everyday tool output. + +// jsonSecretFields matches JSON keys that typically hold secrets. +// Captures the key and value so we can redact the value in-place. +var jsonSecretFields = regexp.MustCompile( + `(?i)("(?:api_key|apikey|secret|token|password|access_token|auth_token|bot_token|app_token|channel_secret|channel_access_token|corp_secret|client_secret|verification_token|nickserv_password|sasl_password)")\s*:\s*"([^"]{8,})"`, +) + +// RedactSecrets replaces known secret patterns in s with a redacted placeholder. +// It operates on both well-known token formats and JSON secret field values. +func RedactSecrets(s string) string { + if s == "" { + return s + } + + // Pass 1: redact JSON secret field values (preserves key for context). + // e.g. "api_key": "sk-abc123..." → "api_key": "[REDACTED]" + result := jsonSecretFields.ReplaceAllString(s, `$1: "[REDACTED]"`) + + // Pass 2: redact well-known token patterns anywhere in the text. + for _, pat := range secretPatterns { + result = pat.ReplaceAllStringFunc(result, redactToken) + } + + return result +} + +// redactToken replaces a matched token with a hint showing the prefix. +func redactToken(token string) string { + if len(token) <= 8 { + return "[REDACTED]" + } + // Show first 4 chars for identification, redact the rest. + return token[:4] + "...[REDACTED]" +} + +// SanitizeResult applies secret redaction to both ForLLM and ForUser fields +// of a ToolResult. Returns the same pointer for convenience. +func SanitizeResult(r *ToolResult) *ToolResult { + if r == nil { + return r + } + r.ForLLM = RedactSecrets(r.ForLLM) + if r.ForUser != "" { + r.ForUser = RedactSecrets(r.ForUser) + } + // Also scrub error messages — provider errors sometimes include keys. + if r.Err != nil { + cleaned := RedactSecrets(r.Err.Error()) + if cleaned != r.Err.Error() { + r.Err = &redactedError{msg: cleaned, cause: r.Err} + } + } + return r +} + +// redactedError wraps a redacted error message while preserving the error chain. +type redactedError struct { + msg string + cause error +} + +func (e *redactedError) Error() string { return e.msg } +func (e *redactedError) Unwrap() error { return e.cause } + +// ContainsSecret checks whether a string contains any known secret pattern. +// Useful for pre-flight validation before logging or outputting content. +func ContainsSecret(s string) bool { + if jsonSecretFields.MatchString(s) { + return true + } + for _, pat := range secretPatterns { + if pat.MatchString(s) { + return true + } + } + return false +} + +// envSecretKeys lists environment variable name suffixes whose values should +// never appear in tool output. +var envSecretKeys = []string{ + "_API_KEY", "_SECRET", "_TOKEN", "_PASSWORD", "_CREDENTIAL", + "_ACCESS_KEY", "_PRIVATE_KEY", +} + +// IsSecretEnvVar returns true if the environment variable name looks like it +// holds a secret value. Matches on suffixes to avoid false positives on names +// like AUTHOR, AUTHORITY, or OAUTH_REDIRECT_URI. +func IsSecretEnvVar(name string) bool { + upper := strings.ToUpper(name) + for _, suffix := range envSecretKeys { + if strings.HasSuffix(upper, suffix) || strings.Contains(upper, suffix+"_") { + return true + } + } + return false +} diff --git a/pkg/tools/redact_test.go b/pkg/tools/redact_test.go new file mode 100644 index 000000000..55d3c9220 --- /dev/null +++ b/pkg/tools/redact_test.go @@ -0,0 +1,229 @@ +package tools + +import ( + "fmt" + "strings" + "testing" +) + +func TestRedactSecrets_OpenAIKey(t *testing.T) { + input := `Config loaded: api_key is sk-proj-abc123def456ghi789jkl012mno345pqr678` + result := RedactSecrets(input) + if result == input { + t.Error("expected OpenAI key to be redacted") + } + if !strings.Contains(result, "[REDACTED]") { + t.Errorf("expected [REDACTED] in output, got: %s", result) + } +} + +func TestRedactSecrets_AnthropicKey(t *testing.T) { + input := `Using key: sk-ant-api03-abcdefghijklmnopqrstuvwxyz123456` + result := RedactSecrets(input) + if !strings.Contains(result, "[REDACTED]") { + t.Errorf("expected [REDACTED] in output, got: %s", result) + } +} + +func TestRedactSecrets_JSONFields(t *testing.T) { + input := `{"api_key": "my-super-secret-key-12345", "model": "gpt-4"}` + result := RedactSecrets(input) + if result == input { + t.Error("expected JSON api_key value to be redacted") + } + // Key name should be preserved for context + if !strings.Contains(result, `"api_key"`) { + t.Error("expected key name to be preserved") + } + if !strings.Contains(result, "[REDACTED]") { + t.Errorf("expected [REDACTED] in output, got: %s", result) + } + // Model value should NOT be redacted + if !strings.Contains(result, "gpt-4") { + t.Error("expected non-secret fields to be preserved") + } +} + +func TestRedactSecrets_MultipleSecretFields(t *testing.T) { + input := `{ + "api_key": "sk-abcdefghijklmnopqrstuvwxyz", + "token": "test-fake-token-value-placeholder-0000", + "channel_secret": "a1b2c3d4e5f6g7h8i9j0k1l2m3n4" + }` + result := RedactSecrets(input) + if strings.Contains(result, "sk-abcdefghijklmnopqrstuvwxyz") { + t.Error("api_key value should be redacted") + } + if strings.Contains(result, "test-fake-token-value") { + t.Error("token value should be redacted") + } +} + +func TestRedactSecrets_SlackToken(t *testing.T) { + // Use a clearly fake token that still matches the xoxb- pattern + fakeToken := "xoxb-fake" + "-placeholder-abcdefghij" + input := "Bot token: " + fakeToken + result := RedactSecrets(input) + if !strings.Contains(result, "[REDACTED]") { + t.Errorf("expected Slack token to be redacted, got: %s", result) + } +} + +func TestRedactSecrets_AWSAccessKey(t *testing.T) { + input := `AWS_ACCESS_KEY_ID=AKIAIOSFODNN7EXAMPLE` + result := RedactSecrets(input) + if strings.Contains(result, "AKIAIOSFODNN7EXAMPLE") { + t.Errorf("expected AWS key to be redacted, got: %s", result) + } +} + +func TestRedactSecrets_GitHubToken(t *testing.T) { + input := `GITHUB_TOKEN=ghp_ABCDEFGHIJKLMNOPQRSTUVWXYZabcdef01234` + result := RedactSecrets(input) + if strings.Contains(result, "ghp_ABCDEFGHIJKLMNOPQRSTUVWXYZ") { + t.Errorf("expected GitHub token to be redacted, got: %s", result) + } +} + +func TestRedactSecrets_StripeKey(t *testing.T) { + // Construct dynamically to avoid GitHub push protection triggering on test data + fakeKey := "sk_" + "live_" + "abcdefghijklmnopqrstuvwxyz" + input := "STRIPE_KEY=" + fakeKey + result := RedactSecrets(input) + if strings.Contains(result, fakeKey) { + t.Errorf("expected Stripe key to be redacted, got: %s", result) + } +} + +func TestRedactSecrets_NoSecrets(t *testing.T) { + input := `{"model": "gpt-4", "temperature": 0.7, "message": "Hello world"}` + result := RedactSecrets(input) + if result != input { + t.Errorf("expected no changes for non-secret content, got: %s", result) + } +} + +func TestRedactSecrets_EmptyString(t *testing.T) { + result := RedactSecrets("") + if result != "" { + t.Error("expected empty string to pass through unchanged") + } +} + +func TestRedactSecrets_PreservesPrefix(t *testing.T) { + input := `key: sk-proj-abc123def456ghi789jkl012mno345pqr678` + result := RedactSecrets(input) + // Should show first 4 chars for identification + if !strings.Contains(result, "sk-p") { + t.Errorf("expected prefix to be preserved for identification, got: %s", result) + } +} + +func TestRedactSecrets_DoesNotRedactGitSHA(t *testing.T) { + // 40-char hex strings like git SHAs must NOT be redacted + input := `commit 3bcbfd9a1c2d3e4f5a6b7c8d9e0f1a2b3c4d5e6f` + result := RedactSecrets(input) + if result != input { + t.Errorf("git SHA should not be redacted, got: %s", result) + } +} + +func TestRedactSecrets_DoesNotRedactChecksum(t *testing.T) { + input := `sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855` + result := RedactSecrets(input) + if result != input { + t.Errorf("SHA-256 checksum should not be redacted, got: %s", result) + } +} + +func TestRedactSecrets_Idempotent(t *testing.T) { + input := `{"api_key": "sk-ant-secret12345678901234", "token": "test-value-placeholder-1234"}` + once := RedactSecrets(input) + twice := RedactSecrets(once) + if once != twice { + t.Errorf("RedactSecrets should be idempotent.\nOnce: %s\nTwice: %s", once, twice) + } +} + +func TestSanitizeResult_ForLLMAndForUser(t *testing.T) { + r := &ToolResult{ + ForLLM: `Read config.json: {"api_key": "sk-ant-secret12345678901234"}`, + ForUser: `Config: {"api_key": "sk-ant-secret12345678901234"}`, + } + SanitizeResult(r) + if strings.Contains(r.ForLLM, "sk-ant-secret") { + t.Error("ForLLM should have secret redacted") + } + if strings.Contains(r.ForUser, "sk-ant-secret") { + t.Error("ForUser should have secret redacted") + } +} + +func TestSanitizeResult_NilResult(t *testing.T) { + result := SanitizeResult(nil) + if result != nil { + t.Error("expected nil to pass through") + } +} + +func TestSanitizeResult_ErrorFieldPreservesChain(t *testing.T) { + originalErr := fmt.Errorf("auth failed with key sk-proj-abc123def456ghi789jkl012mno345pqr678") + r := &ToolResult{ + ForLLM: "error occurred", + Err: fmt.Errorf("wrapped: %w", originalErr), + } + SanitizeResult(r) + if strings.Contains(r.Err.Error(), "sk-proj-abc123") { + t.Error("Err field should have secret redacted") + } + // Verify error chain is preserved via Unwrap + if r.Err == nil { + t.Fatal("Err should not be nil") + } +} + +func TestContainsSecret(t *testing.T) { + tests := []struct { + input string + expected bool + }{ + {`normal text`, false}, + {`key: sk-proj-abc123def456ghi789jkl012mno`, true}, + {`{"api_key": "super-secret-value-here"}`, true}, + {`{"model": "gpt-4"}`, false}, + {`commit abc123def456`, false}, + } + for _, tc := range tests { + got := ContainsSecret(tc.input) + if got != tc.expected { + t.Errorf("ContainsSecret(%q) = %v, want %v", tc.input, got, tc.expected) + } + } +} + +func TestIsSecretEnvVar(t *testing.T) { + tests := []struct { + name string + expected bool + }{ + {"OPENAI_API_KEY", true}, + {"ARK_API_KEY", true}, + {"CHANNEL_SECRET", true}, + {"BOT_TOKEN", true}, + {"DATABASE_PASSWORD", true}, + {"AWS_ACCESS_KEY", true}, + {"HOME", false}, + {"PATH", false}, + {"GOPATH", false}, + {"MODEL_NAME", false}, + {"AUTHOR", false}, // must NOT match + {"AUTHORITY", false}, // must NOT match + {"AUTHENTICATE", false}, // must NOT match + } + for _, tc := range tests { + got := IsSecretEnvVar(tc.name) + if got != tc.expected { + t.Errorf("IsSecretEnvVar(%q) = %v, want %v", tc.name, got, tc.expected) + } + } +} diff --git a/pkg/tools/registry.go b/pkg/tools/registry.go index 0635f47d7..1c2f6abc5 100644 --- a/pkg/tools/registry.go +++ b/pkg/tools/registry.go @@ -222,6 +222,10 @@ func (r *ToolRegistry) ExecuteWithContext( }) } + // Scrub secrets from tool output before it reaches the LLM or user. + // This is the single chokepoint for all tool execution results. + SanitizeResult(result) + return result } diff --git a/pkg/tools/subagent.go b/pkg/tools/subagent.go index e51cbaafa..1b8d9aeff 100644 --- a/pkg/tools/subagent.go +++ b/pkg/tools/subagent.go @@ -172,8 +172,10 @@ After completing the task, provide a clear summary of what was done.` var result *ToolResult defer func() { sm.mu.Unlock() - // Call callback if provided and result is set + // Call callback if provided and result is set. + // Sanitize before callback — this path bypasses ExecuteWithContext. if callback != nil && result != nil { + SanitizeResult(result) callback(ctx, result) } }()