-
Notifications
You must be signed in to change notification settings - Fork 1
security(tracker): sanitize provider probe error bodies before surfacing in DoctorReport #122
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -320,7 +320,7 @@ func probeProvider(p providerDef, key string) (bool, string) { | |
| if isAuthError(msg) { | ||
| return false, "invalid or expired API key" | ||
| } | ||
| return false, trimErrMsg(msg, 80) | ||
| return false, trimErrMsg(sanitizeProviderError(msg), 80) | ||
| } | ||
| return true, "" | ||
| } | ||
|
|
@@ -342,6 +342,26 @@ func trimErrMsg(msg string, maxLen int) string { | |
| return msg[:maxLen] + "..." | ||
| } | ||
|
|
||
| var providerErrorSanitizers = []struct { | ||
| re *regexp.Regexp | ||
| repl string | ||
| }{ | ||
| {regexp.MustCompile(`(?i)\bBearer\s+[A-Za-z0-9._~+/=-]+\b`), "Bearer [REDACTED]"}, | ||
| {regexp.MustCompile(`\bsk-ant-[A-Za-z0-9_-]+\b`), "[REDACTED_API_KEY]"}, | ||
|
Comment on lines
+349
to
+350
|
||
| {regexp.MustCompile(`\bsk-[A-Za-z0-9_-]{8,}\b`), "[REDACTED_API_KEY]"}, | ||
| {regexp.MustCompile(`\bAIza[0-9A-Za-z_-]{20,}\b`), "[REDACTED_API_KEY]"}, | ||
| {regexp.MustCompile(`(?i)\b(request[-_ ]?id)\s*[:=]\s*[A-Za-z0-9._-]+\b`), "$1=[REDACTED]"}, | ||
| {regexp.MustCompile(`\breq_[A-Za-z0-9_-]+\b`), "req_[REDACTED]"}, | ||
| } | ||
|
|
||
| func sanitizeProviderError(msg string) string { | ||
| sanitized := msg | ||
| for _, rule := range providerErrorSanitizers { | ||
| sanitized = rule.re.ReplaceAllString(sanitized, rule.repl) | ||
| } | ||
| return sanitized | ||
| } | ||
|
|
||
| // checkDippinLib verifies the dippin binary is installed. The full "dippin | ||
| // <ver> at <path>" string goes into the details so the CLI can print a | ||
| // per-item line; the composite summary carries the shorter "dippin <ver>" | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -3,9 +3,13 @@ | |||||
| package tracker | ||||||
|
|
||||||
| import ( | ||||||
| "context" | ||||||
| "os" | ||||||
| "path/filepath" | ||||||
| "strings" | ||||||
| "testing" | ||||||
|
|
||||||
| "github.com/2389-research/tracker/llm" | ||||||
| ) | ||||||
|
|
||||||
| func TestDoctor_NoProbe_KeyPresent(t *testing.T) { | ||||||
|
|
@@ -76,3 +80,60 @@ func TestDoctor_PipelineFileValidation(t *testing.T) { | |||||
| t.Fatal("Pipeline File check missing when PipelineFile set") | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| type doctorProbeTestAdapter struct { | ||||||
| completeErr error | ||||||
| } | ||||||
|
|
||||||
| func (a *doctorProbeTestAdapter) Name() string { return "test" } | ||||||
|
|
||||||
| func (a *doctorProbeTestAdapter) Complete(_ context.Context, _ *llm.Request) (*llm.Response, error) { | ||||||
| return nil, a.completeErr | ||||||
| } | ||||||
|
|
||||||
| func (a *doctorProbeTestAdapter) Stream(_ context.Context, _ *llm.Request) <-chan llm.StreamEvent { | ||||||
| ch := make(chan llm.StreamEvent) | ||||||
| close(ch) | ||||||
| return ch | ||||||
| } | ||||||
|
|
||||||
| func (a *doctorProbeTestAdapter) Close() error { return nil } | ||||||
|
|
||||||
| func TestSanitizeProviderError_RedactsSensitiveTokens(t *testing.T) { | ||||||
| in := "request failed: Authorization: Bearer verySecretToken request-id=req_abc123 key=sk-ant-supersecret AIzaSyA1234567890123456789012345" | ||||||
| got := sanitizeProviderError(in) | ||||||
|
|
||||||
| for _, secret := range []string{ | ||||||
| "verySecretToken", | ||||||
| "req_abc123", | ||||||
| "sk-ant-supersecret", | ||||||
| "AIzaSyA1234567890123456789012345", | ||||||
| } { | ||||||
| if strings.Contains(got, secret) { | ||||||
| t.Fatalf("sanitized message still contains secret %q: %q", secret, got) | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| func TestProbeProvider_SanitizesNonAuthError(t *testing.T) { | ||||||
| secret := "sk-1234567890SECRET" | ||||||
| ok, msg := probeProvider(providerDef{ | ||||||
| name: "OpenAI", | ||||||
| defaultModel: "gpt-4.1-mini", | ||||||
| buildAdapter: func(_ string) (llm.ProviderAdapter, error) { | ||||||
| return &doctorProbeTestAdapter{ | ||||||
| completeErr: &llm.ProviderError{ | ||||||
| SDKError: llm.SDKError{Msg: "boom Bearer topsecret " + secret + " req_abc123"}, | ||||||
| Provider: "openai", | ||||||
| }, | ||||||
| }, nil | ||||||
| }, | ||||||
| }, "test-key") | ||||||
|
|
||||||
| if ok { | ||||||
| t.Fatal("expected auth probe to fail") | ||||||
|
||||||
| t.Fatal("expected auth probe to fail") | |
| t.Fatal("expected non-auth probe to fail") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Bearer-token sanitizer regex ends with a word-boundary (\b). If the token ends with '=' (common for base64/JWT padding), '\b' will not match and the token will remain unredacted. Consider replacing the trailing '\b' with a lookahead like
(?=\s|$)(or otherwise anchoring on whitespace/end) so tokens ending in non-word characters are still sanitized, and add a unit test covering a Bearer token that ends with '='.