fix: use local TTF font and render logo in OG image#148
Merged
Conversation
Replace remote WOFF2 font fetch (unsupported by Satori) with local Inter TTF. Render site logo from chronicle.yaml next to site name in OG image when configured. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe PR extracts asset-loading utilities ( ChangesOG Image Generation Refactoring
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
🚥 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)
Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a 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: 4
🧹 Nitpick comments (1)
packages/chronicle/src/server/routes/og.test.ts (1)
6-6: ⚡ Quick winRemove unused constant.
The
FIXTURESconstant is defined but never used in the test file.♻️ Proposed fix
const PACKAGE_ROOT = path.resolve(__dirname, '../../..'); -const FIXTURES = path.resolve(__dirname, '__fixtures__');🤖 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 `@packages/chronicle/src/server/routes/og.test.ts` at line 6, Remove the unused constant FIXTURES from the test file to eliminate dead code: locate the declaration "const FIXTURES = path.resolve(__dirname, '__fixtures__');" in og.test.ts and delete that line (and any import solely used for it, e.g., path if it's only used here) so the test file no longer contains the unused FIXTURES symbol.
🤖 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 `@packages/chronicle/src/server/routes/og-utils.ts`:
- Around line 4-8: getLogoDataUri currently maps only .svg and .png and falls
back to image/jpeg, which mislabels other formats (e.g., .webp, .gif); update
getLogoDataUri to explicitly map supported extensions (include .webp, .gif,
etc.) or use a reliable lookup (e.g., mime.getType) to derive the MIME from
filePath, and if the MIME cannot be determined or is unsupported, either throw a
clear error or return a safe default (do not silently assume image/jpeg); ensure
references to ext and mime inside getLogoDataUri are updated accordingly.
- Around line 20-28: The loadFont function currently swallows errors and returns
an empty ArrayBuffer which breaks Satori; change loadFont to surface the
underlying error instead of returning new ArrayBuffer(0): catch the thrown error
from fs.readFile in loadFont, log or include the error details, and rethrow (or
throw a new Error with context) so callers (OG route / Satori) fail fast; ensure
the thrown error message references loadFont and the fontPath so it's easy to
trace.
In `@packages/chronicle/src/server/routes/og.tsx`:
- Line 66: Check that the font data loaded by loadFont (fontData) is non-null
and not empty before passing it to Satori; locate the code that constructs the
font object { name: 'Inter', data: fontData, weight: 400, style: 'normal' } and
add a guard like if (!fontData || !(fontData as ArrayBuffer).byteLength) then
log the failure and return a safe fallback response (or use a bundled default
font) instead of calling Satori. Ensure the check references the same fontData
variable and that any early return uses the same response shape your handler
expects.
- Around line 16-18: The code currently calls loadLogo(...) per-request to
compute logoSrc; instead, cache the loaded logo at module scope similar to
fontData: introduce a module-level variable (e.g., cachedLogo or logoData) and
initialize it once when config.logo?.dark is present by calling
loadLogo(__CHRONICLE_PROJECT_ROOT__, config.logo.dark) (or lazily on first use)
and then use that cachedLogo in place of computing logoSrc inside the request
handler; update references to logoSrc and retain use of config.logo.dark and
loadLogo for the initial load.
---
Nitpick comments:
In `@packages/chronicle/src/server/routes/og.test.ts`:
- Line 6: Remove the unused constant FIXTURES from the test file to eliminate
dead code: locate the declaration "const FIXTURES = path.resolve(__dirname,
'__fixtures__');" in og.test.ts and delete that line (and any import solely used
for it, e.g., path if it's only used here) so the test file no longer contains
the unused FIXTURES symbol.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 28b746ec-4be2-44c2-8421-fcb7e06430e2
⛔ Files ignored due to path filters (1)
packages/chronicle/src/fonts/Inter-Regular.ttfis excluded by!**/*.ttf
📒 Files selected for processing (5)
packages/chronicle/src/server/routes/og-utils.tspackages/chronicle/src/server/routes/og.test.tspackages/chronicle/src/server/routes/og.tsxpackages/chronicle/src/server/vite-config.tspackages/chronicle/src/types/globals.d.ts
RootHead now only emits site-level title and jsonLd WebSite schema. Page-level Head (DocsPage, ApiPage, LandingPage) is sole source of og:image, og:title, twitter tags with correct page title. Also bumps OG logo to 48px and site name font to 32px. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Use explicit MIME map, return null for unsupported formats - Throw on font load failure instead of returning empty ArrayBuffer - Cache logo at module level to avoid repeated file I/O Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@packages/chronicle/src/pages/LandingPage.tsx`:
- Around line 18-22: Replace the site-wide title prop passed into the Head
component from config.site.title with a page-specific title so Head can append
the site title correctly; in the LandingPage component change the title prop
(currently config.site.title) to a page-level value (e.g., a new
landingPageTitle variable, config.site.landingTitle, an empty string, or a title
that includes the current version) so Head (from
packages/chronicle/src/lib/head.tsx) produces correct document and social
metadata rather than duplicating the site title.
In `@packages/chronicle/src/server/App.tsx`:
- Around line 85-87: Replace raw JSON.stringify usage for JSON-LD injection with
a shared escaped serializer: implement and export a serializeJsonLd(value:
unknown) function that calls JSON.stringify(value, null, 2) and then escapes
problematic sequences (at minimum replace "</" with "<\/" and the Unicode
U+2028/U+2029 characters) to prevent breaking out of the <script
type="application/ld+json"> block; update the App component's siteJsonLd
injection (dangerouslySetInnerHTML) and the Head component's JSON-LD injection
to use serializeJsonLd(...) instead of JSON.stringify(...), and import the
shared helper where needed.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e175b1b2-aaae-4605-b2e1-aae25f537483
📒 Files selected for processing (3)
packages/chronicle/src/pages/LandingPage.tsxpackages/chronicle/src/server/App.tsxpackages/chronicle/src/server/routes/og.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/chronicle/src/server/routes/og.tsx
Comment on lines
+85
to
+87
| <script | ||
| type='application/ld+json' | ||
| dangerouslySetInnerHTML={{ __html: JSON.stringify(siteJsonLd, null, 2) }} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python - <<'PY'
import json
payload = {"name": "</script><script>alert(1)</script>"}
rendered = json.dumps(payload, indent=2)
print(rendered)
assert "</script>" in rendered, "Expected raw closing script tag in JSON.stringify-equivalent output"
PY
rg -n -C2 "dangerouslySetInnerHTML=\{\{ __html: JSON.stringify" packages/chronicle/srcRepository: raystack/chronicle
Length of output: 826
Security: escape JSON-LD output before injecting into the application/ld+json <script> tag.
dangerouslySetInnerHTML is populated with JSON.stringify(...) in both packages/chronicle/src/server/App.tsx and packages/chronicle/src/lib/head.tsx, so any config-derived value containing </script> (via unescaped <) can break out of the JSON-LD script block and enable script injection. Centralize an escaped serializer and use it in both places.
🔒 Suggested fix
+function serializeJsonLd(value: unknown) {
+ return JSON.stringify(value, null, 2)
+ .replace(/</g, '\\u003c')
+ .replace(/\u2028/g, '\\u2028')
+ .replace(/\u2029/g, '\\u2029');
+}
+
function RootHead({ config }: { config: ChronicleConfig }) {
const siteJsonLd = config.url
? {
'`@context`': 'https://schema.org',
'`@type`': 'WebSite',
name: config.site.title,
description: config.site.description,
url: config.url,
}
: null;
return (
<>
<title>{config.site.title}</title>
{siteJsonLd && (
<script
type='application/ld+json'
- dangerouslySetInnerHTML={{ __html: JSON.stringify(siteJsonLd, null, 2) }}
+ dangerouslySetInnerHTML={{ __html: serializeJsonLd(siteJsonLd) }}
/>
)}
</>
);
}Apply the same serializeJsonLd(...) usage to packages/chronicle/src/lib/head.tsx (preferably via a shared helper used by both files).
🧰 Tools
🪛 ast-grep (0.43.0)
[warning] 86-86: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🤖 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 `@packages/chronicle/src/server/App.tsx` around lines 85 - 87, Replace raw
JSON.stringify usage for JSON-LD injection with a shared escaped serializer:
implement and export a serializeJsonLd(value: unknown) function that calls
JSON.stringify(value, null, 2) and then escapes problematic sequences (at
minimum replace "</" with "<\/" and the Unicode U+2028/U+2029 characters) to
prevent breaking out of the <script type="application/ld+json"> block; update
the App component's siteJsonLd injection (dangerouslySetInnerHTML) and the Head
component's JSON-LD injection to use serializeJsonLd(...) instead of
JSON.stringify(...), and import the shared helper where needed.
Closes #139 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Avoids "Pixxel | Pixxel" duplication in document title and og:title. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
rohanchkrabrty
approved these changes
Jun 4, 2026
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
chronicle.yaml(logo.dark) next to site name in OG imageloadFont,loadLogo,getLogoDataUri) into testable module__CHRONICLE_PACKAGE_ROOT__build-time constant for font path resolutiondateModifiedto Article JSON-LD on doc pagesCloses #120
Closes #139
Test plan
bun test src/server/routes/og.test.ts) — no server needed🤖 Generated with Claude Code