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.

Sign up for GitHub

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

Jump to bottom

feat(upload): scan files for viruses, filter for types #306

Merged
merged 11 commits into from
Jul 29, 2020
Merged

feat(upload): scan files for viruses, filter for types #306

merged 11 commits into from
Jul 29, 2020

Conversation

LoneRifle
Copy link
Contributor

@LoneRifle LoneRifle commented Jul 24, 2020 •
edited
Loading

Problem

Closes #274

Solution

  • Add a controller that will perform checks on uploaded files
  • Implement both a filter for file type and a service that submits files to virus scanning APIs
  • Provide VirusScanService implementations for VirusTotal and Cloudmersive

Tests

  • Both an innocuous PNG file as well as the EICAR test file should be uploaded. Go should accept the former but reject the latter.
  • Go should also reject an attempt to upload an .mp4 file, amongst others outside the allowed list of types

Deploy Notes

The new environment variable detailed below should be added to production to enable scanning

New environment variables:

  • CLOUDMERSIVE_KEY : API key for access to Cloudmersive

@LoneRifle LoneRifle requested a review from liangyuanruo July 24, 2020 07:44
@LoneRifle LoneRifle changed the title Virus scan feat(upload): scan files for viruses, filter for types Jul 24, 2020
LoneRifle and others added 9 commits July 28, 2020 15:54
@LoneRifle @yong-jie
feat(upload): scan for viruses
57a5a14 
Add an Express controller for file uploads to scan for viruses

- Create FileCheckController, which will inspect uploaded files and
  raise 4xx class errors if they fail inspection
- Implement VirusScanService, a dependency of FileCheckController,
  which would submit the file to a scanning service API
- Add plumbing to inversify/config/etc to wire everything together

TODO: tests
@LoneRifle @yong-jie
test(upload): add test for FileCheckController
57743ee
@yong-jie
feat(upload): cloudmersive dependency preparation
3604a2a
@yong-jie
feat(upload): implement cloudmersive file scanning
fb20390
@yong-jie
feat(upload): abstract promisified scanning function into a method
e92b39c
@yong-jie
feat(upload): use static functions
b9bff9b
@yong-jie
refactor(cloudmersive): static function's return syntax
fa25218
@yong-jie
style(inversify-config): remove commented code
f94758d
@LoneRifle @yong-jie
feat(upload): only allow some file types (#302)
0aed57e 
Use file-type and inspection of extension to determine whether
a file can be uploaded

- Bring in file-type as a dependency
- Implement FileTypeFilterService and drop into FileCheckController
- Provide test coverage
@opengovsg opengovsg deleted a comment from LoneRifle Jul 28, 2020
liangyuanruo
liangyuanruo requested changes Jul 28, 2020
View reviewed changes
Copy link
Contributor

@liangyuanruo liangyuanruo left a comment

Choose a reason for hiding this comment

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

as commented

@LoneRifle LoneRifle requested a review from liangyuanruo July 28, 2020 08:56
liangyuanruo
liangyuanruo approved these changes Jul 29, 2020
View reviewed changes
Copy link
Contributor

@liangyuanruo liangyuanruo left a comment

Choose a reason for hiding this comment

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

nitpicking on HTTP status codes because I've found it helpful for incident diagnosis, but lgtm otherwise!

@LoneRifle LoneRifle merged commit e683574 into develop Jul 29, 2020
@LoneRifle LoneRifle deleted the virus-scan branch July 29, 2020 05:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@liangyuanruo liangyuanruo liangyuanruo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add scanning of files before upload [By Jul 24]
3 participants
@LoneRifle @liangyuanruo @yong-jie