fix(llm): 允许无密钥 LLM 配置#193
Conversation
Some OpenAI-compatible LLM endpoints do not require an API key, so the LLM validation and runtime polish paths should not invent one or fail early when the key field is blank. The client now omits the Authorization header when the key is empty, the coordinator stops treating a blank LLM key as a hard error, and the settings validation continues to distinguish a missing model from a genuinely failed connection. Constraint: Keep the change minimal and preserve the existing HTTP-status error mapping. Rejected: Keep key-required behavior for all LLM endpoints | it mislabels keyless endpoints such as local 11443-style servers as invalid. Confidence: high Scope-risk: moderate Directive: Empty LLM API keys are allowed only for the OpenAI-compatible path; ASR remains key-required. Tested: npm run build; cargo test --lib Not-tested: Live behavior against a real keyless endpoint outside the test harness
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0fe00ced9a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if api_key_required && api_key.trim().is_empty() { | ||
| return Err("API Key 为空".to_string()); |
There was a problem hiding this comment.
This change allows empty API keys for llm, but the model-listing path still unconditionally sends Authorization: Bearer {api_key} in fetch_provider_models. When the key is empty, some OpenAI-compatible endpoints reject the empty Bearer credential, so the Settings “Fetch models” flow fails even though keyless LLM polish/validation now works. Please make model discovery omit Authorization when the key is blank, matching the new keyless behavior.
Useful? React with 👍 / 👎.
Model discovery now matches the keyless LLM path by omitting Authorization when the stored API key is blank. This keeps Fetch models usable for endpoints that reject empty Bearer credentials while preserving authenticated discovery for configured providers. Constraint: ASR still requires an API key Rejected: send a fake or empty Bearer header | breaks keyless endpoints Confidence: high Scope-risk: narrow Directive: Do not reintroduce auth headers for blank LLM keys without a provider-specific requirement Tested: cargo test --lib fetch_provider_models_omits_authorization_when_api_key_is_empty; rustfmt --edition 2021 --check src/commands.rs Not-tested: full frontend integration flow
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1d0dcef18c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let api_key = CredentialsVault::get(CredentialAccount::ArkApiKey)?.unwrap_or_default(); | ||
| if api_key.is_empty() { | ||
| anyhow::bail!("ark api key missing"); | ||
| } | ||
| let model = CredentialsVault::get(CredentialAccount::ArkModelId)? | ||
| .filter(|s| !s.is_empty()) |
There was a problem hiding this comment.
In polish_text the API-key emptiness check was removed, but this function still falls back to the public Ark URL when no endpoint is configured. That means users with an empty ArkApiKey (including fresh installs with default Light mode) now send raw transcript content to https://ark.cn-beijing.volces.com/... and only fail after the network call, whereas previously the request was blocked locally. This is a privacy and behavior regression introduced by this commit; the same pattern also appears in translate_text and answer_chat_dispatch.
Useful? React with 👍 / 👎.
Polish, translate, and QA chat now fail locally when ArkApiKey is blank, so fresh installs and keyless LLM setups do not leak transcript content to the default public Ark endpoint. Model discovery remains keyless because it is a separate non-content path. Constraint: LLM model discovery must stay keyless Rejected: rely on downstream provider auth failures | leaks user content over the network Confidence: high Scope-risk: narrow Directive: Keep the blank-key guard ahead of any Ark endpoint fallback in content-sending flows Tested: cargo test --lib require_ark_api_key_rejects_blank_key; rustfmt --edition 2021 --check src/coordinator.rs Not-tested: full UI settings flow
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
PR Reviewer Guide 🔍(Review updated until commit b85006c)Here are some key observations to aid the review process:
|
…k leakage Content flows now allow empty Ark API keys only when a custom Ark endpoint is configured. If both key and endpoint are empty, we fail locally instead of falling back to the public Ark endpoint, preserving the privacy guard while restoring ticket Open-Less#192 keyless self-hosted behavior. Constraint: Keep no-key self-hosted OpenAI-compatible LLM support from issue Open-Less#192 Constraint: Prevent transcript content from being sent to default public Ark endpoint when key is empty Rejected: unconditional API-key requirement in coordinator | breaks keyless self-hosted usage Rejected: unconditional default-endpoint fallback | risks accidental data egress on fresh installs Confidence: high Scope-risk: narrow Directive: Any future endpoint fallback change must preserve the blank-key + blank-endpoint local guard for content-sending flows Tested: rustfmt --edition 2021 --check src/coordinator.rs; cargo test --lib resolve_ark_endpoint_ Not-tested: end-to-end UI interaction for keyless custom endpoint polish/translate/chat
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
Persistent review updated to latest commit b85006c |
1 participant
User description
摘要
Fixes #192。
本 PR 修复 LLM 配置时 API Key 被强制要求的问题。
部分本地或自托管 OpenAI-compatible LLM endpoint 不需要 API Key，例如本地 11443 类服务或内网代理服务。此前 LLM 验证和运行时 polish 路径会把空 API Key 直接视为凭据缺失，导致这类 endpoint 即使本身可用，也会在客户端被提前判定为配置错误。
本次改动允许 OpenAI-compatible LLM 在 API Key 为空时继续验证和运行。API Key 为空时不会发送
Authorizationheader；API Key 非空时继续发送Authorization: Bearer ...。ASR 路径仍保持 API Key 必填，避免把 LLM 的无密钥兼容逻辑扩散到语音识别服务。修复 / 新增 / 改进
LLM provider 配置读取时不再强制要求 API Key。
ASR provider 配置读取仍保持 API Key 必填。
LLM polish 非流式请求中：
AuthorizationheaderAuthorization: Bearer ...LLM streaming 请求中：
AuthorizationheaderAuthorization: Bearer ...coordinator 运行时 polish / translate / chat 路径不再因为 LLM API Key 为空提前失败。
设置页 LLM 验证可以覆盖无密钥 OpenAI-compatible endpoint。
保持现有 HTTP status 错误映射不变。
新增测试覆盖：
Authorization: Bearer兼容
不包含：
对现有用户 / 本地环境 / 构建流程的影响：
测试计划
命令：
npm run build结果：通过
证据路径：本地构建输出
命令：
cargo test --lib结果：通过
证据路径：本地测试输出
真实无密钥 OpenAI-compatible endpoint 联调
说明：未在测试 harness 之外使用真实 keyless endpoint 验证 live 行为。
主要改动文件
openless-all/app/src-tauri/src/commands.rsopenless-all/app/src-tauri/src/coordinator.rsopenless-all/app/src-tauri/src/polish.rs备注
本 PR 只允许 OpenAI-compatible LLM 路径接受空 API Key。空 Key 时不会伪造
Authorizationheader；非空 Key 时仍沿用现有 Bearer 鉴权行为。ASR 仍保持密钥必填。PR Type
Bug fix, Tests
Description
Allow empty API key for LLM providers in config, HTTP requests, and coordinator flows
Coordinator Ark content paths now require either API key or custom endpoint to prevent leaks
Retain mandatory API key for ASR provider
Add unit tests for keyless LLM headers, endpoint policy, and successful completion
Diagram Walkthrough
flowchart LR A["read_openai_provider_config"] -->|"llm: key optional"| B["LLM provider"] A -->|"asr: key required"| C["ASR provider unchanged"] B --> D["HTTP requests: omit Authorization if key empty"] B --> E["coordinator: resolve_ark_endpoint blocks empty key without custom endpoint"] D --> F["Tests check header omission"] E --> G["Tests check endpoint policy"]File Walkthrough
commands.rs
Make LLM API key optional in provider config and model fetchopenless-all/app/src-tauri/src/commands.rs
provider config
non-empty
polish.rs
Remove mandatory key check, conditionally send Authorization headeropenless-all/app/src-tauri/src/polish.rs
answer_chat_streaming
key empty
coordinator.rs
Guard Ark content flows, allow keyless with custom endpointopenless-all/app/src-tauri/src/coordinator.rs
paths
endpoint is provided
content leaks
endpoint allowed)