Skip to content

Commit 17b63dd

Browse files
committed
fix(wizard): tighten dlx allowlist + cover yarn dlx + dedupe e2e import
Three small follow-ups from the latest review pass: - isAllowedDlxCommand used \`rest.startsWith(t)\`, which would let \`bunx drizzle-kit-malicious\` slip through on \`drizzle-kit\`. Tighten to a token-boundary match (\`rest === t || rest.startsWith(\`\${t} \`)\`) so only the exact tool or the tool followed by a space matches. - errors-runner.test.ts covered npx/bunx/pnpm dlx but not yarn dlx — add the missing case to both \`classifyError\` and \`classifyHttpError\` suites for symmetry. - e2e/tests/package-managers.e2e.test.ts had two separate \`node:child_process\` imports (\`execFileSync\` and \`spawnSync\`). Merge into one. Out of scope from the same review pass: the pre-existing \`excludeOperatorFamily || true\` in supabase-migration.ts (introduced 2026-04-28, before this branch); the duplicate \`migrationHeader\` in the test file (already triaged in prior review); the runner.ts duplication between protect/drizzle (explicitly out-of-scope per the plan).
1 parent b9c2c30 commit 17b63dd

3 files changed

Lines changed: 17 additions & 3 deletions

File tree

e2e/tests/package-managers.e2e.test.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
1-
import { execFileSync } from 'node:child_process'
1+
import { execFileSync, spawnSync } from 'node:child_process'
22
import { existsSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'
33
import { tmpdir } from 'node:os'
44
import { dirname, join, resolve } from 'node:path'
55
import { fileURLToPath } from 'node:url'
6-
import { spawnSync } from 'node:child_process'
76
import { afterEach, beforeAll, beforeEach, describe, expect, it } from 'vitest'
87

98
import { createBaseProvider } from '../../packages/cli/src/commands/init/providers/base.js'

packages/wizard/src/__tests__/errors-runner.test.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,12 @@ describe('classifyError runner', () => {
1919
'Run: pnpm dlx stash auth login',
2020
)
2121
})
22+
23+
it('uses yarn dlx when runner=yarn dlx for auth failure', () => {
24+
expect(classifyError('authentication_failed', '', 'yarn dlx')).toContain(
25+
'Run: yarn dlx stash auth login',
26+
)
27+
})
2228
})
2329

2430
describe('classifyHttpError runner', () => {
@@ -39,4 +45,10 @@ describe('classifyHttpError runner', () => {
3945
'Run: pnpm dlx stash auth login',
4046
)
4147
})
48+
49+
it('uses yarn dlx when runner=yarn dlx for 401', () => {
50+
expect(classifyHttpError(401, '', 'yarn dlx')).toContain(
51+
'Run: yarn dlx stash auth login',
52+
)
53+
})
4254
})

packages/wizard/src/agent/interface.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,10 @@ function isAllowedDlxCommand(cmd: string): boolean {
8484
for (const prefix of RUNNER_PREFIXES) {
8585
if (cmd.startsWith(`${prefix} `)) {
8686
const rest = cmd.slice(prefix.length + 1)
87-
return ALLOWED_DLX_TOOLS.some((t) => rest.startsWith(t))
87+
// Token-boundary match: the tool name must be the entire remainder, or
88+
// the tool name followed by a space (then args). A bare `startsWith`
89+
// would let `drizzle-kit-malicious` slip through `drizzle-kit`.
90+
return ALLOWED_DLX_TOOLS.some((t) => rest === t || rest.startsWith(`${t} `))
8891
}
8992
}
9093
return false

0 commit comments

Comments
 (0)