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