From 3133a5eb6a1806f1ae6718b870dfacab489c9546 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 13:51:21 +0000 Subject: [PATCH 1/3] fix(rules): close five gaps Codex found in #947 after it merged - WH013: `continue-on-error: true` counts as a mask only on the step that performs the write. A flag on a sibling step no longer marks another step's unmasked push as masked. - WH013: a `steps: # comment` key is recognised. - WH013: both masked findings now name `continue-on-error: true` in their reason and fix, instead of only the shell masks. - Job extraction: a comment between jobs is no longer appended to the job above it. A comment such as `# used to git push origin main` was read as a write. - RE005: a double-quoted trap handler may contain escaped quotes (`trap "kill \"$PID\" || true" EXIT`). Tests: 5 new cases; 114/114 pass, and 4 fail against main. The full WorkflowHardening scan and RE005 report the same findings on hypatia, standards and echidna before and after. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01CYR5bf5FBLS7cXQK72aVNM --- lib/rules/research_extensions.ex | 2 +- lib/rules/workflow_hardening.ex | 83 ++++++++++++++++++++++--------- test/research_extensions_test.exs | 16 ++++++ test/workflow_hardening_test.exs | 64 ++++++++++++++++++++++++ 4 files changed, 140 insertions(+), 25 deletions(-) diff --git a/lib/rules/research_extensions.ex b/lib/rules/research_extensions.ex index fb54af0d..69ae82bd 100644 --- a/lib/rules/research_extensions.ex +++ b/lib/rules/research_extensions.ex @@ -630,7 +630,7 @@ defmodule Hypatia.Rules.ResearchExtensions do # same line (`trap '…' EXIT; cargo test || true`) is still judged. has_or_true? = stripped_body - |> String.replace(~r/\btrap[ \t]+(?:'[^'\n]*'|"[^"\n]*")/, "trap") + |> String.replace(~r/\btrap[ \t]+(?:'[^'\n]*'|"(?:[^"\\\n]|\\.)*")/, "trap") |> then(&Regex.match?(~r/\|\|\s*true\b/, &1)) # A step carrying `hypatia:ignore RE005 -- ` (or the diff --git a/lib/rules/workflow_hardening.ex b/lib/rules/workflow_hardening.ex index cb36b57e..d2dab208 100644 --- a/lib/rules/workflow_hardening.ex +++ b/lib/rules/workflow_hardening.ex @@ -1162,14 +1162,16 @@ defmodule Hypatia.Rules.WorkflowHardening do severity: :warn, reason: "workflow #{rel} performs a write with its own credential (PAT or app " <> - "token), and the write is masked (`2>/dev/null`, `|| true`, or `|| echo`). " <> + "token), and the write is masked (`2>/dev/null`, `|| true`, `|| echo`, or the " <> + "step's `continue-on-error: true`). " <> "If the credential expires or loses scope, the write fails and the " <> "workflow stays green.", action: :report, fix_recipe: "unmask-credentialed-write", detail: %{ fix: - "Remove the `2>/dev/null` / `|| true` / `|| echo` mask so a failed write " <> + "Remove the `2>/dev/null` / `|| true` / `|| echo` mask, or the step's " <> + "`continue-on-error: true`, so a failed write " <> "fails the job. Skip an empty commit with `git diff --cached --quiet` " <> "rather than swallowing `git commit`'s exit." } @@ -1201,7 +1203,8 @@ defmodule Hypatia.Rules.WorkflowHardening do reason: "workflow #{rel} performs a write (push/commit/release/PR) with NO " <> "`contents: write` at the workflow level or any job level, AND the " <> - "write is masked (`2>/dev/null`, `|| true`, or `|| echo`). The push " <> + "write is masked (`2>/dev/null`, `|| true`, `|| echo`, or the step's " <> + "`continue-on-error: true`). The push " <> "is denied, the error is swallowed, and the workflow stays GREEN " <> "while recording nothing.", action: :report, @@ -1209,7 +1212,8 @@ defmodule Hypatia.Rules.WorkflowHardening do detail: %{ fix: "Add `permissions: {contents: write}` to the job that performs the " <> - "write, and remove the `2>/dev/null` / `|| echo` mask so a denied " <> + "write, and remove the `2>/dev/null` / `|| echo` mask (or the step's " <> + "`continue-on-error: true`) so a denied " <> "push fails the job instead of warning." } } @@ -1257,37 +1261,64 @@ defmodule Hypatia.Rules.WorkflowHardening do ~r/(?:git\s+push|git\s+commit|gh\s+pr\s+(?:create|merge)|gh\s+release\s+(?:create|upload|edit))[^\n]*\|\|\s*echo/ ] + @steps_key ~r/^[ \t]*steps:[ \t]*(?:#.*)?$/ + @step_continue_on_error ~r/^[ \t]*(?:-[ \t]+)?continue-on-error:[ \t]*true\b/m + @doc """ Return true when a write operation's failure is masked, so a denied write - cannot turn the job red: a shell mask on the write itself, or a STEP-level - `continue-on-error: true` in text that performs a write. The step flag is - judged per job body, so it may belong to a sibling step. A job-level + cannot turn the job red: a shell mask on the write itself, or + `continue-on-error: true` on the very step that performs the write. A flag + on a sibling step does not mask this write, and a job-level `continue-on-error` is not counted: it is a deliberate, visible job setting (standards' github-backup-mirror-reusable.yml documents and surfaces it). """ def masked_write?(content) when is_binary(content) do Enum.any?(@write_masks, &Regex.match?(&1, content)) or - (step_continue_on_error?(content) and performs_contents_write?(content)) + content + |> step_bodies() + |> Enum.any?(&(Regex.match?(@step_continue_on_error, &1) and performs_contents_write?(&1))) end - # `continue-on-error: true` indented under a `steps:` key, i.e. on a step. - defp step_continue_on_error?(content) do - lines = String.split(content, "\n") + # The text of each `- ` item under every `steps:` key in `content`. An item + # runs until the next item at the same indent or a line back at (or above) + # the `steps:` indent; blank and comment lines never end one. + defp step_bodies(content) do + {acc, current, _state} = + content + |> String.split("\n") + |> Enum.reduce({[], nil, nil}, fn line, {acc, current, state} -> + trimmed = String.trim_leading(line) - case Enum.find(lines, &Regex.match?(~r/^[ \t]*steps:[ \t]*$/, &1)) do - nil -> - false + cond do + Regex.match?(@steps_key, line) -> + {flush_step(acc, current), nil, {indent_of(line), nil}} - steps_line -> - steps_indent = indent_of(steps_line) + is_nil(state) -> + {acc, nil, nil} - Enum.any?(lines, fn line -> - Regex.match?(~r/^[ \t]*(?:-[ \t]+)?continue-on-error:[ \t]*true\b/, line) and - indent_of(line) > steps_indent - end) - end + trimmed == "" or String.starts_with?(trimmed, "#") -> + {acc, append_step(current, line), state} + + indent_of(line) <= elem(state, 0) -> + {flush_step(acc, current), nil, nil} + + String.starts_with?(trimmed, "- ") and elem(state, 1) in [nil, indent_of(line)] -> + {flush_step(acc, current), [line], {elem(state, 0), indent_of(line)}} + + true -> + {acc, append_step(current, line), state} + end + end) + + acc |> flush_step(current) |> Enum.reverse() end + defp append_step(nil, _line), do: nil + defp append_step(lines, line), do: [line | lines] + + defp flush_step(acc, nil), do: acc + defp flush_step(acc, lines), do: [lines |> Enum.reverse() |> Enum.join("\n") | acc] + # ─── Internals ────────────────────────────────────────────────────── # Extract every `run:` block from the workflow content. Returns @@ -1422,11 +1453,15 @@ defmodule Hypatia.Rules.WorkflowHardening do _ -> cond do - # A comment between jobs (` # Publish`) does not end `jobs:`. - line == "" or indent_of(line) >= jobs_indent + 1 or - String.starts_with?(String.trim_leading(line), "#") -> + line == "" or indent_of(line) >= jobs_indent + 1 -> {acc, true, jobs_indent, append_line(current, line)} + # A comment between jobs (` # Publish`) neither ends `jobs:` + # nor joins the job above it: WH013 reads bodies as raw text, + # and `# the publisher used to git push` is not a write. + String.starts_with?(String.trim_leading(line), "#") -> + {acc, true, jobs_indent, current} + true -> acc2 = flush(acc, current) {acc2, false, nil, nil} diff --git a/test/research_extensions_test.exs b/test/research_extensions_test.exs index f00d9273..05339a7d 100644 --- a/test/research_extensions_test.exs +++ b/test/research_extensions_test.exs @@ -450,6 +450,22 @@ defmodule Hypatia.Rules.ResearchExtensionsTest do File.rm_rf!(repo) end + test "a double-quoted trap handler with escaped quotes is still skipped" do + repo = + create_repo_with_workflow(~S""" + jobs: + x: + steps: + - name: run tests + run: | + trap "kill \"$PID\" 2>/dev/null || true" EXIT + cargo test + """) + + assert ResearchExtensions.re005_test_swallows_exit(repo) == [] + File.rm_rf!(repo) + end + test "a swallow after a trap on the same line is still reported" do repo = create_repo_with_workflow(""" diff --git a/test/workflow_hardening_test.exs b/test/workflow_hardening_test.exs index 69f30ec8..639094d6 100644 --- a/test/workflow_hardening_test.exs +++ b/test/workflow_hardening_test.exs @@ -1090,6 +1090,70 @@ defmodule Hypatia.Rules.WorkflowHardeningTest do File.rm_rf!(repo) end + test "continue-on-error under a commented `steps:` key is found, and named in the reason" do + repo = + create_repo_with_workflow(""" + name: Release + permissions: + contents: read + jobs: + publish: + runs-on: ubuntu-latest + steps: # publication steps + - continue-on-error: true + env: + GH_TOKEN: ${{ secrets.RELEASE_PAT }} + run: gh release create v1 dist/* + """) + + assert [%{fix_recipe: "unmask-credentialed-write", reason: reason, detail: %{fix: fix}}] = + WorkflowHardening.wh013_permission_starved_write(repo) + + assert reason =~ "continue-on-error" + assert fix =~ "continue-on-error" + File.rm_rf!(repo) + end + + test "continue-on-error on a sibling step does not mask another step's write" do + repo = + create_repo_with_workflow(""" + name: Release + permissions: + contents: read + jobs: + publish: + runs-on: ubuntu-latest + steps: + - name: optional lint + continue-on-error: true + run: make lint + - env: + GH_TOKEN: ${{ secrets.RELEASE_PAT }} + run: gh release create v1 dist/* + """) + + assert [] = WorkflowHardening.wh013_permission_starved_write(repo) + File.rm_rf!(repo) + end + + test "a comment between jobs is not read as part of the job above it" do + repo = + create_repo_with_workflow(""" + name: CI + permissions: + contents: read + jobs: + test: + runs-on: ubuntu-latest + steps: + - run: mix test + # The publisher used to git push origin main; it now lives in release.yml. + """) + + assert [] = WorkflowHardening.wh013_permission_starved_write(repo) + File.rm_rf!(repo) + end + test "a job-level continue-on-error is a deliberate setting, not a mask" do repo = create_repo_with_workflow(""" From 8468f85b6e687d79d84d12b39f950105b514ade8 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 13:59:40 +0000 Subject: [PATCH 2/3] fix(rules): read WH013's step flag at the step's own key column CodeRabbit on #950: - `continue-on-error: true` inside a step's `run: |` script (a heredoc that writes a workflow file) was read as the step's own flag. It now counts only on the `- ` line or at the step's key column. - A step opened by a bare `-` line was not split out, so its flag was missed. A trimmed `-` now starts a step. - A comment indented deeper than the job header was still appended to the job above it. Comments now never join a job body, at any indent. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01CYR5bf5FBLS7cXQK72aVNM --- lib/rules/workflow_hardening.ex | 41 +++++++++++++++----- test/workflow_hardening_test.exs | 64 ++++++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+), 9 deletions(-) diff --git a/lib/rules/workflow_hardening.ex b/lib/rules/workflow_hardening.ex index d2dab208..cd5d3bf5 100644 --- a/lib/rules/workflow_hardening.ex +++ b/lib/rules/workflow_hardening.ex @@ -1276,10 +1276,32 @@ defmodule Hypatia.Rules.WorkflowHardening do Enum.any?(@write_masks, &Regex.match?(&1, content)) or content |> step_bodies() - |> Enum.any?(&(Regex.match?(@step_continue_on_error, &1) and performs_contents_write?(&1))) + |> Enum.any?(&(step_continue_on_error?(&1) and performs_contents_write?(&1))) end - # The text of each `- ` item under every `steps:` key in `content`. An item + # `continue-on-error: true` as a key of the step itself: on its `- ` line or + # at the step's key column. Deeper text (a `run: |` script that writes a + # workflow file, a `with:` value) is not the step's flag. + defp step_continue_on_error?(step) do + [item | rest] = String.split(step, "\n") + keys = Enum.reject(rest, &blank_or_comment?/1) + + key_col = + case Regex.run(~r/^([ \t]*-[ \t]+)\S/, item) do + [_, prefix] -> String.length(prefix) + nil -> keys |> List.first("") |> indent_of() + end + + Regex.match?(@step_continue_on_error, item) or + Enum.any?(keys, &(indent_of(&1) == key_col and Regex.match?(@step_continue_on_error, &1))) + end + + defp blank_or_comment?(line) do + trimmed = String.trim(line) + trimmed == "" or String.starts_with?(trimmed, "#") + end + + # The text of each `- ` (or bare `-`) item under every `steps:` key. An item # runs until the next item at the same indent or a line back at (or above) # the `steps:` indent; blank and comment lines never end one. defp step_bodies(content) do @@ -1302,7 +1324,8 @@ defmodule Hypatia.Rules.WorkflowHardening do indent_of(line) <= elem(state, 0) -> {flush_step(acc, current), nil, nil} - String.starts_with?(trimmed, "- ") and elem(state, 1) in [nil, indent_of(line)] -> + (String.trim(line) == "-" or String.starts_with?(trimmed, "- ")) and + elem(state, 1) in [nil, indent_of(line)] -> {flush_step(acc, current), [line], {elem(state, 0), indent_of(line)}} true -> @@ -1453,15 +1476,15 @@ defmodule Hypatia.Rules.WorkflowHardening do _ -> cond do - line == "" or indent_of(line) >= jobs_indent + 1 -> - {acc, true, jobs_indent, append_line(current, line)} - - # A comment between jobs (` # Publish`) neither ends `jobs:` - # nor joins the job above it: WH013 reads bodies as raw text, - # and `# the publisher used to git push` is not a write. + # A comment, at any indent, neither ends `jobs:` nor joins a + # job body: WH013 reads bodies as raw text, and + # `# the publisher used to git push` is not a write. String.starts_with?(String.trim_leading(line), "#") -> {acc, true, jobs_indent, current} + line == "" or indent_of(line) >= jobs_indent + 1 -> + {acc, true, jobs_indent, append_line(current, line)} + true -> acc2 = flush(acc, current) {acc2, false, nil, nil} diff --git a/test/workflow_hardening_test.exs b/test/workflow_hardening_test.exs index 639094d6..18e879b4 100644 --- a/test/workflow_hardening_test.exs +++ b/test/workflow_hardening_test.exs @@ -1154,6 +1154,70 @@ defmodule Hypatia.Rules.WorkflowHardeningTest do File.rm_rf!(repo) end + test "an indented comment after the last job is not read as part of it" do + repo = + create_repo_with_workflow(""" + name: CI + permissions: + contents: read + jobs: + test: + runs-on: ubuntu-latest + steps: + - run: mix test + # The publisher used to git push origin main; it now lives in release.yml. + """) + + assert [] = WorkflowHardening.wh013_permission_starved_write(repo) + File.rm_rf!(repo) + end + + test "continue-on-error inside a step's run script is not the step's flag" do + repo = + create_repo_with_workflow(""" + name: Release + permissions: + contents: read + jobs: + publish: + runs-on: ubuntu-latest + steps: + - env: + GH_TOKEN: ${{ secrets.RELEASE_PAT }} + run: | + cat > generated.yml <<'YML' + continue-on-error: true + YML + gh release create v1 generated.yml + """) + + assert [] = WorkflowHardening.wh013_permission_starved_write(repo) + File.rm_rf!(repo) + end + + test "continue-on-error on a step opened by a bare `-` counts as masked" do + repo = + create_repo_with_workflow(""" + name: Release + permissions: + contents: read + jobs: + publish: + runs-on: ubuntu-latest + steps: + - + continue-on-error: true + env: + GH_TOKEN: ${{ secrets.RELEASE_PAT }} + run: gh release create v1 dist/* + """) + + assert [%{fix_recipe: "unmask-credentialed-write"}] = + WorkflowHardening.wh013_permission_starved_write(repo) + + File.rm_rf!(repo) + end + test "a job-level continue-on-error is a deliberate setting, not a mask" do repo = create_repo_with_workflow(""" From f352a70ad41671668784f584d11b34bb576aa96b Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 14:14:52 +0000 Subject: [PATCH 3/3] fix(ci): port #948, dropping harden-runner until actions.lock records it `governance / Actions lockfile verify` fails on this PR, and on main since #945. Of the verifier's findings, only the three `step-security/harden-runner@v2.22.2` "not-pinned" records are not accepted by standards' wrapper. #948 removes those steps. This is the same three-file change; it no-ops once #948 (or a lock regeneration) lands on main. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01CYR5bf5FBLS7cXQK72aVNM --- .github/workflows/ci-health-sweep.yml | 8 -------- .github/workflows/dependabot-automerge.yml | 8 -------- .github/workflows/roadmap-sync.yml | 8 -------- 3 files changed, 24 deletions(-) diff --git a/.github/workflows/ci-health-sweep.yml b/.github/workflows/ci-health-sweep.yml index 3d705b66..c26d8ecc 100644 --- a/.github/workflows/ci-health-sweep.yml +++ b/.github/workflows/ci-health-sweep.yml @@ -45,14 +45,6 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 30 steps: - # Egress allowlist (RE001): this job talks only to the GitHub API. - - name: Harden runner - uses: step-security/harden-runner@v2.22.2 - with: - egress-policy: block - allowed-endpoints: > - api.github.com:443 - github.com:443 - uses: actions/checkout@v7.0.1 - name: Run CI-health sweep env: diff --git a/.github/workflows/dependabot-automerge.yml b/.github/workflows/dependabot-automerge.yml index d1875204..86842d10 100644 --- a/.github/workflows/dependabot-automerge.yml +++ b/.github/workflows/dependabot-automerge.yml @@ -62,14 +62,6 @@ jobs: pull-requests: write # needed to approve steps: - # Egress allowlist (RE001): this job talks only to the GitHub API. - - name: Harden runner - uses: step-security/harden-runner@v2.22.2 - with: - egress-policy: block - allowed-endpoints: > - api.github.com:443 - github.com:443 - name: Fetch Dependabot metadata id: meta uses: dependabot/fetch-metadata@v3.1.0 diff --git a/.github/workflows/roadmap-sync.yml b/.github/workflows/roadmap-sync.yml index ea784ee8..eae7a233 100644 --- a/.github/workflows/roadmap-sync.yml +++ b/.github/workflows/roadmap-sync.yml @@ -46,14 +46,6 @@ jobs: EXCLUDE: 'idaptik burble rattlescript vcl-ut' WINDOW_MIN: ${{ inputs.window_min || '45' }} # look-back window (> cron interval for overlap) steps: - # Egress allowlist (RE001): this job talks only to the GitHub API. - - name: Harden runner - uses: step-security/harden-runner@v2.22.2 - with: - egress-policy: block - allowed-endpoints: > - api.github.com:443 - github.com:443 - name: Mint App installation token id: app uses: actions/create-github-app-token@v3.3.0