-
Couldn't load subscription status.
- Fork 85
feat(core): improve secret encryption #1063
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
Conversation
commit: |
🚀 Website Preview DeployedYour website preview is ready! Preview URL: https://d84f147c-alchemy-website.alchemy-run.workers.dev This preview was built from commit 1508515 🤖 This comment will be updated automatically when you push new commits to this PR. |
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.
works tests passing
| ); | ||
| } | ||
| if (typeof value["@secret"] === "object") { | ||
| if (typeof value["@secret"] === "object" && "data" in value["@secret"]) { |
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.
What happens if data is not present? Do we just skip it?
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.
Quick answer: no, we don't skip it. The data property indicates an encrypted object; otherwise it's an encrypted string. This is the same as before.
- if (typeof value["@secret"] === "object") {
+ if (typeof value["@secret"] === "object" && "data" in value["@secret"]) {
return new Secret(
JSON.parse(
await decryptWithKey(value["@secret"].data, scope.password),
),
);
}
return new Secret(await decryptWithKey(value["@secret"], scope.password));Why the change? The encrypted value is now an object with properties, so typeof value["@secret"] === "object" is always true. Checking for the data property is how we differentiate the two cases.
The serialization logic is completely unchanged.
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.
I was looking for the code that is handling backwards compatibility (decrypting old secrets). Can you point that out to me.
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.
That's handled by the decryptWithKey function internally: https://github.com/alchemy-run/alchemy/pull/1063/files#diff-d321ef7ae3f0b1da62fe32aea4ed9f267260c1c052ecda14ec8e8c2eb4827e4bR23-R31
Might close #1062. What do we think?
Uses AES-256-GCM with scrypt-generated keys.