From 58dddbe3543c84d5a88dcb9a0204b21c2081d25b Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Thu, 12 Nov 2026 19:53:13 -0300 Subject: [PATCH] fix(security): send GITHUB_TOKEN only to GitHub's own hosts A Feature given as a tarball URL got `Authorization: Bearer $GITHUB_TOKEN` whenever its URL started with "https://github.com" or "https://api.github.com", so github.com.attacker.example, github.com@attacker.example and api.github.com.evil all received the token. Anyone who could put a Feature URL in a devcontainer.json, or in a Feature's dependsOn, could collect it. The token now goes only to an https URL with no userinfo whose host is github.com or api.github.com, ignoring case and one trailing dot, on any port. The reference CLI has the same prefix check, so this is a deliberate divergence, recorded in docs/DIVERGENCES.md and pinned by unit tests, including one where a TLS server reached through a userinfo look-alike receives no token. Co-Authored-By: Claude Opus 5.5 --- docs/DIVERGENCES.md | 17 +++++ internal/cli/feature_install.go | 13 +++- internal/cli/feature_tarball_test.go | 99 ++++++++++++++++++++++++++-- 3 files changed, 123 insertions(+), 6 deletions(-) diff --git a/docs/DIVERGENCES.md b/docs/DIVERGENCES.md index e7cbeb9..30733bf 100644 --- a/docs/DIVERGENCES.md +++ b/docs/DIVERGENCES.md @@ -66,6 +66,23 @@ touches a compared surface, reflected in the parity matrix. (GHSA-vh4v-2xq2-g5cg). Hardening off is therefore already safe here; the diagnostics still report what hardening *would* change, so the flag remains a faithful compatibility probe. +- **`GITHUB_TOKEN` is sent only to GitHub's own hosts when a Feature is a tarball URL.** + TS `getRequestHeaders` adds `Authorization: Bearer $GITHUB_TOKEN` to any direct-tarball + URI that starts with `https://github.com` or `https://api.github.com`, so it also hands + the token to `https://github.com.attacker.example/…`, `https://github.com@attacker.example/…` + (userinfo) and `https://api.github.com.evil/…`: anyone who can put a Feature URL in a + devcontainer.json, or in a Feature's `dependsOn`, can collect it. This CLI sends the + token, and logs `Using environment GITHUB_TOKEN.` / `No environment GITHUB_TOKEN + available.`, only when the URL parses, its scheme is `https`, it has no userinfo and its + host is exactly `github.com` or `api.github.com`; for any other host it sends no token + and logs neither line. Matching ignores case and one trailing dot, and allows any port, + as TS does (TS normalizes the id with `new URL(...)`, which lowercases the host): these + still name GitHub's host, and TLS verification binds the token to GitHub's certificate. + No parity case can compare this with TS without leaking a token, so unit tests pin it: + `TestIsGitHubTarballURI`, `TestFeatureRequestHeadersSendTheTokenOnlyToGitHub` (header and + log lines for real and look-alike hosts) and + `TestDownloadFeatureTarballKeepsTheTokenFromALookAlikeURL` (a TLS server reached through + `https://github.com@/…` receives no token). - **`BUILDKIT_INLINE_CACHE=1`** is omitted when `--cache-to` is an inline exporter (`/type\s*=\s*inline/i`), matching TS `isBuildxCacheToInline` — a parity fix over the earlier unconditional build-arg. diff --git a/internal/cli/feature_install.go b/internal/cli/feature_install.go index 6d2f1ff..9b1135f 100644 --- a/internal/cli/feature_install.go +++ b/internal/cli/feature_install.go @@ -973,8 +973,19 @@ func featureRequestHeaders(tarballURL string, env map[string]string, logger log. return headers } +// isGitHubTarballURI reports whether GITHUB_TOKEN may be sent to u. TS checks +// only that the URI starts with "https://github.com" or "https://api.github.com", +// which also matches github.com.attacker.example and github.com@attacker.example; +// this CLI requires the host itself to be GitHub's (docs/DIVERGENCES.md). Case, +// a trailing dot and a port are allowed as in TS: they name the same host, and +// TLS verification still binds the token to GitHub's certificate. func isGitHubTarballURI(u string) bool { - return strings.HasPrefix(u, "https://github.com") || strings.HasPrefix(u, "https://api.github.com") + parsed, err := url.Parse(u) + if err != nil || parsed.Scheme != "https" || parsed.User != nil { + return false + } + host := strings.TrimSuffix(strings.ToLower(parsed.Hostname()), ".") + return host == "github.com" || host == "api.github.com" } func isPlainHTTPURL(u string) bool { diff --git a/internal/cli/feature_tarball_test.go b/internal/cli/feature_tarball_test.go index 7cc2242..df978cd 100644 --- a/internal/cli/feature_tarball_test.go +++ b/internal/cli/feature_tarball_test.go @@ -1,11 +1,15 @@ package cli import ( + "bytes" "context" "crypto/sha256" "encoding/hex" + "encoding/pem" "net/http" "net/http/httptest" + "os" + "path/filepath" "strings" "testing" @@ -15,11 +19,25 @@ import ( func TestIsGitHubTarballURI(t *testing.T) { cases := map[string]bool{ - "https://github.com/owner/repo/releases/download/v1/feat.tgz": true, - "https://api.github.com/repos/owner/repo/releases/assets/1": true, - "https://raw.githubusercontent.com/owner/repo/main/feat.tgz": false, - "https://example.com/feat.tgz": false, - "http://github.com/owner/repo/feat.tgz": false, + "https://github.com/owner/repo/releases/download/v1/feat.tgz": true, + "https://api.github.com/repos/owner/repo/releases/assets/1": true, + "https://raw.githubusercontent.com/owner/repo/main/feat.tgz": false, + "https://example.com/feat.tgz": false, + "http://github.com/owner/repo/feat.tgz": false, + "https://GitHub.COM/owner/repo/feat.tgz": true, + "HTTPS://API.github.com/repos/owner/repo/releases/assets/1": true, + "https://github.com./owner/repo/feat.tgz": true, + "https://github.com:443/owner/repo/feat.tgz": true, + "https://github.com.attacker.example/devcontainer-feature-x.tgz": false, + "https://github.com@attacker.example/devcontainer-feature-x.tgz": false, + "https://github.com:pw@attacker.example/feat.tgz": false, + "https://user@github.com/owner/repo/feat.tgz": false, + "https://api.github.com.evil/devcontainer-feature-x.tgz": false, + "https://github.company.example/feat.tgz": false, + "https://evilgithub.com/feat.tgz": false, + "http://api.github.com/repos/owner/repo/releases/assets/1": false, + "https://github.com\\@attacker.example/feat.tgz": false, + "github.com/owner/repo/feat.tgz": false, } for u, want := range cases { if got := isGitHubTarballURI(u); got != want { @@ -70,6 +88,77 @@ func TestFeatureRequestHeaders(t *testing.T) { } } +// GITHUB_TOKEN goes only to GitHub's own hosts. TS sends it to every URI that +// starts with "https://github.com" or "https://api.github.com", look-alike +// hosts included (docs/DIVERGENCES.md); no parity case can compare that +// without leaking a token, so these cases pin it. +func TestFeatureRequestHeadersSendTheTokenOnlyToGitHub(t *testing.T) { + cases := []struct { + url string + github bool + }{ + {"https://github.com/o/r/releases/download/v1/devcontainer-feature-x.tgz", true}, + {"https://api.github.com/repos/o/r/releases/assets/1", true}, + {"https://GitHub.com/o/r/devcontainer-feature-x.tgz", true}, + {"https://github.com.attacker.example/devcontainer-feature-x.tgz", false}, + {"https://github.com@attacker.example/devcontainer-feature-x.tgz", false}, + {"https://api.github.com.evil/devcontainer-feature-x.tgz", false}, + {"http://github.com/o/r/devcontainer-feature-x.tgz", false}, + } + for _, c := range cases { + for _, token := range []string{"secret", ""} { + var logged bytes.Buffer + logger := log.New(log.Options{Writer: &logged, Format: "json"}) + h := featureRequestHeaders(c.url, map[string]string{"GITHUB_TOKEN": token}, logger) + + wantAuth, wantLog := "", "" + if c.github && token != "" { + wantAuth, wantLog = "Bearer secret", "Using environment GITHUB_TOKEN." + } else if c.github { + wantLog = "No environment GITHUB_TOKEN available." + } + if got := h["Authorization"]; got != wantAuth { + t.Errorf("%s (token %q): Authorization = %q, want %q", c.url, token, got, wantAuth) + } + if h["User-Agent"] != "devcontainer" { + t.Errorf("%s: User-Agent = %q, want devcontainer", c.url, h["User-Agent"]) + } + if wantLog != "" && !strings.Contains(logged.String(), wantLog) { + t.Errorf("%s (token %q): log %q lacks %q", c.url, token, logged.String(), wantLog) + } + if wantLog == "" && strings.Contains(logged.String(), "GITHUB_TOKEN") { + t.Errorf("%s (token %q): log mentions GITHUB_TOKEN: %q", c.url, token, logged.String()) + } + } + } +} + +// A URL whose userinfo is "github.com" reaches another server; that server +// must not see the token. +func TestDownloadFeatureTarballKeepsTheTokenFromALookAlikeURL(t *testing.T) { + var gotAuth string + srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotAuth = r.Header.Get("Authorization") + w.Write([]byte("tgz")) + })) + defer srv.Close() + caFile := filepath.Join(t.TempDir(), "ca.pem") + certPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: srv.Certificate().Raw}) + if err := os.WriteFile(caFile, certPEM, 0o600); err != nil { + t.Fatal(err) + } + t.Setenv("NODE_EXTRA_CA_CERTS", "") + t.Setenv("SSL_CERT_FILE", caFile) + + lookAlike := "https://github.com@" + strings.TrimPrefix(srv.URL, "https://") + "/devcontainer-feature-x.tgz" + if _, _, err := downloadFeatureTarball(t.Context(), nil, lookAlike, map[string]string{"GITHUB_TOKEN": "secret"}); err != nil { + t.Fatal(err) + } + if strings.Contains(gotAuth, "Bearer") || strings.Contains(gotAuth, "secret") { + t.Errorf("look-alike host received the token: Authorization = %q", gotAuth) + } +} + func TestDownloadFeatureTarball_HeadersAndDigest(t *testing.T) { payload := []byte("fake-tarball-bytes") var gotUA, gotAuth string