fix(test-page): per-user test API keys and stale key prevention#910
Open
AnoshanJ wants to merge 4 commits into
Open
fix(test-page): per-user test API keys and stale key prevention#910AnoshanJ wants to merge 4 commits into
AnoshanJ wants to merge 4 commits into
Conversation
Each call to IssueTestAPIKey now derives the key name from a truncated
SHA-256 of the caller's JWT sub claim (console-test-<12 hex chars>).
This gives every user their own DB row per agent, so concurrent sessions
across users no longer rotate each other's keys.
- models: APIKeyTestKeyName → APIKeyTestKeyPrefix ("console-test-")
- services: add testKeyName(userSub) helper; update IssueTestAPIKey
signature and body; tighten CreateAPIKey guard to strings.HasPrefix
- controllers: extract claims.Sub in IssueTestAPIKey handler and pass
it to the service; return 401 when claims are absent
Signed-off-by: Anoshan Jayahanthan <101160077+AnoshanJ@users.noreply.github.com>
When a user switches tabs and comes back, the test key is re-fetched immediately instead of waiting for the 9-minute refetchInterval. This catches cases where another session rotated the key while the tab was in the background, avoiding a 401 on the next send. Signed-off-by: Anoshan Jayahanthan <101160077+AnoshanJ@users.noreply.github.com>
…rendering Signed-off-by: Anoshan Jayahanthan <101160077+AnoshanJ@users.noreply.github.com>
Contributor
📝 WalkthroughWalkthroughThe PR implements per-user test API key isolation by deriving unique key names from JWT subject hashes, replacing a single global test key name. The service layer includes a new hashing helper, interface updates, and validation to reserve prefix-based names. The controller extracts JWT claims and passes the user subject. Frontend improvements enable cache refresh on window focus and proper component lifecycle management. ChangesPer-User Test API Key Isolation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
agent-manager-service/controllers/agent_apikey_controller.go (1)
262-266: 💤 Low valueStale docstring: no longer uses a single fixed key name.
The comment still references "the single short-lived test API key" and "scoped to the fixed name 'console-test'", but the implementation now issues per-user keys derived from the JWT subject.
📝 Suggested update
// IssueTestAPIKey handles POST /api/v1/orgs/{orgName}/projects/{projName}/agents/{agentName}/environments/{envID}/api-keys/test // -// Issues (or rotates) the single short-lived test API key for the agent. -// Used by the console Try-It flow. The key is test-scoped, scoped to the -// fixed name "console-test", and never appears in the user-facing list. +// Issues (or rotates) a short-lived test API key for the calling user. +// Used by the console Try-It flow. Each user gets their own key (derived +// from their JWT subject), so concurrent sessions don't invalidate each +// other. Test keys never appear in the user-facing list.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@agent-manager-service/controllers/agent_apikey_controller.go` around lines 262 - 266, The docstring for IssueTestAPIKey is stale: it still says "single short-lived test API key" and "scoped to the fixed name 'console-test'"; update the comment in agent_apikey_controller.go for the IssueTestAPIKey handler to state that it issues short-lived, per-user test API keys derived from the JWT subject (rather than a single fixed key name), and adjust any wording about scoping/visibility to reflect the current behavior (per-user console test keys and how they appear in listings).console/workspaces/libs/api-client/src/hooks/agent-api-keys.ts (1)
102-105: ⚡ Quick winUpdate comment to reflect actual timing: 9 minutes, not 10.
The comment mentions "10-minute test API key" but the code uses 9 minutes for both
staleTimeandrefetchInterval(lines 123-124).📝 Suggested documentation fix
-// useTestAgentAPIKey issues (or rotates) a 10-minute test API key for the +// useTestAgentAPIKey issues (or rotates) a 9-minute test API key for the // agent's Try-It flow. Each refetch rotates the same logical row on the🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@console/workspaces/libs/api-client/src/hooks/agent-api-keys.ts` around lines 102 - 105, The top comment above the useTestAgentAPIKey hook incorrectly states "10-minute test API key" while the actual configuration uses 9 minutes; update that comment to say "9-minute test API key" (or otherwise match the 9-minute timing) so it reflects the values used for staleTime and refetchInterval in the hook; locate the comment block above the useTestAgentAPIKey function and adjust the wording to match the staleTime/refetchInterval constants used in the hook.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@console/workspaces/libs/api-client/src/hooks/agent-api-keys.ts`:
- Line 125: The query option refetchOnWindowFocus is set to true but the PR
description specifies "always"; update the useQuery call (the hook in
agent-api-keys.ts, e.g., the useAgentApiKeys/query options where
refetchOnWindowFocus is set) to use refetchOnWindowFocus: "always" so the query
refetches on every window focus regardless of staleness; if the intended
behavior was actually to only refetch when stale, instead update the PR
description to state `true` and explain the 9-minute staleness behavior.
---
Nitpick comments:
In `@agent-manager-service/controllers/agent_apikey_controller.go`:
- Around line 262-266: The docstring for IssueTestAPIKey is stale: it still says
"single short-lived test API key" and "scoped to the fixed name 'console-test'";
update the comment in agent_apikey_controller.go for the IssueTestAPIKey handler
to state that it issues short-lived, per-user test API keys derived from the JWT
subject (rather than a single fixed key name), and adjust any wording about
scoping/visibility to reflect the current behavior (per-user console test keys
and how they appear in listings).
In `@console/workspaces/libs/api-client/src/hooks/agent-api-keys.ts`:
- Around line 102-105: The top comment above the useTestAgentAPIKey hook
incorrectly states "10-minute test API key" while the actual configuration uses
9 minutes; update that comment to say "9-minute test API key" (or otherwise
match the 9-minute timing) so it reflects the values used for staleTime and
refetchInterval in the hook; locate the comment block above the
useTestAgentAPIKey function and adjust the wording to match the
staleTime/refetchInterval constants used in the hook.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ec5628fe-0dc5-4627-b553-125aa8670bc1
📒 Files selected for processing (5)
agent-manager-service/controllers/agent_apikey_controller.goagent-manager-service/models/apikey.goagent-manager-service/services/agent_apikey_service.goconsole/workspaces/libs/api-client/src/hooks/agent-api-keys.tsconsole/workspaces/pages/test/src/Test.Component.tsx
Signed-off-by: Anoshan Jayahanthan <101160077+AnoshanJ@users.noreply.github.com>
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.
Problem
Three intermittent issues were observed in the agent Try-It (test) page:
Multi-user 401s — The backend stored a single
console-testkey row per agent per environment. When two users (or two browser tabs) opened the same agent's test page, eachIssueTestAPIKeycall rotated that shared row, invalidating the other session's cached key and causing 401s.Stale key on tab refocus — When a user switched away, opening two tabs for the same agent try out view and came back, the test key was getting rotated, leaving a window where a rotated key could cause a 401.
Message history bleed on agent switch — Navigating from Agent A to Agent B in the sidebar reused the same
AgentChatcomponent instance (React reconciliation), so the previous agent's messages, errors, and endpoint state leaked into the new agent's view.Summary by CodeRabbit
New Features
Improvements