diff --git a/internal/document/clean/contentType.go b/internal/document/clean/contentType.go index cb954d60..10cdb095 100644 --- a/internal/document/clean/contentType.go +++ b/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 { diff --git a/internal/document/clean/service_test.go b/internal/document/clean/service_test.go index 4741f91a..f931fa91 100644 --- a/internal/document/clean/service_test.go +++ b/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") + }) + } +} diff --git a/internal/document/types/pdf_test.go b/internal/document/types/pdf_test.go index bd969fdd..b9600669 100644 --- a/internal/document/types/pdf_test.go +++ b/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)) diff --git a/test/aarete.corrupt.test.pdf.files/--a0jHn00000CJ4QlIAL-3-10-2021-Universal-IAMHP-Roster-Template11.pdf b/test/aarete.corrupt.test.pdf.files/--a0jHn00000CJ4QlIAL-3-10-2021-Universal-IAMHP-Roster-Template11.pdf new file mode 100644 index 00000000..156c7b60 Binary files /dev/null and b/test/aarete.corrupt.test.pdf.files/--a0jHn00000CJ4QlIAL-3-10-2021-Universal-IAMHP-Roster-Template11.pdf differ diff --git a/test/aarete.corrupt.test.pdf.files/--a0jHn00000CJ7WfIAL-RE_ Aperion Care Wesley Retro Approval.pdf b/test/aarete.corrupt.test.pdf.files/--a0jHn00000CJ7WfIAL-RE_ Aperion Care Wesley Retro Approval.pdf new file mode 100644 index 00000000..7d456165 Binary files /dev/null and b/test/aarete.corrupt.test.pdf.files/--a0jHn00000CJ7WfIAL-RE_ Aperion Care Wesley Retro Approval.pdf differ