Skip to content

Commit fdc3152

Browse files
Jean-BeruJean-Beru
committed
review
1 parent 2f87eac commit fdc3152

File tree

7 files changed

+100
-53
lines changed

7 files changed

+100
-53
lines changed

‎src/Symfony/Bundle/FrameworkBundle/Resources/config/asset_mapper.php

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,8 @@
1717
use Symfony\Component\AssetMapper\AssetMapperInterface;
1818
use Symfony\Component\AssetMapper\AssetMapperRepository;
1919
use Symfony\Component\AssetMapper\Command\AssetMapperCompileCommand;
20-
use Symfony\Component\AssetMapper\Command\AuditCommand;
20+
use Symfony\Component\AssetMapper\Command\ImportMapAuditCommand;
2121
use Symfony\Component\AssetMapper\Command\DebugAssetMapperCommand;
22-
use Symfony\Component\AssetMapper\Command\ImportMapExportCommand;
2322
use Symfony\Component\AssetMapper\Command\ImportMapInstallCommand;
2423
use Symfony\Component\AssetMapper\Command\ImportMapRemoveCommand;
2524
use Symfony\Component\AssetMapper\Command\ImportMapRequireCommand;
@@ -198,7 +197,8 @@
198197

199198
->set('asset_mapper.importmap.auditor', ImportMapAuditor::class)
200199
->args([
201-
service('asset_mapper.importmap.manager'),
200+
service('asset_mapper.importmap.config_reader'),
201+
service('asset_mapper.importmap.resolver'),
202202
service('http_client'),
203203
])
204204

@@ -219,7 +219,11 @@
219219
->tag('console.command')
220220

221221
->set('asset_mapper.importmap.command.install', ImportMapInstallCommand::class)
222-
->args([service('asset_mapper.importmap.manager')])
223-
->tag('console.command')
222+
->args([service('asset_mapper.importmap.manager')])
223+
->tag('console.command')
224+
225+
->set('asset_mapper.importmap.command.audit', ImportMapAuditCommand::class)
226+
->args([service('asset_mapper.importmap.auditor')])
227+
->tag('console.command')
224228
;
225229
};

‎src/Symfony/Component/AssetMapper/Command/AuditCommand.php renamed to ‎src/Symfony/Component/AssetMapper/Command/ImportMapAuditCommand.php

Lines changed: 29 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
use Symfony\Component\Console\Style\SymfonyStyle;
2222

2323
#[AsCommand(name: 'importmap:audit', description: 'Checks for security vulnerability advisories for dependencies.')]
24-
class AuditCommand extends Command
24+
class ImportMapAuditCommand extends Command
2525
{
2626
private const SEVERITY_COLORS = [
2727
'critical' => 'red',
@@ -98,18 +98,20 @@ private function displayTxt(array $audit): int
9898
return self::SUCCESS;
9999
}
100100

101-
$table = $this->io->createTable();
102-
$table->setHeaders([
103-
'Severity',
104-
'Title',
105-
'Package',
106-
'Version',
107-
'Patched in',
108-
'More info',
109-
]);
110-
$table->addRows($rows);
111-
$table->render();
112-
$this->io->newLine();
101+
if ([] !== $rows) {
102+
$table = $this->io->createTable();
103+
$table->setHeaders([
104+
'Severity',
105+
'Title',
106+
'Package',
107+
'Version',
108+
'Patched in',
109+
'More info',
110+
]);
111+
$table->addRows($rows);
112+
$table->render();
113+
$this->io->newLine();
114+
}
113115

114116
$this->io->text(sprintf('%d package%s found: %d audited / %d skipped',
115117
$packagesCount,
@@ -125,20 +127,22 @@ private function displayTxt(array $audit): int
125127
));
126128
}
127129

128-
$vulnerabilityCount = 0;
129-
$vulnerabilitySummary = [];
130-
foreach ($vulnerabilitiesCount as $severity => $count) {
131-
if (0 === $count) {
132-
continue;
130+
if ([] !== $rows) {
131+
$vulnerabilityCount = 0;
132+
$vulnerabilitySummary = [];
133+
foreach ($vulnerabilitiesCount as $severity => $count) {
134+
if (0 === $count) {
135+
continue;
136+
}
137+
$vulnerabilitySummary[] = sprintf( '%d %s', $count, ucfirst($severity));
138+
$vulnerabilityCount += $count;
133139
}
134-
$vulnerabilitySummary[] = sprintf( '%d %s', $count, ucfirst($severity));
135-
$vulnerabilityCount += $count;
140+
$this->io->text(sprintf('%d vulnerabilit%s found: %s',
141+
$vulnerabilityCount,
142+
1 === $vulnerabilityCount ? 'y' : 'ies',
143+
implode(' / ', $vulnerabilitySummary),
144+
));
136145
}
137-
$this->io->text(sprintf('%d vulnerabilit%s found: %s',
138-
$vulnerabilityCount,
139-
1 === $vulnerabilityCount ? 'y' : 'ies',
140-
implode(' / ', $vulnerabilitySummary),
141-
));
142146

143147
return self::FAILURE;
144148
}

‎src/Symfony/Component/AssetMapper/ImportMap/ImportMapAuditor.php

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
namespace Symfony\Component\AssetMapper\ImportMap;
1313

1414
use Symfony\Component\AssetMapper\Exception\RuntimeException;
15+
use Symfony\Component\AssetMapper\ImportMap\Resolver\PackageResolverInterface;
1516
use Symfony\Component\HttpClient\HttpClient;
1617
use Symfony\Contracts\HttpClient\HttpClientInterface;
1718

@@ -22,7 +23,8 @@ class ImportMapAuditor
2223
private readonly HttpClientInterface $httpClient;
2324

2425
public function __construct(
25-
private readonly ImportMapManager $importMapManager,
26+
private readonly ImportMapConfigReader $configReader,
27+
private readonly PackageResolverInterface $packageResolver,
2628
HttpClientInterface $httpClient = null,
2729
) {
2830
$this->httpClient = $httpClient ?? HttpClient::create();
@@ -33,7 +35,7 @@ public function __construct(
3335
*/
3436
public function audit(): array
3537
{
36-
$entries = $this->importMapManager->loadImportMapEntries();
38+
$entries = $this->configReader->getEntries();
3739

3840
if ([] === $entries) {
3941
return [];
@@ -49,11 +51,13 @@ public function audit(): array
4951
if (null === $entry->url) {
5052
continue;
5153
}
54+
$version = $entry->version ?? $this->packageResolver->getPackageVersion($entry->url);
55+
5256
$installed[$entry->importName] ??= [];
53-
$installed[$entry->importName][] = $entry->version;
57+
$installed[$entry->importName][] = $version;
5458

55-
$packageVersion = $entry->importName.($entry->version ? '@'.$entry->version : '');
56-
$packageAudits[$packageVersion] ??= new ImportMapPackageAudit($entry->importName, $entry->version);
59+
$packageVersion = $entry->importName.($version ? '@'.$version : '');
60+
$packageAudits[$packageVersion] ??= new ImportMapPackageAudit($entry->importName, $version);
5761
$affectsQuery[] = $packageVersion;
5862
}
5963

‎src/Symfony/Component/AssetMapper/ImportMap/ImportMapConfigReader.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ public function getEntries(): ImportMapEntries
3838

3939
$entries = new ImportMapEntries();
4040
foreach ($importMapConfig ?? [] as $importName => $data) {
41-
$validKeys = ['path', 'url', 'downloaded_to', 'type', 'entrypoint'];
41+
$validKeys = ['path', 'url', 'downloaded_to', 'type', 'entrypoint', 'version'];
4242
if ($invalidKeys = array_diff(array_keys($data), $validKeys)) {
4343
throw new \InvalidArgumentException(sprintf('The following keys are not valid for the importmap entry "%s": "%s". Valid keys are: "%s".', $importName, implode('", "', $invalidKeys), implode('", "', $validKeys)));
4444
}
@@ -57,6 +57,7 @@ public function getEntries(): ImportMapEntries
5757
isDownloaded: isset($data['downloaded_to']),
5858
type: $type,
5959
isEntrypoint: $isEntry,
60+
version: $data['version'] ?? null,
6061
));
6162
}
6263

‎src/Symfony/Component/AssetMapper/Tests/ImportMap/ImportMapAuditorTest.php

Lines changed: 43 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,25 +14,31 @@
1414
use PHPUnit\Framework\TestCase;
1515
use Symfony\Component\AssetMapper\Exception\RuntimeException;
1616
use Symfony\Component\AssetMapper\ImportMap\ImportMapAuditor;
17+
use Symfony\Component\AssetMapper\ImportMap\ImportMapConfigReader;
18+
use Symfony\Component\AssetMapper\ImportMap\ImportMapEntries;
1719
use Symfony\Component\AssetMapper\ImportMap\ImportMapEntry;
1820
use Symfony\Component\AssetMapper\ImportMap\ImportMapManager;
1921
use Symfony\Component\AssetMapper\ImportMap\ImportMapPackageAudit;
2022
use Symfony\Component\AssetMapper\ImportMap\ImportMapPackageAuditVulnerability;
23+
use Symfony\Component\AssetMapper\ImportMap\Resolver\PackageResolver;
24+
use Symfony\Component\AssetMapper\ImportMap\Resolver\PackageResolverInterface;
2125
use Symfony\Component\HttpClient\MockHttpClient;
2226
use Symfony\Component\HttpClient\Response\MockResponse;
2327
use Symfony\Contracts\HttpClient\HttpClientInterface;
2428

2529
class ImportMapAuditorTest extends TestCase
2630
{
27-
private ImportMapManager $importMapManager;
31+
private ImportMapConfigReader $importMapConfigReader;
32+
private PackageResolverInterface $packageResolver;
2833
private HttpClientInterface $httpClient;
2934
private ImportMapAuditor $importMapAuditor;
3035

3136
protected function setUp(): void
3237
{
33-
$this->importMapManager = $this->createMock(ImportMapManager::class);
38+
$this->importMapConfigReader = $this->createMock(ImportMapConfigReader::class);
39+
$this->packageResolver = $this->createMock(PackageResolverInterface::class);
3440
$this->httpClient = new MockHttpClient();
35-
$this->importMapAuditor = new ImportMapAuditor($this->importMapManager, $this->httpClient);
41+
$this->importMapAuditor = new ImportMapAuditor($this->importMapConfigReader, $this->packageResolver, $this->httpClient);
3642
}
3743

3844
public function testAudit()
@@ -63,7 +69,7 @@ public function testAudit()
6369
],
6470
],
6571
])));
66-
$this->importMapManager->method('loadImportMapEntries')->willReturn([
72+
$this->importMapConfigReader->method('getEntries')->willReturn(new ImportMapEntries([
6773
'@hotwired/stimulus' => new ImportMapEntry(
6874
importName: '@hotwired/stimulus',
6975
url: 'https://unpkg.com/@hotwired/stimulus@3.2.1/dist/stimulus.js',
@@ -79,7 +85,7 @@ public function testAudit()
7985
url: 'https://ga.jspm.io/npm:lodash@4.17.21/lodash.js',
8086
version: '4.17.21',
8187
),
82-
]);
88+
]));
8389

8490
$audit = $this->importMapAuditor->audit();
8591

@@ -119,13 +125,13 @@ public function testAuditWithVersionRange(bool $expectMatch, string $version, ?s
119125
],
120126
],
121127
])));
122-
$this->importMapManager->method('loadImportMapEntries')->willReturn([
128+
$this->importMapConfigReader->method('getEntries')->willReturn(new ImportMapEntries([
123129
'json5' => new ImportMapEntry(
124130
importName: 'json5',
125131
url: "https://cdn.jsdelivr.net/npm/json5@$version/+esm",
126132
version: $version,
127133
),
128-
]);
134+
]));
129135

130136
$audit = $this->importMapAuditor->audit();
131137

@@ -145,17 +151,43 @@ public function provideAuditWithVersionRange(): iterable
145151
yield [false, '1.2.0', '> 1.0.0, < 1.2.0'];
146152
}
147153

148-
public function testAuditError()
154+
public function testAuditWithVersionResolving()
149155
{
156+
$this->httpClient->setResponseFactory(new MockResponse('[]'));
157+
$this->importMapConfigReader->method('getEntries')->willReturn(new ImportMapEntries([
158+
'@hotwired/stimulus' => new ImportMapEntry(
159+
importName: '@hotwired/stimulus',
160+
url: 'https://unpkg.com/@hotwired/stimulus/dist/stimulus.js',
161+
version: '3.2.1',
162+
),
163+
'json5' => new ImportMapEntry(
164+
importName: 'json5',
165+
url: 'https://cdn.jsdelivr.net/npm/json5/+esm',
166+
),
167+
'lodash' => new ImportMapEntry(
168+
importName: 'lodash',
169+
url: 'https://ga.jspm.io/npm:lodash@4.17.21/lodash.js',
170+
),
171+
]));
172+
$this->packageResolver->method('getPackageVersion')->willReturn('1.2.3');
150173

174+
$audit = $this->importMapAuditor->audit();
175+
176+
$this->assertSame('3.2.1', $audit[0]->version);
177+
$this->assertSame('1.2.3', $audit[1]->version);
178+
$this->assertSame('1.2.3', $audit[2]->version);
179+
}
180+
181+
public function testAuditError()
182+
{
151183
$this->httpClient->setResponseFactory(new MockResponse('Server error', ['http_code' => 500]));
152-
$this->importMapManager->method('loadImportMapEntries')->willReturn([
184+
$this->importMapConfigReader->method('getEntries')->willReturn(new ImportMapEntries([
153185
'json5' => new ImportMapEntry(
154186
importName: 'json5',
155-
url: "https://cdn.jsdelivr.net/npm/json5@1.0.0/+esm",
187+
url: 'https://cdn.jsdelivr.net/npm/json5@1.0.0/+esm',
156188
version: '1.0.0',
157189
),
158-
]);
190+
]));
159191

160192
$this->expectException(RuntimeException::class);
161193
$this->expectExceptionMessage('Error 500 auditing packages. Response: Server error');

‎src/Symfony/Component/AssetMapper/Tests/ImportMap/Resolver/JsDelivrEsmResolverTest.php

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -396,7 +396,7 @@ public static function provideImportRegex(): iterable
396396
/**
397397
* @dataProvider provideGetPackageVersion
398398
*/
399-
public function testGetPackageVersion(string $url, string $expected)
399+
public function testGetPackageVersion(string $url, ?string $expected)
400400
{
401401
$resolver = new JsDelivrEsmResolver();
402402

@@ -405,7 +405,8 @@ public function testGetPackageVersion(string $url, string $expected)
405405

406406
public static function provideGetPackageVersion(): iterable
407407
{
408-
yield 'package_name' => ['https://cdn.jsdelivr.net/npm/lodash.js@1.2.3/+esm', '1.2.3'];
409-
yield 'package_name_with_a_slash' => ['https://cdn.jsdelivr.net/npm/@popperjs/core@2.11.7/+esm', '2.11.7'];
408+
yield 'with no result' => ['https://cdn.jsdelivr.net/npm/lodash.js/+esm', null];
409+
yield 'with a package name' => ['https://cdn.jsdelivr.net/npm/lodash.js@1.2.3/+esm', '1.2.3'];
410+
yield 'with a dash in the package_name' => ['https://cdn.jsdelivr.net/npm/@popperjs/core@2.11.7/+esm', '2.11.7'];
410411
}
411412
}

‎src/Symfony/Component/AssetMapper/Tests/ImportMap/Resolver/JspmResolverTest.php

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ public static function provideResolvePackagesTests(): iterable
162162
/**
163163
* @dataProvider provideGetPackageVersion
164164
*/
165-
public function testGetPackageVersion(string $url, string $expected)
165+
public function testGetPackageVersion(string $url, ?string $expected)
166166
{
167167
$resolver = new JspmResolver();
168168

@@ -171,7 +171,8 @@ public function testGetPackageVersion(string $url, string $expected)
171171

172172
public static function provideGetPackageVersion(): iterable
173173
{
174-
yield 'package_name' => ['https://ga.jspm.io/npm:lodash@1.2.3/lodash.js', '1.2.3'];
175-
yield 'package_name_with_a_dash' => ['https://ga.jspm.io/npm:lodash-dependency@9.8.7/lodash-dependency.js', '9.8.7'];
174+
yield 'with no result' => ['https://ga.jspm.io/npm:lodash/lodash.js', null];
175+
yield 'with a package name' => ['https://ga.jspm.io/npm:lodash@1.2.3/lodash.js', '1.2.3'];
176+
yield 'with a dash in the package_name' => ['https://ga.jspm.io/npm:lodash-dependency@9.8.7/lodash-dependency.js', '9.8.7'];
176177
}
177178
}

0 commit comments

Comments
 (0)