Skip to content
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.

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

[framework-bundle] Prefix generated APP_SECRET by NotSecure- #1314

Closed
wants to merge 2 commits into from

Conversation

dunglas
Copy link
Member

@dunglas dunglas commented May 15, 2024

Q A
License MIT
Doc issue/PR n/a

While it's ok to commit the APP_SECRET used for dev and test environments, in production, the APP_SECRET environment variable must be overridden by a local value that is not committed (using a secret vault, using Symfony Secrets, creating a local environment variable, or creating an unversioned .env.prod file are acceptable options).

With this patch, Flex will prefix the generated value with NotSecure- to make it clear that the committed value must not be used in production.

Brought to our attention by @ramsey's https://phpc.social/@ramsey/112437517679689885.
See also symfony/symfony#38021.

We'll also have to document that APP_SECRET is used as the value for https://symfony.com/doc/current/reference/configuration/framework.html#secret

@symfony-recipes-bot symfony-recipes-bot enabled auto-merge (squash) May 15, 2024 08:38
Copy link

github-actions bot commented May 15, 2024

Thanks for the PR 😍

How to test these changes in your application

  1. Define the SYMFONY_ENDPOINT environment variable:

    # On Unix-like (BSD, Linux and macOS)
    export SYMFONY_ENDPOINT=https://raw.githubusercontent.com/symfony/recipes/flex/pull-1314/index.json
    # On Windows
    SET SYMFONY_ENDPOINT=https://raw.githubusercontent.com/symfony/recipes/flex/pull-1314/index.json
  2. Install the package(s) related to this recipe:

    composer req 'symfony/flex:^1.16'
    composer req 'symfony/framework-bundle:^7.0'
  3. Don't forget to unset the SYMFONY_ENDPOINT environment variable when done:

    # On Unix-like (BSD, Linux and macOS)
    unset SYMFONY_ENDPOINT
    # On Windows
    SET SYMFONY_ENDPOINT=

Diff between recipe versions

In order to help with the review stage, I'm in charge of computing the diff between the various versions of patched recipes.
I'm going keep this comment up to date with any updates of the attached patch.

symfony/framework-bundle

3.3 vs 3.4
diff --git a/symfony/framework-bundle/3.3/config/packages/framework.yaml b/symfony/framework-bundle/3.4/config/packages/framework.yaml
index d2b31bf..f532576 100644
--- a/symfony/framework-bundle/3.3/config/packages/framework.yaml
+++ b/symfony/framework-bundle/3.4/config/packages/framework.yaml
@@ -7,6 +7,7 @@ framework:
     # Remove or comment this section to explicitly disable session support.
     session:
         handler_id: null
+        cookie_samesite: lax
 
     #esi: true
     #fragments: true
3.4 vs 4.2
diff --git a/symfony/framework-bundle/3.4/config/bootstrap.php b/symfony/framework-bundle/4.2/config/bootstrap.php
index 2a47186..55560fb 100644
--- a/symfony/framework-bundle/3.4/config/bootstrap.php
+++ b/symfony/framework-bundle/4.2/config/bootstrap.php
@@ -13,38 +13,8 @@ if (!class_exists(Dotenv::class)) {
 if (is_array($env = @include dirname(__DIR__).'/.env.local.php') && (!isset($env['APP_ENV']) || ($_SERVER['APP_ENV'] ?? $_ENV['APP_ENV'] ?? $env['APP_ENV']) === $env['APP_ENV'])) {
     (new Dotenv(false))->populate($env);
 } else {
-    $path = dirname(__DIR__).'/.env';
-    $dotenv = new Dotenv(false);
-
     // load all the .env files
-    if (method_exists($dotenv, 'loadEnv')) {
-        $dotenv->loadEnv($path);
-    } else {
-        // fallback code in case your Dotenv component is not 4.2 or higher (when loadEnv() was added)
-
-        if (file_exists($path) || !file_exists($p = "$path.dist")) {
-            $dotenv->load($path);
-        } else {
-            $dotenv->load($p);
-        }
-
-        if (null === $env = $_SERVER['APP_ENV'] ?? $_ENV['APP_ENV'] ?? null) {
-            $dotenv->populate(array('APP_ENV' => $env = 'dev'));
-        }
-
-        if ('test' !== $env && file_exists($p = "$path.local")) {
-            $dotenv->load($p);
-            $env = $_SERVER['APP_ENV'] ?? $_ENV['APP_ENV'] ?? $env;
-        }
-
-        if (file_exists($p = "$path.$env")) {
-            $dotenv->load($p);
-        }
-
-        if (file_exists($p = "$path.$env.local")) {
-            $dotenv->load($p);
-        }
-    }
+    (new Dotenv(false))->loadEnv(dirname(__DIR__).'/.env');
 }
 
 $_SERVER += $_ENV;
diff --git a/symfony/framework-bundle/3.4/config/packages/framework.yaml b/symfony/framework-bundle/4.2/config/packages/framework.yaml
index f532576..cad7f78 100644
--- a/symfony/framework-bundle/3.4/config/packages/framework.yaml
+++ b/symfony/framework-bundle/4.2/config/packages/framework.yaml
@@ -1,3 +1,4 @@
+# see https://symfony.com/doc/current/reference/configuration/framework.html
 framework:
     secret: '%env(APP_SECRET)%'
     #csrf_protection: true
@@ -7,6 +8,7 @@ framework:
     # Remove or comment this section to explicitly disable session support.
     session:
         handler_id: null
+        cookie_secure: auto
         cookie_samesite: lax
 
     #esi: true
diff --git a/symfony/framework-bundle/3.4/config/services.yaml b/symfony/framework-bundle/4.2/config/services.yaml
index 07d653c..99d51bd 100644
--- a/symfony/framework-bundle/3.4/config/services.yaml
+++ b/symfony/framework-bundle/4.2/config/services.yaml
@@ -10,15 +10,12 @@ services:
     _defaults:
         autowire: true      # Automatically injects dependencies in your services.
         autoconfigure: true # Automatically registers your services as commands, event subscribers, etc.
-        public: false       # Allows optimizing the container by removing unused services; this also means
-                            # fetching services directly from the container via $container->get() won't work.
-                            # The best practice is to be explicit about your dependencies anyway.
 
     # makes classes in src/ available to be used as services
     # this creates a service per class whose id is the fully-qualified class name
     App\:
         resource: '../src/*'
-        exclude: '../src/{DependencyInjection,Entity,Migrations,Tests,Kernel.php}'
+        exclude: '../src/{DependencyInjection,Entity,Kernel.php}'
 
     # controllers are imported separately to make sure services can be injected
     # as action arguments even if you don't extend any base controller class
diff --git a/symfony/framework-bundle/3.4/manifest.json b/symfony/framework-bundle/4.2/manifest.json
index aa0150e..101b2aa 100644
--- a/symfony/framework-bundle/3.4/manifest.json
+++ b/symfony/framework-bundle/4.2/manifest.json
@@ -14,13 +14,14 @@
     "env": {
         "APP_ENV": "dev",
         "APP_SECRET": "%generate(secret)%",
-        "#TRUSTED_PROXIES": "127.0.0.1,127.0.0.2",
+        "#TRUSTED_PROXIES": "127.0.0.0/8,10.0.0.0/8,172.16.0.0/12,192.168.0.0/16",
         "#TRUSTED_HOSTS": "'^(localhost|example\\.com)$'"
     },
     "gitignore": [
         "/.env.local",
         "/.env.local.php",
         "/.env.*.local",
+        "/%CONFIG_DIR%/secrets/prod/prod.decrypt.private.php",
         "/%PUBLIC_DIR%/bundles/",
         "/%VAR_DIR%/",
         "/vendor/"
diff --git a/symfony/framework-bundle/3.4/post-install.txt b/symfony/framework-bundle/4.2/post-install.txt
index 944aa06..12f3669 100644
--- a/symfony/framework-bundle/3.4/post-install.txt
+++ b/symfony/framework-bundle/4.2/post-install.txt
@@ -1,7 +1,6 @@
   * Run your application:
     1. Go to the project directory
     2. Create your code repository with the git init command
-    3. Download the Symfony CLI at https://symfony.com/download to install a development web server,
-       or run composer require server --dev for a minimalist one
+    3. Download the Symfony CLI at https://symfony.com/download to install a development web server
 
   * Read the documentation at https://symfony.com/doc
diff --git a/symfony/framework-bundle/3.4/src/Kernel.php b/symfony/framework-bundle/4.2/src/Kernel.php
index 68b7a56..1cd0572 100644
--- a/symfony/framework-bundle/3.4/src/Kernel.php
+++ b/symfony/framework-bundle/4.2/src/Kernel.php
@@ -13,19 +13,9 @@ class Kernel extends BaseKernel
 {
     use MicroKernelTrait;
 
-    const CONFIG_EXTS = '.{php,xml,yaml,yml}';
+    private const CONFIG_EXTS = '.{php,xml,yaml,yml}';
 
-    public function getCacheDir()
-    {
-        return $this->getProjectDir().'/var/cache/'.$this->environment;
-    }
-
-    public function getLogDir()
-    {
-        return $this->getProjectDir().'/var/log';
-    }
-
-    public function registerBundles()
+    public function registerBundles(): iterable
     {
         $contents = require $this->getProjectDir().'/config/bundles.php';
         foreach ($contents as $class => $envs) {
@@ -35,13 +25,16 @@ class Kernel extends BaseKernel
         }
     }
 
-    protected function configureContainer(ContainerBuilder $container, LoaderInterface $loader)
+    public function getProjectDir(): string
+    {
+        return \dirname(__DIR__);
+    }
+
+    protected function configureContainer(ContainerBuilder $container, LoaderInterface $loader): void
     {
         $container->addResource(new FileResource($this->getProjectDir().'/config/bundles.php'));
-        // Feel free to remove the "container.autowiring.strict_mode" parameter
-        // if you are using symfony/dependency-injection 4.0+ as it's the default behavior
-        $container->setParameter('container.autowiring.strict_mode', true);
-        $container->setParameter('container.dumper.inline_class_loader', true);
+        $container->setParameter('container.dumper.inline_class_loader', \PHP_VERSION_ID < 70400 || $this->debug);
+        $container->setParameter('container.dumper.inline_factories', true);
         $confDir = $this->getProjectDir().'/config';
 
         $loader->load($confDir.'/{packages}/*'.self::CONFIG_EXTS, 'glob');
@@ -50,7 +43,7 @@ class Kernel extends BaseKernel
         $loader->load($confDir.'/{services}_'.$this->environment.self::CONFIG_EXTS, 'glob');
     }
 
-    protected function configureRoutes(RouteCollectionBuilder $routes)
+    protected function configureRoutes(RouteCollectionBuilder $routes): void
     {
         $confDir = $this->getProjectDir().'/config';
 
4.2 vs 4.4
diff --git a/symfony/framework-bundle/4.4/config/preload.php b/symfony/framework-bundle/4.4/config/preload.php
new file mode 100644
index 0000000..064bdcd
--- /dev/null
+++ b/symfony/framework-bundle/4.4/config/preload.php
@@ -0,0 +1,9 @@
+<?php
+
+if (file_exists(dirname(__DIR__).'/var/cache/prod/srcApp_KernelProdContainer.preload.php')) {
+    require dirname(__DIR__).'/var/cache/prod/srcApp_KernelProdContainer.preload.php';
+}
+
+if (file_exists(dirname(__DIR__).'/var/cache/prod/App_KernelProdContainer.preload.php')) {
+    require dirname(__DIR__).'/var/cache/prod/App_KernelProdContainer.preload.php';
+}
diff --git a/symfony/framework-bundle/4.4/config/routes/dev/framework.yaml b/symfony/framework-bundle/4.4/config/routes/dev/framework.yaml
new file mode 100644
index 0000000..bcbbf13
--- /dev/null
+++ b/symfony/framework-bundle/4.4/config/routes/dev/framework.yaml
@@ -0,0 +1,3 @@
+_errors:
+    resource: '@FrameworkBundle/Resources/config/routing/errors.xml'
+    prefix: /_error
diff --git a/symfony/framework-bundle/4.2/config/services.yaml b/symfony/framework-bundle/4.4/config/services.yaml
index 99d51bd..9557caa 100644
--- a/symfony/framework-bundle/4.2/config/services.yaml
+++ b/symfony/framework-bundle/4.4/config/services.yaml
@@ -14,13 +14,16 @@ services:
     # makes classes in src/ available to be used as services
     # this creates a service per class whose id is the fully-qualified class name
     App\:
-        resource: '../src/*'
-        exclude: '../src/{DependencyInjection,Entity,Kernel.php}'
+        resource: '../src/'
+        exclude:
+            - '../src/DependencyInjection/'
+            - '../src/Entity/'
+            - '../src/Kernel.php'
 
     # controllers are imported separately to make sure services can be injected
     # as action arguments even if you don't extend any base controller class
     App\Controller\:
-        resource: '../src/Controller'
+        resource: '../src/Controller/'
         tags: ['controller.service_arguments']
 
     # add more service definitions when explicit configuration is needed
diff --git a/symfony/framework-bundle/4.2/public/index.php b/symfony/framework-bundle/4.4/public/index.php
index 929197c..d0b6e02 100644
--- a/symfony/framework-bundle/4.2/public/index.php
+++ b/symfony/framework-bundle/4.4/public/index.php
@@ -1,7 +1,7 @@
 <?php
 
 use App\Kernel;
-use Symfony\Component\Debug\Debug;
+use Symfony\Component\ErrorHandler\Debug;
 use Symfony\Component\HttpFoundation\Request;
 
 require dirname(__DIR__).'/config/bootstrap.php';
4.4 vs 5.1
diff --git a/symfony/framework-bundle/4.4/config/bootstrap.php b/symfony/framework-bundle/4.4/config/bootstrap.php
deleted file mode 100644
index 55560fb..0000000
--- a/symfony/framework-bundle/4.4/config/bootstrap.php
+++ /dev/null
@@ -1,23 +0,0 @@
-<?php
-
-use Symfony\Component\Dotenv\Dotenv;
-
-require dirname(__DIR__).'/vendor/autoload.php';
-
-if (!class_exists(Dotenv::class)) {
-    throw new LogicException('Please run "composer require symfony/dotenv" to load the ".env" files configuring the application.');
-}
-
-// Load cached env vars if the .env.local.php file exists
-// Run "composer dump-env prod" to create it (requires symfony/flex >=1.2)
-if (is_array($env = @include dirname(__DIR__).'/.env.local.php') && (!isset($env['APP_ENV']) || ($_SERVER['APP_ENV'] ?? $_ENV['APP_ENV'] ?? $env['APP_ENV']) === $env['APP_ENV'])) {
-    (new Dotenv(false))->populate($env);
-} else {
-    // load all the .env files
-    (new Dotenv(false))->loadEnv(dirname(__DIR__).'/.env');
-}
-
-$_SERVER += $_ENV;
-$_SERVER['APP_ENV'] = $_ENV['APP_ENV'] = ($_SERVER['APP_ENV'] ?? $_ENV['APP_ENV'] ?? null) ?: 'dev';
-$_SERVER['APP_DEBUG'] = $_SERVER['APP_DEBUG'] ?? $_ENV['APP_DEBUG'] ?? 'prod' !== $_SERVER['APP_ENV'];
-$_SERVER['APP_DEBUG'] = $_ENV['APP_DEBUG'] = (int) $_SERVER['APP_DEBUG'] || filter_var($_SERVER['APP_DEBUG'], FILTER_VALIDATE_BOOLEAN) ? '1' : '0';
diff --git a/symfony/framework-bundle/4.4/config/preload.php b/symfony/framework-bundle/5.1/config/preload.php
index 064bdcd..5ebcdb2 100644
--- a/symfony/framework-bundle/4.4/config/preload.php
+++ b/symfony/framework-bundle/5.1/config/preload.php
@@ -1,9 +1,5 @@
 <?php
 
-if (file_exists(dirname(__DIR__).'/var/cache/prod/srcApp_KernelProdContainer.preload.php')) {
-    require dirname(__DIR__).'/var/cache/prod/srcApp_KernelProdContainer.preload.php';
-}
-
 if (file_exists(dirname(__DIR__).'/var/cache/prod/App_KernelProdContainer.preload.php')) {
     require dirname(__DIR__).'/var/cache/prod/App_KernelProdContainer.preload.php';
 }
diff --git a/symfony/framework-bundle/4.4/public/index.php b/symfony/framework-bundle/5.1/public/index.php
index d0b6e02..097baa3 100644
--- a/symfony/framework-bundle/4.4/public/index.php
+++ b/symfony/framework-bundle/5.1/public/index.php
@@ -1,10 +1,13 @@
 <?php
 
 use App\Kernel;
+use Symfony\Component\Dotenv\Dotenv;
 use Symfony\Component\ErrorHandler\Debug;
 use Symfony\Component\HttpFoundation\Request;
 
-require dirname(__DIR__).'/config/bootstrap.php';
+require dirname(__DIR__).'/vendor/autoload.php';
+
+(new Dotenv())->bootEnv(dirname(__DIR__).'/.env');
 
 if ($_SERVER['APP_DEBUG']) {
     umask(0000);
diff --git a/symfony/framework-bundle/4.4/src/Kernel.php b/symfony/framework-bundle/5.1/src/Kernel.php
index 1cd0572..655e796 100644
--- a/symfony/framework-bundle/4.4/src/Kernel.php
+++ b/symfony/framework-bundle/5.1/src/Kernel.php
@@ -3,52 +3,36 @@
 namespace App;
 
 use Symfony\Bundle\FrameworkBundle\Kernel\MicroKernelTrait;
-use Symfony\Component\Config\Loader\LoaderInterface;
-use Symfony\Component\Config\Resource\FileResource;
-use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
 use Symfony\Component\HttpKernel\Kernel as BaseKernel;
-use Symfony\Component\Routing\RouteCollectionBuilder;
+use Symfony\Component\Routing\Loader\Configurator\RoutingConfigurator;
 
 class Kernel extends BaseKernel
 {
     use MicroKernelTrait;
 
-    private const CONFIG_EXTS = '.{php,xml,yaml,yml}';
-
-    public function registerBundles(): iterable
+    protected function configureContainer(ContainerConfigurator $container): void
     {
-        $contents = require $this->getProjectDir().'/config/bundles.php';
-        foreach ($contents as $class => $envs) {
-            if ($envs[$this->environment] ?? $envs['all'] ?? false) {
-                yield new $class();
-            }
+        $container->import('../config/{packages}/*.yaml');
+        $container->import('../config/{packages}/'.$this->environment.'/*.yaml');
+
+        if (is_file(\dirname(__DIR__).'/config/services.yaml')) {
+            $container->import('../config/services.yaml');
+            $container->import('../config/{services}_'.$this->environment.'.yaml');
+        } elseif (is_file($path = \dirname(__DIR__).'/config/services.php')) {
+            (require $path)($container->withPath($path), $this);
         }
     }
 
-    public function getProjectDir(): string
+    protected function configureRoutes(RoutingConfigurator $routes): void
     {
-        return \dirname(__DIR__);
-    }
+        $routes->import('../config/{routes}/'.$this->environment.'/*.yaml');
+        $routes->import('../config/{routes}/*.yaml');
 
-    protected function configureContainer(ContainerBuilder $container, LoaderInterface $loader): void
-    {
-        $container->addResource(new FileResource($this->getProjectDir().'/config/bundles.php'));
-        $container->setParameter('container.dumper.inline_class_loader', \PHP_VERSION_ID < 70400 || $this->debug);
-        $container->setParameter('container.dumper.inline_factories', true);
-        $confDir = $this->getProjectDir().'/config';
-
-        $loader->load($confDir.'/{packages}/*'.self::CONFIG_EXTS, 'glob');
-        $loader->load($confDir.'/{packages}/'.$this->environment.'/*'.self::CONFIG_EXTS, 'glob');
-        $loader->load($confDir.'/{services}'.self::CONFIG_EXTS, 'glob');
-        $loader->load($confDir.'/{services}_'.$this->environment.self::CONFIG_EXTS, 'glob');
-    }
-
-    protected function configureRoutes(RouteCollectionBuilder $routes): void
-    {
-        $confDir = $this->getProjectDir().'/config';
-
-        $routes->import($confDir.'/{routes}/'.$this->environment.'/*'.self::CONFIG_EXTS, '/', 'glob');
-        $routes->import($confDir.'/{routes}/*'.self::CONFIG_EXTS, '/', 'glob');
-        $routes->import($confDir.'/{routes}'.self::CONFIG_EXTS, '/', 'glob');
+        if (is_file(\dirname(__DIR__).'/config/routes.yaml')) {
+            $routes->import('../config/routes.yaml');
+        } elseif (is_file($path = \dirname(__DIR__).'/config/routes.php')) {
+            (require $path)($routes->withPath($path), $this);
+        }
     }
 }
5.1 vs 5.2
diff --git a/symfony/framework-bundle/5.1/manifest.json b/symfony/framework-bundle/5.2/manifest.json
index 101b2aa..17fa50a 100644
--- a/symfony/framework-bundle/5.1/manifest.json
+++ b/symfony/framework-bundle/5.2/manifest.json
@@ -13,9 +13,7 @@
     },
     "env": {
         "APP_ENV": "dev",
-        "APP_SECRET": "%generate(secret)%",
-        "#TRUSTED_PROXIES": "127.0.0.0/8,10.0.0.0/8,172.16.0.0/12,192.168.0.0/16",
-        "#TRUSTED_HOSTS": "'^(localhost|example\\.com)$'"
+        "APP_SECRET": "%generate(secret)%"
     },
     "gitignore": [
         "/.env.local",
diff --git a/symfony/framework-bundle/5.1/public/index.php b/symfony/framework-bundle/5.2/public/index.php
index 097baa3..3bcee0b 100644
--- a/symfony/framework-bundle/5.1/public/index.php
+++ b/symfony/framework-bundle/5.2/public/index.php
@@ -15,14 +15,6 @@ if ($_SERVER['APP_DEBUG']) {
     Debug::enable();
 }
 
-if ($trustedProxies = $_SERVER['TRUSTED_PROXIES'] ?? false) {
-    Request::setTrustedProxies(explode(',', $trustedProxies), Request::HEADER_X_FORWARDED_FOR | Request::HEADER_X_FORWARDED_PORT | Request::HEADER_X_FORWARDED_PROTO);
-}
-
-if ($trustedHosts = $_SERVER['TRUSTED_HOSTS'] ?? false) {
-    Request::setTrustedHosts([$trustedHosts]);
-}
-
 $kernel = new Kernel($_SERVER['APP_ENV'], (bool) $_SERVER['APP_DEBUG']);
 $request = Request::createFromGlobals();
 $response = $kernel->handle($request);
5.2 vs 5.3
diff --git a/symfony/framework-bundle/5.2/config/packages/framework.yaml b/symfony/framework-bundle/5.3/config/packages/framework.yaml
index cad7f78..7853e9e 100644
--- a/symfony/framework-bundle/5.2/config/packages/framework.yaml
+++ b/symfony/framework-bundle/5.3/config/packages/framework.yaml
@@ -2,7 +2,7 @@
 framework:
     secret: '%env(APP_SECRET)%'
     #csrf_protection: true
-    #http_method_override: true
+    http_method_override: false
 
     # Enables session support. Note that the session will ONLY be started if you read or write from it.
     # Remove or comment this section to explicitly disable session support.
@@ -10,8 +10,15 @@ framework:
         handler_id: null
         cookie_secure: auto
         cookie_samesite: lax
+        storage_factory_id: session.storage.factory.native
 
     #esi: true
     #fragments: true
     php_errors:
         log: true
+
+when@test:
+    framework:
+        test: true
+        session:
+            storage_factory_id: session.storage.factory.mock_file
diff --git a/symfony/framework-bundle/5.2/config/packages/test/framework.yaml b/symfony/framework-bundle/5.2/config/packages/test/framework.yaml
deleted file mode 100644
index d051c84..0000000
--- a/symfony/framework-bundle/5.2/config/packages/test/framework.yaml
+++ /dev/null
@@ -1,4 +0,0 @@
-framework:
-    test: true
-    session:
-        storage_id: session.storage.mock_file
diff --git a/symfony/framework-bundle/5.2/config/routes/dev/framework.yaml b/symfony/framework-bundle/5.2/config/routes/dev/framework.yaml
deleted file mode 100644
index bcbbf13..0000000
--- a/symfony/framework-bundle/5.2/config/routes/dev/framework.yaml
+++ /dev/null
@@ -1,3 +0,0 @@
-_errors:
-    resource: '@FrameworkBundle/Resources/config/routing/errors.xml'
-    prefix: /_error
diff --git a/symfony/framework-bundle/5.3/config/routes/framework.yaml b/symfony/framework-bundle/5.3/config/routes/framework.yaml
new file mode 100644
index 0000000..0fc74bb
--- /dev/null
+++ b/symfony/framework-bundle/5.3/config/routes/framework.yaml
@@ -0,0 +1,4 @@
+when@dev:
+    _errors:
+        resource: '@FrameworkBundle/Resources/config/routing/errors.xml'
+        prefix: /_error
diff --git a/symfony/framework-bundle/5.2/config/services.yaml b/symfony/framework-bundle/5.3/config/services.yaml
index 9557caa..2d6a76f 100644
--- a/symfony/framework-bundle/5.2/config/services.yaml
+++ b/symfony/framework-bundle/5.3/config/services.yaml
@@ -2,7 +2,7 @@
 # Files in the packages/ subdirectory configure your dependencies.
 
 # Put parameters here that don't need to change on each machine where the app is deployed
-# https://symfony.com/doc/current/best_practices/configuration.html#application-related-configuration
+# https://symfony.com/doc/current/best_practices.html#use-parameters-for-application-configuration
 parameters:
 
 services:
@@ -20,11 +20,5 @@ services:
             - '../src/Entity/'
             - '../src/Kernel.php'
 
-    # controllers are imported separately to make sure services can be injected
-    # as action arguments even if you don't extend any base controller class
-    App\Controller\:
-        resource: '../src/Controller/'
-        tags: ['controller.service_arguments']
-
     # add more service definitions when explicit configuration is needed
     # please note that last definitions always *replace* previous ones
diff --git a/symfony/framework-bundle/5.2/public/index.php b/symfony/framework-bundle/5.3/public/index.php
index 3bcee0b..9982c21 100644
--- a/symfony/framework-bundle/5.2/public/index.php
+++ b/symfony/framework-bundle/5.3/public/index.php
@@ -1,22 +1,9 @@
 <?php
 
 use App\Kernel;
-use Symfony\Component\Dotenv\Dotenv;
-use Symfony\Component\ErrorHandler\Debug;
-use Symfony\Component\HttpFoundation\Request;
 
-require dirname(__DIR__).'/vendor/autoload.php';
+require_once dirname(__DIR__).'/vendor/autoload_runtime.php';
 
-(new Dotenv())->bootEnv(dirname(__DIR__).'/.env');
-
-if ($_SERVER['APP_DEBUG']) {
-    umask(0000);
-
-    Debug::enable();
-}
-
-$kernel = new Kernel($_SERVER['APP_ENV'], (bool) $_SERVER['APP_DEBUG']);
-$request = Request::createFromGlobals();
-$response = $kernel->handle($request);
-$response->send();
-$kernel->terminate($request, $response);
+return function (array $context) {
+    return new Kernel($context['APP_ENV'], (bool) $context['APP_DEBUG']);
+};
diff --git a/symfony/framework-bundle/5.2/src/Kernel.php b/symfony/framework-bundle/5.3/src/Kernel.php
index 655e796..8e96873 100644
--- a/symfony/framework-bundle/5.2/src/Kernel.php
+++ b/symfony/framework-bundle/5.3/src/Kernel.php
@@ -19,8 +19,8 @@ class Kernel extends BaseKernel
         if (is_file(\dirname(__DIR__).'/config/services.yaml')) {
             $container->import('../config/services.yaml');
             $container->import('../config/{services}_'.$this->environment.'.yaml');
-        } elseif (is_file($path = \dirname(__DIR__).'/config/services.php')) {
-            (require $path)($container->withPath($path), $this);
+        } else {
+            $container->import('../config/{services}.php');
         }
     }
 
@@ -31,8 +31,8 @@ class Kernel extends BaseKernel
 
         if (is_file(\dirname(__DIR__).'/config/routes.yaml')) {
             $routes->import('../config/routes.yaml');
-        } elseif (is_file($path = \dirname(__DIR__).'/config/routes.php')) {
-            (require $path)($routes->withPath($path), $this);
+        } else {
+            $routes->import('../config/{routes}.php');
         }
     }
 }
5.3 vs 5.4
diff --git a/symfony/framework-bundle/5.3/src/Kernel.php b/symfony/framework-bundle/5.4/src/Kernel.php
index 8e96873..779cd1f 100644
--- a/symfony/framework-bundle/5.3/src/Kernel.php
+++ b/symfony/framework-bundle/5.4/src/Kernel.php
@@ -3,36 +3,9 @@
 namespace App;
 
 use Symfony\Bundle\FrameworkBundle\Kernel\MicroKernelTrait;
-use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
 use Symfony\Component\HttpKernel\Kernel as BaseKernel;
-use Symfony\Component\Routing\Loader\Configurator\RoutingConfigurator;
 
 class Kernel extends BaseKernel
 {
     use MicroKernelTrait;
-
-    protected function configureContainer(ContainerConfigurator $container): void
-    {
-        $container->import('../config/{packages}/*.yaml');
-        $container->import('../config/{packages}/'.$this->environment.'/*.yaml');
-
-        if (is_file(\dirname(__DIR__).'/config/services.yaml')) {
-            $container->import('../config/services.yaml');
-            $container->import('../config/{services}_'.$this->environment.'.yaml');
-        } else {
-            $container->import('../config/{services}.php');
-        }
-    }
-
-    protected function configureRoutes(RoutingConfigurator $routes): void
-    {
-        $routes->import('../config/{routes}/'.$this->environment.'/*.yaml');
-        $routes->import('../config/{routes}/*.yaml');
-
-        if (is_file(\dirname(__DIR__).'/config/routes.yaml')) {
-            $routes->import('../config/routes.yaml');
-        } else {
-            $routes->import('../config/{routes}.php');
-        }
-    }
 }
5.4 vs 6.2
diff --git a/symfony/framework-bundle/5.4/config/packages/framework.yaml b/symfony/framework-bundle/6.2/config/packages/framework.yaml
index 7853e9e..6d85c29 100644
--- a/symfony/framework-bundle/5.4/config/packages/framework.yaml
+++ b/symfony/framework-bundle/6.2/config/packages/framework.yaml
@@ -3,6 +3,7 @@ framework:
     secret: '%env(APP_SECRET)%'
     #csrf_protection: true
     http_method_override: false
+    handle_all_throwables: true
 
     # Enables session support. Note that the session will ONLY be started if you read or write from it.
     # Remove or comment this section to explicitly disable session support.
6.2 vs 6.4
diff --git a/symfony/framework-bundle/6.2/config/packages/framework.yaml b/symfony/framework-bundle/6.4/config/packages/framework.yaml
index 6d85c29..980ee45 100644
--- a/symfony/framework-bundle/6.2/config/packages/framework.yaml
+++ b/symfony/framework-bundle/6.4/config/packages/framework.yaml
@@ -2,6 +2,7 @@
 framework:
     secret: '%env(APP_SECRET)%'
     #csrf_protection: true
+    annotations: false
     http_method_override: false
     handle_all_throwables: true
 
@@ -11,7 +12,6 @@ framework:
         handler_id: null
         cookie_secure: auto
         cookie_samesite: lax
-        storage_factory_id: session.storage.factory.native
 
     #esi: true
     #fragments: true
diff --git a/symfony/framework-bundle/6.2/manifest.json b/symfony/framework-bundle/6.4/manifest.json
index 17fa50a..92f0c2d 100644
--- a/symfony/framework-bundle/6.2/manifest.json
+++ b/symfony/framework-bundle/6.4/manifest.json
@@ -13,7 +13,7 @@
     },
     "env": {
         "APP_ENV": "dev",
-        "APP_SECRET": "%generate(secret)%"
+        "APP_SECRET": "NotSecure-%generate(secret)%"
     },
     "gitignore": [
         "/.env.local",
6.4 vs 7.0
diff --git a/symfony/framework-bundle/6.4/config/packages/framework.yaml b/symfony/framework-bundle/7.0/config/packages/framework.yaml
index 980ee45..877eb25 100644
--- a/symfony/framework-bundle/6.4/config/packages/framework.yaml
+++ b/symfony/framework-bundle/7.0/config/packages/framework.yaml
@@ -2,21 +2,12 @@
 framework:
     secret: '%env(APP_SECRET)%'
     #csrf_protection: true
-    annotations: false
-    http_method_override: false
-    handle_all_throwables: true
 
-    # Enables session support. Note that the session will ONLY be started if you read or write from it.
-    # Remove or comment this section to explicitly disable session support.
-    session:
-        handler_id: null
-        cookie_secure: auto
-        cookie_samesite: lax
+    # Note that the session will be started ONLY if you read or write from it.
+    session: true
 
     #esi: true
     #fragments: true
-    php_errors:
-        log: true
 
 when@test:
     framework:

@javiereguiluz
Copy link
Member

Thanks Kévin, but I'm not sure this is the best solution for the issue mentioned:

  • Now, a new Symfony project will show NotSecure-... as the value of the secret; some people will think that Symfony is not safe by default;
  • Others will just remove the NotSecure- prefix or will replace this value entirely but will keep committing it to the repo.

In my opinion, we can't properly solve this without a good comment below APP_SECRET explaining the situation and giving clear indications about what to do.

@dunglas
Copy link
Member Author

dunglas commented May 15, 2024

@javiereguiluz by definition, .env contains placeholder environment variables that serve as examples of how to configure the app on production. To me, having a NotSecure- prefix in one of these values doesn't say that Symfony isn't secure but on the contrary, that Symfony takes security seriously and makes it crystal clear that this value must not be used as-is in production.

API Platform has always used a similar placeholder for APP_SECRET (which, IMHO, is even more clear), and as far as I know, nobody says that API Platform isn't secure: https://github.com/api-platform/api-platform/blob/main/api/.env#L23

@wouterj
Copy link
Member

wouterj commented May 15, 2024

I agree with @dunglas.

Besides, I think whatever we do in the env file, we must also document the secret in the getting started docs. I'll propose something tonight/tomorrow!

@nicolas-grekas
Copy link
Member

nicolas-grekas commented May 15, 2024

API Platform has always used a similar placeholder for APP_SECRET (which, IMHO, is even more clear), and as far as I know, nobody says that API Platform isn't secure

Then I'm going to be the one saying this is highly insecure. There are scanners out there that do test for vulnerable Symfony apps that didn't change their APP_SECRET and that's a successful approach to hack many webapps apparently. "not on us" is not a satisfactory answer IMHO. This is related to symfony/symfony#52471

I'm 👎 with this PR because I think we should do better on the topic.

Here are some thoughts:

Ideally, we would remove this APP_SECRET entirely for skeleton apps. And for apps that do need it, we already have another stable secret we can use: SYMFONY_DECRYPTION_SECRET. It'd be great if we could derivate kernel.secret from that var when available.

I also had a look at services that do need this secret, here they are:

If we think we do need kernel.secret, then an idea could be to turn this into a service instead of a parameter. That service would have one responsibility: throw a nice and actionable error message when no secret is configured, with a __toString method to pass the secret to consumers when its set.

@dunglas
Copy link
Member Author

dunglas commented May 15, 2024

@nicolas-grekas I agree with your long-term approach. However, this patch has the benefit of improving the security and the DX right now, and can easily be reverted when your approach is implemented.

Big +1 for the service, so we could also improve the support for Docker and Kubernetes secrets, HashiCorp Vault, etc. by providing ad-hoc implementations.

Then I'm going to be the one saying this is highly insecure. There are scanners out there that do test for vulnerable Symfony apps that didn't change their APP_SECRET and that's a successful approach to hack many webapps apparently.

Most apps also need API tokens, SMTP passwords etc. We need to find a way to make it clear that values in .env are only placeholders that sometimes need to be changed in production.
Maybe could we do something like throwing if we detect at runtime that an env var suffixed by _SECRET has not been overrode?

@OskarStark
Copy link
Contributor

What about renaming .env to .env.placeholders or .env.template or .env.whatever ?

I like your idea @nicolas-grekas 👍🏻

@dunglas
Copy link
Member Author

dunglas commented May 15, 2024

@OskarStark We follow the same convention as most ecosystems (Angular, Ruby etc) and it's widely accepted that .env is the place for placeholders. Let's not bring our own convention while there is already a one that is popular.

@nicolas-grekas
Copy link
Member

nicolas-grekas commented May 16, 2024

Just to be clear: using this generated value in production is mostly fine.

Here, with the services I listed, committing the secret means anyone who had access to this token (aka people in the dev team) could fake a remember me token or a login link. This "Not-Secure" prefix is way more scary than needed. Saying "this is not secure" is FUD if the threat model is not clear.

To me, this is mostly secure, and we can and should document this threat model and how one should guard against it when they care.

Note that this threat model analysis makes me realize we don't have any way to rotate the secret, so that people leaving the company who had access to the secret couldn't get long term access to such capabilities. This is most concerning to me: not having a way to properly guard against a threat you want to alleviate.

@ebitkov
Copy link

ebitkov commented May 17, 2024

Note that this threat model analysis makes me realize we don't have any way to rotate the secret, so that people leaving the company who had access to the secret couldn't get long term access to such capabilities. This is most concerning to me: not having a way to properly guard against a threat you want to alleviate.

I noticed the same problem when facing this topic. These are the ways of generating a new APP_SECRET value I found so far:

  • Look up an online hex number generator and create a new value there.
  • Re-install all recipes by running rm symfony.lock; composer recipes:install --force. This generates a new value for the APP_SECRET variable, but also resets all your configuration to the default.
  • Create a new blank project and take the generated value from there
  • Use bin/console secrets:set APP_SECRET --random to generate a new random value in the vault. This generates a shorter value (16 chars) and not a hex number, but a random string (dunno, if that actually matters)

The last one is the most sensible, but for me as a developer, it's confusing, that secret:set --random creates a structurally different value then the one generated in the .env file (it's not a hex string and way shorter).

So yes, an easy way to generate a new value for APP_SECRET, that is well documented, would already help. Maybe extending secret:set --random would be the way to go, since secrets belong into the vault anyway.

fabpot added a commit to symfony/symfony that referenced this pull request May 21, 2024
…kernel.secret% with the rate-limiter (nicolas-grekas)

This PR was merged into the 7.1 branch.

Discussion
----------

[SecurityBundle] Use %container.build_hash% instead of %kernel.secret% with the rate-limiter

| Q             | A
| ------------- | ---
| Branch?       | 7.2
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Issues        | -
| License       | MIT

In #51434, we decided to hash the username and the client IP in order to anonymize logs.
I propose to use %container.build_hash% instead of %kernel.secret% in order to use %kernel.secret% one less time.
%container.build_hash% looks good enough to me, and it doesn't need any external configuration.
Related to the discussion happening in symfony/recipes#1314

Commits
-------

574f573 [SecurityBundle] Use %container.build_hash% instead of %kernel.secret% with the rate-limiter
@nicolas-grekas
Copy link
Member

Closing in favor of #1317 🤞
Thanks for shaking this topic :)

auto-merge was automatically disabled May 22, 2024 16:13

Pull request was closed

@nicolas-grekas nicolas-grekas deleted the dunglas-patch-1 branch May 22, 2024 16:13
fabpot added a commit to symfony/symfony that referenced this pull request May 25, 2024
…cryption secret when its env var is not defined (nicolas-grekas)

This PR was merged into the 7.2 branch.

Discussion
----------

[FrameworkBundle] Derivate `kernel.secret` from the decryption secret when its env var is not defined

| Q             | A
| ------------- | ---
| Branch?       | 7.2
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Issues        | #38021
| License       | MIT

I'm pursuing the goal of making `APP_SECRET` empty in the default recipe. See symfony/recipes#1314 for background.

At the moment, `kernel.secret` is used for remember-be, login-links and ESI. This means that when you start a project, you don't need it. But once you do enable those features, you'll get an "APP_SECRET env var not found" error message.

I think we can live with this error and the related DX. We need good doc of course.
Still, in order to make DX a bit smoother, I propose to derivate APP_SECRET from SYMFONY_DECRYPTION_SECRET when it's set.

This is what this PR does.

Of course, we should also document that creating a separate `APP_SECRET` is likely a good idea.
FTR, here is how one can trivially generate a value for APP_SECRET and put it in the vault, thus fixing #38021:

```sh
symfony console secrets:set APP_SECRET --random
```

Commits
-------

4749871 [FrameworkBundle] Derivate kernel.secret from the decryption secret when its env var is not defined
symfony-splitter pushed a commit to symfony/framework-bundle that referenced this pull request May 25, 2024
…cryption secret when its env var is not defined (nicolas-grekas)

This PR was merged into the 7.2 branch.

Discussion
----------

[FrameworkBundle] Derivate `kernel.secret` from the decryption secret when its env var is not defined

| Q             | A
| ------------- | ---
| Branch?       | 7.2
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Issues        | #38021
| License       | MIT

I'm pursuing the goal of making `APP_SECRET` empty in the default recipe. See symfony/recipes#1314 for background.

At the moment, `kernel.secret` is used for remember-be, login-links and ESI. This means that when you start a project, you don't need it. But once you do enable those features, you'll get an "APP_SECRET env var not found" error message.

I think we can live with this error and the related DX. We need good doc of course.
Still, in order to make DX a bit smoother, I propose to derivate APP_SECRET from SYMFONY_DECRYPTION_SECRET when it's set.

This is what this PR does.

Of course, we should also document that creating a separate `APP_SECRET` is likely a good idea.
FTR, here is how one can trivially generate a value for APP_SECRET and put it in the vault, thus fixing #38021:

```sh
symfony console secrets:set APP_SECRET --random
```

Commits
-------

4749871a29 [FrameworkBundle] Derivate kernel.secret from the decryption secret when its env var is not defined
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants