refactor(validation): tokenize shell-safe command pattern
Replace the flat character-class regex for SHELL_SAFE_COMMAND_PATTERN with a token-aware alternation. The parser now recognizes explicit tokens (`&&`, `||`, balanced single/double quotes, whitespace, and an unquoted safe-char run) instead of a bag of characters, which lets us extend the accepted grammar without loosening the guarantees. New surface area, with tests: - logical OR chaining (`make build || make clean`) - shell globs and bang (`rm *.tmp`, `cp src/?.js dist/`, `! grep -q foo`) - single-quoted arguments are now treated as balanced runs rather than rejected per-character Preserved surface area: - && chaining, balanced "..." and '...' quotes, the previous safe path / argument characters, and the existing error-path contract in ApplicationDeploymentJob::validateShellSafeCommand(). Also refreshes the user-facing validation messages in General.php so the allow/deny list shown on failure matches the new grammar. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
parent
1cf6c7d0ae
commit
817128c5af
3 changed files with 153 additions and 16 deletions
|
|
@ -197,12 +197,12 @@ protected function messages(): array
|
||||||
'baseDirectory.regex' => 'The base directory must be a valid path starting with / and containing only safe characters.',
|
'baseDirectory.regex' => 'The base directory must be a valid path starting with / and containing only safe characters.',
|
||||||
'publishDirectory.regex' => 'The publish directory must be a valid path starting with / and containing only safe characters.',
|
'publishDirectory.regex' => 'The publish directory must be a valid path starting with / and containing only safe characters.',
|
||||||
'dockerfileTargetBuild.regex' => 'The Dockerfile target build must contain only alphanumeric characters, dots, hyphens, and underscores.',
|
'dockerfileTargetBuild.regex' => 'The Dockerfile target build must contain only alphanumeric characters, dots, hyphens, and underscores.',
|
||||||
'dockerComposeCustomStartCommand.regex' => 'The Docker Compose start command contains invalid characters. Shell operators like ;, |, $, and backticks are not allowed.',
|
'dockerComposeCustomStartCommand.regex' => 'The Docker Compose start command contains invalid characters. Allowed: alphanumerics, && / || chaining, balanced quotes, globs (*, ?), !, and safe path/arg chars. Blocked: bare &, bare |, ;, $, backtick, (, ), <, >, \\, newlines.',
|
||||||
'dockerComposeCustomBuildCommand.regex' => 'The Docker Compose build command contains invalid characters. Shell operators like ;, |, $, and backticks are not allowed.',
|
'dockerComposeCustomBuildCommand.regex' => 'The Docker Compose build command contains invalid characters. Allowed: alphanumerics, && / || chaining, balanced quotes, globs (*, ?), !, and safe path/arg chars. Blocked: bare &, bare |, ;, $, backtick, (, ), <, >, \\, newlines.',
|
||||||
'customDockerRunOptions.regex' => 'The custom Docker run options contain invalid characters. Shell operators like ;, |, $, and backticks are not allowed.',
|
'customDockerRunOptions.regex' => 'The custom Docker run options contain invalid characters. Allowed: alphanumerics, && / || chaining, balanced quotes, globs (*, ?), !, and safe path/arg chars. Blocked: bare &, bare |, ;, $, backtick, (, ), <, >, \\, newlines.',
|
||||||
'installCommand.regex' => 'The install command contains invalid characters. Shell operators like ;, |, $, and backticks are not allowed.',
|
'installCommand.regex' => 'The install command contains invalid characters. Allowed: alphanumerics, && / || chaining, balanced quotes, globs (*, ?), !, and safe path/arg chars. Blocked: bare &, bare |, ;, $, backtick, (, ), <, >, \\, newlines.',
|
||||||
'buildCommand.regex' => 'The build command contains invalid characters. Shell operators like ;, |, $, and backticks are not allowed.',
|
'buildCommand.regex' => 'The build command contains invalid characters. Allowed: alphanumerics, && / || chaining, balanced quotes, globs (*, ?), !, and safe path/arg chars. Blocked: bare &, bare |, ;, $, backtick, (, ), <, >, \\, newlines.',
|
||||||
'startCommand.regex' => 'The start command contains invalid characters. Shell operators like ;, |, $, and backticks are not allowed.',
|
'startCommand.regex' => 'The start command contains invalid characters. Allowed: alphanumerics, && / || chaining, balanced quotes, globs (*, ?), !, and safe path/arg chars. Blocked: bare &, bare |, ;, $, backtick, (, ), <, >, \\, newlines.',
|
||||||
'preDeploymentCommandContainer.regex' => 'The pre-deployment command container name must contain only alphanumeric characters, dots, hyphens, and underscores.',
|
'preDeploymentCommandContainer.regex' => 'The pre-deployment command container name must contain only alphanumeric characters, dots, hyphens, and underscores.',
|
||||||
'postDeploymentCommandContainer.regex' => 'The post-deployment command container name must contain only alphanumeric characters, dots, hyphens, and underscores.',
|
'postDeploymentCommandContainer.regex' => 'The post-deployment command container name must contain only alphanumeric characters, dots, hyphens, and underscores.',
|
||||||
'name.required' => 'The Name field is required.',
|
'name.required' => 'The Name field is required.',
|
||||||
|
|
|
||||||
|
|
@ -36,15 +36,31 @@ class ValidationPatterns
|
||||||
public const DOCKER_TARGET_PATTERN = '/^[a-zA-Z0-9][a-zA-Z0-9._-]*$/';
|
public const DOCKER_TARGET_PATTERN = '/^[a-zA-Z0-9][a-zA-Z0-9._-]*$/';
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Pattern for shell-safe command strings (docker compose commands, docker run options)
|
* Token-aware pattern for shell-safe command strings (docker compose commands, docker run options).
|
||||||
* Blocks dangerous shell metacharacters: ; | ` $ ( ) > < newlines and carriage returns
|
*
|
||||||
* Allows & for command chaining (&&) which is common in multi-step build commands
|
* Accepts a sequence of the following tokens only:
|
||||||
* Allows double quotes for build args with spaces (e.g. --build-arg KEY="value")
|
* [ \t]+ — whitespace (space / tab)
|
||||||
* Blocks backslashes to prevent escape-sequence attacks
|
* && — logical AND (matched before bare & can match anything)
|
||||||
* Allows single and double quotes for quoted arguments (e.g. --entrypoint "sh -c 'npm start'")
|
* || — logical OR (matched before bare | can match anything)
|
||||||
* Uses [ \t] instead of \s to explicitly exclude \n and \r (which act as command separators)
|
* "[^"$`\\\n\r]*" — balanced double-quoted string; blocks $, backtick, \, newlines inside
|
||||||
|
* '[^'\n\r]*' — balanced single-quoted string; blocks newlines inside (all else literal)
|
||||||
|
* [safe-chars]+ — unquoted alphanumerics + safe path/arg chars (includes glob *, ?, and !)
|
||||||
|
*
|
||||||
|
* Blocked everywhere (outside and inside unquoted tokens):
|
||||||
|
* bare & (background op), bare |, ;, $, `, (, ), <, >, \, newline, CR
|
||||||
|
*
|
||||||
|
* Blocked inside double-quoted spans specifically:
|
||||||
|
* $ (variable/command expansion), ` (command substitution), \ (escape)
|
||||||
|
*
|
||||||
|
* Legitimate use cases preserved:
|
||||||
|
* docker compose build && docker tag x && docker push y
|
||||||
|
* make build || make clean
|
||||||
|
* rm *.tmp cp src/?.js dist/
|
||||||
|
* ! grep -q foo && echo missing
|
||||||
|
* docker compose up -d --build-arg VERSION="1.0.0"
|
||||||
|
* --entrypoint "sh -c 'npm start'"
|
||||||
*/
|
*/
|
||||||
public const SHELL_SAFE_COMMAND_PATTERN = '/^[a-zA-Z0-9 \t._\-\/=:@,+\[\]{}#%^~&"\']+$/';
|
public const SHELL_SAFE_COMMAND_PATTERN = '/^(?:[ \t]+|&&|\|\||"[^"$`\\\\\n\r]*"|\'[^\'\n\r]*\'|[a-zA-Z0-9._\-\/=:@,+\[\]{}#%^~*?!]+)+$/';
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Pattern for Docker volume names
|
* Pattern for Docker volume names
|
||||||
|
|
|
||||||
|
|
@ -414,7 +414,7 @@
|
||||||
expect($validator->fails())->toBeTrue();
|
expect($validator->fails())->toBeTrue();
|
||||||
});
|
});
|
||||||
|
|
||||||
test('rejects single quotes in docker_compose_custom_start_command', function () {
|
test('allows single-quoted arguments in docker_compose_custom_start_command', function () {
|
||||||
$rules = sharedDataApplications();
|
$rules = sharedDataApplications();
|
||||||
|
|
||||||
$validator = validator(
|
$validator = validator(
|
||||||
|
|
@ -422,7 +422,7 @@
|
||||||
['docker_compose_custom_start_command' => $rules['docker_compose_custom_start_command']]
|
['docker_compose_custom_start_command' => $rules['docker_compose_custom_start_command']]
|
||||||
);
|
);
|
||||||
|
|
||||||
expect($validator->fails())->toBeTrue();
|
expect($validator->fails())->toBeFalse();
|
||||||
});
|
});
|
||||||
|
|
||||||
test('allows double quotes in docker_compose_custom_start_command', function () {
|
test('allows double quotes in docker_compose_custom_start_command', function () {
|
||||||
|
|
@ -474,6 +474,127 @@
|
||||||
expect($method->invoke($instance, 'docker compose up -d --build', 'docker_compose_custom_start_command'))
|
expect($method->invoke($instance, 'docker compose up -d --build', 'docker_compose_custom_start_command'))
|
||||||
->toBe('docker compose up -d --build');
|
->toBe('docker compose up -d --build');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('rejects bare ampersand PoC payload (GHSA-chg4-63hm-xv9x)', function () {
|
||||||
|
$rules = sharedDataApplications();
|
||||||
|
$payload = 'true & docker run --rm -v /:/h alpine sh -c "cp /h/etc/shadow /h/tmp/leak"';
|
||||||
|
|
||||||
|
$validator = validator(
|
||||||
|
['docker_compose_custom_start_command' => $payload],
|
||||||
|
['docker_compose_custom_start_command' => $rules['docker_compose_custom_start_command']]
|
||||||
|
);
|
||||||
|
|
||||||
|
expect($validator->fails())->toBeTrue();
|
||||||
|
});
|
||||||
|
|
||||||
|
test('rejects bare ampersand across every shell-safe field', function ($field) {
|
||||||
|
$rules = sharedDataApplications();
|
||||||
|
|
||||||
|
$validator = validator(
|
||||||
|
[$field => 'cmd1 & cmd2'],
|
||||||
|
[$field => $rules[$field]]
|
||||||
|
);
|
||||||
|
|
||||||
|
expect($validator->fails())->toBeTrue();
|
||||||
|
})->with([
|
||||||
|
'install_command',
|
||||||
|
'build_command',
|
||||||
|
'start_command',
|
||||||
|
'docker_compose_custom_build_command',
|
||||||
|
'docker_compose_custom_start_command',
|
||||||
|
'custom_docker_run_options',
|
||||||
|
]);
|
||||||
|
|
||||||
|
test('rejects command substitution inside double quotes', function ($payload) {
|
||||||
|
$rules = sharedDataApplications();
|
||||||
|
|
||||||
|
$validator = validator(
|
||||||
|
['build_command' => "echo $payload"],
|
||||||
|
['build_command' => $rules['build_command']]
|
||||||
|
);
|
||||||
|
|
||||||
|
expect($validator->fails())->toBeTrue();
|
||||||
|
})->with(['"$(whoami)"', '"`whoami`"']);
|
||||||
|
|
||||||
|
test('rejects unbalanced quotes', function ($payload) {
|
||||||
|
$rules = sharedDataApplications();
|
||||||
|
|
||||||
|
$validator = validator(
|
||||||
|
['build_command' => $payload],
|
||||||
|
['build_command' => $rules['build_command']]
|
||||||
|
);
|
||||||
|
|
||||||
|
expect($validator->fails())->toBeTrue();
|
||||||
|
})->with(['echo "unterminated', "echo 'unterminated"]);
|
||||||
|
|
||||||
|
test('rejects backslash anywhere', function ($payload) {
|
||||||
|
$rules = sharedDataApplications();
|
||||||
|
|
||||||
|
$validator = validator(
|
||||||
|
['build_command' => $payload],
|
||||||
|
['build_command' => $rules['build_command']]
|
||||||
|
);
|
||||||
|
|
||||||
|
expect($validator->fails())->toBeTrue();
|
||||||
|
})->with(['echo \\;', 'echo \\$HOME']);
|
||||||
|
|
||||||
|
test('runtime validateShellSafeCommand rejects bare ampersand payload', function () {
|
||||||
|
$job = new ReflectionClass(ApplicationDeploymentJob::class);
|
||||||
|
$method = $job->getMethod('validateShellSafeCommand');
|
||||||
|
$method->setAccessible(true);
|
||||||
|
|
||||||
|
$instance = $job->newInstanceWithoutConstructor();
|
||||||
|
|
||||||
|
expect(fn () => $method->invoke($instance, 'true & whoami', 'docker_compose_custom_start_command'))
|
||||||
|
->toThrow(RuntimeException::class, 'contains forbidden shell characters');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('allows logical OR chaining', function ($cmd) {
|
||||||
|
$rules = sharedDataApplications();
|
||||||
|
|
||||||
|
$validator = validator(
|
||||||
|
['build_command' => $cmd],
|
||||||
|
['build_command' => $rules['build_command']]
|
||||||
|
);
|
||||||
|
|
||||||
|
expect($validator->fails())->toBeFalse();
|
||||||
|
})->with([
|
||||||
|
'make build || make clean',
|
||||||
|
'npm run build || npm run fallback',
|
||||||
|
'cmd-a || cmd-b && cmd-c',
|
||||||
|
]);
|
||||||
|
|
||||||
|
test('allows glob and bang tokens', function ($cmd) {
|
||||||
|
$rules = sharedDataApplications();
|
||||||
|
|
||||||
|
$validator = validator(
|
||||||
|
['build_command' => $cmd],
|
||||||
|
['build_command' => $rules['build_command']]
|
||||||
|
);
|
||||||
|
|
||||||
|
expect($validator->fails())->toBeFalse();
|
||||||
|
})->with([
|
||||||
|
'rm *.tmp',
|
||||||
|
'cp src/?.js dist/',
|
||||||
|
'! grep -q foo && echo missing',
|
||||||
|
'docker build --tag app-v1!',
|
||||||
|
]);
|
||||||
|
|
||||||
|
test('rejects bare pipe even though || is allowed', function ($cmd) {
|
||||||
|
$rules = sharedDataApplications();
|
||||||
|
|
||||||
|
$validator = validator(
|
||||||
|
['build_command' => $cmd],
|
||||||
|
['build_command' => $rules['build_command']]
|
||||||
|
);
|
||||||
|
|
||||||
|
expect($validator->fails())->toBeTrue();
|
||||||
|
})->with([
|
||||||
|
'cmd | cat',
|
||||||
|
'cmd|cat',
|
||||||
|
'a |b',
|
||||||
|
'a| b',
|
||||||
|
]);
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('custom_docker_run_options validation', function () {
|
describe('custom_docker_run_options validation', function () {
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue