Skip to content

Commit b19a40c

Browse files
wshallwshallclaude
andcommitted
ci(notice): a red DAST nightly notified nobody, and the issue body named the wrong workflow (BACKLOG #318)
#318 carries this as a tracked follow-up rather than deferred scope: `dast.yml` has NO `pull_request` trigger, deliberately, and is not a required context, so every finding from the authenticated authorization sweep surfaced in the Actions tab and nowhere else. An authenticated security sweep reporting into the void is the exact shape `nightly-notice.yml` exists to end -- it just was not watching it. Widened `workflows:` to include DAST, which is the fix the item names. ALSO FIXED, because widening made it worse rather than introducing it: the issue BODY opened with a hardcoded "Nightly (scheduled) CI failed." whatever had actually run. The TITLE was already derived from $WF_NAME; the body was not, so a red Security run has been opening an issue whose first line names CI. With a third watched workflow that is a reader deciding what broke from a wrong sentence. It now reads from the same $WF_NAME the title does. NEW GUARD, one level up from the item. A watched name that no workflow answers to, or one whose workflow has no `schedule:` trigger, can never match: the notice job fires only when the completed run's event was `schedule`, so such an entry sits in the list looking like protection and matches nothing, forever, silently. That is the notice's own failure mode turned on itself. `test_every_watched_workflow_exists_and_can_actually_fire` asserts both arms for EVERY watched name, so a fourth workflow added later cannot be added wrongly. Its scan carries a positive control, because an empty name map would make every assertion in it vacuous. The module docstring claimed it "pins the three ways" the notice could stop working; there were already six tests. A count in prose has no checker and has to be maintained by whoever adds the next test, so it now states the kind rather than the number (SDS-3.6). Four mutants, all killed: drop DAST from the list; watch a name no workflow answers to; watch a real workflow that has no cron; restore the hardcoded CI in the body. Every mutant asserted a unique anchor and a changed file hash before scoring, and every restore was byte-identical. Two share a test by design -- they are its two arms -- and their failure messages were checked to differ, the typo case listing every name actually present. Scope: the nightly-notice widening only. Increment 2 of #318 (schema-driven breadth, the MLLP/TCP/X12 ingress fuzzing, the /ui plane, a TLS black-box target) is untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 71e559a commit b19a40c

2 files changed

Lines changed: 92 additions & 9 deletions

File tree

.github/workflows/nightly-notice.yml

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,23 @@ name: Nightly failure notice
2020
# SCOPE: schedule-only, deliberately. A push/PR failure is already visible on the PR itself; alerting
2121
# there would be pure noise, and `workflow_run` fires for every CI completion regardless of trigger.
2222
#
23-
# TWO WORKFLOWS, ONE ISSUE PER WORKFLOW. `Security` was added because its daily cron carries jobs that
24-
# do not run on a PR -- `released-line-audit` most of all, whose whole subject (the LATEST RELEASE) no
25-
# PR can change. The issue title is DERIVED from the completed workflow's name, so a green nightly CI
26-
# cannot close an issue opened by a red Security run: a single shared title would let one signal close
27-
# the other, which is the same silence this workflow exists to end.
23+
# THREE WORKFLOWS, ONE ISSUE PER WORKFLOW. `Security` was added because its daily cron carries jobs
24+
# that do not run on a PR -- `released-line-audit` most of all, whose whole subject (the LATEST
25+
# RELEASE) no PR can change. `DAST` was added for a stronger version of the same reason (BACKLOG
26+
# #318): it has NO `pull_request` trigger AT ALL, deliberately, so before this every DAST finding
27+
# surfaced in the Actions tab and nowhere else -- an authenticated authorization sweep reporting into
28+
# the void, which is precisely the shape this workflow exists to end.
29+
#
30+
# A WATCHED WORKFLOW MUST HAVE A `schedule:` TRIGGER. The job below fires only when the completed
31+
# run's event was `schedule`, so watching a workflow with no cron adds a name that can never match:
32+
# dead config that reads as coverage. `tests/test_nightly_notice.py` pins this for every watched name.
33+
#
34+
# The issue title is DERIVED from the completed workflow's name, so a green nightly CI cannot close an
35+
# issue opened by a red Security or DAST run: a single shared title would let one signal close
36+
# another, which is the same silence this workflow exists to end.
2837
on:
2938
workflow_run:
30-
workflows: ["CI", "Security"]
39+
workflows: ["CI", "Security", "DAST"]
3140
types: [completed]
3241

3342
# Read-only by default; the one job that writes escalates to `issues: write` and nothing else.
@@ -93,7 +102,7 @@ jobs:
93102
exit 0
94103
fi
95104
96-
BODY="Nightly (scheduled) CI failed.
105+
BODY="Nightly (scheduled) $WF_NAME failed.
97106
98107
- run: $RUN_URL
99108
- commit: \`$HEAD_SHA\`
@@ -102,7 +111,8 @@ jobs:
102111
A scheduled run is not a PR context, so this failure appears nowhere else. For CI that is the
103112
server-DB store, load/throughput and service-smoke legs the three required \`test\` legs skip;
104113
for Security it is the daily dependency audits and the released-line audit, whose subject is a
105-
published release that no pull request can change.
114+
published release that no pull request can change; for DAST it is the entire workflow, which
115+
carries no \`pull_request\` trigger by design.
106116
107117
This issue is opened once and commented on each subsequent failure; it closes itself when a
108118
nightly goes green again."

tests/test_nightly_notice.py

Lines changed: 74 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
suites (exactly what the three required ``test`` legs SKIP) could break invisibly.
1111
1212
``.github/workflows/nightly-notice.yml`` turns that silence into one deduplicated issue. This module
13-
pins the three ways it could quietly stop working.
13+
pins the structural ways it could quietly stop working -- at least the ones checkable before it ships.
1414
1515
WHAT CANNOT BE TESTED HERE, stated rather than papered over. A ``workflow_run`` workflow only triggers
1616
from the **default branch**, so this one cannot fire on the PR that adds it — its end-to-end behaviour
@@ -70,6 +70,79 @@ def test_it_also_watches_the_security_workflow() -> None:
7070
)
7171

7272

73+
def test_it_also_watches_the_dast_workflow() -> None:
74+
"""DAST needs this more than either of the others (BACKLOG #318).
75+
76+
``dast.yml`` has NO ``pull_request`` trigger at all -- deliberately -- so before this widening a
77+
genuine authorization finding surfaced in the Actions tab and nowhere else. An authenticated
78+
security sweep reporting into the void is the exact shape this notice exists to end.
79+
"""
80+
watched = _on(_load(_NOTICE))["workflow_run"]["workflows"]
81+
dast_name = _load(_WORKFLOWS / "dast.yml").get("name")
82+
assert dast_name, "dast.yml has no `name:` -- workflow_run has nothing to key on"
83+
assert dast_name in watched, (
84+
f"nightly-notice.yml watches {watched} but dast.yml is named {dast_name!r}. Its findings "
85+
"would then reach nobody, which is the gap BACKLOG #318 recorded."
86+
)
87+
88+
89+
def test_every_watched_workflow_exists_and_can_actually_fire() -> None:
90+
"""A watched name that no workflow answers to, or that has no cron, is dead config reading as
91+
coverage.
92+
93+
The notice job gates on ``workflow_run.event == 'schedule'``, so a watched workflow with no
94+
``schedule:`` trigger can never satisfy it -- the name sits in the list looking like protection
95+
and matches nothing, forever, silently. That is the same failure the notice exists to fix, one
96+
level up, so it is asserted for EVERY watched name rather than per workflow.
97+
"""
98+
watched = _on(_load(_NOTICE))["workflow_run"]["workflows"]
99+
assert watched, "the watch list is empty"
100+
101+
by_name: dict[str, Path] = {}
102+
for path in sorted(_WORKFLOWS.glob("*.yml")):
103+
name = _load(path).get("name")
104+
if isinstance(name, str):
105+
by_name.setdefault(name, path)
106+
# Positive control: the scan must actually be reading workflows, or every assertion below would
107+
# be vacuous against an empty map.
108+
assert len(by_name) > 5, f"the workflow scan found only {len(by_name)} named files"
109+
110+
for name in watched:
111+
path = by_name.get(name)
112+
assert path is not None, (
113+
f"nightly-notice.yml watches {name!r} but no workflow in {_WORKFLOWS.name}/ is named that. "
114+
f"A workflow_run trigger matches on the NAME, so this entry can never fire. "
115+
f"Names present: {sorted(by_name)}"
116+
)
117+
triggers = _on(_load(path))
118+
assert "schedule" in triggers, (
119+
f"nightly-notice.yml watches {name!r} ({path.name}) but that workflow has no `schedule:` "
120+
"trigger. The notice job only fires when the completed run's event was `schedule`, so "
121+
"this entry can never match -- dead config that reads as coverage."
122+
)
123+
124+
125+
def test_the_issue_body_names_the_workflow_that_failed() -> None:
126+
"""The TITLE was always derived from the completed workflow; the BODY was not.
127+
128+
It opened with a hardcoded "CI failed" whatever had run, so a red Security run produced an issue
129+
whose first line named the wrong workflow. Harmless-looking, and exactly the kind of thing a
130+
reader uses to decide what broke. Widening the watch list to a third workflow made it worse
131+
rather than introducing it.
132+
"""
133+
body = "\n".join(
134+
str(s.get("run", "")) for s in _load(_NOTICE)["jobs"]["notice"]["steps"] if "run" in s
135+
)
136+
assert body, "the notice job has no `run:` step to inspect"
137+
assert "Nightly (scheduled) $WF_NAME failed." in body, (
138+
"the issue body does not name the workflow that actually failed. It must read from $WF_NAME, "
139+
"the same value the title is derived from, or it will assert the wrong workflow broke."
140+
)
141+
assert "Nightly (scheduled) CI failed." not in body, (
142+
"the body still hardcodes CI, so a Security or DAST failure opens an issue naming CI."
143+
)
144+
145+
73146
def test_it_only_reacts_to_scheduled_runs() -> None:
74147
"""Without this gate every PR and push failure opens an issue.
75148

0 commit comments

Comments
 (0)