From 33042e79918196f8c4e585844649622329e6f041 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Fri, 15 May 2026 18:49:39 -0400 Subject: [PATCH] fix(cloud-tests): address cubic findings on production deploy PR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four real issues caught by cubic on the main→release deploy review. The two Jest-hoisting findings (exception/reconciliation specs) were skipped — that rule is babel-jest only, project uses ts-jest, all tests pass in CI. P1 — "Fix All" could target passing findings Service groups store the merged failed+passed set, and `canFixFinding` returns a key for any finding with a `findingKey` regardless of status. The batch dialog was happily including already-passing checks in the remediation target list. Now filter by `status === 'failed'` before consulting canFixFinding. P2 — exception-expiry accepted timezone-less timestamps ISO 8601 regex made the timezone offset optional, so `2026-08-13T23:59:59` passed validation but `new Date()` parsed it in server-local time — same input, different expiry on UTC vs Pacific hosts. Made the offset required; updated the spec to assert both acceptance (with offset) and rejection (without offset). P2 — dead `!prior.passed === false` in reconciliation The line evaluated identically to the very next `if (prior.passed) continue`. Removed; behavior unchanged, clarity restored. P2 — ISO control-number regex was case-sensitive `/\bA\.\d+\.\d+(\.\d+)?\b/` had no /i flag, so lowercase variants like "a.5.1.2" would slip past the forbidden-content guard. Added /i and a regression test. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../ai-description.prompt.spec.ts | 17 +++++++++++++++++ .../cloud-security/ai-description.prompt.ts | 5 +++-- .../exception-expiry.utils.spec.ts | 18 +++++++++++++++++- .../cloud-security/exception-expiry.utils.ts | 8 +++++--- .../cloud-security/reconciliation.service.ts | 3 +-- .../components/CloudTestsSection.tsx | 5 +++++ 6 files changed, 48 insertions(+), 8 deletions(-) diff --git a/apps/api/src/cloud-security/ai-description.prompt.spec.ts b/apps/api/src/cloud-security/ai-description.prompt.spec.ts index c5872ec48..9684acdac 100644 --- a/apps/api/src/cloud-security/ai-description.prompt.spec.ts +++ b/apps/api/src/cloud-security/ai-description.prompt.spec.ts @@ -69,6 +69,23 @@ describe('ai-description.prompt', () => { ).toMatchObject({ field: 'whyItMatters' }); }); + it('flags ISO 27001 control numbers in lowercase variants (a.5.1)', () => { + // The regex must be case-insensitive — auditors won't accept the + // model getting around the gate by lowercasing a citation. + expect( + findForbiddenContent({ + ...baseline, + whyItMatters: 'Maps to a.9.4.3.', + }), + ).toMatchObject({ field: 'whyItMatters' }); + expect( + findForbiddenContent({ + ...baseline, + description: 'Control a.5.1.2 enforces this.', + }), + ).toMatchObject({ field: 'description' }); + }); + it('flags HIPAA / NIST framework citations', () => { expect( findForbiddenContent({ diff --git a/apps/api/src/cloud-security/ai-description.prompt.ts b/apps/api/src/cloud-security/ai-description.prompt.ts index d07d42408..0e5424d75 100644 --- a/apps/api/src/cloud-security/ai-description.prompt.ts +++ b/apps/api/src/cloud-security/ai-description.prompt.ts @@ -104,9 +104,10 @@ const FORBIDDEN_PATTERNS: readonly RegExp[] = [ // prefixes — catches "CIS 1.8", "PCI 8.2.3", "NIST AC-2", // "HIPAA 164.312" even without the full framework name re-mentioned. /\b(CIS|PCI|NIST|HIPAA|HITRUST|FedRAMP) ?[A-Z]*[- ]?\d+(\.\d+){0,3}\b/i, - // SOC 2 / ISO control-number formats + // SOC 2 / ISO control-number formats — case-insensitive so lowercase + // variants (e.g. "a.5.1.2") are blocked too. /\bCC\d+\.\d+\b/i, - /\bA\.\d+\.\d+(\.\d+)?\b/, + /\bA\.\d+\.\d+(\.\d+)?\b/i, // URLs /https?:\/\//i, /www\./i, diff --git a/apps/api/src/cloud-security/exception-expiry.utils.spec.ts b/apps/api/src/cloud-security/exception-expiry.utils.spec.ts index 0de020432..5d3bdfaa0 100644 --- a/apps/api/src/cloud-security/exception-expiry.utils.spec.ts +++ b/apps/api/src/cloud-security/exception-expiry.utils.spec.ts @@ -75,9 +75,25 @@ describe('parseExceptionExpiry', () => { ); }); - it('accepts strict ISO 8601 timestamps via the fallback', () => { + it('accepts strict ISO 8601 timestamps with an explicit timezone offset', () => { expect(parseExceptionExpiry('2026-08-13T23:59:59Z')).not.toBeNull(); expect(parseExceptionExpiry('2026-08-13T23:59:59.999Z')).not.toBeNull(); expect(parseExceptionExpiry('2026-08-13T23:59:59+02:00')).not.toBeNull(); + expect(parseExceptionExpiry('2026-08-13T23:59:59-07:00')).not.toBeNull(); + }); + + it('rejects timestamps without an explicit timezone offset', () => { + // No `Z`, no `+02:00` etc. — `new Date()` would parse these in server + // local time, giving inconsistent expiries across environments. Force + // the caller to commit to a timezone explicitly. + expect(() => parseExceptionExpiry('2026-08-13T23:59:59')).toThrow( + BadRequestException, + ); + expect(() => parseExceptionExpiry('2026-08-13T23:59')).toThrow( + BadRequestException, + ); + expect(() => + parseExceptionExpiry('2026-08-13T23:59:59.999'), + ).toThrow(BadRequestException); }); }); diff --git a/apps/api/src/cloud-security/exception-expiry.utils.ts b/apps/api/src/cloud-security/exception-expiry.utils.ts index d3d9334db..52e894592 100644 --- a/apps/api/src/cloud-security/exception-expiry.utils.ts +++ b/apps/api/src/cloud-security/exception-expiry.utils.ts @@ -45,12 +45,14 @@ export function parseExceptionExpiry( // Reject anything that isn't strict ISO 8601 — `new Date()` happily parses // locale-specific strings like "January 1, 2026" and "2026/08/13", which - // would silently bypass the documented contract. + // would silently bypass the documented contract. The timezone offset is + // REQUIRED — without it, `new Date("2026-08-13T23:59:59")` is parsed in + // server-local time, giving different expiries on UTC vs Pacific hosts. const ISO_8601 = - /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}(:\d{2})?(\.\d+)?(Z|[+-]\d{2}:?\d{2})?$/; + /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}(:\d{2})?(\.\d+)?(Z|[+-]\d{2}:?\d{2})$/; if (!ISO_8601.test(input)) { throw new BadRequestException( - 'expiresAt must be a valid ISO date or YYYY-MM-DD calendar date.', + 'expiresAt must be a YYYY-MM-DD calendar date or an ISO 8601 timestamp with an explicit timezone offset (e.g. "2026-08-13T23:59:59Z").', ); } const parsed = new Date(input); diff --git a/apps/api/src/cloud-security/reconciliation.service.ts b/apps/api/src/cloud-security/reconciliation.service.ts index 342ef8653..70c0e8370 100644 --- a/apps/api/src/cloud-security/reconciliation.service.ts +++ b/apps/api/src/cloud-security/reconciliation.service.ts @@ -124,8 +124,7 @@ export class CloudReconciliationService { // Resolutions: prior failed → current absent or passed. for (const [key, prior] of priorMap.entries()) { - if (!prior.passed === false) continue; // only interested in prior failures - if (prior.passed) continue; + if (prior.passed) continue; // only interested in prior failures const current = currentMap.get(key); if (current && !current.passed) continue; // still failing diff --git a/apps/app/src/app/(app)/[orgId]/cloud-tests/components/CloudTestsSection.tsx b/apps/app/src/app/(app)/[orgId]/cloud-tests/components/CloudTestsSection.tsx index 24af1346a..294edc8c1 100644 --- a/apps/app/src/app/(app)/[orgId]/cloud-tests/components/CloudTestsSection.tsx +++ b/apps/app/src/app/(app)/[orgId]/cloud-tests/components/CloudTestsSection.tsx @@ -455,7 +455,12 @@ export function CloudTestsSection({ if (!batchServiceId) return null; const group = serviceGroups.find((g) => g.serviceId === batchServiceId); if (!group) return null; + // `group.findings` is the merged failed+passed set — restrict the batch + // to failing findings only. `canFixFinding` doesn't gate on status, so + // without this filter a passing check could be targeted by "Fix All". const fixable = group.findings.filter((f) => { + const isFailed = f.status === 'failed' || f.status === 'FAILED'; + if (!isFailed) return false; const match = canFixFinding(f); return match?.key && match.enabled; });