Skip to content

fix(search): strip GitHub qualifier injection from search filter input#691

Open
anshul23102 wants to merge 1 commit into
GitMetricsLab:mainfrom
anshul23102:fix/690-search-input-sanitization
Open

fix(search): strip GitHub qualifier injection from search filter input#691
anshul23102 wants to merge 1 commit into
GitMetricsLab:mainfrom
anshul23102:fix/690-search-input-sanitization

Conversation

@anshul23102
Copy link
Copy Markdown
Contributor

@anshul23102 anshul23102 commented Jun 3, 2026

Description

The search filter value in useGitHubData was appended verbatim to the GitHub search query string. A user who typed a GitHub qualifier (e.g. repo:other/repo, author:someone) into the search field could override the query operators the hook already set, retrieving issues outside the intended username scope.

Root Cause

The filter value was concatenated without sanitization:

  • q += filters.search in:title

A search term containing repo:org/repo would inject an extra repository scope, bypassing the author:-based filter.

Related Issue

Closes #690

Type of Change

  • Bug fix

Changes Made

  • src/hooks/useGitHubData.ts: Added a sanitize step that strips key:value qualifier patterns from filters.search before appending to the query. Plain keyword searches continue to work as intended.

Testing Done

  1. Search for "fix bug": query builds correctly as before.
  2. Search for "repo:other/repo fix": qualifier is stripped, only "fix" is appended.
  3. Search for "author:someone": stripped entirely, query not modified.

Checklist

  • My code follows the style and formatting of this project
  • I have tested my changes locally and they work as expected
  • There are no merge conflicts with the base branch
  • This PR is linked to the correct issue

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced GitHub search input validation to ensure search queries are processed correctly and prevent unintended query modifications.

The search filter value was appended to the GitHub query string verbatim.
A caller who types a GitHub qualifier such as 'repo:other/repo' into the
search field could override the repository scope and retrieve issues from
unintended repositories, bypassing the username-scoped query the hook
builds.

Strip key:value qualifier patterns from filters.search before appending
it to the query. This allows plain keyword searches to work as intended
while preventing operator injection.

Closes GitMetricsLab#690
@netlify
Copy link
Copy Markdown

netlify Bot commented Jun 3, 2026

Deploy Preview for github-spy ready!

Name Link
🔨 Latest commit 2c90f42
🔍 Latest deploy log https://app.netlify.com/projects/github-spy/deploys/6a2027a3129b9d0008b3c3ea
😎 Deploy Preview https://deploy-preview-691--github-spy.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jun 3, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

The PR sanitizes GitHub API search queries by filtering qualifier tokens from user input before constructing the search string. The change removes key:value-style qualifiers from filters.search and appends only the sanitized text to the in:title query parameter, replacing previous direct concatenation of raw user input.

Changes

Search Input Sanitization

Layer / File(s) Summary
GitHub search query sanitization
src/hooks/useGitHubData.ts
filters.search input is sanitized by stripping key:value qualifier tokens using regex filtering before appending it to the in:title GitHub query parameter, replacing direct concatenation with guarded conditional append.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A rabbit hops through queries bright,
Removing qualifiers with all its might,
No injections shall pass today,
Safe search terms light the way! ✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR addresses a security vulnerability but targets GitHub API query injection in useGitHubData rather than SQL injection in backend database queries as required by issue #690. Update the PR to address SQL injection vulnerabilities in backend database queries (backend/routes/repos.js or backend/services/repoService.js) as specified in issue #690, using parameterized queries instead of string concatenation.
Out of Scope Changes check ⚠️ Warning The changes target GitHub API query sanitization in the frontend hook, which differs from the backend SQL injection vulnerability explicitly outlined in issue #690. The PR modifies frontend GitHub API query construction rather than fixing the backend SQL injection vulnerability described in issue #690. Refocus changes on the backend database query layer.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: preventing GitHub qualifier injection in the search filter by stripping such qualifiers.
Description check ✅ Passed The PR description comprehensively covers the issue, root cause, solution, testing performed, and includes all required template sections with detailed information.
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

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.

Copy link
Copy Markdown
Contributor

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

🧹 Nitpick comments (1)
src/hooks/useGitHubData.ts (1)

49-59: 💤 Low value

Sanitization logic is sound for the intended threat model.

The regex effectively strips common GitHub qualifiers (repo:, author:, is:, label:, etc.) and prevents query scope bypass. The implementation correctly handles negative qualifiers like -label:bug since the hyphen is included in the character class.

One edge case to consider: quoted qualifier values with embedded spaces (e.g., label:"bug fix") will be partially stripped, leaving orphaned tokens like fix". This isn't a security bypass but could cause unexpected search behavior.

Optional: Handle quoted values more completely
      const sanitizedSearch = filters.search
-        .replace(/[a-zA-Z_-]+:[^\s]*/g, '')
+        .replace(/[a-zA-Z_-]+:(?:"[^"]*"|[^\s]*)/g, '')
        .trim();
🤖 Prompt for 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.

In `@src/hooks/useGitHubData.ts` around lines 49 - 59, The current sanitization
leaves orphaned tokens when qualifiers use quoted values (e.g., label:"bug
fix"); update the logic that computes sanitizedSearch in useGitHubData (where
filters.search is processed) to first remove quoted qualifiers and then remove
unquoted qualifiers — e.g., perform a replace pass that strips patterns like
key:"..."(including the quotes and inner spaces) before the existing regex that
removes key:token forms — so sanitizedSearch no longer contains leftover
fragments like fix".
🤖 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.

Nitpick comments:
In `@src/hooks/useGitHubData.ts`:
- Around line 49-59: The current sanitization leaves orphaned tokens when
qualifiers use quoted values (e.g., label:"bug fix"); update the logic that
computes sanitizedSearch in useGitHubData (where filters.search is processed) to
first remove quoted qualifiers and then remove unquoted qualifiers — e.g.,
perform a replace pass that strips patterns like key:"..."(including the quotes
and inner spaces) before the existing regex that removes key:token forms — so
sanitizedSearch no longer contains leftover fragments like fix".

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b5a1f6cb-17ed-46b1-9e5d-498e4269002c

📥 Commits

Reviewing files that changed from the base of the PR and between 53f820b and 2c90f42.

📒 Files selected for processing (1)
  • src/hooks/useGitHubData.ts

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Repository Search Lacks Input Validation - SQL Injection Vulnerability

1 participant