ci: probe production health off-host on a schedule - #113
Open
aterga wants to merge 2 commits into
Open
Conversation
Item 8 of the rollout plan. The status dashboard already runs every check this needs, but it runs on the host and is pull-only: something has to look at it. A host that is down cannot tell you it is down, which is the one failure mode that matters most and the one the current setup structurally cannot report. This runs the same suite from GitHub's infrastructure every fifteen minutes. No new service, no new vendor, no dependencies — the dashboard package has none, so the probe is a checkout and a node invocation. Escalation is labelled honestly rather than overstated. A failure fails the workflow, which notifies watchers, and posts to Slack once a webhook secret exists. Neither is a page and nobody is on call, so the Slack step is conditional on the secret being present: the workflow is useful the day it lands and gains routing later with no code change. Real escalation belongs in the organisation's Prometheus/OpsGenie stack, where an alert reaches someone who has agreed to be woken; this is the stopgap and says so. Deliberately not --strict. That escalates warnings to failures, and a third of the suite probes Internet Identity and the IC, which this team does not operate. Paging on someone else's degradation is how a channel learns to ignore its alerts. Without the flag the CLI exits non-zero exactly when the overall verdict is "fail" — the same threshold the dashboard already uses to serve 503. Three things found by running it rather than reasoning about it. The probe refuses any origin outside its SSRF allow-list, so the production host has to be named explicitly or the run reports a usage error instead of a health verdict. Report sections carry `title`, not `label`, so the summary table would have rendered every section as "?". And the Slack step was gated on failure(), which never fires here: the probe swallows its own exit code on purpose, so nothing has failed at that point in the job — the notification would have been silently dead while the run still went red and looked correct from outside. Verified against three inputs: a healthy production report, a report from an origin that answers but is not the service (eight checks fail), and a missing report standing in for an unreachable host. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R8ZshwKmjD5fZ4Hs9dS6zh
There was a problem hiding this comment.
Pull request overview
Adds off-host production health monitoring through a scheduled GitHub Actions workflow.
Changes:
- Runs the MCP health suite every 15 minutes.
- Generates failure summaries and optionally notifies Slack.
- Marks unhealthy runs as failed.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Four findings, all valid, all confirmed before changing anything. No job timeout. The default is six hours, and `concurrency` here lets one run proceed without cancelling — so a wedged run does not merely waste a slot, it queues every later tick behind itself. Six hours is twenty-four missed probes during which the monitor says nothing, and silence from a monitor is indistinguishable from health. Capped below the cadence, and the Slack request now carries --max-time so a stalled webhook cannot consume the budget either. The webhook was mapped at job scope. Every step and the checked-out probe code could read it, none of which needs it, and log masking does not stop a process reading its own environment. It is now scoped to the single step that posts it. The step condition needs to know whether the secret exists without seeing it, so the job exports the comparison rather than the value — an expression that resolves to "true" or "false", since `secrets` is unavailable in a step `if:`. Node 20 is end-of-life since April and no longer receives security fixes, which is a poor choice for a job that parses responses from an internet-facing service. Moved to 22, which is what every probe run during development already used. deploy-native.yml and the host still pin 20; that is a separate change. The summary table interpolated values that are not ours. The probed server supplies II instance names in /version and the dashboard puts them straight into check labels and section titles, so a misconfigured or compromised host could inject a pipe, a newline or Markdown and corrupt or spoof the summary this job writes. Cells are now escaped and length-capped, verified against a report carrying a name built to forge an extra table row. That last one also falsified a comment written one commit earlier, which claimed the interpolated labels were our own static strings. They are not, and the note explaining why sanitising was cheap-but-optional now says the opposite. Same sink class, two places: the output line had been hardened while the table next to it had not. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R8ZshwKmjD5fZ4Hs9dS6zh
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Suppressed comments (4)
.github/workflows/health.yml:196
- The summary output is derived from server-advertised instance names, and interpolating it directly into a double-quoted shell command permits command substitution (for example, an instance name containing
$(...)) when a named II check fails. Pass the output throughenvso the shell treats it only as data.
echo "::error::production health check failed (${{ steps.summary.outputs.failed }})"
.github/workflows/health.yml:154
- Collapsing whitespace protects
$GITHUB_OUTPUT, but this value is later rendered as Slack mrkdwn. A server-advertised name such as<!channel>or<https://attacker.example|Run log>can therefore trigger mentions or spoof links in the alert. Slack requires&,<, and>to be entity-escaped when inserting untrusted text.
line = " ".join(line.split())[:400]
.github/workflows/health.yml:122
- This only escapes table separators and actually removes existing backslashes; Markdown constructs such as
[Run log](https://attacker.example), images, emphasis, and autolinks remain active. Since the value can contain a server-advertised instance name, it can still spoof the workflow summary despite the comment's stated protection. Escape all Markdown punctuation before rendering the cell.
This issue also appears in the following locations of the same file:
- line 154
- line 196
return " ".join(t.split()).replace("\\", "").replace("|", "\\|")[:120] or "?"
.github/workflows/health.yml:10
- Repository watchers are not automatically emailed for failed Actions runs. Scheduled-run notifications are associated with the scheduled workflow's actor (typically the user who last changed the cron) and that user's notification settings, so with the optional Slack secret absent this remains pull-only for the rest of the team rather than providing the alert described by the PR. Add or require an explicit team notification route before treating workflow failure as the baseline signal.
# Escalation, honestly labelled: a failure here fails the workflow, which
# notifies watchers by email, and posts to Slack when a webhook is configured.
sea-snake
approved these changes
Aug 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Item 8 of the rollout plan — "Set up alerts based on the status page".
The status dashboard already runs every check this needs (MCP, the full OAuth suite, TLS, and the Internet Identity linkage). But it runs on the host and it is pull-only: something has to look at it. A host that is down cannot tell you it is down, and that is both the failure mode that matters most and the one the current setup structurally cannot report.
This runs the same suite from GitHub's infrastructure every fifteen minutes. No new service, no new vendor, no dependencies —
monitoring/mcp-statushas none, so the probe is a checkout and anodeinvocation.Related issues
Complements the hosted dashboard rather than replacing it. The longer-term answer is the organisation's Prometheus/OpsGenie stack (
dfinity-ops/k8s), which is a separate conversation with #eng-observability.Changes
.github/workflows/health.yml— a scheduledprobejob.Design decisions worth review:
SLACK_WEBHOOK_URL— no code change. That deliberately keeps a configuration decision from blocking the signal.--strict. That flag escalates warnings to failures, and a third of the suite probes Internet Identity and the IC — infrastructure this team does not operate. Paging on someone else's degradation is how a channel learns to ignore its alerts. Without it the CLI exits non-zero exactly whenoverallisfail, the same threshold the dashboard uses to serve 503.Testing
Three defects were found by running it rather than reasoning about it, and all three would have shipped a workflow that looked correct:
id.ai, so it must be named explicitly — otherwise the run reports a usage error instead of a health verdict.title, notlabel. The summary table would have rendered every section as?.failure(), which never fires here. The probe swallows its own exit code on purpose, so nothing has failed at that point in the job. The notification would have been silently dead while the run still went red and looked correct from outside.Verified against three inputs:
overall: pass, 18 checks, summary reads "All checks passed."overall: fail, 8 checks fail, summary table renders real section titles and check labels.if:asserted by parsing the file, not by eye.$GITHUB_OUTPUTvalue collapsed to one line and length-capped — that file is line-oriented, so a newline in a value could declare further outputs. Static strings today; free to harden now.cargo/npm— no application code changed.Checklist
Limits
Fifteen minutes is the cadence, but GitHub's scheduler is best-effort and drifts under load, so treat this as noticed within the hour, not an SLA. Scheduled workflows also disable themselves after 60 days without repository activity.
There is no flap suppression: a sustained outage produces one failure per tick. If that proves noisy, transition-detection is the follow-up — I would rather see the real noise level than guess at it now.
To enable Slack: add a repository secret
SLACK_WEBHOOK_URLpointing at an incoming webhook for #eng-identity-imcp2. Nothing else changes.Generated by Claude Code