Skip to content
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

feat(refacto): changed a bit of things to make better dx #984

Merged
merged 1 commit into from
Aug 19, 2023

Conversation

StanGirard
Copy link
Collaborator

Description

Please include a summary of the changes and the related issue. Please also include relevant motivation and context.

Checklist before requesting a review

Please delete options that are not relevant.

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented hard-to-understand areas
  • I have ideally added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged

Screenshots (if appropriate):

@vercel
Copy link

vercel bot commented Aug 19, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs 🔄 Building (Inspect) Visit Preview Aug 19, 2023 11:31am
quivrapp ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 19, 2023 11:31am

@StanGirard StanGirard merged commit d0370ab into main Aug 19, 2023
3 of 4 checks passed
@github-actions
Copy link
Contributor

Risk Level 5 - /home/runner/work/quivr/quivr/install_helper.sh

  1. The script is asking for sensitive information such as SUPABASE_SERVICE_KEY, PG_DATABASE_URL, OPENAI_API_KEY, and JWT_SECRET_KEY and storing them in plain text in environment variable files. This is a high risk as it exposes sensitive information. Consider using a secure method to store these keys, such as using a secrets manager or encrypting the keys.

  2. The script does not handle the case where the user does not provide the required environment variables. This could potentially lead to unexpected behavior. Consider adding checks to ensure that the required environment variables are provided.


Risk Level 5 - /home/runner/work/quivr/quivr/backend/models/databases/supabase/api_key_handler.py

The create_api_key method stores the API key in plain text. This is a high risk as it exposes sensitive data. Consider hashing the API key before storing it.

import hashlib

hashed_api_key = hashlib.sha256(new_api_key.encode()).hexdigest()

Risk Level 5 - /home/runner/work/quivr/quivr/backend/models/databases/supabase/brains.py

The openai_api_key is being stored as plain text which is a high security risk. It's recommended to store such sensitive data in a more secure manner, for example, by using environment variables or a secure vault service. Also, it's not clear if there is any validation on the input data before it's being used or stored. This could lead to potential security vulnerabilities such as SQL injection or cross-site scripting (XSS) attacks. Always validate and sanitize input data.

# Instead of this
openai_api_key: Optional[str] = None

# Use this
openai_api_key: Optional[str] = os.getenv('OPENAI_API_KEY')

🔑🔓🚫


Powered by Code Review GPT

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.

1 participant