From ed916d6ddc4ae0374823286d71cdfd1808c5e0c1 Mon Sep 17 00:00:00 2001 From: "david.merle" Date: Thu, 5 Feb 2026 15:06:04 +0100 Subject: [PATCH] Fix cli correlation-id option not recognized --- README.md | 90 ++++++++++-- config/services.yaml | 4 +- src/DependencyInjection/Configuration.php | 8 +- .../CorrelationIdExtension.php | 2 +- src/EventListener/ConsoleListener.php | 11 +- .../Integration/ConsoleIntegrationTest.php | 20 +-- .../DependencyInjection/ConfigurationTest.php | 4 +- .../EventListener/ConsoleListenerTest.php | 133 ++++++++++++++++-- 8 files changed, 216 insertions(+), 56 deletions(-) diff --git a/README.md b/README.md index 8a581d9..47d9aa3 100644 --- a/README.md +++ b/README.md @@ -70,7 +70,7 @@ correlation_id: validation: enabled: true max_length: 255 - pattern: null + pattern: '/^[a-zA-Z0-9-_]+$/' monolog: enabled: true @@ -85,7 +85,49 @@ correlation_id: cli: enabled: true prefix: 'CLI-' - allow_option: true + allow_env_var: true +``` + +### Validation & Security + +By default, the bundle validates correlation IDs to prevent malicious input: + +```yaml +correlation_id: + validation: + enabled: true + max_length: 255 + pattern: '/^[a-zA-Z0-9-_]+$/' # Default: alphanumeric, dashes, underscores only +``` + +**Security features:** +- Rejects empty values +- Limits length to prevent buffer overflow attacks +- Blocks special characters that could be used for injection attacks (XSS, SQL injection, shell injection) +- Automatically generates a safe ID if validation fails + +**Customize the pattern** for your needs: + +#### UUID v4 only (strictest) +```yaml +correlation_id: + validation: + max_length: 36 + pattern: '/^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i' +``` + +#### Allow more characters +```yaml +correlation_id: + validation: + pattern: '/^[a-zA-Z0-9-_:]+$/' # Add colon for custom formats like "APP:REQ:123" +``` + +#### Disable pattern validation (not recommended) +```yaml +correlation_id: + validation: + pattern: null # Only length validation will apply ``` ### Configuration Examples @@ -100,15 +142,6 @@ Always generate a new ID, ignoring incoming headers: correlation_id: trust_header: false ``` -Strict Validation -Validate incoming IDs with specific rules: -```yaml -correlation_id: - validation: - enabled: true - max_length: 36 - pattern: '/^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i' -``` #### Enable Monolog Integration Automatically add correlation ID to all logs: ```yaml @@ -214,15 +247,42 @@ If Monolog is not installed, the integration is automatically disabled. When CLI integration is enabled (default), the bundle manages correlation IDs for Symfony Console commands. -### Global Option -If `cli.allow_option` is `true`, a global `--correlation-id` option is added to all commands: +### Environment Variable +If `cli.allow_env_var` is `true` (default), you can pass a correlation ID via the `CORRELATION_ID` environment variable: ```bash -php bin/console app:my-command --correlation-id=custom-id-123 +CORRELATION_ID=custom-id-123 php bin/console app:my-command +``` + +This is particularly useful when executing commands from a Process within an HTTP request context: + +```php +use Symfony\Component\Process\Process; + +class MyService +{ + public function __construct( + private readonly CorrelationIdStorage $correlationIdStorage + ) {} + + public function executeCommand(): void + { + // Get current correlation ID from HTTP request + $correlationId = $this->correlationIdStorage->get(); + + // Pass it to the console command via environment variable + $process = new Process( + ['php', 'bin/console', 'app:my-command'], + env: ['CORRELATION_ID' => $correlationId] + ); + + $process->run(); + } +} ``` ### Automatic ID Generation -If no option is provided, an ID is automatically generated using the configured generator and prefixed with `cli.prefix` (default: `CLI-`): +If no environment variable is provided, an ID is automatically generated using the configured generator and prefixed with `cli.prefix` (default: `CLI-`): **Example output for a generated ID:** `CLI-550e8400-e29b-41d4-a716-446655440000` diff --git a/config/services.yaml b/config/services.yaml index 19ff5ef..c16e097 100644 --- a/config/services.yaml +++ b/config/services.yaml @@ -48,6 +48,6 @@ services: $generator: '@MdavidDev\SymfonyCorrelationIdBundle\Service\Generator\CorrelationIdGeneratorInterface' $validator: '@MdavidDev\SymfonyCorrelationIdBundle\Validator\CorrelationIdValidator' $prefix: '%correlation_id.cli.prefix%' - $allowOption: '%correlation_id.cli.allow_option%' + $allowEnvVar: '%correlation_id.cli.allow_env_var%' tags: - - { name: kernel.event_subscriber } \ No newline at end of file + - { name: kernel.event_subscriber } diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 61b5076..9cd5d2c 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -43,8 +43,8 @@ public function getConfigTreeBuilder(): TreeBuilder ->min(1) ->end() ->scalarNode('pattern') - ->info('Regex pattern to validate correlation ID format') - ->defaultNull() + ->info('Regex pattern to validate correlation ID format (security: alphanumeric, dashes, underscores only)') + ->defaultValue('/^[a-zA-Z0-9-_]+$/') ->end() ->end() ->end() @@ -91,8 +91,8 @@ public function getConfigTreeBuilder(): TreeBuilder ->info('Prefix for CLI-generated correlation IDs') ->defaultValue('CLI-') ->end() - ->booleanNode('allow_option') - ->info('Allow --correlation-id option in commands') + ->booleanNode('allow_env_var') + ->info('Allow CORRELATION_ID environment variable') ->defaultTrue() ->end() ->end() diff --git a/src/DependencyInjection/CorrelationIdExtension.php b/src/DependencyInjection/CorrelationIdExtension.php index e5d340b..9d32ecb 100644 --- a/src/DependencyInjection/CorrelationIdExtension.php +++ b/src/DependencyInjection/CorrelationIdExtension.php @@ -34,7 +34,7 @@ public function load(array $configs, ContainerBuilder $container): void $container->setParameter('correlation_id.cli', $config['cli']); $container->setParameter('correlation_id.cli.enabled', $config['cli']['enabled']); $container->setParameter('correlation_id.cli.prefix', $config['cli']['prefix']); - $container->setParameter('correlation_id.cli.allow_option', $config['cli']['allow_option']); + $container->setParameter('correlation_id.cli.allow_env_var', $config['cli']['allow_env_var']); $loader = new YamlFileLoader( $container, diff --git a/src/EventListener/ConsoleListener.php b/src/EventListener/ConsoleListener.php index 8f6f33d..5126196 100644 --- a/src/EventListener/ConsoleListener.php +++ b/src/EventListener/ConsoleListener.php @@ -22,7 +22,7 @@ public function __construct( private readonly CorrelationIdGeneratorInterface $generator, private readonly CorrelationIdValidator $validator, private readonly string $prefix = 'CLI-', - private readonly bool $allowOption = true + private readonly bool $allowEnvVar = true ) { } @@ -43,13 +43,12 @@ public function onConsoleCommand(ConsoleCommandEvent $event): void return; } - $input = $event->getInput(); $correlationId = null; - if ($this->allowOption) { - $value = $input->getParameterOption('--' . self::OPTION_NAME, null); - if (is_string($value) && $value !== '') { - $correlationId = $this->validator->sanitize($value); + if ($this->allowEnvVar) { + $envValue = $_SERVER['CORRELATION_ID'] ?? $_ENV['CORRELATION_ID'] ?? null; + if (is_string($envValue) && $envValue !== '') { + $correlationId = $this->validator->sanitize($envValue); } } diff --git a/tests/Functional/Integration/ConsoleIntegrationTest.php b/tests/Functional/Integration/ConsoleIntegrationTest.php index 00f8aeb..912e5fc 100644 --- a/tests/Functional/Integration/ConsoleIntegrationTest.php +++ b/tests/Functional/Integration/ConsoleIntegrationTest.php @@ -68,8 +68,10 @@ protected function execute($input, $output): int /** * @throws Exception */ - public function testConsoleCommandUsesProvidedOption(): void + public function testConsoleCommandUsesProvidedEnvVar(): void { + $_SERVER['CORRELATION_ID'] = 'manual-id-123'; + $kernel = new ConsoleTestKernel('test', true); $kernel->boot(); @@ -83,20 +85,12 @@ public function testConsoleCommandUsesProvidedOption(): void $application->setDispatcher($dispatcher); $application->setAutoExit(false); - $definition = $application->getDefinition(); - $definition->addOption(new InputOption( - 'correlation-id', - null, - InputOption::VALUE_REQUIRED, - 'Correlation ID for this command execution' - )); - $capturedId = null; $command = new class($storage, $capturedId) extends Command { public ?string $capturedId = null; public function __construct(private readonly CorrelationIdStorage $storage, &$capturedId) { - parent::__construct('test:option'); + parent::__construct('test:envvar'); $this->capturedId = &$capturedId; } @@ -108,15 +102,13 @@ protected function execute($input, $output): int }; $application->addCommands([$command]); - $input = new ArrayInput([ - 'command' => 'test:option', - '--correlation-id' => 'manual-id-123' - ]); + $input = new ArrayInput(['command' => 'test:envvar']); $application->run($input, new NullOutput()); $this->assertSame('manual-id-123', $capturedId); $this->assertFalse($storage->has()); + unset($_SERVER['CORRELATION_ID']); $kernel->shutdown(); } } diff --git a/tests/Unit/DependencyInjection/ConfigurationTest.php b/tests/Unit/DependencyInjection/ConfigurationTest.php index 7c1936c..31122ea 100644 --- a/tests/Unit/DependencyInjection/ConfigurationTest.php +++ b/tests/Unit/DependencyInjection/ConfigurationTest.php @@ -29,7 +29,7 @@ public function testDefaultConfiguration(): void $this->assertTrue($config['validation']['enabled']); $this->assertSame(255, $config['validation']['max_length']); - $this->assertNull($config['validation']['pattern']); + $this->assertSame('/^[a-zA-Z0-9-_]+$/', $config['validation']['pattern']); $this->assertTrue($config['monolog']['enabled']); $this->assertSame('correlation_id', $config['monolog']['key']); @@ -39,7 +39,7 @@ public function testDefaultConfiguration(): void $this->assertTrue($config['cli']['enabled']); $this->assertSame('CLI-', $config['cli']['prefix']); - $this->assertTrue($config['cli']['allow_option']); + $this->assertTrue($config['cli']['allow_env_var']); } public function testCustomConfiguration(): void diff --git a/tests/Unit/EventListener/ConsoleListenerTest.php b/tests/Unit/EventListener/ConsoleListenerTest.php index c25fb41..4d2d23d 100644 --- a/tests/Unit/EventListener/ConsoleListenerTest.php +++ b/tests/Unit/EventListener/ConsoleListenerTest.php @@ -52,14 +52,14 @@ public function testSubscribedEvents(): void $this->assertArrayHasKey(ConsoleEvents::ERROR, $events); } - public function testGeneratesIdWithPrefixWhenNoOptionProvided(): void + public function testGeneratesIdWithPrefixWhenNoEnvVarProvided(): void { + unset($_SERVER['CORRELATION_ID'], $_ENV['CORRELATION_ID']); + $input = $this->createMock(InputInterface::class); $output = $this->createMock(OutputInterface::class); $command = $this->createMock(Command::class); - $input->method('getParameterOption')->with('--correlation-id', null)->willReturn(null); - $this->generator->expects($this->once())->method('generate')->willReturn('uuid-123'); $event = new ConsoleCommandEvent($command, $input, $output); @@ -68,54 +68,60 @@ public function testGeneratesIdWithPrefixWhenNoOptionProvided(): void $this->assertSame('CLI-uuid-123', $this->storage->get()); } - public function testUsesOptionWhenProvidedAndValid(): void + public function testUsesEnvVarWhenProvidedAndValid(): void { + $_SERVER['CORRELATION_ID'] = 'custom-id'; + $input = $this->createMock(InputInterface::class); $output = $this->createMock(OutputInterface::class); $command = $this->createMock(Command::class); - $input->method('getParameterOption')->with('--correlation-id', null)->willReturn('custom-id'); - $this->generator->expects($this->never())->method('generate'); $event = new ConsoleCommandEvent($command, $input, $output); $this->listener->onConsoleCommand($event); $this->assertSame('custom-id', $this->storage->get()); + + unset($_SERVER['CORRELATION_ID']); } - public function testGeneratesIdWhenOptionProvidedButInvalid(): void + public function testGeneratesIdWhenEnvVarProvidedButInvalid(): void { + $_SERVER['CORRELATION_ID'] = ' '; + $input = $this->createMock(InputInterface::class); $output = $this->createMock(OutputInterface::class); $command = $this->createMock(Command::class); - $input->method('getParameterOption')->with('--correlation-id', null)->willReturn(' '); - $this->generator->expects($this->once())->method('generate')->willReturn('uuid-456'); $event = new ConsoleCommandEvent($command, $input, $output); $this->listener->onConsoleCommand($event); $this->assertSame('CLI-uuid-456', $this->storage->get()); + + unset($_SERVER['CORRELATION_ID']); } - public function testDoesNotReadOptionWhenAllowOptionIsFalse(): void + public function testDoesNotReadEnvVarWhenAllowEnvVarIsFalse(): void { + $_SERVER['CORRELATION_ID'] = 'should-be-ignored'; + $listener = new ConsoleListener($this->storage, $this->generator, $this->validator, 'CLI-', false); $input = $this->createMock(InputInterface::class); $output = $this->createMock(OutputInterface::class); $command = $this->createMock(Command::class); - $input->expects($this->never())->method('getParameterOption'); - $this->generator->expects($this->once())->method('generate')->willReturn('uuid-789'); $event = new ConsoleCommandEvent($command, $input, $output); $listener->onConsoleCommand($event); $this->assertSame('CLI-uuid-789', $this->storage->get()); + + unset($_SERVER['CORRELATION_ID']); } public function testOnConsoleTerminateClearsStorage(): void @@ -151,4 +157,107 @@ public function testOnConsoleCommandDoesNothingIfCommandIsNull(): void $this->listener->onConsoleCommand($event); $this->assertNull($this->storage->get()); } + + public function testRejectsInvalidEnvVarAndGeneratesSafeId(): void + { + $invalidPayloads = [ + '', + ' ', + 'a' . str_repeat('x', 300), + ]; + + foreach ($invalidPayloads as $index => $payload) { + $_SERVER['CORRELATION_ID'] = $payload; + + $generator = $this->createMock(CorrelationIdGeneratorInterface::class); + $generator->expects($this->once())->method('generate')->willReturn('safe-uuid-' . $index); + + $listener = new ConsoleListener($this->storage, $generator, $this->validator, 'CLI-', true); + + $input = $this->createMock(InputInterface::class); + $output = $this->createMock(OutputInterface::class); + $command = $this->createMock(Command::class); + + $event = new ConsoleCommandEvent($command, $input, $output); + $listener->onConsoleCommand($event); + + $storedId = $this->storage->get(); + $this->assertStringStartsWith('CLI-', $storedId); + $this->assertStringContainsString('safe-uuid-' . $index, $storedId); + + $this->storage->clear(); + unset($_SERVER['CORRELATION_ID']); + } + } + + public function testRejectsSpecialCharactersWithStrictPattern(): void + { + $maliciousPayloads = [ + '', + '"; DROP TABLE users; --', + '$(rm -rf /)', + '../../../etc/passwd', + "\x00\x01\x02", + ]; + + foreach ($maliciousPayloads as $index => $payload) { + $_SERVER['CORRELATION_ID'] = $payload; + + $strictValidator = new CorrelationIdValidator( + enabled: true, + maxLength: 255, + pattern: '/^[a-zA-Z0-9-]+$/' + ); + + $generator = $this->createMock(CorrelationIdGeneratorInterface::class); + $generator->expects($this->once())->method('generate')->willReturn('safe-uuid-' . $index); + + $listener = new ConsoleListener($this->storage, $generator, $strictValidator, 'CLI-', true); + + $input = $this->createMock(InputInterface::class); + $output = $this->createMock(OutputInterface::class); + $command = $this->createMock(Command::class); + + $event = new ConsoleCommandEvent($command, $input, $output); + $listener->onConsoleCommand($event); + + $storedId = $this->storage->get(); + $this->assertStringStartsWith('CLI-', $storedId); + $this->assertStringContainsString('safe-uuid-' . $index, $storedId); + $this->assertStringNotContainsString('<', $storedId); + $this->assertStringNotContainsString('>', $storedId); + $this->assertStringNotContainsString(';', $storedId); + + $this->storage->clear(); + unset($_SERVER['CORRELATION_ID']); + } + } + + public function testAcceptsValidEnvVarFormats(): void + { + $validPayloads = [ + 'simple-id', + 'UUID-550e8400-e29b-41d4-a716-446655440000', + 'alphanumeric123', + 'with-dashes-and-underscores_123', + ]; + + foreach ($validPayloads as $payload) { + $_SERVER['CORRELATION_ID'] = $payload; + + $input = $this->createMock(InputInterface::class); + $output = $this->createMock(OutputInterface::class); + $command = $this->createMock(Command::class); + + $this->generator->expects($this->never())->method('generate'); + + $event = new ConsoleCommandEvent($command, $input, $output); + $this->listener->onConsoleCommand($event); + + $this->assertSame($payload, $this->storage->get()); + + $this->storage->clear(); + unset($_SERVER['CORRELATION_ID']); + } + } }