CAMEL-24218: Add Security-First execution layer to Camel JBang MCP Server

[oscerd]

Summary

Adds an opt-in security execution layer for the Camel JBang MCP Server that provides:

• Tool-level authorization: Three access levels (read-only / read-write / admin) derived from existing MCP tool annotations (readOnlyHint/destructiveHint) — no separate ACL needed
• Audit trail: Structured JSON logging of tool invocations (who, what, when, outcome) via dedicated logger category org.apache.camel.mcp.security.audit
• Input sanitization: Centralized validation (argument length limits, control character stripping) before tool execution
• Secret redaction: Regex-based filtering of credentials, tokens, API keys, AWS keys, and connection strings from tool responses
• Configuration: All features controlled via Quarkus @ConfigProperty with camel.mcp.security.* prefix

Architecture

Uses a CDI interceptor (@McpSecured) for input sanitization, audit logging, and secret redaction, plus a global ToolFilter for tool visibility control:

Component	API	Purpose
McpAccessFilter	ToolFilter (global CDI bean)	Hides unauthorized tools from tools/list + audit logs denied access
McpSecurityInterceptor	@AroundInvoke CDI interceptor	Input sanitization, audit logging, secret redaction
McpSecured	@InterceptorBinding	Marks tool classes for security interception
McpSecurityConfig	CDI Bean	Configuration holder with PatternSyntaxException handling
McpAuditLogger	CDI Bean	Structured audit log formatter (RFC 8259 compliant JSON escaping)
McpSecretRedactor	CDI Bean	Regex-based redaction engine

Note: The initial implementation used ToolInputGuardrail/ToolOutputGuardrail APIs, but these require per-tool @ToolGuardrails annotations and are not auto-discovered as global CDI beans. The CDI interceptor approach provides the same security guarantees with lower maintenance overhead (19 class annotations vs 65 per-method annotations).

All features are disabled by default (camel.mcp.security.enabled=false) to preserve backward compatibility.

Access Level Model

Level	Permits	Tool count
read-only	readOnlyHint=true only	57
read-write	above + destructiveHint=false	62
admin (default)	all tools	65

Test plan

• McpSecurityConfigTest — AccessLevel.permits() logic (9 combinations), defaults, custom patterns
• McpAccessFilterTest — Tool visibility at each access level
• McpSecretRedactorTest — All redaction patterns, false positives, null/empty input
• McpAuditLoggerTest — JSON escaping (including \b/\f), logger instantiation
• McpSecurityInterceptorTest — Input sanitization (control chars via Unicode escapes, null, empty)
• All 334 existing tests pass (no regressions)

Claude Code on behalf of oscerd

🤖 Generated with Claude Code

[gnodet]

Well-designed security layer — defense-in-depth with both filter (hides tools from discovery) and guardrail (blocks execution even if filter bypassed), all disabled by default for backward compatibility, clean CDI architecture using Quarkus MCP Server guardrail APIs.

What's good:

• Three-tier access model (read-only / read-write / admin) derived from existing MCP tool annotations — no separate ACL needed
• Input sanitization (control char stripping, argument length limits) and output secret redaction are orthogonal concerns properly separated into distinct CDI beans
• Structured JSON audit logging to a dedicated logger category (org.apache.camel.mcp.security.audit) — operators can route it independently
• Good test coverage: 5 test files covering config, access filter, guardrail, audit logger, and secret redactor
• AccessLevel.parse() is lenient (accepts read-only, readonly, READ_ONLY) — good UX for config

Issues to address:

1. Binary test file — McpAccessGuardrailTest.java contains a literal null byte (0x00) in the stripControlCharsRemovesNullBytes() test string. This causes git to treat the entire file as binary, making it invisible to GitHub diff/review. Fix: use Java escape sequences instead — "hello\0world" for the null byte, "�" for the bell character in the other test. Same test coverage, reviewable file.

2. Missing McpOutputGuardrailTest — there's no test for the output guardrail, which handles secret redaction on tool responses and audit trail logging for results. The McpSecretRedactorTest covers pattern matching, but the output guardrail's integration logic (iterating Content items, replacing TextContent, constructing new ToolResponse) is untested.

Minor observations (not blocking):

• McpOutputGuardrail.logToolResult always passes durationMs=0 since the output guardrail has no access to when the tool started. Consider either omitting the field when unavailable, or documenting that it's only populated when a timing wrapper is present.
• The escape() method in McpAuditLogger handles \, ", \n, \r, \t but not \b (U+0008) or \f (U+000C). Fine for practical purposes, but a dedicated JSON writer (Jackson/Vert.x JsonObject) would be more robust.
• Custom redaction patterns from config (camel.mcp.security.redaction.patterns) are compiled without any safeguard against pathological regexes. Consider adding a compile-time timeout or a brief note in docs about pattern complexity.

Checklist:

• Code quality — clean, well-separated concerns, proper CDI usage
• Security model — defense-in-depth (filter + guardrail), safe defaults (disabled, admin level)
• Backward compatible — all features opt-in, disabled by default
• No Lombok, no FQCNs, no Thread.sleep()
• Commit convention — CAMEL-24218: prefix
• Binary test file needs fix (literal null byte → escape sequence)
• Missing output guardrail test
• CI — pending

Reviewed with Claude Code on behalf of gnodet. This review was generated by an AI agent and may contain inaccuracies; please verify all suggestions before applying.

[davsclaus]

Thank you for the contribution! The security layer concept is well-designed — clean separation of concerns, safe defaults (disabled by default), and good test coverage for individual components. I appreciate the defense-in-depth approach with both filter and guardrail layers.

However, I found a critical issue with the guardrail wiring that prevents most of the security features from working at runtime. See inline comments for details.

Summary of findings (ordered by severity):

1. Critical — Guardrails never fire at runtime — ToolInputGuardrail and ToolOutputGuardrail require per-tool @ToolGuardrails annotation (or programmatic ToolManager registration) in the Quarkus MCP Server API. Unlike ToolFilter, they are NOT auto-discovered as global CDI beans. The upstream Quarkus MCP Server source and tests confirm this — every @Tool uses explicit @ToolGuardrails(input=..., output=...). This renders input sanitization, audit logging, and secret redaction non-functional.

2. Medium — Binary test file — McpAccessGuardrailTest.java contains literal null bytes, causing Git to treat it as binary. The file is invisible in GitHub's diff view. Use Java escape sequences ("\0") instead of literal null bytes.

3. Medium — Missing McpOutputGuardrailTest — No test for the output guardrail's integration logic.

4. Low — No documentation — User-facing config properties (camel.mcp.security.*) need a documentation page.

5. Low — durationMs=0 always — Output guardrail has no access to timing info; consider omitting the field or documenting this limitation.

This review does not replace specialized tools such as CodeRabbit, Sourcery, or SonarCloud.

This review was generated by an AI agent (Claude Code on behalf of davsclaus) and may contain inaccuracies. Please verify all suggestions before applying.

[davsclaus]

Critical: This guardrail will never be invoked at runtime.

In the Quarkus MCP Server API, ToolFilter is auto-discovered as a global CDI bean (your McpAccessFilter works correctly), but ToolInputGuardrail requires explicit per-tool association via @ToolGuardrails(input = McpAccessGuardrail.class) on each @Tool method, or programmatic registration via ToolManager.setInputGuardrails().

I verified this against the upstream source and upstream tests — every tool that uses guardrails has an explicit @ToolGuardrails annotation.

Without this wiring, the input sanitization, authorization defense-in-depth, and audit logging handled by this class are completely non-functional. The unit tests pass because they test the logic directly, bypassing the Quarkus CDI/MCP wiring.

There are 266 @Tool methods in camel-jbang-mcp and zero @ToolGuardrails annotations. Options to fix:

• Add @ToolGuardrails to every @Tool method (high churn)
• Move this logic into the ToolFilter (which IS global) or a Vert.x/CDI interceptor
• Check with the Quarkus MCP Server team if global guardrail support is planned

[oscerd]

Fixed — both guardrails were replaced with a CDI interceptor in the previous commit. The wasRedacted tracking bug is also fixed in the latest push.

Claude Code on behalf of oscerd

[davsclaus]

Same issue as McpAccessGuardrail — this ToolOutputGuardrail implementation will never be invoked at runtime without per-tool @ToolGuardrails(output = McpOutputGuardrail.class) annotation on each @Tool method.

This means secret redaction on tool responses and audit logging of tool results are non-functional.

Also, durationMs is always 0 (line 86) since the output guardrail has no timing context — consider either omitting this field or documenting the limitation.

[oscerd]

Fixed — output guardrail was removed and replaced by the CDI interceptor. Duration is now tracked via System.nanoTime() and the redacted flag is properly set when redaction occurs.

Claude Code on behalf of oscerd

[davsclaus]

Minor: this hand-rolled JSON escaping handles \, ", \n, \r, \t but misses \b (U+0008) and \f (U+000C), which are also required JSON escapes per RFC 8259. Consider using Jackson's JsonStringEncoder or Vert.x JsonObject for more robust output — both are already on the classpath.

[oscerd]

Fixed — escape() now handles \\b (U+0008) and \\f (U+000C). Kept the hand-rolled approach since only two extra replacements were needed and both Jackson/Vert.x are heavier for this use case.

Claude Code on behalf of oscerd

[github-actions]

🌟 Thank you for your contribution to the Apache Camel project! 🌟
🤖 CI automation will test this PR automatically.

🐫 Apache Camel Committers, please review the following items:

• First-time contributors require MANUAL approval for the GitHub Actions to run
• You can use the command /component-test (camel-)component-name1 (camel-)component-name2.. to request a test from the test bot although they are normally detected and executed by CI.
• You can label PRs using skip-tests and test-dependents to fine-tune the checks executed by this PR.
• Build and test logs are available in the summary page. Only Apache Camel committers have access to the summary.

⚠️ Be careful when sharing logs. Review their contents before sharing them publicly.

[github-actions]

🧪 CI tested the following changed modules:

• dsl/camel-jbang/camel-jbang-mcp

---

🔬 Scalpel shadow comparison — Scalpel: 1 tested, 0 compile-only — current: 1 all tested

Maveniverse Scalpel detected 1 affected modules (current approach: 1).

Skip-tests mode would test 1 modules (1 direct + 0 downstream), skip tests for 0 (generated code, meta-modules)

Modules Scalpel would test (1)

• camel-jbang-mcp

ℹ️ Shadow mode — Scalpel observes but does not affect test execution. Learn more

⚠️ Some tests are disabled on GitHub Actions (@DisabledIfSystemProperty(named = "ci.env.name")) and require manual verification:

• dsl/camel-jbang/camel-jbang-mcp: 1 test(s) disabled on GitHub Actions

---

⚙️ View full build and test results

[gnodet]

LGTM — well-designed security layer for the Camel MCP server. The architecture uses McpAccessFilter (implementing ToolFilter, auto-discovered as a global CDI bean) for tool visibility filtering, and McpSecurityInterceptor (CDI @AroundInvoke bound via @McpSecured) for input sanitization, audit logging, and secret redaction. This is a clean alternative to the per-tool @ToolGuardrails approach.

Key observations:

• Comprehensive test coverage across 5 test files
• Proper System.nanoTime() duration measurement in the interceptor
• RFC 8259-compliant JSON escaping (including \b and \f)
• Operator-controlled redaction patterns align with Camel's trust model

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of @gnodet

[gnodet]

Well-structured opt-in security layer for the MCP server — clean separation of concerns (filter, interceptor, config, audit, redaction) and the disabled-by-default approach is the right call for backward compatibility.

A few issues and observations below. None are blocking since the feature is entirely opt-in, but items marked with ⚠️ warrant attention.

Missing documentation: The PR adds 7 new user-facing configuration properties (camel.mcp.security.*) and a new security layer with annotations on all 19 tool classes. Per project conventions, user-visible changes should be documented in the component docs and mentioned in the upgrade guide for discoverability.

Minor config robustness: getMaxArgumentLength() silently treats negative values as the default (10000) without logging — could cause confusion during troubleshooting. getRedactionPatterns() compiles user-provided regexes without a try-catch for PatternSyntaxException — a malformed pattern would crash with an unhelpful stack trace on first tool invocation.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of Guillaume Nodet

[gnodet]

⚠️ Bug: The redacted parameter is always hardcoded to false here, but redaction can actually occur at lines 117-120 above. When redactor.containsSecret(s) is true and redactor.redact(s) is called, this audit entry should report redacted: true.

Consider tracking it as a local variable:

boolean wasRedacted = false;
// ... in try block:
if (config.isRedactionEnabled() && result instanceof String s) {
    if (redactor.containsSecret(s)) {
        result = redactor.redact(s);
        wasRedacted = true;
    }
}
// ... in finally block:
auditLogger.logToolResult(toolName, "", isError, wasRedacted, durationMs);

[oscerd]

Fixed — introduced wasRedacted variable that's set to true when redaction occurs, and passed to the audit logger in the finally block. Good catch!

Claude Code on behalf of oscerd

[gnodet]

⚠️ logAccessDenied() is defined and unit-tested but never called in any production code path. When McpAccessFilter denies a tool, the framework itself handles the rejection — but no audit log entry is written for denied attempts.

This means denied access attempts are invisible in the audit trail, which undermines the security audit story. Consider calling this from McpAccessFilter.test() when it returns false, or removing it if execution-time denial auditing is out of scope for this PR.

[oscerd]

Fixed — logAccessDenied() is now called from McpAccessFilter.test() when a tool is filtered out due to insufficient access level. Also attempts to extract connectionId and clientName from the FilterContext (with fallback to empty strings if not available).

Claude Code on behalf of oscerd

[gnodet]

Minor: connectionId and clientName are always empty strings here. These are key forensic fields in the audit schema. If the MCP connection context provides this info (e.g., via InvocationContext or request scope), it would strengthen the audit trail. If not currently available, a TODO comment noting the limitation would help future contributors.

[oscerd]

Added a TODO comment documenting this limitation. CDI interceptors don't have access to the MCP connection context — these fields will be populated once the Quarkus MCP Server supports global guardrails. Meanwhile, McpAccessFilter now extracts connection context where available.

Claude Code on behalf of oscerd

[gnodet]

Re-reviewed after the latest commits. The author addressed the key findings from the previous review:

• ✅ wasRedacted flag now properly tracked and passed to audit logger
• ✅ logAccessDenied no longer dead code — called from McpAccessFilter.test()
• ✅ PatternSyntaxException handling added in getRedactionPatterns()
• ✅ Binary test file issue resolved
• ✅ davsclaus's guardrails concern addressed — replaced with CDI @AroundInvoke interceptor

Two low-severity items remain (silent maxArgumentLength fallback, missing .adoc documentation) but are non-blocking since the feature is opt-in and disabled by default.

This review was generated by an AI agent (Claude Code) and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of Guillaume Nodet