feat(bedrock): 切换至 Bearer Token 认证，移除 AK/SK 与 STS 支持 #8671
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Co-authored-by: amazon-q-developer[bot] <208079219+amazon-q-developer[bot]@users.noreply.github.com>
Co-authored-by: amazon-q-developer[bot] <208079219+amazon-q-developer[bot]@users.noreply.github.com>
Co-authored-by: amazon-q-developer[bot] <208079219+amazon-q-developer[bot]@users.noreply.github.com>
Co-authored-by: amazon-q-developer[bot] <208079219+amazon-q-developer[bot]@users.noreply.github.com>
Co-authored-by: amazon-q-developer[bot] <208079219+amazon-q-developer[bot]@users.noreply.github.com>
support AWS BEARER_TOKEN
Simplifies Bedrock authentication by streamlining the payload to primarily use bearer token and region, removing deprecated and less relevant fields. Introduces AWS_BEDROCK_MODEL_LIST configuration to allow explicit control over available Bedrock models. Adds NoAvailableModels and InvalidBedrockRegion error types for more precise error reporting, improving the clarity of connection checks. Refactors i18n calls in the Checker component for better consistency and removes unused state. Performs minor code cleanups, including consistent use of slice for string manipulation.
|
@cnkang is attempting to deploy a commit to the LobeHub Community Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
Reviewer's GuideThis PR overhauls the Bedrock integration to use Bearer Token authentication exclusively, removing legacy AK/SK/STS support, and refactors both runtime and UI layers—including direct HTTP streaming with a new JSON parser, dynamic model listing, precise error types, updated configuration schemas, documentation, and corresponding tests. Sequence diagram for Bedrock authentication flow (Bearer Token only)sequenceDiagram
actor User
participant UI as UI/Settings
participant Server as Server/ModelRuntime
participant Bedrock as AWS Bedrock API
User->>UI: Enter Bearer Token and Region
UI->>Server: Send Bearer Token and Region
Server->>Bedrock: HTTP request with Authorization: Bearer <token>
Bedrock-->>Server: Model response
Server-->>UI: Return result
UI-->>User: Display result
ER diagram for Bedrock provider config schema changeserDiagram
AWSBedrockKeyVault {
string bearerToken
string region
}
LLMConfig {
string AWS_BEARER_TOKEN_BEDROCK
string AWS_REGION
string AWS_BEDROCK_MODEL_LIST
}
AWSBedrockKeyVault ||--o{ LLMConfig: "uses"
Class diagram for AWSBedrockKeyVault type changeclassDiagram
class AWSBedrockKeyVault {
+ bearerToken?: string
+ region?: string
}
Class diagram for new StreamingJsonParser utilityclassDiagram
class StreamingJsonParser {
- buffer: string
+ processChunk(chunk: string): any[]
+ reset(): void
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Member
|
👍 @cnkang Thank you for raising your pull request and contributing to our Community |
Contributor
|
There is too much information in the pull request to test. |
dosubot
bot
added
the
size:XXL
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey @cnkang - I've reviewed your changes and found some issues that need to be addressed.
Blocking issues:
- The OpenAI-compatible stream implementation may enqueue objects instead of Uint8Array chunks. (link)
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/libs/model-runtime/bedrock/index.ts:139` </location>
<code_context>
- contentType: 'application/json',
- modelId: model,
- });
+ // Use the Converse API format for bearer token requests
+ const converseMessages = user_messages
+ .filter(msg => msg.content && (msg.content as string).trim())
+ .map(msg => ({
</code_context>
<issue_to_address>
The code assumes all user messages are non-empty strings, but does not handle the case where all messages are filtered out.
If `converseMessages` is empty, the Bedrock API may fail. Add a check to ensure at least one valid user message exists before the API call, and throw a clear error if not.
</issue_to_address>
### Comment 2
<location> `src/libs/model-runtime/bedrock/index.ts:193` </location>
<code_context>
+ const openaiStream = new ReadableStream({
</code_context>
<issue_to_address>
The OpenAI-compatible stream implementation may enqueue objects instead of Uint8Array chunks.
controller.enqueue should receive a Uint8Array or string, not a JavaScript object. Serialize the chunk (e.g., with JSON.stringify) and encode it before enqueuing to avoid runtime issues.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Consolidates Bedrock provider tests by unifying how bearer tokens are handled. The test now utilizes the field within for Bedrock bearer tokens, removing the need for separate AWS access key and secret fields. This change also eliminates a redundant test case for Bedrock's bearer tokens, simplifying the test suite and standardizing the payload structure.
Updates Bedrock provider tests to use a bearer token instead of access key ID and secret access key for authentication. This aligns the test with the updated Bedrock integration.
Adds `defaultSettings: {}` to initial state mocks across all provider tests to ensure consistent test setup.
Ensures that API requests to Bedrock include at least one valid message, preventing calls with empty message lists. Formats streaming responses into the correct Server-Sent Events (SSE) format, including the `data:` prefix and double newlines, for proper client consumption.
Refactors how stream chunks are enqueued, removing redundant stringification and encoding for direct object handling. This simplifies stream processing and optimizes data flow. Adds comprehensive error handling to the streaming pipeline, ensuring that any issues during chunk processing are caught and propagated correctly. This enhances the stability and reliability of the model runtime.
Author
|
@sourcery-ai review |
Contributor
There was a problem hiding this comment.
Hey @cnkang - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/libs/model-runtime/bedrock/index.ts:171` </location>
<code_context>
- const [prod, debug] = claudeStream.tee();
+ // Add temperature and top_p if specified
+ if (temperature !== undefined) {
+ requestBody.inferenceConfig.temperature = temperature / 2;
+ }
</code_context>
<issue_to_address>
Dividing temperature by 2 may not be appropriate for all models.
If dividing by 2 is required for a specific model, please document the reason or apply this adjustment only for that model.
</issue_to_address>
### Comment 2
<location> `src/libs/model-runtime/bedrock/index.ts:153` </location>
<code_context>
- try {
- // Ask Claude for a streaming chat completion given the prompt
- const res = await this.client.send(command, { abortSignal: options?.signal });
+ // Ensure at least one valid message exists
+ if (converseMessages.length === 0) {
+ throw new Error('No valid user messages found. At least one non-empty message is required.');
</code_context>
<issue_to_address>
The check for at least one valid user message is strict.
If system-only prompts are valid for some models, consider making this check model-specific.
</issue_to_address>
### Comment 3
<location> `src/libs/model-runtime/bedrock/index.ts:309` </location>
<code_context>
- try {
- // Ask Claude for a streaming chat completion given the prompt
- const res = await this.client.send(command);
+ private async invokeEmbeddingModelWithBearerToken(
+ payload: EmbeddingsPayload,
+ options?: EmbeddingsOptions,
</code_context>
<issue_to_address>
No normalization or validation of embedding response shape.
Validate the response structure to handle potential API changes or error formats and prevent runtime errors.
</issue_to_address>
### Comment 4
<location> `src/libs/model-runtime/bedrock/index.ts:107` </location>
<code_context>
+ async models() {
+ // Get models from environment config
+ const { getLLMConfig } = await import('@/config/llm');
+ const config = getLLMConfig();
+ const modelList = config.AWS_BEDROCK_MODEL_LIST;
+
</code_context>
<issue_to_address>
Model list parsing does not handle exclusions or '-all' logic.
The current logic only includes models with '+' or no prefix, and does not process '-' or 'all'. Please update the logic to handle exclusions and the 'all' keyword as documented.
Suggested implementation:
```typescript
async models() {
// Get models from environment config
const { getLLMConfig } = await import('@/config/llm');
const config = getLLMConfig();
const modelList = config.AWS_BEDROCK_MODEL_LIST;
// Assume this is the full list of available models (should be imported or defined elsewhere)
const ALL_AVAILABLE_MODELS = [
// e.g. 'model-a', 'model-b', 'model-c'
// TODO: Replace with actual available models
];
// Parse modelList for inclusions, exclusions, and 'all'
const inclusions: string[] = [];
const exclusions: string[] = [];
let includeAll = false;
for (const entry of modelList) {
const trimmed = entry.trim();
if (trimmed === 'all') {
includeAll = true;
} else if (trimmed.startsWith('-')) {
exclusions.push(trimmed.slice(1));
} else if (trimmed.startsWith('+')) {
inclusions.push(trimmed.slice(1));
} else {
inclusions.push(trimmed);
}
}
let finalModels: string[];
if (includeAll) {
finalModels = ALL_AVAILABLE_MODELS.filter(m => !exclusions.includes(m));
} else {
finalModels = inclusions.filter(m => !exclusions.includes(m));
}
// Return the final list of models (replace with actual return logic as needed)
return finalModels;
```
- You must define or import the actual list of available models as `ALL_AVAILABLE_MODELS` for the 'all' keyword to work.
- Adjust the return value to match the expected format for your application (e.g., objects instead of strings).
- If `AWS_BEDROCK_MODEL_LIST` is not always an array, ensure it is parsed as such.
</issue_to_address>
### Comment 5
<location> `src/services/__tests__/chat.test.ts:2146` </location>
<code_context>
expect(runtime['_runtime']).toBeInstanceOf(LobeMoonshotAI);
});
- it('Bedrock provider: with accessKeyId, region, secretAccessKey', async () => {
+ it('Bedrock provider: with bearer token and region', async () => {
merge(initialSettingsState, {
+ defaultSettings: {},
</code_context>
<issue_to_address>
Test for Bedrock provider with bearer token and region is present.
Please add a test for when the bearer token is missing to verify error handling.
Suggested implementation:
```typescript
it('Bedrock provider: with bearer token and region', async () => {
merge(initialSettingsState, {
defaultSettings: {},
});
it('Bedrock provider: missing bearer token should throw error', async () => {
merge(initialSettingsState, {
defaultSettings: {},
settings: {
keyVaults: {
bedrock: {
// bearerToken is intentionally omitted
region: 'us-west-2',
},
},
},
});
let error;
try {
// Replace with the actual function or initialization that uses the Bedrock provider
await initializeBedrockProvider(); // This should be the function under test
} catch (e) {
error = e;
}
expect(error).toBeDefined();
// Optionally, check for a specific error message:
// expect(error.message).toMatch(/bearer token/i);
```
- Replace `initializeBedrockProvider()` with the actual function or method that initializes or uses the Bedrock provider in your codebase.
- Adjust the error message check as needed to match your implementation's error handling.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Refactors Bedrock model list parsing to correctly apply `all`, inclusion (`+`), and exclusion (`-`) rules, ensuring accurate model availability. Adjusts chat message validation to permit system-only prompts, supporting models that do not require an initial user message. Implements specific temperature scaling for Bedrock Claude models, aligning the 0-2 range with their expected 0-1 range. Adds validation for embedding API responses to ensure data integrity and prevent errors from malformed replies. Includes a test to verify that the Bedrock provider correctly throws an error when a bearer token is missing.
Database and Performance Optimizations: - Optimize database initialization with unique DB names per test process - Move initializeDB() from beforeEach to beforeAll to prevent concurrent initialization - Add proper transaction-based data cleanup in beforeEach for all test files - Force MemoryFS usage in test environment for better isolation and performance - Skip localStorage cache checks in test environment for faster execution Upstream Compatibility Fixes: - Restore IdbFs usage in db.ts with proper test environment handling - Fix userId values to match upstream patterns (file-user, session-user, etc.) - Add trx.delete(users) before insert to prevent constraint violations - Use transaction-based data setup in beforeEach as per upstream - Remove unnecessary afterEach cleanup to match upstream patterns Test Environment Configuration: - Revert to upstream vitest configuration (remove custom poolOptions, maxConcurrency) - Restore original canvas mock conditions matching upstream/main - Remove location.search mock from setup.ts (upstream uses per-test beforeEach) - Maintain compatibility with upstream while preserving performance improvements Results: - Fixes all test failures and compatibility issues with upstream/main - Maintains performance optimizations while ensuring stability - 100% test pass rate with proper upstream alignment
added 3 commits
August 22, 2025 09:26
…ow conflicts - Merge latest upstream changes including package refactoring - Resolve test workflow conflicts by adopting upstream structure - Maintain existing test optimizations and bedrock bearer token support - Keep database performance improvements and test isolation - Integrate chain refactoring into @lobechat/prompts package
- Fix file ordering test failure by adding timestamp delay for proper createdAt ordering - Restore upstream fs selection logic for database initialization consistency - Improve client database test reliability with optimized initialization - Resolve database initialization timeouts in CI environment - Optimize test performance with MemoryFS usage and proper isolation - Maintain upstream compatibility while preserving performance improvements - Ensure 100% test pass rate with proper database transaction handling This consolidates all test infrastructure improvements into a single coherent fix.
cnkang
force-pushed
the
dev
branch
2 times, most recently
from
8057f5d to
b37d0ee
Compare
- Fix eval types export path in packages/types/src/index.ts - Add missing EvalEvaluationStatus import in ragEval.ts - Implement proper test database strategy using getTestDB utility - Use TEST_SERVER_DB environment variable to switch between client/server DB - Restore test files to upstream structure (beforeEach instead of beforeAll) - Update table count expectation from 59 to 63 for new ragEvals tables - Resolve all TypeScript, lint, and test failures ✅ All tests passing after upstream merge
- Add comprehensive mocks for @electric-sql/pglite, fetch, and WebAssembly - Fix vi.mock compatibility issues with vitest - Ensure all 11 client DB tests pass consistently - Mock external dependencies to avoid network requests and PGlite initialization - Use proper timing assertions (>= instead of >) for edge cases - Maintain upstream test structure while ensuring local compatibility ✅ Resolves all client database test failures and unhandled rejections
* move utils * move utils * move utils * update * update * update * update * update * refactor to clean the tests * fix release workflow * fix tests * fix tests * fix tests * fix tests * fix tests * fix tests * try to fix client db migration issue * fix tests
### [Version 1.114.4](lobehub/lobe-chat@v1.114.3...v1.114.4) <sup>Released on **2025-08-22**</sup> #### ♻ Code Refactoring - **misc**: Move database to packages. <br/> <details> <summary><kbd>Improvements and Fixes</kbd></summary> #### Code refactoring * **misc**: Move database to packages, closes [lobehub#8874](lobehub#8874) ([af1f715](lobehub@af1f715)) </details> <div align="right"> [](#readme-top) </div>
- Updated Mistral model vision abilities - Preserved AWS Bedrock bearertoken support - Version bump to 1.114.5 - Updated changelog and package.json
<sup>Released on **2025-08-22**</sup> - **misc**: Move database to packages. <br/> <details> <summary><kbd>Improvements and Fixes</kbd></summary> * **misc**: Move database to packages, closes [lobehub#8874](lobehub#8874) ([af1f715](lobehub@af1f715)) </details> <div align="right"> [](#readme-top) </div>
# Conflicts: # CHANGELOG.md
- Filter out PostGIS system tables (geography_columns, geometry_columns, spatial_ref_sys) - Filter out other common system table prefixes (pg_%, sql_%, information_schema%) - Update test expectation to maintain 60 application tables - Makes tests more robust against database extensions adding system tables
Contributor
There was a problem hiding this comment.
Sorry @cnkang, your pull request is too large to review
Author
|
已经更新到最新的代码, please review |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #8671 +/- ##
==========================================
+ Coverage 83.90% 83.96% +0.05%
==========================================
Files 863 867 +4
Lines 70108 71426 +1318
Branches 6439 6523 +84
==========================================
+ Hits 58827 59974 +1147
- Misses 11239 11410 +171
Partials 42 42
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Member
|
@cnkang 感谢贡献！ 需要 rebase 下 main， 目前冲突有点多 |
Member
@cnkang Thanks for the contribution! Need to rebase main, there are a lot of conflicts at present |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🔀 变更说明 | Description of Change
AWS_ACCESS_KEY_ID与AWS_SECRET_ACCESS_KEY不再生效，所有 AK/SK 和 STS 方式的认证方法已被删除。NoAvailableModels和InvalidBedrockRegion错误类型，以便更精确地反馈连接检查失败的原因。Checker组件中重构 i18n 调用以保持一致性，移除未使用的 state，并进行若干小规模代码清理（如统一使用slice进行字符串操作）。📝 补充信息 | Additional Information
AWS_ACCESS_KEY_ID与AWS_SECRET_ACCESS_KEY的依赖。Summary by Sourcery
Switch Bedrock provider to Bearer Token authentication, remove legacy AWS credentials and STS, and refactor runtime, config, UI, docs, and tests to support the new auth workflow
New Features:
Enhancements:
Build:
Documentation:
Tests:
Chores: