fix(auth,config,db): guard nil pointers causing server panics - #4146
Draft
cursor[bot] wants to merge 3 commits into
Draft
fix(auth,config,db): guard nil pointers causing server panics#4146cursor[bot] wants to merge 3 commits into
cursor[bot] wants to merge 3 commits into
Conversation
When an OIDC provider returns no id_token and the UserInfo endpoint fails, userInfo stays nil but Profile was read unconditionally, panicking the server during sign-in. Co-authored-by: Denis Gukov <fiftin@outlook.com>
SqlDbConnection.Close checked d.sql.Db but not d.sql itself. Migration rollback used ignoreErrors=false, panicking when undo SQL files are missing. Co-authored-by: Denis Gukov <fiftin@outlook.com>
IsUseRemoteRunner, GetRunnerRegistrationToken, and admin info dereferenced conf.Runners without a nil check when runners config is omitted. Co-authored-by: Denis Gukov <fiftin@outlook.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.
Summary
Rebases and consolidates carryover critical panic fixes onto current
develop. Supersedes conflicting PR #4096 and includes the migration rollback fix from PR #4103.Bugs fixed
1. OIDC UserInfo nil dereference (
api/login.go)Impact: Server panic during OIDC login when the provider returns no
id_tokenand the UserInfo endpoint fails.Root cause:
userInfo.Profilewas read outside theerr == nilblock.Fix: Extract
oidcClaimsFromUserInfothat returns early on UserInfo error.2. Nil
Runnersconfig panic (util/config.go,api/admin_info.go)Impact: Server panic on admin info or runner registration when
runnersconfig section is omitted.Root cause:
conf.Runnersaccessed without nil guard.Fix: Nil-safe helpers
IsUseRemoteRunner,GetRunnerRegistrationToken,DefaultGlobalRunnersMode.3. SqlDb Close panic (
db/sql/SqlDb.go)Impact: Panic when closing a DB connection that was never opened.
Root cause: Checked
d.sql.Dbbut notd.sqlitself.Fix: Guard
d.sql == nilbefore accessingd.sql.Db.4. Migration rollback panic (
db/sql/migration.go)Impact: Panic when rolling back migrations lacking
.err.sqlundo files (~98 migrations).Root cause:
getVersionSQL(..., false)panics on missing files (regression in0bd3589a).Fix: Restore
ignoreErrors=truefor rollback path.Validation
go test ./util/... ./db/sql/...— passgo test ./services/tasks/... ./db/...— passRecent commit scan
No new commits on
developsince 2026-08-10 (d1238fad). Re-verified multi-select survey vars (PR #4118/#4124) and PocketID OIDC redirect (PR #4112) — no additional critical issues found.