Conversation
…attachments, configure node debugger, etc..
…canned and cleanly running thru clamav with no errors..
…ood and bad attachments..
… and code clean-up..
…ully resolve failures in clamav instead of crashing..
…ded, need to keep app afloat, this finishes 1753 ticket..
…s in clamav-ts file..
…s in clamav-ts file..
…an-pix-1754' into attach-scan-pix-1755
…kend still needs touch ups as it hangs..
…, all good so far...
…tuning to follow..
…ng multiple file ok too..
…ut looking good now..
…th of sec-error-png canned file..
paulsolt-ofsw
left a comment
There was a problem hiding this comment.
When I tried again from iOS the file is not removed on mobile.
<test.mage.geointapps.com>
2026-4-21.ClamAV.code.review.mp4
If we don't update the lastModified data on the server, than the iOS app never attempts to update things. Something feels off around this logic for swapping to a placeholder image.
I still have the "infected" file in my local storage on iOS, and I never see the "security image".
There are a lot of changes on this PR that don't appear to be related to the actual work.
I'm a little concerned to merge this as is, because of the scope of changes, and chance for regressions.
| "@ngageoint/mage.arcgis.service": "../plugins/arcgis/service", | ||
| "@ngageoint/mage.arcgis.web-app": "../plugins/arcgis/web-app/dist/main", | ||
| "@ngageoint/mage.sftp.web": "../plugins/sftp/web/dist/main", | ||
| "@ngageoint/mage.sftp.service": "../plugins/sftp/service", | ||
| "@ngageoint/mage.nga-msi": "../plugins/nga-msi", | ||
| "@ngageoint/mage.arcgis.service": "file:../plugins/arcgis/service", | ||
| "@ngageoint/mage.arcgis.web-app": "file:../plugins/arcgis/web-app/dist/main", | ||
| "@ngageoint/mage.nga-msi": "file:../plugins/nga-msi", | ||
| "@ngageoint/mage.service": "../service", | ||
| "@ngageoint/mage.web-app": "../web-app/dist" | ||
| "@ngageoint/mage.sftp.service": "file:../plugins/sftp/service", | ||
| "@ngageoint/mage.sftp.web": "file:../plugins/sftp/web/dist/main", |
There was a problem hiding this comment.
Why change these, is it related to the actual work?
There was a problem hiding this comment.
These were pulled in automatically during a rebuild on preceding branch 1756, not an intentional dependency addition. The key commit is b88a899f
There was a problem hiding this comment.
Ok, so npm install or something changed this to use file:?
Why doesn't this "../web-app/dist" get the same treatment? Does it break something in the build/install?
"@ngageoint/mage.sftp.web": "file:../plugins/sftp/web/dist/main",
"@ngageoint/mage.web-app": "../web-app/dist",
|
Merging this PR is still a ways off. At this point, not comfortable--and won't be until there is requisite clarity and proof. Furthermore, the PR's scope is incomplete/incorrect. The |
Mobile Client Compatibility — Failed Attachment Handling (iOS/Android) #1874
Summary
This branch implements the server-side API contract for failed attachment handling following ClamAV pre-disk scanning, establishing the foundation for mobile client compatibility.
Changes Made
service/src/adapters/observations/adapters.observations.controllers.web.tsto handle ClamAV rejected attachments by storing a placeholder image (SecError.png) in place of the rejected file and returning a structured response includingcontentStored: false,error, andfailuresfieldsservice/src/assets/SecError.png— placeholder image displayed when an attachment is rejected by ClamAVpredisk-scanning-1756Response Structure on Rejection
{ "id": "", "fieldName": "", "observationFormId": "", "lastModified": "", "contentType": "", "name": "", "size": "", "oriented": "", "thumbnails": [], "contentStored": false, "url": "", "relativePath": "", "error": "File failed security scan", "failures": [ { "file": "eicar.txt", "error": "File failed security scan" } ] }Testing
eicar.txt— rejection response matches agreed structureDependencies
CLAM_AV_URLenv variable setdevelopwithpredisk-scanning-1756merged inOpen Items