diff --git a/pkg/clouds/aws/cloudtrail_security_alerts.go b/pkg/clouds/aws/cloudtrail_security_alerts.go index d286d44e..0c3ed04b 100644 --- a/pkg/clouds/aws/cloudtrail_security_alerts.go +++ b/pkg/clouds/aws/cloudtrail_security_alerts.go @@ -52,8 +52,13 @@ func (c CloudTrailSecurityAlertsConfig) RequiresTrailValidation() bool { // CloudTrailAlertSelectors controls which security alerts are enabled. // Each field maps to a CloudWatch metric filter + alarm on the CloudTrail log group. -// Alert names reference AWS Security Hub/CIS CloudWatch controls (CloudWatch.1-14). +// Built-in detectors track AWS Security Hub/CIS CloudWatch controls (CloudWatch.1-14) +// plus a set of high-value additions covering attacker-blinding moves and exposure paths +// that CIS does not cover (GuardDuty/SecurityHub disable, IAM access keys, S3 Block Public +// Access, public Lambda Function URLs, KMS key policy edits, AWS Organizations changes, +// anonymous external probes). type CloudTrailAlertSelectors struct { + // CIS CloudWatch.1-14 RootAccountUsage bool `json:"rootAccountUsage,omitempty" yaml:"rootAccountUsage,omitempty"` // CIS CloudWatch.1 UnauthorizedApiCalls bool `json:"unauthorizedApiCalls,omitempty" yaml:"unauthorizedApiCalls,omitempty"` // CIS CloudWatch.2 ConsoleLoginWithoutMfa bool `json:"consoleLoginWithoutMfa,omitempty" yaml:"consoleLoginWithoutMfa,omitempty"` // CIS CloudWatch.3 @@ -68,6 +73,71 @@ type CloudTrailAlertSelectors struct { NetworkGatewayChanges bool `json:"networkGatewayChanges,omitempty" yaml:"networkGatewayChanges,omitempty"` // CIS CloudWatch.12 RouteTableChanges bool `json:"routeTableChanges,omitempty" yaml:"routeTableChanges,omitempty"` // CIS CloudWatch.13 VpcChanges bool `json:"vpcChanges,omitempty" yaml:"vpcChanges,omitempty"` // CIS CloudWatch.14 + + // Beyond-CIS detectors. Default off so existing deployments don't gain new alerts on plugin upgrade. + GuardDutyDisabled bool `json:"guardDutyDisabled,omitempty" yaml:"guardDutyDisabled,omitempty"` // GuardDuty disabled/detector deleted + SecurityHubDisabled bool `json:"securityHubDisabled,omitempty" yaml:"securityHubDisabled,omitempty"` // Security Hub disabled or standards turned off + AccessKeyCreation bool `json:"accessKeyCreation,omitempty" yaml:"accessKeyCreation,omitempty"` // CreateAccessKey on any IAM user + S3PublicAccessChanges bool `json:"s3PublicAccessChanges,omitempty" yaml:"s3PublicAccessChanges,omitempty"` // Block Public Access toggled at account or bucket scope + LambdaUrlPublic bool `json:"lambdaUrlPublic,omitempty" yaml:"lambdaUrlPublic,omitempty"` // Lambda Function URL created/updated with AuthType=NONE + // KMS-related detectors are split into two by signal density: + // KmsKeyPolicy — PutKeyPolicy: rare, real signal, default threshold 1. The structural + // change to "who can use this key" — page on any occurrence. + // KmsKeyGrants — CreateGrant / RetireGrant / RevokeGrant: high-volume in any IaC-driven + // environment (Pulumi/Terraform issue a grant per encrypted resource). + // Default off; turn on only with the override.threshold tuned to your + // deploy cadence. Splitting was driven by production data showing + // ~25 CreateGrant/hour from one Pulumi bot account that would have + // buried a single PutKeyPolicy signal. + KmsKeyPolicy bool `json:"kmsKeyPolicy,omitempty" yaml:"kmsKeyPolicy,omitempty"` // PutKeyPolicy (rare; default threshold 1) + KmsKeyGrants bool `json:"kmsKeyGrants,omitempty" yaml:"kmsKeyGrants,omitempty"` // CreateGrant / RetireGrant / RevokeGrant (high-volume; default threshold 10) + OrganizationsChanges bool `json:"organizationsChanges,omitempty" yaml:"organizationsChanges,omitempty"` // SCP / account-membership churn in AWS Organizations + AnonymousProbes bool `json:"anonymousProbes,omitempty" yaml:"anonymousProbes,omitempty"` // userIdentity.type=AWSAccount AccessDenied probes from public IPs + + // Overrides allows per-detector tuning without forking the plugin. Keyed by detector + // selector name (e.g. "iamPolicyChanges"). An override can: + // - bake exclusion clauses into the CloudWatch metric filter pattern (preferred + // for governed automation that contributes nothing but noise — Pulumi CI bots, + // known scanners, AWS service-linked roles); + // - raise the alarm threshold so a single matched event no longer trips the alarm + // (CIS Benchmark guidance for unauthorized-api-calls); + // - adjust the evaluation window. + // + // Suppression happens at the metric-filter layer, not in the Lambda enrichment + // step — excluded events never increment the metric, the alarm never trips, and + // no Slack notification is sent. The CW alarm dashboard therefore reflects real + // signal rather than known-good noise (preserves SOC2/ISO audit clarity). + Overrides map[string]CloudTrailAlertOverride `json:"overrides,omitempty" yaml:"overrides,omitempty"` +} + +// CloudTrailAlertOverride tunes a single detector. All fields are optional; zero values +// mean "use plugin default." +type CloudTrailAlertOverride struct { + // Threshold raises the alarm threshold (events per period). 0 = use the plugin default + // for this detector (1 for most, 5 for unauthorizedApiCalls, 10 for anonymousProbes). + Threshold float64 `json:"threshold,omitempty" yaml:"threshold,omitempty"` + // Period in seconds. 0 = 300 (5 min). CloudWatch supports 60, 300, 3600. + Period int `json:"period,omitempty" yaml:"period,omitempty"` + // EvaluationPeriods is how many consecutive periods must breach. 0 = 1. + EvaluationPeriods int `json:"evaluationPeriods,omitempty" yaml:"evaluationPeriods,omitempty"` + + // ExcludeUserNames excludes events where $.userIdentity.userName matches the given + // names exactly. Useful for IAMUser-type principals like CI bot accounts. + ExcludeUserNames []string `json:"excludeUserNames,omitempty" yaml:"excludeUserNames,omitempty"` + // ExcludePrincipalIds excludes by $.userIdentity.principalId (AIDA..., AROA..., AKIA...). + ExcludePrincipalIds []string `json:"excludePrincipalIds,omitempty" yaml:"excludePrincipalIds,omitempty"` + // ExcludeUserArns excludes by exact $.userIdentity.arn match. + ExcludeUserArns []string `json:"excludeUserArns,omitempty" yaml:"excludeUserArns,omitempty"` + // ExcludeUserArnGlobs excludes by $.userIdentity.arn glob (CloudWatch metric filter + // patterns support * within string values, e.g. + // "arn:aws:sts::*:assumed-role/AWSServiceRoleFor*/*"). One glob per list entry. + ExcludeUserArnGlobs []string `json:"excludeUserArnGlobs,omitempty" yaml:"excludeUserArnGlobs,omitempty"` + // ExcludeUserTypes excludes by $.userIdentity.type (e.g. "AWSService", "AWSAccount", + // "AssumedRole"). Useful for stripping AWS internal plumbing from unauthorized-api-calls. + ExcludeUserTypes []string `json:"excludeUserTypes,omitempty" yaml:"excludeUserTypes,omitempty"` + // ExcludeInvokedBy excludes by $.userIdentity.invokedBy (e.g. "s3.amazonaws.com"). + // This is the canonical way to strip AWS service self-probes. + ExcludeInvokedBy []string `json:"excludeInvokedBy,omitempty" yaml:"excludeInvokedBy,omitempty"` } func ReadCloudTrailSecurityAlertsConfig(config *api.Config) (api.Config, error) { diff --git a/pkg/clouds/pulumi/aws/cloudtrail_security_alerts.go b/pkg/clouds/pulumi/aws/cloudtrail_security_alerts.go index ca9f49c9..8c65e756 100644 --- a/pkg/clouds/pulumi/aws/cloudtrail_security_alerts.go +++ b/pkg/clouds/pulumi/aws/cloudtrail_security_alerts.go @@ -3,6 +3,7 @@ package aws import ( "fmt" "sort" + "strings" "github.com/pkg/errors" @@ -19,11 +20,14 @@ import ( ) // securityAlertDef defines a CloudTrail security alert with its metric filter pattern. +// Defaults (zero values) are documented per field. type securityAlertDef struct { - name string - description string - filterPattern string - threshold float64 // default 1 if 0 + name string + description string + filterPattern string + threshold float64 // default 1 when zero + period int // alarm period seconds; default 300 when zero + evaluationPeriods int // default 1 when zero } // securityAlerts maps CloudTrailAlertSelectors field names to their definitions. @@ -36,20 +40,49 @@ var securityAlerts = map[string]securityAlertDef{ description: "Root account API call detected — investigate immediately", filterPattern: `{ $.userIdentity.type = "Root" && $.userIdentity.invokedBy NOT EXISTS && $.eventType != "AwsServiceEvent" }`, }, - // CloudWatch.2 — Unauthorized API calls (threshold 5 to reduce noise from normal permission probing) + // CloudWatch.2 — Unauthorized API calls. + // + // Base filter excludes AWSService + AWSAccount userIdentity types by default: + // - AWSService: AWS service-linked roles (Macie/Config/ResourceExplorer/...) continuously + // probe optional services for capability discovery. 60% of historical events in our + // production sample. Zero security value: an attacker who compromises a service-linked + // role surfaces as AssumedRole, not AWSService. + // - AWSAccount: cross-account access from external accounts. Covered by the dedicated + // anonymousProbes detector (also threshold 10) so excluding here avoids double-paging + // on the same event. + // + // Default threshold is 10 events / 300s (raised from CIS's "1" guidance which is + // universally considered too noisy for any active account; aligned with what production + // data showed absorbs natural permission-evaluation noise without missing real bursts). "unauthorizedApiCalls": { name: "ct-unauthorized-api-calls", description: "Spike in unauthorized API calls — possible credential compromise or misconfiguration", - filterPattern: `{ ($.errorCode = "AccessDenied") || ($.errorCode = "AccessDeniedException") || ` + + filterPattern: `{ (($.errorCode = "AccessDenied") || ($.errorCode = "AccessDeniedException") || ` + `($.errorCode = "UnauthorizedAccess") || ($.errorCode = "Client.UnauthorizedAccess") || ` + - `($.errorCode = "Client.UnauthorizedOperation") || ($.errorCode = "UnauthorizedOperation") }`, - threshold: 5, + `($.errorCode = "Client.UnauthorizedOperation") || ($.errorCode = "UnauthorizedOperation")) ` + + `&& (($.userIdentity.type NOT EXISTS) || (($.userIdentity.type != "AWSService") && ($.userIdentity.type != "AWSAccount"))) }`, + threshold: 10, }, - // CloudWatch.3 — Console login without MFA (successful only) + // CloudWatch.3 — Console login without MFA (successful only). + // + // Scoped to userIdentity.type ∈ {IAMUser, Root}. AWS Identity Center / federated + // logins always emit ConsoleLogin events with additionalEventData.MFAUsed = "No" + // because MFA is enforced upstream at the IdP rather than at the AWS console step; + // without the IAMUser-or-Root scope the detector pages every SSO console session — + // a well-documented CIS CloudWatch.3 false positive. + // + // Root is explicitly included here because Root-without-MFA is the highest-severity + // outcome of this detector. The separate rootAccountUsage detector (CIS CloudWatch.1) + // catches ALL Root activity, including MFA-protected sessions; this one specifically + // surfaces the MFA-disabled subset for triage. Dropping Root from scope to fix the + // SSO FP would have silently created a gap. + // + // Identity Center sessions should be audited via signin.amazonaws.com + // UserAuthentication events (not currently provisioned by this plugin). "consoleLoginWithoutMfa": { name: "ct-console-login-no-mfa", - description: "Successful console login without MFA detected", - filterPattern: `{ ($.eventName = "ConsoleLogin") && ($.additionalEventData.MFAUsed != "Yes") && ($.responseElements.ConsoleLogin = "Success") }`, + description: "Successful IAM user or Root console login without MFA detected", + filterPattern: `{ ($.eventName = "ConsoleLogin") && ($.additionalEventData.MFAUsed != "Yes") && ($.responseElements.ConsoleLogin = "Success") && (($.userIdentity.type = "IAMUser") || ($.userIdentity.type = "Root")) }`, }, // CloudWatch.4 — IAM policy changes // SetDefaultPolicyVersion is included: flipping a managed policy's default version @@ -151,16 +184,154 @@ var securityAlerts = map[string]securityAlertDef{ `($.eventName = "DetachClassicLinkVpc") || ($.eventName = "DisableVpcClassicLink") || ` + `($.eventName = "EnableVpcClassicLink") }`, }, + + // Beyond-CIS — GuardDuty disabled/blinded. + // Why: CIS CloudWatch.9 (configChanges) tracks AWS Config recorder ops but not the + // other detective controls. Disabling GuardDuty is a classic attacker-blinding move + // because findings stop flowing and historical findings can be deleted. + "guardDutyDisabled": { + name: "ct-guardduty-disabled", + description: "GuardDuty detector disabled, deleted, or members removed — security visibility lost", + filterPattern: `{ ($.eventSource = "guardduty.amazonaws.com") && (($.eventName = "DeleteDetector") || ` + + `($.eventName = "UpdateDetector") || ($.eventName = "DisassociateMembers") || ` + + `($.eventName = "DeleteMembers") || ($.eventName = "StopMonitoringMembers")) }`, + }, + + // Beyond-CIS — Security Hub disabled or standards turned off. + // Why: same attacker-blinding category as GuardDuty. + // + // Note: Security Hub "Insights" are saved dashboard searches, not detectors; + // DeleteInsight is intentionally excluded — it's a UI/housekeeping action and + // generates noise for an alarm that's meant to catch "visibility lost" moves. + "securityHubDisabled": { + name: "ct-securityhub-disabled", + description: "Security Hub disabled or standards/import subscriptions turned off — security visibility lost", + filterPattern: `{ ($.eventSource = "securityhub.amazonaws.com") && (($.eventName = "DisableSecurityHub") || ` + + `($.eventName = "BatchDisableStandards") || ($.eventName = "DisableImportFindingsForProduct") || ` + + `($.eventName = "DeleteActionTarget")) }`, + }, + + // Beyond-CIS — IAM access key creation. + // Why: persistent credentials are higher-risk than short-lived STS tokens. Creation + // without a documented rotation/expiry path is a common audit finding. + "accessKeyCreation": { + name: "ct-access-key-creation", + description: "IAM access key created — verify rotation policy and owner attestation", + filterPattern: `{ ($.eventSource = "iam.amazonaws.com") && ($.eventName = "CreateAccessKey") }`, + }, + + // Beyond-CIS — S3 Block Public Access (BPA) toggled at account or bucket scope. + // Why: CIS CloudWatch.8 only catches bucket policy edits; BPA is the higher-leverage + // gate. Turning it off is a single click that exposes previously private buckets. + "s3PublicAccessChanges": { + name: "ct-s3-public-access-changes", + description: "S3 Block Public Access settings modified — possible exposure path opened", + filterPattern: `{ ($.eventSource = "s3.amazonaws.com") && (($.eventName = "PutAccountPublicAccessBlock") || ` + + `($.eventName = "PutBucketPublicAccessBlock") || ($.eventName = "DeleteAccountPublicAccessBlock") || ` + + `($.eventName = "DeleteBucketPublicAccessBlock")) }`, + }, + + // Beyond-CIS — Lambda Function URL created or updated with AuthType=NONE. + // Why: a Function URL with AuthType=NONE is a public HTTPS endpoint with the function's + // IAM role behind it; one click of misconfiguration exposes whatever the function can do. + // We observed anonymous Azure-IP scanners probing GetFunctionUrlConfig in the wild — they + // will hit a real endpoint the moment one exists. + "lambdaUrlPublic": { + name: "ct-lambda-url-public", + description: "Lambda Function URL created or updated with AuthType=NONE — public endpoint exposed", + filterPattern: `{ ($.eventSource = "lambda.amazonaws.com") && (($.eventName = "CreateFunctionUrlConfig") || ($.eventName = "UpdateFunctionUrlConfig")) && ($.requestParameters.authType = "NONE") }`, + }, + + // Beyond-CIS — KMS key policy edits. + // + // Scoped to PutKeyPolicy only. This is the structural change to "who can use this key" — + // a security boundary edit that should page on any occurrence. Other key-related signals + // have their own detectors: + // - Key deletion / disable: kmsKeyDeletion (CIS CloudWatch.7) + // - Grant lifecycle: kmsKeyGrants (below; high-volume; default off) + // + // PutResourcePolicy is not a KMS API — it exists on CloudTrail Lake and other services + // but never under eventSource=kms.amazonaws.com, so it would be dead code if included. + // API ref: https://docs.aws.amazon.com/kms/latest/APIReference/API_Operations.html + "kmsKeyPolicy": { + name: "ct-kms-key-policy", + description: "KMS key policy modified — verify principal scope and conditions", + filterPattern: `{ ($.eventSource = "kms.amazonaws.com") && ($.eventName = "PutKeyPolicy") }`, + }, + + // Beyond-CIS — KMS grant lifecycle. + // + // CreateGrant / RetireGrant / RevokeGrant. Any IaC tool (Pulumi, Terraform) issues at + // least one CreateGrant per resource that needs to use a KMS key. Production observation: + // ~25 CreateGrant/hour from one Pulumi bot during normal deploys. Coalescing this with + // PutKeyPolicy in a single detector with threshold 1 would either flood (current bug we + // fixed) or hide PutKeyPolicy under a high threshold needed to mute Pulumi noise. So this + // is its own detector with default threshold 10/300s — fires only on sustained grant + // churn beyond expected deploy cadence. Default OFF: most consumers won't want this on + // unless they have a specific need (compliance audit, suspected lateral movement via grants). + "kmsKeyGrants": { + name: "ct-kms-key-grants", + description: "KMS grant burst — sustained CreateGrant/RetireGrant/RevokeGrant outside expected deploy cadence", + filterPattern: `{ ($.eventSource = "kms.amazonaws.com") && (($.eventName = "CreateGrant") || ` + + `($.eventName = "RetireGrant") || ($.eventName = "RevokeGrant")) }`, + threshold: 10, + }, + + // Beyond-CIS — AWS Organizations changes. + // Why: SCPs are the strongest preventative control in a multi-account org. Detaching, + // deleting, or moving accounts to a different OU widens blast radius across every + // account it covered. Delegated-administrator events are the canonical "blind the + // management account" move: register a member account as the GuardDuty/Security Hub + // admin, then suppress findings there. + // + // Coverage by category: + // policies: CreatePolicy / DeletePolicy / UpdatePolicy / Attach/DetachPolicy / Enable/DisablePolicyType + // OU & accounts: MoveAccount / RemoveAccountFromOrganization / LeaveOrganization + // delegated admin: RegisterDelegatedAdministrator / DeregisterDelegatedAdministrator + // service trust: EnableAWSServiceAccess / DisableAWSServiceAccess + // + // API ref: https://docs.aws.amazon.com/organizations/latest/APIReference/API_Operations.html + "organizationsChanges": { + name: "ct-organizations-changes", + description: "AWS Organizations policy, OU membership, or delegated admin changed — verify boundary", + filterPattern: `{ ($.eventSource = "organizations.amazonaws.com") && (($.eventName = "CreatePolicy") || ` + + `($.eventName = "DeletePolicy") || ($.eventName = "UpdatePolicy") || ($.eventName = "AttachPolicy") || ` + + `($.eventName = "DetachPolicy") || ($.eventName = "EnablePolicyType") || ($.eventName = "DisablePolicyType") || ` + + `($.eventName = "MoveAccount") || ($.eventName = "RemoveAccountFromOrganization") || ` + + `($.eventName = "LeaveOrganization") || ($.eventName = "RegisterDelegatedAdministrator") || ` + + `($.eventName = "DeregisterDelegatedAdministrator") || ($.eventName = "EnableAWSServiceAccess") || ` + + `($.eventName = "DisableAWSServiceAccess")) }`, + }, + + // Beyond-CIS — Anonymous external probes (recon). + // Why: userIdentity.type=AWSAccount represents another AWS account (or unauthenticated + // AWS API client) hitting our resources. We observed ~400/14d hits in the wild scanning + // for exposed Lambda Function URLs. Default threshold=10 so individual probes don't + // page — we only care about sustained scanning from one source. + "anonymousProbes": { + name: "ct-anonymous-probes", + description: "Anonymous external account probing AWS resources — possible reconnaissance", + filterPattern: `{ ($.userIdentity.type = "AWSAccount") && (($.errorCode = "AccessDenied") || ` + + `($.errorCode = "AccessDeniedException") || ($.errorCode = "UnauthorizedAccess") || ` + + `($.errorCode = "Client.UnauthorizedAccess") || ($.errorCode = "Client.UnauthorizedOperation") || ` + + `($.errorCode = "UnauthorizedOperation")) }`, + threshold: 10, + }, } -// enabledAlerts returns the alert definitions that are enabled in the selector config. -// Results are sorted by name for deterministic Pulumi resource ordering. -func enabledAlerts(selectors awsApi.CloudTrailAlertSelectors) []securityAlertDef { - var result []securityAlertDef - checks := []struct { +// selectorChecks lists every bool selector keyed by the detector slug that maps it to +// a securityAlerts entry. The list is the single source of truth for "what detectors +// can be enabled" — reflection tests assert it covers every securityAlerts key, and the +// override validation step rejects map keys outside this set. +func selectorChecks(selectors awsApi.CloudTrailAlertSelectors) []struct { + key string + enabled bool +} { + return []struct { key string enabled bool }{ + // CIS CloudWatch.1-14 {"cloudTrailTampering", selectors.CloudTrailTampering}, {"configChanges", selectors.ConfigChanges}, {"consoleLoginWithoutMfa", selectors.ConsoleLoginWithoutMfa}, @@ -175,18 +346,193 @@ func enabledAlerts(selectors awsApi.CloudTrailAlertSelectors) []securityAlertDef {"securityGroupChanges", selectors.SecurityGroupChanges}, {"unauthorizedApiCalls", selectors.UnauthorizedApiCalls}, {"vpcChanges", selectors.VpcChanges}, + // Beyond-CIS + {"guardDutyDisabled", selectors.GuardDutyDisabled}, + {"securityHubDisabled", selectors.SecurityHubDisabled}, + {"accessKeyCreation", selectors.AccessKeyCreation}, + {"s3PublicAccessChanges", selectors.S3PublicAccessChanges}, + {"lambdaUrlPublic", selectors.LambdaUrlPublic}, + {"kmsKeyPolicy", selectors.KmsKeyPolicy}, + {"kmsKeyGrants", selectors.KmsKeyGrants}, + {"organizationsChanges", selectors.OrganizationsChanges}, + {"anonymousProbes", selectors.AnonymousProbes}, } - for _, c := range checks { - if c.enabled { - if def, ok := securityAlerts[c.key]; ok { - result = append(result, def) - } +} + +// validateOverrides returns an error when selectors.Overrides contains a key that +// doesn't correspond to any detector. Catches YAML typos like +// `overrides: { unauthorizedApiCall: ... }` (missing trailing s) — without this guard +// the override would be silently dropped at runtime and the operator would never know +// why their exclusion didn't take effect. +func validateOverrides(selectors awsApi.CloudTrailAlertSelectors) error { + if len(selectors.Overrides) == 0 { + return nil + } + known := map[string]struct{}{} + for _, c := range selectorChecks(selectors) { + known[c.key] = struct{}{} + } + var unknown []string + for k := range selectors.Overrides { + if _, ok := known[k]; !ok { + unknown = append(unknown, k) } } + if len(unknown) == 0 { + return nil + } + sort.Strings(unknown) + knownKeys := make([]string, 0, len(known)) + for k := range known { + knownKeys = append(knownKeys, k) + } + sort.Strings(knownKeys) + return errors.Errorf("unknown detector key(s) in alerts.overrides: %v (known: %v)", unknown, knownKeys) +} + +// enabledAlerts returns the alert definitions that are enabled in the selector config. +// Per-detector overrides (exclusions, threshold, period) are baked into the returned +// definitions here so downstream provisioning code sees a single resolved struct per alert. +// Results are sorted by name for deterministic Pulumi resource ordering. +func enabledAlerts(selectors awsApi.CloudTrailAlertSelectors) []securityAlertDef { + var result []securityAlertDef + for _, c := range selectorChecks(selectors) { + if !c.enabled { + continue + } + def, ok := securityAlerts[c.key] + if !ok { + continue + } + if ov, hasOv := selectors.Overrides[c.key]; hasOv { + def = applyOverride(def, ov) + } + result = append(result, def) + } sort.Slice(result, func(i, j int) bool { return result[i].name < result[j].name }) return result } +// applyOverride returns a copy of def with the override applied: +// - exclusion clauses are appended to the filter pattern as `&& (field != "val")`; +// - threshold/period/evaluationPeriods overrides replace the defaults when non-zero. +// +// The original filter pattern is wrapped in parentheses so the appended NOT-clauses +// AND with the entire base predicate, not just its last OR-term. Example: +// +// base: { ($.eventName = "PutRolePolicy") || ($.eventName = "AttachRolePolicy") } +// out: { ( ($.eventName = "PutRolePolicy") || ($.eventName = "AttachRolePolicy") ) && ($.userIdentity.userName != "integrail-deployer-bot") } +// +// CloudWatch metric filter pattern syntax reference: +// +// https://docs.aws.amazon.com/AmazonCloudWatch/latest/logs/FilterAndPatternSyntax.html +func applyOverride(def securityAlertDef, ov awsApi.CloudTrailAlertOverride) securityAlertDef { + if ov.Threshold > 0 { + def.threshold = ov.Threshold + } + if ov.Period > 0 { + def.period = ov.Period + } + if ov.EvaluationPeriods > 0 { + def.evaluationPeriods = ov.EvaluationPeriods + } + + clauses := buildExclusionClauses(ov) + if len(clauses) == 0 { + return def + } + + // Strip the outer braces from the base pattern so we can wrap the predicate + // in parens before AND'ing the exclusions. CloudWatch JSON filter patterns + // are always braced. + base := strings.TrimSpace(def.filterPattern) + base = strings.TrimPrefix(base, "{") + base = strings.TrimSuffix(base, "}") + base = strings.TrimSpace(base) + + def.filterPattern = "{ (" + base + ") && " + strings.Join(clauses, " && ") + " }" + return def +} + +// buildExclusionClauses turns an override into a list of clauses in deterministic order. +// The list is empty when the override has no exclusions. +// +// Each clause is guarded with a `NOT EXISTS` disjunction so that events where the field +// is absent on the top-level $.userIdentity object pass through the filter unchanged. +// This matters because CloudWatch metric filter patterns return FALSE for `$.field != "x"` +// when $.field is absent — without the guard, a single exclusion would silently mask +// every event whose userIdentity lacks that field. Concretely: AssumedRole events do not +// carry $.userIdentity.userName at the top level (the role's userName lives at +// $.userIdentity.sessionContext.sessionIssuer.userName), so an unguarded +// `$.userIdentity.userName != "bot"` would drop every assumed-role event from the +// detector, including ones that have nothing to do with the bot. +// +// Generated form per field is: +// +// ( ($.field NOT EXISTS) || ( ($.field != "v1") && ($.field != "v2") ) ) +// +// Reading it: "field is absent OR field is none of {v1, v2}". The inner conjunction is +// De Morgan'd from "NOT (field == v1 OR field == v2)" — we want to exclude any of the +// enumerated values, which means the event-must-match predicate is the AND of `!=`s. +// +// References: +// - Filter pattern: `!=` returns FALSE when field is missing; AND/OR/NOT EXISTS are +// all supported. +// https://docs.aws.amazon.com/AmazonCloudWatch/latest/logs/FilterAndPatternSyntax.html +// - CloudTrail userIdentity field shape (AssumedRole vs IAMUser): +// https://docs.aws.amazon.com/awscloudtrail/latest/userguide/cloudtrail-event-reference-user-identity.html +func buildExclusionClauses(ov awsApi.CloudTrailAlertOverride) []string { + var clauses []string + add := func(field string, values []string) { + // Sort to keep the generated filter pattern deterministic across deploys + // (so Pulumi sees a stable resource and doesn't churn the filter on every + // run when the user reorders the YAML list). + vs := append([]string(nil), values...) + sort.Strings(vs) + // De-dupe + skip empties — common YAML mistake is a trailing `-` producing + // an empty list entry; would otherwise emit `($.x != "")` clauses that match + // nothing useful and just pad the filter pattern length. + seen := map[string]struct{}{} + nonEmpty := make([]string, 0, len(vs)) + for _, v := range vs { + if v == "" { + continue + } + if _, dup := seen[v]; dup { + continue + } + seen[v] = struct{}{} + nonEmpty = append(nonEmpty, v) + } + if len(nonEmpty) == 0 { + return + } + // Single-value short form skips the inner AND wrapper: more readable in + // the generated filter pattern, identical semantics. + var inner string + if len(nonEmpty) == 1 { + inner = fmt.Sprintf(`($.%s != %q)`, field, nonEmpty[0]) + } else { + parts := make([]string, 0, len(nonEmpty)) + for _, v := range nonEmpty { + parts = append(parts, fmt.Sprintf(`($.%s != %q)`, field, v)) + } + inner = "(" + strings.Join(parts, " && ") + ")" + } + clause := fmt.Sprintf(`(($.%s NOT EXISTS) || %s)`, field, inner) + clauses = append(clauses, clause) + } + // Field order is fixed (not sorted by field name) so a user re-ordering YAML keys + // within a single override doesn't churn Pulumi state. + add("userIdentity.userName", ov.ExcludeUserNames) + add("userIdentity.principalId", ov.ExcludePrincipalIds) + add("userIdentity.arn", ov.ExcludeUserArns) + add("userIdentity.arn", ov.ExcludeUserArnGlobs) // CloudWatch supports * within the quoted value + add("userIdentity.type", ov.ExcludeUserTypes) + add("userIdentity.invokedBy", ov.ExcludeInvokedBy) + return clauses +} + // CloudTrailSecurityAlerts provisions CloudWatch metric filters and alarms // for security-relevant CloudTrail events. Each enabled alert creates: // - A LogMetricFilter on the CloudTrail log group @@ -213,6 +559,13 @@ func CloudTrailSecurityAlerts(ctx *sdk.Context, stack api.Stack, input api.Resou return nil, errors.New("logGroupName is required for CloudTrail security alerts") } + // Fail fast on `alerts.overrides` keys that don't match a known detector — silently + // dropping a typo would create a misleading "no exclusion applied" outcome that's + // very hard to diagnose from a noisy alert channel. + if err := validateOverrides(cfg.Alerts); err != nil { + return nil, err + } + // Pre-flight: if the user declared a trailName, verify the trail has // log-file validation turned on BEFORE we go ahead and provision metric // filters / alarms / Lambdas. Running the security-alerts stack on top @@ -348,6 +701,14 @@ func CloudTrailSecurityAlerts(ctx *sdk.Context, stack api.Stack, input api.Resou if threshold == 0 { threshold = 1 } + period := alertDef.period + if period == 0 { + period = 300 + } + evalPeriods := alertDef.evaluationPeriods + if evalPeriods == 0 { + evalPeriods = 1 + } // createAlert suffixes cfg.name with "-execution-role" (15 chars) and Pulumi adds // an 8-char random suffix when it auto-names the IAM role. Cap the base so the // resulting physical role name stays within AWS's 64-char limit (with headroom). @@ -363,8 +724,8 @@ func CloudTrailSecurityAlerts(ctx *sdk.Context, stack api.Stack, input api.Resou MetricName: sdk.String(metricName), Namespace: sdk.String(metricNamespace), Statistic: sdk.String("Sum"), - Period: sdk.Int(300), - EvaluationPeriods: sdk.Int(1), + Period: sdk.Int(period), + EvaluationPeriods: sdk.Int(evalPeriods), Threshold: sdk.Float64(threshold), ComparisonOperator: sdk.String("GreaterThanOrEqualToThreshold"), TreatMissingData: sdk.String("notBreaching"), diff --git a/pkg/clouds/pulumi/aws/cloudtrail_security_alerts_test.go b/pkg/clouds/pulumi/aws/cloudtrail_security_alerts_test.go index fdf2fedc..38328a3c 100644 --- a/pkg/clouds/pulumi/aws/cloudtrail_security_alerts_test.go +++ b/pkg/clouds/pulumi/aws/cloudtrail_security_alerts_test.go @@ -1,6 +1,8 @@ package aws import ( + "reflect" + "strings" "testing" . "github.com/onsi/gomega" @@ -8,10 +10,17 @@ import ( awsApi "github.com/simple-container-com/api/pkg/clouds/aws" ) +// totalDetectors is the count of built-in security detectors. Update when adding new ones. +// Composition: 14 CIS CloudWatch.1-14 + 9 beyond-CIS additions +// (kmsKeyPolicy + kmsKeyGrants count as two; they were split from the prior kmsKeyPolicyChanges +// because the events have different signal density and warrant different thresholds). +const totalDetectors = 23 + func TestEnabledAlerts_AllEnabled(t *testing.T) { RegisterTestingT(t) selectors := awsApi.CloudTrailAlertSelectors{ + // CIS RootAccountUsage: true, UnauthorizedApiCalls: true, ConsoleLoginWithoutMfa: true, @@ -26,10 +35,20 @@ func TestEnabledAlerts_AllEnabled(t *testing.T) { NetworkGatewayChanges: true, RouteTableChanges: true, VpcChanges: true, + // Beyond-CIS + GuardDutyDisabled: true, + SecurityHubDisabled: true, + AccessKeyCreation: true, + S3PublicAccessChanges: true, + LambdaUrlPublic: true, + KmsKeyPolicy: true, + KmsKeyGrants: true, + OrganizationsChanges: true, + AnonymousProbes: true, } alerts := enabledAlerts(selectors) - Expect(alerts).To(HaveLen(14)) + Expect(alerts).To(HaveLen(totalDetectors)) names := make(map[string]bool) for _, a := range alerts { @@ -38,20 +57,117 @@ func TestEnabledAlerts_AllEnabled(t *testing.T) { Expect(a.filterPattern).ToNot(BeEmpty()) } - Expect(names).To(HaveKey("ct-root-account-usage")) - Expect(names).To(HaveKey("ct-unauthorized-api-calls")) - Expect(names).To(HaveKey("ct-console-login-no-mfa")) - Expect(names).To(HaveKey("ct-iam-policy-changes")) - Expect(names).To(HaveKey("ct-cloudtrail-tampering")) - Expect(names).To(HaveKey("ct-failed-console-logins")) - Expect(names).To(HaveKey("ct-kms-key-deletion")) - Expect(names).To(HaveKey("ct-s3-bucket-policy-changes")) - Expect(names).To(HaveKey("ct-config-changes")) - Expect(names).To(HaveKey("ct-security-group-changes")) - Expect(names).To(HaveKey("ct-nacl-changes")) - Expect(names).To(HaveKey("ct-network-gateway-changes")) - Expect(names).To(HaveKey("ct-route-table-changes")) - Expect(names).To(HaveKey("ct-vpc-changes")) + for _, expected := range []string{ + "ct-root-account-usage", + "ct-unauthorized-api-calls", + "ct-console-login-no-mfa", + "ct-iam-policy-changes", + "ct-cloudtrail-tampering", + "ct-failed-console-logins", + "ct-kms-key-deletion", + "ct-s3-bucket-policy-changes", + "ct-config-changes", + "ct-security-group-changes", + "ct-nacl-changes", + "ct-network-gateway-changes", + "ct-route-table-changes", + "ct-vpc-changes", + "ct-guardduty-disabled", + "ct-securityhub-disabled", + "ct-access-key-creation", + "ct-s3-public-access-changes", + "ct-lambda-url-public", + "ct-kms-key-policy", + "ct-kms-key-grants", + "ct-organizations-changes", + "ct-anonymous-probes", + } { + Expect(names).To(HaveKey(expected)) + } +} + +func TestConsoleLoginWithoutMfa_RestrictedToIAMUser(t *testing.T) { + // The CIS CloudWatch.3 pattern fires on AWS Identity Center sessions by default + // because MFAUsed=No (MFA happens upstream). Constraining to IAMUser type avoids + // this well-known false positive. + RegisterTestingT(t) + Expect(securityAlerts["consoleLoginWithoutMfa"].filterPattern).To( + ContainSubstring(`$.userIdentity.type = "IAMUser"`)) +} + +func TestAnonymousProbes_DefaultThreshold(t *testing.T) { + // Single anonymous probe is not actionable; default threshold 10 in 5min reflects + // "sustained reconnaissance" rather than one-off enumeration. + RegisterTestingT(t) + Expect(securityAlerts["anonymousProbes"].threshold).To(Equal(float64(10))) +} + +func TestUnauthorizedApiCalls_DefaultThresholdIsTen(t *testing.T) { + // CIS Benchmark's "1 event = alert" is too noisy for any active AWS account; + // production data showed 10/300s absorbs natural permission-evaluation noise + // (eventual consistency, mistyped CLI commands, optional-service self-probes) + // without missing real bursts. + RegisterTestingT(t) + Expect(securityAlerts["unauthorizedApiCalls"].threshold).To(Equal(float64(10))) +} + +func TestUnauthorizedApiCalls_DefaultExcludesAWSService(t *testing.T) { + // AWS service-linked roles continuously probe optional services for capability + // discovery (Macie scanning S3 buckets that aren't enrolled, etc.). 60% of + // historical events in our production sample were AWSService-type AccessDenied + // — pure noise that adds zero security signal in every AWS account. Excluding + // at the BASE filter (not just via consumer override) makes the detector quiet + // out-of-the-box. NOT EXISTS guard preserves events where the field is absent. + RegisterTestingT(t) + p := securityAlerts["unauthorizedApiCalls"].filterPattern + Expect(p).To(ContainSubstring(`($.userIdentity.type NOT EXISTS)`)) + Expect(p).To(ContainSubstring(`$.userIdentity.type != "AWSService"`)) +} + +func TestUnauthorizedApiCalls_DefaultExcludesAWSAccount(t *testing.T) { + // AWSAccount cross-account AccessDenied events are covered by the dedicated + // anonymousProbes detector (same threshold). Excluding here avoids double-paging + // the same event class. Same NOT EXISTS guard. + RegisterTestingT(t) + p := securityAlerts["unauthorizedApiCalls"].filterPattern + Expect(p).To(ContainSubstring(`$.userIdentity.type != "AWSAccount"`)) +} + +func TestKmsKeyPolicy_HighSignalDefault(t *testing.T) { + // kmsKeyPolicy is the structural "who can use this key" change detector. Default + // threshold 1 — page on any PutKeyPolicy. Scoped to PutKeyPolicy only; grants + // live in the separate kmsKeyGrants detector. + RegisterTestingT(t) + def := securityAlerts["kmsKeyPolicy"] + Expect(def.filterPattern).To(ContainSubstring(`$.eventName = "PutKeyPolicy"`)) + Expect(def.filterPattern).ToNot(ContainSubstring(`CreateGrant`), + "kmsKeyPolicy must NOT include grants — they belong in kmsKeyGrants") + Expect(def.filterPattern).ToNot(ContainSubstring(`RetireGrant`)) + Expect(def.filterPattern).ToNot(ContainSubstring(`RevokeGrant`)) + Expect(def.threshold).To(Equal(float64(0)), + "kmsKeyPolicy uses the default threshold (1) which is encoded as zero in the def") +} + +func TestKmsKeyGrants_HighVolumeDefault(t *testing.T) { + // kmsKeyGrants is the high-volume detector. Default threshold 10/300s because + // any IaC tool issues a CreateGrant per resource that needs KMS — at typical + // deploy cadence ~25/hour from one bot. + RegisterTestingT(t) + def := securityAlerts["kmsKeyGrants"] + Expect(def.filterPattern).To(ContainSubstring(`CreateGrant`)) + Expect(def.filterPattern).To(ContainSubstring(`RetireGrant`)) + Expect(def.filterPattern).To(ContainSubstring(`RevokeGrant`)) + Expect(def.filterPattern).ToNot(ContainSubstring(`PutKeyPolicy`), + "kmsKeyGrants must NOT include PutKeyPolicy — it belongs in kmsKeyPolicy") + Expect(def.threshold).To(Equal(float64(10))) +} + +func TestKmsKeyPolicyChanges_OldNameRemoved(t *testing.T) { + // The old aggregate name kmsKeyPolicyChanges was split into kmsKeyPolicy + + // kmsKeyGrants. The old name must be gone from securityAlerts so that + // validateOverrides flags any consumer YAML still using it. + RegisterTestingT(t) + Expect(securityAlerts).ToNot(HaveKey("kmsKeyPolicyChanges")) } func TestEnabledAlerts_PartialEnabled(t *testing.T) { @@ -101,7 +217,7 @@ func TestEnabledAlerts_Deterministic(t *testing.T) { func TestSecurityAlertDefinitions(t *testing.T) { RegisterTestingT(t) - Expect(securityAlerts).To(HaveLen(14)) + Expect(securityAlerts).To(HaveLen(totalDetectors)) for key, def := range securityAlerts { t.Run(key, func(t *testing.T) { @@ -114,6 +230,253 @@ func TestSecurityAlertDefinitions(t *testing.T) { } } +func TestApplyOverride_Exclusions(t *testing.T) { + RegisterTestingT(t) + + base := securityAlerts["iamPolicyChanges"] + out := applyOverride(base, awsApi.CloudTrailAlertOverride{ + ExcludeUserNames: []string{"integrail-deployer-bot"}, + }) + + // Base pattern is preserved (wrapped in parens), exclusion is AND'd on + Expect(out.filterPattern).To(HavePrefix(`{ (`)) + Expect(out.filterPattern).To(HaveSuffix(` }`)) + Expect(out.filterPattern).To(ContainSubstring(`PutRolePolicy`)) // base predicate intact + + // Single-value exclusion form: (NOT EXISTS) || (!= "value") + // The NOT EXISTS guard keeps events where the field is absent (e.g., + // AssumedRole events lacking top-level $.userIdentity.userName) IN the + // detector; without it, every assumed-role event would silently bypass + // the alarm. + Expect(out.filterPattern).To(ContainSubstring(`(($.userIdentity.userName NOT EXISTS) || ($.userIdentity.userName != "integrail-deployer-bot"))`)) +} + +func TestApplyOverride_NotExistsGuard_MultipleValues(t *testing.T) { + // Multi-value form uses inner-AND: De Morgan'd "NOT (v1 OR v2)" = "!= v1 AND != v2". + // The OR with NOT EXISTS still keeps absent-field events flowing through. + RegisterTestingT(t) + + out := applyOverride(securityAlerts["iamPolicyChanges"], awsApi.CloudTrailAlertOverride{ + ExcludeUserNames: []string{"alpha", "beta", "gamma"}, + }) + Expect(out.filterPattern).To(ContainSubstring( + `(($.userIdentity.userName NOT EXISTS) || (($.userIdentity.userName != "alpha") && ($.userIdentity.userName != "beta") && ($.userIdentity.userName != "gamma")))`, + )) +} + +func TestApplyOverride_MultipleExclusionFields(t *testing.T) { + RegisterTestingT(t) + + out := applyOverride(securityAlerts["unauthorizedApiCalls"], awsApi.CloudTrailAlertOverride{ + ExcludeUserNames: []string{"prowler-readonly"}, + ExcludeUserTypes: []string{"AWSService", "AWSAccount"}, + ExcludeInvokedBy: []string{"s3.amazonaws.com", "lambda.amazonaws.com"}, + ExcludeUserArnGlobs: []string{ + "arn:aws:sts::*:assumed-role/AWSServiceRoleFor*/*", + }, + }) + + // Each exclusion field gets its own NOT EXISTS guard. + Expect(out.filterPattern).To(ContainSubstring(`($.userIdentity.userName NOT EXISTS)`)) + Expect(out.filterPattern).To(ContainSubstring(`($.userIdentity.type NOT EXISTS)`)) + Expect(out.filterPattern).To(ContainSubstring(`($.userIdentity.invokedBy NOT EXISTS)`)) + Expect(out.filterPattern).To(ContainSubstring(`($.userIdentity.arn NOT EXISTS)`)) + // Values still wired up correctly: + Expect(out.filterPattern).To(ContainSubstring(`"prowler-readonly"`)) + Expect(out.filterPattern).To(ContainSubstring(`"AWSService"`)) + Expect(out.filterPattern).To(ContainSubstring(`"AWSAccount"`)) + Expect(out.filterPattern).To(ContainSubstring(`"s3.amazonaws.com"`)) + Expect(out.filterPattern).To(ContainSubstring(`"lambda.amazonaws.com"`)) + Expect(out.filterPattern).To(ContainSubstring(`"arn:aws:sts::*:assumed-role/AWSServiceRoleFor*/*"`)) +} + +func TestApplyOverride_Deterministic(t *testing.T) { + // Re-ordered input lists must produce the same filter pattern so Pulumi sees no diff. + RegisterTestingT(t) + + ovA := awsApi.CloudTrailAlertOverride{ + ExcludeUserNames: []string{"alpha", "beta", "gamma"}, + ExcludeUserTypes: []string{"AWSService", "AWSAccount"}, + } + ovB := awsApi.CloudTrailAlertOverride{ + ExcludeUserNames: []string{"gamma", "alpha", "beta"}, + ExcludeUserTypes: []string{"AWSAccount", "AWSService"}, + } + + a := applyOverride(securityAlerts["iamPolicyChanges"], ovA).filterPattern + b := applyOverride(securityAlerts["iamPolicyChanges"], ovB).filterPattern + Expect(a).To(Equal(b)) +} + +func TestApplyOverride_ThresholdAndPeriod(t *testing.T) { + RegisterTestingT(t) + + out := applyOverride(securityAlerts["unauthorizedApiCalls"], awsApi.CloudTrailAlertOverride{ + Threshold: 10, + Period: 600, + EvaluationPeriods: 2, + }) + Expect(out.threshold).To(Equal(float64(10))) + Expect(out.period).To(Equal(600)) + Expect(out.evaluationPeriods).To(Equal(2)) +} + +func TestApplyOverride_EmptyOverrideIsNoop(t *testing.T) { + RegisterTestingT(t) + + base := securityAlerts["iamPolicyChanges"] + out := applyOverride(base, awsApi.CloudTrailAlertOverride{}) + Expect(out.filterPattern).To(Equal(base.filterPattern)) + Expect(out.threshold).To(Equal(base.threshold)) +} + +// TestApplyOverride_WorstCaseBasePattern guards the trim-and-rewrap logic that wraps +// the base pattern in parens before AND'ing exclusions. The shape we have to preserve: +// - leading/trailing whitespace inside the braces (idiomatic indentation), +// - internal parentheses from existing OR-groups, +// - mixed AND/OR/&& at the top level. +// If a future contributor writes a pattern with leading whitespace or extra braces, +// the wrap output should still be syntactically valid CloudWatch. +func TestApplyOverride_WorstCaseBasePattern(t *testing.T) { + RegisterTestingT(t) + + // Synthetic worst-case: leading whitespace, internal OR-group, trailing whitespace, + // AND with a sub-expression. + worst := securityAlertDef{ + name: "worst-case", + description: "synthetic", + filterPattern: `{ ($.eventName = "A") || ($.eventName = "B") && ($.eventSource = "x.amazonaws.com") }`, + } + out := applyOverride(worst, awsApi.CloudTrailAlertOverride{ + ExcludeUserNames: []string{"bot"}, + }) + + // Output must still be a single braced expression + Expect(out.filterPattern).To(HavePrefix("{ ")) + Expect(out.filterPattern).To(HaveSuffix(" }")) + // Base predicate atoms intact + Expect(out.filterPattern).To(ContainSubstring(`($.eventName = "A")`)) + Expect(out.filterPattern).To(ContainSubstring(`($.eventName = "B")`)) + Expect(out.filterPattern).To(ContainSubstring(`($.eventSource = "x.amazonaws.com")`)) + // Exclusion present + Expect(out.filterPattern).To(ContainSubstring(`"bot"`)) + // Top-level structure: base wrapped in parens, then `&&` to exclusion + Expect(out.filterPattern).To(MatchRegexp(`^\{ \(.*\) && \(.*\) \}$`)) +} + +func TestApplyOverride_EmptyStringsSkipped(t *testing.T) { + // Empty list entries (common YAML mistake: trailing `-`) must not produce + // nonsense clauses like `($.x != "")`. Each non-empty value still contributes + // a NOT-EXISTS-guarded clause: in single-value form we get the field name twice + // (once for NOT EXISTS, once for !=). + RegisterTestingT(t) + + out := applyOverride(securityAlerts["iamPolicyChanges"], awsApi.CloudTrailAlertOverride{ + ExcludeUserNames: []string{"", "real-bot", ""}, + }) + // Single-value clause shape: (($.field NOT EXISTS) || ($.field != "real-bot")) + // — two occurrences of $.userIdentity.userName, no empty-string match. + Expect(strings.Count(out.filterPattern, `$.userIdentity.userName`)).To(Equal(2)) + Expect(out.filterPattern).To(ContainSubstring(`"real-bot"`)) + Expect(out.filterPattern).ToNot(ContainSubstring(`!= ""`)) +} + +func TestApplyOverride_DeDupesValues(t *testing.T) { + // User pastes the same exclusion twice — generated filter must dedupe so we + // don't emit a redundant `&& ($.field != "v") && ($.field != "v")`. + RegisterTestingT(t) + + out := applyOverride(securityAlerts["iamPolicyChanges"], awsApi.CloudTrailAlertOverride{ + ExcludeUserNames: []string{"bot", "bot", "bot"}, + }) + Expect(strings.Count(out.filterPattern, `"bot"`)).To(Equal(1)) +} + +func TestEnabledAlerts_OverrideApplied(t *testing.T) { + RegisterTestingT(t) + + selectors := awsApi.CloudTrailAlertSelectors{ + IamPolicyChanges: true, + Overrides: map[string]awsApi.CloudTrailAlertOverride{ + "iamPolicyChanges": { + ExcludeUserNames: []string{"integrail-deployer-bot"}, + }, + }, + } + alerts := enabledAlerts(selectors) + Expect(alerts).To(HaveLen(1)) + Expect(alerts[0].name).To(Equal("ct-iam-policy-changes")) + Expect(alerts[0].filterPattern).To(ContainSubstring(`"integrail-deployer-bot"`)) +} + +// TestSelectorChecksWireUpAllDetectors guards against the regression where a contributor +// adds a new bool to CloudTrailAlertSelectors + a new entry to securityAlerts but forgets +// to wire it through selectorChecks. Without this test, the detector would never appear +// in enabledAlerts and the new toggle would be silently dead. +func TestSelectorChecksWireUpAllDetectors(t *testing.T) { + RegisterTestingT(t) + + // Flip every bool selector on via reflection. selectorChecks then enumerates + // the (key, enabled) pairs; we assert each securityAlerts key appears exactly once. + sel := awsApi.CloudTrailAlertSelectors{} + v := reflect.ValueOf(&sel).Elem() + for i := 0; i < v.NumField(); i++ { + f := v.Field(i) + if f.Kind() == reflect.Bool { + f.SetBool(true) + } + } + + checkedKeys := map[string]int{} + for _, c := range selectorChecks(sel) { + Expect(c.enabled).To(BeTrue(), "selectorChecks did not pick up the bool for %q (likely missing wireup)", c.key) + checkedKeys[c.key]++ + } + + // Bidirectional: every securityAlerts key must be in selectorChecks + for key := range securityAlerts { + Expect(checkedKeys).To(HaveKey(key), "securityAlerts has %q but selectorChecks doesn't wire it up", key) + Expect(checkedKeys[key]).To(Equal(1), "selectorChecks lists %q more than once", key) + } + // And every selectorChecks key must be in securityAlerts + for key := range checkedKeys { + Expect(securityAlerts).To(HaveKey(key), "selectorChecks lists %q but securityAlerts has no entry", key) + } + // And the count must equal totalDetectors (catches half-applied additions) + Expect(checkedKeys).To(HaveLen(totalDetectors)) +} + +func TestValidateOverrides_Empty(t *testing.T) { + RegisterTestingT(t) + Expect(validateOverrides(awsApi.CloudTrailAlertSelectors{})).To(Succeed()) +} + +func TestValidateOverrides_KnownKey(t *testing.T) { + RegisterTestingT(t) + err := validateOverrides(awsApi.CloudTrailAlertSelectors{ + Overrides: map[string]awsApi.CloudTrailAlertOverride{ + "iamPolicyChanges": {ExcludeUserNames: []string{"bot"}}, + }, + }) + Expect(err).To(Succeed()) +} + +func TestValidateOverrides_UnknownKeyIsLoudError(t *testing.T) { + // Common YAML mistake: misspell the detector key. Without validation the override + // is silently dropped and the operator gets no signal — they just keep seeing the + // alarm fire and wonder why their exclusion didn't take. Fail at deploy time instead. + RegisterTestingT(t) + err := validateOverrides(awsApi.CloudTrailAlertSelectors{ + Overrides: map[string]awsApi.CloudTrailAlertOverride{ + "unauthorizedApiCall": {ExcludeUserNames: []string{"bot"}}, // missing trailing 's' + }, + }) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("unauthorizedApiCall")) + // Should hint at what the user can use + Expect(err.Error()).To(ContainSubstring("known")) +} + func TestSecurityAlertDefinitions_UniqueNames(t *testing.T) { RegisterTestingT(t)