Add structured vulnerability findings workflow#915
Open
ross0x01 wants to merge 36 commits into
Open
Conversation
Contributor
Author
|
Reopening immediately to request a fresh Trigger.dev GitHub App clone after the isolated pre-build clone failure. The identical commit already deployed successfully as Trigger preview version 20260716.2. |
ross0x01
force-pushed
the
codex/hac-30-findings
branch
from
July 16, 2026 22:47
9a89ddb to
4064018
Compare
Contributor
Author
|
@coderabbitai review |
Contributor
Author
|
Final integration evidence for
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughChangesThis PR adds structured vulnerability findings with CVSS scoring, validated persistence, agent reporting, sharing sanitization, deletion cleanup, sidebar integration, source-message navigation, and responsive list/detail interfaces. Vulnerability findings
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Agent
participant ConvexFindings
participant FindingsPage
participant FindingDetail
Agent->>ConvexFindings: createFindingForBackend(report)
ConvexFindings-->>Agent: finding_id and compact result
FindingsPage->>ConvexFindings: listFindings(filters)
ConvexFindings-->>FindingsPage: paginated finding summaries
FindingsPage->>FindingDetail: open selected finding
FindingDetail->>ConvexFindings: getFinding(finding_id)
ConvexFindings-->>FindingDetail: finding detail record
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (1)
convex/__tests__/findings.shared-security.test.ts (1)
6-19: 🔒 Security & Privacy | 🔵 Trivial | 🏗️ Heavy liftExercise the handlers instead of checking source text.
This regex can pass when the sanitizer exists but is no longer applied to the returned or inserted parts. Invoke
getSharedMessagesandforkSharedChatwith the private fixture, then assert their observable outputs contain onlydata-shared-finding.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@convex/__tests__/findings.shared-security.test.ts` around lines 6 - 19, Replace the source-text regex assertions in the test covering messagesSource and forksSource with handler-level checks: invoke getSharedMessages and forkSharedChat using the private fixture, then assert their returned messages or inserted fork parts contain only data-shared-finding. Preserve coverage for both public reads and shared-chat forks while validating observable outputs rather than sanitizer presence.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/`(chat)/findings/page.tsx:
- Around line 296-316: Update the mobile detail branch around selectedFindingId
and FindingDetail to use the existing modal Dialog/Sheet primitive, or
equivalent modal semantics, instead of a fixed container. Move focus into the
detail view when it opens, trap focus within it while open, mark the findings
list inert, and restore focus to the triggering finding control when it closes.
In `@app/components/MessagePartHandler.tsx`:
- Around line 265-273: Update arePropsEqual to compare part.type for all message
parts, and when the type is "data-shared-finding", deep-compare
prevProps.part.data with nextProps.part.data so FindingCard updates when
shared-finding content changes. Preserve the existing comparisons for other
props and part types.
In `@app/components/SidebarHeader.tsx`:
- Around line 174-186: Update both Findings controls in the SidebarHeader
component, including the expanded button near the other occurrence, to set
aria-current="page" when pathname equals "/findings" and omit it otherwise. Keep
the existing visual active-state class and button behavior unchanged.
In `@convex/chats.ts`:
- Around line 1624-1632: Replace the unbounded findings query and deletion loop
in the chat deletion path with the existing batched deletion workflow, using
query.take(n) for each batch. Delete only the bounded batch per mutation, then
use ctx.scheduler.runAfter(...) to schedule continuation until all findings for
the chat are removed; do not call .collect() here.
In `@convex/findings.ts`:
- Around line 270-288: Update getFindingSourceChats to derive the unique chat
IDs from the authenticated user’s findings, then return only matching chats.
Replace the unbounded chats query collect() with for await iteration or
pagination, while preserving the existing access check and chat ordering.
- Around line 16-26: Update getChat and getFindingByPublicId to accept the
shared typed Convex context used by query and mutation helpers instead of any,
and replace first() with unique() for both by_chat_id and by_finding_id lookups
so duplicate records surface as errors.
In `@convex/schema.ts`:
- Around line 251-253: Add the by_user_severity_chat_created index to the
relevant schema definition in convex/schema.ts over user_id, severity, chat_id,
and created_at. In convex/findings.ts, update listFindings to use this index via
withIndex when both severity and chat filters are provided, and remove the
corresponding post-query filter.
In `@lib/ai/tools/schemas.ts`:
- Around line 595-596: Revise the vulnerability-report guidance so idempotency
means one successfully persisted report per root cause, while allowing a retry
after an explicit general “not saved” failure; prohibit retries only when the
response indicates a duplicate. Apply this distinction in
lib/ai/tools/schemas.ts lines 595-596 and 859-862, mirror it in
lib/system-prompt.ts lines 323-327, and update
lib/__tests__/system-prompt.test.ts lines 131-136 to assert successful
persistence, duplicate suppression, and general-failure retry behavior.
---
Nitpick comments:
In `@convex/__tests__/findings.shared-security.test.ts`:
- Around line 6-19: Replace the source-text regex assertions in the test
covering messagesSource and forksSource with handler-level checks: invoke
getSharedMessages and forkSharedChat using the private fixture, then assert
their returned messages or inserted fork parts contain only data-shared-finding.
Preserve coverage for both public reads and shared-chat forks while validating
observable outputs rather than sanitizer presence.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 24187c37-d790-4487-b1bf-62615773f22f
⛔ Files ignored due to path filters (1)
convex/_generated/api.d.tsis excluded by!**/_generated/**
📒 Files selected for processing (46)
app/(chat)/findings/__tests__/page.test.tsxapp/(chat)/findings/page.tsxapp/components/ComputerSidebar.tsxapp/components/MessagePartHandler.tsxapp/components/SidebarHeader.tsxapp/components/computer-sidebar-utils.tsxapp/components/findings/FindingCard.tsxapp/components/findings/FindingDetail.tsxapp/components/findings/__tests__/FindingCard.test.tsxapp/components/findings/__tests__/FindingDetail.test.tsxapp/components/tools/FindingToolHandler.tsxapp/components/tools/__tests__/FindingToolHandler.test.tsxapp/hooks/useSidebarNavigation.tsapp/share/[shareId]/components/SharedMessagePartHandler.tsxconvex/__tests__/chats.agentApprovalLifecycle.test.tsconvex/__tests__/findings.shared-security.test.tsconvex/__tests__/findings.test.tsconvex/__tests__/userDeletion.test.tsconvex/chats.tsconvex/findings.tsconvex/messages.tsconvex/schema.tsconvex/sharedChats.tsconvex/userDeletion.tslib/__tests__/system-prompt.test.tslib/ai/tools/__tests__/findings.test.tslib/ai/tools/__tests__/schemas.test.tslib/ai/tools/findings.tslib/ai/tools/index.tslib/ai/tools/schemas.tslib/api/__tests__/agent-long-contracts.test.tslib/chat/chat-processor.tslib/chat/compaction/__tests__/prune-tool-outputs.test.tslib/chat/compaction/prune-tool-outputs.tslib/db/actions.tslib/findings/__tests__/cvss31.test.tslib/findings/__tests__/share-sanitizer.test.tslib/findings/__tests__/validation.test.tslib/findings/cvss31.tslib/findings/share-sanitizer.tslib/findings/validation.tslib/system-prompt.tslib/utils/sidebar-utils.tstypes/chat.tstypes/finding.tstypes/index.ts
Contributor
Author
|
Also addressed the non-inline shared-security test nit in Validation after the review fixes:
|
Contributor
Author
|
@coderabbitai review\n\nThe latest status check passed, but the walkthrough reports that the review itself failed. Please run a fresh review of |
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/components/Messages.tsx`:
- Around line 126-134: Update the useSourceMessageNavigation call in
Messages.tsx to track pagination progress using the unfiltered loaded message
count rather than visibleMessageIds, while preserving visibleMessageIds for
source-message navigation. Ensure pages containing only auto-continue messages
still allow the next loadMore(28) request.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8b8c5afb-52bc-4881-bc5e-00ba9ceeae63
📒 Files selected for processing (8)
app/(chat)/findings/__tests__/page.test.tsxapp/(chat)/findings/page.tsxapp/components/Messages.tsxapp/components/findings/FindingDetail.tsxapp/components/findings/__tests__/FindingDetail.test.tsxapp/hooks/__tests__/useSourceMessageNavigation.test.tsxapp/hooks/useSourceMessageNavigation.tslib/findings/source-message.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- app/components/findings/tests/FindingDetail.test.tsx
- app/(chat)/findings/page.tsx
- app/(chat)/findings/tests/page.test.tsx
- app/components/findings/FindingDetail.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/hooks/useSourceMessageNavigation.ts`:
- Around line 48-51: Remove the explicit window.dispatchEvent call in the source
message navigation flow, leaving target.dispatchEvent with the bubbling
CustomEvent as the sole STICKY_BOTTOM_ESCAPE_EVENT dispatch so listeners are
triggered only once.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 45cfa10b-1b62-4117-88af-92a2f5cf03f2
📒 Files selected for processing (3)
app/components/Messages.tsxapp/hooks/__tests__/useSourceMessageNavigation.test.tsxapp/hooks/useSourceMessageNavigation.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- app/components/Messages.tsx
- app/hooks/tests/useSourceMessageNavigation.test.tsx
Contributor
Author
|
@coderabbitai review Please review the Findings workspace and report presentation refactor in |
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/components/tools/ToolErrorHandler.tsx`:
- Around line 42-48: Update the ToolErrorHandler component to pass the
handleKeyDown value returned by useToolSidebar into ToolErrorBlock, then forward
that prop through ToolErrorBlock to ToolBlock, matching the keyboard-handler
flow used by other interactive tool cards.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7a47f734-b0a3-4c9f-906b-95982fb86c7e
📒 Files selected for processing (38)
app/(chat)/findings/__tests__/page.test.tsxapp/(chat)/findings/page.tsxapp/components/ComputerSidebar.tsxapp/components/MessagePartHandler.tsxapp/components/Messages.tsxapp/components/__tests__/ComputerSidebar.reconnect.test.tsxapp/components/__tests__/MessagePartHandler.memo.test.tsapp/components/computer-sidebar-utils.tsxapp/components/findings/FindingDetail.tsxapp/components/findings/__tests__/FindingDetail.test.tsxapp/components/tools/FindingToolHandler.tsxapp/components/tools/TerminalToolHandler.tsxapp/components/tools/TodoToolHandler.tsxapp/components/tools/ToolErrorDetail.tsxapp/components/tools/ToolErrorHandler.tsxapp/components/tools/WebToolHandler.tsxapp/components/tools/__tests__/FindingToolHandler.test.tsxapp/components/tools/__tests__/ToolErrorDetail.test.tsxapp/components/tools/__tests__/shell-tool-utils.test.tsapp/components/tools/shell-tool-utils.tsapp/globals.cssapp/hooks/__tests__/useSourceMessageNavigation.test.tsxapp/hooks/useSourceMessageNavigation.tsapp/share/[shareId]/components/SharedMessagePartHandler.tsxcomponents/ui/__tests__/tool-block.test.tsxcomponents/ui/dialog.tsxcomponents/ui/tool-block.tsxlib/ai/tools/__tests__/schemas.test.tslib/ai/tools/schemas.tslib/chat/__tests__/tool-error-display.test.tslib/chat/tool-error-display.tslib/findings/__tests__/share-sanitizer.test.tslib/findings/__tests__/validation.test.tslib/findings/share-sanitizer.tslib/findings/validation.tslib/utils/__tests__/sidebar-utils.test.tslib/utils/sidebar-utils.tstypes/chat.ts
🚧 Files skipped from review as they are similar to previous changes (12)
- lib/findings/tests/share-sanitizer.test.ts
- lib/findings/share-sanitizer.ts
- app/share/[shareId]/components/SharedMessagePartHandler.tsx
- app/components/computer-sidebar-utils.tsx
- app/hooks/tests/useSourceMessageNavigation.test.tsx
- app/components/tools/FindingToolHandler.tsx
- app/hooks/useSourceMessageNavigation.ts
- app/components/ComputerSidebar.tsx
- app/components/Messages.tsx
- lib/ai/tools/schemas.ts
- lib/findings/validation.ts
- lib/findings/tests/validation.test.ts
Contributor
Author
|
@coderabbitai review Please review the final head |
|
✅ Action performedReview finished.
|
Contributor
Author
|
@coderabbitai review Please review the final head |
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/`(chat)/findings/page.tsx:
- Around line 78-79: Update escapeCsvCell to prefix a guard character for values
whose first character is =, +, -, @, tab, or carriage return, then retain the
existing quote-wrapping and double-quote escaping. Apply the guard before
String(value) is quoted so exported finding.title and finding.target cells
cannot be interpreted as formulas.
In `@components/ui/radio-group.tsx`:
- Line 30: Update the class strings used by the radio indicator in the
radio-group component, including both occurrences, to remove the fixed
text-foreground color or replace it with text-inherit so the indicator inherits
the selected or unselected label color. Preserve the remaining styling classes
unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d2285ba5-8aaa-446e-b8c7-5a14d55b2a45
📒 Files selected for processing (16)
app/(chat)/findings/__tests__/page.test.tsxapp/(chat)/findings/page.tsxapp/components/findings/FindingCard.tsxapp/components/findings/FindingDetail.tsxapp/components/findings/__tests__/FindingDetail.test.tsxcomponents/ui/radio-group.tsxconvex/__tests__/findings.test.tsconvex/findings.tsconvex/schema.tslib/findings/__tests__/category.test.tslib/findings/__tests__/lifecycle.test.tslib/findings/__tests__/validation.test.tslib/findings/category.tslib/findings/lifecycle.tslib/findings/validation.tstypes/finding.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- types/finding.ts
- app/components/findings/FindingCard.tsx
- convex/schema.ts
- lib/findings/validation.ts
Contributor
Author
|
CodeRabbit follow-up addressed in
Validation: focused tests, typecheck, lint, Prettier, and the full 300-suite / 2,956-test pre-commit run all pass. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/findingsworkspace and navigation for saved vulnerability reportscreate_vulnerability_reporttool with strict validation, deterministic same-chat dedupe, server-derived CVSS 3.1 scoring/severity, and concise tool outputValidation
pnpm test --runInBand— 300 suites, 2,956 tests passedpnpm typecheckpnpm lint— passes with 6 existing warningspnpm exec prettier --check <16 changed files>Visual verification
Verified in Google Chrome on desktop and a 390×844 mobile viewport:
/findingsManual verification
/findings. Verify search plus severity, category, and status filters; use Start new scan; export the current filtered results./findings. Verify the blurred report modal closes with Back/X/Escape and its source link opens the exact originating assistant message.Summary by CodeRabbit