helo.checks: only skip subsequent checks if success (#3352)

- helo: emit log entry after all other hooks
- clean up stray config files from repackaged plugins

Changes.md

@@ -21,6 +21,7 @@
 - deps: bump all versions to latest #3303, #3344
 - dkim: repackaged as NPM module #3311
 - greylist: repackaged as NPM module
+- helo.checks: require a successful HELO/EHLO #3352
 - new NPM plugin dns-list, repackages dnsbl, dnswl, backscatterer #3313
 - when using message-stream, don't send default options #3290
 - rcpt_to.host_list: add connection ID to log messages #3322

plugins/helo.checks.js

@@ -36,17 +36,17 @@ exports.register = function () {
         this.register_hook('ehlo', c);
     }
 
-    // Always emit a log entry
-    this.register_hook('helo', 'emit_log');
-    this.register_hook('ehlo', 'emit_log');
-
     // IP literal that doesn't match remote IP
     this.register_hook('helo', 'literal_mismatch');
     this.register_hook('ehlo', 'literal_mismatch');
 
     this.cfg.check.literal_mismatch = this.cfg.check.literal_mismatch ?? 2;
     this.cfg.reject.literal_mismatch = this.cfg.reject.literal_mismatch ?? false;
 
+    // Always emit a log entry
+    this.register_hook('helo', 'emit_log');
+    this.register_hook('ehlo', 'emit_log');
+
     if (this.cfg.check.match_re) {
         const load_re_file = () => {
             const regex_list = utils.valid_regexes(this.config.get('helo.checks.regexps', 'list', load_re_file));
@@ -110,7 +110,7 @@ exports.init = function (next, connection, helo) {
 }
 
 exports.should_skip = function (connection, test_name) {
-    if (connection.results.has('helo.checks', '_skip_hooks', test_name)) {
+    if (connection.hello.host && connection.results.has('helo.checks', '_skip_hooks', test_name)) {
         this.logdebug(connection, `SKIPPING: ${test_name}`);
         return true;
     }

test/plugins/helo.checks.js

@@ -301,8 +301,12 @@ describe('helo.checks', () => {
             this.plugin.cfg.reject.big_company=true;
             this.plugin.big_company((rc) => {
                 assert.equal(rc, DENY);
-                assert.ok(this.connection.results.get('helo.checks').fail.length);
-                done();
+                assert.equal(this.connection.results.get('helo.checks').fail.length, 1);
+                this.plugin.big_company((rc) => {
+                    assert.equal(rc, DENY);
+                    assert.ok(this.connection.results.get('helo.checks').fail.length, 2);
+                    done();
+                }, this.connection, test_helo);
             }, this.connection, test_helo);
         })
     })