Diagnose Withings refresh failures and accept documented token response #4

Merged
jlxq0 merged 4 commits from fix/refresh-numeric-userid-and-status-diagnostic into main 2026-10-06 01:22:36 +00:00
Owner

Source-only fix for Withings refresh and sanitized failure diagnosis. Official authorization responses use numeric userid and include scope; documented refresh responses may omit both. Preserve stored identity/scope on refresh, fail closed on incomplete consent, verify full persisted record before using rotated token, and expose only stage and numeric status in error diagnostics. Remove userid from startup logs. Bump rustls lockfile 0.23.43 → 0.23.45 to clear RUSTSEC-2026-0285.

Offline checks at 905b96acb3: fmt, clippy -D warnings, 88 tests, cargo audit and cargo deny all passed on pinned Rust 1.98.0; independent rerun also passed. Synthetic regression sabotage went red. No production token/measurement read, release, or live OAuth probe. The old withings_api_error proves only an unmapped nonzero Withings envelope status; exact live status is unknown. Clark owns tagged release and live verification after review.

Source-only fix for Withings refresh and sanitized failure diagnosis. Official authorization responses use numeric userid and include scope; documented refresh responses may omit both. Preserve stored identity/scope on refresh, fail closed on incomplete consent, verify full persisted record before using rotated token, and expose only stage and numeric status in error diagnostics. Remove userid from startup logs. Bump rustls lockfile 0.23.43 → 0.23.45 to clear RUSTSEC-2026-0285. Offline checks at 905b96acb3b3c61b55ff08788679425e88fada03: fmt, clippy -D warnings, 88 tests, cargo audit and cargo deny all passed on pinned Rust 1.98.0; independent rerun also passed. Synthetic regression sabotage went red. No production token/measurement read, release, or live OAuth probe. The old `withings_api_error` proves only an unmapped nonzero Withings envelope status; exact live status is unknown. Clark owns tagged release and live verification after review.
Withings' documented refresh_token response carries only access_token,
refresh_token and expires_in, and its authorization_code response carries
userid as a JSON number. TokenResponse required a string userid, so a
successful refresh in the documented shape would fail as
withings_invalid_response with the rotated token unpersisted. Every fixture
used a string userid, so the suite never saw it.

- userid (string or number) and scope are optional in TokenResponse.
- A refresh carries userid/scope forward from the stored record; a value
  the response does carry wins.
- The persist read-back compares the whole record, so a store that keeps the
  token but loses the identity is refused before the access token is used.
- A consent fails closed without userid and scope, in exchange_code and
  again in adopt, before the store is touched.
- map_withings_error puts stage (refresh|measure) and withings_status /
  http_status into error.data and emits one warn with only those fields and
  the code string. The refresh info line no longer logs the userid.

Tests use invented values and check that no token, secret, response body or
userid reaches the wire or the log.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
cargo audit reds on RUSTSEC-2026-0285 (TLS 1.3 handshake messages accepted
across encryption level boundaries), fixed in rustls >= 0.23.45. Lockfile
only, via `cargo update -p rustls --precise 0.23.45`: one package's version
and checksum change, its dependency list does not, and nothing else moves.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Stop logging the stored userid at startup, and pin it
All checks were successful
CI / cargo (pull_request) Successful in 2m9s
CI / docker (pull_request) Successful in 54s
905b96acb3
build_app logged the stored record's userid on every start, so every
release's log carried the account's Withings id. It now says only that
tokens are held.

startup_with_stored_tokens_does_not_log_the_userid drives build_app with a
stored record and checks the captured log. no_log_line_carries_a_userid
scans every source file for a `userid` tracing field or a %/? capture of
one, so a log line on a path no test drives is caught too. Restoring the
old line reds both.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Report token-store failures by step, never by the store's text
All checks were successful
CI / cargo (pull_request) Successful in 36s
CI / docker (pull_request) Successful in 29s
167a6ea8fb
persist_before_use logged `%error` and put the store's error text into
the InvalidInput a client sees, and load_or_unauthorized did the same on
both reads before a refresh. A store's error is free text: it can name a
path, or quote the file it failed to parse, which holds the credential.

Every store failure on the refresh path now goes through one function
and becomes WithingsError::TokenStore { step }, with step one of load,
persist, read_back or mismatch. The client sees code token_store_error,
class internal, stage refresh and store_step. The log carries the step
and a bounded cause (io with its ErrorKind, parse, withings, other). The
OAuth callback's adopt failure logs the same bounded cause instead of
`%error`.

Fail-closed ordering is unchanged: persist and read back before the new
access token is returned.

a_token_store_failure_names_its_step_and_nothing_private drives the real
transport through six breakages (first load, re-read under the gate,
save, read-back, and read-back and first load returning an unparsable
file) with a store whose errors carry an invented sentinel, an invented
path and the rotated token. It asserts none of them reaches the envelope
or the captured log, and that measure is never called. Putting `%error`
back into the log, or the old load_or_unauthorized message back into the
envelope, reds it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
jlxq0 merged commit a427c0290a into main 2026-10-06 01:22:36 +00:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
jlxq0/withings-mcp!4
No description provided.