Fix delete remote directory hang and add UMASK env var

[nitrobass24]

Summary

• Fix Delete Remote on directory hanging - Resolves infinite spinner when deleting a remote directory
• Add UMASK environment variable - Allow users to control file permissions for downloaded/extracted files via UMASK env var (e.g. 002 for 775/664, 000 for 777/666), so other services like Plex and Sonarr can read/write files on shared volumes. Fixes BUG: 0.12.4 - DL file permissions #91.

Test plan

• Build Docker image locally: make build
• Run with UMASK=000, download a file, verify permissions are 666/777
• Run without UMASK set, verify default behavior unchanged (644/755)
• Run with UMASK=002, verify permissions are 664/775
• Test Delete Remote on a directory, verify no infinite spinner

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • UI now clears invalid file actions when a file’s deletability or status changes.
  • Improved robustness of command cleanup to avoid controller crashes.
  • More reliable remote deletion flow with clearer start/success logging.

• Tests

  • Added unit tests for file property change detection and action state handling.

• Documentation / Deployment

  • Added UMASK configuration docs and examples; runtime now respects UMASK in the container.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8faca1f and 6fade95.

📒 Files selected for processing (4)

• src/docker/build/docker-image/Dockerfile
• src/docker/build/docker-image/entrypoint.sh
• website/docs/configuration.md
• website/docs/installation.md

---

📝 Walkthrough

Walkthrough

Adds unit tests for FileComponent.onChanges, tightens frontend logic to clear action when status or deletability changes, makes controller cleanup tolerant to propagate_exception failures, and simplifies remote-delete SSH handling and logging.

Changes

Cohort / File(s)	Summary
Angular: component & tests src/angular/src/app/pages/files/file.component.ts, src/angular/src/app/pages/files/file.component.spec.ts	Adds ngOnChanges unit tests and refactors on-change logic to clear activeAction when file status or specific deletability (remote/local) changes in ways that invalidate the current action.
Python: controller cleanup src/python/controller/controller.py	Wraps propagate_exception() call in try/except and logs a warning on failure to avoid crashing the controller during command cleanup.
Python: remote deletion process src/python/controller/delete/delete_process.py	Removes SshcpError handling and outer try/except, always computes escaped remote path, runs rm -rf via SSH, logs command output at debug and operation start/success at info; import adjusted to drop SshcpError.
Docker runtime & entrypoint src/docker/build/docker-image/Dockerfile, src/docker/build/docker-image/entrypoint.sh	Introduces UMASK env var in image and applies it in entrypoint when set (calls umask "$UMASK" and echoes value).
Docs: configuration & installation website/docs/configuration.md, website/docs/installation.md	Documents new UMASK env var, adds examples/tips for Docker Compose/Run, and documents SSH-mounted .ssh volume guidance.

Sequence Diagram(s)

sequenceDiagram
    participant Client as File UI
    participant Frontend as FileComponent
    participant Controller as Backend Controller
    participant DeleteProc as DeleteProcess
    participant SSH as Remote SSH

    Client->>Frontend: user triggers delete remote
    Frontend->>Controller: send delete request
    Controller->>DeleteProc: start remote delete
    DeleteProc->>SSH: execute rm -rf <escaped-path>
    SSH-->>DeleteProc: command output / exit status
    DeleteProc-->>Controller: success / failure
    Controller->>Controller: cleanup commands
    Controller--xDeleteProc: propagate_exception() guarded (logged on exception)
    Controller-->>Frontend: respond with operation result

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I nudged the code where deletes would stall,
Now hops and checks and clears actions small.
SSH trims paths and logs the run,
Controller guards so crashes shun.
Tests applaud — a tidy, bouncing call. 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately reflects the main changes: fixing the delete remote directory hang issue and adding the UMASK environment variable feature.
Linked Issues check	✅ Passed	Changes comprehensively address issue #86 by clearing activeAction on deletability transitions and fixing error handling in deletion processes.
Out of Scope Changes check	✅ Passed	All changes are within scope: file component deletion logic, controller error handling, delete process improvements, and UMASK environment variable documentation/implementation.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/delete-remote-directory-hang

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/python/controller/delete/delete_process.py`:
- Around line 50-53: The branch that handles file paths starting with "~"
currently calls escape_remote_path_double which leaves the path wrapped in
double quotes and allows shell expansion; instead expand the tilde to the actual
home path (e.g., via os.path.expanduser or equivalent) and then call
escape_remote_path_single so the resolved path is single-quote escaped and
prevents shell expansion/injection; update the code path where
file_path.startswith("~") to first expand the user and then pass the result to
escape_remote_path_single (referencing file_path.startswith,
escape_remote_path_double, escape_remote_path_single).

---

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 862fd5c and 8faca1f.

📒 Files selected for processing (4)

• src/angular/src/app/pages/files/file.component.spec.ts
• src/angular/src/app/pages/files/file.component.ts
• src/python/controller/controller.py
• src/python/controller/delete/delete_process.py

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# First, locate and examine the escape helper functions
echo "=== Searching for escape_remote_path helpers ==="
find src/python -type f -name "*.py" | xargs grep -l "def escape_remote_path" 

# Then get the implementation details
echo ""
echo "=== Helper implementations and usage context ==="
rg -n -C5 'def escape_remote_path_double|def escape_remote_path_single' src/python/

# Check the delete_process.py context around lines 50-53
echo ""
echo "=== Context around delete_process.py lines 50-53 ==="
rg -n -C8 'startswith\("~"\)' src/python/controller/delete/delete_process.py

Repository: nitrobass24/seedsync

Length of output: 2382

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Get the complete escape_remote_path_double function
cat src/python/common/remote_path_utils.py

Repository: nitrobass24/seedsync

Length of output: 684

---

Harden ~ path escaping to avoid shell expansion/injection.

At line 51, the ~ branch uses escape_remote_path_double(), which converts ~ to $HOME and wraps in double quotes. However, double quotes do not escape shell-expanding characters ($, backticks, \, "), allowing crafted file names to execute arbitrary commands. For example, a path like ~/.ssh/$(malicious_command) would execute the embedded command.

Use single-quote escaping (which prevents all expansion) after resolving the home directory:

🔧 Safer approach

-        if file_path.startswith("~"):
-            escaped_path = escape_remote_path_double(file_path)
-        else:
-            escaped_path = escape_remote_path_single(file_path)
+        if file_path.startswith("~"):
+            remote_home = self.__ssh.shell('printf "%s" "$HOME"').decode("utf-8", errors="strict")
+            file_path = remote_home + file_path[1:]
+        escaped_path = escape_remote_path_single(file_path)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/python/controller/delete/delete_process.py` around lines 50 - 53, The
branch that handles file paths starting with "~" currently calls
escape_remote_path_double which leaves the path wrapped in double quotes and
allows shell expansion; instead expand the tilde to the actual home path (e.g.,
via os.path.expanduser or equivalent) and then call escape_remote_path_single so
the resolved path is single-quote escaped and prevents shell
expansion/injection; update the code path where file_path.startswith("~") to
first expand the user and then pass the result to escape_remote_path_single
(referencing file_path.startswith, escape_remote_path_double,
escape_remote_path_single).