Skip to content

Fix SQLAlchemy and operator misuse causing broken query predicates and cross-tournament access#1721

Open
Copilot wants to merge 4 commits into
masterfrom
copilot/fix-sqlalchemy-column-expressions
Open

Fix SQLAlchemy and operator misuse causing broken query predicates and cross-tournament access#1721
Copilot wants to merge 4 commits into
masterfrom
copilot/fix-sqlalchemy-column-expressions

Conversation

Copilot AI commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Python's and operator is unsafe with SQLAlchemy clause elements: in SQLAlchemy 2.0 it raises TypeError (routes return 500); in older versions it short-circuits to the right-hand operand, silently dropping the id = X filter and enabling cross-tournament entity access.

Changes

routes/util.py

  • team_dependency: replace and with &
  • round_dependency / match_dependency: rounds and matches have no direct tournament_id; replace the broken and chain with proper JOINs through stage_items → stages to enforce tournament scoping

routes/courts.py

  • create_court post-insert fetch: replace and with &

tests/.../teams_test.py

  • Add test_cross_tournament_team_access_denied: asserts that a PUT on tournament A's URL using a team_id belonging to tournament B returns 404
# Before (BROKEN — evaluates to just the right-hand side)
teams.select().where(teams.c.id == team_id and teams.c.tournament_id == tournament_id)

# After (CORRECT)
teams.select().where((teams.c.id == team_id) & (teams.c.tournament_id == tournament_id))

# round_dependency — no tournament_id on rounds table, use JOIN
rounds.select()
    .join(stage_items, rounds.c.stage_item_id == stage_items.c.id)
    .join(stages, stage_items.c.stage_id == stages.c.id)
    .where((rounds.c.id == round_id) & (stages.c.tournament_id == tournament_id))

…nament regression test

- Replace Python `and` with `&` in team_dependency and courts.py create_court
- Fix round_dependency and match_dependency to use proper JOINs through
  stage_items/stages since those tables lack a tournament_id column
- Add test_cross_tournament_team_access_denied regression test verifying
  a team from tournament B returns 404 when accessed via tournament A's URL
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jun 25, 2026

Copy link
Copy Markdown

Deploying bracket-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: b0d4db7
Status: ✅  Deploy successful!
Preview URL: https://257cc5d6.bracket-docs.pages.dev
Branch Preview URL: https://copilot-fix-sqlalchemy-colum.bracket-docs.pages.dev

View logs

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jun 25, 2026

Copy link
Copy Markdown

Deploying bracket-frontend with  Cloudflare Pages  Cloudflare Pages

Latest commit: b0d4db7
Status: ✅  Deploy successful!
Preview URL: https://7a0101f4.bracket-abd.pages.dev
Branch Preview URL: https://copilot-fix-sqlalchemy-colum.bracket-abd.pages.dev

View logs

Copilot AI changed the title [WIP] Fix incorrect use of Python and with SQLAlchemy column expressions Fix SQLAlchemy and operator misuse causing broken query predicates and cross-tournament access Jun 25, 2026
Copilot AI requested a review from evroon June 25, 2026 10:46
@codecov

codecov Bot commented Jun 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.83%. Comparing base (366c4c4) to head (f1c1338).
⚠️ Report is 36 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1721      +/-   ##
==========================================
+ Coverage   94.80%   94.83%   +0.02%     
==========================================
  Files         120      120              
  Lines        4387     4393       +6     
==========================================
+ Hits         4159     4166       +7     
+ Misses        228      227       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@evroon evroon marked this pull request as ready for review June 25, 2026 13:06
@evroon

evroon commented Jun 25, 2026

Copy link
Copy Markdown
Owner

@copilot fix ci errors

Copilot AI commented Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

@copilot fix ci errors

Fixed in the latest commit. The mypy errors ("Join" has no attribute "where") were caused by sqlalchemy-stubs==0.4 annotating FromClause.join() as returning Join — so Select.join() was also inferred to return Join, which lacks .where().

The fix replaces rounds.select().join(...).where(...) with rounds.select().select_from(rounds.join(...)).where(...) (and similarly for matches). select_from() accepts a FromClause (which Join is a subclass of) and returns Select, satisfying the type checker while producing the same SQL.

@evroon

evroon commented Jun 26, 2026

Copy link
Copy Markdown
Owner

@copilot fix ci errors

Copilot AI commented Jun 26, 2026

Copy link
Copy Markdown
Contributor Author

@copilot fix ci errors

Fixed in the latest commit. Two pylint issues were reported:

  • W0621 in routes/util.py:49 — renamed local variable stages to stages_result in stage_dependency to stop it shadowing the stages table import
  • C0301 in teams_test.py:11,13 — split the two long import lines into multi-line form

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.

2 participants