Skip to content

security: Phase-1 hardening — scope nested ballot/component routes, rate limits - #11

Merged
Putr merged 15 commits into
masterfrom
security/phase-1-hardening
Jun 13, 2026
Merged

security: Phase-1 hardening — scope nested ballot/component routes, rate limits#11
Putr merged 15 commits into
masterfrom
security/phase-1-hardening

Conversation

@Putr

@Putr Putr commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Phase-1 security hardening — web_engine

Additive only; no API contract change.

Fixes

  • Cross-tenant IDOR (High): ScopeRouteBindings middleware on the nested ballot/component/vote groups → 404 when a {ballot} is not in the {election} or a {component} is not in the {ballot}. Previously can: only authorized the {election}, so a caller could read/export/alter another election's ballot and (decrypted) votes.
  • Explicit election/ballot route binds so the check sees models even on routes that do not type the param (component listing).
  • Rate limiting: throttle:votes on the public vote-submission endpoints.
  • photo_url validation: owner personalization must be an http(s) URL (rejects javascript:/data: rendered on voter-facing pages).
  • Rate-limit hardening: TrustProxies set so the vote limiter buckets per real client, not the proxy IP (prevents collapsing the whole electorate onto one bucket).

Verification

  • PHPStan level 8: clean · php artisan test: 24 passed.
  • Live: routes wired; ip() honors X-Forwarded-For.

Tests

Proposed, not yet written — see PROPOSED_TESTS.md in the superproject (B4/E2/E3 + CR1).

Prod note

TrustProxies uses *; ensure the edge proxy sanitises X-Forwarded-For.

🤖 Generated with Claude Code

Putr and others added 15 commits June 10, 2026 13:20
The feature suite was entirely red at rest: no test used RefreshDatabase,
so the sqlite :memory: DB had no schema and every DB-backed test failed
with "no such table". Additionally BallotCrudTest hit stale URLs that no
longer match routes/api.php (ballot update is /{ballot}/update; the votes
endpoint is /{ballot}/vote, not /votes).

Add RefreshDatabase to the 3 feature tests and correct the stale URLs.
Suite now green (23 passed) on Laravel 9 — a baseline for the L9->12 upgrade.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Framework 9 -> 10. Bumped version-locked deps (phpunit 9->10, collision
6->7, larastan 1->2, spatie/laravel-ignition 1->2, paratest 6->7,
doctrine/dbal 2->3, goldspecdigital/laravel-eloquent-uuid 9->10), added
explicit php ^8.1 floor, and removed fruitcake/laravel-cors (CORS is
built into the framework since L9). Livewire kept at 2 (fine through L10;
the 2->3 jump comes with L11+).

Only code change: swap \Fruitcake\Cors\HandleCors for the framework's
\Illuminate\Http\Middleware\HandleCors in app/Http/Kernel.php (config/cors.php
already present). Tests green: 23 passed.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Livewire 2.12 -> 3.8. Done while still on Laravel 10 (Livewire 3 supports
L10) to isolate this from the L10->11 framework bump that follows. The two
PHP components (Session, RankedChoiceLivewire) need no changes: no
emit/listeners, and the v2 full-page layout macros (->extends()->slot())
still exist in v3. config/livewire.php (v2 format) is read fine via v3
defaults; class_namespace App\Http\Livewire still resolves nested
@livewire() components. Backend test suite green: 23 passed.

KNOWN FOLLOW-UP (frontend, not test-covered): app.js bundles Alpine v2
(require("alpinejs"), package.json alpinejs ^2.8.2) while Livewire 3 ships
its own Alpine v3 -> double-load conflict on interactive pages. The 5
Alpine directives (password show/hide on ballot-code + session-ballot) are
v3-compatible, but ballot-code may render on non-Livewire pages where
Livewire's Alpine isn't loaded. Needs a frontend strategy + Mix rebuild
(node toolchain) + BROWSER verification of the ballot voting page before
this is truly done. Not committed here.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Framework 10 -> 11. PHP floor ^8.2, phpunit ^11, collision ^8, larastan
renamed nunomaduro/larastan -> larastan/larastan ^3 (project is alive, just
repackaged). Livewire 3 already in place satisfies L11.

Dropped goldspecdigital/laravel-eloquent-uuid (no L11 release) and replaced
its Uuid trait with a local App\Models\Concerns\HasUuidV4 on the 4 affected
models (Vote, Ballot, BallotComponent, Election). The local trait reproduces
the package's behavior EXACTLY: a random Ramsey v4 UUID on create. Did NOT
switch to native HasUuids, which generates ordered/time-based UUIDs — those
would leak record creation order, unacceptable for secret-ballot Vote ids.

Tests green: 23 passed.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Framework 11 -> 12.62 — a maintenance-level leap, no app code changes
required. Tests green: 23 passed. The prior composer security advisory is
also resolved by the L12 dependency set.

web_engine now aligned with web_app and web_sender on Laravel 12.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Two issues that broke the ballot voting UI under Livewire 3:

1. Double Alpine. resources/js/app.js loaded a standalone Alpine v2
   (package.json alpinejs ^2.8.2) while Livewire 3 bundles and starts its
   own Alpine v3 -> two Alpine instances, broken x-data. Removed the
   standalone Alpine (and the dep); Livewire's Alpine v3 now handles the
   app's only directives (a password show/hide toggle, which is v3-safe).
   app.js rebuilt 83KB -> 975 bytes, zero Alpine.

2. Malformed Livewire asset URL. config/livewire.php (v2 format) had a
   duplicate 'asset_url' key whose second definition (env('APP_URL')) won,
   so Livewire 3 emitted <script src="http://engine.evote.local?id=..">
   with no /livewire/livewire.js path -> Alpine/Livewire never loaded.
   Removed the duplicate; asset_url back to null -> correct relative
   /livewire/livewire.js?id=.. (verified 200, Alpine present).

Verified by rendering a real ballot page: single Alpine (v3 via Livewire),
correct asset URL, x-data directive intact. Backend tests still 23 passed.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Renders the voter-facing ballot page (election + active ballot + matching
vote) and asserts it loads Livewire 3 assets at the correct relative path
"/livewire/livewire.js". Regression-guards the duplicate-asset_url bug from
3723ed7: if that path goes missing from the markup again, this fails.

Markup-level only — does not cover the webpack-built app.js (standalone
Alpine re-bundling would need a browser/Dusk test). Suite: 24 passed.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Fix the larastan include path (nunomaduro -> larastan/larastan rename),
drop the obsolete PHPStan-1 checkMissingIterableValueType param, set level 3.
Resolved all 49 findings by root cause, not line-by-line:

- Models: added relation return types (BelongsTo/HasMany) + @Property /
  @property-read docblocks for DB columns, accessors and relations on
  Ballot/Vote/BallotComponent/Election. Fixed Election's malformed @method
  PHPDoc. This cleared the bulk (undefined property on $ballot->election,
  ->components, ->cast_votes, $vote->ballot, etc.).
- Factories: added @extends Factory<Model> generics so factory()->make()
  resolves to the concrete model (cleared the in-app component test errors).
- Untyped closures over relation collections: typed the $component params
  in BallotService + Session so property access resolves.
- Scaffolding: removed the imprecise Laravel-9 "@var array" docblocks that
  conflicted (covariance) with L11's typed framework base properties
  (Handler, Kernel, 4 middleware, 2 providers, TrustProxies).
- Small: Authenticate::redirectTo() now declares ?string; removed an
  orphaned @var docblock in RouteServiceProvider.

Behavioral (pre-approved):
- config/auth.php provider model App\Models\User (nonexistent) -> ApiUser,
  the actual header-token authenticated user.
- Session Livewire: declared `public array $componentTree` so it is a real
  Livewire property and reaches the session-ballot view under Livewire 3
  (was a dynamic prop, lost in v3). Redis::set(k,1,'ex',60) -> setex(k,60,1)
  (identical behavior, unambiguous signature).

No baseline/ignores used. Tests: 24 passed.
Level 4 surfaced several real latent bugs in the evote: console commands,
all now fixed (no baseline/ignores):

- ElectionCreate: abstainable was compared `=== 'Yes'` against a lowercase
  string / bool, so it was ALWAYS stored false (CLI elections could never be
  abstainable). Normalize the input to a real bool and store it directly.
- BallotComponentCreate: handle() read $this->option('version'), but the
  signature defines the option as `--variant` (default "-1" = latest). The
  wrong key fell through to Symfony's global boolean --version flag, so the
  value was never read and "use latest" never worked. Read 'variant', and
  resolve the "-1" => latest sentinel BEFORE validation (was an unreachable
  block guarded by a precedence-buggy `!$version == -1`). "Use latest" works.
- BallotCodesGenerate + BallotComponentCreate: Ballot::where(['id', $id])
  passed a flat array, compiling to `WHERE 0 = 'id' AND 1 = $id` which never
  matches a real UUID -> the ballot lookup always failed. Use where('id',$id).
- BallotCodesGenerate: quantity validation `!is_int((int)$x)` was dead and let
  negatives through; replaced with `$quantity < 1`.

Tests: 24 passed.
Level 5 (callback/argument type checks):
- Added generics to the model relation methods
  (@return HasMany<X, $this> / BelongsTo<X, $this>) so ->get() yields a
  correctly-typed collection. This made the typed closures over those
  collections (Session filter, Election contains, BallotService map/reduce)
  type-check instead of seeing a base Model.
- Typed BallotService::$components registry as
  array<string, array<string, class-string<BallotComponentType>>>, so the
  dynamic ::getSubmissionValidator()/::strings() static calls resolve.
- Declared the registry's contract on BallotComponentType: static
  $needsOptions, $optionsValidator, and abstract strings() (all four concrete
  components already implement these).

Real bug fixed (ranked-choice tallying):
- RankedChoice counted "votes cast" via countBy(fn => bool)->first(), which
  returns the size of whichever group (cast vs not-cast) happens to appear
  first in the vote list -> the majority denominator (max($state) >=
  cast/2 + 1) could be computed against the wrong number. Replaced with
  filter(...)->count() to count the votes actually cast.

No baseline/ignores. Tests: 24 passed.
Swept levels 6-8 (missing-type, union, null-safety) across the domain code
via parallel agents — Models, Services, BallotComponents (+ their tests),
controllers, Livewire components, and CLI commands. Framework scaffolding
(Exceptions, Http/Kernel, Middleware, Providers, Console/Kernel) is excluded
from the strict sweep in phpstan.neon (still covered through level 5).

Almost entirely type annotations (return/param/property types, array<K,V>
and list<T> shapes, generics like HasMany<X,$this> and
Collection<int,X>); no behavior changes intended. Notable type correction:
BallotComponentType::calculateResults() (and RankedChoice runIteration/
getTotals) are now typed `array<int, Vote>` — they are always called with
Vote models, not plain arrays.

Tests: 24 passed. No baseline/ignores used.

7 errors remain at level 8, all genuine null-safety / real-bug findings that
need a decision rather than a type fix (BallotController vote lookup,
Controller::getOwner auth nullability, BallotService valuesToCsv nullable) —
left untouched and being reviewed separately.
…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.
Phase-1 hardening (no API contract change):
- Add ScopeRouteBindings middleware on the nested election/ballot/component and
  vote route groups: 404 when a {ballot} is not in the {election} or a
  {component} is not in the {ballot}. Previously can: only authorized the
  {election}, so a caller could reach another election's ballot/votes.
- Add explicit election/ballot route binds so the check sees models even on
  routes that don't type the param (component listing).
- Rate-limit the public vote-submission endpoints (throttle:votes).
- Validate owner personalization photo_url as an http(s) URL (reject
  javascript:/data: injection rendered on voter-facing pages).
- Trust forwarded headers (TrustProxies) so the vote limiter buckets per real
  client instead of collapsing onto the proxy IP.
…rdening

# Conflicts:
#	app/Http/Middleware/TrustProxies.php
#	composer.json
#	composer.lock
@Putr
Putr changed the base branch from upgrade/laravel-12 to master June 13, 2026 08:30
@Putr
Putr marked this pull request as ready for review June 13, 2026 08:35
@Putr
Putr merged commit dd1bf4c into master Jun 13, 2026
1 check passed
@Putr
Putr deleted the security/phase-1-hardening branch June 13, 2026 08:35
Putr added a commit that referenced this pull request Jun 22, 2026
The voter-facing hint was a loose lang key in each form blade, bypassing
getStrings()/ComponentMetadata (the string source name/description flow through).
Add 'hint' to every component's getStrings(), expose it via a BallotComponent
$type_hint accessor (mirrors $type_name), and have the forms read it from there.
The component tree/API now also carries the hint. A new type can't ship a form
with a missing/mismatched hint silently.

Tests: type_hint resolves per type + null for an unregistered type. Engine
suite green (320); PHPStan L8 clean.
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