Repository navigation
fix(oauth): show and sign a client's host in ASCII, so a look-alike reads as punycode - #24
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a7600c2ff2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if _, err := netip.ParseAddr(host); err == nil { | ||
| return host, true |
There was a problem hiding this comment.
For an HTTPS redirect such as https://[fe80::1%25еth0]/cb, url.Hostname() returns fe80::1%еth0, and netip.ParseAddr accepts the Unicode zone identifier. This branch consequently returns the host unchanged and marks it valid, so checkRedirect accepts it and the non-ASCII value reaches both the consent UI and the signed ClientHost, defeating this change's ASCII-only safety guarantee. Reject zones containing non-ASCII characters or normalize and validate the zone separately.
Useful? React with 👍 / 👎.
…eads as punycode A registered client's redirect host, its client_uri and a metadata document's host went to the consent page, the box page and the signed offer as url.Hostname() gave them, so https://сlaude.ai/cb with a Cyrillic с arrived as a look-alike clientHost. Every host shown or signed now goes through asciiHost: lower case ASCII, a name in punycode via golang.org/x/net/idna (UTS 46 lookup), an address as it is. A redirect, a client_id URL or a client_uri with no ASCII form is refused. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…s, canonicalise addresses, keep ASCII hosts that worked Review of #24: - asciiHost refuses a trailing dot, an empty or over-long label, a zone, and a name whose last label a browser's URL parser reads as an IPv4 number (2130706433, 0177.0.0.1, 0x7f.1). An address goes to its canonical form (::0001 is ::1). - A host already in ASCII is not held to the IDNA rules, so a client registered with an underscore or a "--" in its redirect host still passes checkRedirect at authorize; only a host with letters outside ASCII goes through UTS 46 lookup. - A client_id document URL must name its host as asciiHost gives it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
a7600c2 to
108284f
Compare
1 participant
What
The access-UI review found that FTW Cloud took
url.Hostname()of a registered redirect with no ASCII check (internal/oauth/authorize.go,clients.go). Sohttps://сlaude.ai/cb, with a Cyrillic с, reached the consent page, the box page and the signed offer as a look-alikeclientHost.asciiHost(internal/oauth/host.go): lower case ASCII. A name with letters outside ASCII goes to punycode throughgolang.org/x/net/idna(UTS 46 lookup profile); an address to its canonical form (::0001is::1). Refused: a host that fails conversion, an empty or over-long label, a trailing dot (claude.ai.), an address with a zone, and a name whose last label a browser's URL parser reads as an IPv4 number (2130706433,0177.0.0.1,0x7f.1).--in its redirect host still passescheckRedirectwhen it authorizes again.clientHost, the box page's client host, the consent page's redirect host and the error page's link host all go through it.сlaude.ainow shows and signs asxn--laude-0ye.ai.checkRedirect), and aclient_idmetadata URL whose host is not already in that form (checkCIMDURL). Aclient_uriwith none is dropped, as an http one already is.golang.org/x/netmoves from indirect to direct.The app will also refuse non-ASCII hosts on its side (ios-access-ui session).
Tests
TestHostsAreASCII: names, upper case, the Cyrillic look-alike, an IDN, underscore and--hosts, canonical addresses; refused: a trailing dot, empty and long labels, numbers a browser reads as IPv4, a zone, a zero-width joiner, a right-to-left override, a space, a leading or trailing hyphen.TestADocumentURLNamesItsHostInASCIIandTestRedirectsInASCIIStillWork.TestALookAlikeHostIsShownAndSignedAsPunycode: a registered client withhttps://сlaude.ai/cbreaches the box page, the signed offer (opened withprotocol.OpenOffer) and the consent page asxn--laude-0ye.ai, and the Cyrillic never shows; redirects with no ASCII form do not register.go vet ./...,go test -race ./...,go test -tags ftwclouddevandscripts/check-imports.sh: green with-p 2.Separate from #23; no file in common. Rebased on main after #16 and #22.
🤖 Generated with Claude Code