deploy: ship the vault dependency, keep it opt-in - #106
Merged
Conversation
The encrypted credential vault (saved SSH credentials) is gated on WEBSH_VAULT_ENABLE=1 plus the optional cryptography dependency. Neither was supplied by the shipped deploy artifacts, so operators using the bundled systemd unit or Docker image got no saved-credential UI even though the feature is complete — and cryptography was never declared as a dependency, so even setting the flag alone did nothing on a clean image. Make it work out of the box for the bundled deploy paths while leaving a bare `python3 server.py` opt-in: - requirements.txt: declare cryptography (>=3.4.8, the documented floor). - Dockerfile: install requirements; set WEBSH_VAULT_ENABLE=1; point WEBSH_CREDS_PATH at a websh-owned /data volume (the default cwd path is not writable under USER websh) so saved credentials persist. - websh.service: set WEBSH_VAULT_ENABLE=1 and WEBSH_CREDS_PATH, and add StateDirectory=websh so /var/lib/websh is writable despite ProtectSystem=strict / ProtectHome=read-only. - docs: reflect the new default in configuration.md, encryption.md and the systemd install steps. The in-code default stays off, so dev / library-style use is unchanged.
Declaring `cryptography` nowhere meant the credential vault was impossible to enable in the bundled Docker image and systemd unit even with WEBSH_VAULT_ENABLE=1 set — the HAS_CRYPTOGRAPHY gate kept it off. Ship the dependency (requirements.txt + install steps) and pre-provision a writable creds path (Docker /data volume, systemd StateDirectory) so enabling the vault is a one-line opt-in, but leave WEBSH_VAULT_ENABLE off by default: turning persistent server-side credential storage on is an operator's decision, not a packaging side effect. Also fix the manual-install dependency step (PEP 668 makes a bare `pip install` fail on Debian/Ubuntu's externally-managed Python) and document Docker volume persistence.
Owner
|
Pushed a follow-up commit ( The core fix is right and worth shipping: What changed in the follow-up:
Docs ( |
# Conflicts: # Dockerfile # docs/configuration.md # docs/deployment.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The encrypted credential vault (the "Save this connection" UI) is gated on
WEBSH_VAULT_ENABLE=1and the optionalcryptographydependency (server.py:vault_enabled = HAS_CRYPTOGRAPHY and WEBSH_VAULT_ENABLE and not _vault_disabled). The shipped deploy artifacts provided neither:websh.serviceand theDockerfiledid not setWEBSH_VAULT_ENABLE.cryptographywas not declared as a dependency anywhere (norequirements.txt/pip install), so even setting the flag did nothing on a cleanpython:3-slimimage.Net effect: an operator deploying the bundled unit or Docker image got no saved-credential UI even though the feature is fully implemented.
Change
Make the vault work out of the box for the bundled deploy paths, while a bare
python3 server.pystays opt-in (the in-code default is unchanged):requirements.txt(new): declarecryptography>=3.4.8(the documented floor fromencryption.md).Dockerfile:pip install -r requirements.txt; setWEBSH_VAULT_ENABLE=1; pointWEBSH_CREDS_PATHat awebsh-owned/datavolume — the default cwd path (/app) is not writable underUSER websh, and the volume lets saved credentials survive container replacement.websh.service: setWEBSH_VAULT_ENABLE=1+WEBSH_CREDS_PATH=/var/lib/websh/websh.creds.json, and addStateDirectory=webshso that path is writable despiteProtectSystem=strict/ProtectHome=read-only.configuration.md,encryption.md(status + quick-start) and the systemd install steps to reflect the new default.Failure stays graceful: a host without
cryptographystill runs, with the saved-credential UI hidden.Verification
python3 test_server.py→ 472 OK (skipped=1).ProtectSystem=strict/ProtectHome=read-onlyunit): withStateDirectory=websh+WEBSH_CREDS_PATH,/api/configreportsvault_enabled: true, a save returns200and writes/var/lib/websh/websh.creds.jsonat mode0600, andsave_deletereturns204and reaps the entry. Without the writable path the save fails (cwd/is read-only), which is exactly what this PR fixes.cryptographyinstalls from a manylinux wheel onpython:3-slim;/dataiswebsh-owned and writable).