Fix GitHub issue search repository scope

[ndisidore]

GitHub issue search was relying on an appended repo: qualifier, but caller-controlled search syntax could escape that scope.

This keeps the repository boundary local by quoting search inputs, validating results before caching or returning them, and retaining the source URL for cache revalidation. It also logs blocked scope mismatches and avoids caching issue bodies.

Added focused regression tests for injection attempts, case handling, and malformed result URLs.

Tested with the full test suite, type checks, and lint.

[ndisidore]

Temporarily closing to re-trigger stuck CI checks.

[ask-bonk]

Review: Fix GitHub issue search repository scope

I reviewed the diff, ran the new tests (9 passing), tsc --noEmit (clean), and oxlint (only a pre-existing warning). The fix is sound and well-targeted. Notes below.

What the fix does well

• Two independent layers. buildIssueSearchQuery quotes every caller-controlled fragment (text, author, assignee, labels) via JSON.stringify, so an injected repo: / OR qualifier is matched as a literal phrase rather than parsed as syntax. assertIssueSearchResultsInRepo then re-derives the repo boundary from each result's html_url and rejects anything outside https://github.com/<owner>/<repo>/issues/.... Defense-in-depth: even if quoting were bypassed, the result check still contains the blast radius.
• Validation runs on cache hits too. assertSearchScope(results) is called after #loadCachedWithEtag returns (github.ts:2592), not only on the fresh path (github.ts:2582), so a poisoned or stale cache entry can't leak cross-repo results. Bumping the cache key to search-issues-scoped-v1 correctly orphans old entries that lack the stored source URL, and storage-schema.md was updated to match.
• Only validated data is cached. CachedIssueSearchResult stores just html_url + the GitHubIssueSummary (no bodyMarkdown — that lives only on GitHubIssueDetails), matching the "avoid caching issue bodies" goal.
• state/sort/direction need no quoting — they're constrained by string-literal unions at the RPC type boundary, so leaving state:${query.state} unquoted is fine.
• Correct scope of the fix. The PR search path (#searchPullSummaries) goes through the REST /repos/{owner}/{repo}/pulls endpoint with local pullResponseMatchesSearch filtering, so it never had the repo:-escape exposure that /search/issues did. Fixing only the issue path is right.
• The advanced_search: true addition is serialized as "true" (via String(value)), which the new searchIssuesConditional test asserts.

One behavioral change worth confirming (not blocking)

Previously PRs returned by the search were silently dropped (.filter(item => !item.pull_request)); now a /pull/ URL makes assertIssueSearchResultsInRepo throw ("non-issue result"), failing the whole page rather than skipping the row. With is:issue + advanced_search GitHub shouldn't return PRs, so in practice this should never fire — but it converts a previously-tolerated upstream quirk into a hard error for the caller. If you'd rather keep the search resilient, consider filtering non-issue results out and only throwing on the true cross-repo case (a wrong owner/repo), which is the actual security boundary. The scope-mismatch logger.warn is a nice touch either way.

Nits

• assertIssueSearchResultsInRepo reads result.html_url positionally from the path (resultOwner, resultRepo, resultKind); the tests cover prefix-only names, wrong host, malformed URLs, and the pull/issue distinction, which is good coverage for the parsing.

Overall this is a clean, minimal fix that closes the scope-escape and adds solid regression tests. LGTM aside from confirming the PR-result-throws behavior is intentional.

[ask-bonk]

Review posted to PR #49.

Summary

I reviewed PR #49 ("Fix GitHub issue search repository scope"), verified it locally (9 tests pass, tsc --noEmit clean, oxlint shows only a pre-existing warning), and posted a review.

The fix is sound. The vulnerability was that GitHub issue search built a q string with an appended repo:owner/repo qualifier, but caller-controlled text/author/assignee/labels could inject OR repo:other/repo and escape that scope. The PR closes this with two independent layers:

1. Query construction (github-search.ts): quotes every free-form fragment via JSON.stringify so injected qualifiers are treated as literal phrases, plus enables advanced_search: true.
2. Result validation (assertIssueSearchResultsInRepo): re-derives the repo boundary from each result's html_url and rejects anything outside github.com/<owner>/<repo>/issues/... — and this runs on cache hits too, so a poisoned cache can't leak.

Things I verified as correct:

• Cache key bumped to search-issues-scoped-v1 (orphans old, unvalidated entries) with storage-schema.md updated to match.
• Cached shape stores only html_url + summary (no issue bodies).
• The PR-search path was correctly left alone — it uses the scoped REST /repos/{owner}/{repo}/pulls endpoint, so it never had this exposure.
• state/sort/direction don't need quoting (constrained by RPC string-literal unions).

One non-blocking flag I raised: the old code silently filtered out PRs from results; the new code throws on a /pull/ URL, converting a tolerated upstream quirk into a hard page failure. With is:issue it shouldn't fire, but I suggested filtering non-issues and only throwing on true cross-repo mismatches if they want the search to stay resilient.

github run