feat(cloud-tests): auditor visibility improvements (phases 1-5)

[github-actions]

Summary

• Surfaces evidence JSON, run metadata, resource labels, and a History tab so auditors can see what was checked, what data led to pass/fail, who fixed what, and what is intentionally accepted (with reason) — without leaving the platform.
• Adds AI-generated "About this check" panels (Haiku 4.5, lazy, cached per-org, sanitized) and per-check sub-grouping with "Show all results" toggles inside service groups.
• Enables auditors (who hold finding:update but not task:update) to comment on findings via a dynamic-permission guard on the comments controller.

Phase-by-phase

Phase 1 — Quick wins

• evidence-sanitizer.ts redacts sensitive keys (suffix-match strategy preserves requirePassword: true-style config booleans)
• Last-scan metadata strip: Ran 5m ago • 47 checks • 41 passed • 6 failed • Duration 3.2s
• Resource labels on each row (IAM User: john, S3 Bucket: logs-archive)
• Rename "Security Findings" → "Scan Results" (header + tab)

Phase 2 — AI check descriptions

• Tier 3 content: title / description / passCriteria / failCriteria / whyItMatters
• Claude Haiku 4.5 via @ai-sdk/anthropic. Cached per (orgId, checkId) in new CheckDefinition table with source-hash invalidation.
• Server-side strip of forbidden patterns (SOC 2 control numbers, ISO refs, URLs, framework citations) as backstop to prompt instructions.
• GCP/Azure don't use AI — synchronous passthrough from SCC / Defender catalog metadata.

Phase 3 — Per-check sub-grouping

• Findings within a service group now nested by normalized checkKey (iam-no-mfa-john → iam-no-mfa).
• Each sub-group shows X of Y failing. "Show all results" reveals passing rows.
• 100-row display cap per check with "Show more" affordance (skipped a virtualization lib dep to keep scope tight).

Phase 4 — Comments on auditor findings + cosmetic sweep

• Added finding to CommentEntityType enum.
• New CommentsPermissionGuard resolves entity-type-specific permission dynamically — finding:update for finding comments, task:update for task comments, etc. Auditors now unblocked.
• Comments thread rendered on /overview/findings detail sheet.
• Cosmetic: 4 remaining "findings" → "results" strings inside cloud-tests scope.

Phase 5 — Resolution + exceptions history

• New tables: FindingException, FindingResolution, FindingRegression; FindingResolutionMethod enum.
• scannedServices + failedServices columns on IntegrationCheckRun prevent false "resolved" events when a service wasn't scanned (rate limit / partial-scan protection).
• Reconciliation runs after every scan, classifies each resolution as one of:
  • platform_fix — matched a successful RemediationAction between scans
  • external_fix — customer fixed it in the provider console
  • resource_deleted — resource no longer exists in current scan
  • exception_marked — user marked it as an exception
• Regressions detected when a previously-resolved (checkKey, resourceId) fails again.
• Mark as exception modal: required reason (20+ chars), optional reviewer (free text), optional auto-review date.
• New History tab between Scan Results and Activity: summary card + Resolutions + Active Exceptions + Regressions.

Architecture notes

• getFindings now filters out findings under active (non-revoked, non-expired) exceptions by default; includeExceptions=true opt-out for the History tab.
• Comment.entityId is already polymorphic — only the enum and permission guard needed extending.
• Reconciliation is fire-and-isolated from the scan path: failures log + continue so audit-trail bugs never break a customer's scan.
• Reconciliation is idempotent per currentRunId.

DB migrations (3)

• 20260513213400_cloud_tests_check_definition — Phase 2
• 20260513220233_comment_entity_type_finding — Phase 4
• 20260513221148_cloud_tests_phase5_history — Phase 5

Test plan

• 96 new tests passing (59 api Jest + 37 app Vitest):
  evidence-sanitizer (15) · ai-description.prompt (10) · check-definition.service (7) · comments-permission.guard (8) · exception.service (10) · reconciliation.service (9) · check-groups (11) · CheckGroupBlock (7) · CheckDefinitionPanel (5) · EvidenceJsonViewer (5) · MarkExceptionModal (4) · HistoryTab (5)
• npx tsc --noEmit clean for every file touched in api + app
• Manual browser test against a real AWS connection — not run from this environment
• Sample 20 real evidence payloads to refine sanitizer wordlist before broad rollout
• (Recommended) talk to 1-2 more auditors beyond Chris before celebrating Phase 2 content quality

Known limitations / follow-ups

• Resource renaming (e.g., AWS IAM user rename) shows as resource_deleted + new failure pair. Underlying ARN tracking would fix this; deferred.
• Comments on PUT/DELETE still fall back to task:update (commentId is opaque without an extra DB lookup). Author-only editing means the practical impact is small.
• GCP/Azure scan code doesn't currently capture per-service success/failure into scannedServices — reconciliation falls back to "treat all as scanned" when the array is empty. Once GCP/Azure scans start populating it, partial-scan protection extends to those providers automatically.
• No audit-design-system skill run on touched frontend files — same @trycompai/ui + lucide-react patterns as existing code; full DS migration is separate scope.

🤖 Generated with Claude Code

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
app	Ready	Preview, Comment	May 13, 2026 11:27pm
comp-framework-editor	Ready	Preview, Comment	May 13, 2026 11:27pm

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
portal	Skipped		May 13, 2026 11:27pm

[cubic-dev-ai]

11 issues found across 52 files

Confidence score: 2/5

• Merge risk is high because apps/api/src/comments/comments-permission.guard.ts allows API key/service-token requests without permission-scope checks, which is a concrete authorization bypass on comments endpoints.
• apps/api/src/cloud-security/evidence-sanitizer.ts has multiple redaction gaps (snake_case keys like access_key_id/secret_access_key, and arrays under sensitive keys), creating a realistic path for secret exposure.
• There are also data-consistency/regression risks in apps/api/src/cloud-security/exception.service.ts, apps/api/src/cloud-security/reconciliation.service.ts, and apps/api/src/cloud-security/cloud-security-query.legacy.ts (duplicate exception/history rows and potentially hidden latest legacy failures).
• Pay close attention to apps/api/src/comments/comments-permission.guard.ts, apps/api/src/cloud-security/evidence-sanitizer.ts, and apps/api/src/cloud-security/exception.service.ts - auth bypass and secret/data-integrity issues are likely to affect production behavior.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/app/src/app/(app)/[orgId]/cloud-tests/components/EvidenceJsonViewer.tsx">

<violation number="1" location="apps/app/src/app/(app)/[orgId]/cloud-tests/components/EvidenceJsonViewer.tsx:68">
P2: The copy button is hover-only visible; add focus visibility so keyboard users can see the focused control.</violation>
</file>

<file name="apps/app/src/app/(app)/[orgId]/cloud-tests/components/MarkExceptionModal.tsx">

<violation number="1" location="apps/app/src/app/(app)/[orgId]/cloud-tests/components/MarkExceptionModal.tsx:163">
P2: The minimum expiration date is computed in UTC, which can incorrectly block valid next-day selections in some time zones.</violation>
</file>

<file name="apps/api/src/cloud-security/evidence-sanitizer.ts">

<violation number="1" location="apps/api/src/cloud-security/evidence-sanitizer.ts:48">
P1: Sensitive key detection misses snake_case credential names, so secrets like `access_key_id`/`secret_access_key` can bypass redaction.</violation>

<violation number="2" location="apps/api/src/cloud-security/evidence-sanitizer.ts:67">
P1: Arrays under sensitive keys are not redacted, which can expose secrets when credentials are represented as string lists.</violation>
</file>

<file name="apps/api/src/cloud-security/exception.service.spec.ts">

<violation number="1" location="apps/api/src/cloud-security/exception.service.spec.ts:197">
P2: The revocation success test does not assert `revokedAt`, so it can pass even if revocation timestamp writing regresses.</violation>
</file>

<file name="apps/api/src/comments/comments-permission.guard.ts">

<violation number="1" location="apps/api/src/comments/comments-permission.guard.ts:68">
P0: API key and service-token requests are granted without any permission scope check, creating an authorization bypass on comments endpoints.</violation>
</file>

<file name="apps/api/src/cloud-security/ai-description.prompt.ts">

<violation number="1" location="apps/api/src/cloud-security/ai-description.prompt.ts:102">
P2: The forbidden-content regexes are too narrow and miss blocked control citations like “CIS 1.8” and “PCI 8.2.3”, allowing policy-violating output to pass server-side validation.</violation>
</file>

<file name="apps/api/src/cloud-security/exception.service.ts">

<violation number="1" location="apps/api/src/cloud-security/exception.service.ts:55">
P1: `markAsException` is non-atomic and can create duplicate active exceptions under concurrent requests.</violation>
</file>

<file name="apps/api/src/cloud-security/cloud-security-query.legacy.ts">

<violation number="1" location="apps/api/src/cloud-security/cloud-security-query.legacy.ts:63">
P1: Filtering with `completedTime <= lastRunTime` can hide the newest legacy failure results when a run writes `IntegrationResult` but does not advance `lastRunAt`.</violation>
</file>

<file name="apps/api/src/cloud-security/cloud-security.controller.ts">

<violation number="1" location="apps/api/src/cloud-security/cloud-security.controller.ts:131">
P2: Date-only exception expirations are parsed as UTC midnight, which can expire exceptions earlier than the user-selected date.</violation>
</file>

<file name="apps/api/src/cloud-security/reconciliation.service.ts">

<violation number="1" location="apps/api/src/cloud-security/reconciliation.service.ts:69">
P1: The idempotency check ignores `FindingRegression`, so rerunning a regression-only run can write duplicate regression history rows.</violation>
</file>

_{Tip: instead of fixing issues one by one fix them all with cubic}

[tofikwest]

Pushed fix(cloud-tests): address cubic review findings (c05da23) in response to the cubic review.

Real bugs fixed

#	Severity	Issue	Fix
1	P0	CommentsPermissionGuard granted API keys / service tokens through without scope checks (authorization bypass)	Replicated standard PermissionGuard's API-key + service-token scope enforcement, but with the dynamic ${entityType}:${action} scope so finding-permission scopes work end-to-end
2	P1	evidence-sanitizer missed snake_case (access_key_id, secret_access_key) and other separator variants	Normalize _, -, ., whitespace before suffix-matching
3	P1	Arrays under sensitive keys (tokens: [...], credentials: [{...}]) passed through unredacted	Strings and objects inside arrays now redact to [REDACTED]; plurals added to suffix list
4	P1	markAsException non-atomic → concurrent requests could create duplicate active exceptions	New @@unique([orgId, connectionId, checkId, resourceId]) constraint + Prisma upsert. Re-marking after revoke clears revokedAt on the same row; mark/revoke history still lives in AuditLog
5	P1	Reconciliation idempotency probe only checked FindingResolution → regression-only runs could write duplicate FindingRegression rows	Now probes both tables
7	P2	Forbidden-content regex missed bare control citations like CIS 1.8 / PCI 8.2.3 / NIST AC-2	Added broader pattern \b(CIS|PCI|NIST|HIPAA|HITRUST|FedRAMP) ?[A-Z]*[- ]?\d+(\.\d+){0,3}\b
8	P2	EvidenceJsonViewer copy button had no focus visibility for keyboard users	Added focus-visible:opacity-100 + ring
9 / 11	P2	Date-only exception expiry parsed as UTC midnight, expiring ~8h early in US Pacific etc.	Frontend min uses local date; backend expands bare YYYY-MM-DD to end-of-day UTC
10	P2	Revoke test didn't assert revokedAt	Now asserts it's a Date in the expected window

Skipped (not introduced by this PR)

• People #6 cloud-security-query.legacy.ts — the completedTime <= lastRunTime filter is pre-existing legacy code that pre-dates this PR. I just moved it to a separate file during the Phase 1 split. Filed as a follow-up rather than fixing here to keep the PR scope clean.

New migration

• 20260513225000_finding_exception_unique — adds unique constraint enabling the upsert fix above.

Test counts

• Was: 96 (59 api + 37 frontend)
• Now: 105 (68 api + 37 frontend)
• Added 9 tests: API-key/service-token scope enforcement (3), snake_case + array redaction (4), regression-only idempotency (1), bare-control-number forbidden patterns (1), tightened revoke test assertions.

[cubic-dev-ai]

2 issues found across 15 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/api/src/cloud-security/evidence-sanitizer.ts">

<violation number="1" location="apps/api/src/cloud-security/evidence-sanitizer.ts:76">
P1: Nested arrays under sensitive keys are returned unchanged, so secret strings inside them can leak.</violation>
</file>

<file name="apps/api/src/cloud-security/cloud-security.controller.ts">

<violation number="1" location="apps/api/src/cloud-security/cloud-security.controller.ts:989">
P2: Validate `YYYY-MM-DD` components before calling `Date.UTC`; invalid calendar dates are currently rolled over to a different day instead of being rejected.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.}

[tofikwest]

Follow-up pushed (d52294eb6) answering both questions you raised:

1. Legacy filter (cubic finding #6)

Originally I'd skipped this as pre-existing code — the bug is real, just predates this PR. Since the code is on this branch anyway, fixed it here rather than punting:

• Bug: getLegacyFindings used completedAt <= lastRunAt as the upper bound. When a scan wrote IntegrationResult rows but did NOT advance Integration.lastRunAt (a known consistency gap on the legacy path), the newest results had completedAt > lastRunAt and got silently dropped.
• Fix: Extracted the filter to a pure filterToLatestScanResults helper that uses max(lastRunAt, latestResultCompletedAt) per integration as the reference point. Either signal can now be stale without dropping data.
• Tests: 8 unit tests covering the missed-advance recovery, per-integration scoping, null-lastRunAt fallback, window exclusion, and mixed-age data.

[tofikwest]

Both findings real. Fixed in 12411c7e6:

#1 P1 — nested arrays in sanitizer

redactArray only handled string + object elements. Nested arrays fell through return item unchanged, so { secrets: [["s1","s2"], ["s3"]] } would leak the inner strings. Added recursion + a test that pins down the new behavior.

#2 P2 — invalid calendar dates silently rolling forward

new Date(Date.UTC(2026, 1, 30)) doesn't throw — it rolls Feb 30 → March 2. An exception meant to expire Feb 30 would actually expire days later than the user expected. Extracted to exception-expiry.utils.ts and added component-validation that throws BadRequestException for impossible calendar dates (Feb 30, month 13, day 0, Feb 29 in non-leap years, etc.).

122 tests passing now (was 113 — added 1 nested-array sanitizer + 8 expiry).