haraka-plugin-rspamd - npm Package Compare versions

Comparing version 1.1.0 to 1.1.1

Changes.md

@@ -1,3 +0,12 @@
-# 1.1.0 - 2018-01-12
 
+## 1.1.1 - 2018-05-10
+
+- pass TLS-Cipher and TLS-Version headers to rspamd (fixes #4)
+- code smell: return cleanups
+- es6: use arrow functions
+- refactored hook_data_post, addressing excessive cognitive complexity
+
+
+## 1.1.0 - 2018-01-12
+
 - use /checkv2 endpoint (requires rspamd 1.6+)
@@ -7,4 +16,5 @@ - support setting SMTP message from rspamd
 
-# 1.0.0 - 201_-__-__
+ 
+## 1.0.0 - 201_-__-__
 
 - initial release

index.js

@@ -11,3 +11,3 @@ 'use strict';
     this.load_rspamd_ini();
-};
+}
 
@@ -29,3 +29,3 @@ exports.load_rspamd_ini = function () {
         ],
-    }, function () {
+    }, () => {
         plugin.load_rspamd_ini();
@@ -52,3 +52,4 @@ });
             plugin.cfg.main.add_headers = 'always';
-        } else {
+        }
+        else {
             plugin.cfg.main.add_headers = 'sometimes';
@@ -61,3 +62,3 @@ }
     }
-};
+}
 
@@ -129,17 +130,68 @@ exports.get_options = function (connection) {
 
+    if (connection.tls.enabled) {
+        options.headers['TLS-Cipher'] = connection.tls.cipher.name;
+        options.headers['TLS-Version'] = connection.tls.cipher.version;
+    }
+
     return options;
-};
+}
 
-exports.hook_data_post = function (next, connection) {
-    if (!connection.transaction) return next();
+exports.get_smtp_message = function (r) {
+    const plugin = this;
 
+    if (!plugin.cfg.smtp_message.enabled || !r.data.messages) return;
+    if (typeof(r.data.messages) !== 'object') return;
+    if (!r.data.messages.smtp_message) return;
+
+    return r.data.messages.smtp_message;
+}
+
+exports.do_rewrite = function (connection, data) {
     const plugin = this;
-    const cfg = plugin.cfg;
 
-    const authed = connection.notes.auth_user;
-    if (authed && !cfg.check.authenticated) return next();
-    if (!cfg.check.private_ip && connection.remote.is_private) {
-        return next();
+    if (!plugin.cfg.rewrite_subject.enabled) return false;
+    if (data.action !== 'rewrite subject') return false;
+
+    const rspamd_subject = data.subject || plugin.cfg.subject;
+    const old_subject = connection.transaction.header.get('Subject') || '';
+    const new_subject = rspamd_subject.replace('%s', old_subject);
+
+    connection.transaction.remove_header('Subject');
+    connection.transaction.add_header('Subject', new_subject);
+}
+
+exports.do_dkim = function (connection, data) {
+    const plugin = this;
+
+    if (!plugin.cfg.dkim.enabled) return;
+    if (!data['dkim-signature']) return;
+
+    connection.transaction.add_header('DKIM-Signature', data['dkim-signature']);
+}
+
+exports.do_milter_headers = function (connection, data) {
+    const plugin = this;
+
+    if (!plugin.cfg.rmilter_headers.enabled) return;
+    if (!data.milter) return;
+
+    if (data.milter.remove_headers) {
+        Object.keys(data.milter.remove_headers).forEach((key) => {
+            connection.transaction.remove_header(key);
+        })
     }
 
+    if (data.milter.add_headers) {
+        Object.keys(data.milter.add_headers).forEach((key) => {
+            connection.transaction.add_header(key, data.milter.add_headers[key]);
+        })
+    }
+}
+
+exports.hook_data_post = function (next, connection) {
+    const plugin = this;
+
+    if (!connection.transaction) return next();
+    if (plugin.wants_skip(connection)) return next();
+
     let timer;
@@ -149,3 +201,3 @@ const timeout = plugin.cfg.main.timeout || plugin.timeout - 1;
     let calledNext=false;
-    const callNext = function (code, msg) {
+    function nextOnce (code, msg) {
         clearTimeout(timer);
@@ -157,85 +209,77 @@ if (calledNext) return;
 
-    timer = setTimeout(function () {
+    timer = setTimeout(() => {
         if (!connection) return;
         if (!connection.transaction) return;
         connection.transaction.results.add(plugin, {err: 'timeout'});
-        callNext();
+        nextOnce();
     }, timeout * 1000);
 
-    const options = plugin.get_options(connection);
-
-    let req;
-    let rawData = '';
     const start = Date.now();
-    connection.transaction.message_stream.pipe(
-        req = http.request(options, function (res) {
-            res.on('data', function (chunk) { rawData += chunk; });
-            res.on('end', function () {
-                const r = plugin.parse_response(rawData, connection);
-                if (!r) return callNext();
-                if (!r.data) return callNext();
-                if (!r.log) return callNext();
 
-                r.log.emit = true; // spit out a log entry
-                r.log.time = (Date.now() - start)/1000;
+    const req = http.request(plugin.get_options(connection), (res) => {
+        let rawData = '';
 
-                if (!connection.transaction) return callNext();
-                connection.transaction.results.add(plugin, r.log);
+        res.on('data', (chunk) => { rawData += chunk; });
 
-                let smtp_message;
-                if (cfg.smtp_message.enabled && r.data.messages &&
-                  typeof(r.data.messages) == 'object' && r.data.messages.smtp_message) {
-                    smtp_message = r.data.messages.smtp_message;
-                }
+        res.on('end', () => {
+            const r = plugin.parse_response(rawData, connection);
+            if (!r || !r.data || !r.log) return nextOnce();
 
-                function no_reject () {
-                    if (cfg.dkim.enabled && r.data['dkim-signature']) {
-                        connection.transaction.add_header('DKIM-Signature', r.data['dkim-signature']);
-                    }
-                    if (cfg.rmilter_headers.enabled && r.data.milter) {
-                        if (r.data.milter.remove_headers) {
-                            Object.keys(r.data.milter.remove_headers).forEach(function (key) {
-                                connection.transaction.remove_header(key);
-                            })
-                        }
-                        if (r.data.milter.add_headers) {
-                            Object.keys(r.data.milter.add_headers).forEach(function (key) {
-                                connection.transaction.add_header(key, r.data.milter.add_headers[key]);
-                            })
-                        }
-                    }
-                    if (plugin.wants_headers_added(r.data)) {
-                        plugin.add_headers(connection, r.data);
-                    }
-                    return callNext();
-                }
+            r.log.emit = true; // spit out a log entry
+            r.log.time = (Date.now() - start)/1000;
 
-                if (cfg.rewrite_subject.enabled && r.data.action === 'rewrite subject') {
-                    const rspamd_subject = r.data.subject || cfg.subject;
-                    const old_subject = connection.transaction.header.get('Subject') || '';
-                    const new_subject = rspamd_subject.replace('%s', old_subject);
-                    connection.transaction.remove_header('Subject');
-                    connection.transaction.add_header('Subject', new_subject);
-                }
+            if (!connection.transaction) return nextOnce();
 
-                if (cfg.soft_reject.enabled && r.data.action === 'soft reject') {
-                    return callNext(DENYSOFT, DSN.sec_unauthorized(smtp_message || cfg.soft_reject.message, 451));
-                }
+            connection.transaction.results.add(plugin, r.log);
 
-                if (r.data.action !== 'reject') return no_reject();
-                if (!authed && !cfg.reject.spam) return no_reject();
-                if (authed && !cfg.reject.authenticated) return no_reject();
+            const smtp_message = plugin.get_smtp_message(r);
 
-                return callNext(DENY, smtp_message || cfg.reject.message);
-            });
-        })
-    );
+            plugin.do_rewrite(connection, r.data);
 
-    req.on('error', function (err) {
+            if (plugin.cfg.soft_reject.enabled && r.data.action === 'soft reject') {
+                nextOnce(DENYSOFT, DSN.sec_unauthorized(smtp_message || plugin.cfg.soft_reject.message, 451));
+            }
+            else if (plugin.wants_reject(connection, r.data)) {
+                nextOnce(DENY, smtp_message || plugin.cfg.reject.message);
+            }
+            else {
+                plugin.do_dkim(connection, r.data);
+                plugin.do_milter_headers(connection, r.data);
+                plugin.add_headers(connection, r.data);
+
+                nextOnce();
+            }
+        });
+    })
+
+    req.on('error', (err) => {
         if (!connection || !connection.transaction) return;
         connection.transaction.results.add(plugin, { err: err.message});
-        return callNext();
+        nextOnce();
     });
-};
 
+    connection.transaction.message_stream.pipe(req);
+    // pipe calls req.end() asynchronously
+}
+
+exports.wants_skip = function (connection) {
+    const plugin = this;
+
+    if (!plugin.cfg.check.authenticated && connection.notes.auth_user) return true;
+
+    if (!plugin.cfg.check.private_ip && connection.remote.is_private) return true;
+
+    return false;
+}
+
+exports.wants_reject = function (connection, data) {
+    const plugin = this;
+
+    if (data.action !== 'reject') return false;
+    if (!connection.notes.auth_user && !plugin.cfg.reject.spam) return false;
+    if (connection.notes.auth_user && !plugin.cfg.reject.authenticated) return false;
+
+    return true;
+}
+
 exports.wants_headers_added = function (rspamd_data) {
@@ -250,7 +294,60 @@ const plugin = this;
     return false;
-};
+}
 
+exports.get_clean = function (data, connection) {
+    const plugin = this;
+    const clean = { symbols: {} };
+
+    if (data.symbols) {
+        Object.keys(data.symbols).forEach(key => {
+            const a = data.symbols[key];
+            // transform { name: KEY, score: VAL } -> { KEY: VAL }
+            if (a.name && a.score !== undefined) {
+                clean.symbols[ a.name ] = a.score;
+                return;
+            }
+            // unhandled type
+            connection.logerror(plugin, a);
+        })
+    }
+
+    // objects that may exist
+    ['action', 'is_skipped', 'required_score', 'score'].forEach((key) => {
+        switch (typeof data[key]) {
+            case 'boolean':
+            case 'number':
+            case 'string':
+                clean[key] = data[key];
+                break;
+            default:
+                connection.loginfo(plugin, "skipping unhandled: " + typeof data[key]);
+        }
+    });
+
+    // arrays which might be present
+    ['urls', 'emails', 'messages'].forEach(b => {
+        // collapse to comma separated string, so values get logged
+        if (!data[b]) return;
+
+        if (data[b].length) {
+            clean[b] = data[b].join(',');
+            return;
+        }
+
+        if (typeof(data[b]) == 'object') {
+            // 'messages' is probably a dictionary
+            Object.keys(data[b]).map((k) => {
+                return `${k} : ${data[b][k]}`;
+            }).join(',');
+        }
+    });
+
+    return clean;
+}
+
 exports.parse_response = function (rawData, connection) {
     const plugin = this;
 
+    if (!rawData) return;
+
     let data;
@@ -267,2 +364,4 @@ try {
 
+    if (Object.keys(data).length === 0) return;
+
     if (Object.keys(data).length === 1 && data.error) {
@@ -275,48 +374,7 @@ connection.transaction.results.add(plugin, {
 
-    // make cleaned data for logs
-    const dataClean = {symbols: {}};
-    Object.keys(data.symbols).forEach(function (key) {
-        const a = data.symbols[key];
-        // transform { name: KEY, score: VAL } -> { KEY: VAL }
-        if (a.name && a.score !== undefined) {
-            dataClean.symbols[ a.name ] = a.score;
-        } else {
-            // unhandled type
-            connection.logerror(plugin, a);
-        }
-    });
-    const wantKeys = ["action", "is_skipped", "required_score", "score"];
-    wantKeys.forEach(function (key) {
-        const a = data[key];
-        switch (typeof a) {
-            case 'boolean':
-            case 'number':
-            case 'string':
-                dataClean[key] = a;
-                break;
-            default:
-                connection.loginfo(plugin, "skipping unhandled: " + typeof a);
-        }
-    });
-
-    // arrays which might be present
-    ['urls', 'emails', 'messages'].forEach(function (b) {
-        // collapse to comma separated string, so values get logged
-        if (data[b]) {
-            if (data[b].length) {
-                dataClean[b] = data[b].join(',');
-            } else if (typeof(data[b]) == 'object') {
-                // 'messages' is probably a dictionary
-                Object.keys(data[b]).map(function (k) {
-                    return k + " : " + data[b][k];
-                }).join(',');
-            }
-        }
-    });
-
     return {
         'data' : data,
-        'log' : dataClean,
+        'log' : plugin.get_clean(data, connection),
     };
-};
+}
 
@@ -327,2 +385,4 @@ exports.add_headers = function (connection, data) {
 
+    if (!plugin.wants_headers_added(data)) return;
+
     if (cfg.header && cfg.header.bar) {
@@ -364,2 +424,2 @@ let spamBar = '';
     }
-};
+}

package.json

 {
   "name": "haraka-plugin-rspamd",
-  "version": "1.1.0",
+  "version": "1.1.1",
   "description": "Haraka plugin for rspamd",
@@ -5,0 +5,0 @@ "main": "index.js",

test/index.js

@@ -6,4 +6,3 @@ 'use strict';
 
-const Connection   = fixtures.connection;
-const Transaction  = fixtures.transaction;
+const connection   = fixtures.connection;
 
@@ -14,7 +13,7 @@ const _set_up = function (done) {
     this.plugin.register();
-    this.connection = Connection.createConnection();
-    this.connection.transaction = Transaction.createTransaction();
+    this.connection = connection.createConnection();
+    this.connection.init_transaction();
 
     done();
-};
+}
 
@@ -35,3 +34,3 @@ exports.register = {
     },
-};
+}
 
@@ -46,3 +45,3 @@ exports.load_rspamd_ini = {
     },
-};
+}
 
@@ -76,2 +75,3 @@ exports.add_headers = {
         };
+        this.plugin.cfg.main.add_headers = 'always';
         this.plugin.add_headers(this.connection, test_data);
@@ -88,2 +88,3 @@ test.equal(this.connection.transaction.header.headers['X-Rspamd-Score'], '1.1');
         };
+        this.plugin.cfg.main.add_headers = 'always';
         this.plugin.add_headers(this.connection, test_data);
@@ -95,3 +96,3 @@ // console.log(this.connection.transaction.header);
     }
-};
+}
 
@@ -132,1 +133,22 @@ exports.wants_headers_added = {
 }
+
+exports.parse_response = {
+    setUp : _set_up,
+    'returns undef on empty string': function (test) {
+        test.expect(1);
+        // console.log(this.connection.transaction);
+        test.equal(
+            this.plugin.parse_response('', this.connection),
+            undefined
+        );
+        test.done();
+    },
+    'returns undef on empty object': function (test) {
+        test.expect(1);
+        test.equal(
+            this.plugin.parse_response('{}', this.connection),
+            undefined
+        );
+        test.done();
+    },
+}

Fixed alerts

New author

Supply chain risk

A new npm collaborator published a version of the package for the first time. New collaborators are usually benign additions to a project, but do indicate a change to the security surface area of a package.

Found 1 instance in 1 package

Improved metrics

Total package byte prevSize

26334

increased by5.37%

Lines of code

505

increased by10.99%

Number of medium supply chain risk alerts

1

decreased by-50%

No dependency changes