Potential fix for code scanning alert no. 7: Clear-text storage of sensitive information #378
Conversation
…nsitive information Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| f"\nTo persist this setting, add the following line to your shell config " | ||
| f"(e.g. ~/.bashrc or ~/.zshrc):\n {export_line}\n" |
Check failure
Code scanning / CodeQL
Clear-text logging of sensitive information High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 days ago
In general, the fix is to ensure that sensitive values (API keys) are never included in log or print output. If we still want to help the user persist the variable, we can print only the variable name and a template/export command without the actual secret, or instruct the user in prose. This maintains functionality (guidance on how to persist) without echoing the secret back.
For this specific code, the best change is within _save_env_var in cortex/first_run_wizard.py. Instead of building export_line with the full value and printing it, we should either (a) print an export command with a placeholder like <YOUR_API_KEY_HERE> or (b) simply instruct the user to add an appropriate line with their value, without constructing a string that contains value. The environment variable still gets set for the current process via os.environ[name] = value, so existing runtime behavior is preserved. Only user-facing messaging changes.
Concretely:
-
Keep
os.environ[name] = valueunchanged. -
Remove or change
export_line = f'export {name}="{value}"'. -
Replace the
printcall so that it no longer interpolatesvalue, and instead uses a placeholder or generic instruction, e.g.:print( f"\nTo persist this setting, add a line like the following to your shell " f"config (e.g. ~/.bashrc or ~/.zshrc), replacing <YOUR_SECRET> with your " f"actual value:\n export {name}=\"<YOUR_SECRET>\"\n" )
No new imports or helper methods are required; we only adjust this one method’s body.
| @@ -763,17 +763,18 @@ | ||
|
|
||
| To avoid storing sensitive data in clear text on disk, this method no longer | ||
| appends the value to shell configuration files. Instead, it sets the variable | ||
| for the current process and prints the export command so the user can add it | ||
| manually if they choose. | ||
| for the current process and prints guidance so the user can add it | ||
| manually if they choose, without echoing the secret value. | ||
| """ | ||
| # Set for current session only | ||
| os.environ[name] = value | ||
|
|
||
| # Show user how to persist it without doing so automatically | ||
| export_line = f'export {name}="{value}"' | ||
| # Show user how to persist it without doing so automatically, | ||
| # and without printing the sensitive value. | ||
| print( | ||
| f"\nTo persist this setting, add the following line to your shell config " | ||
| f"(e.g. ~/.bashrc or ~/.zshrc):\n {export_line}\n" | ||
| f"\nTo persist this setting, add a line like the following to your shell " | ||
| f"config (e.g. ~/.bashrc or ~/.zshrc), replacing <YOUR_SECRET> with your " | ||
| f"actual value:\n export {name}=\"<YOUR_SECRET>\"\n" | ||
| ) | ||
|
|
||
|
|
Potential fix for https://github.com/cortexlinux/cortex/security/code-scanning/7
General approach: Avoid storing raw API keys directly in long‑lived, plain‑text shell configuration files. Either (1) don’t persist the key at all (use it only in‑memory), (2) store it in a dedicated config file with restricted permissions, or (3) if we must assist the user in adding it to their shell, print instructions rather than editing the rc file automatically.
Best fix with minimal functional change: Keep setting the environment variable for the current process (so the rest of the wizard behaves as before) but stop appending the raw secret to shell rc files. Instead,
_save_env_varwill setos.environ[name] = valueand log a message telling the user to add the appropriateexportline themselves if they want persistence. This avoids writing secrets to disk while still giving the user the information they need.Concretely:
FirstRunWizard._save_env_varincortex/first_run_wizard.py:export_line, and appends it to that file.os.environ[name] = value.exportline), but without writing it automatically._setup_claude_api,_setup_openai_api), because they already go through_save_env_var.Needed elements:
_save_env_varand can rely on existing imports (os,logger).Suggested fixes powered by Copilot Autofix. Review carefully before merging.