Skip to content

Conversation

@tommairs
Copy link
Collaborator

This adds functionality to use Hornetsecurity's spam and malware protection as documented here.

@tommairs tommairs requested a review from wez October 31, 2025 05:05
-- IE: {['port']=800,['timeout']=30,['use_tls']=true,['maxsize']=2097152}
-- returns: array of hornet server connection values, AKA "hornet connection array"
--
function mod:connect(vhost, vparams)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This module functions should be functions rather than methods, because they don't store any state in "self":

Suggested change
function mod:connect(vhost, vparams)
function mod.connect(vhost, vparams)

Having just done a pass through the docs, I can see that they have sections like this:

local hornethost =
    hornet:connect('172.31.17.26', { port = '8080', use_tls = 'false' })

  if not hornethost then
    print 'No connection to Hornetsecurity host'
  else

the if not hornesthost is impossible to trigger because the connect method unconditionally just returns an object, so we should update the docs to simplify things by removing the if bit.

Since this function isn't actually connecting to anything, I would advocate rolling it into the scan function below, making things simpler overall

print 'No connection table provided'
return false
end

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would collapse the mod:connect function here like this:

Suggested change
local params = {
host = params.host or '127.0.0.1',
port = params.port or 8080,
timeout = params.timeout or 10,
use_tls = params.use_tls or false,
}

local msgdata = msg:get_data()

-- decompose msg here into vars to pass
local modifymsg = extraparams.addheaders or false -- Add result headers directly to the message?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd also collapse extraparams into params just to keep things simpler with a single table style parameter to the scan function.


-- catch usecase if a raw number is provided for API timeout
if type(duration) == 'number' then
duration = '' .. tostring(duration) .. 's'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
duration = '' .. tostring(duration) .. 's'
duration = tostring(duration) .. 's'

Comment on lines +145 to +169
local disposition = string.format(
'%d %s: %s',
response:status_code(),
response:status_reason(),
response:text()
)

if string.sub(disposition, 1, 8) == '200 OK: ' then
scanresult = disposition
scanresult = string.sub(scanresult, 9, string.len(scanresult))
else
scanresult = disposition
print 'Security scan failed'
end

-- If requested, add headers to message to describe the scan results
if modifymsg == true then
scanresult_json = kumo.serde.json_parse(scanresult)
for skey, svalue in pairs(scanresult_json) do
skey = 'X-Hornet-' .. skey
msg:append_header(skey, svalue)
end
end

return scanresult
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no need to form an SMTP style disposition just to crack it apart again; you can directly use the response information.

Unconditionally printing status in these helpers for per-message actions will generate a lot of noise in the diagnostic logs, so we should avoid that. In addition, a print for an error with no error being propagated out is a silent error until someone looks at the diagnostic logs, and should be avoided as well.

I'd suggest phrasing the logic this way, which causes failures to talk to the API to bubble up as 421 technical difficulties errors in the incoming session. There are other ways to handle this, but we should probably have a design session to talk about the higher level plans for this sort of integration.

Suggested change
local disposition = string.format(
'%d %s: %s',
response:status_code(),
response:status_reason(),
response:text()
)
if string.sub(disposition, 1, 8) == '200 OK: ' then
scanresult = disposition
scanresult = string.sub(scanresult, 9, string.len(scanresult))
else
scanresult = disposition
print 'Security scan failed'
end
-- If requested, add headers to message to describe the scan results
if modifymsg == true then
scanresult_json = kumo.serde.json_parse(scanresult)
for skey, svalue in pairs(scanresult_json) do
skey = 'X-Hornet-' .. skey
msg:append_header(skey, svalue)
end
end
return scanresult
local scanresult = response:text()
if response:status_code() ~= 200 then
-- this will produce a 421 technical difficulties response
error('failed to query hornetsecurity: ' .. scanresult)
end
-- invalid json will produce a 421 technical difficulties response
scanresult_json = kumo.serde.json_parse(scanresult)
if modifymsg then
for skey, svalue in pairs(scanresult_json) do
msg:append_header('X-Hornet-' .. skey, svalue)
end
end
return scanresult_json

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants