[TT-5070] Duplicate WriteHeader calls cause malformed responses from golang plugins

[shults]

Description

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 resolves a critical bug that caused panics when a Go plugin writes its own HTTP response. The issue stemmed from duplicate WriteHeader calls: one from the plugin and a subsequent one from the gateway's generic middleware handler when the plugin returned an error. This would result in a superfluous WriteHeader call error and a malformed response.

The fix introduces a control-flow mechanism using two new sentinel errors, ErrResponseSucceed and ErrResponseErrorSent. The Go plugin middleware now catches when a plugin writes to the ResponseWriter and returns one of these specific errors. The generic middleware handler has been updated to recognize these errors, preventing it from attempting to write a second response and allowing the plugin's response to be sent correctly.

Files Changed Analysis

• gateway/mw_go_plugin.go: Introduces ErrResponseSucceed and ErrResponseErrorSent. The handlePluginResponse and handleErrorResponse functions are updated to return these errors when the plugin has already written a response, effectively signaling the state to the upstream middleware handler.
• gateway/middleware.go: The core middleware runner is modified to check for the new sentinel errors. It now correctly interprets ErrResponseSucceed as a successful termination and avoids its own error handling logic when ErrResponseErrorSent is received, preventing the duplicate write.
• goplugin/mw_go_plugin_test.go: Adds a comprehensive new test suite, TestGoPlugin_DontWriteBodyInCaseIfPluginRespondsWith4xxOrHigher, which validates the fix for both API-level and endpoint-level plugins. It ensures that responses sent by plugins (with and without bodies) are handled correctly without causing panics.
• test/goplugins/test_goplugin.go: Adds two new plugin functions, RejectWithBody and RejectWithoutBody, used by the new integration tests to simulate plugins sending their own error responses.
• Makefile: A new build-plugins target has been added to simplify the process of compiling the Go plugins required for the new tests.

Architecture & Impact Assessment

• What this PR accomplishes: It fixes a stability issue that caused the gateway to panic when Go plugins took control of the HTTP response. This makes the Go plugin functionality more robust and reliable for custom response handling.
• Key technical changes introduced: The primary change is the use of sentinel errors for inter-middleware communication and control flow. This serves as a non-breaking workaround to a limitation in the TykMiddleware interface, which lacks a mechanism to signal that a response has already been written.
• Affected system components: The changes directly affect the gateway's middleware processing engine and the Go plugin middleware implementation. It establishes a clearer, safer contract for how Go plugins can manage the HTTP response lifecycle.

Middleware Control Flow

sequenceDiagram
    participant Client
    participant MiddlewareHandler
    participant GoPluginMiddleware
    participant Plugin

    Client->>+MiddlewareHandler: Sends Request
    MiddlewareHandler->>+GoPluginMiddleware: ProcessRequest(w, r)
    GoPluginMiddleware->>+Plugin: Executes plugin function

    alt Plugin sends custom error response
        Plugin-->>Plugin: w.WriteHeader(403)#59; w.Write(...)
        Plugin-->>-GoPluginMiddleware: returns
        GoPluginMiddleware-->>-MiddlewareHandler: return ErrResponseErrorSent
        MiddlewareHandler-->>MiddlewareHandler: Detects ErrResponseErrorSent, skips its own error response
        MiddlewareHandler-->>-Client: Returns plugin's 403 response
    else Plugin succeeds and sends response
        Plugin-->>Plugin: w.WriteHeader(200)#59; w.Write(...)
        Plugin-->>-GoPluginMiddleware: returns
        GoPluginMiddleware-->>-MiddlewareHandler: return ErrResponseSucceed
        MiddlewareHandler-->>MiddlewareHandler: Detects ErrResponseSucceed, stops processing
        MiddlewareHandler-->>-Client: Returns plugin's 200 response
    end

Loading

Scope Discovery & Context Expansion

The fix is well-contained within the Go plugin middleware system. However, it highlights a design limitation in the generic TykMiddleware interface. The ProcessRequest method signature does not natively support returning the state of the ResponseWriter, necessitating the sentinel error workaround.

While this PR effectively solves the problem for Go plugins, any other custom middleware that might need to write a response directly could face the same issue. A future architectural improvement could involve refactoring the TykMiddleware interface to return a more expressive type that explicitly signals the response state (e.g., ProcessRequest(...) (error, bool) where the boolean indicates responseWritten). This would be a breaking change but would create a more robust and explicit contract for all middleware.

Metadata

• Review Effort: 2 / 5
• Primary Label: bug

---

Powered by Visor from Probelabs

Last updated: 2026-04-15T04:02:16.022Z | Triggered by: pr_updated | Commit: 7d14a28

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

[probelabs]

✅ Security Check Passed

No security issues found – changes LGTM.

✅ Performance Check Passed

No performance issues found – changes LGTM.

Quality Issues (1)

Severity	Location	Issue
🟠 Error	gateway/middleware.go:183-196	The changes introduce new error handling logic and sentinel errors (`ErrResponseSucceed`, `ErrResponseErrorSent`) to manage the request lifecycle in middleware, but no corresponding tests have been added. Modifying the core middleware processing loop without automated tests is risky, can lead to regressions, and makes future refactoring difficult. 💡 Suggestion Add unit and/or integration tests to cover the new logic. Specifically: 1. A test where a middleware returns `ErrResponseSucceed` to ensure the error is handled as a success condition and `HandleError` is not called. 2. A test where a middleware returns an error wrapped with `ErrResponseErrorSent` to ensure the generic error handler is called but does not attempt to write another response (`writeResponse` is false). 3. A test where a middleware returns a standard error to ensure the existing error handling path works as expected (`writeResponse` is true). 4. Add unit tests for the changed functions in `gateway/mw_go_plugin.go` to verify they return the new sentinel errors under the correct conditions.

---

Powered by Visor from Probelabs

Last updated: 2026-04-15T04:01:56.460Z | Triggered by: pr_updated | Commit: 7d14a28

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

[github-actions]

API Changes

--- prev.txt	2026-04-15 04:02:03.454609934 +0000
+++ current.txt	2026-04-15 04:01:55.014563200 +0000
@@ -9489,6 +9489,18 @@
 	ErrNoMatchingKIDFound    = errors.New("no matching KID could be found")
 )
 var (
+	ErrResponseSucceed = errpack.New(
+		"response succeed",
+		errpack.WithType(errpack.TypeDomain),
+		errpack.WithLogLevel(logrus.TraceLevel),
+	)
+	ErrResponseErrorSent = errpack.New(
+		"plugin error",
+		errpack.WithType(errpack.TypeDomain),
+		errpack.WithLogLevel(logrus.DebugLevel),
+	)
+)
+var (
 	ProxyingRequestFailedErr     = errors.New("there was a problem proxying the request")
 	GraphQLDepthLimitExceededErr = errors.New("depth limit exceeded")
 )
@@ -15082,6 +15094,14 @@
 func MyResponsePluginAccessingOASAPI2(rw http.ResponseWriter, _ *http.Response, req *http.Request)
     MyResponsePluginAccessingOASAPI2 fake plugin which modifies headers
 
+func RejectWithBody(
+	rw http.ResponseWriter,
+	_ *http.Request,
+)
+func RejectWithoutBody(
+	rw http.ResponseWriter,
+	_ *http.Request,
+)
 # Package: ./tests/accesslog
 
 # Package: ./tests/coprocess

[github-actions]

🚨 Jira Linter Failed

Commit: 7d14a28
Failed at: 2026-04-15 04:01:02 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-5070: 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
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud