feat: report the agentic wallet in doctor - #92
Merged
Conversation
doctor covered the organization wallet only. The agentic wallet is a separate object with its own local config and its own credential, so an agent running on one had no way to learn whether it was configured, still accepted, or funded. The check reads GET /api/agentic-wallet/credit, signing with the HMAC secret already in ~/.keeperhub/wallet.json rather than the user's bearer token. Signing as the wallet is the security property, not an implementation detail: the signature proves possession of one specific wallet's secret, so the check can only ever report the wallet held on this machine, and no endpoint has to answer "describe wallet X" for an account that merely has platform access. That is also why this does not add a bearer-authenticated lookup route, which would make wallets enumerable. Reported states: not configured, configured with its address and remaining credit, rejected (which usually means the secret was rotated), and unreachable. The secret is never printed, logged, or included in an error. Two tests hold that line, one over the rendered error and one over the command's stdout, which is the surface an agent captures. internal/agentic carries the config reader and the signer. Its signature is pinned against a vector produced by the platform's own computeSignature, so a divergence in the signing string fails here rather than as a 401 in the field, and a second test asserts every signed field actually changes the output.
joelorzet
requested review from
a team,
OleksandrUA,
eskp and
suisuss
and removed request for
a team
August 7, 2026 15:50
The message named POST /api/integrations/wallet, which does not exist. The real creation route is POST /api/user/wallet, but wallet provisioning is session-only, so no endpoint is worth naming to a caller holding an API key. Settings is the only path that actually works.
The credit amount is a decimal string, not a number: the platform builds it with toFixed(2). Decoding it into a float64 failed on every real response, so the check reported "could not parse credit response" for a wallet that was working. The fake server had been written to emit a JSON number, which is why the tests passed against a shape the platform never sends. It now sends what the platform sends, and two tests fail if the float decode returns. The signed path now comes from the request URL instead of a package constant. A host carrying a path prefix signed one path and requested another, and the resulting 401 was reported as a rotated secret. Signing moved into internal/agentic alongside Sign, so the request and the signature cannot drift and a second caller does not have to copy the header sequence. A 404 is now distinguished from an unspecified failure. It is the documented answer when the platform holds no secrets for the sub-org, which happens after rotation past the grace window or when pointed at the wrong host, and the message says so. Absent is now pass rather than warn. Most installs never provision an agentic wallet, and a permanent warning on nearly every run devalues the level for checks that mean something. This matches how the spend-cap check treats billing not being enabled. A config that exists but lacks a required field is now its own state, so someone who just ran `kh wallet add` is not told to run it again. A 200 missing the expected fields no longer renders as a funded wallet with zero credit, which read as "out of funds" rather than "that was not the credit endpoint". The tests isolated XDG_CONFIG_HOME but not HOME, so on any machine with a provisioned wallet the suite read the developer's real config and signed a live request with their secret. Every test now gets its own HOME. TestConfig_SecretIsNotInTheJSONRoundTrip asserted nothing about the secret and its name described a property that was false. Config now has a String method that redacts the secret, which is a guarantee worth having on a credential, and the tests assert it in both directions. Dropped the unused HeaderKeyVersion. Not sending it is deliberate, so the reason is recorded where the constants are.
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.
Stacked on #91. Base that PR first, or retarget this to
mainafter it merges.kh doctorcovered the organization wallet only. The agentic wallet is a separate object with its own local config and its own credential, so an agent running on one had no way to learn whether it was configured, still accepted, or funded.Why HMAC and not a bearer token
The check signs with the HMAC secret already in
~/.keeperhub/wallet.json. That is the security property, not an implementation detail: the signature proves possession of one specific wallet's secret, so the check can only ever report the wallet held on this machine.The alternative was a bearer-authenticated lookup route such as
GET /api/agentic-wallet/me. That would make wallets enumerable by any account with platform access, so it was rejected. No platform change was needed;GET /api/agentic-wallet/creditalready exists and already verifies the signature.Reported states
not configured (kh wallet add)rejected by the platform (secret may have been rotated)could not check agentic wallet (HTTP n)The secret
Never printed, logged, or included in an error. Two tests hold that: one over the rendered error from a malformed config, one over the command's stdout, which is the surface an agent captures.
Verification
The signing string is pinned against a vector produced by the platform's own
computeSignature, so a divergence fails here rather than as a 401 in the field:A second test asserts every signed field (method, path, subOrgId, body, timestamp, secret) actually changes the output, so nothing silently drops out of the signature.
At the doctor level the fake server recomputes the signature the way the platform does and rejects a mismatch, so a client signing the wrong string fails the test rather than passing on the presence of some headers. It also enforces the five minute replay window.
Full suite: 29 packages ok.
go vetclean.go generate ./docs/committed alongside the help text change.