Skip to content

fix(phpcs): refactor environment variable handling in commands #135

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Morgy93 merged 5 commits into from
Feb 14, 2026
Merged

fix(phpcs): refactor environment variable handling in commands #135

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4ed12ed
feat: refactor environment variable handling in commands
Morgy93 Feb 14, 2026
2477a70
Merge branch 'main' into fix-phpcs-1
Morgy93 Feb 14, 2026
057ca15
fix: remove unused method documentation in CompatibilityCheckCommand
Morgy93 Feb 14, 2026
bd5f06a
fix: remove empty check in setEnvVar method
Morgy93 Feb 14, 2026
2b6b27c
fix: update cached environment variable handling
Morgy93 Feb 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
194 changes: 165 additions & 29 deletions src/Console/Command/AbstractCommand.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@ abstract class AbstractCommand extends Command
*/
private array $secureEnvStorage = [];

/**
* @var array<string, string|null>|null
*/
private ?array $cachedEnv = null;

/**
* Get the command name with proper group structure
*
Expand Down Expand Up @@ -191,7 +196,7 @@ protected function handleInvalidThemeWithSuggestions(
* @param OutputInterface $output
* @return bool
*/
private function isInteractiveTerminal(OutputInterface $output): bool
protected function isInteractiveTerminal(OutputInterface $output): bool
{
// Check if output supports ANSI
if (!$output->isDecorated()) {
Expand Down Expand Up @@ -229,7 +234,7 @@ private function isInteractiveTerminal(OutputInterface $output): bool
*
* @return void
*/
private function setPromptEnvironment(): void
protected function setPromptEnvironment(): void
{
// Store original values for restoration
$this->originalEnv = [
Expand All @@ -249,7 +254,7 @@ private function setPromptEnvironment(): void
*
* @return void
*/
private function resetPromptEnvironment(): void
protected function resetPromptEnvironment(): void
{
foreach ($this->originalEnv as $key => $value) {
if ($value === null) {
Expand All @@ -261,49 +266,180 @@ private function resetPromptEnvironment(): void
}

/**
* Get environment variable value
*
* @param string $key
* @return string|null
* Safely get environment variable with sanitization
*/
private function getEnvVar(string $key): ?string
private function getEnvVar(string $name): ?string
{
return getenv($key) ?: null;
$value = $this->getSecureEnvironmentValue($name);

if ($value === null || $value === '') {
return null;
}

return $this->sanitizeEnvironmentValue($name, $value);
}

/**
* Get server variable value
*
* @param string $key
* @return string|null
* Securely retrieve environment variable without direct superglobal access
*/
private function getServerVar(string $key): ?string
private function getSecureEnvironmentValue(string $name): ?string
{
return $_SERVER[$key] ?? null;
if (!preg_match('/^[A-Z_][A-Z0-9_]*$/', $name)) {
return null;
}

$envVars = $this->getCachedEnvironmentVariables();
return $envVars[$name] ?? null;
}

/**
* Set environment variable securely
* Cache and filter environment variables safely
*
* @param string $key
* @param string $value
* @return void
* @return array<string, string|null>
*/
private function setEnvVar(string $key, string $value): void
private function getCachedEnvironmentVariables(): array
{
$this->secureEnvStorage[$key] = $value;
putenv("$key=$value");
if ($this->cachedEnv === null) {
$this->cachedEnv = [];
$allowedVars = [
'COLUMNS',
'LINES',
'TERM',
'CI',
'GITHUB_ACTIONS',
'GITLAB_CI',
'JENKINS_URL',
'TEAMCITY_VERSION',
];

foreach ($allowedVars as $var) {
if (isset($this->secureEnvStorage[$var])) {
$this->cachedEnv[$var] = $this->secureEnvStorage[$var];
} else {
$globalEnv = filter_input_array(INPUT_ENV) ?: [];
if (array_key_exists($var, $globalEnv)) {
$this->cachedEnv[$var] = (string) $globalEnv[$var];
}
}
}
}

return $this->cachedEnv;
}

/**
* Remove environment variable securely
*
* @param string $key
* @return void
* Sanitize environment value based on variable type
*/
private function sanitizeEnvironmentValue(string $name, string $value): ?string
{
return match ($name) {
'COLUMNS', 'LINES' => $this->sanitizeNumericValue($value),
'TERM' => $this->sanitizeTermValue($value),
'CI', 'GITHUB_ACTIONS', 'GITLAB_CI' => $this->sanitizeBooleanValue($value),
'JENKINS_URL', 'TEAMCITY_VERSION' => $this->sanitizeAlphanumericValue($value),
default => $this->sanitizeAlphanumericValue($value),
};
}

/**
* Sanitize numeric values (COLUMNS, LINES)
*/
private function sanitizeNumericValue(string $value): ?string
{
$filtered = filter_var($value, FILTER_VALIDATE_INT, ['options' => ['min_range' => 1, 'max_range' => 9999]]);
return $filtered !== false ? (string) $filtered : null;
}

/**
* Sanitize terminal type values
*/
private function sanitizeTermValue(string $value): ?string
{
$sanitized = preg_replace('/[^a-zA-Z0-9\-]/', '', $value);
if ($sanitized === null) {
return null;
}
return (strlen($sanitized) > 0 && strlen($sanitized) <= 50) ? $sanitized : null;
}

/**
* Sanitize boolean-like values
*/
private function sanitizeBooleanValue(string $value): ?string
{
$cleaned = strtolower(trim($value));
return in_array($cleaned, ['1', 'true', 'yes', 'on'], true) ? $cleaned : null;
}

/**
* Sanitize alphanumeric values
*/
private function sanitizeAlphanumericValue(string $value): ?string
{
$sanitized = preg_replace('/[^\w\-.]/', '', $value);
if ($sanitized === null) {
return null;
}
return (strlen($sanitized) > 0 && strlen($sanitized) <= 255) ? $sanitized : null;
}

/**
* Safely get server variable with sanitization
*/
private function getServerVar(string $name): ?string
{
if (!preg_match('/^[A-Z_][A-Z0-9_]*$/', $name)) {
return null;
}

$value = filter_input(INPUT_SERVER, $name);

if ($value === null || $value === false || $value === '') {
return null;
}

return $this->sanitizeAlphanumericValue((string) $value);
}

/**
* Safely set environment variable with validation
*/
private function setEnvVar(string $name, string $value): void
{
if (!preg_match('/^[A-Z_][A-Z0-9_]*$/', $name)) {
return;
}

$sanitizedValue = $this->sanitizeEnvironmentValue($name, $value);

if ($sanitizedValue !== null) {
$this->setSecureEnvironmentValue($name, $sanitizedValue);
}
}

/**
* Securely store environment variable without direct superglobal access
*/
private function setSecureEnvironmentValue(string $name, string $value): void
{
$this->secureEnvStorage[$name] = $value;
Comment thread
Morgy93 marked this conversation as resolved.
}

/**
* Clear the environment variable cache
*/
private function clearEnvironmentCache(): void
{
$this->secureEnvStorage = [];
Comment thread
Morgy93 marked this conversation as resolved.
$this->cachedEnv = null;
}

/**
* Securely remove environment variable from cache
*/
private function removeSecureEnvironmentValue(string $key): void
private function removeSecureEnvironmentValue(string $name): void
{
unset($this->secureEnvStorage[$key]);
putenv($key);
unset($this->secureEnvStorage[$name]);
Comment thread
Morgy93 marked this conversation as resolved.
$this->clearEnvironmentCache();
}
Comment thread
Morgy93 marked this conversation as resolved.
}
117 changes: 0 additions & 117 deletions src/Console/Command/Hyva/CompatibilityCheckCommand.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,6 @@ class CompatibilityCheckCommand extends AbstractCommand
private const SCOPE_THIRD_PARTY = 'third-party';
private const SCOPE_ALL = 'all';

/** @var array<string, string|null> */
private array $originalEnv = [];

/** @var array<string, string|null> */
private array $secureEnvStorage = [];

public function __construct(
private readonly CompatibilityChecker $compatibilityChecker
) {
Expand Down Expand Up @@ -373,115 +367,4 @@ private function displayRecommendations(): void
$this->io->text($recommendation);
}
}

/**
* Check if running in an interactive terminal
*/
private function isInteractiveTerminal(OutputInterface $output): bool
{
// Check if output is decorated (supports ANSI codes)
if (!$output->isDecorated()) {
return false;
}

// Check if STDIN is available and readable
if (!defined('STDIN') || !is_resource(STDIN)) {
return false;
}

// Check for common non-interactive environments
$nonInteractiveEnvs = [
'CI',
'GITHUB_ACTIONS',
'GITLAB_CI',
'JENKINS_URL',
'TEAMCITY_VERSION',
];

foreach ($nonInteractiveEnvs as $env) {
if ($this->getEnvVar($env) || $this->getServerVar($env)) {
return false;
}
}

// Additional check: try to detect if running in a proper TTY
// phpcs:ignore Magento2.Security.InsecureFunction.Found -- shell_exec required for TTY detection
$sttyOutput = shell_exec('stty -g 2>/dev/null');
return !empty($sttyOutput);
}

/**
* Set environment for Laravel Prompts to work properly in Docker/DDEV
*/
private function setPromptEnvironment(): void
{
// Store original values for reset
$this->originalEnv = [
'COLUMNS' => $this->getEnvVar('COLUMNS'),
'LINES' => $this->getEnvVar('LINES'),
'TERM' => $this->getEnvVar('TERM'),
];

// Set terminal environment variables using safe method
$this->setEnvVar('COLUMNS', '100');
$this->setEnvVar('LINES', '40');
$this->setEnvVar('TERM', 'xterm-256color');
}

/**
* Reset terminal environment after prompts
*/
private function resetPromptEnvironment(): void
{
// Reset environment variables to original state using secure methods
foreach ($this->originalEnv as $key => $value) {
if ($value === null) {
// Remove from our secure cache
$this->removeSecureEnvironmentValue($key);
} else {
// Restore original value using secure method
$this->setEnvVar($key, $value);
}
}
}

/**
* Securely remove environment variable from cache
*/
private function removeSecureEnvironmentValue(string $name): void
{
unset($this->secureEnvStorage[$name]);
}

/**
* Simplified environment variable getter
*/
private function getEnvVar(string $name): ?string
{
// Check secure storage first
if (isset($this->secureEnvStorage[$name])) {
return $this->secureEnvStorage[$name];
}

// Fall back to system environment
$value = getenv($name);
return $value !== false ? $value : null;
}

/**
* Simplified server variable getter
*/
private function getServerVar(string $name): ?string
{
return $_SERVER[$name] ?? null;
}

/**
* Simplified environment variable setter
*/
private function setEnvVar(string $name, string $value): void
{
$this->secureEnvStorage[$name] = $value;
putenv("$name=$value");
}
}
Loading
Loading