Skip to content

Suppress CodeQL.SM05137#862

Merged
dtivel merged 1 commit intomainfrom
dtivel/codeql
May 9, 2025
Merged

Suppress CodeQL.SM05137#862
dtivel merged 1 commit intomainfrom
dtivel/codeql

Conversation

@dtivel
Copy link
Collaborator

@dtivel dtivel commented May 8, 2025

Resolve #861.

@dtivel dtivel requested a review from a team as a code owner May 8, 2025 20:11
@dlemstra
Copy link
Collaborator

dlemstra commented May 8, 2025

I cannot find any information about CodeQL.SM05137. What kind of warning is this?

@dtivel
Copy link
Collaborator Author

dtivel commented May 8, 2025

It's a warning generated by internal code quality scanners that basically recommends against using DefaultAzureCredential in services and instead prefer specific authentication strategies (like ManagedIdentityCredential and WorkloadIdentityCredential) to ensure highly deterministic behavior.

It doesn't apply here because Sign CLI isn't a service.

Still, it might make sense at some point to introduce distinct paths for ManagedIdentityCredential and WorkloadIdentityCredential for users who want to opt in to exactly those strategies.

@dlemstra
Copy link
Collaborator

dlemstra commented May 9, 2025

I think we should take another look at adding an option like I did in #725 that got reverted for goods reasons. We could then allow only a single credential type. I was also running something similar yesterday. Because I was using something "later in the chain" I first saw all kinds of failed request. So maybe we should reintroduce this option to allow our users to specify which single credential type they want to use.

@dtivel
Copy link
Collaborator Author

dtivel commented May 9, 2025

@dlemstra, I do think that's a step in the right direction. If you want to use strategy X, we should use only strategy X. DefaultAzureCredential may still be a fallback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add CodeQL suppression

4 participants