-
Notifications
You must be signed in to change notification settings - Fork 0
Feat: phase 3 polish sse #7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| import { writable } from 'svelte/store'; | ||
| import { getHistory } from '$lib/api'; | ||
|
|
||
| export const scanCount = writable<number>(0); | ||
|
|
||
| export async function refreshScanCount() { | ||
| try { | ||
| const history = await getHistory(); | ||
| scanCount.set(history.length); | ||
| } catch (e) { | ||
| console.error('Failed to refresh scan count:', e); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,16 @@ | ||
| <script lang="ts"> | ||
| import "../app.css"; | ||
| import { page } from "$app/state"; | ||
| import { scanCount, refreshScanCount } from "$lib/stores/usage"; | ||
| import { onMount } from "svelte"; | ||
| let { children } = $props(); | ||
|
|
||
| const maxScans = 10; | ||
|
|
||
| onMount(async () => { | ||
| await refreshScanCount(); | ||
| }); | ||
|
|
||
| const navItems = [ | ||
| { name: "Dashboard", href: "/", icon: "layout-grid", disabled: false }, | ||
| { name: "Scan Code", href: "/scan", icon: "search", disabled: false }, | ||
|
|
@@ -56,9 +64,9 @@ | |
| <p class="text-xs font-semibold text-zinc-500 uppercase tracking-wider mb-2">Usage Plan</p> | ||
| <p class="text-sm font-medium mb-1">Free Tier</p> | ||
| <div class="w-full bg-zinc-800 h-1.5 rounded-full overflow-hidden mt-2"> | ||
| <div class="bg-brand-primary w-2/5 h-full rounded-full shadow-[0_0_8px_rgba(236,72,153,0.5)]"></div> | ||
| <div class="bg-brand-primary h-full rounded-full shadow-[0_0_8px_rgba(236,72,153,0.5)]" style="width: {($scanCount / maxScans) * 100}%"></div> | ||
| </div> | ||
| <p class="text-[10px] text-zinc-500 mt-2">4/10 scans remaining</p> | ||
| <p class="text-[10px] text-zinc-500 mt-2">{maxScans - $scanCount}/{maxScans} scans remaining</p> | ||
|
Comment on lines
+67
to
+69
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Clamp the usage math once the free tier is exhausted. If 🤖 Prompt for AI Agents |
||
| </div> | ||
| </div> | ||
| </aside> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| <script lang="ts"> | ||
| import { type Finding } from "$lib/api"; | ||
| import { aiConfig } from "$lib/stores/aiConfig.svelte"; | ||
| import { refreshScanCount } from "$lib/stores/usage"; | ||
|
|
||
| const LANGUAGES = [ | ||
| { value: "python", label: "Python" }, | ||
|
|
@@ -20,6 +21,7 @@ | |
| ]; | ||
|
|
||
| let selectedLanguage = $state("python"); | ||
| let selectedMinSeverity = $state("info"); | ||
|
|
||
|
Comment on lines
23
to
25
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 2. No zod for minseverity A new user input (selectedMinSeverity / min_severity) is introduced and sent to the API without Zod-based validation. This weakens input validation guarantees and violates the Zod-only validation requirement for forms/API inputs. Agent Prompt
|
||
| let code = $state(`// Paste your code here to scan for vulnerabilities | ||
| def get_user(user_id): | ||
|
|
@@ -69,7 +71,8 @@ def get_user(user_id): | |
| api_key: aiConfig.apiKey, | ||
| model: aiConfig.model, | ||
| endpoint: aiConfig.endpoint || undefined, | ||
| } : undefined | ||
| } : undefined, | ||
| min_severity: selectedMinSeverity !== "info" ? selectedMinSeverity : undefined | ||
| }) | ||
|
Comment on lines
71
to
76
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 1. Direct fetch in +page.svelte The scan page modifies a direct fetch request instead of using the typed API client wrapper, reducing consistency and type safety for frontend network calls. This violates the requirement to route frontend network access through apps/web/src/lib/api.ts. Agent Prompt
|
||
| }); | ||
|
|
||
|
|
@@ -95,6 +98,7 @@ def get_user(user_id): | |
| scanStatus = "Scan complete!"; | ||
| isScanning = false; | ||
| eventSource.close(); | ||
| refreshScanCount(); | ||
| break; | ||
| case 'error': | ||
| scanStatus = `Error: ${data.data}`; | ||
|
|
@@ -106,8 +110,13 @@ def get_user(user_id): | |
|
|
||
| eventSource.onerror = () => { | ||
| console.error("SSE connection failed"); | ||
| isScanning = false; | ||
| eventSource.close(); | ||
| scanStatus = "Connection lost. Reconnecting or finished."; | ||
| // We don't necessarily want to set isScanning to false here because SSE often reconnects automatically. | ||
| // But for Zenvra's simple model, we'll close it to avoid zombie states if the scan is actually done. | ||
| if (eventSource.readyState === EventSource.CLOSED || eventSource.readyState === EventSource.CONNECTING) { | ||
| isScanning = false; | ||
| eventSource.close(); | ||
| } | ||
|
Comment on lines
111
to
+119
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🌐 Web query:
💡 Result: readyState === EventSource.CONNECTING during the onerror event means the EventSource connection has encountered an error (e.g., network issue, server error) and the browser has set the state to CONNECTING to automatically attempt reconnection, as per the WHATWG HTML spec. This is the normal reconnection process: the browser will retry after a delay (default ~3s, adjustable via retry: field), keeping readyState as CONNECTING until success (OPEN) or permanent failure (CLOSED). The client should NOT close the connection in onerror when readyState is CONNECTING. Closing it (es.close) sets readyState to CLOSED, permanently stopping retries. Official docs (MDN, WHATWG) do not recommend closing; they advise logging/handling the error and letting the browser retry automatically for transient failures. Only close for fatal/permanent errors (when readyState === CLOSED) or per app logic. Many implementations add manual reconnection logic only if readyState === CLOSED to handle cases where browser stops retrying (e.g., certain HTTP errors like 500 in some browsers). Citations:
Remove
Current problematic codeif (eventSource.readyState === EventSource.CLOSED || eventSource.readyState === EventSource.CONNECTING) {
isScanning = false;
eventSource.close();
}Only close if 🤖 Prompt for AI Agents |
||
| }; | ||
|
|
||
| } catch (error) { | ||
|
|
@@ -144,7 +153,8 @@ def get_user(user_id): | |
| api_key: aiConfig.apiKey, | ||
| model: aiConfig.model, | ||
| endpoint: aiConfig.endpoint || undefined, | ||
| } : undefined | ||
| } : undefined, | ||
| min_severity: selectedMinSeverity !== "info" ? selectedMinSeverity : undefined | ||
| }) | ||
| }); | ||
|
|
||
|
|
@@ -168,6 +178,7 @@ def get_user(user_id): | |
| scanStatus = "Scan complete!"; | ||
| isScanning = false; | ||
| eventSource.close(); | ||
| refreshScanCount(); | ||
| break; | ||
| case 'error': | ||
| scanStatus = `Error: ${data.data}`; | ||
|
|
@@ -325,6 +336,21 @@ def get_user(user_id): | |
| </select> | ||
| </div> | ||
|
|
||
| <!-- Severity Filter --> | ||
| <div class="flex items-center gap-3"> | ||
| <label class="text-xs font-bold text-zinc-500 uppercase tracking-widest whitespace-nowrap">Min Severity</label> | ||
| <select | ||
| bind:value={selectedMinSeverity} | ||
| class="glass bg-zinc-900/80 px-3 py-2 rounded-xl border border-zinc-800 text-xs font-medium text-zinc-300 focus:ring-2 ring-brand-primary outline-none transition-all flex-1" | ||
| > | ||
| <option value="info">All (Info+)</option> | ||
| <option value="low">Low+</option> | ||
| <option value="medium">Medium+</option> | ||
| <option value="high">High+</option> | ||
| <option value="critical">Critical Only</option> | ||
| </select> | ||
| </div> | ||
|
|
||
| <div class="flex-1 glass rounded-2xl overflow-hidden border-zinc-800 flex flex-col relative group"> | ||
| <div class="px-6 py-3 border-b border-border bg-zinc-900/50 flex items-center justify-between"> | ||
| <span class="text-xs font-bold text-zinc-500 uppercase tracking-widest">Input Code</span> | ||
|
|
@@ -352,6 +378,20 @@ def get_user(user_id): | |
| </label> | ||
| </div> | ||
|
|
||
| <div class="px-6 py-3 border-b border-border bg-zinc-900/50 flex items-center justify-between"> | ||
| <span class="text-xs font-bold text-zinc-500 uppercase tracking-widest">Min Severity Filter</span> | ||
| <select | ||
| bind:value={selectedMinSeverity} | ||
| class="bg-transparent text-xs font-bold text-zinc-400 outline-none cursor-pointer" | ||
| > | ||
| <option value="info" class="bg-zinc-900">All Levels</option> | ||
| <option value="low" class="bg-zinc-900">Low+</option> | ||
| <option value="medium" class="bg-zinc-900">Medium+</option> | ||
| <option value="high" class="bg-zinc-900">High+</option> | ||
| <option value="critical" class="bg-zinc-900">Critical</option> | ||
| </select> | ||
| </div> | ||
|
|
||
| <div class="flex-1 overflow-y-auto p-4 space-y-2 custom-scrollbar"> | ||
| {#if workspaceFiles.length === 0} | ||
| <div class="flex flex-col items-center justify-center h-full text-center space-y-3 opacity-50 py-12"> | ||
|
|
@@ -391,6 +431,12 @@ def get_user(user_id): | |
| {#if findings.length > 0} | ||
| <button onclick={() => { findings = []; scanProgress = 0; scanStatus = "Ready to scan"; }} class="text-xs text-zinc-500 hover:text-zinc-300 transition-colors">Clear</button> | ||
| {/if} | ||
| {#if isScanning} | ||
| <div class="flex items-center gap-2"> | ||
| <span class="w-1.5 h-1.5 rounded-full bg-emerald-500 animate-pulse"></span> | ||
| <span class="text-[10px] font-black text-emerald-500 uppercase tracking-tighter">Live Stream</span> | ||
| </div> | ||
| {/if} | ||
| </div> | ||
|
|
||
| <div class="flex-1 overflow-y-auto p-6 space-y-4 custom-scrollbar"> | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -112,8 +112,8 @@ fn build_rules() -> Vec<AiCodeRule> { | |||||||||||||||||||||||||||||||||||
| AiCodeRule { | ||||||||||||||||||||||||||||||||||||
| name: "Unauthenticated Route Handler", | ||||||||||||||||||||||||||||||||||||
| regex: NO_AUTH_ROUTE_REGEX.get_or_init(|| { | ||||||||||||||||||||||||||||||||||||
| Regex::new(r"(?i)@(app|router)\.(delete|put|patch)\s*\([^)]+\)\s*\nasync\s+def\s+[a-z_]+\((?!.*\bauth\b|.*\bdepends\b|.*\btoken\b|.*\buser\b)") | ||||||||||||||||||||||||||||||||||||
| .unwrap() | ||||||||||||||||||||||||||||||||||||
| // Simplified: detect the decorator. Manual filtering below. | ||||||||||||||||||||||||||||||||||||
| Regex::new(r"(?i)@(app|router)\.(delete|put|patch)\s*\(").unwrap() | ||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+115
to
117
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion | 🟠 Major Avoid introducing new Line 116 and Line 155 add new panic paths in library code. Please propagate regex construction errors instead of unwrapping. Proposed direction-fn build_rules() -> Vec<AiCodeRule> {
+fn build_rules() -> Result<Vec<AiCodeRule>> {
vec![
AiCodeRule {
name: "Unauthenticated Route Handler",
- regex: NO_AUTH_ROUTE_REGEX.get_or_init(|| {
- Regex::new(r"(?i)@(app|router)\.(delete|put|patch)\s*\(").unwrap()
- }).clone(),
+ regex: Regex::new(r"(?i)@(app|router)\.(delete|put|patch)\s*\(")?,
...
},
AiCodeRule {
name: "Plain HTTP Endpoint (No TLS)",
- regex: PLAIN_HTTP_REGEX.get_or_init(|| {
- Regex::new(r#"(?i)(url\s*=\s*['"]http://|fetch\s*\(\s*['"]http://)"#).unwrap()
- }).clone(),
+ regex: Regex::new(r#"(?i)(url\s*=\s*['"]http://|fetch\s*\(\s*['"]http://)"#)?,
...
},
]
}
...
- let rules = build_rules();
+ let rules = build_rules()?;As per coding guidelines: Also applies to: 154-156 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||
| .clone(), | ||||||||||||||||||||||||||||||||||||
| severity: Severity::Medium, | ||||||||||||||||||||||||||||||||||||
|
|
@@ -151,8 +151,8 @@ fn build_rules() -> Vec<AiCodeRule> { | |||||||||||||||||||||||||||||||||||
| AiCodeRule { | ||||||||||||||||||||||||||||||||||||
| name: "Plain HTTP Endpoint (No TLS)", | ||||||||||||||||||||||||||||||||||||
| regex: PLAIN_HTTP_REGEX.get_or_init(|| { | ||||||||||||||||||||||||||||||||||||
| Regex::new(r#"(?i)(url\s*=\s*['"]http://(?!localhost|127\.0\.0\.1)|fetch\s*\(\s*['"]http://(?!localhost))"#) | ||||||||||||||||||||||||||||||||||||
| .unwrap() | ||||||||||||||||||||||||||||||||||||
| // Simplified: detect http:// without localhost/127.0.0.1 | ||||||||||||||||||||||||||||||||||||
| Regex::new(r#"(?i)(url\s*=\s*['"]http://|fetch\s*\(\s*['"]http://)"#).unwrap() | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+154
to
+155
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Plain HTTP regex became too narrow and likely regresses detection coverage. The new pattern only catches Broader pattern while keeping local-host suppression in
|
||||||||||||||||||||||||||||||||||||
| // Simplified: detect http:// without localhost/127.0.0.1 | |
| Regex::new(r#"(?i)(url\s*=\s*['"]http://|fetch\s*\(\s*['"]http://)"#).unwrap() | |
| // Simplified: detect http:// without localhost/127.0.0.1 | |
| Regex::new(r#"(?i)['"]http://[^'"]+['"]"#).unwrap() |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/scanner/src/engines/ai_code.rs` around lines 154 - 155, The current
Regex::new(...) in crates/scanner/src/engines/ai_code.rs is too narrow (only
matches `url = "http://..."` and `fetch("http://..."`); broaden the pattern used
when constructing the Regex in the code (the Regex::new call) to match other
common HTTP usages like function calls and attribute accesses (e.g.,
`requests.get("http://...")`, `axios.get("http://...")`, plain quoted
`"http://..."`, and similar call/assignment patterns) while keeping the existing
localhost/127.0.0.1 suppression logic inside the run function intact; update the
Regex::new invocation to a more general case-insensitive pattern that captures
quoted http:// URLs and typical call forms and leave the filtering in run
unchanged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
user keyword suppression is over-broad and drops real findings.
At Line 210, l.contains("user") suppresses valid unauthenticated mutation routes like @router.delete("/users/{id}"), creating false negatives.
Targeted fix
- if l.contains("auth")
- || l.contains("depends")
- || l.contains("token")
- || l.contains("user")
+ if l.contains("auth")
+ || l.contains("depends(")
+ || l.contains("token")
{
continue;
}📝 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.
| if rule.name == "Unauthenticated Route Handler" { | |
| let l = line.to_lowercase(); | |
| if l.contains("auth") | |
| || l.contains("depends") | |
| || l.contains("token") | |
| || l.contains("user") | |
| { | |
| continue; | |
| } | |
| if rule.name == "Unauthenticated Route Handler" { | |
| let l = line.to_lowercase(); | |
| if l.contains("auth") | |
| || l.contains("depends(") | |
| || l.contains("token") | |
| { | |
| continue; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/scanner/src/engines/ai_code.rs` around lines 205 - 213, The
suppression check is too broad: l.contains("user") hides valid routes like
"/users/{id}"; update the condition inside the rule.name == "Unauthenticated
Route Handler" block to only suppress when "user" appears as an auth-related
token or standalone identifier (not as part of "users"). Replace
l.contains("user") with a stricter match (e.g., a regex word-boundary check or
explicit tokens) such as testing
r"\b(user|current_user|user_id|get_current_user|authenticated_user)\b" or
equivalent so "/users" no longer matches while "{user}" or "current_user" still
suppress. Reference: the if block checking rule.name == "Unauthenticated Route
Handler" and the variable l.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This store is counting a capped history page, not actual usage.
refreshScanCount()uses/api/v1/history, but the server only returns the latest 50 scans. After that pointscanCountstops reflecting real usage, so anything deriving quota/billing state from this value will drift. A dedicated usage/count endpoint would be safer here.🤖 Prompt for AI Agents