Open
Conversation
Change-Id: I22df4c38bc6150c54ecf2f75d49108073e964623
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Change-Id: If25ed4556b47e92d748521a80f126ce4a1a8a2b3
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Ⅰ. Describe what this PR did
本 PR 修复了 Bedrock Provider 在
c12183ca之后引入的 SigV4 canonical URI 编码回归问题。背景
c12183ca主要修复了 Bedrock 在 AK/SK 模式下“只对部分 API 签名”的问题,这部分改动是正确且必要的。但同一提交中,
encodeSigV4Path从“按段直接PathEscape”改为“PathUnescape后再按单编码重建”,导致已编码路径段(例如%3A、%2F)在 canonical URI 中的表达变化,进而在部分场景触发 AWS 签名校验失败。已复现的典型模型名:
global.amazon.nova-2-lite-v1:0arn:aws:bedrock:us-east-1:123456789012:inference-profile/global.anthropic.claude-sonnet-4-20250514-v1:0主要变更
仅回退 canonical URI 编码策略(
provider/bedrock.go):?后内容不参与 canonical URI)PathUnescape + 自定义重编码的“单编码归一化”逻辑url.PathEscape(seg)(历史可用行为)删除不再需要的辅助函数(
provider/bedrock.go):sigV4EscapePathSegmentisSigV4Unreserved新增并强化 SigV4 单元测试(
provider/bedrock_sigv4_path_test.go):%等边界场景保留
c12183ca的有效修复能力:修复前后对比
%3A)canonical URI%3A%253A口径%2F)canonical URI%2F%252F口径global.amazon.nova-2-lite-v1:0相关签名回归Ⅱ. Does this pull request fix one issue?
修复了一个 SigV4 回归问题:
c12183ca中 canonical URI 编码策略调整导致部分 Bedrock model path 与 AWS 服务端计算口径不一致,出现The request signature we calculated does not match the signature you provided。Ⅲ. Why don't you add test cases (unit test/integration test)?
已新增并执行单元测试(
provider/bedrock_sigv4_path_test.go):TestEncodeSigV4Path:)%3A)/)%转义路径TestOverwriteRequestPathHeaderPreservesSingleEncodedRequestPathoverwriteRequestPathHeader在 plain model 和 pre-encoded model 场景都不会把转发路径双重编码TestGenerateSignatureIgnoresQueryStringInCanonicalURITestGenerateSignatureDiffersForRawAndPreEncodedModelPath同时执行了回归:
go test -gcflags="all=-N -l" ./providergo test -gcflags="all=-N -l" -run TestBedrock ./...Ⅳ. Describe how to verify it
方式一:运行单元测试
方式二:手动验证
model分别使用:global.amazon.nova-2-lite-v1:0arn:aws:bedrock:us-east-1:123456789012:inference-profile/global.anthropic.claude-sonnet-4-20250514-v1:0Authorization: AWS4-HMAC-SHA256 ...X-Amz-Date: ...%3A/%253A、%2F/%252F口径场景)。Ⅴ. Special notes for reviews
c12183ca的签名覆盖修复。123456789012。Ⅵ. AI Coding Tool Usage Checklist (if applicable)
Please check all applicable items:
AI Coding Summary
问题根因:
c12183ca将encodeSigV4Path改为“单编码归一化”(先解码再重编码)%3A、%2F)产生 canonical URI 口径变化修复方案:
url.PathEscape(seg)影响范围:
provider/bedrock.go:encodeSigV4Path逻辑回退并清理相关辅助函数provider/bedrock_sigv4_path_test.go:新增/强化 SigV4 canonical URI 与签名测试Ⅰ. Describe what this PR did
This PR fixes a SigV4 canonical URI encoding regression introduced by Bedrock Provider after
c12183ca.Background
c12183camainly fixes the problem of "only signing some APIs" in Bedrock in AK/SK mode. This part of the change is correct and necessary.However, in the same submission,
encodeSigV4Pathwas changed from "directlyPathEscapeby segment" to "PathUnescapeand then reconstructed by single encoding", resulting in changes in the expression of encoded path segments (such as%3A,%2F) in the canonical URI, which in turn triggered AWS signature verification failure in some scenarios.Typical model names that have been reproduced:
global.amazon.nova-2-lite-v1:0arn:aws:bedrock:us-east-1:123456789012:inference-profile/global.anthropic.claude-sonnet-4-20250514-v1:0Major changes
Fallback only canonical URI encoding strategy (
provider/bedrock.go):?does not participate in canonical URI)PathUnescape + Custom Recodingurl.PathEscape(seg)(historically available behavior)Remove helper functions that are no longer needed (
provider/bedrock.go):sigV4EscapePathSegmentisSigV4UnreservedAdded and enhanced SigV4 unit test (
provider/bedrock_sigv4_path_test.go):%, etc.Retain the effective repair capability of
c12183ca:Comparison before and after repair
%3A) canonical URI%3A%253Acaliber%2F) canonical URI%2F%252Fcaliberglobal.amazon.nova-2-lite-v1:0related signature regressionⅡ. Does this pull request fix one issue?
Fixed a SigV4 regression:
The adjustment of the canonical URI encoding strategy in
c12183cacaused some Bedrock model paths to be inconsistent with the AWS server calculation caliber, andThe request signature we calculated does not match the signature you providedappeared.Ⅲ. Why don't you add test cases (unit test/integration test)?
Unit test has been added and executed (
provider/bedrock_sigv4_path_test.go):TestEncodeSigV4Path:)%3A)/)%escape pathTestOverwriteRequestPathHeaderPreservesSingleEncodedRequestPathoverwriteRequestPathHeaderwill not double-encode the forwarding path in plain model and pre-encoded model scenarios.TestGenerateSignatureIgnoresQueryStringInCanonicalURITestGenerateSignatureDiffersForRawAndPreEncodedModelPathRegression was also performed:
go test -gcflags="all=-N -l" ./providergo test -gcflags="all=-N -l" -run TestBedrock ./...Ⅳ. Describe how to verify it
Method 1: Run unit tests
Method 2: Manual verification
modelrespectively:global.amazon.nova-2-lite-v1:0arn:aws:bedrock:us-east-1:123456789012:inference-profile/global.anthropic.claude-sonnet-4-20250514-v1:0Authorization: AWS4-HMAC-SHA256 ...X-Amz-Date: ...%3A/%253Aand%2F/%252Fcaliber scenarios).Ⅴ. Special notes for reviews
c12183ca.123456789012.Ⅵ. AI Coding Tool Usage Checklist (if applicable)
Please check all applicable items:
AI Coding Summary
Root cause of the problem:
c12183cachangesencodeSigV4Pathto "single encoding normalization" (decode first and then re-encode)%3A,%2F)Fix:
url.PathEscape(seg)Scope of influence:
provider/bedrock.go:encodeSigV4Pathlogic rolls back and cleans up related auxiliary functionsprovider/bedrock_sigv4_path_test.go: Add/enhance SigV4 canonical URI and signature test