-
Notifications
You must be signed in to change notification settings - Fork 9.4k
fix(security): require browser origin for folder import when desktop gate inactive (#5480) #6073
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
Open
CVE-Hunter-Leo
wants to merge
1
commit into
nexu-io:main
Choose a base branch
from
CVE-Hunter-Leo:fix/folder-import-auth-gate-5480
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+139
−0
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,106 @@ | ||
| import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; | ||
| import express from 'express'; | ||
| import request from 'supertest'; | ||
| import { registerImportRoutes } from '../src/import-export-routes.js'; | ||
| import { resetDesktopAuthForTests } from '../src/desktop-auth.js'; | ||
|
|
||
| /** | ||
| * Tests for the folder-import / working-dir auth gate (issue #5480). | ||
| * | ||
| * When isDesktopAuthGateActive() returns false (non-desktop mode), both | ||
| * POST /api/import/folder and POST /api/projects/:id/working-dir must | ||
| * reject requests that lack a browser Origin header, preventing arbitrary | ||
| * directory binding by non-browser local processes. | ||
| */ | ||
| describe('folder import auth gate (issue #5480)', () => { | ||
| let app: express.Express; | ||
|
|
||
| function createApp() { | ||
| const app = express(); | ||
| app.use(express.json()); | ||
|
|
||
| // Minimal mock context that satisfies the route registrar. | ||
| const ctx: any = { | ||
| db: {}, | ||
| http: { | ||
| sendApiError: (res: any, status: number, _code: string, message: string) => { | ||
| res.status(status).json({ error: message }); | ||
| }, | ||
| }, | ||
| uploads: { importUpload: () => [] }, | ||
| node: { fs, path: require('node:path') }, | ||
| ids: { randomId: () => 'test-id' }, | ||
| paths: { PROJECTS_DIR: '/tmp/projects', RUNTIME_DATA_DIR_CANONICAL: '/tmp/data' }, | ||
| imports: { | ||
| importClaudeDesignZip: vi.fn(), | ||
| projectDir: vi.fn(() => '/tmp/projects/test'), | ||
| detectEntryFile: vi.fn(async () => null), | ||
| }, | ||
| auth: { | ||
| consumedImportNonces: new Map(), | ||
| desktopAuthSecret: () => null, | ||
| isDesktopAuthGateActive: () => false, // non-desktop mode | ||
| pruneExpiredImportNonces: vi.fn(), | ||
| verifyDesktopImportToken: vi.fn(), | ||
| }, | ||
| projectStore: { | ||
| getProject: vi.fn(() => ({ id: 'test-id', metadata: {} })), | ||
| insertProject: vi.fn(), | ||
| updateProject: vi.fn(() => ({ id: 'test-id', metadata: {} })), | ||
| }, | ||
| conversations: { insertConversation: vi.fn() }, | ||
| projectFiles: { setTabs: vi.fn() }, | ||
| validation: { validateProjectDesignSystemId: vi.fn(() => null) }, | ||
| }; | ||
|
|
||
| registerImportRoutes(app, ctx); | ||
| return app; | ||
| } | ||
|
|
||
| beforeEach(() => { | ||
| resetDesktopAuthForTests(); | ||
| app = createApp(); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| resetDesktopAuthForTests(); | ||
| }); | ||
|
|
||
| describe('POST /api/import/folder', () => { | ||
| it('rejects requests without Origin header (non-browser client)', async () => { | ||
| const res = await request(app) | ||
| .post('/api/import/folder') | ||
| .send({ baseDir: '/tmp' }); | ||
| expect(res.status).toBe(403); | ||
| expect(res.body.error).toContain('browser origin'); | ||
| }); | ||
|
|
||
| it('accepts requests with a browser Origin header', async () => { | ||
| const res = await request(app) | ||
| .post('/api/import/folder') | ||
| .set('Origin', 'http://localhost:3000') | ||
| .send({ baseDir: '/tmp' }); | ||
| // Should NOT get 403 — it may get 400 for other validation reasons, | ||
| // but must not be blocked by the auth gate. | ||
| expect(res.status).not.toBe(403); | ||
| }); | ||
| }); | ||
|
|
||
| describe('POST /api/projects/:id/working-dir', () => { | ||
| it('rejects requests without Origin header (non-browser client)', async () => { | ||
| const res = await request(app) | ||
| .post('/api/projects/test-id/working-dir') | ||
| .send({ baseDir: '/tmp' }); | ||
| expect(res.status).toBe(403); | ||
| expect(res.body.error).toContain('browser origin'); | ||
| }); | ||
|
|
||
| it('accepts requests with a browser Origin header', async () => { | ||
| const res = await request(app) | ||
| .post('/api/projects/test-id/working-dir') | ||
| .set('Origin', 'http://localhost:3000') | ||
| .send({ baseDir: '/tmp' }); | ||
| expect(res.status).not.toBe(403); | ||
| }); | ||
| }); | ||
| }); |
Oops, something went wrong.
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.
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.
🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.folderImportAuthorizationReason()is treating the presence of anOriginheader as proof that the request came from the web UI, but the daemon's own origin model does not give you that guarantee.server.tsexplicitly allows non-browser clients whenOriginis missing, andisAllowedBrowserOrigin()only checks that a supplied origin string looks like a same-host loopback/LAN origin. A raw local client can therefore sendOrigin: http://127.0.0.1:<port>(or another allowed loopback/LAN origin) and still reach these routes, so the unauthenticated directory-binding hole remains open even though the new no-Origin case is blocked. Please switch this gate to a credential that a local script cannot forge, such as keeping the existing desktop import-token/HMAC flow for these privileged routes or another explicit daemon-issued secret, and extend the regression coverage to prove that a non-browser request with a forged allowedOriginis rejected.