Fix broken documentation links on GitHub Pages with security improvements#78
Fix broken documentation links on GitHub Pages with security improvements#78
Conversation
Co-authored-by: luandev <6452989+luandev@users.noreply.github.com>
Co-authored-by: luandev <6452989+luandev@users.noreply.github.com>
Co-authored-by: luandev <6452989+luandev@users.noreply.github.com>
| ## Quick start | ||
| 1. **Install the app** | ||
|
|
||
| **Option A: Standalone Bundle (Recommended - No Node.js required)** |
There was a problem hiding this comment.
@copilot first option should be docker since releases aren't working right now
There was a problem hiding this comment.
Updated! Docker is now Option A (recommended), with Standalone Bundle as Option B and Desktop App as Option C. Changes applied to both user guide files. (ae43053)
Co-authored-by: luandev <6452989+luandev@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes broken documentation links on GitHub Pages by creating a nested docs/docs/ directory structure with HTML wrappers that dynamically render Markdown files using marked.js and github-markdown-css. The landing page links are updated to point to directories (e.g., docs/) instead of raw Markdown files (e.g., docs/README.md), aligning with GitHub Pages' URL routing.
Key changes:
- Updated landing page (
docs/index.html) to use directory-based URLs for documentation links - Created HTML wrapper pages that fetch and render Markdown dynamically
- Duplicated documentation from original locations to new nested structure
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
docs/index.html |
Updated all documentation links from .md file paths to directory paths (e.g., docs/README.md → docs/) to fix GitHub Pages 404 errors |
docs/docs/index.html |
New HTML wrapper that loads and renders the Developer Guide README.md with navigation header |
docs/docs/README.md |
Duplicated developer guide documentation (missing important Crocdb offline warning from original) |
docs/docs/user/index.html |
New HTML wrapper that loads and renders the User Guide README.md with navigation header |
docs/docs/user/README.md |
Duplicated user guide documentation with incorrect information about download defaults |
docs/docs/user/retro-handhelds.md |
Duplicated retro handheld guide for nested structure |
| const html = marked.parse(markdown); | ||
| document.getElementById('content').innerHTML = html; | ||
| } catch (error) { | ||
| document.getElementById('content').innerHTML = '<p>Error loading documentation. Please visit the <a href="https://github.com/luandev/jacare">GitHub repository</a> for the latest documentation.</p>'; |
There was a problem hiding this comment.
The error message display uses innerHTML to inject potentially unsafe content. While the current implementation only displays a hardcoded message, consider using textContent for the error path or sanitizing any dynamic content to prevent potential XSS issues if the error handling is modified in the future.
| document.getElementById('content').innerHTML = '<p>Error loading documentation. Please visit the <a href="https://github.com/luandev/jacare">GitHub repository</a> for the latest documentation.</p>'; | |
| const contentEl = document.getElementById('content'); | |
| if (!contentEl) return; | |
| // Clear any existing content safely | |
| contentEl.textContent = ''; | |
| // Build the error message using DOM APIs to avoid innerHTML | |
| const p = document.createElement('p'); | |
| p.appendChild(document.createTextNode('Error loading documentation. Please visit the ')); | |
| const link = document.createElement('a'); | |
| link.href = 'https://github.com/luandev/jacare'; | |
| link.textContent = 'GitHub repository'; | |
| p.appendChild(link); | |
| p.appendChild(document.createTextNode(' for the latest documentation.')); | |
| contentEl.appendChild(p); |
| @@ -0,0 +1,187 @@ | |||
| # Jacare Developer Guide 🧑💻🐊 | |||
|
|
|||
There was a problem hiding this comment.
The important warning notice about Crocdb being offline is missing from this new documentation file. The original docs/README.md includes a critical notice at the top about the main Crocdb source being offline. This information should be included here for consistency and to inform users of current service status.
| > Crocdb service status: The primary Crocdb metadata service is currently offline and not actively maintained. Features that depend on live requests to the Crocdb API (`https://api.crocdb.net`) may fail or behave differently from what is described in this document. Jacare still works for managing a local library and any previously cached metadata, but online lookups should be considered best-effort only. |
There was a problem hiding this comment.
Added the Crocdb offline warning notice to the nested developer guide. (4a98375)
| @@ -0,0 +1,66 @@ | |||
| # Jacare Friendly Guide 😀🐊 | |||
|
|
|||
There was a problem hiding this comment.
The important warning notice about Crocdb being offline is missing from this new documentation file. The original docs/user/README.md includes a critical notice at the top about the main Crocdb source being offline. This information should be included here for consistency and to inform users of current service status.
| > **Important:** The main Crocdb metadata service is currently offline/unavailable. | |
| > Jacare will still work for managing your local ROM library, but features that depend on Crocdb (online search, metadata enrichment, and downloads) may be limited or fail until the service is restored. |
There was a problem hiding this comment.
Added the Crocdb offline warning notice to the nested user guide. (4a98375)
| ## Staying organized | ||
| - Keep your ROMs in clearly named folders organized by platform (e.g., `library/snes/`, `library/n64/`). | ||
| - Re-run **Scan** after adding new games so Jacare can index them. | ||
| - If you're offline, you can still browse what was already scanned, and Jacare will use cached results until you reconnect. |
There was a problem hiding this comment.
This statement is inaccurate. According to the project's coding guidelines, downloads are disabled by default for safety and require setting CROCDESK_ENABLE_DOWNLOADS=true to enable. The text should be corrected to: "Downloads are disabled by default. Enable them with CROCDESK_ENABLE_DOWNLOADS=true environment variable."
There was a problem hiding this comment.
Fixed! Downloads are now correctly documented as disabled by default, with CROCDESK_ENABLE_DOWNLOADS=true required to enable them. (4a98375)
| <link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/github-markdown-css@5.5.1/github-markdown.min.css"> | ||
| <style> | ||
| :root { | ||
| --bg: #0d1117; | ||
| --text: #c9d1d9; | ||
| --link: #58a6ff; | ||
| --border: #30363d; | ||
| } | ||
|
|
||
| @media (prefers-color-scheme: light) { | ||
| :root { | ||
| --bg: #ffffff; | ||
| --text: #24292f; | ||
| --link: #0969da; | ||
| --border: #d0d7de; | ||
| } | ||
| } | ||
|
|
||
| body { | ||
| margin: 0; | ||
| padding: 0; | ||
| background: var(--bg); | ||
| color: var(--text); | ||
| font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', 'Noto Sans', Helvetica, Arial, sans-serif; | ||
| } | ||
|
|
||
| .header { | ||
| background: var(--bg); | ||
| border-bottom: 1px solid var(--border); | ||
| padding: 16px 24px; | ||
| position: sticky; | ||
| top: 0; | ||
| z-index: 100; | ||
| display: flex; | ||
| justify-content: space-between; | ||
| align-items: center; | ||
| } | ||
|
|
||
| .header-title { | ||
| font-size: 18px; | ||
| font-weight: 600; | ||
| display: flex; | ||
| align-items: center; | ||
| gap: 8px; | ||
| } | ||
|
|
||
| .header-links { | ||
| display: flex; | ||
| gap: 12px; | ||
| } | ||
|
|
||
| .header-links a { | ||
| color: var(--link); | ||
| text-decoration: none; | ||
| font-size: 14px; | ||
| } | ||
|
|
||
| .header-links a:hover { | ||
| text-decoration: underline; | ||
| } | ||
|
|
||
| .container { | ||
| max-width: 980px; | ||
| margin: 0 auto; | ||
| padding: 24px; | ||
| } | ||
|
|
||
| .markdown-body { | ||
| box-sizing: border-box; | ||
| min-width: 200px; | ||
| max-width: 980px; | ||
| margin: 0 auto; | ||
| } | ||
|
|
||
| @media (max-width: 767px) { | ||
| .container { | ||
| padding: 15px; | ||
| } | ||
| } | ||
| </style> | ||
| </head> | ||
| <body> | ||
| <div class="header"> | ||
| <div class="header-title"> | ||
| <span>🐊</span> | ||
| <span>Jacare Developer Guide</span> | ||
| </div> | ||
| <div class="header-links"> | ||
| <a href="../../">Home</a> | ||
| <a href="../user/">User Guide</a> | ||
| <a href="https://github.com/luandev/jacare">GitHub</a> | ||
| </div> | ||
| </div> | ||
|
|
||
| <div class="container"> | ||
| <article class="markdown-body" id="content"> | ||
| <p>Loading documentation...</p> | ||
| </article> | ||
| </div> | ||
|
|
||
| <script src="https://cdn.jsdelivr.net/npm/marked@11.1.1/marked.min.js"></script> |
There was a problem hiding this comment.
External CDN dependencies from jsdelivr.net are loaded without Subresource Integrity (SRI) hashes. This creates a security risk as CDN content could be compromised. Consider either: (1) adding SRI integrity attributes to the script and link tags, (2) hosting these resources locally, or (3) documenting the security trade-off of using CDN resources for GitHub Pages deployment.
There was a problem hiding this comment.
Added SRI integrity hashes and crossorigin="anonymous" attributes to both CDN resources (github-markdown-css and marked.js) in both HTML wrapper pages. (4a98375)
| <link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/github-markdown-css@5.5.1/github-markdown.min.css"> | ||
| <style> | ||
| :root { | ||
| --bg: #0d1117; | ||
| --text: #c9d1d9; | ||
| --link: #58a6ff; | ||
| --border: #30363d; | ||
| } | ||
|
|
||
| @media (prefers-color-scheme: light) { | ||
| :root { | ||
| --bg: #ffffff; | ||
| --text: #24292f; | ||
| --link: #0969da; | ||
| --border: #d0d7de; | ||
| } | ||
| } | ||
|
|
||
| body { | ||
| margin: 0; | ||
| padding: 0; | ||
| background: var(--bg); | ||
| color: var(--text); | ||
| font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', 'Noto Sans', Helvetica, Arial, sans-serif; | ||
| } | ||
|
|
||
| .header { | ||
| background: var(--bg); | ||
| border-bottom: 1px solid var(--border); | ||
| padding: 16px 24px; | ||
| position: sticky; | ||
| top: 0; | ||
| z-index: 100; | ||
| display: flex; | ||
| justify-content: space-between; | ||
| align-items: center; | ||
| } | ||
|
|
||
| .header-title { | ||
| font-size: 18px; | ||
| font-weight: 600; | ||
| display: flex; | ||
| align-items: center; | ||
| gap: 8px; | ||
| } | ||
|
|
||
| .header-links { | ||
| display: flex; | ||
| gap: 12px; | ||
| } | ||
|
|
||
| .header-links a { | ||
| color: var(--link); | ||
| text-decoration: none; | ||
| font-size: 14px; | ||
| } | ||
|
|
||
| .header-links a:hover { | ||
| text-decoration: underline; | ||
| } | ||
|
|
||
| .container { | ||
| max-width: 980px; | ||
| margin: 0 auto; | ||
| padding: 24px; | ||
| } | ||
|
|
||
| .markdown-body { | ||
| box-sizing: border-box; | ||
| min-width: 200px; | ||
| max-width: 980px; | ||
| margin: 0 auto; | ||
| } | ||
|
|
||
| @media (max-width: 767px) { | ||
| .container { | ||
| padding: 15px; | ||
| } | ||
| } | ||
| </style> | ||
| </head> | ||
| <body> | ||
| <div class="header"> | ||
| <div class="header-title"> | ||
| <span>😀🐊</span> | ||
| <span>Jacare User Guide</span> | ||
| </div> | ||
| <div class="header-links"> | ||
| <a href="../../../">Home</a> | ||
| <a href="../../">Developer Guide</a> | ||
| <a href="https://github.com/luandev/jacare">GitHub</a> | ||
| </div> | ||
| </div> | ||
|
|
||
| <div class="container"> | ||
| <article class="markdown-body" id="content"> | ||
| <p>Loading documentation...</p> | ||
| </article> | ||
| </div> | ||
|
|
||
| <script src="https://cdn.jsdelivr.net/npm/marked@11.1.1/marked.min.js"></script> |
There was a problem hiding this comment.
External CDN dependencies from jsdelivr.net are loaded without Subresource Integrity (SRI) hashes. This creates a security risk as CDN content could be compromised. Consider either: (1) adding SRI integrity attributes to the script and link tags, (2) hosting these resources locally, or (3) documenting the security trade-off of using CDN resources for GitHub Pages deployment.
| const html = marked.parse(markdown); | ||
| document.getElementById('content').innerHTML = html; | ||
| } catch (error) { | ||
| document.getElementById('content').innerHTML = '<p>Error loading documentation. Please visit the <a href="https://github.com/luandev/jacare">GitHub repository</a> for the latest documentation.</p>'; |
There was a problem hiding this comment.
The error message display uses innerHTML to inject potentially unsafe content. While the current implementation only displays a hardcoded message, consider using textContent for the error path or sanitizing any dynamic content to prevent potential XSS issues if the error handling is modified in the future.
| document.getElementById('content').innerHTML = '<p>Error loading documentation. Please visit the <a href="https://github.com/luandev/jacare">GitHub repository</a> for the latest documentation.</p>'; | |
| const contentEl = document.getElementById('content'); | |
| if (!contentEl) return; | |
| // Clear any existing content | |
| contentEl.textContent = ''; | |
| const paragraph = document.createElement('p'); | |
| paragraph.textContent = 'Error loading documentation. Please visit the '; | |
| const link = document.createElement('a'); | |
| link.href = 'https://github.com/luandev/jacare'; | |
| link.textContent = 'GitHub repository'; | |
| paragraph.appendChild(link); | |
| paragraph.appendChild(document.createTextNode(' for the latest documentation.')); | |
| contentEl.appendChild(paragraph); |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: luandev <6452989+luandev@users.noreply.github.com>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Summary
docs/directory structure didn't match the URL paths referenced inindex.htmldocs/docs/structure with HTML wrappers that dynamically render Markdown using marked.js and github-markdown-cssTesting
Structure
URL Resolution
/docs/→docs/docs/index.html(Developer Guide)/docs/user/→docs/docs/user/index.html(User Guide)Security Improvements
crossorigin="anonymous"attributes for CORS protectionChecklist
/semver: patch,/semver: minor, or/semver: major. (See template for examples)Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.