diff --git a/lib/rules/pin_integrity.ex b/lib/rules/pin_integrity.ex index 6f66f469..c7cd6727 100644 --- a/lib/rules/pin_integrity.ex +++ b/lib/rules/pin_integrity.ex @@ -53,7 +53,12 @@ defmodule Hypatia.Rules.PinIntegrity do or to the owner as `flag` when the repair is not a substitution. """ - @uses_regex ~r/^\s*-?\s*uses:\s*(?[A-Za-z0-9_.-]+\/[A-Za-z0-9_.\/-]*?)@(?[^\s#]+)\s*(?:#\s*(?.*?))?\s*$/ + # A `uses:` pin site, in block form (`- uses: a/b@ref # v1`) or KYAML form + # (`uses: "a/b@ref", # v1` and one-line `{ uses: "a/b@ref" },`). The key + # must open the line — after indentation and an optional `-` or `{` — so a + # `uses:` inside a `run:` string never counts. The value may be quoted; the + # quote, trailing comma and closing brace are not part of the ref. + @uses_regex ~r/^\s*(?:-\s*|\{\s*)?uses:\s*(?["']?)(?[A-Za-z0-9_.-]+\/[A-Za-z0-9_.\/-]*?)@(?[^\s#"',}]+)\k\s*\}?\s*,?\s*(?:#\s*(?.*?))?\s*$/ @doc """ Parse every `uses:` pin site in a workflow file. diff --git a/lib/rules/research_extensions.ex b/lib/rules/research_extensions.ex index 3c0c6e51..d8b1ad18 100644 --- a/lib/rules/research_extensions.ex +++ b/lib/rules/research_extensions.ex @@ -195,7 +195,7 @@ defmodule Hypatia.Rules.ResearchExtensions do repo_path |> workflow_files() |> Enum.flat_map(fn path -> - content = File.read!(path) + content = read_workflow(path) rel = Path.relative_to(path, repo_path) # Comments are not runner configuration. Preserve physical line numbers @@ -450,7 +450,7 @@ defmodule Hypatia.Rules.ResearchExtensions do repo_path |> workflow_files() |> Enum.flat_map(fn path -> - content = File.read!(path) + content = read_workflow(path) rel = Path.relative_to(path, repo_path) uses_cache? = Regex.match?(~r/uses:\s*actions\/cache(?:@|\s)/, content) @@ -593,7 +593,7 @@ defmodule Hypatia.Rules.ResearchExtensions do repo_path |> workflow_files() |> Enum.flat_map(fn path -> - content = File.read!(path) + content = read_workflow(path) rel = Path.relative_to(path, repo_path) content @@ -1039,6 +1039,100 @@ defmodule Hypatia.Rules.ResearchExtensions do } end + # ─── KYAML (flow-style) workflows ─────────────────────────────────── + + # Read a workflow for the line- and indent-based rules. A KYAML workflow + # (YAML-POLICY Y-3: the estate's base form, `yq -o kyaml`) is rewritten to + # the equivalent block layout one line for one line, so physical line + # numbers in findings still point at the source. Block YAML is returned + # byte-identical. + @doc false + def read_workflow(path), do: path |> File.read!() |> flow_to_block() + + # Rewrite a KYAML document to block layout, preserving the line count: + # the root `{` and every closer become blank lines, `key: {`/`key: [` + # lose their opener, the first key of a `{` sequence item and each scalar + # sequence item gain `- `, trailing commas go, and plain double-quoted + # scalars are unquoted. Anything whose first significant line is not a + # bare `{` is not KYAML and passes through untouched — block YAML's own + # `on: [push]` and `permissions: {}` are never rewritten. + @doc false + def flow_to_block(content) when is_binary(content) do + lines = String.split(content, "\n") + + if kyaml_root?(lines) do + {out, _stack, _dash} = Enum.reduce(lines, {[], [], false}, &flow_line/2) + out |> Enum.reverse() |> Enum.join("\n") + else + content + end + end + + # True when the first line that is neither blank nor a comment is a bare `{`. + defp kyaml_root?(lines) do + lines + |> Enum.map(&String.trim/1) + |> Enum.find(&(&1 != "" and not String.starts_with?(&1, "#"))) + |> Kernel.==("{") + end + + # Rewrite one KYAML line. The stack holds the open collections (`:map` or + # `:seq`); `dash` marks that the next property opens a sequence item. + defp flow_line(line, {out, stack, dash}) do + trimmed = String.trim(line) + + if trimmed == "" or String.starts_with?(trimmed, "#") do + {[dedent(line) | out], stack, dash} + else + {head, comment} = + case Regex.run(~r/^(.*?),?(\s+#[^"]*)?$/, String.trim_trailing(line)) do + [_, head, comment] -> {head, comment} + [_, head] -> {head, ""} + end + + indent = String.duplicate(" ", indent_of(head)) + body = String.trim(head) + in_seq? = match?([:seq | _], stack) + + cond do + body in ["}", "]"] -> + {["" | out], Enum.drop(stack, 1), false} + + body == "{" -> + {["" | out], [:map | stack], in_seq?} + + String.ends_with?(body, ": {") or String.ends_with?(body, ": [") -> + kind = if String.ends_with?(body, "{"), do: :map, else: :seq + key = String.slice(body, 0..-3//1) + {[emit(indent, key, comment, dash) | out], [kind | stack], false} + + in_seq? -> + item = Regex.replace(~r/^\{\s*(.*?)\s*\}$/, body, "\\1") + {[emit(indent <> " ", unquote_scalar(item), comment, true) | out], stack, false} + + true -> + {[emit(indent, unquote_scalar(body), comment, dash) | out], stack, false} + end + end + end + + # Assemble one block-layout line, two columns left of the KYAML one, with a + # `- ` sequence marker in the two columns just above the key when `dash`. + defp emit(indent, body, comment, dash) do + prefix = if dash, do: String.slice(indent, 2..-1//1) <> "- ", else: indent + dedent(prefix <> body <> (comment || "")) + end + + # Drop the two columns of indentation the KYAML root mapping adds. + defp dedent(" " <> rest), do: rest + defp dedent(line), do: line + + # Unquote a `key: "value"` or bare `"value"` whose value holds no quote or + # escape; anything else keeps its quotes. + defp unquote_scalar(body) do + Regex.replace(~r/^((?:[^\s"':]+:\s+)?)"([^"\\]*)"$/, body, "\\1\\2") + end + # ─── Internals ────────────────────────────────────────────────────── # Extract step entries from a workflow's `steps:` blocks. Returns diff --git a/test/kyaml_workflow_test.exs b/test/kyaml_workflow_test.exs new file mode 100644 index 00000000..0a525aa9 --- /dev/null +++ b/test/kyaml_workflow_test.exs @@ -0,0 +1,164 @@ +# SPDX-License-Identifier: MPL-2.0 +# Copyright (c) 2026 Jonathan D.A. Jewell (hyperpolymath) + +defmodule Hypatia.Rules.KyamlWorkflowTest do + @moduledoc """ + KYAML (flow-style) workflows must be judged exactly like their block-style + equivalents (standards YAML-POLICY Y-3). The fixtures are the literal output + of `yq -p yaml -o kyaml '.'`: every value is quoted, every mapping is braced, + `run: |` bodies become one quoted string, and step items open with a bare `{`. + Each rule gets a positive (must flag), a control (must not flag) and, where + a line scan could be fooled, a decoy. + """ + use ExUnit.Case, async: true + + alias Hypatia.Rules.{PinIntegrity, ResearchExtensions} + + @sha "0123456789012345678901234567890123456789" + + defp repo_with(content) do + repo = Path.join(System.tmp_dir!(), "kyaml_test_#{System.unique_integer([:positive])}") + wf = Path.join([repo, ".github", "workflows"]) + File.mkdir_p!(wf) + File.write!(Path.join(wf, "ci.yml"), content) + repo + end + + defp secrets_job(hardener) do + """ + # SPDX-License-Identifier: MPL-2.0 + { + name: "deploy", + on: "push", + jobs: { + deploy: { + runs-on: "ubuntu-latest", + steps: [ + #{hardener} { + name: "deploy", + run: "deploy --token=${{ secrets.DEPLOY_KEY }}\\n", + }, + ], + }, + }, + } + """ + end + + @hardener """ + { + uses: "step-security/harden-runner@#{@sha}", # v2 + with: { + egress-policy: "block", + }, + }, + """ + + describe "RE001 on KYAML" do + test "a secrets job without harden-runner is flagged" do + findings = ResearchExtensions.re001_missing_harden_runner(repo_with(secrets_job(""))) + assert [%{rule: "RE001", line: 11}] = findings + end + + test "control: the same job with harden-runner is not flagged" do + assert [] == + ResearchExtensions.re001_missing_harden_runner(repo_with(secrets_job(@hardener))) + end + end + + describe "RE003 on KYAML" do + test "a quoted actions/cache with a head_ref key is flagged" do + repo = + repo_with(""" + { + on: "pull_request", + jobs: { + b: { + runs-on: "ubuntu-latest", + steps: [ + { + uses: "actions/cache@#{@sha}", # v4 + with: { + key: "x-${{ github.head_ref }}", + }, + }, + ], + }, + }, + } + """) + + assert [%{rule: "RE003"}] = ResearchExtensions.re003_cache_key_poisoning(repo) + end + end + + describe "RE005 on KYAML" do + test "a test step whose quoted run body ends in || true is flagged" do + repo = + repo_with(""" + { + on: "push", + jobs: { + t: { + runs-on: "ubuntu-latest", + steps: [ + { + name: "run tests", + run: "npm test || true\\n", + }, + ], + }, + }, + } + """) + + assert [%{rule: "RE005"}] = ResearchExtensions.re005_test_swallows_exit(repo) + end + + test "control: a test step without a swallow is not flagged" do + repo = + repo_with(""" + { + jobs: { + t: { + runs-on: "ubuntu-latest", + steps: [ + { + name: "run tests", + run: "npm test\\n", + }, + ], + }, + }, + } + """) + + assert [] == ResearchExtensions.re005_test_swallows_exit(repo) + end + end + + describe "PinIntegrity.pin_sites/1 on KYAML" do + test "a quoted pin with a trailing comma and comment is one site, unquoted" do + assert [%{line: 1, action: "actions/checkout", ref: @sha, comment: "v4.2.2"}] = + PinIntegrity.pin_sites(~s( uses: "actions/checkout@#{@sha}", # v4.2.2)) + end + + test "a one-line flow step is a site" do + assert [%{action: "actions/setup-node", ref: "v4"}] = + PinIntegrity.pin_sites(~s( { uses: "actions/setup-node@v4" },)) + end + + test "a single-quoted pin is a site" do + assert [%{ref: @sha}] = PinIntegrity.pin_sites(" - uses: 'actions/checkout@#{@sha}'") + end + + test "decoy: a uses: inside a quoted run string is not a site" do + assert [] == PinIntegrity.pin_sites(~s( run: "echo uses: actions/checkout@v4\\n",)) + end + + test "control: block-style pins are unchanged" do + assert [%{action: "actions/checkout", ref: @sha, comment: "v4"}] = + PinIntegrity.pin_sites(" - uses: actions/checkout@#{@sha} # v4") + end + end +end diff --git a/test/rules/pin_integrity_test.exs b/test/rules/pin_integrity_test.exs index 1e323b1d..59c26da7 100644 --- a/test/rules/pin_integrity_test.exs +++ b/test/rules/pin_integrity_test.exs @@ -253,6 +253,29 @@ defmodule Hypatia.Rules.PinIntegrityTest do refute new_content =~ "# v3" end + test "KYAML: a quoted poisoned pin is caught and repaired in place" do + kyaml = """ + { + jobs: { + a: { + steps: [ + { uses: "actions/checkout@v7.0.1" }, + { + uses: "github/codeql-action/init@#{@poison}", # v4.38.1 + }, + ], + }, + }, + } + """ + + assert [%{line: 7}] = PI.pi001_denylisted_pin(kyaml, @policy) + + assert {:ok, new_content, [_]} = PI.substitute_denylisted_pins(kyaml, @policy) + assert new_content =~ ~s(uses: "github/codeql-action/init@#{@good}", # v4.38.0) + assert PI.pi001_denylisted_pin(new_content, @policy) == [] + end + test "a file with no denylisted site is returned untouched" do clean = "- uses: actions/checkout@v7.0.1\n"