fix(forgejo): images in private pull request descriptions load#24706

Approvability

Verdict: Needs Human Approval

Macroscope's review found this PR requires human approval — This Forgejo bug fix adds authenticated server-side media fetching and URL rewriting across web and mobile. The new outbound-fetch and redirect behavior has SSRF and credential-boundary implications, including an unresolved High-severity finding, so it requires human review.

Needs human approval because:

• 1 blocking correctness issue found at or above your repo's Minimum Blocking Severity

^{Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more.}

Review in Change Stack →

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

External image Recent review info

External image Run configuration

• Source mode: Auto
• Repository: pingdotgg/t3code
• Branch: main
• File: .coderabbit.config.ts
• Commit: da8cc59
• Layers: Repository TS config
• Review profile: CHILL
• Plan: Advanced
• Run ID: 9ee637f2-481b-4b41-b4fc-f998061c9149

External image Commits

Reviewing files that changed from the base of the PR and between f4bbf12ac3ca2d093efbc856e37755e31bc6963e and 18f4de3e6f3f44477d986fcc009ce01cb8eaa156.

External image Files selected for processing: 1

• apps/mobile/src/features/pull-requests/PullRequestMarkdown.tsx

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.

thanks @only21mil. your parsed markdown destination handling preserves code spans and correctly resolves private attachment paths. i kept your commits and added the fixes on top, including current main.

media fetching now requires a configured https origin and follows only same-origin redirects. existing tea credentials work through tea login helper get; fj keeps its existing authentication/refresh path. i moved the mobile image hook into its sole markdown caller, restoring the generic media component to main and fixing the mobile video-test regression.

verification: blacksmith vp run check:changed github/main passed format, lint, knip, affected typechecks, and tests. focused attachment/credential/markdown tests and all 10 existing mobile video tests passed. a real private gitea 1.25.1 repository with tea 0.16 reproduced unavailable images on main and loaded description, list, and comment images through signed assets after the fix. image previews work and literal code stays unchanged. two independent reviewers found no remaining issues on 18f4de3e6f3f44477d986fcc009ce01cb8eaa156. the old arbitrary-origin/redirect findings are addressed and resolved.

all 29 checks now pass. final coderabbit review raised two minor mobile timing findings, confirmed against the code: a route change during signing can settle an image to the old connection url; project context arriving after initial render can leave recognized media settled to a direct unauthenticated url. these remain known limits under the requested one-round-plus-follow-up review cap. the threads remain open for maintainer review. native mobile runtime verification is unverified because device access is disabled here; packaged desktop is also unverified. maintainer approval remains required. plain http, unconfigured hosts, and cross-origin object-storage redirects are intentionally unsupported.

main: private attachment images unavailable

updated: the same private description images load

description and comment image previews

Verified in a real dev environment

verified the existing head 18f4de3e6f3f44477d986fcc009ce01cb8eaa156 as-is on this linux boat vm. no source changes, commits or pushes.

ran vp i and background vp run dev on github/main (6575ce3250) in a separate worktree, then stopped that server before starting the exact pr head with its own isolated state. drove the actual web client with repository playwright and /usr/bin/google-chrome. created the project and codex gpt-6.1-sol threads through the client. the pr-head turn read the real repository README with a tool, returned its heading, and the reply persisted after navigation.

used a real private gitea 1.25.1 repository over https with tea 0.16.0 authentication and genuine uploaded screenshots. opened its pr through the client's pull requests view. main showed unavailable images in the description, list item and comment. this head loaded all three through signed assets. opened and closed all three image previews. literal markdown kept its relative path, the attachment link resolved to gitea, and the ordinary repository link kept its destination. three consecutive client reloads loaded all three images with no unavailable state.

server transcript:

command transcript

blacksmith ran the existing markdown, forgejo definition, media-fetch and source-control-provider tests: 54 passed across 4 files. the tested head also has 29 successful ci checks, none failing or pending.

main: the same private description, list and comment uploads are unavailable

pr head: private description and list uploads load through signed assets

opening and closing description, list and comment image previews

real codex gpt-6.1-sol turn returns the repository heading in the client

Not verified

native ios/android and packaged desktop were not exercised on this linux web setup. the two existing mobile timing findings remain unverified. remote relay/tunnel connections, fj oauth, http-only origins, mounted forge servers, external storage and live redirects were not exercised; redirect boundaries were covered by the existing tests.

one second-browser load returned an asset 502 after 30 seconds. reloading recovered, and the three later reloads passed. its cause is unverified. the test repository initially also lacked a git credential helper; configuring its local tea helper made real background git fetch pass. main's codex messages disappeared after navigation in this setup; that did not reproduce on the pr head.

@maria-rcks, an offer for the two open mobile timing threads, as one fast-forward of your head 18f4de3e6f:

4b4bcb516d on only21mil/t3code branch fix/i18757-images-in-private-forgejo-gitea-pull-req-offer

What it changes, in apps/mobile/src/features/pull-requests/PullRequestMarkdown.tsx:

• A recognized upload stays pending until the project and connection are known. The renderer then retries it with the next resolver, instead of settling on the direct URL.
• The signed URL comes from refreshAssetUrl, which resolves against the connection that is current when the URL comes back.

Verification: the new PullRequestMarkdown.test.tsx fails on your head (the upload settles to its direct URL) and passes on the offer. vp run check:changed passes. Not covered: a native device run, and the branch's conflict with main, which this offer leaves alone.

Happy to push it to your branch if you'd like.

@maria-rcks (cc @juliusmarminge), an offer for Julius's review: one merge of main and two commits on top of the earlier offer 4b4bcb516d, still a fast-forward of your head 18f4de3e6f:

f125b41fe5 on only21mil/t3code branch fix/i18757-images-in-private-forgejo-gitea-pull-req-offer

What it does:

• ff3f8719aa merges current main. The conflicts take main's image path from #31741, so this branch's own link resolver, the web relative-URL plugin and the mobile link-resolver context go.
• 731c88a799: main's resolveChangeRequestMediaUrl now asks the host's resolveRelativeUrl hook before its root-relative rule. Forgejo's hook reads the repository URL, so an upload on a server mounted below a path, and the attachments/<uuid> form, still load. Mobile passes the repository URL, as web already did.
• f125b41fe5: main's web plugin also resolves the src of a sized <img>, which is how Forgejo writes an image pasted from a high-density screen. It applies to every host.

Kept as you built it: the authenticated media fetch (ForgejoCli.mediaToken, forgejoMediaFetch.ts with its origin and redirect limits, and the driver wiring), forgejoMediaUrl, and the mobile native image source, which stays pending until the project and connection are known and signs through refreshAssetUrl.

Dropped: links to non-image uploads no longer resolve, since main's resolver covers media only. If you want them back, it's a hook call in the link handlers.

Verification:

• On the merged tree without the two commits, the new resolver, Forgejo hook and web plugin cases fail: main puts a mounted upload at the server root and leaves the <img> relative. With the two commits they pass.
• vp run check:changed passes.
• For images, the offer resolves the same URLs as your head, so your captures should still hold.

Not covered:

• A fresh capture on a private Forgejo server.
• The one unrelated commit main has gained since the merge, which merges cleanly.

After it lands, the body's first Change bullet and its non-image-link line would be out of date. Suggested text:

• The Forgejo definition gets a resolveRelativeUrl hook, which turns /attachments/<uuid> and attachments/<uuid> into an absolute URL on the repository's server, below its mount when it has one. The pull request media resolver from #31741 asks that hook before its own root-relative rule. So the web and mobile PR views resolve each image their markdown parsers produce, and on web the src of a sized <img>, while a link written inside code keeps the text the author wrote. On mobile, images the native renderer draws itself, such as one inside a list item, go through the same resolver and signed asset URL as standalone ones. It runs at render time, so editing a description still starts from the text Forgejo returned. Rewriting on the server would have saved absolute links back on the next edit.

• A link to a non-image upload keeps its relative destination, since the media resolver covers images only. A sized <img> upload resolves like a markdown image; I did not capture it.

Happy to push it to your branch if you'd like.