fix(web): gate the dashboard on the server, repair two nav links - #19
Open
hallelx2 wants to merge 1 commit into
Open
fix(web): gate the dashboard on the server, repair two nav links#19hallelx2 wants to merge 1 commit into
hallelx2 wants to merge 1 commit into
Conversation
Three issues, one of them structural. 1. The dashboard had no server-side authentication. (dashboard)/layout.tsx was a client component calling useSession() and redirecting from a useEffect. That is not protection. The check runs only after the server has sent the HTML and the browser has downloaded, parsed and hydrated the whole dashboard bundle, so an unauthenticated visitor received the entire screen, saw a spinner, and was then bounced. The route existence, layout, navigation and feature set all leaked, and a user with an expired session got a flash of the UI first. It is now a server component: resolve the session, redirect if absent. An unauthenticated request never receives a dashboard response. Proof in the build output -- every /dashboard route moved from static to dynamic, which only happens because the layout now reads request headers. Worth stating plainly: customer DATA was never exposed. Every /api/dashboard route proxies through forwardToCP, which returns 401 when the vls_session cookie is missing and otherwise hands it to the control plane to validate. The leak was the shell, not the contents -- a real defect, not an incident. This is a coarse gate. Per-page checks remain the standard, but every page below is still a client component and cannot do it yet; converting them is tracked separately. 2. Docs pointed at /dashboard. Anyone looking for documentation was sent into the product, and once the gate above landed they would have been bounced to /login. Now points at docs.vectorless.store, overridable via NEXT_PUBLIC_DOCS_URL so preview deployments can target a staging docs site. 3. Pricing, FAQ and How-it-works were dead on /whitepaper. The sections do exist and carry the right ids -- but only on the landing page, and Nav renders on both. A bare "#pricing" resolves against the current document, so on /whitepaper it did nothing. Changed to "/#pricing" so the links work from either page.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Reviewer's GuideServer-side authentication is added to gate all /dashboard routes before rendering, and the main navigation’s docs and section links are corrected to point to the external docs site and to work from both the landing page and /whitepaper. Sequence diagram for server-side gating of dashboard layoutsequenceDiagram
actor User
participant Browser
participant NextApp
participant DashboardLayout
participant ServerAuth as getServerSession
participant LoginRoute as login_route
participant DashboardShell
User->>Browser: request /dashboard
Browser->>NextApp: HTTP GET /dashboard
NextApp->>DashboardLayout: render
DashboardLayout->>ServerAuth: getServerSession
alt [session exists]
ServerAuth-->>DashboardLayout: session
DashboardLayout->>DashboardShell: render children
DashboardShell-->>Browser: dashboard HTML
else [no session]
ServerAuth-->>DashboardLayout: null
DashboardLayout->>NextApp: redirect /login
NextApp-->>Browser: 307 redirect /login
Browser->>LoginRoute: follow /login
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
📝 WalkthroughWalkthroughThe dashboard layout now performs server-side session checks and redirects unauthenticated requests. Navigation uses root-relative section anchors and a configurable external documentation URL. ChangesDashboard navigation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant DashboardLayout
participant getServerSession
participant Login
Browser->>DashboardLayout: request dashboard route
DashboardLayout->>getServerSession: read server session
getServerSession-->>DashboardLayout: session or no session
DashboardLayout->>Login: redirect unauthenticated request to /login
DashboardLayout-->>Browser: render dashboard shell for authenticated request
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
Owner
Author
|
Correcting the framing in the description above. I wrote that per-page auth checks are "follow-up work, not this PR", which reads like a tidy-up. Measured against the architecture standard, it is not:
Median page 273 lines; largest 754 ( The consequence that matters here: no page can protect itself, because a client component cannot resolve a session on the server. So the whole dashboard now rests on the single layout gate this PR adds — it closes the leak, but with nothing behind it. The gate is still worth merging. It should not be read as making the rest optional. Tracked as HAL-636. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@apps/web/app/`(dashboard)/layout.tsx:
- Around line 38-39: Update the session handling around getServerSession so
control-plane timeouts and 5xx failures produce a distinct unavailable result
that renders or throws a 5xx response instead of redirecting. Restrict
redirect("/login") to genuinely absent, invalid, or expired sessions, preserving
normal rendering for valid sessions.
- Line 38: Update the dashboard layout’s getServerSession call to use a bounded
timeout for the underlying /admin/v1/auth/me request, passing a cancellation
signal through the existing session-fetch configuration. Convert timeout
failures to a null session so the existing unauthenticated redirect path runs,
while preserving successful session handling and other error behavior.
In `@apps/web/components/Nav.tsx`:
- Around line 8-18: Update the DOCS_URL initialization to trim
NEXT_PUBLIC_DOCS_URL and use the documented default URL when the value is empty
or whitespace-only, while preserving nonblank override values for both Docs
links.
🪄 Autofix
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 Plus
Run ID: 39c5dca7-dd33-4676-a674-91e73a0fbd02
📒 Files selected for processing (2)
apps/web/app/(dashboard)/layout.tsxapps/web/components/Nav.tsx
| if (!session) { | ||
| return null; | ||
| } | ||
| const session = await getServerSession(); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the authentication call and the declared runtime limits.
ast-grep outline apps/web/lib/server-auth.ts --items all
rg -n -C 4 'getServerSession|fetch\(|AbortSignal\.timeout|signal:|maxDuration' apps/web
fd -HI 'package.json|.nvmrc|.node-version|Dockerfile|next.config.*|vercel.json' . -x sed -n '1,180p' {}Repository: hallelx2/vectorless
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate files =="
git ls-files | rg '(^|/)apps/web/lib/server-auth\.(ts|tsx|js|jsx)$|^apps/web/next\.config\.(ts|js|mjs|cjs)$|(^|/)vercel\.json$|package\.json$' || true
echo
echo "== server-auth outline =="
ast-grep outline apps/web/lib/server-auth.ts --items all || true
echo
echo "== server-auth source =="
cat -n apps/web/lib/server-auth.ts
echo
echo "== dashboard layout relevant =="
sed -n '1,140p' apps/web/app/'(dashboard)'/layout.tsx | cat -n
echo
echo "== focused auth/control-plane references =="
rg -n -C 3 'getServerSession|setServerSession|fetch\(|CLOVA|control|signal:|AbortSignal\.timeout|maxDuration' apps/web/lib apps/web/next.config.* 2>/dev/null || true
echo
echo "== focused config references =="
jq '{scripts, dependencies, devDependencies, next, vercel}' apps/web/package.json 2>/dev/null || sed -n '1,220p' apps/web/package.json
for f in apps/web/next.config.* vercel.json; do
[ -f "$f" ] && { echo "--- $f"; sed -n '1,220p' "$f"; }
doneRepository: hallelx2/vectorless
Length of output: 33731
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Vercel runtime config =="
cat -n apps/web/vercel.json
echo
echo "== top package runtime context =="
for f in package.json apps/web/package.json; do
echo "--- $f"
jq '{engines, scripts, dependencies, devDependencies}' "$f" 2>/dev/null || sed -n '1,220p' "$f"
doneRepository: hallelx2/vectorless
Length of output: 3140
Bound the control-plane session request.
getServerSession() calls fetch() for /admin/v1/auth/me without a cancellation signal, and the /dashboard layout awaits it before rendering. Add a bounded timeout to this auth request; return null when it times out so unauthenticated/failed requests redirect instead of exhausting the function’s runtime.
🤖 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 `@apps/web/app/`(dashboard)/layout.tsx at line 38, Update the dashboard
layout’s getServerSession call to use a bounded timeout for the underlying
/admin/v1/auth/me request, passing a cancellation signal through the existing
session-fetch configuration. Convert timeout failures to a null session so the
existing unauthenticated redirect path runs, while preserving successful session
handling and other error behavior.
Comment on lines
+38
to
+39
| const session = await getServerSession(); | ||
| if (!session) redirect("/login"); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Do not redirect valid users when the control plane is unavailable.
getServerSession() returns null for fetch failures and every non-401/403 response from the control plane. This branch redirects those failures to /login.
Return a distinct unavailable result for timeouts and 5xx responses. Render or throw a 5xx response for that result. Keep the redirect for only absent, invalid, or expired sessions.
🤖 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 `@apps/web/app/`(dashboard)/layout.tsx around lines 38 - 39, Update the session
handling around getServerSession so control-plane timeouts and 5xx failures
produce a distinct unavailable result that renders or throws a 5xx response
instead of redirecting. Restrict redirect("/login") to genuinely absent,
invalid, or expired sessions, preserving normal rendering for valid sessions.
Comment on lines
+8
to
+18
| /** | ||
| * Docs live on their own Fumadocs deployment, not in this app. The link | ||
| * used to point at /dashboard, which sent anyone looking for | ||
| * documentation into the product — and, before the layout was gated on | ||
| * the server, into a login redirect. | ||
| * | ||
| * Overridable so a preview deployment can point at a staging docs site | ||
| * without a code change. | ||
| */ | ||
| const DOCS_URL = process.env.NEXT_PUBLIC_DOCS_URL ?? 'https://docs.vectorless.store'; | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Treat blank documentation URLs as unset.
?? does not replace an empty string. If NEXT_PUBLIC_DOCS_URL is empty or whitespace-only, both Docs links receive an invalid URL instead of the documented fallback.
Proposed fix
-const DOCS_URL = process.env.NEXT_PUBLIC_DOCS_URL ?? 'https://docs.vectorless.store';
+const DOCS_URL =
+ process.env.NEXT_PUBLIC_DOCS_URL?.trim() || 'https://docs.vectorless.store';📝 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
| /** | |
| * Docs live on their own Fumadocs deployment, not in this app. The link | |
| * used to point at /dashboard, which sent anyone looking for | |
| * documentation into the product — and, before the layout was gated on | |
| * the server, into a login redirect. | |
| * | |
| * Overridable so a preview deployment can point at a staging docs site | |
| * without a code change. | |
| */ | |
| const DOCS_URL = process.env.NEXT_PUBLIC_DOCS_URL ?? 'https://docs.vectorless.store'; | |
| /** | |
| * Docs live on their own Fumadocs deployment, not in this app. The link | |
| * used to point at /dashboard, which sent anyone looking for | |
| * documentation into the product — and, before the layout was gated on | |
| * the server, into a login redirect. | |
| * | |
| * Overridable so a preview deployment can point at a staging docs site | |
| * without a code change. | |
| */ | |
| const DOCS_URL = | |
| process.env.NEXT_PUBLIC_DOCS_URL?.trim() || 'https://docs.vectorless.store'; |
🤖 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 `@apps/web/components/Nav.tsx` around lines 8 - 18, Update the DOCS_URL
initialization to trim NEXT_PUBLIC_DOCS_URL and use the documented default URL
when the value is empty or whitespace-only, while preserving nonblank override
values for both Docs links.
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
1 participant
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.
Three issues you reported, one of them structural.
1 · The dashboard had no server-side authentication
(dashboard)/layout.tsxwas a client component callinguseSession()and redirecting from auseEffect. That is not protection. The check runs only after the server has sent the HTML and the browser has downloaded, parsed and hydrated the whole dashboard bundle — so an unauthenticated visitor received the entire screen, saw a spinner, and was then bounced.What leaked: route existence, layout, navigation, feature set. A user with an expired session also got a flash of the UI before the redirect.
Now a server component — resolve the session, redirect if absent. Proof it is live: every
/dashboardroute moved from static toƒ(dynamic) in the build output, which only happens because the layout now reads request headers.Customer data was never exposed
Worth stating plainly, because "no auth on the dashboard" sounds worse than it was. Every
/api/dashboard/*route proxies throughforwardToCP, which returns 401 when thevls_sessioncookie is missing and otherwise forwards it to the control plane to validate. There is no service-token fallback for user data — theapiKeypath is only for the shared demo corpus.So the leak was the shell, not the contents. A real defect, not an incident.
I checked this specifically before writing it up, because my first three greps all pointed the wrong way —
lib/server-auth.tsexists, and the route handlers authenticate indirectly through the proxy rather than by calling a session helper directly, so a naive search forgetSessionreturns nothing.Scope
This is a coarse gate. Per-page checks remain the standard — a route's protection should be readable in its own file — but every page below is still
"use client"and cannot do it yet. Converting them is follow-up work, not this PR.2 · Docs pointed at
/dashboardAnyone looking for documentation was sent into the product. Once the gate above landed, they would have been bounced to
/logininstead.Now points at
docs.vectorless.store, overridable viaNEXT_PUBLIC_DOCS_URLso a preview deployment can target a staging docs site without a code change.3 · Pricing, FAQ and How-it-works were dead on
/whitepaperThe sections do exist and carry the right ids —
#pricing,#faq,#how— but only on the landing page, andNavrenders on both it and/whitepaper. A bare#pricingresolves against the current document, so on the whitepaper page it did nothing.Changed to
/#pricingso the links work from either page: scroll on the landing page, navigate-then-scroll from elsewhere.Verification
tsc --noEmitclean,next buildclean, all dashboard routes confirmed dynamic.Not verified: the redirect behaviour against a live control plane —
getServerSessioncalls/admin/v1/auth/me, which I have no session cookie to exercise here. Worth a manual check on preview: hit/dashboardlogged out and confirm you get a redirect rather than a spinner.Summary by Sourcery
Gate the dashboard layout on server-side authentication, and repair navigation links for docs and key marketing sections.
Bug Fixes:
Enhancements:
Summary by CodeRabbit
Bug Fixes
Improvements