Skip to content

Expand scraper coverage and diagnostics - #2334

Open
moshloop wants to merge 7 commits into
mainfrom
expand-scraper-coverage-diagnostics
Open

Expand scraper coverage and diagnostics#2334
moshloop wants to merge 7 commits into
mainfrom
expand-scraper-coverage-diagnostics

Conversation

@moshloop

@moshloop moshloop commented Jul 27, 2026

Copy link
Copy Markdown
Member

What

  • Add GitHub organization scraping and scraper doctor diagnostics.
  • Add resource-scoped GCP IAM hierarchy scraping.
  • Stabilize frontend, Playwright tests, release, ClickHouse, and Gavel CI jobs.

Why

  • Expand scraper coverage while reporting permission and rate-limit conditions clearly.

Summary by CodeRabbit

  • New Features

    • Added a doctor command to diagnose GitHub scraper configurations from a local file or scraper ID.
    • Added checks for GitHub repositories and organization settings, permissions, security, applications, members, and rulesets.
    • Added GitHub organization scraping, including applications, code security configurations, RBAC, teams, and repositories.
    • Added GCP resource hierarchy and resource-scoped IAM access discovery.
  • Documentation

    • Added usage instructions and output options for diagnosing scrapers.
  • Bug Fixes

    • Improved IAM access deduplication and clarified GitHub organization configuration options.

moshloop added 7 commits July 26, 2026 19:17
Pin the release workflow to a known action revision and use the dedicated
release token. Reclaim runner disk before test-clickhouse builds its image,
which had started failing on disk exhaustion.

Claude-Session-Id: 30a271bb-42a0-47aa-843e-c4cf03ce62df
Adds organization-level scraping to the GitHub scraper: org settings and
metadata, installed apps, code security configuration, and the org RBAC
graph of members, teams and roles, with the CRD and JSON schema regenerated
to match.

Auxiliary API calls degrade instead of aborting the scrape, and scraping
stops before the rate limit is exhausted, reporting the reset time so a
throttled run is distinguishable from a broken one.

Claude-Session-Id: 30a271bb-42a0-47aa-843e-c4cf03ce62df
Walks the organization, folder and project hierarchy and attaches
resource-scoped IAM bindings, linking hierarchy roots back to the projects
already in inventory so the two graphs join up.

Hierarchy discovery failures fall back to the flat policy results rather
than failing the whole scrape. Promotes cloud.google.com/go/iam from an
indirect to a direct dependency.

Claude-Session-Id: 30a271bb-42a0-47aa-843e-c4cf03ce62df
Pin @babel/core through a pnpm override so the lockfile resolves, declare
the iconify-icon custom element on preact's JSX namespace, and give
JsonView's entries an explicit tuple type. tsc now type-checks the scrape
UI cleanly.

Claude-Session-Id: 30a271bb-42a0-47aa-843e-c4cf03ce62df
Move the specs that launch a real browser into an e2e-tagged file so the
pure parseOutput specs compile and run without bun, chromium or the e2e
build tag.

Claude-Session-Id: 30a271bb-42a0-47aa-843e-c4cf03ce62df
Run gavel over the same scope as test.yml's `test` job — same runner,
timeout, step order, pinned SHAs and cache key — so the two checks agree on
what they cover. tests/e2e is ignored to match `gotest`'s --skip-package
(and is the only consumer of the loki/opensearch services, so the job needs
none), and bench is left to benchmark.yml since ginkgo -r never runs Go
benchmarks.

An explicit envtest step remains because gavel invokes packages directly
rather than through `make test`, so it does not inherit that target's
prerequisites. The check fails the build on test failures rather than
reporting advisory-only.

Claude-Session-Id: 30a271bb-42a0-47aa-843e-c4cf03ce62df
Introduce a doctor API and CLI for validating scraper access against local configs or persisted scraper IDs. Add GitHub repository and organization probes with pass/fail/skip results and permission evidence, while avoiding inferred repository grants for selected app installations and making rulesets independently opt-in.
@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Changes

Doctor checks and GitHub organization scraping

Layer / File(s) Summary
Doctor contracts and configuration
api/..., config/schemas/..., chart/crds/..., fixtures/..., README.md
Adds doctor result models, GitHub organization configuration, schemas, fixtures, deepcopy support, and usage documentation.
Doctor command and orchestration
cmd/doctor.go, scrapers/doctor.go, api/v1/doctor_test.go, cmd/doctor_test.go, scrapers/doctor_test.go
Adds the CLI command, target loading, doctor scraper dispatch, result aggregation, failure formatting, and tests.
GitHub doctor probes
scrapers/github/doctor*.go, scrapers/github/doctor_test.go
Adds repository and organization permission, settings, ruleset, security, app, role, member, and team probes with pass/fail/skip classification.
GitHub organization scraping and access mapping
scrapers/github/organization.go, scrapers/github/apps.go, scrapers/github/code_security.go, scrapers/github/org_rbac.go, scrapers/github/scraper.go, scrapers/github/*_test.go
Scrapes configured organizations and maps organization settings, code security, app installations, teams, users, roles, groups, and repository access relationships.

GCP IAM hierarchy and resource-scoped access

Layer / File(s) Summary
Resource hierarchy and IAM access
scrapers/gcp/iam.go, scrapers/gcp/iam_hierarchy.go, scrapers/gcp/iam_test.go, go.mod
Discovers GCP project ancestry, emits hierarchy results and policies, and scopes IAM access identities and deduplication to resources.

CI and release workflow updates

Layer / File(s) Summary
Workflow execution and release authentication
.github/workflows/gavel.yml, .github/workflows/test.yml, .github/workflows/release.yml
Updates Gavel provisioning and test execution, adds test-load disk cleanup, and pins the reusable release workflow with the updated token.

Frontend and Playwright test maintenance

Layer / File(s) Summary
Frontend type compatibility
cmd/scrapeui/frontend/package.json, cmd/scrapeui/frontend/src/components/JsonView.tsx, cmd/scrapeui/frontend/src/iconify-icon.d.ts
Pins a Babel override and adds TypeScript typing improvements for JSON entries and the iconify-icon element.
Browser and parser test separation
scrapers/playwright/browser_e2e_test.go, scrapers/playwright/parse_test.go
Keeps browser tests behind the e2e build tag and expands standard parser tests for multiple output formats and artifacts.

Sequence Diagram(s)

sequenceDiagram
  participant Operator
  participant DoctorCLI
  participant DoctorRunner
  participant GithubScraper
  participant GitHubAPI
  Operator->>DoctorCLI: Provide fixture path or scraper UUID
  DoctorCLI->>DoctorRunner: Load configuration and run doctors
  DoctorRunner->>GithubScraper: Invoke Doctor
  GithubScraper->>GitHubAPI: Probe repositories and organizations
  GitHubAPI-->>GithubScraper: Responses, scopes, and errors
  GithubScraper-->>DoctorRunner: DoctorResults
  DoctorRunner-->>DoctorCLI: Render table or JSON failure output
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 17.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main theme of the PR: broader scraper coverage plus new diagnostics.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch expand-scraper-coverage-diagnostics
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch expand-scraper-coverage-diagnostics

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (4)
scrapers/gcp/iam_hierarchy.go (2)

116-168: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add negative-path tests for hierarchy validation.

buildResourceManagerHierarchy is a pure function, so its "incomplete hierarchy: expected X, got Y" (mismatched parent) and "missing X" (unterminated chain) error branches are inexpensive to cover with unit tests, but only the successful chain is currently exercised in iam_test.go.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scrapers/gcp/iam_hierarchy.go` around lines 116 - 168, Add unit tests for
buildResourceManagerHierarchy covering both hierarchy-validation failures: nodes
whose metadata name differs from the expected parent should return the “expected
X, got Y” error, and a chain ending with a non-empty expectedName should return
the “missing X” error. Keep the existing successful-chain test unchanged and
assert the returned error and nil results for each negative case.

68-97: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Don't discard hierarchy metadata just because GetIamPolicy fails.

fetchResourceManagerNode treats a GetIamPolicy failure as fatal for the whole node, even though the Get call already succeeded. Since resourcemanager.{folders,organizations}.getIamPolicy is commonly a narrower permission than .get, this means a single missing IAM-policy permission on one ancestor discards the entire hierarchy chain (via the warn-and-fallback in fetchIAMPolicies), losing folder/org structure that was otherwise fully available. This runs counter to the PR's goal of reporting permission conditions more precisely.

Consider tolerating a GetIamPolicy failure by keeping the resource with a nil Policy (and logging/warning), so the ancestor chain and config items are still emitted even when only bindings are inaccessible.

♻️ Proposed fix
 	switch {
 	case strings.HasPrefix(name, "folders/"):
 		folder, err := service.Folders.Get(name).Context(ctx).Do()
 		if err != nil {
 			return resourceManagerNode{}, fmt.Errorf("get GCP folder %s: %w", name, err)
 		}
 		policy, err := service.Folders.GetIamPolicy(name, request).Context(ctx).Do()
 		if err != nil {
-			return resourceManagerNode{}, fmt.Errorf("get IAM policy for GCP folder %s: %w", name, err)
+			ctx.Warnf("gcp iam policies: IAM policy for GCP folder %s unavailable: %v", name, err)
+			return resourceManagerNode{Resource: folder}, nil
 		}
 		return resourceManagerNode{Resource: folder, Policy: policy}, nil
 	case strings.HasPrefix(name, "organizations/"):
 		organization, err := service.Organizations.Get(name).Context(ctx).Do()
 		if err != nil {
 			return resourceManagerNode{}, fmt.Errorf("get GCP organization %s: %w", name, err)
 		}
 		policy, err := service.Organizations.GetIamPolicy(name, request).Context(ctx).Do()
 		if err != nil {
-			return resourceManagerNode{}, fmt.Errorf("get IAM policy for GCP organization %s: %w", name, err)
+			ctx.Warnf("gcp iam policies: IAM policy for GCP organization %s unavailable: %v", name, err)
+			return resourceManagerNode{Resource: organization}, nil
 		}
 		return resourceManagerNode{Resource: organization, Policy: policy}, nil
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scrapers/gcp/iam_hierarchy.go` around lines 68 - 97, Update
fetchResourceManagerNode so GetIamPolicy failures for folders and organizations
do not discard the successfully fetched resource: retain the resource with a nil
Policy, and emit an appropriate warning or log for the policy error. Continue
returning errors from the resource Get calls, and preserve successful policy
retrieval behavior so the hierarchy remains available when only IAM bindings are
inaccessible.
cmd/doctor.go (1)

134-148: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

File targets shouldn't hard-fail on an unreachable database.

When DB_URL is set but the database is down, doctor fixtures/github-doctor.yaml aborts with initialize database: ... even though a file target needs no DB. Consider only connecting when requireDatabase is true (or falling back to dutycontext.New() on connect failure for file targets).

♻️ Proposed change
 func newDoctorContext(requireDatabase bool) (dutycontext.Context, error) {
 	config := dutyapi.DefaultConfig.ReadEnv()
-	if config.ConnectionString == "" {
-		if requireDatabase {
-			return dutycontext.Context{}, fmt.Errorf("scraper-id doctor target requires a configured database")
-		}
+	if config.ConnectionString == "" || !requireDatabase {
+		if requireDatabase {
+			return dutycontext.Context{}, fmt.Errorf("scraper-id doctor target requires a configured database")
+		}
 		return dutycontext.New(), nil
 	}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cmd/doctor.go` around lines 134 - 148, Update newDoctorContext so database
initialization via duty.Start occurs only when requireDatabase is true; when a
database is not required, return dutycontext.New() even if ConnectionString is
configured or the database is unreachable. Preserve the existing error for
required database targets.
scrapers/github/doctor_result.go (1)

31-60: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Handle GitHub rate-limit errors in githubDoctorResult.

githubHTTPResponse only restores the Response for *gogithub.ErrorResponse, and githubGrantEvidence returns "request denied" for any error. When go-github returns *gogithub.RateLimitError or *gogithub.AbuseRateLimitError, the result stays generic instead of surfacing the rate-limit condition consistently with resolveDoctorRepositories, who explicitly emits Message: "GitHub API rate limit reached". Add centralized rate-limit detection before the current permissions/evidence handling.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scrapers/github/doctor_result.go` around lines 31 - 60, Update
githubDoctorResult to centrally detect *gogithub.RateLimitError and
*gogithub.AbuseRateLimitError before deriving permissions and grant evidence,
and set the result message to "GitHub API rate limit reached" consistently with
resolveDoctorRepositories. Preserve the existing status, knownDisabled handling,
and normal permission/evidence behavior for non-rate-limit errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/gavel.yml:
- Around line 33-34: Update the “Checkout code” step using actions/checkout to
set persist-credentials to false, disabling persistent Git credentials while
leaving the existing checkout action and version unchanged.

In `@scrapers/github/apps.go`:
- Around line 174-191: Update the installation handling around
effectiveInstallationRole so an installation with repositories but no effective
repository role is skipped rather than causing buildAppInstallations to return
an error. Preserve role emission for installations with a valid role and
continue processing all remaining installations.

In `@scrapers/github/code_security.go`:
- Around line 20-53: Update scrapeCodeSecurityConfigurations and
codeSecurityConfigurationRepositories to iterate through every paginated
response from GetCodeSecurityConfigurations and
GetRepositoriesForCodeSecurityConfiguration, passing the returned cursor or page
options on each request. Preserve existing error handling and aggregate all
configurations and repositories before building results so larger organizations
are fully represented.

---

Nitpick comments:
In `@cmd/doctor.go`:
- Around line 134-148: Update newDoctorContext so database initialization via
duty.Start occurs only when requireDatabase is true; when a database is not
required, return dutycontext.New() even if ConnectionString is configured or the
database is unreachable. Preserve the existing error for required database
targets.

In `@scrapers/gcp/iam_hierarchy.go`:
- Around line 116-168: Add unit tests for buildResourceManagerHierarchy covering
both hierarchy-validation failures: nodes whose metadata name differs from the
expected parent should return the “expected X, got Y” error, and a chain ending
with a non-empty expectedName should return the “missing X” error. Keep the
existing successful-chain test unchanged and assert the returned error and nil
results for each negative case.
- Around line 68-97: Update fetchResourceManagerNode so GetIamPolicy failures
for folders and organizations do not discard the successfully fetched resource:
retain the resource with a nil Policy, and emit an appropriate warning or log
for the policy error. Continue returning errors from the resource Get calls, and
preserve successful policy retrieval behavior so the hierarchy remains available
when only IAM bindings are inaccessible.

In `@scrapers/github/doctor_result.go`:
- Around line 31-60: Update githubDoctorResult to centrally detect
*gogithub.RateLimitError and *gogithub.AbuseRateLimitError before deriving
permissions and grant evidence, and set the result message to "GitHub API rate
limit reached" consistently with resolveDoctorRepositories. Preserve the
existing status, knownDisabled handling, and normal permission/evidence behavior
for non-rate-limit errors.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 50678211-ccb3-4e33-b5e3-14088985b8db

📥 Commits

Reviewing files that changed from the base of the PR and between a7e264d and 46ca3c4.

📒 Files selected for processing (46)
  • .github/workflows/gavel.yml
  • .github/workflows/release.yml
  • .github/workflows/test.yml
  • README.md
  • api/global.go
  • api/v1/common.go
  • api/v1/doctor.go
  • api/v1/doctor_test.go
  • api/v1/github.go
  • api/v1/zz_generated.deepcopy.go
  • chart/crds/configs.flanksource.com_scrapeconfigs.yaml
  • cmd/doctor.go
  • cmd/doctor_suite_test.go
  • cmd/doctor_test.go
  • cmd/scrapeui/frontend/package.json
  • cmd/scrapeui/frontend/src/components/JsonView.tsx
  • cmd/scrapeui/frontend/src/iconify-icon.d.ts
  • config/schemas/config_github.schema.json
  • config/schemas/scrape_config.schema.json
  • fixtures/github-doctor.yaml
  • fixtures/github.yaml
  • go.mod
  • scrapers/doctor.go
  • scrapers/doctor_test.go
  • scrapers/gcp/iam.go
  • scrapers/gcp/iam_hierarchy.go
  • scrapers/gcp/iam_test.go
  • scrapers/github/apps.go
  • scrapers/github/apps_test.go
  • scrapers/github/code_security.go
  • scrapers/github/code_security_test.go
  • scrapers/github/doctor.go
  • scrapers/github/doctor_organization.go
  • scrapers/github/doctor_organization_apps.go
  • scrapers/github/doctor_organization_members.go
  • scrapers/github/doctor_organization_settings.go
  • scrapers/github/doctor_result.go
  • scrapers/github/doctor_security.go
  • scrapers/github/doctor_test.go
  • scrapers/github/org_rbac.go
  • scrapers/github/org_rbac_test.go
  • scrapers/github/organization.go
  • scrapers/github/organization_test.go
  • scrapers/github/scraper.go
  • scrapers/playwright/browser_e2e_test.go
  • scrapers/playwright/parse_test.go
💤 Files with no reviewable changes (1)
  • scrapers/playwright/parse_test.go

Comment on lines +33 to +34
- name: Checkout code
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== workflow files =="
git ls-files .github/workflows || true

echo "== target workflow excerpt =="
if [ -f .github/workflows/gavel.yml ]; then
  nl -ba .github/workflows/gavel.yml | sed -n '1,140p'
else
  echo "missing .github/workflows/gavel.yml"
fi

echo "== search token/action usage in workflows =="
rg -n "GITHUB_TOKEN|permissions:|actions/checkout|gavel|dependency|envtest|persist-credentials|environment:" .github/workflows || true

Repository: flanksource/config-db

Length of output: 517


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== target workflow excerpt =="
cat -n .github/workflows/gavel.yml | sed -n '1,160p'

echo "== token/action usage in gavel workflow =="
grep -nE "GITHUB_TOKEN|permissions:|actions/checkout|gavel|dependency|envtest|persist-credentials|environment:" .github/workflows/gavel.yml || true

echo "== token/action usage in all workflows =="
grep -RInE -n "\${{ secrets\.|GITHUB_TOKEN|permissions:|actions/checkout|envtest|dependency setup|persist-credentials" .github/workflows || true

Repository: flanksource/config-db

Length of output: 8120


🌐 Web query:

actions/checkout README persist-credentials default true stores credentials git config

💡 Result:

The persist-credentials input in the actions/checkout GitHub Action controls whether the repository's authentication credentials (either the GITHUB_TOKEN or an SSH key) are configured in the local Git environment, allowing subsequent workflow steps to perform authenticated Git operations [1][2][3]. Although the default value for persist-credentials is true [4][1][2], its implementation has evolved to improve security [3][5][6]: 1. Historical Behavior: Originally, this setting wrote the authentication token or SSH key directly into the local.git/config file [2][5]. 2. Current Behavior: As of version v6.0.0, the action was updated to store credentials in a separate file located under $RUNNER_TEMP instead of writing them directly to.git/config [3][5][6]. This change reduces the risk of accidental exposure and improves security while maintaining the same functionality for subsequent Git commands [3][6]. The token or key remains active for the duration of the job and is removed during the post-job cleanup process [1][2][3]. Users who do not require authenticated Git access in subsequent steps can opt out by setting persist-credentials: false in their workflow configuration [1][3]. Note that while there have been community discussions and proposals to change the default value to false to enhance security, the default remains true as of the current versions [7][4][2][8].

Citations:


Disable checkout credential persistence.

actions/checkout enables authenticated Git access for later steps by default; this workflow does not need persistent checkout credentials because dependency/setup/token access is passed explicitly where required. Add persist-credentials: false under the checkout step.

Proposed fix
       - name: Checkout code
         uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
+        with:
+          persist-credentials: false
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: Checkout code
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
- name: Checkout code
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
with:
persist-credentials: false
🧰 Tools
🪛 zizmor (1.26.1)

[warning] 33-34: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false

(artipacked)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/gavel.yml around lines 33 - 34, Update the “Checkout code”
step using actions/checkout to set persist-credentials to false, disabling
persistent Git credentials while leaving the existing checkout action and
version unchanged.

Source: Linters/SAST tools

Comment thread scrapers/github/apps.go
Comment on lines +174 to +191
if len(installed.Repositories) == 0 {
continue
}

role, err := effectiveInstallationRole(permissions)
if err != nil {
return appInstallations{}, fmt.Errorf("github app %q: %w", installation.GetAppSlug(), err)
}
roleAlias := githubRepositoryRoleAlias(input.Owner, role)
if _, ok := seenRoles[roleAlias]; !ok {
seenRoles[roleAlias] = struct{}{}
result.Roles = append(result.Roles, models.ExternalRole{
Tenant: input.Owner,
Aliases: pq.StringArray{roleAlias},
RoleType: "GitHub::Repository",
Name: role,
})
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

One org-scoped app can abort the whole installation scrape.

An installation with repository_selection: all but only organization-scoped permissions (e.g. organization_administration) yields a non-empty installed.Repositories, so effectiveInstallationRole returns missing effective repository role and buildAppInstallations aborts — discarding results for every installation in the org. The same app installed with selected is handled gracefully by the len(...) == 0 guard, so the failure mode is inconsistent.

Consider skipping access/role emission for that installation (as the selected path does) instead of failing the batch.

🛠️ Proposed fix
 		role, err := effectiveInstallationRole(permissions)
 		if err != nil {
-			return appInstallations{}, fmt.Errorf("github app %q: %w", installation.GetAppSlug(), err)
+			// Organization-scoped installations carry no repository role; the
+			// config item is still emitted, just without access edges.
+			continue
 		}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if len(installed.Repositories) == 0 {
continue
}
role, err := effectiveInstallationRole(permissions)
if err != nil {
return appInstallations{}, fmt.Errorf("github app %q: %w", installation.GetAppSlug(), err)
}
roleAlias := githubRepositoryRoleAlias(input.Owner, role)
if _, ok := seenRoles[roleAlias]; !ok {
seenRoles[roleAlias] = struct{}{}
result.Roles = append(result.Roles, models.ExternalRole{
Tenant: input.Owner,
Aliases: pq.StringArray{roleAlias},
RoleType: "GitHub::Repository",
Name: role,
})
}
if len(installed.Repositories) == 0 {
continue
}
role, err := effectiveInstallationRole(permissions)
if err != nil {
// Organization-scoped installations carry no repository role; the
// config item is still emitted, just without access edges.
continue
}
roleAlias := githubRepositoryRoleAlias(input.Owner, role)
if _, ok := seenRoles[roleAlias]; !ok {
seenRoles[roleAlias] = struct{}{}
result.Roles = append(result.Roles, models.ExternalRole{
Tenant: input.Owner,
Aliases: pq.StringArray{roleAlias},
RoleType: "GitHub::Repository",
Name: role,
})
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scrapers/github/apps.go` around lines 174 - 191, Update the installation
handling around effectiveInstallationRole so an installation with repositories
but no effective repository role is skipped rather than causing
buildAppInstallations to return an error. Preserve role emission for
installations with a valid role and continue processing all remaining
installations.

Comment on lines +20 to +53
func scrapeCodeSecurityConfigurations(ctx api.ScrapeContext, scrape *organizationScrape) v1.ScrapeResults {
var results v1.ScrapeResults
org := scrape.name()

configurations, _, err := scrape.client.Client.Organizations.GetCodeSecurityConfigurations(ctx, org)
if err != nil {
if isOrganizationFeatureUnavailable(err) {
ctx.Logger.V(2).Infof("skipping code security configurations for %s: %v", org, err)
return nil
}

results.Errorf(err, "failed to list code security configurations for GitHub organization %s", org)
return results
}

for _, configuration := range configurations {
if configuration.GetID() == 0 {
results.Errorf(fmt.Errorf("missing id"), "invalid code security configuration %q for GitHub organization %s",
configuration.GetName(), org)
continue
}

repositories, err := codeSecurityConfigurationRepositories(ctx, scrape, configuration.GetID())
if err != nil {
results.Errorf(err, "failed to list repositories for code security configuration %q of GitHub organization %s",
configuration.GetName(), org)
continue
}

results = append(results, buildCodeSecurityConfigurationResult(scrape, configuration, repositories))
}

return results
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

Does go-github v73's Organizations.GetCodeSecurityConfigurations and GetRepositoriesForCodeSecurityConfiguration support pagination via ListOptions, or do they return all results in one call?

💡 Result:

The go-github library methods Organizations.GetCodeSecurityConfigurations and Organizations.GetRepositoriesForCodeSecurityConfiguration do not automatically return all results in a single call; they adhere to the GitHub REST API's pagination standards [1][2][3]. Regarding pagination support: 1. Organizations.GetCodeSecurityConfigurations: According to the GitHub REST API documentation for this endpoint, it supports per_page, before, and after query parameters, which are characteristic of cursor-based pagination [4][5]. While go-github provides a ListOptions struct for standard offset-based pagination (using Page and PerPage), endpoints that utilize cursor-based pagination (using before and after parameters) often require passing a struct that includes these cursor fields rather than the generic ListOptions [2][6]. 2. Organizations.GetRepositoriesForCodeSecurityConfiguration: This endpoint also follows standard GitHub REST API pagination conventions [4][5]. Consequently, it does not aggregate all results into one response [2][3]. To retrieve all results, you must implement a loop that checks the Response object returned by these methods. You can inspect the NextPage field (for offset pagination) or use the cursors provided in the response headers (for cursor pagination) to make subsequent requests until all data has been fetched [1][2][6]. Alternatively, you can use middleware such as gofri/go-github-pagination to automatically handle the aggregation of paginated results [1][2][3].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Files matching code_security.go:"
fd -a 'code_security\.go$' . || true

echo
echo "Inspect relevant file outline and sections:"
file=$(fd 'code_security\.go$' . | head -n1 || true)
if [ -n "$file" ]; then
  echo "FILE=$file"
  wc -l "$file"
  ast-grep outline "$file" || true
  sed -n '1,120p' "$file" | cat -n
fi

echo
echo "Search for all code security API calls:"
rg -n "GetCodeSecurityConfigurations|GetRepositoriesForCodeSecurityConfiguration|ListOptions|NextPage" . --glob '!vendor/**' --glob '!node_modules/**' || true

Repository: flanksource/config-db

Length of output: 16754


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Inspect existing pagination handling for the same code-security endpoints:"
sed -n '120,170p' scrapers/github/doctor_organization_settings.go | cat -n

echo
echo "Determine go-github v73 dependency path from module files:"
fd '^go\.(mod|sum)$' . | while read -r f; do
  echo "--- $f"
  rg -n 'github.com/google/go-github/v73|github.com/google/go-github' "$f" || true
done

echo
echo "Search generated/external go-github signatures in installed module cache, if present:"
module_version=$(rg -n 'github.com/google/go-github/v73|github.com/google/go-github' go.mod go.sum 2>/dev/null | sed -n '1,5p' || true)
echo "$module_version"
goenv=$(go env GOPATH 2>/dev/null || true)
echo "GOPATH=$goenv"
if [ -n "$goenv" -a "$goenv" != "" ]; then
  fd 'organizations*|code_security*.*' "$goenv/pkg/mod/github.com/google!go-github" 2>/dev/null | sort | sed -n '1,80p' || true
fi

python3 - <<'PYTHON'
# Read-only behavioral probe: inspect installed go-github source signatures if cache exists.
# This does not import/configure/run repository code.
PYTHON

Repository: flanksource/config-db

Length of output: 2569


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path
gopath = Path.home() / "go"
candidates = []

for root, dirs, files in Path(gopath / "pkg" / "mod" / "github.com").glob("google/go-github**"):
    if not root.is_dir():
        continue
    candidates.append((root, "organizations.go", "code_security_configurations.go"))

print("GOPATH", gopath)
print("candidates", len(candidates))

for root, org, cfg in candidates:
    print("--- root", root)
    for name in (org, cfg):
        p = root / "github" / name
        if p.exists():
            print(f"FOUND github/{name}")
            text = p.read_text(errors="replace").splitlines()
            for i, line in enumerate(text, 1):
                if "GetCodeSecurityConfigurations" in line or "GetRepositoriesForCodeSecurityConfiguration" in line or "ListCursorOptions" in line or "ListOptions" in line:
                    # nearby 20 lines
                    start=max(1, i-15)
                    if name == "code_security_configurations.go":
                        end=min(len(text), i+40)
                    else:
                        end=min(len(text), i+40)
                    for j in range(start, end+1):
                        print(f"{name}:{j}:{text[j-1]}")
PY

Repository: flanksource/config-db

Length of output: 933


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path
gopath = Path.home() / "go"
pkg = gopath / "pkg" / "mod" / "github.com"
found = []

for root in pkg.iterdir():
    if not root.is_dir() or root.name != "google":
        continue
    try:
        for root2 in root.iterdir():
            if not root2.is_dir() or root2.name != "go-github":
                continue
            for root3 in root2.iterdir():
                if root3.is_dir() and any(part in ("v73.0.0", "v73") for part in root3.parts):
                    found.append(root3)
    except PermissionError:
        print("permission error under", root)

print("found:", found)
for root in found:
    for name in ["organizations.go", "code_security_configurations.go"]:
        p = root / "github" / name
        if not p.exists():
            print(f"MISSING {root}/{name}")
            print("github children:", [x.name for x in (root / "github").iterdir() if x.is_file()][:80], "...")
            continue
        print(f"\n--- {root}/{name}")
        lines = p.read_text(errors="replace").splitlines()
        for i, line in enumerate(lines, 1):
            if "GetCodeSecurityConfigurations" in line or "GetRepositoriesForCodeSecurityConfiguration" in line:
                start = max(1, i - 20)
                end = min(len(lines), i + 45)
                for j in range(start, end + 1):
                    print(f"{j:4}: {lines[j-1]}")
PY

Repository: flanksource/config-db

Length of output: 169


🌐 Web query:

github.com/google/go-github v73.0.0 GetCodeSecurityConfigurations GetRepositoriesForCodeSecurityConfiguration signatures ListCursorOptions

💡 Result:

In the google/go-github library (version 73.0.0), the Code Security Configurations API and cursor-based pagination are implemented as follows: Code Security Configurations Methods Support for Code Security Configurations was introduced to interact with the GitHub REST API endpoints [1]. Relevant methods available in the library include: GetCodeSecurityConfigurations: Used to list code security configurations, typically available under the relevant service (e.g., organizations or enterprises) [1][2]. GetRepositoriesForCodeSecurityConfiguration: Used to retrieve repositories associated with a specific code security configuration [3]. Note that in later versions of the library, the return type for this method was updated to RepositoryAttachment [4][5]. ListCursorOptions The ListCursorOptions struct is used for cursor-based pagination, which is required by many modern GitHub REST API endpoints that do not use traditional page-number pagination [6]. It is defined in the github package and contains fields to support navigation using cursors provided in response Link headers [6]. The struct definition includes the following fields [6]: Page (string): For paginated result sets, the page of results to retrieve. PerPage (int): The number of results to include per page. First (int): The number of results to include per page (max 100), starting from the first matching result. Last (int): The number of results to include per page (max 100), starting from the last matching result. After (string): A cursor, as given in the Link header; the query only searches for results after this cursor. Before (string): A cursor, as given in the Link header; the query only searches for results before this cursor. Cursor (string): A cursor, as given in the Link header; the query continues the search using this cursor. These options can be passed directly to list methods or embedded within more specific options structs [7][8]. Cursor information is also available via the github.Response struct returned by API calls [7][6].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
import urllib.request

base = "https://raw.githubusercontent.com/google/go-github/v73.0.0/github"
files = [f"{base}/code_security_configurations.go", f"{base}/organizations.go"]

for url in files:
    print(f"\n--- {url.split('/')[-1]}")
    try:
        text = urllib.request.urlopen(url, timeout=20).read().decode()
    except Exception as e:
        print(f"ERROR: {e}")
        continue
    lines = text.splitlines()
    for i, line in enumerate(lines, 1):
        if "GetRepositoriesForCodeSecurityConfiguration" in line or "GetCodeSecurityConfigurations" in line or "ListCursorOptions" in line:
            start = max(1, i-15)
            end = min(len(lines), i+45)
            for j in range(start, end+1):
                print(f"{j:4}: {lines[j-1]}")
PY

Repository: flanksource/config-db

Length of output: 502


🌐 Web query:

GitHub API code security configurations list code security configurations cursor-based pagination before after

💡 Result:

The GitHub REST API endpoints for listing code security configurations support cursor-based pagination using the before and after query parameters [1][2][3]. These parameters function as follows: after (string): If specified, the API returns results immediately following the provided cursor [1][3][4]. before (string): If specified, the API returns results immediately preceding the provided cursor [1][3][4]. These cursors are obtained from the Link header returned in the API response [1][2][3]. Additionally, the per_page parameter can be used to control the number of results returned per page, with a maximum value of 100 [1][3][5]. For general guidance on implementing pagination with GitHub's REST API, including handling Link headers, developers should refer to the official documentation on using pagination in the REST API [6][7].

Citations:


Paginate both code security list calls.

GetCodeSecurityConfigurations and GetRepositoriesForCodeSecurityConfiguration are not unbounded; they return paginated responses and do not automatically fetch all results. Loop with the returned response (or go-github cursors) and pass the appropriate cursor options, or attachments and/or configurations can be silently dropped for larger organizations.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scrapers/github/code_security.go` around lines 20 - 53, Update
scrapeCodeSecurityConfigurations and codeSecurityConfigurationRepositories to
iterate through every paginated response from GetCodeSecurityConfigurations and
GetRepositoriesForCodeSecurityConfiguration, passing the returned cursor or page
options on each request. Preserve existing error handling and aggregate all
configurations and repositories before building results so larger organizations
are fully represented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant