From e34f546b8c9fde7ae0bc6cc10f0504fa7ac9d5f9 Mon Sep 17 00:00:00 2001 From: Why_So_Serious <91526214+kimnamwook1@users.noreply.github.com> Date: Mon, 27 Jul 2026 02:11:38 +0900 Subject: [PATCH] fix(github): restore the recovery path for revoked user OAuth tokens (#10995) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(github): honour the force flag in checkRefreshToken `checkRefreshToken` accepts a `force` parameter but never reads it, so the only caller that passes `force = true` (worker.ts, when re-syncing a user) behaves exactly like the non-forced call and skips the refresh entirely. Gate the refresh on `force || expired` instead of on expiry alone. As a side effect a record with a `refreshToken` but a null `expiresIn` is now validated when forced, instead of being reported as valid unchecked. Signed-off-by: koreanjoker * fix(github): return undefined from getOctokit when the token is dead When `checkRefreshToken` reports failure, `getOctokit` cleared `record.octokit` and then fell straight through to constructing a new Octokit from the very token that was just rejected. The method therefore never returned `undefined` for a revoked user, so the `(await getOctokit(...)) ?? container.container.octokit` installation-token fallback that the sync code already writes at 13 call sites was unreachable. Return `undefined` after clearing the client so the existing fallback can take effect. Signed-off-by: koreanjoker * fix(github): preserve accounts when deserialising a user secret `updateUser` serialises the whole `GithubUserRecord` — `accounts` included — into the integration secret, but `secretToUserRecord` placed a literal `accounts: {}` after the spread of the parsed payload, discarding whatever was stored. Every consumer of a record loaded through `getAccount` therefore saw an empty map. `revokeUserAuth` iterates `Object.entries(record.accounts)`, so its body never ran and the re-authorisation notice was never written to any workspace. Read `accounts` back from the parsed payload, keeping `{}` as the fallback for records written before the field existed. Signed-off-by: koreanjoker --------- Signed-off-by: koreanjoker --- services/github/pod-github/src/platform.ts | 3 ++- services/github/pod-github/src/users.ts | 5 +++-- services/github/pod-github/src/worker.ts | 1 + 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/services/github/pod-github/src/platform.ts b/services/github/pod-github/src/platform.ts index edf1929209..97455c5eb0 100644 --- a/services/github/pod-github/src/platform.ts +++ b/services/github/pod-github/src/platform.ts @@ -738,7 +738,8 @@ export class PlatformWorker { } async checkRefreshToken (ctx: MeasureContext, auth: GithubUserRecord, force: boolean = false): Promise { - if (auth.refreshToken != null && auth.expiresIn != null && auth.expiresIn < Date.now() / 1000) { + const expired = auth.expiresIn != null && auth.expiresIn < Date.now() / 1000 + if (auth.refreshToken != null && (force || expired)) { const uri = 'https://github.com/login/oauth/access_token?' + makeQuery({ diff --git a/services/github/pod-github/src/users.ts b/services/github/pod-github/src/users.ts index 3771cd52be..ccb12372cd 100644 --- a/services/github/pod-github/src/users.ts +++ b/services/github/pod-github/src/users.ts @@ -36,11 +36,12 @@ export class UserManager { } private secretToUserRecord (secret: IntegrationSecret, login: string): GithubUserRecord | undefined { + const parsed = JSON.parse(secret.secret) ?? {} // TODO: Add security return { - ...(JSON.parse(secret.secret) ?? {}), // TODO: Add security + ...parsed, account: secret.socialId, _id: login, - accounts: {} + accounts: parsed.accounts ?? {} } } diff --git a/services/github/pod-github/src/worker.ts b/services/github/pod-github/src/worker.ts index 244de40828..530f0d35b0 100644 --- a/services/github/pod-github/src/worker.ts +++ b/services/github/pod-github/src/worker.ts @@ -643,6 +643,7 @@ export class GithubWorker implements IntegrationManager { ctx.info('get octokit', { account, recordId: record._id, workspace: this.workspace.uuid }) if (!(await this.platform.checkRefreshToken(ctx, record))) { record.octokit = undefined + return undefined } if (record.octokit !== undefined) { return record.octokit