From 17731bb1c406379ba92c7041e8fb0462ca9a7646 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 9 Nov 2026 13:02:56 +0000 Subject: [PATCH 1/3] docs(workflow_yaml): document the shared reader's public API Hypatia.WorkflowYaml became a public module in #1005, but most of its functions still carried their documentation as `#` comments from their time as private RE001 helpers, and two were `@doc false`. CodeRabbit's docstring-coverage check on #1005 flagged it. Every public function now has an @doc: the existing comment, moved verbatim where there was one (step_key_column, job_steps, key_line?, lines_in_steps, step_indent, strip_yaml_comment, workflow_job_lines, read_workflow, flow_to_block), and a short new one where there was none (expand_aliases, nesting_at, direct_job_field?, job_field_indent, indent_of). No code changes. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01CYR5bf5FBLS7cXQK72aVNM --- lib/hypatia/workflow_yaml.ex | 106 ++++++++++++++++++++++++----------- 1 file changed, 73 insertions(+), 33 deletions(-) diff --git a/lib/hypatia/workflow_yaml.ex b/lib/hypatia/workflow_yaml.ex index b2dc5a2d..aee3793f 100644 --- a/lib/hypatia/workflow_yaml.ex +++ b/lib/hypatia/workflow_yaml.ex @@ -71,6 +71,16 @@ defmodule Hypatia.WorkflowYaml do # Nested anchors can multiply copies exponentially (the "billion laughs" # file), so expansion stops after this many copied lines per workflow. @alias_budget 5_000 + + @doc """ + Expand YAML anchors and aliases in numbered lines (`{line, line_number}`), + as GitHub does: each `*name` is replaced in place by the node anchored + `&name` (the nearest one above), a `<<:` merge adds the anchored entries + the mapping does not set itself, and every line an alias brings carries + the alias's line number. Anchors and aliases count only where a YAML node + starts, never inside quoted scalars or block-scalar text. Expansion stops + after #{@alias_budget} copied lines, so hostile input costs bounded work. + """ def expand_aliases(lines) do codes = yaml_codes(lines) @@ -292,10 +302,12 @@ defmodule Hypatia.WorkflowYaml do end) end - # The column of a step's own properties: just past `- ` on its opening line, - # or the indent of the first property under a bare `-`. A `-` followed only - # by a comment (`- # publish`) is a bare `-`: YAML ignores the comment, so - # the step's keys start on the next line. + @doc """ + The column of a step's own properties: just past `- ` on its opening line, + or the indent of the first property under a bare `-`. A `-` followed only + by a comment (`- # publish`) is a bare `-`: YAML ignores the comment, so + the step's keys start on the next line. + """ def step_key_column([{first, _} | rest]) do case Regex.run(~r/^(\s*-\s+)[^\s#]/, first) do [_, prefix] -> @@ -310,7 +322,9 @@ defmodule Hypatia.WorkflowYaml do def step_key_column([]), do: 0 - # The job's steps, each as its own physical lines. + @doc """ + The job's steps, each as its own physical lines. + """ def job_steps(job_lines) do with field_indent when is_integer(field_indent) <- job_field_indent(job_lines), step_lines when step_lines != [] <- lines_in_steps(job_lines, field_indent), @@ -336,6 +350,10 @@ defmodule Hypatia.WorkflowYaml do end end + @doc """ + The flow-collection depth (`[`/`{` opened minus `]`/`}` closed) of `text` + before byte offset `position`; 0 outside any flow collection. + """ def nesting_at(text, position) do text |> binary_part(0, position) @@ -347,6 +365,11 @@ defmodule Hypatia.WorkflowYaml do end) end + @doc """ + True when the job (as `workflow_job_lines/1` gives it, header line first) + sets `field` as one of its own keys, at the job's field column, and not as + a deeper key of a step or a nested mapping. + """ def direct_job_field?(job_lines, field) do case job_field_indent(job_lines) do nil -> @@ -359,13 +382,19 @@ defmodule Hypatia.WorkflowYaml do end end - # A line that opens the mapping key `key`: YAML reads `env:`, `"env":` and - # `'env':` as the same key, and so does GitHub. + @doc """ + A line that opens the mapping key `key`: YAML reads `env:`, `"env":` and + `'env':` as the same key, and so does GitHub. + """ def key_line?(line, key) do key = Regex.escape(key) Regex.match?(~r/^\s*(?:#{key}|"#{key}"|'#{key}')\s*:(?:\s|$)/, line) end + @doc """ + The column of a job's own keys: the shallowest indent among its non-blank + lines after the header. nil for a job with no body. + """ def job_field_indent([_header | job_lines]) do job_lines |> Enum.map(&elem(&1, 0)) @@ -376,9 +405,11 @@ defmodule Hypatia.WorkflowYaml do def job_field_indent([]), do: nil - # The lines of the job's `steps:` value. Its items may sit deeper than the - # key or, as an indentless sequence YAML also allows, at the key's own - # column: `steps:` then `- run: …` directly beneath it. + @doc """ + The lines of the job's `steps:` value. Its items may sit deeper than the + key or, as an indentless sequence YAML also allows, at the key's own + column: `steps:` then `- run: …` directly beneath it. + """ def lines_in_steps(job_lines, field_indent) do case Enum.find_index(job_lines, fn {line, _} -> indent_of(line) == field_indent and key_line?(line, "steps") @@ -396,8 +427,10 @@ defmodule Hypatia.WorkflowYaml do end end - # The column of the step items: the shallowest `-` in the steps value, which - # is the key's own column for an indentless sequence. + @doc """ + The column of the step items: the shallowest `-` in the steps value, which + is the key's own column for an indentless sequence. + """ def step_indent(step_lines, field_indent) do step_lines |> Enum.filter(fn {line, _} -> @@ -409,11 +442,13 @@ defmodule Hypatia.WorkflowYaml do defp sequence_item?(line), do: Regex.match?(~r/^\s*-(?:\s|$)/, line) - # A line without its YAML comment: a `#` at the start or after whitespace, - # outside a quoted scalar. A quote opens a scalar only where one can start - # (after whitespace or a flow indicator), so the apostrophe in `it's` does not. - # With `blank_quoted`, each quoted scalar's text becomes spaces of the same - # byte length, so what remains is structure at its original offsets. + @doc """ + A line without its YAML comment: a `#` at the start or after whitespace, + outside a quoted scalar. A quote opens a scalar only where one can start + (after whitespace or a flow indicator), so the apostrophe in `it's` does not. + With `blank_quoted`, each quoted scalar's text becomes spaces of the same + byte length, so what remains is structure at its original offsets. + """ def strip_yaml_comment(line, blank_quoted \\ false) do text = fn c -> if blank_quoted, do: String.duplicate(" ", byte_size(c)), else: c end @@ -453,8 +488,10 @@ defmodule Hypatia.WorkflowYaml do # properties (an anchor `&all_jobs`, a tag `!!map`) before its block value. @jobs_key ~r/^(?:jobs|"jobs"|'jobs')\s*:\s*(?:(?:&[A-Za-z0-9_-]+|![^\s#]*)\s*)*(?:#.*)?$/ - # Follow block-style jobs by indentation, retaining physical source lines. - # As with the other research rules, this is a local static text analysis. + @doc """ + Follow block-style jobs by indentation, retaining physical source lines. + As with the other research rules, this is a local static text analysis. + """ def workflow_job_lines(lines) do {_in_jobs, _indent, groups} = Enum.reduce(lines, {false, nil, []}, fn {line, _} = entry, {in_jobs, indent, groups} -> @@ -489,22 +526,24 @@ defmodule Hypatia.WorkflowYaml do groups |> Enum.reverse() |> Enum.map(&Enum.reverse/1) end - # 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 + @doc """ + 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. + """ 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 + @doc """ + 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. + """ def flow_to_block(content) when is_binary(content) do lines = String.split(content, "\n") @@ -581,6 +620,7 @@ defmodule Hypatia.WorkflowYaml do Regex.replace(~r/^((?:[^\s"':]+:\s+)?)"([^"\\]*)"$/, body, "\\1\\2") end + @doc "The number of leading whitespace characters of `line`." def indent_of(line) do case Regex.run(~r/^(\s*)/, line) do [_, ws] -> String.length(ws) From ae522d3bca4d4e51e3692ae7e8929748e14d105b Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 9 Nov 2026 13:04:58 +0000 Subject: [PATCH 2/3] test(workflow_hardening): clear reused temp repos before writing WorkflowHardeningTest names its temporary repositories `wh_test_` and `wh_empty_`. System.unique_integer/1 restarts with every VM, so a later run reuses the names an earlier run used. A run that crashed before its cleanup (for example, one whose modules failed to compile) leaves its workflows behind, and the next run scans them alongside its own. A stale pull_request_target workflow then trips WH001 in unrelated tests. Clear the directory before writing, as ResearchExtensionsTest's helper already does. With 2,000 stale directories planted under TMPDIR, the suite fails 24 of 104 on main and 0 of 104 with this change; it also passes on a clean TMPDIR. CI is unaffected (fresh runners); this guards local re-runs. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01CYR5bf5FBLS7cXQK72aVNM --- test/workflow_hardening_test.exs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/workflow_hardening_test.exs b/test/workflow_hardening_test.exs index 8ef5189c..1f131642 100644 --- a/test/workflow_hardening_test.exs +++ b/test/workflow_hardening_test.exs @@ -10,6 +10,9 @@ defmodule Hypatia.Rules.WorkflowHardeningTest do defp create_repo_with_workflow(yaml_content, filename \\ "test.yml") do repo = Path.join(@tmp_dir, "wh_test_#{System.unique_integer([:positive])}") + # unique_integer restarts with each VM, so clear what an earlier run left: + # a workflow from a crashed run would otherwise be scanned with this one. + File.rm_rf!(repo) wf = Path.join([repo, ".github", "workflows"]) File.mkdir_p!(wf) File.write!(Path.join(wf, filename), yaml_content) @@ -550,6 +553,7 @@ defmodule Hypatia.Rules.WorkflowHardeningTest do test "returns [] on a repo without .github/workflows/" do repo = Path.join(@tmp_dir, "wh_empty_#{System.unique_integer([:positive])}") + File.rm_rf!(repo) File.mkdir_p!(repo) assert WorkflowHardening.scan(repo).total == 0 File.rm_rf!(repo) From fbbb5736711b2a59175c274829290b8f24b8326c Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 9 Nov 2026 13:19:19 +0000 Subject: [PATCH 3/3] docs(workflow_yaml) + test(workflow_hardening): scope the alias and nesting docs; keep concurrent VMs apart Codex review on #1027 raised three points: - expand_aliases/1: the new @doc said every alias is replaced in place. The code expands only within the top-level jobs: and env: sections, so the doc now says so, and says an alias elsewhere is returned unchanged. - nesting_at/2: it counts every bracket character, including those in quoted scalars and comments. The doc now says callers pass text with those already blanked. - wh_test_* and wh_empty_* fixtures: unique_integer/1 is unique only within one VM, so two concurrent `mix test` runs on one TMPDIR could pick the same name, and the new rm_rf! could delete the other run's live fixture. The OS pid is now part of the name. The rm_rf! still clears what a crashed run with a reused pid left behind. RE + WH suites: 222/222, twice. workflow_yaml.ex compiles with --warnings-as-errors. mix format is clean. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01CYR5bf5FBLS7cXQK72aVNM --- lib/hypatia/workflow_yaml.ex | 17 ++++++++++++----- test/workflow_hardening_test.exs | 7 ++++--- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/lib/hypatia/workflow_yaml.ex b/lib/hypatia/workflow_yaml.ex index aee3793f..3d588847 100644 --- a/lib/hypatia/workflow_yaml.ex +++ b/lib/hypatia/workflow_yaml.ex @@ -74,11 +74,14 @@ defmodule Hypatia.WorkflowYaml do @doc """ Expand YAML anchors and aliases in numbered lines (`{line, line_number}`), - as GitHub does: each `*name` is replaced in place by the node anchored - `&name` (the nearest one above), a `<<:` merge adds the anchored entries - the mapping does not set itself, and every line an alias brings carries - the alias's line number. Anchors and aliases count only where a YAML node - starts, never inside quoted scalars or block-scalar text. Expansion stops + as GitHub does, within what jobs read: the top-level `jobs:` and `env:` + sections. There each `*name` is replaced in place by the node anchored + `&name` (the nearest one above, wherever it is defined), a `<<:` merge adds + the anchored entries the mapping does not set itself, and every line an + alias brings carries the alias's line number. An alias in any other + top-level section (`on:`, `permissions:`, ...) is returned unchanged. + Anchors and aliases count only where a YAML node starts, never inside + quoted scalars or block-scalar text. Expansion stops after #{@alias_budget} copied lines, so hostile input costs bounded work. """ def expand_aliases(lines) do @@ -353,6 +356,10 @@ defmodule Hypatia.WorkflowYaml do @doc """ The flow-collection depth (`[`/`{` opened minus `]`/`}` closed) of `text` before byte offset `position`; 0 outside any flow collection. + + Every bracket character counts, including one inside a quoted scalar or a + comment, so pass text with those already blanked (as the reader's own + YAML codes are); on raw YAML the depth can be wrong. """ def nesting_at(text, position) do text diff --git a/test/workflow_hardening_test.exs b/test/workflow_hardening_test.exs index 1f131642..8bbdde78 100644 --- a/test/workflow_hardening_test.exs +++ b/test/workflow_hardening_test.exs @@ -9,8 +9,9 @@ defmodule Hypatia.Rules.WorkflowHardeningTest do @tmp_dir System.tmp_dir!() defp create_repo_with_workflow(yaml_content, filename \\ "test.yml") do - repo = Path.join(@tmp_dir, "wh_test_#{System.unique_integer([:positive])}") - # unique_integer restarts with each VM, so clear what an earlier run left: + repo = Path.join(@tmp_dir, "wh_test_#{System.pid()}_#{System.unique_integer([:positive])}") + # The OS pid keeps two concurrent VMs apart; unique_integer restarts with + # each VM, so a reused pid can meet what an earlier run left. Clear it: # a workflow from a crashed run would otherwise be scanned with this one. File.rm_rf!(repo) wf = Path.join([repo, ".github", "workflows"]) @@ -552,7 +553,7 @@ defmodule Hypatia.Rules.WorkflowHardeningTest do end test "returns [] on a repo without .github/workflows/" do - repo = Path.join(@tmp_dir, "wh_empty_#{System.unique_integer([:positive])}") + repo = Path.join(@tmp_dir, "wh_empty_#{System.pid()}_#{System.unique_integer([:positive])}") File.rm_rf!(repo) File.mkdir_p!(repo) assert WorkflowHardening.scan(repo).total == 0