Fix and validate Compose quick start
The list-form environment syntax in the README passed quote characters through as part of the value, so LIBRARY_ID was read as '"<id>"' and no matching library was found. Switch to mapping syntax, where YAML strips the quotes. - Pin the example to a versioned image instead of implicit latest - Add a checked-in compose.example.yaml - Validate it with `docker compose config` in CI - Link the Docker Hub image from the README
This commit is contained in:
Maxim Shulga committed
1 parent
a7abdab1f7
commit
d500ddc7ab
4 files changed
+231
-7
No files matched your search
+197
@@ -0,0 +1,197 @@
|
||||
# Improvement Plan
|
||||
|
||||
This plan prioritizes user safety and a working release pipeline before public
|
||||
portfolio polish. The project already has useful adoption evidence: more than
|
||||
5,800 Docker Hub pulls, external issues, and external contributions.
|
||||
|
||||
Security-sensitive findings should be fixed and released before detailed
|
||||
exploit scenarios are discussed in public issues.
|
||||
|
||||
## P0 - Release blockers
|
||||
|
||||
### Restore reproducible image builds
|
||||
|
||||
- [ ] Replace the unavailable Seafile APT repository used in `Dockerfile`.
|
||||
- [ ] Install a supported Seafile CLI version from an official source.
|
||||
- [ ] Pin the version and verify its checksum or signature during the build.
|
||||
- [ ] Remove `apt-key`, which is deprecated and trusts keys globally.
|
||||
- [ ] Add a smoke check for `seaf-cli --version` to the image build.
|
||||
- [ ] Scan the resulting image and review every high or critical finding.
|
||||
|
||||
Acceptance criteria:
|
||||
|
||||
- A clean `docker build --no-cache .` succeeds in CI.
|
||||
- The installed Seafile version is explicit and current.
|
||||
- A missing or invalid artifact signature fails the build.
|
||||
|
||||
### Remove shell and path injection risks
|
||||
|
||||
- [ ] Pass subprocess arguments as arrays instead of joining values into shell
|
||||
command strings in `dsc/client.py` and `dsc/misc.py`.
|
||||
- [ ] Replace `su -c` with a mechanism such as `runuser --` that does not need a
|
||||
shell.
|
||||
- [ ] Treat server-provided library names as untrusted input.
|
||||
- [ ] Resolve every target directory and prove that it remains under
|
||||
`/dsc/seafile` before creating or writing it.
|
||||
- [ ] Define how duplicate, absolute, empty, and traversal-style library names
|
||||
are handled.
|
||||
|
||||
Acceptance criteria:
|
||||
|
||||
- Tests cover quotes, command substitutions, separators, newlines, absolute
|
||||
paths, parent traversal, Unicode, and colliding library names.
|
||||
- No configuration or server value can execute a command or escape the data
|
||||
directory.
|
||||
|
||||
### Support secrets safely
|
||||
|
||||
- [ ] Implement `PASSWORD_FILE` for Docker Compose and Swarm secrets.
|
||||
- [ ] Add token-based authentication and `TOKEN_FILE` if supported by the
|
||||
installed Seafile CLI.
|
||||
- [ ] Define precedence and reject conflicting secret sources.
|
||||
- [ ] Ensure secrets are absent from application logs.
|
||||
- [ ] Document any upstream limitation that exposes a secret in process
|
||||
arguments.
|
||||
|
||||
Acceptance criteria:
|
||||
|
||||
- A container can authenticate with a mounted file under `/run/secrets`.
|
||||
- Tests reject conflicting inputs and verify newline handling.
|
||||
- Logs never contain the password or token.
|
||||
|
||||
### Correct environment parsing and defaults
|
||||
|
||||
- [ ] Replace direct `os.getenv()` assignments in `start.py` with typed parsers.
|
||||
- [ ] Preserve the documented defaults for upload limit, download limit, and
|
||||
delete-confirm threshold when variables are absent.
|
||||
- [ ] Validate UID, GID, limits, threshold, and booleans before daemon startup.
|
||||
- [ ] Reject negative and malformed numeric values with useful messages.
|
||||
- [ ] Do not pass `None` as a Seafile configuration value.
|
||||
|
||||
Acceptance criteria:
|
||||
|
||||
- Table-driven tests cover unset, zero, valid, negative, and malformed values.
|
||||
- A default container receives exactly the defaults documented in the README.
|
||||
|
||||
### Make startup and shutdown deterministic
|
||||
|
||||
- [ ] Check every important subprocess return code.
|
||||
- [ ] Add timeouts to daemon startup, shutdown, and readiness polling.
|
||||
- [ ] Handle `SIGTERM` and `SIGINT` and stop the daemon in `finally`.
|
||||
- [ ] Return a nonzero container exit code on configuration or daemon failure.
|
||||
- [ ] Produce actionable errors instead of waiting forever.
|
||||
|
||||
Acceptance criteria:
|
||||
|
||||
- A daemon that never becomes ready causes a bounded failure.
|
||||
- `docker stop` performs a graceful shutdown within Docker's timeout.
|
||||
- Failure-injection tests cover command errors and interrupted startup.
|
||||
|
||||
### Add real tests
|
||||
|
||||
- [ ] Add unit tests for configuration parsing, command construction, library
|
||||
resolution, path validation, retries, and status parsing.
|
||||
- [ ] Add regression tests for externally reported password and deletion
|
||||
threshold issues.
|
||||
- [ ] Add a container smoke test for imports, installed CLI version, startup,
|
||||
and shutdown using controlled fakes.
|
||||
- [ ] Keep image construction as a separate CI job so packaging failures are
|
||||
distinguishable from application failures.
|
||||
|
||||
Acceptance criteria:
|
||||
|
||||
- CI fails on a deliberate parsing, quoting, or path-safety regression.
|
||||
- Application tests run without requiring a live production server.
|
||||
|
||||
### Repair and validate the quick start
|
||||
|
||||
- [x] Change the Compose example to mapping syntax so quote characters are not
|
||||
passed as part of `LIBRARY_ID`.
|
||||
- [x] Pin the example to a versioned image instead of an implicit latest tag.
|
||||
- [x] Add a checked-in `compose.example.yaml`.
|
||||
- [x] Validate it with `docker compose config` in CI.
|
||||
|
||||
## P1 - Reliability and supply chain
|
||||
|
||||
### Networking and protocol behavior
|
||||
|
||||
- [ ] Add explicit connect and read timeouts to all HTTP requests.
|
||||
- [ ] Define a bounded retry budget with backoff and jitter.
|
||||
- [ ] Handle authentication, rate limits, malformed responses, and retryable
|
||||
server errors separately.
|
||||
- [ ] Make TLS configuration consistent between Python requests and Seafile.
|
||||
- [ ] Support a custom CA bundle; keep certificate disabling as a last resort.
|
||||
- [ ] Prefer structured JSON output from `seaf-cli` over parsing display text.
|
||||
- [ ] Reject ambiguous library names and an empty requested library set.
|
||||
|
||||
### Runtime hardening
|
||||
|
||||
- [ ] Run the long-lived process as the configured non-root user after setup.
|
||||
- [ ] Reject UID or GID zero unless a documented use case requires it.
|
||||
- [ ] Add a health check for the daemon and RPC endpoint.
|
||||
- [ ] Handle temporary RPC failures and daemon restarts without corrupting
|
||||
state.
|
||||
- [ ] Document supported CPU architectures instead of implying multi-platform
|
||||
support.
|
||||
|
||||
### CI and release pipeline
|
||||
|
||||
- [ ] Replace `actions/checkout@v2` and `docker/build-push-action@v1`.
|
||||
- [ ] Set workflow permissions explicitly to `contents: read` by default.
|
||||
- [ ] Use a scoped Docker Hub access token.
|
||||
- [ ] Build a release image once and attach all version and `latest` tags to
|
||||
the same manifest digest.
|
||||
- [ ] Add OCI labels, an SBOM, provenance, and image signing.
|
||||
- [ ] Add dependency updates for Python, Docker, and GitHub Actions.
|
||||
- [ ] Run dependency and image scans on pull requests and on a schedule.
|
||||
|
||||
Acceptance criteria:
|
||||
|
||||
- Every tag for one release resolves to the same manifest digest.
|
||||
- Pull requests cannot access publication credentials.
|
||||
- The current dependency update pull request passes all checks.
|
||||
|
||||
## P2 - Maintainability and presentation
|
||||
|
||||
### Simplify the implementation
|
||||
|
||||
- [ ] Separate configuration, subprocess execution, library resolution, and
|
||||
status monitoring so each can be tested independently.
|
||||
- [ ] Remove unused imports and unnecessary dependencies.
|
||||
- [ ] Add type annotations and static checks at external-input boundaries.
|
||||
- [ ] Make logging configurable and avoid forced dependency debug logs.
|
||||
- [ ] Add `.dockerignore` and reduce unnecessary image packages.
|
||||
|
||||
### Establish a visible maintenance process
|
||||
|
||||
- [ ] Publish GitHub Releases for future tags with compatibility and rollback
|
||||
notes.
|
||||
- [ ] Add `CHANGELOG.md`, `SECURITY.md`, and concise contribution guidance.
|
||||
- [ ] Add issue templates that request image, client, and server versions plus
|
||||
sanitized logs.
|
||||
- [ ] Triage stale issues and document the status of Docker secret support.
|
||||
- [ ] Enable branch protection and require passing CI before merge.
|
||||
|
||||
### Improve the public project page
|
||||
|
||||
- [ ] Link directly to the Docker Hub image near the top of `README.md`.
|
||||
- [ ] Add CI, release, Docker pulls, and license badges after the checks are
|
||||
trustworthy.
|
||||
- [ ] Add a short architecture and process-lifecycle section.
|
||||
- [ ] Add a tested compatibility matrix.
|
||||
- [ ] Explain deletion behavior, volume ownership, state loss, backup
|
||||
expectations, and recovery.
|
||||
- [ ] Fix example inconsistencies and typographical errors.
|
||||
|
||||
## Release gate
|
||||
|
||||
Do not publish the next image until all of the following are true:
|
||||
|
||||
- [ ] A clean, reproducible build uses a supported Seafile client.
|
||||
- [ ] External values cannot reach a shell or escape the data directory.
|
||||
- [ ] Secret-file or token authentication is covered by tests.
|
||||
- [ ] Startup, retries, and shutdown are bounded and observable.
|
||||
- [ ] Unit and container smoke tests pass.
|
||||
- [ ] The steady-state process is non-root and has a useful health check.
|
||||
- [ ] Release tags point to one scanned and signed image digest.
|
||||
- [ ] The README quick start succeeds from a clean environment.
|
||||
Reference in new issue
Block a user