CC-64: Add First Playwright Tests#333
Merged
ianpaschal merged 16 commits intoMay 22, 2026
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 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: 8
🤖 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:
- Around line 1-12: Add an explicit top-level permissions block to the workflow
(e.g., set minimal scopes like contents: read) to avoid relying on repo
defaults, and update the checkout step in the test job (the actions/checkout@v4
step) to include persist-credentials: false so the GITHUB_TOKEN is not left in
the checked-out repository; modify the job under "name: CI" / "jobs: test" to
include these changes.
- Around line 12-14: Replace the floating action tags with immutable commit
SHAs: find each uses: entry (e.g., actions/checkout@v4, actions/setup-node@v4,
actions/upload-artifact@v4, and actions/github-script@v7) in the CI workflows
and replace the `@vN` tag with the corresponding full commit SHA for that action's
repo (owner/repo@<commit-sha>); update the three occurrences in
.github/workflows/ci.yml and the occurrences in .github/workflows/pr-opened.yml
and .github/workflows/pr-closed.yml so every actions/* reference uses a specific
commit SHA instead of the floating `@v`* tag.
In `@convex/_model/users/actions/internal/setUserDefaultAvatar.ts`:
- Line 26: The Dicebear avatar URL is built from raw user.username, which can
contain reserved URL characters; update the construction in setUserDefaultAvatar
(the avatarUrl variable) to URL-encode the seed by applying
encodeURIComponent(user.username) (or equivalent) before interpolating into the
query string so the generated URL is always valid.
In `@convex/_model/utils/createTestTournament.ts`:
- Line 42: The expression for maxCompetitors incorrectly uses operator
precedence so (competitorCount ?? useTeams) becomes the ternary condition;
update the definition of maxCompetitors to use competitorCount ?? (useTeams ? 12
: 24) so the nullish coalescing selects competitorCount when not null/undefined
and otherwise uses the ternary fallback; locate the symbol maxCompetitors in
createTestTournament (and the variables competitorCount and useTeams) and
parenthesize the ternary fallback accordingly.
In `@playwright.config.ts`:
- Around line 19-21: The Playwright webServer command in playwright.config.ts
currently hardcodes a user-specific CONVEX_DEPLOYMENT value
(CONVEX_DEPLOYMENT=local:local-ian_paschal-combat_command-1) which breaks other
developers; update the webServer command to run npx convex dev without the
personal slug (or read CONVEX_DEPLOYMENT from an environment variable like
process.env.CONVEX_DEPLOYMENT if callers need to override), and ensure the url
remains http://127.0.0.1:3210 so tests target the same local server.
In `@src/pages/TournamentPairingsPage/TournamentPairingsPage.tsx`:
- Around line 96-101: The current logic uses draftPairings.length === 0 to mean
“uninitialized”, which conflates an intentionally-empty draft with
never-initialized and breaks unsaved-change detection; change draftPairings to
an explicit optional/uninitialized state (e.g., initialize with null or a
separate boolean) and update the useEffect and any checks that now read
draftPairings.length to instead check the initializer (e.g., draftPairings ===
null) before copying existingPairings; update setDraftPairings usages and
downstream checks (including any included/excluded fallback logic) to handle the
new null/initialized empty-array distinction so clearing all slots no longer
gets treated as uninitialized.
In `@test/fixtures.ts`:
- Around line 33-38: Replace the current single-origin lookup with a scan over
all Playwright origins: iterate state.origins (each origin's localStorage array)
to locate an item whose name startsWith('__convexAuthJWT_'), call
client.setAuth(found.value) as soon as you find it, and if no such entry exists
after scanning all origins throw an Error immediately to fail fast; update
references around jwtEntry and client.setAuth to use this new logic so missing
JWTs do not silently proceed.
In `@test/tournament-pairings.spec.ts`:
- Around line 279-281: The equality check is unstable because savedPairings and
confirmedPairings are arrays of DraftTournamentPairing and are being sorted
without a comparator; add a deterministic comparator and sort both arrays with
it before comparing. Implement a comparator (used when calling
Array.prototype.sort) that compares stable pairing fields from
snapshotPairings/DraftTournamentPairing (e.g., player1Id, player2Id, tableNumber
or a joined key like `${player1Id}|${player2Id}|${table}`) and then replace
savedPairings.sort()/confirmedPairings.sort() with
savedPairings.sort(yourComparator) and confirmedPairings.sort(yourComparator)
prior to expect(...).toEqual(...).
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 190b066b-0fe6-4b0c-8d35-ffa9767273ca
⛔ Files ignored due to path filters (2)
convex/_generated/api.d.tsis excluded by!**/_generated/**package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (39)
.github/workflows/ci.yml.gitignoreconvex/_model/_test/_helpers/testUsers.tsconvex/_model/_test/actions/populateUsers.tsconvex/_model/_test/index.tsconvex/_model/_test/mutations/cleanUp.tsconvex/_model/common/errors.tsconvex/_model/tournamentPairings/_helpers/assignTables.test.tsconvex/_model/tournamentPairings/mutations/createTournamentPairings.tsconvex/_model/tournamentPairings/mutations/deleteTournamentPairings.tsconvex/_model/tournamentRegistrations/queries/getTournamentRegistrationByTournamentUser.tsconvex/_model/tournaments/queries/getTournamentOpenRound.tsconvex/_model/users/actions/internal/setUserDefaultAvatar.tsconvex/_model/utils/createTestTournament.tsconvex/_model/utils/createTestTournamentPairings.tsconvex/_model/utils/createTestUsers.tsconvex/_model/utils/index.tsconvex/test.tsconvex/utils.tspackage.jsonplaywright.config.tssrc/components/ContextMenu/ContextMenu.tsxsrc/components/TournamentPairingGenerationForm/TournamentPairingGenerationForm.schema.tssrc/components/TournamentPairingGenerationForm/components/TournamentCompetitorChecklist/TournamentCompetitorChecklist.tsxsrc/components/TournamentPairingRow/TournamentPairingRow.tsxsrc/components/TournamentProvider/TournamentContextMenu.tsxsrc/components/generic/Form/FormField.tsxsrc/hooks/useFormDialog.tsxsrc/pages/TournamentPairingsPage/TournamentPairingsPage.schema.tssrc/pages/TournamentPairingsPage/TournamentPairingsPage.tsxsrc/pages/TournamentPairingsPage/TournamentPairingsPage.utils.tsxsrc/pages/TournamentPairingsPage/components/Droppable/Droppable.tsxsrc/pages/TournamentPairingsPage/components/PairingsGrid/PairingsGrid.hooks.tssrc/pages/TournamentPairingsPage/components/PairingsGrid/PairingsGrid.tsxtest/README.mdtest/auth.setup.tstest/fixtures.tstest/global-setup.tstest/tournament-pairings.spec.ts
💤 Files with no reviewable changes (4)
- convex/_model/tournamentPairings/mutations/createTournamentPairings.ts
- src/components/generic/Form/FormField.tsx
- convex/_model/common/errors.ts
- convex/_model/tournamentPairings/mutations/deleteTournamentPairings.ts
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 `@convex/_model/users/actions/inviteUser.ts`:
- Around line 59-62: The username-generation loop in inviteUser.ts using
generateUsername() and ctx.runQuery(internal.users.getUserByUsername, { username
}) is unbounded; add a max-attempt guard (e.g., const MAX_USERNAME_ATTEMPTS = 10
or 100) and an attempts counter around that while loop, retrying until attempts
>= MAX_USERNAME_ATTEMPTS, and on exceeding the cap throw a clear, descriptive
error (e.g., "Failed to generate unique username after X attempts") so
inviteUser fails fast instead of hanging.
- Around line 59-62: inviteUser's current generateUsername + loop using
internal.users.getUserByUsername is race-prone because the users table only has
a non-unique index by_username and other code paths (createAccount /
createOrUpdateUser) also use query-then-insert; replace this with an atomic
reservation approach: derive a deterministic reservation key from the candidate
username, attempt a single write (e.g., insert a "username_reservations" doc or
a reservation field) that will conflict on duplicate keys, and on conflict
generate a new username and retry; update inviteUser to attempt reservation via
that atomic insert and only insert the actual user after successful reservation,
and update other creators (createAccount / createOrUpdateUser) to use the same
reservation write path so uniqueness is enforced without relying on
getUserByUsername or index('by_username').
In `@test/globalSetup.ts`:
- Around line 18-43: The cleanup invocation is vulnerable to shell injection and
may target the wrong deployment because it interpolates CONVEX_PREVIEW_NAME into
a shell string; update the globalSetup to (1) require an explicit preview target
before running destructive cleanup (throw or skip if
process.env.CONVEX_PREVIEW_NAME is missing or doesn't match the convexUrl host),
(2) stop using execSync with a single interpolated command and use a
spawn/execFile variant that accepts an argv array (e.g.,
child_process.execFileSync or spawnSync) to call "npx" with arguments
["convex","run","--preview-name", previewName, "test:cleanUp",
JSON.stringify(cleanUpArgs")] so arguments are not shell-interpolated, and (3)
pass the JSON payload as a single safe argument (the existing cleanUpArgs
variable) and similarly invoke "test:populateUsers" with argv to avoid shells;
reference ConvexHttpClient, convexUrl, process.env.CONVEX_PREVIEW_NAME,
cleanUpArgs, execSync, and the "test:cleanUp"/"test:populateUsers" commands when
making these changes.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 2dea19b7-9482-4954-a32e-d3faf1163443
📒 Files selected for processing (17)
.github/workflows/ci.yml.github/workflows/pr-closed.yml.github/workflows/pr-opened.ymlconvex/_model/_test/index.tsconvex/_model/_test/mutations/cleanUp.tsconvex/_model/users/actions/internal/setUserDefaultAvatar.tsconvex/_model/users/actions/inviteUser.tsconvex/_model/users/index.tsconvex/_model/users/queries/internal/getUserByUsername.tsconvex/_model/users/table.tsconvex/_model/utils/createTestTournament.tsconvex/test.tsconvex/users.tsplaywright.config.tstest/fixtures.tstest/globalSetup.tstest/tournament-pairings.spec.ts
Comment on lines
+18
to
+43
| const preview = process.env.CONVEX_PREVIEW_NAME ? `--preview-name ${process.env.CONVEX_PREVIEW_NAME}` : ''; | ||
|
|
||
| const client = new ConvexHttpClient(convexUrl!); | ||
| const { page: users } = await client.query(api.users.getUsers, { | ||
| paginationOpts: { numItems: 1, cursor: null }, | ||
| }); | ||
| const usersExist = users.length > 0; | ||
|
|
||
| const cleanUpArgs = { | ||
| preservedTables: usersExist ? [ | ||
| 'authAccounts', | ||
| 'authSessions', | ||
| 'authRefreshTokens', | ||
| 'authVerificationCodes', | ||
| 'authVerifiers', | ||
| 'authRateLimits', | ||
| 'users', | ||
| 'userPreferences', | ||
| ] : [], | ||
| }; | ||
|
|
||
| execSync( `npx convex run ${preview} test:cleanUp '${JSON.stringify(cleanUpArgs)}'`, { stdio: 'inherit' }); | ||
|
|
||
| if (!usersExist) { | ||
| execSync(`npx convex run ${preview} test:populateUsers`, { stdio: 'inherit' }); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and print the relevant portion of the file
ls -la test || true
sed -n '1,120p' test/globalSetup.ts | nl -ba
# Also grep for cleanup/populate invocations to see if gating exists elsewhere
rg -n "test:cleanUp|test:populateUsers|CONVEX_PREVIEW_NAME|execSync|execFileSync" -S test/globalSetup.ts test -g'*.ts' || true
# Show where convexUrl is defined (to determine whether it could be non-local)
rg -n "convexUrl" -S test/globalSetup.ts test -g'*.ts' || trueRepository: ianpaschal/combat-command
Length of output: 580
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Print first ~220 lines with line numbers using awk
sed -n '1,220p' test/globalSetup.ts | awk '{printf "%5d %s\n", NR, $0}'
# Search for execSync usage and any gating related to preview/CI elsewhere
rg -n "execSync\\(|execFileSync\\(|CONVEX_PREVIEW_NAME|test:cleanUp|test:populateUsers|globalSetup" -S . -g'*.ts'
# Show where convexUrl is sourced (in case gating depends on it)
rg -n "convexUrl" -S . -g'*.ts'Repository: ianpaschal/combat-command
Length of output: 2809
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find where the CLI actions are defined
rg -n "test:cleanUp|test:populateUsers|action.*cleanUp|action.*populateUsers" -S . -g'*.ts' -g'*.js' -g'*.tsx'
# Inspect the convex actions implementation(s), if present
fd -t f "test.*(cleanUp|populateUsers)" . || true
fd -t f "cleanUp" . || true
fd -t f "populateUsers" . || trueRepository: ianpaschal/combat-command
Length of output: 726
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show cleanUp implementation
if [ -f convex/_model/_test/mutations/cleanUp.ts ]; then
echo "---- convex/_model/_test/mutations/cleanUp.ts ----"
sed -n '1,220p' convex/_model/_test/mutations/cleanUp.ts | awk '{printf "%4d %s\n", NR, $0}'
else
echo "Missing: convex/_model/_test/mutations/cleanUp.ts"
fi
# Show populateUsers implementation
if [ -f convex/_model/_test/actions/populateUsers.ts ]; then
echo "---- convex/_model/_test/actions/populateUsers.ts ----"
sed -n '1,260p' convex/_model/_test/actions/populateUsers.ts | awk '{printf "%4d %s\n", NR, $0}'
else
echo "Missing: convex/_model/_test/actions/populateUsers.ts"
fi
# Check test scaffolding / fixtures that might show intended deployment targeting
echo "---- test/fixtures.ts (relevant bits) ----"
sed -n '1,120p' test/fixtures.ts | awk '{printf "%4d %s\n", NR, $0}'
echo "---- playwright.config.ts (relevant bits) ----"
sed -n '1,120p' playwright.config.ts | awk '{printf "%4d %s\n", NR, $0}'Repository: ianpaschal/combat-command
Length of output: 6646
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect package.json for scripts / convex config usage
ls -la
cat package.json 2>/dev/null || true
# Search for convex config files and deployment env vars used in repo
fd -t f "convex.*" . || true
fd -t f ".env*" . || true
rg -n "CONVEX_DEPLOYMENT|CONVEX_DEPLOY_KEY|CONVEX_PREVIEW_NAME|--preview-name|convex run|convex dev" -S . -g'*.ts' -g'*.js' -g'*.json' -g'*.md' -g'*.yml' -g'*.yaml' || true
# Show any convex.json / convex.config.* if present
fd -t f "convex.json" . || true
fd -t f "convex.config.*" . || trueRepository: ianpaschal/combat-command
Length of output: 28806
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show convex config
if [ -f convex/convex.config.ts ]; then
echo "---- convex/convex.config.ts ----"
sed -n '1,220p' convex/convex.config.ts | awk '{printf "%4d %s\n", NR, $0}'
fi
# Show getEnvironment helper
if [ -f convex/_model/common/_helpers/getEnvironment.ts ]; then
echo "---- convex/_model/common/_helpers/getEnvironment.ts ----"
sed -n '1,220p' convex/_model/common/_helpers/getEnvironment.ts | awk '{printf "%4d %s\n", NR, $0}'
fi
# Show test README that may document env vars
if [ -f test/README.md ]; then
echo "---- test/README.md ----"
sed -n '1,200p' test/README.md | awk '{printf "%4d %s\n", NR, $0}'
fi
# Search for how CONVEX_DEPLOYMENT is used in scripts/workflows
rg -n "CONVEX_DEPLOYMENT" -S . -g'*.ts' -g'*.js' -g'*.md' -g'*.yml' -g'*.yaml' || true
# Search for CONVEX_PREVIEW_NAME usage
rg -n "CONVEX_PREVIEW_NAME" -S . -g'*.ts' -g'*.js' -g'*.md' -g'*.yml' -g'*.yaml' || trueRepository: ianpaschal/combat-command
Length of output: 2256
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "getEnvironment\\(" -S convex test -g'*.ts'
rg -n "CONVEX_DEPLOY_KEY" -S convex test -g'*.ts'Repository: ianpaschal/combat-command
Length of output: 348
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect convex/auth.ts around the getEnvironment check
if [ -f convex/auth.ts ]; then
sed -n '80,170p' convex/auth.ts | awk '{printf "%4d %s\n", NR+79, $0}'
else
echo "Missing convex/auth.ts"
fi
# Inspect CI workflows for env vars related to e2e/playwright/convex
fd -t f ".github/workflows" . || true
fd -t f ".github/workflows/*" . || true
fd -t f ".github/workflows" . -e "*.yml" -e "*.yaml" || true
# Grep for playwright/e2e invocation and convex-related env vars
rg -n "playwright test|test:e2e|test:e2e:slow|CONVEX_PREVIEW_NAME|CONVEX_DEPLOY_KEY|CONVEX_DEPLOYMENT|JWT_PRIVATE_KEY|VITE_CONVEX_URL" .github/workflows -S || true
# Also search for any mention in other CI config files
rg -n "playwright test|test:e2e|CONVEX_PREVIEW_NAME|CONVEX_DEPLOY_KEY|CONVEX_DEPLOYMENT" .github -S || trueRepository: ianpaschal/combat-command
Length of output: 3927
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,120p' .github/workflows/ci.yml | awk '{printf "%4d %s\n", NR, $0}'
sed -n '120,260p' .github/workflows/ci.yml | awk '{printf "%4d %s\n", NR, $0}'Repository: ianpaschal/combat-command
Length of output: 2721
Harden cleanup execution: avoid shell interpolation and block destructive cleanup without an explicit preview target.
test/globalSetup.tsexecutes destructive cleanup vianpx convex run ... test:cleanUp, and thecleanUpmutation deletes all non-preserved tables.- The current
execSyncbuilds a shell command by interpolatingCONVEX_PREVIEW_NAME, which risks command injection and can target a different deployment than the one used byVITE_CONVEX_URL(sincenpx convex runrelies on its default target when--preview-nameis omitted).
Suggested patch
-import { execSync } from 'child_process';
+import { execFileSync } from 'child_process';
@@
export default async function globalSetup(): Promise<void> {
- const preview = process.env.CONVEX_PREVIEW_NAME ? `--preview-name ${process.env.CONVEX_PREVIEW_NAME}` : '';
+ const previewName = process.env.CONVEX_PREVIEW_NAME;
+ const previewArgs = previewName ? ['--preview-name', previewName] : [];
+ if (process.env.CI && !previewName) {
+ throw new Error('CONVEX_PREVIEW_NAME must be set in CI before running test cleanup');
+ }
+ if (!previewName && !/^https?:\/\/(localhost|127\.0\.0\.1)(:\d+)?/.test(convexUrl)) {
+ throw new Error('Refusing to run destructive cleanup without preview name on a non-local deployment');
+ }
@@
- execSync( `npx convex run ${preview} test:cleanUp '${JSON.stringify(cleanUpArgs)}'`, { stdio: 'inherit' });
+ execFileSync('npx', ['convex', 'run', ...previewArgs, 'test:cleanUp', JSON.stringify(cleanUpArgs)], {
+ stdio: 'inherit',
+ });
@@
if (!usersExist) {
- execSync(`npx convex run ${preview} test:populateUsers`, { stdio: 'inherit' });
+ execFileSync('npx', ['convex', 'run', ...previewArgs, 'test:populateUsers'], { stdio: 'inherit' });
}
}📝 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
| const preview = process.env.CONVEX_PREVIEW_NAME ? `--preview-name ${process.env.CONVEX_PREVIEW_NAME}` : ''; | |
| const client = new ConvexHttpClient(convexUrl!); | |
| const { page: users } = await client.query(api.users.getUsers, { | |
| paginationOpts: { numItems: 1, cursor: null }, | |
| }); | |
| const usersExist = users.length > 0; | |
| const cleanUpArgs = { | |
| preservedTables: usersExist ? [ | |
| 'authAccounts', | |
| 'authSessions', | |
| 'authRefreshTokens', | |
| 'authVerificationCodes', | |
| 'authVerifiers', | |
| 'authRateLimits', | |
| 'users', | |
| 'userPreferences', | |
| ] : [], | |
| }; | |
| execSync( `npx convex run ${preview} test:cleanUp '${JSON.stringify(cleanUpArgs)}'`, { stdio: 'inherit' }); | |
| if (!usersExist) { | |
| execSync(`npx convex run ${preview} test:populateUsers`, { stdio: 'inherit' }); | |
| } | |
| const previewName = process.env.CONVEX_PREVIEW_NAME; | |
| const previewArgs = previewName ? ['--preview-name', previewName] : []; | |
| if (process.env.CI && !previewName) { | |
| throw new Error('CONVEX_PREVIEW_NAME must be set in CI before running test cleanup'); | |
| } | |
| if (!previewName && !/^https?:\/\/(localhost|127\.0\.0\.1)(:\d+)?/.test(convexUrl)) { | |
| throw new Error('Refusing to run destructive cleanup without preview name on a non-local deployment'); | |
| } | |
| const client = new ConvexHttpClient(convexUrl!); | |
| const { page: users } = await client.query(api.users.getUsers, { | |
| paginationOpts: { numItems: 1, cursor: null }, | |
| }); | |
| const usersExist = users.length > 0; | |
| const cleanUpArgs = { | |
| preservedTables: usersExist ? [ | |
| 'authAccounts', | |
| 'authSessions', | |
| 'authRefreshTokens', | |
| 'authVerificationCodes', | |
| 'authVerifiers', | |
| 'authRateLimits', | |
| 'users', | |
| 'userPreferences', | |
| ] : [], | |
| }; | |
| execFileSync('npx', ['convex', 'run', ...previewArgs, 'test:cleanUp', JSON.stringify(cleanUpArgs)], { | |
| stdio: 'inherit', | |
| }); | |
| if (!usersExist) { | |
| execFileSync('npx', ['convex', 'run', ...previewArgs, 'test:populateUsers'], { stdio: 'inherit' }); | |
| } |
🧰 Tools
🪛 OpenGrep (1.21.0)
[ERROR] 39-39: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
[ERROR] 42-42: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🤖 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 `@test/globalSetup.ts` around lines 18 - 43, The cleanup invocation is
vulnerable to shell injection and may target the wrong deployment because it
interpolates CONVEX_PREVIEW_NAME into a shell string; update the globalSetup to
(1) require an explicit preview target before running destructive cleanup (throw
or skip if process.env.CONVEX_PREVIEW_NAME is missing or doesn't match the
convexUrl host), (2) stop using execSync with a single interpolated command and
use a spawn/execFile variant that accepts an argv array (e.g.,
child_process.execFileSync or spawnSync) to call "npx" with arguments
["convex","run","--preview-name", previewName, "test:cleanUp",
JSON.stringify(cleanUpArgs")] so arguments are not shell-interpolated, and (3)
pass the JSON payload as a single safe argument (the existing cleanUpArgs
variable) and similarly invoke "test:populateUsers" with argv to avoid shells;
reference ConvexHttpClient, convexUrl, process.env.CONVEX_PREVIEW_NAME,
cleanUpArgs, execSync, and the "test:cleanUp"/"test:populateUsers" commands when
making these changes.
ianpaschal
merged commit ab7b482
into
ian/cc-2-improve-pairing-workflow
4 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves CC-64
Summary by CodeRabbit
New Features
Bug Fixes
Accessibility
Documentation