From 8ec67e1c27b9d16466397b1d2e88d58753aff9d8 Mon Sep 17 00:00:00 2001 From: mike Date: Thu, 7 Mar 2024 16:53:36 -0800 Subject: [PATCH 1/4] fix(workspace setup): A few fixes to the workspace setup found while configuring a new mac --- docs/assets/setup.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/assets/setup.sh b/docs/assets/setup.sh index bad59366f8..3b33c90d98 100755 --- a/docs/assets/setup.sh +++ b/docs/assets/setup.sh @@ -67,6 +67,8 @@ if ! which brew > /dev/null ; then /bin/bash -c "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/HEAD/install.sh)" fi +export PATH="$homebrewprefix:$PATH" + # Install the AWS CLI, used to interact with any/all AWS services if ! which aws > /dev/null ; then brew install awscli session-manager-plugin @@ -136,6 +138,7 @@ export NVM_DIR="$HOME/.nvm" export PATH=/usr/local/go/bin:\$PATH export PATH=\$PATH:$(go env GOPATH)/bin +export PATH="$homebrewprefix/bin:\$PATH" eval \"\$($homebrewprefix/bin/brew shellenv)\" eval \"\$(direnv hook $shell)\" From 420e12f4b654a488478d973c62aa1e06a143629c Mon Sep 17 00:00:00 2001 From: Mike Dial Date: Thu, 7 Mar 2024 17:19:29 -0800 Subject: [PATCH 2/4] fix(kion): Add .kion.yml file --- docs/assets/setup.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/assets/setup.sh b/docs/assets/setup.sh index 3b33c90d98..49cdc09814 100755 --- a/docs/assets/setup.sh +++ b/docs/assets/setup.sh @@ -126,6 +126,7 @@ fi # Install kion-cli, a go package used to authenticate to Kion and access AWS go install github.com/kionsoftware/kion-cli@latest +touch ~/.kion.yml touch $macprorcfile echo """ From 935a34174a929930bc57a9555c77516676e29c48 Mon Sep 17 00:00:00 2001 From: Mike Dial Date: Sun, 10 Mar 2024 11:41:02 -0400 Subject: [PATCH 3/4] fix(uploads def trigger): Fix the download definitions trigger, which had a permission issue preventing success --- .../ui-infra/handlers/emptyBuckets.ts | 48 +++++++++--- src/services/ui-infra/serverless.yml | 1 + src/services/uploads/serverless.yml | 12 +-- src/services/uploads/src/emptyBuckets.ts | 78 +------------------ 4 files changed, 40 insertions(+), 99 deletions(-) diff --git a/src/services/ui-infra/handlers/emptyBuckets.ts b/src/services/ui-infra/handlers/emptyBuckets.ts index d4ea61c0f4..1b1a9d9431 100644 --- a/src/services/ui-infra/handlers/emptyBuckets.ts +++ b/src/services/ui-infra/handlers/emptyBuckets.ts @@ -10,25 +10,30 @@ const s3Client = new S3Client({ region: process.env.region }); export const handler: Handler = async (event, context) => { console.log("request:", JSON.stringify(event, undefined, 2)); - console.log("Request:", JSON.stringify(event, undefined, 2)); const responseData: any = {}; let responseStatus: ResponseStatus = SUCCESS; try { if (event.RequestType === "Create" || event.RequestType === "Update") { console.log("This resource does nothing on Create and Update events."); } else if (event.RequestType === "Delete") { - const buckets = event.ResourceProperties.Buckets; - for (const bucket of buckets) { - try { - const objects = await listObjectVersions(bucket); - await deleteObjectVersions(bucket, objects); - console.log( - `Successfully cleared all versions and delete markers in the S3 bucket: ${bucket}` - ); - } catch (error) { - console.error("Error in clearing the S3 bucket:", error); - throw error; // Rethrowing the error indicates failure in Lambda execution + const stage = process.env.stage; + if (isStageValid(stage)) { + console.log("Stage meets all specified criteria:", stage); + const buckets = event.ResourceProperties.Buckets; + for (const bucket of buckets) { + try { + const objects = await listObjectVersions(bucket); + await deleteObjectVersions(bucket, objects); + console.log( + `Successfully cleared all versions and delete markers in the S3 bucket: ${bucket}` + ); + } catch (error) { + console.error("Error in clearing the S3 bucket:", error); + throw error; // Rethrowing the error indicates failure in Lambda execution + } } + } else { + console.log("Delete is forbidden. Doing nothing."); } } } catch (error) { @@ -92,3 +97,22 @@ async function deleteObjectVersions( await s3Client.send(new DeleteObjectsCommand(deleteParams)); } } + +function isStageValid(stage: string | undefined): stage is string { + // Check if stage is not null, not undefined, is a string, and not an empty string + const isValidString = + stage !== null && + stage !== undefined && + typeof stage === "string" && + stage.trim() !== ""; + // Further checks to ensure stage is not one of the specified values and does not contain 'prod' + if (isValidString) { + const stageLower = stage.toLowerCase(); + const forbiddenValues = ["master", "val", "production"]; + const containsForbiddenString = + forbiddenValues.includes(stageLower) || /prod/.test(stageLower); + + return !containsForbiddenString; + } + return false; +} diff --git a/src/services/ui-infra/serverless.yml b/src/services/ui-infra/serverless.yml index 752e2b0653..b0004cdc57 100644 --- a/src/services/ui-infra/serverless.yml +++ b/src/services/ui-infra/serverless.yml @@ -12,6 +12,7 @@ provider: SERVICE: ${self:service} environment: region: ${self:provider.region} + stage: ${sls:stage} iam: role: path: /delegatedadmin/developer/ diff --git a/src/services/uploads/serverless.yml b/src/services/uploads/serverless.yml index 2037a681eb..5c8a3ce86e 100644 --- a/src/services/uploads/serverless.yml +++ b/src/services/uploads/serverless.yml @@ -92,6 +92,8 @@ functions: environment: CLAMAV_BUCKET_NAME: !Ref ClamDefsBucket PATH_TO_AV_DEFINITIONS: "lambda/s3-antivirus/av-definitions" + events: + - schedule: "cron(0 10 */1 * ? *)" triggerInitialDownload: handler: src/triggerInitialDownload.handler timeout: 300 # 300 seconds = 5 minutes @@ -167,16 +169,6 @@ resources: Principal: s3.amazonaws.com SourceAccount: ${aws:accountId} SourceArn: arn:aws:s3:::${self:service}-${sls:stage}-attachments-${aws:accountId} - AvDownloadDefinitionsEventsRuleSchedule1: - Type: AWS::Events::Rule - Properties: - ScheduleExpression: cron(0 10 */1 * ? *) - State: ENABLED - Targets: - - Arn: arn:aws:lambda:${self:provider.region}:${aws:accountId}:function:${self:custom.project}-${self:service}-avDownloadDefinitions - Id: avDownloadDefinitionsSchedule - DependsOn: - - EmptyBuckets ClamDefsBucket: Type: AWS::S3::Bucket Properties: diff --git a/src/services/uploads/src/emptyBuckets.ts b/src/services/uploads/src/emptyBuckets.ts index d4ea61c0f4..b5998c76b8 100644 --- a/src/services/uploads/src/emptyBuckets.ts +++ b/src/services/uploads/src/emptyBuckets.ts @@ -1,12 +1,6 @@ import { Handler } from "aws-lambda"; import { send, SUCCESS, FAILED } from "cfn-response-async"; type ResponseStatus = typeof SUCCESS | typeof FAILED; -import { - S3Client, - ListObjectVersionsCommand, - DeleteObjectsCommand, -} from "@aws-sdk/client-s3"; -const s3Client = new S3Client({ region: process.env.region }); export const handler: Handler = async (event, context) => { console.log("request:", JSON.stringify(event, undefined, 2)); @@ -14,23 +8,7 @@ export const handler: Handler = async (event, context) => { const responseData: any = {}; let responseStatus: ResponseStatus = SUCCESS; try { - if (event.RequestType === "Create" || event.RequestType === "Update") { - console.log("This resource does nothing on Create and Update events."); - } else if (event.RequestType === "Delete") { - const buckets = event.ResourceProperties.Buckets; - for (const bucket of buckets) { - try { - const objects = await listObjectVersions(bucket); - await deleteObjectVersions(bucket, objects); - console.log( - `Successfully cleared all versions and delete markers in the S3 bucket: ${bucket}` - ); - } catch (error) { - console.error("Error in clearing the S3 bucket:", error); - throw error; // Rethrowing the error indicates failure in Lambda execution - } - } - } + console.log("This resource does nothing, and will soon be removed."); } catch (error) { console.error(error); responseStatus = FAILED; @@ -38,57 +16,3 @@ export const handler: Handler = async (event, context) => { await send(event, context, responseStatus, responseData, "static"); } }; - -async function listObjectVersions( - bucketName: string -): Promise { - let isTruncated = true; - let keyMarker; - let versionIdMarker; - const objects: DeletableObject[] = []; - - while (isTruncated) { - const params = { - Bucket: bucketName, - KeyMarker: keyMarker, - VersionIdMarker: versionIdMarker, - }; - const response = await s3Client.send(new ListObjectVersionsCommand(params)); - response.Versions?.forEach((version) => - objects.push({ Key: version.Key!, VersionId: version.VersionId }) - ); - response.DeleteMarkers?.forEach((marker) => - objects.push({ Key: marker.Key!, VersionId: marker.VersionId }) - ); - - keyMarker = response.NextKeyMarker; - versionIdMarker = response.NextVersionIdMarker; - isTruncated = response.IsTruncated ?? false; - } - - return objects; -} - -type DeletableObject = { - Key: string; - VersionId?: string; -}; - -async function deleteObjectVersions( - bucketName: string, - objects: DeletableObject[] -) { - if (objects.length === 0) return; - - for (let i = 0; i < objects.length; i += 1000) { - const chunk = objects.slice(i, i + 1000); - const deleteParams = { - Bucket: bucketName, - Delete: { - Objects: chunk.map(({ Key, VersionId }) => ({ Key, VersionId })), - Quiet: true, - }, - }; - await s3Client.send(new DeleteObjectsCommand(deleteParams)); - } -} From 25e2cacf2acfca574d24f6a0a9ec7de18f4d8c96 Mon Sep 17 00:00:00 2001 From: Mike Dial <48921055+mdial89f@users.noreply.github.com> Date: Mon, 11 Mar 2024 05:40:33 -0400 Subject: [PATCH 4/4] fix(file type check): Modify how we determine the declared extension (#435) * corrections * fix(file type check): Modify how we determine the declared extension, from relying on aws added content-type to including extension in filename * allow jpg * mirror logic in frontend... reallyv etting the mime type is allowed based off the extension. consistency first --- src/services/api/handlers/getUploadUrl.ts | 15 ++- src/services/ui/src/api/submissionService.ts | 6 +- src/services/uploads/package.json | 3 +- src/services/uploads/src/antivirus.ts | 23 ++-- src/services/uploads/src/clamav.ts | 104 +++++++++++++++---- 5 files changed, 119 insertions(+), 32 deletions(-) diff --git a/src/services/api/handlers/getUploadUrl.ts b/src/services/api/handlers/getUploadUrl.ts index bdae4fe353..ad2599c92c 100644 --- a/src/services/api/handlers/getUploadUrl.ts +++ b/src/services/api/handlers/getUploadUrl.ts @@ -1,7 +1,9 @@ import { response } from "../libs/handler"; +import { APIGatewayEvent } from "aws-lambda"; import { S3Client, PutObjectCommand } from "@aws-sdk/client-s3"; import { getSignedUrl } from "@aws-sdk/s3-request-presigner"; import { v4 as uuidv4 } from "uuid"; +import * as path from "node:path"; checkEnvVariables(["attachmentsBucketName", "attachmentsBucketRegion"]); @@ -9,10 +11,19 @@ const s3 = new S3Client({ region: process.env.attachmentsBucketRegion, }); -export const handler = async () => { +export const handler = async (event: APIGatewayEvent) => { try { + if (!event.body) { + return response({ + statusCode: 400, + body: { message: "Event body required" }, + }); + } + const body = JSON.parse(event.body); const bucket = process.env.attachmentsBucketName; - const key = uuidv4(); + const fileName = body.fileName; + const extension = path.extname(fileName); + const key = `${uuidv4()}${extension}`; // ex: 123e4567-e89b-12d3-a456-426614174000.pdf const url = await getSignedUrl( s3, new PutObjectCommand({ diff --git a/src/services/ui/src/api/submissionService.ts b/src/services/ui/src/api/submissionService.ts index 8644aaa381..e1cb88879f 100644 --- a/src/services/ui/src/api/submissionService.ts +++ b/src/services/ui/src/api/submissionService.ts @@ -161,9 +161,11 @@ export const submit = async >({ ); // Generate a presigned url for each attachment const preSignedURLs: PreSignedURL[] = await Promise.all( - attachments.map(() => + attachments.map((attachment) => API.post("os", "/getUploadUrl", { - body: {}, + body: { + fileName: attachment.file.name, + }, }) ) ); diff --git a/src/services/uploads/package.json b/src/services/uploads/package.json index 741c8ee544..5d4acae174 100644 --- a/src/services/uploads/package.json +++ b/src/services/uploads/package.json @@ -9,6 +9,7 @@ "license": "CC0-1.0", "devDependencies": {}, "dependencies": { - "file-type": "^19.0.0" + "file-type": "^19.0.0", + "mime-types": "^2.1.35" } } diff --git a/src/services/uploads/src/antivirus.ts b/src/services/uploads/src/antivirus.ts index cacf958731..31e38cd92f 100644 --- a/src/services/uploads/src/antivirus.ts +++ b/src/services/uploads/src/antivirus.ts @@ -3,6 +3,7 @@ import { HeadObjectCommand, GetObjectCommand, PutObjectTaggingCommand, + HeadObjectCommandOutput, } from "@aws-sdk/client-s3"; import { randomUUID } from "crypto"; import fs from "fs"; @@ -22,12 +23,22 @@ export async function isS3FileTooBig( const res: HeadObjectCommandOutput = await s3Client.send( new HeadObjectCommand({ Key: key, Bucket: bucket }) ); - return res.ContentLength > constants.MAX_FILE_SIZE; + if ( + res.ContentLength === undefined || + res.ContentLength === null || + typeof res.ContentLength !== 'number' + ) { + utils.generateSystemMessage( + `ContentLength is invalid for S3 Object: s3://${bucket}/${key}` + ); + return false; // Or handle accordingly + } + return res.ContentLength > parseInt(constants.MAX_FILE_SIZE); } catch (e) { utils.generateSystemMessage( `Error finding size of S3 Object: s3://${bucket}/${key}` ); - return false; + return true; } } @@ -39,7 +50,7 @@ async function downloadFileFromS3( fs.mkdirSync(constants.TMP_DOWNLOAD_PATH); } - const localPath: string = `${constants.TMP_DOWNLOAD_PATH}${randomUUID()}.tmp`; + const localPath: string = `${constants.TMP_DOWNLOAD_PATH}${randomUUID()}--${s3ObjectKey}`; const writeStream: fs.WriteStream = fs.createWriteStream(localPath); utils.generateSystemMessage( @@ -88,11 +99,7 @@ const scanAndTagS3Object = async ( s3ObjectBucket ); utils.generateSystemMessage("Set virusScanStatus"); - const metadata = await s3Client.send(new HeadObjectCommand({ - Bucket: s3ObjectBucket, - Key: s3ObjectKey, - })); - virusScanStatus = await scanLocalFile(fileLoc, metadata.ContentType); + virusScanStatus = await scanLocalFile(fileLoc); utils.generateSystemMessage(`virusScanStatus=${virusScanStatus}`); } diff --git a/src/services/uploads/src/clamav.ts b/src/services/uploads/src/clamav.ts index d0789387d0..0e529a86ec 100644 --- a/src/services/uploads/src/clamav.ts +++ b/src/services/uploads/src/clamav.ts @@ -8,10 +8,13 @@ import { import { spawnSync, SpawnSyncReturns } from "child_process"; import path from "path"; import fs from "fs"; +import readline from 'readline'; import asyncfs from "fs/promises"; import * as constants from "./constants"; import * as utils from "./utils"; import { FileExtension, MimeType, fileTypeFromFile } from "file-type"; +import mimeTypes from 'mime-types'; +import {FILE_TYPES} from "shared-types" const s3Client: S3Client = new S3Client(); @@ -196,6 +199,40 @@ export const uploadAVDefinitions = async (): Promise => { return await Promise.all(uploadPromises); }; +async function looksLikeCsv(filePath: string, delimiter: string = ',', maxLinesToCheck: number = 10): Promise { + const fileStream = fs.createReadStream(filePath); + + const rl = readline.createInterface({ + input: fileStream, + crlfDelay: Infinity + }); + + let lineNumber = 0; + let previousNumberOfFields = 0; + + for await (const line of rl) { + lineNumber++; + + // Skip empty lines + if (line.trim() === '') continue; + + const fields = line.split(delimiter); + + // Check if the number of fields is consistent across rows + if (lineNumber > 1 && fields.length !== previousNumberOfFields) { + return false; + } + + previousNumberOfFields = fields.length; + + if (lineNumber >= maxLinesToCheck) { + break; + } + } + + return true; +} + /** * Function to scan the given file. This function requires ClamAV and the definitions to be available. * This function does not download the file so the file should also be accessible. @@ -209,24 +246,43 @@ export const uploadAVDefinitions = async (): Promise => { */ export const scanLocalFile = async ( pathToFile: string, - contentType: string | undefined ): Promise => { - try { - if (!contentType) { - utils.generateSystemMessage("FAILURE - EXTENSION UNKNOWN"); + try { + // Calculate the mime type based off the extension. + let mimeTypeFromExtension = mimeTypes.lookup(path.extname(pathToFile)); + + // Error out if mimeTypes couldn't figure out the mime type. + if (!mimeTypeFromExtension) { + utils.generateSystemMessage("FAILURE - CANNOT DETERMINE MIMETYPE FROM EXTENSION"); return constants.STATUS_UNKNOWN_EXTENSION; } - let detectedContentType = await getFileTypeFromContents(pathToFile); - if (detectedContentType) { - console.log(`File declared extension: ${contentType}`); - console.log(`File detected extension: ${detectedContentType}`); - let same = areMimeTypesEquivalent(contentType, detectedContentType); - if (!same) { - utils.generateSystemMessage( - `FAILURE - FILE EXTENSION DOES NOT MATCH FILE CONTENTS` - ); - return constants.STATUS_EXTENSION_MISMATCH_FILE; - } + + // Error out if the extension is not allowed + if (!isAllowedMime(mimeTypeFromExtension)) { + utils.generateSystemMessage("FAILURE - EXTENSION IS NOT OF AN ALLOWED TYPE"); + return constants.STATUS_UNKNOWN_EXTENSION; + } + + // Caclulate the mime type based off the file's contents. + let mimeTypeFromContents = await getFileTypeFromContents(pathToFile); + // Error out if file-type couldn't determine the mime type. + if(!mimeTypeFromContents) { + utils.generateSystemMessage("FAILURE - CANNOT DETERMINE MIMETYPE FROM CONTENTS"); + return constants.STATUS_UNKNOWN_EXTENSION; + } + + // Log + console.log(`File mimetype from extension: ${mimeTypeFromExtension}`); + console.log(`File mimetype from contents: ${mimeTypeFromContents}`); + + // Check if the mimes are equivalent + let same = areMimeTypesEquivalent(mimeTypeFromExtension, mimeTypeFromContents); + // Error out if we can't determine equivalence + if (!same) { + utils.generateSystemMessage( + `FAILURE - MIMETYPE CALCULATED FROM EXTENSION DOES NOT MATCH MIMETYPE CALCULATED FROM CONTENTS` + ); + return constants.STATUS_EXTENSION_MISMATCH_FILE; } const avResult: SpawnSyncReturns = spawnSync( @@ -257,9 +313,13 @@ export const scanLocalFile = async ( } }; +function isAllowedMime(mime: string): boolean { + return FILE_TYPES.some((fileType) => fileType.mime === mime); +} + async function getFileTypeFromContents( filePath: string -): Promise { +): Promise { try { const fileBuffer = await fs.promises.readFile(filePath); @@ -267,14 +327,19 @@ async function getFileTypeFromContents( const type = await fileTypeFromFile(filePath); if (!type) { + if(path.extname(filePath) == ".csv") { + if((await looksLikeCsv(filePath, ",", 100))) { + return mimeTypes.lookup(".csv") + } + } console.log("Could not determine file type."); - return null; + return false; } - console.log(`File type is ${type.mime} with extension ${type.ext}`); + console.log(`getFileTypeFromContents: File determined to be mime:${type.mime} ext:${type.ext}`); return type.mime; } catch (error) { console.error("Error reading file:", error); - return null; + return false; } } @@ -291,6 +356,7 @@ function areMimeTypesEquivalent(mime1: string, mime2: string): boolean { return true; } for (const baseType in equivalentTypes) { + console.log("Mime types not identical... checking AKAs for equivalence...") const equivalents = equivalentTypes[baseType]; if ( (mime1 === baseType && equivalents.has(mime2)) ||