Add first version of BioAmp-Arduino-Firmware-Explorer - #1
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughA self-contained browser repository explorer is added. It loads GitHub repository trees, supports filtering and navigation, prefetches files, renders text, Markdown, images, and binary notices, provides editing controls, and previews HTML applications in a sandboxed iframe. ChangesRepository Explorer
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant app.js
participant GitHubREST
participant RawGitHub
participant Viewer
participant PreviewIframe
Browser->>app.js: provide repository query parameters
app.js->>GitHubREST: fetch metadata and recursive tree
GitHubREST-->>app.js: return repository tree
app.js->>Viewer: render navigation and begin text prefetch
Browser->>Viewer: select a file
Viewer->>RawGitHub: fetch or reuse cached file content
RawGitHub-->>Viewer: return file content
Viewer->>PreviewIframe: assign rebuilt HTML srcdoc when previewing HTML
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. 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: 3
🧹 Nitpick comments (3)
assets/style.css (1)
17-17: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFont keyword casing flagged by stylelint.
🎨 Fix
- --font: Calibri, "Segoe UI", "Trebuchet MS", sans-serif; + --font: calibri, "Segoe UI", "Trebuchet MS", sans-serif;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@assets/style.css` at line 17, Update the --font declaration to use the casing required by stylelint for the font-family keywords, while preserving the existing fallback order and values.Source: Linters/SAST tools
assets/app.js (2)
66-68: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
stlmissing fromBINARY_EXTS.Line 599's comment explicitly anticipates ".stl meshes" as a large-file case, but
stlisn't in this list. Unlike other binary formats,.stlfiles won't get the "binary file, can't be previewed" fallback — they'll be fetched, decoded as UTF-8, and shown as raw (likely garbled) bytes in the viewer instead.🔧 Fix
- var BINARY_EXTS = ["pdf", "zip", "exe", "bin", "hex", "elf", "o", "a", "dll", - "so", "class", "jar", "mp3", "mp4", "mov", "ttf", "woff", "woff2", "eot"]; + var BINARY_EXTS = ["pdf", "zip", "exe", "bin", "hex", "elf", "o", "a", "dll", + "so", "class", "jar", "mp3", "mp4", "mov", "ttf", "woff", "woff2", "eot", "stl"];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@assets/app.js` around lines 66 - 68, Add "stl" to the BINARY_EXTS list so STL mesh files follow the existing binary-file fallback instead of being fetched and rendered as UTF-8 text. Leave IMAGE_EXTS and the other extension classifications unchanged.
691-701: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winClipboard write has no rejection handling.
If
navigator.clipboard.writeTextrejects (denied permission, insecure context, etc.), nothing happens — no error surfaced, button never gives feedback.🔧 Add a rejection handler
- navigator.clipboard.writeText(getCurrentText()).then(function () { + navigator.clipboard.writeText(getCurrentText()).then(function () { btn.classList.add("copied"); btn.innerHTML = ICONS.check + "<span>Copied</span>"; setTimeout(function () { btn.classList.remove("copied"); btn.innerHTML = ICONS.copy + "<span>Copy</span>"; }, 1500); - }); + }, function () { + btn.innerHTML = "Copy failed"; + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@assets/app.js` around lines 691 - 701, Add rejection handling to the navigator.clipboard.writeText promise in the copyBtn click listener. On failure, surface feedback to the user and ensure the button does not remain in a misleading copied state, while preserving the existing success behavior and timeout in the click handler.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@assets/app.js`:
- Around line 716-720: Update checkedJson so it throws "RATE_LIMIT" for a 403
only when the response’s X-RateLimit-Remaining header indicates the rate limit
is exhausted; otherwise throw the regular HTTP status error. Preserve the
existing handling for non-403 failures and ensure friendlyFetchError can
distinguish other forbidden responses.
- Around line 291-314: Update openFile so the URLs interpolated into the image
src and binary-file href HTML are HTML-escaped before assigning
contentEl.innerHTML. Apply the escaping to rawUrl and blobUrl while preserving
their URL construction and the existing image and binary preview behavior.
In `@index.html`:
- Line 8: Add SRI integrity hashes and crossorigin="anonymous" to every
CodeMirror, marked, and DOMPurify CDN link and script in index.html, using
hashes matching the exact referenced asset versions. Ensure no cdnjs resource
remains without both attributes.
---
Nitpick comments:
In `@assets/app.js`:
- Around line 66-68: Add "stl" to the BINARY_EXTS list so STL mesh files follow
the existing binary-file fallback instead of being fetched and rendered as UTF-8
text. Leave IMAGE_EXTS and the other extension classifications unchanged.
- Around line 691-701: Add rejection handling to the
navigator.clipboard.writeText promise in the copyBtn click listener. On failure,
surface feedback to the user and ensure the button does not remain in a
misleading copied state, while preserving the existing success behavior and
timeout in the click handler.
In `@assets/style.css`:
- Line 17: Update the --font declaration to use the casing required by stylelint
for the font-family keywords, while preserving the existing fallback order and
values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 12caad0b-7cc9-40dd-8234-4d685fcced04
📒 Files selected for processing (3)
assets/app.jsassets/style.cssindex.html
| function openFile(path) { | ||
| var name = path.split("/").pop(); | ||
| var e = ext(name); | ||
| var rawUrl = RAW_BASE + path.split("/").map(encodeURIComponent).join("/"); | ||
|
|
||
| if (IMAGE_EXTS.indexOf(e) !== -1) { | ||
| contentEl.innerHTML = | ||
| '<div class="file-panel">' + | ||
| pathHeaderHtml(path) + | ||
| '<div class="image-preview"><img src="' + rawUrl + '" alt="' + escapeHtml(name) + '"/></div>' + | ||
| "</div>"; | ||
| return; | ||
| } | ||
|
|
||
| if (BINARY_EXTS.indexOf(e) !== -1) { | ||
| var blobUrl = REPO_URL + "/blob/" + BRANCH + "/" + path.split("/").map(encodeURIComponent).join("/"); | ||
| contentEl.innerHTML = | ||
| '<div class="file-panel">' + | ||
| pathHeaderHtml(path) + | ||
| '<div class="binary-notice">This is a binary file and can\'t be previewed here.<br/><a href="' + | ||
| blobUrl + '" target="_blank" rel="noopener">Open on GitHub</a></div>' + | ||
| "</div>"; | ||
| return; | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | sed -n '1,120p'
echo "== locate app.js =="
fd -a 'app\.js$' . || true
echo "== outline app.js if present =="
for f in $(fd 'app\.js$' .); do
echo "--- $f"
wc -l "$f"
ast-grep outline "$f" --view compact 2>/dev/null | sed -n '1,160p' || true
done
echo "== relevant lines == "
f=$(fd 'app\.js$' . | head -n1)
if [ -n "${f:-}" ]; then
sed -n '1,80p;260,370p;700,760p' "$f" | cat -n
fiRepository: upsidedownlabs/BioAmp-Arduino-Firmware-Explorer
Length of output: 12420
🌐 Web query:
GitHub URLs path encoding spaces special characters raw.githubusercontent.com blob URL
💡 Result:
GitHub URLs, including those for raw files (raw.githubusercontent.com) and repository blob paths (github.com/.../blob/...), require proper URL encoding when file paths or names contain spaces or special characters [1][2]. Because these URLs are standard web addresses, they must follow RFC specifications, where spaces and reserved characters are replaced with their percent-encoded equivalents [3][2]. General URL Encoding Rules for GitHub: - Spaces: Replace with %20 [4][3][2]. - Special Characters: Use standard URL percent-encoding (e.g., # becomes %23,? becomes %3F, / becomes %2F) [5][1][2]. - Path Segments: Each component of the path should be individually encoded to avoid unintended encoding of structural separators like slashes [5][6]. In Markdown files on GitHub: - You can link to a file with spaces by using the encoded URL, such as File Name [4][2]. - Alternatively, wrapping the path in angle brackets—e.g., File Name—is often supported by Markdown parsers to handle spaces without explicit manual encoding [3][7]. Common Pitfalls: - Incorrect Encoding: Simply replacing spaces with %20 is often insufficient for characters that have structural meaning in URLs, such as?, #, or /. These must be escaped (e.g.,? as %3F, # as %23) to prevent the browser or server from interpreting them as query parameters or URL fragments [5][8][1]. - Double Encoding: Ensure your tools are not double-encoding paths (e.g., encoding the URL twice), which can result in broken links [9][6]. - API Usage: When interacting with the GitHub REST API, always use proper URL encoding functions (e.g., encodeURIComponent in JavaScript or equivalent libraries in other languages) to construct dynamic paths safely [5][8]. For raw.githubusercontent.com URLs specifically, the path portion behaves like a standard file path, so ensuring the full path is correctly percent-encoded is essential for the browser or external services to retrieve the raw file successfully [10][11][12].
Citations:
- 1: https://github.com/negibokken/github-url-test
- 2: https://www.tutorialpedia.org/blog/github-relative-link-in-markdown-file/
- 3: https://stackoverflow.com/questions/34569256/link-to-filenames-with-spaces-in-bitbucket-markdown
- 4: https://stackoverflow.com/questions/47050129/how-to-link-to-a-file-with-spaces-in-the-filename-in-github-readme-md
- 5: URL-encode parameters in request paths octokit/octokit.js#1355
- 6: Incomplete URL escaping github/gh-gei#772
- 7: Markdown previews should render links with spaces in the name microsoft/vscode#251504
- 8: fix(Github Node): Fix fetch of file names with ? character n8n-io/n8n#12206
- 9: Handle spaces and other special characters in /contents API PyGithub/PyGithub#614
- 10: https://forum.plantuml.net/9426/proxy-service-urlencode-needed
- 11: GLTFLoader: Assets with url-encoded UTF8 characters in filenames don't load correctly mrdoob/three.js#29963
- 12: https://stackoverflow.com/questions/24721575/github-url-to-raw-files
🌐 Web query:
Git ref names naming rules control characters space tilde caret colon allowed characters
💡 Result:
Git enforces strict naming rules for references (refnames), such as branch and tag names, to ensure they can be safely parsed and manipulated by shell-based tools and internal Git commands [1][2]. These rules are formally defined by the git check-ref-format command [1]. The following characters and sequences are explicitly prohibited in any part of a reference name: - ASCII control characters: Any byte with a value lower than \040 (decimal 32) or \177 (DEL) [1][3]. - Whitespace: Space characters are not allowed [1][4]. - Reserved characters: Tilde (~), caret (^), colon (:), question mark (?), asterisk (), open bracket ([), and backslash () [1][2]. - Combinations: The sequence @{ is forbidden [1][2]. - Consecutive dots: The sequence.. is forbidden anywhere in the name [1][3]. - Specific components: Slash-separated components cannot begin with a dot (.) or end with the sequence.lock [1][5]. - Endings: Refnames cannot end with a dot (.) [1][2]. - Structure: By default, refnames must contain at least one forward slash (/) to define a category (e.g., heads/, tags/), unless the --allow-onelevel option is used [1][6]. Names cannot start or end with a slash, nor can they contain multiple consecutive slashes [1][2]. - Special cases: The single character @ is forbidden [1][2]. While the asterisk () is generally forbidden, it is permitted if the --refspec-pattern option is used, in which case a single * may be used as a wildcard in a refspec [1][5]. Note that specific commands, such as git branch, may impose additional, stricter rules beyond those enforced by git check-ref-format, such as prohibiting dashes at the very beginning of a branch name [7].
Citations:
- 1: https://git-scm.com/docs/git-check-ref-format
- 2: https://kernel.googlesource.com/pub/scm/git/git/+/9db41eba4259126dbe1c68e2207d342d11f14745/Documentation/git-check-ref-format.txt
- 3: https://www.baeldung.com/ops/git-illegal-characters-ref-branch-tag
- 4: https://stackoverflow.com/questions/3651860/which-characters-are-illegal-within-a-branch-name
- 5: https://www.kernel.org/pub/software/scm/git/docs/git-check-ref-format.html
- 6: https://www.man7.org/linux/man-pages/man1/git-check-ref-format.1.html
- 7: https://kernel.googlesource.com/pub/scm/git/git/+/HEAD/Documentation/git-check-ref-format.adoc
Escape RAW_BASE and blobUrl before writing them to innerHTML.
branch comes directly from the URL and is embedded in RAW_BASE/REPO_URL; branch names can include <, >, and ", so the current unescaped URL interpolation into image src and <a href> sinks enables reflected XSS when an image or binary file is opened.
🛡️ Fix: escape before interpolating into innerHTML
if (IMAGE_EXTS.indexOf(e) !== -1) {
contentEl.innerHTML =
'<div class="file-panel">' +
pathHeaderHtml(path) +
- '<div class="image-preview"><img src="' + rawUrl + '" alt="' + escapeHtml(name) + '"/></div>' +
+ '<div class="image-preview"><img src="' + escapeHtml(rawUrl) + '" alt="' + escapeHtml(name) + '"/></div>' +
"</div>";
return;
}
if (BINARY_EXTS.indexOf(e) !== -1) {
var blobUrl = REPO_URL + "/blob/" + BRANCH + "/" + path.split("/").map(encodeURIComponent).join("/");
contentEl.innerHTML =
'<div class="file-panel">' +
pathHeaderHtml(path) +
'<div class="binary-notice">This is a binary file and can\'t be previewed here.<br/><a href="' +
- blobUrl + '" target="_blank" rel="noopener">Open on GitHub</a></div>' +
+ escapeHtml(blobUrl) + '" target="_blank" rel="noopener">Open on GitHub</a></div>' +
"</div>";
return;
}📝 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.
| function openFile(path) { | |
| var name = path.split("/").pop(); | |
| var e = ext(name); | |
| var rawUrl = RAW_BASE + path.split("/").map(encodeURIComponent).join("/"); | |
| if (IMAGE_EXTS.indexOf(e) !== -1) { | |
| contentEl.innerHTML = | |
| '<div class="file-panel">' + | |
| pathHeaderHtml(path) + | |
| '<div class="image-preview"><img src="' + rawUrl + '" alt="' + escapeHtml(name) + '"/></div>' + | |
| "</div>"; | |
| return; | |
| } | |
| if (BINARY_EXTS.indexOf(e) !== -1) { | |
| var blobUrl = REPO_URL + "/blob/" + BRANCH + "/" + path.split("/").map(encodeURIComponent).join("/"); | |
| contentEl.innerHTML = | |
| '<div class="file-panel">' + | |
| pathHeaderHtml(path) + | |
| '<div class="binary-notice">This is a binary file and can\'t be previewed here.<br/><a href="' + | |
| blobUrl + '" target="_blank" rel="noopener">Open on GitHub</a></div>' + | |
| "</div>"; | |
| return; | |
| } | |
| function openFile(path) { | |
| var name = path.split("/").pop(); | |
| var e = ext(name); | |
| var rawUrl = RAW_BASE + path.split("/").map(encodeURIComponent).join("/"); | |
| if (IMAGE_EXTS.indexOf(e) !== -1) { | |
| contentEl.innerHTML = | |
| '<div class="file-panel">' + | |
| pathHeaderHtml(path) + | |
| '<div class="image-preview"><img src="' + escapeHtml(rawUrl) + '" alt="' + escapeHtml(name) + '"/></div>' + | |
| "</div>"; | |
| return; | |
| } | |
| if (BINARY_EXTS.indexOf(e) !== -1) { | |
| var blobUrl = REPO_URL + "/blob/" + BRANCH + "/" + path.split("/").map(encodeURIComponent).join("/"); | |
| contentEl.innerHTML = | |
| '<div class="file-panel">' + | |
| pathHeaderHtml(path) + | |
| '<div class="binary-notice">This is a binary file and can\'t be previewed here.<br/><a href="' + | |
| escapeHtml(blobUrl) + '" target="_blank" rel="noopener">Open on GitHub</a></div>' + | |
| "</div>"; | |
| return; | |
| } |
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 296-300: Avoid assigning untrusted data to innerHTML/outerHTML or document.write
Context: contentEl.innerHTML =
'
pathHeaderHtml(path) +
'
"
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(inner-outer-html)
[warning] 306-311: Avoid assigning untrusted data to innerHTML/outerHTML or document.write
Context: contentEl.innerHTML =
'
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(inner-outer-html)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@assets/app.js` around lines 291 - 314, Update openFile so the URLs
interpolated into the image src and binary-file href HTML are HTML-escaped
before assigning contentEl.innerHTML. Apply the escaping to rawUrl and blobUrl
while preserving their URL construction and the existing image and binary
preview behavior.
| function checkedJson(res) { | ||
| if (res.status === 403) throw new Error("RATE_LIMIT"); | ||
| if (!res.ok) throw new Error("HTTP " + res.status); | ||
| return res.json(); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
All HTTP 403s from the GitHub API are labeled as rate-limiting.
checkedJson throws "RATE_LIMIT" for any 403, but GitHub also returns 403 for other reasons (e.g. abuse-detection triggers, SSO-protected orgs). Checking the X-RateLimit-Remaining header would let friendlyFetchError distinguish an actual exhausted rate limit from other forbidden-access cases and avoid showing a misleading message.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@assets/app.js` around lines 716 - 720, Update checkedJson so it throws
"RATE_LIMIT" for a 403 only when the response’s X-RateLimit-Remaining header
indicates the rate limit is exhausted; otherwise throw the regular HTTP status
error. Preserve the existing handling for non-403 failures and ensure
friendlyFetchError can distinguish other forbidden responses.
| <meta name="viewport" content="width=device-width, initial-scale=1.0" /> | ||
| <title>Repository Explorer</title> | ||
| <link rel="stylesheet" href="assets/style.css" /> | ||
| <link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/codemirror/5.65.20/codemirror.min.css" /> |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | sed -n '1,120p'
echo "== index.html relevant external links/scripts =="
if [ -f index.html ]; then
nl -ba index.html | sed -n '1,90p'
fi
echo "== CSP/integrity attributes in tracked HTML =="
python3 - <<'PY'
import pathlib,re
for p in pathlib.Path('.').rglob('*.html'):
if '.git' in p.parts:
continue
txt=p.read_text(errors='replace')
attrs=bool(re.search(r'content-security-policy|integrity\s*=|script-src|style-src', txt, re.I|re.S))
print(p, "csp_or_integrity=", attrs)
PY
echo "== CDN tags across repo =="
rg -n --glob '*.html' 'cdnjs\.cloudflare\.com|unpkg\.com|jsdelivr\.net|integrity\s*=|crossorigin\s*=|content-security-policy|script-src|style-src' .Repository: upsidedownlabs/BioAmp-Arduino-Firmware-Explorer
Length of output: 357
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== index.html lines =="
awk '{printf "%6d %s\n", NR, $0}' index.html | sed -n '1,90p'
echo "== CDN tags and CSP/integrity matches across tracked HTML =="
python3 - <<'PY'
import pathlib,re, subprocess
paths = [p.as_posix() for p in pathlib.Path('.').rglob('*.html') if '.git' not in p.parts]
for p in paths:
txt = pathlib.Path(p).read_text(errors='replace')
print(f"--- {p}")
print("csp_or_subresource_attr_present=", bool(re.search(r'content-security-policy|integrity\s*=|script-src|style-src', txt, re.I|re.S)))
for m in re.finditer(r'<(link|script)[^>]*>\s*', txt, re.I|re.S):
line = txt.count("\n", 0, m.start()) + 1
block = re.sub(r'\s+', ' ', m.group(0)).strip()
cdn = bool(re.search(r'cdnjs\.cloudflare\.com|unpkg\.com|jsdelivr\.net', block))
integrity = bool(re.search(r'\bintegrity\s*=', block, re.I))
crossorigin = bool(re.search(r'\bcrossorigin\s*=', block, re.I))
if cdn:
print(f"line {line} cdn/integrity/crossorigin= {integrity}/{crossorigin}: {block[:220]}")
PYRepository: upsidedownlabs/BioAmp-Arduino-Firmware-Explorer
Length of output: 6872
Add SRI/crossorigin to all CDN resources.
Every CodeMirror, marked, and DOMPurify asset in index.html is loaded from cdnjs.cloudflare.com without integrity hashes or crossorigin, and the page has no CSP. Add integrity and crossorigin="anonymous" to these <link>/<script> tags so compromised or tampered CDN responses cannot execute in the main page origin.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@index.html` at line 8, Add SRI integrity hashes and crossorigin="anonymous"
to every CodeMirror, marked, and DOMPurify CDN link and script in index.html,
using hashes matching the exact referenced asset versions. Ensure no cdnjs
resource remains without both attributes.
1 participant
This enables exploring and testing repositories for any BioAmp-Arduino-Firmwares
Summary by CodeRabbit