OAuth refresh: enforce client binding only when both sides present a client_id (Spec: ACE-033)

[ashwin-agami]

Summary

Fast-follow to #88 (ACE-033), addressing a Copilot review finding before v0.3.8 cuts.

An authorize can complete without a client_id, so a refresh token's stored client_id may be blank. The bind check in _grant_refresh_token (if client_id and client_id != stored) then spuriously rejected a refresh that did send a client_id (non-empty ≠ blank → invalid_grant), breaking renewal for such a client. (claude.ai registers a client via DCR and uses it consistently, so it isn't affected — but it's a real robustness gap for the general case.)

Fix

Make the binding check symmetric — enforce the match only when both the presented and the stored client_id are non-empty; a blank on either side skips it (the token secret + hash-at-rest remain the real gate). This matches the already-chosen lenient-binding posture (RFC 6749 §6 doesn't require client_id for a public client).

Chose this over Copilot's alternative of tightening authorize() to require a non-empty client_id, which would change the auth-code flow and could break a client that legitimately authorizes without one.

Test

test_refresh_with_client_id_when_the_code_had_none — authorize with a blank client_id, exchange for a token pair, then refresh with a client_id and assert it succeeds. Full gate green (1315 passed).

Spec: ACE-033. Rolls into v0.3.8 with the rest of the refresh-token work.

[gitguardian]

️✅ There are no secrets present in this pull request anymore.

If these secrets were true positive and are still valid, we highly recommend you to revoke them.
While these secrets were previously flagged, we no longer have a reference to the
specific commits where they were detected. Once a secret has been leaked into a git
repository, you should consider it compromised, even if it was deleted immediately.
Find here more information about risks.

---

^{_{🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.}}

[Copilot]

Pull request overview

Adjusts OAuth refresh-token client binding to avoid incorrectly rejecting refresh requests when either the presented or stored client_id is blank, aligning with the intended lenient public-client behavior for ACE-033.

Changes:

• Make refresh-token client_id binding enforcement symmetric: only enforce a match when both presented and stored client_id are non-empty.
• Add a regression test covering the “auth code issued with blank client_id, refresh sent with non-blank client_id” scenario.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
packages/agami-core/src/oauth_server.py	Updates refresh-token grant logic to only enforce client binding when both sides have a non-empty client_id.
tests/test_oauth_server.py	Adds a targeted test to ensure refresh succeeds when the original authorization had a blank client_id but refresh includes one.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[ashwin-agami]

Already addressed in the latest commit (343a76b) — the test now gets its token pair via the _token_pair helper, which does assert r.status_code == 200, r.text before .json(), and the renewal goes through _refresh which likewise asserts 200. There's no longer a bare .json() on an unchecked response. (The rewrite was to blank the stored client_id directly in the DB rather than via a second authorize.)