[Feature][Connector-V2] Fix HTTP credential handling - add scheme check and exclude token from toString

[zhang-arvin]

Fixes #12025: Two gaps in connector-http-base credential handling:

1. No scheme check on credential tokens
2. Token appears in @DaTa toString() which can leak to logs

Changes

• Added scheme validation for credential tokens
• Excluded token from toString() to prevent accidental log leakage

[DanielLeens]

What Problem Does This PR Solve?

Thanks for tackling this, @zhang-arvin! This PR closes #12025, which flagged two real weak spots in connector-http-base's credential handling: (1) nothing warned a user when they configured an Authorization-style header against a plain http:// URL, so credentials could silently travel in clear text, and (2) HttpParameter is a Lombok @Data class, so its auto-generated toString() used to print the whole headers map — meaning any log statement, exception dump, or debugger inspection that stringified the parameter object would leak the raw token value.

The fix adds a validateCredentialScheme() method to the shared HttpParameter base class that logs a WARN when the URL isn't https:// and a header key looks like an authorization header, and it excludes headers from the generated toString() via @ToString.Exclude. The new validation call is wired into all 13 HTTP-family source connectors plus the generic HttpSink.

Before: http://api.example.com + Authorization: Bearer xxx → no warning, and httpParameter.toString() (e.g. in a stray log line) would print the bearer token in full.
After: the same config logs "The HTTP connector URL '...' uses a non-HTTPS scheme while credential headers are configured...", and toString() no longer includes headers at all.

1. Code Change Review

1.1 Core Logic Analysis

I traced validateCredentialScheme() (HttpParameter.java:103-127) and its call sites across the whole connector-http module (all 13 *SourceParameter classes + HttpSink.java), and found two real gaps that mean the fix does not do what it advertises for a subset of the very connectors it touches:

Issue 1 (High) — hasAuthHeader heuristic misses non-"authorization" credential headers, so the check is a silent no-op for at least 2 of the 13 connectors this PR wires up.

• Location: seatunnel-connectors-v2/connector-http/connector-http-base/.../config/HttpParameter.java:114-119
• Problem: hasAuthHeader only matches header keys whose lowercased form contains the substring "authorization". But:
  • GitlabSourceParameter.java:29-33 puts the token under GitlabSourceOptions.PRIVATE_TOKEN, whose value is "PRIVATE-TOKEN" (GitlabSourceOptions.java:26) — never matches.
  • PersistiqSourceParameter.java:32-33 puts the token under X_API_KEY = "x-api-key" (PersistiqSourceParameter.java:27) — never matches either.
• Potential risk: For GitLab and PersistIQ configured over plain HTTP with real credentials, validateCredentialScheme() runs (it is correctly wired into both classes) but hasAuthHeader is always false, so the warning never fires. Since these are exactly the two connectors singled out by name in this PR's diff, this isn't a hypothetical edge case — it's a verified gap in the very fix being shipped.
• Best improvement: match on a small allow-list of known credential header names (authorization, private-token, x-api-key, etc.) or, more robustly, let each subclass pass the specific header key(s) it just set into validateCredentialScheme(String... credentialHeaderKeys) instead of re-deriving "looks like a credential" from the key name.
• Severity: High
• Raised by another reviewer: No

Issue 2 (Medium) — AirtableSink builds the same Bearer-token headers but never calls validateCredentialScheme().

• Location: seatunnel-connectors-v2/connector-http/connector-http-airtable/.../sink/AirtableSink.java:58-59
• Problem: AirtableSink's constructor independently builds a fresh HttpParameter, sets the URL, and calls AirtableConfig.buildAuthHeaders(token, null) to inject the same Authorization: Bearer ... header that AirtableSourceParameter.java:52 (touched by this PR) protects on the source side. The sink constructor never calls httpParameter.validateCredentialScheme().
• Potential risk: An Airtable sink configured with a plain-HTTP apiBaseUrl gets zero warning, even though the sibling source path for the same connector is fixed in this exact diff.
• Best improvement: add httpParameter.validateCredentialScheme(); after line 59 in AirtableSink.java, mirroring what was already done for HttpSink.java:60.
• Severity: Medium
• Raised by another reviewer: No

Aside from those two gaps, the wiring itself is sound: I checked every *SourceParameter subclass's buildWithConfig() — 11 of 13 call super.buildWithConfig(pluginConfig) (which already runs validateCredentialScheme() once with the base-class headers) and then call it again after adding their own auth header, which is correct and not wasteful (single cheap string scan). StripeSourceParameter and PostHogSourceParameter don't call super.buildWithConfig(), so their own explicit call at the end of buildWithConfig() is the only one that runs — also correct. No subclass declares its own @Data/@ToString, so the @ToString.Exclude on the base class's headers field applies uniformly across the whole family; I checked for stray credential values leaking into the non-excluded params map (e.g. Stripe's pagination limit param) and found none.

1.2 Compatibility Impact

Fully backward compatible. No config option renamed or removed, no default value changed, no exception is thrown (only a WARN log is added), and the new validateCredentialScheme() method is purely additive. The only visible behavior change is that HttpParameter.toString() no longer includes headers — I checked for tests asserting toString() content and found none, so this is safe.

1.3 Performance / Side-Effect Analysis

Negligible. validateCredentialScheme() runs once per connector instance at construction/config time (not per-row, not in the hot read/write path), and it's a single pass over a typically tiny headers map. No new I/O, locks, retries, or allocations of note.

1.4 Error Handling and Logging

Good practice here: the warning is logged at WARN (appropriate — it's not a failure) and the message only includes the URL, never the header value itself, so it doesn't itself become a new leakage vector. No exceptions are swallowed by this change.

2. Code Quality Assessment

2.1 Coding Standards

Issue 3 (High) — the contributor's own fork CI is red on Run / Code style (spotless check) for exactly this change.

• Location: fork Actions run for zhang-arvin/seatunnel@4a96627e — job Run / Code style
• Problem: spotless:check fails on connector-http-base/.../config/HttpParameter.java — the new import ordering and Javadoc line-wrapping don't match the project's Google/AOSP format (e.g. the StringUtils import position and the validateCredentialScheme() Javadoc wrapping).
• Best improvement: run ./mvnw spotless:apply -pl seatunnel-connectors-v2/connector-http/connector-http-base -am and push the reformatted diff.
• Severity: High (this alone is enough to keep CI red)
• Raised by another reviewer: No

Also, HttpParameter.java is missing a trailing newline at end of file per the diff (\ No newline at end of file) — spotless will likely also flag/fix this.

2.2 Test Coverage and Test Stability

Issue 4 (Medium) — no unit tests were added for the new validateCredentialScheme() logic.

• Location: connector-http-base (no new/updated test file in this diff)
• Problem: this is new, branchy, security-adjacent logic (blank-URL short-circuit, https short-circuit, empty-headers short-circuit, case-insensitive key matching) touched by 14 call sites, with zero direct coverage.
• Best improvement: a focused HttpParameterTest covering: https URL (no warning), http URL without auth header (no warning), http URL with an Authorization header (warning), and — once Issue 1 is fixed — a PRIVATE-TOKEN/x-api-key case.
• Severity: Medium
• Raised by another reviewer: No

This PR does not touch any existing test files, so the mandatory stability rating doesn't apply here; I'd simply ask for the coverage above before merge.

2.3 Documentation Updates

No docs/en/docs/zh changes, and I don't think any are strictly required since this only adds passive logging behavior, not a new config option. A one-line mention in the HTTP connector's security/notes section would be a nice-to-have but is non-blocking.

3. Architectural Soundness

3.1 Elegance of the Solution

The overall shape — one shared validation method on the base class, called once headers are finalized — is a sensible, minimal-footprint approach that avoids duplicating logic across 13 modules. The weak point is purely the "does this header look like a credential" heuristic (Issue 1).

3.2 Maintainability

Because every subclass must remember to call validateCredentialScheme() after it finishes mutating headers, it's easy for a future 14th connector (or a sink like AirtableSink) to simply forget the call — which is exactly what happened with AirtableSink in this very PR (Issue 2). Consider making this harder to skip, e.g. by overriding setHeaders() in the base class to auto-trigger validation, or by validating lazily right before the first HTTP call is made in HttpClientProvider, so there's a single choke point that every connector passes through regardless of whether it remembers the explicit call.

3.3 Extensibility

Related to Issue 1: hard-coding "authorization" as the only recognized substring doesn't scale to the variety of credential header conventions already present in this same connector family (PRIVATE-TOKEN, x-api-key). An allow-list or a per-connector-supplied header-name parameter would extend more gracefully to future connectors with yet another convention.

3.4 Historical-Version Compatibility

No concerns — no serialization, checkpoint, or protocol surface is touched.

4. Issue Summary

1. Issue 1 (High) — hasAuthHeader heuristic in HttpParameter.validateCredentialScheme() never matches GitLab's PRIVATE-TOKEN or PersistIQ's x-api-key headers, silently defeating the fix for those two connectors. HttpParameter.java:114-119.
2. Issue 3 (High) — fork CI's Run / Code style (spotless) job is currently failing on HttpParameter.java; needs mvn spotless:apply.
3. Issue 2 (Medium) — AirtableSink.java:58-59 builds Bearer-token headers but never calls validateCredentialScheme(), unlike its sibling source path and HttpSink.
4. Issue 4 (Medium) — no unit tests for the new validation logic.

5. Merge Recommendation

Conclusion: Ready to merge after fixes

1. Blockers - must be fixed
   • Issue 1: broaden the credential-header detection so it actually covers GitLab/PersistIQ (or explain why those are intentionally out of scope).
   • Issue 3: fix the spotless/code-style failure so fork CI goes green.
2. Recommended fixes - non-blocking
   • Issue 2: wire validateCredentialScheme() into AirtableSink for parity with the source side.
   • Issue 4: add focused unit tests for validateCredentialScheme().

Overall this is a genuinely useful hardening PR — the toString() exclusion is a clean, correct fix on its own, and the scheme-check idea is the right direction. The main thing holding it back is that the detection heuristic doesn't actually cover two of the connectors it was wired into, which undercuts the security intent of the change. Once the header-matching gap and the CI failure are addressed, this looks good to me. Thanks again for the contribution — happy to take another look once these are updated!