Skip to content

fix codequality #51

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

Closed
dermatz wants to merge 2 commits into from
+309 −276
Closed

fix codequality #51

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
da9266e
✨ feat: add terminal environment services for improved interactivity
dermatz Dec 19, 2025
f554435
✨ feat: add verbose output for theme auto-repair process
dermatz Dec 19, 2025
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
284 changes: 11 additions & 273 deletions src/Console/Command/Theme/BuildCommand.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
use OpenForgeProject\MageForge\Console\Command\AbstractCommand;
use OpenForgeProject\MageForge\Model\ThemeList;
use OpenForgeProject\MageForge\Model\ThemePath;
use OpenForgeProject\MageForge\Service\Terminal\InteractiveTerminalChecker;
use OpenForgeProject\MageForge\Service\Terminal\TerminalEnvironmentService;
use OpenForgeProject\MageForge\Service\ThemeBuilder\BuilderPool;
use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Helper\Table;
Expand All @@ -22,18 +24,19 @@
*/
class BuildCommand extends AbstractCommand
{
private array $originalEnv = [];
private array $secureEnvStorage = [];

/**
* @param ThemePath $themePath
* @param ThemeList $themeList
* @param BuilderPool $builderPool
* @param TerminalEnvironmentService $environmentService
* @param InteractiveTerminalChecker $terminalChecker
*/
public function __construct(
private readonly ThemePath $themePath,
private readonly ThemeList $themeList,
private readonly BuilderPool $builderPool
private readonly BuilderPool $builderPool,
private readonly TerminalEnvironmentService $environmentService,
private readonly InteractiveTerminalChecker $terminalChecker
) {
parent::__construct();
}
Expand Down Expand Up @@ -66,14 +69,14 @@ protected function executeCommand(InputInterface $input, OutputInterface $output
$options = array_map(fn($theme) => $theme->getCode(), $themes);

// Check if we're in an interactive terminal environment
if (!$this->isInteractiveTerminal($output)) {
if (!$this->terminalChecker->isInteractiveTerminal($output)) {
// Fallback for non-interactive environments
$this->displayAvailableThemes($this->io);
return Command::SUCCESS;
}

// Set environment variables for Laravel Prompts
$this->setPromptEnvironment();
$this->environmentService->setPromptEnvironment();

$themeCodesPrompt = new MultiSelectPrompt(
label: 'Select themes to build',
Expand All @@ -88,7 +91,7 @@ protected function executeCommand(InputInterface $input, OutputInterface $output
\Laravel\Prompts\Prompt::terminal()->restoreTty();

// Reset environment
$this->resetPromptEnvironment();
$this->environmentService->resetPromptEnvironment();

// If no themes selected, show available themes
if (empty($themeCodes)) {
Expand All @@ -97,7 +100,7 @@ protected function executeCommand(InputInterface $input, OutputInterface $output
}
} catch (\Exception $e) {
// Reset environment on exception
$this->resetPromptEnvironment();
$this->environmentService->resetPromptEnvironment();
// Fallback if prompt fails
$this->io->error('Interactive mode failed: ' . $e->getMessage());
$this->displayAvailableThemes($this->io);
Expand Down Expand Up @@ -274,269 +277,4 @@ private function displayBuildSummary(SymfonyStyle $io, array $successList, float

$io->newLine();
}

/**
* Safely get environment variable with sanitization
* Uses secure method to avoid direct superglobal access
*/
private function getEnvVar(string $name): ?string
{
// Use a secure method to check environment variables
$value = $this->getSecureEnvironmentValue($name);

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

// Apply specific sanitization based on variable type
return $this->sanitizeEnvironmentValue($name, $value);
}

/**
* Securely retrieve environment variable without direct superglobal access
*/
private function getSecureEnvironmentValue(string $name): ?string
{
// Validate the variable name first
if (!preg_match('/^[A-Z_][A-Z0-9_]*$/', $name)) {
return null;
}

// Create a safe way to access environment without direct $_ENV access
$envVars = $this->getCachedEnvironmentVariables();
return $envVars[$name] ?? null;
}

/**
* Cache and filter environment variables safely
*/
private function getCachedEnvironmentVariables(): array
{
static $cachedEnv = null;

if ($cachedEnv === null) {
$cachedEnv = [];
// Only cache the specific variables we need
$allowedVars = ['COLUMNS', 'LINES', 'TERM', 'CI', 'GITHUB_ACTIONS', 'GITLAB_CI', 'JENKINS_URL', 'TEAMCITY_VERSION'];

foreach ($allowedVars as $var) {
// Check secure storage first
if (isset($this->secureEnvStorage[$var])) {
$cachedEnv[$var] = $this->secureEnvStorage[$var];
} else {
// Use array_key_exists to safely check without triggering warnings
$globalEnv = filter_input_array(INPUT_ENV) ?: [];
if (array_key_exists($var, $globalEnv)) {
$cachedEnv[$var] = (string) $globalEnv[$var];
}
}
}
}

return $cachedEnv;
}

/**
* 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);
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);
return (strlen($sanitized) > 0 && strlen($sanitized) <= 255) ? $sanitized : null;
}

/**
* Safely get server variable with sanitization
* Uses secure method to avoid direct superglobal access
*/
private function getServerVar(string $name): ?string
{
// Validate the variable name first
if (!preg_match('/^[A-Z_][A-Z0-9_]*$/', $name)) {
return null;
}

// Use filter_input to safely access server variables without deprecated filter
$value = filter_input(INPUT_SERVER, $name);

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

// Apply additional sanitization
return $this->sanitizeAlphanumericValue((string) $value);
}

/**
* Safely set environment variable with validation
* Avoids direct $_ENV access and putenv usage
*/
private function setEnvVar(string $name, string $value): void
{
// Validate input parameters
if (empty($name) || !is_string($name)) {
return;
}

// Validate variable name
if (!preg_match('/^[A-Z_][A-Z0-9_]*$/', $name)) {
return;
}

// Sanitize the value based on variable type
$sanitizedValue = $this->sanitizeEnvironmentValue($name, $value);

if ($sanitizedValue !== null) {
// Store in our safe cache instead of direct $_ENV manipulation
$this->setSecureEnvironmentValue($name, $sanitizedValue);
}
}

/**
* Securely store environment variable without direct superglobal access
*/
private function setSecureEnvironmentValue(string $name, string $value): void
{
// For this implementation, we'll store values in a class property
// to avoid direct manipulation of superglobals
if (!isset($this->secureEnvStorage)) {
$this->secureEnvStorage = [];
}
$this->secureEnvStorage[$name] = $value;
}

/**
* Clear the environment variable cache
*/
private function clearEnvironmentCache(): void
{
// Reset our secure storage
$this->secureEnvStorage = [];
} /**
* Check if the current environment supports interactive terminal input
*
* @param OutputInterface $output
* @return bool
*/
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
// This is a safer alternative to posix_isatty()
$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
* Uses secure method without direct $_ENV or putenv
*/
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
{
// Remove the specific variable from our secure storage
unset($this->secureEnvStorage[$name]);

// Clear the static cache to force refresh on next access
$this->clearEnvironmentCache();
}
}
49 changes: 49 additions & 0 deletions src/Service/Terminal/InteractiveTerminalChecker.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
<?php

declare(strict_types=1);

namespace OpenForgeProject\MageForge\Service\Terminal;

use Symfony\Component\Console\Output\OutputInterface;

/**
* Service for checking if the terminal supports interactive input
*/
class InteractiveTerminalChecker
{
public function __construct(
private readonly TerminalEnvironmentService $environmentService
) {
}

/**
* Check if the current environment supports interactive terminal input
*/
public function isInteractiveTerminal(OutputInterface $output): bool
{
if (!$output->isDecorated()) {
return false;
}

if (!defined('STDIN') || !is_resource(STDIN)) {
return false;
}

$nonInteractiveEnvs = [
'CI',
'GITHUB_ACTIONS',
'GITLAB_CI',
'JENKINS_URL',
'TEAMCITY_VERSION',
];

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

$sttyOutput = shell_exec('stty -g 2>/dev/null');
return !empty($sttyOutput);
}
}
Loading