Platform/bill/16144 #16550
Platform/bill/16144 #16550
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Manifest Files |
| @@ -114,6 +118,10 @@ class FHIRTranslator( | |||
| logger.trace("Preparing to send original message") | |||
| val originalReport = reportService.getRootReport(message.reportId) | |||
| val bodyBytes = BlobAccess.downloadBlobAsByteArray(originalReport.bodyUrl) | |||
| val localDigest = BlobUtils.digestToString(sha256Digest(bodyBytes)) | |||
| check(message.digest == localDigest) { | |||
There was a problem hiding this comment.
This seems wrong to me. The digest value in the message I believe would be the digest of the blob associated with the passed in report (so the report that is downloaded in sendTranslated). Here, you are downloading the ROOT report, which would have a different digest.
There was a problem hiding this comment.
So, if I'm understanding correctly, we should be comparing message.digest with blobInfo.digest (after the file is uploaded)?
There was a problem hiding this comment.
message.digest is already used in sendTranslated and that's the only place it should be used. Here, you are downloading the root report, so you need to to compare the downloaded root report to the root report's digest (which is stored in the DB report_file table)
There was a problem hiding this comment.
Latest changes should address this.
| @@ -163,6 +171,18 @@ class FHIRTranslator( | |||
| topic = message.topic | |||
| ) | |||
|
|
|||
| val builder = ReportStreamItemEventBuilder( | |||
There was a problem hiding this comment.
We should prefer the style that is shown in FHIRDestinationFilter where we pass in the builder directly to sendItemEvent (so we remain consistent):
reportEventService.sendItemEvent(
eventName = ReportStreamEventName.ITEM_ROUTED,
childReport = report,
pipelineStepName = TaskAction.destination_filter
) {
parentReportId(queueMessage.reportId)
params(
mapOf(
ReportStreamEventProperties.RECEIVER_NAME to receiver.fullName,
ReportStreamEventProperties.BUNDLE_DIGEST
to bundleDigestExtractor.generateDigest(bundle)
)
)
trackingId(bundle)
}
There was a problem hiding this comment.
Should be in there now.
…Service actually having a value in builder.
| @@ -114,6 +121,11 @@ class FHIRTranslator( | |||
| logger.trace("Preparing to send original message") | |||
| val originalReport = reportService.getRootReport(message.reportId) | |||
| val bodyBytes = BlobAccess.downloadBlobAsByteArray(originalReport.bodyUrl) | |||
There was a problem hiding this comment.
nit: Have we considered updating this to use BlobAccess.downloadBlob(...) which will do the digest check for us?
There was a problem hiding this comment.
Made the change.
prime-router/src/test/kotlin/fhirengine/azure/FHIRTranslatorIntegrationTests.kt
Show resolved
Hide resolved
arnejduranovic
temporarily deployed
to
staging
GitHub Actions
Inactive
|
This PR contains coding changes for user story 16144.
Test Steps:
Changes
Checklist
Testing
./prime testor./gradlew testSmokeagainst local Docker ReportStream container?npm run lint:write?Process
Linked Issues
To Be Done
Specific Security-related subjects a reviewer should pay specific attention to