Repository navigation
fix: Bedrock SigV4 signing and secret redaction - #248
Merged
Merged
Conversation
The signing key chain passed every HMAC its key and data swapped and dropped the AWS4 prefix on the secret, so AWS rejected every request signed with an access key. The canonical request also left path segments unencoded (the ':' in Bedrock model ids), kept the query unsorted and did not collapse header whitespace. Bedrock, Claude on AWS and Polly share this signer. The new tests pin signatures computed by botocore's SigV4Auth. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A3WxDBWBA4ituJjXsHaEAa
…tput
AwsCredentials and CodexTokens derived Debug, so a {:?} in logs or an
error message printed the secret access key, session token, and the
Codex access and refresh tokens. Both now have hand-written Debug impls
that print [REDACTED] for the secrets.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A3WxDBWBA4ituJjXsHaEAa
This was referenced Oct 12, 2026
cunninghamcard-bit
marked this pull request as ready for review
October 12, 2026 11:09
Rust's f64 parser accepts `inf`, `Infinity` and `NaN`, and `as i64` saturates infinity, so `retry-after-ms: Infinity` produced an i64::MAX hint. The core retry loop dropped it through its window check, but the public hint (FFI `aimux_error_retry_ms`, the Node and Python `retry_ms`) reported it. A non-finite value, including seconds that overflow once converted to milliseconds, is now no hint, as upstream never uses it either. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A3WxDBWBA4ituJjXsHaEAa
is_sensitive_key matched only cookie, key, authorization, api-key and *token names, so X-Prodia-Key and Azure's Ocp-Apim-Subscription-Key were written in clear text into recordings (and logs, which share the policy) whenever recording was on. Both names are now sensitive; a recording test sends them through the HTTP layer and checks that the stored exchange holds [REDACTED]. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A3WxDBWBA4ituJjXsHaEAa
sanitized_request_url dropped only the query, so a base URL written as https://user:password@host kept its credentials in tracing fields, ApiCallError.url and recorded exchanges. It now clears the username and password too, also for a URL that does not parse. The download guard's rejection message reuses it, so a blocked ftp://user:pass@ URL no longer echoes the password. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A3WxDBWBA4ituJjXsHaEAa
The Dart NativeFinalizer called aimux_drop_handle(uint64_t) through a void (*)(void *) pointer. On 32-bit ARM (armeabi-v7a, which the release ships) a uint64_t argument does not travel like a pointer, so the drop received a garbage high word and a GC'd Model or ProviderHandle leaked its handle. aimux-ffi now exports aimux_drop_handle_ptr(void *), which takes the handle as a pointer-sized value and drops it, and the Flutter finalizer uses it. Tests: the Rust unit test and the export smoke test release handles through it, and a Flutter test checks it releases the handle its address names. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A3WxDBWBA4ituJjXsHaEAa
Fourteen of the binding's block_on calls (generate_text, generate_object, consume_stream_text, generate_text_as_openai, ProviderHandle.list_models, get_model_specs and the eight multimodal calls) held the GIL while they waited on the network, so every other Python thread stopped for the whole request. They now run inside allow_threads, as decide() and the stream iterator already did, and so do the recording flushes, which wait on the writer thread. Results and errors are unchanged. test_gil.py serves the mock API from a thread of the test process: it can only answer while the call releases the GIL. Without the fix both calls fail with their 10 s total timeout. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A3WxDBWBA4ituJjXsHaEAa
A request's `api_key: "env:VAR"` read any environment variable of the console process. Together with a caller-supplied base_url, a client of a non-loopback console could send any server variable to a host of its choice. resolve_api_key now accepts only the variable the provider's key is registered under (the native providers' standard variable, or the registry / overlay entry's) and refuses every other name before reading it. This covers the calls, cache-probe and replay paths, which all resolve keys through it. aimux-providers gains provider_api_key_env(), the variable provider() reads a key from when none is passed, so the console does not keep its own copy of the registry. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A3WxDBWBA4ituJjXsHaEAa
/api/calls and /api/cache-probe resolved the key from the request spec, then the Settings key store, then the provider's env var, and passed the request's base_url to the provider unchanged. A client could point base_url at its own host and receive the console's key. Replay had the same hole through an imported recording's base URL. When the base URL differs from the provider's own default, only a key carried in the request (plaintext, loopback binds only) is used: the Settings key and the env var are not read, env: references are refused, and a call without a key goes out with an empty key instead of the provider's env fallback. The test sends calls with a custom base_url to a local server and checks that neither the stored key nor the env key arrives. The alignment doc's §0.6 note now records the web fixes as done. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A3WxDBWBA4ituJjXsHaEAa
This was referenced Oct 14, 2026
eric8810
reviewed
Oct 15, 2026
eric8810
left a comment
eric8810
left a comment
Contributor
There was a problem hiding this comment.
评审结论：批准合并
9 个提交与 #228 的 9 项一一对应（N1/B11/B12/B13/S2-1/S2-2/N2/C30/S2-3，经 issue 原文核对），SigV4 修复经独立复算验证正确，所有 redaction 均有诱饵 canary 测试且实测全绿。
关键核验：
- SigV4：旧 bug 确认为 HMAC key/message 全链互换；修复后的派生链（AWS4+secret→date→region→service→aws4_request）、canonical request、非 S3 URI 逐段双重编码、query 解码重编码排序、header 空白折叠均符合 AWS 文档。4 个 pinned 签名用独立实现按官方文档逐位复算，全部一致。
- redaction：录制层 X-Prodia-Key / Ocp-Apim-Subscription-Key 落盘 [REDACTED]；URL userinfo 在日志/错误/录制三处统一剥除（sanitized_request_url）；web 端到端用真实 TCP capture server + 三个诱饵证明 stored/env key 不外发到自定义 base_url。
- RFC-0040 §3.1 对齐：
Some("")显式空值不落回 env 的语义恰是 RFC 要求的提前落地；错误只含字段名/env 名。 - 本机实跑（/tmp 抽取副本）：sigv4 5/5、aimux-web 26/26、core error/recording 59、http_recording 5/5、ffi exports 22/22。
两点请回复：
- native 路径空 key 实际发
Authorization: Bearer（空 bearer）而非字面"不带 key"——安全等价，但与描述有出入，可考虑省略该 header； x-amz-content-sha256作为签名头是本实现自选（botocore 对非 S3 默认不加该头），请确认 pin 值是给 botocore 显式加该头后算出的。
注意：diff 里对 master 的大额删除是分叉后 RFC 批次合入的幻影，rebase 即消；与 #247 在 wire.rs 的冲突按描述由后合方解决。roadmap：#228 追踪的 master 线修复，无节点对应，与"master 线修复不等 V4 迁移"的分工一致。
3 of 12 tasks
cunninghamcard-bit
pushed a commit
that referenced
this pull request
Oct 16, 2026
Brings in RFC-0042 (#212) and the Bedrock SigV4 fix (#248); no conflicts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0112xDERMUyD5JALkFhnsdWS
cunninghamcard-bit
pushed a commit
that referenced
this pull request
Oct 16, 2026
…low-ups, the RFC-0043 docs, the RFC-0036 §12.2 back-link, a test race fix and the review follow-ups The restored lower-layer tests take this layer's types and JSON: the trace suite's stream file is a `LanguageModelFile`, the repeated-id test reads the tool result's `input` from the AI SDK JSON, and the FFI recoverable-frame test finds the error part by its `type`. The Gemini media-type check keeps this layer's `LanguageModelFile`, and the alignment doc's decision paragraph keeps this layer's camelCase text with the Anthropic adapter added. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A3WxDBWBA4ituJjXsHaEAa
Contributor
Author
Generated by Claude Code |
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
3 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.
Requested by li
Summary
Before: on master, Bedrock SigV4 signatures do not match AWS (the signing-key derivation swaps the HMAC key and message, and the canonical URI, query and header-value rules are missing), and
AwsCredentials/CodexTokensprint their secrets in{:?}. aimux-web sends the Settings key or the provider's env key to whateverbase_urlthe caller passes, andenv:VARin a request reads any environment variable of the console process. Recordings keep the Prodia and Azure subscription key headers in clear text, and request URLs keepuser:password@in logs, errors and recordings. ARetry-After: Infinityturns into a hint ofi64::MAXms. The Python binding holds the GIL while it waits on the network, and the FlutterNativeFinalizercalls theuint64_tdrop function through avoid*signature.After: signatures are computed as botocore's
SigV4Authcomputes them (tests pin botocore signatures), and both structs print[REDACTED]. aimux-web uses only a key carried in the request when the base URL is not the provider's default (no Settings key, no env key,env:refused), andenv:VARis accepted only for the provider's registered key variable. The two headers are recorded as[REDACTED]; userinfo is stripped wherever a request URL is shown. Non-finiteRetry-Aftervalues give no hint. Every blocking Python call releases the GIL. The Flutter finalizer uses a new pointer-sized entry point,aimux_drop_handle_ptr(void *).These are fixes for the released line, independent of the AI SDK migration series ([1/8] to [8/8]), so this PR can merge first.
Type of change
Checklist
cargo fmt --all -- --checkpassescargo clippy --workspace --all-targets -- -D warningspassescargo test --workspacepasses (cassette playback — no network/keys)README.md,docs/,rfc/,bindings/README.md)Notes for reviewers
master. Not part of the [1/8] to [8/8] series; the same fixes are carried into the series in the layer that rewrites each piece of code.base_urlonly uses a key sent in the request, and plaintext keys are accepted only on a loopback bind. On a LAN bind such a call goes out without a key. README and RFC-0029 §5.5 say so.aimux_drop_handle_ptr; nothing removed or changed.tools/aimux-web/src/wire.rs, which the web wire types PR (refactor(web): generate the web wire types with ts-rs #247) also changes. Whichever merges second resolves it.ffi-x86_64-unknown-linux-gnuffi-aarch64-apple-darwinffi-x86_64-pc-windows-msvcnode-x86_64-unknown-linux-gnunode-aarch64-unknown-linux-gnunode-aarch64-apple-darwinnode-x86_64-apple-darwinnode-x86_64-pc-windows-msvcnode-aarch64-pc-windows-msvc