From be443dc386799cd9b843722c40ea58f3f5dc36fe Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 9 Nov 2026 15:49:56 +0000 Subject: [PATCH 1/2] fix(research_extensions): read `on:` brackets outside comments and quotes `Hypatia.WorkflowYaml.nesting_at/2` counts every bracket it is given, so callers must pass text with comments and quoted scalars blanked (its doc says so since #1027). RE001's trigger reader passed raw text in three places. Each place had a failure mode: - Continuation lines of a multi-line `on: [...]`: only whole-line comments are dropped before the rule reads a workflow, so a trailing comment reached both the bracket count and the event scan. - `issues] # was pull_request_target` named a trigger that is not there. - `] # see [ops notes` left a bracket open, so the reader swallowed the rest of the file and took words from `run:` lines as events. - Inline `on: {...}` maps: a bracket inside a quoted scalar (`tags: ["v1.0]"]`) shifted the depth, and a real top-level event key was dropped. - Flow `with:` mappings on a step: they had the same quoted-bracket miscount when deciding whether the mapping continues. The fix strips each continuation line's comment, and reads the depth through `open_flow?/1`, which blanks quoted scalars first (`strip_yaml_comment/2`, offsets preserved). Tests: - 4 new RE001 tests: a positive control plus 3 decoys. The 3 decoys fail on main; the control passes on both. - research_extensions_test: 122/122. It compiles with --warnings-as-errors, and `mix format --check-formatted` is clean. - RE001 findings on hypatia, standards, echidna, gitbot-fleet and mathesis are identical before and after (18 findings). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01CYR5bf5FBLS7cXQK72aVNM --- lib/rules/research_extensions.ex | 30 +++++++++--- test/research_extensions_test.exs | 76 +++++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 7 deletions(-) diff --git a/lib/rules/research_extensions.ex b/lib/rules/research_extensions.ex index 253071d6..f4fbd1d5 100644 --- a/lib/rules/research_extensions.ex +++ b/lib/rules/research_extensions.ex @@ -120,6 +120,7 @@ defmodule Hypatia.Rules.ResearchExtensions do step_indent: 2, step_key_column: 1, strip_yaml_comment: 1, + strip_yaml_comment: 2, workflow_job_lines: 1 ] @@ -796,19 +797,20 @@ defmodule Hypatia.Rules.ResearchExtensions do on_line |> String.split(":", parts: 2) |> List.last() - |> String.replace(~r/\s+#.*$/, "") + |> strip_yaml_comment() # `on: &events` anchors the block mapping that follows. |> String.replace(~r/^\s*&[A-Za-z0-9_-]+/, "") |> String.trim() if inline != "" do # A flow collection may continue past the `on:` line: read on until - # its brackets balance. + # its brackets balance. A continuation line can end in a comment, + # which names no event and opens no bracket. rest - |> Enum.map(&elem(&1, 0)) + |> Enum.map(fn {line, _} -> line |> strip_yaml_comment() |> String.trim() end) |> Enum.reduce_while(inline, fn line, acc -> - if nesting_at(acc, byte_size(acc)) > 0, - do: {:cont, acc <> " " <> String.trim(line)}, + if open_flow?(acc), + do: {:cont, acc <> " " <> line}, else: {:halt, acc} end) |> inline_events() @@ -837,10 +839,16 @@ defmodule Hypatia.Rules.ResearchExtensions do # `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 + # Depth is read from the map with its quoted scalars blanked: a bracket + # in `tags: ["v1.0]"]` is text. Blanking keeps every byte offset. + code = strip_yaml_comment(map, true) + ~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: [] + if nesting_at(code, min(start, byte_size(code))) == 1, + do: [binary_part(map, from, len)], + else: [] end) end @@ -848,6 +856,14 @@ defmodule Hypatia.Rules.ResearchExtensions do ~r/[A-Za-z_]+/ |> Regex.scan(scalar_or_list) |> List.flatten() end + # Whether flow text still has an open `{` or `[`. nesting_at/2 counts every + # bracket it is given, so quoted scalars are blanked first; the text has no + # comments left by the time it is read here. + defp open_flow?(flow) do + code = strip_yaml_comment(flow, true) + nesting_at(code, byte_size(code)) > 0 + end + defp harden_runner_step?(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), @@ -923,7 +939,7 @@ defmodule Hypatia.Rules.ResearchExtensions do # A flow mapping may continue past the `with:` line. rest |> Enum.reduce_while(inline, fn line, acc -> - if nesting_at(acc, byte_size(acc)) > 0, + if open_flow?(acc), do: {:cont, acc <> " " <> String.trim(line)}, else: {:halt, acc} end) diff --git a/test/research_extensions_test.exs b/test/research_extensions_test.exs index 6ce2bd1c..c0ae35b7 100644 --- a/test/research_extensions_test.exs +++ b/test/research_extensions_test.exs @@ -277,6 +277,82 @@ defmodule Hypatia.Rules.ResearchExtensionsTest do File.rm_rf!(repo) end + # An inline `on:` list may continue past its first line. Only whole-line + # comments are dropped before the rule reads it, so a continuation line + # can still carry a trailing comment. + test "an untrusted-input trigger on a continuation line of the `on:` list is an exposure" do + repo = + create_repo_with_workflow(""" + on: [push, + issue_comment] # replies to [bot] mentions + jobs: + reply: + runs-on: ubuntu-latest + steps: + - run: ./reply.sh ${{ secrets.BOT_PAT }} + """) + + assert [%{severity: :warn, detail: %{exposures: [exposure]}}] = + ResearchExtensions.re001_missing_harden_runner(repo) + + assert exposure =~ "issue_comment" + File.rm_rf!(repo) + end + + test "decoy: an event named in a comment on a continuation line is not a trigger" do + repo = + create_repo_with_workflow(""" + on: [push, + workflow_dispatch] # was pull_request_target until #12 + jobs: + sync: + runs-on: ubuntu-latest + steps: + - run: ./sync.sh ${{ secrets.BOT_PAT }} + """) + + assert [%{severity: :warn, detail: %{exposures: []}}] = + ResearchExtensions.re001_missing_harden_runner(repo) + + File.rm_rf!(repo) + end + + test "decoy: an open bracket in a continuation comment does not swallow the workflow" do + repo = + create_repo_with_workflow(""" + on: [push, + workflow_dispatch] # see [ops notes + jobs: + sync: + runs-on: ubuntu-latest + steps: + - run: ./issue_comment.sh ${{ secrets.BOT_PAT }} + """) + + assert [%{severity: :warn, detail: %{exposures: []}}] = + ResearchExtensions.re001_missing_harden_runner(repo) + + File.rm_rf!(repo) + end + + test "decoy: a bracket inside a quoted scalar of an `on:` map does not hide its events" do + repo = + create_repo_with_workflow(""" + on: {push: {tags: ["v1.0]"]}, issue_comment: {types: [created]}} + jobs: + reply: + runs-on: ubuntu-latest + steps: + - run: ./reply.sh ${{ secrets.BOT_PAT }} + """) + + assert [%{severity: :warn, detail: %{exposures: [exposure]}}] = + ResearchExtensions.re001_missing_harden_runner(repo) + + assert exposure =~ "issue_comment" + File.rm_rf!(repo) + end + test "decoy: `issues:` under top-level permissions is not an issues trigger" do repo = create_repo_with_workflow(""" From 72d632a64bc3957508a0947fe63abad03fbed5f1 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 9 Nov 2026 16:22:53 +0000 Subject: [PATCH 2/2] fix(research_extensions): read a multi-line `on:` value as one flow text Codex found two cases the line-by-line comment stripping missed: - A quoted scalar spanning lines lost its quote state at the line break, so `foo #hi"}}}, issue_comment: {}}` was cut at `#`. The closing quote, the braces and the issue_comment event went with it. - An apostrophe inside a plain scalar (`description: Say 'hi`) opened a quote, blanked the structure after it, and hid a later top-level event. flow_text/2 replaces strip_yaml_comment/2 for flow values. It scans the whole value, carrying quote state across newlines. A quote opens a scalar only where a flow node starts, at the start or after `{`, `[`, `,` or `:`. A ` #` outside quotes starts a comment that ends at its own line. Byte offsets are preserved, so the depth and key positions line up as before. Two new tests reproduce the findings and fail on be443dc; 124/124 pass. `mix format` is clean and the code compiles with --warnings-as-errors. RE001 findings on hypatia, standards, echidna, gitbot-fleet and mathesis are unchanged. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01CYR5bf5FBLS7cXQK72aVNM --- lib/rules/research_extensions.ex | 94 +++++++++++++++++++++++++------ test/research_extensions_test.exs | 37 ++++++++++++ 2 files changed, 113 insertions(+), 18 deletions(-) diff --git a/lib/rules/research_extensions.ex b/lib/rules/research_extensions.ex index f4fbd1d5..55c6a7be 100644 --- a/lib/rules/research_extensions.ex +++ b/lib/rules/research_extensions.ex @@ -120,7 +120,6 @@ defmodule Hypatia.Rules.ResearchExtensions do step_indent: 2, step_key_column: 1, strip_yaml_comment: 1, - strip_yaml_comment: 2, workflow_job_lines: 1 ] @@ -793,26 +792,25 @@ defmodule Hypatia.Rules.ResearchExtensions do [] {_, [{on_line, _} | rest]} -> - inline = - on_line - |> String.split(":", parts: 2) - |> List.last() - |> strip_yaml_comment() - # `on: &events` anchors the block mapping that follows. - |> String.replace(~r/^\s*&[A-Za-z0-9_-]+/, "") - |> String.trim() - - if inline != "" do + first = on_line |> String.split(":", parts: 2) |> List.last() + # `on: &events` anchors the block mapping that follows. + unanchor = &(&1 |> String.replace(~r/^\s*&[A-Za-z0-9_-]+/, "") |> String.trim()) + + if unanchor.(flow_text(first, false)) != "" do # A flow collection may continue past the `on:` line: read on until - # its brackets balance. A continuation line can end in a comment, - # which names no event and opens no bracket. + # its brackets balance. The lines are read as one text, so a quoted + # scalar spanning lines keeps its quotes and a comment ends at its + # own line; neither names an event or opens a bracket. rest - |> Enum.map(fn {line, _} -> line |> strip_yaml_comment() |> String.trim() end) - |> Enum.reduce_while(inline, fn line, acc -> + |> Enum.map(&elem(&1, 0)) + |> Enum.reduce_while(first, fn line, acc -> if open_flow?(acc), - do: {:cont, acc <> " " <> line}, + do: {:cont, acc <> "\n" <> line}, else: {:halt, acc} end) + |> flow_text(false) + |> String.replace("\n", " ") + |> then(unanchor) |> inline_events() else block = @@ -841,7 +839,7 @@ defmodule Hypatia.Rules.ResearchExtensions do defp inline_events("{" <> _ = map) do # Depth is read from the map with its quoted scalars blanked: a bracket # in `tags: ["v1.0]"]` is text. Blanking keeps every byte offset. - code = strip_yaml_comment(map, true) + code = flow_text(map, true) ~r/["']?([A-Za-z_]+)["']?\s*:/ |> Regex.scan(map, return: :index) @@ -860,10 +858,70 @@ defmodule Hypatia.Rules.ResearchExtensions do # bracket it is given, so quoted scalars are blanked first; the text has no # comments left by the time it is read here. defp open_flow?(flow) do - code = strip_yaml_comment(flow, true) + code = flow_text(flow, true) nesting_at(code, byte_size(code)) > 0 end + # Flow text, possibly spanning lines, with its comments blanked and, when + # `blank_quoted`, the insides of its quoted scalars blanked too. Every + # byte keeps its offset (a blanked byte becomes a space; newlines stay). + # + # Unlike strip_yaml_comment/2, which reads one line, this reads the whole + # value: a quoted scalar that spans lines keeps its state, so a ` #` inside + # it is text. A quote opens a scalar only where a flow node starts (at the + # start, or after `{`, `[`, `,` or `:` and optional spaces), so the + # apostrophe in the plain scalar `Say 'hi` is text as well. + defp flow_text(text, blank_quoted), + do: flow_scan(text, blank_quoted, :plain, ?\n, nil, []) + + # state: :plain | :comment | {:quote, ?'} | {:quote, ?"} | :escape + # prev: the previous byte; sig: the previous non-space byte outside quotes + defp flow_scan(<<>>, _b, _state, _prev, _sig, acc), + do: acc |> Enum.reverse() |> IO.iodata_to_binary() + + defp flow_scan(<>, b, state, prev, sig, acc) do + keep = fn byte -> if b and byte != ?\n, do: ?\s, else: byte end + + case state do + :comment when c == ?\n -> + flow_scan(rest, b, :plain, c, sig, [c | acc]) + + :comment -> + flow_scan(rest, b, :comment, c, sig, [?\s | acc]) + + :escape -> + flow_scan(rest, b, {:quote, ?"}, c, sig, [keep.(c) | acc]) + + {:quote, ?"} when c == ?\\ -> + flow_scan(rest, b, :escape, c, sig, [keep.(c) | acc]) + + {:quote, q} when c == q -> + case {q, rest} do + # '' inside a single-quoted scalar is an escaped quote. + {?', <>} -> + flow_scan(more, b, {:quote, q}, ?', sig, [keep.(?'), keep.(?') | acc]) + + _ -> + flow_scan(rest, b, :plain, c, c, [c | acc]) + end + + {:quote, _} -> + flow_scan(rest, b, state, c, sig, [keep.(c) | acc]) + + :plain when c == ?# and prev in [?\s, ?\t, ?\n] -> + flow_scan(rest, b, :comment, c, sig, [?\s | acc]) + + :plain when c in [?', ?"] and sig in [nil, ?{, ?[, ?,, ?:] -> + flow_scan(rest, b, {:quote, c}, c, c, [c | acc]) + + :plain when c in [?\s, ?\t, ?\n, ?\r] -> + flow_scan(rest, b, :plain, c, sig, [c | acc]) + + :plain -> + flow_scan(rest, b, :plain, c, c, [c | acc]) + end + end + defp harden_runner_step?(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), diff --git a/test/research_extensions_test.exs b/test/research_extensions_test.exs index c0ae35b7..74893569 100644 --- a/test/research_extensions_test.exs +++ b/test/research_extensions_test.exs @@ -353,6 +353,43 @@ defmodule Hypatia.Rules.ResearchExtensionsTest do File.rm_rf!(repo) end + test "a quoted scalar spanning lines keeps its quotes: a ` #` inside it is not a comment" do + repo = + create_repo_with_workflow(""" + on: {workflow_dispatch: {inputs: {why: {description: "first line + foo #hi"}}}, issue_comment: {}} + jobs: + reply: + runs-on: ubuntu-latest + steps: + - run: ./reply.sh ${{ secrets.BOT_PAT }} + """) + + assert [%{severity: :warn, detail: %{exposures: [exposure]}}] = + ResearchExtensions.re001_missing_harden_runner(repo) + + assert exposure =~ "issue_comment" + File.rm_rf!(repo) + end + + test "an apostrophe inside a plain scalar does not open a quote" do + repo = + create_repo_with_workflow(""" + on: {workflow_dispatch: {inputs: {why: {description: Say 'hi}}}, issue_comment: {}} + jobs: + reply: + runs-on: ubuntu-latest + steps: + - run: ./reply.sh ${{ secrets.BOT_PAT }} + """) + + assert [%{severity: :warn, detail: %{exposures: [exposure]}}] = + ResearchExtensions.re001_missing_harden_runner(repo) + + assert exposure =~ "issue_comment" + File.rm_rf!(repo) + end + test "decoy: `issues:` under top-level permissions is not an issues trigger" do repo = create_repo_with_workflow("""