Skip to content

31 August 2026

34 changes:

  • The half of OAuth discovery that was missing
  • Three components that were nearly accessible
  • The Model Context Protocol, over HTTP, for somebody else's assistant
  • Offering an MCP capability without writing a class
  • A page head that stops claiming things the page never said
  • What the site tells a machine that arrives uninvited
  • A keyboard can get past the navigation, and the consent screen stops phoning out
  • A token outlived the client that issued it, and kept working
  • An error log that was mostly not errors
  • A client secret nobody had to present
  • The SMTP password was readable to anyone who could read the database
  • The webhook signing key and the TOTP seed, likewise
  • A tenant scope that held until you asked for page 1
  • A grant on one record opened the whole collection
  • The session cookie lost its secure flag behind every TLS-terminating proxy
  • The legacy CSRF check compared tokens with ===
  • A LIMIT built by string concatenation
  • addslashes() as an escaping fallback, and as a PHP-literal encoder
  • The README described a framework this is not
  • The schema promised tenant isolation nobody implements
  • Joins on two columns, and aliases that survive resolution
  • Permissions can now be resolved within one organisation
  • Roles had two tables, a permissions screen, and no way to make one
  • The client secret is hashed, and the realtime key moves out of its way
  • A public client can say it is one
  • The roles screen had a menu entry and no address
  • A guard that read a variable that did not exist (FW-047)
  • A pager that could not be told to stop counting (FW-048)
  • Two components no theme could style
  • SchemaBuilder::hasIndex()
  • Tokens are encrypted at rest, and matched on a digest

The half of OAuth discovery that was missing

/.well-known/oauth-protected-resource — OAuth 2.0 Protected Resource Metadata, RFC 9728.

An installation has answered /.well-known/oauth-authorization-server for a long time. That document says where the authorization server is. This one says where the resource is and which authorization servers it trusts, and nothing was answering it.

A client holding only the first half has to be told the second out of band — configuration somebody types into a file, gets wrong, and has no way to verify. Both halves exist so that a client can start from either end and find the other.

Which is where an MCP client starts

The Model Context Protocol's authorization flow begins here: a client calls a protected endpoint, is refused with a WWW-Authenticate header naming this document, reads it, finds the authorization server, and then runs the ordinary OAuth 2.1 authorization-code-with-PKCE exchange it already knows how to run. Every part of that except this document was already in place.

{
  "resource": "https://example.com",
  "authorization_servers": ["https://example.com"],
  "scopes_supported": ["profile", "email", "phone", "address", "user", "openid", "offline_access"],
  "bearer_methods_supported": ["header"],
  "resource_documentation": "https://example.com/docs"
}

Three things about that document are deliberate:

  • scopes_supported is read from Scopes, not written out. A hardcoded list drifts the first time somebody adds a scope, and the failure is a client asking for something the server refuses — after the person has already been walked through a consent screen.
  • bearer_methods_supported is ["header"] and only that. RFC 6750 also allows a token in a form body and in a query string; the query form puts a credential into every access log, proxy log and Referer between the client and here. This framework does not accept it, so it must not advertise it — an unstated capability is one a client tries anyway.
  • No trailing slash on the identifiers. resource is compared as a string when a token's audience is checked, and https://example.com and https://example.com/ are the same address and different strings. The mismatch presents as a token that is valid and rejected, which is the least debuggable failure OAuth has.

init scaffolds the rewrite rule alongside the ones for the other well-known paths, so a generated project answers it without anybody adding a line.

On the tests

The existing discovery tests for this file assert against a copy of the array the method builds, rather than against what it renders — a shape that keeps passing if the method is deleted. The tests added here call the action and read the rendered document.

Three components that were nearly accessible

Field, Pagination and Datatable. All three were nearly right, which is why the gaps lasted: every page link already carried an aria-label, every field already had a real <label> with a for, every table already had proper headers.

What was missing in each case was a relationship — between a control and the text about it, between a run of links and the page's regions, between a table and the rows that arrive after the request finishes. A relationship is invisible when you can see the layout, because the layout is the relationship.

Form fields say what they mean

aria-describedby and aria-invalid did not appear anywhere in src/Pramnos/Html/.

A <small> under an input is not attached to it; it is a sentence that happens to sit nearby. Somebody using a screen reader heard the label and the control and never the explanation — so the field that needed the most explanation gave none. The input has carried an id all along, so only the association was missing.

Field::$error is new and is the rendering end of validation, not a second validator: Validation\FormRequest and View::$errors still own that. Set it and the field is marked aria-invalid, and the message is joined to the control:

$field        = new Field('email', 'email', 'Email', null, 'Your work address');
$field->error = 'That address is already registered.';
// → aria-describedby="email-error email-description" aria-invalid="true"

Three details that are deliberate. The error id comes first, because aria-describedby is read in order and a field that is wrong should say so before it explains what it wanted. The message carries role="alert" — it appears after a failed submit and has to interrupt to be heard — while the field does not, since an alert there re-announces everything each time anything changes. And a field with neither text gets no attribute at all: an aria-describedby pointing at an id that is not on the page is worse than its absence, because the reader announces the field and then nothing and the silence looks like its own bug.

Selects render down a different path in that class and get the same treatment, which is exactly how one of two paths otherwise ends up accessible and the other does not.

Pagination is a landmark

It had aria-label on every link and aria-current="page" on the current one — all of which help once you are inside it. Getting inside was the part that did not work: with no <nav> it was an anonymous run of anchors that region-by-region navigation passed straight over. Breadcrumb, immediately beside it, has been a landmark since it was written. The difference was an inconsistency, not a decision.

A caller who set containerElement to nav themselves gets the label on their own element rather than a second region wrapped around it — two nested navigation landmarks announce the region twice and neither is the answer.

A datatable says what it is, and that it changed

Datatable::$caption renders a visually hidden <caption>. Without one a table is announced as "table, 7 columns" and nothing else, so a reader moving between the several tables an admin screen carries cannot tell them apart. It is empty by default rather than guessed at: a caption invented from the table's internal name — dt-emails — is worse than none, because a reader hears it and believes it was written for them.

The rows are fetched, so the table is also aria-live="polite". Without it, sorting or paging is silence followed by different data under the same heading. Polite rather than assertive because a result set is not an emergency — it waits for the reader to finish its sentence.

The visually-hidden recipe already existed for the omnibox label; the caption shares that rule in all three scaffolded themes rather than adding a second copy of the same nine declarations.

The Model Context Protocol, over HTTP, for somebody else's assistant

POST /mcp — JSON-RPC in, JSON-RPC out, authenticated with an access token this server issued.

The infrastructure was almost all here already, which is the interesting part of this change. McpServer::dispatch() has always taken a JSON-RPC message and returned one; run() is only a loop reading STDIN around it. UnifiedAuthMiddleware has always validated a bearer token, loaded its scopes from usertokens and resolved the user. What was missing was a transport, a decision about which tools may be served, and the discovery document the previous entry adds.

Two registries, because the default has to be the safe one

The internal server carries nineteen development tools — coverage, the style checker, the framework's own docs — every one written for somebody who already has a shell on the machine.

Filtering a shared list would mean every tool written from then on is public until somebody remembers to exclude it, and the failure is silent and remote: a twentieth tool is added and it answers over HTTP before anybody notices it is reachable. So public exposure is a type, not a flag:

interface ScopedMcpTool extends McpToolInterface
{
    public function requiredScope(): string;
}

A tool that does not implement it cannot enter PublicRegistry, and no configuration overrides that. One that implements it and returns an empty scope is refused with an exception rather than skipped — a tool quietly dropped at boot is absent at run time for a reason nobody can find.

Refusing correctly is half the feature

An unauthenticated call gets 401 with WWW-Authenticate: Bearer resource_metadata="…/.well-known/oauth-protected-resource".

That header is the discovery mechanism: an MCP client calls blind, is refused, reads the document it is pointed at, finds the authorization server, and comes back with a token. A bare 401 ends the conversation.

The scope check is structural

The server is built per request with only the tools the caller's scopes reach. A tool the caller may not use is not in that server, so naming it directly answers unknown tool rather than forbidden — one decision instead of two that can disagree, nothing gained by guessing, and no confirmation that the guessed tool exists.

A token whose scopes cannot be read reaches nothing. "I do not know what this caller may do" is not a reason to serve everything.

SearchTool

The one worth shipping, because it needs no code from an application beyond the Registry::register() calls it already makes for its own search box.

Search\Registry was built for a box that several kinds of user share: each source declares a permission, each row is scoped by a filter callable that receives the current user, and both fail closed — a dropped source leaves no trace rather than an empty group, because an empty group named "Invoices" tells somebody who may not see invoices that invoices exist. An MCP caller is a signed-in person with a token, so it is the same question, scoped by the same code.

Its grouped-not-ranked shape suits a language model better than a flat list: "5 users, 3 orders" is a fact it can reason about, where a merged ranking invites it to read meaning into position.

It is not registered for you. Enabling an authenticated endpoint should not also decide what that endpoint exposes, even when the thing exposed is already permission-scoped:

PublicRegistry::add(new \Pramnos\Mcp\Tools\SearchTool());

init scaffolds POST /mcp when the authserver feature is on — not with auth, because an application that authenticates people but issues nobody a token has no caller for this.

Offering an MCP capability without writing a class

ScopedMcpTool is five methods. Most of what an application wants to offer is a name, a sentence and a closure, and a class file for three lines of logic is the reason capabilities do not get offered at all. So there is a short door:

PublicRegistry::offer(
    name:        'station-health',
    scope:       'user',
    description: 'Report the last successful stream check for a station.',
    input:       ['station_id' => 'integer', 'verbose' => 'boolean?'],
    handler:     fn (array $in) => Station::health((int) $in['station_id']),
);

That is a complete, authenticated, scope-gated tool.

The input spec, and why it has no wall

['station_id' => 'integer', 'verbose' => 'boolean?'] is the same document as fourteen lines of nested arrays, and the fourteen lines are where people stop. A trailing ? marks a parameter optional; everything else is required, because required-by-default is the safer mistake — a model omitting something the tool needs gets a clear refusal, where a tool quietly running without it produces a wrong answer nobody questions.

The compact form is not a dialect anybody has to stay inside. A single property can be spelt out in longhand among the shorthand, and a spec carrying a top-level type passes through untouched — so enums, patterns and nested objects are written as ordinary JSON Schema and nothing interferes. PublicRegistry::schema() is public, so a tool written as a class can use the short form too.

Both doors, one set of rules

The short door refuses a tool with no scope exactly as the long one does, with the same exception. Two ways in must not mean two sets of rules — that is how the convenient one becomes the one that skips the check.

Write the class when the tool has state, needs injecting, or is worth unit-testing on its own. Neither door is the real one.

A page head that stops claiming things the page never said

Five findings from an SEO/GEO review of what the framework renders, all in the same place.

Empty meta tags were emitted unconditionallydescription, og:title, og:url, og:site_name, og:description. content="" is not the absence of a description. It is a claim that this page has none, and a crawler records the claim rather than falling back to the page text. An application that never set the property said nothing of the sort.

It is the rule Seo::jsonLd() has documented as absent is not empty since it was written, and the rule this renderer broke on every page it produced.

Emptiness is judged after escaping rather than before: escapeHeadValue() already refuses anything that is not scalar and answers with a blank, and casting first would have turned that safe answer into a page announcing that its description is Array.

viewport moved onto the document. It lived in the scaffolded themes and nowhere else, so a theme that omitted it produced a page Google labels not mobile-friendly, with no signal anywhere that anything was missing. It is a property of an HTML document, not of a decoration around one.

twitter:card is emitted only when there is an image for it. X reads the OpenGraph tags for everything else; a large-image card promising an image the page does not have renders worse than no card at all.

xmlns:og and xmlns:fb are gone — RDFa declarations from 2010, parsed by nothing since Facebook moved to <meta property>, occupying the first hundred bytes of every page this framework has ever rendered.

And the site now says what it is

Html\SiteIdentity puts Organization and WebSite on every page. Until now the only @type anywhere was BreadcrumbList — the shape of a page's position in a hierarchy that was never named. Document::addStructuredData() existed the whole time and the framework called it from nowhere.

It asserts only what is configured, for the same reason the meta tags now do: structured data is a set of assertions and an empty one is a false one. An unset logo is absent, not "". A malformed entry in sameAs is dropped rather than published, because it does not fail loudly — it quietly stops the organisation matching the entity it names. A SearchAction appears only where Search\Registry actually has sources, since one leading to a page that returns nothing is offered to a reader, tried once, and teaches them the site is broken.

What the site tells a machine that arrives uninvited

/robots.txt and /llms.txt, both generated rather than shipped as files — the one line that matters in each is derived from the installation's own URL, and a static file in a scaffold is a static file with somebody else's domain in it.

robots.txt, and why the AI crawlers are named one by one

Twelve of them: GPTBot, ClaudeBot, PerplexityBot, Google-Extended, Applebot-Extended, CCBot and the rest. Every one reads robots.txt and honours it.

Absence is not neutrality. With nothing said, each crawler decides for itself, and they decide differently — Google-Extended opts a site out of model training while leaving Search untouched, which is a distinction a site cannot express by staying silent. The default is allow, because a framework must not choose a site's licensing posture; what it must do is make the choice visible and settable, which absence never did. One setting flips all twelve.

The paths with side effects are Disallowed rather than left to noindex: noindex keeps a page out of an index after it has been fetched. On an authentication server every one of those pages either costs a session, sends mail, or answers differently per visitor.

llms.txt

The GEO counterpart of a sitemap: a short markdown document saying what this site is and where the things worth reading are. A crawler follows links; a model arriving cold guesses, and guessing is how a site gets described wrongly and confidently.

It is deliberately short — the format's premise is that it fits in a context window beside the question somebody actually asked.

And it is where the MCP endpoint is announced, which is the one honest connection between these two pieces of work. A model reading it learns the site has tools it can call and where to authenticate for them. Without it the endpoint is a service nobody discovers — machinery complete and unreachable, which is the failure this framework keeps producing. It is announced only when something has actually been offered, because an endpoint serving an empty list is not worth pointing anybody at.

Two more from the same review, both in the scaffolded themes — which means both were in every site generated from them.

The first thing any accessibility audit checks, and the <main> landmark it needs has been in these themes all along. Only the link to it was missing, so reaching the content of a page meant tabbing through every navigation item, on every page.

Positioned off-screen rather than hidden: display:none takes an element out of the tab order, which removes the one thing a skip link exists to provide. It appears on focus.

The avatar fell back to Gravatar when an account had none of its own:

$this->user->avatar ?? 'https://www.gravatar.com/avatar/' . md5(strtolower(trim($this->user->email)))

So every render of /oauth/authorize sent md5(email) of the person signing in to another company — from the one page in the whole flow where they are deciding what to disclose, and to a party they were never asked about.

An md5 of an address is not anonymous. It is the address, hashed: the set of email addresses that matter is small enough to enumerate, and Gravatar's entire product is the lookup. This was found during an SEO review, which is where an accessibility and privacy pass tends to find things — nobody goes looking for it in the file it is in.

The avatar is now rendered when the account has one, and omitted when it does not.

The test for it strips PHP comments before asserting, because the comment explaining why the fallback is gone names Gravatar — and a test that cannot tell an explanation from the thing it explains is a test that punishes writing the explanation down.

A token outlived the client that issued it, and kept working

fk_usertokens_applicationid was created with ON DELETE SET NULL. Delete an OAuth client and its tokens were not removed — their applicationid became NULL, and a token with a null applicationid is exactly what a token issued outside OAuth looks like. Each one silently changed category from issued by this client to not an OAuth token at all, and carried on authenticating.

Measured on a working installation before the change: 507 of 522 tokens had a null applicationid, and thirteen of those were still active and unexpired. Thirteen live credentials belonging to clients that had been deleted.

SET NULL is a reasonable default for a column that annotates a row. applicationid does not annotate — it answers who may use this token, and on whose behalf. Removing the answer does not retire the question; it makes it unanswerable while leaving the token valid. And deleting a client is the one action an operator takes precisely to stop it having access.

CASCADE is what the neighbouring constraints already do for the same reason: fk_tokenactions_tokenid cascades from usertokens, fk_usertokens_userid cascades from users. The application is the same kind of parent.

Rows that are already detached are left alone, deliberately. A token whose applicationid is already NULL cannot be traced back to a client — the old rule destroyed the reference rather than recording it anywhere. Nothing separates "was issued by a client that is gone" from "was never an OAuth token", so a sweep would have to guess, and guessing here revokes working credentials. This only stops more being made.

The constraint is dropped and recreated, because neither PostgreSQL nor MySQL can alter an existing foreign key's delete rule — there is no ALTER CONSTRAINT for it.

An error log that was mostly not errors

MassMessagesController offers two optional filters: account groups and organizations. Both belong to features an installation can be built without, so both are deliberate feature gates — a screen that refused to render because one optional filter has nothing to offer would be the wrong answer, and that part was always right.

What was wrong was how the question was asked. Letting the query fail and catching it means the database layer logs relation "usergroups" does not exist at error level on every render.

The cost is not the disk. It is that the log stops being usable. On one installation it held 106 errors over thirty days, and most of them were designed-for conditions — so finding a real problem meant investigating every line to discover whether it was one. That is not a log, it is a pile.

hasTable() asks the same question without raising anything. The try blocks stay, because a table can exist and still be unreadable, but they stop being the normal path.

Seventeen log entries per suite run became zero.

A client secret nobody had to present

Application::validateCredentials() added the apisecret condition to its query only when a secret arrived:

if ($clientSecret !== null) {
    $qb->where('apisecret', $clientSecret);
}
return $qb->count() > 0;

A request that omitted client_secret therefore matched on apikey + status alone, and every active application authenticated with no secret at all.

Why it was reachable

Not through a corner of the API — through the front door. league/oauth2-server 8.5 resolves an absent client_secret to null, and AbstractGrant::validateClient() hands that value to the client repository without examining it. The repository asked this method, and this method said yes.

So:

POST /oauth/token
grant_type=client_credentials&client_id=<apikey>&scope=<allowed>

with no secret returned an access token. client_id is a public identifier by design: it travels in redirect URLs and ships inside every SPA bundle and mobile binary. The same path is taken by authorization_code, refresh_token and password, which all route through AbstractGrant::validateClient().

ClientRepository::validateClient() carried a docblock promising the check — "Public clients (secret=null) are accepted only when the application is configured as non-confidential" — that no code performed. Application::isConfidential() returns a hardcoded true, so nothing else was going to catch it either.

Fixed

The application row is read once and the secret compared against the stored value with hash_equals(). A registered secret must be presented and must match; the comparison is constant-time and no longer travels into a WHERE clause.

A client whose apisecret is empty is left exactly as it was — nothing is registered for it, so nothing is asked of it — but it accepts only a request that presents no secret either. Accepting an arbitrary string there would have been looser than the version this replaces, where a non-empty secret simply failed to match an empty column.

Tests

tests/Integration/Auth/OAuth2ClientSecretRequiredTest.php pins the contract against a real database: registered secret absent, registered secret empty, correct secret, wrong secret (including a prefix of the real one), no secret registered, disabled application, unknown client_id, and another client's secret. Verified to fail against the previous implementation before the fix landed.

One existing assertion had pinned the bug as intended behaviour — assertTrue($app->validateCredentials('test_key', null)) in tests/Unit/Pramnos/Auth/ApplicationTest.php — and now asserts the opposite.

Still open

isConfidential() returning a hardcoded true means the public column on applications is not consulted, and a public client is indistinguishable from a confidential one. That is a separate decision about how public clients should be registered, not a fix that belongs beside this one.

The SMTP password was readable to anyone who could read the database

smtp_pass sat in the settings table as plaintext. Anyone who could read that table could send mail as the operator: a leaked backup, a dump handed to a contractor, SQL injection in some unrelated endpoint, a hosting neighbour, a DBA.

Added: Pramnos\Security\Encrypter

Authenticated encryption for values that have to be stored and read back. NaCl secretbox (XSalsa20-Poly1305) via libsodium — the primitive ChannelEncrypter already uses, rather than a second cipher for the same job — keyed from APP_KEY.

$stored = Encrypter::encrypt($value);      // "enc:v1:…"
$value  = Encrypter::decrypt($stored);
$value  = Encrypter::maybeDecrypt($row);   // plaintext passes through unchanged

maybeDecrypt() is the whole migration story. A value without the enc:v1: marker comes back as it is, so a column can be read through it from the first deploy and converts itself as rows are rewritten. No migration script, no downtime, no window where half the rows are unreadable.

Hash what you verify, encrypt what you use

The distinction the class exists to hold. A secret the application only ever checks — a password, a 2FA backup code — is hashed, and nothing should be able to recover it. A secret it has to use — an SMTP password, a signing key, a TOTP seed — has to be recoverable, so encryption is the only option available.

Encrypting the first kind would be a downgrade dressed as extra safety: APP_KEY lives in .env beside the database credentials, so a reversible secret hands back what a hash never would.

What it defends against

Every way a database is read without the filesystem. Not an attacker who owns the host — they read .env. That distinction belongs in any compliance answer that uses the phrase "encrypted at rest".

Fixed: smtp_pass

Settings now names it in ENCRYPTED_SETTINGS and encrypts on write, decrypts on read. Every caller is unaffected: getSetting('smtp_pass') returns the plaintext it always returned. An existing row converts itself the next time the settings screen is saved, and with no APP_KEY the value is stored as before — a settings screen that refuses to save is worse than the problem it would be avoiding.

A value that will not decrypt (after an APP_KEY rotation) yields the setting's default rather than the ciphertext, so the failure reads as "no password configured" instead of arriving at an SMTP server as a password.

Tests

tests/Unit/Security/EncrypterTest.php — round trip including UTF-8, binary and 4 KB values, nonce freshness, tampering, wrong key, truncation, non-base64 payload, missing APP_KEY, and the non-base64: key forms. 100% line coverage of the class.

tests/Unit/Application/SettingsEncryptionTest.php for the boundary, and tests/Integration/Application/SettingsEncryptionAtRestTest.php which asserts on the column itself — a broken implementation that stored plaintext and returned plaintext would satisfy every API-level test there is.

Also fixed

SettingsPostgreSQLTest::testBulkLoadReadsEverySettingOnPostgreSQL had been failing on a pristine tree. The bulk read is cached for 300s under the settings category, keyed on a query that does not change when setUp() drops and recreates the table, so a previous run's empty result answered for the next five minutes. Only setSetting() invalidates that category, and the test inserts its rows around it. It now flushes the category after seeding.

Still open

oauth2_webhooks.secret_key and user_twofactor.secret are the same shape of problem. See the next entry.

The webhook signing key and the TOTP seed, likewise

The two remaining plaintext credentials, now encrypted through the same Encrypter. Both are recoverable secrets — the application needs the actual bytes — so encryption is the only option available for either.

oauth2_webhook_endpoints.secret_key

The HMAC key every delivery is signed with. A copy of it forges a webhook the receiver accepts as ours, correctly signed: this user's permissions changed, this token was revoked. It is already write-only through the API — register returns it once and nothing re-displays it — so encrypting the column changed no caller.

WebhookService::deliverEvent() reads through maybeDecrypt(). A key that will not open fails that delivery attempt with a stated reason rather than signing with the ciphertext, which would have surfaced as the receiver rejecting a forged-looking request and sent the operator to the wrong side of the connection to debug it.

user_twofactor.secret and twofactor_setup.temp_secret

The seed every TOTP code is derived from. A copy is a permanent bypass of the second factor for that account — no expiry, and nothing the user would ever see. The in-progress enrolment seed is the same secret at an earlier moment, so both columns are covered; the dev panel's pending-enrolment view decrypts too, since a developer wants a seed they can type into an authenticator.

Backup codes were already hashed with PasswordHash::make() and stay that way. They are checked, never used, so a hash is the stronger answer and encrypting them would have been a downgrade.

A migration, found by the test

user_twofactor.secret and twofactor_setup.temp_secret were VARCHAR(64) — ample for a 32-character base32 seed, and 45 characters short of the encrypted form. MySQL refuses that write outright:

Data too long for column 'temp_secret' at row 1

which is how it was found: the integration test asserting the column holds ciphertext could not write one. 2026_08_31_000001_widen_totp_secret_columns.php widens both to 255 — room for a later format without another migration. Widening only: no value changes, nothing truncates, and down() deliberately does nothing, because narrowing back would truncate a seed and lock an account out of its own second factor.

Its priority is 90, above the 80 and 85 of the tables it alters. At 65 it ran first, found no column, did nothing, and left the old width in place — the migration runner orders by priority, not by filename date.

Tests

At-rest assertions on the columns themselves, on both drivers for the 2FA seed (TwoFactorAuthServiceMySQLTest, TwoFactorAuthServicePostgreSQLTest), because the part that varies per driver is the widening migration. Each also proves the factor still works end to end: enrol, complete, getSecret() returns the seed as enrolled, and a live code verifies.

A seed enrolled before encryption is asserted to still verify afterwards. Getting that wrong locks every existing 2FA user out at once, so it is asserted rather than assumed.

WebhookSubscriptionTest covers the signing key, including the no-APP_KEY case: registering an endpoint must not fail because a key was never generated.

A tenant scope that held until you asked for page 1

OrmModel overrode _getList() to apply the soft-delete filter and the registered global scopes. It did not override _getPaginated() or _datatablesRecordsTotal() — and those are exactly what _getApiList() calls the moment a page is requested, which is what every REST list endpoint, generated CRUD screen and datatable does.

So the same model, with the same scope registered, answered two different questions depending on how it was asked:

Post::addGlobalScope('tenant', fn($f) => …' tenant_id = ' . Auth::tenantId());

$post->_getApiList(['id', 'title']);                       // scoped
$post->_getApiList(['id', 'title'], '', '', '', '', '', null, null, 1, 20);  // not

The second returns every tenant's rows, and reports a total counted over the whole table. That is a leak plus a pager offering pages that come back empty, plus a disclosure of how many records the other tenants hold — from one missing override.

HasScopes documents global scopes with a tenant example, so this is not a hazard somebody had to go looking for: an application scoping its models the documented way got the scope on its admin screens and none of it on its API.

The second bug, found on the way

mergeSoftDeleteFilter() wraps its input in parentheses, and ApiListQuery hands the list methods a filter that already begins with where. Together:

(where status = 'x') AND (deleted_at IS NULL)

A syntax error — returned as an empty result set, with the message buried in the response envelope's error key. Any soft-deleting model listed with a filter came back empty and said nothing about why.

Fixed

One mergeListConditions() helper on OrmModel — strips a leading where, merges the soft-delete filter and the scopes, returns null when empty — used by _getList() and by new overrides of _getPaginated() and _datatablesRecordsTotal(). Nothing was added to the ApiListSource interface: a new method on a published interface would break any application class implementing it directly.

The count merges and then calls parent::_getPaginated() rather than the base _datatablesRecordsTotal(), which would route back through the override and apply every global scope twice.

Tests

tests/Integration/Application/OrmScopesApplyToApiListTest.php — six tests against a real table. Two of them fail on the previous implementation, which is how the shape of this was pinned down: the paginated list returned the other tenant's rows, and the unpaginated list with a filter returned nothing at all.

The rest hold the edges: a caller's filter is combined with the scope rather than replacing it (otherwise any endpoint accepting a filter is a way to shed the tenant condition), a model with no scopes still lists everything, and _getList() and _getApiList() are asserted to agree — the property the whole change is about.

Known limit

A local scope queued with applyScope() is consumed by the first query it reaches, so on a datatables request it applies to the page and not to the total. Global scopes and soft deletes are re-derived per call and are unaffected. Tenant isolation belongs in a global scope; the ORM guide now says so.

Documentation

The ORM guide's Scopes section described a fluent $query->where(...) API that this framework does not have — Laravel's, not HasScopes'. Rewritten against the real filter-string API, with a section on where scopes apply and why that is a security property rather than a detail.

A grant on one record opened the whole collection

object_id is the permission store's only per-record mechanism, and the loop behind ApiCrudController::authorize() ignored it. Every grant matching (object_type, action) counted, whatever record it named.

So a grant written as read invoice 42 — the careful, narrow thing an administrator reaches for when they mean one record — was read by the generated endpoints as read invoices, and list returned all of them.

It was wrong in the other direction too. A deny on invoice 42 matched the same way, so denying one record denied the list for everybody.

Fixed

A grant now applies where it was written to:

Grant endpoint question invoice 42 invoice 43
object_id NULL or * allow allow allow
object_id = 42 no rule allow no rule
object_id = 42, deny no rule deny no rule

"No rule" rather than a denial, because authorize() is deliberately three-valued: a project that has granted nothing has to keep working. So this narrows what a grant opens without locking anybody out of anything they had.

And a way to ask the record question

authorize(string $action) is not given an id — it asks about the endpoint, which is all it can see. permissionForObject($action, $objectId) is the second question, for a controller that has the id:

// In a generated controller, where the action already has the id.
public function read($id): mixed
{
    if (($denied = $this->guard('read')) !== null) {
        return \Pramnos\Http\Response::json($denied, $denied['status']);
    }

    if ($this->permissionForObject('read', (string) $id) === false) {
        return \Pramnos\Http\Response::json(
            ['error' => 'forbidden'], 403
        );
    }

    // …
}

authorize()'s signature is untouched. Adding even an optional parameter to it would have broken every generated controller that already overrides it — PHP rejects a child declaring fewer parameters than its parent.

Tests

tests/Unit/Application/ApiCrudObjectScopedGrantsTest.php, 14 tests over a resolver reading canned grants — reached through a new permissionResolver() seam, the same pattern ClientRepository::makeApplication() already uses, so the matching rules are tested rather than an authserver schema.

Four of them fail when the object-matching is removed, which is how the old behaviour was pinned: the record grant opening the collection, the record grant answering for a different record, and both directions of the deny.

Session::isHttps() read $_SERVER['HTTPS'], which describes the connection this process received. Behind a load balancer or reverse proxy that terminates TLS, that is the plaintext hop between the proxy and PHP: the browser is on HTTPS and the variable is empty.

The session cookie's secure flag is set from that answer. So on the most ordinary production topology there is — anything behind nginx, HAProxy, an ALB, Cloudflare — the cookie was issued without it, and then travelled on any http:// request to the domain.

Fixed, without opening a worse hole

X-Forwarded-Proto is consulted, but only when the peer is in trusted_proxies — the same list ClientIpResolver already uses for clientIp(). The header is client-supplied: believing it unconditionally would let any visitor assert https and be handed a cookie marked secure over a plaintext connection, which is worse than the bug, so the untrusted case has a test of its own.

With no proxies declared the answer is $_SERVER['HTTPS'] alone, exactly as before. The header may also arrive as a list — https, http, two proxies deep — and the first entry is the one the client spoke.

The legacy CSRF check compared tokens with ===

Session::checkTokenValue() compared the submitted token to the session fingerprint with ===, which returns as soon as two bytes differ.

The token is not weak: it is an HMAC-SHA256 keyed by the session's own 256-bit random token, so it cannot be predicted from the user agent and IP it hashes. An external report described the fingerprint itself as predictable, which it is not. What was wrong is narrower and still worth fixing — a comparison that leaks how far it got has no business in a security check, and the argument for leaving it is only that exploiting it would be difficult.

Now hash_equals(), matching verifyCsrfToken() on the synchronizer path, which was always constant-time. A non-string is refused rather than coerced: a request that sent token[]=x has not submitted a token, and hash_equals() would raise on it.

The legacy path is not a deprecated corner — the account controllers, the settings form and the scaffolded templates all still emit it, so it is what a new project gets. It had no direct test coverage at all, which is how the === stayed. It has six tests now, including that the hidden field and the check agree, which nothing asserted before.

A LIMIT built by string concatenation

User::getFeed($limit = 10) put its own parameter into the SQL by concatenation:

. "order by `date` desc limit " . $limit

$limit is a public parameter. A controller forwarding a request value — the obvious thing to write for a "load more" endpoint — hands a visitor the end of the statement.

Cast to int now, with a floor of 1. The friend-id IN list beside it is cast too: those values come from the database and were safe, but they were safe by where they came from rather than by anything the line does.

An external audit flagged the IN list and missed the LIMIT.

getFeed() is left on prepareQuery() rather than converted to the query builder. Rule 12 would convert it; rule 2 wants characterization tests over User first, and getFeed() is the one method those tests deliberately exclude — UserSocialFeaturesCharacterizationTest says why. Converting behaviour nobody has pinned down is how it changes by accident.

addslashes() as an escaping fallback, and as a PHP-literal encoder

Two unrelated uses, both wrong, both removed.

As a last-resort escape

Database::prepareInput() fell back to addslashes() when neither driver's escape function was available. That is unsafe in two ways at once: addslashes() knows nothing about the connection's character set — in GBK and other multibyte encodings that is the well-known %bf%27 bypass — and it fails open, returning a string that looks escaped and is not.

A missing extension is a broken installation, not a state to degrade into; there is no connection to escape against either. It now throws.

The old third branch was a separate bug: on PostgreSQL without pg_escape_string it called mysqli_real_escape_string() with a PostgreSQL connection handle, which is a TypeError, not an escape.

As a PHP-literal encoder

Init.php used addslashes() to embed the admin's credentials into a generated PHP snippet — not into SQL. addslashes() escapes the double quote, and inside a single-quoted PHP string \" stays a literal backslash followed by a quote:

$safe = addslashes('pa"ss');   // pa\"ss
$x = 'pa\"ss';                 // → pa\"ss, not pa"ss

So an admin password containing " was written to the database with a backslash in front of it, and the operator could not log in with the password the installer had just printed to their terminal. Now var_export(), which emits a real PHP literal — quotes included — and round-trips quotes, backslashes and UTF-8 unchanged.

The README described a framework this is not

It asked for PHP 7.4 and ext-pdo. composer.json requires >=8.1, does not mention PDO, and the Dockerfile is php:8.5-apache; the database layer calls mysqli_* and pgsql directly and has never used PDO. A reader who took the README at its word would install the wrong extension and target a version the code does not run on.

Corrected, along with the contributing guidelines, which still told contributors to keep src/ PHP 7.4-compatible, and the Docker section, which said 8.4.

The schema promised tenant isolation nobody implements

No code changed here. Two comments and a guide did, and this is the finding with the worst consequences of anything in today's list, because it is the one that makes somebody build on a guarantee that is not there.

authserver.user_roles carried the table comment "supports temporary and org-scoped grants" and a docblock saying assignments "can be scoped to an organisation". There is no organisation column on that table. The row is (userid, roleid) and nothing else.

user_organizations opened with "A user must be a member of an organisation before they can be assigned any organisation-scoped role… This mirrors the GitHub organisation membership model". Nothing enforces that. No foreign key, no code path.

And underneath both: PermissionResolver has no organisation dimension at all. It scopes by application (permissions.app_id) and returns every active role a user holds, whatever organisation that role names. A role scoped to organisation A grants its permissions everywhere.

The pieces are real — organizations, user_organizations, the org column on roles — which is what makes the shape convincing. They are storage. Nothing reads them for an access decision.

What changed

Both comments now say what is true, and say where the reader has to look instead. user_roles' table comment is also corrected, though the migration is hasTable()- guarded and will not re-run, so an existing installation keeps the old text — which is why the docblock is where the correction is really aimed.

The authorization guide gains a Multi-tenancy section: what the framework does not do, and the two places isolation actually belongs — a global scope on the models for reads, an ABAC condition on the grant for authorization. Plus the warning that follows from combining this with authorize()'s deliberate "no rule means allowed": a scaffolded multi-tenant API is open by default, so the scope goes in before the endpoints are generated, not after.

Not fixed here

Giving PermissionResolver an organisation dimension is a design decision, not a bug fix — it changes what a role means. Recorded rather than quietly done.

Joins on two columns, and aliases that survive resolution

Two QueryBuilder limitations, fixed together because they are useless apart.

A join could only say one thing: join($table, $first, $operator, $second). A join on two columns — a composite key, a membership row matched on both the user and the organisation — had no expression at all short of joinRaw(), which takes the SQL as given and resolves no table names.

And the moment a join was given an alias, the qualified name stopped being resolved: the check that turns authserver.roles into its physical name skipped anything containing a space. On MySQL that reaches the driver as a reference to a schema that does not exist. Aliases are exactly what a multi-condition join needs, so either fix alone would have been half of one.

use Pramnos\Database\JoinClause;

$qb->table('authserver.user_roles ur')
   ->join('authserver.roles rd', 'rd.roleid', '=', 'ur.roleid')
   ->leftJoin('authserver.user_organizations uo', function (JoinClause $join) {
       $join->on('uo.userid', '=', 'ur.userid')
            ->on('uo.organization_id', '=', 'rd.organization_id');
   });

on() ANDs, orOn() ORs, on('a.x', 'b.x') means equality. Both sides of a condition are column references and there is deliberately no where() on a JoinClause: a comparison against a value belongs in the query's WHERE, where it is bound rather than pasted into the ON. A join builder that quietly accepts user input is the bug this avoids.

Permissions can now be resolved within one organisation

authserver.roles.organization_id has been in the schema from the beginning, NULL meaning "system-wide". Nothing read it. resolve() returned every active role a user held whatever organisation it belonged to, so a role defined for organisation 5 decided questions about organisation 3's data — and an application built on the framework's RBAC had no tenant isolation whatsoever.

resolveForOrganization($userId, $appId, $organizationId) asks the scoped question. A role counts when it is system-wide, or when it belongs to that organisation and the user is an active, unexpired member of it — the rule the user_organizations migration described from the start and nothing enforced.

resolve() is untouched: unscoped, exactly as before, because applications depend on it and it is the right question when there is one tenant.

Membership is read the way the admin screen writes it

OrganizationsController::removemember() sets is_active = 0 rather than deleting, to keep the audit trail. A join that only asked whether the membership row existed would therefore have left every former member's access exactly where it was — the screen would say "removed" and nothing would have been. expires_at is honoured for the same reason: a temporary membership whose window has closed is not a membership.

Leaving deletes nothing. The role assignment stays and stops counting; rejoining restores the same set. Asserted rather than assumed, because nothing does anything on leaving — which is the point, and exactly what a later "tidy up orphaned roles" change would undo without noticing.

Why one join rather than two queries

Measured, on 500 users holding 50 of 400 roles, MySQL in the container:

ms/op
today, no organisation 0.105
join roles, filter in SQL 0.138
+ join user_organizations 0.154
memberships already in memory, filter in PHP 0.141
separate memberships query, filter in PHP 0.213
membership as one PK lookup, then join 0.226

The membership join costs 16 µs over filtering by organisation alone. Reading the memberships separately costs 59 µs more than the join saves — the round trip dominates, twice over. Filtering in PHP wins only if something has already loaded the memberships for free, and nothing in the request path does; 13 µs does not justify an API for callers to pass them in.

Opt-in, deliberately

Nothing calls the scoped method for you. ApiCrudController, Gate and Permissions are not given an organisation and cannot guess one. The authorization guide's multi-tenancy section now says which method answers which question, and that for reads the answer is usually a global scope on the model rather than an authorization call at all.

Also

AuthCharacterizationTest has been failing on its own and passing in a full run, which is the least useful combination a test can have: it took whatever the previous test left in the Database singleton. It loads the fixture settings and takes a real connection now.

Roles had two tables, a permissions screen, and no way to make one

authserver.roles and authserver.user_roles ship with the auth feature's migrations. PermissionsController will grant a permission to role 7. Nothing in the framework could create role 7, or give it to anybody.

So an installation that enabled the feature got two thirds of an RBAC system: the storage, and the half of the UI that assumes the other half exists. Which is also why the membership rule the user_organizations migration described had never been enforced anywhere — there was no write path to enforce it in.

Added

Pramnos\Auth\Role, and a RolesController with the screens the rest of the admin area has: a list, one role with its permissions and holders, a create/edit form, and a holders screen that adds and removes people. Wired into the nav beside Permissions (usertype ≥ 90, authserver feature), into the scaffolded route prefixes, into project:publish-views, and into the breadcrumb map of all three themes — twelve view files, because a screen that only exists in one theme is not a feature.

The rule, in the model

Role::assignTo() refuses a role belonging to an organisation the user is not an active, unexpired member of, and says so in words an administrator can act on. It reads membership exactly as resolveForOrganization() does, so a screen cannot grant something the resolver would then quietly ignore.

It lives in the model rather than the controller because an API caller reaching the model directly has to get the same answer as somebody clicking the button.

What does not delete

Revoking a role deactivates the assignment. Deactivating a role deactivates the role. Both stay readable and the resolver ignores both. Deleting a role does remove its assignments — a row naming a role that no longer exists is not history anybody can read.

And a role's organisation cannot be changed once anybody holds it: the holders who are not members of the new organisation would silently lose it, which is a permission change disguised as an edit. The form says so and the save refuses.

Tests

tests/Integration/Auth/RoleAssignmentTest.php — the organisation rule in both directions, membership that is inactive or expired, idempotence, re-activation, the audit columns, and the deliberate non-behaviour: leaving an organisation leaves the assignment exactly where it is.

Both new integration suites build every shared table from its migration. Two hand-rolled copies that disagreed about whether granted_by exists is exactly the confusion that produces, and it cost a debugging round here before the rule was adopted.

Also

AuthCharacterizationTest builds the authserver.permissions table it writes to. Depending on some other suite having created it is what made the class order-sensitive — passing in a full run, failing on its own.

The client secret is hashed, and the realtime key moves out of its way

applications.apisecret sat in the database as the value itself. Anyone who could read that table could impersonate any OAuth2 client and mint tokens as it.

Hashed now — PasswordHash::make(), the same treatment a password gets — because the server only ever verifies it. Encryption would have been the weaker choice: APP_KEY lives in .env beside the database credentials, so a reversible secret hands back what a hash never does.

The realtime key had to move first

broadcast_secret was added on 2026-08-20 precisely so the realtime HMAC key and the client secret would stop sharing a value — that migration's own words, "the exposure profiles differ". It was nullable with apisecret as the documented fallback, and nothing ever wrote it. No controller, no backfill. So in practice every application's realtime key still was its apisecret, and the separation existed only as a column.

An HMAC key cannot be hashed: the sender needs the actual bytes. Hashing without splitting first would have broken channel authorization for every application at once, in a way that surfaces as subscribers failing to connect and points nowhere near the cause.

So split_broadcast_secret_from_apisecret copies the current value across, keeping the key each application already has — nothing to re-issue, no subscriber notices. It deliberately does not generate a fresh one: a migration is the wrong place to rotate a credential. AuthServerAppRegistry then reads broadcast_secret only, and the fallback is gone; the column is encrypted at rest, since unlike the client secret it has to be read back.

No rotation, no downtime

A row still holding a plaintext secret is compared in constant time as before, and re-hashed in place after it verifies. An installation converts itself one successful authentication at a time. A failed guess converts nothing, or a wrong secret would overwrite the row with a hash of itself.

What changes for an operator

The client secret can no longer be read off the application's screen, because there is nothing there to read. It is shown once, in the message that follows creating or rotating it, and the screens say so. Rotation used to report only that a new secret had been generated — fine while the value could be looked up afterwards, useless now.

A public client can say it is one

Auth\Application::isConfidential() returned a hardcoded true, so a single-page app and a server-side integration were indistinguishable. Every client had to have a secret — and since the token endpoint began requiring one, that means an SPA has to embed one. An embedded secret is not a secret.

is_confidential (default 1) records the difference, and isConfidential() reads it. Unticking Client Type on the applications form registers a public client: PKCE for the authorization code, and client_credentials refused, because that grant authenticates the application with nothing but a secret.

Not the public column

An earlier reading of this said to use the existing applications.public. That column means publicly listed — 0 = private, 1 = shown in a directory. Driving client authentication from it would mean the first person to tick "list this app" silently turned off its client authentication. A separate column, and the model now documents which is which.

The roles screen had a menu entry and no address

registerAdminNav() registers admin.roles unconditionally. The administration area resolves src/Admin/Controllers before the framework's own, so a project with no class there gets a menu entry that answers 404.

Found by an application's own test — a check that walks every link the navigation offers and reports the ones that do not open. It said /admin/Roles => 404, which is a sentence this framework's own suite could never have produced: nothing here routes through a generated project.

init scaffolds the wrapper now, the way it already did for Applications, Tokens, Users and the rest.

And a check for the general case

The specific fix is one file. The mistake is structural: register a nav item, forget the wrapper, and each half looks finished on its own — the link is drawn, the screen exists, and only a request joins them.

So there is a test that reads every $admin('…') target out of registerAdminNav() and asserts that init scaffolds a controller for it. It is deliberately tolerant of two things it would otherwise report as faults: the nav names a screen the way a URL does (users) while the file is Users.php, and Health lives in src/Controllers on purpose, because /health/check is what a monitor calls and has to answer without a session.

A guard that read a variable that did not exist (FW-047)

Logs\Logger::log() sniffs a message for JSON, but only — supposedly — when the caller has not already said what the entry is:

if (!isset($content['type'])) {   // the parameter is $context

$content does not exist at that point. It is assigned further down, inside the startoffile branch, and holds a file's contents as a string. So isset() was false and the guard passed on every single call.

It lasted because isset() never warns about an undeclared variable. The line looks like a check and behaves like nothing.

The consequence that mattered

Not the wasted json_decode on every log write. It is that a type the caller supplied was overwritten with 'json' whenever the message happened to parse as JSON — and a bare number parses, and so do true, null and a quoted string:

Logger::log('42', 'push', context: ['type' => 'error']);   // filed as json

So an error logged as '42' was filed as JSON, and a reader filtering the log by type never saw it. The class of bug this framework spent the day on: a failure whose only symptom is an absence.

Reported by an application migrating onto this framework, which keeps a register of findings against it. Three of its six open items turned out to be already implemented and one was fixed on 29 August — this was the one still standing.

On the test

Its first version set the output mode in setUp() and then set it to a different fixed value in tearDown(), because null — meaning "resolve from the environment each time" — is not a value setOutputMode() accepts. Three unrelated tests that read their own log files started failing. It saves and restores the private static by reflection now: restoring exactly what was there is the only version that does not leak.

A pager that could not be told to stop counting (FW-048)

Html\Pagination had displayFirstLast, displayNextPrevious and displayEdgePages — and no way to suppress the row of page numbers. $out .= $this->numbers() was unconditional.

displayPageNumbers (default true) leaves only previous and next.

Why that is a design case and not a preference

In search results the page count is large and moves with the filters. A reader on page 7 of 40 cannot say what page 7 is, and after narrowing the query it is a different page 7. Two links are the entire meaningful interface there, and a row of twelve numbers is noise that changes width as you read it.

It is distinct from displayEdgePages, which only decides whether 1 and the last page are pinned inside that row. This removes the row.

And nothing to show is nothing to render

With the numbers off and neither button pair enabled, every branch is skipped and what was left was an empty <nav>. An empty landmark is worse than no landmark: it appears in a reader's list of regions and leads nowhere. Same rule as the single-page case, reached a different way.

On where this came from

Reported by an application migrating onto this framework. Its own pager is a 421-line fork, and this one switch was what kept it alive — two call sites set it, and the third already uses this class and works.

The report also said what it did not want: the fork's displayButtons, which renders <button onclick="window.location.href=…"> instead of an anchor. Not crawlable, does not open in a new tab, needs JavaScript to work at all. Declining to import a bad feature along with a good one is the harder half of accepting a request.

Two components no theme could style

The convention across the components is that markup carries a neutral pf-* hook and each scaffolded theme's stylesheet marries it to that theme's look. Twenty-three names work that way already, with thirty-odd rules per theme — pf-omnibox, pf-skip-link, pf-visually-hidden.

Two components were outside it, in opposite directions.

Breadcrumb emitted class="breadcrumb" — Bootstrap's own name, as a literal. It reads as neutral and is not: a project on Tailwind got an element carrying a name nothing in its stylesheet defines, and one on Bootstrap got styling it never asked this component for. It is $listClass = 'pf-breadcrumb' now, and a caller who wants Bootstrap's name back sets it.

Pagination emitted no class at all. $containerElementClass defaulted to '', so no theme could reach it, and a project that wanted the pager to look like anything had to set the class at every call site or write a selector against nav > div > a. It defaults to pf-pagination.

Both hooks now have rules in all three scaffolded themes, and a test asserts that — a hook with no rule anywhere is exactly the failure the convention exists to avoid, and it is invisible: the markup is correct and the page merely looks wrong in a way somebody blames on their own CSS.

A test whose name was the finding

HtmlCharacterizationTest::testRenderContainsBootstrapBreadcrumbStructure asserted class="breadcrumb". A characterization test had recorded a coupling to one CSS framework as though it were the component's structure, and its name said so plainly for however long it had been there.

It is testRenderContainsAThemeableBreadcrumbStructure now, still guarding what it was really for — a landmark, a list, closed properly — plus an assertion that the framework's markup names no CSS framework of its own accord, and a second test that a caller can still ask for one.

What this did not do

There is a second convention in the framework: Form\FieldStyles::for($theme) hands Input and Select the theme's own classes — form-control, or ten Tailwind utilities. That is not the same mechanism and it is not wrong. Tailwind's model is utilities in the markup; hiding ten of them behind a pf-input and re-applying them with @apply fights the framework it is meant to serve.

The rule that separates them: a structural hook, one per element, is pf-*. A utility-dense set the theme wants visible in the markup is a keyed preset. Datatable::$jui — a boolean choosing between Bootstrap 3 and jQuery UI, with no path to anything else — is an older generation of the same idea and is left for its own change.

SchemaBuilder::hasIndex()

hasTable() and hasColumn() existed; the index question did not. A migration that needed to add an index idempotently had to catch the driver's duplicate error or leave the guard out — two ways of writing "I could not ask", and the token migration below needed exactly this.

It matches on the index name, not on its columns. Two indexes over the same columns are legal, so a guard asking "is there an index on this column" would skip creating the one a migration needs because an unrelated one covers the same ground. A constraint-backed index counts as existing, since what a caller means is whether creating it would collide.

Per driver, because this has no standard: MySQL keeps one row per index column in information_schema.statistics, PostgreSQL one row per index in pg_indexes. Tested against both.

Tokens are encrypted at rest, and matched on a digest

usertokens.token held the token itself, and every lookup was WHERE token = <presented>. Anyone who could read that table held live bearer credentials — usable until they expired, without needing a client secret or anything else. The framework's own token screen already refused to display the value, with a comment saying why; the column it read from was handing it out anyway.

Why not simply hash it

Because a hashed token cannot be shown, and showing it is a feature somebody uses: an administrator reproducing a failing integration copies a token into curl. Taking that away to gain at-rest protection would be trading one real thing for another.

So the column is split, since the two jobs pull in opposite directions — matching needs a deterministic value, a copy button needs the original back:

Column Holds For
token_lookup sha256(token), unique, indexed the fifteen authentication lookups
token the value, encrypted Token::reveal()

The digest is unkeyed, deliberately

A keyed HMAC is the reflex and would be right for a secret somebody could guess. Every value here is 256 bits from random_bytes() or a signed JWT, so there is no dictionary to attack and the key buys nothing measurable.

It would cost something: keying the lookup makes APP_KEY load-bearing for authentication, so rotating it would sign every session out at once. Unkeyed, a rotation costs the ability to reveal a token and leaves authentication working. That is the better failure to have.

An index that was never there

token is TEXT, which MySQL cannot index without a prefix length — so it never was. All fifteen authentication lookups were full table scans, on every API request. token_lookup is fixed-length hex with a unique index, so the auth path is now faster than before any of this.

Migration

Both columns are filled from the plaintext already in the table: no token is invalidated and nobody is signed out. Easier than the client-secret conversion, where the plaintext was gone by the time it mattered.

User::setupDb() also brings an existing table up to date now. It was only ever CREATE TABLE IF NOT EXISTS, so a column added to those definitions reached a fresh install and nothing else — which is what every caller that sets up a schema without migrating had been quietly missing.

Consumers

An application reading usertokens.token for display has to go through Token::reveal(), and one matching on the value has to use token_lookup. That is a real change for anybody who has written their own token screen rather than using the framework's.

A form's look, from what the application already declared

app/app.php carries scaffold_theme — the UI framework a project was generated against, in the same vocabulary Form\FieldStyles is keyed by: plain, bootstrap, tailwind. Controller read it to resolve views and ScaffoldingHelper to find directories. Nothing connected it to the presets.

So SettingsForm asked the caller to name the theme at every call site, and the default when they forgot was the literal plain:

public function __construct(string $name = 'settings', string $theme = 'plain')

A Tailwind project that omitted the argument rendered its forms with inline styles instead of utilities. A wrong look, no error, on a page nobody rechecks because the form works.

It is FieldStyles::configured() now, and the argument is optional — a derived default, overridable by naming one, which is the same shape as og_title falling back to the page title and Breadcrumb::$listClass defaulting to a hook.

An unknown value falls back rather than propagating: scaffold_theme names a scaffolding directory, and that set can grow past the presets here. A test asserts the two lists match, because if they diverge every project on the missing one silently renders as plain.

And the pf-* rules added earlier today are rewritten in tokens

A project's colours live in one file — app/themes/theme.css, propagated by pramnos theme:build to a stylesheet, a JSON file and ThemeTokens::token(). The neighbouring hook rules are written against it: var(--text-main, #1e293b), var(--primary-color, #2563eb).

The pf-breadcrumb and pf-pagination rules added earlier were not — they used currentColor and bare values. So they would have been the one part of a scaffolded theme that did not change when the palette did, which is the failure the single-file palette exists to prevent. They speak in tokens now, with the same fallbacks.

Component class names, declared once in app.php

'component_classes' => [
    'breadcrumb'         => 'breadcrumb',
    'pagination'         => 'pagination',
    'pagination.current' => 'active',
],

Six keys, every one listed in Html\ComponentClasses::KEYS with the pf-* hook it replaces. An unlisted key is reported by unknownKeys() rather than ignored, because a misspelling is otherwise silent and silence is indistinguishable from a feature that does not work. An empty string means no class and is honoured — there the caller did speak.

It exists because the objects are not all yours

Breadcrumb::$listClass and Pagination::$containerElementClass were already public, so the override existed. The problem is where the objects are made: a Breadcrumb is constructed in eight places in a scaffolded project — Document, Application, and an admin_breadcrumb and account_breadcrumb partial in each of three themes — and two of those are inside this framework, on the path that renders every page.

So a per-object property covers six of eight, and misses the two that always render. The defaults are read at construction now, so one declaration covers all of them, and the property still wins for a single object: configuration is a statement about the project, a property about one breadcrumb.

And it is still not the first thing to reach for

The components emit neutral hooks and each theme's stylesheet dresses them. One CSS rule reaches all eight sites, needs no PHP, and is the right answer to make this look different — colours and radii come from app/themes/theme.css and reach the hooks from there.

This is for the other case: markup that must carry a specific name because something other than a stylesheet is looking for it. A jQuery plugin doing $('.breadcrumb') does not read CSS.

The key is scaffolded into a new project's app.php commented out, with that reasoning beside it — so somebody finds it by reading their own config rather than this framework's source, and finds the reason not to use it at the same moment.

A model over a schema-qualified table could not read its own row on MySQL

Role declares authserver.roles. The QueryBuilder has resolved that per driver since from() was taught to — a schema on PostgreSQL, prefix_authserver_roles on MySQL, which has none — but a Model builds its own SQL, and never asked. So every _load() and _save() sent authserver.roles to MySQL, which read it as another database, and threw.

Model::getFullTableName() now sends a dotted _dbtable through the same resolver:

protected $_dbtable = 'authserver.roles';   // works on both, with nothing else to write

Delegated to SchemaBuilder::resolveTable() rather than repeated — the flattening rule and the prefix guard that keeps pramnos_pramnos_x from happening live there, and two copies would eventually disagree. #PREFIX# still wins over the dot, because it has already said where the prefix goes.

Found by executing the roles admin screens. They shipped complete, with a menu entry and a scaffolded wrapper, and could not create a role on a MySQL installation.

A NOT NULL date the model never set was written as timestamp zero

The same screen, one line further in. Model::_save() writes every column it finds, and a NOT NULL column with null in it was coerced to '' — fine for a string, impossible for a date. authserver.roles.created_at is NOT NULL DEFAULT CURRENT_TIMESTAMP, so a model with no opinion about it was asking for the column's default and requesting the zero timestamp, which strict MySQL and PostgreSQL both refuse.

A NOT NULL date, time or timestamp holding null is now omitted from the write: the default fills it on an insert, the stored value stays on an update. Strings still coerce to '', because models have relied on that far longer than this has been wrong.

In both loops. The column list is read twice — once cold from SHOW COLUMNS, once from self::$columnCache — and fixing the cold path alone made the failure depend on whether anything had saved to that table earlier in the process. Which is the worse bug: it passes in isolation.

One reader for authserver_organization_table

Role::membershipTable() read it with a default of ''; the user_organizations migration read it with a default of 'user_organizations'. An installation holding the setting as an empty string therefore got authserver.user_organizations from the model and authserver. from the migration. The migration asks Role now.

Coverage attribution, which is not the same as coverage

JoinClauseTest declared #[CoversClass(JoinClause::class)] and nothing else, so twenty-one lines it genuinely runs — QueryBuilder::resolveJoinTable(), Grammar's multi-condition join — were recorded as untested. PHPUnit credits only the classes a test names. Both are named now.

Worth stating as a rule: a CoversClass list narrower than what the test exercises does not weaken the test, it weakens the report — and the report is what tells the next person where to look.

Two token lookups the encryption change left behind

Both fail closed, silently, and both shipped this morning in the same commit that split usertokens.token into an encrypted value and a token_lookup digest:

  • Oauth::selectTokenRow() — introspection answered {"active": false} for every token this server had issued. A resource server that trusts introspection refuses every request.
  • OAuth2Middleware::loadTokenFromDatabase() — every Bearer token failed validation, so the whole authenticated API answered as anonymous.

Fifteen lookups were converted and these two were not, because both are written as where('ut.token', …) — the aliased form, which a grep for where('token' does not see. The column no longer holds anything a presented value can be compared to, and every caller reads "no row" as "not a valid token", so the failure has no error and no log line. It looks like a client presenting a bad token.

selectTokenRow() also inner-joined users and applications to decorate the answer with a username and a client id. Those are left joins now: the token's own row is the authority on whether it is active, and an inner join made a token whose client row had been removed introspect as dead while OAuth2Middleware — which left-joined already — went on accepting it. Two components disagreeing about a live credential, invisible from either side.

Why the suite was green

There are unit tests over introspect() and revoke(). They mock the query builder, and a mocked builder returns the prepared row whatever the WHERE says — so they cannot answer a question about a column. They still earn their place (they cover the controller's decisions), and the column is pinned separately now: TokenAtRestTest::testNoLookupMatchesOnTheTokenColumn() reads src/ and fails on any surviving comparison against token, alias or not. Verified by reintroducing the bug: it fails.

Found by another project's integration suite, which routes real requests through the real authentication path. Worth naming as the general lesson — a mock cannot notice a schema change, and fifteen call sites converted by hand want an assertion about the absence of the sixteenth.

The same model fix again, for PostgreSQL — twice

The schema-qualified-table fix earlier today was written against MySQL, tested against MySQL, shipped, and the first PostgreSQL request answered 500:

INSERT INTO public.authserver.roles () VALUES ()

Two defects in one line.

public. should not be there. The dotted-name resolution was placed after the branch that prepends the connection's schema, so on PostgreSQL it never ran and public was prepended to a name that already carried a schema. A qualified name is resolved first now: it has said where it lives, and prepending another schema to it can only be wrong.

() VALUES () should never have been composed. The column-introspection query asked for table_schema = 'public' AND table_name = 'authserver.roles' — the schema from one place, the whole dotted name from another. That matches nothing; an empty column list is not an error anywhere; and _save() built an INSERT with no columns. So the operator saw a syntax error and the cause — a table name that does not resolve — was two steps back.

There were three copies of that query in Model, and the third one already split the name correctly. That is the shape of the whole thing: the fix existed in one of three places and nothing made the other two agree. One reader now, plus a guard: a save that finds no writable columns throws and says which table it could not read.

The lesson is about the lane, not the code

The integration lane runs one engine. An engine-specific mistake cannot fail there, and this one was shipped by a suite that was green on 13,065 tests. So the regression test is a unit test where the engine is a property of a throwaway connection, and it asserts all four backends:

  • MySQL — flattened to prefix_authserver_roles.
  • MariaDB — the same, asserted separately because isMySQL() being true on MariaDB is a decision somebody could reverse, and nothing else in the suite runs a MariaDB connection.
  • PostgreSQL — left alone.
  • TimescaleDB — identical to PostgreSQL, plus an assertion that a type of timescaledb normalises to postgresql. That one matters: this method compares type == 'postgresql' literally, and a connection left reporting timescaledb would fall through to the MySQL path and flatten a schema into a table PostgreSQL does not have.

The connection is a clone, not the mutated singleton: DatabaseCapabilities memoises per Database instance in a WeakMap, so flipping ->type on the shared object leaves isMySQL() answering for whichever engine it was asked about first — a test that passes or fails depending on the order its own methods ran in.

Verified the only way this kind of fix can be: by running a save, a read-back and a delete through the model against a live PostgreSQL/TimescaleDB installation, and by reintroducing each defect to watch the new tests fail.

Auth\NewDeviceAuthLink — 109 statements, none of them run by anything. It is the one new-device action every account can satisfy, because it needs nothing but the mailbox, so it is the one an installation actually falls back on.

Nineteen tests now run it against a real store. The four rules that are the method's whole security, each asserted on its own because each is load-bearing when the others are absent:

  • Single use — the hash is cleared before an id comes back, so a link cannot sign two sessions in. The mail stays in the inbox afterwards, and a provider's link-preview fetch counts as an open.
  • Fifteen minutes, from the constant rather than a number somebody remembers — including the boundary, which is < and not <=: a link expiring this very second is still good.
  • One at a time — issuing again invalidates the link the person is already holding.
  • The rate limit — the interval, the count per window, and that sends older than the window stop counting.

Plus the two decisions that surprise people, asserted so nobody quietly reverses them: with no activity-log table a send is allowed (refusing every link when a log is missing would refuse the login itself), and a failed delivery is not recorded as a send (a dead mail server must not tell somebody to watch an inbox nothing is coming to, and spend their rate limit doing it).

One seam, because the happy path was unreachable

send() built its notifier inline, so the only reachable parts of it were its refusals — the fifteen lines that generate the token, store it, mail it and record it could not be run without a mail server. notifier() is now a protected seam, the same idiom as Controllers\Me::resolveUser().

That closes the test worth having most: the link in the mail is the link the store accepts. The two are produced in different places — hashed into userdetails, then formatted into a URL — and a mismatch would fail only for real users, never for a test that reads the store directly.

Suite: 13,076 → 13,095 tests, wall clock unchanged at 2:23.

The template screens, run on both backends from one set of assertions

MailTemplatesController — 131 of 137 statements never executed. Worse than it sounds for this screen in particular: mailtemplates shipped as a table with no editor, and a screen that has never been run is indistinguishable from the screen that was missing.

Nineteen tests, asserting what makes the screen worth having rather than decorative: the grouping that answers is the reset email translated into Greek, the placeholders read from the template instead of a maintained list that goes stale, the body kept as markup while the labels are stripped, delete, the gate — and the test send actually rendering, [name] where each placeholder lands, in the wrapper the template names.

mailer() joins notifier() as a protected seam, for the same reason: the mailer was built inline, so the only reachable part of test() was its refusals and the two things a test send exists to prove could not be tested at all.

And then all nineteen again, on PostgreSQL

class MailTemplatesScreenPostgreSQLTest extends MailTemplatesScreenTest
{
    protected function settingsFixture(): string
    {
        return ROOT . DS . 'tests' . DS . 'fixtures' . DS . 'app' . DS . 'pg_settings.php';
    }
}

Fifteen lines, and the whole file runs against PostgreSQL/TimescaleDB as well — 38 tests in one second. settingsFixture() is the seam; the assertions are about the screen, and the only thing that differs is what is underneath it.

This is the pattern worth copying, and it exists because of what happened earlier today: a Model that addressed its own table correctly on MySQL and not on PostgreSQL, written and tested on one engine, shipped green past 13,065 tests, and answered 500 on the first request from the other. A suite that runs one backend cannot fail for the second. Where a test's subject is engine-independent, saying so in fifteen lines is cheaper than finding out it was not.

Suite: 13,095 → 13,133 tests, wall clock 2:14 — faster than the 2:23 before it, so the cross-backend pass costs nothing measurable.

The inbox crashed on PostgreSQL and apologised on MySQL

MessagesController — 102 of 106 statements never executed, on the screen that exists to end a dead end: MassMessageDispatcher wrote a row per recipient, the count was right, the progress screen reported every recipient delivered, and nobody could read a word of it.

Sixteen tests now run it, and then run it again on PostgreSQL. The second engine found two defects in the first minute.

A try/catch around a query is only half the handling

try {
    $result = $qb->…->get();
} catch (\Throwable $e) {
    return [];          // never reached on PostgreSQL
}

while (($row = $result->fetch()) !== null) {   // fatal: fetch() on false

MySQL throws where PostgreSQL answers false. So with an unreadable messages table the listing reported the problem politely on one backend and fatally crashed on the other, from the same lines — and the catch block that made it look handled is what hid it. Both listFor() and loadFor() now guard the result as well as the call.

There are 28 more sites in src/ with the same shape — ->get() followed by ->fetch() with no falsy guard. They only bite when a query fails, which is exactly when the handling was supposed to work. The real fix is engine parity in the driver rather than 28 guards, and that is the next thing worth doing.

messages.attachmenttext has no default

TEXT NOT NULL with nothing to fall back on, so an insert omitting it fails — on PostgreSQL always, on MySQL under strict mode. MassMessageDispatcher already passes '' with a comment explaining why, which means the trap is known and every future writer has to remember it independently. Now documented in the Email guide beside the screen.

Not changed here: the column is in a migration deployed since 2020, and altering a long-live table is not something to do inside a coverage pass.

Suite: 13,133 → 13,165 tests, wall clock 2:14 — unchanged.

The test suite now fails where an installation would merely be wrong

Yesterday's finding, chased to its cause. Measured on the same builder call against a table that does not exist:

MySQL PostgreSQL, lenient
get() throws false
first() throws false
count() throws 0

The asymmetry is not the framework's: mysqli has thrown by default since PHP 8.1, while pg_* answers false. So the shape most of src/ is written in —

try {
    $result = $qb->…->get();
} catch (\Throwable $e) {
    return [];                                  // never reached on PostgreSQL
}
while (($row = $result->fetch()) !== null) {    // fatal: fetch() on false

— is complete on one engine and half the handling on the other, and the catch block is what makes it look finished.

Database::$throwOnError already closed the gap. What was missing is that nothing turned it on, so it is now readable from settings:

'database' => ['throwOnError' => true],

Read on construction, so it survives the singleton being reset — which a suite does constantly, and which is why setting the property at bootstrap did not work.

What one flag found

Both test fixtures set it. That change alone, with nothing else touched, surfaced three things:

  • Two second-factor tests passing against a missing accounts table. They assert that a wrong password is refused; the refusal was happening because the query failed. PermissionsPostgreSQLTest drops public.users on the same connection, so it depended on test order — and while a failed query answered false, User::load() found nothing either way and the difference was invisible.
  • A characterization test pinning the lenient path, which now accepts the message through either channel. Its real subject — that a failed prepare must not lose the error text — is unchanged and still asserted.
  • The inbox crash fixed in the previous commit, which is what started this.

Fourteen new tests pin the parity itself, on both backends, including the asymmetry: if a future PHP or driver release makes mysqli lenient, the suite says so here rather than through a screen crashing.

The runtime default stays false. Turning it on globally would convert every silently-empty answer in every existing application into an exception — a BC break dressed up as a fix. The guide says where it belongs instead: in a test environment always, in production deliberately.

Suite: 13,165 → 13,179 tests, wall clock 2:17.

An empty field list compiled to SELECT FROM

ApiListSqlBuilder — 149 of 219 statements never executed, on the class that decides what a caller is allowed to ask for. Forty-two tests now run it, twice: once per backend, because it branches on the driver in nine places.

The defect they found is one line. Its first branch reads

if ($queryFields === null || $queryFields === '' || $queryFields === '*') {
    return $queryFields ?? '*';
}

— the condition says all three spellings of nothing asked for mean the same thing, and the return disagrees: ?? '*' covers null only, so an empty string came back as an empty string. Both callers hand the result straight to select(), which compiles it to

SELECT  FROM users a

a syntax error — a 500 on MySQL, and outside strict mode on PostgreSQL an empty list that reads as this account has no records.

Model::_getApiList() already treats '' as equivalent to null and '*' thirty lines further down, so the class disagreed with itself and only one of the two places produced SQL.

An existing test pinned the old answer, and reading it was the point: its docblock said "empty field lists are passed straight through — there is no field list to inspect or extend". True about inspecting, and not a decision about what to return. It asserts * now, with the reason.

What the forty-two assert

The security half, mostly. ?order= is a query string, so the tests include six shapes that must never reach an ORDER BYusername; DROP TABLE users, username)--, (SELECT 1), username' OR '1'='1 — each of which must produce the default order and nothing else. Quoting is not validation; the whitelist is.

And the two edges nobody would guess: = null emits IS NULL, because = NULL matches nothing and reads as a bug in the caller's data; IN with an empty array is dropped, because IN () is invalid and IN (NULL) would silently mean nothing matches.

Suite: 13,179 → 13,221 tests. Measured A/B on the same machine in the same minute — without these files 2:34.9, with them 2:33.6 — so they cost nothing detectable. (The earlier 2:14 readings were on a quieter host; the whole spread is contention, and a run with strict mode off took 2:51.)

The screen that mails everybody, and its refusals

MassMessagesController — 156 of 283 statements never executed, on the action with the least recoverable consequence in the framework. Everything else here can be corrected; this one reaches every person on the list.

Forty-two tests, twice over the two backends, and most of them assert a refusal:

  • a GET cannot queue anything, token or not — a GET that mails everybody is one link prefetch away from happening by itself, and a chat client unfurling a URL counts;
  • a POST without the anti-CSRF token cannot either;
  • criteria matching nobody queue nobody, and say so rather than reporting a send of zero;
  • a sent message cannot be edited — it is the record of what people received, and editing it would change the record without changing what anybody got, which makes it a lie rather than out of date;
  • a sent message cannot be deleted, for the same reason;
  • and below usertype 90 none of the above is even reachable.

The two traps in reading the form

Both are documented in the code as traps, which is exactly why they deserved tests: a comment does not fail.

A false boolean is not an empty value. array_filter cannot tell unticked from not mentioned, so validated_only and active_only are written after it. Dropped, they revert to their defaults — which excludes accounts the operator chose to include, and makes the count on the screen disagree with the send.

An empty template is a decision. No wrapper for this campaign looks exactly like an empty form field, so the form posts __default__ for silence and '' for a choice. Only the choice is stored, and the audit record omits the options key entirely when nothing was chosen — an empty object there reads as a decision somebody made.

One assertion that only passed on one backend

Mine, not the framework's. request is a JSON column on MySQL and text on PostgreSQL, and MySQL reformats the document — its own key order, spaces after colons. So assertStringContainsString('"language":"el"', …) passed on PostgreSQL and failed on MySQL. Decode and compare the value; the serialisation belongs to the server. Noted in the guide beside the column.

Suite: 13,221 → 13,263 tests, wall clock 2:34 — unchanged within the spread.

Two backends taking different branches, on purpose

timescale:ensure — 115 of 213 statements never executed. The stubbed tests covered what the command concludes about a table; nothing covered the entry point, and the entry point is where its one interesting branch lives: what it does on a backend that has no TimescaleDB.

That branch was written for the case the command repairs — a database that gained the extension after the migrations had already run — so it has to behave correctly on a database that never will. Which makes this the one place in the suite where the two lanes run different code on purpose rather than the same code twice:

lane what runs
MySQL / MariaDB continuous aggregates, then the documented bow-out
PostgreSQL / TimescaleDB the same, then the whole hypertable plan

Each lane skips the tests belonging to the other, so the skips in the output are the design rather than a gap.

The interval comparison, which is why a changed declaration reaches the database

sameInterval() exists because the command used to compare a policy's presence: a declaration changed from 30 days to 7 left the old policy in place for ever, silently, and the only symptom was a disk bill. Now pinned in all three directions:

  • equivalent spellings are equal — @ 30 days, 30 DAYS, 30 days, 30 day;
  • a real difference is detected, including across units (24 months2 years, because they are different in PostgreSQL and guessing otherwise would invent a policy nobody declared);
  • an unparsable interval is treated as equal. The safe direction: reading an unfamiliar spelling as drift would rewrite the policy on every single run, which is the failure the comparison exists to avoid.

And the repair path: a blocked table is reported and counted without abandoning the run — the remaining tables are the ones still growing without bound — while a conversion announces its row count and that it holds an exclusive lock before starting, because these are audit tables with millions of rows and somebody running this at 10am deserves to read that sentence first.

Suite: 13,263 → 13,282 tests, wall clock 2:23.

FW-049: the action that outlived its request

Reported by an application trying to migrate off its own legacy request class, and correct as reported. calcParams() clears the controller and not the action:

self::$_controller = '';        // cleared
// self::$action                   was not

self::$action is written only when the path has a second segment, and was cleared only by resetInstance(). So calcParams('module') — a bare module — left the previous call's action in place, and getAction() answered with it.

The asymmetry inside those six lines was the whole tell: if an action were meant to survive a re-route, the controller beside it would not be cleared either.

One request per process hides it in production. A test suite is one process for thousands of requests, and that is where it surfaced: a controller reading the action as an identifying hash terminated with Invalid User because it inherited the action of an unrelated earlier test. Testing\TestClient already documents the same leak for the controller — "/ served the previous URL's controller" — and fixes it by resetting the instance. The action was simply missed.

Cleared where the controller is cleared, and two tests that fail without it.

And the second half of the report: document the decomposition

The same report asked, fairly, that if the path decomposition differs from the legacy algorithm it be written downso we know what we are migrating to. It is now a table in the Routing guide, generated by running the code and pinned by a characterization test. Writing it turned up two anomalies neither side had named:

Leftover path segments become $_GET keys. module/action/x sets _option = x and $_GET['module'] = 'action' — parts 0 and 1 are never removed before the key/value pairing loop walks what is left. On /jobposts/view/479, code reading $_GET['jobposts'] gets 'view'.

A slash inside the query string changes the decomposition. $slashes is recounted after the query string is appended to the path, so an ordinary return-url parameter moves the same path to a different branch:

r=jobposts/view/479                            → _option=479, jobposts=view
r=jobposts/view/479 ?return=/account/settings  → _option=479, 479=null

One slash is enough, and _option survives — which is exactly why it goes unnoticed. It is the leftover keys that move, and code reading one of them by name reads something else. That is a plausible mechanism for a page that serves differently after a migration.

Neither is fixed here, and that is deliberate: both decide which page is served for URLs already in use, so changing them is a routing change for every installation and belongs in a release somebody chose, not in a coverage pass. What ships is that they are visible, executable, and will fail loudly the day they change.

Suite: 13,284 → 13,294 tests, wall clock 2:14.

Behaviour change: a slash in a query string no longer re-routes the path

The two anomalies the last post recorded are fixed. Recorded first, fixed second, deliberately — both decided what ends up in $_GET for URLs already in use, which makes them routing changes rather than bug fixes you take for granted.

Leftover path segments became $_GET keys. module/action/x set _option = x and $_GET['module'] = 'action'. Only $parts[2] was removed before the key/value pairing loop walked the rest, so the controller and the action were paired into an entry of their own. The two neighbouring branches already removed them; that one was the outlier — which is what an omission looks like from the outside.

A slash inside the query string changed the decomposition.

before:  r=jobposts/view/479                            → _option=479, jobposts=view
before:  r=jobposts/view/479 ?return=/account/settings  → _option=479, 479=null
now:     both                                           → _option=479

$slashes chooses the branch, and it was recounted after the query string had been appended to $request. The append had no other purpose: the next line threw the query string away again, $mainString[1] is never read, and $request is not used past that point. So four lines existed only to corrupt a count.

One slash was enough, and _option survived either way — which is exactly why nobody found it from the outside. It was the leftover keys that moved.

What made it safe to do

The measurement, not the reasoning. The change was applied first and the suite run before anything was rewritten: it failed the four characterization tests written an hour earlier to record the anomalies, and nothing else in 13,294 tests. A routing change with a blast radius that small is a routing change worth making.

If you relied on either: a controller reading $_GET['<module-name>'] on a three-segment URL, or on a path segment appearing as its own $_GET key, no longer gets it. Read _option and the named pairs. Nothing else about the decomposition moved — the table in the Routing guide is generated by running the code, and the rest of it is unchanged.

Suite: 13,294 tests, wall clock 2:51 on a busy host (2:14–2:51 is this evening's spread with identical code; a measured A/B earlier put the new tests at no detectable cost).

A cache flush that broke a user activation

Found by a coverage run of this framework's own suite — the timing differs enough under coverage to expose a race the ordinary lane hides:

UnexpectedValueException: RecursiveDirectoryIterator::__construct(…/var/cache/userlist):
    Failed to open directory: No such file or directory
  FileAdapter.php:610 → Cache.php:728 → Database.php:2673 → User.php:755
  ← User::activate()

listDirectoryFiles() guarded with is_dir() — and a guard cannot fix a race, because it is a check followed by a use. The directory goes between the two: another request flushing the same group, or this adapter's own cleanEmptyDirectories() from a concurrent call.

The last line of that trace is the point. The throw did not break a cache flush; it broke a user activation. save() flushes the user list, the flush raised, and the operation somebody asked for failed because of housekeeping that had already succeeded — the directory was gone, which is exactly the state the flush wanted.

Caught, around the loop rather than the constructor alone since a subdirectory can vanish mid-walk with the same result, and whatever was collected before it went is returned. One catch, six call sites.

The is_dir() guard stays: it is the common case, and an exception per flush of a group nothing ever wrote to would be a cost paid constantly for a condition that is normal — which the tests assert, by counting that a missing directory never reaches the iterator at all.

Covering a race honestly

A race cannot be reproduced by arranging files, so directoryIterator() is now a protected seam and the test makes it throw. That is the only honest way to cover a catch for something that happens between two statements — and it is the same idiom as notifier() and mailer() earlier today.

The Adminer route, and the two things a mistake there would not announce

167 of 235 statements never executed on /adminer, a route that serves a database browser. The gate is documented at length; what nothing ran was the rewriting, and two parts of it are security rather than presentation.

Which files it will send. The path comes from a query parameter and the directory sits in vendor/. Eight traversal shapes are now asserted refused — ../../../../etc/passwd, static/../../../../etc/passwd, static/./../../composer.json, static/ + twelve ../ — and so are five extensions. static/x.php is the one worth naming: nothing here would execute it, but it would be read out verbatim, which is how a config file leaks. The whitelist runs before any filesystem call, and a positive case is asserted too, so the refusals cannot be passing because everything is refused.

What its links carry. Adminer puts the server, the user and the database in its own URLs. Left in, every link on a page this route served would name this installation's database host and account — in the address bar, in the browser history, and in a Referer on the way to adminer.org. All seven driver keys are asserted stripped, not only server; absolute URLs are asserted untouched, because those are Adminer's own outbound links and rewriting them would corrupt somebody else's URL to no purpose.

Suite: 13,294 → 13,313 tests, wall clock 2:15. Total coverage 90.03% → 90.49%.

The password-reset token, and a fixture that proved nothing

Seven methods of Account — store, resolve, clear, the link, the mail and the two lookups — none of them executed by anything, on the credential path with the widest audience on any installation: the forgot-password form can be submitted by anybody, from any page, for any address.

Twenty-eight tests, on both backends, because storeResetToken() uses upsert() — which compiles to ON DUPLICATE KEY UPDATE on MySQL and ON CONFLICT … DO UPDATE on PostgreSQL. One live token per account was a claim about two different statements, and only one had ever run.

Two things I had backwards, and both are now written down

Resolving a token does not spend it. I asserted single use on consumeResetToken() and it failed — correctly. The flow clears the token after updatePassword(), and that is the right design: burning it when the form is merely opened would lose the only link of anybody who mistypes their confirmation. The single-use property is real and lives one level up, so the test now pins the order — cleared after the write, never before, since the reverse leaves an account with neither a password change nor a live link.

A language catalogue defines $lang; it does not return an array.

$lang['Password reset'] = 'Αλλαγή κωδικού';   // right
return ['Password reset' => '…'];             // loads, defines nothing, load() answers false

My fixture used the second form, so Language::using('el', …) switched nothing and the test passed on an assertion about currentlang() that was itself wrong — using() swaps the strings and only restores the language name in its finally, so currentlang() reports the previous one while the callback runs. Two wrong things agreeing. The test now asserts what a reader of the mail would notice: the translated words.

Worth the note in the guide, because the failure mode of a return-style catalogue is that nothing raises: every string renders as itself, which looks like a site written in English rather than a catalogue that was never loaded.

And one decision worth having a test

The mail goes out in the recipient's language, not the request's. That form can be submitted by anybody — a Greek visitor asking for an English speaker's reset — and the person who reads the mail is the account holder. composeAndSendResetEmail() is protected now so the decision is observable at all; private, it was a documented intention with nothing checking it.

Suite: 13,313 → 13,341 tests, wall clock 2:21.

An export that aborted whole, and a test I should not have written

buildExportData() reads eleven tables, several belonging to features an installation may not have enabled — and composed as a single array literal, one unreadable table aborted the whole export. Somebody exercising a data-subject request got an error, and the ten sections that were perfectly readable went with it. For a document an installation is legally obliged to produce, partial is worth more than nothing.

Each section is resolved on its own now. Two details of the recovery are deliberate: the section stays present and empty, because a missing key reads as this framework has no such concept while an empty one reads as you have none of it — and only the second is something an installation is entitled to say; and the failure is logged, because in the file those two are indistinguishable, so they have to be distinguishable to the operator, who is the only person who can act.

Found while writing tests under the strict mode the fixtures now set, which is the point of having turned it on: in lenient mode each helper's if ($result) quietly produced an empty section and nothing aborted, so the defect only exists where failures are loud — and only shows up in production as an export that errors for some installations and not others.

And the part worth admitting

I wrote sixteen tests for that export before noticing AccountExportTest already existed, covering the same four properties — sections populated, secrets excluded, the extensibility hook, a missing table degrading. Pure duplication, deleted. The remaining file covers what nothing did: revokesession(), which is POST-only with the anti-CSRF token and scoped to the caller's own account — a session id is a string in a form, and an unscoped update would let anybody sign anybody out.

The duplication also produced a genuine finding, so it was not wasted:

Two tests, one framework table, two shapes

AccountExportTest builds sessions with its own columns. UsersControllerTest builds tokenactions and usertokens with theirs. Both use CREATE TABLE IF NOT EXISTS, so whichever runs first decides the shape, and the loser fails on an insert naming a column the winner never created. In isolation both pass; together one fails, and which one depends on the filter you happened to use.

Now in the Testing guide as three rules, in order of preference: do not create a table you do not read (my own fix — the revocation test needs sessions and nothing else); drop before you create when you do need one; and build from the real migration rather than a hand-written CREATE TABLE, because two hand-rolled shapes drift from each other as surely as each drifts from production.

Two flakes that are not this

Chasing the above turned up two pre-existing timing flakes, one run in three: UsersControllerTest::testTheTokenActionPanelFindsARealAction and PageCacheTest::testAnAbandonedLockExpires. Neither is caused by this work — the second is a lock-expiry test asserting on wall-clock behaviour — and both are named here rather than guessed at, as the next thing worth fixing.

Suite: 13,341 → 13,353 tests, wall clock 2:23.

FW-051: a property declared, cleared, and never assigned

Request::$originalRequestNoChange has existed since the class was written. resetInstance() clears it. Nothing ever assigned it, so every read answered ''.

Its own docblock states the contract it was not keeping — the original $_GET request that should never change — as against $originalRequest, which calcParams() is free to rewrite.

Assigned now where r is read, before calcParams() can touch anything, because "as it arrived" is precisely what must not reflect a re-route. Three tests: the value is kept, it survives a re-route that rewrites $originalRequest, and resetInstance() clears it so one request does not answer for the next. All three fail without the assignment.

Why an empty string was the worst possible wrong answer

Because it is a plausible one. An application building a page's canonical URL from it gets sURL alone, compares that with the real address, finds them different, and redirects the page to itself:

if (sURL . Request::$originalRequestNoChange != $model->getFriendlyUrl()) {
    $this->redirect($model->getFriendlyUrl(), true, 301);
}

An infinite loop that curl -L abandons at ten hops. No exception, no log line, and the code doing it reads correctly. The reporter's own account of finding it is the part worth keeping: "two wrong assumptions before the right one, both because I was measuring the effect instead of the cause." The effect was a redirect loop, and it was attributed first to the $_GET leftovers fixed earlier today — reasonably, since those were real and had just been documented as a plausible mechanism. Only a probe that ran both implementations over the same input, showed the decomposition now agreeing, and then backtraced the redirect found the line above.

Which is the argument for the characterization table added with FW-049: once the decomposition was written down and pinned, it could be ruled out, and the search moved on.

Suite: 13,353 → 13,356 tests, wall clock 2:18.

The gate on the database browser, and a property read before its guard

/adminer serves a database browser with the connection supplied from configuration, so reaching the page is reaching the database. There is no second credential to stop a mistake in the gate — and 167 of the route's 235 statements had never been executed.

Nine tests drive the real thing rather than a stub: the signed-in account through RequestIdentity and $_SESSION, because getCurrentUser() and staticIsLogged() are independent questions and mayOpen() needs yes to each; the environment through APP_DEBUG; the feature through FeatureRegistry. Every clause asserted on its own, including the one that reads as belt-and-braces and is not — in development, below root, the floor only applies if the devpanel feature is actually enabled, because a floor configured for a panel the installation does not have is a number with nothing behind it.

Two findings came out of writing them.

A property read before the guard that protects it

$user     = User::getCurrentUser();
$usertype = (int) ($user->usertype ?? 0);     // false->usertype, before the check below
if ($user === null || !Session::staticIsLogged()) {
    return false;
}

getCurrentUser() answers false for an anonymous visitor, not null. So the usertype line raised attempt to read property on bool before the guard ran — harmless in the result, since it lands on 0 and the visitor is refused, and a warning in the log of the one route where a log entry is the only visible trace of somebody trying the door. Guard first, and !is_object() rather than === null, since false was never covered by that comparison either.

FeatureRegistry::loadFromConfig() only adds

The test asserting "refused without the DevPanel" reported the gate opening. The gate was right; the test was wrong, and so was my assumption: loadFromConfig([]) is a no-op, not "no features". It enables what you pass and never disables anything — invisible in production, where it runs once at boot, and in a suite it means whatever an earlier test enabled is still enabled.

reset() exists for exactly this and says so in its docblock. Now in the Testing guide, with the half that is easy to forget: restore what the suite runs with afterwards, because a test that leaves the registry empty makes the next test's isEnabled('auth') answer false, and that test fails for a reason that has nothing to do with it.

Suite: 13,356 → 13,365 tests, wall clock 2:15. Total coverage 90.49% → 90.63%.

Security: the log endpoint would read any file on the disk

Auth\Controllers\ApiAdmin — the SPA administration endpoints — had 48 statements and none of them had ever been executed. Writing tests for it found three defects in one method, and the first is a vulnerability:

GET /admin/logs?file=../../../../etc/passwd
→ 200 {"lines":["root:x:0:0:root:/root:/bin/bash", …],"total":18}

The class's own docblock says why that must not happen — "The file is chosen from the viewer's whitelist, never from the raw parameter: a log endpoint that accepts a path is a file-disclosure endpoint" — and it did it anyway. The reason is one condition:

if ($checkWhitelist && !empty($this->whitelist) && !in_array($filename, $this->whitelist)) {

An empty whitelist skipped the check entirely. "Nothing is configured" was read as "everything is allowed" — the fail-open shape. Every caller in the framework passes a whitelist except ApiAdmin::logs(), which constructs new LogViewer(), so that endpoint was the one place the short-circuit was reachable. Authenticated and permission-checked, so this is privilege escalation rather than an open door: a grant of may read the log file was a grant of may read any file this process can.

Two fixes, and the first is independent of the second on purpose:

  • A filename is a name, never a path — refused if it contains a separator or .., before the whitelist and even when a caller passes $checkWhitelist = false, because that argument asks to skip a list, not to be handed a path.
  • No whitelist means the log directory's contents, plus the two names that map outside it. A viewer constructed with no arguments is asking to read this installation's logs, not any file on the disk.

A test was protecting it

LogViewerTest::testSetFileAcceptsAnyFilenameWithEmptyWhitelist asserted the fail-open behaviour, and its own docblock says what it was: "This covers the !empty($this->whitelist) short-circuit". A test written to execute a line, which ended up defending what the line did. Nothing in it claims an empty whitelist should allow every file; it only recorded that it did.

Worth stating as a rule, because this loop has now hit it twice: a characterization test earns its place by describing behaviour, and the description is not an argument. When one blocks a fix, read what it says it covers — if the answer is "this branch", the branch is the thing under discussion, not the test.

And the two ordinary bugs beside it

The default file name was wrong. 'pramnosframework' without the extension, against a whitelist of 'pramnosframework.log' and a path builder that appends the name unchanged — so the endpoint threw on its own default parameter.

A log that has never been written was a 500. The try covered setFile() — the "may I read this?" question — while processFile() throws for a file that does not exist, which is the normal state of a fresh installation. Uncaught, the panel answered 500 for the endpoint somebody opens precisely because they are trying to find out what went wrong. An empty page now, in the shape the reader already handles.

Twenty tests, on both backends: 401 and 403 distinctly from every action, each action authorised under its own name (search is not users — an omnibox reaches whatever the application registered), the omnibox limit capped here rather than taken from the request, four traversal shapes refused, and a missing dashboard table counted as null rather than 0, because "no sessions" and "no sessions table" are different things and only one deserves a number.

Suite: 13,365 → 13,386 tests. Total coverage 90.63% → measured next round.