v7: move operational ADRs to gcgov/deploy, renumber, widen the glossary - #2
Open
andrewsauder wants to merge 31 commits into
Open
v7: move operational ADRs to gcgov/deploy, renumber, widen the glossary#2andrewsauder wants to merge 31 commits into
andrewsauder wants to merge 31 commits into
Conversation
Resolve Symfony-style %env(...)% references inside app.json /
environment.json at load time so secrets can come from the process
environment, Docker/Kubernetes secrets, or a .env file instead of being
stored in the config files. Fully backwards compatible: a file with no
"%env(" substring takes a byte-for-byte identical path.
- New service \gcgov\framework\services\environment:
- envVarResolver: recursive resolver over the decoded config tree.
Whole-value refs yield typed results (int/bool/float/array/stdClass/
string); embedded refs are string-substituted. Processors (applied
right-to-left): string, bool, not, int, float, trim, file, base64,
json, default. `file` reads the file at the variable's value (the
Docker-secrets pattern). `default` is a literal, innermost, greedy
fallback (documented deviation from Symfony) so colons are legal.
Env lookup: $_ENV -> $_SERVER (excluding HTTP_*) -> getenv().
- dotEnvLoader: idempotent symfony/dotenv wrapper; loads {root}/.env
then .env.local (real environment always wins); usePutenv so getenv()
sites observe values. No APP_ENV cascade.
- environmentException: neutral, wrapped per layer.
- Wire the three config choke points (config::setAppConfig/
setEnvironmentConfig, appContext::loadEnvironmentConfig): load .env,
resolve, and rethrow environmentException as configException /
cliException naming the offending variable.
- tokenReplacer: add conf, template, yml, yaml, example extensions so
`gf setup` replaces {app_*} tokens in Docker/nginx files.
- composer.json: require symfony/dotenv ^7.1.
- Tests for the resolver, the dotenv loader, and the two extended CLI
suites. Docs: readme/environment-variables.md plus CLAUDE.md, gf.md,
README.md updates.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JWacdhEDN9upUvYeagNKru
Environment selection is now environment-variable driven: apps commit a
single app/config/environment.json parameterized with %env(...), and the
process environment (container env, Docker secrets, .env) IS the
environment. BREAKING — the file-copy activation machinery is removed.
- envVarResolver::resolveJson() gains an overlayVars parameter: overlay >
$_ENV > $_SERVER (non-HTTP_*) > getenv(); a variable missing from the
overlay falls back to the ambient lookup.
- dotEnvLoader::parseFile(): parse a dotenv file to an array without
mutating the process environment (Symfony FormatException -> neutral
environmentException).
- appContext::loadEnvironmentConfig($variant): variant reads now resolve
the committed environment.json with the gitignored
app/config/{variant}.env overlay (new getEnvironmentOverlayPath /
describeEnvironmentConfigSource helpers). The legacy
environment-{variant}.json read path is removed; a leftover legacy file
triggers an error pointing at the migration guide.
getEnvironmentVariants() globs app/config/*.env (excludes *.env.example).
- environmentFiles is deleted. gf env no longer copies files: bare `gf env`
lists variants and validates the active environment; `gf env <name>`
resolves and validates a variant overlay (exit 1 naming the first
unresolvable variable). gf deploy drops the --env option and the
activation step.
- db:restore hardening for env-resolved config: --to=prod refused by
variant NAME regardless of resolved type; new findIdenticalPairs() guard
refuses a pair whose source and target resolve to the same uri+database
(the incomplete-overlay signature); type guard message names the overlay
source. db:run/db:restore help text updated.
- gf setup prompts only for {token}s actually present in the tree
(filterPromptsToPresentTokens/tokensForPromptKey), skips the Microsoft
confirm when no Microsoft tokens remain, and no longer suggests
`gf env local`.
- Tests updated/replaced to pin the new model; docs rewritten (gf.md incl.
"Migrating a v6 app to v7", environment-variables.md overlay section,
README.md scaffold tree, CLAUDE.md).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JWacdhEDN9upUvYeagNKru
Merge app.json + environment.json into a single config.json at the
APPLICATION ROOT and make \gcgov\framework\config the one configuration
API. Per-variant overlay files move to the root as well ({root}/prod.env)
— verified collision-free: docker compose reads only .env, glob('*.env')
excludes dotfiles and *.env.example, .gitignore/.dockerignore patterns
scope cleanly, and the web root is /www so root files are never served.
- New \gcgov\framework\models\unifiedConfig merges every field and helper
of the deleted appConfig + environmentConfig models (app, email,
settings + type, urls, mongo/sql databases, microsoft, jwtAuth,
payjunction, logging, appDictionary; getRootUrl/getBaseUrl/getBasePath/
isLocal/getDefaultSqlDatabase/getSqlDatabaseByName).
- \gcgov\framework\config loads {root}/config.json once (dotenv + %env()
resolution as before; getConfigFilePath()) and exposes everything
directly: getApp, getEmail, getSettings, getType, isLocal,
getServerName, getRootUrl, getBaseUrl, getBasePath, getCookieUrl,
getPhpPath, getLogging, getMongoDatabases, getSqlDatabases,
getDefaultSqlDatabase, getSqlDatabaseByName, getMicrosoft, getJwtAuth,
getPayjunction, getAppDictionary. getAppConfig()/getEnvironmentConfig()
and getConfigDir() are removed (BREAKING; plugin routers change
config::getEnvironmentConfig()->getBasePath() -> config::getBasePath()).
- Every internal call site rewritten to the flattened accessors
(renderer, router, jwtAuth, log, microsoft, mongodb dispatcher/_meta/
auth user/tools, pdodb).
- gf CLI: appContext::loadConfig($variant) resolves {root}/config.json
(variant overlays at {root}/{variant}.env; getConfigPath/
getVariantOverlayPath/describeConfigSource); variant discovery globs
{root}/*.env; legacy split-config files (app/config/app.json,
environment{-variant}.json) are detected and produce a migration hint;
phpProcess/cliCommand/env/db:restore/db:run/routeCatalog updated.
- Tests updated to the unified model + root paths; docs (CLAUDE.md,
README.md, readme/gf.md incl. the expanded v6->v7 migration guide,
environment-variables.md, mongodb.md) rewritten for the single-file
layout.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JWacdhEDN9upUvYeagNKru
Restore both v6 accessors as @deprecated pass-throughs that return the unified config object. Because unifiedConfig carries every former environmentConfig field/helper AND the app/email/settings sections, existing plugin and app call patterns keep working unchanged: config::getEnvironmentConfig()->getBasePath() config::getEnvironmentConfig()->mongoDatabases config::getAppConfig()->settings->forceMfaForPasswordUsers config::getAppConfig()->app->title Plugins can migrate to the flattened accessors (config::getBasePath(), config::getSettings(), ...) gradually instead of as a hard prerequisite for adopting v7. Marked with @deprecated + #[Deprecated]; test pins the v6 call patterns; migration guide and CLAUDE.md updated to say the old methods still work. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JWacdhEDN9upUvYeagNKru
…en resolver
Redesign the foreign-environment mechanism and apply the code-review
security/correctness fixes.
Foreign environments (db:restore --from, db:run --env, gf env <name>) no
longer come from gitignored {name}.env overlay files. They come from an
`environments` section committed inside config.json, keyed by environment
name, whose %env() references use environment-PREFIXED variable names
(e.g. PROD_MONGO_URI) kept in the same .env. This removes the silent
local-value fallback hazard by construction (distinct names fail loudly),
keeps prod secrets out of developer workstations except the one Mongo
credential a restore needs, and makes the `type` guard reliable again
(committed literal). --from-uri/--from-db flags were considered and
dropped.
- New services\environment\configLoader: the single load pipeline
(config.json -> .env -> resolve %env -> hydrate) shared by
\gcgov\framework\config (runtime) and appContext (CLI); strips the
CLI-only `environments` section for the active config, extracts one
entry for loadVariantEnvironment(). Removes the duplicated pipeline the
review flagged and gives both layers identical resolution + errors.
- New models\config\variantEnvironment (type + mongoDatabases) for an
environments entry.
- envVarResolver: request-data injection guard now applies by NAME across
$_ENV/$_SERVER/getenv() (was $_SERVER-only, bypassable under CGI/FastCGI
where headers reach getenv) — HTTP_* plus the CGI meta-variable set are
never resolved from the ambient environment; a leftover %env( after
resolution (e.g. ')' inside a default: literal) now throws instead of
silently shipping the literal; the overlay param is gone (resolveDecoded
added for in-place tree resolution).
- dotEnvLoader: loads .env and/or .env.local (either may exist alone — the
.env-only early return silently skipped a lone .env.local); FormatException
from a malformed file is wrapped as environmentException; parseFile()
removed.
- config.php: uses configLoader; getConfigDir() restored as a deprecated
shim (parity with the other kept shims); getAppConfig() now returns a
v6-shaped appConfig VIEW (app/email/settings only) so serializing it no
longer leaks mongo/microsoft/payjunction secrets; environmentConfig
restored as an autoloadable class_alias to unifiedConfig so v6 type
references keep working.
- cliCommand: a present-but-unresolvable config.json now surfaces loudly
instead of being swallowed (which discarded the configured phpPath);
--php help says config.json.
- setupCommand: the token haystack (full tree read) is built once and
shared between the two prompt-filter calls; the "already set up" message
only prints when NO prompts (incl. Microsoft) remain.
- envCommand/dbRestore/dbRun rewired to the environments section; env
command validates an environments.{name} entry or the active config.
- Tests reworked for the environments model + security guards
(ConfigLoaderTest added); docs updated (CLAUDE.md, README.md,
environment-variables.md, gf.md).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JWacdhEDN9upUvYeagNKru
Fixes one word per concept across code, docs and conversation. Notably disambiguates "environment", which currently names three unrelated things (a deployment target, a variable set, and a config section), and records the v6 terms that no longer name anything. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012kqJazGHDMq19pQ64732i6
Zone names the network isolation boundary (internal-only / public with internal access / public without) and is explicitly distinguished from Environment, which the two were being conflated into. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012kqJazGHDMq19pQ64732i6
0001 fail-closed configuration (no default:, empty is unset) 0002 immutable Release pinned by digest, replacing in-place gf deploy 0003 secrets never decrypt in CI or on hosts (SOPS, per-Zone KMS, operator provisioning) 0004 one self-hosted runner per Zone, dedicated host, no Docker socket Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012kqJazGHDMq19pQ64732i6
v7 is container-only and its images are built on php:8.4-fpm, so the framework's floor moves with them. CI drops the 8.3 leg. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012kqJazGHDMq19pQ64732i6
… routes
Configuration
- envVarResolver: delete the `default` processor; every reference is now
required and a set-but-empty variable counts as unset. Removes the greedy
argument parsing that only existed to let a fallback literal hold colons.
- New `secret` processor implementing the conventional _FILE indirection:
%env(secret:MONGO_URI)% reads MONGO_URI_FILE's file when set, else
MONGO_URI. A _FILE naming a missing file is a hard error and never falls
back — that fallback would substitute a stale environment value for a
secret that failed to mount. One committed config.json now serves both a
developer machine and production.
- Processor set trimmed to secret/file/trim/int/bool/json; string, not,
float and base64 had no users.
- collectReferences()/configLoader::references() enumerate what config.json
needs without resolving it, so the .env manifest is derived rather than
hand-kept.
- Drop serverName, cookieUrl and phpPath: no reader in the framework or in
any of the five framework services. app.guid stays — the oauth server uses
it as the OAuth client_id. jwtAuth issuer/audience now derive from
rootUrl/basePath when unset.
- Delete the `environments` section and all variant plumbing.
Runtime
- jwtAuth.keyPath makes the signing-key directory configurable. The keys are
gitignored, so they are never in a built image; a container must point this
at a provisioned mount or authentication cannot work at all.
- logging.destination (stderr default, JSON lines / file / both). A container
filesystem does not survive a deploy, so file logs would be per-replica and
destroyed on every release.
- The framework contributes GET {basePath}/health (liveness, no I/O) and
/health/ready (readiness, pings Mongo, 503 when down), merged before
services and the app. Not opt-in: a deploy pipeline cannot gate on an
endpoint an application might omit. Split because a shared probe turns a
brief database outage into a crash loop.
CLI
- Remove `deploy` (in-place git+composer on the server), `db:restore`
(production credentials on every workstation), `db:run --env`, `setup` and
tokenReplacer.
- `gf env` gains --list and --init; `gf init` replaces the setup wizard,
non-interactive so it runs from a scaffolding script or devcontainer;
`gf migrate` converts a v6 application, its plan() a pure function of the
two v6 documents so it is unit-tested rather than run hopefully.
542 tests pass; PHPStan level 5 clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012kqJazGHDMq19pQ64732i6
….json Framework Services were separate Composer packages, switched on by returning their namespace strings from \app\app::registerFrameworkServiceNamespaces(). That put activation in PHP and configuration in config.json, so "how is auth set up here?" had two answers in two places; it also meant a forgotten namespace was a silent 404 rather than an error, since the router swallowed the ReflectionException. The five services now live in src/services/ and are enabled by a typed `services` section of config.json. Presence enables: a block that is absent is off, a block that is present — even empty — is on, and its contents are that service's settings. This reuses the nullable-section pattern already used by kmsProviders::$gcp, so no new hydration machinery was needed and %env(...) works throughout. registerFrameworkServiceNamespaces() is deleted rather than deprecated. No v7 application is deployed, so there is no installed base a dual mechanism would protect, and carrying both would reintroduce exactly the "which list won?" ambiguity ADR 0001 removed the default: processor to avoid. The two auth services merge into one, chosen by `provider`. Everything downstream of establishing an identity was already identical — the guard, the JWKS document, the short-lived file token — and existed as two near copies that could not be deduplicated while the packages were separate. One provider key also makes two auth providers unrepresentable, so no conflict check is needed. The standalone packages stay published for v6 applications. Since documentation and cronMonitor keep their namespaces, framework v7 declares a `conflict` against all five so an application cannot resolve both and get two definitions of the same class. Also in this change, because the code was being touched anyway: - Split interfaces\router. It required _before()/_after() of every router and the framework only ever called \app\router's, and \app\router's getRunFrameworkServiceRouteAuthentication() was duck-typed via method_exists() with no interface declaring it. Now interfaces\router is getRoutes()+authentication(), interfaces\appRouter adds the lifecycle hooks, and the opt-out is interfaces\router\skipsServiceAuthentication. - Refuse to boot when routes declare authentication:true and neither an auth service nor \app\router::providesAuthentication() will guard them. Such routes were reachable by anyone while looking protected, because the scaffolded authentication() returns true. - MFA enrollment QR codes render as SVG. BaconQrCodeProvider defaults to the Imagick backend, which would have made ext-imagick a hard requirement of the framework for every application in order to draw a square. - The MFA issuer label was the literal 'GCGOV Narcotics Tracking', so every application's authenticator showed that name. It is now the app title. - The Microsoft token exchange constructed a controllerException without throwing it, so a missing Authorization header fell through instead of returning 401. - The openid-configuration route pointed at method 'openid'; the controller method is openId, so calling it fataled. A new test asserts every route registered by a service resolves to a real method. - The documentation service derives the framework directory from its own location instead of hardcoding vendor/gcgov/framework. Under a path repository that hardcoded path did not exist and was silently dropped, so nothing of the framework was documented in development. Its src/services is now scanned too, so the services' own annotations reach the document for the first time. - Removed settings.useSession, which nothing read, and gave sqlDatabase the _afterJsonDeserialize guard unifiedConfig and mongoDatabase already have — without it a partial entry raised a raw PHP Error instead of a configException. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KmBBiV3fQaaRdrmspZarS5
The five packages' test suites move into tests/Unit/Services/ mirroring src/. Three needed more than a namespace rewrite: - The two auth router tests became one. They now cover the same class, and both were written against v6 — a no-arg constructor, models\environmentConfig, and the lifecycle hooks the service router interface no longer declares. The consolidated test asserts the route set each provider contributes and the guard's refusal behaviour, which used to be two near copies. - The jwks/fileToken assertions moved to a test for the shared controller, alongside one asserting neither provider still declares them. Note the old test named the method 'openid' and passed anyway: ReflectionMethod is case-insensitive, which is why nothing caught the router pointing at a method that does not exist. - UserControllerTest now runs in separate processes. It needs \app\models\user to exist so request::getUserClassFqdn() resolves to the stub, while the framework's RequestTest asserts the opposite — that with no application user model the Mongo default is returned. One process cannot hold both, so the stub is required in setUpBeforeClass, which runs only in the child. Two router tests relied on whatever configuration a previously-run test had left in the static; they now seed their own. The documentation one keeps its multi-segment base path, which catches a router assuming one path element. gf migrate gains the services half of the conversion: - detectServices() reads app/app.php and reports the namespaces registered and the configuration singletons called. It strips comments with the tokenizer rather than matching text, because the scaffolded app.php ships the alternatives commented out directly above the live array — a plain search reports services the application does not run, and a conflict between the two auth services that is not there. Verified against the real app/app.php from framework-app-template's v7 branch. - plan() takes what was detected and writes the services section, moves appDictionary.cronMonitorUrl to its own typed cronMonitor.url, and turns singleton calls into warnings naming the config keys that replace them. It reports rather than guesses, as it already does for sqlDatabases. - execute() removes the service packages from the application's composer.json. The framework conflicts with them, so leaving them makes the application unresolvable rather than untidy. It stops short of running composer update: resolution does not belong in a command that is otherwise file manipulation. 688 tests pass, up from 542. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KmBBiV3fQaaRdrmspZarS5
CONTEXT.md defined Framework Service by the mechanism this work deletes
("when the Application registers its namespace"), so the glossary entry is
rewritten and Provider added beside it. Service namespace registration and
"auth plugin" join Retired language.
ADR 0005 records why, including the part that will not be reconstructible
from the diff: the packages are properly released and independently
versioned — an earlier draft claimed otherwise, having misread clones that
had not fetched tags — so folding in trades that away deliberately, in
exchange for the three things the split caused and could not fix.
CLAUDE.md, README.md, readme/app.php.md, readme/router.php.md and
readme/gf.md drop the plugin vocabulary and describe the services section.
app.php.md needed rewriting rather than editing: its subject was the deleted
method.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KmBBiV3fQaaRdrmspZarS5
framework.php kept `new \app\app();` as a bare statement, which PHPStan reads as having no effect. The instance is held for the lifetime of the request as it always has been — there is simply nothing left to ask it for. The documentation controller's exclusion list is now built by appending rather than by unset(), so it has no holes and array_values() was a no-op. Two findings remain and predate this work, in files byte-identical to the base commit: dotEnvLoader's spread of a list PHPStan cannot see is non-empty, and gridfs's unsafe new static(). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KmBBiV3fQaaRdrmspZarS5
…ret keys Two decisions taken while making gcgov/deploy operational, both of which trade away something the design originally assumed it would have. 0006 — Let's Encrypt DNS-01 on one registered domain every Zone shares. All three Zones serve names under garrettcountymd.gov, and a Cloudflare token scopes to a registered domain, so per-Zone token scoping is not achievable and every Zone holds a credential with power over every other Zone's names. Accepted for the bridge pilot, where only one token exists, on the condition that _acme-challenge is delegated per Zone before a second Zone is provisioned — enforced by an unresolved placeholder in ZONE_ACME_DELEGATION rather than by anyone remembering. An internal CA and a wildcard certificate were both considered; the wildcard is rejected outright, since a wildcard on the internal host is a certificate valid for www. The ADR also records that DNS-01 publishes internal hostnames to Certificate Transparency permanently, which is why paloalto-tools was renamed to netops-tools before first issuance, and that ZONE_ACME_EMAIL is a registration contact rather than a monitoring backstop now that Let's Encrypt no longer sends expiry mail. 0007 — deployment secrets are encrypted with KMS keys in a GCP project of their own. The MongoDB queryable-encryption credential lives on an application host; ADR 0003 says the sops keys must never be reachable from one. Sharing a project puts both in the same IAM surface, which per-key bindings contain today and a project-level binding granted later would not. Access goes through a Google group per Zone so that offboarding is one membership removal rather than three IAM edits that can be half-finished. CONTEXT.md gains Ops Project, Delegation Zone, Break-glass Key and Escrow Custodian. Documentation only — no PHP changes. composer ci was not run: composer install cannot authenticate to github.com from this environment, and ext-mongodb is absent. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N3hHTQA6apQYF5mntbU8tf
Rewritten in place rather than superseded. ADR 0007 exists only on this unmerged branch and has never been in the mainline, so a superseding ADR would document the reversal of a decision that never took effect anywhere. The reversal came out of building it. The original ADR argued for a dedicated GCP project so the deploy keys would not share an IAM surface with the MongoDB queryable-encryption credential, which lives on an application host. That argument still holds and is kept. What did not survive contact was the access model: there is no GCP organization and no group layer, operators sign in with individual Google accounts, so access would have been per-person IAM bindings and offboarding one edit per key per person — three chances to half-finish a revocation, which is the failure the ADR was written to prevent. Entra already has the groups, and more to the point already has the joiner/mover/leaver process, so revoking decrypt becomes a consequence of offboarding rather than a separate thing to remember. SOPS supports Key Vault natively and bin/provision only shells out to sops. With Mongo staying on GCP, the separation the ADR wanted is now across two clouds rather than two projects — a stronger form of the same property, arrived at sideways. Recorded consequences worth having in writing: the break-glass key becomes more load-bearing because the vaults are Entra and a tenant compromise takes the primary path outright; Azure key URLs are version-pinned so rotating a key means sops updatekeys across every file; Key Vault audit logging is off by default, which is what the offboarding runbook's claim about reading decrypt records depends on; and access is standing rather than just-in-time, since PIM would need Entra ID P2 and the county holds P1. CONTEXT.md replaces Ops Project, a GCP-shaped term, with Zone Key Vault. Documentation only. composer ci was not run: composer install cannot authenticate to github.com from this environment, and ext-mongodb is absent. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N3hHTQA6apQYF5mntbU8tf
…ecrets Two details in the summary of ADR 0003 no longer describe what is implemented, while the decision itself — operator-workstation decryption, no key on a host or in CI — is honoured exactly. The wrapping key moved to Azure Key Vault, one vault per Zone, in ADR 0007. And the plaintext lands in /etc/gcgov/secrets on the host, not /run/secrets: /run is a tmpfs, so anything written there is gone after a reboot and every container fails to start on the way back up. /run/secrets is what the container sees — the bind-mount target, not the host path. Recorded as an amendment note rather than an edit to the decision, the way ADR 0007 already amends this one, so the record stays readable as history. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018NP3QWawCQVvvzFLQMR5iZ
…s, credential handling
A max-effort review of this branch surfaced three recurring themes. This addresses
them, plus the individual defects underneath.
1. The service fold-in was unfinished. Code moved into src/services/ kept behaviour
the framework now forbids and skipped normalization it now provides.
- Routes at the domain root. getBasePath() returns '/' there, which is right for
the token audience and wrong as a route prefix: the auth, userCrud and
documentation routers registered //user and //auth/authorize, which FastRoute
stores and matches as literal strings, so every Framework Service endpoint 404'd
while /health worked because it alone happened to rtrim. Adds
config::getRoutePrefix() ('' at the root) and points all four routers at it.
getBaseUrl() had the same trailing-slash defect in the advertised OAuth callback.
- Removed a debug error_log() that wrote the user's stored password hash on every
MFA-secret verification, and routed the auth service's remaining error_log calls
through services\log so they honour logging.destination.
- Replaced three exit; calls in the oauth controller with a 302 controllerResponse,
so an OAuth sign-in no longer skips the controller, renderer and app _after hooks.
2. The new fail-closed checks threw a class nobody caught. configException extends
\LogicException while runApp() caught only routeException, so the checks written to
refuse loudly instead produced a bare PHP fatal with the rest of the lifecycle
skipped. Same escape for FastRoute's BadRouteException and a \TypeError from a
mistyped \app\router. runApp() now catches these, logs the detail in full and
renders a generic 500 — the messages carry route patterns, config paths and
environment-variable names, so they stay out of the response. An application
defining a route the framework already registers now wins it rather than taking
every route down with it.
3. Docs and code contradicted each other in both directions. The canonical \app\router
example still said interfaces\router, which no longer boots; README advertised three
deleted gf commands and a config section that never existed; CLAUDE.md's layout,
base-path guidance and never-exit rule were out of date.
Also fixed:
- userCrud save() ignored its {_id} route parameter, so a body could retarget the
write to any account, roles included. The URL now binds.
- createAccessTokenResponse() built controllerException with message and code
swapped, raising a TypeError instead of the intended 500; the outer catch that
caused it is gone, along with two inlined copies of the helper that had drifted.
- Roles are narrowed to strings and compared strictly: a non-string truthy element
satisfied a loose in_array() against every required role.
- cert:generate-auth ignored jwtAuth.keyPath, so it wrote keys where jwtAuth would
not look and the error message named itself as the remedy. Both sides now share
one resolver.
- gf init round-tripped config.json through json_decode(assoc), rewriting {} as []
and silently disabling the services the template declares.
- gf env --init is now genuinely additive, as its help text always claimed;
--force still rewrites but says what it discards. --list now loads .env, and the
"is it set" check is shared with the resolver rather than reimplemented.
- gf migrate wrote .env values unquoted, corrupting any secret containing $, # or a
space. Verified round-trip against symfony/dotenv.
- Readiness probes now fail fast instead of parking a worker for the driver's 30s
default, and report status without echoing driver messages that name internal
hosts and ports on an unauthenticated endpoint.
- mkdir(777) decimal created an unwritable directory; the write after it was
unchecked. The bool env processor now fails closed like int. cronMonitor honours
the documented empty-url off switch. The OAuth state is encoded on the way out and
no longer double-decoded on the way in.
Tests: adds coverage for the health service and the auth guard (both previously
untested), the domain-root routing regression, role narrowing, the lifecycle catch,
route override, gf init JSON preservation, .env quoting and .env preservation. Adds a
shared config-seeding trait so tests stop leaking global config, and drops a
CoversClass for a class this branch deleted.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01R2sLagem1ERvXQGwcoBgA1
…n, CLI env, deprecation shim
- verifyMfaSecret passed a nullable, client-supplied userMultifactorId to a
non-nullable parameter, so a body omitting it produced a TypeError and an opaque
500 where the deserialization guard was meant to yield 400. Both MFA endpoints now
build the caller's ObjectId through one helper that answers 401 rather than
InvalidArgumentException when the token carries no data.userId claim.
- The documentation service excluded {root}/vendor while adding the framework's own
src to the scan list. Under a normal Composer install the framework lives inside
that tree, so the exclusion was a prefix match over everything just added: the
Framework Service annotations the change exists to publish were dropped again, and
only a symlinked dev checkout behaved as documented. The scan list is explicit, so
the exclusion is removed — and a service that is not enabled is now excluded
instead, since documenting it advertises endpoints that 404.
- gf cli chose the PHP interpreter before anything loaded .env, so a GF_PHP set there
was invisible and the route ran on whatever PHP was on PATH.
- The deprecated appConfig shim promised v6 call sites "including ones that serialize
the object" would keep working, but no longer extended the class that provided
serialization. It now implements JsonSerializable, and the docblock states plainly
what a v6 appConfig could do that a view onto loaded configuration cannot.
getAppConfig() is memoized again, keyed on the unifiedConfig it views so replacing
the configuration cannot leave a stale view behind.
- gf migrate now names the two router contract changes nothing else surfaces:
\app\router must implement appRouter, and the service-auth opt-out is an interface
rather than a duck-typed method, so a leftover v6 method is silently ignored and
self-authenticated routes start returning 401.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01R2sLagem1ERvXQGwcoBgA1
requiredRoles is declared on \gcgov\framework\models\route and carried into routeHandler
— framework-level models, present on every route of every application — but the only code
that read it was guard::authenticate(), inside the optional auth service. Two supported
configurations therefore declared roles that nothing checked, while looking protected in
the route table, in `gf cli:list` and in review:
· No services.auth block, with \app\router::providesAuthentication() returning true. The
boot check is satisfied and userCrud::authentication() returns true unconditionally, so
the framework's own /user routes ran with User.Read and User.Write checked by nobody —
roles the application author never wrote and had no reason to know about.
· Any route where skipsServiceAuthentication skipped the service guards, taking the one
role check in the codebase with them.
assertAuthenticationIsProvided() cannot see either case: it asks whether *something*
authenticates, never whether anything enforces roles.
Enforcement moves to router::assertRequiredRoles(), called after the app router and every
service router have run, so it holds however the caller was authenticated. The guard keeps
the part only it can do — validate the token and establish authUser — and its copy of the
loop is deleted rather than duplicated, since two enforcement paths are what let the
answers diverge. For the auth-service path the observable behaviour is unchanged: same
check, same message, same 403, one step later in the chain.
Fails closed on the case the boot check cannot detect: a route that declares roles when
nothing established a user is refused with a 401, because an \app\router::authentication()
that returns true is indistinguishable from one that verified something. The client gets a
generic message; the log gets the route and the remedy. This is a behaviour change for an
application that authenticates its own routes without recording the caller, so both
interfaces that can reach it now say so explicitly: providesAuthentication() and
skipsServiceAuthentication both spell out that the authenticator must populate the
request-scoped authUser via request::getAuthUser()->setFromUser(), and that opting out of
the service guards never opts out of requiredRoles.
Also corrects the contracts that documented the gap: route.php's @PARAM said roles must be
implemented in \app\router::authentication(), CLAUDE.md's guard flow credited the auth
service, and userCrud's docblock argued "installed but unguarded is no longer reachable"
from the boot check alone — now true for both halves rather than one.
A route declaring requiredRoles with authentication:false is contradictory: it returns
before the guard chain, so its roles can never be checked. That is warned at boot rather
than refused — the declaration was already inert, so failing an application's boot over it
would break something that works rather than protect anything.
Tests: the 401-with-no-user case is the regression this exists for and previously passed
silently. Also covers the empty-roles route (which must not start requiring
authentication), exact and subset role holdings, and non-string scope elements through the
new call path. GuardTest asserts the guard has not kept a second copy of the check, so the
move is provably a move.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01R2sLagem1ERvXQGwcoBgA1
- userCrud: POST /user/new assigns a fresh ObjectId instead of unset():
the model's typed $_id is read unconditionally by factory::save(), so an
unset property was a fatal uninitialized-property Error on every create.
The test stub's save() now mirrors that unconditional read so the suite
fails the way production did.
- router: the app-overrides-framework dedup now keys routes the way
FastRoute defines a duplicate — by compiled shape (patternShapes()), not
spelling — so user/{id} vs user/{_id} no longer slips past the filter
into a BadRouteException that 500s every url; a static app route
shadowed by a variable service route also drops the service route. The
override notice and the roles-without-authentication warning are gated
behind lifecycle logging: routes rebuild per request, and both logged
one identical line per request forever.
- health: when services.auth is enabled, /health/ready checks that the key
directory holds usable signing keys, so an unmounted or empty key mount
fails the deploy gate instead of surfacing as a configException at the
first production sign-in. ready() also loses its duplicated array keys
and over-wide try block.
- cert:generate-auth: resolves jwtAuth.keyPath without demanding the whole
config.json resolve, so `gf init` can generate keys on a fresh scaffold
whose .env is still empty (this also repairs the command's own tests,
which ran without a config.json and threw); a relative keyPath anchors
to the application root.
- gf env: declaredNames() parses .env with the same symfony/dotenv parser
the runtime loads it with (the regex miscounted multi-line quoted
values); reserved CGI meta-variable names are reported as RESERVED
rather than MISSING and written as guidance rather than dead lines; the
secret _FILE hint carries the /run/secrets/<app>/ segment the deployment
convention uses and is shared with gf migrate via one helper.
- envVarResolver: public isReservedName(); tests pin the fail-closed bool
processor and isSatisfied().
- test suite health: AuthUserRolesTest's stub-loading test runs in a
separate process — it defined \app\models\user for the whole run and
failed RequestTest's default-model assertion; LifecycleExceptionTest's
double-quoted assertion interpolated an undefined $e and checked the
wrong string.
- readme/gf.md: describe the --init append behavior and the configured
cert keyPath, both changed in v7 but undocumented in the authoritative
CLI reference.
composer ci: phpstan clean, 787 tests green (the branch baseline had two
errors, one failure and two warnings).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012SmBhj1hdgf4hFcvCN78m5
On Windows guid::create() returns uppercase GUIDs (com_create_guid), and the ops repository's provisioning lowercases every secret filename it writes to the host — so an uppercase GUID put a lowercase pem file on the case-sensitive host while guids.json kept the uppercase spelling jwtAuth looks files up by, and every sign-in failed with nothing pointing at the casing. Lowercasing at the source keeps the filename and its guids.json entry in agreement everywhere. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012SmBhj1hdgf4hFcvCN78m5
A freshly scaffolded v7 application could not be brought up on a developer machine by following its own instructions. Three of the four blockers were in the app template; this commit carries the framework half. - gf user:create creates the account you sign in as, saved through the model the application actually resolves so the password is hashed by it and every model hook runs. An app with services.auth enabled had no way to get its first user: blockNewUsers defaults true, every /user route needs a caller already holding User.Write, and a hand written mongosh document has no password anyone can sign in with because the model hashes on write. --force updates an existing email in place, leaving options you did not pass — the password included — alone, so it is also how you grant a role. The option mapping and role parsing are pure statics, driven directly by the test rather than through a database. - gf init now appends to an existing .env instead of skipping it. The step delegated to `env --init`, which is additive by design, so it tops up the references a file lacks and leaves filled-in values alone. Skipping broke the documented bootstrap the moment it began with `cp .env.example .env`: the file existed, the application's own variables were never appended, and `gf env` then failed on the first of them. - Documented the replica-set requirement, which was written down nowhere. save/saveMany/delete/deleteMany/deleteManyBy each open a transaction when not handed a session, and MongoDB offers transactions only on a replica set or mongos — so a standalone mongod serves every read and fails every write. That is the failure a new application hits first, and it survives a smoke test because the list endpoints work. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014PQDyvhd1iz7g1AF6RsCdT
…e framework Follow-up from a design review of the local development work. Five changes, each one a decision that had been made implicitly and is now written down where it will be found. - ADR 0008 records that every write opens a transaction, so MongoDB must be a replica set. This was a real property of factory::save() that nothing stated, and it is now load-bearing for every developer and every CI run. The ADR keeps the rejected alternative — open a transaction only when the save spans more than one write — because that is exactly what the next reader will propose on finding a session around a single-document write. It trades an unconditional invariant for one conditioned on attributes that change over time, and on which OTHER models embed a copy of this one, which the model being saved cannot see. - readme/local-development.md holds what any application needs in order to run locally: the replica set, fail-closed configuration, the signing keys, and the Bootstrap User. It deliberately names no compose services. The application template keeps the commands, because those are its own — and because a scaffolded application's copy of any file is frozen at Scaffold time, while this page reaches it through Composer. - CONTEXT.md gains Bootstrap and Bootstrap User. Bootstrap was listed under Scaffold's _Avoid_, which no longer holds: Scaffold is the one-time copy from the template, Bootstrap is the idempotent act of making a scaffolded application runnable, and the two are genuinely different. Bootstrap User names the account that breaks the circle a fail-closed auth posture creates — a domain concept, which is why it is here and the host/container variable split is not. - gf init's help still said "run once after scaffolding" while the command documented re-running for the guid and its .env step was just made additive. Reworded around idempotence, and it now says plainly that it cannot create the first user: nothing can be written to a database that .env does not yet describe. - gf user:create warns when settings.forceMfaForPasswordUsers is on. Such an account cannot sign in with its password alone — the first authorize returns an enrolment challenge and a token carrying no roles — and the command that created it said nothing, which is the same looks-like-success failure that readiness checks the signing keys to avoid. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014PQDyvhd1iz7g1AF6RsCdT
All four are pre-existing and environment-dependent — nothing in the local development work touched config paths, phpProcess, or the route runner. Two distinct causes. Three of them compared a temp root built from sys_get_temp_dir(), which on Windows carries backslashes, against accessors that normalise separators (configLoader::configFilePath, unifiedConfig::getJwtKeyPath). The fixture was wrong, and wrong in a way worth naming: config::$rootDir is only ever reached through setAppDir(), which forward-slashes it, or through appContext::normalize(), which does the same — so a backslash root cannot occur at runtime and ConfigTest was manufacturing one with reflection. Both fixtures now normalise, matching the pattern AppContextTest already uses. Normalising the fixture would leave configFilePath's own test vacuous on a system whose temp path has no backslashes, so it now asserts the contract outright against a literal C:\app. The fourth proved that $argv survived by checking the missing autoload path appeared in the child's output — which happened only because PHP displayed the fatal from require. Whether it does is entirely the host php.ini's business: with display_errors Off and error_log naming a file, both ordinary on a server, the child exits 255 having printed nothing anywhere the caller looks. That is a real gap rather than an unlucky assertion. `gf cli` is what Task Scheduler and cron run, and a wrong vendor path currently fails silently. run-route.php now checks the autoloader itself and reports through the $gfWriteError closure it already uses for a missing $argv and an undefined STDERR — the same class of problem, previously unhandled — exiting 2, the code it already gives every other bad invocation. The test asserts that exit code, so its evidence no longer depends on how the host reports errors. Verified on Linux by reproducing both failures: run-route.php under `-ddisplay_errors=0 -derror_log=<file>` produced empty output and exit 255, and now names the path and exits 2; injecting a backslash root into config reproduced the two reported expected/actual strings exactly, and the normalised fixture matches. The Windows workstation is still the only place the originals can be confirmed fixed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014PQDyvhd1iz7g1AF6RsCdT
Nothing was failing. A green run printed four JSON log records — a router warning, two readiness warnings with a driver stack trace, and two records from LogTest — because log's default destination is stderr and these tests exercise paths that log. Output that reads like failure on a passing run is worth removing on its own; it also buries the failures that are real. A new capturesFrameworkLog trait swaps a channel's logger for a Monolog TestHandler, so the records land somewhere the test can read. Restoration hangs off #[After] rather than tearDown(), which lets it compose with seedsFrameworkConfig — that trait already defines one. Swallowing the records would have been the easy fix and the wrong one, so each is now asserted: - Readiness deliberately withholds the database host and port from an unauthenticated 503. That detail has to reach the operator somewhere, so the test now pins that the log names the failing database — and, for the key check, that it names the command that fixes it. - A role-gated route with nobody authenticated refuses with 401 AND explains why. Those are one behaviour: a 401 whose cause is unlogged sends an application developer hunting through the guard chain for a route that is simply unguarded. The test moves from expectException to try/catch so it can assert both halves. LogTest's two destination tests now assert the handlers rather than logging. That is not just about noise — testStderrIsTheDefaultDestinationAndEmitsJsonLines never asserted anything about JSON lines. It logged a record and checked only that no file appeared, which is equally true of implementations that are broken in several interesting ways. It now checks that the stderr destination builds exactly one StreamHandler on php://stderr with a JsonFormatter, and that "both" adds the file handler alongside it. Monolog opens streams lazily, so building the handlers writes nothing and creates no file. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014PQDyvhd1iz7g1AF6RsCdT
`gf db:restore` was removed in v7 and readme/gf.md pointed at "the separate
backup-restore workflow" for what replaced it. Nothing described that workflow,
so this writes it down where the other local-development rules live.
Three things hold for every application, whatever stack it runs: a workstation
reads no other Environment's database, so a dump file travels instead of the
credentials; a restored account carries a hash and no password anyone knows, so
`gf user:create --force` is still the way in; and a dump of an encrypted
collection is ciphertext to a computer without the keys.
gcgov/framework-app-template now ships one implementation of it — a mongo-restore
container reading db/backup/{DatabaseName} — and the section links to it, since
the commands belong to the application.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rw19sb82Jy2GQTLdd7N8f7
Four ADRs carrying the county's operational threat model (secrets, runners, DNS-01, Key Vault) move out of this public repository into the gcgov/deploy Ops Repo. The ADRs that stay are renumbered into a clean 0001-0004 sequence: - 0001 fail-closed configuration (unchanged) - 0002 immutable Release, pinned by digest (unchanged) - 0003 Framework Services are built in (was 0005) - 0004 writes are transactional; Mongo replica set (was 0008) Add docs/adr/README.md with the mapping and citation rule, refresh the ADR index and citations in CLAUDE.md and the readme/ files. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NzByhoDp7hsD39aoThJ9rv
gcgov/deploy has deployed browser bundles as well as APIs for months, and the escrow has one operator rather than two. Widen the glossary to match: - Application now covers both Application Kinds, not only a REST API. - Add Application Kind (api or frontend). - Provisioning covers the compose file and Zone values, not only Secrets. - A Release is a set of named content digests, one per image. - Escrow Custodian describes one custodian plus a second safe-opener. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NzByhoDp7hsD39aoThJ9rv
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This branch is
v7plus the "truthful and armed" documentation work, so merging it is thev7 → mainmerge for this repository. Merge aftergcgov/deploy.What changed
gcgov/deploy. This repository is public, and those four (secrets never decrypt, one runner per Zone, Let's Encrypt DNS-01, Azure Key Vault) carry the county's operational threat model — they belong in the Ops Repo beside the mechanism they describe.0001–0004sequence:0001fail-closed configuration (unchanged)0002immutable Release, pinned by digest (unchanged)0003Framework Services are built in (was0005)0004writes are transactional; Mongo is a replica set (was0008)docs/adr/README.mdrecords the old-to-new mapping (both directions) and the citation rule.CLAUDE.md(including the "ADRs recorded so far" index, which the move made stale),readme/app.php.md,readme/mongodb.md,readme/local-development.md.CONTEXT.mdglossary widened for the work that has happened since it was written:Applicationnow covers both Application Kinds; a newApplication Kindterm (api/frontend);Provisioningcovers the compose file and Zone values; aReleaseis a set of named content digests;Escrow Custodiandescribes one custodian plus a second safe-opener.No code changed — this is documentation and the ADR tree only.
Note
docs/agents/domain.mdstill containsADR-0007 (event-sourced orders)— that is example text from the domain-modeling skill, not a reference to a real ADR, and was deliberately left as-is.🤖 Generated with Claude Code
https://claude.ai/code/session_01NzByhoDp7hsD39aoThJ9rv
Generated by Claude Code