diff --git a/assets/js/dashboard.js b/assets/js/dashboard.js index 825365d..2c542d7 100644 --- a/assets/js/dashboard.js +++ b/assets/js/dashboard.js @@ -1219,29 +1219,30 @@ function populateKanbanCards() { if (dec) dec.textContent = '(' + Math.max(0, (parseInt(dec.textContent.replace(/\D/g,''),10)||1) - 1) + ')'; if (inc) inc.textContent = '(' + ((parseInt(inc.textContent.replace(/\D/g,''),10)||0) + 1) + ')'; - // POST status update - fetch('/api/update_ticket.php', { - method: 'POST', - credentials: 'same-origin', - headers: { 'Content-Type': 'application/json', 'X-CSRF-Token': window.CSRF_TOKEN || '' }, - body: JSON.stringify({ ticket_id: String(ticketId), status: newStatus }) - }) - .then(r => r.json()) - .then(data => { - if (data.success) { - lt.toast.success('Ticket #' + ticketId + ' → ' + newStatus, 2500); - movedCard.dataset.status = newStatus; - } else { - lt.toast.error('Status update failed: ' + (data.error || 'Unknown error')); - // Revert: put card back in original column - const origCol = document.getElementById(Object.keys(colStatusMap).find(k => colStatusMap[k] === oldStatus)); - if (origCol) origCol.appendChild(movedCard); - movedCard.dataset.status = oldStatus; - } - }) - .catch(() => { - lt.toast.error('Network error — status not saved'); - }); + // Revert the card to its original column and undo the optimistic counts. + const revert = function () { + const origCol = document.getElementById(Object.keys(colStatusMap).find(k => colStatusMap[k] === oldStatus)); + if (origCol) origCol.appendChild(movedCard); + movedCard.dataset.status = oldStatus; + if (dec) dec.textContent = '(' + ((parseInt(dec.textContent.replace(/\D/g, ''), 10) || 0) + 1) + ')'; + if (inc) inc.textContent = '(' + Math.max(0, (parseInt(inc.textContent.replace(/\D/g, ''), 10) || 1) - 1) + ')'; + }; + + // POST status update via the shared wrapper (adds CSRF + JSON, throws on non-2xx) + lt.api.post('/api/update_ticket.php', { ticket_id: String(ticketId), status: newStatus }) + .then(function (data) { + if (data && data.success) { + lt.toast.success('Ticket #' + ticketId + ' → ' + newStatus, 2500); + movedCard.dataset.status = newStatus; + } else { + lt.toast.error('Status update failed: ' + ((data && data.error) || 'Unknown error')); + revert(); + } + }) + .catch(function () { + lt.toast.error('Status update failed — reverting'); + revert(); + }); } Object.keys(columns).forEach(status => { diff --git a/helpers/CacheHelper.php b/helpers/CacheHelper.php index de4a22a..0e5f2e5 100644 --- a/helpers/CacheHelper.php +++ b/helpers/CacheHelper.php @@ -125,16 +125,23 @@ class CacheHelper return !file_exists($filePath) || @unlink($filePath); } - // Delete all files with this prefix - $pattern = self::getCacheDir() . '/' . preg_replace('/[^a-zA-Z0-9_]/', '_', $prefix) . '*.json'; - $files = glob($pattern); + // Delete all entries for this prefix. A key is either the bare prefix or + // prefix + '_' + md5(identifier) (32 hex chars, see makeKey). Match exactly + // that so a prefix can't clobber a different prefix that merely shares a + // leading substring — e.g. delete('workflow') must not wipe 'workflow_rules'. + $safePrefix = preg_replace('/[^a-zA-Z0-9_]/', '_', $prefix); + $keyRegex = '/^' . preg_quote($safePrefix, '/') . '(_[0-9a-f]{32})?$/'; + + $files = glob(self::getCacheDir() . '/' . $safePrefix . '*.json') ?: []; foreach ($files as $file) { - @unlink($file); + if (preg_match($keyRegex, basename($file, '.json'))) { + @unlink($file); + } } - // Clear memory cache entries with this prefix + // Clear matching memory cache entries foreach (array_keys(self::$memoryCache) as $key) { - if (strpos($key, $prefix) === 0) { + if (preg_match($keyRegex, $key)) { unset(self::$memoryCache[$key]); } } diff --git a/helpers/SynapseHelper.php b/helpers/SynapseHelper.php index 0792503..8f30f35 100644 --- a/helpers/SynapseHelper.php +++ b/helpers/SynapseHelper.php @@ -4,8 +4,9 @@ * SynapseHelper * * Resolves local (SSO) usernames → Matrix user IDs by querying the - * Synapse Admin REST API directly. No caching — every call is live - * so results never go stale. + * Synapse Admin REST API directly. Results are memoized per-request (not + * across requests, so they don't go stale between requests), and a batch + * resolve has an overall time budget to bound request latency. * * Required config (.env) keys: * MATRIX_DOMAIN e.g. matrix.lotusguild.org @@ -14,6 +15,12 @@ */ class SynapseHelper { + /** Per-request memo of username => Matrix ID|null, so repeat watchers are free. */ + private static array $cache = []; + + /** Total wall-clock budget (seconds) for a single resolveUsernames() batch. */ + private const RESOLVE_BUDGET_SECONDS = 5; + /** * Resolve a local SSO username to its Matrix user ID. * @@ -29,6 +36,11 @@ class SynapseHelper */ public static function resolveUsername(string $username): ?string { + // Serve from the per-request cache when we've already looked this up. + if (array_key_exists($username, self::$cache)) { + return self::$cache[$username]; + } + $baseUrl = $GLOBALS['config']['SYNAPSE_ADMIN_URL'] ?? null; $token = $GLOBALS['config']['SYNAPSE_ADMIN_TOKEN'] ?? null; $domain = $GLOBALS['config']['MATRIX_DOMAIN'] ?? null; @@ -49,6 +61,7 @@ class SynapseHelper 'Accept: application/json', ]); curl_setopt($ch, CURLOPT_RETURNTRANSFER, true); + curl_setopt($ch, CURLOPT_CONNECTTIMEOUT, 2); // fail fast when Synapse is unreachable curl_setopt($ch, CURLOPT_TIMEOUT, 5); $body = curl_exec($ch); @@ -56,25 +69,24 @@ class SynapseHelper $curlError = curl_error($ch); curl_close($ch); + $resolved = null; if ($curlError) { error_log("SynapseHelper: cURL error resolving '{$username}': {$curlError}"); - return null; - } - - if ($httpCode === 200) { + } elseif ($httpCode === 200) { $data = json_decode($body, true); // Confirm the response contains the name we expect if (!empty($data['name'])) { - return $data['name']; // e.g. "@jared:matrix.lotusguild.org" + $resolved = $data['name']; // e.g. "@jared:matrix.lotusguild.org" } - } - - // 404 = user not found in Synapse; other codes = error - if ($httpCode !== 404) { + } elseif ($httpCode !== 404) { + // 404 = user not found in Synapse; other codes = error error_log("SynapseHelper: unexpected HTTP {$httpCode} resolving '{$username}'"); } - return null; + // Memoize for the rest of this request (including negative results, so a + // missing/unreachable user isn't retried within the same request). + self::$cache[$username] = $resolved; + return $resolved; } /** @@ -87,7 +99,16 @@ class SynapseHelper public static function resolveUsernames(array $usernames): array { $ids = []; + $deadline = microtime(true) + self::RESOLVE_BUDGET_SECONDS; foreach ($usernames as $username) { + // Cached lookups are free and always allowed; for uncached ones, stop + // making live calls once the batch budget is spent so a slow/unreachable + // Synapse can't stall the request for N × per-call timeout. + $cached = array_key_exists($username, self::$cache); + if (!$cached && microtime(true) >= $deadline) { + error_log('SynapseHelper: resolve budget exhausted; skipping remaining lookups'); + break; + } $id = self::resolveUsername($username); if ($id !== null) { $ids[] = $id; diff --git a/middleware/RateLimitMiddleware.php b/middleware/RateLimitMiddleware.php index 1a9999b..97eb84d 100644 --- a/middleware/RateLimitMiddleware.php +++ b/middleware/RateLimitMiddleware.php @@ -84,28 +84,43 @@ class RateLimitMiddleware $ipHash = hash('sha256', $ip . '_' . $type); $filePath = self::getRateLimitDir() . '/' . $ipHash . '.json'; - // Load existing rate data + // Hold an exclusive lock across the whole read-modify-write so concurrent + // requests from the same IP can't both read the same count and each write + // count+1 (which would undercount and let the limit be exceeded). + $fh = @fopen($filePath, 'c+'); + if ($fh === false) { + // Can't open the counter file — fail open (don't block legitimate traffic). + return true; + } + + if (!flock($fh, LOCK_EX)) { + fclose($fh); + return true; + } + + $content = stream_get_contents($fh); $rateData = ['count' => 0, 'window_start' => $now]; - if (file_exists($filePath)) { - $content = @file_get_contents($filePath); - if ($content !== false) { - $decoded = json_decode($content, true); - if (is_array($decoded)) { - $rateData = $decoded; - } + if ($content !== false && $content !== '') { + $decoded = json_decode($content, true); + if (is_array($decoded)) { + $rateData = $decoded; } } - // Check if window has expired - if ($now - $rateData['window_start'] >= self::WINDOW_SECONDS) { + // Reset when the window has expired + if ($now - ($rateData['window_start'] ?? $now) >= self::WINDOW_SECONDS) { $rateData = ['count' => 0, 'window_start' => $now]; } - // Increment count $rateData['count']++; - // Save updated data - @file_put_contents($filePath, json_encode($rateData), LOCK_EX); + // Rewrite the file in place while still holding the lock + rewind($fh); + ftruncate($fh, 0); + fwrite($fh, json_encode($rateData)); + fflush($fh); + flock($fh, LOCK_UN); + fclose($fh); // Check if over limit return $rateData['count'] <= $limit; diff --git a/models/BulkOperationsModel.php b/models/BulkOperationsModel.php index ec1a383..2305670 100644 --- a/models/BulkOperationsModel.php +++ b/models/BulkOperationsModel.php @@ -104,6 +104,11 @@ class BulkOperationsModel $success = false; try { + // NOTE: bulk_status / bulk_close intentionally do NOT run + // WorkflowModel::isTransitionAllowed(). Bulk operations are an + // admin-only escape hatch for forcing ticket states (e.g. mass + // re-opening), so they bypass the workflow transition rules that + // the single-ticket update path enforces. This is by design. switch ($operation['operation_type']) { case 'bulk_close': // Get current ticket from pre-loaded batch diff --git a/models/DependencyModel.php b/models/DependencyModel.php index ba7395c..ad5e85f 100644 --- a/models/DependencyModel.php +++ b/models/DependencyModel.php @@ -190,14 +190,25 @@ class DependencyModel */ private function wouldCreateCycle($ticketId, $dependsOnId, $type): bool { - // Only check for cycles in blocking relationships + // Only blocking relationships impose an ordering that can form a cycle. if (!in_array($type, ['blocks', 'blocked_by'])) { return false; } - // Check if dependsOnId already has ticketId in its dependency chain + // Normalize the new row to a precedence edge "from must finish before to": + // (t, d, 'blocks') => t blocks d => edge t -> d + // (t, d, 'blocked_by') => t blocked_by d => edge d -> t + if ($type === 'blocks') { + $from = $ticketId; + $to = $dependsOnId; + } else { // blocked_by + $from = $dependsOnId; + $to = $ticketId; + } + + // Adding edge from->to creates a cycle iff a path to ->* from already exists. $visited = []; - return $this->hasDependencyPath($dependsOnId, $ticketId, $visited, 0); + return $this->hasDependencyPath($to, $from, $visited, 0); } /** @@ -236,15 +247,22 @@ class DependencyModel $visited[] = $source; - $sql = "SELECT depends_on_id FROM ticket_dependencies - WHERE ticket_id = ? AND dependency_type IN ('blocks', 'blocked_by')"; + // Walk the unified precedence graph forward from $source. Both directions + // of expression contribute an outgoing edge "$source must finish before X": + // blocks rows where ticket_id=$source -> X = depends_on_id + // blocked_by rows where depends_on_id=$source -> X = ticket_id + $sql = "SELECT depends_on_id AS next_id FROM ticket_dependencies + WHERE ticket_id = ? AND dependency_type = 'blocks' + UNION + SELECT ticket_id AS next_id FROM ticket_dependencies + WHERE depends_on_id = ? AND dependency_type = 'blocked_by'"; $stmt = $this->conn->prepare($sql); - $stmt->bind_param("s", $source); + $stmt->bind_param("ss", $source, $source); $stmt->execute(); $result = $stmt->get_result(); while ($row = $result->fetch_assoc()) { - if ($this->hasDependencyPath($row['depends_on_id'], $target, $visited, $depth + 1)) { + if ($this->hasDependencyPath($row['next_id'], $target, $visited, $depth + 1)) { $stmt->close(); return true; }