Skip to content

fix(compliance-audit): handle 403 permission errors in CodeQL default setup check#221

Merged
don-petry merged 12 commits into
mainfrom
claude/issue-112-20260508-1733
May 17, 2026
Merged

fix(compliance-audit): handle 403 permission errors in CodeQL default setup check#221
don-petry merged 12 commits into
mainfrom
claude/issue-112-20260508-1733

Conversation

@don-petry
Copy link
Copy Markdown
Contributor

@don-petry don-petry commented May 8, 2026

Summary

  • Fixes a persistent false-positive codeql-default-setup-not-configured compliance finding caused by the audit token (ORG_SCORECARD_TOKEN) lacking security_events scope on the code-scanning API
  • The CodeQL default setup is actually already configured (state: configured, set 2026-04-25) — the audit was misreporting a permission error as "not configured"
  • Replaces the broken gh_api() retry-wrapper call with a direct gh api call that captures the HTTP response body and exit code separately, then detects 403 errors by the "status":"403" field in the response JSON

Root Cause

gh api outputs the error JSON body to stdout before exiting non-zero on HTTP errors. The gh_api() retry wrapper loops 3 times without suppressing stdout, so the captured output became 3 concatenated error bodies:

{"message":"Resource not accessible by personal access token","status":"403"}{"message":"...","status":"403"}{"message":"...","status":"403"}

This string is not "configured", so a finding was always added — even though the actual state (when checked with a properly-scoped token) is configured.

Fix

check_codeql_default_setup now:

  1. Calls gh api directly (no retry wrapper) and captures response body + exit code separately
  2. On non-zero exit: checks if the response body contains "status":"403" — if yes, logs an info message and skips without adding a finding
  3. On success (exit 0): extracts .state and checks it as before
  4. Other errors (404, 500, etc.) still produce a finding

Closes #112

Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved permission error handling in compliance audits. The audit now correctly distinguishes between permission limitations and missing configurations, avoiding false findings when audit tokens lack required permissions.

Review Change Stack

Copilot AI review requested due to automatic review settings May 8, 2026 17:38
@don-petry don-petry requested a review from a team as a code owner May 8, 2026 17:38
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 8, 2026

Warning

Rate limit exceeded

@don-petry has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 47 minutes and 54 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: e5d97d5d-4bd3-4664-b1d6-7e2554daab89

📥 Commits

Reviewing files that changed from the base of the PR and between 90e4786 and f43f401.

📒 Files selected for processing (1)
  • scripts/compliance-audit.sh
📝 Walkthrough

Walkthrough

This PR updates the CodeQL default-setup check in the compliance audit script to distinguish between missing permissions (403 status) and actual configuration failures. The function now directly calls the GitHub API without the retry wrapper, captures both response and exit code, detects permission errors by checking for 403 status, and only reports configuration findings when the API succeeds.

Changes

CodeQL Default Setup Check

Layer / File(s) Summary
API Error Handling Logic
scripts/compliance-audit.sh
check_codeql_default_setup() now bypasses gh_api() retry wrapper for direct gh api calls, captures response body and exit code separately, detects 403 permission status via jq filtering, skips reporting findings on 403 errors, and emits codeql-default-setup-not-configured only when state is missing or not configured.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: handling 403 permission errors in the CodeQL default setup check, which directly addresses the root cause in issue #112.
Linked Issues check ✅ Passed The code changes fully address the requirements from issue #112: detecting 403 permission responses, skipping false-positive findings when the token lacks permissions, and preserving accurate reporting for other states.
Out of Scope Changes check ✅ Passed All changes are scoped to fixing the 403 error handling in the CodeQL default setup check function with no unrelated modifications to the compliance-audit script.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/issue-112-20260508-1733

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 and usage tips.

@don-petry
Copy link
Copy Markdown
Contributor Author

@petry-projects/org-leads — PR is ready for review and merge. All CI checks are green (CodeQL still scanning, but this is a shell script-only change with no new code paths). This fixes a persistent false-positive compliance finding: CodeQL default setup was already configured since 2026-04-25, but the audit token was getting 403 on the check endpoint and misreporting it as 'not configured'.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adjusts the compliance audit’s CodeQL default-setup check to avoid persistent false-positive findings when the audit token lacks permission to read the code-scanning default-setup endpoint.

Changes:

  • Replaces the gh_api() retry-wrapper usage with a direct gh api call so stdout/error JSON isn’t duplicated across retries.
  • Detects and skips the check on HTTP 403 (“Resource not accessible…”) instead of misreporting it as “not configured”.
  • Keeps the existing behavior of filing a finding when the API succeeds but the .state is not configured (and still flags stray codeql.yml workflows).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread scripts/compliance-audit.sh
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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 `@scripts/compliance-audit.sh`:
- Around line 526-553: The current gh api call that sets raw_response and api_ok
bypasses gh_api() retries, so transient non-403 failures (5xx or rate limits)
cause immediate findings; modify the block around raw_response/api_ok so that if
the response is non-403 and api_ok != 0 you perform a limited retry loop (e.g.,
3 attempts with backoff) re-running gh api
"repos/$ORG/$repo/code-scanning/default-setup" and updating raw_response and
api_ok each attempt, break early on success, then only if all retries fail and
the response is not a 403 call
add_finding("codeql-default-setup-not-configured", ...) as currently written;
keep the existing jq check for '.status == "403"' to skip permission errors and
retain separate capture of stdout into raw_response and exit code into api_ok.
🪄 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: ASSERTIVE

Plan: Pro

Run ID: 9e2fbed2-60b4-4aa8-8779-cddfef5a1445

📥 Commits

Reviewing files that changed from the base of the PR and between 3b0d1a4 and 90e4786.

📒 Files selected for processing (1)
  • scripts/compliance-audit.sh

Comment thread scripts/compliance-audit.sh
@don-petry don-petry closed this May 11, 2026
@don-petry don-petry reopened this May 11, 2026
@don-petry don-petry closed this May 12, 2026
@don-petry don-petry reopened this May 12, 2026
@don-petry don-petry enabled auto-merge (squash) May 12, 2026 01:42
@github-actions
Copy link
Copy Markdown
Contributor

Auto-rebase failed — merge conflict — this branch has conflicts with main that must be resolved manually.

Please resolve the conflicts and push:

git fetch origin
git merge origin/main
# resolve conflicts, then:
git add .
git commit
git push

…setup

The ORG_SCORECARD_TOKEN used by the compliance audit lacks the
security_events scope required by the code-scanning/default-setup API.
When the endpoint returns 403, the gh_api() retry wrapper loops 3 times
and concatenates all three error response bodies into the captured output,
which is then compared against "configured" — always failing, and
producing a persistent false-positive finding even when CodeQL default
setup is actually configured.

This commit replaces the gh_api() call with a direct gh api call that
captures the response body and exit code separately. A 403 response is
now detected via its "status":"403" JSON field and silently skipped
rather than reported as "not configured". Other non-zero exit codes
(404, 500, etc.) still produce a finding as before.

Closes #112

Co-authored-by: Don Petry <don-petry@users.noreply.github.com>
@don-petry don-petry force-pushed the claude/issue-112-20260508-1733 branch from de67abe to 7a16ca0 Compare May 13, 2026 16:54
@donpetry-bot
Copy link
Copy Markdown
Contributor

donpetry-bot commented May 13, 2026

Superseded by automated re-review at 8516b5bb3d5038afa0ba2aca6ff9d05ebc01cf7e — click to expand prior review.

Review — fix requested (cycle 1/3)

The automated review identified the following issues. Please address each one:

Findings to fix

[Findings would be inserted here]

Additional tasks

  1. Resolve all unresolved review thread comments from other reviewers
  2. Ensure all CI checks pass after your changes
  3. Rebase on the target branch if behind
  4. Do NOT modify files unrelated to the findings above

The review cascade will automatically re-review after new commits are pushed.

@donpetry-bot
Copy link
Copy Markdown
Contributor

donpetry-bot commented May 14, 2026

Superseded by automated re-review at f43f401cbae91c1a7f22de77a5b520ce4be41915 — click to expand prior review.

Review — fix requested (cycle 2/3)

The automated review identified the following issues. Please address each one:

Findings to fix

[Findings would be inserted here]

Additional tasks

  1. Resolve all unresolved review thread comments from other reviewers
  2. Ensure all CI checks pass after your changes
  3. Rebase on the target branch if behind
  4. Do NOT modify files unrelated to the findings above

The review cascade will automatically re-review after new commits are pushed.

@sonarqubecloud
Copy link
Copy Markdown

Copy link
Copy Markdown
Contributor

@donpetry-bot donpetry-bot left a comment

Choose a reason for hiding this comment

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

Automated review — APPROVED ✓

Risk: LOW
Reviewed commit: f43f401cbae91c1a7f22de77a5b520ce4be41915
Review mode: triage-approved (single reviewer)

Summary

Focused 35/−15 line fix to scripts/compliance-audit.sh that resolves a persistent false-positive codeql-default-setup-not-configured finding. check_codeql_default_setup now bypasses the gh_api() retry wrapper (which was concatenating 3 error bodies into the captured output) and calls gh api directly, capturing stdout and exit code separately. A 403 response — caused by ORG_SCORECARD_TOKEN lacking the security_events scope — is now detected via jq -e '.status == "403"' and silently skipped instead of being misreported as "not configured". Other failures (404, 500, etc.) still produce a finding, and the success path still extracts .state and checks for configured. The stray-codeql.yml-workflow drift check below is untouched.

Linked issue analysis

Closes #112 (persistent false-positive codeql-default-setup-not-configured). The root-cause analysis in the PR description matches the diff: the previous gh_api() retry wrapper looped 3× on 403, concatenating error JSON bodies into the captured string, which never equaled "configured" and therefore always added a finding. The new code distinguishes a 403 permission error from real configuration failures, so the audit no longer misreports an already-configured repo as misconfigured. Substantively addressed.

Findings

None blocking.

  • Informational: CodeRabbit's earlier CHANGES_REQUESTED (2026-05-10) suggested adding a bounded retry loop for non-403 transient errors (5xx/rate limits). That is a reasonable hardening but not strictly required — the original gh_api() wrapper masked the very error being fixed here, and CodeRabbit subsequently posted an APPROVED review at this head SHA (f43f401). Optional follow-up if non-403 transient failures become noisy in practice.
  • Informational: raw_response is captured with 2>/dev/null, so the stderr message from gh api (e.g. on a 5xx) is suppressed. The exit-code path still files a finding with the generic "query returned no state" detail, which preserves observability of the failure even without the stderr line. Acceptable.

CI status

All required checks passing on f43f401cbae91c1a7f22de77a5b520ce4be41915: Lint, ShellCheck, Agent Security Scan, Secret scan (gitleaks), AgentShield, CodeQL (Analyze actions + tool run), SonarCloud (Quality Gate passed — 0 new issues, 0 security hotspots, 0 duplications), Dependency audit (no applicable ecosystems → skipped sub-jobs), CodeRabbit. mergeStateStatus: BLOCKED reflects the pending org-leads team review, not a technical gate.


Reviewed automatically by the PR-review agent (single-reviewer mode: opus 4.7). Reply if you need a human review.

@don-petry don-petry merged commit c7104f4 into main May 17, 2026
18 checks passed
@don-petry don-petry deleted the claude/issue-112-20260508-1733 branch May 17, 2026 00:14
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.

Compliance: codeql-default-setup-not-configured

3 participants