130 Commits

Include renames

Author	SHA1	Message	Date
jaredandClaude Opus 5.5	ad201691e4	Fix ticket page 500 from unbounded audit timeline (#112) ... A ticket updated by hwmonDaemon had ~52k audit rows; getTicketTimeline() loaded all of them and exhausted PHP's 128MB memory limit, so the ticket page returned 500. Not related to the MCP server. - getTicketTimeline() takes a limit (newest first). The ticket page shows the latest 500 events with a note when older ones are omitted; the JSON export caps at 5000. - create_ticket_api.php: the description is refreshed on every run, and that alone wrote a reason-only audit row every few minutes per open ticket. Audit only real title/priority changes. - create_ticket_api.php: after creating a brand-new ticket the dedup retry loop fell through into a second iteration on a closed connection, appending a 500 error body after the success response. Exit instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X	2026-09-25 15:34:44 -04:00
jaredandClaude Opus 5.5	9e462f7f00	Extract ticket assignment from assign_ticket.php into AssignmentService (#111) ... The access check, the admin/creator/current-assignee permission rule, unassign/assign, the audit log, the optional Matrix assignment notification and the stats-cache invalidation move into services/AssignmentService.php for reuse by the MCP assign_ticket tool. Error messages and status codes are unchanged. One deliberate difference: assign_ticket.php's early error responses (400/403/404) used a bare echo and so omitted the CSRF token that bootstrap had just rotated. Every response now goes through apiRespond(), which includes it. The front end already resyncs its token from any response body, so this is compatible, and a failed assign can no longer leave the page holding a stale token. Verified over real HTTP: the assignee can reassign (200); a user who can see the ticket but isn't admin/creator/assignee gets 403 'Permission denied'. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X	2026-09-24 19:13:31 -04:00
jaredandClaude Opus 5.5	d5832fb58a	Extract comment creation from add_comment.php into CommentService (#111) ... Validation, the ticket access check, reply-parent validation, @mention extraction (audit-logged, notified only to mentioned users who can see the ticket), and comment/watcher notifications move into services/CommentService.php, so the MCP add_comment tool runs one code path with the web UI. add_comment.php keeps session, CSRF, JSON parsing and response codes. Error messages and status codes are unchanged; the extracted body diffs against the original only where each 'emit error and exit' became a 'return [..., http_status]'. Verified the web endpoint over real HTTP: a comment is trimmed, saved with its @mention and the rotated CSRF token returned; a confidential ticket the user can't see still gets 403 'Access denied'; empty text still gets 400. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X	2026-09-24 19:13:31 -04:00
jaredandClaude Opus 5.5	6ce3380d6a	Move ApiTicketController out of update_ticket.php into its own file (#111) ... The partial-update controller (status transitions incl. requires_comment with the comment in the same transaction, field edits, visibility, audit delta, status-change notifications) was defined inline inside api/update_ticket.php, so nothing else could reuse it. Moved it verbatim to controllers/ApiTicketController.php so the MCP update_status tool can run the exact same code path as the web UI; update_ticket.php now just require_once's it. The class body is byte-identical to the original (diffed against HEAD, modulo the 4-space dedent). Verified the web endpoint over real HTTP with a real session + CSRF: Open -> In Progress succeeds, In Progress -> Closed without a comment is refused with requires_comment (400), and with a comment closes the ticket and persists the reason. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X	2026-09-24 19:13:31 -04:00
jaredandClaude Sonnet 5	6609320c83	Restrict API keys to public-visibility tickets by default (#70) ... api/tickets_api.php (both the single-ticket read and the list/triage path) bypassed ticket visibility entirely for any 'read'-scope key, regardless of who it was issued to or what it was for — any key got blanket read access to Confidential and Internal ticket titles, descriptions, and comments, with no way to scope a key more narrowly. Added see_all_visibility to api_keys (migration 006), defaulting to false for both new and existing keys — the prior blanket-access behavior is what's being restricted here, so unlike scope's own un-migrated-database fallback (which defaults toward preserving old behavior), a missing/null value here defaults to the new, restrictive one. An admin can opt a specific key in via a new checkbox in the API Key Management UI when it genuinely needs the full queue. tickets_api.php now builds a synthetic "no special access" user and runs it through TicketModel's existing per-user visibility plumbing (getVisibilityFilter/canUserAccessTicket) instead of a separate SQL path, so this stays in lockstep with however visibility rules evolve for real users. That synthetic user_id is -1, not 0: testing surfaced that canUserAccessTicket()'s confidential-ticket check does a PHP-level (int) cast, and (int)null === 0, so an unassigned confidential ticket's NULL assigned_to would otherwise false-positive-match a user_id of 0. Verified against real MariaDB with public/confidential/internal test tickets: a public-only-scoped key's list only returns the public ticket, and canUserAccessTicket() correctly returns false for both the confidential ticket (unassigned, then reassigned to a real user — both cases) and the internal one; a see_all_visibility key sees all three, unchanged from the prior behavior. Also verified createKey()/ validateKey()'s default-false and explicit-true paths round-trip correctly through the real DB. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0117oBw2jN4kALYeS8HPq4zV	2026-09-12 01:17:53 -04:00
jaredandClaude Sonnet 5	e91f4547b6	Log watch/unwatch actions to the audit trail (#93) ... api/watch_ticket.php performed the ticket_watchers INSERT IGNORE/DELETE directly with no AuditLogModel call, unlike every other ticket-adjacent mutation (comments, attachments, dependencies, status/field changes), so watching/unwatching never showed up in a ticket's timeline. Added AuditLogModel::log() calls to both the watch and unwatch paths, gated on the DB statement's affected_rows so a no-op (already watching, already not watching) doesn't produce a duplicate timeline entry. Added 'watch'/'unwatch' to AuditLogModel's VALID_ACTION_TYPES, and timeline rendering in views/TicketView.php ("started watching this ticket" / "stopped watching this ticket"). Verified against real MariaDB: watch -> unwatch -> watch again produces exactly 2 timeline entries (not 4) since the two no-op repeats correctly produced zero rows changed and were not logged; confirmed formatAction()/ getEventIcon() render both action types correctly. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0117oBw2jN4kALYeS8HPq4zV	2026-09-12 01:17:13 -04:00
jaredandClaude Sonnet 5	863f84f37e	Switch LDAP avatar lookups to LDAPS (#95) ... api/user_avatar.php connected via ldap://$ldapHost:$ldapPort — never ldaps://, and there was no ldap_start_tls() call anywhere in the codebase. LDAP_BIND_PW was sent over the wire unencrypted on every avatar fetch. Switched to ldaps://, and changed LDAP_HOST/LDAP_PORT's defaults to ldap.lotusguild.org:6360 (lldap's LDAPS listener) instead of the bare IP on port 3890 (plaintext). PHP's ldap extension verifies the server cert's hostname by default, so a bare IP won't validate against the LDAPS cert (issued for *.lotusguild.org) — LDAP_HOST has to be a hostname the cert covers. This is deliberately not configurable back to plaintext ldap://. Infra change (pve-infra, separate repo/commit): added a Pi-hole split-horizon override so ldap.lotusguild.org resolves internally to the real LDAP server's LAN IP — its existing public DNS record points elsewhere (an unrelated host), and there was no internal-only DNS entry for it before this. Verified against the real lldap server (pct 147, LDAPS on 6360, a live Let's Encrypt *.lotusguild.org cert): confirmed the Pi-hole override resolves correctly from hosts using it as their resolver, then ran the exact ldap_connect/ldap_bind sequence via `php -r` directly on the production tinker_tickets host (10.10.10.45) with a deliberately wrong bind password — got "Invalid credentials" (a real LDAP protocol response), not a transport/TLS error, proving the full connect + TLS handshake + hostname verification + bind path works end-to-end in the actual deployment environment. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0117oBw2jN4kALYeS8HPq4zV	2026-09-12 00:56:00 -04:00
jaredandClaude Sonnet 5	dcf9b0cfa1	Generate real resized thumbnails for image attachments (#98) ... The attachment grid's <img> thumbnail pointed at the same download_attachment.php URL as the full-size original, so previewing a multi-MB photo attachment cost a full multi-MB download just to render a small grid preview. loading="lazy" only deferred off-screen images; it never reduced per-image transfer size. Generate a resized JPEG thumbnail (longest side capped at 300px) via GD at upload time, from the same metadata-stripped image stripImageMetadata() already produces, reusing its decompression-bomb guard (~40MP decode cap). Store the thumbnail's filename in a new nullable ticket_attachments. thumbnail_filename column (migration 005); NULL means no thumbnail exists (non-image, GD unavailable, or an attachment predating this change) and callers fall back to the full-size original. download_attachment.php serves the thumbnail when requested via ?thumb=1 and one exists, falling back to the original otherwise. The attachments grid now requests thumb=1 for its <img> preview; the lightbox link is unchanged and still opens the full-size original. delete_attachment.php removes the thumbnail file alongside the original, and cleanup_orphan_uploads.php's orphan lookup now also matches thumbnail_filename so generated thumbnails aren't swept up as orphans. Verified against real MariaDB + GD: a 1600x1200 test JPEG produced a 300x225 thumbnail at ~1.8KB vs. the 52KB original (~29x smaller); confirmed the serving logic picks the thumbnail for image attachments with one, falls back to the original for a non-image attachment even when thumb=1 is requested, and that the updated orphan-cleanup lookup matches both the original and thumbnail filename (and correctly finds neither for an unrelated filename). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0117oBw2jN4kALYeS8HPq4zV	2026-09-11 22:17:35 -04:00
jaredandClaude Sonnet 5	844677bbce	Fix atomicity docblock and surface per-ticket bulk-op errors (#33) ... processBulkOperation()'s docblock claimed the transaction "ensures atomicity - either all tickets are updated or none are," but that's only true when $atomic = true is passed, and the only real caller (api/bulk_operation.php) never passes it — the actual default is best-effort: per-ticket failures are skipped and recorded, and every other ticket in the batch still commits. Reworded the docblock to describe the actual default behavior and when $atomic changes it. The model already collected per-ticket failure reasons into $result['errors'] (dashboard.js's bulkResultMessage() already reads data.errors to render them), but api/bulk_operation.php's success response dropped that field entirely, so admins only ever saw a bare "N succeeded, M failed" count with no way to see which tickets failed or why. Added 'errors' to the response when present. Verified against real MariaDB: a bulk_status operation against a Closed ticket (no transition defined) and an Open ticket (Open->Pending defined) correctly processed 1/1, and the API response now includes errors: ["Ticket ...: transition not allowed (Closed -> Pending)"]. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0117oBw2jN4kALYeS8HPq4zV	2026-09-11 21:56:26 -04:00
jaredandClaude Sonnet 5	310dcd0840	Persist status-change comments transactionally with the update (#37) ... A comment accompanying a status change (required or user-supplied) was posted via a separate, independent HTTP call/write (add_comment.php, or a second add_comment call in lt.ticketStatus.submit()'s requires_comment retry path) before the status update itself. A failure partway through — or the client never issuing the second call — could leave a "reason" comment persisted with no matching status change, or vice versa, with no rollback tying the two together. api/update_ticket.php and api/ticket_status_api.php now post the comment and apply the status update inside one transaction, rolling back both on any failure. assets/js/ticket.js and lt.ticketStatus.submit() in assets/js/base.js no longer make a separate add_comment.php call; they pass the comment directly to update_ticket.php, which persists it server-side alongside the status change. Verified against real MariaDB by extracting the live ApiTicketController and the ticket_status_api.php transaction logic and running them directly: a forced optimistic-lock conflict correctly rolled back both the comment and the status change, and a successful call persisted exactly one comment alongside the status change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0117oBw2jN4kALYeS8HPq4zV	2026-09-11 21:41:48 -04:00
jaredandClaude Sonnet 5	86ef91abcb	Reject duplicate workflow transitions with a clear error (#62) ... status_transitions already has a DB-level UNIQUE KEY on (from_status, to_status), so a genuine duplicate pair was never actually possible to insert — but hitting that constraint raw surfaced as an opaque "An internal error occurred" to the admin instead of a clear message, since manage_workflows.php only validated from_status !== to_status before attempting the insert/update. Added an explicit existence check before insert/update in both the POST and PUT handlers (excluding the row's own ID on update), so the common case — an admin re-adding or renaming into a pair that already exists — gets a specific 409 with the conflicting pair named, instead of a generic 500. Also added ORDER BY transition_id to WorkflowModel::getAllTransitions() as a defense-in-depth backstop: since it collapses rows into a PHP array keyed by [from_status][to_status] with no defined winner otherwise, if the DB constraint were ever weakened or bypassed, this at least makes which row wins deterministic (most recently created). Verified against a real running server + real MariaDB: creating a duplicate active pair, a duplicate inactive pair, and updating a different row into an existing pair are all correctly rejected with the friendly message; updating a row to keep its own existing pair succeeds; and a genuinely different pair still creates normally. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv	2026-09-11 14:28:21 -04:00
jaredandClaude Sonnet 5	09cea2b388	Include ticket_id in comment-edit audit log so it appears on the timeline (#87) ... AuditLogModel::getTicketTimeline() requires, for entity_type='comment' rows, that details.ticket_id match the ticket being viewed. logCommentCreate() and delete-comment's audit call both correctly include it; update_comment.php's audit call only set comment_text_preview, so an edited comment's audit row was written (visible in the admin's global Audit Log) but never matched the timeline's join condition — a comment edit left no trace on the ticket's own history, while deleting the same comment would be visible. Added ticket_id to the details array, using $comment['ticket_id'] already loaded earlier in the file for the access check. Verified against real MariaDB: the fixed shape now correctly appears in getTicketTimeline()'s results. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv	2026-09-11 14:05:01 -04:00
jaredandClaude Sonnet 5	3db3749c46	Re-check ticket visibility before surfacing in-app notifications (#48) ... All four notification queries in api/notifications.php (assign, comment, status-change, mention) were scoped purely by created_by/assigned_to/ticket_watchers membership and historical audit_log contents — never by canUserAccessTicket(). If a ticket's visibility was later tightened, or a user's group/watcher access revoked, a notification still surfaced in their bell dropdown, disclosing the ticket's title and that activity occurred even though opening the ticket itself would now be blocked. Batch-fetches the tickets referenced by all candidate notifications (via the existing getTicketsByIds()) and filters out any whose current state canUserAccessTicket() would reject for the requesting user, before formatting the response — so a notification for a ticket the user can no longer see simply disappears rather than lingering as a disclosure. Verified against real MariaDB with a running server: an assignment notification is visible while the user is the assignee of a public ticket, and disappears once the ticket is reassigned away and made confidential. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv	2026-09-11 13:31:08 -04:00
jaredandClaude Sonnet 5	3266373cdf	Wire Custom Fields into ticket creation and viewing (#47) ... CustomFieldModel::setValue()/setValues()/getValuesForTicket() were never called anywhere outside api/custom_fields.php's own admin CRUD for field *definitions* — CreateTicketView.php never rendered custom fields, TicketController::create() never collected or saved them, and TicketView.php never displayed them. The whole feature (admin defines fields at /admin/custom-fields, including marking them Required) was config-only with zero consumer; is_required was enforced nowhere. Added: - api/ticket_custom_fields.php: new endpoint (bootstrap.php-based, so any ticket editor can use it, not just admins) that saves values for a ticket. Only considers fields applicable to the ticket's current category (or category-less fields); enforces is_required, validates select values against the field's configured options, and validates number fields are numeric. Values for fields that don't apply are silently ignored rather than persisted, so a value typed before a category change can't linger as orphaned data. - CreateTicketView.php: renders every active field definition (all categories, since the ticket doesn't exist yet), grouped with a data-custom-field-category attribute and toggled client-side as the Category select changes, matching the existing visibility-groups toggle pattern. Submitted as part of the same form. - TicketController::create(): validates is_required server-side against the fields applicable to the *submitted* category (not just whatever was visible client-side) before creating the ticket, then persists via CustomFieldModel::setValues() after a successful create. - TicketView.php: new "Custom Fields" tab (only shown when the ticket's category has applicable fields) rendering current values with a single Save button, calling the new endpoint — a panel-plus-save interaction rather than per-field inline auto-save, to keep scope contained across 6 field types. Verified against real MariaDB and a real running server: a required field left blank is correctly rejected (both at ticket-creation time and when editing an existing ticket) with no partial write; a valid submission persists exactly the fields applicable to that ticket's category; an invalid select value is rejected without touching previously-saved values; and each rejection correctly returns a rotated recovery csrf_token (initially missed on the validation-error paths, since they used a plain echo/exit instead of the bootstrap.php apiRespond() helper every other endpoint's error paths use for this). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv	2026-09-11 13:12:42 -04:00
jaredandClaude Sonnet 5	6adbb29964	Fire notifications and fix audit_log shape for bulk status changes (#67, #74) ... BulkOperationsModel's bulk_close/bulk_status paths had zero references to NotificationHelper — the exact same status transition (e.g. Open->Closed) silently produced no Matrix/watcher notification when performed via bulk actions, while the single-ticket edit page and Bearer API both notify on every status change. Separately, their audit_log entries used a bare ['status' => 'Closed', ...] shape instead of the {'status': {'from': X, 'to': Y}} shape every other status-change path uses, which broke two downstream consumers: TicketView.php's timeline fell back to a generic "updated this ticket" instead of "updated status", and notifications.php's $details['status']['from'] on a string produced a broken "? -> ?" notification title. Fixed the audit_log shape for both operation types, and added a notification queue collected during the per-ticket loop and flushed only after a successful commit (so atomic-mode rollback correctly sends zero notifications, matching how nothing else about a rolled- back batch takes effect either). Also fixed an incidental bug found while matching this to the single-ticket path: update_ticket.php's notifyWatchers() call never passed the ticket's visibility, silently defaulting to 'public' and always including the shared notify list even for confidential/internal tickets — the exact leak #71 fixed elsewhere in NotificationHelper itself, just never reaching this call site. Verified against real MariaDB with a real local webhook-capturing server: bulk_close correctly fires sendStatusChangeNotification() + notifyWatchers() with the right old/new status and a redacted title for a confidential ticket; audit_log rows show the correct {from,to} shape; and an atomic-mode rollback (one ticket's transition invalid) sends zero notifications and leaves both tickets unchanged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv	2026-09-11 12:37:37 -04:00
jaredandClaude Sonnet 5	71bf64c1e2	Complete ErrorHandler rollout: wire into all endpoints, fix display_errors gaps, add styled 500 page (#38, #39, #105) ... README documented ErrorHandler.php as a "global error/exception handler", but ErrorHandler::init() had exactly one caller app-wide (api/get_template.php). 13 endpoints never called ini_set('display_errors', 0) at all, relying on the server's global php.ini default, and index.php never registered any handler — a genuine PHP fatal during a page render fell through to PHP's raw default handling with no app-level 500 response, styled or otherwise. Investigating the "13 endpoints" claim turned up that 9 of them (assign_ticket.php, audit_log.php, check_duplicates.php, get_comments.php, get_users.php, notifications.php, saved_filters.php, user_preferences.php, watch_ticket.php) already require api/bootstrap.php as their first statement, which itself calls ini_set('display_errors', 0) — so they were never actually exposed; the static grep just couldn't see through the require. The 3 that were genuinely unprotected (bulk_operation.php, download_attachment.php, health.php) are fixed here. ticket_dependencies.php already has its own complete hand-rolled equivalent (shutdown handler, error handler, exception handler, output-buffer aware) and was deliberately left alone rather than risk double-registering handlers. Rather than duplicate the fix 30+ times, wired ErrorHandler::init() directly into api/bootstrap.php (covering all 9 files above at once) and into each of the other endpoints' own ini_set/error_reporting pair, replacing it in place — additive only: existing try/catch blocks in every endpoint still handle what they already handled identically, this only adds a safety net for genuinely uncaught fatals that fell through everything else. Before doing this app-wide, removed ErrorHandler::init()'s override of PHP's 'error_log' ini setting: it redirected every error_log() call in the request to a fixed /tmp file, which would have silently diverted logs away from wherever the server is actually configured to send them the moment this got wired into more than one endpoint. That override only existed to support getRecentErrors(), which has zero callers app-wide. For index.php (page views, not JSON), added an 'html' response mode to ErrorHandler that renders a new views/error_500.php instead of a JSON body. That view is deliberately self-contained (no layout_header.php, no $GLOBALS/session/DB dependency) since a genuine fatal can happen before config.php finishes loading or mid-session-start. Verified: a real uncaught error with no prior output correctly produces a clean JSON 500 (API mode) or the styled HTML page (page mode) to the client while the full stack trace goes to error_log, not the response; normal (non-fatal) requests through both a bootstrap.php-based endpoint and index.php are byte-for-byte unaffected. Full project phpcs pass is clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv	2026-09-11 12:17:15 -04:00
jaredandClaude Sonnet 5	9d8a73c355	Add recovery csrf_token to 12 hand-rolled CSRF rejection responses (#85) ... api/bootstrap.php's centralized CSRF handling echoes CsrfMiddleware::getToken() on a 403 rejection specifically so lt.api's client-side resync (assets/js/base.js) can recover once window.CSRF_TOKEN goes stale (token expiry, or a write in another tab rotating the shared session-scoped token). 12 endpoints duplicate CsrfMiddleware::validateToken() inline instead of routing through bootstrap.php, and their 403 body omitted csrf_token entirely — custom_fields.php, clone_ticket.php, delete_comment.php, delete_attachment.php, bulk_operation.php, generate_api_key.php, manage_templates.php, manage_recurring.php, revoke_api_key.php, manage_workflows.php, ticket_dependencies.php, and upload_attachment.php. Once a client's token drifted out of sync, the next write to any of these 12 endpoints returned a 403 with no way to self-heal — every subsequent write to any endpoint kept failing until a manual reload, since the resync mechanism was only wired up on a minority of the app's write surface. Took the minimal fix the issue names as sufficient (add 'csrf_token' => CsrfMiddleware::getToken() to each rejection body) rather than restructuring all 12 through bootstrap.php, to avoid behavioral risk from rewiring each endpoint's differing auth/bootstrapping. generate_api_key.php and revoke_api_key.php threw a generic Exception for this case (swallowed into a plain error-message response with no room for extra fields), so those two now short-circuit with a direct JSON response instead, matching the other 10. Verified end-to-end against real running endpoints with a real session and real MariaDB: sent a wrong CSRF token to one endpoint of each response shape (plain json_encode, ResponseHelper::error, and the formerly exception-based path) and confirmed all three now return the current valid csrf_token in the 403 body. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv	2026-09-11 11:42:46 -04:00
jaredandClaude Sonnet 5	c78d24154a	Make TRUSTED_PROXIES' insecure-by-default risk loudly visible (#94) ... TRUSTED_PROXIES ships empty in .env.example, which disables AuthMiddleware's reverse-proxy allowlist entirely — a fresh deployment that doesn't explicitly set it has zero verification that Remote-User/Remote-Groups headers actually came from the trusted Authelia proxy. Anything that can reach the app directly (a misconfigured firewall rule, an exposed container port, SSRF from another internal service) can set Remote-User: admin and fully impersonate any user with zero authentication. The enforcement logic itself was already correct; this was purely a dangerous, easy-to-miss default. Added a boxed, unmissable warning around TRUSTED_PROXIES in .env.example (previously just an inline comment easy to skim past), added the same warning to README's setup instructions (which didn't mention this variable at all), and added a Check 8 to api/health.php that reports a 'warning' status when TRUSTED_PROXIES is empty, so a deployment that forgets it doesn't go unnoticed after the fact. Verified against real MariaDB via a running server: the health endpoint correctly reports 'warning' when empty and 'ok' once set. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv	2026-09-11 11:42:29 -04:00
jaredandClaude Sonnet 5	5709c3134f	Verify mentioned-user access before sending @mention notifications (#69) ... sendMentionNotification(), called from add_comment.php, had no visibility check at all — unlike sendCommentNotification()/ notifyWatchers() which redact the comment preview for non-public tickets. Mentioning a user with zero standing access to a confidential ticket (not creator/assignee/admin, not in visibility_groups) sent them a Matrix DM with the full ticket title AND comment text — worse than #46 since it's delivered directly to an individual rather than diluted into a shared list. add_comment.php now filters mentioned users through canUserAccessTicket() before resolving Matrix IDs, skipping the notification entirely for anyone without access (one of the two options the issue names as acceptable). getMentionedUsers() needed to start selecting is_admin and groups alongside user_id/username/ display_name, since canUserAccessTicket() requires them. Verified against real MariaDB: a user mentioned on a confidential ticket they don't own/aren't assigned to is correctly denied, a user in the matching visibility_groups for an internal ticket is correctly allowed, and the same user is correctly denied on a different internal ticket whose group they're not in. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015nCxwFFsy8ouMWzn56rPVP	2026-09-08 21:49:42 -04:00
jaredandClaude Sonnet 5	60bafae8a0	Redact ticket title for non-public tickets in assignment notifications (#72) ... sendAssignmentNotification() had the same missing-visibility gap as #46 in a separate function: assigning a user to a confidential/internal ticket broadcast the ticket title to the shared Matrix notify list unconditionally when MATRIX_NOTIFY_ASSIGNMENTS is enabled. Threaded visibility through using the same redactedTitle() helper added for #46, wired up from the already-fetched ticket row in assign_ticket.php. The assignee is still DMed directly regardless, since being assigned gives them standing access to the ticket — but because notify_users is one shared payload, they see the same redacted title as everyone else on it rather than a personalized one. Verified end-to-end with a local HTTP server capturing the webhook payload for both public and confidential tickets. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015nCxwFFsy8ouMWzn56rPVP	2026-09-08 21:49:34 -04:00
jaredandClaude Sonnet 5	442cd1d6f6	Redact ticket title for non-public tickets in create/status-change Matrix notifications (#46) ... sendTicketNotification() and sendStatusChangeNotification() always sent the ticket title to the shared MATRIX_NOTIFY_USERS list regardless of visibility, unlike sendCommentNotification()/notifyWatchers() which already redact the comment/activity preview for non-public tickets. Creating or changing the status of a confidential ticket broadcast its title to a shared Matrix room, defeating the point of the Confidential visibility level. Added a shared redactedTitle() helper and threaded visibility through both functions (sendTicketNotification reads it from the existing $ticketData['visibility'] key; sendStatusChangeNotification takes a new optional parameter, wired up in both callers from the already-fetched ticket row). Verified end-to-end with a local HTTP server capturing the actual webhook payloads: public tickets pass the title through unchanged, confidential/internal tickets get the redacted placeholder. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015nCxwFFsy8ouMWzn56rPVP	2026-09-08 21:49:06 -04:00
jaredandClaude Sonnet 5	d7940b1e31	Fix ticket_watchers.ticket_id type mismatch and missing FK (#32) ... ticket_watchers.ticket_id was int(11) while every other satellite table (ticket_comments, ticket_attachments, ticket_dependencies, custom_field_values) uses varchar(9)/varchar(10) matching tickets.ticket_id, and it had no FK constraint at all — unlike every other satellite table — so orphaned watcher rows could never be caught by referential integrity. Changed the column to varchar(9) with an ON DELETE CASCADE FK to tickets, in both 000_baseline.sql and a new idempotent 004_fix_ticket_watchers_type.sql (which also deletes any pre-existing orphaned watcher rows before adding the constraint, since orphans would otherwise make the ADD CONSTRAINT fail). Updated watch_ticket.php, NotificationHelper::notifyWatchers(), and notifications.php's audit-log JOIN to bind/compare ticket_id as a string instead of casting to int, including replacing a fragile CAST(entity_id AS UNSIGNED) with a direct string comparison. Verified against real MariaDB: applied 004 against a simulated pre-fix deployment with one valid and one orphaned watcher row — the orphan is removed, the column converts losslessly, the FK is added, and the migration is idempotent on re-run. Confirmed ON DELETE CASCADE actually removes watchers when their ticket is deleted, that inserting a watcher for a nonexistent ticket now fails with a real FK violation, and exercised the updated watch/unwatch and status-change-notification query paths end-to-end against the fixed schema. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015nCxwFFsy8ouMWzn56rPVP	2026-09-08 21:28:42 -04:00
jaredandClaude Sonnet 5	3d5adbbfda	Paginate attachment listing (#100) ... AttachmentModel::getAttachments() had no LIMIT/OFFSET, so a ticket with hundreds of attachments loaded and rendered every one of them in a single API response and DOM grid, unbounded. Added optional limit/offset to getAttachments(), matching the pattern already used by CommentModel::getCommentsByTicketId(). The GET handler in upload_attachment.php now accepts limit/offset (default 40, capped at 100) and returns total/has_more alongside the page of attachments. ticket.js's loadAttachments()/renderAttachments() now fetch and append pages, showing a "Load more attachments (N remaining)" control when more are available. Verified against real MariaDB with 12 attachments across 3 pages of 5: no duplicates or gaps across pages, and the legacy unlimited call (getAttachments($ticketId) with no limit/offset) still returns everything unchanged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015nCxwFFsy8ouMWzn56rPVP	2026-09-08 20:57:22 -04:00
jaredandClaude Sonnet 5	b0765eb7f4	Add HTTP Range/partial-content support to attachment downloads (#99) ... download_attachment.php always streamed the entire file regardless of any Range request header, and never advertised Accept-Ranges. Large video/PDF attachments couldn't be scrubbed in-browser, and an interrupted download had to restart from byte 0. Now parses a single-range "bytes=start-end" (including open-ended and suffix forms) request header and responds with 206 Partial Content and a Content-Range header, seeking the file handle to the requested offset; out-of-range requests get 416 with Content-Range: bytes */<size>. Verified against a real file served over a local PHP dev server with curl for exact-range, open-ended, suffix, no-Range, and out-of-bounds cases, confirming byte-identical output for each. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015nCxwFFsy8ouMWzn56rPVP	2026-09-08 20:57:16 -04:00
jaredandClaude Sonnet 5	338bed7eb7	Add per-ticket attachment count/storage quota (#55) ... api/upload_attachment.php enforced a per-file size cap but nothing bounded the total number of attachments on a single ticket or their cumulative size over time — an authenticated low-privilege user could slowly fill the uploads/ disk by attaching many files across tickets, bounded only by the general rate limiter (which throttles request rate, not storage volume). Added MAX_ATTACHMENTS_PER_TICKET (50) and MAX_TOTAL_ATTACHMENT_SIZE_PER_TICKET (100MB) config defaults, enforced before move_uploaded_file() using AttachmentModel::getAttachmentCount() and getTotalSizeForTicket() — both already existed in the model with zero callers, apparently added for exactly this purpose but never wired in. Verified against a local MariaDB instance: with 3 existing 1MB attachments and a 3-attachment cap, the count check correctly rejects a 4th; with a 5MB total cap, a 2.5MB upload that would push the ticket over the limit is correctly rejected while a small one that fits is not. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X	2026-09-08 14:14:00 -04:00
jaredandClaude Sonnet 5	3f1e06479d	Strip EXIF/GPS metadata from image uploads (#97) ... api/upload_attachment.php did a raw move_uploaded_file() with zero image processing. A photo attached from a phone retained embedded EXIF, including GPS coordinates, and download_attachment.php streams the file byte-for-byte back to any user with ticket visibility — for an infrastructure company, this could leak a data center or office's precise physical location through a routine ticket photo, especially on Confidential-visibility tickets whose whole point is restricting exactly this kind of detail. Added stripImageMetadata(): decodes and re-encodes JPEG/PNG/GIF/WebP uploads via GD, which drops EXIF chunks that aren't part of the pixel data. Best-effort — leaves the file untouched on any failure (corrupt image, unsupported format, GD unavailable, or an oversized decoded pixel count guarding against a decompression-bomb-style crafted image) rather than blocking the upload. Verified with a real GPS-tagged JPEG (generated via piexif) and a GD/PHP harness: GPS EXIF is gone after stripping, the image stays valid and correctly sized, PNG alpha transparency is preserved, and corrupt files / non-image MIME types are left byte-for-byte unchanged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X	2026-09-08 12:22:02 -04:00
jaredandClaude Sonnet 5	1e972fe7dc	Add memory_limit/max_execution_time sanity checks (#106) ... config/requirements.php only checked PHP version and 6 extensions. A deployment on a host with a low default memory_limit (e.g. shared- hosting-style 128M) passed the startup requirements check cleanly and only surfaced as a mysterious failure under real load — a large CSV export, an oversized dashboard query on a big install. Added min_memory_limit_mb (256) and min_max_execution_time (30s) thresholds to config/requirements.php, checked as warnings (not hard failures, since a low limit doesn't break every request) in both scripts/check_requirements.php (CI) and api/health.php (production monitoring). -1/0 (unlimited) always passes. Verified the ini-size parsing and warning logic directly with low/high/unlimited memory_limit and max_execution_time values. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X	2026-09-08 11:58:20 -04:00
jaredandClaude Sonnet 5	0d6b08f5d2	Cap get_users.php result set as defense-in-depth (#42) ... api/get_users.php returned every user's user_id/username/display_name to any authenticated session with no pagination or limit — needed for mention/assignment typeahead, but a blanket enumeration a compromised low-privilege session could scrape in one call. Added a LIMIT 500; every caller already only uses this for typeahead/dropdown filtering, never a literal full roster, so this doesn't change behavior for any real deployment size while bounding the response. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X	2026-09-08 11:53:36 -04:00
jaredandClaude Sonnet 5	6183bcd421	Notification titles: handle non-status ticket edits (#84) ... The 'update' notification formatter unconditionally read details['status']['from']/['to'], so any title/priority/description/ category/type/visibility-only edit fell through to '?' on both sides and produced a broken "changed status on #123: ? → ?" title regardless of what actually changed. Now it branches on the delta shape actually present: the flat {field, from, to} shape used for visibility changes, then each per-field {from, to} delta in priority order, falling back to a generic "updated ticket" message only if none match. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X	2026-09-08 10:41:29 -04:00
jaredandClaude Sonnet 5	6e0863449f	Trim comment text before persisting, not just for validation ... add_comment.php computed a trimmed copy of comment_text only to check for empty input, then passed the original untrimmed $data through to CommentModel::addComment(), so any leading/trailing whitespace the user typed (or pasted) was written to ticket_comments.comment_text as-is. update_comment.php already trims before saving edits, so a comment could pass through this endpoint once with untrimmed text (creation) and be silently corrected the moment it was next edited — inconsistent storage that, combined with the markdown parser's line-anchored regexes (headings, tables, lists all match on ^), could make a markdown-enabled comment mis-render after a reload depending on whether its first line carried leading whitespace. Also trims in the "Load more comments" pagination re-render path in TicketView.php, matching the two on-load renderers in markdown.js so all three code paths that call parseMarkdown() on stored comment text treat leading whitespace consistently. Closes #18 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>	2026-08-31 21:03:00 -04:00
jared	9d982ab73f	Bulk status/close: enforce Workflow Designer rules (#21) ... Bulk status changes previously bypassed the workflow entirely — the model carried an explicit "admin-only escape hatch" note — so bulk edit could drive tickets through transitions the designer forbids and skip comments the designer requires. BulkOperationsModel now applies the same rules as the single-ticket path: - Transitions absent from status_transitions are refused per ticket and reported with a reason, instead of being forced through. - requires_comment is checked up front across the whole selection, so a batch is rejected before any ticket is mutated rather than half-applied. - The reason is persisted as a comment on each ticket changed, matching what a single-ticket close records. - Tickets already in the target status are a no-op success, not a failure. requires_admin needs no extra check: api/bulk_operation.php already gates the endpoint on admin. Client: both bulk modals now collect a reason, the close path gets a real modal instead of a bare confirm, and per-ticket skip reasons surface in the result toast instead of a bare failure count.	2026-08-07 22:35:36 -04:00
jaredandClaude Opus 4.8	d46f8ffd77	Add Bearer API: list/read tickets, post comments, change status ... Extends the Bearer-key API beyond create-only (all rate-limited, scope- enforced, per-key-label attribution): - GET /api/tickets_api.php: triage the queue (status/priority/host title match + pagination) or read one ticket + its comments. read scope. - POST /api/ticket_comment_api.php: post a comment as the key (user_name = key name, linked to the key owner). read_write scope. - POST /api/ticket_status_api.php: change/close status with workflow validation + requires_comment; posts the close reason in the same call, fires the Matrix status notification, invalidates stats. read_write scope. Reuses TicketModel/CommentModel/WorkflowModel/NotificationHelper; a read key cannot mutate. Reachability requires the reverse-proxy Authelia bypass (handled separately). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>	2026-07-15 18:40:52 -04:00
jaredandClaude Opus 4.8	5cf5aa9591	API keys: add read/read_write scopes + admin scope selector & pagination ... Foundation for extending the Bearer API beyond create-only: - api_keys gains a scope column (read | read_write); baseline schema updated and the column applied to the live DB. Existing keys default to read_write so the hwmon create key keeps working. - ApiKeyModel: createKey() takes a validated scope; validateKey() always surfaces scope (defaults read_write); getAllKeys() is paginated ({keys,total,page,perPage}, key_hash stripped). - ApiKeyAuth: expose getKeyContext() (scope/key_name/created_by/api_key_id) and requireScope() (403 on insufficient scope); existing return values unchanged. - create_ticket_api.php: require read_write scope (a read key can't create). - Admin /admin/api-keys: scope selector on the create form, a scope column, and pagination (revoked keys were stacking up). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>	2026-07-15 18:24:50 -04:00
jaredandClaude Opus 4.8	d535557e5a	Strip trailing whitespace failing phpcs (unblocks CI/deploy) ... CI has been red since the CSRF-drift changes landed a trailing space on the 'success' => false line in these two endpoints, which blocks the deploy job (and therefore beta/prod). No logic change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>	2026-07-15 14:46:04 -04:00
jared	7a537f46bc	Fix CSRF token drift in add_comment and update_ticket endpoints	2026-07-14 23:19:26 -04:00
jaredandClaude Opus 4.8	622cae8bbd	Fix PHP 8.4 breakage: drop deprecated mysqli::ping(), harden dep handler ... The hosts were upgraded to PHP 8.4, where mysqli::ping() is deprecated (auto-reconnect was removed in 8.2). Database::getConnection() called it on every reused connection, and api/ticket_dependencies.php's custom error handler treated the deprecation as a fatal 500 ('A server error occurred'), breaking the ticket Dependencies tab. - Database.php: remove the redundant ping()/reconnect check (connection is request-scoped; no liveness check needed on PHP 8.2+). - ticket_dependencies.php: only abort on genuine errors; log notices/ warnings/deprecations and continue, so a future deprecation can't 500 it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>	2026-07-10 19:12:10 -04:00
jaredandClaude Opus 4.8	d11cb989bf	Fix API correctness: external API stub/collision, recurring dates, CSV, audit ... - create_ticket_api.php: remove the wrong CREATE TABLE stub that broke a fresh DB; generate collision-safe ticket_ids so a genuine id collision isn't misreported as a duplicate and a hw alert dropped; stop leaking raw DB errors; correct a reopen comment that falsely claimed refreshed sensor data - manage_recurring.php: fix next-run so create/edit no longer skips the current period (monthly day-of-month this month, daily today if time not passed, correct ISO weekday, month-length clamp); only recompute on schedule changes to avoid double-fire - export_tickets.php, audit_log.php: neutralize CSV formula injection - revoke_api_key.php, generate_api_key.php: correct HTTP status codes and stop the catch clobbering specific 4xx codes - health.php: stop leaking PHP version / extension names / paths to unauthenticated callers - watch_ticket.php: define $data before use - manage_templates/recurring/custom_fields: add audit logging for CRUD; add recurring_ticket + custom_field to the audit entity whitelist Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>	2026-07-10 12:26:39 -04:00
jaredandClaude Opus 4.8	327c225ded	Fix API security: dependency/visibility leaks, authz, CSRF, comment spoofing ... - ticket_dependencies.php: pass current user id/groups/is_admin into the visibility-filtered DependencyModel methods; drop (int) casts that stripped leading zeros from varchar ticket_ids - update_ticket.php: authorize visibility changes (admin or creator only); enforce requires_comment transitions server-side (400 + requires_comment flag so the client can prompt-and-retry); return proper 401/400/403 - add_comment.php: take commenter name from the session not the client (anti-spoofing); validate parent_comment_id belongs to the ticket; reject empty comments; pass ticket visibility to notifications so non-public comment bodies aren't leaked - add_comment/update_comment/bulk_operation: validate CSRF for all state-changing methods, not just POST - bootstrap.php: return the current CSRF token on rejection and never rotate it on a rejected request, so a desynced client can auto-recover - correct auth->401 and validation->400 status codes across these endpoints Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>	2026-07-10 11:48:34 -04:00
jaredandClaude Opus 4.8	99c840fce0	Fix logic bugs found in third multi-agent review ... Medium: - create_ticket_api.php: environment tags were parsed with explode('][') which left brackets on the first/last tag so the whitelist never matched, dropping the env tag from the dedup hash — a [production] and [staging] issue with otherwise-identical components could collide onto one ticket. Use a bracket-aware regex. - CommentModel::getThreadedCommentsPaged only fetched DIRECT children of root comments, so when pagination is active, nested replies at depth 2-3 vanished from the thread. Expand replies level-by-level (bounded to depth 3). - StatsModel::getTicketsByAssignee ignored the visibility filter the rest of the stats apply, so a non-admin's "by assignee" widget counted (leaked) confidential tickets. Thread the same filter through. - watch_ticket.php GET path returned watch state / watcher names / count for any ticket with no access check (the POST path checks it) — added canUserAccessTicket. - dashboard.js kanban: every card rendered as P4 because the [class*="lt-p"] selector never matched the lt-badge-p1 class and the fallback didn't strip "P". Extract the digit directly. Low: - audit_log.php CSV: "Log ID" column was always blank ($log['log_id'] vs the real audit_id column). Use audit_id. - check_duplicates.php: the graceful-degradation try/catch only covered the throw path; guard the false-return (non-exception mysqli) path too. - notifications.php: owner-who-is-also-@mentioned got two notifications for one comment; drop the duplicate comment row when a mention covers the same comment. - dashboard.js hover preview rendered "PP1" (doubled prefix); strip the leading P. - markdown.js: code/inline-code restore used string replace, so $&, $$, $`, $' in user code were treated as replacement patterns; use a function replacer. Also removed an unused loop var. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>	2026-06-30 14:21:35 -04:00
jaredandClaude Opus 4.8	e0e92e326a	Quick-win fixes from second review ... - create_ticket_api.php: validate status (against TICKET_STATUSES) and priority (numeric 1-5). A non-numeric priority previously cast to 0 and escalated the ticket below P1 on the dedup/update path. - manage_workflows.php: reject empty/invalid from_status/to_status on POST and PUT (must be valid ticket statuses) so the workflow table can't be populated with bogus transitions. - TicketModel::getAllTickets: COUNT(*) OVER() rides on returned rows, so a page past the last row returned total/pages = 0. Fall back to a direct COUNT when an over-range page yields no rows, keeping pager math correct. - DashboardView: stop double-escaping category/type/assigned active-filter labels (they were htmlspecialchars'd into the label and again at output, rendering R&D as R&amp;D); output escaping is retained. - check_duplicates.php / NotificationHelper::notifyWatchers: wrap the DB lookups in try/catch so a failed prepare/query degrades gracefully (advisory dup-check returns none; best-effort watcher notify is skipped) instead of fataling the request. Works whether mysqli throws or returns false. (manage_* endpoints already have a top-level try/catch.) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>	2026-06-30 12:26:28 -04:00
jaredandClaude Opus 4.8	4164f85051	Fix bugs found in second multi-agent review ... XSS / security: - markdown.js: sanitize footnote labels to a safe slug before using them in id/href attributes. Labels are captured before the HTML-escape pass, so a label like x"><img onerror=...> broke out → stored XSS (the earlier quote- escape fix didn't cover this path). Verified neutralized. - RateLimitMiddleware: only trust X-Forwarded-For / X-Real-IP when REMOTE_ADDR is a configured trusted proxy, and use the rightmost (proxy-appended) entry. Previously any client could rotate XFF to escape the per-IP rate limit. - .env.example: document TRUSTED_PROXIES so fresh deploys aren't fail-open on the Authelia forward-auth spoofing protection. Correctness: - notifications.php: my previous assigned-to LIKE fix anchored only on '}', so BULK assignments (logged {"assigned_to":N,"bulk_operation_id":..}) produced no "assigned to you" notification — now matches both '}' and ',' delimiters. - notifications.php: implement the documented @mention notifications (query action_type='mention' rows for the current user); they were never delivered. - NotificationHelper::notifyWatchers: guard unchecked prepare() so a missing ticket_watchers table can't fatal the request after its DB write committed. - AuditLogModel::getTicketTimeline: JSON_UNQUOTE the extracted ticket_id so comment events actually match (string vs JSON-number comparison never did). - AuditLogModel/audit_log.php: CSV export no longer silently truncates to the 1000-row UI cap; uses a dedicated higher export limit. - DashboardView: quick-preview drawer read .ticket-link from the title cell (which has none), so the title was always blank — use the cell text. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>	2026-06-30 12:00:36 -04:00
jaredandClaude Opus 4.8	b2c19745eb	Add trusted-proxy auth hardening + PHP requirements checks ... Trusted-proxy hardening (defense-in-depth for Authelia forward-auth): - AuthMiddleware now only honors Remote-* identity headers when REMOTE_ADDR is in a configured TRUSTED_PROXIES allowlist; otherwise it refuses with 403 and logs an 'untrusted_proxy' security event. Previously anything that could reach PHP directly could spoof Remote-User/Remote-Groups and log in as admin. - New config TRUSTED_PROXIES (comma-separated, from .env). Empty = enforcement off, so this is backward compatible until the allowlist is set on a host. Requirements checks (so a PHP upgrade dropping an extension can't silently break features like avatars again): - config/requirements.php: single source of truth for min PHP version and required extensions (ldap, mysqli, curl, mbstring, fileinfo, json). - scripts/check_requirements.php: CI script that fails the build if the environment doesn't satisfy them. - New 'requirements' CI job installs those extensions and runs the check; deploy now depends on it. - api/health.php: adds php_extensions + php_version checks so production monitoring surfaces the drift (returns 503 if a required extension is gone). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>	2026-06-30 10:22:53 -04:00
jaredandClaude Opus 4.8	2b8d593ab0	Fix issues found in multi-agent code review ... Verified, high-confidence fixes from a project-wide review: - markdown.js: escape " and ' in the HTML-escape step. User-controlled image/link URLs and alt text were interpolated into "..." attributes without quote escaping, allowing attribute breakout and injected event handlers (stored XSS, only mitigated by CSP). Flagged independently by two reviewers. - cron/create_recurring_tickets.php & cron/cleanup_ratelimit.php: a mangled crontab example inside the docblock contained */ which closed the comment early, causing a fatal parse error — both cron jobs never ran. Rewrote the docblocks without a literal */. - update_ticket.php: validate visibility BEFORE the core DB write so an invalid payload can't leave the ticket updated while the request reports failure (which also skipped the audit delta and stats cache invalidation). - watch_ticket.php: GET watcher_count was capped at 6 (count of a LIMIT 6 list); use an unbounded COUNT(*) so it matches the POST path. - notifications.php: "assigned to me" LIKE pattern lacked a trailing delimiter, so user 12 also matched 120/123/etc.; anchor with }. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>	2026-06-30 09:14:38 -04:00
jaredandClaude Opus 4.8	5808b93cdb	Fix avatar negative-cache poisoning on transient LDAP errors ... user_avatar.php wrote a ".none" sentinel whenever it failed to obtain avatar bytes, conflating "LDAP errored/timed out" with "user has no avatar". A brief lldap blip (restart, slow response past the 3s timeout, network hiccup) therefore cached a 404 for the full AVATAR_CACHE_TTL (1h default), leaving avatars broken long after lldap recovered. Track whether the LDAP query actually completed (`$ldapQueryOk`) and only write the negative-cache sentinel when lldap genuinely answered with no/ invalid avatar. On error/timeout, leave no sentinel so the lookup retries once lldap is healthy again. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>	2026-06-30 09:01:17 -04:00
jaredandClaude Sonnet 4.6	3a4a13db7b	Fix semgrep security findings to pass CI security scan ... - index.php: replace SQL string interpolation with concatenation + explicit (int) casts for LIMIT/OFFSET; add nosemgrep for tainted-sql false positive (WHERE clause built from hardcoded fragments with bound params only) - api/upload_attachment.php: add realpath() path-traversal guard after mkdir - api/user_avatar.php: make (int) cast explicit at cache-path construction; add nosemgrep for tainted-filename false positive (integer-only input) - assets/js/ticket.js: add nosemgrep for insertAdjacentHTML — all dynamic content already escaped via lt.escHtml() before insertion - .gitea/workflows/security.yml: exclude echoed-request rule globally — all echo in API context is json_encode() output, not HTML; htmlentities() fix semgrep suggests would corrupt JSON responses Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>	2026-04-16 08:42:47 -04:00
jared	5fce716489	style: fix final 2 phpcs violations; exclude line-length rule	2026-04-13 21:08:19 -04:00
jared	c90bdc8ac8	style: auto-fix 1340 phpcs PSR-12 violations via phpcbf; exclude MissingNamespace and SideEffects	2026-04-13 20:56:10 -04:00
jaredandClaude Sonnet 4.6	55c2d5c596	Fix visibility bypass in export and insecure cookie in preferences ... api/export_tickets.php: getAllTickets() was called without $currentUser, so visibility filtering was skipped — any authenticated user could export all tickets including confidential/internal ones. api/user_preferences.php: the single-preference setcookie() call was missing httponly/secure flags (batch path had them correctly). Also cast preference values to string before passing to setPreference(string). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>	2026-04-11 14:29:09 -04:00
jaredandClaude Sonnet 4.6	60f23051a9	Fix notifications not detecting comment events ... AuditLogModel::logCommentCreate logs comments with action_type='comment' not 'create'. The notification query was filtering on action_type='create' only, so comment events on watched/owned tickets were never surfaced. Widen the filter to IN ('comment', 'create') to match the actual logged values while staying compatible with any legacy entries. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>	2026-04-11 13:45:04 -04:00
jaredandClaude Sonnet 4.6	47b70b0ee8	Fix ticket ID handling in assign and delete_attachment APIs ... assign_ticket.php: preserve string ticket ID (ctype_digit validation) instead of (int) cast for consistent audit logging and URL generation. delete_attachment.php: use string ticket_id from DB for the upload directory path — (int) cast was stripping leading zeros, causing the wrong path (/uploads/123456/) instead of /uploads/000123456/. Also pass raw string to getTicketById() to let TicketModel handle type coercion. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>	2026-04-11 13:31:10 -04:00