Skip to content

Conversation

@mtrezza
Copy link
Member

@mtrezza mtrezza commented Dec 10, 2025

Pull Request

Issue

CI performance benchmark runs with elevated permissions.

Approach

Remove elevated permissions in CI performance benchmark.

Summary by CodeRabbit

  • Chores
    • Performance benchmarking workflow now triggers on pull requests (expanded branch set: alpha, beta, release, release-*, next-major) with existing path-ignore filters.
    • Workflow permissions reduced to read-only.
    • PR-specific artifact uploads and automatic PR comments removed; benchmarking still generates baseline vs. PR comparison summaries and final reports.

✏️ Tip: You can customize this high-level summary in your review settings.

@parse-github-assistant
Copy link

parse-github-assistant bot commented Dec 10, 2025

🚀 Thanks for opening this pull request!

@coderabbitai
Copy link

coderabbitai bot commented Dec 10, 2025

📝 Walkthrough

Walkthrough

CI performance workflow updated: trigger changed from pull_request_target to pull_request with expanded branch patterns; permissions reduced to read-only; PR-specific checkout, artifact upload/storage, and PR comment posting steps removed; baseline benchmarking, comparison, and final summary generation remain.

Changes

Cohort / File(s) Summary
CI Performance Workflow
/.github/workflows/ci-performance.yml
Trigger switched from pull_request_target to pull_request with added branch patterns (alpha, beta, release, release-[0-9]+.x.x, next-major); permissions tightened (removed write for PRs/issues); removed PR-specific checkout, PR benchmark artifact storage/upload, and PR comment preparation/posting steps; kept baseline benchmarking, comparison, and summary generation.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5–15 minutes

  • Single workflow file modification, mostly declarative.
  • Review focus:
    • correctness of new trigger branch patterns and paths-ignore.
    • reduced permissions scope and any impacted steps.
    • removal of artifact/comment steps and downstream references to removed artifacts.

Possibly related PRs

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically summarizes the main change: removing elevated permissions from the CI performance benchmark workflow.
Description check ✅ Passed The PR description includes the issue statement and approach, but lacks linked issue reference and incomplete task checklist, though these are non-critical sections.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9ddc9fd and f975a26.

📒 Files selected for processing (1)
  • .github/workflows/ci-performance.yml (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). (15)
  • GitHub Check: PostgreSQL 16, PostGIS 3.5
  • GitHub Check: MongoDB 8, ReplicaSet
  • GitHub Check: MongoDB 7, ReplicaSet
  • GitHub Check: Node 18
  • GitHub Check: Redis Cache
  • GitHub Check: PostgreSQL 18, PostGIS 3.6
  • GitHub Check: MongoDB 6, ReplicaSet
  • GitHub Check: Node 22
  • GitHub Check: PostgreSQL 17, PostGIS 3.5
  • GitHub Check: Node 20
  • GitHub Check: PostgreSQL 15, PostGIS 3.3
  • GitHub Check: PostgreSQL 15, PostGIS 3.4
  • GitHub Check: PostgreSQL 15, PostGIS 3.5
  • GitHub Check: Docker Build
  • GitHub Check: Benchmarks
🔇 Additional comments (2)
.github/workflows/ci-performance.yml (2)

150-157: Inconsistency: AI summary claims PR artifact removal, but code still uploads PR results.

The AI-generated summary states "eliminated steps that stored PR benchmark results (pr.json) and associated artifact upload," yet lines 150–157 still contain an Upload PR results step uploading pr.json with a 7-day retention. Clarify whether:

  • This artifact upload is intentional and the summary is imprecise, or
  • This step should have been removed as part of the security fix and was inadvertently left in place.

If intentional, update the summary for accuracy.


3-3: Security fix is sound—no functional issues with pull_request trigger.

The change from pull_request_target to pull_request combined with contents: read permissions is a legitimate security hardening. All context fields referenced in the workflow (github.event.pull_request.head.sha, github.base_ref, github.event.pull_request.number) are available in the pull_request trigger, and artifact uploads do not require repository write permissions—they use GitHub Actions' separate artifact storage service. The workflow will function correctly on fork PRs.


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.

@parseplatformorg
Copy link
Contributor

parseplatformorg commented Dec 10, 2025

Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@mtrezza mtrezza changed the title fix: Remove CI permissions fix: Remove elevated permissions in CI performance benchmark Dec 10, 2025
coderabbitai[bot]
coderabbitai bot previously approved these changes Dec 10, 2025
@codecov
Copy link

codecov bot commented Dec 10, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.58%. Comparing base (e78e58d) to head (f975a26).
⚠️ Report is 5 commits behind head on alpha.

Additional details and impacted files
@@            Coverage Diff             @@
##            alpha    #9966      +/-   ##
==========================================
+ Coverage   92.56%   92.58%   +0.01%     
==========================================
  Files         191      191              
  Lines       15544    15544              
  Branches      177      177              
==========================================
+ Hits        14389    14391       +2     
+ Misses       1143     1141       -2     
  Partials       12       12              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mtrezza mtrezza changed the title fix: Remove elevated permissions in CI performance benchmark fix: Remove elevated permissions in GitHub CI performance benchmark Dec 10, 2025
@mtrezza mtrezza merged commit 6b9f896 into parse-community:alpha Dec 10, 2025
26 of 28 checks passed
parseplatformorg pushed a commit that referenced this pull request Dec 10, 2025
# [8.6.0-alpha.2](8.6.0-alpha.1...8.6.0-alpha.2) (2025-12-10)

### Bug Fixes

* Remove elevated permissions in GitHub CI performance benchmark ([#9966](#9966)) ([6b9f896](6b9f896))
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 8.6.0-alpha.2

@parseplatformorg parseplatformorg added the state:released-alpha Released as alpha version label Dec 10, 2025
@mtrezza mtrezza deleted the fix/remove-ci-benchmark-permissions branch December 10, 2025 22:42
parseplatformorg pushed a commit that referenced this pull request Dec 10, 2025
# [8.6.0](8.5.0...8.6.0) (2025-12-10)

### Bug Fixes

* Remove elevated permissions in GitHub CI performance benchmark ([#9966](#9966)) ([6b9f896](6b9f896))

### Features

* Add GraphQL query `cloudConfig` to retrieve and mutation `updateCloudConfig` to update Cloud Config ([#9947](#9947)) ([3ca85cd](3ca85cd))
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 8.6.0

@parseplatformorg parseplatformorg added the state:released Released as stable version label Dec 10, 2025
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 8.6.0

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

Labels

state:released Released as stable version state:released-8.x.x state:released-alpha Released as alpha version

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants