From 52a11782eabe08ed27234c9ecde58fe9743dcf68 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 01:29:56 +0000 Subject: [PATCH 1/6] fix(rules): scope RE001 to credentials exposed to code the job cannot vouch for RE001 fired on every job that named any `secrets.*` without harden-runner. That made it noise: 18 of 18 jobs here, 18 in standards, 5 in echidna, and the gate cannot tell the one that matters from the rest. A job is now reported only when it holds a real credential (a secret other than GITHUB_TOKEN, which is the ambient `github.token` every job holds) AND has an exposure: an untrusted-input trigger (pull_request_target, issue_comment, issues, workflow_run, ...), a checkout or git fetch of a pull-request or triggering-run head (naming the head SHA in env/if/ concurrency is not one), a piped installer, or a dependency install/build (`npm ci`, `cargo build`, `mix deps.get`, ...). Severity is :high when the credential and the code exposure share one step, :warn when they share only the job, since an earlier step can still poison what a later credentialed step runs. Measured with the old and new rule: repo old new remaining hypatia 18 2 estate-rescan rescan (warn), tests integration-tests (high) standards 18 2 hypatia-scan-reusable scan, mirror-radicle (both warn) echidna 5 0 Tests: 12 new RE001 cases (each exposure kind, the ambient token, decoys for a step name, `permissions: issues:`, a head SHA in env/if/concurrency and a comment on `on:`, same-step escalation, mixed ambient and real secret); 8 of them fail against the old rule. Existing fixtures gain an `npm ci` step after the credential so they keep testing the runner, hardening and comment logic rather than passing vacuously. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01J4QvDfStHgMdZGTeqaBgM1 --- lib/rules/research_extensions.ex | 223 ++++++++++++++++++++--- test/kyaml_workflow_test.exs | 2 +- test/research_extensions_test.exs | 220 +++++++++++++++++++++- test/research_extensions_wiring_test.exs | 6 +- 4 files changed, 422 insertions(+), 29 deletions(-) diff --git a/lib/rules/research_extensions.ex b/lib/rules/research_extensions.ex index 69ae82bd..b4f74ca4 100644 --- a/lib/rules/research_extensions.ex +++ b/lib/rules/research_extensions.ex @@ -21,11 +21,14 @@ defmodule Hypatia.Rules.ResearchExtensions do Each rule cites its upstream source (tool name and/or paper). - - **RE001** — Workflow accesses `secrets.*` but does not install + - **RE001** — A job holds a real credential (any `secrets.*` other + than the ambient `GITHUB_TOKEN`) alongside code it cannot vouch for + (an untrusted-input trigger, a PR-head checkout, `curl | sh`, or a + dependency install/build), and does not install `step-security/harden-runner`. StepSecurity's Harden-Runner - provides outbound-egress monitoring/blocking for workflows; its - absence on a secrets-touching workflow leaves a clear telemetry - gap. From StepSecurity Harden-Runner deployment guide. + provides outbound-egress monitoring/blocking; without it, nothing + watches the path by which that code would exfiltrate the + credential. From StepSecurity Harden-Runner deployment guide. - **RE002** — Harden-Runner is installed but configured with `egress-policy: audit` (telemetry-only, never blocks). On a protected-branch workflow this provides no actual containment. @@ -155,15 +158,63 @@ defmodule Hypatia.Rules.ResearchExtensions do end end - # ─── RE001: Harden-Runner absent on secrets-touching workflow ───────── + # ─── RE001: credential exposed to unvetted code, no Harden-Runner ───── - @doc """ - RE001: Reports each locally executed workflow job that references - `${{ secrets.* }}` without a `step-security/harden-runner@...` step in the - same job. Full-line comments and reusable-workflow jobs without a direct - `runs-on` field are ignored. + # Events whose payload carries text or code authored by people without + # write access to the repository. + @re001_untrusted_events ~w(pull_request_target issue_comment issues discussion + discussion_comment pull_request_review + pull_request_review_comment workflow_run) + + # The secret every job already holds as `github.token`. Naming it adds no + # credential the job did not have, so it is not an exposure on its own. + @re001_ambient_secrets ["GITHUB_TOKEN"] + + # The head of a pull request or of the run that triggered this one. + @re001_head_ref ~r/github\.event\.(?:pull_request|workflow_run)\.head\.|github\.head_ref/ - Each finding points to the first matching secret reference in the job. + # Where naming that head means fetching its code: a checkout's `ref:` or + # `repository:`, or a git checkout/fetch/switch. A head SHA in `env:`, an + # `if:` or a `concurrency:` group only names it. + @re001_head_fetch ~r/^\s*(?:-\s+)?(?:ref|repository):|\bgit\s+(?:checkout|fetch|switch)\b/ + + # Code fetched at run time: piped installers, package installs, and builds + # or test runs that execute dependencies' build scripts. + @re001_fetched_code ~r/(?:curl|wget)\b[^\n|]*\|\s*(?:sudo\s+)?(?:ba|z)?sh\b|\b(?:npm|pnpm|yarn)\s+(?:i|install|ci)\b|\bnpx\s|\bpip3?\s+install\b|\b(?:pipx|uvx)\s|\bcargo\s+(?:install|build|test|run)\b|\bgo\s+(?:install|build|test|run)\b|\bgem\s+install\b|\bbundle\s+install\b|\bmix\s+(?:deps\.get|compile|test)\b|\bdeno\s+(?:run|install)\b|\bbun\s+(?:i|install|run)\b|\bopam\s+install\b|\bcabal\s+(?:install|build|test)\b|\bstack\s+(?:build|test)\b/ + + @doc """ + RE001: Reports each locally executed workflow job that holds a real + credential next to code it cannot vouch for, without a + `step-security/harden-runner@...` step in the same job. + + A job is reported when all of these hold: + + * it references `${{ secrets.X }}` for some `X` other than + `GITHUB_TOKEN`. That one is the ambient `github.token` every job holds + anyway, so a job naming only it exposes nothing new; + * it has at least one **exposure**: the workflow runs on an + untrusted-input event (`pull_request_target`, `issue_comment`, + `issues`, `workflow_run`, …), the job checks out or fetches the head of + a pull request or triggering run (a `ref:`/`repository:` or a git + checkout/fetch/switch naming it; merely naming the head SHA in `env:` + or `if:` is not an exposure), or a step pipes a download into a + shell or installs or builds dependencies (`npm ci`, `cargo build`, + `mix deps.get`, …); + * it does not install harden-runner. + + Severity is `:high` when the credential and a code exposure share one + step. That code has the credential in its own environment, so nothing + has to be smuggled across steps. Otherwise it is `:warn`: code in one step + can still poison the workspace or tools a later credentialed step uses. + + A credential in a job that runs only pinned actions and its own scripts + is not reported. Pinning is `actions.lock`'s job, and harden-runner adds + little there. Known gap: code fetched by other means, such as `git clone` + followed by running the clone, is not recognised. + + Full-line comments and reusable-workflow jobs without a direct `runs-on` + field are ignored. Each finding points to the credential reference: the + one in the exposed step when there is one, else the first in the job. Provenance: StepSecurity Harden-Runner deployment guide. ## Why this is structural, not a regex @@ -189,7 +240,7 @@ defmodule Hypatia.Rules.ResearchExtensions do indent closes, because treating it as a step raises a **false negative** that suppresses a genuine finding. - Severity: `:warn`. Action: `:report`. + Severity: `:high` or `:warn`, as above. Action: `:report`. """ def re001_missing_harden_runner(repo_path) do repo_path @@ -206,46 +257,168 @@ defmodule Hypatia.Rules.ResearchExtensions do |> Enum.with_index(1) |> Enum.reject(fn {line, _} -> String.starts_with?(String.trim_leading(line), "#") end) + untrusted_events = + active_lines + |> workflow_events() + |> Enum.filter(&(&1 in @re001_untrusted_events)) + active_lines |> workflow_job_lines() |> Enum.flat_map(fn job_lines -> - secret_line = - Enum.find(job_lines, fn {line, _} -> - Regex.match?(~r/\$\{\{\s*secrets\.[A-Za-z_][A-Za-z0-9_]*/, line) - end) + credential_lines = Enum.filter(job_lines, &credential_line?/1) # Reusable jobs delegate their runtime to the source workflow. A sibling's # runner or hardener cannot establish this job's execution policy. local_runner? = direct_job_field?(job_lines, "runs-on") - installs_harden? = harden_runner_step?(job_lines) - if not is_nil(secret_line) and local_runner? and not installs_harden? do - {_source, line} = secret_line + with [first_credential | _] <- credential_lines, + true <- local_runner?, + exposures when exposures != [] <- re001_exposures(job_lines, untrusted_events), + false <- harden_runner_step?(job_lines) do + # A step that both holds the credential and runs fetched or head code + # hands the credential straight to that code. + shared_step = + job_lines + |> job_steps() + |> Enum.find(fn step -> + Enum.any?(step, &credential_line?/1) and Enum.any?(step, &code_exposure_line?/1) + end) + + {severity, {_source, line}} = + case shared_step do + nil -> {:warn, first_credential} + step -> {:high, Enum.find(step, &credential_line?/1)} + end [ %{ rule: "RE001", file: rel, - severity: :warn, + severity: severity, line: line, reason: - "job in #{rel} references `secrets.*` but does not install " <> - "`step-security/harden-runner` — review outbound-egress monitoring", + "job in #{rel} holds a credential (`secrets.*` other than GITHUB_TOKEN) " <> + "next to #{Enum.join(exposures, ", ")}" <> + if(shared_step, do: " in the same step", else: "") <> + ", but does not install `step-security/harden-runner` — " <> + "review outbound-egress monitoring", action: :report, detail: %{ + exposures: exposures, fix: - "Add harden-runner as the first step of this job, with egress-policy: block " <> - "and an allowlist derived from the job's actual required endpoints." + "Move the credential into a job that runs no fetched or untrusted code " <> + "(pass data between jobs as artifacts), or add harden-runner as the " <> + "first step of this job, with egress-policy: block and an allowlist " <> + "derived from the job's actual required endpoints." } } ] else - [] + _ -> [] end end) end) end + # A reference to a secret other than the ambient GITHUB_TOKEN. + defp credential_line?({line, _}) do + ~r/\$\{\{\s*secrets\.([A-Za-z_][A-Za-z0-9_]*)/ + |> Regex.scan(line, capture: :all_but_first) + |> Enum.any?(fn [name] -> name not in @re001_ambient_secrets end) + end + + # A line that checks out or runs head code, or fetches and runs code. Step + # names are labels, not commands, so `name: cargo build` is not one. + defp code_exposure_line?({line, _}) do + not Regex.match?(~r/^\s*(?:-\s+)?name:/, line) and + (head_fetch_line?(line) or Regex.match?(@re001_fetched_code, line)) + end + + defp head_fetch_line?(line) do + Regex.match?(@re001_head_ref, line) and Regex.match?(@re001_head_fetch, line) + end + + defp re001_exposures(job_lines, untrusted_events) do + code_lines = Enum.filter(job_lines, &code_exposure_line?/1) + + [ + untrusted_events != [] && + "an untrusted-input trigger (#{Enum.join(untrusted_events, ", ")})", + Enum.any?(code_lines, fn {line, _} -> head_fetch_line?(line) end) && + "a pull-request or triggering-run head checkout", + Enum.any?(code_lines, fn {line, _} -> Regex.match?(@re001_fetched_code, line) end) && + "fetched code (a piped installer or a dependency install/build)" + ] + |> Enum.filter(&is_binary/1) + end + + # The job's steps, each as its own physical lines. + defp 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), + step_indent when is_integer(step_indent) <- step_indent(step_lines, field_indent) do + step_lines + |> Enum.chunk_while( + [], + fn {line, _} = entry, acc -> + if indent_of(line) == step_indent and Regex.match?(~r/^\s*-(?:\s|$)/, line) and + acc != [] do + {:cont, Enum.reverse(acc), [entry]} + else + {:cont, [entry | acc]} + end + end, + fn + [] -> {:cont, []} + acc -> {:cont, Enum.reverse(acc), []} + end + ) + else + _ -> [] + end + end + + # The event names under the workflow's top-level `on:`, in any of its forms: + # `on: push`, `on: [push, issues]`, or a block of event keys or list items. + defp workflow_events(lines) do + case Enum.split_while(lines, fn {line, _} -> + not Regex.match?(~r/^["']?on["']?:/, line) + end) do + {_, []} -> + [] + + {_, [{on_line, _} | rest]} -> + inline = + on_line + |> String.split(":", parts: 2) + |> List.last() + |> String.replace(~r/\s+#.*$/, "") + |> String.trim() + + if inline != "" do + Regex.scan(~r/[A-Za-z_]+/, inline) |> List.flatten() + else + block = + Enum.take_while(rest, fn {line, _} -> + String.trim(line) == "" or indent_of(line) > 0 + end) + |> Enum.reject(fn {line, _} -> String.trim(line) == "" end) + + event_indent = + block |> Enum.map(fn {line, _} -> indent_of(line) end) |> Enum.min(fn -> nil end) + + block + |> Enum.filter(fn {line, _} -> indent_of(line) == event_indent end) + |> Enum.flat_map(fn {line, _} -> + case Regex.run(~r/^\s*(?:-\s+)?["']?([A-Za-z_]+)["']?\s*:?/, line) do + [_, event] -> [event] + _ -> [] + end + end) + end + end + end + defp direct_job_field?(job_lines, field) do case job_field_indent(job_lines) do nil -> diff --git a/test/kyaml_workflow_test.exs b/test/kyaml_workflow_test.exs index 0a525aa9..abe4d157 100644 --- a/test/kyaml_workflow_test.exs +++ b/test/kyaml_workflow_test.exs @@ -36,7 +36,7 @@ defmodule Hypatia.Rules.KyamlWorkflowTest do steps: [ #{hardener} { name: "deploy", - run: "deploy --token=${{ secrets.DEPLOY_KEY }}\\n", + run: "npm ci\\ndeploy --token=${{ secrets.DEPLOY_KEY }}\\n", }, ], }, diff --git a/test/research_extensions_test.exs b/test/research_extensions_test.exs index 05339a7d..fef23722 100644 --- a/test/research_extensions_test.exs +++ b/test/research_extensions_test.exs @@ -37,6 +37,7 @@ defmodule Hypatia.Rules.ResearchExtensionsTest do runs-on: ubuntu-latest steps: - run: deploy --token=${{ secrets.DEPLOY_KEY }} + - run: npm ci """) findings = ResearchExtensions.re001_missing_harden_runner(repo) @@ -72,6 +73,7 @@ defmodule Hypatia.Rules.ResearchExtensionsTest do runs-on: ubuntu-latest steps: - run: deploy --token=${{ secrets.DEPLOY_KEY }} + - run: npm ci """) [finding] = ResearchExtensions.re001_missing_harden_runner(repo) @@ -91,6 +93,7 @@ defmodule Hypatia.Rules.ResearchExtensionsTest do with: egress-policy: block - run: deploy --token=${{ secrets.DEPLOY_KEY }} + - run: npm ci """) assert ResearchExtensions.re001_missing_harden_runner(repo) == [] @@ -106,11 +109,13 @@ defmodule Hypatia.Rules.ResearchExtensionsTest do steps: - uses: "step-security/harden-runner@main" - run: deploy --token=${{ secrets.DOUBLE_QUOTED_KEY }} + - run: npm ci single-quoted: runs-on: ubuntu-latest steps: - uses: 'step-security/harden-runner@main' - run: deploy --token=${{ secrets.SINGLE_QUOTED_KEY }} + - run: npm ci """) assert ResearchExtensions.re001_missing_harden_runner(repo) == [] @@ -135,6 +140,7 @@ defmodule Hypatia.Rules.ResearchExtensionsTest do runs-on: ubuntu-latest - uses: step-security/harden-runner@main deploy --token=${{ secrets.EXPOSED }} + - run: npm ci """) [finding] = ResearchExtensions.re001_missing_harden_runner(repo) @@ -165,14 +171,16 @@ defmodule Hypatia.Rules.ResearchExtensionsTest do steps: - uses: step-security/harden-runner@main - run: deploy --token=${{ secrets.ONE }} + - run: npm ci exposed: runs-on: ubuntu-latest steps: - run: deploy --token=${{ secrets.TWO }} + - run: npm ci """) [finding] = ResearchExtensions.re001_missing_harden_runner(repo) - assert finding.line == 10 + assert finding.line == 11 assert finding.severity == :warn File.rm_rf!(repo) end @@ -189,6 +197,216 @@ defmodule Hypatia.Rules.ResearchExtensionsTest do assert ResearchExtensions.re001_missing_harden_runner(repo) == [] File.rm_rf!(repo) end + + # RE001 v2: a finding needs a real credential AND an exposure. + + test "the ambient GITHUB_TOKEN alone is not a credential, even next to fetched code" do + repo = + create_repo_with_workflow(""" + on: [pull_request_target] + jobs: + label: + runs-on: ubuntu-latest + steps: + - run: npm ci && gh label create x + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + """) + + assert ResearchExtensions.re001_missing_harden_runner(repo) == [] + File.rm_rf!(repo) + end + + test "a credential next to only pinned actions and the repo's own scripts is not reported" do + repo = + create_repo_with_workflow(""" + on: [push] + jobs: + sweep: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7.0.1 + - name: cargo build (label only) + run: ./scripts/sweep.sh + env: + GH_TOKEN: ${{ secrets.DISPATCH_PAT }} + """) + + assert ResearchExtensions.re001_missing_harden_runner(repo) == [] + File.rm_rf!(repo) + end + + test "an untrusted-input trigger in block form is an exposure" do + repo = + create_repo_with_workflow(""" + on: + pull_request_target: + types: [opened] + jobs: + comment: + runs-on: ubuntu-latest + steps: + - run: ./scripts/comment.sh + env: + TOKEN: ${{ secrets.BOT_PAT }} + """) + + [finding] = ResearchExtensions.re001_missing_harden_runner(repo) + assert finding.severity == :warn + assert finding.line == 10 + assert finding.reason =~ "untrusted-input trigger (pull_request_target)" + File.rm_rf!(repo) + end + + test "an untrusted-input trigger in an inline list is an exposure" do + repo = + create_repo_with_workflow(""" + on: [push, issue_comment] + jobs: + reply: + runs-on: ubuntu-latest + steps: + - run: ./reply.sh ${{ secrets.BOT_PAT }} + """) + + assert [%{severity: :warn}] = ResearchExtensions.re001_missing_harden_runner(repo) + File.rm_rf!(repo) + end + + test "decoy: `issues:` under top-level permissions is not an issues trigger" do + repo = + create_repo_with_workflow(""" + on: + push: + permissions: + issues: write + jobs: + sync: + runs-on: ubuntu-latest + steps: + - run: ./sync.sh ${{ secrets.BOT_PAT }} + """) + + assert ResearchExtensions.re001_missing_harden_runner(repo) == [] + File.rm_rf!(repo) + end + + test "a pull-request head checkout is an exposure" do + repo = + create_repo_with_workflow(""" + on: [push] + jobs: + scan: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7.0.1 + with: + ref: ${{ github.event.pull_request.head.sha }} + - run: ./upload.sh + env: + PAT: ${{ secrets.SCAN_PAT }} + """) + + [finding] = ResearchExtensions.re001_missing_harden_runner(repo) + assert finding.severity == :warn + assert finding.detail.exposures == ["a pull-request or triggering-run head checkout"] + File.rm_rf!(repo) + end + + test "decoy: naming the head SHA in env, if or concurrency is not a checkout" do + repo = + create_repo_with_workflow(""" + on: [push] # issues are triaged elsewhere + jobs: + report: + if: ${{ startsWith(github.head_ref, 'dependabot/') }} + concurrency: + group: report-${{ github.head_ref }} + runs-on: ubuntu-latest + steps: + - run: ./report.sh + env: + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + PAT: ${{ secrets.REPORT_PAT }} + """) + + assert ResearchExtensions.re001_missing_harden_runner(repo) == [] + File.rm_rf!(repo) + end + + test "a git fetch of the head is a checkout" do + repo = + create_repo_with_workflow(""" + on: [push] + jobs: + report: + runs-on: ubuntu-latest + steps: + - run: git fetch origin ${{ github.head_ref }} && ./report.sh + - run: ./upload.sh ${{ secrets.REPORT_PAT }} + """) + + assert [%{severity: :warn, line: 7}] = ResearchExtensions.re001_missing_harden_runner(repo) + File.rm_rf!(repo) + end + + test "a piped installer is an exposure" do + repo = + create_repo_with_workflow(""" + on: [push] + jobs: + deploy: + runs-on: ubuntu-latest + steps: + - run: curl -fsSL https://example.invalid/install.sh | bash + - run: deploy --token=${{ secrets.DEPLOY_KEY }} + """) + + assert [%{severity: :warn, line: 7}] = ResearchExtensions.re001_missing_harden_runner(repo) + File.rm_rf!(repo) + end + + test "a credential in the same step as a dependency build is :high, at that step's reference" do + repo = + create_repo_with_workflow(""" + on: [push] + jobs: + integration: + runs-on: ubuntu-latest + steps: + - run: ./setup.sh ${{ secrets.SETUP_KEY }} + - name: Run integration tests + run: cargo test --workspace + env: + TEST_TOKEN: ${{ secrets.TEST_TOKEN }} + """) + + [finding] = ResearchExtensions.re001_missing_harden_runner(repo) + assert finding.severity == :high + assert finding.line == 10 + assert finding.reason =~ "in the same step" + File.rm_rf!(repo) + end + + test "a job naming GITHUB_TOKEN and a real credential is judged on the credential" do + repo = + create_repo_with_workflow(""" + on: [push] + jobs: + release: + runs-on: ubuntu-latest + steps: + - run: mix deps.get + - run: ./publish.sh + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + HEX_API_KEY: ${{ secrets.HEX_API_KEY }} + """) + + assert [%{severity: :warn, line: 10}] = ResearchExtensions.re001_missing_harden_runner(repo) + + File.rm_rf!(repo) + end end # ─── RE002 ────────────────────────────────────────────────────────── diff --git a/test/research_extensions_wiring_test.exs b/test/research_extensions_wiring_test.exs index 69cbe228..6e2690c3 100644 --- a/test/research_extensions_wiring_test.exs +++ b/test/research_extensions_wiring_test.exs @@ -25,8 +25,9 @@ defmodule Hypatia.Rules.ResearchExtensionsWiringTest do @tmp_dir System.tmp_dir!() - # Trips RE001 (touches `secrets.*` with no harden-runner; :warn, source line) - # and RE004 (`docker://` pinned by tag; :warn, line nested under :detail). + # Trips RE001 (a credential next to fetched code, no harden-runner; :warn, + # source line) and RE004 (`docker://` pinned by tag; :warn, line nested + # under :detail). @tripwire """ name: Deploy on: [push] @@ -36,6 +37,7 @@ defmodule Hypatia.Rules.ResearchExtensionsWiringTest do steps: - uses: docker://alpine:3.21 - run: deploy --token=${{ secrets.DEPLOY_KEY }} + - run: npm ci """ defp tripwire_repo do From 9411b014732ba01e64c2d9d576bb75079b74df15 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 01:42:45 +0000 Subject: [PATCH 2/6] fix(rules): RE001 judges what a step executes, not every line that mentions it Four precision gaps Codex found on #966: - Secret names are case-insensitive, so `secrets.github_token` is the ambient token too; the comparison now upcases the name. - An inline trigger map (`on: {push: {paths: [issues/**]}}`) contributed every word as an event, so `issues` from a path filter read as an issues trigger. Only the map's top-level keys are events now. - Exposures were matched on any non-`name:` line, so an `env:` value or a YAML comment mentioning `npm ci` counted as executed. Fetched code is now matched only in a step's `run:` script (inline or block scalar, comments removed); a head counts as fetched on a checkout's `ref:`/`repository:` or a git checkout/fetch/switch inside `run:`. - That script is matched joined, so a folded `run: >-` that splits `npm` and `ci` across lines is recognised. Five new tests (one per gap, plus inline-map top-level keys); the four gap tests fail against the previous commit. Findings on hypatia, standards and echidna are unchanged (2 / 2 / 0). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01J4QvDfStHgMdZGTeqaBgM1 --- lib/rules/research_extensions.ex | 122 +++++++++++++++++++++++------- test/research_extensions_test.exs | 88 +++++++++++++++++++++ 2 files changed, 182 insertions(+), 28 deletions(-) diff --git a/lib/rules/research_extensions.ex b/lib/rules/research_extensions.ex index b4f74ca4..2ae21a28 100644 --- a/lib/rules/research_extensions.ex +++ b/lib/rules/research_extensions.ex @@ -174,9 +174,10 @@ defmodule Hypatia.Rules.ResearchExtensions do @re001_head_ref ~r/github\.event\.(?:pull_request|workflow_run)\.head\.|github\.head_ref/ # Where naming that head means fetching its code: a checkout's `ref:` or - # `repository:`, or a git checkout/fetch/switch. A head SHA in `env:`, an - # `if:` or a `concurrency:` group only names it. - @re001_head_fetch ~r/^\s*(?:-\s+)?(?:ref|repository):|\bgit\s+(?:checkout|fetch|switch)\b/ + # `repository:`, or a git checkout/fetch/switch in a `run:` script. A head + # SHA in `env:`, an `if:` or a `concurrency:` group only names it. + @re001_checkout_key ~r/^\s*(?:-\s+)?(?:ref|repository):/ + @re001_git_fetch ~r/\bgit\s+(?:checkout|fetch|switch)\b/ # Code fetched at run time: piped installers, package installs, and builds # or test runs that execute dependencies' build scripts. @@ -270,18 +271,17 @@ defmodule Hypatia.Rules.ResearchExtensions do # Reusable jobs delegate their runtime to the source workflow. A sibling's # runner or hardener cannot establish this job's execution policy. local_runner? = direct_job_field?(job_lines, "runs-on") + steps = job_steps(job_lines) with [first_credential | _] <- credential_lines, true <- local_runner?, - exposures when exposures != [] <- re001_exposures(job_lines, untrusted_events), + exposures when exposures != [] <- re001_exposures(steps, untrusted_events), false <- harden_runner_step?(job_lines) do # A step that both holds the credential and runs fetched or head code # hands the credential straight to that code. shared_step = - job_lines - |> job_steps() - |> Enum.find(fn step -> - Enum.any?(step, &credential_line?/1) and Enum.any?(step, &code_exposure_line?/1) + Enum.find(steps, fn step -> + Enum.any?(step, &credential_line?/1) and step_code_exposures(step) != [] end) {severity, {_source, line}} = @@ -320,38 +320,79 @@ defmodule Hypatia.Rules.ResearchExtensions do end) end - # A reference to a secret other than the ambient GITHUB_TOKEN. + # A reference to a secret other than the ambient GITHUB_TOKEN. Secret names + # are case-insensitive, so `secrets.github_token` is the ambient one too. defp credential_line?({line, _}) do ~r/\$\{\{\s*secrets\.([A-Za-z_][A-Za-z0-9_]*)/ |> Regex.scan(line, capture: :all_but_first) - |> Enum.any?(fn [name] -> name not in @re001_ambient_secrets end) + |> Enum.any?(fn [name] -> String.upcase(name) not in @re001_ambient_secrets end) end - # A line that checks out or runs head code, or fetches and runs code. Step - # names are labels, not commands, so `name: cargo build` is not one. - defp code_exposure_line?({line, _}) do - not Regex.match?(~r/^\s*(?:-\s+)?name:/, line) and - (head_fetch_line?(line) or Regex.match?(@re001_fetched_code, line)) - end - - defp head_fetch_line?(line) do - Regex.match?(@re001_head_ref, line) and Regex.match?(@re001_head_fetch, line) - end - - defp re001_exposures(job_lines, untrusted_events) do - code_lines = Enum.filter(job_lines, &code_exposure_line?/1) + defp re001_exposures(steps, untrusted_events) do + code = Enum.flat_map(steps, &step_code_exposures/1) [ untrusted_events != [] && "an untrusted-input trigger (#{Enum.join(untrusted_events, ", ")})", - Enum.any?(code_lines, fn {line, _} -> head_fetch_line?(line) end) && - "a pull-request or triggering-run head checkout", - Enum.any?(code_lines, fn {line, _} -> Regex.match?(@re001_fetched_code, line) end) && - "fetched code (a piped installer or a dependency install/build)" + :head in code && "a pull-request or triggering-run head checkout", + :fetched in code && "fetched code (a piped installer or a dependency install/build)" ] |> Enum.filter(&is_binary/1) end + # What a step executes that the job cannot vouch for: `:head` when it checks + # out or fetches the head of a pull request or triggering run, `:fetched` + # when its `run:` script pipes a download into a shell or installs or builds + # dependencies. Only executable content counts: an `env:` value or a YAML + # comment that mentions `npm ci` runs nothing. + defp step_code_exposures(step) do + run_lines = step_run_lines(step) + # Joined with newlines so a folded `run: >-` command split across lines + # still matches: the patterns' `\s+` spans the line break. + run = Enum.join(run_lines, "\n") + + head? = + Enum.any?(step, fn {line, _} -> + Regex.match?(@re001_checkout_key, line) and Regex.match?(@re001_head_ref, line) + end) or + Enum.any?(run_lines, fn line -> + Regex.match?(@re001_git_fetch, line) and Regex.match?(@re001_head_ref, line) + end) + + [head? && :head, Regex.match?(@re001_fetched_code, run) && :fetched] + |> Enum.filter(& &1) + end + + # The script of a step's `run:`, inline or as a block scalar, with comments + # removed (YAML and shell agree that `#` after whitespace starts one). + defp step_run_lines(step) do + case Enum.split_while(step, fn {line, _} -> + not Regex.match?(~r/^\s*(?:-\s+)?run:/, line) + end) do + {_, []} -> + [] + + {_, [{run_line, _} | rest]} -> + [prefix, value] = + Regex.run(~r/^(\s*(?:-\s+)?)run:\s*(.*)$/, run_line, capture: :all_but_first) + + key_column = String.length(prefix) + + body = + if Regex.match?(~r/^[>|][0-9+-]*\s*(?:#.*)?$/, value) do + rest + |> Enum.take_while(fn {line, _} -> + String.trim(line) == "" or indent_of(line) > key_column + end) + |> Enum.map(&elem(&1, 0)) + else + [value] + end + + Enum.map(body, &String.replace(&1, ~r/(?:^|\s)#.*$/, "")) + end + end + # The job's steps, each as its own physical lines. defp job_steps(job_lines) do with field_indent when is_integer(field_indent) <- job_field_indent(job_lines), @@ -396,7 +437,7 @@ defmodule Hypatia.Rules.ResearchExtensions do |> String.trim() if inline != "" do - Regex.scan(~r/[A-Za-z_]+/, inline) |> List.flatten() + inline_events(inline) else block = Enum.take_while(rest, fn {line, _} -> @@ -419,6 +460,31 @@ defmodule Hypatia.Rules.ResearchExtensions do end end + # `on: {push: {paths: [issues/**]}}` names one event, push: only the map's + # top-level keys are events. A scalar or list names each of its words. + defp inline_events("{" <> _ = map) do + ~r/["']?([A-Za-z_]+)["']?\s*:/ + |> Regex.scan(map, return: :index) + |> Enum.flat_map(fn [{start, _}, {from, len}] -> + if nesting_at(map, start) == 1, do: [binary_part(map, from, len)], else: [] + end) + end + + defp inline_events(scalar_or_list) do + ~r/[A-Za-z_]+/ |> Regex.scan(scalar_or_list) |> List.flatten() + end + + defp nesting_at(text, position) do + text + |> binary_part(0, position) + |> String.graphemes() + |> Enum.reduce(0, fn + c, depth when c in ["{", "["] -> depth + 1 + c, depth when c in ["}", "]"] -> depth - 1 + _, depth -> depth + end) + end + defp direct_job_field?(job_lines, field) do case job_field_indent(job_lines) do nil -> diff --git a/test/research_extensions_test.exs b/test/research_extensions_test.exs index fef23722..88534bfe 100644 --- a/test/research_extensions_test.exs +++ b/test/research_extensions_test.exs @@ -350,6 +350,94 @@ defmodule Hypatia.Rules.ResearchExtensionsTest do File.rm_rf!(repo) end + test "secret names are case-insensitive: secrets.github_token is the ambient token" do + repo = + create_repo_with_workflow(""" + on: [push] + jobs: + label: + runs-on: ubuntu-latest + steps: + - run: npm ci && gh label create x + env: + GH_TOKEN: ${{ secrets.github_token }} + """) + + assert ResearchExtensions.re001_missing_harden_runner(repo) == [] + File.rm_rf!(repo) + end + + test "decoy: a word inside an inline trigger map's filters is not an event" do + repo = + create_repo_with_workflow(""" + on: {push: {paths: [issues/**]}} + jobs: + sync: + runs-on: ubuntu-latest + steps: + - run: ./sync.sh ${{ secrets.BOT_PAT }} + """) + + assert ResearchExtensions.re001_missing_harden_runner(repo) == [] + File.rm_rf!(repo) + end + + test "an inline trigger map's top-level keys are events" do + repo = + create_repo_with_workflow(""" + on: {push: {branches: [main]}, issue_comment: {types: [created]}} + jobs: + reply: + runs-on: ubuntu-latest + steps: + - run: ./reply.sh ${{ secrets.BOT_PAT }} + """) + + [finding] = ResearchExtensions.re001_missing_harden_runner(repo) + assert finding.reason =~ "untrusted-input trigger (issue_comment)" + File.rm_rf!(repo) + end + + test "decoy: install commands in env values and comments run nothing" do + repo = + create_repo_with_workflow(""" + on: [push] + jobs: + deploy: + runs-on: ubuntu-latest + steps: + - run: ./deploy.sh # npm ci happens in the image + env: + NOTE: "run npm ci locally first" + TOKEN: ${{ secrets.DEPLOY_KEY }} + - run: | + # cargo build is done upstream + ./publish.sh + """) + + assert ResearchExtensions.re001_missing_harden_runner(repo) == [] + File.rm_rf!(repo) + end + + test "a folded run scalar is matched as the command it folds into" do + repo = + create_repo_with_workflow(""" + on: [push] + jobs: + build: + runs-on: ubuntu-latest + steps: + - run: >- + npm + ci + env: + TOKEN: ${{ secrets.NPM_TOKEN }} + """) + + assert [%{severity: :high, line: 10}] = ResearchExtensions.re001_missing_harden_runner(repo) + File.rm_rf!(repo) + end + test "a piped installer is an exposure" do repo = create_repo_with_workflow(""" From 652ac168b7c37cff1d609172b0fae85867debe69 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 01:44:17 +0000 Subject: [PATCH 3/6] fix(rules): RE001 honours job env, event gates, workflow_run heads, bare `-` steps Four more gaps from Codex's review of #966: - A credential in the job's own `env:` is in every step's environment, so a step that builds dependencies holds it directly: :high, pointing at the job-level reference. - A job-level `if:` made only of `github.event_name == ''` tests joined by `||` restricts which of the workflow's untrusted events reach the job. Any other condition is not interpreted and the job is judged on all of them. - `workflow_run` names its head `head_sha` / `head_branch` / `head_repository`, not `head.*`; those count as a head checkout now. - `step_indent/2` required whitespace after the step dash, so a step opened by a bare `-` was not found. It now accepts a bare dash, as the step chunker already did; harden-runner detection shares the helper. Four new tests, each failing against the previous commit. Findings on hypatia, standards and echidna are unchanged (2 / 2 / 0). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01J4QvDfStHgMdZGTeqaBgM1 --- lib/rules/research_extensions.ex | 70 +++++++++++++++++++++---- test/research_extensions_test.exs | 85 +++++++++++++++++++++++++++++++ 2 files changed, 146 insertions(+), 9 deletions(-) diff --git a/lib/rules/research_extensions.ex b/lib/rules/research_extensions.ex index 2ae21a28..dcd8d4ea 100644 --- a/lib/rules/research_extensions.ex +++ b/lib/rules/research_extensions.ex @@ -171,7 +171,9 @@ defmodule Hypatia.Rules.ResearchExtensions do @re001_ambient_secrets ["GITHUB_TOKEN"] # The head of a pull request or of the run that triggered this one. - @re001_head_ref ~r/github\.event\.(?:pull_request|workflow_run)\.head\.|github\.head_ref/ + # (`workflow_run` names its head fields `head_sha`, `head_branch`, + # `head_repository`; a pull request nests them under `head.`.) + @re001_head_ref ~r/github\.event\.pull_request\.head\.|github\.event\.workflow_run\.head(?:\.|_(?:sha|branch|repository)\b)|github\.head_ref/ # Where naming that head means fetching its code: a checkout's `ref:` or # `repository:`, or a git checkout/fetch/switch in a `run:` script. A head @@ -272,22 +274,31 @@ defmodule Hypatia.Rules.ResearchExtensions do # runner or hardener cannot establish this job's execution policy. local_runner? = direct_job_field?(job_lines, "runs-on") steps = job_steps(job_lines) + # A credential in the job's own `env:` is in every step's environment. + job_env_credential = job_env_lines(job_lines) |> Enum.find(&credential_line?/1) + + job_untrusted_events = + case job_event_gate(job_lines) do + nil -> untrusted_events + gated -> Enum.filter(untrusted_events, &(&1 in gated)) + end with [first_credential | _] <- credential_lines, true <- local_runner?, - exposures when exposures != [] <- re001_exposures(steps, untrusted_events), + exposures when exposures != [] <- re001_exposures(steps, job_untrusted_events), false <- harden_runner_step?(job_lines) do # A step that both holds the credential and runs fetched or head code # hands the credential straight to that code. - shared_step = - Enum.find(steps, fn step -> - Enum.any?(step, &credential_line?/1) and step_code_exposures(step) != [] + shared = + Enum.find_value(steps, fn step -> + credential = Enum.find(step, &credential_line?/1) || job_env_credential + credential && step_code_exposures(step) != [] && credential end) {severity, {_source, line}} = - case shared_step do + case shared do nil -> {:warn, first_credential} - step -> {:high, Enum.find(step, &credential_line?/1)} + credential -> {:high, credential} end [ @@ -299,7 +310,7 @@ defmodule Hypatia.Rules.ResearchExtensions do reason: "job in #{rel} holds a credential (`secrets.*` other than GITHUB_TOKEN) " <> "next to #{Enum.join(exposures, ", ")}" <> - if(shared_step, do: " in the same step", else: "") <> + if(shared, do: " in the same step", else: "") <> ", but does not install `step-security/harden-runner` — " <> "review outbound-egress monitoring", action: :report, @@ -393,6 +404,47 @@ defmodule Hypatia.Rules.ResearchExtensions do end end + # The lines under the job's own `env:` key. + defp job_env_lines(job_lines) do + with field_indent when is_integer(field_indent) <- job_field_indent(job_lines), + index when is_integer(index) <- + Enum.find_index(job_lines, fn {line, _} -> + indent_of(line) == field_indent and + String.starts_with?(String.trim_leading(line), "env:") + end) do + job_lines + |> Enum.drop(index) + |> then(fn [env_line | rest] -> + [env_line | Enum.take_while(rest, fn {line, _} -> indent_of(line) > field_indent end)] + end) + else + _ -> [] + end + end + + # The events a job-level `if:` restricts the job to, when the condition is + # nothing but `github.event_name == ''` tests joined by `||`. + # Anything more complex is not interpreted, and the job is judged on all the + # workflow's events. + defp job_event_gate(job_lines) do + with field_indent when is_integer(field_indent) <- job_field_indent(job_lines), + {line, _} <- + Enum.find(job_lines, fn {line, _} -> + indent_of(line) == field_indent and + String.starts_with?(String.trim_leading(line), "if:") + end), + [_, condition] <- Regex.run(~r/^\s*if:\s*(.*?)\s*$/, line), + condition = condition |> String.replace(~r/^\$\{\{\s*|\s*\}\}$/, ""), + test = ~S"github\.event_name\s*==\s*'([a-z_]+)'", + true <- Regex.match?(~r/^\s*#{test}(?:\s*\|\|\s*#{test})*\s*$/, condition) do + ~r/github\.event_name\s*==\s*'([a-z_]+)'/ + |> Regex.scan(condition, capture: :all_but_first) + |> List.flatten() + else + _ -> nil + end + end + # The job's steps, each as its own physical lines. defp job_steps(job_lines) do with field_indent when is_integer(field_indent) <- job_field_indent(job_lines), @@ -570,7 +622,7 @@ defmodule Hypatia.Rules.ResearchExtensions do defp step_indent(step_lines, field_indent) do step_lines |> Enum.filter(fn {line, _} -> - indent_of(line) > field_indent and Regex.match?(~r/^\s*-\s+/, line) + indent_of(line) > field_indent and Regex.match?(~r/^\s*-(?:\s|$)/, line) end) |> Enum.map(fn {line, _} -> indent_of(line) end) |> Enum.min(fn -> nil end) diff --git a/test/research_extensions_test.exs b/test/research_extensions_test.exs index 88534bfe..67e0d6e8 100644 --- a/test/research_extensions_test.exs +++ b/test/research_extensions_test.exs @@ -438,6 +438,91 @@ defmodule Hypatia.Rules.ResearchExtensionsTest do File.rm_rf!(repo) end + test "a credential in the job's env is in the same step as that job's build: :high" do + repo = + create_repo_with_workflow(""" + on: [push] + jobs: + build: + runs-on: ubuntu-latest + env: + NPM_TOKEN: ${{ secrets.NPM_TOKEN }} + steps: + - run: npm ci + """) + + assert [%{severity: :high, line: 6}] = ResearchExtensions.re001_missing_harden_runner(repo) + File.rm_rf!(repo) + end + + test "a job gated to trusted events is not exposed by the workflow's untrusted trigger" do + repo = + create_repo_with_workflow(""" + on: [push, issue_comment] + jobs: + deploy: + if: github.event_name == 'push' + runs-on: ubuntu-latest + steps: + - run: ./deploy.sh ${{ secrets.DEPLOY_KEY }} + reply: + if: ${{ github.event_name == 'push' || github.event_name == 'issue_comment' }} + runs-on: ubuntu-latest + steps: + - run: ./reply.sh ${{ secrets.BOT_PAT }} + complex: + if: github.event_name == 'push' && github.actor != 'x' + runs-on: ubuntu-latest + steps: + - run: ./other.sh ${{ secrets.OTHER_PAT }} + """) + + # `deploy` is gated to push; `reply` admits issue_comment; a condition + # that is not a plain event_name test is not interpreted. + assert [%{line: 12}, %{line: 17}] = + repo |> ResearchExtensions.re001_missing_harden_runner() |> Enum.sort_by(& &1.line) + + File.rm_rf!(repo) + end + + test "a workflow_run head checkout is recognised by its head_sha field" do + repo = + create_repo_with_workflow(""" + on: [push] + jobs: + report: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7.0.1 + with: + ref: ${{ github.event.workflow_run.head_sha }} + token: ${{ secrets.REPORT_PAT }} + """) + + [finding] = ResearchExtensions.re001_missing_harden_runner(repo) + assert finding.severity == :high + assert finding.detail.exposures == ["a pull-request or triggering-run head checkout"] + File.rm_rf!(repo) + end + + test "a step opened by a bare `-` is still a step" do + repo = + create_repo_with_workflow(""" + on: [push] + jobs: + build: + runs-on: ubuntu-latest + steps: + - + run: npm ci + env: + NPM_TOKEN: ${{ secrets.NPM_TOKEN }} + """) + + assert [%{severity: :high}] = ResearchExtensions.re001_missing_harden_runner(repo) + File.rm_rf!(repo) + end + test "a piped installer is an exposure" do repo = create_repo_with_workflow(""" From 496f6f600ac85b5e8d07b050bf69c285617af6a4 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 02:01:35 +0000 Subject: [PATCH 4/6] fix(rules): RE001 precision, round three of Codex's review on #966 - A step that sets the same variable as a job-level `env:` secret replaces it, so that step does not hold the credential: no :high from it. - `ref:`/`repository:` naming a head is a checkout only on an `actions/checkout` step; a reporting action's input merely names it. - A git checkout/fetch/switch of the head is matched within one command of the joined `run:` script, so a folded `run: >-` that splits it across lines is caught, and `git fetch origin main && echo $HEAD_REF` is not. - `on: &anchor` followed by a block is read as that block. Five new tests, each failing against the previous commit. Findings on hypatia, standards and echidna are unchanged (2 / 2 / 0). Not changed: telling an install command from the same words inside quoted shell data (`echo 'run npm ci'`) needs shell parsing; stripping quoted strings would hide `bash -c "npm ci"` and break KYAML inline `run:` values. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01J4QvDfStHgMdZGTeqaBgM1 --- lib/rules/research_extensions.ex | 39 ++++++++++--- test/research_extensions_test.exs | 92 +++++++++++++++++++++++++++++++ 2 files changed, 123 insertions(+), 8 deletions(-) diff --git a/lib/rules/research_extensions.ex b/lib/rules/research_extensions.ex index dcd8d4ea..3e712676 100644 --- a/lib/rules/research_extensions.ex +++ b/lib/rules/research_extensions.ex @@ -179,7 +179,10 @@ defmodule Hypatia.Rules.ResearchExtensions do # `repository:`, or a git checkout/fetch/switch in a `run:` script. A head # SHA in `env:`, an `if:` or a `concurrency:` group only names it. @re001_checkout_key ~r/^\s*(?:-\s+)?(?:ref|repository):/ - @re001_git_fetch ~r/\bgit\s+(?:checkout|fetch|switch)\b/ + @re001_checkout_action ~r/^\s*(?:-\s+)?uses:\s*["']?actions\/checkout@/ + # One git command naming the head: no `;`, `&` or `|` between them. `\s` + # spans the line break of a folded `run: >-`. + @re001_git_head_fetch ~r/\bgit\s+(?:checkout|fetch|switch)\b[^;&|]*?(?:github\.event\.pull_request\.head\.|github\.event\.workflow_run\.head(?:\.|_(?:sha|branch|repository)\b)|github\.head_ref)/ # Code fetched at run time: piped installers, package installs, and builds # or test runs that execute dependencies' build scripts. @@ -291,7 +294,11 @@ defmodule Hypatia.Rules.ResearchExtensions do # hands the credential straight to that code. shared = Enum.find_value(steps, fn step -> - credential = Enum.find(step, &credential_line?/1) || job_env_credential + credential = + Enum.find(step, &credential_line?/1) || + (job_env_credential && not overrides_env?(step, job_env_credential) && + job_env_credential) + credential && step_code_exposures(step) != [] && credential end) @@ -362,13 +369,15 @@ defmodule Hypatia.Rules.ResearchExtensions do # still matches: the patterns' `\s+` spans the line break. run = Enum.join(run_lines, "\n") + # A `ref:`/`repository:` input fetches the head only on a checkout; the + # same input on a reporting action merely names it. + checkout? = Enum.any?(step, fn {line, _} -> Regex.match?(@re001_checkout_action, line) end) + head? = - Enum.any?(step, fn {line, _} -> - Regex.match?(@re001_checkout_key, line) and Regex.match?(@re001_head_ref, line) - end) or - Enum.any?(run_lines, fn line -> - Regex.match?(@re001_git_fetch, line) and Regex.match?(@re001_head_ref, line) - end) + (checkout? and + Enum.any?(step, fn {line, _} -> + Regex.match?(@re001_checkout_key, line) and Regex.match?(@re001_head_ref, line) + end)) or Regex.match?(@re001_git_head_fetch, run) [head? && :head, Regex.match?(@re001_fetched_code, run) && :fetched] |> Enum.filter(& &1) @@ -404,6 +413,18 @@ defmodule Hypatia.Rules.ResearchExtensions do end end + # True when the step sets the same variable as the job-level `env:` line, + # so the step-level value replaces the inherited credential. + defp overrides_env?(step, {job_line, _}) do + case Regex.run(~r/^\s*([A-Za-z_][A-Za-z0-9_]*)\s*:/, job_line) do + [_, key] -> + Enum.any?(step, fn {line, _} -> Regex.match?(~r/^\s*#{key}\s*:/, line) end) + + _ -> + false + end + end + # The lines under the job's own `env:` key. defp job_env_lines(job_lines) do with field_indent when is_integer(field_indent) <- job_field_indent(job_lines), @@ -486,6 +507,8 @@ defmodule Hypatia.Rules.ResearchExtensions do |> String.split(":", parts: 2) |> List.last() |> String.replace(~r/\s+#.*$/, "") + # `on: &events` anchors the block mapping that follows. + |> String.replace(~r/^\s*&[A-Za-z0-9_-]+/, "") |> String.trim() if inline != "" do diff --git a/test/research_extensions_test.exs b/test/research_extensions_test.exs index 67e0d6e8..1ded895b 100644 --- a/test/research_extensions_test.exs +++ b/test/research_extensions_test.exs @@ -523,6 +523,98 @@ defmodule Hypatia.Rules.ResearchExtensionsTest do File.rm_rf!(repo) end + test "a step that overrides the job-level secret variable does not hold it" do + repo = + create_repo_with_workflow(""" + on: [push] + jobs: + build: + runs-on: ubuntu-latest + env: + NPM_TOKEN: ${{ secrets.NPM_TOKEN }} + steps: + - run: npm ci + env: + NPM_TOKEN: "" + - run: ./publish.sh + """) + + # The job still holds the credential next to the install, but not in + # the install's own environment. + assert [%{severity: :warn, line: 6}] = ResearchExtensions.re001_missing_harden_runner(repo) + File.rm_rf!(repo) + end + + test "decoy: a head repository passed to a non-checkout action is not a checkout" do + repo = + create_repo_with_workflow(""" + on: [push] + jobs: + report: + runs-on: ubuntu-latest + steps: + - uses: some-org/pr-reporter@v1 + with: + repository: ${{ github.event.pull_request.head.repo.full_name }} + token: ${{ secrets.REPORT_PAT }} + """) + + assert ResearchExtensions.re001_missing_harden_runner(repo) == [] + File.rm_rf!(repo) + end + + test "a folded git fetch of the head is matched across its lines" do + repo = + create_repo_with_workflow(""" + on: [push] + jobs: + report: + runs-on: ubuntu-latest + steps: + - run: >- + git fetch origin + ${{ github.head_ref }} + - run: ./upload.sh ${{ secrets.REPORT_PAT }} + """) + + assert [%{severity: :warn}] = ResearchExtensions.re001_missing_harden_runner(repo) + File.rm_rf!(repo) + end + + test "decoy: a git fetch and a head reference in different commands are not a head fetch" do + repo = + create_repo_with_workflow(""" + on: [push] + jobs: + report: + runs-on: ubuntu-latest + steps: + - run: git fetch origin main && echo ${{ github.head_ref }} + - run: ./upload.sh ${{ secrets.REPORT_PAT }} + """) + + assert ResearchExtensions.re001_missing_harden_runner(repo) == [] + File.rm_rf!(repo) + end + + test "an anchored block of triggers is read as a block" do + repo = + create_repo_with_workflow(""" + on: &events + issue_comment: + types: [created] + jobs: + reply: + runs-on: ubuntu-latest + steps: + - run: ./reply.sh ${{ secrets.BOT_PAT }} + """) + + [finding] = ResearchExtensions.re001_missing_harden_runner(repo) + assert finding.reason =~ "untrusted-input trigger (issue_comment)" + File.rm_rf!(repo) + end + test "a piped installer is an exposure" do repo = create_repo_with_workflow(""" From b846d0bd76d85831ad986ae55d37c8f39dd95742 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 02:10:25 +0000 Subject: [PATCH 5/6] fix(rules): RE001 precision, round four of Codex's review on #966 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - A flow trigger mapping that continues past the `on:` line is read until its brackets balance. - A piped installer is matched within one command of the joined script (`[^|;&]*` instead of `[^\n|]*`), so a folded `curl … | bash` split across lines is caught. - Only the step's own `run:` property is its script; an `env:` variable named `run` is a nested key and is ignored. - A checkout's `ref:`/`repository:` is recognised in flow form (`with: {ref: …}`) as well as block form. Four new tests, each failing against the previous commit. Findings on hypatia, standards and echidna are unchanged (2 / 2 / 0). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01J4QvDfStHgMdZGTeqaBgM1 --- lib/rules/research_extensions.ex | 41 ++++++++++++++++-- test/research_extensions_test.exs | 72 +++++++++++++++++++++++++++++++ 2 files changed, 109 insertions(+), 4 deletions(-) diff --git a/lib/rules/research_extensions.ex b/lib/rules/research_extensions.ex index 3e712676..f9f54a58 100644 --- a/lib/rules/research_extensions.ex +++ b/lib/rules/research_extensions.ex @@ -178,7 +178,8 @@ defmodule Hypatia.Rules.ResearchExtensions do # Where naming that head means fetching its code: a checkout's `ref:` or # `repository:`, or a git checkout/fetch/switch in a `run:` script. A head # SHA in `env:`, an `if:` or a `concurrency:` group only names it. - @re001_checkout_key ~r/^\s*(?:-\s+)?(?:ref|repository):/ + # Block (`ref: …`) or flow (`with: {ref: …}`) form. + @re001_checkout_key ~r/(?:^|[\s{,])(?:ref|repository)\s*:/ @re001_checkout_action ~r/^\s*(?:-\s+)?uses:\s*["']?actions\/checkout@/ # One git command naming the head: no `;`, `&` or `|` between them. `\s` # spans the line break of a folded `run: >-`. @@ -186,7 +187,7 @@ defmodule Hypatia.Rules.ResearchExtensions do # Code fetched at run time: piped installers, package installs, and builds # or test runs that execute dependencies' build scripts. - @re001_fetched_code ~r/(?:curl|wget)\b[^\n|]*\|\s*(?:sudo\s+)?(?:ba|z)?sh\b|\b(?:npm|pnpm|yarn)\s+(?:i|install|ci)\b|\bnpx\s|\bpip3?\s+install\b|\b(?:pipx|uvx)\s|\bcargo\s+(?:install|build|test|run)\b|\bgo\s+(?:install|build|test|run)\b|\bgem\s+install\b|\bbundle\s+install\b|\bmix\s+(?:deps\.get|compile|test)\b|\bdeno\s+(?:run|install)\b|\bbun\s+(?:i|install|run)\b|\bopam\s+install\b|\bcabal\s+(?:install|build|test)\b|\bstack\s+(?:build|test)\b/ + @re001_fetched_code ~r/(?:curl|wget)\b[^|;&]*\|\s*(?:sudo\s+)?(?:ba|z)?sh\b|\b(?:npm|pnpm|yarn)\s+(?:i|install|ci)\b|\bnpx\s|\bpip3?\s+install\b|\b(?:pipx|uvx)\s|\bcargo\s+(?:install|build|test|run)\b|\bgo\s+(?:install|build|test|run)\b|\bgem\s+install\b|\bbundle\s+install\b|\bmix\s+(?:deps\.get|compile|test)\b|\bdeno\s+(?:run|install)\b|\bbun\s+(?:i|install|run)\b|\bopam\s+install\b|\bcabal\s+(?:install|build|test)\b|\bstack\s+(?:build|test)\b/ @doc """ RE001: Reports each locally executed workflow job that holds a real @@ -386,8 +387,15 @@ defmodule Hypatia.Rules.ResearchExtensions do # The script of a step's `run:`, inline or as a block scalar, with comments # removed (YAML and shell agree that `#` after whitespace starts one). defp step_run_lines(step) do + key_column = step_key_column(step) + + # Only the step's own `run:` property: an `env:` variable named `run` is a + # nested key, deeper than the step's properties. case Enum.split_while(step, fn {line, _} -> - not Regex.match?(~r/^\s*(?:-\s+)?run:/, line) + case Regex.run(~r/^(\s*(?:-\s+)?)run:/, line) do + [_, prefix] -> String.length(prefix) != key_column + _ -> true + end end) do {_, []} -> [] @@ -413,6 +421,22 @@ defmodule Hypatia.Rules.ResearchExtensions 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 `-`. + defp step_key_column([{first, _} | rest]) do + case Regex.run(~r/^(\s*-\s+)\S/, first) do + [_, prefix] -> + String.length(prefix) + + _ -> + Enum.find_value(rest, indent_of(first) + 2, fn {line, _} -> + String.trim(line) != "" && indent_of(line) + end) + end + end + + defp step_key_column([]), do: 0 + # True when the step sets the same variable as the job-level `env:` line, # so the step-level value replaces the inherited credential. defp overrides_env?(step, {job_line, _}) do @@ -512,7 +536,16 @@ defmodule Hypatia.Rules.ResearchExtensions do |> String.trim() if inline != "" do - inline_events(inline) + # A flow collection may continue past the `on:` line: read on until + # its brackets balance. + rest + |> Enum.map(&elem(&1, 0)) + |> Enum.reduce_while(inline, fn line, acc -> + if nesting_at(acc, byte_size(acc)) > 0, + do: {:cont, acc <> " " <> String.trim(line)}, + else: {:halt, acc} + end) + |> inline_events() else block = Enum.take_while(rest, fn {line, _} -> diff --git a/test/research_extensions_test.exs b/test/research_extensions_test.exs index 1ded895b..c017923f 100644 --- a/test/research_extensions_test.exs +++ b/test/research_extensions_test.exs @@ -615,6 +615,78 @@ defmodule Hypatia.Rules.ResearchExtensionsTest do File.rm_rf!(repo) end + test "a flow trigger mapping continued past the on: line is read whole" do + repo = + create_repo_with_workflow(""" + on: { + push: {branches: [main]}, + issue_comment: {types: [created]} + } + jobs: + reply: + runs-on: ubuntu-latest + steps: + - run: ./reply.sh ${{ secrets.BOT_PAT }} + """) + + [finding] = ResearchExtensions.re001_missing_harden_runner(repo) + assert finding.reason =~ "untrusted-input trigger (issue_comment)" + File.rm_rf!(repo) + end + + test "a folded piped installer is matched across its lines" do + repo = + create_repo_with_workflow(""" + on: [push] + jobs: + deploy: + runs-on: ubuntu-latest + steps: + - run: >- + curl -fsSL + https://example.invalid/install.sh + | bash + - run: deploy --token=${{ secrets.DEPLOY_KEY }} + """) + + assert [%{severity: :warn}] = ResearchExtensions.re001_missing_harden_runner(repo) + File.rm_rf!(repo) + end + + test "decoy: an env variable named run is not the step's script" do + repo = + create_repo_with_workflow(""" + on: [push] + jobs: + deploy: + runs-on: ubuntu-latest + steps: + - env: + run: "npm ci" + TOKEN: ${{ secrets.DEPLOY_KEY }} + run: ./deploy.sh + """) + + assert ResearchExtensions.re001_missing_harden_runner(repo) == [] + File.rm_rf!(repo) + end + + test "a flow-style checkout with: mapping naming the head is a head checkout" do + repo = + create_repo_with_workflow(""" + on: [push] + jobs: + report: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7.0.1 + with: {ref: "${{ github.head_ref }}", token: "${{ secrets.REPORT_PAT }}"} + """) + + assert [%{severity: :high}] = ResearchExtensions.re001_missing_harden_runner(repo) + File.rm_rf!(repo) + end + test "a piped installer is an exposure" do repo = create_repo_with_workflow(""" From eb6c4f591eeba1654e7cc4cba179d797220f1204 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 02:11:19 +0000 Subject: [PATCH 6/6] fix(rules): RE001 checks every job-level secret a step inherits Overriding one job-level secret in a step left the step holding the others, but only the first job-level credential was considered, so the direct exposure was downgraded to :warn. Every job-level credential is now checked against the step's overrides. New test fails against the previous commit; findings on hypatia, standards and echidna are unchanged (2 / 2 / 0). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01J4QvDfStHgMdZGTeqaBgM1 --- lib/rules/research_extensions.ex | 5 ++--- test/research_extensions_test.exs | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/lib/rules/research_extensions.ex b/lib/rules/research_extensions.ex index f9f54a58..0035836b 100644 --- a/lib/rules/research_extensions.ex +++ b/lib/rules/research_extensions.ex @@ -279,7 +279,7 @@ defmodule Hypatia.Rules.ResearchExtensions do local_runner? = direct_job_field?(job_lines, "runs-on") steps = job_steps(job_lines) # A credential in the job's own `env:` is in every step's environment. - job_env_credential = job_env_lines(job_lines) |> Enum.find(&credential_line?/1) + job_env_credentials = job_env_lines(job_lines) |> Enum.filter(&credential_line?/1) job_untrusted_events = case job_event_gate(job_lines) do @@ -297,8 +297,7 @@ defmodule Hypatia.Rules.ResearchExtensions do Enum.find_value(steps, fn step -> credential = Enum.find(step, &credential_line?/1) || - (job_env_credential && not overrides_env?(step, job_env_credential) && - job_env_credential) + Enum.find(job_env_credentials, &(not overrides_env?(step, &1))) credential && step_code_exposures(step) != [] && credential end) diff --git a/test/research_extensions_test.exs b/test/research_extensions_test.exs index c017923f..d69e7c5d 100644 --- a/test/research_extensions_test.exs +++ b/test/research_extensions_test.exs @@ -545,6 +545,26 @@ defmodule Hypatia.Rules.ResearchExtensionsTest do File.rm_rf!(repo) end + test "overriding one job-level secret leaves the step holding the others" do + repo = + create_repo_with_workflow(""" + on: [push] + jobs: + build: + runs-on: ubuntu-latest + env: + NPM_TOKEN: ${{ secrets.NPM_TOKEN }} + REGISTRY_KEY: ${{ secrets.REGISTRY_KEY }} + steps: + - run: npm ci + env: + NPM_TOKEN: "" + """) + + assert [%{severity: :high, line: 7}] = ResearchExtensions.re001_missing_harden_runner(repo) + File.rm_rf!(repo) + end + test "decoy: a head repository passed to a non-checkout action is not a checkout" do repo = create_repo_with_workflow("""