test(contract): cover the 10 public endpoints that had no wire proof - #167
Merged
Conversation
gate-25 contract-coverage: 10 -> 0 (PASS). Measured at gate package 365fa31.
Ten `#[NoAdminRequired]` / `#[PublicPage]` endpoints were registered in
appinfo/routes.php with no Newman collection and no PHPUnit test touching
them. gate-25 matches on an actual `->method(` call in a *Test.php, so these
could not be satisfied by annotation — only by really calling them.
AppOverrideController saveUser, getUser, clearUser (8 tests)
AutomationsController disable (2 tests)
RulesController testAll (4 tests)
ShopController githubSearch, githubInstall (5 tests, new file)
GitHubSyncController link, pull (7 tests, new file)
ApplicationsController saveManifest (2 tests, new file)
They pin authorisation and input validation rather than restating the happy
path, because that is where a network-facing endpoint actually fails:
* anonymous -> 401 AND the service is never called (expects(never())), so a
guard that returns the right status while still doing the work would fail;
* non-owner -> 403 with a REAL PermissionResolver, not a stubbed verdict, so
the role logic is genuinely exercised — including a VIEWER being refused a
write, the case a coarse "has any role" check would wrongly allow;
* malformed owner/repo/ref -> 400 BEFORE any outbound GitHub call, which is
the guard that keeps an attacker-supplied string out of the request URL.
Proven not blind: replacing the `->githubSearch()` calls in ShopControllerTest
with a dummy name flips gate-25 PASS -> FAIL(1), and restoring them returns it
to PASS.
Verified by running PHPUnit locally in a DISPOSABLE container against this
worktree (nextcloud:34.0.0-apache, PHP 8.4) rather than the shared instance,
whose bind mount points at the main checkout and would have tested that lib/
instead of this branch's: 797 tests, 2480 assertions, OK (was 769).
One incidental finding while writing these: ShopController::githubSearch reads
`$result['brokerUsed']` unguarded. Every documented return path of
GitHubCatalogService::search() supplies it, so the fixture was at fault and was
corrected — but note search() has a `return $cached` path, so a cache entry
written before that key existed would surface as an undefined-key warning.
Not changed here; recorded for whoever touches that cache.
Contributor
Quality Report — ConductionNL/openbuild @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| test-l10n | ✅ | ||||
| check-gitignore | ✅ | ||||
| check-nc-floor | ✅ | ||||
| composer | ✅ | ✅ 101/101 | |||
| npm | ✅ | ✅ 654/654 | |||
| PHPUnit | ✅ | ||||
| Newman | ⏭️ | ||||
| Playwright | ❌ | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-08-09 21:40 UTC
Download the full PDF report from the workflow artifacts.
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.
gate-25 contract-coverage: 10 → 0 (PASS). Measured at gate package
365fa31d09a26f980e6dc76cb0800575ef005a4e.Ten
#[NoAdminRequired]endpoints were registered inappinfo/routes.phpwith no Newman collection and no PHPUnit test touching them. gate-25 matches on an actual->method(call inside a*Test.php, so these could not be satisfied by an annotation — only by really calling them.AppOverrideControllersaveUser,getUser,clearUserAutomationsControllerdisableRulesControllertestAllShopControllergithubSearch,githubInstallGitHubSyncControllerlink,pullApplicationsControllersaveManifestThey pin authorisation, not the happy path
That is where a network-facing endpoint actually fails, so that is what these assert:
expects(self::never())). A guard that returns the right status while still doing the work would fail these.PermissionResolver, not a stubbed verdict — so the role logic is genuinely exercised. That includes a viewer being refused a write, which is exactly the case a coarse "has any role" check would wrongly allow.owner/repo/ref→ 400 before any outbound GitHub call — the guard that keeps an attacker-supplied string out of the request URL.Proven not blind
Replacing the
->githubSearch()calls inShopControllerTestwith a dummy name flips gate-25 PASS → FAIL(1); restoring them returns it to PASS.How it was verified
PHPUnit was run locally in a disposable container (
nextcloud:34.0.0-apache, PHP 8.4) mounted against this worktree — deliberately not the shared dev instance, whose bind mount points at the main checkout and would have exercised thatlib/instead of this branch's.One incidental finding, recorded not fixed
ShopController::githubSearchreads$result['brokerUsed']unguarded. Every documented return path ofGitHubCatalogService::search()supplies it, so my initial fixture was at fault and was corrected rather than the controller. Butsearch()has areturn $cachedpath — a cache entry written before that key existed would surface as an undefined-key warning in production. Not changed here; flagged for whoever touches that cache.