-
Notifications
You must be signed in to change notification settings - Fork 408
feat: Api Keys database encryption. #33
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,62 @@ | ||||||||||||||||||||||||||||||||||
| import crypto from 'crypto'; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| const ALGORITHM = 'aes-256-cbc'; | ||||||||||||||||||||||||||||||||||
| const IV_LENGTH = 16; // For AES, this is always 16 bytes | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||
| * Encrypts a plaintext string using AES-256-CBC. | ||||||||||||||||||||||||||||||||||
| * The encryption key is derived from the ENCRYPTION_SECRET environment variable. | ||||||||||||||||||||||||||||||||||
| * The output is a base64-encoded string in the format "ciphertext:iv". | ||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||
| * @param text The plaintext string to encrypt. | ||||||||||||||||||||||||||||||||||
| * @returns The encrypted string in "ciphertext:iv" format (base64-encoded). | ||||||||||||||||||||||||||||||||||
| * @throws Error if ENCRYPTION_SECRET is not set. | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+6
to
+13
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix documentation: output is hex-encoded, not base64. The JSDoc comment states the output is "base64-encoded" but the implementation uses hex encoding (lines 26-29). Update the documentation to match the actual behavior. Suggested fix /**
* Encrypts a plaintext string using AES-256-CBC.
* The encryption key is derived from the ENCRYPTION_SECRET environment variable.
- * The output is a base64-encoded string in the format "ciphertext:iv".
+ * The output is a hex-encoded string in the format "ciphertext:iv".
*
* `@param` text The plaintext string to encrypt.
- * `@returns` The encrypted string in "ciphertext:iv" format (base64-encoded).
+ * `@returns` The encrypted string in "ciphertext:iv" format (hex-encoded).
* `@throws` Error if ENCRYPTION_SECRET is not set.
*/📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||
| export function encrypt(text: string): string { | ||||||||||||||||||||||||||||||||||
| const ENCRYPTION_SECRET = process.env.ENCRYPTION_SECRET; | ||||||||||||||||||||||||||||||||||
| if (!ENCRYPTION_SECRET) { | ||||||||||||||||||||||||||||||||||
| throw new Error('ENCRYPTION_SECRET environment variable is not set.'); | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // Use a consistent key length for AES-256 (32 bytes) | ||||||||||||||||||||||||||||||||||
| const key = crypto.scryptSync(ENCRYPTION_SECRET, 'salt', 32); | ||||||||||||||||||||||||||||||||||
| const iv = crypto.randomBytes(IV_LENGTH); | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+21
to
+23
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use a unique salt instead of a fixed literal. The hardcoded Options to improve:
Option 2 is more secure but requires a format change. Option 1 is simpler and still provides meaningful protection. 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||
| const cipher = crypto.createCipheriv(ALGORITHM, key, iv); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| let encrypted = cipher.update(text, 'utf8', 'hex'); | ||||||||||||||||||||||||||||||||||
| encrypted += cipher.final('hex'); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| return `${encrypted}:${iv.toString('hex')}`; | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||
| * Decrypts an encrypted string (in "ciphertext:iv" format) using AES-256-CBC. | ||||||||||||||||||||||||||||||||||
| * The encryption key is derived from the ENCRYPTION_SECRET environment variable. | ||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||
| * @param encryptedText The encrypted string in "ciphertext:iv" format (base64-encoded). | ||||||||||||||||||||||||||||||||||
| * @returns The decrypted plaintext string. | ||||||||||||||||||||||||||||||||||
| * @throws Error if ENCRYPTION_SECRET is not set or if the format is invalid. | ||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+32
to
+39
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix documentation for decrypt function as well. Same issue: the JSDoc says "base64-encoded" but the function expects hex-encoded input. Suggested fix /**
* Decrypts an encrypted string (in "ciphertext:iv" format) using AES-256-CBC.
* The encryption key is derived from the ENCRYPTION_SECRET environment variable.
*
- * `@param` encryptedText The encrypted string in "ciphertext:iv" format (base64-encoded).
+ * `@param` encryptedText The encrypted string in "ciphertext:iv" format (hex-encoded).
* `@returns` The decrypted plaintext string.
* `@throws` Error if ENCRYPTION_SECRET is not set or if the format is invalid.
*/📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||
| export function decrypt(encryptedText: string): string { | ||||||||||||||||||||||||||||||||||
| const ENCRYPTION_SECRET = process.env.ENCRYPTION_SECRET; | ||||||||||||||||||||||||||||||||||
| if (!ENCRYPTION_SECRET) { | ||||||||||||||||||||||||||||||||||
| throw new Error('ENCRYPTION_SECRET environment variable is not set.'); | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| const parts = encryptedText.split(':'); | ||||||||||||||||||||||||||||||||||
| if (parts.length !== 2) { | ||||||||||||||||||||||||||||||||||
| throw new Error('Invalid encrypted text format. Expected "ciphertext:iv".'); | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| const encrypted = parts[0]; | ||||||||||||||||||||||||||||||||||
| const iv = Buffer.from(parts[1], 'hex'); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // Use a consistent key length for AES-256 (32 bytes) | ||||||||||||||||||||||||||||||||||
| const key = crypto.scryptSync(ENCRYPTION_SECRET, 'salt', 32); | ||||||||||||||||||||||||||||||||||
| const decipher = crypto.createDecipheriv(ALGORITHM, key, iv); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| let decrypted = decipher.update(encrypted, 'hex', 'utf8'); | ||||||||||||||||||||||||||||||||||
| decrypted += decipher.final('utf8'); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| return decrypted; | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| -- This migration marks the AI API key columns in organization_settings | ||
| -- as intended for encrypted storage. The application layer will handle | ||
| -- encryption and decryption. | ||
|
|
||
| -- Add comments to the columns for clarity | ||
| COMMENT ON COLUMN public.organization_settings.ai_google_key IS 'Encrypted Google/Gemini API key (ciphertext:iv)'; | ||
| COMMENT ON COLUMN public.organization_settings.ai_openai_key IS 'Encrypted OpenAI API key (ciphertext:iv)'; | ||
| COMMENT ON COLUMN public.organization_settings.ai_anthropic_key IS 'Encrypted Anthropic/Claude API key (ciphertext:iv)'; | ||
|
Comment on lines
+1
to
+8
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing data migration for existing plaintext keys. This migration only adds documentation comments but does not encrypt any existing plaintext API keys in the database. If there are existing rows in Consider adding a data migration script (run separately or as part of deployment) to:
Alternatively, document that this is a breaking change requiring fresh data or a manual migration step. 🤖 Prompt for AI Agents |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove quotes from the example value and add key requirements.
The double quotes around the placeholder value will be interpreted literally in
.envfiles, causing the actual secret to include the quote characters. Also, consider documenting the recommended key entropy.Suggested fix
📝 Committable suggestion
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 37-37: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
🤖 Prompt for AI Agents