Skip to content

fix: multiple Windows shell-compat bugs in dev-skills CLI#8

Open
Zhiying-Li-dot wants to merge 4 commits into
mainfrom
fix/show-env-windows-shell-quoting
Open

fix: multiple Windows shell-compat bugs in dev-skills CLI#8
Zhiying-Li-dot wants to merge 4 commits into
mainfrom
fix/show-env-windows-shell-quoting

Conversation

@Zhiying-Li-dot
Copy link
Copy Markdown

Bug

`optima-show-env` 在 Windows 上完全不可用,报:

```
❌ Error: Command failed: curl -s -X POST "https://secrets.optima.onl/api/v1/auth/universal-auth/login\" ...
```

直接 copy-paste 同样的 curl 命令到 Git Bash 跑是 200。

根因

`bin/helpers/show-env.ts` 用 `execSync` 拼一个含 `-d '{...}'` 的命令字符串:

```ts
execSync(`curl ... -d '{"clientId": "${id}"}'`)
```

Node 的 `execSync` 在 Windows 走 `cmd.exe`,cmd 里单引号不是字符串定界符,是字面字符。所以 curl 实际收到 `-d` 的值是字面 `'{"clientId":"..."}'`(带单引号),Infisical 解析失败 → curl 非零 → execSync 抛错。

Linux/macOS 上 execSync 走 `/bin/sh`,单引号正常 → 一切正常。这就是为什么 main 分支上 macOS 没人发现。

修法

把两处 `execSync` 改成 `execFileSync`,参数作为 argv 数组传给 curl —— 完全绕开 shell 解析,每个参数原样传递:

```ts
execFileSync('curl', ['-s', '-X', 'POST', url, '-H', 'Content-Type: application/json', '-d', body])
```

跨平台一律安全:Node 调用 CreateProcess (Windows) / execvp (Unix),每个 argv 元素逐字传递,shell quoting 完全无关。

Verified

Windows 11 + Node 22:

```
$ optima-show-env optima-generation stage --filter KIE
✓ Obtained Infisical access token
✓ Retrieved secrets from Infisical (path: /services/optima-generation)
KIE_API_KEY=...
```

之前跑同样命令是直接报 `Command failed: curl`。

Files

  • `bin/helpers/show-env.ts`: 两处 execSync → execFileSync (改了 token 接口和 secrets 接口)
  • `package.json`: 0.7.32 → 0.7.33

Test plan

  • Windows 11 + Node 22 本地验证通过(取到真实 KIE_API_KEY)
  • macOS / Linux 不变行为(execFileSync 在 POSIX 上等价于直接 `execvp`,不走 shell,本来 single-quote 也用不到)

🤖 Generated with Claude Code

… bug

show-env was failing on Windows with `Command failed: curl -s -X POST ...`
because execSync goes through cmd.exe, where single quotes are literal
characters (not string delimiters as in /bin/sh). The Infisical auth body
`-d '{"clientId":"..."}'` was reaching curl as the literal string
`'{"clientId":"..."}'` (with single quotes baked in), which Infisical
rejected.

Switched the two curl invocations from execSync (shell-interpreted command
string) to execFileSync (argv array, no shell). This works on every
platform regardless of the shell because Node calls CreateProcess /
execvp directly with each argv element preserved.

Verified on Windows 11 + Node 22:
  $ optima-show-env optima-generation stage --filter KIE
  ✓ Obtained Infisical access token
  ✓ Retrieved secrets from Infisical (path: /services/optima-generation)
  KIE_API_KEY=...

Bump 0.7.32 → 0.7.33.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@Zhiying-Li-dot
Copy link
Copy Markdown
Author

Review 结论:可以合并 ✅

修复正确性

根因分析准确,修复方向是对的:

  • Node execSync 在 Windows 走 cmd.exe,cmd 不把单引号当字符串定界符,所以 -d '{...}' 的单引号会被当字面字符传给 curl,Infisical 解析失败。
  • 改用 execFileSync('curl', [...args]) 完全绕开 shell,argv 逐个原样传给子进程,跨平台一律安全。这是正确且最小化的修复。
  • 顺手把手工字符串拼 JSON 改成 JSON.stringify(...),边角上还修了一个潜在问题:之前如果 clientSecret 含双引号会破坏 JSON。算是免费的 bonus。

小问题(非阻塞)

  1. 注释里有占位符没填 (bin/helpers/show-env.ts:60-61):

    // see https://github.com/Optima-Chat/optima-dev-skills/issues/<n>.
    

    <n> 没替换。建议改成 "see PR fix: multiple Windows shell-compat bugs in dev-skills CLI #8" 或直接删掉这行 URL。

  2. 依赖系统 curl: Windows 10 1803+ 自带 C:\Windows\System32\curl.exe,新版本基本都有;但既然在做跨平台清理,后续可以考虑用 Node 18+ 内置 fetch 彻底去掉 curl 子进程依赖。本 PR 范围外,不必现在改。

  3. execSync 仍保留(line 48 的 gh variable get)是合理的——那条命令没有 shell quoting 问题,而且依赖 PATH 里的 gh,无需改动。import 里两个都留是对的,不是死代码。

  4. Test plan 第二项未勾: macOS/Linux 回归未实测。理论上 execFileSync 在 POSIX 上行为等价(都不走 shell),但 merge 前最好在一台 Linux/Mac 上跑一遍 optima-show-env 确认无回归——毕竟这是团队主力平台。

安全/性能

  • 无安全风险,反而修了一个 minor JSON injection 边角(见上)。
  • 性能无影响,execFileSyncexecSync 略快(少一次 shell fork)。

建议

把注释里的 <n> 填上或删掉,然后在 macOS 上手跑一次 sanity check 就可以 merge。版本号 patch bump 也合理。

@Zhiying-Li-dot
Copy link
Copy Markdown
Author

跟进:

  • <n> 占位符已在 commit d2e0b04 修掉
  • ⏳ Test plan 第二项(macOS/Linux 回归)还没勾 — 建议在 macOS 上跑一次 optima-show-env <service> stage --filter X sanity check 后再 merge,避免 POSIX 路径意外回归

合并请用 safe-merge-pr 8 --merge --repo Optima-Chat/optima-dev-skills

The same execSync+single-quoted-JSON bug exists in two other helper files,
not just show-env.ts. Discovered when trying optima-grant-balance on Windows
hit identical Infisical auth failure path.

Applies the exact same execFileSync(argv) fix:
  - bin/helpers/db-utils.ts (shared by grant-balance / grant-subscription)
  - bin/helpers/query-db.ts (used by query-db tool)

After this commit, optima-grant-balance / optima-grant-subscription /
optima-query-db all work on Windows alongside the already-fixed
optima-show-env.

Note: optima-grant-balance has a separate Windows issue (SSH key parsing
in libcrypto) — that's downstream of this fix and tracked separately.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@Zhiying-Li-dot
Copy link
Copy Markdown
Author

扩大修复范围(commit b3c2b19):

optima-grant-balance 在 Windows 跑时撞上完全相同的 bug——`db-utils.ts` 和 `query-db.ts` 里都有同款 `execSync` + 单引号 JSON 拼字符串。所以加进本 PR:

  • ✅ `bin/helpers/db-utils.ts`(被 grant-balance / grant-subscription 共用)
  • ✅ `bin/helpers/query-db.ts`(被 query-db CLI 用)

同款 `execFileSync(argv)` pattern,reviewer 已 approve 过这思路,所以没另开 PR——加进来比单独 PR 维护成本更低。

合后 4 个工具(show-env / grant-balance / grant-subscription / query-db)全部在 Windows 可用。

题外话: `optima-grant-balance` 还有个独立的 Windows 问题——SSH key 在 libcrypto 解不出,"Permission denied"。需要单独跟进,不在本 PR 范围。

Two Windows-only bugs in bin/helpers/db-utils.ts that broke
optima-grant-balance / optima-grant-subscription end-to-end. Surfaced
when topping up a prod user wallet: CLI reported success but DB never
changed (lost a real $20 grant on first attempt).

1. findPsqlPath(): on Windows, `which psql` under Git Bash returns an
   MSYS path like `/c/Program Files/PostgreSQL/18/bin/psql`. Node's
   execSync runs that through cmd.exe, which cannot resolve MSYS paths
   and throws "系统找不到指定的路径" (system cannot find the specified
   path). Fix: probe known Windows install paths first, then fall back
   to `where psql` which returns native Windows paths. Pattern matches
   the already-correct findPsqlPath in query-db.ts.

2. queryDB(): used `psql -c "<multi-line SQL>"`. cmd.exe truncates
   embedded newlines inside a quoted -c argument, so multi-statement
   transactions like grant-balance's BEGIN; UPDATE; INSERT; COMMIT only
   execute their first line (BEGIN), and psql still exits 0 — the CLI
   prints "✓ Granted" while granted_balance_micros is unchanged. This
   is the dangerous one: silent partial success on prod money paths.
   Fix: write SQL to a temp file and use `psql -f`, bypassing all shell
   quoting; add `-v ON_ERROR_STOP=1` so any failing statement aborts
   with non-zero exit instead of psql swallowing it.

query-db.ts has the same -c pattern (line 230) but only takes
single-statement SQL from CLI argv, so it doesn't hit the newline
truncation; left for a follow-up.

Verified on Windows: optima-grant-balance --amount 20 --env prod now
actually inserts into usd_wallet_topups and updates granted_balance_micros.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@Zhiying-Li-dot Zhiying-Li-dot changed the title fix(show-env): Windows cmd.exe single-quote bug breaks Infisical auth fix: multiple Windows shell-compat bugs in dev-skills CLI May 21, 2026
Copy link
Copy Markdown
Author

@Zhiying-Li-dot Zhiying-Li-dot left a comment

Choose a reason for hiding this comment

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

@
Self-review notes — 三个 bug 都修到根因 (cmd.exe 单引号字面化 / MSYS 路径 / -c 多行 SQL 截断),execFileSync 绕过 shell + temp file -f 是治本不是 quoting 补丁。第三个 silent partial success 在 prod money path 上,加 ON_ERROR_STOP=1 修得到位。

Merge 前留几条非阻塞的 nit:

1. encodeURIComponent(secretPath) 三份 helper 行为不一致

  • bin/helpers/db-utils.ts (本 PR) — 新加了 encode
  • bin/helpers/query-db.ts (本 PR 改后) — 仍是裸 ${secretPath}
  • bin/helpers/show-env.ts — 老代码就一直有 encode

不会真挂 (/services/... 两种 Infisical 都吃),但同一段逻辑三处行为不同,后面有人复制粘贴会困惑。建议 query-db.ts 那处也补上 encode 对齐。

2. Test plan 里 macOS/Linux 验证还没勾
execFileSync 在 POSIX 走 execvp 不过 shell,理论上必然 OK;但 db-utils.ts 的 temp-file -f 写法在 macOS 上没真跑过。merge 前在 mac 上跑一次 optima-query-dboptima-grant-balance --dry-run 成本很低,顺手验一下。

3. findPsqlPath 硬编码 PG 14–18
没覆盖 C:\Program Files (x86)\PostgreSQL\ (32-bit) 和未来的 PG 19+。where psql fallback 兜底所以不会挂,不用改,只是 mark 一下。

4. query-db.ts:230 仍有 -c 模式
commit message 里已经说明单语句 argv 不踩 newline 截断、留 follow-up — scope 收紧合理。建议起个 issue 别让它飘了。
@

@Zhiying-Li-dot
Copy link
Copy Markdown
Author

跟进 self-review 4 条 nit:

#1 encodeURIComponent 三处对齐 — ✅ 实际已经一致,self-review 信息过时:

bin/helpers/db-utils.ts:63   secretPath=${encodeURIComponent(secretPath)}
bin/helpers/query-db.ts:145  secretPath=${encodeURIComponent(secretPath)}
bin/helpers/show-env.ts:76   secretPath=${encodeURIComponent(secretPath)}

无需改动。

#2 macOS/Linux 回归 — ⏳ 我在 Windows 环境,无法本地跑 macOS。execFileSync(argv) 在 POSIX 走 execvp 不过 shell,理论必然 OK;temp-file -f 模式也是 psql 标准用法。Merge 前请在 mac/linux 上跑一次 optima-show-env <service> stage --filter Xoptima-query-db user-auth "SELECT 1" stage 兜底。

#3 findPsqlPath PG 版本硬编码 — 📝 Noted,非阻塞。当前覆盖 14-18,遇到 PG 19+ 或 32-bit 路径会落到 where psql fallback,不会挂。先不动。

#4 query-db.ts:230 残留 -c — ✅ 已开 follow-up issue #10。Scope 收紧合理(单语句 argv 不踩 newline 截断),issue 跟进。

Self-approve 这 4 条都不阻塞 merge。等 macOS sanity check 完就可以走 safe-merge-pr 8 --merge --repo Optima-Chat/optima-dev-skills

@Zhiying-Li-dot
Copy link
Copy Markdown
Author

@xbfool 这个 PR 改了 db-utils.tsquery-db.ts,本人在 Windows 上验证过;麻烦你在 macOS 上跑一次以下任一命令兜底,确认 POSIX 路径无回归:

optima-show-env user-auth stage --filter DATABASE_URL
# or
optima-query-db user-auth "SELECT 1" stage
# or
optima-grant-balance <test-email> --amount 0.01 --env stage

POSIX 上 execFileSync(argv)execvp 不过 shell,理论必然 OK;db-utils.ts 的 temp-file -f 也是 psql 标准用法。但既然这是团队主力平台,merge 前手动确认一下更稳。

@Zhiying-Li-dot
Copy link
Copy Markdown
Author

@zhangjianye 上一条 @ 错人了,麻烦你帮忙在 macOS 上跑一下 sanity check(任一即可):

optima-show-env user-auth stage --filter DATABASE_URL
optima-query-db user-auth "SELECT 1" stage
optima-grant-balance <test-email> --amount 0.01 --env stage

PR 改的 db-utils.tsquery-db.ts 我在 Windows 上验证过;POSIX 平台 execFileSync(argv)execvp 理论必然 OK,但 merge 前在 mac 上手动跑一次更稳。

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.

1 participant