Skip to content

🔒 Hardened /tmp permissions in Nextcloud container #228

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Ven0m0 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

🔒 Hardened /tmp permissions in Nextcloud container #228

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 13 additions & 13 deletions RaspberryPi/Scripts/Fix.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,23 +125,23 @@ fix_nextcloud() {
fi

log "Fixing Nextcloud /tmp permissions"
if sudo docker exec nextcloud ls -ld /tmp &>/dev/null; then
local perms_ok=1
if ! sudo docker exec nextcloud chown root:root /tmp; then
warn "Failed to chown /tmp in nextcloud"
perms_ok=0
if ! sudo docker exec nextcloud sh -c '
set -eu
if [ -L /tmp ]; then
rm /tmp
Comment on lines +130 to +131
fi
if ! sudo docker exec nextcloud chmod 1777 /tmp; then
warn "Failed to chmod /tmp in nextcloud"
perms_ok=0
fi
if (( perms_ok )); then
log "Nextcloud permissions fixed"
mkdir -p /tmp
if getent group www-data >/dev/null 2>&1; then
chown root:www-data /tmp
chmod 1770 /tmp
else
warn "Nextcloud permission fix incomplete"
chown root:root /tmp
chmod 1777 /tmp
fi
'; then
warn "Failed to fix /tmp permissions in nextcloud"
else
warn "Failed to access /tmp in nextcloud container"
log "Nextcloud permissions fixed"
fi
Comment on lines +128 to 145
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Mar 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This is a great security hardening improvement. I have a few suggestions to align it more closely with the repository's style guide and improve readability:

  • Use [[ ... ]] for tests: The style guide (line 117) specifies using [[ ... ]] for tests. The check for the symbolic link should be updated from [ -L /tmp ] to [[ -L /tmp ]].
  • Include pipefail: The style guide (line 108) recommends set -euo pipefail. The pipefail option is missing from the set -eu command within the docker exec script.
  • Improve readability: The if ! ...; then [fail]; else [success]; fi logic is inverted and can be harder to read. It's more conventional to use if ...; then [success]; else [fail]; fi.

Here is a suggested implementation that incorporates these points:

Suggested change
if ! sudo docker exec nextcloud sh -c '
set -eu
if [ -L /tmp ]; then
rm /tmp
fi
if ! sudo docker exec nextcloud chmod 1777 /tmp; then
warn "Failed to chmod /tmp in nextcloud"
perms_ok=0
fi
if (( perms_ok )); then
log "Nextcloud permissions fixed"
mkdir -p /tmp
if getent group www-data >/dev/null 2>&1; then
chown root:www-data /tmp
chmod 1770 /tmp
else
warn "Nextcloud permission fix incomplete"
chown root:root /tmp
chmod 1777 /tmp
fi
'; then
warn "Failed to fix /tmp permissions in nextcloud"
else
warn "Failed to access /tmp in nextcloud container"
log "Nextcloud permissions fixed"
fi
if sudo docker exec nextcloud sh -c '
set -euo pipefail
if [[ -L /tmp ]]; then
rm /tmp
fi
mkdir -p /tmp
if getent group www-data >/dev/null 2>&1; then
chown root:www-data /tmp
chmod 1770 /tmp
else
chown root:root /tmp
chmod 1777 /tmp
fi
'; then
log "Nextcloud permissions fixed"
else
warn "Failed to fix /tmp permissions in nextcloud"
fi
References
  1. The style guide mandates using [[ ... ]] for tests. (Line 117: Tests: [[ ... ]]) (link)
  2. The style guide mandates set -euo pipefail for scripts. (Line 108: set -euo pipefail) (link)

}

Expand Down
Loading