fix: allowlist checks (#10)

* fix: allowlist checks

Signed-off-by: Gustavo Carvalho <gustavo.carvalho@container-solutions.com>

* fix: copilot issues

Signed-off-by: Gustavo Carvalho <gustavo.carvalho@container-solutions.com>

---------

Signed-off-by: Gustavo Carvalho <gustavo.carvalho@container-solutions.com>

Author: gusfcarvalho

README.md

@@ -11,6 +11,14 @@ For the moment, it is solely limited to authenticated Github organizations with
   - `read:org` for the organization to be queried. Note - you _might_ need to be an administrator of the GH Org to work correctly
   - `read:members` to be able to read teams
 
+### IP allow-list data
+
+GitHub organization IP allow-list collection is disabled by default. Enable it only when the plugin is configured with a Classic Personal Access Token with organization administrator permissions. Classic PATs are broad credentials, so prefer leaving `collect_ip_allow_list` set to `false` unless IP allow-list policies are required.
+
+Fine-grained Personal Access Tokens and GitHub App installation tokens may be able to read other organization settings, but GitHub does not currently allow them to query `organization.ipAllowListEntries` through GraphQL.
+
+If IP allow-list data cannot be fetched, the plugin continues with partial data and policies that depend on `ip_allow_list` should report a skip reason instead of evaluating incomplete evidence.
+
 ## Building
 
 Once you are ready to serve the plugin, you need to build the binaries which can be used by the agent.
@@ -37,6 +45,7 @@ In the example above, setting an empty token, and an environment variable `CCF_P
 
 ```shell
 export CCF_PLUGINS_GITHUB_CONFIG_TOKEN="github_pat_1234..."
+export CCF_PLUGINS_GITHUB_CONFIG_COLLECT_IP_ALLOW_LIST="false"
 ```
 
 ```yaml
@@ -46,6 +55,7 @@ plugins:
     config:
       token: "" # Will be read from the CCF_PLUGINS_GITHUB_CONFIG_TOKEN environment variable
       organization: test-org # The name of the organization
+      collect_ip_allow_list: false # Set to true only when using a Classic PAT and IP allow-list evidence is required
 ```
 
 ## Releasing

go.mod

@@ -8,6 +8,7 @@ require (
 	github.com/hashicorp/go-hclog v1.6.3
 	github.com/hashicorp/go-plugin v1.7.0
 	github.com/mitchellh/mapstructure v1.5.0
+	github.com/open-policy-agent/opa v1.14.1
 )
 
 require (
@@ -36,7 +37,6 @@ require (
 	github.com/mattn/go-colorable v0.1.14 // indirect
 	github.com/mattn/go-isatty v0.0.20 // indirect
 	github.com/oklog/run v1.2.0 // indirect
-	github.com/open-policy-agent/opa v1.14.1 // indirect
 	github.com/rcrowley/go-metrics v0.0.0-20250401214520-65e299d6c5c9 // indirect
 	github.com/segmentio/asm v1.2.1 // indirect
 	github.com/sirupsen/logrus v1.9.4 // indirect

internal/data.go

@@ -2,6 +2,7 @@ package internal
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"net/http"
 
@@ -29,7 +30,7 @@ type GithubData struct {
 	Teams       []*github.Team       `json:"teams"`
 	Members     []*github.User       `json:"members"`
 	SSO         *OrgSSO              `json:"sso"`
-	IPAllowList []IPAllowListEntry   `json:"ip_allow_list"`
+	IPAllowList *[]IPAllowListEntry  `json:"ip_allow_list"`
 }
 
 type DataFetcher struct {
@@ -44,7 +45,7 @@ func NewDataFetcher(logger hclog.Logger, client *github.Client) *DataFetcher {
 	}
 }
 
-func (df DataFetcher) FetchData(ctx context.Context, organization string) (*GithubData, []*proto.Step, error) {
+func (df DataFetcher) FetchData(ctx context.Context, organization string, collectIPAllowList bool) (*GithubData, []*proto.Step, error) {
 	steps := make([]*proto.Step, 0)
 
 	steps = append(steps, &proto.Step{
@@ -76,26 +77,30 @@ func (df DataFetcher) FetchData(ctx context.Context, organization string) (*Gith
 		Remarks:     policy_manager.Pointer("More information: https://docs.github.com/en/enterprise-cloud@latest/organizations/managing-saml-single-sign-on-for-your-organization/about-identity-and-access-management-with-saml-single-sign-on"),
 	})
 
-	steps = append(steps, &proto.Step{
-		Title:       "Get IP Allow-List",
-		Description: "Fetches the IP allow-list entries for the organization via the GitHub GraphQL API",
-		Remarks:     policy_manager.Pointer("More information: https://docs.github.com/en/graphql/reference/objects#ipallowlistentry"),
-	})
+	if collectIPAllowList {
+		steps = append(steps, &proto.Step{
+			Title:       "Get IP Allow-List",
+			Description: "Fetches the IP allow-list entries for the organization via the GitHub GraphQL API",
+			Remarks:     policy_manager.Pointer("More information: https://docs.github.com/en/graphql/reference/objects#ipallowlistentry"),
+		})
+	}
 
 	org, _, err := df.client.Organizations.Get(ctx, organization)
 	if err != nil {
 		df.logger.Error("Error getting organization information", "org", organization, "error", err)
 		return nil, nil, err
 	}
 
-	var allTeams []*github.Team
+	var accumulatedErrors error
+	allTeams := make([]*github.Team, 0)
 	paginationOpt := &github.ListOptions{PerPage: 100}
 
 	for {
 		teams, resp, err := df.client.Teams.ListTeams(ctx, organization, paginationOpt)
 		if err != nil {
-			df.logger.Error("Error getting teams information", "org", organization, "error", err)
-			return nil, nil, err
+			df.logger.Warn("Skipping teams collection after GitHub API error", "org", organization, "error", err)
+			accumulatedErrors = errors.Join(accumulatedErrors, fmt.Errorf("failed to fetch teams: %w", err))
+			break
 		}
 
 		allTeams = append(allTeams, teams...)
@@ -105,7 +110,7 @@ func (df DataFetcher) FetchData(ctx context.Context, organization string) (*Gith
 		paginationOpt.Page = resp.NextPage
 	}
 
-	var allAdminMembers []*github.User
+	allAdminMembers := make([]*github.User, 0)
 	memberOpt := &github.ListMembersOptions{
 		Role:        "admin",
 		ListOptions: github.ListOptions{PerPage: 100},
@@ -114,8 +119,9 @@ func (df DataFetcher) FetchData(ctx context.Context, organization string) (*Gith
 	for {
 		members, resp, err := df.client.Organizations.ListMembers(ctx, organization, memberOpt)
 		if err != nil {
-			df.logger.Error("Error getting admin members", "org", organization, "error", err)
-			return nil, nil, err
+			df.logger.Warn("Skipping admin member collection after GitHub API error", "org", organization, "error", err)
+			accumulatedErrors = errors.Join(accumulatedErrors, fmt.Errorf("failed to fetch admin members: %w", err))
+			break
 		}
 
 		allAdminMembers = append(allAdminMembers, members...)
@@ -127,23 +133,28 @@ func (df DataFetcher) FetchData(ctx context.Context, organization string) (*Gith
 
 	ssoData, err := df.fetchSSO(ctx, organization)
 	if err != nil {
-		df.logger.Error("Error getting SSO configuration", "org", organization, "error", err)
-		return nil, nil, err
+		df.logger.Warn("Skipping SSO collection after GitHub API error", "org", organization, "error", err)
+		accumulatedErrors = errors.Join(accumulatedErrors, fmt.Errorf("failed to fetch SSO configuration: %w", err))
+		ssoData = nil
 	}
 
-	ipAllowList, err := df.fetchIPAllowList(ctx, organization)
-	if err != nil {
-		df.logger.Error("Error getting IP allow-list", "org", organization, "error", err)
-		return nil, nil, err
+	var ipAllowList []IPAllowListEntry
+	if collectIPAllowList {
+		ipAllowList, err = df.fetchIPAllowList(ctx, organization)
+		if err != nil {
+			df.logger.Warn("Skipping IP allow-list collection after GitHub API error", "org", organization, "error", err)
+			accumulatedErrors = errors.Join(accumulatedErrors, fmt.Errorf("failed to fetch IP allow-list. Please confirm a Classic token PAT is used to gather IP allowlist information: %w", err))
+			ipAllowList = nil
+		}
 	}
 
 	return &GithubData{
 		Settings:    org,
 		Teams:       allTeams,
 		Members:     allAdminMembers,
 		SSO:         ssoData,
-		IPAllowList: ipAllowList,
-	}, steps, nil
+		IPAllowList: &ipAllowList,
+	}, steps, accumulatedErrors
 }
 
 func (df DataFetcher) fetchSSO(ctx context.Context, organization string) (*OrgSSO, error) {
@@ -233,7 +244,7 @@ func (df DataFetcher) fetchIPAllowList(ctx context.Context, organization string)
 		}
 	}`
 
-	var entries []IPAllowListEntry
+	entries := make([]IPAllowListEntry, 0)
 	var after *string
 	for {
 		gqlQuery := graphqlRequest{

internal/data_test.go

@@ -10,6 +10,7 @@ import (
 
 	"github.com/google/go-github/v71/github"
 	"github.com/hashicorp/go-hclog"
+	"github.com/open-policy-agent/opa/v1/rego"
 )
 
 func testGithubClient(t *testing.T, handler http.Handler) (*github.Client, func()) {
@@ -27,6 +28,142 @@ func testGithubClient(t *testing.T, handler http.Handler) (*github.Client, func(
 	return client, server.Close
 }
 
+func TestFetchDataReturnsErrorWhenOrganizationFetchFails(t *testing.T) {
+	client, cleanup := testGithubClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		http.Error(w, "organization unavailable", http.StatusInternalServerError)
+	}))
+	defer cleanup()
+
+	fetcher := NewDataFetcher(hclog.NewNullLogger(), client)
+	data, steps, err := fetcher.FetchData(context.Background(), "acme", false)
+	if err == nil {
+		t.Fatal("FetchData should return an error when the required organization endpoint fails")
+	}
+	if data != nil {
+		t.Fatalf("data = %#v, want nil", data)
+	}
+	if steps != nil {
+		t.Fatalf("steps = %#v, want nil", steps)
+	}
+}
+
+func TestFetchDataSkipsOptionalCollectionErrors(t *testing.T) {
+	optionalRequests := make(map[string]int)
+	client, cleanup := testGithubClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		switch r.URL.Path {
+		case "/orgs/acme":
+			w.Header().Set("Content-Type", "application/json")
+			_, _ = w.Write([]byte(`{"login":"acme","name":"Acme","url":"https://api.github.com/orgs/acme"}`))
+		case "/orgs/acme/teams", "/orgs/acme/members", "/orgs/acme/sso", "/graphql":
+			optionalRequests[r.URL.Path]++
+			http.Error(w, "optional data unavailable", http.StatusInternalServerError)
+		default:
+			t.Fatalf("unexpected request path %s", r.URL.Path)
+		}
+	}))
+	defer cleanup()
+
+	fetcher := NewDataFetcher(hclog.NewNullLogger(), client)
+	data, steps, err := fetcher.FetchData(context.Background(), "acme", true)
+	if err == nil {
+		t.Fatal("FetchData should return accumulated errors when optional collections fail")
+	}
+	if data == nil {
+		t.Fatal("data should be returned when only optional collection fails")
+	}
+	if data.Settings.GetLogin() != "acme" {
+		t.Fatalf("organization login = %q, want acme", data.Settings.GetLogin())
+	}
+	if len(data.Teams) != 0 {
+		t.Fatalf("len(Teams) = %d, want 0", len(data.Teams))
+	}
+	if len(data.Members) != 0 {
+		t.Fatalf("len(Members) = %d, want 0", len(data.Members))
+	}
+	if data.SSO != nil {
+		t.Fatalf("SSO = %#v, want nil", data.SSO)
+	}
+	if data.IPAllowList == nil {
+		t.Fatal("IPAllowList pointer should be set")
+	}
+	if *data.IPAllowList != nil {
+		t.Fatalf("IPAllowList = %#v, want nil slice for skipped collection", *data.IPAllowList)
+	}
+	if len(steps) == 0 {
+		t.Fatal("steps should still describe the collection activity")
+	}
+	for _, path := range []string{"/orgs/acme/teams", "/orgs/acme/members", "/orgs/acme/sso", "/graphql"} {
+		if optionalRequests[path] != 1 {
+			t.Fatalf("%s requests = %d, want 1", path, optionalRequests[path])
+		}
+	}
+}
+
+func TestFetchDataDoesNotCollectIPAllowListWhenDisabled(t *testing.T) {
+	graphqlRequests := 0
+	client, cleanup := testGithubClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		switch r.URL.Path {
+		case "/orgs/acme":
+			_, _ = w.Write([]byte(`{"login":"acme","name":"Acme","url":"https://api.github.com/orgs/acme"}`))
+		case "/orgs/acme/teams", "/orgs/acme/members":
+			_, _ = w.Write([]byte(`[]`))
+		case "/orgs/acme/sso":
+			http.NotFound(w, r)
+		case "/graphql":
+			graphqlRequests++
+			t.Fatalf("GraphQL IP allow-list request should not be made when collection is disabled")
+		default:
+			t.Fatalf("unexpected request path %s", r.URL.Path)
+		}
+	}))
+	defer cleanup()
+
+	fetcher := NewDataFetcher(hclog.NewNullLogger(), client)
+	data, steps, err := fetcher.FetchData(context.Background(), "acme", false)
+	if err != nil {
+		t.Fatalf("FetchData returned error: %v", err)
+	}
+	if data.IPAllowList == nil {
+		t.Fatal("IPAllowList pointer should be set")
+	}
+	if *data.IPAllowList != nil {
+		t.Fatalf("IPAllowList = %#v, want nil slice when collection is disabled", *data.IPAllowList)
+	}
+	encoded, err := json.Marshal(data)
+	if err != nil {
+		t.Fatalf("marshaling GithubData: %v", err)
+	}
+	var payload map[string]interface{}
+	if err := json.Unmarshal(encoded, &payload); err != nil {
+		t.Fatalf("unmarshaling GithubData: %v", err)
+	}
+	if value, ok := payload["ip_allow_list"]; !ok || value != nil {
+		t.Fatalf("JSON ip_allow_list = %#v, present = %v, want present null", value, ok)
+	}
+	evaluation, err := rego.New(
+		rego.Query("input.ip_allow_list"),
+		rego.Input(data),
+	).Eval(context.Background())
+	if err != nil {
+		t.Fatalf("evaluating OPA input: %v", err)
+	}
+	if len(evaluation) != 1 || len(evaluation[0].Expressions) != 1 {
+		t.Fatalf("OPA evaluation = %#v, want one null expression", evaluation)
+	}
+	if evaluation[0].Expressions[0].Value != nil {
+		t.Fatalf("OPA input.ip_allow_list = %#v, want null", evaluation[0].Expressions[0].Value)
+	}
+	if graphqlRequests != 0 {
+		t.Fatalf("GraphQL requests = %d, want 0", graphqlRequests)
+	}
+	for _, step := range steps {
+		if step.Title == "Get IP Allow-List" {
+			t.Fatal("IP allow-list collection step should not be present when collection is disabled")
+		}
+	}
+}
+
 func TestFetchSSOUsesRelativeURL(t *testing.T) {
 	client, cleanup := testGithubClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		if r.Method != http.MethodGet {

internal/eval_integration_test.go

@@ -21,7 +21,7 @@ func TestDataFetcher_FetchData(t *testing.T) {
 			client: github.NewClient(nil).WithAuthToken(os.Getenv("GITHUB_TOKEN")),
 		}
 
-		org, _, err := fetcher.FetchData(ctx, "compliance-framework")
+		org, _, err := fetcher.FetchData(ctx, "compliance-framework", false)
 		if err != nil {
 			t.Error(err)
 		}

main.go

@@ -15,8 +15,9 @@ import (
 )
 
 type PluginConfig struct {
-	Token        string `mapstructure:"token"`
-	Organization string `mapstructure:"organization"`
+	Token              string `mapstructure:"token"`
+	Organization       string `mapstructure:"organization"`
+	CollectIPAllowList bool   `mapstructure:"collect_ip_allow_list"`
 }
 
 type Validator interface {
@@ -72,7 +73,7 @@ func (l *CompliancePlugin) Configure(req *proto.ConfigureRequest) (*proto.Config
 	// In this method, you should save any configuration values to your plugin struct, so you can later
 	// re-use them in PrepareForEval and Eval.
 	config := &PluginConfig{}
-	err := mapstructure.Decode(req.GetConfig(), config)
+	err := mapstructure.WeakDecode(req.GetConfig(), config)
 	if err != nil {
 		l.logger.Error("Configuration cannot be decoded. Ensure the correct data has been passed.")
 		return nil, err
@@ -127,11 +128,16 @@ func (l *CompliancePlugin) Eval(request *proto.EvalRequest, apiHelper runner.Api
 
 	dataFetcher := internal.NewDataFetcher(l.logger, l.githubClient)
 
-	data, collectSteps, err := dataFetcher.FetchData(ctx, l.config.Organization)
-	if err != nil {
-		return &proto.EvalResponse{
-			Status: proto.ExecutionStatus_FAILURE,
-		}, fmt.Errorf("failed to fetch data: %w", err)
+	data, collectSteps, dataErr := dataFetcher.FetchData(ctx, l.config.Organization, l.config.CollectIPAllowList)
+	if dataErr != nil {
+		l.logger.Warn("Completed with non-fatal data collection errors", "error", dataErr)
+		// Continue with partial data - errors are accumulated but not fatal
+		// Policies will use skip_reason for fields that couldn't be fetched
+		if data == nil {
+			return &proto.EvalResponse{
+				Status: proto.ExecutionStatus_FAILURE,
+			}, fmt.Errorf("failed to fetch data: %w", dataErr)
+		}
 	}
 
 	stepActivities := append(activities, &proto.Activity{
@@ -163,7 +169,11 @@ func (l *CompliancePlugin) Eval(request *proto.EvalRequest, apiHelper runner.Api
 		Status: evalStatus,
 	}
 
-	return resp, nil
+	if dataErr != nil {
+		resp.Status = proto.ExecutionStatus_FAILURE
+	}
+
+	return resp, dataErr
 }
 
 func main() {