fix(cli): restrict attach auth injection to loopback URLs
This commit is contained in:
@@ -3,6 +3,7 @@ import { describe, it, expect, mock, beforeEach, afterEach, afterAll } from "bun
|
|||||||
import * as originalSdk from "@opencode-ai/sdk"
|
import * as originalSdk from "@opencode-ai/sdk"
|
||||||
import * as originalPortUtils from "../../shared/port-utils"
|
import * as originalPortUtils from "../../shared/port-utils"
|
||||||
import * as originalBinaryResolver from "./opencode-binary-resolver"
|
import * as originalBinaryResolver from "./opencode-binary-resolver"
|
||||||
|
import * as originalServerAuth from "../../shared/opencode-server-auth"
|
||||||
|
|
||||||
const originalConsole = globalThis.console
|
const originalConsole = globalThis.console
|
||||||
|
|
||||||
@@ -13,11 +14,15 @@ const mockCreateOpencode = mock(() =>
|
|||||||
server: { url: "http://127.0.0.1:4096", close: mockServerClose },
|
server: { url: "http://127.0.0.1:4096", close: mockServerClose },
|
||||||
})
|
})
|
||||||
)
|
)
|
||||||
const mockCreateOpencodeClient = mock(() => ({ session: {} }))
|
const mockCreateOpencodeClient = mock((options?: { baseUrl?: string }) => ({
|
||||||
|
session: {},
|
||||||
|
baseUrl: options?.baseUrl,
|
||||||
|
}))
|
||||||
const mockIsPortAvailable = mock(() => Promise.resolve(true))
|
const mockIsPortAvailable = mock(() => Promise.resolve(true))
|
||||||
const mockGetAvailableServerPort = mock(() => Promise.resolve({ port: 4096, wasAutoSelected: false }))
|
const mockGetAvailableServerPort = mock(() => Promise.resolve({ port: 4096, wasAutoSelected: false }))
|
||||||
const mockConsoleLog = mock(() => {})
|
const mockConsoleLog = mock(() => {})
|
||||||
const mockWithWorkingOpencodePath = mock((startServer: () => Promise<unknown>) => startServer())
|
const mockWithWorkingOpencodePath = mock((startServer: () => Promise<unknown>) => startServer())
|
||||||
|
const mockInjectServerAuthIntoClient = mock(() => {})
|
||||||
|
|
||||||
mock.module("@opencode-ai/sdk", () => ({
|
mock.module("@opencode-ai/sdk", () => ({
|
||||||
createOpencode: mockCreateOpencode,
|
createOpencode: mockCreateOpencode,
|
||||||
@@ -34,10 +39,15 @@ mock.module("./opencode-binary-resolver", () => ({
|
|||||||
withWorkingOpencodePath: mockWithWorkingOpencodePath,
|
withWorkingOpencodePath: mockWithWorkingOpencodePath,
|
||||||
}))
|
}))
|
||||||
|
|
||||||
|
mock.module("../../shared/opencode-server-auth", () => ({
|
||||||
|
injectServerAuthIntoClient: mockInjectServerAuthIntoClient,
|
||||||
|
}))
|
||||||
|
|
||||||
afterAll(() => {
|
afterAll(() => {
|
||||||
mock.module("@opencode-ai/sdk", () => originalSdk)
|
mock.module("@opencode-ai/sdk", () => originalSdk)
|
||||||
mock.module("../../shared/port-utils", () => originalPortUtils)
|
mock.module("../../shared/port-utils", () => originalPortUtils)
|
||||||
mock.module("./opencode-binary-resolver", () => originalBinaryResolver)
|
mock.module("./opencode-binary-resolver", () => originalBinaryResolver)
|
||||||
|
mock.module("../../shared/opencode-server-auth", () => originalServerAuth)
|
||||||
mock.restore()
|
mock.restore()
|
||||||
})
|
})
|
||||||
|
|
||||||
@@ -52,6 +62,7 @@ describe("createServerConnection", () => {
|
|||||||
mockServerClose.mockClear()
|
mockServerClose.mockClear()
|
||||||
mockConsoleLog.mockClear()
|
mockConsoleLog.mockClear()
|
||||||
mockWithWorkingOpencodePath.mockClear()
|
mockWithWorkingOpencodePath.mockClear()
|
||||||
|
mockInjectServerAuthIntoClient.mockClear()
|
||||||
globalThis.console = { ...console, log: mockConsoleLog } as typeof console
|
globalThis.console = { ...console, log: mockConsoleLog } as typeof console
|
||||||
})
|
})
|
||||||
|
|
||||||
@@ -59,6 +70,49 @@ describe("createServerConnection", () => {
|
|||||||
globalThis.console = originalConsole
|
globalThis.console = originalConsole
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it("attach mode injects auth only for loopback URLs", async () => {
|
||||||
|
// given
|
||||||
|
const signal = new AbortController().signal
|
||||||
|
|
||||||
|
// when
|
||||||
|
const localhostResult = await createServerConnection({ attach: "http://localhost:8080", signal })
|
||||||
|
const loopbackResult = await createServerConnection({ attach: "http://127.0.0.1:8080", signal })
|
||||||
|
const anyBindResult = await createServerConnection({ attach: "http://0.0.0.0:8080", signal })
|
||||||
|
const remoteResult = await createServerConnection({ attach: "https://example.com", signal })
|
||||||
|
|
||||||
|
// then
|
||||||
|
expect(mockCreateOpencodeClient).toHaveBeenCalledWith({ baseUrl: "http://localhost:8080" })
|
||||||
|
expect(mockCreateOpencodeClient).toHaveBeenCalledWith({ baseUrl: "http://127.0.0.1:8080" })
|
||||||
|
expect(mockCreateOpencodeClient).toHaveBeenCalledWith({ baseUrl: "http://0.0.0.0:8080" })
|
||||||
|
expect(mockCreateOpencodeClient).toHaveBeenCalledWith({ baseUrl: "https://example.com" })
|
||||||
|
expect(mockInjectServerAuthIntoClient).toHaveBeenCalledTimes(3)
|
||||||
|
expect(mockInjectServerAuthIntoClient).toHaveBeenNthCalledWith(1, localhostResult.client)
|
||||||
|
expect(mockInjectServerAuthIntoClient).toHaveBeenNthCalledWith(2, loopbackResult.client)
|
||||||
|
expect(mockInjectServerAuthIntoClient).toHaveBeenNthCalledWith(3, anyBindResult.client)
|
||||||
|
expect(mockInjectServerAuthIntoClient).not.toHaveBeenCalledWith(remoteResult.client)
|
||||||
|
expect(mockWithWorkingOpencodePath).not.toHaveBeenCalled()
|
||||||
|
localhostResult.cleanup()
|
||||||
|
loopbackResult.cleanup()
|
||||||
|
anyBindResult.cleanup()
|
||||||
|
remoteResult.cleanup()
|
||||||
|
expect(mockServerClose).not.toHaveBeenCalled()
|
||||||
|
})
|
||||||
|
|
||||||
|
it("attach mode skips auth injection for invalid attach URLs", async () => {
|
||||||
|
// given
|
||||||
|
const signal = new AbortController().signal
|
||||||
|
const attachUrl = "not-a-url"
|
||||||
|
|
||||||
|
// when
|
||||||
|
const result = await createServerConnection({ attach: attachUrl, signal })
|
||||||
|
|
||||||
|
// then
|
||||||
|
expect(mockCreateOpencodeClient).toHaveBeenCalledWith({ baseUrl: attachUrl })
|
||||||
|
expect(mockInjectServerAuthIntoClient).not.toHaveBeenCalled()
|
||||||
|
result.cleanup()
|
||||||
|
expect(mockServerClose).not.toHaveBeenCalled()
|
||||||
|
})
|
||||||
|
|
||||||
it("attach mode returns client with no-op cleanup", async () => {
|
it("attach mode returns client with no-op cleanup", async () => {
|
||||||
// given
|
// given
|
||||||
const signal = new AbortController().signal
|
const signal = new AbortController().signal
|
||||||
@@ -68,8 +122,6 @@ describe("createServerConnection", () => {
|
|||||||
const result = await createServerConnection({ attach: attachUrl, signal })
|
const result = await createServerConnection({ attach: attachUrl, signal })
|
||||||
|
|
||||||
// then
|
// then
|
||||||
expect(mockCreateOpencodeClient).toHaveBeenCalledWith({ baseUrl: attachUrl })
|
|
||||||
expect(mockWithWorkingOpencodePath).not.toHaveBeenCalled()
|
|
||||||
expect(result.client).toBeDefined()
|
expect(result.client).toBeDefined()
|
||||||
expect(result.cleanup).toBeDefined()
|
expect(result.cleanup).toBeDefined()
|
||||||
result.cleanup()
|
result.cleanup()
|
||||||
|
|||||||
@@ -5,6 +5,17 @@ import { injectServerAuthIntoClient } from "../../shared/opencode-server-auth"
|
|||||||
import { getAvailableServerPort, isPortAvailable, DEFAULT_SERVER_PORT } from "../../shared/port-utils"
|
import { getAvailableServerPort, isPortAvailable, DEFAULT_SERVER_PORT } from "../../shared/port-utils"
|
||||||
import { withWorkingOpencodePath } from "./opencode-binary-resolver"
|
import { withWorkingOpencodePath } from "./opencode-binary-resolver"
|
||||||
|
|
||||||
|
const LOOPBACK_HOSTS = new Set(["127.0.0.1", "localhost", "::1", "[::1]", "0.0.0.0"])
|
||||||
|
|
||||||
|
function isLoopbackAttachUrl(url: string): boolean {
|
||||||
|
try {
|
||||||
|
const parsed = new URL(url)
|
||||||
|
return LOOPBACK_HOSTS.has(parsed.hostname)
|
||||||
|
} catch {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
function isPortStartFailure(error: unknown, port: number): boolean {
|
function isPortStartFailure(error: unknown, port: number): boolean {
|
||||||
if (!(error instanceof Error)) {
|
if (!(error instanceof Error)) {
|
||||||
return false
|
return false
|
||||||
@@ -41,7 +52,9 @@ export async function createServerConnection(options: {
|
|||||||
if (attach !== undefined) {
|
if (attach !== undefined) {
|
||||||
console.log(pc.dim("Attaching to existing server at"), pc.cyan(attach))
|
console.log(pc.dim("Attaching to existing server at"), pc.cyan(attach))
|
||||||
const client = createOpencodeClient({ baseUrl: attach })
|
const client = createOpencodeClient({ baseUrl: attach })
|
||||||
injectServerAuthIntoClient(client)
|
if (isLoopbackAttachUrl(attach)) {
|
||||||
|
injectServerAuthIntoClient(client)
|
||||||
|
}
|
||||||
return { client, cleanup: () => {} }
|
return { client, cleanup: () => {} }
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user