Skip to content

Commit 276875f

Browse files
authored
refactor: extract difc.NewComponents to deduplicate DIFC initialization (#4498)
The four-step DIFC initialization sequence (`ParseEnforcementMode` → `NewAgentRegistryWithDefaults` → `NewCapabilities` → `NewEvaluatorWithMode`) was duplicated verbatim in both `server/unified.go` and `proxy/proxy.go`, with silently diverging default modes (`EnforcementStrict` vs `EnforcementFilter`). ## Changes - **`internal/difc/evaluator.go`** — Adds `DIFCComponents` struct and `NewComponents(modeStr string, defaultMode EnforcementMode) DIFCComponents` constructor that encapsulates the full initialization sequence - **`internal/server/unified.go`** — Replaces inline DIFC init with `difc.NewComponents(cfg.DIFCMode, difc.EnforcementStrict)` - **`internal/proxy/proxy.go`** — Replaces inline DIFC init with `difc.NewComponents(cfg.DIFCMode, difc.EnforcementFilter)`, preserving the proxy-specific warning log for invalid mode strings ```go // internal/difc/evaluator.go type DIFCComponents struct { Mode EnforcementMode AgentRegistry *AgentRegistry Capabilities *Capabilities Evaluator *Evaluator } func NewComponents(modeStr string, defaultMode EnforcementMode) DIFCComponents { mode, err := ParseEnforcementMode(modeStr) if err != nil { mode = defaultMode } return DIFCComponents{ Mode: mode, AgentRegistry: NewAgentRegistryWithDefaults(nil, nil), Capabilities: NewCapabilities(), Evaluator: NewEvaluatorWithMode(mode), } } ``` Future changes to the DIFC initialization sequence (e.g. adding a new component) now have a single update point. > [!WARNING] > > <details> > <summary>Firewall rules blocked me from connecting to one or more addresses (expand for details)</summary> > > #### I tried to connect to the following addresses, but was blocked by firewall rules: > > - `example.com` > - Triggering command: `/tmp/go-build2613986933/b513/launcher.test /tmp/go-build2613986933/b513/launcher.test -test.testlogfile=/tmp/go-build2613986933/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build2613986933/b435/vet.cfg _.a -I x_amd64/vet --gdwarf-5 balancer/gracefu-atomic -o x_amd64/vet -W cks...&#34; t.go x_amd64/compile . --gdwarf2 893947/b314/ x_amd64/compile` (dns block) > - Triggering command: `/tmp/go-build1963674460/b509/launcher.test /tmp/go-build1963674460/b509/launcher.test -test.testlogfile=/tmp/go-build1963674460/b509/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true tHjXILjQ_ -buildtags /opt/hostedtoolcache/go/1.25.9/xjson -errorsas -ifaceassert -nilfunc StwtP8fWF_W- logs�� tes.crt aw-mcpg/internal/proxy/graphql_r--log-format 7c330d9c11d84b8d-d -errorsas -ifaceassert -nilfunc ache/go/1.25.9/x-test.timeout=10m0s` (dns block) > - `invalid-host-that-does-not-exist-12345.com` > - Triggering command: `/tmp/go-build2613986933/b495/config.test /tmp/go-build2613986933/b495/config.test -test.testlogfile=/tmp/go-build2613986933/b495/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build2613986933/b368/vet.cfg _.a otection x_amd64/vet --gdwarf-5 credentials -o x_amd64/vet -E w6WVsrdoR -I x_amd64/vet -I .io/auto/sdk -imultiarch x_amd64/vet` (dns block) > - Triggering command: `/tmp/go-build1963674460/b491/config.test /tmp/go-build1963674460/b491/config.test -test.testlogfile=/tmp/go-build1963674460/b491/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true -unreachable=fal--log-format /tmp/go-build261json docker-buildx f19db7ad4b9e741c980b3140ec557422c5d/log.json nwind-tables p/bin/as docker-buildx -ato�� 40a26d5ba8f85386 -buildtags ine by/6e9cb3f11e119/usr/lib/open-iscsi/net-interface-handler -ifaceassert -nilfunc ine` (dns block) > - `nonexistent.local` > - Triggering command: `/tmp/go-build2613986933/b513/launcher.test /tmp/go-build2613986933/b513/launcher.test -test.testlogfile=/tmp/go-build2613986933/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build2613986933/b435/vet.cfg _.a -I x_amd64/vet --gdwarf-5 balancer/gracefu-atomic -o x_amd64/vet -W cks...&#34; t.go x_amd64/compile . --gdwarf2 893947/b314/ x_amd64/compile` (dns block) > - Triggering command: `/tmp/go-build1963674460/b509/launcher.test /tmp/go-build1963674460/b509/launcher.test -test.testlogfile=/tmp/go-build1963674460/b509/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true tHjXILjQ_ -buildtags /opt/hostedtoolcache/go/1.25.9/xjson -errorsas -ifaceassert -nilfunc StwtP8fWF_W- logs�� tes.crt aw-mcpg/internal/proxy/graphql_r--log-format 7c330d9c11d84b8d-d -errorsas -ifaceassert -nilfunc ache/go/1.25.9/x-test.timeout=10m0s` (dns block) > - `slow.example.com` > - Triggering command: `/tmp/go-build2613986933/b513/launcher.test /tmp/go-build2613986933/b513/launcher.test -test.testlogfile=/tmp/go-build2613986933/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build2613986933/b435/vet.cfg _.a -I x_amd64/vet --gdwarf-5 balancer/gracefu-atomic -o x_amd64/vet -W cks...&#34; t.go x_amd64/compile . --gdwarf2 893947/b314/ x_amd64/compile` (dns block) > - Triggering command: `/tmp/go-build1963674460/b509/launcher.test /tmp/go-build1963674460/b509/launcher.test -test.testlogfile=/tmp/go-build1963674460/b509/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true tHjXILjQ_ -buildtags /opt/hostedtoolcache/go/1.25.9/xjson -errorsas -ifaceassert -nilfunc StwtP8fWF_W- logs�� tes.crt aw-mcpg/internal/proxy/graphql_r--log-format 7c330d9c11d84b8d-d -errorsas -ifaceassert -nilfunc ache/go/1.25.9/x-test.timeout=10m0s` (dns block) > - `this-host-does-not-exist-12345.com` > - Triggering command: `/tmp/go-build2613986933/b522/mcp.test /tmp/go-build2613986933/b522/mcp.test -test.testlogfile=/tmp/go-build2613986933/b522/testlog.txt -test.paniconexit0 -test.timeout=10m0s 8939�� cfg elemetry.io/otel-ifaceassert x_amd64/vet --gdwarf-5 telabs/wazero/in/usr/bin/runc -o x_amd64/vet cfg 893947/b432/_pkg_.a -trimpath x_amd64/vet -p /internal/httpco/usr/bin/runc -lang=go1.17 x_amd64/vet` (dns block) > - Triggering command: `/tmp/go-build1963674460/b518/mcp.test /tmp/go-build1963674460/b518/mcp.test -test.testlogfile=/tmp/go-build1963674460/b518/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true y -buildtags` (dns block) > > If you need me to access, download, or install something from one of these locations, you can either: > > - Configure [Actions setup steps](https://gh.io/copilot/actions-setup-steps) to set up my environment, which run before the firewall is enabled > - Add the appropriate URLs or hosts to the custom allowlist in this repository's [Copilot coding agent settings](https://github.com/github/gh-aw-mcpg/settings/copilot/coding_agent) (admins only) > > </details>
2 parents 82f23b7 + d5126a1 commit 276875f

3 files changed

Lines changed: 49 additions & 21 deletions

File tree

internal/difc/evaluator.go

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,37 @@ func ParseEnforcementMode(s string) (EnforcementMode, error) {
8989
}
9090
}
9191

92+
// DIFCComponents holds the set of DIFC objects needed by a server or proxy.
93+
type DIFCComponents struct {
94+
Mode EnforcementMode
95+
AgentRegistry *AgentRegistry
96+
Capabilities *Capabilities
97+
Evaluator *Evaluator
98+
}
99+
100+
// NewComponents initializes the standard DIFC component set and returns it
101+
// together with any parse error.
102+
// When modeStr is empty or cannot be parsed, defaultMode is used and the parse
103+
// error is returned so callers can decide whether to log a warning.
104+
func NewComponents(modeStr string, defaultMode EnforcementMode) (DIFCComponents, error) {
105+
mode := defaultMode
106+
var parseErr error
107+
if modeStr != "" {
108+
parsed, err := ParseEnforcementMode(modeStr)
109+
if err != nil {
110+
parseErr = err
111+
} else {
112+
mode = parsed
113+
}
114+
}
115+
return DIFCComponents{
116+
Mode: mode,
117+
AgentRegistry: NewAgentRegistryWithDefaults(nil, nil),
118+
Capabilities: NewCapabilities(),
119+
Evaluator: NewEvaluatorWithMode(mode),
120+
}, parseErr
121+
}
122+
92123
// AccessDecision represents the result of a DIFC evaluation
93124
type AccessDecision int
94125

internal/proxy/proxy.go

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -100,15 +100,13 @@ func New(ctx context.Context, cfg Config) (*Server, error) {
100100
apiURL = strings.TrimRight(apiURL, "/")
101101
logProxy.Printf("Using upstream GitHub API URL: %s", apiURL)
102102

103-
// Parse enforcement mode
104-
difcMode, err := difc.ParseEnforcementMode(cfg.DIFCMode)
105-
if err != nil {
106-
if cfg.DIFCMode != "" {
107-
log.Printf("[proxy] WARNING: invalid DIFC mode %q, defaulting to filter", cfg.DIFCMode)
108-
}
109-
difcMode = difc.EnforcementFilter // default to filter for proxy
103+
// Initialize DIFC components (defaults to filter mode for the proxy).
104+
// NewComponents returns any parse error so we can warn without parsing twice.
105+
difcComponents, difcParseErr := difc.NewComponents(cfg.DIFCMode, difc.EnforcementFilter)
106+
if difcParseErr != nil {
107+
log.Printf("[proxy] WARNING: invalid DIFC mode %q, defaulting to filter", cfg.DIFCMode)
110108
}
111-
logProxy.Printf("Enforcement mode resolved: %s", difcMode)
109+
logProxy.Printf("Enforcement mode resolved: %s", difcComponents.Mode)
112110

113111
// Load the WASM guard
114112
logProxy.Printf("Loading WASM guard from: %s", cfg.WasmPath)
@@ -120,12 +118,12 @@ func New(ctx context.Context, cfg Config) (*Server, error) {
120118

121119
s := &Server{
122120
guard: g,
123-
evaluator: difc.NewEvaluatorWithMode(difcMode),
124-
agentRegistry: difc.NewAgentRegistryWithDefaults(nil, nil),
125-
capabilities: difc.NewCapabilities(),
121+
evaluator: difcComponents.Evaluator,
122+
agentRegistry: difcComponents.AgentRegistry,
123+
capabilities: difcComponents.Capabilities,
126124
githubToken: cfg.GitHubToken,
127125
githubAPIURL: apiURL,
128-
enforcementMode: difcMode,
126+
enforcementMode: difcComponents.Mode,
129127
httpClient: &http.Client{
130128
Timeout: 60 * time.Second,
131129
Transport: &http.Transport{
@@ -144,7 +142,7 @@ func New(ctx context.Context, cfg Config) (*Server, error) {
144142
logProxy.Printf("No guard policy configured, running without policy enforcement")
145143
}
146144

147-
logProxy.Printf("Proxy server created successfully: mode=%s", difcMode)
145+
logProxy.Printf("Proxy server created successfully: mode=%s", difcComponents.Mode)
148146
return s, nil
149147
}
150148

internal/server/unified.go

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -142,11 +142,10 @@ func NewUnified(ctx context.Context, cfg *config.Config) (*UnifiedServer, error)
142142
logUnified.Printf("Payload configuration: dir=%s, pathPrefix=%s, sizeThreshold=%d bytes (%.2f KB)",
143143
payloadDir, payloadPathPrefix, payloadSizeThreshold, float64(payloadSizeThreshold)/1024)
144144

145-
// Parse DIFC enforcement mode
146-
difcMode, err := difc.ParseEnforcementMode(cfg.DIFCMode)
147-
if err != nil {
148-
// Default to strict mode if not specified or invalid
149-
difcMode = difc.EnforcementStrict
145+
// Initialize DIFC components (defaults to strict mode for the server)
146+
difcComponents, difcParseErr := difc.NewComponents(cfg.DIFCMode, difc.EnforcementStrict)
147+
if difcParseErr != nil {
148+
logger.LogWarn("startup", "invalid DIFC mode %q, defaulting to strict: %v", cfg.DIFCMode, difcParseErr)
150149
}
151150

152151
us := &UnifiedServer{
@@ -164,9 +163,9 @@ func NewUnified(ctx context.Context, cfg *config.Config) (*UnifiedServer, error)
164163

165164
// Initialize DIFC components
166165
guardRegistry: guard.NewRegistry(),
167-
agentRegistry: difc.NewAgentRegistryWithDefaults(nil, nil),
168-
capabilities: difc.NewCapabilities(),
169-
evaluator: difc.NewEvaluatorWithMode(difcMode),
166+
agentRegistry: difcComponents.AgentRegistry,
167+
capabilities: difcComponents.Capabilities,
168+
evaluator: difcComponents.Evaluator,
170169
cfg: cfg, // Store config for guard loading
171170

172171
// Cache tracer at construction to avoid calling otel.Tracer on every request.

0 commit comments

Comments
 (0)