Skip to content

Commit ba81d8e

Browse files
authored
SIMSBIOHUB-884: Fix Malware Publish Edge Cases (#340)
* refactor(malware): Fixed a few maleware edge cases issues. * fix(malware): fix integration tests * fix(maleware): lint
1 parent fe15a53 commit ba81d8e

12 files changed

+217
-120
lines changed

api/.mocharc.system.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,6 @@
22
"extension": ["ts"],
33
"spec": "src/__integration__/system/*.integration.ts",
44
"require": "ts-node/register",
5+
"node-option": ["no-experimental-strip-types"],
56
"timeout": 60000
67
}

api/src/__integration__/system/malware-scan-worker.integration.ts

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { expect } from 'chai';
66
import { Knex, knex } from 'knex';
77
import sinon from 'sinon';
88
import * as tar from 'tar-stream';
9+
import { defaultPoolConfig, getAPIUserDBConnection, initDBPool } from '../../database/db';
910
import { initPgBoss, stopPgBoss } from '../../queue/pg-boss-service';
1011
import * as publisher from '../../queue/publisher';
1112
import { BucketType, ObjectStorageService } from '../../services/object-storage/object-storage-service';
@@ -48,6 +49,8 @@ describe('Malware Scan Worker', function () {
4849
let db: Knex;
4950
let storageService: ObjectStorageService;
5051
const createdArtifactIds: number[] = [];
52+
const createdUploadIds: string[] = [];
53+
const createdSubmissionIds: number[] = [];
5154
const createdObjectKeys: string[] = [];
5255

5356
before(async () => {
@@ -65,6 +68,7 @@ describe('Malware Scan Worker', function () {
6568
storageService = new ObjectStorageService();
6669
// Stub the downstream job publisher so the scan doesn't chain into validation
6770
sinon.stub(publisher, 'publishProcessSubmissionFeaturesJob').resolves();
71+
initDBPool(defaultPoolConfig);
6872
await initPgBoss();
6973
});
7074

@@ -80,8 +84,16 @@ describe('Malware Scan Worker', function () {
8084
)
8185
.del();
8286
await db('biohub.artifact_security').whereIn('artifact_id', createdArtifactIds).del();
87+
await db('biohub.upload_archive').whereIn('artifact_id', createdArtifactIds).del();
8388
await db('biohub.artifact').whereIn('artifact_id', createdArtifactIds).del();
8489
}
90+
if (createdUploadIds.length > 0) {
91+
await db('biohub.submission_upload').whereIn('upload_id', createdUploadIds).del();
92+
await db('biohub.upload').whereIn('upload_id', createdUploadIds).del();
93+
}
94+
if (createdSubmissionIds.length > 0) {
95+
await db('biohub.submission').whereIn('submission_id', createdSubmissionIds).del();
96+
}
8597
for (const key of createdObjectKeys) {
8698
try {
8799
await storageService.deleteFile(BucketType.QUARANTINE, key);
@@ -125,6 +137,42 @@ describe('Malware Scan Worker', function () {
125137
.returning('artifact_id');
126138
createdArtifactIds.push(artifact.artifact_id);
127139

140+
// Create upload + upload_archive + submission_upload (mirrors completeArchiveUpload)
141+
const [upload] = await db('biohub.upload')
142+
.insert({
143+
upload_status: 'completed',
144+
record_end_date: '9999-12-31',
145+
create_user: SYSTEM_USER_ID
146+
})
147+
.returning('upload_id');
148+
createdUploadIds.push(upload.upload_id);
149+
150+
await db('biohub.upload_archive').insert({
151+
upload_id: upload.upload_id,
152+
artifact_id: artifact.artifact_id,
153+
archive_status: 'blocked',
154+
create_user: SYSTEM_USER_ID
155+
});
156+
157+
const [submission] = await db('biohub.submission')
158+
.insert({
159+
uuid: db.raw('gen_random_uuid()'),
160+
system_user_id: SYSTEM_USER_ID,
161+
source_system: 'SIMS',
162+
name: 'integration-test',
163+
description: 'integration-test',
164+
comment: '',
165+
create_user: SYSTEM_USER_ID
166+
})
167+
.returning('submission_id');
168+
createdSubmissionIds.push(submission.submission_id);
169+
170+
await db('biohub.submission_upload').insert({
171+
submission_id: submission.submission_id,
172+
upload_id: upload.upload_id,
173+
create_user: SYSTEM_USER_ID
174+
});
175+
128176
const [security] = await db('biohub.artifact_security')
129177
.insert({
130178
artifact_id: artifact.artifact_id,
@@ -133,7 +181,14 @@ describe('Malware Scan Worker', function () {
133181
})
134182
.returning('artifact_security_id');
135183

136-
await publisher.publishMalwareScanJob({ artifactSecurityId: security.artifact_security_id });
184+
const connection = getAPIUserDBConnection();
185+
try {
186+
await connection.open();
187+
await publisher.publishMalwareScanJob(connection, { artifactSecurityId: security.artifact_security_id });
188+
await connection.commit();
189+
} finally {
190+
connection.release();
191+
}
137192
const result = await waitForScanResult(db, security.artifact_security_id);
138193

139194
return { result, objectKey };

api/src/queue/publisher.test.ts

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -238,8 +238,9 @@ describe('publisher', () => {
238238

239239
sinon.stub(pgBossService, 'getPgBoss').returns(mockBoss as any);
240240

241+
const mockConnection = getMockDBConnection();
241242
const data = { artifactSecurityId: 'artifact-security-123' };
242-
const result = await publishMalwareScanJob(data);
243+
const result = await publishMalwareScanJob(mockConnection, data);
243244

244245
expect(createQueueStub.calledOnce).to.be.true;
245246
expect(createQueueStub.firstCall.args[0]).to.equal(JobQueues.MALWARE_SCAN);
@@ -257,7 +258,7 @@ describe('publisher', () => {
257258

258259
sinon.stub(pgBossService, 'getPgBoss').returns(mockBoss as any);
259260

260-
await publishMalwareScanJob({ artifactSecurityId: 'artifact-security-456' });
261+
await publishMalwareScanJob(getMockDBConnection(), { artifactSecurityId: 'artifact-security-456' });
261262

262263
const options = sendStub.firstCall.args[2];
263264
expect(options.retryLimit).to.equal(3);
@@ -273,20 +274,36 @@ describe('publisher', () => {
273274

274275
sinon.stub(pgBossService, 'getPgBoss').returns(mockBoss as any);
275276

276-
await publishMalwareScanJob({ artifactSecurityId: '123' });
277+
await publishMalwareScanJob(getMockDBConnection(), { artifactSecurityId: '123' });
277278

278279
const options = sendStub.firstCall.args[2];
279280
expect(options.singletonKey).to.equal('artifact-security-123');
280281
});
281282

283+
it('passes db option to boss.send for transactional publishing', async () => {
284+
const sendStub = sinon.stub().resolves('scan-job-id');
285+
const createQueueStub = sinon.stub().resolves();
286+
const mockBoss = { send: sendStub, createQueue: createQueueStub };
287+
288+
sinon.stub(pgBossService, 'getPgBoss').returns(mockBoss as any);
289+
290+
await publishMalwareScanJob(getMockDBConnection(), { artifactSecurityId: 'tx-test-123' });
291+
292+
const options = sendStub.firstCall.args[2];
293+
expect(options.db).to.exist;
294+
expect(options.db.executeSql).to.be.a('function');
295+
});
296+
282297
it('returns duplicate status when send returns null', async () => {
283298
const sendStub = sinon.stub().resolves(null);
284299
const createQueueStub = sinon.stub().resolves();
285300
const mockBoss = { send: sendStub, createQueue: createQueueStub };
286301

287302
sinon.stub(pgBossService, 'getPgBoss').returns(mockBoss as any);
288303

289-
const result = await publishMalwareScanJob({ artifactSecurityId: 'artifact-security-999' });
304+
const result = await publishMalwareScanJob(getMockDBConnection(), {
305+
artifactSecurityId: 'artifact-security-999'
306+
});
290307

291308
expect(result.status).to.equal('duplicate');
292309
expect((result as { status: 'duplicate'; message: string }).message).to.equal(
@@ -297,7 +314,9 @@ describe('publisher', () => {
297314
it('returns error status when pg-boss throws', async () => {
298315
sinon.stub(pgBossService, 'getPgBoss').throws(new Error('pg-boss not initialized'));
299316

300-
const result = await publishMalwareScanJob({ artifactSecurityId: 'artifact-security-000' });
317+
const result = await publishMalwareScanJob(getMockDBConnection(), {
318+
artifactSecurityId: 'artifact-security-000'
319+
});
301320

302321
expect(result.status).to.equal('error');
303322
expect((result as { status: 'error'; message: string }).message).to.equal('pg-boss not initialized');

api/src/queue/publisher.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,11 +182,13 @@ export const publishProcessSubmissionFeaturesJob = async (
182182
*
183183
* Queues ClamAV scanning for an uploaded artifact.
184184
*
185+
* @param {IDBConnection} connection Database connection for submission validation tracking
185186
* @param {IMalwareScanJobData} data Job data containing artifactSecurityId
186187
* @param {IPublishOptions} [options={}] Job options
187188
* @return {*} {Promise<PublishJobResult>} Result indicating success, duplicate, or error
188189
*/
189190
export const publishMalwareScanJob = async (
191+
connection: IDBConnection,
190192
data: IMalwareScanJobData,
191193
options: IPublishOptions = {}
192194
): Promise<PublishJobResult> => {
@@ -196,10 +198,18 @@ export const publishMalwareScanJob = async (
196198

197199
await boss.createQueue(JobQueues.MALWARE_SCAN);
198200

201+
const db = {
202+
executeSql: async (text: string, values: any[]) => {
203+
const result = await connection.query(text, values);
204+
return { rows: result.rows, rowCount: result.rowCount };
205+
}
206+
};
207+
199208
// Use singletonKey to prevent duplicate concurrent jobs for the same artifact security record
200209
const jobId = await boss.send(JobQueues.MALWARE_SCAN, data, {
201210
...mergedOptions,
202-
singletonKey: `artifact-security-${data.artifactSecurityId}`
211+
singletonKey: `artifact-security-${data.artifactSecurityId}`,
212+
db
203213
});
204214

205215
if (jobId) {

api/src/repositories/upload/submission-upload-repository.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ export class SubmissionUploadRepository extends BaseRepository {
189189
* @param {string} uploadId - The upload_id to look up.
190190
* @returns {Promise<SubmissionUpload | null>} - The submission_upload record, or null if not found.
191191
*/
192-
async findSubmissionUploadByUploadId(uploadId: string): Promise<SubmissionUpload | null> {
192+
async getSubmissionUploadByUploadId(uploadId: string): Promise<SubmissionUpload> {
193193
const sqlStatement = SQL`
194194
SELECT
195195
submission_upload_id,
@@ -202,7 +202,15 @@ export class SubmissionUploadRepository extends BaseRepository {
202202
`;
203203

204204
const response = await this.connection.sql(sqlStatement, SubmissionUpload);
205-
return response.rows[0] ?? null;
205+
206+
if (response.rowCount !== 1) {
207+
throw new ApiExecuteSQLError('Failed to get submission upload record', [
208+
'SubmissionUploadRepository->getSubmissionUploadByUploadId',
209+
`rowCount was ${response.rowCount}, expected 1`
210+
]);
211+
}
212+
213+
return response.rows[0];
206214
}
207215

208216
/**

api/src/repositories/upload/upload-archive-repository.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ export class UploadArchiveRepository extends BaseRepository {
8585
* @param {string} artifactId - The ID of the artifact.
8686
* @returns {Promise<UploadArchive | null>} - The upload archive or null if not found.
8787
*/
88-
async getUploadArchiveByArtifactId(artifactId: string): Promise<UploadArchive | null> {
88+
async getUploadArchiveByArtifactId(artifactId: string): Promise<UploadArchive> {
8989
const sqlStatement = SQL`
9090
SELECT
9191
upload_archive_id,
@@ -99,7 +99,15 @@ export class UploadArchiveRepository extends BaseRepository {
9999
`;
100100

101101
const response = await this.connection.sql(sqlStatement, UploadArchive);
102-
return response.rows[0] ?? null;
102+
103+
if (response.rowCount !== 1) {
104+
throw new ApiExecuteSQLError('Failed to get upload archive record', [
105+
'UploadArchiveRepository->getUploadArchiveByArtifactId',
106+
`rowCount was ${response.rowCount}, expected 1`
107+
]);
108+
}
109+
110+
return response.rows[0];
103111
}
104112

105113
/**

0 commit comments

Comments
 (0)