Fix bugs across data layer, API, frontend, ops (multi-agent review) #24

Merged

jared merged 8 commits from development into main 2026-07-10 20:26:15 -04:00

Conversation 0 Commits 8 Files Changed 50

+1734 -479

jared commented 2026-07-10 17:46:08 -04:00

Owner

Copy Link

Copy Source

This PR lands a coordinated, multi-agent bug-fix pass (no feature changes). Every change was code-reviewed and lint-checked; backend fixes were runtime-verified on beta.

Highlights:

• Critical fatals fixed: CustomFieldModel bind_param by-ref (PHP 8), RecurringTicketModel swapped bind types, missing DB schema baseline, wrong CREATE TABLE stub in the external API.
• Security: fail-closed forward-auth trust, dependency/comment visibility leaks, visibility-change authz, server-side requires_comment enforcement, CSRF method-gap + desync auto-recovery, comment author spoofing, CSV injection, health info leak.
• Correctness: varchar ticket_id bindings (leading-zero IDs), timezone alignment, recurring next-run dates, workflow cache poisoning, stats-cache invalidation on all create paths.
• Frontend: CSRF resync, status-change comment orchestration, markdown XSS hardening, kanban/mention/keyboard fixes.
• Ops/docs: schema baseline (validated), recurring-cron notifications, audit-log retention cron, safe orphan-upload cleanup, README/.env accuracy.

CI is green on the development branch. Backend paths verified on beta (custom fields, recurring, leading-zero ticket lookup, timezone, workflow).

🤖 Generated with Claude Code

This PR lands a coordinated, multi-agent bug-fix pass (no feature changes). Every change was code-reviewed and lint-checked; backend fixes were runtime-verified on beta. Highlights: - Critical fatals fixed: CustomFieldModel bind_param by-ref (PHP 8), RecurringTicketModel swapped bind types, missing DB schema baseline, wrong CREATE TABLE stub in the external API. - Security: fail-closed forward-auth trust, dependency/comment visibility leaks, visibility-change authz, server-side requires_comment enforcement, CSRF method-gap + desync auto-recovery, comment author spoofing, CSV injection, health info leak. - Correctness: varchar ticket_id bindings (leading-zero IDs), timezone alignment, recurring next-run dates, workflow cache poisoning, stats-cache invalidation on all create paths. - Frontend: CSRF resync, status-change comment orchestration, markdown XSS hardening, kanban/mention/keyboard fixes. - Ops/docs: schema baseline (validated), recurring-cron notifications, audit-log retention cron, safe orphan-upload cleanup, README/.env accuracy. CI is green on the development branch. Backend paths verified on beta (custom fields, recurring, leading-zero ticket lookup, timezone, workflow). 🤖 Generated with [Claude Code](https://claude.com/claude-code)

jared added 7 commits 2026-07-10 17:46:08 -04:00

Fix data-layer bugs: bind_param fatals, ticket_id bindings, cache poisoning ... 882ab2662c

- CustomFieldModel: assign ?? fallbacks to variables before bind_param
  (by-reference args cannot be ?? expressions; fatal on PHP 8.2, custom
  fields were uncreatable/uneditable)
- RecurringTicketModel::create: fix swapped bind type for schedule_type
  (enum bound as int coerced 'daily' to 0, breaking the cron)
- TicketModel/CommentModel: bind varchar ticket_id as string not int so
  the unique index is usable and leading-zero IDs match; ticket_watchers
  (int column) left as integer
- TicketModel::deleteTicket: delete from custom_field_values (real table)
  not the nonexistent ticket_custom_fields
- TicketModel search: honor literal '0'; never emit AGAINST('*') on
  all-special-char input (fall back to LIKE)
- TicketModel::updateTicket: disambiguate not-found vs no-op vs genuine
  optimistic-lock conflict on zero affected rows
- WorkflowModel: do not cache transitions/statuses on DB failure (a
  transient error no longer blocks all status changes for the TTL)
- DependencyModel: filter linked tickets by visibility (new optional user
  context params) to stop confidential metadata leaking via dependencies
- BulkOperationsModel: validate status/priority/assignee before mutating
- AuditLogModel: gate getClientIP forwarded headers on trusted proxies;
  add missing action/entity types so audit-log filters work
- WorkflowModel: add transitionRequiresComment() accessor for enforcement
- CommentModel: stop leaking raw DB errors to clients (log instead)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Fix helpers/config: timezone, comment leak, silent misconfig, cache perms ... c5f7a01e1d

- Database.php: pin MySQL session time_zone to the configured named zone
  (mysql.time_zone tables now loaded on the DB) with a fixed-offset
  fallback, so NOW()/TIMESTAMP and PHP agree regardless of the DB server's
  SYSTEM tz. Best-effort, never fatals the connection.
- NotificationHelper: redact comment-body previews for internal/
  confidential tickets in sendCommentNotification and notifyWatchers so
  they are not leaked to the shared Matrix notify list (new $visibility
  param; callers wired in the API batch).
- config.php: die with a clear error if parse_ini_file fails instead of
  silently falling back to insecure defaults (empty DB pass / proxies).
- CacheHelper: create cache dir 0700 and cache files 0600 so other local
  users cannot read or poison security-relevant cached data.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Fix API security: dependency/visibility leaks, authz, CSRF, comment spoofing ... 327c225ded

- 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>

Fix API correctness: external API stub/collision, recurring dates, CSV, audit ... d11cb989bf

- 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>

Fix frontend JS: CSRF resync, status-comment flow, markdown/XSS, kanban ... 113b7f9d3f

- base.js lt.api: resync window.CSRF_TOKEN from response bodies before
  throwing on errors and attach err.data/err.status, so a desynced client
  auto-recovers without a reload
- add lt.ticketStatus.submit: status changes that require a comment now
  prompt, post the comment, and retry update_ticket with it; wired into
  the ticket dropdown, dashboard quick-status, kanban drag-drop and the
  1-4 keyboard shortcuts (bulk ops unchanged) — matches the new server
  requires_comment enforcement
- base.js markdown.render: drop the unsafe marked/markdownit delegation;
  always use the built-in XSS-safe renderer
- ticket.js: XHR upload sends the X-CSRF-Token header and resyncs the
  token; use lt.escHtml instead of a re-inlined escape chain; @-mention
  trigger requires a word boundary (no firing inside emails); idempotent,
  anchor-safe highlightMentions
- base.js typeahead: discard out-of-order async results
- markdown.js: balanced table tbody/thead; ticket-ref linkification runs
  after code extraction so #ids inside code aren't linked
- dashboard.js kanban: don't swallow the click after a drag
- keyboard-shortcuts.js: J/K skip hidden/skeleton rows; drop duplicate ?

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Fix views/controllers/router: command palette, create form, admin views ... 27a5db8c85

- Consolidate the duplicated command palette to a single overlay + init in
  the footer; fix New Ticket to route to /ticket/create (was a 404 /create);
  keep the CSP nonce and all commands
- TicketController create(): trim title, require a non-empty description,
  and honor the posted status (validated against the canonical list) instead
  of silently discarding it
- UserActivityView: 'Active Users' counts only users active in the selected
  range, not every registered user
- layout_footer/DashboardView: local esc() now escapes quotes so values used
  in HTML attributes can't break out
- TicketView: comments tab badge shows the true total, not just page one
- layout_header: gate the 'View activity log' link behind the admin flag
- index.php: validate /admin/user-activity date params; anchor the legacy
  /ticket.php route; align the audit action-type whitelist with the dropdown
- ApiKeysView: correct the external API sample to /create_ticket_api.php

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Add schema baseline, fix cron/retention, restore cleanup, correct docs ... d6214a0339

- migrations/000_baseline.sql: full schema baseline captured from prod
  (validated on a throwaway DB: 17 tables/17 FKs), so the schema is
  reproducible for fresh installs / disaster recovery
- create_recurring_tickets cron: send the Matrix ticket-created
  notification and invalidate the stats cache like the other create paths
- create_ticket_api.php + TicketController::create: invalidate the stats
  cache on create/escalate/reopen so dashboard counts aren't stale
- scripts/cleanup_orphan_uploads.php: restored, made safe (24h mtime
  grace, 9-digit-dir only, skips avatars/symlinks, matches the unique
  filename column, --dry-run)
- cron/cleanup_audit_log.php: enforce the configured audit-log retention
  (deleteOldLogs was implemented but never called)
- README: correct CSRF-rotation, hwmon dedup (no 24h window), SLA (no P3),
  stats-cache callers, and the project structure/endpoint listing
- .env.example: document TRUSTED_PROXIES fail-open risk and .env quoting

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

jared added 1 commit 2026-07-10 19:12:18 -04:00

Fix PHP 8.4 breakage: drop deprecated mysqli::ping(), harden dep handler ... 622cae8bbd

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>

jared merged commit 55087bf2cb into main 2026-07-10 20:26:15 -04:00

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

Clear labels

bug

Something is not working

duplicate

This issue or pull request already exists

enhancement

New feature

help wanted

Need some help

invalid

Something is wrong

question

More information is needed

wontfix

This won't be fixed

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

cole (Cole Keller) jared (Jared Vititoe) nathan (Nathan Vititoe)

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: LotusGuild/tinker_tickets#24