fix: RemoteStep writes HTTP response to _http_response_writer on StopPipeline

[Copilot]

External plugin steps (e.g. step.authz_check) signal denial via StopPipeline: true with response_status, response_body, and response_headers in their output — but RemoteStep.Execute was ignoring these fields, leaving the HTTP trigger and openapi handler to send their own default response (202 or 200 with full pipeline context), leaking internal state like tokens and claims.

Changes

• plugin/external/remote_step.go: After a StopPipeline=true response, checks for response_status in the output; if _http_response_writer is present in pipeline metadata, writes the encoded response (headers, status code, body) and sets _response_handled=true. Mirrors the pattern in step.auth_validate for 401 responses.

• plugin/external/remote_step_test.go: Three new test cases covering:

  • 403 status + body + headers are written and _response_handled is set
  • No _http_response_writer in metadata — no panic, _response_handled unset
  • StopPipeline=true without response_status — response writer is left untouched

Before / After

# Before
POST /api/protected  →  200 OK  {"response_status":403, "token":"...", ...}

# After  
POST /api/protected  →  403 Forbidden  {"error":"forbidden: insufficient permissions"}

Original prompt

---

This section details on the original issue you should resolve

<issue_title>step.authz_check: write HTTP 403 response on authorization denial</issue_title>
<issue_description>When step.authz_check denies a request, it returns StopPipeline: true with response_status: 403, response_body, and response_headers in the step output. However, it does NOT write the 403 response to the _http_response_writer.

Both the HTTP trigger handler and the openapi module handler check rw.written after pipeline execution:

• If no step wrote to the ResponseWriter, the HTTP trigger returns 202 {"status": "workflow triggered"}
• If no step wrote to the ResponseWriter, the openapi handler writes 200 OK with result.Current — leaking all pipeline context data (tokens, claims, internal state)

This differs from step.auth_validate, which correctly writes a 401 response to _http_response_writer before stopping the pipeline (see pipeline_step_auth_validate.go:110).

Expected behavior: step.authz_check should write the 403 JSON response to _http_response_writer on denial, matching the pattern used by step.auth_validate.

Reproduce:

1. Configure a pipeline with step.authz_check where the subject is NOT authorized
2. Invoke the pipeline via the openapi module's x-pipeline handler
3. Observe: HTTP response is 200 OK with full pipeline context (including response_status: 403 in the JSON body)
4. Expected: HTTP response is 403 Forbidden with {"error":"forbidden: ..."} body

Workaround: Ensure Casbin policies cover all routes so authz never fails for legitimate requests. There is no way to add a response-writing step after step.authz_check because StopPipeline: true prevents subsequent steps from executing.

Note: step.authz_check is provided by the workflow-plugin-authz external plugin. The fix likely needs to be in that plugin's forbiddenResult() function.</issue_description>

Comments on the Issue (you are @copilot in this section)

• Fixes step.authz_check: write HTTP 403 response on authorization denial #251

---

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

[Copilot]

Pull request overview

This PR ensures that when an external (gRPC) plugin step stops a pipeline and returns an HTTP response payload (status/body/headers) in its output, the engine writes that response to the in-flight http.ResponseWriter so HTTP handlers don’t fall back to default 200/202 responses that may leak pipeline context.

Changes:

• Update RemoteStep.Execute to detect StopPipeline=true + response_status and write headers/status/body to _http_response_writer, setting _response_handled=true.
• Add unit tests covering response-writing behavior, absence of _http_response_writer, and stop-without-response_status.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
plugin/external/remote_step.go	Writes HTTP response from plugin output when StopPipeline is set and a response writer is available.
plugin/external/remote_step_test.go	Adds tests validating the response-writing behavior and guard rails.

[Copilot]

output := structToMap(resp.Output) can be nil when the plugin returns a response with output unset. In the StopPipeline branch the code indexes output["response_status"], which will panic on a nil map. Initialize output to an empty map (or guard output != nil) before reading response_status/response_headers/response_body.

[Copilot]

Add a regression test for the nil-output case: when the plugin returns StopPipeline=true but ExecuteStepResponse.Output is nil, RemoteStep.Execute should not panic and should not attempt to index into the output map. This test will protect the response-writing path once the nil-map panic is fixed.

[intel352]

@copilot apply changes based on the comments in this thread

[intel352]

@copilot apply changes based on the comments in this thread