Validate AWS region format for S3 audit event streaming

Pratyaksh Golashrequested to merge
608459-validate-aws-region-format into master

What does this MR do and why?

Audit event streaming to AWS S3 raises Aws::Errors::InvalidRegionError on every stream attempt when a destination is saved with a malformed AWS region.

awsRegion was validated for presence and length but never for format — it was the only field in the AWS destination config schema without a pattern, where accessKeyXid and bucketName both have one. A non-empty but malformed value such as us east 1, US_EAST_1, or a copy-pasted Unicode en dash therefore saves successfully and then fails on every subsequent stream, inside Aws::S3::Client.new at lib/aws/s3_client.rb:5.

Worth separating the two AWS SDK errors, because they are often conflated:

Region value SDK error
nil or "" Aws::Errors::MissingRegionError
non-empty, fails Seahorse::Util.host_label? Aws::Errors::InvalidRegionError

Sentry reports InvalidRegionError, so the stored value is malformed rather than blank. Blank is already covered — aws_region has a presence validation on both legacy models, and awsRegion is required with minLength: 1 in the JSON schema — which is why the blank? guard proposed in the issue would never fire on the input that is actually failing.

This MR adds the missing format validation, and strips surrounding whitespace before validating so that a copy-pasted region carrying a leading or trailing space is corrected rather than rejected.

Changes

File Change
audit_events_aws_external_streaming_destination_config.json pattern on awsRegion
concerns/audit_events/externally_streamable.rb normalize_aws_region strips whitespace, if: :aws?
audit_events/amazon_s3_configuration.rb AWS_REGION_REGEXP + format validation + strip

Why this regex

\A[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\z mirrors the SDK's own check, Seahorse::Util.host_label?, so we can never reject a region the SDK would have accepted, and it stays correct as AWS adds regions. Verified against every region in aws-partitions, including the GovCloud and ISO partitions — zero rejections.

The regex suggested in the issue description, /\A[a-z]{2}-[a-z]+-\d+\z|aws-global|aws-cn-global|aws-us-gov-global/, rejects 8 valid regions: us-gov-east-1, us-gov-west-1, aws-iso-global, us-iso-east-1, us-iso-west-1, aws-iso-b-global, us-isob-east-1, eu-isoe-west-1. Its alternation is also ungrouped, so the anchors only bind to the first branch.

Restricting to lowercase is deliberate and is the one place this is stricter than the SDK. Every real region code is lowercase, and an uppercase region reaches SigV4 signing and fails at AWS with a 403 rather than working.

Note on existing records

Danger flags that new validations can break existing records, so to be explicit: validation runs on save only, so destinations already storing a malformed region continue to fail until they are next edited, and at that point the region must be corrected before any other edit can be saved. That is intentional — the alternative is letting a known-broken destination keep saving.

No backfill migration is proposed. Detection query for anyone auditing this:

AuditEvents::Group::ExternalStreamingDestination.where(category: :aws).find_each do |d|
  region = d.config['awsRegion'].to_s
  puts "#{d.id}\t#{region.inspect}" unless region.match?(/\A[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\z/)
end

References

How to set up and validate locally

  1. Run the specs:

    bundle exec rspec \
  ee/spec/models/audit_events/amazon_s3_configuration_spec.rb \
  ee/spec/models/audit_events/instance/amazon_s3_configuration_spec.rb \
  ee/spec/models/audit_events/group/external_streaming_destination_spec.rb \
  ee/spec/models/audit_events/instance/external_streaming_destination_spec.rb

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Merge request reports