Problem
app.signOut's (and now, after #502, onAuthLost's) authenticated-session
teardown fires several best-effort authenticated requests fire-and-forget:
workbench.destroy()'s KILL QUERY, exportService.cancelExport()/
cancelExportScript()'s own KILL QUERY. These go through
net/ch-client.ts's authedFetch:
export async function authedFetch(ctx: ChCtx, url: string, sql: string, signal?: AbortSignal): Promise<Response> {
const token = await ctx.getToken();
if (!token) { ctx.onSignedOut(); throw new Error('not signed in'); }
let bearer = token;
let attempt = 0;
const authHeader = ctx.authHeader || ((t: string) => 'Bearer ' + t);
for (;;) {
const resp = await ctx.fetch(url, { method: 'POST', body: sql, headers: { Authorization: authHeader(bearer) }, signal });
...
authHeader(bearer) is called only after await ctx.getToken() resolves —
i.e. in a microtask that resumes after the caller's own synchronous code has
finished. Both app.signOut and onAuthLost clear credentials
(clearTokens(), which unconditionally resets authMode to 'oauth') in the
same synchronous tick as firing this teardown, so by the time authHeader()
actually runs, authMode has already been reset.
For an HTTP-Basic-mode session (connectBasic), connection-session.ts's
authHeader:
function authHeader(t: string): string {
if (authMode === 'basic') return 'Basic ' + t;
if (chAuthVal !== 'basic') return 'Bearer ' + t;
...
}
sees the reset authMode === 'oauth' instead of 'basic', so it takes the
wrong branch and builds Authorization: Bearer <base64(user:pass)> — the
captured Basic credential sent under the wrong scheme — instead of
Authorization: Basic <base64(user:pass)>. ClickHouse almost certainly
rejects that, so the teardown's KILL QUERY/export-cancel requests silently
fail for Basic-mode users specifically, even though the token value itself
was captured correctly (this part of getToken()'s synchronous null-check
runs before the reset, and is unaffected).
Impact
A Basic-auth user whose session ends (explicit sign-out or involuntary auth
loss) may leave an in-flight query, export, or stream running server-side
past sign-out/login — the exact failure mode #502 addressed, but only for
this one path, and only for Basic-mode sessions.
Scope note
Pre-existing in app.signOut's teardown before #502; #502 made onAuthLost
consistent with it, not worse. Not fixed as part of #502 because it needs a
real design decision (e.g. capturing the Authorization header eagerly,
before the fire-and-forget call, rather than resolving it lazily inside
authedFetch), not a one-line reorder.
Suggested fix
Capture ctx.authHeader(token) synchronously (before firing the
fire-and-forget teardown call, or before clearing credentials) rather than
resolving it lazily inside authedFetch's retry loop — or accept a
pre-built Authorization header as an argument for teardown-only callers.
Problem
app.signOut's (and now, after #502,onAuthLost's) authenticated-sessionteardown fires several best-effort authenticated requests fire-and-forget:
workbench.destroy()'sKILL QUERY,exportService.cancelExport()/cancelExportScript()'s ownKILL QUERY. These go throughnet/ch-client.ts'sauthedFetch:authHeader(bearer)is called only afterawait ctx.getToken()resolves —i.e. in a microtask that resumes after the caller's own synchronous code has
finished. Both
app.signOutandonAuthLostclear credentials(
clearTokens(), which unconditionally resetsauthModeto'oauth') in thesame synchronous tick as firing this teardown, so by the time
authHeader()actually runs,
authModehas already been reset.For an HTTP-Basic-mode session (
connectBasic),connection-session.ts'sauthHeader:sees the reset
authMode === 'oauth'instead of'basic', so it takes thewrong branch and builds
Authorization: Bearer <base64(user:pass)>— thecaptured Basic credential sent under the wrong scheme — instead of
Authorization: Basic <base64(user:pass)>. ClickHouse almost certainlyrejects that, so the teardown's
KILL QUERY/export-cancel requests silentlyfail for Basic-mode users specifically, even though the token value itself
was captured correctly (this part of
getToken()'s synchronous null-checkruns before the reset, and is unaffected).
Impact
A Basic-auth user whose session ends (explicit sign-out or involuntary auth
loss) may leave an in-flight query, export, or stream running server-side
past sign-out/login — the exact failure mode #502 addressed, but only for
this one path, and only for Basic-mode sessions.
Scope note
Pre-existing in
app.signOut's teardown before #502; #502 madeonAuthLostconsistent with it, not worse. Not fixed as part of #502 because it needs a
real design decision (e.g. capturing the Authorization header eagerly,
before the fire-and-forget call, rather than resolving it lazily inside
authedFetch), not a one-line reorder.Suggested fix
Capture
ctx.authHeader(token)synchronously (before firing thefire-and-forget teardown call, or before clearing credentials) rather than
resolving it lazily inside
authedFetch's retry loop — or accept apre-built
Authorizationheader as an argument for teardown-only callers.