Repository navigation
fix(oauth): clear GitLab Duo setup message instead of 500 (#3861) - #3868
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Code Review
This pull request improves the error handling for GitLab Duo OAuth setup when the client ID is unconfigured. Instead of throwing an error and returning a generic 500 Internal Server Error, the system now returns a clear, actionable setup message. The changes include returning null from buildAuthUrl when clientId is missing, handling this case in the OAuth route, and adding a regression unit test. The reviewer suggested dynamically constructing the redirect URI in the error message using the request's origin instead of hardcoding http://localhost:20128/callback to support custom deployment setups.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if (provider === "gitlab-duo" && !authData.authUrl) { | ||
| return NextResponse.json({ | ||
| ...authData, | ||
| supported: false, | ||
| error: | ||
| "GitLab Duo OAuth is not configured. Register an OAuth application at " + | ||
| "https://gitlab.com/-/profile/applications with redirect URI " + | ||
| "http://localhost:20128/callback and scopes \"ai_features read_user\", then set " + | ||
| "GITLAB_DUO_OAUTH_CLIENT_ID (and optionally GITLAB_DUO_OAUTH_CLIENT_SECRET) and restart.", | ||
| }); |
There was a problem hiding this comment.
Hardcoding the redirect URI to http://localhost:20128/callback in the error message can be misleading for operators running the application on a custom port, domain, or in a shared/VPS environment (as noted in the repository guide).
It is highly recommended to dynamically construct the redirect URI using the request's origin so that the instructions match the operator's actual deployment setup. Please ensure that the variable name request matches the parameter name of your GET route handler.
| if (provider === "gitlab-duo" && !authData.authUrl) { | |
| return NextResponse.json({ | |
| ...authData, | |
| supported: false, | |
| error: | |
| "GitLab Duo OAuth is not configured. Register an OAuth application at " + | |
| "https://gitlab.com/-/profile/applications with redirect URI " + | |
| "http://localhost:20128/callback and scopes \"ai_features read_user\", then set " + | |
| "GITLAB_DUO_OAUTH_CLIENT_ID (and optionally GITLAB_DUO_OAUTH_CLIENT_SECRET) and restart.", | |
| }); | |
| if (provider === "gitlab-duo" && !authData.authUrl) { | |
| const origin = typeof request !== "undefined" ? new URL(request.url).origin : "http://localhost:20128"; | |
| return NextResponse.json({ | |
| ...authData, | |
| supported: false, | |
| error: | |
| "GitLab Duo OAuth is not configured. Register an OAuth application at " + | |
| "https://gitlab.com/-/profile/applications with redirect URI " + | |
| origin + "/callback and scopes \"ai_features read_user\", then set " + | |
| "GITLAB_DUO_OAUTH_CLIENT_ID (and optionally GITLAB_DUO_OAUTH_CLIENT_SECRET) and restart.", | |
| }); | |
| } |
* chore(release): continue v3.8.25 development cycle after main code-sync (r5) main fast-forwarded to release/v3.8.25 (#3863): unblocked Build+Docker via #3864, plus #3837 (mimocode proxy) and #3862 (trivy bump). This marker re-opens the umbrella PR for further v3.8.25 work. No version bump. * fix(db): persist the Keep-latest-backups retention setting (#3834) (#3867) * fix(oauth): clear GitLab Duo setup message instead of 500 (#3861) (#3868) * test(oauth): prove refresh_token preserved on real gemini-cli/antigravity dispatch (#3850) (#3869) * feat(compression-ui): unified compression config UI — per-engine pages + combos editor + menu + WS default-on (#3860) Integrated into release/v3.8.25 — feat(compression-ui): unified compression configuration UI (Compression Hub + per-engine Lite/Aggressive/Ultra pages + combos editor + sidebar entry + live-WS default-on). File-size re-baselined for sidebarVisibility.ts/chatCore.ts growth; orphan ws test relocated to a collected path. * docs(changelog): complete the v3.8.25 release notes + credit all contributors Audited every commit since v3.8.24 and filled the gaps the [3.8.25] section was missing: a New Features section (compression engines + Compression Studios #3848, compression UI #3860, injection-guard #3857, kiro discovery #3836, Veo #3839, mimocode proxy #3837, Arena ELO flag #3821), 9 more Fixed entries (#3811/#3807/#3759/#3849/#3838/#3835/#3814/#3820/#3819), a Security section (CCR IDOR #3859, supply-chain #3824), and an Internal/Quality section. Every contributor and issue reporter is now credited. * docs(changelog): restore + complete the v3.8.25 release notes Re-adds CHANGELOG.md (a prior server-side commit accidentally dropped it) with the complete, audited [3.8.25] section: New Features, the full Fixed list, Security & Hardening, and Internal/Quality — every contributor and issue reporter credited. * chore(release): finalize v3.8.25 — reconcile CHANGELOG + i18n mirrors, document OMNIROUTE_MAX_PENDING_MIGRATIONS, green the unit suite Release-gate reconciliation for v3.8.25: - CHANGELOG: dated 2026-06-14, linked #3826, rolled up file-size re-baselines (#3823/#3833), recorded the test-greening; re-synced all 41 i18n CHANGELOG mirrors. - Documented OMNIROUTE_MAX_PENDING_MIGRATIONS (#3416) in .env.example + ENVIRONMENT.md. - Greened the unit suite (was merged red on 4 CI shards): aligned 10 stale tests to this cycle's intended behavior (#3838/#3822/#3501/SOCKS5/Vertex-Express/Antigravity) and the same-provider 503 fall-through test; de-flaked the compression benchmark reproducibility and ServiceSupervisor crash tests. No production code changed. * ci(security): clear OpenSSF Scorecard code-scanning noise + harden workflow token permissions The Security tab held 155 open alerts, ALL from the advisory OpenSSF Scorecard tool (#3824) — supply-chain/posture scores, not code vulnerabilities — which drowned out real CodeQL findings. - scorecard.yml: stop uploading SARIF to the code-scanning tab (drop the upload-sarif step + the now-unused security-events: write). The run still produces the OpenSSF badge (publish_results) and a downloadable SARIF artifact. - TokenPermissions hardening (the high-severity, genuinely-valuable subset): set each workflow's top-level token to read-only and grant the exact writes at the job level that needs them — npm-publish (id-token/packages on publish jobs), docker-publish (packages on build), electron-release (contents on build/release, id-token/packages on publish-npm), build-fork (packages on build), claude (empty top-level; job grants its own). The 155 existing alerts were dismissed. Not adopting repo-wide SHA-pinning (143 PinnedDependencies advisories) — declined. * test(integration): align stale wiring/socks5 integration tests to this cycle's behavior These were red on the CI Integration job (pre-existing). No production code changed: - integration-wiring: the combos page no longer renders a per-page EmailPrivacyToggle (#3822 consolidated it into Settings → Appearance); the provider-detail test-result masking and upstream-proxy copy moved to decomposed components (#3501 BatchTestResultsModal / UpstreamProxyCard) — assertions now read the owning files. - api-routes-critical: SOCKS5 is now enabled by default (opt-out), so the disabled- rejection test must set ENABLE_SOCKS5_PROXY=false explicitly (an unset env now means enabled). (The ~32 live-Gemini integration tests are gated on OMNIROUTE_API_KEY and skip in CI; they only 'fail' locally when that key is present without a running server.)
* chore(release): continue v3.8.25 development cycle after main code-sync (r5) main fast-forwarded to release/v3.8.25 (cheaper-inference#3863): unblocked Build+Docker via cheaper-inference#3864, plus cheaper-inference#3837 (mimocode proxy) and cheaper-inference#3862 (trivy bump). This marker re-opens the umbrella PR for further v3.8.25 work. No version bump. * fix(db): persist the Keep-latest-backups retention setting (cheaper-inference#3834) (cheaper-inference#3867) * fix(oauth): clear GitLab Duo setup message instead of 500 (cheaper-inference#3861) (cheaper-inference#3868) * test(oauth): prove refresh_token preserved on real gemini-cli/antigravity dispatch (cheaper-inference#3850) (cheaper-inference#3869) * feat(compression-ui): unified compression config UI — per-engine pages + combos editor + menu + WS default-on (cheaper-inference#3860) Integrated into release/v3.8.25 — feat(compression-ui): unified compression configuration UI (Compression Hub + per-engine Lite/Aggressive/Ultra pages + combos editor + sidebar entry + live-WS default-on). File-size re-baselined for sidebarVisibility.ts/chatCore.ts growth; orphan ws test relocated to a collected path. * docs(changelog): complete the v3.8.25 release notes + credit all contributors Audited every commit since v3.8.24 and filled the gaps the [3.8.25] section was missing: a New Features section (compression engines + Compression Studios cheaper-inference#3848, compression UI cheaper-inference#3860, injection-guard cheaper-inference#3857, kiro discovery cheaper-inference#3836, Veo cheaper-inference#3839, mimocode proxy cheaper-inference#3837, Arena ELO flag cheaper-inference#3821), 9 more Fixed entries (cheaper-inference#3811/cheaper-inference#3807/cheaper-inference#3759/cheaper-inference#3849/cheaper-inference#3838/cheaper-inference#3835/cheaper-inference#3814/cheaper-inference#3820/cheaper-inference#3819), a Security section (CCR IDOR cheaper-inference#3859, supply-chain cheaper-inference#3824), and an Internal/Quality section. Every contributor and issue reporter is now credited. * docs(changelog): restore + complete the v3.8.25 release notes Re-adds CHANGELOG.md (a prior server-side commit accidentally dropped it) with the complete, audited [3.8.25] section: New Features, the full Fixed list, Security & Hardening, and Internal/Quality — every contributor and issue reporter credited. * chore(release): finalize v3.8.25 — reconcile CHANGELOG + i18n mirrors, document OMNIROUTE_MAX_PENDING_MIGRATIONS, green the unit suite Release-gate reconciliation for v3.8.25: - CHANGELOG: dated 2026-06-14, linked cheaper-inference#3826, rolled up file-size re-baselines (cheaper-inference#3823/cheaper-inference#3833), recorded the test-greening; re-synced all 41 i18n CHANGELOG mirrors. - Documented OMNIROUTE_MAX_PENDING_MIGRATIONS (cheaper-inference#3416) in .env.example + ENVIRONMENT.md. - Greened the unit suite (was merged red on 4 CI shards): aligned 10 stale tests to this cycle's intended behavior (cheaper-inference#3838/cheaper-inference#3822/cheaper-inference#3501/SOCKS5/Vertex-Express/Antigravity) and the same-provider 503 fall-through test; de-flaked the compression benchmark reproducibility and ServiceSupervisor crash tests. No production code changed. * ci(security): clear OpenSSF Scorecard code-scanning noise + harden workflow token permissions The Security tab held 155 open alerts, ALL from the advisory OpenSSF Scorecard tool (cheaper-inference#3824) — supply-chain/posture scores, not code vulnerabilities — which drowned out real CodeQL findings. - scorecard.yml: stop uploading SARIF to the code-scanning tab (drop the upload-sarif step + the now-unused security-events: write). The run still produces the OpenSSF badge (publish_results) and a downloadable SARIF artifact. - TokenPermissions hardening (the high-severity, genuinely-valuable subset): set each workflow's top-level token to read-only and grant the exact writes at the job level that needs them — npm-publish (id-token/packages on publish jobs), docker-publish (packages on build), electron-release (contents on build/release, id-token/packages on publish-npm), build-fork (packages on build), claude (empty top-level; job grants its own). The 155 existing alerts were dismissed. Not adopting repo-wide SHA-pinning (143 PinnedDependencies advisories) — declined. * test(integration): align stale wiring/socks5 integration tests to this cycle's behavior These were red on the CI Integration job (pre-existing). No production code changed: - integration-wiring: the combos page no longer renders a per-page EmailPrivacyToggle (cheaper-inference#3822 consolidated it into Settings → Appearance); the provider-detail test-result masking and upstream-proxy copy moved to decomposed components (cheaper-inference#3501 BatchTestResultsModal / UpstreamProxyCard) — assertions now read the owning files. - api-routes-critical: SOCKS5 is now enabled by default (opt-out), so the disabled- rejection test must set ENABLE_SOCKS5_PROXY=false explicitly (an unset env now means enabled). (The ~32 live-Gemini integration tests are gated on OMNIROUTE_API_KEY and skip in CI; they only 'fail' locally when that key is present without a running server.)
* chore(release): continue v3.8.25 development cycle after main code-sync (r5) main fast-forwarded to release/v3.8.25 (cheaper-inference#3863): unblocked Build+Docker via cheaper-inference#3864, plus cheaper-inference#3837 (mimocode proxy) and cheaper-inference#3862 (trivy bump). This marker re-opens the umbrella PR for further v3.8.25 work. No version bump. * fix(db): persist the Keep-latest-backups retention setting (cheaper-inference#3834) (cheaper-inference#3867) * fix(oauth): clear GitLab Duo setup message instead of 500 (cheaper-inference#3861) (cheaper-inference#3868) * test(oauth): prove refresh_token preserved on real gemini-cli/antigravity dispatch (cheaper-inference#3850) (cheaper-inference#3869) * feat(compression-ui): unified compression config UI — per-engine pages + combos editor + menu + WS default-on (cheaper-inference#3860) Integrated into release/v3.8.25 — feat(compression-ui): unified compression configuration UI (Compression Hub + per-engine Lite/Aggressive/Ultra pages + combos editor + sidebar entry + live-WS default-on). File-size re-baselined for sidebarVisibility.ts/chatCore.ts growth; orphan ws test relocated to a collected path. * docs(changelog): complete the v3.8.25 release notes + credit all contributors Audited every commit since v3.8.24 and filled the gaps the [3.8.25] section was missing: a New Features section (compression engines + Compression Studios cheaper-inference#3848, compression UI cheaper-inference#3860, injection-guard cheaper-inference#3857, kiro discovery cheaper-inference#3836, Veo cheaper-inference#3839, mimocode proxy cheaper-inference#3837, Arena ELO flag cheaper-inference#3821), 9 more Fixed entries (cheaper-inference#3811/cheaper-inference#3807/cheaper-inference#3759/cheaper-inference#3849/cheaper-inference#3838/cheaper-inference#3835/cheaper-inference#3814/cheaper-inference#3820/cheaper-inference#3819), a Security section (CCR IDOR cheaper-inference#3859, supply-chain cheaper-inference#3824), and an Internal/Quality section. Every contributor and issue reporter is now credited. * docs(changelog): restore + complete the v3.8.25 release notes Re-adds CHANGELOG.md (a prior server-side commit accidentally dropped it) with the complete, audited [3.8.25] section: New Features, the full Fixed list, Security & Hardening, and Internal/Quality — every contributor and issue reporter credited. * chore(release): finalize v3.8.25 — reconcile CHANGELOG + i18n mirrors, document OMNIROUTE_MAX_PENDING_MIGRATIONS, green the unit suite Release-gate reconciliation for v3.8.25: - CHANGELOG: dated 2026-06-14, linked cheaper-inference#3826, rolled up file-size re-baselines (cheaper-inference#3823/cheaper-inference#3833), recorded the test-greening; re-synced all 41 i18n CHANGELOG mirrors. - Documented OMNIROUTE_MAX_PENDING_MIGRATIONS (cheaper-inference#3416) in .env.example + ENVIRONMENT.md. - Greened the unit suite (was merged red on 4 CI shards): aligned 10 stale tests to this cycle's intended behavior (cheaper-inference#3838/cheaper-inference#3822/cheaper-inference#3501/SOCKS5/Vertex-Express/Antigravity) and the same-provider 503 fall-through test; de-flaked the compression benchmark reproducibility and ServiceSupervisor crash tests. No production code changed. * ci(security): clear OpenSSF Scorecard code-scanning noise + harden workflow token permissions The Security tab held 155 open alerts, ALL from the advisory OpenSSF Scorecard tool (cheaper-inference#3824) — supply-chain/posture scores, not code vulnerabilities — which drowned out real CodeQL findings. - scorecard.yml: stop uploading SARIF to the code-scanning tab (drop the upload-sarif step + the now-unused security-events: write). The run still produces the OpenSSF badge (publish_results) and a downloadable SARIF artifact. - TokenPermissions hardening (the high-severity, genuinely-valuable subset): set each workflow's top-level token to read-only and grant the exact writes at the job level that needs them — npm-publish (id-token/packages on publish jobs), docker-publish (packages on build), electron-release (contents on build/release, id-token/packages on publish-npm), build-fork (packages on build), claude (empty top-level; job grants its own). The 155 existing alerts were dismissed. Not adopting repo-wide SHA-pinning (143 PinnedDependencies advisories) — declined. * test(integration): align stale wiring/socks5 integration tests to this cycle's behavior These were red on the CI Integration job (pre-existing). No production code changed: - integration-wiring: the combos page no longer renders a per-page EmailPrivacyToggle (cheaper-inference#3822 consolidated it into Settings → Appearance); the provider-detail test-result masking and upstream-proxy copy moved to decomposed components (cheaper-inference#3501 BatchTestResultsModal / UpstreamProxyCard) — assertions now read the owning files. - api-routes-critical: SOCKS5 is now enabled by default (opt-out), so the disabled- rejection test must set ENABLE_SOCKS5_PROXY=false explicitly (an unset env now means enabled). (The ~32 live-Gemini integration tests are gated on OMNIROUTE_API_KEY and skip in CI; they only 'fail' locally when that key is present without a running server.)
* chore(release): continue v3.8.25 development cycle after main code-sync (r5) main fast-forwarded to release/v3.8.25 (cheaper-inference#3863): unblocked Build+Docker via cheaper-inference#3864, plus cheaper-inference#3837 (mimocode proxy) and cheaper-inference#3862 (trivy bump). This marker re-opens the umbrella PR for further v3.8.25 work. No version bump. * fix(db): persist the Keep-latest-backups retention setting (cheaper-inference#3834) (cheaper-inference#3867) * fix(oauth): clear GitLab Duo setup message instead of 500 (cheaper-inference#3861) (cheaper-inference#3868) * test(oauth): prove refresh_token preserved on real gemini-cli/antigravity dispatch (cheaper-inference#3850) (cheaper-inference#3869) * feat(compression-ui): unified compression config UI — per-engine pages + combos editor + menu + WS default-on (cheaper-inference#3860) Integrated into release/v3.8.25 — feat(compression-ui): unified compression configuration UI (Compression Hub + per-engine Lite/Aggressive/Ultra pages + combos editor + sidebar entry + live-WS default-on). File-size re-baselined for sidebarVisibility.ts/chatCore.ts growth; orphan ws test relocated to a collected path. * docs(changelog): complete the v3.8.25 release notes + credit all contributors Audited every commit since v3.8.24 and filled the gaps the [3.8.25] section was missing: a New Features section (compression engines + Compression Studios cheaper-inference#3848, compression UI cheaper-inference#3860, injection-guard cheaper-inference#3857, kiro discovery cheaper-inference#3836, Veo cheaper-inference#3839, mimocode proxy cheaper-inference#3837, Arena ELO flag cheaper-inference#3821), 9 more Fixed entries (cheaper-inference#3811/cheaper-inference#3807/cheaper-inference#3759/cheaper-inference#3849/cheaper-inference#3838/cheaper-inference#3835/cheaper-inference#3814/cheaper-inference#3820/cheaper-inference#3819), a Security section (CCR IDOR cheaper-inference#3859, supply-chain cheaper-inference#3824), and an Internal/Quality section. Every contributor and issue reporter is now credited. * docs(changelog): restore + complete the v3.8.25 release notes Re-adds CHANGELOG.md (a prior server-side commit accidentally dropped it) with the complete, audited [3.8.25] section: New Features, the full Fixed list, Security & Hardening, and Internal/Quality — every contributor and issue reporter credited. * chore(release): finalize v3.8.25 — reconcile CHANGELOG + i18n mirrors, document OMNIROUTE_MAX_PENDING_MIGRATIONS, green the unit suite Release-gate reconciliation for v3.8.25: - CHANGELOG: dated 2026-06-14, linked cheaper-inference#3826, rolled up file-size re-baselines (cheaper-inference#3823/cheaper-inference#3833), recorded the test-greening; re-synced all 41 i18n CHANGELOG mirrors. - Documented OMNIROUTE_MAX_PENDING_MIGRATIONS (cheaper-inference#3416) in .env.example + ENVIRONMENT.md. - Greened the unit suite (was merged red on 4 CI shards): aligned 10 stale tests to this cycle's intended behavior (cheaper-inference#3838/cheaper-inference#3822/cheaper-inference#3501/SOCKS5/Vertex-Express/Antigravity) and the same-provider 503 fall-through test; de-flaked the compression benchmark reproducibility and ServiceSupervisor crash tests. No production code changed. * ci(security): clear OpenSSF Scorecard code-scanning noise + harden workflow token permissions The Security tab held 155 open alerts, ALL from the advisory OpenSSF Scorecard tool (cheaper-inference#3824) — supply-chain/posture scores, not code vulnerabilities — which drowned out real CodeQL findings. - scorecard.yml: stop uploading SARIF to the code-scanning tab (drop the upload-sarif step + the now-unused security-events: write). The run still produces the OpenSSF badge (publish_results) and a downloadable SARIF artifact. - TokenPermissions hardening (the high-severity, genuinely-valuable subset): set each workflow's top-level token to read-only and grant the exact writes at the job level that needs them — npm-publish (id-token/packages on publish jobs), docker-publish (packages on build), electron-release (contents on build/release, id-token/packages on publish-npm), build-fork (packages on build), claude (empty top-level; job grants its own). The 155 existing alerts were dismissed. Not adopting repo-wide SHA-pinning (143 PinnedDependencies advisories) — declined. * test(integration): align stale wiring/socks5 integration tests to this cycle's behavior These were red on the CI Integration job (pre-existing). No production code changed: - integration-wiring: the combos page no longer renders a per-page EmailPrivacyToggle (cheaper-inference#3822 consolidated it into Settings → Appearance); the provider-detail test-result masking and upstream-proxy copy moved to decomposed components (cheaper-inference#3501 BatchTestResultsModal / UpstreamProxyCard) — assertions now read the owning files. - api-routes-critical: SOCKS5 is now enabled by default (opt-out), so the disabled- rejection test must set ENABLE_SOCKS5_PROXY=false explicitly (an unset env now means enabled). (The ~32 live-Gemini integration tests are gated on OMNIROUTE_API_KEY and skip in CI; they only 'fail' locally when that key is present without a running server.)
Closes #3861
Problem
Adding a GitLab Duo connection (Providers → GitLab Duo → Add Connection) returned an opaque
Internal server errorbefore any redirect to GitLab. This is distinct from #3732 (a redirect-URI mismatch that only surfaces after the redirect): herebuildAuthUrlthrows whenGITLAB_DUO_OAUTH_CLIENT_IDis unconfigured (GitLab Duo requires a self-registered OAuth app), and the authorize route swallowed the throw into a generic 500.Fix (mirror the existing Qoder pattern)
src/lib/oauth/providers/gitlab-duo.ts:buildAuthUrlnowreturn nullinstead ofthrowwhenclientIdis missing.src/app/api/oauth/[provider]/[action]/route.ts: agitlab-duoguard (next to the existingqoderone) returns{ supported: false, error }with actionable setup steps — register an app athttps://gitlab.com/-/profile/applications, redirect URIhttp://localhost:20128/callback, scopesai_features read_user, setGITLAB_DUO_OAUTH_CLIENT_ID.The operator still needs to register the OAuth app to actually connect — but they now get a clear instruction instead of a 500.
Tests (TDD)
tests/unit/oauth-providers-config.test.ts: assertsbuildAuthUrlreturnsnull(not throws) with an emptyclientId, and returns a valid authorize URL (client_id + PKCE challenge) when configured. The null assertion fails on the unfixed code (it threw).typecheck:coreclean, lint 0 errors, file-size re-baselined (903→916, qoder-parallel guard, justified).