Skip to content

feat: favicon from logo-icon.svg — all required sizes, ICO + PNG (Closes #471) #673

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
LaphoqueRC wants to merge 1 commit into from
+204 −0
Closed

feat: favicon from logo-icon.svg — all required sizes, ICO + PNG (Closes #471) #673

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file added assets/favicons/apple-touch-icon.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added assets/favicons/favicon-192x192.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added assets/favicons/favicon-512x512.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added assets/favicons/favicon.ico
View file
Open in desktop
Binary file not shown.
35 changes: 35 additions & 0 deletions assets/favicons/site.webmanifest
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
{
"name": "SolFoundry",
"short_name": "SolFoundry",
"description": "Autonomous AI Software Factory on Solana",
"start_url": "/",
"display": "standalone",
"background_color": "#0f0f23",
"theme_color": "#9945FF",
"icons": [
{
"src": "/assets/favicons/favicon-192x192.png",
"sizes": "192x192",
"type": "image/png",
"purpose": "any"
},
{
"src": "/assets/favicons/favicon-192x192.png",
"sizes": "192x192",
"type": "image/png",
"purpose": "maskable"
},
Comment on lines +16 to +21
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if logo-icon.svg exists and inspect its viewBox to assess padding
if [[ -f "assets/logo-icon.svg" ]]; then
  echo "=== SVG viewBox and dimensions ==="
  grep -oP '(viewBox|width|height)="[^"]*"' assets/logo-icon.svg | head -5
  echo ""
  echo "=== Manual check needed: verify icon content has ~10% padding for maskable safe zone ==="
else
  echo "logo-icon.svg not found at expected path"
fi

Repository: SolFoundry/solfoundry

Length of output: 261

🏁 Script executed:

#!/bin/bash
# Examine the SVG content to assess padding and visual element placement
echo "=== SVG Content (first 50 lines) ==="
head -50 assets/logo-icon.svg

echo ""
echo "=== Checking favicon files referenced in manifest ==="
ls -la assets/favicons/ | grep -E "(png|svg)"

echo ""
echo "=== Checking webmanifest for icon paths ==="
grep -A 3 '"src"' assets/favicons/site.webmanifest

Repository: SolFoundry/solfoundry

Length of output: 3092

Maskable icons may have suboptimal background clipping—verify PNG generation handles this properly.

The same PNG files are used for both purpose: "any" and purpose: "maskable" (lines 16-21 and 28-33). The source SVG (assets/logo-icon.svg) shows the background gradient extends to the full 512×512 canvas edges, which will be clipped when Android masks the icon to circular or other shapes.

The main anvil/hammer icon content is centered and has adequate padding, but the background treatment may result in visual issues on devices that apply masking. Verify that:

  1. The PNG generation process accounts for maskable clipping, or
  2. Create separate maskable-specific variants that either remove the background or ensure it handles masking gracefully
  3. Use a tool like Maskable.app to preview the actual display
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@assets/favicons/site.webmanifest` around lines 16 - 21, site.webmanifest
currently reuses the same PNG for both "purpose": "any" and "purpose":
"maskable" which risks background clipping on masked displays; regenerate or
produce maskable-specific PNGs from assets/logo-icon.svg that either remove or
extend/flatten the background to accommodate circular/masked crops, update the
manifest entries for the maskable sizes (the entries with "purpose": "maskable")
to point to those new files, and verify the result using a preview tool like
Maskable.app to ensure no important background gradients or edges are lost.

{
"src": "/assets/favicons/favicon-512x512.png",
"sizes": "512x512",
"type": "image/png",
"purpose": "any"
},
{
"src": "/assets/favicons/favicon-512x512.png",
"sizes": "512x512",
"type": "image/png",
"purpose": "maskable"
}
]
}
9 changes: 9 additions & 0 deletions index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,15 @@
<meta charset="UTF-8">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<title>SolFoundry — Autonomous AI Software Factory on Solana</title>
<!-- Favicons -->
<link rel="icon" type="image/x-icon" href="/assets/favicons/favicon.ico">
<link rel="icon" type="image/png" sizes="32x32" href="/assets/favicons/favicon.ico">
<link rel="icon" type="image/png" sizes="16x16" href="/assets/favicons/favicon.ico">
Comment on lines +9 to +10
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

MIME type mismatch: type="image/png" but href points to .ico file.

Lines 9-10 declare type="image/png" but reference favicon.ico, which is an ICO file (image/x-icon). This semantic mismatch may cause browsers to incorrectly interpret the resource or fail to use it.

The generation script (generate-favicons.sh) creates temporary 16×16 and 32×32 PNGs only to embed them into the ICO, then deletes them. If you want separate PNG link tags for these sizes, the script should preserve those files, or these lines should be removed/corrected to avoid the type mismatch.

Options:

  1. Remove lines 9-10 entirely (rely on the ICO which already embeds both sizes)
  2. Change type to image/x-icon to match the actual file type
  3. Update the generation script to output separate favicon-16x16.png and favicon-32x32.png files and reference those here
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@index.html` around lines 9 - 10, The two link elements that declare
type="image/png" but reference /assets/favicons/favicon.ico are a MIME mismatch;
fix by either changing the type attribute on those link tags to image/x-icon to
match the .ico file (update the link elements with
href="/assets/favicons/favicon.ico" accordingly) or (if you want separate PNG
assets) update the favicon generation script to preserve/output
favicon-16x16.png and favicon-32x32.png and change the hrefs to those PNG
filenames while keeping type="image/png"; remove duplicate/incorrect tags if you
choose to rely solely on the ICO.

<link rel="apple-touch-icon" sizes="180x180" href="/assets/favicons/apple-touch-icon.png">
<link rel="icon" type="image/png" sizes="192x192" href="/assets/favicons/favicon-192x192.png">
<link rel="icon" type="image/png" sizes="512x512" href="/assets/favicons/favicon-512x512.png">
<link rel="manifest" href="/assets/favicons/site.webmanifest">
<meta name="theme-color" content="#9945FF">
<link href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/6.4.0/css/all.min.css" rel="stylesheet">
<style>
* {
Expand Down
57 changes: 57 additions & 0 deletions scripts/generate-favicons.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
#!/usr/bin/env bash
# generate-favicons.sh — Regenerate favicon assets from assets/logo-icon.svg
# Usage: ./scripts/generate-favicons.sh
# Requires: rsvg-convert (librsvg) and ImageMagick 6 (convert) or 7 (magick)
set -euo pipefail

SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
REPO_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)"
SOURCE_SVG="$REPO_ROOT/assets/logo-icon.svg"
OUT_DIR="$REPO_ROOT/assets/favicons"

# ── Dependency checks ────────────────────────────────────────────────────────

if ! command -v rsvg-convert &>/dev/null; then
echo "ERROR: rsvg-convert not found. Install with: apt install librsvg2-bin / brew install librsvg" >&2
exit 1
fi

if command -v magick &>/dev/null; then
IMAGEMAGICK_CMD="magick"
elif command -v convert &>/dev/null; then
IMAGEMAGICK_CMD="convert"
else
echo "ERROR: ImageMagick not found (need 'magick' or 'convert'). Install with: apt install imagemagick / brew install imagemagick" >&2
exit 1
fi

if [[ ! -f "$SOURCE_SVG" ]]; then
echo "ERROR: Source SVG not found: $SOURCE_SVG" >&2
exit 1
fi

mkdir -p "$OUT_DIR"

echo "Generating favicons from $SOURCE_SVG using $IMAGEMAGICK_CMD …"

# ── PNG sizes ────────────────────────────────────────────────────────────────

rsvg-convert -w 16 -h 16 "$SOURCE_SVG" -o /tmp/_favicon-16.png
rsvg-convert -w 32 -h 32 "$SOURCE_SVG" -o /tmp/_favicon-32.png
Comment on lines +39 to +40
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Insecure use of predictable temp file paths in shared /tmp directory.

Using hardcoded paths like /tmp/_favicon-16.png and /tmp/_favicon-32.png in a shared directory creates a symlink race condition vulnerability. In multi-user environments, an attacker could pre-create a symlink at these paths pointing to a sensitive file, causing the script to overwrite it when run by another user.

Use mktemp to create secure temporary files:

🔒 Proposed fix using mktemp 
+# Create secure temp directory
+TMPDIR=$(mktemp -d)
+trap 'rm -rf "$TMPDIR"' EXIT
+
 # ── PNG sizes ────────────────────────────────────────────────────────────────
 
-rsvg-convert -w 16  -h 16  "$SOURCE_SVG" -o /tmp/_favicon-16.png
-rsvg-convert -w 32  -h 32  "$SOURCE_SVG" -o /tmp/_favicon-32.png
+rsvg-convert -w 16  -h 16  "$SOURCE_SVG" -o "$TMPDIR/favicon-16.png"
+rsvg-convert -w 32  -h 32  "$SOURCE_SVG" -o "$TMPDIR/favicon-32.png"
 rsvg-convert -w 180 -h 180 "$SOURCE_SVG" -o "$OUT_DIR/apple-touch-icon.png"
 rsvg-convert -w 192 -h 192 "$SOURCE_SVG" -o "$OUT_DIR/favicon-192x192.png"
 rsvg-convert -w 512 -h 512 "$SOURCE_SVG" -o "$OUT_DIR/favicon-512x512.png"
 # ── Multi-size ICO (embeds 16x16 + 32x32) ────────────────────────────────────
 
-"$IMAGEMAGICK_CMD" /tmp/_favicon-16.png /tmp/_favicon-32.png "$OUT_DIR/favicon.ico"
-rm -f /tmp/_favicon-16.png /tmp/_favicon-32.png
+"$IMAGEMAGICK_CMD" "$TMPDIR/favicon-16.png" "$TMPDIR/favicon-32.png" "$OUT_DIR/favicon.ico"
+# Cleanup handled by trap

Also applies to: 51-52

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@scripts/generate-favicons.sh` around lines 39 - 40, The script currently
writes to predictable temp paths (/tmp/_favicon-16.png, /tmp/_favicon-32.png and
similarly at the later rsvg-convert calls), which is vulnerable to symlink/race
attacks; change the logic so you create secure temporary files with mktemp (e.g.
TMP_16=$(mktemp) and TMP_32=$(mktemp)), use those variables in the rsvg-convert
invocations (references: the rsvg-convert commands and the /tmp/_favicon-*.png
names), and ensure you register a trap to remove the temp files on exit to avoid
leaks.

rsvg-convert -w 180 -h 180 "$SOURCE_SVG" -o "$OUT_DIR/apple-touch-icon.png"
rsvg-convert -w 192 -h 192 "$SOURCE_SVG" -o "$OUT_DIR/favicon-192x192.png"
rsvg-convert -w 512 -h 512 "$SOURCE_SVG" -o "$OUT_DIR/favicon-512x512.png"

echo " ✔ apple-touch-icon.png (180x180)"
echo " ✔ favicon-192x192.png (192x192)"
echo " ✔ favicon-512x512.png (512x512)"

# ── Multi-size ICO (embeds 16x16 + 32x32) ────────────────────────────────────

"$IMAGEMAGICK_CMD" /tmp/_favicon-16.png /tmp/_favicon-32.png "$OUT_DIR/favicon.ico"
rm -f /tmp/_favicon-16.png /tmp/_favicon-32.png

echo " ✔ favicon.ico (16x16 + 32x32 embedded)"
echo ""
echo "Done. 4 files in $OUT_DIR"
echo "Binary count: $(find "$OUT_DIR" -maxdepth 1 \( -name '*.png' -o -name '*.ico' \) | wc -l | tr -d ' ')"
103 changes: 103 additions & 0 deletions scripts/test-favicons.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
#!/usr/bin/env bash
# test-favicons.sh — Verify favicon assets are present and correctly referenced
set -euo pipefail

SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
REPO_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)"
FAVICONS="$REPO_ROOT/assets/favicons"
INDEX="$REPO_ROOT/index.html"
MANIFEST="$FAVICONS/site.webmanifest"
GENERATE="$REPO_ROOT/scripts/generate-favicons.sh"

PASS=0
FAIL=0

assert_file() {
if [[ -f "$1" ]]; then
echo " ✔ $2"
((PASS++)) || true
else
echo " ✗ $2 — MISSING: $1"
((FAIL++)) || true
fi
}

assert_contains() {
if grep -q "$2" "$1" 2>/dev/null; then
echo " ✔ $3"
((PASS++)) || true
else
echo " ✗ $3 — not found in $1"
((FAIL++)) || true
fi
}

assert_min_size() {
local size
size=$(wc -c < "$1" 2>/dev/null || echo 0)
if [[ "$size" -ge "$2" ]]; then
echo " ✔ $3 (${size} bytes)"
((PASS++)) || true
else
echo " ✗ $3 — too small (${size} bytes, expected ≥$2)"
((FAIL++)) || true
fi
}

echo "=== Favicon Asset Tests ==="
echo ""
echo "── Files present ──────────────────────────────────"
assert_file "$FAVICONS/favicon.ico" "favicon.ico exists"
assert_file "$FAVICONS/apple-touch-icon.png" "apple-touch-icon.png exists (180x180)"
assert_file "$FAVICONS/favicon-192x192.png" "favicon-192x192.png exists"
assert_file "$FAVICONS/favicon-512x512.png" "favicon-512x512.png exists"
assert_file "$FAVICONS/site.webmanifest" "site.webmanifest exists"

echo ""
echo "── File sizes (non-empty) ──────────────────────────"
[[ -f "$FAVICONS/favicon.ico" ]] && assert_min_size "$FAVICONS/favicon.ico" 1024 "favicon.ico is non-trivial (≥1KB)"
[[ -f "$FAVICONS/apple-touch-icon.png" ]] && assert_min_size "$FAVICONS/apple-touch-icon.png" 5000 "apple-touch-icon.png is non-trivial (≥5KB)"
[[ -f "$FAVICONS/favicon-192x192.png" ]] && assert_min_size "$FAVICONS/favicon-192x192.png" 5000 "favicon-192x192.png is non-trivial (≥5KB)"
[[ -f "$FAVICONS/favicon-512x512.png" ]] && assert_min_size "$FAVICONS/favicon-512x512.png" 20000 "favicon-512x512.png is non-trivial (≥20KB)"
Comment on lines +58 to +61
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider unconditional size assertions for clearer test failure reporting.

The pattern [[ -f "$file" ]] && assert_min_size ... silently skips size checks if files are missing. While assert_file above catches missing files, if a file is deleted between checks (unlikely but possible), or if someone reorders tests, the size check would be silently skipped without incrementing FAIL.

For more explicit test behavior, consider always calling assert_min_size and having it handle the missing file case internally:

♻️ Optional: Handle missing files in assert_min_size 
 assert_min_size() {
   local size
+  if [[ ! -f "$1" ]]; then
+    echo "  ✗ $3 — file not found: $1"
+    ((FAIL++)) || true
+    return
+  fi
   size=$(wc -c < "$1" 2>/dev/null || echo 0)

Then remove the [[ -f ... ]] && guards on lines 58-61.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@scripts/test-favicons.sh` around lines 58 - 61, Remove the conditional guards
and make the size assertions unconditional: call assert_min_size for each
favicon path (favicon.ico, apple-touch-icon.png, favicon-192x192.png,
favicon-512x512.png) without the leading [[ -f ... ]] &&. Update the
assert_min_size function to detect a missing file (using the same existence
logic as assert_file) and fail the test with a clear message when the file does
not exist, and keep its existing size check behavior otherwise; reference
assert_min_size and the FAVICONS variable to locate the changes and remove the
guarded calls currently surrounding those four filename checks.


echo ""
echo "── Binary file count (must be ≤5) ─────────────────"
BINARY_COUNT=$(find "$FAVICONS" -maxdepth 1 \( -name '*.png' -o -name '*.ico' \) | wc -l | tr -d ' ')
if [[ "$BINARY_COUNT" -le 5 ]]; then
echo " ✔ Binary count = $BINARY_COUNT (≤5)"
((PASS++)) || true
else
echo " ✗ Binary count = $BINARY_COUNT — exceeds limit of 5!"
((FAIL++)) || true
fi

echo ""
echo "── index.html references ──────────────────────────"
assert_contains "$INDEX" 'favicon.ico' "index.html: favicon.ico link"
assert_contains "$INDEX" 'apple-touch-icon' "index.html: apple-touch-icon link"
assert_contains "$INDEX" 'favicon-192x192' "index.html: favicon-192x192 link"
assert_contains "$INDEX" 'favicon-512x512' "index.html: favicon-512x512 link"
assert_contains "$INDEX" 'site.webmanifest' "index.html: manifest link"
assert_contains "$INDEX" 'theme-color' "index.html: theme-color meta"

echo ""
echo "── site.webmanifest content ────────────────────────"
assert_contains "$MANIFEST" '"name"' "manifest: name field"
assert_contains "$MANIFEST" '"icons"' "manifest: icons array"
assert_contains "$MANIFEST" '"purpose"' "manifest: icon purpose field"
assert_contains "$MANIFEST" '"192x192"' "manifest: 192x192 icon entry"
assert_contains "$MANIFEST" '"512x512"' "manifest: 512x512 icon entry"
assert_contains "$MANIFEST" '"maskable"' "manifest: maskable purpose present"

echo ""
echo "── generate-favicons.sh ────────────────────────────"
assert_file "$GENERATE" "generate-favicons.sh exists"
assert_contains "$GENERATE" 'rsvg-convert' "script: uses rsvg-convert"
assert_contains "$GENERATE" 'IMAGEMAGICK_CMD' "script: ImageMagick 6/7 compat var"
assert_contains "$GENERATE" 'magick' "script: detects magick (IM7)"
assert_contains "$GENERATE" 'convert' "script: fallback to convert (IM6)"
assert_contains "$GENERATE" 'logo-icon.svg' "script: uses logo-icon.svg as source"

echo ""
echo "=== Results: ${PASS} passed, ${FAIL} failed ==="
[[ "$FAIL" -eq 0 ]] && exit 0 || exit 1
Loading