Skip to content

Git integration: gaps found testing the GitLab provider end to end (provider messages hidden, private self-hosted hosts blocked, no repository picker) #656

Open

Description

@gorlix

Follow-up to #648 and #649. While testing com.paca.gitlab 0.6.0 against a real GitLab CE instance and core master, I hit four things that need a core change. They are independent. 1 and 4 are small and I'm preparing PRs for them (linked here when open); 2 and 3 change security behaviour or add a feature, so I'd like your view on the design before writing code.

1. Provider error messages never reach the user

Linking a repository. resolveThroughConnections (services/api/internal/service/git/service.go, around line 160) maps any CodeNotFound / CodeForbidden from ResolveRepository to gitdom.ErrNotVisible ("none of the connections can see this repository") and drops the provider's Message.

The GitLab provider (Paca-AI/paca-plugin-gitlab#3) now explains what each connection covers, for example: "The connected project token covers only acme/ui. Add a connection for acme/api, or use a group access token or a service account." The user only ever sees the generic sentence.

Registering a webhook. ensureWebhook (repo_webhooks.go) returns the provider error, but the web UI always shows the fixed webhook.description string ("the token probably lacks the permission to manage webhooks… Maintainer role"). That is misleading when the real cause is something else, for example GitLab refusing a URL it cannot reach, or a validation error.

Proposal: keep the error codes and add the provider's message as detail.

  • ErrNotVisible (and the webhook failure) carry the provider message, e.g. a wrapped error with a Detail field.
  • The presenter puts it in the error response (details.provider_message or similar).
  • The UI shows it under the existing generic text when present, so providers that send no message look the same as today.

Provider messages are plugin-supplied text, so the UI renders them as plain text only.

2. Self-hosted GitLab on a private network cannot be used

paca.fetch rejects any host that resolves to a private or internal IP (isAllowedFetchDomain in platform/plugin/runtime.go, plus netguard.NewSafeHTTPClient pinning the dial). It also requires https.

That guard is right as a default (DNS rebinding, GHSA-cj3q-c44j-q8p9). But a common self-hosted setup is Paca and GitLab on the same LAN or VPN, e.g. gitlab.internal → 10.0.0.5. Today that combination cannot work at all, even after the operator adds the host to the plugin's allowedOutboundDomains.

Proposal: an operator-only opt-in, off by default.

  • New env var, e.g. PLUGIN_FETCH_PRIVATE_HOSTS=gitlab.internal,git.corp.example: exact hostnames (no wildcards) that may resolve to private addresses.
  • A host must be in that list and in the plugin's manifest allowlist.
  • The dial stays pinned to the validated IP, so the rebinding protection is kept.
  • Loopback and link-local (169.254.0.0/16, metadata endpoints) stay blocked even when listed.
  • https stays required; operators with an internal CA mount it in the API container.
  • Document it next to allowedOutboundDomains in the plugin docs.

3. No repository picker when linking

The SPI already has ListRepositories (the GitLab provider lists the projects a token is a member of), but core never calls it: Service.ListRepositories only returns already-linked rows. Users have to type group/subgroup/project by hand, and with several connections they can't tell which one sees what.

Proposal:

  • GET /projects/{id}/git/connections/{identityId}/repositories?q= → calls the provider's ListRepositories for that connection (gated by git:write, paginated, with the q filter passed through).
  • In GitSettings, the link form gets a searchable picker fed by that endpoint, with free text still allowed (a provider without ListRepositories, or a repo the token sees but isn't a member of).

4. Small bug: an edited event reopens a closed change request

crState (service/git/service.go, around line 413) returns "open" for every action other than closed and merged. A title or description edit (edited), or a synchronized event, on an already merged or closed MR/PR therefore stores it as open again.

Proposal: for edited and synchronized, keep the stored state, as is already done for review events. One-line fix plus a test.

Smaller notes (no change proposed, maybe docs)

  • The webhook URL is built from PUBLIC_URL. When Paca runs on localhost, GitLab gets a hook URL it cannot reach, and the error looks like a permission problem (see 1).
  • In GitLab, editing a hook's URL in the UI clears its secret token. If an admin moves a hook by hand, signature checks start failing until "Register webhook" is clicked again, which re-sends the token.

Open questions

  1. For 1: a details field on the existing error codes, or new codes?
  2. For 2: is an env var acceptable, or would you rather have this in the admin settings?
  3. For 3: should the picker live in this round, or after v0.20?

Activity

  1. pullfrog commented on Oct 28, 2026

    @pullfrog
    Contributor
  2. gorlix commented on Oct 28, 2026

    @gorlix
    ContributorAuthor

    PRs for the two small items:

    2 and 3 are waiting for your view on the design.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions