Skip to content

Commit b89714a

Browse files
fix(governance): require explicit ruleset bypass mode
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent 1c2d856 commit b89714a

3 files changed

Lines changed: 94 additions & 2 deletions

File tree

‎pkg/github/__toolsnaps__/create_repository_ruleset.snap‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,8 @@
4242
}
4343
},
4444
"required": [
45-
"actor_type"
45+
"actor_type",
46+
"bypass_mode"
4647
],
4748
"type": "object"
4849
},

‎pkg/github/rulesets.go‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -798,7 +798,7 @@ func rulesetWriteProperties() map[string]*jsonschema.Schema {
798798
Description: "When the specified actor can bypass the ruleset. 'pull_request' only applies to branch rulesets and is not valid for the 'DeployKey' actor type. 'exempt' means rules are not run for that actor and no bypass audit entry is created.",
799799
},
800800
},
801-
Required: []string{"actor_type"},
801+
Required: []string{"actor_type", "bypass_mode"},
802802
},
803803
},
804804
}
@@ -888,6 +888,15 @@ func buildRepositoryRulesetFromArgs(args map[string]any) (github.RepositoryRules
888888
return github.RepositoryRuleset{}, utils.NewToolResultError(fmt.Sprintf("bypass_actors[%d]: unsupported or unrecognized key: %q", i, key))
889889
}
890890
}
891+
bypassMode, ok := actorMap["bypass_mode"].(string)
892+
if !ok {
893+
return github.RepositoryRuleset{}, utils.NewToolResultError(fmt.Sprintf("bypass_actors[%d].bypass_mode is required and must be a string", i))
894+
}
895+
switch github.BypassMode(bypassMode) {
896+
case github.BypassModeAlways, github.BypassModePullRequest, github.BypassModeExempt:
897+
default:
898+
return github.RepositoryRuleset{}, utils.NewToolResultError(fmt.Sprintf("bypass_actors[%d].bypass_mode must be one of \"always\", \"pull_request\", or \"exempt\"", i))
899+
}
891900
}
892901
payload["bypass_actors"] = bypassActorsArr
893902
}

‎pkg/github/rulesets_test.go‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -564,6 +564,11 @@ func Test_CreateRepositoryRuleset(t *testing.T) {
564564

565565
resolvedSchema, err := schema.Resolve(nil)
566566
require.NoError(t, err)
567+
bypassActorSchema := schema.Properties["bypass_actors"].Items
568+
require.NotNil(t, bypassActorSchema)
569+
assert.ElementsMatch(t, []string{"actor_type", "bypass_mode"}, bypassActorSchema.Required)
570+
assert.ElementsMatch(t, []any{"always", "pull_request", "exempt"}, bypassActorSchema.Properties["bypass_mode"].Enum)
571+
567572
validArgs := map[string]any{
568573
"level": "repository",
569574
"owner": "owner",
@@ -581,6 +586,15 @@ func Test_CreateRepositoryRuleset(t *testing.T) {
581586
}
582587
require.NoError(t, resolvedSchema.Validate(validArgs))
583588

589+
t.Run("bypass actor requires an explicit documented mode", func(t *testing.T) {
590+
args := maps.Clone(validArgs)
591+
args["bypass_actors"] = []any{map[string]any{"actor_type": "OrganizationAdmin"}}
592+
require.Error(t, resolvedSchema.Validate(args))
593+
594+
args["bypass_actors"] = []any{map[string]any{"actor_type": "OrganizationAdmin", "bypass_mode": "never"}}
595+
require.Error(t, resolvedSchema.Validate(args))
596+
})
597+
584598
for _, test := range []struct {
585599
field string
586600
typo string
@@ -944,6 +958,74 @@ func Test_CreateRepositoryRuleset(t *testing.T) {
944958
assert.NotEmpty(t, capturedBody)
945959
})
946960

961+
for _, bypassMode := range []string{"always", "pull_request", "exempt"} {
962+
t.Run("bypass_actors serializes "+bypassMode+" exactly", func(t *testing.T) {
963+
var capturedRaw []byte
964+
client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
965+
"POST /repos/{owner}/{repo}/rulesets": func(w http.ResponseWriter, r *http.Request) {
966+
capturedRaw, _ = io.ReadAll(r.Body)
967+
w.WriteHeader(http.StatusCreated)
968+
_, _ = w.Write(capturedRaw)
969+
},
970+
}))
971+
deps := BaseDeps{Client: client}
972+
handler := toolDef.Handler(deps)
973+
request := createMCPRequest(map[string]any{
974+
"level": "repository",
975+
"owner": "owner",
976+
"repo": "repo",
977+
"name": "main protection",
978+
"enforcement": "active",
979+
"rules": []any{map[string]any{"type": "creation"}},
980+
"bypass_actors": []any{
981+
map[string]any{"actor_type": "OrganizationAdmin", "bypass_mode": bypassMode},
982+
},
983+
})
984+
985+
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
986+
require.NoError(t, err)
987+
require.False(t, result.IsError)
988+
989+
var outbound struct {
990+
BypassActors []struct {
991+
BypassMode string `json:"bypass_mode"`
992+
} `json:"bypass_actors"`
993+
}
994+
require.NoError(t, json.Unmarshal(capturedRaw, &outbound))
995+
require.Len(t, outbound.BypassActors, 1)
996+
assert.Equal(t, bypassMode, outbound.BypassActors[0].BypassMode)
997+
})
998+
}
999+
1000+
t.Run("bypass_actors without bypass_mode is rejected before request", func(t *testing.T) {
1001+
called := false
1002+
client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
1003+
"POST /repos/{owner}/{repo}/rulesets": func(w http.ResponseWriter, _ *http.Request) {
1004+
called = true
1005+
w.WriteHeader(http.StatusCreated)
1006+
},
1007+
}))
1008+
deps := BaseDeps{Client: client}
1009+
handler := toolDef.Handler(deps)
1010+
request := createMCPRequest(map[string]any{
1011+
"level": "repository",
1012+
"owner": "owner",
1013+
"repo": "repo",
1014+
"name": "main protection",
1015+
"enforcement": "active",
1016+
"rules": []any{map[string]any{"type": "creation"}},
1017+
"bypass_actors": []any{
1018+
map[string]any{"actor_type": "OrganizationAdmin"},
1019+
},
1020+
})
1021+
1022+
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
1023+
require.NoError(t, err)
1024+
require.True(t, result.IsError)
1025+
assert.Contains(t, getErrorResult(t, result).Text, "bypass_actors[0].bypass_mode")
1026+
assert.False(t, called)
1027+
})
1028+
9471029
t.Run("bypass_actors accepts exempt bypass mode and enterprise actor types", func(t *testing.T) {
9481030
var capturedBody github.RepositoryRuleset
9491031
client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{

0 commit comments

Comments
 (0)