Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion context/github-sync-invariants.md
Original file line number Diff line number Diff line change
Expand Up @@ -742,7 +742,13 @@ app to an endpoint its credential cannot use, which fails even though the token
chain "correctly" falls back to the PAT.
- Private `user-attachments` reads are the exception to app-token-first reads:
GitHub returns 404 to installation tokens, so the repo-scoped image proxy must
use the user's PAT/`gh` chain (`internal/github/client.go::GetMarkdownImage`).
use the user's PAT/`gh` chain (`internal/github/markdown_images.go::GetMarkdownImage`).
- Repository-file markdown images (`blob`/`raw` web URLs, `raw.githubusercontent.com`)
are proxied only for the route's own repository, use the normal read chain, and are
type-sniffed because the contents raw media type hides the file type; web URLs do not
delimit ref from path, so ref splits are tried shortest-first past 404s. They are
marked mutable because the ref is usually a branch. The frontend must apply the
same URL rules (`internal/github/markdown_images.go::getRepositoryFileImage`).
Config may carry multiple `[[github_apps]]` rows for one host, but those rows
represent distinct app credentials. Management commands must target one row by
app owner/installation account or app id, and duplicate installation accounts on
Expand Down
5 changes: 5 additions & 0 deletions context/platform-sync-invariants.md
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,11 @@ GitLab private Markdown upload web URLs do not accept API-token authentication.
Translate only repo-scoped upload URLs to the authenticated project-upload API;
never proxy arbitrary provider URLs. (`internal/platform/gitlab/markdown_images.go::GetMarkdownImage`)

The markdown image cache is keyed by stable repository identity, never the owner/name
route, so a replacement occupant of a reused route cannot receive the previous
repository's bytes; providers mark ref-addressed sources `Mutable` and the server then
caches them for minutes instead of a year (`internal/server/markdown_images.go::markdownImageCacheKey`).

GitLab merge request and issue `iid` values are repo-scoped numbers. Persist
provider object ids separately from user-visible numbers, and scope events by
provider identity so equal GitHub/GitLab ids do not collide.
Expand Down
45 changes: 45 additions & 0 deletions frontend/src/lib/utils/markdown.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,51 @@ describe("renderMarkdown task lists", () => {
}
});

it("proxies images committed to the repository through the repo-scoped API", async () => {
const repo = {
provider: "github",
platformHost: "github.com",
owner: "acme",
name: "widgets",
repoPath: "acme/widgets",
};
const sources = [
"https://github.com/acme/widgets/blob/feat/search-controls/docs/images/search.png?raw=true",
"https://github.com/acme/widgets/raw/main/docs/images/search.png",
"https://raw.githubusercontent.com/Acme/Widgets/main/docs/images/search.png",
];

for (const source of sources) {
const html = await renderMarkdown(`![Search options](${source})`, repo);
expect(html).toContain(
`src="/api/v1/repo/github/acme/widgets/markdown-image?source=${encodeURIComponent(source)}"`,
);
}
});

it("leaves GitHub images outside the repository on direct loading", async () => {
const repo = {
provider: "github",
platformHost: "github.com",
owner: "acme",
name: "widgets",
repoPath: "acme/widgets",
};
const sources = [
"https://github.com/acme/other/blob/main/docs/images/search.png?raw=true",
"https://raw.githubusercontent.com/acme/other/main/docs/images/search.png",
"https://github.com/acme/widgets/tree/main/docs/images/search.png",
"https://github.com/acme/widgets/blob/main",
"https://gist.githubusercontent.com/acme/widgets/main/docs/images/search.png",
];

for (const source of sources) {
const html = await renderMarkdown(`![Search options](${source})`, repo);
expect(html).toContain(`src="${source}"`);
expect(html).not.toContain("markdown-image");
}
});

it("proxies private GitLab upload images through the repo-scoped API", async () => {
const source = "/uploads/0123456789abcdef/private-image.png";
const canonicalSource = "https://gitlab.example.com/group/project/uploads/0123456789abcdef/private-image.png";
Expand Down
28 changes: 24 additions & 4 deletions frontend/src/lib/utils/markdown.ts
Original file line number Diff line number Diff line change
Expand Up @@ -426,12 +426,12 @@ function proxiedMarkdownImageSource(source: string, repo: RepoContext): string |
provider === "gitlab" ? normalizedGitLabMarkdownImageSource(source, host, repo.repoPath) : source;
if (!normalizedSource) return null;
const url = new URL(normalizedSource);
if (url.protocol !== "https:" || url.host.toLowerCase() !== host.toLowerCase()) return null;
if (provider === "github" && !url.pathname.startsWith("/user-attachments/assets/")) return null;
if (url.protocol !== "https:") return null;
if (provider === "github" && !isProxiedGitHubImage(url, host, repo.repoPath)) return null;
if (
provider === "gitlab" &&
!url.pathname.startsWith(`/${repo.repoPath}/uploads/`) &&
!/^\/-\/project\/\d+\/uploads\//.test(url.pathname)
(url.host.toLowerCase() !== host.toLowerCase() ||
(!url.pathname.startsWith(`/${repo.repoPath}/uploads/`) && !/^\/-\/project\/\d+\/uploads\//.test(url.pathname)))
)
return null;
return providerRepoResourceURL(repo, "/markdown-image", { source: url.toString() });
Expand All @@ -440,6 +440,26 @@ function proxiedMarkdownImageSource(source: string, repo: RepoContext): string |
}
}

// GitHub images that need the repository credential: private user-attachments
// uploads, and files committed to this repository referenced through blob or
// raw web URLs. Other repositories' files stay on direct loading because the
// route's credential is only known to reach this repository. The server
// applies the same rules (internal/github/markdown_images.go).
function isProxiedGitHubImage(url: URL, host: string, repoPath: string): boolean {
const urlHost = url.host.toLowerCase();
const platformHost = host.toLowerCase();
const segments = url.pathname.split("/").filter(Boolean);
const inRepository = segments.slice(0, 2).join("/").toLowerCase() === repoPath.toLowerCase();
if (urlHost === platformHost) {
if (url.pathname.startsWith("/user-attachments/assets/")) return true;
return inRepository && (segments[2] === "blob" || segments[2] === "raw") && segments.length >= 5;
}
if (urlHost === "raw.githubusercontent.com" && platformHost === "github.com") {
return inRepository && segments.length >= 4;
}
return false;
}

function normalizedGitLabMarkdownImageSource(source: string, host: string, repoPath: string): string | null {
if (/^https:\/\//i.test(source)) return source;
const uploadPath = source.replace(/^\//, "");
Expand Down
92 changes: 0 additions & 92 deletions internal/github/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,8 @@ import (
"encoding/json"
"errors"
"fmt"
"io"
"log/slog"
"mime"
"net/http"
"net/url"
"sort"
"strconv"
"strings"
Expand Down Expand Up @@ -183,95 +180,6 @@ type repoUserClient interface {
) (*gh.User, error)
}

// markdownImageClient carries the full repository identity even though the
// attachment URL is host-scoped: credential selection is per repository, so a
// repo-scoped route must be able to pick its own token for the fetch.
type markdownImageClient interface {
GetMarkdownImage(
ctx context.Context, owner, repo, sourceURL string,
) (platform.MarkdownImage, error)
}

const maxMarkdownImageBytes = 25 << 20

var allowedMarkdownImageTypes = map[string]struct{}{
"image/avif": {},
"image/bmp": {},
"image/gif": {},
"image/jpeg": {},
"image/png": {},
"image/webp": {},
}

func (c *liveClient) GetMarkdownImage(
ctx context.Context,
owner, _, sourceURL string,
) (platform.MarkdownImage, error) {
parsed, err := url.Parse(sourceURL)
if err != nil || parsed.Scheme != "https" || parsed.User != nil ||
!strings.EqualFold(parsed.Host, c.platformHost) ||
!strings.HasPrefix(parsed.EscapedPath(), "/user-attachments/assets/") {
return platform.MarkdownImage{}, &platform.Error{
Code: platform.ErrCodeInvalidArgument, Provider: platform.KindGitHub,
PlatformHost: c.platformHost, Field: "source", Err: errors.New("unsupported markdown image URL"),
}
}

// github.com/user-attachments accepts the user's credential but returns
// 404 for installation tokens, even when the app can read the repository.
authCtx := tokenauth.WithMutationAuth(tokenauth.WithGitHubOwner(ctx, owner))
if c.source == nil {
return platform.MarkdownImage{}, errors.New("GitHub markdown image token source is unavailable")
}
token, err := c.source.Token(authCtx)
if err != nil {
return platform.MarkdownImage{}, err
}
req, err := http.NewRequestWithContext(authCtx, http.MethodGet, parsed.String(), nil)
if err != nil {
return platform.MarkdownImage{}, err
}
req.Header.Set("Accept", "application/octet-stream")
req.Header.Set("Authorization", "Bearer "+token)
client := c.markdownImageHTTPClient
if client == nil {
client = http.DefaultClient
}
resp, err := client.Do(req)
if err != nil {
return platform.MarkdownImage{}, err
}
defer resp.Body.Close()
if resp.StatusCode == http.StatusUnauthorized || resp.StatusCode == http.StatusForbidden {
return platform.MarkdownImage{}, platform.PermissionDenied(platform.KindGitHub, c.platformHost, errors.New(resp.Status))
}
if resp.StatusCode == http.StatusNotFound {
return platform.MarkdownImage{}, &platform.Error{Code: platform.ErrCodeNotFound, Provider: platform.KindGitHub, PlatformHost: c.platformHost}
}
if resp.StatusCode < 200 || resp.StatusCode >= 300 {
return platform.MarkdownImage{}, fmt.Errorf("fetch GitHub markdown image: %s", resp.Status)
}

contentType, _, err := mime.ParseMediaType(resp.Header.Get("Content-Type"))
if err != nil {
return platform.MarkdownImage{}, fmt.Errorf("parse GitHub markdown image content type: %w", err)
}
if _, ok := allowedMarkdownImageTypes[contentType]; !ok {
return platform.MarkdownImage{}, &platform.Error{
Code: platform.ErrCodeInvalidArgument, Provider: platform.KindGitHub,
PlatformHost: c.platformHost, Field: "source", Err: fmt.Errorf("unsupported image content type %q", contentType),
}
}
content, err := io.ReadAll(io.LimitReader(resp.Body, maxMarkdownImageBytes+1))
if err != nil {
return platform.MarkdownImage{}, err
}
if len(content) > maxMarkdownImageBytes {
return platform.MarkdownImage{}, fmt.Errorf("GitHub markdown image exceeds %d bytes", maxMarkdownImageBytes)
}
return platform.MarkdownImage{Content: content, ContentType: contentType}, nil
}

type conditionalPullRequestGetter interface {
GetPullRequestIfChanged(
ctx context.Context,
Expand Down
Loading
Loading