Fix nodejs: Create directory before executing script

[TheZoker]

Something I did not notice during my testing was, that the directory for nvm needs to be created first before the script can be installed. This PR fixes this.

[TheZoker]

@mafredri @matifali Can you guys please have a look at this? Thanks!

Edit:
So this is the issue I see in my logs:

Failed to install nvm: You have $NVM_DIR set to "$HOME/.nvm/nvm", but that directory does not exist. Check your profile files and environment.

But the directory does exists.

When I set this in the module config:

module "nodejs" {
  source               = "registry.coder.com/modules/nodejs/coder"
  version              = "1.0.8"
  agent_id             = coder_agent.main.id
  nvm_install_prefix   = "/home/${local.username}/.nvm"
  node_versions        = [
    "18",
    "20"
  ]
  default_node_version = "20"
}

it works.

But this seems to not work by default:

module "nodejs" {
  source               = "registry.coder.com/modules/nodejs/coder"
  version              = "1.0.8"
  agent_id             = coder_agent.main.id
  nvm_install_prefix   = "$HOME/.nvm"
  node_versions        = [
    "18",
    "20"
  ]
  default_node_version = "20"
}

Any idea why this is the case and how to fix it?

[mafredri]

Ah right, it makes sense that the latter doesn't work because we're trying to install it into the literal $HOME directory (echo \$HOME), not whatever $HOME expands to.

I think one question we need to ask is, should our modules support variables names in inputs? If so, we need to handle that consistently across our modules.

One approach is to allow "well known" variables/syntax, like $HOME, ${HOME} and ~. Then we can string-replace those, where applicable.

Allowing variable expansion is always a bit iffy, IMO, as it could be possible to inject things or make the script behave in unexpected ways. Just imagine if someone gave this as an input: $(rm -rf /).

Next week when @matifali is back, I'd love to have his input on this as well.

[matifali]

I think its a sane choice to not allow variable names as directories.

[TheZoker]

What's the best way to achieve this?

[matifali]

@TheZoker We can treat the input as a relative directory to $HOME and expand it within the script. This will endure that user can not pass something dangerous.

For example,

module "nodejs" {
  source               = "registry.coder.com/modules/nodejs/coder"
  version              = "1.0.8"
  agent_id             = coder_agent.main.id
  nvm_install_prefix   = ".nvm". # relative to $HOME
  node_versions        = [
    "18",
    "20"
  ]
  default_node_version = "20"
}

and then within the coder_script we do,

export NVM_DIR="$HOME/$${INSTALL_PREFIX}/nvm"
mkdir -p "$NVM_DIR"

What do you think @mafredri?

[mafredri]

What do you think @mafredri?

@matifali Seems like an OK solution 👍🏻.

Explicitly noting that we have to set $HOME/ in NVM_DIR (as you demonstrated) because otherwise the relative directory might be affected by coder_agent.dir.

[TheZoker]

@mafredri @matifali Can you guys please have a look at the latest commit?