fix: CVE notifications during scan + help chat doc loading + Dockerfile #55
@@ -3877,3 +3877,15 @@ tbody tr:last-child td {
|
||||
.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; }
|
||||
|
||||
/* ═══════════════════════════════════════════════════════════════
|
||||
COPY BUTTON — Reusable clipboard copy component
|
||||
═══════════════════════════════════════════════════════════════ */
|
||||
.copy-btn { background: none; border: 1px solid var(--border); border-radius: 6px; padding: 5px 7px; color: var(--text-secondary); cursor: pointer; display: inline-flex; align-items: center; transition: color 0.15s, border-color 0.15s, background 0.15s; flex-shrink: 0; }
|
||||
.copy-btn:hover { color: var(--accent); border-color: var(--accent); background: var(--accent-muted); }
|
||||
.copy-btn-sm { padding: 3px 5px; border-radius: 4px; }
|
||||
/* Copyable inline field pattern: value + copy button side by side */
|
||||
.copyable { display: flex; align-items: center; gap: 6px; }
|
||||
.copyable code, .copyable .mono { flex: 1; min-width: 0; overflow: hidden; text-overflow: ellipsis; white-space: nowrap; }
|
||||
.code-snippet-wrapper { position: relative; }
|
||||
.code-snippet-header { display: flex; align-items: center; justify-content: space-between; margin-bottom: 4px; gap: 8px; }
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
use dioxus::prelude::*;
|
||||
|
||||
use crate::components::copy_button::CopyButton;
|
||||
|
||||
#[component]
|
||||
pub fn CodeSnippet(
|
||||
code: String,
|
||||
@@ -7,15 +9,18 @@ pub fn CodeSnippet(
|
||||
#[props(default)] line_number: u32,
|
||||
) -> Element {
|
||||
rsx! {
|
||||
div {
|
||||
if !file_path.is_empty() {
|
||||
div {
|
||||
style: "font-size: 12px; color: var(--text-secondary); margin-bottom: 4px; font-family: monospace;",
|
||||
"{file_path}"
|
||||
if line_number > 0 {
|
||||
":{line_number}"
|
||||
div { class: "code-snippet-wrapper",
|
||||
div { class: "code-snippet-header",
|
||||
if !file_path.is_empty() {
|
||||
span {
|
||||
style: "font-size: 12px; color: var(--text-secondary); font-family: monospace;",
|
||||
"{file_path}"
|
||||
if line_number > 0 {
|
||||
":{line_number}"
|
||||
}
|
||||
}
|
||||
}
|
||||
CopyButton { value: code.clone(), small: true }
|
||||
}
|
||||
pre { class: "code-block", "{code}" }
|
||||
}
|
||||
|
||||
45
compliance-dashboard/src/components/copy_button.rs
Normal file
@@ -0,0 +1,45 @@
|
||||
use dioxus::prelude::*;
|
||||
use dioxus_free_icons::icons::bs_icons::*;
|
||||
use dioxus_free_icons::Icon;
|
||||
|
||||
/// A small copy-to-clipboard button that shows a checkmark after copying.
|
||||
///
|
||||
/// Usage: `CopyButton { value: "text to copy" }`
|
||||
#[component]
|
||||
pub fn CopyButton(value: String, #[props(default = false)] small: bool) -> Element {
|
||||
let mut copied = use_signal(|| false);
|
||||
|
||||
let size = if small { 12 } else { 14 };
|
||||
let class = if small {
|
||||
"copy-btn copy-btn-sm"
|
||||
} else {
|
||||
"copy-btn"
|
||||
};
|
||||
|
||||
|
|
||||
rsx! {
|
||||
button {
|
||||
sharang
commented
[medium] Complex boolean expression in conditional rendering The conditional rendering logic uses a complex boolean expression that mixes multiple conditions and string formatting in a way that's hard to follow. Specifically, the title attribute uses a ternary operator with string literals that could be simplified. Suggested fix: Simplify the title attribute by extracting the conditional logic into a separate variable or using a more explicit if-else structure to improve readability. *Scanner: code-review/complexity | * **[medium] Complex boolean expression in conditional rendering**
The conditional rendering logic uses a complex boolean expression that mixes multiple conditions and string formatting in a way that's hard to follow. Specifically, the title attribute uses a ternary operator with string literals that could be simplified.
Suggested fix: Simplify the title attribute by extracting the conditional logic into a separate variable or using a more explicit if-else structure to improve readability.
*Scanner: code-review/complexity | *
<!-- compliance-fp:62e907b94ed58051a23185bad2f5d84e935d8bc4fc3bca5c12eede2541bf6430 -->
|
||||
class: class,
|
||||
title: if copied() { "Copied!" } else { "Copy to clipboard" },
|
||||
|
sharang
commented
[medium] Potential security vulnerability in JavaScript injection The code manually escapes single quotes and backslashes for JavaScript string interpolation, but this approach is vulnerable to XSS attacks and doesn't properly escape all special characters. The current implementation could be exploited if the copied text contains malicious content. Suggested fix: Use a proper escaping mechanism or consider using the Clipboard API directly through web_sys instead of injecting raw JavaScript strings. *Scanner: code-review/convention | * **[medium] Potential security vulnerability in JavaScript injection**
The code manually escapes single quotes and backslashes for JavaScript string interpolation, but this approach is vulnerable to XSS attacks and doesn't properly escape all special characters. The current implementation could be exploited if the copied text contains malicious content.
Suggested fix: Use a proper escaping mechanism or consider using the Clipboard API directly through web_sys instead of injecting raw JavaScript strings.
*Scanner: code-review/convention | *
<!-- compliance-fp:a46aa9630125de1e1b4b1490360747499e9f97248f9af42f563623df18c8175b -->
sharang
commented
[medium] Potential security vulnerability in JavaScript string escaping The JavaScript string escaping logic only handles backslash, single quote, newline, and carriage return characters. However, it doesn't escape other potentially dangerous characters like double quotes or control characters that could break out of the JavaScript string context when used in certain contexts, potentially leading to XSS vulnerabilities. Suggested fix: Use a more comprehensive escaping mechanism or a dedicated JavaScript escaping library to properly escape all special characters that could break out of JavaScript string literals. *Scanner: code-review/convention | * **[medium] Potential security vulnerability in JavaScript string escaping**
The JavaScript string escaping logic only handles backslash, single quote, newline, and carriage return characters. However, it doesn't escape other potentially dangerous characters like double quotes or control characters that could break out of the JavaScript string context when used in certain contexts, potentially leading to XSS vulnerabilities.
Suggested fix: Use a more comprehensive escaping mechanism or a dedicated JavaScript escaping library to properly escape all special characters that could break out of JavaScript string literals.
*Scanner: code-review/convention | *
<!-- compliance-fp:067fb60c50814224f51114ba611aa5fea72282e2db5dd9895be8749d3c59bc35 -->
|
||||
onclick: move |_| {
|
||||
let val = value.clone();
|
||||
|
sharang
commented
[high] Incorrect JavaScript string escaping in copy functionality The JavaScript string escaping in the copy button implementation doesn't properly handle all cases that could break the generated JavaScript. Specifically, if the copied text contains a single quote followed by a backslash, it will result in malformed JavaScript. For example, if Suggested fix: Use a proper JavaScript string escaping function or encode the value using a more robust method like JSON.stringify instead of manual replacement. Alternatively, use a template literal approach with proper escaping. *Scanner: code-review/logic | * **[high] Incorrect JavaScript string escaping in copy functionality**
The JavaScript string escaping in the copy button implementation doesn't properly handle all cases that could break the generated JavaScript. Specifically, if the copied text contains a single quote followed by a backslash, it will result in malformed JavaScript. For example, if `value` is "It\'s a test", the escaped version becomes "It\\\'s a test" which when embedded in the JS string literal becomes: navigator.clipboard.writeText('It\\\'s a test') - this will not work correctly because the single quote is not properly escaped within the JS string.
Suggested fix: Use a proper JavaScript string escaping function or encode the value using a more robust method like JSON.stringify instead of manual replacement. Alternatively, use a template literal approach with proper escaping.
*Scanner: code-review/logic | *
<!-- compliance-fp:7cb3d70d745dbaf52acd935b838873788fb638662e5b49b96cc6c05ce2a07505 -->
|
||||
// Escape single quotes for JS
|
||||
|
sharang
commented
[medium] Inconsistent error handling in copy button The CopyButton component uses Suggested fix: Add proper error handling around the clipboard operation. Consider using a more robust approach like calling a JS function that returns a promise and handling both success and failure cases. *Scanner: code-review/convention | * **[medium] Inconsistent error handling in copy button**
The CopyButton component uses `document::eval(&js)` which can fail silently. If the JavaScript execution fails, the UI will still show 'Copied!' but the actual copy operation may have failed. The component should handle potential errors from the clipboard API and provide appropriate feedback to the user.
Suggested fix: Add proper error handling around the clipboard operation. Consider using a more robust approach like calling a JS function that returns a promise and handling both success and failure cases.
*Scanner: code-review/convention | *
<!-- compliance-fp:e4197dc6c9724e894671628be9b225edf52664ac3dd631e1890740783d6f1cc5 -->
sharang
commented
[high] Potential XSS via Copy Button The CopyButton component directly interpolates user-provided 'value' into JavaScript code without proper sanitization. Although the code attempts to escape single quotes and backslashes, it's insufficient to prevent XSS when the value contains malicious JavaScript. An attacker could inject a value like "'; alert(1); //" which would result in executable JavaScript being evaluated by navigator.clipboard.writeText(). This allows for potential XSS exploitation through the copy functionality. Suggested fix: Use a more robust escaping mechanism or sanitize the input before inserting into JavaScript. Consider using a library specifically designed for HTML/JS escaping, or ensure all user inputs are properly encoded according to JavaScript string literal rules. Scanner: code-review/security | CWE: CWE-79 **[high] Potential XSS via Copy Button**
The CopyButton component directly interpolates user-provided 'value' into JavaScript code without proper sanitization. Although the code attempts to escape single quotes and backslashes, it's insufficient to prevent XSS when the value contains malicious JavaScript. An attacker could inject a value like "'; alert(1); //" which would result in executable JavaScript being evaluated by navigator.clipboard.writeText(). This allows for potential XSS exploitation through the copy functionality.
Suggested fix: Use a more robust escaping mechanism or sanitize the input before inserting into JavaScript. Consider using a library specifically designed for HTML/JS escaping, or ensure all user inputs are properly encoded according to JavaScript string literal rules.
*Scanner: code-review/security | CWE: CWE-79*
<!-- compliance-fp:40464c023887db6f7315f26111f91822d77feef57d239367c5edd5bc0ba492f9 -->
|
||||
let escaped = val.replace('\\', "\\\\").replace('\'', "\\'");
|
||||
let js = format!("navigator.clipboard.writeText('{escaped}')");
|
||||
|
sharang
commented
[high] Potential XSS via Copy Button The CopyButton component directly incorporates user-provided 'value' into JavaScript code without proper sanitization. While the code attempts to escape certain characters, it's insufficient to prevent XSS when the value contains malicious content that could be interpreted as JavaScript when executed in the browser context. Suggested fix: Use a more robust escaping mechanism or avoid inline JavaScript execution entirely. Consider using a safer method like creating a temporary textarea element and using document.execCommand('copy') instead of navigator.clipboard.writeText(). Scanner: code-review/security | CWE: CWE-79 **[high] Potential XSS via Copy Button**
The CopyButton component directly incorporates user-provided 'value' into JavaScript code without proper sanitization. While the code attempts to escape certain characters, it's insufficient to prevent XSS when the value contains malicious content that could be interpreted as JavaScript when executed in the browser context.
Suggested fix: Use a more robust escaping mechanism or avoid inline JavaScript execution entirely. Consider using a safer method like creating a temporary textarea element and using document.execCommand('copy') instead of navigator.clipboard.writeText().
*Scanner: code-review/security | CWE: CWE-79*
<!-- compliance-fp:f846e60574550662dea9fe90e0e2e40f7dc0c1e669c1096a2d1e77c7f89b07b0 -->
sharang
commented
[medium] Complex boolean expression in conditional rendering The conditional rendering logic uses a complex boolean expression with multiple nested conditions that could be simplified for better readability and reduced bug risk. The 'if copied()' condition is used both for determining the button's icon and for setting the title attribute. Suggested fix: Extract the conditional logic into separate variables or functions to make the intent clearer and reduce the chance of errors when modifying the component behavior. *Scanner: code-review/complexity | * **[medium] Complex boolean expression in conditional rendering**
The conditional rendering logic uses a complex boolean expression with multiple nested conditions that could be simplified for better readability and reduced bug risk. The 'if copied()' condition is used both for determining the button's icon and for setting the title attribute.
Suggested fix: Extract the conditional logic into separate variables or functions to make the intent clearer and reduce the chance of errors when modifying the component behavior.
*Scanner: code-review/complexity | *
<!-- compliance-fp:afdff2902065a38a633365c759efaf6cd7e9f479161e3e2e1c1b83d85328a183 -->
|
||||
document::eval(&js);
|
||||
|
sharang
commented
[medium] Inconsistent async timeout handling The component uses different async timeout mechanisms for web vs non-web targets (gloo_timers vs tokio). This creates inconsistent behavior between platforms and could lead to timing issues or compilation errors on non-web targets. Suggested fix: Standardize on one async runtime or ensure consistent behavior across platforms by using platform-agnostic alternatives. *Scanner: code-review/convention | * **[medium] Inconsistent async timeout handling**
The component uses different async timeout mechanisms for web vs non-web targets (gloo_timers vs tokio). This creates inconsistent behavior between platforms and could lead to timing issues or compilation errors on non-web targets.
Suggested fix: Standardize on one async runtime or ensure consistent behavior across platforms by using platform-agnostic alternatives.
*Scanner: code-review/convention | *
<!-- compliance-fp:1314f362a2fe49b29ffc8b06d8bfb2d678a13f666bef7e32b57ede106c27cf88 -->
sharang
commented
[medium] Inconsistent error handling in copy button The CopyButton component uses Suggested fix: Add proper error handling around the clipboard operation by either catching JavaScript errors or using a more robust clipboard API approach that provides feedback on success/failure. *Scanner: code-review/convention | * **[medium] Inconsistent error handling in copy button**
The CopyButton component uses `document::eval(&js)` to execute JavaScript for copying to clipboard, but doesn't handle potential errors from the clipboard API. If the clipboard operation fails (which can happen due to browser permissions or other restrictions), the UI will still show success (copied state) without any indication of failure.
Suggested fix: Add proper error handling around the clipboard operation by either catching JavaScript errors or using a more robust clipboard API approach that provides feedback on success/failure.
*Scanner: code-review/convention | *
<!-- compliance-fp:ba7d78289cd65f3daed40ba740a4378eb55ee28d01cc6e24c43d997a9d3eba30 -->
|
||||
copied.set(true);
|
||||
|
sharang
commented
[medium] Deeply nested control flow in async block The async block contains nested conditional compilation directives (#cfg) which creates a complex control flow path that's difficult to reason about. The branching logic between web and non-web platforms is embedded within an async context, making it harder to follow the execution path. Suggested fix: Extract the platform-specific timeout logic into separate functions or use a platform-agnostic approach to reduce nesting and make the control flow clearer. *Scanner: code-review/complexity | * **[medium] Deeply nested control flow in async block**
The async block contains nested conditional compilation directives (#cfg) which creates a complex control flow path that's difficult to reason about. The branching logic between web and non-web platforms is embedded within an async context, making it harder to follow the execution path.
Suggested fix: Extract the platform-specific timeout logic into separate functions or use a platform-agnostic approach to reduce nesting and make the control flow clearer.
*Scanner: code-review/complexity | *
<!-- compliance-fp:8559df88144649599c41bf223ef2f2b952a1c708827cdfeb7c91825776b339be -->
|
||||
spawn(async move {
|
||||
#[cfg(feature = "web")]
|
||||
gloo_timers::future::TimeoutFuture::new(2000).await;
|
||||
|
sharang
commented
[medium] Inconsistent async behavior between web and non-web targets The CopyButton component uses different async mechanisms for different targets (gloo_timers::future::TimeoutFuture for web, tokio::time::sleep for non-web). This inconsistency could lead to different timing behaviors or potential runtime errors if the non-web target doesn't have tokio available or if the async runtime isn't properly configured. Suggested fix: Standardize on a single async approach or ensure both paths are properly handled with appropriate feature detection and fallbacks. *Scanner: code-review/convention | * **[medium] Inconsistent async behavior between web and non-web targets**
The CopyButton component uses different async mechanisms for different targets (gloo_timers::future::TimeoutFuture for web, tokio::time::sleep for non-web). This inconsistency could lead to different timing behaviors or potential runtime errors if the non-web target doesn't have tokio available or if the async runtime isn't properly configured.
Suggested fix: Standardize on a single async approach or ensure both paths are properly handled with appropriate feature detection and fallbacks.
*Scanner: code-review/convention | *
<!-- compliance-fp:2655fb38ae454e02f649af573b7dbca84badbc20ddc0c6e2347b4e7b294a6797 -->
|
||||
#[cfg(not(feature = "web"))]
|
||||
tokio::time::sleep(std::time::Duration::from_secs(2)).await;
|
||||
copied.set(false);
|
||||
});
|
||||
},
|
||||
if copied() {
|
||||
Icon { icon: BsCheckLg, width: size, height: size }
|
||||
} else {
|
||||
Icon { icon: BsClipboard, width: size, height: size }
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -2,6 +2,7 @@ pub mod app_shell;
|
||||
pub mod attack_chain;
|
||||
pub mod code_inspector;
|
||||
pub mod code_snippet;
|
||||
pub mod copy_button;
|
||||
pub mod file_tree;
|
||||
pub mod help_chat;
|
||||
pub mod notification_bell;
|
||||
|
||||
@@ -259,7 +259,10 @@ pub fn McpServersPage() -> Element {
|
||||
div { class: "mcp-detail-row",
|
||||
Icon { icon: BsGlobe, width: 13, height: 13 }
|
||||
span { class: "mcp-detail-label", "Endpoint" }
|
||||
code { class: "mcp-detail-value", "{server.endpoint_url}" }
|
||||
div { class: "copyable",
|
||||
code { class: "mcp-detail-value", "{server.endpoint_url}" }
|
||||
crate::components::copy_button::CopyButton { value: server.endpoint_url.clone(), small: true }
|
||||
}
|
||||
}
|
||||
div { class: "mcp-detail-row",
|
||||
Icon { icon: BsHddNetwork, width: 13, height: 13 }
|
||||
|
||||
@@ -137,11 +137,18 @@ pub fn RepositoriesPage() -> Element {
|
||||
"For SSH URLs: add this deploy key (read-only) to your repository"
|
||||
}
|
||||
div {
|
||||
style: "margin-top: 4px; padding: 8px; background: var(--bg-secondary); border-radius: 4px; font-family: monospace; font-size: 11px; word-break: break-all; user-select: all;",
|
||||
if ssh_public_key().is_empty() {
|
||||
"Loading..."
|
||||
} else {
|
||||
"{ssh_public_key}"
|
||||
class: "copyable",
|
||||
style: "margin-top: 4px; padding: 8px; background: var(--bg-secondary); border-radius: 4px;",
|
||||
code {
|
||||
style: "font-size: 11px; word-break: break-all; user-select: all;",
|
||||
if ssh_public_key().is_empty() {
|
||||
"Loading..."
|
||||
|
sharang
commented
[medium] Potential race condition in SSH public key display The SSH public key is checked for emptiness twice - once for displaying 'Loading...' and again for determining whether to show the copy button. If the key becomes empty between these two checks, it could result in inconsistent UI state where the copy button appears but the key is actually empty. Suggested fix: Store the SSH public key result in a local variable before checking its emptiness to ensure consistency across both conditions. *Scanner: code-review/logic | * **[medium] Potential race condition in SSH public key display**
The SSH public key is checked for emptiness twice - once for displaying 'Loading...' and again for determining whether to show the copy button. If the key becomes empty between these two checks, it could result in inconsistent UI state where the copy button appears but the key is actually empty.
Suggested fix: Store the SSH public key result in a local variable before checking its emptiness to ensure consistency across both conditions.
*Scanner: code-review/logic | *
<!-- compliance-fp:8e8d7aa2dde6cf0672a1a99ad7f35ed0ff16798088569a6517f4d1ab05885d45 -->
sharang
commented
[medium] Complex boolean expression in conditional rendering The code uses repeated calls to Suggested fix: Store the result of *Scanner: code-review/complexity | * **[medium] Complex boolean expression in conditional rendering**
The code uses repeated calls to `ssh_public_key()` which could lead to performance issues or unexpected behavior if the function has side effects. The same check `!ssh_public_key().is_empty()` appears twice in the same component.
Suggested fix: Store the result of `ssh_public_key()` in a local variable before checking its emptiness to avoid redundant function calls and potential side effects.
*Scanner: code-review/complexity | *
<!-- compliance-fp:700fccab879542d3afc49c9c3861d1bcfb217fd5f0789f4276a7f5e062ee56f5 -->
sharang
commented
[low] Potential duplicate computation of ssh_public_key() The Suggested fix: Store the result of *Scanner: code-review/convention | * **[low] Potential duplicate computation of ssh_public_key()**
The `ssh_public_key()` function is called twice in the same conditional block - once for the display text and once for the CopyButton component. This could be inefficient if the function involves expensive operations or network calls.
Suggested fix: Store the result of `ssh_public_key()` in a local variable before the conditional check to avoid redundant calls.
*Scanner: code-review/convention | *
<!-- compliance-fp:0b9c380c78999b1706d166cde1e45752aa92ca1300b6cc90c7169d1b9f6513e3 -->
sharang
commented
[low] Potential duplicate computation of ssh_public_key() The Suggested fix: Store the result of *Scanner: code-review/convention | * **[low] Potential duplicate computation of ssh_public_key()**
The `ssh_public_key()` function is called twice in the same conditional block - once for the display text and once for the CopyButton component. This could be inefficient if the function involves expensive operations or network calls.
Suggested fix: Store the result of `ssh_public_key()` in a local variable before the conditional check to avoid redundant calls.
*Scanner: code-review/convention | *
<!-- compliance-fp:0b9c380c78999b1706d166cde1e45752aa92ca1300b6cc90c7169d1b9f6513e3 -->
sharang
commented
[medium] Complex boolean expression in conditional rendering The code uses repeated calls to Suggested fix: Store the result of *Scanner: code-review/complexity | * **[medium] Complex boolean expression in conditional rendering**
The code uses repeated calls to `ssh_public_key()` which could lead to performance issues or unexpected behavior if the function has side effects. The same check `!ssh_public_key().is_empty()` appears twice in the same component.
Suggested fix: Store the result of `ssh_public_key()` in a local variable before checking its emptiness to avoid redundant function calls and potential side effects.
*Scanner: code-review/complexity | *
<!-- compliance-fp:700fccab879542d3afc49c9c3861d1bcfb217fd5f0789f4276a7f5e062ee56f5 -->
sharang
commented
[medium] Potential race condition in SSH public key display The SSH public key is checked for emptiness twice - once for displaying 'Loading...' and again for determining whether to show the copy button. If the key becomes empty between these two checks, it could result in inconsistent UI state where the copy button appears but the key is actually empty. Suggested fix: Store the SSH public key result in a local variable before checking its emptiness to ensure consistency across both conditions. *Scanner: code-review/logic | * **[medium] Potential race condition in SSH public key display**
The SSH public key is checked for emptiness twice - once for displaying 'Loading...' and again for determining whether to show the copy button. If the key becomes empty between these two checks, it could result in inconsistent UI state where the copy button appears but the key is actually empty.
Suggested fix: Store the SSH public key result in a local variable before checking its emptiness to ensure consistency across both conditions.
*Scanner: code-review/logic | *
<!-- compliance-fp:8e8d7aa2dde6cf0672a1a99ad7f35ed0ff16798088569a6517f4d1ab05885d45 -->
|
||||
} else {
|
||||
"{ssh_public_key}"
|
||||
}
|
||||
}
|
||||
if !ssh_public_key().is_empty() {
|
||||
crate::components::copy_button::CopyButton { value: ssh_public_key(), small: true }
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -390,28 +397,37 @@ pub fn RepositoriesPage() -> Element {
|
||||
}
|
||||
div { class: "form-group",
|
||||
label { "Webhook URL" }
|
||||
input {
|
||||
r#type: "text",
|
||||
readonly: true,
|
||||
style: "font-family: monospace; font-size: 12px;",
|
||||
value: {
|
||||
#[cfg(feature = "web")]
|
||||
let origin = web_sys::window()
|
||||
.and_then(|w: web_sys::Window| w.location().origin().ok())
|
||||
.unwrap_or_default();
|
||||
#[cfg(not(feature = "web"))]
|
||||
let origin = String::new();
|
||||
format!("{origin}/webhook/{}/{eid}", edit_webhook_tracker())
|
||||
},
|
||||
{
|
||||
#[cfg(feature = "web")]
|
||||
let origin = web_sys::window()
|
||||
.and_then(|w: web_sys::Window| w.location().origin().ok())
|
||||
.unwrap_or_default();
|
||||
#[cfg(not(feature = "web"))]
|
||||
|
sharang
commented
[medium] Inconsistent error handling with unwrap_or_default() in web feature The code uses Suggested fix: Consider using a more explicit error handling approach or logging when the origin cannot be determined, rather than silently falling back to an empty string. *Scanner: code-review/convention | * **[medium] Inconsistent error handling with unwrap_or_default() in web feature**
The code uses `unwrap_or_default()` on `w.location().origin().ok()` which can mask potential errors in URL parsing. This could lead to unexpected behavior when the origin cannot be determined, especially in non-web environments where the window object might not be available.
Suggested fix: Consider using a more explicit error handling approach or logging when the origin cannot be determined, rather than silently falling back to an empty string.
*Scanner: code-review/convention | *
<!-- compliance-fp:f8286e8285fd0f26e4373284aee8d24d930f1dadb527be31347172ea45c0d418 -->
sharang
commented
[medium] Potential Information Disclosure via Copy Button Implementation The implementation introduces copy buttons for SSH public keys, webhook URLs, and secrets. While the copy functionality itself is not inherently vulnerable, the way these values are exposed through the UI could potentially lead to information disclosure if the page is cached or logged. Specifically, the secret value is directly rendered in an input field and made available via the copy button, which could be exploited if the application's logging or caching mechanisms capture this content. Suggested fix: Ensure that sensitive values like secrets are not logged or cached in any form. Consider implementing additional safeguards such as disabling browser autocomplete for sensitive fields and ensuring that the copy functionality does not inadvertently expose these values through other means (e.g., browser history, developer tools). Scanner: code-review/security | CWE: CWE-200 **[medium] Potential Information Disclosure via Copy Button Implementation**
The implementation introduces copy buttons for SSH public keys, webhook URLs, and secrets. While the copy functionality itself is not inherently vulnerable, the way these values are exposed through the UI could potentially lead to information disclosure if the page is cached or logged. Specifically, the secret value is directly rendered in an input field and made available via the copy button, which could be exploited if the application's logging or caching mechanisms capture this content.
Suggested fix: Ensure that sensitive values like secrets are not logged or cached in any form. Consider implementing additional safeguards such as disabling browser autocomplete for sensitive fields and ensuring that the copy functionality does not inadvertently expose these values through other means (e.g., browser history, developer tools).
*Scanner: code-review/security | CWE: CWE-200*
<!-- compliance-fp:115226cd8daa91f657d135e9134c31ab662335f8df71f562499ffa941063f6b0 -->
sharang
commented
[medium] Potential Information Disclosure via Copy Button Implementation The implementation introduces copy buttons for SSH public keys, webhook URLs, and secrets. While the copy functionality itself is not inherently vulnerable, the way these values are exposed through the UI could potentially lead to information disclosure if the page is cached or logged. Specifically, the secret value is directly rendered in an input field and made available via the copy button, which could be exploited if the application's logging or caching mechanisms capture this content. Suggested fix: Ensure that sensitive values like secrets are not logged or cached in any form. Consider implementing additional safeguards such as disabling browser autocomplete for sensitive fields and ensuring that the copy functionality does not inadvertently expose these values through other means (e.g., browser history, developer tools). Scanner: code-review/security | CWE: CWE-200 **[medium] Potential Information Disclosure via Copy Button Implementation**
The implementation introduces copy buttons for SSH public keys, webhook URLs, and secrets. While the copy functionality itself is not inherently vulnerable, the way these values are exposed through the UI could potentially lead to information disclosure if the page is cached or logged. Specifically, the secret value is directly rendered in an input field and made available via the copy button, which could be exploited if the application's logging or caching mechanisms capture this content.
Suggested fix: Ensure that sensitive values like secrets are not logged or cached in any form. Consider implementing additional safeguards such as disabling browser autocomplete for sensitive fields and ensuring that the copy functionality does not inadvertently expose these values through other means (e.g., browser history, developer tools).
*Scanner: code-review/security | CWE: CWE-200*
<!-- compliance-fp:115226cd8daa91f657d135e9134c31ab662335f8df71f562499ffa941063f6b0 -->
sharang
commented
[medium] Inconsistent error handling with unwrap_or_default() in web feature The code uses Suggested fix: Consider using a more explicit error handling approach or logging when the origin cannot be determined, rather than silently falling back to an empty string. *Scanner: code-review/convention | * **[medium] Inconsistent error handling with unwrap_or_default() in web feature**
The code uses `unwrap_or_default()` on `w.location().origin().ok()` which can mask potential errors in URL parsing. This could lead to unexpected behavior when the origin cannot be determined, especially in non-web environments where the window object might not be available.
Suggested fix: Consider using a more explicit error handling approach or logging when the origin cannot be determined, rather than silently falling back to an empty string.
*Scanner: code-review/convention | *
<!-- compliance-fp:f8286e8285fd0f26e4373284aee8d24d930f1dadb527be31347172ea45c0d418 -->
|
||||
let origin = String::new();
|
||||
let webhook_url = format!("{origin}/webhook/{}/{eid}", edit_webhook_tracker());
|
||||
rsx! {
|
||||
div { class: "copyable",
|
||||
input {
|
||||
r#type: "text",
|
||||
readonly: true,
|
||||
style: "font-family: monospace; font-size: 12px; flex: 1;",
|
||||
value: "{webhook_url}",
|
||||
}
|
||||
crate::components::copy_button::CopyButton { value: webhook_url.clone() }
|
||||
}
|
||||
|
sharang
commented
[medium] Inconsistent use of clone() for webhook URL The webhook URL is cloned when passed to the CopyButton component, but the input field uses a string literal reference. This inconsistency could lead to unnecessary allocations or potential issues if the value needs to be used elsewhere after the clone operation. Suggested fix: Use the same approach for both the input field and the copy button - either clone the value consistently or pass a reference to avoid unnecessary allocations. *Scanner: code-review/logic | * **[medium] Inconsistent use of clone() for webhook URL**
The webhook URL is cloned when passed to the CopyButton component, but the input field uses a string literal reference. This inconsistency could lead to unnecessary allocations or potential issues if the value needs to be used elsewhere after the clone operation.
Suggested fix: Use the same approach for both the input field and the copy button - either clone the value consistently or pass a reference to avoid unnecessary allocations.
*Scanner: code-review/logic | *
<!-- compliance-fp:f0b50d83ec153658f4955ca523c0c4be075bec84a0c7080f940868a9cea49a85 -->
sharang
commented
[medium] Inconsistent use of clone() for webhook URL The webhook URL is cloned when passed to the CopyButton component, but the input field uses a string literal reference. This inconsistency could lead to unnecessary allocations or potential issues if the value needs to be used elsewhere after the clone operation. Suggested fix: Use the same approach for both the input field and the copy button - either clone the value consistently or pass a reference to avoid unnecessary allocations. *Scanner: code-review/logic | * **[medium] Inconsistent use of clone() for webhook URL**
The webhook URL is cloned when passed to the CopyButton component, but the input field uses a string literal reference. This inconsistency could lead to unnecessary allocations or potential issues if the value needs to be used elsewhere after the clone operation.
Suggested fix: Use the same approach for both the input field and the copy button - either clone the value consistently or pass a reference to avoid unnecessary allocations.
*Scanner: code-review/logic | *
<!-- compliance-fp:f0b50d83ec153658f4955ca523c0c4be075bec84a0c7080f940868a9cea49a85 -->
|
||||
}
|
||||
}
|
||||
}
|
||||
div { class: "form-group",
|
||||
label { "Webhook Secret" }
|
||||
input {
|
||||
r#type: "text",
|
||||
readonly: true,
|
||||
style: "font-family: monospace; font-size: 12px;",
|
||||
value: "{secret}",
|
||||
div { class: "copyable",
|
||||
input {
|
||||
r#type: "text",
|
||||
readonly: true,
|
||||
style: "font-family: monospace; font-size: 12px; flex: 1;",
|
||||
value: "{secret}",
|
||||
|
sharang
commented
[low] Redundant clone() in CopyButton component The Suggested fix: Pass the secret directly without cloning, or ensure the CopyButton component accepts a reference instead of owned values. *Scanner: code-review/convention | * **[low] Redundant clone() in CopyButton component**
The `secret.clone()` call in the CopyButton component creates an unnecessary clone of the secret string. Since the value is already owned by the component, cloning it just to pass it to another component is wasteful.
Suggested fix: Pass the secret directly without cloning, or ensure the CopyButton component accepts a reference instead of owned values.
*Scanner: code-review/convention | *
<!-- compliance-fp:93f575d103be7ace49c35795a7d43b5619798aa1f37e45cfa2f5c08bca0b7d34 -->
sharang
commented
[low] Redundant clone() in CopyButton component The Suggested fix: Pass the secret directly without cloning, or ensure the CopyButton component accepts a reference instead of owned values. *Scanner: code-review/convention | * **[low] Redundant clone() in CopyButton component**
The `secret.clone()` call in the CopyButton component creates an unnecessary clone of the secret string. Since the value is already owned by the component, cloning it just to pass it to another component is wasteful.
Suggested fix: Pass the secret directly without cloning, or ensure the CopyButton component accepts a reference instead of owned values.
*Scanner: code-review/convention | *
<!-- compliance-fp:93f575d103be7ace49c35795a7d43b5619798aa1f37e45cfa2f5c08bca0b7d34 -->
|
||||
}
|
||||
crate::components::copy_button::CopyButton { value: secret.clone() }
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
[high] Incorrect escaping of single quotes in JavaScript string
The code escapes single quotes with backslashes for JavaScript, but doesn't handle the case where the value itself contains backslashes that should also be escaped. This could lead to malformed JavaScript when the copied text contains backslashes followed by single quotes.
Suggested fix: Use a more robust escaping mechanism that handles all special characters properly, or better yet, use template literals or JSON.stringify to safely pass the value to JavaScript instead of manual string concatenation.
*Scanner: code-review/logic | *