-
Notifications
You must be signed in to change notification settings - Fork 26
fix: encrypting api key + session on the fly #294
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?
Conversation
|
this works. it does worry me a little though so i went ahead and got some tests written too for it. |
|
|
||
| // Index = Username | ||
| func SetApikey(ctx context.Context, Userdata User) error { | ||
| log.Printf("[AUDIT] Setting API key %s", Userdata.ApiKey) |
Check failure
Code scanning / CodeQL
Clear-text logging of sensitive information High
Sensitive data returned by an access to ApiKey
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 5 days ago
In general, to fix clear‑text logging of sensitive information, you should avoid logging secrets entirely, or at minimum log only non‑sensitive derivatives (such as a truncated or hashed value) that cannot be used to authenticate. For API keys, the usual patterns are: log only the associated username/user ID; or log a short, non‑reversible fingerprint of the key (e.g., first/last few characters, or a cryptographic hash).
For this specific case, the best fix that preserves functionality is to change the log line in SetApikey so it no longer outputs the raw Userdata.ApiKey. Instead, we can log the username (already available as Userdata.Username) and optionally a short, non‑sensitive representation of the key. Since crypto/sha256 is already imported in this file, we can compute a SHA‑256 hash of the API key and log only the hex‑encoded hash as a stable identifier, or we can log the last 4 characters to help operators correlate events without exposing the full secret. To avoid introducing new helpers outside the shown snippet, we’ll implement the masking inline in SetApikey using simple string operations and keep imports untouched.
Concretely: in db-connector.go, around line 4906, replace log.Printf("[AUDIT] Setting API key %s", Userdata.ApiKey) with a version that either omits the key and logs only the username, or logs a masked form such as **** plus the last 4 characters. This change does not alter any business logic—the API key is still stored as before—but removes clear‑text exposure in logs. No additional methods or imports are strictly required; we can do masking with the standard len and slicing operations already available.
-
Copy modified lines R4906-R4910
| @@ -4903,7 +4903,11 @@ | ||
|
|
||
| // Index = Username | ||
| func SetApikey(ctx context.Context, Userdata User) error { | ||
| log.Printf("[AUDIT] Setting API key %s", Userdata.ApiKey) | ||
| maskedKey := "****" | ||
| if len(Userdata.ApiKey) > 4 { | ||
| maskedKey = "****" + Userdata.ApiKey[len(Userdata.ApiKey)-4:] | ||
| } | ||
| log.Printf("[AUDIT] Setting API key for user %s (key suffix %s)", Userdata.Username, maskedKey) | ||
|
|
||
| newapiUser := new(Userapi) | ||
| newapiUser.Username = strings.ToLower(Userdata.Username) |
| if uuidRegex.MatchString(user.ApiKey) { | ||
| log.Printf("[AUDIT] API key is a UUID: %s", user.ApiKey) | ||
|
|
||
| encryptedKey, err := HandleKeyEncryption([]byte(user.ApiKey), "apikey", true) |
Check failure
Code scanning / CodeQL
Clear-text logging of sensitive information High
Sensitive data returned by an access to ApiKey
Copilot Autofix
AI 5 days ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
No description provided.