Skip to content

Never present a failed or undecided check as a clean result - #4

Merged
overjoyde merged 2 commits into
overjoyde:mainfrom
pepo72:fix/incomplete-is-not-clean
Sep 26, 2026
Merged

overjoyde merged 2 commits into
overjoyde:mainfrom
pepo72:fix/incomplete-is-not-clean

Conversation

@pepo72

@pepo72 pepo72 commented Sep 26, 2026

Copy link
Copy Markdown

Summary

The README says a failed check never counts as clean. A few paths still presented a failed or undecided check as a clean result:

  • pyHanko's reader.embedded_signatures raises on the first signature whose CMS container cannot be read. The error only went into the facts, the analysis stayed complete, and the only finding was a LOW note that blamed a legacy format. One damaged signature also left every other signature in the file unvalidated.
  • When pdfsig could not verify a signature, no finding was produced.
  • signature.validation-error had LOW confidence, so it counted as LOW.
  • When an analyser failed, the summary still listed its area under "Checked and found in order", and the batch table showed the verdict with no marker and counted the file as "without significant findings".

Changes

  • signatures.py: signatures are loaded one by one, with the same SubFilter filter pyHanko uses. A CMS container that cannot be read is signature.unparseable (HIGH); other loader failures are signature.validation-error, now with MEDIUM confidence. A pyHanko failure while reading the file marks the analysis incomplete. Document timestamps are validated with validate_pdf_timestamp. Files with only an owner password are decrypted before reading.
  • pdfsig.py: new pdfsig.integrity-unknown (MEDIUM, low confidence) when Poppler did not verify a signature and pyHanko did not settle its integrity either. Empty signature fields are skipped.
  • summary.py: "Checked and found in order" lines are only written for analysers that ran without error, for PDF and Office. The signature line is withheld when any signature finding leaves integrity in doubt. The batch summary counts incomplete files separately and lists them in the JSON.
  • render.py: the batch table marks incomplete verdicts.
  • cli.py: new opt-in --fail-on-incomplete exits 1 when an analysis is incomplete or a file could not be analysed. --fail-on is unchanged. Legacy .doc/.xls/.ppt files are always incomplete, which is why this is a separate flag and noted in the README.

Testing

  • pytest: all tests pass, including new tests for a damaged signature among valid ones, document timestamps with and without a prior signature, an empty signature field, an owner-password-only signed file, analyser failures in the summary and batch output, and both exit-code flags.
  • Checked with pdfsig enabled on genuine files: single and double signatures, certification and certification plus approval, signature timestamps, document timestamps, an empty signature field, and owner-password encryption. Each gives the same or a lower verdict than before; document timestamps and the encrypted file go from low to info. Damaged signature containers go from low to high.
  • The bank-statement examples give identical results before and after.

This touches summary.py and tests/pdfgen.py next to #3; whichever is merged second may need a small rebase, which I can do.

Signatures
- Load each signature separately. pyHanko's embedded_signatures raised
  on the first unreadable signature container, which left every
  signature in the file unvalidated and only produced a LOW note that
  blamed a legacy format.
- An unreadable signature container is now signature.unparseable (HIGH).
  A pyHanko failure while reading the signatures marks the analysis
  incomplete instead of only being recorded in the facts.
- signature.validation-error keeps MEDIUM effective severity (confidence
  MEDIUM instead of LOW): integrity is unknown, which needs review.
- pdfsig reporting that it did not verify a signature is now
  pdfsig.integrity-unknown instead of producing no finding.

Summary, batch report and exit code
- "Checked and found in order" lines are only written for analysers
  that ran and reported no error, for PDF and Office alike, and the
  signature line is withheld when any signature finding leaves
  integrity in doubt.
- The batch report marks incomplete verdicts and counts them apart
  from documents without significant findings.
- --fail-on also exits 1 when an analysis is incomplete or a file
  could not be analysed.
…s, and make --fail-on-incomplete opt-in

Follow-up after review:

- Document timestamps (/DocTimeStamp) are validated with
  validate_pdf_timestamp instead of failing validate_pdf_signature, so
  PAdES-LTA and other timestamped files are no longer flagged.
- pdfsig: an unsigned signature field is skipped, and "integrity
  unknown" is only reported when pyHanko did not settle the integrity
  of that signature either (Poppler does not verify document
  timestamps).
- pyHanko decrypts files that have only an owner password (empty user
  password), or uses --password, before reading the signatures.
- signature.unparseable is only used when the CMS container itself
  cannot be read; other loader failures are validation errors.
  signature.not-validated is not added when pyHanko could not read the
  file at all, since the analysis is then already incomplete.
- Field names are stored as plain strings (pyHanko returns proxy
  objects for encrypted files, which broke report serialisation).
- --fail-on keeps its previous behaviour. The new --fail-on-incomplete
  exits 1 when an analysis is incomplete or a file could not be
  analysed. The batch summary lists incomplete files in its JSON.
@overjoyde
overjoyde merged commit e125ca3 into overjoyde:main Sep 26, 2026
6 checks passed
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