Skip to content

Commit 6eb8f2b

Browse files
committed
Extract _chroot_argv helper and harden user/file ops
Address svartkanin's review on #4473: factor the ['arch-chroot', '-S', str(self.target), ...] boilerplate into a private Installer._chroot_argv() helper, and migrate the seven existing argv-form call sites to it (useradd, gpasswd, chpasswd, chsh, chown, snapper-create-config, grub-install). Two related hardening tweaks while in the area: - Raise gpasswd failure log from debug() to warn(). The group-add loop has no return-False feedback channel for the caller, so a silent debug() means a half-configured user looks like a successful install. - Add `--` end-of-options separator for useradd and chown so a username or path starting with `-` cannot smuggle flags. The TUI validates usernames, but parse_arguments() in models/users.py does not, so config.json is the residual hole; this closes it for these two sites at zero cost.
1 parent 53b9be2 commit 6eb8f2b

1 file changed

Lines changed: 12 additions & 25 deletions

File tree

archinstall/lib/installer.py

Lines changed: 12 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -739,6 +739,9 @@ def arch_chroot(self, cmd: str, run_as: str | None = None, peek_output: bool = F
739739

740740
return self.run_command(cmd, peek_output=peek_output)
741741

742+
def _chroot_argv(self, *args: str) -> list[str]:
743+
return ['arch-chroot', '-S', str(self.target), *args]
744+
742745
def drop_to_shell(self) -> None:
743746
subprocess.check_call(f'arch-chroot {self.target}', shell=True)
744747

@@ -987,17 +990,7 @@ def setup_btrfs_snapshot(
987990
}
988991

989992
for config_name, mountpoint in snapper.items():
990-
command = [
991-
'arch-chroot',
992-
'-S',
993-
str(self.target),
994-
'snapper',
995-
'--no-dbus',
996-
'-c',
997-
config_name,
998-
'create-config',
999-
mountpoint,
1000-
]
993+
command = self._chroot_argv('snapper', '--no-dbus', '-c', config_name, 'create-config', mountpoint)
1001994

1002995
try:
1003996
SysCommand(command, peek_output=True)
@@ -1336,13 +1329,7 @@ def _add_grub_bootloader(
13361329

13371330
boot_dir = Path('/boot')
13381331

1339-
command = [
1340-
'arch-chroot',
1341-
'-S',
1342-
str(self.target),
1343-
'grub-install',
1344-
'--debug',
1345-
]
1332+
command = self._chroot_argv('grub-install', '--debug')
13461333

13471334
if SysInfo.has_uefi():
13481335
if not efi_partition:
@@ -1922,12 +1909,12 @@ def _create_user(self, user: User) -> None:
19221909
if not handled_by_plugin:
19231910
info(f'Creating user {user.username}')
19241911

1925-
cmd = ['arch-chroot', '-S', str(self.target), 'useradd', '-m']
1912+
cmd = self._chroot_argv('useradd', '-m')
19261913

19271914
if user.sudo:
19281915
cmd += ['-G', 'wheel']
19291916

1930-
cmd.append(user.username)
1917+
cmd += ['--', user.username]
19311918

19321919
try:
19331920
run(cmd)
@@ -1943,11 +1930,11 @@ def _create_user(self, user: User) -> None:
19431930
self.set_user_password(user)
19441931

19451932
for group in user.groups:
1946-
cmd = ['arch-chroot', '-S', str(self.target), 'gpasswd', '-a', user.username, group]
1933+
cmd = self._chroot_argv('gpasswd', '-a', user.username, group)
19471934
try:
19481935
run(cmd)
19491936
except CalledProcessError as err:
1950-
debug(f'Error adding {user.username} to group {group}: {err}')
1937+
warn(f'Failed to add {user.username} to group {group}: {err}')
19511938

19521939
if user.sudo:
19531940
self.enable_sudo(user)
@@ -1962,7 +1949,7 @@ def set_user_password(self, user: User) -> bool:
19621949
return False
19631950

19641951
input_data = f'{user.username}:{enc_password}'.encode()
1965-
cmd = ['arch-chroot', '-S', str(self.target), 'chpasswd', '--encrypted']
1952+
cmd = self._chroot_argv('chpasswd', '--encrypted')
19661953

19671954
try:
19681955
run(cmd, input_data=input_data)
@@ -1974,7 +1961,7 @@ def set_user_password(self, user: User) -> bool:
19741961
def user_set_shell(self, user: str, shell: str) -> bool:
19751962
info(f'Setting shell for {user} to {shell}')
19761963

1977-
cmd = ['arch-chroot', '-S', str(self.target), 'chsh', '-s', shell, user]
1964+
cmd = self._chroot_argv('chsh', '-s', shell, user)
19781965
try:
19791966
run(cmd)
19801967
return True
@@ -1984,7 +1971,7 @@ def user_set_shell(self, user: str, shell: str) -> bool:
19841971

19851972
def chown(self, owner: str, path: str, options: list[str] | None = None) -> bool:
19861973
options = options or []
1987-
cmd = ['arch-chroot', '-S', str(self.target), 'chown', *options, owner, path]
1974+
cmd = self._chroot_argv('chown', *options, '--', owner, path)
19881975
try:
19891976
run(cmd)
19901977
return True

0 commit comments

Comments
 (0)