diff --git a/e2e-tests/security_review.spec.ts b/e2e-tests/security_review.spec.ts index 9d1b0e2..3ee4156 100644 --- a/e2e-tests/security_review.spec.ts +++ b/e2e-tests/security_review.spec.ts @@ -43,3 +43,33 @@ test("security review - edit and use knowledge", async ({ po }) => { await po.waitForChatCompletion(); await po.snapshotServerDump("all-messages"); }); + +test("security review - multi-select and fix issues", async ({ po }) => { + await po.setUp({ autoApprove: true }); + await po.sendPrompt("tc=1"); + + await po.selectPreviewMode("security"); + + await po.page + .getByRole("button", { name: "Run Security Review" }) + .first() + .click(); + await po.waitForChatCompletion(); + + // Select the first two issues using individual checkboxes + const checkboxes = po.page.getByRole("checkbox"); + // Skip the first checkbox (select all) + await checkboxes.nth(1).click(); + await checkboxes.nth(2).click(); + + // Wait for the "Fix X Issues" button to appear + const fixSelectedButton = po.page.getByRole("button", { + name: "Fix 2 Issues", + }); + await fixSelectedButton.waitFor({ state: "visible" }); + + // Click the fix selected button + await fixSelectedButton.click(); + await po.waitForChatCompletion(); + await po.snapshotMessages(); +}); diff --git a/e2e-tests/snapshots/security_review.spec.ts_security-review---multi-select-and-fix-issues-1.aria.yml b/e2e-tests/snapshots/security_review.spec.ts_security-review---multi-select-and-fix-issues-1.aria.yml new file mode 100644 index 0000000..73f46dd --- /dev/null +++ b/e2e-tests/snapshots/security_review.spec.ts_security-review---multi-select-and-fix-issues-1.aria.yml @@ -0,0 +1,74 @@ +- paragraph: "Please fix the following 2 security issues in a simple and effective way:" +- list: + - listitem: + - strong: SQL Injection in User Lookup + - text: (critical severity) + - strong: What + - text: ": User input flows directly into database queries without validation, allowing attackers to execute arbitrary SQL commands" +- paragraph: + - strong: Risk + - text: ": An attacker could steal all customer data, delete your entire database, or take over admin accounts by manipulating the URL" +- paragraph: + - strong: Potential Solutions + - text: ":" +- list: + - listitem: + - text: "Use parameterized queries:" + - code: "`db.query('SELECT * FROM users WHERE id = ?', [userId])`" + - listitem: + - text: Add input validation to ensure + - code: "`userId`" + - text: is a number + - listitem: Implement an ORM like Prisma or TypeORM that prevents SQL injection by default +- paragraph: + - strong: Relevant Files + - text: ":" + - code: "`src/api/users.ts`" +- list: + - listitem: + - strong: Hardcoded AWS Credentials in Source Code + - text: (critical severity) + - strong: What + - text: ": AWS access keys are stored directly in the codebase and committed to version control, exposing full cloud infrastructure access" +- paragraph: + - strong: Risk + - text: ": Anyone with repository access (including former employees or compromised accounts) could spin up expensive resources, access S3 buckets with customer data, or destroy production infrastructure" +- paragraph: + - strong: Potential Solutions + - text: ":" +- list: + - listitem: Immediately rotate the exposed credentials in AWS IAM + - listitem: + - text: Use environment variables and add + - code: "`.env`" + - text: to + - code: "`.gitignore`" + - listitem: Implement AWS Secrets Manager or similar vault solution + - listitem: + - text: Scan git history and purge the credentials using tools like + - code: "`git-filter-repo`" +- paragraph: + - strong: Relevant Files + - text: ":" + - code: "`src/config/aws.ts`" + - text: "," + - code: "`src/services/s3-uploader.ts`" +- img +- text: file1.txt +- button "Edit": + - img +- img +- text: file1.txt +- paragraph: More EOM +- button: + - img +- img +- text: Approved +- img +- text: less than a minute ago +- img +- text: wrote 1 file(s) +- button "Undo": + - img +- button "Retry": + - img \ No newline at end of file diff --git a/e2e-tests/snapshots/security_review.spec.ts_security-review-1.aria.yml b/e2e-tests/snapshots/security_review.spec.ts_security-review-1.aria.yml index 773cb36..7765bbb 100644 --- a/e2e-tests/snapshots/security_review.spec.ts_security-review-1.aria.yml +++ b/e2e-tests/snapshots/security_review.spec.ts_security-review-1.aria.yml @@ -1,11 +1,15 @@ - table: - rowgroup: - - row "Level Issue Action": + - row "Select all issues Level Issue Action": + - cell "Select all issues": + - checkbox "Select all issues" - cell "Level" - cell "Issue" - cell "Action" - rowgroup: - - 'row "critical SQL Injection in User Lookup What: User input flows directly into database queries without validation, allowing attackers to execute arbitrary SQL commands Risk: An attac... Show more Fix Issue"': + - 'row "Select SQL Injection in User Lookup critical SQL Injection in User Lookup What: User input flows directly into database queries without validation, allowing attackers to execute arbitrary SQL commands Risk: An attac... Show more Fix Issue"': + - cell "Select SQL Injection in User Lookup": + - checkbox "Select SQL Injection in User Lookup" - cell "critical": - img - 'cell "SQL Injection in User Lookup What: User input flows directly into database queries without validation, allowing attackers to execute arbitrary SQL commands Risk: An attac... Show more"': @@ -20,7 +24,9 @@ - img - cell "Fix Issue": - button "Fix Issue" - - 'row "critical Hardcoded AWS Credentials in Source Code What: AWS access keys are stored directly in the codebase and committed to version control, exposing full cloud infrastructure access Risk: A... Show more Fix Issue"': + - 'row "Select Hardcoded AWS Credentials in Source Code critical Hardcoded AWS Credentials in Source Code What: AWS access keys are stored directly in the codebase and committed to version control, exposing full cloud infrastructure access Risk: A... Show more Fix Issue"': + - cell "Select Hardcoded AWS Credentials in Source Code": + - checkbox "Select Hardcoded AWS Credentials in Source Code" - cell "critical": - img - 'cell "Hardcoded AWS Credentials in Source Code What: AWS access keys are stored directly in the codebase and committed to version control, exposing full cloud infrastructure access Risk: A... Show more"': @@ -35,7 +41,9 @@ - img - cell "Fix Issue": - button "Fix Issue" - - 'row "high Missing Authentication on Admin Endpoints What: Administrative API endpoints can be accessed without authentication, relying only on URL obscurity Risk: An attacker who discovers thes... Show more Fix Issue"': + - 'row "Select Missing Authentication on Admin Endpoints high Missing Authentication on Admin Endpoints What: Administrative API endpoints can be accessed without authentication, relying only on URL obscurity Risk: An attacker who discovers thes... Show more Fix Issue"': + - cell "Select Missing Authentication on Admin Endpoints": + - checkbox "Select Missing Authentication on Admin Endpoints" - cell "high": - img - 'cell "Missing Authentication on Admin Endpoints What: Administrative API endpoints can be accessed without authentication, relying only on URL obscurity Risk: An attacker who discovers thes... Show more"': @@ -50,7 +58,9 @@ - img - cell "Fix Issue": - button "Fix Issue" - - 'row "high JWT Secret Using Default Value What: The application uses a hardcoded default JWT secret (\"your-secret-key\") for signing authentication tokens Risk: Attackers can forge val... Show more Fix Issue"': + - 'row "Select JWT Secret Using Default Value high JWT Secret Using Default Value What: The application uses a hardcoded default JWT secret (\"your-secret-key\") for signing authentication tokens Risk: Attackers can forge val... Show more Fix Issue"': + - cell "Select JWT Secret Using Default Value": + - checkbox "Select JWT Secret Using Default Value" - cell "high": - img - 'cell "JWT Secret Using Default Value What: The application uses a hardcoded default JWT secret (\"your-secret-key\") for signing authentication tokens Risk: Attackers can forge val... Show more"': @@ -65,7 +75,9 @@ - img - cell "Fix Issue": - button "Fix Issue" - - 'row "medium Unvalidated File Upload Extensions What: The file upload endpoint accepts any file type without validating extensions or content, only checking file size Risk: An attacker coul... Show more Fix Issue"': + - 'row "Select Unvalidated File Upload Extensions medium Unvalidated File Upload Extensions What: The file upload endpoint accepts any file type without validating extensions or content, only checking file size Risk: An attacker coul... Show more Fix Issue"': + - cell "Select Unvalidated File Upload Extensions": + - checkbox "Select Unvalidated File Upload Extensions" - cell "medium": - img - 'cell "Unvalidated File Upload Extensions What: The file upload endpoint accepts any file type without validating extensions or content, only checking file size Risk: An attacker coul... Show more"': @@ -80,7 +92,9 @@ - img - cell "Fix Issue": - button "Fix Issue" - - 'row "medium Missing CSRF Protection on State-Changing Operations What: POST, PUT, and DELETE endpoints don''t implement CSRF tokens, making them vulnerable to cross-site request forgery attacks Risk: An atta... Show more Fix Issue"': + - 'row "Select Missing CSRF Protection on State-Changing Operations medium Missing CSRF Protection on State-Changing Operations What: POST, PUT, and DELETE endpoints don''t implement CSRF tokens, making them vulnerable to cross-site request forgery attacks Risk: An atta... Show more Fix Issue"': + - cell "Select Missing CSRF Protection on State-Changing Operations": + - checkbox "Select Missing CSRF Protection on State-Changing Operations" - cell "medium": - img - 'cell "Missing CSRF Protection on State-Changing Operations What: POST, PUT, and DELETE endpoints don''t implement CSRF tokens, making them vulnerable to cross-site request forgery attacks Risk: An atta... Show more"': @@ -95,7 +109,9 @@ - img - cell "Fix Issue": - button "Fix Issue" - - 'row "low Verbose Error Messages Expose Stack Traces What: Production error responses include full stack traces and internal file paths that are sent to end users Risk: Attackers can use this in... Show more Fix Issue"': + - 'row "Select Verbose Error Messages Expose Stack Traces low Verbose Error Messages Expose Stack Traces What: Production error responses include full stack traces and internal file paths that are sent to end users Risk: Attackers can use this in... Show more Fix Issue"': + - cell "Select Verbose Error Messages Expose Stack Traces": + - checkbox "Select Verbose Error Messages Expose Stack Traces" - cell "low": - img - 'cell "Verbose Error Messages Expose Stack Traces What: Production error responses include full stack traces and internal file paths that are sent to end users Risk: Attackers can use this in... Show more"': @@ -110,7 +126,9 @@ - img - cell "Fix Issue": - button "Fix Issue" - - 'row "low Missing Security Headers What: The application doesn''t set recommended security headers like `X-Frame-Options`, `X-Content-Type-Options`, and `Strict-Transport-Security` ... Show more Fix Issue"': + - 'row "Select Missing Security Headers low Missing Security Headers What: The application doesn''t set recommended security headers like `X-Frame-Options`, `X-Content-Type-Options`, and `Strict-Transport-Security` ... Show more Fix Issue"': + - cell "Select Missing Security Headers": + - checkbox "Select Missing Security Headers" - cell "low": - img - 'cell "Missing Security Headers What: The application doesn''t set recommended security headers like `X-Frame-Options`, `X-Content-Type-Options`, and `Strict-Transport-Security` ... Show more"': diff --git a/src/components/preview_panel/SecurityPanel.tsx b/src/components/preview_panel/SecurityPanel.tsx index ecad7a4..2ec05fb 100644 --- a/src/components/preview_panel/SecurityPanel.tsx +++ b/src/components/preview_panel/SecurityPanel.tsx @@ -20,11 +20,13 @@ import { Info, ChevronDown, Pencil, + Wrench, } from "lucide-react"; import { useNavigate } from "@tanstack/react-router"; import { useStreamChat } from "@/hooks/useStreamChat"; import { showError } from "@/lib/toast"; import { Badge } from "@/components/ui/badge"; +import { Checkbox } from "@/components/ui/checkbox"; import type { SecurityFinding, SecurityReviewResult } from "@/ipc/ipc_types"; import { useState, useEffect } from "react"; import { VanillaMarkdownParser } from "@/components/chat/DyadMarkdownParser"; @@ -202,12 +204,36 @@ function SecurityHeader({ onRun, data, onOpenEditRules, + selectedCount, + onFixSelected, + isFixingSelected, }: { isRunning: boolean; onRun: () => void; data?: SecurityReviewResult | undefined; onOpenEditRules: () => void; + selectedCount: number; + onFixSelected: () => void; + isFixingSelected: boolean; }) { + const [isButtonVisible, setIsButtonVisible] = useState(false); + const [shouldRender, setShouldRender] = useState(false); + + useEffect(() => { + if (selectedCount > 0) { + // Show immediately + setShouldRender(true); + // Trigger animation after render + setTimeout(() => setIsButtonVisible(true), 10); + } else { + // Trigger exit animation + setIsButtonVisible(false); + // Hide after animation completes + const timer = setTimeout(() => setShouldRender(false), 300); + return () => clearTimeout(timer); + } + }, [selectedCount]); + return (
@@ -235,12 +261,59 @@ function SecurityHeader({
{data && data.findings.length > 0 && }
-
+
- +
+ + +
@@ -390,12 +463,28 @@ function FindingsTable({ onOpenDetails, onFix, fixingFindingKey, + selectedFindings, + onToggleSelection, + onToggleSelectAll, }: { findings: SecurityFinding[]; onOpenDetails: (finding: SecurityFinding) => void; onFix: (finding: SecurityFinding) => void; fixingFindingKey?: string | null; + selectedFindings: Set; + onToggleSelection: (findingKey: string) => void; + onToggleSelectAll: () => void; }) { + const sortedFindings = [...findings].sort( + (a, b) => getSeverityOrder(a.level) - getSeverityOrder(b.level), + ); + + const allSelected = + sortedFindings.length > 0 && + sortedFindings.every((finding) => + selectedFindings.has(createFindingKey(finding)), + ); + return (
+ + + Level @@ -416,102 +512,107 @@ function FindingsTable({ - {[...findings] - .sort( - (a, b) => getSeverityOrder(a.level) - getSeverityOrder(b.level), - ) - .map((finding, index) => { - const isLongDescription = - finding.description.length > DESCRIPTION_PREVIEW_LENGTH; - const displayDescription = isLongDescription - ? finding.description.substring(0, DESCRIPTION_PREVIEW_LENGTH) + - "..." - : finding.description; - const findingKey = createFindingKey(finding); - const isFixing = fixingFindingKey === findingKey; + {sortedFindings.map((finding, index) => { + const isLongDescription = + finding.description.length > DESCRIPTION_PREVIEW_LENGTH; + const displayDescription = isLongDescription + ? finding.description.substring(0, DESCRIPTION_PREVIEW_LENGTH) + + "..." + : finding.description; + const findingKey = createFindingKey(finding); + const isFixing = fixingFindingKey === findingKey; + const isSelected = selectedFindings.has(findingKey); - return ( - - - - - -
onOpenDetails(finding)} - onKeyDown={(e) => { - if (e.key === "Enter" || e.key === " ") { - e.preventDefault(); - onOpenDetails(finding); - } - }} - > -
- {finding.title} -
-
- -
- {isLongDescription && ( - - )} + return ( + + + onToggleSelection(findingKey)} + aria-label={`Select ${finding.title}`} + onClick={(e) => e.stopPropagation()} + /> + + + + + +
onOpenDetails(finding)} + onKeyDown={(e) => { + if (e.key === "Enter" || e.key === " ") { + e.preventDefault(); + onOpenDetails(finding); + } + }} + > +
+ {finding.title}
- - - - - - ); - })} +
+ +
+ {isLongDescription && ( + + )} +
+ + + + + + ); + })}
@@ -607,6 +708,10 @@ export const SecurityPanel = () => { const [rulesContent, setRulesContent] = useState(""); const [fixingFindingKey, setFixingFindingKey] = useState(null); const [isSaving, setIsSaving] = useState(false); + const [selectedFindings, setSelectedFindings] = useState>( + new Set(), + ); + const [isFixingSelected, setIsFixingSelected] = useState(false); const { content: fetchedRules, @@ -623,6 +728,11 @@ export const SecurityPanel = () => { } }, [fetchedRules]); + // Clear selections when data changes (e.g., after a new review) + useEffect(() => { + setSelectedFindings(new Set()); + }, [data]); + const handleSaveRules = async () => { if (!selectedAppId) { showError("No app selected"); @@ -725,6 +835,82 @@ ${finding.description}`; } }; + const handleToggleSelection = (findingKey: string) => { + setSelectedFindings((prev) => { + const newSet = new Set(prev); + if (newSet.has(findingKey)) { + newSet.delete(findingKey); + } else { + newSet.add(findingKey); + } + return newSet; + }); + }; + + const handleToggleSelectAll = () => { + if (!data?.findings) return; + + const sortedFindings = [...data.findings].sort( + (a, b) => getSeverityOrder(a.level) - getSeverityOrder(b.level), + ); + + const allKeys = sortedFindings.map((finding) => createFindingKey(finding)); + const allSelected = allKeys.every((key) => selectedFindings.has(key)); + + if (allSelected) { + setSelectedFindings(new Set()); + } else { + setSelectedFindings(new Set(allKeys)); + } + }; + + const handleFixSelected = async () => { + if (!selectedAppId || selectedFindings.size === 0 || !data?.findings) { + showError("No issues selected"); + return; + } + + try { + setIsFixingSelected(true); + + // Get the selected findings + const findingsToFix = data.findings.filter((finding) => + selectedFindings.has(createFindingKey(finding)), + ); + + // Create a new chat + const chatId = await IpcClient.getInstance().createChat(selectedAppId); + + // Navigate to the new chat + setSelectedChatId(chatId); + await navigate({ to: "/chat", search: { id: chatId } }); + + // Build a comprehensive prompt for all selected issues + const issuesList = findingsToFix + .map( + (finding, index) => + `${index + 1}. **${finding.title}** (${finding.level} severity)\n${finding.description}`, + ) + .join("\n\n"); + + const prompt = `Please fix the following ${findingsToFix.length} security issue${findingsToFix.length !== 1 ? "s" : ""} in a simple and effective way: + +${issuesList}`; + + await streamMessage({ + prompt, + chatId, + onSettled: () => { + setIsFixingSelected(false); + setSelectedFindings(new Set()); + }, + }); + } catch (err) { + showError(`Failed to create fix chat: ${err}`); + setIsFixingSelected(false); + } + }; + if (isLoading) { return ; } @@ -746,6 +932,9 @@ ${finding.description}`; refetchRules(); } }} + selectedCount={selectedFindings.size} + onFixSelected={handleFixSelected} + isFixingSelected={isFixingSelected} /> {isRunningReview ? ( @@ -761,6 +950,9 @@ ${finding.description}`; onOpenDetails={openFindingDetails} onFix={handleFixIssue} fixingFindingKey={fixingFindingKey} + selectedFindings={selectedFindings} + onToggleSelection={handleToggleSelection} + onToggleSelectAll={handleToggleSelectAll} /> ) : ( diff --git a/src/ipc/ipc_client.ts b/src/ipc/ipc_client.ts index 5ed0f8d..958f060 100644 --- a/src/ipc/ipc_client.ts +++ b/src/ipc/ipc_client.ts @@ -445,12 +445,14 @@ export class IpcClient { attachments: fileDataArray, }) .catch((err) => { + console.error("Error streaming message:", err); showError(err); onError(String(err)); this.chatStreams.delete(chatId); }); }) .catch((err) => { + console.error("Error streaming message:", err); showError(err); onError(String(err)); this.chatStreams.delete(chatId); @@ -465,6 +467,7 @@ export class IpcClient { selectedComponent, }) .catch((err) => { + console.error("Error streaming message:", err); showError(err); onError(String(err)); this.chatStreams.delete(chatId); @@ -475,12 +478,6 @@ export class IpcClient { // Method to cancel an ongoing stream public cancelChatStream(chatId: number): void { this.ipcRenderer.invoke("chat:cancel", chatId); - const callbacks = this.chatStreams.get(chatId); - if (callbacks) { - this.chatStreams.delete(chatId); - } else { - console.error("Tried canceling chat that doesn't exist"); - } } // Create a new chat for an app