Skip to content

feat(gitlab): two-way sync between Huly and GitLab - #52

Open
robertozimek wants to merge 23 commits into
Platform-Collective:developfrom
robertozimek:feature/gitlab-phase1
Open

robertozimek wants to merge 23 commits into
Platform-Collective:developfrom
robertozimek:feature/gitlab-phase1

Conversation

@robertozimek

@robertozimek robertozimek commented Oct 10, 2026 •

Copy link
Copy Markdown

This adds a GitLab integration, built the same way as the GitHub one. It keeps issues, comments, merge requests, reviews and images in sync between Huly and GitLab, in both directions.

Before you deploy:

  • Run the account migration first. It adds GitLab as a login type.
  • The GitLab service has two new settings: GITLAB_ALLOWED_HOSTS, for GitLab servers on a private network, and GITLAB_REQUEST_TIMEOUT_MS.

It also fixes the front-end build on develop, which failed because the language files couldn't be loaded.

The steps for testing by hand are in services/gitlab/pod-gitlab/Readme.md.

Signed-off-by: Robert Ozimek <robertozimek90@gmail.com>
Merge requests (with stored diffs, approvals and review threads), reviews, uploads and per-integration image mode; translations in all locales; dependencies for the GitLab packages.

Signed-off-by: Robert Ozimek <robertozimek90@gmail.com>
The GitLab service syncs merge requests, diffs, discussions, approvals and review ToDos both ways, links or copies images, serves linked images with the viewer's own GitLab token, uses per-integration webhook URLs and secrets, moves issues between projects, and pauses inactive workspaces. Adds GITLAB_READONLY and STORAGE_CONFIG to the dev compose file.

Signed-off-by: Robert Ozimek <robertozimek90@gmail.com>
Merge request list and view, diff panel with review threads and viewed files, approvals, repository picker in the issue dialog, image mode setting and GitLab image viewer.

Signed-off-by: Robert Ozimek <robertozimek90@gmail.com>
TodoAutomationHelper gains an optional onAutoCompleteTester. When it returns false, the assignee-change and Won/Lost automations leave the ToDo open, so integrations can own the lifecycle of their ToDos.

Signed-off-by: Robert Ozimek <robertozimek90@gmail.com>
Plugins can register a component at presentation.extension.LinkMark with an hrefPattern; Mark.svelte renders matching links with it.

Signed-off-by: Robert Ozimek <robertozimek90@gmail.com>
FileDiffView added addedLines twice instead of addedLines + deletedLines.

Signed-off-by: Robert Ozimek <robertozimek90@gmail.com>
Formatter output plus fixes for the ESLint errors it reported: no needless type assertions, explicit sort comparators, named promise resolvers, no dynamic delete, and an unused import removed.

Signed-off-by: Robert Ozimek <robertozimek90@gmail.com>
Signed-off-by: Robert Ozimek <robertozimek90@gmail.com>
The GitLab integration no longer needs changes outside services/gitlab:
- Remove the LinkMark extension point from presentation; GitLab images
  are now listed from the issue header instead.
- Remove the ToDo automation veto from time; the GitLab service reopens
  ToDos that an automation closed.
- Drop the fast-start.sh mode change.
The GitLab social id cases move from their own file into the existing
operations, service operations and postgres tests.
The gitlab service leaves dev/docker-compose.yaml. It runs on request:
docker compose -f docker-compose.yaml -f docker-compose.gitlab.yaml up -d
The overlay also passes GITLAB_ALLOWED_HOSTS and GITLAB_REQUEST_TIMEOUT_MS.
- server-gitlab owns the list of service-only classes, shared by the
  model trigger, the broadcast filter and the model test.
- The API exports GitlabTodoPurpose and records description image URLs
  on GitlabIssue; GITLAB_IMAGE_FRAGMENT moves into the pod.
- The model drops the ToDo auto-complete veto (the GitLab service now
  reopens ToDos an automation closed), and the issue's repository
  attribute uses the translated Repository label.
- The trigger's sync decision no longer depends on call order.
- Drop unused dependencies; model and trigger tests are typed and build
  the model once.
Security and correctness:
- Refuse private and reserved addresses unless listed in
  GITLAB_ALLOWED_HOSTS; every GitLab call takes an explicit fetch and host
  guard, and dev mode still requires http(s) redirect targets.
- Bound request time, response size and concurrency; retry 429s, and a
  closing worker stops waiting out a 429 without marking documents as
  failed.
- A disconnect drops tokens that are being fetched.
- Reopen GitLab ToDos that a server automation closed.
- Keep the links of an attachment block written by another Huly as
  comment text instead of importing an empty comment.

Structure:
- One "authorization expired" text and error helpers in sync/errors.ts.
- Shared sync helpers in sync/docs.ts; generic note API methods.
- Merge request patches, reviews and ToDos live in patch.ts, reviews.ts
  and todos.ts; Huly-side issue moves in sync/issue-move.ts.
- Hook install and removal in hooks.ts; named constants for timeouts,
  batch and cache sizes; dead exports removed.
- Shared test helpers; tests wait on conditions, not fixed ticks.
Structure:
- Business logic leaves components for tested modules: errors (one
  reportError), gitlab-host, service (one request envelope), project-link
  (link/unlink and webhook rollback), app-status, viewed-files,
  review-thread, issue-header, integration-card.
- Shared pieces as github-resources does: an authentication store,
  a confirm dialog helper, GitlabRefLink and ActivityFrame.
- Label helpers return IntlString; unused strings removed from all locales.

Fixes:
- The OAuth landing tab loads without a session (no reload loop) and
  shows translated errors.
- A review-thread snippet never shows a stale hunk.
- The integration card shows an error instead of loading forever.
- Issue headers list GitLab description images in a paging popup.
- The repository list shows the linked project's name once it loads.
Brings in the ESLint 9 / Jest 30 / dependency update and the CalDAV
groundwork. Conflicts:
- dev/prod and desktop platform.ts: keep the GitLab imports and add
  GitlabLang next to the other assets' lang modules; gitlab-assets gains
  src/lang.ts and its ./lang export, as the other assets packages did.
- FileDiffView.svelte: develop's formatting with the large-diff limit
  still counting deleted lines.
Matches develop's migration of the other packages:
- eslint.config.mjs from the platform-rig profile instead of .eslintrc.js
- ESLint 9, typescript-eslint 8, eslint-config-love, eslint-plugin-svelte 3,
  Jest 30; eslint-plugin-import and standard-with-typescript dropped
- pod-gitlab's base image moves to v20261006
Formatter fixes under the new rules (unnecessary type assertions, unused
eslint-disable directives, Svelte mustaches) and the imports they left
unused.
The lang refactor points each assets package's ./lang export at
lib/lang.js, but most string loaders still imported
`<pkg>/lang/${lang}.json`. Webpack no longer resolves those, so the
front bundle failed with "Package path ./lang is exported ... but no
valid target file was found" for every assets package.

- dev/prod and desktop: every loader whose package exports ./lang uses
  its `loadLang` (as platform, core and text-editor already did).
- chat-assets had the new ./lang export but no src/lang.ts; inbox-assets
  had neither. Both now match the other assets packages.
@robertozimek robertozimek changed the title Add GitLab integration feat(gitlab): two-way sync between Huly and GitLab Oct 10, 2026
@ArtyomSavchenko

Copy link
Copy Markdown
Member

Hi @robertozimek
Thank you for the contribution.
Please take a look at review comments:


Should fix

1. The pod trusts Huly document fields that a client may be able to modify (medium–high)

GitlabIntegration documents live in core.space.Configuration. Their host and connectedBy fields are marked @ReadOnly(), which in Huly is a UI hint only; the server does not enforce it. The only server trigger in this PR narrows the broadcast of DocSyncInfo and GitlabUpload. I found no write protection for these documents.

worker.integrationApi() (pod-gitlab/src/worker/worker.ts) builds its client from the document:

const record = await this.userRecord(integration.connectedBy)
return record === undefined ? undefined : this.api(integration.host, record.token)

It never checks that record.host === integration.host. The same pattern appears in service.ts (resolveRepository, removeIntegration, the refresh loop). By contrast, userApi() and apiFor() in the same file do check own.host !== integration.host, so the check is simply missing on the integration-token path.

If a regular workspace member can update such a document, they could:

  • Exfiltrate a token: set host to a server they control. HostGuard only blocks private addresses, so a public host passes, and the pod sends Authorization: Bearer <victim's OAuth token> to it.
  • Act as another user: set connectedBy to another member's social id, so the pod reads or writes GitLab with that member's token.
  • Reach other projects: change projectId on GitlabIntegrationRepository to point at another project the token owner can access.

2. /api/v1/refresh is open to any workspace member (low–medium)

The route only verifies the token and that the social id belongs to the caller. It then iterates every integration in the workspace as System, using each owner's token, and calls GitLab. There is no rate limit, so any member can trigger repeated sweeps and burn other members' GitLab rate limits. Restrict it to Maintainer and add a rate limit.

3. Unauthenticated request bodies are parsed before the webhook token is checked (low–medium)

server.ts registers bp.json({ limit: '10mb' }) globally, so up to 10 MB is parsed for every request before x-gitlab-token is verified. Mount the webhook route ahead of the global parser, check the header first, then parse with its own limit.

4. DNS rebinding gap in HostGuard (low)

HostGuard resolves the hostname, caches the result for 60 s, and fetch then resolves it again. A host that answers with a public address for the check and a private one for the request would pass. Pin the validated address in the request's lookup. Until then, document GITLAB_ALLOWED_HOSTS as recommended for production.


Nits

  • window.open(authorizeUrl) has no noopener (gitlab-resources/src/utils.ts), so the GitLab page gets window.opener. The URL comes from the pod and its scheme is not checked on the client; safeHttpUrl already exists.
  • OAuth state is replayable within its 10-minute TTL. The nonce is generated but never checked or consumed. Impact is low, because the GitLab code is single-use and the state is bound to the user and workspace. Either drop the nonce or track it.
  • cors() allows any origin. Harmless today (auth is by token in the body, no cookies), but restricting it to the front URL costs nothing.
  • hostKey ignores the URL scheme, so http://x and https://x share one identity key. Only relevant with GITLAB_ALLOW_INSECURE_HOSTS, which is dev-only.
  • No strength check for WEBHOOK_SECRET and SERVER_SECRET. The dev compose overlay defaults both to secret. A startup warning for weak values outside insecure mode would be cheap insurance.

- Server middleware keeps GitLab connection documents service-written,
  and the pod uses a token only for the host and user it was issued for
- Refresh is maintainer-only and limited to once a minute per workspace
- Webhook token is checked before the body is parsed
- Connection DNS lookups go through the host guard (DNS rebinding)
- Single-use OAuth state, CORS limited to the front, noopener on the
  authorize link, scheme-aware host keys, weak secret warning
# Conflicts:
#	desktop/src/ui/platform.ts
#	dev/prod/src/platform.ts
#	plugins/chat-assets/src/lang.ts
#	plugins/inbox-assets/src/lang.ts
@robertozimek

Copy link
Copy Markdown
Author

Thanks so much for taking the time to review this, @ArtyomSavchenko! I think this project is awesome and I'm glad to be contributing. I've implemented all the changes, including the nits.

Signed-off-by: Robert Ozimek <robertozimek90@gmail.com>
Signed-off-by: Robert Ozimek <robertozimek90@gmail.com>
@ArtyomSavchenko

ArtyomSavchenko commented Oct 14, 2026 •

Copy link
Copy Markdown
Member

Hi @robertozimek
Thank you for the kind words.
Could you also take a look at the following recommendations?

Should fix

Any workspace member can bypass the "connector or maintainer" check when enabling a repository.

  1. GitlabIntegrationMiddleware lets a User-role member write enabled and gitlabProject on GitlabIntegrationRepository. The sync worker activates a repository on those fields alone (worker.ts:643), so a raw transaction skips the check in repository-enable.
  2. Require Maintainer (or Owner) for these fields, or make the worker ignore repositories the service did not enable itself.
  3. Add a middleware test that covers this case.

Reuse existing code

Swapping the SSRF guard for @hcengineering/safe-fetch removes about 330 lines of code and about 340 lines of tests; four smaller replacements follow.

Replace host-guard.ts and gitlab/fetch.ts with safe-fetch

PR #49 is already in develop and registered in rush.json. GitLab would be its first production consumer, so coordinate with the #49 author. Required adaptations:

  1. In GitlabApi.send, pass redirect: followRedirects ? 'follow' : 'error' and maxRedirects: 3. safe-fetch follows redirects by default.
  2. Keep GITLAB_ALLOWED_HOSTS exclusive with a small wrapper. In safe-fetch, allowlist only grants exceptions to the blocked ranges.
  3. Map dev mode (GITLAB_ALLOW_INSECURE_HOSTS) to allowHttp: true plus an allowlist of private CIDRs and localhost.
  4. Set maxBodyBytes explicitly. The default is 10 MiB per response and MR diffs can be larger.
  5. Replace hostGuard.assertAllowed in saveApp with validateUrl and resolveAndCheck.
  6. Adapt error handling and tests to SafeFetchError codes.
  7. Add 100::/64 to BLOCKED_IPV6_RANGES in safe-fetch, since GitLab's guard blocks it.

Other replacements

Custom code Replace with Saves Notes
ttl-cache.ts (pod) lru-cache about 50 lines + tests Already ^11.1.0 in 5 packages. Check how to inject the clock for tests.
hunk.ts parseDiff from diffview-resources about 50 lines Export it from the package index and confirm it can be imported from the front end.
createTextCache (UI) lru-cache about 35 lines Low priority: lru-cache is not yet used in front-end code.

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