Skip to content

fix: validate GitLab connectivity before saving integration#916

Merged
Gkrumbach07 merged 1 commit intoambient-code:mainfrom
jwboyer:fix/gitlab-connectivity-check
Mar 19, 2026
Merged

fix: validate GitLab connectivity before saving integration#916
Gkrumbach07 merged 1 commit intoambient-code:mainfrom
jwboyer:fix/gitlab-connectivity-check

Conversation

@jwboyer
Copy link
Contributor

@jwboyer jwboyer commented Mar 13, 2026

Summary

  • add a connectivity gate in GitLab integration save flow (POST /api/auth/gitlab/connect)
  • verify GitLab instance hostname resolves before saving credentials
  • verify PAT authentication against GitLab API before saving credentials
  • return a clear 400 error when DNS resolution or authentication fails
  • add unit test coverage for DNS resolution and PAT auth failure paths

Why

Saving unreachable or invalid GitLab integrations caused failures later during runtime operations. This change fails fast at integration save time with actionable errors.

Related Issues

  • N/A

Testing Performed

  • gofmt -l components/backend/handlers/gitlab_auth.go components/backend/handlers/gitlab_auth_test.go
  • go vet ./handlers
  • golangci-lint run ./handlers (v2, 0 issues)
  • go test -tags test ./handlers -count=1

Screenshots

  • N/A (no UI changes)

Breaking Changes

  • None

@coderabbitai
Copy link

coderabbitai bot commented Mar 13, 2026

Walkthrough

Adds GitLab connectivity validation that checks instance URL format, resolves hostnames, and verifies token validity against the GitLab API before storing integration settings. Validates through HTTP 400 responses on failure and supports testability via dependency injection.

Changes

Cohort / File(s) Summary
GitLab Connectivity Validation
components/backend/handlers/gitlab_auth.go
Introduces validateGitLabConnectivity() function that parses/validates instance URLs, performs hostname resolution with 5-second timeout, and verifies token authenticity against GitLab API. Integrates validation into ConnectGitLabGlobal handler with HTTP 400 error responses on failure. Adds validateGitLabConnectivityFn package variable for dependency injection and testability.
Connectivity Validation Tests
components/backend/handlers/gitlab_auth_test.go
Adds test setup/teardown logic to mock connectivity checker via originalValidateGitLabConnectivity capture. Includes unit tests for hostname resolution failures and token authentication failures within existing test suites, asserting HTTP 400 responses with appropriate error messages.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Handler as GitLab Handler
    participant URLValidator as URL Validator
    participant DNSResolver as DNS Resolver
    participant GitLabAPI as GitLab API
    
    Client->>Handler: POST /connect-gitlab with URL & token
    Handler->>URLValidator: validateGitLabConnectivity(ctx, URL, token)
    URLValidator->>URLValidator: Parse & validate URL format
    URLValidator->>DNSResolver: Resolve hostname (5s timeout)
    alt Resolution Fails
        DNSResolver-->>URLValidator: Error
        URLValidator-->>Handler: Return error
        Handler-->>Client: HTTP 400 Error
    else Resolution Succeeds
        DNSResolver-->>URLValidator: Address list
        URLValidator->>GitLabAPI: Verify token validity
        alt Token Invalid
            GitLabAPI-->>URLValidator: Auth error
            URLValidator-->>Handler: Return error
            Handler-->>Client: HTTP 400 Error
        else Token Valid
            GitLabAPI-->>URLValidator: Success
            URLValidator-->>Handler: Return nil
            Handler->>Handler: Store GitLab settings
            Handler-->>Client: HTTP 200 Success
        end
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: adding validation for GitLab connectivity before saving integration, which matches the core objective of the PR.
Description check ✅ Passed The description is comprehensive and directly related to the changeset, covering the what, why, and testing performed for the GitLab connectivity validation.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Tip

You can disable sequence diagrams in the walkthrough.

Disable the reviews.sequence_diagrams setting to disable sequence diagrams in the walkthrough.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@components/backend/handlers/gitlab_auth.go`:
- Around line 450-454: The project-scoped handler ConnectGitLab is missing the
GitLab connectivity/auth validation that ConnectGitLabGlobal performs; after
ConnectGitLab's input validation step, call validateGitLabConnectivityFn with
the request context, req.InstanceURL, and req.PersonalAccessToken, and if it
returns an error respond with c.JSON(http.StatusBadRequest, gin.H{"error":
fmt.Sprintf("GitLab connectivity check failed: %v", err)}) and return — mirror
the same logic used in ConnectGitLabGlobal to prevent saving invalid/unreachable
credentials.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 40c43b27-ed22-40d2-a810-7deda0fac324

📥 Commits

Reviewing files that changed from the base of the PR and between f1314eb and 7aadb0f.

📒 Files selected for processing (2)
  • components/backend/handlers/gitlab_auth.go
  • components/backend/handlers/gitlab_auth_test.go

@Gkrumbach07 Gkrumbach07 added this to the Review Queue milestone Mar 16, 2026
@Gkrumbach07 Gkrumbach07 merged commit 86ed296 into ambient-code:main Mar 19, 2026
27 checks passed
@ambient-code ambient-code bot removed this from the Review Queue milestone Mar 19, 2026
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.

2 participants