CAMEL-24220: Add camel_security_scan tool for route security analysis by oscerd · Pull Request #25026 · apache/camel

oscerd · 2026-07-22T12:58:40Z

Summary

Adds a new MCP tool camel_security_scan for static security analysis of Camel route definitions.

Distinct from camel_route_harden_context (general security context + CVE advisories), this tool performs line-by-line static analysis to detect specific anti-patterns:

Exposed secrets in URIs (passwords, tokens, API keys) — property placeholder-aware, correctly flags RAW()-wrapped values
Insecure configuration options from SecurityUtils.getSecurityOptions() (trustAllCertificates, allowJavaSerializedObject, etc.) — skips options with empty insecureValue
Missing Camel* header filters on HTTP/REST consumers — per-consumer scoping with nearby filter detection
Unencrypted protocols (HTTP, FTP, LDAP, SMTP vs secure alternatives)
Command injection risk via exec component
SQL injection risk — parameterized query check with composite-scheme-safe matching
Path traversal risk in file component with dynamic expressions

Returns findings with severity, line number, category, and remediation guidance.

Test plan

All 324 MCP server tests pass (no regressions)
Review feedback addressed: RAW() detection, empty insecureValue, composite scheme matching, per-consumer header filter scoping

Claude Code on behalf of oscerd

🤖 Generated with Claude Code

Adds a new MCP tool that performs static analysis of Camel route definitions to detect security anti-patterns. Distinct from camel_route_harden_context (which provides general security context and CVE advisories), this tool performs line-by-line analysis and returns actionable findings with severity, line numbers, and remediation guidance. Detection categories: - Insecure options from SecurityUtils (trustAllCertificates, allowJavaSerializedObject, transferException, etc. — 24 options across ssl/serialization/dev categories) - Plain-text secrets in URIs (password, token, apiKey, etc.) - Connection strings with embedded credentials (mongodb://, amqp://, etc.) - Unencrypted protocols (HTTP vs HTTPS, FTP vs SFTP, LDAP vs LDAPS, SMTP vs SMTPS) - Missing Camel* header filters on HTTP consumers (CVE-2025-27636 family) - Command injection via exec component - SQL injection risk (missing parameterized queries) - File path traversal with dynamic expressions Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Andrea Cosentino <ancosen@gmail.com>

gnodet

Well-designed static security scanner — good complement to HardenTools (general context/CVEs) by providing line-by-line analysis with actionable findings.

What's good:

Reuses SecurityUtils.getSecurityOptions() from camel-util for the canonical 24 insecure options — no duplicate maintenance
Property placeholder awareness: SECRET_IN_URI regex correctly excludes {{…}}, ${…}, and RAW(…) prefixed values
Scheme detection via containsScheme(text, scheme + ":") is precise — "https:" does NOT match "http:" check
platform-http correctly excluded from the unencrypted-HTTP check
Good false-positive tests (placeholder passwords, HTTPS, SFTP all correctly suppressed)
24 test cases covering detection, suppression, line numbers, severity ordering, and counts
Clean result model with records, severity sorting (critical first)
Proper MCP annotations (readOnlyHint=true, destructiveHint=false)

Minor observations (not blocking — inherent to text-based static analysis):

scanMissingHeaderFilters checks the entire route text for removeHeaders/headerfilter presence, so a filter in one route suppresses the finding for all consumers in the same input. Acceptable for a single-route scanner, but worth a Javadoc note.
scanExecComponent flags any line containing exec:, which could match YAML comments or description fields. Fine as an advisory finding.
No test for LDAP/LDAPS detection (the scanner checks for it but there's no corresponding test case)

Checklist:

Code quality — clean, well-structured, proper use of camel-util APIs
Security model — aligns with design/security.adoc threat model categories
Tests — 24 cases with both positive detection and false-positive suppression
No Lombok, no FQCNs, no Thread.sleep()
Backward compatible — new tool, no existing changes
Commit convention — CAMEL-24220: prefix
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.

gnodet

The static security scanner has good structure and useful output format, but contains correctness bugs that produce wrong results. All findings below were independently verified by a second agent.

Key issues (verified):

RAW() false negative (high) — the secret-detection regex skips password=RAW(mysecret), but SecurityUtils.isPlainTextSecret explicitly documents that RAW() is NOT a security mechanism
Empty insecureValue false positives (high) — three security options with insecureValue="" cause the scanner to flag ANY value assignment (e.g., sslEndpointAlgorithm=HTTPS is incorrectly flagged)
SQL scheme substring matching (medium) — google-bigquery-sql: URIs are falsely flagged as SQL injection risks because containsScheme(lower, "sql") uses substring matching
Code duplication with HardenTools (medium) — HTTP/HTTPS, FTP, exec, SQL, LDAP, and file path traversal checks are reimplemented here

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 · 2026-07-22T13:31:12Z

+
+    private static final Pattern SECRET_IN_URI = Pattern.compile(
+            "(?i)(password|passwd|pwd|secret|token|apikey|api[_-]?key|access[_-]?key)\\s*[=:]\\s*(?!\\{\\{)(?!\\$\\{)(?!RAW\\()([^\\s,;}'\"\\]&]+)");
+


Correctness bug: RAW() negative lookahead creates false negatives — The (?!RAW\\() in SECRET_IN_URI prevents matching password=RAW(mysecret123). However, SecurityUtils.isPlainTextSecret (line 146-147) explicitly documents:

"RAW() is a URI encoding wrapper, not a security mechanism -- RAW(password) is still plain text."

This means password=RAW(mysecret123) is silently skipped by the scanner, creating a false negative for actual plain-text secrets. Remove the (?!RAW\\() negative lookahead.

gnodet · 2026-07-22T13:31:12Z

+    }
+
+    private void scanInsecureOptions(String[] lines, List<Finding> findings) {
+        Map<String, SecurityUtils.SecurityOption> securityOptions = SecurityUtils.getSecurityOptions();


Correctness bug: empty insecureValue causes false positives — Three options (sslendpointalgorithm, stricthostkeychecking, x509hostnameverifier) have insecureValue="". The check lower.contains(optionKey + "=" + insecureValue) becomes lower.contains("sslendpointalgorithm=") which matches ANY assignment — including secure ones like sslEndpointAlgorithm=HTTPS.

Note that SecurityUtils.isInsecureValue correctly uses equalsIgnoreCase which only matches when the value is actually empty. Add a guard:

if (insecureValue.isEmpty()) { continue; }

gnodet · 2026-07-22T13:31:12Z

+                            "high",
+                            "header-injection",
+                            "Consumer '" + consumer + "' without Camel* header filter",
+                            lineNum,


False positive: google-bigquery-sql: URIs — containsScheme(lower, "sql") checks for sql: as a substring, which matches the composite scheme google-bigquery-sql:project:dataset.table. BigQuery SQL uses a different query model and does not use :#param syntax. The same issue applies to other composite schemes ending with a target scheme name.

gnodet · 2026-07-22T13:31:12Z

+            }
+            if (containsScheme(lower, "ldap") && !containsScheme(lower, "ldaps")) {
+                findings.add(new Finding(
+                        "medium",


Route-level header filter check — scanMissingHeaderFilters checks the ENTIRE route content for removeheaders/headerfilter/camel*. This means: (a) if a route has two HTTP consumers and only one has a header filter, neither gets flagged; (b) any occurrence of "camel*" anywhere in the route (even in a comment or description) suppresses all header-injection warnings globally.

gnodet · 2026-07-22T13:31:12Z

+import org.apache.camel.util.SecurityUtils;
+
+/**
+ * MCP Tool for static security analysis of Camel route definitions.


Code duplication with HardenTools.analyzeSecurityConcerns() — The HTTP-vs-HTTPS, FTP, exec, SQL injection, file path traversal, and LDAP/SMTP checks are all reimplemented here. These tools serve different purposes (catalog-aware context vs. line-level scanning) but share substantial string-matching logic that will diverge as one file is updated but not the other. Consider extracting shared detection rules into a common utility class.

github-actions · 2026-07-22T13:47:22Z

🌟 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 · 2026-07-22T14:03:23Z

🧪 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

- Fix RAW() false negative: remove RAW() negative lookahead from SECRET_IN_URI regex — RAW() is a URI encoding wrapper, not a security mechanism - Fix empty insecureValue false positives: skip options with empty insecureValue to avoid matching any assignment - Fix SQL scheme false positive on composite schemes: use boundary check in containsScheme() to avoid matching google-bigquery-sql: as sql: - Fix route-level header filter check: scope per-consumer line with hasHeaderFilterNearby() instead of checking entire route text Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Andrea Cosentino <ancosen@gmail.com>

oscerd · 2026-07-24T07:32:44Z

Thank you for the thorough review — all 4 blocking findings addressed:

High Severity (fixed):

✅ RAW() false negative — Removed (?!RAW\\() negative lookahead from SECRET_IN_URI regex. password=RAW(secret) is now correctly detected as a plain-text secret, consistent with SecurityUtils.isPlainTextSecret documentation.
✅ Empty insecureValue false positives — Added early continue when insecureValue is null or empty. Options like sslEndpointAlgorithm with empty insecureValue no longer match any assignment.
✅ SQL composite scheme false positive — containsScheme() now checks that the character before the scheme is not a letter, digit, or hyphen. google-bigquery-sql: no longer falsely matches as sql:.
✅ Route-level header filter check — Replaced route-wide text search with per-consumer hasHeaderFilterNearby() that searches the next 20 lines after each consumer declaration (bounded by the next consumer occurrence). A "camel*" in a comment no longer suppresses all warnings.

Suggestion (acknowledged):
5. Code duplication with HardenTools — Valid concern. The two tools serve different purposes (catalog-aware context vs line-level scanning) with different output formats. Extracting shared detection logic is a good follow-up but would need careful API design to serve both use cases.

Claude Code on behalf of oscerd

gnodet

Re-review (new commits since last review)

All 4 blocking findings from the previous review are correctly fixed in the code, and CI passes. The per-consumer hasHeaderFilterNearby() approach is a solid improvement. However, the 4 bug-fix scenarios lack dedicated regression tests, and the refactoring left dead code.

Verified findings (4):

#	Severity	Finding
1	medium	Missing regression tests for 4 fixed bugs (RAW() detection, empty insecureValue, composite scheme, per-consumer header filter)
2	low	Dead `findLineContaining()` method — never called after refactoring
3	low	Operator precedence in `containsScheme()` — explicit parentheses would improve clarity
4	low	No test for LDAP/LDAPS detection (unlike HTTP/FTP/SMTP protocol pairs)

None of these are blocking — the code is correct and the primary use cases are well-tested — but addressing them would strengthen the PR.

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 · 2026-07-24T13:23:37Z

+        }
+        return text.contains("\"" + scheme + "\"") || text.contains("'" + scheme + "'");
+    }
+


Low: Operator precedence readability — the expression idx == 0 || !Character.isLetterOrDigit(text.charAt(idx - 1)) && text.charAt(idx - 1) != '-' is functionally correct due to Java precedence (! > && > ||), but adding explicit parentheses would make the intent immediately clear:

Suggested change

if (idx == 0 || (!Character.isLetterOrDigit(text.charAt(idx - 1)) && text.charAt(idx - 1) != '-')) {

gnodet · 2026-07-24T13:23:37Z

+            if (lines[i].toLowerCase(Locale.ROOT).contains(lower)) {
+                return i + 1;
+            }
+        }


Low: Dead code — findLineContaining() is defined but never called. The refactoring to per-consumer hasHeaderFilterNearby() eliminated its only call site. Can be removed.

Suggested change

}

gnodet · 2026-07-24T13:23:37Z

+            default -> 4;
+        };
+    }
+}


Medium: The 4 bug fixes from this commit lack dedicated regression tests. Per project rules, every PR must include tests for bug fixes. Specifically:

Test that password=RAW(mysecret) IS detected as a plain-text secret (existing detectsPlainTextPasswordInUri uses password=mysecret123 which doesn't exercise the RAW() path)

Test that sslEndpointAlgorithm=HTTPS is NOT flagged (empty insecureValue case)

Test that google-bigquery-sql:project:dataset is NOT flagged as SQL injection (composite scheme)

Test with two consumers where only one has a header filter, verifying the unprotected one is still flagged

Also: no test for LDAP/LDAPS detection, unlike the other protocol pairs (HTTP/FTP/SMTP).

gnodet approved these changes Jul 22, 2026

View reviewed changes

gnodet requested changes Jul 22, 2026

View reviewed changes

github-actions Bot added the dsl label Jul 22, 2026

oscerd requested a review from gnodet July 24, 2026 11:03

gnodet reviewed Jul 24, 2026

View reviewed changes


		private static final Pattern SECRET_IN_URI = Pattern.compile(
		"(?i)(password\|passwd\|pwd\|secret\|token\|apikey\|api[_-]?key\|access[_-]?key)\\s[=:]\\s(?!\\{\\{)(?!\\$\\{)(?!RAW\\()([^\\s,;}'\"\\]&]+)");


	if (idx == 0 \|\| (!Character.isLetterOrDigit(text.charAt(idx - 1)) && text.charAt(idx - 1) != '-')) {

Uh oh!

Conversation

oscerd commented Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

gnodet left a comment

Choose a reason for hiding this comment

Uh oh!

gnodet left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jul 22, 2026

Uh oh!

github-actions Bot commented Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oscerd commented Jul 24, 2026

Uh oh!

gnodet left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

oscerd commented Jul 22, 2026 •

edited

Loading

github-actions Bot commented Jul 22, 2026 •

edited

Loading