Skip to content

Add remote CIFS/SMB storage sync with session-only credentials - #12

Merged
lamewarden merged 1 commit into
mainfrom
feature/remote-sync
Aug 21, 2026
Merged

lamewarden merged 1 commit into
mainfrom
feature/remote-sync

Conversation

@lamewarden

Copy link
Copy Markdown
Owner

Adds a Settings -> General remote-sync card that mounts an institutional CIFS/SMB share and copies experiment images to it as they are captured. Replaces the legacy hardcoded-credential approach.

Behaviour

  • On/off toggle; server path pre-filled with //ds.asuch.cas.cz/ueb/lhr but editable.
  • Username + masked password. When a password is set the UI never reveals its length.
  • Check connection stays disabled until both fields are filled; reports the real mount error on failure.
  • Sync entire folder now for a one-shot backfill of the user's existing experiments.
  • Remote layout: mounted share -> subfolder named after the researcher -> experiment folders (mirrors the legacy behaviour).
  • Sync stops when toggled off, or when the researcher name changes.

Credentials are session-only, by design

The password is held in memory and never written to disk. It is lost on restart, deliberately. The UI says so in two places: always-visible helper text under the password field, and a confirmation toast on save --

For better security, credentials are not stored on disk and must be entered again after every system restart.

When sync is on but the password is gone after a restart, the card turns orange and reads "Inactive - credentials needed after restart" rather than appearing on while silently doing nothing.

Security design

This box has no auth and binds 0.0.0.0, and the mount runs under sudo, so both directions matter:

  • Password cannot leak through the API structurally - there is no password field on any response model. The request model is separate and write-only; responses expose only a passwordSet boolean. Tested against every GET endpoint including /openapi.json.
  • Never in ps aux - passed via a 0600 credentials file in a tmpfs runtime dir, unlinked in a finally covering success, non-zero exit, timeout and unexpected exceptions.
  • No shell=True anywhere; fixed argv only. The server string is validated against a strict allowlist (plus length cap and .. rejection) since it flows into a sudo command - 27 hostile inputs are tested as rejected.
  • Hardening options are literal and last in the sudoers pattern. Mount options are last-one-wins, so nosuid,nodev,noexec and the unprivileged uid/gid apply even if something unexpected slipped through validation. This is what keeps the grant from being an escalation path.
  • sudoers is validated with visudo -c before installation and is not installed if validation fails (a malformed file in /etc/sudoers.d can lock the account out of sudo entirely). Scoped to exactly two commands, never a blanket ALL. Removed by uninstall.sh.

Robustness

  • Copies run off the capture path on worker threads with timeouts - a hung or dead share can never stall the experiment schedule.
  • A sync failure never aborts a run: it is logged, marked pending, and retried on the next capture. The local experiment is the source of truth; the remote copy is best-effort.
  • Degrades gracefully in simulation mode with no CIFS server present.

Notes on two judgment calls

  1. Remote-sync config lives in its own remote_sync.json, not DeviceSettings - a share path is not a property of how an image was taken, and it keeps credentials out of per-experiment config snapshots.
  2. mounted is a cached flag, not a live stat(). Statting a hung CIFS mount blocks uninterruptibly and the panel polls every 5s, so a live stat would let a dead share freeze the single-process API.

156 backend tests pass (77 new); npm run typecheck and npm run build clean.

Lets a researcher mount an institutional SMB share from Settings -> General
and have experiment images copied there automatically as they are captured,
replacing the legacy hardcoded `sudo mount -t cifs ... -o user=,pass=` step.

Remote layout mirrors the legacy convention: mounted share -> subfolder named
after the researcher (created if missing) -> one folder per experiment.

Security (this repo just had a credential leak cleaned up):
- The password is session-only. It lives in memory on RemoteSyncService and is
  written nowhere -- not settings.json, not remote_sync.json, not logs. Only
  the non-secret half (server/username/enabled/researcher) persists.
- The password is strictly write-only over the API: accepted on PUT, and
  absent from every response model. RemoteSyncStatus has no field for it, only
  `passwordSet`. The box has no auth and binds 0.0.0.0, so this matters.
- It never reaches an argv (`ps aux` is world-readable): mount gets it via
  `-o credentials=<file>`, created 0600 with tempfile.mkstemp in the service's
  private /run/rapidboxes-cifs directory and unlinked in a `finally` the
  instant mount returns, on success and every failure path alike.
- No shell=True anywhere; fixed argument lists only, plus a strict allowlist on
  the //host/share string so a leading `-` or a comma cannot smuggle mount
  options into a sudo command. A test parses the AST to enforce the former.
- install.sh writes /etc/sudoers.d/rapidboxes (0440), validated with
  `visudo -c` before installation, scoped to exactly one mount point and one
  option string -- never a blanket ALL. uninstall.sh removes it.

Robustness:
- Copies run on a background queue, never on the capture path. A slow, hung or
  dead share cannot delay the deadline scheduler or fail an experiment; failed
  items are counted pending and retried on the next capture.
- `mounted` is a cached flag rather than a fresh stat(), because statting a
  hung CIFS mount would block the single-process API on every settings poll.
- Simulation mode emulates the share with a local directory, so the stack still
  runs on a laptop with no CIFS server.

UX of the session-only choice, which is the main risk of it:
- Static helper text next to the credential fields sets the expectation while
  the password is being typed, and an informational toast confirms it the
  moment credentials are accepted.
- After a restart clears the password, sync does not silently do nothing: the
  card turns orange and reads "Inactive -- credentials needed after restart".

The backend learns the active researcher from the `username` already carried by
every experiment config (client-side localStorage state); sync switches itself
off if that name changes, per spec.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@lamewarden
lamewarden merged commit 1e0cf31 into main Aug 21, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant