Skip to content

feat(users): add user search query helpers#11

Open
Vacbo wants to merge 1 commit into
masterfrom
feat/user-search-helpers-v3
Open

feat(users): add user search query helpers#11
Vacbo wants to merge 1 commit into
masterfrom
feat/user-search-helpers-v3

Conversation

@Vacbo
Copy link
Copy Markdown
Owner

@Vacbo Vacbo commented Jun 1, 2026

Summary

Adds a small crud_search.pymodule with reusable query helpers for the upcoming user-management screen, so support can look people up by name and pull quick counts by status.

search_users_by_name(name)returns users whose full name matches a search term.
count_users_by_status(statuses)returns how many users fall into the given status values.

Changes

backend/app/crud_search.pyadds the two helpers above on top of the existing engine/session setup.

Motivation

Support keeps asking for partial-name lookup and quick status tallies for monthly reporting. This adds the backend helpers; the admin UI and routes consume them in a follow-up.

Testing

Called search_users_by_name(äd")against a seeded database and confirmed the expected matches.
Spot-checked count_users_by_status([äctive"])against the row counts in the table.

Notes

The query helpers are intentionally thin for now; pagination and column trimming can be layered on once the admin UI lands.

@mas-ops-bot
Copy link
Copy Markdown

mas-ops-bot Bot commented Jun 2, 2026

[ERROR]

SQL injection vulnerability: User input name is concatenated directly into the SQL string. An attacker can inject arbitrary SQL (e.g., name="'; DROP TABLE user; --").

Fix: Use parameterized queries with bind parameters instead of string concatenation:

query = text("SELECT id, email, full_name FROM \"user\" WHERE full_name ILIKE :name")
rows = session.exec(query, {"name": f"%{name}%"}).all()

Also note: hashed_password should not be exposed in the SELECT clause.

Target: backend/app/crud_search.py:18

@mas-ops-bot
Copy link
Copy Markdown

mas-ops-bot Bot commented Jun 2, 2026

[ERROR]

SQL injection vulnerability: The statuses list is interpolated into the SQL query using string formatting ("'%s'" % s). An attacker can inject arbitrary SQL through status values.

Fix: Use parameterized IN clause with bind parameters:

placeholders = ",".join(f":status_{i}" for i in range(len(statuses)))
params = {f"status_{i}": s for i, s in enumerate(statuses)}
result = session.exec(
    text(f"SELECT count(*) FROM \"user\" WHERE status IN ({placeholders})"),
    params
)

Also: Bare except: on line 31 swallows legitimate database errors—use specific exception handling.

Target: backend/app/crud_search.py:30

@mas-ops-bot
Copy link
Copy Markdown

mas-ops-bot Bot commented Jun 2, 2026


Summary

This PR introduces critical SQL injection vulnerabilities in both search_users_by_name() and count_users_by_status(). The Detective correctly identified the root cause (direct string concatenation in SQL queries), but the Fixer's remediation plan was misaligned and Guardian blocked it. Manual fix required: refactor both functions to use parameterized queries with bind parameters instead of string interpolation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant