From 7caef4987f028adaf32fb5be9015d0c4f0f84436 Mon Sep 17 00:00:00 2001 From: Zach <3724288+zedkipp@users.noreply.github.com> Date: Wed, 8 Apr 2026 17:02:33 -0600 Subject: [PATCH] feat: add input validation for user secret env names and file paths (#24103) Adds backend validation for user secret environment variable names and file paths. Env name validation enforces POSIX naming rules and blocks a deliberately aggressive denylist of reserved names and prefixes. The denylist errs on the side of blocking too much since it's easier to remove entries later than to add them after users have created conflicting secrets. File path validation requires paths to start with ~/ or /. --- codersdk/usersecretvalidation.go | 191 ++++++++++++++++++++++++++ codersdk/usersecretvalidation_test.go | 183 ++++++++++++++++++++++++ site/src/api/typesGenerated.ts | 14 ++ 3 files changed, 388 insertions(+) create mode 100644 codersdk/usersecretvalidation.go create mode 100644 codersdk/usersecretvalidation_test.go diff --git a/codersdk/usersecretvalidation.go b/codersdk/usersecretvalidation.go new file mode 100644 index 0000000000..9e2cb82089 --- /dev/null +++ b/codersdk/usersecretvalidation.go @@ -0,0 +1,191 @@ +package codersdk + +import ( + "regexp" + "strings" + + "golang.org/x/xerrors" +) + +// UserSecretEnvValidationOptions controls deployment-aware behavior +// in environment variable name validation. +type UserSecretEnvValidationOptions struct { + // AIGatewayEnabled indicates that the deployment has AI Gateway + // configured. When true, AI Gateway environment variables + // (OPENAI_API_KEY, etc.) are reserved to prevent conflicts. + AIGatewayEnabled bool +} + +var ( + // posixEnvNameRegex matches valid POSIX environment variable names: + // must start with a letter or underscore, followed by letters, + // digits, or underscores. + posixEnvNameRegex = regexp.MustCompile(`^[a-zA-Z_][a-zA-Z0-9_]*$`) + + // reservedEnvNames are system environment variables that must not + // be overridden by user secrets. This list is intentionally + // aggressive because it is easier to remove entries later than + // to add them after users have already created conflicting + // secrets. + reservedEnvNames = map[string]struct{}{ + // Core POSIX/login variables. Overriding these breaks + // basic shell and session behavior. + "PATH": {}, + "HOME": {}, + "SHELL": {}, + "USER": {}, + "LOGNAME": {}, + "PWD": {}, + "OLDPWD": {}, + + // Locale and terminal. Agents and IDEs depend on these + // being set correctly by the system. + "LANG": {}, + "TERM": {}, + + // Shell behavior. Overriding these can silently break + // word splitting, directory resolution, and script + // execution in every shell session and agent script. + "IFS": {}, + "CDPATH": {}, + + // Shell startup files. ENV is sourced by POSIX sh for + // interactive shells; BASH_ENV is sourced by bash for + // every non-interactive invocation (scripts, subshells). + // Allowing users to set these would inject arbitrary + // code into every shell and script in the workspace. + "ENV": {}, + "BASH_ENV": {}, + + // Temp directories. Overriding these is a security risk + // (symlink attacks, world-readable paths). + "TMPDIR": {}, + "TMP": {}, + "TEMP": {}, + + // Host identity. + "HOSTNAME": {}, + + // SSH session variables. The Coder agent sets + // SSH_AUTH_SOCK in agentssh.go; the others are set by + // sshd and should never be faked. + "SSH_AUTH_SOCK": {}, + "SSH_CLIENT": {}, + "SSH_CONNECTION": {}, + "SSH_TTY": {}, + + // Editor/pager. The Coder agent sets these so that git + // operations inside workspaces work non-interactively. + "EDITOR": {}, + "VISUAL": {}, + "PAGER": {}, + + // IDE integration. The agent sets these for code-server + // and VS Code Remote proxying. + "VSCODE_PROXY_URI": {}, + "CS_DISABLE_GETTING_STARTED_OVERRIDE": {}, + + // XDG base directories. Overriding these redirects + // config, cache, and runtime data for every tool in the + // workspace. + "XDG_RUNTIME_DIR": {}, + "XDG_CONFIG_HOME": {}, + "XDG_DATA_HOME": {}, + "XDG_CACHE_HOME": {}, + "XDG_STATE_HOME": {}, + + // OIDC token. The Coder agent injects a short-lived + // OIDC token for cloud auth flows (e.g. GCP workload + // identity). Overriding it could break provisioner and + // agent authentication. + "OIDC_TOKEN": {}, + } + + // aiGatewayReservedEnvNames are reserved only when AI Gateway + // is enabled on the deployment. When AI Gateway is disabled, + // users may legitimately want to inject their own API keys + // via secrets. + aiGatewayReservedEnvNames = map[string]struct{}{ + "OPENAI_API_KEY": {}, + "OPENAI_BASE_URL": {}, + "ANTHROPIC_AUTH_TOKEN": {}, + "ANTHROPIC_BASE_URL": {}, + } + + // reservedEnvPrefixes are namespace prefixes where every + // variable in the family is reserved. Checked after the + // exact-name map. The CODER / CODER_* namespace is handled + // separately with its own error message (see below). + reservedEnvPrefixes = []string{ + // The Coder agent sets GIT_SSH_COMMAND, GIT_ASKPASS, + // GIT_AUTHOR_*, GIT_COMMITTER_*, and several others. + // Blocking the entire GIT_* namespace avoids an arms + // race with new git env vars. + "GIT_", + + // Locale variables. LC_ALL, LC_CTYPE, LC_MESSAGES, + // etc. control character encoding, sorting, and + // formatting. Overriding them can break text + // processing in agents and IDEs. + "LC_", + + // Dynamic linker variables. Allowing users to set + // these would let a secret inject arbitrary shared + // libraries into every process in the workspace. + "LD_", + "DYLD_", + } +) + +// UserSecretEnvNameValid validates an environment variable name for +// a user secret. Empty string is allowed (means no env injection). +// The opts parameter controls deployment-aware checks such as AI +// bridge variable reservation. +func UserSecretEnvNameValid(s string, opts UserSecretEnvValidationOptions) error { + if s == "" { + return nil + } + + if !posixEnvNameRegex.MatchString(s) { + return xerrors.New("must start with a letter or underscore, followed by letters, digits, or underscores") + } + + upper := strings.ToUpper(s) + + if _, ok := reservedEnvNames[upper]; ok { + return xerrors.Errorf("%s is a reserved environment variable name", upper) + } + + if upper == "CODER" || strings.HasPrefix(upper, "CODER_") { + return xerrors.New("environment variable names starting with CODER_ are reserved for internal use") + } + + for _, prefix := range reservedEnvPrefixes { + if strings.HasPrefix(upper, prefix) { + return xerrors.Errorf("environment variables starting with %s are reserved", prefix) + } + } + + if opts.AIGatewayEnabled { + if _, ok := aiGatewayReservedEnvNames[upper]; ok { + return xerrors.Errorf("%s is reserved when AI Gateway is enabled", upper) + } + } + + return nil +} + +// UserSecretFilePathValid validates a file path for a user secret. +// Empty string is allowed (means no file injection). Non-empty paths +// must start with ~/ or /. +func UserSecretFilePathValid(s string) error { + if s == "" { + return nil + } + + if strings.HasPrefix(s, "~/") || strings.HasPrefix(s, "/") { + return nil + } + + return xerrors.New("file path must start with ~/ or /") +} diff --git a/codersdk/usersecretvalidation_test.go b/codersdk/usersecretvalidation_test.go new file mode 100644 index 0000000000..879ddcbf3a --- /dev/null +++ b/codersdk/usersecretvalidation_test.go @@ -0,0 +1,183 @@ +package codersdk_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/coder/coder/v2/codersdk" +) + +func TestUserSecretEnvNameValid(t *testing.T) { + t.Parallel() + + // noAIGateway is the default for most tests — AI Gateway disabled. + noAIGateway := codersdk.UserSecretEnvValidationOptions{} + withAIGateway := codersdk.UserSecretEnvValidationOptions{AIGatewayEnabled: true} + + tests := []struct { + name string + input string + opts codersdk.UserSecretEnvValidationOptions + wantErr bool + errMsg string + }{ + // Valid names. + {name: "SimpleUpper", input: "GITHUB_TOKEN", opts: noAIGateway}, + {name: "SimpleLower", input: "github_token", opts: noAIGateway}, + {name: "StartsWithUnderscore", input: "_FOO", opts: noAIGateway}, + {name: "SingleChar", input: "A", opts: noAIGateway}, + {name: "WithDigits", input: "A1B2", opts: noAIGateway}, + {name: "Empty", input: "", opts: noAIGateway}, + + // Invalid POSIX names. + {name: "StartsWithDigit", input: "1FOO", opts: noAIGateway, wantErr: true, errMsg: "must start with"}, + {name: "ContainsHyphen", input: "FOO-BAR", opts: noAIGateway, wantErr: true, errMsg: "must start with"}, + {name: "ContainsDot", input: "FOO.BAR", opts: noAIGateway, wantErr: true, errMsg: "must start with"}, + {name: "ContainsSpace", input: "FOO BAR", opts: noAIGateway, wantErr: true, errMsg: "must start with"}, + + // Reserved system names — core POSIX/login. + {name: "ReservedPATH", input: "PATH", opts: noAIGateway, wantErr: true, errMsg: "reserved"}, + {name: "ReservedHOME", input: "HOME", opts: noAIGateway, wantErr: true, errMsg: "reserved"}, + {name: "ReservedSHELL", input: "SHELL", opts: noAIGateway, wantErr: true, errMsg: "reserved"}, + {name: "ReservedUSER", input: "USER", opts: noAIGateway, wantErr: true, errMsg: "reserved"}, + {name: "ReservedLOGNAME", input: "LOGNAME", opts: noAIGateway, wantErr: true, errMsg: "reserved"}, + {name: "ReservedPWD", input: "PWD", opts: noAIGateway, wantErr: true, errMsg: "reserved"}, + {name: "ReservedOLDPWD", input: "OLDPWD", opts: noAIGateway, wantErr: true, errMsg: "reserved"}, + + // Reserved system names — locale/terminal. + {name: "ReservedLANG", input: "LANG", opts: noAIGateway, wantErr: true, errMsg: "reserved"}, + {name: "ReservedTERM", input: "TERM", opts: noAIGateway, wantErr: true, errMsg: "reserved"}, + + // Reserved system names — shell behavior. + {name: "ReservedIFS", input: "IFS", opts: noAIGateway, wantErr: true, errMsg: "reserved"}, + {name: "ReservedCDPATH", input: "CDPATH", opts: noAIGateway, wantErr: true, errMsg: "reserved"}, + + // Reserved system names — shell startup files. + {name: "ReservedENV", input: "ENV", opts: noAIGateway, wantErr: true, errMsg: "reserved"}, + {name: "ReservedBASH_ENV", input: "BASH_ENV", opts: noAIGateway, wantErr: true, errMsg: "reserved"}, + + // Reserved system names — temp directories. + {name: "ReservedTMPDIR", input: "TMPDIR", opts: noAIGateway, wantErr: true, errMsg: "reserved"}, + {name: "ReservedTMP", input: "TMP", opts: noAIGateway, wantErr: true, errMsg: "reserved"}, + {name: "ReservedTEMP", input: "TEMP", opts: noAIGateway, wantErr: true, errMsg: "reserved"}, + + // Reserved system names — host identity. + {name: "ReservedHOSTNAME", input: "HOSTNAME", opts: noAIGateway, wantErr: true, errMsg: "reserved"}, + + // Reserved system names — SSH. + {name: "ReservedSSH_AUTH_SOCK", input: "SSH_AUTH_SOCK", opts: noAIGateway, wantErr: true, errMsg: "reserved"}, + {name: "ReservedSSH_CLIENT", input: "SSH_CLIENT", opts: noAIGateway, wantErr: true, errMsg: "reserved"}, + {name: "ReservedSSH_CONNECTION", input: "SSH_CONNECTION", opts: noAIGateway, wantErr: true, errMsg: "reserved"}, + {name: "ReservedSSH_TTY", input: "SSH_TTY", opts: noAIGateway, wantErr: true, errMsg: "reserved"}, + + // Reserved system names — editor/pager. + {name: "ReservedEDITOR", input: "EDITOR", opts: noAIGateway, wantErr: true, errMsg: "reserved"}, + {name: "ReservedVISUAL", input: "VISUAL", opts: noAIGateway, wantErr: true, errMsg: "reserved"}, + {name: "ReservedPAGER", input: "PAGER", opts: noAIGateway, wantErr: true, errMsg: "reserved"}, + + // Reserved system names — IDE integration. + {name: "ReservedVSCODE_PROXY_URI", input: "VSCODE_PROXY_URI", opts: noAIGateway, wantErr: true, errMsg: "reserved"}, + {name: "ReservedCS_DISABLE", input: "CS_DISABLE_GETTING_STARTED_OVERRIDE", opts: noAIGateway, wantErr: true, errMsg: "reserved"}, + + // Reserved system names — XDG. + {name: "ReservedXDG_RUNTIME_DIR", input: "XDG_RUNTIME_DIR", opts: noAIGateway, wantErr: true, errMsg: "reserved"}, + {name: "ReservedXDG_CONFIG_HOME", input: "XDG_CONFIG_HOME", opts: noAIGateway, wantErr: true, errMsg: "reserved"}, + {name: "ReservedXDG_DATA_HOME", input: "XDG_DATA_HOME", opts: noAIGateway, wantErr: true, errMsg: "reserved"}, + {name: "ReservedXDG_CACHE_HOME", input: "XDG_CACHE_HOME", opts: noAIGateway, wantErr: true, errMsg: "reserved"}, + {name: "ReservedXDG_STATE_HOME", input: "XDG_STATE_HOME", opts: noAIGateway, wantErr: true, errMsg: "reserved"}, + + // Reserved system names — OIDC. + {name: "ReservedOIDC_TOKEN", input: "OIDC_TOKEN", opts: noAIGateway, wantErr: true, errMsg: "reserved"}, + + // AI Gateway vars — blocked when AI Gateway is enabled. + {name: "AIGateway/OPENAI_API_KEY/Enabled", input: "OPENAI_API_KEY", opts: withAIGateway, wantErr: true, errMsg: "AI Gateway"}, + {name: "AIGateway/OPENAI_BASE_URL/Enabled", input: "OPENAI_BASE_URL", opts: withAIGateway, wantErr: true, errMsg: "AI Gateway"}, + {name: "AIGateway/ANTHROPIC_AUTH_TOKEN/Enabled", input: "ANTHROPIC_AUTH_TOKEN", opts: withAIGateway, wantErr: true, errMsg: "AI Gateway"}, + {name: "AIGateway/ANTHROPIC_BASE_URL/Enabled", input: "ANTHROPIC_BASE_URL", opts: withAIGateway, wantErr: true, errMsg: "AI Gateway"}, + + // AI Gateway vars — allowed when AI Gateway is disabled. + {name: "AIGateway/OPENAI_API_KEY/Disabled", input: "OPENAI_API_KEY", opts: noAIGateway}, + {name: "AIGateway/OPENAI_BASE_URL/Disabled", input: "OPENAI_BASE_URL", opts: noAIGateway}, + {name: "AIGateway/ANTHROPIC_AUTH_TOKEN/Disabled", input: "ANTHROPIC_AUTH_TOKEN", opts: noAIGateway}, + {name: "AIGateway/ANTHROPIC_BASE_URL/Disabled", input: "ANTHROPIC_BASE_URL", opts: noAIGateway}, + + // Case insensitivity. + {name: "ReservedCaseInsensitive", input: "path", opts: noAIGateway, wantErr: true, errMsg: "reserved"}, + + // CODER_ prefix. + {name: "CoderExact", input: "CODER", opts: noAIGateway, wantErr: true, errMsg: "CODER_"}, + {name: "CoderPrefix", input: "CODER_WORKSPACE_NAME", opts: noAIGateway, wantErr: true, errMsg: "CODER_"}, + {name: "CoderAgentToken", input: "CODER_AGENT_TOKEN", opts: noAIGateway, wantErr: true, errMsg: "CODER_"}, + {name: "CoderLowerCase", input: "coder_foo", opts: noAIGateway, wantErr: true, errMsg: "CODER_"}, + + // GIT_* prefix. + {name: "GitSSHCommand", input: "GIT_SSH_COMMAND", opts: noAIGateway, wantErr: true, errMsg: "GIT_"}, + {name: "GitAskpass", input: "GIT_ASKPASS", opts: noAIGateway, wantErr: true, errMsg: "GIT_"}, + {name: "GitAuthorName", input: "GIT_AUTHOR_NAME", opts: noAIGateway, wantErr: true, errMsg: "GIT_"}, + {name: "GitLowerCase", input: "git_editor", opts: noAIGateway, wantErr: true, errMsg: "GIT_"}, + + // LC_* prefix (locale). + {name: "LcAll", input: "LC_ALL", opts: noAIGateway, wantErr: true, errMsg: "LC_"}, + {name: "LcCtype", input: "LC_CTYPE", opts: noAIGateway, wantErr: true, errMsg: "LC_"}, + + // LD_* prefix (dynamic linker). + {name: "LdPreload", input: "LD_PRELOAD", opts: noAIGateway, wantErr: true, errMsg: "LD_"}, + {name: "LdLibraryPath", input: "LD_LIBRARY_PATH", opts: noAIGateway, wantErr: true, errMsg: "LD_"}, + + // DYLD_* prefix (macOS dynamic linker). + {name: "DyldInsert", input: "DYLD_INSERT_LIBRARIES", opts: noAIGateway, wantErr: true, errMsg: "DYLD_"}, + {name: "DyldLibraryPath", input: "DYLD_LIBRARY_PATH", opts: noAIGateway, wantErr: true, errMsg: "DYLD_"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + err := codersdk.UserSecretEnvNameValid(tt.input, tt.opts) + if tt.wantErr { + assert.Error(t, err) + if tt.errMsg != "" { + assert.Contains(t, err.Error(), tt.errMsg) + } + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestUserSecretFilePathValid(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + input string + wantErr bool + }{ + // Valid paths. + {name: "TildePath", input: "~/foo"}, + {name: "TildeSSH", input: "~/.ssh/id_rsa"}, + {name: "AbsolutePath", input: "/home/coder/.ssh/id_rsa"}, + {name: "RootPath", input: "/"}, + {name: "Empty", input: ""}, + + // Invalid paths. + {name: "BareRelative", input: "foo/bar", wantErr: true}, + {name: "DotRelative", input: ".ssh/id_rsa", wantErr: true}, + {name: "JustFilename", input: "credentials", wantErr: true}, + {name: "TildeNoSlash", input: "~foo", wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + err := codersdk.UserSecretFilePathValid(tt.input) + if tt.wantErr { + assert.Error(t, err) + assert.Contains(t, err.Error(), "must start with") + } else { + assert.NoError(t, err) + } + }) + } +} diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 8ea7cf2c6c..e277b6368c 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -8081,6 +8081,20 @@ export interface UserSecret { readonly updated_at: string; } +// From codersdk/usersecretvalidation.go +/** + * UserSecretEnvValidationOptions controls deployment-aware behavior + * in environment variable name validation. + */ +export interface UserSecretEnvValidationOptions { + /** + * AIGatewayEnabled indicates that the deployment has AI Gateway + * configured. When true, AI Gateway environment variables + * (OPENAI_API_KEY, etc.) are reserved to prevent conflicts. + */ + readonly AIGatewayEnabled: boolean; +} + // From codersdk/users.go export type UserStatus = "active" | "dormant" | "suspended";