fix(gate-7): drop a redundant (string) cast that hid a real ownership guard - #196
Merged
Merged
Conversation
… guard `AppOverrideController::getUser()` is owner-scoped: the UID is never a request parameter, it comes from the session and is handed to `AppOverrideService::getUserDelta(appId:, uid:)`. That is gate-7's Pattern 6 (session-identity hand-off) and it should pass. It did not, and the reason is one token. Four arms on the shipped file, changing nothing else: A uid: (string)$user->getUID() FAIL <- shipped B uid: $user->getUID() PASS C $uid = (string)$user->getUID(); ... uid: $uid FAIL D appId: (string)$appId, uid: $user->getUID() PASS E $uid = $user->getUID(); ... uid: $uid PASS The guard is identical in all five. Only its spelling moves the verdict, so this is a gate-7 false positive, not an app defect — and the tempting repair (add a guard) would have added a redundant check to an endpoint that already had one. `IUser::getUID()` is documented `@return string` since 8.0.0 and `getUserDelta()` declares `string $uid`, so the cast was a no-op. Psalm and PHPStan are the control on that. gate-7 on lib/Controller/*.php: 7 -> 6.
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 | ✅ | ✅ 106/106 | |||
| npm | ✅ | ✅ 654/654 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| PHPUnit | ✅ | ||||
| Newman | ⏭️ | ||||
| Playwright | ✅ | ||||
| Hydra gates | ❌ |
Quality workflow — 2026-08-12 23:58 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.
What
Drop one redundant
(string)cast inAppOverrideController::getUser().Why — this is a gate-7 false positive, and the obvious repair would have been wrong
getUser()is already owner-scoped. The UID is never a request parameter; it comes from the session and is handed toAppOverrideService::getUserDelta(appId:, uid:). That is exactly gate-7's Pattern 6 — session-identity hand-off, and the method's own docblock says so.gate-7 flagged it anyway. Four arms on the shipped file, changing nothing but the expression carrying the identity:
uid: (string)$user->getUID()uid: $user->getUID()$uid = (string)$user->getUID();…uid: $uidappId: (string)$appId, uid: $user->getUID()$uid = $user->getUID();…uid: $uidThe guard is identical in all five arms. Only its spelling moves the verdict. Arm D rules out "any cast in the call breaks it" — casting an unrelated argument is harmless. Arm C rules out "hoist it into a local" as the fix: the cast defeats the recogniser through a variable too.
So the finding was never about this endpoint's authorization. Had it been treated as real, the repair would have been to add a guard to an endpoint that already had one — which is precisely the wrong repair the gate's own preamble warns about.
Is dropping the cast safe?
IUser::getUID()is documented@return string(since 8.0.0,lib/public/IUser.phponstable34), andAppOverrideService::getUserDelta()declaresstring $uidunderdeclare(strict_types=1). The cast was a no-op. Psalm and PHPStan are the control — ifgetUID()could return anything else here, they fail this PR.Nothing can now return
200that previously errored: the change is inside an argument expression on a call that already ran.What this does NOT do
gate-7 on
lib/Controller/*.phpgoes 7 → 6. It does not turn theHydra Gatescell green —gate-19(158 scenarios),gate-26(4 components) andgate-49(2 methods) are all still red ondevelopment, and the remaining six gate-7 findings (RulesController::evaluate/schema/testAll,ShopController::githubInstall,StoreController::search/install) are untriaged and deliberately not touched here.Fleet note
The
(string)$user->getUID()spelling occurs 7 times across the six apps in this sweep (openbuild 5, launchpad 2), but only 1 of the 20 live gate-7 findings is this class. Reported as measured, not extrapolated — the other 12 fleet apps are unmeasured.Scope
One token in one file.