Skip to content

Commit

Permalink
added various validation checks for rerunning file scan
Browse files Browse the repository at this point in the history
  • Loading branch information
ARADDCC002 committed Dec 4, 2024
1 parent e96d63d commit fb3fbb8
Show file tree
Hide file tree
Showing 20 changed files with 210 additions and 27 deletions.
1 change: 1 addition & 0 deletions backend/config/default.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,7 @@ module.exports = {

fileScanners: {
kinds: [],
retryDelayInMinutes: 60,
},
},

Expand Down
2 changes: 2 additions & 0 deletions backend/src/connectors/audit/Base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
Expand Down Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions backend/src/connectors/audit/__mocks__/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ const audit = {
onViewFiles: vi.fn(),
onDeleteFile: vi.fn(),
onCreateFile: vi.fn(),
onUpdateFile: vi.fn(),

onCreateRelease: vi.fn(),
onViewRelease: vi.fn(),
Expand Down
1 change: 1 addition & 0 deletions backend/src/connectors/audit/silly.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {}
Expand Down
6 changes: 6 additions & 0 deletions backend/src/connectors/audit/stdout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 })
Expand Down
2 changes: 2 additions & 0 deletions backend/src/connectors/authorisation/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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]

Expand Down Expand Up @@ -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,
Expand Down
10 changes: 9 additions & 1 deletion backend/src/connectors/fileScanning/Base.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { FileInterface, ScanStateKeys } from '../../models/File.js'
import { FileInterface } from '../../models/File.js'
export interface FileScanResult {
toolName: string
scannerVersion?: string
Expand All @@ -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<FileScanResult[]>
Expand Down
4 changes: 2 additions & 2 deletions backend/src/connectors/fileScanning/clamAv.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
4 changes: 2 additions & 2 deletions backend/src/connectors/fileScanning/modelScan.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand Down
10 changes: 1 addition & 9 deletions backend/src/models/File.ts
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand Down
42 changes: 40 additions & 2 deletions backend/src/services/file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand Down Expand Up @@ -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) {
Expand All @@ -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 })
Expand Down
1 change: 1 addition & 0 deletions backend/src/utils/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export interface Config {

fileScanners: {
kinds: FileScanKindKeys[]
retryDelayInMinutes: number
}
}

Expand Down
3 changes: 3 additions & 0 deletions backend/src/utils/string.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export const plural = (value: number, phrase: string) => {
return `${value} ${phrase}${value === 1 ? '' : 's'}`
}
10 changes: 10 additions & 0 deletions backend/test/models/Token.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
}))
Expand Down
10 changes: 10 additions & 0 deletions backend/test/routes/model/accessRequest/postAccessRequest.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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])),
Expand Down
58 changes: 57 additions & 1 deletion backend/test/services/file.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,15 @@ 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,
getFilesByIds,
getFilesByModel,
getTotalFileSize,
removeFile,
rerunFileScan,
uploadFile,
} from '../../src/services/file.js'

Expand Down Expand Up @@ -43,6 +44,7 @@ const configMock = vi.hoisted(
connectors: {
fileScanners: {
kinds: ['clamAV'],
retryDelayInMinutes: 5,
},
},
}) as any,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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/,
)
})
})
10 changes: 10 additions & 0 deletions backend/test/services/mirroredModel.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
}))
Expand Down
Loading

0 comments on commit fb3fbb8

Please sign in to comment.