You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Pick up new version of Glamour to avoid breaking hyperlinks #188
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.
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. 😉 (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 vim configuration 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.
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.
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?
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
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 v2 glamour and lipgloss effort
My point is that, right now with the older glamour you're already using, links split across lines are broken:
Screen.Recording.2025-04-04.at.10.07.55.AM-2.mov
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.
The hyperlinks work when wrapped using glamour v2.0.0:
package main
import (
"fmt""charm.land/glamour/v2""github.com/cli/go-gh/v2/pkg/markdown""github.com/cli/go-gh/v2/pkg/term"
)
constmd=`# HeadingThis is a [link](https://github.com/cli/go-gh/blob/450618ae2e11b09957ba1164a4c52ae8c9dd5828/pkg/markdown/markdown.go).So is this <https://github.com/cli/go-gh/blob/450618ae2e11b09957ba1164a4c52ae8c9dd5828/pkg/markdown/markdown.go>.Text | Link--- | ---Link | https://github.com/cli/go-gh/blob/450618ae2e11b09957ba1164a4c52ae8c9dd5828/pkg/markdown/markdown.gohttps://github.com/cli/go-gh/blob/450618ae2e11b09957ba1164a4c52ae8c9dd5828/pkg/markdown/markdown.go | Text`funcmain() {
t:=term.FromEnv()
iftext, err:=markdown.Render(md, glamour.WithStandardStyle(t.Theme()), markdown.WithWrap(80)); err==nil {
fmt.Println(text)
}
}
Screenshots look the same as the ones above in both the default Terminal as well as Ghostty, but when you Cmd+Click on the link in Ghostty in the latter case it opens the right URL; whereas, with the version of glamour you currently use it tries to open https://github.com/cli/go- which, of course, doesn't exist.
PR #281 builds atop #280 and should be a PR stack, but I can't install the gh-stack extension on my personal device until cli/cli#6675 is resolved or create the stack on a managed device. I can split or otherwise rebase each manual, though.
It would be great to upgrade to a stable, supported version. You're still using a prerelease 0.x version of glamour and they're already on v2.
Activity
andyfeller commentedon Mar 31, 2025
@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:
vimconfiguration that changes simple aesthetics I can live without if not presentAll of that to say, I use the Mac Terminal 😅 as my daily driver so I can ensure the default Apple experience for people.
andyfeller commentedon Mar 31, 2025
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
hrefrather than trusting the displayed text before clicking on such a link.heaths commentedon Mar 31, 2025
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.
BagToad commentedon Mar 31, 2025
This might need to be transferred to cli/go-gh since that's where markdown rendering and our glamour usage comes from AFAIK.
jtmcg commentedon Apr 2, 2025
Good suggestion, @BagToad, I've transferred the issue to
go-ghThis 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?
andyfeller commentedon Apr 4, 2025
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:
ghosttyGist: https://gist.github.com/andyfeller/ce831a39227a03373ba9467327d179a6
Screenshare.-.2025-04-04.9_05_04.AM.mp4
Concerns
Lack of terminal detection capability
ghdoesn't respect TERM, dumps colors / codes everywhere, breaks in dumb terms when prompting cli#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: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: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 feat(ansi): add hyperlink support charmbracelet/glamour#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.
heaths commentedon Apr 4, 2025
You already use glamour to render markdown:
go-gh/pkg/markdown/markdown.go
Line 67 in 61bf393
My point is that, right now with the older glamour you're already using, links split across lines are broken:
Screen.Recording.2025-04-04.at.10.07.55.AM-2.mov
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.
heaths commentedon Aug 13, 2026
Still repros in both Terminal and Ghostty:
Terminal
Ghostty
heaths commentedon Aug 13, 2026
The hyperlinks work when wrapped using glamour v2.0.0:
Screenshots look the same as the ones above in both the default Terminal as well as Ghostty, but when you Cmd+Click on the link in Ghostty in the latter case it opens the right URL; whereas, with the version of glamour you currently use it tries to open
https://github.com/cli/go-which, of course, doesn't exist.PR #281 builds atop #280 and should be a PR stack, but I can't install the
gh-stackextension on my personal device until cli/cli#6675 is resolved or create the stack on a managed device. I can split or otherwise rebase each manual, though.It would be great to upgrade to a stable, supported version. You're still using a prerelease 0.x version of glamour and they're already on v2.
Upgrade glamour to fix hyperlinks (OSC8)
Upgrade glamour to fix hyperlinks (OSC8)
Upgrade glamour to fix hyperlinks (OSC8)
Upgrade glamour to fix hyperlinks (OSC8)
Upgrade glamour to fix hyperlinks (OSC8)