[clamav] Add plugin for ClamAV - #4429
Conversation
|
Congratulations! One of the builds has completed. 🍾 You can install the built RPMs by following these steps:
Please note that the RPMs should be used only in a testing environment. |
1 similar comment
|
Congratulations! One of the builds has completed. 🍾 You can install the built RPMs by following these steps:
Please note that the RPMs should be used only in a testing environment. |
TurboTurtle
left a comment
There was a problem hiding this comment.
Minor notes. I'm going to pause on reviewing the other many new plugin PRs for the moment, as several of them seem to follow the same pattern of specifying units for journal and service status collection without defining the services tuple.
I also need to ask, were these PRs AI-assisted? It's fine if they were, but this project requests that that fact be part of the commit message with an Assisted-By line, as noted in our AI Policy
| self.add_service_status([ | ||
| 'clamav-daemon', | ||
| 'clamav-freshclam', | ||
| 'clamd@scan', | ||
| 'freshclam', | ||
| ]) | ||
|
|
||
| self.add_journal(units=[ | ||
| 'clamav-daemon', | ||
| 'clamav-freshclam', | ||
| 'clamd@*', | ||
| 'freshclam', | ||
| ]) |
There was a problem hiding this comment.
If you add these items to the services tuple as part of the class definition, the journals and service statuses are collected automatically, and serve as an additional enable trigger.
There was a problem hiding this comment.
Thanks for the review, and for pausing rather than working through them one at
a time — that was the right call, there were too many at once.
For context on where they came from: I've spent about eight years in enterprise
support — Red Hat , then AWS, now Cloudera — working escalations across
RHEL, clustering and cloud infrastructure. The plugins came out of services I
have repeatedly had no data for when working case
There was a problem hiding this comment.
Ok, but that doesn't answer the question. The replies throughout these PRs are obviously AI generated, as are the commit messages. The sheer volume of changes coming in also supports this.
We do not mind accepting AI-assisted changes, but it needs to be noted as described in policy. Additionally, we expect non-code interactions, such as discussions, to be with humans.
There was a problem hiding this comment.
Hi @TurboTurtle ,
Thanks for calling this out. I want to clarify that the PRs and the discussions/comments from my side are written by me. I have been working on these changes myself and interacting on the PRs directly.
I do use AI tools occasionally as an assistance tool for things like wording or reviewing, but the technical investigation, changes, testing, and decisions behind the PRs are mine. I also understand and respect the project policy around disclosing AI assistance, and I’ll make sure to follow that going forward where applicable.
Regarding the concern about non-code interactions, I’m happy to continue the discussions directly and address any technical questions myself.
I appreciate you raising this and I’m happy to clarify anything further.
|
|
||
| # The signature database is large and binary; a listing is enough | ||
| # to show which definitions are present and how old they are. | ||
| self.add_dir_listing('/var/lib/clamav', tags='clamav_database') |
There was a problem hiding this comment.
The DB location is configurable. Is it worth sourcing the directory from the config file?
There was a problem hiding this comment.
Yes, agreed. DatabaseDirectory in freshclam.conf defaults to /var/lib/clamav
but gets moved often enough that a hardcoded path would miss it exactly when
someone is debugging stale signatures.
ClamAV is packaged for Fedora, RHEL via EPEL, Debian and Ubuntu, but sos has no plugin for it and no other plugin references it. Neither the daemon configuration, the freshclam update state nor the signature database age reaches an sosreport. The configuration layout differs by distribution: Red Hat splits the daemon configuration into /etc/clamd.d/ while Debian keeps everything under /etc/clamav/, so both are collected. Defaults are taken from the upstream samples in etc/, which set DatabaseDirectory to /var/lib/clamav and UpdateLogFile to /var/log/freshclam.log. clamconf is ClamAV's own configuration dump utility and is run in preference to parsing the files directly. The signature database is not copied. It is hundreds of megabytes of binary .cvd data; a directory listing still shows which definitions are present and how old they are. etc/freshclam.conf.sample documents HTTPProxyUsername and HTTPProxyPassword for outbound proxy access, so postproc() redacts both. HTTPProxyServer is left intact. Example of the substitution: HTTPProxyPassword mypass -> HTTPProxyPassword ******** Signed-off-by: Suraj Patil <surajpatil522@gmail.com>
42c9ddf to
62e2863
Compare
cron.py covers cron, but nothing in the tree references atd, /etc/at.allow, /etc/at.deny, /var/spool/at or atq. An sosreport from a host using at contains no record of the daemon, its access control files or its pending queue. The plugin collects the allow and deny lists, the distribution defaults files, atq output, and the service status and journal for atd. Queued jobs under /var/spool/at are not copied. Each is a complete shell script that embeds the submitting user's environment, which frequently includes exported credentials. A recursive directory listing is taken instead, which still shows the pending jobs, their queue, ownership and submission time. The units are declared in the services tuple rather than explicit add_service_status() and add_journal() calls, per review on sosreport#4429. Signed-off-by: Suraj Patil <surajpatil522@gmail.com>
TurboTurtle
left a comment
There was a problem hiding this comment.
Code looks ok now.
Please add an Assisted-by line to the commit message when using an AI assistant as per our AI policy.
ClamAV is packaged for Fedora, RHEL via EPEL, Debian and Ubuntu, but sos has no
plugin for it and no other plugin references it. Neither the daemon
configuration, the freshclam update state nor the signature database age
reaches an sosreport.
The configuration layout differs by distribution: Red Hat splits the daemon
configuration into
/etc/clamd.d/while Debian keeps everything under/etc/clamav/, so both are collected. Defaults are taken from the upstreamsamples in
etc/, which setDatabaseDirectoryto/var/lib/clamavandUpdateLogFileto/var/log/freshclam.log.clamconfis ClamAV's own configuration dump utility and is run in preferenceto parsing the files directly.
The signature database is not copied. It is hundreds of megabytes of binary
.cvddata; a directory listing still shows which definitions are present andhow old they are.
etc/freshclam.conf.sampledocumentsHTTPProxyUsernameandHTTPProxyPasswordfor outbound proxy access, sopostproc()redacts both.HTTPProxyServeris left intact.Example of the substitution:
Paths and defaults are taken from the upstream configuration samples rather
than a running system. The
postproc()regex was tested against sample lines.Unit names vary between distributions — I have covered
clamav-daemon,clamav-freshclam,clamd@scanandfreshclam; confirmation of the packagednames would be welcome.
Please place an 'X' inside each '[]' to confirm you adhere to our Contributor Guidelines