-
-
Notifications
You must be signed in to change notification settings - Fork 427
feat: add authNext and deprecate legacy auth #883
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: 8ca41ab The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
4 issues found across 15 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/server-core/src/auth/utils.ts">
<violation number="1" location="packages/server-core/src/auth/utils.ts:75">
P1: Security: Passing access keys in URL query parameters exposes credentials to logging, browser history, Referer headers, and proxy caches. Sensitive authentication tokens should only be transmitted via headers (like the existing `x-console-access-key` header) or POST body, not in URLs. Consider removing query parameter authentication for production security.</violation>
</file>
<file name="packages/server-core/src/auth/next.ts">
<violation number="1" location="packages/server-core/src/auth/next.ts:79">
P2: Inconsistent route configuration behavior: `publicRoutes` merges config with provider routes, but `consoleRoutes` completely replaces defaults when provided. If a user sets `consoleRoutes: ["/my-admin"]`, they'll lose all DEFAULT_CONSOLE_ROUTES (like "WS /ws", "GET /api/logs", etc.). Consider merging instead: `const consoleRoutes = [...DEFAULT_CONSOLE_ROUTES, ...(config.consoleRoutes ?? [])];`</violation>
</file>
<file name="packages/server-core/src/websocket/setup.ts">
<violation number="1" location="packages/server-core/src/websocket/setup.ts:39">
P2: Empty block for dev bypass on protected routes leaves `user` as `null`. Consider setting a user object (e.g., `{ id: "dev", type: "dev-bypass" }`) for consistency with the legacy flow, or add explicit documentation if leaving user as null is intentional.</violation>
</file>
<file name="packages/server-hono/src/auth/middleware.ts">
<violation number="1" location="packages/server-hono/src/auth/middleware.ts:124">
P2: Dev bypass doesn't apply to console routes, but error message suggests it does. If `hasConsoleAccess` fails, the middleware returns 401 before reaching the `isDevRequest` check. Consider adding the dev bypass check for console routes to match the error hint.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
| // 2. Console Access Key check (for production) | ||
| const consoleKey = req.headers.get("x-console-access-key"); | ||
| const url = new URL(req.url, "http://localhost"); | ||
| const queryKey = url.searchParams.get("key"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P1: Security: Passing access keys in URL query parameters exposes credentials to logging, browser history, Referer headers, and proxy caches. Sensitive authentication tokens should only be transmitted via headers (like the existing x-console-access-key header) or POST body, not in URLs. Consider removing query parameter authentication for production security.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/server-core/src/auth/utils.ts, line 75:
<comment>Security: Passing access keys in URL query parameters exposes credentials to logging, browser history, Referer headers, and proxy caches. Sensitive authentication tokens should only be transmitted via headers (like the existing `x-console-access-key` header) or POST body, not in URLs. Consider removing query parameter authentication for production security.</comment>
<file context>
@@ -68,9 +71,11 @@ export function hasConsoleAccess(req: Request): boolean {
// 2. Console Access Key check (for production)
const consoleKey = req.headers.get("x-console-access-key");
+ const url = new URL(req.url, "http://localhost");
+ const queryKey = url.searchParams.get("key");
const configuredKey = process.env.VOLTAGENT_CONSOLE_ACCESS_KEY;
</file context>
| return "public"; | ||
| } | ||
|
|
||
| const consoleRoutes = config.consoleRoutes ?? DEFAULT_CONSOLE_ROUTES; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: Inconsistent route configuration behavior: publicRoutes merges config with provider routes, but consoleRoutes completely replaces defaults when provided. If a user sets consoleRoutes: ["/my-admin"], they'll lose all DEFAULT_CONSOLE_ROUTES (like "WS /ws", "GET /api/logs", etc.). Consider merging instead: const consoleRoutes = [...DEFAULT_CONSOLE_ROUTES, ...(config.consoleRoutes ?? [])];
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/server-core/src/auth/next.ts, line 79:
<comment>Inconsistent route configuration behavior: `publicRoutes` merges config with provider routes, but `consoleRoutes` completely replaces defaults when provided. If a user sets `consoleRoutes: ["/my-admin"]`, they'll lose all DEFAULT_CONSOLE_ROUTES (like "WS /ws", "GET /api/logs", etc.). Consider merging instead: `const consoleRoutes = [...DEFAULT_CONSOLE_ROUTES, ...(config.consoleRoutes ?? [])];`</comment>
<file context>
@@ -0,0 +1,85 @@
+ return "public";
+ }
+
+ const consoleRoutes = config.consoleRoutes ?? DEFAULT_CONSOLE_ROUTES;
+ if (matchesAnyRoute(method, path, consoleRoutes)) {
+ return "console";
</file context>
| * Helper to check console access for WebSocket IncomingMessage | ||
| */ | ||
| function hasWebSocketConsoleAccess(req: IncomingMessage, url: URL): boolean { | ||
| if (isWebSocketDevBypass(req, url)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: Empty block for dev bypass on protected routes leaves user as null. Consider setting a user object (e.g., { id: "dev", type: "dev-bypass" }) for consistency with the legacy flow, or add explicit documentation if leaving user as null is intentional.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/server-core/src/websocket/setup.ts, line 39:
<comment>Empty block for dev bypass on protected routes leaves `user` as `null`. Consider setting a user object (e.g., `{ id: "dev", type: "dev-bypass" }`) for consistency with the legacy flow, or add explicit documentation if leaving user as null is intentional.</comment>
<file context>
@@ -21,34 +23,30 @@ function isDevWebSocketRequest(req: IncomingMessage): boolean {
+ * Helper to check console access for WebSocket IncomingMessage
+ */
+function hasWebSocketConsoleAccess(req: IncomingMessage, url: URL): boolean {
+ if (isWebSocketDevBypass(req, url)) {
return true;
}
</file context>
| if (access === "console") { | ||
| if (hasConsoleAccess(c.req.raw)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: Dev bypass doesn't apply to console routes, but error message suggests it does. If hasConsoleAccess fails, the middleware returns 401 before reaching the isDevRequest check. Consider adding the dev bypass check for console routes to match the error hint.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/server-hono/src/auth/middleware.ts, line 124:
<comment>Dev bypass doesn't apply to console routes, but error message suggests it does. If `hasConsoleAccess` fails, the middleware returns 401 before reaching the `isDevRequest` check. Consider adding the dev bypass check for console routes to match the error hint.</comment>
<file context>
@@ -127,3 +99,138 @@ export function createAuthMiddleware(authProvider: AuthProvider<Request>) {
+ return next();
+ }
+
+ if (access === "console") {
+ if (hasConsoleAccess(c.req.raw)) {
+ return next();
</file context>
| if (access === "console") { | |
| if (hasConsoleAccess(c.req.raw)) { | |
| if (access === "console") { | |
| if (hasConsoleAccess(c.req.raw) || isDevRequest(c.req.raw)) { |
Deploying voltagent with
|
| Latest commit: |
8ca41ab
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://9536fef2.voltagent.pages.dev |
| Branch Preview URL: | https://feat-auth-next.voltagent.pages.dev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="website/docs/api/authentication.md">
<violation number="1" location="website/docs/api/authentication.md:84">
P2: Documentation inconsistency: Route Access Summary shows 'Console key or JWT' for Observability/Updates in legacy auth, but the Legacy section states only 'Console Access Key' is required. This contradiction could cause users to misconfigure authentication for observability endpoints.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
| | Management | `GET /agents`, `GET /workflows`, `GET /tools` | Console key | Public | | ||
| | Docs + UI | `GET /`, `GET /doc`, `GET /ui` | Console key | Public | | ||
| | Discovery | `GET /mcp/servers`, `GET /agents/:id/card` | Console key | Public | | ||
| | Observability | `/observability/*`, `GET /api/logs`, `WS /ws/observability/**` | Console key | Console key or JWT | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: Documentation inconsistency: Route Access Summary shows 'Console key or JWT' for Observability/Updates in legacy auth, but the Legacy section states only 'Console Access Key' is required. This contradiction could cause users to misconfigure authentication for observability endpoints.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At website/docs/api/authentication.md, line 84:
<comment>Documentation inconsistency: Route Access Summary shows 'Console key or JWT' for Observability/Updates in legacy auth, but the Legacy section states only 'Console Access Key' is required. This contradiction could cause users to misconfigure authentication for observability endpoints.</comment>
<file context>
@@ -48,6 +48,48 @@ new VoltAgent({
+| Management | `GET /agents`, `GET /workflows`, `GET /tools` | Console key | Public |
+| Docs + UI | `GET /`, `GET /doc`, `GET /ui` | Console key | Public |
+| Discovery | `GET /mcp/servers`, `GET /agents/:id/card` | Console key | Public |
+| Observability | `/observability/*`, `GET /api/logs`, `WS /ws/observability/**` | Console key | Console key or JWT |
+| Updates | `GET /updates`, `POST /updates`, `POST /updates/:packageName` | Console key | Console key or JWT |
+| Custom routes | `GET /health`, `POST /webhooks/*` | User token (JWT) by default | Public by default |
</file context>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've fixed the documentation inconsistency in the Route Access Summary table. The table previously showed "Console key or JWT" for Observability and Updates endpoints under Legacy auth, but the Legacy section (line 612) clearly states that these endpoints "require Console Access Key in production" with no JWT alternative.
Change made:
- Updated lines 84-85 in the Route Access Summary table to show "Console key" instead of "Console key or JWT" for both Observability and Updates under the Legacy auth Access column
This aligns the table with the authoritative description in the Legacy section, preventing users from incorrectly assuming JWT authentication would work for observability endpoints when using legacy auth.
PR: #885
PR Checklist
Please check if your PR fulfills the following requirements:
Bugs / Features
What is the current behavior?
What is the new behavior?
fixes (issue)
Notes for reviewers
Summary by cubic
Introduces authNext, a new authentication policy that protects all routes by default and separates access into public, console, and user. Deprecates legacy auth while preserving backward compatibility, updates middleware and WebSocket handling, and refreshes docs and examples.
New Features
Migration
Written for commit 8ca41ab. Summary will update automatically on new commits.