feat(guardrails): Lakera, Presidio, OpenAI Moderation kinds (3-pack) - #730
Conversation
Three remote guardrail kinds: lakera (Guard /v2/guard — injection/ jailbreak blocks, PII-only detections mask via payload offsets), openai_moderation (block on flagged, optional per-category score thresholds), presidio (self-hosted analyze->anonymize, per-entity mask/block, selectable replace/mask/hash/redact operator). Lakera and Presidio moderate via the segment pass so masks write back positionally on request and response including streaming (BufferFull hold-back); the blob path maps maskable outcomes to Block (the bedrock ANONYMIZE contract). Monitor mode rides the platform enforcement_mode. Regenerated guardrail.schema.json; heartbeat supported_kinds reports the new kinds; openapi variant titles + kind descriptions added.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 41 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAdds three new guardrail integrations—Lakera Guard, OpenAI Moderation, and Presidio—including new config types, GuardrailKind variants, feature-gated HTTP dispatcher implementations, build wiring, JSON schema and OpenAPI documentation updates, heartbeat test updates, and corresponding E2E test suites. ChangesNew Guardrail Providers
Estimated code review effort: 4 (Complex) | ~75 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
tests/e2e/src/cases/guardrail-openai-moderation-e2e.test.ts (1)
256-323: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffTests 4 and 5 share and mutate the same guardrail, creating an order dependency.
Test 4 (
enforcement_mode=monitor) and test 5 (category threshold mode) bothPUTto the same sharedguardrailId, each building on the config left behind by the previous test. If test execution order changes (e.g.,test.concurrent, filtering, reordering) or test 4 fails/skips, test 5 starts from an unexpected baseline instead of a known-clean state.Consider giving each test its own guardrail (via
admin.createGuardrail/POST) rather than reusing and mutating a single shared one across the suite.As per path instructions,
**/*.{test,spec,e2e}.{js,ts,jsx,tsx}should "Avoid explicit dependencies between tests and hidden execution order assumptions."🤖 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-openai-moderation-e2e.test.ts` around lines 256 - 323, Tests using the shared guardrailId are order-dependent because both the enforcement_mode=monitor and category threshold mode cases mutate the same guardrail state. Update these tests to create and use their own guardrail instance via admin.createGuardrail/POST (or an equivalent isolated setup) instead of reusing the shared guardrailId, and ensure each test configures its guardrail from a known-clean baseline before calling waitConfigPropagation or client().chat.completions.create.Source: Path instructions
crates/aisix-core/src/models/guardrail.rs (2)
459-461: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInternal shorthand in doc comments (CP/DP/kine-projection).
Both
LakeraConfigandOpenaiModerationConfigstruct doc comments use internal shorthand — "The CP (cp-api) decrypts the envelope-encryptedapi_keyat kine-projection time so the DP always holds plaintext in memory."PresidioConfig's equivalent doc (lines 572-583) correctly avoids this by spelling things out ("No vendor secret — both URLs point at customer-run containers"). Please rewrite these two comments similarly, withoutCP,cp-api,kine-projection, orDP.As per coding guidelines:
crates/aisix-core/src/models/**/*.rs: "write descriptions as public API reference text, avoid internal shorthand (such as DP, CP, kine row, wire shape, mock server, bridge dispatch, or issue-only context)".✏️ Suggested rewording
-/// The CP (cp-api) decrypts the envelope-encrypted `api_key` at kine- -/// projection time so the DP always holds plaintext in memory. The key -/// is never logged. +/// The control plane decrypts the envelope-encrypted `api_key` before this +/// config is projected to the data plane, which always holds plaintext in +/// memory only. The key is never logged.Apply the equivalent change to
OpenaiModerationConfig's doc comment.Also applies to: 511-513
🤖 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-core/src/models/guardrail.rs` around lines 459 - 461, Rewrite the doc comments on LakeraConfig and OpenaiModerationConfig in guardrail.rs to use public API language rather than internal shorthand; replace references to CP, cp-api, kine-projection, and DP with a plain explanation that the service decrypts the envelope-encrypted api_key during configuration handling so the secret is held in memory only as needed and is never logged. Keep the meaning aligned with PresidioConfig’s comment style, and update both struct comments consistently so they read as external-facing reference text.Source: Coding guidelines
531-537: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winNo bounds validation on
category_thresholdsvalues.Unlike
PresidioConfig::score_threshold, which is constrained with#[schemars(range(min = 0.0, max = 1.0))](line 617),category_thresholdsvalues have no range constraint. A threshold outside[0.0, 1.0](e.g. negative or >1) would produce confusing or unintended enforcement (a category that never blocks, or one interpreted incorrectly downstream). Consider validating these at build time (similar toPiiAction::parsevalidation inbuild.rs) or documenting the expected range.🤖 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-core/src/models/guardrail.rs` around lines 531 - 537, The category threshold map in guardrail.rs lacks value bounds, so invalid scores can slip through. Update the GuardrailConfig::category_thresholds definition to enforce or validate each threshold stays within 0.0 to 1.0, using the same style as PresidioConfig::score_threshold or the build-time validation approach used by PiiAction::parse. Keep the fix localized around category_thresholds and ensure invalid values are rejected or clearly documented before enforcement logic uses them.tests/e2e/src/cases/guardrail-presidio-e2e.test.ts (3)
145-147: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd error handling to
server.listen.If
listenfails (e.g. a TOCTOU port race right afterpickFreePort()), the promise never resolves or rejects, hanging the test run instead of failing fast.🔧 Proposed fix
const port = await pickFreePort(); - await new Promise<void>((resolve) => server.listen(port, "127.0.0.1", resolve)); + await new Promise<void>((resolve, reject) => { + server.once("error", reject); + server.listen(port, "127.0.0.1", resolve); + });As per coding guidelines, "Every function return value must be checked for errors (if applicable); errors must be properly handled, not ignored or silently swallowed."
🤖 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-presidio-e2e.test.ts` around lines 145 - 147, The `server.listen` call in the e2e test setup is only wired to resolve, so a bind failure can leave the promise hanging instead of failing the test. Update the `new Promise` around `server.listen` to also handle the error path for the `server` instance, and ensure the surrounding setup logic rejects or throws immediately when listen fails. Use the existing `pickFreePort()` and `server.listen` block as the location to add proper error handling.Source: Coding guidelines
168-184: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHidden execution-order dependency: the "hash operator" test mutates the shared guardrail created in
beforeAll.The
redact,block, andstreamingtests all rely on the guardrail's originaloperator: "replace"config. The final test PUTs the sameguardrailIdto switch it to"hash"(Line 383-387). This makes the suite implicitly order-dependent — inserting a new test after "streaming output" but before "hash operator", or reordering tests, would silently break assumptions about the active operator without any test declaring that dependency.Prefer creating a second, independent guardrail (with its own name/id) for the hash-operator test rather than mutating the shared one.
As per coding guidelines, "Avoid explicit dependencies between tests and hidden execution order assumptions."
Also applies to: 378-413
🤖 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-presidio-e2e.test.ts` around lines 168 - 184, Create a separate guardrail instance for the hash-operator scenario instead of updating the shared one created in beforeAll; the current guardrailBody helper and shared guardrailId are reused by redact/block/streaming tests, so mutating that same Presidio guardrail in the hash-operator test introduces hidden test-order coupling. Add a second independently created guardrail (with its own name/id) for the hash test and use that specific identifier there, leaving the original guardrail untouched for the other tests.Source: Coding guidelines
159-414: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCoverage gaps:
mask/redactoperators andscore_thresholdboundary are untested.Only
replaceandhashoperators (Line 260, 386) are exercised, plus no test verifies thescore_thresholdboundary (e.g. an entity whose score falls below the configured threshold should not be flagged/masked).As per coding guidelines, "Tests must cover boundary cases (empty values, min/max), invalid inputs, combination scenarios, and extreme cases (high load, failures)."
🤖 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-presidio-e2e.test.ts` around lines 159 - 414, The Presidio e2e suite only covers the replace and hash operator paths, so add coverage in the existing guardrail-presidio-e2e.describe block for the mask/redact behavior and for the score_threshold boundary. Extend the guardrailBody/operator-driven setup or add focused tests that create a guardrail with operator set to mask/redact and verify both request and response redaction via the existing client(), upstream, and presidio mocks. Also add a boundary test that uses a low-confidence entity score below score_threshold and asserts it is not flagged or masked, using AdminClient and the same guardrail provisioning flow so the new cases stay aligned with the current test helpers.Source: Coding guidelines
🤖 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 `@tests/e2e/src/cases/guardrail-lakera-e2e.test.ts`:
- Around line 199-292: The later e2e cases currently assume the first test has
already waited for guardrail propagation, which creates hidden ordering
dependencies. Add the same readiness check used by waitConfigPropagation in the
clean prompt, PII masking, and fail_open tests before calling
client().chat.completions.create, so each test independently waits for the
Lakera guardrail to be live and no longer depends on injection phrase test
execution order.
---
Nitpick comments:
In `@crates/aisix-core/src/models/guardrail.rs`:
- Around line 459-461: Rewrite the doc comments on LakeraConfig and
OpenaiModerationConfig in guardrail.rs to use public API language rather than
internal shorthand; replace references to CP, cp-api, kine-projection, and DP
with a plain explanation that the service decrypts the envelope-encrypted
api_key during configuration handling so the secret is held in memory only as
needed and is never logged. Keep the meaning aligned with PresidioConfig’s
comment style, and update both struct comments consistently so they read as
external-facing reference text.
- Around line 531-537: The category threshold map in guardrail.rs lacks value
bounds, so invalid scores can slip through. Update the
GuardrailConfig::category_thresholds definition to enforce or validate each
threshold stays within 0.0 to 1.0, using the same style as
PresidioConfig::score_threshold or the build-time validation approach used by
PiiAction::parse. Keep the fix localized around category_thresholds and ensure
invalid values are rejected or clearly documented before enforcement logic uses
them.
In `@tests/e2e/src/cases/guardrail-openai-moderation-e2e.test.ts`:
- Around line 256-323: Tests using the shared guardrailId are order-dependent
because both the enforcement_mode=monitor and category threshold mode cases
mutate the same guardrail state. Update these tests to create and use their own
guardrail instance via admin.createGuardrail/POST (or an equivalent isolated
setup) instead of reusing the shared guardrailId, and ensure each test
configures its guardrail from a known-clean baseline before calling
waitConfigPropagation or client().chat.completions.create.
In `@tests/e2e/src/cases/guardrail-presidio-e2e.test.ts`:
- Around line 145-147: The `server.listen` call in the e2e test setup is only
wired to resolve, so a bind failure can leave the promise hanging instead of
failing the test. Update the `new Promise` around `server.listen` to also handle
the error path for the `server` instance, and ensure the surrounding setup logic
rejects or throws immediately when listen fails. Use the existing
`pickFreePort()` and `server.listen` block as the location to add proper error
handling.
- Around line 168-184: Create a separate guardrail instance for the
hash-operator scenario instead of updating the shared one created in beforeAll;
the current guardrailBody helper and shared guardrailId are reused by
redact/block/streaming tests, so mutating that same Presidio guardrail in the
hash-operator test introduces hidden test-order coupling. Add a second
independently created guardrail (with its own name/id) for the hash test and use
that specific identifier there, leaving the original guardrail untouched for the
other tests.
- Around line 159-414: The Presidio e2e suite only covers the replace and hash
operator paths, so add coverage in the existing guardrail-presidio-e2e.describe
block for the mask/redact behavior and for the score_threshold boundary. Extend
the guardrailBody/operator-driven setup or add focused tests that create a
guardrail with operator set to mask/redact and verify both request and response
redaction via the existing client(), upstream, and presidio mocks. Also add a
boundary test that uses a low-confidence entity score below score_threshold and
asserts it is not flagged or masked, using AdminClient and the same guardrail
provisioning flow so the new cases stay aligned with the current test helpers.
🪄 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: c525fb04-ab59-4b8a-bf57-15dc91094077
📒 Files selected for processing (14)
crates/aisix-admin/src/openapi.rscrates/aisix-core/src/models/guardrail.rscrates/aisix-core/src/models/mod.rscrates/aisix-guardrails/Cargo.tomlcrates/aisix-guardrails/src/build.rscrates/aisix-guardrails/src/lakera.rscrates/aisix-guardrails/src/lib.rscrates/aisix-guardrails/src/openai_moderation.rscrates/aisix-guardrails/src/presidio.rscrates/aisix-server/src/heartbeat.rsschemas/resources/guardrail.schema.jsontests/e2e/src/cases/guardrail-lakera-e2e.test.tstests/e2e/src/cases/guardrail-openai-moderation-e2e.test.tstests/e2e/src/cases/guardrail-presidio-e2e.test.ts
…, threshold bounds - each e2e test now waits for guardrail propagation itself (no hidden execution-order dependency); presidio spec also asserts score_threshold forwarding - mock servers reject on listen error instead of hanging - openai_moderation category_thresholds outside 0..=1 reject the row at build time (+ test) - de-jargon LakeraConfig/OpenaiModerationConfig struct docs
What
The three "first-class" remote guardrail kinds from #52:
lakera— Lakera Guard (POST {endpoint}/v2/guard, Bearer key, optionalproject_id). Prompt-injection / jailbreak / content detections block; detections that are exclusivelypii/*are masked in place using the span offsets Lakera returns ([MASKED <TYPE>]) and the request continues.openai_moderation— OpenAI Moderation API (POST {endpoint}/moderations). Detection-only block on the API'sflaggedboolean; optionalcategory_thresholdsenforces only the listed categories at operator-chosen score cutoffs.presidio— self-hosted Microsoft Presidio, two-stepanalyze→anonymize. Per-entitymask/blockactions (same shape askind: "pii"),language,score_threshold, and a selectable anonymize operator:replace(default),mask,hash,redact.How
aisix-guardrails, each behind its own default-on cargo feature (lakera/openai-moderation/presidio), following the prompt_shield/bedrock house patterns: per-hook fail policies (fail_openinput /output_fail_openoutput), timeout, 429/5xx/4xx failure taxonomy with stableguardrail_bypassed_reasontags, no matched content in reasons or logs (bug: output guardrail error message echoes the matched forbidden literal back to caller #153).moderates_segments/moderate_*_segments): masks write back positionally through the existing wire walkers on request and response, including streaming (both forceBufferFullhold-back likekind: "pii"). On the blob path (families without a write-back channel) a maskable outcome maps to Block — the kind=bedrock ANONYMIZE contract.enforcement_mode: monitor; no bespoke flag-only mode.schemas/resources/guardrail.schema.jsonregenerated — the admin API accepts the new kinds and the heartbeatsupported_kindsreports them to the CP.LiteLLM baseline:
lakeramirrorslakera_ai_v2(flagged + PII-only → mask via payload offsets, otherwise block);openai_moderationmirrors LiteLLM's block-on-flagged default (category_thresholdsis a superset knob LiteLLM doesn't have, empty by default);presidiomirrors LiteLLM's per-entity MASK/BLOCK + language + skip-empty-text (operator selection is a superset — LiteLLM always uses Presidio's default replace). The operator knob also closes the hash/placeholder action gap tracked in api7/AISIX-Cloud#562.Tests
aisixbinary + etcd + mock guard/moderation/presidio endpoints + mock upstream):guardrail-lakera-e2e(block / PII mask before upstream / 5xx fail-open),guardrail-openai-moderation-e2e(block / clean / monitor mode / threshold mode / 5xx fail-open),guardrail-presidio-e2e(per-entity block / input redaction asserted on the upstream body / output redaction non-streaming + split-across-chunks streaming / hash operator / 5xx fail-open).Fixes #52 (link added post-merge for the Development field; the issue closes with the CP exposure PR). The CP exposure (descriptors, dashboard form, credential encryption) and the
api7/docspages land as paired PRs; the CP PR carries the closing keyword.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes