Add egress control through allowlist to prevent source code exfiltration by v-abhishekbhaskar · Pull Request #230 · dependabot/proxy

v-abhishekbhaskar · 2026-09-02T06:48:41Z

What are you trying to accomplish?

Add domain-based egress control to the proxy to help prevent source-code exfiltration during Dependabot update jobs. Even when an attacker abuses a legitimately-configured registry token (e.g. Artifactory/Nexus), traffic to unknown hosts like evil.com should be visible and, eventually, blockable — something the earlier Method-header approach could not address.

This PR lands the observe (log-only) foundation plus the per-job dynamic allowlist needed before enforcement can be turned on: it builds a per-job allowlist and logs any request to a host not on it, without changing traffic behavior by default.

Anything you want to highlight for special attention from reviewers?

Two independent experiment flags (not a config block): proxy_egress_observe (log non-allowlisted requests, still allow) and proxy_egress_enforce (drop with 403, matching the existing blockMetadataAPIHosts response). Both default false ⇒ fail-open, so existing jobs and GHES are unaffected. Using experiments lets us roll this out gradually without a config-schema change.
Static base allowlist (union, no PACKAGE_MANAGER dependency): GitHub/Dependabot infrastructure domains plus the union of every ecosystem's default registry/CDN hosts, applied to every job. This is deliberate — launchers don't reliably set PACKAGE_MANAGER (the action doesn't; the CLI needed patching), and multi-ecosystem jobs need registries beyond their nominal package manager. All entries are trusted public registries, so the containment trade-off is negligible versus the exfil protection (unknown hosts are still blocked).
Domains live in an embedded YAML (internal/handlers/egress_allowlist_defaults.yaml, //go:embed) parsed at init(). The per-ecosystem keyed map is retained there for provenance/documentation; the handler consumes the deduped, sorted union. A &docker_registries YAML anchor shares the container-registry set across docker, docker_compose, devcontainers, and helm.
Dynamic per-job hosts (derived once from cfg.Credentials at construction — no per-request cost):
- Configured registries: the host of every URL-bearing credential field (host, url, index-url, registry, api-host, repo, endpoint, replaces-base), so a job's private registries (Artifactory/Nexus/private npm, etc.) are never treated as exfiltration.
- OIDC token-exchange endpoints: the identity-provider auth hosts the proxy itself contacts when a credential is OIDC-configured — Azure (login.microsoftonline.com), AWS (sts.amazonaws.com), GCP (sts/iamcredentials/www.googleapis.com), Cloudsmith (api-host or api.cloudsmith.io). Detection mirrors the identifying keys in oidc.CreateOIDCCredential; the eventual registry target is already covered by the credential-host extraction.
- Backend-supplied domains escape hatch: an explicit per-credential domains list is merged verbatim (bare hosts or leading-dot suffix patterns) for redirect/CDN targets the proxy can't derive.
Boundary-safe matching: helpers.HostMatchesDomain uses a leading-dot suffix check so one entry covers subdomains (.pkg.dev → us-docker.pkg.dev) while lookalikes like evilnpmjs.org are rejected. Exact entries and the suffix form both normalize an absolute DNS name (trailing dot), so registry.npmjs.org. matches.
Handler placement: registered in proxy.go right after request logging and before all credential-injecting handlers, so drops/logs happen before any auth is added. Works for HTTP and MITM'd HTTPS.
No metrics in this PR. Per-host counting was intentionally dropped (high cardinality / wrong tool for "top hosts"); allowlist tuning uses the observe logs (* egress not allowlisted <host>) via the logging pipeline / Kusto.

How will you know you've accomplished your goal?

Unit: HostMatchesDomain boundary matrix (subdomains, case/trailing-dot normalization, lookalike negatives); handler flag matrix (fail-open, observe-logs-allows, enforce-403, observe+enforce); GitHub-infra always allowed; union covers every ecosystem's defaults regardless of PACKAGE_MANAGER; absolute-FQDN exact match; embedded-YAML load/dedup guard; collector separates two hosts into distinct series.
Integration (proxy_test.go): end-to-end through MITM — non-allowlisted host returns 403 under enforce, and 200 + a egress not allowlisted <host> log line under observe (asserted from captured logger output).
Full suite green: go build ./..., go test ./..., gofmt, and yamllint all pass.

Rollout plan

Observe (this PR): collect logs of non-allowlisted hosts per ecosystem.
Verify the dynamic allowlist (configured registries + OIDC + backend domains) covers real jobs — confirm from logs that enforce would not have broken anything.
Enforce per-ecosystem by flipping proxy_egress_enforce (already wired).

Checklist

I have run the complete test suite to ensure all tests and linters pass.
I have thoroughly tested my code changes to ensure they work as expected, including adding additional tests for new functionality.
I have written clear and descriptive commit messages.
I have provided a detailed description of the changes in the pull request, including the problem it addresses, how it fixes the problem, and any relevant details about the implementation.
I have ensured that the code is well-documented and easy to understand.

Copilot

Copilot review overview

🟡 Changes recommended

The configuration contract is inconsistent, per-host metrics aggregate incorrectly, and exact hostname normalization is incomplete.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Balanced
Findings: 3 · 1

New issues introduced by this change (4)

Severity	Finding
	`internal/handlers/egress_allowlist.go` — The advertised config contract is not implemented here: the PR description says callers enable…
	`internal/handlers/egress_allowlist.go` — This does not produce per-host counts with the real metrics client. `CollectorClient.SendMetric`…
	`internal/handlers/egress_allowlist.go` — Exact allowlist entries do not receive the trailing-dot normalization provided by…
	`proxy_test.go` — This integration test claims to cover observe-mode logging but only checks that traffic is allowed,…

What changed in this PR

Adds experiment-gated, domain-based egress monitoring and enforcement to the proxy.

Changes:

Adds static ecosystem allowlists and boundary-safe domain matching.
Logs, measures, or blocks non-allowlisted requests.
Adds unit and MITM integration coverage.

File	Description
`.gitignore`	Ignores VS Code settings.
`proxy.go`	Registers the egress handler.
`proxy_test.go`	Adds proxy integration tests.
`internal/helpers/helpers.go`	Adds domain-suffix matching.
`internal/helpers/helpers_test.go`	Tests domain matching boundaries.
`internal/handlers/egress_allowlist.go`	Implements egress handling.
`internal/handlers/egress_allowlist_test.go`	Tests handler modes and metrics.
`internal/handlers/egress_allowlist_defaults.go`	Defines ecosystem allowlists.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+func TestProxyEgressAllowlistObserveAllows(t *testing.T) {
+	upstream := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusOK)
+	}))
+	defer upstream.Close()
+
+	cfg := &config.Config{
+		CA:          testProxyConfig.CA,
+		Experiments: config.Experiments{"proxy_egress_observe": true},
+	}
+	env := config.ProxyEnvSettings{PackageManager: "npm_and_yarn"}
+	client, proxy := testProxyServerWithEnv(t, env, cfg, nil, upstream.Certificate())
+	closeOnCleanup(t, proxy)
+
+	// Observe mode logs the non-allowlisted host but still lets it through.
+	req, err := http.NewRequestWithContext(t.Context(), http.MethodGet, upstream.URL, nil)
+	require.NoError(t, err)
+	rsp, err := client.Do(req)
+	require.NoError(t, err)
+	defer func() {
+		require.NoError(t, rsp.Body.Close())
+	}()
+	assert.Equal(t, http.StatusOK, rsp.StatusCode)
+}


…d enforcing

…rring from credentials

v-abhishekbhaskar self-assigned this Sep 2, 2026

v-abhishekbhaskar marked this pull request as ready for review September 8, 2026 06:12

v-abhishekbhaskar requested a review from a team as a code owner September 8, 2026 06:12

Copilot AI balanced review requested due to automatic review settings September 8, 2026 06:12

Copilot started reviewing on behalf of v-abhishekbhaskar September 8, 2026 06:13 View session

Copilot AI reviewed Sep 8, 2026

View reviewed changes

v-robaiken reviewed Sep 8, 2026

View reviewed changes

v-abhishekbhaskar mentioned this pull request Sep 8, 2026

Set PACKAGE_MANAGER on the proxy container github/dependabot-action#1773

Closed

v-abhishekbhaskar added 7 commits September 9, 2026 16:22

add egress allowlist functionality

a1b77e8

delete egress control implementation plan doc

4b190ef

add missing ecosystems and update subdomain suffix matcher

a9785ea

add non-allowlisted domain metrics and add experiments for logging an…

2870b43

…d enforcing

add remaining ecosystems to allowlist

394cfda

remve indexing by package_manager and move allowlist to yaml file

cc59549

remove egress_not_allowlisted_count metric and add dynamic hosts infe…

ed312a3

…rring from credentials

honeyankit force-pushed the abhishekbhaskar/add-egress-allowlist-log-only branch from 1d05f73 to ed312a3 Compare September 9, 2026 23:22

remove functionality for supporting backend-supplied domains

f6b9100

v-robaiken approved these changes Sep 10, 2026

View reviewed changes

v-kbukum1 approved these changes Sep 10, 2026

View reviewed changes

v-abhishekbhaskar merged commit a72f522 into main Sep 10, 2026
210 of 213 checks passed

v-abhishekbhaskar deleted the abhishekbhaskar/add-egress-allowlist-log-only branch September 10, 2026 17:30

Navigation Menu

Repository navigation

Add egress control through allowlist to prevent source code exfiltration - #230

Add egress control through allowlist to prevent source code exfiltration#230
v-abhishekbhaskar merged 8 commits into
mainfrom
abhishekbhaskar/add-egress-allowlist-log-only

v-abhishekbhaskar commented Sep 2, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

v-abhishekbhaskar commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What are you trying to accomplish?

Anything you want to highlight for special attention from reviewers?

How will you know you've accomplished your goal?

Rollout plan

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

v-abhishekbhaskar commented Sep 2, 2026 •

edited

Loading