-
Notifications
You must be signed in to change notification settings - Fork 26
DNS validation fixes. More dev tools for bot env. #19
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -218,6 +218,61 @@ describe('/api/models/discover', () => { | |
| await server.close(); | ||
| }); | ||
|
|
||
| it('should return models for local Ollama URL', async () => { | ||
| const fakeModels = { data: [{ id: 'qwen3-coder:30b' }, { id: 'nomic-embed-text' }] }; | ||
| const jsonBytes = new TextEncoder().encode(JSON.stringify(fakeModels)); | ||
| const mockFetch = vi.fn().mockResolvedValue( | ||
| new Response(jsonBytes, { status: 200, headers: { 'content-type': 'application/json' } }), | ||
| ); | ||
| vi.stubGlobal('fetch', mockFetch); | ||
|
|
||
| const server = await createTestServer(); | ||
| const token = await getAuthToken(server); | ||
| const response = await server.inject({ | ||
| method: 'POST', | ||
| url: '/api/models/discover', | ||
| payload: { baseUrl: 'http://localhost:11434/v1' }, | ||
| headers: { Authorization: `Bearer ${token}` }, | ||
| }); | ||
| expect(response.statusCode).toBe(200); | ||
| const body = JSON.parse(response.body) as { models: string[] }; | ||
| expect(body.models).toContain('qwen3-coder:30b'); | ||
| expect(body.models).toContain('nomic-embed-text'); | ||
| expect(mockFetch).toHaveBeenCalledOnce(); | ||
| // localhost is rewritten to host.docker.internal by toDockerHostUrl | ||
| expect(mockFetch.mock.calls[0][0]).toBe('http://host.docker.internal:11434/v1/models'); | ||
|
|
||
| vi.unstubAllGlobals(); | ||
| await server.close(); | ||
|
Comment on lines
+227
to
+246
|
||
| }); | ||
|
|
||
| it('should return models for host.docker.internal URL', async () => { | ||
| const fakeModels = { data: [{ id: 'llama3:8b' }] }; | ||
| const jsonBytes = new TextEncoder().encode(JSON.stringify(fakeModels)); | ||
| const mockFetch = vi.fn().mockResolvedValue( | ||
| new Response(jsonBytes, { status: 200, headers: { 'content-type': 'application/json' } }), | ||
| ); | ||
| vi.stubGlobal('fetch', mockFetch); | ||
|
|
||
| const server = await createTestServer(); | ||
| const token = await getAuthToken(server); | ||
| const response = await server.inject({ | ||
| method: 'POST', | ||
| url: '/api/models/discover', | ||
| payload: { baseUrl: 'http://host.docker.internal:11434/v1' }, | ||
| headers: { Authorization: `Bearer ${token}` }, | ||
| }); | ||
| expect(response.statusCode).toBe(200); | ||
| const body = JSON.parse(response.body) as { models: string[] }; | ||
| expect(body.models).toContain('llama3:8b'); | ||
| expect(mockFetch).toHaveBeenCalledOnce(); | ||
| // URL should NOT be rewritten by resolveAndValidateUrl — passed through directly | ||
| expect(mockFetch.mock.calls[0][0]).toBe('http://host.docker.internal:11434/v1/models'); | ||
|
|
||
| vi.unstubAllGlobals(); | ||
| await server.close(); | ||
| }); | ||
|
|
||
| it('should reject invalid URL', async () => { | ||
| const server = await createTestServer(); | ||
| const token = await getAuthToken(server); | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -293,16 +293,20 @@ export async function buildServer(): Promise<FastifyInstance> { | |||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // Register security headers | ||||||||||||||||||||||||||||||
| await server.register(fastifyHelmet, { | ||||||||||||||||||||||||||||||
| // BotMaker runs on plain HTTP behind a LAN; HSTS and upgrade-insecure-requests | ||||||||||||||||||||||||||||||
| // cause browsers to silently fail when there is no TLS terminator. | ||||||||||||||||||||||||||||||
| hsts: false, | ||||||||||||||||||||||||||||||
|
Comment on lines
294
to
+298
|
||||||||||||||||||||||||||||||
| // Register security headers | |
| await server.register(fastifyHelmet, { | |
| // BotMaker runs on plain HTTP behind a LAN; HSTS and upgrade-insecure-requests | |
| // cause browsers to silently fail when there is no TLS terminator. | |
| hsts: false, | |
| // Determine whether to enable HSTS. Default is enabled; set BOTMAKER_HSTS=false | |
| // in environments that are strictly HTTP-only (e.g., plain HTTP behind a LAN). | |
| const enableHsts = process.env.BOTMAKER_HSTS !== 'false'; | |
| // Register security headers | |
| await server.register(fastifyHelmet, { | |
| // HSTS can cause issues when there is no TLS terminator; allow opting out | |
| // via BOTMAKER_HSTS=false instead of disabling it unconditionally. | |
| hsts: enableHsts, |
Copilot
AI
Feb 9, 2026
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.
In the isLocal branch, the handler bypasses isPrivateUrl(...) protocol validation and proceeds to build/fetch the URL. That means non-http(s) local URLs (e.g. ftp://localhost/...) would be treated as allowlisted and return 200 with empty models instead of a 400. Add an explicit protocol check (http/https only) before entering the local fast-path, or update isLocalDiscoveryUrl to also require an http(s) scheme.
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.
Installing Rust via
rustupwith--default-toolchain stablemakes the image build non-reproducible (the toolchain can change over time), which can lead to unexpected build breakages. Consider pinning a specific Rust toolchain version via anARG/ENV(and optionally verifying the installer) to keep builds deterministic.