Bearer read-scope API keys bypass ticket visibility entirely — confirm this is an intentional, documented decision #70

Closed
opened 2026-09-01 00:07:09 -04:00 by jared · 1 comment
Owner

Severity: High (design decision requiring explicit confirmation, not necessarily a bug)

api/tickets_api.php — both the single-ticket read path (ticketModel->getTicketById($ticketId), no canUserAccessTicket() call) and the list/triage path ($user passed as null, so TicketModel::getAllTickets()'s visibility filter is skipped entirely per its own logic) bypass the app's entire visibility subsystem. There's an existing code comment explaining this is intentional ("trusted server credential, not an end user").

Concern: Per the README and admin UI, a read-scope API key can be generated for any external tool, not just hwmonDaemon — there's no way to scope a key to public-visibility-only tickets. Any read-scope key, however it's issued or to whomever, gets blanket read access to Confidential-visibility ticket titles, descriptions, and comments. If a key is ever handed to a less-trusted integration or third party, this is a full confidentiality bypass with no visibility distinction in the API surface itself.

Ask: Confirm this is the intended trust model (all API keys are equally trusted regardless of scope, by design) and if so, document it explicitly in the README's API Key Management / Bearer API sections so it's not accidentally assumed to respect visibility. If it's NOT intended, consider adding an optional visibility-scoping flag when generating a key.

**Severity:** High (design decision requiring explicit confirmation, not necessarily a bug) `api/tickets_api.php` — both the single-ticket read path (`ticketModel->getTicketById($ticketId)`, no `canUserAccessTicket()` call) and the list/triage path (`$user` passed as `null`, so `TicketModel::getAllTickets()`'s visibility filter is skipped entirely per its own logic) bypass the app's entire visibility subsystem. There's an existing code comment explaining this is intentional ("trusted server credential, not an end user"). **Concern:** Per the README and admin UI, a `read`-scope API key can be generated for *any* external tool, not just hwmonDaemon — there's no way to scope a key to public-visibility-only tickets. Any `read`-scope key, however it's issued or to whomever, gets blanket read access to Confidential-visibility ticket titles, descriptions, and comments. If a key is ever handed to a less-trusted integration or third party, this is a full confidentiality bypass with no visibility distinction in the API surface itself. **Ask:** Confirm this is the intended trust model (all API keys are equally trusted regardless of scope, by design) and if so, document it explicitly in the README's API Key Management / Bearer API sections so it's not accidentally assumed to respect visibility. If it's NOT intended, consider adding an optional visibility-scoping flag when generating a key.
jared added the needs-decisionpriority/highsecurity labels 2026-09-08 10:15:47 -04:00
Author
Owner

Decision: restrict by default. Fixed in commit 6609320 (merged to main in d0a7630).

Added see_all_visibility to api_keys (migration 006), defaulting to false for both new and existing keys. api/tickets_api.php now restricts both the single-ticket read and list/triage paths to public-visibility tickets unless a key is explicitly marked see_all_visibility (new checkbox in the API Key Management admin UI). Documented in the README's API Key Management / Bearer API sections.

Verified against real MariaDB with public/confidential/internal test tickets: a public-only key's list only returns the public ticket, and per-ticket access checks correctly exclude both confidential (assigned and unassigned) and internal tickets; a see_all_visibility key sees all three, unchanged from prior behavior. Testing also caught and fixed a real edge case: the synthetic 'no access' user needed user_id=-1, not 0, since canUserAccessTicket()'s PHP-level (int) cast makes (int)null === 0 for an unassigned confidential ticket.

Decision: restrict by default. Fixed in commit 6609320 (merged to main in d0a7630). Added see_all_visibility to api_keys (migration 006), defaulting to false for both new and existing keys. api/tickets_api.php now restricts both the single-ticket read and list/triage paths to public-visibility tickets unless a key is explicitly marked see_all_visibility (new checkbox in the API Key Management admin UI). Documented in the README's API Key Management / Bearer API sections. Verified against real MariaDB with public/confidential/internal test tickets: a public-only key's list only returns the public ticket, and per-ticket access checks correctly exclude both confidential (assigned and unassigned) and internal tickets; a see_all_visibility key sees all three, unchanged from prior behavior. Testing also caught and fixed a real edge case: the synthetic 'no access' user needed user_id=-1, not 0, since canUserAccessTicket()'s PHP-level (int) cast makes (int)null === 0 for an unassigned confidential ticket.
jared closed this issue 2026-09-12 01:21:14 -04:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: LotusGuild/tinker_tickets#70