Skip to content

Harden alert lifecycle: TLS verification, cleared-issue detection, pagination - #2

Open
ethangardner wants to merge 1 commit into
task/initial-action-buildfrom
task/workflow-safeguards
Open

Harden alert lifecycle: TLS verification, cleared-issue detection, pagination#2
ethangardner wants to merge 1 commit into
task/initial-action-buildfrom
task/workflow-safeguards

Conversation

@ethangardner

@ethangardner ethangardner commented Sep 8, 2026

Copy link
Copy Markdown

Summary

Fixes two correctness/security issues found in review of #1 and hardens two related edge cases, all in the alert action's core logic (scripts/snapshot.py, scripts/issues.py, scripts/site_scanning_alerts.py), plus new test coverage for the previously-untested issue lifecycle.

Why

This Action fetches remote data and auto-opens/comments on issues unattended on a daily cron across every consuming repo, so correctness and network-security issues here have real blast radius: wrong data in → spam or missed issues out.

Changes

  • Removed disabled TLS certificate verification (snapshot.py). The snapshot fetch previously used ssl._create_unverified_context() for every request, including the default api.gsa.gov URL and any override supplied via the snapshot_url/previous_snapshot_url inputs. With verification off, anyone on-path could serve spoofed CSV content that gets parsed straight into GitHub issues — e.g. serving a "clean" snapshot to mask a real outage, or garbage data to spam every watched domain as broken. Now uses the default verified urlopen context.
  • Replaced fragile string-matching for "condition cleared" detection with an explicit flag (issues.py, site_scanning_alerts.py). handle_alert_lifecycle previously inferred whether alerts had cleared by searching the rendered markdown body for the literal phrases "have returned to normal" / "operating as expected" — a hidden coupling to wording chosen in rules.py that nothing enforced and nothing tested. A future wording tweak there would have silently broken the clear-detection path, leaving a resolved alert issue open forever. handle_alert_lifecycle now takes an explicit is_clear: bool argument; callers pass len(all_alerts) == 0.
  • Paginated find_open_issue (issues.py). The open-issue search previously read only the first 100 open issues for the label. If a label is ever shared with >100 other open issues, the real alert issue could sit past page 1 and the action would create a duplicate issue every run instead of updating the existing one. It now walks pages until a short page confirms there are no more.
  • Fixed a dropped issue URL on no-op clears (issues.py). When a condition cleared, an issue existed, but comment_on_clear was false, the function returned issue_url: None even though an open issue does exist — which the entrypoint then reports as N/A, reading as "no issue" when one is actually open. It now returns the existing issue's URL in that case.
  • New test coverage (tests/test_issues.py): the full create/no-op/comment/cleared decision matrix for handle_alert_lifecycle (previously untested — only fingerprint helpers and client init had coverage), plus tests for the new pagination behavior.

Testing

  • python3 -m unittest discover tests -v — 42 tests pass (6 new lifecycle tests, 2 new pagination tests).
  • No behavior change to the happy path: same rule evaluation, same rendering, same issue titles/labels.

@ethangardner ethangardner changed the title add workflow safeguards Harden alert lifecycle: TLS verification, cleared-issue detection, pagination Sep 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant