Commit Graph
9 Commits
Author SHA1 Message Date
jaredandClaude Sonnet 5 1b1801696f Overhaul Bearer API rate limiting: real config, per-key isolation, skip session (#80, #81, #82, #83)
Lint / PHP (phpcs PSR-12) (push) Successful in 28s
Lint / JS (eslint) (push) Successful in 12s
Lint / PHP requirements (version + extensions) (push) Successful in 30s
Lint / Notify on failure (push) Skipped
Security / PHP Security (semgrep) (push) Successful in 3m9s
Lint / Deploy (push) Successful in 2s
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 Opus 4.8 9941fd2dfa Address remaining review items: Synapse caching, cycle detection, cache/ratelimit/kanban
Security / PHP Security (semgrep) (push) Successful in 1m45s
Lint / Deploy (push) Successful in 3s
Lint / Notify on failure (push) Has been skipped
Lint / PHP (phpcs PSR-12) (push) Successful in 21s
Lint / JS (eslint) (push) Successful in 9s
Lint / PHP requirements (version + extensions) (push) Successful in 26s
- SynapseHelper: memoize username->Matrix-ID lookups per-request (incl. negative
  results) and add an overall time budget to resolveUsernames() plus a 2s connect
  timeout, so notifying N watchers with a slow/unreachable Synapse can't stall the
  request for N x 5s. (Chosen over async/queue per maintainer.)
- DependencyModel: fix cycle detection treating 'blocks' and 'blocked_by' as the
  same edge direction. They are inverse relationships (single row each, no mirror
  row), so the traversal now walks a unified precedence graph (blocks: ticket->
  depends_on; blocked_by: depends_on->ticket) and wouldCreateCycle normalizes the
  new edge's direction. Prevents both false-positive and missed cycles.
- CacheHelper: anchor prefix-delete to exact key boundaries (bare prefix or
  prefix + '_' + md5) so delete('workflow') can't wipe a 'workflow_rules' cache.
- RateLimitMiddleware: hold an exclusive flock across the per-IP counter's
  read-modify-write so concurrent requests can't both read N and write N+1
  (undercounting past the limit). Fails open if the file can't be locked.
- dashboard.js: kanban status update now uses lt.api.post (per no-raw-fetch
  convention) and reverts the card AND the optimistic column counts on failure
  (the old raw-fetch catch left the card moved without reverting).
- BulkOperationsModel: document that bulk_status/bulk_close intentionally bypass
  workflow transition validation (admin override, by design).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-30 12:55:50 -04:00
jaredandClaude Opus 4.8 4164f85051 Fix bugs found in second multi-agent review
Security / PHP Security (semgrep) (push) Successful in 1m14s
Lint / Deploy (push) Successful in 3s
Lint / PHP (phpcs PSR-12) (push) Successful in 19s
Lint / JS (eslint) (push) Successful in 8s
Lint / PHP requirements (version + extensions) (push) Successful in 24s
Lint / Notify on failure (push) Has been skipped
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
jared c90bdc8ac8 style: auto-fix 1340 phpcs PSR-12 violations via phpcbf; exclude MissingNamespace and SideEffects
Lint / PHP (phpcs PSR-12) (push) Failing after 29s
Lint / JS (eslint) (push) Successful in 12s
2026-04-13 20:56:10 -04:00
jaredandClaude Sonnet 4.6 f983269f93 Fix file upload security, bind_param mismatch, and cookie flags
- upload_attachment.php: derive stored file extension from validated MIME type
  instead of user-supplied filename, preventing executable extension attacks
  (e.g. a PHP file renamed to evil.txt would now be stored as .txt)
- CustomFieldModel.php: fix bind_param type string in updateDefinition()
  'sssssiiiii' (10 chars) → 'sssssiiii' (9 chars) to match 9 SQL placeholders
- RateLimitMiddleware.php: replace MD5 with SHA256 for rate limit file hashing
- user_preferences.php: add httponly, secure, samesite=Lax flags to ticketsPerPage
  cookie to prevent XSS/CSRF cookie theft

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-03-29 18:14:18 -04:00
jaredandClaude Opus 4.5 7575d6a277 Add performance, security, and reliability improvements
- Consolidate all 20 API files to use centralized Database helper
- Add optimistic locking to ticket updates to prevent concurrent conflicts
- Add caching to StatsModel (60s TTL) for dashboard performance
- Add health check endpoint (api/health.php) for monitoring
- Improve rate limit cleanup with cron script and efficient DirectoryIterator
- Enable rate limit response headers (X-RateLimit-*)
- Add audit logging for workflow transitions
- Log Discord webhook failures instead of silencing
- Fix visibility check on export_tickets.php
- Add database migration system with performance indexes
- Fix cron recurring tickets to use assignTicket method

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2026-01-30 14:39:13 -05:00
jaredandClaude Opus 4.5 37be81b3e2 Add PHP 7.4+ type hints to helpers, models, and middleware
Added strict typing with parameter types, return types, and property
types across all core classes:
- helpers: Database, ErrorHandler, CacheHelper
- models: TicketModel, UserModel, WorkflowModel, TemplateModel, UserPreferencesModel
- middleware: RateLimitMiddleware, CsrfMiddleware, SecurityHeadersMiddleware

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2026-01-29 11:04:36 -05:00
jaredandClaude Opus 4.5 fa40010287 Security hardening and performance improvements
- Add visibility check to attachment downloads (prevents unauthorized access)
- Fix ticket ID collision with uniqueness verification loop
- Harden CSP: replace unsafe-inline with nonce-based script execution
- Add IP-based rate limiting (supplements session-based)
- Add visibility checks to bulk operations
- Validate internal visibility requires groups
- Optimize user activity query (JOINs vs subqueries)
- Update documentation with design decisions and security info

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2026-01-28 20:27:15 -05:00
jaredandClaude Opus 4.5 be505b7312 Implement comprehensive improvement plan (Phases 1-6)
Security (Phase 1-2):
- Add SecurityHeadersMiddleware with CSP, X-Frame-Options, etc.
- Add RateLimitMiddleware for API rate limiting
- Add security event logging to AuditLogModel
- Add ResponseHelper for standardized API responses
- Update config.php with security constants

Database (Phase 3):
- Add migration 014 for additional indexes
- Add migration 015 for ticket dependencies
- Add migration 016 for ticket attachments
- Add migration 017 for recurring tickets
- Add migration 018 for custom fields

Features (Phase 4-5):
- Add ticket dependencies with DependencyModel and API
- Add duplicate detection with check_duplicates API
- Add file attachments with AttachmentModel and upload/download APIs
- Add @mentions with autocomplete and highlighting
- Add quick actions on dashboard rows

Collaboration (Phase 5):
- Add mention extraction in CommentModel
- Add mention autocomplete dropdown in ticket.js
- Add mention highlighting CSS styles

Admin & Export (Phase 6):
- Add StatsModel for dashboard widgets
- Add dashboard stats cards (open, critical, unassigned, etc.)
- Add CSV/JSON export via export_tickets API
- Add rich text editor toolbar in markdown.js
- Add RecurringTicketModel with cron job
- Add CustomFieldModel for per-category fields
- Add admin views: RecurringTickets, CustomFields, Workflow,
  Templates, AuditLog, UserActivity
- Add admin APIs: manage_workflows, manage_templates,
  manage_recurring, custom_fields, get_users
- Add admin routes in index.php

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2026-01-20 09:55:01 -05:00