aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.72k stars 3.94k forks source link

aws-securityhub: Note updatedBy is expecting an object when it should be a string #26749

Open kpapagno opened 1 year ago

kpapagno commented 1 year ago

Describe the bug

When creating a securityhub automation rule; the rule will not accept a properly formatted note to be updated in the finding. Specifically, the cdk expects the "updatedBy" to be an object and fails synth. A standard cloudformation script with the updatedBy as a string works just fine.

Example cdk that will work; un-comment the note action and it will fail.


import * as cdk from 'aws-cdk-lib';
import * as securityhub from 'aws-cdk-lib/aws-securityhub';
import { Construct } from 'constructs';

export class MyStack extends cdk.Stack {
    constructor(scope: Construct, id: string, props: cdk.StackProps = {}) {
      super(scope, id, props);

      new securityhub.CfnAutomationRule(this, "SampleRule", {
        description: "This is a sample rule",
        isTerminal: true,
        ruleName: 'Sample Rule',
        ruleOrder: 100,
        ruleStatus: "ENABLED",
        actions: [{
            findingFieldsUpdate: {
              // note: {
              //   text: "Company XYZ Exception Approved Exception August 2023",
              //   updatedBy: "SecurityHub Automation"
              // },
              workflow: { status: 'SUPPRESSED', },
            },
            type: 'FINDING_FIELDS_UPDATE',
        }],
        criteria: {
            productName:      [{comparison: 'EQUALS', value: 'Security Hub', }],
            complianceStatus: [{comparison: 'EQUALS', value: 'FAILED', }],
            workflowStatus:   [{comparison: 'EQUALS', value: 'NEW', }], 
            recordState:      [{comparison: 'EQUALS', value: 'ACTIVE', }],
            generatorId:      [{comparison: 'EQUALS', value: 'aws-foundational-security-best-practices/v/1.0.0/Athena.1', }],
        }
      });
  }
}

// for development, use account/region from cdk cli
const devEnv = {
  account: process.env.CDK_DEFAULT_ACCOUNT,
  region: process.env.CDK_DEFAULT_REGION,
};

const app = new cdk.App();

new MyStack(app, 'ruleexample-dev', { env: devEnv });

app.synth();

Expected Behavior

The provided code should work, with just a string passed in as the updatedBy text.

Current Behavior

Error on deploy is: CfnSynthesisError: Resolution error: Supplied properties not correct for "CfnAutomationRuleProps" actions: element 0: supplied properties not correct for "AutomationRulesActionProperty" findingFieldsUpdate: supplied properties not correct for "AutomationRulesFindingFieldsUpdateProperty" note: supplied properties not correct for "NoteUpdateProperty" updatedBy: "SecurityHub Automation" should be an 'object'. at ValidationResult.assertSuccess (/Users/ken/codecnxc/security/ruleexample/node_modules/aws-cdk-lib/core/lib/runtime.js:1:2643) at convertCfnAutomationRulePropsToCloudFormation (/Users/ken/codecnxc/security/ruleexample/node_modules/aws-cdk-lib/aws-securityhub/lib/securityhub.generated.js:1:45129) at CfnAutomationRule.renderProperties (/Users/ken/codecnxc/security/ruleexample/node_modules/aws-cdk-lib/aws-securityhub/lib/securityhub.generated.js:1:2556) at PostResolveToken.Resources (/Users/ken/codecnxc/security/ruleexample/node_modules/aws-cdk-lib/core/lib/cfn-resource.js:1:7030) at PostResolveToken.postProcess (/Users/ken/codecnxc/security/ruleexample/node_modules/aws-cdk-lib/core/lib/util.js:1:1565) at Object.postProcess (/Users/ken/codecnxc/security/ruleexample/node_modules/aws-cdk-lib/core/lib/private/resolve.js:1:1018) at DefaultTokenResolver.resolveToken (/Users/ken/codecnxc/security/ruleexample/node_modules/aws-cdk-lib/core/lib/resolvable.js:1:1320) at resolve (/Users/ken/codecnxc/security/ruleexample/node_modules/aws-cdk-lib/core/lib/private/resolve.js:1:2510) at Object.resolve (/Users/ken/codecnxc/security/ruleexample/node_modules/aws-cdk-lib/core/lib/private/resolve.js:1:892) at resolve (/Users/ken/codecnxc/security/ruleexample/node_modules/aws-cdk-lib/core/lib/private/resolve.js:1:2787) { type: 'CfnSynthesisError' }

Reproduction Steps

  1. generate a new cdk project
  2. deply a securityhub automation rule that has a note with text and an updatedBy value

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.91.0 (build eea12ea)

Framework Version

No response

Node.js Version

16.18.3

OS

OSX

Language

Typescript

Language Version

ts-node v10.9.1

Other information

No response

pahud commented 1 year ago

The updatedBy should be a json object https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-securityhub-automationrule-noteupdate.html#cfn-securityhub-automationrule-noteupdate-updatedby

e.g.

findingFieldsUpdate: {
    note: {
        text: "Company XYZ Exception Approved Exception August 2023",
        updatedBy: {'name': 'pahud'},

    },
kpapagno commented 1 year ago

When you do it as an object, then the cloudformation chokes on it saying it expects string.

`12:24:31 PM | UPDATE_FAILED | AWS::SecurityHub::AutomationRule | SampleRule Properties validation failed for resource SampleRule with message:

/Actions/0/FindingFieldsUpdate/Note/UpdatedBy: #: no subschema matched out of the total 2 subschemas

/Actions/0/FindingFieldsUpdate/Note/UpdatedBy: expected type: String, found: JSONObject

/Actions/0/FindingFieldsUpdate/Note/UpdatedBy: expected type: String, found: JSONObject`

peterwoodworth commented 1 year ago

We're consuming the model that CloudFormation offers - since we match CloudFormation (both want an object here), we need CloudFormation/SecurityHub to update their model. You can report an issue at the CloudFormation Coverage Roadmap if one doesn't already exist

If it's expecting a string at deploy time, what happens if you use an escape hatch to override the UpdatedBy value to a string? Does it work? Or is this just completely broken

kpapagno commented 1 year ago

Thanks @peterwoodworth you helped me along. I had no clue about these escape hatches! Luckily this workaround was simple, and because it was already a L1 construct i didn't have to do what most of the examples did!

I was able to just add a property override, which I had no idea that you could do (Architect, not day to day "programmer"). Still a bit unclear on where/what to say the issue is, but the cdk workaround is easy enough!

const samplerule = new securityhub.CfnAutomationRule(this, "SampleRule", {
        description: "This is a sample rule",
        isTerminal: true,
        ruleName: 'Sample Rule',
        ruleOrder: 100,
        ruleStatus: "ENABLED",
        actions: [{
            findingFieldsUpdate: {
              note: {
                text: "Company XYZ Exception Approved Exception August 2023",
                updatedBy: {'name': 'pahud'},
              },
              workflow: { status: 'SUPPRESSED', },
            },
            type: 'FINDING_FIELDS_UPDATE',
        }],
        criteria: {
            productName:      [{comparison: 'EQUALS', value: 'Security Hub', }],
            complianceStatus: [{comparison: 'EQUALS', value: 'FAILED', }],
            workflowStatus:   [{comparison: 'EQUALS', value: 'NEW', }], 
            recordState:      [{comparison: 'EQUALS', value: 'ACTIVE', }],
            generatorId:      [{comparison: 'EQUALS', value: 'aws-foundational-security-best-practices/v/1.0.0/Athena.1', }],
        }
      });

      samplerule.addPropertyOverride('Actions.0.FindingFieldsUpdate.Note.UpdatedBy', 'Securityhub Automation Rule')