devcontainers / devcontainers/cli

Dotfiles install script interpolates targetPath/repository unquoted: paths with spaces break git clone/cd

未关闭
#1,283 0 条评论 0 个 reaction 已指派 0 人 在 GitHub 查看
主要语言
TypeScript
星标
3k
派生
457
平均合并
13 小时 17 分钟
30 天内合并 PR
6

描述

## Summary

`installDotfiles()` interpolates `${targetPath}` and `${repository}` into a generated POSIX shell script **without quoting**, while environment-variable values in the very same script *are* escaped through `quoteValue()`. Any user-configured dotfiles path containing spaces (or other shell metacharacters) breaks the script via word splitting — `git clone`, `[ -e ]` and `cd` all operate on the wrong words — producing confusing failures during container start.

## Location

- File: [`src/spec-common/dotfiles.ts`](https://github.com/devcontainers/cli/blob/33073dbaba2545c51b4f8396e179c18231e80124/src/spec-common/dotfiles.ts)
- Function: `installDotfiles`
- Unquoted interpolations: lines 46, 48 (`[ -e ${targetPath} ]`, `git clone … ${targetPath}`, `cd ${targetPath}`) and the same pattern at 77–79; also line 90 (`ls -d ${targetPath}/.*`)
- Contrast: lines 33–35 + `quoteValue()` (124–126) deliberately single-quote-escape every environment value passed into the same script

```ts
// env values are quoted...
const allEnv = Object.keys(dockerEnvAndSecrets)
.reduce((env, key) => `${env}${key}=${quoteValue(dockerEnvAndSecrets[key])} `, '');
...
await shellServer.exec(`# Clone & install dotfiles
...
[ -e ${targetPath} ] || ${allEnv}git clone --depth 1 ${repository} ${targetPath} || exit $?
echo Setting current directory to '${targetPath}'
cd ${targetPath}
...`);
```

## Problem

`targetPath` is user-configurable (`dotfiles.targetPath`, see `ResolverParameters` in `devContainers.ts`; default `'~/dotfiles'`) and flows verbatim into the script. For a value containing whitespace, e.g. `/home/user/My Dotfiles`, the generated lines become:

```sh
[ -e /home/user/My Dotfiles ] || git clone --depth 1 /home/user/My Dotfiles || exit $?
cd /home/user/My Dotfiles
```

which word-split into `[ -e /home/user/My` and `Dotfiles ]`, a two-argument `clone` invocation with a stray `Dotfiles` argument, and a two-directory `cd`. The result is a failed or mis-cloned install with opaque shell errors rather than either success or a clear message. The same applies to `repository` if it contains characters interpreted by the shell.

Note that naive quoting cannot simply be added around `${targetPath}` for the *default* value, because `~/dotfiles` currently relies on unquoted tilde expansion — so the fix needs to handle tilde explicitly (e.g. expand to `$HOME` in TypeScript, or emit `"${HOME}/dotfiles"`), which is presumably why the current code avoids quotes.

## Trigger / Reproduction

Static analysis finding — not confirmed by execution; derived from the template literals at `main` (`33073dba`):

```jsonc
// devcontainer.json / CLI option
"dotfiles": {
"repository": "https://github.com/user/dotfiles.git",
"targetPath": "/home/user/My Dotfiles"
}
```

Run `devcontainer up` with dotfile installation enabled → the generated script splits words at the space and the install fails mid-way.

## Expected Behavior

Values interpolated into the shell script should be quoted/escaped consistently with how env values already are (`quoteValue`), with tilde handled explicitly so the default `~/dotfiles` keeps working.

## Actual Behavior

Unquoted expansion; paths with spaces (or glob/metacharacters) are split by the shell and every downstream command misbehaves.

## Impact

Any non-trivial `dotfiles.targetPath` silently corrupts the install script. Because the surrounding code already goes to the trouble of safely quoting environment values, this looks like an oversight rather than a constraint, and it produces hard-to-diagnose failures during dev-container startup.

## Suggested Direction

Emit `TARGET_PATH`/`REPO` as properly quoted assignments (reusing `quoteValue`), convert a leading `~/` to `$HOME/` before quoting, and reference `"$TARGET_PATH"` throughout the script. A unit test exercising a `targetPath` with a space would prevent regressions.

贡献指南

打开贡献指南

调研方向

Start in src/spec-common/dotfiles.ts at installDotfiles and read quoteValue, then inspect ResolverParameters in devContainers.ts to understand targetPath defaults. Add a unit test for a targetPath containing spaces and verify the generated install script preserves the path, handles the default tilde path, and completes the dotfiles installation successfully.

由索引模型根据 Issue 内容生成。

评估

技术栈
shell, typescript
领域
cli
Issue 类型
缺陷
难度
3/5
预计耗时
1-2 天
活跃度
活跃
描述清晰度
描述清楚
新手友好度
78/100

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。