-
Notifications
You must be signed in to change notification settings - Fork 24
Fix preview workflow for forks #146
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…ancements. I saw that the `preview.yml` workflow was failing for pull requests from forked repositories because it could not access the `EXPO_TOKEN` secret. To fix this, I made the following changes: 1. I changed the workflow trigger from `pull_request` to `pull_request_target`. This allows the workflow to access secrets on pull requests from forks. 2. I added a security measure to prevent the workflow from running automatically on forked PRs. The workflow will now only run if the pull request has the label 'run-preview'. This allows a maintainer to review the code before triggering the preview build. 3. I'm now explicitly checking out the head of the pull request to ensure the preview is generated for the code in the PR.
This commit adds a `workflow_dispatch` trigger to the `preview.yml` workflow. This allows maintainers to manually trigger the workflow, in addition to the existing label-based trigger on pull requests. The workflow has been updated to handle both `pull_request_target` and `workflow_dispatch` events correctly.
This commit introduces a series of optimizations to the GitHub Actions workflows to improve security, speed, and maintainability. **`run-tests.yml`** - Secured forked PRs by switching to `pull_request_target` and requiring a `run-tests` label for execution. - Added `workflow_dispatch` for manual runs. - Added caching for pip dependencies to speed up test runs. - Made the workflow compatible with both `pull_request_target` and `workflow_dispatch` triggers. **Bundle Analysis Workflows** - Combined `bundle-analysis.yml` and `rn-bundle-analysis.yml` into a single `code-analysis.yml` workflow with two parallel jobs for web and React Native bundle analysis. - Secured the new workflow for forked PRs with `pull_request_target` and a `run-analysis` label requirement. - Added `workflow_dispatch` for manual runs. - Moved `metro-visualizer` and `@expo/metro-config` to `devDependencies` in `package.json`. - Replaced the `bc` dependency with `awk` for bundle size calculations.
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Join our Discord community for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
| if: contains(github.event.pull_request.labels.*.name, 'run-analysis') || github.event_name == 'workflow_dispatch' | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v5 | ||
| with: | ||
| ref: ${{ github.event.pull_request.head.sha || github.ref }} | ||
| fetch-depth: 0 | ||
|
|
||
| - name: Set up Node.js | ||
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: '18' | ||
| cache: 'npm' | ||
| cache-dependency-path: frontend/package-lock.json | ||
|
|
||
| - name: Install Dependencies | ||
| run: npm install | ||
| working-directory: ./frontend | ||
|
|
||
| - name: Setup EAS | ||
| uses: expo/expo-github-action@v8 | ||
| with: | ||
| eas-version: latest | ||
| token: ${{ secrets.EXPO_TOKEN }} | ||
|
|
||
| - name: Build for Bundle Analysis (Web) | ||
| run: | | ||
| if npx expo export --platform web --output-dir web-build 2>/dev/null; then | ||
| echo "✅ Expo web export successful" | ||
| else | ||
| echo "⚠️ Web export failed, using alternative bundle analysis approach" | ||
| npx metro build --entry-file index.js --platform web --bundle-output web-build/bundle.js --assets-dest web-build/static || true | ||
| fi | ||
| working-directory: ./frontend | ||
|
|
||
| - name: Analyze Web Bundle Size | ||
| working-directory: ./frontend | ||
| run: | | ||
| if [ ! -d "web-build" ]; then | ||
| echo "❌ No web-build directory found. Skipping analysis." | ||
| exit 0 | ||
| fi | ||
|
|
||
| TOTAL_SIZE=0 | ||
| if [ -d "web-build/static/js" ]; then | ||
| TOTAL_SIZE=$(find web-build/static/js -name "*.js" -type f -exec stat -c%s {} \; | awk '{sum+=$1} END {print sum}') | ||
| elif [ -f "web-build/bundle.js" ]; then | ||
| TOTAL_SIZE=$(stat -c%s web-build/bundle.js) | ||
| fi | ||
|
|
||
| if [ "$TOTAL_SIZE" -gt 0 ]; then | ||
| TOTAL_SIZE_MB=$(awk "BEGIN {printf \"%.2f\", $TOTAL_SIZE / 1024 / 1024}") | ||
| echo "📊 Total Web JavaScript bundle size: ${TOTAL_SIZE_MB} MB" | ||
|
|
||
| echo "# Web Bundle Analysis Report" > bundle-analysis-web.md | ||
| echo "## Bundle Size Summary" >> bundle-analysis-web.md | ||
| echo "- Total JavaScript bundle size: **${TOTAL_SIZE_MB} MB**" >> bundle-analysis-web.md | ||
| else | ||
| echo "❌ No JavaScript bundles found in expected locations." | ||
| fi | ||
|
|
||
| - name: Upload Web Bundle Analysis Report | ||
| if: always() && hashFiles('frontend/bundle-analysis-web.md') != '' | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: web-bundle-analysis-report | ||
| path: frontend/bundle-analysis-web.md | ||
|
|
||
| rn-bundle-analysis: |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
To fix the problem, explicitly set the permissions key at the top level of the workflow file. This will apply the specified permissions to all jobs in the workflow unless overridden at the job level. Since the jobs only need to read repository contents and upload artifacts, the minimal required permission is contents: read. This change should be made near the top of the file, after the name field and before the on field, to ensure it applies globally. No additional methods, imports, or definitions are needed.
-
Copy modified lines R1-R2
| @@ -1,3 +1,5 @@ | ||
| permissions: | ||
| contents: read | ||
| name: Code Analysis | ||
|
|
||
| on: |
| if: contains(github.event.pull_request.labels.*.name, 'run-analysis') || github.event_name == 'workflow_dispatch' | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v5 | ||
| with: | ||
| ref: ${{ github.event.pull_request.head.sha || github.ref }} | ||
| fetch-depth: 0 | ||
|
|
||
| - name: Set up Node.js | ||
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: '18' | ||
| cache: 'npm' | ||
| cache-dependency-path: frontend/package-lock.json | ||
|
|
||
| - name: Install Dependencies | ||
| run: npm install | ||
| working-directory: ./frontend | ||
|
|
||
| - name: Setup EAS | ||
| uses: expo/expo-github-action@v8 | ||
| with: | ||
| eas-version: latest | ||
| token: ${{ secrets.EXPO_TOKEN }} | ||
|
|
||
| - name: Build for Bundle Analysis (RN) | ||
| run: | | ||
| if npx expo export --platform android --output-dir android-build --max-workers 1; then | ||
| echo "✅ Android export successful" | ||
| else | ||
| echo "⚠️ Android export failed. Skipping analysis." | ||
| fi | ||
| working-directory: ./frontend | ||
|
|
||
| - name: Analyze RN Bundle Size | ||
| working-directory: ./frontend | ||
| run: | | ||
| if [ ! -d "android-build" ]; then | ||
| echo "❌ No android-build directory found. Skipping analysis." | ||
| exit 0 | ||
| fi | ||
|
|
||
| BUNDLE_FILE=$(find android-build/bundles -name "android-*.js" | head -1) | ||
| if [ -f "$BUNDLE_FILE" ]; then | ||
| BUNDLE_SIZE=$(stat -c%s "$BUNDLE_FILE") | ||
| BUNDLE_SIZE_MB=$(awk "BEGIN {printf \"%.2f\", $BUNDLE_SIZE / 1024 / 1024}") | ||
|
|
||
| echo "# React Native Bundle Analysis Report" > bundle-analysis-rn.md | ||
| echo "## Bundle Size Summary" >> bundle-analysis-rn.md | ||
| echo "- Platform: Android" >> bundle-analysis-rn.md | ||
| echo "- Bundle size: **${BUNDLE_SIZE_MB} MB**" >> bundle-analysis-rn.md | ||
| else | ||
| echo "❌ No Android bundle found." | ||
| fi | ||
|
|
||
| - name: Upload RN Bundle Analysis Report | ||
| if: always() && hashFiles('frontend/bundle-analysis-rn.md') != '' | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: rn-bundle-analysis-report | ||
| path: frontend/bundle-analysis-rn.md |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
To fix the problem, add a permissions block to the workflow file. This block should be placed at the top level (root) of the workflow, just after the name: and before the on: key, to apply to all jobs unless overridden. The minimal required permission for this workflow is likely contents: read, since the jobs only check out code, install dependencies, build, and upload artifacts, but do not interact with issues, pull requests, or perform repository writes. If in the future a job needs more permissions, they can be granted at the job level. No changes to the jobs themselves are required.
What to change:
- In
.github/workflows/code-analysis.yml, add:after thepermissions: contents: read
name: Code Analysisline and before theon:block (i.e., after line 1).
No new methods, imports, or definitions are needed.
-
Copy modified lines R1-R2
| @@ -1,3 +1,5 @@ | ||
| permissions: | ||
| contents: read | ||
| name: Code Analysis | ||
|
|
||
| on: |
This change fixes the
preview.ymlworkflow to allow it to run on pull requests from forked repositories, while also adding a security measure to require maintainer approval before running the workflow.