From 1723a64da9d441954c7a64f2564eafad898a48c4 Mon Sep 17 00:00:00 2001 From: Weslei Juan Moser Pereira Date: Wed, 11 Jun 2025 18:37:15 -0300 Subject: [PATCH 1/5] fix: add missing content-type headers in public api --- .../v1alpha/lib/pipelines_api/logs/get.ex | 5 ++-- .../lib/pipelines_api/pipelines/common.ex | 11 ++++--- .../v1alpha/lib/pipelines_api/router.ex | 19 ++++++++---- .../lib/pipelines_api/util/api_response.ex | 30 +++++++++++++++++++ 4 files changed, 54 insertions(+), 11 deletions(-) create mode 100644 public-api/v1alpha/lib/pipelines_api/util/api_response.ex diff --git a/public-api/v1alpha/lib/pipelines_api/logs/get.ex b/public-api/v1alpha/lib/pipelines_api/logs/get.ex index 76888f02e..3bbb3e182 100644 --- a/public-api/v1alpha/lib/pipelines_api/logs/get.ex +++ b/public-api/v1alpha/lib/pipelines_api/logs/get.ex @@ -49,7 +49,8 @@ defmodule PipelinesAPI.Logs.Get do {:ok, token} -> conn |> put_resp_header("location", build_loghub2_url(conn, job_id, token)) - |> send_resp(conn.status || 302, "") + |> put_status(conn.status || 302) + |> text("") e -> RespCommon.respond(e, conn) @@ -73,7 +74,7 @@ defmodule PipelinesAPI.Logs.Get do end defp prepare_response(events) do - Enum.join(['{ "events": [', Enum.join(events, ","), "] }"], "") + Enum.join([~c'{ "events": [', Enum.join(events, ","), "] }"], "") end defp build_loghub2_url(conn, job_id, token) do diff --git a/public-api/v1alpha/lib/pipelines_api/pipelines/common.ex b/public-api/v1alpha/lib/pipelines_api/pipelines/common.ex index 6d8a2e67f..733df1874 100644 --- a/public-api/v1alpha/lib/pipelines_api/pipelines/common.ex +++ b/public-api/v1alpha/lib/pipelines_api/pipelines/common.ex @@ -2,6 +2,7 @@ defmodule PipelinesAPI.Pipelines.Common do @moduledoc false import Plug.Conn + import PipelinesAPI.Util.APIResponse def respond(state, conn, encode? \\ true) @@ -28,13 +29,15 @@ defmodule PipelinesAPI.Pipelines.Common do # Sobelow is always warning when using send_resp when value is not actual hardcoded string nor encoded, # json encoded content is safe to use send_resp. # sobelow_skip ["XSS.SendResp"] - defp respond_(conn, code, content, _encode? = true), - do: send_resp(conn, code, content |> Poison.encode!()) + defp respond_(conn, code, content, _encode? = true) do + json(conn, content |> Poison.encode!()) + end # The Sobelow.XSS.SendResp is focused on Phoenix apps so it does not recognise Plug.HTML.html_escape/1 # sobelow_skip ["XSS.SendResp"] - defp respond_(conn, code, content, _encode? = false), - do: send_resp(conn, code, Plug.HTML.html_escape(content)) + defp respond_(conn, code, content, _encode? = false) do + text(conn, code, Plug.HTML.html_escape(content)) + end def respond_paginated({:error, e}, conn), do: respond({:error, e}, conn) diff --git a/public-api/v1alpha/lib/pipelines_api/router.ex b/public-api/v1alpha/lib/pipelines_api/router.ex index bd241d8ab..40cc6780b 100644 --- a/public-api/v1alpha/lib/pipelines_api/router.ex +++ b/public-api/v1alpha/lib/pipelines_api/router.ex @@ -1,6 +1,8 @@ defmodule PipelinesAPI.Router do use Plug.{Router, ErrorHandler} + import PipelinesAPI.Util.APIResponse + alias PipelinesAPI.Pipelines.Terminate alias PipelinesAPI.Pipelines.DescribeTopology alias PipelinesAPI.Pipelines.PartialRebuild @@ -156,7 +158,7 @@ defmodule PipelinesAPI.Router do match("/logs/:job_id", via: :get, to: PipelinesAPI.Logs.Get) get "/health_check/ping" do - send_resp(conn, 200, "pong") + text(conn, "pong") end # sobelow_skip ["XSS.SendResp"] @@ -167,20 +169,27 @@ defmodule PipelinesAPI.Router do case reason.__struct__ do Plug.Parsers.ParseError -> %{exception: %{message: message}} = reason - send_resp(conn, conn.status, "Malformed request: " <> Plug.HTML.html_escape(message)) + + conn + |> put_status(conn.status) + |> text("Malformed request: " <> Plug.HTML.html_escape(message)) _ -> - send_resp(conn, conn.status, "Something went wrong") + conn + |> put_status(conn.status) + |> text("Something went wrong") end end # Root path has to return 200 OK in order to pass health checks made by ingress # on Kubernets get "/" do - send_resp(conn, 200, "pong") + text(conn, "pong") end match _ do - send_resp(conn, 404, "oops") + conn + |> put_status(404) + |> text("oops") end end diff --git a/public-api/v1alpha/lib/pipelines_api/util/api_response.ex b/public-api/v1alpha/lib/pipelines_api/util/api_response.ex new file mode 100644 index 000000000..8f8e1df57 --- /dev/null +++ b/public-api/v1alpha/lib/pipelines_api/util/api_response.ex @@ -0,0 +1,30 @@ +defmodule PipelinesAPI.Util.APIResponse do + import Plug.Conn + + @doc """ + Sends JSON response. + + + iex> json(conn, %{id: 123}) + + """ + @spec json(Plug.Conn.t(), term) :: Plug.Conn.t() + def json(conn, data) do + send_resp(conn, conn.status || 200, "application/json", data) + end + + @doc """ + Sends text response. + + ## Examples + + iex> text(conn, "hello") + + iex> text(conn, :implements_to_string) + + """ + @spec text(Plug.Conn.t(), String.Chars.t()) :: Plug.Conn.t() + def text(conn, data) do + send_resp(conn, conn.status || 200, "text/plain", to_string(data)) + end +end From 927edcfa3dcf32f231f04e30dd03b117e652e83e Mon Sep 17 00:00:00 2001 From: Weslei Juan Moser Pereira Date: Wed, 11 Jun 2025 18:40:15 -0300 Subject: [PATCH 2/5] fix: move json encoding to helper function --- public-api/v1alpha/lib/pipelines_api/pipelines/common.ex | 2 +- public-api/v1alpha/lib/pipelines_api/util/api_response.ex | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/public-api/v1alpha/lib/pipelines_api/pipelines/common.ex b/public-api/v1alpha/lib/pipelines_api/pipelines/common.ex index 733df1874..ec0bfd112 100644 --- a/public-api/v1alpha/lib/pipelines_api/pipelines/common.ex +++ b/public-api/v1alpha/lib/pipelines_api/pipelines/common.ex @@ -30,7 +30,7 @@ defmodule PipelinesAPI.Pipelines.Common do # json encoded content is safe to use send_resp. # sobelow_skip ["XSS.SendResp"] defp respond_(conn, code, content, _encode? = true) do - json(conn, content |> Poison.encode!()) + json(conn, content) end # The Sobelow.XSS.SendResp is focused on Phoenix apps so it does not recognise Plug.HTML.html_escape/1 diff --git a/public-api/v1alpha/lib/pipelines_api/util/api_response.ex b/public-api/v1alpha/lib/pipelines_api/util/api_response.ex index 8f8e1df57..400042d0d 100644 --- a/public-api/v1alpha/lib/pipelines_api/util/api_response.ex +++ b/public-api/v1alpha/lib/pipelines_api/util/api_response.ex @@ -10,7 +10,8 @@ defmodule PipelinesAPI.Util.APIResponse do """ @spec json(Plug.Conn.t(), term) :: Plug.Conn.t() def json(conn, data) do - send_resp(conn, conn.status || 200, "application/json", data) + content = Poison.encode!(data) + send_resp(conn, conn.status || 200, "application/json", content) end @doc """ From 2c70261a4ae294a4a6d90cc6959e04fa02ff72a2 Mon Sep 17 00:00:00 2001 From: Weslei Juan Moser Pereira Date: Wed, 11 Jun 2025 18:47:20 -0300 Subject: [PATCH 3/5] fix: add missing import --- public-api/v1alpha/lib/pipelines_api/logs/get.ex | 2 ++ 1 file changed, 2 insertions(+) diff --git a/public-api/v1alpha/lib/pipelines_api/logs/get.ex b/public-api/v1alpha/lib/pipelines_api/logs/get.ex index 3bbb3e182..7b0067e9c 100644 --- a/public-api/v1alpha/lib/pipelines_api/logs/get.ex +++ b/public-api/v1alpha/lib/pipelines_api/logs/get.ex @@ -5,6 +5,8 @@ defmodule PipelinesAPI.Logs.Get do use Plug.Builder + import PipelinesAPI.Util.APIResponse + require Logger alias PipelinesAPI.Pipelines.Common, as: RespCommon alias PipelinesAPI.Util.{Metrics} From 43bdfd1893be85753592cf13827ee10cae65cec2 Mon Sep 17 00:00:00 2001 From: Weslei Juan Moser Pereira Date: Wed, 11 Jun 2025 18:54:27 -0300 Subject: [PATCH 4/5] fix: function arity --- public-api/v1alpha/lib/pipelines_api/pipelines/common.ex | 4 +++- public-api/v1alpha/lib/pipelines_api/util/api_response.ex | 6 ++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/public-api/v1alpha/lib/pipelines_api/pipelines/common.ex b/public-api/v1alpha/lib/pipelines_api/pipelines/common.ex index ec0bfd112..2605de348 100644 --- a/public-api/v1alpha/lib/pipelines_api/pipelines/common.ex +++ b/public-api/v1alpha/lib/pipelines_api/pipelines/common.ex @@ -36,7 +36,9 @@ defmodule PipelinesAPI.Pipelines.Common do # The Sobelow.XSS.SendResp is focused on Phoenix apps so it does not recognise Plug.HTML.html_escape/1 # sobelow_skip ["XSS.SendResp"] defp respond_(conn, code, content, _encode? = false) do - text(conn, code, Plug.HTML.html_escape(content)) + conn + |> put_status(code) + |> text(Plug.HTML.html_escape(content)) end def respond_paginated({:error, e}, conn), do: respond({:error, e}, conn) diff --git a/public-api/v1alpha/lib/pipelines_api/util/api_response.ex b/public-api/v1alpha/lib/pipelines_api/util/api_response.ex index 400042d0d..33e0fe3a2 100644 --- a/public-api/v1alpha/lib/pipelines_api/util/api_response.ex +++ b/public-api/v1alpha/lib/pipelines_api/util/api_response.ex @@ -1,6 +1,4 @@ defmodule PipelinesAPI.Util.APIResponse do - import Plug.Conn - @doc """ Sends JSON response. @@ -11,7 +9,7 @@ defmodule PipelinesAPI.Util.APIResponse do @spec json(Plug.Conn.t(), term) :: Plug.Conn.t() def json(conn, data) do content = Poison.encode!(data) - send_resp(conn, conn.status || 200, "application/json", content) + Plug.Conn.send_resp(conn, conn.status || 200, "application/json", content) end @doc """ @@ -26,6 +24,6 @@ defmodule PipelinesAPI.Util.APIResponse do """ @spec text(Plug.Conn.t(), String.Chars.t()) :: Plug.Conn.t() def text(conn, data) do - send_resp(conn, conn.status || 200, "text/plain", to_string(data)) + Plug.Conn.send_resp(conn, conn.status || 200, "text/plain", to_string(data)) end end From cef9304fe9f427a2cce52cdd952ec9b1b7d254b1 Mon Sep 17 00:00:00 2001 From: Weslei Juan Moser Pereira Date: Wed, 11 Jun 2025 19:04:47 -0300 Subject: [PATCH 5/5] fix: add helper function for api response and credo issues --- .../lib/pipelines_api/util/api_response.ex | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/public-api/v1alpha/lib/pipelines_api/util/api_response.ex b/public-api/v1alpha/lib/pipelines_api/util/api_response.ex index 33e0fe3a2..0e25327ac 100644 --- a/public-api/v1alpha/lib/pipelines_api/util/api_response.ex +++ b/public-api/v1alpha/lib/pipelines_api/util/api_response.ex @@ -1,4 +1,6 @@ defmodule PipelinesAPI.Util.APIResponse do + @moduledoc false + @doc """ Sends JSON response. @@ -9,7 +11,7 @@ defmodule PipelinesAPI.Util.APIResponse do @spec json(Plug.Conn.t(), term) :: Plug.Conn.t() def json(conn, data) do content = Poison.encode!(data) - Plug.Conn.send_resp(conn, conn.status || 200, "application/json", content) + send_resp(conn, conn.status || 200, "application/json", content) end @doc """ @@ -24,6 +26,21 @@ defmodule PipelinesAPI.Util.APIResponse do """ @spec text(Plug.Conn.t(), String.Chars.t()) :: Plug.Conn.t() def text(conn, data) do - Plug.Conn.send_resp(conn, conn.status || 200, "text/plain", to_string(data)) + send_resp(conn, conn.status || 200, "text/plain", to_string(data)) + end + + defp send_resp(conn, default_status, default_content_type, body) do + conn + |> ensure_resp_content_type(default_content_type) + |> Plug.Conn.send_resp(conn.status || default_status, body) + end + + defp ensure_resp_content_type(%Plug.Conn{resp_headers: resp_headers} = conn, content_type) do + if List.keyfind(resp_headers, "content-type", 0) do + conn + else + content_type = content_type <> "; charset=utf-8" + %Plug.Conn{conn | resp_headers: [{"content-type", content_type} | resp_headers]} + end end end