Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#4019 - Virus Scan False Positives #4129

Merged
merged 2 commits into from
Dec 18, 2024

Conversation

bidyashish
Copy link
Collaborator

@bidyashish bidyashish commented Dec 17, 2024

Acceptance Criteria

  • Investigate pdfs failing virus scanning and fix
  • Try to update clamav to the most updated version (nice to have)

Notes
MaxFiles 100 was causing issue with file being not scanned and using ClamAV virus Database bank to not scan file.

MaxFiles in ClamAV's configuration refers to the maximum number of files to be scanned within an archive, document, or any other container file. Here's a detailed explanation: For example: If scanning a ZIP file containing 15,000 files with MaxFiles 10000: Only the first 10,000 files will be scanned The remaining 5,000 files will be skipped If AlertExceedsMax is enabled, it will trigger a "Heuristics.Limits.Exceeded.MaxFiles" alert

Update Clam AV Docker from BCGOV Repo
Link: https://github.com/bcgov/common-hosted-clamav-service/pkgs/container/clamav-unprivileged

Demo: Manual test in Dev using Config update.
image

@@ -562,7 +562,7 @@ MaxRecursion 10
# Note: disabling this limit or setting it too high may result in severe damage
# to the system.
# Default: 10000
MaxFiles 100
MaxFiles 10000
Copy link
Collaborator

Choose a reason for hiding this comment

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

As it is set to default value please comment this line leaving the default value to be in effect.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks @dheepak-aot

Default value is commented.

@@ -562,7 +562,7 @@ MaxRecursion 10
# Note: disabling this limit or setting it too high may result in severe damage
# to the system.
# Default: 10000
MaxFiles 100
MaxFiles 10000
Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks @dheepak-aot

Default value is commented.

Copy link

Backend Unit Tests Coverage Report

Totals Coverage
Statements: 22% ( 3786 / 17207 )
Methods: 10.01% ( 219 / 2187 )
Lines: 25.38% ( 3281 / 12930 )
Branches: 13.68% ( 286 / 2090 )

Copy link

E2E Workflow Workers Coverage Report

Totals Coverage
Statements: 65.43% ( 583 / 891 )
Methods: 59.26% ( 64 / 108 )
Lines: 68.54% ( 464 / 677 )
Branches: 51.89% ( 55 / 106 )

Copy link

E2E Queue Consumers Coverage Report

Totals Coverage
Statements: 80.48% ( 1328 / 1650 )
Methods: 64.59% ( 135 / 209 )
Lines: 83.83% ( 1115 / 1330 )
Branches: 70.27% ( 78 / 111 )

Copy link
Collaborator

@dheepak-aot dheepak-aot left a comment

Choose a reason for hiding this comment

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

Thanks for doing the changes @bidyashish. Looks good 👍

Copy link

E2E SIMS API Coverage Report

Totals Coverage
Statements: 67.4% ( 5922 / 8787 )
Methods: 65.09% ( 729 / 1120 )
Lines: 71.33% ( 4647 / 6515 )
Branches: 47.4% ( 546 / 1152 )

Copy link
Collaborator

@andrepestana-aot andrepestana-aot left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks for the explanation. I just think that some explanation about why that happened and how it works would be good to be added in the description of this PR as someone can as ask one of the DEVs in the future why we change that.

@bidyashish bidyashish added this pull request to the merge queue Dec 18, 2024
Merged via the queue into main with commit 1eb9906 Dec 18, 2024
21 checks passed
@bidyashish bidyashish deleted the feature/#4019-clamav-virus-false-positives branch December 18, 2024 00:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants