Pick up new version of Glamour to avoid breaking hyperlinks
Describe the feature or problem you’d like to solve
When rendering markdown, hyperlinks that wrap or are cut off get broken, such that a line with "https://git" and another with "hub.com/cli/cli" will have at least one link that doesn't work (or, worse, goes somewhere NSFW, etc.) and another line that probably won't be recognized as a link.
Fortunately, this issue was fixed in Glamour: https://github.com/charmbracelet/glamour/pull/411
Once they cut a new release, it would be great to pick that up. Â While I thought I opened or commented on a bug on this, I can't seem to find it. Apologies if it's dup'd.
Proposed solution
Hyperlinks in rendered markdown aren't broken if they wrap lines or are otherwise cut off.
Additional context
This is unrelated to years-long discussions on hyperlinks (OSC8) printed directly by gh, though it might be worth revisiting that in 2025. At least as of 2022, the main concern seemed to be Mac's default Terminal app...though, doesn't everyone replace that with iTerm2 or something these days. :wink: (I do appreciate that problem, though).
But since you use Glamour anyway for markdown rendering, it would be good to pick this new version up to fix broken hyperlinks when it renders them.
At least as of 2022, the main concern seemed to be Mac's default Terminal app...though, doesn't everyone replace that with iTerm2 or something these days. 😉 (I do appreciate that problem, though).
@heaths : Being vulnerable, I practice a very spartan, native tools heavy approach to software development that will allow me to function on any computer I pick up without customized setups or configurations. That means, I:
- use the native terminal
- use a
vimconfiguration that changes simple aesthetics I can live without if not present - can jump to a container, vm, cloud resource, etc and be productive instantly
All of that to say, I use the Mac Terminal 😅 as my daily driver so I can ensure the default Apple experience for people.
Also adding https://gist.github.com/egmontkob/eb114294efbcd5adb1944c9f3cb5feda for a little deeper insight into the underlying escape sequences used for OSC 8 hyperlinks.
Without seeing the experience, I wonder how users will protect themselves from maliciously designed links that mislead users. 🤔 Normally, people would hover over links to see the actual href rather than trusting the displayed text before clicking on such a link.
Please note, though: I'm not advocating for using OSC 8 anywhere you render URLs, even though listing runs completely truncates the URLs to be unusable.
You already use Glamour to render markdown for issue/pr bodies and links therein are already broken if they span lines. A new release of Glamour with the referenced change will fix that.
This might need to be transferred to cli/go-gh since that's where markdown rendering and our glamour usage comes from AFAIK.
Good suggestion, @BagToad, I've transferred the issue to go-gh
This seems worthwhile to me, but I think @andyfeller had some concerns that this might not be as straightforward as a glamour version bump given some other dependency interactions.
@andyfeller, could you capture those concerns here?
Thanks! Let's see how this works and capture a few thoughts. Ultimately, I expect my concern is likely considered outside of a specification.
Demo
Terminal: ghostty
Gist: https://gist.github.com/andyfeller/ce831a39227a03373ba9467327d179a6
https://github.com/user-attachments/assets/d1a66d49-8d46-4515-90cf-9d9f6d24d899
Concerns
-
Lack of terminal detection capability
https://github.com/cli/cli/issues/5721 demonstrates the GitHub CLI has a population of users who prefer dumb terminals, where any escape codes impact the user'e experience. I'm unsure how this would also factor into blind users' experience with screen readers should the escape sequences be displayed. 🤔
From
egmontkob"Hyperlinks (a.k.a. HTML-like anchors) in terminal emulators" section on detection:Currently there's no way of detecting whether the terminal emulator supports hyperlinks. We're hoping to address this at some point in the future.
The hyperlink feature should be used for providing convenient quick access to a target URI, but (at least by default) should not be the only means of figuring out the target.
-
Specification treating URL verification as a terminal / emulation defense in depth concern
In
egmontkob"Hyperlinks (a.k.a. HTML-like anchors) in terminal emulators", similar consideration for security arise however leave it as an exercise outside of the specification:This feature doesn't introduce anything that's not already present while browsing the web. Therefore we believe this feature doesn't have security aspects to worry about.
In particular, if a webpage is exploitable by making someone visit a URL, passing along their cookies (e.g. doesn't have proper CSRF protection), it's already exploitable from a malicious website.
Moreover, there's no "Referer" leakage to worry about.
That being said, a few points have been raised that are worth noting here.
Some locally installed applications might register a handle for some custom URI scheme (e.g. foobar://), and the handler application might be vulnerable in case the rest of the URI is maliciously crafted. Terminal emulators might decide to whitelist only some well known schemes and ask for the user's confirmation on less known ones.
Some are worried that this feature is unexpected from users, and that introducing this somewhat automated link between the terminal and the browser works against the concept of "defense in depth". That is, it's possible that a multi-step attack, exploiting a vulnerability of a website, takes place by using social engineering to get someone follow such a link that they somehow receive in the terminal emulator.
It's out of the scope of this specification to deal with such scenarios, this specification can only be responsible for direct security vulnerabilities that it might open.
However, terminal emulators might consider adding the following lines of defense:
- They shouldn't open the link on a simple mouse click (that's for copy-pasting or reporting mouse events typically, anyway), only on some more complex user action such as Ctrl+click or via the right-click menu.
- They should let the user know the URI upfront.
- They could decide to present a confirmation dialog before opening it.
- They could even offer to disable this feature (or even have it disabled by default). People working in critical environments (or their sysadmins) could decide to disable this feature entirely.
I understand the limits of what a specification can achieve in this situation.
Does this absolve the GitHub CLI if users view markdown content and unknowingly follow links that are hard to discern the ultimately destination? 🤷
-
This is part of a larger
v2glamour and lipgloss effortI didn't see this at first, but https://github.com/charmbracelet/glamour/pull/411 is part of a larger
glamourandlipglossv2 effort that has not been released or merged into the default branch:$ go get github.com/charmbracelet/glamour@v2-exp go: downloading github.com/charmbracelet/glamour v0.9.2-0.20250326005806-f8f160e6cd7b go: downloading github.com/charmbracelet/lipgloss/v2 v2.0.0-alpha.2.0.20250319221657-e0b75f7d5b68 go: upgraded github.com/charmbracelet/glamour v0.9.2-0.20250319212134-549f544650e3 => v0.9.2-0.20250326005806-f8f160e6cd7b go: added github.com/charmbracelet/lipgloss/v2 v2.0.0-alpha.2.0.20250319221657-e0b75f7d5b68 go: added github.com/muesli/cancelreader v0.2.2I mention that because it is likely going to be a larger upgrade as
charmbracelet/glamouris currentlyv0.9.1. -
Missing OSC 8 support
Source: https://github.com/Alhadis/OSC8-Adoption/
The most notable terminal missing is Apple Terminal, which we touched on above. Not a deal breaker; maybe something we can follow up on internally.
You already use glamour to render markdown: https://github.com/cli/go-gh/blob/61bf393cf4aeea6d00a6251390f5f67f5b67e727/pkg/markdown/markdown.go#L67
My point is that, right now with the older glamour you're already using, links split across lines are broken:
https://github.com/user-attachments/assets/f85a0212-e919-4e57-9dfc-8e41561d2a9f
Notice how when I hover over the long link, the first line would 404 and the second line isn't even recognized as a link. Using glamour/v2-exp the links render the same (the dashed underscores are iTerm2 - ignore that) but are actually valid should I Cmd+Click on them.
This isn't about hiding anything from the user (which any web site including github.com can do, but I digress), but about the links - whether rendered with OSC 8 or merely recognized by the terminal app (Windows Terminal has the same problem when it tries to auto-recognize links) - being actually valid.