Skip to content

fix(claims): authenticate catalog ownership lookups - #583

Merged
zerocodefast merged 1 commit into
mainfrom
fix/claim-notice-api-key-20261026
Oct 26, 2026
Merged

zerocodefast merged 1 commit into
mainfrom
fix/claim-notice-api-key-20261026

Conversation

@zerocodefast

Copy link
Copy Markdown
Collaborator

Authenticate claim-notice ownership lookups with a dedicated catalog API credential so the workflow can use its authenticated quota while anonymous bot protections remain in place. Use the direct catalog API endpoint and prevent credential forwarding to redirected requests or GitHub calls.

Require the credential in single-notice and scheduled retry runs. All 36 claim-notice, backfill and credential-boundary tests pass locally; CI includes the new boundary tests.

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Authenticate claim-notice catalog ownership lookups

🐞 Bug fix 🧪 Tests ⚙️ Configuration changes 🕐 10-20 Minutes

Grey Divider

AI Description

• Use an authenticated catalog endpoint for claim-notice ownership checks to access its
 authenticated quota.
• Require the catalog credential for notice delivery and scheduled retries.
• Restrict credential use to catalog reads and prevent forwarding on redirects; add boundary tests
 to CI.
Diagram

graph TD
  S["Catalog key secret"] --> W["Claim notice workflow"] --> P["Notice script"] --> C["HOL catalog API"]
  W --> B["Retry dispatcher"] --> W
  P --> G["GitHub API"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Dedicated catalog request helper
  • ➕ Separates catalog credential handling from the generic HTTP request function.
  • ➕ Makes future catalog call sites use an explicit authenticated path.
  • ➖ Adds an abstraction for the current single catalog lookup path.
  • ➖ Requires moving or duplicating some existing request and retry behavior.

Recommendation: Keep the scoped request guard for this fix: it limits the key to one HTTPS read endpoint without changing other callers. Consider a dedicated catalog helper if authenticated catalog operations expand.

Files changed (4) +67 / -7

Bug fix (1) +10 / -2
post-claim-notice.pyConstrain catalog authentication to the ownership read endpoint +10/-2

Constrain catalog authentication to the ownership read endpoint

• Defaults to the direct catalog API and reads the dedicated credential. Authenticated registry requests must be HTTPS GETs to the exact catalog path; the key is added as an unredirected header so redirects do not receive it.

scripts/post-claim-notice.py

Tests (2) +47 / -1
validate-contribution.ymlRun catalog credential-boundary tests in CI +2/-1

Run catalog credential-boundary tests in CI

• Adds the new test file to the workflow's path triggers and claim-notice unittest command.

.github/workflows/validate-contribution.yml

test_claim_notice_api_key.pyTest catalog key destination and redirect boundaries +45/-0

Test catalog key destination and redirect boundaries

• Verifies that catalog reads receive the key, redirects and GitHub requests do not, and invalid origins, paths, or methods are rejected before a request is sent.

tests/test_claim_notice_api_key.py

Other (1) +10 / -4
claim-notice.ymlRequire the catalog secret for notice and retry runs +10/-4

Require the catalog secret for notice and retry runs

• Points both runs at the direct catalog API and supplies the catalog key secret. Each run fails early if the secret is empty; the scheduled backfill still dispatches separate notice workflow runs.

.github/workflows/claim-notice.yml

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Tip of the day
💡 Did you know, you can type 'qodo, fix this' on a finding and the fix lands right on your PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

@kantorcodes kantorcodes left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Checks green, review threads resolved, quiet window elapsed at b0e2fdc.

@internet-dot internet-dot 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.

Checks green, review threads resolved, quiet window elapsed at b0e2fdc.

@zerocodefast
zerocodefast merged commit 87c5d09 into main Oct 26, 2026
16 checks passed
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.

3 participants