fix(guardrails): add azure_content_safety to loader JSON schema oneOf - #439

Merged
moonming merged 2 commits into
mainfrom
fix/guardrail-schema-acs
May 29, 2026
Merged

fix(guardrails): add azure_content_safety to loader JSON schema oneOf#439
moonming merged 2 commits into
mainfrom
fix/guardrail-schema-acs

Conversation

@moonming

@moonmingmoonming commented May 29, 2026

Copy link
Copy Markdown
Member

Summary

The DP loader (aisix_etcd) validates each guardrail kine entry against the JSON schema in crates/aisix-core/src/models/schema.rsbefore deserializing into the Rust Guardrail struct. That schema's kind enum and oneOf listed only keyword and bedrock — so every azure_content_safety guardrail was rejected and silently skipped at load time, even though the struct (guardrail.rs) and the check (aisix-guardrails/src/prompt_shield.rs) both already support it on main:

aisix_etcd::loader: schema validation failed; skipping .../guardrails/<id>
{... "kind":"azure_content_safety" ...} is not valid under any of the schemas listed in the 'oneOf' keyword

Net effect: ACS Prompt Shield guardrails never take effect — chats are never blocked.

Fixes#437.

Change

guardrail_schema() in schema.rs:

  • Add azure_content_safety to the top-level kind enum.
  • Add an azure_content_safetyoneOf branch mirroring AzureContentSafetyConfig (guardrail.rs): endpoint + api_key required & non-empty, timeout_ms optional u32 (struct default 5000).

Why unit tests caught it but the existing test didn't

azure_content_safety_kind_parses (guardrail.rs) exercises serde deserialization only. The loader's JSON-schema gate is a separate code path that no test covered. This PR adds three schema-level regression tests:

  • guardrail_azure_content_safety_passes (no timeout → default)
  • guardrail_azure_content_safety_with_timeout_passes
  • guardrail_azure_content_safety_missing_api_key_rejected

Validation

  • cargo test -p aisix-core --lib guardrail → 17 passed (incl. the 3 new).
  • cargo clippy -p aisix-core --lib clean; cargo fmt --check clean.
  • Full cross-repo e2e: surfaced by the AISIX-Cloud P1 ACS suite (api7/AISIX-Cloud#551). With cp-api ready (api7/AISIX-Cloud#567 merged) + this fix, the chain to green is complete; the e2e's DP-mediated 422 + wire-shape assertions validate once a :dev image is built from this.

Discovery context

Found while validating api7/AISIX-Cloud#551 end-to-end against a real stack — the cp-api half accepted/projected the ACS guardrail (201) but the DP dropped it at this schema gate. Last DP-side piece for #379 P1 Prompt Shield.

Test plan

  • cargo test -p aisix-core --lib guardrail (17 pass)
  • cargo clippy / cargo fmt --check clean
  • CI green
  • Independent audit

Summary by CodeRabbit

  • New Features
    • Added Azure Content Safety as a guardrail option with configurable endpoint, API key, and optional timeout.
  • Tests
    • Expanded validation tests covering required fields, optional timeout, and timeout boundary behavior (maximum accepted value and overflow rejection).

Review Change Stack

The DP loader (aisix_etcd) validates each guardrail kine entry against
the JSON schema in models/schema.rs BEFORE deserializing into the Rust
Guardrail struct. That schema's `kind` enum and `oneOf` listed only
`keyword` and `bedrock` — so every `azure_content_safety` guardrail was
rejected and skipped at load time, even though the struct
(guardrail.rs) and the check (aisix-guardrails/prompt_shield.rs) both
support it:
aisix_etcd::loader: schema validation failed; skipping .../guardrails/...
{... "kind":"azure_content_safety" ...} is not valid under any of the
schemas listed in the 'oneOf' keyword
Add the `azure_content_safety` branch, mirroring AzureContentSafetyConfig
in guardrail.rs: `endpoint` + `api_key` required (non-empty), `timeout_ms`
optional (u32, struct default 5000). Also add it to the top-level `kind`
enum.
The existing struct-level test (azure_content_safety_kind_parses)
exercises serde only — it could not catch this because the JSON-schema
gate is a separate code path. Adds three schema-level regression tests.
Fixes#437.
@coderabbitai

coderabbitaiBot commented May 29, 2026

Copy link
Copy Markdown
ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Free

Run ID: 080f16a3-2aaa-4530-9d09-e35385a783c4

📥 Commits

Reviewing files that changed from the base of the PR and between 3afa061 and aa52d93.

📒 Files selected for processing (1)
  • crates/aisix-core/src/models/schema.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • crates/aisix-core/src/models/schema.rs

📝 Walkthrough

Walkthrough

The guardrail JSON schema adds a new azure_content_safety variant: kind now includes it, the schema defines required endpoint and api_key plus an optional bounded timeout_ms, and unit tests cover presence, missing required fields, and timeout_ms boundary behavior.

Changes

Azure Content Safety Guardrail Schema Support

Layer / File(s)Summary
Schema contract and Azure Content Safety variant definition
crates/aisix-core/src/models/schema.rs
The guardrail schema's top-level kind enum is expanded to include azure_content_safety, and a new oneOf branch defines required endpoint and api_key fields and an optional timeout_ms integer with an explicit max bound.
Azure Content Safety guardrail validation tests
crates/aisix-core/src/models/schema.rs
Unit tests validate that azure_content_safety guardrails succeed with required fields only, succeed when optional timeout_ms is provided, fail when required api_key is missing, and enforce timeout_ms boundary limits (max accepted and overflow rejected).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes


Note

🎁 Summarized by CodeRabbit Free

Your organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above.

Comment @coderabbitai help to get the list of available commands and usage tips.

Audit follow-up for #439 (MEDIUM-2 / LOW-1). The #437 bug shipped
precisely because no test guarded the hand-written loader schema
against the struct's field range. Add two boundary tests:
- max_timeout_passes: schema must accept timeout_ms = u32::MAX (the
struct's max). Fails loud if a future edit tightens the schema below
u32::MAX, re-introducing the stricter-than-struct regression class.
- timeout_overflow_rejected: u32::MAX + 1 must be rejected at the gate.
Keeping the schema's timeout_ms upper bound at u32::MAX (not cp-api's
tighter 0/[100,30000]) is intentional: the loader gate must mirror the
struct it deserializes into, never be stricter — cp-api validates the
tighter range on write, so no valid row is dropped.
@moonming

Copy link
Copy Markdown
MemberAuthor

Independent audit: APPROVE (no HIGH). Findings addressed.

The audit compiled a serde repro of the exact struct shape and cross-checked cp-api's real contract (guardrails_azure_cs.go), confirming the loader schema is a faithful, looser-not-stricter mirror of AzureContentSafetyConfig.

  • MEDIUM-2 (boundary regression test) — ✅ addressed. Added guardrail_azure_content_safety_max_timeout_passes (schema must accept timeout_ms = u32::MAX). This is the exact guard whose absence let bug(guardrails): loader JSON schema (schema.rs) missing azure_content_safety oneOf branch → DP drops ACS guardrails #437 ship.
  • LOW-1 (negative bound test) — ✅ addressed. Added guardrail_azure_content_safety_timeout_overflow_rejected (u32::MAX + 1 rejected at the gate).
  • MEDIUM-1 (timeout upper bound = u32::MAX vs cp-api's tighter {0}∪[100,30000]) — ⚖️ justified, kept as-is. The loader gate must mirror the struct it deserializes into, never be stricter; cp-api validates the tighter range on write, so no valid row is ever dropped. Tightening here would re-introduce the stricter-than-struct risk this PR fixes. (The bedrock branch's tight range is the riskier pattern, not the model to follow.)
  • LOW-2 (pre-existing: no keyword schema test) — out of scope; not introduced here. Worth a separate test-coverage issue.

All 5 ACS schema tests + the existing 12 guardrail tests pass locally.

@moonming
moonming merged commit ee4c710 into mainMay 29, 2026
8 checks passed
@moonming
moonming deleted the fix/guardrail-schema-acs branch May 29, 2026 03:20
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(guardrails): loader JSON schema (schema.rs) missing azure_content_safety oneOf branch → DP drops ACS guardrails

1 participant

@moonming
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

fix(guardrails): add azure_content_safety to loader JSON schema oneOf - #439

Merged
moonming merged 2 commits into
mainfrom
fix/guardrail-schema-acs
May 29, 2026
Merged

fix(guardrails): add azure_content_safety to loader JSON schema oneOf#439
moonming merged 2 commits into
mainfrom
fix/guardrail-schema-acs

Conversation

@moonming

@moonmingmoonming commented May 29, 2026

Copy link
Copy Markdown
Member

Summary

The DP loader (aisix_etcd) validates each guardrail kine entry against the JSON schema in crates/aisix-core/src/models/schema.rsbefore deserializing into the Rust Guardrail struct. That schema's kind enum and oneOf listed only keyword and bedrock — so every azure_content_safety guardrail was rejected and silently skipped at load time, even though the struct (guardrail.rs) and the check (aisix-guardrails/src/prompt_shield.rs) both already support it on main:

aisix_etcd::loader: schema validation failed; skipping .../guardrails/<id>
{... "kind":"azure_content_safety" ...} is not valid under any of the schemas listed in the 'oneOf' keyword

Net effect: ACS Prompt Shield guardrails never take effect — chats are never blocked.

Fixes#437.

Change

guardrail_schema() in schema.rs:

  • Add azure_content_safety to the top-level kind enum.
  • Add an azure_content_safetyoneOf branch mirroring AzureContentSafetyConfig (guardrail.rs): endpoint + api_key required & non-empty, timeout_ms optional u32 (struct default 5000).

Why unit tests caught it but the existing test didn't

azure_content_safety_kind_parses (guardrail.rs) exercises serde deserialization only. The loader's JSON-schema gate is a separate code path that no test covered. This PR adds three schema-level regression tests:

  • guardrail_azure_content_safety_passes (no timeout → default)
  • guardrail_azure_content_safety_with_timeout_passes
  • guardrail_azure_content_safety_missing_api_key_rejected

Validation

  • cargo test -p aisix-core --lib guardrail → 17 passed (incl. the 3 new).
  • cargo clippy -p aisix-core --lib clean; cargo fmt --check clean.
  • Full cross-repo e2e: surfaced by the AISIX-Cloud P1 ACS suite (api7/AISIX-Cloud#551). With cp-api ready (api7/AISIX-Cloud#567 merged) + this fix, the chain to green is complete; the e2e's DP-mediated 422 + wire-shape assertions validate once a :dev image is built from this.

Discovery context

Found while validating api7/AISIX-Cloud#551 end-to-end against a real stack — the cp-api half accepted/projected the ACS guardrail (201) but the DP dropped it at this schema gate. Last DP-side piece for #379 P1 Prompt Shield.

Test plan

  • cargo test -p aisix-core --lib guardrail (17 pass)
  • cargo clippy / cargo fmt --check clean
  • CI green
  • Independent audit

Summary by CodeRabbit

  • New Features
    • Added Azure Content Safety as a guardrail option with configurable endpoint, API key, and optional timeout.
  • Tests
    • Expanded validation tests covering required fields, optional timeout, and timeout boundary behavior (maximum accepted value and overflow rejection).

Review Change Stack

The DP loader (aisix_etcd) validates each guardrail kine entry against
the JSON schema in models/schema.rs BEFORE deserializing into the Rust
Guardrail struct. That schema's `kind` enum and `oneOf` listed only
`keyword` and `bedrock` — so every `azure_content_safety` guardrail was
rejected and skipped at load time, even though the struct
(guardrail.rs) and the check (aisix-guardrails/prompt_shield.rs) both
support it:
aisix_etcd::loader: schema validation failed; skipping .../guardrails/...
{... "kind":"azure_content_safety" ...} is not valid under any of the
schemas listed in the 'oneOf' keyword
Add the `azure_content_safety` branch, mirroring AzureContentSafetyConfig
in guardrail.rs: `endpoint` + `api_key` required (non-empty), `timeout_ms`
optional (u32, struct default 5000). Also add it to the top-level `kind`
enum.
The existing struct-level test (azure_content_safety_kind_parses)
exercises serde only — it could not catch this because the JSON-schema
gate is a separate code path. Adds three schema-level regression tests.
Fixes#437.
@coderabbitai

coderabbitaiBot commented May 29, 2026

Copy link
Copy Markdown
ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Free

Run ID: 080f16a3-2aaa-4530-9d09-e35385a783c4

📥 Commits

Reviewing files that changed from the base of the PR and between 3afa061 and aa52d93.

📒 Files selected for processing (1)
  • crates/aisix-core/src/models/schema.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • crates/aisix-core/src/models/schema.rs

📝 Walkthrough

Walkthrough

The guardrail JSON schema adds a new azure_content_safety variant: kind now includes it, the schema defines required endpoint and api_key plus an optional bounded timeout_ms, and unit tests cover presence, missing required fields, and timeout_ms boundary behavior.

Changes

Azure Content Safety Guardrail Schema Support

Layer / File(s)Summary
Schema contract and Azure Content Safety variant definition
crates/aisix-core/src/models/schema.rs
The guardrail schema's top-level kind enum is expanded to include azure_content_safety, and a new oneOf branch defines required endpoint and api_key fields and an optional timeout_ms integer with an explicit max bound.
Azure Content Safety guardrail validation tests
crates/aisix-core/src/models/schema.rs
Unit tests validate that azure_content_safety guardrails succeed with required fields only, succeed when optional timeout_ms is provided, fail when required api_key is missing, and enforce timeout_ms boundary limits (max accepted and overflow rejected).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes


Note

🎁 Summarized by CodeRabbit Free

Your organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above.

Comment @coderabbitai help to get the list of available commands and usage tips.

Audit follow-up for #439 (MEDIUM-2 / LOW-1). The #437 bug shipped
precisely because no test guarded the hand-written loader schema
against the struct's field range. Add two boundary tests:
- max_timeout_passes: schema must accept timeout_ms = u32::MAX (the
struct's max). Fails loud if a future edit tightens the schema below
u32::MAX, re-introducing the stricter-than-struct regression class.
- timeout_overflow_rejected: u32::MAX + 1 must be rejected at the gate.
Keeping the schema's timeout_ms upper bound at u32::MAX (not cp-api's
tighter 0/[100,30000]) is intentional: the loader gate must mirror the
struct it deserializes into, never be stricter — cp-api validates the
tighter range on write, so no valid row is dropped.
@moonming

Copy link
Copy Markdown
MemberAuthor

Independent audit: APPROVE (no HIGH). Findings addressed.

The audit compiled a serde repro of the exact struct shape and cross-checked cp-api's real contract (guardrails_azure_cs.go), confirming the loader schema is a faithful, looser-not-stricter mirror of AzureContentSafetyConfig.

  • MEDIUM-2 (boundary regression test) — ✅ addressed. Added guardrail_azure_content_safety_max_timeout_passes (schema must accept timeout_ms = u32::MAX). This is the exact guard whose absence let bug(guardrails): loader JSON schema (schema.rs) missing azure_content_safety oneOf branch → DP drops ACS guardrails #437 ship.
  • LOW-1 (negative bound test) — ✅ addressed. Added guardrail_azure_content_safety_timeout_overflow_rejected (u32::MAX + 1 rejected at the gate).
  • MEDIUM-1 (timeout upper bound = u32::MAX vs cp-api's tighter {0}∪[100,30000]) — ⚖️ justified, kept as-is. The loader gate must mirror the struct it deserializes into, never be stricter; cp-api validates the tighter range on write, so no valid row is ever dropped. Tightening here would re-introduce the stricter-than-struct risk this PR fixes. (The bedrock branch's tight range is the riskier pattern, not the model to follow.)
  • LOW-2 (pre-existing: no keyword schema test) — out of scope; not introduced here. Worth a separate test-coverage issue.

All 5 ACS schema tests + the existing 12 guardrail tests pass locally.

@moonming
moonming merged commit ee4c710 into mainMay 29, 2026
8 checks passed
@moonming
moonming deleted the fix/guardrail-schema-acs branch May 29, 2026 03:20
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(guardrails): loader JSON schema (schema.rs) missing azure_content_safety oneOf branch → DP drops ACS guardrails

1 participant

@moonming
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(guardrails): add azure_content_safety to loader JSON schema oneOf - #439

Merged
moonming merged 2 commits into
mainfrom
fix/guardrail-schema-acs
May 29, 2026
Merged

fix(guardrails): add azure_content_safety to loader JSON schema oneOf#439
moonming merged 2 commits into
mainfrom
fix/guardrail-schema-acs

Conversation

@moonming

@moonmingmoonming commented May 29, 2026

Copy link
Copy Markdown
Member

Summary

The DP loader (aisix_etcd) validates each guardrail kine entry against the JSON schema in crates/aisix-core/src/models/schema.rsbefore deserializing into the Rust Guardrail struct. That schema's kind enum and oneOf listed only keyword and bedrock — so every azure_content_safety guardrail was rejected and silently skipped at load time, even though the struct (guardrail.rs) and the check (aisix-guardrails/src/prompt_shield.rs) both already support it on main:

aisix_etcd::loader: schema validation failed; skipping .../guardrails/<id>
{... "kind":"azure_content_safety" ...} is not valid under any of the schemas listed in the 'oneOf' keyword

Net effect: ACS Prompt Shield guardrails never take effect — chats are never blocked.

Fixes#437.

Change

guardrail_schema() in schema.rs:

  • Add azure_content_safety to the top-level kind enum.
  • Add an azure_content_safetyoneOf branch mirroring AzureContentSafetyConfig (guardrail.rs): endpoint + api_key required & non-empty, timeout_ms optional u32 (struct default 5000).

Why unit tests caught it but the existing test didn't

azure_content_safety_kind_parses (guardrail.rs) exercises serde deserialization only. The loader's JSON-schema gate is a separate code path that no test covered. This PR adds three schema-level regression tests:

  • guardrail_azure_content_safety_passes (no timeout → default)
  • guardrail_azure_content_safety_with_timeout_passes
  • guardrail_azure_content_safety_missing_api_key_rejected

Validation

  • cargo test -p aisix-core --lib guardrail → 17 passed (incl. the 3 new).
  • cargo clippy -p aisix-core --lib clean; cargo fmt --check clean.
  • Full cross-repo e2e: surfaced by the AISIX-Cloud P1 ACS suite (api7/AISIX-Cloud#551). With cp-api ready (api7/AISIX-Cloud#567 merged) + this fix, the chain to green is complete; the e2e's DP-mediated 422 + wire-shape assertions validate once a :dev image is built from this.

Discovery context

Found while validating api7/AISIX-Cloud#551 end-to-end against a real stack — the cp-api half accepted/projected the ACS guardrail (201) but the DP dropped it at this schema gate. Last DP-side piece for #379 P1 Prompt Shield.

Test plan

  • cargo test -p aisix-core --lib guardrail (17 pass)
  • cargo clippy / cargo fmt --check clean
  • CI green
  • Independent audit

Summary by CodeRabbit

  • New Features
    • Added Azure Content Safety as a guardrail option with configurable endpoint, API key, and optional timeout.
  • Tests
    • Expanded validation tests covering required fields, optional timeout, and timeout boundary behavior (maximum accepted value and overflow rejection).

Review Change Stack

The DP loader (aisix_etcd) validates each guardrail kine entry against
the JSON schema in models/schema.rs BEFORE deserializing into the Rust
Guardrail struct. That schema's `kind` enum and `oneOf` listed only
`keyword` and `bedrock` — so every `azure_content_safety` guardrail was
rejected and skipped at load time, even though the struct
(guardrail.rs) and the check (aisix-guardrails/prompt_shield.rs) both
support it:
aisix_etcd::loader: schema validation failed; skipping .../guardrails/...
{... "kind":"azure_content_safety" ...} is not valid under any of the
schemas listed in the 'oneOf' keyword
Add the `azure_content_safety` branch, mirroring AzureContentSafetyConfig
in guardrail.rs: `endpoint` + `api_key` required (non-empty), `timeout_ms`
optional (u32, struct default 5000). Also add it to the top-level `kind`
enum.
The existing struct-level test (azure_content_safety_kind_parses)
exercises serde only — it could not catch this because the JSON-schema
gate is a separate code path. Adds three schema-level regression tests.
Fixes#437.
@coderabbitai

coderabbitaiBot commented May 29, 2026

Copy link
Copy Markdown
ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Free

Run ID: 080f16a3-2aaa-4530-9d09-e35385a783c4

📥 Commits

Reviewing files that changed from the base of the PR and between 3afa061 and aa52d93.

📒 Files selected for processing (1)
  • crates/aisix-core/src/models/schema.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • crates/aisix-core/src/models/schema.rs

📝 Walkthrough

Walkthrough

The guardrail JSON schema adds a new azure_content_safety variant: kind now includes it, the schema defines required endpoint and api_key plus an optional bounded timeout_ms, and unit tests cover presence, missing required fields, and timeout_ms boundary behavior.

Changes

Azure Content Safety Guardrail Schema Support

Layer / File(s)Summary
Schema contract and Azure Content Safety variant definition
crates/aisix-core/src/models/schema.rs
The guardrail schema's top-level kind enum is expanded to include azure_content_safety, and a new oneOf branch defines required endpoint and api_key fields and an optional timeout_ms integer with an explicit max bound.
Azure Content Safety guardrail validation tests
crates/aisix-core/src/models/schema.rs
Unit tests validate that azure_content_safety guardrails succeed with required fields only, succeed when optional timeout_ms is provided, fail when required api_key is missing, and enforce timeout_ms boundary limits (max accepted and overflow rejected).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes


Note

🎁 Summarized by CodeRabbit Free

Your organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above.

Comment @coderabbitai help to get the list of available commands and usage tips.

Audit follow-up for #439 (MEDIUM-2 / LOW-1). The #437 bug shipped
precisely because no test guarded the hand-written loader schema
against the struct's field range. Add two boundary tests:
- max_timeout_passes: schema must accept timeout_ms = u32::MAX (the
struct's max). Fails loud if a future edit tightens the schema below
u32::MAX, re-introducing the stricter-than-struct regression class.
- timeout_overflow_rejected: u32::MAX + 1 must be rejected at the gate.
Keeping the schema's timeout_ms upper bound at u32::MAX (not cp-api's
tighter 0/[100,30000]) is intentional: the loader gate must mirror the
struct it deserializes into, never be stricter — cp-api validates the
tighter range on write, so no valid row is dropped.
@moonming

Copy link
Copy Markdown
MemberAuthor

Independent audit: APPROVE (no HIGH). Findings addressed.

The audit compiled a serde repro of the exact struct shape and cross-checked cp-api's real contract (guardrails_azure_cs.go), confirming the loader schema is a faithful, looser-not-stricter mirror of AzureContentSafetyConfig.

  • MEDIUM-2 (boundary regression test) — ✅ addressed. Added guardrail_azure_content_safety_max_timeout_passes (schema must accept timeout_ms = u32::MAX). This is the exact guard whose absence let bug(guardrails): loader JSON schema (schema.rs) missing azure_content_safety oneOf branch → DP drops ACS guardrails #437 ship.
  • LOW-1 (negative bound test) — ✅ addressed. Added guardrail_azure_content_safety_timeout_overflow_rejected (u32::MAX + 1 rejected at the gate).
  • MEDIUM-1 (timeout upper bound = u32::MAX vs cp-api's tighter {0}∪[100,30000]) — ⚖️ justified, kept as-is. The loader gate must mirror the struct it deserializes into, never be stricter; cp-api validates the tighter range on write, so no valid row is ever dropped. Tightening here would re-introduce the stricter-than-struct risk this PR fixes. (The bedrock branch's tight range is the riskier pattern, not the model to follow.)
  • LOW-2 (pre-existing: no keyword schema test) — out of scope; not introduced here. Worth a separate test-coverage issue.

All 5 ACS schema tests + the existing 12 guardrail tests pass locally.

@moonming
moonming merged commit ee4c710 into mainMay 29, 2026
8 checks passed
@moonming
moonming deleted the fix/guardrail-schema-acs branch May 29, 2026 03:20
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(guardrails): loader JSON schema (schema.rs) missing azure_content_safety oneOf branch → DP drops ACS guardrails

1 participant

@moonming
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(guardrails): add azure_content_safety to loader JSON schema oneOf - #439

Merged
moonming merged 2 commits into
mainfrom
fix/guardrail-schema-acs
May 29, 2026
Merged

fix(guardrails): add azure_content_safety to loader JSON schema oneOf#439
moonming merged 2 commits into
mainfrom
fix/guardrail-schema-acs

Conversation

@moonming

@moonmingmoonming commented May 29, 2026

Copy link
Copy Markdown
Member

Summary

The DP loader (aisix_etcd) validates each guardrail kine entry against the JSON schema in crates/aisix-core/src/models/schema.rsbefore deserializing into the Rust Guardrail struct. That schema's kind enum and oneOf listed only keyword and bedrock — so every azure_content_safety guardrail was rejected and silently skipped at load time, even though the struct (guardrail.rs) and the check (aisix-guardrails/src/prompt_shield.rs) both already support it on main:

aisix_etcd::loader: schema validation failed; skipping .../guardrails/<id>
{... "kind":"azure_content_safety" ...} is not valid under any of the schemas listed in the 'oneOf' keyword

Net effect: ACS Prompt Shield guardrails never take effect — chats are never blocked.

Fixes#437.

Change

guardrail_schema() in schema.rs:

  • Add azure_content_safety to the top-level kind enum.
  • Add an azure_content_safetyoneOf branch mirroring AzureContentSafetyConfig (guardrail.rs): endpoint + api_key required & non-empty, timeout_ms optional u32 (struct default 5000).

Why unit tests caught it but the existing test didn't

azure_content_safety_kind_parses (guardrail.rs) exercises serde deserialization only. The loader's JSON-schema gate is a separate code path that no test covered. This PR adds three schema-level regression tests:

  • guardrail_azure_content_safety_passes (no timeout → default)
  • guardrail_azure_content_safety_with_timeout_passes
  • guardrail_azure_content_safety_missing_api_key_rejected

Validation

  • cargo test -p aisix-core --lib guardrail → 17 passed (incl. the 3 new).
  • cargo clippy -p aisix-core --lib clean; cargo fmt --check clean.
  • Full cross-repo e2e: surfaced by the AISIX-Cloud P1 ACS suite (api7/AISIX-Cloud#551). With cp-api ready (api7/AISIX-Cloud#567 merged) + this fix, the chain to green is complete; the e2e's DP-mediated 422 + wire-shape assertions validate once a :dev image is built from this.

Discovery context

Found while validating api7/AISIX-Cloud#551 end-to-end against a real stack — the cp-api half accepted/projected the ACS guardrail (201) but the DP dropped it at this schema gate. Last DP-side piece for #379 P1 Prompt Shield.

Test plan

  • cargo test -p aisix-core --lib guardrail (17 pass)
  • cargo clippy / cargo fmt --check clean
  • CI green
  • Independent audit

Summary by CodeRabbit

  • New Features
    • Added Azure Content Safety as a guardrail option with configurable endpoint, API key, and optional timeout.
  • Tests
    • Expanded validation tests covering required fields, optional timeout, and timeout boundary behavior (maximum accepted value and overflow rejection).

Review Change Stack

The DP loader (aisix_etcd) validates each guardrail kine entry against
the JSON schema in models/schema.rs BEFORE deserializing into the Rust
Guardrail struct. That schema's `kind` enum and `oneOf` listed only
`keyword` and `bedrock` — so every `azure_content_safety` guardrail was
rejected and skipped at load time, even though the struct
(guardrail.rs) and the check (aisix-guardrails/prompt_shield.rs) both
support it:
aisix_etcd::loader: schema validation failed; skipping .../guardrails/...
{... "kind":"azure_content_safety" ...} is not valid under any of the
schemas listed in the 'oneOf' keyword
Add the `azure_content_safety` branch, mirroring AzureContentSafetyConfig
in guardrail.rs: `endpoint` + `api_key` required (non-empty), `timeout_ms`
optional (u32, struct default 5000). Also add it to the top-level `kind`
enum.
The existing struct-level test (azure_content_safety_kind_parses)
exercises serde only — it could not catch this because the JSON-schema
gate is a separate code path. Adds three schema-level regression tests.
Fixes#437.
@coderabbitai

coderabbitaiBot commented May 29, 2026

Copy link
Copy Markdown
ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Free

Run ID: 080f16a3-2aaa-4530-9d09-e35385a783c4

📥 Commits

Reviewing files that changed from the base of the PR and between 3afa061 and aa52d93.

📒 Files selected for processing (1)
  • crates/aisix-core/src/models/schema.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • crates/aisix-core/src/models/schema.rs

📝 Walkthrough

Walkthrough

The guardrail JSON schema adds a new azure_content_safety variant: kind now includes it, the schema defines required endpoint and api_key plus an optional bounded timeout_ms, and unit tests cover presence, missing required fields, and timeout_ms boundary behavior.

Changes

Azure Content Safety Guardrail Schema Support

Layer / File(s)Summary
Schema contract and Azure Content Safety variant definition
crates/aisix-core/src/models/schema.rs
The guardrail schema's top-level kind enum is expanded to include azure_content_safety, and a new oneOf branch defines required endpoint and api_key fields and an optional timeout_ms integer with an explicit max bound.
Azure Content Safety guardrail validation tests
crates/aisix-core/src/models/schema.rs
Unit tests validate that azure_content_safety guardrails succeed with required fields only, succeed when optional timeout_ms is provided, fail when required api_key is missing, and enforce timeout_ms boundary limits (max accepted and overflow rejected).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes


Note

🎁 Summarized by CodeRabbit Free

Your organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above.

Comment @coderabbitai help to get the list of available commands and usage tips.

Audit follow-up for #439 (MEDIUM-2 / LOW-1). The #437 bug shipped
precisely because no test guarded the hand-written loader schema
against the struct's field range. Add two boundary tests:
- max_timeout_passes: schema must accept timeout_ms = u32::MAX (the
struct's max). Fails loud if a future edit tightens the schema below
u32::MAX, re-introducing the stricter-than-struct regression class.
- timeout_overflow_rejected: u32::MAX + 1 must be rejected at the gate.
Keeping the schema's timeout_ms upper bound at u32::MAX (not cp-api's
tighter 0/[100,30000]) is intentional: the loader gate must mirror the
struct it deserializes into, never be stricter — cp-api validates the
tighter range on write, so no valid row is dropped.
@moonming

Copy link
Copy Markdown
MemberAuthor

Independent audit: APPROVE (no HIGH). Findings addressed.

The audit compiled a serde repro of the exact struct shape and cross-checked cp-api's real contract (guardrails_azure_cs.go), confirming the loader schema is a faithful, looser-not-stricter mirror of AzureContentSafetyConfig.

  • MEDIUM-2 (boundary regression test) — ✅ addressed. Added guardrail_azure_content_safety_max_timeout_passes (schema must accept timeout_ms = u32::MAX). This is the exact guard whose absence let bug(guardrails): loader JSON schema (schema.rs) missing azure_content_safety oneOf branch → DP drops ACS guardrails #437 ship.
  • LOW-1 (negative bound test) — ✅ addressed. Added guardrail_azure_content_safety_timeout_overflow_rejected (u32::MAX + 1 rejected at the gate).
  • MEDIUM-1 (timeout upper bound = u32::MAX vs cp-api's tighter {0}∪[100,30000]) — ⚖️ justified, kept as-is. The loader gate must mirror the struct it deserializes into, never be stricter; cp-api validates the tighter range on write, so no valid row is ever dropped. Tightening here would re-introduce the stricter-than-struct risk this PR fixes. (The bedrock branch's tight range is the riskier pattern, not the model to follow.)
  • LOW-2 (pre-existing: no keyword schema test) — out of scope; not introduced here. Worth a separate test-coverage issue.

All 5 ACS schema tests + the existing 12 guardrail tests pass locally.

@moonming
moonming merged commit ee4c710 into mainMay 29, 2026
8 checks passed
@moonming
moonming deleted the fix/guardrail-schema-acs branch May 29, 2026 03:20
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(guardrails): loader JSON schema (schema.rs) missing azure_content_safety oneOf branch → DP drops ACS guardrails

1 participant

@moonming
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

fix(guardrails): add azure_content_safety to loader JSON schema oneOf - #439

Merged
moonming merged 2 commits into
mainfrom
fix/guardrail-schema-acs
May 29, 2026
Merged

fix(guardrails): add azure_content_safety to loader JSON schema oneOf#439
moonming merged 2 commits into
mainfrom
fix/guardrail-schema-acs

Conversation

@moonming

@moonmingmoonming commented May 29, 2026

Copy link
Copy Markdown
Member

Summary

The DP loader (aisix_etcd) validates each guardrail kine entry against the JSON schema in crates/aisix-core/src/models/schema.rsbefore deserializing into the Rust Guardrail struct. That schema's kind enum and oneOf listed only keyword and bedrock — so every azure_content_safety guardrail was rejected and silently skipped at load time, even though the struct (guardrail.rs) and the check (aisix-guardrails/src/prompt_shield.rs) both already support it on main:

aisix_etcd::loader: schema validation failed; skipping .../guardrails/<id>
{... "kind":"azure_content_safety" ...} is not valid under any of the schemas listed in the 'oneOf' keyword

Net effect: ACS Prompt Shield guardrails never take effect — chats are never blocked.

Fixes#437.

Change

guardrail_schema() in schema.rs:

  • Add azure_content_safety to the top-level kind enum.
  • Add an azure_content_safetyoneOf branch mirroring AzureContentSafetyConfig (guardrail.rs): endpoint + api_key required & non-empty, timeout_ms optional u32 (struct default 5000).

Why unit tests caught it but the existing test didn't

azure_content_safety_kind_parses (guardrail.rs) exercises serde deserialization only. The loader's JSON-schema gate is a separate code path that no test covered. This PR adds three schema-level regression tests:

  • guardrail_azure_content_safety_passes (no timeout → default)
  • guardrail_azure_content_safety_with_timeout_passes
  • guardrail_azure_content_safety_missing_api_key_rejected

Validation

  • cargo test -p aisix-core --lib guardrail → 17 passed (incl. the 3 new).
  • cargo clippy -p aisix-core --lib clean; cargo fmt --check clean.
  • Full cross-repo e2e: surfaced by the AISIX-Cloud P1 ACS suite (api7/AISIX-Cloud#551). With cp-api ready (api7/AISIX-Cloud#567 merged) + this fix, the chain to green is complete; the e2e's DP-mediated 422 + wire-shape assertions validate once a :dev image is built from this.

Discovery context

Found while validating api7/AISIX-Cloud#551 end-to-end against a real stack — the cp-api half accepted/projected the ACS guardrail (201) but the DP dropped it at this schema gate. Last DP-side piece for #379 P1 Prompt Shield.

Test plan

  • cargo test -p aisix-core --lib guardrail (17 pass)
  • cargo clippy / cargo fmt --check clean
  • CI green
  • Independent audit

Summary by CodeRabbit

  • New Features
    • Added Azure Content Safety as a guardrail option with configurable endpoint, API key, and optional timeout.
  • Tests
    • Expanded validation tests covering required fields, optional timeout, and timeout boundary behavior (maximum accepted value and overflow rejection).

Review Change Stack

The DP loader (aisix_etcd) validates each guardrail kine entry against
the JSON schema in models/schema.rs BEFORE deserializing into the Rust
Guardrail struct. That schema's `kind` enum and `oneOf` listed only
`keyword` and `bedrock` — so every `azure_content_safety` guardrail was
rejected and skipped at load time, even though the struct
(guardrail.rs) and the check (aisix-guardrails/prompt_shield.rs) both
support it:
aisix_etcd::loader: schema validation failed; skipping .../guardrails/...
{... "kind":"azure_content_safety" ...} is not valid under any of the
schemas listed in the 'oneOf' keyword
Add the `azure_content_safety` branch, mirroring AzureContentSafetyConfig
in guardrail.rs: `endpoint` + `api_key` required (non-empty), `timeout_ms`
optional (u32, struct default 5000). Also add it to the top-level `kind`
enum.
The existing struct-level test (azure_content_safety_kind_parses)
exercises serde only — it could not catch this because the JSON-schema
gate is a separate code path. Adds three schema-level regression tests.
Fixes#437.
@coderabbitai

coderabbitaiBot commented May 29, 2026

Copy link
Copy Markdown
ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Free

Run ID: 080f16a3-2aaa-4530-9d09-e35385a783c4

📥 Commits

Reviewing files that changed from the base of the PR and between 3afa061 and aa52d93.

📒 Files selected for processing (1)
  • crates/aisix-core/src/models/schema.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • crates/aisix-core/src/models/schema.rs

📝 Walkthrough

Walkthrough

The guardrail JSON schema adds a new azure_content_safety variant: kind now includes it, the schema defines required endpoint and api_key plus an optional bounded timeout_ms, and unit tests cover presence, missing required fields, and timeout_ms boundary behavior.

Changes

Azure Content Safety Guardrail Schema Support

Layer / File(s)Summary
Schema contract and Azure Content Safety variant definition
crates/aisix-core/src/models/schema.rs
The guardrail schema's top-level kind enum is expanded to include azure_content_safety, and a new oneOf branch defines required endpoint and api_key fields and an optional timeout_ms integer with an explicit max bound.
Azure Content Safety guardrail validation tests
crates/aisix-core/src/models/schema.rs
Unit tests validate that azure_content_safety guardrails succeed with required fields only, succeed when optional timeout_ms is provided, fail when required api_key is missing, and enforce timeout_ms boundary limits (max accepted and overflow rejected).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes


Note

🎁 Summarized by CodeRabbit Free

Your organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above.

Comment @coderabbitai help to get the list of available commands and usage tips.

Audit follow-up for #439 (MEDIUM-2 / LOW-1). The #437 bug shipped
precisely because no test guarded the hand-written loader schema
against the struct's field range. Add two boundary tests:
- max_timeout_passes: schema must accept timeout_ms = u32::MAX (the
struct's max). Fails loud if a future edit tightens the schema below
u32::MAX, re-introducing the stricter-than-struct regression class.
- timeout_overflow_rejected: u32::MAX + 1 must be rejected at the gate.
Keeping the schema's timeout_ms upper bound at u32::MAX (not cp-api's
tighter 0/[100,30000]) is intentional: the loader gate must mirror the
struct it deserializes into, never be stricter — cp-api validates the
tighter range on write, so no valid row is dropped.
@moonming

Copy link
Copy Markdown
MemberAuthor

Independent audit: APPROVE (no HIGH). Findings addressed.

The audit compiled a serde repro of the exact struct shape and cross-checked cp-api's real contract (guardrails_azure_cs.go), confirming the loader schema is a faithful, looser-not-stricter mirror of AzureContentSafetyConfig.

  • MEDIUM-2 (boundary regression test) — ✅ addressed. Added guardrail_azure_content_safety_max_timeout_passes (schema must accept timeout_ms = u32::MAX). This is the exact guard whose absence let bug(guardrails): loader JSON schema (schema.rs) missing azure_content_safety oneOf branch → DP drops ACS guardrails #437 ship.
  • LOW-1 (negative bound test) — ✅ addressed. Added guardrail_azure_content_safety_timeout_overflow_rejected (u32::MAX + 1 rejected at the gate).
  • MEDIUM-1 (timeout upper bound = u32::MAX vs cp-api's tighter {0}∪[100,30000]) — ⚖️ justified, kept as-is. The loader gate must mirror the struct it deserializes into, never be stricter; cp-api validates the tighter range on write, so no valid row is ever dropped. Tightening here would re-introduce the stricter-than-struct risk this PR fixes. (The bedrock branch's tight range is the riskier pattern, not the model to follow.)
  • LOW-2 (pre-existing: no keyword schema test) — out of scope; not introduced here. Worth a separate test-coverage issue.

All 5 ACS schema tests + the existing 12 guardrail tests pass locally.

@moonming
moonming merged commit ee4c710 into mainMay 29, 2026
8 checks passed
@moonming
moonming deleted the fix/guardrail-schema-acs branch May 29, 2026 03:20
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(guardrails): loader JSON schema (schema.rs) missing azure_content_safety oneOf branch → DP drops ACS guardrails

1 participant

@moonming
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(guardrails): add azure_content_safety to loader JSON schema oneOf - #439

Merged
moonming merged 2 commits into
mainfrom
fix/guardrail-schema-acs
May 29, 2026
Merged

fix(guardrails): add azure_content_safety to loader JSON schema oneOf#439
moonming merged 2 commits into
mainfrom
fix/guardrail-schema-acs

Conversation

@moonming

@moonmingmoonming commented May 29, 2026

Copy link
Copy Markdown
Member

Summary

The DP loader (aisix_etcd) validates each guardrail kine entry against the JSON schema in crates/aisix-core/src/models/schema.rsbefore deserializing into the Rust Guardrail struct. That schema's kind enum and oneOf listed only keyword and bedrock — so every azure_content_safety guardrail was rejected and silently skipped at load time, even though the struct (guardrail.rs) and the check (aisix-guardrails/src/prompt_shield.rs) both already support it on main:

aisix_etcd::loader: schema validation failed; skipping .../guardrails/<id>
{... "kind":"azure_content_safety" ...} is not valid under any of the schemas listed in the 'oneOf' keyword

Net effect: ACS Prompt Shield guardrails never take effect — chats are never blocked.

Fixes#437.

Change

guardrail_schema() in schema.rs:

  • Add azure_content_safety to the top-level kind enum.
  • Add an azure_content_safetyoneOf branch mirroring AzureContentSafetyConfig (guardrail.rs): endpoint + api_key required & non-empty, timeout_ms optional u32 (struct default 5000).

Why unit tests caught it but the existing test didn't

azure_content_safety_kind_parses (guardrail.rs) exercises serde deserialization only. The loader's JSON-schema gate is a separate code path that no test covered. This PR adds three schema-level regression tests:

  • guardrail_azure_content_safety_passes (no timeout → default)
  • guardrail_azure_content_safety_with_timeout_passes
  • guardrail_azure_content_safety_missing_api_key_rejected

Validation

  • cargo test -p aisix-core --lib guardrail → 17 passed (incl. the 3 new).
  • cargo clippy -p aisix-core --lib clean; cargo fmt --check clean.
  • Full cross-repo e2e: surfaced by the AISIX-Cloud P1 ACS suite (api7/AISIX-Cloud#551). With cp-api ready (api7/AISIX-Cloud#567 merged) + this fix, the chain to green is complete; the e2e's DP-mediated 422 + wire-shape assertions validate once a :dev image is built from this.

Discovery context

Found while validating api7/AISIX-Cloud#551 end-to-end against a real stack — the cp-api half accepted/projected the ACS guardrail (201) but the DP dropped it at this schema gate. Last DP-side piece for #379 P1 Prompt Shield.

Test plan

  • cargo test -p aisix-core --lib guardrail (17 pass)
  • cargo clippy / cargo fmt --check clean
  • CI green
  • Independent audit

Summary by CodeRabbit

  • New Features
    • Added Azure Content Safety as a guardrail option with configurable endpoint, API key, and optional timeout.
  • Tests
    • Expanded validation tests covering required fields, optional timeout, and timeout boundary behavior (maximum accepted value and overflow rejection).

Review Change Stack

The DP loader (aisix_etcd) validates each guardrail kine entry against
the JSON schema in models/schema.rs BEFORE deserializing into the Rust
Guardrail struct. That schema's `kind` enum and `oneOf` listed only
`keyword` and `bedrock` — so every `azure_content_safety` guardrail was
rejected and skipped at load time, even though the struct
(guardrail.rs) and the check (aisix-guardrails/prompt_shield.rs) both
support it:
aisix_etcd::loader: schema validation failed; skipping .../guardrails/...
{... "kind":"azure_content_safety" ...} is not valid under any of the
schemas listed in the 'oneOf' keyword
Add the `azure_content_safety` branch, mirroring AzureContentSafetyConfig
in guardrail.rs: `endpoint` + `api_key` required (non-empty), `timeout_ms`
optional (u32, struct default 5000). Also add it to the top-level `kind`
enum.
The existing struct-level test (azure_content_safety_kind_parses)
exercises serde only — it could not catch this because the JSON-schema
gate is a separate code path. Adds three schema-level regression tests.
Fixes#437.
@coderabbitai

coderabbitaiBot commented May 29, 2026

Copy link
Copy Markdown
ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Free

Run ID: 080f16a3-2aaa-4530-9d09-e35385a783c4

📥 Commits

Reviewing files that changed from the base of the PR and between 3afa061 and aa52d93.

📒 Files selected for processing (1)
  • crates/aisix-core/src/models/schema.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • crates/aisix-core/src/models/schema.rs

📝 Walkthrough

Walkthrough

The guardrail JSON schema adds a new azure_content_safety variant: kind now includes it, the schema defines required endpoint and api_key plus an optional bounded timeout_ms, and unit tests cover presence, missing required fields, and timeout_ms boundary behavior.

Changes

Azure Content Safety Guardrail Schema Support

Layer / File(s)Summary
Schema contract and Azure Content Safety variant definition
crates/aisix-core/src/models/schema.rs
The guardrail schema's top-level kind enum is expanded to include azure_content_safety, and a new oneOf branch defines required endpoint and api_key fields and an optional timeout_ms integer with an explicit max bound.
Azure Content Safety guardrail validation tests
crates/aisix-core/src/models/schema.rs
Unit tests validate that azure_content_safety guardrails succeed with required fields only, succeed when optional timeout_ms is provided, fail when required api_key is missing, and enforce timeout_ms boundary limits (max accepted and overflow rejected).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes


Note

🎁 Summarized by CodeRabbit Free

Your organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above.

Comment @coderabbitai help to get the list of available commands and usage tips.

Audit follow-up for #439 (MEDIUM-2 / LOW-1). The #437 bug shipped
precisely because no test guarded the hand-written loader schema
against the struct's field range. Add two boundary tests:
- max_timeout_passes: schema must accept timeout_ms = u32::MAX (the
struct's max). Fails loud if a future edit tightens the schema below
u32::MAX, re-introducing the stricter-than-struct regression class.
- timeout_overflow_rejected: u32::MAX + 1 must be rejected at the gate.
Keeping the schema's timeout_ms upper bound at u32::MAX (not cp-api's
tighter 0/[100,30000]) is intentional: the loader gate must mirror the
struct it deserializes into, never be stricter — cp-api validates the
tighter range on write, so no valid row is dropped.
@moonming

Copy link
Copy Markdown
MemberAuthor

Independent audit: APPROVE (no HIGH). Findings addressed.

The audit compiled a serde repro of the exact struct shape and cross-checked cp-api's real contract (guardrails_azure_cs.go), confirming the loader schema is a faithful, looser-not-stricter mirror of AzureContentSafetyConfig.

  • MEDIUM-2 (boundary regression test) — ✅ addressed. Added guardrail_azure_content_safety_max_timeout_passes (schema must accept timeout_ms = u32::MAX). This is the exact guard whose absence let bug(guardrails): loader JSON schema (schema.rs) missing azure_content_safety oneOf branch → DP drops ACS guardrails #437 ship.
  • LOW-1 (negative bound test) — ✅ addressed. Added guardrail_azure_content_safety_timeout_overflow_rejected (u32::MAX + 1 rejected at the gate).
  • MEDIUM-1 (timeout upper bound = u32::MAX vs cp-api's tighter {0}∪[100,30000]) — ⚖️ justified, kept as-is. The loader gate must mirror the struct it deserializes into, never be stricter; cp-api validates the tighter range on write, so no valid row is ever dropped. Tightening here would re-introduce the stricter-than-struct risk this PR fixes. (The bedrock branch's tight range is the riskier pattern, not the model to follow.)
  • LOW-2 (pre-existing: no keyword schema test) — out of scope; not introduced here. Worth a separate test-coverage issue.

All 5 ACS schema tests + the existing 12 guardrail tests pass locally.

@moonming
moonming merged commit ee4c710 into mainMay 29, 2026
8 checks passed
@moonming
moonming deleted the fix/guardrail-schema-acs branch May 29, 2026 03:20
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(guardrails): loader JSON schema (schema.rs) missing azure_content_safety oneOf branch → DP drops ACS guardrails

1 participant

@moonming
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(guardrails): add azure_content_safety to loader JSON schema oneOf - #439

Merged
moonming merged 2 commits into
mainfrom
fix/guardrail-schema-acs
May 29, 2026
Merged

fix(guardrails): add azure_content_safety to loader JSON schema oneOf#439
moonming merged 2 commits into
mainfrom
fix/guardrail-schema-acs

Conversation

@moonming

@moonmingmoonming commented May 29, 2026

Copy link
Copy Markdown
Member

Summary

The DP loader (aisix_etcd) validates each guardrail kine entry against the JSON schema in crates/aisix-core/src/models/schema.rsbefore deserializing into the Rust Guardrail struct. That schema's kind enum and oneOf listed only keyword and bedrock — so every azure_content_safety guardrail was rejected and silently skipped at load time, even though the struct (guardrail.rs) and the check (aisix-guardrails/src/prompt_shield.rs) both already support it on main:

aisix_etcd::loader: schema validation failed; skipping .../guardrails/<id>
{... "kind":"azure_content_safety" ...} is not valid under any of the schemas listed in the 'oneOf' keyword

Net effect: ACS Prompt Shield guardrails never take effect — chats are never blocked.

Fixes#437.

Change

guardrail_schema() in schema.rs:

  • Add azure_content_safety to the top-level kind enum.
  • Add an azure_content_safetyoneOf branch mirroring AzureContentSafetyConfig (guardrail.rs): endpoint + api_key required & non-empty, timeout_ms optional u32 (struct default 5000).

Why unit tests caught it but the existing test didn't

azure_content_safety_kind_parses (guardrail.rs) exercises serde deserialization only. The loader's JSON-schema gate is a separate code path that no test covered. This PR adds three schema-level regression tests:

  • guardrail_azure_content_safety_passes (no timeout → default)
  • guardrail_azure_content_safety_with_timeout_passes
  • guardrail_azure_content_safety_missing_api_key_rejected

Validation

  • cargo test -p aisix-core --lib guardrail → 17 passed (incl. the 3 new).
  • cargo clippy -p aisix-core --lib clean; cargo fmt --check clean.
  • Full cross-repo e2e: surfaced by the AISIX-Cloud P1 ACS suite (api7/AISIX-Cloud#551). With cp-api ready (api7/AISIX-Cloud#567 merged) + this fix, the chain to green is complete; the e2e's DP-mediated 422 + wire-shape assertions validate once a :dev image is built from this.

Discovery context

Found while validating api7/AISIX-Cloud#551 end-to-end against a real stack — the cp-api half accepted/projected the ACS guardrail (201) but the DP dropped it at this schema gate. Last DP-side piece for #379 P1 Prompt Shield.

Test plan

  • cargo test -p aisix-core --lib guardrail (17 pass)
  • cargo clippy / cargo fmt --check clean
  • CI green
  • Independent audit

Summary by CodeRabbit

  • New Features
    • Added Azure Content Safety as a guardrail option with configurable endpoint, API key, and optional timeout.
  • Tests
    • Expanded validation tests covering required fields, optional timeout, and timeout boundary behavior (maximum accepted value and overflow rejection).

Review Change Stack

The DP loader (aisix_etcd) validates each guardrail kine entry against
the JSON schema in models/schema.rs BEFORE deserializing into the Rust
Guardrail struct. That schema's `kind` enum and `oneOf` listed only
`keyword` and `bedrock` — so every `azure_content_safety` guardrail was
rejected and skipped at load time, even though the struct
(guardrail.rs) and the check (aisix-guardrails/prompt_shield.rs) both
support it:
aisix_etcd::loader: schema validation failed; skipping .../guardrails/...
{... "kind":"azure_content_safety" ...} is not valid under any of the
schemas listed in the 'oneOf' keyword
Add the `azure_content_safety` branch, mirroring AzureContentSafetyConfig
in guardrail.rs: `endpoint` + `api_key` required (non-empty), `timeout_ms`
optional (u32, struct default 5000). Also add it to the top-level `kind`
enum.
The existing struct-level test (azure_content_safety_kind_parses)
exercises serde only — it could not catch this because the JSON-schema
gate is a separate code path. Adds three schema-level regression tests.
Fixes#437.
@coderabbitai

coderabbitaiBot commented May 29, 2026

Copy link
Copy Markdown
ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Free

Run ID: 080f16a3-2aaa-4530-9d09-e35385a783c4

📥 Commits

Reviewing files that changed from the base of the PR and between 3afa061 and aa52d93.

📒 Files selected for processing (1)
  • crates/aisix-core/src/models/schema.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • crates/aisix-core/src/models/schema.rs

📝 Walkthrough

Walkthrough

The guardrail JSON schema adds a new azure_content_safety variant: kind now includes it, the schema defines required endpoint and api_key plus an optional bounded timeout_ms, and unit tests cover presence, missing required fields, and timeout_ms boundary behavior.

Changes

Azure Content Safety Guardrail Schema Support

Layer / File(s)Summary
Schema contract and Azure Content Safety variant definition
crates/aisix-core/src/models/schema.rs
The guardrail schema's top-level kind enum is expanded to include azure_content_safety, and a new oneOf branch defines required endpoint and api_key fields and an optional timeout_ms integer with an explicit max bound.
Azure Content Safety guardrail validation tests
crates/aisix-core/src/models/schema.rs
Unit tests validate that azure_content_safety guardrails succeed with required fields only, succeed when optional timeout_ms is provided, fail when required api_key is missing, and enforce timeout_ms boundary limits (max accepted and overflow rejected).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes


Note

🎁 Summarized by CodeRabbit Free

Your organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above.

Comment @coderabbitai help to get the list of available commands and usage tips.

Audit follow-up for #439 (MEDIUM-2 / LOW-1). The #437 bug shipped
precisely because no test guarded the hand-written loader schema
against the struct's field range. Add two boundary tests:
- max_timeout_passes: schema must accept timeout_ms = u32::MAX (the
struct's max). Fails loud if a future edit tightens the schema below
u32::MAX, re-introducing the stricter-than-struct regression class.
- timeout_overflow_rejected: u32::MAX + 1 must be rejected at the gate.
Keeping the schema's timeout_ms upper bound at u32::MAX (not cp-api's
tighter 0/[100,30000]) is intentional: the loader gate must mirror the
struct it deserializes into, never be stricter — cp-api validates the
tighter range on write, so no valid row is dropped.
@moonming

Copy link
Copy Markdown
MemberAuthor

Independent audit: APPROVE (no HIGH). Findings addressed.

The audit compiled a serde repro of the exact struct shape and cross-checked cp-api's real contract (guardrails_azure_cs.go), confirming the loader schema is a faithful, looser-not-stricter mirror of AzureContentSafetyConfig.

  • MEDIUM-2 (boundary regression test) — ✅ addressed. Added guardrail_azure_content_safety_max_timeout_passes (schema must accept timeout_ms = u32::MAX). This is the exact guard whose absence let bug(guardrails): loader JSON schema (schema.rs) missing azure_content_safety oneOf branch → DP drops ACS guardrails #437 ship.
  • LOW-1 (negative bound test) — ✅ addressed. Added guardrail_azure_content_safety_timeout_overflow_rejected (u32::MAX + 1 rejected at the gate).
  • MEDIUM-1 (timeout upper bound = u32::MAX vs cp-api's tighter {0}∪[100,30000]) — ⚖️ justified, kept as-is. The loader gate must mirror the struct it deserializes into, never be stricter; cp-api validates the tighter range on write, so no valid row is ever dropped. Tightening here would re-introduce the stricter-than-struct risk this PR fixes. (The bedrock branch's tight range is the riskier pattern, not the model to follow.)
  • LOW-2 (pre-existing: no keyword schema test) — out of scope; not introduced here. Worth a separate test-coverage issue.

All 5 ACS schema tests + the existing 12 guardrail tests pass locally.

@moonming
moonming merged commit ee4c710 into mainMay 29, 2026
8 checks passed
@moonming
moonming deleted the fix/guardrail-schema-acs branch May 29, 2026 03:20
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(guardrails): loader JSON schema (schema.rs) missing azure_content_safety oneOf branch → DP drops ACS guardrails

1 participant

@moonming
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

fix(guardrails): add azure_content_safety to loader JSON schema oneOf - #439

Merged
moonming merged 2 commits into
mainfrom
fix/guardrail-schema-acs
May 29, 2026
Merged

fix(guardrails): add azure_content_safety to loader JSON schema oneOf#439
moonming merged 2 commits into
mainfrom
fix/guardrail-schema-acs

Conversation

@moonming

@moonmingmoonming commented May 29, 2026

Copy link
Copy Markdown
Member

Summary

The DP loader (aisix_etcd) validates each guardrail kine entry against the JSON schema in crates/aisix-core/src/models/schema.rsbefore deserializing into the Rust Guardrail struct. That schema's kind enum and oneOf listed only keyword and bedrock — so every azure_content_safety guardrail was rejected and silently skipped at load time, even though the struct (guardrail.rs) and the check (aisix-guardrails/src/prompt_shield.rs) both already support it on main:

aisix_etcd::loader: schema validation failed; skipping .../guardrails/<id>
{... "kind":"azure_content_safety" ...} is not valid under any of the schemas listed in the 'oneOf' keyword

Net effect: ACS Prompt Shield guardrails never take effect — chats are never blocked.

Fixes#437.

Change

guardrail_schema() in schema.rs:

  • Add azure_content_safety to the top-level kind enum.
  • Add an azure_content_safetyoneOf branch mirroring AzureContentSafetyConfig (guardrail.rs): endpoint + api_key required & non-empty, timeout_ms optional u32 (struct default 5000).

Why unit tests caught it but the existing test didn't

azure_content_safety_kind_parses (guardrail.rs) exercises serde deserialization only. The loader's JSON-schema gate is a separate code path that no test covered. This PR adds three schema-level regression tests:

  • guardrail_azure_content_safety_passes (no timeout → default)
  • guardrail_azure_content_safety_with_timeout_passes
  • guardrail_azure_content_safety_missing_api_key_rejected

Validation

  • cargo test -p aisix-core --lib guardrail → 17 passed (incl. the 3 new).
  • cargo clippy -p aisix-core --lib clean; cargo fmt --check clean.
  • Full cross-repo e2e: surfaced by the AISIX-Cloud P1 ACS suite (api7/AISIX-Cloud#551). With cp-api ready (api7/AISIX-Cloud#567 merged) + this fix, the chain to green is complete; the e2e's DP-mediated 422 + wire-shape assertions validate once a :dev image is built from this.

Discovery context

Found while validating api7/AISIX-Cloud#551 end-to-end against a real stack — the cp-api half accepted/projected the ACS guardrail (201) but the DP dropped it at this schema gate. Last DP-side piece for #379 P1 Prompt Shield.

Test plan

  • cargo test -p aisix-core --lib guardrail (17 pass)
  • cargo clippy / cargo fmt --check clean
  • CI green
  • Independent audit

Summary by CodeRabbit

  • New Features
    • Added Azure Content Safety as a guardrail option with configurable endpoint, API key, and optional timeout.
  • Tests
    • Expanded validation tests covering required fields, optional timeout, and timeout boundary behavior (maximum accepted value and overflow rejection).

Review Change Stack

The DP loader (aisix_etcd) validates each guardrail kine entry against
the JSON schema in models/schema.rs BEFORE deserializing into the Rust
Guardrail struct. That schema's `kind` enum and `oneOf` listed only
`keyword` and `bedrock` — so every `azure_content_safety` guardrail was
rejected and skipped at load time, even though the struct
(guardrail.rs) and the check (aisix-guardrails/prompt_shield.rs) both
support it:
aisix_etcd::loader: schema validation failed; skipping .../guardrails/...
{... "kind":"azure_content_safety" ...} is not valid under any of the
schemas listed in the 'oneOf' keyword
Add the `azure_content_safety` branch, mirroring AzureContentSafetyConfig
in guardrail.rs: `endpoint` + `api_key` required (non-empty), `timeout_ms`
optional (u32, struct default 5000). Also add it to the top-level `kind`
enum.
The existing struct-level test (azure_content_safety_kind_parses)
exercises serde only — it could not catch this because the JSON-schema
gate is a separate code path. Adds three schema-level regression tests.
Fixes#437.
@coderabbitai

coderabbitaiBot commented May 29, 2026

Copy link
Copy Markdown
ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Free

Run ID: 080f16a3-2aaa-4530-9d09-e35385a783c4

📥 Commits

Reviewing files that changed from the base of the PR and between 3afa061 and aa52d93.

📒 Files selected for processing (1)
  • crates/aisix-core/src/models/schema.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • crates/aisix-core/src/models/schema.rs

📝 Walkthrough

Walkthrough

The guardrail JSON schema adds a new azure_content_safety variant: kind now includes it, the schema defines required endpoint and api_key plus an optional bounded timeout_ms, and unit tests cover presence, missing required fields, and timeout_ms boundary behavior.

Changes

Azure Content Safety Guardrail Schema Support

Layer / File(s)Summary
Schema contract and Azure Content Safety variant definition
crates/aisix-core/src/models/schema.rs
The guardrail schema's top-level kind enum is expanded to include azure_content_safety, and a new oneOf branch defines required endpoint and api_key fields and an optional timeout_ms integer with an explicit max bound.
Azure Content Safety guardrail validation tests
crates/aisix-core/src/models/schema.rs
Unit tests validate that azure_content_safety guardrails succeed with required fields only, succeed when optional timeout_ms is provided, fail when required api_key is missing, and enforce timeout_ms boundary limits (max accepted and overflow rejected).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes


Note

🎁 Summarized by CodeRabbit Free

Your organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above.

Comment @coderabbitai help to get the list of available commands and usage tips.

Audit follow-up for #439 (MEDIUM-2 / LOW-1). The #437 bug shipped
precisely because no test guarded the hand-written loader schema
against the struct's field range. Add two boundary tests:
- max_timeout_passes: schema must accept timeout_ms = u32::MAX (the
struct's max). Fails loud if a future edit tightens the schema below
u32::MAX, re-introducing the stricter-than-struct regression class.
- timeout_overflow_rejected: u32::MAX + 1 must be rejected at the gate.
Keeping the schema's timeout_ms upper bound at u32::MAX (not cp-api's
tighter 0/[100,30000]) is intentional: the loader gate must mirror the
struct it deserializes into, never be stricter — cp-api validates the
tighter range on write, so no valid row is dropped.
@moonming

Copy link
Copy Markdown
MemberAuthor

Independent audit: APPROVE (no HIGH). Findings addressed.

The audit compiled a serde repro of the exact struct shape and cross-checked cp-api's real contract (guardrails_azure_cs.go), confirming the loader schema is a faithful, looser-not-stricter mirror of AzureContentSafetyConfig.

  • MEDIUM-2 (boundary regression test) — ✅ addressed. Added guardrail_azure_content_safety_max_timeout_passes (schema must accept timeout_ms = u32::MAX). This is the exact guard whose absence let bug(guardrails): loader JSON schema (schema.rs) missing azure_content_safety oneOf branch → DP drops ACS guardrails #437 ship.
  • LOW-1 (negative bound test) — ✅ addressed. Added guardrail_azure_content_safety_timeout_overflow_rejected (u32::MAX + 1 rejected at the gate).
  • MEDIUM-1 (timeout upper bound = u32::MAX vs cp-api's tighter {0}∪[100,30000]) — ⚖️ justified, kept as-is. The loader gate must mirror the struct it deserializes into, never be stricter; cp-api validates the tighter range on write, so no valid row is ever dropped. Tightening here would re-introduce the stricter-than-struct risk this PR fixes. (The bedrock branch's tight range is the riskier pattern, not the model to follow.)
  • LOW-2 (pre-existing: no keyword schema test) — out of scope; not introduced here. Worth a separate test-coverage issue.

All 5 ACS schema tests + the existing 12 guardrail tests pass locally.

@moonming
moonming merged commit ee4c710 into mainMay 29, 2026
8 checks passed
@moonming
moonming deleted the fix/guardrail-schema-acs branch May 29, 2026 03:20
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(guardrails): loader JSON schema (schema.rs) missing azure_content_safety oneOf branch → DP drops ACS guardrails

1 participant

@moonming