feat(security-agent): add audit reports and refresh management UX#4069
feat(security-agent): add audit reports and refresh management UX#4069jeanduplessis wants to merge 13 commits into
Conversation
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Code Review SummaryStatus: 3 Issues Found | Recommendation: Address before merge Executive SummaryThe Overview
Issue Details (click to expand)CRITICAL
WARNING
Resolved from Prior Review
Files Reviewed (66 files)
Fix these issues in Kilo Cloud Previous Review Summaries (3 snapshots, latest commit 985b43f)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 985b43f)Status: 2 Issues Found | Recommendation: Address before merge Executive SummaryTwo audit trail integrity concerns from the initial review persist: silently swallowed audit write failures in the remediation service and a latent undefined-in-event-key path in auto-dismiss. The incremental diff (commit Overview
Issue Details (click to expand)WARNING
Incremental Review (baa0f39 → HEAD)31 files changed. Key changes:
No new issues found. Files Reviewed (31 files)
Fix these issues in Kilo Cloud Previous review (commit baa0f39)Status: 2 Issues Found | Recommendation: Address before merge Executive SummaryTwo audit trail integrity concerns in the remediation and auto-dismiss service workers: silently swallowed audit write failures and a latent undefined-in-event-key path. No new issues found in the incremental diff (commit Overview
Issue Details (click to expand)WARNING
Incremental Review (commit baa0f39 vs f4e1e3b)15 files changed in this increment. Key changes:
Files Reviewed (12 key files + 15 incremental)
Fix these issues in Kilo Cloud Previous review (commit f4e1e3b)Status: 2 Issues Found | Recommendation: Address before merge Executive SummaryTwo audit trail integrity concerns in the remediation and auto-dismiss service workers: silently swallowed audit write failures and a latent undefined-in-event-key path. Overview
Issue Details (click to expand)WARNING
Files Reviewed (12 key files)
Reviewed by deepseek-v4-pro-20260423 · 1,092,096 tokens Review guidance: REVIEW.md from base branch |
jeanduplessis
force-pushed
the
sa-audit-report
branch
from
985b43f to
a72514f
Compare
| ALTER TABLE "security_audit_log" ADD COLUMN "schema_version" smallint;--> statement-breakpoint | ||
| ALTER TABLE "security_audit_log" ADD COLUMN "finding_snapshot" jsonb;--> statement-breakpoint | ||
| ALTER TABLE "security_audit_log" ADD COLUMN "source_context" text;--> statement-breakpoint | ||
| CREATE UNIQUE INDEX "UQ_security_audit_log_org_event_key" ON "security_audit_log" USING btree ("owned_by_organization_id","event_key") WHERE "security_audit_log"."owned_by_organization_id" IS NOT NULL AND "security_audit_log"."event_key" IS NOT NULL;--> statement-breakpoint |
There was a problem hiding this comment.
CRITICAL: CREATE UNIQUE INDEX on populated security_audit_log table without CONCURRENTLY
The security_audit_log table has existed since migration 0027 and is populated in production. Creating unique indexes (lines 10-11) and composite indexes (lines 12-13) without CONCURRENTLY acquires an AccessExclusiveLock, blocking all concurrent reads and writes for the duration of each index build. Use CREATE UNIQUE INDEX CONCURRENTLY / CREATE INDEX CONCURRENTLY for these.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| } | ||
|
|
||
| eligible++; | ||
| byConfidence[triage.confidence]++; |
There was a problem hiding this comment.
WARNING: Unvalidated triage.confidence from DB JSONB can produce NaN counters
The old code if (confidence && confidence in byConfidence) guarded against unexpected confidence values before incrementing. The new code byConfidence[triage.confidence]++ increments without validation. If the DB-stored JSON has a confidence value outside 'high' | 'medium' | 'low' (e.g., from a legacy version or corrupted data), byConfidence[someValue] is undefined, and undefined++ yields NaN, propagating corrupt count data to the UI. Consider restoring the in byConfidence guard.
| byConfidence[triage.confidence]++; | |
| byConfidence[triage.confidence] = (byConfidence[triage.confidence] ?? 0) + 1; |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| }) | ||
| .where(eq(security_findings.id, params.findingId)); | ||
| if (sandbox) { | ||
| if (sandbox.isExploitable !== false || sandbox.suggestedAction !== 'dismiss') return; |
There was a problem hiding this comment.
WARNING: Sandbox analysis gating now blocks triage-based auto-dismiss as fallback
The new sandbox gate (lines 204-222) returns early for ALL sandbox-ful findings that don't meet the stricter dual condition isExploitable === false AND suggestedAction === 'dismiss'. Previously, findings with sandbox analysis that was not isExploitable === false would fall through to triage evaluation (line 225). Now, if sandbox ran but didn't explicitly recommend dismissal, triage is never evaluated as a fallback — the function returns without auto-dismissing, even if triage independently recommended dismissal. The auto-dismiss-service.ts comment block explicitly documents this as intentional ("After Tier 2 sandbox: dismiss only when analysis says not exploitable and recommends dismissal"), but the behavioral change from the old worker code path should be confirmed intentional.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
1 participant
Summary
Security Agent now records durable Security Finding activity, provides owner-scoped Audit Reports, and guides users through analysis, dismissal, and remediation workflows.
Why this change is needed
Security owners need period-bounded evidence of material Security Finding actions and outcomes for investigation and compliance work without implying complete legacy reconstruction or aggregate historical SLA compliance. Existing management surfaces also obscure action eligibility, stale-analysis decisions, and remediation policy rejections, making safe next steps harder to understand.
How this is addressed
Human Verification
origin/main..specs/security-agent.mdandCONTEXT.md.Reviewer Notes
Human Reviewer Flags
Code Reviewer Agent
Code Reviewer Notes