From 06fa3217dc95e670df6b2191000537e33df09a4b Mon Sep 17 00:00:00 2001 From: Thomas Date: Tue, 7 Jul 2026 06:00:15 -0500 Subject: [PATCH 1/2] feat(bundle): graceful handling of missing secrets in env var processor Missing secrets now throw Symfony's EnvNotFoundException (manager and key in the message, original SecretNotFoundException as previous) instead of leaking the raw exception, which also lets the built-in default: processor compose. Opt in to warning-plus-null resolution per reference with the new secretaryOptional:/secretaryArrayOptional: prefixes, or globally with the allow_missing_secrets bundle config. Only SecretNotFoundException is handled; other adapter errors still bubble up. Closes #25 Co-Authored-By: Claude Fable 5 --- composer.json | 1 + .../DependencyInjection/Configuration.php | 4 + .../SecretaryExtension.php | 3 + .../EnvVar/EnvVarProcessor.php | 39 +++- src/Bundle/SecretaryBundle/README.md | 40 +++- .../Tests/EnvVarProcessorTest.php | 205 ++++++++++++++++++ .../Tests/SecretaryBundleTest.php | 55 +++++ src/Bundle/SecretaryBundle/composer.json | 1 + 8 files changed, 340 insertions(+), 8 deletions(-) create mode 100644 src/Bundle/SecretaryBundle/Tests/EnvVarProcessorTest.php diff --git a/composer.json b/composer.json index 1700f14..f28e2cc 100644 --- a/composer.json +++ b/composer.json @@ -10,6 +10,7 @@ "guzzlehttp/guzzle": "^7.0", "mockery/mockery": "^1.6.12", "phpunit/phpunit": "^10.5 || ^11.0 || ^12.0 || ^13.0", + "psr/log": "^1.0 || ^2.0 || ^3.0", "psr/simple-cache": "^1.0 || ^2.0 || ^3.0", "symfony/config": "^5.3 || ^6.0 || ^7.0 || ^8.0", "symfony/dependency-injection": "^5.0 || ^6.0 || ^7.0 || ^8.0", diff --git a/src/Bundle/SecretaryBundle/DependencyInjection/Configuration.php b/src/Bundle/SecretaryBundle/DependencyInjection/Configuration.php index d6f4074..18ea359 100644 --- a/src/Bundle/SecretaryBundle/DependencyInjection/Configuration.php +++ b/src/Bundle/SecretaryBundle/DependencyInjection/Configuration.php @@ -28,6 +28,10 @@ public function getConfigTreeBuilder(): TreeBuilder $rootNode ->children() + ->booleanNode('allow_missing_secrets') + ->defaultFalse() + ->info('When true, missing secrets resolve to null with a warning instead of throwing') + ->end() ->append($this->addAdaptersSection()) ->end(); diff --git a/src/Bundle/SecretaryBundle/DependencyInjection/SecretaryExtension.php b/src/Bundle/SecretaryBundle/DependencyInjection/SecretaryExtension.php index fe0da9b..4d959a8 100644 --- a/src/Bundle/SecretaryBundle/DependencyInjection/SecretaryExtension.php +++ b/src/Bundle/SecretaryBundle/DependencyInjection/SecretaryExtension.php @@ -17,6 +17,7 @@ use Symfony\Component\DependencyInjection\Alias; use Symfony\Component\DependencyInjection\Argument\IteratorArgument; use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\DependencyInjection\Extension\Extension; use Symfony\Component\DependencyInjection\Reference; @@ -74,6 +75,8 @@ public function load(array $configs, ContainerBuilder $container): void $container->register('secretary.env_var_processor', EnvVarProcessor::class) ->addArgument(new IteratorArgument($services)) + ->addArgument(new Reference('logger', ContainerInterface::IGNORE_ON_INVALID_REFERENCE)) + ->addArgument($config['allow_missing_secrets']) ->addTag('container.env_var_processor') ->setPublic(false); } diff --git a/src/Bundle/SecretaryBundle/EnvVar/EnvVarProcessor.php b/src/Bundle/SecretaryBundle/EnvVar/EnvVarProcessor.php index c6a597b..465df44 100644 --- a/src/Bundle/SecretaryBundle/EnvVar/EnvVarProcessor.php +++ b/src/Bundle/SecretaryBundle/EnvVar/EnvVarProcessor.php @@ -10,8 +10,12 @@ namespace Secretary\Bundle\SecretaryBundle\EnvVar; +use Psr\Log\LoggerInterface; +use Psr\Log\NullLogger; +use Secretary\Exception\SecretNotFoundException; use Secretary\Manager; use Symfony\Component\DependencyInjection\EnvVarProcessorInterface; +use Symfony\Component\DependencyInjection\Exception\EnvNotFoundException; class EnvVarProcessor implements EnvVarProcessorInterface { @@ -20,9 +24,15 @@ class EnvVarProcessor implements EnvVarProcessorInterface */ private array $managers; - public function __construct(\Traversable $managers) - { + private LoggerInterface $logger; + + public function __construct( + \Traversable $managers, + ?LoggerInterface $logger = null, + private bool $allowMissingSecrets = false + ) { $this->managers = iterator_to_array($managers); + $this->logger = $logger ?? new NullLogger(); } /** @@ -38,7 +48,20 @@ public function getEnv(string $prefix, string $name, \Closure $getEnv): mixed $manager = $this->managers[$parts[0]]; $key = $getEnv($parts[1]); - $value = $manager->getSecret($key)->getValue(); + + try { + $value = $manager->getSecret($key)->getValue(); + } catch (SecretNotFoundException $e) { + $message = sprintf('Secret "%s" was not found in manager "%s".', $key, $parts[0]); + + if (!$this->allowMissingSecrets && !str_ends_with($prefix, 'Optional')) { + throw new EnvNotFoundException($message, 0, $e); + } + + $this->logger->warning($message); + + return null; + } if (array_key_exists(2, $parts)) { if (!is_array($value)) { @@ -57,10 +80,12 @@ public function getEnv(string $prefix, string $name, \Closure $getEnv): mixed public static function getProvidedTypes(): array { return [ - 'secretary' => 'bool|int|float|string', - 'secret' => 'bool|int|float|string', - 'secretArray' => 'array', - 'secretaryArray' => 'array', + 'secretary' => 'bool|int|float|string', + 'secret' => 'bool|int|float|string', + 'secretArray' => 'array', + 'secretaryArray' => 'array', + 'secretaryOptional' => 'bool|int|float|string', + 'secretaryArrayOptional' => 'array', ]; } } diff --git a/src/Bundle/SecretaryBundle/README.md b/src/Bundle/SecretaryBundle/README.md index 7691f11..ac8fa1f 100644 --- a/src/Bundle/SecretaryBundle/README.md +++ b/src/Bundle/SecretaryBundle/README.md @@ -46,4 +46,42 @@ secretary: enabled: true type: psr6 service_id: cache.secrets -``` \ No newline at end of file +``` + +### Resolving secrets in env vars + +```yaml +parameters: + db_pass: '%env(secretary:default:DB_PASS)%' # scalar secret + db_conf: '%env(secretaryArray:default:DB_CONFIG)%' # array secret +``` + +### Missing secrets + +By default, a missing secret throws Symfony's `EnvNotFoundException` (with the manager name and +secret key in the message, and the original `SecretNotFoundException` as `previous`). + +To resolve `null` instead — with a warning logged through the default logger — you can either +opt in per reference with the `secretaryOptional:` / `secretaryArrayOptional:` prefixes: + +```yaml +parameters: + feature_key: '%env(secretaryOptional:default:FEATURE_KEY)%' +``` + +or globally via the bundle config: + +```yaml +secretary: + allow_missing_secrets: true # defaults to false +``` + +Symfony's built-in `default:` processor also composes, if you want a fallback parameter instead: + +```yaml +parameters: + fallback: 'some-default' + db_pass: '%env(default:fallback:secretary:default:DB_PASS)%' +``` + +Only missing secrets are handled this way — network, auth, and other adapter errors always bubble up. \ No newline at end of file diff --git a/src/Bundle/SecretaryBundle/Tests/EnvVarProcessorTest.php b/src/Bundle/SecretaryBundle/Tests/EnvVarProcessorTest.php new file mode 100644 index 0000000..0bef858 --- /dev/null +++ b/src/Bundle/SecretaryBundle/Tests/EnvVarProcessorTest.php @@ -0,0 +1,205 @@ + + * @date 2019 + * @license https://opensource.org/licenses/MIT + */ + +namespace Secretary\Tests; + +use PHPUnit\Framework\Attributes\CoversClass; +use PHPUnit\Framework\TestCase; +use Psr\Log\AbstractLogger; +use Secretary\Adapter\AbstractAdapter; +use Secretary\Bundle\SecretaryBundle\EnvVar\EnvVarProcessor; +use Secretary\Exception\SecretNotFoundException; +use Secretary\Manager; +use Secretary\Secret; +use Symfony\Component\DependencyInjection\Exception\EnvNotFoundException; + +#[CoversClass(EnvVarProcessor::class)] +class EnvVarProcessorTest extends TestCase +{ + private SpyLogger $logger; + + protected function setUp(): void + { + $this->logger = new SpyLogger(); + } + + public function testMissingSecretThrowsEnvNotFoundException(): void + { + $processor = $this->createProcessor(); + + try { + $processor->getEnv('secretary', 'default:MISSING_KEY', fn (string $name) => $name); + $this->fail('Expected EnvNotFoundException to be thrown'); + } catch (EnvNotFoundException $e) { + $this->assertStringContainsString('MISSING_KEY', $e->getMessage()); + $this->assertStringContainsString('default', $e->getMessage()); + $this->assertInstanceOf(SecretNotFoundException::class, $e->getPrevious()); + } + } + + public function testOptionalPrefixLogsWarningAndReturnsNull(): void + { + $processor = $this->createProcessor(); + + $value = $processor->getEnv('secretaryOptional', 'default:MISSING_KEY', fn (string $name) => $name); + + $this->assertNull($value); + $this->assertCount(1, $this->logger->records); + $this->assertSame('warning', $this->logger->records[0]['level']); + $this->assertStringContainsString('MISSING_KEY', $this->logger->records[0]['message']); + } + + public function testArrayOptionalPrefixLogsWarningAndReturnsNull(): void + { + $processor = $this->createProcessor(); + + $value = $processor->getEnv('secretaryArrayOptional', 'default:MISSING_KEY', fn (string $name) => $name); + + $this->assertNull($value); + $this->assertCount(1, $this->logger->records); + $this->assertSame('warning', $this->logger->records[0]['level']); + } + + public function testAllowMissingSecretsMakesStandardPrefixGraceful(): void + { + $processor = $this->createProcessor(allowMissingSecrets: true); + + $value = $processor->getEnv('secretary', 'default:MISSING_KEY', fn (string $name) => $name); + + $this->assertNull($value); + $this->assertCount(1, $this->logger->records); + $this->assertSame('warning', $this->logger->records[0]['level']); + } + + public function testMissingSecretWithoutLoggerStillResolvesNull(): void + { + $manager = new Manager(new InMemoryAdapter([])); + $processor = new EnvVarProcessor(new \ArrayIterator(['default' => $manager])); + + $value = $processor->getEnv('secretaryOptional', 'default:MISSING_KEY', fn (string $name) => $name); + + $this->assertNull($value); + } + + public function testExistingSecretResolvesNormally(): void + { + $processor = $this->createProcessor(['DB_PASS' => 'hunter2']); + + $value = $processor->getEnv('secretary', 'default:DB_PASS', fn (string $name) => $name); + + $this->assertSame('hunter2', $value); + $this->assertCount(0, $this->logger->records); + } + + public function testExistingSecretResolvesNormallyWithOptionalPrefix(): void + { + $processor = $this->createProcessor(['DB_PASS' => 'hunter2']); + + $value = $processor->getEnv('secretaryOptional', 'default:DB_PASS', fn (string $name) => $name); + + $this->assertSame('hunter2', $value); + $this->assertCount(0, $this->logger->records); + } + + public function testOtherExceptionsStillBubbleUp(): void + { + $manager = new Manager(new ThrowingAdapter()); + $processor = new EnvVarProcessor(new \ArrayIterator(['default' => $manager]), $this->logger); + + $this->expectException(\RuntimeException::class); + $this->expectExceptionMessage('adapter blew up'); + + $processor->getEnv('secretary', 'default:ANY_KEY', fn (string $name) => $name); + } + + public function testOtherExceptionsStillBubbleUpWithOptionalPrefix(): void + { + $manager = new Manager(new ThrowingAdapter()); + $processor = new EnvVarProcessor(new \ArrayIterator(['default' => $manager]), $this->logger); + + $this->expectException(\RuntimeException::class); + + $processor->getEnv('secretaryOptional', 'default:ANY_KEY', fn (string $name) => $name); + } + + public function testGetProvidedTypesIncludesOptionalPrefixes(): void + { + $types = EnvVarProcessor::getProvidedTypes(); + + $this->assertSame('bool|int|float|string', $types['secretaryOptional']); + $this->assertSame('array', $types['secretaryArrayOptional']); + } + + /** + * @param array $secrets + */ + private function createProcessor(array $secrets = [], bool $allowMissingSecrets = false): EnvVarProcessor + { + $manager = new Manager(new InMemoryAdapter($secrets)); + + return new EnvVarProcessor(new \ArrayIterator(['default' => $manager]), $this->logger, $allowMissingSecrets); + } +} + +class SpyLogger extends AbstractLogger +{ + /** @var list */ + public array $records = []; + + public function log($level, \Stringable|string $message, array $context = []): void + { + $this->records[] = ['level' => (string) $level, 'message' => (string) $message, 'context' => $context]; + } +} + +class InMemoryAdapter extends AbstractAdapter +{ + /** + * @param array $secrets + */ + public function __construct(private array $secrets) + { + } + + public function getSecret(string $key, ?array $options = []): Secret + { + if (!array_key_exists($key, $this->secrets)) { + throw new SecretNotFoundException($key); + } + + return new Secret($key, $this->secrets[$key]); + } + + public function putSecret(Secret $secret, ?array $options = []): Secret + { + return $secret; + } + + public function deleteSecret(Secret $secret, ?array $options = []): void + { + } +} + +class ThrowingAdapter extends AbstractAdapter +{ + public function getSecret(string $key, ?array $options = []): Secret + { + throw new \RuntimeException('adapter blew up'); + } + + public function putSecret(Secret $secret, ?array $options = []): Secret + { + return $secret; + } + + public function deleteSecret(Secret $secret, ?array $options = []): void + { + } +} diff --git a/src/Bundle/SecretaryBundle/Tests/SecretaryBundleTest.php b/src/Bundle/SecretaryBundle/Tests/SecretaryBundleTest.php index ce4cb25..52c7971 100644 --- a/src/Bundle/SecretaryBundle/Tests/SecretaryBundleTest.php +++ b/src/Bundle/SecretaryBundle/Tests/SecretaryBundleTest.php @@ -17,6 +17,8 @@ use Secretary\Bundle\SecretaryBundle\SecretaryBundle; use Secretary\Manager; use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\ContainerInterface; +use Symfony\Component\DependencyInjection\Reference; #[CoversClass(SecretaryBundle::class)] #[CoversClass(SecretaryExtension::class)] @@ -141,4 +143,57 @@ public function testEnvVarProcessorIsRegistered(): void $definition = $container->getDefinition('secretary.env_var_processor'); $this->assertEquals(EnvVarProcessor::class, $definition->getClass()); } + + public function testEnvVarProcessorIsWiredWithOptionalLoggerAndDefaults(): void + { + $container = new ContainerBuilder(); + $extension = new SecretaryExtension(); + + $extension->load([ + [ + 'adapters' => [ + 'default' => [ + 'adapter' => 'Secretary\Adapter\AWS\SecretsManager\AWSSecretsManagerAdapter', + 'config' => [], + 'cache' => [ + 'enabled' => false, + ], + ], + ], + ], + ], $container); + + $definition = $container->getDefinition('secretary.env_var_processor'); + + $logger = $definition->getArgument(1); + $this->assertInstanceOf(Reference::class, $logger); + $this->assertEquals('logger', (string) $logger); + $this->assertEquals(ContainerInterface::IGNORE_ON_INVALID_REFERENCE, $logger->getInvalidBehavior()); + + $this->assertFalse($definition->getArgument(2)); + } + + public function testAllowMissingSecretsConfigIsPassedToEnvVarProcessor(): void + { + $container = new ContainerBuilder(); + $extension = new SecretaryExtension(); + + $extension->load([ + [ + 'allow_missing_secrets' => true, + 'adapters' => [ + 'default' => [ + 'adapter' => 'Secretary\Adapter\AWS\SecretsManager\AWSSecretsManagerAdapter', + 'config' => [], + 'cache' => [ + 'enabled' => false, + ], + ], + ], + ], + ], $container); + + $definition = $container->getDefinition('secretary.env_var_processor'); + $this->assertTrue($definition->getArgument(2)); + } } diff --git a/src/Bundle/SecretaryBundle/composer.json b/src/Bundle/SecretaryBundle/composer.json index d2745f7..2f4ddbb 100644 --- a/src/Bundle/SecretaryBundle/composer.json +++ b/src/Bundle/SecretaryBundle/composer.json @@ -20,6 +20,7 @@ "minimum-stability": "dev", "require": { "php": "^8.2", + "psr/log": "^1.0 || ^2.0 || ^3.0", "secretary/core": "self.version" }, "require-dev": { From 9609685b438a964fac85db4699e6d23a52f219e8 Mon Sep 17 00:00:00 2001 From: Thomas Date: Tue, 7 Jul 2026 06:07:20 -0500 Subject: [PATCH 2/2] fix(bundle): suppress psalm UndefinedInterfaceMethod on lowest symfony/config Co-Authored-By: Claude Fable 5 --- .../SecretaryBundle/DependencyInjection/Configuration.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Bundle/SecretaryBundle/DependencyInjection/Configuration.php b/src/Bundle/SecretaryBundle/DependencyInjection/Configuration.php index 18ea359..cad15e9 100644 --- a/src/Bundle/SecretaryBundle/DependencyInjection/Configuration.php +++ b/src/Bundle/SecretaryBundle/DependencyInjection/Configuration.php @@ -19,6 +19,9 @@ */ class Configuration implements ConfigurationInterface { + /** + * @psalm-suppress UndefinedInterfaceMethod + */ public function getConfigTreeBuilder(): TreeBuilder { $treeBuilder = new TreeBuilder('secretary', 'array');