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
- 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
// 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:
[ -e /home/user/My Dotfiles ] || git clone --depth 1 <repo> /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):
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.
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 throughquoteValue(). Any user-configured dotfiles path containing spaces (or other shell metacharacters) breaks the script via word splitting —git clone,[ -e ]andcdall operate on the wrong words — producing confusing failures during container start.Location
src/spec-common/dotfiles.tsinstallDotfiles[ -e ${targetPath} ],git clone … ${targetPath},cd ${targetPath}) and the same pattern at 77–79; also line 90 (ls -d ${targetPath}/.*)quoteValue()(124–126) deliberately single-quote-escape every environment value passed into the same scriptProblem
targetPathis user-configurable (dotfiles.targetPath, seeResolverParametersindevContainers.ts; default'~/dotfiles') and flows verbatim into the script. For a value containing whitespace, e.g./home/user/My Dotfiles, the generated lines become:which word-split into
[ -e /home/user/MyandDotfiles ], a two-argumentcloneinvocation with a strayDotfilesargument, and a two-directorycd. The result is a failed or mis-cloned install with opaque shell errors rather than either success or a clear message. The same applies torepositoryif it contains characters interpreted by the shell.Note that naive quoting cannot simply be added around
${targetPath}for the default value, because~/dotfilescurrently relies on unquoted tilde expansion — so the fix needs to handle tilde explicitly (e.g. expand to$HOMEin 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):Run
devcontainer upwith 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~/dotfileskeeps 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.targetPathsilently 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/REPOas properly quoted assignments (reusingquoteValue), convert a leading~/to$HOME/before quoting, and reference"$TARGET_PATH"throughout the script. A unit test exercising atargetPathwith a space would prevent regressions.