Skip to content

feat(gitlab): add admin job runner for credential migration#4625

Merged
eshurakov merged 2 commits into
mainfrom
scintillating-thread
Jul 20, 2026
Merged

feat(gitlab): add admin job runner for credential migration#4625
eshurakov merged 2 commits into
mainfrom
scintillating-thread

Conversation

@eshurakov

Copy link
Copy Markdown
Contributor

Summary

Adds a database-backed job runner that executes the GitLab credential migration (the follow-up to #4539's encryption-at-rest work) in resumable, lease-fenced batches instead of a single long-running invocation.

  • New gitlab_credential_migration_jobs table (migration 0190): phase state machine (public_auditbackfill/scrubprivate_audit → final audits), lease token + expiry, cursor checkpoints, persisted audit counts, and a singleton active-job partial unique index keyed on a constant expression so only one queued/running job can exist.
  • Admin tRPC router (adminGitLabCredentialMigrationRouter) to start (confirmation-string gated for backfill/scrub), run one batch per call, inspect, list, and cancel jobs. Every state mutation is fenced by the lease token, and the requesting admin is re-validated on each step.
  • Private-audit HTTP client for the git-token service /internal/gitlab/credential-audit endpoint: internal service token with a dedicated audience, bounded 16 KB responses, 30 s timeout, and terminal/retryable error classification.
  • Migration core refactored into processGitLabCredentialMigrationBatch with keyset pagination. The credential-table anomaly audit now counts only orphaned credentials — previously it counted any non-GitLab parent, which would have blocked every run in production since Bitbucket legitimately shares those tables.

Visual Changes

N/A

Reviewer Notes

  • schema.ts: the UQ_gitlab_credential_migration_jobs_active partial unique index intentionally uses a constant expression ((1)) — keying it on status would allow one queued + one running job and turn lease acquisition into an unhandled 23505.
  • credential-migration.ts auditOrphanedCredentials: deliberately only flags credentials whose parent integration is gone.
  • The runner is invoked manually per batch by an admin; there is no cron. Each call processes 100 integrations.
  • Cancel does not roll back completed backfill/scrub mutations (disclosed in the API response); both operations are idempotent.

Add a database-backed job runner that drives the GitLab credential
migration (public audit, backfill, private audit, scrub, final audits)
in resumable, lease-fenced batches of 100 integrations:

- New gitlab_credential_migration_jobs table (migration 0190) with a
  singleton active-job partial unique index, lease fencing, cursor
  checkpoints, and audit count persistence
- Admin tRPC router to start (confirmation-gated), run, inspect, list,
  and cancel migration jobs
- Private-audit client for the git-token service credential-audit
  endpoint with bounded responses and retry classification
- Refactor the migration core into processGitLabCredentialMigrationBatch
  with keyset pagination; the anomaly audit now only counts orphaned
  credentials so legitimate non-GitLab rows do not block the gate
@kilo-code-bot

kilo-code-bot Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

The previous suggestion regarding checkpointGitLabCredentialMigrationJob's inability to distinguish an omitted private-audit key field from an explicit clear has been fixed by making privateAuditKeyId and privateAuditPublicKeySha256 required (string | null) parameters, forcing every caller to explicitly choose between preserving or clearing the value; both existing call sites in credential-migration-job.ts already pass explicit values, so this is a safe, correct fix with no new issues introduced.

Files Reviewed (1 file)
  • apps/web/src/lib/integrations/platforms/gitlab/credential-migration-job-repository.ts
Previous Review Summary (commit e97ff57)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit e97ff57)

Status: 1 Issue Found | Recommendation: Merge (address suggestion when convenient)

Executive Summary

A minor data-durability footgun exists in the checkpoint repository function for private-audit key metadata, but current callers already avoid triggering it; no CRITICAL or WARNING issues were found across the migration job repository, state machine, batch processor, private-audit HTTP client, admin tRPC router, or the new database migration.

Overview

Severity Count
CRITICAL 0
WARNING 0
SUGGESTION 1
Issue Details (click to expand)

SUGGESTION

File Line Issue
apps/web/src/lib/integrations/platforms/gitlab/credential-migration-job-repository.ts 234 checkpointGitLabCredentialMigrationJob can't distinguish an omitted privateAuditKeyId/privateAuditPublicKeySha256 from an explicit clear; current call sites always pass the existing value back, so no data loss occurs today, but the API is fragile for future callers.
Files Reviewed (11 files)
  • apps/web/src/lib/integrations/platforms/gitlab/credential-migration-job-repository.ts - 1 issue
  • apps/web/src/lib/integrations/platforms/gitlab/credential-migration-job-repository.test.ts
  • apps/web/src/lib/integrations/platforms/gitlab/credential-migration-job.ts
  • apps/web/src/lib/integrations/platforms/gitlab/credential-migration.test.ts
  • apps/web/src/lib/integrations/platforms/gitlab/credential-migration.ts
  • apps/web/src/lib/integrations/platforms/gitlab/credential-private-audit-client.test.ts
  • apps/web/src/lib/integrations/platforms/gitlab/credential-private-audit-client.ts
  • apps/web/src/routers/admin-gitlab-credential-migration-router.test.ts
  • apps/web/src/routers/admin-gitlab-credential-migration-router.ts
  • apps/web/src/routers/admin-router.ts
  • packages/db/src/migrations/0190_early_landau.sql and packages/db/src/schema.ts (generated meta/0190_snapshot.json and meta/_journal.json excluded from review)

Reviewed by claude-sonnet-5 · Input: 18 · Output: 2.9K · Cached: 355.3K

Review guidance: REVIEW.md from base branch main

…heckpoint

checkpointGitLabCredentialMigrationJob accepted optional
privateAuditKeyId/privateAuditPublicKeySha256 and wrote them with ?? null,
so a caller omitting them would silently clear persisted private-audit key
metadata. Make both parameters required so the choice between preserving
the persisted value and clearing it is explicit at compile time.
@eshurakov
eshurakov merged commit 93a3232 into main Jul 20, 2026
63 checks passed
@eshurakov
eshurakov deleted the scintillating-thread branch July 20, 2026 12:10
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