From 083491cc765a378783b296138d7e5f22597502ce Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 1 May 2026 14:53:10 +0000 Subject: [PATCH 1/3] Initial plan From eb710749d0bd9f1689d58aeb63f854aed2e733e0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 1 May 2026 14:57:40 +0000 Subject: [PATCH 2/3] refactor: extract validateIntegrityField helper to eliminate duplicate switch blocks Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/aa47bbd9-ec09-410d-8b1d-a05c155849f6 Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --- internal/guard/wasm_payload.go | 31 ++++++------------------------- internal/guard/wasm_validate.go | 21 +++++++++++++++++++++ 2 files changed, 27 insertions(+), 25 deletions(-) create mode 100644 internal/guard/wasm_validate.go diff --git a/internal/guard/wasm_payload.go b/internal/guard/wasm_payload.go index 1c448f40..73fcd9a7 100644 --- a/internal/guard/wasm_payload.go +++ b/internal/guard/wasm_payload.go @@ -114,15 +114,8 @@ func buildStrictLabelAgentPayload(policy interface{}) (map[string]interface{}, e return nil, fmt.Errorf("invalid repos value: expected all, public, or non-empty array of scoped strings") } - integrity, ok := integrityRaw.(string) - if !ok { - return nil, fmt.Errorf("invalid integrity value: expected one of none|unapproved|approved|merged") - } - - switch strings.ToLower(strings.TrimSpace(integrity)) { - case "none", "unapproved", "approved", "merged": - default: - return nil, fmt.Errorf("invalid integrity value: expected one of none|unapproved|approved|merged") + if err := validateIntegrityField("integrity", integrityRaw); err != nil { + return nil, err } // Validate blocked-users if present: must be a non-empty array of non-empty strings. @@ -199,27 +192,15 @@ func buildStrictLabelAgentPayload(policy interface{}) (map[string]interface{}, e // Validate disapproval-integrity if present. if disIntRaw, ok := allowOnly["disapproval-integrity"]; ok { - disInt, ok := disIntRaw.(string) - if !ok { - return nil, fmt.Errorf("invalid disapproval-integrity value: expected one of none|unapproved|approved|merged") - } - switch strings.ToLower(strings.TrimSpace(disInt)) { - case "none", "unapproved", "approved", "merged": - default: - return nil, fmt.Errorf("invalid disapproval-integrity value: expected one of none|unapproved|approved|merged") + if err := validateIntegrityField("disapproval-integrity", disIntRaw); err != nil { + return nil, err } } // Validate endorser-min-integrity if present. if endMinRaw, ok := allowOnly["endorser-min-integrity"]; ok { - endMin, ok := endMinRaw.(string) - if !ok { - return nil, fmt.Errorf("invalid endorser-min-integrity value: expected one of none|unapproved|approved|merged") - } - switch strings.ToLower(strings.TrimSpace(endMin)) { - case "none", "unapproved", "approved", "merged": - default: - return nil, fmt.Errorf("invalid endorser-min-integrity value: expected one of none|unapproved|approved|merged") + if err := validateIntegrityField("endorser-min-integrity", endMinRaw); err != nil { + return nil, err } } diff --git a/internal/guard/wasm_validate.go b/internal/guard/wasm_validate.go new file mode 100644 index 00000000..343431ad --- /dev/null +++ b/internal/guard/wasm_validate.go @@ -0,0 +1,21 @@ +package guard + +import ( + "fmt" + "strings" +) + +// validateIntegrityField returns an error if raw is not a valid integrity-level +// string. fieldName is used in the error message (e.g. "disapproval-integrity"). +func validateIntegrityField(fieldName string, raw interface{}) error { + s, ok := raw.(string) + if !ok { + return fmt.Errorf("invalid %s value: expected one of none|unapproved|approved|merged", fieldName) + } + switch strings.ToLower(strings.TrimSpace(s)) { + case "none", "unapproved", "approved", "merged": + return nil + default: + return fmt.Errorf("invalid %s value: expected one of none|unapproved|approved|merged", fieldName) + } +} From d1461ddabb6659ea045cb81262f5c58a6ed9170a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 1 May 2026 15:31:11 +0000 Subject: [PATCH 3/3] refactor(guard): centralise integrity levels and use actual field name in error messages Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/e17d0df0-3b26-42a6-82ce-cd488a8c8b44 Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --- internal/guard/wasm_payload.go | 4 +++- internal/guard/wasm_test.go | 2 +- internal/guard/wasm_validate.go | 27 ++++++++++++++++++++++----- 3 files changed, 26 insertions(+), 7 deletions(-) diff --git a/internal/guard/wasm_payload.go b/internal/guard/wasm_payload.go index 73fcd9a7..50dd3555 100644 --- a/internal/guard/wasm_payload.go +++ b/internal/guard/wasm_payload.go @@ -92,8 +92,10 @@ func buildStrictLabelAgentPayload(policy interface{}) (map[string]interface{}, e reposRaw, hasRepos := allowOnly["repos"] integrityRaw, hasIntegrity := allowOnly["min-integrity"] + integrityFieldName := "min-integrity" if !hasIntegrity { integrityRaw, hasIntegrity = allowOnly["integrity"] + integrityFieldName = "integrity" } if !hasRepos || !hasIntegrity { return nil, fmt.Errorf("invalid guard policy transport shape: missing required fields repos and/or min-integrity in allow-only object") @@ -114,7 +116,7 @@ func buildStrictLabelAgentPayload(policy interface{}) (map[string]interface{}, e return nil, fmt.Errorf("invalid repos value: expected all, public, or non-empty array of scoped strings") } - if err := validateIntegrityField("integrity", integrityRaw); err != nil { + if err := validateIntegrityField(integrityFieldName, integrityRaw); err != nil { return nil, err } diff --git a/internal/guard/wasm_test.go b/internal/guard/wasm_test.go index 776501fe..716e50d4 100644 --- a/internal/guard/wasm_test.go +++ b/internal/guard/wasm_test.go @@ -358,7 +358,7 @@ func TestBuildStrictLabelAgentPayloadExtended(t *testing.T) { _, err := buildStrictLabelAgentPayload(policy) require.Error(t, err) - assert.Contains(t, err.Error(), "invalid integrity value") + assert.Contains(t, err.Error(), "invalid min-integrity value") }) t.Run("valid allow-only policy succeeds", func(t *testing.T) { diff --git a/internal/guard/wasm_validate.go b/internal/guard/wasm_validate.go index 343431ad..4db27ae6 100644 --- a/internal/guard/wasm_validate.go +++ b/internal/guard/wasm_validate.go @@ -5,17 +5,34 @@ import ( "strings" ) +// allowedIntegrityLevels is the single source of truth for valid integrity-level values. +var allowedIntegrityLevels = []string{"none", "unapproved", "approved", "merged"} + +var allowedIntegrityLevelSet = map[string]struct{}{ + "none": {}, + "unapproved": {}, + "approved": {}, + "merged": {}, +} + +func invalidIntegrityFieldError(fieldName string) error { + return fmt.Errorf( + "invalid %s value: expected one of %s", + fieldName, + strings.Join(allowedIntegrityLevels, "|"), + ) +} + // validateIntegrityField returns an error if raw is not a valid integrity-level // string. fieldName is used in the error message (e.g. "disapproval-integrity"). func validateIntegrityField(fieldName string, raw interface{}) error { s, ok := raw.(string) if !ok { - return fmt.Errorf("invalid %s value: expected one of none|unapproved|approved|merged", fieldName) + return invalidIntegrityFieldError(fieldName) } - switch strings.ToLower(strings.TrimSpace(s)) { - case "none", "unapproved", "approved", "merged": + normalized := strings.ToLower(strings.TrimSpace(s)) + if _, ok := allowedIntegrityLevelSet[normalized]; ok { return nil - default: - return fmt.Errorf("invalid %s value: expected one of none|unapproved|approved|merged", fieldName) } + return invalidIntegrityFieldError(fieldName) }