fix(885): an if: is an expression unwrapped, so the scoped detector reads that key whole

The `${{ }}` span scoping added in a6225e4ee was correct about prose and wrong about one
real spelling: `if:` is the only key the expression grammar lets omit the delimiters in, so
`if: secrets.REGISTRY_PASSWORD != ''` named a stored secret in a document holding no `${{`
at all, and the collector reported it clean. Measured on the previous head e35e1b772:
`secret_refs("secrets.REGISTRY_PASSWORD != ''")` -> `[]`, and the same string as a
job-level or step-level `if:` on a synthetic `pull_request` job -> `stored_secret_faults(...)
== []`. That an unwrapped condition is evaluated is not inferred — `docker-build.yml`'s own
`build` job carries `if: github.event_name != 'pull_request'` bare, and `PR_EXCLUDING_IFS`
pins that exact string.

`condition_refs` reads an `if:` value as one span with the delimiters neutralised to a
SPACE (deleting them collapses `${{ secrets.A }}${{ secrets.B }}` into the single identifier
`secrets.Asecrets`, losing a reference), and `secret_name_counts` routes the value there
instead of onto the stack, so a wrapped condition still counts once. Everywhere else the
scoping stands and the English `# We pass no secrets. Then …` still costs nothing.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015QqCpYFsKgnAnx6jVwrKiV

scripts/tests/test_workflow_persist_credentials.py

@@ -390,11 +390,43 @@ _SECRET_REF = re.compile(
 # `secret_refs` below rather than in the patterns, which cannot express "within the enclosing span".
 # Applied to the whole string instead, `_SECRET_REF` reads the English `# We pass no secrets. Then
 # the pull is anonymous.` as a reference to a secret named `Then` — a fabricated name, faulting a
-# PR-route job for a comment, on this branch's own subject. A reference the runner actually resolves
-# is always inside an expression, so the scoping costs no real spelling.
+# PR-route job for a comment, on this branch's own subject.
+#
+# THE SCOPING COSTS EXACTLY ONE REAL SPELLING, AND IT IS NOT NOTHING: an `if:` value is an expression
+# WHETHER OR NOT it is wrapped, so `if: secrets.REGISTRY_PASSWORD != ''` names a stored secret in a
+# document that contains no `${{` at all. That an unwrapped condition is evaluated rather than read
+# as text is demonstrated by this repo's own `docker-build.yml` and not inferred — `build` carries
+# `if: github.event_name != 'pull_request'` bare, and `PR_EXCLUDING_IFS` below pins that exact
+# unwrapped string. `if:` is the only key whose value the grammar lets omit the delimiters, so the
+# scoping is repaired AT THAT KEY rather than abandoned: `condition_refs` reads an `if:` value as one
+# span, and `secret_name_counts` routes the value there instead of through `secret_refs`. Everywhere
+# else the scoping stands, and the English sentence above still costs nothing.
 _EXPRESSION = re.compile(r"\$\{\{(.*?)\}\}", re.S)
 _SECRETS_TOKEN = re.compile(r"(?<![A-Za-z0-9_])secrets(?![A-Za-z0-9_])", re.I)
 
+# `${{` and `}}` as tokens, for reading an `if:` value that mixes the wrapped and unwrapped forms.
+# They are replaced by a SPACE and not deleted: `${{ secrets.A }}${{ secrets.B }}` collapsed by
+# deletion reads as the one identifier `secrets.Asecrets` and LOSES a reference, the fail-open
+# direction.
+_EXPRESSION_DELIMITER = re.compile(r"\$\{\{|\}\}")
+
+# The one key whose value the expression grammar evaluates with the delimiters omitted. Matched
+# case-INSENSITIVELY where it is read, the direction `_SECRET_REF` and `SECRETS_KEY` both take.
+IF_KEY = "if"
+
+
+def _refs_in_expression(expression: str) -> list[str]:
+    """Every `secrets` reference inside ONE expression span — the resolved names, then the residue.
+
+    Shared by both entry points, so that widening a spelling widens the wrapped and the unwrapped
+    reading together: a second copy of this resolution would be free to drift from the one the
+    assertion runs on, which is the shape this guard exists to catch.
+    """
+    resolved = [next(group for group in m.groups() if group is not None) for m in _SECRET_REF.finditer(expression)]
+    # The residue: `secrets` tokens in this span that resolved to no literal name.
+    residue = len(_SECRETS_TOKEN.findall(expression)) - len(resolved)
+    return resolved + [WHOLE_SECRETS_CONTEXT] * residue
+
 
 def secret_refs(text: str) -> list[str]:
     """Every `secrets` reference in one string — occurrences, not names.
@@ -405,13 +437,22 @@ def secret_refs(text: str) -> list[str]:
     """
     found: list[str] = []
     for expression in _EXPRESSION.findall(text):
-        resolved = [next(group for group in m.groups() if group is not None) for m in _SECRET_REF.finditer(expression)]
-        found.extend(resolved)
-        # The residue: `secrets` tokens in this span that resolved to no literal name.
-        found.extend([WHOLE_SECRETS_CONTEXT] * (len(_SECRETS_TOKEN.findall(expression)) - len(resolved)))
+        found.extend(_refs_in_expression(expression))
     return found
 
 
+def condition_refs(condition: str) -> list[str]:
+    """Every `secrets` reference in an `if:` value, which is an expression with or without `${{ }}`.
+
+    The WHOLE value is read as one span, with the delimiters neutralised rather than honoured, so
+    that a condition mixing the two forms — `${{ true }} && secrets.X != ''` — is covered by the same
+    read as the bare one, and a fully wrapped condition still counts each reference exactly once.
+    This is strictly more demanding than `secret_refs` on the same string and never less: an `if:` is
+    never prose, so the over-match the span scoping exists to avoid cannot arise here.
+    """
+    return _refs_in_expression(_EXPRESSION_DELIMITER.sub(" ", condition))
+
+
 # The ONLY job-level `if:` in this repo that takes a job OFF the `pull_request` route. This is a PIN,
 # not an expression parser, and the direction is the point: an `if:` that is not in this set leaves
 # the job IN the population, so an unrecognised guard reddens rather than exempting. Parsing
@@ -483,6 +524,12 @@ def secret_name_counts(node: object) -> Counter:
     only this direction is lossless: `secret_names` is derived from it below. A second traversal
     with a different accumulator would be a copy of a mechanism, free to drift from the one the
     assertion runs on — the shape this guard exists to catch, in the guard itself.
+
+    One key is read differently, and it is a KEY rather than a place in the document: the value of an
+    `if:` is an expression with or without `${{ }}`, so it goes through `condition_refs`. Read as an
+    ordinary string it would be scoped to its `${{ }}` spans and a bare `if: secrets.X != ''` would
+    be invisible, on the route where the head writes the file. The value is routed there INSTEAD of
+    onto the stack, so a wrapped condition is counted once rather than twice.
     """
     found: Counter = Counter()
     stack: list[object] = [node]
@@ -491,7 +538,10 @@ def secret_name_counts(node: object) -> Counter:
         if isinstance(item, dict):
             for key, value in item.items():
                 stack.append(key)
-                stack.append(value)
+                if isinstance(key, str) and key.lower() == IF_KEY and isinstance(value, str):
+                    found.update(condition_refs(value))
+                else:
+                    stack.append(value)
         elif isinstance(item, list):
             stack.extend(item)
         elif isinstance(item, str):
@@ -682,6 +732,13 @@ def test_the_DOCUMENT_walk_finds_every_secret_reference_the_TEXT_does() -> None:
     A YAML anchor/alias would also redden benignly (the walk visits the aliased node once per
     reference, the text carries `*alias`); measured 2026-09-05 no tracked workflow uses one.
 
+    A THIRD asymmetry, which is not the strip's: an `if:` value is read by `condition_refs`, so an
+    UNWRAPPED `if: secrets.X != ''` is counted by the walk and not by the text half, which has no key
+    to read it by and is scoped to `${{ }}` spans. The walk's count is then the larger one, which
+    reddens — the safe direction, and on a document that is already faulting the stored-secret
+    assertion for the same reference. Measured 2026-09-05, no tracked workflow names a secret in an
+    `if:` at all, so nothing in the tree reaches it.
+
     WHAT THIS CROSS-CHECK STRUCTURALLY CANNOT REPORT, since it is the reason `secret_refs` has to be
     widened rather than leaned on: both halves read through that one function, so a spelling IT does
     not recognise is invisible to both and they agree at zero. The dot/index/whole-context spellings
@@ -905,6 +962,57 @@ def test_the_collector_sees_every_SPELLING_of_a_secret_reference() -> None:
         as_text = f"jobs:\n  j:\n    steps:\n      - run: |\n          {sentence}\n          true\n"
         assert walk_versus_text_faults("synthetic.yml", as_text) == [], sentence
 
+    # AND THE SPELLING THAT SCOPING COSTS: an `if:` value is an expression whether or not it is
+    # wrapped, so a condition naming a stored secret contains no `${{` and the span scoping cannot
+    # see it. `docker-build.yml`'s own `build` job carries an unwrapped `if:`, so this is the shape
+    # the repo already writes and not a hypothetical. Driven against the REAL predecessor: reading
+    # the value as an ordinary string — which is how `secret_name_counts` routed it before
+    # `condition_refs` — is `secret_refs`, and it is asserted empty on every row.
+    condition = "secrets.REGISTRY_PASSWORD != ''"
+    assert secret_refs(condition) == [], (
+        "an unwrapped condition is supposed to be invisible to the span-scoped reader — if it is "
+        "not, these rows prove nothing about `condition_refs`."
+    )
+    assert condition_refs(condition) == ["REGISTRY_PASSWORD"]
+
+    job_level = {True: {"pull_request": None}, "jobs": {"j": {"if": condition, "steps": [{"run": "true"}]}}}
+    step_level = {
+        True: {"pull_request": None},
+        "jobs": {"j": {"steps": [{"if": "secrets.RENOVATE_TOKEN != ''", "run": "true"}]}},
+    }
+    for doc, name in ((job_level, "REGISTRY_PASSWORD"), (step_level, "RENOVATE_TOKEN")):
+        faults = stored_secret_faults("synthetic.yml", doc)
+        assert len(faults) == 1, (name, faults)
+        assert name in faults[0], (name, faults)
+
+    # A WRAPPED condition counts once, not twice — the delimiters are neutralised rather than read as
+    # a second span, so the cross-check still agrees with the text half on the shape workflows write.
+    wrapped = "jobs:\n  j:\n    if: ${{ secrets.REGISTRY_PASSWORD != '' }}\n    steps:\n      - run: true\n"
+    assert secret_name_counts(yaml.safe_load(wrapped)) == Counter({"REGISTRY_PASSWORD": 1})
+    assert walk_versus_text_faults("synthetic.yml", wrapped) == []
+
+    # A condition MIXING the two forms is read whole, so the unwrapped half is not lost behind the
+    # wrapped one.
+    assert condition_refs("${{ true }} && secrets.RENOVATE_TOKEN != ''") == ["RENOVATE_TOKEN"]
+    # And two wrapped references in one condition stay two: the delimiters become a SPACE, so the
+    # names cannot collapse into one identifier.
+    assert condition_refs("${{ secrets.REGISTRY_USER }}${{ secrets.REGISTRY_PASSWORD }}") == [
+        "REGISTRY_USER",
+        "REGISTRY_PASSWORD",
+    ]
+
+    # The clause is on the `if:` KEY, so the English sentences above are untouched by it: they are
+    # `run:` scalars, where the word is prose and the scoping still costs nothing.
+    still_clean = {
+        True: {"pull_request": None},
+        "jobs": {"j": {"if": "github.event_name == 'push'", "steps": [{"run": "# We pass no secrets. Then true"}]}},
+    }
+    assert stored_secret_faults("synthetic.yml", still_clean) == []
+
+    # The pinned exclusion is itself an unwrapped condition and must stay clean — it names no secret,
+    # and reading conditions must not start faulting every gated job.
+    assert condition_refs("github.event_name != 'pull_request'") == []
+
 
 def test_a_SECRETS_HANDOVER_naming_nothing_is_reported_under_the_WHOLE_CONTEXT_sentinel() -> None:
     """`secrets: inherit` hands the WHOLE store to a `uses:` job while naming no secret at all.