docs: record the swept lenses and re-measure coverage - #829
Merged
Conversation
The defect record says where the defects have been. This adds the other half — where they are not — so a later session can start somewhere else instead of re-deriving it. Each entry carries the reason it is sound rather than a verdict, because two of them read the wrong way round at a glance: `getSecureAppPath()` alone would allow `b=/config` on the resource route, since APP_ROOT contains it; what actually stops a forged base directory is the HMAC `ResourceBase` verifies over every query parameter. And the account history query cannot be permission-filtered at all — history has to outlive the account it describes — so the ACL check moved into `AccountHistoryHelper::checkAccess()` instead, which looks like a missing filter until you read it. Coverage is re-measured rather than restated: 97.20% (22268/22910), against the 96.63% (21973/22740) recorded before roughly sixty tests were added. The derived figures move with it — 642 uncovered statements rather than 767, 303 of them in 200 files missing three or fewer, and the bootstrap block is 72 rather than the ~90 it was estimated at. `Base.php`, `BootstrapBase`, `CoreDefinitions` and `Cli/Init` remain the four largest gaps, which is what that section already predicts: they run before the container the tests build. pcov was in the "installed but disabled" state the same section warns about, so it was enabled rather than reinstalled, and the ini removed afterwards.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The defect record says where the defects have been. This adds the other half — where they are not — so a later session can start somewhere else instead of re-deriving it.
The swept lenses
Injection, path traversal on the resource route, LDAP filters and binds, object-level authorisation, deserialization, foreign keys, import atomicity, the deployment layout, and four smaller checks. Each carries the reason it is sound rather than a verdict, because two of them read the wrong way round at a glance:
getSecureAppPath()alone would allowb=/configon theresource/css|jsroute, sinceAPP_ROOTcontains it — and that route answers unauthenticated. What actually stops a forged base directory and file list is the HMACResourceBaseverifies over every query parameter, keyed by the password salt, before any action runs.AccountHistoryHelper::checkAccess()instead — which looks like a missing filter until you read it.The rest are recorded compactly: every
where()carrying a variable also carries a placeholder;ORDER BYis amatchover constants; array binds expand to one placeholder per element; every LDAP filter value goes throughldap_escape; the unauthenticated-bind bypass is closed three times over; nine foreign-key gaps each have a structural reason (polymorphic targets, ids fromactions.yaml, audit rows that must survive a user's deletion,parentId's0sentinel).Coverage, re-measured
97.20% (
22268/22910), against the 96.63% (21973/22740) recorded before roughly sixty tests were added over recent changes. The derived figures move with it:Base.php,BootstrapBase,CoreDefinitionsandCli/Initremain the four largest gaps, which is exactly what that section already predicts — they run before the container the tests build.Measured the way the section prescribes: pcov was in the "installed but disabled" state it warns about, so it was enabled rather than reinstalled, both suites were run with
--coverage-clover, the two reports merged by reading//filevia xpath (clover nests files inside<package>, so$xml->project->filefinds only the namespace-less ones), and the ini removed afterwards so later runs are not slowed.Documentation only — no code changes.