Make a refused presence claim diagnosable from our logs, not from the reply - #288
Merged
Merged
Conversation
… reply The website hand-built a test presence claim using their identity cookie's internal field names -- `subject` and `solvedAt` -- rather than the agreed `human` / `human_iat` / `human_sub`. They got `no_human` twice and were one step from concluding this gate was rejecting their valid tokens. Their mistake, but our coarseness made it hard to see. `no_human` is returned whether the claim is absent, malformed, or from the future, and that is deliberate: this side should not be usable to probe what a valid claim looks like. The cost is that an integrator cannot tell "you did not send it" from "you sent it wrong", and two identical wrong answers read as a finding rather than one mistake made twice. So the distinction now lives in the log, where it can be looked up, and nowhere else. `human_presence_detail` says which of the four it was; the response is unchanged. Both halves are asserted, because either alone is the bug: a coarse log is undiagnosable and a detailed response is a probe. The test checks the detail appears in the log and does *not* appear in the response body. Writing that test also caught the test helper: it always included the claim keys, so setting one to None tested "present but null" rather than absent -- which is exactly the case the website hit, since their claim had neither agreed key at all. It can omit keys now. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ause Three findings in the fix for "an integrator cannot tell why they were refused", all of them the same failure one level in. `human_presence_detail` ended with the clock message as a catch-all. Add a `no_human` branch to `human_presence_reason` -- they are separate functions with nothing linking them -- and the detail would have reported a clock disagreement with total confidence, sending an integrator after the wrong cause. Which is precisely what the detail exists to prevent. It now says it is out of step, which is the honest answer when it does not know. It used wall-clock `time()` while the reason takes `now` as a parameter, so the two disagreed about the present whenever a caller supplied a clock. The test caught that within a minute of being written, which is the most useful minute of this change. Both take the same `now` now. And the first version of the drift guard was vacuous against the regression it existed to stop. It enumerates the known causes and asserts each has a detail -- which passes just as well with a catch-all, because then every cause returns something plausible and the "out of step" string never appears. Restoring the catch-all did not fail it. The guard that works calls the detail on claims that are *not* a failure at all. Those reach the end of the function, so they are the one input that distinguishes an honest fallback from a confident one. Restoring the catch-all fails it. Also bounded the repr of a rejected `human` value in the log line. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
Prompted by the website session's integration, where the defect was theirs and the difficulty of finding it was ours.
What happened
They hand-built a test presence claim using their identity cookie's internal field names —
subjectandsolvedAt— rather than the agreedhuman/human_iat/human_sub. They gotno_humantwice and were one step from concluding this gate was rejecting their valid tokens.Their mistake. But our coarseness made it hard to see:
no_humanis returned whether the claim is absent, malformed, or from the future, and that is deliberate — this side should not be usable to probe what a valid claim looks like.The cost is that an integrator cannot tell "you did not send it" from "you sent it wrong". Two identical wrong answers read as a finding rather than one mistake made twice, which is the same trap as the
goneboundary.The fix
The distinction now lives in the log and nowhere else.
human_presence_detailreports which of the four cases it was; the response is unchanged.Both halves are asserted, because either alone is the bug — a coarse log is undiagnosable and a detailed response is a probe. The test checks the detail appears in the log and does not appear in the response body.
Two things the adversarial pass caught
The test helper always included the claim keys, so setting one to
Nonetested "present but null" rather than absent — which is exactly the case the website hit, since their claim had neither agreed key at all. It can omit keys now.My first sabotage silently did not apply (ruff had reformatted the target line), so the run that followed proved nothing. Re-done reading the actual text: all four cases fail with the detail removed, all four pass with it restored.
🤖 Generated with Claude Code