diff --git a/pkg/actionpins/data.go b/pkg/actionpins/data.go index e5751dc356e..6a3a9ec5ffb 100644 --- a/pkg/actionpins/data.go +++ b/pkg/actionpins/data.go @@ -71,6 +71,8 @@ func getCachedActionPins() *actionPinsCache { }) if cachedPins == nil { + // Build-time invariant: actionPinsOnce.Do above always assigns cachedPins + // unless loadActionPinsData panicked, so this is unreachable in practice. panic("action pins cache was not initialized") } return cachedPins @@ -87,6 +89,9 @@ func loadActionPinsData(raw []byte) ActionPinsData { var data ActionPinsData if err := json.Unmarshal(raw, &data); err != nil { actionPinsLog.Printf("Failed to unmarshal action pins JSON: %v", err) + // Build-time invariant: data/action_pins.json is embedded at compile time and + // validated by this package's tests; unmarshal can only fail for corrupted + // release data, never dynamic user input. panic(fmt.Sprintf("failed to load action pins: %v", err)) } @@ -95,6 +100,8 @@ func loadActionPinsData(raw []byte) ActionPinsData { } if emptyKeys := collectEntriesWithEmptySHA(data.Entries); len(emptyKeys) > 0 { + // Build-time invariant: an empty SHA in the embedded pin data would produce + // invalid workflow YAML at release time and must be caught before shipping. panic(fmt.Sprintf("action_pins.json has %d entries with empty SHA %v — these would produce invalid workflow YAML (e.g. 'owner/repo@ # version'); remove or fix these entries before releasing", len(emptyKeys), emptyKeys)) } diff --git a/pkg/workflow/agentic_engine.go b/pkg/workflow/agentic_engine.go index d53781b5ca6..924c381aff3 100644 --- a/pkg/workflow/agentic_engine.go +++ b/pkg/workflow/agentic_engine.go @@ -577,6 +577,10 @@ func NewEngineRegistry() *EngineRegistry { } for _, engine := range builtins { if err := registry.Register(engine); err != nil { + // Build-time invariant: Register only rejects a negative + // dedicatedLLMGatewayPort, and every built-in engine above is constructed + // with a compile-time constant port, so this is a programming error caught + // by CI (TestBuiltinEnginesRegisterWithoutError), never user input. panic(fmt.Sprintf("BUG: failed to register built-in engine: %v", err)) } } diff --git a/pkg/workflow/mcp_setup_gateway.go b/pkg/workflow/mcp_setup_gateway.go index 64577d52159..6fcfb3115c8 100644 --- a/pkg/workflow/mcp_setup_gateway.go +++ b/pkg/workflow/mcp_setup_gateway.go @@ -130,6 +130,8 @@ func writeMCPGatewayStepEnvWithCustomGatewayEnvNames(yaml *strings.Builder, mcpE if len(customEnvVarNames) > 0 { customEnvNamesJSON, err := json.Marshal(customEnvVarNames) if err != nil { + // Build-time invariant: customEnvVarNames is a []string, which json.Marshal + // always serialises successfully; this branch is unreachable in practice. panic(fmt.Sprintf("BUG: failed to marshal MCP gateway environment variable names: %v", err)) } yaml.WriteString(formatYAMLEnv(" ", mcpGatewayCustomEnvNamesVar, string(customEnvNamesJSON))) diff --git a/pkg/workflow/mcp_setup_safe_outputs.go b/pkg/workflow/mcp_setup_safe_outputs.go index 502590d4eb6..33346f1ce38 100644 --- a/pkg/workflow/mcp_setup_safe_outputs.go +++ b/pkg/workflow/mcp_setup_safe_outputs.go @@ -56,6 +56,9 @@ func generateSafeOutputsSetup(c *Compiler, yaml *strings.Builder, safeOutputConf }}, }) if err != nil { + // Build-time invariant: fileRenderConfig above is a fixed struct literal of + // strings, which json.Marshal always serialises successfully; this branch is + // unreachable in practice. panic(fmt.Sprintf("BUG: failed to marshal generated file render config: %v", err)) } yaml.WriteString(" - name: Generate Safe Outputs Config\n") diff --git a/pkg/workflow/model_aliases.go b/pkg/workflow/model_aliases.go index 201fa071d09..f4027d7daa9 100644 --- a/pkg/workflow/model_aliases.go +++ b/pkg/workflow/model_aliases.go @@ -54,7 +54,7 @@ func loadBuiltinModelAliases() (map[string][]string, error) { return builtinModelAliasesLoader.Get(func() (map[string][]string, error) { var data builtinModelAliasesFile if err := json.Unmarshal(builtinModelAliasesJSON, &data); err != nil { - return nil, fmt.Errorf("BUG: workflow: failed to parse embedded model_aliases.json: %w (try 'make build' to rebuild with the latest data)", err) + return nil, fmt.Errorf("BUG: workflow: could not parse embedded model_aliases.json: %w (expected valid JSON matching the aliases schema; run 'make build' to rebuild with the latest data)", err) } return data.Aliases, nil }) @@ -88,6 +88,9 @@ func getBuiltinOnlyAliasMap() map[string][]string { builtinOnlyAliasMapOnce.Do(func() { data, err := loadBuiltinModelAliases() if err != nil { + // Build-time invariant: model_aliases.json is embedded at compile time and + // validated by TestBuiltinModelAliases; a real unmarshal failure here can only + // follow a corrupted release build, never dynamic user input. panic(err) } builtinOnlyAliasMap = data @@ -145,6 +148,9 @@ func isBuiltinOnlyAliasMap(m map[string][]string) bool { func BuiltinModelAliases() map[string][]string { data, err := loadBuiltinModelAliases() if err != nil { + // Build-time invariant: model_aliases.json is embedded at compile time and + // validated by TestBuiltinModelAliases; a real unmarshal failure here can only + // follow a corrupted release build, never dynamic user input. panic(err) } // Return a fresh deep copy so callers may freely modify map entries and slices. diff --git a/pkg/workflow/panic_invariants_test.go b/pkg/workflow/panic_invariants_test.go new file mode 100644 index 00000000000..bb6bc0f5d9a --- /dev/null +++ b/pkg/workflow/panic_invariants_test.go @@ -0,0 +1,85 @@ +//go:build !integration + +package workflow + +import ( + "encoding/json" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// The tests in this file are executable guardrails for the "build-time invariant" +// panic sites documented in this package. Each test exercises the real code path +// that precedes a panic() so that a refactor which makes the panic reachable fails +// the test suite instead of only surfacing at runtime. + +// TestEmbeddedModelAliasesAreLoadable guards the panics in getBuiltinOnlyAliasMap +// and BuiltinModelAliases, which fire only when the embedded model_aliases.json +// cannot be unmarshaled. +func TestEmbeddedModelAliasesAreLoadable(t *testing.T) { + aliases, err := loadBuiltinModelAliases() + require.NoError(t, err, "embedded model_aliases.json must unmarshal at build time") + assert.NotEmpty(t, aliases) + + assert.NotEmpty(t, getBuiltinOnlyAliasMap()) + assert.NotEmpty(t, BuiltinModelAliases()) +} + +// TestEmbeddedToolsetPermissionsAreLoadable guards the panic in +// getToolsetPermissionsMap, which fires only when the embedded GitHub toolsets +// JSON cannot be unmarshaled. +func TestEmbeddedToolsetPermissionsAreLoadable(t *testing.T) { + assert.NotEmpty(t, getToolsetPermissionsMap()) +} + +// TestBuiltinEnginesRegisterWithoutError guards the panic in NewEngineRegistry. +// Register only rejects a negative dedicatedLLMGatewayPort, so re-registering every +// built-in engine asserts exactly the failure mode the panic guards against. +func TestBuiltinEnginesRegisterWithoutError(t *testing.T) { + registry := NewEngineRegistry() + + for _, id := range registry.GetSupportedEngines() { + engine, err := registry.GetEngine(id) + require.NoError(t, err) + assert.NoError(t, NewEngineRegistry().Register(engine), "built-in engine %q must register without error", id) + } +} + +// TestBuildPiModelsJSONMarshalsRuntimeValues guards the panic in buildPiModelsJSON. +// The payload mixes literals with runtime arguments, so this asserts that those +// argument types always serialise. +func TestBuildPiModelsJSONMarshalsRuntimeValues(t *testing.T) { + out := buildPiModelsJSON(4141, "COPILOT_GITHUB_TOKEN", "claude-sonnet-4-20250514") + + var decoded map[string]any + require.NoError(t, json.Unmarshal([]byte(out), &decoded)) + assert.Contains(t, decoded, "providers") +} + +// TestMCPGatewayCustomEnvNamesMarshal guards the panic in +// writeMCPGatewayStepEnvWithCustomGatewayEnvNames, which fires only if the +// []string of env var names fails to marshal. +func TestMCPGatewayCustomEnvNamesMarshal(t *testing.T) { + var stepEnv strings.Builder + gatewayEnv := map[string]string{"CUSTOM_ONE": "value-one", "CUSTOM_TWO": "value-two"} + writeMCPGatewayStepEnvForTest(&stepEnv, nil, nil, gatewayEnv) + + assert.Contains(t, stepEnv.String(), mcpGatewayCustomEnvNamesVar) + assert.Contains(t, stepEnv.String(), `[\"CUSTOM_ONE\",\"CUSTOM_TWO\"]`) +} + +// TestFileRenderConfigMarshals guards the panic in generateSafeOutputsSetup, which +// fires only if the fixed fileRenderConfig struct fails to marshal. +func TestFileRenderConfigMarshals(t *testing.T) { + out, err := json.Marshal(fileRenderConfig{ + Files: []fileRenderItem{{ + Path: "safeoutputs/config.json", + ContentEnv: "GH_AW_SAFE_OUTPUTS_CONFIG", + }}, + }) + require.NoError(t, err) + assert.JSONEq(t, `{"files":[{"path":"safeoutputs/config.json","content_env":"GH_AW_SAFE_OUTPUTS_CONFIG"}]}`, string(out)) +} diff --git a/pkg/workflow/permissions_toolset_data.go b/pkg/workflow/permissions_toolset_data.go index d99eb78560d..90f5a4ca294 100644 --- a/pkg/workflow/permissions_toolset_data.go +++ b/pkg/workflow/permissions_toolset_data.go @@ -41,6 +41,9 @@ var getToolsetPermissionsMap = sync.OnceValue(func() map[string]GitHubToolsetPer var data GitHubToolsetsData if err := json.Unmarshal(githubToolsetsPermissionsJSON, &data); err != nil { + // Build-time invariant: the embedded GitHub toolsets JSON is validated by + // TestToolsetPermissionsLoadedFromJSON; unmarshal can only fail for corrupted + // release data, never dynamic user input. panic(fmt.Sprintf("BUG: failed to load GitHub toolsets permissions from JSON: %v", err)) } diff --git a/pkg/workflow/pi_engine.go b/pkg/workflow/pi_engine.go index 7ece31d33b1..45429f89c42 100644 --- a/pkg/workflow/pi_engine.go +++ b/pkg/workflow/pi_engine.go @@ -144,8 +144,9 @@ func buildPiModelsJSON(gatewayPort int, secretEnvVarName, modelID string) string } b, err := json.Marshal(payload) if err != nil { - // json.Marshal only fails for non-serialisable types; our map is always - // serialisable, so this branch is unreachable in practice. + // Build-time invariant: the payload holds only strings, maps and slices — + // json.Marshal only errors for non-serialisable types such as channels or + // functions, so no runtime argument value can reach this branch. panic(fmt.Sprintf("BUG: buildPiModelsJSON failed to marshal JSON: %v", err)) } return string(b)