Skip to content

fix(oauth): fall back to public Code Suggestions on any GitLab Duo direct_access 403 (#12958) - #13758

Merged
diegosouzapw merged 3 commits into
release/v3.8.51from
fix/12958-gitlab-duo-direct-access-403
Sep 16, 2026
Merged

diegosouzapw merged 3 commits into
release/v3.8.51from
fix/12958-gitlab-duo-direct-access-403

Conversation

@diegosouzapw

Copy link
Copy Markdown
Owner

Closes #12958

Root cause (short)

Two independent bugs in OmniRoute's own GitLab Duo code (not GitLab's server-side
behavior, which we can't fully verify without a live Duo-seat account):

  1. shouldFallbackToPublicCodeSuggestions() only treated a direct_access 403 as
    recoverable when the body contained GitLab's exact "direct connections are disabled"
    tenant-config message ([BUG] GitLab Duo: Token invalid or revoked #10365/fix(providers): fall back to public Code Suggestions endpoint on GitLab Duo direct_access 401 (#10365) #10499). The reporter's 403 is an entitlement/
    scope-resolution failure — a different failure class — so the public Code
    Suggestions fallback (which the reporter proved works with the same token) was
    never attempted, and both the connection-test path and the real chat-path executor
    hard-failed instead.
  2. In the connection-test path (testOAuthConnection), res.text() was read once to
    decide the fallback and then read again to build the error message. A fetch
    response body can only be read once, so the second read of an already-drained
    stream silently resolved to "", and the stored/surfaced error collapsed to a
    generic "Access denied" — discarding the real upstream body the operator needs to
    tell an entitlement issue apart from an instance-config issue or a genuinely revoked
    token.

Fix

  • src/lib/oauth/gitlab.ts: broadened shouldFallbackToPublicCodeSuggestions() to
    status === 401 || status === 403 (any direct_access 403, not only the tenant-
    config message). isGitLabDirectAccessDisabled() is kept for log/diagnostic
    labeling but no longer gates the fallback decision.
  • open-sse/executors/gitlab.ts (resolveRequestTarget()): removed the
    !isGitLabDirectAccessDisabled(...) guard around the hard-403 branch, so the real
    chat-path executor falls back to the public completions endpoint for the same
    broadened set of 403s instead of hard-failing every non-tenant-config 403. Kept a
    diagnostic log line distinguishing the two 403 sub-cases.
  • src/app/api/providers/[id]/test/route.ts (testOAuthConnection()): captured the
    single res.text() read for gitlab-duo in an outer-scope variable and reused it
    for the generic error-body selection instead of re-reading a drained stream. When a
    403 fails both the direct_access and the public-fallback probe, the real upstream
    body is now surfaced (trimmed of control characters, capped at 300 chars per
    docs/security/ERROR_SANITIZATION.md — this is GitLab's own JSON error body, not a
    stack trace, but capped/stripped defensively) instead of a hardcoded "Access denied".

Left the reporter's namespace_path/rate-limit suggestions as a documented follow-up
(not blocking this fix) — GitLab's public Code Suggestions API docs don't document any
accepted request-body field for direct_access, and their troubleshooting docs
describe the "multiple Duo namespaces / no default" case as a 422, not a 403, so
whether namespace_path changes server-side entitlement resolution can only be
verified against a live gitlab.com Duo-seat account.

Regression test

tests/unit/issue-12958-gitlab-duo-403-entitlement-fallback.test.ts (new):

RED on unfixed code:

✖ gitlab-duo Retest does NOT fall back on an entitlement-flavored 403 (#12958)
  AssertionError: false !== true (fallback never attempted)
✖ gitlab-duo Retest surfaces the real upstream 403 body when BOTH endpoints reject (#12958)
  AssertionError: expected the real upstream direct_access body to be surfaced, got: "Access denied"
ℹ tests 2 | pass 0 | fail 2

GREEN after the fix:

✔ gitlab-duo Retest does NOT fall back on an entitlement-flavored 403 (#12958)
✔ gitlab-duo Retest surfaces the real upstream 403 body when BOTH endpoints reject (#12958)
ℹ tests 2 | pass 2 | fail 0

Also extended tests/unit/executor-gitlab.test.ts with an equivalent case for the
chat-path executor (resolveRequestTarget()), confirming the fallback fires for an
entitlement-flavored 403 there too — 8/8 pass.

Gates run

  • npx eslint --suppressions-location config/quality/eslint-suppressions.json <changed files> → clean, 0 new warnings
  • npm run typecheck:core → clean
  • npm run check:open-sse-typecheck → openSseTypecheckErrors=0
  • node scripts/check/check-file-size.mjs → 1 pre-existing violation on open-sse/utils/stream.ts, untouched by this PR (base drift, not mine)
  • node scripts/check/check-test-discovery.mjs → OK, new test file discovered
  • node scripts/check/check-complexity.mjs → OK (2825 violations vs baseline 3218)
  • node scripts/check/check-cognitive-complexity.mjs → OK (1276 violations vs baseline 1437)
  • tests/unit/issue-12958-gitlab-duo-403-entitlement-fallback.test.ts → 2/2 pass
  • tests/unit/executor-gitlab.test.ts → 8/8 pass (7 pre-existing + 1 new)
  • tests/unit/gitlab-duo-oauth-test-401-fallback.test.ts → 3/3 pass (unaffected pre-existing 401/403-disabled fallback contract still holds)
  • pre-commit hooks (lint-staged, docs-sync, any-budget:t11, tracked-artifacts) → all passed on commit

Existing tests aligned

None weakened or removed. tests/unit/executor-gitlab.test.ts got one new test case;
the pre-existing 401/403-disabled fallback tests in both files pass unchanged, confirming
the broadened predicate is additive (still covers the original two cases plus the new one).

diegosouzapw and others added 3 commits September 15, 2026 13:58
…rect_access 403 (#12958)

Root cause: shouldFallbackToPublicCodeSuggestions() only treated a direct_access
403 as recoverable when the body contained GitLab's exact "direct connections are
disabled" tenant-config message (#10365/#10499). An entitlement/scope-resolution
403 GitLab returns for an API-only client is a different failure class, so the
public-completions fallback was never attempted even though the reporter's same
token was accepted by that endpoint. Separately, the connection-test path read
res.text() twice for gitlab-duo (once for the fallback decision, once for the
error body), so the second read of an already-drained stream silently collapsed
to "" and the real upstream error was replaced with a generic "Access denied".

Fix: broaden the fallback predicate to any 401/403, remove the
isGitLabDirectAccessDisabled() gate on the chat-path executor's hard-403 branch,
and reuse the single body read in testOAuthConnection() so a 403 that fails both
endpoints now surfaces GitLab's real (sanitized, capped) error text.

Regression test: tests/unit/issue-12958-gitlab-duo-403-entitlement-fallback.test.ts
(RED on unfixed code: fallback not attempted, body collapses to "Access denied";
GREEN after the fix). Extended tests/unit/executor-gitlab.test.ts with the same
entitlement-403 case for the chat-path executor.
@diegosouzapw
diegosouzapw merged commit 5ff85c6 into release/v3.8.51 Sep 16, 2026
18 of 21 checks passed
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…rect_access 403 (diegosouzapw#12958) (diegosouzapw#13758)

Merged in the 2026-09-16 sweep of the maintainer's own open PRs, at the owner's explicit instruction. No push was made to the PR branch: the merge took the head as the owning session left it (verified OPEN, non-draft and MERGEABLE against the release tip immediately before merging).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(providers): GitLab Duo (gitlab.com): health check 403 "direct access scope is unavailable" even with valid seat + default Duo namespace

1 participant