-
-
Notifications
You must be signed in to change notification settings - Fork 905
security: remedy dependabot alerts #2723
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
nicktrn
commented
Dec 2, 2025
- upgrade vite
- override js-yaml
- emails: add typecheck
- emails: update nodemailer, and because of this had to @aws-sdk/client-ses -> sesv2
|
WalkthroughThis PR updates the emails package dependencies and configuration. It upgrades the AWS SES client from v1 to v2, updates nodemailer from v6 to v7 with corresponding type definitions, and adds a typecheck script to the package.json. Additionally, the root package.json is updated to bump vite and add pnpm overrides for js-yaml versions to address security vulnerabilities. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes Areas requiring extra attention:
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
internal-packages/emails/src/transports/aws-ses.ts (1)
4-18: SESv2 + Nodemailer v7 setup matches official SES transport pattern.Using
SESv2Clientand wiring Nodemailer withSES: { sesClient, SendEmailCommand }is consistent with the documented SESv2 transport configuration and should work correctly with nodemailer 7’s built-in SES transport. (nodemailer.com)
If you ever need custom region/credentials or easier testing, consider allowing a preconfiguredSESv2Clientto be injected viaAwsSesMailTransportOptions, but that’s an optional refinement rather than a blocker.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
internal-packages/emails/package.json(1 hunks)internal-packages/emails/src/transports/aws-ses.ts(2 hunks)package.json(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,ts,jsx,tsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier
Files:
package.jsoninternal-packages/emails/src/transports/aws-ses.tsinternal-packages/emails/package.json
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
Files:
internal-packages/emails/src/transports/aws-ses.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
internal-packages/emails/src/transports/aws-ses.ts
🧠 Learnings (6)
📚 Learning: 2025-11-27T16:27:48.109Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T16:27:48.109Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use vitest for unit tests
Applied to files:
package.json
📚 Learning: 2025-11-27T16:26:37.432Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T16:26:37.432Z
Learning: Applies to **/*.{test,spec}.{ts,tsx} : Use vitest for all tests in the Trigger.dev repository
Applied to files:
package.jsoninternal-packages/emails/package.json
📚 Learning: 2025-11-26T14:40:07.146Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 2710
File: packages/schema-to-json/package.json:0-0
Timestamp: 2025-11-26T14:40:07.146Z
Learning: Node.js 24+ has native TypeScript support and can execute .ts files directly without tsx or ts-node for scripts that use only erasable TypeScript syntax (type annotations, interfaces, etc.). The trigger.dev repository uses Node.js 24.11.1+ and scripts like updateVersion.ts can be run with `node` instead of `tsx`.
Applied to files:
package.jsoninternal-packages/emails/package.json
📚 Learning: 2025-11-27T16:27:48.109Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T16:27:48.109Z
Learning: pnpm version `10.23.0` and Node.js version `20.11.1` are required for development
Applied to files:
package.json
📚 Learning: 2025-11-27T16:26:37.432Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T16:26:37.432Z
Learning: Applies to {packages/core,apps/webapp}/**/*.{ts,tsx} : Use zod for validation in packages/core and apps/webapp
Applied to files:
internal-packages/emails/package.json
📚 Learning: 2025-11-27T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Applies to apps/webapp/**/*.{ts,tsx} : Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp
Applied to files:
internal-packages/emails/package.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (23)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (4)
package.json (2)
65-65: Vite 5 upgrade looks reasonable; verify app build/dev flows.Bumping from Vite 4.x to 5.4.21 is a major upgrade and looks appropriate for security triage, but it’s worth running the Vite-based apps through their usual dev/build flows under the repo’s supported Node version to catch any config or plugin regressions.
91-93: Axios/js‑yaml pnpm overrides align with security hardening.These overrides centralize axios and js‑yaml onto patched versions across the workspace, which is a good way to address Dependabot alerts; just be aware they can override transitive dependency expectations, so keep an eye on any packages that intentionally pin older versions and remove these overrides once upstreams are updated.
internal-packages/emails/package.json (2)
9-11: Local typecheck script is a good addition.Adding
typecheck: "tsc --noEmit"for the emails package is a solid, low-risk improvement and should help catch transport/type regressions early, especially around the new SES/Nodemailer integration.
13-24: Dependency bumps line up with the SESv2/Nodemailer transport change.Switching from
@aws-sdk/client-sesto@aws-sdk/client-sesv2and bumpingnodemailer+@types/nodemailerto v7 aligns with the updatedaws-ses.tstransport wiring and with Nodemailer’s SESv2 transport documentation; the set looks internally consistent. (nodemailer.com)