feat: Get tokens for Grafana Assistant via OAuth#1118
feat: Get tokens for Grafana Assistant via OAuth#1118rafa-munoz wants to merge 1 commit intomainfrom
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Abort during authorization returns error instead of aborted
- The abort handler now rejects with
signal.reason, preserving theAbortErrorshape so the state machine correctly returns an aborted result.
- The abort handler now rejects with
- ✅ Fixed: HTTP server not closed on callback error paths
- All callback rejection paths now remove the abort listener and close the HTTP server before rejecting, preventing leaked listeners and occupied ports.
Or push these changes by commenting:
@cursor push e54149f6bb
Preview (e54149f6bb)
diff --git a/src/services/grafana-assistant/callback-server.ts b/src/services/grafana-assistant/callback-server.ts
--- a/src/services/grafana-assistant/callback-server.ts
+++ b/src/services/grafana-assistant/callback-server.ts
@@ -21,7 +21,7 @@
signal: AbortSignal
): Promise<{ port: number; result: Promise<CallbackParams> }> {
let resolveCallback!: (params: CallbackParams) => void
- let rejectCallback!: (error: Error) => void
+ let rejectCallback!: (error: unknown) => void
const result = new Promise<CallbackParams>((resolve, reject) => {
resolveCallback = resolve
@@ -44,6 +44,10 @@
if (!code || !state || !endpoint) {
res.writeHead(400)
res.end('Missing code, state, or endpoint')
+ signal.removeEventListener('abort', handleAbort)
+ if (server.listening) {
+ server.close()
+ }
rejectCallback(new Error('Missing code, state, or endpoint in callback'))
return
}
@@ -51,6 +55,10 @@
if (state !== expectedState) {
res.writeHead(400)
res.end('Invalid state')
+ signal.removeEventListener('abort', handleAbort)
+ if (server.listening) {
+ server.close()
+ }
rejectCallback(new Error('State mismatch: possible CSRF attack'))
return
}
@@ -60,13 +68,19 @@
'<html><body><h1>Authentication successful</h1><p>You can close this tab and return to k6 Studio.</p></body></html>'
)
- server.close()
+ signal.removeEventListener('abort', handleAbort)
+ if (server.listening) {
+ server.close()
+ }
resolveCallback({ code, state, endpoint })
})
const handleAbort = () => {
- server.close()
- rejectCallback(new Error('Authentication aborted'))
+ signal.removeEventListener('abort', handleAbort)
+ if (server.listening) {
+ server.close()
+ }
+ rejectCallback(signal.reason)
}
signal.addEventListener('abort', handleAbort, { once: true })| const handleAbort = () => { | ||
| server.close() | ||
| rejectCallback(new Error('Authentication aborted')) | ||
| } |
There was a problem hiding this comment.
Abort during authorization returns error instead of aborted
Medium Severity
The handleAbort callback rejects with new Error('Authentication aborted'), which has .name === 'Error'. The wasAborted function checks for .name === 'AbortError', so this error is never recognized as an abort. When the user aborts during the authorization phase, the result will be { type: 'error', message: 'Authentication aborted' } instead of { type: 'aborted' }. The existing pattern in src/handlers/utils.ts correctly uses reject(signal.reason) to propagate the default AbortError from the signal.
Additional Locations (1)
| res.end('Invalid state') | ||
| rejectCallback(new Error('State mismatch: possible CSRF attack')) | ||
| return | ||
| } |
There was a problem hiding this comment.
HTTP server not closed on callback error paths
Medium Severity
When rejectCallback is called on error paths (missing parameters or state mismatch), server.close() is never called. The server continues listening on the port. Only the success path and the abort handler close the server. This leaks the HTTP server and occupies a port from the limited range (54321–54399), potentially exhausting available ports on retries.
rafa-munoz
force-pushed
the
grafana-assistant-token-retrieval
branch
from
252a414 to
d95d8a7
Compare
1 participant
Ticket
https://github.com/grafana/k6-cloud/issues/4152
Description
This is a PoC that reuses the OAuth flow from Assistant CLI as it is explained in the Proposal 2 of the Design Doc.
k6-studio-assistant-integration.mov
Things that still need to be done:
How to Test
Consolein "Developer Tools".const result = await window.studio.grafanaAssistant.connect('https://mystack.grafana.net')JSON.stringify(result)cat ~/Library/Application\ Support/k6\ Studio/k6-studio-grafana-assistant-dev.json(for OSX).Checklist
Related PR(s)/Issue(s)
Note
Cursor Bugbot is generating a summary for commit 252a414. Configure here.