feat: Add `managed_secrets` app and integrate Azure Key Vault #574

devxpy · 2024-12-24T16:03:58Z

Introduce ManagedSecret model and admin interface for managing secrets
Update FunctionsPage and API key UI with manage_secrets_view for creating, editing, and deleting secrets
Configure AZURE_KEY_VAULT_ENDPOINT and add dependencies (azure-identity, azure-keyvault-secrets, etc.) to securely handle secrets
Enhance security by enabling encrypted secret storage and streamlined secret management in the UI

This change improves the application's security by providing a robust mechanism for managing sensitive environment variables using Azure Key Vault.

Q/A checklist

If you add new dependencies, did you update the lock file?

poetry lock --no-update

Run tests

ulimit -n unlimited && ./scripts/run-tests.sh

Do a self code review of the changes - Read the diff at least twice.
Carefully think about the stuff that might break because of this change - this sounds obvious but it's easy to forget to do "Go to references" on each function you're changing and see if it's used in a way you didn't expect.
The relevant pages still run when you press submit
The API for those pages still work (API tab)
The public API interface doesn't change if you didn't want it to (check API tab > docs page)
Do your UI changes (if applicable) look acceptable on mobile?
Ensure you have not regressed the import time unless you have a good reason to do so.
You can visualize this using tuna:

python3 -X importtime -c 'import server' 2> out.log && tuna out.log

To measure import time for a specific library:

$ time python -c 'import pandas'

________________________________________________________
Executed in    1.15 secs    fish           external
   usr time    2.22 secs   86.00 micros    2.22 secs
   sys time    0.72 secs  613.00 micros    0.72 secs

To reduce import times, import libraries that take a long time inside the functions that use them instead of at the top of the file:

def my_function():
    import pandas as pd
    ...

Legal Boilerplate

Look, I get it. The entity doing business as “Gooey.AI” and/or “Dara.network” was incorporated in the State of Delaware in 2020 as Dara Network Inc. and is gonna need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Dara Network Inc can use, modify, copy, and redistribute my contributions, under its choice of terms.

greptile-apps

PR Summary

This PR introduces a new managed secrets system using Azure Key Vault for secure environment variable management. Here are the key points to review:

Added ManagedSecret model in /managed_secrets/models.py with Azure Key Vault integration for secure secret storage and retrieval
Empty tests.py file is concerning for a security-critical feature - comprehensive test coverage should be added before merging
Signal handlers in signals.py need error handling and logging for Azure Key Vault operations
The executor.js changes expose environment variables to function execution context - careful security review needed
The UI implementation in views.py should add input validation and rate limiting for secret operations

The implementation looks solid overall but requires additional security hardening and test coverage before production deployment.

_{💡 (2/5) Greptile learns from your feedback when you react with 👍/👎!}

_{16 file(s) reviewed, 7 comment(s)}
_{Edit PR Review Bot Settings | Greptile}

managed_secrets/models.py

greptile-apps · 2024-12-24T16:05:55Z

managed_secrets/tests.py

+from django.test import TestCase
+
+# Create your tests here.


logic: Missing critical test coverage for ManagedSecret model, including: secret creation/deletion, Azure Key Vault integration, workspace-secret relationships, and access control. Add comprehensive tests to verify secure handling of sensitive data.

managed_secrets/signals.py

managed_secrets/views.py

recipes/Functions.py

greptile-apps · 2024-12-24T16:07:09Z

usage_costs/migrations/0023_alter_modelpricing_model_name.py

+        migrations.AlterField(
+            model_name='modelpricing',
+            name='model_name',
+            field=models.CharField(choices=[('o1_preview', 'o1-preview (openai)'), ('o1_mini', 'o1-mini (openai)'), ('gpt_4_o', 'GPT-4o (openai)'), ('gpt_4_o_mini', 'GPT-4o-mini (openai)'), ('chatgpt_4_o', 'ChatGPT-4o (openai) 🧪'), ('gpt_4_turbo_vision', 'GPT-4 Turbo with Vision (openai)'), ('gpt_4_vision', 'GPT-4 Vision (openai) 🔻'), ('gpt_4_turbo', 'GPT-4 Turbo (openai)'), ('gpt_4', 'GPT-4 (openai)'), ('gpt_4_32k', 'GPT-4 32K (openai) 🔻'), ('gpt_3_5_turbo', 'ChatGPT (openai)'), ('gpt_3_5_turbo_16k', 'ChatGPT 16k (openai)'), ('gpt_3_5_turbo_instruct', 'GPT-3.5 Instruct (openai) 🔻'), ('llama3_3_70b', 'Llama 3.3 70B'), ('llama3_2_90b_vision', 'Llama 3.2 90B + Vision (Meta AI)'), ('llama3_2_11b_vision', 'Llama 3.2 11B + Vision (Meta AI)'), ('llama3_2_3b', 'Llama 3.2 3B (Meta AI)'), ('llama3_2_1b', 'Llama 3.2 1B (Meta AI)'), ('llama3_1_70b', 'Llama 3.1 70B (Meta AI)'), ('llama3_1_8b', 'Llama 3.1 8B (Meta AI)'), ('llama3_70b', 'Llama 3 70B (Meta AI)'), ('llama3_8b', 'Llama 3 8B (Meta AI)'), ('mixtral_8x7b_instruct_0_1', 'Mixtral 8x7b Instruct v0.1 (Mistral)'), ('gemma_2_9b_it', 'Gemma 2 9B (Google)'), ('gemma_7b_it', 'Gemma 7B (Google)'), ('gemini_1_5_flash', 'Gemini 1.5 Flash (Google)'), ('gemini_1_5_pro', 'Gemini 1.5 Pro (Google)'), ('gemini_1_pro_vision', 'Gemini 1.0 Pro Vision (Google)'), ('gemini_1_pro', 'Gemini 1.0 Pro (Google)'), ('palm2_chat', 'PaLM 2 Chat (Google)'), ('palm2_text', 'PaLM 2 Text (Google)'), ('claude_3_5_sonnet', 'Claude 3.5 Sonnet (Anthropic)'), ('claude_3_opus', 'Claude 3 Opus [L] (Anthropic)'), ('claude_3_sonnet', 'Claude 3 Sonnet [M] (Anthropic)'), ('claude_3_haiku', 'Claude 3 Haiku [S] (Anthropic)'), ('afrollama_v1', 'AfroLlama3 v1 (Jacaranda)'), ('llama3_8b_cpt_sea_lion_v2_1_instruct', 'Llama3 8B CPT SEA-LIONv2.1 Instruct (aisingapore)'), ('sarvam_2b', 'Sarvam 2B (sarvamai)'), ('llama_3_groq_70b_tool_use', 'Llama 3 Groq 70b Tool Use [Deprecated]'), ('llama_3_groq_8b_tool_use', 'Llama 3 Groq 8b Tool Use [Deprecated]'), ('llama2_70b_chat', 'Llama 2 70B Chat [Deprecated] (Meta AI)'), ('sea_lion_7b_instruct', 'SEA-LION-7B-Instruct [Deprecated] (aisingapore)'), ('llama3_8b_cpt_sea_lion_v2_instruct', 'Llama3 8B CPT SEA-LIONv2 Instruct [Deprecated] (aisingapore)'), ('text_davinci_003', 'GPT-3.5 Davinci-3 [Deprecated] (openai)'), ('text_davinci_002', 'GPT-3.5 Davinci-2 [Deprecated] (openai)'), ('code_davinci_002', 'Codex [Deprecated] (openai)'), ('text_curie_001', 'Curie [Deprecated] (openai)'), ('text_babbage_001', 'Babbage [Deprecated] (openai)'), ('text_ada_001', 'Ada [Deprecated] (openai)'), ('protogen_2_2', 'Protogen V2.2 (darkstorm2150)'), ('epicdream', 'epiCDream (epinikion)'), ('flux_1_dev', 'FLUX.1 [dev]'), ('dream_shaper', 'DreamShaper (Lykon)'), ('dreamlike_2', 'Dreamlike Photoreal 2.0 (dreamlike.art)'), ('sd_2', 'Stable Diffusion v2.1 (stability.ai)'), ('sd_1_5', 'Stable Diffusion v1.5 (RunwayML)'), ('dall_e', 'DALL·E 2 (OpenAI)'), ('dall_e_3', 'DALL·E 3 (OpenAI)'), ('openjourney_2', 'Open Journey v2 beta (PromptHero)'), ('openjourney', 'Open Journey (PromptHero)'), ('analog_diffusion', 'Analog Diffusion (wavymulder)'), ('protogen_5_3', 'Protogen v5.3 (darkstorm2150)'), ('jack_qiao', 'Stable Diffusion v1.4 [Deprecated] (Jack Qiao)'), ('rodent_diffusion_1_5', 'Rodent Diffusion 1.5 [Deprecated] (NerdyRodent)'), ('deepfloyd_if', 'DeepFloyd IF [Deprecated] (stability.ai)'), ('dream_shaper', 'DreamShaper (Lykon)'), ('dreamlike_2', 'Dreamlike Photoreal 2.0 (dreamlike.art)'), ('sd_2', 'Stable Diffusion v2.1 (stability.ai)'), ('sd_1_5', 'Stable Diffusion v1.5 (RunwayML)'), ('dall_e', 'Dall-E (OpenAI)'), ('instruct_pix2pix', '✨ InstructPix2Pix (Tim Brooks)'), ('openjourney_2', 'Open Journey v2 beta (PromptHero) 🐢'), ('openjourney', 'Open Journey (PromptHero) 🐢'), ('analog_diffusion', 'Analog Diffusion (wavymulder) 🐢'), ('protogen_5_3', 'Protogen v5.3 (darkstorm2150) 🐢'), ('jack_qiao', 'Stable Diffusion v1.4 [Deprecated] (Jack Qiao)'), ('rodent_diffusion_1_5', 'Rodent Diffusion 1.5 [Deprecated] (NerdyRodent)'), ('sd_2', 'Stable Diffusion v2.1 (stability.ai)'), ('runway_ml', 'Stable Diffusion v1.5 (RunwayML)'), ('dall_e', 'Dall-E (OpenAI)'), ('jack_qiao', 'Stable Diffusion v1.4 [Deprecated] (Jack Qiao)'), ('wav2lip', 'LipSync (wav2lip)'), ('sadtalker', 'LipSync (sadtalker)')], help_text='The name of the model. Only used for Display purposes.', max_length=255),


logic: Duplicate model entries found for 'dream_shaper', 'dreamlike_2', 'sd_2', 'sd_1_5', 'dall_e', and 'jack_qiao'. This will cause issues with model selection since choices must be unique.

greptile-apps

PR Summary

This PR continues to evolve the managed secrets implementation. Here are the key new changes and concerns:

Added manage_secrets_table widget in /managed_secrets/widgets.py for streamlined secret management UI with validation and secure value handling
Updated account.py to integrate secrets management into the API Keys tab with proper section organization
Modified Functions.py to support optional code field and secret loading/decryption functionality
Added Azure Key Vault endpoint configuration in settings.py with proper dependency management

Key points to address:

The widgets.py implementation needs input sanitization and rate limiting for secret operations
Secret value validation and maximum length constraints should be added in models.py
Azure Key Vault operations in Functions.py need retry logic and error handling
The account page integration should consider access control granularity for secret management

_{16 file(s) reviewed, 10 comment(s)}
_{Edit PR Review Bot Settings | Greptile}

daras_ai_v2/manage_api_keys_widget.py

managed_secrets/admin.py

managed_secrets/models.py

managed_secrets/widgets.py

recipes/Functions.py

functions/executor.js

greptile-apps

PR Summary

This PR continues to evolve the managed secrets implementation. Here are the key new changes and concerns:

Duplicate model entries in usage_costs/migrations/0023_alter_modelpricing_model_name.py need to be deduplicated to prevent database integrity issues
The admin.py configuration needs to explicitly mark the secret value field as read-only to prevent exposure through the admin interface
The models.py implementation lacks proper transaction handling between database and Key Vault operations
The empty tests.py file still needs comprehensive test coverage for Azure Key Vault integration and error scenarios

The implementation is progressing but requires cleanup of model definitions and additional security hardening before production deployment.

_{16 file(s) reviewed, 2 comment(s)}
_{Edit PR Review Bot Settings | Greptile}

managed_secrets/signals.py

routers/account.py

devxpy

will add tests & migration fix later

- Introduce `ManagedSecret` model and admin interface for managing secrets - Update `FunctionsPage` and API key UI with `manage_secrets_view` for creating, editing, and deleting secrets - Configure `AZURE_KEY_VAULT_ENDPOINT` and add dependencies (`azure-identity`, `azure-keyvault-secrets`, etc.) to securely handle secrets - Enhance security by enabling encrypted secret storage and streamlined secret management in the UI This change improves the application's security by providing a robust mechanism for managing sensitive environment variables using Azure Key Vault.

greptile-apps

PR Summary

This PR continues to evolve the managed secrets implementation. Here are the key new changes and concerns:

Added Azure Key Vault client caching in /managed_secrets/models.py to avoid creating new clients for each operation
Improved error handling in /managed_secrets/signals.py for Azure Key Vault operations with proper transaction management
Added rate limiting and input validation in /managed_secrets/widgets.py for secret management operations
Implemented proper sandboxing and validation for environment variables in /functions/executor.js

The implementation shows good progress on security hardening but still requires test coverage before production deployment.

_{15 file(s) reviewed, 1 comment(s)}
_{Edit PR Review Bot Settings | Greptile}

greptile-apps · 2024-12-25T16:38:53Z

recipes/Functions.py

+        gui.multiselect(
+            label="",
+            options=options,
+            key="secrets",
+            allow_none=True,
+        )


logic: multiselect needs validation to prevent selecting non-existent secrets that may be in session state

greptile-apps bot reviewed Dec 24, 2024

View reviewed changes

devxpy force-pushed the secrets branch from 8ce30bb to 30abc89 Compare December 25, 2024 16:17

greptile-apps bot reviewed Dec 25, 2024

View reviewed changes

devxpy force-pushed the secrets branch from 30abc89 to c2283d4 Compare December 25, 2024 16:28

greptile-apps bot reviewed Dec 25, 2024

View reviewed changes

managed_secrets/signals.py Show resolved Hide resolved

routers/account.py Show resolved Hide resolved

devxpy commented Dec 25, 2024

View reviewed changes

devxpy force-pushed the secrets branch from c2283d4 to 597372e Compare December 25, 2024 16:36

devxpy merged commit cc7e582 into master Dec 25, 2024
4 of 5 checks passed

devxpy deleted the secrets branch December 25, 2024 16:37

greptile-apps bot reviewed Dec 25, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add `managed_secrets` app and integrate Azure Key Vault #574

feat: Add `managed_secrets` app and integrate Azure Key Vault #574

devxpy commented Dec 24, 2024

greptile-apps bot left a comment

greptile-apps bot Dec 24, 2024

greptile-apps bot Dec 24, 2024

greptile-apps bot left a comment

greptile-apps bot left a comment

devxpy left a comment

greptile-apps bot left a comment

greptile-apps bot Dec 25, 2024

feat: Add managed_secrets app and integrate Azure Key Vault #574

feat: Add managed_secrets app and integrate Azure Key Vault #574

Conversation

devxpy commented Dec 24, 2024

Q/A checklist

Legal Boilerplate

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

greptile-apps bot Dec 24, 2024

Choose a reason for hiding this comment

greptile-apps bot Dec 24, 2024

Choose a reason for hiding this comment

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

devxpy left a comment

Choose a reason for hiding this comment

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

greptile-apps bot Dec 25, 2024

Choose a reason for hiding this comment

feat: Add `managed_secrets` app and integrate Azure Key Vault #574

feat: Add `managed_secrets` app and integrate Azure Key Vault #574