fix: make third-party token encryption fail closed#5223
Conversation
|
@Krishnx21 is attempting to deploy a commit to the jhasourav07's projects Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 41872aeb5b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Aamod-Dev
left a comment
There was a problem hiding this comment.
Thanks for putting this together. The code looks clean and solves the issue effectively.
I'm happy to approve this. Great job!
Aamod-Dev
left a comment
There was a problem hiding this comment.
Thanks for the contribution. I went through the changes and everything looks solid. The code is readable, well-structured, and aligns with the project conventions.
I'll go ahead and approve this PR. Thanks again for the contribution!
Aamod-Dev
left a comment
There was a problem hiding this comment.
Thanks for the contribution. I went through the changes and the overall approach looks good, but there are a few issues that should be addressed before this can be merged. Most of the concerns are related to correctness and maintainability.
- There are merge conflicts with the base branch. Please resolve them to ensure existing functionality isn't broken.
Once these issues are addressed, I'll be happy to take another look. Thanks again for the contribution.
Aamod-Dev
left a comment
There was a problem hiding this comment.
Thanks for the contribution. It looks like there are merge conflicts with the base branch. Please rebase and resolve the conflicts so we can proceed with testing and merging. Thanks!
|
@Aamod007 yeah got it i m working on it . |
41872ae to
928b4af
Compare
|
Rebased and resolved the base-branch conflict. The branch is now based on the latest |
|
🚨 Hey @Krishnx21, the CI Pipeline is failing on this PR and it has been marked as Please fix the issues before this can be reviewed. Here's how: 1. Run checks locally before pushing: npm run format:check # Check Prettier formatting
npm run lint # Run ESLint
npm run typecheck # TypeScript type check
npm run test # Run unit tests (Vitest)
npm run build # Verify production build passes2. Auto-fix common issues: npm run format # Auto-fix formatting with Prettier
npm run lint -- --fix # Auto-fix lint errors where possible3. Check the full failure log here: Once you push a fix and the CI passes, the |
|
@Aamod007 how can i fix this ci issue ?? guide me |
Aamod-Dev
left a comment
There was a problem hiding this comment.
Thanks for resolving the merge conflicts! Approving.
|
🎉 Congratulations @Krishnx21! Your PR has been successfully merged. 🚀 Thank you for contributing to CommitPulse. Your work helps us build a better tool for the community.
Keep building! 💻✨ |
Summary
ENCRYPTION_KEYwith at least 32 charactersSecurity impact
Previously, deployments missing
ENCRYPTION_KEYencrypted sensitive tokens with a publicly known key.decryptTokenalso silently accepted plaintext values as if they had been decrypted successfully.Encryption and decryption now fail closed when configuration or ciphertext is invalid.
Verification
npm run test -- utils/encryption.test.ts utils(29 files, 217 tests passed)npm run typecheckgit diff --checknpm run buildcould not start compilation because Windows/OneDrive denied Next.js permission to remove the pre-existing generated.next/diagnosticsdirectory (EPERM).Fixes #5222