From 5542e63c62e3c4a296932ddeaa7e4a9ec7da57f1 Mon Sep 17 00:00:00 2001 From: Gustavo Carvalho Date: Tue, 5 May 2026 09:51:24 -0300 Subject: [PATCH 1/2] feat: add checks for ip whitelist feat: add checks for sso fix: use new configurations endpoint for dependabot and secret scanning Signed-off-by: Gustavo Carvalho --- internal/data.go | 178 +++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 174 insertions(+), 4 deletions(-) diff --git a/internal/data.go b/internal/data.go index 4d0869f..3cdc0cb 100644 --- a/internal/data.go +++ b/internal/data.go @@ -2,6 +2,8 @@ package internal import ( "context" + "fmt" + "net/http" policy_manager "github.com/compliance-framework/agent/policy-manager" @@ -10,9 +12,24 @@ import ( "github.com/hashicorp/go-hclog" ) +type OrgSSO struct { + Enabled bool `json:"enabled"` + SSOURL string `json:"sso_url"` + IDPIssuer string `json:"idp_issuer"` +} + +type IPAllowListEntry struct { + AllowListValue string `json:"allow_list_value"` + IsActive bool `json:"is_active"` + Name string `json:"name"` +} + type GithubData struct { - Settings *github.Organization `json:"settings"` - Teams []*github.Team `json:"teams"` + Settings *github.Organization `json:"settings"` + Teams []*github.Team `json:"teams"` + Members []*github.User `json:"members"` + SSO *OrgSSO `json:"sso"` + IPAllowList []IPAllowListEntry `json:"ip_allow_list"` } type DataFetcher struct { @@ -47,6 +64,24 @@ func (df DataFetcher) FetchData(ctx context.Context, organization string) (*Gith Remarks: policy_manager.Pointer("More information about data being sent back can be found here: https://docs.github.com/en/rest/teams/teams?apiVersion=2022-11-28#list-teams"), }) + steps = append(steps, &proto.Step{ + Title: "Get Admin Members", + Description: "Using the client's native APIs, list organization members with the owner/admin role", + Remarks: policy_manager.Pointer("More information about data being sent back can be found here: https://docs.github.com/en/rest/orgs/members?apiVersion=2022-11-28#list-organization-members"), + }) + + steps = append(steps, &proto.Step{ + Title: "Get SSO Configuration", + Description: "Fetches the SAML SSO configuration for the organization to verify identity provider enforcement", + Remarks: policy_manager.Pointer("More information: https://docs.github.com/en/rest/orgs/orgs?apiVersion=2022-11-28#get-an-organization"), + }) + + 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) @@ -70,8 +105,143 @@ func (df DataFetcher) FetchData(ctx context.Context, organization string) (*Gith paginationOpt.Page = resp.NextPage } + var allAdminMembers []*github.User + memberOpt := &github.ListMembersOptions{ + Role: "admin", + ListOptions: github.ListOptions{PerPage: 100}, + } + + 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 + } + + allAdminMembers = append(allAdminMembers, members...) + if resp.NextPage == 0 { + break + } + memberOpt.Page = resp.NextPage + } + + ssoData, err := df.fetchSSO(ctx, organization) + if err != nil { + df.logger.Warn("Could not fetch SSO configuration; marking SSO as disabled", "org", organization, "error", err) + ssoData = &OrgSSO{Enabled: false} + } + + var ipAllowList []IPAllowListEntry + if org.Plan != nil && org.Plan.Name != nil && *org.Plan.Name == "enterprise" { + ipAllowList, err = df.fetchIPAllowList(ctx, organization) + if err != nil { + df.logger.Warn("Could not fetch IP allow-list; treating as empty", "org", organization, "error", err) + ipAllowList = []IPAllowListEntry{} + } + } else { + df.logger.Info("Skipping IP allow-list fetch: requires GitHub Enterprise Cloud", "org", organization, "plan", org.Plan) + ipAllowList = []IPAllowListEntry{} + } + return &GithubData{ - Settings: org, - Teams: allTeams, + Settings: org, + Teams: allTeams, + Members: allAdminMembers, + SSO: ssoData, + IPAllowList: ipAllowList, }, steps, nil } + +func (df DataFetcher) fetchSSO(ctx context.Context, organization string) (*OrgSSO, error) { + type samlIdentityProvider struct { + SSOURL string `json:"sso_url"` + Issuer string `json:"issuer"` + IDPCertID string `json:"idp_cert_fingerprint"` + } + type ssoResponse struct { + SAMLIdentityProvider *samlIdentityProvider `json:"saml_identity_provider"` + } + + url := fmt.Sprintf("https://api.github.com/orgs/%s/sso", organization) + req, err := df.client.NewRequest(http.MethodGet, url, nil) + if err != nil { + return nil, fmt.Errorf("building SSO request: %w", err) + } + + var ssoResp ssoResponse + httpResp, err := df.client.Do(ctx, req, &ssoResp) + if err != nil { + if httpResp != nil && httpResp.StatusCode == http.StatusNotFound { + return &OrgSSO{Enabled: false}, nil + } + return nil, fmt.Errorf("fetching SSO config: %w", err) + } + + if ssoResp.SAMLIdentityProvider == nil { + return &OrgSSO{Enabled: false}, nil + } + + return &OrgSSO{ + Enabled: true, + SSOURL: ssoResp.SAMLIdentityProvider.SSOURL, + IDPIssuer: ssoResp.SAMLIdentityProvider.Issuer, + }, nil +} + +func (df DataFetcher) fetchIPAllowList(ctx context.Context, organization string) ([]IPAllowListEntry, error) { + type graphqlRequest struct { + Query string `json:"query"` + } + type ipAllowListEntryNode struct { + AllowListValue string `json:"allowListValue"` + IsActive bool `json:"isActive"` + Name string `json:"name"` + } + type ipAllowListEdge struct { + Node ipAllowListEntryNode `json:"node"` + } + type ipAllowListConnection struct { + Edges []ipAllowListEdge `json:"edges"` + } + type orgNode struct { + IPAllowListEntries ipAllowListConnection `json:"ipAllowListEntries"` + } + type graphqlData struct { + Organization orgNode `json:"organization"` + } + type graphqlResponse struct { + Data graphqlData `json:"data"` + Errors []struct { + Message string `json:"message"` + } `json:"errors"` + } + + gqlQuery := graphqlRequest{ + Query: fmt.Sprintf(`{ organization(login: "%s") { ipAllowListEntries(first: 100) { edges { node { allowListValue isActive name } } } } }`, organization), + } + + req, err := df.client.NewRequest(http.MethodPost, "https://api.github.com/graphql", gqlQuery) + if err != nil { + return nil, fmt.Errorf("building IP allow-list GraphQL request: %w", err) + } + + var gqlResp graphqlResponse + _, err = df.client.Do(ctx, req, &gqlResp) + if err != nil { + return nil, fmt.Errorf("executing IP allow-list GraphQL query: %w", err) + } + + if len(gqlResp.Errors) > 0 { + return nil, fmt.Errorf("GraphQL error: %s", gqlResp.Errors[0].Message) + } + + entries := make([]IPAllowListEntry, 0, len(gqlResp.Data.Organization.IPAllowListEntries.Edges)) + for _, edge := range gqlResp.Data.Organization.IPAllowListEntries.Edges { + entries = append(entries, IPAllowListEntry{ + AllowListValue: edge.Node.AllowListValue, + IsActive: edge.Node.IsActive, + Name: edge.Node.Name, + }) + } + return entries, nil +} From e0fb5d75168cd122bd9b649e2bcd3848d7c08841 Mon Sep 17 00:00:00 2001 From: Gustavo Carvalho Date: Tue, 5 May 2026 11:11:43 -0300 Subject: [PATCH 2/2] fix: copilot issues Signed-off-by: Gustavo Carvalho --- internal/data.go | 109 ++++++++++++++++++++----------- internal/data_test.go | 146 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 217 insertions(+), 38 deletions(-) create mode 100644 internal/data_test.go diff --git a/internal/data.go b/internal/data.go index 3cdc0cb..dcb9149 100644 --- a/internal/data.go +++ b/internal/data.go @@ -73,7 +73,7 @@ func (df DataFetcher) FetchData(ctx context.Context, organization string) (*Gith steps = append(steps, &proto.Step{ Title: "Get SSO Configuration", Description: "Fetches the SAML SSO configuration for the organization to verify identity provider enforcement", - Remarks: policy_manager.Pointer("More information: https://docs.github.com/en/rest/orgs/orgs?apiVersion=2022-11-28#get-an-organization"), + 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{ @@ -127,20 +127,14 @@ func (df DataFetcher) FetchData(ctx context.Context, organization string) (*Gith ssoData, err := df.fetchSSO(ctx, organization) if err != nil { - df.logger.Warn("Could not fetch SSO configuration; marking SSO as disabled", "org", organization, "error", err) - ssoData = &OrgSSO{Enabled: false} + df.logger.Error("Error getting SSO configuration", "org", organization, "error", err) + return nil, nil, err } - var ipAllowList []IPAllowListEntry - if org.Plan != nil && org.Plan.Name != nil && *org.Plan.Name == "enterprise" { - ipAllowList, err = df.fetchIPAllowList(ctx, organization) - if err != nil { - df.logger.Warn("Could not fetch IP allow-list; treating as empty", "org", organization, "error", err) - ipAllowList = []IPAllowListEntry{} - } - } else { - df.logger.Info("Skipping IP allow-list fetch: requires GitHub Enterprise Cloud", "org", organization, "plan", org.Plan) - ipAllowList = []IPAllowListEntry{} + 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 } return &GithubData{ @@ -162,7 +156,7 @@ func (df DataFetcher) fetchSSO(ctx context.Context, organization string) (*OrgSS SAMLIdentityProvider *samlIdentityProvider `json:"saml_identity_provider"` } - url := fmt.Sprintf("https://api.github.com/orgs/%s/sso", organization) + url := fmt.Sprintf("orgs/%s/sso", organization) req, err := df.client.NewRequest(http.MethodGet, url, nil) if err != nil { return nil, fmt.Errorf("building SSO request: %w", err) @@ -190,7 +184,8 @@ func (df DataFetcher) fetchSSO(ctx context.Context, organization string) (*OrgSS func (df DataFetcher) fetchIPAllowList(ctx context.Context, organization string) ([]IPAllowListEntry, error) { type graphqlRequest struct { - Query string `json:"query"` + Query string `json:"query"` + Variables map[string]interface{} `json:"variables"` } type ipAllowListEntryNode struct { AllowListValue string `json:"allowListValue"` @@ -201,7 +196,11 @@ func (df DataFetcher) fetchIPAllowList(ctx context.Context, organization string) Node ipAllowListEntryNode `json:"node"` } type ipAllowListConnection struct { - Edges []ipAllowListEdge `json:"edges"` + Edges []ipAllowListEdge `json:"edges"` + PageInfo struct { + HasNextPage bool `json:"hasNextPage"` + EndCursor *string `json:"endCursor"` + } `json:"pageInfo"` } type orgNode struct { IPAllowListEntries ipAllowListConnection `json:"ipAllowListEntries"` @@ -216,32 +215,66 @@ func (df DataFetcher) fetchIPAllowList(ctx context.Context, organization string) } `json:"errors"` } - gqlQuery := graphqlRequest{ - Query: fmt.Sprintf(`{ organization(login: "%s") { ipAllowListEntries(first: 100) { edges { node { allowListValue isActive name } } } } }`, organization), - } + query := `query($login: String!, $after: String) { + organization(login: $login) { + ipAllowListEntries(first: 100, after: $after) { + edges { + node { + allowListValue + isActive + name + } + } + pageInfo { + hasNextPage + endCursor + } + } + } + }` - req, err := df.client.NewRequest(http.MethodPost, "https://api.github.com/graphql", gqlQuery) - if err != nil { - return nil, fmt.Errorf("building IP allow-list GraphQL request: %w", err) - } + var entries []IPAllowListEntry + var after *string + for { + gqlQuery := graphqlRequest{ + Query: query, + Variables: map[string]interface{}{ + "login": organization, + "after": after, + }, + } - var gqlResp graphqlResponse - _, err = df.client.Do(ctx, req, &gqlResp) - if err != nil { - return nil, fmt.Errorf("executing IP allow-list GraphQL query: %w", err) - } + req, err := df.client.NewRequest(http.MethodPost, "graphql", gqlQuery) + if err != nil { + return nil, fmt.Errorf("building IP allow-list GraphQL request: %w", err) + } - if len(gqlResp.Errors) > 0 { - return nil, fmt.Errorf("GraphQL error: %s", gqlResp.Errors[0].Message) - } + var gqlResp graphqlResponse + _, err = df.client.Do(ctx, req, &gqlResp) + if err != nil { + return nil, fmt.Errorf("executing IP allow-list GraphQL query: %w", err) + } + + if len(gqlResp.Errors) > 0 { + return nil, fmt.Errorf("GraphQL error: %s", gqlResp.Errors[0].Message) + } + + connection := gqlResp.Data.Organization.IPAllowListEntries + for _, edge := range connection.Edges { + entries = append(entries, IPAllowListEntry{ + AllowListValue: edge.Node.AllowListValue, + IsActive: edge.Node.IsActive, + Name: edge.Node.Name, + }) + } - entries := make([]IPAllowListEntry, 0, len(gqlResp.Data.Organization.IPAllowListEntries.Edges)) - for _, edge := range gqlResp.Data.Organization.IPAllowListEntries.Edges { - entries = append(entries, IPAllowListEntry{ - AllowListValue: edge.Node.AllowListValue, - IsActive: edge.Node.IsActive, - Name: edge.Node.Name, - }) + if !connection.PageInfo.HasNextPage { + break + } + if connection.PageInfo.EndCursor == nil { + return nil, fmt.Errorf("GraphQL response indicated another IP allow-list page without an end cursor") + } + after = connection.PageInfo.EndCursor } return entries, nil } diff --git a/internal/data_test.go b/internal/data_test.go new file mode 100644 index 0000000..2ae5130 --- /dev/null +++ b/internal/data_test.go @@ -0,0 +1,146 @@ +package internal + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "net/url" + "testing" + + "github.com/google/go-github/v71/github" + "github.com/hashicorp/go-hclog" +) + +func testGithubClient(t *testing.T, handler http.Handler) (*github.Client, func()) { + t.Helper() + + server := httptest.NewServer(handler) + client := github.NewClient(server.Client()) + + baseURL, err := url.Parse(server.URL + "/") + if err != nil { + t.Fatalf("parsing test server URL: %v", err) + } + client.BaseURL = baseURL + + return client, server.Close +} + +func TestFetchSSOUsesRelativeURL(t *testing.T) { + client, cleanup := testGithubClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet { + t.Fatalf("method = %s, want GET", r.Method) + } + if r.URL.Path != "/orgs/acme/sso" { + t.Fatalf("path = %s, want /orgs/acme/sso", r.URL.Path) + } + + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"saml_identity_provider":{"sso_url":"https://idp.example/sso","issuer":"https://idp.example"}}`)) + })) + defer cleanup() + + fetcher := NewDataFetcher(hclog.NewNullLogger(), client) + sso, err := fetcher.fetchSSO(context.Background(), "acme") + if err != nil { + t.Fatalf("fetchSSO returned error: %v", err) + } + if !sso.Enabled { + t.Fatal("SSO should be enabled") + } + if sso.SSOURL != "https://idp.example/sso" { + t.Fatalf("SSOURL = %q, want https://idp.example/sso", sso.SSOURL) + } + if sso.IDPIssuer != "https://idp.example" { + t.Fatalf("IDPIssuer = %q, want https://idp.example", sso.IDPIssuer) + } +} + +func TestFetchSSONotFoundMeansDisabled(t *testing.T) { + client, cleanup := testGithubClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.NotFound(w, r) + })) + defer cleanup() + + fetcher := NewDataFetcher(hclog.NewNullLogger(), client) + sso, err := fetcher.fetchSSO(context.Background(), "acme") + if err != nil { + t.Fatalf("fetchSSO returned error: %v", err) + } + if sso.Enabled { + t.Fatal("SSO should be disabled when the endpoint returns 404") + } +} + +func TestFetchIPAllowListUsesVariablesAndPaginates(t *testing.T) { + page := 0 + var afterValues []interface{} + client, cleanup := testGithubClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + t.Fatalf("method = %s, want POST", r.Method) + } + if r.URL.Path != "/graphql" { + t.Fatalf("path = %s, want /graphql", r.URL.Path) + } + + var request struct { + Query string `json:"query"` + Variables map[string]interface{} `json:"variables"` + } + if err := json.NewDecoder(r.Body).Decode(&request); err != nil { + t.Fatalf("decoding GraphQL request: %v", err) + } + if request.Variables["login"] != "acme" { + t.Fatalf("login variable = %v, want acme", request.Variables["login"]) + } + afterValues = append(afterValues, request.Variables["after"]) + + w.Header().Set("Content-Type", "application/json") + switch page { + case 0: + _, _ = w.Write([]byte(`{"data":{"organization":{"ipAllowListEntries":{"edges":[{"node":{"allowListValue":"192.0.2.0/24","isActive":true,"name":"office"}}],"pageInfo":{"hasNextPage":true,"endCursor":"cursor-1"}}}}}`)) + case 1: + _, _ = w.Write([]byte(`{"data":{"organization":{"ipAllowListEntries":{"edges":[{"node":{"allowListValue":"198.51.100.0/24","isActive":false,"name":"vpn"}}],"pageInfo":{"hasNextPage":false,"endCursor":null}}}}}`)) + default: + t.Fatalf("unexpected GraphQL page request %d", page) + } + page++ + })) + defer cleanup() + + fetcher := NewDataFetcher(hclog.NewNullLogger(), client) + entries, err := fetcher.fetchIPAllowList(context.Background(), "acme") + if err != nil { + t.Fatalf("fetchIPAllowList returned error: %v", err) + } + if len(entries) != 2 { + t.Fatalf("len(entries) = %d, want 2", len(entries)) + } + if entries[0].AllowListValue != "192.0.2.0/24" || entries[1].AllowListValue != "198.51.100.0/24" { + t.Fatalf("entries = %#v", entries) + } + if len(afterValues) != 2 { + t.Fatalf("after values = %#v, want two requests", afterValues) + } + if afterValues[0] != nil { + t.Fatalf("first after = %#v, want nil", afterValues[0]) + } + if afterValues[1] != "cursor-1" { + t.Fatalf("second after = %#v, want cursor-1", afterValues[1]) + } +} + +func TestFetchIPAllowListErrorsWithoutEndCursor(t *testing.T) { + client, cleanup := testGithubClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"data":{"organization":{"ipAllowListEntries":{"edges":[],"pageInfo":{"hasNextPage":true,"endCursor":null}}}}}`)) + })) + defer cleanup() + + fetcher := NewDataFetcher(hclog.NewNullLogger(), client) + _, err := fetcher.fetchIPAllowList(context.Background(), "acme") + if err == nil { + t.Fatal("fetchIPAllowList should error when a next page has no end cursor") + } +}