From 57bbc6d517d3803a89eeb7e2d38851306eaeeae3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Louis-F=C3=A9lix=20Nothias?= Date: Fri, 21 Aug 2026 10:48:17 +0200 Subject: [PATCH 1/5] chore: a changelog, and the ordinary example profile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nothing here was ever tagged, so anyone who cloned since 2026-07-29 holds a copy with defects that are mostly silent — a watch that reports running jobs as finished, a precedence rule that discards every documented override, a submit that fails on any current OpenSSH. The changelog leads with what CHANGED rather than what was added, because that is the part that can surprise someone upgrading: sbatch arguments are quoted now, a remote script path is literal, and watch has a third exit status. The examples directory had only the hardest case, a cluster behind a full-tunnel VPN that also enforces TOTP. Most people start with neither, and README solicits worked profiles from contributors — so the simple shape should be there to copy. --- CHANGELOG.md | 111 +++++++++++++++++++++++++++++++++++ README.md | 10 +++- examples/keyonly-direct.conf | 37 ++++++++++++ 3 files changed, 156 insertions(+), 2 deletions(-) create mode 100644 CHANGELOG.md create mode 100644 examples/keyonly-direct.conf diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..58e7ee0 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,111 @@ +# Changelog + +## 0.1.0 — 2026-08-21 + +First tagged release. The repository has been public since 2026-07-29 and there was no +tag before this one, so anyone who cloned in between holds a copy with the defects listed +under **Fixed** — several of which are silent. Upgrading is worth it. + +Two audits, in July and August, produced the work below. Every fix carries a test, and +each test was checked against the specific defect it guards rather than only against the +previous state of `main`. The suite went from 37 assertions to 188. + +### Changed — read before upgrading + +- **`submit`'s extra arguments are quoted** and no longer re-parsed by the remote shell. + `README` always promised they reached `sbatch` unchanged; they now do. An argument that + relied on remote expansion (a `$VAR`, a backtick) will stop expanding. +- **A remote script path given to `submit` is literal.** `HS_REMOTE_WORKDIR` is still + expanded by the cluster, deliberately — it is where a profile's `$USER` has to resolve. +- **`watch` clamps its interval** to `HS_WATCH_MIN_INTERVAL` (30 s) and stops at + `HS_WATCH_MAX_SECONDS` (one hour), saying so in both cases. +- **`watch` has a third exit status.** `0` finished, `1` unknown, **`3` stopped at the cap + with the job still queued**. Only `0` means the job is done. +- **`queue` prints the whole job id.** The column is ragged now; it used to be truncated, + which turned the array task `12345678_10` into `12345678_1` — a valid id for a + different task. +- **`watch`, `fetch`, `cancel` and `submit` reject anything that is not a job id**, before + it reaches a remote command line. +- **The no-argument subcommands reject trailing arguments.** `open -p bigiron` used to + open the *default* cluster while appearing to select a profile, and spend a code on it. +- **`local` no longer claims to help `sshfs`**, which reads neither `RSYNC_RSH` nor + `GIT_SSH_COMMAND`. Use `-o ssh_command="ssh $(hpc-session ssh-opts)"`. +- **A VPN configured with `HS_VPN_STATUS_CMD` alone is recognised**, which is the shape + `docs/vpn-hooks.md` recommends for a tunnel you raise yourself. It previously reported + as "not configured". +- **The tool's own remote commands run under `sh`**, whatever login shell the account + uses. `hpc-session run` is untouched: that is your command line. + +### Added + +- **CI.** The suite runs on Linux and on macOS's `/bin/bash` 3.2 — the real floor this + tool is written to — plus `shellcheck`. +- **`hpc-session templates`**, and `render ` resolved against `HS_TEMPLATE_DIR`, so + a skill that owns a job type can own its job shape without hardcoding a path into this + repository. `SKILL.md` states the contract across that seam. +- `HS_WATCH_INTERVAL`, `HS_WATCH_MIN_INTERVAL`, `HS_WATCH_MAX_SECONDS`, + `HS_WATCH_MAX_MISSES`, `HS_TEMPLATE_DIR`. +- `SLURM_NTASKS` as a template placeholder. It was hard-coded to 1 while `--nodes` was a + placeholder, so any `HS_SLURM_NODES` above 1 allocated nodes that then sat idle. +- A second worked example profile, for the ordinary key-only cluster. + +### Fixed + +- **`watch` reported a running job as finished** whenever a remote query failed with + anything other than ssh's own 255 — an invalid job id, a `squeue` behind a module, a + restarting controller, a `csh` login shell. The output was identical to a real + completion. It now requires positive evidence that the controller was reached. +- **Configuration precedence was inverted.** The profile was sourced last, so every + documented one-off override (`HS_HOST=other hpc-session status`) was silently discarded. +- **The shipped `HS_REMOTE_WORKDIR` broke `submit` on every OpenSSH ≥ 9.0 client**, which + speaks SFTP and runs no remote shell to expand `$USER`. +- **`fetch` could deliver an unrelated file** from the remote home: `ls -1` on a matching + *directory* prints its contents as bare relative names. It also skipped a workdir whose + last component was a symlink, reporting "nothing matching" — which reads as "the job + wrote nothing". +- **`fetch` returned 0 on a partial retrieval.** Its status was the last copy's. +- **`init` could not create a profile named with `-p`**, the only selector `README` + documents, and the error's own suggested remedy failed when `HS_PROFILE` was exported. +- **The `command` TOTP backend could not open a session at all** — it was asked for a + stored seed it has none of by design. +- **A bad key under `AuthenticationMethods publickey,keyboard-interactive`** was retried + three times over ~90 s instead of failing at once. +- **A local code-generation failure waited out three full time steps** for something that + could never change, judging a stale error string. +- **A failed control-directory `mkdir` was swallowed**, so the session opened and simply + never multiplexed — which looks like a slow cluster, not a broken setup. +- **A mistyped `HS_TOTP_ALGO` or `HS_TOTP_PERIOD` was reported as an invalid seed**, + sending the user to re-enrol over a typo. +- **`--help` printed executable code**, having sliced a line range that had drifted. +- `sbatch`'s federated message form made `submit` return the **cluster name** as the job + id; `--parsable` made it fail *after* the job was queued. +- `config.example` omitted `HS_CONTROL_DIR`, `HS_CONFIG_DIR` and `HS_OTP`. A test now + holds every default to appearing there. +- Four documentation claims the code did not honour, including two in `SECURITY.md`. + +### Security + +- **The TOTP seed no longer passes through `argv`** on the keychain path, where `ps` and + any exec-logging agent could see it. It travels on stdin. +- **A live code no longer survives a signal.** A trap removes the temporary file on + `EXIT`, `INT`, `TERM` and `HUP`; previously a Ctrl-C between writing it and `ssh` + reading it left a valid second factor on disk with nothing left to delete it. +- **The keychain identifiers are refused if they contain a quote, a backslash or a + newline.** `security -i` re-tokenises the storing command and runs one command per line, + so either could inject options — or a whole second command — into something running + against the user's keychain. +- **The `file` backend enforces mode 0600** on an existing file. `umask` governs creation + only, so a seed written into a file that was already 0644 stayed world-readable. +- `SECURITY.md` now records what `-T /usr/bin/security` costs: any process running as you + can read the seed back without a prompt, which is what makes unattended `open` work. + +### Known limitations + +- `README` states OpenSSH 6.7+, which is correct for multiplexing. The unattended TOTP + path additionally needs `SSH_ASKPASS_REQUIRE`, which is newer + ([#13](https://github.com/HolobiomicsLab/hpc-session/issues/13)). A caller with a + terminal is unaffected. +- Multi-cluster (federated) submission is out of scope. `submit` says so when `sbatch` + reports another cluster. +- Everything above is verified offline, against stubs. No part of this release has been + exercised against a live SLURM controller. diff --git a/README.md b/README.md index 838110f..e931132 100644 --- a/README.md +++ b/README.md @@ -285,8 +285,9 @@ in either skill. ## Contributing Issues and pull requests are welcome, particularly worked profiles for other clusters -(as `examples/.conf`, with placeholders instead of real logins) and VPN hooks for -clients not yet covered. Please keep `tests/run_tests.sh` green and add a case for any +(as `examples/.conf`, with placeholders instead of real logins — see +[`examples/`](examples/) for the two shapes already there) and VPN hooks for clients not +yet covered. Please keep `tests/run_tests.sh` green and add a case for any behaviour you change — CI runs it on Linux and on macOS's bash 3.2, plus `shellcheck`. ## Authors @@ -294,6 +295,11 @@ behaviour you change — CI runs it on Linux and on macOS's bash 3.2, plus `shel Developed at the [HolobiomicsLab](https://github.com/HolobiomicsLab) — CNRS and Université Côte d'Azur. +## Changelog + +[CHANGELOG.md](CHANGELOG.md). Behaviour changes worth knowing before upgrading are listed +first in each release. + ## License MIT — see [LICENSE](LICENSE). Copyright CNRS and Université Côte d'Azur. diff --git a/examples/keyonly-direct.conf b/examples/keyonly-direct.conf new file mode 100644 index 0000000..c27bc49 --- /dev/null +++ b/examples/keyonly-direct.conf @@ -0,0 +1,37 @@ +# Example profile: the ordinary case — a cluster you reach directly, with an SSH key and +# no second factor. Start here; the other example adds a VPN and TOTP on top. +# +# Copy to ~/.config/hpc-session/.conf, replace every PLACEHOLDER, then: +# hpc-session -p doctor +# +# Contains no credentials, and should never contain any. + +# Requires a matching block in ~/.ssh/config, which is where your key, user and any jump +# host belong — every other tool can see them there too: +# Host mycluster +# HostName login.cluster.example.edu +# User YOUR_CLUSTER_LOGIN +HS_HOST="mycluster" + +# Job scripts are copied here and submitted from here. Escape $USER so the REMOTE shell +# expands it — since OpenSSH 9.0 scp speaks SFTP and runs no remote shell, so an +# unexpanded one would reach the copy verbatim. Prefer scratch over your home directory. +HS_REMOTE_WORKDIR="/scratch/\$USER/jobs" + +# Defaults for `hpc-session render`. Leave a value empty and its #SBATCH line is dropped, +# so an unset account does not become an `--account=` that SLURM rejects. +HS_SLURM_ACCOUNT="" +HS_SLURM_PARTITION="YOUR_PARTITION" +HS_SLURM_TIME="02:00:00" +HS_SLURM_CPUS="4" +HS_SLURM_MEM="16G" + +# No VPN and no second factor: leave every hook empty and the tool behaves as if neither +# existed. `open` is then a plain key login, and `doctor` needs no python3. +HS_VPN_UP_CMD="" +HS_VPN_STATUS_CMD="" +HS_TOTP_BACKEND="none" + +# Nothing here monopolises your link, so a long-lived master is pure win: one login covers +# a working day. Shorten it if your site drops idle connections. +HS_CONTROL_PERSIST="8h" From 02265e679ce3ca6f7c6c99c2f8a0ab6a7a50762c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Louis-F=C3=A9lix=20Nothias?= Date: Fri, 21 Aug 2026 13:11:29 +0200 Subject: [PATCH 2/5] feat: let the tool say which release it is MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A first tag makes version a question with an answer, and CHANGELOG.md opens with behaviour changes — so a caller has to be able to tell which side of them it is on. Nothing in the tool could say. `version` is dispatched before the profile loads, like `init`: a setup that does not load is exactly when someone needs to report which copy they are running. `doctor` opens with the same line, since that output is what gets pasted into an issue. A test holds the string equal to the newest heading in CHANGELOG.md, so a tag cannot drift from what the tool reports. Verified by mutation: drifting the string fails three assertions, dropping the doctor line fails one, and moving the dispatch after the loader fails the two that cover a broken profile. Also corrects two claims in the changelog itself — the repository's public date, which was off by a timezone, and the assertion count. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 11 +++++++---- README.md | 3 ++- SKILL.md | 3 +++ bin/hpc-session | 11 +++++++++++ tests/run_tests.sh | 21 +++++++++++++++++++++ 5 files changed, 44 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 58e7ee0..b4c0909 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,13 +2,13 @@ ## 0.1.0 — 2026-08-21 -First tagged release. The repository has been public since 2026-07-29 and there was no -tag before this one, so anyone who cloned in between holds a copy with the defects listed -under **Fixed** — several of which are silent. Upgrading is worth it. +First tagged release. The repository has been public since late July 2026 and there was +no tag before this one, so anyone who cloned in between holds a copy with the defects +listed under **Fixed** — several of which are silent. Upgrading is worth it. Two audits, in July and August, produced the work below. Every fix carries a test, and each test was checked against the specific defect it guards rather than only against the -previous state of `main`. The suite went from 37 assertions to 188. +previous state of `main`. The suite went from 37 assertions to 194. ### Changed — read before upgrading @@ -47,6 +47,9 @@ previous state of `main`. The suite went from 37 assertions to 188. `HS_WATCH_MAX_MISSES`, `HS_TEMPLATE_DIR`. - `SLURM_NTASKS` as a template placeholder. It was hard-coded to 1 while `--nodes` was a placeholder, so any `HS_SLURM_NODES` above 1 allocated nodes that then sat idle. +- **`hpc-session version`**, also reported by `doctor`. It answers before the profile + loads, so a broken setup can still say which copy it is; a test holds it equal to the + newest heading in this file. - A second worked example profile, for the ordinary key-only cluster. ### Fixed diff --git a/README.md b/README.md index e931132..ca08ecc 100644 --- a/README.md +++ b/README.md @@ -298,7 +298,8 @@ Université Côte d'Azur. ## Changelog [CHANGELOG.md](CHANGELOG.md). Behaviour changes worth knowing before upgrading are listed -first in each release. +first in each release. `hpc-session version` says which copy you have, and `doctor` opens +with the same line — quote it when reporting anything. ## License diff --git a/SKILL.md b/SKILL.md index eda7ae6..6bbccfc 100644 --- a/SKILL.md +++ b/SKILL.md @@ -20,10 +20,13 @@ of `hpc-session`; see [README.md](README.md) for the full reference and ## Before anything else ```bash +hpc-session version # which release this is — behaviour differs between them hpc-session doctor # no network — reports what is configured and what is missing hpc-session status # is the master up? the VPN? is a TOTP seed present? ``` +`version` answers even when the profile is missing or broken, so it is safe to ask first. + If no profile exists, run `hpc-session init` — or `hpc-session -p init` for a second cluster — and tell the user which keys they must fill in (`HS_HOST` at minimum). Do not invent a hostname, account or partition — ask. diff --git a/bin/hpc-session b/bin/hpc-session index 64ba3b3..97cc5a4 100755 --- a/bin/hpc-session +++ b/bin/hpc-session @@ -15,6 +15,7 @@ # hpc-session watch 12345 # poll until it leaves the queue # hpc-session fetch 12345 ./results # bring the job's files back # hpc-session close # drop master + VPN, freeing the link +# hpc-session version # which release this copy is # # Setup: hpc-session init [profile] -> edit the profile -> hpc-session doctor # Docs: README.md, docs/2fa-enrollment.md, docs/vpn-hooks.md @@ -82,6 +83,7 @@ hs_doctor_vpn() { # Everything checkable without touching the network — safe to run before a first login. hs_doctor() { + echo "hpc-session $HS_VERSION" echo "profile $HS_PROFILE ($(hs_profile_path "$HS_PROFILE"))" [ -n "$HS_HOST" ] && hs_report host ok "$HS_HOST" || hs_report host fail "HS_HOST is unset" hs_doctor_tool ssh "the whole tool depends on it" @@ -122,6 +124,15 @@ if [ "${1:-}" = init ]; then exit fi +# Dispatched before the loader, like `init`: a missing or broken profile is exactly when +# someone needs to say which copy of the tool they are running, and the answer does not +# depend on a profile. CHANGELOG.md is the other half of this string; a test holds them +# together so a tag cannot drift from what the tool reports. +HS_VERSION="0.1.0" +case "${1:-}" in + version|--version) printf 'hpc-session %s\n' "$HS_VERSION"; exit 0 ;; +esac + hs_load_profile case "${1:-}" in diff --git a/tests/run_tests.sh b/tests/run_tests.sh index 0c42484..618f67f 100755 --- a/tests/run_tests.sh +++ b/tests/run_tests.sh @@ -907,6 +907,27 @@ help_text=$("$HS_ROOT/bin/hpc-session" --help) check_contains "help mentions open" "hpc-session open" "$help_text" check_contains "help mentions submit" "hpc-session submit" "$help_text" +# A release whose tool cannot say which release it is leaves a caller — an agent most of +# all — no way to know which side of a behaviour change it is on. The version has to +# survive a profile that does not load, and it has to match the newest CHANGELOG heading, +# or the tag, the notes and the tool drift apart silently. +ver_out=$("$HS_ROOT/bin/hpc-session" version 2>&1); rc=$? +check "version exits 0" 0 "$rc" +check_contains "version names the tool" "hpc-session" "$ver_out" +ver_num=${ver_out##* } +changelog_num=$(sed -n 's/^## \([0-9][0-9.]*\) .*/\1/p' "$HS_ROOT/CHANGELOG.md" | head -1) +check "version matches the changelog" "$changelog_num" "$ver_num" + +# It must answer with a profile that does not exist, which is when someone is diagnosing. +ver_broken=$(HS_CONFIG_DIR="$(mktemp -d "${TMPDIR:-/tmp}/hstest.XXXXXX")" \ + "$HS_ROOT/bin/hpc-session" -p nosuchprofile version 2>&1); rc=$? +check "version survives a missing profile" 0 "$rc" +check "and still reports the version" "hpc-session $changelog_num" "$ver_broken" + +doc_ver=$(HS_CONFIG_DIR="$(mktemp -d "${TMPDIR:-/tmp}/hstest.XXXXXX")" \ + "$HS_ROOT/bin/hpc-session" doctor 2>&1) +check_contains "doctor reports the version" "hpc-session $changelog_num" "$doc_ver" + bad=$("$HS_ROOT/bin/hpc-session" push only-one-arg 2>&1); rc=$? check "push with one argument fails" 1 "$rc" check_contains "push explains itself" "usage: push" "$bad" From 079556ecf9df532baea5f318f93c097173f3a1b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Louis-F=C3=A9lix=20Nothias?= Date: Fri, 21 Aug 2026 16:05:53 +0200 Subject: [PATCH 3/5] docs: the release has now been run against a live controller MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "Known limitations" section said no part of this release had been exercised against a live SLURM controller. That is no longer true: one end-to-end run — open, render, submit, queue, watch to completion, fetch, cancel, close — was made against a TOTP-gated cluster behind a VPN, and behaved as documented. Stated as one site and one SLURM version, which is what it is, rather than as a compatibility claim. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b4c0909..02389d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -110,5 +110,7 @@ previous state of `main`. The suite went from 37 assertions to 194. terminal is unaffected. - Multi-cluster (federated) submission is out of scope. `submit` says so when `sbatch` reports another cluster. -- Everything above is verified offline, against stubs. No part of this release has been - exercised against a live SLURM controller. +- The test suite is entirely offline, against stubs. Separately, one end-to-end run was + made against a live SLURM controller (a TOTP-gated cluster behind a VPN): `open`, + `render`, `submit`, `queue`, `watch` to completion, `fetch`, `cancel`, `close`, all as + documented. That is one site and one SLURM version, not a compatibility claim. From 3d356bba5621d5d43f0400d4d3b8b4ac1d61e970 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Louis-F=C3=A9lix=20Nothias?= Date: Thu, 1 Oct 2026 14:55:34 +0200 Subject: [PATCH 4/5] fix(totp): say why a seed read failed, and try it once more MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A failed read sent the backend's stderr to /dev/null, so open, doctor and status all said "no seed — run store-seed" while the seed sat in the keychain, refused for one request only. The backend's own message, its exit status and, for the keychain, the macOS session the read ran in are now appended to HS_TOTP_ERROR_LOG, and the hint repeats the latest reason. The seed itself is never written anywhere. The read is tried once more after HS_TOTP_READ_RETRY_DELAY seconds, which absorbs a refusal that clears by itself; not after a cancelled prompt, and not for a missing seed file. Thirteen new assertions, with `security` shadowed throughout; they fail on the previous code. Suite: 207 passed. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 14 +++++++ SECURITY.md | 4 ++ config.example | 6 +++ docs/2fa-enrollment.md | 1 + lib/config.sh | 4 ++ lib/totp.sh | 88 +++++++++++++++++++++++++++++++++++++++--- tests/run_tests.sh | 55 ++++++++++++++++++++++++++ 7 files changed, 166 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 02389d3..d1d15be 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,19 @@ # Changelog +## Unreleased + +### Fixed + +- **A seed read that fails now says why.** The backend's message used to go to + `/dev/null`, so `open`, `doctor` and `status` said "no seed — run store-seed" even when + the seed was stored and the keychain had only refused one request. They now repeat the + backend's own reason. Every failed read is appended to `HS_TOTP_ERROR_LOG` (default + `$HS_CONFIG_DIR/.totp-errors.log`) with its exit status and, for the keychain, + the macOS session it ran in. Only the error is written; the seed never is. +- **A failed read is tried once more** after `HS_TOTP_READ_RETRY_DELAY` seconds (default + 2), which absorbs a refusal that clears by itself. Not after a cancelled prompt, which + would ask again someone who just said no, and not for a missing seed file. + ## 0.1.0 — 2026-08-21 First tagged release. The repository has been public since late July 2026 and there was diff --git a/SECURITY.md b/SECURITY.md index 108e033..9c4bf27 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -28,6 +28,10 @@ TOTP seed if you configure a backend that stores one. code (in which case nothing is stored here). - **The seed never appears in `argv` or the environment.** It is piped into the code generator on stdin, so `ps` cannot see it. +- **A failed seed read is logged; the seed never is.** The backend's error message, its + exit status and the time go to `HS_TOTP_ERROR_LOG`, created mode 0600 beside your + profiles. A successful read hands the seed on with a shell builtin, so it still reaches + no `argv`, no environment and no file. - **The generated code** is written to a `mktemp` file (mode 0600) and read exactly once: the askpass helper prints it and deletes it. That single-shot behaviour is deliberate — a helper that kept answering would let `ssh` retry a stale code in a loop. diff --git a/config.example b/config.example index f2717ff..5a5499c 100644 --- a/config.example +++ b/config.example @@ -67,6 +67,12 @@ HS_TOTP_DIGITS="6" HS_TOTP_PERIOD="30" HS_TOTP_ALGO="sha1" +# A seed read that fails is tried once more after this many seconds, and the backend's +# own message — never the seed — is appended to the log. Empty means the default, +# $HS_CONFIG_DIR/.totp-errors.log. open, doctor and status repeat the latest one. +HS_TOTP_READ_RETRY_DELAY="2" +HS_TOTP_ERROR_LOG="" + # --- connection tuning -------------------------------------------------------------- HS_CONTROL_PERSIST="8h" # how long an idle master survives HS_CONNECT_TIMEOUT="25" # seconds diff --git a/docs/2fa-enrollment.md b/docs/2fa-enrollment.md index d745753..79be2df 100644 --- a/docs/2fa-enrollment.md +++ b/docs/2fa-enrollment.md @@ -106,6 +106,7 @@ stall for up to 30 seconds waiting for a fresh step. |---|---|---| | `auth failed … waiting Ns for a fresh code` | the previous code was already consumed | it retries by itself; wait one step | | `could not open the master`, repeatedly | VPN down, or wrong seed | `hpc-session status`; compare `hpc-session code` with the phone | +| `the seed in 'keychain' could not be read: …` | the keychain refused this read, often because it wanted to show a prompt and the command ran outside your logged-in session | the message names the reason; `HS_TOTP_ERROR_LOG` keeps every failed read with the session it ran in. Run `doctor` from your own terminal to compare | | Commands hang instead of failing | stale socket after the tunnel dropped | `hpc-session close` then `open` (it also cleans up on its own) | | Every code rejected | clock skew — TOTP is time-based | check the machine's clock is NTP-synced | | Codes rejected only sometimes | your site may not use the default profile | set `HS_TOTP_DIGITS` / `HS_TOTP_PERIOD` / `HS_TOTP_ALGO` | diff --git a/lib/config.sh b/lib/config.sh index 959e919..d826b8c 100644 --- a/lib/config.sh +++ b/lib/config.sh @@ -56,6 +56,10 @@ hs_apply_defaults() { : "${HS_TOTP_DIGITS:=6}" : "${HS_TOTP_PERIOD:=30}" : "${HS_TOTP_ALGO:=sha1}" + # A failed seed read is tried once more after this many seconds, and the backend's own + # message — never the seed — goes to this log. See hs_seed_read. + : "${HS_TOTP_READ_RETRY_DELAY:=2}" + : "${HS_TOTP_ERROR_LOG:=$HS_CONFIG_DIR/$HS_PROFILE.totp-errors.log}" : "${HS_REMOTE_WORKDIR:=.}" # Where `render ` looks. Point it at another skill's assets and that skill owns its # own job shapes without owning any paths — see "Composing with other skills" in SKILL.md. diff --git a/lib/totp.sh b/lib/totp.sh index 57123a8..d010a6b 100644 --- a/lib/totp.sh +++ b/lib/totp.sh @@ -39,16 +39,84 @@ print(str(truncated % (10 ** digits)).zfill(digits)) # Seconds remaining in the current TOTP step. hs_step_left() { echo $(( HS_TOTP_PERIOD - ($(date +%s) % HS_TOTP_PERIOD) )); } -# Print the stored base32 seed. Fails if the backend holds none. -hs_seed_read() { +# One read from the backend, with nothing hidden: the seed on stdout, the backend's own +# complaint, if any, on stderr. +hs_seed_fetch() { case "$HS_TOTP_BACKEND" in - keychain) security find-generic-password -s "$HS_TOTP_SERVICE" -a "$HS_TOTP_ACCOUNT" -w 2>/dev/null ;; - pass) pass show "$HS_TOTP_PASS_ENTRY" 2>/dev/null | head -1 ;; - file) [ -f "$HS_TOTP_FILE" ] && head -1 "$HS_TOTP_FILE" ;; - *) return 1 ;; + keychain) security find-generic-password -s "$HS_TOTP_SERVICE" -a "$HS_TOTP_ACCOUNT" -w ;; + pass) pass show "$HS_TOTP_PASS_ENTRY" | head -1 ;; + file) [ -f "$HS_TOTP_FILE" ] || { echo "no file at $HS_TOTP_FILE" >&2; return 1; } + head -1 "$HS_TOTP_FILE" ;; + *) echo "backend '$HS_TOTP_BACKEND' stores no seed" >&2; return 1 ;; esac } +# Print the stored base32 seed. Fails if the backend holds none, or will not hand it over. +# +# A failed read used to vanish: the backend's stderr went to /dev/null, and every caller then +# said "no seed — run store-seed", even when the seed was stored and the keychain had merely +# refused this one request. That pointed the user at re-enrolling a second factor that was +# fine, and left a refusal that came and went with nothing to diagnose it from. Now the +# backend's message is appended to HS_TOTP_ERROR_LOG, and the read is tried once more after +# HS_TOTP_READ_RETRY_DELAY seconds, which absorbs a refusal that clears by itself. +# +# Only stderr is ever written down. stdout IS the seed: it is held in a local variable and +# handed on with `printf`, a builtin, so it still reaches no argv, no environment and no file. +# +# No second attempt after a cancelled prompt, which would ask a user who just said no again, +# nor for a file that does not exist, which a few seconds will not create. +hs_seed_read() { + local attempt seed rc err_file delay="${HS_TOTP_READ_RETRY_DELAY:-2}" + case "$delay" in ''|*[!0-9]*) delay=2 ;; esac + for attempt in 1 2; do + err_file=$(mktemp "${TMPDIR:-/tmp}/hsseed.XXXXXX") || return 1 + seed=$(hs_seed_fetch 2>"$err_file"); rc=$? + if [ "$rc" -eq 0 ] && [ -n "$seed" ]; then + rm -f "$err_file" "$(hs_seed_error_log).last" + printf '%s\n' "$seed" + return 0 + fi + seed="" + hs_seed_error_record "$attempt" "$rc" "$err_file" + rm -f "$err_file" + [ "$attempt" = 1 ] || return 1 + case "$HS_TOTP_BACKEND:$HS_SEED_LAST_ERROR" in + file:*|*[Cc]ancel*) return 1 ;; + esac + sleep "$delay" + done + return 1 +} + +hs_seed_error_log() { + echo "${HS_TOTP_ERROR_LOG:-${HS_CONFIG_DIR:-$HOME/.config/hpc-session}/${HS_PROFILE:-default}.totp-errors.log}" +} + +# Append one line per failed read: when, which backend, which attempt, its exit status, the +# macOS session the read ran in, and what the backend said. The latest message is also kept +# beside the log, so the hint printed by open, doctor and status can repeat it; a read that +# succeeds removes it. +# +# The session matters for the keychain. `security` started outside the logged-in GUI +# session cannot show a prompt, so a keychain that would ask the user refuses instead — one +# way a read can work from a terminal and fail from an agent at the same moment. +hs_seed_error_record() { # attempt, exit status, file holding the backend's stderr + local why log session="" + why=$(tr '\n' ' ' < "$3" | sed 's/ */ /g; s/ $//' | cut -c1-300) + [ -n "$why" ] || why="the backend printed nothing and gave no reason" + HS_SEED_LAST_ERROR="$why" + if [ "$HS_TOTP_BACKEND" = keychain ] && command -v launchctl >/dev/null 2>&1; then + session=" session=$(launchctl managername 2>/dev/null || echo unknown)" + fi + log=$(hs_seed_error_log) + ( umask 077 + mkdir -p "$(dirname "$log")" \ + && printf '%s %s attempt=%s exit=%s%s %s\n' "$(date -u +%Y-%m-%dT%H:%M:%SZ)" \ + "$HS_TOTP_BACKEND" "$1" "$2" "$session" "$why" >> "$log" \ + && printf '%s\n' "$why" > "$log.last" ) 2>/dev/null + return 0 +} + # Can this profile produce a code at all? Asked before the VPN goes up, so a failure is # cheap rather than stranding the link. # @@ -68,9 +136,17 @@ hs_have_seed() { # step of setup: telling a `command` user to run `store-seed` — for a backend that stores # nothing, and whose store-seed exits 1 saying exactly that — left them with no way forward # until they reached `open`, which is the one place the right message used to live. +# +# When the last read left a reason behind, that reason leads. "No seed" is only one of the +# ways a read fails; the keychain refusing this one request is another, and re-enrolling +# does nothing for it. hs_seed_hint() { + local last log + log=$(hs_seed_error_log) if [ "$HS_TOTP_BACKEND" = command ]; then echo "HS_TOTP_CMD is empty — set it to a command that prints one code" + elif last=$(cat "$log.last" 2>/dev/null) && [ -n "$last" ]; then + echo "the seed in '$HS_TOTP_BACKEND' could not be read: $last (history in $log; if no seed was ever stored, run: hpc-session store-seed)" else echo "no seed in '$HS_TOTP_BACKEND' and no HS_OTP — run: hpc-session store-seed" fi diff --git a/tests/run_tests.sh b/tests/run_tests.sh index 618f67f..9c04e1b 100755 --- a/tests/run_tests.sh +++ b/tests/run_tests.sh @@ -686,6 +686,61 @@ poison_probe "o'brien" me >/dev/null 2>&1 check "an apostrophe is accepted" 0 "$(status_of test -s "$keychain_log")" rm -rf "$keychain_bin" +# A failed seed read must say why, and get a second chance. It used to be silent: stderr +# went to /dev/null, and open, doctor and status all said "no seed — run store-seed" while +# the seed sat in the keychain, refused for one request only. `security` is SHADOWED here +# for the same reason as above: nothing in this block may reach a real keychain. +read_bin=$(mktemp -d "${TMPDIR:-/tmp}/hstest.XXXXXX") +read_log="$read_bin/errors.log" +cat > "$read_bin/security" </dev/null || echo 0); n=\$((n + 1)); echo "\$n" > "$read_bin/count" +case "\$FAKE_SECURITY" in + flaky) [ "\$n" -ge 2 ] && { echo GEZDGNBVGY3TQOJQ; exit 0; } ;; + cancel) echo "security: SecKeychainSearchCopyNext: User canceled the operation." >&2; exit 128 ;; +esac +echo "security: SecKeychainSearchCopyNext: User interaction is not allowed." >&2 +exit 36 +STUB +chmod +x "$read_bin/security" +seed_probe() { # stub mode; prints whatever the read handed over + rm -f "$read_bin/count" "$read_log" "$read_log.last" + ( PATH="$read_bin:$PATH"; export FAKE_SECURITY="$1" + HS_TOTP_BACKEND=keychain HS_TOTP_SERVICE=svc HS_TOTP_ACCOUNT=me \ + HS_TOTP_ERROR_LOG="$read_log" HS_TOTP_READ_RETRY_DELAY=0 hs_seed_read ) 2>/dev/null +} +seed=$(seed_probe flaky); rc=$? +check "a refusal that clears is absorbed" "0|GEZDGNBVGY3TQOJQ" "$rc|$seed" +check "and is recorded once" 1 "$(grep -c . "$read_log")" +check_contains "with the keychain's own reason" "User interaction is not allowed" "$(cat "$read_log")" +check_contains "and its exit status" "exit=36" "$(cat "$read_log")" +check "the seed itself is never written down" 1 "$(status_of grep -q GEZDGNBVGY3TQOJQ "$read_log")" +check "a good read clears the latest reason" 1 "$(status_of test -e "$read_log.last")" +seed=$(seed_probe refuse); rc=$? +check "a refusal that persists still fails" "1|" "$rc|$seed" +check "after exactly two attempts" 2 "$(grep -c . "$read_log")" +hint=$(HS_TOTP_BACKEND=keychain HS_TOTP_ERROR_LOG="$read_log" hs_seed_hint) +check_contains "the hint repeats the reason" "User interaction is not allowed" "$hint" +seed_probe cancel >/dev/null +check "a cancelled prompt is not shown twice" 1 "$(grep -c . "$read_log")" +rm -f "$read_log" "$read_log.last" +( HS_TOTP_BACKEND=file HS_TOTP_FILE=/nonexistent/seed HS_TOTP_ERROR_LOG="$read_log" \ + HS_TOTP_READ_RETRY_DELAY=0 hs_seed_read ) >/dev/null 2>&1 +check "a missing seed file is not read twice" 1 "$(grep -c . "$read_log")" +# Through the subcommand, as for the hint below: the fix is about what doctor says. +kc_dir=$(mktemp -d "${TMPDIR:-/tmp}/hstest.XXXXXX") +cat > "$kc_dir/kc.conf" <<'PROFILE' +HS_HOST="cluster.invalid" +HS_TOTP_BACKEND="keychain" +HS_TOTP_READ_RETRY_DELAY="0" +PROFILE +rm -f "$read_bin/count" +kc_out=$(PATH="$read_bin:$PATH" FAKE_SECURITY=refuse HS_CONFIG_DIR="$kc_dir" \ + "$HS_ROOT/bin/hpc-session" -p kc doctor 2>&1) +check_contains "doctor says why the keychain refused" "User interaction is not allowed" "$kc_out" +check "and keeps the history beside the profile" 0 "$(status_of test -s "$kc_dir/kc.totp-errors.log")" +rm -rf "$kc_dir" "$read_bin" + # `doctor` is documented as the last step of setup, so it must not send a `command` backend # user to store-seed — a subcommand that exits 1 saying it stores nothing. # From f9a2b8283fa577a68f7e4518110ac7f252fcfea2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Louis-F=C3=A9lix=20Nothias?= Date: Wed, 7 Oct 2026 11:27:27 +0200 Subject: [PATCH 5/5] feat: close keeps a link another session still holds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One master and one tunnel serve every process on the machine that uses the profile, and `close` tore both down unconditionally. Two sessions sharing the link — an agent and a wrapper script, two agents — therefore cut each other's connection with their own close, and under 2FA the reopen cost a code the other side could not always produce. `open`, and the implicit open under run, push, pull and local, now record a holder file under $HS_CONTROL_DIR/holders; `close` removes its own and tears down only when no live holder remains, saying who still holds the link. A holder is the calling process by default — live while its pid is — or HS_LEASE= for a session whose commands each run in a fresh shell; both lapse after HS_LEASE_TTL seconds idle (default 14400), refreshed by every open. HS_CLOSE_FORCE=1 closes regardless and clears the holders. `status` lists them. Thirteen offline tests: a close with another holder keeps the link and names it; the last close tears down; a pid holder whose process ended, and a named holder past the TTL, do not keep it; a fresh named holder does; the forced close closes and clears; an implicit open registers; status lists; a holder name cannot leave the directory. Co-Authored-By: Claude Fable 5.1 --- CHANGELOG.md | 12 +++++++ README.md | 11 +++++- SKILL.md | 6 ++++ config.example | 10 ++++++ lib/config.sh | 4 +++ lib/session.sh | 83 ++++++++++++++++++++++++++++++++++++++++++++-- tests/run_tests.sh | 68 +++++++++++++++++++++++++++++++++++++ 7 files changed, 190 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d1d15be..7927ff3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,18 @@ ## Unreleased +### Added + +- **`close` keeps a link another session still holds.** One master and one tunnel serve + every process on the machine that uses the profile, and `close` tore both down + unconditionally — so two sessions sharing the link (an agent and a wrapper script, two + agents) each cut the other's connection with its own close, and under 2FA the reopen + cost a code. `open`, and the implicit open under `run`, `push`, `pull` and `local`, now + record a holder; `close` removes its own and tears down only when no live holder remains. + A holder is the calling process by default, or `HS_LEASE=` for a session whose + commands each run in a fresh shell; it lapses after `HS_LEASE_TTL` seconds idle (default + 14400); `HS_CLOSE_FORCE=1` closes regardless and clears them. `status` lists the holders. + ### Fixed - **A seed read that fails now says why.** The backend's message used to go to diff --git a/README.md b/README.md index ca08ecc..dda93af 100644 --- a/README.md +++ b/README.md @@ -55,12 +55,21 @@ the same connection, hand it `$(hpc-session ssh-opts)`. hpc-session open # VPN up + one authenticated master hpc-session run squeue -u \$USER # runs ON the cluster hpc-session run -- sacct -j 12345 -hpc-session close # drop master + VPN, freeing the link +hpc-session close # drop master + VPN once no one else holds them ``` `open` … `run` … `run` … `close` is the pattern that matters. Everything between the first and last call is free. +Two sessions on one machine — an agent and a wrapper script, two agents — share one +master and one tunnel. `open` records a *holder* for the caller, and `close` tears the +link down only when no live holder remains, so one session's `close` no longer cuts the +other's connection. The holder is the calling process by default; a session that runs +each command in a fresh shell should export `HS_LEASE=` +so its holder outlives the shell. `status` lists the holders, a holder idle for +`HS_LEASE_TTL` seconds (default 4 h) lapses, and `HS_CLOSE_FORCE=1 hpc-session close` +tears down regardless. + ### Running things, in the right place The distinction that bites people: diff --git a/SKILL.md b/SKILL.md index 6bbccfc..68a4e24 100644 --- a/SKILL.md +++ b/SKILL.md @@ -145,6 +145,12 @@ never has to appear in your instructions. Then: - **open once, close once.** Everything in between is free; a connect-per-step loop costs a code each time. +- **name your holder when you share the machine.** Another agent or a wrapper script may + be on the same master and tunnel; `close` tears them down only when no live holder + remains. Your holder is the calling process by default, which for an agent is a shell + that ends with the command — so export `HS_LEASE=` on + every call (a session id does), and close with the same name. `status` lists the + holders; `HS_CLOSE_FORCE=1 hpc-session close` is for a holder you know is dead. - **capture the job id from `submit`'s stdout** — `job=$(hpc-session submit job.slurm)` — and pass that value on. Do not re-derive it from a log. - **read `watch`'s exit status** (0 finished, 1 unknown, 3 still queued). Only 0 licenses diff --git a/config.example b/config.example index 5a5499c..912012b 100644 --- a/config.example +++ b/config.example @@ -44,6 +44,16 @@ HS_VPN_UP_CMD="" HS_VPN_DOWN_CMD="" HS_VPN_STATUS_CMD="" +# --- sharing the link between sessions ---------------------------------------------- +# One master and one tunnel serve every process that uses this profile. `open` records a +# holder for the caller; `close` tears the link down only when no live holder remains. +# The holder is the calling process by default. A session that runs each command in a +# fresh shell (an agent) should set HS_LEASE to a name stable for that session, usually +# from the environment: HS_LEASE="${CLAUDE_CODE_SESSION_ID:-}" or "${TMUX_PANE:-}". +HS_LEASE="" +HS_LEASE_TTL="14400" # a holder idle this many seconds lapses (4 h); every open refreshes it +HS_CLOSE_FORCE="0" # 1 = close tears down whoever holds the link, and clears the holders + # --- two-factor authentication (optional) ------------------------------------------ # none — key-only login (the common case) # keychain — macOS login Keychain (see HS_TOTP_SERVICE / HS_TOTP_ACCOUNT) diff --git a/lib/config.sh b/lib/config.sh index d826b8c..6e8d41e 100644 --- a/lib/config.sh +++ b/lib/config.sh @@ -42,6 +42,10 @@ hs_apply_defaults() { : "${HS_HOST:=}" : "${HS_CONTROL_DIR:=$HOME/.ssh/hpc-session}" : "${HS_CONTROL_PERSIST:=8h}" + # Who holds the shared link; see "who holds the link" in lib/session.sh. + : "${HS_LEASE:=}" + : "${HS_LEASE_TTL:=14400}" + : "${HS_CLOSE_FORCE:=0}" : "${HS_CONNECT_TIMEOUT:=25}" : "${HS_AUTH_ATTEMPTS:=3}" : "${HS_VPN_UP_CMD:=}" diff --git a/lib/session.sh b/lib/session.sh index d6e7ae7..9f0d060 100644 --- a/lib/session.sh +++ b/lib/session.sh @@ -200,6 +200,68 @@ hs_open_with_retries() { hs_die "could not open the master. Check: VPN up? seed correct ('hpc-session code')? enrolled?" } +# --- who holds the link --------------------------------------------------------------- +# +# One master and one tunnel serve every process on the machine that uses the profile, and +# `close` used to tear both down unconditionally. Two sessions sharing the link — two +# agents, a wrapper script and a shell — therefore took each other's connection away: the +# other side's next command failed on a dead socket, or on a tunnel that was no longer up, +# and under 2FA the reopen cost a code it could not always produce. A holder file per +# opener fixes that: `open` (and the implicit open under run, push, pull and local) records +# who holds the link, `close` removes its own record and tears the link down only when no +# live holder remains. `status` lists them. +# +# A holder is named by HS_LEASE when set, otherwise by the pid of the process that invoked +# the tool — a wrapper script, an interactive shell. The file records that pid and the +# time. A pid holder is live while its process is; a named holder is live until it closes, +# because its pid is usually a transient shell (an agent runs each command in a new one). +# Both expire HS_LEASE_TTL seconds after their last open — the backstop against a session +# that ended without closing — and every open refreshes the stamp, so the expiry is an idle +# time, not a lifetime. HS_CLOSE_FORCE=1 tears down regardless and clears every holder. +hs_lease_name() { + local name="${HS_LEASE:-$PPID}" + case "$name" in + ""|*[!A-Za-z0-9._-]*) hs_die "HS_LEASE must be made of letters, digits, '.', '_' and '-' (got '$name')" ;; + esac + printf '%s' "$name" +} + +hs_pid_alive() { [ "${1:-0}" -gt 1 ] 2>/dev/null && kill -0 "$1" 2>/dev/null; } + +hs_lease_take() { + local dir="$HS_CONTROL_DIR/holders" name named=no + name=$(hs_lease_name) || exit 1 + [ -n "${HS_LEASE:-}" ] && named=yes + mkdir -p "$dir" || hs_die "cannot create $dir" + printf 'pid=%s\nnamed=%s\nsince=%s\n' "$PPID" "$named" "$(date +%s)" > "$dir/$name" \ + || hs_die "cannot record the holder $dir/$name" +} + +hs_lease_drop() { + local name + name=$(hs_lease_name) || exit 1 + rm -f "$HS_CONTROL_DIR/holders/$name" +} + +# Print the live holders, one per line, and remove the rest. +hs_lease_live() { + local dir="$HS_CONTROL_DIR/holders" f key value pid named since now + [ -d "$dir" ] || return 0 + now=$(date +%s) + for f in "$dir"/*; do + [ -f "$f" ] || continue + pid='' named='' since='' + while IFS='=' read -r key value; do + case "$key" in pid) pid=$value ;; named) named=$value ;; since) since=$value ;; esac + done < "$f" + if [ $((now - ${since:-0})) -lt "$HS_LEASE_TTL" ] && { [ "$named" = yes ] || hs_pid_alive "$pid"; }; then + printf '%s\n' "${f##*/}" + else + rm -f "$f" + fi + done +} + hs_open_session() { hs_require_host # Not swallowed: `set -uo pipefail` has no -e, so a failure here used to let the open @@ -207,7 +269,7 @@ hs_open_session() { mkdir -p "$HS_CONTROL_DIR" && chmod 700 "$HS_CONTROL_DIR" \ || hs_die "cannot create the control directory $HS_CONTROL_DIR — without it nothing multiplexes" hs_clean_stale - hs_master_up && { hs_note "master already up"; return 0; } + hs_master_up && { hs_lease_take; hs_note "master already up — joined it as holder $(hs_lease_name)"; return 0; } # Verify a code can be PRODUCED before raising the tunnel — not that it will be # accepted, which cannot be tested from here: under a full tunnel the login node is # usually unreachable until the tunnel is up. Catching the common failure early is still @@ -217,11 +279,19 @@ hs_open_session() { hs_die "cannot produce a TOTP code: $(hs_seed_hint)" fi hs_vpn_connect - hs_open_with_retries + hs_open_with_retries && hs_lease_take } hs_close_session() { hs_require_host + local others + hs_lease_drop + others=$(hs_lease_live | tr '\n' ' ') + if [ -n "$others" ] && [ "$HS_CLOSE_FORCE" != 1 ]; then + hs_note "link kept — still held by: ${others}(HS_CLOSE_FORCE=1 closes it anyway)" + return 0 + fi + [ "$HS_CLOSE_FORCE" = 1 ] && rm -rf "$HS_CONTROL_DIR/holders" hs_ssh -O exit "$HS_HOST" >/dev/null 2>&1 && hs_note "master closed" || hs_note "no master to close" if hs_uses_vpn && hs_vpn_up && [ -n "$HS_VPN_DOWN_CMD" ]; then eval "$HS_VPN_DOWN_CMD" >/dev/null 2>&1 && hs_note "VPN disconnected (link freed)" @@ -229,7 +299,11 @@ hs_close_session() { return 0 } -hs_ensure_open() { hs_master_up || hs_open_session || exit 1; } +# An implicit open joins the link as a holder too: a session that only ever runs commands +# over a master someone else raised is still a user of it. +hs_ensure_open() { + if hs_master_up; then hs_lease_take; else hs_open_session || exit 1; fi +} # Run a command ON THE CLUSTER. BatchMode means a dead master fails fast instead of # hanging on a prompt nothing can answer. @@ -288,7 +362,10 @@ hs_pull() { } hs_status() { + local holders hs_master_up && echo "master: UP" || echo "master: down" + holders=$(hs_lease_live | tr '\n' ' ') + echo "holders: ${holders:-none}" if hs_uses_vpn; then hs_vpn_up && echo "vpn: connected" || echo "vpn: disconnected" else diff --git a/tests/run_tests.sh b/tests/run_tests.sh index 9c04e1b..788c76b 100755 --- a/tests/run_tests.sh +++ b/tests/run_tests.sh @@ -998,5 +998,73 @@ HS_CONFIG_DIR="$init_dir" HS_PROFILE=bigiron "$HS_ROOT/bin/hpc-session" init >/d check "exported HS_PROFILE init creates it" 0 "$(status_of test -f "$init_dir/bigiron.conf")" rm -rf "$(dirname "$init_dir")" +# --- two sessions on one link ---------------------------------------------------------- + +# `close` tore the master and the tunnel down unconditionally, so two sessions sharing the +# link — a wrapper script and an agent, two agents — each took the other's connection away +# with its own close. Holders: `open` records one, `close` removes its own and tears down +# only when none is left. Stubbed at hs_ssh (the `-O exit`), at hs_master_up and at the VPN +# hooks, so nothing leaves the host. +lease_ctl=$(mktemp -d "${TMPDIR:-/tmp}/hstest.XXXXXX") +lease_log=$(mktemp "${TMPDIR:-/tmp}/hstest.XXXXXX") +hs_real_ssh=$(declare -f hs_ssh) +hs_ssh() { case "$*" in *"-O exit"*) echo "master-exit" >> "$lease_log" ;; esac; return 0; } +hs_master_up() { return 0; } +lease_env() { + HS_HOST=cluster HS_CONTROL_DIR="$lease_ctl" HS_VPN_STATUS_CMD=true HS_VPN_UP_CMD="" \ + HS_VPN_DOWN_CMD="echo vpn-down >> $lease_log" "$@" +} +torn_down() { tr '\n' ' ' < "$lease_log" | sed 's/ $//'; } + +HS_LEASE=alpha lease_env hs_open_session >/dev/null 2>&1 +HS_LEASE=beta lease_env hs_open_session >/dev/null 2>&1 +out=$(HS_LEASE=alpha lease_env hs_close_session 2>&1) +check "a close with another holder keeps the link" 0 "$(status_of test ! -s "$lease_log")" +check_contains "and names who still holds it" "beta" "$out" +check "the closing holder is gone" 1 "$(status_of test -f "$lease_ctl/holders/alpha")" +HS_LEASE=beta lease_env hs_close_session >/dev/null 2>&1 +check "the last close tears the link down" "master-exit vpn-down" "$(torn_down)" + +# A holder named by a pid is only as live as that process. +: > "$lease_log" +sh -c 'exit 0' & dead_pid=$!; wait "$dead_pid" +printf 'pid=%s\nnamed=no\nsince=%s\n' "$dead_pid" "$(date +%s)" > "$lease_ctl/holders/$dead_pid" +HS_LEASE=gamma lease_env hs_close_session >/dev/null 2>&1 +check "a holder whose process ended does not keep the link" "master-exit vpn-down" "$(torn_down)" + +# A named holder outlives its shell, but not HS_LEASE_TTL: the backstop against a session +# that ended without closing. +: > "$lease_log" +printf 'pid=%s\nnamed=yes\nsince=0\n' "$$" > "$lease_ctl/holders/stale" +HS_LEASE=gamma lease_env hs_close_session >/dev/null 2>&1 +check "a holder past HS_LEASE_TTL does not keep the link" "master-exit vpn-down" "$(torn_down)" +: > "$lease_log" +printf 'pid=%s\nnamed=yes\nsince=%s\n' "$$" "$(date +%s)" > "$lease_ctl/holders/fresh" +HS_LEASE=gamma lease_env hs_close_session >/dev/null 2>&1 +check "a fresh named holder does" "" "$(torn_down)" + +# HS_CLOSE_FORCE=1 is the way out when a holder is known to be dead: it closes and clears. +HS_LEASE=alpha HS_CLOSE_FORCE=1 lease_env hs_close_session >/dev/null 2>&1 +check "HS_CLOSE_FORCE=1 closes over a live holder" "master-exit vpn-down" "$(torn_down)" +check "and clears every holder" 1 "$(status_of test -d "$lease_ctl/holders")" + +# A session that only runs commands over a master someone else raised holds it too, and +# status says who does. +HS_LEASE=runner lease_env hs_ensure_open >/dev/null 2>&1 +check "an implicit open registers a holder" 0 "$(status_of test -f "$lease_ctl/holders/runner")" +check_contains "status lists the holders" "holders: runner" \ + "$(HS_TOTP_BACKEND=none lease_env hs_status 2>&1)" + +# The holder's name becomes a file name under the control directory. +out=$(HS_LEASE='../escape' lease_env hs_open_session 2>&1); rc=$? +check "a holder name cannot leave the directory" 1 "$rc" +check "and nothing was written outside it" 1 "$(status_of test -e "$lease_ctl/escape")" + +rm -rf "$lease_ctl" "$lease_log" +unset -f lease_env torn_down +eval "$hs_real_ssh" +restore_lib +. "$HS_ROOT/lib/session.sh" + printf '\n%s passed, %s failed\n' "$PASSED" "$FAILED" [ "$FAILED" -eq 0 ]