Replace nodemailer-openpgp with built-in implementation + bump nodemailer and postcss/cssnano#14404
Conversation
…iler and postcss/cssnano
julio-rocketchat
requested review from
Grubba27,
fredmaiaarantes,
henriquealbert,
italojs and
nachocodoner
as code owners
✅ Deploy Preview for v3-meteor-api-docs canceled.
|
✅ Deploy Preview for v3-migration-docs canceled.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a local OpenPGP Nodemailer stream plugin and tests, replaces the external nodemailer-openpgp dependency with a local implementation, upgrades nodemailer and adds openpgp, updates email test fixtures/expectations, and separately bumps minifier-css and its npm deps. ChangesEmail Package OpenPGP Integration
Minifier-CSS Dependency Update
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/email/email_openpgp_tests.js`:
- Around line 207-439: The tests set global state (Email.customTransport and
Email.hookSend) but only reset them on the success path; wrap each test's
override in a try/finally so Email.customTransport is restored and any hook is
stopped in the finally block. Specifically, for tests that assign
Email.customTransport, ensure a try { ... await Email.sendAsync(...) ...
assertions ... } finally { Email.customTransport = undefined; } and for the test
that uses const hook = Email.hookSend(...) ensure the finally calls hook.stop()
before clearing Email.customTransport to guarantee cleanup even if sendAsync,
transport.sendMail, or assertions throw.
In `@packages/email/openpgp-encrypt.js`:
- Around line 13-29: readHeaders currently unfolds continuation lines and stores
only name/value pairs, causing writeHeaders to reserialize passthrough headers
as single lines and lose original folding (see readHeaders and the passthrough
handling around lines 49-50). Change the parsing to retain the original raw
header text for any passthrough header (store an additional raw string field,
e.g., raw or originalLine, alongside name/value) or preserve the folded lines
array so writeHeaders can emit the original raw text; alternatively implement
proper RFC 5322 re-folding when emitting headers. Update readHeaders to capture
the exact physical header lines (including continuations) for each header and
adjust writeHeaders to prefer emitting that raw text for passthrough headers
instead of reconstructing a single unfolded line.
- Around line 226-248: The SMTP path bypasses openpgpEncrypt because the
built-in transport is created without per-mail options; update transport
registration so the plugin is applied unconditionally (or ensure mail options
are threaded into transport creation) by registering openpgpEncrypt on the
built-in transport during initialization rather than only when a transport
explicitly calls openpgpEncrypt, and have handleStream (the function returned by
openpgpEncrypt) read per-message flags from mail.data (encryptionKeys /
shouldSign) and no-op when absent; concretely, ensure the code path that
constructs the default transport used by Email.sendAsync attaches the
openpgpEncrypt plugin (or accepts mail-level options and passes them into
mail.message.transform/PGPMimeTransform) so messages sent via Email.sendAsync
honor encryptionKeys and shouldSign without requiring Email.customTransport.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b0d16d4a-fc19-44aa-b406-feabbebbd7cc
⛔ Files ignored due to path filters (1)
packages/email/test/fixtures/test_public.pemis excluded by!**/*.pem
📒 Files selected for processing (8)
packages/email/email.jspackages/email/email_openpgp_tests.jspackages/email/email_tests_data.jspackages/email/openpgp-encrypt.jspackages/email/package.jspackages/email/test/fixtures/test_private_1024bit.keypackages/email/test/fixtures/test_private_2048bit.keypackages/minifier-css/package.js
| Tinytest.addAsync( | ||
| 'email - openpgp - Email.sendAsync encrypts through the full stack', | ||
| async function (test) { | ||
| let capturedMessage = null; | ||
|
|
||
| Email.customTransport = async (options) => { | ||
| const transport = nodemailer.createTransport({ | ||
| streamTransport: true, | ||
| buffer: true, | ||
| newline: 'unix', | ||
| }); | ||
| if (options.encryptionKeys || options.shouldSign) { | ||
| transport.use( | ||
| 'stream', | ||
| openpgpEncrypt({ | ||
| signingKey: PRIVATE_KEY_2048, | ||
| passphrase: PASSPHRASE, | ||
| }) | ||
| ); | ||
| } | ||
| const info = await transport.sendMail(options); | ||
| capturedMessage = info.message.toString(); | ||
| }; | ||
|
|
||
| await Email.sendAsync({ | ||
| from: 'a@b.com', | ||
| to: 'c@d.com', | ||
| subject: 'Full stack encrypt', | ||
| text: 'Secret message', | ||
| encryptionKeys: [PUBLIC_KEY], | ||
| }); | ||
|
|
||
| test.isNotNull(capturedMessage, 'customTransport should have been called'); | ||
| test.isTrue( | ||
| capturedMessage.indexOf('multipart/encrypted') >= 0, | ||
| 'output should be multipart/encrypted' | ||
| ); | ||
| test.isTrue( | ||
| capturedMessage.indexOf( | ||
| 'This is an OpenPGP/MIME encrypted message' | ||
| ) >= 0, | ||
| 'output should contain the OpenPGP encryption marker' | ||
| ); | ||
|
|
||
| Email.customTransport = undefined; | ||
| } | ||
| ); | ||
|
|
||
| Tinytest.addAsync( | ||
| 'email - openpgp - Email.sendAsync signs through the full stack', | ||
| async function (test) { | ||
| let capturedMessage = null; | ||
|
|
||
| Email.customTransport = async (options) => { | ||
| const transport = nodemailer.createTransport({ | ||
| streamTransport: true, | ||
| buffer: true, | ||
| newline: 'unix', | ||
| }); | ||
| if (options.encryptionKeys || options.shouldSign) { | ||
| transport.use( | ||
| 'stream', | ||
| openpgpEncrypt({ | ||
| signingKey: PRIVATE_KEY_2048, | ||
| passphrase: PASSPHRASE, | ||
| }) | ||
| ); | ||
| } | ||
| const info = await transport.sendMail(options); | ||
| capturedMessage = info.message.toString(); | ||
| }; | ||
|
|
||
| await Email.sendAsync({ | ||
| from: 'a@b.com', | ||
| to: 'c@d.com', | ||
| subject: 'Full stack sign', | ||
| text: 'Signed message', | ||
| shouldSign: true, | ||
| }); | ||
|
|
||
| test.isNotNull(capturedMessage, 'customTransport should have been called'); | ||
| test.isTrue( | ||
| capturedMessage.indexOf('multipart/signed') >= 0, | ||
| 'output should be multipart/signed' | ||
| ); | ||
| test.isTrue( | ||
| capturedMessage.indexOf( | ||
| 'Content-Description: OpenPGP signed message' | ||
| ) >= 0, | ||
| 'output should contain the OpenPGP signed message marker' | ||
| ); | ||
|
|
||
| Email.customTransport = undefined; | ||
| } | ||
| ); | ||
|
|
||
| Tinytest.addAsync( | ||
| 'email - openpgp - hooks work with encrypted messages', | ||
| async function (test) { | ||
| let hookReceivedOptions = null; | ||
| const hook = Email.hookSend((options) => { | ||
| hookReceivedOptions = options; | ||
| return true; | ||
| }); | ||
|
|
||
| Email.customTransport = async (options) => { | ||
| const transport = nodemailer.createTransport({ | ||
| streamTransport: true, | ||
| buffer: true, | ||
| newline: 'unix', | ||
| }); | ||
| if (options.encryptionKeys || options.shouldSign) { | ||
| transport.use( | ||
| 'stream', | ||
| openpgpEncrypt({ | ||
| signingKey: PRIVATE_KEY_2048, | ||
| passphrase: PASSPHRASE, | ||
| }) | ||
| ); | ||
| } | ||
| await transport.sendMail(options); | ||
| }; | ||
|
|
||
| await Email.sendAsync({ | ||
| from: 'a@b.com', | ||
| to: 'c@d.com', | ||
| text: 'Hook test', | ||
| encryptionKeys: [PUBLIC_KEY], | ||
| }); | ||
|
|
||
| test.isNotNull( | ||
| hookReceivedOptions, | ||
| 'hook should have been called with mail options' | ||
| ); | ||
| test.equal( | ||
| hookReceivedOptions.from, | ||
| 'a@b.com', | ||
| 'hook should receive the mail options' | ||
| ); | ||
|
|
||
| hook.stop(); | ||
| Email.customTransport = undefined; | ||
| } | ||
| ); | ||
|
|
||
| Tinytest.addAsync( | ||
| 'email - openpgp - encrypted output decrypts and verifies', | ||
| async function (test) { | ||
| const selfPubKey = await SELF_PUBLIC_KEY; | ||
| let capturedMessage = null; | ||
|
|
||
| Email.customTransport = async (options) => { | ||
| const transport = nodemailer.createTransport({ | ||
| streamTransport: true, | ||
| buffer: true, | ||
| newline: 'unix', | ||
| }); | ||
| transport.use( | ||
| 'stream', | ||
| openpgpEncrypt({ | ||
| signingKey: PRIVATE_KEY_2048, | ||
| passphrase: PASSPHRASE, | ||
| }) | ||
| ); | ||
| const info = await transport.sendMail(options); | ||
| capturedMessage = info.message.toString(); | ||
| }; | ||
|
|
||
| const secretText = `Secret round-trip ${Date.now()}`; | ||
|
|
||
| await Email.sendAsync({ | ||
| from: 'a@b.com', | ||
| to: 'c@d.com', | ||
| subject: 'Round trip', | ||
| text: secretText, | ||
| encryptionKeys: [selfPubKey], | ||
| }); | ||
|
|
||
| // Extract the PGP armored block from the MIME output | ||
| const beginMarker = '-----BEGIN PGP MESSAGE-----'; | ||
| const endMarker = '-----END PGP MESSAGE-----'; | ||
| const beginIdx = capturedMessage.indexOf(beginMarker); | ||
| const endIdx = capturedMessage.indexOf(endMarker, beginIdx); | ||
|
|
||
| test.isTrue( | ||
| beginIdx >= 0, | ||
| 'MIME output should contain a PGP message armor block' | ||
| ); | ||
|
|
||
| const armoredMessage = capturedMessage.substring( | ||
| beginIdx, | ||
| endIdx + endMarker.length | ||
| ); | ||
|
|
||
| // Decrypt and verify | ||
| const privKey = await openpgp.readPrivateKey({ | ||
| armoredKey: PRIVATE_KEY_2048, | ||
| }); | ||
| const decryptedKey = await openpgp.decryptKey({ | ||
| privateKey: privKey, | ||
| passphrase: PASSPHRASE, | ||
| }); | ||
| const pubKey = await openpgp.readKey({ | ||
| armoredKey: selfPubKey, | ||
| }); | ||
|
|
||
| const message = await openpgp.readMessage({ | ||
| armoredMessage, | ||
| }); | ||
| const { data, signatures } = await openpgp.decrypt({ | ||
| message, | ||
| decryptionKeys: decryptedKey, | ||
| verificationKeys: pubKey, | ||
| }); | ||
|
|
||
| test.isTrue( | ||
| data.includes(secretText), | ||
| 'decrypted data should contain the original secret text' | ||
| ); | ||
| test.isTrue( | ||
| signatures.length > 0, | ||
| 'should have at least one signature after decryption' | ||
| ); | ||
|
|
||
| const sigVerified = await signatures[0].verified; | ||
| test.isTrue( | ||
| sigVerified === true, | ||
| 'signature should verify against the signing key' | ||
| ); | ||
|
|
||
| Email.customTransport = undefined; | ||
| } | ||
| ); |
There was a problem hiding this comment.
Restore Email.customTransport and hooks in finally.
These tests mutate process-wide state and only clean it up on the happy path. If Email.sendAsync, transport.sendMail, or an assertion throws before the reset, later cases inherit the custom transport or hook and fail for the wrong reason. Wrapping each override in try/finally would keep the suite isolated.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/email/email_openpgp_tests.js` around lines 207 - 439, The tests set
global state (Email.customTransport and Email.hookSend) but only reset them on
the success path; wrap each test's override in a try/finally so
Email.customTransport is restored and any hook is stopped in the finally block.
Specifically, for tests that assign Email.customTransport, ensure a try { ...
await Email.sendAsync(...) ... assertions ... } finally { Email.customTransport
= undefined; } and for the test that uses const hook = Email.hookSend(...)
ensure the finally calls hook.stop() before clearing Email.customTransport to
guarantee cleanup even if sendAsync, transport.sendMail, or assertions throw.
|
@italojs @nachocodoner Not trying to toot my own horn but it's important to have a built-in alarm system instead of relying on the community to report these findings! |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/email/openpgp-encrypt.js`:
- Around line 75-79: The code converts MIME byte buffers to UTF-8 strings which
corrupts 8bit/binary content; update the three sites (the async _flush method
that calls this._processMessage, and the places that call createMessage with a
text field) to pass raw bytes as binary Uint8Array instead of strings: stop
using raw.toString() and instead use the Buffer/Uint8Array directly and call
openpgp.createMessage({ binary: <Uint8Array> }) so the exact byte sequences are
preserved for signing/encryption; adjust any downstream helpers (e.g.,
_processMessage and the createMessage callers at the two other locations) to
accept and forward binary Uint8Array rather than text.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a49aff01-8f53-49c4-b495-419fcd80f13d
📒 Files selected for processing (2)
packages/email/email.jspackages/email/openpgp-encrypt.js
| async _flush(done) { | ||
| try { | ||
| const raw = Buffer.concat(this._chunks, this._length); | ||
| const result = await this._processMessage(raw.toString()); | ||
| this.push(Buffer.from(result)); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In OpenPGP.js v5, what is the supported way to sign or encrypt an exact MIME byte sequence without converting it to a UTF-8 string first? Specifically, does createMessage({ binary: Uint8Array })preserve bytes for detachedsign(...)andencrypt(...)?
💡 Result:
Yes—OpenPGP.js v5 supports signing/encrypting raw bytes without converting them to a UTF-8 string first, by creating the message from a Uint8Array/ReadableStream via createMessage({ binary }), and then using sign(...)/encrypt(...) with binary output. 1) Supported API for raw bytes - The v5 changelog shows createMessage({ binary: data }) as the way to create a binary message, and shows encrypt(..., format: 'binary') producing a Uint8Array encrypted result [1]. - The JSDoc for Message describes that signDetached signs “the literal data packet of the message” (i.e., it signs the message’s literal data content, not a UTF-8 string conversion step) [2]. 2) Detached sign: createMessage({ binary: ... }) is the correct input - The official docs examples cover detached signatures and show the message parameter is a Message object created via createMessage({ text: ... }) in the example [3]. While that example uses text, the same mechanism applies: the literal data that gets signed comes from the Message object you pass, and createMessage({ binary: ... }) is the supported way to populate that literal data with exact bytes [2][4]. 3) Encrypt: bytes are preserved through the Message pipeline - The v5 changelog explicitly demonstrates encrypting a binary Message created from Uint8Array via createMessage({ binary: data }) and obtaining binary output (format: 'binary') [1]. 4) Important note about MIME “exact byte sequence” OpenPGP.js signs/encrypts the bytes you put into the literal data packet. It does not automatically understand MIME boundaries/headers as “canonical bytes”; so if you require a specific exact MIME byte sequence to be signed, you must ensure that the Uint8Array you pass into createMessage({ binary }) is exactly that sequence (including any CRLF normalization you require). OpenPGP.js will not redo MIME canonicalization for you; it will just treat your provided bytes as the literal data [2]. Concretely, for an exact byte sequence: - Prepare your MIME bytes yourself (e.g., in a Uint8Array). - Create the OpenPGP message with createMessage({ binary: mimeBytes }). - Use sign({ message, signingKeys, detached: true, format: 'binary' }) for detached signatures (binary signature output) and encrypt({ message, encryptionKeys, signingKeys, format: 'binary' }) for binary encrypted output. Bottom line: createMessage({ binary: Uint8Array }) is the supported way in OpenPGP.js v5 to sign/encrypt exact bytes without converting to UTF-8 first, and yes, detached sign(...) and encrypt(...) operate on the bytes contained in that Message object [1][2].
Citations:
- 1: https://github.com/openpgpjs/openpgpjs/releases/tag/v5.0.0-4
- 2: https://docs.openpgpjs.org/Message.html
- 3: https://docs.openpgpjs.org/
- 4: https://github.com/openpgpjs/openpgpjs/wiki/V5-Changelog
🏁 Script executed:
find . -path "*/packages/email/openpgp-encrypt.js" -type fRepository: meteor/meteor
Length of output: 93
🏁 Script executed:
cat -n packages/email/openpgp-encrypt.js | head -200Repository: meteor/meteor
Length of output: 7221
🏁 Script executed:
cat -n packages/email/openpgp-encrypt.js | sed -n '70,85p'Repository: meteor/meteor
Length of output: 505
🏁 Script executed:
cat -n packages/email/openpgp-encrypt.js | sed -n '128,145p'Repository: meteor/meteor
Length of output: 756
🏁 Script executed:
cat -n packages/email/openpgp-encrypt.js | sed -n '167,185p'Repository: meteor/meteor
Length of output: 808
Use OpenPGP.js binary APIs to preserve exact byte sequences in MIME bodies.
Converting to UTF-8 string (raw.toString() at line 78, then createMessage({ text: ... }) at lines 133 and 172) corrupts 8bit/binary encoded content before encryption and signing. OpenPGP.js v5 supports createMessage({ binary: Uint8Array }) to sign/encrypt exact byte sequences without string conversion—use that instead.
This pattern affects all three locations: lines 75–79, 133–138, and 172–179.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/email/openpgp-encrypt.js` around lines 75 - 79, The code converts
MIME byte buffers to UTF-8 strings which corrupts 8bit/binary content; update
the three sites (the async _flush method that calls this._processMessage, and
the places that call createMessage with a text field) to pass raw bytes as
binary Uint8Array instead of strings: stop using raw.toString() and instead use
the Buffer/Uint8Array directly and call openpgp.createMessage({ binary:
<Uint8Array> }) so the exact byte sequences are preserved for
signing/encryption; adjust any downstream helpers (e.g., _processMessage and the
createMessage callers at the two other locations) to accept and forward binary
Uint8Array rather than text.
| const signingKey = mail.data.signingKey || (options && options.signingKey); | ||
| const passphrase = mail.data.passphrase || (options && options.passphrase); | ||
| const hasSigningKey = !!signingKey; | ||
| const mailKeys = mail.data.encryptionKeys; | ||
| const hasMailKeys = | ||
| Array.isArray(mailKeys) && mailKeys.length > 0; | ||
| const wantsSign = hasSigningKey && mail.data.shouldSign !== false; | ||
|
|
||
| if (!hasMailKeys && !wantsSign) { | ||
| return setImmediate(done); | ||
| } | ||
|
|
||
| mail.message.transform(() => { | ||
| return new PGPMimeTransform({ | ||
| signingKey, | ||
| passphrase, | ||
| encryptionKeys: mail.data.encryptionKeys, | ||
| shouldSign: mail.data.shouldSign, | ||
| }); |
There was a problem hiding this comment.
Reject invalid OpenPGP requests instead of sending plaintext.
If callers set shouldSign: true without a signing key, or pass encryptionKeys as an empty/non-array value, this path currently falls through to an unprotected send. In the sign-only path, a truthy non-array encryptionKeys also reaches this._encryptionKeys.map(...) and fails later. Validate and normalize the request before the early return.
Proposed fix
export function openpgpEncrypt(options) {
return function handleStream(mail, done) {
const signingKey = mail.data.signingKey || (options && options.signingKey);
const passphrase = mail.data.passphrase || (options && options.passphrase);
const hasSigningKey = !!signingKey;
- const mailKeys = mail.data.encryptionKeys;
- const hasMailKeys =
- Array.isArray(mailKeys) && mailKeys.length > 0;
+ const rawMailKeys = mail.data.encryptionKeys;
+ const mailKeys = Array.isArray(rawMailKeys)
+ ? rawMailKeys
+ : rawMailKeys ? [rawMailKeys] : [];
+ const hasMailKeys = mailKeys.length > 0;
const wantsSign = hasSigningKey && mail.data.shouldSign !== false;
+
+ if (mail.data.shouldSign === true && !hasSigningKey) {
+ return setImmediate(() =>
+ done(new Error('OpenPGP signing requested but no signingKey was provided'))
+ );
+ }
+
+ if (rawMailKeys !== undefined && !hasMailKeys) {
+ return setImmediate(() =>
+ done(new Error('OpenPGP encryption requested but no encryptionKeys were provided'))
+ );
+ }
if (!hasMailKeys && !wantsSign) {
return setImmediate(done);
}
mail.message.transform(() => {
return new PGPMimeTransform({
signingKey,
passphrase,
- encryptionKeys: mail.data.encryptionKeys,
+ encryptionKeys: mailKeys,
shouldSign: mail.data.shouldSign,
});
});As per coding guidelines, "This is a core Meteor Atmosphere package. Focus on API backwards compatibility, DDP/reactivity correctness, and client/server split. Avoid nitpicking style — the codebase has legacy patterns."
|
Hi @julio-rocketchat, I'll do a closer review tomorrow( I think all looks fine) and ask @nachocodoner to take a look too – I think we can release a version of this package for your codebase as soon as we get the review right. |
There was a problem hiding this comment.
This is just a copy eh? I think we can skip the review for this file
There was a problem hiding this comment.
It's not an exact copy of nodemailer-openpgp, it's more of a reimplementation and there are some changes/improvements to it. In terms of behavior, it's pretty much the same as nodemailer-openpgp tho. A review would definitely be appreciated
There was a problem hiding this comment.
I was actually post yesterday but with the downtime on GitHub I was not able to.
openpgp-encrypt.js seems like a clean reimplementation of nodemailer-openpgp, not a copy (different class name, helpers split out, dynamic micalg). The PR body explains the reason too: upstream hasn't been touched in 2 years and pins a vulnerable openpgp.
May we add a small note about that in the file header? Just so future maintainers don't have to dig into the PR to figure out why we're carrying this in-tree.
There was a problem hiding this comment.
Added the header with more information to openpgp-encrypt.js.
There was a problem hiding this comment.
@nachocodoner @Grubba27 anything else you guys would need in terms of changes?
There was a problem hiding this comment.
I was actually post yesterday but with the downtime on GitHub I was not able to.
openpgp-encrypt.js seems like a clean reimplementation of nodemailer-openpgp, not a copy (different class name, helpers split out, dynamic micalg). The PR body explains the reason too: upstream hasn't been touched in 2 years and pins a vulnerable openpgp.
May we add a small note about that in the file header? Just so future maintainers don't have to dig into the PR to figure out why we're carrying this in-tree.
| postcss: '8.5.1', | ||
| cssnano: '5.1.15' | ||
| postcss: '8.5.13', | ||
| cssnano: '7.1.9', |
There was a problem hiding this comment.
Since cssnano is only used internally by minifier-css and Meteor does not expose cssnano YAML config to users, the cssnano 5 => 7 config-file breaking change likely does not affect users. ✅
postcss jsut bumped patch version so we don't need to worry about. ✅
Definitely. We already have Dependabot, but that mostly covers npm dependencies rather than Atmosphere packages or Meteor tool dependencies: Meteor security advisories. We still need more signals around Atmosphere packages themselves. Would anybody like to work on a CI check that, using either AI or a deterministic script, scans Atmosphere packages and Meteor tool and generates security audits/reports? This is something we would like to evaluate and bring closer to the core over time. ✅ Of course, signals alone are not enough, we also need time and people to act on them. I started some time ago from the Meteor tool level: #13814 and #14247. Dependabot alerts only grow over time, and while some can be fixed with simple version bumps, others introduce breaking changes and additional risks. Contributions from the community and companies using Meteor in production are always welcome. 🙏 |
nachocodoner
added
the
Next 3.x.x
4 participants
At Rocket.Chat, we have users that have a zero critical/high-severity CVE policy and we're working on updating as many dependencies as we can to fix such issues. Some of the vulnerable packages that come up in SCA scans (by using Trivy, OWASP's Dependency Check, and the like) are Meteor packages or transitive dependencies of Meteor packages. This PR's intention is fixing some of the issues, as outlined below:
minifier-css: Bumped
postcssandcssnanoin order to bump the transitivesvgodependency and fix CVE-2026-29074email: Bumped
nodemailerfrom 6.9.10 to 7.0.11 to fix CVE-2025-14874 and other medium-severity CVEs. The new version generates headers in a different order, so existing test expectations were updated - this should be safe since email headers are parsed by name, not position.email: Replaced
nodemailer-openpgpwith a built-in implementation (openpgp-encrypt.js). The original library hasn't been updated in 2 years and pins a vulnerable version ofopenpgp. Since it's a small library (around 250 LOC, I think), it was reimplemented directly inside the email package withopenpgp@5.11.3as a direct dependency instead. In my opinion, it's better than forking the library and using the fork. The new implementation preserves the same plugin API and features, with a few improvements.email: Added new tests covering direct stream encryption, Nodemailer plugin integration, sign-only mode, sign opt-out, weak key rejection, full-stack encryption and signing via
Email.sendAsync, hook interaction with encrypted messages, and a round-trip encrypt → decrypt → verify test.Summary by CodeRabbit
New Features
Tests
Chores