oscerd commented on PR #26268: URL: https://github.com/apache/camel/pull/26268#issuecomment-5661067822
Thanks @davsclaus and @gnodet for the thorough reviews — pushed a commit addressing the outstanding findings. **1 — header metadata contradicted the code:** `SqlConstants.SQL_QUERY` and `SqlStoredConstants.SQL_STORED_TEMPLATE` now state the header is ignored unless the endpoint enables `allowQueryFromHeader` / `allowTemplateFromHeader`. Metadata + catalog regenerated. **2 — no component-level option:** added `allowQueryFromHeader` to `SqlComponent` and `allowTemplateFromHeader` to `SqlStoredComponent`, propagated Component → Endpoint in `createEndpoint` before `setProperties(...)` so a per-endpoint URI option still overrides the component default. Migration is now a single `camel.component.sql.allow-query-from-header=true` / `camel.component.sql-stored.allow-template-from-header=true` switch instead of editing every URI. **6 + 8 — the sql-stored gate test didn't gate:** reworked `SqlStoredAllowTemplateFromHeaderTest`. Both routes call the same `SUBNUMBERS` procedure, but the endpoint template and the `CamelSqlStoredTemplate` header alias the OUT parameter differently (`resultfromendpoint` vs `resultfromheader`), so the key present in the result map proves which template actually executed. `headerTemplateIgnoredByDefault` (gate off) asserts the endpoint alias ran and the header alias is absent; `headerTemplateHonouredWhenAllowed` (gate on) asserts the reverse — mutation-resistant in both directions, and it doubles as the missing positive test. **9 + 10 — missing security marker:** both `@UriParam`s and the two new component `@Metadata` options now carry `security = "insecure:dev", insecureValue = "true"` (and the sql one is `label = "producer,security"`), so `camel.main.profile=prod` can enforce the policy. **3 — observability reads `CamelSqlQuery` unconditionally:** filed as a follow-up, CAMEL-24718. It spans `camel-console` (`SqlTraceDevConsole`, which also serves `jdbc:`, so it must consult the endpoint's `allowQueryFromHeader` rather than drop the header), plus `camel-telemetry` and `camel-tracing` (`SqlSpanDecorator`), so it's cleaner as its own change than folded in here. **5 — `CamelSqlDataSource`:** consciously out of scope for CAMEL-24292. It carries a `DataSource` object (route/operator-supplied), not attacker-influenced query text, so it isn't a resource-resolution or statement-injection vector. **7 — stale PR description:** updated. `components/camel-sql` builds clean and the unit tests pass; the only local failure is `SqlFunctionDataSourceTest`, which needs an embedded MariaDB that won't install in my environment (unrelated to this change). Still a draft pending the PMC call on the default — thanks for the deny-by-default endorsements, which line up with the documented trust boundary and the sibling template components. _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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
