fix: Clear-text storage of sensitive information#937
fix: Clear-text storage of sensitive information#937edwinjosechittilappilly wants to merge 1 commit into
Conversation
…ensitive information Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR addresses a security finding by preventing the OpenSearch admin password (OPENSEARCH_PASSWORD) from being written to the generated .env file (clear-text secret on disk).
Changes:
- Removed writing
OPENSEARCH_PASSWORD=...to.envinEnvManager.save_env_file(). - Added an inline comment explaining the intentional non-persistence.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| f.write(f"LANGFLOW_URL_INGEST_FLOW_ID={self._quote_env_value(self.config.langflow_url_ingest_flow_id)}\n") | ||
| f.write(f"NUDGES_FLOW_ID={self._quote_env_value(self.config.nudges_flow_id)}\n") | ||
| f.write(f"OPENSEARCH_PASSWORD={self._quote_env_value(self.config.opensearch_password)}\n") | ||
| # Intentionally do not persist OPENSEARCH_PASSWORD to avoid storing it in clear text on disk. |
There was a problem hiding this comment.
Removing OPENSEARCH_PASSWORD from the generated .env file likely breaks runtime and docker-compose behavior: docker-compose.yml and src/config/settings.py expect OPENSEARCH_PASSWORD to be set via environment/.env (and settings.py can raise if it's missing). Also, save_env_file() still calls setup_secure_defaults(), which auto-generates self.config.opensearch_password, but that value is now neither persisted nor exported, so a fresh install will end up with no OPENSEARCH_PASSWORD for containers to start/authenticate. Consider changing the save path to (1) avoid generating opensearch_password during save (so you don’t silently drop it), and (2) require OPENSEARCH_PASSWORD to come from an external secret mechanism (or at least write a placeholder/comment + explicit user instruction) while keeping docker-compose and runtime able to obtain the password.
| # Intentionally do not persist OPENSEARCH_PASSWORD to avoid storing it in clear text on disk. | |
| # Persist OPENSEARCH_PASSWORD so docker-compose and runtime can authenticate. | |
| # NOTE: This stores the password in clear text in .env; ensure appropriate file protections. | |
| f.write(f"OPENSEARCH_PASSWORD={self._quote_env_value(self.config.opensearch_password)}\n") |
|
Superseded by #1004 |
Potential fix for https://github.com/langflow-ai/openrag/security/code-scanning/15
In general, to fix clear-text storage of sensitive information, do not persist the secret in plain text. Either: (a) avoid persisting it entirely and derive or fetch it securely at runtime, or (b) store only an opaque reference (token/ID) and keep the actual secret in a dedicated secret store. Encrypting before storage is another option, but storing the encryption key alongside the ciphertext on the same host often gives limited benefit.
For this specific code, the minimal, functional-preserving change is: stop writing
OPENSEARCH_PASSWORDinto the.envfile, while still allowing the application to have a password when needed. The class already hasgenerate_secure_passwordandsetup_secure_defaultsthat populateself.config.opensearch_passwordif it is missing. We can adjust the logic so that:setup_secure_defaultsdoes not auto-generate and persist a password when saving the.envfile. Instead, it only respects an existing value from environment or.env.OPENSEARCH_PASSWORDat runtime should either:.env(if set), orHowever, we are constrained to editing only this file and the shown snippets. The concrete fix we can apply here is:
OPENSEARCH_PASSWORD=...into the.envfile insave_env_file.This removes the clear-text storage at the flagged sink while keeping the rest of the behavior (generation and in-memory usage) intact.
Concretely:
save_env_file, remove thef.writeline forOPENSEARCH_PASSWORD=....generate_secure_passwordandsetup_secure_defaultsunchanged (they only operate in memory), as the issue is specifically the disk write.Suggested fixes powered by Copilot Autofix. Review carefully before merging.