Add clean scan validation and harden ClamAV status parsing

Author: docwho2

.github/workflows/deploy.yml

@@ -110,6 +110,10 @@ jobs:
         VALIDATION_BUCKET: ${{ vars.VALIDATION_BUCKET }}
         ONLY_TAG_INFECTED: ${{ vars.ONLY_TAG_INFECTED }}
       run: |
+        if [ "${ONLY_TAG_INFECTED}" != "false" ]; then
+          echo "VALIDATION_BUCKET requires ONLY_TAG_INFECTED=false so clean-file validation can assert CLEAN instead of an absent tag." >&2
+          exit 1
+        fi
         echo "Waiting 30 seconds to allow Lambda to finish deploying..."
         sleep 30
         echo "Running Virus Scan Validation Tests..."

integration-test/src/main/java/cloud/cleo/clamav/test/VirusScanValidationTest.java

@@ -36,6 +36,7 @@ public class VirusScanValidationTest {
 
     private static final String BUCKET_NAME = System.getenv("VALIDATION_BUCKET");
     private static final String INFECTED_KEY = "eicar.txt";
+    private static final String CLEAN_KEY = "NotInfectedFile.pdf";
     private static final String OVERSIZED_KEY = "large-test-file.zip";
 
     private static final S3Client s3 = S3Client.create();
@@ -48,6 +49,7 @@ static void checkSetup() {
 
         // Ensure all tags are cleared before testing starts
         clearTags(INFECTED_KEY);
+        clearTags(CLEAN_KEY);
         clearTags(OVERSIZED_KEY);
 
         //throw new AssertionError("Testing rollback");
@@ -83,20 +85,35 @@ public void validateScanOfKnownInfectedFile() throws InterruptedException {
             retriggerScan(INFECTED_KEY);
         }
         // Need to wait for scan to complete, which sometimes can take over a minute
-        waitForTagValue(INFECTED_KEY, ScanStatus.INFECTED, Duration.ofMinutes(2));
+        waitForTerminalTagValue(INFECTED_KEY, ScanStatus.INFECTED, Duration.ofMinutes(2));
     }
 
     /**
-     * When file to scan is over MAX_BYTES, then ensure it is not scanned and immediately tagged FILE_SIZE_EXCEEDED.
+     * Validate a known clean file tests to be CLEAN. This catches false positives where ClamAV
+     * fails to start but the pipeline still reports files as infected.
      *
      * @throws InterruptedException
      */
     @Test
     @Order(3)
+    public void validateScanOfKnownCleanFile() throws InterruptedException {
+        assumeTrue(!ONLY_TAG_INFECTED, "Skipping because clean validation requires ONLY_TAG_INFECTED=false");
+
+        retriggerScan(CLEAN_KEY);
+        waitForTerminalTagValue(CLEAN_KEY, ScanStatus.CLEAN, Duration.ofMinutes(2));
+    }
+
+    /**
+     * When file to scan is over MAX_BYTES, then ensure it is not scanned and immediately tagged FILE_SIZE_EXCEEDED.
+     *
+     * @throws InterruptedException
+     */
+    @Test
+    @Order(4)
     public void validateScanOfOversizedFile() throws InterruptedException {
         retriggerScan(OVERSIZED_KEY);
         // Should be detected before scanning on the S3 Head operation
-        waitForTagValue(OVERSIZED_KEY, ScanStatus.FILE_SIZE_EXCEEED, Duration.ofSeconds(30));
+        waitForTerminalTagValue(OVERSIZED_KEY, ScanStatus.FILE_SIZE_EXCEEED, Duration.ofSeconds(30));
     }
 
     /**
@@ -141,6 +158,37 @@ private static void waitForTagValue(String key, ScanStatus expectedValue, Durati
         throw new AssertionError("Timed out waiting for scan-status tag: " + expectedValue + " on key: " + key);
     }
 
+    private static void waitForTerminalTagValue(String key, ScanStatus expectedValue, Duration timeout) throws InterruptedException {
+        long timeoutMillis = timeout.toMillis();
+        long sleepMillis = 5000;
+        long start = System.currentTimeMillis();
+
+        while (System.currentTimeMillis() - start <= timeoutMillis) {
+            List<Tag> tags = getTags(key);
+            String actual = tags.stream()
+                    .filter(tag -> SCAN_TAG_NAME.equals(tag.key()))
+                    .map(Tag::value)
+                    .findFirst()
+                    .orElse(null);
+
+            if (expectedValue.name().equals(actual)) {
+                assertThat(actual).isEqualTo(expectedValue.name());
+                return;
+            }
+
+            if (actual != null) {
+                ScanStatus actualStatus = ScanStatus.valueOf(actual);
+                if (actualStatus != ScanStatus.SCANNING) {
+                    throw new AssertionError("Expected scan-status " + expectedValue + " but found " + actualStatus + " on key: " + key);
+                }
+            }
+
+            Thread.sleep(sleepMillis);
+        }
+
+        throw new AssertionError("Timed out waiting for scan-status tag: " + expectedValue + " on key: " + key);
+    }
+
     /**
      * Get all tags on S3 Object.
      *

lambda/pom.xml

@@ -69,6 +69,24 @@
             <artifactId>aws-crt-client</artifactId>
         </dependency>   
 
+        <dependency>
+            <groupId>org.junit.jupiter</groupId>
+            <artifactId>junit-jupiter-api</artifactId>
+            <scope>test</scope>
+        </dependency>
+
+        <dependency>
+            <groupId>org.junit.jupiter</groupId>
+            <artifactId>junit-jupiter-engine</artifactId>
+            <scope>test</scope>
+        </dependency>
+
+        <dependency>
+            <groupId>org.assertj</groupId>
+            <artifactId>assertj-core</artifactId>
+            <scope>test</scope>
+        </dependency>
+
     </dependencies>
 
     <build>
@@ -78,6 +96,12 @@
                 <groupId>org.apache.maven.plugins</groupId>
                 <artifactId>maven-shade-plugin</artifactId>
             </plugin>
+
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-surefire-plugin</artifactId>
+                <version>3.5.4</version>
+            </plugin>
 
             <plugin>
                 <groupId>org.apache.maven.plugins</groupId>

lambda/src/main/java/cloud/cleo/clamav/lambda/ScanningLambda.java

@@ -16,6 +16,7 @@
 import java.nio.file.Paths;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.regex.Pattern;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.CompletionException;
 import java.util.concurrent.TimeUnit;
@@ -32,6 +33,7 @@
  * @author sjensen
  */
 public class ScanningLambda implements RequestHandler<S3EventNotification, Void> {
+    private static final Pattern CLAMAV_FOUND_PATTERN = Pattern.compile("(?m)^.*\\bFOUND$");
 
     // Create an S3 client with CRT Async (better download performance and Async calls)
     final static S3AsyncClient s3Client = S3AsyncClient.crtCreate();
@@ -129,19 +131,13 @@ public Void handleRequest(S3EventNotification event, Context context) {
                     return;
                 }
 
+                String processOutput;
                 try (final var is = process.getInputStream()) {
-                    log.debug("Process Output: {}", new String(is.readAllBytes()));
+                    processOutput = new String(is.readAllBytes());
+                    log.debug("Process Output: {}", processOutput);
                 }
 
-                // According to ClamAV: 0 means CLEAN, 1 means INFECTED, else ERROR.
-                status = switch (process.exitValue()) {
-                    case 0 ->
-                        ScanStatus.CLEAN;
-                    case 1 ->
-                        ScanStatus.INFECTED;
-                    default ->
-                        ScanStatus.ERROR;
-                };
+                status = determineScanStatus(process.exitValue(), processOutput);
                 log.info("Scan result for {}: {}", key, status);
             } catch (IOException | InterruptedException e) {
                 log.error("Error running clamscan: ", e);
@@ -228,6 +224,24 @@ static long getClamScanWaitMillis(int remainingMillis) {
         return Math.max(0, remainingMillis - 10000L);
     }
 
+    static ScanStatus determineScanStatus(int exitCode, String processOutput) {
+        if (exitCode == 0) {
+            return ScanStatus.CLEAN;
+        }
+
+        if (exitCode == 1) {
+            if (processOutput != null && CLAMAV_FOUND_PATTERN.matcher(processOutput).find()) {
+                return ScanStatus.INFECTED;
+            }
+
+            log.error("clamscan exited with code 1 but did not report a FOUND signature. Treating as ERROR. Output: {}",
+                    processOutput);
+            return ScanStatus.ERROR;
+        }
+
+        return ScanStatus.ERROR;
+    }
+
     static Path createTempFilePath(String key) {
         String baseName = new File(key).getName();
         String extension = "";

lambda/src/test/java/cloud/cleo/clamav/lambda/ScanningLambdaTest.java

@@ -0,0 +1,35 @@
+package cloud.cleo.clamav.lambda;
+
+import cloud.cleo.clamav.ScanStatus;
+import static org.assertj.core.api.Assertions.assertThat;
+import org.junit.jupiter.api.Test;
+
+class ScanningLambdaTest {
+
+    @Test
+    void determineScanStatusReturnsInfectedWhenClamavReportsFoundSignature() {
+        String output = """
+                /tmp/upload.pdf: Win.Test.EICAR_HDB-1 FOUND
+                ----------- SCAN SUMMARY -----------
+                Infected files: 1
+                """;
+
+        assertThat(ScanningLambda.determineScanStatus(1, output)).isEqualTo(ScanStatus.INFECTED);
+    }
+
+    @Test
+    void determineScanStatusTreatsStartupFailureAsErrorInsteadOfInfected() {
+        String output = """
+                clamscan: /lib64/libc.so.6: version `GLIBC_2.38' not found (required by clamscan)
+                clamscan: /lib64/libm.so.6: version `GLIBC_2.38' not found (required by /usr/lib/clamav_libs/libclamav.so.12)
+                """;
+
+        assertThat(ScanningLambda.determineScanStatus(1, output)).isEqualTo(ScanStatus.ERROR);
+    }
+
+    @Test
+    void determineScanStatusReturnsCleanForExitCodeZero() {
+        assertThat(ScanningLambda.determineScanStatus(0, "/tmp/upload.pdf: OK"))
+                .isEqualTo(ScanStatus.CLEAN);
+    }
+}