fix: security & correctness batch (#99, #100, #101, #102, #106)
Universal: Pre-Release / Build Pre-Release (${{ inputs.stability || github.ref_name }}) (push) Successful in 16s
Universal: Pre-Release / Build Pre-Release (${{ inputs.stability || github.ref_name }}) (push) Successful in 16s
#99 — AI AJAX endpoint hardening: - require core.edit/core.create on com_content before generating (was reachable by any authenticated back-end user → paid-credit abuse) - callAiApi: 20s timeout + HTTP status check (throw on non-200) instead of silently returning an empty string #100 — Sitemap information disclosure + robustness: - filter to public (guest) view levels so registered/special-access articles are never written into the public sitemap - atomic write (temp file + rename) so concurrent saves can't expose a half-written sitemap.xml - (throttling + SEF URLs remain follow-ups, noted on the issue) #101 — Expose newer columns in CSV + API: - og_video, event_data, recipe_data, custom_schema added to CSV export/import (appended, so existing CSVs still import) and to the REST API field whitelist - import validates JSON fields as arrays/objects and og_video as http(s) (prevents re-introducing the #97 scalar-JSON-LD crash via import) #102 — Forward-compat (complete): - Factory::getLanguage() -> getApplication()->getLanguage() (4 sites) - Joomla\CMS\Filesystem\File/Folder -> Joomla\Filesystem\* (ImageHelper, ImageGenerator) #106 — partial: loadArticle() now caches null misses (array_key_exists), getArticleDate() skips 0000-00-00 dates. Batch-JS halt deferred — the offset=0 design re-fetches failed rows, so the created>0 guard prevents an infinite loop; a safe fix needs cursor-based pagination in BatchController.
This commit is contained in:
@@ -31,10 +31,14 @@ class JsonapiView extends BaseApiView
|
||||
'og_description',
|
||||
'og_image',
|
||||
'og_type',
|
||||
'og_video',
|
||||
'seo_title',
|
||||
'meta_description',
|
||||
'robots',
|
||||
'canonical_url',
|
||||
'event_data',
|
||||
'recipe_data',
|
||||
'custom_schema',
|
||||
'language',
|
||||
'published',
|
||||
'created',
|
||||
@@ -54,10 +58,14 @@ class JsonapiView extends BaseApiView
|
||||
'og_description',
|
||||
'og_image',
|
||||
'og_type',
|
||||
'og_video',
|
||||
'seo_title',
|
||||
'meta_description',
|
||||
'robots',
|
||||
'canonical_url',
|
||||
'event_data',
|
||||
'recipe_data',
|
||||
'custom_schema',
|
||||
'language',
|
||||
'published',
|
||||
'created',
|
||||
|
||||
@@ -60,6 +60,10 @@ class ImportExportController extends BaseController
|
||||
$db->quoteName('t.robots'),
|
||||
$db->quoteName('t.canonical_url'),
|
||||
$db->quoteName('t.language'),
|
||||
$db->quoteName('t.og_video'),
|
||||
$db->quoteName('t.event_data'),
|
||||
$db->quoteName('t.recipe_data'),
|
||||
$db->quoteName('t.custom_schema'),
|
||||
])
|
||||
->from($db->quoteName('#__mokoog_tags', 't'))
|
||||
->leftJoin(
|
||||
@@ -84,7 +88,7 @@ class ImportExportController extends BaseController
|
||||
'content_type', 'content_id', 'article_title',
|
||||
'og_title', 'og_description', 'og_image', 'og_type',
|
||||
'seo_title', 'meta_description', 'robots', 'canonical_url',
|
||||
'language',
|
||||
'language', 'og_video', 'event_data', 'recipe_data', 'custom_schema',
|
||||
]);
|
||||
|
||||
foreach ($rows as $row) {
|
||||
@@ -187,6 +191,10 @@ class ImportExportController extends BaseController
|
||||
$robots = trim($row[9] ?? '');
|
||||
$canonicalUrl = trim($row[10] ?? '');
|
||||
$language = trim($row[11] ?? '*');
|
||||
$ogVideo = $this->sanitizeUrl($row[12] ?? '');
|
||||
$eventData = $this->validateJsonField($row[13] ?? '');
|
||||
$recipeData = $this->validateJsonField($row[14] ?? '');
|
||||
$customSchema = $this->validateJsonField($row[15] ?? '');
|
||||
|
||||
// Validate language tag format (e.g., 'en-GB', '*')
|
||||
if ($language !== '*' && !preg_match('/^[a-z]{2,3}-[A-Z]{2}$/', $language)) {
|
||||
@@ -229,6 +237,10 @@ class ImportExportController extends BaseController
|
||||
'robots' => $robots,
|
||||
'canonical_url' => $canonicalUrl,
|
||||
'language' => $language,
|
||||
'og_video' => $ogVideo,
|
||||
'event_data' => $eventData,
|
||||
'recipe_data' => $recipeData,
|
||||
'custom_schema' => $customSchema,
|
||||
'published' => 1,
|
||||
'modified' => $now,
|
||||
];
|
||||
@@ -252,4 +264,45 @@ class ImportExportController extends BaseController
|
||||
);
|
||||
$app->redirect('index.php?option=com_mokoog&view=tags');
|
||||
}
|
||||
|
||||
/**
|
||||
* Validate a JSON field — returns trimmed JSON only if it is an object/array.
|
||||
*
|
||||
* Scalars and invalid JSON are dropped to '' so an import can never inject a
|
||||
* payload that crashes the frontend JSON-LD renderer.
|
||||
*
|
||||
* @param string $value Raw CSV cell value
|
||||
*
|
||||
* @return string
|
||||
*/
|
||||
private function validateJsonField(string $value): string
|
||||
{
|
||||
$value = trim($value);
|
||||
|
||||
if ($value === '' || !\is_array(json_decode($value, true))) {
|
||||
return '';
|
||||
}
|
||||
|
||||
return $value;
|
||||
}
|
||||
|
||||
/**
|
||||
* Sanitize a URL to only allow http/https schemes.
|
||||
*
|
||||
* @param string $url Raw CSV cell value
|
||||
*
|
||||
* @return string Sanitized URL or empty string
|
||||
*/
|
||||
private function sanitizeUrl(string $url): string
|
||||
{
|
||||
$url = trim($url);
|
||||
|
||||
if ($url === '') {
|
||||
return '';
|
||||
}
|
||||
|
||||
$scheme = strtolower((string) parse_url($url, PHP_URL_SCHEME));
|
||||
|
||||
return \in_array($scheme, ['http', 'https'], true) ? $url : '';
|
||||
}
|
||||
}
|
||||
|
||||
@@ -139,7 +139,7 @@ final class MokoOG extends CMSPlugin implements SubscriberInterface
|
||||
}
|
||||
|
||||
// og:locale from current language
|
||||
$langTag = Factory::getLanguage()->getTag();
|
||||
$langTag = $this->getApplication()->getLanguage()->getTag();
|
||||
$ogLocale = str_replace('-', '_', $langTag);
|
||||
$doc->setMetaData('og:locale', $ogLocale, 'property');
|
||||
|
||||
@@ -476,7 +476,7 @@ final class MokoOG extends CMSPlugin implements SubscriberInterface
|
||||
->where($db->quoteName('content_type') . ' = ' . $db->quote($option))
|
||||
->where($db->quoteName('content_id') . ' = ' . (int) $id)
|
||||
->where($db->quoteName('published') . ' = 1')
|
||||
->where('(' . $db->quoteName('language') . ' = ' . $db->quote(Factory::getLanguage()->getTag())
|
||||
->where('(' . $db->quoteName('language') . ' = ' . $db->quote($this->getApplication()->getLanguage()->getTag())
|
||||
. ' OR ' . $db->quoteName('language') . ' = ' . $db->quote('*') . ')')
|
||||
->order('CASE WHEN ' . $db->quoteName('language') . ' = ' . $db->quote('*') . ' THEN 1 ELSE 0 END ASC');
|
||||
|
||||
@@ -496,7 +496,7 @@ final class MokoOG extends CMSPlugin implements SubscriberInterface
|
||||
private function loadOgDataByType(string $contentType, int $contentId): ?object
|
||||
{
|
||||
$db = Factory::getContainer()->get(\Joomla\Database\DatabaseInterface::class);
|
||||
$lang = Factory::getLanguage()->getTag();
|
||||
$lang = $this->getApplication()->getLanguage()->getTag();
|
||||
|
||||
$query = $db->getQuery(true)
|
||||
->select('*')
|
||||
@@ -523,7 +523,7 @@ final class MokoOG extends CMSPlugin implements SubscriberInterface
|
||||
private function loadOgDataByMenu(int $menuId): ?object
|
||||
{
|
||||
$db = Factory::getContainer()->get(\Joomla\Database\DatabaseInterface::class);
|
||||
$lang = Factory::getLanguage()->getTag();
|
||||
$lang = $this->getApplication()->getLanguage()->getTag();
|
||||
|
||||
$query = $db->getQuery(true)
|
||||
->select('*')
|
||||
@@ -672,7 +672,9 @@ final class MokoOG extends CMSPlugin implements SubscriberInterface
|
||||
{
|
||||
static $cache = [];
|
||||
|
||||
if (isset($cache[$id])) {
|
||||
// array_key_exists (not isset) so a negative lookup (null) is also cached
|
||||
// and not re-queried on every call within the request.
|
||||
if (\array_key_exists($id, $cache)) {
|
||||
return $cache[$id];
|
||||
}
|
||||
|
||||
@@ -704,8 +706,15 @@ final class MokoOG extends CMSPlugin implements SubscriberInterface
|
||||
private function getArticleDate(int $id, string $field): string
|
||||
{
|
||||
$article = $this->loadArticle($id);
|
||||
$value = $article->$field ?? '';
|
||||
|
||||
return $article->$field ?? '';
|
||||
// Skip zero/empty dates — emitting "0000-00-00 00:00:00" as
|
||||
// article:published_time/modified_time produces invalid metadata.
|
||||
if ($value === '' || str_starts_with($value, '0000-00-00')) {
|
||||
return '';
|
||||
}
|
||||
|
||||
return $value;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -860,6 +869,14 @@ final class MokoOG extends CMSPlugin implements SubscriberInterface
|
||||
return;
|
||||
}
|
||||
|
||||
// Require article-edit capability — this triggers outbound paid AI calls,
|
||||
// so it must not be reachable by every authenticated back-end user.
|
||||
if (!$app->getIdentity()->authorise('core.edit', 'com_content')
|
||||
&& !$app->getIdentity()->authorise('core.create', 'com_content')) {
|
||||
$event->setArgument('result', ['Forbidden — insufficient permissions']);
|
||||
return;
|
||||
}
|
||||
|
||||
if (!$this->params->get('ai_enabled', 0)) {
|
||||
$event->setArgument('result', ['AI generation is not enabled']);
|
||||
return;
|
||||
@@ -904,6 +921,9 @@ final class MokoOG extends CMSPlugin implements SubscriberInterface
|
||||
{
|
||||
$http = \Joomla\CMS\Http\HttpFactory::getHttp();
|
||||
|
||||
// Cap how long a hung provider can block the admin request.
|
||||
$timeout = 20;
|
||||
|
||||
if ($provider === 'claude') {
|
||||
$response = $http->post(
|
||||
'https://api.anthropic.com/v1/messages',
|
||||
@@ -916,9 +936,14 @@ final class MokoOG extends CMSPlugin implements SubscriberInterface
|
||||
'Content-Type' => 'application/json',
|
||||
'x-api-key' => $apiKey,
|
||||
'anthropic-version' => '2023-06-01',
|
||||
]
|
||||
],
|
||||
$timeout
|
||||
);
|
||||
|
||||
if ((int) $response->code !== 200) {
|
||||
throw new \RuntimeException('Claude API request failed (HTTP ' . (int) $response->code . ')');
|
||||
}
|
||||
|
||||
$data = json_decode($response->body, true);
|
||||
|
||||
return trim($data['content'][0]['text'] ?? '');
|
||||
@@ -934,9 +959,14 @@ final class MokoOG extends CMSPlugin implements SubscriberInterface
|
||||
[
|
||||
'Content-Type' => 'application/json',
|
||||
'Authorization' => 'Bearer ' . $apiKey,
|
||||
]
|
||||
],
|
||||
$timeout
|
||||
);
|
||||
|
||||
if ((int) $response->code !== 200) {
|
||||
throw new \RuntimeException('OpenAI API request failed (HTTP ' . (int) $response->code . ')');
|
||||
}
|
||||
|
||||
$data = json_decode($response->body, true);
|
||||
|
||||
return trim($data['choices'][0]['message']['content'] ?? '');
|
||||
|
||||
@@ -12,7 +12,7 @@ namespace Joomla\Plugin\System\MokoOG\Helper;
|
||||
|
||||
defined('_JEXEC') or die;
|
||||
|
||||
use Joomla\CMS\Filesystem\Folder;
|
||||
use Joomla\Filesystem\Folder;
|
||||
use Joomla\CMS\Log\Log;
|
||||
|
||||
class ImageGenerator
|
||||
|
||||
@@ -12,8 +12,8 @@ namespace Joomla\Plugin\System\MokoOG\Helper;
|
||||
|
||||
defined('_JEXEC') or die;
|
||||
|
||||
use Joomla\CMS\Filesystem\File;
|
||||
use Joomla\CMS\Filesystem\Folder;
|
||||
use Joomla\Filesystem\File;
|
||||
use Joomla\Filesystem\Folder;
|
||||
use Joomla\CMS\Log\Log;
|
||||
|
||||
class ImageHelper
|
||||
|
||||
@@ -37,12 +37,20 @@ class SitemapBuilder
|
||||
|
||||
$db = Factory::getContainer()->get(\Joomla\Database\DatabaseInterface::class);
|
||||
|
||||
// Only include content the public (guest, user id 0) can view — never
|
||||
// leak registered/special-access articles into the public sitemap.
|
||||
$publicLevels = array_map('intval', \Joomla\CMS\Access\Access::getAuthorisedViewLevels(0));
|
||||
|
||||
// Get all published articles
|
||||
$query = $db->getQuery(true)
|
||||
->select($db->quoteName(['a.id', 'a.alias', 'a.catid', 'a.modified', 'a.language']))
|
||||
->from($db->quoteName('#__content', 'a'))
|
||||
->where($db->quoteName('a.state') . ' = 1');
|
||||
|
||||
if (!empty($publicLevels)) {
|
||||
$query->where($db->quoteName('a.access') . ' IN (' . implode(',', $publicLevels) . ')');
|
||||
}
|
||||
|
||||
$db->setQuery($query);
|
||||
$articles = $db->loadObjectList();
|
||||
|
||||
@@ -104,7 +112,19 @@ class SitemapBuilder
|
||||
public static function writeToFile(string $xml): bool
|
||||
{
|
||||
$path = JPATH_ROOT . '/sitemap.xml';
|
||||
$tmp = $path . '.' . uniqid('tmp', true);
|
||||
|
||||
return (bool) file_put_contents($path, $xml);
|
||||
if (file_put_contents($tmp, $xml) === false) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Atomic replace so concurrent saves never expose a half-written sitemap.
|
||||
if (!@rename($tmp, $path)) {
|
||||
@unlink($tmp);
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user