feat: complete Phase 2 - API server, Dashboard, and CVE sync#5
feat: complete Phase 2 - API server, Dashboard, and CVE sync#5chojuninengu wants to merge 19 commits intomainfrom
Conversation
…nce scan results UI
…d database schema for scan tracking
…ase database connection pool, and disable caching for history API responses.
…RL, and improve endpoint path handling for AI providers
…support for completed scans
📝 WalkthroughWalkthroughScans were reworked to run asynchronously with SSE streaming; server persistence, CVE synchronization, AI model listing, and DB migrations were added; web UI, VS Code extension, CLI config, Dockerfiles, GitHub Actions for GHCR publishing, and related build scripts were introduced or updated. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client (Web / VSCode)
participant Server as API Server (axum)
participant Scanner as Scanner Engine (crates/scanner)
participant DB as Postgres
Client->>Server: POST /api/v1/scan {code, language, ai_config?}
Server->>Server: generate scan_id, create broadcast sender, cache slot
Server->>Scanner: spawn scan_stream(scan_config, sender)
Scanner->>Server: -> channel: ScanEvent::Progress / Finding / Complete / Error
Server->>DB: upsert scan_results and scan summary on events
Server->>Server: cache events for replay
Client->>Server: GET /api/v1/scan/:id/events (SSE)
Server->>Client: stream cached + live ScanEvent SSE messages
Client->>Client: update UI/diagnostics as events arrive
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Review Summary by QodoComplete Phase 2 - API server with SSE streaming, CVE sync, and full-stack dashboard
WalkthroughsDescription• **API Server Refactor:** Complete rewrite with async SSE streaming for real-time scan progress, PostgreSQL persistence, and CVE enrichment from local database • **CVE Synchronization:** New module syncing vulnerability data from NVD API v2 with CVSS scores and descriptions stored in PostgreSQL • **Event Streaming Architecture:** Added ScanEvent enum and scan_stream() function enabling real-time progress updates across all components • **Dashboard Enhancements:** New pages for scan history, AI settings configuration, and CVE database management with real-time progress indicators • **VS Code Extension:** Implemented sidebar webview provider with SSE streaming integration for in-editor scan results and diagnostics • **CLI Configuration:** Added local config management with ~/.config/zenvra/config.json for persistent AI settings • **Model Discovery:** Unified list_models() interface across all AI providers (Anthropic, OpenAI, Google, Custom) • **Infrastructure:** Added Docker Compose configuration, multi-stage Dockerfiles for API and web services, database migrations, and GitHub Actions workflow for container publishing • **Dependencies:** Updated server dependencies for database support (sqlx, chrono), CLI (clap), and streaming (tokio-stream, dashmap) Diagramflowchart LR
NVD["NVD API v2"]
OSV["OSV Database"]
CVESync["CVE Sync Module"]
PG["PostgreSQL"]
Redis["Redis"]
APIServer["Axum API Server"]
SSE["SSE Streaming"]
Dashboard["SvelteKit Dashboard"]
VSCode["VS Code Extension"]
Scanner["Scanner Engine"]
NVD -- "fetch vulnerabilities" --> CVESync
OSV -- "fetch vulnerabilities" --> CVESync
CVESync -- "upsert records" --> PG
Scanner -- "emit events" --> SSE
APIServer -- "stream events" --> SSE
SSE -- "real-time updates" --> Dashboard
SSE -- "real-time updates" --> VSCode
Dashboard -- "trigger scans" --> APIServer
VSCode -- "trigger scans" --> APIServer
APIServer -- "cache results" --> Redis
APIServer -- "persist metadata" --> PG
PG -- "CVE enrichment" --> APIServer
File Changes1. crates/server/src/main.rs
|
Code Review by Qodo
|
| let provider = $state(browser ? (localStorage.getItem('zenvra_ai_provider') ?? 'anthropic') : 'anthropic'); | ||
| let model = $state(browser ? (localStorage.getItem('zenvra_ai_model') ?? '') : ''); | ||
| let apiKey = $state(browser ? (localStorage.getItem('zenvra_ai_api_key') ?? '') : ''); | ||
| let endpoint = $state(browser ? (localStorage.getItem('zenvra_ai_endpoint') ?? '') : ''); | ||
|
|
||
| const isConfigured = $derived(!!model && !!apiKey); | ||
|
|
||
| /** Persist changes to both reactive state and localStorage atomically. */ | ||
| function save(p: string, m: string, key: string, ep: string): void { | ||
| provider = p; | ||
| model = m; | ||
| apiKey = key; | ||
| endpoint = ep; | ||
|
|
||
| if (browser) { | ||
| localStorage.setItem('zenvra_ai_provider', p); | ||
| localStorage.setItem('zenvra_ai_model', m); | ||
| localStorage.setItem('zenvra_ai_api_key', key); | ||
| if (ep) localStorage.setItem('zenvra_ai_endpoint', ep); | ||
| else localStorage.removeItem('zenvra_ai_endpoint'); |
There was a problem hiding this comment.
3. Api key stored in localstorage 📘 Rule violation ⛨ Security
The web app persists zenvra_ai_api_key in localStorage, placing a secret in browser-accessible storage. This violates the rule prohibiting secrets/API keys in client-side code paths.
Agent Prompt
## Issue description
The frontend stores the user's AI API key in `localStorage`, which is client-side accessible.
## Issue Context
Compliance requires secrets not be present in browser-delivered code or storage.
## Fix Focus Areas
- apps/web/src/lib/stores/aiConfig.svelte.ts[12-31]
- apps/web/src/routes/settings/ai/+page.svelte[6-55]
- apps/web/src/routes/scan/+page.svelte[34-46]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| const response = await fetch("http://localhost:8080/api/v1/scan", { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify({ | ||
| code, | ||
| language: "python", | ||
| engines: ["sast", "secrets"], | ||
| ai_config: hasAiConfig ? { | ||
| provider: aiConfig.provider, | ||
| api_key: aiConfig.apiKey, | ||
| model: aiConfig.model, | ||
| endpoint: aiConfig.endpoint || undefined, | ||
| } : undefined | ||
| }) | ||
| }); | ||
|
|
||
| if (!response.ok) throw new Error("Failed to start scan"); | ||
| const { scan_id } = await response.json(); | ||
|
|
||
| // Step 2: Subscribe to the SSE stream | ||
| const eventSource = new EventSource(`http://localhost:8080/api/v1/scan/${scan_id}/events`); | ||
|
|
There was a problem hiding this comment.
4. scan/+page.svelte uses raw fetch 📘 Rule violation ⚙ Maintainability
The scan page introduces direct fetch(...) and EventSource(...) calls instead of routing requests through the typed API client in apps/web/src/lib/api.ts. This breaks the frontend networking standard and bypasses centralized typing/error handling.
Agent Prompt
## Issue description
The scan page uses ad-hoc `fetch` and hardcoded URLs instead of the typed API client.
## Issue Context
Typed API client should centralize base URL handling, typing, and consistent errors.
## Fix Focus Areas
- apps/web/src/routes/scan/+page.svelte[34-55]
- apps/web/src/lib/api.ts[38-96]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| const handleSave = () => { | ||
| if (!selectedModel.trim()) { error = "Model name is required."; return; } | ||
| if (!apiKey.trim()) { error = "API key is required."; return; } | ||
| error = null; | ||
|
|
||
| // Persist via the store — writes localStorage atomically | ||
| aiConfig.save(provider, selectedModel.trim(), apiKey.trim(), endpoint.trim()); | ||
|
|
||
| saveSuccess = true; | ||
| clearTimeout(successTimer); | ||
| successTimer = setTimeout(() => { saveSuccess = false; }, 3500); | ||
| }; |
There was a problem hiding this comment.
5. Forms lack zod validation 📘 Rule violation ⛨ Security
The new AI settings form performs only manual string checks and does not use zod schemas for input validation. This violates the requirement for schema-based validation at form/API trust boundaries.
Agent Prompt
## Issue description
AI settings inputs are not validated with Zod; only ad-hoc checks are used.
## Issue Context
Zod schemas should validate provider/model/key/endpoint (including URL validation for custom endpoints) before saving.
## Fix Focus Areas
- apps/web/src/routes/settings/ai/+page.svelte[44-55]
- apps/web/src/lib/stores/aiConfig.svelte.ts[19-33]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| while let Some(event) = scan_rx.recv().await { | ||
| // Cache event for late subscribers | ||
| all_events.push(event.clone()); | ||
|
|
||
| // Broadcast to any connected SSE subscribers | ||
| let _ = tx.send(event.clone()); | ||
|
|
||
| // Process specific events for DB persistence | ||
| match event { | ||
| ScanEvent::Finding(mut finding) => { | ||
| let sev_str = finding.severity.to_string().to_lowercase(); | ||
| *severity_counts.entry(sev_str).or_insert(0) += 1; | ||
|
|
||
| // Enrich from local DB | ||
| if let Some(cve_id) = &finding.cve_id { | ||
| if let Ok(Some(row)) = sqlx::query("SELECT title, description FROM vulnerabilities WHERE cve_id = $1") | ||
| .bind(cve_id) | ||
| .fetch_optional(&state_task.db) | ||
| .await | ||
| { | ||
| use sqlx::Row; | ||
| finding.title = row.get("title"); | ||
| } | ||
| } |
There was a problem hiding this comment.
13. Broadcasts pre-enrichment findings 🐞 Bug ≡ Correctness
The server broadcasts ScanEvent::Finding to SSE subscribers before enriching the finding from the vulnerabilities DB, so clients and cached replays never see the enriched title/description.
Agent Prompt
### Issue description
SSE consumers receive stale/un-enriched findings because the broadcast happens before DB enrichment.
### Issue Context
The server enriches the finding title (and selects description) from `vulnerabilities`, but doesn’t broadcast the updated value.
### Fix Focus Areas
- crates/server/src/main.rs[176-222]
### Implementation notes
- Move broadcasting to after enrichment for `ScanEvent::Finding`, e.g.:
- On `Finding(f)`: enrich `f`, then send `ScanEvent::Finding(f.clone())`.
- For non-finding events, keep current broadcast behavior.
- If you want cached replay to reflect enrichment, cache the enriched event, not the original.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
Actionable comments posted: 11
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/routes/scan/+page.svelte (1)
1-296: 🛠️ Refactor suggestion | 🟠 MajorComponent exceeds 200 line limit.
Per coding guidelines, component files should be max 200 lines. This file is ~309 lines. Consider extracting the finding card into a separate
FindingCard.sveltecomponent.As per coding guidelines: "Component files max 200 lines — split into smaller components if larger".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/routes/scan/`+page.svelte around lines 1 - 296, This Svelte page is over the 200-line limit; extract the repeated findings rendering into a new FindingCard.svelte component: move the entire markup inside the {`#each` findings as finding ...} block (the container showing severity badge, title, description, technical analysis, vulnerable_code and fixed_code sections) into FindingCard.svelte as a component that accepts a single prop (e.g. export let finding), and emit the same classes and structure; in the page replace the {`#each` findings as finding ...} block with {`#each` findings as finding}<FindingCard {finding} />{/each} and import FindingCard at top; also move or export the getSeverityColor helper (or accept a computed class prop) so the badge styling still works (either import getSeverityColor from a small util or compute class inside FindingCard) and ensure any references to finding.id, finding.line_start, finding.cve_id, etc., remain unchanged so behavior is preserved.
🟡 Minor comments (15)
ISSUE_DRAFT.md-9-9 (1)
9-9:⚠️ Potential issue | 🟡 MinorFix heading level jump at Line 9
The document starts body headings at
###, which triggers MD001 and hurts structure consistency. Use##here (and keep subsequent headings at the same level unless nesting intentionally).Suggested diff
-### Which area does this relate to? +## Which area does this relate to?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ISSUE_DRAFT.md` at line 9, The heading "Which area does this relate to?" uses level ### causing a markdown heading level jump and MD001; change that heading to level ## (i.e., replace "### Which area does this relate to?" with "## Which area does this relate to?") and ensure subsequent section headings remain consistent (keep other headings at the same level unless intentionally nested) so the document structure passes MD001 checks.ISSUE_DRAFT.md-1-7 (1)
1-7:⚠️ Potential issue | 🟡 MinorClarify template activation intent
The filename
ISSUE_DRAFT.mdindicates this is a draft. If this is meant to become an active GitHub issue template, it should be renamed (remove "DRAFT") and moved to.github/ISSUE_TEMPLATE/. If it remains a draft, the current location is appropriate.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ISSUE_DRAFT.md` around lines 1 - 7, The ISSUE_DRAFT.md file name signals a draft; decide whether this is an active issue template or a draft and act accordingly: if you intend it to be an active GitHub issue template, rename the file to remove "DRAFT" (e.g., ISSUE.md or a descriptive template name) and move it into .github/ISSUE_TEMPLATE/ so GitHub recognizes it as an issue template; if it should remain a draft, keep the current filename/location and add a short note in the file (or in its frontmatter) indicating it's a draft to avoid confusion.apps/web/src/routes/settings/ai/+page.svelte-15-15 (1)
15-15:⚠️ Potential issue | 🟡 MinorTimer not cleaned up on component unmount.
successTimeris set but never cleared when the component is destroyed, which could cause a memory leak or unexpected state updates.🔧 Proposed fix using $effect for cleanup
<script lang="ts"> import { fetchAiModels } from "$lib/api"; import { aiConfig } from "$lib/stores/aiConfig.svelte"; + import { onDestroy } from "svelte"; // ... other state ... let saveSuccess = $state(false); let successTimer: ReturnType<typeof setTimeout>; + onDestroy(() => { + clearTimeout(successTimer); + });Also applies to: 53-54
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/routes/settings/ai/`+page.svelte at line 15, The declared successTimer (let successTimer: ReturnType<typeof setTimeout>) is never cleared on component unmount or before creating a new timer, which can leak or trigger state updates after destroy; update the component to clear the timer by calling clearTimeout(successTimer) in an onDestroy handler and also clear any existing successTimer immediately before assigning a new setTimeout (e.g., in the success path where successTimer is set) so you don't leave multiple active timers; ensure the declaration of successTimer remains and use the Svelte onDestroy lifecycle (or a $: reactive cleanup) to perform the clearTimeout.apps/web/src/routes/settings/ai/+page.svelte-37-38 (1)
37-38:⚠️ Potential issue | 🟡 MinorReplace
anywithunknownfor error handling.As per coding guidelines: "TypeScript strict mode is ON — no
any, no@ts-ignore".🔧 Proposed fix
- } catch (err: any) { - error = err.message || "Failed to fetch models. Check your API key and connection."; + } catch (err: unknown) { + error = err instanceof Error ? err.message : "Failed to fetch models. Check your API key and connection.";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/routes/settings/ai/`+page.svelte around lines 37 - 38, Replace the catch parameter type from any to unknown in the catch block and safely extract the message: change "catch (err: any)" to "catch (err: unknown)" and inside the block normalize the error (e.g., use "err instanceof Error ? err.message : String(err)") before assigning to the local error variable; update the catch in the +page.svelte scope where "error = err.message || 'Failed to fetch models...'" is set so you never access err.message on an unknown type.apps/web/src/routes/settings/ai/+page.svelte-156-166 (1)
156-166:⚠️ Potential issue | 🟡 MinorAdd key to available models
{#each}block.Use the model name
mas the key since model IDs are unique.🔧 Proposed fix
- {`#each` availableModels as m} + {`#each` availableModels as m (m)} <button🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/routes/settings/ai/`+page.svelte around lines 156 - 166, The {`#each` availableModels as m} block is missing a key which can cause rendering/rehydration bugs; update the Svelte each block to use the unique model name as the key (use m) so it becomes {`#each` availableModels as m (m)} and keep the rest of the rendering logic (button, selectedModel comparison, classes, and inner conditional) unchanged.apps/web/src/routes/history/+page.svelte-14-16 (1)
14-16:⚠️ Potential issue | 🟡 MinorReplace
anywith a specific error type.Using
anyviolates TypeScript strict mode. Useunknownand narrow the type safely.As per coding guidelines: "TypeScript strict mode is ON — no
any, no@ts-ignore".🔧 Proposed fix
- } catch (err: any) { - console.error("Failed to load history", err); - error = err.message || "An unexpected error occurred while loading your history."; + } catch (err: unknown) { + console.error("Failed to load history", err); + error = err instanceof Error ? err.message : "An unexpected error occurred while loading your history.";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/routes/history/`+page.svelte around lines 14 - 16, The catch clause currently types the caught value as `any` (catch (err: any)); change it to `unknown` and then narrow it before using it: update the catch to `catch (err: unknown)` and set the `error` variable to `err instanceof Error ? err.message : String(err) || "An unexpected error occurred while loading your history."`, and update the console.error call to log the narrowed value (or the original err) so TypeScript strict mode is satisfied and runtime-safe checks are performed in this +page.svelte error handling block.apps/web/src/routes/history/+page.svelte-117-124 (1)
117-124:⚠️ Potential issue | 🟡 MinorAdd key to severity counts
{#each}block.Use
severityas the key since it's unique within each scan's severity counts.🔧 Proposed fix
- {`#each` Object.entries(scan.severity_counts) as [severity, count]} + {`#each` Object.entries(scan.severity_counts) as [severity, count] (severity)} {`#if` count > 0}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/routes/history/`+page.svelte around lines 117 - 124, The each block iterating over Object.entries(scan.severity_counts) lacks a key which can cause diffing/rendering issues; update the Svelte {`#each`} to provide a key using the severity value (e.g., {`#each` Object.entries(scan.severity_counts) as [severity, count] (severity)}) so each item in the loop is keyed, referencing the scan.severity_counts iteration and getSeverityColor(severity) usage to locate the block.apps/web/src/routes/settings/ai/+page.svelte-89-97 (1)
89-97:⚠️ Potential issue | 🟡 MinorAdd key to providers
{#each}block.Use
p.idas the unique key for provider buttons.🔧 Proposed fix
- {`#each` providers as p} + {`#each` providers as p (p.id)} <button🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/routes/settings/ai/`+page.svelte around lines 89 - 97, The {`#each`} block rendering provider buttons lacks a unique key which can cause incorrect DOM updates; update the Svelte each block for providers to use p.id as the key (i.e., change the iterator from "{`#each` providers as p}" to include the key "(p.id)"), leaving the button contents and click handler (setting provider, availableModels, selectedModel, error) unchanged so Svelte can track list items reliably.apps/web/src/routes/settings/cve/+page.svelte-17-19 (1)
17-19:⚠️ Potential issue | 🟡 MinorReplace
anywithunknownfor error handling.As per coding guidelines: "TypeScript strict mode is ON — no
any, no@ts-ignore".🔧 Proposed fix
- } catch (error: any) { - lastSyncStatus = "error"; - statusMessage = error.message || "Failed to synchronize vulnerability databases."; + } catch (error: unknown) { + lastSyncStatus = "error"; + statusMessage = error instanceof Error ? error.message : "Failed to synchronize vulnerability databases.";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/routes/settings/cve/`+page.svelte around lines 17 - 19, Change the catch parameter from any to unknown in the try/catch around the sync logic (the catch currently using "error: any"), then narrow the unknown before reading .message: check if error is an instance of Error and set statusMessage = error.message, otherwise set statusMessage = String(error) or a fallback "Failed to synchronize vulnerability databases."; leave lastSyncStatus = "error" as-is. This removes use of any while preserving safe error-message extraction for the catch block that assigns lastSyncStatus and statusMessage.crates/scanner/src/engine.rs-36-43 (1)
36-43:⚠️ Potential issue | 🟡 MinorGuard against division by zero when engines list is empty.
If
config.enginesis empty,total_engineswill be 0, causing a division by zero at line 39 when calculating the progress percentage. While0 / 0in floating-point yieldsNaN(not a panic), it produces an invalid progress value.🛡️ Proposed fix
pub async fn run_stream( config: &ScanConfig, tx: UnboundedSender<ScanEvent>, ) -> anyhow::Result<Vec<RawFinding>> { let mut all_findings = Vec::new(); let total_engines = config.engines.len(); + + if total_engines == 0 { + let _ = tx.send(ScanEvent::Progress { + percentage: 100, + message: "No engines configured.".to_string(), + }); + return Ok(all_findings); + } for (i, engine) in config.engines.iter().enumerate() {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/scanner/src/engine.rs` around lines 36 - 43, The code computes progress using total_engines = config.engines.len() and divides by it, which yields invalid progress when config.engines is empty; add a guard before the loop to handle total_engines == 0 (or check inside the loop) and set a sensible progress value (e.g., 100 or 0) and message so you never divide by zero: update the logic around total_engines / the progress calculation used when building the ScanEvent::Progress sent via tx.send so that when config.engines.is_empty() you send a deterministic progress and message (e.g., percentage: 100, message: "No engines to run") instead of performing the division.apps/web/src/routes/history/+page.svelte-70-72 (1)
70-72:⚠️ Potential issue | 🟡 MinorAdd key to
{#each}block for skeleton items.The linter flags missing keys. For skeleton placeholders, use the index as a key.
🔧 Proposed fix
- {`#each` Array(5) as _} + {`#each` Array(5) as _, i (i)} <div class="h-24 glass rounded-2xl animate-pulse border-zinc-800/50"></div> {/each}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/routes/history/`+page.svelte around lines 70 - 72, The {`#each`} block rendering skeleton placeholders is missing a key, causing linter warnings; update the block to capture the index (e.g., change "as _" to "as _, i") and supply the index as the key expression (use the index in the each-key clause) so each placeholder has a stable key for Reactivity and linting.extensions/vscode/src/extension.ts-102-142 (1)
102-142:⚠️ Potential issue | 🟡 MinorSSE parsing may fail on multi-event chunks or split chunks.
The SSE parsing splits on
\nand looks for lines starting withdata:. However:
- A single chunk may contain multiple complete SSE events
- An SSE event may be split across chunks (partial
data:line)The current implementation handles case 1 correctly (iterates all lines), but case 2 could cause JSON parse failures for partial data. Consider buffering incomplete lines.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extensions/vscode/src/extension.ts` around lines 102 - 142, The SSE reader can receive partial events across chunks, so change the parsing loop in the reader.read handling to maintain a persistent buffer string (e.g., let sseBuffer = '') that you append decoder.decode(value, { stream: true }) to on each read, then split the buffer by the SSE event delimiter (double newline '\n\n' or '\r\n\r\n') to extract only complete events for JSON.parse and leave any trailing partial event in sseBuffer for the next iteration; update the current switch handling (cases 'progress','finding','complete','error') to consume only fully parsed events and keep using findings, updateDiagnostics(document, findings), and sidebarProvider.postMessage as before.extensions/vscode/src/extension.ts-100-100 (1)
100-100:⚠️ Potential issue | 🟡 MinorUse
constinstead ofletfor the findings array.The array reference is never reassigned; only its contents are modified via
push. The linter correctly flags this.- let findings: Finding[] = []; + const findings: Finding[] = [];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extensions/vscode/src/extension.ts` at line 100, Change the variable declaration for the findings array from a mutable binding to an immutable binding: replace the "let findings: Finding[] = [];" declaration with a const binding ("const findings: Finding[] = [];") in extension.ts so the reference cannot be reassigned while still allowing pushes to the array; update any code that might reassign findings (if present) to instead mutate the array or use a new variable name.apps/web/src/routes/scan/+page.svelte-2-3 (1)
2-3:⚠️ Potential issue | 🟡 MinorRemove unused imports.
Both
onMountandscanare imported but never used, as flagged by the linter. The scan functionality now uses directfetchcalls.<script lang="ts"> - import { onMount } from "svelte"; - import { scan, type Finding } from "$lib/api"; + import { type Finding } from "$lib/api"; import { aiConfig } from "$lib/stores/aiConfig.svelte";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/routes/scan/`+page.svelte around lines 2 - 3, Remove the dead imports to satisfy the linter: delete the unused imports onMount and scan from the import line (currently importing onMount from "svelte" and scan from "$lib/api"), leaving only the used symbol(s) such as type Finding if still needed; update the import statement in +page.svelte to import only the actually used exports (e.g., "import type { Finding } from '$lib/api'") or remove the entire import line if nothing from those modules is referenced.crates/server/src/main.rs-248-256 (1)
248-256:⚠️ Potential issue | 🟡 MinorRace condition: late subscribers may miss events if they connect between scan completion and cache population.
There's a brief window between when
state_task.scans.remove(&scan_id)is called andstate_task.results.insert(scan_id, all_events)where a subscriber connecting viasubscribe_to_scanwould get a 404 "Scan not found" error because the scan_id exists in neither map.Suggested fix: insert to results before removing from scans
- // Move results to cache so late SSE subscribers can replay them - state_task.scans.remove(&scan_id); - state_task.results.insert(scan_id, all_events); + // Move results to cache so late SSE subscribers can replay them + // Insert to results first to avoid race condition window + state_task.results.insert(scan_id, all_events); + state_task.scans.remove(&scan_id);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/server/src/main.rs` around lines 248 - 256, The race occurs because state_task.scans.remove(&scan_id) runs before state_task.results.insert(scan_id, all_events), so a subscriber calling subscribe_to_scan can see neither map and get a 404; to fix, reverse the operations: insert the completed events into state_task.results with state_task.results.insert(scan_id, all_events) first, then remove from state_task.scans with state_task.scans.remove(&scan_id), ensuring subscribe_to_scan will find results for late subscribers.
🧹 Nitpick comments (23)
migrations/202604040001_create_vulnerabilities_table.sql (2)
19-20: Redundant index oncve_id.The
UNIQUEconstraint oncve_id(line 7) automatically creates an index. The explicitidx_vulnerabilities_cve_idindex is redundant and wastes storage.♻️ Remove redundant index
-- Index for fast lookup by CVE ID -CREATE INDEX IF NOT EXISTS idx_vulnerabilities_cve_id ON vulnerabilities(cve_id); +-- Note: cve_id already has an implicit index from the UNIQUE constraint🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@migrations/202604040001_create_vulnerabilities_table.sql` around lines 19 - 20, The CREATE INDEX statement for idx_vulnerabilities_cve_id is redundant because the UNIQUE constraint on vulnerabilities.cve_id already creates an index; remove the line "CREATE INDEX IF NOT EXISTS idx_vulnerabilities_cve_id ON vulnerabilities(cve_id);" (or drop the redundant index) to avoid duplicate indexes and wasted storage while keeping the UNIQUE constraint on cve_id.
12-13: Default timestamps may mask missing source data.
published_atandlast_modified_atdefault toCURRENT_TIMESTAMP, but these fields should reflect dates from the NVD/OSV data source. Consider making them nullable without defaults, or using a sentinel value, so missing source data is explicit rather than silently defaulting to insert time.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@migrations/202604040001_create_vulnerabilities_table.sql` around lines 12 - 13, The published_at and last_modified_at columns currently default to CURRENT_TIMESTAMP which hides missing source dates; update the migration so published_at and last_modified_at do not use DEFAULT CURRENT_TIMESTAMP and are nullable (or use a clear sentinel) so that absence of NVD/OSV source dates is explicit; locate the column definitions for published_at and last_modified_at in the migration and remove the DEFAULT CURRENT_TIMESTAMP clauses and ensure the columns allow NULL (or set a documented sentinel value) and adjust any insertion code that relied on the defaults to explicitly supply timestamps when available.apps/web/package.json (1)
16-17: Consider removing unused@sveltejs/adapter-auto.Since
svelte.config.jsnow imports@sveltejs/adapter-node, the@sveltejs/adapter-autodependency on line 16 is no longer used and could be removed to reduce dependencies.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/package.json` around lines 16 - 17, Remove the unused dependency entry "@sveltejs/adapter-auto" from apps/web's package.json (it is redundant because svelte.config.js imports "@sveltejs/adapter-node"); delete the "@sveltejs/adapter-auto" line in package.json and then run your package manager's install/lockfile update (npm install / npm prune or yarn install) to update lockfiles and ensure the dependency is removed from the lockfile.crates/cli/src/main.rs (2)
127-127: Move imports to the top of the file.The
serdeimport at line 127 should be grouped with other imports at the top of the file for better code organization.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/cli/src/main.rs` at line 127, Move the stray import `use serde::{Deserialize, Serialize};` from line 127 into the file's top import block where other `use ...` statements are declared; cut the line and place it with the other imports (keeping project import ordering/alphabetization conventions and avoiding duplicate serde imports) so `Deserialize` and `Serialize` are imported alongside the rest of the module-level `use` declarations.
157-160:HOMEenvironment variable is not cross-platform.
HOMEis Unix-specific and won't exist on Windows, causing fallback to"."which writes config to the current directory. Consider using thedirscrate for cross-platform config paths.♻️ Example using dirs crate
fn get_path() -> std::path::PathBuf { - let home = std::env::var("HOME").unwrap_or_else(|_| ".".to_string()); - std::path::PathBuf::from(home).join(".config/zenvra/config.json") + dirs::config_dir() + .unwrap_or_else(|| std::path::PathBuf::from(".")) + .join("zenvra/config.json") }This would require adding
dirs = "5"toCargo.toml.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/cli/src/main.rs` around lines 157 - 160, get_path currently reads the Unix-only HOME env var and falls back to "."; change get_path to use a cross-platform config directory (e.g., dirs::config_dir()) to locate the user config folder, fall back to current_dir() only if config_dir() is None, and then join the application subpath (e.g., "zenvra/config.json") so the file is placed under the OS-specific config location; add the dirs = "5" dependency to Cargo.toml and update the PathBuf construction in get_path accordingly.crates/scanner/src/ai/anthropic.rs (1)
64-65: Add doc comment explaining the fallback behavior (if retained).If the fallback is intentional (perhaps Anthropic's
/v1/modelsendpoint is unreliable), the doc comment should warn callers that the returned list may be a static fallback rather than live API data.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/scanner/src/ai/anthropic.rs` around lines 64 - 65, Update the doc comment for the async function list_models(api_key: &str, endpoint: Option<&str>) to document the fallback behavior: state whether the function queries Anthropic’s /v1/models endpoint and, if it uses or may return a static fallback when the API is unavailable, explicitly warn callers that results may not be live API data; include when the fallback is used and any implications for caching or staleness so callers can handle this behavior appropriately.apps/web/Dockerfile (2)
21-23: Consider copying only production dependencies to reduce image size.Copying the entire
node_modulesincludes dev dependencies. For a smaller production image, consider runningpnpm install --prodin a separate step or usingpnpm deployto create a production-only bundle.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/Dockerfile` around lines 21 - 23, The Dockerfile currently copies the entire node_modules directory from the builder stage (COPY --from=builder /app/node_modules ./node_modules), which pulls dev dependencies into the final image; change the build pipeline to install or produce production-only dependencies and copy those instead—e.g., in the builder stage run a production install (pnpm install --prod or pnpm deploy to create a production bundle) and then update the final stage to COPY the production node_modules or the deploy output (replace COPY --from=builder /app/node_modules ./node_modules with copying only the production install output or build artifact) so the final image excludes devDependencies.
4-4: Pin pnpm version to matchpackage.json.The
package.jsonspecifiespnpm@10.24.0butnpm install -g pnpminstalls the latest version, which could cause inconsistent builds.♻️ Proposed fix
-RUN npm install -g pnpm +RUN npm install -g pnpm@10.24.0🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/Dockerfile` at line 4, The Dockerfile currently runs "RUN npm install -g pnpm" which installs the latest pnpm and can diverge from package.json; pin pnpm to the package.json version by replacing that line with a pinned install (e.g. use an ARG like "ARG PNPM_VERSION=10.24.0" and "RUN npm install -g pnpm@$PNPM_VERSION") or hardcode "RUN npm install -g pnpm@10.24.0" so the container uses the same pnpm version as package.json.crates/scanner/src/ai/openai.rs (1)
73-101: LGTM — consider extracting duplicated endpoint normalization.The
list_modelsfunction correctly handles both/v1-suffixed and bare endpoints. However, the URL normalization logic (trim trailing slash, check for/v1suffix) is duplicated betweenlist_models(lines 76-82) andOpenAiProvider::call(lines 115-120).♻️ Optional: Extract shared helper
fn normalize_endpoint(endpoint: &str, path: &str) -> String { let ep = endpoint.trim_end_matches('/'); if ep.ends_with("/v1") { format!("{}/{}", ep, path) } else { format!("{}/v1/{}", ep, path) } }Then use:
- let url = if ep.ends_with("/v1") { - format!("{}/models", ep) - } else { - format!("{}/v1/models", ep) - }; + let url = normalize_endpoint(ep, "models");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/scanner/src/ai/openai.rs` around lines 73 - 101, The endpoint normalization logic is duplicated between list_models and OpenAiProvider::call; extract a small helper (e.g., normalize_endpoint or build_endpoint) that accepts the raw endpoint &str and a path segment (like "models" or "chat/completions") and returns the correctly normalized URL by trimming trailing slashes and ensuring a single /v1 is present; then replace the inline logic in list_models and OpenAiProvider::call to call this helper to build the final URL, keeping existing error handling and header usage unchanged.apps/web/src/routes/settings/ai/+page.svelte (1)
190-221: Component exceeds 200-line limit.At 224 lines, this component slightly exceeds the guideline limit. The info panel (lines 190-221) could be extracted into a separate component.
As per coding guidelines: "Component files max 200 lines — split into smaller components if larger".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/routes/settings/ai/`+page.svelte around lines 190 - 221, The component is over the 200-line limit; extract the info panel and the active policies panel into their own Svelte components (e.g., BYOKInfo.svelte for the "Bring Your Own Key" block and ActivePolicies.svelte for the "Active Policies" block), move the corresponding markup into those files, export them as default Svelte components, then import and use <BYOKInfo/> and <ActivePolicies/> inside +page.svelte to replace the original divs; there are no props required so just replace the markup with the new component tags and ensure any local classes or assets referenced remain available.apps/web/src/routes/history/+page.svelte (1)
132-134: Navigation button has no action.The arrow button suggests navigation to scan details but lacks an
onclickhandler orhref. If this is intentional for a future feature, consider adding a TODO comment or disabling the button.Do you want me to help implement navigation to a scan details page, or should this be tracked as a separate issue?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/routes/history/`+page.svelte around lines 132 - 134, The navigation arrow button in +page.svelte has no action; add a navigation handler (or make it an anchor) so the button goes to the scan details route (e.g., `/history/{scan.id}`) when clicked: either wrap the <button> in an <a href="..."> pointing to the scan detail path or add an on:click that calls SvelteKit's goto from $app/navigation (or dispatches a route change) using the scan's unique id (e.g., scan.id or item.id) referenced in the surrounding loop; if navigation is intentionally deferred, disable the button or add a TODO comment explaining it's pending.crates/server/src/cve_sync/mod.rs (1)
90-124: Consider batching database inserts for better performance.Executing individual
INSERTstatements in a loop for potentially thousands of CVEs is inefficient. Consider using a batch insert or wrapping in a transaction.♻️ Suggested batch approach
// Collect all values first let mut tx = pool.begin().await?; for item in nvd_data.vulnerabilities { // ... same upsert logic but using &mut tx instead of pool ... sqlx::query(/* ... */) .execute(&mut *tx) .await?; } tx.commit().await?;Or use
sqlx'sQueryBuilderfor true batch inserts withUNNESTarrays.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/server/src/cve_sync/mod.rs` around lines 90 - 124, The current loop over nvd_data.vulnerabilities performs one sqlx::query INSERT per CVE which is slow; change to collect rows then perform batched upserts by starting a transaction (let mut tx = pool.begin().await?) and executing the queries against &mut tx, or better use sqlx::QueryBuilder to build a single bulk INSERT ... ON CONFLICT upsert using arrays/UNNEST; update the code that references nvd_data.vulnerabilities and the sqlx::query call to use either tx.execute(...) and commit once (tx.commit().await?) or replace with QueryBuilder to bind all (cve_id, title, description, severity, data_source) tuples and execute in one statement for improved performance.apps/web/src/routes/settings/cve/+page.svelte (1)
46-54: Hardcoded sync statistics.The "Synced CVEs" count (100+) and progress bar are static placeholders. Consider fetching actual statistics from the backend for a more informative UI.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/routes/settings/cve/`+page.svelte around lines 46 - 54, The UI currently shows hardcoded sync stats ("Synced CVEs" 100+ and progress bar w-2/5); replace these placeholders by fetching real stats from the backend (e.g., implement a +page.server.ts load() or client-side onMount() that calls your CVE stats API endpoint) and bind the returned values to the displayed count span and to a reactive width/percentage used instead of the static "w-2/5" class; update the JSX in +page.svelte to render the dynamic count and compute the progress bar width/style from the fetched percent so the UI reflects real backend data.crates/server/src/main.rs (3)
185-199: Finding enrichment overwrites title but doesn't use fetched description.The code fetches both
titleanddescriptionfrom the vulnerabilities table but only usestitle. The description might be valuable for enriching findings. Also, the query selectsdescriptionbut doesn't use it.Suggested fix to use description
if let Ok(Some(row)) = sqlx::query("SELECT title, description FROM vulnerabilities WHERE cve_id = $1") .bind(cve_id) .fetch_optional(&state_task.db) .await { use sqlx::Row; finding.title = row.get("title"); + if let Ok(desc) = row.try_get::<Option<String>, _>("description") { + if finding.description.is_none() { + finding.description = desc; + } + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/server/src/main.rs` around lines 185 - 199, ScanEvent::Finding enrichment currently assigns only finding.title from the DB row even though the query selects description; update the enrichment block handling the sqlx::query for vulnerabilities (inside ScanEvent::Finding where cve_id is bound) to also assign finding.description = row.get("description") (and ensure you keep finding.title = row.get("title") too), using the same sqlx::Row import to retrieve both fields so the fetched description is applied to the finding.
99-102: CORS is overly permissive withallow_origin(Any).Using
Anyfor allowed origins in CORS exposes the API to cross-origin requests from any domain. For a security scanner API, consider restricting to known dashboard origins in production.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/server/src/main.rs` around lines 99 - 102, The CORS configuration is overly permissive: replace the use of allow_origin(Any) on the CorsLayer created with CorsLayer::new() so production only permits trusted dashboard origins; update the CorsLayer instantiation (the code using CorsLayer::new(), .allow_origin(Any), .allow_methods(Any), .allow_headers(Any)) to use a restricted Origin or Origin list (e.g., a list built from configuration/env) instead of Any, and ensure the new origin list is used when building the middleware so only the allowed dashboard origins are accepted while retaining .allow_methods(Any) and .allow_headers(Any) as needed.
270-295: SSE events are sent without an explicit event name.All
ScanEvents are serialized via.json_data()without calling.event()to set the SSE event type. This means all events arrive as the default "message" event type.The Svelte page handles this correctly by using
onmessageand parsing the JSON to checkdata.type. The VS Code extension also parses the JSONtypefield fromdata:lines. Both clients work correctly with this format.However, for cleaner SSE semantics and easier client-side handling, consider extracting the event type:
Optional improvement
fn scan_event_to_sse(event: &ScanEvent) -> Event { let event_type = match event { ScanEvent::Progress { .. } => "progress", ScanEvent::Finding(_) => "finding", ScanEvent::Complete => "complete", ScanEvent::Error(_) => "error", }; Event::default() .event(event_type) .json_data(event) .unwrap_or_else(|_| Event::default()) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/server/src/main.rs` around lines 270 - 295, The SSEs are being sent without explicit event names (only Event::json_data is used), so add an event name based on ScanEvent variants and use it when building the Event; implement a helper like scan_event_to_sse(&ScanEvent) -> Event that matches on ScanEvent (e.g., Progress -> "progress", Finding -> "finding", Complete -> "complete", Error -> "error") and returns Event::default().event(event_type).json_data(event).unwrap_or_else(|_| Event::default()), then replace the anonymous closures in both branches (the cached branch mapping and the BroadcastStream mapping where .json_data(event) is used) to call this helper so all SSEs include .event(...) as well as .json_data(...).extensions/vscode/src/sidebarProvider.ts (3)
46-50: Type themessageparameter instead of usingany.The static analysis correctly flags the
anytype. Define a union type for the expected message shapes to improve type safety.Suggested fix
+type SidebarMessage = + | { type: 'progress'; data: { percentage: number; message: string } } + | { type: 'finding'; data: { severity: string; title: string } } + | { type: 'complete' }; - public postMessage(message: any) { + public postMessage(message: SidebarMessage) { if (this._view) { this._view.webview.postMessage(message); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extensions/vscode/src/sidebarProvider.ts` around lines 46 - 50, The postMessage method uses an untyped any for its message parameter; replace it with a specific union type that models all expected message shapes (e.g., an interface/union like SidebarMessage = | { type: 'init' } | { type: 'update', payload: UpdatePayload } | ...), then update the method signature public postMessage(message: SidebarMessage) and ensure callers produce values conforming to SidebarMessage; keep the existing null-check on this._view and continue to call this._view.webview.postMessage(message) so the runtime behavior is unchanged.
52-52: Prefix unused parameter with underscore.The
webviewparameter is unused. Prefix it with_to satisfy the linter rule requiring unused args to match/^_/u.Suggested fix
- private _getHtmlForWebview(webview: vscode.Webview) { + private _getHtmlForWebview(_webview: vscode.Webview) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extensions/vscode/src/sidebarProvider.ts` at line 52, The parameter `webview` in the `_getHtmlForWebview` method is unused and must be prefixed with an underscore to satisfy the linter; rename the parameter to `_webview` in the `private _getHtmlForWebview(webview: vscode.Webview)` signature (i.e., `private _getHtmlForWebview(_webview: vscode.Webview)`) and ensure there are no remaining references to `webview` inside the method (update any occurrences if present).
53-218: Consider adding a Content Security Policy (CSP) to the webview HTML.The webview enables scripts (
enableScripts: true) but lacks a CSP meta tag. WhilelocalResourceRootsrestricts resource loading, a CSP adds defense-in-depth against XSS within the webview.Suggested CSP addition
<head> <meta charset="UTF-8"> + <meta http-equiv="Content-Security-Policy" content="default-src 'none'; style-src 'unsafe-inline'; script-src 'unsafe-inline';"> <meta name="viewport" content="width=device-width, initial-scale=1.0">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extensions/vscode/src/sidebarProvider.ts` around lines 53 - 218, The webview HTML lacks a Content Security Policy which is risky since scripts are enabled (enableScripts: true) and the UI uses acquireVsCodeApi() and inline scripts; generate a cryptographic nonce in the sidebar provider (where the HTML is produced), pass that nonce into the HTML template, add a meta CSP tag like default-src 'none'; script-src 'nonce-<nonce-value>'; style-src 'unsafe-inline' (or 'nonce-<nonce-value>' if you prefer nonces for styles); and ensure the inline <script> tag and any injected resources use that same nonce so only your trusted script can run (update the template-returning function that emits the HTML and the place that calls enableScripts).extensions/vscode/src/extension.ts (1)
98-98: Replaceanytypes with proper types.Line 98 uses
as anyfor the ReadableStream reader, and line 143 usesanyfor the error. These can be properly typed.Suggested fixes
- const reader = (body as any).getReader(); + const reader = body.getReader();For the catch block, use
unknownand narrow:- } catch (err: any) { - vscode.window.showErrorMessage(`Zenvra Scan Failed: ${err.message}`); + } catch (err: unknown) { + const message = err instanceof Error ? err.message : String(err); + vscode.window.showErrorMessage(`Zenvra Scan Failed: ${message}`);Also applies to: 143-143
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extensions/vscode/src/extension.ts` at line 98, Replace the ad-hoc any usages by correctly typing the stream reader and caught errors: when creating the reader from (body as any).getReader(), cast body to ReadableStream<Uint8Array> (or appropriate chunk type) and declare reader with the type ReadableStreamDefaultReader<Uint8Array> (refer to the reader variable assignment and getReader() call); in the catch block replace error: any with error: unknown and then narrow it (e.g., check instanceof Error or typeof message checks) before logging or using error.message (refer to the catch where the error is handled on line ~143) so all uses are type-safe and no any remains.crates/scanner/src/lib.rs (1)
94-111: Spawned task errors may be silently lost.If
scan_streamreturns an error after sendingScanEvent::Error, the error propagates via the channel. However, if the spawned task panics or ifscan_streamreturnsErrbefore sending any event, the caller receives no events and exits cleanly with an emptyVec. Consider joining the task or ensuring errors are always sent to the channel.Additionally,
ScanEvent::Progressevents are silently discarded inscan(). If this is intentional (sincescanis batch mode), the code is correct but a comment would clarify intent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/scanner/src/lib.rs` around lines 94 - 111, The scan function spawns scan_stream into a detached task so panics or early Errs can be lost and ScanEvent::Progress is ignored without explanation; update scan to keep the JoinHandle (from tokio::spawn) and await/join it after draining rx so any task panic or returned Err is observed and converted into an Err from scan (handle tokio::task::JoinError and propagate underlying scan_stream error), and modify the spawn block to ensure any early errors are sent into tx (or propagate them via the JoinHandle) so they aren't dropped; also either handle or add a brief comment explaining that ScanEvent::Progress is intentionally ignored in scan() (batch mode) and reference the functions/variants scan, scan_stream, ScanEvent::Error and ScanEvent::Progress so reviewers can find the changes.apps/web/src/lib/api.ts (1)
63-67: Consider adding Zod schemas for API response validation.Per coding guidelines, "use zod for all form and API input validation." The API responses are cast directly via
res.json()without runtime validation. If the server returns malformed data, TypeScript won't catch it at runtime.Example with Zod validation
import { z } from 'zod'; const ScanHistorySchema = z.object({ id: z.string(), language: z.string(), target_name: z.string().optional(), findings_count: z.number(), severity_counts: z.record(z.string(), z.number()), created_at: z.string(), }); export async function getHistory(): Promise<ScanHistory[]> { const res = await fetch(`${BASE_URL}/api/v1/history`); if (!res.ok) throw new Error('Failed to fetch scan history'); const data = await res.json(); return z.array(ScanHistorySchema).parse(data); }As per coding guidelines: "Use zod for all form and API input validation".
Also applies to: 72-76, 81-96
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/lib/api.ts` around lines 63 - 67, The getHistory function returns res.json() without runtime validation; add a Zod schema (e.g., ScanHistorySchema using z.object(...) matching the ScanHistory type) and import z from 'zod', then parse the response with z.array(ScanHistorySchema).parse(data) before returning so malformed responses throw a validation error; apply the same pattern to the other API functions in this file that call res.json() (referenced by their function names such as getHistory and the other fetch helpers) and include a clear error message when the fetch fails or parsing throws.extensions/vscode/package.json (1)
116-120: Consider markingaiApiKeyas a secret setting.The AI API key setting stores sensitive credentials. While VS Code settings are stored locally, consider using
"scope": "machine"or documenting that users should preferzenvra.setApiTokencommand which uses the secrets API.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extensions/vscode/package.json` around lines 116 - 120, The setting "zenvra.aiApiKey" stores sensitive credentials; update its contribution in package.json to set "scope": "machine" to avoid per-workspace storage and adjust the description to advise users to prefer the zenvra.setApiToken command (which uses VS Code SecretStorage) for storing secrets; locate the "zenvra.aiApiKey" entry in the contributes.configuration block and add the "scope": "machine" property and a short note in "description" about using zenvra.setApiToken/SecretStorage.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c7b73b40-eb52-4ef6-87b4-001f77beaf85
⛔ Files ignored due to path filters (3)
Cargo.lockis excluded by!**/*.lockapps/web/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlextensions/vscode/assets/icon.pngis excluded by!**/*.png
📒 Files selected for processing (34)
.github/workflows/release.yml.gitignoreCargo.tomlISSUE_DRAFT.mdapps/web/Dockerfileapps/web/package.jsonapps/web/src/lib/api.tsapps/web/src/lib/stores/aiConfig.svelte.tsapps/web/src/routes/+layout.svelteapps/web/src/routes/history/+page.svelteapps/web/src/routes/scan/+page.svelteapps/web/src/routes/settings/ai/+page.svelteapps/web/src/routes/settings/cve/+page.svelteapps/web/svelte.config.jscrates/cli/src/main.rscrates/scanner/src/ai/anthropic.rscrates/scanner/src/ai/google.rscrates/scanner/src/ai/mod.rscrates/scanner/src/ai/openai.rscrates/scanner/src/engine.rscrates/scanner/src/finding.rscrates/scanner/src/lib.rscrates/server/Cargo.tomlcrates/server/Dockerfilecrates/server/src/cve_sync/mod.rscrates/server/src/main.rsdocker-compose.ymlextensions/vscode/package.jsonextensions/vscode/src/extension.tsextensions/vscode/src/sidebarProvider.tsextensions/vscode/src/types.tsextensions/vscode/zenvra-0.1.1.vsixmigrations/202604040001_create_vulnerabilities_table.sqlmigrations/202604040002_create_scans_table.sql
| # Stage 1: Build | ||
| FROM node:20-slim AS builder | ||
| WORKDIR /app | ||
| RUN npm install -g pnpm |
There was a problem hiding this comment.
Security: Container runs as root user.
The static analysis tool correctly flagged that no non-root user is specified. Running as root in production containers is a security risk. Add a non-root user in the runtime stage.
🔒 Proposed fix to add non-root user
# Stage 2: Runtime
FROM node:20-slim
WORKDIR /app
+RUN addgroup --system --gid 1001 nodejs && \
+ adduser --system --uid 1001 --ingroup nodejs nodeuser
COPY --from=builder /app/build ./build
COPY --from=builder /app/package.json ./package.json
COPY --from=builder /app/node_modules ./node_modules
+RUN chown -R nodeuser:nodejs /app
+USER nodeuser
EXPOSE 3000
ENV NODE_ENV=production🧰 Tools
🪛 Trivy (0.69.3)
[error] 1-1: Image user should not be 'root'
Specify at least 1 USER command in Dockerfile with non-root user as argument
Rule: DS-0002
(IaC/Dockerfile)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/Dockerfile` around lines 1 - 4, The Dockerfile runs as root; add a
non-root runtime user and switch to it in the final stage: create a dedicated
user/group (e.g., app or appuser with a stable UID), chown the app workdir and
any runtime-needed files to that user, and add a USER app (or chosen name)
before the image is finalized. Ensure these steps occur in the final/runtime
stage (not the builder) and reference the existing WORKDIR /app and any
artifacts produced by the builder so the non-root user owns /app at runtime.
| if (browser) { | ||
| localStorage.setItem('zenvra_ai_provider', p); | ||
| localStorage.setItem('zenvra_ai_model', m); | ||
| localStorage.setItem('zenvra_ai_api_key', key); | ||
| if (ep) localStorage.setItem('zenvra_ai_endpoint', ep); | ||
| else localStorage.removeItem('zenvra_ai_endpoint'); | ||
| } |
There was a problem hiding this comment.
API keys stored in cleartext localStorage.
The static analysis correctly flags that storing API keys in localStorage exposes them to XSS attacks. While this is a deliberate "Bring Your Own Key" design, the Settings UI (line 213 in +page.svelte) claims "Encrypted Local Storage" which is misleading—the keys are stored in plaintext.
Consider either:
- Removing the "Encrypted Local Storage" claim from the UI
- Actually encrypting the stored values (e.g., using Web Crypto API with a user-derived key)
🧰 Tools
🪛 ast-grep (0.42.0)
[warning] 28-28: Sensitive information detected in localStorage. Storing sensitive data like emails, usernames, or personal information in localStorage exposes it to malicious scripts and XSS attacks. Use secure storage alternatives or avoid storing sensitive data client-side.
Context: localStorage.setItem('zenvra_ai_api_key', key)
Note: [CWE-312] Cleartext Storage of Sensitive Information [REFERENCES]
- https://cwe.mitre.org/data/definitions/312.html
- https://owasp.org/www-community/vulnerabilities/HTML5_Local_Storage_Security
(local-storage-sensitive-information)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/lib/stores/aiConfig.svelte.ts` around lines 26 - 32, The code
stores API keys in cleartext via localStorage.setItem (variables p, m, key, ep
in aiConfig.svelte.ts), which contradicts the Settings UI claim "Encrypted Local
Storage"; either remove that UI claim or implement real encryption: add helper
functions (e.g., encryptValue/decryptValue or setEncryptedItem/getEncryptedItem)
that use the Web Crypto API with a user-derived key (PBKDF2 from a passphrase)
to encrypt values before calling localStorage.setItem and to decrypt on read,
update the write paths that call localStorage.setItem('zenvra_ai_api_key'...)
and localStorage.setItem('zenvra_ai_endpoint'...) to use the encrypted helpers,
and ensure reading paths decrypt and handle missing/legacy plaintext values for
backward compatibility.
| const response = await fetch("http://localhost:8080/api/v1/scan", { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify({ | ||
| code, | ||
| language: "python", | ||
| engines: ["sast", "secrets"], | ||
| ai_config: hasAiConfig ? { | ||
| provider: aiConfig.provider, | ||
| api_key: aiConfig.apiKey, | ||
| model: aiConfig.model, | ||
| endpoint: aiConfig.endpoint || undefined, | ||
| } : undefined | ||
| }) | ||
| }); |
There was a problem hiding this comment.
Hardcoded API URL should use environment variable or api.ts.
The scan page uses hardcoded http://localhost:8080 for the API URL instead of using the BASE_URL from $lib/api.ts. This breaks production deployments and violates the coding guideline that "All fetch calls must go through typed API client functions in apps/web/src/lib/api.ts."
Suggested approach
Consider exporting BASE_URL from api.ts or creating a startScan helper that returns the scan_id, then construct the SSE URL from it:
// In api.ts
export const BASE_URL = (import.meta.env.PUBLIC_API_URL || 'http://localhost:8080').replace(/\/$/, '');
export async function startScan(req: ScanRequest): Promise<string> {
const res = await fetch(`${BASE_URL}/api/v1/scan`, {
method: "POST",
headers: { "Content-Type": "application/json" },
body: JSON.stringify(req)
});
if (!res.ok) throw new Error("Failed to start scan");
const { scan_id } = await res.json();
return scan_id;
}As per coding guidelines: "All fetch calls must go through typed API client functions in apps/web/src/lib/api.ts".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/routes/scan/`+page.svelte around lines 34 - 48, The fetch to
"http://localhost:8080/api/v1/scan" is hardcoded; replace it by calling a typed
API client in apps/web/src/lib/api.ts (export BASE_URL and/or add a
startScan(req: ScanRequest): Promise<string> helper that posts the scan request
and returns scan_id). Update the +page.svelte code to call startScan with the
same payload (code, language, engines, ai_config) and use the returned scan_id
to construct any SSE URLs (using exported BASE_URL if needed); ensure startScan
throws on non-ok responses so the page can handle errors.
| # Stage 2: Runtime | ||
| FROM debian:bookworm-slim | ||
| RUN apt-get update && apt-get install -y libssl3 ca-certificates curl && rm -rf /var/lib/apt/lists/* | ||
| WORKDIR /app | ||
| COPY --from=builder /app/target/release/zenvra-server /usr/local/bin/zenvra-server |
There was a problem hiding this comment.
Runtime container runs as root — add a non-root user.
The runtime stage lacks a USER directive, so the container runs as root. This is a security concern flagged by static analysis (DS-0002). Additionally, apt-get install should use --no-install-recommends to minimize image size and attack surface (DS-0029).
🔒 Proposed fix to run as non-root user and optimize apt-get
# Stage 2: Runtime
FROM debian:bookworm-slim
-RUN apt-get update && apt-get install -y libssl3 ca-certificates curl && rm -rf /var/lib/apt/lists/*
+RUN apt-get update && apt-get install -y --no-install-recommends libssl3 ca-certificates curl && rm -rf /var/lib/apt/lists/*
+RUN useradd --create-home --shell /bin/bash zenvra
WORKDIR /app
COPY --from=builder /app/target/release/zenvra-server /usr/local/bin/zenvra-server
+USER zenvra
EXPOSE 8080
CMD ["zenvra-server"]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Stage 2: Runtime | |
| FROM debian:bookworm-slim | |
| RUN apt-get update && apt-get install -y libssl3 ca-certificates curl && rm -rf /var/lib/apt/lists/* | |
| WORKDIR /app | |
| COPY --from=builder /app/target/release/zenvra-server /usr/local/bin/zenvra-server | |
| # Stage 2: Runtime | |
| FROM debian:bookworm-slim | |
| RUN apt-get update && apt-get install -y --no-install-recommends libssl3 ca-certificates curl && rm -rf /var/lib/apt/lists/* | |
| RUN useradd --create-home --shell /bin/bash zenvra | |
| WORKDIR /app | |
| COPY --from=builder /app/target/release/zenvra-server /usr/local/bin/zenvra-server | |
| USER zenvra |
🧰 Tools
🪛 Trivy (0.69.3)
[error] 19-19: 'apt-get' missing '--no-install-recommends'
'--no-install-recommends' flag is missed: 'apt-get update && apt-get install -y libssl3 ca-certificates curl && rm -rf /var/lib/apt/lists/*'
Rule: DS-0029
(IaC/Dockerfile)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/server/Dockerfile` around lines 17 - 21, Runtime stage currently runs
as root and uses apt-get without --no-install-recommends; create a non-root
user/group (e.g., zenvra or appuser), chown the application binary
(/usr/local/bin/zenvra-server) and any WORKDIR (/app) to that user, add a USER
directive to switch to that non-root user, and change the apt-get install
invocation in the runtime stage to use --no-install-recommends to reduce
packages; ensure these changes are applied in the Stage 2 block that defines
FROM debian:bookworm-slim and references /usr/local/bin/zenvra-server so the
runtime container no longer runs as root and has a minimal installed footprint.
| async fn sync_nvd(pool: &Pool<Postgres>, client: &Client) -> anyhow::Result<()> { | ||
| let api_key = env::var("NVD_API_KEY").ok(); | ||
| if api_key.is_none() { | ||
| info!("NVD_API_KEY not set. Running in rate-limited mode."); | ||
| } | ||
|
|
||
| let params = vec![("resultsPerPage", "100".to_string())]; | ||
| let url = reqwest::Url::parse_with_params("https://services.nvd.nist.gov/rest/json/cves/2.0", ¶ms)?; | ||
|
|
||
| info!("Calling NVD API: {}", url); | ||
|
|
||
| let mut request = client | ||
| .get(url) | ||
| .header("User-Agent", "Zenvra-Scanner/0.1.0"); | ||
|
|
||
| if let Some(key) = api_key { | ||
| request = request.header("apiKey", key); | ||
| } | ||
|
|
||
| let response: reqwest::Response = request.send().await?; | ||
|
|
||
| if !response.status().is_success() { | ||
| let status = response.status(); | ||
| let body = response.text().await.unwrap_or_else(|_| "Empty body".to_string()); | ||
| error!("NVD API error (Status: {}): {}", status, body); | ||
| anyhow::bail!("NVD API returned error status: {}", status); | ||
| } | ||
|
|
||
| let nvd_data = response.json::<NvdResponse>().await?; |
There was a problem hiding this comment.
NVD sync only fetches first 100 CVEs — pagination missing.
The NVD API contains 200k+ vulnerabilities, but this implementation only fetches the first page (resultsPerPage=100). The NVD API returns totalResults and supports startIndex for pagination.
🔧 Suggested approach for pagination
async fn sync_nvd(pool: &Pool<Postgres>, client: &Client) -> anyhow::Result<()> {
let api_key = env::var("NVD_API_KEY").ok();
let mut start_index = 0;
let page_size = 2000; // NVD max is 2000
loop {
let params = vec![
("resultsPerPage", page_size.to_string()),
("startIndex", start_index.to_string()),
];
let url = reqwest::Url::parse_with_params(
"https://services.nvd.nist.gov/rest/json/cves/2.0",
¶ms
)?;
// ... fetch and process ...
let total_results = nvd_data.total_results; // Add to NvdResponse struct
start_index += page_size;
if start_index >= total_results {
break;
}
// Respect rate limits (6 req/min without key, 50/30s with key)
tokio::time::sleep(Duration::from_secs(if api_key.is_some() { 1 } else { 10 })).await;
}
Ok(())
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/server/src/cve_sync/mod.rs` around lines 60 - 88, sync_nvd currently
only fetches the first 100 results; change it to paginate using startIndex and
resultsPerPage in a loop: add a mutable start_index (0) and choose page_size (up
to 2000), build the URL with both ("resultsPerPage", page_size) and
("startIndex", start_index), send the request, parse into NvdResponse (extend
the NvdResponse struct to include totalResults/total_results), process each
page, then increment start_index by page_size and break once start_index >=
totalResults; also add a delay between requests based on whether NVD_API_KEY is
set to respect rate limits. Ensure you still include the existing User-Agent and
apiKey header logic and keep the same error handling for non-success responses.
| export interface ScanRequest { | ||
| code: string; | ||
| language: string; | ||
| engines?: string[]; | ||
| aiConfig?: AiConfig; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check ScanRequest struct definition in server
ast-grep --pattern 'struct ScanRequest {
$$$
}'Repository: Cameroon-Developer-Network/Zenvra
Length of output: 392
🏁 Script executed:
#!/bin/bash
# Read the context around the ScanRequest struct to see attributes
sed -n '35,50p' crates/server/src/main.rsRepository: Cameroon-Developer-Network/Zenvra
Length of output: 496
🏁 Script executed:
#!/bin/bash
# Find AiConfig struct definition and its fields
ast-grep --pattern 'struct AiConfig {
$$$
}'Repository: Cameroon-Developer-Network/Zenvra
Length of output: 59
🏁 Script executed:
#!/bin/bash
# Search for serde attributes on ScanRequest or ai_config field
rg -A 5 'struct ScanRequest' crates/server/src/main.rsRepository: Cameroon-Developer-Network/Zenvra
Length of output: 218
🏁 Script executed:
#!/bin/bash
# Find AiConfig struct in the scanner crate
find . -name "*.rs" -type f | xargs rg -l "struct AiConfig"Repository: Cameroon-Developer-Network/Zenvra
Length of output: 107
🏁 Script executed:
#!/bin/bash
# Get AiConfig struct definition and fields
rg -A 10 "struct AiConfig" crates/scanner/src/ai/Repository: Cameroon-Developer-Network/Zenvra
Length of output: 736
🏁 Script executed:
#!/bin/bash
# Get full AiConfig struct definition with derives
sed -n '1,50p' crates/scanner/src/ai/mod.rs | tail -30Repository: Cameroon-Developer-Network/Zenvra
Length of output: 994
🏁 Script executed:
#!/bin/bash
# Search for pub struct AiConfig with more context to see derives
rg -B 3 "pub struct AiConfig" crates/scanner/src/ai/mod.rsRepository: Cameroon-Developer-Network/Zenvra
Length of output: 287
🏁 Script executed:
#!/bin/bash
# Also verify TypeScript interface - check if aiConfig field exists in types.ts
rg "aiConfig" extensions/vscode/src/types.tsRepository: Cameroon-Developer-Network/Zenvra
Length of output: 99
Critical: Field name mismatch causes AI config to be silently ignored.
The VS Code extension sends aiConfig and apiKey (camelCase), but the server's ScanRequest and AiConfig structs use ai_config and api_key (snake_case) without #[serde(rename_all = "camelCase")]. During deserialization, these fields will not match and Serde will silently ignore them, disabling AI-powered scanning.
Either rename the TypeScript fields to snake_case, or add #[serde(rename_all = "camelCase")] to the server structs.
🐛 Option 1: Fix TypeScript to use snake_case
export interface AiConfig {
provider: string;
- apiKey: string;
+ api_key: string;
model: string;
endpoint?: string;
}
export interface ScanRequest {
code: string;
language: string;
engines?: string[];
- aiConfig?: AiConfig;
+ ai_config?: AiConfig;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@extensions/vscode/src/types.ts` around lines 12 - 17, The server-side structs
ScanRequest and AiConfig are using snake_case fields (ai_config, api_key) while
the client sends camelCase (aiConfig, apiKey), so add #[serde(rename_all =
"camelCase")] to the ScanRequest and AiConfig struct definitions (or
alternatively rename the TypeScript fields to snake_case) to ensure Serde
correctly maps aiConfig -> ai_config and apiKey -> api_key and prevent the AI
config from being ignored.
…ion, and optimize scanner event handling
…s to settings pages
…ty and key stability in history page components
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
crates/server/src/cve_sync/mod.rs (1)
66-88:⚠️ Potential issue | 🟠 MajorThis still stops after a single NVD page.
resultsPerPageis fixed at 100 and the job never advances through additional pages, so a successful sync only ingests that first slice of CVEs. That leaves the table materially incomplete and misses most lookup hits until pagination is implemented.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/server/src/cve_sync/mod.rs` around lines 66 - 88, The code only fetches a single page because params is fixed to resultsPerPage=100 and there is no pagination loop; modify the logic around params/url/request/response/NvdResponse to implement pagination: add a startIndex (or equivalent offset) param and wrap the request/send/json steps in a loop that updates startIndex each iteration, accumulating CVE items from each NvdResponse; stop when you've fetched all results (use totalResults from NvdResponse or when returned items length is zero or less than resultsPerPage) and handle rate limits between requests as needed.
🧹 Nitpick comments (4)
apps/web/src/routes/scan/+page.svelte (4)
83-87: SSE error handler should updatescanStatusfor user feedback.When the SSE connection fails, the user only sees the scan stop without explanation. Setting
scanStatusto an error message would improve UX consistency with thecase 'error'branch.♻️ Proposed enhancement
eventSource.onerror = () => { console.error("SSE connection failed"); + scanStatus = "Connection lost — please retry"; isScanning = false; eventSource.close(); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/routes/scan/`+page.svelte around lines 83 - 87, The SSE onerror handler currently only logs, sets isScanning = false and closes eventSource; update it to also set scanStatus to an error message (similar to the existing case 'error' branch) so users see a descriptive failure state; modify the eventSource.onerror callback to assign scanStatus = "SSE connection failed" (or reuse the same error text used in the 'error' case) before setting isScanning = false and closing eventSource.
36-56: Fetch calls should go through typed API client in$lib/api.ts.The direct
fetch()call andEventSourceURL construction bypass the typed API client. While the URL is now correctly sourced fromaiConfig.apiBaseUrl(addressing the hardcoded URL concern), the coding guideline requires all fetch calls to go through typed API client functions.Consider adding
startScanand exportingBASE_URLfromapi.ts:♻️ Suggested api.ts additions
// In apps/web/src/lib/api.ts export const BASE_URL = (import.meta.env.PUBLIC_API_URL || 'http://localhost:8080').replace(/\/$/, ''); export interface ScanRequest { code: string; language: string; engines: string[]; ai_config?: { provider: string; api_key: string; model: string; endpoint?: string; }; } export async function startScan(req: ScanRequest): Promise<string> { const res = await fetch(`${BASE_URL}/api/v1/scan`, { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify(req) }); if (!res.ok) throw new Error("Failed to start scan"); const { scan_id } = await res.json(); return scan_id; } export function getScanEventsUrl(scanId: string): string { return `${BASE_URL}/api/v1/scan/${scanId}/events`; }As per coding guidelines: "All fetch calls must go through typed API client functions in
apps/web/src/lib/api.ts".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/routes/scan/`+page.svelte around lines 36 - 56, Replace the direct fetch and EventSource usage in +page.svelte with the typed API client functions: add/export BASE_URL, the ScanRequest type, a startScan(req: ScanRequest): Promise<string> that POSTs to `${BASE_URL}/api/v1/scan` and returns scan_id, and a getScanEventsUrl(scanId: string): string that builds `${BASE_URL}/api/v1/scan/${scanId}/events`; then in +page.svelte call startScan(...) to obtain scan_id and use getScanEventsUrl(scan_id) to create the EventSource, removing the inline fetch and URL construction and ensuring the ai_config shape matches the ScanRequest type.
58-81: SSE event data should be validated with zod.The
JSON.parse(event.data)result is used directly without type validation. Per coding guidelines, API input should be validated with zod to ensure type safety and catch malformed server responses gracefully.♻️ Suggested validation schema
// In api.ts or a shared schema file import { z } from 'zod'; const ScanEventSchema = z.discriminatedUnion('type', [ z.object({ type: z.literal('progress'), data: z.object({ percentage: z.number(), message: z.string() }) }), z.object({ type: z.literal('finding'), data: FindingSchema }), // assuming FindingSchema exists z.object({ type: z.literal('complete'), data: z.unknown() }), z.object({ type: z.literal('error'), data: z.string() }), ]); // Usage in onmessage handler: const parsed = ScanEventSchema.safeParse(JSON.parse(event.data)); if (!parsed.success) { console.error("Invalid event data", parsed.error); return; } const data = parsed.data;As per coding guidelines: "Use zod for all form and API input validation".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/routes/scan/`+page.svelte around lines 58 - 81, Validate SSE payloads before use: create a zod discriminated union (e.g., ScanEventSchema) that covers the four event types (progress with {percentage:number,message:string}, finding with data matching FindingSchema, complete (unknown), and error (string)), export it from a shared schema file (api.ts) and in the eventSource.onmessage handler call ScanEventSchema.safeParse(JSON.parse(event.data)); if parsing fails, log the parsed.error and return early, otherwise use parsed.data in the existing switch logic (referencing eventSource.onmessage and FindingSchema to locate where to wire the schema).
107-298: Component exceeds 200-line limit.At ~300 lines, this file exceeds the coding guideline maximum of 200 lines. Consider extracting the progress panel (lines 183-217) and finding card (lines 227-291) into separate components.
As per coding guidelines: "Component files max 200 lines — split into smaller components if larger".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/routes/scan/`+page.svelte around lines 107 - 298, This file is over the 200-line limit; extract the live progress panel (the block using scanProgress and scanStatus with the circular and linear progress visuals) into a new ProgressPanel component and extract the finding card (the repeated {`#each` findings as finding ...} block that references finding, getSeverityColor, and displays description/explanation/vulnerable_code/fixed_code) into a new FindingCard component; update +page.svelte to import and render <ProgressPanel {scanProgress} {scanStatus} {isScanning}/> where the original progress block was and replace the finding card markup with <FindingCard {finding} getSeverityColor={getSeverityColor}/> (or pass only needed props), ensure FindingCard accepts props for finding and uses getSeverityColor or a mapped color prop, move any internal logic/conditionals (e.g., description !== explanation) into the new component, and keep all styling/classes intact so behavior and appearance remain unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 156-173: The CI workflow's Docker steps "Build and push API
(latest)" and "Build and push Web (latest)" push :latest tags that conflict with
the release.yml docker-publish job; update these steps to avoid the race by
either (A) changing the tags from ghcr.io/${{ github.repository }}-api:latest
and -web:latest to a CI-specific tag like :main or :edge, or (B) adding a
condition to these jobs to skip when a Git tag event is present (e.g., check
github.ref for refs/tags), or (C) remove the push of the :latest tag here and
keep it only in release.yml—apply the chosen change to both the API and Web
docker/build-push-action steps so tags do not collide.
In `@apps/web/src/routes/scan/`+page.svelte:
- Around line 2-3: Remove the unused imports causing the linter failure: delete
onMount and scan from the import statements (leave type Finding if it’s still
referenced), i.e., update the Svelte module import so it only imports symbols
actually used in the component (remove the unused onMount from "svelte" import
and remove scan from "$lib/api" import) to satisfy the linter and fix CI.
In `@apps/web/src/routes/settings/cve/`+page.svelte:
- Around line 36-53: The UI is showing a hardcoded "Connected" badge, non-zero
"Synced CVEs" and progress bar even when no feed health data is loaded; update
the component to conditionally render real feed health metrics (or a clear
placeholder) instead of hardcoded values: read the NVD feed state provided by
the page load/store (e.g., a feedHealth or nvdStatus object) and only render the
"Connected" span, the synced-count span (currently showing "100+"), and the
progress bar div when valid data exists; otherwise show a neutral state such as
"Not connected" / "No data" or a "Placeholder" label and hide zero/positive
metrics. Apply the same conditional rendering pattern for the other card
instances (the block around the "Connected" span and the synced metrics/progress
bar, also present in the 58-72 section).
In `@crates/server/src/cve_sync/mod.rs`:
- Around line 130-167: The sync_osv function currently inserts placeholder rows
(sample_id like "OSV-...-SAMPLE-001") into the vulnerabilities table when an
ecosystem is empty, which prevents real OSV data from ever backfilling and
pollutes downstream logic; remove that behavior and instead only perform a
connectivity/status check or mark any non-production rows clearly so they don't
block real data. Specifically, in sync_osv replace the block that constructs and
INSERTs sample_id/OSV-*-SAMPLE-001 and the COUNT(*) guard with either (a) no
INSERT at all and just log the ecosystem connectivity status, or (b) if you must
keep example data for demos, write them to a separate demo table or set a
deterministic flag/column (e.g., is_demo=true or data_source="osv-demo") so they
are distinguishable and will not prevent real OSV syncs from
replacing/backfilling real advisories; update code references around sync_osv,
sample_id, and the SQL INSERT/COUNT logic accordingly.
- Around line 48-173: Add a #[cfg(test)] mod at the bottom that provides async
tests for sync_nvd, sync_osv and the parsing fallbacks: write tokio::test cases
that spin up a lightweight HTTP mock server (e.g., httpmock/httptest) to serve
NVD JSON with missing descriptions and missing cvss_v31 to verify the
description default and severity fallback logic in sync_nvd, and serve OSV-like
responses or call sync_osv to validate the initial-insert path; use a temporary
test database (in-memory or a test Postgres instance via sqlx) to assert rows
inserted/updated by sync_nvd/sync_osv (check cve_id, description, severity,
data_source, ecosystem), and include setup/teardown so tests are hermetic; place
the module at the file bottom inside #[cfg(test)] and reference sync_nvd,
sync_osv, and sync_all in the tests.
---
Duplicate comments:
In `@crates/server/src/cve_sync/mod.rs`:
- Around line 66-88: The code only fetches a single page because params is fixed
to resultsPerPage=100 and there is no pagination loop; modify the logic around
params/url/request/response/NvdResponse to implement pagination: add a
startIndex (or equivalent offset) param and wrap the request/send/json steps in
a loop that updates startIndex each iteration, accumulating CVE items from each
NvdResponse; stop when you've fetched all results (use totalResults from
NvdResponse or when returned items length is zero or less than resultsPerPage)
and handle rate limits between requests as needed.
---
Nitpick comments:
In `@apps/web/src/routes/scan/`+page.svelte:
- Around line 83-87: The SSE onerror handler currently only logs, sets
isScanning = false and closes eventSource; update it to also set scanStatus to
an error message (similar to the existing case 'error' branch) so users see a
descriptive failure state; modify the eventSource.onerror callback to assign
scanStatus = "SSE connection failed" (or reuse the same error text used in the
'error' case) before setting isScanning = false and closing eventSource.
- Around line 36-56: Replace the direct fetch and EventSource usage in
+page.svelte with the typed API client functions: add/export BASE_URL, the
ScanRequest type, a startScan(req: ScanRequest): Promise<string> that POSTs to
`${BASE_URL}/api/v1/scan` and returns scan_id, and a getScanEventsUrl(scanId:
string): string that builds `${BASE_URL}/api/v1/scan/${scanId}/events`; then in
+page.svelte call startScan(...) to obtain scan_id and use
getScanEventsUrl(scan_id) to create the EventSource, removing the inline fetch
and URL construction and ensuring the ai_config shape matches the ScanRequest
type.
- Around line 58-81: Validate SSE payloads before use: create a zod
discriminated union (e.g., ScanEventSchema) that covers the four event types
(progress with {percentage:number,message:string}, finding with data matching
FindingSchema, complete (unknown), and error (string)), export it from a shared
schema file (api.ts) and in the eventSource.onmessage handler call
ScanEventSchema.safeParse(JSON.parse(event.data)); if parsing fails, log the
parsed.error and return early, otherwise use parsed.data in the existing switch
logic (referencing eventSource.onmessage and FindingSchema to locate where to
wire the schema).
- Around line 107-298: This file is over the 200-line limit; extract the live
progress panel (the block using scanProgress and scanStatus with the circular
and linear progress visuals) into a new ProgressPanel component and extract the
finding card (the repeated {`#each` findings as finding ...} block that references
finding, getSeverityColor, and displays
description/explanation/vulnerable_code/fixed_code) into a new FindingCard
component; update +page.svelte to import and render <ProgressPanel
{scanProgress} {scanStatus} {isScanning}/> where the original progress block was
and replace the finding card markup with <FindingCard {finding}
getSeverityColor={getSeverityColor}/> (or pass only needed props), ensure
FindingCard accepts props for finding and uses getSeverityColor or a mapped
color prop, move any internal logic/conditionals (e.g., description !==
explanation) into the new component, and keep all styling/classes intact so
behavior and appearance remain 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
Run ID: 7f68bfa3-04d8-490b-973c-a57ad78aea1c
📒 Files selected for processing (9)
.github/workflows/ci.ymlapps/web/src/lib/stores/aiConfig.svelte.tsapps/web/src/routes/scan/+page.svelteapps/web/src/routes/settings/cve/+page.sveltecrates/scanner/src/finding.rscrates/scanner/src/lib.rscrates/server/src/cve_sync/mod.rscrates/server/src/main.rsmigrations/202604060001_add_osv_fields.sql
✅ Files skipped from review due to trivial changes (1)
- migrations/202604060001_add_osv_fields.sql
🚧 Files skipped from review as they are similar to previous changes (3)
- crates/scanner/src/finding.rs
- crates/scanner/src/lib.rs
- crates/server/src/main.rs
| - name: Build and push API (latest) | ||
| uses: docker/build-push-action@v5 | ||
| with: | ||
| context: . | ||
| file: crates/server/Dockerfile | ||
| push: true | ||
| tags: ghcr.io/${{ github.repository }}-api:latest | ||
| cache-from: type=gha | ||
| cache-to: type=gha,mode=max | ||
|
|
||
| - name: Build and push Web (latest) | ||
| uses: docker/build-push-action@v5 | ||
| with: | ||
| context: ./apps/web | ||
| push: true | ||
| tags: ghcr.io/${{ github.repository }}-web:latest | ||
| cache-from: type=gha | ||
| cache-to: type=gha,mode=max |
There was a problem hiding this comment.
Potential :latest tag conflict with release.yml.
Both this deploy job and the docker-publish job in release.yml push :latest tags to the same image names (ghcr.io/${{ github.repository }}-api:latest and -web:latest). When a release tag is pushed to main, both workflows may run concurrently, causing race conditions on the :latest tag.
Consider one of these approaches:
- Use a different tag here (e.g.,
:mainor:edge) to distinguish CI builds from releases - Add a condition to skip this job when a version tag is present
- Remove the
:latesttag push from one of the workflows
Option 1: Use distinct tag for CI builds
- name: Build and push API (latest)
uses: docker/build-push-action@v5
with:
context: .
file: crates/server/Dockerfile
push: true
- tags: ghcr.io/${{ github.repository }}-api:latest
+ tags: ghcr.io/${{ github.repository }}-api:edge
cache-from: type=gha
cache-to: type=gha,mode=max
- name: Build and push Web (latest)
uses: docker/build-push-action@v5
with:
context: ./apps/web
push: true
- tags: ghcr.io/${{ github.repository }}-web:latest
+ tags: ghcr.io/${{ github.repository }}-web:edge
cache-from: type=gha
cache-to: type=gha,mode=max🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/ci.yml around lines 156 - 173, The CI workflow's Docker
steps "Build and push API (latest)" and "Build and push Web (latest)" push
:latest tags that conflict with the release.yml docker-publish job; update these
steps to avoid the race by either (A) changing the tags from ghcr.io/${{
github.repository }}-api:latest and -web:latest to a CI-specific tag like :main
or :edge, or (B) adding a condition to these jobs to skip when a Git tag event
is present (e.g., check github.ref for refs/tags), or (C) remove the push of the
:latest tag here and keep it only in release.yml—apply the chosen change to both
the API and Web docker/build-push-action steps so tags do not collide.
| <div class="glass p-8 rounded-3xl border-zinc-800 relative overflow-hidden group"> | ||
| <div class="flex items-start justify-between mb-6"> | ||
| <div class="w-12 h-12 rounded-2xl bg-brand-primary/10 flex items-center justify-center text-brand-primary"> | ||
| <svg xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round"><path d="M12 22s8-4 8-10V5l-8-3-8 3v7c0 6 8 10 8 10"/></svg> | ||
| </div> | ||
| <span class="text-[10px] font-bold bg-emerald-500/10 text-emerald-500 px-2 py-1 rounded tracking-widest uppercase">Connected</span> | ||
| </div> | ||
| <h3 class="text-xl font-bold mb-2">NVD Data Feed</h3> | ||
| <p class="text-sm text-zinc-500 leading-relaxed max-w-[240px]">Synchronizing with the National Vulnerability Database via API v2.</p> | ||
|
|
||
| <div class="mt-8 space-y-3"> | ||
| <div class="flex justify-between text-xs text-zinc-500"> | ||
| <span>Synced CVEs</span> | ||
| <span class="text-zinc-300 font-bold tracking-widest">100+</span> | ||
| </div> | ||
| <div class="w-full bg-zinc-900 h-1.5 rounded-full overflow-hidden"> | ||
| <div class="bg-brand-primary w-2/5 h-full rounded-full"></div> | ||
| </div> |
There was a problem hiding this comment.
Don't present placeholder feed health as real status.
These cards always render Connected plus non-zero coverage even though this page never loads any actual feed health or sync metrics. On a security settings page that creates false confidence after failed or never-run syncs; either back them with real API data or clearly label/hide them as placeholders for now.
Also applies to: 58-72
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/routes/settings/cve/`+page.svelte around lines 36 - 53, The UI
is showing a hardcoded "Connected" badge, non-zero "Synced CVEs" and progress
bar even when no feed health data is loaded; update the component to
conditionally render real feed health metrics (or a clear placeholder) instead
of hardcoded values: read the NVD feed state provided by the page load/store
(e.g., a feedHealth or nvdStatus object) and only render the "Connected" span,
the synced-count span (currently showing "100+"), and the progress bar div when
valid data exists; otherwise show a neutral state such as "Not connected" / "No
data" or a "Placeholder" label and hide zero/positive metrics. Apply the same
conditional rendering pattern for the other card instances (the block around the
"Connected" span and the synced metrics/progress bar, also present in the 58-72
section).
| /// Sync all vulnerability data sources. | ||
| pub async fn sync_all(pool: &Pool<Postgres>) -> anyhow::Result<()> { | ||
| info!("Starting full CVE synchronization..."); | ||
|
|
||
| let client = Client::new(); | ||
| sync_nvd(pool, &client).await?; | ||
| sync_osv(pool, &client).await?; | ||
|
|
||
| info!("CVE synchronization completed successfully."); | ||
| Ok(()) | ||
| } | ||
|
|
||
| async fn sync_nvd(pool: &Pool<Postgres>, client: &Client) -> anyhow::Result<()> { | ||
| let api_key = env::var("NVD_API_KEY").ok(); | ||
| if api_key.is_none() { | ||
| info!("NVD_API_KEY not set. Running in rate-limited mode."); | ||
| } | ||
|
|
||
| let params = vec![("resultsPerPage", "100".to_string())]; | ||
| let url = reqwest::Url::parse_with_params("https://services.nvd.nist.gov/rest/json/cves/2.0", ¶ms)?; | ||
|
|
||
| info!("Calling NVD API: {}", url); | ||
|
|
||
| let mut request = client | ||
| .get(url) | ||
| .header("User-Agent", "Zenvra-Scanner/0.1.0"); | ||
|
|
||
| if let Some(key) = api_key { | ||
| request = request.header("apiKey", key); | ||
| } | ||
|
|
||
| let response: reqwest::Response = request.send().await?; | ||
|
|
||
| if !response.status().is_success() { | ||
| let status = response.status(); | ||
| let body = response.text().await.unwrap_or_else(|_| "Empty body".to_string()); | ||
| error!("NVD API error (Status: {}): {}", status, body); | ||
| anyhow::bail!("NVD API returned error status: {}", status); | ||
| } | ||
|
|
||
| let nvd_data = response.json::<NvdResponse>().await?; | ||
|
|
||
| for item in nvd_data.vulnerabilities { | ||
| let cve = item.cve; | ||
| let id = cve.id; | ||
| let description = cve | ||
| .descriptions | ||
| .first() | ||
| .map(|d| d.value.clone()) | ||
| .unwrap_or_default(); | ||
| let severity = cve | ||
| .metrics | ||
| .and_then(|m| m.cvss_v31) | ||
| .and_then(|v: Vec<CvssMetricV31>| { | ||
| v.first() | ||
| .map(|c| c.cvss_data.base_severity.to_lowercase()) | ||
| }) | ||
| .unwrap_or_else(|| "medium".to_string()); | ||
|
|
||
| sqlx::query( | ||
| r#" | ||
| INSERT INTO vulnerabilities (cve_id, title, description, severity, data_source) | ||
| VALUES ($1, $2, $3, $4, $5) | ||
| ON CONFLICT (cve_id) DO UPDATE SET | ||
| description = EXCLUDED.description, | ||
| severity = EXCLUDED.severity, | ||
| updated_at = CURRENT_TIMESTAMP | ||
| "# | ||
| ) | ||
| .bind(&id) | ||
| .bind(format!("Vulnerability {}", id)) | ||
| .bind(&description) | ||
| .bind(&severity) | ||
| .bind("nvd") | ||
| .execute(pool) | ||
| .await?; | ||
| } | ||
|
|
||
| info!("NVD sync completed."); | ||
| Ok(()) | ||
| } | ||
|
|
||
| async fn sync_osv(pool: &Pool<Postgres>, _client: &Client) -> anyhow::Result<()> { | ||
| info!("Starting OSV synchronization for popular ecosystems..."); | ||
|
|
||
| let ecosystems = vec!["npm", "PyPI", "Go", "crates.io"]; | ||
|
|
||
| for ecosystem in ecosystems { | ||
| info!("Fetching recent vulnerabilities for ecosystem: {}", ecosystem); | ||
|
|
||
| // In a real implementation, we would fetch the list of affected packages or use the GS storage. | ||
| // For this MVP, we fetch a few well-known recent vulnerability reports to demonstrate the platform's capability. | ||
| // We simulate this by querying the OSV API with a common vulnerable package example if we had one. | ||
| // Instead, we will implement a basic "Status: Online" for now by just checking connectivity, | ||
| // and inserting a few sample records if the DB is empty for that ecosystem. | ||
|
|
||
| let count: (i64,) = sqlx::query_as("SELECT COUNT(*) FROM vulnerabilities WHERE data_source = 'osv' AND ecosystem = $1") | ||
| .bind(ecosystem) | ||
| .fetch_one(pool) | ||
| .await?; | ||
|
|
||
| if count.0 == 0 { | ||
| info!("Populating initial OSV data for {}", ecosystem); | ||
| let sample_id = format!("OSV-{}-SAMPLE-001", ecosystem.to_uppercase()); | ||
| sqlx::query( | ||
| r#" | ||
| INSERT INTO vulnerabilities (cve_id, title, description, severity, data_source, ecosystem, package_name) | ||
| VALUES ($1, $2, $3, $4, $5, $6, $7) | ||
| ON CONFLICT (cve_id) DO NOTHING | ||
| "# | ||
| ) | ||
| .bind(&sample_id) | ||
| .bind(format!("Sample Vulnerability in {}", ecosystem)) | ||
| .bind(format!("Automatically monitored advisory for {} packages. More details will be fetched during deep scans.", ecosystem)) | ||
| .bind("medium") | ||
| .bind("osv") | ||
| .bind(ecosystem) | ||
| .bind("sample-package") | ||
| .execute(pool) | ||
| .await?; | ||
| } | ||
| } | ||
|
|
||
| info!("OSV synchronization completed."); | ||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Please add a #[cfg(test)] module for this sync logic.
This file now owns non-trivial parsing and persistence behavior but has no regression coverage. At minimum, cover the description/severity fallbacks and the NVD/OSV insert paths so upstream payload or schema changes fail fast.
As per coding guidelines, "crates/**/*.rs: Write tests in #[cfg(test)] modules at the bottom of each file".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/server/src/cve_sync/mod.rs` around lines 48 - 173, Add a #[cfg(test)]
mod at the bottom that provides async tests for sync_nvd, sync_osv and the
parsing fallbacks: write tokio::test cases that spin up a lightweight HTTP mock
server (e.g., httpmock/httptest) to serve NVD JSON with missing descriptions and
missing cvss_v31 to verify the description default and severity fallback logic
in sync_nvd, and serve OSV-like responses or call sync_osv to validate the
initial-insert path; use a temporary test database (in-memory or a test Postgres
instance via sqlx) to assert rows inserted/updated by sync_nvd/sync_osv (check
cve_id, description, severity, data_source, ecosystem), and include
setup/teardown so tests are hermetic; place the module at the file bottom inside
#[cfg(test)] and reference sync_nvd, sync_osv, and sync_all in the tests.
| async fn sync_osv(pool: &Pool<Postgres>, _client: &Client) -> anyhow::Result<()> { | ||
| info!("Starting OSV synchronization for popular ecosystems..."); | ||
|
|
||
| let ecosystems = vec!["npm", "PyPI", "Go", "crates.io"]; | ||
|
|
||
| for ecosystem in ecosystems { | ||
| info!("Fetching recent vulnerabilities for ecosystem: {}", ecosystem); | ||
|
|
||
| // In a real implementation, we would fetch the list of affected packages or use the GS storage. | ||
| // For this MVP, we fetch a few well-known recent vulnerability reports to demonstrate the platform's capability. | ||
| // We simulate this by querying the OSV API with a common vulnerable package example if we had one. | ||
| // Instead, we will implement a basic "Status: Online" for now by just checking connectivity, | ||
| // and inserting a few sample records if the DB is empty for that ecosystem. | ||
|
|
||
| let count: (i64,) = sqlx::query_as("SELECT COUNT(*) FROM vulnerabilities WHERE data_source = 'osv' AND ecosystem = $1") | ||
| .bind(ecosystem) | ||
| .fetch_one(pool) | ||
| .await?; | ||
|
|
||
| if count.0 == 0 { | ||
| info!("Populating initial OSV data for {}", ecosystem); | ||
| let sample_id = format!("OSV-{}-SAMPLE-001", ecosystem.to_uppercase()); | ||
| sqlx::query( | ||
| r#" | ||
| INSERT INTO vulnerabilities (cve_id, title, description, severity, data_source, ecosystem, package_name) | ||
| VALUES ($1, $2, $3, $4, $5, $6, $7) | ||
| ON CONFLICT (cve_id) DO NOTHING | ||
| "# | ||
| ) | ||
| .bind(&sample_id) | ||
| .bind(format!("Sample Vulnerability in {}", ecosystem)) | ||
| .bind(format!("Automatically monitored advisory for {} packages. More details will be fetched during deep scans.", ecosystem)) | ||
| .bind("medium") | ||
| .bind("osv") | ||
| .bind(ecosystem) | ||
| .bind("sample-package") | ||
| .execute(pool) | ||
| .await?; |
There was a problem hiding this comment.
Don't store placeholder rows as synced OSV data.
This function never calls OSV; when an ecosystem is empty it inserts OSV-*-SAMPLE-001 rows into vulnerabilities instead. Those records are indistinguishable from real advisories to downstream lookup/history code, and the count == 0 guard means a later real sync will never backfill that ecosystem once the sample row exists.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/server/src/cve_sync/mod.rs` around lines 130 - 167, The sync_osv
function currently inserts placeholder rows (sample_id like
"OSV-...-SAMPLE-001") into the vulnerabilities table when an ecosystem is empty,
which prevents real OSV data from ever backfilling and pollutes downstream
logic; remove that behavior and instead only perform a connectivity/status check
or mark any non-production rows clearly so they don't block real data.
Specifically, in sync_osv replace the block that constructs and INSERTs
sample_id/OSV-*-SAMPLE-001 and the COUNT(*) guard with either (a) no INSERT at
all and just log the ecosystem connectivity status, or (b) if you must keep
example data for demos, write them to a separate demo table or set a
deterministic flag/column (e.g., is_demo=true or data_source="osv-demo") so they
are distinguishable and will not prevent real OSV syncs from
replacing/backfilling real advisories; update code references around sync_osv,
sample_id, and the SQL INSERT/COUNT logic accordingly.
…pdate serialization aliases, and refine CLI/VSCode configurations.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/cli/src/main.rs (1)
46-48:⚠️ Potential issue | 🟡 MinorInconsistent default model name in help text vs. code.
The
--ai_modelhelp text on line 46 referencesclaude-sonnet-4-20250514, but the actual default for Anthropic on line 353 isclaude-3-5-sonnet-20240620. Update one to match the other for consistency.Also applies to: 352-354
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/cli/src/main.rs` around lines 46 - 48, The help text for the CLI option ai_model (the ai_model field annotated with #[arg(long)]) is inconsistent with the actual Anthropic default model used elsewhere (the default string set for Anthropic models, e.g., "claude-3-5-sonnet-20240620"); update either the ai_model help text to mention the real default ("claude-3-5-sonnet-20240620") or change the Anthropic default constant/value to match the help text ("claude-sonnet-4-20250514") so both the #[arg(long)] ai_model description and the Anthropic default value reference the same model name. Ensure the same change is applied wherever the Anthropic default model string is defined/used (the default assignment for Anthropic model).
🧹 Nitpick comments (5)
extensions/vscode/src/sidebarProvider.ts (1)
46-50: Consider typing themessageparameter for better type safety.The
postMessagemethod acceptsunknown, but based on the extension's usage, the messages have specific shapes ({ type: 'progress' | 'finding' | 'complete', data?: unknown }).💡 Optional: Add message type
+type SidebarMessage = + | { type: 'progress'; data: { percentage: number; message: string } } + | { type: 'finding'; data: unknown } + | { type: 'complete' }; - public postMessage(message: unknown) { + public postMessage(message: SidebarMessage) { if (this._view) { this._view.webview.postMessage(message); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extensions/vscode/src/sidebarProvider.ts` around lines 46 - 50, The postMessage method currently accepts unknown which reduces type safety; update the signature of postMessage to accept a discriminated union matching the extension's message shapes (e.g., an interface/union for { type: 'progress' | 'finding' | 'complete', data?: unknown }) and use that type for the message parameter, then pass it through to this._view.webview.postMessage(message) unchanged; modify any callers to conform if necessary and add the new Message type (or a namespaced alias) near the SidebarProvider class so references like postMessage and this._view.webview.postMessage are clearly typed.crates/server/src/main.rs (2)
102-105: Permissive CORS configuration may expose API to unintended origins.
CorsLayer::allow_origin(Any)allows requests from any origin. While acceptable for local development, this should be restricted in production to prevent unauthorized cross-origin access to the scan API.💡 Suggested improvement for production
+ let allowed_origins = std::env::var("CORS_ORIGINS") + .map(|origins| origins.split(',').map(|s| s.trim().to_string()).collect::<Vec<_>>()) + .ok(); + let cors = CorsLayer::new() - .allow_origin(Any) + .allow_origin(match allowed_origins { + Some(origins) => origins.iter().filter_map(|o| o.parse().ok()).collect::<Vec<_>>().into(), + None => Any.into(), + }) .allow_methods(Any) .allow_headers(Any);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/server/src/main.rs` around lines 102 - 105, The CORS setup uses CorsLayer::new().allow_origin(Any) which is too permissive for production; update the cors construction (variable cors) to read allowed origins from configuration or an environment variable and pass a restricted origin list instead of Any (e.g., build a HeaderValue/Origin list or use Any only in dev), and apply allow_methods/allow_headers as needed; locate the CorsLayer::new() call and replace allow_origin(Any) with logic that parses a configured ALLOWED_ORIGINS and constructs the appropriate allow_origin value for production.
200-212: CVE enrichment overwrites scanner-generated title unconditionally.When a CVE match is found, the original
finding.titlefrom the scanner is replaced with the CVE database title. If the scanner's title contains more context (e.g., code-specific details), this information is lost.Consider appending or preserving both:
💡 Optional: Preserve both titles
if let Ok(Some(row)) = sqlx::query( - "SELECT title, description FROM vulnerabilities WHERE cve_id = $1", + "SELECT title FROM vulnerabilities WHERE cve_id = $1", ) .bind(cve_id) .fetch_optional(&state_task.db) .await { use sqlx::Row; - finding.title = row.get("title"); + let cve_title: String = row.get("title"); + if !cve_title.is_empty() && cve_title != finding.title { + finding.title = format!("{} ({})", finding.title, cve_title); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/server/src/main.rs` around lines 200 - 212, The enrichment code unconditionally replaces the scanner-generated finding.title when a CVE row is found; change the logic in the CVE enrichment block (the section that checks finding.cve_id, runs sqlx::query and uses row.get("title")) so it preserves the original finding.title and merges or appends the DB title instead of overwriting it (for example, set finding.title to a combination like "{scanner_title} — {cve_title}" or only append if scanner_title is non-empty); ensure you still read row.get("title") and only modify finding.title when appropriate, keeping references to finding, finding.title, cve_id, row.get("title"), and state_task.db to locate the code.extensions/vscode/src/extension.ts (2)
95-99: Avoidanycast for ReadableStream reader.The
(body as any).getReader()cast triggers a lint warning and bypasses type checking. Consider using a more specific type assertion:💡 Proposed fix
const body = sseResponse.body; if (!body) throw new Error('Failed to connect to event stream'); - const reader = (body as any).getReader(); + const reader = (body as ReadableStream<Uint8Array>).getReader(); const decoder = new TextDecoder();This preserves type safety while acknowledging the web streams API.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extensions/vscode/src/extension.ts` around lines 95 - 99, The code casts sseResponse.body to any to call getReader(), which silences type checks; instead assert a concrete Web Streams type and call getReader() with proper typing. Replace the any cast by treating sseResponse.body as a ReadableStream<Uint8Array> (or ReadableStream<Uint8Array> | null-guard) before calling getReader() so the reader variable is typed correctly; update the null check on body and use that typed stream for (body as ReadableStream<Uint8Array>).getReader() (or narrow body first) so the Web Streams API is used with type safety in extension.ts around the reader/decoder logic.
153-156: Diagnostic range doesn't highlight the vulnerable code.The range
(line, 0, line, 0)creates a zero-width marker that won't visually highlight the problematic code. Consider usingline_endfrom the finding to create a proper range:💡 Suggested improvement
const line = Math.max(0, f.line_start - 1); - const range = new vscode.Range(line, 0, line, 0); // TODO: improve range mapping + const endLine = Math.max(line, f.line_end - 1); + const lineText = document.lineAt(line).text; + const range = new vscode.Range(line, 0, endLine, lineText.length);Would you like me to help implement a more accurate range mapping that considers the actual code span?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extensions/vscode/src/extension.ts` around lines 153 - 156, The current range uses a zero-width marker (const range = new vscode.Range(line, 0, line, 0)); update the mapping to use the finding's actual span: derive startLine = Math.max(0, f.line_start - 1) and endLine = Math.max(startLine, f.line_end - 1), and use column offsets from f.col_start/f.col_end (falling back to 0 and Number.MAX_SAFE_INTEGER or the document line length if absent) to construct new vscode.Range(startLine, startCol, endLine, endCol) so the diagnostic highlights the full vulnerable span (replace usage of line, range with this new range calculation).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/cli/src/main.rs`:
- Around line 185-195: The Show branch (ConfigAction::Show) currently serializes
and prints the full config (variable config) via serde_json::to_string_pretty,
leaking secrets like ai_api_key; before printing, create a redacted copy or
mapping of the config where sensitive fields (e.g., ai_api_key) are replaced
with a masked value such as "<redacted>" or a partial mask, then serialize and
print that redacted structure (reference symbols: ConfigAction::Show, config,
ZenvraConfig::get_path, serde_json::to_string_pretty) so the path still shows
but secret contents are not exposed.
In `@crates/scanner/src/lib.rs`:
- Around line 94-113: The spawned background task calling scan_stream may exit
without sending ScanEvent::Complete or ScanEvent::Error, causing the receiver
loop in scan to silently stop without signaling failure; update the tokio::spawn
block that calls scan_stream(config_clone, tx) so it captures the Result
returned by scan_stream and always sends an explicit final event over the tx
before exiting: if scan_stream returns Err send ScanEvent::Error with the error
(using the same error message type currently used by ScanEvent::Error), and if
it returns Ok ensure ScanEvent::Complete is sent (only if not already sent by
scan_stream), so the receiver in scan (which matches on
ScanEvent::{Finding,Complete,Error}) always receives a terminal event instead of
silently losing the sender.
In `@extensions/vscode/src/sidebarProvider.ts`:
- Around line 197-206: The code in the 'finding' case uses item.innerHTML to
inject message.data.title and message.data.severity into the DOM (creating
sevClass from severity), which allows XSS; instead, build DOM nodes and set
textContent for user-supplied values and only use a safe/sanitized class name
for severity. Replace the innerHTML usage in the 'finding' case by creating span
elements (e.g., severitySpan and titleStrong) and append them to item using
severitySpan.textContent = message.data.severity and titleStrong.textContent =
message.data.title; compute sevClass by sanitizing message.data.severity (e.g.,
strip/replace non-alphanumeric characters or map known severities) before
assigning item.className or severitySpan.className, and then append item to
findingsList.
---
Outside diff comments:
In `@crates/cli/src/main.rs`:
- Around line 46-48: The help text for the CLI option ai_model (the ai_model
field annotated with #[arg(long)]) is inconsistent with the actual Anthropic
default model used elsewhere (the default string set for Anthropic models, e.g.,
"claude-3-5-sonnet-20240620"); update either the ai_model help text to mention
the real default ("claude-3-5-sonnet-20240620") or change the Anthropic default
constant/value to match the help text ("claude-sonnet-4-20250514") so both the
#[arg(long)] ai_model description and the Anthropic default value reference the
same model name. Ensure the same change is applied wherever the Anthropic
default model string is defined/used (the default assignment for Anthropic
model).
---
Nitpick comments:
In `@crates/server/src/main.rs`:
- Around line 102-105: The CORS setup uses CorsLayer::new().allow_origin(Any)
which is too permissive for production; update the cors construction (variable
cors) to read allowed origins from configuration or an environment variable and
pass a restricted origin list instead of Any (e.g., build a HeaderValue/Origin
list or use Any only in dev), and apply allow_methods/allow_headers as needed;
locate the CorsLayer::new() call and replace allow_origin(Any) with logic that
parses a configured ALLOWED_ORIGINS and constructs the appropriate allow_origin
value for production.
- Around line 200-212: The enrichment code unconditionally replaces the
scanner-generated finding.title when a CVE row is found; change the logic in the
CVE enrichment block (the section that checks finding.cve_id, runs sqlx::query
and uses row.get("title")) so it preserves the original finding.title and merges
or appends the DB title instead of overwriting it (for example, set
finding.title to a combination like "{scanner_title} — {cve_title}" or only
append if scanner_title is non-empty); ensure you still read row.get("title")
and only modify finding.title when appropriate, keeping references to finding,
finding.title, cve_id, row.get("title"), and state_task.db to locate the code.
In `@extensions/vscode/src/extension.ts`:
- Around line 95-99: The code casts sseResponse.body to any to call getReader(),
which silences type checks; instead assert a concrete Web Streams type and call
getReader() with proper typing. Replace the any cast by treating
sseResponse.body as a ReadableStream<Uint8Array> (or ReadableStream<Uint8Array>
| null-guard) before calling getReader() so the reader variable is typed
correctly; update the null check on body and use that typed stream for (body as
ReadableStream<Uint8Array>).getReader() (or narrow body first) so the Web
Streams API is used with type safety in extension.ts around the reader/decoder
logic.
- Around line 153-156: The current range uses a zero-width marker (const range =
new vscode.Range(line, 0, line, 0)); update the mapping to use the finding's
actual span: derive startLine = Math.max(0, f.line_start - 1) and endLine =
Math.max(startLine, f.line_end - 1), and use column offsets from
f.col_start/f.col_end (falling back to 0 and Number.MAX_SAFE_INTEGER or the
document line length if absent) to construct new vscode.Range(startLine,
startCol, endLine, endCol) so the diagnostic highlights the full vulnerable span
(replace usage of line, range with this new range calculation).
In `@extensions/vscode/src/sidebarProvider.ts`:
- Around line 46-50: The postMessage method currently accepts unknown which
reduces type safety; update the signature of postMessage to accept a
discriminated union matching the extension's message shapes (e.g., an
interface/union for { type: 'progress' | 'finding' | 'complete', data?: unknown
}) and use that type for the message parameter, then pass it through to
this._view.webview.postMessage(message) unchanged; modify any callers to conform
if necessary and add the new Message type (or a namespaced alias) near the
SidebarProvider class so references like postMessage and
this._view.webview.postMessage are clearly typed.
🪄 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
Run ID: 5ae14d28-59df-42a4-8461-c5881ac6406f
📒 Files selected for processing (16)
apps/web/eslint.config.jsapps/web/src/routes/history/+page.svelteapps/web/src/routes/scan/+page.svelteapps/web/src/routes/settings/ai/+page.svelteapps/web/src/routes/settings/cve/+page.sveltecrates/cli/src/main.rscrates/scanner/src/ai/mod.rscrates/scanner/src/ai/openai.rscrates/scanner/src/engine.rscrates/scanner/src/lib.rscrates/server/src/cve_sync/mod.rscrates/server/src/main.rsextensions/vscode/package.jsonextensions/vscode/src/extension.tsextensions/vscode/src/sidebarProvider.tsmigrations/202604040001_create_vulnerabilities_table.sql
🚧 Files skipped from review as they are similar to previous changes (8)
- crates/scanner/src/ai/openai.rs
- crates/scanner/src/ai/mod.rs
- apps/web/src/routes/settings/ai/+page.svelte
- migrations/202604040001_create_vulnerabilities_table.sql
- crates/server/src/cve_sync/mod.rs
- crates/scanner/src/engine.rs
- apps/web/src/routes/scan/+page.svelte
- extensions/vscode/package.json
| ConfigAction::Show => { | ||
| use colored::Colorize; | ||
| println!("{}", "Zenvra CLI Configuration:".bold()); | ||
| println!( | ||
| " Path: {}", | ||
| ZenvraConfig::get_path().display().to_string().dimmed() | ||
| ); | ||
| println!(); | ||
| let json = serde_json::to_string_pretty(&config)?; | ||
| println!("{}", json); | ||
| } |
There was a problem hiding this comment.
config show leaks API keys in plaintext.
The Show action serializes the entire config including ai_api_key and prints it to the terminal. This exposes secrets in terminal history and logs, similar to the issue previously flagged for the Set action.
🔒 Proposed fix to redact sensitive fields when displaying config
ConfigAction::Show => {
use colored::Colorize;
println!("{}", "Zenvra CLI Configuration:".bold());
println!(
" Path: {}",
ZenvraConfig::get_path().display().to_string().dimmed()
);
println!();
- let json = serde_json::to_string_pretty(&config)?;
- println!("{}", json);
+ // Create a redacted copy for display
+ let display_config = serde_json::json!({
+ "ai_provider": config.ai_provider,
+ "ai_api_key": config.ai_api_key.as_ref().map(|_| "********"),
+ "ai_model": config.ai_model,
+ "ai_endpoint": config.ai_endpoint,
+ });
+ let json = serde_json::to_string_pretty(&display_config)?;
+ println!("{}", json);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ConfigAction::Show => { | |
| use colored::Colorize; | |
| println!("{}", "Zenvra CLI Configuration:".bold()); | |
| println!( | |
| " Path: {}", | |
| ZenvraConfig::get_path().display().to_string().dimmed() | |
| ); | |
| println!(); | |
| let json = serde_json::to_string_pretty(&config)?; | |
| println!("{}", json); | |
| } | |
| ConfigAction::Show => { | |
| use colored::Colorize; | |
| println!("{}", "Zenvra CLI Configuration:".bold()); | |
| println!( | |
| " Path: {}", | |
| ZenvraConfig::get_path().display().to_string().dimmed() | |
| ); | |
| println!(); | |
| // Create a redacted copy for display | |
| let display_config = serde_json::json!({ | |
| "ai_provider": config.ai_provider, | |
| "ai_api_key": config.ai_api_key.as_ref().map(|_| "********"), | |
| "ai_model": config.ai_model, | |
| "ai_endpoint": config.ai_endpoint, | |
| }); | |
| let json = serde_json::to_string_pretty(&display_config)?; | |
| println!("{}", json); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/cli/src/main.rs` around lines 185 - 195, The Show branch
(ConfigAction::Show) currently serializes and prints the full config (variable
config) via serde_json::to_string_pretty, leaking secrets like ai_api_key;
before printing, create a redacted copy or mapping of the config where sensitive
fields (e.g., ai_api_key) are replaced with a masked value such as "<redacted>"
or a partial mask, then serialize and print that redacted structure (reference
symbols: ConfigAction::Show, config, ZenvraConfig::get_path,
serde_json::to_string_pretty) so the path still shows but secret contents are
not exposed.
| pub async fn scan(config: &ScanConfig) -> anyhow::Result<Vec<Finding>> { | ||
| let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel(); | ||
| let config_clone = config.clone(); | ||
|
|
||
| // Run scan in background and collect findings | ||
| tokio::spawn(async move { | ||
| let _ = scan_stream(config_clone, tx).await; | ||
| }); | ||
|
|
||
| let mut findings = Vec::new(); | ||
| while let Some(event) = rx.recv().await { | ||
| match event { | ||
| ScanEvent::Finding(f) => findings.push(*f), | ||
| ScanEvent::Complete => break, | ||
| ScanEvent::Error(e) => return Err(anyhow::anyhow!(e)), | ||
| _ => {} | ||
| } | ||
| } | ||
|
|
||
| Ok(findings) |
There was a problem hiding this comment.
Potential hang if scan_stream fails before sending Complete or Error.
If scan_stream returns early due to an error before sending ScanEvent::Complete or ScanEvent::Error, the spawned task drops the sender tx. The while let Some(event) = rx.recv().await loop will then return None and exit cleanly, but only because the channel closes. However, if an error occurs after sending some findings but before Complete, the caller won't know the scan terminated abnormally.
Consider ensuring that scan errors are always propagated via the channel:
💡 Suggested improvement
tokio::spawn(async move {
- let _ = scan_stream(config_clone, tx).await;
+ if let Err(e) = scan_stream(config_clone, tx.clone()).await {
+ let _ = tx.send(ScanEvent::Error(e.to_string()));
+ }
});Note: scan_stream already sends ScanEvent::Error on engine::run_stream failure (line 50), but errors from AI provider enrichment (lines 59-72) don't send an error event if they cause early termination.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/scanner/src/lib.rs` around lines 94 - 113, The spawned background task
calling scan_stream may exit without sending ScanEvent::Complete or
ScanEvent::Error, causing the receiver loop in scan to silently stop without
signaling failure; update the tokio::spawn block that calls
scan_stream(config_clone, tx) so it captures the Result returned by scan_stream
and always sends an explicit final event over the tx before exiting: if
scan_stream returns Err send ScanEvent::Error with the error (using the same
error message type currently used by ScanEvent::Error), and if it returns Ok
ensure ScanEvent::Complete is sent (only if not already sent by scan_stream), so
the receiver in scan (which matches on ScanEvent::{Finding,Complete,Error})
always receives a terminal event instead of silently losing the sender.
| case 'finding': | ||
| const item = document.createElement('div'); | ||
| item.className = 'finding-item'; | ||
| const sevClass = 'severity-' + message.data.severity.toLowerCase(); | ||
| item.innerHTML = \` | ||
| <span class="finding-severity \${sevClass}">\${message.data.severity}</span> | ||
| <strong>\${message.data.title}</strong> | ||
| \`; | ||
| findingsList.appendChild(item); | ||
| break; |
There was a problem hiding this comment.
Potential XSS via innerHTML with finding data.
The message.data.title and message.data.severity are inserted into the DOM via innerHTML without escaping. If a finding title contains HTML/script content, it could execute in the webview context.
🔒 Proposed fix using textContent
case 'finding':
- const item = document.createElement('div');
- item.className = 'finding-item';
- const sevClass = 'severity-' + message.data.severity.toLowerCase();
- item.innerHTML = \`
- <span class="finding-severity \${sevClass}">\${message.data.severity}</span>
- <strong>\${message.data.title}</strong>
- \`;
+ const item = document.createElement('div');
+ item.className = 'finding-item';
+ const sevClass = 'severity-' + message.data.severity.toLowerCase();
+ const sevSpan = document.createElement('span');
+ sevSpan.className = 'finding-severity ' + sevClass;
+ sevSpan.textContent = message.data.severity;
+ const titleEl = document.createElement('strong');
+ titleEl.textContent = message.data.title;
+ item.appendChild(sevSpan);
+ item.appendChild(titleEl);
findingsList.appendChild(item);
break;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@extensions/vscode/src/sidebarProvider.ts` around lines 197 - 206, The code in
the 'finding' case uses item.innerHTML to inject message.data.title and
message.data.severity into the DOM (creating sevClass from severity), which
allows XSS; instead, build DOM nodes and set textContent for user-supplied
values and only use a safe/sanitized class name for severity. Replace the
innerHTML usage in the 'finding' case by creating span elements (e.g.,
severitySpan and titleStrong) and append them to item using
severitySpan.textContent = message.data.severity and titleStrong.textContent =
message.data.title; compute sevClass by sanitizing message.data.severity (e.g.,
strip/replace non-alphanumeric characters or map known severities) before
assigning item.className or severitySpan.className, and then append item to
findingsList.
… VS Code extension build utilities
CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
There was a problem hiding this comment.
Actionable comments posted: 12
♻️ Duplicate comments (2)
crates/scanner/src/lib.rs (1)
183-185:⚠️ Potential issue | 🟠 Major
scan()can still report success after an internal scan failure.This spawned task drops the
scan_streamresult, so failures before a terminalScanEventis sent — for exampleai::create_provider(...)returningErr— just close the channel and letscan()fall through toOk(findings). Please forward the error ontotxbefore the task exits.Suggested fix
tokio::spawn(async move { - let _ = scan_stream(config_clone, tx).await; + if let Err(e) = scan_stream(config_clone, tx.clone()).await { + let _ = tx.send(ScanEvent::Error(e.to_string())); + } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/scanner/src/lib.rs` around lines 183 - 185, The spawned task currently ignores the Result from scan_stream so scan() can return Ok(...) even if scan_stream failed; change the task body in the tokio::spawn block that calls scan_stream(config_clone, tx) so that it matches on the Result and sends an error-bearing ScanEvent (or otherwise forwards the error) onto tx before exiting (e.g., convert Err from ai::create_provider(...) or other internal errors into a terminal ScanEvent and tx.send it), ensuring scan_stream and scan() observe failures rather than silently closing the channel.crates/cli/src/main.rs (1)
186-195:⚠️ Potential issue | 🟠 Major
config showstill printsai_api_keyin plaintext.This branch serializes the full config, so the saved API key ends up in terminal history, logs, and screenshots. The previous review already flagged this and the current code still exposes it.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/cli/src/main.rs` around lines 186 - 195, The ConfigAction::Show branch currently serializes and prints the full `config` (via serde_json::to_string_pretty), exposing `ai_api_key`; instead, create a redacted copy or transform of the config before serializing (e.g., clone `config`, set `ai_api_key` to a masked value like "****" or an empty string) and then call serde_json::to_string_pretty on that redacted object; update the println block that shows the path (ZenvraConfig::get_path) and the JSON print to use the redacted config to ensure secrets are never printed to stdout or saved in shell history.
🧹 Nitpick comments (1)
extensions/vscode/build-vsix.cjs (1)
18-18: Avoid hardcoding the VSIX version in the output filename.Line 18 duplicates version data already in
extensions/vscode/package.json; this will drift on the next version bump.Proposed fix
+const pkg = require('./package.json'); @@ - vsce.main(['package', '--no-git-check', '-o', 'zenvra-0.1.1-rc.2.vsix']).then(() => { + vsce.main(['package', '--no-git-check', '-o', `zenvra-${pkg.version}.vsix`]).then(() => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extensions/vscode/build-vsix.cjs` at line 18, The build script hardcodes the VSIX filename in the vsce.main(['package', ...]) call causing version drift; update the script to read the extension version from extensions/vscode/package.json (parse JSON and extract the "version" field) and construct the output name dynamically (e.g. `zenvra-${version}.vsix`) before calling vsce.main (the existing vsce.main invocation should use that computed filename), using path/fs to resolve the package.json location and failing loudly if version is missing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/cli/src/main.rs`:
- Around line 137-145: ZenvraConfig::load currently swallows malformed JSON by
using unwrap_or_default; change load to return a Result<Self, Error> (or
Result<Self, anyhow::Error>) and propagate errors from std::fs::read_to_string
and serde_json::from_str instead of falling back to Self::default(); update
callers to handle the Result. Specifically, modify the ZenvraConfig::load
signature to return Result<Self, _>, use Self::get_path() then map
read_to_string errors and pass the file content into serde_json::from_str
returning its Err on parse failure (only use Self::default() explicitly where a
true missing-file case is desired and handled), and update code that calls
ZenvraConfig::load to handle the error (e.g., surface to the user).
In `@crates/scanner/src/ai/mod.rs`:
- Around line 42-49: Incoming JSON uses "provider": "openai" but ProviderKind's
current serde rename expects "open_ai", causing deserialization failures when
ScanRequest contains ai_config: Option<AiConfig>. Fix by updating the
ProviderKind enum to accept the extension's token (e.g., add a serde
rename/alias for the OpenAi variant such as #[serde(rename = "openai")] or
#[serde(alias = "openai")]) so deserialization of AiConfig::provider succeeds;
keep references to AiConfig and ScanRequest intact.
In `@crates/scanner/src/lib.rs`:
- Around line 119-125: The error logging in the engine::run_stream match arm
uses file_config.file_path.as_ref().unwrap(), which can panic; change the log to
avoid unwrap by using file_config.file_path.as_deref().unwrap_or("<unknown>")
(or capture the original file path earlier into a local Option/str and use
unwrap_or("<unknown>")) so tracing::error prints a safe path string without
risking a panic in the error path of run_stream.
In `@crates/server/src/main.rs`:
- Around line 85-89: The connection error message currently includes the full
DATABASE_URL (db_url) which may leak credentials; update the error handling
around PgPoolOptions::new().connect(&db_url).await (the pool creation) to avoid
logging db_url directly — either log a generic message like "Failed to connect
to PostgreSQL" or extract and log only a redacted host/database identifier (no
user/password) before creating the anyhow::anyhow! error; ensure the change
touches the closure passed to .map_err so the secret never appears in the error
string.
- Around line 337-397: The workspace scan loop currently never moves the
completed scan into in-memory state or removes its live sender; replicate the
single-file completion behavior by, when handling ScanEvent::Complete in the
spawned task, inserting the collected findings (or a ScanResult-like value) into
state_task.results under scan_id and removing the live sender from
state_task.scans (and drop/close the sender if required) before breaking; locate
this logic around the ScanEvent::Complete match branch in the tokio::spawn block
and perform the state_task.results.insert(scan_id, findings) and
state_task.scans.remove(&scan_id) operations (using the same types/structures as
single-file scan finalization).
- Around line 229-246: The current SQL insert in sqlx::query persists full
source for each finding (columns vulnerable_code and fixed_code bound from
finding.vulnerable_code and finding.fixed_code); change it to avoid storing raw
code by removing those columns and .bind calls, instead persist only metadata
and a short snippet or hash (e.g., compute a sha256 or store a truncated
snippet) derived from finding.vulnerable_code/fixed_code and bind that value;
update the INSERT column list and corresponding .bind(...) calls in the block
that executes against state_task.db (and the scan_results schema if needed) so
only scan_id, engine, cve_id, cwe_id, severity, title, description, file_path,
line_start/line_end and the new snippet_hash/snippet column are stored.
In `@extensions/vscode/build-vsix.cjs`:
- Line 15: The code imports the internal build output path via "const vsce =
require('@vscode/vsce/out/main')" which is unsupported; replace that require
with the public entry: use "const vsce = require('@vscode/vsce')" and update
downstream calls to use the public API (e.g. call vsce.createVSIX(...) for
packaging or vsce.publish/listFiles as appropriate), ensuring any options/return
handling matches the public function signatures used by createVSIX/publish.
In `@extensions/vscode/package-vsix.cjs`:
- Line 21: The current env assignment ({ ...process.env, NODE_OPTIONS:
'--no-warnings' }) overwrites any existing NODE_OPTIONS; change it to merge the
existing process.env.NODE_OPTIONS with the '--no-warnings' flag instead of
replacing it: read process.env.NODE_OPTIONS, append '--no-warnings' when it's
not already present, and set that combined string as NODE_OPTIONS in the env
spread so existing CI/runtime flags are preserved; update the env construction
that sets NODE_OPTIONS accordingly.
- Around line 3-11: The polyfill for global.File only affects the parent process
but vsce is invoked via child processes using execSync (symbol: execSync) so
undici in Node 18+ fails; replace the execSync-based invocations of "vsce" with
an in-process invocation using the programmatic API (as implemented in
build-vsix.cjs) so the polyfilled global.File is present; locate the execSync
calls in package-vsix.cjs and refactor them to require/import and call the same
programmatic functions used by build-vsix.cjs (or directly use the vsce package
API) rather than spawning a subprocess.
In `@extensions/vscode/src/extension.ts`:
- Around line 77-104: The workspace discovery misses some scanner-supported
types, doesn't match Dockerfile, and incorrectly defaults extensionless files to
'js'; update the supportedExtensions array to include all CLI-scanned extensions
(add missing ones like 'dockerfile' as both lowercase and an explicit filename
match if needed), change globPattern to also account for filename-only matches
(e.g., include '**/Dockerfile' or adjust pattern to support names without
extensions), and in the files -> workspaceFiles mapping replace the naive
fallback ext = relativePath.split('.').pop() || 'js' with logic that detects
extensionless filenames (like 'Dockerfile') or returns an explicit language
mapping (use the file basename check and a lookup map) so extensionless files
are classified correctly (refer to supportedExtensions, globPattern,
excludePattern, files, and workspaceFiles).
- Around line 156-193: The inner try/catch is swallowing server 'error' events
because throw new Error(event.data) is caught immediately; change the flow so
'error' events are propagated to the outer handler instead of being thrown
inside the parse block — e.g., in the switch for event.type === 'error' set a
local variable (e.g., serverError = event.data), post a message to
sidebarProvider ({ type: 'error', data: event.data }) and after the try block,
if serverError is set call the outer error handling path (throw new
Error(serverError) or invoke the same handler used elsewhere) so
updateDiagnosticsForUri, status messages and the outer error handler see the
failure; avoid removing JSON.parse error handling — only move the
throw/propagation outside the catch scope.
- Around line 147-156: The SSE parsing in scanWorkspace and scanDocument
discards fragments when a chunk splits an event; fix both by maintaining a
persistent string buffer outside the reader loop, append decoder.decode(value,
{stream: true}) to that buffer each iteration, then split buffer on '\n' and
process every complete line except the final partial fragment (keep that
trailing piece as the new buffer for the next loop); after reader.done is true,
process any remaining complete lines and handle a final partial line if it forms
a full SSE event. Update the parsing blocks in scanWorkspace and scanDocument to
use this buffered-accumulation approach so "data: " JSON is never lost across
chunk boundaries.
---
Duplicate comments:
In `@crates/cli/src/main.rs`:
- Around line 186-195: The ConfigAction::Show branch currently serializes and
prints the full `config` (via serde_json::to_string_pretty), exposing
`ai_api_key`; instead, create a redacted copy or transform of the config before
serializing (e.g., clone `config`, set `ai_api_key` to a masked value like
"****" or an empty string) and then call serde_json::to_string_pretty on that
redacted object; update the println block that shows the path
(ZenvraConfig::get_path) and the JSON print to use the redacted config to ensure
secrets are never printed to stdout or saved in shell history.
In `@crates/scanner/src/lib.rs`:
- Around line 183-185: The spawned task currently ignores the Result from
scan_stream so scan() can return Ok(...) even if scan_stream failed; change the
task body in the tokio::spawn block that calls scan_stream(config_clone, tx) so
that it matches on the Result and sends an error-bearing ScanEvent (or otherwise
forwards the error) onto tx before exiting (e.g., convert Err from
ai::create_provider(...) or other internal errors into a terminal ScanEvent and
tx.send it), ensuring scan_stream and scan() observe failures rather than
silently closing the channel.
---
Nitpick comments:
In `@extensions/vscode/build-vsix.cjs`:
- Line 18: The build script hardcodes the VSIX filename in the
vsce.main(['package', ...]) call causing version drift; update the script to
read the extension version from extensions/vscode/package.json (parse JSON and
extract the "version" field) and construct the output name dynamically (e.g.
`zenvra-${version}.vsix`) before calling vsce.main (the existing vsce.main
invocation should use that computed filename), using path/fs to resolve the
package.json location and failing loudly if version is missing.
🪄 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
Run ID: 0b883fff-6097-4c83-a084-fa31a9a0130a
📒 Files selected for processing (13)
crates/cli/src/main.rscrates/scanner/src/ai/mod.rscrates/scanner/src/lib.rscrates/server/src/main.rsextensions/vscode/LICENSEextensions/vscode/build-vsix.cjsextensions/vscode/package-vsix.cjsextensions/vscode/package.jsonextensions/vscode/polyfill-build.cjsextensions/vscode/src/extension.tsextensions/vscode/src/sidebarProvider.tsextensions/vscode/src/types.tsextensions/vscode/zenvra-0.1.1-rc.2.vsix
✅ Files skipped from review due to trivial changes (2)
- extensions/vscode/LICENSE
- extensions/vscode/polyfill-build.cjs
🚧 Files skipped from review as they are similar to previous changes (3)
- extensions/vscode/src/sidebarProvider.ts
- extensions/vscode/package.json
- extensions/vscode/src/types.ts
| impl ZenvraConfig { | ||
| fn load() -> Self { | ||
| let config_path = Self::get_path(); | ||
| if let Ok(content) = std::fs::read_to_string(config_path) { | ||
| serde_json::from_str(&content).unwrap_or_default() | ||
| } else { | ||
| Self::default() | ||
| } | ||
| } |
There was a problem hiding this comment.
Surface invalid config instead of silently resetting it.
unwrap_or_default() turns a malformed config file into an empty config, so scans silently ignore saved settings and the next config set can overwrite the broken file. Returning Result<Self> here would preserve that failure mode for the caller.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/cli/src/main.rs` around lines 137 - 145, ZenvraConfig::load currently
swallows malformed JSON by using unwrap_or_default; change load to return a
Result<Self, Error> (or Result<Self, anyhow::Error>) and propagate errors from
std::fs::read_to_string and serde_json::from_str instead of falling back to
Self::default(); update callers to handle the Result. Specifically, modify the
ZenvraConfig::load signature to return Result<Self, _>, use Self::get_path()
then map read_to_string errors and pass the file content into
serde_json::from_str returning its Err on parse failure (only use
Self::default() explicitly where a true missing-file case is desired and
handled), and update code that calls ZenvraConfig::load to handle the error
(e.g., surface to the user).
| #[serde(rename_all = "snake_case")] | ||
| pub struct AiConfig { | ||
| /// Which provider to use. | ||
| pub provider: ProviderKind, | ||
|
|
||
| /// API key for the provider. | ||
| #[serde(alias = "apiKey")] | ||
| pub api_key: String, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, examine the ProviderKind enum definition
cat -n crates/scanner/src/ai/mod.rs | head -100Repository: Cameroon-Developer-Network/Zenvra
Length of output: 4143
🏁 Script executed:
# Search for all uses of "openai" string in the codebase
rg '"openai"' --type rust --type typescriptRepository: Cameroon-Developer-Network/Zenvra
Length of output: 111
🏁 Script executed:
# Check the ProviderKind enum more carefully, looking for serde attributes
ast-grep --pattern 'enum ProviderKind { $$$ }'Repository: Cameroon-Developer-Network/Zenvra
Length of output: 59
🏁 Script executed:
# Verify serde behavior by looking at how ProviderKind is currently configured
rg -A 10 'enum ProviderKind'Repository: Cameroon-Developer-Network/Zenvra
Length of output: 672
🏁 Script executed:
# Search for "openai" in Rust files
rg '"openai"' crates/ --type rustRepository: Cameroon-Developer-Network/Zenvra
Length of output: 229
🏁 Script executed:
# Also check for openai in uppercase or mixed case
rg -i 'openai|open_ai' crates/ --type rust | head -50Repository: Cameroon-Developer-Network/Zenvra
Length of output: 3531
🏁 Script executed:
# Check the CLI main.rs and server main.rs for provider handling
cat -n crates/cli/src/main.rs | sed -n '330,360p'Repository: Cameroon-Developer-Network/Zenvra
Length of output: 1423
🏁 Script executed:
# Check server main.rs as well
cat -n crates/server/src/main.rs | sed -n '500,520p'Repository: Cameroon-Developer-Network/Zenvra
Length of output: 1039
🏁 Script executed:
# Look for extension.ts
find . -name 'extension.ts' -type fRepository: Cameroon-Developer-Network/Zenvra
Length of output: 113
🏁 Script executed:
# Check if there are any tests that validate the serde behavior
rg -A 5 'test.*openai|test.*provider' crates/scanner --type rustRepository: Cameroon-Developer-Network/Zenvra
Length of output: 59
🏁 Script executed:
# Check the VSCode extension for "openai" usage
cat -n extensions/vscode/src/extension.ts | grep -i openaiRepository: Cameroon-Developer-Network/Zenvra
Length of output: 59
🏁 Script executed:
# Check what the server payload type is for the models endpoint
cat -n crates/server/src/main.rs | sed -n '490,505p'Repository: Cameroon-Developer-Network/Zenvra
Length of output: 601
🏁 Script executed:
# Search for where AiConfig might be deserialized from JSON
rg 'AiConfig' crates/ --type rust -B 2 -A 2Repository: Cameroon-Developer-Network/Zenvra
Length of output: 2455
🏁 Script executed:
# Check if there are any tests or examples that deserialize AiConfig
rg -A 10 'serde_json.*AiConfig|from_str.*AiConfig|from_json.*AiConfig' crates/ --type rustRepository: Cameroon-Developer-Network/Zenvra
Length of output: 59
🏁 Script executed:
# Look for where ScanRequest is used as input to a server endpoint
rg 'ScanRequest' crates/server/src/main.rs -B 5 -A 10Repository: Cameroon-Developer-Network/Zenvra
Length of output: 1515
🏁 Script executed:
# Check if there's a POST endpoint that accepts ScanRequest
rg 'post\|Post\|Json<ScanRequest' crates/server/src/main.rsRepository: Cameroon-Developer-Network/Zenvra
Length of output: 59
🏁 Script executed:
# Look at the ScanRequest definition in lib.rs
cat -n crates/scanner/src/lib.rs | sed -n '1,150p'Repository: Cameroon-Developer-Network/Zenvra
Length of output: 5819
🏁 Script executed:
# Check git history to see if AiConfig or ProviderKind serde attributes changed recently
git log --oneline -20 -- crates/scanner/src/ai/mod.rsRepository: Cameroon-Developer-Network/Zenvra
Length of output: 188
🏁 Script executed:
# Check if there are any actual tests that send JSON with provider field
rg -A 10 'test.*ai_config\|#\[test\]' crates/scanner/src/ai/ --type rustRepository: Cameroon-Developer-Network/Zenvra
Length of output: 59
🏁 Script executed:
# Look for any example configs or documentation showing expected JSON format
find . -name '*.json' -o -name '*.md' | xargs grep -l 'ai_config\|provider' 2>/dev/null | head -10Repository: Cameroon-Developer-Network/Zenvra
Length of output: 158
🏁 Script executed:
# Check if the server actually deserializes AiConfig from incoming JSON
cat -n crates/server/src/main.rs | sed -n '200,250p'Repository: Cameroon-Developer-Network/Zenvra
Length of output: 2826
🏁 Script executed:
# Check VSCode extension for how it sends AI config to the server
cat -n extensions/vscode/src/extension.tsRepository: Cameroon-Developer-Network/Zenvra
Length of output: 14601
🏁 Script executed:
# Also check if there are any example requests or API documentation
cat -n CHANGELOG.md | head -100Repository: Cameroon-Developer-Network/Zenvra
Length of output: 1582
🏁 Script executed:
# More specifically, search for any place in the codebase where JSON is sent with provider
rg 'provider.*openai|"provider"' extensions/ docs/ --type typescript --type jsonRepository: Cameroon-Developer-Network/Zenvra
Length of output: 111
openai JSON payloads from the VSCode extension will fail to deserialize.
The extension sends JSON with "provider": "openai", but ProviderKind with #[serde(rename_all = "snake_case")] expects "open_ai". When the server deserializes the incoming ScanRequest with ai_config: Option<AiConfig>, the enum variant deserialization will fail.
Suggested fix
pub enum ProviderKind {
Anthropic,
+ #[serde(rename = "openai", alias = "open_ai")]
OpenAi,
Google,
Custom,
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/scanner/src/ai/mod.rs` around lines 42 - 49, Incoming JSON uses
"provider": "openai" but ProviderKind's current serde rename expects "open_ai",
causing deserialization failures when ScanRequest contains ai_config:
Option<AiConfig>. Fix by updating the ProviderKind enum to accept the
extension's token (e.g., add a serde rename/alias for the OpenAi variant such as
#[serde(rename = "openai")] or #[serde(alias = "openai")]) so deserialization of
AiConfig::provider succeeds; keep references to AiConfig and ScanRequest intact.
| let raw_findings = match engine::run_stream(&file_config, tx.clone()).await { | ||
| Ok(f) => f, | ||
| Err(e) => { | ||
| tracing::error!( | ||
| "Engine failed for {}: {}", | ||
| file_config.file_path.as_ref().unwrap(), | ||
| e |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Avoid unwrap() in this library error path.
file_path is populated today, but panicking while handling an engine failure makes this path brittle for future callers. Log as_deref().unwrap_or("<unknown>") or keep the original file.path around instead. As per coding guidelines, "crates/{scanner,server}/**/*.rs: Do not use .unwrap() or .expect() in library and API code".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/scanner/src/lib.rs` around lines 119 - 125, The error logging in the
engine::run_stream match arm uses file_config.file_path.as_ref().unwrap(), which
can panic; change the log to avoid unwrap by using
file_config.file_path.as_deref().unwrap_or("<unknown>") (or capture the original
file path earlier into a local Option/str and use unwrap_or("<unknown>")) so
tracing::error prints a safe path string without risking a panic in the error
path of run_stream.
| let pool = PgPoolOptions::new() | ||
| .max_connections(20) | ||
| .connect(&db_url) | ||
| .await | ||
| .map_err(|e| anyhow::anyhow!("Failed to connect to PostgreSQL at {}: {}", db_url, e))?; |
There was a problem hiding this comment.
Do not include DATABASE_URL in the connection error.
If the URL contains credentials, this path leaks them into logs and process output on startup failures. Prefer a generic message or log only a redacted host/database identifier.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/server/src/main.rs` around lines 85 - 89, The connection error message
currently includes the full DATABASE_URL (db_url) which may leak credentials;
update the error handling around PgPoolOptions::new().connect(&db_url).await
(the pool creation) to avoid logging db_url directly — either log a generic
message like "Failed to connect to PostgreSQL" or extract and log only a
redacted host/database identifier (no user/password) before creating the
anyhow::anyhow! error; ensure the change touches the closure passed to .map_err
so the secret never appears in the error string.
| // Persist individual finding | ||
| if let Err(e) = sqlx::query( | ||
| "INSERT INTO scan_results (scan_id, engine, cve_id, cwe_id, severity, title, description, vulnerable_code, fixed_code, line_start, line_end, file_path) | ||
| VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12)" | ||
| ) | ||
| .bind(scan_id) | ||
| .bind(format!("{:?}", finding.engine)) | ||
| .bind(&finding.cve_id) | ||
| .bind(&finding.cwe_id) | ||
| .bind(finding.severity.to_string()) | ||
| .bind(&finding.title) | ||
| .bind(&finding.description) | ||
| .bind(&finding.vulnerable_code) | ||
| .bind(&finding.fixed_code) | ||
| .bind(finding.line_start as i32) | ||
| .bind(finding.line_end as i32) | ||
| .bind(&finding.file_path) | ||
| .execute(&state_task.db) |
There was a problem hiding this comment.
Stop persisting raw code in scan_results.
This stores full vulnerable_code (and the generated replacement) in PostgreSQL for every finding. That creates long-lived retention of user source and any embedded secrets; the workspace path below repeats the same pattern. Persist metadata/snippets/hashes only, and keep full code transient in the live scan stream if you still need it. Based on learnings: "Do not store raw code in the database longer than needed — store scan results only".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/server/src/main.rs` around lines 229 - 246, The current SQL insert in
sqlx::query persists full source for each finding (columns vulnerable_code and
fixed_code bound from finding.vulnerable_code and finding.fixed_code); change it
to avoid storing raw code by removing those columns and .bind calls, instead
persist only metadata and a short snippet or hash (e.g., compute a sha256 or
store a truncated snippet) derived from finding.vulnerable_code/fixed_code and
bind that value; update the INSERT column list and corresponding .bind(...)
calls in the block that executes against state_task.db (and the scan_results
schema if needed) so only scan_id, engine, cve_id, cwe_id, severity, title,
description, file_path, line_start/line_end and the new snippet_hash/snippet
column are stored.
| if (typeof global.File === 'undefined') { | ||
| global.File = class File extends Blob { | ||
| constructor(blobParts, fileName, options = {}) { | ||
| super(blobParts, options); | ||
| this.name = fileName; | ||
| this.lastModified = options.lastModified || Date.now(); | ||
| } | ||
| }; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n extensions/vscode/package-vsix.cjsRepository: Cameroon-Developer-Network/Zenvra
Length of output: 1183
🏁 Script executed:
# Check for Node.js version constraints
cat package.json | grep -A 5 '"node"' || echo "No explicit node version found in package.json"
# Check if there are any error logs or comments about undici/File issues
rg -i "undici|file.*error|node.*18" --type json --type ts --type js --max-count 5Repository: Cameroon-Developer-Network/Zenvra
Length of output: 185
🏁 Script executed:
# Find package.json locations
fd package.json
# Also check if there's a .nvmrc or similar
fd -e nvmrc -e versionsRepository: Cameroon-Developer-Network/Zenvra
Length of output: 129
🏁 Script executed:
# Check the vscode extension package.json
cat extensions/vscode/package.json | head -50
# Check if vsce is a dependency and what version
grep -A 2 -B 2 "vsce\|node.*version" extensions/vscode/package.json || echo "Not found with grep"Repository: Cameroon-Developer-Network/Zenvra
Length of output: 1574
🏁 Script executed:
# Search for any error logs or issues related to File/undici
rg -i "file|undici|error" extensions/vscode --type json --type md --type txt --max-count 10
# Check GitHub workflows to see if Node 18 is used
fd -e yml -e yaml | xargs grep -l "node.*version\|18\|20" 2>/dev/null || echo "No version info found"
# Check if there are any existing issues or TODOs about this
rg -i "polyfill|child.*process|vsce" extensions/vscodeRepository: Cameroon-Developer-Network/Zenvra
Length of output: 4247
🏁 Script executed:
cat -n extensions/vscode/build-vsix.cjs
# Also check package.json scripts to see which one is actually used
grep -A 5 '"scripts"' extensions/vscode/package.jsonRepository: Cameroon-Developer-Network/Zenvra
Length of output: 1327
🏁 Script executed:
# Check if either .cjs file is referenced elsewhere or called by npm scripts
grep -r "package-vsix\|build-vsix" extensions/vscode/ --exclude-dir=node_modules --exclude="*.lock*"
# Check git history to see which file is newer
git log --oneline -n 20 -- extensions/vscode/package-vsix.cjs extensions/vscode/build-vsix.cjs 2>/dev/null || echo "Git history not available"Repository: Cameroon-Developer-Network/Zenvra
Length of output: 273
🏁 Script executed:
# Check CI/build files for references
fd -e yml -e yaml -e json | xargs grep -l "package-vsix\|build-vsix" 2>/dev/null || echo "No CI references found"
# Check if there are any npm scripts or other references
grep -r "package-vsix\|build-vsix" extensions/vscode --exclude-dir=node_modules --exclude="*.lock*" --exclude="*.vsix"
# Check the git diff or status of these files
git log --oneline --all -- "extensions/vscode/package-vsix.cjs" "extensions/vscode/build-vsix.cjs" 2>/dev/null | head -10Repository: Cameroon-Developer-Network/Zenvra
Length of output: 211
global.File polyfill does not reach the vsce process spawned via execSync.
Lines 3–11 polyfill global.File in the parent process, but lines 19–22 and line 26 execute vsce in child processes via execSync(). Child processes do not inherit JavaScript globals from the parent; they get a fresh Node.js runtime. This will fail on Node 18+ where undici (a dependency of vsce) requires the File class.
The fix is to run vsce in-process using its programmatic API rather than spawning it as a subprocess. This approach is already implemented in build-vsix.cjs in the same directory.
Example fix
const { execSync } = require('child_process');
+const { File } = require('buffer');
// Polyfill File for undici in Node 18
if (typeof global.File === 'undefined') {
- global.File = class File extends Blob {
- constructor(blobParts, fileName, options = {}) {
- super(blobParts, options);
- this.name = fileName;
- this.lastModified = options.lastModified || Date.now();
- }
- };
+ global.File = File;
}
console.log('Starting packaging...');
-try {
- execSync('npx -y `@vscode/vsce` package --no-git-check', {
- stdio: 'inherit',
- env: { ...process.env, NODE_OPTIONS: '--no-warnings' }
- });
-} catch (e) {
- console.error('Packaging failed, trying fallback...');
- execSync('./node_modules/.bin/vsce package --no-git-check', { stdio: 'inherit' });
-}
+try {
+ const vsce = require('@vscode/vsce/out/main');
+ vsce.main(['package', '--no-git-check']).then(() => {
+ console.log('VSIX generated successfully');
+ }).catch(err => {
+ console.error('Packaging failed:', err);
+ process.exit(1);
+ });
+} catch (e) {
+ console.error('Failed to load vsce:', e);
+ process.exit(1);
+}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@extensions/vscode/package-vsix.cjs` around lines 3 - 11, The polyfill for
global.File only affects the parent process but vsce is invoked via child
processes using execSync (symbol: execSync) so undici in Node 18+ fails; replace
the execSync-based invocations of "vsce" with an in-process invocation using the
programmatic API (as implemented in build-vsix.cjs) so the polyfilled
global.File is present; locate the execSync calls in package-vsix.cjs and
refactor them to require/import and call the same programmatic functions used by
build-vsix.cjs (or directly use the vsce package API) rather than spawning a
subprocess.
| // Try to run vsce via npx | ||
| execSync('npx -y @vscode/vsce package --no-git-check', { | ||
| stdio: 'inherit', | ||
| env: { ...process.env, NODE_OPTIONS: '--no-warnings' } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd extensions/vscode && head -30 package-vsix.cjs | cat -nRepository: Cameroon-Developer-Network/Zenvra
Length of output: 1183
🏁 Script executed:
wc -l extensions/vscode/package-vsix.cjsRepository: Cameroon-Developer-Network/Zenvra
Length of output: 114
🏁 Script executed:
sed -n '1,35p' extensions/vscode/package-vsix.cjs | cat -nRepository: Cameroon-Developer-Network/Zenvra
Length of output: 1183
🏁 Script executed:
rg "NODE_OPTIONS" --type js --type cjs --type mjsRepository: Cameroon-Developer-Network/Zenvra
Length of output: 104
🏁 Script executed:
rg "process\.env" extensions/vscode/ --type js --type cjs --type mjs -A 2Repository: Cameroon-Developer-Network/Zenvra
Length of output: 104
🏁 Script executed:
rg "NODE_OPTIONS" .Repository: Cameroon-Developer-Network/Zenvra
Length of output: 176
🏁 Script executed:
rg "process\.env" extensions/vscode/ -A 2Repository: Cameroon-Developer-Network/Zenvra
Length of output: 266
Merge NODE_OPTIONS instead of overwriting.
Line 21 overwrites any existing NODE_OPTIONS from the parent process environment. CI and runtime environments frequently set NODE_OPTIONS with essential flags (heap limits, security flags, etc.), and this code will drop them.
Proposed fix
- env: { ...process.env, NODE_OPTIONS: '--no-warnings' }
+ env: {
+ ...process.env,
+ NODE_OPTIONS: [process.env.NODE_OPTIONS, '--no-warnings'].filter(Boolean).join(' ')
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| env: { ...process.env, NODE_OPTIONS: '--no-warnings' } | |
| env: { | |
| ...process.env, | |
| NODE_OPTIONS: [process.env.NODE_OPTIONS, '--no-warnings'].filter(Boolean).join(' ') | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@extensions/vscode/package-vsix.cjs` at line 21, The current env assignment ({
...process.env, NODE_OPTIONS: '--no-warnings' }) overwrites any existing
NODE_OPTIONS; change it to merge the existing process.env.NODE_OPTIONS with the
'--no-warnings' flag instead of replacing it: read process.env.NODE_OPTIONS,
append '--no-warnings' when it's not already present, and set that combined
string as NODE_OPTIONS in the env spread so existing CI/runtime flags are
preserved; update the env construction that sets NODE_OPTIONS accordingly.
| const supportedExtensions = [ | ||
| 'py', 'js', 'mjs', 'cjs', 'ts', 'tsx', 'jsx', 'rs', 'go', 'java', | ||
| 'cs', 'cpp', 'cc', 'c', 'h', 'rb', 'php', 'swift', 'kt', 'kts', | ||
| 'yaml', 'yml', 'toml', 'json', 'xml', 'env', 'sh', 'bash', 'zsh', | ||
| 'dockerfile', 'svelte', 'vue' | ||
| ]; | ||
|
|
||
| const globPattern = `**/*.{${supportedExtensions.join(',')}}`; | ||
| const excludePattern = '{**/node_modules/**,**/target/**,**/.git/**,**/dist/**,**/build/**}'; | ||
|
|
||
| const files = await vscode.workspace.findFiles(globPattern, excludePattern, 100); // Limit to 100 for now | ||
|
|
||
| if (files.length === 0) { | ||
| vscode.window.showInformationMessage('Zenvra: No scannable files found in workspace.'); | ||
| return; | ||
| } | ||
|
|
||
| const workspaceFiles: WorkspaceFile[] = await Promise.all( | ||
| files.map(async (uri) => { | ||
| const content = await vscode.workspace.fs.readFile(uri); | ||
| const relativePath = vscode.workspace.asRelativePath(uri); | ||
| const ext = relativePath.split('.').pop() || 'js'; | ||
|
|
||
| return { | ||
| path: relativePath, | ||
| code: Buffer.from(content).toString('utf8'), | ||
| language: ext | ||
| }; |
There was a problem hiding this comment.
Workspace file discovery no longer matches the scanner's supported types.
This list is missing several extensions the CLI scans, and the brace glob will not match Dockerfile at all. On top of that, extensionless files fall back to 'js', so even if they were discovered they'd be classified incorrectly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@extensions/vscode/src/extension.ts` around lines 77 - 104, The workspace
discovery misses some scanner-supported types, doesn't match Dockerfile, and
incorrectly defaults extensionless files to 'js'; update the supportedExtensions
array to include all CLI-scanned extensions (add missing ones like 'dockerfile'
as both lowercase and an explicit filename match if needed), change globPattern
to also account for filename-only matches (e.g., include '**/Dockerfile' or
adjust pattern to support names without extensions), and in the files ->
workspaceFiles mapping replace the naive fallback ext =
relativePath.split('.').pop() || 'js' with logic that detects extensionless
filenames (like 'Dockerfile') or returns an explicit language mapping (use the
file basename check and a lookup map) so extensionless files are classified
correctly (refer to supportedExtensions, globPattern, excludePattern, files, and
workspaceFiles).
| while (true) { | ||
| const { done, value } = await reader.read(); | ||
| if (done) break; | ||
|
|
||
| const chunk = decoder.decode(value, { stream: true }); | ||
| const lines = chunk.split('\n'); | ||
|
|
||
| for (const line of lines) { | ||
| if (line.startsWith('data: ')) { | ||
| try { |
There was a problem hiding this comment.
❓ Verification inconclusive
Script executed:
cat -n extensions/vscode/src/extension.ts | head -180 | tail -50Repository: Cameroon-Developer-Network/Zenvra
Repository: Cameroon-Developer-Network/Zenvra
Exit code: 0
stdout:
131 throw new Error(errorMsg || response.statusText);
132 }
133
134 const { scan_id } = (await response.json()) as { scan_id: string };
135
136 // Subscribe to SSE stream
137 const sseResponse = await fetch(`${apiUrl}/api/v1/scan/${scan_id}/events`);
138 const body = sseResponse.body;
139 if (!body) throw new Error('Failed to connect to event stream');
140
141 const reader = (body as any).getReader();
142 const decoder = new TextDecoder();
143 const allFindings: Record<string, Finding[]> = {};
144
145 sidebarProvider.postMessage({ type: 'progress', data: { message: `Scanning ${files.length} files...`, percentage: 10 } });
146
147 while (true) {
148 const { done, value } = await reader.read();
149 if (done) break;
150
151 const chunk = decoder.decode(value, { stream: true });
152 const lines = chunk.split('\n');
153
154 for (const line of lines) {
155 if (line.startsWith('data: ')) {
156 try {
157 const event = JSON.parse(line.slice(6));
158
159 switch (event.type) {
160 case 'progress':
161 vscode.window.setStatusBarMessage(`$(sync~spin) Zenvra: ${event.data.message}`, 2000);
162 sidebarProvider.postMessage({ type: 'progress', data: event.data });
163 break;
164 case 'finding': {
165 const finding = event.data as Finding;
166 const filePath = finding.file_path || 'unknown';
167 if (!allFindings[filePath]) {
168 allFindings[filePath] = [];
169 }
170 allFindings[filePath].push(finding);
171
172 // Update diagnostics for this specific file
173 const workspaceFolder = vscode.workspace.workspaceFolders?.[0];
174 if (workspaceFolder) {
175 const fileUri = vscode.Uri.joinPath(workspaceFolder.uri, filePath);
176 updateDiagnosticsForUri(fileUri, allFindings[filePath]);
177 }
178
179 sidebarProvider.postMessage({ type: 'finding', data: finding });
180 break;
Script executed:
sed -n '240,270p' extensions/vscode/src/extension.tsRepository: Cameroon-Developer-Network/Zenvra
Repository: Cameroon-Developer-Network/Zenvra
Exit code: 0
stdout:
const { scan_id } = (await response.json()) as { scan_id: string };
// Subscribe to SSE stream
const sseResponse = await fetch(`${apiUrl}/api/v1/scan/${scan_id}/events`);
const body = sseResponse.body;
if (!body) throw new Error('Failed to connect to event stream');
const reader = (body as any).getReader();
const decoder = new TextDecoder();
const findings: Finding[] = [];
while (true) {
const { done, value } = await reader.read();
if (done) break;
const chunk = decoder.decode(value, { stream: true });
const lines = chunk.split('\n');
for (const line of lines) {
if (line.startsWith('data: ')) {
try {
const event = JSON.parse(line.slice(6));
switch (event.type) {
case 'progress':
vscode.window.setStatusBarMessage(`$(sync~spin) Zenvra: ${event.data.message}`, 2000);
// Also notify sidebar
sidebarProvider.postMessage({ type: 'progress', data: event.data });
break;
case 'finding':
Script executed:
rg -n "while \(true\)" extensions/vscode/src/extension.ts | head -5Repository: Cameroon-Developer-Network/Zenvra
Repository: Cameroon-Developer-Network/Zenvra
Exit code: 0
stdout:
147: while (true) {
252: while (true) {
Script executed:
sed -n '147,200p' extensions/vscode/src/extension.tsRepository: Cameroon-Developer-Network/Zenvra
Repository: Cameroon-Developer-Network/Zenvra
Exit code: 0
stdout:
while (true) {
const { done, value } = await reader.read();
if (done) break;
const chunk = decoder.decode(value, { stream: true });
const lines = chunk.split('\n');
for (const line of lines) {
if (line.startsWith('data: ')) {
try {
const event = JSON.parse(line.slice(6));
switch (event.type) {
case 'progress':
vscode.window.setStatusBarMessage(`$(sync~spin) Zenvra: ${event.data.message}`, 2000);
sidebarProvider.postMessage({ type: 'progress', data: event.data });
break;
case 'finding': {
const finding = event.data as Finding;
const filePath = finding.file_path || 'unknown';
if (!allFindings[filePath]) {
allFindings[filePath] = [];
}
allFindings[filePath].push(finding);
// Update diagnostics for this specific file
const workspaceFolder = vscode.workspace.workspaceFolders?.[0];
if (workspaceFolder) {
const fileUri = vscode.Uri.joinPath(workspaceFolder.uri, filePath);
updateDiagnosticsForUri(fileUri, allFindings[filePath]);
}
sidebarProvider.postMessage({ type: 'finding', data: finding });
break;
}
case 'complete': {
const totalCount = Object.values(allFindings).flat().length;
vscode.window.setStatusBarMessage(`$(shield) Zenvra: Workspace scan complete (${totalCount} issues)`, 5000);
sidebarProvider.postMessage({ type: 'complete' });
return;
}
case 'error':
throw new Error(event.data);
}
} catch (e) {
console.error('Error parsing SSE event:', e);
}
}
}
}
} catch (err: unknown) {
const errorMsg = err instanceof Error ? err.message : String(err);
vscode.window.showErrorMessage(`Zenvra Workspace Scan Failed: ${errorMsg}`);
}
Script executed:
sed -n '252,305p' extensions/vscode/src/extension.tsRepository: Cameroon-Developer-Network/Zenvra
Repository: Cameroon-Developer-Network/Zenvra
Exit code: 0
stdout:
while (true) {
const { done, value } = await reader.read();
if (done) break;
const chunk = decoder.decode(value, { stream: true });
const lines = chunk.split('\n');
for (const line of lines) {
if (line.startsWith('data: ')) {
try {
const event = JSON.parse(line.slice(6));
switch (event.type) {
case 'progress':
vscode.window.setStatusBarMessage(`$(sync~spin) Zenvra: ${event.data.message}`, 2000);
// Also notify sidebar
sidebarProvider.postMessage({ type: 'progress', data: event.data });
break;
case 'finding':
findings.push(event.data);
updateDiagnostics(document, findings);
sidebarProvider.postMessage({ type: 'finding', data: event.data });
break;
case 'complete': {
const count = findings.length;
if (count === 0) {
vscode.window.setStatusBarMessage('$(shield) Zenvra: No issues found', 3000);
} else {
vscode.window.setStatusBarMessage(`$(warning) Zenvra: Found ${count} issue(s)`, 3000);
}
sidebarProvider.postMessage({ type: 'complete' });
return;
}
case 'error':
throw new Error(event.data);
}
} catch (e) {
console.error('Error parsing SSE event:', e);
}
}
}
}
} catch (err: unknown) {
const errorMsg = err instanceof Error ? err.message : String(err);
vscode.window.showErrorMessage(`Zenvra Scan Failed: ${errorMsg}`);
vscode.window.setStatusBarMessage('$(error) Zenvra: Scan failed', 3000);
}
}
function updateDiagnostics(document: vscode.TextDocument, findings: Finding[]): void {
updateDiagnosticsForUri(document.uri, findings);
}
function updateDiagnosticsForUri(uri: vscode.Uri, findings: Finding[]): void {
Buffer SSE data across chunks before parsing it.
Both scanWorkspace (lines 147–200) and scanDocument (lines 252–305) share identical SSE parsing logic that splits each decoded chunk on \n. ReadableStream chunks are arbitrary in size and can fragment SSE events across boundaries. For example, a chunk may end mid-JSON (data: {"foo"), with the rest arriving in the next chunk. Splitting each chunk independently causes these fragments to be orphaned—they won't match the data: prefix check and are silently discarded. This causes intermittent loss of findings, progress, and complete events.
Accumulate incomplete lines in a buffer and only parse complete lines once they end with \n.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@extensions/vscode/src/extension.ts` around lines 147 - 156, The SSE parsing
in scanWorkspace and scanDocument discards fragments when a chunk splits an
event; fix both by maintaining a persistent string buffer outside the reader
loop, append decoder.decode(value, {stream: true}) to that buffer each
iteration, then split buffer on '\n' and process every complete line except the
final partial fragment (keep that trailing piece as the new buffer for the next
loop); after reader.done is true, process any remaining complete lines and
handle a final partial line if it forms a full SSE event. Update the parsing
blocks in scanWorkspace and scanDocument to use this buffered-accumulation
approach so "data: " JSON is never lost across chunk boundaries.
| try { | ||
| const event = JSON.parse(line.slice(6)); | ||
|
|
||
| switch (event.type) { | ||
| case 'progress': | ||
| vscode.window.setStatusBarMessage(`$(sync~spin) Zenvra: ${event.data.message}`, 2000); | ||
| sidebarProvider.postMessage({ type: 'progress', data: event.data }); | ||
| break; | ||
| case 'finding': { | ||
| const finding = event.data as Finding; | ||
| const filePath = finding.file_path || 'unknown'; | ||
| if (!allFindings[filePath]) { | ||
| allFindings[filePath] = []; | ||
| } | ||
| allFindings[filePath].push(finding); | ||
|
|
||
| // Update diagnostics for this specific file | ||
| const workspaceFolder = vscode.workspace.workspaceFolders?.[0]; | ||
| if (workspaceFolder) { | ||
| const fileUri = vscode.Uri.joinPath(workspaceFolder.uri, filePath); | ||
| updateDiagnosticsForUri(fileUri, allFindings[filePath]); | ||
| } | ||
|
|
||
| sidebarProvider.postMessage({ type: 'finding', data: finding }); | ||
| break; | ||
| } | ||
| case 'complete': { | ||
| const totalCount = Object.values(allFindings).flat().length; | ||
| vscode.window.setStatusBarMessage(`$(shield) Zenvra: Workspace scan complete (${totalCount} issues)`, 5000); | ||
| sidebarProvider.postMessage({ type: 'complete' }); | ||
| return; | ||
| } | ||
| case 'error': | ||
| throw new Error(event.data); | ||
| } | ||
| } catch (e) { | ||
| console.error('Error parsing SSE event:', e); | ||
| } |
There was a problem hiding this comment.
Do not swallow server error events inside the parse loop.
Right now throw new Error(event.data) is immediately caught by the inner catch and only logged as a parse error. That means failed scans never hit the outer error handler, so diagnostics/status can stay stale and the user gets no failure message.
Also applies to: 261-290
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@extensions/vscode/src/extension.ts` around lines 156 - 193, The inner
try/catch is swallowing server 'error' events because throw new
Error(event.data) is caught immediately; change the flow so 'error' events are
propagated to the outer handler instead of being thrown inside the parse block —
e.g., in the switch for event.type === 'error' set a local variable (e.g.,
serverError = event.data), post a message to sidebarProvider ({ type: 'error',
data: event.data }) and after the try block, if serverError is set call the
outer error handling path (throw new Error(serverError) or invoke the same
handler used elsewhere) so updateDiagnosticsForUri, status messages and the
outer error handler see the failure; avoid removing JSON.parse error handling —
only move the throw/propagation outside the catch scope.
This PR finalizes the Phase 2 development milestone for Zenvra. Key highlights include:
🚀 Features
🛠️ Improvements
OnceLock.AGENTS.mdand added aCHANGELOG.mdto track progress.📦 Infrastructure
Created by Antigravity AI
Summary by CodeRabbit
New Features
Enhancements
Chores