Added logic for alias recognition to VitePress#925
Added logic for alias recognition to VitePress#925n-boshnakov wants to merge 3 commits intogardener:masterfrom
Conversation
|
@n-boshnakov: The label(s) DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
✅ Deploy Preview for gardener-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
gardener-prow
bot
added
do-not-merge/needs-kind
📝 WalkthroughWalkthroughAdds an alias-redirect feature: parses Markdown frontmatter Changes
Sequence Diagram(s)sequenceDiagram
participant MF as Markdown Files
participant Build as VitePress Build
participant AM as Alias Manager
participant Dev as Dev Server
participant FS as Output Filesystem
rect rgba(100,150,200,0.5)
Note over MF,AM: Parse & map aliases
Build->>MF: Read .md from srcDir
Build->>AM: Extract frontmatter aliases
AM->>AM: Normalize paths & deduplicate
end
rect rgba(150,200,100,0.5)
Note over Dev,AM: Dev-server redirect flow
Dev->>AM: Incoming HTTP GET/HEAD (strip basePath)
AM->>AM: Lookup alias map
AM-->>Dev: 302 Location -> target path
end
rect rgba(200,150,100,0.5)
Note over Build,FS: Production redirect generation
Build->>AM: Trigger buildEnd(siteConfig)
AM->>FS: Write static redirect HTML files to outDir
FS-->>Build: Redirect files included in output
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.vitepress/config.mts (1)
222-299: Consider adding type annotations for Vite server APIs to improve IDE support.The dev redirect plugin logic is sound with good caching, proper cache invalidation on file changes, and correct middleware ordering (calls
next()when no redirect is found, allowing other handlers to process the request). Error handling gracefully falls through to the next middleware.The
anytypes forserver,req, andresreduce type safety. Vite and Node.js provide standard type exports that should be used:🛠️ Optional: Add type imports for better IDE support
+import type { ViteDevServer } from 'vite' +import type { IncomingMessage, ServerResponse } from 'node:http' function createAliasRedirectDevPlugin(srcDir: string, basePath: string) { // ... return { name: 'vitepress-aliases-dev-redirects', apply: 'serve', - configureServer(server: any) { + configureServer(server: ViteDevServer) { // ... - server.middlewares.use((req: any, res: any, next: () => void) => { + server.middlewares.use((req: IncomingMessage, res: ServerResponse, next: () => void) => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.vitepress/config.mts around lines 222 - 299, The plugin uses untyped any for Vite server and HTTP req/res; add proper type annotations to improve IDE support: import ViteDevServer from 'vite' and use it for the configureServer(server) parameter in createAliasRedirectDevPlugin, and annotate the middleware handler parameters (req, res, next) with appropriate Node types (http.IncomingMessage / http.ServerResponse) or Connect types so IDEs can resolve properties like url and method; update signatures for configureServer(server: ViteDevServer) and the middleware callback inside configureServer (and refreshOnMarkdownChange/loadAliases references remain the same) and adjust any watcher/middleware usages to match the typed APIs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.vitepress/config.mts:
- Around line 28-45: createRedirectHtml currently embeds targetPath via
JSON.stringify directly into a <script>, which allows breaking out if targetPath
contains "</script>" (XSS). Fix by creating a script-safe string (e.g., let
safeScript = JSON.stringify(targetPath).replace(/<\/script>/gi,
'<\\/script>').replace(/\u2028/g, '\\u2028').replace(/\u2029/g, '\\u2029')) and
use safeScript inside the window.location.replace(...) call while keeping the
HTML-escaped escapedTarget for the meta/link/body; update createRedirectHtml to
use this safeScript instead of JSON.stringify(targetPath).
---
Nitpick comments:
In @.vitepress/config.mts:
- Around line 222-299: The plugin uses untyped any for Vite server and HTTP
req/res; add proper type annotations to improve IDE support: import
ViteDevServer from 'vite' and use it for the configureServer(server) parameter
in createAliasRedirectDevPlugin, and annotate the middleware handler parameters
(req, res, next) with appropriate Node types (http.IncomingMessage /
http.ServerResponse) or Connect types so IDEs can resolve properties like url
and method; update signatures for configureServer(server: ViteDevServer) and the
middleware callback inside configureServer (and
refreshOnMarkdownChange/loadAliases references remain the same) and adjust any
watcher/middleware usages to match the typed APIs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
.vitepress/plugins/alias-redirects.ts (1)
11-21:⚠️ Potential issue | 🔴 CriticalMake the inline redirect script safe for
</script>sequences.
JSON.stringify()is not enough inside a<script>tag. If a page path or configured base ever contains</script>, the generated redirect page becomes executable markup.🔒 Proposed fix
const createRedirectHtml = (targetPath: string) => { const escapedTarget = escapeHtml(targetPath) + const safeScriptTarget = JSON.stringify(targetPath) + .replace(/</g, '\\u003c') + .replace(/\u2028/g, '\\u2028') + .replace(/\u2029/g, '\\u2029') return `<!doctype html> <html lang="en"> <head> <meta charset="utf-8"> <title>Redirecting...</title> <meta http-equiv="refresh" content="0; url=${escapedTarget}"> <link rel="canonical" href="${escapedTarget}"> - <script>window.location.replace(${JSON.stringify(targetPath)})</script> + <script>window.location.replace(${safeScriptTarget})</script> </head>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.vitepress/plugins/alias-redirects.ts around lines 11 - 21, The inline redirect script in createRedirectHtml is unsafe because JSON.stringify(targetPath) alone can still allow a closing "</script>" sequence to break out of the script tag; update createRedirectHtml to sanitize the string used inside the script by escaping any "</script>" occurrences (e.g. replace "</script>" with "<\/script>") after JSON.stringify(targetPath) (and likewise ensure escapedTarget used in attributes is properly HTML-escaped), so the generated script and markup cannot be terminated by a malicious path value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.vitepress/plugins/alias-redirects.ts:
- Around line 98-107: Reject aliases containing dot-segments before joining them
into outDir: in toRedirectOutputPath, after computing normalizedAliasPath check
for any ".." path segment (e.g., normalizedAliasPath === '/..' or
normalizedAliasPath.includes('/..') or by splitting on '/' and detecting '..')
and return null (or otherwise refuse to create a redirect) if found to prevent
path traversal; apply the same validation to the other path-joining code in this
file that builds redirect output paths (the second occurrence around the other
redirect helper) so no alias can escape siteConfig.outDir.
---
Duplicate comments:
In @.vitepress/plugins/alias-redirects.ts:
- Around line 11-21: The inline redirect script in createRedirectHtml is unsafe
because JSON.stringify(targetPath) alone can still allow a closing "</script>"
sequence to break out of the script tag; update createRedirectHtml to sanitize
the string used inside the script by escaping any "</script>" occurrences (e.g.
replace "</script>" with "<\/script>") after JSON.stringify(targetPath) (and
likewise ensure escapedTarget used in attributes is properly HTML-escaped), so
the generated script and markup cannot be terminated by a malicious path value.
🪄 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: 62231597-0a4f-477b-88b4-66e7b226caf4
📒 Files selected for processing (2)
.vitepress/config.mts.vitepress/plugins/alias-redirects.ts
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.vitepress/plugins/alias-redirects.ts (1)
57-60:⚠️ Potential issue | 🟠 MajorReject
.segments too.
path.join()normalizes the joined path, including.segments, so aliases like/./or/docs/./faq/still resolve insideoutDirand can overwrite the root or a real page. The current helper only blocks... (nodejs.org)🛡️ Proposed fix
function hasDotSegment(urlPath: string): boolean { - if (urlPath === '/..' || urlPath.includes('/../')) return true - return urlPath.split('/').some((segment) => segment === '..') + return urlPath.split('/').some((segment) => segment === '.' || segment === '..') }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.vitepress/plugins/alias-redirects.ts around lines 57 - 60, The helper hasDotSegment currently only rejects '..' segments; update it to also reject single-dot segments so paths containing '/./', '/.' or '/docs/./faq' are considered invalid. Locate the hasDotSegment function and change its logic to check segments for segment === '.' || segment === '..' (e.g., split on '/' and test for either '.' or '..'), ensuring edge cases like a trailing '/.' or occurrences anywhere in the path are caught.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.vitepress/plugins/alias-redirects.ts:
- Around line 57-60: The helper hasDotSegment currently only rejects '..'
segments; update it to also reject single-dot segments so paths containing
'/./', '/.' or '/docs/./faq' are considered invalid. Locate the hasDotSegment
function and change its logic to check segments for segment === '.' || segment
=== '..' (e.g., split on '/' and test for either '.' or '..'), ensuring edge
cases like a trailing '/.' or occurrences anywhere in the path are caught.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7a662704-9272-4c50-995b-1820e31a34f8
📒 Files selected for processing (1)
.vitepress/plugins/alias-redirects.ts
1 participant
How to categorize this PR?
/kind documentation
What this PR does / why we need it:
This PR adds alias redirect infrastructure to the website.
Which issue(s) this PR fixes:
Fixes #926
Special notes for your reviewer:
Summary by CodeRabbit
New Features
Refactor