Announce synthetic traffic with X-Blog-Check on every live-check request - #73
Conversation
97.7% of the traffic reaching production is ours, and the only thing separating it from real visitors was a coincidence: the CI runner's curl and the VPS host's curl are byte-identical at 8.5.0, so user agent cannot tell our deploy gate from their smoke probe, and the CI half of the client address rotates every run. One runner image bump merges the two and nothing would report it. The host side captures the field already and their ci/smoke.sh sends vps/smoke. This is the other half. The value carries provenance rather than a boolean, <source>/<id>, so "which run produced this 404" is one query rather than a correlation across timestamps. Derived rather than configured: github/<run-id>-<attempt> under Actions, proxmox/manual elsewhere, and CHECK_TAG overrides both to name a purpose for a hand run. The run attempt is in the id deliberately, which is a refinement on the design as proposed. A re-run of a failed workflow keeps its GITHUB_RUN_ID and takes a new GITHUB_RUN_ATTEMPT, so the id alone merges a retry into the run it was retrying, and that is exactly the case someone reads the log to understand. It is two curl config files rather than one, and that is the part not to collapse later. The tag is unconditional; the Pangolin token is sent only to the origin it belongs to, because a redirect that one day points off-site must not mail the credential there. Folding them together would either give the tag that restriction for no reason or take it away from the token. Both are now assembled from the tag first, with the token appended where it is allowed, so every request is attributable including the off-site hop that deliberately carries no credential. Verified rather than assumed: the header is on the wire under curl -v, all three derivations produce the expected value, the full run still reports PASS - 1253 URLs honored, and two tagged requests were sent to production for the host side to confirm capture. CHECK_TAG is added to the env-docs gate's KNOBS and described in ENVIRONMENT.md, and the gate was watched failing on it before the row was written. Design agreed with the VPS agent in their section 31. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds a synthetic-traffic provenance header (X-Blog-Check) to all requests made by the live URL contract checker, so production logs can reliably distinguish CI/probes from real visitors and attribute failures to a specific run/attempt.
Changes:
- Emit an
X-Blog-Check: <source>/<id>header on everychecks/check-live-urls.shrequest, deriving a default tag from GitHub Actions run ID + attempt or a localproxmox/manualfallback. - Keep credential and non-credential curl config separated (tag always; token only same-origin), while ensuring the tag is still carried across off-site hops.
- Document the new
CHECK_TAGknob and teach the env-docs gate to accept it.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| ENVIRONMENT.md | Documents the CHECK_TAG per-invocation knob and its derived default values. |
| checks/check-live-urls.sh | Adds unconditional X-Blog-Check header via a curl config file and threads it through same-origin credential handling. |
| checks/check-env-docs.py | Updates the env-docs validation allowlist to include CHECK_TAG. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…t headers A curl config file is a list of directives rather than a list of headers, so a value interpolated into a quoted `header = "..."` line can leave it. A newline ends the line and starts a new directive; a double quote closes the string with the same effect. Either turns an override into "add a curl option nobody typed". CHECK_TAG takes a strict allowlist, because this repo defines its grammar: the `<source>/<id>` the design already states, so letters, digits, dot, underscore, hyphen and the separating slash. Anything else fails at entry. The Pangolin token values take a narrower rule, refusing only a quote or a newline, because the grammar of a credential belongs to its issuer and not to this script. Neither character is legal in an HTTP header value, so a token carrying one is a paste accident rather than a token. The failure names the variable and never echoes the value. Each guard was demonstrated failing rather than assumed: CHECK_TAG=$'proxmox/x\noutput = /tmp/pwned' exit 2 CHECK_TAG='proxmox/x" header = "X-Evil: 1' exit 2 CHECK_TAG='proxmox/media dev' exit 2 PANGOLIN_ACCESS_TOKEN_ID=$'a\nb' exit 2 And that exercise found one of my own: I had added a guard rejecting an empty CHECK_TAG, and it is unreachable. An empty value is already treated as unset and takes the derived default, which is the behaviour we want, so the guard could never fire and its message told the reader the opposite of what happens. Removed rather than left as a rule describing a case that cannot occur. Full run still reports PASS - 1253 URLs honored. shellcheck and shfmt clean. Found by Copilot review on #73. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (1)
checks/check-live-urls.sh:81
- The CHECK_TAG validation comment/doc says the grammar is
<source>/<id>, but the current check only enforces an allowlist of characters and will accept values with no slash (or leading/trailing slash), which undermines the provenance format and makes log queries less reliable. Consider enforcing the exact<source>/<id>shape (one slash, non-empty halves) in the validation itself.
# The grammar is the one the design already states, `<source>/<id>`, kept deliberately narrow:
# letters, digits, dot, underscore, hyphen, and the slash that separates the two halves.
case "$CHECK_TAG" in
*[!A-Za-z0-9._/-]*)
echo "FAIL CHECK_TAG may contain only letters, digits, and . _ - / -- got '$CHECK_TAG'" >&2
…bing it The validation checked a character set and the comment above it said the grammar was <source>/<id>. Those are different rules, and the weaker one was the one running: `smoke`, `/smoke`, `proxmox/` and `a/b/c` all passed while reading as conforming. That matters because the shape is the whole point of provenance over a boolean. Grouping the log by source is only reliable if every tag has a source half, and a tag with no slash or three slashes breaks the query quietly rather than loudly. Now enforced: exactly one slash, both halves non-empty, from the same narrow character set. Each rule was demonstrated rejecting and the two legal shapes demonstrated passing: smoke must be <source>/<id> /smoke needs a non-empty half either side proxmox/ needs a non-empty half either side a/b/c takes exactly one / proxmox/media dev character set proxmox/media-dev accepted github/999-3 accepted Both derived defaults were re-checked against their own rule rather than assumed to satisfy it, which is the failure mode of adding a validator after the values it governs. ENVIRONMENT.md now states the shape is enforced instead of expected. Full run still PASS - 1253 URLs honored. Found by Copilot review on #73, as a suppressed comment. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Fixed in fc81267. Right, and this is the same failure that has come up repeatedly today: a comment describing a rule stricter than the one actually running.
The charset check passed all of these while every one of them reads as conforming: Why the shape and not just the characters. The whole value of provenance over a boolean is grouping the log by source — Now enforced: exactly one I also re-checked the two derived defaults against the new rule rather than assuming they satisfy it —
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (2)
checks/check-live-urls.sh:95
- This failure message also has an extra space before the comma (
<source>/<id> ,), which looks like a typo. Tighten punctuation so the diagnostic reads naturally.
*)
echo "FAIL CHECK_TAG must be <source>/<id> , such as proxmox/media-dev -- got '$CHECK_TAG'" >&2
exit 2
checks/check-live-urls.sh:86
- The error message has an extra space before the comma (
/ ,), which reads like a typo in a user-facing failure path. Remove the stray space to keep diagnostics clean.
This issue also appears on line 93 of the same file.
*/*/*)
echo "FAIL CHECK_TAG takes exactly one / , as <source>/<id> -- got '$CHECK_TAG'" >&2
exit 2
A stray space before a comma, in `one / , as <source>/<id>` and in `<source>/<id> , such as`, which reads as a typo in the one place a reader is already confused. It came from trying to keep a bare `/` from running into the punctuation after it. Fixed by naming the character instead of printing it: "takes exactly one slash" and "either side of the slash" have nothing to collide with. The example message loses the space and keeps the comma. The character-set message had the same collision in a different form, `. _ - / -- got`, where the slash ran into the separator. The set is quoted now rather than bare. All four read back as a user sees them: may contain only letters, digits, and the characters '. _ - /' -- got 'proxmox/media dev' takes exactly one slash, as <source>/<id> -- got 'a/b/c' needs a non-empty half either side of the slash -- got '/smoke' must be <source>/<id>, such as proxmox/media-dev -- got 'smoke' Found by Copilot review on #73, which also pointed out the second instance rather than only the first. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Fixed in 84db125, both instances plus a third of the same shape you did not flag.
Both were mine trying to keep a bare The third instance was the character-set message, where the slash ran into the separator the same way: Read back as a user sees them: Full run still Thanks for naming the second occurrence explicitly rather than only the first — that is what made me sweep for the third instead of fixing the two and moving on. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (1)
ENVIRONMENT.md:92
- The CHECK_TAG row says the value is limited to letters/digits and
.,_,-, but the enforced format is<source>/<id>(which necessarily includes a single/). Update the wording so the allowed character set matches the script’s validation (slash allowed only as the single separator).
| `CHECK_TAG` | the `X-Blog-Check` provenance this run announces on every request. **`<source>/<id>` is enforced, not merely expected**: exactly one `/`, both halves non-empty, and only letters, digits, `.`, `_`, `-`. Rarely set by hand, since `check-live-urls.sh` derives `github/<run-id>-<attempt>` under Actions and `proxmox/manual` elsewhere. Set it to name a purpose for a hand run, as `proxmox/media-dev` |
The ENVIRONMENT.md row required "exactly one /" and then said the value may contain "only letters, digits, . _ -", a set with no slash in it. Read strictly the two clauses contradict; read charitably the reader has to guess that the character set governs the halves rather than the whole. The row now says the slash is the separator and the only one allowed, and that each half is drawn from the character set. That is what the script enforces: the charset check permits a slash anywhere, and the two rules after it leave exactly one, in the separator position, so each half can only contain the rest. Found by Copilot review on #73, as a suppressed comment. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Fixed in cd95e28.
Right — the row required "exactly one Now:
That is what the script actually does, and it is worth spelling out because the implementation gets there in two steps rather than one: the charset check permits a slash anywhere, and the two rules after it leave exactly one in the separator position — so each half can only contain the rest. The doc now describes the net effect rather than either half of the mechanism. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (1)
checks/check-live-urls.sh:81
- The CHECK_TAG allowlist uses glob ranges (
A-Za-z0-9) which are locale-dependent in shell pattern matching. In non-C locales, these ranges can include unexpected characters, weakening the "strict allowlist" guarantee for values written into the curl config file. Prefer an explicit ASCII character class (or forceLC_ALL=C) to keep the validation stable across environments.
case "$CHECK_TAG" in
*[!A-Za-z0-9._/-]*)
echo "FAIL CHECK_TAG may contain only letters, digits, and the characters '. _ - /' -- got '$CHECK_TAG'" >&2
…sts on it `A-Za-z0-9` in a shell pattern is a collation range, not an ASCII range, so what the allowlist actually permits depends on a shell option this script never set. It passes today because `globasciiranges` is on by default in bash 5.2, which makes the guarantee an accident of the build. Demonstrated rather than argued, under en_US.UTF-8: shopt -u globasciiranges aé -> ACCEPT aÉ -> ACCEPT shopt -s globasciiranges aé -> REJECT aÉ -> REJECT So a value containing an accented letter would have been written into the curl config on a shell where the option is off, while the comment above it called the set narrow. The consequence was never a config injection: neither a quote nor a newline can arrive this way, and those are the two characters that break out of a quoted config line. What was wrong is the claim. A validator that describes itself as strict has to be strict on its own terms rather than on the terms of whatever shell runs it. Set explicitly, and re-verified against the hostile case: with the option forced off beforehand, in a UTF-8 locale, `proxmox/café` is rejected. Full run still PASS - 1253 URLs honored. shellcheck and shfmt clean. Found by Copilot review on #73, as a suppressed comment. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Fixed in 27cce35, and this one reproduces. I nearly declined it, then measured.
It does not misbehave here — bash 5.2.37 has But the guarantee was resting on a default the script never set. With the option off, in a UTF-8 locale: So on a shell where it is off, an accented letter would have been written into the curl config while the comment above called the set narrow. Worth being precise about the consequence, because it is smaller than "weakens the allowlist" suggests and the reason matters. Neither a quote nor a newline can arrive through a widened letter range, and those are the only two characters that break out of a quoted config line — so this was never a path to config injection. What was wrong is the claim: a validator describing itself as strict has to be strict on its own terms rather than on the terms of whatever shell happens to run it. That is the same failure as the last two rounds — a rule asserting more than it enforced — arriving a third time in a different disguise. Good catch. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (2)
checks/check-live-urls.sh:121
- The Pangolin token guard rejects quotes and LF, but it still allows carriage returns.
\ris also invalid in HTTP header values and can participate in header injection/syntax breakage in the generated curl config. Reject CR as well (alongside LF).
for name in PANGOLIN_ACCESS_TOKEN_ID PANGOLIN_ACCESS_TOKEN; do
case "${!name}" in
*'"'* | *$'\n'*)
echo "FAIL $name contains a quote or a newline, which cannot appear in an HTTP header value" >&2
exit 2
checks/check-live-urls.sh:85
shopt -s globasciirangesis relied on to make the[A-Za-z0-9]allowlist ASCII-strict, but the script does not fail if thatshoptis unsupported (there is noset -e). On a system with an older bash, the validation can silently fall back to locale-dependent ranges, contradicting the guarantee in the surrounding comment.
This issue also appears on line 117 of the same file.
# The range `A-Za-z0-9` is collation-dependent, so the allowlist below is only ASCII-strict
# because `globasciiranges` happens to be on. Set explicitly rather than inherited, since a
# guarantee resting on a build default is not a guarantee. Demonstrated rather than assumed:
# with the option off, under en_US.UTF-8, `aé` and `aÉ` are both ACCEPTED by this pattern,
# and with it on they are rejected.
shopt -s globasciiranges
case "$CHECK_TAG" in
Two findings, and the second is the more embarrassing. The token guard refused a quote and a newline and allowed a carriage return. Header injection is classically CRLF and a lone CR is enough on its own, so refusing LF while allowing CR leaves exactly the shape the guard exists for. Measured before fixing: a token containing \r was accepted. Both variables now reject all three, and the LF and quote cases were re-run to confirm nothing regressed. And the `shopt -s globasciiranges` added in the previous commit was itself unchecked. This script runs under `set -uo pipefail` and not `-e`, so on a shell without that option the command prints to stderr, returns 1, and execution steps straight over it — leaving the allowlist locale-dependent underneath a comment promising it is not. Which is the same defect the previous commit set out to fix, reintroduced one line below it: a guarantee resting on something nobody checked. `shopt` returns 1 on an unknown option name, verified, so the failure is now fatal and says why. Full run still PASS - 1253 URLs honored. shellcheck and shfmt clean. Both found by Copilot review on #73, as suppressed comments. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Both fixed in af22557. The second one is the more embarrassing and the better catch. 1. CR in the token guard
Correct, and measured before fixing — a token containing Both variables now reject all three, and I re-ran the LF and quote cases to confirm nothing regressed:
2. The unchecked
|
Implements the header the VPS agent proposed in their §31, with one refinement. Their host already captures the field and their
ci/smoke.shalready sendsvps/smoke; this is the other half.Why a coincidence was holding this up
97.7% of the traffic reaching production is synthetic, and the only thing separating it from real visitors was accidental. Measured on their side:
curl/8.14.1curl/8.5.0smoke.shcurl/8.5.0The CI runner's curl and the VPS host's curl are byte-identical, so UA cannot separate our gate from their probe — only client address can, and the CI half of that rotates every run. One runner-image bump merges the two and nothing reports it.
The design, and the one refinement
Agreed as proposed: one request header carrying provenance rather than a boolean, so "which run produced this 404" is a single query.
The refinement: the run attempt is part of the id. A re-run of a failed workflow keeps the same
GITHUB_RUN_IDand takes a newGITHUB_RUN_ATTEMPT, so the id alone merges a retry into the run it was retrying — exactly the case someone reads the log to understand. Sogithub/<run-id>-<attempt>rather thangithub/<run-id>.Derived rather than configured, so a hand run is never accidentally untagged:
github/31291656581-2proxmox/manualCHECK_TAGsetproxmox/media-devTwo config files, which is the part not to collapse later
The token is sent only to the origin it belongs to — a redirect that one day points off-site must not mail the credential there. The tag has no such restriction and every request should be attributable.
Folding them into one file would either give the tag that restriction for no reason, or take it away from the token. So both arrays are assembled tag-first with the token appended where allowed, and the off-site hop that deliberately carries no credential still carries the tag.
Verification
Not inferred from the code:
shellcheckandshfmt -dclean. Two tagged requests (proxmox/q-handshake, one 200 and one 404) were sent to production at2026-08-09T03:47:51Zfor the host side to confirm capture from our end.CHECK_TAGis added to the env-docs gate'sKNOBSand described inENVIRONMENT.md— and the gate was watched failing on the undocumented knob before the row was written.What this deliberately does not do
Per their §31.4, and worth keeping in the record: it is forgeable, it gates nothing, and it must never reach auth, rate limiting, robots handling, or caching. Its absence is not proof of a human — a scanner sends no header either — so it pairs with the scanner-shape filters rather than replacing them. It is also not retrospective: the traffic already logged has no such field.