Handle unrecognized probes and detectors in report digest generation#1663
Handle unrecognized probes and detectors in report digest generation#1663precognitivem0nk wants to merge 1 commit intoNVIDIA:mainfrom
Conversation
Signed-off-by: precognitivem0nk <rextedgorman@gmail.com>
|
DCO Assistant Lite bot: I have read the DCO Document and I hereby sign the DCO You can retrigger this bot by commenting recheck in this Pull Request |
|
I have read the DCO Document and I hereby sign the DCO |
jmartin-tech
left a comment
jmartin-tech
left a comment
There was a problem hiding this comment.
Thank you for taking a look at this.
I would suggest that digest_report is not meant to be used in with reports from different versions at this time. The actual format for report.jsonl files is evolving rapidly between releases. While guards and warning are useful if the report cannot be processed accurately for a report from an older version the utility should simply report that it is not compatible and refuse to produce an html file. Creating an html report that results an incomplete or inaccurate view is generally discouraged.
In short I think this PR should likely redirect some, instead of suppressing the errors it should produce more clear indicators that the errors mean the report is not compatible wit the version being utilized. This may also benefit from adding a version compatibility check early in the process, the version check may need to be very strict until the project reaches a 1.0.0 release.
Since the methods impacted are used both during initial run digesting of the report and for post processing when utilizing as a utility this needs to be accounted for carefully as well.
@leondz may want to weigh in on this was well since he filed the original issue.
|
Thanks for the feedback, that’s a good point. I hadn’t considered that a
partial report could actually be worse than failing outright, especially
for a security tool. Makes total sense.
I’m happy to rework this. Would the right approach be to check the garak
version from the report’s init entry against the current version and bail
out early if they don’t match? Or would it be better to only flag it when
an unknown plugin comes up, but with a clear incompatibility error instead
of generating partial HTML?
…-Ram
On Wed, Apr 1, 2026 at 6:33 AM Jeffrey Martin ***@***.***> wrote:
***@***.**** requested changes on this pull request.
Thank you for taking a look at this.
I would suggest that digest_report is not meant to be used in with
reports from different versions at this time. The actual format for
report.jsonl files is evolving rapidly between releases. While guards and
warning are useful if the report cannot be processed accurately for a
report from an older version the utility should simply report that it is
not compatible and refuse to produce an html file. Creating an html report
that results an incomplete or inaccurate view is generally discouraged.
In short I think this PR should likely redirect some, instead of
suppressing the errors it should produce more clear indicators that the
errors mean the report is not compatible wit the version being utilized.
This may also benefit from adding a version compatibility check early in
the process, the version check may need to be very strict until the project
reaches a 1.0.0 release.
Since the methods impacted are used both during initial run digesting of
the report and for post processing when utilizing as a utility this needs
to be accounted for carefully as well.
@leondz <https://github.com/leondz> may want to weigh in on this was well
since he filed the original issue.
—
Reply to this email directly, view it on GitHub
<#1663?email_source=notifications&email_token=CBAUJ5ORFNY5KCSISDB2AWL4TULB3A5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTIMBUGQZDINZSGQY2M4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2L24DSL5ZGK5TJMV3V63TPORUWM2LDMF2GS33OONPWG3DJMNVQ#pullrequestreview-4044247241>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/CBAUJ5PKQ7HJJI7APO4X4SL4TULB3AVCNFSM6AAAAACXIXETXCVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHM2DANBUGI2DOMRUGE>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
tagging related PR #1523 |
|
I would think the latter is acceptable, raising an exception with details when the data is not compatible and then catching that and reporting it would be acceptable. |
3 participants
Fixes #1024
The three PluginCache.plugin_info() call sites in report_digest.py crash with a ValueError when a report references a probe or detector that no longer exists in the current codebase (e.g. due to a rename between versions).
This PR wraps each call in try/except, logs a warning, and continues report generation with placeholder values instead of crashing.
Changes:
_init_populate_result_db (line ~131): falls back to empty tags, grouping the probe under "other"
_get_probe_info (line ~255): uses the classpath as description, empty tags, and None tier
_get_probe_detector_details (line ~305): uses the detector name as its description
Tested with a JSONL file referencing a nonexistent probe class. Before the fix it crashed with ValueError. After the fix it generates a valid HTML report and logs a warning.