anthropic api key analyzer #3878
Conversation
| // SecretInfo hold the information about the anthropic key | ||
| type SecretInfo struct { | ||
| Valid bool | ||
| Type string // key type - TODO: Handle Anthropic Admin Keys |
There was a problem hiding this comment.
question: Is this still a TODO, or is this a relic?
There was a problem hiding this comment.
It's a TODO. Right now our anthropic detector does not detect Admin Keys. Once we update the detector we can update the analyzer as well to work with both type of keys.
| } | ||
|
|
||
| // AnthropicResource is any resource that can be accessed with anthropic key | ||
| type AnthropicResource struct { |
There was a problem hiding this comment.
Makes sense, the reason I did this is to avoid the confusion between AnalyzerResult Binding Resource and Anthropic Resource.
@abmussani @zricethezav your thoughts? Shall we go with this name or rename it to just Resource?
| } | ||
|
|
||
| bodyBytes, err := json.Marshal(body) | ||
| // https://docs.anthropic.com/en/api/models-list |
| @@ -104,23 +104,6 @@ func TestAnthropic_FromChunk(t *testing.T) { | |||
| wantErr: false, | |||
| wantVerificationErr: true, | |||
| }, | |||
| { | |||
There was a problem hiding this comment.
question: Are we no longer concerned with this test case?
There was a problem hiding this comment.
Honestly, I don't know. This test case was failing with valid secret so I removed it 😆
|
|
||
| var secretInfo = &SecretInfo{} | ||
|
|
||
| secretInfo.Type = "API-Key" // TODO: implement Admin-Key type as well |
There was a problem hiding this comment.
question: Do you think we can make this "magic" string a const?
| // create a HTTP client | ||
| client := analyzers.NewAnalyzeClient(cfg) | ||
|
|
||
| var secretInfo = &SecretInfo{} |
There was a problem hiding this comment.
question/suggestion: Is there a reason you used an empty variable declaration followed by setting the Type field, instead of a single assignment? I'm not very familiar with analyzers, so if this is standard practice in the codebase, feel free to disregard.
Ex:
secretInfo := &SecretInfo{Type: "API-Key"}
There was a problem hiding this comment.
My bad! 😶🌫️
|
|
||
| // anthropic key has full access only | ||
| secretInfo.Permissions = PermissionStrings[FullAccess] | ||
| secretInfo.Valid = true |
There was a problem hiding this comment.
question: Is there ever a scenario where secretInfo.Valid = false?
There was a problem hiding this comment.
Yes, it happens when the key passed is not valid. We print this on terminal. Once we are able to get resources that mean the key is valid.
|
|
||
| result := analyzers.AnalyzerResult{ | ||
| AnalyzerType: analyzers.AnalyzerAnthropic, | ||
| Metadata: map[string]any{}, |
There was a problem hiding this comment.
question: It looks like the Metadata is just Valid_Key. Could we just set it here directly?
Metadata: map[string]any{"Valid_Key": info.Valid}
| result := analyzers.AnalyzerResult{ | ||
| AnalyzerType: analyzers.AnalyzerAnthropic, | ||
| Metadata: map[string]any{}, | ||
| Bindings: make([]analyzers.Binding, 0), |
There was a problem hiding this comment.
suggestion: Looks like we can pre-allocate this slice.
make([]analyzers.Binding, 0, len(info.AnthropicResources))
| } | ||
|
|
||
| // secretInfoToAnalyzerResult translate secret info to Analyzer Result | ||
| func secretInfoToAnalyzerResult(info *SecretInfo) *analyzers.AnalyzerResult { |
There was a problem hiding this comment.
question: It looks like we collect AnthropicResources from listModels and listMessageBatches in *SecretInfo, then iterate over them again to create Bindings. Could we skip the intermediary *SecretItem and build Bindings directly by passing it to listModels and listMessageBatches? That seems like it would eliminate the extra iteration. I might be missing something, so feel free to correct me if I'm off base.
There was a problem hiding this comment.
EDIT: After reviewing another analyzer, I don't think we should buck the trend of how these currently work. Please ignore the above comment.
Description:
This PR add a new analyzer for anthropic detector
Test Cases:
Checklist:
make test-community)?make lintthis requires golangci-lint)?