Pass bfabric tus credentials via environment - #6
Draft
Caushi wants to merge 1 commit into
Draft
Conversation
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.
What this adds
Plumbing so a host using
tool="tus"can authenticate to B-Fabric. The launcher staystransfer-agnostic — it still only reads
versionfrom the XML — so this is purely about gettingcredentials and the right extra to BioBeamer.
The secret never travels as an argument
The existing
passwordgoes throughcmd.extend(["--password", ...])and thenlogger.info(f"Running BioBeamer: {' '.join(cmd)}")— i.e. written to the log in plaintext, andvisible in
/proc/*/cmdline. Rather than extend that pattern for an OAuth secret, the client secretis passed through the environment (
subprocess.run(..., env=...)), and only the non-secretsettings travel as arguments.
That also fixes the pre-existing leak:
redact_commandmasks the value after--password, so theexisting network password no longer lands in the log either.
redact_configdoes the same for theconfig dump, and
--debugprintsexport BFABRIC_CLIENT_SECRET=...instead of the value.Verified by asserting the secret is absent from the printed config and the logged command line, while
<set>still tells the operator it is configured.New launcher.ini keys
bfabric_base_urlbfabric_client_idbfabric_scopetus— B-Fabric's default scope does not grant itbfabric_client_secretBFABRIC_CLIENT_SECRETenv var; an already-set env var winsInstalling the tus extra only where it is needed
extras_for_hostreturns["tus"]when the host'stoolistus, souv pip install -egets.[tus]for those hosts and plain.for everyone else. bfabric is a large dependency tree thatrequires Python 3.11+, and the robocopy instrument PCs must keep installing.
Note the venv is only built when the entry point is missing, so a host opting into tus needs a new
versionvalue (which yields a freshBioBeamer-venv-<version>) for the extra to be installed. Thefeat-tus-uploadbranch name serves that purpose during testing.Also fixed in passing: the venv
PATHwas joined with a hardcoded:separator, wrong on Windows.Files
src/biobeamer_launcher/launcher.py— config keys,env=on the subprocess,redact_config/redact_command,extras_for_host, PATH separatorconfig/launcher.ini.example— documents the four keys and why the secret belongs in the environmenttests/conftest.py— the stub BioBeamer must accept the new non-secret flags