Skip to content

Conversation

@remicolin
Copy link
Collaborator

@remicolin remicolin commented Aug 21, 2025

Summary by CodeRabbit

  • Chores

    • CI now runs primarily on PRs (push triggers removed), adds staging as a target, and narrows path filters per workflow.
    • Mobile deploy CI moved to OIDC/GCP auth (id-token/ADC), added fast secret validation and dedicated Java setup for Android to fail faster on invalid secrets.
    • Version/build bumps and version-file updates moved to a PR-based flow; app build numbers and deployment timestamps updated.
    • Release/upload flows updated to prefer ADC in CI and fallback for local dev; improved error messaging on upload failures.
  • Documentation

    • README now instructs contributors to open PRs from the staging branch.

remicolin and others added 6 commits August 20, 2025 13:19
* update workflow to use workload identity federation

* add token permissions

* correct provider name

* chore: incrementing android build version for version 2.6.4 [github action]

---------

Co-authored-by: Self GitHub Actions <[email protected]>
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Aug 21, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

Removed push CI triggers and tightened workflow path filters; added staging to PR targets. Mobile deploy switched to OIDC/Workload Identity (ADC), moved version bumps to PR-based flow, added Android secret fast-fail checks and Java setup. Fastfile updated to prefer ADC in CI; README and app/version.json updated.

Changes

Cohort / File(s) Summary of changes
CI workflows (triggers)
.github/workflows/circuits.yml, .../.github/workflows/contracts.yml
Removed push triggers; added staging to pull_request.branches; tightened path filters to repo-specific dirs (e.g., circuits/**, contracts/**); removed obsolete branches/filters (openpassportv2, common/**).
Mobile deploy workflow
.github/workflows/mobile-deploy.yml
Added permissions: id-token: write; added google-github-actions/auth@v2 for Workload Identity Federation (ADC); replaced in-repo commit bumps with peter-evans/create-pull-request@v6 for iOS/Android and for version-file updates; removed unconditional Play Store JSON key usage in CI; added Android Java setup and early secret verification fast-paths.
Fastlane local vs CI auth & signing
app/fastlane/Fastfile
Made Play Store JSON key requirement conditional to local_development; CI uses ADC when available; replaced android.injected.* vars with MYAPP_UPLOAD_*; build upload now assembles upload_options and includes json_key only when appropriate; improved Play Store permission error messages; preserved local keystore initialization.
Docs
README.md
Added prominent notices instructing contributors to open PRs from staging; warns PRs from other branches will be auto-closed.
Version metadata
app/version.json
Bumped ios.build 163 → 165 and android.build 85 → 86; updated android.lastDeployed timestamp; ios.lastDeployed unchanged.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Dev as Developer
  participant GH as GitHub Actions (mobile-deploy)
  participant GCP as Google Cloud (WIF / ADC)
  participant PR as GitHub Pull Requests
  participant Repo as Repository

  Dev->>GH: Open PR / trigger workflow
  note right of GH: permissions: id-token: write (added)
  GH->>GCP: Authenticate via Workload Identity (google-github-actions/auth@v2)
  GCP-->>GH: Federated ADC credentials

  rect rgb(242,249,255)
    note right of GH: Build & validation (Android fast-fail added)
    GH->>GH: Setup Java
    GH->>GH: Verify Android secrets (fast-fail on invalid)
    GH->>GH: Build artifacts
  end

  GH->>PR: Create PR(s) for version bumps & version-file updates (target: staging)
  PR-->>Repo: PRs opened for review/merge
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested labels

codex

Suggested reviewers

  • aaronmgdr

Poem

Staging doors swing open wide,
Tokens bridge the cloud and tide.
PRs whisper version-change,
Secrets checked to stop the strange.
Builds take flight — a tidy ride. 🚀

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.


📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 76c08e2 and d17f1c2.

📒 Files selected for processing (1)
  • app/fastlane/Fastfile (3 hunks)
✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch staging

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
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.

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)
.github/workflows/contracts.yml (1)

46-49: Enforce Contract Tests in CI

The CI workflow at .github/workflows/contracts.yml currently skips the entire contracts test suite via if: false (lines 46–49), leaving on-chain code and deployment tooling untested. This poses a medium-to-high security risk.

Required actions:

  • Remove the if: false guard so that yarn test runs by default.
  • Identify and temporarily mark any genuinely flaky tests (e.g., via –testNamePattern or a custom tag) to restore CI green state.
  • Ensure that all critical contract tests (especially those covering deployment scripts, ABI changes, and common utilities) run on every merge.

Proposed minimal diff:

-      - name: Run Tests (Contracts)
-        working-directory: ./contracts
-        # skip until they get fixed
-        if: false
-        run: yarn test
+      - name: Run Tests (Contracts)
+        working-directory: ./contracts
+        run: yarn test --reporter=dot

Please isolate failures, tag flaky suites, and re-enable the full suite in CI.

.github/workflows/mobile-deploy.yml (3)

257-273: Remove debug step that prints SHA256 of secrets

Even hashed, publishing derived values of credentials is unnecessary risk and can enable offline guessing. The step is disabled, but leaving it around is a footgun.

Apply this diff to delete the step:

-      # for debugging...which can take some time :(
-      - name: Verify ios secret checksums
-        if: false # for debugging
-        run: |
-          echo "SHA256 of dist_cert.p12:"
-          shasum -a 256 ${{ env.APP_PATH }}${{ env.IOS_DIST_CERT_PATH }}
-          echo "SHA256 of profile.mobileprovision:"
-          shasum -a 256 ${{ env.APP_PATH }}${{ env.IOS_PROV_PROFILE_PROJ_PATH }}
-          echo "SHA256 of connect_api_key.p8:"
-          shasum -a 256 ${{ env.APP_PATH }}${{ env.IOS_CONNECT_API_KEY_PATH }}
-          echo "Certificate file size:"
-          ls -l ${{ env.APP_PATH }}${{ env.IOS_DIST_CERT_PATH }}
-          echo "SHA256 of password:"
-          echo -n "${{ secrets.IOS_P12_PASSWORD }}" | shasum -a 256
-          echo "SHA256 of connect_api_key_base64:"
-          echo -n "${{ secrets.IOS_CONNECT_API_KEY_BASE64 }}" | shasum -a 256
-          echo "Verifying certificate..."
-          if openssl pkcs12 -in ${{ env.APP_PATH }}${{ env.IOS_DIST_CERT_PATH }} -password pass:'${{ secrets.IOS_P12_PASSWORD }}' -info >/dev/null 2>&1 || openssl pkcs12 -in ${{ env.APP_PATH }}${{ env.IOS_DIST_CERT_PATH }} -password pass:'${{ secrets.IOS_P12_PASSWORD }}' -info 2>&1 | grep -q "MAC:"; then
-            echo "✅ Certificate verification successful (algorithm warning can be safely ignored)"
-          else
-            echo "❌ Certificate verification failed - please check certificate validity"
-            exit 1
-          fi

324-329: Keychain created with an empty password

Creating and unlocking a keychain with an empty password is avoidable even on ephemeral runners. Use a random, in-memory password.

-          security create-keychain -p "" build.keychain >/dev/null 2>&1
+          KEYCHAIN_PASS="$(openssl rand -hex 16)"
+          security create-keychain -p "$KEYCHAIN_PASS" build.keychain >/dev/null 2>&1
           security default-keychain -s build.keychain >/dev/null 2>&1
-          security unlock-keychain -p "" build.keychain >/dev/null 2>&1
+          security unlock-keychain -p "$KEYCHAIN_PASS" build.keychain >/dev/null 2>&1
           security import ${{ env.APP_PATH }}${{ env.IOS_DIST_CERT_PATH }} -k build.keychain -P '${{ secrets.IOS_P12_PASSWORD }}' -T /usr/bin/codesign >/dev/null 2>&1
-          security set-key-partition-list -S apple-tool:,apple: -s -k "" build.keychain >/dev/null 2>&1
+          security set-key-partition-list -S apple-tool:,apple: -s -k "$KEYCHAIN_PASS" build.keychain >/dev/null 2>&1

Remember to unset the variable at the end of the job: unset KEYCHAIN_PASS.


734-747: Avoid passing Android keystore secrets directly on the command line

Passing -storepass and -keypass inline risks exposure in process listings and logs. keytool supports reading passwords from environment variables or files using the :env or :file modifiers. Use them to reduce leakage risk.

-          if ! keytool -list -v -keystore "${{ env.APP_PATH }}${{ env.ANDROID_KEYSTORE_PATH }}" -storepass "${{ secrets.ANDROID_KEYSTORE_PASSWORD }}" >/dev/null 2>&1; then
+          if ! ANDROID_KEYSTORE_PASSWORD="${{ secrets.ANDROID_KEYSTORE_PASSWORD }}" \
+               keytool -list -v -keystore "${{ env.APP_PATH }}${{ env.ANDROID_KEYSTORE_PATH }}" -storepass:env ANDROID_KEYSTORE_PASSWORD >/dev/null 2>&1; then
             echo "❌ Error: Invalid keystore password"
             exit 1
           fi
           # Verify the key alias exists
-          if ! keytool -list -v -keystore "${{ env.APP_PATH }}${{ env.ANDROID_KEYSTORE_PATH }}" -storepass "${{ secrets.ANDROID_KEYSTORE_PASSWORD }}" -alias "${{ secrets.ANDROID_KEY_ALIAS }}" >/dev/null 2>&1; then
+          if ! ANDROID_KEYSTORE_PASSWORD="${{ secrets.ANDROID_KEYSTORE_PASSWORD }}" \
+               keytool -list -v -keystore "${{ env.APP_PATH }}${{ env.ANDROID_KEYSTORE_PATH }}" -storepass:env ANDROID_KEYSTORE_PASSWORD -alias "${{ secrets.ANDROID_KEY_ALIAS }}" >/dev/null 2>&1; then
             echo "❌ Error: Key alias '${{ secrets.ANDROID_KEY_ALIAS }}' not found in keystore"
             exit 1
           fi
           # Verify the key password
-          if ! keytool -list -v -keystore "${{ env.APP_PATH }}${{ env.ANDROID_KEYSTORE_PATH }}" -storepass "${{ secrets.ANDROID_KEYSTORE_PASSWORD }}" -alias "${{ secrets.ANDROID_KEY_ALIAS }}" -keypass "${{ secrets.ANDROID_KEY_PASSWORD }}" >/dev/null 2>&1; then
+          if ! ANDROID_KEYSTORE_PASSWORD="${{ secrets.ANDROID_KEYSTORE_PASSWORD }}" \
+               ANDROID_KEY_PASSWORD="${{ secrets.ANDROID_KEY_PASSWORD }}" \
+               keytool -list -v -keystore "${{ env.APP_PATH }}${{ env.ANDROID_KEYSTORE_PATH }}" -storepass:env ANDROID_KEYSTORE_PASSWORD -alias "${{ secrets.ANDROID_KEY_ALIAS }}" -keypass:env ANDROID_KEY_PASSWORD >/dev/null 2>&1; then
             echo "❌ Error: Invalid key password"
             exit 1
           fi

This uses keytool’s :env mechanism to avoid printing secret values in command lines. See Oracle docs for -storepass[:env|:file] and related options. (docs.oracle.com)

🧹 Nitpick comments (3)
.github/workflows/mobile-deploy.yml (3)

530-541: Pin third-party action: create-pull-request

This step has write permissions and operates on branches. Pin to a specific commit SHA for defense-in-depth.

-      - name: Open PR for iOS build number bump
-        if: ${{ !env.ACT && success() }}
-        uses: peter-evans/create-pull-request@v6
+      - name: Open PR for iOS build number bump
+        if: ${{ !env.ACT && success() }}
+        uses: peter-evans/create-pull-request@<COMMIT_SHA_FOR_v6>

Repeat for Android and update-version PR steps below.


800-811: Same as iOS: pin create-pull-request to a commit SHA

This action writes branches and PRs; pinning reduces supply-chain risk.

-        uses: peter-evans/create-pull-request@v6
+        uses: peter-evans/create-pull-request@<COMMIT_SHA_FOR_v6>

902-916: Update-version PR: pin action and double-check included paths

  • Pin the action to a commit SHA.
  • Confirm you actually want to include yarn.lock here. yarn version may not touch the lockfile depending on your workspace setup; including it can create noisy diffs.
-      - name: Open PR to update version files
-        uses: peter-evans/create-pull-request@v6
+      - name: Open PR to update version files
+        uses: peter-evans/create-pull-request@<COMMIT_SHA_FOR_v6>
         with:
           title: "chore: update version files after deployment"
           body: |
             Automated update of version files after successful deployment.
             Includes updates to `app/version.json`, `app/package.json`, and `yarn.lock`.
           commit-message: "chore: update version files after deployment [skip ci]"
           branch: ci/update-version-${{ github.run_id }}
           base: staging
           add-paths: |
             app/version.json
             app/package.json
             yarn.lock
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 1cbabbb and 53b8790.

📒 Files selected for processing (5)
  • .github/workflows/circuits.yml (1 hunks)
  • .github/workflows/contracts.yml (1 hunks)
  • .github/workflows/mobile-deploy.yml (5 hunks)
  • README.md (1 hunks)
  • app/version.json (1 hunks)
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: e2e-ios
  • GitHub Check: analyze-android
  • GitHub Check: analyze-ios
🔇 Additional comments (3)
app/version.json (1)

3-9: Version metadata sanity check: iOS build bumped but lastDeployed unchanged

  • ios.build was incremented to 165 while ios.lastDeployed remains at 2025-08-08T22:35:10Z. If no iOS deployment actually occurred, consider reverting the build bump or updating lastDeployed to the actual deployment timestamp to avoid misleading release automation (e.g., tag generation in .github/workflows/mobile-deploy.yml create-release-tags). Similarly, confirm that the Android lastDeployed reflects the exact rollout time.
.github/workflows/contracts.yml (1)

6-6: Trigger update LGTM

Adding staging to the PR branches aligns with the repo’s new staging-first flow. No concerns here.

.github/workflows/circuits.yml (1)

6-6: Trigger update LGTM

Including staging in PR branches is consistent with the staging-first policy. No issues.

Comment on lines 8 to 10
paths:
- "circuits/**"
- "common/**"
jobs:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Path filter may miss changes in shared code used by circuits

This workflow only triggers on changes under circuits/. If circuits depend on code in common/ (typical in monorepos), changes there won’t run circuit tests. That’s a blind spot that can let breaking changes slip through.

Consider adding common/** back to the paths, or split the job condition with a more precise dependency map. Example:

 on:
   pull_request:
     branches:
       - dev
       - staging
       - main
     paths:
-      - "circuits/**"
+      - "circuits/**"
+      - "common/**"

If dependency is unidirectional and truly isolated now, ignore this; otherwise, I recommend restoring the coverage.


🏁 Script executed:

#!/bin/bash
# List top-level directories to check for a shared “common” folder
echo "Top-level directories:"
ls -1

# Verify if common/ exists
if [ -d common ]; then
  echo "common/ directory exists"
else
  echo "common/ directory does not exist"
fi

# Search circuits/ for imports that reference common/ via relative paths
echo -e "\nSearching for relative imports from common in circuits/:"
rg -nP --type ts --type tsx "from ['\"](?:\.\.\/)+common" -C3 circuits || echo "No relative TS/TSX imports from common found"
rg -nP --type js --type jsx "from ['\"](?:\.\.\/)+common" -C3 circuits || echo "No relative JS/JSX imports from common found"

# Search circuits/ for path-alias imports (e.g. '@common/...')
echo -e "\nSearching for alias imports from common in circuits/:"
rg -nP "from ['\"][\@]?common" -C3 circuits || echo "No alias imports from common found"

Length of output: 1105


🏁 Script executed:

#!/bin/bash
# Thoroughly search for any reference to "common" within circuits/
echo "Searching for literal 'common/' references in circuits/:"
rg -n "common/" -C3 circuits || echo "No literal 'common/' references found in circuits"

echo -e "\nSearching for ES module imports referencing 'common':"
rg -n "import .* from ['\"].*common" -C3 circuits || echo "No ES module imports referencing 'common' found in circuits"

echo -e "\nSearching for CommonJS requires referencing 'common':"
rg -n "require\s*\(['\"].*common" -C3 circuits || echo "No CommonJS require() calls referencing 'common' found in circuits"

Length of output: 32539


CI trigger must include shared common/

The circuits workflow is currently scoped to changes under circuits/**, but the circuits tests import and rely on code from common/** (e.g., path aliases in circuits/tsconfig.spec.json and numerous @selfxyz/common/... imports in the test files). Without adding common/** to the paths filter, updates to shared utilities won’t trigger the circuits job, risking undetected breaking changes.

Changes required:

• In .github/workflows/circuits.yml, update the paths section:

 on:
   pull_request:
     branches:
       - dev
       - staging
       - main
     paths:
-      - "circuits/**"
+      - "circuits/**"
+      - "common/**"

This ensures any edits to the shared common code will rerun the circuits tests.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
paths:
- "circuits/**"
- "common/**"
jobs:
on:
pull_request:
branches:
- dev
- staging
- main
paths:
- "circuits/**"
- "common/**"
jobs:
#
🤖 Prompt for AI Agents
In .github/workflows/circuits.yml around lines 8 to 10 the paths filter only
includes "circuits/**" which means changes to shared code under common/** won’t
trigger the circuits workflow; update the paths list to include "common/**"
alongside "circuits/**" so that edits to the shared utilities will run the
circuits CI job (i.e., add an entry "common/**" under the paths key).

permissions:
contents: write
pull-requests: write
id-token: write
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Scope OIDC permissions to least privilege

id-token: write is set at the workflow level, granting all jobs the ability to request OIDC tokens. Only the iOS job uses Google Workload Identity. Move this permission to the iOS job to reduce blast radius.

Apply this diff to remove the top-level grant:

 permissions:
   contents: write
   pull-requests: write
-  id-token: write

Then add job-scoped permissions under build-ios:

# under jobs.build-ios:
permissions:
  contents: write
  pull-requests: write
  id-token: write
🤖 Prompt for AI Agents
.github/workflows/mobile-deploy.yml around line 33: the workflow grants
id-token: write at the top-level which should be scoped to the iOS job only;
remove the top-level id-token: write entry and add job-scoped permissions under
jobs.build-ios instead, setting permissions to include contents: write,
pull-requests: write, and id-token: write so only the iOS job can request OIDC
tokens.

Comment on lines 120 to 125
- uses: 'google-github-actions/auth@v2'
with:
project_id: 'plucky-tempo-454713-r0'
workload_identity_provider: 'projects/852920390127/locations/global/workloadIdentityPools/gh-self/providers/github-by-repos'
service_account: "[email protected]"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

OIDC auth step: pin action to a commit SHA and consider audience restriction

You’re authenticating to GCP; treat this as sensitive supply chain surface. Pin third-party actions to immutable SHAs and (optionally) set an explicit audience if relying parties check it.

  • Replace google-github-actions/auth@v2 with the specific commit SHA for v2.
  • If any downstream tool verifies aud, set audience: accordingly.

Example:

-      - uses: 'google-github-actions/auth@v2'
+      - uses: google-github-actions/auth@<COMMIT_SHA_FOR_v2>
         with:
           project_id: 'plucky-tempo-454713-r0'
           workload_identity_provider: 'projects/852920390127/locations/global/workloadIdentityPools/gh-self/providers/github-by-repos'
           service_account: "[email protected]"
+          # audience: 'https://sts.googleapis.com' # if required by downstream consumer

Repeat pinning for other third-party actions in this workflow (e.g., peter-evans/create-pull-request). This materially reduces supply-chain risk.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- uses: 'google-github-actions/auth@v2'
with:
project_id: 'plucky-tempo-454713-r0'
workload_identity_provider: 'projects/852920390127/locations/global/workloadIdentityPools/gh-self/providers/github-by-repos'
service_account: "[email protected]"
- uses: google-github-actions/auth@<COMMIT_SHA_FOR_v2>
with:
project_id: 'plucky-tempo-454713-r0'
workload_identity_provider: 'projects/852920390127/locations/global/workloadIdentityPools/gh-self/providers/github-by-repos'
service_account: "[email protected]"
# audience: 'https://sts.googleapis.com' # if required by downstream consumer
🤖 Prompt for AI Agents
.github/workflows/mobile-deploy.yml lines 120-125: the OIDC auth step currently
uses a floating tag google-github-actions/auth@v2 and lacks an explicit
audience; replace the tag with the specific pinned commit SHA for the v2 release
(e.g., google-github-actions/auth@<commit-sha>) and, if your GCP workload
identity or downstream services validate the JWT audience, add an audience:
field with the expected value; also audit and pin other third-party actions in
this workflow (like peter-evans/create-pull-request) to immutable SHAs to reduce
supply-chain risk.

Comment on lines +89 to +90
> **Important:** Please open your pull request from the `staging` branch. Pull requests from other branches will be automatically closed.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Clarify PR policy: “to staging” vs “from staging” and ensure enforcement

The sentence suggests opening PRs “from the staging branch,” which is atypical and likely means “to the staging branch.” Also, the claim that PRs from other branches are auto-closed isn’t enforced by any workflow in this PR. Recommend rewording and, if desired, adding a small workflow to enforce it.

Apply this wording fix:

- > **Important:** Please open your pull request from the `staging` branch. Pull requests from other branches will be automatically closed.
+ > **Important:** Please open your pull request to the `staging` branch. Pull requests targeting other base branches may be closed per our workflow policy.

If you want auto-closure, I can add a lightweight workflow that comments and closes PRs whose base != staging. Want me to open a follow-up PR?

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
> **Important:** Please open your pull request from the `staging` branch. Pull requests from other branches will be automatically closed.
> **Important:** Please open your pull request to the `staging` branch. Pull requests targeting other base branches may be closed per our workflow policy.
🤖 Prompt for AI Agents
In README.md around lines 89 to 90, the sentence wrongly instructs opening PRs
"from the staging branch" and asserts auto-closure of PRs from other branches
without an enforcing workflow; update the wording to say "open your pull request
to the staging branch" and remove or soften the claim about automatic closure
(e.g., "PRs not targeting staging may be closed"). If you want enforcement, add
a lightweight GitHub Actions workflow that checks pull_request.base.ref !=
'staging' and posts a comment then closes the PR; otherwise omit the enforcement
claim from the README.

Copy link
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.

Actionable comments posted: 0

♻️ Duplicate comments (2)
.github/workflows/mobile-deploy.yml (2)

33-33: Scope OIDC permission to the job that actually needs it (Android)

Top-level id-token: write grants all jobs OIDC token issuance. Only the Android job uses google-github-actions/auth now, so scope this permission to that job to minimize blast radius. Reiterating prior guidance with updated scope.

Apply this diff:

 permissions:
   contents: write
   pull-requests: write
-  id-token: write

Then under jobs.build-android add:

 jobs:
   build-android:
+    permissions:
+      id-token: write
     runs-on: ubuntu-latest

575-580: Pin google-github-actions/auth to a commit SHA and (optionally) set audience

Floating tags increase supply-chain risk. Pin the action to an immutable commit SHA for v2 and consider adding audience if your relying party validates it. This is especially important for WIF to GCP.

-      - uses: 'google-github-actions/auth@v2'
+      - uses: google-github-actions/auth@<COMMIT_SHA_FOR_v2>
         with:
           project_id: 'plucky-tempo-454713-r0'
           workload_identity_provider: 'projects/852920390127/locations/global/workloadIdentityPools/gh-self/providers/github-by-repos'
           service_account: "[email protected]"
+          # audience: 'https://sts.googleapis.com' # uncomment if required by downstream verifier

If you’d like, I can follow up with the exact SHAs for the actions you’re using.

🧹 Nitpick comments (3)
.github/workflows/mobile-deploy.yml (3)

526-536: Pin peter-evans/create-pull-request to a commit SHA

This action has repository write permissions; pinning it to an immutable SHA reduces supply-chain risk. Repeat for other uses below.

-      - name: Open PR for iOS build number bump
+      - name: Open PR for iOS build number bump
         if: ${{ !env.ACT && success() }}
-        uses: peter-evans/create-pull-request@v6
+        uses: peter-evans/create-pull-request@<COMMIT_SHA_FOR_v6>
         with:
           title: "chore: bump iOS build for ${{ env.VERSION }}"
           body: "Automated bump of iOS build number by CI"
           commit-message: "chore: incrementing ios build number for version ${{ env.VERSION }} [github action]"
           branch: ci/bump-ios-build-${{ github.run_id }}
           base: staging
           add-paths: |
             app/version.json

794-805: Pin peter-evans/create-pull-request to a commit SHA (Android PR bump)

Same supply-chain rationale as above.

-      - name: Open PR for Android build number bump
+      - name: Open PR for Android build number bump
         if: ${{ !env.ACT && success() }}
-        uses: peter-evans/create-pull-request@v6
+        uses: peter-evans/create-pull-request@<COMMIT_SHA_FOR_v6>
         with:
           title: "chore: bump Android build for ${{ env.VERSION }}"
           body: "Automated bump of Android build number by CI"
           commit-message: "chore: incrementing android build version for version ${{ env.VERSION }} [github action]"
           branch: ci/bump-android-build-${{ github.run_id }}
           base: staging
           add-paths: |
             app/version.json

896-910: Pin peter-evans/create-pull-request to a commit SHA (post-deploy version PR)

This step updates multiple files; pin the action to reduce risk of tag hijack.

-      - name: Open PR to update version files
-        uses: peter-evans/create-pull-request@v6
+      - name: Open PR to update version files
+        uses: peter-evans/create-pull-request@<COMMIT_SHA_FOR_v6>
         with:
           title: "chore: update version files after deployment"
           body: |
             Automated update of version files after successful deployment.
             Includes updates to `app/version.json`, `app/package.json`, and `yarn.lock`.
           commit-message: "chore: update version files after deployment [skip ci]"
           branch: ci/update-version-${{ github.run_id }}
           base: staging
           add-paths: |
             app/version.json
             app/package.json
             yarn.lock
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 53b8790 and 4d956b1.

📒 Files selected for processing (2)
  • .github/workflows/mobile-deploy.yml (7 hunks)
  • app/fastlane/Fastfile (3 hunks)
🧰 Additional context used
🪛 RuboCop (1.76.1)
app/fastlane/Fastfile

[convention] 382-382: Avoid comma after the last parameter of a method call.

(Style/TrailingCommaInArguments)

⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: build-android
  • GitHub Check: e2e-ios
  • GitHub Check: build
  • GitHub Check: lint
🔇 Additional comments (4)
app/fastlane/Fastfile (3)

359-361: Good call: avoid Play Store JSON key in CI by scoping the requirement to local dev

Conditionally requiring ANDROID_PLAY_STORE_JSON_KEY_PATH only for local_development reduces secret sprawl in CI and aligns with ADC/WIF usage. No concerns.


379-385: Validation gated to local machines is the right trade-off

Running validate_play_store_json_key only when local_development is true keeps CI free of long-lived JSON keys and enforces correctness for developer machines. This is the safer posture with WIF.


406-419: CI-safe upload path: omit json_key and rely on ADC

Building upload_options and adding json_key only for local_development prevents accidental key usage in CI. The explicit options hash also improves readability and future extension. Looks solid.

.github/workflows/mobile-deploy.yml (1)

718-721: Guardrail to ensure ADC is active before build

Failing fast when GOOGLE_APPLICATION_CREDENTIALS is unset catches misconfigured WIF early and prevents confusing Fastlane failures. This is a meaningful security and DX improvement.

Copy link
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.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
.github/workflows/mobile-deploy.yml (2)

937-947: Release tags may use stale build numbers — they’re read from files on a new checkout, while bumps are only opened as PRs

create-release-tags reads package.json and version.json from a fresh checkout. Your build number bumps are proposed via PRs (not merged), so tags like vX.Y.Z-ios-<build> and vX.Y.Z-android-<build> risk being created with previous build numbers, not the ones that actually shipped. This can mislabel releases and break traceability.

Minimal fix: pass the effective version and platform build numbers as job outputs from build-ios / build-android, then consume them here instead of re-reading files.

Patch for create-release-tags to use job outputs:

       - name: Create and push tags
         run: |
-          cd ${{ env.APP_PATH }}
-
-          # Read current version info
-          VERSION=$(cat package.json | jq -r .version)
-          IOS_BUILD=$(cat version.json | jq -r .ios.build)
-          ANDROID_BUILD=$(cat version.json | jq -r .android.build)
+          VERSION='${{ needs.build-ios.outputs.version || needs.build-android.outputs.version }}'
+          IOS_BUILD='${{ needs.build-ios.outputs.ios_build }}'
+          ANDROID_BUILD='${{ needs.build-android.outputs.android_build }}'
+          cd ${{ env.APP_PATH }}

Add outputs to the build jobs (outside the selected ranges; example snippet):

# jobs.build-ios
outputs:
  version: ${{ steps.readver.outputs.version }}
  ios_build: ${{ steps.readver.outputs.ios_build }}
steps:
  # after fastlane deploy_auto completes
  - id: readver
    name: Read deployed iOS version/build
    run: |
      cd "${{ env.APP_PATH }}"
      echo "version=$(jq -r .version version.json)" >> "$GITHUB_OUTPUT"
      echo "ios_build=$(jq -r .ios.build version.json)" >> "$GITHUB_OUTPUT"

# jobs.build-android
outputs:
  version: ${{ steps.readver.outputs.version }}
  android_build: ${{ steps.readver.outputs.android_build }}
steps:
  # after fastlane android deploy_auto completes
  - id: readver
    name: Read deployed Android version/build
    run: |
      cd "${{ env.APP_PATH }}"
      echo "version=$(jq -r .version version.json)" >> "$GITHUB_OUTPUT"
      echo "android_build=$(jq -r .android.build version.json)" >> "$GITHUB_OUTPUT"

This ensures tags reflect the deployed artifacts even before the PRs are merged. I can provide a full, consistent patch across the three jobs if helpful.

Also applies to: 952-967


253-269: Don’t log secret material (even hashed) — remove password/hash echoes from the debug step

Although this block is gated by if: false, the lines printing the SHA256 of the P12 password and base64 API key would still leak sensitive signals if someone flips the flag. Avoid emitting any derivation of secrets to logs.

Apply this diff to strip the risky lines while keeping the rest of the debug harness intact:

       - name: Verify ios secret checksums
         if: false # for debugging
         run: |
           echo "SHA256 of dist_cert.p12:"
           shasum -a 256 ${{ env.APP_PATH }}${{ env.IOS_DIST_CERT_PATH }}
           echo "SHA256 of profile.mobileprovision:"
           shasum -a 256 ${{ env.APP_PATH }}${{ env.IOS_PROV_PROFILE_PROJ_PATH }}
           echo "SHA256 of connect_api_key.p8:"
           shasum -a 256 ${{ env.APP_PATH }}${{ env.IOS_CONNECT_API_KEY_PATH }}
-          echo "Certificate file size:"
-          ls -l ${{ env.APP_PATH }}${{ env.IOS_DIST_CERT_PATH }}
-          echo "SHA256 of password:"
-          echo -n "${{ secrets.IOS_P12_PASSWORD }}" | shasum -a 256
-          echo "SHA256 of connect_api_key_base64:"
-          echo -n "${{ secrets.IOS_CONNECT_API_KEY_BASE64 }}" | shasum -a 256
+          echo "Certificate file size:"
+          ls -l ${{ env.APP_PATH }}${{ env.IOS_DIST_CERT_PATH }}

If you need diagnostics, prefer “presence and shape” checks (sizes, permission bits) and ephemeral test actions over printing any secret-derived values.

♻️ Duplicate comments (1)
.github/workflows/mobile-deploy.yml (1)

33-33: Scope OIDC to the Android job; remove top-level id-token grant

Granting id-token: write at workflow scope lets every job mint OIDC tokens. Only the Android job calls google-github-actions/auth, so least-privilege suggests moving this permission to jobs.build-android. This reduces the blast radius if any other step/action is compromised.

Apply this diff to drop the top-level grant and add it to the Android job:

 permissions:
   contents: write
   pull-requests: write
-  id-token: write
 jobs:
   build-android:
+    permissions:
+      contents: write          # needed for peter-evans/create-pull-request
+      pull-requests: write     # needed for peter-evans/create-pull-request
+      id-token: write          # needed for google-github-actions/auth OIDC
     runs-on: ubuntu-latest
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 4d956b1 and cd3671e.

📒 Files selected for processing (1)
  • .github/workflows/mobile-deploy.yml (6 hunks)
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: e2e-ios
  • GitHub Check: analyze-android
  • GitHub Check: analyze-ios
  • GitHub Check: gitleaks

Comment on lines +575 to +580
- uses: 'google-github-actions/auth@v2'
with:
project_id: 'plucky-tempo-454713-r0'
workload_identity_provider: 'projects/852920390127/locations/global/workloadIdentityPools/gh-self/providers/github-by-repos'
service_account: "[email protected]"
# Fail fast: set up JDK for keytool and verify Android secrets early
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Pin third‑party actions to immutable SHAs; optionally set OIDC audience

Supply-chain hardening: avoid floating tags for third‑party actions. Pin to the release commit SHAs for reproducibility and tamper resistance. For OIDC, consider setting audience: if a relying party validates aud.

Pin these occurrences (replace PLACEHOLDER_SHA with the official release commit SHAs):

-      - uses: 'google-github-actions/auth@v2'
+      - uses: google-github-actions/auth@PLACEHOLDER_SHA
         with:
           project_id: 'plucky-tempo-454713-r0'
           workload_identity_provider: 'projects/852920390127/locations/global/workloadIdentityPools/gh-self/providers/github-by-repos'
           service_account: "[email protected]"
+          # audience: 'https://sts.googleapis.com' # optional, if enforced downstream
-        uses: peter-evans/create-pull-request@v6
+        uses: peter-evans/create-pull-request@PLACEHOLDER_SHA

(repeat for lines 796-806 and 898-912)

-        uses: softprops/action-gh-release@v1
+        uses: softprops/action-gh-release@PLACEHOLDER_SHA
-        uses: android-actions/setup-android@v3
+        uses: android-actions/setup-android@PLACEHOLDER_SHA
-        uses: maxim-lobanov/setup-xcode@v1
+        uses: maxim-lobanov/setup-xcode@PLACEHOLDER_SHA

Note: You can keep GitHub-maintained actions (e.g., actions/checkout, actions/cache, actions/setup-node, actions/setup-java) on major tags if your policy allows, but pinning them too is even stronger. If you want, I can prepare a patch with the exact SHAs you’re targeting.

Also applies to: 526-536, 796-806, 898-912, 1000-1008, 712-716, 122-127

🤖 Prompt for AI Agents
.github/workflows/mobile-deploy.yml lines 575-580: the workflow uses
google-github-actions/auth@v2 with a floating tag—pin the action to its
immutable release commit SHA (replace the tag with the corresponding commit SHA
for that release) and optionally add an audience: field under with: if your OIDC
relying party checks aud; apply the same change to the other occurrences listed
(lines 122-127, 526-536, 712-716, 796-806, 898-912, 1000-1008) to harden the
supply chain and ensure reproducible deployments.

Copy link
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.

Actionable comments posted: 0

♻️ Duplicate comments (8)
.github/workflows/mobile-deploy.yml (8)

124-127: Pin third-party action to immutable SHA (setup-xcode)

Pin maxim-lobanov/setup-xcode@v1 to a release commit SHA to harden supply chain. Floating tags are mutable.

-        uses: maxim-lobanov/setup-xcode@v1
+        uses: maxim-lobanov/setup-xcode@<COMMIT_SHA_FOR_v1>

728-731: Pin android-actions/setup-android to an immutable SHA

Floating tags increase supply‑chain risk. Pin the action to a release commit SHA.

-        uses: android-actions/setup-android@v3
+        uses: android-actions/setup-android@<COMMIT_SHA_FOR_v3>

1015-1015: Pin softprops/action-gh-release to commit SHA

Pinning reduces tampering risk.

-        uses: softprops/action-gh-release@v1
+        uses: softprops/action-gh-release@<COMMIT_SHA_FOR_v1>

33-33: Scope OIDC permission to least privilege

id-token: write is granted at workflow level. In this workflow, only the Android job uses google-github-actions/auth. Move id-token: write down to jobs.build-android to reduce token issuance surface.

 permissions:
   contents: write
   pull-requests: write
-  id-token: write

Add job-scoped permissions for build-android:

 jobs:
   build-android:
+    permissions:
+      id-token: write
     runs-on: ubuntu-latest

526-537: Pin create-pull-request action to a commit SHA (iOS bump PR)

Security/reproducibility: pin peter-evans/create-pull-request to an immutable SHA.

-        uses: peter-evans/create-pull-request@v6
+        uses: peter-evans/create-pull-request@<COMMIT_SHA_FOR_v6>

575-580: Pin google-github-actions/auth and consider audience

Pin auth@v2 to a commit SHA and optionally set audience if any relying party validates aud.

-      - uses: 'google-github-actions/auth@v2'
+      - uses: google-github-actions/auth@<COMMIT_SHA_FOR_v2>
         with:
           project_id: 'plucky-tempo-454713-r0'
           workload_identity_provider: 'projects/852920390127/locations/global/workloadIdentityPools/gh-self/providers/github-by-repos'
           service_account: "[email protected]"
+          # audience: 'https://sts.googleapis.com' # if enforced by downstream tools

811-821: Pin create-pull-request action to a commit SHA (Android bump PR)

Same as iOS bump step—pin to SHA for reproducibility.

-        uses: peter-evans/create-pull-request@v6
+        uses: peter-evans/create-pull-request@<COMMIT_SHA_FOR_v6>

913-926: Pin create-pull-request action to a commit SHA (update-version PR)

Apply the same pinning here.

-      - name: Open PR to update version files
-        uses: peter-evans/create-pull-request@v6
+      - name: Open PR to update version files
+        uses: peter-evans/create-pull-request@<COMMIT_SHA_FOR_v6>
🧹 Nitpick comments (3)
app/fastlane/Fastfile (1)

406-419: ADC-only uploads: confirm Fastlane Supply version and add resilience to transient 5xx

CI now relies on ADC (no json_key). Ensure your fastlane/supply version supports GOOGLE_APPLICATION_CREDENTIALS default credentials in headless CI. If there’s any doubt, explicitly export GOOGLE_APPLICATION_CREDENTIALS in the job’s env (the workflow already checks it exists) and keep supply up to date.

Optionally, add retry around upload_to_play_store for transient 5xx/rate limits:

-          upload_to_play_store(upload_options)
+          Fastlane::Helpers.with_retry(max_retries: 3, delay: 15) do
+            upload_to_play_store(upload_options)
+          end

Also consider matching permission errors case‑insensitively and surfacing the underlying reason if available to speed up debugging.

.github/workflows/mobile-deploy.yml (2)

581-636: Secret verification is solid; add set -euo pipefail for stricter failure on errors

The ADC and keystore validations are thorough. To avoid partial‑success masking, enable strict mode at the top of the run block.

-        run: |
+        run: |
+          set -euo pipefail
           # Verify Google Cloud auth via Workload Identity Federation (ADC)
           if [ -z "$GOOGLE_APPLICATION_CREDENTIALS" ] || [ ! -f "$GOOGLE_APPLICATION_CREDENTIALS" ]; then
             ...

22-29: Remove unused ANDROID_PLAY_STORE_JSON_KEY_PATH from workflow env

The pipeline has migrated to ADC and no longer uses JSON key files in CI. Keeping ANDROID_PLAY_STORE_JSON_KEY_PATH at the workflow env level can confuse future contributors and encourage regressions to file-based creds. Remove it from global env; local developers can rely on Fastlane defaults.

   # Certificate/keystore paths
   ANDROID_KEYSTORE_PATH: /android/app/upload-keystore.jks
-  ANDROID_PLAY_STORE_JSON_KEY_PATH: /android/play-store-key.json
   IOS_DIST_CERT_PATH: /ios/certs/dist_cert.p12
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between cd3671e and 130f192.

📒 Files selected for processing (2)
  • .github/workflows/mobile-deploy.yml (6 hunks)
  • app/fastlane/Fastfile (3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-29T01:08:28.530Z
Learnt from: transphorm
PR: selfxyz/self#795
File: app/android/app/build.gradle:157-158
Timestamp: 2025-07-29T01:08:28.530Z
Learning: For this React Native project, the team prefers build flexibility over fail-fast behavior for release builds in app/android/app/build.gradle. They intentionally allow fallback to debug signing for local development runs, relying on Google Play Console validation to catch any improperly signed releases during upload.

Applied to files:

  • app/fastlane/Fastfile
🪛 RuboCop (1.76.1)
app/fastlane/Fastfile

[convention] 382-382: Avoid comma after the last parameter of a method call.

(Style/TrailingCommaInArguments)

🔇 Additional comments (4)
app/fastlane/Fastfile (3)

359-361: Good: JSON key required only for local development

Conditionally requiring ANDROID_PLAY_STORE_JSON_KEY_PATH only when running locally reduces the blast radius of long‑lived keys in CI and aligns with ADC. No issues.


379-385: Good: Validate Play JSON key only in local dev (ADC in CI)

Keeping validate_play_store_json_key behind local_development avoids coupling CI to file credentials. This is the right direction given Workload Identity Federation.


391-395: Release signing configuration validated

I’ve confirmed that in app/android/app/build.gradle the release signingConfigs block checks for MYAPP_UPLOAD_STORE_FILE and correctly assigns:

  • storeFile file(MYAPP_UPLOAD_STORE_FILE)
  • storePassword MYAPP_UPLOAD_STORE_PASSWORD
  • keyAlias MYAPP_UPLOAD_KEY_ALIAS
  • keyPassword MYAPP_UPLOAD_KEY_PASSWORD

And buildTypes.release uses signingConfigs.release (see lines 149–153 and 168). As a result, Fastlane’s MYAPP_UPLOAD_* properties are wired in, and unsigned builds won’t occur. No changes needed here.

.github/workflows/mobile-deploy.yml (1)

120-121: No-op change

The empty change around these lines is harmless. Nothing to address.

Copy link
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.

Actionable comments posted: 3

🧹 Nitpick comments (2)
app/fastlane/Fastfile (2)

379-385: Gate Play Store key validation to local only; fix trailing comma flagged by RuboCop

Nice call to skip JSON key validation in CI. RuboCop flagged a trailing comma in the argument list (Style/TrailingCommaInArguments). Removing it keeps the linters green.

Apply this diff:

-      validate_play_store_json_key(
-        json_key: ENV["ANDROID_PLAY_STORE_JSON_KEY_PATH"],
-      )
+      validate_play_store_json_key(
+        json_key: ENV["ANDROID_PLAY_STORE_JSON_KEY_PATH"]
+      )

391-395: Ensure legacy AGP signing props are forwarded alongside new MYAPP_UPLOAD_ variables*

The release signing block in app/android/app/build.gradle only checks for MYAPP_UPLOAD_* properties and will fall back to debug signing if those aren’t present. Until all CI and local invocations switch over to the new keys, it’s safer to pass both sets of properties from Fastlane to avoid inadvertent debug‐signed releases.

• File: app/fastlane/Fastfile (around lines 391–395)
• Action: add the legacy android.injected.signing.* properties under properties: in your lane

Suggested diff:

           properties: {
             "MYAPP_UPLOAD_STORE_FILE"     => ENV["ANDROID_KEYSTORE_PATH"],
             "MYAPP_UPLOAD_STORE_PASSWORD" => ENV["ANDROID_KEYSTORE_PASSWORD"],
             "MYAPP_UPLOAD_KEY_ALIAS"      => ENV["ANDROID_KEY_ALIAS"],
             "MYAPP_UPLOAD_KEY_PASSWORD"   => ENV["ANDROID_KEY_PASSWORD"] == "EMPTY" ? "" : ENV["ANDROID_KEY_PASSWORD"],
+            // Back-compat with AGP injected signing to avoid breakage if Gradle isn’t yet updated
+            "android.injected.signing.store.file"     => ENV["ANDROID_KEYSTORE_PATH"],
+            "android.injected.signing.store.password" => ENV["ANDROID_KEYSTORE_PASSWORD"],
+            "android.injected.signing.key.alias"      => ENV["ANDROID_KEY_ALIAS"],
+            "android.injected.signing.key.password"   => ENV["ANDROID_KEY_PASSWORD"] == "EMPTY" ? "" : ENV["ANDROID_KEY_PASSWORD"],
           },

This ensures that any callers still using Gradle’s legacy -Pandroid.injected.signing.* flags continue to produce properly signed release APKs.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 130f192 and ffc482b.

📒 Files selected for processing (1)
  • app/fastlane/Fastfile (3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-29T01:08:28.530Z
Learnt from: transphorm
PR: selfxyz/self#795
File: app/android/app/build.gradle:157-158
Timestamp: 2025-07-29T01:08:28.530Z
Learning: For this React Native project, the team prefers build flexibility over fail-fast behavior for release builds in app/android/app/build.gradle. They intentionally allow fallback to debug signing for local development runs, relying on Google Play Console validation to catch any improperly signed releases during upload.

Applied to files:

  • app/fastlane/Fastfile
🪛 RuboCop (1.76.1)
app/fastlane/Fastfile

[convention] 382-382: Avoid comma after the last parameter of a method call.

(Style/TrailingCommaInArguments)

⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: e2e-ios
  • GitHub Check: analyze-android
🔇 Additional comments (1)
app/fastlane/Fastfile (1)

414-417: LGTM: Conditional json_key only for local dev

Passing json_key only for local development and relying on ADC in CI is the right trade-off for least-privilege and secret hygiene.

Comment on lines +359 to 361
# Only require JSON key path when not running in CI (local development)
required_env_vars << "ANDROID_PLAY_STORE_JSON_KEY_PATH" if local_development

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

ADC in CI: good direction; verify GitHub Actions OIDC wiring to avoid silent auth falls-backs

Requiring ANDROID_PLAY_STORE_JSON_KEY_PATH only for local development is aligned with minimizing long‑lived secrets in CI. To ensure uploads don’t fail due to misconfigured Workload Identity Federation, please verify the workflow contains the google-github-actions/auth step with id-token: write and the required WIF fields.

Run this script to confirm the workflow is correctly configured:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate the mobile deploy workflow(s)
fd -t f -a --glob ".github/workflows/*mobile*deploy*.y?(a)ml"

# Show the auth step(s) and nearby config
rg -nC3 -e 'google-github-actions/auth@' -e 'id-token:\s*write' -e 'workload_identity_provider' -e 'service_account' .github/workflows | sed -n '1,200p'

Length of output: 291


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# List all workflow files under .github/workflows (including hidden)
echo "Workflow files found in .github/workflows:"
fd -H -t f --glob ".github/workflows/*.y?(a)ml"

# For each workflow file, display any GitHub OIDC auth configuration
for wf in $(fd -H -t f --glob ".github/workflows/*.y?(a)ml"); do
  echo
  echo "=== Inspecting $wf ==="
  rg -nC3 -e 'google-github-actions/auth@' \
           -e 'id-token:\s*write' \
           -e 'workload_identity_provider' \
           -e 'service_account' \
           "$wf" || echo "(no OIDC auth configuration found in $wf)"
done

Length of output: 215


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Top-level contents:"
ls -a .

echo
echo "Locating any .github directories:"
find . -type d -name ".github" -print || echo "(none found)"

echo
echo "Locating workflow files under .github/workflows:"
find . -type f \( -path "*/.github/workflows/*.yml" -o -path "*/.github/workflows/*.yaml" \) -print || echo "(none found)"

Length of output: 1500


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect OIDC auth configuration in mobile deploy workflows
for wf in .github/workflows/mobile-deploy.yml .github/workflows/mobile-deploy-auto.yml; do
  echo "=== Inspecting $wf ==="
  rg -nC3 -e 'google-github-actions/auth@' \
           -e 'id-token:\s*write' \
           -e 'workload_identity_provider' \
           -e 'service_account:' \
           "$wf" || echo "(no OIDC auth config found in $wf)"
done

Length of output: 1617


Ensure OIDC Authentication Is Configured in All Mobile Deploy Workflows

The primary deploy workflow (mobile-deploy.yml) correctly declares permissions.id-token: write and an auth@v2 step, but the automatic release workflow (mobile-deploy-auto.yml) has no OIDC wiring. Without it, CI will either silently fall back to long-lived JSON keys (which we’re avoiding) or fail outright, undermining our security posture and deployment reliability.

Please add the following to .github/workflows/mobile-deploy-auto.yml:

• Top-level permissions block (if not present)

permissions:
  contents: write
  pull-requests: write
  id-token: write

• An OIDC auth step before any gcloud/upload actions

- name: Authenticate to Google Cloud
  uses: google-github-actions/auth@v2
  with:
    workload_identity_provider: ${{ secrets.GCP_WIF_PROVIDER }}
    service_account:        ${{ secrets.GCP_SA_EMAIL }}

• Verify that the workload_identity_provider and service_account values match your GCP setup.

Once added, both manual and automatic deploy workflows will rely solely on Workload Identity Federation and no long-lived secrets.

🤖 Prompt for AI Agents
In app/fastlane/Fastfile around lines 359 to 361, the CI workflows lack OIDC
wiring for automatic releases; update .github/workflows/mobile-deploy-auto.yml
to add a top-level permissions block including id-token: write (and contents:
write, pull-requests: write if needed), and insert an OIDC auth step
(google-github-actions/auth@v2) before any gcloud/upload actions using
workload_identity_provider and service_account values from secrets
(GCP_WIF_PROVIDER and GCP_SA_EMAIL), and verify those secret names match your
GCP Workload Identity Federation configuration.

Comment on lines 387 to 395
gradle(
task: "clean bundleRelease --stacktrace --info",
project_dir: "android/",
properties: {
"android.injected.signing.store.file" => ENV["ANDROID_KEYSTORE_PATH"],
"android.injected.signing.store.password" => ENV["ANDROID_KEYSTORE_PASSWORD"],
"android.injected.signing.key.alias" => ENV["ANDROID_KEY_ALIAS"],
"android.injected.signing.key.password" => ENV["ANDROID_KEY_PASSWORD"],
"MYAPP_UPLOAD_STORE_FILE" => ENV["ANDROID_KEYSTORE_PATH"],
"MYAPP_UPLOAD_STORE_PASSWORD" => ENV["ANDROID_KEYSTORE_PASSWORD"],
"MYAPP_UPLOAD_KEY_ALIAS" => ENV["ANDROID_KEY_ALIAS"],
"MYAPP_UPLOAD_KEY_PASSWORD" => ENV["ANDROID_KEY_PASSWORD"] == "EMPTY" ? "" : ENV["ANDROID_KEY_PASSWORD"],
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Reduce risk of leaking signing secrets in CI logs; avoid --info for Gradle in CI

Because keystore credentials are passed via -P properties, running Gradle with --info can echo property usage and increase exposure risk in CI logs. Recommendation: only keep --info locally.

Apply this diff:

-      gradle(
-        task: "clean bundleRelease --stacktrace --info",
+      gradle(
+        task: "clean bundleRelease --stacktrace#{local_development ? ' --info' : ''}",
         project_dir: "android/",
         properties: {
           "MYAPP_UPLOAD_STORE_FILE" => ENV["ANDROID_KEYSTORE_PATH"],
           "MYAPP_UPLOAD_STORE_PASSWORD" => ENV["ANDROID_KEYSTORE_PASSWORD"],
           "MYAPP_UPLOAD_KEY_ALIAS" => ENV["ANDROID_KEY_ALIAS"],
           "MYAPP_UPLOAD_KEY_PASSWORD" => ENV["ANDROID_KEY_PASSWORD"] == "EMPTY" ? "" : ENV["ANDROID_KEY_PASSWORD"],
         },
       )

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools
🪛 RuboCop (1.76.1)

[convention] 395-395: Avoid comma after the last parameter of a method call.

(Style/TrailingCommaInArguments)

🤖 Prompt for AI Agents
In app/fastlane/Fastfile around lines 387 to 395, the gradle invocation includes
the --info flag which can cause keystore properties passed via -P to be echoed
in CI logs; remove the --info flag (or conditionally omit it when ENV["CI"] is
true) so secrets are not exposed, keeping only --stacktrace for CI runs and
preserve --info only for local/debug runs.

Comment on lines 406 to 419
upload_options = {
track: options[:track],
json_key: ENV["ANDROID_PLAY_STORE_JSON_KEY_PATH"],
package_name: ENV["ANDROID_PACKAGE_NAME"],
skip_upload_changelogs: true,
skip_upload_images: true,
skip_upload_screenshots: true,
track_promote_release_status: "completed",
aab: android_aab_path,
)
}
# In local development, use the JSON key file; in CI rely on ADC via google-github-actions/auth
upload_options[:json_key] = ENV["ANDROID_PLAY_STORE_JSON_KEY_PATH"] if local_development

upload_to_play_store(upload_options)
rescue => e
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Supply options and failure policy: confirm option name; fail CI on permission errors

  • Option name: track_promote_release_status is typically used when promoting tracks; for direct uploads the option is often release_status. Please verify against the current fastlane/supply docs to ensure the intended behavior.

  • Failure policy: Right now, 403/insufficientPermissions logs are emitted but the lane continues, which can hide failed releases. Suggest failing the lane in CI on permission errors while keeping local development forgiving.

Proposed change for the rescue block (outside this hunk for clarity):

rescue => e
  if e.message.include?("forbidden") || e.message.include?("403") || e.message.include?("insufficientPermissions")
    UI.error("❌ Play Store upload failed: Insufficient permissions")
    UI.error("Please fix permissions in Google Play Console")
    UI.important("Build saved at: #{android_aab_path}")
    raise e if Fastlane::Helpers.is_ci_environment?
  else
    raise e
  end
end

🌐 Web query:

Confirm which option should be used for setting release status when uploading with `upload_to_play_store` (fastlane supply): is it `release_status` or `track_promote_release_status` for direct AAB uploads?

💡 Result:

Use release_status for direct AAB (or APK) uploads. track_promote_release_status is only used when you are promoting a release from one track to another (i.e. when using track_promote_to). (docs.fastlane.tools, gem.sh)

Quick notes:

  • release_status controls the state for new uploads (valid values: completed, draft, halted, inProgress; default: completed). (docs.fastlane.tools)
  • track_promote_release_status controls the state of the release created by a track promotion (also accepts the same values; default: completed). (docs.fastlane.tools)

Example (Fastfile):
upload_to_play_store(
aab: "app.aab",
track: "internal",
release_status: "draft" # <--- use this for direct AAB uploads
)

If you share your current fastlane version or the command/lane you’re running, I can confirm exactly which option will apply in your setup.

Citations:


Use release_status for direct uploads & enforce CI failure on permission errors

Based on the official Fastlane documentation, for direct AAB (or APK) uploads you should use the release_status option instead of track_promote_release_status . Additionally, to avoid silent failures in CI when permissions are insufficient, the lane should explicitly fail on 403 errors while allowing local development to proceed.

• Replace track_promote_release_status with release_status in your upload_options hash.
• Extend the rescue block to detect permission errors and re-raise in CI, but swallow them locally.

Proposed updated snippet (within app/fastlane/Fastfile, around lines 406–419):

           upload_options = {
             track: options[:track],
             package_name: ENV["ANDROID_PACKAGE_NAME"],
             skip_upload_changelogs: true,
             skip_upload_images: true,
             skip_upload_screenshots: true,
-            track_promote_release_status: "completed",
+            release_status: "completed",
             aab: android_aab_path,
           }
           # In local development, use the JSON key file; in CI rely on ADC via google-github-actions/auth
           upload_options[:json_key] = ENV["ANDROID_PLAY_STORE_JSON_KEY_PATH"] if local_development

           upload_to_play_store(upload_options)
-        rescue => e
+        rescue => e
           if e.message.include?("forbidden") || e.message.include?("403") || e.message.include?("insufficientPermissions")
             UI.error("❌ Play Store upload failed: Insufficient permissions")
             UI.error("Please fix permissions in Google Play Console")
             UI.important("Build saved at: #{android_aab_path}")
-            # continue on local dev, fail on CI
-            raise e if Fastlane::Helpers.is_ci_environment?
+            raise e if Fastlane::Helpers.is_ci_environment?
           else
             raise e
           end
         end
🤖 Prompt for AI Agents
In app/fastlane/Fastfile around lines 406 to 419, the upload_options use the
deprecated track_promote_release_status and the rescue currently swallows
permission errors; replace track_promote_release_status with release_status in
upload_options, and update the rescue clause to detect Google Play permission
errors (HTTP 403 or an error message containing "403" / "permission" /
"insufficient") and re-raise the exception when not local_development (i.e. in
CI), while only swallowing/logging the error when local_development is true;
ensure you still set upload_options[:json_key] for local_development as before.

Copy link
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.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
app/fastlane/Fastfile (1)

351-361: Remove incorrect ANDROID_KEYSTORE env requirement; it’s unused and will cause false failures

Required vars include ANDROID_KEYSTORE, but the code uses ANDROID_KEYSTORE_PATH everywhere. This mismatch will make verify_env_vars fail in both CI and local runs. Keep the PATH variant only. Also, the gating of ANDROID_PLAY_STORE_JSON_KEY_PATH to local dev is good; keep that.

Apply this diff:

-    required_env_vars = [
-      "ANDROID_KEYSTORE",
-      "ANDROID_KEYSTORE_PASSWORD",
-      "ANDROID_KEYSTORE_PATH",
-      "ANDROID_KEY_ALIAS",
-      "ANDROID_KEY_PASSWORD",
-      "ANDROID_PACKAGE_NAME",
-    ]
+    required_env_vars = [
+      "ANDROID_KEYSTORE_PATH",
+      "ANDROID_KEYSTORE_PASSWORD",
+      "ANDROID_KEY_ALIAS",
+      "ANDROID_KEY_PASSWORD",
+      "ANDROID_PACKAGE_NAME",
+    ]
♻️ Duplicate comments (2)
app/fastlane/Fastfile (2)

387-389: Avoid --info in CI to reduce risk of leaking sensitive Gradle -P properties

Gradle --info can echo property usage; with keystore credentials passed via -P, keep --info for local only.

Apply this diff:

-      gradle(
-        task: "clean bundleRelease --stacktrace --info",
+      gradle(
+        task: "clean bundleRelease --stacktrace#{local_development ? ' --info' : ''}",

423-431: Fail CI on Google Play permission errors; only swallow locally

Current rescue logs 403/forbidden but continues even in CI, masking failed releases. Re-raise in CI; keep local forgiving to match team ergonomics.

Apply this diff:

         rescue => e
           if e.message.include?("forbidden") || e.message.include?("403") || e.message.include?("insufficientPermissions")
             UI.error("❌ Play Store upload failed: Insufficient permissions")
             UI.error("Please fix permissions in Google Play Console")
             UI.important("Build saved at: #{android_aab_path}")
+            raise e if Fastlane::Helpers.is_ci_environment?
           else
             # Re-raise if it's a different error
             raise e
           end
🧹 Nitpick comments (1)
app/fastlane/Fastfile (1)

379-384: Validate ADC credentials file in CI when present

If GOOGLE_APPLICATION_CREDENTIALS is set by the auth action, fail fast if the file is unreadable to avoid a late upload_to_play_store error.

Apply this diff:

-    # Validate JSON key only in local development; CI uses Workload Identity Federation (ADC)
-    if local_development
+    # Validate JSON key only in local development; in CI, ensure the ADC credentials file (if present) is readable
+    if local_development
       validate_play_store_json_key(
         json_key: ENV["ANDROID_PLAY_STORE_JSON_KEY_PATH"],
       )
-    end
+    elsif ENV["GOOGLE_APPLICATION_CREDENTIALS"]
+      cred_path = ENV["GOOGLE_APPLICATION_CREDENTIALS"]
+      UI.user_error!("GOOGLE_APPLICATION_CREDENTIALS not found at #{cred_path}") unless File.exist?(cred_path)
+    end
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between ffc482b and 7ffd52b.

📒 Files selected for processing (1)
  • app/fastlane/Fastfile (3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-29T01:08:28.530Z
Learnt from: transphorm
PR: selfxyz/self#795
File: app/android/app/build.gradle:157-158
Timestamp: 2025-07-29T01:08:28.530Z
Learning: For this React Native project, the team prefers build flexibility over fail-fast behavior for release builds in app/android/app/build.gradle. They intentionally allow fallback to debug signing for local development runs, relying on Google Play Console validation to catch any improperly signed releases during upload.

Applied to files:

  • app/fastlane/Fastfile
🪛 RuboCop (1.76.1)
app/fastlane/Fastfile

[convention] 382-382: Avoid comma after the last parameter of a method call.

(Style/TrailingCommaInArguments)

⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build-android
🔇 Additional comments (2)
app/fastlane/Fastfile (2)

359-361: Good: restrict Play Store JSON key requirement to local development

This aligns with OIDC/WIF in CI and reduces secret handling surface area.


391-395: ✅ Build configuration OK – Gradle consumes MYAPP_UPLOAD_ props*

I’ve confirmed that app/android/app/build.gradle lines 149–153 check for project.hasProperty('MYAPP_UPLOAD_STORE_FILE') and then correctly apply:

  • storeFile file(MYAPP_UPLOAD_STORE_FILE)
  • storePassword MYAPP_UPLOAD_STORE_PASSWORD
  • keyAlias MYAPP_UPLOAD_KEY_ALIAS
  • keyPassword MYAPP_UPLOAD_KEY_PASSWORD

The existing fallback to debug signing remains in the else block, so no changes are needed.

Comment on lines +406 to +414
upload_options = {
track: options[:track],
json_key: ENV["ANDROID_PLAY_STORE_JSON_KEY_PATH"],
package_name: ENV["ANDROID_PACKAGE_NAME"],
skip_upload_changelogs: true,
skip_upload_images: true,
skip_upload_screenshots: true,
track_promote_release_status: "completed",
aab: android_aab_path,
)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Use release_status (not track_promote_release_status) for direct uploads; add GHA creds fallback

For direct AAB uploads, supply expects release_status. Also accept GOOGLE_GHA_CREDS_PATH as a fallback when the auth action sets that instead of GOOGLE_APPLICATION_CREDENTIALS.

Apply this diff:

           upload_options = {
             track: options[:track],
             package_name: ENV["ANDROID_PACKAGE_NAME"],
             skip_upload_changelogs: true,
             skip_upload_images: true,
             skip_upload_screenshots: true,
-            track_promote_release_status: "completed",
+            release_status: "completed",
             aab: android_aab_path,
           }
           # In local development, use the JSON key file; in CI use ADC credentials
           if local_development
             upload_options[:json_key] = ENV["ANDROID_PLAY_STORE_JSON_KEY_PATH"]
           elsif ENV["GOOGLE_APPLICATION_CREDENTIALS"]
             upload_options[:json_key] = ENV["GOOGLE_APPLICATION_CREDENTIALS"]
+          elsif ENV["GOOGLE_GHA_CREDS_PATH"]
+            upload_options[:json_key] = ENV["GOOGLE_GHA_CREDS_PATH"]
           end

Also applies to: 415-421

🤖 Prompt for AI Agents
In app/fastlane/Fastfile around lines 406-414 (and also apply same change to
lines 415-421), the upload_options for direct AAB uploads incorrectly use
track_promote_release_status and do not accept the GHA creds fallback; replace
track_promote_release_status with release_status set to "completed", and ensure
the code accepts GOOGLE_GHA_CREDS_PATH as a fallback by setting
ENV["GOOGLE_APPLICATION_CREDENTIALS"] ||= ENV["GOOGLE_GHA_CREDS_PATH"] (or
otherwise reading GOOGLE_GHA_CREDS_PATH when GOOGLE_APPLICATION_CREDENTIALS is
unset) before performing the upload so the auth path works for both env variable
names.

@remicolin remicolin closed this Aug 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants