Skip to content

fix(github): harden cross-referencing PR parser against null GraphQL fields - #1582

Merged
anderdc merged 2 commits into
entrius:testfrom
davion-knight:fix/base-repository-null-guard
Jul 8, 2026
Merged

fix(github): harden cross-referencing PR parser against null GraphQL fields#1582
anderdc merged 2 commits into
entrius:testfrom
davion-knight:fix/base-repository-null-guard

Conversation

@davion-knight

@davion-knight davion-knight commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Summary

_search_issue_referencing_prs_graphql parses GitHub's GraphQL timeline for the PRs that cross-reference an issue. Two nullable GraphQL fields were dereferenced without a guard, and either one raises an exception that propagates to find_prs_for_issue's outer handler and collapses the whole lookup to the None "lookup failed" sentinel — so a single bad node hides every other valid submission for that issue:

  1. Null timeline node. timelineItems.nodes elements are nullable — GitHub returns a null element when the cross-reference lives in a repo the token can't see (redacted item). node.get('source') then raises AttributeError, and a literal "nodes": null makes for node in None raise TypeError.
  2. Null baseRepository. PullRequest.baseRepository is nullable (comes back null when the PR's base repo has been deleted/is inaccessible). pr.get('baseRepository', {}).get('nameWithOwner', '') returns None from the present-but-null key, so .get('nameWithOwner') raises AttributeError.

Both are guarded the same way the sibling functions in this same file already handle them:

  • _select_current_close_event / _closing_issue_numbers_for_repo skip null nodes (if not node: continue) and use .get('nodes', []) or [].
  • _solver_from_closed_event guards baseRepository with (x or {}).get('nameWithOwner') or ''.

Related Issues

None.

Type of Change

  • Bug fix

Testing

  • Tests added — TestSearchIssueReferencingPrsGraphql exercises the real parser for a null node element, a null nodes list, and a null baseRepository (mixed with a valid PR, and alone). Each fails on test before the fix and passes after.
  • Full suite green (956 passed), ruff + pyright clean.

Checklist

  • Code follows project style guidelines
  • Self-review completed
  • Changes are documented (if applicable)

@xiao-xiao-mao xiao-xiao-mao Bot added the bug Something isn't working label Jul 4, 2026
@davion-knight

Copy link
Copy Markdown
Contributor Author

@anderdc @LandynDev whenever you have a moment — small null-safety bug fix. baseRepository is nullable in GitHub's GraphQL schema, and the referencing-PR parser (_search_issue_referencing_prs_graphql) reads it unguarded, so one cross-referencing PR with a deleted base repo throws AttributeError and collapses the whole submission lookup to the None sentinel. The fix mirrors the guard _solver_from_closed_event already uses on the same field. Couldn't self-apply the bug label / request review (no triage access on the repo) — flagging here instead. Thanks!

@davion-knight
davion-knight force-pushed the fix/base-repository-null-guard branch from f458c48 to 0a976a0 Compare July 6, 2026 05:54
…fields

_search_issue_referencing_prs_graphql parses GitHub's GraphQL timeline for
PRs that reference an issue. Two nullable fields were dereferenced unguarded,
and either one raises AttributeError/TypeError that propagates to
find_prs_for_issue's outer handler and collapses the whole lookup to the None
"lookup failed" sentinel — so one bad node hides every other valid submission
for that issue:

- timelineItems.nodes elements are nullable (GitHub returns a null element when
  the cross-reference lives in a repo the token can't see). `node.get('source')`
  then crashes, and a literal `"nodes": null` makes `for node in None` raise.
- PullRequest.baseRepository is nullable (null when the base repo was deleted).
  `pr.get('baseRepository', {}).get('nameWithOwner', '')` returns None from the
  present-but-null key, so `.get('nameWithOwner')` crashes.

Guard both the same way the sibling functions in this file already do:
_select_current_close_event / _closing_issue_numbers_for_repo skip null nodes
and use `.get('nodes', []) or []`; _solver_from_closed_event guards
baseRepository with `(x or {}).get(...) or ''`.

Add regression tests covering a null node element, a null nodes list, and a
null baseRepository (mixed with a valid PR, and alone).
@davion-knight
davion-knight force-pushed the fix/base-repository-null-guard branch from 0a976a0 to 3f68c2f Compare July 7, 2026 02:56
@davion-knight davion-knight changed the title fix(github): guard null baseRepository in cross-referencing PR parser fix(github): harden cross-referencing PR parser against null GraphQL fields Jul 7, 2026
@anderdc
anderdc merged commit 2f5224c into entrius:test Jul 8, 2026
@davion-knight

Copy link
Copy Markdown
Contributor Author

@anderdc @LandynDev — sorry to ping here, but I emailed x9entrius@gmail.com ~10h ago with no reply and this is time-sensitive. My SN74 miner wallet's seed was stolen. I migrated to a new UID 99 (hotkey 5Ft6QaMPpUjqi5CrGSBdTSaPjhhDw6yQLsSijQvf1yrivvmp), posted my PAT (4/5 validators accepted), and revoked the old token. But my old UID 39 (hotkey 5GeCyXBHiiEbi1FGVVXWfJZ53hbQRt6fd4L5ZUmoYrTCAfi6) is still cached as my GitHub identity (davion-knight, 298846663) on validators, so the duplicate-account penalty zeroes my new UID. Could you deregister/blacklist UID 39 or restart validators to clear the cache? Been stuck ~24h. Thank you 🙏

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

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants