Repository navigation
🐛 Stop synthetic 503 for backend calls when kc-agent is suppressed; tolerate rotated JWT in flight - #24496
Conversation
…ight agentFetch short-circuited every request with a fake 503 whenever the local kc-agent was suppressed (in-cluster / console-live), including same-origin /api/mcp/* backend URLs used by the useDeployments and gpu-nodes REST fallbacks. The request never hit the wire, so the console logged '[useDeployments] All fetch sources failed: API error: 503' while every real /api/mcp/deployments response was 200. Only non-same-origin (kc-agent) URLs are short-circuited now. POST /auth/refresh revoked the previous JTI immediately, so data hooks that fired concurrently with the still-old kc_auth cookie were rejected with 401 'Token has been revoked'. Rotated JTIs are now tolerated for a short grace window (RotationGracePeriod) while the revocation is still persisted immediately; explicit logout keeps revoking instantly. Fixes #24495 Signed-off-by: Andrew Anderson <andy@clubanderson.com>
kubestellar-prow
Bot
added
the
dco-signoff: yes
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
👋 Hey @clubanderson — thanks for opening this PR!
This is an automated message. |
kubestellar-prow
Bot
added
the
size/L
✅ Deploy Preview for kubestellarconsole ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
🐝 Hi @clubanderson! I'm Trusted users — org members and contributors with write access — can mention Automation may take a moment to start, and follow-up happens through workflow activity rather than chat replies. |
hivecommons-hive
Bot
added
author/clanker
✅ Test Coverage CheckAll new source files in this PR have corresponding test files. Checked |
There was a problem hiding this comment.
🟡 Changes recommended
Token grace weakens logout guarantees, behaves inconsistently across replicas, and the suppression exemption also admits agent-only requests.
4 open issues
What changed in this PR
Addresses Console Live failures by allowing backend requests during local-agent suppression and adding a grace period for rotated authentication tokens.
Changes:
- Exempts same-origin requests from agent suppression.
- Adds a 15-second token-rotation grace period.
- Adds regression coverage for request routing and token acceptance.
Build and lint are validated by CI on the PR.
| File | Description |
|---|---|
| web/src/hooks/mcp/agentFetch.ts | Allows same-origin requests through suppression. |
| web/src/hooks/mcp/__tests__/agentFetch.suppressed.test.ts | Tests suppressed-agent request routing. |
| pkg/api/middleware/auth_revocation.go | Implements rotation grace tracking. |
| pkg/api/middleware/auth_revocation_test.go | Tests grace, expiry, and cleanup. |
| pkg/api/handlers/auth/handler.go | Applies grace during refresh. |
| pkg/api/handlers/auth/auth_test.go | Tests previous-cookie acceptance during grace. |
Review effort: Balanced
| expiresAt = claims.ExpiresAt.Time | ||
| } | ||
| middleware.RevokeToken(claims.ID, expiresAt) | ||
| middleware.RevokeRotatedToken(claims.ID, expiresAt) |
| c.rotated[jti] = acceptUntil | ||
| c.Unlock() | ||
| c.Revoke(jti, expiresAt) |
| if c.inRotationGrace(jti, time.Now()) { | ||
| return false, nil |
| */ | ||
| export function isSameOriginBackendRequest(input: RequestInfo | URL): boolean { | ||
| const url = requestURLString(input) | ||
| if (!url || url.startsWith('/')) return true |
![hivecommons-hive[bot]](https://avatars.githubusercontent.com/in/3568013?s=60&v=4){kind=small}
There was a problem hiding this comment.
Read the tree at head 213a787, not just the diff. Both fixes check out; this looks correct to me (comment, not approval).
Synthetic 503 (web/src/hooks/mcp/agentFetch.ts): Verified the fall-through path. When suppressed and the target is same-origin, control reaches getAgentToken(), which itself checks isLocalAgentSuppressed() and resolves '' immediately (agentFetch.ts:147-151 at head), so backend requests go out with no agent Authorization header and no kc-agent round-trip — matching the new test's assertion. isSameOriginBackendRequest correctly handles string/URL/Request inputs (a Request with a relative path normalizes to the page origin, so it compares equal). The useDeployments → fetchWithRetry('/api/mcp/...') chain described in the body is real; this unblocks it.
Rotation grace (pkg/api/middleware/auth_revocation.go): The design is sound: RevokeAfterRotation writes the revocation through to the store immediately and only defers local acceptance; acceptUntil is capped at token expiry; logout (RevokeToken) is untouched; cleanup() and inRotationGrace both prune expired entries so the map is bounded. The double-checked delete in inRotationGrace (re-reading under the write lock before deleting) avoids racing a concurrent re-add.
Two non-blocking notes:
- Multi-replica caveat (open question, not a defect):
rotatedis per-process while the revocation is persisted immediately, so in a multi-replica deployment an in-flight request landing on a different replica than the one that served/auth/refreshwill still 401 via the store-backed check inIsRevokedChecked. Fine for the single-replica console, but worth a one-line comment onRotationGracePeriodif HA is ever on the roadmap. - Security note (info): a superseded JTI now remains accepted for up to 15s after refresh. Scope is narrow — rotation only, logout revokes instantly, window capped at expiry — and this mirrors standard refresh-rotation grace practice. Stating it for the record.
Tests cover the grace window, expiry cap, post-grace rejection, logout, ValidateJWT acceptance, and cleanup pruning — adequate for the change.
— hive: agent=reviewer backend=copilot model=claude-fable-5 copilot=1.0.88
|
hive/awaiting-ci: build-gate is green at head 213a787, but the hive merge request exhausted while the gate was pending and the guard now pins this head ("push a new commit or merge it manually"). This PR is human-authored so the hive will not push a refresh commit. A maintainer push or manual merge unblocks it; tide will also merge it once lgtm/approve land. 🐝 Hive Agent: — hive: agent=scanner backend=copilot model=claude-fable-5 copilot=1.0.88 |
hivecommons-hive
Bot
added
stage/060-reviewed
|
Ready for a human merge: verdict approved @213a787, 44/44 checks green, DCO yes, soak 30m0s elapsed. Blocked only by The hive never approves on your behalf: |
|
hive/awaiting-ci: all checks green on head 213a787 (build-gate SUCCESS), but hive-merge refuses: an earlier merge request exhausted on this head at 2026-10-20T16:54:36Z while build-gate was still pending, and the exhaustion guard now pins it ("push a new commit or merge it manually"). Since this PR is human-authored, the hive will not push a bump commit — a maintainer merge (or any new push by the author) unblocks it. 🐝 Hive Agent: — hive: agent=scanner backend=copilot model=claude-fable-5 copilot=1.0.88 |
hivecommons-hive
Bot
added
stage/060-reviewed
|
hive/awaiting-ci: earlier merge request exhausted on head 213a787 while build-gate was pending (guard pins that head even though the gate later passed). Branch updated to a fresh head; sweep will merge when build-gate is green on the new head. 🐝 Hive Agent: — hive: agent=scanner backend=copilot model=claude-fable-5 copilot=1.0.88 |
hivecommons-hive
Bot
added
stage/050-review-pending
|
stage 060-reviewed → 050-review-pending: contributor-lane:no-claim |
There was a problem hiding this comment.
Read the diff at head ec37017 plus the callers before the PR body. Both fixes check out against the tree.
Root cause 1 verified. getClusterModeBaseUrl() returns /api/mcp in cluster mode (web/src/lib/cache/fetcherUtils.ts:56-58), so the REST fallback in workloadQueries/deployments.ts:456-458 goes through fetchWithRetry → agentFetch (fetchWithRetry.ts:61), where the old code fabricated a 503 for every URL under suppression. The new isSameOriginBackendRequest (agentFetch.ts:108-117) passes relative /api/... through and still short-circuits LOCAL_AGENT_HTTP_URL (different origin). On the passthrough path getAgentToken() returns '' immediately when suppressed (agentFetch.ts:147-151), so no agent token leaks into backend requests and nothing hangs.
Root cause 2 verified. RevokeRotatedToken writes the revocation through immediately and only defers local acceptance (auth_revocation.go, RevokeAfterRotation); acceptUntil is capped at token expiry; logout still uses RevokeToken and is unaffected; cleanup() prunes the rotated map.
One narrow edge, low severity: inRotationGrace is checked before the revocation list in IsRevokedChecked, and RevokeToken does not clear a rotated entry. So a JTI superseded by refresh stays accepted for the remainder of the 15 s window even if something explicitly revokes it in that window (e.g. logout immediately after a refresh leaves the pre-refresh cookie valid for up to ~15 s). Exposure is bounded and arguably inherent to any grace window; if you want logout to be airtight, have RevokeToken also delete(c.rotated, jti).
Looks correct to me overall (not approving — that's a human's signature).
— hive: agent=reviewer backend=copilot model=claude-fable-5 copilot=1.0.88
hivecommons-hive
Bot
added
stage/060-reviewed
|
Ready for a human merge: verdict approved @ec37017, 37/37 checks green, DCO yes, soak 30m0s elapsed. Blocked only by The hive never approves on your behalf: |
|
Thank you for your contribution! Your PR has been merged. Check out what's new:
Stay connected: Slack #kubestellar-dev | Multi-Cluster Survey |
|
Merged via hive sweep (squash 2d775f7). 🐝 Hive Agent: — hive: agent=scanner backend=copilot model=claude-fable-5 copilot=1.0.88 |
|
Post-merge build verification passed ✅ Both Go and frontend builds compiled successfully against merge commit |
✅ Post-Merge Verification: passedCommit: |
) When the kc-agent is suppressed (in-cluster/console-live), LOCAL_AGENT_HTTP_URL is empty and settingsFetch('/settings') degrades to a relative URL; since #24496 agentFetch passes relative URLs through to the app origin, so the debounced settings PUT produced 405s that failed the live canary gate (run 39524410941). Treat suppression like the existing Netlify case: no agent requests, sync status 'offline'. Fixes #24513 Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com> Co-authored-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…s, DaemonSets and HPAs (#24529) The live /deployments page fetches /api/mcp/replicasets and /api/mcp/hpas in addition to deployments; after #24496 those requests reach the backend and fail 503 on every CI cluster because the groundtruth reader ClusterRole only grants apps/deployments. Seen in Console Live Promote run 39601160527. StatefulSets and DaemonSets are included because the same workload hooks (web/src/hooks/mcp/workloadQueries/infrastructure.ts) fetch them with the identical pattern, avoiding another one-resource-per-run failure cycle. Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com> Co-authored-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fixes #24495
Follow-up to #24493 for the red Console Live Promote run 39452389795. Both remaining failures are app-side; related to #24242 / #24490.
Root cause 1 — synthetic
API error: 503(the failing assertion)web/src/hooks/mcp/agentFetch.tsreturned a fabricated503 local_agent_suppressedResponse for every URL whenisLocalAgentSuppressed()was true (in-cluster / console-live).useDeployments()without a cluster arg (useClusterData,useUniversalStats,useSearchIndex, …) skips the cluster-gated branches and reaches the REST fallback indeployments.ts:457→fetchWithRetry('/api/mcp/deployments?…')→agentFetch→ fake 503, no network request. That is why the Playwright trace has only 200s for/api/mcp/deploymentsand the error fires with no matching request.compute.ts:228(gpu-nodes REST fallback) had the same latent bug.Fix:
agentFetchnow only short-circuits requests whose target is not the same origin (i.e. real kc-agent URLs). Same-origin/api/...calls go through to the Go backend.Root cause 2 —
401 {"error":"Token has been revoked"}Trace correlation (same
kc_authcookie):POST /auth/refresh200 at15:55:00.655→/api/mcp/clusters?,/api/mcp/events401 at.666; again at15:55:15.406→/api/mcp/pods?cluster=…×3 and/api/mcp/clusters401 at.419–.427.restoreCookieSession()POSTs/auth/refreshon every page load andRefreshTokenrevoked the old JTI instantly, so hooks that fired concurrently with the previous cookie were rejected. The#6026stale-bearer fallback only covered the Authorization-header case. This is deterministic app logic, not a revoked PAT/App token —/api/mekeeps returning 200 with the rotated cookie.Fix:
RevokeRotatedTokenkeeps the superseded JTI accepted forRotationGracePeriod(15s) while still persisting the revocation immediately. Logout (RevokeToken) is unchanged and still revokes instantly.Tests
web/src/hooks/mcp/__tests__/agentFetch.suppressed.test.ts— realagentFetch(bypassing the global passthrough mock) with suppression on: backend path is fetched and returns the real 200 body; kc-agent URL still gets the synthetic 503 without a network call. Fails 5/7 without the fix.pkg/api/middleware/auth_revocation_test.go— grace window, cap at token expiry, post-grace revocation, logout has no grace,ValidateJWTaccepts rotated token during grace, cleanup prunes grace entries.pkg/api/handlers/auth/auth_test.go::TestRefreshToken_FullRefreshCycleupdated: in-flight request with the previous cookie succeeds, then token is revoked once grace elapses.npm run build,npm run lint:check(baseline gate),go test ./pkg/api/middleware ./pkg/api/handlers/auth ./pkg/storepass locally.