feat: hourly CVE alerting with notification bell and API #53

Merged
sharang merged 1 commits from feat/cve-alerts into main 2026-03-30 10:39:39 +00:00
15 changed files with 754 additions and 10 deletions

View File

@@ -1,6 +1,6 @@
FROM rust:1.94-bookworm AS builder
RUN cargo install dioxus-cli --version 0.7.4
RUN cargo install dioxus-cli --version 0.7.3 --locked
Review

[low] Inconsistent version pinning in Dockerfile

The Dockerfile changes from using an unspecified version of dioxus-cli to explicitly pinning version 0.7.3 with --locked flag. While this provides reproducibility, it's inconsistent with previous behavior and may introduce compatibility issues if not carefully managed.

Suggested fix: Ensure that version pinning is intentional and documented, or consider using a more flexible approach like specifying a version range instead of exact version.

*Scanner: code-review/convention | *

**[low] Inconsistent version pinning in Dockerfile** The Dockerfile changes from using an unspecified version of dioxus-cli to explicitly pinning version 0.7.3 with --locked flag. While this provides reproducibility, it's inconsistent with previous behavior and may introduce compatibility issues if not carefully managed. Suggested fix: Ensure that version pinning is intentional and documented, or consider using a more flexible approach like specifying a version range instead of exact version. *Scanner: code-review/convention | * <!-- compliance-fp:bc7dad71697cf159b399056dc0665b499ed4df2a3f10768bed9dcf0c0b9fdf73 -->
ARG DOCS_URL=/docs

View File

@@ -6,6 +6,7 @@ pub mod graph;
pub mod health;
pub mod help_chat;
pub mod issues;
pub mod notifications;
pub mod pentest_handlers;
pub use pentest_handlers as pentest;
pub mod repos;

View File

@@ -0,0 +1,178 @@
use axum::extract::Extension;
use axum::http::StatusCode;
use axum::Json;
use mongodb::bson::doc;
use serde::Deserialize;
use compliance_core::models::notification::CveNotification;
use super::dto::{AgentExt, ApiResponse};
/// GET /api/v1/notifications — List CVE notifications (newest first)
#[tracing::instrument(skip_all)]
pub async fn list_notifications(
Extension(agent): AgentExt,
axum::extract::Query(params): axum::extract::Query<NotificationFilter>,
) -> Result<Json<ApiResponse<Vec<CveNotification>>>, StatusCode> {
let mut filter = doc! {};
Review

[medium] Complex boolean expression in notification filtering

The status filtering logic in list_notifications uses a complex match expression that's hard to follow and could lead to incorrect filtering behavior if the status parameter contains unexpected values. The logic mixes default cases with explicit matches in a way that's not immediately clear.

Suggested fix: Simplify the status filtering by using a more direct approach like if let Some(status) = params.status followed by explicit handling of 'all' case, or extract the filtering logic into a separate helper function.

*Scanner: code-review/complexity | *

**[medium] Complex boolean expression in notification filtering** The status filtering logic in `list_notifications` uses a complex match expression that's hard to follow and could lead to incorrect filtering behavior if the status parameter contains unexpected values. The logic mixes default cases with explicit matches in a way that's not immediately clear. Suggested fix: Simplify the status filtering by using a more direct approach like `if let Some(status) = params.status` followed by explicit handling of 'all' case, or extract the filtering logic into a separate helper function. *Scanner: code-review/complexity | * <!-- compliance-fp:13d69705a319c582197e92ad5926db5f9460547118ec31150633d836e98ef603 -->
// Filter by status (default: show new + read, exclude dismissed)
match params.status.as_deref() {
Some("all") => {}
Some(s) => {
filter.insert("status", s);
}
None => {
filter.insert("status", doc! { "$in": ["new", "read"] });
}
}
// Filter by severity
if let Some(ref sev) = params.severity {
filter.insert("severity", sev.as_str());
}
Review

[medium] Potential Denial of Service via Unbounded Query Parameters

The list_notifications handler accepts page and limit parameters but does not enforce strict limits on the maximum allowed values. While there's a minimum limit of 1 and maximum limit of 200, attackers could potentially send very large page numbers which would cause MongoDB to skip a large number of documents, leading to performance degradation or timeouts.

Suggested fix: Add stricter bounds checking on both page and limit parameters. For example, cap page to a reasonable maximum value (e.g., 1000) to prevent excessive skipping in database queries.

Scanner: code-review/security | CWE: CWE-400

**[medium] Potential Denial of Service via Unbounded Query Parameters** The `list_notifications` handler accepts `page` and `limit` parameters but does not enforce strict limits on the maximum allowed values. While there's a minimum limit of 1 and maximum limit of 200, attackers could potentially send very large page numbers which would cause MongoDB to skip a large number of documents, leading to performance degradation or timeouts. Suggested fix: Add stricter bounds checking on both `page` and `limit` parameters. For example, cap `page` to a reasonable maximum value (e.g., 1000) to prevent excessive skipping in database queries. *Scanner: code-review/security | CWE: CWE-400* <!-- compliance-fp:9cb155b24558319e8e369492d1815e7a2b6c56e207366ae5546eb7faf752c717 -->
// Filter by repo
if let Some(ref repo_id) = params.repo_id {
filter.insert("repo_id", repo_id.as_str());
}
let page = params.page.unwrap_or(1).max(1);
let limit = params.limit.unwrap_or(50).min(200);
let skip = (page - 1) * limit as u64;
Review

[medium] Inconsistent handling of page and limit parameters in notifications listing

The list_notifications function uses page and limit parameters to calculate skip, but it doesn't validate that limit is within acceptable bounds before using it in the MongoDB query. If limit is set to a very large value, it could cause performance issues or resource exhaustion.

Suggested fix: Add validation to ensure limit is within reasonable bounds (e.g., between 1 and 1000) before applying it to the MongoDB query.

*Scanner: code-review/logic | *

**[medium] Inconsistent handling of page and limit parameters in notifications listing** The `list_notifications` function uses `page` and `limit` parameters to calculate `skip`, but it doesn't validate that `limit` is within acceptable bounds before using it in the MongoDB query. If `limit` is set to a very large value, it could cause performance issues or resource exhaustion. Suggested fix: Add validation to ensure `limit` is within reasonable bounds (e.g., between 1 and 1000) before applying it to the MongoDB query. *Scanner: code-review/logic | * <!-- compliance-fp:ab454b4b25bdbea666aa6b00592f00629df7599262f5b1e2d305ead219748d92 -->
let total = agent
.db
Review

[medium] Inconsistent error handling with unwrap() in production code

The code uses unwrap_or(0) on database count operations which can mask failures and lead to incorrect behavior. This pattern should be replaced with proper error propagation to ensure failures are handled appropriately rather than silently defaulting to 0.

Suggested fix: Replace unwrap_or(0) with proper error handling using ? operator or explicit error mapping to maintain consistency with the rest of the error handling pattern in the function.

*Scanner: code-review/convention | *

**[medium] Inconsistent error handling with unwrap() in production code** The code uses `unwrap_or(0)` on database count operations which can mask failures and lead to incorrect behavior. This pattern should be replaced with proper error propagation to ensure failures are handled appropriately rather than silently defaulting to 0. Suggested fix: Replace `unwrap_or(0)` with proper error handling using `?` operator or explicit error mapping to maintain consistency with the rest of the error handling pattern in the function. *Scanner: code-review/convention | * <!-- compliance-fp:04a147cda1ee08d23922ff8010b142ca156e42af8a08927a2d462c28fc845939 -->
.cve_notifications()
.count_documents(filter.clone())
.await
.unwrap_or(0);
let notifications: Vec<CveNotification> = match agent
.db
Review

[high] Missing error handling for database operations in notifications listing

The list_notifications function uses unwrap_or(0) when counting documents, which can mask potential database errors. If the database connection fails or there's a network issue, this will silently return 0 instead of propagating the error.

Suggested fix: Replace unwrap_or(0) with proper error propagation to ensure database errors are handled appropriately.

*Scanner: code-review/logic | *

**[high] Missing error handling for database operations in notifications listing** The `list_notifications` function uses `unwrap_or(0)` when counting documents, which can mask potential database errors. If the database connection fails or there's a network issue, this will silently return 0 instead of propagating the error. Suggested fix: Replace `unwrap_or(0)` with proper error propagation to ensure database errors are handled appropriately. *Scanner: code-review/logic | * <!-- compliance-fp:53e06284ca96a607d3c216dbd4f51024175ae36605b2fc9bd8dde5f316a32bce -->
.cve_notifications()
.find(filter)
Review

[high] Insecure Direct Object Reference (IDOR) in Notification Management

The notification management endpoints allow users to manipulate notifications by ID without proper authorization checks. Specifically, the mark_read, dismiss_notification, and notification_count handlers do not validate whether the requesting user has permission to access or modify the specified notification. This could allow an attacker to read, mark as read, or dismiss notifications belonging to other users if they can guess or obtain valid notification IDs.

Suggested fix: Implement proper authorization checks before allowing modification or access to notifications. Ensure that each notification's owner or authorized user can only interact with their own notifications. Add a check to verify that the authenticated user has permission to access the notification before performing any operations.

Scanner: code-review/security | CWE: CWE-285

**[high] Insecure Direct Object Reference (IDOR) in Notification Management** The notification management endpoints allow users to manipulate notifications by ID without proper authorization checks. Specifically, the `mark_read`, `dismiss_notification`, and `notification_count` handlers do not validate whether the requesting user has permission to access or modify the specified notification. This could allow an attacker to read, mark as read, or dismiss notifications belonging to other users if they can guess or obtain valid notification IDs. Suggested fix: Implement proper authorization checks before allowing modification or access to notifications. Ensure that each notification's owner or authorized user can only interact with their own notifications. Add a check to verify that the authenticated user has permission to access the notification before performing any operations. *Scanner: code-review/security | CWE: CWE-285* <!-- compliance-fp:a91e13a47fecbaa94c1ca541ada7f1f6bb3ca7cc22e5662a4157cb4cebcad3be -->
.sort(doc! { "created_at": -1 })
.skip(skip)
.limit(limit)
.await
{
Ok(cursor) => {
use futures_util::StreamExt;
let mut items = Vec::new();
let mut cursor = cursor;
while let Some(Ok(n)) = cursor.next().await {
items.push(n);
}
Review

[medium] Inconsistent error handling with unwrap() in production code

The code uses unwrap_or(0) on database count operations which can mask failures and lead to incorrect behavior. This pattern should be replaced with proper error propagation to ensure failures are handled appropriately rather than silently defaulting to 0.

Suggested fix: Replace unwrap_or(0) with proper error handling using ? operator or explicit error mapping to maintain consistency with the rest of the error handling pattern in the function.

*Scanner: code-review/convention | *

**[medium] Inconsistent error handling with unwrap() in production code** The code uses `unwrap_or(0)` on database count operations which can mask failures and lead to incorrect behavior. This pattern should be replaced with proper error propagation to ensure failures are handled appropriately rather than silently defaulting to 0. Suggested fix: Replace `unwrap_or(0)` with proper error handling using `?` operator or explicit error mapping to maintain consistency with the rest of the error handling pattern in the function. *Scanner: code-review/convention | * <!-- compliance-fp:8e2ffde00ac780dcca6a742bc8c7fd2dbb88b97d57ceb521d21ace2dea10a449 -->
Review

[medium] Incorrect MongoDB query for notification count

The notification_count function filters for notifications with status 'new', but the database may contain notifications with other statuses that should also be considered unread. This could lead to an undercount of unread notifications.

Suggested fix: Update the filter to include all statuses that represent unread notifications, such as 'new' and potentially others like 'unseen'.

*Scanner: code-review/logic | *

**[medium] Incorrect MongoDB query for notification count** The `notification_count` function filters for notifications with status 'new', but the database may contain notifications with other statuses that should also be considered unread. This could lead to an undercount of unread notifications. Suggested fix: Update the filter to include all statuses that represent unread notifications, such as 'new' and potentially others like 'unseen'. *Scanner: code-review/logic | * <!-- compliance-fp:e6fabebae11b6efea326a3aafd025f176c6bd7d11c95a1cb4f94b5ebe0d230d4 -->
items
}
Err(e) => {
tracing::error!("Failed to list notifications: {e}");
return Err(StatusCode::INTERNAL_SERVER_ERROR);
}
};
Ok(Json(ApiResponse {
data: notifications,
total: Some(total),
page: Some(page),
}))
}
/// GET /api/v1/notifications/count — Count of unread notifications
#[tracing::instrument(skip_all)]
pub async fn notification_count(
Extension(agent): AgentExt,
) -> Result<Json<serde_json::Value>, StatusCode> {
let count = agent
.db
.cve_notifications()
.count_documents(doc! { "status": "new" })
.await
.unwrap_or(0);
Ok(Json(serde_json::json!({ "count": count })))
}
/// PATCH /api/v1/notifications/:id/read — Mark a notification as read
#[tracing::instrument(skip_all, fields(id = %id))]
pub async fn mark_read(
Extension(agent): AgentExt,
axum::extract::Path(id): axum::extract::Path<String>,
) -> Result<Json<serde_json::Value>, StatusCode> {
let oid = mongodb::bson::oid::ObjectId::parse_str(&id).map_err(|_| StatusCode::BAD_REQUEST)?;
let result = agent
.db
.cve_notifications()
.update_one(
doc! { "_id": oid },
doc! { "$set": {
"status": "read",
"read_at": mongodb::bson::DateTime::now(),
}},
)
.await
.map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?;
if result.matched_count == 0 {
return Err(StatusCode::NOT_FOUND);
}
Ok(Json(serde_json::json!({ "status": "read" })))
}
/// PATCH /api/v1/notifications/:id/dismiss — Dismiss a notification
#[tracing::instrument(skip_all, fields(id = %id))]
pub async fn dismiss_notification(
Extension(agent): AgentExt,
axum::extract::Path(id): axum::extract::Path<String>,
) -> Result<Json<serde_json::Value>, StatusCode> {
let oid = mongodb::bson::oid::ObjectId::parse_str(&id).map_err(|_| StatusCode::BAD_REQUEST)?;
let result = agent
.db
.cve_notifications()
.update_one(
doc! { "_id": oid },
doc! { "$set": { "status": "dismissed" } },
)
.await
.map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?;
if result.matched_count == 0 {
return Err(StatusCode::NOT_FOUND);
}
Ok(Json(serde_json::json!({ "status": "dismissed" })))
}
/// POST /api/v1/notifications/read-all — Mark all new notifications as read
#[tracing::instrument(skip_all)]
pub async fn mark_all_read(
Extension(agent): AgentExt,
) -> Result<Json<serde_json::Value>, StatusCode> {
let result = agent
.db
.cve_notifications()
.update_many(
doc! { "status": "new" },
doc! { "$set": {
"status": "read",
"read_at": mongodb::bson::DateTime::now(),
}},
)
.await
.map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?;
Ok(Json(
serde_json::json!({ "updated": result.modified_count }),
))
}
#[derive(Debug, Deserialize)]
pub struct NotificationFilter {
pub status: Option<String>,
pub severity: Option<String>,
pub repo_id: Option<String>,
pub page: Option<u64>,
pub limit: Option<i64>,
}

View File

@@ -101,6 +101,27 @@ pub fn build_router() -> Router {
)
// Help chat (documentation-grounded Q&A)
.route("/api/v1/help/chat", post(handlers::help_chat::help_chat))
// CVE notification endpoints
.route(
"/api/v1/notifications",
get(handlers::notifications::list_notifications),
)
.route(
"/api/v1/notifications/count",
get(handlers::notifications::notification_count),
)
.route(
"/api/v1/notifications/read-all",
post(handlers::notifications::mark_all_read),
)
.route(
"/api/v1/notifications/{id}/read",
patch(handlers::notifications::mark_read),
)
.route(
"/api/v1/notifications/{id}/dismiss",
patch(handlers::notifications::dismiss_notification),
)
// Pentest API endpoints
.route(
"/api/v1/pentest/lookup-repo",

View File

@@ -42,7 +42,7 @@ pub fn load_config() -> Result<AgentConfig, AgentError> {
.unwrap_or(3001),
scan_schedule: env_var_opt("SCAN_SCHEDULE").unwrap_or_else(|| "0 0 */6 * * *".to_string()),
cve_monitor_schedule: env_var_opt("CVE_MONITOR_SCHEDULE")
.unwrap_or_else(|| "0 0 0 * * *".to_string()),
.unwrap_or_else(|| "0 0 * * * *".to_string()),
Review

[medium] Complex boolean expression in schedule configuration

The CVE monitor schedule default value was changed from '0 0 0 * * *' to '0 0 * * * *'. This change affects the cron schedule parsing logic which could introduce bugs if the parsing implementation doesn't properly handle the new format or if there are assumptions about the number of fields in cron expressions.

Suggested fix: Add explicit validation for the cron schedule format to ensure the new default value is correctly parsed and handled by the underlying cron library. Consider adding unit tests for different cron formats to verify parsing behavior.

*Scanner: code-review/complexity | *

**[medium] Complex boolean expression in schedule configuration** The CVE monitor schedule default value was changed from '0 0 0 * * *' to '0 0 * * * *'. This change affects the cron schedule parsing logic which could introduce bugs if the parsing implementation doesn't properly handle the new format or if there are assumptions about the number of fields in cron expressions. Suggested fix: Add explicit validation for the cron schedule format to ensure the new default value is correctly parsed and handled by the underlying cron library. Consider adding unit tests for different cron formats to verify parsing behavior. *Scanner: code-review/complexity | * <!-- compliance-fp:301202ed81ce2f9d5ba9e99727ebd42c2f40d83b412df04dcd445fbfdb2e880b -->
Review

[high] Insecure Default Cron Schedule

The default cron schedule for CVE monitoring was changed from '0 0 0 * * *' (daily at midnight) to '0 0 * * * *' (every minute). This introduces a significant performance issue and potential resource exhaustion as the system will now run the CVE monitor every minute instead of daily, leading to excessive database queries and processing overhead.

Suggested fix: Revert to the original daily schedule '0 0 0 * * *' or implement proper rate limiting to prevent excessive execution frequency.

Scanner: code-review/security | CWE: CWE-707

**[high] Insecure Default Cron Schedule** The default cron schedule for CVE monitoring was changed from '0 0 0 * * *' (daily at midnight) to '0 0 * * * *' (every minute). This introduces a significant performance issue and potential resource exhaustion as the system will now run the CVE monitor every minute instead of daily, leading to excessive database queries and processing overhead. Suggested fix: Revert to the original daily schedule '0 0 0 * * *' or implement proper rate limiting to prevent excessive execution frequency. *Scanner: code-review/security | CWE: CWE-707* <!-- compliance-fp:bdfcd200bf9e6fee57fac7dc0dfeb62acb44e21849723b123a43e1953b75cc02 -->
Review

[high] Incorrect cron schedule syntax

The CVE monitor schedule was changed from "0 0 0 * * *" to "0 0 * * * *". This is incorrect cron syntax - it should be "0 0 0 * * *" (which means 'at 00:00 every day') rather than "0 0 * * * *" (which would mean 'at 00:00 every minute'). The new value will cause the scheduler to run every minute instead of daily.

Suggested fix: Change back to "0 0 0 * * *" to maintain daily execution at midnight

*Scanner: code-review/logic | *

**[high] Incorrect cron schedule syntax** The CVE monitor schedule was changed from "0 0 0 * * *" to "0 0 * * * *". This is incorrect cron syntax - it should be "0 0 0 * * *" (which means 'at 00:00 every day') rather than "0 0 * * * *" (which would mean 'at 00:00 every minute'). The new value will cause the scheduler to run every minute instead of daily. Suggested fix: Change back to "0 0 0 * * *" to maintain daily execution at midnight *Scanner: code-review/logic | * <!-- compliance-fp:435ce43f7611e56398894855aee3143ac090cc7496f3fc49d4a2244e5c0ae50b -->
Review

[medium] Inconsistent error handling with unwrap_or_else

The code uses unwrap_or_else consistently for environment variable parsing, but this pattern can mask configuration loading failures. If an environment variable is present but invalid, it will fall back to the default value instead of failing fast, potentially leading to unexpected behavior.

Suggested fix: Consider using unwrap_or instead of unwrap_or_else to make the fallback behavior more explicit and avoid potential silent failures when environment variables contain invalid values.

*Scanner: code-review/convention | *

**[medium] Inconsistent error handling with unwrap_or_else** The code uses `unwrap_or_else` consistently for environment variable parsing, but this pattern can mask configuration loading failures. If an environment variable is present but invalid, it will fall back to the default value instead of failing fast, potentially leading to unexpected behavior. Suggested fix: Consider using `unwrap_or` instead of `unwrap_or_else` to make the fallback behavior more explicit and avoid potential silent failures when environment variables contain invalid values. *Scanner: code-review/convention | * <!-- compliance-fp:11df5870c6150171dcd661756c416d9f83a61990f27b31e2c85e7f5040ebf5e6 -->
git_clone_base_path: env_var_opt("GIT_CLONE_BASE_PATH")
.unwrap_or_else(|| "/tmp/compliance-scanner/repos".to_string()),
ssh_key_path: env_var_opt("SSH_KEY_PATH")

View File

@@ -78,6 +78,25 @@ impl Database {
)
.await?;
// cve_notifications: unique cve_id + repo_id + package, status filter
Review

[medium] Missing error propagation in database index creation

The database index creation operations use .await? but don't propagate errors properly. If index creation fails, the application might continue running with missing indexes, which could lead to data consistency issues or performance problems. The error handling should ensure that index creation failures are properly reported.

Suggested fix: Add explicit error handling around the index creation calls to ensure that failures during index creation are properly propagated up the call stack rather than being silently ignored.

*Scanner: code-review/convention | *

**[medium] Missing error propagation in database index creation** The database index creation operations use `.await?` but don't propagate errors properly. If index creation fails, the application might continue running with missing indexes, which could lead to data consistency issues or performance problems. The error handling should ensure that index creation failures are properly reported. Suggested fix: Add explicit error handling around the index creation calls to ensure that failures during index creation are properly propagated up the call stack rather than being silently ignored. *Scanner: code-review/convention | * <!-- compliance-fp:8f5c2d8e5c4635ece75e7de2f705bba2629348aba28c3ebac50f30a0a0dbe3ad -->
self.cve_notifications()
.create_index(
IndexModel::builder()
.keys(
doc! { "cve_id": 1, "repo_id": 1, "package_name": 1, "package_version": 1 },
)
.options(IndexOptions::builder().unique(true).build())
.build(),
)
.await?;
self.cve_notifications()
.create_index(
IndexModel::builder()
.keys(doc! { "status": 1, "created_at": -1 })
.build(),
)
.await?;
// tracker_issues: unique finding_id
self.tracker_issues()
.create_index(
@@ -222,6 +241,12 @@ impl Database {
self.inner.collection("cve_alerts")
}
pub fn cve_notifications(
Review

[low] Inconsistent public API design

The cve_notifications method is added as a public getter but doesn't follow the same pattern as other collection getters like cve_alerts and tracker_issues. This creates an inconsistent API surface where some collections have dedicated methods while others don't, potentially confusing users of the Database struct.

Suggested fix: Either add similar getter methods for all collections or remove the inconsistency by making all collection access consistent through a single pattern.

*Scanner: code-review/convention | *

**[low] Inconsistent public API design** The `cve_notifications` method is added as a public getter but doesn't follow the same pattern as other collection getters like `cve_alerts` and `tracker_issues`. This creates an inconsistent API surface where some collections have dedicated methods while others don't, potentially confusing users of the Database struct. Suggested fix: Either add similar getter methods for all collections or remove the inconsistency by making all collection access consistent through a single pattern. *Scanner: code-review/convention | * <!-- compliance-fp:f1e245853073a64bd6f9c74be6e99051603c70a645fb6c77fc85153a5db4db5b -->
&self,
) -> Collection<compliance_core::models::notification::CveNotification> {
self.inner.collection("cve_notifications")
}
pub fn tracker_issues(&self) -> Collection<TrackerIssue> {
self.inner.collection("tracker_issues")
}

View File

@@ -82,24 +82,158 @@ async fn scan_all_repos(agent: &ComplianceAgent) {
}
async fn monitor_cves(agent: &ComplianceAgent) {
use compliance_core::models::notification::{parse_severity, CveNotification};
use compliance_core::models::SbomEntry;
use futures_util::StreamExt;
Review

[medium] Deeply nested control flow in CVE monitoring

The monitor_cves function contains deeply nested control flow with multiple levels of error handling and conditional logic. The main loop processing entries by repo contains nested conditionals for scanning, upserting alerts, updating entries, and creating notifications, making it difficult to follow the logical flow and increasing risk of bugs during maintenance.

Suggested fix: Refactor the main loop into smaller functions with single responsibilities. Extract the scanning logic, alert upserting, entry updates, and notification creation into separate functions to flatten the nesting and improve readability.

*Scanner: code-review/complexity | *

**[medium] Deeply nested control flow in CVE monitoring** The `monitor_cves` function contains deeply nested control flow with multiple levels of error handling and conditional logic. The main loop processing entries by repo contains nested conditionals for scanning, upserting alerts, updating entries, and creating notifications, making it difficult to follow the logical flow and increasing risk of bugs during maintenance. Suggested fix: Refactor the main loop into smaller functions with single responsibilities. Extract the scanning logic, alert upserting, entry updates, and notification creation into separate functions to flatten the nesting and improve readability. *Scanner: code-review/complexity | * <!-- compliance-fp:2ba3c96d10b859d29c786938973e436c45285e10599b1a1b0496f3294530093c -->
// Re-scan all SBOM entries for new CVEs
// Fetch all SBOM entries grouped by repo
let cursor = match agent.db.sbom_entries().find(doc! {}).await {
Ok(c) => c,
Err(e) => {
tracing::error!("Failed to list SBOM entries for CVE monitoring: {e}");
tracing::error!("CVE monitor: failed to list SBOM entries: {e}");
return;
}
};
let entries: Vec<_> = cursor.filter_map(|r| async { r.ok() }).collect().await;
let entries: Vec<SbomEntry> = cursor.filter_map(|r| async { r.ok() }).collect().await;
if entries.is_empty() {
tracing::debug!("CVE monitor: no SBOM entries, skipping");
return;
}
tracing::info!("CVE monitor: checking {} dependencies", entries.len());
// The actual CVE checking is handled by the CveScanner in the pipeline
// This is a simplified version that just logs the activity
tracing::info!(
"CVE monitor: checking {} dependencies for new CVEs",
Review

[medium] Potential panic from unwrap_or_else() in notification creation

The code uses repo_names.get(&repo_id).cloned().unwrap_or_else(|| repo_id.clone()) which can panic if repo_id contains invalid UTF-8. While this is unlikely in practice, it's an anti-pattern that could cause runtime panics if the underlying data is malformed. The code should handle this case more gracefully.

Suggested fix: Use a more robust fallback mechanism that doesn't rely on unwrap_or_else() for string operations, or validate that repo_id is valid UTF-8 before using it as a key.

*Scanner: code-review/convention | *

**[medium] Potential panic from unwrap_or_else() in notification creation** The code uses `repo_names.get(&repo_id).cloned().unwrap_or_else(|| repo_id.clone())` which can panic if repo_id contains invalid UTF-8. While this is unlikely in practice, it's an anti-pattern that could cause runtime panics if the underlying data is malformed. The code should handle this case more gracefully. Suggested fix: Use a more robust fallback mechanism that doesn't rely on unwrap_or_else() for string operations, or validate that repo_id is valid UTF-8 before using it as a key. *Scanner: code-review/convention | * <!-- compliance-fp:a8accb7d45230c70fafa594eee0f338cf1165310e7ab5cd05251d352d193719b -->
Review

[medium] Sensitive Data Exposure in Logs

The code logs sensitive information such as repo.name and alert.cve_id in debug and info level logs. Although these logs are typically not exposed externally, if they are stored or transmitted without proper access controls, they could expose repository names and CVE identifiers to unauthorized parties.

Suggested fix: Avoid logging sensitive data like repository names and CVE IDs unless strictly necessary for debugging. Consider using structured logging with configurable log levels to prevent accidental exposure.

Scanner: code-review/security | CWE: CWE-532

**[medium] Sensitive Data Exposure in Logs** The code logs sensitive information such as `repo.name` and `alert.cve_id` in debug and info level logs. Although these logs are typically not exposed externally, if they are stored or transmitted without proper access controls, they could expose repository names and CVE identifiers to unauthorized parties. Suggested fix: Avoid logging sensitive data like repository names and CVE IDs unless strictly necessary for debugging. Consider using structured logging with configurable log levels to prevent accidental exposure. *Scanner: code-review/security | CWE: CWE-532* <!-- compliance-fp:4d41c06a604adcd93d3815fe8bc1e1122ff065d9740b46606d0e73e3c2a4e772 -->
Review

[medium] Potential Command Injection via User-Controlled Repository Name

The code constructs a repository name from user-provided data (repo.name) and uses it in logging and notification creation. While this doesn't directly execute commands, if any downstream system processes this name as part of a shell command or file path, it could lead to command injection. Specifically, the repo_name variable is used in tracing::info! log messages and passed to CveNotification::new() which may be used in contexts where it's processed further.

Suggested fix: Sanitize or validate the repo.name field before using it in logs or notifications. Ensure that any downstream processing treats this value as literal text rather than executable code.

Scanner: code-review/security | CWE: CWE-78

**[medium] Potential Command Injection via User-Controlled Repository Name** The code constructs a repository name from user-provided data (`repo.name`) and uses it in logging and notification creation. While this doesn't directly execute commands, if any downstream system processes this name as part of a shell command or file path, it could lead to command injection. Specifically, the `repo_name` variable is used in `tracing::info!` log messages and passed to `CveNotification::new()` which may be used in contexts where it's processed further. Suggested fix: Sanitize or validate the `repo.name` field before using it in logs or notifications. Ensure that any downstream processing treats this value as literal text rather than executable code. *Scanner: code-review/security | CWE: CWE-78* <!-- compliance-fp:9c84d13d743395fb1bbc8ae1221b18a750c81d591fe2a47a0cde1fe6cf94fb3f -->
entries.len()
);
// Build a repo_id → repo_name lookup
let repo_ids: std::collections::HashSet<String> =
entries.iter().map(|e| e.repo_id.clone()).collect();
let mut repo_names: std::collections::HashMap<String, String> =
std::collections::HashMap::new();
for rid in &repo_ids {
if let Ok(oid) = mongodb::bson::oid::ObjectId::parse_str(rid) {
if let Ok(Some(repo)) = agent.db.repositories().find_one(doc! { "_id": oid }).await {
repo_names.insert(rid.clone(), repo.name.clone());
}
}
}
// Use the existing CveScanner to query OSV.dev
let nvd_key = agent.config.nvd_api_key.as_ref().map(|k| {
use secrecy::ExposeSecret;
k.expose_secret().to_string()
});
Review

[medium] Inconsistent error handling with unwrap() in critical path

The code uses mongodb::bson::to_bson(...).unwrap_or_default() multiple times in the CVE monitoring flow. While this prevents panics, it silently discards serialization errors which could lead to missing data in database operations. This inconsistent error handling approach (mixing unwrap_or_default with proper error logging) could hide failures that should be properly reported.

Suggested fix: Replace unwrap_or_default() with proper error handling that logs the serialization failure and potentially aborts the operation or retries, rather than silently ignoring BSON conversion errors.

*Scanner: code-review/convention | *

**[medium] Inconsistent error handling with unwrap() in critical path** The code uses `mongodb::bson::to_bson(...).unwrap_or_default()` multiple times in the CVE monitoring flow. While this prevents panics, it silently discards serialization errors which could lead to missing data in database operations. This inconsistent error handling approach (mixing unwrap_or_default with proper error logging) could hide failures that should be properly reported. Suggested fix: Replace unwrap_or_default() with proper error handling that logs the serialization failure and potentially aborts the operation or retries, rather than silently ignoring BSON conversion errors. *Scanner: code-review/convention | * <!-- compliance-fp:e48b3f130e8c1f645792b1841629dcda325aa6c15b1abc0d09d0a952b673d1c6 -->
let scanner = crate::pipeline::cve::CveScanner::new(
agent.http.clone(),
agent.config.searxng_url.clone(),
nvd_key,
);
// Group entries by repo for scanning
Review

[medium] Incorrect handling of empty known_vulnerabilities in SBOM entry updates

In the CVE monitoring logic, when updating SBOM entries with known vulnerabilities, the code checks if entry.known_vulnerabilities.is_empty() and skips the update if true. However, this check is flawed because it should only skip updating if there are NO vulnerabilities to report, but the current logic prevents any update even when vulnerabilities exist but are empty. This could lead to stale data in the database.

Suggested fix: The condition should be inverted to only skip when there are NO vulnerabilities to process. Change if entry.known_vulnerabilities.is_empty() to if !entry.known_vulnerabilities.is_empty() or better yet, remove the check entirely since we want to update regardless of whether vulnerabilities exist.

*Scanner: code-review/logic | *

**[medium] Incorrect handling of empty known_vulnerabilities in SBOM entry updates** In the CVE monitoring logic, when updating SBOM entries with known vulnerabilities, the code checks `if entry.known_vulnerabilities.is_empty()` and skips the update if true. However, this check is flawed because it should only skip updating if there are NO vulnerabilities to report, but the current logic prevents any update even when vulnerabilities exist but are empty. This could lead to stale data in the database. Suggested fix: The condition should be inverted to only skip when there are NO vulnerabilities to process. Change `if entry.known_vulnerabilities.is_empty()` to `if !entry.known_vulnerabilities.is_empty()` or better yet, remove the check entirely since we want to update regardless of whether vulnerabilities exist. *Scanner: code-review/logic | * <!-- compliance-fp:181b8fb3e314d10e55a8c23aa10b55c25abf215c256d797b1f7cb945eb567025 -->
let mut entries_by_repo: std::collections::HashMap<String, Vec<SbomEntry>> =
std::collections::HashMap::new();
Review

[high] Insecure Direct Object Reference (IDOR) in CVE Notification Creation

When creating CVE notifications, the code uses a MongoDB filter based on cve_id, repo_id, package_name, and package_version. However, there's no explicit authorization check to ensure that the authenticated user has access to the specified repo_id. If an attacker can forge a request to create a notification for a repository they don't own, they could potentially pollute the notification system with false positives or malicious data.

Suggested fix: Add an authorization check before creating CVE notifications to verify that the requesting entity has permission to access the given repo_id. This should involve validating the user's role or ownership of the repository.

Scanner: code-review/security | CWE: CWE-285

**[high] Insecure Direct Object Reference (IDOR) in CVE Notification Creation** When creating CVE notifications, the code uses a MongoDB filter based on `cve_id`, `repo_id`, `package_name`, and `package_version`. However, there's no explicit authorization check to ensure that the authenticated user has access to the specified `repo_id`. If an attacker can forge a request to create a notification for a repository they don't own, they could potentially pollute the notification system with false positives or malicious data. Suggested fix: Add an authorization check before creating CVE notifications to verify that the requesting entity has permission to access the given `repo_id`. This should involve validating the user's role or ownership of the repository. *Scanner: code-review/security | CWE: CWE-285* <!-- compliance-fp:a5235af8a103fcddbed4b7f0249a3cef28dcf16db671146d943bbe67b4c2cae6 -->
for entry in entries {
entries_by_repo
.entry(entry.repo_id.clone())
.or_default()
.push(entry);
}
let mut new_notifications = 0u32;
for (repo_id, mut repo_entries) in entries_by_repo {
let repo_name = repo_names
.get(&repo_id)
.cloned()
.unwrap_or_else(|| repo_id.clone());
// Scan dependencies for CVEs
let alerts = match scanner.scan_dependencies(&repo_id, &mut repo_entries).await {
Ok(a) => a,
Err(e) => {
tracing::warn!("CVE monitor: scan failed for {repo_name}: {e}");
continue;
}
};
// Upsert CVE alerts (existing logic)
for alert in &alerts {
let filter = doc! { "cve_id": &alert.cve_id, "repo_id": &alert.repo_id };
let update = doc! { "$setOnInsert": mongodb::bson::to_bson(alert).unwrap_or_default() };
let _ = agent
.db
.cve_alerts()
.update_one(filter, update)
.upsert(true)
.await;
}
// Update SBOM entries with discovered vulnerabilities
for entry in &repo_entries {
if entry.known_vulnerabilities.is_empty() {
continue;
}
if let Some(entry_id) = &entry.id {
let _ = agent
.db
.sbom_entries()
.update_one(
doc! { "_id": entry_id },
doc! { "$set": {
"known_vulnerabilities": mongodb::bson::to_bson(&entry.known_vulnerabilities).unwrap_or_default(),
"updated_at": mongodb::bson::DateTime::now(),
}},
)
.await;
}
}
// Create notifications for NEW CVEs (dedup against existing notifications)
for alert in &alerts {
let filter = doc! {
"cve_id": &alert.cve_id,
"repo_id": &alert.repo_id,
"package_name": &alert.affected_package,
"package_version": &alert.affected_version,
};
// Only insert if not already exists (upsert with $setOnInsert)
let severity = parse_severity(alert.severity.as_deref(), alert.cvss_score);
let mut notification = CveNotification::new(
alert.cve_id.clone(),
repo_id.clone(),
repo_name.clone(),
alert.affected_package.clone(),
alert.affected_version.clone(),
severity,
);
notification.cvss_score = alert.cvss_score;
notification.summary = alert.summary.clone();
notification.url = Some(format!("https://osv.dev/vulnerability/{}", alert.cve_id));
let update = doc! {
"$setOnInsert": mongodb::bson::to_bson(&notification).unwrap_or_default()
};
match agent
.db
.cve_notifications()
.update_one(filter, update)
.upsert(true)
.await
{
Ok(result) if result.upserted_id.is_some() => {
new_notifications += 1;
}
Err(e) => {
tracing::warn!("CVE monitor: failed to create notification: {e}");
}
_ => {} // Already exists
}
}
}
if new_notifications > 0 {
tracing::info!("CVE monitor: created {new_notifications} new notification(s)");
} else {
tracing::info!("CVE monitor: no new CVEs found");
}
}

View File

@@ -7,6 +7,7 @@ pub mod finding;
pub mod graph;
pub mod issue;
pub mod mcp;
pub mod notification;
pub mod pentest;
pub mod repository;
pub mod sbom;
@@ -27,6 +28,7 @@ pub use graph::{
};
pub use issue::{IssueStatus, TrackerIssue, TrackerType};
pub use mcp::{McpServerConfig, McpServerStatus, McpTransport};
pub use notification::{CveNotification, NotificationSeverity, NotificationStatus};
Review

[low] Public API exposure of internal notification module

The notification module was made public in the models re-export, but it appears to be an internal implementation detail used only within the compliance-agent. Making it public exposes an internal API contract that may change without notice, breaking consumers who depend on it.

Suggested fix: Consider making the notification module private or moving it to a separate internal crate if it's truly meant to be internal, or document clearly that it's part of the public API contract.

*Scanner: code-review/convention | *

**[low] Public API exposure of internal notification module** The notification module was made public in the models re-export, but it appears to be an internal implementation detail used only within the compliance-agent. Making it public exposes an internal API contract that may change without notice, breaking consumers who depend on it. Suggested fix: Consider making the notification module private or moving it to a separate internal crate if it's truly meant to be internal, or document clearly that it's part of the public API contract. *Scanner: code-review/convention | * <!-- compliance-fp:5ca4ef59a4c774d8d6eeb9ef8966eb504500ec7dfc68d37beb773d596ff8263c -->
pub use pentest::{
AttackChainNode, AttackNodeStatus, AuthMode, CodeContextHint, Environment, IdentityProvider,
PentestAuthConfig, PentestConfig, PentestEvent, PentestMessage, PentestSession, PentestStats,

View File

@@ -0,0 +1,103 @@
use chrono::{DateTime, Utc};
use serde::{Deserialize, Serialize};
/// Status of a CVE notification
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
#[serde(rename_all = "lowercase")]
pub enum NotificationStatus {
/// Newly created, not yet seen by the user
New,
/// User has seen it (e.g., opened the notification panel)
Read,
/// User has explicitly acknowledged/dismissed it
Dismissed,
}
/// Severity level for notification filtering
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)]
#[serde(rename_all = "lowercase")]
pub enum NotificationSeverity {
Low,
Medium,
High,
Critical,
}
/// A notification about a newly discovered CVE affecting a tracked dependency.
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct CveNotification {
#[serde(rename = "_id", skip_serializing_if = "Option::is_none")]
pub id: Option<bson::oid::ObjectId>,
/// The CVE/GHSA identifier
pub cve_id: String,
/// Repository where the vulnerable dependency is used
pub repo_id: String,
/// Repository name (denormalized for display)
pub repo_name: String,
/// Affected package name
pub package_name: String,
/// Affected version
pub package_version: String,
/// Human-readable severity
pub severity: NotificationSeverity,
/// CVSS score if available
pub cvss_score: Option<f64>,
/// Short summary of the vulnerability
pub summary: Option<String>,
/// Link to vulnerability details
pub url: Option<String>,
/// Notification lifecycle status
pub status: NotificationStatus,
/// When the CVE was first detected for this dependency
#[serde(with = "super::serde_helpers::bson_datetime")]
pub created_at: DateTime<Utc>,
/// When the user last interacted with this notification
pub read_at: Option<DateTime<Utc>>,
}
impl CveNotification {
pub fn new(
cve_id: String,
repo_id: String,
repo_name: String,
package_name: String,
package_version: String,
severity: NotificationSeverity,
) -> Self {
Self {
id: None,
Review

[medium] Potential panic in notification creation due to missing error propagation

The CveNotification::new constructor creates a new notification with default values but doesn't validate any of its parameters. If any of the required fields (like cve_id, repo_id, etc.) are empty or invalid, this could lead to inconsistent state in the notification system without proper error reporting.

Suggested fix: Consider adding validation checks or returning a Result type from the constructor to propagate potential errors during notification creation.

*Scanner: code-review/convention | *

**[medium] Potential panic in notification creation due to missing error propagation** The `CveNotification::new` constructor creates a new notification with default values but doesn't validate any of its parameters. If any of the required fields (like `cve_id`, `repo_id`, etc.) are empty or invalid, this could lead to inconsistent state in the notification system without proper error reporting. Suggested fix: Consider adding validation checks or returning a Result type from the constructor to propagate potential errors during notification creation. *Scanner: code-review/convention | * <!-- compliance-fp:38733d61b0095d0334b329ebc9563043a18eb6717152d3f2671eb74e26ce6e0a -->
cve_id,
repo_id,
repo_name,
package_name,
package_version,
severity,
cvss_score: None,
summary: None,
url: None,
status: NotificationStatus::New,
created_at: Utc::now(),
read_at: None,
}
}
}
Review

[medium] Complex boolean expression in severity parsing

The parse_severity function contains a complex boolean expression that mixes CVSS score checking with string severity mapping. This increases cognitive load when trying to understand the logic flow and could lead to incorrect severity assignment if the conditions are reordered or modified.

Suggested fix: Break down the conditional logic into separate functions or use early returns to simplify the boolean expression and make the severity mapping more readable.

*Scanner: code-review/complexity | *

**[medium] Complex boolean expression in severity parsing** The `parse_severity` function contains a complex boolean expression that mixes CVSS score checking with string severity mapping. This increases cognitive load when trying to understand the logic flow and could lead to incorrect severity assignment if the conditions are reordered or modified. Suggested fix: Break down the conditional logic into separate functions or use early returns to simplify the boolean expression and make the severity mapping more readable. *Scanner: code-review/complexity | * <!-- compliance-fp:dcf8698f6e820a0b83a62005de7c55ce7010cb3715f2a4768205476650d53601 -->
/// Map an OSV/NVD severity string to our notification severity
pub fn parse_severity(s: Option<&str>, cvss: Option<f64>) -> NotificationSeverity {
// Prefer CVSS score if available
Review

[high] Off-by-one error in CVSS severity thresholds

The parse_severity function uses inclusive thresholds for CVSS scores that should be exclusive. Specifically, a score of exactly 9.0 is classified as Critical, but according to CVSS standards, 9.0 should be considered High (scores 7.0-8.9). This causes incorrect categorization of vulnerabilities with CVSS scores at boundary values.

Suggested fix: Change the comparison operators to use strict inequality for boundary values. For example, change s if s >= 9.0 => NotificationSeverity::Critical to s if s > 9.0 => NotificationSeverity::Critical and adjust other thresholds accordingly.

*Scanner: code-review/logic | *

**[high] Off-by-one error in CVSS severity thresholds** The `parse_severity` function uses inclusive thresholds for CVSS scores that should be exclusive. Specifically, a score of exactly 9.0 is classified as Critical, but according to CVSS standards, 9.0 should be considered High (scores 7.0-8.9). This causes incorrect categorization of vulnerabilities with CVSS scores at boundary values. Suggested fix: Change the comparison operators to use strict inequality for boundary values. For example, change `s if s >= 9.0 => NotificationSeverity::Critical` to `s if s > 9.0 => NotificationSeverity::Critical` and adjust other thresholds accordingly. *Scanner: code-review/logic | * <!-- compliance-fp:bddaad5308cef0c8df610c6aabd5daf4f8b28698d71d50b64b3fcd700a428617 -->
if let Some(score) = cvss {
Review

[medium] Inconsistent error handling in notification severity parsing

The parse_severity function uses pattern matching on Option<&str> but doesn't handle the case where the string might be invalid or unexpected. While it handles None cases properly, it assumes all non-None strings are valid severity indicators, which could lead to incorrect severity assignment if malformed input is encountered.

Suggested fix: Add explicit error handling or validation for unexpected string values in the severity parsing logic to ensure robustness against malformed inputs.

*Scanner: code-review/convention | *

**[medium] Inconsistent error handling in notification severity parsing** The `parse_severity` function uses pattern matching on `Option<&str>` but doesn't handle the case where the string might be invalid or unexpected. While it handles None cases properly, it assumes all non-None strings are valid severity indicators, which could lead to incorrect severity assignment if malformed input is encountered. Suggested fix: Add explicit error handling or validation for unexpected string values in the severity parsing logic to ensure robustness against malformed inputs. *Scanner: code-review/convention | * <!-- compliance-fp:2c2c64b5a0c8e658335d46625ee90c0813795f33aa5974584f5000688768d5e7 -->
return match score {
s if s >= 9.0 => NotificationSeverity::Critical,
s if s >= 7.0 => NotificationSeverity::High,
s if s >= 4.0 => NotificationSeverity::Medium,
_ => NotificationSeverity::Low,
};
}
// Fall back to string severity
match s.map(|s| s.to_uppercase()).as_deref() {
Some("CRITICAL") => NotificationSeverity::Critical,
Some("HIGH") => NotificationSeverity::High,
Some("MODERATE" | "MEDIUM") => NotificationSeverity::Medium,
_ => NotificationSeverity::Low,
}
}

View File

@@ -3847,3 +3847,33 @@ tbody tr:last-child td {
.help-chat-send:not(:disabled):hover {
background: var(--accent-hover);
}
/* ═══════════════════════════════════════════════════════════════
NOTIFICATION BELL — CVE alert dropdown
═══════════════════════════════════════════════════════════════ */
.notification-bell-wrapper { position: fixed; top: 16px; right: 28px; z-index: 48; }
.notification-bell-btn { position: relative; background: var(--bg-elevated); border: 1px solid var(--border); border-radius: 10px; padding: 8px 10px; color: var(--text-secondary); cursor: pointer; display: flex; align-items: center; transition: color 0.15s, border-color 0.15s; }
.notification-bell-btn:hover { color: var(--text-primary); border-color: var(--border-bright); }
.notification-badge { position: absolute; top: -4px; right: -4px; background: var(--danger); color: #fff; font-size: 10px; font-weight: 700; min-width: 18px; height: 18px; border-radius: 9px; display: flex; align-items: center; justify-content: center; padding: 0 4px; font-family: 'Outfit', sans-serif; }
.notification-panel { position: absolute; top: 44px; right: 0; width: 380px; max-height: 480px; background: var(--bg-secondary); border: 1px solid var(--border-bright); border-radius: 12px; overflow: hidden; box-shadow: 0 12px 48px rgba(0,0,0,0.5); display: flex; flex-direction: column; }
.notification-panel-header { display: flex; align-items: center; justify-content: space-between; padding: 12px 16px; border-bottom: 1px solid var(--border); font-family: 'Outfit', sans-serif; font-weight: 600; font-size: 14px; color: var(--text-primary); }
.notification-close-btn { background: none; border: none; color: var(--text-secondary); cursor: pointer; padding: 2px; }
.notification-panel-body { overflow-y: auto; flex: 1; padding: 8px; }
.notification-loading, .notification-empty { display: flex; flex-direction: column; align-items: center; justify-content: center; padding: 32px 16px; color: var(--text-secondary); font-size: 13px; gap: 8px; }
.notification-item { padding: 10px 12px; border-radius: 8px; margin-bottom: 4px; background: var(--bg-card); border: 1px solid var(--border); transition: border-color 0.15s; }
.notification-item:hover { border-color: var(--border-bright); }
.notification-item-header { display: flex; align-items: center; gap: 8px; margin-bottom: 4px; }
.notification-sev { font-size: 10px; font-weight: 700; padding: 2px 6px; border-radius: 4px; text-transform: uppercase; letter-spacing: 0.5px; font-family: 'Outfit', sans-serif; }
.notification-sev.sev-critical { background: var(--danger-bg); color: var(--danger); }
.notification-sev.sev-high { background: rgba(255,140,0,0.12); color: #ff8c00; }
.notification-sev.sev-medium { background: var(--warning-bg); color: var(--warning); }
.notification-sev.sev-low { background: rgba(0,200,255,0.08); color: var(--accent); }
.notification-cve-id { font-size: 12px; font-weight: 600; color: var(--text-primary); font-family: 'JetBrains Mono', monospace; }
.notification-cve-id a { color: var(--accent); text-decoration: none; }
.notification-cve-id a:hover { text-decoration: underline; }
.notification-cvss { font-size: 10px; color: var(--text-secondary); margin-left: auto; font-family: 'JetBrains Mono', monospace; }
.notification-dismiss-btn { background: none; border: none; color: var(--text-tertiary); cursor: pointer; padding: 2px; margin-left: 4px; }
.notification-dismiss-btn:hover { color: var(--danger); }
.notification-item-pkg { font-size: 12px; color: var(--text-primary); font-family: 'JetBrains Mono', monospace; }
.notification-item-repo { font-size: 11px; color: var(--text-secondary); margin-bottom: 4px; }
.notification-item-summary { font-size: 11px; color: var(--text-secondary); line-height: 1.4; display: -webkit-box; -webkit-line-clamp: 2; -webkit-box-orient: vertical; overflow: hidden; }

View File

@@ -2,6 +2,7 @@ use dioxus::prelude::*;
use crate::app::Route;
use crate::components::help_chat::HelpChat;
use crate::components::notification_bell::NotificationBell;
use crate::components::sidebar::Sidebar;
use crate::components::toast::{ToastContainer, Toasts};
use crate::infrastructure::auth_check::check_auth;
@@ -21,6 +22,7 @@ pub fn AppShell() -> Element {
main { class: "main-content",
Outlet::<Route> {}
}
NotificationBell {}
Review

[medium] Missing Input Validation in NotificationBell Component

The NotificationBell component appears to be added to the app shell without any validation of user-provided data that might be passed to it. If this component processes any user input directly (such as notification messages, titles, or metadata) without sanitization or validation, it could lead to XSS vulnerabilities or other injection attacks. Without seeing the full implementation of NotificationBell, we cannot determine the exact risk, but adding a new component that handles potentially untrusted data without proper validation creates an attack surface.

Suggested fix: Review the NotificationBell component implementation to ensure all user-provided data is properly sanitized and validated before rendering or processing. Implement input validation and output encoding to prevent XSS and other injection vulnerabilities.

Scanner: code-review/security | CWE: CWE-79

**[medium] Missing Input Validation in NotificationBell Component** The NotificationBell component appears to be added to the app shell without any validation of user-provided data that might be passed to it. If this component processes any user input directly (such as notification messages, titles, or metadata) without sanitization or validation, it could lead to XSS vulnerabilities or other injection attacks. Without seeing the full implementation of NotificationBell, we cannot determine the exact risk, but adding a new component that handles potentially untrusted data without proper validation creates an attack surface. Suggested fix: Review the NotificationBell component implementation to ensure all user-provided data is properly sanitized and validated before rendering or processing. Implement input validation and output encoding to prevent XSS and other injection vulnerabilities. *Scanner: code-review/security | CWE: CWE-79* <!-- compliance-fp:7a7ed653a81d2faca1b58121ac48d2497846487d23508388981aea5c3017b157 -->
Review

[high] Missing import of NotificationBell in app_shell.rs

The NotificationBell component is used in AppShell but not properly imported. While the module is declared in mod.rs, the specific component needs to be imported in the app_shell.rs file to be available for use.

Suggested fix: Add use crate::components::notification_bell::NotificationBell; at the top of the file after the other imports.

*Scanner: code-review/logic | *

**[high] Missing import of NotificationBell in app_shell.rs** The NotificationBell component is used in AppShell but not properly imported. While the module is declared in mod.rs, the specific component needs to be imported in the app_shell.rs file to be available for use. Suggested fix: Add `use crate::components::notification_bell::NotificationBell;` at the top of the file after the other imports. *Scanner: code-review/logic | * <!-- compliance-fp:942ded8c666e8ca25d0308a7e6d4baea0e4980fcb45be6c0a9987c7cbe64d2ad -->
Review

[medium] Inconsistent error handling in auth check

The check_auth function in src/components/app_shell.rs uses expect() which will panic on failure, while other parts of the application likely use proper error propagation. This inconsistency can cause unexpected crashes instead of graceful error handling.

Suggested fix: Replace expect() with proper error handling using ? operator or explicit error propagation to maintain consistency with the rest of the application's error handling pattern.

*Scanner: code-review/convention | *

**[medium] Inconsistent error handling in auth check** The `check_auth` function in `src/components/app_shell.rs` uses `expect()` which will panic on failure, while other parts of the application likely use proper error propagation. This inconsistency can cause unexpected crashes instead of graceful error handling. Suggested fix: Replace `expect()` with proper error handling using `?` operator or explicit error propagation to maintain consistency with the rest of the application's error handling pattern. *Scanner: code-review/convention | * <!-- compliance-fp:d12621e39568997ac37db7aef4c581f1c13d90a949197070c70bda3a9db9cc27 -->
Review

[medium] Missing error handling in component initialization

The AppShell component calls check_auth() without handling potential errors, which could lead to unhandled panics when authentication fails. The component should handle authentication errors gracefully rather than crashing.

Suggested fix: Add proper error handling around the check_auth() call to catch and handle authentication failures appropriately, such as redirecting to login or showing an error message.

*Scanner: code-review/convention | *

**[medium] Missing error handling in component initialization** The `AppShell` component calls `check_auth()` without handling potential errors, which could lead to unhandled panics when authentication fails. The component should handle authentication errors gracefully rather than crashing. Suggested fix: Add proper error handling around the `check_auth()` call to catch and handle authentication failures appropriately, such as redirecting to login or showing an error message. *Scanner: code-review/convention | * <!-- compliance-fp:0be63a4dc744e7308fba454955beaa8cfb382745ab323c9d15b6de85b1bfe281 -->
ToastContainer {}
HelpChat {}
}

View File

@@ -4,6 +4,7 @@ pub mod code_inspector;
pub mod code_snippet;
pub mod file_tree;
pub mod help_chat;
pub mod notification_bell;
pub mod page_header;
pub mod pagination;
pub mod pentest_wizard;

View File

@@ -0,0 +1,155 @@
use dioxus::prelude::*;
use dioxus_free_icons::icons::bs_icons::*;
use dioxus_free_icons::Icon;
use crate::infrastructure::notifications::{
dismiss_notification, fetch_notification_count, fetch_notifications,
mark_all_notifications_read,
};
#[component]
pub fn NotificationBell() -> Element {
let mut is_open = use_signal(|| false);
let mut count = use_signal(|| 0u64);
let mut notifications = use_signal(Vec::new);
let mut is_loading = use_signal(|| false);
// Poll notification count every 30 seconds
use_resource(move || async move {
loop {
if let Ok(c) = fetch_notification_count().await {
count.set(c);
}
#[cfg(feature = "web")]
{
gloo_timers::future::TimeoutFuture::new(30_000).await;
}
Review

[medium] Inconsistent Error Handling in Resource Polling

The polling loop in use_resource handles errors from fetch_notification_count() by logging and continuing, while other similar operations in the same module use more explicit error reporting. This inconsistency could mask failures in notification fetching.

Suggested fix: Consider propagating errors from fetch_notification_count() to a global error state or logging them more explicitly to maintain consistency with the module's error handling patterns.

*Scanner: code-review/convention | *

**[medium] Inconsistent Error Handling in Resource Polling** The polling loop in `use_resource` handles errors from `fetch_notification_count()` by logging and continuing, while other similar operations in the same module use more explicit error reporting. This inconsistency could mask failures in notification fetching. Suggested fix: Consider propagating errors from `fetch_notification_count()` to a global error state or logging them more explicitly to maintain consistency with the module's error handling patterns. *Scanner: code-review/convention | * <!-- compliance-fp:e455f086ef5e6483caaea3fbea70fed3eae0e3d5b63e58f4cd6b057c7235e328 -->
#[cfg(not(feature = "web"))]
{
tokio::time::sleep(std::time::Duration::from_secs(30)).await;
}
}
});
// Load notifications when panel opens
Review

[high] Race condition in notification loading

In the load_notifications function, there's a potential race condition between setting is_loading.set(true) and the subsequent async block execution. If the component unmounts or is_open changes before the async block completes, the state updates might occur on an unmounted component, leading to undefined behavior or panics. The current implementation doesn't guard against such scenarios.

Suggested fix: Use Dioxus's built-in lifecycle management or cancellation tokens to ensure that async operations don't continue after the component has been unmounted or the state has changed unexpectedly.

*Scanner: code-review/logic | *

**[high] Race condition in notification loading** In the `load_notifications` function, there's a potential race condition between setting `is_loading.set(true)` and the subsequent async block execution. If the component unmounts or `is_open` changes before the async block completes, the state updates might occur on an unmounted component, leading to undefined behavior or panics. The current implementation doesn't guard against such scenarios. Suggested fix: Use Dioxus's built-in lifecycle management or cancellation tokens to ensure that async operations don't continue after the component has been unmounted or the state has changed unexpectedly. *Scanner: code-review/logic | * <!-- compliance-fp:33953e2ee10ef629d426a84e45c3d96e497f27512cf5b3a270ba52f42c96121b -->
let load_notifications = move |_| {
is_open.set(!is_open());
if !is_open() {
return;
}
is_loading.set(true);
spawn(async move {
if let Ok(resp) = fetch_notifications().await {
notifications.set(resp.data);
}
// Mark all as read when panel opens
let _ = mark_all_notifications_read().await;
count.set(0);
is_loading.set(false);
Review

[high] Incorrect notification dismissal logic

The notification dismissal logic uses a nested and_then chain to extract the $oid field from the notification ID, but this approach is flawed. If the ID structure doesn't exactly match the expected format (e.g., missing $oid key or different nesting), it will return None and the comparison != Some(&id) will fail, causing notifications to not be properly removed from the UI. Additionally, the dismissal handler passes id.clone() to the async block, but then tries to compare against n.id.as_ref().and_then(...).and_then(...) which may not match due to the complex extraction logic.

Suggested fix: Simplify the notification dismissal logic by using a direct string comparison on the notification ID field, assuming it's already a valid string. Alternatively, ensure the ID extraction logic is robust and handles all possible ID formats correctly.

*Scanner: code-review/logic | *

**[high] Incorrect notification dismissal logic** The notification dismissal logic uses a nested `and_then` chain to extract the `$oid` field from the notification ID, but this approach is flawed. If the ID structure doesn't exactly match the expected format (e.g., missing `$oid` key or different nesting), it will return `None` and the comparison `!= Some(&id)` will fail, causing notifications to not be properly removed from the UI. Additionally, the dismissal handler passes `id.clone()` to the async block, but then tries to compare against `n.id.as_ref().and_then(...).and_then(...)` which may not match due to the complex extraction logic. Suggested fix: Simplify the notification dismissal logic by using a direct string comparison on the notification ID field, assuming it's already a valid string. Alternatively, ensure the ID extraction logic is robust and handles all possible ID formats correctly. *Scanner: code-review/logic | * <!-- compliance-fp:60ad2fac2595934bb1595936c0dd5d13840a40ee5dd2ce6990b60f5b72884988 -->
});
};
let on_dismiss = move |id: String| {
spawn(async move {
let _ = dismiss_notification(id.clone()).await;
notifications.write().retain(|n| {
n.id.as_ref()
.and_then(|v| v.get("$oid"))
.and_then(|v| v.as_str())
Review

[medium] Complex boolean expression in notification dismissal

The notification dismissal logic uses a complex chain of and_then calls to extract the OID from the notification ID, which is hard to follow and prone to runtime panics if the structure doesn't match expectations. This could lead to silent failures or crashes when processing notifications.

Suggested fix: Extract the OID extraction into a separate helper function with proper error handling, or use a more robust deserialization approach instead of chaining multiple and_then calls.

*Scanner: code-review/complexity | *

**[medium] Complex boolean expression in notification dismissal** The notification dismissal logic uses a complex chain of `and_then` calls to extract the OID from the notification ID, which is hard to follow and prone to runtime panics if the structure doesn't match expectations. This could lead to silent failures or crashes when processing notifications. Suggested fix: Extract the OID extraction into a separate helper function with proper error handling, or use a more robust deserialization approach instead of chaining multiple `and_then` calls. *Scanner: code-review/complexity | * <!-- compliance-fp:6320974cba200bdd5acd34cb44afc46575043e2a0e18d0f501c085d1881763a1 -->
!= Some(&id)
});
});
};
rsx! {
div { class: "notification-bell-wrapper",
// Bell button
button {
class: "notification-bell-btn",
onclick: load_notifications,
title: "CVE Alerts",
Icon { icon: BsBell, width: 18, height: 18 }
if count() > 0 {
span { class: "notification-badge", "{count()}" }
}
}
// Dropdown panel
if is_open() {
div { class: "notification-panel",
div { class: "notification-panel-header",
span { "CVE Alerts" }
button {
class: "notification-close-btn",
onclick: move |_| is_open.set(false),
Icon { icon: BsX, width: 16, height: 16 }
Review

[medium] Deeply nested control flow in notification rendering

The notification panel rendering contains deeply nested conditional logic (if/else chains) that makes it difficult to understand the flow and increases the risk of logic errors when modifying the component. The nested structure makes it hard to reason about what gets rendered under what conditions.

Suggested fix: Flatten the conditional rendering by extracting each state (loading, empty, notifications) into separate components or functions, reducing nesting levels and improving readability.

*Scanner: code-review/complexity | *

**[medium] Deeply nested control flow in notification rendering** The notification panel rendering contains deeply nested conditional logic (if/else chains) that makes it difficult to understand the flow and increases the risk of logic errors when modifying the component. The nested structure makes it hard to reason about what gets rendered under what conditions. Suggested fix: Flatten the conditional rendering by extracting each state (loading, empty, notifications) into separate components or functions, reducing nesting levels and improving readability. *Scanner: code-review/complexity | * <!-- compliance-fp:bd5b02f2fcbf56e7e8ed9034ef2384d12c35734e4503a99c4ac8171e0b42e7a8 -->
}
}
div { class: "notification-panel-body",
if is_loading() {
div { class: "notification-loading", "Loading..." }
} else if notifications().is_empty() {
div { class: "notification-empty",
Icon { icon: BsShieldCheck, width: 32, height: 32 }
p { "No CVE alerts" }
}
} else {
for notif in notifications().iter() {
{
let id = notif.id.as_ref()
.and_then(|v| v.get("$oid"))
.and_then(|v| v.as_str())
.unwrap_or("")
.to_string();
let sev_class = match notif.severity.as_str() {
"critical" => "sev-critical",
"high" => "sev-high",
"medium" => "sev-medium",
_ => "sev-low",
};
let dismiss_id = id.clone();
rsx! {
div { class: "notification-item",
div { class: "notification-item-header",
span { class: "notification-sev {sev_class}",
"{notif.severity.to_uppercase()}"
}
span { class: "notification-cve-id",
if let Some(ref url) = notif.url {
a { href: "{url}", target: "_blank", "{notif.cve_id}" }
} else {
"{notif.cve_id}"
}
}
if let Some(score) = notif.cvss_score {
span { class: "notification-cvss", "CVSS {score:.1}" }
}
button {
class: "notification-dismiss-btn",
title: "Dismiss",
onclick: move |_| on_dismiss(dismiss_id.clone()),
Review

[high] Potential XSS vulnerability in notification CVE ID rendering

The notification component renders CVE IDs directly from API responses without sanitization. If an attacker can control the CVE ID field in the notification data, they could inject malicious HTML/JavaScript into the page. This occurs in the notification item rendering where '{notif.cve_id}' is directly inserted into the DOM.

Suggested fix: Sanitize the CVE ID before rendering it to prevent XSS. Use a library like dioxus-html or manually escape HTML characters in the CVE ID field before displaying it.

Scanner: code-review/security | CWE: CWE-79

**[high] Potential XSS vulnerability in notification CVE ID rendering** The notification component renders CVE IDs directly from API responses without sanitization. If an attacker can control the CVE ID field in the notification data, they could inject malicious HTML/JavaScript into the page. This occurs in the notification item rendering where '{notif.cve_id}' is directly inserted into the DOM. Suggested fix: Sanitize the CVE ID before rendering it to prevent XSS. Use a library like `dioxus-html` or manually escape HTML characters in the CVE ID field before displaying it. *Scanner: code-review/security | CWE: CWE-79* <!-- compliance-fp:4bea04577fe797ea08b8c5d70524da55c58323d3011013056ed675655a8a88df -->
Review

[medium] Potential Panic in Notification Dismissal

The on_dismiss handler uses unwrap_or("") when extracting the notification ID, which could panic if the ObjectId structure changes unexpectedly. This creates an inconsistency with the module's established pattern of graceful error handling.

Suggested fix: Use a more robust approach like map_or_else or match to handle the case where the ObjectId might not be present, maintaining consistency with the module's error handling strategy.

*Scanner: code-review/convention | *

**[medium] Potential Panic in Notification Dismissal** The `on_dismiss` handler uses `unwrap_or("")` when extracting the notification ID, which could panic if the ObjectId structure changes unexpectedly. This creates an inconsistency with the module's established pattern of graceful error handling. Suggested fix: Use a more robust approach like `map_or_else` or `match` to handle the case where the ObjectId might not be present, maintaining consistency with the module's error handling strategy. *Scanner: code-review/convention | * <!-- compliance-fp:dac048ae73c7e1dfdec3ffa14976548a6b565b7626b468f1f56f50b97c7774f6 -->
Review

[medium] Unwrapped Option in Notification Processing

The code uses .unwrap_or("") on a potentially critical value extraction path, which could lead to silent failures if the MongoDB ObjectId structure changes or becomes malformed. This violates the module's error handling consistency where other operations use if let Ok(...) patterns.

Suggested fix: Replace .unwrap_or("") with proper error handling using if let Some(...) or match to handle the case where the ObjectId might be missing or malformed, ensuring consistent error propagation throughout the module.

*Scanner: code-review/convention | *

**[medium] Unwrapped Option in Notification Processing** The code uses `.unwrap_or("")` on a potentially critical value extraction path, which could lead to silent failures if the MongoDB ObjectId structure changes or becomes malformed. This violates the module's error handling consistency where other operations use `if let Ok(...)` patterns. Suggested fix: Replace `.unwrap_or("")` with proper error handling using `if let Some(...)` or `match` to handle the case where the ObjectId might be missing or malformed, ensuring consistent error propagation throughout the module. *Scanner: code-review/convention | * <!-- compliance-fp:78fe4c1c13ebe3d2b7bd9a486e31d12e0117ea8c6062d4fcdf23dae78ff540ce -->
Icon { icon: BsXCircle, width: 14, height: 14 }
}
}
div { class: "notification-item-pkg",
"{notif.package_name} {notif.package_version}"
}
div { class: "notification-item-repo",
"{notif.repo_name}"
}
if let Some(ref summary) = notif.summary {
div { class: "notification-item-summary",
"{summary}"
}
Review

[high] Potential XSS vulnerability in notification summary rendering

The notification component renders summary text directly from API responses without sanitization. If an attacker can control the summary field in notification data, they could inject malicious HTML/JavaScript. This occurs in the notification item rendering where '{summary}' is directly inserted into the DOM.

Suggested fix: Sanitize the summary text before rendering it to prevent XSS. Either escape HTML characters or use a proper HTML sanitization library before inserting into the DOM.

Scanner: code-review/security | CWE: CWE-79

**[high] Potential XSS vulnerability in notification summary rendering** The notification component renders summary text directly from API responses without sanitization. If an attacker can control the summary field in notification data, they could inject malicious HTML/JavaScript. This occurs in the notification item rendering where '{summary}' is directly inserted into the DOM. Suggested fix: Sanitize the summary text before rendering it to prevent XSS. Either escape HTML characters or use a proper HTML sanitization library before inserting into the DOM. *Scanner: code-review/security | CWE: CWE-79* <!-- compliance-fp:e06c0860588342cb98d68b273e11d9800c6e3f8975d2829af93f9390a0d19d91 -->
}
}
}
}
}
}
}
}
}
}
}
}

View File

@@ -8,6 +8,7 @@ pub mod graph;
pub mod help_chat;
pub mod issues;
pub mod mcp;
pub mod notifications;
pub mod pentest;
#[allow(clippy::too_many_arguments)]
pub mod repositories;

View File

@@ -0,0 +1,91 @@
use dioxus::prelude::*;
use serde::{Deserialize, Serialize};
#[derive(Debug, Clone, Serialize, Deserialize, Default)]
pub struct NotificationListResponse {
pub data: Vec<CveNotificationData>,
#[serde(default)]
pub total: Option<u64>,
}
#[derive(Debug, Clone, Serialize, Deserialize, Default)]
pub struct CveNotificationData {
#[serde(rename = "_id")]
pub id: Option<serde_json::Value>,
pub cve_id: String,
Review

[medium] Unvalidated API responses in notification data structures

The CveNotificationData struct uses serde_json::Value for id and created_at fields which bypasses compile-time type checking. This could lead to runtime panics or unexpected behavior if the API returns unexpected data types for these fields, especially since they're marked as optional but still parsed as generic JSON values.

Suggested fix: Consider using more specific types for id and created_at fields, or add validation logic to ensure these fields contain expected data types when deserializing from the API response.

*Scanner: code-review/convention | *

**[medium] Unvalidated API responses in notification data structures** The CveNotificationData struct uses serde_json::Value for id and created_at fields which bypasses compile-time type checking. This could lead to runtime panics or unexpected behavior if the API returns unexpected data types for these fields, especially since they're marked as optional but still parsed as generic JSON values. Suggested fix: Consider using more specific types for id and created_at fields, or add validation logic to ensure these fields contain expected data types when deserializing from the API response. *Scanner: code-review/convention | * <!-- compliance-fp:4bcbc6b881d37e2bfc7569d61d867c4441ab71f41422424540ef71a852459bec -->
pub repo_name: String,
pub package_name: String,
pub package_version: String,
pub severity: String,
pub cvss_score: Option<f64>,
pub summary: Option<String>,
pub url: Option<String>,
pub status: String,
#[serde(default)]
pub created_at: Option<serde_json::Value>,
}
Review

[high] Missing error handling for HTTP request failures

The fetch_notification_count, fetch_notifications, mark_all_notifications_read, and dismiss_notification functions do not check the HTTP status code of the response. If the API returns an error status (like 4xx or 5xx), the code will still attempt to parse the response body as success, potentially leading to incorrect behavior or panics when trying to deserialize error responses.

Suggested fix: Add explicit status code checking before deserializing the response body. For example: if !resp.status().is_success() { return Err(ServerFnError::new(format!("HTTP Error: {}", resp.status()))); }

*Scanner: code-review/logic | *

**[high] Missing error handling for HTTP request failures** The `fetch_notification_count`, `fetch_notifications`, `mark_all_notifications_read`, and `dismiss_notification` functions do not check the HTTP status code of the response. If the API returns an error status (like 4xx or 5xx), the code will still attempt to parse the response body as success, potentially leading to incorrect behavior or panics when trying to deserialize error responses. Suggested fix: Add explicit status code checking before deserializing the response body. For example: `if !resp.status().is_success() { return Err(ServerFnError::new(format!("HTTP Error: {}", resp.status()))); }` *Scanner: code-review/logic | * <!-- compliance-fp:16047c1ed6f8992ba23b5e56cf7acb100051740d618c64a538c9ddb5017adfb8 -->
Review

[high] Server-Side Request Forgery (SSRF) via User-Controlled URL

The fetch_notifications, fetch_notification_count, mark_all_notifications_read, and dismiss_notification server functions construct URLs by concatenating user-provided data from state.agent_api_url without proper validation. If an attacker can control the agent_api_url configuration value, they could redirect requests to internal services or external malicious endpoints, enabling SSRF attacks.

Suggested fix: Validate and sanitize the agent_api_url configuration value to ensure it points to allowed domains or use a whitelist approach. Consider using a proxy or restricted network access for API calls.

Scanner: code-review/security | CWE: CWE-918

**[high] Server-Side Request Forgery (SSRF) via User-Controlled URL** The `fetch_notifications`, `fetch_notification_count`, `mark_all_notifications_read`, and `dismiss_notification` server functions construct URLs by concatenating user-provided data from `state.agent_api_url` without proper validation. If an attacker can control the `agent_api_url` configuration value, they could redirect requests to internal services or external malicious endpoints, enabling SSRF attacks. Suggested fix: Validate and sanitize the `agent_api_url` configuration value to ensure it points to allowed domains or use a whitelist approach. Consider using a proxy or restricted network access for API calls. *Scanner: code-review/security | CWE: CWE-918* <!-- compliance-fp:9f6223c6fdc7cf34ddd198fa78ba18451eb9ef04f2a7f37803f4e0ddd49f7b42 -->
#[derive(Debug, Clone, Serialize, Deserialize, Default)]
pub struct NotificationCountResponse {
pub count: u64,
}
#[server]
pub async fn fetch_notification_count() -> Result<u64, ServerFnError> {
let state: super::server_state::ServerState =
dioxus_fullstack::FullstackContext::extract().await?;
Review

[medium] Complex boolean expressions in error handling

The error handling pattern uses nested map_err calls which can make it difficult to trace the flow of errors and may lead to less informative error messages during debugging.

Suggested fix: Consider using a more explicit error handling approach with early returns or a custom error conversion function to improve readability and maintainability.

*Scanner: code-review/complexity | *

**[medium] Complex boolean expressions in error handling** The error handling pattern uses nested `map_err` calls which can make it difficult to trace the flow of errors and may lead to less informative error messages during debugging. Suggested fix: Consider using a more explicit error handling approach with early returns or a custom error conversion function to improve readability and maintainability. *Scanner: code-review/complexity | * <!-- compliance-fp:64592d8433d089395e62949972ac8db5f27cba694ba6639253828244d5ff8f97 -->
Review

[medium] Inconsistent error handling in notification API

The notification API functions use inconsistent error handling patterns. Some functions map errors to ServerFnError::new(e.to_string()) while others could potentially hide failures by not properly propagating HTTP status codes or response parsing errors. The error mapping should be consistent across all server functions.

Suggested fix: Standardize error handling by ensuring all HTTP request failures and JSON parsing errors are consistently wrapped with appropriate ServerFnError variants, and consider including HTTP status codes in error messages for better debugging.

*Scanner: code-review/convention | *

**[medium] Inconsistent error handling in notification API** The notification API functions use inconsistent error handling patterns. Some functions map errors to ServerFnError::new(e.to_string()) while others could potentially hide failures by not properly propagating HTTP status codes or response parsing errors. The error mapping should be consistent across all server functions. Suggested fix: Standardize error handling by ensuring all HTTP request failures and JSON parsing errors are consistently wrapped with appropriate ServerFnError variants, and consider including HTTP status codes in error messages for better debugging. *Scanner: code-review/convention | * <!-- compliance-fp:5269eecb73d798b26d7eb1f71c9282cceff2ade8d914cb6c87b2ce38d3507a8b -->
let url = format!("{}/api/v1/notifications/count", state.agent_api_url);
let resp = reqwest::get(&url)
.await
.map_err(|e| ServerFnError::new(e.to_string()))?;
let body: NotificationCountResponse = resp
.json()
.await
.map_err(|e| ServerFnError::new(e.to_string()))?;
Ok(body.count)
}
#[server]
pub async fn fetch_notifications() -> Result<NotificationListResponse, ServerFnError> {
let state: super::server_state::ServerState =
dioxus_fullstack::FullstackContext::extract().await?;
let url = format!("{}/api/v1/notifications?limit=20", state.agent_api_url);
let resp = reqwest::get(&url)
.await
.map_err(|e| ServerFnError::new(e.to_string()))?;
let body: NotificationListResponse = resp
.json()
.await
.map_err(|e| ServerFnError::new(e.to_string()))?;
Ok(body)
}
#[server]
pub async fn mark_all_notifications_read() -> Result<(), ServerFnError> {
let state: super::server_state::ServerState =
dioxus_fullstack::FullstackContext::extract().await?;
let url = format!("{}/api/v1/notifications/read-all", state.agent_api_url);
reqwest::Client::new()
.post(&url)
.send()
.await
.map_err(|e| ServerFnError::new(e.to_string()))?;
Ok(())
}
#[server]
pub async fn dismiss_notification(id: String) -> Result<(), ServerFnError> {
let state: super::server_state::ServerState =
dioxus_fullstack::FullstackContext::extract().await?;
Review

[medium] Potential silent failure in dismiss_notification

The dismiss_notification function sends a PATCH request but doesn't check the HTTP response status code before considering the operation successful. This could lead to silent failures when the API returns an error status code, making it appear as though the notification was dismissed when it wasn't.

Suggested fix: Add response status code validation using resp.status().is_success() or similar before considering the operation complete, and return appropriate ServerFnError for non-success status codes.

*Scanner: code-review/convention | *

**[medium] Potential silent failure in dismiss_notification** The dismiss_notification function sends a PATCH request but doesn't check the HTTP response status code before considering the operation successful. This could lead to silent failures when the API returns an error status code, making it appear as though the notification was dismissed when it wasn't. Suggested fix: Add response status code validation using `resp.status().is_success()` or similar before considering the operation complete, and return appropriate ServerFnError for non-success status codes. *Scanner: code-review/convention | * <!-- compliance-fp:3187491f1dee8453ad0eb374529e9b949d4ccaa679a583f5874eb56940ffa3d3 -->
Review

[medium] Incorrect HTTP method for dismissing notification

The dismiss_notification function uses PATCH method but the API endpoint appears to be designed for dismissal operations which typically would use POST or DELETE. Using PATCH might not be semantically correct for this operation, though this could also be a design choice depending on the backend implementation.

Suggested fix: Verify that the API endpoint actually supports PATCH requests for dismissing notifications. If not, consider using POST or DELETE instead.

*Scanner: code-review/logic | *

**[medium] Incorrect HTTP method for dismissing notification** The `dismiss_notification` function uses PATCH method but the API endpoint appears to be designed for dismissal operations which typically would use POST or DELETE. Using PATCH might not be semantically correct for this operation, though this could also be a design choice depending on the backend implementation. Suggested fix: Verify that the API endpoint actually supports PATCH requests for dismissing notifications. If not, consider using POST or DELETE instead. *Scanner: code-review/logic | * <!-- compliance-fp:1920a0be46c40a4f2d3e04414a4148f1b3379bc271ec0d46afe3a66076d250db -->
let url = format!("{}/api/v1/notifications/{id}/dismiss", state.agent_api_url);
reqwest::Client::new()
.patch(&url)
.send()
.await
.map_err(|e| ServerFnError::new(e.to_string()))?;
Ok(())
}