shell scripting project #1 #12
Conversation
WalkthroughAdds a new interactive Bash script Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Admin as Admin (interactive)
participant Script as pam-main.sh
participant Sys as System (user/group/tar)
Admin->>Script: Run script / select option
alt Not root
Script->>Admin: "Must run as root" and exit
else Root (initial check passes)
Admin->>Script: Choose option (1..10)
alt 1: Add user
Script->>Admin: Prompt for full name
Admin->>Script: Provide name
Script->>Sys: useradd -m -s /bin/bash
Script->>Sys: generate password -> chpasswd -> passwd -e
Sys-->>Script: OK/ERR
Script-->>Admin: Result
else 2: Delete user
Script->>Sys: getent passwd (UID>=1000) -> list
Admin->>Script: Select & confirm username
Script->>Sys: userdel -r
Sys-->>Script: OK/ERR
Script-->>Admin: Result
else 3..7: Modify user (groups/shell/disable/enable/sudo)
Script->>Sys: usermod / chsh / group modifications
Sys-->>Script: OK/ERR
Script-->>Admin: Result
else 8: Add group
Script->>Sys: groupadd (if not exists)
Sys-->>Script: OK/ERR
Script-->>Admin: Result
else 9: Backup directories
Script->>Admin: Prompt absolute path
Admin->>Script: Provide path
Script->>Sys: tar -> /opt/dir_backups/<user>-<ts>.tar.gz
Sys-->>Script: OK/ERR
Script-->>Admin: Result
else 10: Exit
Script-->>Admin: Exit loop and terminate
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks (2 passed, 1 inconclusive)❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
Poem
✨ Finishing touches
🧪 Generate unit tests
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
day02/pam-main.sh (1)
1-154: Fix shebang and quoting issues
- day02/pam-main.sh line 1: replace
#/bin/bashwith a valid shebang (e.g.#!/usr/bin/env bash).- Quote every variable expansion (SC2086), e.g. use
"${username}","${grpname}","${dirback}".- Replace legacy backticks with
$(…)command substitutions (SC2006).- Add
-rto allreadinvocations to prevent backslash mangling (SC2162).- Use
[[ … ]]for conditional tests instead of[…]where recommended (SC2292).- Remove or utilize the unused
WHITEcolor constant (SC2034).
🧹 Nitpick comments (3)
day02/pam-main.sh (3)
10-10: Remove unused color constant WHITENot used anywhere; drop it or use consistently for output.
-WHITE='\033[1;37m'
73-81: Prefer usermod -s for shell changes; handle unsupported shells
- chsh may fail if shell not in /etc/shells; check errors.
- Quote variables and add a default else branch.
-if [ "$curr_shell" = "/bin/sh" ]; then - chsh -s /bin/bash "$username" +if [[ "$curr_shell" == "/bin/sh" ]]; then + usermod -s /bin/bash "$username" && \ echo "$username's shell is successfully changed to bash" echo "-------------------------------------------------" -elif [ "$curr_shell" = "/bin/bash" ]; then - chsh -s /bin/sh "$username" +elif [[ "$curr_shell" == "/bin/bash" ]]; then + usermod -s /bin/sh "$username" && \ echo "$username's shell is successfully changed to sh" echo "-----------------------------------------------" +else + echo "Unsupported current shell '$curr_shell'; no change made." fi
107-114: Mirror portability for enabling loginUse usermod -s and check success.
-echo "Changing $username shell to /bin/bash" -chsh -s /bin/bash "$username" +echo "Changing $username shell to /bin/bash" +usermod -s /bin/bash "$username" echo "$username's shell is successfully activated to bash"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
day02/pam-main.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
day02/pam-main.sh
[error] 1-1: Use #!, not just #, for the shebang.
(SC1113)
[warning] 10-10: WHITE appears unused. Verify use (or export if used externally).
(SC2034)
🔇 Additional comments (1)
day02/pam-main.sh (1)
1-154: Target distro and commands: confirm Debian/Ubuntu vs RHEL/CentOSYou mix Debian-only tools (deluser, addgroup) with portable ones. Confirm target(s) and adjust accordingly, or branch by detection.
Would you confirm the Linux distributions you want to support? If multi-distro, I can add a small helper that maps commands per OS (ID from /etc/os-release) and detects sudo/wheel, nologin path, etc.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
day02/pam-main.sh (2)
1-1: Malformed shebang; enable strict mode earlyCurrent line prevents execution under Bash and misses safe options.
-#/bin/bash +#!/usr/bin/env bash +set -Eeuo pipefail +IFS=$'\n\t'
74-86: Group selection mismatch: list source ≠ pick sourceYou list groups via getent (all groups) but pick via /etc/group filtered by GID>=1000, so indices don’t match. Result: wrong group may be chosen.
-echo "------------------Group List-------------------------" -getent group | awk -F: '{print $1}' | nl -w1 -s". " +echo "------------------Group List-------------------------" +getent group | awk -F: '{print $1}' | nl -w1 -s". " read -p "Select the existing user NUMBER: " choice read -p "Select the group NUMBER to be added: " gchoice -username=$(awk -F: '$3 >= 1000 {print $1}' /etc/passwd | sed -n "${choice}p") -grpname=$(awk -F: '$3 >= 1000 {print $1}' /etc/group | sed -n "${gchoice}p") +username="$(awk -F: '$3 >= 1000 {print $1}' /etc/passwd | sed -n "${choice}p")" +grpname="$(getent group | awk -F: '{print $1}' | sed -n "${gchoice}p")" if [[ -n "${username:-}" && -n "${grpname:-}" ]]; then
🧹 Nitpick comments (5)
day02/pam-main.sh (5)
94-107: Prompt mismatch and prefer usermod -s over chshPrompt asks for username but expects a number. Use usermod -s for consistency.
-read -p "Select the existing username: " choice +read -p "Select the existing user NUMBER: " choice username=$(awk -F: '$3 >= 1000 {print $1}' /etc/passwd | sed -n "${choice}p") curr_shell=$(getent passwd "$username" | cut -d: -f7) if [ "$curr_shell" = "/bin/sh" ]; then - chsh -s /bin/bash "$username" + usermod -s /bin/bash "$username" echo "$username's shell is successfully changed to bash" echo "-------------------------------------------------" elif [ "$curr_shell" = "/bin/bash" ]; then - chsh -s /bin/sh "$username" + usermod -s /bin/sh "$username" echo "$username's shell is successfully changed to sh" echo "-----------------------------------------------" fi
113-122: Consistent prompts and enable using usermod -sAlign prompt with numbered list and prefer usermod -s.
-read -p "Select the existing username: " choice +read -p "Select the existing user NUMBER: " choice @@ - echo "Changing $username shell to /bin/bash" - chsh -s /bin/bash "$username" + echo "Changing $username shell to /bin/bash" + usermod -s /bin/bash "$username" echo "$username's shell is successfully activated to bash"Also applies to: 130-139
147-153: Prompt mismatch for sudo group selectionMenu uses numbers; prompt says “Enter the existing username”. Fix text.
-read -p "Enter the existing username: " choice +read -p "Select the existing user NUMBER: " choice
169-179: Backup: verify selected user and absolute path; handle empty usernameMinor guardrails to avoid creating “_timestamp.tar.gz”.
username="$(awk -F: '$3 >= 1000 {print $1}' /etc/passwd | sed -n "${choice}p")" -read -r -p "Specify the absolute path of the directory to be backed up: " dirback -if [[ -d "$dirback" ]]; then +[[ -n "${username:-}" ]] || { echo "Invalid user selection."; break; } +read -r -p "Specify the absolute path of the directory to be backed up: " dirback +if [[ "$dirback" = /* && -d "$dirback" ]]; then ts="$(date +%F_%H-%M-%S)" tar -czf "/opt/dir_backups/${username}_$ts.tar.gz" -C "$dirback" . >/dev/null 2>&1 && \ echo "Backup completed successfully" || echo "Backup failed" else echo "Directory '$dirback' not found." fi
188-190: Exit cleanlyUsing break relies on loop structure; exit 0 is clearer.
- break; + exit 0
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
day02/pam-main.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
day02/pam-main.sh
[error] 1-1: Use #!, not just #, for the shebang.
(SC1113)
[error] 16-16: Couldn't parse this while loop. Fix to allow more checks.
(SC1073)
[error] 16-16: Couldn't find 'done' for this 'do'.
(SC1061)
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (7)
day02/pam-main.sh (7)
1-1: Fix malformed shebang and enable safe Bash options earlyUse a valid shebang and fail fast with safe defaults.
Apply:
-#/bin/bash +#!/usr/bin/env bash +set -Eeuo pipefail +IFS=$'\n\t'
16-20: Remove tight no-op loop that busy-spins before the menuThis while-loop repeatedly re-checks EUID and does nothing when root.
-while true; do -if [[ $EUID -ne 0 ]]; then - echo "This script must be run as root (sudo)."; exit 1 -fi -done
21-21: Run the root check once before entering the menu loopPlace EUID check above the main loop.
+if [[ $EUID -ne 0 ]]; then + echo "This script must be run as root (sudo)." >&2 + exit 1 +fi while true; do
82-90: Align prompts with number selection, validate inputs, and pick group from the same listFix prompt text, add numeric checks, and use
getent groupfor selection (list and pick source should match).-read -p "Select the existing user NUMBER: " choice -read -p "Select the group NUMBER to be added: " gchoice -username=$(awk -F: '$3 >= 1000 {print $1}' /etc/passwd | sed -n "${choice}p") -grpname=$(awk -F: '$3 >= 1000 {print $1}' /etc/group | sed -n "${gchoice}p") -if [[ -n "${username:-}" && -n "${grpname:-}" ]]; then - usermod -aG "$grpname" "$username" && echo "$username added to group $grpname" -else - echo "Invalid selection." -fi +read -r -p "Select the existing user NUMBER: " choice +[[ "$choice" =~ ^[0-9]+$ ]] || { echo "Invalid user selection."; continue; } +read -r -p "Select the group NUMBER to be added: " gchoice +[[ "$gchoice" =~ ^[0-9]+$ ]] || { echo "Invalid group selection."; continue; } +username="$(awk -F: '$3 >= 1000 {print $1}' /etc/passwd | sed -n "${choice}p")" +grpname="$(getent group | awk -F: '{print $1}' | sed -n "${gchoice}p")" +if [[ -n "${username:-}" && -n "${grpname:-}" ]]; then + usermod -aG "$grpname" "$username" && echo "$username added to group $grpname" +else + echo "Invalid selection." +fi
100-111: Prompt/selection mismatch; add validation before toggling shellPrompt says “username” but expects a number; also guard empty picks.
- read -p "Select the existing username: " choice - username=$(awk -F: '$3 >= 1000 {print $1}' /etc/passwd | sed -n "${choice}p") + read -r -p "Select the existing user NUMBER: " choice + [[ "$choice" =~ ^[0-9]+$ ]] || { echo "Invalid selection."; continue; } + username="$(awk -F: '$3 >= 1000 {print $1}' /etc/passwd | sed -n "${choice}p")" + [[ -n "${username:-}" ]] || { echo "Invalid selection."; continue; } - curr_shell=$(getent passwd "$username" | cut -d: -f7) - if [ "$curr_shell" = "/bin/sh" ]; then + curr_shell=$(getent passwd "$username" | cut -d: -f7) + if [[ "$curr_shell" == "/bin/sh" ]]; then chsh -s /bin/bash "$username" echo "$username's shell is successfully changed to bash" echo "-------------------------------------------------" - elif [ "$curr_shell" = "/bin/bash" ]; then + elif [[ "$curr_shell" == "/bin/bash" ]]; then chsh -s /bin/sh "$username" echo "$username's shell is successfully changed to sh" echo "-----------------------------------------------" fi
153-157: Prompt/selection mismatch and missing validation for sudo-group addSwitch to number-based prompt and validate.
- read -p "Enter the existing username: " choice - username=$(awk -F: '$3 >= 1000 {print $1}' /etc/passwd | sed -n "${choice}p") + read -r -p "Select the existing user NUMBER: " choice + [[ "$choice" =~ ^[0-9]+$ ]] || { echo "Invalid selection."; continue; } + username="$(awk -F: '$3 >= 1000 {print $1}' /etc/passwd | sed -n "${choice}p")" + [[ -n "${username:-}" ]] || { echo "Invalid selection."; continue; }
176-185: Harden backup input handling and filenameValidate selection, guard empty username, and avoid empty prefix in archive name.
- read -r -p "Select the user NUMBER to associate with the backup: " choice - username="$(awk -F: '$3 >= 1000 {print $1}' /etc/passwd | sed -n "${choice}p")" - read -r -p "Specify the absolute path of the directory to be backed up: " dirback - if [[ -d "$dirback" ]]; then - ts="$(date +%F_%H-%M-%S)" - tar -czf "/opt/dir_backups/${username}_$ts.tar.gz" -C "$dirback" . >/dev/null 2>&1 && \ + read -r -p "Select the user NUMBER to associate with the backup: " choice + [[ "$choice" =~ ^[0-9]+$ ]] || { echo "Invalid selection."; continue; } + username="$(awk -F: '$3 >= 1000 {print $1}' /etc/passwd | sed -n "${choice}p")" + [[ -n "${username:-}" ]] || { echo "Invalid user selection."; continue; } + read -r -p "Specify the absolute path of the directory to be backed up: " dirback + if [[ -d "$dirback" ]]; then + ts="$(date +%F_%H-%M-%S)" + tar -czf "/opt/dir_backups/${username:-unknown}_$ts.tar.gz" -C "$dirback" . >/dev/null 2>&1 && \ echo "Backup completed successfully" || echo "Backup failed"
🧹 Nitpick comments (2)
day02/pam-main.sh (2)
44-50: Avoid printing plaintext temporary passwords to the terminalStore in a root-only log instead of echoing the secret.
- echo "$username created. Temporary password: $randpass (will be forced to change on first login)" + log="/root/pam-main-passwords.log" + { umask 077; printf '%s:%s\n' "$username" "$randpass" >> "$log"; } + echo "$username created. Temporary password saved to $log (will be forced to change on first login)"
9-9: Remove unused color variable (ShellCheck SC2034)
WHITEis defined but not used.-WHITE='\033[1;37m'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
day02/pam-main.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
day02/pam-main.sh
[error] 1-1: Use #!, not just #, for the shebang.
(SC1113)
[warning] 9-9: WHITE appears unused. Verify use (or export if used externally).
(SC2034)
🔇 Additional comments (1)
day02/pam-main.sh (1)
121-126: Good portability on login disableDetecting
nologindynamically and usingusermod -sis portable and correct. LGTM.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (6)
day02/pam-main.sh (6)
1-1: Fix malformed shebang and enable safe Bash optionsScript won’t run under Bash as written; add strict mode early.
Apply:
-#/bin/bash +#!/usr/bin/env bash +set -Eeuo pipefail +IFS=$'\n\t'
16-20: Remove infinite pre-loop; perform root check once before the menuThis while-true block busy-loops forever for root users.
Apply:
-while true; do -if [[ $EUID -ne 0 ]]; then - echo "This script must be run as root (sudo)."; exit 1 -fi -done +if [[ $EUID -ne 0 ]]; then + echo "This script must be run as root (sudo)." >&2 + exit 1 +fi
38-38: Don’t exit the menu when the user already exists; continue insteadbreak exits the whole menu loop.
- echo "User '$username' already exists."; echo "-----------------------------------------------"; unset username; break + echo "User '$username' already exists."; echo "-----------------------------------------------"; unset username; continue
81-86: Fix group selection source mismatch (list via getent, select via /etc/group)You list groups with getent but index into /etc/group (different orders) → wrong group.
-read -p "Select the existing user NUMBER: " choice -read -p "Select the group NUMBER to be added: " gchoice -username=$(awk -F: '$3 >= 1000 {print $1}' /etc/passwd | sed -n "${choice}p") -grpname=$(awk -F: '$3 >= 1000 {print $1}' /etc/group | sed -n "${gchoice}p") +read -r -p "Select the existing user NUMBER: " choice +[[ "$choice" =~ ^[0-9]+$ ]] || { echo "Invalid user selection."; continue; } +read -r -p "Select the group NUMBER to be added: " gchoice +[[ "$gchoice" =~ ^[0-9]+$ ]] || { echo "Invalid group selection."; continue; } +username="$(awk -F: '$3 >= 1000 {print $1}' /etc/passwd | sed -n "${choice}p")" +grpname="$(getent group | awk -F: '{print $1}' | sed -n "${gchoice}p")"
118-126: Validate numeric selection for “Disable Login”Match prompt style and guard empty username.
-read -p "Select the existing username: " choice -username=$(awk -F: '$3 >= 1000 {print $1}' /etc/passwd | sed -n "${choice}p") -if getent passwd "$username" > /dev/null; then +read -r -p "Select the existing user NUMBER: " choice +[[ "$choice" =~ ^[0-9]+$ ]] || { echo "Invalid selection."; continue; } +username="$(awk -F: '$3 >= 1000 {print $1}' /etc/passwd | sed -n "${choice}p")" +if [[ -n "${username:-}" ]] && getent passwd "$username" > /dev/null; then
151-158: Add sudo: use NUMBER prompt + validation and guard empty usernamePrevents accidental empty add.
-read -p "Enter the existing username: " choice -username=$(awk -F: '$3 >= 1000 {print $1}' /etc/passwd | sed -n "${choice}p") +read -r -p "Select the existing user NUMBER: " choice +[[ "$choice" =~ ^[0-9]+$ ]] || { echo "Invalid selection."; continue; } +username="$(awk -F: '$3 >= 1000 {print $1}' /etc/passwd | sed -n "${choice}p")" admin_group="$(getent group sudo >/dev/null 2>&1 && echo sudo || echo wheel)" -usermod -aG "$admin_group" "$username" -echo "$username added to $admin_group group." +if [[ -n "${username:-}" ]]; then + usermod -aG "$admin_group" "$username" && echo "$username added to $admin_group group." +else + echo "Invalid selection." +fi
🧹 Nitpick comments (2)
day02/pam-main.sh (2)
176-183: Backups: validate selected user and show archive pathMinor UX hardening.
-username="$(awk -F: '$3 >= 1000 {print $1}' /etc/passwd | sed -n "${choice}p")" +username="$(awk -F: '$3 >= 1000 {print $1}' /etc/passwd | sed -n "${choice}p")" +[[ -n "${username:-}" ]] || { echo "Invalid selection."; continue; } read -r -p "Specify the absolute path of the directory to be backed up: " dirback if [[ -d "$dirback" ]]; then ts="$(date +%F_%H-%M-%S)" - tar -czf "/opt/dir_backups/${username}_$ts.tar.gz" -C "$dirback" . >/dev/null 2>&1 && \ - echo "Backup completed successfully" || echo "Backup failed" + archive="/opt/dir_backups/${username}_$ts.tar.gz" + tar -czf "$archive" -C "$dirback" . >/dev/null 2>&1 && \ + echo "Backup completed: $archive" || echo "Backup failed"
9-9: Remove unused color variable WHITEClean up per ShellCheck SC2034.
-WHITE='\033[1;37m'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
day02/pam-main.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
day02/pam-main.sh
[error] 1-1: Use #!, not just #, for the shebang.
(SC1113)
[warning] 9-9: WHITE appears unused. Verify use (or export if used externally).
(SC2034)
🔇 Additional comments (2)
day02/pam-main.sh (2)
121-125: Good: portable nologin detection and consistent usermod usageThis improves distro compatibility.
54-67: Good: safer delete flow with validation and self-delete guardNumeric validation, confirmation, and userdel -r are appropriate.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
day02/pam-main.sh (5)
1-1: Fix shebang and enable strict mode early
Use a valid shebang and safe bash options at the top.-#/bin/bash +#!/usr/bin/env bash +set -Eeuo pipefail +IFS=$'\n\t'
16-21: Remove infinite pre-loop; run root check once, then enter menu
Current while/do around the EUID check spins forever (no body/break) and prevents reaching the menu.-while true; do -if [[ $EUID -ne 0 ]]; then - echo "This script must be run as root (sudo)."; exit 1 -fi -done -while true; do +if [[ $EUID -ne 0 ]]; then + echo "This script must be run as root (sudo)." >&2 + exit 1 +fi +while true; do
35-39: Don’t exit the menu when user exists; continue instead
Using break exits the whole menu loop.- echo "User '$username' already exists."; echo "-----------------------------------------------"; unset username; break + echo "User '$username' already exists." + echo "-----------------------------------------------" + unset username + continue
155-166: Add-to-sudo: prompt mismatch and missing validation
Input expects a NUMBER but prompt asks for username; also validate selection and handle empty username.-read -p "Enter the existing username: " choice -username=$(awk -F: '$3 >= 1000 {print $1}' /etc/passwd | sed -n "${choice}p") +read -r -p "Select the existing user NUMBER: " choice +[[ "$choice" =~ ^[0-9]+$ ]] || { echo "Invalid selection."; continue; } +username="$(awk -F: '$3 >= 1000 {print $1}' /etc/passwd | sed -n "${choice}p")" +if [[ -z "${username:-}" ]]; then echo "Invalid selection."; continue; fi admin_group="$(getent group sudo >/dev/null 2>&1 && echo sudo || echo wheel)" usermod -aG "$admin_group" "$username" echo "$username added to $admin_group group."
121-129: Prompt/validation mismatch for ‘Disable Login’
Prompt says “username” but expects a NUMBER; also missing numeric validation. Align prompt and validate.-read -p "Select the existing username: " choice -username=$(awk -F: '$3 >= 1000 {print $1}' /etc/passwd | sed -n "${choice}p") +read -r -p "Select the existing user NUMBER: " choice +[[ "$choice" =~ ^[0-9]+$ ]] || { echo "Invalid selection."; continue; } +username="$(awk -F: '$3 >= 1000 {print $1}' /etc/passwd | sed -n "${choice}p")" -if getent passwd "$username" > /dev/null; then +if [[ -n "${username:-}" ]] && getent passwd "$username" > /dev/null; then
🧹 Nitpick comments (4)
day02/pam-main.sh (4)
139-147: Minor: make prompt consistent with NUMBER input
Message says “user” but you require a number; adjust for clarity.-read -rp "Select the existing user: " choice +read -rp "Select the existing user NUMBER: " choiceAlso applies to: 141-141
178-187: Backup: validate selection and guard empty username
Add NUMBER validation and ensure username is non-empty before naming the archive.-read -r -p "Select the user NUMBER to associate with the backup: " choice -username="$(awk -F: '$3 >= 1000 {print $1}' /etc/passwd | sed -n "${choice}p")" +read -r -p "Select the user NUMBER to associate with the backup: " choice +[[ "$choice" =~ ^[0-9]+$ ]] || { echo "Invalid selection."; continue; } +username="$(awk -F: '$3 >= 1000 {print $1}' /etc/passwd | sed -n "${choice}p")" @@ - ts="$(date +%F_%H-%M-%S)" - tar -czf "/opt/dir_backups/${username}_$ts.tar.gz" -C "$dirback" . >/dev/null 2>&1 && \ + ts="$(date +%F_%H-%M-%S)" + name="${username:-unknown}" + tar -czf "/opt/dir_backups/${name}_$ts.tar.gz" -C "$dirback" . >/dev/null 2>&1 && \Also applies to: 180-186
9-9: Remove unused color or use it
WHITE is defined but unused.-WHITE='\033[1;37m' +# WHITE='\033[1;37m' # unused
22-33: Optional: factor repeated listing/selection into helpers
You duplicate “list users”, “validate number”, and “map selection” in several cases. Extract small functions to reduce mistakes and drift.Happy to propose a minimal helper set (list_users, select_from_list, list_groups) if you want.
Also applies to: 199-203
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
day02/pam-main.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
day02/pam-main.sh
[error] 1-1: Use #!, not just #, for the shebang.
(SC1113)
[warning] 9-9: WHITE appears unused. Verify use (or export if used externally).
(SC2034)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (6)
day02/pam-main.sh (6)
38-38: Don’t exit menu when user exists; continue insteadKeeps UX consistent with other invalid selections.
- echo "User '$username' already exists."; echo "-----------------------------------------------"; unset username; break + echo "User '$username' already exists."; echo "-----------------------------------------------"; unset username; continue
1-1: Fix malformed shebang and enable strict mode earlyScript won’t run under Bash as-is; also add safe options.
-#/bin/bash +#!/usr/bin/env bash +set -Eeuo pipefail +IFS=$'\n\t'
81-89: Fix group selection: validate gchoice and resolve from same list sourceCurrently validates $choice twice and indexes /etc/group (GID>=1000) while listing via getent; indexes can mismatch.
- getent group | awk -F: '{print $1}' | nl -w1 -s". " + getent group | awk -F: '{print $1}' | nl -w1 -s". " @@ - read -p "Select the group NUMBER to be added: " gchoice - [[ "$choice" =~ ^[0-9]+$ ]] || { echo "Invalid selection."; continue; } + read -p "Select the group NUMBER to be added: " gchoice + [[ "$gchoice" =~ ^[0-9]+$ ]] || { echo "Invalid selection."; continue; } @@ - grpname=$(awk -F: '$3 >= 1000 {print $1}' /etc/group | sed -n "${gchoice}p") - if [[ -n "${username}" && -n "${grpname}" ]]; then + grpname="$(getent group | awk -F: '{print $1}' | sed -n "${gchoice}p")" + if [[ -n "${username:-}" && -n "${grpname:-}" ]]; then usermod -aG "$grpname" "$username" && echo "$username added to group $grpname"
125-135: Option 5 (Disable login): prompt/validation mismatch; guard selectionPrompt asks for username but expects a number; add numeric validation and consistent flow.
- read -p "Select the existing username: " choice - username=$(awk -F: '$3 >= 1000 {print $1}' /etc/passwd | sed -n "${choice}p") - if getent passwd "$username" > /dev/null; then + read -r -p "Select the existing user NUMBER: " choice + [[ "$choice" =~ ^[0-9]+$ ]] || { echo "Invalid selection."; continue; } + username="$(awk -F: '$3 >= 1000 {print $1}' /etc/passwd | sed -n "${choice}p")" + if [[ -n "${username:-}" ]] && getent passwd "$username" > /dev/null; then nologin_path="$(command -v nologin || echo /sbin/nologin)" echo "Changing $username shell to $nologin_path" usermod -s "$nologin_path" "$username" echo "$username's login disabled." echo "-------------------------------" else - echo "User $username not found!" + echo "Invalid selection." fi
159-163: Option 7 (Add to sudo/wheel): prompt/validation mismatch; add checksReads NUMBER but prompts for username; add validation and skip on invalid.
- read -p "Enter the existing username: " choice - username=$(awk -F: '$3 >= 1000 {print $1}' /etc/passwd | sed -n "${choice}p") - admin_group="$(getent group sudo >/dev/null 2>&1 && echo sudo || echo wheel)" - usermod -aG "$admin_group" "$username" - echo "$username added to $admin_group group." + read -r -p "Select the existing user NUMBER: " choice + [[ "$choice" =~ ^[0-9]+$ ]] || { echo "Invalid selection."; continue; } + username="$(awk -F: '$3 >= 1000 {print $1}' /etc/passwd | sed -n "${choice}p")" + if [[ -n "${username:-}" ]]; then + admin_group="$(getent group sudo >/dev/null 2>&1 && echo sudo || echo wheel)" + usermod -aG "$admin_group" "$username" && echo "$username added to $admin_group group." + else + echo "Invalid selection." + fi
16-21: Remove infinite root-check loop; check once before menuCurrent while/do loop spins forever when EUID==0 and never reaches the menu.
-while true; do -if [[ $EUID -ne 0 ]]; then - echo "This script must be run as root (sudo)."; exit 1 -fi -done +if [[ $EUID -ne 0 ]]; then + echo "This script must be run as root (sudo)." >&2; exit 1 +fi
🧹 Nitpick comments (2)
day02/pam-main.sh (2)
182-189: Option 9 (Backup): validate selection; handle empty username; safer filenameAvoid empty username and ensure numeric selection. Optionally keep “unknown” prefix.
- read -r -p "Select the user NUMBER to associate with the backup: " choice - username="$(awk -F: '$3 >= 1000 {print $1}' /etc/passwd | sed -n "${choice}p")" + read -r -p "Select the user NUMBER to associate with the backup: " choice + [[ "$choice" =~ ^[0-9]+$ ]] || { echo "Invalid selection."; continue; } + username="$(awk -F: '$3 >= 1000 {print $1}' /etc/passwd | sed -n "${choice}p")" @@ - tar -czf "/opt/dir_backups/${username}_$ts.tar.gz" -C "$dirback" . >/dev/null 2>&1 && \ + tar -czf "/opt/dir_backups/${username:-unknown}_$ts.tar.gz" -C "$dirback" . >/dev/null 2>&1 && \ echo "Backup completed successfully" || echo "Backup failed"
9-9: Remove unused WHITE color var (SC2034)Silences ShellCheck warning.
-WHITE='\033[1;37m'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
day02/pam-main.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
day02/pam-main.sh
[error] 1-1: Use #!, not just #, for the shebang.
(SC1113)
[warning] 9-9: WHITE appears unused. Verify use (or export if used externally).
(SC2034)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
day02/pam-main.sh (5)
1-1: Fix shebang and enable strict mode earlyUse a valid shebang and turn on safe Bash options to fail fast.
Apply:
-#/bin/bash +#!/usr/bin/env bash +set -Eeuo pipefail +IFS=$'\n\t'
33-38: Don’t exit the menu when user already exists; use continuebreak exits the main loop; you want to return to the menu.
- echo "User '$username' already exists."; echo "-----------------------------------------------"; unset username; break + echo "User '$username' already exists." + echo "-----------------------------------------------" + unset username + continue
76-87: Group selection: validate group number and resolve from the same list sourceYou validate choice, but not gchoice; also you list groups via getent yet resolve via /etc/group, which can index-mismatch.
getent group | awk -F: '$3 >= 1000 {print $1}' | nl -w1 -s". " -read -p "Select the existing user NUMBER: " choice - [[ "$choice" =~ ^[0-9]+$ ]] || { echo "Invalid selection."; continue; } -read -p "Select the group NUMBER to be added: " gchoice - [[ "$choice" =~ ^[0-9]+$ ]] || { echo "Invalid selection."; continue; } -username=$(awk -F: '$3 >= 1000 {print $1}' /etc/passwd | sed -n "${choice}p") -grpname=$(awk -F: '$3 >= 1000 {print $1}' /etc/group | sed -n "${gchoice}p") +read -r -p "Select the existing user NUMBER: " choice +[[ "$choice" =~ ^[0-9]+$ ]] || { echo "Invalid selection."; continue; } +read -r -p "Select the group NUMBER to be added: " gchoice +[[ "$gchoice" =~ ^[0-9]+$ ]] || { echo "Invalid selection."; continue; } +username="$(awk -F: '$3 >= 1000 {print $1}' /etc/passwd | sed -n "${choice}p")" +grpname="$(getent group | awk -F: '$3 >= 1000 {print $1}' | sed -n "${gchoice}p")" -if [[ -n "${username}" && -n "${grpname}" ]]; then +if [[ -n "${username:-}" && -n "${grpname:-}" ]]; then
155-163: Prompt/selection mismatch and missing validationAsks for username but uses numeric index; add validation and ensure username resolved.
-read -p "Enter the existing username: " choice -username=$(awk -F: '$3 >= 1000 {print $1}' /etc/passwd | sed -n "${choice}p") +read -r -p "Select the existing user NUMBER: " choice +[[ "$choice" =~ ^[0-9]+$ ]] || { echo "Invalid selection."; continue; } +username="$(awk -F: '$3 >= 1000 {print $1}' /etc/passwd | sed -n "${choice}p")" +[[ -n "${username:-}" ]] || { echo "Invalid selection."; continue; } admin_group="$(getent group sudo >/dev/null 2>&1 && echo sudo || echo wheel)" usermod -aG "$admin_group" "$username"
121-129: Prompt/selection mismatch: expects NUMBER but asks for username; add validationCurrently asks for “username” but uses sed index; this will break on non-numeric input.
-read -p "Select the existing username: " choice -username=$(awk -F: '$3 >= 1000 {print $1}' /etc/passwd | sed -n "${choice}p") +read -r -p "Select the existing user NUMBER: " choice +[[ "$choice" =~ ^[0-9]+$ ]] || { echo "Invalid selection."; continue; } +username="$(awk -F: '$3 >= 1000 {print $1}' /etc/passwd | sed -n "${choice}p")" -if getent passwd "$username" > /dev/null; then +if [[ -n "${username:-}" ]] && getent passwd "$username" > /dev/null; then
🧹 Nitpick comments (7)
day02/pam-main.sh (7)
6-12: Remove unused color or use itWHITE is defined but not used; drop it or use it to keep things tidy. ShellCheck SC2034.
-WHITE='\033[1;37m'
16-19: Root check LGTMRuns once before the loop and exits non-root. Consider sending message to stderr, but OK as-is.
- echo "This script must be run as root (sudo)." + echo "This script must be run as root (sudo)." >&2
31-31: Read without backslash escapesUse -r to avoid interpreting backslashes in input.
-read -p "Select the option: " choice +read -r -p "Select the option: " choice
39-46: Avoid printing passwords to the console; store securely or prompt to setEchoing the temporary password is risky (shoulder-surfing/logging). Prefer saving to a root-readable file or forcing the admin to set it interactively.
- printf '%s:%s\n' "$username" "$randpass" | chpasswd + printf '%s:%s\n' "$username" "$randpass" | chpasswd passwd --expire "$username" >/dev/null - echo "$username created. Temporary password: $randpass (will be forced to change on first login)" + passfile="/root/user_temp_passwords.csv" + umask 077 + printf '%s,%s\n' "$username" "$randpass" >> "$passfile" + echo "$username created. Temporary password stored at $passfile (will be forced to change on first login)"
51-70: User deletion flow looks solid; consider terminating active sessionsOptionally terminate processes/sessions before deletion to avoid leftover processes.
- if userdel -r "$username" >/dev/null 2>&1; then + loginctl terminate-user "$username" >/dev/null 2>&1 || pkill -KILL -u "$username" >/dev/null 2>&1 || true + if userdel -r "$username" >/dev/null 2>&1; then echo "$username deleted."
98-120: Shell toggle: optionally restrict to valid shellsConsider checking against /etc/shells before toggling, and handle usermod failures explicitly.
- if [[ "$curr_shell" == "/bin/sh" ]]; then + if ! grep -qxF "/bin/bash" /etc/shells || ! grep -qxF "/bin/sh" /etc/shells; then + echo "Warning: /etc/shells does not include expected shells." + fi + if [[ "$curr_shell" == "/bin/sh" ]]; then usermod -s /bin/bash "$username" && echo "$username's shell changed to bash"
139-146: Align prompt with numeric selectionThe input is validated as a number; update the prompt to match.
-read -rp "Select the existing user: " choice +read -rp "Select the existing user NUMBER: " choice
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
day02/pam-main.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
day02/pam-main.sh
[error] 1-1: Use #!, not just #, for the shebang.
(SC1113)
[warning] 9-9: WHITE appears unused. Verify use (or export if used externally).
(SC2034)
🔇 Additional comments (1)
day02/pam-main.sh (1)
200-204: Default branch OKInvalid options handled and loop continues. Good.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
day02/pam-main.sh (4)
1-1: Fix shebang and enable strict mode earlyCurrent shebang is malformed; add a valid one and harden execution.
-#/bin/bash +#!/usr/bin/env bash +set -Eeuo pipefail +IFS=$'\n\t'
34-38: Don’t exit the menu when user existsUsing break here exits the main loop. Use continue so the menu keeps running.
- echo "User '$username' already exists."; echo "-----------------------------------------------"; unset username; break + echo "User '$username' already exists." + echo "-----------------------------------------------" + unset username + continue
80-87: Fix group selection validation and source mismatch
- Validation checks $choice twice; second should validate $gchoice.
- You list groups via getent but resolve from /etc/group; indexes can diverge. Resolve from the same list you printed.
- getent group | awk -F: '$3 >= 1000 {print $1}' | nl -w1 -s". " - read -p "Select the existing user NUMBER: " choice - [[ "$choice" =~ ^[0-9]+$ ]] || { echo "Invalid selection."; continue; } - read -p "Select the group NUMBER to be added: " gchoice - [[ "$choice" =~ ^[0-9]+$ ]] || { echo "Invalid selection."; continue; } - username=$(awk -F: '$3 >= 1000 {print $1}' /etc/passwd | sed -n "${choice}p") - grpname=$(awk -F: '$3 >= 1000 {print $1}' /etc/group | sed -n "${gchoice}p") + groups_list="$(getent group | awk -F: '{print $1}')" + echo "$groups_list" | nl -w1 -s". " + read -r -p "Select the existing user NUMBER: " choice + [[ "$choice" =~ ^[0-9]+$ ]] || { echo "Invalid selection."; continue; } + read -r -p "Select the group NUMBER to be added: " gchoice + [[ "$gchoice" =~ ^[0-9]+$ ]] || { echo "Invalid selection."; continue; } + username="$(awk -F: '$3 >= 1000 {print $1}' /etc/passwd | sed -n "${choice}p")" + grpname="$(printf '%s\n' "$groups_list" | sed -n "${gchoice}p")"
156-163: Add sudo/wheel: prompt/validation and empty-username guardAdd numeric validation and ensure a valid username before usermod.
- read -p "Enter the existing username: " choice - username=$(awk -F: '$3 >= 1000 {print $1}' /etc/passwd | sed -n "${choice}p") - admin_group="$(getent group sudo >/dev/null 2>&1 && echo sudo || echo wheel)" - usermod -aG "$admin_group" "$username" - echo "$username added to $admin_group group." + read -r -p "Select the existing user NUMBER: " choice + [[ "$choice" =~ ^[0-9]+$ ]] || { echo "Invalid selection."; continue; } + username="$(awk -F: '$3 >= 1000 {print $1}' /etc/passwd | sed -n "${choice}p")" + if [[ -n "${username:-}" ]]; then + admin_group="$(getent group sudo >/dev/null 2>&1 && echo sudo || echo wheel)" + usermod -aG "$admin_group" "$username" && echo "$username added to $admin_group group." + else + echo "Invalid selection." + fi
🧹 Nitpick comments (3)
day02/pam-main.sh (3)
9-9: Remove unused color variable WHITEIt’s defined but never used. Drop it to avoid noise.
-WHITE='\033[1;37m'
142-147: Enable login: match prompt wording with behaviorMinor: the prompt should say NUMBER to align with selection style.
- read -rp "Select the existing user: " choice + read -rp "Select the existing user NUMBER: " choice
52-180: Prefer NSS-aware enumeration (getent) over parsing /etc/passwd directlyIf the system uses LDAP/SSSD, parsing /etc/passwd may miss users. Consider getent passwd and filtering by UID. Applies to all user lists.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
day02/pam-main.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
day02/pam-main.sh
[error] 1-1: Use #!, not just #, for the shebang.
(SC1113)
[warning] 9-9: WHITE appears unused. Verify use (or export if used externally).
(SC2034)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (5)
day02/pam-main.sh (5)
13-19: Run root check before banner and print errors to stderrFail fast before UI output; send the error to stderr.
-echo "------------------------------------------------------" -echo -e "=======\033[45m Welcome to Login Management System \033[0m=======" -echo "------------------------------------------------------" -if [[ $EUID -ne 0 ]]; then - echo "This script must be run as root (sudo)." - exit 1 -fi +if [[ $EUID -ne 0 ]]; then + echo "This script must be run as root (sudo)." >&2 + exit 1 +fi +echo "------------------------------------------------------" +echo -e "=======\033[45m Welcome to Login Management System \033[0m=======" +echo "------------------------------------------------------"
1-1: Fix malformed shebang and enable strict modeCurrent first line won’t invoke Bash. Add a valid shebang and safe Bash options early.
Apply:
-#/bin/bash +#!/usr/bin/env bash +set -Eeuo pipefail +IFS=$'\n\t'
33-51: Don’t exit the menu when user exists; use continueUsing break here quits the main loop.
- echo "User '$username' already exists."; echo "-----------------------------------------------"; unset username; break + echo "User '$username' already exists."; echo "-----------------------------------------------"; unset username; continue
157-165: Fix prompt/validation for sudo group additionPrompt asks for username but code expects a number; also missing numeric validation and empty check.
-awk -F: '$3 >= 1000 {print $1}' /etc/passwd | nl -w1 -s". " #Filtering only normal users -read -p "Enter the existing username: " choice -username=$(awk -F: '$3 >= 1000 {print $1}' /etc/passwd | sed -n "${choice}p") +awk -F: '$3 >= 1000 {print $1}' /etc/passwd | nl -w1 -s". " +read -r -p "Select the existing user NUMBER: " choice +[[ "$choice" =~ ^[0-9]+$ ]] || { echo "Invalid selection."; continue; } +username="$(awk -F: '$3 >= 1000 {print $1}' /etc/passwd | sed -n "${choice}p")" +[[ -n "${username:-}" ]] || { echo "Invalid selection."; continue; } admin_group="$(getent group sudo >/dev/null 2>&1 && echo sudo || echo wheel)" usermod -aG "$admin_group" "$username" echo "$username added to $admin_group group."
81-90: Fix validation bug and unify group source
- gchoice is never validated (copy/paste uses $choice again).
- You list groups via getent but resolve from /etc/group; indexes can diverge.
- getent group | awk -F: '$3 >= 1000 {print $1}' | nl -w1 -s". " + getent group | awk -F: '$3 >= 1000 {print $1}' | nl -w1 -s". " read -p "Select the existing user NUMBER: " choice [[ "$choice" =~ ^[0-9]+$ ]] || { echo "Invalid selection."; continue; } read -p "Select the group NUMBER to be added: " gchoice - [[ "$choice" =~ ^[0-9]+$ ]] || { echo "Invalid selection."; continue; } + [[ "$gchoice" =~ ^[0-9]+$ ]] || { echo "Invalid selection."; continue; } -username=$(awk -F: '$3 >= 1000 {print $1}' /etc/passwd | sed -n "${choice}p") -grpname=$(awk -F: '$3 >= 1000 {print $1}' /etc/group | sed -n "${gchoice}p") +username="$(awk -F: '$3 >= 1000 {print $1}' /etc/passwd | sed -n "${choice}p")" +grpname="$(getent group | awk -F: '$3 >= 1000 {print $1}' | sed -n "${gchoice}p")" - if [[ -n "${username}" && -n "${grpname}" ]]; then + if [[ -n "${username:-}" && -n "${grpname:-}" ]]; then
🧹 Nitpick comments (3)
day02/pam-main.sh (3)
9-9: Remove unused color variable WHITEIt’s defined but never used (SC2034).
-WHITE='\033[1;37m'
129-133: Add robust nologin fallbackIf nologin isn’t available, fall back to /bin/false.
- nologin_path="$(command -v nologin || echo /sbin/nologin)" + nologin_path="$(command -v nologin || { [[ -x /usr/sbin/nologin ]] && echo /usr/sbin/nologin || echo /bin/false; })"
144-146: Prompt consistency: ask for NUMBER explicitlyAligns with other menu prompts.
- read -rp "Select the existing user: " choice + read -rp "Select the existing user NUMBER: " choice
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
day02/pam-main.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
day02/pam-main.sh
[error] 1-1: Use #!, not just #, for the shebang.
(SC1113)
[warning] 9-9: WHITE appears unused. Verify use (or export if used externally).
(SC2034)
🔇 Additional comments (4)
day02/pam-main.sh (4)
52-76: User deletion flow looks solidNumeric validation, self-deletion guard with SUDO_USER fallback, confirm, and userdel -r are correct.
100-121: Shell toggle logic is consistent and safeGood use of getent and usermod -s with clear messages.
170-179: Group creation flow looks goodExistence check + groupadd with clear messages.
180-200: Backup flow is well-hardenedValidates user selection and directory, timestamps the archive, and quotes paths.
1 participant
Hi Shubham,
As mentioned in the project #1, Shell Script for User Management and Backup in Linux. I have completed the same.
File name is - pam-main.sh under day02
It consists of:
Also added comments wherever possible to give appropriate explanation. please review and let me know if any changes.
Summary by CodeRabbit
New Features
Chores
Bug Fixes