diff --git a/lib/rules/actions_lock.ex b/lib/rules/actions_lock.ex index fce24544..41a1ad01 100644 --- a/lib/rules/actions_lock.ex +++ b/lib/rules/actions_lock.ex @@ -3,7 +3,7 @@ defmodule Hypatia.Rules.ActionsLock do @moduledoc """ - Strict local validation for machine-generated `gh actions-lock` v0.0.2 files. + Strict local validation for machine-generated `gh actions-lock` files (v0.0.2, v0.0.3). This module validates lockfile *shape and association*. It does not replace `gh actions-lock --verify`, which remains authoritative for resolving refs @@ -13,14 +13,27 @@ defmodule Hypatia.Rules.ActionsLock do The parser is deliberately line-oriented. `actions.lock` is generated by a single tool with a versioned stable format, and Hypatia must not add a general - YAML dependency merely to consume that format. Unknown or malformed v0.0.2 - lines fail closed rather than being guessed at. + YAML dependency merely to consume that format. Unknown or malformed lines + fail closed rather than being guessed at. + + Format v0.0.3 is the v0.0.2 line grammar plus one optional dependency field, + `hostname` (schema: github/actions-lockfile `schema/lockfile-v0.0.3.json`). + It names the GitHub instance that owns a pin and is written only for pins + that are not on the lock's home host (GHE.com data residency). It is + accepted in v0.0.3 only, and only as `github.com` or a lowercase + `.ghe.com`. Any other new line shape still fails closed. + + Owner/repository ID consistency is checked per host (IDs are numbered per + GitHub instance). Limit: `pinned?/3` keys on `owner/repo@ref` and ignores the + host, and an omitted hostname is a scope of its own, distinct from an + explicit `github.com`. """ - @supported_version "v0.0.2" + @supported_versions ["v0.0.2", "v0.0.3"] @action_ref ~r/^[A-Za-z0-9_.-]+\/[A-Za-z0-9_.\/-]+@[^\s':]+$/ @dependency_key ~r/^[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+@[^\s':]+$/ @sha_ref ~r/^[0-9A-Fa-f]{40}$/ + @hostname ~r/^(github\.com|[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?\.ghe\.com)$/ defstruct version: nil, workflows: %{}, dependencies: %{} @@ -29,6 +42,7 @@ defmodule Hypatia.Rules.ActionsLock do commit: String.t(), owner_id: pos_integer(), repo_id: pos_integer(), + hostname: String.t() | nil, uses: MapSet.t(String.t()) } @type t :: %__MODULE__{ @@ -38,7 +52,7 @@ defmodule Hypatia.Rules.ActionsLock do } @doc """ - Parse and validate a complete v0.0.2 lockfile. + Parse and validate a complete lockfile in a supported format version. Returns `{:error, reason}` for missing, unsupported, malformed, incomplete, duplicate, or contradictory input. One bad dependency invalidates the whole @@ -135,18 +149,20 @@ defmodule Hypatia.Rules.ActionsLock do defp consume_line(line, line_number, %{section: :preamble} = state) do cond do match = Regex.run(~r/^version: '([^']+)'$/, line) -> - case match do - [_, @supported_version] when is_nil(state.version) -> - {:ok, %{state | version: @supported_version}} + [_, version] = match - [_, @supported_version] -> + cond do + not is_nil(state.version) -> line_error(line_number, :duplicate_version) - [_, version] -> + version in @supported_versions -> + {:ok, %{state | version: version}} + + true -> line_error(line_number, {:unsupported_version, version}) end - line == "workflows:" and state.version == @supported_version -> + line == "workflows:" and not is_nil(state.version) -> {:ok, %{state | section: :workflows}} true -> @@ -213,6 +229,23 @@ defmodule Hypatia.Rules.ActionsLock do Regex.match?(~r/^ (?:owner_id|repo_id):/, line) -> line_error(line_number, :malformed_dependency_identity) + match = Regex.run(~r/^ hostname: '([^']*)'$/, line) -> + [_, hostname] = match + + cond do + state.version != "v0.0.3" -> + line_error(line_number, {:hostname_requires_v0_0_3, state.version}) + + Regex.match?(@hostname, hostname) -> + put_dependency_field(state, :hostname, hostname, line_number) + + true -> + line_error(line_number, {:malformed_dependency_hostname, hostname}) + end + + Regex.match?(~r/^ hostname:/, line) -> + line_error(line_number, :malformed_dependency_hostname) + line == " uses:" -> put_dependency_field(state, :uses, MapSet.new(), line_number) @@ -318,7 +351,7 @@ defmodule Hypatia.Rules.ActionsLock do :ok <- validate_dependency_consistency(key, ref, commit, header_line), :ok <- validate_repository_identity( - state.dependencies, + scope_to_host(state.dependencies, Map.get(dependency, :hostname)), normalized, owner_id, repo_id, @@ -330,6 +363,7 @@ defmodule Hypatia.Rules.ActionsLock do commit: commit, owner_id: owner_id, repo_id: repo_id, + hostname: Map.get(dependency, :hostname), uses: Map.get(dependency, :uses, MapSet.new()) } @@ -345,6 +379,16 @@ defmodule Hypatia.Rules.ActionsLock do end end + # Owner and repository IDs are numbered per GitHub instance (the v0.0.3 + # schema defines them "on the recorded hostname"), so identity is compared + # only among pins recorded on the same host. An omitted hostname is its own + # scope (the lock's home host). + defp scope_to_host(dependencies, hostname) do + dependencies + |> Enum.filter(fn {_ref, dependency} -> dependency.hostname == hostname end) + |> Map.new() + end + defp validate_repository_identity(dependencies, normalized, owner_id, repo_id, line_number) do [repository, _ref] = String.split(normalized, "@", parts: 2) [owner, _repo] = String.split(repository, "/", parts: 2) diff --git a/test/rules/actions_lock_test.exs b/test/rules/actions_lock_test.exs index e7e9c9eb..82d471e4 100644 --- a/test/rules/actions_lock_test.exs +++ b/test/rules/actions_lock_test.exs @@ -101,6 +101,133 @@ defmodule Hypatia.Rules.ActionsLockTest do ActionsLock.parse(missing_dependency) end + # #926: `gh actions-lock` began writing v0.0.3: the v0.0.2 line grammar plus + # an optional dependency `hostname` (github/actions-lockfile lockfile-v0.0.3.json). + describe "format v0.0.3" do + @v3_lock String.replace(@valid_lock, "version: 'v0.0.2'", "version: 'v0.0.3'") + + test "is accepted, and records the version it was read as" do + assert {:ok, lock} = ActionsLock.parse(@v3_lock) + assert lock.version == "v0.0.3" + assert ActionsLock.pinned?(lock, "ci.yml", "actions/checkout@v7.0.1") + refute ActionsLock.pinned?(lock, "other.yml", "actions/checkout@v7.0.1") + + assert {:ok, %{version: "v0.0.2"}} = ActionsLock.parse(@valid_lock) + end + + test "is held to the same grammar: a malformed line still fails closed" do + short = String.replace(@v3_lock, ~r/sha1-[0-9a-f]{40}/, "sha1-1234") + assert {:error, {:line, _, :malformed_dependency_commit}} = ActionsLock.parse(short) + + unknown = + String.replace(@v3_lock, " repo_id:", " integrity: 'x'\n repo_id:") + + assert {:error, {:line, _, :malformed_dependency_entry}} = ActionsLock.parse(unknown) + end + + test "accepts and records the optional dependency hostname (GHE.com pins)" do + lock = + String.replace( + @v3_lock, + " owner_id:", + " hostname: 'github.com'\n owner_id:" + ) + + assert {:ok, parsed} = ActionsLock.parse(lock) + assert parsed.dependencies["actions/checkout@v7.0.1"].hostname == "github.com" + assert ActionsLock.pinned?(parsed, "ci.yml", "actions/checkout@v7.0.1") + + tenant = String.replace(lock, "github.com", "octocorp.ghe.com") + assert {:ok, parsed} = ActionsLock.parse(tenant) + assert parsed.dependencies["actions/checkout@v7.0.1"].hostname == "octocorp.ghe.com" + + assert {:ok, parsed} = ActionsLock.parse(@v3_lock) + assert parsed.dependencies["actions/checkout@v7.0.1"].hostname == nil + end + + test "scopes owner/repo identity checks to the dependency's host" do + # Same owner name and repo, different numeric IDs: a conflict within one + # host, but legitimate across hosts (IDs are numbered per instance). + two_pins = """ + version: 'v0.0.3' + workflows: + '.github/workflows/ci.yml': + - 'actions/checkout@v7.0.1' + - 'actions/setup-node@v7.0.0' + dependencies: + 'actions/checkout@v7.0.1': + ref: 'v7.0.1' + commit: 'sha1-3d3c42e5aac5ba805825da76410c181273ba90b1' + owner_id: 44036562 + repo_id: 197814629 + 'actions/setup-node@v7.0.0': + HOST + ref: 'v7.0.0' + commit: 'sha1-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' + owner_id: 99 + repo_id: 12345 + """ + + same_host = Regex.replace(~r/^ *HOST\n/m, two_pins, "") + + assert {:error, {:line, _, {:owner_identity_mismatch, "actions", 44_036_562, 99}}} = + ActionsLock.parse(same_host) + + cross_host = String.replace(two_pins, "HOST", "hostname: 'octocorp.ghe.com'") + assert {:ok, parsed} = ActionsLock.parse(cross_host) + assert parsed.dependencies["actions/setup-node@v7.0.0"].hostname == "octocorp.ghe.com" + end + + test "refuses a malformed, repeated, or v0.0.2 hostname" do + with_host = fn host -> + String.replace( + @v3_lock, + " owner_id:", + " hostname: '#{host}'\n owner_id:" + ) + end + + for bad <- [ + "Octocorp.ghe.com", + "https://github.com", + "github.com:443", + "evil.example.com", + "", + "a.ghe.com/x" + ] do + assert {:error, {:line, _, reason}} = ActionsLock.parse(with_host.(bad)) + assert reason in [:malformed_dependency_hostname, {:malformed_dependency_hostname, bad}] + end + + twice = + String.replace( + with_host.("github.com"), + " owner_id:", + " hostname: 'github.com'\n owner_id:" + ) + + assert {:error, {:line, _, {:duplicate_dependency_field, _, :hostname}}} = + ActionsLock.parse(twice) + + v2 = String.replace(with_host.("github.com"), "v0.0.3", "v0.0.2") + assert {:error, {:line, _, {:hostname_requires_v0_0_3, "v0.0.2"}}} = ActionsLock.parse(v2) + end + + test "negative control: a version past v0.0.3 is still refused" do + assert {:error, {:line, 2, {:unsupported_version, "v0.0.4"}}} = + @valid_lock + |> String.replace("v0.0.2", "v0.0.4") + |> ActionsLock.parse() + end + + test "a second version line is refused, even when both are supported" do + twice = + String.replace(@v3_lock, "version: 'v0.0.3'\n", "version: 'v0.0.3'\nversion: 'v0.0.2'\n") + + assert {:error, {:line, 3, :duplicate_version}} = ActionsLock.parse(twice) + end + end + test "rejects symbolic-ref and SHA-key contradictions" do symbolic = String.replace(@valid_lock, "ref: 'v7.0.1'", "ref: 'v6.0.0'")