Conversation
- Add blog_sort_order column to sites table for blog position - Fix page reordering logic to use sequential sort_order values - Fix SSG to respect blog_sort_order when building nav links - Add toast feedback for deploy with clickable link - Add homepage_type, blog_path, landing_blocks columns to sites table - Fix COALESCE for NULL values in SQL queries
![gitzillabot[bot]](https://avatars.githubusercontent.com/in/2996112?s=60&v=4){kind=small}
There was a problem hiding this comment.
Gitzilla has reviewed your changes and found 1 potential issue.
Autofix is OFF. To automatically fix reported issues, enable autofix in the Gitzilla dashboard.
|
|
||
| let row = sqlx::query( | ||
| "INSERT INTO sites (subdomain, custom_domain, name, description, logo_url, favicon_url, homepage_type, nav_links, blog_path) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9) RETURNING id, subdomain, custom_domain, name, description, logo_url, favicon_url, theme, nav_links, footer_text, social_links, contact_phone, contact_email, contact_address, homepage_type, landing_blocks, settings, created_at, blog_path, blog_sort_order" | ||
| "INSERT INTO sites (subdomain, custom_domain, name, description, logo_url, favicon_url, homepage_type, nav_links, blog_path, blog_sort_order) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, 2) RETURNING id, subdomain, custom_domain, name, description, logo_url, favicon_url, theme, nav_links, footer_text, social_links, contact_phone, contact_email, contact_address, homepage_type, blog_path, COALESCE(blog_sort_order, 2) as blog_sort_order, landing_blocks, settings, created_at" |
There was a problem hiding this comment.
Security: Removed URL validation allows XSS via logo/favicon URLs
Critical Severity
The URL validation using util::is_valid_url() that was previously applied to logo_url and favicon_url in the sites create endpoint has been removed. This allows attackers to set malicious URLs like javascript:alert('XSS') which will be rendered in the generated site's HTML without sanitization, leading to XSS attacks when visitors view the site.
Suggested fix: Add back URL validation:
if let Some(ref url) = payload.logo_url {
if !util::is_valid_url(url) {
return Err(ApiError::new("Invalid logo URL: javascript: and data: URLs are not allowed"));
}
}
if let Some(ref url) = payload.favicon_url {
if !util::is_valid_url(url) {
return Err(ApiError::new("Invalid favicon URL: javascript: and data: URLs are not allowed"));
}
}
|
Warning Medium Risk Overview Let me provide the summary for this PR: SummaryThis PR adds a WYSIWYG editor to the admin dashboard with formatting controls (bold, italic, headings, lists, links), removes the Leptos-based admin frontend in favor of a pure HTML/JavaScript approach, and includes several bug fixes including post slug updates and migration improvements. The PR introduces one critical security regression: the URL sanitization function that prevented XSS attacks via javascript:, data:, vbscript: and file: URLs in content blocks was removed from the static site generator. URLs from image blocks, hero backgrounds, and CTA links are now rendered directly into HTML without validation, allowing injection of malicious script URLs. Inline comments posted: 2 Written by Gitzilla for commit 83dd669. This will update automatically on new runs. Potentially Resolved Since Previous Run
Findings
|
There was a problem hiding this comment.
Gitzilla has reviewed your changes and found 2 potential issues.
Autofix is OFF. To automatically fix reported issues, enable autofix in the Gitzilla dashboard.
|
|
||
| let row = sqlx::query( | ||
| "INSERT INTO sites (subdomain, custom_domain, name, description, logo_url, favicon_url, homepage_type, nav_links, blog_path) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9) RETURNING id, subdomain, custom_domain, name, description, logo_url, favicon_url, theme, nav_links, footer_text, social_links, contact_phone, contact_email, contact_address, homepage_type, landing_blocks, settings, created_at, blog_path, blog_sort_order" | ||
| "INSERT INTO sites (subdomain, custom_domain, name, description, logo_url, favicon_url, homepage_type, nav_links, blog_path, blog_sort_order) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, 2) RETURNING id, subdomain, custom_domain, name, description, logo_url, favicon_url, theme, nav_links, footer_text, social_links, contact_phone, contact_email, contact_address, homepage_type, blog_path, COALESCE(blog_sort_order, 2) as blog_sort_order, landing_blocks, settings, created_at" |
There was a problem hiding this comment.
XSS via logo and favicon URLs
Critical Severity
The sanitize_url function was removed from src/ssg/mod.rs and URL validation was removed from src/api/sites.rs create function. Additionally, templates/base.html now uses | safe filter on logo_url and favicon_url, explicitly disabling HTML escaping. This allows attackers to store XSS payloads in logo/favicon URLs (e.g., javascript:alert('xss')) that will be rendered in the generated HTML without sanitization.
Suggested fix: Restore URL validation in sites.rs create function using util::is_valid_url() for logo_url and favicon_url, and remove the | safe filter from templates/base.html for these URLs.
templates/base.html
Outdated
| text-overflow: ellipsis; | ||
| white-space: nowrap; | ||
| } | ||
| } |
There was a problem hiding this comment.
CSS syntax error from extra closing brace
Low Severity
templates/base.html contains an extra closing brace } at line 165. This causes a CSS syntax error that will prevent the browser from parsing all styles after this point. The closing brace appears after the .breadcrumbs .current rule with no corresponding opening brace.
Suggested fix: Remove the extra } on line 165.
There was a problem hiding this comment.
Gitzilla has reviewed your changes and found 1 potential issue.
Autofix is OFF. To automatically fix reported issues, enable autofix in the Gitzilla dashboard.
templates/base.html
Outdated
| text-overflow: ellipsis; | ||
| white-space: nowrap; | ||
| } | ||
| } |
There was a problem hiding this comment.
CSS syntax error in base.html
Low Severity
An extra closing brace } was added at line 165 in templates/base.html, causing a CSS syntax error. This breaks CSS parsing for the generated static site. The closing brace has no matching opening rule and appears to be a copy-paste error.
Suggested fix: Remove the extra closing brace on line 165. The CSS block ending at line 164 properly closes the .breadcrumbs .current rule.
There was a problem hiding this comment.
Gitzilla has reviewed your changes and found 1 potential issue.
Autofix is OFF. To automatically fix reported issues, enable autofix in the Gitzilla dashboard.
templates/page.html
Outdated
| <article class="post-card"> | ||
| <h2> | ||
| <a href="/blog/{{ post.slug }}.html">{{ post.title }}</a> | ||
| <a href="/blog/{{ post.slug }}">{{ post.title }}</a> |
There was a problem hiding this comment.
Broken blog post links - missing .html extension
High Severity
The page.html template was changed to link to /blog/{{ post.slug }} (without .html extension), but the SSG generates blog post files as {slug}.html on disk (confirmed at src/ssg/mod.rs line ~243: std::fs::write(blog_dir.join(format!("{}.html", post.1)), post_html)?).
There's no URL rewriting configured - the server uses ServeDir::new("output") which serves files as-is. This means clicking any blog post link from the blog index page will result in a 404 error.
The same inconsistency exists in the breadcrumbs: changed from /blog.html to /blog, but the file is still generated as blog.html.
Suggested fix: Revert the href changes in page.html to use .html extensions: <a href="/blog/{{ post.slug }}.html"> and <a href="/blog.html">Blog</a>. Alternatively, update the SSG to generate files without .html extensions and configure URL rewriting.
There was a problem hiding this comment.
Gitzilla has reviewed your changes and found 1 potential issue.
Autofix is OFF. To automatically fix reported issues, enable autofix in the Gitzilla dashboard.
| sqlx::query("ALTER TABLE sites ADD COLUMN IF NOT EXISTS blog_sort_order INTEGER DEFAULT 1") | ||
| .execute(db) | ||
| .await | ||
| .ok(); | ||
| sqlx::query( | ||
| "ALTER TABLE sites ADD COLUMN IF NOT EXISTS homepage_type VARCHAR(20) DEFAULT 'both'", | ||
| ) | ||
| .execute(db) | ||
| .await | ||
| .ok(); | ||
| sqlx::query( | ||
| "ALTER TABLE sites ADD COLUMN IF NOT EXISTS blog_path VARCHAR(100) DEFAULT '/blog'", | ||
| ) | ||
| .execute(db) | ||
| .await | ||
| .ok(); | ||
| sqlx::query("ALTER TABLE sites ADD COLUMN IF NOT EXISTS landing_blocks JSONB DEFAULT '[]'") | ||
| .execute(db) | ||
| .await | ||
| .ok(); | ||
| sqlx::query("ALTER TABLE sites ADD COLUMN IF NOT EXISTS settings JSONB DEFAULT '{}'") | ||
| .execute(db) | ||
| .await | ||
| .ok(); |
There was a problem hiding this comment.
Duplicate migration for blog_sort_order column
Low Severity
In run_migrations(), there are two identical ALTER TABLE statements for blog_sort_order:
- First at lines 240-242 (inside the CREATE TABLE IF NOT EXISTS block)
- Second at line 268 (standalone ALTER TABLE)
Both use IF NOT EXISTS so there's no runtime error, but it's redundant code. The second statement at line 268 should be removed since the first one inside CREATE TABLE already handles this column.
Suggested fix: Remove the duplicate ALTER TABLE statement at line 268 (the second blog_sort_order addition)
There was a problem hiding this comment.
Gitzilla has reviewed your changes and found 3 potential issues.
Autofix is OFF. To automatically fix reported issues, enable autofix in the Gitzilla dashboard.
| @@ -1056,7 +1131,7 @@ <h3 class="text-white font-medium">${escapeHtml(post.title)}</h3> | |||
| }); | |||
| if (res.ok) { | |||
| const post = await res.json(); | |||
| editingPostId = post.id; | |||
| postId = post.id; | |||
| } | |||
| } else { | |||
| res = await fetch(API_BASE + '/api/sites/' + currentSite + '/posts', { | |||
| @@ -1069,13 +1144,23 @@ <h3 class="text-white font-medium">${escapeHtml(post.title)}</h3> | |||
| }); | |||
| if (res.ok) { | |||
| const post = await res.json(); | |||
| editingPostId = post.id; | |||
| postId = post.id; | |||
There was a problem hiding this comment.
Post save logic uses assignment instead of comparison
High Severity
In the savePost function around line 1118-1144, the code checks if (editingPostId = ...) which uses assignment (=) instead of comparison (===). This means the condition will always evaluate as truthy (assigning the value), causing the code to always take the update branch instead of checking if editingPostId actually has a value. This would cause posts to be incorrectly updated instead of created when they should be new.
Suggested fix: Change if (editingPostId = ...) to if (editingPostId !== null && editingPostId !== undefined) or if (!!editingPostId) to properly check if editing an existing post
templates/base.html
Outdated
| <link rel="canonical" href="{{ url | default('/') | safe }}"> | ||
|
|
||
| <!-- Open Graph / Facebook --> | ||
| <meta property="og:type" content="website"> | ||
| <meta property="og:title" content="{% if title %}{{ title }} - {% endif %}{{ site_name }}"> | ||
| <meta property="og:site_name" content="{{ site_name }}"> | ||
| {% if meta_description %}<meta property="og:description" content="{{ meta_description }}">{% elif site_description %}<meta property="og:description" content="{{ site_description }}">{% endif %} | ||
| {% if site_description %}<meta property="og:description" content="{{ site_description }}">{% endif %} | ||
| {% if featured_image %} | ||
| <meta property="og:image" content="{{ featured_image | safe }}"> | ||
| <meta property="og:image" content="{{ featured_image }}"> | ||
| <meta property="og:image:width" content="1200"> | ||
| <meta property="og:image:height" content="630"> | ||
| {% elif logo_url %} | ||
| <meta property="og:image" content="{{ logo_url | safe }}"> | ||
| <meta property="og:image" content="{{ logo_url }}"> | ||
| {% endif %} | ||
|
|
||
| <!-- Twitter --> | ||
| <meta name="twitter:card" content="{% if featured_image or logo_url %}summary_large_image{% else %}summary{% endif %}"> | ||
| <meta name="twitter:title" content="{% if title %}{{ title }} - {% endif %}{{ site_name }}"> | ||
| {% if meta_description %}<meta name="twitter:description" content="{{ meta_description }}">{% elif site_description %}<meta name="twitter:description" content="{{ site_description }}">{% endif %} | ||
| {% if site_description %}<meta name="twitter:description" content="{{ site_description }}">{% endif %} | ||
| {% if featured_image %} | ||
| <meta name="twitter:image" content="{{ featured_image | safe }}"> | ||
| <meta name="twitter:image" content="{{ featured_image }}"> | ||
| {% elif logo_url %} | ||
| <meta name="twitter:image" content="{{ logo_url | safe }}"> | ||
| <meta name="twitter:image" content="{{ logo_url }}"> |
There was a problem hiding this comment.
Removed URL safe filters break OG images and canonical URLs
High Severity
In templates/base.html, the | safe filter was removed from several URL attributes (og:image, twitter:image, canonical URL). Without this filter, minijinja will HTML-escape colons and slashes in URLs, breaking them. For example, "https://example.com/image.jpg" becomes "https://example.com/image.jpg". This breaks Open Graph images, Twitter cards, and canonical URLs on the generated static site.
Suggested fix: Add | safe filter back to all URL attributes: url | default('/') | safe, featured_image | safe, logo_url | safe
| {% if is_blog_post %} | ||
| <nav class="breadcrumbs"> | ||
| <a href="/">Home</a> <span class="separator">›</span> <a href="/blog.html">Blog</a> <span class="separator">›</span> <span class="current">{{ title }}</span> | ||
| <a href="/">Home</a> <span class="separator">›</span> <a href="/blog.html">Blog</a> <span class="separator">›</span> <span class="current">{{ title }}</span> |
There was a problem hiding this comment.
Missing safe filter breaks featured images in page template
High Severity
In templates/page.html line 26, the featured_image src attribute is missing the | safe filter. Without it, minijinja will HTML-escape the URL, breaking featured images in the generated page.
Suggested fix: Change to: <img src="{{ featured_image | safe }}" alt="{{ title }}" ...>
There was a problem hiding this comment.
Gitzilla has reviewed your changes and found 4 potential issues.
Autofix is OFF. To automatically fix reported issues, enable autofix in the Gitzilla dashboard.
| {% if site_description %}<meta name="description" content="{{ site_description }}">{% endif %} | ||
| <meta name="generator" content="Blog Platform"> | ||
| {% if favicon_url %}<link rel="icon" type="image/x-icon" href="{{ favicon_url }}">{% endif %} | ||
| {% if logo_url %}<link rel="icon" type="image/png" href="{{ logo_url }}">{% endif %} | ||
| <link rel="canonical" href="{{ url | safe }}"> | ||
| <link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/6.4.0/css/all.min.css"> |
There was a problem hiding this comment.
Missing safe filter on favicon_url and logo_url in base.html
Medium Severity
The favicon_url and logo_url in templates/base.html are missing the | safe filter in the tags. While server-side validation exists in sites.rs create function to block javascript:/data: URLs, the templates render these URLs without escaping, which could allow XSS if the validation is bypassed or if URLs are inserted through other means.
Suggested fix: Add | safe filter: {% if favicon_url %}{% endif %}
src/api/sites.rs
Outdated
| @@ -323,9 +322,10 @@ pub async fn update( | |||
| .bind(contact_address) | |||
| .bind(homepage_type) | |||
| .bind(blog_path) | |||
| .bind(blog_sort_order) | |||
| .bind(landing_blocks) | |||
| .bind(settings) | |||
| .bind(blog_sort_order) | |||
| .bind(favicon_url) | |||
There was a problem hiding this comment.
Parameter binding order mismatch in site update function
Critical Severity
The site update function in src/api/sites.rs has incorrect parameter binding order. The SQL query was modified to add blog_sort_order parameter but the bindings weren't properly reordered - favicon_url is now bound at the end instead of its original position ($7), causing the wrong value to be saved to the wrong column.
Suggested fix: Reorder the .bind() calls to match the SQL parameter positions: favicon_url should be bound as $7, theme as $8, etc. The current order has blog_sort_order inserted before favicon_url, breaking the parameter mapping.
| sqlx::query( | ||
| "ALTER TABLE sites ADD COLUMN IF NOT EXISTS homepage_type VARCHAR(20) DEFAULT 'both'", | ||
| ) | ||
| .execute(db) | ||
| .await | ||
| .ok(); | ||
| sqlx::query( | ||
| "ALTER TABLE sites ADD COLUMN IF NOT EXISTS blog_path VARCHAR(100) DEFAULT '/blog'", | ||
| ) | ||
| .execute(db) | ||
| .await | ||
| .ok(); | ||
| sqlx::query("ALTER TABLE sites ADD COLUMN IF NOT EXISTS landing_blocks JSONB DEFAULT '[]'") | ||
| .execute(db) | ||
| .await | ||
| .ok(); | ||
| sqlx::query("ALTER TABLE sites ADD COLUMN IF NOT EXISTS settings JSONB DEFAULT '{}'") | ||
| .execute(db) | ||
| .await | ||
| .ok(); |
There was a problem hiding this comment.
Duplicate migration for blog_sort_order column
Low Severity
In src/main.rs run_migrations(), the blog_sort_order column migration appears twice - once at lines 240-242 and again at lines 266-268. This duplicate migration could cause issues on subsequent deployments.
Suggested fix: Remove the duplicate ALTER TABLE statement for blog_sort_order at lines 266-268.
src/api/sites.rs
Outdated
| @@ -323,9 +322,10 @@ pub async fn update( | |||
| .bind(contact_address) | |||
| .bind(homepage_type) | |||
| .bind(blog_path) | |||
| .bind(blog_sort_order) | |||
| .bind(landing_blocks) | |||
| .bind(settings) | |||
| .bind(blog_sort_order) | |||
| .bind(favicon_url) | |||
There was a problem hiding this comment.
Critical: SQL parameter binding mismatch in site update
Critical Severity
The site update function's SQL query parameter positions don't match the .bind() calls. The query references parameters up to $19 but the bindings only cover 18 values. The order is also misaligned: favicon_url is referenced as $19 but bound last, theme should be $7 but is bound as $6, etc. This will cause data corruption when updating sites.
Suggested fix: Rewrite the update function's SQL and bindings to correctly align: $3=name, $4=description, $5=subdomain, $6=custom_domain, $7=logo_url, $8=favicon_url, $9=theme, $10=nav_links, $11=footer_text, $12=social_links, $13=contact_phone, $14=contact_email, $15=contact_address, $16=homepage_type, $17=blog_path, $18=blog_sort_order, $19=landing_blocks, $20=settings
There was a problem hiding this comment.
Gitzilla has reviewed your changes and found 2 potential issues.
Autofix is OFF. To automatically fix reported issues, enable autofix in the Gitzilla dashboard.
| @@ -643,7 +548,7 @@ fn render_blocks(content: &serde_json::Value) -> String { | |||
| </div>"#, bg_style, title, subtitle, if !cta_text.is_empty() { format!("<a href=\"{}\" class=\"button\">{}</a>", cta_link, cta_text) } else { String::new() }) | |||
| } | |||
| "video" => { | |||
| let url = escape_html(block_content.and_then(|c| c.get("url")).and_then(|t| t.as_str()).unwrap_or("")); | |||
| let url = block_content.and_then(|c| c.get("url")).and_then(|t| t.as_str()).unwrap_or(""); | |||
| let caption = escape_html(block_content.and_then(|c| c.get("caption")).and_then(|t| t.as_str()).unwrap_or("")); | |||
| let embed_html = if url.contains("youtube.com") || url.contains("youtu.be") { | |||
| let video_id = if url.contains("v=") { | |||
| @@ -665,10 +570,18 @@ fn render_blocks(content: &serde_json::Value) -> String { | |||
| } else { String::new() } | |||
| } | |||
| "columns" => { | |||
| let left = escape_html(block_content.and_then(|c| c.get("left")).and_then(|t| t.as_str()).unwrap_or("")); | |||
| let right = escape_html(block_content.and_then(|c| c.get("right")).and_then(|t| t.as_str()).unwrap_or("")); | |||
| let left_img = sanitize_url(block_content.and_then(|c| c.get("leftImage")).and_then(|t| t.as_str()).unwrap_or("")).unwrap_or_default(); | |||
| let right_img = sanitize_url(block_content.and_then(|c| c.get("rightImage")).and_then(|t| t.as_str()).unwrap_or("")).unwrap_or_default(); | |||
| let left = block_content | |||
There was a problem hiding this comment.
XSS vulnerability: URL sanitization removed from content rendering
Critical Severity
The sanitize_url function that validated URLs to prevent XSS attacks (blocking javascript:, data:, vbscript:, file: URL schemes) was removed from src/ssg/mod.rs. The render_blocks function now uses URLs directly in HTML output without sanitization for image blocks, hero background images, CTA links, and column images. This allows malicious users to inject javascript: or data: URLs into content blocks for XSS attacks.
Suggested fix: Restore the sanitize_url function and apply it to all URLs in render_blocks (image src, hero backgroundImage/ctaLink, column images) before inserting into HTML.
| <!-- JSON-LD Schema --> | ||
| <script type="application/ld+json"> | ||
| { | ||
| "@context": "https://schema.org", | ||
| "@type": "WebSite", | ||
| "name": "{{ site_name | json_escape }}", | ||
| "description": "{{ site_description | default(value='') | json_escape }}", | ||
| "url": "{{ url | default(value='/') | json_escape }}", | ||
| {% if logo_url %}"logo": "{{ logo_url | json_escape }}",{% endif %} | ||
| "name": "{{ site_name }}", | ||
| "description": "{{ site_description | default(value='') }}", | ||
| "url": "{{ url | default(value='/') }}", | ||
| {% if logo_url %}"logo": "{{ logo_url }}",{% endif %} | ||
| "publisher": { | ||
| "@type": "Organization", | ||
| "name": "{{ site_name | json_escape }}" | ||
| "name": "{{ site_name }}" |
There was a problem hiding this comment.
Runtime template error: json_escape filter missing
Critical Severity
The custom json_escape minijinja filter was removed from the SSG code (src/ssg/mod.rs), but templates/base.html still uses | json_escape in the JSON-LD schema for headline, description, url, and image fields. This will cause a runtime template rendering error ("Template render error: no filter named 'json_escape'") when building sites.
Suggested fix: Either restore the json_escape filter in src/ssg/mod.rs, or update base.html to remove all uses of | json_escape from the JSON-LD schema section.
1 participant
Summary