refac: runtime settings are read once at startup and passed in through the composition root; no handler, router or repository reads environment variables

features/runtime_settings_injection.feature

@@ -0,0 +1,120 @@
+Feature: Runtime settings are read once at startup
+
+  As a maintainer of RenewCMS
+  I want the environment, CORS origins, public URL, upload dir and SMTP
+  settings to be read from the process environment exactly once, in the
+  composition root, and handed down explicitly from there
+  So that no handler, router or repository reads an environment variable at
+    request or construction time, and changing the process environment after
+    startup can never change already-built behaviour
+
+  Background:
+    Given before this slice, os.Getenv is called outside cmd/server/main.go
+      in four places: api/router.go's enableLoggingIfDev reads ENVIRONMENT
+      on every router build, api/router.go's enableCorsIfConfigured reads
+      CORS_ALLOWED_ORIGINS on every router build,
+      api/handlers/auth.shouldCookieBeSecure reads ENVIRONMENT on every
+      Login/Register/Logout call, internal/infrastructure/mailer.MailRepository.Send
+      reads SMTP_EMAIL, SMTP_PASSWORD, SMTP_HOST and SMTP_PORT on every call,
+      and internal/infrastructure/persistence.ImageRepository.Create and
+      .Delete read UPLOAD_DIR on every call
+    And this slice introduces internal/infrastructure/config.Settings, a
+      plain struct with fields Environment, CorsAllowedOrigins ([]string),
+      PublicURL, UploadDir, JWTSecret, DBFile, Port and an Smtp sub-struct
+      (Email, Password, Host, Port)
+    And cmd/server/main.go's initEnvVariables still loads the .env file and
+      applies the same defaults and required-var checks as before, then
+      builds exactly one config.Settings value from the process environment,
+      before api.InitContainer or api.InitRoutes is ever called
+    And api.InitContainer and api.InitRoutes now take that config.Settings
+      value, and every downstream constructor receives its own slice of it
+      explicitly instead of reading os.Getenv: auth.NewHandler receives
+      whether the environment is production, mailer.NewMailRepository
+      receives the Smtp settings, persistence.NewImageRepository receives
+      the upload dir, and the router's logging and CORS setup receive the
+      environment string and the CORS origins list as parameters
+    And after this slice, os.Getenv (and os.LookupEnv) for ENVIRONMENT,
+      CORS_ALLOWED_ORIGINS, SMTP_EMAIL, SMTP_PASSWORD, SMTP_HOST, SMTP_PORT
+      and UPLOAD_DIR appears only inside cmd/server/main.go and
+      internal/infrastructure/config
+    And a component built from a config.Settings value keeps behaving
+      according to the values it was built with even if the process
+      environment changes afterwards, because it holds the captured values,
+      not the environment
+    And GET /healthz is mounted directly on the outer API router
+      (api/router.go), not under /v1, so the CORS middleware wraps it the
+      same way it wraps every route
+
+  Scenario Outline: The jwt cookie's Secure flag follows the injected environment, not the process environment
+    Given a config.Settings value with Environment "<injected environment>"
+    And the auth handler is built from that settings value
+    And the process environment variable ENVIRONMENT is set to "<process environment>"
+    When the client sends POST /v1/auth/login with valid credentials for an
+      existing, verified user
+    Then the response's Set-Cookie header for the "jwt" cookie has its
+      Secure attribute set to <secure expected>
+
+    Examples:
+      | injected environment | process environment | secure expected |
+      | production            | production           | true             |
+      | production            | development           | true             |
+      | development            | production           | false            |
+      | development            | development           | false            |
+
+  Scenario: CORS allows a request from an injected allowed origin
+    Given a config.Settings value with CorsAllowedOrigins
+      ["https://cms.example.com"]
+    And the API router is built from that settings value
+    When the client sends GET /healthz with header
+      Origin: https://cms.example.com
+    Then the response has header Access-Control-Allow-Origin:
+      https://cms.example.com
+
+  Scenario: CORS does not allow a request from an origin outside the injected list
+    Given a config.Settings value with CorsAllowedOrigins
+      ["https://cms.example.com"]
+    And the API router is built from that settings value
+    When the client sends GET /healthz with header
+      Origin: https://attacker.example.com
+    Then the response has no Access-Control-Allow-Origin header
+
+  Scenario: An empty injected origins list disables CORS entirely
+    Given a config.Settings value with CorsAllowedOrigins as an empty list
+    And the API router is built from that settings value
+    When the client sends GET /healthz with header
+      Origin: https://cms.example.com
+    Then the response has no Access-Control-Allow-Origin header
+
+  Scenario: An uploaded image is stored under the injected upload directory
+    Given a config.Settings value with UploadDir pointing at a fresh
+      directory created for this scenario
+    And the image repository is built from that settings value, over a
+      fresh in-memory database
+    And the process environment variable UPLOAD_DIR is unset or points at a
+      different directory
+    When internal/infrastructure/persistence.ImageRepository.Create is
+      called with a PNG file
+    Then the image file is written inside the injected upload directory,
+      not inside the process environment's UPLOAD_DIR
+    And the returned image's Path still starts with "/static/uploadedImages/"
+
+  Scenario: Changing the process environment after construction does not change already-built behaviour
+    Given a config.Settings value with Environment "production"
+    And the auth handler is built from that settings value
+    When the process environment variable ENVIRONMENT is then changed to
+      "development"
+    And the client sends POST /v1/auth/login with valid credentials for an
+      existing, verified user
+    Then the response's Set-Cookie header for the "jwt" cookie still has its
+      Secure attribute set to true
+
+  Scenario: The mailer is constructed from injected SMTP settings, not the environment
+    Given a config.Settings value with Smtp.Email "notify@example.com",
+      Smtp.Host "smtp.example.com" and Smtp.Port 587
+    And internal/infrastructure/mailer.NewMailRepository is called with that
+      Smtp settings value and no SMTP_* environment variable set
+    Then constructing the MailRepository does not read any SMTP_*
+      environment variable
+    And the maintainer verifies by manual test (see QA procedure) that an
+      actual send still uses those injected values, since a real SMTP
+      dialog is never exercised by the automated suite

features/runtime_settings_injection.qa.md

@@ -0,0 +1,113 @@
+# QA procedure: runtime_settings_injection
+
+Package: `test` (black-box).
+
+## Setup shared by every case
+1. Call `validator.InitValidator()` once.
+2. For the secure-cookie and upload-directory cases, build only the pieces
+   under test directly (no real SQLite file), mirroring
+   `test/article_publishing_http_test.go` and siblings.
+3. For the CORS cases, build the **real** router construction path — see
+   "CORS" section below. A test-local reimplementation of the CORS wiring
+   (a helper that copies `enableCorsIfConfigured`'s logic instead of calling
+   it) is not acceptable: the point of these scenarios is to prove the
+   actual `api/router.go` wiring, driven by injected settings, behaves
+   correctly, not that some parallel logic does.
+4. Never set `t.Setenv` before constructing a component under test in a way
+   that the component is expected to read; the whole point of this suite is
+   that env is captured once elsewhere. Where a scenario needs to prove
+   the environment has no effect, set it with `t.Setenv` explicitly *after*
+   construction, and say so in the test's own scenario name (not as a code
+   comment).
+
+## Secure cookie flag (Scenario Outline + regression scenario)
+5. Build a `mockUserRepo` (reuse `test/user_test.go`'s fake) seeded with one
+   verified user and a known bcrypt hash via `hashing.NewBcryptHasher()`.
+6. Build `auth.NewHandler(token, getUseCase, listUseCase, createUseCase,
+   authenticateUseCase, <injected production flag>)` — this slice adds a
+   constructor parameter carrying the environment/production flag in place
+   of `shouldCookieBeSecure`'s `os.Getenv` call. Mount only `POST /login` on
+   a fresh `chi.NewRouter()`.
+7. For each row of the outline: optionally `t.Setenv("ENVIRONMENT", ...)`
+   with the process value from the table (this must have zero effect), then
+   send `POST /login` with valid JSON credentials for the seeded user via
+   `httptest.NewRequest` + `httptest.NewRecorder()`. Read the `Set-Cookie`
+   response header value with `rec.Result().Header.Values("Set-Cookie")`,
+   parse it with `http.ParseSetCookie(...)`, and assert `.Secure` equals the
+   table's expected value.
+8. For the "changing the process environment after construction" scenario,
+   build the handler once with the production flag fixed at construction
+   time, then call `t.Setenv("ENVIRONMENT", "development")`, then perform the
+   same login request and assert `Secure` is still `true`.
+
+## CORS
+9. Build a `*api.UseCases` (or whatever the composition-root container is
+   named after this slice) backed by the existing in-memory fakes
+   (`mockArticleRepo`, `mockUserRepo`, `mockImageRepo`, `mockMailRepo`, a
+   test `jwtauth.New("HS256", []byte("test-secret"), nil)`), then call the
+   real `api.InitRoutes(container, settings)` (or whichever exported
+   constructor this slice ends up exposing for the outer router, e.g. an
+   exported `NewApiRouter(container, settings)`) with a
+   `config.Settings{CorsAllowedOrigins: ...}` value. Do not hand-roll a
+   second `cors.Handler` call in the test; call the same construction path
+   `cmd/server/main.go` calls.
+10. For the allowed-origin case, build the router with
+    `CorsAllowedOrigins: []string{"https://cms.example.com"}`, send
+    `GET /healthz` with header `Origin: https://cms.example.com`, and assert
+    the response header `Access-Control-Allow-Origin` equals
+    `"https://cms.example.com"`.
+11. For the disallowed-origin case, same router, send the request with
+    `Origin: https://attacker.example.com`, and assert the response has no
+    `Access-Control-Allow-Origin` header (`rec.Result().Header.Get(...)`
+    returns `""`).
+12. For the empty-list case, build the router with `CorsAllowedOrigins: nil`
+    (or `[]string{}`), send a request to `GET /healthz` with any `Origin`
+    header, and assert the header is again absent.
+
+## Upload directory
+13. Reuse `openInMemoryDb(t)` and `models.Image{}` migration as
+    `test/image_extension_mapping_test.go` does, but construct the
+    repository as `persistence.NewImageRepository(db, uploadDir)` where
+    `uploadDir := t.TempDir()` — no `t.Setenv("UPLOAD_DIR", ...)` at all.
+    Additionally call `t.Setenv("UPLOAD_DIR", t.TempDir())` with a
+    *different* temp directory right before `Create`, to prove it has no
+    effect.
+14. Call `repo.Create(imageDomain.ImageInput{...})` with a PNG payload.
+    Assert no error; assert the returned `Path` starts with
+    `/static/uploadedImages/`; assert, via `os.Stat`, that the file exists
+    under the constructor's `uploadDir` and does **not** exist under the
+    `t.Setenv` directory from step 13.
+15. Update `test/image_extension_mapping_test.go` in the same slice to pass
+    `t.TempDir()` straight into `persistence.NewImageRepository(db,
+    uploadDir)` instead of `t.Setenv("UPLOAD_DIR", ...)`, since the
+    constructor signature changes.
+
+## Mailer
+16. Construct `mailer.NewMailRepository(config.SmtpSettings{Email:
+    "notify@example.com", Host: "smtp.example.com", Port: 587, Password:
+    "unused-in-test"})` with no `SMTP_*` environment variable set (do not
+    call `t.Setenv` for any `SMTP_*` name in this test). Assert construction
+    does not panic and does not error — this is the only automated
+    assertion available, since exercising `Send` requires a real SMTP dialog
+    that this suite deliberately never performs (matches
+    `test/mail_test.go`, which already tests `SendMailUseCase` against
+    `mockMailRepo`, not the real `MailRepository`).
+
+## Manual verification (not exercised by the Go test)
+- Real SMTP send: run `task dev` (or a built binary) with a real `.env`
+  populated with SMTP_EMAIL/SMTP_PASSWORD/SMTP_HOST/SMTP_PORT, trigger a
+  flow that calls `SendMailUseCase.SendMail`, and confirm the message
+  actually leaves via the injected host/port/credentials, and that changing
+  those env vars requires a process restart to take effect (there is no
+  live-reload).
+- Dev-only request logging: run the binary with `ENVIRONMENT=development`
+  vs `ENVIRONMENT=production` and confirm `httplog` output is present only
+  in the development case; this is not response-observable so it isn't
+  asserted by `httptest`.
+- Confirm `api/dependecyInjection.go`'s `getDb` and `getUseCases` consume
+  `DB_FILE` and `JWT_SECRET` from the same `config.Settings` value built in
+  `cmd/server/main.go`, rather than calling `os.Getenv` themselves — this
+  stays inside the composition root per CLAUDE.md and is not a behaviour
+  change, so it has no dedicated scenario, but review the diff to confirm
+  no stray `os.Getenv` remains there either.
+- SPA-visible behaviour: none: this slice has no SPA-facing change.