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 <noreply@anthropic.com>
This commit is contained in:
@@ -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.');
|
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);
|
$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)
|
// Strip quotes from values if present (parse_ini_file may include them)
|
||||||
if ($envVars) {
|
if ($envVars) {
|
||||||
|
|||||||
+14
-2
@@ -21,7 +21,13 @@ class CacheHelper
|
|||||||
if (self::$cacheDir === null) {
|
if (self::$cacheDir === null) {
|
||||||
self::$cacheDir = sys_get_temp_dir() . '/tinker_tickets_cache';
|
self::$cacheDir = sys_get_temp_dir() . '/tinker_tickets_cache';
|
||||||
if (!is_dir(self::$cacheDir)) {
|
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;
|
return self::$cacheDir;
|
||||||
@@ -106,7 +112,13 @@ class CacheHelper
|
|||||||
|
|
||||||
// Store in file cache
|
// Store in file cache
|
||||||
$filePath = self::getCacheDir() . '/' . $key . '.json';
|
$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;
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
@@ -57,6 +57,32 @@ class Database
|
|||||||
// Set charset to utf8mb4 for proper Unicode support
|
// Set charset to utf8mb4 for proper Unicode support
|
||||||
$conn->set_charset('utf8mb4');
|
$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;
|
return $conn;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -96,21 +96,31 @@ class NotificationHelper
|
|||||||
* @param string $commentText Plain text (first 200 chars will be sent)
|
* @param string $commentText Plain text (first 200 chars will be sent)
|
||||||
* @param string|null $authorDisplay Display name of commenter
|
* @param string|null $authorDisplay Display name of commenter
|
||||||
* @param bool $isInternal True if the comment is internal-only
|
* @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();
|
$notifyUsers = self::notifyUsers();
|
||||||
if (empty($notifyUsers)) {
|
if (empty($notifyUsers)) {
|
||||||
return;
|
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([
|
self::fire([
|
||||||
'event' => 'comment_added',
|
'event' => 'comment_added',
|
||||||
'ticket_id' => $ticketId,
|
'ticket_id' => $ticketId,
|
||||||
'title' => $ticketTitle,
|
'title' => $ticketTitle,
|
||||||
'author' => $authorDisplay,
|
'author' => $authorDisplay,
|
||||||
'preview' => mb_strimwidth($commentText, 0, 200, '…'),
|
'preview' => $preview,
|
||||||
'is_internal' => $isInternal,
|
'is_internal' => $isInternal,
|
||||||
'url' => UrlHelper::ticketUrl($ticketId),
|
'url' => UrlHelper::ticketUrl($ticketId),
|
||||||
'notify_users' => $notifyUsers,
|
'notify_users' => $notifyUsers,
|
||||||
@@ -155,8 +165,14 @@ class NotificationHelper
|
|||||||
* @param string $event One of: status_changed, comment_added, assigned
|
* @param string $event One of: status_changed, comment_added, assigned
|
||||||
* @param array $extraData Merged into the payload (old_status/new_status, author, etc.)
|
* @param array $extraData Merged into the payload (old_status/new_status, author, etc.)
|
||||||
* @param int|null $excludeUserId Don't notify the actor themselves
|
* @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;
|
$webhookUrl = $GLOBALS['config']['MATRIX_WEBHOOK_URL'] ?? null;
|
||||||
$domain = $GLOBALS['config']['MATRIX_DOMAIN'] ?? null;
|
$domain = $GLOBALS['config']['MATRIX_DOMAIN'] ?? null;
|
||||||
@@ -164,6 +180,12 @@ class NotificationHelper
|
|||||||
return;
|
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
|
// Fetch watcher usernames, excluding the actor so they don't notify
|
||||||
// themselves. Notifications are best-effort: if the watchers table is
|
// themselves. Notifications are best-effort: if the watchers table is
|
||||||
// absent or the query fails, skip silently rather than fataling the
|
// absent or the query fails, skip silently rather than fataling the
|
||||||
|
|||||||
Reference in New Issue
Block a user