Merged in bugfix/pdf-verification (pull request #218)

fix pdf checking

* fix pdf checking

https://aarete.atlassian.net/browse/DEVOPS-651

internal/document/clean/contentType.go

@@ -30,13 +30,28 @@ var (
 	PDFSignatureEnd = []byte(PDFSignatureEndStr)
 )
 
+// getAcceptedMimeType determines the file's MIME type by inspecting actual content.
+// S3 ContentType metadata is derived from the file extension and cannot be trusted,
+// so content-based validation (magic bytes) always runs first.
 func (s *Service) getAcceptedMimeType(ctx context.Context, params *CleanParams, contentType *string, length int64) (documenttypes.MimeType, error) {
-	mimeType := s.getMetadataMimeType(contentType)
-	if mimeType != documenttypes.MimeTypeInvalid && mimeType != documenttypes.MimeTypeBinaryOctetStream {
-		return mimeType, nil
+	bodyMimeType, err := s.getBodyMimeType(ctx, params, length)
+	if err != nil {
+		return documenttypes.MimeTypeInvalid, err
 	}
 
-	return s.getBodyMimeType(ctx, params, length)
+	if bodyMimeType != documenttypes.MimeTypeInvalid {
+		return bodyMimeType, nil
+	}
+
+	// Content didn't match any known signature - fall back to metadata
+	// only for truly unknown content (binary/octet-stream or missing metadata)
+	mimeType := s.getMetadataMimeType(contentType)
+	if mimeType != documenttypes.MimeTypeInvalid && mimeType != documenttypes.MimeTypeBinaryOctetStream {
+		// Metadata claims a known type but content doesn't confirm it - reject
+		return documenttypes.MimeTypeInvalid, nil
+	}
+
+	return documenttypes.MimeTypeInvalid, nil
 }
 
 func (s *Service) getMetadataMimeType(contentType *string) documenttypes.MimeType {

internal/document/clean/service_test.go

@@ -131,3 +131,85 @@ func TestDocumentCleanClean_ReturnsCleanResult(t *testing.T) {
 	assert.False(t, failResult.Passed, "non-PDF document should fail validation")
 	assert.NotNil(t, failResult.FailReason, "failed document should have a FailReason")
 }
+
+// TestClean_RejectsNonPDFWithPDFContentType verifies that files with
+// application/pdf content type metadata but non-PDF content are rejected.
+// This is the core scenario where S3 metadata lies about the file type.
+func TestClean_RejectsNonPDFWithPDFContentType(t *testing.T) {
+	cfg := &DocCleanConfig{}
+	test.CreateDB(t, cfg)
+	test.CreateAWSResources(t, cfg)
+	ctx := t.Context()
+
+	svc := documentclean.New(cfg)
+
+	uniqueSuffix := uuid.New().String()[:8]
+	clientID := "mimetype_check_" + uniqueSuffix
+	err := cfg.GetDBQueries().CreateClient(ctx, &repository.CreateClientParams{
+		Clientid: clientID,
+		Name:     "Test MIME Check " + uniqueSuffix,
+	})
+	require.NoError(t, err)
+
+	tests := []struct {
+		name    string
+		content []byte
+	}{
+		{
+			name:    "xlsx with PDF content type",
+			content: []byte("PK\x03\x04fake xlsx content that is definitely not a PDF file at all"),
+		},
+		{
+			name:    "OLE2/msg with PDF content type",
+			content: []byte("\xD0\xCF\x11\xE0\xA1\xB1\x1A\xE1fake OLE2 msg content not a PDF"),
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			location := objectstore.BucketKey{
+				Location:  objectstore.Import,
+				ClientID:  clientID,
+				CreatedAt: time.Now().UTC(),
+				EntityID:  uuid.New(),
+			}
+
+			// Upload with application/pdf content type despite non-PDF content
+			_, err := cfg.GetStoreClient().PutObject(ctx, &s3.PutObjectInput{
+				Bucket:      aws.String(cfg.GetBucket()),
+				Key:         aws.String(location.String()),
+				Body:        bytes.NewReader(tt.content),
+				ContentType: aws.String("application/pdf"),
+			})
+			require.NoError(t, err)
+
+			headResp, err := cfg.GetStoreClient().HeadObject(ctx, &s3.HeadObjectInput{
+				Bucket: aws.String(cfg.GetBucket()),
+				Key:    aws.String(location.String()),
+			})
+			require.NoError(t, err)
+			etag := *headResp.ETag
+
+			docID, err := cfg.GetDBQueries().CreateDocument(ctx, &repository.CreateDocumentParams{
+				Clientid: clientID,
+				Hash:     etag,
+			})
+			require.NoError(t, err)
+
+			err = cfg.GetDBQueries().AddDocumentEntry(ctx, &repository.AddDocumentEntryParams{
+				Documentid: docID,
+				Bucket:     cfg.GetBucket(),
+				Key:        location.String(),
+			})
+			require.NoError(t, err)
+
+			result, err := svc.Clean(ctx, docID)
+			require.NoError(t, err)
+			require.NotNil(t, result)
+			assert.False(t, result.Passed, "file with non-PDF content should be rejected even if content type says application/pdf")
+			require.NotNil(t, result.FailReason, "rejected document should have a FailReason")
+			assert.Equal(t, "invalid_mimetype", *result.FailReason,
+				"non-PDF content should be rejected at MIME type check, not at PDF parse stage")
+		})
+	}
+}

internal/document/types/pdf_test.go

@@ -240,6 +240,40 @@ func TestIsCorrupt(t *testing.T) {
 	}
 }
 
+// TestIsCorrupt_NonPDFFiles verifies that IsCorrupt rejects files that have
+// .pdf extensions but are not actually PDFs (xlsx, msg, etc).
+func TestIsCorrupt_NonPDFFiles(t *testing.T) {
+	ctx := t.Context()
+	corruptDir := "../../../test/aarete.corrupt.test.pdf.files"
+
+	entries, err := os.ReadDir(corruptDir)
+	require.NoError(t, err)
+	require.NotEmpty(t, entries, "expected corrupt test files in %s", corruptDir)
+
+	for _, entry := range entries {
+		if entry.IsDir() {
+			continue
+		}
+
+		fileName := entry.Name()
+		if strings.ToLower(filepath.Ext(fileName)) != ".pdf" {
+			continue
+		}
+
+		t.Run(fileName, func(t *testing.T) {
+			fullPath := filepath.Join(corruptDir, fileName)
+			file, err := os.Open(fullPath)
+			require.NoError(t, err)
+			defer file.Close()
+
+			pdf := NewPDF(file)
+			reason := pdf.IsCorrupt(ctx)
+			require.NotNil(t, reason, "expected non-PDF file %q to be detected as corrupt", fileName)
+			assert.Equal(t, InvalidDocumentRead, *reason)
+		})
+	}
+}
+
 func TestNewPDF(t *testing.T) {
 	t.Run("valid", func(t *testing.T) {
 		pdf := NewPDF(strings.NewReader(pdfHelloWorld))