diff --git a/CHANGELOG.md b/CHANGELOG.md index 34fd63269..1f5c6b738 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,11 +5,24 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [6.3.3] - 2024-12-27 + +### Fixed +- Overlays not checking for valid S3 buckets +- Failures when updating deployments created in version 6.1.0 and prior [#559](https://github.com/aws-solutions/serverless-image-handler/issues/559) + +### Security + +- Added allowlist on sharp operations. [Info](https://docs.aws.amazon.com/solutions/latest/serverless-image-handler/create-and-use-image-requests.html#restricted-operations) +- Added deny list on custom headers for base64 encoded requests. [Info](https://docs.aws.amazon.com/solutions/latest/serverless-image-handler/create-and-use-image-requests.html#include-custom-response-headers) +- Added inference of Content-Type header if S3 Metadata provides an unsupported value + ## [6.3.2] - 2024-11-22 ### Fixed - Upgrade cross-spawn to v7.0.6 for vulnerability [CVE-2024-9506](https://github.com/advisories/GHSA-5j4c-8p2g-v4jx) + ## [6.3.1] - 2024-10-02 ### Fixed diff --git a/VERSION.txt b/VERSION.txt index f9da12e11..d9b300f1c 100644 --- a/VERSION.txt +++ b/VERSION.txt @@ -1 +1 @@ -6.3.2 \ No newline at end of file +6.3.3 \ No newline at end of file diff --git a/source/constructs/cdk.json b/source/constructs/cdk.json index 3c27b908c..b8658363a 100644 --- a/source/constructs/cdk.json +++ b/source/constructs/cdk.json @@ -2,7 +2,7 @@ "app": "npx ts-node --prefer-ts-exts bin/constructs.ts", "context": { "solutionId": "SO0023", - "solutionVersion": "custom-v6.3.2", + "solutionVersion": "custom-v6.3.3", "solutionName": "serverless-image-handler" } } \ No newline at end of file diff --git a/source/constructs/lib/common-resources/common-resources-construct.ts b/source/constructs/lib/common-resources/common-resources-construct.ts index 988605850..e3f84ff56 100644 --- a/source/constructs/lib/common-resources/common-resources-construct.ts +++ b/source/constructs/lib/common-resources/common-resources-construct.ts @@ -27,6 +27,7 @@ export interface AppRegistryApplicationProps { readonly description: string; readonly solutionId: string; readonly applicationName: string; + readonly solutionName: string; readonly solutionVersion: string; } @@ -91,13 +92,13 @@ export class CommonResources extends Construct { const applicationType = "AWS-Solutions"; const application = new appreg.Application(stack, "AppRegistry", { - applicationName: Fn.join("-", ["AppRegistry", Aws.STACK_NAME, Aws.REGION, Aws.ACCOUNT_ID]), - description: `Service Catalog application to track and manage all your resources for the solution ${props.applicationName}`, + applicationName: props.applicationName, + description: `Service Catalog application to track and manage all your resources for the solution ${props.solutionName}`, }); application.associateApplicationWithStack(stack); Tags.of(application).add("Solutions:SolutionID", props.solutionId); - Tags.of(application).add("Solutions:SolutionName", props.applicationName); + Tags.of(application).add("Solutions:SolutionName", props.solutionName); Tags.of(application).add("Solutions:SolutionVersion", props.solutionVersion); Tags.of(application).add("Solutions:ApplicationType", applicationType); @@ -108,7 +109,7 @@ export class CommonResources extends Construct { applicationType, version: props.solutionVersion, solutionID: props.solutionId, - solutionName: props.applicationName, + solutionName: props.solutionName, }, }); attributeGroup.associateWith(application); diff --git a/source/constructs/lib/common-resources/custom-resources/custom-resource-construct.ts b/source/constructs/lib/common-resources/custom-resources/custom-resource-construct.ts index cbc43b91e..2b38765c0 100644 --- a/source/constructs/lib/common-resources/custom-resources/custom-resource-construct.ts +++ b/source/constructs/lib/common-resources/custom-resources/custom-resource-construct.ts @@ -49,6 +49,7 @@ export class CustomResourcesConstruct extends Construct { private readonly customResourceRole: Role; private readonly customResourceLambda: LambdaFunction; public readonly uuid: string; + public appRegApplicationName: string; constructor(scope: Construct, id: string, props: CustomResourcesConstructProps) { super(scope, id); @@ -116,6 +117,40 @@ export class CustomResourcesConstruct extends Construct { }), ], }), + AppRegistryPolicy: new PolicyDocument({ + statements: [ + new PolicyStatement({ + effect: Effect.ALLOW, + actions: ["cloudformation:DescribeStackResources"], + resources: [ + Stack.of(this).formatArn({ + partition: Aws.PARTITION, + service: "cloudformation", + region: Aws.REGION, + account: Aws.ACCOUNT_ID, + resource: "stack", + resourceName: `${Aws.STACK_NAME}/*`, + arnFormat: ArnFormat.SLASH_RESOURCE_NAME, + }), + ], + }), + new PolicyStatement({ + effect: Effect.ALLOW, + actions: ["servicecatalog:GetApplication"], + resources: [ + Stack.of(this).formatArn({ + partition: Aws.PARTITION, + service: "servicecatalog", + region: Aws.REGION, + account: Aws.ACCOUNT_ID, + resource: "applications", + resourceName: `*`, + arnFormat: ArnFormat.SLASH_RESOURCE_SLASH_RESOURCE_NAME, + }), + ], + }), + ], + }), }, }); @@ -188,6 +223,17 @@ export class CustomResourcesConstruct extends Construct { SourceBuckets: props.sourceBuckets, }); + const getAppRegApplicationNameResults = this.createCustomResource( + "CustomResourceGetAppRegApplicationName", + this.customResourceLambda, + { + CustomAction: "getAppRegApplicationName", + Region: Aws.REGION, + DefaultName: Fn.join("-", ["AppRegistry", Aws.STACK_NAME, Aws.REGION, Aws.ACCOUNT_ID]), + } + ); + this.appRegApplicationName = getAppRegApplicationNameResults.getAttString("ApplicationName"); + this.createCustomResource( "CustomResourceCheckFallbackImage", this.customResourceLambda, diff --git a/source/constructs/lib/serverless-image-stack.ts b/source/constructs/lib/serverless-image-stack.ts index f1a68d3c7..61dd2cb48 100644 --- a/source/constructs/lib/serverless-image-stack.ts +++ b/source/constructs/lib/serverless-image-stack.ts @@ -219,7 +219,8 @@ export class ServerlessImageHandlerStack extends Stack { description: `${props.solutionId} - ${props.solutionName}. Version ${props.solutionVersion}`, solutionVersion: props.solutionVersion, solutionId: props.solutionId, - applicationName: props.solutionName, + solutionName: props.solutionName, + applicationName: commonResources.customResources.appRegApplicationName, }); this.templateOptions.metadata = { diff --git a/source/constructs/package-lock.json b/source/constructs/package-lock.json index 18167533d..62985b40e 100644 --- a/source/constructs/package-lock.json +++ b/source/constructs/package-lock.json @@ -1,12 +1,12 @@ { "name": "constructs", - "version": "6.3.2", + "version": "6.3.3", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "constructs", - "version": "6.3.2", + "version": "6.3.3", "license": "Apache-2.0", "dependencies": { "metrics-utils": "file:../metrics-utils", diff --git a/source/constructs/package.json b/source/constructs/package.json index 3a988134d..207a72548 100644 --- a/source/constructs/package.json +++ b/source/constructs/package.json @@ -1,6 +1,6 @@ { "name": "constructs", - "version": "6.3.2", + "version": "6.3.3", "description": "Serverless Image Handler Constructs", "license": "Apache-2.0", "author": { diff --git a/source/constructs/test/__snapshots__/constructs.test.ts.snap b/source/constructs/test/__snapshots__/constructs.test.ts.snap index e3dec7b3d..5181a56c5 100644 --- a/source/constructs/test/__snapshots__/constructs.test.ts.snap +++ b/source/constructs/test/__snapshots__/constructs.test.ts.snap @@ -81,7 +81,7 @@ exports[`Serverless Image Handler Stack Snapshot 1`] = ` "Config": { "AnonymousUsage": "Yes", "SolutionId": "S0ABC", - "Version": "v6.3.2", + "Version": "v6.3.3", }, }, }, @@ -387,20 +387,9 @@ exports[`Serverless Image Handler Stack Snapshot 1`] = ` "Properties": { "Description": "Service Catalog application to track and manage all your resources for the solution sih", "Name": { - "Fn::Join": [ - "-", - [ - "AppRegistry", - { - "Ref": "AWS::StackName", - }, - { - "Ref": "AWS::Region", - }, - { - "Ref": "AWS::AccountId", - }, - ], + "Fn::GetAtt": [ + "CommonResourcesCustomResourcesCustomResourceGetAppRegApplicationName62472E55", + "ApplicationName", ], }, "Tags": { @@ -408,7 +397,7 @@ exports[`Serverless Image Handler Stack Snapshot 1`] = ` "Solutions:ApplicationType": "AWS-Solutions", "Solutions:SolutionID": "S0ABC", "Solutions:SolutionName": "sih", - "Solutions:SolutionVersion": "v6.3.2", + "Solutions:SolutionVersion": "v6.3.3", }, }, "Type": "AWS::ServiceCatalogAppRegistry::Application", @@ -1277,7 +1266,7 @@ exports[`Serverless Image Handler Stack Snapshot 1`] = ` }, "S3Key": "Omitted to remove snapshot dependency on hash", }, - "Description": "sih (v6.3.2): Performs image edits and manipulations", + "Description": "sih (v6.3.3): Performs image edits and manipulations", "Environment": { "Variables": { "AUTO_WEBP": { @@ -1977,7 +1966,7 @@ exports[`Serverless Image Handler Stack Snapshot 1`] = ` }, "S3Key": "Omitted to remove snapshot dependency on hash", }, - "Description": "sih (v6.3.2): Custom resource", + "Description": "sih (v6.3.3): Custom resource", "Environment": { "Variables": { "RETRY_SECONDS": "5", @@ -2004,6 +1993,40 @@ exports[`Serverless Image Handler Stack Snapshot 1`] = ` }, "Type": "AWS::Lambda::Function", }, + "CommonResourcesCustomResourcesCustomResourceGetAppRegApplicationName62472E55": { + "DeletionPolicy": "Delete", + "Properties": { + "CustomAction": "getAppRegApplicationName", + "DefaultName": { + "Fn::Join": [ + "-", + [ + "AppRegistry", + { + "Ref": "AWS::StackName", + }, + { + "Ref": "AWS::Region", + }, + { + "Ref": "AWS::AccountId", + }, + ], + ], + }, + "Region": { + "Ref": "AWS::Region", + }, + "ServiceToken": { + "Fn::GetAtt": [ + "CommonResourcesCustomResourcesCustomResourceFunction0D924235", + "Arn", + ], + }, + }, + "Type": "AWS::CloudFormation::CustomResource", + "UpdateReplacePolicy": "Delete", + }, "CommonResourcesCustomResourcesCustomResourceRole8958A1ED": { "Metadata": { "cfn_nag": { @@ -2160,6 +2183,66 @@ exports[`Serverless Image Handler Stack Snapshot 1`] = ` }, "PolicyName": "EC2Policy", }, + { + "PolicyDocument": { + "Statement": [ + { + "Action": "cloudformation:DescribeStackResources", + "Effect": "Allow", + "Resource": { + "Fn::Join": [ + "", + [ + "arn:", + { + "Ref": "AWS::Partition", + }, + ":cloudformation:", + { + "Ref": "AWS::Region", + }, + ":", + { + "Ref": "AWS::AccountId", + }, + ":stack/", + { + "Ref": "AWS::StackName", + }, + "/*", + ], + ], + }, + }, + { + "Action": "servicecatalog:GetApplication", + "Effect": "Allow", + "Resource": { + "Fn::Join": [ + "", + [ + "arn:", + { + "Ref": "AWS::Partition", + }, + ":servicecatalog:", + { + "Ref": "AWS::Region", + }, + ":", + { + "Ref": "AWS::AccountId", + }, + ":/applications/*", + ], + ], + }, + }, + ], + "Version": "2012-10-17", + }, + "PolicyName": "AppRegistryPolicy", + }, ], "Tags": [ { @@ -2583,7 +2666,7 @@ exports[`Serverless Image Handler Stack Snapshot 1`] = ` "applicationType": "AWS-Solutions", "solutionID": "S0ABC", "solutionName": "sih", - "version": "v6.3.2", + "version": "v6.3.3", }, "Description": "Attribute group for solution information", "Name": { diff --git a/source/constructs/test/constructs.test.ts b/source/constructs/test/constructs.test.ts index 4a2ae169d..e6193ca4e 100644 --- a/source/constructs/test/constructs.test.ts +++ b/source/constructs/test/constructs.test.ts @@ -11,14 +11,14 @@ test("Serverless Image Handler Stack Snapshot", () => { context: { solutionId: "SO0023", solutionName: "serverless-image-handler", - solutionVersion: "v6.3.2", + solutionVersion: "v6.3.3", }, }); const stack = new ServerlessImageHandlerStack(app, "TestStack", { solutionId: "S0ABC", solutionName: "sih", - solutionVersion: "v6.3.2", + solutionVersion: "v6.3.3", }); const template = Template.fromStack(stack); diff --git a/source/custom-resource/index.ts b/source/custom-resource/index.ts index 62f3923aa..fac886b30 100644 --- a/source/custom-resource/index.ts +++ b/source/custom-resource/index.ts @@ -1,8 +1,10 @@ // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 +import CloudFormation from "aws-sdk/clients/cloudformation"; import EC2, { DescribeRegionsRequest } from "aws-sdk/clients/ec2"; import S3, { CreateBucketRequest, PutBucketEncryptionRequest, PutBucketPolicyRequest } from "aws-sdk/clients/s3"; +import ServiceCatalogAppRegistry from "aws-sdk/clients/servicecatalogappregistry"; import SecretsManager from "aws-sdk/clients/secretsmanager"; import axios, { RawAxiosRequestConfig, AxiosResponse } from "axios"; import { createHash } from "crypto"; @@ -28,11 +30,14 @@ import { ResourcePropertyTypes, SendMetricsRequestProperties, StatusTypes, + GetAppRegApplicationNameRequestProperties, } from "./lib"; const awsSdkOptions = getOptions(); const s3Client = new S3(awsSdkOptions); const ec2Client = new EC2(awsSdkOptions); +const cloudformationClient = new CloudFormation(awsSdkOptions); +const serviceCatalogClient = new ServiceCatalogAppRegistry(awsSdkOptions); const secretsManager = new SecretsManager(awsSdkOptions); const { SOLUTION_ID, SOLUTION_VERSION, AWS_REGION, RETRY_SECONDS } = process.env; @@ -90,6 +95,14 @@ export async function handler(event: CustomResourceRequest, context: LambdaConte ); break; } + case CustomResourceActions.GET_APP_REG_APPLICATION_NAME: { + const allowedRequestTypes = [CustomResourceRequestTypes.CREATE, CustomResourceRequestTypes.UPDATE]; + await performRequest(getAppRegApplicationName, RequestType, allowedRequestTypes, response, { + ...ResourceProperties, + StackId: event.StackId, + } as GetAppRegApplicationNameRequestProperties); + break; + } case CustomResourceActions.CHECK_SECRETS_MANAGER: { const allowedRequestTypes = [CustomResourceRequestTypes.CREATE, CustomResourceRequestTypes.UPDATE]; await performRequest( @@ -404,6 +417,39 @@ async function validateBuckets(requestProperties: CheckSourceBucketsRequestPrope } } +/** + * Provides the existing app registry application name if it exists, otherwise, returns the default. + * @param requestProperties The request properties. + * @returns The result of validation. + */ +async function getAppRegApplicationName( + requestProperties: GetAppRegApplicationNameRequestProperties +): Promise<{ ApplicationName?: string }> { + try { + const stackResources = await cloudformationClient + .describeStackResources({ + StackName: requestProperties.StackId, + LogicalResourceId: "AppRegistry968496A3", + }) + .promise(); + + const application = await serviceCatalogClient + .getApplication({ + application: stackResources.StackResources[0].PhysicalResourceId, + }) + .promise(); + console.log(application); + return { + ApplicationName: application?.name ?? requestProperties.DefaultName, + }; + } catch (error) { + console.error(error); + return { + ApplicationName: requestProperties.DefaultName, + }; + } +} + /** * Checks if AWS Secrets Manager secret is valid. * @param requestProperties The request properties. diff --git a/source/custom-resource/lib/enums.ts b/source/custom-resource/lib/enums.ts index c709dadec..fa7778199 100644 --- a/source/custom-resource/lib/enums.ts +++ b/source/custom-resource/lib/enums.ts @@ -9,6 +9,7 @@ export enum CustomResourceActions { CHECK_SECRETS_MANAGER = "checkSecretsManager", CHECK_FALLBACK_IMAGE = "checkFallbackImage", CREATE_LOGGING_BUCKET = "createCloudFrontLoggingBucket", + GET_APP_REG_APPLICATION_NAME = "getAppRegApplicationName", } export enum CustomResourceRequestTypes { diff --git a/source/custom-resource/lib/interfaces.ts b/source/custom-resource/lib/interfaces.ts index f909f1a75..c37acfbf1 100644 --- a/source/custom-resource/lib/interfaces.ts +++ b/source/custom-resource/lib/interfaces.ts @@ -30,6 +30,11 @@ export interface CheckSourceBucketsRequestProperties extends CustomResourceReque SourceBuckets: string; } +export interface GetAppRegApplicationNameRequestProperties extends CustomResourceRequestPropertiesBase { + StackId: string; + DefaultName: string; +} + export interface CheckSecretManagerRequestProperties extends CustomResourceRequestPropertiesBase { SecretsManagerName: string; SecretsManagerKey: string; diff --git a/source/custom-resource/lib/types.ts b/source/custom-resource/lib/types.ts index b58ef2534..31460f26c 100644 --- a/source/custom-resource/lib/types.ts +++ b/source/custom-resource/lib/types.ts @@ -9,6 +9,7 @@ import { CustomResourceRequestPropertiesBase, PutConfigRequestProperties, SendMetricsRequestProperties, + GetAppRegApplicationNameRequestProperties, } from "./interfaces"; export type ResourcePropertyTypes = @@ -18,7 +19,8 @@ export type ResourcePropertyTypes = | CheckSourceBucketsRequestProperties | CheckSecretManagerRequestProperties | CheckFallbackImageRequestProperties - | CreateLoggingBucketRequestProperties; + | CreateLoggingBucketRequestProperties + | GetAppRegApplicationNameRequestProperties; export class CustomResourceError extends Error { constructor(public readonly code: string, public readonly message: string) { diff --git a/source/custom-resource/package-lock.json b/source/custom-resource/package-lock.json index 91120c5a6..b9f504965 100644 --- a/source/custom-resource/package-lock.json +++ b/source/custom-resource/package-lock.json @@ -1,12 +1,12 @@ { "name": "custom-resource", - "version": "6.3.2", + "version": "6.3.3", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "custom-resource", - "version": "6.3.2", + "version": "6.3.3", "license": "Apache-2.0", "dependencies": { "aws-sdk": "^2.1529.0", diff --git a/source/custom-resource/package.json b/source/custom-resource/package.json index 2784c9c15..01004b345 100644 --- a/source/custom-resource/package.json +++ b/source/custom-resource/package.json @@ -1,6 +1,6 @@ { "name": "custom-resource", - "version": "6.3.2", + "version": "6.3.3", "private": true, "description": "Serverless Image Handler custom resource", "license": "Apache-2.0", diff --git a/source/custom-resource/test/get-app-reg-application-name.spec.ts b/source/custom-resource/test/get-app-reg-application-name.spec.ts new file mode 100644 index 000000000..366a52fad --- /dev/null +++ b/source/custom-resource/test/get-app-reg-application-name.spec.ts @@ -0,0 +1,116 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { + mockCloudFormation, + mockServiceCatalogAppRegistry, + mockContext, +} from "./mock"; +import { CustomResourceActions, CustomResourceRequestTypes, CustomResourceRequest } from "../lib"; +import { handler } from "../index"; + +describe("GET_APP_REG_APPLICATION_NAME", () => { + // Mock event data + const defaultApplicationName = "ServerlessImageHandlerDefaultApplicationName"; + const event: CustomResourceRequest = { + RequestType: CustomResourceRequestTypes.CREATE, + ResponseURL: "/cfn-response", + PhysicalResourceId: "mock-physical-id", + StackId: "mock-stack-id", + ServiceToken: "mock-service-token", + RequestId: "mock-request-id", + LogicalResourceId: "mock-logical-resource-id", + ResourceType: "mock-resource-type", + ResourceProperties: { + CustomAction: CustomResourceActions.GET_APP_REG_APPLICATION_NAME, + DefaultName: defaultApplicationName, + }, + }; + + beforeEach(() => { + jest.resetAllMocks(); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + it("Should return default application name when application name could not be retrieved", async () => { + mockCloudFormation.describeStackResources.mockImplementation(() => ({ + promise() { + return Promise.resolve({ + StackResources: [ + { + LogicalResourceId: "SourceBucketA", + PhysicalResourceId: "bucket-a", + }, + ], + }); + }, + })); + + mockServiceCatalogAppRegistry.getApplication.mockImplementation(() => ({ + promise() { + return Promise.resolve({}); + }, + })); + + const result = await handler(event, mockContext); + expect(result).toEqual({ + Status: "SUCCESS", + Data: { ApplicationName: defaultApplicationName }, + }); + }); + + it("Should return default application name when application does not yet exist in the stack", async () => { + mockCloudFormation.describeStackResources.mockImplementation(() => ({ + promise() { + return Promise.resolve({ + StackResources: [], + }); + }, + })); + + mockServiceCatalogAppRegistry.getApplication.mockImplementation(() => ({ + promise() { + return Promise.resolve({}); + }, + })); + + const result = await handler(event, mockContext); + expect(result).toEqual({ + Status: "SUCCESS", + Data: { ApplicationName: defaultApplicationName }, + }); + }); + + it("Should return application name when available", async () => { + const applicationName = "SIHApplication"; + mockCloudFormation.describeStackResources.mockImplementation(() => ({ + promise() { + return Promise.resolve({ + StackResources: [ + { + LogicalResourceId: "SourceBucketA", + PhysicalResourceId: "bucket-a", + }, + ], + }); + }, + })); + + mockServiceCatalogAppRegistry.getApplication.mockImplementation(() => ({ + promise() { + return Promise.resolve({ + name: applicationName, + }); + }, + })); + + const result = await handler(event, mockContext); + expect(result).toEqual({ + Status: "SUCCESS", + Data: { ApplicationName: applicationName }, + }); + }); +}); diff --git a/source/custom-resource/test/mock.ts b/source/custom-resource/test/mock.ts index 14c8134cf..793e58531 100644 --- a/source/custom-resource/test/mock.ts +++ b/source/custom-resource/test/mock.ts @@ -29,6 +29,18 @@ export const mockAwsSecretManager = { jest.mock("aws-sdk/clients/secretsmanager", () => jest.fn(() => ({ ...mockAwsSecretManager }))); +export const mockCloudFormation = { + describeStackResources: jest.fn(), +}; + +jest.mock("aws-sdk/clients/cloudformation", () => jest.fn(() => ({ ...mockCloudFormation }))); + +export const mockServiceCatalogAppRegistry = { + getApplication: jest.fn(), +}; + +jest.mock("aws-sdk/clients/servicecatalogappregistry", () => jest.fn(() => ({ ...mockServiceCatalogAppRegistry }))); + export const mockAxios = { put: jest.fn(), post: jest.fn(), diff --git a/source/demo-ui/package-lock.json b/source/demo-ui/package-lock.json index d0dfcce0c..aa49dff22 100644 --- a/source/demo-ui/package-lock.json +++ b/source/demo-ui/package-lock.json @@ -1,12 +1,12 @@ { "name": "demo-ui", - "version": "6.3.2", + "version": "6.3.3", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "demo-ui", - "version": "6.3.2", + "version": "6.3.3", "hasInstallScript": true, "license": "Apache-2.0", "dependencies": { diff --git a/source/demo-ui/package.json b/source/demo-ui/package.json index 82d435cbe..1cd4105a5 100644 --- a/source/demo-ui/package.json +++ b/source/demo-ui/package.json @@ -1,6 +1,6 @@ { "name": "demo-ui", - "version": "6.3.2", + "version": "6.3.3", "private": true, "description": "Serverless Image Handler demo ui", "license": "Apache-2.0", diff --git a/source/image-handler/image-handler.ts b/source/image-handler/image-handler.ts index 9c90b0ef5..28f9ae64c 100644 --- a/source/image-handler/image-handler.ts +++ b/source/image-handler/image-handler.ts @@ -17,6 +17,8 @@ import { RekognitionCompatibleImage, StatusCodes, } from "./lib"; +import { getAllowedSourceBuckets } from "./image-request"; +import { SHARP_EDIT_ALLOWLIST_ARRAY } from "./lib/constants"; export class ImageHandler { private readonly LAMBDA_PAYLOAD_LIMIT = 6 * 1024 * 1024; @@ -165,7 +167,7 @@ export class ImageHandler { break; } default: { - if (edit in originalImage) { + if (SHARP_EDIT_ALLOWLIST_ARRAY.includes(edit as any)) { originalImage[edit](edits[edit]); } } @@ -474,6 +476,13 @@ export class ImageHandler { alpha: string, sourceImageMetadata: sharp.Metadata ): Promise { + if (!getAllowedSourceBuckets().includes(bucket)) { + throw new ImageHandlerError( + StatusCodes.FORBIDDEN, + "ImageBucket::CannotAccessBucket", + "The overlay image bucket you specified could not be accessed. Please check that the bucket is specified in your SOURCE_BUCKETS." + ); + } const params = { Bucket: bucket, Key: key }; try { const { width, height } = sourceImageMetadata; diff --git a/source/image-handler/image-request.ts b/source/image-handler/image-request.ts index 6b22608d3..a06203273 100644 --- a/source/image-handler/image-request.ts +++ b/source/image-handler/image-request.ts @@ -15,6 +15,7 @@ import { ImageRequestInfo, RequestTypes, StatusCodes, + HEADER_DENY_LIST, } from "./lib"; import { SecretProvider } from "./secret-provider"; import { ThumborMapper } from "./thumbor-mapper"; @@ -168,14 +169,11 @@ export class ImageRequest { ); } const imageBuffer = Buffer.from(originalImage.Body as Uint8Array); - + // Infer from hex headers if provided content type is not supported if (originalImage.ContentType) { - // If using default S3 ContentType infer from hex headers - if (["binary/octet-stream", "application/octet-stream"].includes(originalImage.ContentType)) { - result.contentType = this.inferImageType(imageBuffer); - } else { - result.contentType = originalImage.ContentType; - } + result.contentType = Object.values(ContentTypes).includes(originalImage.ContentType) + ? originalImage.ContentType + : this.inferImageType(imageBuffer); } else { result.contentType = "image"; } @@ -217,7 +215,7 @@ export class ImageRequest { if (request.bucket !== undefined) { // Check the provided bucket against the allowed list - const sourceBuckets = this.getAllowedSourceBuckets(); + const sourceBuckets = getAllowedSourceBuckets(); if (sourceBuckets.includes(request.bucket)) { return request.bucket; @@ -230,12 +228,12 @@ export class ImageRequest { } } else { // Try to use the default image source bucket env var - const sourceBuckets = this.getAllowedSourceBuckets(); + const sourceBuckets = getAllowedSourceBuckets(); return sourceBuckets[0]; } } else if (requestType === RequestTypes.THUMBOR || requestType === RequestTypes.CUSTOM) { // Use the default image source bucket env var - const sourceBuckets = this.getAllowedSourceBuckets(); + const sourceBuckets = getAllowedSourceBuckets(); // Take the path and split it at "/" to get each "word" in the url as array let potentialBucket = event.path .split("/") @@ -394,7 +392,7 @@ export class ImageRequest { if (requestType === RequestTypes.DEFAULT) { const { headers } = this.decodeRequest(event); if (headers) { - return headers; + return filterRestrictedHeaders(headers); } } } @@ -430,25 +428,6 @@ export class ImageRequest { } } - /** - * Returns a formatted image source bucket allowed list as specified in the SOURCE_BUCKETS environment variable of the image handler Lambda function. - * Provides error handling for missing/invalid values. - * @returns A formatted image source bucket. - */ - public getAllowedSourceBuckets(): string[] { - const { SOURCE_BUCKETS } = process.env; - - if (SOURCE_BUCKETS === undefined) { - throw new ImageHandlerError( - StatusCodes.BAD_REQUEST, - "GetAllowedSourceBuckets::NoSourceBuckets", - "The SOURCE_BUCKETS variable could not be read. Please check that it is not empty and contains at least one source bucket, or multiple buckets separated by commas. Spaces can be provided between commas and bucket names, these will be automatically parsed out when decoding." - ); - } else { - return SOURCE_BUCKETS.replace(/\s+/g, "").split(","); - } - } - /** * Return the output format depending on the accepts headers and request type. * @param event Lambda request body. @@ -547,3 +526,47 @@ export class ImageRequest { } } } + +/** + * Returns a formatted image source bucket allowed list as specified in the SOURCE_BUCKETS environment variable of the image handler Lambda function. + * Provides error handling for missing/invalid values. + * @returns A formatted image source bucket. + */ +export function getAllowedSourceBuckets(): string[] { + const { SOURCE_BUCKETS } = process.env; + + if (SOURCE_BUCKETS === undefined) { + throw new ImageHandlerError( + StatusCodes.BAD_REQUEST, + "GetAllowedSourceBuckets::NoSourceBuckets", + "The SOURCE_BUCKETS variable could not be read. Please check that it is not empty and contains at least one source bucket, or multiple buckets separated by commas. Spaces can be provided between commas and bucket names, these will be automatically parsed out when decoding." + ); + } else { + return SOURCE_BUCKETS.replace(/\s+/g, "").split(","); + } +} + +/** + * Filters out headers that match any pattern in the deny list and returns the safe headers + * @param headers Input headers to filter + * @returns Safe headers + */ +function filterRestrictedHeaders(headers: Record): Headers { + const safeHeaders: Record = {}; + + // Process each header using for...of loop + for (const [header, value] of Object.entries(headers)) { + const headerLower = header.toLowerCase(); + + // If the header matches any pattern in the deny list, log and skip + if (HEADER_DENY_LIST.some((pattern) => pattern.test(headerLower))) { + console.warn("Filtered out restricted header:", header); + continue; + } + + // Add safe headers + safeHeaders[header] = value; + } + + return safeHeaders; +} diff --git a/source/image-handler/index.ts b/source/image-handler/index.ts index 232d8b5fe..d4c13e49b 100755 --- a/source/image-handler/index.ts +++ b/source/image-handler/index.ts @@ -36,18 +36,20 @@ export async function handler(event: ImageHandlerEvent): Promise; // eslint-disable-next-line @typescript-eslint/no-explicit-any -export type ImageEdits = Record; +export type ImageEdits = Partial>; export class ImageHandlerError extends Error { constructor(public readonly status: StatusCodes, public readonly code: string, public readonly message: string) { super(); } } + +type AllowlistedEdit = (typeof SHARP_EDIT_ALLOWLIST_ARRAY)[number] | (typeof ALTERNATE_EDIT_ALLOWLIST_ARRAY)[number]; diff --git a/source/image-handler/package-lock.json b/source/image-handler/package-lock.json index f57ed40bb..e334d9f12 100644 --- a/source/image-handler/package-lock.json +++ b/source/image-handler/package-lock.json @@ -1,12 +1,12 @@ { "name": "image-handler", - "version": "6.3.2", + "version": "6.3.3", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "image-handler", - "version": "6.3.2", + "version": "6.3.3", "license": "Apache-2.0", "dependencies": { "aws-sdk": "^2.1529.0", diff --git a/source/image-handler/package.json b/source/image-handler/package.json index a859b1027..0a3146a59 100644 --- a/source/image-handler/package.json +++ b/source/image-handler/package.json @@ -1,6 +1,6 @@ { "name": "image-handler", - "version": "6.3.2", + "version": "6.3.3", "private": true, "description": "A Lambda function for performing on-demand image edits and manipulations.", "license": "Apache-2.0", diff --git a/source/image-handler/test/image-handler/allowlist.spec.ts b/source/image-handler/test/image-handler/allowlist.spec.ts new file mode 100644 index 000000000..26074edab --- /dev/null +++ b/source/image-handler/test/image-handler/allowlist.spec.ts @@ -0,0 +1,38 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +import Rekognition from "aws-sdk/clients/rekognition"; +import S3 from "aws-sdk/clients/s3"; +import sharp from "sharp"; + +import { ImageHandler } from "../../image-handler"; +import { ImageRequestInfo, RequestTypes } from "../../lib"; +import fs from "fs"; + +const s3Client = new S3(); +const rekognitionClient = new Rekognition(); + +describe("allowlist", () => { + it("Non-allowlisted filters should not be called", async () => { + // Arrange + const originalImage = fs.readFileSync("./test/image/1x1.jpg"); + const request: ImageRequestInfo = { + requestType: RequestTypes.DEFAULT, + bucket: "sample-bucket", + key: "test.jpg", + edits: { rotate: null, toFile: "test" } as unknown as any, // toFile is not allowlisted + originalImage, + }; + + const toFileSpy = jest.spyOn(sharp.prototype, "toFile"); + const rotateSpy = jest.spyOn(sharp.prototype, "rotate"); + + // Act + const imageHandler = new ImageHandler(s3Client, rekognitionClient); + await imageHandler.process(request); + + // Assert + expect(toFileSpy).toBeCalledTimes(0); + expect(rotateSpy).toBeCalledTimes(1); + }); +}); diff --git a/source/image-handler/test/image-handler/overlay.spec.ts b/source/image-handler/test/image-handler/overlay.spec.ts index 79bbe77e8..ab5c42d58 100644 --- a/source/image-handler/test/image-handler/overlay.spec.ts +++ b/source/image-handler/test/image-handler/overlay.spec.ts @@ -17,6 +17,7 @@ const rekognitionClient = new Rekognition(); describe("overlay", () => { beforeEach(() => { jest.resetAllMocks(); + process.env.SOURCE_BUCKETS = "validBucket, sourceBucket, bucket, sample-bucket"; }); afterEach(() => { @@ -305,7 +306,7 @@ describe("overlay", () => { expect(overlayImageMetadata.height).toEqual(11); }); - it("Should throw an error if an invalid bucket or key name is provided, simulating a nonexistent overlay image", async () => { + it("Should throw an error if an invalid key name is provided, simulating a nonexistent overlay image", async () => { // Mock mockAwsS3.getObject.mockImplementationOnce(() => ({ promise() { @@ -328,11 +329,11 @@ describe("overlay", () => { ) ).metadata(); try { - await imageHandler.getOverlayImage("invalidBucket", "invalidKey", "100", "100", "20", metadata); + await imageHandler.getOverlayImage("bucket", "invalidKey", "100", "100", "20", metadata); } catch (error) { // Assert expect(mockAwsS3.getObject).toHaveBeenCalledWith({ - Bucket: "invalidBucket", + Bucket: "bucket", Key: "invalidKey", }); expect(error).toMatchObject({ @@ -342,6 +343,26 @@ describe("overlay", () => { }); } }); + it("Should throw an error if an invalid bucket is provided", async () => { + // Act + const imageHandler = new ImageHandler(s3Client, rekognitionClient); + const metadata = await sharp( + Buffer.from( + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8z8BQDwAEhQGAhKmMIQAAAABJRU5ErkJggg==", + "base64" + ) + ).metadata(); + try { + await imageHandler.getOverlayImage("invalidBucket", "key", "100", "100", "20", metadata); + } catch (error) { + // Assert + expect(error).toMatchObject({ + status: StatusCodes.FORBIDDEN, + code: "ImageBucket::CannotAccessBucket", + message: "The overlay image bucket you specified could not be accessed. Please check that the bucket is specified in your SOURCE_BUCKETS.", + }); + } + }); }); describe("calcOverlaySizeOption", () => { diff --git a/source/image-handler/test/image-handler/round-crop.spec.ts b/source/image-handler/test/image-handler/round-crop.spec.ts index 138214b8b..31fc53937 100644 --- a/source/image-handler/test/image-handler/round-crop.spec.ts +++ b/source/image-handler/test/image-handler/round-crop.spec.ts @@ -44,7 +44,7 @@ describe("roundCrop", () => { const result = await imageHandler.applyEdits(image, edits, false); // Assert - const expectedResult: ImageEdits = { width: metadata.width / 2, height: metadata.height / 2 }; + const expectedResult = { width: metadata.width / 2, height: metadata.height / 2 }; expect(result["options"].input).not.toEqual(expectedResult); expect(hasRoundCropSpy).toHaveReturnedWith(true); expect(validRoundCropParamSpy).toHaveBeenCalledTimes(4); @@ -70,7 +70,7 @@ describe("roundCrop", () => { const result = await imageHandler.applyEdits(image, edits, false); // Assert - const expectedResult: ImageEdits = { width: metadata.width / 2, height: metadata.height / 2 }; + const expectedResult = { width: metadata.width / 2, height: metadata.height / 2 }; expect(result["options"].input).not.toEqual(expectedResult); expect(hasRoundCropSpy).toHaveReturnedWith(true); expect(validRoundCropParamSpy).toHaveReturnedWith(true); diff --git a/source/image-handler/test/image-request/get-allowed-source-buckets.spec.ts b/source/image-handler/test/image-request/get-allowed-source-buckets.spec.ts index 73ab08eb0..f38182ca9 100644 --- a/source/image-handler/test/image-request/get-allowed-source-buckets.spec.ts +++ b/source/image-handler/test/image-request/get-allowed-source-buckets.spec.ts @@ -1,25 +1,17 @@ // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 -import S3 from "aws-sdk/clients/s3"; -import SecretsManager from "aws-sdk/clients/secretsmanager"; - -import { ImageRequest } from "../../image-request"; +import { getAllowedSourceBuckets } from "../../image-request"; import { StatusCodes } from "../../lib"; -import { SecretProvider } from "../../secret-provider"; describe("getAllowedSourceBuckets", () => { - const s3Client = new S3(); - const secretsManager = new SecretsManager(); - const secretProvider = new SecretProvider(secretsManager); it("Should pass if the SOURCE_BUCKETS environment variable is not empty and contains valid inputs", () => { // Arrange process.env.SOURCE_BUCKETS = "allowedBucket001, allowedBucket002"; // Act - const imageRequest = new ImageRequest(s3Client, secretProvider); - const result = imageRequest.getAllowedSourceBuckets(); + const result = getAllowedSourceBuckets(); // Assert const expectedResult = ["allowedBucket001", "allowedBucket002"]; @@ -31,11 +23,9 @@ describe("getAllowedSourceBuckets", () => { process.env = {}; // Act - const imageRequest = new ImageRequest(s3Client, secretProvider); - // Assert try { - imageRequest.getAllowedSourceBuckets(); + getAllowedSourceBuckets(); } catch (error) { expect(error).toMatchObject({ status: StatusCodes.BAD_REQUEST, diff --git a/source/image-handler/test/image-request/parse-image-headers.spec.ts b/source/image-handler/test/image-request/parse-image-headers.spec.ts index f02812cfa..557171015 100644 --- a/source/image-handler/test/image-request/parse-image-headers.spec.ts +++ b/source/image-handler/test/image-request/parse-image-headers.spec.ts @@ -30,6 +30,29 @@ describe("parseImageHeaders", () => { expect(result).toEqual(expectedResult); }); + it("001/Should remove restricted headers if included with a base64-encoded image request", () => { + //Arrange + /** Includes restricted headers: + * 'Transfer-encoding', + * 'x-api-key', + * 'x-amz-header', + * 'content-type', + * 'Access-Control-Allow-Origin' + **/ + const event = { + path: "/eyJidWNrZXQiOiJ2YWxpZEJ1Y2tldCIsImtleSI6InZhbGlkS2V5IiwiaGVhZGVycyI6eyJDYWNoZS1Db250cm9sIjoibWF4LWFnZT0zMTUzNjAwMCxwdWJsaWMiLCAiVHJhbnNmZXItZW5jb2RpbmciOiAidmFsdWUiLCAieC1hcGkta2V5IjogInZhbHVlIiwgIngtYW16LWhlYWRlciI6ICJ2YWx1ZSIsICJjb250ZW50LXR5cGUiOiAiaHRtbCIsICJBY2Nlc3MtQ29udHJvbC1BbGxvdy1PcmlnaW4iOiAiKiJ9LCJvdXRwdXRGb3JtYXQiOiJqcGVnIn0=", + }; + // Act + const imageRequest = new ImageRequest(s3Client, secretProvider); + const result = imageRequest.parseImageHeaders(event, RequestTypes.DEFAULT); + + // Assert + const expectedResult = { + "Cache-Control": "max-age=31536000,public", + }; + expect(result).toEqual(expectedResult); + }); + it("001/Should return undefined if headers are not provided for a base64-encoded image request", () => { // Arrange const event = { diff --git a/source/image-handler/test/image-request/setup.spec.ts b/source/image-handler/test/image-request/setup.spec.ts index 6622591ed..57824b5b3 100644 --- a/source/image-handler/test/image-request/setup.spec.ts +++ b/source/image-handler/test/image-request/setup.spec.ts @@ -221,7 +221,7 @@ describe("setup", () => { promise() { return Promise.resolve({ CacheControl: "max-age=300,public", - ContentType: "custom-type", + ContentType: "image/jpeg", Expires: "Tue, 24 Dec 2019 13:46:28 GMT", LastModified: "Sat, 19 Dec 2009 16:30:47 GMT", Body: Buffer.from("SampleImageContent\n"), @@ -242,7 +242,7 @@ describe("setup", () => { }, originalImage: Buffer.from("SampleImageContent\n"), cacheControl: "max-age=300,public", - contentType: "custom-type", + contentType: "image/jpeg", expires: "Tue, 24 Dec 2019 13:46:28 GMT", lastModified: "Sat, 19 Dec 2009 16:30:47 GMT", }; @@ -271,7 +271,7 @@ describe("setup", () => { promise() { return Promise.resolve({ CacheControl: "max-age=300,public", - ContentType: "custom-type", + ContentType: "image/jpeg", Expires: "Tue, 24 Dec 2019 13:46:28 GMT", LastModified: "Sat, 19 Dec 2009 16:30:47 GMT", Body: Buffer.from("SampleImageContent\n"), @@ -292,7 +292,7 @@ describe("setup", () => { }, originalImage: Buffer.from("SampleImageContent\n"), cacheControl: "max-age=300,public", - contentType: "custom-type", + contentType: "image/jpeg", expires: "Tue, 24 Dec 2019 13:46:28 GMT", lastModified: "Sat, 19 Dec 2009 16:30:47 GMT", }; diff --git a/source/package-lock.json b/source/package-lock.json index 783769605..7306732f2 100644 --- a/source/package-lock.json +++ b/source/package-lock.json @@ -1,12 +1,12 @@ { "name": "source", - "version": "6.3.2", + "version": "6.3.3", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "source", - "version": "6.3.2", + "version": "6.3.3", "license": "Apache-2.0", "devDependencies": { "@types/node": "^20.10.4", diff --git a/source/package.json b/source/package.json index 6af0d8a47..0c6a2e2d3 100644 --- a/source/package.json +++ b/source/package.json @@ -1,6 +1,6 @@ { "name": "source", - "version": "6.3.2", + "version": "6.3.3", "private": true, "description": "ESLint and prettier dependencies to be used within the solution", "license": "Apache-2.0", diff --git a/source/solution-utils/package-lock.json b/source/solution-utils/package-lock.json index ca7d6ade4..5d6115216 100644 --- a/source/solution-utils/package-lock.json +++ b/source/solution-utils/package-lock.json @@ -1,12 +1,12 @@ { "name": "solution-utils", - "version": "6.3.2", + "version": "6.3.3", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "solution-utils", - "version": "6.3.2", + "version": "6.3.3", "license": "Apache-2.0", "devDependencies": { "@types/jest": "^29.5.5", diff --git a/source/solution-utils/package.json b/source/solution-utils/package.json index 6a2118cb9..b8a7646ba 100644 --- a/source/solution-utils/package.json +++ b/source/solution-utils/package.json @@ -1,6 +1,6 @@ { "name": "solution-utils", - "version": "6.3.2", + "version": "6.3.3", "private": true, "description": "Utilities to be used within this solution", "license": "Apache-2.0",