Repository navigation
Fixes #7950: Add unified AI and MCP invocation auditing - #8002
BobSong-dev wants to merge 39 commits into
Conversation
…ateway-p105 # Conflicts: # shenyu-plugin/shenyu-plugin-agent-gateway/src/main/java/org/apache/shenyu/plugin/agent/gateway/AgentGatewayPlugin.java # shenyu-plugin/shenyu-plugin-agent-gateway/src/main/java/org/apache/shenyu/plugin/agent/gateway/protocol/AgentMcpHttpHandler.java # shenyu-plugin/shenyu-plugin-ai/shenyu-plugin-ai-proxy/src/main/java/org/apache/shenyu/plugin/ai/proxy/enhanced/service/AiProxyEngine.java # shenyu-spring-boot-starter/shenyu-spring-boot-starter-plugin/shenyu-spring-boot-starter-plugin-agent-gateway/src/main/java/org/apache/shenyu/springboot/starter/plugin/agent/gateway/AgentGatewayPluginConfiguration.java
…audit-7950-integration
…to integration/agent-gateway-p105 # Conflicts: # .github/scripts/test-agent-gateway-migrations.py # shenyu-plugin/shenyu-plugin-ai/shenyu-plugin-ai-proxy/src/main/java/org/apache/shenyu/plugin/ai/proxy/enhanced/service/AiProxyEngine.java # shenyu-plugin/shenyu-plugin-metrics/src/main/java/org/apache/shenyu/plugin/metrics/reporter/MetricsReporter.java # shenyu-spring-boot-starter/shenyu-spring-boot-starter-plugin/shenyu-spring-boot-starter-plugin-agent-gateway/src/main/java/org/apache/shenyu/springboot/starter/plugin/agent/gateway/AgentGatewayPluginConfiguration.java
…nt-gateway-audit-7950-integration # Conflicts: # shenyu-plugin/shenyu-plugin-agent-gateway/src/main/java/org/apache/shenyu/plugin/agent/gateway/protocol/AgentMcpDispatcher.java # shenyu-plugin/shenyu-plugin-agent-gateway/src/main/java/org/apache/shenyu/plugin/agent/gateway/protocol/AgentMcpExecutionContext.java # shenyu-plugin/shenyu-plugin-agent-gateway/src/main/java/org/apache/shenyu/plugin/agent/gateway/protocol/AgentMcpHttpHandler.java # shenyu-plugin/shenyu-plugin-ai/shenyu-plugin-ai-proxy/src/main/java/org/apache/shenyu/plugin/ai/proxy/enhanced/service/AiProxyEngine.java # shenyu-plugin/shenyu-plugin-ai/shenyu-plugin-ai-proxy/src/test/java/org/apache/shenyu/plugin/ai/proxy/enhanced/AiProxyPluginTest.java # shenyu-plugin/shenyu-plugin-metrics/src/main/java/org/apache/shenyu/plugin/metrics/reporter/MetricsReporter.java # shenyu-spring-boot-starter/shenyu-spring-boot-starter-plugin/shenyu-spring-boot-starter-plugin-agent-gateway/src/main/java/org/apache/shenyu/springboot/starter/plugin/agent/gateway/AgentGatewayPluginConfiguration.java # shenyu-spring-boot-starter/shenyu-spring-boot-starter-plugin/shenyu-spring-boot-starter-plugin-ai-proxy/src/main/java/org/apache/shenyu/springboot/starter/plugin/ai/proxy/AiProxyPluginConfiguration.java
…nt-gateway-audit-7950-integration # Conflicts: # shenyu-plugin/shenyu-plugin-metrics/src/main/java/org/apache/shenyu/plugin/metrics/reporter/MetricsReporter.java # shenyu-spring-boot-starter/shenyu-spring-boot-starter-plugin/shenyu-spring-boot-starter-plugin-agent-gateway/src/main/java/org/apache/shenyu/springboot/starter/plugin/agent/gateway/AgentGatewayPluginConfiguration.java
Aias00
left a comment
Aias00
left a comment
There was a problem hiding this comment.
Incremental audit review: 62 targeted tests passed, covering audit-event bounds and metadata, audit observer bridging, bounded collector delivery, key-auth rejections, AI/Agent starter wiring, and loopback HTTP governance. The bridge isolates sink failures from request delivery and reuses the existing terminal owners; no new blocking issue found in the reviewed audit delta.
Validation baseline was head e69b7cc plus a clean, uncommitted merge of master aab453f. The raw historical head fails Maven model loading because its inherited AI PII parent POM versions remain 2.7.2-SNAPSHOT. The master merge fixes those baseline versions; no PR source was manually patched.
This remains a stacked Draft depending on #7860/#7921. It also inherits the released migration edits and standalone order script that #7860 has now corrected in f9eb7ba/a34127bc8. Incorporate the updated prerequisites and rebuild the branch so the reviewable diff is limited to auditing; this comment is not approval of the inherited migration changes.
Validation used isolated worktrees, the Maven wrapper, JDK 21 with the project compiler settings, and targeted tests. Checkstyle passed; RAT and JaCoCo were skipped. This does not claim a full reactor, e2e, or live deployment test run.
# Conflicts: # shenyu-spring-boot-starter/shenyu-spring-boot-starter-plugin/shenyu-spring-boot-starter-plugin-agent-gateway/src/main/java/org/apache/shenyu/springboot/starter/plugin/agent/gateway/AgentGatewayPluginConfiguration.java
# Conflicts: # shenyu-plugin/shenyu-plugin-ai/shenyu-plugin-ai-proxy/src/main/java/org/apache/shenyu/plugin/ai/proxy/enhanced/AiProxyPlugin.java # shenyu-plugin/shenyu-plugin-ai/shenyu-plugin-ai-proxy/src/test/java/org/apache/shenyu/plugin/ai/proxy/enhanced/AiProxyPluginTest.java
…ion/agent-gateway-p105 # Conflicts: # shenyu-spring-boot-starter/shenyu-spring-boot-starter-plugin/shenyu-spring-boot-starter-plugin-agent-gateway/src/main/java/org/apache/shenyu/springboot/starter/plugin/agent/gateway/AgentGatewayPluginConfiguration.java
…ateway-p105 # Conflicts: # shenyu-plugin/shenyu-plugin-ai/shenyu-plugin-ai-proxy/src/test/java/org/apache/shenyu/plugin/ai/proxy/enhanced/AiProxyPluginTest.java
…to integration/agent-gateway-p105
…to feat/agent-gateway-audit-7950-integration
Aias00
left a comment
Aias00
left a comment
There was a problem hiding this comment.
Reviewed e9dc976.
The updated branch incorporates the corrected prerequisite migrations and current-master baseline. The audit bridge continues to isolate sink failures and reuse the existing terminal owner. Reviewed the incremental migration/audit mapping changes and reran the audit, observation, starter, ordering and migration regressions; no new blocking issue found.
This remains a stacked change: coordinate the prerequisite merge order and rebase the reviewable delta as those PRs land.
Targeted test report: 460 cases, 0 failures, 0 errors, 0 skipped.
Validation used ./mvnw, Amazon Corretto 21 and project target Java 17. Targeted Checkstyle, separately executed Apache RAT and git diff --check pass. No full-repository or Docker E2E pass is claimed; required CI must succeed before merge.
Aias00
left a comment
Aias00
left a comment
There was a problem hiding this comment.
Found one audit-completeness issue. An additional HTTP-handler regression test confirms that a denied tool call can suppress its logical audit record through the requested tool name.
| final Integer status, final long elapsed, final Integer attempt) { | ||
| AgentInvocationAuditEvent.Builder builder = Objects.isNull(attempt) ? AgentInvocationAuditEvent.logical() : AgentInvocationAuditEvent.attempt(attempt); | ||
| sink.record(builder.requestId(context.getRequestId()).tenant(context.getTenant()).subject(context.getSubject()).apiKeyId(context.getApiKeyId()) | ||
| .selectorId(context.getSelectorId()).ruleId(context.getRuleId()).target(context.getTarget()) |
There was a problem hiding this comment.
[P2] Preserve rejection audits when requested target text is invalid
The MCP terminal context contains the caller-supplied tool name, and the request parser only checks that this name is a nonempty string. Passing it directly to the audit builder makes any name longer than 256 characters (also padded/control-character names) fail nullableText; the surrounding safely then silently drops the entire logical audit event. Thus an authenticated caller can send denied tool calls without leaving the rejection audit this PR introduces. I reproduced this through AgentMcpHttpHandler with the audit bridge: an unknown name missing returns 403 and records one event, while a 257-character name with its matching Mcp-Name header returns the same 403 and records zero. Normalize/omit invalid optional target metadata while preserving the rejection event, or ensure parser rejections occur before that target enters the terminal context.
Aias00
left a comment
Aias00
left a comment
There was a problem hiding this comment.
Re-reviewed 4c6d01b against current master. Invalid optional caller-supplied target text is now omitted from the audit projection while preserving the logical rejection event, so oversized/padded/control-character tool names no longer suppress rejection audits. Reviewed the master integration and retained observation ownership, failure isolation and API-key identity mapping. This resolves the prior audit-completeness finding. Validation: 156 targeted common/audit collector/MCP HTTP/governance/starter/AI plugin tests passed with 0 failures, errors or skips; Maven Checkstyle and separately executed Apache RAT passed. Required CI remains the merge gate; no full Docker E2E pass is claimed.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #8002 +/- ##
=============================================
+ Coverage 61.49% 71.57% +10.08%
- Complexity 8818 17064 +8246
=============================================
Files 1261 1622 +361
Lines 39273 65258 +25985
Branches 3725 7767 +4042
=============================================
+ Hits 24151 46711 +22560
- Misses 12952 14512 +1560
- Partials 2170 4035 +1865
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Aias00
left a comment
Aias00
left a comment
There was a problem hiding this comment.
Stream Agent analysis
Cross-file FINAL review of #8002 (unified AI/MCP invocation auditing) at head b9a5bdc: 29 partitions all PASS, complete diff present, but CI not passed; verdict INCONCLUSIVE.
Evidence
- Frozen review attachment: all_parts_passed=true, complete_diff=true, global_complete_diff=true, 29 partition reviews all PASS.
- CI object: check_count=88, complete=true, passed=false, legacy_state=pending.
- Head sha b9a5bdc matches reviewed_head.
Recommended next steps
- Investigate CI failures (88 checks, passed=false) to ensure new audit modules compile and tests pass.
- Confirm cross-module contracts with prerequisite PRs #7860/#7921 if still open.
- Re-run FINAL review after CI passes.
Scope: captured diff and GitHub check metadata only; no local tests were executed.
Task: 38ece911-ad93-4cca-8b05-e471839a937a · Commit: b9a5bdc723b5fdd549ea52057083f54e761df3f8
Review verdict: INCONCLUSIVE
SYNTHESIS/FINAL scope: all 29 child partition reviews returned PASS with no HIGH/CRITICAL findings and complete_diff=true for each and globally. However, the aggregated CI status reports complete=true but passed=false. The policy mandates all_parts_passed true, complete_diff true, and CI passed before a FINAL PASS. Because CI did not pass, a FINAL PASS cannot be issued. No concrete blocker found in diff, but inability to confirm build/test success yields INCONCLUSIVE rather than PASS. No NEEDS_CHANGES because no anchored HIGH/CRITICAL defects exist.
Publication: COMMENT (INCONCLUSIVE_REVIEW)
Review details
FINAL synthesis of PR #8002 at head b9a5bdc: all 29 partition reviews passed and complete diff was provided, but CI status indicates not passed (88 checks complete, passed=false). Per review policy, a FINAL PASS requires all_parts_passed, complete_diff true, and CI passed. Since CI did not pass, the verdict is INCONCLUSIVE. No HIGH/CRITICAL defects were identified in any partition. Recommend verifying CI failures and re-running checks before merge.
Aias00
left a comment
Aias00
left a comment
There was a problem hiding this comment.
Stream Agent analysis
FINAL review of apache/shenyu PR #8002 (head b9a5bdc) adding unified AI/MCP invocation auditing. All 29 partition reviews passed with no blocking findings; complete diff and CI passed.
Evidence
- all_parts_passed=true with 29 partition reviews (part-0..part-28) each verdict PASS and empty blocking_findings
- ci.passed=true, complete=true, 90 checks; complete_diff=true and global_complete_diff=true per frozen attachment
- head_sha b9a5bdc matches reviewed head; base_sha 585f99d
Recommended next steps
- Confirm prerequisite PRs #7860/#7921 are merged or otherwise satisfied before final release cut
- Merge PR #8002 to main branch per maintainer workflow
- Monitor audit collector metrics in a staging deployment to validate off-thread delivery
Scope: captured diff and GitHub check metadata only; no local tests were executed.
Task: 622ed2e3-13c6-44ed-9ec2-c1c34248f944 · Commit: b9a5bdc723b5fdd549ea52057083f54e761df3f8
Review verdict: PASS
FINAL synthesis per scope: assessed cross-file correctness, compatibility, contracts, tests and interactions from all child review summaries. All 29 child partitions reported PASS with no NEEDS_CHANGES or INCONCLUSIVE; all_parts_passed=true, complete_diff=true, global_complete_diff=true, and CI passed=true. No concrete HIGH/CRITICAL defect anchored to any diff path/line/side exists. Therefore cross-file assessment yields no blocking defect. Limitation: prerequisites #7860/#7921 noted as open in some partitions but not part of this PR's diff; they do not override the PASS verdict for supplied scope.
Publication: APPROVE (EXPLICIT_PASS)
Review details
This PR introduces unified AI and MCP invocation audit logging across plugins and starters. The durable partition review covered all 29 diff segments (docs, DTOs, plugin wiring, new audit module, tests) with zero HIGH/CRITICAL defects. CI passed (90 checks). Recommendation: approve/merge after confirming any open prerequisite PRs are integrated. No write actions taken.
Aias00
left a comment
Aias00
left a comment
There was a problem hiding this comment.
Stream Agent analysis
Reviewed #8002 (WHOLE_PR, VERSIONED_DIFF): unified, privacy-preserving AI/MCP invocation auditing with an immutable event contract, bridge/observer, a bounded logging-backed collector and Spring Boot starter, plus DTO/plugin/context wiring and tests. The complete pinned diff was delivered and CI is green; no concrete blocking defect was verified.
Evidence
- Frozen Whole-PR evidence: the complete pinned diff (base 149dba4 -> head 41c792d) is recorded as delivered in full (path '' 132630 chars, complete=true), covering 40 changed files including the audit contract/bridge/observer, collector+consumer+auto-configuration starter, DTOs, plugin wiring and tests; complete_diff=true.
- CI in frozen evidence: complete=true, passed=true, check_count=93 with all listed checks status 'completed'/'success' (build, e2e, sql-matrix, CodeQL, compat_java21 and test_group), failed_checks empty; the 'legacy_state:pending' field is stale metadata rather than an incomplete check.
- Targeted head reads: AgentInvocationAuditEvent enforces an immutable credential-free contract (SCHEMA_VERSION=1, bounded/stripped text validation, attempt>=1, elapsedMillis>=0, defaulted timestamp); AgentInvocationAuditCollector reuses the bounded logging queue, copies no request payload (desensitizeLog no-op), and exposes failedRecords instrumentation when the sink throws RuntimeException.
Recommended next steps
- Track that the prerequisite PRs #7860 and #7921 land before this draft is merged, as noted in the PR description.
Scope: captured diff and GitHub check metadata only; no local tests were executed.
Task: 1538af28-e5f6-48b4-85f5-2de4b18902fb · Commit: 41c792d5714ce279caf766a59439dc0f1b355d27
Review verdict: PASS
Whole-PR scope with full versioned-diff coverage and passing CI as reported by the frozen evidence. Targeted verification of the core audit event and collector found no concrete HIGH/CRITICAL defect anchored to a diff path/line/side, so NEEDS_CHANGES is not supported and no evidence gap remains to force INCONCLUSIVE. Tests, build and CI results come from the frozen evidence only; this review did not execute tests, merge, or submit a review, and coverage alone does not prove correctness.
Publication: APPROVE (EXPLICIT_PASS)
Fixes #7950
Background
Metrics and traces do not provide one correlated, privacy-preserving terminal record for AI and MCP invocations, especially when traffic is rejected before an upstream call. The gateway also had no shared vocabulary for correlating a logical invocation with its retry or fallback attempts.
This draft integrates the audit contract with request context, terminal observation, security, and resource behavior from #7860 and runtime governance from #7921. Those prerequisites are still open, so this branch intentionally contains normal merge commits from their reviewed heads.
Changes
AgentInvocationAuditEventwith logical and attempt records, trusted identity, internal API-key id, target, decision, bounded outcome/reason, status, timing, and correlation fields.AbstractLogCollectordelivery, existing queue instrumentation, and an application-replaceable consumer; it does not create another logging subsystem.docs/agent-invocation-audit.md.Verification
Local, Windows/JDK 17:
python .github/scripts/check-agent-gateway-migrations.py: mysql, ob, og, oracle, and pg each matched 24 Agent Gateway seed rows.python .github/scripts/test-agent-gateway-migrations.py: 11 tests, 0 failures, 0 errors.testwith dependencies: 33 reactor modules, BUILD SUCCESS; 3,069 tests, 0 failures, 0 errors, 4 skipped. Key results: Agent Gateway plugin 658 tests; Agent Gateway starter 197 tests; AI Proxy 338 tests; Tracing plugin 71 tests; Metrics plugin 53 tests.git diff --checkpassed.KeyAuthMcpHttpTest; the test itself passes and the build succeeds.Not run: full-project tests, e2e tests, a deployed Admin/WebSocket/Bootstrap run, or RISC-V/QEMU verification for this reconciled head.
GitHub CI is pending for the newly pushed head.