neiam
/gitgud
Git Gud
public · Issues · Pulls · Labels · Forks · Compare · Actions success · Badges · Packages
⭐
1
Log in to mark this repository.
Runners report only their own jobs; caches take uploads from their reserving job; registry tokens sign only repository scopes
dcb7d88 · Gabriel Morell · 2026-10-18 18:50
Message
{commit_body(@commit)}
Files changed
modified
lib/git_gud/workflows/actions_cache.ex
+11
−1
@@ -126,8 +126,12 @@ defmodule GitGud.Workflows.ActionsCache do
| 126 | 126 | `(repo, key, version, ref)` unique key — re-uploading the same |
| 127 | 127 | triple replaces the prior row's storage_key and clears the |
| 128 | 128 | archived_at, so lookups won't return an outdated entry mid-upload. |
| 129 | + | |
| 130 | + `job_id` is the reserving job: only it may then upload to and finalize | |
| 131 | + the entry (`reserved_by?/2`). A re-reservation hands the row to the | |
| 132 | + job that made it. | |
| 129 | 133 | """ |
| 130 | − def reserve(%Repository{id: rid}, key, version, ref) do | |
| 134 | + def reserve(%Repository{id: rid}, key, version, ref, job_id \\ nil) do | |
| 131 | 135 | {:ok, row} = |
| 132 | 136 | Repo.insert( |
| 133 | 137 | %CacheEntry{} |
@@ -136,6 +140,7 @@ defmodule GitGud.Workflows.ActionsCache do
| 136 | 140 | key: key, |
| 137 | 141 | version: version, |
| 138 | 142 | ref: ref, |
| 143 | + reserved_by_job_id: job_id, | |
| 139 | 144 | # We'll patch storage_key with the real value once we know the row id. |
| 140 | 145 | storage_key: "pending" |
| 141 | 146 | }), |
@@ -144,6 +149,7 @@ defmodule GitGud.Workflows.ActionsCache do
| 144 | 149 | storage_key: "pending", |
| 145 | 150 | archived_at: nil, |
| 146 | 151 | size: nil, |
| 152 | + reserved_by_job_id: job_id, | |
| 147 | 153 | updated_at: DateTime.utc_now(:second) |
| 148 | 154 | ] |
| 149 | 155 | ], |
@@ -170,6 +176,10 @@ defmodule GitGud.Workflows.ActionsCache do
| 170 | 176 | |
| 171 | 177 | def get(id), do: Repo.get(CacheEntry, id) |
| 172 | 178 | |
| 179 | + @doc "Whether job `job_id` reserved `entry` (and so may upload to it)." | |
| 180 | + def reserved_by?(%CacheEntry{reserved_by_job_id: jid}, job_id), | |
| 181 | + do: is_integer(jid) and jid == job_id | |
| 182 | + | |
| 173 | 183 | @doc """ |
| 174 | 184 | Store one uploaded chunk of a pending entry: the bytes at `offset` |
| 175 | 185 | (from the request's `Content-Range`). Each chunk is its own object and |
modified
lib/git_gud/workflows/cache_entry.ex
+2
−1
@@ -12,6 +12,7 @@ defmodule GitGud.Workflows.CacheEntry do
| 12 | 12 | field :size, :integer |
| 13 | 13 | field :archived_at, :utc_datetime |
| 14 | 14 | field :last_used_at, :utc_datetime |
| 15 | + field :reserved_by_job_id, :id | |
| 15 | 16 | |
| 16 | 17 | belongs_to :repository, Repository |
| 17 | 18 |
@@ -20,7 +21,7 @@ defmodule GitGud.Workflows.CacheEntry do
| 20 | 21 | |
| 21 | 22 | def reserve_changeset(struct, attrs) do |
| 22 | 23 | struct |
| 23 | − |> cast(attrs, [:repository_id, :key, :version, :ref, :storage_key]) | |
| 24 | + |> cast(attrs, [:repository_id, :key, :version, :ref, :storage_key, :reserved_by_job_id]) | |
| 24 | 25 | |> validate_required([:repository_id, :key, :version, :storage_key]) |
| 25 | 26 | |> validate_length(:key, max: 600) |
| 26 | 27 | |> validate_length(:version, max: 128) |
modified
lib/git_gud_web/controllers/actions_cache_controller.ex
+16
−6
@@ -13,8 +13,9 @@ defmodule GitGudWeb.ActionsCacheController do
| 13 | 13 | Auth via the per-task `RunnerJwt` plug. The JWT carries |
| 14 | 14 | `pid` (repository id) and `rid` (workflow run id); we use the run to |
| 15 | 15 | resolve the current ref and the fallback refs (a PR's base branch, |
| 16 | − the default branch). Uploads and finalize only touch entries of the | |
| 17 | − token's repository. | |
| 16 | + the default branch). Uploads and finalize only touch an entry the | |
| 17 | + token's own job reserved: another job of the same repository — a fork | |
| 18 | + pull request's, say — can't write into a default-branch job's upload. | |
| 18 | 19 | """ |
| 19 | 20 | |
| 20 | 21 | use GitGudWeb, :controller |
@@ -66,7 +67,7 @@ defmodule GitGudWeb.ActionsCacheController do
| 66 | 67 | with repo_id when is_integer(repo_id) <- repo_id, |
| 67 | 68 | repo = Repositories.get_repository!(repo_id), |
| 68 | 69 | {ref, _fallbacks} <- refs_for(conn), |
| 69 | − {:ok, entry} <- ActionsCache.reserve(repo, key, version, ref) do | |
| 70 | + {:ok, entry} <- ActionsCache.reserve(repo, key, version, ref, jwt_job_id(conn)) do | |
| 70 | 71 | body = |
| 71 | 72 | Jason.encode!(%{ |
| 72 | 73 | "cacheId" => entry.id, |
@@ -123,13 +124,15 @@ defmodule GitGudWeb.ActionsCacheController do
| 123 | 124 | |
| 124 | 125 | # ── helpers ──────────────────────────────────────────────────────── |
| 125 | 126 | |
| 126 | − # The entry, if it belongs to the token's repository. Anything else is | |
| 127 | − # a 404, so a job can't learn which ids exist elsewhere. | |
| 127 | + # The entry, if the token's job reserved it (in the token's repository). | |
| 128 | + # Anything else is a 404, so a job can't learn which ids exist | |
| 129 | + # elsewhere. | |
| 128 | 130 | defp own_entry(conn, id) do |
| 129 | 131 | repo_id = jwt_repo_id(conn) |
| 130 | 132 | |
| 131 | 133 | with {n, ""} <- Integer.parse(id), |
| 132 | − %{repository_id: ^repo_id} = entry when is_integer(repo_id) <- ActionsCache.get(n) do | |
| 134 | + %{repository_id: ^repo_id} = entry when is_integer(repo_id) <- ActionsCache.get(n), | |
| 135 | + true <- ActionsCache.reserved_by?(entry, jwt_job_id(conn)) do | |
| 133 | 136 | {:ok, entry} |
| 134 | 137 | else |
| 135 | 138 | _ -> {:error, :not_found} |
@@ -163,6 +166,13 @@ defmodule GitGudWeb.ActionsCacheController do
| 163 | 166 | |> send_resp(500, Jason.encode!(%{"error" => inspect(reason)})) |
| 164 | 167 | end |
| 165 | 168 | |
| 169 | + defp jwt_job_id(conn) do | |
| 170 | + case conn.assigns[:runner_jwt_claims] do | |
| 171 | + %{"tid" => tid} when is_integer(tid) -> tid | |
| 172 | + _ -> nil | |
| 173 | + end | |
| 174 | + end | |
| 175 | + | |
| 166 | 176 | defp jwt_repo_id(conn) do |
| 167 | 177 | case conn.assigns[:runner_jwt_claims] do |
| 168 | 178 | %{"pid" => pid} when is_integer(pid) -> pid |
modified
lib/git_gud_web/controllers/registry_controller.ex
+13
−5
@@ -36,7 +36,13 @@ defmodule GitGudWeb.RegistryController do
| 36 | 36 | |
| 37 | 37 | def token(conn, params) do |
| 38 | 38 | principal = Auth.authenticate(conn) |
| 39 | − access = Enum.map(List.wrap(params["scope"]), &grant(principal, &1)) | |
| 39 | + | |
| 40 | + access = | |
| 41 | + params["scope"] | |
| 42 | + |> List.wrap() | |
| 43 | + |> Enum.flat_map(&String.split(&1, " ", trim: true)) | |
| 44 | + |> Enum.map(&grant(principal, &1)) | |
| 45 | + |> Enum.reject(&is_nil/1) | |
| 40 | 46 | |
| 41 | 47 | case Token.issue(subject(principal), access) do |
| 42 | 48 | {:ok, jwt, claims} -> |
@@ -54,9 +60,11 @@ defmodule GitGudWeb.RegistryController do
| 54 | 60 | end |
| 55 | 61 | end |
| 56 | 62 | |
| 57 | − # The requested scope, cut down to the actions `principal` may perform. | |
| 58 | − # Scopes for anything but a repository pass through untouched; the token | |
| 59 | − # carries repository access only. | |
| 63 | + # The requested scope, cut down to the actions `principal` may perform, | |
| 64 | + # or nil for a scope this endpoint doesn't understand. Only repository | |
| 65 | + # scopes are ever signed: anything else (`registry:catalog:*`, a | |
| 66 | + # `repository(plugin):` class) is dropped here rather than signed as | |
| 67 | + # asked and left for something downstream to refuse. | |
| 60 | 68 | defp grant(principal, scope) do |
| 61 | 69 | case String.split(scope, ":", parts: 3) do |
| 62 | 70 | ["repository", image, actions] -> |
@@ -74,7 +82,7 @@ defmodule GitGudWeb.RegistryController do
| 74 | 82 | "repository:" <> image <> ":" <> Enum.join(granted, ",") |
| 75 | 83 | |
| 76 | 84 | _ -> |
| 77 | − scope | |
| 85 | + nil | |
| 78 | 86 | end |
| 79 | 87 | end |
| 80 | 88 |
modified
lib/git_gud_web/controllers/runner_controller.ex
+29
−2
@@ -384,7 +384,7 @@ defmodule GitGudWeb.RunnerController do
| 384 | 384 | def update_task(conn, _params) do |
| 385 | 385 | with {:ok, %UpdateTaskRequest{state: state} = req} <- |
| 386 | 386 | decode_request(conn, UpdateTaskRequest), |
| 387 | − %RunnerSchema{} = _runner <- authenticate(conn) do | |
| 387 | + %RunnerSchema{} = runner <- authenticate(conn) do | |
| 388 | 388 | job_id = (state && state.id) || 0 |
| 389 | 389 | |
| 390 | 390 | require Logger |
@@ -395,7 +395,7 @@ defmodule GitGudWeb.RunnerController do
| 395 | 395 | "outputs=#{inspect(req.outputs |> Map.keys() |> Enum.take(5))}" |
| 396 | 396 | ) |
| 397 | 397 | |
| 398 | − case GitGud.Repo.get(WorkflowJob, job_id) do | |
| 398 | + case task_job(runner, job_id) do | |
| 399 | 399 | nil -> |
| 400 | 400 | # Ack-and-ignore. Forgejo-runner retries indefinitely on a |
| 401 | 401 | # 404, which clobbers logs even when the work is fundamentally |
@@ -466,6 +466,33 @@ defmodule GitGudWeb.RunnerController do
| 466 | 466 | end |
| 467 | 467 | end |
| 468 | 468 | |
| 469 | + # `state.id` is a bare integer from the runner. A job this runner | |
| 470 | + # didn't claim is treated like one that doesn't exist: acked, nothing | |
| 471 | + # recorded. Otherwise any registered runner — and anyone may register | |
| 472 | + # one for their own repository — could set another repository's job | |
| 473 | + # results (which required checks read), step states and outputs | |
| 474 | + # (which reach later jobs, and their secrets, as `needs.*.outputs`). | |
| 475 | + defp task_job(runner, job_id) when is_integer(job_id) and job_id > 0 do | |
| 476 | + case GitGud.Repo.get(WorkflowJob, job_id) do | |
| 477 | + %WorkflowJob{runner_id: rid} = job when rid == runner.id -> | |
| 478 | + job | |
| 479 | + | |
| 480 | + %WorkflowJob{} -> | |
| 481 | + require Logger | |
| 482 | + | |
| 483 | + Logger.warning( | |
| 484 | + "update_task: job #{job_id} is not claimed by runner_id=#{runner.id}; ignoring it" | |
| 485 | + ) | |
| 486 | + | |
| 487 | + nil | |
| 488 | + | |
| 489 | + nil -> | |
| 490 | + nil | |
| 491 | + end | |
| 492 | + end | |
| 493 | + | |
| 494 | + defp task_job(_runner, _job_id), do: nil | |
| 495 | + | |
| 469 | 496 | defp map_result(:RESULT_SUCCESS), do: "success" |
| 470 | 497 | defp map_result(:RESULT_FAILURE), do: "failure" |
| 471 | 498 | defp map_result(:RESULT_CANCELLED), do: "cancelled" |
added
priv/repo/migrations/20261018182859_add_reserving_job_to_cache_entries.exs
+15
−0
@@ -0,0 +1,15 @@
| 1 | +defmodule GitGud.Repo.Migrations.AddReservingJobToCacheEntries do | |
| 2 | + use Ecto.Migration | |
| 3 | + | |
| 4 | + def change do | |
| 5 | + # The job that reserved a pending entry, and so the only one that may | |
| 6 | + # upload its chunks and finalize it. Without it any job of the same | |
| 7 | + # repository — a fork pull request's, say — could write into an | |
| 8 | + # entry a default-branch job had reserved. NULL (entries reserved | |
| 9 | + # before this column, or whose job is gone) can't be uploaded to or | |
| 10 | + # finalized by anyone; reserving it again takes it over. | |
| 11 | + alter table(:cache_entries) do | |
| 12 | + add :reserved_by_job_id, references(:workflow_jobs, on_delete: :nilify_all) | |
| 13 | + end | |
| 14 | + end | |
| 15 | +end |
modified
test/git_gud_web/controllers/actions_endpoints_test.exs
+26
−0
@@ -309,6 +309,32 @@ defmodule GitGudWeb.ActionsEndpointsTest do
| 309 | 309 | refute CacheEntry.finalized?(GitGud.Repo.get!(CacheEntry, id)) |
| 310 | 310 | end |
| 311 | 311 | |
| 312 | + test "another job of the same repository gets 404 on upload and finalize", %{ | |
| 313 | + conn: conn, | |
| 314 | + repo: repo, | |
| 315 | + token: token | |
| 316 | + } do | |
| 317 | + # The default branch's job reserves; a pull request's job — a | |
| 318 | + # fork's, which runs without secrets — must not write into it. | |
| 319 | + id = reserve(conn, token, "deps") | |
| 320 | + | |
| 321 | + pr_run = | |
| 322 | + run_fixture(repo, %{number: 2, trigger: "pull_request", head_ref: "refs/pull/1/head"}) | |
| 323 | + | |
| 324 | + pr_token = RunnerJwt.mint(job_fixture(pr_run, "build"), pr_run) | |
| 325 | + | |
| 326 | + assert upload_chunk(conn, pr_token, id, 0, "poison").status == 404 | |
| 327 | + assert finalize_cache(conn, pr_token, id, 6).status == 404 | |
| 328 | + | |
| 329 | + entry = GitGud.Repo.get!(CacheEntry, id) | |
| 330 | + refute CacheEntry.finalized?(entry) | |
| 331 | + refute GitGud.Repo.get_by(GitGud.Workflows.CacheEntryPart, cache_entry_id: entry.id) | |
| 332 | + | |
| 333 | + # The reserving job still uploads as before. | |
| 334 | + assert upload_chunk(conn, token, id, 0, "good").status == 204 | |
| 335 | + assert finalize_cache(conn, token, id, 4).status == 204 | |
| 336 | + end | |
| 337 | + | |
| 312 | 338 | test "chunks uploaded out of order are restored in order", %{conn: conn, token: token} do |
| 313 | 339 | id = reserve(conn, token, "deps") |
| 314 | 340 | chunks = for c <- ["a", "b", "c"], do: String.duplicate(c, 1000) |
modified
test/git_gud_web/controllers/registry_controller_test.exs
+26
−0
@@ -110,6 +110,32 @@ defmodule GitGudWeb.RegistryControllerTest do
| 110 | 110 | assert granted(conn, auth, path(repo) <> "/web/assets") == ["pull", "push"] |
| 111 | 111 | end |
| 112 | 112 | |
| 113 | + test "scopes it doesn't understand are dropped, not signed as asked", %{conn: conn} do | |
| 114 | + {_owner, repo} = repository_fixture(%{visibility: "public"}) | |
| 115 | + | |
| 116 | + for scope <- [ | |
| 117 | + "registry:catalog:*", | |
| 118 | + "repository(plugin):#{path(repo)}:pull", | |
| 119 | + "registry:catalog:* repository:#{path(repo)}:pull" | |
| 120 | + ] do | |
| 121 | + body = conn |> get("/v2/token", scope: scope) |> json_response(200) | |
| 122 | + {:ok, claims} = Joken.peek_claims(body["token"]) | |
| 123 | + | |
| 124 | + assert Enum.all?(claims["access"], &(&1["type"] == "repository")), scope | |
| 125 | + refute Enum.any?(claims["access"], &(&1["name"] == "catalog")), scope | |
| 126 | + end | |
| 127 | + | |
| 128 | + # The repository half of a space-separated request is still granted. | |
| 129 | + body = | |
| 130 | + conn | |
| 131 | + |> get("/v2/token", scope: "registry:catalog:* repository:#{path(repo)}:pull") | |
| 132 | + |> json_response(200) | |
| 133 | + | |
| 134 | + {:ok, claims} = Joken.peek_claims(body["token"]) | |
| 135 | + assert [%{"name" => name, "actions" => ["pull"]}] = claims["access"] | |
| 136 | + assert name == path(repo) | |
| 137 | + end | |
| 138 | + | |
| 113 | 139 | test "names that aren't a repository grant nothing", %{conn: conn} do |
| 114 | 140 | {owner, repo} = repository_fixture() |
| 115 | 141 | auth = pat(owner, ["repo"]) |
modified
test/git_gud_web/controllers/runner_controller_test.exs
+51
−0
@@ -494,6 +494,57 @@ defmodule GitGudWeb.RunnerControllerTest do
| 494 | 494 | |
| 495 | 495 | assert GitGud.Repo.get!(GitGud.Workflows.WorkflowJob, task.id).status == "skipped" |
| 496 | 496 | end |
| 497 | + | |
| 498 | + test "UpdateTask for another runner's job is acked and changes nothing", %{ | |
| 499 | + conn: conn, | |
| 500 | + runner_token: token | |
| 501 | + } do | |
| 502 | + {task, _run} = fetch_task(conn, token, %{trigger: "push"}) | |
| 503 | + | |
| 504 | + # Anyone can register a runner for their own repository; the job | |
| 505 | + # above belongs to someone else's. | |
| 506 | + {:ok, _other, other_token} = | |
| 507 | + GitGud.Workflows.Runners.register(%{ | |
| 508 | + name: "someone-elses-runner", | |
| 509 | + version: "v1", | |
| 510 | + labels: %{"labels" => ["ubuntu-latest"]} | |
| 511 | + }) | |
| 512 | + | |
| 513 | + body = | |
| 514 | + %Runner.V1.UpdateTaskRequest{ | |
| 515 | + state: %Runner.V1.TaskState{ | |
| 516 | + id: task.id, | |
| 517 | + result: :RESULT_SUCCESS, | |
| 518 | + steps: [%Runner.V1.StepState{id: 0, result: :RESULT_SUCCESS}] | |
| 519 | + }, | |
| 520 | + outputs: %{"token" => "planted"} | |
| 521 | + } | |
| 522 | + |> Runner.V1.UpdateTaskRequest.encode() | |
| 523 | + |> IO.iodata_to_binary() | |
| 524 | + | |
| 525 | + resp = | |
| 526 | + conn | |
| 527 | + |> recycle() | |
| 528 | + |> put_req_header("content-type", "application/protobuf") | |
| 529 | + |> put_req_header("authorization", "Bearer " <> other_token) | |
| 530 | + |> post("/api/actions/_apis/runner.v1.RunnerService/UpdateTask", body) | |
| 531 | + | |
| 532 | + assert resp.status == 200 | |
| 533 | + assert %{sent_outputs: []} = Runner.V1.UpdateTaskResponse.decode(resp.resp_body) | |
| 534 | + | |
| 535 | + job = | |
| 536 | + GitGud.Workflows.WorkflowJob | |
| 537 | + |> GitGud.Repo.get!(task.id) | |
| 538 | + |> GitGud.Repo.preload(:steps) | |
| 539 | + | |
| 540 | + assert job.status == "running" | |
| 541 | + assert job.outputs == %{} | |
| 542 | + assert Enum.all?(job.steps, &(&1.status == "pending")) | |
| 543 | + | |
| 544 | + # The job's own runner still reports as before. | |
| 545 | + assert %{result: :RESULT_SUCCESS} = update_task(conn, token, task.id, :RESULT_SUCCESS) | |
| 546 | + assert GitGud.Repo.get!(GitGud.Workflows.WorkflowJob, task.id).status == "success" | |
| 547 | + end | |
| 497 | 548 | end |
| 498 | 549 | |
| 499 | 550 | describe "cancellation reaches the runner" do |
Parents: f5a1289