Repository: brefphp/bref (language: php)

Issue:
Make bref 2 even lighter by making `async-aws/ssm` optional
Move dep to suggest and adjust code to to do a class_exists check and crash out with good DX if someone tries to resolve an env variable via ssm without installing the dep (https://github.com/brefphp/bref/blob/2.0.0-beta11/src/Secrets/Secrets.php#L90).


Discussion / hints:

Fix patch:
diff --git a/composer.json b/composer.json
index d180be263..41dc5a8ba 100644
--- a/composer.json
+++ b/composer.json
@@ -29,13 +29,13 @@
         "riverline/multipart-parser": "^2.0.6",
         "psr/http-server-handler": "^1.0",
         "nyholm/psr7": "^1.2",
-        "psr/container": "^1.0|^2.0",
-        "async-aws/ssm": "^1.3"
+        "psr/container": "^1.0|^2.0"
     },
     "require-dev": {
         "aws/aws-sdk-php": "^3.172",
         "async-aws/core": "^1.0",
         "async-aws/lambda": "^1.0",
+        "bref/secrets-loader": "^1.0",
         "phpunit/phpunit": "^9.0",
         "dms/phpunit-arraysubset-asserts": "^0.2.0",
         "doctrine/coding-standard": "^8.0",
diff --git a/docs/environment/variables.md b/docs/environment/variables.md
index 35f4ebcab..19ed4d31a 100644
--- a/docs/environment/variables.md
+++ b/docs/environment/variables.md
@@ -88,9 +88,13 @@ Disadvantages:
 
 #### At runtime
 
-Alternatively, Bref can fetch the secret values at runtime when the Lambda function starts (aka the "cold start").
+Alternatively, Bref can fetch the secret values at runtime when the Lambda function starts (aka the "cold start"). To use that feature, we **must install** the `bref/secrets-loader` package:
 
-To do so, the environment variable should contain the path to the SSM parameter prefixed with `bref-ssm:`. We also need to authorize Lambda to retrieve the parameter. For example:
+```
+composer require bref/secrets-loader
+```
+
+To use it, the environment variable should contain the path to the SSM parameter prefixed with `bref-ssm:`. We also need to authorize Lambda to retrieve the parameter. For example:
 
 ```yaml
 provider:
diff --git a/index.js b/index.js
index 29e975f7a..6cffdeb20 100644
--- a/index.js
+++ b/index.js
@@ -3,6 +3,7 @@
 const {listLayers} = require('./plugin/layers');
 const {runConsole} = require('./plugin/run-console');
 const {runLocal} = require('./plugin/local');
+const {warnIfUsingSecretsWithoutTheBrefDependency} = require('./plugin/secrets');
 const fs = require('fs');
 const path = require('path');
 
@@ -20,6 +21,7 @@ class ServerlessPlugin {
         if (!utils) {
             throw new serverless.classes.Error('Bref requires Serverless Framework v3, but an older v2 version is running.\nPlease upgrade to Serverless Framework v3.');
         }
+        this.utils = utils;
 
         // Automatically enable faster deployments (unless a value is already set)
         // https://www.serverless.com/framework/docs/providers/aws/guide/deploying#deployment-method
@@ -110,6 +112,7 @@ class ServerlessPlugin {
         this.hooks = {
             'initialize': () => {
                 this.processPhpRuntimes();
+                warnIfUsingSecretsWithoutTheBrefDependency(this.serverless, utils.log);
                 try {
                     this.telemetry();
                 } catch (e) {
diff --git a/plugin/secrets.js b/plugin/secrets.js
new file mode 100644
index 000000000..b7eb408c5
--- /dev/null
+++ b/plugin/secrets.js
@@ -0,0 +1,35 @@
+const fs = require('fs');
+
+function warnIfUsingSecretsWithoutTheBrefDependency(serverless, log) {
+    const config = serverless.service;
+    const allVariables = [];
+
+    const check = ([name, value]) => {
+        if (typeof value === 'string' && value.startsWith('bref-ssm:')) {
+            allVariables.push(`${name}: ${value}`);
+        }
+    };
+    Object.entries(config.provider.environment || {}).forEach(check);
+    Object.values(config.functions).forEach(f => Object.entries(f.environment || {}).forEach(check));
+
+    if (allVariables.length > 0) {
+        // Check if the bref/secrets-loader dependency is installed in composer.json
+        if (! fs.existsSync('composer.json')) {
+            return;
+        }
+        const composerJson = JSON.parse(fs.readFileSync('composer.json', 'utf8'));
+        const dependencies = Object.keys(composerJson.require || {});
+        if (dependencies.includes('bref/secrets-loader')) {
+            return;
+        }
+
+        log.warning(`The following environment variables use the "bref-ssm:" prefix, but the "bref/secrets-loader" dependency is not installed via "composer.json".`);
+        allVariables.forEach(variable => log.warning(`    ${variable}`));
+        log.warning(`The "bref/secrets-loader" dependency is required to use the "bref-ssm:" prefix. Install it by running:`);
+        log.warning(`    composer require bref/secrets-loader`);
+        log.warning(`Learn more at https://bref.sh/docs/environment/variables.html#secrets`);
+        log.warning();
+    }
+}
+
+module.exports = {warnIfUsingSecretsWithoutTheBrefDependency};
diff --git a/src/ConsoleRuntime/Main.php b/src/ConsoleRuntime/Main.php
index 40ce8761f..d42435b55 100755
--- a/src/ConsoleRuntime/Main.php
+++ b/src/ConsoleRuntime/Main.php
@@ -4,8 +4,8 @@
 
 use Bref\Bref;
 use Bref\Context\Context;
+use Bref\LazySecretsLoader;
 use Bref\Runtime\LambdaRuntime;
-use Bref\Secrets\Secrets;
 use Exception;
 use Symfony\Component\Process\Process;
 
@@ -16,7 +16,7 @@ class Main
 {
     public static function run(): void
     {
-        Secrets::decryptSecretEnvironmentVariables();
+        LazySecretsLoader::loadSecretEnvironmentVariables();
 
         Bref::triggerHooks('beforeStartup');
 
diff --git a/src/FpmRuntime/Main.php b/src/FpmRuntime/Main.php
index 98bc5a06a..6c8aa842b 100755
--- a/src/FpmRuntime/Main.php
+++ b/src/FpmRuntime/Main.php
@@ -3,8 +3,8 @@
 namespace Bref\FpmRuntime;
 
 use Bref\Bref;
+use Bref\LazySecretsLoader;
 use Bref\Runtime\LambdaRuntime;
-use Bref\Secrets\Secrets;
 use Throwable;
 
 /**
@@ -18,7 +18,7 @@ public static function run(): void
         ini_set('display_errors', '1');
         error_reporting(E_ALL);
 
-        Secrets::decryptSecretEnvironmentVariables();
+        LazySecretsLoader::loadSecretEnvironmentVariables();
 
         Bref::triggerHooks('beforeStartup');
 
diff --git a/src/FunctionRuntime/Main.php b/src/FunctionRuntime/Main.php
index eaf7a1877..c4843eda4 100644
--- a/src/FunctionRuntime/Main.php
+++ b/src/FunctionRuntime/Main.php
@@ -3,8 +3,8 @@
 namespace Bref\FunctionRuntime;
 
 use Bref\Bref;
+use Bref\LazySecretsLoader;
 use Bref\Runtime\LambdaRuntime;
-use Bref\Secrets\Secrets;
 use Throwable;
 
 /**
@@ -14,7 +14,7 @@ class Main
 {
     public static function run(): void
     {
-        Secrets::decryptSecretEnvironmentVariables();
+        LazySecretsLoader::loadSecretEnvironmentVariables();
 
         Bref::triggerHooks('beforeStartup');
 
diff --git a/src/LazySecretsLoader.php b/src/LazySecretsLoader.php
new file mode 100644
index 000000000..cc3f2b42a
--- /dev/null
+++ b/src/LazySecretsLoader.php
@@ -0,0 +1,51 @@
+<?php declare(strict_types=1);
+
+namespace Bref;
+
+use Bref\Secrets\Secrets;
+use Exception;
+
+/**
+ * Checks environment variables for the "bref-ssm:xxx" syntax.
+ *
+ * If found, it ensures the "bref/secrets-loader" package is installed
+ * and then loads the secrets.
+ *
+ * That lets us make the "bref/secrets-loader" package an optional dependency.
+ *
+ * @internal
+ */
+final class LazySecretsLoader
+{
+    public static function loadSecretEnvironmentVariables(): void
+    {
+        if (! self::areThereSecretsToLoad()) {
+            return;
+        }
+
+        if (! class_exists(Secrets::class)) {
+            throw new Exception('The "bref/secrets-loader" package is required to load SSM parameters via the "bref-ssm:xxx" syntax in environment variables. Please add it to your "require" section in composer.json.');
+        }
+
+        Secrets::loadSecretEnvironmentVariables();
+    }
+
+    /**
+     * Checks if there are any environment variable that starts with "bref-ssm:".
+     */
+    private static function areThereSecretsToLoad(): bool
+    {
+        /** @var array<string,string>|string|false $envVars */
+        $envVars = getenv(local_only: true); // @phpstan-ignore-line PHPStan is wrong
+        if (! is_array($envVars)) {
+            return false;
+        }
+
+        // Only consider environment variables that start with "bref-ssm:"
+        $envVarsToDecrypt = array_filter($envVars, function (string $value): bool {
+            return str_starts_with($value, 'bref-ssm:');
+        });
+
+        return ! empty($envVarsToDecrypt);
+    }
+}
diff --git a/src/Secrets/Secrets.php b/src/Secrets/Secrets.php
deleted file mode 100644
index a9b4960cf..000000000
--- a/src/Secrets/Secrets.php
+++ /dev/null
@@ -1,130 +0,0 @@
-<?php declare(strict_types=1);
-
-namespace Bref\Secrets;
-
-use AsyncAws\Ssm\SsmClient;
-use Closure;
-use RuntimeException;
-
-class Secrets
-{
-    /**
-     * Decrypt environment variables that are encrypted with AWS SSM.
-     *
-     * @param SsmClient $ssmClient To allow mocking in tests.
-     */
-    public static function decryptSecretEnvironmentVariables(?SsmClient $ssmClient = null): void
-    {
-        /** @var array<string,string>|string|false $envVars */
-        $envVars = getenv(local_only: true); // @phpstan-ignore-line PHPStan is wrong
-        if (! is_array($envVars)) {
-            return;
-        }
-
-        // Only consider environment variables that start with "bref-ssm:"
-        $envVarsToDecrypt = array_filter($envVars, function (string $value): bool {
-            return str_starts_with($value, 'bref-ssm:');
-        });
-        if (empty($envVarsToDecrypt)) {
-            return;
-        }
-
-        // Extract the SSM parameter names by removing the "bref-ssm:" prefix
-        $ssmNames = array_map(function (string $value): string {
-            return substr($value, strlen('bref-ssm:'));
-        }, $envVarsToDecrypt);
-
-        $actuallyCalledSsm = false;
-        $parameters = self::readParametersFromCacheOr(function () use ($ssmClient, $ssmNames, &$actuallyCalledSsm) {
-            $actuallyCalledSsm = true;
-            return self::retrieveParametersfromSsm($ssmClient, array_values($ssmNames));
-        });
-
-        foreach ($parameters as $parameterName => $parameterValue) {
-            $envVar = array_search($parameterName, $ssmNames, true);
-            $_SERVER[$envVar] = $_ENV[$envVar] = $parameterValue;
-            putenv("$envVar=$parameterValue");
-        }
-
-        // Only log once (when the cache was empty) else it might spam the logs in the function runtime
-        // (where the process restarts on every invocation)
-        if ($actuallyCalledSsm) {
-            $stderr = fopen('php://stderr', 'ab');
-            fwrite($stderr, '[Bref] Loaded these environment variables from SSM: ' . implode(', ', array_keys($envVarsToDecrypt)) . PHP_EOL);
-        }
-    }
-
-    /**
-     * Cache the parameters in a temp file.
-     * Why? Because on the function runtime, the PHP process might
-     * restart on every invocation (or on error), so we don't want to
-     * call SSM every time.
-     *
-     * @param Closure(): array<string, string> $paramResolver
-     * @return array<string, string> Map of parameter name -> value
-     */
-    private static function readParametersFromCacheOr(Closure $paramResolver): array
-    {
-        // Check in cache first
-        $cacheFile = sys_get_temp_dir() . '/bref-ssm-parameters.php';
-        if (is_file($cacheFile)) {
-            $parameters = json_decode(file_get_contents($cacheFile), true);
-            if (is_array($parameters)) {
-                return $parameters;
-            }
-        }
-
-        // Not in cache yet: we resolve it
-        $parameters = $paramResolver();
-
-        // Using json_encode instead of var_export due to possible security issues
-        file_put_contents($cacheFile, json_encode($parameters));
-
-        return $parameters;
-    }
-
-    /**
-     * @param string[] $ssmNames
-     * @return array<string, string> Map of parameter name -> value
-     */
-    private static function retrieveParametersfromSsm(?SsmClient $ssmClient, array $ssmNames): array
-    {
-        $ssm = $ssmClient ?? new SsmClient([
-            'region' => $_ENV['AWS_REGION'] ?? $_ENV['AWS_DEFAULT_REGION'],
-        ]);
-
-        /** @var array<string, string> $parameters Map of parameter name -> value */
-        $parameters = [];
-        $parametersNotFound = [];
-
-        // The API only accepts up to 10 parameters at a time, so we batch the calls
-        foreach (array_chunk($ssmNames, 10) as $batchOfSsmNames) {
-            try {
-                $result = $ssm->getParameters([
-                    'Names' => $batchOfSsmNames,
-                    'WithDecryption' => true,
-                ]);
-                foreach ($result->getParameters() as $parameter) {
-                    $parameters[$parameter->getName()] = $parameter->getValue();
-                }
-            } catch (RuntimeException $e) {
-                if ($e->getCode() === 400) {
-                    // Extra descriptive error message for the most common error
-                    throw new RuntimeException(
-                        "Bref was not able to resolve secrets contained in environment variables from SSM because of a permissions issue with the SSM API. Did you add IAM permissions in serverless.yml to allow Lambda to access SSM? (docs: https://bref.sh/docs/environment/variables.html#at-deployment-time).\nFull exception message: {$e->getMessage()}",
-                        $e->getCode(),
-                        $e,
-                    );
-                }
-                throw $e;
-            }
-            $parametersNotFound = array_merge($parametersNotFound, $result->getInvalidParameters());
-        }
-
-        if (count($parametersNotFound) > 0) {
-            throw new RuntimeException('The following SSM parameters could not be found: ' . implode(', ', $parametersNotFound));
-        }
-
-        return $parameters;
-    }
-}


Test patch:
diff --git a/tests/Secrets/SecretsTest.php b/tests/Secrets/SecretsTest.php
deleted file mode 100644
index 333a570bf..000000000
--- a/tests/Secrets/SecretsTest.php
+++ /dev/null
@@ -1,93 +0,0 @@
-<?php declare(strict_types=1);
-
-namespace Bref\Test\Secrets;
-
-use AsyncAws\Core\Test\ResultMockFactory;
-use AsyncAws\Ssm\Result\GetParametersResult;
-use AsyncAws\Ssm\SsmClient;
-use AsyncAws\Ssm\ValueObject\Parameter;
-use Bref\Secrets\Secrets;
-use PHPUnit\Framework\TestCase;
-
-class SecretsTest extends TestCase
-{
-    public function setUp(): void
-    {
-        if (file_exists(sys_get_temp_dir() . '/bref-ssm-parameters.php')) {
-            unlink(sys_get_temp_dir() . '/bref-ssm-parameters.php');
-        }
-    }
-
-    public function test decrypts env variables(): void
-    {
-        putenv('SOME_VARIABLE=bref-ssm:/some/parameter');
-        putenv('SOME_OTHER_VARIABLE=helloworld');
-
-        // Sanity checks
-        $this->assertSame('bref-ssm:/some/parameter', getenv('SOME_VARIABLE'));
-        $this->assertSame('helloworld', getenv('SOME_OTHER_VARIABLE'));
-
-        Secrets::decryptSecretEnvironmentVariables($this->mockSsmClient());
-
-        $this->assertSame('foobar', getenv('SOME_VARIABLE'));
-        $this->assertSame('foobar', $_SERVER['SOME_VARIABLE']);
-        $this->assertSame('foobar', $_ENV['SOME_VARIABLE']);
-        // Check that the other variable was not modified
-        $this->assertSame('helloworld', getenv('SOME_OTHER_VARIABLE'));
-    }
-
-    public function test caches parameters to call SSM only once(): void
-    {
-        putenv('SOME_VARIABLE=bref-ssm:/some/parameter');
-
-        // Call twice, the mock will assert that SSM was only called once
-        $ssmClient = $this->mockSsmClient();
-        Secrets::decryptSecretEnvironmentVariables($ssmClient);
-        Secrets::decryptSecretEnvironmentVariables($ssmClient);
-
-        $this->assertSame('foobar', getenv('SOME_VARIABLE'));
-    }
-
-    public function test throws a clear error message on missing permissions(): void
-    {
-        putenv('SOME_VARIABLE=bref-ssm:/app/test');
-
-        $ssmClient = $this->getMockBuilder(SsmClient::class)
-            ->disableOriginalConstructor()
-            ->getMock();
-        $result = ResultMockFactory::createFailing(GetParametersResult::class, 400, 'User: arn:aws:sts::123456:assumed-role/app-dev-us-east-1-lambdaRole/app-dev-hello is not authorized to perform: ssm:GetParameters on resource: arn:aws:ssm:us-east-1:123456:parameter/app/test because no identity-based policy allows the ssm:GetParameters action');
-        $ssmClient->method('getParameters')
-            ->willReturn($result);
-
-        $expected = preg_quote("Bref was not able to resolve secrets contained in environment variables from SSM because of a permissions issue with the SSM API. Did you add IAM permissions in serverless.yml to allow Lambda to access SSM? (docs: https://bref.sh/docs/environment/variables.html#at-deployment-time).\nFull exception message:", '/');
-        $this->expectExceptionMessageMatches("/$expected .+/");
-        Secrets::decryptSecretEnvironmentVariables($ssmClient);
-    }
-
-    private function mockSsmClient(): SsmClient
-    {
-        $ssmClient = $this->getMockBuilder(SsmClient::class)
-            ->disableOriginalConstructor()
-            ->onlyMethods(['getParameters'])
-            ->getMock();
-
-        $result = ResultMockFactory::create(GetParametersResult::class, [
-            'Parameters' => [
-                new Parameter([
-                    'Name' => '/some/parameter',
-                    'Value' => 'foobar',
-                ]),
-            ],
-        ]);
-
-        $ssmClient->expects($this->once())
-            ->method('getParameters')
-            ->with([
-                'Names' => ['/some/parameter'],
-                'WithDecryption' => true,
-            ])
-            ->willReturn($result);
-
-        return $ssmClient;
-    }
-}