Skip to content

Add GuardDuty malware scanning to storage module#258

Open
jrpbc wants to merge 2 commits intomainfrom
johan/storage-malware-scanning
Open

Add GuardDuty malware scanning to storage module#258
jrpbc wants to merge 2 commits intomainfrom
johan/storage-malware-scanning

Conversation

@jrpbc
Copy link

@jrpbc jrpbc commented Feb 27, 2026

Ticket

Resolves #965

Changes

What was added, updated, or removed in this PR.

Add to the Infra Template Storage module malware scanning:

  • Scans all Storage uploaded documents for malware in real-time
  • Prevents users from downloading infected files

Testing instructions, background context, more in-depth details of the implementation, and anything else you'd like to call out or ask reviewers.

Clean File Upload/Download Test

Upload clean document
echo "Testing #965" >> 965.tst.txt
aws s3 cp 965.tst.txt s3://<workspace>-platform-test-app-dev

Verify document uploaded and tagged applied
aws s3api get-object-tagging --bucket <workspace>-platform-test-app-dev --key 965.tst.txt

Verify you can download file
aws s3 cp s3://<workspace>-platform-test-app-dev/965.tst.txt ./965.tst.txt

Malware Detection Test

Use EICAR test file (standard malware test string)
echo 'X5O!P%@AP[4\PZX54(P^)7CC)7}$EICAR-STANDARD-ANTIVIRUS-TEST-FILE!$H+H*' > eicar.txt

Upload test malware
aws s3 cp eicar.txt s3://<workspace>-platform-test-app-dev

Verify threat tag
aws s3api get-object-tagging –bucket <workspace>-platform-test-app-dev –key eicar.txt

Verify you cannot download the file
aws s3 cp s3://<workspace>-platform-test-app-dev/eicar.txt ./eicar.txt

Provide evidence that the code works as expected. Explain what was done for testing and the results of the test plan. Include screenshots, GIF demos, shell commands or output to help show the changes working as expected. ProTip: you can drag and drop or paste images into this textbox.

Clean File Upload/Download Test

image

Malware Detection Test

image

Preview environment for app-nextjs

Preview environment for app

Preview environment for app-rails

Preview environment for app-flask

@jrpbc jrpbc force-pushed the johan/storage-malware-scanning branch 10 times, most recently from 5edf672 to cbf85a9 Compare February 28, 2026 01:45
@jrpbc jrpbc requested a review from doshitan February 28, 2026 02:27
@jrpbc jrpbc force-pushed the johan/storage-malware-scanning branch 2 times, most recently from 0093aa2 to 7515c58 Compare March 5, 2026 21:22
- Enable configurable GuardDuty detector at account level
- Add S3 malware protection with automatic object tagging
- Block S3 access to objects tagged with THREATS_FOUND
@jrpbc jrpbc force-pushed the johan/storage-malware-scanning branch from 7515c58 to d0d8882 Compare March 5, 2026 21:57
Copy link
Contributor

@doshitan doshitan left a comment

Choose a reason for hiding this comment

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

Updated naming seems odd, I think these are separate words?

  • threatdetection -> threat_detection
  • malwaredetection -> malware_detection

We need docs describing capabilities and how to configure.

allowed_actions = [for aws_service in module.project_config.aws_services : "${aws_service}:*"]
}

# GuardDuty module - account-wide security detector
Copy link
Contributor

Choose a reason for hiding this comment

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

This is account-wide, but region specific. template-infra only really supports project_config.default_region + us-east-1 (for things that require existing there), but in the future we will have more greater support for multi-region configs.

We could lay some ground work for that with a local var like utilized_regions = distinct([local.region, 'us-east-1']) and create instances of the module for each region? Or have the module accept a list of regions.

Thoughts?

Copy link
Author

Choose a reason for hiding this comment

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

agree. thoughts on adding utilized_regions as a list type variable, in project-config defaulting to default_region? I just added it, and tried to use it within the threatdection module - had to revert back the threatdetection work - the current Terraform AWS Provider we are using ("5.6.0") does not support specifying region on the aws_guardduty_detector. This was added on latest version of the AWS Terraform provider. Added TODO comments on the infra/modules/threatdetection.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please create issues in template-infra (if they don't exist) for:

  • AWS provider update
  • Multi-region support for GuardDuty setup

Then link the latter here.

actions = ["s3:*"]
resources = [
aws_s3_bucket.storage.arn,
"${aws_s3_bucket.storage.arn}/*"
Copy link
Contributor

Choose a reason for hiding this comment

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

This change seems unrelated? Can you break it out into a separate PR?

Copy link
Author

Choose a reason for hiding this comment

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

Took it out of this PR, and will be addressing it on a new PR.

@jrpbc jrpbc requested a review from doshitan March 13, 2026 17:03
Copy link
Contributor

@doshitan doshitan left a comment

Choose a reason for hiding this comment

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

Main feedback from previous review has not been addressed: #258 (review)

Comment on lines +20 to +22
sid = "RestrictToTLSRequestsOnly"
effect = "Deny"
actions = ["s3:*"]
Copy link
Contributor

Choose a reason for hiding this comment

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

These formatting changes seem unrelated, remove?

allowed_actions = [for aws_service in module.project_config.aws_services : "${aws_service}:*"]
}

# GuardDuty module - account-wide security detector
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please create issues in template-infra (if they don't exist) for:

  • AWS provider update
  • Multi-region support for GuardDuty setup

Then link the latter here.

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.

2 participants