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
- 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>
P1-A: Fix CSP - add fonts.googleapis.com to style-src, fonts.gstatic.com to font-src
P1-B: CSRF token rotation - add rotateToken() to CsrfMiddleware; bootstrap.php rotates
after successful validation and stores in $GLOBALS['_new_csrf_token']; add
apiRespond() helper to append token to responses; lt.api interceptor in
layout_footer.php auto-updates window.CSRF_TOKEN from responses
P1-C: Styled 403/404 error views with TDS layout instead of raw text; index.php now
uses requireAdmin() helper eliminating 7 duplicated guard blocks (P3-D)
P2-A: Remove duplicate JS-generated keyboard help modal from keyboard-shortcuts.js;
'?' key now routes to static #lt-keys-help modal in footer
P2-B: Asset versioning driven by config ASSET_VERSION key; base.css and base.js get
?v= cache-busting in layout_header.php
P2-C: Add data-theme="dark" to <html> tag to prevent FOUC on light-mode users
P2-E: Escape status value in dashboard.js hover preview class attribute via lt.escHtml()
P2-F: Replace bespoke showLoadingOverlay() with lt-spinner / lt-loading-text from
base.css; add .lt-loading-overlay wrapper CSS to dashboard.css
P2-G: Add keyboard-shortcuts.js to all 7 admin views so J/K nav and ? help work
P3-A: APP_NAME, APP_SUBTITLE, APP_VERSION driven from config.php; layout header/footer
use config values instead of hardcoded strings
P3-G: Replace custom initTableSorting() with lt.sortTable.init() which manages aria-sort
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Security:
- Fix IDOR in delete/update comment (add ticket visibility check)
- XSS defense-in-depth in DashboardView active filters
- Replace innerHTML with DOM construction in toast.js
- Remove redundant real_escape_string in check_duplicates
- Add rate limiting to get_template, download_attachment, audit_log,
saved_filters, user_preferences endpoints
Bug fixes:
- Session timeout now reads from config instead of hardcoded 18000
- TicketController uses $GLOBALS['config'] instead of duplicate .env parsing
- Add DISCORD_WEBHOOK_URL to centralized config
- Cleanup script uses hashmap for O(1) ticket ID lookups
Dead code removal (~100 lines):
- Remove dead getTicketComments() from TicketModel (wrong bind_param type)
- Remove dead getCategories()/getTypes() from DashboardController
- Remove ~80 lines dead Discord webhook code from update_ticket API
Consolidation:
- Create api/bootstrap.php for shared API setup (auth, CSRF, rate limit)
- Convert 6 API endpoints to use bootstrap
- Extract escapeHtml/getTicketIdFromUrl into shared utils.js
- Batch save for user preferences (1 request instead of 7)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jaredandClaude Sonnet 5