-
Notifications
You must be signed in to change notification settings - Fork 2.2k
[INS-240] Datadog detector verification fix #4616
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
base: main
Are you sure you want to change the base?
[INS-240] Datadog detector verification fix #4616
Conversation
| if len(uniqueFoundUrls) == 0 { | ||
| // In case no endpoints were found in data, use the default cloud endpoint | ||
| s.UseCloudEndpoint(true) | ||
| endpoints = s.Endpoints() | ||
| } else { | ||
| // In case endpoints were found in data, use them | ||
| // also add cloud endpoint at the end so if not validated against found endpoints, try cloud endpoint as well | ||
| endpoints = s.configureEndpoints(uniqueFoundUrls) | ||
| endpoints = append(endpoints, s.CloudEndpoint()) | ||
| } |
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.
UseCloudEndpoint defaults to true when no endpoints are configured. I think it’s clearer to reverse the condition and explicitly disable it only when we find endpoints in the data:
// If we found any endpoints in the data, disable the cloud endpoint
// and validate only against the found endpoints.
if len(uniqueFoundUrls) > 0 {
s.UseCloudEndpoint(false)
}
// Endpoints behavior:
// A) If the user configured endpoints, only those will be returned.
// B) If endpoints were found in the data, only those will be returned.
// C) If neither applies, it will return only cloud endpoint.
endpoints = s.Endpoints(uniqueFoundUrls...)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.
After this, we can drop the configureEndpoints function and normalize the endpoints:
for i, endpoint := range endpoints {
// Normalize endpoints so they all share the same format.
// - User-configured endpoints may be missing "https://"
// - Discovered endpoints also omit the scheme
if !strings.HasPrefix(endpoint, "https://") {
endpoint = "https://" + endpoint
}
// Ensure all endpoints end with "/api"
// (user-configured and cloud endpoints may omit it)
if !strings.HasSuffix(endpoint, "/api") {
endpoint = endpoint + "/api"
}
endpoints[i] = endpoint
}
// At this point, all endpoints start with "https://" and end with "/api",
// so they’re ready for direct use.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.
This can be normalized even more I believe but this seems like a reasonable good approach to me.
Description:
Datadog token verification previously relied on an App-key–dependent endpoint (/api/v2/users) to validate the Application Key + API Key combination. If this endpoint returned an error (for example, due to insufficient App Key permissions), the detector would mark the token as unverified and skip API key validation entirely.
This behavior caused valid Datadog API keys to be reported as unverified, resulting in false negatives.
To resolve this issue I have done the following changes:
Treat /api/v2/users as the primary verification path for App + API key combinations
Add a fallback verification step using the API-key–only validation endpoint when App key verification fails
Avoid calling the fallback endpoint when App key verification succeeds
Ensure valid API keys are marked verified even when the App key lacks required permissions
Checklist:
make test-community)?make lintthis requires golangci-lint)?