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
10 changes: 10 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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
Expand Down
55 changes: 33 additions & 22 deletions internal/data.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package internal

import (
"context"
"errors"
"fmt"
"net/http"

Expand Down Expand Up @@ -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 {
Expand All @@ -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{
Expand Down Expand Up @@ -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...)
Expand All @@ -105,7 +110,7 @@ func (df DataFetcher) FetchData(ctx context.Context, organization string) (*Gith
paginationOpt.Page = resp.NextPage
}
Comment on lines 94 to 111

var allAdminMembers []*github.User
allAdminMembers := make([]*github.User, 0)
memberOpt := &github.ListMembersOptions{
Role: "admin",
ListOptions: github.ListOptions{PerPage: 100},
Expand All @@ -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...)
Expand All @@ -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))
Comment thread
gusfcarvalho marked this conversation as resolved.
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) {
Expand Down Expand Up @@ -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{
Expand Down
137 changes: 137 additions & 0 deletions internal/data_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand All @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion internal/eval_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
28 changes: 19 additions & 9 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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
Comment thread
gusfcarvalho marked this conversation as resolved.
Comment thread
gusfcarvalho marked this conversation as resolved.
}

func main() {
Expand Down
Loading
Loading