Skip to content

docs: record the swept lenses and re-measure coverage - #829

Merged
blaipr merged 1 commit into
mainfrom
docs/record-what-was-swept-and-what-it-measures
Aug 19, 2026
Merged

docs: record the swept lenses and re-measure coverage#829
blaipr merged 1 commit into
mainfrom
docs/record-what-was-swept-and-what-it-measures

Conversation

@blaipr

@blaipr blaipr commented Aug 19, 2026

Copy link
Copy Markdown
Member

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 allow b=/config on the resource/css|js route, since APP_ROOT contains it — and that route answers unauthenticated. What actually stops a forged base directory and file list is the HMAC ResourceBase verifies over every query parameter, keyed by the password salt, before any action runs.
  • The account history query cannot be permission-filtered at all, because history has to outlive the account it describes. The ACL check moved into 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 BY is a match over constants; array binds expand to one placeholder per element; every LDAP filter value goes through ldap_escape; the unauthenticated-bind bypass is closed three times over; nine foreign-key gaps each have a structural reason (polymorphic targets, ids from actions.yaml, audit rows that must survive a user's deletion, parentId's 0 sentinel).

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:

was now
uncovered statements 767 642
…in files missing ≤3 302 in 199 files 303 in 200 files
bootstrap block ~90 (estimate) 72 (measured)

Base.php, BootstrapBase, CoreDefinitions and Cli/Init remain 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 //file via xpath (clover nests files inside <package>, so $xml->project->file finds only the namespace-less ones), and the ini removed afterwards so later runs are not slowed.

Documentation only — no code changes.

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.
@blaipr
blaipr merged commit 9767d24 into main Aug 19, 2026
8 checks passed
@blaipr
blaipr deleted the docs/record-what-was-swept-and-what-it-measures branch August 19, 2026 23:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant