devcontainers / devcontainers/cli

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

Đang mở
#1,283 0 bình luận 0 reaction 0 người được giao Xem trên GitHub
Ngôn ngữ chính
TypeScript
Star
3k
Fork
457
Merge trung bình
13 giờ 17 phút
Pull request đã merge (30 ngày)
6

Mô tả

## 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.

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Hướng nghiên cứu

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.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Đánh giá

Công nghệ
shell, typescript
Lĩnh vực
cli
Loại issue
Lỗi
Độ khó
3/5
Thời gian dự kiến
1-2 ngày
Mức độ hoạt động
Sôi nổi
Độ rõ ràng
Đặc tả rõ ràng
Mức phù hợp với người mới
78/100

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.