Skip to content

Commit a1340a5

Browse files
authored
fix(upload): detect SSO interstitials titled with the org display name (#55)
* fix(upload): detect SSO interstitials titled with the org display name When an org's display name differs from its slug, the SAML SSO interstitial's title shows the display name ("Sign in to Acme Holdings, Inc" for slug acme-inc) and the page carries no /orgs/<slug>/sso link, so neither isSignInInterstitial nor the slug-based isSAMLProtected recognized it and users with full repo access were told they may lack upload permission. Add isAuthInterstitial as a fallback after both specific checks: any "Sign in to ..." title combined with a missing "currentUser" marker is an auth interstitial (a real repo page always embeds currentUser and its title starts "GitHub - owner/repo"). Since a stale session and an unauthorized SSO org are indistinguishable here, the error names both causes with their fixes. Fixes #52 * docs(skill): reword uploadToken-not-found troubleshooting row Sign-in and SSO interstitials now surface their own error messages before the generic one, so this symptom usually indicates a genuine access problem rather than an expired session.
1 parent e8757c6 commit a1340a5

3 files changed

Lines changed: 92 additions & 1 deletion

File tree

internal/upload/token.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ var uploadTokenRe = regexp.MustCompile(`"uploadToken":"([^"]+)"`)
1414

1515
var signInTitleRe = regexp.MustCompile(`(?i)<title>\s*Sign in to GitHub\b`)
1616

17+
var authTitleRe = regexp.MustCompile(`(?i)<title>\s*Sign in to [^\s<]`)
18+
1719
// isSignInInterstitial reports whether the page is GitHub's generic "Sign in to
1820
// GitHub" interstitial rather than the real repo page. When the user_session
1921
// cookie is present but invalid or expired, GitHub neither redirects nor errors:
@@ -31,6 +33,23 @@ func isSignInInterstitial(body []byte) bool {
3133
return signInTitleRe.Match(body) && !bytes.Contains(body, []byte(`"currentUser"`))
3234
}
3335

36+
// isAuthInterstitial reports whether the page is ANY "Sign in to …" auth
37+
// interstitial — GitHub's own sign-in page or an org SSO page. It is the
38+
// fallback for SSO pages that isSAMLProtected cannot recognize: when an org's
39+
// display name differs from its slug, the interstitial's title shows the
40+
// display name ("Sign in to Acme Holdings, Inc" for slug acme-inc) and the
41+
// page carries no /orgs/<slug>/sso link, so no slug-derived pattern can match
42+
// (issue #52). The caller only ever knows the slug.
43+
//
44+
// The generic title alone would be unsafe — a repo named "sign-in-to-x" is
45+
// fine because a real repo page's title starts "GitHub - <owner>/<repo>", but
46+
// we still require "currentUser" to be absent: the repo page embeds it in its
47+
// JS payload whether or not the request is authenticated, while auth
48+
// interstitials have no such payload.
49+
func isAuthInterstitial(body []byte) bool {
50+
return authTitleRe.Match(body) && !bytes.Contains(body, []byte(`"currentUser"`))
51+
}
52+
3453
// isSAMLProtected reports whether the repo page is a SAML SSO "Sign in to
3554
// <owner>" interstitial rather than the real repo page. When an organization
3655
// enforces SAML SSO and the browser session is authenticated but not
@@ -98,6 +117,16 @@ func (c *Client) getUploadToken(owner, repo string) (string, error) {
98117
"authorize in a browser at https://github.com/orgs/%s/sso (lasts ~24h), then retry. "+
99118
"Repository access alone is not enough", owner, owner)
100119
}
120+
// An org SSO interstitial whose title shows the org's display name rather
121+
// than its slug matches neither check above (issue #52); we cannot tell it
122+
// apart from a stale session here, so name both causes.
123+
if isAuthInterstitial(body) {
124+
return "", fmt.Errorf("GitHub served a sign-in page instead of %s/%s — either your session token is "+
125+
"invalid or expired (re-run `gh image extract-token`, or refresh GH_SESSION_TOKEN in CI), "+
126+
"or %s enforces SAML SSO and your session is not authorized for it "+
127+
"(authorize in a browser at https://github.com/orgs/%s/sso, lasts ~24h). "+
128+
"Repository access alone is not enough", owner, repo, owner, owner)
129+
}
101130
return "", fmt.Errorf("uploadToken not found on repo page — you may not have upload access to %s/%s "+
102131
"(or, if %s enforces SAML SSO, authorize at https://github.com/orgs/%s/sso)",
103132
owner, repo, owner, owner)

internal/upload/token_test.go

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,57 @@ func TestIsSignInInterstitial(t *testing.T) {
130130
}
131131
}
132132

133+
func TestIsAuthInterstitial(t *testing.T) {
134+
cases := []struct {
135+
name string
136+
body string
137+
want bool
138+
}{
139+
{
140+
// The issue #52 page: SSO interstitial titled with the org's display
141+
// name, which shares nothing with the slug the caller knows.
142+
name: "SSO interstitial titled with the org display name",
143+
body: `<title>Sign in to Acme Holdings, Inc</title>`,
144+
want: true,
145+
},
146+
{
147+
name: "GitHub's own sign-in interstitial",
148+
body: `<title>Sign in to GitHub · GitHub</title><form action="/session">`,
149+
want: true,
150+
},
151+
{
152+
name: "anonymous repo page must NOT match",
153+
body: `<title>GitHub - octocat/hello: hi</title>{"currentUser":null}`,
154+
want: false,
155+
},
156+
{
157+
name: "signed-in repo page must NOT match",
158+
body: `<title>GitHub - octocat/hello: hi</title>{"currentUser":{"login":"octocat"}}`,
159+
want: false,
160+
},
161+
{
162+
// The real title starts "GitHub - <owner>/<repo>", so the anchored
163+
// match cannot fire on a repo that merely mentions signing in.
164+
name: "repo about signing in must NOT match",
165+
body: `<title>GitHub - acme/auth: Sign in to GitHub from the CLI</title>`,
166+
want: false,
167+
},
168+
{
169+
// A title that ends at "Sign in to " names nothing; require a name.
170+
name: "bare 'Sign in to ' with no name must NOT match",
171+
body: `<title>Sign in to </title>`,
172+
want: false,
173+
},
174+
}
175+
for _, tc := range cases {
176+
t.Run(tc.name, func(t *testing.T) {
177+
if got := isAuthInterstitial([]byte(tc.body)); got != tc.want {
178+
t.Errorf("isAuthInterstitial(%q) = %v, want %v", tc.body, got, tc.want)
179+
}
180+
})
181+
}
182+
}
183+
133184
func TestGetUploadToken(t *testing.T) {
134185
cases := []struct {
135186
name string
@@ -169,6 +220,17 @@ func TestGetUploadToken(t *testing.T) {
169220
errContains: []string{"invalid or expired"},
170221
errExcludes: []string{"SAML"},
171222
},
223+
{
224+
// Issue #52: the SSO interstitial's title shows the org's display name,
225+
// not the slug, and the page has no /orgs/<slug>/sso link. Neither the
226+
// stale-session nor the slug-based SAML check can fire; the generic
227+
// auth-interstitial branch must, naming both possible causes.
228+
name: "SSO interstitial with display-name title names both causes, not access",
229+
owner: "acme-inc",
230+
body: `<title>Sign in to Acme Holdings, Inc</title>`,
231+
errContains: []string{"sign-in page", "gh image extract-token", "SAML SSO", "/orgs/acme-inc/sso"},
232+
errExcludes: []string{"you may not have upload access"},
233+
},
172234
{
173235
name: "no token and no interstitial markers gives the generic message",
174236
owner: "octocat",

skills/github-image-upload/SKILL.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ To fetch an attachment rather than post one, `gh image download <user-attachment
155155
| Symptom | Fix |
156156
|---|---|
157157
| `<org> enforces SAML SSO …` | Authorize the session at `https://github.com/orgs/<org>/sso` (lasts ~24h), then retry. Not a permissions problem. |
158-
| `uploadToken not found …` | Usually an expired session, not permissions — read access is enough. Re-authenticate; authorize SSO if the org uses it. |
158+
| `uploadToken not found …` | Expired-session and SSO pages get their own messages, so this likely means no access to the repo — verify the `--repo` value and your access. If both look right, re-authenticate; authorize SSO if the org uses it. |
159159
| No `user_session` cookie found | Log into GitHub in a supported browser, or set `GH_SESSION_TOKEN`. |
160160
| Windows + Chrome 127+ | Cookie-library limitation — use another browser or `GH_SESSION_TOKEN`. |
161161
| CI / headless | Set `GH_SESSION_TOKEN` from a dedicated bot account. |

0 commit comments

Comments
 (0)