neiam
/gitgud
Git Gud
public · Issues · Pulls · Labels · Forks · Compare · Actions success · Badges · Packages
⭐
1
Log in to mark this repository.
Downloads of uploaded bytes never render on the forge's origin
e999b5e · Gabriel Morell · 2026-10-20 18:32
Message
{commit_body(@commit)}
Files changed
modified
lib/git_gud_web/controllers/actions_artifact_controller.ex
+6
−1
@@ -146,8 +146,13 @@ defmodule GitGudWeb.ActionsArtifactController do
| 146 | 146 | with {:ok, run_id} <- own_run(conn, run_id), |
| 147 | 147 | %WorkflowArtifact{workflow_run_id: ^run_id} = artifact <- get_artifact(artifact_id), |
| 148 | 148 | {:ok, body} <- Storage.get(@bucket, artifact.storage_key) do |
| 149 | + # Bytes a job uploaded, so never a type a browser would render on | |
| 150 | + # this origin — whatever the row says. See WorkflowArtifactController. | |
| 149 | 151 | conn |
| 150 | − |> put_resp_content_type(artifact.content_type || "application/octet-stream", nil) | |
| 152 | + |> put_resp_content_type("application/octet-stream", nil) | |
| 153 | + |> put_resp_header("content-disposition", "attachment") | |
| 154 | + |> put_resp_header("x-content-type-options", "nosniff") | |
| 155 | + |> put_resp_header("content-security-policy", "default-src 'none'; sandbox") | |
| 151 | 156 | |> send_resp(200, body) |
| 152 | 157 | else |
| 153 | 158 | _ -> send_resp(conn, 404, "") |
modified
lib/git_gud_web/controllers/package_registry_controller.ex
+4
−0
@@ -281,6 +281,10 @@ defmodule GitGudWeb.PackageRegistryController do
| 281 | 281 | conn |
| 282 | 282 | |> put_resp_header("etag", ~s("#{file.sha256}")) |
| 283 | 283 | |> put_resp_header("content-disposition", ~s(attachment; filename="#{file.filename}")) |
| 284 | + # The type comes from the format's own table, never the | |
| 285 | + # uploader; and the :raw pipeline sets no secure headers. | |
| 286 | + |> put_resp_header("x-content-type-options", "nosniff") | |
| 287 | + |> put_resp_header("content-security-policy", "default-src 'none'; sandbox") | |
| 284 | 288 | |> send_body(200, file.content_type || "application/octet-stream", body) |
| 285 | 289 | |
| 286 | 290 | {:error, _} -> |
modified
lib/git_gud_web/controllers/storage_controller.ex
+4
−0
@@ -24,8 +24,12 @@ defmodule GitGudWeb.StorageController do
| 24 | 24 | with {:ok, %{bucket: ^bucket, key: ^key}} <- Disk.verify_token(token, :get), |
| 25 | 25 | path = Disk.object_path(bucket, key), |
| 26 | 26 | true <- File.exists?(path) do |
| 27 | + # Whatever was stored (LFS objects, artifacts), as bytes: on this | |
| 28 | + # origin a browser must neither sniff nor render it. | |
| 27 | 29 | conn |
| 28 | 30 | |> put_resp_content_type("application/octet-stream") |
| 31 | + |> put_resp_header("x-content-type-options", "nosniff") | |
| 32 | + |> put_resp_header("content-security-policy", "default-src 'none'; sandbox") | |
| 29 | 33 | |> send_file(200, path) |
| 30 | 34 | else |
| 31 | 35 | false -> |
modified
lib/git_gud_web/controllers/workflow_artifact_controller.ex
+9
−1
@@ -44,8 +44,16 @@ defmodule GitGudWeb.WorkflowArtifactController do
| 44 | 44 | defp serve(conn, %WorkflowArtifact{} = artifact) do |
| 45 | 45 | case Storage.get(@bucket, artifact.storage_key) do |
| 46 | 46 | {:ok, body} -> |
| 47 | + # What a job uploaded is the job's, served from the forge's own | |
| 48 | + # origin. `content_type` is always octet-stream today, but it is a | |
| 49 | + # column, and an HTML or SVG type here would be stored XSS for | |
| 50 | + # anyone who clicks the link: so the type is fixed, the browser | |
| 51 | + # may not sniff one, and the document is sandboxed should it ever | |
| 52 | + # be rendered anyway. | |
| 47 | 53 | conn |
| 48 | − |> put_resp_content_type(artifact.content_type || "application/octet-stream", nil) | |
| 54 | + |> put_resp_content_type("application/octet-stream", nil) | |
| 55 | + |> put_resp_header("x-content-type-options", "nosniff") | |
| 56 | + |> put_resp_header("content-security-policy", "default-src 'none'; sandbox") | |
| 49 | 57 | |> put_resp_header( |
| 50 | 58 | "content-disposition", |
| 51 | 59 | ~s(attachment; filename="#{filename_safe(artifact.name)}") |
modified
test/git_gud_web/controllers/actions_endpoints_test.exs
+39
−0
@@ -177,6 +177,45 @@ defmodule GitGudWeb.ActionsEndpointsTest do
| 177 | 177 | assert download.resp_body == body |
| 178 | 178 | end |
| 179 | 179 | |
| 180 | + # An artifact is bytes a job uploaded, served from the forge's origin: | |
| 181 | + # an HTML type there would be stored XSS. The row's content_type is | |
| 182 | + # never trusted, even if something wrote one. | |
| 183 | + test "an artifact downloads as an attachment of bytes, whatever its row says", %{ | |
| 184 | + conn: conn, | |
| 185 | + repo: repo, | |
| 186 | + run: run, | |
| 187 | + token: token | |
| 188 | + } do | |
| 189 | + %{"containerId" => id} = | |
| 190 | + Jason.decode!(create_artifact(conn, token, run.id, "x.html").resp_body) | |
| 191 | + | |
| 192 | + assert upload_artifact(conn, token, run.id, id, "<script>alert(1)</script>").status == 200 | |
| 193 | + | |
| 194 | + GitGud.Repo.get!(GitGud.Workflows.WorkflowArtifact, id) | |
| 195 | + |> Ecto.Changeset.change(content_type: "text/html") | |
| 196 | + |> GitGud.Repo.update!() | |
| 197 | + | |
| 198 | + repo = GitGud.Repo.preload(repo, [:owner, :organization]) | |
| 199 | + owner = repo.owner | |
| 200 | + | |
| 201 | + web = | |
| 202 | + build_conn() | |
| 203 | + |> log_in_user(owner) | |
| 204 | + |> get( | |
| 205 | + "/r/#{GitGud.Repositories.Storage.repo_handle(repo)}/#{repo.name}" <> | |
| 206 | + "/actions/#{run.number}/artifacts/#{id}" | |
| 207 | + ) | |
| 208 | + | |
| 209 | + for resp <- [web, download_artifact(conn, token, run.id, id)] do | |
| 210 | + assert resp.status == 200 | |
| 211 | + assert get_resp_header(resp, "content-type") == ["application/octet-stream"] | |
| 212 | + assert ["attachment" <> _] = get_resp_header(resp, "content-disposition") | |
| 213 | + assert get_resp_header(resp, "x-content-type-options") == ["nosniff"] | |
| 214 | + assert [csp] = get_resp_header(resp, "content-security-policy") | |
| 215 | + assert csp =~ "sandbox" | |
| 216 | + end | |
| 217 | + end | |
| 218 | + | |
| 180 | 219 | test "list artifacts returns the persisted set", %{conn: conn, run: run, token: token} do |
| 181 | 220 | auth = fn c -> put_req_header(c, "authorization", "Bearer " <> token) end |
| 182 | 221 |
Parents: 9f1041f