From 837ecde24df60453cdd331e672385a69e9842c7b Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Fri, 5 Jun 2026 17:45:05 -0400 Subject: [PATCH 1/3] fix(cloud-security): make cloudwatch metric-filter auto-fix reliable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Customer (aymenrc) reported AWS auto-fix falling back to manual steps with "required parameters were not properly provided… metric transformations were not properly provided to the CloudWatch Logs API" on the CIS CloudWatch metric-filter checks. That finding IS meant to be auto-fixable (PutMetricFilter + CreateTopic + PutMetricAlarm — all standard API calls), so this is a bug, not an inherently-manual finding. Root cause: PutMetricFilterCommand was absent from REQUIRED_PARAMS, so a step with a missing/mis-shaped `metricTransformations` wasn't caught before execution — it went to AWS, got rejected, and fell back to manual steps. The model commonly emits metricTransformations as a single object instead of an array, or metricValue as the number 1 instead of "1", both of which AWS rejects. - aws-command-executor: add PutMetricFilterCommand to REQUIRED_PARAMS (logGroupName, filterName, filterPattern, metricTransformations) so a missing field fails fast and triggers the existing AI repair pass; and add a deterministic normalizer (normalizeMetricFilterTransformations) that wraps a single transformation object in an array and coerces metricValue to a string. - ai-remediation.prompt: explicit PutMetricFilter shape guidance (metricTransformations array, string metricValue, real logGroupName). - Tests: REQUIRED_PARAMS enforcement + normalizer cases. Full cloud-security suite green (310 passing); changed files typecheck clean. Not yet verified against a live failing finding — covers the failure class. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../cloud-security/ai-remediation.prompt.ts | 6 ++ .../aws-command-executor.spec.ts | 89 +++++++++++++++++++ .../cloud-security/aws-command-executor.ts | 50 +++++++++++ 3 files changed, 145 insertions(+) diff --git a/apps/api/src/cloud-security/ai-remediation.prompt.ts b/apps/api/src/cloud-security/ai-remediation.prompt.ts index 03dbd0366..3624a4698 100644 --- a/apps/api/src/cloud-security/ai-remediation.prompt.ts +++ b/apps/api/src/cloud-security/ai-remediation.prompt.ts @@ -171,6 +171,12 @@ A human will ALWAYS review your plan before execution. Be precise and correct. - To make a recorder record ALL supported resource types, first read the existing recorder with DescribeConfigurationRecordersCommand (service "config-service", in readSteps) to get its exact name and roleARN, then call PutConfigurationRecorderCommand with ConfigurationRecorder = { name, roleARN, recordingGroup: { allSupported: true, includeGlobalResourceTypes: true } }. - NEVER set allSupported:true together with recordingStrategy, exclusionByResourceTypes, or resourceTypes — they are mutually exclusive and AWS rejects the request with a ValidationException. Omit those fields entirely (this also overwrites an existing exclusion-based strategy so global IAM resources are recorded). +## CLOUDWATCH METRIC FILTERS (IMPORTANT) +- For logs:PutMetricFilterCommand (service "cloudwatch-logs"), ALL of these are required: logGroupName, filterName, filterPattern, and metricTransformations. +- metricTransformations MUST be an ARRAY of objects (never a single object), and each object MUST set metricName, metricNamespace, and metricValue. Example: "metricTransformations": [{ "metricName": "compai-cis-metric", "metricNamespace": "CloudTrailMetrics", "metricValue": "1" }]. +- metricValue MUST be a STRING ("1"), not the number 1 — AWS rejects a numeric metricValue. +- logGroupName must be the REAL CloudTrail CloudWatch Logs group name from the read step — never a placeholder. + ## IDEMPOTENCY (CRITICAL) - All fix steps MUST be safe to run even if the resource already exists - For Create operations: our executor automatically handles "already exists" errors — they are treated as success, not failure diff --git a/apps/api/src/cloud-security/aws-command-executor.spec.ts b/apps/api/src/cloud-security/aws-command-executor.spec.ts index 94748773a..4b875fa53 100644 --- a/apps/api/src/cloud-security/aws-command-executor.spec.ts +++ b/apps/api/src/cloud-security/aws-command-executor.spec.ts @@ -3,6 +3,7 @@ import { REQUIRED_PARAMS, looksLikeValidationError, normalizeConfigRecordingGroup, + normalizeMetricFilterTransformations, validatePlanSteps, } from './aws-command-executor'; @@ -401,3 +402,91 @@ describe('normalizeConfigRecordingGroup', () => { expect(input2).toEqual({ ConfigurationRecorder: { name: 'default' } }); }); }); + +/** + * CloudWatch metric filters: `metricTransformations` is a required, non-empty + * array whose entries' `metricValue` must be a string. The model often emits a + * single object or a numeric metricValue, which AWS rejects ("metric + * transformations were not properly provided…") and sends the auto-fix to + * manual steps. normalizeMetricFilterTransformations coerces the valid shape; + * REQUIRED_PARAMS catches a truly-missing field before execution. + */ +describe('PutMetricFilterCommand required params + normalization', () => { + it('enforces the required PutMetricFilter params', () => { + const errors = validatePlanSteps([ + step({ + service: 'cloudwatch-logs', + command: 'PutMetricFilterCommand', + params: { logGroupName: 'lg', filterName: 'fn' }, // missing pattern + transforms + }), + ]); + expect(errors).toEqual( + expect.arrayContaining([ + expect.stringMatching(/Required param "filterPattern" is missing/), + expect.stringMatching(/Required param "metricTransformations" is missing/), + ]), + ); + }); + + it('does not error when all PutMetricFilter params are present', () => { + const errors = validatePlanSteps([ + step({ + service: 'cloudwatch-logs', + command: 'PutMetricFilterCommand', + params: { + logGroupName: 'lg', + filterName: 'fn', + filterPattern: '{ $.eventName = "X" }', + metricTransformations: [ + { metricName: 'm', metricNamespace: 'CloudTrailMetrics', metricValue: '1' }, + ], + }, + }), + ]); + expect( + errors.filter((e) => e.includes('PutMetricFilterCommand')), + ).toHaveLength(0); + }); + + it('wraps a single metricTransformations object in an array', () => { + const input: Record = { + logGroupName: 'lg', + metricTransformations: { + metricName: 'm', + metricNamespace: 'CloudTrailMetrics', + metricValue: '1', + }, + }; + normalizeMetricFilterTransformations(input); + expect(input.metricTransformations).toEqual([ + { metricName: 'm', metricNamespace: 'CloudTrailMetrics', metricValue: '1' }, + ]); + }); + + it('coerces a numeric metricValue to a string', () => { + const input: Record = { + metricTransformations: [ + { metricName: 'm', metricNamespace: 'CloudTrailMetrics', metricValue: 1 }, + ], + }; + normalizeMetricFilterTransformations(input); + expect(input.metricTransformations).toEqual([ + { metricName: 'm', metricNamespace: 'CloudTrailMetrics', metricValue: '1' }, + ]); + }); + + it('leaves a well-formed metricTransformations array untouched', () => { + const good = [ + { metricName: 'm', metricNamespace: 'CloudTrailMetrics', metricValue: '1' }, + ]; + const input: Record = { metricTransformations: good }; + normalizeMetricFilterTransformations(input); + expect(input.metricTransformations).toEqual(good); + }); + + it('is a no-op when metricTransformations is absent', () => { + const input: Record = { logGroupName: 'lg' }; + expect(() => normalizeMetricFilterTransformations(input)).not.toThrow(); + expect(input).toEqual({ logGroupName: 'lg' }); + }); +}); diff --git a/apps/api/src/cloud-security/aws-command-executor.ts b/apps/api/src/cloud-security/aws-command-executor.ts index 5893beee6..ec786902c 100644 --- a/apps/api/src/cloud-security/aws-command-executor.ts +++ b/apps/api/src/cloud-security/aws-command-executor.ts @@ -165,6 +165,12 @@ export const REQUIRED_PARAMS: Record = { StartConfigurationRecorderCommand: ['ConfigurationRecorderName'], PutBucketPolicyCommand: ['Bucket', 'Policy'], CreateTrailCommand: ['Name', 'S3BucketName'], + PutMetricFilterCommand: [ + 'logGroupName', + 'filterName', + 'filterPattern', + 'metricTransformations', + ], }; const REQUIRED_PARAM_ONE_OF: Record = { @@ -276,6 +282,50 @@ function normaliseInputParams( if (command === 'PutConfigurationRecorderCommand') { normalizeConfigRecordingGroup(input); } + + // Rule 5: CloudWatch Logs metric filters — `metricTransformations` is a + // required, NON-EMPTY ARRAY of { metricName, metricNamespace, metricValue } + // where `metricValue` must be a STRING. The model frequently emits it as a + // single object instead of an array, or as a number `1` instead of `"1"`, + // which AWS rejects (the customer-visible "metric transformations were not + // properly provided to the CloudWatch Logs API" failure that sends the + // auto-fix to manual steps). Coerce to the one valid shape. + if (command === 'PutMetricFilterCommand') { + normalizeMetricFilterTransformations(input); + } +} + +export function normalizeMetricFilterTransformations( + input: Record, +): void { + let transformations = input.metricTransformations; + + // A single transformation object → wrap in an array (AWS expects a list). + if ( + transformations && + typeof transformations === 'object' && + !Array.isArray(transformations) + ) { + transformations = [transformations]; + input.metricTransformations = transformations; + } + + if (!Array.isArray(transformations)) return; + + input.metricTransformations = transformations.map((entry) => { + if (!entry || typeof entry !== 'object' || Array.isArray(entry)) { + return entry; + } + const transform = entry as Record; + // metricValue must be a string (e.g. "1"); the model often emits a number. + if ( + transform.metricValue != null && + typeof transform.metricValue !== 'string' + ) { + return { ...transform, metricValue: String(transform.metricValue) }; + } + return transform; + }); } export function normalizeConfigRecordingGroup( From f8f2908f64080d9510d2473690934f75052d0fb7 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Fri, 5 Jun 2026 18:19:20 -0400 Subject: [PATCH 2/3] fix(cloud-security): allow an empty filterPattern for PutMetricFilter validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cubic review on #3050: PutMetricFilterCommand had `filterPattern` in REQUIRED_PARAMS, which (via hasRequiredParamValue) rejects an empty string — but AWS allows an empty filterPattern (min length 0; an empty pattern matches all log events). The parameter must be PRESENT, but its value may be "". - Move `filterPattern` out of REQUIRED_PARAMS (which also rejects "") into a new REQUIRED_PRESENT_PARAMS check that only rejects a missing/null value, so "" is accepted while a truly-omitted pattern is still caught before execution. - logGroupName / filterName / metricTransformations stay non-empty-required. - Tests: empty filterPattern is accepted; a missing one is reported via the present-check. Low practical impact (our CIS metric-filter remediations always emit a non-empty pattern), but aligns the validator with AWS semantics. Executor spec green (43); typecheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../aws-command-executor.spec.ts | 22 +++++++++++++- .../cloud-security/aws-command-executor.ts | 30 +++++++++++++++---- 2 files changed, 45 insertions(+), 7 deletions(-) diff --git a/apps/api/src/cloud-security/aws-command-executor.spec.ts b/apps/api/src/cloud-security/aws-command-executor.spec.ts index 4b875fa53..ff998eac4 100644 --- a/apps/api/src/cloud-security/aws-command-executor.spec.ts +++ b/apps/api/src/cloud-security/aws-command-executor.spec.ts @@ -422,12 +422,32 @@ describe('PutMetricFilterCommand required params + normalization', () => { ]); expect(errors).toEqual( expect.arrayContaining([ - expect.stringMatching(/Required param "filterPattern" is missing/), + // filterPattern must be PRESENT (but may be empty), so a missing one is + // reported via the present-check, not the non-empty REQUIRED_PARAMS one. + expect.stringMatching(/Required param "filterPattern" must be provided/), expect.stringMatching(/Required param "metricTransformations" is missing/), ]), ); }); + it('allows an empty filterPattern (AWS accepts "" — it matches all events)', () => { + const errors = validatePlanSteps([ + step({ + service: 'cloudwatch-logs', + command: 'PutMetricFilterCommand', + params: { + logGroupName: 'lg', + filterName: 'fn', + filterPattern: '', + metricTransformations: [ + { metricName: 'm', metricNamespace: 'CloudTrailMetrics', metricValue: '1' }, + ], + }, + }), + ]); + expect(errors.filter((e) => /filterPattern/.test(e))).toHaveLength(0); + }); + it('does not error when all PutMetricFilter params are present', () => { const errors = validatePlanSteps([ step({ diff --git a/apps/api/src/cloud-security/aws-command-executor.ts b/apps/api/src/cloud-security/aws-command-executor.ts index ec786902c..713ea3e21 100644 --- a/apps/api/src/cloud-security/aws-command-executor.ts +++ b/apps/api/src/cloud-security/aws-command-executor.ts @@ -165,12 +165,18 @@ export const REQUIRED_PARAMS: Record = { StartConfigurationRecorderCommand: ['ConfigurationRecorderName'], PutBucketPolicyCommand: ['Bucket', 'Policy'], CreateTrailCommand: ['Name', 'S3BucketName'], - PutMetricFilterCommand: [ - 'logGroupName', - 'filterName', - 'filterPattern', - 'metricTransformations', - ], + PutMetricFilterCommand: ['logGroupName', 'filterName', 'metricTransformations'], +}; + +/** + * Params that must be PRESENT in the request but may legitimately be an empty + * string — unlike REQUIRED_PARAMS, which also rejects "". For example, AWS + * CloudWatch Logs PutMetricFilter requires `filterPattern` to be supplied but + * accepts an empty pattern (an empty filterPattern matches all log events), so + * we must reject only a missing/null value, not "". + */ +const REQUIRED_PRESENT_PARAMS: Record = { + PutMetricFilterCommand: ['filterPattern'], }; const REQUIRED_PARAM_ONE_OF: Record = { @@ -649,6 +655,18 @@ export function validatePlanSteps(steps: AwsCommandStep[]): string[] { } } + const requiredPresent = REQUIRED_PRESENT_PARAMS[step.command]; + if (requiredPresent) { + for (const key of requiredPresent) { + const value = step.params?.[key]; + // Must be supplied, but an empty string is valid (e.g. an empty + // CloudWatch filterPattern matches all log events). + if (value === undefined || value === null) { + errors.push(`${prefix}: Required param "${key}" must be provided`); + } + } + } + const oneOfGroups = REQUIRED_PARAM_ONE_OF[step.command]; if (oneOfGroups) { for (const group of oneOfGroups) { From e7af1ec0e4edc473cabffb5e9336e8fae87a47e5 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Fri, 5 Jun 2026 18:39:32 -0400 Subject: [PATCH 3/3] fix(cloud-security): reconcile conflicting logGroupName guidance in fix prompt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cubic follow-up on the metric-filter work: the prompt told the model two different things about the CloudWatch logGroupName — the new CLOUDWATCH METRIC FILTERS section said "use the REAL log group from the read step, never a placeholder", while the older NO-PLACEHOLDERS section said to use a made-up "CloudTrail/DefaultLogGroup" default. That inconsistency could produce varying PutMetricFilter plans. Nothing in code resolves the literal "CloudTrail/DefaultLogGroup" string (real values are filled by the refine pass from read-step output), so the real log group is what's needed. Align the older guidance to "discover the trail's CloudWatch Logs log group in a read step and use that exact name — do not invent one", matching the metric-filter section. Prompt-only change; typecheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- apps/api/src/cloud-security/ai-remediation.prompt.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/api/src/cloud-security/ai-remediation.prompt.ts b/apps/api/src/cloud-security/ai-remediation.prompt.ts index 3624a4698..9aa1ec9ce 100644 --- a/apps/api/src/cloud-security/ai-remediation.prompt.ts +++ b/apps/api/src/cloud-security/ai-remediation.prompt.ts @@ -290,7 +290,7 @@ NEVER omit AWSServiceName, leave it as null, or use a placeholder string. - NEVER use placeholder values like "{{variable}}", "", or template syntax - ALWAYS use concrete values in fix step params - If a value depends on the account (like a log group name), put the discovery in readSteps and use a reasonable default or convention in fixSteps: - - CloudTrail log group: use "CloudTrail/DefaultLogGroup" (the system will resolve the real one from readSteps) + - CloudTrail log group: discover the trail's CloudWatch Logs log group in a read step (e.g. from the trail's CloudWatchLogsLogGroupArn) and use that exact, real log group name in fixSteps — do not invent a name - SNS topic: use "CompAI-CIS-Alerts" (will be created if it doesn't exist) - KMS keys: use "alias/aws/service-name" for AWS-managed keys - The finding evidence contains REAL data from the AWS account scan — use those values