Skip to content

fix(auth): reject gitlab.io domains in DNS/HTTP token exchange - #1574

Open
latent-9 wants to merge 1 commit into
modelcontextprotocol:mainfrom
latent-9:fix-auth-reject-gitlab-pages
Open

latent-9 wants to merge 1 commit into
modelcontextprotocol:mainfrom
latent-9:fix-auth-reject-gitlab-pages

Conversation

@latent-9

Copy link
Copy Markdown
Contributor

What

Extends the shared-hosting rejection added in #1506 from github.io to gitlab.io (and subdomains) in ValidateDomainAndTimestamp, which guards both DNS and HTTP token exchange.

Why

GitLab Pages serves <group>.gitlab.io from the <group>/<group>.gitlab.io repository, and that repository can be written to by group members holding the Developer role — a far weaker bar than group ownership. The DNS/HTTP proof therefore demonstrates control of one Pages repository, yet it minted publish permissions for the entire io.gitlab.<group>/* namespace.

This is the same weaker-bar takeover #1506 closed for GitHub Pages; GitLab Pages is the identical mechanic (per-group Pages repository) and was notably absent from that fix.

Change

  • New isGitLabPagesDomain check alongside isGitHubPagesDomain, with a distinct error message pointing GitLab users at apex-domain proof.
  • Test-table rows covering my-group.gitlab.io (including mixed case and subdomains), plus lookalikes (gitlab.io.evil-example.com, my-group.gitlab.io.example.com) that must remain allowed.

Scope note: first-come-claimable hosts without an org-repository model (netlify.app, vercel.app, pages.dev) raise a different trust question and are deliberately left out of this PR.

GitLab Pages serves <group>.gitlab.io from the <group>/<group>.gitlab.io
repository, which ordinary Developers can be granted push access to. The
DNS/HTTP proof therefore demonstrates control of a single Pages
repository, not group ownership, yet it minted publish permissions for
the whole io.gitlab.<group>/* namespace — the same weaker-bar takeover
modelcontextprotocol#1506 closed for GitHub Pages.

Reject gitlab.io (and subdomains) alongside github.io in
ValidateDomainAndTimestamp, with lookalike coverage in the test table.

@JosephDoUrden JosephDoUrden left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pulled the branch, built and ran go test ./internal/api/handlers/v0/auth/..., green. The check sits in ValidateDomainAndTimestamp, which ExchangeToken shares for both the DNS and HTTP paths, so one place covers both. Same shape as the merged github.io fix in #1506, and the gitlab.io mechanic is the same, a group.gitlab.io repo lower-privileged members can publish to, so the proof is a weaker bar than namespace ownership. I also tried the trailing-dot form my-group.gitlab.io. to slip past the suffix match, and it still gets rejected, IsValidDomain drops it before it matters. The scope note leaving netlify/vercel/pages.dev out reads right to me, those are first-come and a different trust question. Looks merge-ready.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants