From da2ebdd87e81e2c0c508934d59ab8d144ced09c2 Mon Sep 17 00:00:00 2001 From: Jarvis Date: Thu, 3 Sep 2026 11:44:19 +0000 Subject: [PATCH 1/5] ci: publish the full version image tag only, no :X.Y or :X MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A release tag published `:X.Y.Z`, `:X.Y`, `:X` and `:latest`. The two abbreviations are removed; `:latest`, `:dev`, `:sha-` and PR refs are unchanged. An abbreviated tag reads like a line but is a snapshot: `:1` published at 1.0.0 stops moving the day 1.1.0 ships, so anyone who pinned it silently keeps an old image under a name that suggests otherwise. The full version and `:latest` are the two honest answers, and the project is about to cut its first non-0.x version, which is when a `:1` would first be created. Also removes the `:X.Y` sentence the release-draft workflow stamps into every stable release body, and the tag list in RELEASING.md — both would otherwise advertise tags that are no longer published. Existing published abbreviations are unaffected; this only stops new ones being created. --- .github/workflows/docker-image.yml | 22 ++++++++++++---------- .github/workflows/release-draft.yml | 11 +++++------ RELEASING.md | 3 ++- 3 files changed, 19 insertions(+), 17 deletions(-) diff --git a/.github/workflows/docker-image.yml b/.github/workflows/docker-image.yml index df526c05b..a719f63cc 100644 --- a/.github/workflows/docker-image.yml +++ b/.github/workflows/docker-image.yml @@ -5,17 +5,20 @@ name: docker-image # Tag strategy. Every push always carries a :sha- tag for # traceability; the other tags layer on top based on the trigger: # - push to main → :dev + :sha- -# - push tag vX.Y.Z → :X.Y.Z + :X.Y + :X + :latest -# + :sha- -# - push tag vX.Y.Z-rc.N → :X.Y.Z-rc.N + :sha- (NO rolling tags) +# - push tag vX.Y.Z → :X.Y.Z + :latest + :sha- +# - push tag vX.Y.Z-rc.N → :X.Y.Z-rc.N + :sha- (NO :latest) # - pull request → build-only (no push), sanity check # - workflow_dispatch → :sha- + optional tag input # +# A version tag is published in full only — there is no :X.Y or :X +# abbreviation. An abbreviation freezes the moment the next release ships: +# whoever pinned it keeps the old image while the tag reads like the line. +# :latest and :dev are the only moving pointers. +# # Pre-release tags exist so a release candidate can be QA'd on the artifacts # customers receive before anything points at it — bar the PGO difference # noted at "Decide PGO build mode" below. They must move no rolling -# pointer: docker/metadata-action already skips :X.Y and :X for -# pre-release semver, and the guards below keep :latest and the published +# pointer: the guards below keep :latest and the published # `openapi-latest.json` on the last stable release. on: @@ -157,16 +160,15 @@ jobs: images: | ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }} ${{ startsWith(github.ref, 'refs/tags/v') && format('{0}/{1}', secrets.DOCKER_REGISTRY, env.IMAGE_NAME) || '' }} - # `latest` is guarded on `!contains(ref_name, '-')` so a `-rc.N` tag - # cannot move it; metadata-action already skips {{major}}.{{minor}} - # and {{major}} for pre-release semver. + # A version tag publishes the full version only — no + # {{major}}.{{minor}} / {{major}} abbreviations, see the tag strategy + # at the top. `latest` is guarded on `!contains(ref_name, '-')` so a + # `-rc.N` tag cannot move it. tags: | type=raw,value=dev,enable=${{ github.event_name == 'push' && github.ref == 'refs/heads/main' }} type=ref,event=pr type=sha,prefix=sha-,format=short type=semver,pattern={{version}} - type=semver,pattern={{major}}.{{minor}} - type=semver,pattern={{major}} type=raw,value=latest,enable=${{ startsWith(github.ref, 'refs/tags/v') && !contains(github.ref_name, '-') }} type=raw,value=${{ github.event.inputs.tag }},enable=${{ github.event_name == 'workflow_dispatch' && github.event.inputs.tag != '' }} labels: | diff --git a/.github/workflows/release-draft.yml b/.github/workflows/release-draft.yml index 17a740b79..f054c39ae 100644 --- a/.github/workflows/release-draft.yml +++ b/.github/workflows/release-draft.yml @@ -44,18 +44,17 @@ jobs: VERSION="${TAG#v}" # v0.4.0 -> 0.4.0 (image tags carry no leading v) - # Rolling-tags sentence. docker-image.yml publishes the :X.Y (and - # :latest) tags only for stable releases; docker/metadata-action skips - # major.minor for pre-release semver, so a -rc draft must not advertise - # an :X.Y tag that never gets pushed. + # Rolling-tag sentence. A version tag is published in full only — + # there is no :X.Y or :X abbreviation — and docker-image.yml moves + # :latest for stable releases only, so a -rc draft must not advertise + # a `latest` tag that never points at it. PKG="https://github.com/orgs/api7/packages/container/package/aisix" case "$TAG" in *-*) ROLLING="Other image tags and the \`docker.io/api7/aisix\` mirror for private/offline deployments are listed on the [package page]($PKG)." ;; *) - MINOR="${VERSION%.*}" # 0.4.0 -> 0.4 - ROLLING="Rolling tags (\`$MINOR\`, \`latest\`) and the \`docker.io/api7/aisix\` mirror for private/offline deployments are listed on the [package page]($PKG)." + ROLLING="The rolling \`latest\` tag and the \`docker.io/api7/aisix\` mirror for private/offline deployments are listed on the [package page]($PKG)." ;; esac diff --git a/RELEASING.md b/RELEASING.md index fa3f22b3f..25de7b424 100644 --- a/RELEASING.md +++ b/RELEASING.md @@ -15,7 +15,8 @@ git push origin vX.Y.Z Pushing the tag triggers two workflows: - **`docker-image.yml`** builds and publishes - `ghcr.io/api7/aisix:X.Y.Z` (plus `:X.Y`, `:X`, `:latest`, `:sha-`), + `ghcr.io/api7/aisix:X.Y.Z` (plus `:latest` and `:sha-`; a version + tag is published in full only, with no `:X.Y` or `:X` abbreviation), mirrors the release tag to `docker.io/api7/aisix` for private/offline deployments, signs the images with cosign, and stamps the version into the binary so a running gateway self-reports `X.Y.Z` (`--version`, `Server` From c42a1910e42911f18d9e2961e4766441a010d5cb Mon Sep 17 00:00:00 2001 From: Jarvis Date: Thu, 3 Sep 2026 11:53:03 +0000 Subject: [PATCH 2/5] fix(obs): count an unscannable-body bypass in aisix_guardrail_bypasses_total MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `aisix_guardrail_bypasses_total` only ever counted bypasses a guardrail member reported: the increment hangs off the per-execution record, and an unscannable body has no execution by construction — the body could not be decoded, so no member ran. The chain recorded that pass-through to the audit log (and so to `UsageEvent.guardrail_bypassed_reason`) and to nothing else. The sibling counter is the other way round: `aisix_guardrail_blocks_total` deliberately includes the fail-closed paths that happen before a member executes. So one situation — the body could not be scanned — was counted when the chain refused and counted nowhere when it let the request through, which is the direction an operator reads `bypasses_total` to find. #1115 widened exactly that fail-open half, so the traffic newly passing through was the traffic the counter could not see. `GuardrailMetricsSink` gains `record_guardrail_bypass(reason)` and `GuardrailChain::record_bypass` now feeds both receivers. Deliberately a second method rather than a synthetic execution: there is no member, kind or duration to report, and inventing them would put a phantom row in the per-execution latency histogram. It is required rather than defaulted so a sink cannot silently drop these. Which pass-throughs count is unchanged and is still decided in one place. `record_unevaluable_{input,output}_bypass` already distinguishes a chain whose members all fail open on that side (a bypass) from one where no member reads that side (never offered to screen it, so not a bypass), off the same member set the refusal gate uses. The counter reads that decision rather than deriving its own. Tests: the chain unit test that pins the distinction now asserts both sinks, so the usage field and the counter cannot disagree about which pass-through was a bypass. All three assertions were verified to go red under mutation — dropping the sink forwarding, counting every pass-through, and letting the counter derive its own predicate. The bypass-reason e2e additionally asserts the `reason="unscannable_body"` delta in `GET /metrics` after driving a real unscannable request. --- crates/aisix-core/src/models/guardrail.rs | 14 +++ crates/aisix-guardrails/src/build.rs | 2 + crates/aisix-guardrails/src/chain.rs | 89 ++++++++++++++++--- crates/aisix-obs/src/metrics.rs | 31 +++++-- .../cases/guardrail-bypass-reason-e2e.test.ts | 23 +++++ 5 files changed, 138 insertions(+), 21 deletions(-) diff --git a/crates/aisix-core/src/models/guardrail.rs b/crates/aisix-core/src/models/guardrail.rs index 8eec1ceb0..2b3b8d858 100644 --- a/crates/aisix-core/src/models/guardrail.rs +++ b/crates/aisix-core/src/models/guardrail.rs @@ -1295,6 +1295,20 @@ pub struct GuardrailExecution<'a> { /// build time, so aisix-guardrails stays free of a metrics dependency. pub trait GuardrailMetricsSink: Send + Sync + 'static { fn record_guardrail_execution(&self, exec: &GuardrailExecution<'_>); + + /// Count one bypass the PROXY performed on the chain's behalf, with + /// no member execution to carry it: a body the gateway could not give + /// the guardrails at all, which a fail-open chain lets through (#1115). + /// + /// The execution-driven bypasses reach the same counter through + /// [`Self::record_guardrail_execution`], which is why this is a second + /// method rather than a synthetic execution — there is no member, no + /// kind and no duration to report, and inventing them would put a + /// phantom row in the per-execution latency histogram. + /// + /// Required rather than defaulted: a sink that silently dropped these + /// would under-report exactly the traffic the counter is read to find. + fn record_guardrail_bypass(&self, reason: &str); } /// Content policy evaluated before or after upstream calls. diff --git a/crates/aisix-guardrails/src/build.rs b/crates/aisix-guardrails/src/build.rs index 8d893424d..e0de294b0 100644 --- a/crates/aisix-guardrails/src/build.rs +++ b/crates/aisix-guardrails/src/build.rs @@ -1869,6 +1869,7 @@ mod tests { .unwrap() .push(exec.error_type.map(str::to_owned)); } + fn record_guardrail_bypass(&self, _reason: &str) {} } for (script, tag) in [ @@ -3814,6 +3815,7 @@ mod tests { exec.result, )); } + fn record_guardrail_bypass(&self, _reason: &str) {} } /// A sink attached via `LiveGuardrailIndex::new_with_sink` reaches every diff --git a/crates/aisix-guardrails/src/chain.rs b/crates/aisix-guardrails/src/chain.rs index e4748ae17..fa54bd03f 100644 --- a/crates/aisix-guardrails/src/chain.rs +++ b/crates/aisix-guardrails/src/chain.rs @@ -182,11 +182,37 @@ impl GuardrailChain { /// member can report it, yet the request was screened by nothing — /// exactly what the field is read to rule out. /// - /// A no-op when the chain carries no log; first-wins is - /// [`GuardrailAuditLog::record_bypass`]'s rule. + /// Reported to BOTH receivers, and they answer different questions. + /// The audit log keeps the first tag only — it feeds a single + /// per-request field. The metrics sink counts every call, matching + /// `aisix_guardrail_bypasses_total`'s per-event meaning on the + /// execution-driven path. The two cannot double-count: a bypass + /// recorded here had no execution to report, and `record_execution` + /// reaches the counter through the sink's execution method instead. + /// + /// Counting here rather than only auditing is what keeps + /// `aisix_guardrail_bypasses_total` symmetric with + /// `aisix_guardrail_blocks_total`, which already counts the + /// pre-execution refusals. Without it the SAME unscannable body was + /// counted when the chain refused and counted nowhere when it let the + /// request through — the direction an operator is reading the counter + /// to find. + /// + /// The tag is clamped once here so both receivers carry the identical + /// value; [`GuardrailAuditLog::record_bypass`] clamps too, and the + /// clamp is idempotent. + /// + /// A no-op for whichever receiver the chain does not carry. pub fn record_bypass(&self, reason: &str) { + if self.audit.is_none() && self.sink.is_none() { + return; + } + let tag = crate::bounded_failure_tag(reason); if let Some(audit) = self.audit.as_ref() { - audit.record_bypass(reason); + audit.record_bypass(&tag); + } + if let Some(sink) = self.sink.as_ref() { + sink.record_guardrail_bypass(&tag); } } @@ -1203,38 +1229,60 @@ mod tests { /// bypass only when something would have read that side. An output-only /// chain never offered to screen the request, so tagging it would fire /// the field on requests that were never going to be screened. + /// + /// Both sinks are asserted from the one member-set decision: + /// `guardrail_bypassed_reason` on the usage event and the + /// `aisix_guardrail_bypasses_total` increment must agree about which + /// pass-through was a bypass, which is only true while they read the + /// same predicate rather than each deriving its own. #[test] fn an_unevaluable_pass_is_a_bypass_only_when_a_member_reads_that_side() { let rule = || vec![KeywordRule::literal("x")]; let log = || Some(Arc::new(GuardrailAuditLog::new())); + let sinked = |g: Arc| { + let sink = Arc::new(RecordingSink::default()); + let chain = GuardrailChain::new(vec![g]) + .with_audit_log(log()) + .with_metrics_sink(Some(Arc::clone(&sink) as Arc)); + (chain, sink) + }; - let open_in = GuardrailChain::new(vec![Arc::new( + let (open_in, open_in_sink) = sinked(Arc::new( KeywordBlocklist::input_only(rule()).with_fail_open(true), - )]) - .with_audit_log(log()); + )); open_in.record_unevaluable_input_bypass("unscannable_body"); assert_eq!( open_in.bypass_reason().as_deref(), Some("unscannable_body"), "a fail-open row that reads the request WAS bypassed", ); + assert_eq!( + open_in_sink.bypasses(), + ["unscannable_body"], + "the same bypass has to reach the counter: no member executed, \ + so nothing else will count it", + ); - let output_only = - GuardrailChain::new(vec![Arc::new(KeywordBlocklist::output_only(rule()))]) - .with_audit_log(log()); + let (output_only, output_only_sink) = + sinked(Arc::new(KeywordBlocklist::output_only(rule()))); output_only.record_unevaluable_input_bypass("unscannable_body"); assert_eq!( output_only.bypass_reason(), None, "nothing here reads the request, so nothing was bypassed", ); + assert!( + output_only_sink.bypasses().is_empty(), + "counting this would make the counter fire on requests that were \ + never going to be screened", + ); // The fail-closed direction never reaches the call at all, but the // predicate must agree with the refusal gate if it ever does. - let closed_in = GuardrailChain::new(vec![Arc::new(KeywordBlocklist::input_only(rule()))]) - .with_audit_log(log()); + let (closed_in, closed_in_sink) = sinked(Arc::new(KeywordBlocklist::input_only(rule()))); closed_in.record_unevaluable_input_bypass("unscannable_body"); assert_eq!(closed_in.bypass_reason(), None); + assert!(closed_in_sink.bypasses().is_empty()); } /// AISIX-Cloud#1365: a fail-CLOSED refusal is an outage, not a policy @@ -1315,6 +1363,7 @@ mod tests { exec.error_type.unwrap_or("none").to_owned(), )); } + fn record_guardrail_bypass(&self, _reason: &str) {} } struct Unavailable; #[async_trait] @@ -1886,11 +1935,16 @@ mod tests { } #[derive(Default)] - struct RecordingSink(std::sync::Mutex>); + struct RecordingSink { + execs: std::sync::Mutex>, + /// Pre-execution bypasses, the `aisix_guardrail_bypasses_total` + /// increments that carry no execution record. + bypasses: std::sync::Mutex>, + } impl GuardrailMetricsSink for RecordingSink { fn record_guardrail_execution(&self, exec: &GuardrailExecution<'_>) { - self.0.lock().unwrap().push(Recorded { + self.execs.lock().unwrap().push(Recorded { guardrail: exec.guardrail_name.to_owned(), kind: exec.kind.to_owned(), phase: exec.phase, @@ -1898,11 +1952,18 @@ mod tests { error_type: exec.error_type.map(str::to_owned), }); } + fn record_guardrail_bypass(&self, reason: &str) { + self.bypasses.lock().unwrap().push(reason.to_owned()); + } } impl RecordingSink { fn take(&self) -> Vec { - std::mem::take(&mut self.0.lock().unwrap()) + std::mem::take(&mut self.execs.lock().unwrap()) + } + + fn bypasses(&self) -> Vec { + self.bypasses.lock().unwrap().clone() } } diff --git a/crates/aisix-obs/src/metrics.rs b/crates/aisix-obs/src/metrics.rs index 019f97992..aa47d1321 100644 --- a/crates/aisix-obs/src/metrics.rs +++ b/crates/aisix-obs/src/metrics.rs @@ -208,11 +208,20 @@ pub const M_USAGE_EVENT_DROPS_TOTAL: &str = "aisix_usage_event_drops_total"; /// Guardrail outcomes (#379 observability). `aisix_guardrail_blocks_total` /// counts requests rejected by guardrail enforcement, including fail-closed /// paths such as a streaming buffer overflow that happen before a guardrail -/// member executes. `aisix_guardrail_bypasses_total` counts fail-open -/// executions — a remote-API guardrail's upstream was unreachable but -/// `fail_open` let the request through — sliced by the bounded DP-internal -/// `reason` (e.g. `bedrock_5xx` / `bedrock_timeout` / -/// `bedrock_throttled`). +/// member executes. `aisix_guardrail_bypasses_total` is its fail-OPEN +/// counterpart and covers the same two shapes: a member that executed and +/// was bypassed — a remote-API guardrail's upstream was unreachable but +/// `fail_open` let the request through — and a bypass that happens before +/// any member executes, a body the gateway could not give the guardrails +/// at all (`unscannable_body`, #1115). Both are sliced by the bounded +/// DP-internal `reason` (e.g. `bedrock_5xx` / `bedrock_timeout` / +/// `bedrock_throttled` / `unscannable_body`). +/// +/// The two counters cover the pre-execution case symmetrically on +/// purpose: one situation — the body could not be scanned — is a block +/// when the chain fails closed and a bypass when it fails open, and an +/// operator reads `bypasses_total` to size how much unscreened traffic +/// got through. pub const M_GUARDRAIL_BLOCKS_TOTAL: &str = "aisix_guardrail_blocks_total"; pub const M_GUARDRAIL_BYPASSES_TOTAL: &str = "aisix_guardrail_bypasses_total"; @@ -1336,7 +1345,11 @@ impl Metrics { ); } - fn record_guardrail_bypass_execution(&self, reason: &str) { + /// Bump `aisix_guardrail_bypasses_total` for one bypass under + /// `reason`. Reached from both producers: a bypassed member execution + /// (below) and the proxy's pre-execution bypass (the + /// `GuardrailMetricsSink` impl). + fn count_guardrail_bypass(&self, reason: &str) { self.cached_counter( M_GUARDRAIL_BYPASSES_TOTAL, 1, @@ -1380,7 +1393,7 @@ impl Metrics { }, ); if let ("bypassed", Some(reason)) = (exec.result, exec.error_type) { - self.record_guardrail_bypass_execution(reason); + self.count_guardrail_bypass(reason); } } @@ -2530,6 +2543,10 @@ impl aisix_core::GuardrailMetricsSink for Metrics { fn record_guardrail_execution(&self, exec: &aisix_core::GuardrailExecution<'_>) { Metrics::record_guardrail_execution(self, exec); } + + fn record_guardrail_bypass(&self, reason: &str) { + self.count_guardrail_bypass(reason); + } } /// Who was called and how, for the `aisix_a2a_*` family. Every field is diff --git a/tests/e2e/src/cases/guardrail-bypass-reason-e2e.test.ts b/tests/e2e/src/cases/guardrail-bypass-reason-e2e.test.ts index 7e814bb20..88f239de4 100644 --- a/tests/e2e/src/cases/guardrail-bypass-reason-e2e.test.ts +++ b/tests/e2e/src/cases/guardrail-bypass-reason-e2e.test.ts @@ -4,6 +4,8 @@ import { EtcdClient, ProxyClient, SeedClient, + metricDelta, + scrapeMetrics, spawnApp, startMockSls, startOpenAiUpstream, @@ -262,9 +264,30 @@ describe("guardrail bypass reason across the handler family", () => { // the tag has to say. A different tag from the case above on purpose: // "the guardrail broke" and "we could not give it the body" are // different outages with different fixes. + const before = await scrapeMetrics(app.metricsUrl); const res = await post("/v1/messages", { model: "bypass-unscannable", max_tokens: 64 }, true); expect(res.status).toBe(200); await expectBypassReason("bypass-unscannable", UNSCANNABLE_TAG); + + // The same pass-through has to show up in the scrape, not only in the + // usage row. No member executed, so the per-execution path that feeds + // this counter for a provider outage never fires here — an operator + // sizing "how much unscreened traffic got through" from + // `aisix_guardrail_bypasses_total` would otherwise read zero for the + // one cause the counter exists to expose. Scraped after the terminal + // event so the request is provably finished. + // + // Only the positive direction is driven here: which pass-throughs are + // bypasses at all is decided by the chain's member set, and that + // discrimination is pinned in aisix-guardrails' chain unit test, where + // an output-only chain can be built without a second seeded route. + const after = await scrapeMetrics(app.metricsUrl); + expect( + metricDelta(before, after, "aisix_guardrail_bypasses_total", { + reason: UNSCANNABLE_TAG, + }), + "an unscannable body a fail-open chain let through did not reach the bypass counter", + ).toBe(1); }); }); From 9b2a7aa5d075a6197b6f78587086b1236c1a6705 Mon Sep 17 00:00:00 2001 From: Jarvis Date: Thu, 3 Sep 2026 12:06:56 +0000 Subject: [PATCH 3/5] docs(obs): state the bypass counter's unit and its coverage limits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review findings on the doc comments added by the previous commit. All three were prose that overstated what the counter says; no behaviour changes. - `blocks_total` and `bypasses_total` are not the same unit, so calling them counterparts invited summing or comparing them. The first fires once per REQUEST off the terminal usage event; the second fires once per bypass EVENT, so one outage on a chain with two fail-open members on both hooks adds 4 for a single request. - "a body the gateway could not give the guardrails at all" claimed the counter covers every unscannable pass-through. It does not: `jobs::scan_output_blob` and `passthrough_route` scan a lossy copy with no failure policy involved and record nothing by design (crates/aisix-proxy/AGENTS.md), so a zero here is not proof that nothing went unread. - The pre-execution producer reaches this counter only. With no execution there is no `aisix_guardrail_latency_seconds` row, so that family's `result="bypassed"` slice stays execution-only and the two no longer agree on a total — worth saying where both are documented together. Also corrects the claim in `GuardrailChain::record_bypass` that the two producers cannot both fire on one request. No EVENT is counted twice, but a request can legitimately produce both: `audio.rs` records an undecodable transcript tail and then scans the decodable remainder, whose members may themselves fail open. --- crates/aisix-guardrails/src/chain.rs | 13 +++++++-- crates/aisix-obs/src/metrics.rs | 43 +++++++++++++++++++--------- 2 files changed, 39 insertions(+), 17 deletions(-) diff --git a/crates/aisix-guardrails/src/chain.rs b/crates/aisix-guardrails/src/chain.rs index fa54bd03f..b51576c03 100644 --- a/crates/aisix-guardrails/src/chain.rs +++ b/crates/aisix-guardrails/src/chain.rs @@ -186,9 +186,16 @@ impl GuardrailChain { /// The audit log keeps the first tag only — it feeds a single /// per-request field. The metrics sink counts every call, matching /// `aisix_guardrail_bypasses_total`'s per-event meaning on the - /// execution-driven path. The two cannot double-count: a bypass - /// recorded here had no execution to report, and `record_execution` - /// reaches the counter through the sink's execution method instead. + /// execution-driven path, where a chain with three bypassed members + /// already increments three times. + /// + /// No event is counted twice: a bypass recorded here had no execution + /// to report, and `record_execution` reaches the counter through the + /// sink's execution method instead. One REQUEST can still produce + /// both, and legitimately — `audio.rs` records an undecodable + /// transcript tail here and then scans the decodable remainder, whose + /// members may themselves fail open. Those are two things that went + /// unscreened, not one counted twice. /// /// Counting here rather than only auditing is what keeps /// `aisix_guardrail_bypasses_total` symmetric with diff --git a/crates/aisix-obs/src/metrics.rs b/crates/aisix-obs/src/metrics.rs index aa47d1321..fcec82594 100644 --- a/crates/aisix-obs/src/metrics.rs +++ b/crates/aisix-obs/src/metrics.rs @@ -206,22 +206,37 @@ pub const M_BUDGET_DETAILS_PRESENT: &str = "aisix_budget_details_present"; pub const M_REDIS_FAILURES_TOTAL: &str = "aisix_redis_failures_total"; pub const M_USAGE_EVENT_DROPS_TOTAL: &str = "aisix_usage_event_drops_total"; /// Guardrail outcomes (#379 observability). `aisix_guardrail_blocks_total` -/// counts requests rejected by guardrail enforcement, including fail-closed +/// counts REQUESTS rejected by guardrail enforcement, including fail-closed /// paths such as a streaming buffer overflow that happen before a guardrail -/// member executes. `aisix_guardrail_bypasses_total` is its fail-OPEN -/// counterpart and covers the same two shapes: a member that executed and -/// was bypassed — a remote-API guardrail's upstream was unreachable but -/// `fail_open` let the request through — and a bypass that happens before -/// any member executes, a body the gateway could not give the guardrails -/// at all (`unscannable_body`, #1115). Both are sliced by the bounded -/// DP-internal `reason` (e.g. `bedrock_5xx` / `bedrock_timeout` / -/// `bedrock_throttled` / `unscannable_body`). +/// member executes. /// -/// The two counters cover the pre-execution case symmetrically on -/// purpose: one situation — the body could not be scanned — is a block -/// when the chain fails closed and a bypass when it fails open, and an -/// operator reads `bypasses_total` to size how much unscreened traffic -/// got through. +/// `aisix_guardrail_bypasses_total` counts fail-OPEN EVENTS, sliced by the +/// bounded DP-internal `reason` (e.g. `bedrock_5xx` / `bedrock_timeout` / +/// `bedrock_throttled` / `unscannable_body`). Two producers reach it: a +/// member that executed and was bypassed — a remote-API guardrail's +/// upstream was unreachable but `fail_open` let the request through — and +/// a bypass the proxy records with no member execution behind it, an +/// unscannable body a fail-open chain let through (#1115). The second +/// producer reaches THIS counter only: with no execution there is no +/// `aisix_guardrail_latency_seconds` row, so that family's +/// `result="bypassed"` slice stays execution-only and the two no longer +/// agree on a total. +/// +/// The two are NOT the same unit and must not be compared or summed. +/// `blocks_total` fires once per request, off the terminal usage event; +/// `bypasses_total` fires once per bypass event, so one outage on a chain +/// with two fail-open members attached to both hooks adds 4 for a single +/// request. Read it as "how many screenings were skipped", never as a +/// count of requests that went unscreened. +/// +/// The pre-execution case is deliberately symmetric across the two: the +/// same unscannable body is a block when the chain fails closed and a +/// bypass when it fails open. It is not the whole of "the gateway could +/// not scan this", though — the sites that scan a lossy copy +/// unconditionally, with no failure policy involved +/// (`jobs::scan_output_blob`, `passthrough_route`), record nothing by +/// design, so a zero here is not proof that nothing went unread. The set +/// and its rationale live in `crates/aisix-proxy/AGENTS.md`. pub const M_GUARDRAIL_BLOCKS_TOTAL: &str = "aisix_guardrail_blocks_total"; pub const M_GUARDRAIL_BYPASSES_TOTAL: &str = "aisix_guardrail_bypasses_total"; From 6dcf44f18790bab1da789458f687c0f964d5edde Mon Sep 17 00:00:00 2001 From: Jarvis Date: Thu, 3 Sep 2026 12:36:46 +0000 Subject: [PATCH 4/5] fix(security): the AWS SigV4 headers are credential slots again MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three headers became reachable by an ordinary glob when the forwarding rules moved out of `config.rs`. `RESERVED_UPSTREAM_HEADERS` used to block `x-amz-security-token`, `x-amz-date` and `x-amz-content-sha256` outright. The split into `forwarded_headers` kept six of that constant's names as `CREDENTIAL_SLOT_HEADERS`, which a glob cannot reach, and the three `x-amz-*` names landed in no list at all — not the credential slots, not `NON_FORWARDABLE_HEADERS`, not `NEVER_FORWARD_FROM_CLIENT`. So a provider key configured `forward_client_headers: ["x-*"]` relayed the caller's AWS session token to the upstream. That contradicts the module's own rule, that a glob is a statement about the operator's own headers and not consent to hand a third party the caller's credential. Bedrock upstreams were never exposed — the SigV4 signer owns all three names and drops any supplied value before signing — so the reach was every OTHER upstream, which is where an AWS credential has no business arriving. The fix restores the credential-SLOT rule rather than the old absolute block: a glob does not reach the three, and an operator who names one exactly still forwards it, exactly as the other six behave. That keeps the capability available to an upstream that genuinely reads the caller's SigV4 headers, which the pre-split behaviour did not allow. `provider_key` and `passthrough_route` enumerate the credential slots in the field descriptions the control plane and the public API reference read, so both gain the three names and the schemas are regenerated. `mcp_server` says "a credential slot" without enumerating and needed no change. Tests name the three headers explicitly rather than iterating the constant — a loop over `CREDENTIAL_SLOT_HEADERS` can only confirm that whatever is in the list behaves, and can never notice a name that dropped out of it, which is exactly how this went unseen. Both go red without the fix, the gateway-level one by showing all three arriving at the upstream under `["x-*"]`. --- crates/aisix-core/src/forwarded_headers.rs | 56 ++++++++++++++++--- .../src/models/passthrough_route.rs | 4 +- crates/aisix-core/src/models/provider_key.rs | 4 +- crates/aisix-gateway/src/upstream_headers.rs | 39 +++++++++++++ .../resources/passthrough_route.schema.json | 2 +- schemas/resources/provider_key.schema.json | 2 +- 6 files changed, 96 insertions(+), 11 deletions(-) diff --git a/crates/aisix-core/src/forwarded_headers.rs b/crates/aisix-core/src/forwarded_headers.rs index 9d12232d7..f29ce0422 100644 --- a/crates/aisix-core/src/forwarded_headers.rs +++ b/crates/aisix-core/src/forwarded_headers.rs @@ -97,20 +97,32 @@ pub const NEVER_FORWARD_FROM_CLIENT: &[&str] = &[ /// for the same reason. pub const NEVER_FORWARD_FROM_CLIENT_PREFIXES: &[&str] = &["x-stainless-"]; -/// Headers whose value is, or stands in for, a caller credential. +/// Headers whose value is, or travels as part of, a caller credential. /// /// Forwardable — an internal upstream reading the end user's own /// credential in the slot it already reads is what this capability is for /// — and the one set a forwarded value may DISPLACE a gateway-injected /// header in. Everything else the gateway put on the request it put there /// to make the exchange work, so a forward leaves it alone. +/// +/// The `x-amz-*` trio is SigV4 material rather than a bearer token on its +/// own: `x-amz-security-token` IS a credential, and the other two are the +/// timestamp and body hash the signature is computed over, which is why +/// they belong to the same request's identity and are listed together. +/// A Bedrock upstream never sees a forwarded copy — its signer owns all +/// three and drops any supplied value — so what this entry governs is +/// every OTHER upstream, where relaying the caller's AWS session token +/// under a `"x-*"` glob would hand a third party a live AWS credential. pub const CREDENTIAL_SLOT_HEADERS: &[&str] = &[ - "api-key", // Azure OpenAI key - "authorization", // OpenAI / Anthropic / Vertex Bearer - "cookie", // session credential - "proxy-authorization", // proxy auth - "x-api-key", // Anthropic raw, also OpenAI legacy proxies - "x-goog-api-key", // Gemini API key + "api-key", // Azure OpenAI key + "authorization", // OpenAI / Anthropic / Vertex Bearer + "cookie", // session credential + "proxy-authorization", // proxy auth + "x-amz-content-sha256", // AWS SigV4 body hash + "x-amz-date", // AWS SigV4 timestamp + "x-amz-security-token", // AWS SigV4 session token + "x-api-key", // Anthropic raw, also OpenAI legacy proxies + "x-goog-api-key", // Gemini API key ]; /// Trace-context headers, which a glob never sweeps in. @@ -299,6 +311,36 @@ mod tests { } } + /// The loop above iterates the list, so it can only ever confirm that + /// whatever is IN the list behaves — it can never notice a name that + /// dropped out. These three did: `x-amz-security-token`, `x-amz-date` + /// and `x-amz-content-sha256` were blocked outright before the + /// forwarding rules moved into this module, and came out of the split + /// naming no list at all, which left a `["x-*"]` pattern relaying the + /// caller's AWS session token to any non-Bedrock upstream. + /// + /// Spelling them out is the point: this assertion goes red if one is + /// removed again, and the loop above would not. + #[test] + fn the_aws_sigv4_trio_is_a_credential_slot() { + for name in ["x-amz-security-token", "x-amz-date", "x-amz-content-sha256"] { + assert!( + exact_match_only(name), + "{name} must need its own name — a glob reaching it hands a \ + third-party upstream the caller's AWS credential", + ); + assert!(!forward_pattern_admits(&["*".into()], name), "{name}"); + assert!(!forward_pattern_admits(&["x-*".into()], name), "{name}"); + assert!(!forward_pattern_admits(&["x-amz-*".into()], name), "{name}"); + // Still a credential SLOT, not an absolute block: an operator + // who names one exactly is opting the header in, the same way + // the other six behave. + assert!(forward_pattern_admits(&[name.to_string()], name), "{name}"); + assert!(client_header_forwardable(name), "{name}"); + assert!(!header_forward_blocked(name), "{name}"); + } + } + #[test] fn only_a_credential_slot_displaces_a_gateway_header() { for name in CREDENTIAL_SLOT_HEADERS { diff --git a/crates/aisix-core/src/models/passthrough_route.rs b/crates/aisix-core/src/models/passthrough_route.rs index 26093bedf..acd6b896c 100644 --- a/crates/aisix-core/src/models/passthrough_route.rs +++ b/crates/aisix-core/src/models/passthrough_route.rs @@ -141,7 +141,9 @@ pub struct PassthroughRoute { /// keep doing so unchanged. /// /// A credential slot — `authorization`, `proxy-authorization`, - /// `x-api-key`, `api-key`, `x-goog-api-key`, `cookie` — and + /// `x-api-key`, `api-key`, `x-goog-api-key`, `cookie`, and the AWS + /// SigV4 trio `x-amz-security-token` / `x-amz-date` / + /// `x-amz-content-sha256` — and /// `traceparent` / `tracestate` are forwarded only when a pattern /// names them exactly. A glob such as `"*"` or `"x-*"` is a statement /// about the operator's own headers, not consent to hand a third party diff --git a/crates/aisix-core/src/models/provider_key.rs b/crates/aisix-core/src/models/provider_key.rs index 0161a6076..04e75ec4b 100644 --- a/crates/aisix-core/src/models/provider_key.rs +++ b/crates/aisix-core/src/models/provider_key.rs @@ -373,7 +373,9 @@ pub struct RequestOverrides { /// A header named here reaches the upstream whatever the gateway would /// otherwise do with it. Naming a credential slot — `authorization`, /// `proxy-authorization`, `x-api-key`, `api-key`, `x-goog-api-key`, - /// `cookie` — hands the upstream the caller's own credential in place of the + /// `cookie`, and the AWS SigV4 trio `x-amz-security-token` / + /// `x-amz-date` / `x-amz-content-sha256` — hands the upstream the + /// caller's own credential in place of the /// one this ProviderKey would inject there, never both. That is what lets an /// internal service that already authorizes on the end user's /// `Authorization` keep doing so unchanged. Any OTHER header the diff --git a/crates/aisix-gateway/src/upstream_headers.rs b/crates/aisix-gateway/src/upstream_headers.rs index 3ac6c927a..5fc53af6e 100644 --- a/crates/aisix-gateway/src/upstream_headers.rs +++ b/crates/aisix-gateway/src/upstream_headers.rs @@ -676,6 +676,45 @@ mod tests { assert!(!headers.contains_key("x-goog-api-key"), "got: {headers:?}"); } + /// The AWS SigV4 headers are credential slots on a NON-Bedrock + /// upstream, where nothing else stops them. A Bedrock provider's + /// signer owns all three and drops any supplied value, so this rule is + /// the only thing standing between a `["x-*"]` pattern and the + /// caller's live AWS session token arriving at some other provider. + #[test] + fn the_aws_sigv4_trio_needs_its_own_name_not_a_glob() { + let inbound = client(&[ + ("x-amz-security-token", "FQoGZXIvYXdzE-caller-session"), + ("x-amz-date", "20260903T120000Z"), + ("x-amz-content-sha256", "e3b0c44298fc1c14"), + ("x-trace-id", "operator-own-header"), + ]); + + // A glob reaches the operator's own header and none of the three. + let r = overrides(&[], &["x-*"]); + let mut headers = HeaderMap::new(); + let ctx = UpstreamHeaderContext::from_overrides(Some(&r)).with_client_headers(&inbound); + apply_request_headers(&mut headers, &ctx); + assert_eq!(headers["x-trace-id"], "operator-own-header"); + for name in ["x-amz-security-token", "x-amz-date", "x-amz-content-sha256"] { + assert!(!headers.contains_key(name), "{name} leaked: {headers:?}"); + } + + // Naming one exactly IS the operator's consent, and is honoured — + // this stays a credential slot, not the absolute block it used to + // be, so an upstream that genuinely reads the caller's SigV4 + // headers can still be opted in. + let r = overrides(&[], &["x-amz-security-token"]); + let mut headers = HeaderMap::new(); + let ctx = UpstreamHeaderContext::from_overrides(Some(&r)).with_client_headers(&inbound); + apply_request_headers(&mut headers, &ctx); + assert_eq!( + headers["x-amz-security-token"], "FQoGZXIvYXdzE-caller-session", + "an exactly-named credential slot must still forward", + ); + assert!(!headers.contains_key("x-amz-date"), "got: {headers:?}"); + } + #[test] fn transport_and_gateway_namespaces_are_never_forwarded() { let r = overrides(&[], &["*"]); diff --git a/schemas/resources/passthrough_route.schema.json b/schemas/resources/passthrough_route.schema.json index 264efa280..e84cbe358 100644 --- a/schemas/resources/passthrough_route.schema.json +++ b/schemas/resources/passthrough_route.schema.json @@ -485,7 +485,7 @@ "type": "boolean" }, "forward_client_headers": { - "description": "Inbound client headers forwarded to the upstream even when this route would otherwise strip them, as single-`*` glob patterns matched case-insensitively against the header name (`\"authorization\"`, `\"x-trace-*\"`). Empty — the default — overrides no stripping.\n\nA route forwards the caller's headers by default, so this field only matters for the ones it removes: the ProviderKey's `strip_headers` under `credential_mode: inject`, and the slot the gateway consumed to authenticate the caller. Naming `authorization` under `auth_mode: gateway_key` therefore puts the caller's own credential back on the upstream request in place of the one this route would inject, never both — which is what lets an internal service that already authorizes on the end user's `Authorization` keep doing so unchanged.\n\nA credential slot — `authorization`, `proxy-authorization`, `x-api-key`, `api-key`, `x-goog-api-key`, `cookie` — and `traceparent` / `tracestate` are forwarded only when a pattern names them exactly. A glob such as `\"*\"` or `\"x-*\"` is a statement about the operator's own headers, not consent to hand a third party the caller's credential or to graft the caller's trace onto that party's telemetry, so a broad pattern overrides the rest of the strip set and leaves those alone.\n\nThis route's own `auth_header_name` and `identity_header` are read the same way. Both are slots this route chose rather than ones the gateway owns — under `auth_mode: header_key` the first carries the gateway credential the caller authenticated with, and the second carries an end-user identity this route records and strips — so a glob does not sweep either, and a pattern that names one in full forwards it.\n\nHeaders whose forwarding would break the exchange rather than change who it comes from are stripped whatever the patterns say: `host`, `content-length`, the hop-by-hop headers that describe the caller's own connection, and the gateway's `x-aisix-*` namespace.", + "description": "Inbound client headers forwarded to the upstream even when this route would otherwise strip them, as single-`*` glob patterns matched case-insensitively against the header name (`\"authorization\"`, `\"x-trace-*\"`). Empty — the default — overrides no stripping.\n\nA route forwards the caller's headers by default, so this field only matters for the ones it removes: the ProviderKey's `strip_headers` under `credential_mode: inject`, and the slot the gateway consumed to authenticate the caller. Naming `authorization` under `auth_mode: gateway_key` therefore puts the caller's own credential back on the upstream request in place of the one this route would inject, never both — which is what lets an internal service that already authorizes on the end user's `Authorization` keep doing so unchanged.\n\nA credential slot — `authorization`, `proxy-authorization`, `x-api-key`, `api-key`, `x-goog-api-key`, `cookie`, and the AWS SigV4 trio `x-amz-security-token` / `x-amz-date` / `x-amz-content-sha256` — and `traceparent` / `tracestate` are forwarded only when a pattern names them exactly. A glob such as `\"*\"` or `\"x-*\"` is a statement about the operator's own headers, not consent to hand a third party the caller's credential or to graft the caller's trace onto that party's telemetry, so a broad pattern overrides the rest of the strip set and leaves those alone.\n\nThis route's own `auth_header_name` and `identity_header` are read the same way. Both are slots this route chose rather than ones the gateway owns — under `auth_mode: header_key` the first carries the gateway credential the caller authenticated with, and the second carries an end-user identity this route records and strips — so a glob does not sweep either, and a pattern that names one in full forwards it.\n\nHeaders whose forwarding would break the exchange rather than change who it comes from are stripped whatever the patterns say: `host`, `content-length`, the hop-by-hop headers that describe the caller's own connection, and the gateway's `x-aisix-*` namespace.", "items": { "type": "string" }, diff --git a/schemas/resources/provider_key.schema.json b/schemas/resources/provider_key.schema.json index 21950af7d..7bb8065f2 100644 --- a/schemas/resources/provider_key.schema.json +++ b/schemas/resources/provider_key.schema.json @@ -129,7 +129,7 @@ "type": "object" }, "forward_client_headers": { - "description": "Inbound client headers forwarded to the upstream, as single-`*` glob patterns matched case-insensitively against the header name (`\"anthropic-beta\"`, `\"x-trace-*\"`, `\"authorization\"`). Empty — the default — forwards nothing, which is the behavior of every standard-protocol endpoint before AISIX-Cloud#1167.\n\nA header named here reaches the upstream whatever the gateway would otherwise do with it. Naming a credential slot — `authorization`, `proxy-authorization`, `x-api-key`, `api-key`, `x-goog-api-key`, `cookie` — hands the upstream the caller's own credential in place of the one this ProviderKey would inject there, never both. That is what lets an internal service that already authorizes on the end user's `Authorization` keep doing so unchanged. Any OTHER header the gateway had already set is left alone: it selects how the exchange works, not who it is from.\n\nA credential slot, and `traceparent` / `tracestate`, are forwarded only when a pattern names them exactly — a glob such as `\"*\"` or `\"x-*\"` is a statement about the operator's own headers, not consent to hand a third party the caller's credential or to graft the caller's trace onto that party's telemetry.\n\nTwo cases where a named header still does not reach the upstream. A `default_headers` entry of the same name wins it for every name except a credential slot: both are operator configuration and the static one is the more specific choice, but in a credential slot the forwarded value is precisely the one that was asked for, so it takes the slot from the static entry. And on an AWS Bedrock provider the request signer owns `authorization`, `x-amz-date`, `x-amz-content-sha256`, `x-amz-security-token`, `x-amz-target` and `x-amzn-bedrock-accept`, and drops any supplied value — a value there would not authenticate anyone: it either loses to the signer or breaks the signature.\n\nNaming a credential slot needs a data plane new enough to honor it; an older one refuses those names outright, so the pattern has no effect there rather than a different one.\n\nHeaders whose forwarding would break the exchange rather than change who it comes from are never forwarded whatever the patterns say: `host`, the hop-by-hop headers that describe the caller's own connection, and the gateway's `x-aisix-*` namespace. The headers describing a body this gateway re-serializes or a response shape it parses (`content-type`, `content-length`, `accept`, `anthropic-version`, `x-stainless-*`) are excluded for the same reason. `traceparent` and `tracestate` are forwarded only when a pattern names them exactly — a glob is not read as consent to graft the caller's trace onto the upstream's telemetry.", + "description": "Inbound client headers forwarded to the upstream, as single-`*` glob patterns matched case-insensitively against the header name (`\"anthropic-beta\"`, `\"x-trace-*\"`, `\"authorization\"`). Empty — the default — forwards nothing, which is the behavior of every standard-protocol endpoint before AISIX-Cloud#1167.\n\nA header named here reaches the upstream whatever the gateway would otherwise do with it. Naming a credential slot — `authorization`, `proxy-authorization`, `x-api-key`, `api-key`, `x-goog-api-key`, `cookie`, and the AWS SigV4 trio `x-amz-security-token` / `x-amz-date` / `x-amz-content-sha256` — hands the upstream the caller's own credential in place of the one this ProviderKey would inject there, never both. That is what lets an internal service that already authorizes on the end user's `Authorization` keep doing so unchanged. Any OTHER header the gateway had already set is left alone: it selects how the exchange works, not who it is from.\n\nA credential slot, and `traceparent` / `tracestate`, are forwarded only when a pattern names them exactly — a glob such as `\"*\"` or `\"x-*\"` is a statement about the operator's own headers, not consent to hand a third party the caller's credential or to graft the caller's trace onto that party's telemetry.\n\nTwo cases where a named header still does not reach the upstream. A `default_headers` entry of the same name wins it for every name except a credential slot: both are operator configuration and the static one is the more specific choice, but in a credential slot the forwarded value is precisely the one that was asked for, so it takes the slot from the static entry. And on an AWS Bedrock provider the request signer owns `authorization`, `x-amz-date`, `x-amz-content-sha256`, `x-amz-security-token`, `x-amz-target` and `x-amzn-bedrock-accept`, and drops any supplied value — a value there would not authenticate anyone: it either loses to the signer or breaks the signature.\n\nNaming a credential slot needs a data plane new enough to honor it; an older one refuses those names outright, so the pattern has no effect there rather than a different one.\n\nHeaders whose forwarding would break the exchange rather than change who it comes from are never forwarded whatever the patterns say: `host`, the hop-by-hop headers that describe the caller's own connection, and the gateway's `x-aisix-*` namespace. The headers describing a body this gateway re-serializes or a response shape it parses (`content-type`, `content-length`, `accept`, `anthropic-version`, `x-stainless-*`) are excluded for the same reason. `traceparent` and `tracestate` are forwarded only when a pattern names them exactly — a glob is not read as consent to graft the caller's trace onto the upstream's telemetry.", "items": { "type": "string" }, From 5708a195e438486b8828c347eebcac40bf8c6a4d Mon Sep 17 00:00:00 2001 From: Jarvis Date: Thu, 3 Sep 2026 12:52:06 +0000 Subject: [PATCH 5/5] test(headers): cover the credential-slot glob rule at the e2e layer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review findings on the previous commit. The SigV4 fix was pinned only at the unit layer, while the bug's observable form is an upstream actually receiving the caller's AWS session token. `forward-client-headers-e2e` already drives a ProviderKey configured `X-*` against a real upstream, so the three names join the request it already makes and the assertion says they must not arrive. Verified to fail without the fix, reporting the token's value at the upstream. That fixture had no coverage of a credential slot being refused by a glob at all — it asserted only the positive direction, an exactly-named `x-api-key` forwarding — so this closes a gap wider than the one commit. Also corrects two counts the previous commit left stale: the module header still enumerated six credential slots, and `exact_match_only_with` said "the two" shared names a passthrough route's own slots may take, which is five once the trio is on the shared list. The claim around it is unaffected — those names are all on the shared list, so the rule stays complete — only the number was wrong. --- crates/aisix-core/src/forwarded_headers.rs | 12 +++++++----- .../src/cases/forward-client-headers-e2e.test.ts | 14 ++++++++++++-- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/crates/aisix-core/src/forwarded_headers.rs b/crates/aisix-core/src/forwarded_headers.rs index f29ce0422..52f64816f 100644 --- a/crates/aisix-core/src/forwarded_headers.rs +++ b/crates/aisix-core/src/forwarded_headers.rs @@ -32,8 +32,9 @@ //! does NOT apply this second tier. //! //! Credential slots — `authorization`, `x-api-key`, `api-key`, -//! `x-goog-api-key`, `proxy-authorization`, `cookie` — are in neither tier -//! on purpose. Handing an internal upstream the end user's own credential, +//! `x-goog-api-key`, `proxy-authorization`, `cookie`, and the AWS SigV4 +//! trio `x-amz-security-token` / `x-amz-date` / `x-amz-content-sha256` — +//! are in neither tier on purpose. Handing an internal upstream the end user's own credential, //! in the slot that upstream already reads, is the capability's whole //! point, and the surface that injects a gateway credential into the same //! slot stands aside for it (see each dispatch site). @@ -163,9 +164,10 @@ pub fn exact_match_only(name: &str) -> bool { /// header_key` the gateway credential arrives in the route's /// `auth_header_name`, and `identity_header` carries an end-user identity /// the route promises to record and strip. The route schema forbids MOST -/// of the shared credential names for those two fields, and the two it -/// does allow (`api-key`, `x-goog-api-key`) are on the shared list -/// already — so the union below is what makes the rule complete, and +/// of the shared credential names for those two fields, and the ones it +/// does allow (`api-key`, `x-goog-api-key`, and the SigV4 trio) are on +/// the shared list already — so the union below is what makes the rule +/// complete, and /// without it a `["x-*"]` pattern sweeps in exactly the header the /// gateway just consumed to authenticate /// the caller and relays it upstream, where it can be replayed against diff --git a/tests/e2e/src/cases/forward-client-headers-e2e.test.ts b/tests/e2e/src/cases/forward-client-headers-e2e.test.ts index 1984e2812..bf79989be 100644 --- a/tests/e2e/src/cases/forward-client-headers-e2e.test.ts +++ b/tests/e2e/src/cases/forward-client-headers-e2e.test.ts @@ -681,11 +681,14 @@ describe("forward_client_headers e2e: one capability across every proxy face", ( await chat(FORWARD_MODEL, { // Every one of these MATCHES a configured pattern — `x-*` the - // first three, `trace*` the last — so what stops them is the rule - // under test, not a pattern that failed to fire. + // first three and the SigV4 trio, `trace*` the last — so what stops + // them is the rule under test, not a pattern that failed to fire. "x-aisix-routing-tags": "spoofed-by-caller", "x-stainless-lang": "js", "x-allowed": "yes", + "x-amz-security-token": "FQoGZXIvYXdzE-caller-session", + "x-amz-date": "20260903T120000Z", + "x-amz-content-sha256": "e3b0c44298fc1c14", traceparent: "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01", }).then((r) => bodyOf(r, "the glob case")); @@ -697,5 +700,12 @@ describe("forward_client_headers e2e: one capability across every proxy face", ( // Trace context needs its own name: a glob is not consent to graft the // caller's trace onto the provider's telemetry. expect(seen.headers.traceparent).toBeUndefined(); + // A credential slot needs its own name for the same reason, and this + // is the shape the bug actually took: the caller's live AWS session + // token arriving at an upstream that has no business holding one. + // This upstream is not Bedrock, so nothing downstream would strip it. + for (const name of ["x-amz-security-token", "x-amz-date", "x-amz-content-sha256"]) { + expect(seen.headers[name], `${name} reached the upstream under a glob`).toBeUndefined(); + } }); });