Skip to content

Commit 9c01ce5

Browse files
Morgy93Morgy93
authored
fix(phpcs): refactor environment variable handling in commands (#135)
* feat: refactor environment variable handling in commands * fix: remove unused method documentation in CompatibilityCheckCommand * fix: remove empty check in setEnvVar method * fix: update cached environment variable handling
1 parent cc8c534 commit 9c01ce5

4 files changed

Lines changed: 189 additions & 688 deletions

File tree

‎src/Console/Command/AbstractCommand.php‎

Lines changed: 165 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,11 @@ abstract class AbstractCommand extends Command
3939
*/
4040
private array $secureEnvStorage = [];
4141

42+
/**
43+
* @var array<string, string|null>|null
44+
*/
45+
private ?array $cachedEnv = null;
46+
4247
/**
4348
* Get the command name with proper group structure
4449
*
@@ -191,7 +196,7 @@ protected function handleInvalidThemeWithSuggestions(
191196
* @param OutputInterface $output
192197
* @return bool
193198
*/
194-
private function isInteractiveTerminal(OutputInterface $output): bool
199+
protected function isInteractiveTerminal(OutputInterface $output): bool
195200
{
196201
// Check if output supports ANSI
197202
if (!$output->isDecorated()) {
@@ -229,7 +234,7 @@ private function isInteractiveTerminal(OutputInterface $output): bool
229234
*
230235
* @return void
231236
*/
232-
private function setPromptEnvironment(): void
237+
protected function setPromptEnvironment(): void
233238
{
234239
// Store original values for restoration
235240
$this->originalEnv = [
@@ -249,7 +254,7 @@ private function setPromptEnvironment(): void
249254
*
250255
* @return void
251256
*/
252-
private function resetPromptEnvironment(): void
257+
protected function resetPromptEnvironment(): void
253258
{
254259
foreach ($this->originalEnv as $key => $value) {
255260
if ($value === null) {
@@ -261,49 +266,180 @@ private function resetPromptEnvironment(): void
261266
}
262267

263268
/**
264-
* Get environment variable value
265-
*
266-
* @param string $key
267-
* @return string|null
269+
* Safely get environment variable with sanitization
268270
*/
269-
private function getEnvVar(string $key): ?string
271+
private function getEnvVar(string $name): ?string
270272
{
271-
return getenv($key) ?: null;
273+
$value = $this->getSecureEnvironmentValue($name);
274+
275+
if ($value === null || $value === '') {
276+
return null;
277+
}
278+
279+
return $this->sanitizeEnvironmentValue($name, $value);
272280
}
273281

274282
/**
275-
* Get server variable value
276-
*
277-
* @param string $key
278-
* @return string|null
283+
* Securely retrieve environment variable without direct superglobal access
279284
*/
280-
private function getServerVar(string $key): ?string
285+
private function getSecureEnvironmentValue(string $name): ?string
281286
{
282-
return $_SERVER[$key] ?? null;
287+
if (!preg_match('/^[A-Z_][A-Z0-9_]*$/', $name)) {
288+
return null;
289+
}
290+
291+
$envVars = $this->getCachedEnvironmentVariables();
292+
return $envVars[$name] ?? null;
283293
}
284294

285295
/**
286-
* Set environment variable securely
296+
* Cache and filter environment variables safely
287297
*
288-
* @param string $key
289-
* @param string $value
290-
* @return void
298+
* @return array<string, string|null>
291299
*/
292-
private function setEnvVar(string $key, string $value): void
300+
private function getCachedEnvironmentVariables(): array
293301
{
294-
$this->secureEnvStorage[$key] = $value;
295-
putenv("$key=$value");
302+
if ($this->cachedEnv === null) {
303+
$this->cachedEnv = [];
304+
$allowedVars = [
305+
'COLUMNS',
306+
'LINES',
307+
'TERM',
308+
'CI',
309+
'GITHUB_ACTIONS',
310+
'GITLAB_CI',
311+
'JENKINS_URL',
312+
'TEAMCITY_VERSION',
313+
];
314+
315+
foreach ($allowedVars as $var) {
316+
if (isset($this->secureEnvStorage[$var])) {
317+
$this->cachedEnv[$var] = $this->secureEnvStorage[$var];
318+
} else {
319+
$globalEnv = filter_input_array(INPUT_ENV) ?: [];
320+
if (array_key_exists($var, $globalEnv)) {
321+
$this->cachedEnv[$var] = (string) $globalEnv[$var];
322+
}
323+
}
324+
}
325+
}
326+
327+
return $this->cachedEnv;
296328
}
297329

298330
/**
299-
* Remove environment variable securely
300-
*
301-
* @param string $key
302-
* @return void
331+
* Sanitize environment value based on variable type
332+
*/
333+
private function sanitizeEnvironmentValue(string $name, string $value): ?string
334+
{
335+
return match ($name) {
336+
'COLUMNS', 'LINES' => $this->sanitizeNumericValue($value),
337+
'TERM' => $this->sanitizeTermValue($value),
338+
'CI', 'GITHUB_ACTIONS', 'GITLAB_CI' => $this->sanitizeBooleanValue($value),
339+
'JENKINS_URL', 'TEAMCITY_VERSION' => $this->sanitizeAlphanumericValue($value),
340+
default => $this->sanitizeAlphanumericValue($value),
341+
};
342+
}
343+
344+
/**
345+
* Sanitize numeric values (COLUMNS, LINES)
346+
*/
347+
private function sanitizeNumericValue(string $value): ?string
348+
{
349+
$filtered = filter_var($value, FILTER_VALIDATE_INT, ['options' => ['min_range' => 1, 'max_range' => 9999]]);
350+
return $filtered !== false ? (string) $filtered : null;
351+
}
352+
353+
/**
354+
* Sanitize terminal type values
355+
*/
356+
private function sanitizeTermValue(string $value): ?string
357+
{
358+
$sanitized = preg_replace('/[^a-zA-Z0-9\-]/', '', $value);
359+
if ($sanitized === null) {
360+
return null;
361+
}
362+
return (strlen($sanitized) > 0 && strlen($sanitized) <= 50) ? $sanitized : null;
363+
}
364+
365+
/**
366+
* Sanitize boolean-like values
367+
*/
368+
private function sanitizeBooleanValue(string $value): ?string
369+
{
370+
$cleaned = strtolower(trim($value));
371+
return in_array($cleaned, ['1', 'true', 'yes', 'on'], true) ? $cleaned : null;
372+
}
373+
374+
/**
375+
* Sanitize alphanumeric values
376+
*/
377+
private function sanitizeAlphanumericValue(string $value): ?string
378+
{
379+
$sanitized = preg_replace('/[^\w\-.]/', '', $value);
380+
if ($sanitized === null) {
381+
return null;
382+
}
383+
return (strlen($sanitized) > 0 && strlen($sanitized) <= 255) ? $sanitized : null;
384+
}
385+
386+
/**
387+
* Safely get server variable with sanitization
388+
*/
389+
private function getServerVar(string $name): ?string
390+
{
391+
if (!preg_match('/^[A-Z_][A-Z0-9_]*$/', $name)) {
392+
return null;
393+
}
394+
395+
$value = filter_input(INPUT_SERVER, $name);
396+
397+
if ($value === null || $value === false || $value === '') {
398+
return null;
399+
}
400+
401+
return $this->sanitizeAlphanumericValue((string) $value);
402+
}
403+
404+
/**
405+
* Safely set environment variable with validation
406+
*/
407+
private function setEnvVar(string $name, string $value): void
408+
{
409+
if (!preg_match('/^[A-Z_][A-Z0-9_]*$/', $name)) {
410+
return;
411+
}
412+
413+
$sanitizedValue = $this->sanitizeEnvironmentValue($name, $value);
414+
415+
if ($sanitizedValue !== null) {
416+
$this->setSecureEnvironmentValue($name, $sanitizedValue);
417+
}
418+
}
419+
420+
/**
421+
* Securely store environment variable without direct superglobal access
422+
*/
423+
private function setSecureEnvironmentValue(string $name, string $value): void
424+
{
425+
$this->secureEnvStorage[$name] = $value;
426+
}
427+
428+
/**
429+
* Clear the environment variable cache
430+
*/
431+
private function clearEnvironmentCache(): void
432+
{
433+
$this->secureEnvStorage = [];
434+
$this->cachedEnv = null;
435+
}
436+
437+
/**
438+
* Securely remove environment variable from cache
303439
*/
304-
private function removeSecureEnvironmentValue(string $key): void
440+
private function removeSecureEnvironmentValue(string $name): void
305441
{
306-
unset($this->secureEnvStorage[$key]);
307-
putenv($key);
442+
unset($this->secureEnvStorage[$name]);
443+
$this->clearEnvironmentCache();
308444
}
309445
}

‎src/Console/Command/Hyva/CompatibilityCheckCommand.php‎

Lines changed: 0 additions & 117 deletions
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,6 @@ class CompatibilityCheckCommand extends AbstractCommand
3434
private const SCOPE_THIRD_PARTY = 'third-party';
3535
private const SCOPE_ALL = 'all';
3636

37-
/** @var array<string, string|null> */
38-
private array $originalEnv = [];
39-
40-
/** @var array<string, string|null> */
41-
private array $secureEnvStorage = [];
42-
4337
public function __construct(
4438
private readonly CompatibilityChecker $compatibilityChecker
4539
) {
@@ -373,115 +367,4 @@ private function displayRecommendations(): void
373367
$this->io->text($recommendation);
374368
}
375369
}
376-
377-
/**
378-
* Check if running in an interactive terminal
379-
*/
380-
private function isInteractiveTerminal(OutputInterface $output): bool
381-
{
382-
// Check if output is decorated (supports ANSI codes)
383-
if (!$output->isDecorated()) {
384-
return false;
385-
}
386-
387-
// Check if STDIN is available and readable
388-
if (!defined('STDIN') || !is_resource(STDIN)) {
389-
return false;
390-
}
391-
392-
// Check for common non-interactive environments
393-
$nonInteractiveEnvs = [
394-
'CI',
395-
'GITHUB_ACTIONS',
396-
'GITLAB_CI',
397-
'JENKINS_URL',
398-
'TEAMCITY_VERSION',
399-
];
400-
401-
foreach ($nonInteractiveEnvs as $env) {
402-
if ($this->getEnvVar($env) || $this->getServerVar($env)) {
403-
return false;
404-
}
405-
}
406-
407-
// Additional check: try to detect if running in a proper TTY
408-
// phpcs:ignore Magento2.Security.InsecureFunction.Found -- shell_exec required for TTY detection
409-
$sttyOutput = shell_exec('stty -g 2>/dev/null');
410-
return !empty($sttyOutput);
411-
}
412-
413-
/**
414-
* Set environment for Laravel Prompts to work properly in Docker/DDEV
415-
*/
416-
private function setPromptEnvironment(): void
417-
{
418-
// Store original values for reset
419-
$this->originalEnv = [
420-
'COLUMNS' => $this->getEnvVar('COLUMNS'),
421-
'LINES' => $this->getEnvVar('LINES'),
422-
'TERM' => $this->getEnvVar('TERM'),
423-
];
424-
425-
// Set terminal environment variables using safe method
426-
$this->setEnvVar('COLUMNS', '100');
427-
$this->setEnvVar('LINES', '40');
428-
$this->setEnvVar('TERM', 'xterm-256color');
429-
}
430-
431-
/**
432-
* Reset terminal environment after prompts
433-
*/
434-
private function resetPromptEnvironment(): void
435-
{
436-
// Reset environment variables to original state using secure methods
437-
foreach ($this->originalEnv as $key => $value) {
438-
if ($value === null) {
439-
// Remove from our secure cache
440-
$this->removeSecureEnvironmentValue($key);
441-
} else {
442-
// Restore original value using secure method
443-
$this->setEnvVar($key, $value);
444-
}
445-
}
446-
}
447-
448-
/**
449-
* Securely remove environment variable from cache
450-
*/
451-
private function removeSecureEnvironmentValue(string $name): void
452-
{
453-
unset($this->secureEnvStorage[$name]);
454-
}
455-
456-
/**
457-
* Simplified environment variable getter
458-
*/
459-
private function getEnvVar(string $name): ?string
460-
{
461-
// Check secure storage first
462-
if (isset($this->secureEnvStorage[$name])) {
463-
return $this->secureEnvStorage[$name];
464-
}
465-
466-
// Fall back to system environment
467-
$value = getenv($name);
468-
return $value !== false ? $value : null;
469-
}
470-
471-
/**
472-
* Simplified server variable getter
473-
*/
474-
private function getServerVar(string $name): ?string
475-
{
476-
return $_SERVER[$name] ?? null;
477-
}
478-
479-
/**
480-
* Simplified environment variable setter
481-
*/
482-
private function setEnvVar(string $name, string $value): void
483-
{
484-
$this->secureEnvStorage[$name] = $value;
485-
putenv("$name=$value");
486-
}
487370
}

0 commit comments

Comments
 (0)