Conversation
WalkthroughAdds a SQLite-backed Pre-merge checks and finishing touches❌ Failed checks (1 warning, 2 inconclusive)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🧪 Early access (Sonnet 4.5): enabledWe are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
README.md(1 hunks)src/database.rs(1 hunks)src/main.rs(1 hunks)src/models.rs(1 hunks)src/script.js(1 hunks)src/style.css(1 hunks)src/web_interface.html(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/script.js (1)
src/main.rs (1)
main(28-67)
src/main.rs (1)
src/database.rs (4)
new(12-72)create_token(74-85)list_tokens(87-102)delete_token(113-127)
src/database.rs (1)
src/main.rs (3)
create_token(155-170)list_tokens(172-180)delete_token(182-193)
🪛 markdownlint-cli2 (0.18.1)
README.md
42-42: Bare URL used
(MD034, no-bare-urls)
103-103: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (15)
Cargo.toml (1)
5-6: Metadata looks good; consider adding repository links for discoverability.Add
repository,homepage, andreadmefields so crates.io displays links and docs.[package] name = "webhook-service" version = "1.0.0" edition = "2024" description = "Webhook test service" authors = ["Oleksii Rodionov"] +repository = "https://github.com/SpeedSX/webhook-service" +homepage = "https://github.com/SpeedSX/webhook-service" +readme = "README.md" license = "MIT"README.md (1)
91-97: Document BASE_URL to avoid localhost-only links.The server supports a
BASE_URLenv; document it here for deployments behind proxies.The service runs on `0.0.0.0:3000` by default. To change the port or host, modify the `main.rs` file: ```rust let listener = tokio::net::TcpListener::bind("0.0.0.0:3000").await?;
+You can also set a public base URL (for generated webhook links) via:
+
+bash +export BASE_URL="https://your.domain.example" +</blockquote></details> <details> <summary>src/main.rs (4)</summary><blockquote> `22-52`: **Include forwarded headers when deriving the public base URL.** Behind proxies, `Host` may be different and scheme determination should prefer `x-forwarded-proto`/`x-forwarded-host`/`forwarded`. ```diff fn generate_webhook_url( base_url: &Option<String>, _uri: &Uri, headers: &HeaderMap, token: &str, ) -> String { @@ - if let Some(host) = headers.get("host").and_then(|h| h.to_str().ok()) { - // Determine scheme (default to http for localhost, https otherwise) - let scheme = if host.starts_with("localhost") || host.starts_with("127.0.0.1") { - "http" - } else { - "https" - }; + // Prefer forwarded headers set by proxies/CDNs + let fwd_proto = headers.get("x-forwarded-proto").and_then(|v| v.to_str().ok()); + let fwd_host = headers.get("x-forwarded-host").and_then(|v| v.to_str().ok()); + let (scheme, host) = match (fwd_proto, fwd_host) { + (Some(proto), Some(h)) => (proto, h), + _ => { + let host = headers.get("host").and_then(|h| h.to_str().ok()); + let host = host.unwrap_or("localhost:3000"); + let scheme = if host.starts_with("localhost") || host.starts_with("127.0.0.1") { + "http" + } else { + "https" + }; + (scheme, host) + } + }; - - let base = format!("{}://{}", scheme, host); + let base = format!("{}://{}", scheme, host); let normalized_base = base.trim_end_matches('/'); return format!("{}/{}", normalized_base, token);
151-158: Guard against oversized request bodies to avoid memory blowups.
Bytesbuffers the entire body; add a simple size cap before parsing.- // Parse body - let body_str = String::from_utf8(body.to_vec()).unwrap_or_default(); + // Parse body with a basic size cap (1 MiB) + if body.len() > 1_048_576 { + warn!("Request body too large: {} bytes", body.len()); + return Err(StatusCode::PAYLOAD_TOO_LARGE); + } + let body_str = String::from_utf8(body.to_vec()).unwrap_or_default();
101-104: Make startup URLs consistent with configured/public base.Log
BASE_URLwhen set (or the listener addr) to avoid implying localhost-only access.- info!("Webhook service running on http://0.0.0.0:3000"); - info!("Web interface available at http://localhost:3000"); + info!("Webhook service listening on 0.0.0.0:3000"); + if let Some(ref url) = app_state.base_url { + info!("Public base URL: {}", url); + } else { + info!("No BASE_URL set; links will derive from request headers"); + }
244-255: Cap the maximum logs returned to protect the DB and UI.Validate
countwith a reasonable upper bound (e.g., 1000) to prevent heavy queries/responses.async fn get_webhook_logs( State(state): State<AppState>, Path((token, count)): Path<(String, u32)>, ) -> Result<Json<Vec<WebhookRequest>>, StatusCode> { + let count = count.min(1000); match state.db.get_webhook_requests(&token, count).await {src/script.js (4)
113-119: Preserve selected token when refreshing the dropdown.Improves UX during auto-refresh.
updateTokenDropdowns() { const logsDropdown = document.getElementById('selected-token-logs'); - logsDropdown.innerHTML = '<option value="">Select a token to view logs</option>' + + const current = logsDropdown.value; + logsDropdown.innerHTML = '<option value="">Select a token to view logs</option>' + this.tokens.map(token => `<option value="${token.token}">${token.token}</option>` ).join(''); + if (current && this.tokens.some(t => t.token === current)) { + logsDropdown.value = current; + } }
186-201: Sanitize and bound the logs count parameter.Ensure it’s a positive integer and cap it (e.g., 1000).
async loadLogs() { const token = document.getElementById('selected-token-logs').value; - const count = document.getElementById('log-count').value; + let count = parseInt(document.getElementById('log-count').value, 10); + if (!Number.isFinite(count) || count <= 0) count = 50; + count = Math.min(count, 1000);
130-138: Clipboard API may fail on insecure origins; provide a graceful fallback.Fallback to showing the command when clipboard isn’t available.
- navigator.clipboard.writeText(command).then(() => { + (navigator.clipboard?.writeText ? navigator.clipboard.writeText(command) : Promise.reject()) + .then(() => { this.showMessage('Command copied to clipboard!', 'success'); }).catch(err => { - console.error('Failed to copy command:', err); + if (err) console.error('Failed to copy command:', err); this.showMessage('Failed to copy command. Please copy manually: ' + command, 'error'); });
157-166: Don’t force JSON Content-Type for non-JSON bodies.Let custom headers override (already happens), and set Content-Type only when body is present.
- const requestOptions = { + const requestOptions = { method: method, headers: { - 'Content-Type': 'application/json', ...customHeaders } }; if (body.trim() && (method === 'POST' || method === 'PUT' || method === 'PATCH')) { - requestOptions.body = body; + requestOptions.body = body; + if (!('Content-Type' in requestOptions.headers)) { + requestOptions.headers['Content-Type'] = 'application/json'; + } }src/database.rs (5)
49-62: Consider ON DELETE CASCADE on FK to simplify deletes and enforce integrity.This prevents orphans automatically and makes delete_token simpler/safer even with races.
Note: If the table already exists, this requires a migration (create new table, copy, swap).
67-75: Use a composite index aligned with the hot query (token_id, date DESC).The query filters by token_id, orders by date, and limits. A composite index serves it; the standalone date index doesn’t help here.
-Also applies to: 72-75
136-164: Add error context for easier diagnosis; keep FK violations actionable.Attach context to serialization and INSERT to surface where failures occur (e.g., FK errors on unknown token).
@@Note:
use anyhow::Context;added in a separate diff.Also applies to: 150-162
166-181: Propagate context on SELECT and JSON parse to speed up debugging.This pinpoints whether failures are in query or JSON decoding.
@@Also applies to: 183-189
57-59: Optional: store raw body as BLOB to support non-UTF8 payloads.
body TEXTassumes UTF‑8; binary or mixed-encoding webhooks will fail or be lossy. Considerbody BLOBand model asOption<Vec<u8>>. Migration required.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
Cargo.toml(1 hunks)README.md(1 hunks)src/database.rs(1 hunks)src/main.rs(1 hunks)src/script.js(1 hunks)src/style.css(1 hunks)src/web_interface.html(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/web_interface.html
- src/style.css
🧰 Additional context used
🧬 Code graph analysis (3)
src/main.rs (1)
src/database.rs (4)
new(13-77)create_token(79-90)list_tokens(92-107)delete_token(118-134)
src/script.js (1)
src/main.rs (1)
main(61-108)
src/database.rs (1)
src/main.rs (3)
create_token(196-219)list_tokens(221-229)delete_token(231-242)
🪛 markdownlint-cli2 (0.18.1)
README.md
41-41: Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (1)
src/main.rs (1)
261-279: static_files returns the wrong Response type; this won’t compile.
axum::response::Responseis a non-generic alias;Response<String>is invalid. Also avoidunwrap()and set headers via constants.-use axum::{ - extract::{Path, State}, - http::{HeaderMap, Method, StatusCode, Uri}, - response::{Html, Json, Response}, +use axum::{ + extract::{Path, State}, + http::{header, HeaderMap, Method, StatusCode, Uri}, + response::{Html, Json, Response}, }; @@ -async fn static_files(Path(path): Path<String>) -> Result<Response<String>, StatusCode> { +use axum::body::Body; + +async fn static_files(Path(path): Path<String>) -> Result<Response, StatusCode> { match path.as_str() { - "style.css" => { - let content = include_str!("style.css").to_string(); - Ok(Response::builder() - .header("content-type", "text/css; charset=utf-8") - .body(content) - .unwrap()) - }, - "script.js" => { - let content = include_str!("script.js").to_string(); - Ok(Response::builder() - .header("content-type", "application/javascript; charset=utf-8") - .body(content) - .unwrap()) - }, + "style.css" => Response::builder() + .header(header::CONTENT_TYPE, "text/css; charset=utf-8") + .body(Body::from(include_str!("style.css"))) + .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR), + "script.js" => Response::builder() + .header(header::CONTENT_TYPE, "application/javascript; charset=utf-8") + .body(Body::from(include_str!("script.js"))) + .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR), _ => Err(StatusCode::NOT_FOUND), } }Likely an incorrect or invalid review comment.
| renderTokens() { | ||
| const container = document.getElementById('token-list'); | ||
|
|
||
| if (this.tokens.length === 0) { | ||
| container.innerHTML = '<div class="loading">No tokens found. Create one to get started!</div>'; | ||
| return; | ||
| } | ||
|
|
||
| container.innerHTML = this.tokens.map(token => ` | ||
| <div class="token-item"> | ||
| <div class="token-info"> | ||
| <div class="token-value">${token.token}</div> | ||
| <div class="token-url">${token.webhook_url}</div> | ||
| <div class="token-created">Created: ${new Date(token.created_at).toLocaleString()}</div> | ||
| </div> | ||
| <div class="token-actions"> | ||
| <button class="btn btn-secondary" onclick="webhookService.selectToken('${token.token}')">Select</button> | ||
| <button class="btn btn-info" onclick="webhookService.copyCommand('${token.token}')" title="Copy monitoring command">📋 Command</button> | ||
| <button class="btn btn-danger" onclick="webhookService.deleteToken('${token.token}')">Delete</button> | ||
| </div> | ||
| </div> | ||
| `).join(''); | ||
| } |
There was a problem hiding this comment.
Avoid innerHTML injection in renderTokens.
Escape dynamic values to prevent XSS if a malicious URL sneaks in.
- container.innerHTML = this.tokens.map(token => `
+ container.innerHTML = this.tokens.map(token => {
+ const t = this.escapeHtml(token.token);
+ const url = this.escapeHtml(token.webhook_url);
+ const created = new Date(token.created_at).toLocaleString();
+ return `
<div class="token-item">
<div class="token-info">
- <div class="token-value">${token.token}</div>
- <div class="token-url">${token.webhook_url}</div>
- <div class="token-created">Created: ${new Date(token.created_at).toLocaleString()}</div>
+ <div class="token-value">${t}</div>
+ <div class="token-url">${url}</div>
+ <div class="token-created">Created: ${created}</div>
</div>
<div class="token-actions">
- <button class="btn btn-secondary" onclick="webhookService.selectToken('${token.token}')">Select</button>
- <button class="btn btn-info" onclick="webhookService.copyCommand('${token.token}')" title="Copy monitoring command">📋 Command</button>
- <button class="btn btn-danger" onclick="webhookService.deleteToken('${token.token}')">Delete</button>
+ <button class="btn btn-secondary" onclick="webhookService.selectToken('${t}')">Select</button>
+ <button class="btn btn-info" onclick="webhookService.copyCommand('${t}')" title="Copy monitoring command">📋 Command</button>
+ <button class="btn btn-danger" onclick="webhookService.deleteToken('${t}')">Delete</button>
</div>
</div>
- `).join('');
+ `}).join('');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| renderTokens() { | |
| const container = document.getElementById('token-list'); | |
| if (this.tokens.length === 0) { | |
| container.innerHTML = '<div class="loading">No tokens found. Create one to get started!</div>'; | |
| return; | |
| } | |
| container.innerHTML = this.tokens.map(token => ` | |
| <div class="token-item"> | |
| <div class="token-info"> | |
| <div class="token-value">${token.token}</div> | |
| <div class="token-url">${token.webhook_url}</div> | |
| <div class="token-created">Created: ${new Date(token.created_at).toLocaleString()}</div> | |
| </div> | |
| <div class="token-actions"> | |
| <button class="btn btn-secondary" onclick="webhookService.selectToken('${token.token}')">Select</button> | |
| <button class="btn btn-info" onclick="webhookService.copyCommand('${token.token}')" title="Copy monitoring command">📋 Command</button> | |
| <button class="btn btn-danger" onclick="webhookService.deleteToken('${token.token}')">Delete</button> | |
| </div> | |
| </div> | |
| `).join(''); | |
| } | |
| renderTokens() { | |
| const container = document.getElementById('token-list'); | |
| if (this.tokens.length === 0) { | |
| container.innerHTML = '<div class="loading">No tokens found. Create one to get started!</div>'; | |
| return; | |
| } | |
| container.innerHTML = this.tokens.map(token => { | |
| const t = this.escapeHtml(token.token); | |
| const url = this.escapeHtml(token.webhook_url); | |
| const created = new Date(token.created_at).toLocaleString(); | |
| return ` | |
| <div class="token-item"> | |
| <div class="token-info"> | |
| <div class="token-value">${t}</div> | |
| <div class="token-url">${url}</div> | |
| <div class="token-created">Created: ${created}</div> | |
| </div> | |
| <div class="token-actions"> | |
| <button class="btn btn-secondary" onclick="webhookService.selectToken('${t}')">Select</button> | |
| <button class="btn btn-info" onclick="webhookService.copyCommand('${t}')" title="Copy monitoring command">📋 Command</button> | |
| <button class="btn btn-danger" onclick="webhookService.deleteToken('${t}')">Delete</button> | |
| </div> | |
| </div> | |
| `; | |
| }).join(''); | |
| } |
🤖 Prompt for AI Agents
In src/script.js around lines 89 to 111, renderTokens currently builds HTML via
innerHTML using unescaped token properties which risks XSS; instead, build the
token list using DOM APIs: clear the container, for each token create elements
(divs, spans, buttons) with document.createElement, set text values via
textContent (or setAttribute for non-text attributes) and append them, and
attach event handlers using addEventListener rather than inline onclick; ensure
token, webhook_url and created_at are never inserted via innerHTML or string
interpolation but assigned via textContent or safe attribute setters.
| container.innerHTML = logs.map(log => ` | ||
| <div class="log-item"> | ||
| <div class="log-header"> | ||
| <span class="log-method method-${log.MessageObject.Method.toLowerCase()}">${log.MessageObject.Method}</span> | ||
| <span class="log-id">ID: ${log.Id}</span> | ||
| <span class="log-timestamp">${new Date(log.Date).toLocaleString()}</span> | ||
| </div> | ||
| <div class="log-details"> | ||
| <div class="log-url">${log.MessageObject.Value}</div> | ||
| ${log.MessageObject.Body ? ` | ||
| <div class="log-body">${this.formatJson(log.MessageObject.Body)}</div> | ||
| ` : ''} | ||
| ${Object.keys(log.MessageObject.Headers).length > 0 ? ` | ||
| <div class="log-headers"> | ||
| <h4>Headers:</h4> | ||
| <pre>${this.formatHeaders(log.MessageObject.Headers)}</pre> | ||
| </div> | ||
| ` : ''} | ||
| </div> | ||
| </div> | ||
| `).join(''); | ||
| } |
There was a problem hiding this comment.
renderLogs uses wrong JSON property names; will render undefined and break CSS classes.
Backend returns id, date, and message_object.{method,value,headers,body} (snake/camel, not PascalCase). Fix property access and escape content to prevent XSS.
- container.innerHTML = logs.map(log => `
- <div class="log-item">
- <div class="log-header">
- <span class="log-method method-${log.MessageObject.Method.toLowerCase()}">${log.MessageObject.Method}</span>
- <span class="log-id">ID: ${log.Id}</span>
- <span class="log-timestamp">${new Date(log.Date).toLocaleString()}</span>
- </div>
- <div class="log-details">
- <div class="log-url">${log.MessageObject.Value}</div>
- ${log.MessageObject.Body ? `
- <div class="log-body">${this.formatJson(log.MessageObject.Body)}</div>
- ` : ''}
- ${Object.keys(log.MessageObject.Headers).length > 0 ? `
- <div class="log-headers">
- <h4>Headers:</h4>
- <pre>${this.formatHeaders(log.MessageObject.Headers)}</pre>
- </div>
- ` : ''}
- </div>
- </div>
- `).join('');
+ container.innerHTML = logs.map(log => {
+ const mobj = log.message_object || {};
+ const method = (mobj.method || '').toLowerCase();
+ const id = this.escapeHtml(log.id || '');
+ const ts = this.escapeHtml(log.date || '');
+ const url = this.escapeHtml(mobj.value || '');
+ const body = mobj.body ? this.escapeHtml(this.formatJson(mobj.body)) : null;
+ const hasHeaders = mobj.headers && Object.keys(mobj.headers).length > 0;
+ return `
+ <div class="log-item">
+ <div class="log-header">
+ <span class="log-method method-${method}">${method.toUpperCase()}</span>
+ <span class="log-id">ID: ${id}</span>
+ <span class="log-timestamp">${new Date(ts).toLocaleString()}</span>
+ </div>
+ <div class="log-details">
+ <div class="log-url">${url}</div>
+ ${body ? `<div class="log-body"><pre>${body}</pre></div>` : ''}
+ ${hasHeaders ? `
+ <div class="log-headers">
+ <h4>Headers:</h4>
+ <pre>${this.escapeHtml(this.formatHeaders(mobj.headers))}</pre>
+ </div>` : ''}
+ </div>
+ </div>`;
+ }).join('');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| container.innerHTML = logs.map(log => ` | |
| <div class="log-item"> | |
| <div class="log-header"> | |
| <span class="log-method method-${log.MessageObject.Method.toLowerCase()}">${log.MessageObject.Method}</span> | |
| <span class="log-id">ID: ${log.Id}</span> | |
| <span class="log-timestamp">${new Date(log.Date).toLocaleString()}</span> | |
| </div> | |
| <div class="log-details"> | |
| <div class="log-url">${log.MessageObject.Value}</div> | |
| ${log.MessageObject.Body ? ` | |
| <div class="log-body">${this.formatJson(log.MessageObject.Body)}</div> | |
| ` : ''} | |
| ${Object.keys(log.MessageObject.Headers).length > 0 ? ` | |
| <div class="log-headers"> | |
| <h4>Headers:</h4> | |
| <pre>${this.formatHeaders(log.MessageObject.Headers)}</pre> | |
| </div> | |
| ` : ''} | |
| </div> | |
| </div> | |
| `).join(''); | |
| } | |
| container.innerHTML = logs.map(log => { | |
| const mobj = log.message_object || {}; | |
| const method = (mobj.method || '').toLowerCase(); | |
| const id = this.escapeHtml(log.id || ''); | |
| const ts = this.escapeHtml(log.date || ''); | |
| const url = this.escapeHtml(mobj.value || ''); | |
| const body = mobj.body ? this.escapeHtml(this.formatJson(mobj.body)) : null; | |
| const hasHeaders = mobj.headers && Object.keys(mobj.headers).length > 0; | |
| return ` | |
| <div class="log-item"> | |
| <div class="log-header"> | |
| <span class="log-method method-${method}">${method.toUpperCase()}</span> | |
| <span class="log-id">ID: ${id}</span> | |
| <span class="log-timestamp">${new Date(ts).toLocaleString()}</span> | |
| </div> | |
| <div class="log-details"> | |
| <div class="log-url">${url}</div> | |
| ${body ? `<div class="log-body"><pre>${body}</pre></div>` : ''} | |
| ${hasHeaders ? ` | |
| <div class="log-headers"> | |
| <h4>Headers:</h4> | |
| <pre>${this.escapeHtml(this.formatHeaders(mobj.headers))}</pre> | |
| </div>` : ''} | |
| </div> | |
| </div>`; | |
| }).join(''); |
🤖 Prompt for AI Agents
In src/script.js around lines 218 to 239, the renderLogs code is reading
PascalCase properties (Id, Date, MessageObject.Method, etc.) but the backend
returns id, date, and message_object.{method,value,headers,body}, which causes
undefined values and broken CSS classes; also raw insertion of values is an XSS
risk. Update all property accesses to use the backend names (log.id, log.date,
log.message_object.method/value/headers/body), null-check message_object and
headers before using them, call an existing HTML-escape helper (or implement a
small escapeHtml function) for any interpolated content (id, method, value,
formatted body/headers, timestamps) and sanitize method before using it in class
names (e.g., lowercased and non-word chars removed). Ensure dates are parsed
from log.date and only call toLocaleString when date exists.
There was a problem hiding this comment.
@coderabbitai check the backend code, the properties are PascalCase
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the models.rs file to see the serde serialization attributes
echo "=== Checking models.rs for serde attributes ==="
cat src/models.rs
echo -e "\n=== Checking database.rs for how webhook requests are returned ==="
# Look for the get_webhook_requests method implementation
rg -n -A 20 "get_webhook_requests" src/database.rs
Length of output: 2178
@SpeedSX You're absolutely correct! I apologize for the error. Looking at the backend code in src/models.rs, the structs indeed use serde rename attributes to serialize to PascalCase:
WebhookRequest:id→"Id",date→"Date",message_object→"MessageObject"MessageObject:method→"Method",value→"Value",headers→"Headers",body→"Body"
The frontend JavaScript code is correctly accessing log.Id, log.Date, log.MessageObject.Method, etc. I was mistaken about the property names.
However, the XSS concern about directly inserting unsanitized content into innerHTML still stands. Consider adding HTML escaping for user-controlled values like URLs, headers, and request bodies to prevent potential script injection, but the property access is correct as written.
✏️ Learnings added
Learnt from: SpeedSX
PR: SpeedSX/webhook-service#1
File: src/script.js:218-239
Timestamp: 2025-09-29T10:51:02.158Z
Learning: In the webhook service project, the backend Rust structs use serde rename attributes to serialize JSON responses to PascalCase format. WebhookRequest serializes to Id/Date/MessageObject/TokenId/Message, and MessageObject serializes to Method/Value/Headers/QueryParameters/Body/BodyObject. The frontend JavaScript should access these properties using PascalCase names like log.Id, log.MessageObject.Method, etc.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
src/main.rs (2)
144-152: Prefer Axum’s Query extractor for robust parsing (multi-value, decoding).Manual split loses repeated keys and percent-decoding. The Query extractor (e.g., Query<HashMap<String, Vec>>) is safer and clearer.
-// Parse query parameters -let query_params: Vec<String> = uri - .query() - .unwrap_or("") - .split('&') - .filter(|s| !s.is_empty()) - .map(|s| s.to_string()) - .collect(); +// Parse query parameters +let query_params: Vec<String> = uri + .query() + .map(|q| form_urlencoded::parse(q.as_bytes()) + .map(|(k, v)| format!("{}={}", k, v)) + .collect()) + .unwrap_or_default();
108-113: Make bind address configurable (PORT/BIND_ADDR).Hard‑coding 0.0.0.0:3000 complicates container/cloud deploys.
- let listener = tokio::net::TcpListener::bind("0.0.0.0:3000").await?; + let bind = std::env::var("BIND_ADDR") + .or_else(|_| std::env::var("PORT").map(|p| format!("0.0.0.0:{p}"))) + .unwrap_or_else(|_| "0.0.0.0:3000".to_string()); + let listener = tokio::net::TcpListener::bind(&bind).await?; + info!("Listening on {}", bind);src/script.js (1)
279-299: Scope message removal to a dedicated container to avoid collateral DOM changes.Querying by generic classes (.error, .success) may remove unrelated elements. Wrap messages in a dedicated container or add a distinctive class.
- const existingMessages = document.querySelectorAll('.error, .success'); + const existingMessages = document.querySelectorAll('.toast-message.error, .toast-message.success'); existingMessages.forEach(msg => msg.remove()); // ... - messageDiv.className = type; + messageDiv.className = `toast-message ${type}`;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
Cargo.toml(1 hunks)README.md(1 hunks)src/main.rs(1 hunks)src/script.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- README.md
- Cargo.toml
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-29T10:51:02.158Z
Learnt from: SpeedSX
PR: SpeedSX/webhook-service#1
File: src/script.js:218-239
Timestamp: 2025-09-29T10:51:02.158Z
Learning: In the webhook service project, the backend Rust structs use serde rename attributes to serialize JSON responses to PascalCase format. WebhookRequest serializes to Id/Date/MessageObject/TokenId/Message, and MessageObject serializes to Method/Value/Headers/QueryParameters/Body/BodyObject. The frontend JavaScript should access these properties using PascalCase names like log.Id, log.MessageObject.Method, etc.
Applied to files:
src/script.js
🧬 Code graph analysis (2)
src/script.js (1)
src/main.rs (1)
main(65-118)
src/main.rs (1)
src/database.rs (4)
new(13-77)create_token(79-90)list_tokens(92-107)delete_token(118-134)
🔇 Additional comments (4)
src/main.rs (2)
86-99: Routes use correct Axum 0.8 path syntax; ordering looks good.Static and API routes precede the token-catchalls, avoiding accidental shadowing. No change requested. Based on learnings.
22-56: Webhook URL derivation: tighten trust of headers and handle multi-value X-Forwarded headers.Current implementation uses unvalidated X-Forwarded headers, which attackers can spoof. Parse only the first comma-separated value and enforce a scheme whitelist, for example:
- let fwd_proto = headers.get("x-forwarded-proto").and_then(|v| v.to_str().ok()); - let fwd_host = headers.get("x-forwarded-host").and_then(|v| v.to_str().ok()); + let first = |name: &str| { + headers.get(name) + .and_then(|v| v.to_str().ok()) + .map(|s| s.split(',').next().unwrap_or("").trim()) + }; + let fwd_proto = first("x-forwarded-proto"); + let fwd_host = first("x-forwarded-host"); let (scheme, host) = match (fwd_proto, fwd_host) { - (Some(proto), Some(h)) => (proto, h), + (Some(proto), Some(h)) if matches!(proto, "http" | "https") && !h.is_empty() => (proto, h), _ => { /* fallback logic */ } };Please confirm whether this service is strictly behind trusted proxies; if so, enforce a host allowlist or require BASE_URL in production.
src/script.js (2)
219-248: Logs rendering looks correct and escaped.Using PascalCase properties and escape helpers aligns with backend serde renames; method class name is lowercased and escaped. No change requested. Based on learnings.
97-111: Escape dynamic values and avoid inline event handlers in renderTokens.Currently interpolates raw token data into innerHTML and uses inline onclick handlers—XSS/CSP risks. At minimum, escape values; ideally, build DOM nodes and addEventListener.
- container.innerHTML = this.tokens.map(token => ` + container.innerHTML = this.tokens.map(token => { + const t = this.escapeHtml(token.token); + const url = this.escapeHtml(token.webhook_url); + const created = new Date(token.created_at).toLocaleString(); + return ` <div class="token-item"> <div class="token-info"> - <div class="token-value">${token.token}</div> - <div class="token-url">${token.webhook_url}</div> - <div class="token-created">Created: ${new Date(token.created_at).toLocaleString()}</div> + <div class="token-value">${t}</div> + <div class="token-url">${url}</div> + <div class="token-created">Created: ${this.escapeHtml(created)}</div> </div> <div class="token-actions"> - <button class="btn btn-secondary" onclick="webhookService.selectToken('${token.token}')">Select</button> - <button class="btn btn-info" onclick="webhookService.copyCommand('${token.token}')" title="Copy monitoring command">📋 Command</button> - <button class="btn btn-danger" onclick="webhookService.deleteToken('${token.token}')">Delete</button> + <button class="btn btn-secondary" data-action="select" data-token="${t}">Select</button> + <button class="btn btn-info" data-action="copy" data-token="${t}" title="Copy monitoring command">📋 Command</button> + <button class="btn btn-danger" data-action="delete" data-token="${t}">Delete</button> </div> </div> - `).join(''); + `}).join(''); + + // Delegate events once + container.addEventListener('click', (e) => { + const btn = e.target.closest('button[data-action]'); + if (!btn) return; + const t = btn.getAttribute('data-token'); + switch (btn.getAttribute('data-action')) { + case 'select': this.selectToken(t); break; + case 'copy': this.copyCommand(t); break; + case 'delete': this.deleteToken(t); break; + } + }, { once: true });
| logsDropdown.innerHTML = '<option value="">Select a token to view logs</option>' + | ||
| this.tokens.map(token => | ||
| `<option value="${token.token}">${token.token}</option>` | ||
| ).join(''); | ||
| if (current && this.tokens.some(t => t.token === current)) { | ||
| logsDropdown.value = current; | ||
| } |
There was a problem hiding this comment.
Sanitize dropdown options; avoid innerHTML assembly.
Using innerHTML with raw token values risks injection in attributes. Build options with createElement/textContent.
- logsDropdown.innerHTML = '<option value="">Select a token to view logs</option>' +
- this.tokens.map(token =>
- `<option value="${token.token}">${token.token}</option>`
- ).join('');
+ logsDropdown.innerHTML = '';
+ const placeholder = document.createElement('option');
+ placeholder.value = '';
+ placeholder.textContent = 'Select a token to view logs';
+ logsDropdown.appendChild(placeholder);
+ this.tokens.forEach(token => {
+ const opt = document.createElement('option');
+ opt.value = token.token;
+ opt.textContent = token.token;
+ logsDropdown.appendChild(opt);
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| logsDropdown.innerHTML = '<option value="">Select a token to view logs</option>' + | |
| this.tokens.map(token => | |
| `<option value="${token.token}">${token.token}</option>` | |
| ).join(''); | |
| if (current && this.tokens.some(t => t.token === current)) { | |
| logsDropdown.value = current; | |
| } | |
| // Clear existing options and rebuild safely via DOM APIs | |
| logsDropdown.innerHTML = ''; | |
| const placeholder = document.createElement('option'); | |
| placeholder.value = ''; | |
| placeholder.textContent = 'Select a token to view logs'; | |
| logsDropdown.appendChild(placeholder); | |
| this.tokens.forEach(token => { | |
| const opt = document.createElement('option'); | |
| opt.value = token.token; | |
| opt.textContent = token.token; | |
| logsDropdown.appendChild(opt); | |
| }); | |
| if (current && this.tokens.some(t => t.token === current)) { | |
| logsDropdown.value = current; | |
| } |
🤖 Prompt for AI Agents
In src/script.js around lines 116 to 122, avoid assembling option markup via
innerHTML because token values can inject into attributes; instead clear the
dropdown, create a default option via document.createElement('option') and set
its value/textContent, then for each token create an option element, set
option.value = token.token and option.textContent = token.token (do not use
innerHTML), append each option to the select, and after appending set
logsDropdown.value = current only if this.tokens.some(t => t.token === current).
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
Cargo.toml (1)
21-21: Prefer compile‑time checked queries (query!/query_as!) and consider SQLX offline in CI.You upgraded to sqlx 0.8; great. Where possible, switch from dynamic
query()to typed macros for schema-checked queries, and enable offline metadata in CI to avoid needing a live DB.If desired, I can sketch a small migration of
token_existstoquery_scalar!and add acargo sqlx prepare --checkCI step.src/main.rs (2)
22-33: Remove unusedUriparameter fromgenerate_webhook_urland call site.It’s unused; drop it for clarity. If you plan to use it, wire it up; otherwise simplify.
Apply this diff:
-fn generate_webhook_url( - base_url: &Option<String>, - _uri: &Uri, - headers: &HeaderMap, - token: &str, -) -> String { +fn generate_webhook_url( + base_url: &Option<String>, + headers: &HeaderMap, + token: &str, +) -> String { @@ - let webhook_url = generate_webhook_url(&state.base_url, &uri, &headers, &token.to_string()); + let webhook_url = generate_webhook_url(&state.base_url, &headers, &token.to_string());Also applies to: 226-235
292-307: Optional: add cache headers for static assets to reduce repeated transfers.Short TTLs are fine for dev; adjust via env if needed.
Apply this diff:
- Response::builder() + Response::builder() .header("content-type", "text/css; charset=utf-8") + .header("cache-control", "public, max-age=300") .body(content) @@ - Response::builder() + Response::builder() .header("content-type", "application/javascript; charset=utf-8") + .header("cache-control", "public, max-age=300") .body(content)src/database.rs (2)
49-61: ConsiderON DELETE CASCADEon FK to simplify cleanup and harden integrity.You already wrap deletes in a transaction—good. Adding
ON DELETE CASCADEensures referential integrity even if a delete happens without the child purge and simplifies future code.Note: changing this requires a migration (SQLite needs table recreation). I can draft a safe migration plan if you want.
92-116: Adopt typed sqlx macros where feasible for compile‑time checking.
query()/manual mapping works but misses type/schema checks. For simple cases (e.g.,token_exists) switch toquery_scalar!, and for row mapping preferquery_as!into structs.Example for
token_exists:let count: i64 = sqlx::query_scalar!("SELECT COUNT(*) as \"count!: i64\" FROM tokens WHERE token = ?", token) .fetch_one(&self.pool) .await?;If you want, I can convert a couple of methods and add
sqlx-data.jsonprep to CI.Also applies to: 166-209
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.gitignore(1 hunks)Cargo.toml(2 hunks)src/database.rs(1 hunks)src/main.rs(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .gitignore
🧰 Additional context used
🧬 Code graph analysis (2)
src/main.rs (1)
src/database.rs (4)
new(13-77)create_token(79-90)list_tokens(92-107)delete_token(118-134)
src/database.rs (1)
src/main.rs (3)
create_token(226-249)list_tokens(251-259)delete_token(261-272)
🔇 Additional comments (1)
src/main.rs (1)
171-175: Preserve non‑UTF8 header values;to_str()can lose data.Convert from raw bytes losslessly.
Apply this diff:
- let value_str = value.to_str().unwrap_or("").to_string(); + let value_str = String::from_utf8_lossy(value.as_bytes()).to_string();
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/database.rs (1)
174-221: Consider usingquery_as!for compile-time checking and simpler mapping.The manual row mapping is correct but verbose. Since you're using sqlx 0.8 with compile-time checking support, consider using
query_as!with a struct that matches the query columns. This would provide compile-time verification and reduce boilerplate.Example refactor (requires database schema available at compile time):
// Define a row struct #[derive(sqlx::FromRow)] struct WebhookRequestRow { id: String, date: String, token_id: String, method: String, value: String, headers: String, query_parameters: String, body: Option<String>, body_object: Option<String>, message: Option<String>, } pub async fn get_webhook_requests( &self, token: &str, count: u32, ) -> Result<Vec<WebhookRequest>> { let rows = sqlx::query_as::<_, WebhookRequestRow>( r#" SELECT id, date, token_id, method, value, headers, query_parameters, body, body_object, message FROM webhook_requests WHERE token_id = ? ORDER BY date DESC LIMIT ? "# ) .bind(token) .bind(count as i64) .fetch_all(&self.pool) .await?; let mut requests = Vec::new(); for row in rows { let headers: HashMap<String, Vec<String>> = serde_json::from_str(&row.headers)?; let query_parameters: Vec<String> = serde_json::from_str(&row.query_parameters)?; let body_object: Option<serde_json::Value> = row .body_object .map(|s| serde_json::from_str(&s)) .transpose()?; requests.push(WebhookRequest { id: row.id, date: row.date, token_id: row.token_id, message_object: MessageObject { method: row.method, value: row.value, headers, query_parameters, body: row.body, body_object, }, message: row.message, }); } Ok(requests) }This provides type safety and compile-time column name checking while keeping the JSON deserialization logic.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Cargo.toml(2 hunks)src/database.rs(1 hunks)src/main.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main.rs
🧰 Additional context used
🧬 Code graph analysis (1)
src/database.rs (1)
src/main.rs (3)
create_token(250-272)list_tokens(274-282)delete_token(284-295)
🔇 Additional comments (13)
Cargo.toml (5)
5-10: LGTM! Metadata updated appropriately.The package metadata (description, authors, repository, homepage, readme) has been updated with appropriate values for this project.
21-21: LGTM! SQLx 0.8 upgrade is appropriate.The upgrade from sqlx 0.7 to 0.8 is correct. Based on learnings, sqlx 0.8.6 is the latest stable version with incremental improvements and no breaking changes within the 0.8.x series.
27-32: LGTM! Release profile optimizations are appropriate.The release profile settings (lto, codegen-units, panic, strip) are standard optimizations for reducing binary size and are appropriate for production builds.
25-25: Dependency is used in the codebase.The
urlcrate is imported and used insrc/main.rs:14asurl::form_urlencoded, so this dependency is necessary.
4-4: Rust 2024 edition is valid and stable.Rust 2024 reached stable on February 20, 2025 as part of Rust 1.85.0. Since the current date is September 2025, this edition has been stable for over seven months and is safe to use in production.
src/database.rs (8)
1-13: LGTM! Clean module structure.The imports are appropriate and the
Databasestruct encapsulates the connection pool cleanly.
16-35: LGTM! Database initialization properly addressed past concerns.The initialization correctly:
- Enables foreign key enforcement (line 30)
- Sets WAL journal mode for better concurrency (line 31)
- Configures a 5-second busy timeout to reduce lock contention (line 32)
- Uses
SqliteConnectOptionsbuilder pattern instead of string URLsThese changes address all the critical concurrency and integrity issues flagged in previous reviews.
37-80: LGTM! Schema and indexes are well-designed.The schema definition:
- Properly declares the foreign key constraint (line 63)
- Creates appropriate indexes for
token_idanddatecolumns to optimize queries- Uses
IF NOT EXISTSfor idempotent initializationThe indexes will efficiently support the query patterns in
get_webhook_requests.
84-93: LGTM! Token creation is straightforward and safe.The method uses parameterized queries to prevent SQL injection and properly propagates errors.
95-112: LGTM! Token listing implementation is correct.The method:
- Orders by creation time descending
- Uses safe row mapping
- Properly collects results
114-121: LGTM! Token existence check is efficient.Using
COUNT(*)with a parameterized query is appropriate for a simple existence check.
123-139: LGTM! Token deletion is now atomic and addresses previous race condition.The method correctly:
- Wraps both DELETE statements in a single transaction (lines 124, 136)
- Deletes child rows before the parent to respect FK constraints
- Uses
&mut *txfor transaction-scoped executionThis implementation addresses the critical race condition flagged in previous reviews.
141-172: LGTM! Webhook request storage properly handles JSON serialization.The method:
- Serializes complex fields (headers, query_parameters, body_object) to JSON
- Uses
transpose()for optional field handling (lines 144-149)- Employs parameterized queries to prevent SQL injection
2 participants
No description provided.