Skip to content

Commit 8eb8202

Browse files
marcofortinamarcofortina
committed
harden plugin shell rendering
1 parent f1e751d commit 8eb8202

17 files changed

Lines changed: 156 additions & 49 deletions

‎src/automax/plugins/auditd.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010

1111
from automax.core.models import ExecutionContext, PluginResult
1212
from automax.plugins.base import BasePlugin, PluginValidationError
13-
from automax.plugins.remote_utils import CHANGE_MARKER, exec_remote, quote, result_from_remote
13+
from automax.plugins.remote_utils import CHANGE_MARKER, exec_remote, heredoc_to_file, quote, result_from_remote
1414

1515

1616
def _sudo(params: Dict[str, Any]) -> str:
@@ -52,7 +52,7 @@ def manual_commands(self, params: Dict[str, Any], context: ExecutionContext) ->
5252
path = self._path(params)
5353
sudo = _sudo(params)
5454
temp = "/tmp/automax-auditd.$$"
55-
commands = [f"cat > {temp} <<'EOF'\n{content}EOF"]
55+
commands = [heredoc_to_file(temp, content)]
5656
if bool(params.get("backup", True)):
5757
commands.append(f"test ! -e {quote(path)} || {sudo}cp -p {quote(path)} {quote(path + str(params.get('backup_suffix', '.bak')))}")
5858
commands.append(f"{sudo}install -D -m 0640 {temp} {quote(path)}")

‎src/automax/plugins/cron.py‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
from automax.core.models import ExecutionContext, PluginResult
1111
from automax.plugins.base import BasePlugin, PluginValidationError
1212
from automax.plugins.file_utils import install_uploaded_file, upload_text_to_temp
13-
from automax.plugins.remote_utils import CHANGE_MARKER, exec_remote, quote, result_from_remote
13+
from automax.plugins.remote_utils import CHANGE_MARKER, exec_remote, quote, result_from_remote, validate_env_name
1414

1515

1616
def _sudo(params: Dict[str, Any]) -> str:
@@ -39,6 +39,13 @@ def validate(self, params: Dict[str, Any]) -> None:
3939
_safe_name(str(params["name"]))
4040
if str(params.get("state", "present")) not in {"present", "absent"}:
4141
raise PluginValidationError("cron.entry state must be present or absent")
42+
env = params.get("env") or {}
43+
if not isinstance(env, dict):
44+
raise PluginValidationError("cron.entry env must be a mapping")
45+
for key, value in env.items():
46+
validate_env_name(key)
47+
if "\n" in str(value) or "\r" in str(value):
48+
raise PluginValidationError("cron.entry env values must be single-line")
4249

4350
def execute(self, params: Dict[str, Any], context: ExecutionContext) -> PluginResult:
4451
self.validate(params)
@@ -49,10 +56,8 @@ def execute(self, params: Dict[str, Any], context: ExecutionContext) -> PluginRe
4956
return result_from_remote(rc=rc, stdout=out, stderr=err, message="cron.entry failed")
5057
env_lines = []
5158
env = params.get("env") or {}
52-
if not isinstance(env, dict):
53-
raise PluginValidationError("cron.entry env must be a mapping")
5459
for key, value in env.items():
55-
env_lines.append(f"{key}={value}")
60+
env_lines.append(f"{validate_env_name(key)}={value}")
5661
content = "\n".join(
5762
[
5863
"# Managed by Automax",

‎src/automax/plugins/hardening.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010

1111
from automax.core.models import ExecutionContext, PluginResult
1212
from automax.plugins.base import BasePlugin, PluginValidationError
13-
from automax.plugins.remote_utils import CHANGE_MARKER, exec_remote, quote, result_from_remote
13+
from automax.plugins.remote_utils import CHANGE_MARKER, exec_remote, heredoc_to_file, quote, result_from_remote
1414

1515

1616
def _sudo(params: Dict[str, Any]) -> str:
@@ -53,7 +53,7 @@ def manual_commands(self, params: Dict[str, Any], context: ExecutionContext) ->
5353
content = self._content(params)
5454
sudo = _sudo(params)
5555
tmp = "/tmp/automax-sshd.$$"
56-
commands = [f"cat > {tmp} <<'EOF'\n{content}EOF"]
56+
commands = [heredoc_to_file(tmp, content)]
5757
if bool(params.get("backup", True)):
5858
commands.append(f"test ! -e {quote(path)} || {sudo}cp -p {quote(path)} {quote(path + str(params.get('backup_suffix', '.bak')))}")
5959
commands.extend([f"{sudo}install -D -m 0644 {tmp} {quote(path)}", f"rm -f {tmp}", f"{sudo}sshd -t"])
@@ -129,7 +129,7 @@ def manual_commands(self, params: Dict[str, Any], context: ExecutionContext) ->
129129
content = self._content(params)
130130
sudo = _sudo(params)
131131
tmp = "/tmp/automax-pwquality.$$"
132-
commands = [f"cat > {tmp} <<'EOF'\n{content}EOF"]
132+
commands = [heredoc_to_file(tmp, content)]
133133
if bool(params.get("backup", True)):
134134
commands.append(f"test ! -e {quote(path)} || {sudo}cp -p {quote(path)} {quote(path + str(params.get('backup_suffix', '.bak')))}")
135135
commands.extend([f"{sudo}install -D -m 0644 {tmp} {quote(path)}", f"rm -f {tmp}"])

‎src/automax/plugins/linux_ops.py‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010

1111
from automax.core.models import ExecutionContext, PluginResult
1212
from automax.plugins.base import BasePlugin, PluginValidationError
13-
from automax.plugins.remote_utils import CHANGE_MARKER, exec_remote, quote, result_from_remote
13+
from automax.plugins.remote_utils import CHANGE_MARKER, exec_remote, heredoc_to_file, normalize_env_mapping, quote, render_env_prefix, result_from_remote
1414

1515

1616
def _sudo(params: Dict[str, Any], default: bool = True) -> str:
@@ -42,7 +42,7 @@ def _mapping(params: Dict[str, Any], key: str) -> dict[str, str]:
4242
raw = params.get(key, {}) or {}
4343
if not isinstance(raw, dict) or not raw:
4444
raise PluginValidationError(f"{key} must be a non-empty mapping")
45-
return {str(name): str(value) for name, value in raw.items()}
45+
return normalize_env_mapping(raw)
4646

4747

4848
class SwapPresentPlugin(BasePlugin):
@@ -153,7 +153,7 @@ def manual_commands(self, params: Dict[str, Any], context: ExecutionContext) ->
153153
path = f"/etc/security/limits.d/{params['name']}.conf"
154154
sudo = _sudo(params)
155155
temp = "/tmp/automax-limits.$$"
156-
commands = [f"cat > {temp} <<'EOF'\n{content}EOF"]
156+
commands = [heredoc_to_file(temp, content)]
157157
if bool(params.get("backup", True)):
158158
commands.append(f"test ! -e {quote(path)} || {sudo}cp -p {quote(path)} {quote(path + str(params.get('backup_suffix', '.bak')))}")
159159
commands.append(f"{sudo}install -m 0644 {temp} {quote(path)}")
@@ -390,7 +390,7 @@ def manual_commands(self, params: Dict[str, Any], context: ExecutionContext) ->
390390
path = self._path_for_backend(params)
391391
content = self._resolved_content(params) if backend == "systemd-resolved" else self._content(params)
392392
temp = "/tmp/automax-resolver.$$"
393-
commands = [f"cat > {temp} <<'EOF'\n{content}EOF"]
393+
commands = [heredoc_to_file(temp, content)]
394394
if backend == "plain-file" and not bool(params.get("force", False)):
395395
commands.append("if [ -L /etc/resolv.conf ]; then echo 'refusing to manage symlinked /etc/resolv.conf with backend=plain-file' >&2; exit 1; fi")
396396
if bool(params.get("backup", True)):
@@ -432,7 +432,7 @@ def manual_commands(self, params: Dict[str, Any], context: ExecutionContext) ->
432432
path = str(params.get("path", "/etc/chrony.d/99-automax.conf"))
433433
sudo = _sudo(params)
434434
temp = "/tmp/automax-chrony.$$"
435-
commands = [f"cat > {temp} <<'EOF'\n{content}EOF"]
435+
commands = [heredoc_to_file(temp, content)]
436436
if bool(params.get("backup", True)):
437437
commands.append(f"test ! -e {quote(path)} || {sudo}cp -p {quote(path)} {quote(path + str(params.get('backup_suffix', '.bak')))}")
438438
commands.append(f"{sudo}install -m 0644 {temp} {quote(path)}")
@@ -507,13 +507,12 @@ def manual_commands(self, params: Dict[str, Any], context: ExecutionContext) ->
507507
variables = _mapping(params, "variables")
508508
scope = str(params.get("scope", "step"))
509509
if scope == "step":
510-
exports = " ".join(f"{name}={quote(value)}" for name, value in sorted(variables.items()))
511-
return [f"export {exports}"]
510+
return [f"export {render_env_prefix(variables)}"]
512511
content = self._content(params)
513512
path = self._path(params)
514513
sudo = _sudo(params, default=scope == "global")
515514
temp = "/tmp/automax-env.$$"
516-
commands = [f"cat > {temp} <<'EOF'\n{content}EOF"]
515+
commands = [heredoc_to_file(temp, content)]
517516
if bool(params.get("backup", True)):
518517
commands.append(f"test ! -e {quote(path)} || {sudo}cp -p {quote(path)} {quote(path + str(params.get('backup_suffix', '.bak')))}")
519518
commands.append(f"{sudo}install -m 0644 {temp} {quote(path)}")

‎src/automax/plugins/local_command.py‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@
1212
from typing import Any, Dict
1313

1414
from automax.core.models import ExecutionContext, PluginResult
15-
from automax.plugins.base import BasePlugin
15+
from automax.plugins.base import BasePlugin, PluginValidationError
16+
from automax.plugins.remote_utils import normalize_env_mapping, render_env_prefix
1617

1718

1819
class LocalCommandPlugin(BasePlugin):
@@ -35,11 +36,9 @@ def manual_commands(
3536
rendered = f"cd {shlex.quote(str(params['cwd']))} && {rendered}"
3637
if params.get("env"):
3738
env = params["env"]
38-
if isinstance(env, dict):
39-
prefix = " ".join(
40-
f"{key}={shlex.quote(str(value))}" for key, value in sorted(env.items())
41-
)
42-
rendered = f"{prefix} {rendered}"
39+
if not isinstance(env, dict):
40+
raise PluginValidationError("local.command env must be a mapping")
41+
rendered = f"{render_env_prefix(env)} {rendered}"
4342
return [rendered]
4443

4544
def execute(self, params: Dict[str, Any], context: ExecutionContext) -> PluginResult:
@@ -50,6 +49,10 @@ def execute(self, params: Dict[str, Any], context: ExecutionContext) -> PluginRe
5049
command = params["command"]
5150
cwd = params.get("cwd")
5251
env = params.get("env")
52+
if env is not None:
53+
if not isinstance(env, dict):
54+
raise PluginValidationError("local.command env must be a mapping")
55+
env = normalize_env_mapping(env)
5356
shell = bool(params.get("shell", isinstance(command, str)))
5457
timeout = params.get("timeout", context.command_timeout)
5558

‎src/automax/plugins/manual_preview.py‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
from typing import Any, Dict
1212

1313
from automax.core.models import ExecutionContext
14+
from automax.plugins.remote_utils import heredoc_to_file, heredoc_to_stdin
1415

1516

1617
def _q(value: Any) -> str:
@@ -165,10 +166,10 @@ def fallback_manual_commands(plugin_name: str, params: Dict[str, Any], context:
165166
if plugin_name.startswith("nftables."):
166167
if plugin_name == "nftables.validate":
167168
src = params.get("src")
168-
return [f"{sudo}nft -c -f {_q(src)}" if src else f"cat <<'EOF' | {sudo}nft -c -f -\n{params.get('content', '')}\nEOF"]
169+
return [f"{sudo}nft -c -f {_q(src)}" if src else heredoc_to_stdin(f"{sudo}nft -c -f -", params.get("content", ""))]
169170
if plugin_name == "nftables.apply":
170171
src = params.get("src")
171-
return [f"{sudo}nft -f {_q(src)}" if src else f"cat <<'EOF' | {sudo}nft -f -\n{params.get('content', '')}\nEOF"]
172+
return [f"{sudo}nft -f {_q(src)}" if src else heredoc_to_stdin(f"{sudo}nft -f -", params.get("content", ""))]
172173

173174
if plugin_name.startswith("pkg."):
174175
packages = _packages(params)
@@ -235,7 +236,7 @@ def fallback_manual_commands(plugin_name: str, params: Dict[str, Any], context:
235236
if plugin_name == "fs.template":
236237
return [f"install -D {_q(params.get('src', '/tmp/template'))} {_q(params.get('dest', '/tmp/dest'))}"]
237238
if plugin_name == "fs.write":
238-
return [f"cat > {_q(path)} <<'EOF'\n{params.get('content', '')}\nEOF"]
239+
return [heredoc_to_file(path, params.get("content", ""))]
239240

240241
if plugin_name in {"fstab.entry", "mount.present"}:
241242
return [f"{sudo}mount {_q(params.get('path', path))}"]

‎src/automax/plugins/network.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
from automax.core.models import ExecutionContext, PluginResult
1212
from automax.plugins.base import BasePlugin, PluginValidationError
1313
from automax.plugins.linux_ops import ResolverConfigPlugin
14-
from automax.plugins.remote_utils import CHANGE_MARKER, exec_remote, quote, result_from_remote
14+
from automax.plugins.remote_utils import CHANGE_MARKER, exec_remote, heredoc_to_file, quote, result_from_remote
1515

1616

1717
def _sudo(params: Dict[str, Any]) -> str:
@@ -46,7 +46,7 @@ def _backup_cmd(path: str, params: Dict[str, Any]) -> str:
4646
def _write_file_cmd(path: str, content: str, mode: str, params: Dict[str, Any]) -> str:
4747
temp = "/tmp/automax-net.$$"
4848
return " && ".join([
49-
f"cat > {temp} <<'EOF'\n{content}EOF",
49+
heredoc_to_file(temp, content),
5050
_backup_cmd(path, params),
5151
f"{_sudo(params)}install -D -m {mode} {temp} {quote(path)}",
5252
f"rm -f {temp}",

‎src/automax/plugins/ops_completeness.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010

1111
from automax.core.models import ExecutionContext, PluginResult
1212
from automax.plugins.base import BasePlugin, PluginValidationError
13-
from automax.plugins.remote_utils import CHANGE_MARKER, exec_remote, quote, result_from_remote
13+
from automax.plugins.remote_utils import CHANGE_MARKER, exec_remote, heredoc_to_file, quote, result_from_remote
1414

1515

1616
def _sudo(params: Dict[str, Any]) -> str:
@@ -271,7 +271,7 @@ def manual_commands(self, params: Dict[str, Any], context: ExecutionContext) ->
271271
return [f"test ! -e {quote(path)} || {_sudo(params)}rm -f {quote(path)}"]
272272
content = f"blacklist {params['module']}\ninstall {params['module']} /bin/false\n"
273273
tmp = "/tmp/automax-modprobe.$$"
274-
cmds = [f"cat > {tmp} <<'EOF'\n{content}EOF"]
274+
cmds = [heredoc_to_file(tmp, content)]
275275
if bool(params.get("backup", True)):
276276
cmds.append(f"test ! -e {quote(path)} || {_sudo(params)}cp -p {quote(path)} {quote(path + str(params.get('backup_suffix', '.bak')))}")
277277
cmds.append(f"{_sudo(params)}install -D -m 0644 {tmp} {quote(path)}")
@@ -362,7 +362,7 @@ def diff_preview(self, params: Dict[str, Any], context: ExecutionContext) -> lis
362362
return _diff(self._path(params), self._content(params), "sysctl-dropin-plan")
363363

364364
def manual_commands(self, params: Dict[str, Any], context: ExecutionContext) -> list[str]:
365-
path=self._path(params); content=self._content(params); tmp="/tmp/automax-sysctl.$$"; cmds=[f"cat > {tmp} <<'EOF'\n{content}EOF"]
365+
path=self._path(params); content=self._content(params); tmp="/tmp/automax-sysctl.$$"; cmds=[heredoc_to_file(tmp, content)]
366366
if bool(params.get("backup", True)):
367367
cmds.append(f"test ! -e {quote(path)} || {_sudo(params)}cp -p {quote(path)} {quote(path + str(params.get('backup_suffix','.bak')))}")
368368
cmds.append(f"{_sudo(params)}install -D -m 0644 {tmp} {quote(path)}")

‎src/automax/plugins/pam_ops.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010

1111
from automax.core.models import ExecutionContext, PluginResult
1212
from automax.plugins.base import BasePlugin, PluginValidationError
13-
from automax.plugins.remote_utils import CHANGE_MARKER, exec_remote, quote, result_from_remote
13+
from automax.plugins.remote_utils import CHANGE_MARKER, exec_remote, heredoc_to_file, quote, result_from_remote
1414

1515

1616
def _sudo(params: Dict[str, Any]) -> str:
@@ -70,7 +70,7 @@ def _state(params: Dict[str, Any]) -> str:
7070

7171
def _install_content_command(path: str, content: str, params: Dict[str, Any], mode: str = "0644") -> list[str]:
7272
tmp = "/tmp/automax-pam.$$"
73-
commands = [f"cat > {tmp} <<'EOF'\n{content}EOF"]
73+
commands = [heredoc_to_file(tmp, content)]
7474
if bool(params.get("backup", True)):
7575
commands.append(_backup(path, params))
7676
commands.extend([f"{_sudo(params)}install -D -m {mode} {tmp} {quote(path)}", f"rm -f {tmp}"])

‎src/automax/plugins/pkg_pinning.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010

1111
from automax.core.models import ExecutionContext, PluginResult
1212
from automax.plugins.base import BasePlugin, PluginValidationError
13-
from automax.plugins.remote_utils import CHANGE_MARKER, exec_remote, quote, result_from_remote
13+
from automax.plugins.remote_utils import CHANGE_MARKER, exec_remote, heredoc_to_file, quote, result_from_remote
1414

1515

1616
def _sudo(params: Dict[str, Any]) -> str:
@@ -134,7 +134,7 @@ def manual_commands(self, params: Dict[str, Any], context: ExecutionContext) ->
134134
apt_path = self._apt_path(params)
135135
apt_content = self._apt_content(params)
136136
temp = "/tmp/automax-pkg-pin.$$"
137-
apt_commands = [f"cat > {temp} <<'EOF'\n{apt_content}EOF"]
137+
apt_commands = [heredoc_to_file(temp, apt_content)]
138138
if bool(params.get("backup", True)):
139139
apt_commands.append(f"test ! -e {quote(apt_path)} || {sudo}cp -p {quote(apt_path)} {quote(apt_path + str(params.get('backup_suffix', '.bak')))}")
140140
apt_commands.extend([f"{sudo}install -m 0644 {temp} {quote(apt_path)}", f"rm -f {temp}"])
@@ -204,7 +204,7 @@ def manual_commands(self, params: Dict[str, Any], context: ExecutionContext) ->
204204
content = self._content(params)
205205
sudo = _sudo(params)
206206
temp = "/tmp/automax-repo-priority.$$"
207-
commands = [f"cat > {temp} <<'EOF'\n{content}EOF"]
207+
commands = [heredoc_to_file(temp, content)]
208208
if bool(params.get("backup", True)):
209209
commands.append(f"test ! -e {quote(path)} || {sudo}cp -p {quote(path)} {quote(path + str(params.get('backup_suffix', '.bak')))}")
210210
commands.append(f"{sudo}install -m 0644 {temp} {quote(path)}")

0 commit comments

Comments
 (0)