From e4ffd7406800da059b2d758c83bf967bf5b64483 Mon Sep 17 00:00:00 2001 From: Thomas Durieux <5577568+tdurieux@users.noreply.github.com> Date: Thu, 18 Jun 2026 04:50:55 -0700 Subject: [PATCH] Security hardening + gist UI fixes (#731) * security: harden against XSS, ReDoS, path traversal, and injection Defensive fixes across the server, storage, and viewer: - XSS (CWE-79): sanitise rendered notebooks with DOMPurify, escape file names interpolated into AngularJS expressions (escapeNgString), set Mermaid securityLevel to 'strict', and stop urlRel2abs from returning javascript:/vbscript:/data:text/html URLs. - Path traversal / zip-slip (CWE-22/23/24): validate URL-derived path components before they reach the storage layer (file/webview routes + StorageBase.assertSafePath) and sanitise zip entry names on extract for both the filesystem and S3 backends. - ReDoS (CWE-1333): escape anonymization terms with catastrophic backtracking shapes to literals instead of compiling them as regexes. - Secret hardening (CWE-798): require SESSION_SECRET / OAuth creds / DB password in production, random dev SESSION_SECRET fallback. - Rate-limit spoofing (CWE-290): derive request.ip via trust-proxy hop count instead of the client-settable cf-connecting-ip header. - NoSQL injection (CWE-943): allow only plain field paths as admin sort keys. - Reject malformed streamer requests missing required string fields. Co-Authored-By: Claude Opus 4.8 * fix(ui): make gists reachable/visible and clarify the ZIP button - Gist & PR routes now accept a trailing slash (/gist/:id/:path*?), so the dashboard links (which end in "/") resolve to the gist/PR page instead of falling through to the 404 route (#725). - Gist viewer picks the default tab after content loads, defaulting to "files" when files exist; previously the ng-init ran before the async load and a files-only gist rendered blank under the hidden comments tab. - Explorer toolbar: relabel ZIP to "Full repo ZIP" with a tooltip, and add tooltips to Raw/Download clarifying they apply to the current file (#721). Co-Authored-By: Claude Opus 4.8 * fix: report SAML-enforced orgs clearly instead of "token expired" When a repo's organization enforces SAML SSO, GitHub returns a 403 whose message differs from the OAuth-App-restriction case. That 403 fell through to the generic handler and surfaced as "token_expired", pushing users to re-login when the real fix is authorizing their token for the org. Detect the "SAML enforcement" message and raise a dedicated, actionable error instead (#379, #550). Co-Authored-By: Claude Opus 4.8 * security: catch nested quantified groups in ReDoS guard and backslash path traversal - hasCatastrophicBacktracking now scans across nested parens ([\s\S]*?) so shapes like ((a+))+ are detected; comment reframed as a heuristic backstop rather than a proof. - file route path-traversal check now rejects backslash separators and a leading backslash, covering Windows-style "..\" payloads (CWE-22/25). Co-Authored-By: Claude Opus 4.8 * chore(dev): track dev-proxy script, ignore .DS_Store and .claude/ scripts/dev-proxy.js is referenced by the "dev:ui" npm script but was never committed, breaking the command on a fresh clone. Add it and ignore local-only macOS/Claude Code files. Co-Authored-By: Claude Opus 4.8 --------- Co-authored-by: Claude Opus 4.8 --- .gitignore | 6 + public/asset-manifest.json | 4 +- public/i18n/locale-en.json | 2 + public/partials/explorer.htm | 11 +- public/script/app.js | 40 ++++- public/script/core.min.js | 2 +- public/script/external/marked-mermaid.js | 7 +- public/script/utils.js | 13 +- public/script/vendor.min.js | 2 +- scripts/dev-proxy.js | 186 +++++++++++++++++++++++ src/config.ts | 44 +++++- src/core/anonymize-utils.ts | 30 ++++ src/core/source/GitHubRepository.ts | 18 +++ src/core/storage/FileSystem.ts | 16 +- src/core/storage/S3.ts | 17 ++- src/core/storage/Storage.ts | 53 +++++++ src/server/index.ts | 12 +- src/server/routes/admin.ts | 11 +- src/server/routes/file.ts | 17 +++ src/server/routes/webview.ts | 9 ++ src/streamer/route.ts | 7 + 21 files changed, 484 insertions(+), 23 deletions(-) create mode 100644 scripts/dev-proxy.js diff --git a/.gitignore b/.gitignore index 038e279..b45f41f 100644 --- a/.gitignore +++ b/.gitignore @@ -4,6 +4,12 @@ build repo/ db_backups message.txt + +# macOS +.DS_Store + +# Local Claude Code settings +.claude/ # Created by https://www.gitignore.io/api/node # Edit at https://www.gitignore.io/?templates=node diff --git a/public/asset-manifest.json b/public/asset-manifest.json index 777e8ec..b0a3ce9 100644 --- a/public/asset-manifest.json +++ b/public/asset-manifest.json @@ -1,6 +1,6 @@ { - "core.min.js": "core.3db744fc07.min.js", - "vendor.min.js": "vendor.09f02f70c0.min.js", + "core.min.js": "core.6332b3c288.min.js", + "vendor.min.js": "vendor.d7d972f465.min.js", "mermaid.min.js": "mermaid.f848a72d16.min.js", "all.min.css": "all.1a9babcb45.min.css" } \ No newline at end of file diff --git a/public/i18n/locale-en.json b/public/i18n/locale-en.json index 8041325..1ff3bf1 100644 --- a/public/i18n/locale-en.json +++ b/public/i18n/locale-en.json @@ -10,6 +10,7 @@ "user_not_found": "The requested user could not be found.", "user_banned": "Your account has been banned. Contact the admin for more information.", "repo_access_limited": "GitHub blocked access because the repository's organization restricts third-party OAuth apps. Ask an org owner to approve Anonymous GitHub under Settings → Third-party Access → OAuth app policy, or anonymize a personal fork instead.", + "repo_saml_enforcement": "The repository's organization enforces SAML single sign-on. Authorize your token for that organization (GitHub → Settings → Applications, or re-run the org's SSO sign-in), then retry. Alternatively, anonymize a personal fork.", "repo_not_found": "The repository was not found on GitHub. Check the URL and spelling, make sure you are signed in to the account that can see it, and confirm the repo isn't hidden under an org that restricts third-party app access.", "repo_empty": "The selected branch has no commits on GitHub. Push at least one commit, or pick a different branch, then retry.", "repo_not_accessible": "Anonymous GitHub cannot access this repository. Verify the repository exists and that Anonymous GitHub has been authorized for the owning organization.", @@ -52,6 +53,7 @@ "path_not_specified": "A file path must be specified.", "path_not_defined": "The file path has not been resolved yet.", "invalid_file_path": "The requested file path is not valid.", + "invalid_request": "The request is missing required fields or is malformed.", "no_file_selected": "Please select a file.", "file_not_found": "The requested file is not found.", "file_not_accessible": "The requested file is not accessible.", diff --git a/public/partials/explorer.htm b/public/partials/explorer.htm index 9947490..1aabcde 100644 --- a/public/partials/explorer.htm +++ b/public/partials/explorer.htm @@ -88,7 +88,8 @@ ng-href="{{url}}" target="__self" class="btn btn-sm" - aria-label="Raw" + aria-label="View raw current file" + title="View the raw content of the current file" > Raw Download ZIP Full repo ZIP /g, ">").replace(/"/g, """); } + // Escape a value for safe interpolation into a single-quoted + // AngularJS expression string (e.g. ng-click="openFolder('...')") + // that itself sits inside a double-quoted HTML attribute which is + // later $compile()d. Backslash/quote are escaped at the Angular + // string level; &<>" are HTML-encoded for the attribute. Without + // this a file name like `');$emit(...)//` would break out of the + // expression string and execute (DOM XSS, CWE-79). + function escapeNgString(str) { + return String(str) + .replace(/\\/g, "\\\\") + .replace(/'/g, "\\'") + .replace(/&/g, "&") + .replace(//g, ">") + .replace(/"/g, """); + } + function buildSearchFilter() { const results = $scope.searchResults; if (!results || !results.length) return null; @@ -675,11 +692,12 @@ angular cssClasses.push("truncated"); } + const ngPath = escapeNgString(path); output += `
  • `; + )}" ng-class="{active: isActive('${ngPath}'), open: ${filterSet ? "opens['" + ngPath + "'] !== false" : "opens['" + ngPath + "']"}}" title="${escapeHtml(sizeTitle)}">`; if (dir) { - output += `${escapeHtml(name)}`; + output += `${escapeHtml(name)}`; if (truncated) { output += ``; } @@ -911,7 +929,13 @@ angular const notebook = nb.parse(json); try { $element.html(""); - $element.append(notebook.render()); + // notebook.render() turns notebook JSON (markdown cells, cell + // outputs) into HTML without sanitising it — a malicious + // notebook could embed