From e1715863ce0d65e1782fced8ab1f43581387e327 Mon Sep 17 00:00:00 2001 From: Torben Buck Date: Sat, 24 Oct 2026 13:34:32 +0200 Subject: [PATCH 1/2] fix(gitlab): render private GitLab uploads in merge requests GitLab writes uploads into merge request bodies and comments as project-relative `/uploads//` links, which the clients treated as local paths, and a private project serves them only to a signed-in request. The GitLab definition now resolves those links against the merge request's project (a new `resolveRelativeLinks` hook, applied by web and mobile before the body is split) and claims upload URLs through `mediaUrl`. The GitLab driver's `fetchMedia` reads them through the upload API (GitLab 17.4+) with the token `glab` stores for that host, and falls back to the upload itself without a login, which still serves a public project. --- .../pull-requests/PullRequestMarkdown.tsx | 30 +++++- .../pullRequest/PullRequestDetailPanel.tsx | 11 +- .../pullRequest/PullRequestMarkdown.tsx | 3 +- docs/user/source-control.md | 3 + .../src/pullRequestMarkdown.test.ts | 26 ++++- .../client-runtime/src/pullRequestMarkdown.ts | 16 +++ .../src/shared/definition.ts | 5 + .../src/server/GitLabCredentials.test.ts | 90 ++++++++++++++++ .../src/server/GitLabCredentials.ts | 88 +++++++++++++++ .../src/server/driver.ts | 8 +- .../src/server/gitLabMediaFetch.test.ts | 90 ++++++++++++++++ .../src/server/gitLabMediaFetch.ts | 75 +++++++++++++ .../src/shared/definition.ts | 3 + .../src/shared/gitLabMedia.test.ts | 96 +++++++++++++++++ .../src/shared/gitLabMedia.ts | 102 ++++++++++++++++++ 15 files changed, 637 insertions(+), 9 deletions(-) create mode 100644 packages/source-control-gitlab/src/server/GitLabCredentials.test.ts create mode 100644 packages/source-control-gitlab/src/server/GitLabCredentials.ts create mode 100644 packages/source-control-gitlab/src/server/gitLabMediaFetch.test.ts create mode 100644 packages/source-control-gitlab/src/server/gitLabMediaFetch.ts create mode 100644 packages/source-control-gitlab/src/shared/gitLabMedia.test.ts create mode 100644 packages/source-control-gitlab/src/shared/gitLabMedia.ts diff --git a/apps/mobile/src/features/pull-requests/PullRequestMarkdown.tsx b/apps/mobile/src/features/pull-requests/PullRequestMarkdown.tsx index 63946016d591f..e942af7551c7c 100644 --- a/apps/mobile/src/features/pull-requests/PullRequestMarkdown.tsx +++ b/apps/mobile/src/features/pull-requests/PullRequestMarkdown.tsx @@ -10,7 +10,10 @@ import { } from "./PullRequestMedia"; import { videoMimeType } from "@t3tools/shared/video"; import { createContext, useContext, useMemo, useState } from "react"; -import { changeRequestReferenceAutolinker } from "@t3tools/client-runtime/pull-request-markdown"; +import { + changeRequestLinkResolver, + changeRequestReferenceAutolinker, +} from "@t3tools/client-runtime/pull-request-markdown"; import type { ReferenceAutolink } from "@t3tools/client-runtime/source-control-clients"; import { linkReferenceAutolinks } from "./pullRequestAutolinks"; @@ -31,6 +34,7 @@ import type { MarkdownImageRenderer } from "../../native/SelectableMarkdownText" const AutolinksContext = createContext<((text: string) => ReadonlyArray) | null>( null, ); +const RelativeLinksContext = createContext<((markdown: string) => string) | null>(null); export function PullRequestMarkdownRepository(props: { /** The change request the markdown belongs to, whose host decides what its text links. */ @@ -44,6 +48,10 @@ export function PullRequestMarkdownRepository(props: { provider === undefined || url === undefined ? null : changeRequestReferenceAutolinker({ provider, url }); + const resolveLinks = + provider === undefined || url === undefined + ? null + : changeRequestLinkResolver({ provider, url }); const target = pullRequestTarget(props.params); const project = useProject({ environmentId: target.environmentId, @@ -53,7 +61,9 @@ export function PullRequestMarkdownRepository(props: { const media = cwd ? { environmentId: target.environmentId, cwd } : null; return ( - {props.children} + + {props.children} + ); } @@ -89,7 +99,19 @@ export function PullRequestMarkdown(props: { readonly markdown: string; readonly imageGroupMarkdown?: string; }) { - const segments = useMemo(() => splitPullRequestBody(props.markdown), [props.markdown]); + const resolveLinks = useContext(RelativeLinksContext); + const markdown = useMemo( + () => resolveLinks?.(props.markdown) ?? props.markdown, + [resolveLinks, props.markdown], + ); + const imageGroupMarkdown = useMemo( + () => + props.imageGroupMarkdown === undefined + ? markdown + : (resolveLinks?.(props.imageGroupMarkdown) ?? props.imageGroupMarkdown), + [resolveLinks, props.imageGroupMarkdown, markdown], + ); + const segments = useMemo(() => splitPullRequestBody(markdown), [markdown]); return ( {segments.map((segment) => @@ -97,7 +119,7 @@ export function PullRequestMarkdown(props: { ) : ( diff --git a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx index 135ae2970ceb9..1f5d05aff9b26 100644 --- a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx +++ b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx @@ -1,6 +1,9 @@ import { projectChangeRequestUrl } from "@t3tools/client-runtime/change-request-links"; import { normalizeThreadPullRequestKey } from "@t3tools/client-runtime/thread-pull-requests"; -import { changeRequestReferenceAutolinker } from "@t3tools/client-runtime/pull-request-markdown"; +import { + changeRequestLinkResolver, + changeRequestReferenceAutolinker, +} from "@t3tools/client-runtime/pull-request-markdown"; import { useAtomValue } from "@effect/atom-react"; import { usePullRequestStack } from "~/state/usePullRequestStack"; import { @@ -239,7 +242,11 @@ function pullRequestMarkdownContext( detail: Pick | null, threadRef: ScopedThreadRef | null, ) { - return { autolinks: changeRequestReferenceAutolinker(detail), threadRef }; + return { + autolinks: changeRequestReferenceAutolinker(detail), + resolveLinks: changeRequestLinkResolver(detail), + threadRef, + }; } function pullRequestStackReference( diff --git a/apps/web/src/components/pullRequest/PullRequestMarkdown.tsx b/apps/web/src/components/pullRequest/PullRequestMarkdown.tsx index 7086d5441bc5a..6a1e4fcdd060d 100644 --- a/apps/web/src/components/pullRequest/PullRequestMarkdown.tsx +++ b/apps/web/src/components/pullRequest/PullRequestMarkdown.tsx @@ -23,6 +23,7 @@ import { remarkPullRequestAutolinks, splitPullRequestBody } from "./pullRequestM export const PullRequestMarkdownContext = createContext<{ autolinks: ((text: string) => ReadonlyArray) | null; + resolveLinks: ((markdown: string) => string) | null; threadRef: ScopedThreadRef | null; } | null>(null); @@ -67,8 +68,8 @@ export function PullRequestMarkdown({ threadRef?: ScopedThreadRef | null; className?: string; }) { - const segments = splitPullRequestBody(text); const context = useContext(PullRequestMarkdownContext); + const segments = splitPullRequestBody(context?.resolveLinks?.(text) ?? text); const autolinks = context?.autolinks; const resolvedThreadRef = threadRef ?? context?.threadRef ?? undefined; const extraRemarkPlugins: NonNullable = autolinks diff --git a/docs/user/source-control.md b/docs/user/source-control.md index 815e5dd0668fa..383860027e231 100644 --- a/docs/user/source-control.md +++ b/docs/user/source-control.md @@ -62,6 +62,9 @@ Install [GitLab CLI](https://gitlab.com/gitlab-org/cli), then sign in: glab auth login ``` +Images and videos uploaded to a private project's merge requests load through this login, which +needs GitLab 17.4 or later. + ### Bitbucket Open **Settings → Source control**, expand **Bitbucket**, and enter one of: diff --git a/packages/client-runtime/src/pullRequestMarkdown.test.ts b/packages/client-runtime/src/pullRequestMarkdown.test.ts index 3395d78313b94..53f211f882f59 100644 --- a/packages/client-runtime/src/pullRequestMarkdown.test.ts +++ b/packages/client-runtime/src/pullRequestMarkdown.test.ts @@ -1,6 +1,10 @@ import { describe, expect, it } from "vite-plus/test"; -import { changeRequestReferenceAutolinker, splitPullRequestBody } from "./pullRequestMarkdown.ts"; +import { + changeRequestLinkResolver, + changeRequestReferenceAutolinker, + splitPullRequestBody, +} from "./pullRequestMarkdown.ts"; describe("pull request body segmentation", () => { // GitHub writes a dropped image into the body as an `` tag, so a bare attachment link on @@ -140,3 +144,23 @@ describe("change request reference autolinks", () => { expect(urls("forgejo", "https://codeberg.org/acme/web/pulls/7", "#5")).toBeNull(); }); }); + +describe("change request relative links", () => { + const upload = "/uploads/66dbcd21ec5d24ed6ea225176098d52b/shot.png"; + + it("resolves a GitLab upload against the merge request's project", () => { + const resolve = changeRequestLinkResolver({ + provider: "gitlab", + url: "https://gitlab.example.com/group/Sub/app/-/merge_requests/7", + }); + expect(resolve?.(`![shot](${upload})`)).toBe( + `![shot](https://gitlab.example.com/group/Sub/app${upload})`, + ); + }); + + it("leaves bodies alone on a host that writes no relative links", () => { + expect( + changeRequestLinkResolver({ provider: "github", url: "https://github.com/acme/web/pull/7" }), + ).toBeNull(); + }); +}); diff --git a/packages/client-runtime/src/pullRequestMarkdown.ts b/packages/client-runtime/src/pullRequestMarkdown.ts index 1e03c32509e9a..95f3a7f722e20 100644 --- a/packages/client-runtime/src/pullRequestMarkdown.ts +++ b/packages/client-runtime/src/pullRequestMarkdown.ts @@ -147,3 +147,19 @@ export function changeRequestReferenceAutolinker( ? null : (text) => find(text, repositoryUrl); } + +/** + * Makes the links the change request's host writes relative to its repository absolute, so they + * load from the host. Null when the host writes none. + */ +export function changeRequestLinkResolver( + changeRequest: { readonly provider: string; readonly url: string } | null | undefined, +): ((markdown: string) => string) | null { + if (!changeRequest) return null; + const resolve = sourceControlClients.get(changeRequest.provider).resolveRelativeLinks; + const repositoryUrl = sourceControlClients.parseChangeRequestUrl(changeRequest.url)?.link + .repositoryUrl; + return resolve === undefined || repositoryUrl === undefined + ? null + : (markdown) => resolve(markdown, repositoryUrl); +} diff --git a/packages/source-control-core/src/shared/definition.ts b/packages/source-control-core/src/shared/definition.ts index 6f99525260129..b115793b67049 100644 --- a/packages/source-control-core/src/shared/definition.ts +++ b/packages/source-control-core/src/shared/definition.ts @@ -272,6 +272,11 @@ export interface SourceControlDefinition { text: string, repositoryUrl: string, ) => ReadonlyArray; + /** + * `markdown` with the links this host writes relative to the repository made absolute against + * `repositoryUrl`. Absent for a host whose bodies carry none. + */ + readonly resolveRelativeLinks?: (markdown: string, repositoryUrl: string) => string; /** Where the host lists a change request's checks. Absent means the change request's own page. */ readonly changeRequestChecksUrl?: (changeRequestUrl: string) => string; /** diff --git a/packages/source-control-gitlab/src/server/GitLabCredentials.test.ts b/packages/source-control-gitlab/src/server/GitLabCredentials.test.ts new file mode 100644 index 0000000000000..898dfb38c91e1 --- /dev/null +++ b/packages/source-control-gitlab/src/server/GitLabCredentials.test.ts @@ -0,0 +1,90 @@ +import { describe, expect, it } from "@effect/vitest"; +import * as Effect from "effect/Effect"; +import * as Layer from "effect/Layer"; +import * as Option from "effect/Option"; +import * as Redacted from "effect/Redacted"; + +import type * as SourceControlHost from "@t3tools/source-control-core/server/SourceControlHost"; +import * as TestSourceControlHost from "@t3tools/source-control-testing/TestSourceControlHost"; +import * as GitLabCredentials from "./GitLabCredentials.ts"; + +/** A scripted glab: `config get --host ` reads `config`, `api` refreshes. */ +function harness(config: Record>, refreshed?: string) { + const runs: Array = []; + const run: SourceControlHost.SourceControlHost["Service"]["process"]["run"] = (input) => + Effect.sync(() => { + runs.push(input); + const host = input.args[input.args.indexOf("--host") + 1] ?? ""; + if (input.args[0] === "api") { + if (refreshed !== undefined) config[input.args[2]!]!.token = refreshed; + return TestSourceControlHost.processOutput("{}"); + } + return TestSourceControlHost.processOutput(`${config[host]?.[input.args[2]!] ?? ""}\n`); + }); + const layer = GitLabCredentials.layer.pipe( + Layer.provide(TestSourceControlHost.layer({ process: { run } })), + ); + return { layer, runs }; +} + +const token = (host: string) => + GitLabCredentials.GitLabCredentials.pipe( + Effect.flatMap((credentials) => credentials.get(host)), + Effect.map(Option.map(Redacted.value)), + ); + +describe("GitLabCredentials", () => { + it.effect("reads only the token glab stores for the host", () => { + const { layer, runs } = harness({ "gitlab.example.com": { token: "stored" } }); + return Effect.gen(function* () { + expect(yield* token("GitLab.Example.com")).toEqual(Option.some("stored")); + expect(runs.map((input) => input.args)).toEqual([ + ["config", "get", "token", "--host", "gitlab.example.com"], + ["config", "get", "is_oauth2", "--host", "gitlab.example.com"], + ["config", "get", "token", "--host", "gitlab.example.com"], + ]); + // glab would otherwise answer every host with an ambient token. + for (const input of runs) { + expect(input.env).toMatchObject({ GITLAB_TOKEN: "", OAUTH_TOKEN: "", CI_JOB_TOKEN: "" }); + } + // Cached: a second read asks glab nothing. + yield* token("gitlab.example.com"); + expect(runs).toHaveLength(3); + }).pipe(Effect.provide(layer)); + }); + + it.effect("has nothing for a host glab holds no login for", () => { + const { layer, runs } = harness({}); + return Effect.gen(function* () { + expect(yield* token("attacker.example")).toEqual(Option.none()); + expect(runs).toHaveLength(1); + }).pipe(Effect.provide(layer)); + }); + + it.effect("refreshes an OAuth login through an API call before reading it", () => { + const { layer, runs } = harness( + { "gitlab.com": { token: "expired", is_oauth2: "true" } }, + "refreshed", + ); + return Effect.gen(function* () { + expect(yield* token("gitlab.com")).toEqual(Option.some("refreshed")); + expect(runs.map((input) => input.args)).toContainEqual([ + "api", + "--hostname", + "gitlab.com", + "user", + ]); + }).pipe(Effect.provide(layer)); + }); + + it.effect("asks glab again after a refused token is invalidated", () => { + const { layer, runs } = harness({ "gitlab.com": { token: "stored" } }); + return Effect.gen(function* () { + const credentials = yield* GitLabCredentials.GitLabCredentials; + yield* credentials.get("gitlab.com"); + yield* credentials.invalidate("gitlab.com"); + yield* credentials.get("gitlab.com"); + expect(runs).toHaveLength(6); + }).pipe(Effect.provide(layer)); + }); +}); diff --git a/packages/source-control-gitlab/src/server/GitLabCredentials.ts b/packages/source-control-gitlab/src/server/GitLabCredentials.ts new file mode 100644 index 0000000000000..541b087d1301d --- /dev/null +++ b/packages/source-control-gitlab/src/server/GitLabCredentials.ts @@ -0,0 +1,88 @@ +import * as Cache from "effect/Cache"; +import * as Context from "effect/Context"; +import * as Duration from "effect/Duration"; +import * as Effect from "effect/Effect"; +import * as Exit from "effect/Exit"; +import * as Layer from "effect/Layer"; +import * as Option from "effect/Option"; +import * as Redacted from "effect/Redacted"; + +import * as HostProcess from "@t3tools/shared/HostProcess"; +import * as SourceControlHost from "@t3tools/source-control-core/server/SourceControlHost"; + +/** How long a token is reused before `glab` is asked again. */ +const TOKEN_TTL = Duration.minutes(5); +/** A missing login is asked again soon, so a fresh `glab auth login` applies. */ +const MISSING_TTL = Duration.seconds(10); + +/** + * glab hands an environment token to whichever host it is asked about. The host here comes from a + * URL someone wrote into a merge request, so only the token stored for that host may answer. + */ +const STORED_ONLY_ENV = { + GITLAB_TOKEN: "", + GITLAB_ACCESS_TOKEN: "", + OAUTH_TOKEN: "", + CI_JOB_TOKEN: "", + GLAB_ENABLE_CI_AUTOLOGIN: "false", +}; + +/** The token `glab` stores for a GitLab host, used to read what that host serves to the user. */ +export class GitLabCredentials extends Context.Service< + GitLabCredentials, + { + /** None when `glab` holds no login for the host. */ + readonly get: (host: string) => Effect.Effect>>; + /** Drops the held token after GitLab refused it, so the next read asks `glab` again. */ + readonly invalidate: (host: string) => Effect.Effect; + } +>()("@t3tools/source-control-gitlab/server/GitLabCredentials") {} + +/** @public */ +export const make = Effect.gen(function* () { + const { process } = yield* SourceControlHost.SourceControlHost; + const workingDirectory = yield* HostProcess.WorkingDirectory; + + const glab = (args: ReadonlyArray) => + process + .run({ + operation: "GitLabCredentials.get", + command: "glab", + args, + cwd: workingDirectory, + env: STORED_ONLY_ENV, + timeoutMs: 10_000, + maxOutputBytes: 64 * 1024, + }) + .pipe(Effect.map((output) => output.stdout.trim())); + + const storedToken = (host: string) => glab(["config", "get", "token", "--host", host]); + + const lookup = Effect.fn("GitLabCredentials.lookup")(function* (host: string) { + if ((yield* storedToken(host)) === "") return Option.none(); + // An OAuth login expires within hours and glab refreshes it only on an API call. + if ((yield* glab(["config", "get", "is_oauth2", "--host", host])) === "true") { + yield* glab(["api", "--hostname", host, "user"]).pipe(Effect.ignore); + } + const token = yield* storedToken(host); + return token === "" ? Option.none() : Option.some(Redacted.make(token)); + }); + + const cache = yield* Cache.makeWith( + (host: string) => lookup(host).pipe(Effect.orElseSucceed(() => Option.none())), + { + capacity: 32, + timeToLive: (exit) => + Exit.isSuccess(exit) && Option.isSome(exit.value) ? TOKEN_TTL : MISSING_TTL, + }, + ); + + return GitLabCredentials.of({ + get: Effect.fn("GitLabCredentials.get")((host) => Cache.get(cache, host.trim().toLowerCase())), + invalidate: Effect.fn("GitLabCredentials.invalidate")((host) => + Cache.invalidate(cache, host.trim().toLowerCase()), + ), + }); +}); + +export const layer = Layer.effect(GitLabCredentials, make); diff --git a/packages/source-control-gitlab/src/server/driver.ts b/packages/source-control-gitlab/src/server/driver.ts index 1773bfd4bdae7..67694e87c32d0 100644 --- a/packages/source-control-gitlab/src/server/driver.ts +++ b/packages/source-control-gitlab/src/server/driver.ts @@ -8,16 +8,22 @@ import * as Layer from "effect/Layer"; import { definition } from "../shared/definition.ts"; import * as GitLabCli from "./GitLabCli.ts"; +import * as GitLabCredentials from "./GitLabCredentials.ts"; +import * as GitLabMediaFetch from "./gitLabMediaFetch.ts"; import * as GitLabPullRequestCli from "./GitLabPullRequestCli.ts"; import * as GitLabPullRequestProvider from "./GitLabPullRequestProvider.ts"; import * as GitLabSourceControlProvider from "./GitLabSourceControlProvider.ts"; -const services = GitLabPullRequestCli.layer.pipe(Layer.provideMerge(GitLabCli.layer)); +const services = Layer.merge( + GitLabPullRequestCli.layer.pipe(Layer.provideMerge(GitLabCli.layer)), + GitLabCredentials.layer, +); const makeInstance = Effect.all({ sourceControl: GitLabSourceControlProvider.make, discovery: Effect.succeed(GitLabSourceControlProvider.discovery), pullRequests: GitLabPullRequestProvider.make, + fetchMedia: GitLabMediaFetch.make, }); export const driver = defineSourceControlDriver({ diff --git a/packages/source-control-gitlab/src/server/gitLabMediaFetch.test.ts b/packages/source-control-gitlab/src/server/gitLabMediaFetch.test.ts new file mode 100644 index 0000000000000..2b2cd3f3c05b2 --- /dev/null +++ b/packages/source-control-gitlab/src/server/gitLabMediaFetch.test.ts @@ -0,0 +1,90 @@ +import { describe, expect, it } from "@effect/vitest"; +import * as Effect from "effect/Effect"; +import * as Layer from "effect/Layer"; +import * as Option from "effect/Option"; +import * as Redacted from "effect/Redacted"; +import { HttpClient, HttpClientResponse } from "effect/http"; + +import * as GitLabCredentials from "./GitLabCredentials.ts"; +import * as GitLabMediaFetch from "./gitLabMediaFetch.ts"; + +const SECRET = "66dbcd21ec5d24ed6ea225176098d52b"; +const UPLOAD = `https://gitlab.example.com/group/app/uploads/${SECRET}/shot.png`; +const API = `https://gitlab.example.com/api/v4/projects/group%2Fapp/uploads/${SECRET}/shot.png`; + +/** A GitLab that answers `respond`, with `token` as the stored `glab` login. */ +function fetchWith( + token: string | null, + respond: (url: string, authorization: string | undefined) => Response, +) { + const requests: Array<[string, string | undefined]> = []; + const invalidated: Array = []; + const layer = Layer.merge( + Layer.mock(GitLabCredentials.GitLabCredentials)({ + get: () => Effect.succeed(Option.fromNullishOr(token).pipe(Option.map(Redacted.make))), + invalidate: (host) => Effect.sync(() => void invalidated.push(host)), + }), + Layer.succeed( + HttpClient.HttpClient, + HttpClient.make((request) => { + requests.push([request.url, request.headers.authorization]); + return Effect.succeed( + HttpClientResponse.fromWeb(request, respond(request.url, request.headers.authorization)), + ); + }), + ), + ); + const status = Effect.gen(function* () { + const fetchMedia = yield* GitLabMediaFetch.make; + return (yield* fetchMedia({ url: UPLOAD, headers: {} }))?.status; + }).pipe(Effect.provide(layer), Effect.scoped); + return { status, requests, invalidated }; +} + +describe("GitLabMediaFetch", () => { + it.effect("reads the upload through the API with the glab login for its host", () => { + const { status, requests } = fetchWith("stored", () => new Response(null, { status: 200 })); + return Effect.gen(function* () { + expect(yield* status).toBe(200); + expect(requests).toEqual([[API, "Bearer stored"]]); + }); + }); + + it.effect("keeps the token off the object store a redirect hands the upload to", () => { + const { status, requests } = fetchWith("stored", (url) => + url === API + ? new Response(null, { status: 302, headers: { location: "https://objects.example/s" } }) + : new Response(null, { status: 200 }), + ); + return Effect.gen(function* () { + expect(yield* status).toBe(200); + expect(requests).toEqual([ + [API, "Bearer stored"], + ["https://objects.example/s", undefined], + ]); + }); + }); + + it.effect("loads the upload itself without a login, as a public project serves it", () => { + const { status, requests } = fetchWith(null, () => new Response(null, { status: 200 })); + return Effect.gen(function* () { + expect(yield* status).toBe(200); + expect(requests).toEqual([[UPLOAD, undefined]]); + }); + }); + + it.effect("drops a refused token and falls back to the upload itself", () => { + const { status, requests, invalidated } = fetchWith( + "revoked", + (url) => new Response(null, { status: url === API ? 401 : 200 }), + ); + return Effect.gen(function* () { + expect(yield* status).toBe(200); + expect(requests).toEqual([ + [API, "Bearer revoked"], + [UPLOAD, undefined], + ]); + expect(invalidated).toEqual(["gitlab.example.com"]); + }); + }); +}); diff --git a/packages/source-control-gitlab/src/server/gitLabMediaFetch.ts b/packages/source-control-gitlab/src/server/gitLabMediaFetch.ts new file mode 100644 index 0000000000000..71638b286f36b --- /dev/null +++ b/packages/source-control-gitlab/src/server/gitLabMediaFetch.ts @@ -0,0 +1,75 @@ +import type { SourceControlMediaRequest } from "@t3tools/source-control-core/server/driver"; +import * as Effect from "effect/Effect"; +import * as Option from "effect/Option"; +import * as Redacted from "effect/Redacted"; +import { + FetchHttpClient, + HttpClient, + HttpClientRequest, + type HttpClientResponse, +} from "effect/http"; + +import { gitLabUploadApiUrl, parseGitLabUpload } from "../shared/gitLabMedia.ts"; + +import * as GitLabCredentials from "./GitLabCredentials.ts"; + +const MAX_REDIRECTS = 3; +const MANUAL_REDIRECT: RequestInit = { redirect: "manual" }; + +export const make = Effect.gen(function* () { + const credentials = yield* GitLabCredentials.GitLabCredentials; + const httpClient = HttpClient.withScope(yield* HttpClient.HttpClient); + + /** Follows redirects by hand so `authorization` reaches only `origin`, not object storage. */ + const fetchFollowingRedirects = Effect.fnUntraced(function* ( + url: string, + headers: Readonly>, + credential: { readonly origin: string; readonly authorization: string } | null, + ) { + let target = url; + for (let hop = 0; ; hop += 1) { + const authorization = + credential !== null && new URL(target).origin === credential.origin + ? credential.authorization + : null; + const response: HttpClientResponse.HttpClientResponse = yield* httpClient + .execute( + HttpClientRequest.get(target).pipe( + HttpClientRequest.setHeaders({ + ...headers, + // The bytes are streamed straight through, so never let an encoding layer in. + "accept-encoding": "identity", + ...(authorization === null ? {} : { authorization }), + }), + ), + ) + .pipe(Effect.provideService(FetchHttpClient.RequestInit, MANUAL_REDIRECT)); + const location = response.headers.location; + if (response.status < 300 || response.status >= 400) return response; + if (!location || hop >= MAX_REDIRECTS) return null; + const next = new URL(location, target); + if (next.protocol !== "https:") return null; + target = next.toString(); + } + }); + + /** + * Reads the upload through the API with the `glab` login for its host. Without a login, or + * where the API refuses it (GitLab before 17.4), the upload itself still serves a public project. + */ + return Effect.fn("GitLabMediaFetch.fetchMedia")(function* (request: SourceControlMediaRequest) { + const url = new URL(request.url); + const upload = parseGitLabUpload(url); + if (upload === null) return null; + const token = yield* credentials.get(url.host); + if (Option.isSome(token)) { + const response = yield* fetchFollowingRedirects(gitLabUploadApiUrl(upload), request.headers, { + origin: upload.origin, + authorization: `Bearer ${Redacted.value(token.value)}`, + }); + if (response !== null && response.status < 400) return response; + if (response?.status === 401) yield* credentials.invalidate(url.host); + } + return yield* fetchFollowingRedirects(request.url, request.headers, null); + }); +}); diff --git a/packages/source-control-gitlab/src/shared/definition.ts b/packages/source-control-gitlab/src/shared/definition.ts index 437320c0d0419..f5efba02b51a5 100644 --- a/packages/source-control-gitlab/src/shared/definition.ts +++ b/packages/source-control-gitlab/src/shared/definition.ts @@ -7,6 +7,7 @@ import { parseChangeRequestPath, projectChangeRequestHost, } from "@t3tools/source-control-core/shared/definition"; +import { gitLabMediaUrl, resolveGitLabUploadLinks } from "./gitLabMedia.ts"; import { icon } from "./icon.ts"; const CHECKOUT_COMMAND = /^glab\s+mr\s+checkout\s+(.+)$/i; @@ -72,4 +73,6 @@ export const definition = defineSourceControl({ isChangeRequestInProjectRepository(projectChangeRequestHost(identity, KIND), identity, link), canReadChangeRequestOnHost: (identity, link) => isChangeRequestOnProjectHost(projectChangeRequestHost(identity, KIND), identity, link), + resolveRelativeLinks: resolveGitLabUploadLinks, + mediaUrl: gitLabMediaUrl, }); diff --git a/packages/source-control-gitlab/src/shared/gitLabMedia.test.ts b/packages/source-control-gitlab/src/shared/gitLabMedia.test.ts new file mode 100644 index 0000000000000..2b2126c4ed6c6 --- /dev/null +++ b/packages/source-control-gitlab/src/shared/gitLabMedia.test.ts @@ -0,0 +1,96 @@ +import { describe, expect, it } from "vite-plus/test"; + +import { + gitLabMediaUrl, + gitLabUploadApiUrl, + parseGitLabUpload, + resolveGitLabUploadLinks, +} from "./gitLabMedia.ts"; + +const SECRET = "66dbcd21ec5d24ed6ea225176098d52b"; + +const media = (url: string) => gitLabMediaUrl(new URL(url)); + +describe("gitLabMedia", () => { + it("maps project-path and project-id upload URLs to the upload API", () => { + const byPath = parseGitLabUpload( + new URL(`https://gitlab.example.com/group/sub/project/uploads/${SECRET}/my%20shot.png`), + ); + expect(byPath && gitLabUploadApiUrl(byPath)).toBe( + `https://gitlab.example.com/api/v4/projects/group%2Fsub%2Fproject/uploads/${SECRET}/my%20shot.png`, + ); + const byId = parseGitLabUpload( + new URL(`https://gitlab.example.com:8443/-/project/42/uploads/${SECRET}/demo.mp4`), + ); + expect(byId && gitLabUploadApiUrl(byId)).toBe( + `https://gitlab.example.com:8443/api/v4/projects/42/uploads/${SECRET}/demo.mp4`, + ); + }); + + it("drops query and fragment from the canonical URL", () => { + expect(media(`https://gitlab.com/team/app/uploads/${SECRET}/demo.mp4?x=1#t=2`)).toEqual({ + fetchUrl: `https://gitlab.com/team/app/uploads/${SECRET}/demo.mp4`, + fileName: "demo.mp4", + }); + }); + + it("rejects anything that is not a project upload over https", () => { + for (const url of [ + `http://gitlab.com/team/app/uploads/${SECRET}/a.png`, + `https://user:pw@gitlab.com/team/app/uploads/${SECRET}/a.png`, + `https://gitlab.com/uploads/${SECRET}/a.png`, + `https://gitlab.com/team/app/-/uploads/${SECRET}/a.png`, + `https://gitlab.com/uploads/-/system/user/avatar/1/avatar.png`, + `https://gitlab.com/team/app/uploads/not-a-secret/a.png`, + ]) { + expect(media(url), url).toBeNull(); + } + }); + + it("names the file from its decoded last segment", () => { + expect(media(`https://gitlab.com/a/b/uploads/${SECRET}/my%20shot.png`)?.fileName).toBe( + "my shot.png", + ); + expect(media(`https://gitlab.com/a/b/uploads/${SECRET}/%E0%A4%A.png`)?.fileName).toBe( + "%E0%A4%A.png", + ); + }); + + it("resolves the relative upload links GitLab writes into bodies", () => { + const body = [ + `![shot](/uploads/${SECRET}/shot.png){width=400}`, + `[demo.mp4]()`, + ``, + `[ref]: /uploads/${SECRET}/ref.pdf`, + `[next-line]:`, + ` /uploads/${SECRET}/next-line.png`, + `Prose mentioning /uploads/${SECRET}/x.png stays prose.`, + `![elsewhere](https://cdn.example.com/uploads/${SECRET}/a.png)`, + "```md", + `![in a fence](/uploads/${SECRET}/fenced.png)`, + "```", + `Inline \`![code](/uploads/${SECRET}/inline-code.png)\` stays as written.`, + ].join("\n"); + expect(resolveGitLabUploadLinks(body, "https://gitlab.com/team/app/")).toBe( + [ + `![shot](https://gitlab.com/team/app/uploads/${SECRET}/shot.png){width=400}`, + `[demo.mp4]()`, + ``, + `[ref]: https://gitlab.com/team/app/uploads/${SECRET}/ref.pdf`, + `[next-line]:`, + ` https://gitlab.com/team/app/uploads/${SECRET}/next-line.png`, + `Prose mentioning /uploads/${SECRET}/x.png stays prose.`, + `![elsewhere](https://cdn.example.com/uploads/${SECRET}/a.png)`, + "```md", + `![in a fence](/uploads/${SECRET}/fenced.png)`, + "```", + `Inline \`![code](/uploads/${SECRET}/inline-code.png)\` stays as written.`, + ].join("\n"), + ); + }); + + it("keeps fenced code as written in a body with CRLF line endings", () => { + const body = ["```md", `![in a fence](/uploads/${SECRET}/fenced.png)`, "```", ""].join("\r\n"); + expect(resolveGitLabUploadLinks(body, "https://gitlab.com/team/app")).toBe(body); + }); +}); diff --git a/packages/source-control-gitlab/src/shared/gitLabMedia.ts b/packages/source-control-gitlab/src/shared/gitLabMedia.ts new file mode 100644 index 0000000000000..5aa125acebe0b --- /dev/null +++ b/packages/source-control-gitlab/src/shared/gitLabMedia.ts @@ -0,0 +1,102 @@ +import type { SourceControlMediaUrl } from "@t3tools/source-control-core/shared/definition"; + +/** + * GitLab writes uploads into a merge request as project-relative `/uploads//` links, + * and a private project serves them only to a signed-in request. + */ +const UPLOAD_PATH_PATTERN = + /^\/(?:-\/project\/(\d+)|((?:[\w.-]+\/)*[\w.-]+))\/uploads\/([0-9a-f]{32})\/([^/]+)$/iu; +/** + * An inline code span, kept as written, or a link destination, HTML `src`/`href`, or reference + * definition pointing at an upload. + */ +const RELATIVE_UPLOAD_PATTERN = + /(`+)[^`]*?\1|(\]\(\s* 0 ? name : "gitlab-upload"; +} + +export function gitLabMediaUrl(url: URL): SourceControlMediaUrl | null { + const upload = parseGitLabUpload(url); + return upload === null + ? null + : { fetchUrl: `${upload.origin}${url.pathname}`, fileName: fileName(upload.fileSegment) }; +} + +/** Rewrites GitLab's project-relative upload links against the project's web URL, outside code. */ +export function resolveGitLabUploadLinks(markdown: string, projectUrl: string): string { + const base = projectUrl.replace(/\/+$/u, ""); + let openFence: string | null = null; + let afterBareLabel = false; + return markdown + .split("\n") + .map((line) => { + const continuesLabel = afterBareLabel; + afterBareLabel = false; + const fence = FENCE_PATTERN.exec(line.replace(/\r$/u, "")); + if (fence !== null) { + const marker = fence[1]!; + if (openFence === null) openFence = marker; + else if ( + marker[0] === openFence[0] && + marker.length >= openFence.length && + fence[2]!.trim() === "" + ) { + openFence = null; + } + return line; + } + if (openFence !== null) return line; + if (continuesLabel) { + line = line.replace(CONTINUED_UPLOAD_PATTERN, (_match, prefix: string) => { + return `${prefix}${base}/uploads/`; + }); + } + afterBareLabel = BARE_REFERENCE_LABEL_PATTERN.test(line); + return line.replace(RELATIVE_UPLOAD_PATTERN, (match, _code, prefix: string | undefined) => + prefix === undefined ? match : `${prefix}${base}/uploads/`, + ); + }) + .join("\n"); +} From acfdc7aa2e43363e69b3ac7ce524165f6c5b2726 Mon Sep 17 00:00:00 2001 From: Torben Buck Date: Sat, 24 Oct 2026 13:48:46 +0200 Subject: [PATCH 2/2] fix(gitlab): fetch uploads only from hosts glab is signed in to, or gitlab.com The upload URL comes from text anyone can write into a merge request, so the server must not request an arbitrary host with it. --- .../src/server/gitLabMediaFetch.test.ts | 18 ++++++++++++++---- .../src/server/gitLabMediaFetch.ts | 8 ++++++-- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/packages/source-control-gitlab/src/server/gitLabMediaFetch.test.ts b/packages/source-control-gitlab/src/server/gitLabMediaFetch.test.ts index 2b2cd3f3c05b2..55310e789d305 100644 --- a/packages/source-control-gitlab/src/server/gitLabMediaFetch.test.ts +++ b/packages/source-control-gitlab/src/server/gitLabMediaFetch.test.ts @@ -16,6 +16,7 @@ const API = `https://gitlab.example.com/api/v4/projects/group%2Fapp/uploads/${SE function fetchWith( token: string | null, respond: (url: string, authorization: string | undefined) => Response, + url = UPLOAD, ) { const requests: Array<[string, string | undefined]> = []; const invalidated: Array = []; @@ -36,7 +37,7 @@ function fetchWith( ); const status = Effect.gen(function* () { const fetchMedia = yield* GitLabMediaFetch.make; - return (yield* fetchMedia({ url: UPLOAD, headers: {} }))?.status; + return (yield* fetchMedia({ url, headers: {} }))?.status; }).pipe(Effect.provide(layer), Effect.scoped); return { status, requests, invalidated }; } @@ -65,11 +66,20 @@ describe("GitLabMediaFetch", () => { }); }); - it.effect("loads the upload itself without a login, as a public project serves it", () => { - const { status, requests } = fetchWith(null, () => new Response(null, { status: 200 })); + it.effect("loads a gitlab.com upload without a login, as a public project serves it", () => { + const upload = `https://gitlab.com/group/app/uploads/${SECRET}/shot.png`; + const { status, requests } = fetchWith(null, () => new Response(null, { status: 200 }), upload); return Effect.gen(function* () { expect(yield* status).toBe(200); - expect(requests).toEqual([[UPLOAD, undefined]]); + expect(requests).toEqual([[upload, undefined]]); + }); + }); + + it.effect("asks no host that glab is not signed in to", () => { + const { status, requests } = fetchWith(null, () => new Response(null, { status: 200 })); + return Effect.gen(function* () { + expect(yield* status).toBeUndefined(); + expect(requests).toEqual([]); }); }); diff --git a/packages/source-control-gitlab/src/server/gitLabMediaFetch.ts b/packages/source-control-gitlab/src/server/gitLabMediaFetch.ts index 71638b286f36b..05bfadc3087e6 100644 --- a/packages/source-control-gitlab/src/server/gitLabMediaFetch.ts +++ b/packages/source-control-gitlab/src/server/gitLabMediaFetch.ts @@ -13,6 +13,7 @@ import { gitLabUploadApiUrl, parseGitLabUpload } from "../shared/gitLabMedia.ts" import * as GitLabCredentials from "./GitLabCredentials.ts"; +const PUBLIC_HOST = "gitlab.com"; const MAX_REDIRECTS = 3; const MANUAL_REDIRECT: RequestInit = { redirect: "manual" }; @@ -54,14 +55,17 @@ export const make = Effect.gen(function* () { }); /** - * Reads the upload through the API with the `glab` login for its host. Without a login, or - * where the API refuses it (GitLab before 17.4), the upload itself still serves a public project. + * Reads the upload through the API with the `glab` login for its host. Where the API refuses it + * (GitLab before 17.4), or gitlab.com without a login, the upload itself serves a public project. */ return Effect.fn("GitLabMediaFetch.fetchMedia")(function* (request: SourceControlMediaRequest) { const url = new URL(request.url); const upload = parseGitLabUpload(url); if (upload === null) return null; const token = yield* credentials.get(url.host); + // Anyone can write the URL into a body, so only a host `glab` is signed in to, or the public + // GitLab, is ever asked. + if (Option.isNone(token) && url.host !== PUBLIC_HOST) return null; if (Option.isSome(token)) { const response = yield* fetchFollowingRedirects(gitLabUploadApiUrl(upload), request.headers, { origin: upload.origin,