fix(pairing): make the listed pending request approvable - #74427
Merged
Conversation
Contributor
૮ >ﻌ< ა ci reviewran on 37d0766 ℹ️ InfoDesktop E2E visual evidence · View test artifacts · View job1 visual diff. inline evidence upload failed. Failed to upload diff-665a0833239e-onboarding-overlay-diff.png with gh image (exit code 1): Error uploading /home/runner/work/_temp/e2e-evidence/diff-665a0833239e-onboarding-overlay-diff.png: step 0 (get upload token): uploadToken not found on repo page — do you have write access to NousResearch/hermes-agent? (or, if NousResearch enforces SAML SSO, authorize at https://github.com/orgs/NousResearch/sso) |
approve_code()'s success path never cleared _failures:{platform}. The
counter is incremented on every non-matching code, persisted in
_rate_limits.json, and only ever reset to 0 when it reaches
MAX_FAILED_ATTEMPTS (firing the lockout). So it counts failures over the
gateway's entire lifetime, not consecutive ones.
An owner who mistypes a pairing code on a handful of separate occasions
— each time immediately retyping it correctly and successfully pairing —
accumulates those isolated typos. A later single fresh typo then hits
MAX_FAILED_ATTEMPTS and locks the whole platform out for an hour, at
which point _is_locked_out gates approve_code and even the *correct*
code is rejected.
Reset the counter on a successful approval, matching standard
brute-force-guard semantics (the counter tracks consecutive failures).
This does not weaken protection: an attacker cannot produce a success
without a valid code, and 5 consecutive wrong attempts still lock out.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Follow-up hardening on the request-id grant path. approve_request took the same lockout treatment as approve_code: gated by it, and recording a miss toward it. But the two paths defend different things. The lockout exists to stop guessing at the 8-char code space over a messaging channel; a request id is only ever obtained by an admin already authenticated to the store, so a miss means the row they clicked went stale. Counting those let a handful of clicks on a stale list lock the operator out of `hermes pairing approve` for an hour — the GUI DoSing the CLI. Also drops the `code`/`code_hash_prefix` compat fields from list_pending. The hash prefix is what admin surfaces mistook for an approvable code in the first place, and re-exporting the request id under the old `code` key just preserves the ambiguity; both consumers in the tree read `request_id` now. The 16-hex sniffing that had been copy-pasted into the CLI and the endpoint (where a chained conditional consulted it against the wrong field) moves to one owner, PairingStore.looks_like_request_id. The endpoint no longer reports a 429 on the request-id path, where lockout can't apply — a stale id surfaced as a bogus "locked out" while the platform sat locked for something else entirely.
OutThisLife
force-pushed
the
bb/pairing-approve-entry
branch
from
July 29, 2026 22:53
9686738 to
37d0766
Compare
Collaborator
This was referenced Jul 29, 2026
randlee
pushed a commit
to randlee/hermes-agent
that referenced
this pull request
Aug 11, 2026
…prove-entry fix(pairing): make the listed pending request approvable
33hodl
pushed a commit
to 33hodl/hermes-agent
that referenced
this pull request
Aug 12, 2026
…prove-entry fix(pairing): make the listed pending request approvable
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
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.
Approving a pending pairing request from an admin surface could not work.
list_pendingreturns the first 8 hex of the code's salted SHA-256 as a field namedcode— deliberately, so reading the store never reveals the real code — and the dashboard posted that value straight back to/api/pairing/approve, which hashes its input and compares. It never matched. Each click also counted toward the brute-force lockout, so five of them locked the platform for an hour and rejected the correct code at the CLI too.Verified against the store on
mainbefore touching anything:The code hashing landed in
2e509422e; the dashboard pairing page arrived nine days later in #36704 against the already-hashed shape. The only coverage asserted a 404 on a garbage code — which is also what a correct code returns now, so the suite stayed green.Pending entries now expose a
request_id(the server-side entry key) andPairingStore.approve_requestgrants on it. That fits how the two surfaces actually authenticate: the DM code proves the channel belongs to the requester, which is whyapprove_codekeeps its proof-of-possession model untouched; a request id is only ever obtained by an admin already authenticated to the store, so it identifies a row rather than proving a secret. Consequently a request-id miss is a stale row, not an attack, and does not touch the lockout counter — otherwise the GUI DoSes the CLI.hermes pairing approveaccepts either form and dispatches on shape.Consolidation
Three open PRs target this defect. Both code-bearing ones are cherry-picked, so authorship survives in git history rather than being reimplemented:
_finish_approvalso both grant paths get itOn top of the salvaged base: dropped the
code/code_hash_prefixcompat fields (the prefix is what surfaces mistook for a credential; re-exporting the id under the oldcodekey kept the ambiguity alive), gave the duplicated 16-hex sniffing one owner inlooks_like_request_id, and fixed the endpoint's chained conditional, which tested that flag against the wrong field and reported a bogus 429 on the request-id path.Verification
hermes pairing list→ copy Request ID →hermes pairing approve telegram <id>→ user approved, disappears from pending. Endpoint exercised throughTestClientagainst a tempHERMES_HOME.Regressions added for the two properties that made this silent: the listing exposes nothing derived from the code and neither grant path accepts the digest prefix; repeated approvals of a stale request id past the lockout threshold leave the code path usable.
60 passedacrosstests/gateway/test_pairing.py,tests/hermes_cli/test_pairing.py,tests/hermes_cli/test_dashboard_admin_endpoints.py;111 passedacross the gateway pairing/authz/multiplex suites;tsc --noEmitclean inweb.Closes #46580
Supersedes #46584
Supersedes #62145
Supersedes #66761