docs/src/components/ui/collapsible/CollapsibleContent.astro:17 |
P1 |
cubic-dev-ai |
A sidebar group that renders closed on the server gets inert set from the render-time state, but the disclosure click handler only toggles data-nb-state — it never removes inert. After the user expands the group… |
thread |
docs/src/components/ui/breadcrumbs/Breadcrumbs.astro:22 |
P2 |
cubic-dev-ai |
When a caller sets maxVisible below four, the fixed two-item head and tail overlap and duplicate breadcrumbs. Derive the head and tail counts from a clamped visible limit, or reject values below the four-item layout… |
thread |
docs/src/components/ui/theme-toggle/ThemeToggle.astro:24 |
P2 |
cubic-dev-ai |
The theme control changes a persistent binary state, but its dynamically updated data-nb-state is not exposed to assistive technology. Add and update aria-pressed alongside the mode state so screen readers can rep… |
thread |
docs/src/components/ui/theme-toggle/ThemeToggle.astro:16 |
P2 |
cubic-dev-ai |
When ThemeToggle is rendered inside a form, clicking it submits the form because the native button defaults to submit. Set its type to button so the theme control never triggers form submission. |
thread |
docs/src/components/ui/banner/Banner.astro:46 |
P2 |
cubic-dev-ai |
When a banner contains an unquoted or entity-encoded javascript: URL, sanitizeBannerHtml leaves it intact and set:html renders executable markup. Replace the regex filtering with a real HTML allowlist/sanitizer,… |
thread |
docs/src/components/ui/package-managers/package-managers.client.ts:32 |
P2 |
cubic-dev-ai |
When the Clipboard API rejects because of permissions or an unsupported context, this handler returns without feedback, so users cannot tell that the command was not copied. Handle an error state like `page-actions.cl… |
thread |
docs/src/components/ui/tabs/TabsTrigger.astro:18 |
P2 |
cubic-dev-ai |
The exported manual TabsTrigger/TabsContent API wires none of the ARIA tab association. The trigger has no id, aria-controls, or initial aria-selected, and the panel has no id/aria-labelledby, so the tab and… |
thread |
docs/src/components/ui/search/SearchDialog.astro:38 |
P2 |
cubic-dev-ai |
The search UI never announces status changes to screen readers. The empty-state <p data-search-empty> sits inside role="listbox" (an ARIA listbox only permits option/group descendants), and the dynamic status … |
thread |
docs/src/components/ui/steps/Step.astro:21 |
P2 |
cubic-dev-ai |
Each renders as a plain ; the step number is drawn only by the CSS ::before counter in Steps.astro. Steps.client.ts explicitly restores role="list" on the markdown for Safari/screen readers,… |
thread |
docs/src/components/Render.astro:107 |
P2 |
cubic-dev-ai |
When a partial declares a components parameter, this spread overwrites the MDX component registry, so registered components in that partial become unavailable. Spread params before components, or reject `compone… |
thread |
docs/src/components/ui/page-actions/page-actions.client.ts:39 |
P2 |
cubic-dev-ai |
When the Markdown request takes time, browsers that require transient user activation can reject this clipboard call because it runs after the asynchronous fetch. Prefetch the Markdown before the click, or invoke `wri… |
thread |
docs/src/components/ui/page-actions/page-actions.client.ts:33 |
P2 |
cubic-dev-ai |
During docs:dev, this request goes to the configured production hostname rather than the local Markdown endpoint, so Copy page fails while the domain is unavailable. Pass a relative Markdown path to the client or re… |
thread |
docs/src/components/ui/toc/MobileTOC.astro:42 |
P2 |
cubic-dev-ai |
When a heading has an empty slug, this renders a selectable entry that cannot navigate. Omit empty-slug headings from the mobile options so every option resolves to a target. |
thread |
docs/src/components/ui/theme-toggle/theme-toggle.client.ts:30 |
P2 |
cubic-dev-ai |
When localStorage.setItem throws (Safari private mode, third-party/restricted storage contexts — the exact case the catch comment targets), the catch swallows the error and __nbApplyTheme() then re-reads the uncha… |
thread |
docs/src/components/ui/search/search.client.ts:32 |
P2 |
cubic-dev-ai |
When a result is highlighted, pressing Enter on the dialog's Esc button selects the result instead of closing the dialog. handleKeydown is attached to the whole dialog; scope it to input so controls and focused re… |
thread |
docs/src/components/ui/steps/Steps.astro:70 |
P2 |
cubic-dev-ai |
When Steps wraps a tight Markdown list containing inline markup, this selector turns each inline element into its own grid item, splitting text such as Run <code>...</code> across rows. Keep inline content in one … |
thread |
docs/src/styles/prose.css:19 |
P2 |
cubic-dev-ai |
Because prose.css follows globals.css, this direct-child rule overrides component margin utilities on classed children. Aside, Card, and Banner can lose their declared top spacing; make this selector zero-sp… |
thread |
docs/src/styles/prose.css:60 |
P2 |
cubic-dev-ai |
Because prose.css follows globals.css, this component-boundary rule overrides typography utilities on classed headings and paragraphs. Card and Aside titles therefore inherit the article's 0.9375rem size instead… |
thread |
docs/src/components/ui/tabs/Tabs.astro:28 |
P2 |
cubic-dev-ai |
Manual tab composition renders an extra empty tablist, so the client binds the indicator and tab-list behavior to that empty list instead of the consumer's TabsList. Add an explicit automatic/manual mode and omit th… |
thread |
docs/src/components/ui/sidebar/sidebar.client.ts:76 |
P2 |
cubic-dev-ai |
When a user closes a group while a filter is active, clearing the filter reopens it because resetFilter unconditionally toggles every group marked as opened by the filter. Only click the trigger when it is still ope… |
thread |
docs/src/components/ui/dialog/dialog.client.ts:11 |
P2 |
cubic-dev-ai |
On every close, unlockScroll() is invoked twice: once from the MutationObserver sync() when the open attribute is removed, and once from the close event onClose. lockScroll() is only ever called once (on o… |
thread |
docs/src/components/ui/aside/Aside.astro:27 |
P3 |
cubic-dev-ai |
When title is omitted, aria-label={displayTitle} defaults to the type's label (e.g. "Note") while the same string is rendered as the visible <p> title inside the aside. Screen-reader users hear the accessible na… |
thread |
docs/src/components/ui/code/Code.astro:1 |
P3 |
cubic-dev-ai |
Nothing imports or registers this Code component, so it is never rendered and its Nimbus themes and transformers do not affect documentation code blocks. Register and use it in the MDX path where these behaviors are… |
thread |
docs/src/components/ui/banner/Banner.astro:47 |
CRITICAL |
gemini-code-assist |
Insecure HTML sanitization using regular expressions in Banner component Problem: The sanitizeBannerHtml function uses regular expressions to sanitize HTML, which is highly prone to bypasses and can lead to Cross-Si… |
thread |
Original review suggestions
cubic-dev-ai (full-PR pass) and gemini-code-assist raised the findings below on PR #106. Every one of them is in a scaffold-owned Nimbus starter file written by
@cloudflare/create-nimbus-docs@0.7.1/@cloudflare/nimbus-docs@0.12.0, not in content or configuration authored in this repository.Reviewer: cubic-dev-ai, gemini-code-assist
PR: #106
docs/src/components/ui/collapsible/CollapsibleContent.astro:17inertset from the render-time state, but the disclosure click handler only togglesdata-nb-state— it never removesinert. After the user expands the group…docs/src/components/ui/breadcrumbs/Breadcrumbs.astro:22maxVisiblebelow four, the fixed two-item head and tail overlap and duplicate breadcrumbs. Derive the head and tail counts from a clamped visible limit, or reject values below the four-item layout…docs/src/components/ui/theme-toggle/ThemeToggle.astro:24data-nb-stateis not exposed to assistive technology. Add and updatearia-pressedalongside the mode state so screen readers can rep…docs/src/components/ui/theme-toggle/ThemeToggle.astro:16ThemeToggleis rendered inside a form, clicking it submits the form because the native button defaults tosubmit. Set its type tobuttonso the theme control never triggers form submission.docs/src/components/ui/banner/Banner.astro:46javascript:URL,sanitizeBannerHtmlleaves it intact andset:htmlrenders executable markup. Replace the regex filtering with a real HTML allowlist/sanitizer,…docs/src/components/ui/package-managers/package-managers.client.ts:32docs/src/components/ui/tabs/TabsTrigger.astro:18id,aria-controls, or initialaria-selected, and the panel has noid/aria-labelledby, so the tab and…docs/src/components/ui/search/SearchDialog.astro:38<p data-search-empty>sits insiderole="listbox"(an ARIA listbox only permitsoption/groupdescendants), and the dynamic status …docs/src/components/ui/steps/Step.astro:21for Safari/screen readers,…
docs/src/components/Render.astro:107componentsparameter, this spread overwrites the MDX component registry, so registered components in that partial become unavailable. Spreadparamsbeforecomponents, or reject `compone…docs/src/components/ui/page-actions/page-actions.client.ts:39docs/src/components/ui/page-actions/page-actions.client.ts:33docs:dev, this request goes to the configured production hostname rather than the local Markdown endpoint, so Copy page fails while the domain is unavailable. Pass a relative Markdown path to the client or re…docs/src/components/ui/toc/MobileTOC.astro:42docs/src/components/ui/theme-toggle/theme-toggle.client.ts:30localStorage.setItemthrows (Safari private mode, third-party/restricted storage contexts — the exact case the catch comment targets), the catch swallows the error and__nbApplyTheme()then re-reads the uncha…docs/src/components/ui/search/search.client.ts:32handleKeydownis attached to the whole dialog; scope it toinputso controls and focused re…docs/src/components/ui/steps/Steps.astro:70Stepswraps a tight Markdown list containing inline markup, this selector turns each inline element into its own grid item, splitting text such asRun <code>...</code>across rows. Keep inline content in one …docs/src/styles/prose.css:19prose.cssfollowsglobals.css, this direct-child rule overrides component margin utilities on classed children.Aside,Card, andBannercan lose their declared top spacing; make this selector zero-sp…docs/src/styles/prose.css:60prose.cssfollowsglobals.css, this component-boundary rule overrides typography utilities on classed headings and paragraphs. Card and Aside titles therefore inherit the article's0.9375remsize instead…docs/src/components/ui/tabs/Tabs.astro:28TabsList. Add an explicit automatic/manual mode and omit th…docs/src/components/ui/sidebar/sidebar.client.ts:76resetFilterunconditionally toggles every group marked as opened by the filter. Only click the trigger when it is still ope…docs/src/components/ui/dialog/dialog.client.ts:11unlockScroll()is invoked twice: once from the MutationObserversync()when theopenattribute is removed, and once from thecloseeventonClose.lockScroll()is only ever called once (on o…docs/src/components/ui/aside/Aside.astro:27titleis omitted,aria-label={displayTitle}defaults to the type's label (e.g. "Note") while the same string is rendered as the visible<p>title inside the aside. Screen-reader users hear the accessible na…docs/src/components/ui/code/Code.astro:1Codecomponent, so it is never rendered and its Nimbus themes and transformers do not affect documentation code blocks. Register and use it in the MDX path where these behaviors are…docs/src/components/ui/banner/Banner.astro:47sanitizeBannerHtmlfunction uses regular expressions to sanitize HTML, which is highly prone to bypasses and can lead to Cross-Si…Status
Decision: Declined (for this PR)
Reason:
docs/src/components/**,docs/src/layouts/**,docs/src/styles/**, anddocs/src/components/Render.astroare the upstream Nimbus template, kept byte-aligned with it sonimbus-docs outdated/nimbus-docs diff --applycan pull upstream fixes cleanly (CLAUDE.md "Docs site", and the matching eslint / Codacy exclusions). Patching them here forks the template: every later upgrade would conflict on these files, and the fixes would silently disappear on the next--apply. The right home for these findings is upstream. Note that theBanner.astrosanitizer item is also mitigated architecturally here: banner content is author-written frontmatter rendered at build time, and this site defines no banner.Action items
cloudflare/nimbusmain; drop items already fixed there.inertdisclosure bug and thetype="button"theme toggle.nimbus-docs diff --applyupgrade, re-check which items are closed and update this issue.