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
20 changes: 16 additions & 4 deletions internal/cmd/flags_difc.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,27 @@ func registerGuardsModeCompletion(cmd *cobra.Command) {
}
}

func registerAllowOnlyScopeFlags(cmd *cobra.Command, public *bool, owner, repo, minIntegrity *string) {
cmd.Flags().BoolVar(public, "allowonly-scope-public", envutil.GetEnvBool(config.EnvAllowOnlyScopePublic, false), "Use public AllowOnly scope")
cmd.Flags().StringVar(owner, "allowonly-scope-owner", envutil.GetEnvString(config.EnvAllowOnlyScopeOwner, ""), "AllowOnly owner scope value")
cmd.Flags().StringVar(repo, "allowonly-scope-repo", envutil.GetEnvString(config.EnvAllowOnlyScopeRepo, ""), "AllowOnly repo name (requires owner)")
cmd.Flags().StringVar(minIntegrity, "allowonly-min-integrity", envutil.GetEnvString(config.EnvAllowOnlyMinIntegrity, ""), "AllowOnly integrity: none|unapproved|approved|merged")

// Cobra validates flags that were explicitly changed, rather than their
// resolved values. When either value has an environment-derived default,
// leave value-based validation to BuildAllowOnlyPolicy so CLI flags can
// clear that default while selecting the other scope.
if !envutil.GetEnvBool(config.EnvAllowOnlyScopePublic, false) && envutil.GetEnvString(config.EnvAllowOnlyScopeOwner, "") == "" {
cmd.MarkFlagsMutuallyExclusive("allowonly-scope-public", "allowonly-scope-owner")
}
}

func init() {
RegisterFlag(func(cmd *cobra.Command) {
registerGuardsModeFlag(cmd, &difcMode)
cmd.Flags().StringVar(&difcSinkServerIDs, "guards-sink-server-ids", envutil.GetEnvString("MCP_GATEWAY_GUARDS_SINK_SERVER_IDS", ""), "Comma-separated server IDs whose RPC JSONL logs should include agent secrecy/integrity tag snapshots")
cmd.Flags().StringVar(&guardPolicyJSON, "guard-policy-json", envutil.GetEnvString(config.EnvGuardPolicyJSON, ""), "Guard policy JSON (e.g. {\"allow-only\":{\"repos\":\"public\",\"min-integrity\":\"none\"}})")
cmd.Flags().BoolVar(&allowOnlyPublic, "allowonly-scope-public", envutil.GetEnvBool(config.EnvAllowOnlyScopePublic, false), "Use public AllowOnly scope")
cmd.Flags().StringVar(&allowOnlyOwner, "allowonly-scope-owner", envutil.GetEnvString(config.EnvAllowOnlyScopeOwner, ""), "AllowOnly owner scope value")
cmd.Flags().StringVar(&allowOnlyRepo, "allowonly-scope-repo", envutil.GetEnvString(config.EnvAllowOnlyScopeRepo, ""), "AllowOnly repo name (requires owner)")
cmd.Flags().StringVar(&allowOnlyMinInt, "allowonly-min-integrity", envutil.GetEnvString(config.EnvAllowOnlyMinIntegrity, ""), "AllowOnly integrity: none|unapproved|approved|merged")
registerAllowOnlyScopeFlags(cmd, &allowOnlyPublic, &allowOnlyOwner, &allowOnlyRepo, &allowOnlyMinInt)
})
}

Expand Down
105 changes: 105 additions & 0 deletions internal/cmd/flags_difc_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package cmd

import (
"os"
"testing"

"github.com/github/gh-aw-mcpg/internal/config"
Expand Down Expand Up @@ -198,6 +199,110 @@ func TestParseDIFCSinkServerIDs(t *testing.T) {
}
}

func setupAllowOnlyScopeCmd(t *testing.T) (*cobra.Command, *bool, *string) {
t.Helper()
cmd := &cobra.Command{
Use: "test",
SilenceUsage: true,
RunE: func(cmd *cobra.Command, args []string) error {
return nil
},
}
public := new(bool)
owner := new(string)
registerAllowOnlyScopeFlags(cmd, public, owner, new(string), new(string))
return cmd, public, owner
}

func TestAllowOnlyScopeFlagsMutuallyExclusive(t *testing.T) {
clearEnv := func(t *testing.T) {
t.Helper()
for _, name := range []string{config.EnvAllowOnlyScopePublic, config.EnvAllowOnlyScopeOwner} {
value, wasSet := os.LookupEnv(name)
require.NoError(t, os.Unsetenv(name))
t.Cleanup(func() {
if wasSet {
require.NoError(t, os.Setenv(name, value))
return
}
require.NoError(t, os.Unsetenv(name))
})
}
}

t.Run("public and owner together are rejected by cobra", func(t *testing.T) {
clearEnv(t)
cmd, _, _ := setupAllowOnlyScopeCmd(t)
cmd.SetArgs([]string{"--allowonly-scope-public", "--allowonly-scope-owner", "octocat"})
err := cmd.Execute()
require.Error(t, err, "should fail when both --allowonly-scope-public and --allowonly-scope-owner are provided")
assert.Contains(t, err.Error(), "allowonly-scope-public", "error should mention allowonly-scope-public")
assert.Contains(t, err.Error(), "allowonly-scope-owner", "error should mention allowonly-scope-owner")
})

t.Run("empty owner environment value does not disable cobra validation", func(t *testing.T) {
clearEnv(t)
t.Setenv(config.EnvAllowOnlyScopeOwner, "")
cmd, _, _ := setupAllowOnlyScopeCmd(t)
cmd.SetArgs([]string{"--allowonly-scope-public", "--allowonly-scope-owner", "octocat"})

require.Error(t, cmd.Execute())
})

t.Run("public alone is accepted", func(t *testing.T) {
clearEnv(t)
cmd, _, _ := setupAllowOnlyScopeCmd(t)
cmd.SetArgs([]string{"--allowonly-scope-public"})
err := cmd.Execute()
require.NoError(t, err, "should succeed when only --allowonly-scope-public is provided")
})

t.Run("owner alone is accepted", func(t *testing.T) {
clearEnv(t)
cmd, _, _ := setupAllowOnlyScopeCmd(t)
cmd.SetArgs([]string{"--allowonly-scope-owner", "octocat"})
err := cmd.Execute()
require.NoError(t, err, "should succeed when only --allowonly-scope-owner is provided")
})

t.Run("neither flag is accepted", func(t *testing.T) {
clearEnv(t)
cmd, _, _ := setupAllowOnlyScopeCmd(t)
cmd.SetArgs([]string{})
err := cmd.Execute()
require.NoError(t, err, "should succeed when neither AllowOnly scope flag is provided")
})

t.Run("public environment default can be cleared for owner scope", func(t *testing.T) {
clearEnv(t)
t.Setenv(config.EnvAllowOnlyScopePublic, "true")
cmd, public, owner := setupAllowOnlyScopeCmd(t)
cmd.SetArgs([]string{"--allowonly-scope-public=false", "--allowonly-scope-owner", "octocat"})

require.NoError(t, cmd.Execute())
assert.False(t, *public)
assert.Equal(t, "octocat", *owner)
})

t.Run("owner environment default can be cleared for public scope", func(t *testing.T) {
clearEnv(t)
t.Setenv(config.EnvAllowOnlyScopeOwner, "octocat")
cmd, public, owner := setupAllowOnlyScopeCmd(t)
cmd.SetArgs([]string{"--allowonly-scope-owner=", "--allowonly-scope-public"})

require.NoError(t, cmd.Execute())
assert.True(t, *public)
assert.Empty(t, *owner)
})
}

// TestAllowOnlyScopeFlagsRegistered verifies that the AllowOnly scope flags
// exist on the root command.
func TestAllowOnlyScopeFlagsRegistered(t *testing.T) {
assert.NotNil(t, rootCmd.Flags().Lookup("allowonly-scope-public"), "allowonly-scope-public flag should be registered on rootCmd")
assert.NotNil(t, rootCmd.Flags().Lookup("allowonly-scope-owner"), "allowonly-scope-owner flag should be registered on rootCmd")
}

func TestBuildAllowOnlyPolicy(t *testing.T) {
t.Run("public scope valid", func(t *testing.T) {
policy, err := config.BuildAllowOnlyPolicy(true, "", "", "none")
Expand Down
Loading