Repository navigation
feat: device pairing, and file transfer over gRPC instead of SFTP - #195
Merged
Merged
Conversation
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds AuthInterceptor to synapse.client.auth implementing all four gRPC client-interceptor shapes. It checks the method name against OPEN_METHODS first, so open RPCs (Info, GetLogs, TailLogs, RequestAuth) never trigger resolution. If no tokens exist locally, resolution is skipped entirely. The first gated call otherwise resolves the device serial via a single Info call and caches the token for the channel's lifetime. Device.__init__ now wraps its channel with auth.intercept(), keeping construction free of network I/O since resolution stays lazy. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Truncate-in-place rewrite could leave the file empty or torn if the process died mid-write (OOM, power loss, ENOSPC), losing every paired device's token, and gave concurrent writers no protection against interleaving bytes into a file that is neither version. Write to a temp file in the same directory (so os.replace stays within one filesystem), fsync before replacing (so the swap-in is never durable ahead of its contents), and clean up the temp file on any failure before the replace. The temp file is 0600 from creation, same as the final file. Also strengthens test_renamed_device_still_matches_by_serial, which previously never renamed anything, and adds tests for the no-leftover- temp-file and original-file-survives-a-failed-write properties. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds the human-facing side of the pairing handshake: `pair` calls the open Info RPC for serial/name, opens the RequestAuth stream, prints the six-digit code for the user to compare against the device touchscreen, waits for the grant/deny, and saves the token keyed on serial. `unpair` forgets the local token, falling back to matching by stored name when the device can't be reached, and refusing to guess when that match is ambiguous. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
setup_device_uri runs ahead of every subcommand and either aborts before unpair() ever runs (a name that fails discovery) or rewrites --uri to an IP (so comparing stored names against it can never match) -- so the "unreachable device, clean up locally" fallback could never fire through the real CLI. Work with the existing plumbing instead of changing shared discovery code: `unpair <serial-or-name>` matches ~/.scifi-env directly (args.uri stays None, so setup_device_uri leaves it alone), while `unpair --uri <device>` keeps contacting the device via Info for the authoritative serial when it's reachable. The positional wins if both are given, and neither given prints a hint plus what's stored instead of guessing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The interrupt during the approval wait was already handled; this is the earlier 10s window, before any code is on screen, which escaped as a raw traceback. Nothing has been asked of the device yet, so there is nothing to withdraw and nothing to save.
Matches the server: the standard bearer-token header (RFC 6750) instead of a bespoke x- key, so proxies and tooling that already understand bearer tokens keep working.
Transfers go through the SynapseDevice service, so file access is governed by the same pairing token as everything else and the separate SFTP password is no longer needed. Uploads are chunked at 1 MiB with the path sent in the first message and no data, so the server can resolve and open the target before any bytes arrive. Downloads resume from whatever is already on disk by default -- a multi-gigabyte recording that dies at 90% should not start over. This adds the client; synapsectl file still uses SFTP until it is rewired. deploy_model's SFTP use is untouched.
file ls/get/rm now go through the SynapseDevice service, so access is governed by the pairing token. --username, --env-file and --forget-password are gone with the SFTP password they existed to manage; they still parse but warn and do nothing, so a script written against the old version fails on a pointless flag rather than an unknown argument. Adds file put, which SFTP had but the CLI never exposed. get resumes a partial download by default (--no-resume to start over), which matters for the multi-gigabyte recordings this is mostly used for. rm asks before a recursive delete unless -y is passed; there is no undo on the device. The SFTP connection and password helpers move to synapse/client/sftp.py, beside the SFTP client, because deploy_model is the last consumer and cli/files.py no longer speaks SFTP at all. When deploy_model moves to WriteFile, all of it goes away. Paths are relative to the data root and cannot escape it, so 'file ls /' shows the data root. SFTP served the whole filesystem to any authenticated user. One capability is lost on purpose: the Permissions column. gRPC does not report st_mode, and inventing one would be a lie.
deploy-model was the last SFTP consumer. Its --username, --env-file and --forget-password are retired like synapsectl file's: still parseable so an existing script gets an explanation, not an unknown-argument error. _ensure_model_dir is deleted rather than ported. WriteFile creates parent directories for whatever path a chunk stream names, so models/ appears when the model lands and there is nothing for a mkdir to do. The stat call it paired with was only ever the 'already exists, overwrite?' prompt, never a transport requirement, so it becomes a ListFiles. DEVICE_MODEL_DIR drops its leading slash. It was a path inside the scifi-sftp chroot, which is the same root the file RPCs serve, so the effective location on disk is unchanged. file put gains --recursive for directories, symmetric with file get -r. Also corrects the same wrong SFTP claim as headstage d3e1269d: scifi-sftp was already chrooted to /opt/scifi/data, so reach is unchanged.
Uploading to an existing directory transferred the whole file and then failed at the final rename, because a file cannot replace a directory: 194 MB were sent before the error appeared. A directory destination now appends the local basename, the same way get -o does. Detected by listing the parent and finding the entry -- listing the target itself cannot distinguish an empty directory from a missing path.
unpair --uri now calls RevokeAuthClient with the SHA-256 of the stored token before forgetting it. A revoke that fails for a transport reason keeps the token so the user can retry; one the device has nothing to revoke for (already gone, or older firmware) forgets it anyway. Drops the stale 'not yet supported' note.
…client RPCs The pin was still at cacec78, from before any of this feature's API work, so a fresh clone would have generated protos with no RequestAuth, file RPCs or RevokeAuthClient.
The device now revokes whichever token the call carries, so the client no longer computes a token id. Tracks synapse-api with that RPC.
…file get treated any same-named local file as an interrupted download: a complete copy produced an empty transfer reported as Saved, and a different, smaller file got the remote file's tail appended to it. Downloads now land in <name>.partial and are renamed into place when complete, so only a .partial is ever resumed. get and put both ask before replacing an existing file (default no), with -y/--yes to overwrite without asking. With no terminal to ask on, the file is skipped.
An interrupted or failed get leaves <name>.partial so the same command can resume it; it now says so, with the size kept, instead of leaving an unexplained file behind.
The data directory holds markers such as .scifi-checked beside every recording. ls now leaves out names starting with '.', and anything inside such a directory, unless -a/--all is given. Listing a hidden directory by name still shows its contents.
The .partial is opened before the first chunk so empty files still get created, but the device's errors only arrive once reading starts, so a missing file left an empty .partial that was then reported as kept. An empty partial is now removed on failure. Single-file get also checks the path first, so a missing file or a directory gets one plain line, and a failed get exits non-zero.
A .partial held raw bytes and nothing else, so resuming after the device's file was replaced by another of the same size or larger spliced the two together. Each partial now has a <name>.partial.json recording the device file's size and modified time; a download continues only if they still match and starts over otherwise. Same format nexus-desktop writes.
Tracks synapse-api 3efc28a, which names byte counts with a _bytes suffix (size_bytes, start_offset_bytes, file_size_bytes).
The pairing, self-revoke and file RPC changes, as squash-merged to main.
Devices on older firmware answer the gRPC file RPCs with UNIMPLEMENTED. file ls/get/put/rm and deploy-model now detect that, tell the user to update the device firmware, and use the previous SFTP path with the device's file-transfer password (--username/--env-file/--forget-password are live again). The SFTP path asks before overwriting (get/put) and before a recursive delete, like the gRPC path. Also: fix setup_connection NameErrors in synapse/client/sftp.py, pin onnx==1.16.2 (older onnx has no cp312 wheels; matches the converter Dockerfile), and name the model-convert extra in the pt->onnx error.
calvinleng-science
changed the title
The SFTP helpers' comment still said deploy-model was their last user and that they would go when it moved to WriteFile; it has, and they remain as the fallback for firmware without the file RPCs, used by both `synapsectl file` and `deploy-model`. The files module docstring said the SFTP password and --username/--env-file were gone; they are the fallback's.
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
1 participant
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 device pairing to
synapsectland the Python client, and movessynapsectl fileanddeploy-modelfrom SFTP to the gRPC file RPCs. Companion to sciencecorp/synapse-api#85 (merged), sciencecorp/synapse-typescript#56 (released as 2.5.0), and the SciFi firmware that serves these RPCs.Pairing
Newer SciFi firmware refuses every call except
InfoandRequestAuthuntil the caller is paired, and a person at the device approves each pairing.synapsectl pairasks the device for access. The device's screen and the terminal both show the same six-digit code; the person at the device compares them and taps Allow. The token is saved in~/.scifi-env(mode 0600, written atomically), keyed on the device's serial.authorization: Bearer <token>. Open calls never trigger the lookup.synapsectl unpair --uri <device>revokes this computer's access on the device (RevokeAuth), then forgets the token.unpair <serial-or-name>only forgets the local token, for a device that is gone.File transfer over gRPC
SFTP needed its own password, so a device carried two credentials. The file RPCs are governed by the same pairing token as everything else, so the SFTP password goes away on firmware that serves them; the reachable tree is the same data directory the SFTP account was confined to.
synapsectl file ls | get | put | rm, now over gRPC.putstreams in 1 MiB chunks (a single message could not exceed gRPC's 4 MB limit);getstreams too.<name>.partialand are renamed into place only when complete, so a cancelled download never leaves a half-written file under the real name or touches an existing copy.getresumes a.partial, but only if the device's file is unchanged since (its size and modified time are recorded in<name>.partial.json); otherwise it starts over rather than splicing two files together. When interrupted it says where the partial is.getand on the device onput(default no,-yto skip the prompt).getof a missing file or a directory says so plainly and exits non-zero.lshides dotfiles (the device keeps.scifi-checkedmarkers beside recordings) unless-a.UNIMPLEMENTED,file ls | get | put | rmanddeploy-modelsay the device firmware predates pairing and should be updated, then use the previous SFTP path with the device's file-transfer password.--username,--env-fileand--forget-passwordare live again for that path. The fallback asks before overwriting and beforerm -r, like the gRPC path. On that firmware the SFTP root is not writable, so a top-level upload says to use a folder such asdisk_writer/ormodels/.deploy-modeluploads over gRPC too. The device creates parent directories itself, so the separate mkdir/stat steps are gone.Version → 2.8.0. Tracks synapse-api
main(f9fc77f).Testing
pytest: 200 tests pass, including the pairing CLI paths, chunked upload framing, resume, the version check, overwrite prompts, and hidden-file filtering.unpair --urirevoking on the device, an 800 MB upload byte-identical with flat server memory, a Ctrl-C'd download resumed byte-identical to a fresh one, and a download restarted (not spliced) after the device's file was replaced at the same size.