514 Commits

This Branch

This Branch

All Branches

Author	SHA1	Message	Date
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	6d68af40e7	Merge into current URL params instead of replacing them in Advanced Search (#58) ... Same root pattern as the earlier chart click-to-filter bug (#29): performAdvancedSearch() built a brand-new URLSearchParams from only the form's own fields and navigated to it, silently dropping any active filter the form doesn't represent (e.g. a category/type filter applied via a dashboard quick-filter pill or stats-widget click). populateCurrentFilters() also only read search/status back out of the URL into the form, not the date ranges/priority range/user fields the form does control. Fixed performAdvancedSearch() to start from the current URL's params and only set/clear the ones this form actually represents, leaving everything else untouched. Also fixed populateCurrentFilters() to restore all of those fields, not just search/status — without that, reopening the modal and submitting without touching anything would now silently wipe date/priority/user filters that were active but shown blank in the form (a new foot-gun the first fix alone would have introduced). Verified via jsdom: category/type/sort params not represented in the form survive a search submission; page resets to 1; and reopening the modal with an active created_from filter correctly restores it into the form and preserves it on a no-op resubmit. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv	2026-09-11 14:28:12 -04:00
jared	aa8173941a	Merge development into main: webhook timeout, comment-edit timeline fix, Bearer rate-limiting overhaul (#77, #80, #81, #82, #83, #87) ... - Add connect-timeout to Matrix webhook calls (#77) - Include ticket_id in comment-edit audit log so it appears on the timeline (#87) - Overhaul Bearer API rate limiting: real config, per-key isolation, skip session (#80, #81, #82, #83) deploy-2026.09.11-210	2026-09-11 14:11:48 -04:00
jaredandClaude Sonnet 5	1b1801696f	Overhaul Bearer API rate limiting: real config, per-key isolation, skip session (#80, #81, #82, #83) ... Four interrelated gaps in the same rate-limiting path: - #80: RATE_LIMIT_DEFAULT/RATE_LIMIT_API were defined in config.php but RateLimitMiddleware never read them (hardcoded class constants instead), and they weren't in .env.example — a deployer editing them saw zero effect with no documented way to actually change the limit. - #81: Bearer traffic was rate-limited purely by a shared IP bucket (the session-based half was a no-op for stateless clients, since a fresh session starts on every request). Two different API keys from the same host/NAT egress IP shared ONE bucket, so a chatty or misbehaving key could 429 a completely unrelated key's traffic. - #82: X-RateLimit-* headers reported the meaningless session counter for Bearer clients instead of whatever bucket actually governed them. - #83: RateLimitMiddleware::check() called session_start() unconditionally, before ApiKeyAuth even runs — continuous session-file churn and an unnecessary Set-Cookie on every stateless API request, using un-hardened cookie defaults since it runs before AuthMiddleware's hardening (which Bearer requests never reach anyway). Fixed as one pass since they're the same code path: config.php now reads RATE_LIMIT_DEFAULT/RATE_LIMIT_API from .env (added there too, documented); the middleware now extracts the raw Bearer token (independent of ApiKeyAuth, so no DB round-trip needed before rate limiting, and it works whether or not the token later turns out valid) and rate-limits it via its own per-token bucket instead of starting a session — the existing IP-based bucket still applies underneath as defense-in-depth against volumetric abuse from one network path, but each distinct key now gets real isolated headroom. getStatus()/addHeaders() report that per-token bucket for Bearer requests instead of the session counter. Verified: a Bearer request creates zero session files (confirmed via real session-directory file count before/after); two different keys from different IPs are fully isolated (one exhausting its own 120/min bucket has zero effect on the other); a config-driven RATE_LIMIT_API override (e.g. 5) is correctly honored for session-based (non-Bearer) traffic; X-RateLimit-* status correctly reflects the per-key bucket for a Bearer request. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv	2026-09-11 14:05:14 -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	9702aafacd	Add connect-timeout to Matrix webhook calls (#77) ... NotificationHelper::fire() set CURLOPT_TIMEOUT (10s total) but no CURLOPT_CONNECTTIMEOUT, so a slow-but-not-hung hookshot endpoint could add up to the full 10s per fire() call — and a single request can call fire() more than once sequentially (e.g. add_comment.php firing mention + comment + watcher notifications back to back), stacking into tens of seconds of added latency on the user-facing response. Added a 3s CURLOPT_CONNECTTIMEOUT so a slow-to-connect endpoint fails fast without needing the full request to time out. Verified the webhook still fires correctly end-to-end against a real local HTTP server after the change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv	2026-09-11 14:04:52 -04:00
jared	66bf82bf46	Merge development into main: visibility-notification pruning + CSRF UX + custom field type validation (#48, #50, #57, #73, #86) ... - Prune watchers when a ticket's visibility is tightened (#73) - Re-check ticket visibility before surfacing in-app notifications (#48) - Use lt.api instead of raw fetch() in notification bell (#57) - Auto-retry once after CSRF token resync in lt.api (#86) - Validate field_type against the allowed enum in custom field definitions (#50) deploy-2026.09.11-206	2026-09-11 13:48:20 -04:00
jaredandClaude Sonnet 5	fca0b42726	Validate field_type against the allowed enum in custom field definitions (#50) ... The setValue()/is_required/select-options half of this issue was already fixed incidentally by #47's new api/ticket_custom_fields.php endpoint. The remaining gap: createDefinition()/updateDefinition() never validated field_type against the six values the schema's enum() actually allows (text/textarea/select/checkbox/date/number), so a malformed type could be stored via the admin API and break whatever UI renders it later. Added an ALLOWED_FIELD_TYPES allowlist check at the top of both methods, returning the same ['success' => false, 'error' => ...] shape they already use for a DB failure — api/custom_fields.php already propagates that shape correctly with no changes needed there. Verified against real MariaDB: an invalid field_type is rejected on both create and update, while a valid one still succeeds. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv	2026-09-11 13:31:33 -04:00
jaredandClaude Sonnet 5	ae12fcd6fd	Auto-retry once after CSRF token resync in lt.api (#86) ... lt.api's fetch wrapper (_apiFetchAuth in base.js — the live implementation lt.api.* resolves to) already resynced window.CSRF_TOKEN from a 403 response's csrf_token field, but still threw immediately — every caller saw a raw "Invalid CSRF token" error on the FIRST attempt, with no transparent retry. Since CsrfMiddleware's token lifetime (1h) is shorter than the session idle timeout (5h), this was a routine, fully recoverable case (an hour of page inactivity, or a write in another tab rotating the shared token), not a real rejection. After resyncing the token from a 403 body that carries one, now retries the original request exactly once with the fresh token before surfacing an error — transparent to the caller on the common case, with a `retried` flag preventing more than one retry so a genuinely broken session still fails cleanly instead of looping. Verified via jsdom with a mocked fetch: a 403-then-succeeds sequence resolves successfully with exactly 2 network calls and the correct final token; a persistently-403 sequence still throws after exactly 2 calls (no infinite retry). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv	2026-09-11 13:31:25 -04:00
jaredandClaude Sonnet 5	5b96e75ff6	Use lt.api instead of raw fetch() in notification bell (#57) ... layout_footer.php's loadNotifications() and "mark all read" handler called fetch() directly instead of lt.api.*, violating the project's own documented convention (README Dev Notes #20). api/bootstrap.php rotates the CSRF token on every successful write and returns it in the response's csrf_token field; lt.api.* reads that and updates window.CSRF_TOKEN, but a raw fetch() never does — so after "mark all read", the server had rotated its token but the client's cached one was stale, causing the user's next write anywhere else in the app to fail once with "Invalid CSRF token" before self-healing. Replaced both fetch() calls with lt.api.get/post, which also drops the now-redundant manual header/credentials boilerplate. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv	2026-09-11 13:31:15 -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	9e83f8903a	Prune watchers when a ticket's visibility is tightened (#73) ... TicketModel::updateVisibility() only updated the tickets row — it never touched ticket_watchers. A user watching a public ticket that's later made confidential/internal, and who isn't creator/assignee/ admin/in the new visibility_groups, kept receiving Matrix notifications (title + redacted activity preview) about a ticket canUserAccessTicket() would now reject them from opening directly. After a successful visibility update, re-evaluates every current watcher against the new visibility rules via the same canUserAccessTicket() check the rest of the app uses, and removes any who no longer qualify. Verified against real MariaDB: tightening to confidential correctly drops watchers with no standing access while keeping an admin watcher; tightening to internal with a specific group correctly keeps a watcher in that group and drops one who isn't. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv	2026-09-11 13:30:59 -04:00
jared	cabdae35cc	Merge development into main: Custom Fields wiring (#47) ... - Wire Custom Fields into ticket creation and viewing (#47) deploy-2026.09.11-202	2026-09-11 13:17:33 -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
jared	f342e446d3	Merge development into main: bulk-op notification/audit fixes + workflow-validated auto-reopen (#67, #68, #74) ... - Fire notifications and fix audit_log shape for bulk status changes (#67, #74) - Route hwmonDaemon's auto-reopen through Workflow Designer validation (#68) deploy-2026.09.11-198	2026-09-11 12:44:06 -04:00
jaredandClaude Sonnet 5	18c213ebd7	Route hwmonDaemon's auto-reopen through Workflow Designer validation (#68) ... create_ticket_api.php's dedup-reopen path wrote status = 'Open' via a raw SQL UPDATE, completely bypassing TicketModel::updateTicket() and WorkflowModel::isTransitionAllowed() — the one status-write path in the app that never consulted the workflow engine at all. If an admin configured the Workflow Designer to disallow a direct Closed->Open transition, this automated path still forced it unconditionally. Now checks isTransitionAllowed('Closed', 'Open', false) first (false since this is an unattended system account, not admin-elevated). If not allowed, falls back to any transition the Workflow Designer does allow from Closed that requires neither a comment nor admin privilege (both of which this unattended automation can't satisfy), and applies it via TicketModel::updateTicket() instead of raw SQL. If no such transition exists at all, the ticket is deliberately left Closed (rather than forcing an unconfigured state) with a comment and audit entry explaining why, so the recurrence is still visible to a human without silently violating workflow rules. Verified against real MariaDB across all three branches: direct Closed->Open allowed (reopens to Open), disallowed but Closed->'In Progress' available unattended (falls back correctly), and no usable transition configured at all (ticket correctly stays Closed). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv	2026-09-11 12:37:46 -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
jared	5a69c41f48	Merge development into main: error-handling rollout + session privilege re-sync (#38, #39, #56, #105) ... - Complete ErrorHandler rollout: wire into all endpoints, fix display_errors gaps, add styled 500 page (#38, #39, #105) - Periodically re-sync session privileges from Authelia (#56) deploy-2026.09.11-194	2026-09-11 12:23:49 -04:00
jaredandClaude Sonnet 5	6b7e67eee4	Periodically re-sync session privileges from Authelia (#56) ... AuthMiddleware::authenticate() only re-read Remote-User/Remote-Groups (and thus is_admin, via UserModel::syncUserFromAuthelia) when $_SESSION['user'] didn't exist yet. Once a session existed, every subsequent request only checked the idle timer — never re-validating against current Authelia/LLDAP state. An admin's group membership revoked in LLDAP, or a logout at the Authelia proxy, left their already-open session with full access for up to SESSION_TIMEOUT (5h default), with no way to force early revocation short of clearing the server-side session store. Added PRIVILEGE_RESYNC_INTERVAL (default 5 min, matching UserModel's own cache TTL) and a resyncPrivileges() check on every already-authenticated request past that interval: re-reads the current request's forward-auth headers (enforcing the trusted-proxy check again, same as a fresh login), and either destroys the session and redirects to re-auth if the user no longer has any required group, or re-syncs is_admin/groups/display_name/email if they do. Best-effort if this particular request doesn't carry forward-auth headers at all (skips silently rather than force-logging out, retried next interval). UserModel::syncUserFromAuthelia() has its own 5-minute in-process cache keyed only by username (not by the groups being synced), so a naive re-call during a resync would have kept returning the pre-revocation cached result for up to 5 more minutes — invalidated that cache entry immediately beforehand to guarantee a real re-sync. Verified against real MariaDB across a fresh login, a same-interval request confirming no premature resync, an admin-privilege-revocation mid-session (is_admin flips to false in both session and DB, verified via a direct query), and a full group-membership revocation (session destroyed, redirected to re-auth, confirmed the request never reaches past that point). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv	2026-09-11 12:17:27 -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
jared	bcc732e605	Merge development into main: connection/security hardening batch (#85, #94, #103, #104) ... - Route index.php and create_ticket_api.php through Database::getConnection() (#103, #104) - Make TRUSTED_PROXIES' insecure-by-default risk loudly visible (#94) - Add recovery csrf_token to 12 hand-rolled CSRF rejection responses (#85) deploy-2026.09.11-190	2026-09-11 11:54:36 -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	98d30cbc58	Route index.php and create_ticket_api.php through Database::getConnection() (#103, #104) ... Both files opened their own raw new mysqli(...) connection instead of using Database::getConnection(), missing the charset/timezone sync every other connection gets. index.php's connection serves nearly all non-API web traffic (dashboard, ticket view, ticket create) — any NOW()/CURDATE()-based query through it used the DB server's default session timezone instead of the app's configured TIMEZONE, and no explicit utf8mb4 charset meant multi-byte characters typed into a ticket via the non-JS POST fallback could get corrupted at write time. create_ticket_api.php (the hwmonDaemon Bearer endpoint) had the same timezone gap, risking created_at landing on the wrong 'day' relative to every other ticket-creation path. index.php's raw die("Connection failed: " . $conn->connect_error) also leaked raw mysqli error text (host/user/failure reason) to any unauthenticated visitor on a DB outage; it now logs via error_log() and shows a generic message instead. create_ticket_api.php already handled this correctly (JSON error + error_log, no leak) and needed no behavior change there beyond the connection source. Verified against real MariaDB: the new connection path reports the configured -04:00 session time_zone and utf8mb4 charset, vs. SYSTEM tz on the old raw-mysqli path; a simulated connection failure (bad DB_NAME) confirmed only the generic message reaches the response body while the raw driver error goes to error_log(). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv	2026-09-11 11:42:21 -04:00
jared	99a0cf59a8	Merge development into main: Matrix-notification visibility-leak batch (#46, #69, #71, #72) ... - Redact ticket title for non-public tickets in create/status-change Matrix notifications (#46) - Exclude shared notify list and redact title in notifyWatchers() for non-public tickets (#71) - Redact ticket title for non-public tickets in assignment notifications (#72) - Verify mentioned-user access before sending @mention notifications (#69) deploy-2026.09.11-186	2026-09-11 11:26:58 -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	90d798966b	Exclude shared notify list and redact title in notifyWatchers() for non-public tickets (#71) ... notifyWatchers() only redacted the comment/activity preview for non-public tickets — the shared MATRIX_NOTIFY_USERS list was still merged into notify_users unconditionally, and the ticket title was never redacted at all. A status-change/comment notification for a confidential ticket with watchers still broadcast that ticket's title to the shared list, even though the function's own docblock intended to protect non-public tickets from it. For non-public tickets, the shared list is now excluded entirely (only actual watchers are notified) and the title is redacted via the same redactedTitle() helper added for #46. Verified against real MariaDB with a real watcher row: for a confidential ticket, the captured webhook payload has only the watcher's Matrix ID (no shared list) and a redacted title; for the same ticket made public, the shared list is included and the title passes through unchanged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015nCxwFFsy8ouMWzn56rPVP	2026-09-08 21:49:17 -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
jared	3664719148	Merge development into main: high-priority security/reliability batch (#27, #28, #30, #32) ... - Add missing rate limiting to create_ticket_api.php (#27) - Fix visibility-group matching disagreement between filter and access check (#28) - Replace illusory transaction wrapping in migrate.php with statement-level resume (#30) - Fix ticket_watchers.ticket_id type mismatch and missing FK (#32) deploy-2026.09.09-182	2026-09-08 21:33:33 -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	fba251b85d	Replace illusory transaction wrapping in migrate.php with statement-level resume (#30) ... migrate.php wrapped each migration file's statements in begin_transaction()/rollback(), but MySQL DDL statements cause an implicit commit — so a rollback couldn't actually undo earlier DDL already executed within the same file. A migration failing partway left the DB altered but unrecorded, and the next run retried the whole file from statement 1, hitting "already exists" errors not on the safe-to-ignore allowlist and permanently wedging the runner. Removed the transaction wrapper (it only gave false confidence) and added a migration_progress table that records the index of the last successfully-executed statement in each file. A re-run after a partial failure now resumes right after the last success instead of re-executing already-applied DDL. Verified against real MariaDB with a 4-statement migration where statement 3 fails: run 1 correctly applies statements 1-2 and records progress at index 1; after fixing the bad statement, run 2 resumes at statement 3, completes, and clears the progress marker. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015nCxwFFsy8ouMWzn56rPVP	2026-09-08 21:28:32 -04:00
jaredandClaude Sonnet 5	fd777aa690	Fix visibility-group matching disagreement between filter and access check (#28) ... getVisibilityFilter() (dashboard list/stats) matched via FIND_IN_SET(?, REPLACE(t.visibility_groups, ' ', '')) — stripping spaces from the column but not from the bound group name — while canUserAccessTicket() (single-ticket access) did a plain trim with no space-stripping at all. For a group name containing a space (e.g. "IT Support"), a member could open an internal ticket directly by URL but never see it in their dashboard list or stats counts. Now strips spaces from the bound parameter too, matching the column- side normalization, so both paths agree. Verified against real MariaDB: a ticket visible via canUserAccessTicket() for a space-containing group is now also matched by getVisibilityFilter()'s SQL, a wrong-group user is denied by both, and the plain no-space case is unaffected. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015nCxwFFsy8ouMWzn56rPVP	2026-09-08 21:28:25 -04:00
jaredandClaude Sonnet 5	a9a39adcf8	Add missing rate limiting to create_ticket_api.php (#27) ... Every other Bearer-key endpoint (ticket_status_api.php, ticket_comment_api.php) calls RateLimitMiddleware::apply('api') before opening a DB connection; create_ticket_api.php didn't, contradicting README.md's claim that the whole Bearer API is rate-limited. A leaked or guessed API key could hammer ticket creation unthrottled, each insert also firing a Matrix webhook. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015nCxwFFsy8ouMWzn56rPVP	2026-09-08 21:28:18 -04:00
jared	23d94bfae7	Merge development into main: quick-win UX/perf batch (#76, #64, #99, #100) ... - Consolidate dashboard clear-filters controls to one shared function (#76) - Make SLA priority-alert banner update live on priority change (#64) - Add HTTP Range/partial-content support to attachment downloads (#99) - Paginate attachment listing (#100) deploy-2026.09.09-178	2026-09-08 21:18:14 -04:00
jaredandClaude Sonnet 5	e39b4f81ea	Avoid insertAdjacentHTML flagged by semgrep in attachment pagination (#100) ... The "load more attachments" append path used grid.insertAdjacentHTML('beforeend', html), which the CI semgrep scan flags as a blocking finding (detection of insertAdjacentHTML from a non-constant string). The content was already fully escaped via lt.escHtml() on every field, but switched to the same temp-element + innerHTML + appendChild pattern used elsewhere to build DOM from a generated HTML string, avoiding the flagged API without changing behavior. Re-verified pagination append/remove behavior via jsdom. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015nCxwFFsy8ouMWzn56rPVP	2026-09-08 21:05:43 -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	d84d8fae58	Make SLA priority-alert banner update live on priority change (#64) ... The P1/P2 SLA breach banner and progress bar were rendered server-side at page load and never touched again. Changing a ticket's priority in edit mode (P1->P3 or P3->P1) left the banner in a stale state — showing/counting for a priority that no longer applied — until the page was reloaded. Moved the banner's render/update/teardown logic into a reusable renderSlaBanner() in ticket.js (verified via jsdom against real DOM: creates the banner for P1/P2, removes it when priority drops below P2 or the ticket is closed, and re-creates it including the already-breached state when priority is raised into P1/P2 range). The priority-change handler now calls it after a successful update, and the initial page load calls it once instead of relying on duplicated server-rendered markup + inline script. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015nCxwFFsy8ouMWzn56rPVP	2026-09-08 20:57:10 -04:00
jaredandClaude Sonnet 5	b6d3cc4e70	Consolidate dashboard clear-filters controls to one shared function (#76) ... The sidebar's own Clear button cleared status/category/type/dates but never search/priority/assigned_to, while the page-level "Clear All Filters" button cleared a different subset. Neither control alone reliably returned the dashboard to a fully unfiltered state. The sidebar button now delegates to clearAllFilters() so both controls always clear the same complete set of params. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015nCxwFFsy8ouMWzn56rPVP	2026-09-08 20:57:03 -04:00
jaredandClaude Sonnet 5	cef1689c05	Merge development into main: 15-issue triage + fix batch ... Fixes 15 tracked issues from the tinker_tickets tracker, all verified against a local MariaDB instance and/or jsdom/manual test harnesses where applicable: #75, #84, #102, #29, #53, #90, #60, #79, #61, #31, #96, #41, #89, #59, #52, #42, #66, #40, #106, #54, #92, #97, #51, #91, #101, #63, #55, #65, #107. Also fixes a real, previously-undetected outage in .env.example: parse_ini_file() could not parse the file as shipped (fragile '#' comment handling plus an unquoted LDAP_BIND_DN value), meaning the documented setup step of `cp .env.example .env` would have broken every fresh deployment. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X deploy-2026.09.08-172	2026-09-08 15:39:36 -04:00
jaredandClaude Sonnet 5	3cca956ee7	Preserve native undo/redo in markdown toolbar buttons (#107) ... insertMarkdownFormat, insertMarkdownText, toolbarList, toolbarHeading, and toolbarQuote all set textarea.value = ... directly. Assigning .value programmatically discards the browser's entire native undo stack (vs. document.execCommand('insertText', ...), which preserves it) — e.g. type a paragraph, click Bold, then Ctrl+Z undid the whole paragraph instead of just the bold markup. Added insertTextPreservingUndo(), which selects the exact range being replaced and routes through execCommand('insertText', ...) — the same mechanism real typing uses — falling back to the old direct assignment (losing undo, matching prior behavior) only if execCommand is unavailable or unsuccessful. Verified with a jsdom harness that the fallback path (jsdom doesn't implement execCommand, since native undo is a real-browser-only feature untestable via jsdom) produces byte-identical resulting text and cursor positions to the original implementation across all 5 toolbar functions, for both selected and cursor-only cases. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X	2026-09-08 14:21:39 -04:00
jaredandClaude Sonnet 5	818af137f3	Add touch-event fallback to lt.sortable for kanban drag-and-drop (#65) ... lt.sortable only wired dragstart/dragend/dragover/drop — the native HTML5 Drag-and-Drop API. iOS Safari doesn't implement HTML5 DnD on arbitrary elements at all, and mobile Chrome's support is poor, so kanban card status-drag was effectively unusable via touch, despite README's "Touch-friendly controls" claim. Added touchstart/touchmove/touchend/touchcancel handling that mirrors the existing mouse-based behavior: a small movement threshold (8px) distinguishes a tap/scroll from drag intent, the dragged card is repositioned via fixed positioning to follow the finger (reparented to document.body to avoid clipping by an overflow:hidden ancestor), and elementFromPoint resolves the hover target for the same placeholder-insertion logic dragover already uses, including cross-column moves via the shared group check. touchmove/touchend/touchcancel are registered on document rather than the sortable list itself: since touch events keep targeting their touchstart element for the whole gesture regardless of DOM mutations, and the dragged item gets reparented to document.body mid-drag, a listener on the original list would stop receiving bubbled events once that reparenting happens. lt.sortable lives in base.js, the shared web_template copy used by other LotusGuild apps (per its own header comment) — this fix should be contributed upstream too, not just kept local to this repo. Verified with a jsdom harness (stubbing getBoundingClientRect and elementFromPoint against known layouts) covering: sub-threshold movement not starting a drag, same-column reorder, cross-column move with correct final DOM parent and order, and touchcancel cleanly resetting state. This caught a real bug during development — an earlier version listened on the list element for touchmove/touchend, which silently stopped receiving events after the drag-start reparenting. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X	2026-09-08 14:18:54 -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	67d3c13bb6	Fix .env.example: missing Matrix vars, plus a real parse_ini_file outage (#63) ... Primary fix (#63): added the 5 env vars config.php reads and README documents but .env.example never listed: MATRIX_DOMAIN, SYNAPSE_ADMIN_URL, SYNAPSE_ADMIN_TOKEN, MATRIX_NOTIFY_COMMENTS, and MATRIX_NOTIFY_ASSIGNMENTS. A deployer following only .env.example had no indication these existed, silently missing watcher Matrix DMs and comment/assignment notifications. While verifying the fix by actually running .env.example through parse_ini_file() (what config.php calls), found this file could not be parsed at all — a real, currently-live outage for anyone following its own first-line instruction ("Copy this file to .env and fill in your values"): 1. PHP's ini parser treats "#" comments as fragile: punctuation like parentheses or quotes inside a "#" comment can throw a syntax error even though the line is meant to be inert. The file's header comment itself (and 15+ other comment lines) tripped this. Switched every comment to ";", which parse_ini_file treats as a true inert comment regardless of content — verified with isolated repros of both prefixes under all three INI_SCANNER_* modes. 2. LDAP_BIND_DN's example value contained unquoted "=" and commas, violating the file's own documented quoting rule and causing a second, independent parse failure. Quoted it (and the two other comma-bearing LDAP DN values) to match the rule. config.php has zero fallback for a parse failure — it die()s immediately — so either bug alone would have taken down every fresh deployment that didn't hand-edit the example file's comments first. Verified end-to-end: copied .env.example to a real .env file unmodified and ran it through config.php's exact parse_ini_file + quote-stripping logic; it now parses cleanly with all 23 keys (including the 5 new ones) and LDAP_BIND_DN resolves to the correct unquoted DN string. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X	2026-09-08 14:12:04 -04:00
jaredandClaude Sonnet 5	e4c240009d	Dashboard cleanup: chart empty-state message, dead click-handler (#101) ... Two small findings from the same dashboard audit pass: 1. Charts rendered a bare empty frame with no message when the filtered dataset was empty (fresh install, or a non-admin's visibility-filtered ticket set happening to be zero). makeDonut/ makeBar now show a "No data for current filters" message in the chart's place instead of silently doing nothing. 2. Stat cards had two independent, redundant click-handler implementations. lt.statsFilter.init() (base.js, shared web_template code) read each card's data-filter-key/data-filter-val attributes and called window.lt_onStatFilter(key, val) on click — but that global is never defined anywhere in this app, so it only toggled a cosmetic .active class with no functional effect. The actual navigation logic is the separate handler at ~line 1282 that ignores those attributes entirely. Both fired on the same click with no visible symptom, but the markup looked load-bearing and wasn't — a trap for a future edit that touches one implementation assuming it's the only one. Removed the dead lt.statsFilter.init() call and the now-unused data-filter-key/data-filter-val attributes from this app's DashboardView.php (left the shared lt.statsFilter module in base.js itself untouched, since other LotusGuild apps consuming the same shared template file may define their own lt_onStatFilter). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X	2026-09-08 12:27:26 -04:00
jaredandClaude Sonnet 5	6553c0227d	Fix dogpile cache-overwrite race in CacheHelper::remember() (#91) ... remember() had no protection against a slow cache-miss recomputation overwriting a fresher write. If Request A started computing stats just before a ticket mutation + invalidateCache(), and Request B started just after (correctly computing fresh, post-mutation data), A could finish (using stale pre-mutation data) after B and overwrite B's fresh cache entry — extending staleness by up to another full TTL. Added a per-prefix invalidation epoch: delete() bumps it, and remember() snapshots it before running the callback and only writes if the epoch hasn't changed since — otherwise a newer invalidation happened mid-computation and the result being written is already stale, so it's dropped (the caller still gets its own result; only the cache write is skipped). Verified with two real concurrent PHP processes racing against the same cache key (a slow "Request A" callback vs. a fast "Request B" that invalidates then recomputes): the cache ends up holding B's fresh value, not A's late stale overwrite. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X	2026-09-08 12:25:04 -04:00
jaredandClaude Sonnet 5	4fade1a9d3	Reject the semantic inverse of an existing ticket dependency (#51) ... addDependency()'s "already exists" check only matched the exact (ticket_id, depends_on_id, dependency_type) tuple. A user could add "A blocks B" from ticket A's page, then separately add "B blocked_by A" from ticket B's page — wouldCreateCycle() correctly found no cycle (both normalize to the same precedence edge), so the insert was allowed, creating two DB rows describing one real relationship (shown twice on ticket B's page: once under Dependencies, once under Dependents). Added an inverse-relationship check before the insert: blocks/ blocked_by are inverses of each other, relates_to is its own inverse (symmetric). duplicates has no defined inverse type in the schema, so both directions remain independently insertable, which is correct — "A duplicates B" and "B duplicates A" are distinct claims. Verified against a local MariaDB instance: the exact repro from the issue (A blocks B, then B blocked_by A) is now rejected, relates_to's symmetric case is rejected in both directions, and duplicates in either direction is unaffected. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X	2026-09-08 12:23:04 -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	caeb9269d9	README: add missing StatsModel::invalidateCache() caller (#92) ... Dev Note #24 listed 7 callers; api/ticket_status_api.php (the Bearer API's status-change endpoint) also correctly calls invalidateCache() but wasn't in the list. Behavior was already correct — this is a pure documentation completeness fix so the caller list stays an accurate reference for future maintainers deciding whether a new mutating path needs the same call. Verified via grep -rl "invalidateCache" that these 8 files (plus StatsModel.php itself, and an unrelated same-named method on UserModel) are the complete set of real callers. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X	2026-09-08 11:59:23 -04:00