diff --git a/apps/api/src/cloud-security/ai-remediation.prompt.ts b/apps/api/src/cloud-security/ai-remediation.prompt.ts index 03dbd0366..9aa1ec9ce 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 @@ -284,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 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..ff998eac4 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,111 @@ 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([ + // 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({ + 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..713ea3e21 100644 --- a/apps/api/src/cloud-security/aws-command-executor.ts +++ b/apps/api/src/cloud-security/aws-command-executor.ts @@ -165,6 +165,18 @@ export const REQUIRED_PARAMS: Record = { StartConfigurationRecorderCommand: ['ConfigurationRecorderName'], PutBucketPolicyCommand: ['Bucket', 'Policy'], CreateTrailCommand: ['Name', 'S3BucketName'], + 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 = { @@ -276,6 +288,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( @@ -599,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) {