Repository navigation
blog: harden /blog routes (close #65, #66, #67, #68, #69, #70, #71) - #73
Conversation
Previously a missing or mis-named content/blog/ silently fell back to an empty post list — the blog index rendered "nothing yet.", the sitemap omitted post URLs, and generateStaticParams returned []. CI passed; prod silently regressed. Throw with an actionable message instead; .gitkeep is the escape hatch for the legitimate empty-posts case. Closes #66
Today output: "export" + dynamicParams = false means the framework only routes pre-validated slugs to this handler. parseSlug at the function boundary makes the invariant explicit instead of relying on transitive framework guarantees, so a future caller from an unguarded path (RSS feed, search, new route) can't regress it. Closes #67
SLUG_RE guards per-post slug interpolation into <loc> bodies. The origin had no equivalent guard — a future preview-URL accident introducing &, <, >, ", or ' would silently emit invalid XML. Closes #71
…ditional
- Slug brand is now backed by a module-local `unique symbol`, so no
external module can write `"foo" as Slug` — parseSlug is the only
callable construction path.
- IsoDate mirrors the pattern; parseEntry consumes it so PostMeta.date
surfaces as a branded value to downstream consumers (RSS feed,
archive page, etc.) without them re-testing ISO_DATE_RE.
- {post.summary ? <p>…</p> : null} on the blog index is dead defensive
code — summary is required + trimmed-non-empty at parse — so emit
the <p> unconditionally.
Closes #69
- Chrome.tsx: a bare `#section` from /blog/foo/ resolves to /blog/foo/#section, not /blog/#section as the comment claimed. - SLUG_RE: drop the CodeQL rule ID (rots when the rule is renamed). - Memoise comment: drop the parentheticals enumerating call sites (count is correct but rots with use). - Sort tie-break: drop the HFS+ aside (irrelevant on modern macOS). - openGraph metadata: 8-line block trimmed to the two non-obvious gotchas — shallow merge + og→twitter fallback timing. Closes #68
SLUG_RE is the central guard against stored-XSS via slug interpolation; the regex was tightened in #61 to forbid empty strings, leading/trailing hyphens, and consecutive hyphens, but none of those tighter constraints were protected against regression. This is the first test in the repo. Vitest is the natural choice given the Next 16 + React 19 + ESM stack. Closes #70
Deploying website with
|
| Latest commit: |
5086cb0
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://99e4ed36.website-70y.pages.dev |
| Branch Preview URL: | https://claude-blog-sweep.website-70y.pages.dev |
There was a problem hiding this comment.
Pull request overview
This PR bundles several follow-ups to harden the /blog feature: it tightens invariants at the lib/blog.ts boundary (slug/date branding + validation), prevents silent regressions in build output (missing posts directory now throws), improves a11y labeling for blog Frames, hardens sitemap XML generation, and introduces the repo’s first test setup using Vitest.
Changes:
- Add Vitest configuration and a
pnpm testscript, plus initialparseSlugregression tests. - Harden blog source handling: unfakeable
Slugbrand, brandedIsoDate, validategetPostinputs, and throw ifcontent/blog/is missing. - Fix
/blogroute a11y (aria-labelledbytargets now exist) and add an XML-safety assert forSITE_URLin the sitemap route.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
vitest.config.ts |
Adds Vitest runner configuration (include/exclude patterns). |
package.json |
Introduces test script and adds vitest devDependency. |
pnpm-lock.yaml |
Locks Vitest/Vite and related transitive dependencies. |
lib/blog.ts |
Hardens slug/date types + parsing, validates getPost, and throws on missing posts directory. |
lib/blog.test.ts |
Adds initial boundary tests for parseSlug. |
components/site/Chrome.tsx |
Tightens/clarifies hash-anchor comment for /blog/* rendering. |
app/sitemap-pages.xml/route.ts |
Adds XML-safety assert for SITE_URL used in <loc> bodies. |
app/blog/page.tsx |
Fixes blog heading id for Frame labeling and removes dead summary conditional. |
app/blog/[slug]/page.tsx |
Fixes post heading id for Frame labeling. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Vite 8 (pulled in transitively by Vitest 4 in 3dc0749) declares `engines.node: "^20.19.0 || >=22.12.0"`. The previous >=20.9.0 floor would let pnpm install on Node 20.9–20.18 with a peer-warning that the dev workflow won't actually run there. CI runs Node 24, so the practical floor is well clear of the new minimum. Addresses PR #73 review.
#67 added a parseSlug check at the getPost boundary so unguarded callers can't reach readEntries with junk input. Add a regression test exercising the same boundary cases as parseSlug (uppercase, double-hyphen, empty, underscore, angle brackets) and asserting null. The invalid-slug short-circuit also means no filesystem read happens for these inputs — captured in a comment on the test block. Addresses PR #73 review.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enhances the blog system by introducing branded types for slugs and dates, adding unit tests with Vitest, and improving error handling for missing content directories. It also includes UI refinements like header IDs and security checks for sitemap generation. Feedback highlights a potential security risk in error logging where unsanitized URLs are included, and suggests performance optimizations such as module-level caching and property filtering during data parsing.
| if (/[<>&"']/.test(SITE_URL)) { | ||
| throw new Error( | ||
| `[sitemap] SITE_URL contains XML-significant characters: ${SITE_URL}`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
The validation for SITE_URL is a good security measure to prevent XML injection. However, the error message includes the raw SITE_URL. If this error is ever displayed in an environment that renders HTML (like some CI logs or error monitoring tools), it could potentially lead to a secondary injection if SITE_URL itself was maliciously crafted. Consider sanitizing the output in the error message or just stating that the URL is invalid.
| if (a.date !== b.date) return b.date.localeCompare(a.date); | ||
| return a.slug.localeCompare(b.slug); |
There was a problem hiding this comment.
The addition of a tie-break on slug for sorting ensures consistent build output across different environments. However, since fs.readdirSync and subsequent file parsing are expensive operations, please implement module-level caching to prevent redundant work. Additionally, ensure that unused properties are filtered out during the initial parsing phase to simplify data retrieval and avoid 'no-unused-vars' warnings.
References
- Implement module-level caching for expensive file-system operations to prevent redundant parsing during build processes.
- Filter out unused properties during the initial parsing phase to simplify data retrieval and avoid eslint 'no-unused-vars' warnings.
Summary
All seven blog-PR-review follow-ups (#65–#71) bundled into one PR. Each is a single commit; the changes don't entangle.
Bugs
aria-labelledbypoints at non-existent header IDs on /blog routes #65 —<Frame id="blog">and<Frame id="post">declaredaria-labelledby="blog-h"/"post-h"but the matching<h1>had noid. Screen readers announced both sections as unnamed. Two single-attribute additions.lib/blog.tssilently fell back tocache = []whencontent/blog/was missing, so a mis-named or pruned directory would silently regress prod to an empty blog with CI still green. Throw with an actionable message instead;.gitkeepis the escape hatch.Hardening
getPostaccepts unvalidated string instead ofSlug#67 —getPost(slug: string)now callsparseSlugfirst and returnsnullon failure. Today this is redundant (static export +dynamicParams = falseonly routes pre-validated slugs), but it locks the invariant at the function boundary instead of relying on transitive framework guarantees.app/sitemap-pages.xml/route.tsinterpolatesSITE_URLraw into<loc>bodies.SLUG_REguards the per-post slug; the origin had no equivalent guard. Module-top assert added against<,>,&,",'.Refactor
Slugbrand type-design follow-ups (unique symbol, IsoDate, dead conditional) #69 —Slugbrand backed by a module-localunique symbol, so no external module can write"foo" as Slug. Same pattern applied asIsoDate;parseEntryconsumes it, soPostMeta.datesurfaces as branded for downstream consumers. Also drops the dead{post.summary ? <p>…</p> : null}guard on the blog index —summaryis required + trimmed-non-empty at parse.Docs
/blog/foo/#sectionresolved to/blog/#section), rot-prone (CodeQL rule ID, enumerated call sites, HFS+ aside), or violated AGENTS.md's "one short line max."Tests
parseSlug(SLUG_RE) #70 — First test in the repo. Wires up Vitest (the natural choice given Next 16 + React 19 + ESM) and adds boundary cases forparseSlug(5 accepts, 9 rejects, plus non-string inputs). 15 tests pass.Test plan
pnpm lint— cleanpnpm exec tsc --noEmit— cleanpnpm exec prettier --check .— cleanpnpm test— 15/15 passpnpm build— static export succeeds; 14 routes prerendered including/blog,/blog/[slug]× 3out/blog/index.htmlcontainsid="blog-h";out/blog/<post>/index.htmlcontainsid="post-h"out/sitemap-pages.xmlopens with a valid<?xml … ?>prolog (origin assert ran without throwing)pnpm dev:/blog/and/blog/<post>/look unchangedcontent/blog/→ build throws with actionable message