diff --git a/backend/config/default.cjs b/backend/config/default.cjs index db97ed0fd..e95883815 100644 --- a/backend/config/default.cjs +++ b/backend/config/default.cjs @@ -222,6 +222,7 @@ module.exports = { fileScanners: { kinds: [], + retryDelayInMinutes: 60, }, }, diff --git a/backend/src/connectors/audit/Base.ts b/backend/src/connectors/audit/Base.ts index 2f23dd155..6cfeaaefb 100644 --- a/backend/src/connectors/audit/Base.ts +++ b/backend/src/connectors/audit/Base.ts @@ -40,6 +40,7 @@ export const AuditInfo = { ViewFile: { typeId: 'ViewFile', description: 'File Downloaded', auditKind: AuditKind.View }, ViewFiles: { typeId: 'ViewFiles', description: 'File Information Viewed', auditKind: AuditKind.View }, DeleteFile: { typeId: 'DeleteFile', description: 'File Information Deleted', auditKind: AuditKind.Delete }, + UpdateFile: { typeId: 'UpdateFile', description: 'File Information Updated', auditKind: AuditKind.Update }, CreateRelease: { typeId: 'CreateRelease', description: 'Release Created', auditKind: AuditKind.Create }, ViewRelease: { typeId: 'ViewRelease', description: 'Release Viewed', auditKind: AuditKind.View }, @@ -138,6 +139,7 @@ export abstract class BaseAuditConnector { abstract onViewFile(req: Request, file: FileInterfaceDoc) abstract onViewFiles(req: Request, modelId: string, files: FileInterface[]) abstract onDeleteFile(req: Request, modelId: string, fileId: string) + abstract onUpdateFile(req: Request, modelId: string, fileId: string) abstract onCreateRelease(req: Request, release: ReleaseDoc) abstract onViewRelease(req: Request, release: ReleaseDoc) diff --git a/backend/src/connectors/audit/__mocks__/index.ts b/backend/src/connectors/audit/__mocks__/index.ts index 87b79c352..6c35ef7a6 100644 --- a/backend/src/connectors/audit/__mocks__/index.ts +++ b/backend/src/connectors/audit/__mocks__/index.ts @@ -14,6 +14,7 @@ const audit = { onViewFiles: vi.fn(), onDeleteFile: vi.fn(), onCreateFile: vi.fn(), + onUpdateFile: vi.fn(), onCreateRelease: vi.fn(), onViewRelease: vi.fn(), diff --git a/backend/src/connectors/audit/silly.ts b/backend/src/connectors/audit/silly.ts index c7f26cd7a..2742f909e 100644 --- a/backend/src/connectors/audit/silly.ts +++ b/backend/src/connectors/audit/silly.ts @@ -32,6 +32,7 @@ export class SillyAuditConnector extends BaseAuditConnector { onViewFile(_req: Request, _file: FileInterfaceDoc) {} onViewFiles(_req: Request, _modelId: string, _files: FileInterface[]) {} onDeleteFile(_req: Request, _modelId: string, _fileId: string) {} + onUpdateFile(_req: Request, _modelId: string, _fileId: string) {} onCreateRelease(_req: Request, _release: ReleaseDoc) {} onViewRelease(_req: Request, _release: ReleaseDoc) {} onUpdateRelease(_req: Request, _release: ReleaseDoc) {} diff --git a/backend/src/connectors/audit/stdout.ts b/backend/src/connectors/audit/stdout.ts index 58365c8a8..7977d3297 100644 --- a/backend/src/connectors/audit/stdout.ts +++ b/backend/src/connectors/audit/stdout.ts @@ -101,6 +101,12 @@ export class StdoutAuditConnector extends BaseAuditConnector { req.log.info(event, req.audit.description) } + onUpdateFile(req: Request, modelId: string, fileId: string) { + this.checkEventType(AuditInfo.UpdateFile, req) + const event = this.generateEvent(req, { modelId, fileId }) + req.log.info(event, req.audit.description) + } + onCreateRelease(req: Request, release: ReleaseDoc) { this.checkEventType(AuditInfo.CreateRelease, req) const event = this.generateEvent(req, { modelId: release.modelId, semver: release.semver }) diff --git a/backend/src/connectors/authorisation/actions.ts b/backend/src/connectors/authorisation/actions.ts index 8093b2c94..23d534436 100644 --- a/backend/src/connectors/authorisation/actions.ts +++ b/backend/src/connectors/authorisation/actions.ts @@ -37,6 +37,7 @@ export const FileAction = { // 'view' refers to the ability to see metadata about the file. 'download' lets the user view the file contents. View: 'file:view', Download: 'file:download', + Update: 'file:update', } as const export type FileActionKeys = (typeof FileAction)[keyof typeof FileAction] @@ -80,6 +81,7 @@ export const ActionLookup = { [FileAction.Upload]: TokenActions.FileWrite.id, [FileAction.View]: TokenActions.FileRead.id, [FileAction.Download]: TokenActions.FileRead.id, + [FileAction.Update]: TokenActions.FileWrite.id, [ImageAction.Pull]: TokenActions.ImageRead.id, [ImageAction.Push]: TokenActions.ImageWrite.id, diff --git a/backend/src/connectors/fileScanning/Base.ts b/backend/src/connectors/fileScanning/Base.ts index e32a55617..50a5ee248 100644 --- a/backend/src/connectors/fileScanning/Base.ts +++ b/backend/src/connectors/fileScanning/Base.ts @@ -1,4 +1,4 @@ -import { FileInterface, ScanStateKeys } from '../../models/File.js' +import { FileInterface } from '../../models/File.js' export interface FileScanResult { toolName: string scannerVersion?: string @@ -8,6 +8,14 @@ export interface FileScanResult { lastRunAt: Date } +export const ScanState = { + NotScanned: 'notScanned', + InProgress: 'inProgress', + Complete: 'complete', + Error: 'error', +} as const +export type ScanStateKeys = (typeof ScanState)[keyof typeof ScanState] + export abstract class BaseFileScanningConnector { abstract info(): string[] abstract scan(file: FileInterface): Promise diff --git a/backend/src/connectors/fileScanning/clamAv.ts b/backend/src/connectors/fileScanning/clamAv.ts index 3de6b58e1..4096536c8 100644 --- a/backend/src/connectors/fileScanning/clamAv.ts +++ b/backend/src/connectors/fileScanning/clamAv.ts @@ -2,11 +2,11 @@ import NodeClam from 'clamscan' import { Readable } from 'stream' import { getObjectStream } from '../../clients/s3.js' -import { FileInterfaceDoc, ScanState } from '../../models/File.js' +import { FileInterfaceDoc } from '../../models/File.js' import log from '../../services/log.js' import config from '../../utils/config.js' import { ConfigurationError } from '../../utils/error.js' -import { BaseFileScanningConnector, FileScanResult } from './Base.js' +import { BaseFileScanningConnector, FileScanResult, ScanState } from './Base.js' let av: NodeClam export const clamAvToolName = 'Clam AV' diff --git a/backend/src/connectors/fileScanning/modelScan.ts b/backend/src/connectors/fileScanning/modelScan.ts index 9d2121f83..da9fe4ed7 100644 --- a/backend/src/connectors/fileScanning/modelScan.ts +++ b/backend/src/connectors/fileScanning/modelScan.ts @@ -3,11 +3,11 @@ import { Readable } from 'stream' import { getModelScanInfo, scanFile } from '../../clients/modelScan.js' import { getObjectStream } from '../../clients/s3.js' -import { FileInterfaceDoc, ScanState } from '../../models/File.js' +import { FileInterfaceDoc } from '../../models/File.js' import log from '../../services/log.js' import config from '../../utils/config.js' import { ConfigurationError } from '../../utils/error.js' -import { BaseFileScanningConnector, FileScanResult } from './Base.js' +import { BaseFileScanningConnector, FileScanResult, ScanState } from './Base.js' export const modelScanToolName = 'ModelScan' diff --git a/backend/src/models/File.ts b/backend/src/models/File.ts index b6e5a5f17..c785c714a 100644 --- a/backend/src/models/File.ts +++ b/backend/src/models/File.ts @@ -1,7 +1,7 @@ import { Document, model, ObjectId, Schema } from 'mongoose' import MongooseDelete from 'mongoose-delete' -import { FileScanResult } from '../connectors/fileScanning/Base.js' +import { FileScanResult, ScanState } from '../connectors/fileScanning/Base.js' // This interface stores information about the properties on the base object. // It should be used for plain object representations, e.g. for sending to the @@ -25,14 +25,6 @@ export interface FileInterface { updatedAt: Date } -export const ScanState = { - NotScanned: 'notScanned', - InProgress: 'inProgress', - Complete: 'complete', - Error: 'error', -} as const -export type ScanStateKeys = (typeof ScanState)[keyof typeof ScanState] - // The doc type includes all values in the plain interface, as well as all the // properties and functions that Mongoose provides. If a function takes in an // object from Mongoose it should use this interface diff --git a/backend/src/services/file.ts b/backend/src/services/file.ts index 32328a142..ae011bf76 100644 --- a/backend/src/services/file.ts +++ b/backend/src/services/file.ts @@ -4,13 +4,14 @@ import { Readable } from 'stream' import { getObjectStream, putObjectStream } from '../clients/s3.js' import { FileAction, ModelAction } from '../connectors/authorisation/actions.js' import authorisation from '../connectors/authorisation/index.js' -import { FileScanResult } from '../connectors/fileScanning/Base.js' +import { FileScanResult, ScanState } from '../connectors/fileScanning/Base.js' import scanners from '../connectors/fileScanning/index.js' -import FileModel, { ScanState } from '../models/File.js' +import FileModel, { FileInterface } from '../models/File.js' import { UserInterface } from '../models/User.js' import config from '../utils/config.js' import { BadReq, Forbidden, NotFound } from '../utils/error.js' import { longId } from '../utils/id.js' +import { plural } from '../utils/string.js' import { getModelById } from './model.js' import { removeFileFromReleases } from './release.js' @@ -157,6 +158,31 @@ export async function getTotalFileSize(fileIds: string[]) { return totalSize.at(0).totalSize } +function fileScanDelay(file: FileInterface): number { + const delay = config.connectors.fileScanners.retryDelayInMinutes + if (delay === undefined) { + return 0 + } + let minutesBeforeRetrying = 0 + for (const scanResult of file.avScan) { + const delayInMilliseconds = delay * 60000 + const scanTimeAtLimit = scanResult.lastRunAt.getTime() + delayInMilliseconds + if (scanTimeAtLimit > new Date().getTime()) { + minutesBeforeRetrying = scanTimeAtLimit - new Date().getTime() + break + } + } + return Math.round(minutesBeforeRetrying / 60000) +} + +function checkIfScanInProgress(file: FileInterface) { + file.avScan.forEach((scanResult) => { + if (scanResult.state === ScanState.InProgress) { + throw BadReq('File scan is currently in progress, please wait before retrying') + } + }) +} + export async function rerunFileScan(user: UserInterface, modelId, fileId: string) { const model = await getModelById(user, modelId) if (!model) { @@ -166,6 +192,18 @@ export async function rerunFileScan(user: UserInterface, modelId, fileId: string if (!file) { throw BadReq('Cannot find requested file', { modelId: modelId, fileId: fileId }) } + const rerunFileScanAuth = await authorisation.file(user, modelId, file, FileAction.Update) + if (!rerunFileScanAuth.success) { + throw Forbidden(rerunFileScanAuth.info, { userDn: user.dn, modelId, file }) + } + if (!file.size || file.size === 0) { + throw BadReq('Cannot run scan on an empty file') + } + checkIfScanInProgress(file) + const minutesBeforeRescanning = fileScanDelay(file) + if (minutesBeforeRescanning > 0) { + throw BadReq(`Please wait ${plural(minutesBeforeRescanning, 'minute')} before attempting a rescan ${file.name}`) + } const auth = await authorisation.model(user, model, ModelAction.Update) if (!auth.success) { throw Forbidden(auth.info, { userDn: user.dn }) diff --git a/backend/src/utils/config.ts b/backend/src/utils/config.ts index 065ab8572..da5fec1ae 100644 --- a/backend/src/utils/config.ts +++ b/backend/src/utils/config.ts @@ -41,6 +41,7 @@ export interface Config { fileScanners: { kinds: FileScanKindKeys[] + retryDelayInMinutes: number } } diff --git a/backend/src/utils/string.ts b/backend/src/utils/string.ts new file mode 100644 index 000000000..f70fdf76f --- /dev/null +++ b/backend/src/utils/string.ts @@ -0,0 +1,3 @@ +export const plural = (value: number, phrase: string) => { + return `${value} ${phrase}${value === 1 ? '' : 's'}` +} diff --git a/backend/test/models/Token.spec.ts b/backend/test/models/Token.spec.ts index 096d7385e..d8eb4238c 100644 --- a/backend/test/models/Token.spec.ts +++ b/backend/test/models/Token.spec.ts @@ -7,6 +7,16 @@ const bcryptMocks = vi.hoisted(() => ({ })) vi.mock('bcryptjs', () => ({ default: bcryptMocks })) +const baseScannerMock = vi.hoisted(() => ({ + ScanState: { + NotScanned: 'notScanned', + InProgress: 'inProgress', + Complete: 'complete', + Error: 'error', + }, +})) +vi.mock('../../src/connectors/filescanning/Base.js', () => baseScannerMock) + const sha256Mocks = vi.hoisted(() => ({ digest: vi.fn(), })) diff --git a/backend/test/routes/model/accessRequest/postAccessRequest.spec.ts b/backend/test/routes/model/accessRequest/postAccessRequest.spec.ts index dee4bb3a1..27bd46196 100644 --- a/backend/test/routes/model/accessRequest/postAccessRequest.spec.ts +++ b/backend/test/routes/model/accessRequest/postAccessRequest.spec.ts @@ -20,6 +20,16 @@ const fileScanResult: FileScanResult = { toolName: 'Test', } +const baseScannerMock = vi.hoisted(() => ({ + ScanState: { + NotScanned: 'notScanned', + InProgress: 'inProgress', + Complete: 'complete', + Error: 'error', + }, +})) +vi.mock('../../src/connectors/filescanning/Base.js', () => baseScannerMock) + const fileScanningMock = vi.hoisted(() => ({ info: vi.fn(() => []), scan: vi.fn(() => new Promise(() => [fileScanResult])), diff --git a/backend/test/services/file.spec.ts b/backend/test/services/file.spec.ts index c13c1809c..04c70bd65 100644 --- a/backend/test/services/file.spec.ts +++ b/backend/test/services/file.spec.ts @@ -3,7 +3,7 @@ import { describe, expect, test, vi } from 'vitest' import { FileAction } from '../../src/connectors/authorisation/actions.js' import authorisation from '../../src/connectors/authorisation/index.js' -import { FileScanResult } from '../../src/connectors/fileScanning/Base.js' +import { FileScanResult, ScanState } from '../../src/connectors/fileScanning/Base.js' import { UserInterface } from '../../src/models/User.js' import { downloadFile, @@ -11,6 +11,7 @@ import { getFilesByModel, getTotalFileSize, removeFile, + rerunFileScan, uploadFile, } from '../../src/services/file.js' @@ -43,6 +44,7 @@ const configMock = vi.hoisted( connectors: { fileScanners: { kinds: ['clamAV'], + retryDelayInMinutes: 5, }, }, }) as any, @@ -100,6 +102,16 @@ const fileModelMocks = vi.hoisted(() => { }) vi.mock('../../src/models/File.js', () => ({ default: fileModelMocks })) +const baseScannerMock = vi.hoisted(() => ({ + ScanState: { + NotScanned: 'notScanned', + InProgress: 'inProgress', + Complete: 'complete', + Error: 'error', + }, +})) +vi.mock('../../src/connectors/filescanning/Base.js', () => baseScannerMock) + const clamscan = vi.hoisted(() => ({ on: vi.fn() })) vi.mock('clamscan', () => ({ __esModule: true, @@ -347,4 +359,48 @@ describe('services > file', () => { expect(size).toBe(42) }) + + test('rerunFileScan > successfully reruns a file scan', async () => { + const createdAtTimeInMilliseconds = new Date().getTime() - 2000000 + fileModelMocks.findOne.mockResolvedValueOnce({ + name: 'file.txt', + avScan: [{ state: ScanState.Complete, lastRunAt: new Date(createdAtTimeInMilliseconds) }], + size: 123, + }) + const scanStatus = await rerunFileScan({} as any, 'model123', 'file123') + expect(scanStatus).toBe('Scan started for file.txt') + }) + + test('rerunFileScan > throws bad request when attemtping to upload an empty file', async () => { + fileModelMocks.findOne.mockResolvedValueOnce({ + name: 'file.txt', + avScan: [{ state: ScanState.Complete }], + size: 0, + }) + await expect(rerunFileScan({} as any, 'model123', 'file123')).rejects.toThrowError( + /^Cannot run scan on an empty file/, + ) + }) + + test('rerunFileScan > does not rerun file scan when scan is in progress', async () => { + fileModelMocks.findOne.mockResolvedValueOnce({ + name: 'file.txt', + avScan: [{ state: ScanState.InProgress }], + size: 123, + }) + await expect(rerunFileScan({} as any, 'model123', 'file123')).rejects.toThrowError( + /^File scan is currently in progress, please wait before retrying/, + ) + }) + + test('rerunFileScan > does not rerun file scan before delay is over', async () => { + fileModelMocks.findOne.mockResolvedValueOnce({ + name: 'file.txt', + avScan: [{ state: ScanState.Complete, lastRunAt: new Date() }], + size: 123, + }) + await expect(rerunFileScan({} as any, 'model123', 'file123')).rejects.toThrowError( + /^Please wait 5 minutes before attempting a rescan file.txt/, + ) + }) }) diff --git a/backend/test/services/mirroredModel.spec.ts b/backend/test/services/mirroredModel.spec.ts index 666a952c4..600decc77 100644 --- a/backend/test/services/mirroredModel.spec.ts +++ b/backend/test/services/mirroredModel.spec.ts @@ -24,6 +24,16 @@ const fflateMock = vi.hoisted(() => ({ })) vi.mock('fflate', async () => fflateMock) +const baseScannerMock = vi.hoisted(() => ({ + ScanState: { + NotScanned: 'notScanned', + InProgress: 'inProgress', + Complete: 'complete', + Error: 'error', + }, +})) +vi.mock('../../src/connectors/filescanning/Base.js', () => baseScannerMock) + const bufferMock = vi.hoisted(() => ({ unzipSync: vi.fn(), })) diff --git a/frontend/src/entry/model/releases/FileDownload.tsx b/frontend/src/entry/model/releases/FileDownload.tsx index d96e9835d..2378bcbfd 100644 --- a/frontend/src/entry/model/releases/FileDownload.tsx +++ b/frontend/src/entry/model/releases/FileDownload.tsx @@ -4,9 +4,11 @@ import { rerunFileScan, useGetFileScannerInfo } from 'actions/fileScanning' import prettyBytes from 'pretty-bytes' import { Fragment, ReactElement, useCallback, useMemo, useState } from 'react' import Loading from 'src/common/Loading' +import useNotification from 'src/hooks/useNotification' import MessageAlert from 'src/MessageAlert' import { FileInterface, isFileInterface, ScanState } from 'types/types' import { formatDateString } from 'utils/dateUtils' +import { getErrorMessage } from 'utils/fetcher' import { plural } from 'utils/stringUtils' type FileDownloadProps = { @@ -23,6 +25,7 @@ interface ChipDetails { export default function FileDownload({ modelId, file }: FileDownloadProps) { const [anchorEl, setAnchorEl] = useState(null) + const sendNotification = useNotification() const { scanners, isScannersLoading, isScannersError } = useGetFileScannerInfo() const open = Boolean(anchorEl) @@ -53,16 +56,53 @@ export default function FileDownload({ modelId, file }: FileDownloadProps) { return { label: 'Virus scan passed', colour: 'success', icon: } }, []) + const handleRerunFileScanOnClick = useCallback(async () => { + const res = await rerunFileScan(modelId, (file as FileInterface)._id) + if (!res.ok) { + sendNotification({ + variant: 'error', + msg: await getErrorMessage(res), + anchorOrigin: { horizontal: 'center', vertical: 'bottom' }, + }) + } else { + sendNotification({ + variant: 'success', + msg: `${file.name} is being rescanned`, + anchorOrigin: { horizontal: 'center', vertical: 'bottom' }, + }) + } + }, [file, modelId, sendNotification]) + + const rerunFileScanButton = useMemo(() => { + return ( + + + + + + ) + }, [handleRerunFileScanOnClick]) + const avChip = useMemo(() => { if ( !isFileInterface(file) || file.avScan === undefined || file.avScan.every((scan) => scan.state === ScanState.NotScanned) ) { - return + return ( + + + {rerunFileScanButton} + + ) } if (file.avScan.some((scan) => scan.state === ScanState.InProgress)) { - return + return ( + + + {rerunFileScanButton} + + ) } return ( <> @@ -74,11 +114,7 @@ export default function FileDownload({ modelId, file }: FileDownloadProps) { onClick={(e) => setAnchorEl(e.currentTarget)} label={chipDetails(file).label} /> - - rerunFileScan(modelId, file._id)}> - - - + {rerunFileScanButton} ( {scanResult.isInfected ? ( - <> + {scanResult.toolName} found the following threats: + {scanResult.scannerVersion && ( + + )} + Last ran at: {formatDateString(scanResult.lastRunAt)}
    {scanResult.viruses && scanResult.viruses.map((virus) =>
  • {virus}
  • )}
- +
) : ( @@ -127,7 +167,7 @@ export default function FileDownload({ modelId, file }: FileDownloadProps) {
) - }, [anchorEl, chipDetails, file, modelId, open]) + }, [anchorEl, chipDetails, file, open, rerunFileScanButton]) if (isScannersError) { return diff --git a/infrastructure/helm/bailo/templates/bailo/bailo.configmap.yaml b/infrastructure/helm/bailo/templates/bailo/bailo.configmap.yaml index c4f19bfb7..5b442f6ab 100644 --- a/infrastructure/helm/bailo/templates/bailo/bailo.configmap.yaml +++ b/infrastructure/helm/bailo/templates/bailo/bailo.configmap.yaml @@ -200,6 +200,7 @@ data: fileScanners: { kinds: {{ toJson .Values.connectors.fileScanners.kinds }} + retryDelayInMinutes: {{ .Values.connectors.fileScanners.retryDelayInMinutes}} }, }, diff --git a/infrastructure/helm/bailo/values.yaml b/infrastructure/helm/bailo/values.yaml index f0dac1160..f7c0ba9bb 100644 --- a/infrastructure/helm/bailo/values.yaml +++ b/infrastructure/helm/bailo/values.yaml @@ -261,6 +261,7 @@ connectors: fileScanners: kinds: [] + retryDelayInMinutes: 60 instrumentation: enabled: false