-
Notifications
You must be signed in to change notification settings - Fork 4.2k
feat(sns): add sns data protection policy #34640
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…an/aws-cdk into feature/sns-data-protection-policy
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
@@ -355,6 +372,15 @@ export class Topic extends TopicBase { | |||
throw new ValidationError(`displayName must be less than or equal to 100 characters, got ${props.displayName.length}`, this); | |||
} | |||
|
|||
// Validate that data protection policy is only used with standard topics | |||
if (props.dataProtectionPolicy && props.fifo) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are no unit tests specifically for this validation logic, would like to see a unit test for this
if (this.customDataIdentifiers.length > 10) { | ||
throw new UnscopedValidationError( | ||
'A maximum of 10 custom data identifiers are supported per data protection policy', | ||
); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The validation for the maximum number of custom data identifiers happens during toJSON() rather than at construction time, which could lead to late failures. Move this validation to the constructor to fail earlier
/** | ||
* The percentage of messages to sample for audit. | ||
* Must be an integer between 0-99. | ||
* @default 99 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How is this default value selected? Could it lead to high costs and performance impacts in production environments with high message volumes?
!logGroupName.startsWith('/aws/vendedlogs/') | ||
) { | ||
throw new UnscopedValidationError( | ||
'CloudWatch Logs log group name must start with "/aws/vendedlogs/"', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enhance the error message by including that this is an AWS requirement for SNS data protection
|
||
// Validate regex length | ||
if (this.regex.length > 200) { | ||
throw new UnscopedValidationError( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The validation uses UnscopedValidationError
which doesn't provide context about which construct is throwing the error. It is better to use ValidationError
with the construct instance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better to refactor every other UnscopedValidationError as well
this.customDataIdentifiers = this.statements | ||
.flatMap((statement) => statement.dataIdentifiers) | ||
.filter( | ||
(dataIdentifier): dataIdentifier is CustomDataIdentifier => | ||
dataIdentifier instanceof CustomDataIdentifier, | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code collects all custom data identifiers from all statements, which could lead to duplicates if the same custom identifier is used in multiple statements. Consider deduplicating the custom identifiers to avoid potential issues, for example by using a Set or checking for duplicates by name.
Issue # (if applicable)
Closes #34136.
Reason for this change
AWS SNS now supports Message Data Protection policies, which allow users to define rules to detect and handle sensitive data in SNS messages. This feature enables users to implement security controls like auditing, masking, redacting, or blocking messages containing sensitive data.
Description of changes
This PR adds support for SNS Data Protection policies to the AWS CDK, including:
Added
dataProtectionPolicy
property toTopicProps
Adds Core Data Protection:
DataProtectionPolicy
- The main construct for defining an SNS data protection policyDataProtectionPolicyStatement
- Represents a rule within the policyDataDirection
- Enum to specify inbound or outbound message flowAuditOperation
,MaskOperation
,RedactOperation
,DenyOperation
)CustomDataIdentifier
Adds Managed Data Identifier collections:
PersonalIdentifiers
- For detecting PII (names, addresses, SSNs, etc.)FinancialIdentifiers
- For detecting financial information (credit cards, bank accounts)CredentialsIdentifiers
- For detecting sensitive credentials (AWS keys, private keys)DeviceIdentifiers
- For detecting device-related information (IP addresses)HealthIdentifiers
- For detecting health-related information (PHI)Updated README with examples
Describe any new or updated permissions being added
Description of how you validated changes
Created unit tests for all new components
Created integration test
Manually tested deployment using
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license