diff --git a/agents/file-explorer/code-searcher.ts b/agents/file-explorer/code-searcher.ts index 43fee7795..68f91659b 100644 --- a/agents/file-explorer/code-searcher.ts +++ b/agents/file-explorer/code-searcher.ts @@ -85,6 +85,7 @@ const codeSearcher: SecretAgentDefinition = { yield { toolName: 'set_output', input: { + message: '', results: toolResults, }, includeToolCall: false, diff --git a/cli/release/package.json b/cli/release/package.json index eca1cf503..3a88e099e 100644 --- a/cli/release/package.json +++ b/cli/release/package.json @@ -1,6 +1,6 @@ { "name": "codebuff", - "version": "1.0.673", + "version": "1.0.674", "description": "AI coding agent", "license": "MIT", "bin": { diff --git a/cli/src/components/tools/__tests__/code-search.test.tsx b/cli/src/components/tools/__tests__/code-search.test.tsx new file mode 100644 index 000000000..590e43517 --- /dev/null +++ b/cli/src/components/tools/__tests__/code-search.test.tsx @@ -0,0 +1,45 @@ +import { describe, expect, test } from 'bun:test' +import React from 'react' +import { renderToStaticMarkup } from 'react-dom/server' + +import { initializeThemeStore } from '../../../hooks/use-theme' +import { CodeSearchComponent } from '../code-search' + +import type { ChatTheme } from '../../../types/theme-system' +import type { ToolBlock } from '../types' + +initializeThemeStore() + +const createToolBlock = ( + output?: string, +): ToolBlock & { toolName: 'code_search' } => ({ + type: 'tool', + toolName: 'code_search', + toolCallId: 'code-search-test', + input: { + pattern: 'getAgentBaseName', + cwd: 'cli/src/utils', + }, + output, +}) + +describe('CodeSearchComponent', () => { + test('uses formatted match count from current code search output', () => { + const result = CodeSearchComponent.render( + createToolBlock(`Found 2 matches +./message-block-helpers.ts: +Line 13: export const getAgentBaseName = (type: string): string => { +Line 196: getAgentBaseName(options.agentType ?? '') === 'code-searcher'`), + {} as ChatTheme, + { + availableWidth: 80, + indentationOffset: 0, + labelWidth: 10, + }, + ) + + const markup = renderToStaticMarkup(<>{result.content}) + + expect(markup).toContain('getAgentBaseName in cli/src/utils (2 results)') + }) +}) diff --git a/cli/src/components/tools/code-search.tsx b/cli/src/components/tools/code-search.tsx index aff023ca2..47d007fee 100644 --- a/cli/src/components/tools/code-search.tsx +++ b/cli/src/components/tools/code-search.tsx @@ -23,13 +23,22 @@ export const CodeSearchComponent = defineToolComponent({ if (toolBlock.output && typeof toolBlock.output === 'string') { const lines = toolBlock.output.split('\n') + const matchCountLine = lines.find((line) => + /^Found \d+ matches?$/.test(line.trim()), + ) + const parsedTotalResults = matchCountLine + ?.trim() + .match(/^Found (\d+) matches?$/)?.[1] - for (const line of lines) { - const trimmed = line.trim() + if (parsedTotalResults !== undefined) { + totalResults = Number(parsedTotalResults) + } else { + for (const line of lines) { + const trimmed = line.trim() - // Result lines start with a number followed by a colon - if (/^\d+:/.test(trimmed)) { - totalResults++ + if (/^(?:Line\s+)?\d+:/.test(trimmed)) { + totalResults++ + } } } } @@ -52,12 +61,7 @@ export const CodeSearchComponent = defineToolComponent({ // Return as content using SimpleToolCallItem return { - content: ( - - ), + content: , } }, }) diff --git a/cli/src/utils/__tests__/message-block-helpers.test.ts b/cli/src/utils/__tests__/message-block-helpers.test.ts index d813de400..55d66522b 100644 --- a/cli/src/utils/__tests__/message-block-helpers.test.ts +++ b/cli/src/utils/__tests__/message-block-helpers.test.ts @@ -376,6 +376,23 @@ describe('extractSpawnAgentResultContent', () => { hasError: false, }) }) + + test('uses an empty structuredOutput message as no display content', () => { + const result = extractSpawnAgentResultContent({ + type: 'structuredOutput', + value: { + message: '', + results: [ + { + stdout: 'Found 1 match\n./file.ts:\nLine 1: needle', + message: 'Exit code: 0', + }, + ], + }, + }) + + expect(result).toEqual({ content: '', hasError: false }) + }) }) describe('appendInterruptionNotice', () => { diff --git a/common/src/constants/model-config.ts b/common/src/constants/model-config.ts index 1a6faadaf..494118b80 100644 --- a/common/src/constants/model-config.ts +++ b/common/src/constants/model-config.ts @@ -54,7 +54,6 @@ export type openrouterModel = (typeof openrouterModels)[keyof typeof openrouterModels] export const openCodeZenModels = { - opencode_minimax_m2_7: 'opencode/minimax-m2.7', opencode_kimi_k2_6: 'opencode/kimi-k2.6', } as const export type OpenCodeZenModel = diff --git a/docs/authentication.md b/docs/authentication.md index d4054b87f..b0dcb4bbd 100644 --- a/docs/authentication.md +++ b/docs/authentication.md @@ -19,7 +19,7 @@ sequenceDiagram CLI->>CLI: Open browser Note over Web: User completes OAuth Web->>DB: Resolve opaque token to signed payload - Web->>DB: Delete opaque token + Web->>DB: Mark opaque token consumed Web->>DB: Check fingerprint ownership Web->>DB: Create/update session loop Every 5s @@ -74,7 +74,7 @@ sequenceDiagram - Signed auth payloads expire after 1 hour - Browser login URLs use opaque 43-character tokens instead of exposing the signed auth payload -- Opaque browser tokens are stored in `verificationToken` under `cli-login:` and consumed with `DELETE ... RETURNING` when onboarding resolves them +- Opaque browser tokens are stored in `verificationToken` under `cli-login:` and atomically moved to `cli-login-consumed:` when onboarding resolves them; consumed markers scrub the signed auth payload from the `token` column - Fingerprint uniqueness: hardware info + 8 random bytes - Ownership conflicts blocked and logged - Sessions linked to fingerprint_id in database diff --git a/docs/testing.md b/docs/testing.md index dcc8ee4e7..3862f66ad 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -9,3 +9,37 @@ CLI hook testing note: React 19 + Bun + RTL `renderHook()` is unreliable; prefer ## CLI tmux Testing For testing CLI behavior via tmux, use the helper scripts in `scripts/tmux/`. These handle bracketed paste mode and session logging automatically. Session data is saved to `debug/tmux-sessions/` in YAML format and can be viewed with `bun scripts/tmux/tmux-viewer/index.tsx`. See `scripts/tmux/README.md` for details. + +Useful workflow for agents: + +```bash +# Start the dev CLI in a detached tmux session. +SESSION=$(./scripts/tmux/tmux-cli.sh start --name cli-check -w 160 -h 40 --wait 6) + +# Capture the initial screen. Captures are written to debug/tmux-sessions/$SESSION/. +./scripts/tmux/tmux-cli.sh capture "$SESSION" --label initial + +# Send a prompt. The helper uses bracketed paste so text is not dropped. +./scripts/tmux/tmux-cli.sh send "$SESSION" "Search for getAgentBaseName and report what you find" --wait-idle 4 + +# Capture after the run, then inspect the saved capture text. +./scripts/tmux/tmux-cli.sh capture "$SESSION" --label after-search --wait 2 + +# Clean up when finished. +./scripts/tmux/tmux-cli.sh stop "$SESSION" +``` + +If a change can be verified with a small local harness instead of a live model-backed CLI run, run that harness inside tmux too. This still checks terminal rendering and produces a capture: + +```bash +SESSION=$(./scripts/tmux/tmux-cli.sh start \ + --name render-check \ + -w 160 -h 20 \ + --wait 1 \ + --command "bun .context/my-render-check.tsx") + +./scripts/tmux/tmux-cli.sh capture "$SESSION" --label rendered +./scripts/tmux/tmux-cli.sh stop "$SESSION" +``` + +When verifying UI output, prefer checking the saved capture file for concrete strings that should and should not appear. For example, after expanding a code-searcher agent, check that the capture shows the search summary but not raw structured payload keys like `results:` or `stdout:`. diff --git a/freebuff/cli/release/package.json b/freebuff/cli/release/package.json index ab5597722..39ea940a9 100644 --- a/freebuff/cli/release/package.json +++ b/freebuff/cli/release/package.json @@ -1,6 +1,6 @@ { "name": "freebuff", - "version": "0.0.84", + "version": "0.0.85", "description": "The world's strongest free coding agent", "license": "MIT", "bin": { diff --git a/freebuff/web/src/app/api/auth/cli/code/route.ts b/freebuff/web/src/app/api/auth/cli/code/route.ts index 6622af094..734d5e4e0 100644 --- a/freebuff/web/src/app/api/auth/cli/code/route.ts +++ b/freebuff/web/src/app/api/auth/cli/code/route.ts @@ -8,7 +8,11 @@ import { and, eq, gt } from 'drizzle-orm' import { NextResponse } from 'next/server' import { z } from 'zod/v4' -import { buildCliAuthCode } from '@/app/onboard/_helpers' +import { + buildCliAuthCode, + getCliAuthCodeHashPrefix, + getCliAuthCodeTokenIdentifier, +} from '@/app/onboard/_helpers' import { logger } from '@/util/logger' import { getLoginUrlOrigin } from './_origin' @@ -66,7 +70,7 @@ export async function POST(req: Request) { const loginToken = randomBytes(32).toString('base64url') await db.insert(schema.verificationToken).values({ - identifier: `cli-login:${loginToken}`, + identifier: getCliAuthCodeTokenIdentifier(loginToken), token: authCode, expires: new Date(expiresAt), }) @@ -82,6 +86,25 @@ export async function POST(req: Request) { ) loginUrl.searchParams.set('auth_code', loginToken) + logger.info( + { + authCodeTokenHashPrefix: getCliAuthCodeHashPrefix(loginToken), + authCodeTokenLength: loginToken.length, + fingerprintIdPrefix: fingerprintId.slice(0, 24), + fingerprintIdLength: fingerprintId.length, + expiresAt, + loginUrlOrigin: loginUrl.origin, + requestOrigin: new URL(req.url).origin, + requestHost: req.headers.get('host'), + forwardedHost: req.headers.get('x-forwarded-host'), + forwardedProto: req.headers.get('x-forwarded-proto'), + originHeader: req.headers.get('origin'), + configuredAppUrl: env.NEXT_PUBLIC_CODEBUFF_APP_URL, + environment: env.NEXT_PUBLIC_CB_ENVIRONMENT, + }, + 'Issued Freebuff CLI auth code token', + ) + return NextResponse.json({ fingerprintId, fingerprintHash, diff --git a/freebuff/web/src/app/onboard/__tests__/helpers.test.ts b/freebuff/web/src/app/onboard/__tests__/helpers.test.ts index 0a19061b8..812360443 100644 --- a/freebuff/web/src/app/onboard/__tests__/helpers.test.ts +++ b/freebuff/web/src/app/onboard/__tests__/helpers.test.ts @@ -3,6 +3,10 @@ import { afterEach, beforeEach, describe, expect, test } from 'bun:test' import { buildCliAuthCode, + getCliAuthCodeHashPrefix, + getCliAuthCodeTokenIdentifier, + getConsumedCliAuthCodeTokenIdentifier, + getConsumedCliAuthCodeTokenValue, isAuthCodeExpired, isOpaqueCliAuthCodeToken, parseAuthCode, @@ -110,6 +114,23 @@ describe('freebuff onboard/_helpers', () => { expect(isOpaqueCliAuthCodeToken(`${'A'.repeat(42)}.`)).toBe(false) }) + test('hashes auth codes for log correlation without logging the token', () => { + expect(getCliAuthCodeHashPrefix('a'.repeat(43))).toBe('66d34fba71f8') + expect(getCliAuthCodeHashPrefix(` ${'a'.repeat(43)}\n`)).toBe( + '66d34fba71f8', + ) + }) + + test('builds active and consumed token identifiers', () => { + expect(getCliAuthCodeTokenIdentifier('token-123')).toBe( + 'cli-login:token-123', + ) + expect(getConsumedCliAuthCodeTokenIdentifier('token-123')).toBe( + 'cli-login-consumed:034192845dc489deca291f9f5ae0bb8e5472c991020bf64b3ebc6dec5a1d7e47', + ) + expect(getConsumedCliAuthCodeTokenValue()).toBe('consumed') + }) + test('resolves an opaque browser token before validation', async () => { const expiresAt = '4102444800000' const fingerprintHash = genAuthCode( @@ -126,10 +147,11 @@ describe('freebuff onboard/_helpers', () => { const result = await resolveCliAuthCode(opaqueToken, async (token) => { expect(token).toBe(opaqueToken) - return signedAuthCode + return { status: 'resolved', authCode: signedAuthCode } }) expect(result).toEqual({ + status: 'ready', authCode: signedAuthCode, resolvedOpaqueToken: true, }) @@ -155,16 +177,47 @@ describe('freebuff onboard/_helpers', () => { const result = await resolveCliAuthCode(signedAuthCode, async () => { lookedUp = true - return null + return { status: 'missing' } }) expect(lookedUp).toBe(false) expect(result).toEqual({ + status: 'ready', authCode: signedAuthCode, resolvedOpaqueToken: false, }) }) + test('classifies reused opaque browser tokens as already consumed', async () => { + const opaqueToken = 'c'.repeat(43) + + const result = await resolveCliAuthCode(opaqueToken, async (token) => { + expect(token).toBe(opaqueToken) + return { status: 'already_consumed' } + }) + + expect(result).toEqual({ + status: 'already_consumed', + authCode: opaqueToken, + resolvedOpaqueToken: false, + }) + }) + + test('keeps never-issued opaque browser tokens invalid', async () => { + const opaqueToken = 'd'.repeat(43) + + const result = await resolveCliAuthCode(opaqueToken, async (token) => { + expect(token).toBe(opaqueToken) + return { status: 'missing' } + }) + + expect(result).toEqual({ + status: 'missing', + authCode: opaqueToken, + resolvedOpaqueToken: false, + }) + }) + test('resolves expired stored payloads so callers can show expired', async () => { const expiresAt = '0' const fingerprintHash = genAuthCode( @@ -178,10 +231,10 @@ describe('freebuff onboard/_helpers', () => { fingerprintHash, ) - const result = await resolveCliAuthCode( - 'b'.repeat(43), - async () => signedAuthCode, - ) + const result = await resolveCliAuthCode('b'.repeat(43), async () => ({ + status: 'resolved', + authCode: signedAuthCode, + })) const parsed = parseAuthCode(result.authCode) expect(isAuthCodeExpired(parsed.expiresAt)).toBe(true) diff --git a/freebuff/web/src/app/onboard/_db.ts b/freebuff/web/src/app/onboard/_db.ts index cf9724b16..50b0a9844 100644 --- a/freebuff/web/src/app/onboard/_db.ts +++ b/freebuff/web/src/app/onboard/_db.ts @@ -6,6 +6,13 @@ import { cookies } from 'next/headers' import { logger } from '@/util/logger' +import { + getCliAuthCodeTokenIdentifier, + getConsumedCliAuthCodeTokenIdentifier, + getConsumedCliAuthCodeTokenValue, + type CliAuthCodeTokenConsumeResult, +} from './_helpers' + type DbTransaction = Parameters[0] extends ( tx: infer T, ) => any @@ -34,15 +41,53 @@ export async function hasCliSessionForAuthHash( export async function consumeCliAuthCodeToken( authCodeToken: string, -): Promise { - const deleted = await db - .delete(schema.verificationToken) +): Promise { + const activeIdentifier = getCliAuthCodeTokenIdentifier(authCodeToken) + const consumedIdentifier = + getConsumedCliAuthCodeTokenIdentifier(authCodeToken) + const getConsumedTokenStatus = + async (): Promise => { + const existingConsumed = await db + .select({ id: schema.verificationToken.identifier }) + .from(schema.verificationToken) + .where(eq(schema.verificationToken.identifier, consumedIdentifier)) + .limit(1) + + return existingConsumed[0] + ? { status: 'already_consumed' } + : { status: 'missing' } + } + + const active = await db + .select({ authCode: schema.verificationToken.token }) + .from(schema.verificationToken) + .where(eq(schema.verificationToken.identifier, activeIdentifier)) + .limit(1) + const authCode = active[0]?.authCode + + if (!authCode) { + return getConsumedTokenStatus() + } + + const consumed = await db + .update(schema.verificationToken) + .set({ + identifier: consumedIdentifier, + token: getConsumedCliAuthCodeTokenValue(), + }) .where( - eq(schema.verificationToken.identifier, `cli-login:${authCodeToken}`), + and( + eq(schema.verificationToken.identifier, activeIdentifier), + eq(schema.verificationToken.token, authCode), + ), ) - .returning({ authCode: schema.verificationToken.token }) + .returning({ id: schema.verificationToken.identifier }) + + if (consumed[0]) { + return { status: 'resolved', authCode } + } - return deleted[0]?.authCode ?? null + return getConsumedTokenStatus() } export async function checkFingerprintConflict( diff --git a/freebuff/web/src/app/onboard/_helpers.ts b/freebuff/web/src/app/onboard/_helpers.ts index a3daf585a..58d5204a5 100644 --- a/freebuff/web/src/app/onboard/_helpers.ts +++ b/freebuff/web/src/app/onboard/_helpers.ts @@ -1,6 +1,15 @@ +import { createHash } from 'node:crypto' + import { genAuthCode } from '@codebuff/common/util/credentials' const OPAQUE_CLI_AUTH_CODE_TOKEN_RE = /^[A-Za-z0-9_-]{43}$/ +const CLI_AUTH_CODE_TOKEN_IDENTIFIER_PREFIX = 'cli-login:' +const CONSUMED_CLI_AUTH_CODE_TOKEN_IDENTIFIER_PREFIX = 'cli-login-consumed:' +const CONSUMED_CLI_AUTH_CODE_TOKEN_VALUE = 'consumed' + +function getCliAuthCodeHash(authCode: string): string { + return createHash('sha256').update(authCode.trim()).digest('hex') +} export function buildCliAuthCode( fingerprintId: string, @@ -14,23 +23,84 @@ export function isOpaqueCliAuthCodeToken(authCode: string): boolean { return OPAQUE_CLI_AUTH_CODE_TOKEN_RE.test(authCode.trim()) } +export function getCliAuthCodeHashPrefix(authCode: string): string { + return getCliAuthCodeHash(authCode).slice(0, 12) +} + +export function getCliAuthCodeTokenIdentifier(authCodeToken: string): string { + return `${CLI_AUTH_CODE_TOKEN_IDENTIFIER_PREFIX}${authCodeToken}` +} + +export function getConsumedCliAuthCodeTokenIdentifier( + authCodeToken: string, +): string { + return `${CONSUMED_CLI_AUTH_CODE_TOKEN_IDENTIFIER_PREFIX}${getCliAuthCodeHash( + authCodeToken, + )}` +} + +export function getConsumedCliAuthCodeTokenValue(): string { + return CONSUMED_CLI_AUTH_CODE_TOKEN_VALUE +} + +export type CliAuthCodeTokenConsumeResult = + | { status: 'resolved'; authCode: string } + | { status: 'already_consumed' } + | { status: 'missing' } + +export type CliAuthCodeResolution = + | { + status: 'ready' + authCode: string + resolvedOpaqueToken: boolean + } + | { + status: 'already_consumed' + authCode: string + resolvedOpaqueToken: false + } + | { + status: 'missing' + authCode: string + resolvedOpaqueToken: false + } + export async function resolveCliAuthCode( authCode: string, - consumeCliAuthCodeToken: (authCodeToken: string) => Promise, -): Promise<{ authCode: string; resolvedOpaqueToken: boolean }> { + consumeCliAuthCodeToken: ( + authCodeToken: string, + ) => Promise, +): Promise { const normalizedAuthCode = authCode.trim() if (!isOpaqueCliAuthCodeToken(normalizedAuthCode)) { - return { authCode: normalizedAuthCode, resolvedOpaqueToken: false } + return { + status: 'ready', + authCode: normalizedAuthCode, + resolvedOpaqueToken: false, + } } - const signedAuthCode = await consumeCliAuthCodeToken(normalizedAuthCode) - if (!signedAuthCode) { - return { authCode: normalizedAuthCode, resolvedOpaqueToken: false } + const tokenResult = await consumeCliAuthCodeToken(normalizedAuthCode) + if (tokenResult.status === 'resolved') { + return { + status: 'ready', + authCode: tokenResult.authCode, + resolvedOpaqueToken: true, + } + } + + if (tokenResult.status === 'already_consumed') { + return { + status: 'already_consumed', + authCode: normalizedAuthCode, + resolvedOpaqueToken: false, + } } return { - authCode: signedAuthCode, - resolvedOpaqueToken: true, + status: 'missing', + authCode: normalizedAuthCode, + resolvedOpaqueToken: false, } } diff --git a/freebuff/web/src/app/onboard/page.tsx b/freebuff/web/src/app/onboard/page.tsx index e39a4a0b3..74ba63ee9 100644 --- a/freebuff/web/src/app/onboard/page.tsx +++ b/freebuff/web/src/app/onboard/page.tsx @@ -12,7 +12,9 @@ import { hasCliSessionForAuthHash, } from './_db' import { + getCliAuthCodeHashPrefix, isAuthCodeExpired, + isOpaqueCliAuthCodeToken, parseAuthCode, resolveCliAuthCode, validateAuthCode, @@ -97,8 +99,37 @@ const Onboard = async ({ searchParams }: PageProps) => { ) } - const { authCode: resolvedAuthCode, resolvedOpaqueToken } = - await resolveCliAuthCode(authCode, consumeCliAuthCodeToken) + const authCodeResolution = await resolveCliAuthCode( + authCode, + consumeCliAuthCodeToken, + ) + + if (authCodeResolution.status === 'already_consumed') { + logger.info( + { + authCodeLength: authCode.length, + authCodeTrimmedLength: authCode.trim().length, + authCodeHashPrefix: getCliAuthCodeHashPrefix(authCode), + isOpaqueAuthCodeToken: isOpaqueCliAuthCodeToken(authCode), + userId: user.id, + }, + 'Reused Freebuff CLI auth code token', + ) + + return ( + + ) + } + + const { + authCode: resolvedAuthCode, + resolvedOpaqueToken, + status: authCodeResolutionStatus, + } = authCodeResolution const { fingerprintId, expiresAt, receivedHash } = parseAuthCode(resolvedAuthCode) const { valid, expectedHash: fingerprintHash } = validateAuthCode( @@ -112,8 +143,13 @@ const Onboard = async ({ searchParams }: PageProps) => { logger.warn( { authCodeLength: authCode.length, + authCodeTrimmedLength: authCode.trim().length, + authCodeHashPrefix: getCliAuthCodeHashPrefix(authCode), + isOpaqueAuthCodeToken: isOpaqueCliAuthCodeToken(authCode), + authCodeResolutionStatus, resolvedAuthCode: resolvedOpaqueToken, resolvedAuthCodeLength: resolvedAuthCode.length, + userId: user.id, dotCount: authCode.match(/\./g)?.length ?? 0, hyphenCount: authCode.match(/-/g)?.length ?? 0, fingerprintIdPrefix: fingerprintId.slice(0, 24), diff --git a/sdk/src/__tests__/change-file.test.ts b/sdk/src/__tests__/change-file.test.ts index dff8969c7..656244906 100644 --- a/sdk/src/__tests__/change-file.test.ts +++ b/sdk/src/__tests__/change-file.test.ts @@ -36,6 +36,37 @@ describe('changeFile', () => { ) }) + test('tolerates absolute paths inside the project for string replacements', async () => { + const fs = createMockFs({ + files: { + '/repo/src/file.ts': 'const value = 1\n', + }, + }) + + const result = await changeFile({ + parameters: { + type: 'patch', + path: '/repo/src/file.ts', + content: '@@ -1,1 +1,1 @@\n-const value = 1\n+const value = 2\n', + }, + cwd: '/repo', + fs, + }) + + expect(result).toEqual([ + { + type: 'json', + value: { + file: 'src/file.ts', + message: 'String replace applied successfully.', + }, + }, + ]) + expect(await fs.readFile('/repo/src/file.ts', 'utf-8')).toBe( + 'const value = 2\n', + ) + }) + test('returns a simple success message for new file writes', async () => { const fs = createMockFs() @@ -63,6 +94,58 @@ describe('changeFile', () => { ) }) + test('tolerates absolute paths inside the project for file writes', async () => { + const fs = createMockFs() + + const result = await changeFile({ + parameters: { + type: 'file', + path: '/repo/src/file.ts', + content: 'const value = 1\n', + }, + cwd: '/repo', + fs, + }) + + expect(result).toEqual([ + { + type: 'json', + value: { + file: 'src/file.ts', + message: 'Created file successfully.', + }, + }, + ]) + expect(await fs.readFile('/repo/src/file.ts', 'utf-8')).toBe( + 'const value = 1\n', + ) + }) + + test('accepts paths whose file names start with two dots inside the project', async () => { + const fs = createMockFs() + + const result = await changeFile({ + parameters: { + type: 'file', + path: '/repo/..config', + content: 'value = true\n', + }, + cwd: '/repo', + fs, + }) + + expect(result).toEqual([ + { + type: 'json', + value: { + file: '..config', + message: 'Created file successfully.', + }, + }, + ]) + expect(await fs.readFile('/repo/..config', 'utf-8')).toBe('value = true\n') + }) + test('returns a simple success message for overwritten file writes', async () => { const fs = createMockFs({ files: { @@ -93,4 +176,20 @@ describe('changeFile', () => { 'const value = 2\n', ) }) + + test('rejects absolute paths outside the project', async () => { + const fs = createMockFs() + + await expect( + changeFile({ + parameters: { + type: 'file', + path: '/outside/file.ts', + content: 'const value = 1\n', + }, + cwd: '/repo', + fs, + }), + ).rejects.toThrow('file path is outside the project directory') + }) }) diff --git a/sdk/src/__tests__/path-utils.test.ts b/sdk/src/__tests__/path-utils.test.ts new file mode 100644 index 000000000..4910dbcaf --- /dev/null +++ b/sdk/src/__tests__/path-utils.test.ts @@ -0,0 +1,58 @@ +import { describe, expect, test } from 'bun:test' + +import { + getProjectPathLookupKeys, + resolveFilePathWithinProject, +} from '../tools/path-utils' + +describe('resolveFilePathWithinProject', () => { + test('normalizes relative paths to full and project-relative paths', () => { + expect(resolveFilePathWithinProject('/repo', 'src/file.ts')).toEqual({ + fullPath: '/repo/src/file.ts', + relativePath: 'src/file.ts', + }) + }) + + test('normalizes absolute paths inside the project', () => { + expect(resolveFilePathWithinProject('/repo', '/repo/src/file.ts')).toEqual({ + fullPath: '/repo/src/file.ts', + relativePath: 'src/file.ts', + }) + }) + + test('allows file names that start with two dots inside the project', () => { + expect(resolveFilePathWithinProject('/repo', '/repo/..config')).toEqual({ + fullPath: '/repo/..config', + relativePath: '..config', + }) + }) + + test('rejects paths outside the project', () => { + expect(resolveFilePathWithinProject('/repo', '../outside.ts')).toBeNull() + expect(resolveFilePathWithinProject('/repo', '/outside.ts')).toBeNull() + expect( + resolveFilePathWithinProject('/repo', '/repo-sibling/file.ts'), + ).toBeNull() + }) +}) + +describe('getProjectPathLookupKeys', () => { + test('returns the normalized relative key before the original absolute key', () => { + expect(getProjectPathLookupKeys('/repo', '/repo/src/file.ts')).toEqual([ + 'src/file.ts', + '/repo/src/file.ts', + ]) + }) + + test('dedupes relative paths that are already normalized', () => { + expect(getProjectPathLookupKeys('/repo', 'src/file.ts')).toEqual([ + 'src/file.ts', + ]) + }) + + test('returns only the original key for paths outside the project', () => { + expect(getProjectPathLookupKeys('/repo', '/outside.ts')).toEqual([ + '/outside.ts', + ]) + }) +}) diff --git a/sdk/src/__tests__/read-files.test.ts b/sdk/src/__tests__/read-files.test.ts index 965662286..afcafb7ac 100644 --- a/sdk/src/__tests__/read-files.test.ts +++ b/sdk/src/__tests__/read-files.test.ts @@ -11,13 +11,11 @@ import { spyOn, } from 'bun:test' - import { getFiles } from '../tools/read-files' import type { CodebuffFileSystem } from '@codebuff/common/types/filesystem' import type { PathLike } from 'node:fs' - // Helper to create a mock filesystem function createMockFs(config: { files?: Record @@ -75,9 +73,10 @@ describe('getFiles', () => { beforeEach(() => { // Default: no files are ignored - isFileIgnoredSpy = spyOn(projectFileTree, 'isFileIgnored').mockResolvedValue( - false, - ) + isFileIgnoredSpy = spyOn( + projectFileTree, + 'isFileIgnored', + ).mockResolvedValue(false) }) afterEach(() => { @@ -320,9 +319,7 @@ describe('getFiles', () => { test('should handle mix of ignored and non-ignored files', async () => { // First call returns false (not ignored), second returns true (ignored) - isFileIgnoredSpy - .mockResolvedValueOnce(false) - .mockResolvedValueOnce(true) + isFileIgnoredSpy.mockResolvedValueOnce(false).mockResolvedValueOnce(true) const mockFs = createMockFs({ files: { @@ -393,7 +390,10 @@ describe('getFiles', () => { const mockFs = createMockFs({ files: {}, errors: { - '/project/broken.ts': { code: 'EACCES', message: 'Permission denied' }, + '/project/broken.ts': { + code: 'EACCES', + message: 'Permission denied', + }, }, }) @@ -423,6 +423,24 @@ describe('getFiles', () => { expect(result['src/index.ts']).toBe('content') }) + + test('should reject absolute paths in sibling directories with matching prefixes', async () => { + const mockFs = createMockFs({ + files: { + '/project-other/src/index.ts': { content: 'outside' }, + }, + }) + + const result = await getFiles({ + filePaths: ['/project-other/src/index.ts'], + cwd: '/project', + fs: mockFs, + }) + + expect(result['/project-other/src/index.ts']).toBe( + FILE_READ_STATUS.OUTSIDE_PROJECT, + ) + }) }) describe('fileFilter option', () => { diff --git a/sdk/src/__tests__/run-file-filter.test.ts b/sdk/src/__tests__/run-file-filter.test.ts index 9f49aff80..5d1be280a 100644 --- a/sdk/src/__tests__/run-file-filter.test.ts +++ b/sdk/src/__tests__/run-file-filter.test.ts @@ -1,4 +1,3 @@ - import * as mainPromptModule from '@codebuff/agent-runtime/main-prompt' import { FILE_READ_STATUS } from '@codebuff/common/old-constants' import * as projectFileTree from '@codebuff/common/project-file-tree' @@ -91,9 +90,7 @@ describe('CodebuffClientOptions fileFilter', () => { let requestedFiles: Record = {} spyOn(mainPromptModule, 'callMainPrompt').mockImplementation( - async ( - params: Parameters[0], - ) => { + async (params: Parameters[0]) => { const { sendAction, promptId, requestFiles } = params const sessionState = getInitialSessionState(getStubProjectFileContext()) @@ -177,9 +174,7 @@ describe('CodebuffClientOptions fileFilter', () => { let requestedFiles: Record = {} spyOn(mainPromptModule, 'callMainPrompt').mockImplementation( - async ( - params: Parameters[0], - ) => { + async (params: Parameters[0]) => { const { sendAction, promptId, requestFiles } = params const sessionState = getInitialSessionState(getStubProjectFileContext()) @@ -259,9 +254,7 @@ describe('CodebuffClientOptions fileFilter', () => { let optionalFileResult: string | null = null spyOn(mainPromptModule, 'callMainPrompt').mockImplementation( - async ( - params: Parameters[0], - ) => { + async (params: Parameters[0]) => { const { sendAction, promptId, requestOptionalFile } = params const sessionState = getInitialSessionState(getStubProjectFileContext()) @@ -319,6 +312,75 @@ describe('CodebuffClientOptions fileFilter', () => { expect(optionalFileResult).toBeNull() }) + it('should tolerate absolute requestOptionalFile paths inside cwd', async () => { + spyOn(databaseModule, 'getUserInfoFromApiKey').mockResolvedValue({ + id: 'user-123', + email: 'test@example.com', + discord_id: null, + stripe_customer_id: null, + banned: false, + created_at: new Date('2024-01-01T00:00:00Z'), + }) + spyOn(databaseModule, 'fetchAgentFromDatabase').mockResolvedValue(null) + spyOn(databaseModule, 'startAgentRun').mockResolvedValue('run-1') + spyOn(databaseModule, 'finishAgentRun').mockResolvedValue(undefined) + spyOn(databaseModule, 'addAgentStep').mockResolvedValue('step-1') + spyOn(projectFileTree, 'isFileIgnored').mockResolvedValue(false) + + const mockFs = createMockFs({ + files: { + '/project/src/index.ts': { content: 'normal file content' }, + }, + }) + + const optionalFileResult: { current: string | null } = { current: null } + + spyOn(mainPromptModule, 'callMainPrompt').mockImplementation( + async (params: Parameters[0]) => { + const { sendAction, promptId, requestOptionalFile } = params + const sessionState = getInitialSessionState(getStubProjectFileContext()) + + optionalFileResult.current = await requestOptionalFile({ + filePath: '/project/src/index.ts', + }) + + await sendAction({ + action: { + type: 'prompt-response', + promptId, + sessionState, + output: { + type: 'lastMessage', + value: [], + }, + }, + }) + + return { + sessionState, + output: { + type: 'lastMessage' as const, + value: [], + }, + } + }, + ) + + const client = new CodebuffClient({ + apiKey: 'test-key', + cwd: '/project', + fsSource: mockFs, + }) + + const result = await client.run({ + agent: 'base2', + prompt: 'read optional file', + }) + + expect(result.output.type).toBe('lastMessage') + expect(optionalFileResult.current).toBe('normal file content') + }) + it('should allow all files when no fileFilter is provided', async () => { spyOn(databaseModule, 'getUserInfoFromApiKey').mockResolvedValue({ id: 'user-123', @@ -343,9 +405,7 @@ describe('CodebuffClientOptions fileFilter', () => { let requestedFiles: Record = {} spyOn(mainPromptModule, 'callMainPrompt').mockImplementation( - async ( - params: Parameters[0], - ) => { + async (params: Parameters[0]) => { const { sendAction, promptId, requestFiles } = params const sessionState = getInitialSessionState(getStubProjectFileContext()) @@ -417,9 +477,7 @@ describe('CodebuffClientOptions fileFilter', () => { }) spyOn(mainPromptModule, 'callMainPrompt').mockImplementation( - async ( - params: Parameters[0], - ) => { + async (params: Parameters[0]) => { const { sendAction, promptId, requestFiles } = params const sessionState = getInitialSessionState(getStubProjectFileContext()) diff --git a/sdk/src/run.ts b/sdk/src/run.ts index 8d0c7986f..89044ab82 100644 --- a/sdk/src/run.ts +++ b/sdk/src/run.ts @@ -27,6 +27,7 @@ import { applyPatchTool } from './tools/apply-patch' import { codeSearch } from './tools/code-search' import { glob } from './tools/glob' import { listDirectory } from './tools/list-directory' +import { getProjectPathLookupKeys } from './tools/path-utils' import { getFiles } from './tools/read-files' import { runTerminalCommand } from './tools/run-terminal-command' @@ -434,7 +435,11 @@ async function runOnce({ cwd, fs, }) - return toOptionalFile(files[filePath] ?? null) + const lookupKeys = cwd + ? getProjectPathLookupKeys(cwd, filePath) + : [filePath] + const fileKey = lookupKeys.find((key) => key in files) + return toOptionalFile(fileKey === undefined ? null : files[fileKey]!) }, sendAction: ({ action }) => { if (action.type === 'action-error') { diff --git a/sdk/src/tools/change-file.ts b/sdk/src/tools/change-file.ts index ff34cc547..dbcb55eff 100644 --- a/sdk/src/tools/change-file.ts +++ b/sdk/src/tools/change-file.ts @@ -4,8 +4,11 @@ import { fileExists } from '@codebuff/common/util/file' import { applyPatch } from 'diff' import z from 'zod/v4' +import { resolveFilePathWithinProject } from './path-utils' + import type { CodebuffToolOutput } from '@codebuff/common/tools/list' import type { CodebuffFileSystem } from '@codebuff/common/types/filesystem' +import type { ResolvedProjectPath } from './path-utils' const FileChangeSchema = z.object({ type: z.enum(['patch', 'file']), @@ -13,20 +16,12 @@ const FileChangeSchema = z.object({ content: z.string(), }) -function containsUpwardTraversal(dirPath: string): boolean { - const normalized = path.normalize(dirPath) - return normalized.includes('..') -} +type FileChange = z.infer -/** - * Checks if a path contains path traversal sequences that would escape the root. - * Uses proper path normalization to prevent traversal attacks. - */ -function containsPathTraversal(filePath: string): boolean { - const normalized = path.normalize(filePath) - // Check for absolute paths or paths starting with .. that escape root - return path.isAbsolute(normalized) || normalized.startsWith('..') -} +type ApplyChangeResult = + | { status: 'created' | 'modified'; file: string } + | { status: 'patchFailed'; file: string; patch: string } + | { status: 'invalid'; file: string } export async function changeFile(params: { parameters: unknown @@ -35,117 +30,78 @@ export async function changeFile(params: { }): Promise> { const { parameters, cwd, fs } = params - if (containsUpwardTraversal(cwd)) { - throw new Error('cwd contains invalid path traversal') - } const fileChange = FileChangeSchema.parse(parameters) - if (containsPathTraversal(fileChange.path)) { - throw new Error('file path contains invalid path traversal') + const resolvedPath = resolveFilePathWithinProject(cwd, fileChange.path) + if (!resolvedPath) { + throw new Error('file path is outside the project directory') } - const { created, modified, invalid, patchFailed } = await applyChanges({ - projectRoot: cwd, - changes: [fileChange], - fs, - }) - - const results: CodebuffToolOutput<'str_replace'>[0]['value'][] = [] + const result = await applyChange({ change: fileChange, resolvedPath, fs }) - for (const file of created) { - results.push({ - file, - message: - fileChange.type === 'patch' - ? 'String replace applied successfully.' - : 'Created file successfully.', - }) - } + return [{ type: 'json', value: formatApplyChangeResult(result, fileChange) }] +} - for (const file of modified) { - results.push({ - file, +function formatApplyChangeResult( + result: ApplyChangeResult, + fileChange: FileChange, +): CodebuffToolOutput<'str_replace'>[0]['value'] { + if (result.status === 'created' || result.status === 'modified') { + return { + file: result.file, message: fileChange.type === 'patch' ? 'String replace applied successfully.' - : 'Overwrote file successfully.', - }) + : result.status === 'created' + ? 'Created file successfully.' + : 'Overwrote file successfully.', + } } - for (const file of patchFailed) { - results.push({ - file, + if (result.status === 'patchFailed') { + return { + file: result.file, errorMessage: `Failed to apply patch.`, - patch: fileChange.content, - }) - } - - for (const file of invalid) { - results.push({ - file, - errorMessage: - 'Failed to write to file: file path caused an error or file could not be written', - }) + patch: result.patch, + } } - if (results.length !== 1) { - throw new Error( - `Internal error: Unexpected result length while modifying files: ${ - results.length - }`, - ) + return { + file: result.file, + errorMessage: + 'Failed to write to file: file path caused an error or file could not be written', } - - return [{ type: 'json', value: results[0] }] } -async function applyChanges(params: { - projectRoot: string - changes: { - type: 'patch' | 'file' - path: string - content: string - }[] +async function applyChange(params: { + change: FileChange + resolvedPath: ResolvedProjectPath fs: CodebuffFileSystem -}) { - const { projectRoot, changes, fs } = params - - const created: string[] = [] - const modified: string[] = [] - const patchFailed: string[] = [] - const invalid: string[] = [] - - for (const change of changes) { - const { path: filePath, content, type } = change - try { - const fullPath = path.join(projectRoot, filePath) - const exists = await fileExists({ filePath: fullPath, fs }) - if (!exists) { - const dirPath = path.dirname(fullPath) - await fs.mkdir(dirPath, { recursive: true }) - } - - if (type === 'file') { - await fs.writeFile(fullPath, content) - } else { - const oldContent = await fs.readFile(fullPath, 'utf-8') - const newContent = applyPatch(oldContent, content) - if (newContent === false) { - patchFailed.push(filePath) - continue - } - await fs.writeFile(fullPath, newContent) - } +}): Promise { + const { change, resolvedPath, fs } = params + const { content, type } = change + const { fullPath, relativePath } = resolvedPath + + try { + const exists = await fileExists({ filePath: fullPath, fs }) + if (!exists) { + const dirPath = path.dirname(fullPath) + await fs.mkdir(dirPath, { recursive: true }) + } - if (exists) { - modified.push(filePath) - } else { - created.push(filePath) + if (type === 'file') { + await fs.writeFile(fullPath, content) + } else { + const oldContent = await fs.readFile(fullPath, 'utf-8') + const newContent = applyPatch(oldContent, content) + if (newContent === false) { + return { status: 'patchFailed', file: relativePath, patch: content } } - } catch (error) { - console.error(`Failed to apply patch to ${filePath}:`, error, content) - invalid.push(filePath) + await fs.writeFile(fullPath, newContent) } - } - return { created, modified, invalid, patchFailed } + return { status: exists ? 'modified' : 'created', file: relativePath } + } catch (error) { + console.error(`Failed to apply patch to ${relativePath}:`, error, content) + return { status: 'invalid', file: relativePath } + } } diff --git a/sdk/src/tools/path-utils.ts b/sdk/src/tools/path-utils.ts new file mode 100644 index 000000000..92fe8a132 --- /dev/null +++ b/sdk/src/tools/path-utils.ts @@ -0,0 +1,41 @@ +import path from 'path' + +export type ResolvedProjectPath = { + fullPath: string + relativePath: string +} + +function escapesProject(relativePath: string): boolean { + return ( + relativePath === '..' || + relativePath.startsWith(`..${path.sep}`) || + path.isAbsolute(relativePath) + ) +} + +export function resolveFilePathWithinProject( + projectRoot: string, + filePath: string, +): ResolvedProjectPath | null { + const resolvedRoot = path.resolve(projectRoot) + const fullPath = path.isAbsolute(filePath) + ? path.resolve(filePath) + : path.resolve(resolvedRoot, filePath) + const relativePath = path.relative(resolvedRoot, fullPath) + + if (relativePath === '' || escapesProject(relativePath)) { + return null + } + + return { fullPath, relativePath } +} + +export function getProjectPathLookupKeys( + projectRoot: string, + filePath: string, +): string[] { + const resolvedPath = resolveFilePathWithinProject(projectRoot, filePath) + const keys = resolvedPath ? [resolvedPath.relativePath, filePath] : [filePath] + + return [...new Set(keys)] +} diff --git a/sdk/src/tools/read-files.ts b/sdk/src/tools/read-files.ts index c3c85cc68..a6462f1a2 100644 --- a/sdk/src/tools/read-files.ts +++ b/sdk/src/tools/read-files.ts @@ -1,8 +1,8 @@ -import path, { isAbsolute } from 'path' - import { FILE_READ_STATUS } from '@codebuff/common/old-constants' import { isFileIgnored } from '@codebuff/common/project-file-tree' +import { resolveFilePathWithinProject } from './path-utils' + import type { CodebuffFileSystem } from '@codebuff/common/types/filesystem' export type FileFilterResult = { @@ -38,15 +38,12 @@ export async function getFiles(params: { continue } - // Convert absolute paths within project to relative paths - const relativePath = filePath.startsWith(cwd) - ? path.relative(cwd, filePath) - : filePath - const fullPath = path.join(cwd, relativePath) - if (isAbsolute(relativePath) || !fullPath.startsWith(cwd)) { - result[relativePath] = FILE_READ_STATUS.OUTSIDE_PROJECT + const resolvedPath = resolveFilePathWithinProject(cwd, filePath) + if (!resolvedPath) { + result[filePath] = FILE_READ_STATUS.OUTSIDE_PROJECT continue } + const { relativePath, fullPath } = resolvedPath // Apply file filter if provided const filterResult = fileFilter?.(relativePath) diff --git a/web/src/app/api/auth/cli/code/route.ts b/web/src/app/api/auth/cli/code/route.ts index 1149a46de..a677e9f09 100644 --- a/web/src/app/api/auth/cli/code/route.ts +++ b/web/src/app/api/auth/cli/code/route.ts @@ -8,7 +8,11 @@ import { and, eq, gt } from 'drizzle-orm' import { NextResponse } from 'next/server' import { z } from 'zod/v4' -import { buildCliAuthCode } from '@/app/onboard/_helpers' +import { + buildCliAuthCode, + getCliAuthCodeHashPrefix, + getCliAuthCodeTokenIdentifier, +} from '@/app/onboard/_helpers' import { logger } from '@/util/logger' import { getLoginUrlOrigin } from './_origin' @@ -68,7 +72,7 @@ export async function POST(req: Request) { const loginToken = randomBytes(32).toString('base64url') await db.insert(schema.verificationToken).values({ - identifier: `cli-login:${loginToken}`, + identifier: getCliAuthCodeTokenIdentifier(loginToken), token: authCode, expires: new Date(expiresAt), }) @@ -84,6 +88,25 @@ export async function POST(req: Request) { ) loginUrl.searchParams.set('auth_code', loginToken) + logger.info( + { + authCodeTokenHashPrefix: getCliAuthCodeHashPrefix(loginToken), + authCodeTokenLength: loginToken.length, + fingerprintIdPrefix: fingerprintId.slice(0, 24), + fingerprintIdLength: fingerprintId.length, + expiresAt, + loginUrlOrigin: loginUrl.origin, + requestOrigin: new URL(req.url).origin, + requestHost: req.headers.get('host'), + forwardedHost: req.headers.get('x-forwarded-host'), + forwardedProto: req.headers.get('x-forwarded-proto'), + originHeader: req.headers.get('origin'), + configuredAppUrl: env.NEXT_PUBLIC_CODEBUFF_APP_URL, + environment: env.NEXT_PUBLIC_CB_ENVIRONMENT, + }, + 'Issued Codebuff CLI auth code token', + ) + return NextResponse.json({ fingerprintId, fingerprintHash, diff --git a/web/src/app/api/v1/chat/completions/__tests__/completions.test.ts b/web/src/app/api/v1/chat/completions/__tests__/completions.test.ts index b72023e14..c1dd1e99f 100644 --- a/web/src/app/api/v1/chat/completions/__tests__/completions.test.ts +++ b/web/src/app/api/v1/chat/completions/__tests__/completions.test.ts @@ -869,10 +869,9 @@ describe('/api/v1/chat/completions POST endpoint', () => { ) it( - 'routes opencode/-prefixed models to the OpenCode Zen provider', + 'routes OpenCode Zen models to the direct OpenCode Zen provider', async () => { const expectedUpstreamModel: Record = { - 'opencode/minimax-m2.7': 'minimax-m2.7', 'opencode/kimi-k2.6': 'kimi-k2.6', } diff --git a/web/src/app/onboard/__tests__/helpers.test.ts b/web/src/app/onboard/__tests__/helpers.test.ts index c47c2f642..d3c0b4a9f 100644 --- a/web/src/app/onboard/__tests__/helpers.test.ts +++ b/web/src/app/onboard/__tests__/helpers.test.ts @@ -3,6 +3,10 @@ import { afterEach, beforeEach, describe, expect, test } from 'bun:test' import { buildCliAuthCode, + getCliAuthCodeHashPrefix, + getCliAuthCodeTokenIdentifier, + getConsumedCliAuthCodeTokenIdentifier, + getConsumedCliAuthCodeTokenValue, isAuthCodeExpired, isOpaqueCliAuthCodeToken, parseAuthCode, @@ -238,6 +242,23 @@ describe('onboard/_helpers', () => { expect(isOpaqueCliAuthCodeToken(`${'A'.repeat(42)}.`)).toBe(false) }) + test('hashes auth codes for log correlation without logging the token', () => { + expect(getCliAuthCodeHashPrefix('a'.repeat(43))).toBe('66d34fba71f8') + expect(getCliAuthCodeHashPrefix(` ${'a'.repeat(43)}\n`)).toBe( + '66d34fba71f8', + ) + }) + + test('builds active and consumed token identifiers', () => { + expect(getCliAuthCodeTokenIdentifier('token-123')).toBe( + 'cli-login:token-123', + ) + expect(getConsumedCliAuthCodeTokenIdentifier('token-123')).toBe( + 'cli-login-consumed:034192845dc489deca291f9f5ae0bb8e5472c991020bf64b3ebc6dec5a1d7e47', + ) + expect(getConsumedCliAuthCodeTokenValue()).toBe('consumed') + }) + test('resolves an opaque browser token before validation', async () => { const expiresAt = '4102444800000' const fingerprintHash = genAuthCode( @@ -254,10 +275,11 @@ describe('onboard/_helpers', () => { const result = await resolveCliAuthCode(opaqueToken, async (token) => { expect(token).toBe(opaqueToken) - return signedAuthCode + return { status: 'resolved', authCode: signedAuthCode } }) expect(result).toEqual({ + status: 'ready', authCode: signedAuthCode, resolvedOpaqueToken: true, }) @@ -283,16 +305,47 @@ describe('onboard/_helpers', () => { const result = await resolveCliAuthCode(signedAuthCode, async () => { lookedUp = true - return null + return { status: 'missing' } }) expect(lookedUp).toBe(false) expect(result).toEqual({ + status: 'ready', authCode: signedAuthCode, resolvedOpaqueToken: false, }) }) + test('classifies reused opaque browser tokens as already consumed', async () => { + const opaqueToken = 'c'.repeat(43) + + const result = await resolveCliAuthCode(opaqueToken, async (token) => { + expect(token).toBe(opaqueToken) + return { status: 'already_consumed' } + }) + + expect(result).toEqual({ + status: 'already_consumed', + authCode: opaqueToken, + resolvedOpaqueToken: false, + }) + }) + + test('keeps never-issued opaque browser tokens invalid', async () => { + const opaqueToken = 'd'.repeat(43) + + const result = await resolveCliAuthCode(opaqueToken, async (token) => { + expect(token).toBe(opaqueToken) + return { status: 'missing' } + }) + + expect(result).toEqual({ + status: 'missing', + authCode: opaqueToken, + resolvedOpaqueToken: false, + }) + }) + test('resolves expired stored payloads so callers can show expired', async () => { const expiresAt = '0' const fingerprintHash = genAuthCode( @@ -306,10 +359,10 @@ describe('onboard/_helpers', () => { fingerprintHash, ) - const result = await resolveCliAuthCode( - 'b'.repeat(43), - async () => signedAuthCode, - ) + const result = await resolveCliAuthCode('b'.repeat(43), async () => ({ + status: 'resolved', + authCode: signedAuthCode, + })) const parsed = parseAuthCode(result.authCode) expect(isAuthCodeExpired(parsed.expiresAt)).toBe(true) diff --git a/web/src/app/onboard/_db.ts b/web/src/app/onboard/_db.ts index cf9724b16..50b0a9844 100644 --- a/web/src/app/onboard/_db.ts +++ b/web/src/app/onboard/_db.ts @@ -6,6 +6,13 @@ import { cookies } from 'next/headers' import { logger } from '@/util/logger' +import { + getCliAuthCodeTokenIdentifier, + getConsumedCliAuthCodeTokenIdentifier, + getConsumedCliAuthCodeTokenValue, + type CliAuthCodeTokenConsumeResult, +} from './_helpers' + type DbTransaction = Parameters[0] extends ( tx: infer T, ) => any @@ -34,15 +41,53 @@ export async function hasCliSessionForAuthHash( export async function consumeCliAuthCodeToken( authCodeToken: string, -): Promise { - const deleted = await db - .delete(schema.verificationToken) +): Promise { + const activeIdentifier = getCliAuthCodeTokenIdentifier(authCodeToken) + const consumedIdentifier = + getConsumedCliAuthCodeTokenIdentifier(authCodeToken) + const getConsumedTokenStatus = + async (): Promise => { + const existingConsumed = await db + .select({ id: schema.verificationToken.identifier }) + .from(schema.verificationToken) + .where(eq(schema.verificationToken.identifier, consumedIdentifier)) + .limit(1) + + return existingConsumed[0] + ? { status: 'already_consumed' } + : { status: 'missing' } + } + + const active = await db + .select({ authCode: schema.verificationToken.token }) + .from(schema.verificationToken) + .where(eq(schema.verificationToken.identifier, activeIdentifier)) + .limit(1) + const authCode = active[0]?.authCode + + if (!authCode) { + return getConsumedTokenStatus() + } + + const consumed = await db + .update(schema.verificationToken) + .set({ + identifier: consumedIdentifier, + token: getConsumedCliAuthCodeTokenValue(), + }) .where( - eq(schema.verificationToken.identifier, `cli-login:${authCodeToken}`), + and( + eq(schema.verificationToken.identifier, activeIdentifier), + eq(schema.verificationToken.token, authCode), + ), ) - .returning({ authCode: schema.verificationToken.token }) + .returning({ id: schema.verificationToken.identifier }) + + if (consumed[0]) { + return { status: 'resolved', authCode } + } - return deleted[0]?.authCode ?? null + return getConsumedTokenStatus() } export async function checkFingerprintConflict( diff --git a/web/src/app/onboard/_helpers.ts b/web/src/app/onboard/_helpers.ts index a3daf585a..58d5204a5 100644 --- a/web/src/app/onboard/_helpers.ts +++ b/web/src/app/onboard/_helpers.ts @@ -1,6 +1,15 @@ +import { createHash } from 'node:crypto' + import { genAuthCode } from '@codebuff/common/util/credentials' const OPAQUE_CLI_AUTH_CODE_TOKEN_RE = /^[A-Za-z0-9_-]{43}$/ +const CLI_AUTH_CODE_TOKEN_IDENTIFIER_PREFIX = 'cli-login:' +const CONSUMED_CLI_AUTH_CODE_TOKEN_IDENTIFIER_PREFIX = 'cli-login-consumed:' +const CONSUMED_CLI_AUTH_CODE_TOKEN_VALUE = 'consumed' + +function getCliAuthCodeHash(authCode: string): string { + return createHash('sha256').update(authCode.trim()).digest('hex') +} export function buildCliAuthCode( fingerprintId: string, @@ -14,23 +23,84 @@ export function isOpaqueCliAuthCodeToken(authCode: string): boolean { return OPAQUE_CLI_AUTH_CODE_TOKEN_RE.test(authCode.trim()) } +export function getCliAuthCodeHashPrefix(authCode: string): string { + return getCliAuthCodeHash(authCode).slice(0, 12) +} + +export function getCliAuthCodeTokenIdentifier(authCodeToken: string): string { + return `${CLI_AUTH_CODE_TOKEN_IDENTIFIER_PREFIX}${authCodeToken}` +} + +export function getConsumedCliAuthCodeTokenIdentifier( + authCodeToken: string, +): string { + return `${CONSUMED_CLI_AUTH_CODE_TOKEN_IDENTIFIER_PREFIX}${getCliAuthCodeHash( + authCodeToken, + )}` +} + +export function getConsumedCliAuthCodeTokenValue(): string { + return CONSUMED_CLI_AUTH_CODE_TOKEN_VALUE +} + +export type CliAuthCodeTokenConsumeResult = + | { status: 'resolved'; authCode: string } + | { status: 'already_consumed' } + | { status: 'missing' } + +export type CliAuthCodeResolution = + | { + status: 'ready' + authCode: string + resolvedOpaqueToken: boolean + } + | { + status: 'already_consumed' + authCode: string + resolvedOpaqueToken: false + } + | { + status: 'missing' + authCode: string + resolvedOpaqueToken: false + } + export async function resolveCliAuthCode( authCode: string, - consumeCliAuthCodeToken: (authCodeToken: string) => Promise, -): Promise<{ authCode: string; resolvedOpaqueToken: boolean }> { + consumeCliAuthCodeToken: ( + authCodeToken: string, + ) => Promise, +): Promise { const normalizedAuthCode = authCode.trim() if (!isOpaqueCliAuthCodeToken(normalizedAuthCode)) { - return { authCode: normalizedAuthCode, resolvedOpaqueToken: false } + return { + status: 'ready', + authCode: normalizedAuthCode, + resolvedOpaqueToken: false, + } } - const signedAuthCode = await consumeCliAuthCodeToken(normalizedAuthCode) - if (!signedAuthCode) { - return { authCode: normalizedAuthCode, resolvedOpaqueToken: false } + const tokenResult = await consumeCliAuthCodeToken(normalizedAuthCode) + if (tokenResult.status === 'resolved') { + return { + status: 'ready', + authCode: tokenResult.authCode, + resolvedOpaqueToken: true, + } + } + + if (tokenResult.status === 'already_consumed') { + return { + status: 'already_consumed', + authCode: normalizedAuthCode, + resolvedOpaqueToken: false, + } } return { - authCode: signedAuthCode, - resolvedOpaqueToken: true, + status: 'missing', + authCode: normalizedAuthCode, + resolvedOpaqueToken: false, } } diff --git a/web/src/app/onboard/page.tsx b/web/src/app/onboard/page.tsx index d751222e0..d89ff7943 100644 --- a/web/src/app/onboard/page.tsx +++ b/web/src/app/onboard/page.tsx @@ -54,10 +54,22 @@ const Onboard = async ({ searchParams }: PageProps) => { ) } - const { authCode: resolvedAuthCode } = await resolveCliAuthCode( + const authCodeResolution = await resolveCliAuthCode( authCode, consumeCliAuthCodeToken, ) + + if (authCodeResolution.status === 'already_consumed') { + return ( + You can close this browser window.

} + /> + ) + } + + const { authCode: resolvedAuthCode } = authCodeResolution const { fingerprintId, expiresAt, receivedHash } = parseAuthCode(resolvedAuthCode) const { valid, expectedHash: fingerprintHash } = validateAuthCode( diff --git a/web/src/llm-api/opencode-zen.ts b/web/src/llm-api/opencode-zen.ts index d5417c4ed..699f5e5f5 100644 --- a/web/src/llm-api/opencode-zen.ts +++ b/web/src/llm-api/opencode-zen.ts @@ -38,14 +38,6 @@ const OPENCODE_ZEN_MODELS: Record< string, { opencodeId: string; pricing: OpenCodeZenPricing } > = { - [openCodeZenModels.opencode_minimax_m2_7]: { - opencodeId: 'minimax-m2.7', - pricing: { - inputCostPerToken: 0.3 / 1_000_000, - cachedInputCostPerToken: 0.06 / 1_000_000, - outputCostPerToken: 1.2 / 1_000_000, - }, - }, [openCodeZenModels.opencode_kimi_k2_6]: { opencodeId: 'kimi-k2.6', pricing: { @@ -56,17 +48,12 @@ const OPENCODE_ZEN_MODELS: Record< }, } -const OPENCODE_ZEN_MODEL_PREFIX = 'opencode/' - -export function isOpenCodeZenModel(model: unknown): model is string { - return typeof model === 'string' && model.startsWith(OPENCODE_ZEN_MODEL_PREFIX) +export function isOpenCodeZenModel(model: string): boolean { + return model in OPENCODE_ZEN_MODELS } function getOpenCodeZenModelId(model: string): string { - return ( - OPENCODE_ZEN_MODELS[model]?.opencodeId ?? - model.slice(OPENCODE_ZEN_MODEL_PREFIX.length) - ) + return OPENCODE_ZEN_MODELS[model]?.opencodeId ?? model } function getOpenCodeZenPricing(model: string): OpenCodeZenPricing {