fix(android): repair the Capacitor APK pipeline after the monorepo migration#238
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
Coverage Report for apps/web
|
…gration The Android release pipeline broke during the apps/web migration and was never revalidated (mobile was deferred to "Milestone M"). Fixes so a published release (or workflow_dispatch) produces a signed, installable APK again — suitable for GitHub-release distribution (e.g. Obtainium), independent of Play. - next.config.mjs: omit `rewrites`/`headers` under CAPACITOR_BUILD — both are unsupported by `output: export` and broke the static build. The hosted backend still serves them; the bundled shell calls it for /api/* so it needs neither. - help/[slug]: split into a server page exporting generateStaticParams (all 13 manual slugs from MANUALS) + a client child. A dynamic segment with no params can't be statically exported; also a free SSG win on the web build. - capacitor.config.ts: moved to apps/web with `android.path: ../../android`, so `cap sync` (run from apps/web) discovers the @capacitor/* plugins from apps/web/package.json and writes into the root android/ project. Plugin/CLI discovery was silently broken (root package.json has no Capacitor deps). - cap-build.js: read version from the root package.json (not apps/web's 0.0.0), write version.properties to the real android/app/, run `next build` in apps/web, and stash api/ + middleware OUTSIDE apps/web so Next's type-check (which covers all **/*.ts) doesn't choke on the moved routes' dangling imports. - android-release.yml: run the web/cap steps from apps/web; drop the redundant version.properties step (cap-build owns it); always upload the signed APK as a run artifact (installable build from any run); let workflow_dispatch build the current branch (blank tag) for pre-release validation. - docs + .gitignore updated. Validated locally: `cap-build.js` static export is green (29 pages, /help/[slug] SSG with all slugs), stash restores cleanly, web typecheck + lint pass. The Gradle/sign/APK half is validated via CI (all four ANDROID_KEYSTORE_* secrets are already set; no PLAY secret, so the Play-upload step auto-skips).
bb4dce7 to
f6a2adf
Compare
📝 WalkthroughWalkthroughThe PR migrates Capacitor configuration and build scripts into the ChangesCapacitor Monorepo Migration
Help Page Static Generation Refactor
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a 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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/android-release.yml:
- Around line 112-116: The actions/upload-artifact action is currently pinned to
a mutable version tag v4, which creates a supply chain security risk. Replace
the `@v4` tag reference with a specific full commit SHA (e.g.,
`@a8a3f3ad30e3422ade9a0ff0ce1887c5066eeccf`) to ensure that the exact version of
the action is used consistently and prevent unexpected changes from upstream
updates.
- Around line 27-29: The actions/checkout@v6 step in the android-release.yml
workflow is missing the persist-credentials configuration. Add
persist-credentials: false to the with section of the actions/checkout@v6 action
to prevent persisting git credentials, since this workflow only performs build
and artifact upload operations that do not require authenticated git operations.
This reduces token exposure risk by following the principle of least privilege.
In `@apps/web/scripts/cap-build.js`:
- Around line 12-14: The stash() function is currently called at line 45 before
the try/finally block begins at line 46, which means if leftover stash files
(API_STASH or MIDDLEWARE_STASH) exist from a prior interrupted build, the
renameSync call inside stash() will fail before entering the try block,
preventing restore() from ever executing in the finally block. Create a new
recoverStaleStash() function that checks for and removes any leftover API_STASH
and MIDDLEWARE_STASH files, then move the stash() call inside the try block and
call recoverStaleStash() immediately after the try statement begins to clean up
stale stashes before attempting to create new ones.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 94f57191-fc08-42b5-9d12-a52c255c7b10
📒 Files selected for processing (9)
.github/workflows/android-release.yml.gitignoreapps/web/capacitor.config.tsapps/web/next.config.mjsapps/web/scripts/cap-build.jsapps/web/src/app/help/[slug]/manual-page-client.tsxapps/web/src/app/help/[slug]/page.tsxcapacitor.config.tsdocs/android-release.md
💤 Files with no reviewable changes (1)
- capacitor.config.ts
| - uses: actions/checkout@v6 | ||
| with: | ||
| ref: ${{ github.event.inputs.tag || github.event.release.tag_name }} | ||
| ref: ${{ github.event.inputs.tag || github.event.release.tag_name || github.ref_name }} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
nl -ba .github/workflows/android-release.yml | sed -n '24,36p'Repository: RyRy79261/intake-tracker
Length of output: 111
🏁 Script executed:
cat -n .github/workflows/android-release.yml | head -40Repository: RyRy79261/intake-tracker
Length of output: 1593
🌐 Web query:
actions/checkout v6 persist-credentials security best practice
💡 Result:
For security best practices regarding actions/checkout@v6 and the persist-credentials input, you should default to setting persist-credentials: false unless your workflow explicitly requires authenticated Git operations (such as pushing code back to the repository) [1][2]. Security Context and Improvements in v6 In versions prior to v6, actions/checkout persisted the GITHUB_TOKEN directly into the.git/config file within the workspace [3][4][5]. This created a significant security risk where credentials could be inadvertently exposed if the workspace was archived and uploaded as an artifact (often referred to as an Artipacked vulnerability) [4]. In actions/checkout v6, the implementation was updated to address this [3][6]: - Credential storage was moved from.git/config to a protected location under $RUNNER_TEMP [3][4][5]. - This change mitigates the risk of credential leakage through standard artifact uploads because the credentials are no longer inside the workspace directory [4][7]. Best Practice Recommendations 1. Principle of Least Privilege: Always set persist-credentials: false by default [1]. If your workflow step does not need to perform authenticated git commands (e.g., git push), disabling this feature reduces the surface area for potential credential abuse [1][2]. 2. When to Enable: If you must perform authenticated git operations, set persist-credentials: true [3]. Ensure that the job permissions are restricted using the permissions key to the minimum necessary scopes (e.g., contents: write) [3][7][8]. 3. Awareness of Behavior Changes: Note that in v6, setting persist-credentials: false causes the credentials to be unavailable immediately after the checkout step [8]. If your workflow relies on git operations occurring later in the same job, you must either keep persistence enabled or manually configure authentication for those subsequent steps [8]. Using persist-credentials: false remains the recommended security hardening practice to ensure that no credentials persist in the runner environment longer than strictly necessary [1].
Citations:
- 1: https://cheatsheetseries.owasp.org/cheatsheets/GitHub_Actions_Security_Cheat_Sheet.html
- 2: Remove
persist-credentialsor change the default tofalseactions/checkout#485 - 3: https://github.com/actions/checkout/?tab=readme-ov-file
- 4: https://sisaku-security.github.io/lint/docs/rules/artipacked/
- 5: Persist creds to a separate file actions/checkout#2286
- 6: https://github.com/actions/checkout/tree/v6.0.2?tab=readme-ov-file
- 7: [security] Escalate concerning default
persist-credentials=trueplease actions/checkout#2312 - 8: astral-sh/ruff-action@4919ec5
🏁 Script executed:
cat -n .github/workflows/android-release.yml | tail -n +40Repository: RyRy79261/intake-tracker
Length of output: 6340
Add persist-credentials: false to reduce token exposure risk.
actions/checkout@v6 should disable credential persistence since this workflow performs no authenticated git operations (it only builds and uploads artifacts). This follows the principle of least privilege and reduces the attack surface.
Proposed patch
- uses: actions/checkout@v6
with:
ref: ${{ github.event.inputs.tag || github.event.release.tag_name || github.ref_name }}
+ persist-credentials: false📝 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.
| - uses: actions/checkout@v6 | |
| with: | |
| ref: ${{ github.event.inputs.tag || github.event.release.tag_name }} | |
| ref: ${{ github.event.inputs.tag || github.event.release.tag_name || github.ref_name }} | |
| - uses: actions/checkout@v6 | |
| with: | |
| ref: ${{ github.event.inputs.tag || github.event.release.tag_name || github.ref_name }} | |
| persist-credentials: false |
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 27-29: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 27-27: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/android-release.yml around lines 27 - 29, The
actions/checkout@v6 step in the android-release.yml workflow is missing the
persist-credentials configuration. Add persist-credentials: false to the with
section of the actions/checkout@v6 action to prevent persisting git credentials,
since this workflow only performs build and artifact upload operations that do
not require authenticated git operations. This reduces token exposure risk by
following the principle of least privilege.
Source: Linters/SAST tools
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: intake-tracker-apk-v${{ steps.version.outputs.version }} | ||
| path: ${{ steps.artifacts.outputs.apk }} | ||
| if-no-files-found: error |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/android-release.yml"
rg -n 'uses:\s*actions/upload-artifact@' "$FILE"
rg -nP 'uses:\s*actions/upload-artifact@[0-9a-f]{40}$' "$FILE" || trueRepository: RyRy79261/intake-tracker
Length of output: 113
🏁 Script executed:
# Check if there are multiple instances of upload-artifact in the file
rg 'actions/upload-artifact' .github/workflows/android-release.ymlRepository: RyRy79261/intake-tracker
Length of output: 109
Pin actions/upload-artifact@v4 to a commit SHA.
Tag-based pinning is mutable and poses a supply-chain risk. Replace @v4 with a full commit SHA for stronger reproducibility and security guarantees in CI/CD workflows.
🧰 Tools
🪛 zizmor (1.25.2)
[error] 112-112: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/android-release.yml around lines 112 - 116, The
actions/upload-artifact action is currently pinned to a mutable version tag v4,
which creates a supply chain security risk. Replace the `@v4` tag reference with a
specific full commit SHA (e.g., `@a8a3f3ad30e3422ade9a0ff0ce1887c5066eeccf`) to
ensure that the exact version of the action is used consistently and prevent
unexpected changes from upstream updates.
Source: Linters/SAST tools
| const API_STASH = path.join(__dirname, '..', '..', '..', '.cap-api-stash'); | ||
| const MIDDLEWARE = path.join(__dirname, '..', 'src', 'middleware.ts'); | ||
| const MIDDLEWARE_STASH = path.join(__dirname, '..', 'src', 'middleware.ts.bak'); | ||
| const MIDDLEWARE_STASH = path.join(__dirname, '..', '..', '..', '.cap-middleware-stash.ts'); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
nl -ba apps/web/scripts/cap-build.js | sed -n '1,90p' | rg -n "function stash|function restore|API_STASH|MIDDLEWARE_STASH|stash\\(\\);|try \\{|finally"Repository: RyRy79261/intake-tracker
Length of output: 109
🏁 Script executed:
cat apps/web/scripts/cap-build.jsRepository: RyRy79261/intake-tracker
Length of output: 2604
Move stash() inside try/finally and recover from stale stashes first.
stash() runs at line 45, before the try/finally block at lines 46–51. If a prior interrupted build leaves .cap-api-stash or .cap-middleware-stash.ts behind, the next run's renameSync will fail before reaching try, so finally never runs and restore() is skipped. This strands src/app/api and middleware.ts at the repo root, breaking subsequent builds.
Add a recoverStaleStash() function to clean up leftover stashes from prior interrupted runs, then call it first inside the try block before stash().
Also applies to: 16–25, 45–51
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/scripts/cap-build.js` around lines 12 - 14, The stash() function is
currently called at line 45 before the try/finally block begins at line 46,
which means if leftover stash files (API_STASH or MIDDLEWARE_STASH) exist from a
prior interrupted build, the renameSync call inside stash() will fail before
entering the try block, preventing restore() from ever executing in the finally
block. Create a new recoverStaleStash() function that checks for and removes any
leftover API_STASH and MIDDLEWARE_STASH files, then move the stash() call inside
the try block and call recoverStaleStash() immediately after the try statement
begins to clean up stale stashes before attempting to create new ones.
What & why
The
Android Releasepipeline broke during theapps/webmonorepo migration and was never revalidated (mobile was deferred to "Milestone M"). This repairs it end-to-end so a published release (orworkflow_dispatch) produces a signed, installable APK — ideal for GitHub-release distribution via Obtainium, independent of the Play Store.✅ Validated on CI: run 27675201909 (dispatched on this branch) built a signed
intake-tracker-apk-v1.32.1(4.5 MB). Play upload auto-skipped (noPLAY_SERVICE_ACCOUNT_JSON).Fixes
next.config.mjs— omitrewrites/headersunderCAPACITOR_BUILD(both unsupported byoutput: export; the hosted backend still serves them)./help/[slug]— split into a server page exportinggenerateStaticParams(all 13MANUALSslugs) + a client child; a dynamic segment with no params can't be statically exported (also a free SSG win on the web build).capacitor.config.ts— moved intoapps/webwithandroid.path: ../../androidsocap syncdiscovers@capacitor/*plugins fromapps/web/package.json(rootpackage.jsonhas none — discovery was silently broken).cap-build.js— version from the rootpackage.json(notapps/web's0.0.0),version.propertiesto the realandroid/app/,next buildinapps/web, and stashapi/+middleware outsideapps/webso the type-check (covers all**/*.ts) doesn't choke on the moved routes.android-release.yml— web/cap steps run fromapps/web; always upload the signed APK as a run artifact (installable build from any run);workflow_dispatchcan build the current branch (blank tag) for pre-release validation; dropped the redundantversion.propertiesstep..gitignoreupdated.Verification
cap-build.jsstatic export green (29 pages,/help/[slug]SSG with all slugs); stash restores cleanly;turbo typecheck+eslint srcclean.After merge — getting it onto the Releases page
This is committed as
fix(android):, so release-please will cut a release; merging that release PR publishes a release → this workflow attaches the APK + AAB to it (what Obtainium reads). Or cut one manually withgh release create.Summary by CodeRabbit
New Features
tagparameter; defaults to current branch when left blank.Improvements
Documentation