fix: paginate credential_audit search to stop silently dropping results past page 1

[karthikchundi-commits]

Summary

_search() in both copies of the credential-audit tool only ever fetches one page from GitHub's Search API. find_spray_citations() calls it with per_page=100 and find_merges() with per_page=50, but neither passes/increments a page parameter, so any subject with more issues (or more merged PRs into the target repo) than fit on that first page has every result past it silently dropped - no error, no warning, just an incomplete result set.

GitHub's Search API returns up to 1000 results per query, paginated (see https://docs.github.com/en/rest/search#about-search). A single unpaginated request only ever sees the first page of that window.

Why this matters for what the tool is actually for

This is backwards for a credential-spray detector specifically: a contributor prolific enough to have filed more than 100 issues total is also the contributor most likely to have real spray citations sitting on page 2+. As written, the audit can silently report NONE/LOW risk precisely for the highest-volume accounts it exists to catch, while a low-volume account gets fully covered by chance. Same shape of gap for find_merges() and prolific target-repo contributors, though the practical ceiling there (50+ merged PRs into one repo by one person) is a higher bar.

Fix

Paginate _search() through GitHub's full 1000-result window, stopping early when a page comes back shorter than per_page (the natural last-page signal). Applied to both copies since they implement this helper independently:

• agent-governance-python/agent-compliance/src/agent_compliance/cli/credential_audit.py
• scripts/credential_audit.py

Testing

Added TestSearchPagination to both test suites (mocking _api), covering:

• multi-page aggregation across a full page + a short page
• stopping immediately on a short first page (no wasted request)
• capping at the 1000-result window when every page comes back full (doesn't loop unboundedly)
• an empty/None first-page response returning no items

Ran both suites locally (pytest 9.0.3, Python 3.12.3):

• scripts/tests/test_credential_audit.py: 13/13 passed (4 new + 9 existing, no regressions)
• agent-governance-python/agent-compliance/tests/test_credential_audit.py (new file) + test_contributor_check.py: 7/7 passed

I don't have a real GitHub token with search-API access exercised against live data in this environment, so verification is at the _search()/pagination-logic level via mocks, not an end-to-end run of audit_credentials() against a live account - flagging that honestly.

Scope note

agent_compliance/cli/contributor_check.py's _search_issues() has the identical single-page pattern at several call sites (per_page=50/100, no page loop) - same underlying gap, different tool. Left untouched here to keep this PR scoped to credential_audit.py; happy to send a follow-up if useful.

AI-assisted contribution disclosure

Per CONTRIBUTING.md: this is an AI-assisted contribution (Claude, directed by me) - I reviewed the diff, understand the fix and can explain/defend it, and it is not an autonomous/unreviewed submission, so no additional disclosure is required, but noting it for transparency.

[azure-pipelines]

Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

[github-actions]

Welcome to the Agent Governance Toolkit! Thanks for your first pull request.
Please ensure tests pass, code follows style (ruff check), and you have signed the CLA.
See our Contributing Guide.

[github-actions]

PR Review Summary

Check	Status	Details
🔍 Code Review	⚠️ Missing	No current-run comment
🛡️ Security Scan	⚠️ Missing	No current-run comment
🔄 Breaking Changes	⚠️ Missing	No current-run comment
📝 Docs Sync	⚠️ Missing	No current-run comment
🧪 Test Coverage	⚠️ Missing	No current-run comment

Verdict: ⚠️ AI review incomplete; ready for human review

AI review comments are untrusted advisory output. The summary reports workflow-generated completion status only, not model-authored pass/fail claims.

[github-actions]

🟡 Contributor Check: MEDIUM

Check	Result
Profile	MEDIUM
Credential	LOW
Overall	MEDIUM

Automated check by AGT Contributor Check.

[karthikchundi-commits]

@microsoft-github-policy-service agree

[Imran Siddique (imran-siddique)]

The bug is real and the fix is right. The loop terminates correctly on all three exit conditions, and
the tests cover each one, which is more than most pagination fixes get.

Worth being explicit about why the old code was silently wrong rather than merely incomplete: it
requested per_page=100 and returned data["items"] with no page parameter and no check on
whether the page was full. A subject with 100 or fewer results looked identical to a subject with 900,
and the tool reported a clean audit in both cases. For a credential-spray auditor that is the worst
possible failure direction, and your comment makes the right point that the lost pages are exactly
the later, farther-out citations a real spray produces.

Checked the arithmetic against GitHub's actual constraint. The Search API rejects requests where
page * per_page exceeds 1000, so the cap has to be on the product and not just on the count.
max_pages = 1000 // 100 = 10 gives page 10 covering results 901 to 1000 and stops before the 422.
max(1, ...) handles a caller passing per_page above 1000. Both correct.

The short-page break is the right primary terminator, and keeping the empty-page break as well is
correct rather than redundant: _api returns None on failure, and without that branch a transient
error mid-pagination would keep looping to page 10 issuing failing requests.

Sequencing, and this one matters. #3876 is open and refactors scripts/credential_audit.py down
to a 27-line compatibility shim that does sys.modules[__name__] = _implementation, deleting the
implementation from that file entirely. This PR edits both copies. So:

• If this merges first, #3876 rebases and drops your scripts/ hunk, which is a clean outcome since
  your packaged-module fix is the one that survives.
• If #3876 merges first, your scripts/credential_audit.py hunk no longer applies and needs removing.

Landing this one first is the lower-friction order, given it is 160 lines against 3,200.

Your new tests in scripts/tests/test_credential_audit.py should keep passing either way, since under
the shim import credential_audit resolves to the packaged module and patch.object still finds
_api. They would just be exercising the packaged implementation twice, which is harmless and worth a
tidy-up in whichever PR lands second.

One thing to fix: Validate PR title currently shows red on this PR, but there is a later SUCCESS
for the same check in the rollup, so that entry is stale rather than a real failure. Worth knowing
before anyone treats it as a blocker.

Nothing blocking.

[Carlos Hernandez (carloshvp)]

Reviewed head ab8f8571 on a clean merge with current main at 359a2332. The scripts suite passed 13/13, and the packaged credential-audit plus contributor-check tests passed 7/7. I then applied the new tests to current main without the implementation; each suite failed three pagination cases. The tests distinguish the fix from the old single-page behavior. DCO, CLA, and the live check rollup are green.

Both production callers use per_page=50 or 100, so the loop covers GitHub's full 1000-result window and closes the reported undercount. One non-blocking edge remains in the helper contract: floor division stops a future per_page=30 caller after 33 pages and 990 items, while GitHub accepts page 34. Ceiling division plus a non-divisor test would cover that case. No current credential-audit path uses such a value.

This PR overlaps my #3876 in scripts/credential_audit.py. Landing #3812 first gives the lower-conflict sequence: #3876 can then rebase, retain the packaged implementation from this PR, and drop its deleted standalone implementation hunk. I found no blocker in this change and approve it.