-
Notifications
You must be signed in to change notification settings - Fork 6.4k
Description
Summary
CodeQL has resulted in a number of false positives/low grade issues. Given that it's going to continue to be used, it seems like it may be worth it to acede to some of its demands. One of the is to not call a field that references a password a password (or a secret), but just anything that isn't a password, since it isn't the password, it's just something else.
Motivation
CodeQL will complain about this:
cmd/argocd/commands/admin/redis_initial_password.go:50
Sensitive data returned by an access to redisInitialPasswordSecretName flows to a logging call.
redisInitialPasswordSecretName := common.DefaultRedisInitialPasswordSecretName
redisInitialPasswordKey := common.DefaultRedisInitialPasswordKey
fmt.Printf("Checking for initial Redis password in secret %s/%s at key %s. \n", namespace, redisInitialPasswordSecretName, redisInitialPasswordKey)
CodeQL
config, err := clientConfig.ClientConfig()
errors.CheckError(err)
Proposal
Rename fields from PasswordSecret
to Credentials
and PasswordKey
to CredentialsKey
because the busywork definition doesn't include it:
https://github.com/github/codeql/blob/590e93d8edec4d7216935ed4425a7ab77b3b2f34/go/ql/lib/semmle/go/security/SensitiveActions.qll#L87-L101
Thus,
redisInitialCredentialsName := common.DefaultRedisInitialCredentialsName
redisInitialCredentialsKey := common.DefaultRedisInitialCredentialsKey
fmt.Printf("Checking for initial Redis password in secret %s/%s at key %s. \n", namespace, redisInitialCredentialsName, redisInitialCredentialsKey)
Yes, I'm aware that the Kubernetes type Secret
is washed away from redisInitialCredentialsName
, as it's more or less locally used right next to secret %s/%s
, I don't think that's particularly problematic.
No, I'm not a huge fan of this exercise, but, having just filed a bunch of bugs against CodeQL and closed a bunch of tickets here, I'd rather not spend time closing more tickets here if we can fairly easily appease CodeQL and not cause us too much grief in the process.
This issue is submitted under protest.