[fix] honor dev query bypass in hasConsoleAccess#1189
[fix] honor dev query bypass in hasConsoleAccess#1189omeraplak merged 1 commit intoVoltAgent:mainfrom
Conversation
🦋 Changeset detectedLatest commit: 93b47ff The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
📝 WalkthroughWalkthroughThis PR fixes a bug where Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
packages/server-core/src/auth/utils.spec.ts (2)
37-54: Consider adding test for console key via header.The
hasConsoleAccessfunction supports reading the console key from both thex-console-access-keyheader and thekeyquery param. Only the query param path is tested. Consider adding coverage for the header path:🧪 Suggested additional test
it("still accepts a configured console access key from query params", () => { vi.stubEnv("NODE_ENV", "production"); vi.stubEnv("VOLTAGENT_CONSOLE_ACCESS_KEY", "secret-key"); const req = new Request("http://localhost/ws?key=secret-key"); expect(hasConsoleAccess(req)).toBe(true); }); + + it("accepts a configured console access key from header", () => { + vi.stubEnv("NODE_ENV", "production"); + vi.stubEnv("VOLTAGENT_CONSOLE_ACCESS_KEY", "secret-key"); + + const req = new Request("http://localhost/ws", { + headers: { "x-console-access-key": "secret-key" }, + }); + + expect(hasConsoleAccess(req)).toBe(true); + }); + + it("rejects when no valid key is provided in production", () => { + vi.stubEnv("NODE_ENV", "production"); + vi.stubEnv("VOLTAGENT_CONSOLE_ACCESS_KEY", "secret-key"); + + const req = new Request("http://localhost/ws?key=wrong-key"); + + expect(hasConsoleAccess(req)).toBe(false); + }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server-core/src/auth/utils.spec.ts` around lines 37 - 54, Tests for hasConsoleAccess currently only cover the query param key; add a test that verifies the header path by stubbing NODE_ENV to "production" and VOLTAGENT_CONSOLE_ACCESS_KEY to a secret, creating a Request to any URL (e.g., "/ws") with the header "x-console-access-key" set to that secret, and asserting hasConsoleAccess(req) returns true; reference the hasConsoleAccess function and add the new it(...) block alongside the existing tests in utils.spec.ts.
9-35: Consider adding test for header rejection in production.The tests cover the critical paths well. Consider adding a test to explicitly verify that the dev header is also rejected in production, mirroring the query param rejection test:
🧪 Suggested additional test
it("rejects the dev query param in production", () => { vi.stubEnv("NODE_ENV", "production"); const req = new Request("http://localhost/ws?dev=true"); expect(isDevRequest(req)).toBe(false); }); + + it("rejects the dev header in production", () => { + vi.stubEnv("NODE_ENV", "production"); + + const req = new Request("http://localhost/api", { + headers: { "x-voltagent-dev": "true" }, + }); + + expect(isDevRequest(req)).toBe(false); + }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server-core/src/auth/utils.spec.ts` around lines 9 - 35, Add a unit test in packages/server-core/src/auth/utils.spec.ts that mirrors the existing query-param production case but for the header: stub NODE_ENV to "production", create a Request with header "x-voltagent-dev": "true", call isDevRequest(req) and assert it returns false; this ensures the isDevRequest function rejects the dev header in production just like it does the dev query param.packages/server-core/src/auth/utils.ts (1)
74-91: Minor: URL parsed twice when dev bypass fails.When
isDevRequestreturnsfalsein non-production, the URL is parsed twice (once inisDevRequestat line 45, then again here at line 82). This is a minor inefficiency but acceptable for the improved code clarity and separation of concerns.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server-core/src/auth/utils.ts` around lines 74 - 91, The hasConsoleAccess function unnecessarily reparses the request URL when isDevRequest already parsed it; to fix, modify hasConsoleAccess to reuse the parsed URL from isDevRequest (or have isDevRequest return or attach the parsed URL) instead of new URL(req.url, ...), so update hasConsoleAccess to obtain the query key from that shared URL object and remove the duplicate new URL(...) call; reference functions: hasConsoleAccess and isDevRequest.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/server-core/src/auth/utils.spec.ts`:
- Around line 37-54: Tests for hasConsoleAccess currently only cover the query
param key; add a test that verifies the header path by stubbing NODE_ENV to
"production" and VOLTAGENT_CONSOLE_ACCESS_KEY to a secret, creating a Request to
any URL (e.g., "/ws") with the header "x-console-access-key" set to that secret,
and asserting hasConsoleAccess(req) returns true; reference the hasConsoleAccess
function and add the new it(...) block alongside the existing tests in
utils.spec.ts.
- Around line 9-35: Add a unit test in
packages/server-core/src/auth/utils.spec.ts that mirrors the existing
query-param production case but for the header: stub NODE_ENV to "production",
create a Request with header "x-voltagent-dev": "true", call isDevRequest(req)
and assert it returns false; this ensures the isDevRequest function rejects the
dev header in production just like it does the dev query param.
In `@packages/server-core/src/auth/utils.ts`:
- Around line 74-91: The hasConsoleAccess function unnecessarily reparses the
request URL when isDevRequest already parsed it; to fix, modify hasConsoleAccess
to reuse the parsed URL from isDevRequest (or have isDevRequest return or attach
the parsed URL) instead of new URL(req.url, ...), so update hasConsoleAccess to
obtain the query key from that shared URL object and remove the duplicate new
URL(...) call; reference functions: hasConsoleAccess and isDevRequest.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9afe6338-b05b-426f-ba61-b517069a0e2a
📒 Files selected for processing (3)
.changeset/fair-geckos-dream.mdpackages/server-core/src/auth/utils.spec.tspackages/server-core/src/auth/utils.ts
|
Hey @pandego , |
Summary
?dev=trueinsideisDevRequest()sohasConsoleAccess()matches the existing WebSocket dev-bypass behavior@voltagent/server-coreType of Change
Testing
pnpm --filter @voltagent/server-core test -- --run src/auth/utils.spec.tspnpm exec biome check packages/server-core/src/auth/utils.ts packages/server-core/src/auth/utils.spec.tsRelated Issue
fixes #1185
Checklist
Additional Context
The WebSocket setup path already honors
?dev=truein non-production, but the sharedRequest-based console-access helper only checked the header. This patch makes the shared helper consistent with the WebSocket path so development/ws?dev=truerequests are accepted as expected.Summary by cubic
Honor the
?dev=truequery param inisDevRequest()sohasConsoleAccess()matches the WebSocket dev-bypass in non-production. This restores expected dev console access on Request-based/wsroutes and ships a patch for@voltagent/server-core.?dev=truein non-production; production remains strict.hasConsoleAccess()with WebSocket path behavior; fixes [BUG] hasConsoleAccess is false for /ws?dev=true #1185.@voltagent/server-core.Written for commit 93b47ff. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Tests