Skip to content

Commit d40955d

Browse files
committed
Fix null-safety + ballot-integrity bugs; web_engine clean at PHPStan level 8
Resolves the final 7 level-8 findings, all genuine (not annotation gaps): - BallotController::vote()/voteComponent(): removed a redundant, null-unsafe re-fetch `Vote::find(['code'=>..,'ballot_id'=>..])->first()` (find() with an array treats the values as primary keys, so ballot_id was ignored and the result was ?Vote used unguarded). The vote is already fetched and validated at the top of each method; reuse it. - SECURITY: the guard `!$vote->ballot->id == $ballot->id` had an operator- precedence bug (`!$vote->ballot->id` bound first), making the ballot-match check dead code — a voting code for one ballot could be submitted against another. Fixed to `$vote->ballot->id !== $ballot->id`. - Controller::getOwner(): Auth::user() is ?ApiUser; narrowed via @var (all callers run behind auth.api middleware, so non-null at runtime). - BallotService::resultsCsv(): pass `$vote->values ?? []` to valuesToCsv() (castVotes() already filters non-null, so the fallback never triggers). web_engine now passes PHPStan level 8 (domain code; framework scaffolding excluded) with zero baseline/ignore entries. Tests: 24 passed.
1 parent 070b39c commit d40955d

3 files changed

Lines changed: 9 additions & 10 deletions

File tree

app/Http/Controllers/BallotController.php

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ public function view(Election $election, Ballot $ballot, Request $request, Ballo
2929
/** @var Vote|null $vote */
3030
$vote = Vote::find($code);
3131

32-
if (!$vote || !$vote->ballot->id == $ballot->id) {
32+
if (!$vote || $vote->ballot->id !== $ballot->id) {
3333
return view('404', ['code' => 404]);
3434
}
3535

@@ -62,7 +62,7 @@ public function vote(Election $election, Ballot $ballot, Request $request): View
6262
/** @var Vote|null $vote */
6363
$vote = Vote::find($code);
6464

65-
if (!$vote || !$vote->ballot->id == $ballot->id) {
65+
if (!$vote || $vote->ballot->id !== $ballot->id) {
6666
return view('404', ['code' => 404]);
6767
}
6868

@@ -81,9 +81,7 @@ public function vote(Election $election, Ballot $ballot, Request $request): View
8181
return view('vote-failed', ['election' => $election, 'ballot' => $ballot, 'errors' => $errors]);
8282
}
8383

84-
$code = $request->input('code');
8584
$values = $request->except(['code', '_token']); // Could get the component slugs and say ->only
86-
$vote = Vote::find(['code' => $code, 'ballot_id' => $ballot->id])->first();
8785
$vote->values = $values;
8886
$vote->save();
8987

@@ -101,7 +99,7 @@ public function voteComponent(Election $election, Ballot $ballot, BallotComponen
10199
/** @var Vote|null $vote */
102100
$vote = Vote::find($code);
103101

104-
if (!$vote || !$vote->ballot->id == $ballot->id) {
102+
if (!$vote || $vote->ballot->id !== $ballot->id) {
105103
return view('404', ['code' => 404]);
106104
}
107105

@@ -120,9 +118,6 @@ public function voteComponent(Election $election, Ballot $ballot, BallotComponen
120118
return view('vote-failed', ['election' => $election, 'ballot' => $ballot, 'errors' => $errors]);
121119
}
122120

123-
$code = $request->input('code');
124-
$vote = Vote::find(['code' => $code, 'ballot_id' => $ballot->id])->first();
125-
126121
$values = $request->except(['code', '_token']); // Could get the component slugs and say ->only
127122
$oldValues = $vote->values;
128123
if (!is_array($oldValues)) {

app/Http/Controllers/Controller.php

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,10 @@ public function basicResponse(int $code = 200, array $extra = []): \Illuminate\H
5353

5454
protected function getOwner(): string
5555
{
56-
return Auth::user()->owner;
56+
// Routes calling getOwner() run behind the auth.api middleware, which
57+
// logs in an ApiUser, so Auth::user() is never null here.
58+
/** @var \App\Models\ApiUser $user */
59+
$user = Auth::user();
60+
return $user->owner;
5761
}
5862
}

app/Services/BallotService.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ public function resultsCsv(Ballot $ballot): string
138138
$results_per_component = $components->map(function (BallotComponent $component) use ($votes) {
139139
$componentClass = $this->getBallotComponentClassInstance($component['type'], $component['version'], $component['settings']);
140140
return $votes->map(function (Vote $vote) use ($componentClass, $component) {
141-
return $componentClass::valuesToCsv($vote->values, $component->id);
141+
return $componentClass::valuesToCsv($vote->values ?? [], $component->id);
142142
});
143143
});
144144

0 commit comments

Comments
 (0)