Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion lib/rules/pin_integrity.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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*(?<action>[A-Za-z0-9_.-]+\/[A-Za-z0-9_.\/-]*?)@(?<ref>[^\s#]+)\s*(?:#\s*(?<comment>.*?))?\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*(?<q>["']?)(?<action>[A-Za-z0-9_.-]+\/[A-Za-z0-9_.\/-]*?)@(?<ref>[^\s#"',}]+)\k<q>\s*\}?\s*,?\s*(?:#\s*(?<comment>.*?))?\s*$/

@doc """
Parse every `uses:` pin site in a workflow file.
Expand Down
100 changes: 97 additions & 3 deletions lib/rules/research_extensions.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
164 changes: 164 additions & 0 deletions test/kyaml_workflow_test.exs
Original file line number Diff line number Diff line change
@@ -0,0 +1,164 @@
# SPDX-License-Identifier: MPL-2.0
# Copyright (c) 2026 Jonathan D.A. Jewell (hyperpolymath) <j.d.a.jewell@open.ac.uk>

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
23 changes: 23 additions & 0 deletions test/rules/pin_integrity_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
Loading