Add i18n check, URL edge-case coverage, and lint fixes#69
Add i18n check, URL edge-case coverage, and lint fixes#69CodewithEvilxd wants to merge 2 commits intomrmps:mainfrom
Conversation
|
@CodewithEvilxd is attempting to deploy a commit to the Explainer Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughAdds an i18n shape-check script and package script, expands URL private-IP detection to handle IPv4-mapped IPv6 forms with new tests, inserts ESLint disables for img elements, adjusts some tests to accept 401, and renames an unused request field in a gravity route. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 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 |
Greptile OverviewGreptile SummaryAdded i18n validation tooling and hardened SSRF protections with comprehensive test coverage.
Confidence Score: 5/5
Important Files Changed
Last reviewed commit: 02a7adc |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
server/routes/gravity.ts (1)
153-159:⚠️ Potential issue | 🟡 MinorStale comment:
_adProvideris not actually logged to ClickHouse.Line 158 states "adProvider from client is used only for ClickHouse logging", but
_adProvideris never passed totrackAdEvent(lines 172–192). The comment is misleading — either remove it or, if the intent is to log it, actually pass it as a field.Suggested comment fix
const { type, sessionId, hostname, brandName, adTitle, adText, clickUrl, impUrl, cta, favicon, deviceType, os, browser, adProvider: _adProvider } = body; // Derive provider from impUrl prefix only — never trust client-sent adProvider // for forwarding decisions (prevents spoofing to skip Gravity billing) const isZeroClick = impUrl?.startsWith("zeroclick://") ?? false; - // adProvider from client is used only for ClickHouse logging, not forwarding logic + // adProvider from client is accepted but intentionally unused const provider = isZeroClick ? "zeroclick" : "gravity";app/api/og/route.tsx (1)
124-142:⚠️ Potential issue | 🟡 MinorESLint suppression is correct —
next/imageis unsupported inImageResponse(Satori).However, there's an unaddressed SSRF vulnerability:
articleImageoriginates from unvalidated user input (?image=...query parameter or fetched article metadata) and Satori will fetch it server-side during image generation. An attacker could pass?image=http://169.254.169.254/latest/meta-data/or other private IP addresses. Apply the existingisPrivateIP()validation fromlib/validation/url.tstoarticleImagebefore rendering to prevent this attack vector.
🤖 Fix all issues with AI agents
In `@lib/validation/url.test.ts`:
- Around line 80-89: Add unit tests to cover the new IPv4-mapped hex-hextet and
expanded IPv6 parsing branches that currently lack coverage: add tests invoking
normalizeUrl with "[::ffff:a00:1]" (hex-hextet private) and "[::ffff:808:808]"
(hex-hextet public) and with the expanded form "[0:0:0:0:0:ffff:10.0.0.1]" to
assert the private addresses throw "Cannot access internal or private network
addresses." and the public hex-hextet returns the normalized URL; place these in
lib/validation/url.test.ts alongside the existing IPv4-mapped tests so the code
paths in url.ts (the hex-hextet branch and the expanded form branch) are
exercised.
🧹 Nitpick comments (3)
scripts/check-i18n.ts (2)
86-97: Static analysis:forEachcallbacks should not return a value.Biome flags lines 88, 92, and 96 because the arrow-function shorthand implicitly returns the result of
console.log(). Use block bodies to silence the lint errors.Proposed fix
if (missing.length) { console.log(` Missing (${missing.length})`); - missing.forEach((key) => console.log(` - ${key}`)); + missing.forEach((key) => { console.log(` - ${key}`); }); } if (extra.length) { console.log(` Extra (${extra.length})`); - extra.forEach((key) => console.log(` - ${key}`)); + extra.forEach((key) => { console.log(` - ${key}`); }); } if (typeMismatches.length) { console.log(` Type mismatches (${typeMismatches.length})`); - typeMismatches.forEach((key) => console.log(` - ${key}`)); + typeMismatches.forEach((key) => { console.log(` - ${key}`); }); }
60-66: Consider wrapping the base file read in a try/catch with a user-friendly error.If
messages/en.jsondoesn't exist (e.g., running from wrong directory),readFileSyncwill throw a rawENOENTerror. A small guard would improve DX.Proposed improvement
const projectRoot = process.cwd(); const messagesDir = join(projectRoot, "messages"); const baseFile = join(messagesDir, "en.json"); +import { existsSync } from "fs"; +if (!existsSync(baseFile)) { + console.error(`Base locale file not found: ${baseFile}`); + process.exit(1); +} + const baseJson = readJson(baseFile);lib/validation/url.ts (1)
103-110: Overly broadincludes("ffff:")match may false-positive on non-mapped IPv6.Any IPv6 address containing the substring
"ffff:"will enter this branch — e.g.2001:db8:0:0:0:ffff:192.168.1.1is a legitimate (non-mapped) IPv6 address whose embedded IPv4 tail happens to be private. This fails safely (over-blocks rather than under-blocks), but it could be tightened to only match the canonical mapped prefix pattern:Tighter match for expanded IPv4-mapped forms
- // Handle expanded IPv4-mapped IPv6 forms (e.g., 0:0:0:0:0:ffff:10.0.0.1) - if (lower.includes("ffff:")) { - const lastColon = lower.lastIndexOf(":"); - const tail = lastColon >= 0 ? lower.slice(lastColon + 1) : ""; - if (tail.includes(".") && /^\d{1,3}(?:\.\d{1,3}){3}$/.test(tail)) { - return isPrivateIP(tail); - } - } + // Handle expanded IPv4-mapped IPv6 forms (e.g., 0:0:0:0:0:ffff:10.0.0.1) + const expandedMapped = lower.match( + /^(?:0+:){5}(?:0*f{4}):(\d{1,3}(?:\.\d{1,3}){3})$/ + ); + if (expandedMapped) { + return isPrivateIP(expandedMapped[1]); + }
1 participant
What
check-i18nscript to verify locale key consistency.<img>lint warnings in OG + lightbox.adProvidervar in gravity route.Why
Testing
Summary by CodeRabbit
New Features
Improvements
Tests
Chores