feat: add spire blog app with overrideSections support#1561
Merged
Conversation
Introduces the spire/ app integrating with the Spire Blog API. Adds an `overrideSections` prop to the app config, allowing users to replace any default block renderer (e.g. "product-shelf") with a custom section via the deco admin UI. Made-with: Cursor
Contributor
Tagging OptionsShould a new tag be published when this PR is merged?
|
Contributor
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughA new Spire blog integration app is added to the codebase, providing loaders for individual blog posts and paginated post listings, along with associated section components for rendering posts, SEO metadata, and a collection of block components for content structure. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Loader as BlogPostPage Loader
participant API as Spire API
participant Converter as spirePostToBlogPost
participant Template
participant SEO
Client->>Loader: Request blog post (slug)
Loader->>API: GET /blog/:account/posts/:slug
API-->>Loader: SpirePost (with version blocks)
Loader->>Converter: Convert SpirePost
Converter-->>Loader: BlogPost (with sections)
Loader-->>Template: Return BlogPostPage
Template->>Template: Render title, excerpt, date, image
Template->>Template: Map blocks to sections via blocksToSections
Template-->>Client: Rendered HTML
SEO->>SEO: Compose metadata (title, description, image, canonical)
SEO-->>Client: SEO tags & JSON-LD
sequenceDiagram
participant Client
participant Loader as BlogpostListing Loader
participant API as Spire API
participant Converter as spirePostSummaryToBlogPost
participant PageComponent
Client->>Loader: Request blog listing (page, count)
Loader->>Loader: Parse & validate page/count
Loader->>API: GET /blog/:account (with page, perPage)
API-->>Loader: SpireListingResponse (posts[], pagination, blog)
Loader->>Converter: Convert each SpirePostSummary
Converter-->>Loader: BlogPost[]
Loader->>Loader: Build pageInfo (nextPage, previousPage)
Loader-->>PageComponent: Return BlogPostListingPage
PageComponent->>PageComponent: Render posts list with pagination
PageComponent-->>Client: Rendered HTML
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
11 issues found across 28 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="spire/loaders/BlogpostList.ts">
<violation number="1" location="spire/loaders/BlogpostList.ts:29">
P2: Normalize `page`/`count` in `cacheKey` to avoid cache fragmentation from raw query strings.</violation>
<violation number="2" location="spire/loaders/BlogpostList.ts:41">
P2: Validate and sanitize `page`/`count` before calling the API; `Number(...)` can produce `NaN` for query input.</violation>
</file>
<file name="spire/loaders/BlogpostListing.ts">
<violation number="1" location="spire/loaders/BlogpostListing.ts:44">
P2: Guard against non-numeric `page`/`count` query values so you don’t send `NaN` to the API. Fall back to defaults when parsing fails.</violation>
</file>
<file name="spire/utils/blocksToSections.ts">
<violation number="1" location="spire/utils/blocksToSections.ts:33">
P2: Override sections lose their configured props because only `__resolveType` is read from the override and the returned section is rebuilt from `block.content` only.</violation>
</file>
<file name="spire/sections/blocks/StatGroup.tsx">
<violation number="1" location="spire/sections/blocks/StatGroup.tsx:17">
P1: Validate the parsed JSON type before assigning to `items`; non-array JSON currently causes a runtime crash when `.slice()` is called.</violation>
</file>
<file name="spire/sections/blocks/CardGroup.tsx">
<violation number="1" location="spire/sections/blocks/CardGroup.tsx:17">
P2: Validate the parsed JSON type before using array methods; non-array JSON can crash rendering.</violation>
<violation number="2" location="spire/sections/blocks/CardGroup.tsx:40">
P1: Unsanitized `dangerouslySetInnerHTML` renders user-provided HTML and can introduce XSS.</violation>
</file>
<file name="spire/sections/blocks/Checklist.tsx">
<violation number="1" location="spire/sections/blocks/Checklist.tsx:10">
P2: Validate the parsed JSON is an array before assigning it to `list`; otherwise valid non-array JSON will crash rendering when `.map` is called.</violation>
</file>
<file name="spire/mod.ts">
<violation number="1" location="spire/mod.ts:71">
P3: Spire app metadata uses the Weather logo URL, which causes incorrect branding for this app.</violation>
</file>
<file name="spire/sections/blocks/Steps.tsx">
<violation number="1" location="spire/sections/blocks/Steps.tsx:12">
P2: Validate that parsed JSON is an array before assigning to `list`; otherwise `list.map(...)` can crash at runtime.</violation>
</file>
<file name="spire/sections/blocks/Video.tsx">
<violation number="1" location="spire/sections/blocks/Video.tsx:21">
P1: Validate and restrict `iframe` URLs before assigning `src`; currently untrusted input can be embedded directly when it doesn't match the YouTube/Vimeo rewrite rules.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![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: 18
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (4)
spire/sections/blocks/CardGroup.tsx-16-21 (1)
16-21:⚠️ Potential issue | 🟡 MinorGuard against valid-but-wrong JSON shapes.
try/catchonly handles invalid JSON. Values like"null"or"{}"will parse successfully and then blow up onslice/map. Normalize the parsed value withArray.isArray(...) ? ... : []before using it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spire/sections/blocks/CardGroup.tsx` around lines 16 - 21, The JSON parsing currently only catches invalid JSON but doesn't guard against valid-but-wrong shapes (e.g. "null" or "{}") which later cause failures when using slice/map; after parsing the value from cards (and still inside the try/catch), normalize the parsed result by replacing items with an array only if Array.isArray(parsed) else fallback to [] (keep the existing catch to set items = [] on parse errors) so that variables like items, clamped and GRID_CLASS lookups are always working with an array.spire/sections/blocks/Steps.tsx-11-13 (1)
11-13:⚠️ Potential issue | 🟡 MinorValidate the parsed
stepsvalue before iterating it.A payload like
"null"or"{}"passesJSON.parseand then fails atlist.map(...). Please coerce non-array results to[]after parsing instead of assuming the JSON shape is always correct.Also applies to: 22-24
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spire/sections/blocks/Steps.tsx` around lines 11 - 13, The JSON.parse result for `steps` must be validated and coerced to an array before use: after the existing try/catch around `list = JSON.parse(steps ?? "[]");` check if the parsed value is an Array (e.g. Array.isArray(parsed)) and if not set `list = []`; do the same validation/coercion for the other parse block mentioned (the second JSON.parse at lines 22-24) so any parsed `null` or object becomes an empty array before calling `list.map(...)` or other array methods.spire/loaders/BlogpostList.ts-64-76 (1)
64-76:⚠️ Potential issue | 🟡 MinorDon’t coerce a missing publish date to
"".
SpirePostSummary.publishedAtis nullable, butBlogPost.dateis supposed to be a date-formatted string. Returning""will surface as an invalid date in downstream formatters if Spire sendsnull. Filter those posts out or model the absence explicitly instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spire/loaders/BlogpostList.ts` around lines 64 - 76, spirePostSummaryToBlogPost currently coerces a nullable SpirePostSummary.publishedAt to "" which yields invalid dates; instead either filter out summaries with null publishedAt before mapping or preserve the absence by assigning the raw nullable value (e.g. date: summary.publishedAt) and update BlogPost.date to be optional/null-able (string | null | undefined) so downstream formatters can handle missing dates explicitly; locate and change mapping in spirePostSummaryToBlogPost and adjust the BlogPost type accordingly or add a pre-filter that removes summaries where publishedAt is null.spire/mod.ts-71-71 (1)
71-71:⚠️ Potential issue | 🟡 MinorIncorrect logo URL - points to weather app instead of Spire.
The logo URLs reference the weather app's logo. Update these to use an appropriate Spire blog logo.
- * `@logo` https://raw.githubusercontent.com/deco-cx/apps/main/weather/logo.png + * `@logo` https://raw.githubusercontent.com/deco-cx/apps/main/spire/logo.pngAnd in the preview props:
logo: - "https://raw.githubusercontent.com/deco-cx/apps/main/weather/logo.png", + "https://raw.githubusercontent.com/deco-cx/apps/main/spire/logo.png",Also applies to: 97-98
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spire/mod.ts` at line 71, The manifest's `@logo` annotation and the preview props currently point to the weather app logo; update both occurrences (the `@logo` field and the preview image/logo entries inside the preview props) to the correct Spire blog logo URL by replacing "https://raw.githubusercontent.com/deco-cx/apps/main/weather/logo.png" with the appropriate Spire logo URL in spire/mod.ts (look for the `@logo` annotation and the preview block around the preview props).
🧹 Nitpick comments (6)
spire/loaders/BlogPostPage.ts (3)
77-77: Inconsistentdatefallback between loaders.This loader falls back to
new Date().toISOString()whenpublishedAtis null, whileBlogpostList.tsuses an empty string. This inconsistency could cause confusing behavior.Consider a unified approach - either both should use current date, or both should indicate the absence differently (e.g., a sentinel value or throwing for unpublished posts).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spire/loaders/BlogPostPage.ts` at line 77, The date fallback in the BlogPostPage loader is inconsistent with BlogpostList.ts: replace the current fallback logic in BlogPostPage (where it sets date: post.publishedAt ?? new Date().toISOString()) with the same strategy used in BlogpostList.ts (either an empty string or the agreed sentinel) or vice versa so both loaders use the same behavior; update the expression referencing post.publishedAt in the BlogPostPage loader to match the chosen unified fallback policy and ensure any downstream consumers of the date field (renderers or serializers) handle that unified sentinel consistently.
36-44: Consider adding error logging for failed requests.Unlike
BlogpostListing.tswhich logs errors, this loader silently returnsnullon failure. Adding logging would help with debugging production issues.♻️ Suggested improvement
+import { logger } from "@deco/deco/o11y"; + if (!response.ok) { + logger.error(`Failed to fetch Spire post: ${slug}, status: ${response.status}`); return null; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spire/loaders/BlogPostPage.ts` around lines 36 - 44, The loader currently returns null silently when the fetch fails or the JSON has no post; update the error handling in BlogPostPage.ts around the response.ok check and the subsequent const { post } extraction to log failures (e.g., using the project logger or console.error) including useful details such as response.status, response.statusText and any error body or parsed error message so you can diagnose bad responses and missing post payloads.
71-75: Fabricated email addresses may be misleading.The email is generated as
${a.slug}@spire.blog, which creates fake email addresses. If theAuthortype requires an email but Spire doesn't provide one, consider using an empty string orundefined(if the type allows) rather than fabricating addresses that could be mistaken for real contact information.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spire/loaders/BlogPostPage.ts` around lines 71 - 75, The authors mapping in BlogPostPage uses a fabricated email `${a.slug}@spire.blog`; change the mapping in the authors: post.authors.map(...) block to stop constructing fake addresses — use undefined (or an empty string if the Author type requires it) instead of `${a.slug}@spire.blog`, and update the Author type or casting if necessary to accept undefined/email optional; locate the authors mapping and replace email: `${a.slug}@spire.blog` with email: undefined (or email: '') and adjust types for post.authors / Author accordingly.spire/loaders/BlogpostListing.ts (3)
64-71: SEOtitleis empty - consider using blog metadata.The
seo.titleis hardcoded to an empty string. The API response includesblogmetadata (perSpireListingResponseinspire/utils/client.ts) which could provide a meaningful title.♻️ Suggested improvement
- const { posts: rawPosts, pagination } = await response.json(); + const { posts: rawPosts, pagination, blog } = await response.json(); const posts: BlogPost[] = (rawPosts ?? []).map(spirePostSummaryToBlogPost); ... return { posts, pageInfo: toPageInfo(pagination, params), seo: { - title: "", + title: blog?.name ?? "", canonical: new URL(url.pathname, url.origin).href, }, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spire/loaders/BlogpostListing.ts` around lines 64 - 71, The SEO title is hardcoded empty in the return object; update the seo.title to use the blog metadata from the listing response (e.g., response.blog?.title or the variable holding the SpireListingResponse.blog) and fall back to a sensible default (site name or "Blog") if missing; modify the object returned in BlogpostListing.ts (the block that returns posts, pageInfo: toPageInfo(pagination, params), seo: { title: "", canonical: new URL(url.pathname, url.origin).href }) to set seo.title to that blog title/fallback.
60-62: Returningnullfor empty posts may not be the best UX.When
posts.length === 0, the loader returnsnull. This could happen on a valid page beyond the last page, or when no posts exist. Consider returning an empty listing page instead, so the UI can show a "no posts found" message rather than a 404-like experience.♻️ Suggested alternative
- if (posts.length === 0) { - return null; - } + // Allow empty listings - UI can display "no posts" message🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spire/loaders/BlogpostListing.ts` around lines 60 - 62, The loader in BlogpostListing.ts currently returns null when posts.length === 0; instead return a valid, empty listing payload so the UI can render a "no posts found" state (e.g., return the same shape your loader normally returns but with posts: [] and total: 0, keeping pagination fields like page/pageSize intact). Update the branch where posts is empty to construct and return that empty listing object (referencing the posts variable and the BlogpostListing loader function) rather than null, and ensure any callers that checked for null now handle an empty posts array to show the empty-state UI.
29-34: Cache key generation may not handle all edge cases correctly.Using
??withurl.searchParams.get()won't distinguish between missing params and params with value"0". Also, the cacheKey logic differs slightly from the loader logic (lines 44-45) in how defaults are applied.Consider using the same resolution logic in both places for consistency:
export const cacheKey = (props: Props, req: Request, ctx: AppContext) => { const url = new URL(req.url); - const page = props.page ?? url.searchParams.get("page") ?? 1; - const count = props.count ?? url.searchParams.get("count") ?? 12; + const page = props.page ?? Number(url.searchParams.get("page")) || 1; + const count = props.count ?? Number(url.searchParams.get("count")) || 12; return `spire-listing-${ctx.account}-page${page}-count${count}`; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spire/loaders/BlogpostListing.ts` around lines 29 - 34, cacheKey currently uses the nullish coalescing (??) with url.searchParams.get() which treats "0" or "" incorrectly and is inconsistent with the loader resolution; update cacheKey to mirror the loader's parameter-resolution logic by explicitly checking for presence (not just nullish) and parsing numeric strings: resolve page and count by first checking props.page/props.count (allowing 0), then check url.searchParams.get("page")/get("count") and treat an empty string as missing, parse them to numbers with a safe fallback to defaults (1 for page, 12 for count), and return the same key format, referencing the cacheKey function to implement this consistent logic (or factor the logic into a shared helper used by both cacheKey and the loader).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@spire/loaders/BlogpostList.ts`:
- Around line 27-31: Normalize and clamp page/count once and reuse them: inside
cacheKey (exported function cacheKey with Params Props, Request, AppContext)
parse props.page and props.count (falling back to url.searchParams or defaults 1
and 12) into positive integers, clamp page >=1 and count to a sane max (e.g.
100), then build the cache key using these normalized numeric values; also
update the subsequent fetch logic in this file that currently reads
props.page/props.count (the fetch block later in the file) to use the same
normalized variables so the cache key and API call match and huge/invalid values
are prevented.
In `@spire/loaders/BlogPostPage.ts`:
- Line 46: The call site passes ctx.overrideMap which is Record<string,
Resolved<Section>> but spirePostToBlogPost declares overrides as Record<string,
Section>, causing a type mismatch; update spirePostToBlogPost’s overrides
parameter type to Record<string, Resolved<Section>> (or alternatively unwrap the
Resolved<Section> values before calling) and apply the same change where
blocksToSections is invoked—either change blocksToSections to accept
Resolved<Section> or map ctx.overrideMap to produce plain Section instances
before passing; adjust any type annotations and usages inside
spirePostToBlogPost and blocksToSections accordingly so the types line up with
ctx.overrideMap.
In `@spire/sections/blocks/Callout.tsx`:
- Around line 59-63: The Callout component currently injects the raw body prop
into the DOM via dangerouslySetInnerHTML (see the body variable usage in
Callout.tsx); sanitize the HTML before rendering or make the trust boundary
explicit in the prop/type so only already-sanitized/ trusted HTML is accepted.
Update the Callout component to run body through a sanitizer (or validate the
prop type as TrustedHtml) before passing it to dangerouslySetInnerHTML, and
ensure any helper used (e.g., sanitizeHtml or a strip-and-allowlist utility) is
imported and applied where body is used so the XSS sink is eliminated.
In `@spire/sections/blocks/CardGroup.tsx`:
- Around line 38-45: In CardGroup.tsx replace the direct use of
dangerouslySetInnerHTML with sanitized HTML outputs: ensure card.title and
card.body are passed through a trusted sanitization function (e.g., DOMPurify or
sanitize-html) before assigning to dangerouslySetInnerHTML, or alternatively
render them as plain text if HTML isn't required; update the component where
card.title and card.body are used (the <strong ... dangerouslySetInnerHTML={{
__html: card.title }} /> and <p ... dangerouslySetInnerHTML={{ __html: card.body
}} /> lines) to use the sanitized result (e.g., sanitizedTitle/sanitizedBody) or
document/validate the upstream trust guarantee at the boundary where the Spire
JSON is parsed.
In `@spire/sections/blocks/Checklist.tsx`:
- Around line 9-11: The code parses items into `list` and then calls `list.map`,
but `JSON.parse(items ?? "[]")` can return a non-array and cause a runtime
error; update the `try` block in Checklist (where `list` is assigned) to
validate the parsed value (e.g., use a temp `parsed` variable and set `list =
Array.isArray(parsed) ? parsed : []`) so that subsequent `list.map` is always
called on an array; keep the catch behavior but ensure `list` is defaulted to an
empty array when parsing yields a non-array.
- Around line 43-46: The Checklist component is rendering untrusted HTML via
dangerouslySetInnerHTML for the item prop (span with dangerouslySetInnerHTML in
Checklist.tsx), creating an XSS risk; fix by importing and applying a sanitizer
(e.g., DOMPurify) before passing HTML into dangerouslySetInnerHTML (sanitize
item in the rendering path inside Checklist component), or alternatively change
the component API to accept only pre-sanitized HTML (rename prop to
sanitizedItem or add a boolean prop requireSanitized and assert callers provide
sanitized content). Also apply the same fix pattern to the other components that
use dangerouslySetInnerHTML (List.tsx, Paragraph.tsx, Steps.tsx, Comparison.tsx,
CardGroup.tsx, Callout.tsx) so all user-supplied HTML is sanitized or required
to be pre-sanitized.
In `@spire/sections/blocks/Comparison.tsx`:
- Line 36: The span is rendering user-provided HTML (`item`) via
dangerouslySetInnerHTML in Comparison.tsx (both occurrences around the span at
line 36 and the similar one at line 56); sanitize the HTML before rendering by
importing a sanitizer (e.g., DOMPurify) and replacing the raw `item` with the
sanitized output (e.g., sanitized = DOMPurify.sanitize(item)) or, if HTML is not
required, render `item` as plain text instead; update the span usages that call
dangerouslySetInnerHTML to use the sanitized value and ensure DOMPurify is
included where the `item` value is produced or just before rendering.
- Around line 14-18: The JSON parse try/catch only prevents parse errors but
doesn't guarantee the expected shape, so modify the Comparison component to
validate shape after parsing: add a runtime guard (e.g., isColumnShape or
ensureItemsArray) that checks leftCol.items and rightCol.items are arrays and
that each item has the expected fields, and coerce missing/invalid values to an
empty array before calling items.map (use these guards where leftCol and
rightCol are used). Also prevent XSS by sanitizing any HTML rendered via
dangerouslySetInnerHTML (import and use a sanitizer such as DOMPurify or
sanitize-html and sanitize item content before passing it into
dangerouslySetInnerHTML); reference leftCol, rightCol, items, and the
dangerouslySetInnerHTML usages in the Comparison component to locate where to
apply these fixes.
In `@spire/sections/blocks/Cta.tsx`:
- Around line 9-11: The Cta component currently forwards the href prop directly
into the anchor which allows dangerous schemes like javascript:; update Cta to
compute a safeHref before rendering by parsing/inspecting href and allowing only
safe schemes (e.g. http, https, mailto, tel, or hash links), falling back to "#"
when invalid or missing, and use that safeHref in the <a> element; also ensure
any external links set target and rel appropriately (e.g. target="_blank" with
rel="noopener noreferrer") so reference the Cta component and the anchor
element/class attributes when making the change.
In `@spire/sections/blocks/Heading.tsx`:
- Around line 25-30: Heading (and sibling blocks Paragraph, CardGroup, Steps,
List, Quote) currently inject the text prop via dangerouslySetInnerHTML without
sanitization; fix by integrating a client-side sanitizer (e.g., DOMPurify) and
always sanitize the incoming text before rendering. In Heading.tsx sanitize the
text prop (e.g., const safe = DOMPurify.sanitize(text)) and use
dangerouslySetInnerHTML={{ __html: safe }} in the Heading render branches
(h2/h3/h4); apply the same change to the other block components (Paragraph,
CardGroup, Steps, List, Quote) so all server/authoring paths consistently
sanitize, and add the DOMPurify import and a small helper sanitize function if
desired.
In `@spire/sections/blocks/List.tsx`:
- Around line 16-33: The List component currently injects HTML unsafely via
entries.map -> <li ... dangerouslySetInnerHTML={{ __html: item }} />, creating
XSS risk; update the component (and similar components Checklist and Callout) to
sanitize item before setting innerHTML by integrating an allowlist HTML
sanitizer (e.g., DOMPurify) at the component level (sanitize each item and use
the sanitized string in dangerouslySetInnerHTML) or, if HTML is not required,
stop using dangerouslySetInnerHTML and render item as plain text instead.
In `@spire/sections/blocks/Paragraph.tsx`:
- Around line 8-10: The Paragraph component uses dangerouslySetInnerHTML with
html/text (see Paragraph.tsx and blocksToSections.ts) and must sanitize input
first to prevent XSS: add a sanitizeHtml utility (or integrate DOMPurify) to
clean the HTML produced from block.content in blocksToSections.ts, export that
sanitized string, and in Paragraph.tsx import and use the sanitizer so
dangerouslySetInnerHTML receives only the sanitized HTML (or fallback to escaped
text) before rendering; ensure the sanitizer is used wherever block.content/html
are converted to HTML to centralize protection.
In `@spire/sections/blocks/StatGroup.tsx`:
- Around line 16-21: The parsed stats value may not be an array, causing
items.slice to throw; in StatGroup.tsx ensure the result of JSON.parse(stats ??
"[]") is validated and coerced to an array before array operations: after
parsing into items, check Array.isArray(items) (and optionally that each element
matches the expected stat shape) and if not, replace items with an empty array
(or a safe fallback), then compute clamped = items.slice(0, 3) and gridCols =
GRID_CLASS[clamped.length] ?? GRID_CLASS[3]; reference the variables items,
stats, clamped, and GRID_CLASS to locate the change.
In `@spire/sections/blocks/Steps.tsx`:
- Around line 43-50: The component is injecting untrusted HTML from step.title
and step.description via dangerouslySetInnerHTML in Steps.tsx; sanitize these
values before rendering to prevent XSS. Import a trusted sanitizer (e.g.,
DOMPurify or sanitize-html) and run DOMPurify.sanitize(step.title) and
DOMPurify.sanitize(step.description) (or sanitize at data parsing time) and then
pass those sanitized strings to dangerouslySetInnerHTML (or render as plain text
when no markup is expected). Ensure the sanitization step is applied wherever
step objects are created or rendered (referencing step.title and
step.description in Steps.tsx) so only sanitized HTML reaches the DOM.
In `@spire/sections/blocks/Video.tsx`:
- Around line 7-15: The current Video.tsx sets src = url ?? "" and falls back to
embedding any input; change the logic to start with src = null and only set src
when a known provider's regex matches (use the existing ytMatch and vimeoMatch
variable patterns) so you derive embed URLs from an allowlist (YouTube and
Vimeo) and return null / nothing from the component when no provider matched;
ensure any iframe rendering code uses this src variable so arbitrary or
unsupported URLs are never embedded.
In `@spire/sections/Seo/SeoBlogPost.tsx`:
- Around line 20-31: The destructuring currently treats ctx.seo as
Record<string, unknown>, making titleTemplate and descriptionTemplate unknown;
narrow ctx.seo to an object with string templates (or validate and coerce)
before destructuring so renderTemplateString receives strings: e.g., inspect or
type-guard ctx.seo to extract titleTemplate and descriptionTemplate as string
(or fallback to "%s") and then call renderTemplateString with those string
values; reference ctx.seo, titleTemplate, descriptionTemplate, and
renderTemplateString to locate the change in SeoBlogPost.tsx.
In `@spire/sections/Seo/SeoBlogPostListing.tsx`:
- Around line 20-31: ctx.seo is being double-cast to unknown which makes
titleTemplate and descriptionTemplate type unknown and causes TS2345 when passed
into renderTemplateString; add runtime type guards to narrow ctx.seo before
using it: check that (ctx as any).seo is an object and that seo.titleTemplate
and seo.descriptionTemplate are typeof "string" (or derive them via a small
helper like getSeoTemplates(ctx) that returns { titleTemplate?: string;
descriptionTemplate?: string }), then only pass the validated string templates
into renderTemplateString; reference the symbols ctx, seo, titleTemplate,
descriptionTemplate, renderTemplateString, jsonLD and props when locating and
updating the code.
In `@spire/utils/blocksToSections.ts`:
- Around line 31-35: The code path for handling overrides currently throws away
the override object's props and logs its config; update the branch that checks
overrides[block.type] to stop console.log, preserve and merge the override
section props into the created section, and only use the override's
__resolveType (if present) plus the merged props so block.content overrides or
extends the configured override. Specifically, call toSection with the name
overrides[block.type]?.__resolveType ?? block.type and as the props spread the
override object (excluding __resolveType if desired) merged with
...(block.content as Record<string, unknown>) so existing override configuration
is retained and block content applies on top; also remove the debug log to avoid
leaking config.
---
Minor comments:
In `@spire/loaders/BlogpostList.ts`:
- Around line 64-76: spirePostSummaryToBlogPost currently coerces a nullable
SpirePostSummary.publishedAt to "" which yields invalid dates; instead either
filter out summaries with null publishedAt before mapping or preserve the
absence by assigning the raw nullable value (e.g. date: summary.publishedAt) and
update BlogPost.date to be optional/null-able (string | null | undefined) so
downstream formatters can handle missing dates explicitly; locate and change
mapping in spirePostSummaryToBlogPost and adjust the BlogPost type accordingly
or add a pre-filter that removes summaries where publishedAt is null.
In `@spire/mod.ts`:
- Line 71: The manifest's `@logo` annotation and the preview props currently point
to the weather app logo; update both occurrences (the `@logo` field and the
preview image/logo entries inside the preview props) to the correct Spire blog
logo URL by replacing
"https://raw.githubusercontent.com/deco-cx/apps/main/weather/logo.png" with the
appropriate Spire logo URL in spire/mod.ts (look for the `@logo` annotation and
the preview block around the preview props).
In `@spire/sections/blocks/CardGroup.tsx`:
- Around line 16-21: The JSON parsing currently only catches invalid JSON but
doesn't guard against valid-but-wrong shapes (e.g. "null" or "{}") which later
cause failures when using slice/map; after parsing the value from cards (and
still inside the try/catch), normalize the parsed result by replacing items with
an array only if Array.isArray(parsed) else fallback to [] (keep the existing
catch to set items = [] on parse errors) so that variables like items, clamped
and GRID_CLASS lookups are always working with an array.
In `@spire/sections/blocks/Steps.tsx`:
- Around line 11-13: The JSON.parse result for `steps` must be validated and
coerced to an array before use: after the existing try/catch around `list =
JSON.parse(steps ?? "[]");` check if the parsed value is an Array (e.g.
Array.isArray(parsed)) and if not set `list = []`; do the same
validation/coercion for the other parse block mentioned (the second JSON.parse
at lines 22-24) so any parsed `null` or object becomes an empty array before
calling `list.map(...)` or other array methods.
---
Nitpick comments:
In `@spire/loaders/BlogpostListing.ts`:
- Around line 64-71: The SEO title is hardcoded empty in the return object;
update the seo.title to use the blog metadata from the listing response (e.g.,
response.blog?.title or the variable holding the SpireListingResponse.blog) and
fall back to a sensible default (site name or "Blog") if missing; modify the
object returned in BlogpostListing.ts (the block that returns posts, pageInfo:
toPageInfo(pagination, params), seo: { title: "", canonical: new
URL(url.pathname, url.origin).href }) to set seo.title to that blog
title/fallback.
- Around line 60-62: The loader in BlogpostListing.ts currently returns null
when posts.length === 0; instead return a valid, empty listing payload so the UI
can render a "no posts found" state (e.g., return the same shape your loader
normally returns but with posts: [] and total: 0, keeping pagination fields like
page/pageSize intact). Update the branch where posts is empty to construct and
return that empty listing object (referencing the posts variable and the
BlogpostListing loader function) rather than null, and ensure any callers that
checked for null now handle an empty posts array to show the empty-state UI.
- Around line 29-34: cacheKey currently uses the nullish coalescing (??) with
url.searchParams.get() which treats "0" or "" incorrectly and is inconsistent
with the loader resolution; update cacheKey to mirror the loader's
parameter-resolution logic by explicitly checking for presence (not just
nullish) and parsing numeric strings: resolve page and count by first checking
props.page/props.count (allowing 0), then check
url.searchParams.get("page")/get("count") and treat an empty string as missing,
parse them to numbers with a safe fallback to defaults (1 for page, 12 for
count), and return the same key format, referencing the cacheKey function to
implement this consistent logic (or factor the logic into a shared helper used
by both cacheKey and the loader).
In `@spire/loaders/BlogPostPage.ts`:
- Line 77: The date fallback in the BlogPostPage loader is inconsistent with
BlogpostList.ts: replace the current fallback logic in BlogPostPage (where it
sets date: post.publishedAt ?? new Date().toISOString()) with the same strategy
used in BlogpostList.ts (either an empty string or the agreed sentinel) or vice
versa so both loaders use the same behavior; update the expression referencing
post.publishedAt in the BlogPostPage loader to match the chosen unified fallback
policy and ensure any downstream consumers of the date field (renderers or
serializers) handle that unified sentinel consistently.
- Around line 36-44: The loader currently returns null silently when the fetch
fails or the JSON has no post; update the error handling in BlogPostPage.ts
around the response.ok check and the subsequent const { post } extraction to log
failures (e.g., using the project logger or console.error) including useful
details such as response.status, response.statusText and any error body or
parsed error message so you can diagnose bad responses and missing post
payloads.
- Around line 71-75: The authors mapping in BlogPostPage uses a fabricated email
`${a.slug}@spire.blog`; change the mapping in the authors: post.authors.map(...)
block to stop constructing fake addresses — use undefined (or an empty string if
the Author type requires it) instead of `${a.slug}@spire.blog`, and update the
Author type or casting if necessary to accept undefined/email optional; locate
the authors mapping and replace email: `${a.slug}@spire.blog` with email:
undefined (or email: '') and adjust types for post.authors / Author accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c77103f4-6ac1-4893-aeac-060866d66b4f
📒 Files selected for processing (28)
deco.tsspire/loaders/BlogPostPage.tsspire/loaders/BlogpostList.tsspire/loaders/BlogpostListing.tsspire/manifest.gen.tsspire/mod.tsspire/sections/Seo/SeoBlogPost.tsxspire/sections/Seo/SeoBlogPostListing.tsxspire/sections/Template.tsxspire/sections/blocks/BlockImage.tsxspire/sections/blocks/Callout.tsxspire/sections/blocks/CardGroup.tsxspire/sections/blocks/Checklist.tsxspire/sections/blocks/Code.tsxspire/sections/blocks/Comparison.tsxspire/sections/blocks/Cta.tsxspire/sections/blocks/Divider.tsxspire/sections/blocks/Heading.tsxspire/sections/blocks/List.tsxspire/sections/blocks/Paragraph.tsxspire/sections/blocks/Quote.tsxspire/sections/blocks/Stat.tsxspire/sections/blocks/StatGroup.tsxspire/sections/blocks/Steps.tsxspire/sections/blocks/Video.tsxspire/types.tsspire/utils/blocksToSections.tsspire/utils/client.ts
There was a problem hiding this comment.
2 issues found across 11 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="vtex/loaders/legacy/productList.ts">
<violation number="1" location="vtex/loaders/legacy/productList.ts:139">
P2: Remove this debug `console.log`; it will spam production logs and can leak request/config data from `props`.</violation>
</file>
<file name="spire/sections/blocks/Video.tsx">
<violation number="1" location="spire/sections/blocks/Video.tsx:16">
P2: The new URL whitelist rejects valid YouTube/Vimeo embed URLs, causing previously renderable videos to disappear.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| if (vimeoMatch) return `https://player.vimeo.com/video/${vimeoMatch[1]}`; | ||
|
|
||
| // Reject anything that didn't match a trusted provider. | ||
| return null; |
There was a problem hiding this comment.
P2: The new URL whitelist rejects valid YouTube/Vimeo embed URLs, causing previously renderable videos to disappear.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At spire/sections/blocks/Video.tsx, line 16:
<comment>The new URL whitelist rejects valid YouTube/Vimeo embed URLs, causing previously renderable videos to disappear.</comment>
<file context>
@@ -3,16 +3,23 @@ export interface Props {
+ if (vimeoMatch) return `https://player.vimeo.com/video/${vimeoMatch[1]}`;
+
+ // Reject anything that didn't match a trusted provider.
+ return null;
+}
+
</file context>
Suggested change
| return null; | |
| const ytEmbedMatch = raw.match(/youtube\.com\/embed\/([a-zA-Z0-9_-]{11})/); | |
| if (ytEmbedMatch) return `https://www.youtube.com/embed/${ytEmbedMatch[1]}`; | |
| const vimeoEmbedMatch = raw.match(/player\.vimeo\.com\/video\/(\d+)/); | |
| if (vimeoEmbedMatch) return `https://player.vimeo.com/video/${vimeoEmbedMatch[1]}`; | |
| return null; |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
spire/loaders/BlogpostListing.ts (1)
28-35: ExtractparseIntParamto a shared utility to reduce duplication.This function is identical to the one in
BlogpostList.ts. Consider moving it to a shared location likespire/utils/params.ts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spire/loaders/BlogpostListing.ts` around lines 28 - 35, Extract the parseIntParam helper into a shared utility module (e.g., create a new function export in spire/utils/params.ts) and replace the local definitions in BlogpostListing (function parseIntParam) and BlogpostList with an import from that module; ensure the exported name is parseIntParam, keep the same signature (value: number | string | null | undefined, fallback: number): number, update both files to import { parseIntParam } and remove the duplicated function, and run type checks to confirm no API changes.spire/loaders/BlogPostPage.ts (2)
71-74: Synthetic email addresses may cause confusion.The email
${a.slug}@spire.blogis fabricated and won't be a real email. Consider usingundefinedinstead, or add a comment clarifying this is a placeholder.♻️ Proposed change
authors: post.authors.map((a) => ({ name: a.name, - email: `${a.slug}@spire.blog`, + email: undefined, // Spire API doesn't provide author emails avatar: a.avatarUrl ?? undefined, })),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spire/loaders/BlogPostPage.ts` around lines 71 - 74, The authors mapping in BlogPostPage.ts currently assigns a synthetic email `${a.slug}@spire.blog` which is misleading; change the assignment in the authors: post.authors.map((a) => ({ ... })) block to use undefined for the email field (email: undefined) or, if you must keep a placeholder, replace it with a clearly marked comment and a constant like email: undefined /* placeholder: synthetic email removed */ so consumers don't treat it as real; update references to a.slug only for non-email uses and ensure avatar uses a.avatarUrl ?? undefined as before.
77-77: Date fallback tonew Date()may cause cache key instability.If
publishedAtis null, every request generates a new timestamp, which could cause issues with 24-hour caching if the post data is otherwise identical.♻️ Proposed fix: use a stable fallback
- date: post.publishedAt ?? new Date().toISOString(), + date: post.publishedAt ?? "",Or use a fixed epoch value if a non-empty date is required.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spire/loaders/BlogPostPage.ts` at line 77, The current fallback for the `date` field uses `new Date().toISOString()` (in the BlogPostPage loader assignment `date: post.publishedAt ?? new Date().toISOString()`), which produces a different value on every request and breaks cache stability; change the fallback to a stable value (e.g., `post.createdAt`, a fixed epoch string like `"1970-01-01T00:00:00.000Z"`, or `null`/empty string if allowed) so the `date` property is deterministic when `post.publishedAt` is missing and caching won't be invalidated each request.spire/loaders/BlogpostList.ts (1)
24-31: Consider adding an upper bound tocountto prevent excessive API requests.
parseIntParamvalidates that values are positive integers but doesn't cap them. A malicious or misconfiguredcount=999999would be passed directly to the API.♻️ Proposed enhancement
function parseIntParam( value: number | string | null | undefined, fallback: number, + max?: number, ): number { const n = parseInt(String(value ?? ""), 10); - return Number.isFinite(n) && n > 0 ? n : fallback; + const valid = Number.isFinite(n) && n > 0 ? n : fallback; + return max !== undefined ? Math.min(valid, max) : valid; }Then use
parseIntParam(count ?? ..., 12, 100)to cap at a reasonable maximum.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spire/loaders/BlogpostList.ts` around lines 24 - 31, The parseIntParam function accepts an unbounded positive integer which allows huge values (e.g., count=999999) to be passed to downstream APIs; modify parseIntParam to accept an optional max parameter (e.g., parseIntParam(value, fallback, max)) and clamp the parsed result to the range [1, max] returning fallback if out of range/NaN, then update call sites (e.g., where parseIntParam is used for count) to pass a reasonable upper bound like 100 (call parseIntParam(count ?? ..., 12, 100)).spire/mod.ts (1)
49-60: Consider validating thataccountis non-empty.If
accountis an empty string, API calls will fail silently. A validation check or default could improve the developer experience.♻️ Proposed enhancement
export default function Spire({ account, overrideSections }: Props) { + if (!account?.trim()) { + throw new Error("Spire app requires a non-empty account slug"); + } + const api = createHttpClient<SpireApi>({🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spire/mod.ts` around lines 49 - 60, Add a runtime validation for Props.account to ensure it is a non-empty string: when Props is received (e.g., in the component/initializer that consumes the Props interface), check Props.account.trim() !== "" and either throw a clear Error (e.g., "spire: account must be a non-empty string") or provide a sensible default/fallback and log a warning; update the code paths that construct or accept Props to perform this validation before making API calls so empty account values cannot proceed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@spire/loaders/BlogpostList.ts`:
- Line 22: Add the missing semicolon to the exported constant declaration to
satisfy formatting: update the export of the `cache` constant (export const
cache = "no-cache") by terminating the statement with a semicolon so `deno fmt
--check` passes.
In `@spire/loaders/BlogpostListing.ts`:
- Around line 74-77: Add a short inline comment explaining that the empty
seo.title in the loader is intentional as a fallback because the
SeoBlogPostListing component uses a Title Override prop that takes precedence;
update the object constructed in the BlogpostListing loader (the seo: { title:
"", canonical: ... } block) to include a comment like "Intentional default;
override via SeoBlogPostListing's title prop" next to title to make the intent
explicit for future readers.
In `@vtex/loaders/legacy/productList.ts`:
- Around line 139-140: Delete the stray debug statement console.log("lEGACY",
props); from vtex/loaders/legacy/productList.ts (it appears inside the product
list loader where props is available); remove the line entirely (or replace with
a proper structured logger call if intentional) and search for the "lEGACY"
string to ensure no other debug logs remain before merging.
---
Nitpick comments:
In `@spire/loaders/BlogpostList.ts`:
- Around line 24-31: The parseIntParam function accepts an unbounded positive
integer which allows huge values (e.g., count=999999) to be passed to downstream
APIs; modify parseIntParam to accept an optional max parameter (e.g.,
parseIntParam(value, fallback, max)) and clamp the parsed result to the range
[1, max] returning fallback if out of range/NaN, then update call sites (e.g.,
where parseIntParam is used for count) to pass a reasonable upper bound like 100
(call parseIntParam(count ?? ..., 12, 100)).
In `@spire/loaders/BlogpostListing.ts`:
- Around line 28-35: Extract the parseIntParam helper into a shared utility
module (e.g., create a new function export in spire/utils/params.ts) and replace
the local definitions in BlogpostListing (function parseIntParam) and
BlogpostList with an import from that module; ensure the exported name is
parseIntParam, keep the same signature (value: number | string | null |
undefined, fallback: number): number, update both files to import {
parseIntParam } and remove the duplicated function, and run type checks to
confirm no API changes.
In `@spire/loaders/BlogPostPage.ts`:
- Around line 71-74: The authors mapping in BlogPostPage.ts currently assigns a
synthetic email `${a.slug}@spire.blog` which is misleading; change the
assignment in the authors: post.authors.map((a) => ({ ... })) block to use
undefined for the email field (email: undefined) or, if you must keep a
placeholder, replace it with a clearly marked comment and a constant like email:
undefined /* placeholder: synthetic email removed */ so consumers don't treat it
as real; update references to a.slug only for non-email uses and ensure avatar
uses a.avatarUrl ?? undefined as before.
- Line 77: The current fallback for the `date` field uses `new
Date().toISOString()` (in the BlogPostPage loader assignment `date:
post.publishedAt ?? new Date().toISOString()`), which produces a different value
on every request and breaks cache stability; change the fallback to a stable
value (e.g., `post.createdAt`, a fixed epoch string like
`"1970-01-01T00:00:00.000Z"`, or `null`/empty string if allowed) so the `date`
property is deterministic when `post.publishedAt` is missing and caching won't
be invalidated each request.
In `@spire/mod.ts`:
- Around line 49-60: Add a runtime validation for Props.account to ensure it is
a non-empty string: when Props is received (e.g., in the component/initializer
that consumes the Props interface), check Props.account.trim() !== "" and either
throw a clear Error (e.g., "spire: account must be a non-empty string") or
provide a sensible default/fallback and log a warning; update the code paths
that construct or accept Props to perform this validation before making API
calls so empty account values cannot proceed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e1684ca5-332e-4286-9a72-42d5476cf8c2
📒 Files selected for processing (11)
spire/loaders/BlogPostPage.tsspire/loaders/BlogpostList.tsspire/loaders/BlogpostListing.tsspire/mod.tsspire/sections/blocks/CardGroup.tsxspire/sections/blocks/Checklist.tsxspire/sections/blocks/StatGroup.tsxspire/sections/blocks/Steps.tsxspire/sections/blocks/Video.tsxspire/utils/blocksToSections.tsvtex/loaders/legacy/productList.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- spire/sections/blocks/StatGroup.tsx
- spire/sections/blocks/Video.tsx
| * @title BlogpostList | ||
| * @description Retrieves a list of Spire blog posts. | ||
| */ | ||
| export const cache = "no-cache" |
Contributor
There was a problem hiding this comment.
Fix formatting: missing semicolon causing CI failure.
The pipeline reports that deno fmt --check failed on this line.
🐛 Proposed fix
-export const cache = "no-cache"
+export const cache = "no-cache";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export const cache = "no-cache" | |
| export const cache = "no-cache"; |
🧰 Tools
🪛 GitHub Actions: ci
[error] 22-22: Deno fmt --check failed. File is not formatted: expected 'export const cache = "no-cache";' (formatting difference reported on line 22).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@spire/loaders/BlogpostList.ts` at line 22, Add the missing semicolon to the
exported constant declaration to satisfy formatting: update the export of the
`cache` constant (export const cache = "no-cache") by terminating the statement
with a semicolon so `deno fmt --check` passes.
Comment on lines
+74
to
+77
| seo: { | ||
| title: "", | ||
| canonical: new URL(url.pathname, url.origin).href, | ||
| }, |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how seo.title is used in SEO components
rg -n "seo\?.title|seo\.title" spire/ -A 2 -B 2Repository: deco-cx/apps
Length of output: 771
🏁 Script executed:
# View the BlogpostListing.ts file around the seo section
cat -n spire/loaders/BlogpostListing.ts | sed -n '60,90p'Repository: deco-cx/apps
Length of output: 961
🏁 Script executed:
# Check how SeoBlogPostListing.tsx uses the data from the loader
cat -n spire/sections/Seo/SeoBlogPostListing.tsxRepository: deco-cx/apps
Length of output: 2038
🏁 Script executed:
# Look for any comments or context around the seo object in BlogpostListing
rg -n "seo\s*:\s*\{" spire/loaders/BlogpostListing.ts -A 5 -B 5Repository: deco-cx/apps
Length of output: 292
🏁 Script executed:
# Find usages of SeoBlogPostListing component to see if titleProp is always provided
rg -n "SeoBlogPostListing" spire/ -A 3 -B 1Repository: deco-cx/apps
Length of output: 682
🏁 Script executed:
# Also check if there's any documentation or usage pattern for BlogpostListing loader
rg -n "BlogpostListing" spire/ -A 2 -B 1 | head -50Repository: deco-cx/apps
Length of output: 1098
🏁 Script executed:
# Search for template or page files that might instantiate SeoBlogPostListing with or without titleProp
find spire -type f \( -name "*.tsx" -o -name "*.ts" \) | xargs grep -l "SeoBlogPostListing" | head -10Repository: deco-cx/apps
Length of output: 77
🏁 Script executed:
# Look for actual page/template instantiations that use the loader
rg -n "BlogpostListing" spire --type tsx --type ts -C 5 | grep -A 10 -B 10 "title"Repository: deco-cx/apps
Length of output: 83
🏁 Script executed:
# Find the BlogPostListingPage type definition
rg -n "BlogPostListingPage" spire/ -A 5 -B 2Repository: deco-cx/apps
Length of output: 2431
🏁 Script executed:
# Check if there are any example files or tests that show usage
find spire -type f \( -name "*example*" -o -name "*test*" -o -name "*demo*" \) 2>/dev/nullRepository: deco-cx/apps
Length of output: 38
🏁 Script executed:
# Look at the complete BlogpostListing.ts file to see if there's any documentation
cat -n spire/loaders/BlogpostListing.tsRepository: deco-cx/apps
Length of output: 3826
Add a comment to explain the intentional empty seo.title in the loader.
The empty title is by design—the SeoBlogPostListing section component receives the listing data from this loader and provides a "Title Override" prop that takes precedence. The empty string serves as the fallback in the chain: titleProp || jsonLD?.seo?.title || "". However, the loader code should include a comment clarifying this intention, such as:
seo: {
title: "", // Intentional default; override via SeoBlogPostListing's title prop
canonical: new URL(url.pathname, url.origin).href,
},
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@spire/loaders/BlogpostListing.ts` around lines 74 - 77, Add a short inline
comment explaining that the empty seo.title in the loader is intentional as a
fallback because the SeoBlogPostListing component uses a Title Override prop
that takes precedence; update the object constructed in the BlogpostListing
loader (the seo: { title: "", canonical: ... } block) to include a comment like
"Intentional default; override via SeoBlogPostListing's title prop" next to
title to make the intent explicit for future readers.
… and correctness - Add sanitizeHtml/sanitizeHref utility (SSR-safe, no external deps) and apply it to all block components that use dangerouslySetInnerHTML: Callout, CardGroup, Checklist, Comparison, Heading, List, Paragraph, Steps - Cta: validate href scheme via sanitizeHref; add target/rel for external links - Comparison: normalize parsed JSON shape (title/items) with runtime guards so items is always an array before .map() - BlogpostList: clamp count to MAX_COUNT (100) in both cacheKey and loader - BlogPostPage: use "" as date fallback (consistent with BlogpostList); log errors before returning null; replace fabricated author email with "" - BlogpostListing: return empty listing instead of null when posts is empty; set seo.title from blog.name with "Blog" fallback - SeoBlogPost/SeoBlogPostListing: narrow ctx.seo titleTemplate and descriptionTemplate to string via typeof guards before renderTemplateString - blocksToSections: add comment clarifying override intent (use store resolveType, keep API props) Made-with: Cursor
viktormarinho
approved these changes
Apr 2, 2026
There was a problem hiding this comment.
3 issues found across 16 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="spire/utils/sanitizeHtml.ts">
<violation number="1" location="spire/utils/sanitizeHtml.ts:17">
P1: XSS bypass via HTML entity encoding: `href="javascript:alert(1)"` evades the literal `javascript` check but the browser decodes entities before interpreting the protocol. Regex-based protocol detection on raw HTML cannot reliably catch encoded variants. Consider decoding HTML entities in attribute values before matching, or using a DOM-based sanitizer (e.g., DOMPurify or the Sanitizer API) that operates on parsed HTML rather than raw strings.</violation>
<violation number="2" location="spire/utils/sanitizeHtml.ts:21">
P1: XSS bypass: the `javascript:`/`data:` protocol stripping only handles quoted attribute values. Unquoted values like `<a href=javascript:alert(1)>` pass through unsanitized and execute when rendered via `dangerouslySetInnerHTML`. Add a third `.replace()` for unquoted attribute values.</violation>
</file>
<file name="spire/sections/blocks/Comparison.tsx">
<violation number="1" location="spire/sections/blocks/Comparison.tsx:19">
P2: `items` is not narrowed to strings before calling `sanitizeHtml(item)`, so non-string array entries can cause a runtime crash (`raw.replace is not a function`).</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| .replace(/<style\b[\s\S]*?<\/style\s*>/gi, "") | ||
| .replace(/\s+on\w+\s*=\s*(?:"[^"]*"|'[^']*'|[^\s>]+)/gi, "") | ||
| .replace( | ||
| /\b(href|src|action)\s*=\s*"(?:javascript|data)[^"]*"/gi, |
There was a problem hiding this comment.
P1: XSS bypass via HTML entity encoding: href="javascript:alert(1)" evades the literal javascript check but the browser decodes entities before interpreting the protocol. Regex-based protocol detection on raw HTML cannot reliably catch encoded variants. Consider decoding HTML entities in attribute values before matching, or using a DOM-based sanitizer (e.g., DOMPurify or the Sanitizer API) that operates on parsed HTML rather than raw strings.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At spire/utils/sanitizeHtml.ts, line 17:
<comment>XSS bypass via HTML entity encoding: `href="javascript:alert(1)"` evades the literal `javascript` check but the browser decodes entities before interpreting the protocol. Regex-based protocol detection on raw HTML cannot reliably catch encoded variants. Consider decoding HTML entities in attribute values before matching, or using a DOM-based sanitizer (e.g., DOMPurify or the Sanitizer API) that operates on parsed HTML rather than raw strings.</comment>
<file context>
@@ -0,0 +1,43 @@
+ .replace(/<style\b[\s\S]*?<\/style\s*>/gi, "")
+ .replace(/\s+on\w+\s*=\s*(?:"[^"]*"|'[^']*'|[^\s>]+)/gi, "")
+ .replace(
+ /\b(href|src|action)\s*=\s*"(?:javascript|data)[^"]*"/gi,
+ '$1="#"',
+ )
</file context>
| '$1="#"', | ||
| ) | ||
| .replace( | ||
| /\b(href|src|action)\s*=\s*'(?:javascript|data)[^']*'/gi, |
There was a problem hiding this comment.
P1: XSS bypass: the javascript:/data: protocol stripping only handles quoted attribute values. Unquoted values like <a href=javascript:alert(1)> pass through unsanitized and execute when rendered via dangerouslySetInnerHTML. Add a third .replace() for unquoted attribute values.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At spire/utils/sanitizeHtml.ts, line 21:
<comment>XSS bypass: the `javascript:`/`data:` protocol stripping only handles quoted attribute values. Unquoted values like `<a href=javascript:alert(1)>` pass through unsanitized and execute when rendered via `dangerouslySetInnerHTML`. Add a third `.replace()` for unquoted attribute values.</comment>
<file context>
@@ -0,0 +1,43 @@
+ '$1="#"',
+ )
+ .replace(
+ /\b(href|src|action)\s*=\s*'(?:javascript|data)[^']*'/gi,
+ "$1='#'",
+ );
</file context>
| const parsed = JSON.parse(left ?? "{}"); | ||
| leftCol = { | ||
| title: typeof parsed?.title === "string" ? parsed.title : "Option A", | ||
| items: Array.isArray(parsed?.items) ? parsed.items : [], |
There was a problem hiding this comment.
P2: items is not narrowed to strings before calling sanitizeHtml(item), so non-string array entries can cause a runtime crash (raw.replace is not a function).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At spire/sections/blocks/Comparison.tsx, line 19:
<comment>`items` is not narrowed to strings before calling `sanitizeHtml(item)`, so non-string array entries can cause a runtime crash (`raw.replace is not a function`).</comment>
<file context>
@@ -11,10 +13,18 @@ export default function Comparison({ left, right }: Props) {
+ const parsed = JSON.parse(left ?? "{}");
+ leftCol = {
+ title: typeof parsed?.title === "string" ? parsed.title : "Option A",
+ items: Array.isArray(parsed?.items) ? parsed.items : [],
+ };
} catch { /* ignore */ }
</file context>
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
spire/app integrating with the Spire Blog APIoverrideSectionsprop (array of{ key: BlockType, value: Section }) to the app config, letting users replace any default block renderer with a custom section via the deco admin UIkeyis a typed union of all 19 block types (renders as a select in the admin),valueis any decoSectionHow it works
When a block type is matched in
blocksToSections, the override map is checked first. If a match exists, the user-provided section is used instead of the defaultspire/sections/blocks/*component.Test plan
key: "product-shelf",value: custom section) and confirm the custom section renders in place of the defaultkeyfield renders as a dropdown in the deco admin UISummary by cubic
Adds the
spireblog app with Spire API integration and safe block overrides viaoverrideSections. Adds loaders, default blocks, SEO, and caching; now sanitizes CMS HTML/URLs and fixes pagination/SEO edge cases.New Features
app("spire"); adds loaders:BlogPostPage,BlogpostListing,BlogpostList(24h cache).overrideSections: map block types to anySection; overrides run before defaults.Templatefor preview.page/countprops or query params; cache keys includeaccountand page/count/slug.Bug Fixes
sanitizeHtml/sanitizeHref; applied to all HTML-rendering blocks;Ctavalidates external links and sets target/rel.countto max 100 in cacheKey and loader;BlogPostPageuses "" as date fallback and empty author email; listing uses blog name or "Blog" for SEO and handles empty results.blocksToSections(use store__resolveType, keep API props).Written for commit 980fe1c. Summary will update on new commits.
Summary by CodeRabbit