fix(tools): add secret redaction layer to prevent credential leaks in tool output
Tool results (read_file, exec, subagent) flow to the LLM context, session history, channel output, and REST API without any sanitization. When a subagent reads config.json during autonomous error recovery, API keys are exposed verbatim to chat channels like Telegram. Add a RedactSecrets middleware that scrubs known credential patterns (OpenAI, Anthropic, Slack, AWS, GitHub, etc.) and JSON secret field values from all tool results. Wire it into ToolRegistry.ExecuteWithContext as a single chokepoint so every tool — including future ones and MCP tools — gets automatic redaction. Fixes #972
This commit is contained in:
parent
555af137b4
commit
0d207e1174
3 changed files with 332 additions and 0 deletions
129
pkg/tools/redact.go
Normal file
129
pkg/tools/redact.go
Normal file
|
|
@ -0,0 +1,129 @@
|
|||
package tools
|
||||
|
||||
import (
|
||||
"regexp"
|
||||
"strings"
|
||||
)
|
||||
|
||||
// secretPatterns matches common API key and credential formats.
|
||||
// Each pattern is compiled once at init and reused across calls.
|
||||
var secretPatterns = []*regexp.Regexp{
|
||||
// OpenAI / OpenAI-compatible (sk-...)
|
||||
regexp.MustCompile(`sk-[A-Za-z0-9_-]{20,}`),
|
||||
// OpenAI project keys (sk-proj-...)
|
||||
regexp.MustCompile(`sk-proj-[A-Za-z0-9_-]{20,}`),
|
||||
// Anthropic (sk-ant-...)
|
||||
regexp.MustCompile(`sk-ant-[A-Za-z0-9_-]{20,}`),
|
||||
// OpenRouter (sk-or-v1-...)
|
||||
regexp.MustCompile(`sk-or-v1-[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,}`),
|
||||
// Generic long bearer/api tokens (40+ hex or base64 chars)
|
||||
regexp.MustCompile(`\b[A-Fa-f0-9]{40,}\b`),
|
||||
// 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}`),
|
||||
}
|
||||
|
||||
// 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}
|
||||
}
|
||||
}
|
||||
return r
|
||||
}
|
||||
|
||||
// redactedError wraps a redacted error message.
|
||||
type redactedError struct {
|
||||
msg string
|
||||
}
|
||||
|
||||
func (e *redactedError) Error() string {
|
||||
return e.msg
|
||||
}
|
||||
|
||||
// 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 patterns whose values should
|
||||
// never appear in tool output. Used by the shell tool guard.
|
||||
var envSecretKeys = []string{
|
||||
"API_KEY", "SECRET", "TOKEN", "PASSWORD", "CREDENTIAL",
|
||||
"ACCESS_KEY", "PRIVATE_KEY", "AUTH",
|
||||
}
|
||||
|
||||
// IsSecretEnvVar returns true if the environment variable name looks like it
|
||||
// holds a secret value (case-insensitive substring match).
|
||||
func IsSecretEnvVar(name string) bool {
|
||||
upper := strings.ToUpper(name)
|
||||
for _, key := range envSecretKeys {
|
||||
if strings.Contains(upper, key) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
199
pkg/tools/redact_test.go
Normal file
199
pkg/tools/redact_test.go
Normal file
|
|
@ -0,0 +1,199 @@
|
|||
package tools
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"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 !containsRedacted(result) {
|
||||
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 !containsRedacted(result) {
|
||||
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 !contains(result, `"api_key"`) {
|
||||
t.Error("expected key name to be preserved")
|
||||
}
|
||||
if !containsRedacted(result) {
|
||||
t.Errorf("expected [REDACTED] in output, got: %s", result)
|
||||
}
|
||||
// Model value should NOT be redacted
|
||||
if !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 contains(result, "sk-abcdefghijklmnopqrstuvwxyz") {
|
||||
t.Error("api_key value should be redacted")
|
||||
}
|
||||
if 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 !containsRedacted(result) {
|
||||
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 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 contains(result, "ghp_ABCDEFGHIJKLMNOPQRSTUVWXYZ") {
|
||||
t.Errorf("expected GitHub token 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 !contains(result, "sk-p") {
|
||||
t.Errorf("expected prefix to be preserved for identification, got: %s", result)
|
||||
}
|
||||
}
|
||||
|
||||
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 contains(r.ForLLM, "sk-ant-secret") {
|
||||
t.Error("ForLLM should have secret redacted")
|
||||
}
|
||||
if 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_ErrorField(t *testing.T) {
|
||||
r := &ToolResult{
|
||||
ForLLM: "error occurred",
|
||||
Err: fmt.Errorf("auth failed with key sk-proj-abc123def456ghi789jkl012mno345pqr678"),
|
||||
}
|
||||
SanitizeResult(r)
|
||||
if contains(r.Err.Error(), "sk-proj-abc123") {
|
||||
t.Error("Err field should have secret redacted")
|
||||
}
|
||||
}
|
||||
|
||||
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},
|
||||
}
|
||||
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},
|
||||
{"HOME", false},
|
||||
{"PATH", false},
|
||||
{"GOPATH", false},
|
||||
{"MODEL_NAME", false},
|
||||
}
|
||||
for _, tc := range tests {
|
||||
got := IsSecretEnvVar(tc.name)
|
||||
if got != tc.expected {
|
||||
t.Errorf("IsSecretEnvVar(%q) = %v, want %v", tc.name, got, tc.expected)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func containsRedacted(s string) bool {
|
||||
return contains(s, "[REDACTED]")
|
||||
}
|
||||
|
||||
func contains(s, substr string) bool {
|
||||
return len(s) >= len(substr) && searchString(s, substr)
|
||||
}
|
||||
|
||||
func searchString(s, substr string) bool {
|
||||
for i := 0; i <= len(s)-len(substr); i++ {
|
||||
if s[i:i+len(substr)] == substr {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
|
@ -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
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue