Merge pull request #3109 from trycompai/main

[comp] Production Deploy

Author: tofikwest

apps/api/src/cloud-security/providers/azure-security.service.spec.ts

@@ -0,0 +1,72 @@
+// AzureSecurityService scans Defender + a suite of ARM service adapters, all of
+// which go through the global `fetch`. These tests mock `fetch` to exercise the
+// all-units-failed guard: a scan where every unit errors must THROW (so it is
+// not stored as a fresh empty "success" run that hides the prior good results),
+// while a genuinely-clean subscription (everything succeeds, 0 findings) must
+// return [] without throwing.
+jest.mock('@db', () => ({ db: {} }));
+
+import { AzureSecurityService } from './azure-security.service';
+
+function azureOkEmpty(): { ok: true; json: () => Promise<unknown> } {
+  return { ok: true, json: async () => ({ value: [] }) };
+}
+
+function azureError(
+  status: number,
+  text = 'error',
+): { ok: false; status: number; text: () => Promise<string> } {
+  return { ok: false, status, text: async () => text };
+}
+
+describe('AzureSecurityService.scanSecurityFindings — all-units-failed guard', () => {
+  let service: AzureSecurityService;
+  let fetchMock: jest.Mock;
+  const originalFetch = global.fetch;
+
+  const creds = { access_token: 'tok' };
+  const vars = { subscription_id: 'sub-1' };
+
+  beforeEach(() => {
+    fetchMock = jest.fn();
+    // @ts-expect-error replacing global fetch with a mock for these tests
+    global.fetch = fetchMock;
+    service = new AzureSecurityService();
+  });
+
+  afterAll(() => {
+    global.fetch = originalFetch;
+  });
+
+  // The bug: a total (non-403) failure used to return [] silently, which got
+  // stored as a fresh "success" run with 0 findings and hid the prior results.
+  it('throws when the whole scan errors out and produces no findings, instead of returning []', async () => {
+    fetchMock.mockResolvedValue(azureError(500, 'internal server error'));
+
+    await expect(service.scanSecurityFindings(creds, vars)).rejects.toThrow();
+  });
+
+  it('does NOT throw on a healthy subscription — adapters emit passing findings', async () => {
+    fetchMock.mockResolvedValue(azureOkEmpty());
+
+    const findings = await service.scanSecurityFindings(creds, vars);
+    expect(findings.length).toBeGreaterThan(0);
+  });
+
+  it('does NOT throw when results are produced even though one adapter fails', async () => {
+    fetchMock.mockImplementation(async (url: string) => {
+      // Storage adapter fails; everything else succeeds (and emits findings).
+      if (url.includes('Microsoft.Storage')) return azureError(500, 'boom');
+      return azureOkEmpty();
+    });
+
+    const findings = await service.scanSecurityFindings(creds, vars);
+    expect(findings.length).toBeGreaterThan(0);
+  });
+
+  it('still enforces required inputs (missing subscription throws AZURE_SUB_MISSING)', async () => {
+    await expect(service.scanSecurityFindings(creds, {})).rejects.toThrow(
+      /AZURE_SUB_MISSING/,
+    );
+  });
+});

apps/api/src/cloud-security/providers/azure-security.service.ts

@@ -106,41 +106,82 @@ export class AzureSecurityService {
 
     this.logger.log(`Scanning Azure subscription ${subscriptionId}`);
     const findings: SecurityFinding[] = [];
+    // Track unit failures so a scan that comes back EMPTY because every unit
+    // (Defender + each adapter) errored fails loudly instead of being stored as
+    // a fresh "success" run with 0 findings — which would hide the previous good
+    // results (the UI shows only the latest run). A healthy subscription still
+    // emits passing findings, so "empty result + a failure" is the real tell.
+    let failedUnits = 0;
+    let firstError: Error | null = null;
 
     // 1. Defender alerts + assessments (always runs)
     if (!enabledServices || enabledServices.includes('defender')) {
-      const defenderFindings = await this.scanDefender(token, subscriptionId);
-      findings.push(...defenderFindings);
+      const defender = await this.scanDefender(token, subscriptionId);
+      findings.push(...defender.findings);
+      if (!defender.anySucceeded) {
+        failedUnits++;
+        if (defender.firstError && !firstError) firstError = defender.firstError;
+      }
     }
 
     // 2. Run service adapters in parallel
-    const adapterPromises = SERVICE_ADAPTERS.filter(
+    const activeAdapters = SERVICE_ADAPTERS.filter(
       (a) => !enabledServices || enabledServices.includes(a.serviceId),
-    ).map(async (adapter) => {
-      try {
-        return await adapter.scan({ accessToken: token, subscriptionId });
-      } catch (error) {
-        const msg = error instanceof Error ? error.message : String(error);
-        this.logger.warn(`Azure ${adapter.serviceId} scan failed: ${msg}`);
-        return [];
+    );
+    const adapterResults = await Promise.all(
+      activeAdapters.map(async (adapter) => {
+        try {
+          const scanned = await adapter.scan({
+            accessToken: token,
+            subscriptionId,
+          });
+          return { ok: true as const, findings: scanned };
+        } catch (error) {
+          const err = error instanceof Error ? error : new Error(String(error));
+          this.logger.warn(`Azure ${adapter.serviceId} scan failed: ${err.message}`);
+          return { ok: false as const, error: err };
+        }
+      }),
+    );
+    for (const result of adapterResults) {
+      if (result.ok) {
+        findings.push(...result.findings);
+      } else {
+        failedUnits++;
+        if (!firstError) firstError = result.error;
       }
-    });
+    }
 
-    const adapterResults = await Promise.all(adapterPromises);
-    for (const result of adapterResults) {
-      findings.push(...result);
+    // An empty result combined with a failed unit is a degenerate/broken scan,
+    // not a genuinely-clean subscription (a clean scan still emits passing
+    // findings). Throw so nothing is stored and the prior good run stays
+    // visible. Mirrors the AWS all-regions-failed and GCP all-scopes-failed
+    // guards. A truly-empty subscription (every unit succeeded, no failures)
+    // still returns [] normally.
+    if (findings.length === 0 && failedUnits > 0) {
+      throw firstError ?? new Error('All Azure service scans failed');
     }
 
     this.logger.log(`Azure scan complete: ${findings.length} total findings`);
     return findings;
   }
 
-  /** Scan Defender for Cloud alerts and assessments. */
+  /**
+   * Scan Defender for Cloud alerts and assessments. Reports whether at least
+   * one query succeeded so the caller can tell a genuinely-clean subscription
+   * (anySucceeded=true, no findings) apart from a total failure.
+   */
   private async scanDefender(
     accessToken: string,
     subscriptionId: string,
-  ): Promise<SecurityFinding[]> {
+  ): Promise<{
+    findings: SecurityFinding[];
+    anySucceeded: boolean;
+    firstError: Error | null;
+  }> {
     const findings: SecurityFinding[] = [];
+    let anySucceeded = false;
+    let firstError: Error | null = null;
 
     // Alerts
     try {
@@ -173,13 +214,17 @@ export class AzureSecurityService {
           createdAt: alert.properties.startTimeUtc || new Date().toISOString(),
         });
       }
+      anySucceeded = true;
     } catch (error) {
       this.handlePermissionError(
         findings,
         error,
         'Security Alerts',
         subscriptionId,
       );
+      if (!firstError) {
+        firstError = error instanceof Error ? error : new Error(String(error));
+      }
     }
 
     // Assessments
@@ -227,16 +272,20 @@ export class AzureSecurityService {
           createdAt: new Date().toISOString(),
         });
       }
+      anySucceeded = true;
     } catch (error) {
       this.handlePermissionError(
         findings,
         error,
         'Security Assessments',
         subscriptionId,
       );
+      if (!firstError) {
+        firstError = error instanceof Error ? error : new Error(String(error));
+      }
     }
 
-    return findings;
+    return { findings, anySucceeded, firstError };
   }
 
   private handlePermissionError(

apps/api/src/cloud-security/providers/gcp-security.service.spec.ts

@@ -698,3 +698,128 @@ describe('resolveGcpServiceId — Vertex AI grouping', () => {
     );
   });
 });
+
+// ── SCC findings response helpers ───────────────────────────────────────────
+function sccPage(
+  findings: Array<Record<string, unknown>>,
+  nextPageToken?: string,
+): { ok: true; json: () => Promise<unknown> } {
+  return {
+    ok: true,
+    json: async () => ({
+      listFindingsResults: findings,
+      ...(nextPageToken ? { nextPageToken } : {}),
+    }),
+  };
+}
+
+function sccError(
+  status: number,
+  text: string,
+): { ok: false; status: number; text: () => Promise<string> } {
+  return { ok: false, status, text: async () => text };
+}
+
+function sccFinding(project: string): Record<string, unknown> {
+  return {
+    finding: {
+      name: `organizations/1/sources/-/findings/${project}-1`,
+      category: 'PUBLIC_BUCKET_ACL',
+      description: 'desc',
+      severity: 'HIGH',
+      state: 'ACTIVE',
+      resourceName: `//storage.googleapis.com/projects/${project}/buckets/b`,
+      eventTime: '2026-01-01T00:00:00Z',
+      createTime: '2026-01-01T00:00:00Z',
+    },
+    resource: {
+      type: 'storage.googleapis.com/Bucket',
+      projectDisplayName: project,
+    },
+  };
+}
+
+describe('GCPSecurityService.scanSecurityFindings — all-scopes-failed guard', () => {
+  let service: GCPSecurityService;
+  let fetchMock: jest.Mock;
+  const originalFetch = global.fetch;
+
+  beforeEach(() => {
+    fetchMock = jest.fn();
+    // @ts-expect-error replacing global fetch with a mock for these tests
+    global.fetch = fetchMock;
+    service = new GCPSecurityService();
+  });
+
+  afterAll(() => {
+    global.fetch = originalFetch;
+  });
+
+  // The bug: when the only scope errored, the scan used to swallow the error and
+  // return [] — which got stored as a fresh "success" run with 0 findings,
+  // hiding the previous good results. It must now throw so nothing is stored.
+  it('throws (re-throwing the actionable SCC error) when the only scope errors, instead of returning []', async () => {
+    fetchMock.mockResolvedValue(
+      sccError(403, 'PERMISSION_DENIED: caller lacks securitycenter.findings.list'),
+    );
+
+    await expect(
+      service.scanSecurityFindings(
+        { access_token: 'tok' },
+        { project_ids: ['wasimil-prod'] },
+      ),
+    ).rejects.toThrow(/Permission denied/);
+  });
+
+  // The exact case from the customer report: project-scoped SCC query on a
+  // no-org account returns 404 NOT_FOUND (SCC never activated for the project).
+  it('maps a 404 NOT_FOUND to the actionable SCC_NOT_ACTIVATED error', async () => {
+    fetchMock.mockResolvedValue(
+      sccError(404, '{"error":{"code":404,"message":"Requested entity was not found.","status":"NOT_FOUND"}}'),
+    );
+
+    await expect(
+      service.scanSecurityFindings(
+        { access_token: 'tok' },
+        { project_ids: ['wasimil-prod'] },
+      ),
+    ).rejects.toThrow(/SCC_NOT_ACTIVATED/);
+  });
+
+  it('throws when EVERY configured scope errors', async () => {
+    fetchMock.mockResolvedValue(sccError(500, 'internal error'));
+
+    await expect(
+      service.scanSecurityFindings(
+        { access_token: 'tok' },
+        { project_ids: ['p1', 'p2'] },
+      ),
+    ).rejects.toThrow();
+  });
+
+  it('does NOT throw when at least one scope succeeds — returns the surviving scope’s findings', async () => {
+    fetchMock.mockImplementation(async (url: string) => {
+      if (url.includes('projects/p1')) return sccError(403, 'PERMISSION_DENIED');
+      return sccPage([sccFinding('p2')]);
+    });
+
+    const findings = await service.scanSecurityFindings(
+      { access_token: 'tok' },
+      { project_ids: ['p1', 'p2'] },
+    );
+
+    expect(findings).toHaveLength(1);
+    expect(findings[0].resourceId).toContain('projects/p2');
+  });
+
+  it('does NOT throw when a scope succeeds with zero findings (genuinely clean account)', async () => {
+    fetchMock.mockResolvedValue(sccPage([]));
+
+    const findings = await service.scanSecurityFindings(
+      { access_token: 'tok' },
+      { project_ids: ['wasimil-prod'] },
+    );
+
+    expect(findings).toEqual([]);
+  });
+});