From c5f7a01e1d1a771316c16d5d476c0b4a506df95e Mon Sep 17 00:00:00 2001 From: Jared Vititoe Date: Fri, 10 Jul 2026 11:17:07 -0400 Subject: [PATCH] Fix helpers/config: timezone, comment leak, silent misconfig, cache perms - 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 --- config/config.php | 3 +++ helpers/CacheHelper.php | 16 ++++++++++++++-- helpers/Database.php | 26 ++++++++++++++++++++++++++ helpers/NotificationHelper.php | 30 ++++++++++++++++++++++++++---- 4 files changed, 69 insertions(+), 6 deletions(-) diff --git a/config/config.php b/config/config.php index e25b5a9..3ffb242 100644 --- a/config/config.php +++ b/config/config.php @@ -6,6 +6,9 @@ if (!file_exists($envFile)) { die('Configuration error: .env file not found. Copy .env.example to .env and configure your database settings.'); } $envVars = parse_ini_file($envFile, false, INI_SCANNER_TYPED); +if (!is_array($envVars)) { + die('Configuration error: .env file could not be parsed. Check for unquoted special characters (e.g. #, ;, =, or quotes) in values and wrap affected values in double quotes.'); +} // Strip quotes from values if present (parse_ini_file may include them) if ($envVars) { diff --git a/helpers/CacheHelper.php b/helpers/CacheHelper.php index 0e5f2e5..40c1c83 100644 --- a/helpers/CacheHelper.php +++ b/helpers/CacheHelper.php @@ -21,7 +21,13 @@ class CacheHelper if (self::$cacheDir === null) { self::$cacheDir = sys_get_temp_dir() . '/tinker_tickets_cache'; if (!is_dir(self::$cacheDir)) { - mkdir(self::$cacheDir, 0755, true); + // 0700: only the app user may read cached data or create files. + // mkdir mode is masked by umask, so chmod to enforce it. + mkdir(self::$cacheDir, 0700, true); + @chmod(self::$cacheDir, 0700); + } elseif (!function_exists('posix_geteuid') || fileowner(self::$cacheDir) === posix_geteuid()) { + // Existing dir we own: harden a previously world-readable dir. + @chmod(self::$cacheDir, 0700); } } return self::$cacheDir; @@ -106,7 +112,13 @@ class CacheHelper // Store in file cache $filePath = self::getCacheDir() . '/' . $key . '.json'; - return @file_put_contents($filePath, json_encode($cached), LOCK_EX) !== false; + $written = @file_put_contents($filePath, json_encode($cached), LOCK_EX) !== false; + if ($written) { + // 0600: cache may feed security-relevant reads; keep it non-readable + // to other local users and non-poisonable by pre-created files. + @chmod($filePath, 0600); + } + return $written; } /** diff --git a/helpers/Database.php b/helpers/Database.php index 7b75de9..b6d2f7a 100644 --- a/helpers/Database.php +++ b/helpers/Database.php @@ -57,6 +57,32 @@ class Database // Set charset to utf8mb4 for proper Unicode support $conn->set_charset('utf8mb4'); + // Pin the MySQL session time zone to the app's configured zone so that + // NOW()/CURRENT_TIMESTAMP and PHP agree on wall-clock time regardless of + // the DB server's SYSTEM tz. Prefer the named zone (requires the + // mysql.time_zone_* tables); if that isn't available, fall back to the + // fixed numeric offset PHP computes for the same zone. Best-effort: a + // failure here must never fatal the connection. + $tz = $GLOBALS['config']['TIMEZONE'] ?? 'UTC'; + try { + $escaped = $conn->real_escape_string($tz); + try { + // mysqli throws (does not return false) on failure under the + // default PHP 8.1+ report mode, so catch it rather than testing + // the return value. + $conn->query("SET time_zone = '{$escaped}'"); + } catch (\Throwable $inner) { + // Named zone unavailable (mysql.time_zone_* not populated) — fall + // back to a fixed numeric offset so PHP and MySQL still agree on + // wall-clock time regardless of the DB server's SYSTEM tz. + $offset = (new DateTime('now', new DateTimeZone($tz)))->format('P'); + $escapedOffset = $conn->real_escape_string($offset); + $conn->query("SET time_zone = '{$escapedOffset}'"); + } + } catch (\Throwable $e) { + error_log('Database: failed to set session time_zone: ' . $e->getMessage()); + } + return $conn; } diff --git a/helpers/NotificationHelper.php b/helpers/NotificationHelper.php index 7bb1304..1afceac 100644 --- a/helpers/NotificationHelper.php +++ b/helpers/NotificationHelper.php @@ -96,21 +96,31 @@ class NotificationHelper * @param string $commentText Plain text (first 200 chars will be sent) * @param string|null $authorDisplay Display name of commenter * @param bool $isInternal True if the comment is internal-only + * @param string $visibility Ticket visibility: 'public', 'internal', or + * 'confidential'. For non-public tickets the + * comment text preview is redacted so it is + * never leaked to the shared notify list. */ - public static function sendCommentNotification($ticketId, string $ticketTitle, string $commentText, ?string $authorDisplay = null, bool $isInternal = false): void + public static function sendCommentNotification($ticketId, string $ticketTitle, string $commentText, ?string $authorDisplay = null, bool $isInternal = false, string $visibility = 'public'): void { - // Skip if this is an internal-only comment — only the assignee/admin need to know $notifyUsers = self::notifyUsers(); if (empty($notifyUsers)) { return; } + // The shared notify list may include users without access to non-public + // tickets, so never post the comment body for internal/confidential + // tickets — only that activity occurred. + $preview = $visibility === 'public' + ? mb_strimwidth($commentText, 0, 200, '…') + : null; + self::fire([ 'event' => 'comment_added', 'ticket_id' => $ticketId, 'title' => $ticketTitle, 'author' => $authorDisplay, - 'preview' => mb_strimwidth($commentText, 0, 200, '…'), + 'preview' => $preview, 'is_internal' => $isInternal, 'url' => UrlHelper::ticketUrl($ticketId), 'notify_users' => $notifyUsers, @@ -155,8 +165,14 @@ class NotificationHelper * @param string $event One of: status_changed, comment_added, assigned * @param array $extraData Merged into the payload (old_status/new_status, author, etc.) * @param int|null $excludeUserId Don't notify the actor themselves + * @param string $visibility Ticket visibility: 'public', 'internal', or + * 'confidential'. notify_users includes the + * shared list, which may contain users without + * access to non-public tickets, so any comment + * body preview in $extraData is redacted for + * non-public tickets. */ - public static function notifyWatchers(\mysqli $conn, $ticketId, string $ticketTitle, string $event, array $extraData = [], ?int $excludeUserId = null): void + public static function notifyWatchers(\mysqli $conn, $ticketId, string $ticketTitle, string $event, array $extraData = [], ?int $excludeUserId = null, string $visibility = 'public'): void { $webhookUrl = $GLOBALS['config']['MATRIX_WEBHOOK_URL'] ?? null; $domain = $GLOBALS['config']['MATRIX_DOMAIN'] ?? null; @@ -164,6 +180,12 @@ class NotificationHelper return; } + // Don't leak comment/body content to the shared notify list for + // non-public tickets — keep only the fact that activity occurred. + if ($visibility !== 'public' && isset($extraData['preview'])) { + $extraData['preview'] = null; + } + // Fetch watcher usernames, excluding the actor so they don't notify // themselves. Notifications are best-effort: if the watchers table is // absent or the query fails, skip silently rather than fataling the