a bunch of smaller fixes suggested by coderabbit

This commit is contained in:
Boy132 2025-11-06 11:43:40 +01:00
parent 805333e9dc
commit d2e2346806
4 changed files with 58 additions and 40 deletions

View File

@ -4,17 +4,24 @@ namespace App\Console\Commands\Plugin;
use App\Facades\Plugins;
use App\Models\Plugin;
use App\Traits\Commands\RequiresDatabaseMigrations;
use Exception;
use Illuminate\Console\Command;
class ComposerPluginsCommand extends Command
{
use RequiresDatabaseMigrations;
protected $signature = 'p:plugin:composer';
protected $description = 'Runs "composer require" on all installed plugins.';
public function handle(): void
{
if (!$this->hasCompletedMigrations()) {
return;
}
$plugins = Plugin::all();
foreach ($plugins as $plugin) {
if (!$plugin->shouldLoad()) {

View File

@ -29,7 +29,7 @@ class MakePluginCommand extends Command
public function handle(): void
{
$name = $this->option('name') ?? $this->ask('Name');
$name = Str::ascii($this->option('name') ?? $this->ask('Name'));
$id = Str::slug($name);
if ($this->filesystem->exists(plugin_path($id))) {
@ -38,10 +38,10 @@ class MakePluginCommand extends Command
return;
}
$author = $this->option('author') ?? $this->ask('author', cache('plugin.author'));
$author = Str::ascii($this->option('author') ?? $this->ask('Author', cache('plugin.author')));
cache()->forever('plugin.author', $author);
$namespace = $author . '\\' . Str::studly($name);
$namespace = Str::studly($author) . '\\' . Str::studly($name);
$class = Str::studly($name . 'Plugin');
if (class_exists('\\' . $namespace . '\\' . $class)) {

View File

@ -134,7 +134,7 @@ class Plugin extends Model implements HasPluginSettings
'namespace' => $data['namespace'],
'class' => $data['class'],
'panels' => $panels,
'panel_version' => Arr::get($data, 'update_url', null),
'panel_version' => Arr::get($data, 'panel_version', null),
'composer_packages' => $composerPackages,
'status' => Arr::get($data, 'meta.status', PluginStatus::NotInstalled->value),
@ -232,9 +232,9 @@ class Plugin extends Model implements HasPluginSettings
public function isCompatible(): bool
{
$panelVersion = config('app.version', 'canary');
$currentPanelVersion = config('app.version', 'canary');
return !$this->panel_version || $panelVersion === 'canary' || version_compare($this->panel_version, $panelVersion, $this->isPanelVersionStrict() ? '=' : '>=');
return !$this->panel_version || $currentPanelVersion === 'canary' || version_compare($currentPanelVersion, $this->panel_version, $this->isPanelVersionStrict() ? '=' : '>=');
}
public function isPanelVersionStrict(): bool
@ -307,7 +307,7 @@ class Plugin extends Model implements HasPluginSettings
$updateData = $this->getUpdateData();
if ($updateData) {
if (array_key_exists($panelVersion, $updateData)) {
return $updateData['panelVersion']['download_url'];
return $updateData[$panelVersion]['download_url'];
}
if (array_key_exists('*', $updateData)) {

View File

@ -15,6 +15,7 @@ use Illuminate\Http\UploadedFile;
use Illuminate\Support\Composer;
use Illuminate\Support\Facades\Artisan;
use Illuminate\Support\Facades\File;
use Illuminate\Support\Facades\Http;
use Illuminate\Support\Facades\Process;
use Illuminate\Support\ServiceProvider;
use Spatie\TemporaryDirectory\TemporaryDirectory;
@ -73,8 +74,9 @@ class PluginService
}
// Autoload src directory
if (!array_key_exists($plugin->namespace, $classLoader->getClassMap())) {
$classLoader->setPsr4($plugin->namespace . '\\', plugin_path($plugin->id, 'src/'));
$namespace = $plugin->namespace . '\\';
if (!array_key_exists($namespace, $classLoader->getPrefixesPsr4())) {
$classLoader->setPsr4($namespace . '\\', plugin_path($plugin->id, 'src/'));
}
// Register service providers
@ -113,13 +115,7 @@ class PluginService
});
}
} catch (Exception $exception) {
if ($this->isDevModeActive()) {
throw ($exception);
}
report($exception);
$this->setStatus($plugin, PluginStatus::Errored, $exception->getMessage());
$this->handlePluginException($plugin, $exception);
}
}
}
@ -150,13 +146,7 @@ class PluginService
$this->enablePlugin($plugin);
}
} catch (Exception $exception) {
if ($this->isDevModeActive()) {
throw ($exception);
}
report($exception);
$this->setStatus($plugin, PluginStatus::Errored, $exception->getMessage());
$this->handlePluginException($plugin, $exception);
}
}
}
@ -193,12 +183,12 @@ class PluginService
public function buildAssets(): bool
{
try {
$result = Process::run('yarn install');
$result = Process::path(base_path())->timeout(300)->run('yarn install');
if ($result->failed()) {
throw new Exception('Could not install dependencies: ' . $result->errorOutput());
}
$result = Process::run('yarn build');
$result = Process::path(base_path())->timeout(600)->run('yarn build');
if ($result->failed()) {
throw new Exception('Could not build assets: ' . $result->errorOutput());
}
@ -232,13 +222,7 @@ class PluginService
}
}
} catch (Exception $exception) {
if ($this->isDevModeActive()) {
throw ($exception);
}
report($exception);
$this->setStatus($plugin, PluginStatus::Errored, $exception->getMessage());
$this->handlePluginException($plugin, $exception);
}
}
@ -251,18 +235,17 @@ class PluginService
cache()->forget("plugins.$plugin->id.update");
} catch (Exception $exception) {
if ($this->isDevModeActive()) {
throw ($exception);
}
report($exception);
$this->setStatus($plugin, PluginStatus::Errored, $exception->getMessage());
$this->handlePluginException($plugin, $exception);
}
}
public function downloadPluginFromFile(UploadedFile $file, bool $cleanDownload = false): void
{
// Validate file size to prevent zip bombs
if ($file->getSize() > 100 * 1024 * 1024) {
throw new Exception('Zip file too large. (max 100 MB)');
}
$zip = new ZipArchive();
if (!$zip->open($file->getPathname())) {
@ -271,6 +254,15 @@ class PluginService
$pluginName = str($file->getClientOriginalName())->before('.zip')->toString();
// Validate zip contents before extraction
for ($i = 0; $i < $zip->numFiles; $i++) {
$filename = $zip->getNameIndex($i);
if (str_contains($filename, '..') || str_starts_with($filename, '/')) {
$zip->close();
throw new Exception('Zip file contains invalid path traversal sequences.');
}
}
if ($cleanDownload) {
File::deleteDirectory(plugin_path($pluginName));
}
@ -278,6 +270,7 @@ class PluginService
$extractPath = $zip->locateName($pluginName) ? base_path('plugins') : plugin_path($pluginName);
if (!$zip->extractTo($extractPath)) {
$zip->close();
throw new Exception('Could not extract zip file.');
}
@ -290,7 +283,14 @@ class PluginService
$tmpDir = TemporaryDirectory::make()->deleteWhenDestroyed();
$tmpPath = $tmpDir->path($info['basename']);
if (!file_put_contents($tmpPath, file_get_contents($url))) {
$content = Http::timeout(60)->connectTimeout(5)->throw()->get($url)->body();
// Validate file size to prevent zip bombs
if (strlen($content) > 100 * 1024 * 1024) {
throw new InvalidFileUploadException('Zip file too large. (100 MB)');
}
if (!file_put_contents($tmpPath, $content)) {
throw new InvalidFileUploadException('Could not write temporary file.');
}
@ -372,4 +372,15 @@ class PluginService
{
return config('panel.plugin.dev_mode', false);
}
private function handlePluginException(string|Plugin $plugin, Exception $exception): void
{
if ($this->isDevModeActive()) {
throw ($exception);
}
report($exception);
$this->setStatus($plugin, PluginStatus::Errored, $exception->getMessage());
}
}