[TT-1981] Client TLS certificate is required" error should be logged at info level

[shults]

Description

Removed duplicate log

Related Issue

Motivation and Context

How This Has Been Tested

Screenshots (if appropriate)

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Refactoring or add test (improvements in base code or adds test coverage to functionality)

Checklist

• I ensured that the documentation is up to date
• I explained why this PR updates go.mod in detail with reasoning why it's required
• I would like a code coverage CI quality gate exception and have explained why

[probelabs]

This pull request removes a WARN-level log message for client TLS certificate validation failures to eliminate what the author describes as a duplicate log. By deleting the specific logging statement in the CertificateCheckMW middleware, the change defers the logging responsibility to a centralized error handler, which records the event at the INFO level as part of the standard access log. This reduces log verbosity for a common client-side error.

Files Changed Analysis

• gateway/mw_certificate_check.go: The change is confined to this single file. Eight lines have been deleted, consisting of the logrus logging statement that reported certificate validation failures and the now-unused imports for logrus and errpack.

Architecture & Impact Assessment

• What this PR accomplishes
  The PR removes a dedicated WARN log for client certificate validation errors. The responsibility for logging this event is now fully deferred to the generic error handling and access logging mechanism that already exists in the middleware chain.

• Key technical changes introduced
  The log.WithField(...) block within the CertificateCheckMW.ProcessRequest function has been deleted. The error is now simply returned up the middleware chain, where it is caught by a generic handler that logs it as part of the standard access log.

• Affected system components
  This change primarily affects the logging and observability of the Tyk Gateway's mTLS functionality.

  • Before: Certificate validation failures were explicitly logged at the WARN level with specific API context (api_id, api_name) directly from the middleware.
  • After: These failures are now logged at the INFO level as part of the general access log, which is generated by a centralized error handler.

  This change reduces log severity for a common client-side error but also removes a specific, high-visibility warning for a security-relevant event. Debugging mTLS issues will now require parsing INFO-level access logs rather than looking for a distinct WARN message.

  sequenceDiagram
      participant Client
      participant Gateway
      participant CertificateCheckMW
      participant UpstreamErrorHandler
      participant AccessLog
  
      Client->>Gateway: Request with invalid certificate
      Gateway->>CertificateCheckMW: ProcessRequest(r)
      CertificateCheckMW->>CertificateCheckMW: crypto.ValidateRequestCerts(r) fails
      Note right of CertificateCheckMW: Before: Logs WARN message here (This code is removed)
      CertificateCheckMW-->>Gateway: return err, http.StatusForbidden
      Gateway->>UpstreamErrorHandler: Handle error
      UpstreamErrorHandler->>AccessLog: Log request failure (INFO level)
      Note right of AccessLog: After: This is the only log for the failure
      UpstreamErrorHandler-->>Client: Respond with 403 Forbidden
  

  Loading

Scope Discovery & Context Expansion

• The change relies on the existing centralized error handling in the gateway's middleware chain. When CertificateCheckMW.ProcessRequest returns an error, it is caught by an upstream handler that is responsible for logging the request details (as an access log) and writing the HTTP response.
• The primary impact is the trade-off between reducing WARN-level noise for a common client error and the potential loss of immediate visibility into a security-related event. Reviewers should consider if an INFO-level access log provides sufficient observability for this type of failure.

References

• gateway/mw_certificate_check.go: The location of the removed WARN log.

Metadata

• Review Effort: 1 / 5
• Primary Label: chore

---

Powered by Visor from Probelabs

Last updated: 2026-04-17T12:09:36.612Z | Triggered by: pr_updated | Commit: 51e85af

💡 TIP: You can chat with Visor using /visor ask <your question>

[probelabs]

Security Issues (1)

Severity	Location	Issue
🟡 Warning	gateway/mw_certificate_check.go:118-123	Removing the explicit WARN-level log for a failed client certificate validation reduces the visibility of a significant security event. While the failure is logged as part of the generic access log at the INFO level, this change makes it harder to distinguish between routine client errors and potential systematic attacks (e.g., scanning for misconfigurations) without parsing all access logs. Security-sensitive failures like certificate validation should ideally be logged at a higher severity level (WARN or ERROR) to enable effective monitoring and alerting. 💡 Suggestion Consider retaining a specific, higher-severity log for this security-relevant event. If the goal is to reduce verbosity for common client errors, this could be made configurable. Alternatively, ensure that the centralized error logging can tag or elevate logs based on the error type, so that certificate validation failures can still trigger alerts.

✅ Architecture Check Passed

No architecture issues found – changes LGTM.

✅ Performance Check Passed

No performance issues found – changes LGTM.

Quality Issues (1)

Severity	Location	Issue
🟡 Warning	gateway/mw_certificate_check.go:117-122	The `CertificateCheckMW` middleware, which handles security-critical client certificate validation, is being modified without any accompanying tests. While the change removes a log statement, it alters the observable behavior for a security failure. The lack of unit or integration tests for this middleware makes it difficult to verify its correctness and prevent future regressions. 💡 Suggestion Add tests for the `CertificateCheckMW` middleware. The tests should cover both successful validation and failure scenarios, such as when a request has an invalid or missing certificate. Tests should assert that an error is returned and the request is rejected with the correct HTTP status code in failure cases.

---

Powered by Visor from Probelabs

Last updated: 2026-04-17T12:09:09.332Z | Triggered by: pr_updated | Commit: 51e85af

💡 TIP: You can chat with Visor using /visor ask <your question>

[github-actions]

API Changes

no api changes detected

[edsonmichaque]

LGTM

[github-actions]

🚨 Jira Linter Failed

Commit: 51e85af
Failed at: 2026-04-17 12:07:31 UTC

The Jira linter failed to validate your PR. Please check the error details below:

🔍 Click to view error details

failed to get Jira issue: failed to fetch Jira issue TT-1981: Issue does not exist or you do not have permission to see it.: request failed. Please analyze the request body for more details. Status code: 404

Next Steps

• Ensure your branch name contains a valid Jira ticket ID (e.g., ABC-123)
• Verify your PR title matches the branch's Jira ticket ID
• Check that the Jira ticket exists and is accessible

---

This comment will be automatically deleted once the linter passes.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[shults]

/release to release-5.8

[probelabs]

⚠️ Cherry-pick encountered conflicts. A draft PR was created: #8086