[comp] Production Deploy

apps/api/src/cloud-security/ai-remediation.prompt.ts

@@ -162,6 +162,15 @@ A human will ALWAYS review your plan before execution. Be precise and correct.
 - ALWAYS make changes reversible when possible
 - For service-linked roles: create them as a setup step using IAM CreateServiceLinkedRoleCommand
 
+## S3 PUBLIC ACCESS AND ACLs (IMPORTANT)
+- NEVER use PutBucketAclCommand or bucket/object ACLs. Modern buckets use Object Ownership = BucketOwnerEnforced, which disables ACLs — the call fails, and the executor strips ACL steps, which can leave an EMPTY plan.
+- To block public access on a bucket: use PutPublicAccessBlockCommand (service "s3") with PublicAccessBlockConfiguration set to { BlockPublicAcls: true, IgnorePublicAcls: true, BlockPublicPolicy: true, RestrictPublicBuckets: true }.
+- To remediate a public bucket POLICY: read it first with GetBucketPolicyCommand, then use PutBucketPolicyCommand with a corrected least-privilege policy (service "s3"). Never rely on ACLs to fix public access.
+
+## AWS CONFIG RECORDER (IMPORTANT)
+- 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).
+
 ## 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
@@ -259,7 +268,7 @@ NEVER omit AWSServiceName, leave it as null, or use a placeholder string.
 
 ## REQUIRED PERMISSIONS (VERY IMPORTANT — GET THIS RIGHT FIRST TIME)
 - List EVERY IAM action needed for the COMPLETE operation, not just the direct API calls
-- Think through the FULL chain: if you CreateBucket, you also need PutBucketPolicy, GetBucketPolicy, PutBucketAcl
+- Think through the FULL chain: if you CreateBucket, you also need PutBucketPolicy, GetBucketPolicy, PutPublicAccessBlock (do NOT use PutBucketAcl — ACLs are disabled on modern buckets)
 - Include iam:CreateRole and iam:PutRolePolicy when creating AWS service delivery roles
 - Include iam:PassRole when attaching a role to an AWS service (CloudTrail, Config, etc.)
 - NEVER include iam:AttachRolePolicy — use iam:PutRolePolicy (inline policies) instead

apps/api/src/cloud-security/ai-remediation.service.spec.ts

@@ -142,9 +142,12 @@ describe('AiRemediationService.generateFixPlan empty-state backstop', () => {
 
   it('leaves the plan untouched when AI returns {}/{} but the plan has no actionable steps', async () => {
     // Verify-only plans (only readSteps) should still be left alone —
-    // we never fabricate state when there's nothing to act on.
+    // we never fabricate state when there's nothing to act on. A plan with no
+    // fix steps is not auto-fixable, so canAutoFix is false (which also means
+    // the empty-plan retry does not apply to it).
     generateObjectMock.mockResolvedValueOnce({
       object: basePlan({
+        canAutoFix: false,
         readSteps: [
           { service: 's3', command: 'GetBucketVersioningCommand', params: {}, purpose: 'check' },
         ],
@@ -172,6 +175,17 @@ describe('AiRemediationService.generateFixPlan empty-state backstop', () => {
       object: basePlan({
         currentState: { versioning: 'Disabled' },
         proposedState: { versioning: 'Enabled' },
+        fixSteps: [
+          {
+            service: 's3',
+            command: 'PutBucketVersioningCommand',
+            params: {
+              Bucket: 'logs-archive',
+              VersioningConfiguration: { Status: 'Enabled' },
+            },
+            purpose: 'enable versioning',
+          },
+        ],
       }),
     });
 
@@ -226,8 +240,11 @@ describe('AiRemediationService.generateFixPlan empty-state backstop', () => {
   });
 
   it('leaves a plan alone when only one side is empty (legitimate verify-only case)', async () => {
+    // Verify-only: no fix steps, so the plan is not auto-fixable (canAutoFix
+    // false) and the empty-plan retry does not apply.
     generateObjectMock.mockResolvedValueOnce({
       object: basePlan({
+        canAutoFix: false,
         currentState: { someField: 'X' },
         proposedState: {},
       }),
@@ -508,3 +525,197 @@ describe('AiRemediationService.generateManualSteps', () => {
     expect(callArgs.prompt).toContain('account-level');
   });
 });
+
+describe('AiRemediationService.generateFixPlan empty-plan retry', () => {
+  const generateObjectMock = generateObject as unknown as jest.Mock;
+
+  beforeEach(() => {
+    generateObjectMock.mockReset();
+  });
+
+  it('retries once when the first plan has canAutoFix=true but zero fixSteps, and uses the non-empty retry', async () => {
+    // First pass: empty fix plan (the "AI generated an empty fix plan" case).
+    generateObjectMock.mockResolvedValueOnce({
+      object: basePlan({ canAutoFix: true, fixSteps: [] }),
+    });
+    // Second pass (higher temperature): a real plan.
+    generateObjectMock.mockResolvedValueOnce({
+      object: basePlan({
+        canAutoFix: true,
+        fixSteps: [
+          {
+            service: 'config-service',
+            command: 'PutConfigurationRecorderCommand',
+            params: { ConfigurationRecorder: { name: 'default' } },
+            purpose: 'Record all resources',
+          },
+        ],
+      }),
+    });
+
+    const service = new AiRemediationService();
+    const plan = await service.generateFixPlan({
+      title: 'AWS Config recorder not fully active',
+      description: null,
+      severity: 'high',
+      resourceType: 'AwsConfigRecorder',
+      resourceId: 'default',
+      remediation: null,
+      findingKey: 'config-recorder-incomplete',
+      evidence: {},
+    });
+
+    expect(generateObjectMock).toHaveBeenCalledTimes(2);
+    // The retry runs at a non-zero temperature so it is a genuinely different sample.
+    expect(generateObjectMock.mock.calls[0][0].temperature).toBe(0);
+    expect(generateObjectMock.mock.calls[1][0].temperature).toBeGreaterThan(0);
+    expect(plan.fixSteps).toHaveLength(1);
+    expect(plan.fixSteps[0].command).toBe('PutConfigurationRecorderCommand');
+  });
+
+  it('does not retry when the first plan already has fix steps', async () => {
+    generateObjectMock.mockResolvedValueOnce({
+      object: basePlan({
+        canAutoFix: true,
+        fixSteps: [
+          {
+            service: 'iam',
+            command: 'UpdateAccountPasswordPolicyCommand',
+            params: {},
+            purpose: 'fix',
+          },
+        ],
+      }),
+    });
+
+    const service = new AiRemediationService();
+    await service.generateFixPlan({
+      title: 'Weak password policy',
+      description: null,
+      severity: null,
+      resourceType: 'AwsIamPolicy',
+      resourceId: 'account-level',
+      remediation: null,
+      findingKey: 'iam-weak-password',
+      evidence: {},
+    });
+
+    expect(generateObjectMock).toHaveBeenCalledTimes(1);
+  });
+});
+
+describe('AiRemediationService GCP/Azure empty-plan retry', () => {
+  const generateObjectMock = generateObject as unknown as jest.Mock;
+
+  beforeEach(() => {
+    generateObjectMock.mockReset();
+  });
+
+  const finding = {
+    title: 'finding',
+    description: null,
+    severity: 'high',
+    resourceType: 'CloudResource',
+    resourceId: 'r',
+    remediation: null,
+    findingKey: 'fk',
+    evidence: {},
+  };
+
+  it('GCP: retries once at higher temperature when the first plan is empty', async () => {
+    generateObjectMock.mockResolvedValueOnce({
+      object: { canAutoFix: true, fixSteps: [] },
+    });
+    generateObjectMock.mockResolvedValueOnce({
+      object: { canAutoFix: true, fixSteps: [{ method: 'PATCH' }] },
+    });
+
+    const service = new AiRemediationService();
+    const plan = await service.generateGcpFixPlan(finding);
+
+    expect(generateObjectMock).toHaveBeenCalledTimes(2);
+    expect(generateObjectMock.mock.calls[0][0].temperature).toBe(0);
+    expect(generateObjectMock.mock.calls[1][0].temperature).toBeGreaterThan(0);
+    expect(plan.fixSteps).toHaveLength(1);
+  });
+
+  it('GCP: does not retry when the first plan already has steps', async () => {
+    generateObjectMock.mockResolvedValueOnce({
+      object: { canAutoFix: true, fixSteps: [{ method: 'PATCH' }] },
+    });
+
+    const service = new AiRemediationService();
+    await service.generateGcpFixPlan(finding);
+
+    expect(generateObjectMock).toHaveBeenCalledTimes(1);
+  });
+
+  it('Azure: retries once at higher temperature when the first plan is empty', async () => {
+    generateObjectMock.mockResolvedValueOnce({
+      object: { canAutoFix: true, fixSteps: [] },
+    });
+    generateObjectMock.mockResolvedValueOnce({
+      object: { canAutoFix: true, fixSteps: [{ method: 'PATCH' }] },
+    });
+
+    const service = new AiRemediationService();
+    const plan = await service.generateAzureFixPlan(finding);
+
+    expect(generateObjectMock).toHaveBeenCalledTimes(2);
+    expect(generateObjectMock.mock.calls[0][0].temperature).toBe(0);
+    expect(generateObjectMock.mock.calls[1][0].temperature).toBeGreaterThan(0);
+    expect(plan.fixSteps).toHaveLength(1);
+  });
+
+  it('Azure: does not retry when the first plan already has steps', async () => {
+    generateObjectMock.mockResolvedValueOnce({
+      object: { canAutoFix: true, fixSteps: [{ method: 'PATCH' }] },
+    });
+
+    const service = new AiRemediationService();
+    await service.generateAzureFixPlan(finding);
+
+    expect(generateObjectMock).toHaveBeenCalledTimes(1);
+  });
+});
+
+describe('AiRemediationService.generateFixPlan retry selection', () => {
+  const generateObjectMock = generateObject as unknown as jest.Mock;
+
+  beforeEach(() => {
+    generateObjectMock.mockReset();
+  });
+
+  it('prefers a canAutoFix=false retry over the original empty canAutoFix=true plan', async () => {
+    // First pass: the degenerate empty plan (canAutoFix true, no steps).
+    generateObjectMock.mockResolvedValueOnce({
+      object: basePlan({ canAutoFix: true, fixSteps: [] }),
+    });
+    // Retry: the model correctly concludes the finding is not auto-fixable.
+    generateObjectMock.mockResolvedValueOnce({
+      object: basePlan({
+        canAutoFix: false,
+        fixSteps: [],
+        reason: 'Requires manual setup',
+        guidedSteps: ['Do the thing in the console'],
+      }),
+    });
+
+    const service = new AiRemediationService();
+    const plan = await service.generateFixPlan({
+      title: 't',
+      description: null,
+      severity: null,
+      resourceType: 'X',
+      resourceId: 'y',
+      remediation: null,
+      findingKey: 'fk',
+      evidence: {},
+    });
+
+    expect(generateObjectMock).toHaveBeenCalledTimes(2);
+    // The non-auto-fixable retry is used → routes to guided steps instead of
+    // the "AI generated an empty fix plan" dead end.
+    expect(plan.canAutoFix).toBe(false);
+  });
+});