fix: validate GitLab connectivity before saving integration#916
Open
jwboyer wants to merge 1 commit intoambient-code:mainfrom
Open
fix: validate GitLab connectivity before saving integration#916jwboyer wants to merge 1 commit intoambient-code:mainfrom
jwboyer wants to merge 1 commit intoambient-code:mainfrom
Conversation
WalkthroughAdds 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
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
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment Tip You can disable sequence diagrams in the walkthrough.Disable the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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
📒 Files selected for processing (2)
components/backend/handlers/gitlab_auth.gocomponents/backend/handlers/gitlab_auth_test.go
Comment on lines
+450
to
+454
| // Validate connectivity/auth before storing credentials. | ||
| if err := validateGitLabConnectivityFn(c.Request.Context(), req.InstanceURL, req.PersonalAccessToken); err != nil { | ||
| c.JSON(http.StatusBadRequest, gin.H{"error": fmt.Sprintf("GitLab connectivity check failed: %v", err)}) | ||
| return | ||
| } |
There was a problem hiding this comment.
Connectivity validation missing in project-scoped handler.
ConnectGitLabGlobal now validates connectivity before storing credentials, but the project-scoped ConnectGitLab handler (lines 156-248) does not perform the same validation. This inconsistency means project-scoped integrations can still save unreachable or invalid GitLab connections.
🐛 Suggested fix: Add connectivity check to ConnectGitLab
Add the connectivity check in ConnectGitLab after input validation (around line 188):
// Validate input
if err := validateGitLabInput(req.InstanceURL, req.PersonalAccessToken); err != nil {
c.JSON(http.StatusBadRequest, gin.H{
"error": fmt.Sprintf("Invalid input: %v", err),
"statusCode": http.StatusBadRequest,
})
return
}
+
+ // Validate connectivity/auth before storing credentials.
+ if err := validateGitLabConnectivityFn(c.Request.Context(), req.InstanceURL, req.PersonalAccessToken); err != nil {
+ c.JSON(http.StatusBadRequest, gin.H{
+ "error": fmt.Sprintf("GitLab connectivity check failed: %v", err),
+ "statusCode": http.StatusBadRequest,
+ })
+ return
+ }
// Get user ID from context (set by authentication middleware)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/backend/handlers/gitlab_auth.go` around lines 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.
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
2 participants
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
POST /api/auth/gitlab/connect)400error when DNS resolution or authentication failsWhy
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
Testing Performed
gofmt -l components/backend/handlers/gitlab_auth.go components/backend/handlers/gitlab_auth_test.gogo vet ./handlersgolangci-lint run ./handlers(v2, 0 issues)go test -tags test ./handlers -count=1Screenshots
Breaking Changes