diff --git a/CHANGELOG.md b/CHANGELOG.md index 47fcfd02..c060b98c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,8 +32,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 Sign-in reuses the GitHub CLI (`GH_TOKEN`, `GITHUB_TOKEN`, then `gh auth token`, found even from a Finder launch); public repositories work signed out, and a private repository or a spent rate limit says to run - `gh auth login`. Remote images in issue text are shown as links rather than - loaded, and non-web link targets are disabled. The Info tab gains a GitHub + `gh auth login`. Screenshots pasted into an issue are shown; images from + any other host are shown as links rather than loaded, and non-web link + targets are disabled. The Info tab gains a GitHub row that opens the branch you are on. The tab talks to `api.github.com` only while you use it. diff --git a/crates/tty7-core/src/core/github/markdown.rs b/crates/tty7-core/src/core/github/markdown.rs index d8bda005..740f5793 100644 --- a/crates/tty7-core/src/core/github/markdown.rs +++ b/crates/tty7-core/src/core/github/markdown.rs @@ -6,8 +6,12 @@ //! //! - **Images load themselves.** An `![](…)` or `` is fetched the //! moment it is drawn, which tells whoever controls that URL that this -//! issue was opened, from which IP, and when. Every image becomes a plain -//! link instead: the reader can still open it, deliberately, in the browser. +//! issue was opened, from which IP, and when. Only images GitHub itself +//! hosts (a screenshot pasted into an issue, see [`is_github_hosted`]) are +//! drawn — reading the issue already told GitHub as much. Any other image +//! becomes a plain link: the reader can still open it, deliberately, in +//! the browser. (github.com proxies third-party images through its own +//! servers for the same reason; tty7 has no proxy, so it does not load them.) //! - **Links open whatever they name.** A click hands the URL to the OS, and //! `file:///…` or an app's custom scheme can launch programs. Only `http`, //! `https` and `mailto` targets survive; anything else is pointed at `#`. @@ -67,12 +71,14 @@ fn sanitize_line(line: &str, image_label: &str) -> String { if let Some(rewritten) = reference_definition(line) { return rewritten; } + // A table row cannot be split across paragraphs without ending the table. + let own_block = !line.trim_start().starts_with('|'); let mut out = String::with_capacity(line.len()); let mut rest = line; while !rest.is_empty() { match rest.find('`') { Some(start) => { - out.push_str(&sanitize_text(&rest[..start], image_label)); + out.push_str(&sanitize_text(&rest[..start], image_label, own_block)); let after = &rest[start..]; let ticks = after.chars().take_while(|c| *c == '`').count(); let closer = "`".repeat(ticks); @@ -90,7 +96,7 @@ fn sanitize_line(line: &str, image_label: &str) -> String { } } None => { - out.push_str(&sanitize_text(rest, image_label)); + out.push_str(&sanitize_text(rest, image_label, own_block)); break; } } @@ -123,14 +129,30 @@ fn reference_definition(line: &str) -> Option { )) } -/// Plain text between code spans. -fn sanitize_text(text: &str, image_label: &str) -> String { +/// Plain text between code spans. `own_block` puts each image kept as an +/// image in a paragraph of its own: the text view draws an image that shares +/// a paragraph with text at line height, a screenshot shrunk to an icon. +fn sanitize_text(text: &str, image_label: &str, own_block: bool) -> String { + let image = |alt: &str, url: &str| match own_block { + true => format!("\n\n![{alt}]({url})\n\n"), + false => format!("![{alt}]({url})"), + }; let mut out = String::with_capacity(text.len()); let bytes = text.as_bytes(); let mut i = 0; while i < text.len() { let rest = &text[i..]; - // `![alt](…)` / `![alt][ref]` → a link with the same target. + // A GitHub-hosted `![alt](url)` stays an image. + if rest.starts_with("![") + && !escaped(bytes, i) + && let Some((alt, url, len)) = inline_image(rest) + && is_github_hosted(url) + { + out.push_str(&image(alt, url)); + i += len; + continue; + } + // Any other `![alt](…)` / `![alt][ref]` → a link with the same target. if rest.starts_with("![") && !escaped(bytes, i) { out.push('['); if rest[2..].starts_with(']') { @@ -173,7 +195,9 @@ fn sanitize_text(text: &str, image_label: &str) -> String { let alt = attr(tag, "alt").filter(|a| !a.trim().is_empty()); let label = alt.as_deref().unwrap_or(image_label); let label = label.replace(['[', ']'], ""); - if is_safe_target(&src) && !src.is_empty() { + if is_github_hosted(&src) { + out.push_str(&image(&label, &src.replace(' ', "%20"))); + } else if is_safe_target(&src) && !src.is_empty() { out.push_str(&format!("[{label}]({})", src.replace(' ', "%20"))); } else { out.push_str(&format!("[{label}](#)")); @@ -300,6 +324,52 @@ fn scheme(url: &str) -> Option<&str> { Some(head) } +/// `![alt](url)` or `![alt](url "title")` at the start of `s`: the alt text, +/// the URL, and how many bytes the whole form takes. `None` for anything +/// else, including the reference form `![alt][id]`. +fn inline_image(s: &str) -> Option<(&str, &str, usize)> { + let rest = s.strip_prefix("![")?; + let alt_end = rest.find(']')?; + let alt = &rest[..alt_end]; + if alt.contains('[') { + return None; + } + let target = rest[alt_end + 1..].strip_prefix('(')?; + let close = target.find(')')?; + let inside = target[..close].trim(); + let url = inside.split_whitespace().next()?; + let url = url + .strip_prefix('<') + .and_then(|u| u.strip_suffix('>')) + .unwrap_or(url); + Some((alt, url, 2 + alt_end + 1 + 1 + close + 1)) +} + +/// Whether an image URL is one GitHub serves itself: attachments pasted into +/// an issue (`github.com/user-attachments/…`, a repository's `/assets/…`) and +/// anything under `githubusercontent.com` (older attachments, raw files, +/// avatars, GitHub's own image proxy). Only `https`. +pub fn is_github_hosted(url: &str) -> bool { + let Some(rest) = url.strip_prefix("https://") else { + return false; + }; + let (host, path) = rest.split_at(rest.find('/').unwrap_or(rest.len())); + // Userinfo or a port would make the "host" above something else. + if host.contains(['@', ':']) { + return false; + } + let host = host.to_ascii_lowercase(); + if host == "github.com" { + let mut parts = path.trim_start_matches('/').split('/'); + return match (parts.next(), parts.next(), parts.next()) { + (Some("user-attachments"), Some(_), _) => true, + (Some(_), Some(_), Some("assets")) => true, + _ => false, + }; + } + host.ends_with(".githubusercontent.com") +} + /// Whether a link target may be handed to the OS opener. /// /// Relative targets and fragments carry no scheme and so cannot name a program; @@ -324,6 +394,60 @@ mod tests { sanitize(src, "image") } + #[test] + fn github_hosted_images_stay_images() { + let pasted = "https://github.com/user-attachments/assets/352d14e0-d11c"; + // Each in a paragraph of its own, so it is drawn at its size. + assert_eq!( + s(&format!("![shot]({pasted})")), + format!("\n\n![shot]({pasted})\n\n") + ); + assert_eq!( + s(&format!("a ![](<{pasted}> \"t\") b")), + format!("a \n\n![]({pasted})\n\n b") + ); + assert_eq!( + s( + r#"Screenshot"# + ), + "\n\n![Screenshot](https://user-images.githubusercontent.com/1/a.png)\n\n" + ); + assert_eq!( + s(&format!(r#""#)), + format!("\n\n![image]({pasted})\n\n") + ); + // In a table row the image stays in its cell. + assert_eq!( + s(&format!("| ![x]({pasted}) |")), + format!("| ![x]({pasted}) |") + ); + } + + #[test] + fn only_github_itself_counts_as_github_hosted() { + for yes in [ + "https://github.com/user-attachments/assets/x", + "https://github.com/owner/repo/assets/1/x", + "https://private-user-images.githubusercontent.com/1/x.png?jwt=y", + "https://camo.githubusercontent.com/abc", + "https://RAW.githubusercontent.com/o/r/main/a.png", + ] { + assert!(is_github_hosted(yes), "{yes}"); + } + for no in [ + "http://github.com/user-attachments/assets/x", + "https://github.com/owner/repo", + "https://github.com.evil.io/user-attachments/assets/x", + "https://evil.io/x.githubusercontent.com/a.png", + "https://githubusercontent.com.evil.io/a.png", + "https://github.com@evil.io/user-attachments/assets/x", + "https://x.io/a.png", + "", + ] { + assert!(!is_github_hosted(no), "{no}"); + } + } + #[test] fn images_become_links_to_the_same_target() { assert_eq!( @@ -339,8 +463,8 @@ mod tests { #[test] fn html_images_become_links_too() { assert_eq!( - s(r#"Screenshot"#), - "[Screenshot](https://github.com/u/a.png)" + s(r#"Screenshot"#), + "[Screenshot](https://x.io/u/a.png)" ); assert_eq!( s(""), diff --git a/docs/reference/privacy.mdx b/docs/reference/privacy.mdx index d531e7f3..e03b7b82 100644 --- a/docs/reference/privacy.mdx +++ b/docs/reference/privacy.mdx @@ -54,8 +54,9 @@ All of the above honour `http_proxy`. [Updates →](/reference/updates#proxies) The GitHub tab stores no credential of its own. It asks `GH_TOKEN`, `GITHUB_TOKEN` or `gh auth token` each time it connects, keeps the answer in memory only, and never writes it to disk or to the log. Images in issue and pull -request text are not fetched — they are shown as links — so opening an issue -does not tell a third-party image host that you read it. +request text are fetched only when GitHub hosts them (screenshots pasted into +an issue, `*.githubusercontent.com`); any other image is shown as a link, so +opening an issue does not tell a third-party image host that you read it. [The GitHub tab →](/window/side-panel#github) ## What is stored, and where diff --git a/docs/window/side-panel.mdx b/docs/window/side-panel.mdx index e4c3b600..f2a6c776 100644 --- a/docs/window/side-panel.mdx +++ b/docs/window/side-panel.mdx @@ -134,9 +134,10 @@ opens the branch you are on, on the remote it tracks — or the repository's pag for a branch you have not pushed. Markdown from GitHub is written by strangers, so it is rendered with two -changes: images are not loaded — each becomes a link you can open in the -browser — and links that would open anything other than a web page or an -e-mail are disabled. See [what leaves your machine](/reference/privacy#what-leaves-your-machine). +changes: only images GitHub hosts itself — the screenshots pasted into an issue +— are loaded, and any other image becomes a link you can open in the browser; +and links that would open anything other than a web page or an e-mail are +disabled. See [what leaves your machine](/reference/privacy#what-leaves-your-machine). ## The editor diff --git a/src/core/update.rs b/src/core/update.rs index 31a865d1..35316fd3 100644 --- a/src/core/update.rs +++ b/src/core/update.rs @@ -1830,6 +1830,20 @@ fn version_from_assets(assets: &[GitHubAsset]) -> Option { .map(|(_, version)| version) } +/// Give gpui an HTTP client, so an `img` with a web URL can load. +/// +/// Until this runs gpui holds a null client and every remote image quietly +/// fails. The only remote images tty7 draws are the GitHub-hosted ones in an +/// issue's text (see `core::github::markdown`). Built with the update check's +/// user agent and proxy; a proxy changed later applies from the next launch. +pub(crate) fn install_image_client(cx: &mut App) { + let manual_proxy = cx.global::().http_proxy.clone(); + match build_http_client(manual_proxy.as_deref()) { + Ok(client) => cx.set_http_client(std::sync::Arc::new(client)), + Err(err) => log::warn!("no HTTP client for images: {err:#}"), + } +} + fn build_http_client(manual_proxy: Option<&str>) -> Result { let user_agent = concat!("tty7/", env!("CARGO_PKG_VERSION")); // Normalise through the same helper the downloader uses, so a bare diff --git a/src/main.rs b/src/main.rs index 7c9f73a7..90e4905c 100644 --- a/src/main.rs +++ b/src/main.rs @@ -723,6 +723,7 @@ fn main() { crate::ui::theme::apply_cursor_hide_mode(cx); spawn_config_watcher(cx); crate::core::update::spawn_check(cx); + crate::core::update::install_image_client(cx); cx.background_executor() .spawn(async { crate::core::agent_hooks::refresh_hooks_at_launch();