From a2a298aaef8190ec6f57b03cd85e9f44838698ea Mon Sep 17 00:00:00 2001 From: Matt Cramer Date: Mon, 15 Jun 2026 17:29:37 -0600 Subject: [PATCH] feat(gitlab): authenticate self-hosted GitLab via ASPECT_GITLAB_TOKEN The API-base override alone wasn't enough for self-hosted GitLab: the features only obtained a token by minting through the Aspect GitLab App, which is registered on gitlab.com and can't authenticate against another instance. Add gitlab_token_cache.resolve_token(), which returns (token, auth_kind): when ASPECT_GITLAB_TOKEN (a personal/group/project access token) is set it short-circuits the mint and uses the PRIVATE-TOKEN header convention; otherwise it falls back to the Aspect-App bearer mint as before. Thread the resolved auth_kind through every gitlab.* call in the three features alongside api_base, replacing the previously hard-coded "bearer". With this, self-hosted GitLab works end to end: CI_API_V4_URL (or an explicit ASPECT_GITLAB_API_URL) selects the host, ASPECT_GITLAB_TOKEN authenticates against it, and build/Web-UI URLs already derive from the native CI_SERVER_URL / CI_JOB_URL (build_metadata.axl, ci.axl). Adds resolve_token unit tests to lib/gitlab_test.axl. --- .../aspect/feature/gitlab_commit_statuses.axl | 5 ++- .../aspect/feature/gitlab_lint_comments.axl | 16 ++++---- .../aspect/feature/gitlab_status_comments.axl | 18 +++++---- .../src/builtins/aspect/lib/gitlab_test.axl | 35 +++++++++++++++++ .../aspect/lib/gitlab_token_cache.axl | 39 +++++++++++++++++++ 5 files changed, 96 insertions(+), 17 deletions(-) diff --git a/crates/aspect-cli/src/builtins/aspect/feature/gitlab_commit_statuses.axl b/crates/aspect-cli/src/builtins/aspect/feature/gitlab_commit_statuses.axl index 619fae086..1b964473a 100644 --- a/crates/aspect-cli/src/builtins/aspect/feature/gitlab_commit_statuses.axl +++ b/crates/aspect-cli/src/builtins/aspect/feature/gitlab_commit_statuses.axl @@ -235,7 +235,7 @@ def _gitlab_commit_statuses(ctx: FeatureContext): target_url = target_url or None, description = description or None, api_base = api_base, - auth_kind = "bearer", + auth_kind = _state["_auth_kind"], ) if not result["success"]: _LOG.warn(ctx.std, "commit_status POST failed (state=%s): %s" % ( @@ -249,7 +249,7 @@ def _gitlab_commit_statuses(ctx: FeatureContext): if _state.get("_gitlab_token"): return True auth_reason = {} - token = gitlab_token_cache.get_or_mint( + token, auth_kind = gitlab_token_cache.resolve_token( ctx, project_path = project_path, project_id = mr.get("project_id"), @@ -263,6 +263,7 @@ def _gitlab_commit_statuses(ctx: FeatureContext): )) return False _state["_gitlab_token"] = token + _state["_auth_kind"] = auth_kind # Re-resolve CI URL with the token in scope (mirrors GH path — # safe no-op when the CI URL is env-derived rather than diff --git a/crates/aspect-cli/src/builtins/aspect/feature/gitlab_lint_comments.axl b/crates/aspect-cli/src/builtins/aspect/feature/gitlab_lint_comments.axl index 7defc9ab8..b91579f02 100644 --- a/crates/aspect-cli/src/builtins/aspect/feature/gitlab_lint_comments.axl +++ b/crates/aspect-cli/src/builtins/aspect/feature/gitlab_lint_comments.axl @@ -239,7 +239,7 @@ def _post_comments(ctx, gl, comments, existing_markers, task_key): body = c["body"], position = position, api_base = gl["api_base"], - auth_kind = "bearer", + auth_kind = gl["auth_kind"], ) if res["success"]: if marker: @@ -251,7 +251,7 @@ def _post_comments(ctx, gl, comments, existing_markers, task_key): def _get_existing_markers(ctx, gl, task_key): """Return `{marker: discussion_id}` for THIS task's existing lint discussions on the MR (dedup against prior runs).""" - listed = gitlab.discussions.list(ctx, token = gl["token"], project = gl["project_path"], mr_iid = gl["mr_iid"], api_base = gl["api_base"], auth_kind = "bearer") + listed = gitlab.discussions.list(ctx, token = gl["token"], project = gl["project_path"], mr_iid = gl["mr_iid"], api_base = gl["api_base"], auth_kind = gl["auth_kind"]) if not listed["success"]: return {} markers = {} @@ -460,7 +460,7 @@ def _cleanup_and_resolve(ctx, gl, relevant_diagnostics, run_id, task_key, sugges for m in suggestion_markers: desired[m] = True - listed = gitlab.discussions.list(ctx, token = gl["token"], project = gl["project_path"], mr_iid = gl["mr_iid"], api_base = gl["api_base"], auth_kind = "bearer") + listed = gitlab.discussions.list(ctx, token = gl["token"], project = gl["project_path"], mr_iid = gl["mr_iid"], api_base = gl["api_base"], auth_kind = gl["auth_kind"]) if not listed["success"]: return @@ -474,9 +474,9 @@ def _cleanup_and_resolve(ctx, gl, relevant_diagnostics, run_id, task_key, sugges genuinely_clean = not relevant_diagnostics and not suggestion_markers plan = compute_cleanup(listed["discussions"], desired, run_id, task_key, genuinely_clean = genuinely_clean) for e in plan["delete"]: - gitlab.discussions.delete(ctx, token = gl["token"], project = gl["project_path"], mr_iid = gl["mr_iid"], discussion_id = e["discussion_id"], note_id = e["note_id"], api_base = gl["api_base"], auth_kind = "bearer") + gitlab.discussions.delete(ctx, token = gl["token"], project = gl["project_path"], mr_iid = gl["mr_iid"], discussion_id = e["discussion_id"], note_id = e["note_id"], api_base = gl["api_base"], auth_kind = gl["auth_kind"]) for did in plan["resolve"]: - gitlab.discussions.resolve(ctx, token = gl["token"], project = gl["project_path"], mr_iid = gl["mr_iid"], discussion_id = did, resolved = True, api_base = gl["api_base"], auth_kind = "bearer") + gitlab.discussions.resolve(ctx, token = gl["token"], project = gl["project_path"], mr_iid = gl["mr_iid"], discussion_id = did, resolved = True, api_base = gl["api_base"], auth_kind = gl["auth_kind"]) def _gitlab_lint_impl(ctx: FeatureContext): lint_trait = ctx.traits[LintTrait] @@ -491,12 +491,13 @@ def _gitlab_lint_impl(ctx: FeatureContext): max_pr_comments = ctx.args.max_pr_comments auth_reason = {} - token = gitlab_token_cache.get_or_mint( + token, auth_kind = gitlab_token_cache.resolve_token( ctx, project_path = mr["project_path"], project_id = mr.get("project_id"), # See gitlab_status_comments.axl: request `api` directly until the # roles.ts mr_notes.write scope mapping is fixed (ENG-1652 follow-up). + # (Ignored on the explicit-token path, which doesn't mint.) roles = ["api"], _reason = auth_reason, ) @@ -508,7 +509,7 @@ def _gitlab_lint_impl(ctx: FeatureContext): # One call gets both the `diff_refs` SHA triple (for positions) and the # per-file diffs (for the new_line→old_line classification below). - mr_res = gitlab.merge_requests.list_changes(ctx, token = token, project = mr["project_path"], mr_iid = mr["mr_iid"], api_base = api_base, auth_kind = "bearer") + mr_res = gitlab.merge_requests.list_changes(ctx, token = token, project = mr["project_path"], mr_iid = mr["mr_iid"], api_base = api_base, auth_kind = auth_kind) if not mr_res["success"]: _LOG.warn(ctx.std, "Could not fetch MR %d changes: %s" % (mr["mr_iid"], mr_res.get("error", "unknown"))) return @@ -534,6 +535,7 @@ def _gitlab_lint_impl(ctx: FeatureContext): "project_path": mr["project_path"], "mr_iid": mr["mr_iid"], "api_base": api_base, + "auth_kind": auth_kind, "diff_refs": diff_refs, "changed_lines": {}, "prefix": "", diff --git a/crates/aspect-cli/src/builtins/aspect/feature/gitlab_status_comments.axl b/crates/aspect-cli/src/builtins/aspect/feature/gitlab_status_comments.axl index d34f1137d..fe5bd4589 100644 --- a/crates/aspect-cli/src/builtins/aspect/feature/gitlab_status_comments.axl +++ b/crates/aspect-cli/src/builtins/aspect/feature/gitlab_status_comments.axl @@ -165,7 +165,7 @@ def _gitlab_status_comments(ctx: FeatureContext): if _state.get("_gitlab_token"): return True auth_reason = {} - token = gitlab_token_cache.get_or_mint( + token, auth_kind = gitlab_token_cache.resolve_token( ctx, project_path = project_path, project_id = project_id, @@ -174,7 +174,8 @@ def _gitlab_status_comments(ctx: FeatureContext): # `gitlabScope: null` and `read_api`, which together resolve # to scope `read_api` only — and GitLab's notes API requires # `api` scope. Until roles.ts marks `mr_notes.write` as - # `api`-scoped, request `api` here directly. + # `api`-scoped, request `api` here directly. (Ignored on the + # explicit-token path, which doesn't mint.) roles = ["api"], _reason = auth_reason, ) @@ -185,6 +186,7 @@ def _gitlab_status_comments(ctx: FeatureContext): )) return False _state["_gitlab_token"] = token + _state["_auth_kind"] = auth_kind return True def _fetch_existing_note_body(ctx, token): @@ -201,7 +203,7 @@ def _gitlab_status_comments(ctx: FeatureContext): # The notes API doesn't expose a single-note GET that filters by id # uniquely without `list`; we approximate by listing and matching id. # Cheap on a small page; expensive only when the MR has many notes. - listed = gitlab.notes.list(ctx, token = token, project = project_path, mr_iid = mr_iid, api_base = api_base, auth_kind = "bearer") + listed = gitlab.notes.list(ctx, token = token, project = project_path, mr_iid = mr_iid, api_base = api_base, auth_kind = _state["_auth_kind"]) if not listed["success"]: _LOG.warn(ctx.std, "List notes failed: %s" % listed.get("error", "unknown")) return None @@ -212,7 +214,7 @@ def _gitlab_status_comments(ctx: FeatureContext): # Note vanished — fall through to re-discovery. _state.pop("_note_id", None) - listed = gitlab.notes.list(ctx, token = token, project = project_path, mr_iid = mr_iid, api_base = api_base, auth_kind = "bearer") + listed = gitlab.notes.list(ctx, token = token, project = project_path, mr_iid = mr_iid, api_base = api_base, auth_kind = _state["_auth_kind"]) if not listed["success"]: _LOG.warn(ctx.std, "List notes failed: %s" % listed.get("error", "unknown")) return None @@ -236,7 +238,7 @@ def _gitlab_status_comments(ctx: FeatureContext): note_id = _state["_note_id"], body = body, api_base = api_base, - auth_kind = "bearer", + auth_kind = _state["_auth_kind"], ) if not res["success"]: _LOG.warn(ctx.std, "Update note failed: %s" % res.get("error", "unknown")) @@ -250,7 +252,7 @@ def _gitlab_status_comments(ctx: FeatureContext): mr_iid = mr_iid, body = body, api_base = api_base, - auth_kind = "bearer", + auth_kind = _state["_auth_kind"], ) if not res["success"]: _LOG.warn(ctx.std, "Create note failed: %s" % res.get("error", "unknown")) @@ -260,7 +262,7 @@ def _gitlab_status_comments(ctx: FeatureContext): # Race mitigation: if two tasks both POSTed within the same # window, keep the lowest note id and delete the rest. Stable # choice across writers so they converge on the same survivor. - listed2 = gitlab.notes.list(ctx, token = token, project = project_path, mr_iid = mr_iid, api_base = api_base, auth_kind = "bearer") + listed2 = gitlab.notes.list(ctx, token = token, project = project_path, mr_iid = mr_iid, api_base = api_base, auth_kind = _state["_auth_kind"]) if not listed2["success"]: return True dupes = [n for n in listed2["notes"] if marker in (n.get("body") or "")] @@ -277,7 +279,7 @@ def _gitlab_status_comments(ctx: FeatureContext): mr_iid = mr_iid, note_id = d["id"], api_base = api_base, - auth_kind = "bearer", + auth_kind = _state["_auth_kind"], ) if not del_res["success"]: _LOG.trace("Cleanup of duplicate note %s failed; will retry next cycle" % d["id"]) diff --git a/crates/aspect-cli/src/builtins/aspect/lib/gitlab_test.axl b/crates/aspect-cli/src/builtins/aspect/lib/gitlab_test.axl index 6cf0cd32a..cb3599b6c 100644 --- a/crates/aspect-cli/src/builtins/aspect/lib/gitlab_test.axl +++ b/crates/aspect-cli/src/builtins/aspect/lib/gitlab_test.axl @@ -11,6 +11,7 @@ Run with: """ load("./gitlab.axl", "gitlab") +load("./gitlab_token_cache.axl", "gitlab_token_cache") def _eq(label, got, want): if got != want: @@ -153,6 +154,40 @@ def _test_impl(ctx): "https://self-hosted.gitlab.com/api/v4", ) + # resolve_token — the explicit-token escape hatch (self-hosted path). + # ASPECT_GITLAB_TOKEN short-circuits before any mint and selects the + # PRIVATE-TOKEN header convention; absent it, the mint path is used + # and auth_kind is "bearer" (and None on mint failure). + def _ctx_with_env(env_vars): + return struct( + std = struct( + env = struct( + var = lambda name: env_vars.get(name, ""), + temp_dir = lambda: "/tmp", + ), + fs = struct(exists = lambda path: False), + process = struct(id = lambda: 1234), + ), + aspect = struct(auth = struct(credentials = lambda profile = None: None)), + ) + + tok, kind = gitlab_token_cache.resolve_token( + _ctx_with_env({"ASPECT_GITLAB_TOKEN": " glpat-xxx "}), + project_path = "group/project", + ) + _eq("resolve_token / explicit token value (trimmed)", tok, "glpat-xxx") + _eq("resolve_token / explicit token uses private_token", kind, "private_token") + + # No explicit token + no creds → mint path fails → (None, "bearer"). + reason = {} + tok, kind = gitlab_token_cache.resolve_token( + _ctx_with_env({"CI": "true"}), + project_path = "group/project", + _reason = reason, + ) + _eq("resolve_token / fallback no-creds returns None", tok, None) + _eq("resolve_token / fallback auth_kind is bearer", kind, "bearer") + # commit_status.STATES exposed if "success" not in gitlab.commit_status.STATES: fail("commit_status.STATES missing 'success'") diff --git a/crates/aspect-cli/src/builtins/aspect/lib/gitlab_token_cache.axl b/crates/aspect-cli/src/builtins/aspect/lib/gitlab_token_cache.axl index 85e521a6a..c05ace41e 100644 --- a/crates/aspect-cli/src/builtins/aspect/lib/gitlab_token_cache.axl +++ b/crates/aspect-cli/src/builtins/aspect/lib/gitlab_token_cache.axl @@ -102,6 +102,45 @@ def get_or_mint(ctx, project_path, project_id = None, profile = None, roles = No ctx.std.fs.write(path, tok) return tok +# Explicit-token escape hatch. Set to a GitLab personal / group / project +# access token with `api` scope to bypass the Aspect GitLab App entirely. +_EXPLICIT_TOKEN_ENV = "ASPECT_GITLAB_TOKEN" + +def resolve_token(ctx, project_path, project_id = None, profile = None, roles = None, _reason = None): + """Resolve a GitLab token plus the auth header convention to use, + returning `(token, auth_kind)`. + + Precedence: + + * `ASPECT_GITLAB_TOKEN` (a personal / group / project access token) + takes precedence when set and is sent as `PRIVATE-TOKEN` + (`auth_kind = "private_token"`). This is the path for self-hosted + GitLab: the Aspect GitLab App is registered on `gitlab.com`, so a + minted token can't authenticate against another instance — the + caller must supply a token valid for their own host. No minting, + no per-process caching (a PAT has no refresh-rotation hazard). + * Otherwise mint via the Aspect GitLab App through `get_or_mint` + and send as a bearer token (`auth_kind = "bearer"`), with the + per-process caching described above. + + Returns `(None, "bearer")` on mint failure, with `_reason` populated + exactly as `gitlab.authenticate` documents. The explicit-token path + never fails here — an invalid token surfaces as a 401 at the first + API call, handled by the feature's existing warn-and-skip path.""" + explicit = ctx.std.env.var(_EXPLICIT_TOKEN_ENV) or "" + if explicit: + return explicit.strip(), "private_token" + tok = get_or_mint( + ctx, + project_path = project_path, + project_id = project_id, + profile = profile, + roles = roles, + _reason = _reason, + ) + return tok, "bearer" + gitlab_token_cache = struct( get_or_mint = get_or_mint, + resolve_token = resolve_token, )