security: close 115 of 136 open CodeQL alerts on develop#813
Merged
Conversation
Bundle of fixes for high-severity static analysis findings: - communities/create: replace Math.random()-derived `random()` with crypto.getRandomValues for the community owner WIF seed (#136 — was 32 bits of entropy from a non-cryptographic PRNG) - api/internal/seo/{sitemap-generate,blacklist-refresh}: stop returning raw error messages in 500 bodies; log server-side instead (#96, #97) - post-renderer twitter enhancer + extension: switch substring matching on https://twitter.com to hostname-equality checks via URL parsing (#98, #99, #101, #102) - render-helper: - sanitize-html: drop redundant javascript: blocklist alongside the positive http(s) whitelist; add /i to srcset scheme check (#103-114) - a.method: tighten dangerous-scheme strip to cover data:/vbscript:/file: in addition to javascript: (#112) - linkify: replace .replace('>', '') with anchored /^>/ (#95) - consts/regexes: escape unescaped `.` in hostname regexes for youtube/twitch/loom/rumble/bitchute/soundcloud/vimm (#122) - bulk: convert `.replace("%40", X)` to `/%40/g` form across profile, entry, wallet, publish, waves, perks pages (~59 incomplete-sanitization alerts) - tiptap-editor hive-post-extension: rewrite ReDoS-prone regex with per-segment `[^@\s/]+` so partition is unambiguous and matching is linear (#16) - mattermost server + channels posts route: stop interpolating user-provided values into the console.error format string (#24, #25) - publish-editor toolbar-fragments: add /i flag to HTML-tag regex so it matches uppercase <SCRIPT> (#26) - get-pure-post-text + publish-validate-post + self-hosted blog-post: loop HTML-tag strip until idempotent so payloads like `<scr<script>ipt>` can't leak a fragment past a single-pass strip (#27-30) - packages/render-helper/dist: rebuilt artifacts pick up source-level fixes (closes #91-95, #103-114, #116-121) Out of scope (follow-up): - #23 SSRF in api/import/route.ts: already mitigated by DNS-resolution IP allowlist + per-hop redirect validation + size/timeout caps; residual TOCTOU window is a documented trade-off (lines 109-114). Tightening requires pinning fetch() to the resolved IP via a custom undici dispatcher — separate PR. - 15× `actions/missing-workflow-permissions`: workflow files need explicit `permissions:` blocks — separate PR.
- selection-popover: encodeURIComponent the tweet-intent URL params so selected post text can't break the share URL (#17) - api/import/route.ts: strip script/style/iframe/event-handlers before setInnerHTML on JSDOM-parsed noscript content; validate lazy/srcset URLs against an http(s)/relative whitelist before promoting to <img src> (#18, #19) — JSDOM doesn't execute scripts, but pre-strip is defense-in-depth - hive-operation-extension: validate the decoded Hive `to` field as a Hive username before rendering it into avatar URLs (#20) - video-upload-threespeak #21: NOT modified — `<source src>` from URL.createObjectURL is a same-origin blob, false positive - wave-follows-card + communities/create: source Math.random with crypto.getRandomValues for leaderboard-pick and username-suffix — closes downstream taint alerts on community-list-item and wave-follows-card sinks (#131-135) - self-hosted floating-menu/utils: reject path segments containing __proto__/constructor/prototype before recursively writing nested config — closes prototype-pollution-utility (#22)
CodeQL's hostname-regex analysis traced the proxyBase string literal (`https://i.ecency.com`) through setProxyBase into a `new RegExp(`^${escapedBase}/p/...`)` call in buildSrcSet. The escape via .replace(/[.*+?^${}()|[\]\\]/g,...) at the use site is correct, but CodeQL was flagging every spec call to setProxyBase plus the source-file literal. Replace the regex match with plain string ops: const proxyPrefix = `${proxyBase}/p/`; if (url.startsWith(proxyPrefix)) { … } Functionally equivalent (the regex only checked startsWith + extracted the segment before the query string, both trivial to do with slice/ indexOf). Spec suite stays green (19 tests in proxify-image-src.spec.ts, all 1072 across render-helper). Dist rebuilt.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f06b82962a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Only http(s) and protocol-/path-relative URLs are accepted as image | ||
| // src values. Blocks javascript:/data:/vbscript: schemes that could | ||
| // otherwise be smuggled through lazy-load attributes on adversarial pages. | ||
| const SAFE_IMG_SRC_RE = /^(https?:\/\/|\/\/|\/[^/])/i; |
There was a problem hiding this comment.
SAFE_IMG_SRC_RE now rejects valid document-relative URLs such as "./img.jpg", "../img.jpg", and "img.jpg". In fixLazyImages, this means data-src/srcset candidates in those forms are skipped whenever src is missing or a data placeholder, so imported articles from sites that lazy-load with relative paths will lose images even though JSDOM/Readability can resolve them against the page URL.
Useful? React with 👍 / 👎.
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (6)
📒 Files selected for processing (13)
📝 WalkthroughWalkthroughNormalize URL-encoded ChangesURL parameter encoding normalization and routing fixes
Security hardening across multiple vectors
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
apps/web/src/app/perks/promote-post/_components/promote-post-setup.tsx (1)
74-74: ⚡ Quick winConsider making the
@removal global for consistency.The first
.replace("@", "")only removes the first occurrence of@, while the second.replace(/%40/g, "")removes all occurrences of%40. If a user types multiple@symbols (e.g.,@@username), only the first would be removed. Making both replacements global would be more consistent and handle edge cases better.♻️ Proposed refinement
- onChange={(e) => setPathQuery(e.target.value.replace("@", "").replace(/%40/g, ""))} + onChange={(e) => setPathQuery(e.target.value.replace(/@/g, "").replace(/%40/g, ""))}🤖 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 `@apps/web/src/app/perks/promote-post/_components/promote-post-setup.tsx` at line 74, The onChange handler for setPathQuery only removes the first "@" because it uses .replace("@", "") while it removes all "%40" with a global regex; update the handler in promote-post-setup.tsx to remove all "@" occurrences by using a global regex (e.g., replace with /@/g) before also removing all "%40" (keep /%40/g), ensuring setPathQuery(e.target.value.replace(/@/g, "").replace(/%40/g, "")) handles multiple "@" reliably.
🤖 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
`@apps/web/src/app/`(dynamicPages)/profile/[username]/wallet/(token)/_components/profile-wallet-token-actions.tsx:
- Line 44: The code currently force-casts username to string for cleanUsername
which can throw if useParams() returned undefined; replace the unsafe assertion
by guarding the params value first (check that username is a non-empty string
via typeof/boolean check or params?.username) before calling .replace(), and
either provide a safe fallback (e.g. empty string) or return early from the
component when username is missing; update the references to cleanUsername so
they rely on the guarded value (symbols: useParams, cleanUsername in
profile-wallet-token-actions.tsx) to avoid runtime errors in downstream queries
and comparisons.
In
`@apps/web/src/app/`(dynamicPages)/profile/[username]/wallet/(token)/_components/profile-wallet-token-summary.tsx:
- Line 42: The code uses (username as string).replace(...) without runtime
checks which will throw if username is undefined; change to guard and normalize
username before calling replace (e.g., derive a rawUsername via typeof username
=== "string" ? username : "" or username ?? ""), then compute cleanUsername from
that normalized string, and use cleanUsername in the query hooks (the places
using cleanUsername and the original username value) so replace is never invoked
on undefined; update any other occurrences (e.g., where cleanUsername is
referenced) to use this safely normalized variable.
In
`@apps/web/src/app/`(dynamicPages)/profile/[username]/wallet/(token)/[token]/_components/hive-engine-token-history.tsx:
- Line 26: The code uses a blind cast for the route param when creating
cleanUsername: replace the unsafe assertion by guarding the username parameter
(the username variable used to compute cleanUsername in
hive-engine-token-history.tsx) — check typeof username === "string" and either
early-return null/placeholder or handle the undefined case before computing
cleanUsername, then call username.replace(/%40/g, ""); update any callers/JSX to
expect the early return if you choose to return null.
In `@apps/web/src/app/api/import/route.ts`:
- Around line 273-274: SAFE_IMG_SRC_RE currently permits protocol-relative
("//...") and root-relative ("/...") URLs but sanitizeHtml() later only accepts
http(s)://, causing recovered images to be dropped; update the import flow to
normalize candidate image URLs to absolute http(s) before assignment: when you
detect a match via SAFE_IMG_SRC_RE (and at the other spots around 309-315,
328-330), resolve the URL against document.baseURI (or use new URL(candidate,
document.baseURI).href) and then validate that the resolved value starts with
"http://" or "https://"; assign that resolved absolute URL as the img src (or
alternatively narrow SAFE_IMG_SRC_RE to only allow ^https?:\/\/), and ensure
sanitizeHtml() sees the normalized http(s) URL so images are preserved.
In
`@apps/web/src/features/post-renderer/components/extensions/twitter-extension.tsx`:
- Around line 28-37: The hostname check for Twitter/X embeds rejects common www
prefixed links; update the validation where the URL is parsed (the URL(href)
block that returns u.protocol === "https:" && (u.hostname === "x.com" ||
u.hostname === "twitter.com")) to also accept "www.twitter.com" and "www.x.com"
(i.e., include u.hostname === "www.twitter.com" and u.hostname === "www.x.com")
so https://www.twitter.com/... and https://www.x.com/... are treated as valid
embeds.
In `@apps/web/src/features/post-renderer/components/utils/twitterEnhancer.tsx`:
- Around line 19-23: The hostname check in twitterEnhancer.tsx currently only
allows "x.com" and "twitter.com" and falsely rejects common "www." variants;
update the logic that inspects the URL (the const url = new URL(href) block and
the subsequent hostname checks) to also accept "www.twitter.com" and "www.x.com"
in addition to the existing hostnames so https://www.twitter.com/... and
https://www.x.com/... are treated as valid Twitter/X URLs; ensure the protocol
check (url.protocol !== "https:") remains, and adjust the conditional that
references url.hostname to include the two "www." variants.
---
Nitpick comments:
In `@apps/web/src/app/perks/promote-post/_components/promote-post-setup.tsx`:
- Line 74: The onChange handler for setPathQuery only removes the first "@"
because it uses .replace("@", "") while it removes all "%40" with a global
regex; update the handler in promote-post-setup.tsx to remove all "@"
occurrences by using a global regex (e.g., replace with /@/g) before also
removing all "%40" (keep /%40/g), ensuring
setPathQuery(e.target.value.replace(/@/g, "").replace(/%40/g, "")) handles
multiple "@" reliably.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1e627d0c-2e91-421f-9b79-fdff173d81f1
⛔ Files ignored due to path filters (6)
packages/render-helper/dist/browser/index.jsis excluded by!**/dist/**packages/render-helper/dist/browser/index.js.mapis excluded by!**/dist/**,!**/*.mappackages/render-helper/dist/node/index.cjsis excluded by!**/dist/**packages/render-helper/dist/node/index.cjs.mapis excluded by!**/dist/**,!**/*.mappackages/render-helper/dist/node/index.mjsis excluded by!**/dist/**packages/render-helper/dist/node/index.mjs.mapis excluded by!**/dist/**,!**/*.map
📒 Files selected for processing (61)
apps/self-hosted/src/features/blog/components/blog-post-page.tsxapps/self-hosted/src/features/floating-menu/utils.tsapps/web/src/api/queries/get-account-posts-feed-query.tsapps/web/src/app/(dynamicPages)/entry/[category]/[author]/(layoutless)/[permlink]/edit-classic/page.tsxapps/web/src/app/(dynamicPages)/entry/[category]/[author]/[permlink]/_components/selection-popover/index.tsxapps/web/src/app/(dynamicPages)/entry/[category]/[author]/[permlink]/page.tsxapps/web/src/app/(dynamicPages)/entry/[category]/[author]/[permlink]/redditbot/route.tsapps/web/src/app/(dynamicPages)/entry/_helpers/generate-entry-metadata.tsapps/web/src/app/(dynamicPages)/feed/[...sections]/_helpers/generate-feed-metadata.tsapps/web/src/app/(dynamicPages)/profile/[username]/[section]/page.tsxapps/web/src/app/(dynamicPages)/profile/[username]/communities/_page.tsxapps/web/src/app/(dynamicPages)/profile/[username]/communities/page.tsxapps/web/src/app/(dynamicPages)/profile/[username]/insights/page.tsxapps/web/src/app/(dynamicPages)/profile/[username]/layout.tsxapps/web/src/app/(dynamicPages)/profile/[username]/page.tsxapps/web/src/app/(dynamicPages)/profile/[username]/permissions/_components/profile-permissions.tsxapps/web/src/app/(dynamicPages)/profile/[username]/permissions/page.tsxapps/web/src/app/(dynamicPages)/profile/[username]/referrals/page.tsxapps/web/src/app/(dynamicPages)/profile/[username]/rss.xml/route.tsxapps/web/src/app/(dynamicPages)/profile/[username]/rss/route.tsxapps/web/src/app/(dynamicPages)/profile/[username]/settings/page.tsxapps/web/src/app/(dynamicPages)/profile/[username]/trail/page.tsxapps/web/src/app/(dynamicPages)/profile/[username]/wallet/(token)/[token]/_components/hive-engine-claim-rewards-button.tsxapps/web/src/app/(dynamicPages)/profile/[username]/wallet/(token)/[token]/_components/hive-engine-token-history.tsxapps/web/src/app/(dynamicPages)/profile/[username]/wallet/(token)/[token]/page.tsxapps/web/src/app/(dynamicPages)/profile/[username]/wallet/(token)/_components/profile-wallet-token-actions.tsxapps/web/src/app/(dynamicPages)/profile/[username]/wallet/(token)/_components/profile-wallet-token-summary.tsxapps/web/src/app/(dynamicPages)/profile/[username]/wallet/(token)/hbd/page.tsxapps/web/src/app/(dynamicPages)/profile/[username]/wallet/(token)/hive/page.tsxapps/web/src/app/(dynamicPages)/profile/[username]/wallet/(token)/hp/page.tsxapps/web/src/app/(dynamicPages)/profile/[username]/wallet/(token)/points/_page.tsxapps/web/src/app/(dynamicPages)/profile/[username]/wallet/(token)/points/page.tsxapps/web/src/app/(dynamicPages)/profile/[username]/wallet/_components/profile-wallet-collect-all-button.tsxapps/web/src/app/(dynamicPages)/profile/[username]/wallet/_components/profile-wallet-external-banner.tsxapps/web/src/app/(dynamicPages)/profile/[username]/wallet/_components/profile-wallet-pending-earnings.tsxapps/web/src/app/(dynamicPages)/profile/[username]/wallet/_components/profile-wallet-summary.tsxapps/web/src/app/(dynamicPages)/profile/[username]/wallet/page.tsxapps/web/src/app/api/import/route.tsapps/web/src/app/api/internal/seo/blacklist-refresh/route.tsapps/web/src/app/api/internal/seo/sitemap-generate/route.tsapps/web/src/app/api/mattermost/channels/[channelId]/posts/route.tsapps/web/src/app/communities/create/_page.tsxapps/web/src/app/perks/promote-post/_components/promote-post-setup.tsxapps/web/src/app/publish/_components/publish-editor-toolbar-fragments.tsxapps/web/src/app/publish/_components/publish-validate-post.tsxapps/web/src/app/publish/entry/[author]/[permlink]/_components/publish-entry-action-bar.tsxapps/web/src/app/publish/entry/[author]/[permlink]/_components/publish-entry-success-state.tsxapps/web/src/app/publish/entry/[author]/[permlink]/_page.tsxapps/web/src/app/waves/[author]/[permlink]/page.tsxapps/web/src/app/waves/_components/wave-follows-card.tsxapps/web/src/features/post-renderer/components/extensions/hive-operation-extension.tsxapps/web/src/features/post-renderer/components/extensions/twitter-extension.tsxapps/web/src/features/post-renderer/components/utils/twitterEnhancer.tsxapps/web/src/features/tiptap-editor/extensions/hive-post-extension.tsxapps/web/src/server/mattermost.tsapps/web/src/utils/get-pure-post-text.tspackages/render-helper/src/consts/regexes.const.tspackages/render-helper/src/methods/a.method.tspackages/render-helper/src/methods/linkify.method.tspackages/render-helper/src/methods/sanitize-html.method.tspackages/render-helper/src/proxify-image-src.ts
Greptile SummaryThis PR batch-closes 115 of 136 open CodeQL alerts across the
Confidence Score: 5/5Safe to merge; all 115 targeted CodeQL alerts are addressed with well-reasoned fixes and no regressions on the 1,072-test render-helper suite. The cryptographic key generation (generateWif) is the highest-stakes change and is correctly implemented using a full 256-bit CSPRNG seed. The scheme whitelist in a.method.ts is a strict improvement over the previous javascript:-only blocklist. The noscript image recovery replaces an innerHTML sink with an attribute-whitelist copy, and URL validation is correctly applied via normalizeImgSrc. No changes introduce wrong data, broken state, or exploitable paths; the two observations flagged are minor precision gaps with no reachable exploits on real Hive blockchain data. apps/web/src/app/api/import/route.ts and apps/web/src/features/post-renderer/components/extensions/hive-operation-extension.tsx have the minor gaps noted in inline comments, but neither presents a real blocker. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["Hive post body / external article URL"] --> B["render-helper pipeline"]
B --> C{"a.method.ts\nisSafeScheme?\nisRelative?"}
C -->|"yes"| D["href kept"]
C -->|"no"| E["href stripped"]
A --> F["api/import/route.ts\nfetchExternalArticle"]
F --> G["JSDOM parse\n(url option set)"]
G --> H{"DOMParser\navailable?"}
H -->|"yes"| I["noscript → DOMParser\nattr-whitelist copy\nnormalizeImgSrc validate"]
H -->|"no"| J["noscript skipped\n(silent)"]
I --> K["Readability extract"]
J --> K
L["HiveOperationRenderer\nop prop (base64 JSON)"] --> M["atob + JSON.parse"]
M --> N{"decodedOp[0]\n=== transfer?"}
N -->|"yes"| O{"HIVE_USERNAME_RE\ntest(to)?"}
O -->|"pass"| P["avatar URL + display"]
O -->|"fail"| Q["transfer block hidden"]
N -->|"no"| R["no transfer UI"]
S["generateWif\ncommunities/create"] --> T["crypto.getRandomValues\n32-byte seed"]
T --> U["sha256 → base58 → WIF"]
Reviews (3): Last reviewed commit: "chore: apply changeset versioning for PR..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Round 2 of fixes responding to automated PR review feedback. CodeQL (new alerts triggered by round 1): - get-pure-post-text + publish-validate-post + blog-post-page: after loop tag strip, also strip residual `<`/`>` so unclosed/truncated tags (e.g. ending `…<script` with no closing `>`) can't leave a `<script` or `<!--` substring in the output. Closes #141, #142. - api/import noscript path: replaced wrapper.innerHTML + regex strip with DOMParser.parseFromString (no script execution) + whitelisted-attribute migration of <img> elements only. Eliminates the innerHTML sink entirely and removes the need for the regex event-handler strip (which greptile flagged for missing /onerror-style slash-separated attributes). Closes #139, #140. Codex P2 + coderabbit #4: SAFE_IMG_SRC_RE accepted protocol-relative and root-relative URLs but rejected document-relative ones, and sanitize-html downstream only keeps http(s)://. Replaced with normalizeImgSrc(candidate, document.baseURI) that resolves every form (absolute, //protocol, /root, ./doc, plain.jpg) against the article's base URL and requires the absolute result be http(s). Recovered lazy images now survive the render-helper pass. Greptile #2: a.method href check inverted from blocklist (javascript:/data:/vbscript:/file:) to positive whitelist (https?/mailto/hive/tel/web+ext + relative forms). Less-common schemes (ms-its:, mk:, res:, intent:, etc.) now also rejected. Control-char strip widened to \f \v \0 in addition to \t \n \r. CodeRabbit: - twitter-extension + twitterEnhancer: also accept www.twitter.com and www.x.com (commonly shared with the www prefix). - profile-wallet-token-actions, profile-wallet-token-summary (×2), hive-engine-token-history: replaced (username as string) force-cast with typeof guard so an undefined param returns "" instead of throwing. - promote-post-setup: `.replace("@", "")` → `.replace(/@/g, "")` for consistency with the adjacent /%40/g. Skipped: - coderabbit autofix bot — not used. - video-upload-threespeak #21 (URL.createObjectURL blob) — same false positive call as before, dismiss in CodeQL UI. Tests: render-helper 1,072/1,072 green (covers the new a.method whitelist via 112 a.method.spec tests). Typecheck shows no new errors on touched files; the pre-existing TS errors in promote-post-setup, publish-validate-post and token-picker are unrelated and were present before this PR.
Round 3 — addresses the only signals from bots that re-reviewed the round-2 commit: CodeQL re-scan (#143, #144) — get-pure-post-text.ts: The `<[^>]+>` and `<!--…-->` patterns were judged incomplete because they don't match unclosed forms (an input ending mid-tag like `…<script` survives). My round-2 `replace(/[<>]/g, "")` cleanup satisfied correctness but the CodeQL rule judges each regex locally and doesn't trace forward. Switch to `(?:>|$)` / `(?:-->|$)` alternations so the regex itself fully strips unclosed tags on the same pass. Same fix applied to publish-validate-post and self-hosted blog-post-page for consistency. Greptile round-2 (5/5 confidence) — a.method.ts: My `isRelative` regex required `\/[^/]` (at least one non-`/` after the slash), so a bare `href="/"` (root link to the site root) failed both isSafeScheme and isRelative and had its href stripped. Loosened to `\/[^/]?` so bare `/` is accepted alongside `/path…`. Render-helper test suite: 1,072 / 1,072 passing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Bundle of fixes for the open CodeQL findings on
develop. Closes 115 of 136 (~85%); remaining 21 are tracked at the bottom for follow-up PRs.Highlights
generateWif(communities/create) — replacedMath.random()-derived seed withcrypto.getRandomValues. The previous form gave ~32 bits of entropy for a Hive owner key.sitemap-generate,blacklist-refresh) — stopped returning rawe.messagein 500 bodies; details now logged server-side.startsWith("https://twitter.com")→new URL(href).hostname === "twitter.com"(would otherwise accepthttps://twitter.com.evil.com).a.method(data:/vbscript:/file: in addition to javascript:); regex.properly escaped on hostname matchers..replace("%40", X)→/%40/gacross 38 profile/entry/wallet/publish/waves/perks files. Closes ~59 incomplete-sanitization alerts.hive-post-extension) — rewrote[^@\s]+to[^@\s/]+so each path segment can't overlap, partition is unambiguous, matching linear.mattermost.ts,channels/.../posts/route.ts) — user-controlled values no longer interpolated intoconsole.errorfirst-arg.publish-editor-toolbar-fragments) — added/iso<SCRIPT>is matched too.get-pure-post-text,publish-validate-post, self-hosted blog-post) — loop strip to idempotency so adversarial payloads like<scr<script>ipt>can't leak a fragment.selection-popovertweet URL nowencodeURIComponent'd;api/importnoscriptinnerHTMLpre-strips script/style/iframe/event-handlers as defense-in-depth and validates lazy/srcset URLs against an http(s)/relative whitelist;hive-operation-extensionvalidates the decodedtofield as a Hive username before rendering into avatar URL.wave-follows-card,communities/create:generateUsername) — switched leaderboard-pick and username-suffix tocrypto.getRandomValues. Closes 5 downstream taint alerts on community-list-item + wave-follows-card sinks.floating-menu/utils) — reject path segments containing__proto__/constructor/prototypebefore recursive writes.render-helper/proxify-image-src.ts:buildSrcSet) — replacednew RegExp(^${escapedBase}/p/...)with plainstartsWith+slice. Closes 9 hostname-regex alerts (7 in spec file + source + self-hosted call site) without touching behavior.Functional regression review
Only one visible behavior change:
a.methodnow stripsdata:/vbscript:/file:hrefs (was onlyjavascript:). OWASP-aligned, but post bodies containing<a href="data:application/pdf;base64,...">PDF</a>-style links lose the href (link text remains). Practically rare in Hive content.Everything else is internally equivalent:
generateWifCSPRNGmessagetwitter.com.evil.com; real twitter.com/x.com still match\..replace(/%40/g, ...)%40— Hive usernames have ≤1linkify/^>//iflag<SCRIPT>tooconsole.errorstructure[^@\s/]+startsWith/sliceTest results
pnpm --filter @ecency/render-helper test: 1,072 / 1,072 passing. Typecheck shows no new errors on touched files; pre-existing Next.js 15params: Promiseandu-flag regex errors are unrelated.Out of scope — follow-up PRs
api/import/route.ts: already mitigated by DNS-IP allowlist + per-hop redirect validation + size/timeout caps; residual TOCTOU window is documented (lines 109–114). Tightening needsfetch()pinned to the resolved IP via a custom undici dispatcher — separate PR.actions/missing-workflow-permissions: workflow files need explicitpermissions:blocks — separate PR.video-upload-threespeak<source src={URL.createObjectURL(file)}>: local blob URL, false positive — dismiss in CodeQL UI.youtubeVideosEnhancer.spec.tsURL match: test mock, false positive — dismiss in CodeQL UI.Test plan
hive-NNNNNNusername and a WIF (now CSPRNG-sourced)@userURL-encoded as%40user) still resolve via the new.replace(/%40/g, ...)Summary by CodeRabbit
Bug Fixes
Security Improvements