Skip to content

Commit 089e872

Browse files
committed
feat(release): reject unsafe override values in VendoredFileChecker
Reject overrides that are absolute or contain `..` segments — overrides must resolve inside the consumer dir. Defense-in-depth against a typo or malicious override that escapes the intended consumer layout. Assisted-by: Claude Code
1 parent d63c585 commit 089e872

2 files changed

Lines changed: 36 additions & 2 deletions

File tree

tools/release/src/VendoredFileChecker.php

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,21 +49,30 @@
4949
/**
5050
* @param array<string, string> $pathOverrides Map of canonical relative
5151
* path → consumer relative path. Unmapped entries default to the
52-
* canonical path. Unknown keys (not in VENDORED_PATHS) throw.
52+
* canonical path. Unknown keys (not in VENDORED_PATHS) throw, as do
53+
* values that are absolute or contain `..` segments — overrides must
54+
* stay inside the consumer dir.
5355
*/
5456
public function __construct(
5557
private string $canonicalRoot,
5658
private string $consumerDir,
5759
private array $pathOverrides = [],
5860
) {
59-
foreach (array_keys($pathOverrides) as $key) {
61+
foreach ($pathOverrides as $key => $value) {
6062
if (!in_array($key, self::VENDORED_PATHS, true)) {
6163
throw new \InvalidArgumentException(sprintf(
6264
'Unknown override key %s; must be one of: %s',
6365
$key,
6466
implode(', ', self::VENDORED_PATHS),
6567
));
6668
}
69+
if ($value === '' || $value[0] === '/' || preg_match('#(^|/)\.\.(/|$)#', $value) === 1) {
70+
throw new \InvalidArgumentException(sprintf(
71+
'Override value for %s must be a relative path inside the consumer dir, got: %s',
72+
$key,
73+
$value,
74+
));
75+
}
6776
}
6877
}
6978

tools/release/tests/VendoredFileCheckerTest.php

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
use OpenEMR\Release\VendoredDriftIssue;
1616
use OpenEMR\Release\VendoredFileChecker;
17+
use PHPUnit\Framework\Attributes\DataProvider;
1718
use PHPUnit\Framework\TestCase;
1819

1920
final class VendoredFileCheckerTest extends TestCase
@@ -269,6 +270,30 @@ public function testUnknownOverrideKeyThrows(): void
269270
);
270271
}
271272

273+
/**
274+
* @return iterable<string, array{string}>
275+
*/
276+
public static function unsafeOverrideValueProvider(): iterable
277+
{
278+
yield 'absolute path' => ['/etc/passwd'];
279+
yield 'parent traversal' => ['../outside.php'];
280+
yield 'embedded parent traversal' => ['src/../../outside.php'];
281+
yield 'trailing parent traversal' => ['src/Release/..'];
282+
yield 'empty value' => [''];
283+
}
284+
285+
#[DataProvider('unsafeOverrideValueProvider')]
286+
public function testUnsafeOverrideValueThrows(string $unsafeValue): void
287+
{
288+
$this->expectException(\InvalidArgumentException::class);
289+
290+
new VendoredFileChecker(
291+
$this->canonicalRoot,
292+
$this->consumerDir,
293+
['src/TagVerifier.php' => $unsafeValue],
294+
);
295+
}
296+
272297
/**
273298
* @param list<string>|null $only restrict which paths to write
274299
*/

0 commit comments

Comments
 (0)