Add remote CIFS/SMB storage sync with session-only credentials - #12
Merged
Merged
Conversation
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>
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.
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
//ds.asuch.cas.cz/ueb/lhrbut editable.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 --
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 undersudo, so both directions matter:passwordSetboolean. Tested against every GET endpoint including/openapi.json.ps aux- passed via a0600credentials file in a tmpfs runtime dir, unlinked in afinallycovering success, non-zero exit, timeout and unexpected exceptions.shell=Trueanywhere; 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.nosuid,nodev,noexecand the unprivileged uid/gid apply even if something unexpected slipped through validation. This is what keeps the grant from being an escalation path.visudo -cbefore installation and is not installed if validation fails (a malformed file in/etc/sudoers.dcan lock the account out of sudo entirely). Scoped to exactly two commands, never a blanketALL. Removed byuninstall.sh.Robustness
Notes on two judgment calls
remote_sync.json, notDeviceSettings- a share path is not a property of how an image was taken, and it keeps credentials out of per-experiment config snapshots.mountedis a cached flag, not a livestat(). 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 typecheckandnpm run buildclean.