* Add a health check for the daemon and RPC endpoint
SeafileClient.is_healthy() checks seaf-cli status and makes a real RPC
call (get_repo_list) against the daemon's Unix socket - neither needs
the Seafile server, so it works without a token or password.
healthcheck.py exposes it as a plain script for Docker's HEALTHCHECK.
Each check is a fresh process spawned by the Docker daemon as root,
unlike the long-lived start.py process, so it drops to the seafile
user with runuser before touching the RPC socket - the one place that
mechanism still belongs after the earlier in-process privilege drop.
Verified against the real image: healthcheck.py exits 0 once the real
daemon is up (through the actual main() entrypoint, not a test
fixture) and nonzero when no daemon has been started.
* Assert the return value in the daemon-down health check test
Copilot review: the test checked the RPC call was skipped but not
what is_healthy() actually returned in that case.
---------
Co-authored-by: Maxim Shulga <maxim.shulga@alludo.com>
* Reject ambiguous library names and empty library-set entries
get_library_id() silently picked the first library matching a
requested name, so two libraries sharing a name would sync whichever
happened to come first in the server's response - not necessarily the
one the operator meant. It now raises ConfigError when a name matches
more than one library.
resolve_libraries() rejects an empty entry produced by an empty
LIBRARY_ID, 'a::b', or a leading/trailing ':' the same way, instead of
silently logging 'library not found' and moving on.
* Clarify get_library_id's ambiguity docstring and error message
Copilot review: the input can be a name or an ID, and the error said
'Library name' even for an ID collision; use neutral wording. Also
made the docstring's second sentence a complete one.
---------
Co-authored-by: Maxim Shulga <maxim.shulga@alludo.com>
* Bound HTTP request timeouts and retry budget
With no request timeout, Retry(total=30, backoff_factor=2,
backoff_max=60) let a single unreachable server hang the client for
around 26 minutes: backoff sleeps alone sum to that, and each of the
30 connection attempts could itself stall indefinitely without a
timeout. Measured a black-holed connect attempt taking 75s on this
machine with no timeout set, confirming the mechanism.
- Add HTTP_TIMEOUT (connect, read) and a tighter retry budget
(HTTP_RETRY_TOTAL/BACKOFF_FACTOR/BACKOFF_MAX) in dsc/const.py,
bounding the worst case to a few minutes.
- Pass timeout=HTTP_TIMEOUT on both SeafileClient HTTP calls (token
fetch, remote library listing).
- Add NetworkError(DscError) so a network failure is caught by
start.py's existing top-level DscError handling instead of
producing a bare traceback.
- Add tests/test_http.py covering the retry policy values, timeout
propagation, and error wrapping.
- Check off the corresponding IMPROVEMENTS.md items.
* Include URL and status code in NetworkError messages
Copilot review on PR #28 flagged that the non-200 path for remote
library listing raised NetworkError with only the raw response body,
losing which request failed and its status code. The token-fetch
error had the same gap. Both now include the URL and HTTP status.
---------
Co-authored-by: Maxim Shulga <maxim.shulga@alludo.com>
set_defaults() assigned os.getenv() results directly, which overwrote the
argparse defaults with None whenever a variable was unset. A default container
therefore ran "seaf-cli config -k delete_confirm_threshold -v None", and the
same for upload_limit and download_limit: none of the three documented
defaults were ever applied.
SEAFILE_UID and SEAFILE_GID had a second problem. argparse type=int applies
only to command line flags, never to set_defaults, so both stayed strings.
The check in setup_uid compares them against pwd integers, and a string is
never equal to one, so usermod ran on every start.
dsc/config.py reads each variable with its documented default and rejects a
malformed or negative value at startup, naming the variable, instead of
passing it on. An empty variable counts as unset.
Booleans are now rendered as true and false rather than Python's True and
False. The daemon reads disable_verify_certificate with its boolean accessor,
which does not recognise the capitalised form. I could not run the daemon to
confirm the previous value's effect, since the AppImage is x86_64 only.
seaf-cli is no longer given a None value for any option.
json.loads succeeding does not mean the result is a list of objects with an
id. A dict, an entry without an id, or a scalar raised TypeError or KeyError,
which escaped unhandled and defeated the point of the change. Both cases now
raise DaemonError, and the original exception is chained so the cause is
visible.
get_local_libraries unpacked "line.rsplit(maxsplit=3)" into three names, but
maxsplit=3 yields up to four fields. seaf-cli prints "name id path" separated
by whitespace, so any library whose name contains a space produced a fourth
field and raised ValueError, which nothing caught.
The failure only appeared after the first run: the list is empty until a
library has been synced, so a container would work when it was set up and then
crash on every restart.
seaf-cli list --json returns name, id and path as structured fields, which
display output cannot represent unambiguously for names or paths containing
whitespace. Its exit code is now checked and unreadable output is reported as
a daemon error instead of a traceback.
The image smoke check asserts that the installed seaf-cli offers --json and
--token, so a future version bump that drops either fails the build rather
than the container.
The password could only be given through the PASSWORD variable, so Docker
Compose and Swarm secrets were unusable and the password sat in the container
environment.
PASSWORD_FILE and TOKEN_FILE read a secret from a mounted file. Only trailing
newlines are stripped, since secret files are usually written with one and a
password that silently carries it just fails to authenticate; everything else,
including surrounding spaces, belongs to the secret. Setting both a variable
and a file for one secret is rejected rather than resolved by precedence,
because the two are meant to be the same value and quietly picking one hides
the mistake until authentication fails.
The installed seaf-cli supports token authentication and ignores the password
when a token is given, so sync now passes -T instead of -p. The account
password no longer appears in any command line. The token does, because
seaf-cli accepts credentials only as arguments, but a token can be revoked on
the server. This is documented in the README.
Both secrets are masked before a command is logged.
Tests cover newline handling, empty and missing files, conflicting sources,
and that neither secret reaches the logs. Requests are now blocked in tests:
the client retries with backoff, so a test that reached the network appeared
to hang instead of failing.
start_daemon and stop_daemon polled "while True" with no bound, and no
seaf-cli exit code was checked. A daemon that failed to start left the
container spinning forever with nothing pointing at the cause, and there was
no signal handling, so "docker stop" waited the full 10 second grace period
and then SIGKILLed the process with the daemon still running.
Readiness polling is now bounded by DAEMON_START_TIMEOUT and
DAEMON_STOP_TIMEOUT. The stop timeout is 8 seconds so a graceful shutdown
fits inside the docker stop grace period, and a test asserts that relation.
SIGTERM and SIGINT raise GracefulShutdown, and the lifecycle moved into
start.run(), which always stops the daemon on the way out and returns the
process exit code: 0 on a clean finish or a stop signal, 1 on a daemon or
configuration failure.
seaf-cli exit codes are checked. A failed sync is reported per library and
does not stop the remaining libraries, since one bad library should not take
the container down.
Verified in the built image: "docker stop" now returns in 0s with exit code 0
and the daemon stopped, against 10s and exit 137 before. A daemon that cannot
start exits 1 with the failing command and its exit code in the log.
Also bumps actions/setup-python to v7, which clears the last Node 20
deprecation warning; v7 only drops the unused pip-install input.
Commands were joined into a string and passed to "su -c", so a login shell
parsed every interpolated value. A library name is chosen on the server, and
"sync -d /dsc/seafile/<name>" put it straight into that string: a library
named 'Docs; touch /tmp/pwned; #' ran as an arbitrary command. Confirmed
against the built image before and after this change.
Every command is now an argument list executed by "runuser --", which execs
directly with no shell. runuser -u sets the same HOME and a PATH that still
finds the seaf-cli wrapper, so the daemon environment is unchanged.
The password no longer needs its surrounding quotes, which also fixes
passwords containing quotes, dollar signs, and backticks.
Library names are untrusted, so dsc/paths.py reduces each one to a single
path component and proves the result stays under the libraries directory
with symlinks resolved. Empty, ".", "..", and names containing a separator
are skipped with a warning; names that would collide are suffixed with their
library ID rather than sharing a directory.
Tests cover quotes, command substitutions, separators, newlines, absolute
paths, parent traversal, Unicode, and colliding names. They run in a separate
CI job so application failures stay distinguishable from packaging failures.
Add a .dockerignore that whitelists only requirements.txt, start.py and
dsc/. The build context previously included .git, .idea, __pycache__ and
any local .env, which slowed builds and put a secrets file within reach
of any future COPY of the whole tree.
Remove the unused urlparse import from dsc/client.py and the redundant
import os.path from start.py; importing os already binds os.path.
Record both in IMPROVEMENTS.md, including the part still outstanding:
binutils and squashfs-tools remain in the runtime image because the
AppImage is unpacked in the same stage.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Implement automatic retry with exponential backoff (up to 30 attempts)
for 502/503/504 errors. Prevents container crash-loop during server
outages, reducing kernel log spam from repeated restarts.
Add dynamic column width alignment for library names up to 30 chars.
Prevents excessive whitespace when library names vary significantly
in length while maintaining readability for typical names.
Please add some escaping in password env, I have password similar "xxxx&zzzz", and "&" broke syncing:
```
2023-10-16 12:50:04,655 Listing local libraries: seaf-cli list
2023-10-16 12:50:05,108 Syncing library btsync: seaf-cli sync -l 3XX2-XXX-2eXXea -s https://xxxxxxxx.xx -d /dsc/seafile/btsync -u xxx@xx.xx -p ********
-bash: line 1: zzzz: command not found
Traceback (most recent call last):
File "/usr/bin/seaf-cli", line 1023, in <module>
main()
File "/usr/bin/seaf-cli", line 1019, in main
args.func(args)
File "/usr/bin/seaf-cli", line 675, in seaf_sync
token = get_token(url, username, password, tfa, conf_dir)
```
Maksim ShulgaandMaxim Shulga
snegov
subvillion