feat(guardrails): add Aliyun text-moderation guardrail kind (#603) - #506
Merged
Conversation
Add a new guardrail kind `aliyun_text_moderation` that calls Aliyun's content-safety guardrail (`TextModerationPlus` on `green-cip.<region>.aliyuncs.com`). The input hook uses `llm_query_moderation`, the output hook `llm_response_moderation`; the DP blocks when the returned `RiskLevel` (none<low<medium<high) reaches the configurable `risk_level_threshold` (default `high`). There is no official Aliyun Rust SDK, so the RPC signature (v1, HMAC-SHA1) is hand-rolled over the shared reqwest client — same approach the codebase already takes for Azure Content Safety. v1 is used over v3 because its canonicalization is unambiguous for RPC-style products and is pinned by a known-vector unit test. Streaming output is moderated incrementally via the windowed StreamOutputPolicy; each window reuses the response's provider request id as Aliyun's sessionId so the chunks of one stream correlate — no proxy or trait change required. Granularity (env/model/api_key/team), input/output hooks, and streaming/non-streaming all reuse the existing guardrail attachment + chain infrastructure.
📝 WalkthroughWalkthroughThis PR implements Aliyun TextModerationPlus as a new guardrail provider. It adds configuration contracts in the core model, extends schema validation, implements the Aliyun v1-signed RPC dispatcher with fail-open/fail-closed policies for both input and output hooks, integrates the runtime builder, and validates the feature end-to-end with comprehensive tests. ChangesAliyun Text Moderation Guardrail
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
tests/e2e/src/cases/guardrail-aliyun-e2e.test.ts (1)
329-362: 💤 Low valueConsider verifying sessionId stability across streaming windows.
The test validates the end-to-end error behavior but doesn't verify that Aliyun receives the same
sessionIdfor all windows of the stream. Givenwindow_size: 16and ~30-char risky content, multiple moderation calls should occur. Checking thataliyun.requests.filter(r => r.service === "llm_response_moderation").every(r => r.sessionId === expectedId)would confirm the correlation feature works end-to-end.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/e2e/src/cases/guardrail-aliyun-e2e.test.ts` around lines 329 - 362, The test "streaming risky output → SSE error event, no [DONE] (windowed)" currently asserts the SSE error and parsed error type but doesn't verify that Aliyun received a stable sessionId across streaming windows; update the test to capture the expected sessionId (e.g., from the proxy request or generated value) and then assert that all moderation requests to the mocked aliyun endpoint (use aliyun.requests.filter(r => r.service === "llm_response_moderation")) have the same sessionId by checking every(r => r.sessionId === expectedId), ensuring you perform this check after the fetch completes and before finishing the test.crates/aisix-guardrails/src/aliyun.rs (1)
159-273: 💤 Low valueConsider refactoring for readability.
The
call()method is 115 lines and handles parameter construction, signing, HTTP request, and response parsing. While the logic is clear, extracting helpers for building signed params or parsing the response could improve maintainability.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/aisix-guardrails/src/aliyun.rs` around lines 159 - 273, The call() method is doing several concerns (service parameter JSON, building the sorted BTreeMap params, signing via sign(), URL-encoding with percent_encode(), constructing the form body, sending the request via self.client.post(...), and parsing AliyunResponse) which makes it long; factor it into small helpers such as build_service_parameters(service, content, session_id) to produce the ServiceParameters JSON, build_sorted_params_map(...) that returns the BTreeMap used for signing, build_form_body(¶ms, &signature) that performs percent-encoding and concatenation, and send_and_handle_response(reqwest::Client, body, timeout) that performs the POST and maps HTTP status to AliyunFailure and returns the parsed AliyunResponse; keep call() as the orchestration layer that calls sign(), these helpers, and then maps AliyunResponse.code into the existing AliyunFailure/Ok branches.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Cargo.toml`:
- Around line 103-104: Update the pinned crate versions for sha1 and hmac in
Cargo.toml to the current releases: change the sha1 entry from "0.10" to
"0.11.0" and the hmac entry from "0.12" to "0.13.0"; after editing the sha1 and
hmac dependency lines, run cargo update (or cargo build/check) to refresh the
lockfile and verify there are no API compatibility issues in modules referencing
those crates (search for uses of the sha1 and hmac crates in your codebase to
fix any minor API changes if needed).
In `@crates/aisix-core/src/models/guardrail.rs`:
- Around line 277-291: The AliyunTextModerationConfig derives Debug while
containing sensitive access_key_secret; implement a custom std::fmt::Debug for
AliyunTextModerationConfig that formats all fields as before but replaces
access_key_secret with a redacted placeholder (e.g., "[REDACTED]") to avoid
leaking secrets; locate the struct AliyunTextModerationConfig and add an impl
std::fmt::Debug block that uses f.debug_struct(...).field(...) for region,
endpoint, access_key_id and other existing fields, and
.field("access_key_secret", &"[REDACTED]") before .finish().
---
Nitpick comments:
In `@crates/aisix-guardrails/src/aliyun.rs`:
- Around line 159-273: The call() method is doing several concerns (service
parameter JSON, building the sorted BTreeMap params, signing via sign(),
URL-encoding with percent_encode(), constructing the form body, sending the
request via self.client.post(...), and parsing AliyunResponse) which makes it
long; factor it into small helpers such as build_service_parameters(service,
content, session_id) to produce the ServiceParameters JSON,
build_sorted_params_map(...) that returns the BTreeMap used for signing,
build_form_body(¶ms, &signature) that performs percent-encoding and
concatenation, and send_and_handle_response(reqwest::Client, body, timeout) that
performs the POST and maps HTTP status to AliyunFailure and returns the parsed
AliyunResponse; keep call() as the orchestration layer that calls sign(), these
helpers, and then maps AliyunResponse.code into the existing AliyunFailure/Ok
branches.
In `@tests/e2e/src/cases/guardrail-aliyun-e2e.test.ts`:
- Around line 329-362: The test "streaming risky output → SSE error event, no
[DONE] (windowed)" currently asserts the SSE error and parsed error type but
doesn't verify that Aliyun received a stable sessionId across streaming windows;
update the test to capture the expected sessionId (e.g., from the proxy request
or generated value) and then assert that all moderation requests to the mocked
aliyun endpoint (use aliyun.requests.filter(r => r.service ===
"llm_response_moderation")) have the same sessionId by checking every(r =>
r.sessionId === expectedId), ensuring you perform this check after the fetch
completes and before finishing the test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 40a92e08-5b87-4e6b-83b5-e4586e52297b
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
Cargo.tomlcrates/aisix-core/src/models/guardrail.rscrates/aisix-core/src/models/mod.rscrates/aisix-core/src/models/schema.rscrates/aisix-guardrails/Cargo.tomlcrates/aisix-guardrails/src/aliyun.rscrates/aisix-guardrails/src/build.rscrates/aisix-guardrails/src/lib.rsdocs/configuration/guardrails.mdschemas/resources/guardrail.schema.jsontests/e2e/src/cases/guardrail-aliyun-e2e.test.ts
) Strengthen the Aliyun streaming E2E to verify every windowed llm_response_moderation call for one response carries the same sessionId (the upstream request id), confirming Aliyun correlates the chunks of a single stream. Per CodeRabbit review on #506.
Contributor
Author
|
Thanks @coderabbitai. Dispositions on the review:
|
|
Tip For best results, initiate chat on the files or code changes.
All good — the PR looks solid. 🐇 |
…603) Ignored-by-default test that exercises the real signer + HTTP + parse against green-cip TextModerationPlus when ALIYUN_AK_ID/ALIYUN_AK_SECRET are set. No credentials are committed.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/aisix-guardrails/src/aliyun.rs (1)
99-101: 💤 Low valueConsider configuring connection timeout on the client.
The reqwest client is built without explicit connection/pool timeouts. While
tokio::time::timeoutwraps the full request incall(), addingconnect_timeout()provides defense-in-depth against DNS or connection establishment hangs.Suggested improvement
let client = reqwest::Client::builder() + .connect_timeout(Duration::from_secs(5)) + .pool_idle_timeout(Duration::from_secs(90)) .build() .expect("reqwest::Client::builder() failed; this should never happen");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/aisix-guardrails/src/aliyun.rs` around lines 99 - 101, The reqwest client in the client initialization path is created without an explicit connection timeout, so update the Client builder in the ali yun client setup to configure a connect_timeout alongside the existing build flow. Use the reqwest::Client::builder() construction in the client creation logic and add a reasonable connect timeout so connection establishment and DNS hangs are bounded even before the tokio::time::timeout in call() applies.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@crates/aisix-guardrails/src/aliyun.rs`:
- Around line 99-101: The reqwest client in the client initialization path is
created without an explicit connection timeout, so update the Client builder in
the ali yun client setup to configure a connect_timeout alongside the existing
build flow. Use the reqwest::Client::builder() construction in the client
creation logic and add a reasonable connect timeout so connection establishment
and DNS hangs are bounded even before the tokio::time::timeout in call()
applies.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a492a10f-013d-43c4-bb90-b0037a4974e8
📒 Files selected for processing (1)
crates/aisix-guardrails/src/aliyun.rs
This was referenced Jun 5, 2026
Closed
12 tasks
This was referenced Jun 30, 2026
Merged
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
2 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.
Problem
A customer uses Aliyun's AI 安全护栏 (AI Guardrails) rather than AWS Bedrock. The guardrail subsystem only shipped
keyword,bedrock, and the two Azure Content Safety kinds. This adds an Aliyun provider so operators can moderate LLM input and output against Aliyun's content-safety service. Resolves api7/AISIX-Cloud#603 (data-plane half).What changed
New guardrail kind
aliyun_text_moderationthat calls Aliyun'sTextModerationPlusaction ongreen-cip.<region>.aliyuncs.com:aisix-core—AliyunTextModerationConfig+GuardrailKind::AliyunTextModerationarm; loader JSON-schema branch; regeneratedschemas/resources/guardrail.schema.json.aisix-guardrails— newaliyun.rsdispatcher behind thealiyun-text-moderationfeature (default-on). Input hook →llm_query_moderation, output hook →llm_response_moderation. Builder wiring inbuild.rs.Behavior
RiskLevel(none<low<medium<high) is blocked when it reaches the configurablerisk_level_threshold(defaulthigh).StreamOutputPolicy; each window reuses the response's provider request id as Aliyun'ssessionId, so the chunks of one stream correlate. No proxy/trait change.fail_open; output defaults fail-closed (output_fail_open=false) so an Aliyun outage cannot release unscanned model output.Signature
There is no official Aliyun Rust SDK. The RPC signature (v1, HMAC-SHA1) is hand-rolled over the shared
reqwestclient — the same approach already used for Azure Content Safety. v1 is chosen over v3 because its canonicalization is unambiguous for RPC-style products and is pinned by a known-vector unit test (openssl-computed reference). Theaccess_key_secretis decrypted by cp-api before kine projection; the DP only holds plaintext in memory and never logs it.Tests
aliyun.rs, wiremock): known-vector signature, percent-encoding, canonical string-to-sign, risk-threshold tuning (low/medium/high), fail-open buckets, app-level error codes, stream-policy mapping.tests/e2e/guardrail-aliyun-e2e.test.ts): realaisix+ etcd + mock upstream + mock green-cip endpoint — risky input → 422content_filterwith upstream never called; benign passes; risky model output → 422 after upstream; streaming risky output → SSEerrorframe with no[DONE]; asserts the output call carries asessionId.Follow-up
Control-plane (cp-api config handler + kine projection of the encrypted secret) and dashboard form are a separate PR against api7/AISIX-Cloud, opened after this merges and a new DP dev image is built.
Summary by CodeRabbit
New Features
Documentation
Tests
Live QA against production Aliyun
Verified the full code path (signer → HTTPS POST → response parse → verdict) against the real
https://green-cip.cn-shanghai.aliyuncs.comendpoint with an active AccessKey, via the env-gatedaliyun::tests::live_smoke_real_endpoint:RiskLevelllm_query_moderation)nonellm_query_moderation)highllm_response_moderation, withsessionId)highThe signature is accepted by Aliyun (no
SignatureDoesNotMatch), both service codes resolve, and the returnedRiskLevelmaps to the expected verdict at the defaulthighthreshold. Credentials are supplied only via environment variables at run time and are not committed.