Skip to content

Fix invalid query#10888

Merged
abnegate merged 1 commit into1.8.xfrom
fix-query-call
Dec 2, 2025
Merged

Fix invalid query#10888
abnegate merged 1 commit into1.8.xfrom
fix-query-call

Conversation

@ItzNotABug
Copy link
Copy Markdown
Contributor

@ItzNotABug ItzNotABug commented Dec 1, 2025 •
edited by stnguyen90
Loading

What does this PR do?

Fixes - Invalid query method: equal because getDocument() does not support passing filter queries like equal.

Test Plan

(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Screenshots may also be helpful.)

Related PRs and Issues

  • (Related PR or issue)

Checklist

  • Have you read the Contributing Guidelines on issues?
  • If the PR includes a change to an API's metadata (desc, label, params, etc.), does it also include updated API specs and example docs?

@ItzNotABug ItzNotABug self-assigned this Dec 1, 2025
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Dec 1, 2025 •
edited
Loading

📝 Walkthrough

Walkthrough

Repository retrieval logic in two GitHub installation endpoints (within app/controllers/api/vcs.php) was modified to use a query-based finder method instead of direct document fetching. The new approach retrieves repositories using two filters: matching both the provided repository ID and the project's internal ID. This changes the control flow and error handling for repository lookups while maintaining the same downstream logic for both endpoints.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

  • Verify that the findOne() query with both $id and projectInternalId filters correctly isolates the intended repository within the project context
  • Confirm error handling behavior differs appropriately between direct document fetch and finder-based query (e.g., what exception or response is returned when no matching repository is found)
  • Validate both endpoint updates are consistent and correctly implemented
  • Ensure the projectInternalId comparison accurately references the current project's sequence value

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Fix invalid query' is vague and generic, using non-descriptive terms that don't convey the specific nature of the fix or which query method is being corrected. Use a more specific title like 'Fix invalid query method in repository retrieval' to clarify what is being fixed.
✅ Passed checks (2 passed)
Check name Status Explanation
Description check ✅ Passed The description directly addresses the changeset by explaining the specific error being fixed ('Invalid query method: equal') and relates to the repository retrieval query changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-query-call
📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between adc0d3d and 4c62ecb.

📒 Files selected for processing (1)
  • app/controllers/api/vcs.php (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: moderate
  • GitHub Check: Setup & Build Appwrite Image
  • GitHub Check: Setup & Build Appwrite Image
  • GitHub Check: scan
🔇 Additional comments (1)
app/controllers/api/vcs.php (1)

1799-1802: LGTM! Correct fix with improved query construction.

The change from getDocument() with implicit filter argument to findOne() with explicit Query::equal() objects is the correct approach. This enforces stricter query validation and resolves issues with the previous query API. The dual filters correctly ensure both repository ID and project ownership are validated, preventing cross-project access.

The Authorization::skip() wrapper and error handling remain appropriately in place.

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.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Dec 1, 2025

Security Scan Results for PR

Docker Image Scan Results

Package Version Vulnerability Severity
imagemagick 7.1.2.3-r0 CVE-2025-62171 HIGH
imagemagick-c++ 7.1.2.3-r0 CVE-2025-62171 HIGH
imagemagick-dev 7.1.2.3-r0 CVE-2025-62171 HIGH
imagemagick-heic 7.1.2.3-r0 CVE-2025-62171 HIGH
imagemagick-jpeg 7.1.2.3-r0 CVE-2025-62171 HIGH
imagemagick-jxl 7.1.2.3-r0 CVE-2025-62171 HIGH
imagemagick-libs 7.1.2.3-r0 CVE-2025-62171 HIGH
imagemagick-openexr 7.1.2.3-r0 CVE-2025-62171 HIGH
imagemagick-webp 7.1.2.3-r0 CVE-2025-62171 HIGH
libecpg 17.6-r0 CVE-2025-12818 HIGH
libecpg-dev 17.6-r0 CVE-2025-12818 HIGH
libpng 1.6.47-r0 CVE-2025-64720 HIGH
libpng 1.6.47-r0 CVE-2025-65018 HIGH
libpng-dev 1.6.47-r0 CVE-2025-64720 HIGH
libpng-dev 1.6.47-r0 CVE-2025-65018 HIGH
libpq 17.6-r0 CVE-2025-12818 HIGH
libpq-dev 17.6-r0 CVE-2025-12818 HIGH
libxml2 2.13.8-r0 CVE-2025-49794 CRITICAL
libxml2 2.13.8-r0 CVE-2025-49796 CRITICAL
libxml2 2.13.8-r0 CVE-2025-49795 HIGH
libxml2 2.13.8-r0 CVE-2025-6021 HIGH
postgresql17-dev 17.6-r0 CVE-2025-12818 HIGH
github.com/containerd/containerd/v2 v2.0.2 CVE-2024-25621 HIGH
golang.org/x/crypto v0.31.0 CVE-2025-22869 HIGH
golang.org/x/oauth2 v0.24.0 CVE-2025-22868 HIGH
stdlib 1.22.10 CVE-2025-47907 HIGH
stdlib 1.22.10 CVE-2025-58183 HIGH
stdlib 1.22.10 CVE-2025-58186 HIGH
stdlib 1.22.10 CVE-2025-58187 HIGH

Source Code Scan Results

🎉 No vulnerabilities found!

@github-actions
Copy link
Copy Markdown

github-actions bot commented Dec 1, 2025

✨ Benchmark results

  • Requests per second: 1,211
  • Requests with 200 status code: 217,943
  • P99 latency: 0.163658592

⚡ Benchmark Comparison

Metric This PR Latest version
RPS 1,211 1,328
200 217,943 239,138
P99 0.163658592 0.15718425

@abnegate abnegate merged commit 5cc6845 into 1.8.x Dec 2, 2025
73 of 74 checks passed
@abnegate abnegate deleted the fix-query-call branch December 2, 2025 06:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@abnegate abnegate abnegate approved these changes

@ChiragAgg5k ChiragAgg5k ChiragAgg5k approved these changes

Assignees

@ItzNotABug ItzNotABug

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ItzNotABug @abnegate @ChiragAgg5k