Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions .github/aw/network.md
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,24 @@ Keywords expanding to curated domain lists:
| `lean` | Lean theorem prover | `lean-lang.org`, `elan.lean-lang.org`, `reservoir.lean-lang.org` |
| `python-native` | Python native build deps | Native toolchain mirrors for building Python packages from source |
| `copilot-vendor` | Copilot plan-specific APIs / telemetry | `api.business.githubcopilot.com`, `api.enterprise.githubcopilot.com`, `api.individual.githubcopilot.com`, `telemetry.enterprise.githubcopilot.com` |
| `threat-detection` | Compatibility alias for Copilot threat detection | Copilot API/telemetry hosts, GitHub API/web, `host.docker.internal`, `registry.npmjs.org` |

## Automatic Engine Domain Sets

Each engine automatically receives the domain set it requires in addition to
`network.allowed`. These named sets are maintained by the compiler for analysis
and reporting; they are not valid `network.allowed` identifiers, except for
the legacy `threat-detection` compatibility alias listed above.

| Engine set | Included domains |
|---|---|
| `copilot` | `api.github.com`, `api.githubcopilot.com`, `github.com`, `host.docker.internal`, `raw.githubusercontent.com` |
| `claude` | Anthropic APIs, GitHub transport, certificate/OCSP services, Ubuntu package metadata, Playwright downloads, and `host.docker.internal` |
| `codex` | `172.30.0.1`, `api.github.com`, `api.openai.com`, `chatgpt.com`, `github.com`, `host.docker.internal`, `openai.com` |
| `gemini` | `*.googleapis.com`, `generativelanguage.googleapis.com`, `github.com`, `host.docker.internal`, `raw.githubusercontent.com` |
| `pi` | `api.githubcopilot.com`, `github.com`, `host.docker.internal`, `raw.githubusercontent.com`; provider-scoped models replace the API host with the selected provider endpoint |
| `pi-base` | `github.com`, `host.docker.internal`, `raw.githubusercontent.com`; applied as the provider-independent baseline before a provider prefix is resolved |
| `threat-detection` | Applied automatically only to Copilot threat-detection runs: Copilot API and telemetry hosts, `api.github.com`, `github.com`, `host.docker.internal`, and `registry.npmjs.org` for read-only lockfile validation. External Claude, Codex, Gemini, and other detection runs use their own engine defaults. |

## Invalid Shorthands

Expand Down
46 changes: 46 additions & 0 deletions docs/adr/55461-centralize-engine-default-domain-sets.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
# ADR-55461: Centralize Engine Default Domain Sets

**Date**: 2026-08-24
**Status**: Accepted
**Deciders**: Unknown

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.

Status is Draft — please update to Accepted once the PR is approved and the implementation is verified to be complete and stable.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed in 5e44cf7: updated the ADR status to Accepted and aligned the decision/consequences text with the final embedded-JSON registry plus retained threat-detection compatibility alias.


---

### Context

Engine-specific unconditional domain allow-lists were scattered across multiple separate static Go variables (`CopilotDefaultDomains`, `CodexDefaultDomains`, `ClaudeDefaultDomains`, `GeminiDefaultDomains`, `PiDefaultDomains`, `PiBaseDefaultDomains`) and an `ecosystem_domains.json` file that also served as the user-configurable ecosystem registry. This made it impossible to enumerate or inspect the full set of engine defaults in one place, and it conflated engine-internal allow-lists (automatically injected by the compiler) with user-selectable ecosystem identifiers (explicitly opted into via `network.allowed`). Specifically, the threat-detection allow-list was only represented as an ecosystem entry, obscuring its role as an internal Copilot detection default.

### Decision

We will centralize all domain allow-list data in the embedded `ecosystem_domains.json` file and load it once with `sync.OnceValues`. Engine-specific unconditional domain allow-lists are exposed in Go through an unexported package-level map (`engineDefaultDomainSets`) and a copy-returning public accessor (`GetEngineDefaultDomainSets()`) for analysis and reporting. Existing exported compatibility variables (`CopilotDefaultDomains`, etc.) derive their values from this registry at initialization time via a `copyEngineDefaultDomainSet` helper. The threat-detection allow-list will live in the engine-default registry while the legacy `network.allowed: [threat-detection]` ecosystem alias is retained as a compatibility path.

### Alternatives Considered

#### Alternative 1: Keep separate static variables, add an aggregation function

Maintain each engine's domain list as its own `var` but introduce a function that aggregates them into a map for reporting. This avoids any initialization-time dependency between the registry and the compatibility variables, but leaves the domain lists distributed across the file. It does not solve the fundamental issue of drift between copies and fails to provide the single source of truth needed for the compiler to automatically reference them.

#### Alternative 2: Keep engine domain lists in Go code

Keep the engine domain lists as Go literals and only aggregate them in code. This preserves inline comments next to the data and avoids JSON parsing at package initialization time, but leaves the maintainer workflow split between code and data files and makes it harder to inspect all domain sets together.

### Consequences

#### Positive
- Single authoritative embedded data source for all ecosystem and engine allow-lists prevents content drift between the registry and the exported compatibility variables.
- `GetEngineDefaultDomainSets()` enables programmatic enumeration of all engine domain sets for analysis, reporting, and the new documentation tables in both network reference files.
- Threat-detection domains are represented as an engine-default set while preserving the legacy `network.allowed: [threat-detection]` compatibility alias.
- Immutability is enforced: `GetEngineDefaultDomainSets()` returns deep copies, and exported variables are initialized from copies, so external callers cannot corrupt the registry.

#### Negative
- `engineDefaultDomainSets` is a mutable package-level variable (not a constant), so code in the same package could modify it at runtime; tests must guard against this.
- Exported compatibility variables (`CopilotDefaultDomains`, etc.) now hold a snapshot from package initialization. Code that modifies these variables directly (e.g., in tests) will not affect what `GetEngineDefaultDomainSets()` returns, creating a subtle two-source-of-truth scenario within the package.
- Any future engine whose domain list needs dynamic construction cannot be fully expressed as a plain JSON array and will require refactoring the registry structure.

#### Neutral
- The PR adds `GetEngineDefaultDomainSets()` as a new public API surface. Future callers may depend on it, so the set of keys and the copy semantics become a stability commitment.
- Documentation tables for engine domain sets are now automatically derivable from the registry, but the two network reference files (`docs/src/content/docs/reference/network.md` and `.github/aw/network.md`) are still updated manually — there is no automated sync between the registry and the docs.

---

*ADR finalized after implementation and compatibility review.*
18 changes: 18 additions & 0 deletions docs/src/content/docs/reference/network.md
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ Mix ecosystem identifiers with specific domains for fine-grained control:
| `bazel` | Bazel build system (`releases.bazel.build`, `bcr.bazel.build`) |
| `clojure` | Clojure packages (`clojars.org`) |
| `copilot-vendor` | Plan-specific Copilot API hosts (`api.business.githubcopilot.com`, `api.enterprise.githubcopilot.com`, `api.individual.githubcopilot.com`) and Copilot telemetry (`telemetry.enterprise.githubcopilot.com`) — not enabled by default, since agents route inference through the firewall gateway |
| `threat-detection` | Compatibility alias for Copilot threat-detection network access (`api.githubcopilot.com`, Copilot plan APIs, telemetry, `api.github.com`, `github.com`, `host.docker.internal`, `registry.npmjs.org`) |
| `dart` | Dart/Flutter packages (`pub.dev`, `storage.googleapis.com`) |
| `deno` | Deno runtime (`deno.land`, `jsr.io`, `googleapis.deno.dev`) |
| `dotnet` | NuGet packages and .NET SDK |
Expand Down Expand Up @@ -116,6 +117,23 @@ Mix ecosystem identifiers with specific domains for fine-grained control:
| `swift` | Swift packages (`swift.org`, `cocoapods.org`) |
| `zig` | Zig packages (`ziglang.org`) |

## Automatic Engine Domain Sets

Each engine automatically receives the domain set it requires in addition to
`network.allowed`. These named sets are maintained by the compiler for analysis
and reporting; they are not valid `network.allowed` identifiers, except for
the legacy `threat-detection` compatibility alias listed above.

| Engine set | Included domains |
|---|---|
| `copilot` | `api.github.com`, `api.githubcopilot.com`, `github.com`, `host.docker.internal`, `raw.githubusercontent.com` |
| `claude` | Anthropic APIs, GitHub transport, certificate/OCSP services, Ubuntu package metadata, Playwright downloads, and `host.docker.internal` |
| `codex` | `172.30.0.1`, `api.github.com`, `api.openai.com`, `chatgpt.com`, `github.com`, `host.docker.internal`, `openai.com` |
| `gemini` | `*.googleapis.com`, `generativelanguage.googleapis.com`, `github.com`, `host.docker.internal`, `raw.githubusercontent.com` |

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.

[/grill-with-docs] The pi-base set exists in engineDefaultDomainSets and is exported via GetEngineDefaultDomainSets() / PiBaseDefaultDomains, but it is absent from both doc tables — leaving a gap between the code and the reference docs.

💡 Suggested addition

Add a row after the pi row in both docs/src/content/docs/reference/network.md and .github/aw/network.md:

| `pi-base` | `host.docker.internal`, `github.com`, `raw.githubusercontent.com`; applied as the provider-independent baseline before a provider prefix is resolved |

@copilot please address this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed in 03a3bcb: added the missing pi-base row to both network reference tables.

| `pi` | `api.githubcopilot.com`, `github.com`, `host.docker.internal`, `raw.githubusercontent.com`; provider-scoped models replace the API host with the selected provider endpoint |
| `pi-base` | `github.com`, `host.docker.internal`, `raw.githubusercontent.com`; applied as the provider-independent baseline before a provider prefix is resolved |
| `threat-detection` | Applied automatically only to Copilot threat-detection runs: Copilot API and telemetry hosts, `api.github.com`, `github.com`, `host.docker.internal`, and `registry.npmjs.org` for read-only lockfile validation. External Claude, Codex, Gemini, and other detection runs use their own engine defaults. |

### Ecosystem Identifier Validation

Single-word entries in `network.allowed` that match the ecosystem identifier pattern (`[a-z][a-z0-9-]*`) are validated against the known ecosystem list at compile time. An unrecognized identifier produces a compilation error with the full list of valid options:
Expand Down
Loading