63 landing page prefetching - #64
Conversation
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
corates | 5f78515 | Commit Preview URL | Dec 16 2025, 04:29 PM |
|
Caution Review failedThe pull request is closed. WalkthroughAdds a '/security' landing page with SEO and sitemap updates; introduces PrefetchLink and replaces internal anchor links for prefetching; switches product images to WebP; adds conditional HSTS in middleware; and strengthens PDF routes with filename validation and viewer-role upload/delete restrictions. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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: 4
🧹 Nitpick comments (4)
packages/workers/src/middleware/securityHeaders.js (1)
14-22: Consider extending HSTS max-age and evaluating preload.The conditional HSTS header is correctly applied only over HTTPS, which prevents protocol downgrade warnings. However:
- The 6-month max-age (15552000 seconds) is moderate; production sites typically use 1-2 years (31536000+).
- The
includeSubDomainsdirective will enforce HTTPS on all subdomains—ensure all subdomains support HTTPS before deploying.- Consider adding
preloaddirective if you plan to submit to the HSTS preload list for stronger protection.Apply this diff if extending to 2 years is appropriate:
- c.header('Strict-Transport-Security', 'max-age=15552000; includeSubDomains'); + c.header('Strict-Transport-Security', 'max-age=63072000; includeSubDomains');Or with preload (only if committing to preload list):
- c.header('Strict-Transport-Security', 'max-age=15552000; includeSubDomains'); + c.header('Strict-Transport-Security', 'max-age=63072000; includeSubDomains; preload');packages/landing/src/components/PrefetchLink.jsx (2)
5-13: Improve URL filtering and consider edge cases.The current implementation has a few limitations:
- Line 6:
!href.startsWith('/')will reject absolute URLs to the same origin (e.g.,https://corates.org/about), even though they could be prefetched. Consider checking against the current origin instead.- Line 6: Hash-only URLs like
#featureswill pass the filter and create unnecessary prefetch links. Add validation to skip fragment-only navigation.- No SSR guard: If this runs during SSR,
documentwill be undefined. Consider wrapping in anisServercheck or usingtypeof document !== 'undefined'.Here's an improved version:
function prefetch(href) { - if (prefetched.has(href) || !href.startsWith('/')) return; + // Skip if already prefetched, external, hash-only, or SSR + if ( + prefetched.has(href) || + !href || + href.startsWith('#') || + href.startsWith('http://') || + href.startsWith('https://') || + typeof document === 'undefined' + ) { + return; + } prefetched.add(href); const link = document.createElement('link'); link.rel = 'prefetch'; link.href = href; + link.as = 'document'; document.head.appendChild(link); }Alternatively, for same-origin absolute URLs:
function prefetch(href) { if ( prefetched.has(href) || !href || href.startsWith('#') || typeof document === 'undefined' ) { return; } // Check if URL is same-origin try { const url = new URL(href, window.location.origin); if (url.origin !== window.location.origin) return; } catch { // Invalid URL, skip return; } prefetched.add(href); const link = document.createElement('link'); link.rel = 'prefetch'; link.href = href; link.as = 'document'; document.head.appendChild(link); }
15-29: Consider extracting hover logic and handling focus.The component correctly uses
splitPropsto avoid breaking reactivity. A few optional enhancements:
- Focus prefetching: Consider adding
onFocusin addition toonMouseEnterto support keyboard navigation and improve accessibility.- Touch devices:
onMouseEnterwon't fire on touch devices. Consider addingonTouchStartfor mobile optimization.Example with focus and touch support:
export default function PrefetchLink(props) { const [local, others] = splitProps(props, ['href', 'children', 'class']); - const handleMouseEnter = () => { + const handlePrefetch = () => { if (local.href) { prefetch(local.href); } }; return ( - <a href={local.href} class={local.class} onMouseEnter={handleMouseEnter} {...others}> + <a + href={local.href} + class={local.class} + onMouseEnter={handlePrefetch} + onFocus={handlePrefetch} + onTouchStart={handlePrefetch} + {...others} + > {local.children} </a> ); }packages/workers/src/routes/pdfs.js (1)
54-61: Consider additional edge case validations.The validation function effectively prevents path traversal and header injection. However, consider adding checks for edge cases:
- Reject
.and..as filenames- Reject leading/trailing whitespace
- Consider checking for null bytes (
\0)Apply this diff to enhance the validation:
function isValidFileName(fileName) { if (!fileName) return false; + const trimmed = fileName.trim(); + if (trimmed !== fileName) return false; + if (trimmed === '.' || trimmed === '..') return false; if (fileName.length > 200) return false; if (/[\\/]/.test(fileName)) return false; if (/\p{C}/u.test(fileName)) return false; if (fileName.includes('"')) return false; + if (fileName.includes('\0')) return false; return true; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
packages/landing/app.config.js(1 hunks)packages/landing/public/sitemap.xml(1 hunks)packages/landing/src/components/DefaultSeo.jsx(1 hunks)packages/landing/src/components/Footer.jsx(5 hunks)packages/landing/src/components/Hero.jsx(1 hunks)packages/landing/src/components/Navbar.jsx(4 hunks)packages/landing/src/components/PrefetchLink.jsx(1 hunks)packages/landing/src/routes/security.jsx(1 hunks)packages/workers/src/middleware/securityHeaders.js(1 hunks)packages/workers/src/routes/pdfs.js(6 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,jsx,ts,tsx}: Prefer modern ES6+ syntax and features
Use aliases for imports when appropriate to improve readability
Files:
packages/workers/src/middleware/securityHeaders.jspackages/landing/src/components/PrefetchLink.jsxpackages/landing/app.config.jspackages/landing/src/routes/security.jsxpackages/landing/src/components/Footer.jsxpackages/landing/src/components/DefaultSeo.jsxpackages/landing/src/components/Navbar.jsxpackages/landing/src/components/Hero.jsxpackages/workers/src/routes/pdfs.js
packages/{web,landing}/src/**/*.{jsx,tsx,css}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use responsive design principles for UI components
Files:
packages/landing/src/components/PrefetchLink.jsxpackages/landing/src/routes/security.jsxpackages/landing/src/components/Footer.jsxpackages/landing/src/components/DefaultSeo.jsxpackages/landing/src/components/Navbar.jsxpackages/landing/src/components/Hero.jsx
🧠 Learnings (10)
📚 Learning: 2025-12-16T05:05:58.178Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-16T05:05:58.178Z
Learning: Applies to packages/web/src/**/*.{jsx,tsx,js,ts} : Create reusable logic in primitives (hooks) that can be shared across components to keep components clean and focused on rendering
Applied to files:
packages/landing/src/components/PrefetchLink.jsxpackages/landing/src/components/Navbar.jsx
📚 Learning: 2025-12-16T05:05:58.178Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-16T05:05:58.178Z
Learning: Applies to packages/web/src/**/*.{jsx,tsx} : Do not destructure props in SolidJS components as it breaks reactivity. Instead access props directly from the props object or wrap them in a function to ensure they are always up-to-date
Applied to files:
packages/landing/src/components/PrefetchLink.jsxpackages/landing/src/routes/security.jsxpackages/landing/src/components/Hero.jsx
📚 Learning: 2025-12-16T00:29:26.670Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursorrules:0-0
Timestamp: 2025-12-16T00:29:26.670Z
Learning: Applies to packages/web/src/**/*.{jsx,tsx,js,ts} : Create reusable logic in 'primitives' (hooks) that can be shared across components to keep components clean and focused on rendering
Applied to files:
packages/landing/src/components/PrefetchLink.jsxpackages/landing/src/components/Navbar.jsx
📚 Learning: 2025-12-16T00:29:26.670Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursorrules:0-0
Timestamp: 2025-12-16T00:29:26.670Z
Learning: Applies to packages/web/src/components/**/*.{jsx,tsx} : In SolidJS components, do not destructure props as it breaks reactivity - instead access props directly from the props object, or wrap them in a function to ensure they are always up-to-date
Applied to files:
packages/landing/src/components/PrefetchLink.jsx
📚 Learning: 2025-12-16T05:05:58.178Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-16T05:05:58.178Z
Learning: Applies to packages/web/src/**/*.{jsx,tsx} : Use `createMemo` for derived values based on props or state to ensure reactive updates
Applied to files:
packages/landing/src/components/PrefetchLink.jsxpackages/landing/src/components/Navbar.jsx
📚 Learning: 2025-12-16T00:29:26.670Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursorrules:0-0
Timestamp: 2025-12-16T00:29:26.670Z
Learning: Applies to packages/web/src/components/**/*.{jsx,tsx} : Do NOT prop-drill application state in SolidJS components - shared or cross-feature state must live in external stores under `packages/web/src/stores/` or relative to the component file
Applied to files:
packages/landing/src/routes/security.jsx
📚 Learning: 2025-12-16T05:05:58.178Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-16T05:05:58.178Z
Learning: Applies to packages/{web,landing}/src/**/*.{jsx,tsx,css} : Use responsive design principles for UI components
Applied to files:
packages/landing/src/components/Navbar.jsxpackages/landing/src/components/Hero.jsx
📚 Learning: 2025-12-16T00:29:26.670Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursorrules:0-0
Timestamp: 2025-12-16T00:29:26.670Z
Learning: Applies to packages/web/src/components/**/*.{jsx,tsx} : Use responsive design principles for UI components
Applied to files:
packages/landing/src/components/Navbar.jsxpackages/landing/src/components/Hero.jsx
📚 Learning: 2025-12-16T00:29:26.670Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursorrules:0-0
Timestamp: 2025-12-16T00:29:26.670Z
Learning: Applies to packages/web/src/components/**/*.{jsx,tsx} : Components should be lean and focused - do not implement business logic in components; move that into stores, utilities, or primitives
Applied to files:
packages/landing/src/components/Navbar.jsx
📚 Learning: 2025-12-16T05:05:58.178Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-16T05:05:58.178Z
Learning: Applies to packages/web/src/**/*.{jsx,tsx} : Use Zag.js for UI components and design system
Applied to files:
packages/landing/src/components/Navbar.jsxpackages/landing/src/components/Hero.jsx
🧬 Code graph analysis (5)
packages/workers/src/middleware/securityHeaders.js (1)
packages/workers/src/index.js (2)
url(131-131)c(198-198)
packages/landing/src/routes/security.jsx (2)
packages/landing/src/components/Navbar.jsx (1)
Navbar(7-112)packages/landing/src/components/Footer.jsx (1)
Footer(4-130)
packages/landing/src/components/Footer.jsx (1)
packages/landing/src/components/PrefetchLink.jsx (1)
PrefetchLink(15-29)
packages/landing/src/components/Navbar.jsx (1)
packages/landing/src/components/PrefetchLink.jsx (1)
PrefetchLink(15-29)
packages/workers/src/routes/pdfs.js (1)
packages/workers/src/config/constants.js (3)
createErrorResponse(125-134)ERROR_CODES(73-117)ERROR_CODES(73-117)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Workers Builds: corates
🔇 Additional comments (20)
packages/landing/src/components/DefaultSeo.jsx (1)
5-5: LGTM! WebP format improves social card performance.The switch from PNG to WebP for Open Graph and Twitter card images should reduce file size while maintaining quality.
packages/landing/app.config.js (1)
14-14: LGTM! Security route added for prerendering.The addition of '/security' to the prerender routes aligns with the new security page and sitemap updates.
packages/landing/public/sitemap.xml (1)
18-20: LGTM! Sitemap includes new security page.The addition of the security URL entry aligns with the route and prerender configuration.
packages/landing/src/components/Hero.jsx (1)
64-76: LGTM! Progressive image enhancement with WebP.The
<picture>element with WebP source and PNG fallback provides optimal loading performance while maintaining compatibility. Thefetchpriority='high'is appropriate for this hero/LCP image, and explicit dimensions prevent layout shift.packages/landing/src/components/Navbar.jsx (4)
5-5: LGTM! PrefetchLink imported for navigation optimization.
27-40: LGTM! Logo link upgraded to PrefetchLink.The logo link now benefits from prefetch-on-hover behavior for the home route.
43-48: LGTM! Navigation links upgraded to PrefetchLink.Internal navigation links now prefetch on hover, improving perceived navigation speed.
137-144: LGTM! Mobile menu links upgraded to PrefetchLink.Mobile navigation now benefits from prefetching, with proper onClick handling to close the menu.
packages/landing/src/components/Footer.jsx (3)
2-2: LGTM! PrefetchLink imported for footer navigation.
13-15: LGTM! Internal footer links upgraded to PrefetchLink.All internal navigation links (brand, pricing, features, about, contact, privacy, terms) now use PrefetchLink for prefetch-on-hover optimization.
Also applies to: 26-31, 34-39, 58-63, 66-71, 81-86, 89-94
96-102: LGTM! Security link added to Legal section.The new security link is correctly integrated with PrefetchLink and aligns with the new /security route.
packages/landing/src/routes/security.jsx (7)
1-6: LGTM!The imports are well-organized and properly use path aliases per coding guidelines. All necessary dependencies for SEO metadata, icons, and layout components are imported.
7-12: LGTM!The component setup follows SolidJS best practices and properly constructs SEO metadata using config values. Modern ES6+ syntax is used appropriately.
13-23: LGTM!Comprehensive SEO metadata implementation with proper canonical URL, Open Graph, and Twitter card tags. This aligns well with the PR's sitemap and SEO updates.
40-123: LGTM!The security features section uses a consistent, well-structured card layout with appropriate semantic HTML and icons. The responsive design principles with Tailwind utility classes align with the coding guidelines.
125-163: LGTM!The responsible disclosure section is comprehensive and professional, with clear guidelines for security researchers. Good use of semantic HTML and appropriate amber color scheme to signal the importance of vulnerability reporting.
165-186: LGTM!The security questions section maintains consistency with the overall page structure and provides clear contact information for general security inquiries.
189-195: LGTM!The page structure properly wraps content with Navbar and Footer components, using a flex layout to ensure proper footer positioning. The component closure is clean and correct.
packages/workers/src/routes/pdfs.js (2)
98-103: LGTM! Proper authorization guard.The permission check correctly enforces read-only access for viewers, preventing unauthorized uploads. The early placement before content processing is efficient.
249-251: LGTM! Proper RFC 5987 encoding for international characters.The Content-Disposition header now correctly supports Unicode filenames using RFC 5987 encoding while maintaining backwards compatibility with the traditional
filenameparameter. The validation ensures no header injection risk.
| if (!isValidFileName(fileName)) { | ||
| return c.json( | ||
| createErrorResponse( | ||
| ERROR_CODES.MISSING_FIELD, | ||
| 'Invalid file name. Avoid quotes, slashes, control characters, and very long names.', | ||
| ), | ||
| 400, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Consider using a more semantically appropriate error code.
The validation logic and placement are correct. However, ERROR_CODES.MISSING_FIELD is semantically incorrect here since the field is present but invalid. Consider using ERROR_CODES.FILE_INVALID_TYPE or requesting a new error code for invalid filenames.
if (!isValidFileName(fileName)) {
return c.json(
createErrorResponse(
- ERROR_CODES.MISSING_FIELD,
+ ERROR_CODES.FILE_INVALID_TYPE,
'Invalid file name. Avoid quotes, slashes, control characters, and very long names.',
),
400,
);
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/workers/src/routes/pdfs.js around lines 170 to 178, the handler
returns ERROR_CODES.MISSING_FIELD for an invalid filename which is semantically
wrong; change the returned error code to ERROR_CODES.FILE_INVALID_TYPE if that
constant exists, otherwise add a new error code (e.g.,
ERROR_CODES.INVALID_FILE_NAME) to the ERROR_CODES enum/definition and return
that here, keeping the existing message and 400 status.
| if (!isValidFileName(fileName)) { | ||
| return c.json(createErrorResponse(ERROR_CODES.MISSING_FIELD, 'Invalid file name'), 400); | ||
| } |
There was a problem hiding this comment.
Same error code semantics issue as in upload.
The validation correctly prevents path traversal attempts. However, the same error code semantics issue applies here—use a more appropriate code than ERROR_CODES.MISSING_FIELD.
if (!isValidFileName(fileName)) {
- return c.json(createErrorResponse(ERROR_CODES.MISSING_FIELD, 'Invalid file name'), 400);
+ return c.json(createErrorResponse(ERROR_CODES.FILE_INVALID_TYPE, 'Invalid file name'), 400);
}🤖 Prompt for AI Agents
In packages/workers/src/routes/pdfs.js around lines 233 to 235, the handler
returns ERROR_CODES.MISSING_FIELD when isValidFileName fails; change the
returned error code to one that correctly represents invalid/unsafe input (e.g.,
ERROR_CODES.INVALID_FIELD or ERROR_CODES.INVALID_INPUT) and update the JSON
response accordingly — if that constant doesn't exist, add a clearly named error
code constant and use it here so the semantics reflect an invalid filename/path
traversal attempt rather than a missing field.
| if (!isValidFileName(fileName)) { | ||
| return c.json(createErrorResponse(ERROR_CODES.MISSING_FIELD, 'Invalid file name'), 400); | ||
| } |
There was a problem hiding this comment.
Same error code semantics issue as in other routes.
The validation is correctly placed and prevents path traversal in delete operations. Apply the same error code fix as suggested for upload and download routes.
if (!isValidFileName(fileName)) {
- return c.json(createErrorResponse(ERROR_CODES.MISSING_FIELD, 'Invalid file name'), 400);
+ return c.json(createErrorResponse(ERROR_CODES.FILE_INVALID_TYPE, 'Invalid file name'), 400);
}📝 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.
| if (!isValidFileName(fileName)) { | |
| return c.json(createErrorResponse(ERROR_CODES.MISSING_FIELD, 'Invalid file name'), 400); | |
| } | |
| if (!isValidFileName(fileName)) { | |
| return c.json(createErrorResponse(ERROR_CODES.FILE_INVALID_TYPE, 'Invalid file name'), 400); | |
| } |
🤖 Prompt for AI Agents
In packages/workers/src/routes/pdfs.js around lines 281 to 283, the validation
branch returns ERROR_CODES.MISSING_FIELD for an invalid file name; update it to
use the correct semantics used in upload/download routes by returning
ERROR_CODES.INVALID_FIELD (and keep the 400 status and current message), so the
error code reflects an invalid field rather than a missing one.
63 landing page prefetching
2 participants
Summary by CodeRabbit
New Features
Chores
Bug Fixes / Security
✏️ Tip: You can customize this high-level summary in your review settings.