Skip to content

Correct three log-review filters that produce plausible wrong answers - #63

Merged
ptr727 merged 3 commits into
developfrom
correct-log-review-prose
Aug 8, 2026
Merged

Correct three log-review filters that produce plausible wrong answers#63
ptr727 merged 3 commits into
developfrom
correct-log-review-prose

Conversation

@ptr727

Copy link
Copy Markdown
Owner

Three filters in OPERATIONS.md "Reading a 404 list" produce plausible wrong answers. The host side found two of them and diagnosed the third; all three are verified here against the same day's log before being written down.

The referer rule as committed does not work

It said to compare the referer against scheme://RequestHost + RequestPath. Read literally that matches nothing, because a scanner reaching an HTTPS site routinely sends an http:// referer for the same address.

FormSurvivors on 2026-08-08 site-host 404s
naive, request's own scheme36, every one a false positive
scheme normalized0

The implementation that produced the original "zero broken links" happened to test both schemes explicitly, so the number was right and the prose anyone would rebuild from was wrong. That is the worse half: a wrong command in a procedure outlives the session that got the right answer despite it.

RequestHost is null on some requests

Two records that day carry no Host header at all, both router exploits. A bare .RequestHost | startswith(...) then fails with startswith() requires string inputs and takes the run with it, part way through a file that has already printed real output. Loud but partial is the worst shape a failure can take, because the output above the error looks complete.

Defaulted with // "".

ServiceName says the edge answered; entryPointName and RequestScheme say why

The two routerless 404s recorded in the previous change were cleartext HTTP to port 443. The websecure router carries tls and matches TLS requests only, so a plaintext request to 443 matches nothing and traefik answers its own 404. Verified here: both records are websecure + http on one ClientPort with a 19-byte body, and every websecure + http request in the log is routerless.

Correct behavior, needing no action — not the proxy reconfiguration window this repo guessed at, which is now removed rather than left as a plausible-sounding cause. The shape that does deserve investigation is a routerless 404 arriving over https, meaning a hostname the proxy serves no route for.

Verification

The documented filter is now a runnable command rather than a description, and it was run:

  • site-host survivors: 0, as the section claims
  • the naive form it replaces: 36
  • no jq abort on the null-host records

markdownlint, editorconfig-checker and the hub's prose_lint.py are all clean, the last at its exact baseline for this file.

The host side diagnosed the routerless pair and corrected the referer
filter. All three verified here against the same day's log before being
written down.
The referer rule as committed does not work. It said to compare against
`scheme://RequestHost + RequestPath`, and read literally that matches
nothing, because a scanner reaching an HTTPS site routinely sends an
`http://` referer for the same address. On this data the naive form
keeps all 36 false positives where the normalised form keeps none. The
implementation used to produce the original count happened to test both
schemes explicitly, so the answer was right and the prose anyone would
rebuild from was wrong.
RequestHost is null on a request that sends no Host header, which router
exploits do, so a bare startswith aborts the run with a type error part
way through a file that has already printed real output. Loud but
partial is the worst shape. Defaulted with `// ""`.
ServiceName says the edge answered; entryPointName with RequestScheme
says why. The two routerless 404s on the site host were cleartext HTTP
to port 443, where the websecure router requires TLS and traefik answers
its own 404. Correct behavior, not the reconfiguration window this repo
guessed at. The shape that does deserve investigation is a routerless
404 arriving over https, which means a hostname with no route.
The documented filter is now a runnable command rather than a
description, and it returns zero on the site host as claimed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CopilotAI lite review requested due to automatic review settings August 8, 2026 18:55

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the operational log-review procedure in OPERATIONS.md to prevent misleading results when filtering Traefik 404s, based on verified behaviors observed in real traffic.

Changes:

  • Clarifies interpretation of routerless 404s by correlating ServiceName with entryPointName and RequestScheme.
  • Replaces an ambiguous referer-check description with a concrete jq command that normalizes schemes and handles null RequestHost.
  • Documents the failure mode of null RequestHost and why // "" is needed to avoid mid-stream jq aborts.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment threadOPERATIONS.md Outdated
Comment threadOPERATIONS.md Outdated
Two review findings, both correct.
The comment justified `// ""` by saying a bare startswith aborts
mid-file, and the command below it used concatenation rather than
startswith. Worse, the guard was doing nothing there: jq treats
null + "/path" as "/path", so only the string functions reject a null
host. Verified both ways before changing it.
The example now includes the host filter, which is where startswith
appears and where the guard earns its place, and the prose says plainly
that the concatenations do not need it. String functions reject null
where arithmetic tolerates it, which is the actual rule.
The filter also selected >= 400 in a section about reading a 404 list.
It selects == 404 now, with a line saying to widen it for the whole
non-200 sweep the table above describes.
Re-verified after the change: zero survivors on the site host, and no
abort on the two records that carry no Host header.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CopilotAI review requested due to automatic review settings August 8, 2026 19:13

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (2)

OPERATIONS.md:282

  • The comment above the jq example says // "" is needed because startswith aborts on a null host, but the example host filter uses an equality check (no startswith). This makes the rationale confusing (and repeats the earlier mismatch you already fixed).
# Narrow to this site's own 404s, then keep only referers pointing somewhere else.
# `// ""` is on the host filter because startswith aborts on a null host; the concatenation
# below needs no guard, since jq treats null + "/path" as "/path".

OPERATIONS.md:267

  • The sentence “ServiceName says that the edge answered” reads as the opposite of the rule stated just above (service name present => routed/site answered; field absent => routerless/edge answered). This is likely to mislead readers trying to interpret routerless 404s.
**`ServiceName` says that the edge answered, and `entryPointName` with `RequestScheme` says why**, which is the difference between a finding and a fault. The common cause is a cleartext request to the TLS port: the `websecure` router carries `tls` and therefore matches TLS requests only, so a plaintext request to 443 matches nothing and Traefik answers its own 404 with a body of a few dozen bytes. That is correct behavior rather than a gap, and it needs no action. Read the two fields together before treating a routerless 404 as a routing problem, because the shape that does deserve investigation is a routerless 404 arriving over **https**, which means a hostname the proxy serves no route for.

…above
Two suppressed findings, both correct, and the first is the same
mismatch class as the one already fixed on this branch. The comment
justified `// ""` on a host filter that uses equality, and equality
tolerates a null exactly as concatenation does, so the guard was inert
in both places it appeared. Protection that is not there is worse than
none, because it stops a reader looking.
The example now carries no guard and says why, and the paragraph below
states the rule across all three operators rather than two: string
functions reject a null host, equality and concatenation tolerate it.
The second sentence opened "ServiceName says that the edge answered",
which reads as the opposite of the rule stated immediately above it,
where a present service name means the site answered. Rewritten to lead
with the absence.
Re-verified: the example returns zero on the site host and does not
abort on the two records carrying no Host header.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CopilotAI review requested due to automatic review settings August 8, 2026 19:21
@ptr727

Copy link
Copy Markdown
OwnerAuthor

Both suppressed findings were real and both are fixed in 50bb469.


OPERATIONS.md:282 — The comment above the jq example says // "" is needed because startswith aborts on a null host, but the example host filter uses an equality check (no startswith). This makes the rationale confusing (and repeats the earlier mismatch you already fixed).

Correct, and the parenthesis is the fair part — this is the same mismatch class I had just claimed to fix. Worse on inspection: equality tolerates a null exactly as concatenation does, so // "" was inert in both places it appeared in that example. A guard that guards nothing is worse than no guard, because it stops the next reader from checking.

The example now carries no guard and a comment saying why, with the note that swapping the equality for startswith makes one mandatory. The paragraph below states the rule across all three operators rather than two: string functions reject a null host, equality and concatenation tolerate it.


OPERATIONS.md:267 — The sentence "ServiceName says that the edge answered" reads as the opposite of the rule stated just above (service name present => routed/site answered; field absent => routerless/edge answered).

Correct. The sentence leads with the field name where the rule is about its absence, so it inverts the paragraph immediately above it. That is exactly the sentence someone reads while trying to interpret a routerless 404, which is the worst place for it. Rewritten to lead with the absence.


Re-verified after the change: the documented example returns zero survivors on the site host, and does not abort on the two records that carry no Host header.

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

@ptr727
ptr727 merged commit 1ee7534 into developAug 8, 2026
5 checks passed
@ptr727
ptr727 deleted the correct-log-review-prose branch August 9, 2026 00:09
Sign up for freeto 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.

2 participants

@ptr727