Repository navigation
fix(encryption): return null on decryption failure to prevent sending encrypted tokens to providers #1462
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
fix(encryption): return null on decryption failure to prevent sending encrypted tokens to providers #1462
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 |
|---|---|---|
|
|
@@ -110,14 +110,16 @@ export function decrypt(ciphertext: string | null | undefined): string | null | | |
| console.warn( | ||
| "[Encryption] Found encrypted data but STORAGE_ENCRYPTION_KEY is not set. Cannot decrypt." | ||
| ); | ||
| return ciphertext; | ||
| // Return null instead of encrypted ciphertext to prevent sending encrypted tokens to providers | ||
| return null; | ||
| } | ||
|
|
||
| const body = ciphertext.slice(PREFIX.length); | ||
| const parts = body.split(":"); | ||
| if (parts.length !== 3) { | ||
| console.error("[Encryption] Malformed encrypted value"); | ||
| return ciphertext; | ||
| // Return null instead of encrypted ciphertext to prevent sending malformed encrypted tokens to providers | ||
| return null; | ||
|
Contributor
Author
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. @/tmp/bot-response.md |
||
| } | ||
|
|
||
| const [ivHex, encryptedHex, authTagHex] = parts; | ||
|
|
@@ -134,7 +136,8 @@ export function decrypt(ciphertext: string | null | undefined): string | null | | |
| } catch (err: unknown) { | ||
| const message = err instanceof Error ? err.message : String(err); | ||
| console.error("[Encryption] Decryption failed:", message); | ||
| return ciphertext; | ||
| // Return null instead of encrypted ciphertext to prevent sending encrypted tokens to providers | ||
| return null; | ||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -66,14 +66,16 @@ test("connection field helpers encrypt and decrypt all supported credential fiel | |
| assert.deepEqual(decrypted, connection); | ||
| }); | ||
|
|
||
| test("decrypt returns the original ciphertext when the value is malformed or the key is wrong", async () => { | ||
| test("decrypt returns null when the value is malformed or the key is wrong", async () => { | ||
| process.env.STORAGE_ENCRYPTION_KEY = "task-304-secret-c"; | ||
| const firstModule = await importFresh("src/lib/db/encryption.ts"); | ||
| const encrypted = firstModule.encrypt("top-secret"); | ||
|
|
||
| process.env.STORAGE_ENCRYPTION_KEY = "task-304-secret-d"; | ||
| const secondModule = await importFresh("src/lib/db/encryption.ts"); | ||
|
|
||
| assert.equal(secondModule.decrypt(encrypted), encrypted); | ||
| assert.equal(secondModule.decrypt("enc:v1:not-valid"), "enc:v1:not-valid"); | ||
| // When decryption fails with wrong key, return null (not encrypted ciphertext) | ||
| // This prevents sending encrypted tokens to APIs | ||
| assert.equal(secondModule.decrypt(encrypted), null); | ||
| assert.equal(secondModule.decrypt("enc:v1:not-valid"), null); | ||
|
Comment on lines
+79
to
+80
Contributor
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. This test confirms that |
||
| }); | ||
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.
The security fix is incomplete. While this change correctly returns
nullfor malformed values, thedecryptfunction still returns the encryptedciphertexton line 113 when theSTORAGE_ENCRYPTION_KEYis missing. To fully satisfy the security goal of preventing encrypted tokens from being sent to upstream APIs, line 113 should also be updated to returnnull.