Skip to content

Commit b9a06df

Browse files
committed
- Security scan fixes and tests to verify fixes
1 parent ab86cff commit b9a06df

10 files changed

Lines changed: 813 additions & 56 deletions

File tree

‎config/artisan-docs.php‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,12 +50,17 @@
5050
|--------------------------------------------------------------------------
5151
|
5252
| Command namespaces to always exclude from the generated documentation.
53-
| Example: ['_', 'completion']
53+
| Namespaces for infrastructure tools (Horizon, Telescope, Nova) are
54+
| included by default because they often surface internal connection names,
55+
| queue names, and worker configuration that should not be published.
5456
|
5557
*/
5658
'excluded_namespaces' => [
57-
'_',
58-
'completion',
59+
'_', // Internal Symfony commands
60+
'completion', // Shell-completion helpers
61+
'horizon', // Laravel Horizon – exposes queue/worker internals
62+
'telescope', // Laravel Telescope – exposes recording configuration
63+
'nova', // Laravel Nova – exposes admin tooling internals
5964
],
6065

6166
/*
@@ -64,9 +69,14 @@
6469
|--------------------------------------------------------------------------
6570
|
6671
| Specific command names to always exclude from the generated documentation.
72+
| Top-level commands (no namespace prefix) that expose runtime environment
73+
| state are listed here by default.
6774
|
6875
*/
69-
'excluded_commands' => [],
76+
'excluded_commands' => [
77+
'tinker', // Launches an interactive REPL – not useful to document
78+
'env', // Displays the current application environment label
79+
],
7080

7181
/*
7282
|--------------------------------------------------------------------------

‎resources/views/commands.blade.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@
7373
<div class="sidebar-group">
7474
<div class="sidebar-group-label">{{ $group }}</div>
7575
@foreach($commands as $cmd)
76-
<a class="sidebar-link" href="#{{ str_replace([':', ' '], ['-', '-'], $cmd['name']) }}" title="{{ $cmd['name'] }}">{{ $cmd['name'] }}</a>
76+
<a class="sidebar-link" href="#{{ strtolower(preg_replace('/[^a-z0-9]+/i', '-', $cmd['name'])) }}" title="{{ $cmd['name'] }}">{{ $cmd['name'] }}</a>
7777
@endforeach
7878
</div>
7979
@endforeach
@@ -90,7 +90,7 @@
9090
<h2 class="group-title">{{ $group }}</h2>
9191

9292
@foreach($commands as $cmd)
93-
<div class="command-card" id="{{ str_replace([':', ' '], ['-', '-'], $cmd['name']) }}">
93+
<div class="command-card" id="{{ strtolower(preg_replace('/[^a-z0-9]+/i', '-', $cmd['name'])) }}">
9494
<div class="command-header">
9595
<div style="flex:1">
9696
<div class="command-name">{{ $cmd['name'] }}</div>

‎src/CommandInspector.php‎

Lines changed: 89 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,29 @@
99

1010
class CommandInspector
1111
{
12+
/**
13+
* Regex pattern for argument / option names whose default values should be
14+
* redacted from generated documentation to prevent accidental secret leakage.
15+
*/
16+
private const SENSITIVE_KEYWORD_PATTERN = '/(password|passwd|secret|token|api[_\-]?key|auth[_\-]?key|private[_\-]?key|credential|apikey)/i';
17+
18+
/**
19+
* Sentinel value written in place of a redacted default.
20+
*/
21+
public const REDACTED_VALUE = '[ REDACTED ]';
22+
23+
/**
24+
* Hardcoded fallback list of Symfony / Artisan global option names that should
25+
* always be stripped from per-command documentation. Used only when the command
26+
* has no bound Application (e.g. bare unit-test stubs).
27+
*
28+
* @var string[]
29+
*/
30+
private const FALLBACK_INTERNAL_OPTIONS = [
31+
'help', 'quiet', 'verbose', 'version',
32+
'ansi', 'no-ansi', 'no-interaction', 'env',
33+
];
34+
1235
/**
1336
* Inspect a single command and return structured metadata.
1437
*
@@ -27,7 +50,7 @@ public function inspect(SymfonyCommand $command): array
2750
'namespace' => $this->extractNamespace($command->getName()),
2851
'group' => null, // resolved later by GenerateDocsCommand
2952
'arguments' => $this->inspectArguments($definition->getArguments()),
30-
'options' => $this->inspectOptions($definition->getOptions()),
53+
'options' => $this->inspectOptions($definition->getOptions(), $command),
3154
];
3255
}
3356

@@ -47,7 +70,10 @@ private function inspectArguments(array $arguments): array
4770
'description' => $argument->getDescription(),
4871
'required' => $argument->isRequired(),
4972
'is_array' => $argument->isArray(),
50-
'default' => $argument->getDefault(),
73+
'default' => $this->redactSensitiveDefault(
74+
$argument->getName(),
75+
$argument->getDefault()
76+
),
5177
];
5278
}
5379

@@ -57,13 +83,17 @@ private function inspectArguments(array $arguments): array
5783
/**
5884
* Inspect and normalise all options from a command definition.
5985
*
86+
* Global / internal options (--help, --quiet, etc.) are resolved dynamically
87+
* from the command's bound Application; a hardcoded fallback is used when no
88+
* application is available (e.g. in unit-test stubs).
89+
*
6090
* @param InputOption[] $options
6191
* @return array<int, array<string, mixed>>
6292
*/
63-
private function inspectOptions(array $options): array
93+
private function inspectOptions(array $options, SymfonyCommand $command): array
6494
{
95+
$internal = $this->resolveInternalOptionNames($command);
6596
$results = [];
66-
$internal = ['help', 'quiet', 'verbose', 'version', 'ansi', 'no-ansi', 'no-interaction', 'env'];
6797

6898
foreach ($options as $option) {
6999
if (in_array($option->getName(), $internal, true)) {
@@ -79,13 +109,54 @@ private function inspectOptions(array $options): array
79109
'accepts_value' => ! $option->isValueOptional() && ! $option->isNegatable()
80110
? $option->isValueRequired()
81111
: ! ($option->getDefault() === false && ! $option->acceptValue()),
82-
'default' => $option->getDefault(),
112+
'default' => $this->redactSensitiveDefault(
113+
$option->getName(),
114+
$option->getDefault()
115+
),
83116
];
84117
}
85118

86119
return $results;
87120
}
88121

122+
/**
123+
* Resolve the names of global / internal options that should be stripped from
124+
* per-command documentation.
125+
*
126+
* When the command is bound to a Symfony Application the list is derived
127+
* directly from the application's input definition, which future-proofs the
128+
* package against new global flags added by Symfony or Laravel. When no
129+
* application is available the hardcoded fallback list is used.
130+
*
131+
* @return string[]
132+
*/
133+
private function resolveInternalOptionNames(SymfonyCommand $command): array
134+
{
135+
if ($app = $command->getApplication()) {
136+
return array_keys($app->getDefinition()->getOptions());
137+
}
138+
139+
return self::FALLBACK_INTERNAL_OPTIONS;
140+
}
141+
142+
/**
143+
* Redact the default value when the argument / option name matches a
144+
* pattern associated with sensitive credentials or secrets.
145+
*/
146+
private function redactSensitiveDefault(string $name, mixed $default): mixed
147+
{
148+
// Nothing to redact when there is no meaningful default.
149+
if ($default === null || $default === false || $default === '') {
150+
return $default;
151+
}
152+
153+
if (preg_match(self::SENSITIVE_KEYWORD_PATTERN, $name)) {
154+
return self::REDACTED_VALUE;
155+
}
156+
157+
return $default;
158+
}
159+
89160
/**
90161
* Extract the namespace portion from a command name.
91162
* E.g. "make:model" → "make", "list" → null.
@@ -133,6 +204,10 @@ public function isApplicationCommand(SymfonyCommand $command, array $appCommandP
133204

134205
/**
135206
* Determine whether a command is from a vendor package.
207+
*
208+
* Uses absolute path-prefix matching against the project's real vendor
209+
* directory instead of a substring search, which would produce false
210+
* positives for projects whose root path happens to contain "/vendor/".
136211
*/
137212
public function isVendorCommand(SymfonyCommand $command): bool
138213
{
@@ -142,6 +217,14 @@ public function isVendorCommand(SymfonyCommand $command): bool
142217
return false;
143218
}
144219

145-
return $file && str_contains(str_replace('\\', '/', $file), '/vendor/');
220+
if (! $file) {
221+
return false;
222+
}
223+
224+
// Normalise both paths to forward slashes for cross-platform safety.
225+
$vendorPrefix = rtrim(str_replace('\\', '/', base_path('vendor')), '/').'/';
226+
$normalizedFile = str_replace('\\', '/', $file);
227+
228+
return str_starts_with($normalizedFile, $vendorPrefix);
146229
}
147230
}

‎src/Commands/GenerateDocsCommand.php‎

Lines changed: 57 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,19 @@ public function __construct(private CommandInspector $inspector)
3232
public function handle(): int
3333
{
3434
$format = $this->option('format') ?: config('artisan-docs.default_format', 'markdown');
35-
$output = $this->option('output')
35+
$rawOutput = $this->option('output')
3636
?? ($this->option('format') !== null ? "docs/commands.{$format}" : null)
3737
?? config('artisan-docs.default_output', 'docs/commands.md');
3838
$title = config('artisan-docs.title', 'Artisan Command Reference');
3939

40+
try {
41+
$resolvedOutput = $this->resolveSafePath($rawOutput);
42+
} catch (\InvalidArgumentException $e) {
43+
$this->error('❌ Invalid output path: '.$e->getMessage());
44+
45+
return self::FAILURE;
46+
}
47+
4048
$commands = $this->gatherCommands();
4149

4250
if (empty($commands)) {
@@ -50,13 +58,13 @@ public function handle(): int
5058
$content = $generator->generate($groups, $title);
5159

5260
if ($this->option('check')) {
53-
return $this->runCheck($content, $output);
61+
return $this->runCheck($content, $resolvedOutput);
5462
}
5563

56-
$this->writeOutput($content, $output);
64+
$this->writeOutput($content, $resolvedOutput);
5765

5866
$total = array_sum(array_map('count', $groups));
59-
$this->info("✅ Documentation generated: <comment>{$output}</comment> ({$total} commands, ".count($groups).' groups)');
67+
$this->info("✅ Documentation generated: <comment>{$rawOutput}</comment> ({$total} commands, ".count($groups).' groups)');
6068

6169
return self::SUCCESS;
6270
}
@@ -153,12 +161,51 @@ private function resolveGenerator(string $format): GeneratorContract
153161
};
154162
}
155163

156-
private function writeOutput(string $content, string $path): void
164+
/**
165+
* Validate and resolve an output path to an absolute path inside base_path().
166+
*
167+
* Rejects:
168+
* – Stream-wrapper schemes (e.g. php://, phar://, file://)
169+
* – Absolute paths (Unix leading slash, Windows drive letters / UNC shares)
170+
* – Directory-traversal (any ".." segment)
171+
*
172+
* @throws \InvalidArgumentException
173+
*/
174+
private function resolveSafePath(string $path): string
157175
{
158-
$absolutePath = str_starts_with($path, '/')
159-
? $path
160-
: base_path($path);
176+
// Reject stream-wrapper schemes (e.g. php://, phar://, file://, compress.zlib://)
177+
if (preg_match('#[a-zA-Z][a-zA-Z0-9+\-.]*://#', $path)) {
178+
throw new \InvalidArgumentException(
179+
"Stream-wrapper schemes are not permitted. Got: \"{$path}\"."
180+
);
181+
}
161182

183+
// Reject absolute paths: Unix leading slash, Windows drive letters, UNC shares
184+
if (
185+
str_starts_with($path, '/')
186+
|| str_starts_with($path, '\\')
187+
|| preg_match('/^[a-zA-Z]:[\\\\\/]/', $path)
188+
) {
189+
throw new \InvalidArgumentException(
190+
'Absolute output paths are not permitted. Use a path relative to the project root. '
191+
."Got: \"{$path}\"."
192+
);
193+
}
194+
195+
// Reject directory-traversal sequences
196+
foreach (explode('/', str_replace('\\', '/', $path)) as $segment) {
197+
if ($segment === '..') {
198+
throw new \InvalidArgumentException(
199+
'Directory traversal sequences ("../") are not permitted in the output path.'
200+
);
201+
}
202+
}
203+
204+
return base_path($path);
205+
}
206+
207+
private function writeOutput(string $content, string $absolutePath): void
208+
{
162209
File::ensureDirectoryExists(dirname($absolutePath));
163210
File::put($absolutePath, $content);
164211
}
@@ -167,12 +214,10 @@ private function writeOutput(string $content, string $path): void
167214
* Compare generated content against the existing file on disk.
168215
* Exits with code 1 when the file is missing or differs.
169216
*/
170-
private function runCheck(string $content, string $path): int
217+
private function runCheck(string $content, string $absolutePath): int
171218
{
172-
$absolutePath = str_starts_with($path, '/') ? $path : base_path($path);
173-
174219
if (! File::exists($absolutePath)) {
175-
$this->error("❌ Check failed: documentation file does not exist at [{$path}].");
220+
$this->error("❌ Check failed: documentation file does not exist at [{$absolutePath}].");
176221
$this->line(' Run <comment>php artisan docs:commands</comment> to generate it.');
177222

178223
return self::FAILURE;

‎src/Generators/JsonGenerator.php‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,18 @@ public function generate(array $groups, string $title): string
2323
];
2424
}
2525

26-
return json_encode($payload, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE);
26+
// JSON_HEX_TAG / JSON_HEX_AMP / JSON_HEX_APOS / JSON_HEX_QUOT ensure that
27+
// HTML-sensitive characters are unicode-escaped so the output is safe when
28+
// embedded in an HTML page or served with an incorrect Content-Type header.
29+
return json_encode(
30+
$payload,
31+
JSON_PRETTY_PRINT
32+
| JSON_UNESCAPED_SLASHES
33+
| JSON_UNESCAPED_UNICODE
34+
| JSON_HEX_TAG
35+
| JSON_HEX_AMP
36+
| JSON_HEX_APOS
37+
| JSON_HEX_QUOT
38+
);
2739
}
2840
}

0 commit comments

Comments
 (0)