Add user to sudo group in container setup script#6
Add user to sudo group in container setup script#6sreeragh-s wants to merge 1 commit intotinkerhub:mainfrom
Conversation
📝 WalkthroughWalkthroughThe container initialization script now grants sudo group membership to SSH-configured users. A single line executing Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/containers.sh`:
- Line 86: The script currently always runs the command `usermod -aG sudo
{user}`, granting sudo to every SSH user; change this to be conditional on an
explicit per-user opt-in (e.g., `sudo: true` in containers.yml) so sudo is only
granted when requested. Update the logic that renders/iterates users in the
script to read the privilege flag from the user record (or an exported variable
for that user) and wrap the `usermod -aG sudo {user}` call in an if-check (only
execute when that user's sudo flag is true); ensure the check uses the same user
identifier used elsewhere in the script so you modify only the `usermod -aG sudo
{user}` execution path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| setup_cmd = f''' | ||
| set -e | ||
| id -u {user} >/dev/null 2>&1 || useradd -m -s /bin/bash {user} | ||
| usermod -aG sudo {user} |
There was a problem hiding this comment.
Gate sudo membership behind explicit config instead of granting it by default.
Line 86 unconditionally elevates every SSH user to sudo. Since containers/containers.yml does not define a privilege field, this silently broadens admin access for all configured accounts. Make sudo opt-in (e.g., sudo: true) and only run usermod when explicitly requested.
Suggested change
@@
- setup_cmd = f'''
+ allow_sudo = bool(c.get('sudo', False))
+ setup_cmd = f'''
set -e
id -u {user} >/dev/null 2>&1 || useradd -m -s /bin/bash {user}
-usermod -aG sudo {user}
+{"usermod -aG sudo " + user if allow_sudo else ""}
mkdir -p /home/{user}/.ssh
printf '%s\\n' '{key}' > /home/{user}/.ssh/authorized_keys
chmod 700 /home/{user}/.ssh
chmod 600 /home/{user}/.ssh/authorized_keys
chown -R {user}:{user} /home/{user}/.ssh
'''🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/containers.sh` at line 86, The script currently always runs the
command `usermod -aG sudo {user}`, granting sudo to every SSH user; change this
to be conditional on an explicit per-user opt-in (e.g., `sudo: true` in
containers.yml) so sudo is only granted when requested. Update the logic that
renders/iterates users in the script to read the privilege flag from the user
record (or an exported variable for that user) and wrap the `usermod -aG sudo
{user}` call in an if-check (only execute when that user's sudo flag is true);
ensure the check uses the same user identifier used elsewhere in the script so
you modify only the `usermod -aG sudo {user}` execution path.
Summary by CodeRabbit