Implement Client Secret Expiry and Renewal for Dynamic Client Registration

[Sanskarzz]

Summary

ToolHive persisted dynamically registered OAuth client credentials, but did not preserve the RFC 7591 and RFC 7592 metadata needed to renew expiring client secrets. Long-running remote-auth workloads could therefore fail once the provider rejected a token refresh that used an expired DCR client secret.

This PR adds the missing DCR client-secret lifecycle:

• Persists client_secret_expires_at, registration_access_token, registration_client_uri, token endpoint auth method, and the callback port used during DCR.
• Gives cached-token restoration sole ownership of the expiry check and sends at most one RFC 7592 PUT per restore attempt, including failure paths.
• Renews expiring secrets through the RFC 7592 client configuration endpoint using the stored registration access token.
• Handles renewal failures softly while the existing secret is still valid, and hard-fails when the secret is already expired.
• Reuses existing secret-manager keys for rotated client secrets and registration access tokens so renewals do not accumulate stale secrets.
• Preserves the shared RemoteAuthConfig pointer after durable state is saved, keeping the runner and auth handler synchronized.
• Builds renewal clients from the shared OAuth HTTP client, restricts redirects and rotated management URIs to the same origin, and blocks private-address dials for public issuers while retaining local/private IdP support.

Fixes #3631

Type of change

• Bug fix
• New feature
• Refactoring (no behavior change)
• Dependency update
• Documentation
• Other (describe):

Test plan

• Unit tests (task test)
• E2E tests (task test-e2e)
• Linting (task lint-fix)
• Manual testing (describe below)

Manual Keycloak validation covered the full renewal flow locally:

• Created a kind cluster and deployed the local ToolHive operator.
• Deployed Keycloak and patched its development hostname to localhost for browser-based OAuth.
• Created a confidential DCR client through Keycloak's OpenID Connect client registration endpoint.
• Assigned the mcp-server-audience default client scope to the DCR client.
• Completed an initial ToolHive remote-auth OAuth login against the authenticated MCP server.
• Seeded cached DCR metadata in the saved run config, including cached_reg_token_ref, cached_reg_client_uri, cached_token_auth_method, and cached_dcr_callback_port.
• Forced cached_secret_expiry into the renewal window and verified startup performed exactly one RFC 7592 renewal before restoring cached OAuth tokens.
• Verified the same ToolHive secret references remained in the run config after renewal.
• Verified Keycloak's client_secret_expires_at: 0 response clears cached_secret_expiry back to the zero time value.

API Compatibility

• This PR does not break the v1beta1 API, OR the api-break-allowed label is applied and the migration guidance is described above.

Changes

Area	Change
DCR discovery	Propagates renewal metadata through successful registration and OAuth flow results.
Remote auth	Centralizes pre-restore renewal, implements RFC 7592 updates, and hardens renewal HTTP and URI handling.
Runner persistence	Persists renewal metadata, reuses secret references, preserves config pointer identity, and mutates memory only after durable save.
Tests	Covers metadata propagation, one renewal per restore attempt, persistence/ref reuse, save failure, URI validation, and HTTP client protections.

Does this introduce a user-facing change?

Yes. Remote-auth workloads that use DCR can now continue operating when an OAuth provider issues expiring client secrets, provided the provider also returns RFC 7592 registration management metadata.

Providers that return client_secret_expires_at: 0 or omit RFC 7592 metadata remain backward compatible: ToolHive treats the secret as non-expiring or logs a warning and falls back to the existing secret until re-authentication is required.

Special notes for reviewers

• Registration access tokens and client secrets are stored through the secret manager. Renewal reuses existing secret names where present to avoid stale secret accumulation.
• registration_client_uri is stored in config because it is endpoint metadata, not a secret.
• The RFC 7592 PUT replays ToolHive's fixed redirect URI, grant type, and response type metadata. Persisting and replaying arbitrary provider-specific registration metadata is a follow-up concern.
• The full diff spans DCR discovery, remote authentication, runner persistence, generated Swagger output, and their tests. These parts form one credential-lifecycle change and cannot be validated independently without duplicating intermediate contracts.

Manual Keycloak test procedure

Manual Testing: DCR Client Secret Expiry and RFC 7592 Renewal

What This Validates

• Seeded DCR renewal metadata is loaded from the saved run config:
  • cached_secret_expiry
  • cached_reg_token_ref
  • cached_reg_client_uri
  • cached_token_auth_method
  • cached_dcr_callback_port
• Cached DCR credentials are checked before token restore.
• A cached secret inside the 24 hour renewal buffer triggers one RFC 7592 PUT.
• Renewed client credentials and Keycloak registration-token rotation are persisted back through the same ToolHive secret references.
• The stored callback port is echoed in the renewal request.
• Renewal failures are soft while the secret is only expiring soon and hard when already expired.

Reference: Keycloak documents its client registration endpoint as /realms/<realm>/clients-registrations/<provider> and the OpenID Connect DCR provider as /realms/<realm>/clients-registrations/openid-connect[/<client id>]: https://www.keycloak.org/securing-apps/client-registration

Prerequisites

• docker or podman
• kind
• kubectl
• helm
• ko
• task
• jq
• a browser on the host machine

Use separate terminals for long-running port-forward and thv run commands.

1. Create a Kind Cluster and Deploy ToolHive Operator

task kind-setup
task operator-install-crds
task operator-deploy-local

Wait for the operator:

kubectl -n toolhive-system get pods --kubeconfig kconfig.yaml

Expected result:

• The ToolHive operator pod is Running.

2. Deploy Keycloak

task keycloak:deploy-dev

Local port 8080 must be free while this task performs its built-in realm setup. The later 8081 port-forward does not replace this requirement because deploy/keycloak/setup-realm.sh currently uses http://localhost:8080.

For this local browser-based OAuth flow, make Keycloak advertise localhost instead of the in-cluster keycloak hostname. Without this, Keycloak redirects the browser to keycloak:<port> and can lose its restart-login cookie.

kubectl -n keycloak patch keycloak keycloak-dev \
  --type merge \
  -p '{"spec":{"hostname":{"hostname":"localhost"}}}' \
  --kubeconfig kconfig.yaml

kubectl -n keycloak wait --for=condition=Ready --timeout=600s keycloaks.k8s.keycloak.org/keycloak-dev --kubeconfig kconfig.yaml

If the hostname patch restarts Keycloak, the development H2 realm data can be reset. Re-run the realm setup once after the patched Keycloak is ready. The setup script currently expects Keycloak on local port 8080, so start a temporary 8080 port-forward in another terminal while this script runs:

kubectl -n keycloak port-forward \
  --address 127.0.0.1 \
  service/keycloak-dev-service \
  8080:8080 \
  --kubeconfig kconfig.yaml

Then run:

deploy/keycloak/setup-realm.sh

Start a persistent Keycloak port-forward in a separate terminal and keep it running.
The example uses local port 8081 because 8080 is often already occupied:

kubectl -n keycloak port-forward \
  --address 127.0.0.1 \
  service/keycloak-dev-service \
  8081:8080 \
  --kubeconfig kconfig.yaml

In another terminal, fetch an admin token. Keycloak admin tokens are short-lived, so re-run this block whenever a later admin API command returns 401 or an empty response:

KC_PORT=8081
KC="http://localhost:${KC_PORT}"
test -n "$KC"
curl -fsS "$KC/realms/master/.well-known/openid-configuration" >/dev/null

ADMIN_USER="$(kubectl get secret keycloak-dev-initial-admin -n keycloak -o jsonpath='{.data.username}' --kubeconfig kconfig.yaml | base64 --decode)"
ADMIN_PASS="$(kubectl get secret keycloak-dev-initial-admin -n keycloak -o jsonpath='{.data.password}' --kubeconfig kconfig.yaml | base64 --decode)"
ADMIN_TOKEN="$(curl -s \
  -d client_id=admin-cli \
  -d username="$ADMIN_USER" \
  -d password="$ADMIN_PASS" \
  -d grant_type=password \
  "$KC/realms/master/protocol/openid-connect/token" | jq -r '.access_token')"

test "$ADMIN_TOKEN" != "null" -a -n "$ADMIN_TOKEN"

Expected result:

• ADMIN_TOKEN is non-empty.
• Use localhost consistently for Keycloak admin, DCR, and ToolHive OAuth calls. Mixing localhost and keycloak in browser flows can lose Keycloak's restart-login cookie.

3. Deploy an Authenticated Remote MCP Server

kubectl apply -f deploy/keycloak/mcpserver-with-auth.yaml --kubeconfig kconfig.yaml
kubectl -n toolhive-system patch mcpoidcconfig keycloak-oidc \
  --type merge \
  -p '{"spec":{"inline":{"issuer":"http://localhost:8081/realms/toolhive"}}}' \
  --kubeconfig kconfig.yaml

until kubectl -n toolhive-system get deployment/fetch-server-keycloak --kubeconfig kconfig.yaml >/dev/null 2>&1; do
  sleep 2
done

kubectl -n toolhive-system wait --for=condition=Available deployment/fetch-server-keycloak --timeout=180s --kubeconfig kconfig.yaml

Start the MCP server port-forward in a separate terminal:

kubectl -n toolhive-system port-forward svc/mcp-fetch-server-keycloak-proxy 9090:9090 --kubeconfig kconfig.yaml

Expected result:

• The authenticated MCP proxy is reachable on http://localhost:9090.
• Both Keycloak and MCP port-forwards stay running while you execute the remaining commands from a third terminal.

4. Create a Confidential DCR Client in Keycloak

ToolHive's normal CLI DCR path registers a public PKCE client. To exercise client-secret renewal with Keycloak, seed ToolHive with a confidential DCR client created through Keycloak's OpenID Connect client registration endpoint.

Create a Keycloak client-registration initial access token:

KC_PORT="${KC_PORT:-8081}"
KC="${KC:-http://localhost:${KC_PORT}}"
test -n "$KC"
curl -fsS "$KC/realms/master/.well-known/openid-configuration" >/dev/null

ADMIN_TOKEN="$(curl -s \
  -d client_id=admin-cli \
  -d username="$ADMIN_USER" \
  -d password="$ADMIN_PASS" \
  -d grant_type=password \
  "$KC/realms/master/protocol/openid-connect/token" | jq -r '.access_token')"

IAT_RESPONSE="$(curl -fsS -X POST \
  "$KC/admin/realms/toolhive/clients-initial-access" \
  -H "Authorization: Bearer $ADMIN_TOKEN" \
  -H "Content-Type: application/json" \
  -d '{"count": 1, "expiration": 3600}')"

INITIAL_ACCESS_TOKEN="$(echo "$IAT_RESPONSE" | jq -r '.token')"
test "$INITIAL_ACCESS_TOKEN" != "null" -a -n "$INITIAL_ACCESS_TOKEN"

Use that initial access token for DCR:

DCR_RESPONSE="$(curl -fsS -X POST \
  "$KC/realms/toolhive/clients-registrations/openid-connect" \
  -H "Authorization: Bearer $INITIAL_ACCESS_TOKEN" \
  -H "Content-Type: application/json" \
  -d '{
    "client_name": "ToolHive MCP Client",
    "redirect_uris": ["http://localhost:8666/callback"],
    "grant_types": ["authorization_code", "refresh_token"],
    "response_types": ["code"],
    "token_endpoint_auth_method": "client_secret_basic"
  }')"

echo "$DCR_RESPONSE" | jq .

CLIENT_ID="$(echo "$DCR_RESPONSE" | jq -r '.client_id')"
CLIENT_SECRET="$(echo "$DCR_RESPONSE" | jq -r '.client_secret')"
REG_TOKEN="$(echo "$DCR_RESPONSE" | jq -r '.registration_access_token')"
REG_URI="$(echo "$DCR_RESPONSE" | jq -r '.registration_client_uri')"

test "$CLIENT_ID" != "null" -a -n "$CLIENT_ID"
test "$CLIENT_SECRET" != "null" -a -n "$CLIENT_SECRET"
test "$REG_TOKEN" != "null" -a -n "$REG_TOKEN"
test "$REG_URI" != "null" -a -n "$REG_URI"

Expected result:

• Keycloak returns client_id, client_secret, registration_access_token, and registration_client_uri.
• registration_client_uri uses http://localhost:8081/.... If it still uses http://keycloak:8081/..., Keycloak is still advertising the old hostname; recheck the hostname patch above before continuing.

If this returns 403, rerun without -f to see Keycloak's error body:

curl -i -sS -X POST \
  "$KC/realms/toolhive/clients-registrations/openid-connect" \
  -H "Authorization: Bearer $INITIAL_ACCESS_TOKEN" \
  -H "Content-Type: application/json" \
  -d '{
    "client_name": "ToolHive MCP Client",
    "redirect_uris": ["http://localhost:8666/callback"],
    "grant_types": ["authorization_code", "refresh_token"],
    "response_types": ["code"],
    "token_endpoint_auth_method": "client_secret_basic"
  }'

5. Give the DCR Client the MCP Server Audience

The sample mcpserver-with-auth.yaml requires the mcp-server audience. Assign the existing mcp-server-audience client scope to the DCR client.

ADMIN_TOKEN="$(curl -s \
  -d client_id=admin-cli \
  -d username="$ADMIN_USER" \
  -d password="$ADMIN_PASS" \
  -d grant_type=password \
  "$KC/realms/master/protocol/openid-connect/token" | jq -r '.access_token')"

CLIENT_UUID="$(curl -s \
  -H "Authorization: Bearer $ADMIN_TOKEN" \
  "$KC/admin/realms/toolhive/clients?clientId=$CLIENT_ID" | jq -r '.[0].id')"

SCOPE_ID="$(curl -s \
  -H "Authorization: Bearer $ADMIN_TOKEN" \
  "$KC/admin/realms/toolhive/client-scopes" | jq -r '.[] | select(.name=="mcp-server-audience") | .id')"

curl -fsS -X PUT \
  "$KC/admin/realms/toolhive/clients/$CLIENT_UUID/default-client-scopes/$SCOPE_ID" \
  -H "Authorization: Bearer $ADMIN_TOKEN"

Expected result:

• The command exits successfully.
• A later 401 Unauthorized from this admin endpoint means ADMIN_TOKEN expired; rerun this step's first command to refresh it.

6. Configure ToolHive Secrets

Use the encrypted provider for writable local secrets.

bin/thv secret provider encrypted

printf '%s' "$CLIENT_SECRET" | bin/thv secret set OAUTH_CLIENT_SECRET_dcr_manual
printf '%s' "$REG_TOKEN" | bin/thv secret set OAUTH_REG_TOKEN_dcr_manual

Expected result:

• Both secrets are stored.

7. Create an Initial OAuth Session

Run a local ToolHive proxy against the authenticated remote MCP server.

Remove any stale workload with the same name before starting a new run:

bin/thv rm dcr-renew-keycloak >/dev/null 2>&1 || true

TOOLHIVE_DEBUG=true bin/thv run http://localhost:9090/mcp \
  --name dcr-renew-keycloak \
  --transport streamable-http \
  --remote-auth \
  --remote-auth-authorize-url "$KC/realms/toolhive/protocol/openid-connect/auth" \
  --remote-auth-token-url "$KC/realms/toolhive/protocol/openid-connect/token" \
  --remote-auth-client-id "$CLIENT_ID" \
  --remote-auth-client-secret "$CLIENT_SECRET" \
  --remote-auth-callback-port 8666 \
  --remote-auth-timeout 2m \
  --remote-auth-scopes openid,profile,email

Complete the browser login:

• Username: toolhive-user
• Password: user123

After authentication succeeds and the workload starts, stop it without deleting the saved run config:

bin/thv stop dcr-renew-keycloak

Expected result:

• The OAuth flow completes.
• A run config exists at:

STATE_FILE="${XDG_STATE_HOME:-$HOME/.local/state}/toolhive/runconfigs/dcr-renew-keycloak.json"
jq '.remote_auth_config' "$STATE_FILE"

8. Seed Cached DCR Renewal Metadata

Force the cached secret into the proactive renewal window by setting the expiry to one hour from now.

Linux:

EXPIRY="$(date -u -d '+1 hour' +%Y-%m-%dT%H:%M:%SZ)"

macOS:

EXPIRY="$(date -u -v+1H +%Y-%m-%dT%H:%M:%SZ)"

Patch the saved run config:

tmp="$(mktemp)"
jq \
  --arg cid "$CLIENT_ID" \
  --arg secret_ref "OAUTH_CLIENT_SECRET_dcr_manual" \
  --arg reg_ref "OAUTH_REG_TOKEN_dcr_manual" \
  --arg reg_uri "$REG_URI" \
  --arg expiry "$EXPIRY" \
  '
    .remote_auth_config.cached_client_id = $cid |
    .remote_auth_config.cached_client_secret_ref = $secret_ref |
    .remote_auth_config.cached_reg_token_ref = $reg_ref |
    .remote_auth_config.cached_reg_client_uri = $reg_uri |
    .remote_auth_config.cached_secret_expiry = $expiry |
    .remote_auth_config.cached_token_auth_method = "client_secret_basic" |
    .remote_auth_config.cached_dcr_callback_port = 8666
  ' "$STATE_FILE" > "$tmp"
mv "$tmp" "$STATE_FILE"

jq '.remote_auth_config | {
  cached_client_id,
  cached_client_secret_ref,
  cached_reg_token_ref,
  cached_reg_client_uri,
  cached_secret_expiry,
  cached_token_auth_method,
  cached_dcr_callback_port
}' "$STATE_FILE"

Expected result:

• The cached DCR fields are populated.
• cached_secret_expiry is about one hour in the future.

9. Trigger Automatic Renewal

Save the old registration token:

OLD_REG_TOKEN="$(bin/thv secret get OAUTH_REG_TOKEN_dcr_manual)"

PROXY_LOG="${XDG_DATA_HOME:-$HOME/.local/share}/toolhive/logs/dcr-renew-keycloak.log"
if [ -f "$PROXY_LOG" ]; then
  LOG_START="$(wc -l < "$PROXY_LOG")"
else
  LOG_START=0
fi

Start the saved workload again so ToolHive reads the patched run config:

TOOLHIVE_DEBUG=true bin/thv start dcr-renew-keycloak

Wait up to one minute for the detached proxy to finish renewal:

attempt=0
while [ "$attempt" -lt 60 ]; do
  if [ -f "$PROXY_LOG" ] && \
     tail -n +"$((LOG_START + 1))" "$PROXY_LOG" | \
       grep -q 'Successfully renewed client secret via RFC 7592'; then
    break
  fi
  sleep 1
  attempt=$((attempt + 1))
done

Expected debug logs:

• Cached client secret is expiring or expired; attempting renewal before token restore
• Attempting RFC 7592 client secret renewal
• Successfully renewed client secret via RFC 7592

Inspect only the proxy-log lines written by this startup. This avoids counting renewal attempts from earlier runs:

CURRENT_RUN_LOG="$(mktemp)"
tail -n +"$((LOG_START + 1))" "$PROXY_LOG" | tee "$CURRENT_RUN_LOG"
grep -q 'Successfully renewed client secret via RFC 7592' "$CURRENT_RUN_LOG"

After startup succeeds, stop it before patching the config again:

bin/thv stop dcr-renew-keycloak

Validate persistence:

NEW_REG_TOKEN="$(bin/thv secret get OAUTH_REG_TOKEN_dcr_manual)"

if [ "$OLD_REG_TOKEN" = "$NEW_REG_TOKEN" ]; then
  echo "Registration token did not rotate. Check Keycloak policy, but renewal may still have succeeded."
else
  echo "Registration token rotated and was persisted."
fi

jq '.remote_auth_config | {
  cached_client_id,
  cached_client_secret_ref,
  cached_reg_token_ref,
  cached_reg_client_uri,
  cached_secret_expiry,
  cached_token_auth_method,
  cached_dcr_callback_port
}' "$STATE_FILE"

Expected result:

• The same secret names are still referenced. There should not be a new OAUTH_CLIENT_SECRET_* or OAUTH_REG_TOKEN_* reference in the run config.
• If Keycloak rotates registration access tokens, NEW_REG_TOKEN differs from OLD_REG_TOKEN.
• cached_secret_expiry is no longer within the forced one-hour renewal window, or is cleared if Keycloak omits client_secret_expires_at.

10. Check for Double Renewal

Count renewal attempts in the isolated step 9 output:

RENEWAL_ATTEMPTS="$(grep -c 'Attempting RFC 7592 client secret renewal' "$CURRENT_RUN_LOG" || true)"
echo "renewal attempts: $RENEWAL_ATTEMPTS"
test "$RENEWAL_ATTEMPTS" -eq 1

Expected result:

• Exactly one Attempting RFC 7592 client secret renewal log appears for a single startup.
• Cached-token restoration is the sole owner of renewal, so a failed or short-lived renewal cannot trigger a second PUT during the same restore attempt.

11. Validate Soft Failure While Expiring Soon

Patch the registration URI to a failing non-root path and keep the expiry in the future:

# Linux
EXPIRY="$(date -u -d '+1 hour' +%Y-%m-%dT%H:%M:%SZ)"

# macOS alternative:
# EXPIRY="$(date -u -v+1H +%Y-%m-%dT%H:%M:%SZ)"

tmp="$(mktemp)"
jq \
  --arg expiry "$EXPIRY" \
  '.remote_auth_config.cached_secret_expiry = $expiry |
   .remote_auth_config.cached_reg_client_uri = "http://localhost:8081/bad-registration-endpoint"' \
  "$STATE_FILE" > "$tmp"
mv "$tmp" "$STATE_FILE"

Start the workload again:

SOFT_LOG_START="$(wc -l < "$PROXY_LOG")"
TOOLHIVE_DEBUG=true bin/thv start dcr-renew-keycloak

attempt=0
while [ "$attempt" -lt 60 ]; do
  if tail -n +"$((SOFT_LOG_START + 1))" "$PROXY_LOG" | \
       grep -q 'Proceeding with expiring client secret after failed renewal attempt'; then
    break
  fi
  sleep 1
  attempt=$((attempt + 1))
done

SOFT_RUN_LOG="$(mktemp)"
tail -n +"$((SOFT_LOG_START + 1))" "$PROXY_LOG" | tee "$SOFT_RUN_LOG"
grep -q 'Client secret renewal failed' "$SOFT_RUN_LOG"
test "$(grep -c 'Attempting RFC 7592 client secret renewal' "$SOFT_RUN_LOG" || true)" -eq 1
bin/thv status dcr-renew-keycloak | grep -q 'Status:.*running'

Expected result:

• Logs include Client secret renewal failed.
• Startup continues to token restore because the secret is only expiring soon.

Stop the workload before restoring the config:

bin/thv stop dcr-renew-keycloak

Restore the valid registration URI:

tmp="$(mktemp)"
jq --arg reg_uri "$REG_URI" '.remote_auth_config.cached_reg_client_uri = $reg_uri' "$STATE_FILE" > "$tmp"
mv "$tmp" "$STATE_FILE"

12. Validate Hard Failure After Expiry

Patch the expiry into the past and use the failing URI again:

# Linux
EXPIRY="$(date -u -d '-1 hour' +%Y-%m-%dT%H:%M:%SZ)"

# macOS alternative:
# EXPIRY="$(date -u -v-1H +%Y-%m-%dT%H:%M:%SZ)"

tmp="$(mktemp)"
jq \
  --arg expiry "$EXPIRY" \
  '.remote_auth_config.cached_secret_expiry = $expiry |
   .remote_auth_config.cached_reg_client_uri = "http://localhost:8081/bad-registration-endpoint"' \
  "$STATE_FILE" > "$tmp"
mv "$tmp" "$STATE_FILE"

Start the workload again:

HARD_LOG_START="$(wc -l < "$PROXY_LOG")"
TOOLHIVE_DEBUG=true bin/thv start dcr-renew-keycloak || true

attempt=0
while [ "$attempt" -lt 60 ]; do
  if tail -n +"$((HARD_LOG_START + 1))" "$PROXY_LOG" | \
       grep -q 'client secret expired at'; then
    break
  fi
  sleep 1
  attempt=$((attempt + 1))
done

HARD_RUN_LOG="$(mktemp)"
tail -n +"$((HARD_LOG_START + 1))" "$PROXY_LOG" | tee "$HARD_RUN_LOG"
grep -q 'client secret expired at' "$HARD_RUN_LOG"
grep -q 'renewal failed' "$HARD_RUN_LOG"
test "$(grep -c 'Attempting RFC 7592 client secret renewal' "$HARD_RUN_LOG" || true)" -eq 1
bin/thv status dcr-renew-keycloak | grep -q 'Status:.*error'

Expected result:

• Startup fails.
• The error contains client secret expired at and renewal failed.

Restore the valid registration URI if you want to keep testing:

tmp="$(mktemp)"
jq --arg reg_uri "$REG_URI" '.remote_auth_config.cached_reg_client_uri = $reg_uri' "$STATE_FILE" > "$tmp"
mv "$tmp" "$STATE_FILE"

13. Cleanup

Stop all port-forwards, then remove the test cluster:

task kind-destroy

Optional local cleanup:

rm -f "${XDG_STATE_HOME:-$HOME/.local/state}/toolhive/runconfigs/dcr-renew-keycloak.json"
bin/thv secret delete OAUTH_CLIENT_SECRET_dcr_manual || true
bin/thv secret delete OAUTH_REG_TOKEN_dcr_manual || true

Notes and Limitations

• The normal ToolHive CLI DCR path currently registers public PKCE clients, so Keycloak may not issue a client_secret or client_secret_expires_at in that path.
• The seeded confidential-client setup above exercises RFC 7592 renewal and renewed-state persistence. Automatic initial DCR metadata propagation is covered by the unit tests because this manual flow seeds that metadata explicitly.
• For a provider that naturally returns client_secret_expires_at during ToolHive-managed DCR, the manual state seeding in step 8 is not needed. Wait until the cached expiry is within 24 hours, then perform the restart in step 9.

Large PR Justification

This is a new feature package with a large test suite, and it needs to land as one coherent phase.

[github-actions]

Large PR Detected

This PR exceeds 1000 lines of changes and requires justification before it can be reviewed.

How to unblock this PR:

Add a section to your PR description with the following format:

## Large PR Justification

[Explain why this PR must be large, such as:]
- Generated code that cannot be split
- Large refactoring that must be atomic
- Multiple related changes that would break if separated
- Migration or data transformation

Alternative:

Consider splitting this PR into smaller, focused changes (< 1000 lines each) for easier review and reduced risk.

See our Contributing Guidelines for more details.

---

This review will be automatically dismissed once you add the justification section.

[codecov]

Codecov Report

❌ Patch coverage is 71.21771% with 78 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.44%. Comparing base (217b0be) to head (a14ff74).

Files with missing lines	Patch %	Lines
pkg/runner/runner.go	43.42%	38 Missing and 5 partials ⚠️
pkg/auth/remote/secret_renewal.go	85.06%	13 Missing and 10 partials ⚠️
pkg/auth/remote/handler.go	52.17%	11 Missing ⚠️
pkg/auth/discovery/discovery.go	93.33%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5377      +/-   ##
==========================================
+ Coverage   71.35%   71.44%   +0.09%     
==========================================
  Files         693      694       +1     
  Lines       70630    70866     +236     
==========================================
+ Hits        50395    50630     +235     
+ Misses      16600    16582      -18     
- Partials     3635     3654      +19     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Large PR justification has been provided. Thank you!

[github-actions]

✅ Large PR justification has been provided. The size review has been dismissed and this PR can now proceed with normal review.

[aponcedeleonch]

This is solid, well-tested work, and it follows a lot of the repo conventions closely (durable-write-before-memory ordering, draining response bodies, generated mocks, backward-compatible zero-expiry handling). Build and the touched-package tests pass for me.

One blocking thing: pkg/state/runconfig.go adds four exported symbols (ReadRunConfigJSON, LoadRunConfigOfType, LoadRunConfigWithFunc, and the RunConfigReadJSONFunc type) that aren't referenced anywhere in the tree, plus the import-alias churn that only exists to support them. It's unrelated to the DCR secret-renewal feature and trips the one-logical-change scope rule. Could you drop it from this PR, or split it into a separate PR where those functions actually get used?

Two non-blocking notes:

• secret_renewal.go around line 513 hardcodes redirect_uris, grant_types, and response_types in the RFC 7592 PUT body rather than echoing what the client was registered with. Since 7592 treats the update as a full replacement, a provider could end up mutating those if the client was registered differently. Might be worth persisting and replaying the original metadata, or at least documenting the assumption.
• After renewal, the runner swaps Config.RemoteAuthConfig for a fresh struct while the handler keeps its original pointer. It works because the secret-manager key names are reused, but a one-line comment noting that coupling would help the next reader.

I didn't run the full Keycloak flow end to end, so my confidence on runtime behavior rests on the unit tests and your documented manual testing. Once the pkg/state code is out, I'm happy with this.

[Sanskarzz]

@aponcedeleonch Thanks for the careful review.
I removed the unrelated pkg/state/runconfig.go helpers and restored the import to its original form, so that file is no longer part of the DCR renewal scope.

For the two notes:

• Added a comment near the RFC 7592 update body documenting that the current renewal request replays ToolHive CLI DCR’s fixed registration metadata, and that future custom DCR metadata should be persisted/replayed if supported.
• Added a runner comment explaining that the auth handler keeps its original RemoteAuthConfig pointer, while renewal relies on reused secret refs and separately refreshes the handler’s in-memory expiry/URI.

[aponcedeleonch]

Follow-up review after the pkg/state/runconfig.go scope fix and the two documentation comments — thanks for addressing those. This pass combines an autonomous code-reviewer agent pass with my own read of the current diff; I verified the main finding empirically with an instrumented test rather than just tracing the code.

Blocking: the double-renewal issue (see inline) is a confirmed regression against the PR description's own "prevented double-renewal" claim — I reproduced it with a temporary test showing 2 RFC 7592 PUT requests for a single restore attempt when renewal fails. The missing test coverage on persistClientCredentials is the other blocker, given it implements the PR's core safety claims (secret-ref reuse, durable-write ordering) with zero direct tests.

Everything else below is a suggestion or nit, not blocking. No secrets/tokens are logged anywhere in the diff, response-body draining is correct, and the durable-write-before-memory-mutation ordering in persistClientCredentials is correct — solid work overall.