You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by "oscerd (via GitHub)" <gi...@apache.org> on 2026/07/24 11:16:12 UTC

[PR] camel-jbang - Redact secrets in structured MCP tool results, not only String results [camel]

oscerd opened a new pull request, #25093:
URL: https://github.com/apache/camel/pull/25093

   ## What
   
   Completes the secret-redaction coverage of the Camel JBang MCP server's opt-in security layer (added in CAMEL-24218). Redaction previously only inspected `String` tool results, so tools returning a `JsonObject` bypassed it entirely, and the value regex could not match JSON-quoted values.
   
   ## Why
   
   The runtime tools return `org.apache.camel.util.json.JsonObject`, not `String`. `camel_runtime_properties` in particular dumps a running application's configuration, which is exactly where a datasource password or an API key surfaces. With redaction enabled, that output was still emitted verbatim because the interceptor only redacted `String` results:
   
   ```java
   if (config.isRedactionEnabled() && result instanceof String s) { ... }
   ```
   
   Separately, the value regex (`[^\s,;}'"\]]+`) stops at a `"`, so it never matched JSON-quoted values even for the cases it was meant to cover.
   
   ## Change
   
   - `McpSecretRedactor.redactStructured(Object)` walks `Map`/`List` results in place (`JsonObject` is a `LinkedHashMap`, `JsonArray` an `ArrayList`), blanks the value of any entry whose key name denotes a secret (separator- and case-insensitive, so `client-secret`, `client_secret` and `clientSecret` all match), and runs the existing regex over string leaf values. The instance is mutated in place, so a tool's declared return type is preserved.
   - Strengthened `DEFAULT_PATTERNS`: `client_secret`, secrets carried as a URL query parameter (`jdbc:...?password=`, which has no userinfo `@` and so was missed by the connection-string rule), PEM private-key blocks, and `passphrase`.
   - `McpSecurityInterceptor` now routes `Map`/`List` results through `redactStructured` and keeps the regex path for `String`.
   - Tests: 7 new cases in `McpSecretRedactorTest`, including the previously-missed `JsonObject` with a `datasource.password` key, nested `client-secret`, URL query-parameter and PEM cases, and a no-change assertion for clean data.
   
   Redaction stays opt-in and off by default; this only changes what happens once an operator sets `camel.mcp.security.redaction.enabled=true`, so there is no behavior change for the default configuration.
   
   ## Known follow-up
   
   Plain record-returning tools (for example `ExampleFileResult.content`) are still not redacted. Doing that type-safely needs either reflective record rebuilding or a framework-level output filter once the Quarkus MCP server exposes global guardrails (the interceptor already carries a TODO for that). This PR deliberately covers the structured `JsonObject` path, which is where live secrets actually surface.
   
   ## Scope note
   
   Per the [Camel security model](https://github.com/apache/camel/blob/main/docs/user-manual/modules/ROOT/pages/security-model.adoc), the MCP server is a management surface whose network exposure is operator responsibility, so this is hardening of an opt-in control rather than a framework vulnerability fix. A separate, smaller item worth considering is that `camel mcp --http` binds `0.0.0.0` by default rather than loopback; that is not addressed here.
   
   ## Testing
   
   ```
   Tests run: 35, Failures: 0, Errors: 0
     (McpSecretRedactorTest, McpSecurityInterceptorTest, McpSecurityConfigTest, McpAccessFilterTest)
   ```
   Formatted with `mvn formatter:format impsort:sort`.
   
   ## Attribution
   
   _Claude Code on behalf of Andrea Cosentino (@oscerd)._ No JIRA was filed for this change; happy to add a `CAMEL-XXXX` reference and rename the branch/commit if you would prefer to track it in JIRA.
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] camel-jbang - Redact secrets in structured MCP tool results, not only String results [camel]

Posted by "oscerd (via GitHub)" <gi...@apache.org>.
oscerd commented on code in PR #25093:
URL: https://github.com/apache/camel/pull/25093#discussion_r3656040950


##########
dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/McpSecurityInterceptor.java:
##########
@@ -84,11 +86,17 @@ Object intercept(InvocationContext ctx) throws Exception {
         try {
             Object result = ctx.proceed();
 
-            // Secret redaction on string results
-            if (config.isRedactionEnabled() && result instanceof String s) {
-                if (redactor.containsSecret(s)) {
-                    result = redactor.redact(s);
-                    wasRedacted = true;
+            // Secret redaction. Free-text (String) results go through the regex redactor; structured results
+            // (Map/List, such as the JsonObject returned by the runtime tools) are scrubbed by key name in place,
+            // which is what catches JSON-quoted values the value regex cannot match.
+            if (config.isRedactionEnabled() && result != null) {
+                if (result instanceof String s) {
+                    if (redactor.containsSecret(s)) {
+                        result = redactor.redact(s);
+                        wasRedacted = true;

Review Comment:
   Done in 8e9decd — the String branch now calls redact(s) once and compares the result to the input to set wasRedacted, so the masking pipeline no longer runs twice. containsSecret() stays as a tested public API on the redactor.\n\n_Claude Code on behalf of Andrea Cosentino (@oscerd)._



##########
dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/McpSecurityInterceptor.java:
##########
@@ -84,11 +86,17 @@ Object intercept(InvocationContext ctx) throws Exception {
         try {
             Object result = ctx.proceed();
 
-            // Secret redaction on string results
-            if (config.isRedactionEnabled() && result instanceof String s) {
-                if (redactor.containsSecret(s)) {
-                    result = redactor.redact(s);
-                    wasRedacted = true;
+            // Secret redaction. Free-text (String) results go through the regex redactor; structured results
+            // (Map/List, such as the JsonObject returned by the runtime tools) are scrubbed by key name in place,
+            // which is what catches JSON-quoted values the value regex cannot match.
+            if (config.isRedactionEnabled() && result != null) {
+                if (result instanceof String s) {
+                    if (redactor.containsSecret(s)) {
+                        result = redactor.redact(s);
+                        wasRedacted = true;
+                    }
+                } else if (result instanceof Map<?, ?> || result instanceof List<?>) {

Review Comment:
   Done in 8e9decd — the String branch now calls redact(s) once and compares the result to the input to set wasRedacted, so the masking pipeline no longer runs twice. containsSecret() stays as a tested public API on the redactor.\n\n_Claude Code on behalf of Andrea Cosentino (@oscerd)._



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] camel-jbang - Redact secrets in structured MCP tool results, not only String results [camel]

Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #25093:
URL: https://github.com/apache/camel/pull/25093#discussion_r3645907647


##########
dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/McpSecurityInterceptor.java:
##########
@@ -84,11 +86,17 @@ Object intercept(InvocationContext ctx) throws Exception {
         try {
             Object result = ctx.proceed();
 
-            // Secret redaction on string results
-            if (config.isRedactionEnabled() && result instanceof String s) {
-                if (redactor.containsSecret(s)) {
-                    result = redactor.redact(s);
-                    wasRedacted = true;
+            // Secret redaction. Free-text (String) results go through the regex redactor; structured results
+            // (Map/List, such as the JsonObject returned by the runtime tools) are scrubbed by key name in place,
+            // which is what catches JSON-quoted values the value regex cannot match.
+            if (config.isRedactionEnabled() && result != null) {
+                if (result instanceof String s) {
+                    if (redactor.containsSecret(s)) {
+                        result = redactor.redact(s);
+                        wasRedacted = true;

Review Comment:
   **Low:** `containsSecret(s)` internally calls `redact(s)` and compares the result. If it returns true, `redact(s)` is called again here — running the full masking pipeline (DefaultMaskingFormatter + custom patterns) twice. Could be simplified:
   
   ```suggestion
                       String redacted = redactor.redact(s);
                       if (!redacted.equals(s)) {
                           result = redacted;
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] camel-jbang - Redact secrets in structured MCP tool results, not only String results [camel]

Posted by "oscerd (via GitHub)" <gi...@apache.org>.
oscerd commented on PR #25093:
URL: https://github.com/apache/camel/pull/25093#issuecomment-5070047390

   Thanks @davsclaus, good call. Reworked to reuse Camel's own secret knowledge instead of a hand-rolled regex:
   
   - **Structured (`JsonObject`) results** are now redacted by key name with `SensitiveUtils.containsSensitive(...)`, the same helper the jbang `ListProperties` command already uses. This is the important path, since the runtime tools (e.g. `camel_runtime_properties`) return a `JsonObject`.
   - **Free-text results** go through `DefaultMaskingFormatter`, which masks key=value, XML and JSON values off `SensitiveUtils.getSensitiveKeys()`.
   - Removed the hand-maintained `DEFAULT_PATTERNS` and `SECRET_KEY_NAME`. Operator-supplied custom patterns (`camel.mcp.security.redaction.patterns`) still apply on top.
   
   One consequence worth flagging: the two format-only cases I had (a `mongodb://user:pass@host` userinfo credential and PEM private-key blocks) are no longer special-cased, since `SensitiveUtils`/`DefaultMaskingFormatter` key off names rather than value shapes. The `?password=` query-parameter form is still caught (it is key=value). If the userinfo/PEM shapes are worth covering, that feels like it belongs in `SensitiveUtils`/`DefaultMaskingFormatter` project-wide rather than here; happy to open a separate issue if you agree.
   
   Tests updated to the canonical behaviour (30 green in the module). Still draft while I settle the record-returning-tools follow-up noted in the description.
   
   _Claude Code on behalf of Andrea Cosentino (@oscerd)._
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] camel-jbang - Redact secrets in structured MCP tool results, not only String results [camel]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #25093:
URL: https://github.com/apache/camel/pull/25093#issuecomment-5069457089

   <!-- ci-tested-modules -->
   :test_tube: **CI tested the following changed modules:**
   
   - `dsl/camel-jbang/camel-jbang-mcp`
   
   ---
   
   <details><summary>:microscope: Scalpel shadow comparison — Scalpel: 1 tested, 0 compile-only — current: 1 all tested</summary>
   
   [Maveniverse Scalpel](https://github.com/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)
   
   <details><summary>Modules Scalpel would test (1)</summary>
   
   - `camel-jbang-mcp`
   
   </details>
   
   > :information_source: Shadow mode — Scalpel observes but does not affect test execution. [Learn more](https://github.com/maveniverse/scalpel)
   
   </details>
   
   :warning: **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
   
   ---
   :gear: [View full build and test results](https://github.com/apache/camel/actions/runs/30088985411)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] camel-jbang - Redact secrets in structured MCP tool results, not only String results [camel]

Posted by "oscerd (via GitHub)" <gi...@apache.org>.
oscerd merged PR #25093:
URL: https://github.com/apache/camel/pull/25093


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] camel-jbang - Redact secrets in structured MCP tool results, not only String results [camel]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #25093:
URL: https://github.com/apache/camel/pull/25093#issuecomment-5069266658

   :star2: Thank you for your contribution to the Apache Camel project! :star2:
     :robot: CI automation will test this PR automatically.
   
     :camel: 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](https://camel.apache.org/community/team/#committers) have access to the summary.
     
     :warning: Be careful when sharing logs. Review their contents before sharing them publicly.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] camel-jbang - Redact secrets in structured MCP tool results, not only String results [camel]

Posted by "davsclaus (via GitHub)" <gi...@apache.org>.
davsclaus commented on code in PR #25093:
URL: https://github.com/apache/camel/pull/25093#discussion_r3646022374


##########
dsl/camel-jbang/camel-jbang-mcp/src/main/java/org/apache/camel/dsl/jbang/core/commands/mcp/McpSecurityInterceptor.java:
##########
@@ -84,11 +86,17 @@ Object intercept(InvocationContext ctx) throws Exception {
         try {
             Object result = ctx.proceed();
 
-            // Secret redaction on string results
-            if (config.isRedactionEnabled() && result instanceof String s) {
-                if (redactor.containsSecret(s)) {
-                    result = redactor.redact(s);
-                    wasRedacted = true;
+            // Secret redaction. Free-text (String) results go through the regex redactor; structured results
+            // (Map/List, such as the JsonObject returned by the runtime tools) are scrubbed by key name in place,
+            // which is what catches JSON-quoted values the value regex cannot match.
+            if (config.isRedactionEnabled() && result != null) {
+                if (result instanceof String s) {
+                    if (redactor.containsSecret(s)) {
+                        result = redactor.redact(s);
+                        wasRedacted = true;
+                    }
+                } else if (result instanceof Map<?, ?> || result instanceof List<?>) {

Review Comment:
   Minor: `containsSecret(s)` now internally calls `redact(s)` and discards the result, then `redact(s)` is called again here. Could be simplified to:
   
   ```suggestion
                   if (result instanceof String s) {
                       String scrubbed = redactor.redact(s);
                       if (!scrubbed.equals(s)) {
                           result = scrubbed;
                           wasRedacted = true;
                       }
   ```
   
   This avoids running the full masking pipeline twice for strings that contain secrets.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org