Add automated Ubuntu & Proxmox LXC installers for Riven (Docker-based) #1
Conversation
Updated the Proxmox LXC installer script to improve structure and clarity. Added comments and organized sections for better readability.
Refactor install script for improved structure and clarity.
Refactor command detection and improve comments.
This script installs Docker and Docker Compose on Ubuntu, sets the timezone, and creates necessary directories and environment files for the Riven application.
Updated docker-compose.yml to define services for riven-db, riven, and riven-frontend with health checks and environment variables.
Refactor install script for better readability and structure, add interactive timezone selection, and improve dependency checks.
Updated warning message for Riven configuration steps.
This README provides detailed instructions for installing and configuring Riven on Ubuntu using Docker and Docker Compose, including supported systems, filesystem setup, deployment steps, required configurations, and troubleshooting tips.
Added instructions for running the Ubuntu installer script.
Updated formatting and section headers in readme.
📝 WalkthroughWalkthroughRemoves legacy Proxmox bare-metal installers and per-media helpers; adds a Docker-first Proxmox LXC flow (creation, bootstrap, Docker Compose deploy, upgrade), a full Ubuntu installer with lifecycle scripts (install/update/uninstall/remount), and new DB maintenance tooling and compose files. Changes
Sequence Diagram(s)sequenceDiagram
actor Admin
participant Proxmox as rgba(50,115,220,0.5)
participant LXC as rgba(80,200,120,0.5)
participant Docker as rgba(220,100,80,0.5)
participant DB as rgba(160,80,200,0.5)
participant Riven as rgba(200,160,60,0.5)
participant Frontend as rgba(60,180,200,0.5)
participant Media as rgba(140,140,140,0.5)
Admin->>Proxmox: run `proxmox/lxc/lxc-create.sh`
Proxmox->>LXC: create Debian 12 unprivileged LXC (FUSE, nesting)
Admin->>LXC: run `proxmox/lxc/lxc-bootstrap.sh`
LXC->>Docker: install & enable Docker + Compose
Admin->>LXC: run `proxmox/lxc/riven-install.sh`
LXC->>Docker: `docker compose up -d`
Docker->>DB: start riven-db (Postgres)
DB-->>Docker: health OK
Docker->>Riven: start backend
Riven-->>Docker: health OK
Docker->>Frontend: start riven-frontend
Docker->>Media: optionally start Jellyfin/Plex/Emby
LXC-->>Admin: output credentials & URLs
sequenceDiagram
actor User
participant Installer as rgba(50,115,220,0.5)
participant Host as rgba(80,200,120,0.5)
participant Docker as rgba(220,100,80,0.5)
participant Compose as rgba(160,80,200,0.5)
participant DB as rgba(200,160,60,0.5)
participant Riven as rgba(60,180,200,0.5)
participant Media as rgba(140,140,140,0.5)
User->>Installer: run `ubuntu/install.sh` (root)
Installer->>Host: validate OS, ensure rshared mount, create paths
Installer->>User: prompt for media/downloader/scraper config
Installer->>Docker: install Docker & Compose
Installer->>Compose: `docker compose -f ubuntu/docker-compose.media.yml up -d`
Compose->>Media: start media container
Installer->>Compose: `docker compose -f ubuntu/docker-compose.yml up -d`
Compose->>DB: start PostgreSQL
DB-->>Compose: health OK
Compose->>Riven: start backend & frontend
Installer-->>User: print summary (paths, creds, URLs)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 35
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (17)
proxmox/README.mdproxmox/build.funcproxmox/create_lxc.shproxmox/install.shproxmox/lxc/docker-compose.ymlproxmox/lxc/lxc-bootstrap.shproxmox/lxc/lxc-create.shproxmox/lxc/riven-install.shproxmox/lxc/upgrade.shproxmox/media-emby.shproxmox/media-jellyfin.shproxmox/media-plex.shproxmox/riven-install.shproxmox/riven.shubuntu/docker-compose.ymlubuntu/install.shubuntu/readme.md
💤 Files with no reviewable changes (7)
- proxmox/create_lxc.sh
- proxmox/riven-install.sh
- proxmox/media-emby.sh
- proxmox/media-jellyfin.sh
- proxmox/riven.sh
- proxmox/build.func
- proxmox/media-plex.sh
🧰 Additional context used
🧬 Code graph analysis (2)
proxmox/install.sh (1)
proxmox/lxc/lxc-create.sh (1)
require_cmd(33-33)
proxmox/lxc/lxc-create.sh (1)
proxmox/install.sh (1)
require_cmd(17-22)
🪛 markdownlint-cli2 (0.18.1)
proxmox/README.md
3-3: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
11-11: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
21-21: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
26-26: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
30-30: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
37-37: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
41-41: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
43-43: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
50-50: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
52-52: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
ubuntu/readme.md
1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
5-5: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
6-6: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Above
(MD022, blanks-around-headings)
🪛 Shellcheck (0.11.0)
proxmox/lxc/lxc-bootstrap.sh
[info] 19-19: Not following: /etc/os-release was not specified as input (see shellcheck -x).
(SC1091)
ubuntu/install.sh
[info] 28-28: Not following: /etc/os-release was not specified as input (see shellcheck -x).
(SC1091)
🔇 Additional comments (18)
proxmox/lxc/lxc-bootstrap.sh (1)
1-9: LGTM!Good error handling with
set -euo pipefailand appropriate use ofDEBIAN_FRONTEND=noninteractivefor automated installation. The package list covers necessary dependencies for Docker installation.ubuntu/docker-compose.yml (2)
68-68: Clarify frontend database usage.The frontend's
DATABASE_URLpoints to/riven/data/riven.dbwithin theriven-frontend-datavolume, while the backend uses PostgreSQL. This suggests the frontend uses a separate SQLite database. If intentional, this is fine, but it should be documented. If not, this could indicate a configuration error.Does the Riven frontend require its own SQLite database separate from the backend's PostgreSQL database, or should this path point to the backend's database?
1-1: Remove the obsoleteversionfield from docker-compose.yml.The
versionfield is obsolete in Docker Compose V2 (GA since April 2022) and is ignored by current versions. Using it triggers a warning. Replace the version declaration withservices:at the top level instead.proxmox/README.md (2)
41-48: No issues found – profile names are correct.The compose profiles
jellyfin,plex, andembyin the documentation match the profiles defined inproxmox/lxc/docker-compose.yml.
30-39: Remove container name verification; clarify settings.json editing workflow.The
docker restart rivencommand correctly matches thecontainer_name: rivendefined indocker-compose.yml, so that concern is unfounded. However, the README should clarify that/mnt/riven/backend/settings.jsonis edited after the containers start (once auto-generated), and point users to the main Riven documentation for configuration details rather than expecting examples in this deployment guide.Likely an incorrect or invalid review comment.
proxmox/lxc/riven-install.sh (4)
1-2: Consider addingpipefailequivalent error handling for subshells.The
set -euo pipefailis good practice. The script properly uses strict mode.
19-22: Mount operations may fail silently on subsequent runs.If
$MOUNT_DIRis already a mountpoint but notrshared, themount --bindis skipped butmount --make-rsharedstill runs. This is correct behavior. However, if the bind mount exists butmake-rsharedfails, the script will exit due toset -ebefore reaching the validation check, which is appropriate.
85-87: Credential extraction may fail if .env has unexpected format.Using
grep -E | cutworks for simple cases but will break if values contain=characters. SincePOSTGRES_PASSWORDis generated viaopenssl rand -hex, this is safe here, but consider using a more robust parser for future-proofing.
53-74: The hardcodedDATABASE_URLpath is intentional and not problematic. The frontend (riven-frontend service) uses its own SQLite database (/riven/data/riven.db) for frontend-local state (authentication, preferences), while the backend (riven service) uses PostgreSQL viaRIVEN_DATABASE_HOST. These are separate databases serving different purposes and are properly configured with distinct environment variables. No conflict exists.Likely an incorrect or invalid review comment.
proxmox/install.sh (2)
183-193: Good user guidance provided.The final output clearly shows how to get the CT IP and access URLs. The log commands are helpful for troubleshooting.
61-72: [Rewritten review comment]
[Classification tag]proxmox/lxc/docker-compose.yml (3)
22-35: Security concerns with elevated container privileges.The
rivencontainer has significant privileges:
cap_add: SYS_ADMIN- broad capabilitysecurity_opt: apparmor:unconfined- disables AppArmor/dev/fusedevice accessThese are likely required for FUSE-based media mounting, but document why these are needed and ensure users understand the security implications.
86-90: GPU device passthrough may fail if /dev/dri doesn't exist.The
devices: [/dev/dri:/dev/dri]will cause container startup failure if the device doesn't exist. Consider making this conditional or documenting the requirement.
92-106: Plex uses host network mode while others use port mapping.This is a known Plex requirement for discovery, but it creates inconsistency. The
PUID/PGIDare hardcoded to 1000—ensure this matches the riven user created by the installer.proxmox/lxc/lxc-create.sh (2)
50-52: Root password logged to console in plaintext.The randomly generated password is echoed to stdout. While necessary for the user, this could be captured in logs. Consider advising users to change it or store securely.
73-79: Security implications of Docker-in-LXC configuration.The configuration sets:
lxc.apparmor.profile: unconfined- disables AppArmorlxc.cgroup2.devices.allow: a- allows access to ALL devicesThis is very permissive and required for Docker-in-LXC, but essentially makes the container nearly as privileged as a privileged container. Document these security implications for users.
ubuntu/install.sh (2)
28-29: Sourcing /etc/os-release is safe but flagged by shellcheck.The static analysis hint (SC1091) is a false positive for this use case—the file exists on all Ubuntu systems and sourcing it is the standard way to detect the OS. The current implementation is correct.
204-227: Clear and helpful final instructions.The warning about required configuration is appropriately prominent. The instructions clearly explain what users must do and the consequences of not doing so.
proxmox/install.sh
Outdated
| require_cmd() { | ||
| command -v "$1" >/dev/null 2>&1 || { | ||
| echo "ERROR: Missing required command: $1" | ||
| exit 1 | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Helper function duplicated across scripts.
This require_cmd() function is nearly identical to the one in proxmox/lxc/lxc-create.sh (line 33). Consider extracting to a shared utilities file to follow DRY principles, though for standalone curl-pipe scripts, duplication may be acceptable for simplicity.
🤖 Prompt for AI Agents
In proxmox/install.sh lines 17-22 the helper function require_cmd() is
duplicated elsewhere (proxmox/lxc/lxc-create.sh line 33); extract this function
into a shared utilities file (e.g., proxmox/scripts/common.sh), make it
POSIX-safe, and replace the duplicated definition in both scripts with a single
source line that loads the common file (use a relative dirname-based path so
sourcing works when scripts are run directly). Ensure scripts still work
standalone when curl-piped by falling back to an inline minimal require_cmd if
the common file cannot be sourced (check for -f before sourcing and only define
the function inline as a last resort).
proxmox/install.sh
Outdated
| PVE_VER_RAW="$(pveversion | head -n1)" | ||
| PVE_VER="$(echo "$PVE_VER_RAW" | sed -nE 's/.*pve-manager\/([0-9]+)\.([0-9]+).*/\1.\2/p')" | ||
|
|
||
| if [[ -z "$PVE_VER" ]]; then | ||
| echo "WARNING: Could not parse Proxmox version from: $PVE_VER_RAW" | ||
| else | ||
| MAJ="${PVE_VER%.*}" | ||
| MIN="${PVE_VER#*.}" | ||
| if (( MAJ < 8 )) || (( MAJ == 8 && MIN < 1 )); then | ||
| echo "ERROR: Proxmox 8.1+ required. Detected: $PVE_VER_RAW" | ||
| exit 1 | ||
| fi | ||
| fi |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Version parsing could fail on non-standard pveversion output.
The regex extraction is reasonable, but if pveversion output format changes, PVE_VER could be empty. The script handles this with a warning (line 39), which is good. However, the comparison logic at lines 41-46 assumes MAJ and MIN are integers.
If PVE_VER is malformed (e.g., 8.x), the arithmetic comparison could fail. Consider adding numeric validation.
🔎 Proposed fix
else
MAJ="${PVE_VER%.*}"
MIN="${PVE_VER#*.}"
+ if ! [[ "$MAJ" =~ ^[0-9]+$ && "$MIN" =~ ^[0-9]+$ ]]; then
+ echo "WARNING: Could not parse version numbers from: $PVE_VER"
+ else
if (( MAJ < 8 )) || (( MAJ == 8 && MIN < 1 )); then
echo "ERROR: Proxmox 8.1+ required. Detected: $PVE_VER_RAW"
exit 1
fi
+ fi
fi🤖 Prompt for AI Agents
In proxmox/install.sh around lines 35 to 47, the script extracts MAJ and MIN
from PVE_VER but does not validate they are numeric before performing arithmetic
comparisons; add a numeric validation step after splitting PVE_VER (e.g., check
MAJ and MIN match ^[0-9]+$) and if either is non-numeric print a clear error
(including PVE_VER_RAW) and exit 1 (or treat non-numeric as 0) so the subsequent
(( MAJ < 8 )) || (( MAJ == 8 && MIN < 1 )) comparisons cannot fail on malformed
input.
proxmox/install.sh
Outdated
| while :; do | ||
| CANDIDATE=$(( (RANDOM % 900) + 100 )) | ||
| if ! pct status "$CANDIDATE" >/dev/null 2>&1; then | ||
| DEFAULT_CTID="$CANDIDATE" | ||
| break | ||
| fi | ||
| done |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
CTID generation loop could theoretically run indefinitely.
If all CTIDs 100-999 are taken, the while :; do loop never exits. This is unlikely in practice but worth noting.
🔎 Add a safeguard
DEFAULT_CTID=""
+ATTEMPTS=0
while :; do
+ (( ++ATTEMPTS > 1000 )) && { echo "ERROR: No free CTID found"; exit 1; }
CANDIDATE=$(( (RANDOM % 900) + 100 ))
if ! pct status "$CANDIDATE" >/dev/null 2>&1; then
DEFAULT_CTID="$CANDIDATE"
break
fi
doneCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In proxmox/install.sh around lines 79-85 the random CTID selection loop can run
forever if all IDs 100-999 are taken; add a safeguard by introducing a
max_attempts counter (e.g., 1000) that increments each loop and breaks when
reached, and if the limit is hit then perform a deterministic fallback: iterate
sequentially from 100 to 999 to pick the first available CTID, and if none are
free exit with a clear error code/message; keep the original random selection
logic but ensure the loop exits in all cases.
ubuntu/install.sh
Outdated
| SERVER_IP="$(ip route get 1.1.1.1 2>/dev/null | awk '{print $7; exit}')" | ||
| [ -z "$SERVER_IP" ] && SERVER_IP="SERVER_IP" |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
IP detection may fail in some network configurations.
The ip route get 1.1.1.1 approach assumes internet connectivity and may fail in isolated networks. The fallback to SERVER_IP string is good, but consider also trying hostname -I.
🔎 More robust IP detection
-SERVER_IP="$(ip route get 1.1.1.1 2>/dev/null | awk '{print $7; exit}')"
-[ -z "$SERVER_IP" ] && SERVER_IP="SERVER_IP"
+SERVER_IP="$(ip route get 1.1.1.1 2>/dev/null | awk '{print $7; exit}' || hostname -I | awk '{print $1}' || echo "SERVER_IP")"Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In ubuntu/install.sh around lines 201-202, the current IP detection using `ip
route get 1.1.1.1` can fail in isolated/no-internet setups and the fallback
currently sets SERVER_IP to the literal string "SERVER_IP"; update the logic to
try `hostname -I` (or `hostname -i` as a secondary option) when the `ip route`
command returns empty, trim and select the first non-empty address, and only
then fall back to a sensible default (e.g., empty string or a configurable
placeholder) instead of the literal "SERVER_IP"; ensure commands are guarded so
failures do not abort the script and preserve current behavior when a valid IP
is found.
ubuntu/readme.md
Outdated
| ## ▶️ How to run the installer (Ubuntu Script) | ||
|
|
||
| ```bash | ||
| sudo bash -c "$(curl -fsSL https://raw.githubusercontent.com/AquaHorizonGaming/distributables/main/ubuntu/install.sh)" |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider security and stability implications.
Piping curl directly to bash is convenient but poses security risks if the source is compromised. Additionally, pointing to the main branch may expose users to unstable code.
Consider:
- Recommending users inspect the script before running it
- Using a stable release tag or branch instead of
main - Providing a checksum for verification
🤖 Prompt for AI Agents
In ubuntu/readme.md around line 4, the install command pipes curl to bash from
the main branch which is a security and stability risk; update the docs to (1)
advise users to inspect the script before running (show how to download and view
it), (2) recommend using a specific release tag or stable branch URL instead of
main, (3) provide or link to a published checksum (SHA256) and explain verifying
the checksum before execution, and (4) optionally suggest running the script in
a dry-run or non-root environment first and only using sudo when verified.
ubuntu/readme.md
Outdated
| REQUIRED CONFIGURATION (DO NOT SKIP) | ||
|
|
||
| AFTER INSTALL YOU MUST EDIT: | ||
| - /mnt/riven/backend/settings.json | ||
|
|
||
| YOU MUST: | ||
| - Add at least ONE scraper | ||
| - Configure at least ONE media server (Plex, Jellyfin, or Emby) | ||
|
|
||
| IF YOU DO NOT DO THIS: | ||
| - Containers WILL appear healthy | ||
| - NO content will load | ||
| - Scraping will silently fail | ||
|
|
||
| RIVEN WILL NOT WORK UNTIL THIS IS CONFIGURED. |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Provide configuration examples or documentation links.
The warning is clear and emphatic, but users may struggle without concrete examples of how to configure scrapers and media servers in settings.json.
Consider adding:
- A link to configuration documentation
- Example JSON snippets for common configurations
- A troubleshooting section for configuration-related issues
🤖 Prompt for AI Agents
In ubuntu/readme.md around lines 45 to 59, the strong warning about editing
/mnt/riven/backend/settings.json lacks concrete guidance; add actionable
configuration help by linking to the full configuration docs, inserting at least
one example JSON snippet for a common scraper and one for a media server
(Plex/Jellyfin/Emby) showing minimal required fields, and include a short
troubleshooting section that lists common misconfigurations and verification
steps (e.g., file path, JSON validity, service credentials, and how to validate
scrapers and media server connections).
Updated instructions for running on Proxmox host.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
proxmox/README.md
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
proxmox/README.md
7-7: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
8-8: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Above
(MD022, blanks-around-headings)
8-8: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
18-18: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
23-23: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
27-27: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
34-34: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
38-38: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
40-40: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
47-47: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
49-49: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
Updated README to improve clarity and structure.
Updated installation script URLs to point to AquaHorizonGaming repository.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
README.md
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-04T07:07:27.617Z
Learnt from: AquaHorizonGaming
Repo: rivenmedia/distributables PR: 1
File: proxmox/lxc/docker-compose.yml:23-23
Timestamp: 2026-01-04T07:07:27.617Z
Learning: In the rivenmedia/distributables repository, the Docker image tags `spoked/riven:dev` and `spoked/riven-frontend:dev` are the main/production build tags, not development tags.
Applied to files:
README.md
🔇 Additional comments (2)
README.md (2)
11-48: Quick Start sections are clear and well-structured.The documentation effectively communicates the two primary deployment paths with:
- Clear one-line installer commands with appropriate privilege handling (sudo for Ubuntu, root context for Proxmox)
- Concise descriptions of what each installer handles
- References to detailed platform-specific READMEs for follow-up configuration
52-67: Repository structure and notes are appropriately concise.The repository structure diagram clearly maps the directory layout, and the notes section provides helpful guidance without over-explaining. This supports the self-contained, per-platform documentation approach well.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @ubuntu/riven-remount-cycle.sh:
- Around line 133-135: Add a one-line explanatory comment above the
sleep/restart sequence to clarify why the delay and restart are needed: mention
that we sleep briefly to allow the remount to settle and for the container to
pick up the reattached volume, then restart the MEDIA_CONTAINER so the media
server detects the remounted filesystem; place this comment immediately before
the sleep 5/docker restart "$MEDIA_CONTAINER" block and keep the existing ok
"Media server restarted (post-mount)" message.
- Around line 18-20: The script defines REMOUNT_WAIT but never uses it; replace
the hardcoded sleep 5 with sleep $REMOUNT_WAIT (or ${REMOUNT_WAIT}) where the
remount pause occurs (currently a plain "sleep 5") so the configured constant is
honored, or if you prefer remove the REMOUNT_WAIT declaration to avoid an unused
variable—update the remount logic in the remount loop/function that currently
calls "sleep 5".
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
ubuntu/riven-remount-cycle.sh
🧰 Additional context used
🪛 Shellcheck (0.11.0)
ubuntu/riven-remount-cycle.sh
[warning] 20-20: REMOUNT_WAIT appears unused. Verify use (or export if used externally).
(SC2034)
🔇 Additional comments (5)
ubuntu/riven-remount-cycle.sh (5)
1-10: LGTM!Good use of strict mode with
set -euo pipefailand clear root privilege enforcement.
22-32: LGTM!Clean helper functions with consistent output formatting. The
is_mountedcheck usingfindmntis reliable.
34-60: LGTM!User prompts with sensible defaults and proper input validation. Variables are correctly quoted throughout.
62-95: LGTM!Robust container stop handling with
|| truefallback, and well-implemented unmount retry logic with proper verification.
97-120: LGTM!Proper bind mount with rshared propagation setup and verification. Accepting both "shared" and "rshared" is correct since rshared implies shared.
ubuntu/riven-remount-cycle.sh
Outdated
| UNMOUNT_RETRIES=5 | ||
| WAIT_BETWEEN=2 | ||
| REMOUNT_WAIT=5 |
There was a problem hiding this comment.
Unused variable REMOUNT_WAIT.
REMOUNT_WAIT is defined but never used. Line 133 uses a hardcoded sleep 5 which appears to be the intended use case for this variable.
Proposed fix
Either use the variable:
-sleep 5
+sleep "$REMOUNT_WAIT"Or remove the unused declaration:
UNMOUNT_RETRIES=5
WAIT_BETWEEN=2
-REMOUNT_WAIT=5📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| UNMOUNT_RETRIES=5 | |
| WAIT_BETWEEN=2 | |
| REMOUNT_WAIT=5 | |
| UNMOUNT_RETRIES=5 | |
| WAIT_BETWEEN=2 |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 20-20: REMOUNT_WAIT appears unused. Verify use (or export if used externally).
(SC2034)
🤖 Prompt for AI Agents
In @ubuntu/riven-remount-cycle.sh around lines 18 - 20, The script defines
REMOUNT_WAIT but never uses it; replace the hardcoded sleep 5 with sleep
$REMOUNT_WAIT (or ${REMOUNT_WAIT}) where the remount pause occurs (currently a
plain "sleep 5") so the configured constant is honored, or if you prefer remove
the REMOUNT_WAIT declaration to avoid an unused variable—update the remount
logic in the remount loop/function that currently calls "sleep 5".
ubuntu/riven-remount-cycle.sh
Outdated
| sleep 5 | ||
| docker restart "$MEDIA_CONTAINER" >/dev/null | ||
| ok "Media server restarted (post-mount)" |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding a clarifying comment for the restart logic.
The sleep-then-restart pattern for the media server is a reasonable workaround to ensure it properly detects the remounted volume, but the rationale isn't immediately obvious. A brief comment would help future maintainers understand why this is necessary.
Suggested improvement
+# Allow media server to initialize, then restart to ensure it detects the remounted volume
sleep 5
docker restart "$MEDIA_CONTAINER" >/dev/null
ok "Media server restarted (post-mount)"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| sleep 5 | |
| docker restart "$MEDIA_CONTAINER" >/dev/null | |
| ok "Media server restarted (post-mount)" | |
| # Allow media server to initialize, then restart to ensure it detects the remounted volume | |
| sleep 5 | |
| docker restart "$MEDIA_CONTAINER" >/dev/null | |
| ok "Media server restarted (post-mount)" |
🤖 Prompt for AI Agents
In @ubuntu/riven-remount-cycle.sh around lines 133 - 135, Add a one-line
explanatory comment above the sleep/restart sequence to clarify why the delay
and restart are needed: mention that we sleep briefly to allow the remount to
settle and for the container to pick up the reattached volume, then restart the
MEDIA_CONTAINER so the media server detects the remounted filesystem; place this
comment immediately before the sleep 5/docker restart "$MEDIA_CONTAINER" block
and keep the existing ok "Media server restarted (post-mount)" message.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @ubuntu/riven-remount-cycle.sh:
- Around line 35-41: show_version() is defined but never called; either remove
the dead function or invoke show_version after the script's root-check sequence
(i.e., immediately following the root-check function or logic) so the version is
displayed at startup; update any callers accordingly and ensure no unused
variables remain if you choose to delete show_version.
- Around line 47-50: Validate the chosen mount directory after reading
MOUNT_PATH (and after applying DEFAULT_MOUNT fallback) by checking test -d
"$MOUNT_PATH"; if it doesn't exist either prompt to create it or attempt mkdir
-p "$MOUNT_PATH" and fail with a clear error via the existing ok/error logging
functions (referencing MOUNT_PATH, DEFAULT_MOUNT and ok) — ensure you perform
this check immediately after the MOUNT_PATH assignment and exit with a non-zero
code on irrecoverable failure.
- Around line 18-21: The script defines REMOUNT_WAIT but still uses a hardcoded
sleep 5; update the remount/wait call that currently does "sleep 5" to use the
REMOUNT_WAIT variable (e.g., sleep "$REMOUNT_WAIT") so the configured timeout is
honored; locate the remount loop or function in riven-remount-cycle.sh where the
hardcoded sleep occurs and replace it with the REMOUNT_WAIT variable, ensuring
the variable is exported/available in scope if needed.
- Around line 74-80: The script currently always prints success via ok "Riven
stopped" and ok "Media server stopped" even when docker stop silently fails;
update the stop logic for RIVEN_CONTAINER and MEDIA_CONTAINER to detect whether
the container exists or docker stop returned an error and emit a warning instead
of a success message. Specifically, before or after calling docker stop for the
variables RIVEN_CONTAINER and MEDIA_CONTAINER, check container existence (e.g.,
via docker ps -a or docker inspect) or capture docker stop’s exit status, call a
warn/error helper when the container is missing or stop failed, and only call ok
"Riven stopped" / ok "Media server stopped" when the stop actually succeeded.
Ensure you reference the RIVEN_CONTAINER and MEDIA_CONTAINER variables and the
ok function when locating where to change behavior.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
ubuntu/riven-remount-cycle.sh
🧰 Additional context used
🪛 Shellcheck (0.11.0)
ubuntu/riven-remount-cycle.sh
[warning] 20-20: REMOUNT_WAIT appears unused. Verify use (or export if used externally).
(SC2034)
🔇 Additional comments (4)
ubuntu/riven-remount-cycle.sh (4)
1-10: LGTM!Good use of
set -euo pipefailfor strict error handling and proper root privilege check.
82-105: LGTM!Robust unmount logic with proper retry handling and final verification.
107-130: LGTM!Mount propagation setup and verification logic is correct.
132-151: LGTM!Container restart sequence is appropriate. Note: the
sleep 5on line 143 should use$REMOUNT_WAITas flagged earlier.
ubuntu/riven-remount-cycle.sh
Outdated
| UNMOUNT_RETRIES=5 | ||
| WAIT_BETWEEN=2 | ||
| REMOUNT_WAIT=5 | ||
| VERSION=1.0 |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use REMOUNT_WAIT variable instead of hardcoded value.
REMOUNT_WAIT is defined but never used. Line 143 hardcodes sleep 5 instead of using this variable.
♻️ Proposed fix
-sleep 5
+sleep "$REMOUNT_WAIT"🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 20-20: REMOUNT_WAIT appears unused. Verify use (or export if used externally).
(SC2034)
🤖 Prompt for AI Agents
In @ubuntu/riven-remount-cycle.sh around lines 18 - 21, The script defines
REMOUNT_WAIT but still uses a hardcoded sleep 5; update the remount/wait call
that currently does "sleep 5" to use the REMOUNT_WAIT variable (e.g., sleep
"$REMOUNT_WAIT") so the configured timeout is honored; locate the remount loop
or function in riven-remount-cycle.sh where the hardcoded sleep occurs and
replace it with the REMOUNT_WAIT variable, ensuring the variable is
exported/available in scope if needed.
| section "Mount Configuration" | ||
| read -rp "Mount path [${DEFAULT_MOUNT}]: " MOUNT_PATH | ||
| MOUNT_PATH="${MOUNT_PATH:-$DEFAULT_MOUNT}" | ||
| ok "Using mount path: $MOUNT_PATH" |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider validating mount path existence.
The script doesn't verify that $MOUNT_PATH exists before proceeding. This could lead to confusing failures during mount operations.
♻️ Proposed validation
MOUNT_PATH="${MOUNT_PATH:-$DEFAULT_MOUNT}"
+[[ -d "$MOUNT_PATH" ]] || fail "Mount path does not exist: $MOUNT_PATH"
ok "Using mount path: $MOUNT_PATH"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| section "Mount Configuration" | |
| read -rp "Mount path [${DEFAULT_MOUNT}]: " MOUNT_PATH | |
| MOUNT_PATH="${MOUNT_PATH:-$DEFAULT_MOUNT}" | |
| ok "Using mount path: $MOUNT_PATH" | |
| section "Mount Configuration" | |
| read -rp "Mount path [${DEFAULT_MOUNT}]: " MOUNT_PATH | |
| MOUNT_PATH="${MOUNT_PATH:-$DEFAULT_MOUNT}" | |
| [[ -d "$MOUNT_PATH" ]] || fail "Mount path does not exist: $MOUNT_PATH" | |
| ok "Using mount path: $MOUNT_PATH" |
🤖 Prompt for AI Agents
In @ubuntu/riven-remount-cycle.sh around lines 47 - 50, Validate the chosen
mount directory after reading MOUNT_PATH (and after applying DEFAULT_MOUNT
fallback) by checking test -d "$MOUNT_PATH"; if it doesn't exist either prompt
to create it or attempt mkdir -p "$MOUNT_PATH" and fail with a clear error via
the existing ok/error logging functions (referencing MOUNT_PATH, DEFAULT_MOUNT
and ok) — ensure you perform this check immediately after the MOUNT_PATH
assignment and exit with a non-zero code on irrecoverable failure.
ubuntu/riven-remount-cycle.sh
Outdated
| ############################################ | ||
| section "Stopping Containers" | ||
| docker stop "$RIVEN_CONTAINER" >/dev/null 2>&1 || true | ||
| ok "Riven stopped" | ||
|
|
||
| docker stop "$MEDIA_CONTAINER" >/dev/null 2>&1 || true | ||
| ok "Media server stopped" |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider warning if containers don't exist.
The script reports success even when containers don't exist, which could mislead users.
♻️ Proposed improvement
section "Stopping Containers"
-docker stop "$RIVEN_CONTAINER" >/dev/null 2>&1 || true
-ok "Riven stopped"
+if docker ps -a --format '{{.Names}}' | grep -qx "$RIVEN_CONTAINER"; then
+ docker stop "$RIVEN_CONTAINER" >/dev/null 2>&1 || true
+ ok "Riven stopped"
+else
+ warn "Riven container not found, skipping"
+fi
-docker stop "$MEDIA_CONTAINER" >/dev/null 2>&1 || true
-ok "Media server stopped"
+if docker ps -a --format '{{.Names}}' | grep -qx "$MEDIA_CONTAINER"; then
+ docker stop "$MEDIA_CONTAINER" >/dev/null 2>&1 || true
+ ok "Media server stopped"
+else
+ warn "Media container '$MEDIA_CONTAINER' not found, skipping"
+fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ############################################ | |
| section "Stopping Containers" | |
| docker stop "$RIVEN_CONTAINER" >/dev/null 2>&1 || true | |
| ok "Riven stopped" | |
| docker stop "$MEDIA_CONTAINER" >/dev/null 2>&1 || true | |
| ok "Media server stopped" | |
| ############################################ | |
| section "Stopping Containers" | |
| if docker ps -a --format '{{.Names}}' | grep -qx "$RIVEN_CONTAINER"; then | |
| docker stop "$RIVEN_CONTAINER" >/dev/null 2>&1 || true | |
| ok "Riven stopped" | |
| else | |
| warn "Riven container not found, skipping" | |
| fi | |
| if docker ps -a --format '{{.Names}}' | grep -qx "$MEDIA_CONTAINER"; then | |
| docker stop "$MEDIA_CONTAINER" >/dev/null 2>&1 || true | |
| ok "Media server stopped" | |
| else | |
| warn "Media container '$MEDIA_CONTAINER' not found, skipping" | |
| fi |
🤖 Prompt for AI Agents
In @ubuntu/riven-remount-cycle.sh around lines 74 - 80, The script currently
always prints success via ok "Riven stopped" and ok "Media server stopped" even
when docker stop silently fails; update the stop logic for RIVEN_CONTAINER and
MEDIA_CONTAINER to detect whether the container exists or docker stop returned
an error and emit a warning instead of a success message. Specifically, before
or after calling docker stop for the variables RIVEN_CONTAINER and
MEDIA_CONTAINER, check container existence (e.g., via docker ps -a or docker
inspect) or capture docker stop’s exit status, call a warn/error helper when the
container is missing or stop failed, and only call ok "Riven stopped" / ok
"Media server stopped" when the stop actually succeeded. Ensure you reference
the RIVEN_CONTAINER and MEDIA_CONTAINER variables and the ok function when
locating where to change behavior.
- Add robust bind remount cycle with rshared propagation - Handle Docker and systemd media servers (Plex, Jellyfin, Emby) - Fix unmount loop to exit cleanly when already unmounted - Add version banner and consistent output helpers - Improve safety under set -euo pipefail
Increment script version to v1.3 after remount, propagation, and media runtime improvements.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @ubuntu/riven-remount-cycle.sh:
- Around line 121-127: The systemctl stop branch can cause the script to exit
under set -e when the service is already stopped or missing; mirror the Docker
branch's graceful handling by swallowing errors: change the systemctl stop
"$MEDIA_SERVICE" line to redirect output and append || true (e.g., systemctl
stop "$MEDIA_SERVICE" >/dev/null 2>&1 || true) so MEDIA_MODE, systemctl stop,
MEDIA_SERVICE and the subsequent ok "Media service stopped" behave consistently
with the docker stop "$MEDIA_CONTAINER" path.
- Around line 134-153: The unmount loop mishandles umount output and lacks a
failure branch: change the umount invocation that currently pipes to grep so it
captures stderr (or test umount's exit status directly) and detect the "not
mounted" message from stderr OR simply check umount's return code instead of
piping (refer to umount and MOUNT_PATH in the loop), ensure you do not discard
umount's exit status, and after the for loop add explicit failure handling when
the mount is still mounted (use is_mounted()) — log an error and exit non‑zero
(or return failure) instead of proceeding to remount; keep UNMOUNT_RETRIES and
WAIT_BETWEEN behavior but fail fast with a clear message if all attempts
exhaust.
- Around line 18-21: REMOUNT_WAIT is defined but unused; replace the hardcoded
sleep 5 calls with sleep "${REMOUNT_WAIT}" at the two locations where they occur
(the sleep calls currently using 5) so the script uses the REMOUNT_WAIT variable
value, and keep the REMOUNT_WAIT=5 declaration as the single source of truth for
that delay.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
ubuntu/riven-remount-cycle.sh
🧰 Additional context used
🪛 Shellcheck (0.11.0)
ubuntu/riven-remount-cycle.sh
[warning] 20-20: REMOUNT_WAIT appears unused. Verify use (or export if used externally).
(SC2034)
🔇 Additional comments (7)
ubuntu/riven-remount-cycle.sh (7)
1-10: LGTM!Good use of strict mode (
set -euo pipefail) and clear root privilege enforcement.
23-33: LGTM!Clean helper functions with consistent formatting. Using
findmntfor mount detection is reliable.
35-51: LGTM!Version display and mount path prompt with sensible defaults.
53-111: LGTM!Well-structured two-step selection with appropriate defaults for known media servers and custom input fallback.
155-178: LGTM!Correct implementation of bind mount with rshared propagation and proper verification.
180-200: LGTM!The start-sleep-restart pattern ensures the media server properly recognizes the new mount. The hardcoded sleep values were noted earlier as candidates for using
REMOUNT_WAIT.
202-206: LGTM!Clean completion output.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In @ubuntu/riven-remount-cycle.sh:
- Around line 121-127: The systemd branch is not tolerant of failures while the
Docker branch silences errors (docker stop ... || true), causing unexpected
exits under set -e; make the systemctl stop call behave similarly by guarding it
(e.g., check service state with systemctl is-active "$MEDIA_SERVICE" || true
before stopping, or append || true to the systemctl stop invocation) and keep
the ok "Media service stopped" call only after handling suppressed errors;
reference MEDIA_MODE, MEDIA_CONTAINER, MEDIA_SERVICE, the docker stop and
systemctl stop invocations, and the ok function when making the change.
- Around line 18-21: Remove the dead variable by deleting the unused
REMOUNT_WAIT assignment (REMOUNT_WAIT=5) from the script; keep UNMOUNT_RETRIES,
WAIT_BETWEEN and VERSION intact and run a quick shellcheck/static lint to
confirm no other references to REMOUNT_WAIT remain.
- Around line 134-153: The unmount retry loop can exit without error if the
mount remains active; after the for loop that uses UNMOUNT_RETRIES and calls
is_mounted and umount on MOUNT_PATH, add a final check that calls is_mounted
once more and if it still returns true log an error (e.g., using
warn/processLogger/error function used in the script) and exit non‑zero to
prevent proceeding to the remount phase; ensure you reference the same log style
used elsewhere and return a non-zero status to fail fast when unmounts are
exhausted.
- Around line 185-200: The start/restart branches for RIVEN_CONTAINER and MEDIA
containers/services lack error handling and diagnostics; update the blocks that
reference RIVEN_CONTAINER, MEDIA_MODE, MEDIA_CONTAINER, and MEDIA_SERVICE to
capture stdout/stderr from docker start/docker restart and systemctl
start/systemctl restart, check each command's exit code, log failures via the
existing ok/error logging functions (including the command output or exit
status), and exit with a non-zero code on fatal failures; also add a brief
comment above the sleep+restart sequence explaining why an immediate restart is
required (post-mount remount delay) so future readers understand the purpose.
- Around line 140-145: The unmount retry logic currently pipes umount into grep
so the loop reacts to grep's exit status; instead run umount on "$MOUNT_PATH",
capture its output/exit code, and then explicitly check the output for "not
mounted" and call ok "Mount was already unmounted" + break if found; for all
other failures simply warn and continue to the next iteration (the existing
is_mounted check at the top will decide whether to retry). Update the block
around the umount call (referencing the umount invocation, MOUNT_PATH, warn, ok,
and is_mounted) to avoid relying on a pipe to grep and make the logic explicit
and clearer.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
ubuntu/riven-remount-cycle.sh
🧰 Additional context used
🪛 Shellcheck (0.11.0)
ubuntu/riven-remount-cycle.sh
[warning] 20-20: REMOUNT_WAIT appears unused. Verify use (or export if used externally).
(SC2034)
🔇 Additional comments (5)
ubuntu/riven-remount-cycle.sh (5)
1-10: LGTM!The shebang, strict mode (
set -euo pipefail), and root enforcement are correctly implemented.
23-33: LGTM!Output helpers and
is_mountedfunction are clean and idiomatic.
45-111: LGTM!The interactive prompts for mount path, media runtime, and server selection are well-structured with sensible defaults and clear feedback.
155-178: LGTM!The bind remount with
rsharedpropagation and verification logic is correctly implemented. The propagation check properly validates both "shared" and "rshared" states.
202-206: LGTM!Clean completion section.
ubuntu/riven-remount-cycle.sh
Outdated
| UNMOUNT_RETRIES=5 | ||
| WAIT_BETWEEN=2 | ||
| REMOUNT_WAIT=5 | ||
| VERSION=1.3 |
There was a problem hiding this comment.
Remove unused variable REMOUNT_WAIT.
REMOUNT_WAIT is defined but never used in the script. Based on the static analysis hint, this appears to be dead code.
Proposed fix
UNMOUNT_RETRIES=5
WAIT_BETWEEN=2
-REMOUNT_WAIT=5
VERSION=1.3📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| UNMOUNT_RETRIES=5 | |
| WAIT_BETWEEN=2 | |
| REMOUNT_WAIT=5 | |
| VERSION=1.3 | |
| UNMOUNT_RETRIES=5 | |
| WAIT_BETWEEN=2 | |
| VERSION=1.3 |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 20-20: REMOUNT_WAIT appears unused. Verify use (or export if used externally).
(SC2034)
🤖 Prompt for AI Agents
In @ubuntu/riven-remount-cycle.sh around lines 18 - 21, Remove the dead variable
by deleting the unused REMOUNT_WAIT assignment (REMOUNT_WAIT=5) from the script;
keep UNMOUNT_RETRIES, WAIT_BETWEEN and VERSION intact and run a quick
shellcheck/static lint to confirm no other references to REMOUNT_WAIT remain.
| if [[ "$MEDIA_MODE" == "docker" ]]; then | ||
| docker stop "$MEDIA_CONTAINER" >/dev/null 2>&1 || true | ||
| ok "Media container stopped" | ||
| else | ||
| systemctl stop "$MEDIA_SERVICE" | ||
| ok "Media service stopped" | ||
| fi |
There was a problem hiding this comment.
Inconsistent error handling between Docker and systemd paths.
docker stop uses || true to suppress failures (line 122), but systemctl stop (line 125) does not. With set -e, if the systemd service doesn't exist or is already stopped, the script will exit unexpectedly.
Proposed fix
if [[ "$MEDIA_MODE" == "docker" ]]; then
docker stop "$MEDIA_CONTAINER" >/dev/null 2>&1 || true
ok "Media container stopped"
else
- systemctl stop "$MEDIA_SERVICE"
+ systemctl stop "$MEDIA_SERVICE" 2>/dev/null || true
ok "Media service stopped"
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if [[ "$MEDIA_MODE" == "docker" ]]; then | |
| docker stop "$MEDIA_CONTAINER" >/dev/null 2>&1 || true | |
| ok "Media container stopped" | |
| else | |
| systemctl stop "$MEDIA_SERVICE" | |
| ok "Media service stopped" | |
| fi | |
| if [[ "$MEDIA_MODE" == "docker" ]]; then | |
| docker stop "$MEDIA_CONTAINER" >/dev/null 2>&1 || true | |
| ok "Media container stopped" | |
| else | |
| systemctl stop "$MEDIA_SERVICE" 2>/dev/null || true | |
| ok "Media service stopped" | |
| fi |
🤖 Prompt for AI Agents
In @ubuntu/riven-remount-cycle.sh around lines 121 - 127, The systemd branch is
not tolerant of failures while the Docker branch silences errors (docker stop
... || true), causing unexpected exits under set -e; make the systemctl stop
call behave similarly by guarding it (e.g., check service state with systemctl
is-active "$MEDIA_SERVICE" || true before stopping, or append || true to the
systemctl stop invocation) and keep the ok "Media service stopped" call only
after handling suppressed errors; reference MEDIA_MODE, MEDIA_CONTAINER,
MEDIA_SERVICE, the docker stop and systemctl stop invocations, and the ok
function when making the change.
| warn "Unmount attempt $attempt" | ||
|
|
||
| if umount "$MOUNT_PATH" 2>&1 | grep -q "not mounted"; then | ||
| ok "Mount was already unmounted" | ||
| break | ||
| fi |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider simplifying umount retry logic.
The pattern umount ... 2>&1 | grep -q "not mounted" checks grep's exit status rather than umount's. While this works for detecting "not mounted" messages, other umount failures (e.g., device busy) will fall through to the retry loop silently. Consider a clearer approach:
Alternative approach
warn "Unmount attempt $attempt"
-
- if umount "$MOUNT_PATH" 2>&1 | grep -q "not mounted"; then
- ok "Mount was already unmounted"
- break
- fi
+ umount "$MOUNT_PATH" 2>/dev/null || true
sleep "$WAIT_BETWEEN"This attempts umount, ignores errors, waits, then relies on the is_mounted check at the top of the next iteration.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| warn "Unmount attempt $attempt" | |
| if umount "$MOUNT_PATH" 2>&1 | grep -q "not mounted"; then | |
| ok "Mount was already unmounted" | |
| break | |
| fi | |
| warn "Unmount attempt $attempt" | |
| umount "$MOUNT_PATH" 2>/dev/null || true |
🤖 Prompt for AI Agents
In @ubuntu/riven-remount-cycle.sh around lines 140 - 145, The unmount retry
logic currently pipes umount into grep so the loop reacts to grep's exit status;
instead run umount on "$MOUNT_PATH", capture its output/exit code, and then
explicitly check the output for "not mounted" and call ok "Mount was already
unmounted" + break if found; for all other failures simply warn and continue to
the next iteration (the existing is_mounted check at the top will decide whether
to retry). Update the block around the umount call (referencing the umount
invocation, MOUNT_PATH, warn, ok, and is_mounted) to avoid relying on a pipe to
grep and make the logic explicit and clearer.
| docker start "$RIVEN_CONTAINER" >/dev/null | ||
| ok "Riven container started" | ||
|
|
||
| if [[ "$MEDIA_MODE" == "docker" ]]; then | ||
| docker start "$MEDIA_CONTAINER" >/dev/null | ||
| ok "Media container started" | ||
| sleep 5 | ||
| docker restart "$MEDIA_CONTAINER" >/dev/null | ||
| ok "Media container restarted (post-mount)" | ||
| else | ||
| systemctl start "$MEDIA_SERVICE" | ||
| ok "Media service started" | ||
| sleep 5 | ||
| systemctl restart "$MEDIA_SERVICE" | ||
| ok "Media service restarted (post-mount)" | ||
| fi |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding error handling for service start commands.
Unlike the stop commands, docker start (line 185, 189) and systemctl start (line 195) lack error suppression. If containers or services fail to start due to configuration issues, the script will exit abruptly without useful diagnostics.
Additionally, the purpose of the sleep-and-restart pattern (lines 191-193, 197-199) would benefit from a brief comment explaining why a restart is needed immediately after start.
Proposed improvement
docker start "$RIVEN_CONTAINER" >/dev/null
ok "Riven container started"
if [[ "$MEDIA_MODE" == "docker" ]]; then
docker start "$MEDIA_CONTAINER" >/dev/null
ok "Media container started"
+ # Allow container to initialize before restarting to pick up refreshed mounts
sleep 5
docker restart "$MEDIA_CONTAINER" >/dev/null
ok "Media container restarted (post-mount)"
else
systemctl start "$MEDIA_SERVICE"
ok "Media service started"
+ # Allow service to initialize before restarting to pick up refreshed mounts
sleep 5
systemctl restart "$MEDIA_SERVICE"
ok "Media service restarted (post-mount)"
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| docker start "$RIVEN_CONTAINER" >/dev/null | |
| ok "Riven container started" | |
| if [[ "$MEDIA_MODE" == "docker" ]]; then | |
| docker start "$MEDIA_CONTAINER" >/dev/null | |
| ok "Media container started" | |
| sleep 5 | |
| docker restart "$MEDIA_CONTAINER" >/dev/null | |
| ok "Media container restarted (post-mount)" | |
| else | |
| systemctl start "$MEDIA_SERVICE" | |
| ok "Media service started" | |
| sleep 5 | |
| systemctl restart "$MEDIA_SERVICE" | |
| ok "Media service restarted (post-mount)" | |
| fi | |
| docker start "$RIVEN_CONTAINER" >/dev/null | |
| ok "Riven container started" | |
| if [[ "$MEDIA_MODE" == "docker" ]]; then | |
| docker start "$MEDIA_CONTAINER" >/dev/null | |
| ok "Media container started" | |
| # Allow container to initialize before restarting to pick up refreshed mounts | |
| sleep 5 | |
| docker restart "$MEDIA_CONTAINER" >/dev/null | |
| ok "Media container restarted (post-mount)" | |
| else | |
| systemctl start "$MEDIA_SERVICE" | |
| ok "Media service started" | |
| # Allow service to initialize before restarting to pick up refreshed mounts | |
| sleep 5 | |
| systemctl restart "$MEDIA_SERVICE" | |
| ok "Media service restarted (post-mount)" | |
| fi |
🤖 Prompt for AI Agents
In @ubuntu/riven-remount-cycle.sh around lines 185 - 200, The start/restart
branches for RIVEN_CONTAINER and MEDIA containers/services lack error handling
and diagnostics; update the blocks that reference RIVEN_CONTAINER, MEDIA_MODE,
MEDIA_CONTAINER, and MEDIA_SERVICE to capture stdout/stderr from docker
start/docker restart and systemctl start/systemctl restart, check each command's
exit code, log failures via the existing ok/error logging functions (including
the command output or exit status), and exit with a non-zero code on fatal
failures; also add a brief comment above the sleep+restart sequence explaining
why an immediate restart is required (post-mount remount delay) so future
readers understand the purpose.
- Start media service before Riven to ensure mount readiness - Restart media after Riven for reliable mount propagation - Bump riven-remount-cycle version to v1.4
Increase wait time before restarting media server to ensure Riven is fully initialized and mounts are ready.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @ubuntu/riven-remount-cycle.sh:
- Around line 135-154: The unmount retry loop may exit without unmounting and
the script continues, so after the loop check the mount state and fail fast:
after the for-loop that uses is_mounted, UNMOUNT_RETRIES, umount and
WAIT_BETWEEN, call is_mounted again for MOUNT_PATH and if it still returns true
log a fatal error (use warn/error) indicating unmount failed after
UNMOUNT_RETRIES and exit non-zero (or return non-zero) to avoid creating a bind
mount over a still-mounted path; ensure messages reference MOUNT_PATH and
include attempt/count context.
- Around line 119-128: The script treats docker stops as tolerant (using "docker
stop ... || true") but lets "systemctl stop $MEDIA_SERVICE" potentially fail the
script; make error handling consistent by treating systemd stop failures as
non-fatal: modify the branch that runs systemctl stop "$MEDIA_SERVICE" (and the
earlier docker stop for RIVEN_CONTAINER if needed) to either append "|| true" or
explicitly check the service existence/status before stopping so that
missing/already-stopped services do not cause the script to exit; target the
systemctl stop call and the MEDIA_SERVICE/MEDIA_MODE logic for the change.
- Around line 18-22: The script defines REMOUNT_WAIT and WAIT_TIME but doesn't
use them; replace the hardcoded sleep 5 calls with the constants by changing the
two occurrences of sleep 5 to sleep "$REMOUNT_WAIT" and sleep "$WAIT_TIME"
respectively (or if you prefer, remove the REMOUNT_WAIT and WAIT_TIME variable
declarations and keep the hardcoded sleeps); ensure the variables are exported
or in scope when used and quoted when passed to sleep.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
ubuntu/riven-remount-cycle.sh
🧰 Additional context used
🪛 Shellcheck (0.11.0)
ubuntu/riven-remount-cycle.sh
[warning] 20-20: REMOUNT_WAIT appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 21-21: WAIT_TIME appears unused. Verify use (or export if used externally).
(SC2034)
🔇 Additional comments (8)
ubuntu/riven-remount-cycle.sh (8)
1-2: LGTM!Good use of
set -euo pipefailfor strict error handling.
7-10: LGTM!Root enforcement is clear and correct.
27-34: LGTM!Output helpers are concise and
is_mounted()correctly defers variable resolution to call time.
62-74: LGTM!Case statements correctly handle all branches with clear fail-fast on invalid input.
161-165: LGTM!Bind mount and rshared propagation are correctly applied. Error handling via
set -eis sufficient here.
172-179: LGTM!Propagation verification with explicit fail on unexpected state is a good safeguard.
186-212: LGTM!The start sequence (media → Riven → restart media) correctly ensures mount propagation as described in the PR objectives. The lack of error suppression on
docker startis appropriate since failure to start indicates a real problem.
218-219: LGTM!Clear completion message.
ubuntu/riven-remount-cycle.sh
Outdated
| UNMOUNT_RETRIES=5 | ||
| WAIT_BETWEEN=2 | ||
| REMOUNT_WAIT=5 | ||
| WAIT_TIME=5 | ||
| VERSION=1.4 |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove or use the unused variables.
REMOUNT_WAIT and WAIT_TIME are defined but never used. Lines 196 and 203 use hardcoded sleep 5 instead of these constants.
♻️ Proposed fix: use the defined constants or remove them
Either remove the unused variables:
UNMOUNT_RETRIES=5
WAIT_BETWEEN=2
-REMOUNT_WAIT=5
-WAIT_TIME=5
VERSION=1.4Or use them in the sleep calls (lines 196 and 203):
-sleep 5
+sleep "$WAIT_TIME"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| UNMOUNT_RETRIES=5 | |
| WAIT_BETWEEN=2 | |
| REMOUNT_WAIT=5 | |
| WAIT_TIME=5 | |
| VERSION=1.4 | |
| UNMOUNT_RETRIES=5 | |
| WAIT_BETWEEN=2 | |
| VERSION=1.4 |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 20-20: REMOUNT_WAIT appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 21-21: WAIT_TIME appears unused. Verify use (or export if used externally).
(SC2034)
🤖 Prompt for AI Agents
In @ubuntu/riven-remount-cycle.sh around lines 18 - 22, The script defines
REMOUNT_WAIT and WAIT_TIME but doesn't use them; replace the hardcoded sleep 5
calls with the constants by changing the two occurrences of sleep 5 to sleep
"$REMOUNT_WAIT" and sleep "$WAIT_TIME" respectively (or if you prefer, remove
the REMOUNT_WAIT and WAIT_TIME variable declarations and keep the hardcoded
sleeps); ensure the variables are exported or in scope when used and quoted when
passed to sleep.
- Reduced mount retry attempts to 3 - Bumped script version to v2.0 - Minor reliability tweaks to remount flow
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @ubuntu/riven-remount-cycle.sh:
- Around line 119-128: The systemd branch inconsistently fails under set -e
because systemctl stop on MEDIA_SERVICE can exit non-zero; modify the MEDIA_MODE
== "docker" else branch to first verify the service exists (e.g., with systemctl
list-unit-files or systemctl is-enabled) before calling systemctl stop
"$MEDIA_SERVICE", and if the service is missing log a warning (using warn) and
skip stopping; ensure behavior mirrors the docker branch's tolerant handling of
missing containers (RIVEN_CONTAINER / MEDIA_CONTAINER) so non-existent services
do not cause the script to exit.
- Around line 18-22: Remove or use the unused variables REMOUNT_WAIT and
WAIT_TIME: either delete both variable definitions (REMOUNT_WAIT and WAIT_TIME)
if you prefer the current hardcoded sleep values, or replace the hardcoded sleep
5 and sleep 15 calls with the variables (e.g., sleep "$REMOUNT_WAIT" and sleep
"$WAIT_TIME"), and if choosing to keep and clarify semantics, rename WAIT_TIME
to a more descriptive name like PROPAGATION_WAIT and update its value and usages
accordingly.
- Around line 135-154: The unmount loop incorrectly relies on grepping umount
stdout for "not mounted" and lacks failure handling; change the loop around
is_mounted/umount (using the is_mounted function and MOUNT_PATH) to check
umount's exit status and capture its stderr/output—treat exit code 0 as success,
if stderr contains "not mounted" treat as already unmounted, otherwise log the
error and retry; after the for loop, if is_mounted still returns true (using
UNMOUNT_RETRIES and WAIT_BETWEEN), fail fast (exit non-zero) with an error
message so the script does not proceed to remount when unmounting ultimately
failed.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
ubuntu/riven-remount-cycle.sh
🧰 Additional context used
🪛 Shellcheck (0.11.0)
ubuntu/riven-remount-cycle.sh
[warning] 20-20: REMOUNT_WAIT appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 21-21: WAIT_TIME appears unused. Verify use (or export if used externally).
(SC2034)
🔇 Additional comments (8)
ubuntu/riven-remount-cycle.sh (8)
1-10: LGTM!Good use of
set -euo pipefailfor strict error handling and clear root privilege enforcement.
24-34: LGTM!Clean helper functions for consistent output formatting and a proper mount check using
findmnt.
36-74: LGTM!Well-structured interactive prompts with sensible defaults and input validation.
76-112: LGTM!Good mapping of media servers to their respective Docker container names and systemd service names with a custom fallback option.
156-165: LGTM!Correct approach for establishing bind mount with rshared propagation for container visibility.
167-179: LGTM!Good verification of mount propagation with clear failure messaging.
181-212: LGTM!The restart sequence ensures proper mount propagation visibility. The hardcoded sleep values were addressed in an earlier comment about unused variables.
214-219: LGTM!Clear completion message.
ubuntu/riven-remount-cycle.sh
Outdated
| UNMOUNT_RETRIES=3 | ||
| WAIT_BETWEEN=2 | ||
| REMOUNT_WAIT=5 | ||
| WAIT_TIME=5 | ||
| VERSION=2.0 |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Remove unused variables.
REMOUNT_WAIT and WAIT_TIME are defined but never used in the script. The hardcoded sleep 5 and sleep 15 calls later in the script (lines 196, 203) could use these variables, or they should be removed to reduce confusion.
♻️ Proposed fix: Either remove unused variables or use them
Option 1: Remove unused variables
UNMOUNT_RETRIES=3
WAIT_BETWEEN=2
-REMOUNT_WAIT=5
-WAIT_TIME=5
VERSION=2.0Option 2: Use the variables for the sleep calls
-# 2. Wait before starting Riven
-sleep 5
+# 2. Wait before starting Riven
+sleep "$REMOUNT_WAIT"
-# 4. Wait for mount propagation
-sleep 15
+# 4. Wait for mount propagation
+sleep "$WAIT_TIME"(Note: If using Option 2, consider renaming WAIT_TIME to something more descriptive like PROPAGATION_WAIT and adjusting the value.)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| UNMOUNT_RETRIES=3 | |
| WAIT_BETWEEN=2 | |
| REMOUNT_WAIT=5 | |
| WAIT_TIME=5 | |
| VERSION=2.0 | |
| UNMOUNT_RETRIES=3 | |
| WAIT_BETWEEN=2 | |
| VERSION=2.0 |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 20-20: REMOUNT_WAIT appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 21-21: WAIT_TIME appears unused. Verify use (or export if used externally).
(SC2034)
🤖 Prompt for AI Agents
In @ubuntu/riven-remount-cycle.sh around lines 18 - 22, Remove or use the unused
variables REMOUNT_WAIT and WAIT_TIME: either delete both variable definitions
(REMOUNT_WAIT and WAIT_TIME) if you prefer the current hardcoded sleep values,
or replace the hardcoded sleep 5 and sleep 15 calls with the variables (e.g.,
sleep "$REMOUNT_WAIT" and sleep "$WAIT_TIME"), and if choosing to keep and
clarify semantics, rename WAIT_TIME to a more descriptive name like
PROPAGATION_WAIT and update its value and usages accordingly.
| docker stop "$RIVEN_CONTAINER" >/dev/null 2>&1 || true | ||
| ok "Riven container stopped" | ||
|
|
||
| if [[ "$MEDIA_MODE" == "docker" ]]; then | ||
| docker stop "$MEDIA_CONTAINER" >/dev/null 2>&1 || true | ||
| ok "Media container stopped" | ||
| else | ||
| systemctl stop "$MEDIA_SERVICE" | ||
| ok "Media service stopped" | ||
| fi |
There was a problem hiding this comment.
Inconsistent error handling between Docker and systemd modes.
The docker stop commands use || true to gracefully handle non-existent containers, but systemctl stop on line 126 will cause the script to exit immediately (due to set -e) if the service doesn't exist or fails to stop.
🐛 Proposed fix
if [[ "$MEDIA_MODE" == "docker" ]]; then
docker stop "$MEDIA_CONTAINER" >/dev/null 2>&1 || true
ok "Media container stopped"
else
- systemctl stop "$MEDIA_SERVICE"
+ systemctl stop "$MEDIA_SERVICE" 2>/dev/null || true
ok "Media service stopped"
fiAlternatively, validate the service exists before stopping:
if systemctl list-unit-files "$MEDIA_SERVICE.service" &>/dev/null; then
systemctl stop "$MEDIA_SERVICE"
ok "Media service stopped"
else
warn "Service $MEDIA_SERVICE not found, skipping"
fi🤖 Prompt for AI Agents
In @ubuntu/riven-remount-cycle.sh around lines 119 - 128, The systemd branch
inconsistently fails under set -e because systemctl stop on MEDIA_SERVICE can
exit non-zero; modify the MEDIA_MODE == "docker" else branch to first verify the
service exists (e.g., with systemctl list-unit-files or systemctl is-enabled)
before calling systemctl stop "$MEDIA_SERVICE", and if the service is missing
log a warning (using warn) and skip stopping; ensure behavior mirrors the docker
branch's tolerant handling of missing containers (RIVEN_CONTAINER /
MEDIA_CONTAINER) so non-existent services do not cause the script to exit.
| for attempt in $(seq 1 $UNMOUNT_RETRIES); do | ||
| if ! is_mounted; then | ||
| ok "Mount is already unmounted" | ||
| break | ||
| fi | ||
|
|
||
| warn "Unmount attempt $attempt" | ||
|
|
||
| if umount "$MOUNT_PATH" 2>&1 | grep -q "not mounted"; then | ||
| ok "Mount was already unmounted" | ||
| break | ||
| fi | ||
|
|
||
| sleep "$WAIT_BETWEEN" | ||
|
|
||
| if ! is_mounted; then | ||
| ok "Mount successfully unmounted" | ||
| break | ||
| fi | ||
| done |
There was a problem hiding this comment.
Unmount loop has logic issues and missing failure handling.
-
Flawed umount check (line 143-146): If
umountsucceeds, it produces no output, sogrep -q "not mounted"will never match. This check only works if umount fails with that specific message. -
No failure after retries exhausted: If the mount is still mounted after all retries, the script silently proceeds to remount, which may cause issues.
🐛 Proposed fix
for attempt in $(seq 1 $UNMOUNT_RETRIES); do
if ! is_mounted; then
ok "Mount is already unmounted"
break
fi
warn "Unmount attempt $attempt"
- if umount "$MOUNT_PATH" 2>&1 | grep -q "not mounted"; then
- ok "Mount was already unmounted"
- break
- fi
-
- sleep "$WAIT_BETWEEN"
-
- if ! is_mounted; then
+ if umount "$MOUNT_PATH" 2>/dev/null; then
ok "Mount successfully unmounted"
break
fi
+
+ sleep "$WAIT_BETWEEN"
done
+
+# Fail if still mounted after all retries
+if is_mounted; then
+ fail "Failed to unmount $MOUNT_PATH after $UNMOUNT_RETRIES attempts"
+fi🤖 Prompt for AI Agents
In @ubuntu/riven-remount-cycle.sh around lines 135 - 154, The unmount loop
incorrectly relies on grepping umount stdout for "not mounted" and lacks failure
handling; change the loop around is_mounted/umount (using the is_mounted
function and MOUNT_PATH) to check umount's exit status and capture its
stderr/output—treat exit code 0 as success, if stderr contains "not mounted"
treat as already unmounted, otherwise log the error and retry; after the for
loop, if is_mounted still returns true (using UNMOUNT_RETRIES and WAIT_BETWEEN),
fail fast (exit non-zero) with an error message so the script does not proceed
to remount when unmounting ultimately failed.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@README.md`:
- Around line 54-59: Replace the incorrect top-level directory name in the
README structure diagram: change the string "Riven-Scripts/" to
"distributables/" in the fenced code block so the diagram accurately reflects
this repository; keep the rest of the lines (proxmox/, ubuntu/, README.md) and
the code block formatting intact.
- Around line 19-20: Add a security notice around the direct remote script
execution examples (the bash/wget installer commands shown at the current
installer snippets) that instructs users to review the script before running it,
shows the safer download-then-review pattern (download with curl/wget, inspect
the file, then run with sudo), recommends verifying integrity via checksums or
signatures, and suggests using tagged release URLs instead of the main branch;
place this guidance immediately adjacent to the two installer command blocks
(the ones currently using wget/curl and sudo bash) so readers see the warning
whenever they encounter those commands.
♻️ Duplicate comments (2)
README.md (2)
19-19: Installer URL references different repository.The Proxmox installer URL points to
AquaHorizonGaming/Riven-Scriptsbut this PR is forrivenmedia/distributables. Users viewing this README in therivenmedia/distributablesrepository will download scripts from a different repository, potentially getting outdated or incorrect versions.If
AquaHorizonGaming/Riven-Scriptsis the canonical upstream and this is a downstream mirror, add a note clarifying the relationship. Otherwise, update to reference this repository:-bash -c "$(wget -qLO - https://raw.githubusercontent.com/AquaHorizonGaming/Riven-Scripts/main/proxmox/install.sh)" +bash -c "$(wget -qLO - https://raw.githubusercontent.com/rivenmedia/distributables/main/proxmox/install.sh)"
39-39: Installer URL references different repository.Same issue as line 19: the Ubuntu installer URL points to
AquaHorizonGaming/Riven-Scriptsinstead ofrivenmedia/distributables.Update to:
-sudo bash -c "$(curl -fsSL https://raw.githubusercontent.com/AquaHorizonGaming/Riven-Scripts/main/ubuntu/install.sh)" +sudo bash -c "$(curl -fsSL https://raw.githubusercontent.com/rivenmedia/distributables/main/ubuntu/install.sh)"
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
README.md
🔇 Additional comments (2)
README.md (2)
63-68: LGTM!The Notes section provides clear guidance about platform independence and directs users to platform-specific documentation. The structure is helpful for maintainability and future expansion.
1-1: Repository name mismatch in title requires manual verification.The title references "Riven Scripts" but this repository is
rivenmedia/distributables. This may indicate outdated documentation, a repository rename, or intentional branding for a mirror/distribution repository. Verify whether this repository should be renamed, the title updated, or if the current naming is intentional given the repository's purpose in the rivenmedia organization.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| bash -c "$(wget -qLO - https://raw.githubusercontent.com/AquaHorizonGaming/Riven-Scripts/main/proxmox/install.sh)" | ||
| ``` |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider security guidance for direct script execution.
Both installer commands execute remote scripts directly (with elevated privileges for Ubuntu). While this is a common installation pattern, consider adding security guidance such as:
- Recommending users review the script first before execution
- Providing checksums for verification
- Offering tagged releases instead of pulling from
mainbranch
Example addition to documentation:
> **Security Note:** Always review scripts before executing them with elevated privileges.
> You can download and inspect the script first:
> ```bash
> curl -fsSL https://raw.githubusercontent.com/.../install.sh -o install.sh
> # Review install.sh
> sudo bash install.sh
> ```Also applies to: 39-40
🤖 Prompt for AI Agents
In `@README.md` around lines 19 - 20, Add a security notice around the direct
remote script execution examples (the bash/wget installer commands shown at the
current installer snippets) that instructs users to review the script before
running it, shows the safer download-then-review pattern (download with
curl/wget, inspect the file, then run with sudo), recommends verifying integrity
via checksums or signatures, and suggests using tagged release URLs instead of
the main branch; place this guidance immediately adjacent to the two installer
command blocks (the ones currently using wget/curl and sudo bash) so readers see
the warning whenever they encounter those commands.
| ```text | ||
| Riven-Scripts/ | ||
| ├── proxmox/ # Proxmox VE LXC installer + docs | ||
| ├── ubuntu/ # Ubuntu installer + docs | ||
| └── README.md # This file | ||
| ``` |
There was a problem hiding this comment.
Repository structure shows incorrect directory name.
The repository structure diagram shows Riven-Scripts/ but this repository is distributables. This should be updated to match the actual repository name:
```text
-Riven-Scripts/
+distributables/
├── proxmox/ # Proxmox VE LXC installer + docs
├── ubuntu/ # Ubuntu installer + docs
└── README.md # This file
<details>
<summary>🤖 Prompt for AI Agents</summary>
In @README.md around lines 54 - 59, Replace the incorrect top-level directory
name in the README structure diagram: change the string "Riven-Scripts/" to
"distributables/" in the fenced code block so the diagram accurately reflects
this repository; keep the rest of the lines (proxmox/, ubuntu/, README.md) and
the code block formatting intact.
</details>
<!-- fingerprinting:phantom:triton:puma -->
<!-- This is an auto-generated comment by CodeRabbit -->
Prevents validation messages from being captured during command substitution by redirecting warnings to stderr, ensuring ORIGIN is always a valid URL.
Update scraper selection to support multiple backends in a single install. Accepts space- or comma-separated input and configures each scraper sequentially with per-scraper prompts and validation.
Display a full configuration summary before install and require confirmation. Reduces misconfiguration and failed installs. Adds v0.6.7.
Resolved an installer crash caused by using inline [[ condition ]] && blocks while running with `set -euo pipefail`. Disabled scrapers were returning non-zero exit codes, which caused the script to terminate unexpectedly during the configuration summary phase. All scraper checks have been converted to explicit `if [[ ... ]]; then` blocks, making the logic safe under `set -e` and preventing premature exits when a scraper is disabled or unset. This ensures the configuration summary always completes and accurately reports enabled scrapers.
Bumped the installer version to v0.6.8. This release includes internal refinements and stability improvements made since v0.6.7, with no changes required from end users. Version metadata and related references have been updated accordingly to reflect the new release.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@ubuntu/install.sh`:
- Around line 154-160: LOG_FILE/LOG_DIR and the redirection via exec > >(tee -a
"$LOG_FILE") create a world-readable log in /tmp; fix by creating a secure log
directory and file with restrictive permissions before redirecting output:
ensure LOG_DIR is set to a secure location (or create /var/log/riven), mkdir -p
"$LOG_DIR" then create the file and set permissions (chmod 600 "$LOG_FILE") or
set umask appropriately before touching the file, and only then run the exec >
>(tee -a "$LOG_FILE") 2>&1 redirection so secrets aren't written to a
world-accessible file.
♻️ Duplicate comments (12)
ubuntu/install.sh (12)
12-13: Compose URLs point to forked repository instead of canonical source.The script downloads compose files from
AquaHorizonGaming/distributablesbut this PR is inrivenmedia/distributables. After merge, the script should reference the canonical repository to avoid maintenance issues if the fork diverges.🔧 Proposed fix
-MEDIA_COMPOSE_URL="https://raw.githubusercontent.com/AquaHorizonGaming/distributables/main/ubuntu/docker-compose.media.yml" -RIVEN_COMPOSE_URL="https://raw.githubusercontent.com/AquaHorizonGaming/distributables/main/ubuntu/docker-compose.yml" +MEDIA_COMPOSE_URL="https://raw.githubusercontent.com/rivenmedia/distributables/main/ubuntu/docker-compose.media.yml" +RIVEN_COMPOSE_URL="https://raw.githubusercontent.com/rivenmedia/distributables/main/ubuntu/docker-compose.yml"
92-94: Remove unusedsanitize()function.This function is defined but never called. The sanitization logic is already embedded in
require_non_empty()andrequire_url().🧹 Proposed fix
-sanitize() { - printf "%s" "$1" | tr -d '\r\n' -}
201-208: Consider explicitif-elseinstead ofA && B || Cpattern.If
dpkg -ssucceeds butok()fails for any reason (e.g., stdout issue), theapt-getblock would still execute. This is unlikely but the pattern is fragile.♻️ Suggested fix
-dpkg -s ca-certificates curl gnupg lsb-release openssl fuse3 >/dev/null 2>&1 \ - && ok "System dependencies already installed" \ - || { - apt-get update || fail "apt update failed" - apt-get install -y ca-certificates curl gnupg lsb-release openssl fuse3 \ - || fail "dependency install failed" - ok "System dependencies installed" - } +if dpkg -s ca-certificates curl gnupg lsb-release openssl fuse3 >/dev/null 2>&1; then + ok "System dependencies already installed" +else + apt-get update || fail "apt update failed" + apt-get install -y ca-certificates curl gnupg lsb-release openssl fuse3 \ + || fail "dependency install failed" + ok "System dependencies installed" +fi
315-363: Critical:ensure_riven_rshared_mount()is defined but never called.The function (lines 315-357) creates a systemd unit for persistent rshared mounts, but it's never invoked. Instead, lines 360-361 run one-time mount commands that:
- Use redundant
sudo(script already runs as root)- Won't persist across reboots
- Have inconsistent indentation (line 363)
🐛 Proposed fix
Replace the inline mount commands with a function call:
fi } - -sudo mount --bind $MOUNT_PATH $MOUNT_PATH -sudo mount --make-rshared $MOUNT_PATH - - banner "Mounted $MOUNT_PATH" +ensure_riven_rshared_mount
347-348: Remove unnecessarydaemon-reexec.
systemctl daemon-reexecre-executes the systemd manager itself, which is heavyweight and typically only needed after systemd package upgrades.daemon-reloadalone is sufficient for loading new unit files.♻️ Proposed fix
- systemctl daemon-reexec systemctl daemon-reload
415-460: Input masking warning is incorrect: API keys are collected without masking.Lines 415-418 warn that "input will NOT be visible," but
require_non_empty()(used at lines 431, 447, 458) does not mask input. The warning misleads users about the security of their input. Useread_masked_non_empty()for consistency and security.🔒 Proposed fix
echo "Paste ONLY the API key value below:" - MEDIA_API_KEY="$(require_non_empty "Enter Jellyfin API Key")" + MEDIA_API_KEY="$(read_masked_non_empty "Enter Jellyfin API Key")" ;; plex) ... echo "Paste the token below:" - MEDIA_API_KEY="$(require_non_empty "Enter Plex X-Plex-Token")" + MEDIA_API_KEY="$(read_masked_non_empty "Enter Plex X-Plex-Token")" ;; emby) ... echo "Paste ONLY the API key value below:" - MEDIA_API_KEY="$(require_non_empty "Enter Emby API Key")" + MEDIA_API_KEY="$(read_masked_non_empty "Enter Emby API Key")"
496-536: Downloader API keys collected without masking.Lines 507, 519, and 531 use
require_non_empty()for debrid API tokens, which displays input in plaintext despite the earlier warning about masked input.🔒 Proposed fix
echo "Paste ONLY the API token value below:" - RIVEN_DOWNLOADERS_REAL_DEBRID_API_KEY="$(require_non_empty "Enter Real-Debrid API Token")" + RIVEN_DOWNLOADERS_REAL_DEBRID_API_KEY="$(read_masked_non_empty "Enter Real-Debrid API Token")" ;; 2) ... - RIVEN_DOWNLOADERS_ALL_DEBRID_API_KEY="$(require_non_empty "Enter All-Debrid API Key")" + RIVEN_DOWNLOADERS_ALL_DEBRID_API_KEY="$(read_masked_non_empty "Enter All-Debrid API Key")" ;; 3) ... - RIVEN_DOWNLOADERS_DEBRID_LINK_API_KEY="$(require_non_empty "Enter Debrid-Link API Key")" + RIVEN_DOWNLOADERS_DEBRID_LINK_API_KEY="$(read_masked_non_empty "Enter Debrid-Link API Key")"
879-917: Security:.envfile containing secrets lacks restrictive permissions.The
.envfile contains sensitive credentials (POSTGRES_PASSWORD, API keys, AUTH_SECRET) but nochmod 600is applied. The install summary (line 1000) claims "Permissions: 600" but this is not enforced. The file will have default umask permissions, potentially allowing other users to read secrets.🔒 Proposed fix
EOF +chmod 600 .env + ############################################ # FIX BROKEN MULTILINE ENV VALUES ############################################
972-979: Install summary incomplete: missing Comet, Jackett, and Zilean scrapers.The scraper section in the install summary only displays Torrentio and Prowlarr. Users who selected Comet (3), Jackett (4), or Zilean (5) will incorrectly see "NONE (❌ invalid state)" even though their selection was configured correctly. Note: the earlier configuration summary (lines 766-796) correctly shows all scrapers.
🐛 Proposed fix
echo "🔍 Scraper" if [[ "$RIVEN_SCRAPING_TORRENTIO_ENABLED" == "true" ]]; then echo " • Torrentio" elif [[ "$RIVEN_SCRAPING_PROWLARR_ENABLED" == "true" ]]; then echo " • Prowlarr ($RIVEN_SCRAPING_PROWLARR_URL)" +elif [[ "$RIVEN_SCRAPING_COMET_ENABLED" == "true" ]]; then + echo " • Comet ($RIVEN_SCRAPING_COMET_URL)" +elif [[ "$RIVEN_SCRAPING_JACKETT_ENABLED" == "true" ]]; then + echo " • Jackett ($RIVEN_SCRAPING_JACKETT_URL)" +elif [[ "$RIVEN_SCRAPING_ZILEAN_ENABLED" == "true" ]]; then + echo " • Zilean ($RIVEN_SCRAPING_ZILEAN_URL)" else echo " • NONE (❌ invalid state)" fi
986-988: Security: Secrets printed to stdout and logged to file.
POSTGRES_PASSWORD,BACKEND_API_KEY, andAUTH_SECRETare echoed in plaintext. Due toexec > >(tee -a "$LOG_FILE")at line 160, these persist in/tmp/logs/riven/install-*.log. Consider either:
- Not printing secrets (direct users to
.env)- Writing secrets to
/dev/ttyto bypass tee- Restricting log file permissions
🔒 Proposed fix
echo "🗄️ Database" echo " • Postgres DB: riven" echo " • User: postgres" echo -echo " • POSTGRES PASSWORD: $POSTGRES_PASSWORD" -echo " • BACKEND API KEY: $BACKEND_API_KEY" -echo " • AUTH SECRET: $AUTH_SECRET" +echo " • Credentials: Stored in $INSTALL_DIR/.env"
248-255: Verify Docker Compose v2 plugin is available after installation.The script uses
docker compose(V2 syntax) at lines 395 and 924, butget.docker.comdoes not guarantee the Compose plugin is installed. Add verification or explicit installation.🔧 Proposed fix
systemctl enable --now docker ok "Docker installed" fi + +# Verify Docker Compose v2 is available +if ! docker compose version >/dev/null 2>&1; then + echo "[*] Installing Docker Compose plugin..." + apt-get update + apt-get install -y docker-compose-plugin || fail "Failed to install docker-compose-plugin" +fi +ok "Docker Compose ready"
923-925: Add service health verification before declaring success.The script starts containers with
docker compose up -dbut doesn't verify they're running before printing "INSTALL COMPLETE". If containers fail to start due to configuration errors, users will be misled.♻️ Suggested validation
banner "Starting Riven" docker compose up -d + +echo "Verifying containers started..." +sleep 5 +for svc in riven-db riven riven-frontend; do + if ! docker compose ps "$svc" 2>/dev/null | grep -q "Up\|running"; then + warn "Container $svc may not be running properly" + fi +done + ok "Riven started"
| LOG_FILE="$LOG_DIR/install-$(date +%Y%m%d-%H%M%S).log" | ||
|
|
||
| mkdir -p "$LOG_DIR" | ||
| touch "$LOG_FILE" | ||
|
|
||
| # Mirror stdout + stderr to terminal AND log | ||
| exec > >(tee -a "$LOG_FILE") 2>&1 |
There was a problem hiding this comment.
Log file in /tmp without restrictive permissions may leak secrets.
The log file is created in world-accessible /tmp and no chmod 600 is applied. Since exec > >(tee -a "$LOG_FILE") captures all stdout, the secrets printed later (lines 986-988) will persist in plaintext.
🔒 Proposed fix
mkdir -p "$LOG_DIR"
touch "$LOG_FILE"
+chmod 600 "$LOG_FILE"
# Mirror stdout + stderr to terminal AND log
exec > >(tee -a "$LOG_FILE") 2>&1Additionally, consider using /var/log/riven for persistence or not printing secrets to stdout at all.
🤖 Prompt for AI Agents
In `@ubuntu/install.sh` around lines 154 - 160, LOG_FILE/LOG_DIR and the
redirection via exec > >(tee -a "$LOG_FILE") create a world-readable log in
/tmp; fix by creating a secure log directory and file with restrictive
permissions before redirecting output: ensure LOG_DIR is set to a secure
location (or create /var/log/riven), mkdir -p "$LOG_DIR" then create the file
and set permissions (chmod 600 "$LOG_FILE") or set umask appropriately before
touching the file, and only then run the exec > >(tee -a "$LOG_FILE") 2>&1
redirection so secrets aren't written to a world-accessible file.
Increased remount wait time and updated version number.
Adds an interactive maintenance script for managing Riven show, season, and episode states. Features: - Safe database backup before changes - Search shows by TVDB ID or title - Delete episodes by state (Indexed, Unknown, Failed, Scraped) - Mark episodes or seasons as Unreleased - Reset episodes/seasons back to Indexed - Update season and show-level states - Final verification summary after execution Intended for advanced maintenance and recovery scenarios.
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Fix all issues with AI agents
In `@db-tools/riven-db-maintenance.sh`:
- Around line 275-345: The script interpolates reset_seasons directly into SQL
IN clauses (used in the docker exec psql calls) without validating it; mirror
the Step 2 fix by trimming/resetting whitespace and validating reset_seasons
against a strict comma-separated-numbers regex (e.g. ^[0-9]+(,[0-9]+)*$) and
reject/reprompt on invalid input, then assign the validated value to a sanitized
variable (e.g. sanitized_seasons) and use that variable in the UPDATE queries
instead of raw reset_seasons to prevent injection and malformed SQL in the
UPDATEs that reference s.number IN (...) and s.id IN (...).
- Line 34: Update every use of the shell builtin read in the script to include
the -r flag so backslashes are treated literally; for example change invocations
like read -p "Do you want to create a backup? (y/n): " backup_choice to read -r
-p "Do you want to create a backup? (y/n): " backup_choice, and apply the same
change to all other read calls that populate user-response variables (e.g., the
remaining read usages that assign to their respective variables throughout the
script).
- Around line 12-21: check_container_status currently runs docker ps then checks
the previous exit code via `$?`; change it to test the command directly by using
a negated conditional (e.g., `if ! docker ps | grep -q "riven-db"; then ... fi`)
so the script fails immediately when the "riven-db" container is absent; update
the `check_container_status` function to use this direct exit-code check and
keep the same error message and `exit 1` behavior.
- Around line 166-179: Validate the user-supplied state after computing
target_state by checking it is one of the known allowed values (Indexed,
Unknown, Failed, Scraped): create an allowed_states list/array and perform a
membership check on target_state (case-insensitive check can be done before the
Title Case conversion) immediately after assigning target_state and before
showing the deletion warning and confirmation prompt; if the value is not in the
allowed list, print a clear error message and skip/exit the delete flow so no
invalid queries are run. Ensure you reference the variables target_state and
raw_target_state in the check and place the validation before the confirmation
block that uses delete_confirm.
- Around line 108-124: The script reads a user-provided show ID into show_id and
then interpolates it directly into SQL (see the read -p "Enter the ID..."
prompt, the docker exec riven-db psql ... "WHERE id = $show_id;" command, and
final_condition="parent.id = $show_id"); add input validation to ensure show_id
contains only digits (e.g. match against ^[0-9]+$), re-prompt or exit on invalid
input, and only proceed to build final_condition and run the psql query after
validation to prevent non-numeric input/SQL injection.
- Around line 81-97: The user inputs tvdb_id and show_name are used raw in the
SQL string causing SQL injection; validate and sanitize before building
search_condition: for tvdb_id (used with variable tvdb_id and search_choice)
enforce a strict numeric pattern (e.g., reject unless matches ^[0-9]+$) and for
show_name (used in search_condition and later in the docker exec psql command)
reject overly long values and escape or reject single quotes (or better: only
allow a safe character subset like letters, numbers, spaces, dashes), then
rebuild search_condition using the sanitized values (mi_show.tvdb_id =
'$tvdb_id' only after numeric validation, and mi_show.title ILIKE '%$show_name%'
only after escaping/whitelisting show_name); update the code that constructs the
docker exec riven-db psql -c "... WHERE $search_condition;" to use those
sanitized variables (or switch to a parameterized psql approach) and ensure
MediaItem / mi_show references remain unchanged.
- Around line 222-250: The script is interpolating user input directly into SQL
via target_season and target_eps (used in ep_condition and the docker exec psql
command), risking SQL errors/injection; validate and sanitize inputs before
building ep_condition: ensure target_season matches ^[0-9]+$ (otherwise
re-prompt or abort), and for target_eps either accept the 'A'/'All' branch or
validate the list matches ^[0-9]+(,[0-9]+)*$ (strip whitespace) and rebuild a
safe numeric CSV; only use the sanitized integers when constructing ep_condition
(e.g., s.number = <sanitized season> and e.number IN (<sanitized-ep-list>)) so
no unvalidated user text reaches the SQL command.
- Around line 32-45: Replace the insecure /tmp path and the vulnerable read
invocation: update the script logic around the backup prompt (variable
backup_choice and the docker exec/docker cp steps) to write the dump to a
persistent, user-controlled location (for example use $HOME/riven_backup.sql or
prompt for a destination) instead of /tmp/riven_backup.sql, and change the read
invocation (read -p ...) to use read -r -p to prevent backslash mangling; ensure
docker exec pg_dump writes to the chosen path inside the container and docker cp
copies from that same path to the host destination, and preserve the existing
success/failure checks around the docker commands.
- Around line 399-425: Validate and sanitize the user-provided manual_seasons
before embedding into the SQL IN clause: ensure the variable manual_seasons
contains only digits, commas and optional spaces by splitting on commas and
checking each token is a non-empty numeric string (e.g., /^[0-9]+$/), then
rejoin into a safe comma-separated list; if any token is invalid, abort or
prompt again. Apply this check before using ms_state and the docker exec psql
UPDATE command so only validated numeric season IDs are interpolated into the
query (referencing variables manual_seasons, ms_choice, ms_state and the docker
exec psql command).
| check_container_status() { | ||
| docker ps | grep -q "riven-db" | ||
| if [ $? -ne 0 ]; then | ||
| echo "╔════════════════════════════════════════════════════════════╗" | ||
| echo "║ ERROR: The 'riven-db' container is not running! ║" | ||
| echo "║ Please start the container and try again. ║" | ||
| echo "╚════════════════════════════════════════════════════════════╝" | ||
| exit 1 | ||
| fi | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Use direct exit code check instead of $?.
The pattern of running a command, then checking $? can be simplified.
♻️ Proposed refactor
check_container_status() {
- docker ps | grep -q "riven-db"
- if [ $? -ne 0 ]; then
+ if ! docker ps | grep -q "riven-db"; then
echo "╔════════════════════════════════════════════════════════════╗"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| check_container_status() { | |
| docker ps | grep -q "riven-db" | |
| if [ $? -ne 0 ]; then | |
| echo "╔════════════════════════════════════════════════════════════╗" | |
| echo "║ ERROR: The 'riven-db' container is not running! ║" | |
| echo "║ Please start the container and try again. ║" | |
| echo "╚════════════════════════════════════════════════════════════╝" | |
| exit 1 | |
| fi | |
| } | |
| check_container_status() { | |
| if ! docker ps | grep -q "riven-db"; then | |
| echo "╔════════════════════════════════════════════════════════════╗" | |
| echo "║ ERROR: The 'riven-db' container is not running! ║" | |
| echo "║ Please start the container and try again. ║" | |
| echo "╚════════════════════════════════════════════════════════════╝" | |
| exit 1 | |
| fi | |
| } |
🧰 Tools
🪛 Shellcheck (0.11.0)
[error] 12-12: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 13-13: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[style] 14-14: Check exit code directly with e.g. 'if ! mycmd;', not indirectly with $?.
(SC2181)
[error] 14-14: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 15-15: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 16-16: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 17-17: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 18-18: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 19-19: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 20-20: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 21-21: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
🤖 Prompt for AI Agents
In `@db-tools/riven-db-maintenance.sh` around lines 12 - 21,
check_container_status currently runs docker ps then checks the previous exit
code via `$?`; change it to test the command directly by using a negated
conditional (e.g., `if ! docker ps | grep -q "riven-db"; then ... fi`) so the
script fails immediately when the "riven-db" container is absent; update the
`check_container_status` function to use this direct exit-code check and keep
the same error message and `exit 1` behavior.
| echo "at '/tmp/riven_backup.sql' on your host machine." | ||
| echo "" | ||
| read -p "Do you want to create a backup? (y/n): " backup_choice | ||
| if [[ "$backup_choice" =~ ^[Yy]$ ]]; then | ||
| echo "" | ||
| echo "Creating backup of the database..." | ||
| # Backup inside the container at /tmp/riven_backup.sql | ||
| docker exec riven-db pg_dump -U postgres -d riven -f /tmp/riven_backup.sql | ||
| if [ $? -eq 0 ]; then | ||
| echo "✓ Backup successful inside the container at /tmp/riven_backup.sql." | ||
| # Copy the backup file from the container to the host | ||
| docker cp riven-db:/tmp/riven_backup.sql /tmp/riven_backup.sql | ||
| if [ $? -eq 0 ]; then | ||
| echo "✓ Backup file successfully copied to the host at /tmp/riven_backup.sql." |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Backup location //tmp is insecure and non-persistent.
The backup is stored in /tmp/riven_backup.sql which has two concerns:
/tmpis typically world-readable, potentially exposing database contents/tmpis often cleared on reboot, so backups may be lost
Consider using a more secure, persistent location (e.g., the user's home directory or alongside the Riven installation).
Also, add -r flag to read to prevent backslash mangling:
♻️ Proposed fix
- read -p "Do you want to create a backup? (y/n): " backup_choice
+ read -rp "Do you want to create a backup? (y/n): " backup_choice📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| echo "at '/tmp/riven_backup.sql' on your host machine." | |
| echo "" | |
| read -p "Do you want to create a backup? (y/n): " backup_choice | |
| if [[ "$backup_choice" =~ ^[Yy]$ ]]; then | |
| echo "" | |
| echo "Creating backup of the database..." | |
| # Backup inside the container at /tmp/riven_backup.sql | |
| docker exec riven-db pg_dump -U postgres -d riven -f /tmp/riven_backup.sql | |
| if [ $? -eq 0 ]; then | |
| echo "✓ Backup successful inside the container at /tmp/riven_backup.sql." | |
| # Copy the backup file from the container to the host | |
| docker cp riven-db:/tmp/riven_backup.sql /tmp/riven_backup.sql | |
| if [ $? -eq 0 ]; then | |
| echo "✓ Backup file successfully copied to the host at /tmp/riven_backup.sql." | |
| echo "at '/tmp/riven_backup.sql' on your host machine." | |
| echo "" | |
| read -rp "Do you want to create a backup? (y/n): " backup_choice | |
| if [[ "$backup_choice" =~ ^[Yy]$ ]]; then | |
| echo "" | |
| echo "Creating backup of the database..." | |
| # Backup inside the container at /tmp/riven_backup.sql | |
| docker exec riven-db pg_dump -U postgres -d riven -f /tmp/riven_backup.sql | |
| if [ $? -eq 0 ]; then | |
| echo "✓ Backup successful inside the container at /tmp/riven_backup.sql." | |
| # Copy the backup file from the container to the host | |
| docker cp riven-db:/tmp/riven_backup.sql /tmp/riven_backup.sql | |
| if [ $? -eq 0 ]; then | |
| echo "✓ Backup file successfully copied to the host at /tmp/riven_backup.sql." |
🧰 Tools
🪛 Shellcheck (0.11.0)
[error] 32-32: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 33-33: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[info] 34-34: read without -r will mangle backslashes.
(SC2162)
[error] 34-34: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 35-35: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 36-36: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 37-37: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 38-38: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 39-39: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[style] 40-40: Check exit code directly with e.g. 'if mycmd;', not indirectly with $?.
(SC2181)
[error] 40-40: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 41-41: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 42-42: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 43-43: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[style] 44-44: Check exit code directly with e.g. 'if mycmd;', not indirectly with $?.
(SC2181)
[error] 44-44: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 45-45: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
🤖 Prompt for AI Agents
In `@db-tools/riven-db-maintenance.sh` around lines 32 - 45, Replace the insecure
/tmp path and the vulnerable read invocation: update the script logic around the
backup prompt (variable backup_choice and the docker exec/docker cp steps) to
write the dump to a persistent, user-controlled location (for example use
$HOME/riven_backup.sql or prompt for a destination) instead of
/tmp/riven_backup.sql, and change the read invocation (read -p ...) to use read
-r -p to prevent backslash mangling; ensure docker exec pg_dump writes to the
chosen path inside the container and docker cp copies from that same path to the
host destination, and preserve the existing success/failure checks around the
docker commands.
| echo "database if something goes wrong. The backup will be stored" | ||
| echo "at '/tmp/riven_backup.sql' on your host machine." | ||
| echo "" | ||
| read -p "Do you want to create a backup? (y/n): " backup_choice |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add -r flag to all read commands throughout the script.
Multiple read commands lack the -r flag, which causes backslashes in user input to be interpreted as escape characters. This affects lines 34, 79, 82, 85, 112, 120, 166, 177, 220, 223, 229, 273, 281, 384, 399, 412, and 455.
🧰 Tools
🪛 Shellcheck (0.11.0)
[info] 34-34: read without -r will mangle backslashes.
(SC2162)
[error] 34-34: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
🤖 Prompt for AI Agents
In `@db-tools/riven-db-maintenance.sh` at line 34, Update every use of the shell
builtin read in the script to include the -r flag so backslashes are treated
literally; for example change invocations like read -p "Do you want to create a
backup? (y/n): " backup_choice to read -r -p "Do you want to create a backup?
(y/n): " backup_choice, and apply the same change to all other read calls that
populate user-response variables (e.g., the remaining read usages that assign to
their respective variables throughout the script).
| if [ "$search_choice" == "1" ]; then | ||
| read -p "Enter TVDB ID: " tvdb_id | ||
| search_condition="mi_show.tvdb_id = '$tvdb_id'" | ||
| elif [ "$search_choice" == "2" ]; then | ||
| read -p "Enter show name (partial match works): " show_name | ||
| search_condition="mi_show.title ILIKE '%$show_name%'" | ||
| else | ||
| echo "✗ Invalid choice. Exiting." | ||
| exit 1 | ||
| fi | ||
|
|
||
| # Find the parent show(s) | ||
| result=$(docker exec riven-db psql -U postgres -d riven -t -A -F',' -c " | ||
| SELECT mi_show.id, mi_show.title | ||
| FROM \"MediaItem\" mi_show | ||
| WHERE mi_show.type = 'show' | ||
| AND $search_condition;") |
There was a problem hiding this comment.
Critical: SQL injection vulnerability via unsanitized user input.
User-provided tvdb_id and show_name are directly interpolated into SQL queries without any validation or escaping. A malicious user could enter values like:
- TVDB ID:
1' OR '1'='1— returns all shows - Show name:
'; DELETE FROM "MediaItem" WHERE '1'='1— data destruction
This script runs with full database privileges.
🔒 Proposed fix: Add input validation
if [ "$search_choice" == "1" ]; then
read -p "Enter TVDB ID: " tvdb_id
+ # Validate tvdb_id is numeric only
+ if ! [[ "$tvdb_id" =~ ^[0-9]+$ ]]; then
+ echo "✗ Invalid TVDB ID. Must be numeric."
+ exit 1
+ fi
search_condition="mi_show.tvdb_id = '$tvdb_id'"
elif [ "$search_choice" == "2" ]; then
read -p "Enter show name (partial match works): " show_name
+ # Escape single quotes to prevent SQL injection
+ show_name="${show_name//\'/\'\'}"
search_condition="mi_show.title ILIKE '%$show_name%'"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if [ "$search_choice" == "1" ]; then | |
| read -p "Enter TVDB ID: " tvdb_id | |
| search_condition="mi_show.tvdb_id = '$tvdb_id'" | |
| elif [ "$search_choice" == "2" ]; then | |
| read -p "Enter show name (partial match works): " show_name | |
| search_condition="mi_show.title ILIKE '%$show_name%'" | |
| else | |
| echo "✗ Invalid choice. Exiting." | |
| exit 1 | |
| fi | |
| # Find the parent show(s) | |
| result=$(docker exec riven-db psql -U postgres -d riven -t -A -F',' -c " | |
| SELECT mi_show.id, mi_show.title | |
| FROM \"MediaItem\" mi_show | |
| WHERE mi_show.type = 'show' | |
| AND $search_condition;") | |
| if [ "$search_choice" == "1" ]; then | |
| read -p "Enter TVDB ID: " tvdb_id | |
| # Validate tvdb_id is numeric only | |
| if ! [[ "$tvdb_id" =~ ^[0-9]+$ ]]; then | |
| echo "✗ Invalid TVDB ID. Must be numeric." | |
| exit 1 | |
| fi | |
| search_condition="mi_show.tvdb_id = '$tvdb_id'" | |
| elif [ "$search_choice" == "2" ]; then | |
| read -p "Enter show name (partial match works): " show_name | |
| # Escape single quotes to prevent SQL injection | |
| show_name="${show_name//\'/\'\'}" | |
| search_condition="mi_show.title ILIKE '%$show_name%'" | |
| else | |
| echo "✗ Invalid choice. Exiting." | |
| exit 1 | |
| fi | |
| # Find the parent show(s) | |
| result=$(docker exec riven-db psql -U postgres -d riven -t -A -F',' -c " | |
| SELECT mi_show.id, mi_show.title | |
| FROM \"MediaItem\" mi_show | |
| WHERE mi_show.type = 'show' | |
| AND $search_condition;") |
🧰 Tools
🪛 Shellcheck (0.11.0)
[error] 81-81: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[info] 82-82: read without -r will mangle backslashes.
(SC2162)
[error] 82-82: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 83-83: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 84-84: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[info] 85-85: read without -r will mangle backslashes.
(SC2162)
[error] 85-85: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 86-86: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 87-87: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 88-88: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 89-89: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 90-90: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 91-91: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 92-92: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 97-97: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
🤖 Prompt for AI Agents
In `@db-tools/riven-db-maintenance.sh` around lines 81 - 97, The user inputs
tvdb_id and show_name are used raw in the SQL string causing SQL injection;
validate and sanitize before building search_condition: for tvdb_id (used with
variable tvdb_id and search_choice) enforce a strict numeric pattern (e.g.,
reject unless matches ^[0-9]+$) and for show_name (used in search_condition and
later in the docker exec psql command) reject overly long values and escape or
reject single quotes (or better: only allow a safe character subset like
letters, numbers, spaces, dashes), then rebuild search_condition using the
sanitized values (mi_show.tvdb_id = '$tvdb_id' only after numeric validation,
and mi_show.title ILIKE '%$show_name%' only after escaping/whitelisting
show_name); update the code that constructs the docker exec riven-db psql -c
"... WHERE $search_condition;" to use those sanitized variables (or switch to a
parameterized psql approach) and ensure MediaItem / mi_show references remain
unchanged.
| echo "$result" | while IFS=',' read -r id title; do | ||
| echo " ID: $id | Title: $title" | ||
| done | ||
| echo "" | ||
| read -p "Enter the ID of the show you want to manage: " show_id | ||
| show_title=$(docker exec riven-db psql -U postgres -d riven -t -c "SELECT title FROM \"MediaItem\" WHERE id = $show_id;" | xargs) | ||
| else | ||
| show_id=$(echo "$result" | cut -d',' -f1) | ||
| show_title=$(echo "$result" | cut -d',' -f2) | ||
| echo "" | ||
| echo "✓ Found: $show_title (ID: $show_id)" | ||
| echo "" | ||
| read -p "Proceed with this show? (y/n): " confirm | ||
| [[ ! "$confirm" =~ ^[Yy](es)?$ ]] && exit 0 | ||
| fi | ||
|
|
||
| final_condition="parent.id = $show_id" |
There was a problem hiding this comment.
Validate show_id input is numeric.
When multiple shows match, the user manually enters a show ID (line 112) which is then used directly in SQL queries throughout the script (WHERE id = $show_id). Non-numeric input could cause query failures or SQL injection.
🔒 Proposed fix
read -p "Enter the ID of the show you want to manage: " show_id
+ if ! [[ "$show_id" =~ ^[0-9]+$ ]]; then
+ echo "✗ Invalid show ID. Must be numeric."
+ exit 1
+ fi
show_title=$(docker exec riven-db psql -U postgres -d riven -t -c "SELECT title FROM \"MediaItem\" WHERE id = $show_id;" | xargs)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| echo "$result" | while IFS=',' read -r id title; do | |
| echo " ID: $id | Title: $title" | |
| done | |
| echo "" | |
| read -p "Enter the ID of the show you want to manage: " show_id | |
| show_title=$(docker exec riven-db psql -U postgres -d riven -t -c "SELECT title FROM \"MediaItem\" WHERE id = $show_id;" | xargs) | |
| else | |
| show_id=$(echo "$result" | cut -d',' -f1) | |
| show_title=$(echo "$result" | cut -d',' -f2) | |
| echo "" | |
| echo "✓ Found: $show_title (ID: $show_id)" | |
| echo "" | |
| read -p "Proceed with this show? (y/n): " confirm | |
| [[ ! "$confirm" =~ ^[Yy](es)?$ ]] && exit 0 | |
| fi | |
| final_condition="parent.id = $show_id" | |
| echo "$result" | while IFS=',' read -r id title; do | |
| echo " ID: $id | Title: $title" | |
| done | |
| echo "" | |
| read -p "Enter the ID of the show you want to manage: " show_id | |
| if ! [[ "$show_id" =~ ^[0-9]+$ ]]; then | |
| echo "✗ Invalid show ID. Must be numeric." | |
| exit 1 | |
| fi | |
| show_title=$(docker exec riven-db psql -U postgres -d riven -t -c "SELECT title FROM \"MediaItem\" WHERE id = $show_id;" | xargs) | |
| else | |
| show_id=$(echo "$result" | cut -d',' -f1) | |
| show_title=$(echo "$result" | cut -d',' -f2) | |
| echo "" | |
| echo "✓ Found: $show_title (ID: $show_id)" | |
| echo "" | |
| read -p "Proceed with this show? (y/n): " confirm | |
| [[ ! "$confirm" =~ ^[Yy](es)?$ ]] && exit 0 | |
| fi | |
| final_condition="parent.id = $show_id" |
🧰 Tools
🪛 Shellcheck (0.11.0)
[error] 108-108: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 109-109: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 110-110: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 111-111: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[info] 112-112: read without -r will mangle backslashes.
(SC2162)
[error] 112-112: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 113-113: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 114-114: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 115-115: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 116-116: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 117-117: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 118-118: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 119-119: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[info] 120-120: read without -r will mangle backslashes.
(SC2162)
[error] 120-120: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 121-121: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 122-122: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 123-123: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 124-124: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
🤖 Prompt for AI Agents
In `@db-tools/riven-db-maintenance.sh` around lines 108 - 124, The script reads a
user-provided show ID into show_id and then interpolates it directly into SQL
(see the read -p "Enter the ID..." prompt, the docker exec riven-db psql ...
"WHERE id = $show_id;" command, and final_condition="parent.id = $show_id"); add
input validation to ensure show_id contains only digits (e.g. match against
^[0-9]+$), re-prompt or exit on invalid input, and only proceed to build
final_condition and run the psql query after validation to prevent non-numeric
input/SQL injection.
| read -p "Enter state name to DELETE those episodes (or press Enter to skip): " raw_target_state | ||
|
|
||
| # Format input to Title Case (e.g., indexed -> Indexed) | ||
| target_state=$(echo "${raw_target_state,,}" | sed 's/./\u&/') | ||
|
|
||
| deleted_count=0 | ||
| affected_seasons="" | ||
|
|
||
| if [ ! -z "$target_state" ]; then | ||
| echo "" | ||
| echo "⚠ WARNING: You are about to DELETE all episodes with state: $target_state" | ||
| read -p "Are you absolutely sure? Type 'DELETE' to confirm: " delete_confirm | ||
|
|
||
| if [ "$delete_confirm" == "DELETE" ]; then |
There was a problem hiding this comment.
Validate state input against allowed values.
While the title-case conversion (line 169) provides some protection, the state value should be validated against the known allowed values (Indexed, Unknown, Failed, Scraped) to prevent invalid queries and ensure predictable behavior.
🛡️ Proposed fix
target_state=$(echo "${raw_target_state,,}" | sed 's/./\u&/')
+# Validate state is one of the allowed values
+if [ ! -z "$target_state" ]; then
+ case "$target_state" in
+ Indexed|Unknown|Failed|Scraped) ;;
+ *)
+ echo "✗ Invalid state. Allowed: Indexed, Unknown, Failed, Scraped"
+ target_state=""
+ ;;
+ esac
+fi
+
deleted_count=0📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| read -p "Enter state name to DELETE those episodes (or press Enter to skip): " raw_target_state | |
| # Format input to Title Case (e.g., indexed -> Indexed) | |
| target_state=$(echo "${raw_target_state,,}" | sed 's/./\u&/') | |
| deleted_count=0 | |
| affected_seasons="" | |
| if [ ! -z "$target_state" ]; then | |
| echo "" | |
| echo "⚠ WARNING: You are about to DELETE all episodes with state: $target_state" | |
| read -p "Are you absolutely sure? Type 'DELETE' to confirm: " delete_confirm | |
| if [ "$delete_confirm" == "DELETE" ]; then | |
| read -p "Enter state name to DELETE those episodes (or press Enter to skip): " raw_target_state | |
| # Format input to Title Case (e.g., indexed -> Indexed) | |
| target_state=$(echo "${raw_target_state,,}" | sed 's/./\u&/') | |
| # Validate state is one of the allowed values | |
| if [ ! -z "$target_state" ]; then | |
| case "$target_state" in | |
| Indexed|Unknown|Failed|Scraped) ;; | |
| *) | |
| echo "✗ Invalid state. Allowed: Indexed, Unknown, Failed, Scraped" | |
| target_state="" | |
| ;; | |
| esac | |
| fi | |
| deleted_count=0 | |
| affected_seasons="" | |
| if [ ! -z "$target_state" ]; then | |
| echo "" | |
| echo "⚠ WARNING: You are about to DELETE all episodes with state: $target_state" | |
| read -p "Are you absolutely sure? Type 'DELETE' to confirm: " delete_confirm | |
| if [ "$delete_confirm" == "DELETE" ]; then |
🧰 Tools
🪛 Shellcheck (0.11.0)
[info] 166-166: read without -r will mangle backslashes.
(SC2162)
[error] 166-166: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 167-167: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 168-168: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[style] 169-169: See if you can use ${variable//search/replace} instead.
(SC2001)
[error] 169-169: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 170-170: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 171-171: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 172-172: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 173-173: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 174-174: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 175-175: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 176-176: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[info] 177-177: read without -r will mangle backslashes.
(SC2162)
[error] 177-177: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 178-178: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 179-179: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
🤖 Prompt for AI Agents
In `@db-tools/riven-db-maintenance.sh` around lines 166 - 179, Validate the
user-supplied state after computing target_state by checking it is one of the
known allowed values (Indexed, Unknown, Failed, Scraped): create an
allowed_states list/array and perform a membership check on target_state
(case-insensitive check can be done before the Title Case conversion)
immediately after assigning target_state and before showing the deletion warning
and confirmation prompt; if the value is not in the allowed list, print a clear
error message and skip/exit the delete flow so no invalid queries are run.
Ensure you reference the variables target_state and raw_target_state in the
check and place the validation before the confirmation block that uses
delete_confirm.
| if [[ "$do_unreleased" =~ ^[Yy](es)?$ ]]; then | ||
| read -p "Enter Season Number: " target_season | ||
| echo "" | ||
| echo "Enter Episode Number(s):" | ||
| echo " • For specific episodes: 1,2,3" | ||
| echo " • For ALL episodes in the season: type 'A' or 'All'" | ||
| echo "" | ||
| read -p "Episode Number(s): " target_eps | ||
|
|
||
| # Handle 'A', 'a', or 'All' | ||
| if [[ "${target_eps,,}" =~ ^a(ll)?$ ]]; then | ||
| ep_condition="s.number = $target_season" | ||
| echo "" | ||
| echo "Marking ALL episodes in Season $target_season as Unreleased..." | ||
| else | ||
| ep_condition="s.number = $target_season AND e.number IN ($target_eps)" | ||
| echo "" | ||
| echo "Marking episodes $target_eps in Season $target_season as Unreleased..." | ||
| fi | ||
|
|
||
| docker exec riven-db psql -U postgres -d riven -c " | ||
| UPDATE \"MediaItem\" SET last_state = 'Unreleased' | ||
| WHERE id IN ( | ||
| SELECT mi.id FROM \"MediaItem\" mi | ||
| INNER JOIN \"Episode\" e ON mi.id = e.id | ||
| INNER JOIN \"Season\" s ON e.parent_id = s.id | ||
| WHERE s.parent_id = (SELECT id FROM \"Show\" WHERE id = $show_id) | ||
| AND $ep_condition | ||
| );" |
There was a problem hiding this comment.
Validate season and episode number inputs.
target_season and target_eps are directly interpolated into SQL without validation:
- Line 233:
s.number = $target_season - Line 237:
e.number IN ($target_eps)
Non-numeric input could cause SQL errors or injection attacks.
🔒 Proposed fix
if [[ "$do_unreleased" =~ ^[Yy](es)?$ ]]; then
- read -p "Enter Season Number: " target_season
+ read -rp "Enter Season Number: " target_season
+ if ! [[ "$target_season" =~ ^[0-9]+$ ]]; then
+ echo "✗ Invalid season number. Must be numeric."
+ exit 1
+ fi
echo ""
echo "Enter Episode Number(s):"
echo " • For specific episodes: 1,2,3"
echo " • For ALL episodes in the season: type 'A' or 'All'"
echo ""
- read -p "Episode Number(s): " target_eps
+ read -rp "Episode Number(s): " target_eps
# Handle 'A', 'a', or 'All'
if [[ "${target_eps,,}" =~ ^a(ll)?$ ]]; then
ep_condition="s.number = $target_season"
echo ""
echo "Marking ALL episodes in Season $target_season as Unreleased..."
else
+ # Validate episode numbers are comma-separated digits
+ if ! [[ "$target_eps" =~ ^[0-9]+(,[0-9]+)*$ ]]; then
+ echo "✗ Invalid episode format. Use comma-separated numbers (e.g., 1,2,3)"
+ exit 1
+ fi
ep_condition="s.number = $target_season AND e.number IN ($target_eps)"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if [[ "$do_unreleased" =~ ^[Yy](es)?$ ]]; then | |
| read -p "Enter Season Number: " target_season | |
| echo "" | |
| echo "Enter Episode Number(s):" | |
| echo " • For specific episodes: 1,2,3" | |
| echo " • For ALL episodes in the season: type 'A' or 'All'" | |
| echo "" | |
| read -p "Episode Number(s): " target_eps | |
| # Handle 'A', 'a', or 'All' | |
| if [[ "${target_eps,,}" =~ ^a(ll)?$ ]]; then | |
| ep_condition="s.number = $target_season" | |
| echo "" | |
| echo "Marking ALL episodes in Season $target_season as Unreleased..." | |
| else | |
| ep_condition="s.number = $target_season AND e.number IN ($target_eps)" | |
| echo "" | |
| echo "Marking episodes $target_eps in Season $target_season as Unreleased..." | |
| fi | |
| docker exec riven-db psql -U postgres -d riven -c " | |
| UPDATE \"MediaItem\" SET last_state = 'Unreleased' | |
| WHERE id IN ( | |
| SELECT mi.id FROM \"MediaItem\" mi | |
| INNER JOIN \"Episode\" e ON mi.id = e.id | |
| INNER JOIN \"Season\" s ON e.parent_id = s.id | |
| WHERE s.parent_id = (SELECT id FROM \"Show\" WHERE id = $show_id) | |
| AND $ep_condition | |
| );" | |
| if [[ "$do_unreleased" =~ ^[Yy](es)?$ ]]; then | |
| read -rp "Enter Season Number: " target_season | |
| if ! [[ "$target_season" =~ ^[0-9]+$ ]]; then | |
| echo "✗ Invalid season number. Must be numeric." | |
| exit 1 | |
| fi | |
| echo "" | |
| echo "Enter Episode Number(s):" | |
| echo " • For specific episodes: 1,2,3" | |
| echo " • For ALL episodes in the season: type 'A' or 'All'" | |
| echo "" | |
| read -rp "Episode Number(s): " target_eps | |
| # Handle 'A', 'a', or 'All' | |
| if [[ "${target_eps,,}" =~ ^a(ll)?$ ]]; then | |
| ep_condition="s.number = $target_season" | |
| echo "" | |
| echo "Marking ALL episodes in Season $target_season as Unreleased..." | |
| else | |
| # Validate episode numbers are comma-separated digits | |
| if ! [[ "$target_eps" =~ ^[0-9]+(,[0-9]+)*$ ]]; then | |
| echo "✗ Invalid episode format. Use comma-separated numbers (e.g., 1,2,3)" | |
| exit 1 | |
| fi | |
| ep_condition="s.number = $target_season AND e.number IN ($target_eps)" | |
| echo "" | |
| echo "Marking episodes $target_eps in Season $target_season as Unreleased..." | |
| fi | |
| docker exec riven-db psql -U postgres -d riven -c " | |
| UPDATE \"MediaItem\" SET last_state = 'Unreleased' | |
| WHERE id IN ( | |
| SELECT mi.id FROM \"MediaItem\" mi | |
| INNER JOIN \"Episode\" e ON mi.id = e.id | |
| INNER JOIN \"Season\" s ON e.parent_id = s.id | |
| WHERE s.parent_id = (SELECT id FROM \"Show\" WHERE id = $show_id) | |
| AND $ep_condition | |
| );" |
🧰 Tools
🪛 Shellcheck (0.11.0)
[error] 222-222: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[info] 223-223: read without -r will mangle backslashes.
(SC2162)
[error] 223-223: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 224-224: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 225-225: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 226-226: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 227-227: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 228-228: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[info] 229-229: read without -r will mangle backslashes.
(SC2162)
[error] 229-229: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 230-230: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 231-231: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 232-232: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 233-233: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 234-234: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 235-235: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 236-236: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 237-237: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 238-238: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 239-239: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 240-240: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 241-241: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 250-250: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
🤖 Prompt for AI Agents
In `@db-tools/riven-db-maintenance.sh` around lines 222 - 250, The script is
interpolating user input directly into SQL via target_season and target_eps
(used in ep_condition and the docker exec psql command), risking SQL
errors/injection; validate and sanitize inputs before building ep_condition:
ensure target_season matches ^[0-9]+$ (otherwise re-prompt or abort), and for
target_eps either accept the 'A'/'All' branch or validate the list matches
^[0-9]+(,[0-9]+)*$ (strip whitespace) and rebuild a safe numeric CSV; only use
the sanitized integers when constructing ep_condition (e.g., s.number =
<sanitized season> and e.number IN (<sanitized-ep-list>)) so no unvalidated user
text reaches the SQL command.
| if [[ "$do_reset" =~ ^[Yy](es)?$ ]]; then | ||
| echo "" | ||
| echo "Enter Season Number(s):" | ||
| echo " • For specific seasons: 1,2,3" | ||
| echo " • For ALL seasons: type 'A' or 'All'" | ||
| echo "" | ||
| read -p "Season Number(s): " reset_seasons | ||
|
|
||
| # Handle 'A', 'a', or 'All' | ||
| if [[ "${reset_seasons,,}" =~ ^a(ll)?$ ]]; then | ||
| echo "" | ||
| echo "Resetting ALL episodes and seasons to 'Indexed'..." | ||
|
|
||
| # Reset all episodes to Indexed | ||
| docker exec riven-db psql -U postgres -d riven -c " | ||
| UPDATE \"MediaItem\" | ||
| SET last_state = 'Indexed', | ||
| scraped_at = NULL, | ||
| scraped_times = 0, | ||
| active_stream = NULL | ||
| WHERE id IN ( | ||
| SELECT mi.id FROM \"MediaItem\" mi | ||
| INNER JOIN \"Episode\" e ON mi.id = e.id | ||
| INNER JOIN \"Season\" s ON e.parent_id = s.id | ||
| WHERE s.parent_id = (SELECT id FROM \"Show\" WHERE id = $show_id) | ||
| );" | ||
|
|
||
| # Reset all seasons to Indexed | ||
| docker exec riven-db psql -U postgres -d riven -c " | ||
| UPDATE \"MediaItem\" | ||
| SET last_state = 'Indexed', | ||
| scraped_at = NULL, | ||
| scraped_times = 0, | ||
| active_stream = NULL | ||
| WHERE type = 'season' AND id IN ( | ||
| SELECT s.id FROM \"Season\" s | ||
| WHERE s.parent_id = (SELECT id FROM \"Show\" WHERE id = $show_id) | ||
| );" | ||
|
|
||
| echo "✓ All episodes and seasons reset to 'Indexed'." | ||
| else | ||
| echo "" | ||
| echo "Resetting Season(s) $reset_seasons to 'Indexed'..." | ||
|
|
||
| # Reset episodes in specified seasons to Indexed | ||
| docker exec riven-db psql -U postgres -d riven -c " | ||
| UPDATE \"MediaItem\" | ||
| SET last_state = 'Indexed', | ||
| scraped_at = NULL, | ||
| scraped_times = 0, | ||
| active_stream = NULL | ||
| WHERE id IN ( | ||
| SELECT mi.id FROM \"MediaItem\" mi | ||
| INNER JOIN \"Episode\" e ON mi.id = e.id | ||
| INNER JOIN \"Season\" s ON e.parent_id = s.id | ||
| WHERE s.parent_id = (SELECT id FROM \"Show\" WHERE id = $show_id) | ||
| AND s.number IN ($reset_seasons) | ||
| );" | ||
|
|
||
| # Reset specified seasons to Indexed | ||
| docker exec riven-db psql -U postgres -d riven -c " | ||
| UPDATE \"MediaItem\" | ||
| SET last_state = 'Indexed', | ||
| scraped_at = NULL, | ||
| scraped_times = 0, | ||
| active_stream = NULL | ||
| WHERE type = 'season' AND id IN ( | ||
| SELECT s.id FROM \"Season\" s | ||
| WHERE s.parent_id = (SELECT id FROM \"Show\" WHERE id = $show_id) | ||
| AND s.number IN ($reset_seasons) | ||
| );" |
There was a problem hiding this comment.
Same input validation issue: reset_seasons needs validation.
Similar to Step 2, reset_seasons (line 281) is interpolated directly into SQL IN clauses (lines 331, 344) without validation. Apply the same numeric/comma-separated validation pattern.
🧰 Tools
🪛 Shellcheck (0.11.0)
[error] 275-275: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 276-276: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 277-277: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 278-278: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 279-279: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 280-280: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[info] 281-281: read without -r will mangle backslashes.
(SC2162)
[error] 281-281: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 282-282: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 283-283: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 284-284: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 285-285: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 286-286: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 287-287: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 288-288: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 300-300: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 301-301: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 302-302: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 312-312: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 313-313: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 314-314: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 315-315: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 316-316: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 317-317: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 318-318: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 319-319: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 332-332: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 333-333: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 334-334: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 345-345: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
🤖 Prompt for AI Agents
In `@db-tools/riven-db-maintenance.sh` around lines 275 - 345, The script
interpolates reset_seasons directly into SQL IN clauses (used in the docker exec
psql calls) without validating it; mirror the Step 2 fix by trimming/resetting
whitespace and validating reset_seasons against a strict comma-separated-numbers
regex (e.g. ^[0-9]+(,[0-9]+)*$) and reject/reprompt on invalid input, then
assign the validated value to a sanitized variable (e.g. sanitized_seasons) and
use that variable in the UPDATE queries instead of raw reset_seasons to prevent
injection and malformed SQL in the UPDATEs that reference s.number IN (...) and
s.id IN (...).
| read -p "Update states for any seasons? Enter season numbers (e.g. 1,2) or press Enter to skip: " manual_seasons | ||
| if [ ! -z "$manual_seasons" ]; then | ||
| echo "" | ||
| echo "Choose state for Season(s) $manual_seasons:" | ||
| echo "" | ||
| echo " 1) Completed" | ||
| echo " 2) Ongoing" | ||
| echo " 3) Unreleased" | ||
| echo " 4) PartiallyCompleted" | ||
| echo " 5) Indexed" | ||
| echo " 6) Scraped" | ||
| echo " 7) Paused" | ||
| echo "" | ||
| read -p "Choice (1-7): " ms_choice | ||
| case $ms_choice in | ||
| 1) ms_state="Completed" ;; | ||
| 2) ms_state="Ongoing" ;; | ||
| 3) ms_state="Unreleased" ;; | ||
| 4) ms_state="PartiallyCompleted" ;; | ||
| 5) ms_state="Indexed" ;; | ||
| 6) ms_state="Scraped" ;; | ||
| 7) ms_state="Paused" ;; | ||
| esac | ||
| if [ ! -z "$ms_state" ]; then | ||
| docker exec riven-db psql -U postgres -d riven -c "UPDATE \"MediaItem\" SET last_state = '$ms_state' WHERE type = 'season' AND id IN (SELECT s.id FROM \"Season\" s WHERE s.parent_id = (SELECT id FROM \"Show\" WHERE id = $show_id) AND s.number IN ($manual_seasons));" | ||
| echo "✓ Seasons $manual_seasons updated to $ms_state." | ||
| fi |
There was a problem hiding this comment.
Validate manual_seasons input.
User-entered season numbers (line 399) are used directly in the SQL IN clause (line 423) without validation. Apply numeric validation as recommended for other season inputs.
🔒 Proposed fix
- read -p "Update states for any seasons? Enter season numbers (e.g. 1,2) or press Enter to skip: " manual_seasons
+ read -rp "Update states for any seasons? Enter season numbers (e.g. 1,2) or press Enter to skip: " manual_seasons
if [ ! -z "$manual_seasons" ]; then
+ if ! [[ "$manual_seasons" =~ ^[0-9]+(,[0-9]+)*$ ]]; then
+ echo "✗ Invalid season format. Use comma-separated numbers (e.g., 1,2)"
+ manual_seasons=""
+ fi
+ fi
+ if [ ! -z "$manual_seasons" ]; then
echo ""📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| read -p "Update states for any seasons? Enter season numbers (e.g. 1,2) or press Enter to skip: " manual_seasons | |
| if [ ! -z "$manual_seasons" ]; then | |
| echo "" | |
| echo "Choose state for Season(s) $manual_seasons:" | |
| echo "" | |
| echo " 1) Completed" | |
| echo " 2) Ongoing" | |
| echo " 3) Unreleased" | |
| echo " 4) PartiallyCompleted" | |
| echo " 5) Indexed" | |
| echo " 6) Scraped" | |
| echo " 7) Paused" | |
| echo "" | |
| read -p "Choice (1-7): " ms_choice | |
| case $ms_choice in | |
| 1) ms_state="Completed" ;; | |
| 2) ms_state="Ongoing" ;; | |
| 3) ms_state="Unreleased" ;; | |
| 4) ms_state="PartiallyCompleted" ;; | |
| 5) ms_state="Indexed" ;; | |
| 6) ms_state="Scraped" ;; | |
| 7) ms_state="Paused" ;; | |
| esac | |
| if [ ! -z "$ms_state" ]; then | |
| docker exec riven-db psql -U postgres -d riven -c "UPDATE \"MediaItem\" SET last_state = '$ms_state' WHERE type = 'season' AND id IN (SELECT s.id FROM \"Season\" s WHERE s.parent_id = (SELECT id FROM \"Show\" WHERE id = $show_id) AND s.number IN ($manual_seasons));" | |
| echo "✓ Seasons $manual_seasons updated to $ms_state." | |
| fi | |
| read -rp "Update states for any seasons? Enter season numbers (e.g. 1,2) or press Enter to skip: " manual_seasons | |
| if [ ! -z "$manual_seasons" ]; then | |
| if ! [[ "$manual_seasons" =~ ^[0-9]+(,[0-9]+)*$ ]]; then | |
| echo "✗ Invalid season format. Use comma-separated numbers (e.g., 1,2)" | |
| manual_seasons="" | |
| fi | |
| fi | |
| if [ ! -z "$manual_seasons" ]; then | |
| echo "" | |
| echo "Choose state for Season(s) $manual_seasons:" | |
| echo "" | |
| echo " 1) Completed" | |
| echo " 2) Ongoing" | |
| echo " 3) Unreleased" | |
| echo " 4) PartiallyCompleted" | |
| echo " 5) Indexed" | |
| echo " 6) Scraped" | |
| echo " 7) Paused" | |
| echo "" | |
| read -p "Choice (1-7): " ms_choice | |
| case $ms_choice in | |
| 1) ms_state="Completed" ;; | |
| 2) ms_state="Ongoing" ;; | |
| 3) ms_state="Unreleased" ;; | |
| 4) ms_state="PartiallyCompleted" ;; | |
| 5) ms_state="Indexed" ;; | |
| 6) ms_state="Scraped" ;; | |
| 7) ms_state="Paused" ;; | |
| esac | |
| if [ ! -z "$ms_state" ]; then | |
| docker exec riven-db psql -U postgres -d riven -c "UPDATE \"MediaItem\" SET last_state = '$ms_state' WHERE type = 'season' AND id IN (SELECT s.id FROM \"Season\" s WHERE s.parent_id = (SELECT id FROM \"Show\" WHERE id = $show_id) AND s.number IN ($manual_seasons));" | |
| echo "✓ Seasons $manual_seasons updated to $ms_state." | |
| fi | |
| fi |
🧰 Tools
🪛 Shellcheck (0.11.0)
[info] 399-399: read without -r will mangle backslashes.
(SC2162)
[error] 399-399: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 400-400: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 401-401: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 402-402: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 403-403: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 404-404: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 405-405: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 406-406: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 407-407: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 408-408: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 409-409: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 410-410: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 411-411: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[info] 412-412: read without -r will mangle backslashes.
(SC2162)
[error] 412-412: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 413-413: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 414-414: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 415-415: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 416-416: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 417-417: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 418-418: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 419-419: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 420-420: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 421-421: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 422-422: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 423-423: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 424-424: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
[error] 425-425: Literal carriage return. Run script through tr -d '\r' .
(SC1017)
🤖 Prompt for AI Agents
In `@db-tools/riven-db-maintenance.sh` around lines 399 - 425, Validate and
sanitize the user-provided manual_seasons before embedding into the SQL IN
clause: ensure the variable manual_seasons contains only digits, commas and
optional spaces by splitting on commas and checking each token is a non-empty
numeric string (e.g., /^[0-9]+$/), then rejoin into a safe comma-separated list;
if any token is invalid, abort or prompt again. Apply this check before using
ms_state and the docker exec psql UPDATE command so only validated numeric
season IDs are interpolated into the query (referencing variables
manual_seasons, ms_choice, ms_state and the docker exec psql command).
Adds documentation for the riven-db-maintenance.sh script, including: - Usage instructions - Table of contents - Safety guarantees and warnings - Supported environments (Ubuntu / Proxmox LXC) - Backup-first maintenance workflow Keeps documentation consistent with existing Riven installer READMEs.
Adds explicit anchor tags so the table of contents links work correctly on GitHub. Improves navigation and keeps documentation consistent with other Riven READMEs.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@db-tools/readme.md`:
- Around line 11-14: The README claim "Proxmox LXC (unprivileged)" is
inconsistent with the PR's target of privileged containers; update the README
entry in db-tools/readme.md to state "Proxmox LXC (privileged)" and add a
one-line note explaining why privileged LXC is required (mount propagation,
Docker-in-LXC, or Docker volume behavior) or alternatively describe both options
with their constraints (unprivileged: limited/no mount propagation; privileged:
required for full Docker/mount behavior). Ensure the wording change and the
explanatory note reference "Proxmox LXC" so it's clear to users which container
mode is needed.
- Around line 110-123: Add blank lines before and after the three Markdown
headings "Routine maintenance", "If Riven behaves incorrectly", and "Last-resort
recovery" to satisfy markdownlint rule MD022; update the section so each heading
is separated from surrounding text/ lists by a blank line (i.e., ensure an empty
line above the heading and an empty line between the heading and the following
list) so the headings render and lint correctly.
| Works on: | ||
| - Ubuntu installs | ||
| - Proxmox LXC (unprivileged) | ||
| - Any Docker-based Riven setup |
There was a problem hiding this comment.
Clarify LXC privilege requirement (doc mismatch).
The README says “Proxmox LXC (unprivileged)”, but the PR objectives explicitly target privileged LXC containers. This mismatch can lead users to deploy the wrong container type and break Docker/mount propagation behavior. Please align the wording with the intended privileged requirement (or explain both with constraints).
🤖 Prompt for AI Agents
In `@db-tools/readme.md` around lines 11 - 14, The README claim "Proxmox LXC
(unprivileged)" is inconsistent with the PR's target of privileged containers;
update the README entry in db-tools/readme.md to state "Proxmox LXC
(privileged)" and add a one-line note explaining why privileged LXC is required
(mount propagation, Docker-in-LXC, or Docker volume behavior) or alternatively
describe both options with their constraints (unprivileged: limited/no mount
propagation; privileged: required for full Docker/mount behavior). Ensure the
wording change and the explanatory note reference "Proxmox LXC" so it's clear to
users which container mode is needed.
| ## 🔁 Recommended Usage | ||
|
|
||
| ### Routine maintenance | ||
| 1. Backup | ||
| 2. Vacuum & Analyze | ||
| 3. Exit | ||
|
|
||
| ### If Riven behaves incorrectly | ||
| 1. Backup | ||
| 2. Clean stale/orphaned data | ||
| 3. Restart Riven | ||
|
|
||
| ### Last-resort recovery | ||
| 1. Backup |
There was a problem hiding this comment.
Add blank lines around headings to satisfy MD022.
markdownlint reports missing blank lines around the “Routine maintenance”, “If Riven behaves incorrectly”, and “Last-resort recovery” headings.
✍️ Suggested formatting fix
### Routine maintenance
1. Backup
2. Vacuum & Analyze
3. Exit
### If Riven behaves incorrectly
1. Backup
2. Clean stale/orphaned data
3. Restart Riven
### Last-resort recovery
1. Backup
2. Reset database
3. Restart Riven
4. Reconfigure and rescrape🧰 Tools
🪛 LanguageTool
[grammar] ~117-~117: Ensure spelling is correct
Context: .... Exit ### If Riven behaves incorrectly 1. Backup 2. Clean stale/orphaned data 3. Restart Riven #...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.20.0)
[warning] 112-112: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 117-117: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 122-122: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🤖 Prompt for AI Agents
In `@db-tools/readme.md` around lines 110 - 123, Add blank lines before and after
the three Markdown headings "Routine maintenance", "If Riven behaves
incorrectly", and "Last-resort recovery" to satisfy markdownlint rule MD022;
update the section so each heading is separated from surrounding text/ lists by
a blank line (i.e., ensure an empty line above the heading and an empty line
between the heading and the following list) so the headings render and lint
correctly.
This script provides functionality to manage MediaItem states in a Dockerized PostgreSQL database. It includes backup creation, state item fetching, resetting states, and displaying current counts.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@db-tools/db_pegger_9000.sh`:
- Line 14: Split the combined declaration+assignment for timestamp (replace
"local timestamp=$(date ...)" with a separate declaration and assignment) to
avoid masking return values; change all read invocations to use read -r (the
interactive prompts that use read at multiple places) to prevent backslash
mangling; and replace any checks against "$?" with direct exit checks (use if !
<command> or if [ $? -ne 0 ] as appropriate) so exit codes are evaluated
robustly (search for the timestamp variable, the read calls, and the places
checking exit status/using "$?" to locate edits).
- Around line 111-127: The psql outputs saved into indexed_count, paused_count,
unknown_count, failed_count and completed_count include leading/trailing
whitespace; update the docker exec psql invocations inside show_current_counts
to trim them (for example add the unaligned flag -A: use psql -t -A -c "SELECT
count(*) ..." or pipe the output through a trim (xargs or sed) so the variables
contain clean numbers) and ensure all five count variables are changed
consistently so the echo lines print without extra spaces.
- Around line 4-10: The container-name check in check_container_status is too
loose (grep "riven-db" can match substrings); change the check to target the
specific container name and running state (e.g., use docker inspect or docker ps
with a name filter) so only a container named exactly "riven-db" is considered
and ensure it is running; update check_container_status to call docker inspect
--format '{{.State.Running}}' riven-db (or docker ps --filter "name=^/riven-db$"
--filter "status=running") and handle non-running/missing results by logging the
same error and exiting.
- Around line 23-27: The script currently runs docker exec riven-db pg_dump -U
postgres -d riven -f "$backup_file" which writes the dump file inside the
container; change it to stream the dump to the host by removing the -f option
and redirecting docker exec's stdout to the host backup_file (e.g., docker exec
riven-db pg_dump -U postgres -d riven and redirect to "$backup_file" on the
host), and update the success check to validate the actual pg_dump exit status
(use pipefail or check docker exec exit code) so the echo "Backup created
successfully: $backup_file" only runs when the host file was written
successfully.
- Around line 62-78: The current pattern runs a full SQL block via `docker exec
riven-db psql -U postgres -d riven -c` including `BEGIN`/`COMMIT` but then calls
a separate `docker exec ... psql ... -c "ROLLBACK;"` on shell failure which is
ineffective; remove that external `ROLLBACK` and instead ensure the single
`psql` invocation performs atomic rollback on SQL errors (e.g., invoke `psql`
with psql error handling such as --set=ON_ERROR_STOP=on or
--set=ON_ERROR_ROLLBACK=on, or wrap the update in a PL/pgSQL block with
EXCEPTION handling) so the transaction around `UPDATE "MediaItem"` for `$state`
is properly rolled back when the update fails and no misleading rollback call
remains.
| check_container_status() { | ||
| docker ps | grep -q "riven-db" | ||
| if [ $? -ne 0 ]; then | ||
| echo "Error: The 'riven-db' container is not running. Please start the container and try again." | ||
| exit 1 | ||
| fi | ||
| } |
There was a problem hiding this comment.
Container name match is too loose.
grep -q "riven-db" matches any container whose docker ps output contains "riven-db" anywhere (e.g., image name, other columns, or a similarly-named container like my-riven-db-backup).
🛡️ Proposed fix: use docker inspect or filter
check_container_status() {
- docker ps | grep -q "riven-db"
- if [ $? -ne 0 ]; then
+ if ! docker ps --filter "name=^riven-db$" --format '{{.Names}}' | grep -q '^riven-db$'; then
echo "Error: The 'riven-db' container is not running. Please start the container and try again."
exit 1
fi
}🧰 Tools
🪛 Shellcheck (0.11.0)
[style] 6-6: Check exit code directly with e.g. 'if ! mycmd;', not indirectly with $?.
(SC2181)
🤖 Prompt for AI Agents
In `@db-tools/db_pegger_9000.sh` around lines 4 - 10, The container-name check in
check_container_status is too loose (grep "riven-db" can match substrings);
change the check to target the specific container name and running state (e.g.,
use docker inspect or docker ps with a name filter) so only a container named
exactly "riven-db" is considered and ensure it is running; update
check_container_status to call docker inspect --format '{{.State.Running}}'
riven-db (or docker ps --filter "name=^/riven-db$" --filter "status=running")
and handle non-running/missing results by logging the same error and exiting.
|
|
||
| # Function to perform a backup | ||
| backup_database() { | ||
| local timestamp=$(date +"%Y-%m-%d_%H-%M-%S") |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Apply Shellcheck recommendations for robustness.
Several Shellcheck hints apply:
- SC2155: Declare and assign separately to avoid masking return values (line 14)
- SC2162: Use
read -rto prevent backslash mangling (lines 18, 41, 58, 87) - SC2181: Check exit codes directly (lines 6, 26, 72)
♻️ Example fixes
backup_database() {
- local timestamp=$(date +"%Y-%m-%d_%H-%M-%S")
+ local timestamp
+ timestamp=$(date +"%Y-%m-%d_%H-%M-%S")
local backup_file="/tmp/riven_backup_${timestamp}.sql"
echo "Do you want to create a backup of the database before proceeding?"
- read -p "Type 'y' to back up or 'n' to skip: " backup_choice
+ read -rp "Type 'y' to back up or 'n' to skip: " backup_choiceApply -r flag to all read calls and refactor exit-code checks:
check_container_status() {
- docker ps | grep -q "riven-db"
- if [ $? -ne 0 ]; then
+ if ! docker ps | grep -q "riven-db"; thenAlso applies to: 18-18, 41-41, 58-58, 87-87
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 14-14: Declare and assign separately to avoid masking return values.
(SC2155)
🤖 Prompt for AI Agents
In `@db-tools/db_pegger_9000.sh` at line 14, Split the combined
declaration+assignment for timestamp (replace "local timestamp=$(date ...)" with
a separate declaration and assignment) to avoid masking return values; change
all read invocations to use read -r (the interactive prompts that use read at
multiple places) to prevent backslash mangling; and replace any checks against
"$?" with direct exit checks (use if ! <command> or if [ $? -ne 0 ] as
appropriate) so exit codes are evaluated robustly (search for the timestamp
variable, the read calls, and the places checking exit status/using "$?" to
locate edits).
| # Use docker exec to dump the database | ||
| docker exec riven-db pg_dump -U postgres -d riven -f "$backup_file" | ||
|
|
||
| if [ $? -eq 0 ]; then | ||
| echo "Backup created successfully: $backup_file" |
There was a problem hiding this comment.
Critical: Backup is written inside the container, not on the host.
pg_dump -f "$backup_file" writes to /tmp inside the container, not on the host filesystem. The backup will be lost when the container is removed or recreated. The success message misleadingly suggests the file is accessible on the host.
🐛 Proposed fix: stream the dump to the host
- # Use docker exec to dump the database
- docker exec riven-db pg_dump -U postgres -d riven -f "$backup_file"
+ # Stream pg_dump output to host filesystem
+ docker exec riven-db pg_dump -U postgres -d riven > "$backup_file"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Use docker exec to dump the database | |
| docker exec riven-db pg_dump -U postgres -d riven -f "$backup_file" | |
| if [ $? -eq 0 ]; then | |
| echo "Backup created successfully: $backup_file" | |
| # Stream pg_dump output to host filesystem | |
| docker exec riven-db pg_dump -U postgres -d riven > "$backup_file" | |
| if [ $? -eq 0 ]; then | |
| echo "Backup created successfully: $backup_file" |
🧰 Tools
🪛 Shellcheck (0.11.0)
[style] 26-26: Check exit code directly with e.g. 'if mycmd;', not indirectly with $?.
(SC2181)
🤖 Prompt for AI Agents
In `@db-tools/db_pegger_9000.sh` around lines 23 - 27, The script currently runs
docker exec riven-db pg_dump -U postgres -d riven -f "$backup_file" which writes
the dump file inside the container; change it to stream the dump to the host by
removing the -f option and redirecting docker exec's stdout to the host
backup_file (e.g., docker exec riven-db pg_dump -U postgres -d riven and
redirect to "$backup_file" on the host), and update the success check to
validate the actual pg_dump exit status (use pipefail or check docker exec exit
code) so the echo "Backup created successfully: $backup_file" only runs when the
host file was written successfully.
| docker exec riven-db psql -U postgres -d riven -c " | ||
| BEGIN; | ||
| UPDATE \"MediaItem\" | ||
| SET last_state = 'Indexed', | ||
| scraped_at = NULL, | ||
| scraped_times = 0, | ||
| active_stream = NULL | ||
| WHERE last_state = '$state'; | ||
| COMMIT; | ||
| " | ||
| if [ $? -eq 0 ]; then | ||
| echo "'$state' items successfully reset to 'Indexed'." | ||
| else | ||
| echo "Error: Database update failed for '$state'. Rolling back changes." | ||
| docker exec riven-db psql -U postgres -d riven -c "ROLLBACK;" | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
ROLLBACK on failure is ineffective—transaction already ended.
The psql -c command executes the entire SQL block (including COMMIT) in one shot. If the UPDATE fails, COMMIT isn't reached, but the subsequent ROLLBACK on line 76 runs in a new psql session and has no effect. Additionally, if COMMIT succeeds but the shell check fails for another reason, you'd attempt to roll back an already-committed transaction.
Consider using set -e within the SQL or checking rows affected, and removing the misleading ROLLBACK call.
🔧 Proposed fix: simplify error handling
docker exec riven-db psql -U postgres -d riven -c "
BEGIN;
UPDATE \"MediaItem\"
SET last_state = 'Indexed',
scraped_at = NULL,
scraped_times = 0,
active_stream = NULL
WHERE last_state = '$state';
COMMIT;
"
if [ $? -eq 0 ]; then
echo "'$state' items successfully reset to 'Indexed'."
else
echo "Error: Database update failed for '$state'."
- docker exec riven-db psql -U postgres -d riven -c "ROLLBACK;"
exit 1
fiFor true atomicity with rollback on error, consider using psql with ON_ERROR_ROLLBACK or wrapping in a PL/pgSQL block with exception handling.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| docker exec riven-db psql -U postgres -d riven -c " | |
| BEGIN; | |
| UPDATE \"MediaItem\" | |
| SET last_state = 'Indexed', | |
| scraped_at = NULL, | |
| scraped_times = 0, | |
| active_stream = NULL | |
| WHERE last_state = '$state'; | |
| COMMIT; | |
| " | |
| if [ $? -eq 0 ]; then | |
| echo "'$state' items successfully reset to 'Indexed'." | |
| else | |
| echo "Error: Database update failed for '$state'. Rolling back changes." | |
| docker exec riven-db psql -U postgres -d riven -c "ROLLBACK;" | |
| exit 1 | |
| fi | |
| docker exec riven-db psql -U postgres -d riven -c " | |
| BEGIN; | |
| UPDATE \"MediaItem\" | |
| SET last_state = 'Indexed', | |
| scraped_at = NULL, | |
| scraped_times = 0, | |
| active_stream = NULL | |
| WHERE last_state = '$state'; | |
| COMMIT; | |
| " | |
| if [ $? -eq 0 ]; then | |
| echo "'$state' items successfully reset to 'Indexed'." | |
| else | |
| echo "Error: Database update failed for '$state'." | |
| exit 1 | |
| fi |
🧰 Tools
🪛 Shellcheck (0.11.0)
[style] 72-72: Check exit code directly with e.g. 'if mycmd;', not indirectly with $?.
(SC2181)
🤖 Prompt for AI Agents
In `@db-tools/db_pegger_9000.sh` around lines 62 - 78, The current pattern runs a
full SQL block via `docker exec riven-db psql -U postgres -d riven -c` including
`BEGIN`/`COMMIT` but then calls a separate `docker exec ... psql ... -c
"ROLLBACK;"` on shell failure which is ineffective; remove that external
`ROLLBACK` and instead ensure the single `psql` invocation performs atomic
rollback on SQL errors (e.g., invoke `psql` with psql error handling such as
--set=ON_ERROR_STOP=on or --set=ON_ERROR_ROLLBACK=on, or wrap the update in a
PL/pgSQL block with EXCEPTION handling) so the transaction around `UPDATE
"MediaItem"` for `$state` is properly rolled back when the update fails and no
misleading rollback call remains.
| # Function to display current state counts after reset | ||
| show_current_counts() { | ||
| echo "Fetching current counts of MediaItem states..." | ||
|
|
||
| indexed_count=$(docker exec riven-db psql -U postgres -d riven -t -c "SELECT count(*) FROM \"MediaItem\" WHERE last_state = 'Indexed';") | ||
| paused_count=$(docker exec riven-db psql -U postgres -d riven -t -c "SELECT count(*) FROM \"MediaItem\" WHERE last_state = 'Paused';") | ||
| unknown_count=$(docker exec riven-db psql -U postgres -d riven -t -c "SELECT count(*) FROM \"MediaItem\" WHERE last_state = 'Unknown';") | ||
| failed_count=$(docker exec riven-db psql -U postgres -d riven -t -c "SELECT count(*) FROM \"MediaItem\" WHERE last_state = 'Failed';") | ||
| completed_count=$(docker exec riven-db psql -U postgres -d riven -t -c "SELECT count(*) FROM \"MediaItem\" WHERE last_state = 'Completed';") | ||
|
|
||
| echo "Current MediaItem States:" | ||
| echo " - Indexed: $indexed_count" | ||
| echo " - Paused: $paused_count" | ||
| echo " - Unknown: $unknown_count" | ||
| echo " - Failed: $failed_count" | ||
| echo " - Completed: $completed_count" | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Counts output includes leading whitespace from psql.
psql -t outputs values with leading/trailing whitespace. The echo statements will display something like " - Indexed: 42" with extra spaces. Consider trimming.
♻️ Proposed fix: trim whitespace
- indexed_count=$(docker exec riven-db psql -U postgres -d riven -t -c "SELECT count(*) FROM \"MediaItem\" WHERE last_state = 'Indexed';")
+ indexed_count=$(docker exec riven-db psql -U postgres -d riven -t -c "SELECT count(*) FROM \"MediaItem\" WHERE last_state = 'Indexed';" | tr -d '[:space:]')Apply to all count variables.
🤖 Prompt for AI Agents
In `@db-tools/db_pegger_9000.sh` around lines 111 - 127, The psql outputs saved
into indexed_count, paused_count, unknown_count, failed_count and
completed_count include leading/trailing whitespace; update the docker exec psql
invocations inside show_current_counts to trim them (for example add the
unaligned flag -A: use psql -t -A -c "SELECT count(*) ..." or pipe the output
through a trim (xargs or sed) so the variables contain clean numbers) and ensure
all five count variables are changed consistently so the echo lines print
without extra spaces.
1 participant
This PR adds fully automated, Docker-based installation scripts for Riven on both
standard Ubuntu systems and Proxmox LXC containers.
What’s included
Ubuntu installer
/mnt/riven/backend/settings.json is configured with at least one scraper
and one media server
Proxmox LXC installer
Summary by CodeRabbit
New Features
Documentation
Chores