fix: 代码审查修复 — 安全、性能和正确性

- triggersApi: 添加 assertSubscriptionBaseUrl 防止 OAuth token 泄露
- claude.ts: 修复流式响应 O(n^2) 字符串拼接,改用数组累积
- claude.ts: 移除未使用的 import,动态 import 改为静态 import
- StatusLine: BuiltinStatusLine 仅在 statusLineEnabled 时显示,修复双行问题
- local-vault: 修复 --reveal 标志位置解析 bug
- share: 修复 sk-proj-* OpenAI 密钥未脱敏问题
- store.ts: 临时文件改用同目录创建,避免跨文件系统 rename 失败
- store.ts: 添加空字符串 key 校验
- permissionValidation: 端口正则限制为有效 TCP 范围 0-65535
- 测试 mock 补全: schedule/vault/skill-store 测试文件
- 移除过期的 biome-ignore 注释

Co-Authored-By: glm-5-turbo <zai-org@claude-code-best.win>
This commit is contained in:
claude-code-best
2026-05-10 09:39:34 +08:00
parent 4f493c83fc
commit 82be5ff05b
12 changed files with 78 additions and 32 deletions

View File

@@ -52,6 +52,14 @@ describe('parseLocalVaultArgs', () => {
})
})
test('get with --reveal before key → reveal=true, key correctly resolved', () => {
expect(parseLocalVaultArgs('get --reveal MY_KEY')).toEqual({
action: 'get',
key: 'MY_KEY',
reveal: true,
})
})
test('get without key → invalid', () => {
const result = parseLocalVaultArgs('get')
expect(result.action).toBe('invalid')

View File

@@ -89,7 +89,11 @@ export function parseLocalVaultArgs(args: string): LocalVaultArgs {
// ── get ───────────────────────────────────────────────────────────────────
if (subCmd === 'get') {
const key = tokens[1]
// Strip flags before extracting the key so that `get --reveal MY_KEY`
// correctly resolves MY_KEY as the key rather than --reveal.
const flags = ['--reveal']
const argsWithoutFlags = tokens.filter(t => !flags.includes(t))
const key = argsWithoutFlags[1] // argsWithoutFlags[0] is 'get'
if (!key) {
return { action: 'invalid', reason: `get requires a key name. ${USAGE}` }
}

View File

@@ -43,6 +43,18 @@ mock.module('src/utils/teleport/api.js', () => ({
Authorization: `Bearer ${token}`,
'anthropic-version': '2023-06-01',
}),
prepareApiRequest: async () => ({
accessToken: mockAccessToken,
orgUUID: mockOrgUUID,
}),
prepareWorkspaceApiRequest: async () => ({
apiKey: 'test-workspace-key',
}),
}))
mock.module('src/services/auth/hostGuard.ts', () => ({
assertSubscriptionBaseUrl: () => {},
assertWorkspaceHost: () => {},
assertNoAnthropicEnvForOpenAI: () => {},
}))
// ── Axios mock ──────────────────────────────────────────────────────────────

View File

@@ -14,6 +14,7 @@
import axios from 'axios'
import { getOauthConfig } from '../../constants/oauth.js'
import { assertSubscriptionBaseUrl } from '../../services/auth/hostGuard.js'
import { getOAuthHeaders, prepareApiRequest } from '../../utils/teleport/api.js'
export type Trigger = {
@@ -85,6 +86,8 @@ async function buildHeaders(): Promise<Record<string, string>> {
401,
)
}
// Guard the host before sending OAuth credentials to prevent token leakage.
assertSubscriptionBaseUrl(triggersBaseUrl())
return {
...getOAuthHeaders(accessToken),
'anthropic-beta': TRIGGERS_BETA_HEADER,

View File

@@ -57,7 +57,7 @@ const SECRET_PATTERNS: Array<{ pattern: RegExp; replacement: string }> = [
replacement: '[REDACTED_ANTHROPIC_KEY]',
},
{
pattern: /\b(sk-[A-Za-z0-9]{20,})/g,
pattern: /\b(sk-[A-Za-z0-9_-]{20,})/g,
replacement: '[REDACTED_API_KEY]',
},
// Bearer / Authorization tokens

View File

@@ -52,6 +52,9 @@ const realTeleportApi = await import('src/utils/teleport/api.js')
mock.module('src/utils/teleport/api.js', () => ({
...realTeleportApi,
getOAuthHeaders: (token: string) => ({ Authorization: `Bearer ${token}` }),
prepareWorkspaceApiRequest: async () => ({
apiKey: 'test-workspace-key',
}),
}))
// ── envUtils config dir injection ────────────────────────────────────────────

View File

@@ -38,6 +38,9 @@ mock.module('src/utils/teleport/api.js', () => ({
getOAuthHeaders: (token: string) => ({
Authorization: `Bearer ${token}`,
}),
prepareWorkspaceApiRequest: async () => ({
apiKey: 'test-workspace-key',
}),
}))
// ── Axios mock ──────────────────────────────────────────────────────────────

View File

@@ -160,10 +160,11 @@ export function statusLineShouldDisplay(settings: ReadonlySettings): boolean {
// Assistant mode: statusline fields (model, permission mode, cwd) reflect the
// REPL/daemon process, not what the agent child is actually running. Hide it.
if (feature('KAIROS') && getKairosActive()) return false;
// Render only when the user has explicitly toggled it on via `/statusline`.
// Default off keeps the REPL clean for users who don't want the extra row;
// /statusline flips `statusLineEnabled` in settings.json.
return settings?.statusLineEnabled === true;
// Show the status line when explicitly enabled, or when a statusLine command
// is configured (backward compatibility for users who set statusLine.command
// without toggling statusLineEnabled). Only hide when explicitly disabled.
if (settings?.statusLineEnabled === false) return false;
return settings?.statusLineEnabled === true || !!settings?.statusLine?.command;
}
function buildStatusLineCommandInput(
@@ -499,30 +500,34 @@ function StatusLineInner({ messagesRef, lastAssistantMessageId, vimMode }: Props
}),
};
// StatusLine has stable height — flexShrink:0 footer means row count changes
// would steal from ScrollBox. We always render 2 rows (top: BuiltinStatusLine
// + Cache pill, bottom: shell command stdout reservation) to keep height
// stable across loading/configured/empty states.
// BuiltinStatusLine + CachePill: only when statusLineEnabled is explicitly true.
// Shell command output: only when a statusLine.command is configured.
// These are independent — a user can have one, both, or neither.
const showBuiltin = settings?.statusLineEnabled === true;
const hasShellCommand = !!settings?.statusLine?.command;
return (
<Box flexDirection="column" paddingX={paddingX}>
{/* Top: built-in fork status (model | ctx | 5h | 7d | cost) + Cache pill */}
<Box gap={2}>
<BuiltinStatusLine
modelName={renderModelName(builtinRuntimeModel)}
contextUsedPct={builtinContextPct}
usedTokens={builtinUsedTokens}
contextWindowSize={builtinContextWindowSize}
totalCostUsd={getTotalCost()}
rateLimits={builtinRateLimits}
/>
<CachePill messages={messagesRef.current} />
</Box>
{showBuiltin && (
<Box gap={2}>
<BuiltinStatusLine
modelName={renderModelName(builtinRuntimeModel)}
contextUsedPct={builtinContextPct}
usedTokens={builtinUsedTokens}
contextWindowSize={builtinContextWindowSize}
totalCostUsd={getTotalCost()}
rateLimits={builtinRateLimits}
/>
<CachePill messages={messagesRef.current} />
</Box>
)}
{/* Bottom: user-configured /statusline shell stdout (reserves row in fullscreen) */}
{statusLineText ? (
<Text dimColor wrap="truncate">
<Ansi>{statusLineText}</Ansi>
</Text>
) : isFullscreenEnvEnabled() ? (
) : hasShellCommand && isFullscreenEnvEnabled() ? (
<Text> </Text>
) : null}
</Box>

View File

@@ -20,6 +20,7 @@ import type {
import type { TextBlockParam } from '@anthropic-ai/sdk/resources/index.mjs'
import type { Stream } from '@anthropic-ai/sdk/streaming.mjs'
import { randomUUID } from 'crypto'
import { existsSync, unlinkSync } from 'node:fs'
import {
getAPIProvider,
isFirstPartyAnthropicBaseUrl,
@@ -124,7 +125,6 @@ import {
getAfkModeHeaderLatched,
getCacheEditingHeaderLatched,
getFastModeHeaderLatched,
getLastApiCompletionTimestamp,
getPromptCache1hAllowlist,
getPromptCache1hEligible,
getSessionId,
@@ -254,7 +254,6 @@ import {
type NonNullableUsage,
} from './logging.js'
import {
CACHE_TTL_1HOUR_MS,
checkResponseForCacheBreak,
recordPromptState,
} from './promptCacheBreakDetection.js'
@@ -1431,8 +1430,6 @@ async function* queryModel(
// unique ephemeral nonce comment to the system prompt so the prefix-cache
// hash changes for this request, forcing a cache miss.
{
const { existsSync, unlinkSync } = await import('node:fs')
const { randomUUID } = await import('node:crypto')
const onceMarker = getBreakCacheMarkerPath()
const alwaysFlag = getBreakCacheAlwaysPath()
const shouldBreak = existsSync(onceMarker) || existsSync(alwaysFlag)
@@ -1842,6 +1839,7 @@ async function* queryModel(
let ttftMs = 0
let partialMessage: BetaMessage | undefined
const contentBlocks: (BetaContentBlock | ConnectorTextBlock)[] = []
const textDeltas = new Map<number, string[]>()
let usage: NonNullableUsage = EMPTY_USAGE
let costUSD = 0
let stopReason: BetaStopReason | null = null
@@ -1940,6 +1938,7 @@ async function* queryModel(
ttftMs = 0
partialMessage = undefined
contentBlocks.length = 0
textDeltas.clear()
usage = EMPTY_USAGE
stopReason = null
isAdvisorInProgress = false
@@ -2096,6 +2095,7 @@ async function* queryModel(
}
break
case 'text':
textDeltas.set(part.index, [])
contentBlocks[part.index] = {
...part.content_block,
// awkwardly, the sdk sometimes returns text as part of a
@@ -2202,7 +2202,7 @@ async function* queryModel(
})
throw new Error('Content block is not a text block')
}
;(contentBlock as { text: string }).text += delta.text
textDeltas.get(part.index)?.push(delta.text!)
break
case 'signature_delta':
if (
@@ -2270,6 +2270,12 @@ async function* queryModel(
})
throw new Error('Message not found')
}
// Merge accumulated text deltas into the content block (O(n) join instead of O(n^2) +=)
const deltas = textDeltas.get(part.index)
if (deltas) {
;(contentBlock as { text: string }).text = deltas.join('')
textDeltas.delete(part.index)
}
const m: AssistantMessage = {
message: {
...partialMessage,

View File

@@ -36,7 +36,7 @@ import {
rmSync,
} from 'node:fs'
import { readFile, writeFile } from 'node:fs/promises'
import { homedir, tmpdir } from 'node:os'
import { homedir } from 'node:os'
import { join } from 'node:path'
import { logError } from '../../utils/log.js'
import { KeychainUnavailableError, tryKeychain } from './keychain.js'
@@ -304,8 +304,9 @@ async function writeVaultFile(data: VaultFile): Promise<void> {
}
const filePath = getVaultFilePath()
// C1: atomic write — tmp file + rename (POSIX rename(2) is atomic)
const vaultDir = join(filePath, '..')
const tmpPath = join(
tmpdir(),
vaultDir,
`.local-vault-${randomBytes(8).toString('hex')}.tmp`,
)
try {
@@ -340,6 +341,8 @@ async function getOrCreateSalt(vaultData: VaultFile): Promise<Buffer> {
// ── Public API ────────────────────────────────────────────────────────────────
export async function setSecret(key: string, value: string): Promise<void> {
if (!key) throw new Error('key must not be empty')
// D1: Guard against unbounded value sizes
const byteLength = Buffer.byteLength(value, 'utf8')
if (byteLength > MAX_SECRET_BYTES) {

View File

@@ -315,7 +315,7 @@ export function validatePermissionRule(
parsed.toolName === 'VaultHttpFetch' &&
behavior === 'deny' &&
parsed.ruleContent !== undefined &&
!/^[A-Za-z0-9._-]{1,128}@(?:\*|(?:\[[A-Fa-f0-9:]+\]|[A-Za-z0-9.-]{1,253})(?::\d{1,5})?)$/.test(
!/^[A-Za-z0-9._-]{1,128}@(?:\*|(?:\[[A-Fa-f0-9:]+\]|[A-Za-z0-9.-]{1,253})(?::(?:[1-9]\d{0,3}|[1-5]\d{4}|6[0-4]\d{3}|65[0-4]\d{2}|655[0-2]\d|6553[0-5]))?)$/.test(
parsed.ruleContent,
)
) {
@@ -367,7 +367,7 @@ export function validatePermissionRule(
if (
parsed.toolName === 'VaultHttpFetch' &&
parsed.ruleContent !== undefined &&
!/^[A-Za-z0-9._-]{1,128}@(?:\*|(?:\[[A-Fa-f0-9:]+\]|[A-Za-z0-9.-]{1,253})(?::\d{1,5})?)$/.test(
!/^[A-Za-z0-9._-]{1,128}@(?:\*|(?:\[[A-Fa-f0-9:]+\]|[A-Za-z0-9.-]{1,253})(?::(?:[1-9]\d{0,3}|[1-5]\d{4}|6[0-4]\d{3}|65[0-4]\d{2}|655[0-2]\d|6553[0-5]))?)$/.test(
parsed.ruleContent,
)
) {

View File

@@ -40,7 +40,6 @@ import { mock } from 'bun:test'
// triggers TS2322 (parameter type contravariance). The biome rule that
// disallows `any` here is already disabled project-wide, so plain `any` is
// the correct escape hatch for an internal test-only union.
// biome-ignore lint/suspicious/noExplicitAny: see comment above
type AnyFn = (...args: any[]) => unknown
export type AxiosMethodStubs = {