<!--IssueSummary start--> <details> <summary> Everyone can contribute. [Help move this issue forward](https://handbook.gitlab.com/handbook/marketing/developer-relations/engineering/community-contributors-workflows/#contributor-links) while earning points, leveling up and collecting rewards. </summary> - [Close this issue](https://contributors.gitlab.com/manage-issue?action=close&projectId=278964&issueIid=628589) </details> <!--IssueSummary end--> ## Why The GitHub continuous sync feature exists as a working proof of concept in merge request [!251883](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/251883) (`Draft: POC - Import GitHub repositories with continuous sync`, branch `feature/github-connected-project-poc`). It is about 40,000 lines, was never code-reviewed, and a real bug was already found in it. That MR will be closed and never merged. We are porting its pieces into `master` as small, reviewable merge requests, all behind the `github_continuous_import` feature flag (type `wip`, default off). This issue is how GitHub-side events reach GitLab. GitHub POSTs a webhook to a public, unauthenticated endpoint. This layer proves the request came from GitHub (an HMAC-SHA256 signature over the raw bytes) and turns it into work for the sync engine from https://gitlab.com/gitlab-org/gitlab/-/work_items/628588. Settled in the 2026-09-10 scope audit: webhooks are hints, not durable records. The PoC's delivery table (`Continuity::WebhookDelivery`), `ProcessWebhookDeliveryService`, and `WebhookDeliveryWorker` are not ported; nothing here persists a webhook. The flow is `AppWebhooksController` reads the raw body and hands it to `WebhookService`, which verifies the HMAC signature and parses the payload, then routes through `WebhookEventRouter`: a sync event calls https://gitlab.com/gitlab-org/gitlab/-/work_items/628588's `ScheduleReconcileService` (bumping `desired_generation` for the affected domains) and enqueues `ReconcileWorker`; a lifecycle event enqueues `LifecycleRepairWorker.perform_async(installation_id)`, which loads every `Import::Sync::Repository` row sharing that `provider_installation_id`, re-derives installation state from GitHub through `ApplyLifecycleEventService`/`ResolveLifecycleStateService`, and never trusts the payload. Two design rules carry the whole security model, and the reviewer should hold the port to both: - The signature is checked before the body is parsed, by anything. If it is checked late, loosely, or only on some paths, anyone on the internet can drive reconciliation or lifecycle changes. - Payloads are hints only. They trigger a re-read of current GitHub state; their fields are never copied into GitLab data. A forged or replayed webhook that somehow passed the check could at worst cause a redundant pass. The same layer handles "lifecycle" events about the GitHub App installation itself (suspended, removed, repository access removed, permissions reduced) by disconnecting the affected repositories and restoring them later when access returns, plus a repair worker that re-derives installation state in case a lifecycle webhook was missed. Installation-scoped webhooks aren't routed to the owning cell under Cells (see the Cells routing issue, https://gitlab.com/gitlab-org/gitlab/-/work_items/628741); the 30-minute repair cron on each cell is what actually applies them there. ## What to do Read the files below and port them into a fresh MR against `master`, renaming `Continuity::Github::` to `Import::Sync::Github::` and `Continuity::Repositories::` to `Import::Sync::Repositories::` (paths move to `app/services/import/sync/...`, `app/controllers/import/sync/...`, `app/workers/import/sync/...`). Rename the advisory lock namespace, queue names, table names in `defer_on_database_health_signal`, and the JSON schema file to match. **Ingestion.** `AppWebhooksController#create` has no `before_action`s: it reads a bounded raw body (`head :payload_too_large` over the limit) and hands `raw_body` plus the `X-Hub-Signature-256`, `X-GitHub-Delivery`, and `X-GitHub-Event` headers to `WebhookService`. That service does `return error(:invalid_signature) unless valid_signature?` before `parse_payload`; `valid_signature?` checks the `sha256=` format, computes `OpenSSL::HMAC.hexdigest('SHA256', webhook_secret, raw_body)`, and compares with `ActiveSupport::SecurityUtils.secure_compare`. It then extracts routing identifiers (`installation_id`, `repository_id`) from the parsed payload and resolves the matching `Import::Sync::Repository` row(s), the same fields the PoC used to look up a connection. From there the port diverges from the PoC: instead of persisting a `WebhookDelivery` row and enqueuing `WebhookDeliveryWorker`, `WebhookService` builds a `WebhookEventRouter` for the event/action and, for a supported sync event, calls `ScheduleReconcileService` directly for the mapped repository and enqueues `ReconcileWorker` with the captured generation (the same two calls `RequestUpdateService` already makes for "Update now"); for a lifecycle event (installation suspend/unsuspend/removal, installation_repositories added/removed, and repository transfer/deletion), it enqueues `LifecycleRepairWorker.perform_async(installation_id)` with no cursor or fingerprint, so the repair worker starts a fresh first page over every repository row sharing that installation ID. `WebhookEventRouter` itself is unchanged: a pure lookup table from event/action to a sync domain or a lifecycle action, no I/O. A duplicate or replayed webhook is harmless under this design: at worst it triggers a redundant `desired_generation` bump (which `ScheduleReconcileService`'s row lock already coalesces) or a redundant `LifecycleRepairWorker` enqueue (deduplicated by its own Sidekiq `deduplicate` declaration), and `ReconcileWorker`/`LifecycleRepairWorker` are themselves idempotent. No delivery-level dedup table or unique index is needed to keep that property. **Do not port the `lib/gitlab/middleware/json_validation.rb` change.** The PoC exempts the webhook path from the JSON validation middleware and pre-fills an empty Rails parameter hash so nothing parses the body before the HMAC check. That is not how GitLab's existing inbound receivers work: the Slack events receiver verifies its signature over `request.raw_post` inside the controller (`Gitlab::Slack::Request.verify!`) and lets the middleware and Rails parameter parsing run first. Follow that precedent. The middleware's bounded parsing is a protection for a public, unauthenticated POST, not a threat to it, and the signature check still means only GitHub-signed payloads reach the controller's own parsing. Keep the controller's body-size cap. **Lifecycle.** `ApplyLifecycleEventService` dispatches on the installation state re-read from GitHub (`ResolveLifecycleStateService`, landed in https://gitlab.com/gitlab-org/gitlab/-/work_items/628585, which never trusts the payload) and sets one of four `disconnect_reason` values: `installation_suspended` (suspend), `installation_removed` (remove), `repository_access_removed` (an active installation lost access to specific repositories), `github_app_permissions_required` (permissions reduced). It loads every `Import::Sync::Repository` row with the matching `provider_installation_id` and applies the outcome to the whole set in one transaction: on disconnect it stores `restore_authority_state` (`provisioning` if the import was still running, else `provider_canonical`) next to `authority_state: :disconnected` on each row; on `activate_installation` it restores every row the same way, with `authority_state: repository.restore_authority_state, disconnect_reason: nil, restore_authority_state: nil, application_generation: ... + 1`, only for those four reasons. `installation_repositories.removed` and `repository` events (transfer/deletion) narrow this to the named repositories only, not the whole installation. `DisconnectForProviderAccessLossService` stays per repository: it re-proves that one repository is still missing from the installation before disconnecting it with `repository_access_removed`; it calls `FailBootstrapService` (https://gitlab.com/gitlab-org/gitlab/-/work_items/628586) when the lost project is still provisioning. `LifecycleRepairWorker` re-drives the lifecycle check per installation ID with a cursor and fingerprint, declares `deduplicate :until_executed, including_scheduled: true` and `idempotent!`; this is what makes a duplicate lifecycle webhook harmless. **`LifecycleMutationBarrier`, `BootstrapMutationBarrier`, and `ApplicationMutationBarrier` are not ported.** All three are PostgreSQL advisory locks; none is landed by https://gitlab.com/gitlab-org/gitlab/-/work_items/628582 or anywhere else. They hold a session-level `pg_advisory_lock` across a transaction and the network calls to GitHub these two services make, and GitLab requires PgBouncer in transaction pooling mode (`doc/administration/postgresql/pgbouncer.md`, line 247), under which a session lock taken on one backend connection can't reliably be released on another and can leak. In the PoC, `ApplyLifecycleEventService#execute` and `DisconnectForProviderAccessLossService#execute` each wrap their whole run in `LifecycleMutationBarrier.new(connection_id).with_exclusive`, a per-connection lock that serializes an entire lifecycle page (read GitHub state, then mutate every affected repository) against any other lifecycle event on the same connection. There is no connection row to key that lock on anymore; drop that outer wrap entirely, and with it the per-connection locking model. `ApplyLifecycleEventService` already re-resolves installation state (the `revoked?`/`removed?` guards, evaluated against the freshly re-read installation) right after starting, so the provider read stays current without a lock held around it; the write side is unaffected because every repository mutation already goes through `with_repository_fence`, which in the PoC stacks `BootstrapMutationBarrier.with_exclusive`, `ApplicationLease.execute`, and `ApplicationMutationBarrier.with_exclusive` around a `repository.with_lock` plus `lease_context.verify_repository!`. Drop the two barrier layers there too; keep the `ApplicationLease` (landed in https://gitlab.com/gitlab-org/gitlab/-/work_items/628582) and the row lock with its generation re-check, exactly as every other caller in this port does. Since one lifecycle event now applies to every repository sharing an installation ID, lock that set of repository rows in a stable order (for example, by `id`) before mutating them, the same way any multi-row update in this codebase avoids deadlocking against another job doing the same fan-out in a different order. `DisconnectForProviderAccessLossService#disconnect_current_binding` follows the same pattern and gets the same fix; its `current_provider_binding?` check already re-verifies `application_generation_current?` immediately before writing, so that fence is unaffected. The accepted narrowing: without the outer per-connection lock, two lifecycle events for the same installation can now interleave their page processing instead of running one fully before the other starts; each individual repository row write still stays consistent under its own row lock, but the page-level "one lifecycle event at a time" ordering the PoC provided is gone. Call this out in the MR description. Landing this issue resolves the one forward reference https://gitlab.com/gitlab-org/gitlab/-/work_items/628588 left open: `DisconnectForProviderAccessLossService` from `ReconcileWorker`. Remove whatever guard https://gitlab.com/gitlab-org/gitlab/-/work_items/628588 added for it if it is no longer needed. **Simulator.** The PoC's offline GitHub simulator is not ported by any issue. Remove these branches: `lifecycle_repair_worker.rb:44` (`return if connection.test_mode?`), `apply_lifecycle_event_service.rb:68` and `:75` (keep the `revoked? || removed?` part of the second). Two specs drive the simulator and must be rewritten with stubbed GitHub HTTP responses (WebMock/Octokit, as the existing importer specs do) and local fixtures: `app_webhook_convergence_spec.rb`, `apply_lifecycle_event_service_spec.rb`. ## PoC files to read Production: `app/services/continuity/github/{webhook_service,webhook_event_router,apply_lifecycle_event_service}.rb`, `app/services/continuity/repositories/disconnect_for_provider_access_loss_service.rb`, `app/controllers/continuity/github/app_webhooks_controller.rb`, `app/workers/continuity/github/lifecycle_repair_worker.rb`, the route in `config/routes.rb` (`post '/github/app/webhooks', to: 'continuity/github/app_webhooks#create', as: :github_app_webhooks, format: false`), and the `continuity_github_lifecycle_repair` entry in `app/workers/all_queues.yml` and `config/sidekiq_queues.yml` (urgency low, weight 1, idempotent, external dependencies). `process_webhook_delivery_service.rb`, `webhook_delivery_worker.rb`, and `app/validators/json_schemas/continuity_webhook_delivery_lifecycle_metadata.json` are not ported (see "Why" above); read `process_webhook_delivery_service.rb` only to see what `WebhookService`'s reconciliation branch and `LifecycleRepairWorker`'s enqueue replace. `resolve_lifecycle_state_service.rb` already landed (https://gitlab.com/gitlab-org/gitlab/-/work_items/628585); read it only to understand what `ApplyLifecycleEventService` calls, don't port it again. `lifecycle_mutation_barrier.rb` is not landed anywhere; read it only to understand what the row-lock replacement above is standing in for. Specs: `spec/services/continuity/github/{webhook_service,webhook_event_router,apply_lifecycle_event_service}_spec.rb`, `spec/services/continuity/repositories/disconnect_for_provider_access_loss_service_spec.rb`, `spec/requests/continuity/github/{app_webhooks,app_webhook_convergence}_spec.rb` (the controller has request specs only, nothing under `spec/controllers/`; both must be rewritten for the no-delivery-table design, since they currently assert on persisted `WebhookDelivery` rows), `spec/workers/continuity/github/lifecycle_repair_worker_spec.rb`. `process_webhook_delivery_service_spec.rb` and `webhook_delivery_worker_spec.rb` are dropped, not ported. ## Acceptance criteria - Everything above exists under the `Import::Sync` namespace with passing specs; the route responds and the queue entries are present and confirmed against `doc/development/sidekiq/`. - The reviewer has confirmed the controller reads the raw body, verifies the signature, and only then parses JSON, matching the Slack events receiver. - No `Continuity::WebhookDelivery` table, `ProcessWebhookDeliveryService`, `WebhookDeliveryWorker`, or `lifecycle_metadata` schema exists anywhere in this layer or its specs. Duplicate or replayed webhooks are harmless because a hint only bumps a generation or enqueues an idempotent worker; the reviewer has confirmed `ReconcileWorker` and `LifecycleRepairWorker` are each safe to run twice for the same event. - Each of the four `disconnect_reason` values and the `restore_authority_state` round trip (set on disconnect, consumed and cleared on restore) has a passing spec, applied across every repository sharing an installation ID, not just a single row. In the PoC, `apply_lifecycle_event_service_spec.rb` covers all four; `disconnect_for_provider_access_loss_service_spec.rb` covers only `repository_access_removed`, which is the only reason that service sets. - No `test_mode?` branch remains; the two simulator-driven specs are rewritten. - No reference to `LifecycleMutationBarrier`, `BootstrapMutationBarrier`, or `ApplicationMutationBarrier` remains anywhere in this layer or its specs. ## Out of scope - Rate limiting of the webhook endpoint (https://gitlab.com/gitlab-org/gitlab/-/work_items/628599). - The PoC's `json_validation.rb` middleware exemption (see above). - `ResolveLifecycleStateService` itself (https://gitlab.com/gitlab-org/gitlab/-/work_items/628585); this issue only calls it. - `LifecycleMutationBarrier`, `BootstrapMutationBarrier`, and `ApplicationMutationBarrier`. None is ported by https://gitlab.com/gitlab-org/gitlab/-/work_items/628582 or any other issue; see the note above for what replaces their usage here. - `Continuity::WebhookDelivery`, `ProcessWebhookDeliveryService`, `WebhookDeliveryWorker`, and the `lifecycle_metadata` JSON schema. None is ported; see "Why" above. - Stop/cutover services and `FinishStopWorker` (https://gitlab.com/gitlab-org/gitlab/-/work_items/628590). - The other routes the PoC adds nearby (`continuous_imports/*`, `app/installations/*`), which belong to https://gitlab.com/gitlab-org/gitlab/-/work_items/628585 and https://gitlab.com/gitlab-org/gitlab/-/work_items/628586. - The offline simulator. ## Notes for the reviewer - Any weakening of signature verification is high severity. Do not approve with one open. - Confirm no code path copies a payload field into a GitLab record; the payload should only influence which domain or lifecycle action runs. - Trace one sync webhook through to its `ScheduleReconcileService`/`ReconcileWorker` enqueue, and one `installation_suspended` disconnect through to restore for every repository sharing that installation ID, checking `restore_authority_state` is cleared on each. Confirm a duplicate of each is harmless without needing a stored delivery row.