fix(server-nestjs): harden gitlab token and user handling

[shikanime]

Revoke old mirror tokens on rotation, validate tokens via API call, restore 1-year token expiry, set ciConfigPath and user creation hardening (canCreateGroup, forceRandomPassword, projectsLimit).

Change-Id: I9dc52277970e58fc62f11363c4989c1d6a6a6964

Issues liées

Extrait de: #2407

---

Quel est le comportement actuel ?

Quel est le nouveau comportement ?

Cette PR introduit-elle un breaking change ?

Autres informations

[shikanime]

Review: PR #2405 — fix(server-nestjs): harden gitlab token and user handling

Verdict: REQUEST CHANGES (compile blocker on the changed app). Tests shown as COMMENT — GitHub blocks Approve/Request-Changes on one's own PR.

Blocker

• [blocker] apps/server-nestjs/src/modules/gitlab/gitlab.service.ts:4 — MirrorUserSecret does not exist.
  The PR imports MirrorUserSecret from ../vault/vault-client.service and uses it as the return type of getOrRotateMirrorCreds (line 487), but vault-client.service.ts never exports that type (it exports SonarqubeUserSecret, not a mirror equivalent). tsc confirms:
  gitlab.service.ts(4,15): error TS2305: Module ".../vault/vault-client.service" has no exported member 'MirrorUserSecret'.
  This breaks pnpm build (nest build → tsc) for the server-nestjs app, which is a CI gate. Vitest still passes (esbuild strips types), so the diff is "green" in unit tests but will fail the build.
  Fix (one of):
  1. Add the type to vault-client.service.ts, matching the sibling pattern:
     export interface MirrorUserSecret { MIRROR_USER: string; MIRROR_TOKEN: string }
     (this is exactly what createMirrorAccessToken returns at lines 521-524).
  2. Or drop the named type and type the return as { MIRROR_USER: string; MIRROR_TOKEN: string } / reuse VaultSecret<{MIRROR_USER:string; MIRROR_TOKEN:string}> and return vaultSecret.data (currently typed any).

Warnings

• [warning] validateProjectToken auth-header reliability (gitlab-client.service.ts:497-505).
  You pass { headers: { 'PRIVATE-TOKEN': token } } to Groups.show. In gitbeaker 40.x the request options are spread into defaultOptionsHandler(serviceOptions, {...options, method}) and then defaultOptions.headers = { ...preconfiguredHeaders }, after which the configured token is layered on via authHeaders (private-token). gitbeaker lowercases header keys (xcase.decamelizeKeys is for body, but the requester sets private-token from the constructor token). The interaction between a caller-supplied PRIVATE-TOKEN and the default private-token auth header is not clearly guaranteed to override gitbeaker's own auth, and the casing (PRIVATE-TOKEN vs private-token) matters for the GitLab API. If gitbeaker's own private-token wins (it is the configured integration token), validateProjectToken will succeed even for a wrong token → the rotation guard silently never fires. Recommend a focused test: mock Groups.show to assert the request actually carries the mirror token and NOT the integration token, or validate against the GitLab API that a bogus token returns 401. Until verified, the whole "revoke stale tokens" feature is unproven.

• [warning] validateProjectToken swallows all errors (gitlab-client.service.ts:501-503).
  catch (err) { return false } hides timeouts/5xx as "invalid token". On a transient GitLab outage every token is judged invalid and rotated (revoked + recreated) on the next sync, churning credentials. Either narrow the catch to 401/403 (GitbeakerRequestError with status === 401), or rethrow on non-auth errors so a transient failure doesn't trigger rotation.

• [warning] createProjectToken expiry uses this.config.mirrorTokenExpirationDays directly (gitlab-client.service.ts:480).
  The old code guarded against non-finite/non-positive values (fell back to 30). Now if the env var is missing/garbage the zod positive() default already forces 365, so it's mostly safe — but the value is now 365 days by default (was 180). Confirm the 1-year expiry is intended (PR text says "restore 1-year token expiry", so OK, just flagging the behaviour change).

Nits

• [nit] ENVIRONMENTS.md:261 still documents GITLAB_MIRROR_TOKEN_ROTATION_THRESHOLD_DAYS (default 90). The PR removes that env var from config (gitlab.config.ts) and the spec (gitlab.config.spec.ts) but the docs were not updated. Docs completeness: remove the row (or note it was replaced by API-based validation).

• [nit] gitlab.config.spec.ts:6 still resets GITLAB_MIRROR_TOKEN_ROTATION_THRESHOLD_DAYS in beforeEach resetEnvs([...]). Since the var no longer exists, it's a no-op but stale — drop it.

• [nit] getProjectGroup now awaits find(...) (gitlab-client.service.ts:338). Harmless, consistent with other call sites.

• [nit] ciConfigPath: '.gitlab-ci-dso.yml' (gitlab-client.service.ts:244). Hardcoded dso config path; confirm that file exists in the created repos (or is committed by commitMirror). If the file is absent the pipelines won't pick it up — verify the mirror bootstrap actually writes that file.

Verification run (local)

• pnpm install --frozen-lockfile: OK
• npx vitest run for the 3 touched specs: 55/55 pass
  (gitlab.config.spec.ts 3, gitlab-client.service.spec.ts 34, gitlab.service.spec.ts 18)
• npx eslint on the 3 changed source files: clean
• npx tsc --noEmit (server-nestjs): the only error attributable to this diff is the MirrorUserSecret TS2305 above. (The other ~266 errors are pre-existing environment issues: unbuilt @cpn-console/* workspace packages and an ungenerated Prisma client, unrelated to this PR.)

Bottom line

The token-hardening logic is sound and well-tested at the unit level, but the MirrorUserSecret missing type is a build-breaking blocker and the validateProjectToken header/catch behaviour needs verification before the rotation guard can be trusted. Fix the type + address the two warnings, then it's good to merge.

[cloud-pi-native-sonarqube]

Quality Gate passed

Issues

• 0 New Issues
• 0 Fixed Issues
• 0 Accepted Issues

Measures

• 0 Security Hotspots
• 0.00% Coverage (26.30% Estimated after merge)
• 0.00% Duplicated Code (4.20% Estimated after merge)

Project ID: cloud-pi-native_console_AYsa46O31ebDtzs-pYyZ

View in SonarQube

[shikanime]

Review: PR #2405 — harden gitlab token and user handling

Verdict: REQUEST CHANGES (GitHub won't let me set the formal state on my own PR, so flagging inline). CI green, branch mergeable. Reviewed across security / correctness / standards.

Security (positive)

• createUser now sets canCreateGroup:false, forceRandomPassword:true, projectsLimit:0 — good user hardening.
• Rotation now validates the live token via API (validateProjectToken) instead of guessing from Vault created_time. Real improvement.

Correctness

BLOCKER — ciConfigPath points at a file that never gets committed (gitlab-client.service.ts:245 vs :444)

• Projects.create(..., { ciConfigPath: '.gitlab-ci-dso.yml' }) makes GitLab look for .gitlab-ci-dso.yml at repo root.
• But commitMirror (line 444) commits .gitlab-ci.yml, never .gitlab-ci-dso.yml.
• GitLab does NOT fall back to .gitlab-ci.yml when ciConfigPath is set but missing — pipelines don't run. Looks like a copy-paste of the old plugin's clone ? '.gitlab-ci-dso.yml' : undefined semantics, which doesn't apply to a console-managed mirror repo.
• Fix: drop ciConfigPath (default .gitlab-ci.yml is what commitMirror writes) — or change commitMirror to write .gitlab-ci-dso.yml. The two must agree.

WARNING — validateProjectToken treats only HTTP 401 as invalid (gitlab-client.service.ts:500-504)

• Any other 4xx (e.g. 403 insufficient scope) is re-thrown and propagates up through getOrRotateMirrorCreds → handleUpsert, breaking reconciliation for that project.
• You already read error.cause?.response.status; extend it to treat all 4xx as invalid (return false) and only 5xx as transient (throw). Matches the test intent (502 throws, client errors = invalid).

WARNING — rotation orchestration in gitlab.service.ts has no service-level test

• gitlab-client.service.spec.ts covers revokeProjectToken, validateProjectToken, getProjectGroup, getProjectToken individually — good.
• But the orchestration (getOrRotateMirrorCreds:487 / getMirrorTokenFromVault:509: vault has a valid token → reuse; vault missing or token invalid → revoke + recreate) is untested at the service level. Add 2-3 cases against gitlab.service to lock the behavior.

Standards / Readability

• NIT createUser test (gitlab-client.service.spec.ts:745) doesn't assert the new hardening fields. Add canCreateGroup:false, forceRandomPassword:true, projectsLimit:0 to the objectContaining check.
• NIT daysAgoFromNow in gitlab.utils.ts:227 is now dead (no callers after removing the time-based rotation). Delete it.
• NIT gitlab.config.spec.ts:6 still resets GITLAB_MIRROR_TOKEN_ROTATION_THRESHOLD_DAYS, which the schema no longer defines. Drop the stale name.