external docs: build pages from registered GitHub sources#152
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a build-time external documentation pipeline with validated GitHub sources, immutable snapshots, page integration, link/image rewriting, Mermaid rendering, deterministic fingerprints, refresh automation, and related site updates. ChangesExternal documentation system
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Build as Documentation build
participant GitHub as GitHub sources
participant Loader as External docs loader
participant Markdoc
participant Artifacts as Generated artifacts
Build->>GitHub: fetch registered Markdown snapshots
GitHub-->>Loader: return immutable source content
Loader->>Markdoc: compile resolved external pages
Loader->>Artifacts: provide content for search, raw, metadata, and fingerprints
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
components/MermaidDiagram.js (1)
110-115: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winConsider sanitizing SVG before
dangerouslySetInnerHTMLas defense-in-depth.
securityLevel: 'strict'already triggers Mermaid's internal DOMPurify pass on SVG output, so this isn't directly exploitable today. Still, since this renders content sourced from external repositories, an explicitDOMPurify.sanitize(svg)call before injection would add defense-in-depth independent of Mermaid's internal behavior (recommended by security research on diagram-renderer XSS).🤖 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 `@components/MermaidDiagram.js` around lines 110 - 115, Update the render return in MermaidDiagram to sanitize the generated svg with DOMPurify immediately before assigning it to dangerouslySetInnerHTML. Import or reuse the project’s existing DOMPurify integration, while preserving Mermaid’s strict security configuration and the current diagram rendering behavior.
🤖 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 @.github/workflows/docs-ci.yml:
- Around line 38-39: Disable credential persistence for the actions/checkout
step in .github/workflows/docs-ci.yml lines 38-39 and
.github/workflows/docs-refresh.yml lines 32-33 by adding the checkout step’s
persist-credentials setting as false under with.
In @.github/workflows/docs-refresh.yml:
- Around line 110-121: Update the “Write summary” step to pass the dynamic diff
outputs and workflow values through the step’s env configuration, then reference
those environment variables inside the bash script instead of inline `${{ }}`
expansions. Preserve the existing fallback values and summary fields while
ensuring shell-safe handling of values such as reason and fingerprints.
In `@components/MermaidDiagram.js`:
- Around line 4-16: Update getMermaid so a rejected dynamic import clears
mermaidModulePromise before propagating the failure, allowing later calls to
retry the import. Preserve the existing successful-module memoization and
module.default fallback behavior.
In `@lib/external-doc-links.js`:
- Line 1: Decode the Git ref extracted from resolved.pathname in both
resolveExternalDocHref and resolveExternalDocImageSrc before reconstructing or
serializing the final GitHub URLs. Preserve the existing URL resolution and path
handling, but ensure branch names such as feature/branch are returned with
literal slashes rather than percent-encoded separators.
In `@lib/external-doc-page.js`:
- Around line 20-23: Update the frontmatter regular expression in
getMarkdownBody to accept both LF and CRLF line endings by allowing optional
carriage returns before newline characters. Preserve the existing frontmatter
matching boundaries and body-slicing behavior.
In `@lib/external-docs.js`:
- Line 147: Update the frontmatter boundary regex in lib/external-docs.js at
lines 147-147 to accept optional carriage returns before each newline,
preserving the existing capture behavior. Apply the same CRLF-aware boundary
update in lib/external-docs-loader.js at lines 10-10 so both parsing and
stripping paths handle LF and CRLF frontmatter consistently.
---
Nitpick comments:
In `@components/MermaidDiagram.js`:
- Around line 110-115: Update the render return in MermaidDiagram to sanitize
the generated svg with DOMPurify immediately before assigning it to
dangerouslySetInnerHTML. Import or reuse the project’s existing DOMPurify
integration, while preserving Mermaid’s strict security configuration and the
current diagram rendering behavior.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: c535bc07-8712-4b41-8313-08fbe5ca17ef
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (39)
.github/workflows/docs-ci.yml.github/workflows/docs-refresh.yml.gitignorecomponents/AnnouncementBanner.jscomponents/CopyRawButton.jscomponents/MermaidDiagram.jscomponents/TableOfContents.jsdocs-sources.ymldocs/remote-markdown.mdlib/compare-docs-fingerprint.jslib/external-doc-links.jslib/external-doc-page.jslib/external-docs-loader.jslib/external-docs-snapshot.jslib/external-docs.jslib/generate-docs-fingerprint.jslib/generate-external-docs-manifest.jslib/generate-last-updated.jslib/generate-raw-pages.jslib/generate-search-index.jslib/navigation.jsmarkdoc/nodes/document.markdoc.jsmarkdoc/nodes/fence.markdoc.jsmarkdoc/nodes/image.markdoc.jsmarkdoc/nodes/index.jsmarkdoc/nodes/link.markdoc.jsmarkdoc/tags/br.markdoc.jsmarkdoc/tags/index.jsnext.config.jspackage.jsonpages/_app.jspages/architecture/epds.mdpages/lexicons/hypercerts-lexicons/index.mdpages/tools/hypercerts-agent-skills.mdpages/tools/hyperindex.mdstyles/globals.csstest/external-doc-links.test.jstest/external-docs-snapshot.test.jstest/external-docs.test.js
d342a99 to
915fd80
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/docs-ci.yml (1)
65-68: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd a timeout to the
curlcommand to prevent hanging CI jobs.Network calls in CI pipelines can occasionally hang indefinitely if the target server stops responding but leaves the connection open. Consider adding a timeout (e.g.,
--max-time 60) so the workflow fails fast rather than wasting runner minutes if the staging server is unreachable.♻️ Proposed refactor
- http_code=$(curl -sS -L -w "%{http_code}" \ + http_code=$(curl -sS -L --max-time 60 -w "%{http_code}" \ -o deployed-docs-fingerprint.json \ "$DOCS_FINGERPRINT_URL")🤖 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 @.github/workflows/docs-ci.yml around lines 65 - 68, Add a finite timeout option such as --max-time 60 to the curl invocation assigning http_code in the documentation CI workflow, preserving the existing URL, output file, redirect handling, and curl_status capture.
🤖 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.
Nitpick comments:
In @.github/workflows/docs-ci.yml:
- Around line 65-68: Add a finite timeout option such as --max-time 60 to the
curl invocation assigning http_code in the documentation CI workflow, preserving
the existing URL, output file, redirect handling, and curl_status capture.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9acab48d-7422-42d6-8077-71191f98e286
📒 Files selected for processing (22)
.github/workflows/docs-ci.yml.github/workflows/docs-refresh.ymlcomponents/CopyRawButton.jscomponents/MermaidDiagram.jslib/compare-docs-fingerprint.jslib/external-doc-links.jslib/external-doc-page.jslib/external-docs-loader.jslib/external-docs-snapshot.jslib/external-docs.jslib/generate-docs-fingerprint.jslib/generate-last-updated.jslib/generate-raw-pages.jslib/generate-search-index.jsmarkdoc/nodes/document.markdoc.jsmarkdoc/nodes/fence.markdoc.jsmarkdoc/nodes/image.markdoc.jsmarkdoc/nodes/link.markdoc.jsmarkdoc/tags/br.markdoc.jsnext.config.jsstyles/globals.csstest/external-docs.test.js
🚧 Files skipped from review as they are similar to previous changes (20)
- markdoc/tags/br.markdoc.js
- markdoc/nodes/link.markdoc.js
- markdoc/nodes/image.markdoc.js
- next.config.js
- markdoc/nodes/fence.markdoc.js
- styles/globals.css
- markdoc/nodes/document.markdoc.js
- lib/external-doc-links.js
- components/CopyRawButton.js
- lib/generate-raw-pages.js
- lib/generate-search-index.js
- lib/external-doc-page.js
- lib/generate-docs-fingerprint.js
- components/MermaidDiagram.js
- lib/external-docs.js
- lib/external-docs-snapshot.js
- lib/external-docs-loader.js
- lib/compare-docs-fingerprint.js
- test/external-docs.test.js
- .github/workflows/docs-refresh.yml
Kzoeps
changed the title
Kzoeps
changed the title
aspiers
left a comment
aspiers
left a comment
There was a problem hiding this comment.
Looks great! Saw a few opportunities for improvement.
fce7345 to
c20060b
Compare
|
@coderabbitai please review |
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
components/MermaidDiagram.js (1)
85-90: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winUnsanitized
mermaid.render()output injected viadangerouslySetInnerHTML; globalsecurityLevel: 'strict'can be overridden from within the diagram source itself.Setting
securityLevel: 'strict'atmermaid.initialize()does not fully protect this pipeline: Mermaid diagrams can carry a%%{init: {"securityLevel":"loose","htmlLabels":true}}%%front-matter directive inside the diagram text that overrides the globally configured security level for that render, and there have been real sanitizer-bypass CVEs (e.g. CVE-2025-54881, CVE-2025-54880) even under default configuration. Sincesourcehere is Markdown fetched from external, non-repo-owned GitHub sources, a compromised upstream commit or an overlooked malicious PR in one of the allow-listed source repos would be able to execute arbitrary JavaScript for every visitor of the rendered page via this unsanitizeddangerouslySetInnerHTML.🛡️ Proposed fix: sanitize the SVG before injecting
+import DOMPurify from 'dompurify'; + ... const result = await mermaid.render(diagramId, source); if (cancelled) return; - setSvg(result.svg); + setSvg(DOMPurify.sanitize(result.svg, { USE_PROFILES: { svg: true, svgFilters: true } }));Please confirm the intended trust boundary for external Markdown sources (are
docs-sources.ymlentries reviewed/pinned, or can any commit torefon an allow-listed repo instantly change rendered content?) — this affects how urgently sanitization should be added.Also applies to: 127-132
🤖 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 `@components/MermaidDiagram.js` around lines 85 - 90, Sanitize the SVG returned by mermaid.render before it reaches dangerouslySetInnerHTML in the Mermaid diagram rendering flow. Apply a trusted SVG sanitizer to result.svg after Mermaid rendering, while retaining the existing mermaid.initialize configuration and render behavior; ensure diagram source directives cannot bypass the final sanitization step.Source: Linters/SAST tools
🤖 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 `@docs/remote-markdown.md`:
- Around line 94-99: Update the “Refresh workflow” documentation to replace the
nonexistent docs-refresh-pr-dry-run.yml reference with docs-ci.yml, and revise
the description to accurately reflect that workflow’s current pull-request
behavior.
---
Nitpick comments:
In `@components/MermaidDiagram.js`:
- Around line 85-90: Sanitize the SVG returned by mermaid.render before it
reaches dangerouslySetInnerHTML in the Mermaid diagram rendering flow. Apply a
trusted SVG sanitizer to result.svg after Mermaid rendering, while retaining the
existing mermaid.initialize configuration and render behavior; ensure diagram
source directives cannot bypass the final sanitization step.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: d3490fa5-5e7f-4cd5-852b-bfb8fe662eea
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (38)
.github/workflows/docs-ci.yml.github/workflows/docs-refresh.yml.gitignorecomponents/CopyRawButton.jscomponents/MermaidDiagram.jscomponents/TableOfContents.jsdocs-sources.ymldocs/remote-markdown.mdlib/compare-docs-fingerprint.jslib/external-doc-links.jslib/external-doc-page.jslib/external-docs-loader.jslib/external-docs-snapshot.jslib/external-docs.jslib/generate-docs-fingerprint.jslib/generate-external-docs-manifest.jslib/generate-last-updated.jslib/generate-raw-pages.jslib/generate-search-index.jslib/heading-ids.mjslib/navigation.jsmarkdoc/nodes/document.markdoc.jsmarkdoc/nodes/fence.markdoc.jsmarkdoc/nodes/image.markdoc.jsmarkdoc/nodes/index.jsmarkdoc/nodes/link.markdoc.jsmarkdoc/tags/br.markdoc.jsnext.config.jspackage.jsonpages/_app.jspages/architecture/epds.mdpages/tools/hyperindex.mdpages/tutorials/epds.mdstyles/globals.csstest/external-doc-links.test.jstest/external-docs-snapshot.test.jstest/external-docs.test.jstest/heading-ids.test.mjs
🚧 Files skipped from review as they are similar to previous changes (29)
- pages/_app.js
- pages/tutorials/epds.md
- markdoc/nodes/index.js
- next.config.js
- docs-sources.yml
- test/external-doc-links.test.js
- pages/architecture/epds.md
- markdoc/nodes/image.markdoc.js
- markdoc/tags/br.markdoc.js
- lib/generate-external-docs-manifest.js
- .gitignore
- markdoc/nodes/link.markdoc.js
- test/external-docs-snapshot.test.js
- lib/generate-search-index.js
- markdoc/nodes/fence.markdoc.js
- components/CopyRawButton.js
- .github/workflows/docs-refresh.yml
- markdoc/nodes/document.markdoc.js
- lib/external-docs.js
- pages/tools/hyperindex.md
- lib/navigation.js
- lib/external-docs-snapshot.js
- lib/compare-docs-fingerprint.js
- lib/external-docs-loader.js
- lib/external-doc-links.js
- lib/external-doc-page.js
- package.json
- styles/globals.css
- lib/generate-docs-fingerprint.js
17d4a21 to
dec8b0f
Compare
2 participants
check out https://hypercerts-v02-documentation-git-d-210225-hypercerts-foundation.vercel.app/architecture/epds for one of the pages dynamically rendered
Summary
Testing
npm test(24 tests passed)Summary by CodeRabbit
Summary by CodeRabbit