fix: reload uidManager with chrootuid/chrootgid from config#1752
Conversation
|
/packit test |
There was a problem hiding this comment.
Code Review
This pull request ensures that the uidManager is reloaded with the chrootuid and chrootgid specified in the configuration, rather than always using the calling user's identity. This involves moving the UID manager initialization and adding a new reload_uidmanager function. Review feedback pointed out that the new function and its variables should follow the project's snake_case naming convention and that the docstring should adhere to the Google style guide.
| def reload_uidmanager(uidmanager, config_opts): | ||
| """Re-create UidManager with chrootuid/chrootgid from config.""" | ||
| unprivuid = config_opts["chrootuid"] | ||
| unprivgid = config_opts.get("chrootgid", uidmanager.unprivGid) | ||
| return UidManager(unprivuid, unprivgid) |
There was a problem hiding this comment.
The function name reload_uidmanager and its internal variables unprivuid/unprivgid do not adhere to the project's naming conventions. Additionally, the docstring should follow the Google style for consistency with the rest of the project.
- Naming: Use
snake_casefor functions and variables (e.g.,reload_uid_manager,unpriv_uid). - Docstrings: Include
ArgsandReturnssections as per Google style.
def reload_uid_manager(uid_manager, config_opts):
"""Re-create UidManager with chrootuid/chrootgid from config.
Args:
uid_manager (UidManager): The current UID manager instance.
config_opts (dict): Configuration options containing chrootuid and optionally chrootgid.
Returns:
UidManager: A new UidManager instance initialized with the configured IDs.
"""
unpriv_uid = config_opts["chrootuid"]
unpriv_gid = config_opts.get("chrootgid", uid_manager.unprivGid)
return UidManager(unpriv_uid, unpriv_gid)The uidManager was initialized before config was loaded, so it always used the calling user's UID/GID for privilege dropping. When config specifies a different chrootuid/chrootgid, the uidManager needs to be reloaded with those values — otherwise operations run under the wrong identity, leading to permission errors in the chroot. Move uidManager initialization after command_parse() (which doesn't need to drop privileges) and reload it with chrootuid/chrootgid from config_opts right after config is loaded (loaded with dropped privileges). Fixes: rpm-software-management#1731 Assisted-By: Claude Code (claude-opus-4-6)
9000530 to
c1b5471
Compare
The uidManager was initialized before config was loaded, so it always used the calling user's UID/GID for privilege dropping. When config specifies a different chrootuid/chrootgid, the uidManager needs to be reloaded with those values — otherwise operations run under the wrong identity, leading to permission errors in the chroot.
Move uidManager initialization after command_parse() (which doesn't need to drop privileges) and reload it with chrootuid/chrootgid from config_opts right after config is loaded (loaded with dropped privileges).
Fixes: #1731
Assisted-By: Claude Code (claude-opus-4-6)