From 7f2135acc18b26f1aeb548e4b3fea3655b993c04 Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Wed, 30 Sep 2026 10:58:59 +0100 Subject: [PATCH 1/2] fix(rules): hardcoded_tmp stops teaching its own false positive; corpus + inline directives honoured The rule's remediation said "use mktemp", yet `mktemp -d /tmp/x.XXXXXX` fired the rule, and mktemp is the wrong cure for a pid file the next run must re-find. Now: * reason text distinguishes pid/state files (per-user XDG ladder) from scratch files (mktemp -d, no /tmp template, trap cleanup); * skip_comment_lines + new skip_if_line_matches (~r/\bmktemp\b/) guard; * "content_patterns" joins @default_exemptions for the training corpus. The key is the EMITTED rule_module (cli.ex normalises content findings to "content_patterns"); a "cicd_rules" key would be vacuous; * `hypatia: allow /` is now honoured by the content engine under either module spelling (was silently inert; only the bare `hypatia:ignore ` needle worked). Rule ids are atoms, so the id is stringified before inline_allowed?/4; * line 1 no longer reads the LAST line as its "previous line" (Enum.at(lines, -1) wraparound); * .hypatia-ignore header and CLAUDE.md document the two-spelling trap. test/hardcoded_tmp_pipeline_test.exs runs the real pipeline (collect_findings -> normalise -> suppress). Four mutants each killed: exemption key renamed (4 red), mktemp skip removed (1 red), n>1 guard reverted (1 red), inline_allowed? clause removed (3 red). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0136eszqrQ53Kj7aBH1D4rXK --- .claude/CLAUDE.md | 6 ++ .hypatia-ignore | 15 +++ lib/hypatia/scanner_suppression.ex | 11 +++ lib/rules/cicd_rules.ex | 50 +++++++++- test/hardcoded_tmp_pipeline_test.exs | 132 +++++++++++++++++++++++++++ 5 files changed, 209 insertions(+), 5 deletions(-) create mode 100644 test/hardcoded_tmp_pipeline_test.exs diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 9832cccc..654b26c2 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -388,6 +388,12 @@ Three mechanisms, in order of preference: ``` Recognised in `#`, `//`, `--`, `;` comment styles. A file-level directive in the first 20 lines covers every match in the file. + Content-pattern rules (`hardcoded_tmp`, `http_in_docs`, …) honour the + same directive on the matching or preceding line, under either module + spelling (`content_patterns/` or `cicd_rules/`). Until + 2026-09-30 they honoured only the bare `hypatia:ignore ` form and + silently ignored `hypatia: allow`. The file-level form is **not** + wired for content patterns. 3. **`.hypatia-ignore`** for file-scoped or directory-scoped exemptions that have a documented org-policy rationale. diff --git a/.hypatia-ignore b/.hypatia-ignore index 246b3df4..35d6bc5d 100644 --- a/.hypatia-ignore +++ b/.hypatia-ignore @@ -43,6 +43,21 @@ cicd_rules/banned_language_file:resolve.py # not duplicate them here. This file is for *repo-specific* exemptions that # carry a stated reason in the comment above each entry. # +# ⚠ TWO SPELLINGS, ONE MODULE. `` must be the string a finding +# is EMITTED under, which is not always the module that defines the rule: +# +# cicd_rules/banned_language_file: banned-language findings +# content_patterns/hardcoded_tmp: every content-pattern rule +# (hardcoded_tmp, http_in_docs, +# …) — normalised in cli.ex +# although defined in CicdRules +# +# `cicd_rules/hardcoded_tmp:…` matches NOTHING and fails silently. Copy the +# id from the alert (`hypatia//`), never from the +# source file. The spellings are pinned by test/hardcoded_tmp_pipeline_test.exs; +# they are not renamed because in-use ignore files across the estate depend +# on them. +# # Inline-allow alternative: for one or two sites, prefer an inline directive # at the call site: # diff --git a/lib/hypatia/scanner_suppression.ex b/lib/hypatia/scanner_suppression.ex index 88bc6ccd..eb47647e 100644 --- a/lib/hypatia/scanner_suppression.ex +++ b/lib/hypatia/scanner_suppression.ex @@ -86,6 +86,17 @@ defmodule Hypatia.ScannerSuppression do # code_safety / migration_rules exemptions above. "structural_drift" => %{ :any => @training_corpus_paths + }, + # ⚠ Keyed "content_patterns", NOT "cicd_rules". The content engine lives + # in CicdRules, but cli.ex normalises its findings with + # `rule_module: "content_patterns"` (alert ids read + # `hypatia/content_patterns/`). A "cicd_rules" key here would be + # vacuous — it names the module, not the string the consumer compares. + # Same training-corpus policy as above: a fixture photographing a bad + # pattern (launch-scaffolder's frozen /tmp launcher) is provenance, and + # the generator's own tests are its detector. + "content_patterns" => %{ + :any => @training_corpus_paths } } diff --git a/lib/rules/cicd_rules.ex b/lib/rules/cicd_rules.ex index 55d40c7b..581dfdf2 100644 --- a/lib/rules/cicd_rules.ex +++ b/lib/rules/cicd_rules.ex @@ -725,9 +725,23 @@ defmodule Hypatia.Rules.CicdRules do %{ id: :hardcoded_tmp, pattern: ~r/["'\/]tmp\//, - reason: "Hardcoded /tmp/ paths -- use mktemp", + # Two cures with OPPOSITE requirements (CWE-377). A pid/state file must + # be re-findable by the next invocation, so mktemp is wrong for it; a + # scratch file must not be predictable, so mktemp is right for it. + reason: + "Hardcoded /tmp/ path (CWE-377: predictable, shared, symlink-attackable). " <> + "Pid/state/log files: use a per-user dir, " <> + "${XDG_RUNTIME_DIR:-${XDG_STATE_HOME:-$HOME/.local/state}}// " <> + "(mktemp is wrong here: the next run cannot find the file). " <> + "Scratch files: mktemp -d with no /tmp template, plus trap 'rm -rf' EXIT.", applies_to: ["*.sh"], - exception: "Containerfile" + exception: "Containerfile", + # A comment naming /tmp is documentation, and a mktemp call is the + # rule's own cure (its random name defeats the predictable-path attack + # even under an explicit /tmp template). Flagging either sends people + # back into the alert they just fixed. + skip_comment_lines: true, + skip_if_line_matches: ~r/\bmktemp\b/ }, %{ id: :template_placeholder, @@ -803,6 +817,8 @@ defmodule Hypatia.Rules.CicdRules do * `negative: true` — emits one finding at line 1 when the regex is absent. * `skip_comment_lines: true` — ignores matching lines whose first non-whitespace characters are `#` or `//`. + * `skip_if_line_matches: ~r/.../` — ignores matching lines that also + match this regex (a rule's own remediation, e.g. `mktemp`). * `strip_yaml_comments: true` — removes unquoted YAML comments before matching while preserving the original line numbers and finding text. * Inline pragma — `hypatia:ignore ` on a matching line or the @@ -939,6 +955,11 @@ defmodule Hypatia.Rules.CicdRules do Map.get(rule, :skip_comment_lines, false) and comment_line?(line) -> [] + # A rule may name a line shape that is its own remediation + # (e.g. hardcoded_tmp and `mktemp`). Default nil: no rule changes. + skip_line_match?(rule, line) -> + [] + ignored?(rule.id, lines, n) -> [] @@ -957,6 +978,9 @@ defmodule Hypatia.Rules.CicdRules do end) end + defp skip_line_match?(%{skip_if_line_matches: %Regex{} = re}, line), do: Regex.match?(re, line) + defp skip_line_match?(_rule, _line), do: false + defp content_for_matching(rule, content) do if Map.get(rule, :strip_yaml_comments, false) do content @@ -997,12 +1021,28 @@ defmodule Hypatia.Rules.CicdRules do end # Inline pragma: this line OR the previous line carries - # `hypatia:ignore ` (in any comment syntax we recognise). + # `hypatia:ignore ` (in any comment syntax we recognise), or the + # estate-wide `hypatia: allow /` directive. Before this was + # wired, `hypatia: allow` was silently inert here although CLAUDE.md teaches + # it as the second rung of the suppression ladder. Both module spellings are + # accepted: the findings are EMITTED as `content_patterns` (cli.ex) but the + # rule lives in this module, so an author can reasonably write either. + # + # Rule ids are ATOMS (`:hardcoded_tmp`): the needle hides that through + # interpolation, but `inline_allowed?/4` compares strings, so stringify. + # + # `n == 1` has no previous line: `Enum.at(lines, -1)` would wrap to the LAST + # line, letting a trailing pragma suppress a hit on line 1. defp ignored?(rule_id, lines, n) do here = Enum.at(lines, n - 1, "") - prev = Enum.at(lines, n - 2, "") + prev = if n > 1, do: Enum.at(lines, n - 2, ""), else: "" needle = "hypatia:ignore #{rule_id}" - String.contains?(here, needle) or String.contains?(prev, needle) + + String.contains?(here, needle) or String.contains?(prev, needle) or + Enum.any?( + ["content_patterns", "cicd_rules"], + &Hypatia.ScannerSuppression.inline_allowed?(here, prev, &1, to_string(rule_id)) + ) end # C4 helper: is this line ENTIRELY a comment? Deliberately conservative for diff --git a/test/hardcoded_tmp_pipeline_test.exs b/test/hardcoded_tmp_pipeline_test.exs new file mode 100644 index 00000000..f2ccf3ae --- /dev/null +++ b/test/hardcoded_tmp_pipeline_test.exs @@ -0,0 +1,132 @@ +# SPDX-License-Identifier: MPL-2.0 + +defmodule Hypatia.HardcodedTmpPipelineTest do + use ExUnit.Case, async: true + + alias Hypatia.CLI + alias Hypatia.Rules.CicdRules + + # hardcoded_tmp end to end: the rule, its own-cure skips, and the + # training-corpus exemption — through the REAL pipeline (scan → + # cli.ex normalisation → ScannerSuppression), never a hand-built + # finding. A hand-built `%{rule_module: "content_patterns"}` fed to the + # suppressor would derive both legs from one source, so a rename on + # either side would leave it green. + + @pid_line ~s|PID_FILE="/tmp/stapeln-server.pid"\n| + + setup do + dir = Path.join(System.tmp_dir!(), "hyp-tmp-test-#{:erlang.unique_integer([:positive])}") + File.mkdir_p!(dir) + on_exit(fn -> File.rm_rf!(dir) end) + {:ok, dir: dir} + end + + defp write!(dir, rel, content) do + path = Path.join(dir, rel) + File.mkdir_p!(Path.dirname(path)) + File.write!(path, content) + end + + defp tmp_findings(dir) do + dir + |> CLI.collect_findings([:content_patterns]) + |> Enum.filter(&(&1.type == "hardcoded_tmp")) + end + + describe "the rule fires on a predictable path (positive control)" do + test "a /tmp pid file outside the training corpus is reported", %{dir: dir} do + write!(dir, "scripts/launcher.sh", @pid_line) + + assert [finding] = tmp_findings(dir) + assert finding.file =~ "scripts/launcher.sh" + assert finding.line == 1 + end + + test "the emitted rule_module is literally \"content_patterns\"", %{dir: dir} do + # Trap: CicdRules emits under TWO spellings — banned_language_file as + # "cicd_rules", the content engine as "content_patterns". Every + # suppression key and .hypatia-ignore line depends on this string. + write!(dir, "scripts/launcher.sh", @pid_line) + + assert [%{rule_module: "content_patterns"}] = tmp_findings(dir) + end + + test "the remediation distinguishes pid files from scratch files" do + rule = Enum.find(CicdRules.blocked_patterns(), &(&1[:id] == :hardcoded_tmp)) + + assert rule.reason =~ "XDG_RUNTIME_DIR" + assert rule.reason =~ "mktemp is wrong here" + assert rule.reason =~ "mktemp -d with no /tmp template" + end + end + + describe "the rule does not fire on its own cure" do + test "the XDG ladder is clean", %{dir: dir} do + write!( + dir, + "scripts/launcher.sh", + ~s|PID_FILE="${XDG_RUNTIME_DIR:-${XDG_STATE_HOME:-$HOME/.local/state}}/app/server.pid"\n| + ) + + assert tmp_findings(dir) == [] + end + + test "mktemp, even with an explicit /tmp template, is clean", %{dir: dir} do + write!(dir, "scripts/scratch.sh", ~s|d=$(mktemp -d /tmp/foo.XXXXXX)\n|) + + assert tmp_findings(dir) == [] + end + + test "a comment naming /tmp is clean", %{dir: dir} do + write!(dir, "scripts/doc.sh", ~s|# never write to "/tmp/x" here\n|) + + assert tmp_findings(dir) == [] + end + end + + describe "training-corpus exemption (content_patterns)" do + for fragment <- ["tests/fixtures/", "test/", "lib/rules/", "scripts/fix-scripts/"] do + test "a /tmp line under #{fragment} is suppressed", %{dir: dir} do + write!(dir, unquote(fragment) <> "minted-launcher.sh", @pid_line) + + assert tmp_findings(dir) == [] + end + end + end + + describe "inline directives (both suppression spellings)" do + for directive <- [ + "# hypatia: allow content_patterns/hardcoded_tmp -- pid must be re-findable", + "# hypatia: allow cicd_rules/hardcoded_tmp -- pid must be re-findable", + "# hypatia: allow hardcoded_tmp -- pid must be re-findable", + "# hypatia:ignore hardcoded_tmp -- pid must be re-findable" + ] do + test "#{directive} on the previous line suppresses", %{dir: dir} do + write!(dir, "scripts/launcher.sh", unquote(directive) <> "\n" <> @pid_line) + assert tmp_findings(dir) == [] + end + end + + test "a directive for a DIFFERENT rule does not suppress", %{dir: dir} do + write!( + dir, + "scripts/launcher.sh", + "# hypatia: allow content_patterns/http_in_docs -- unrelated\n" <> @pid_line + ) + + assert [_] = tmp_findings(dir) + end + + test "a directive on the LAST line does not reach a hit on line 1", %{dir: dir} do + # Enum.at(lines, -1) used to wrap line 1's "previous line" to the end. + write!( + dir, + "scripts/launcher.sh", + @pid_line <> "echo done\n# hypatia:ignore hardcoded_tmp" + ) + + assert [%{line: 1}] = tmp_findings(dir) + end + end +end From 07806f8bee48916b726da37c3e5f99629f2587ad Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Wed, 30 Sep 2026 11:08:47 +0100 Subject: [PATCH 2/2] docs(hypatia-ignore): the governance gate matches whole lines; the scanner matches fragments The two readers of this file ask different questions. The gate is the stricter one on purpose (per-path ledger), so the header now says so instead of letting a directory fragment quiet only half the pipeline. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0136eszqrQ53Kj7aBH1D4rXK --- .hypatia-ignore | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.hypatia-ignore b/.hypatia-ignore index 35d6bc5d..8d4aaae6 100644 --- a/.hypatia-ignore +++ b/.hypatia-ignore @@ -58,6 +58,14 @@ cicd_rules/banned_language_file:resolve.py # they are not renamed because in-use ignore files across the estate depend # on them. # +# ⚠ TWO READERS, TWO MATCHERS. The scanner treats as a SUBSTRING +# fragment. The governance gate (standards governance-reusable.yml, rules +# banned_language_file / banned_config_file) matches the WHOLE LINE +# `:` — so a directory fragment or bare-path +# line quiets the scanner but NOT the gate. This is deliberate: the gate is +# the stricter reader, and per-path listing (see the ledger header above) is +# what stops a new banned file being absorbed silently. List each path in full. +# # Inline-allow alternative: for one or two sites, prefer an inline directive # at the call site: #