Skip to content

feat(pam tunnel): add --foreground/--background/--run modes + cross-process registry#1993

Merged
idimov-keeper merged 1 commit intoKeeper-Security:releasefrom
msawczynk:feat/pam-tunnel-non-interactive
Apr 30, 2026
Merged

feat(pam tunnel): add --foreground/--background/--run modes + cross-process registry#1993
idimov-keeper merged 1 commit intoKeeper-Security:releasefrom
msawczynk:feat/pam-tunnel-non-interactive

Conversation

@msawczynk
Copy link
Copy Markdown
Contributor

@msawczynk msawczynk commented Apr 24, 2026

Summary

Adds non-interactive tunnel modes for automation (CI/CD, systemd, scripts) and a file-based session registry so `pam tunnel list` / `pam tunnel stop` work across separate Commander processes.

This supersedes #1848 (which had drifted off `release` and accidentally reverted upstream improvements to `tunnel_helpers.py` and `PAMTunnelDiagnoseCommand`). This PR is rebased cleanly on the latest `release` (17.2.15) and the diff is now scoped to only the files this feature actually needs.

`pam tunnel start`

  • `--foreground` — run in current process, exit on Ctrl-C or `--timeout`
  • `--background` — spawn detached child, parent returns immediately
  • `--run ""` — start tunnel, exec command, tear down on exit
  • `--timeout` / `--pid-file` for lifecycle control
  • Mutual-exclusivity guards; auto non-interactive when no TTY

`pam tunnel list` / `pam tunnel stop`

  • Surface tunnels owned by other processes via the file registry
  • Stop sends SIGTERM (Unix) / TerminateProcess (Windows) and cleans the row

New module `keepercommander/commands/tunnel_registry.py`

  • Atomic JSON writes under `/keeper-tunnel-sessions/.json`
  • Stale-entry cleanup, duplicate bind detection (host/port aware)
  • `0o700` dir perms on POSIX

Scope (3 files)

File Δ
`keepercommander/commands/tunnel_and_connections.py` +491 / -79
`keepercommander/commands/tunnel_registry.py` (new) +239
`unit-tests/test_tunnel_registry.py` (new) +253

`tunnel_helpers.py` and `PAMTunnelDiagnoseCommand` are untouched — earlier drafts of this branch unintentionally rolled them back.

Test plan

  • `pytest unit-tests/` — 547 passed, 5 skipped
  • `pytest unit-tests/test_tunnel_registry.py` — 15 passed
  • Manual: `pam tunnel start --background` then `pam tunnel list` from a second Commander shell
  • Manual: `pam tunnel stop ` from a second shell terminates the background process
  • Manual: `pam tunnel start --run "psql ..."` exits cleanly after the wrapped command returns

Note on docs

Documentation for the new flags will be opened separately; this PR is code-only.

Made with Cursor


2026-04-30 - rebased onto Keeper-Security/Commander:release (17.2.16)

Conflicts resolved in keepercommander/commands/tunnel_and_connections.py (3 regions):

  1. Imports: merged unregister_tunnel_session, wait_for_tunnel_connection, create_rust_webrtc_settings, and tunnel_registry imports from this PR into upstream import block.
  2. CLI args: kept both workflow args (--reason, --ticket, --auto-checkout, --wait, --wait-timeout) from Workflow pam launch compat #1997 AND the new mode args (--foreground, --background, --run, --timeout, --pid-file) from this PR.
  3. Execute body: merged workflow lease-expiry handling from Workflow pam launch compat #1997 with the new --run / --foreground / --background paths from this PR - both active under if result and result.get('success'):.

No interference: the two PRs touch orthogonal code paths. Workflow gate fires first (validation + lease), then tunnel mode dispatch runs after successful tunnel start.

@msawczynk
Copy link
Copy Markdown
Contributor Author

@sk-keeper fixed issues with old one. Thanks for the help!

@msawczynk msawczynk force-pushed the feat/pam-tunnel-non-interactive branch from 8d7a604 to 8904792 Compare April 24, 2026 22:55
@msawczynk msawczynk force-pushed the feat/pam-tunnel-non-interactive branch from 8904792 to 1529bdf Compare April 30, 2026 01:49
… registry

Adds non-interactive tunnel modes for automation use cases (CI/CD, systemd,
scripts) and a file-based session registry so `pam tunnel list/stop` work
across Commander processes.

PAMTunnelStartCommand:
- --foreground: run in current process, exit on Ctrl-C or --timeout
- --background: spawn detached child, parent returns immediately
- --run "<cmd>": start tunnel, run command, tear down on exit
- --timeout / --pid-file for lifecycle control
- Mutual-exclusivity checks; safe defaults when no TTY (batch mode)

PAMTunnelListCommand / PAMTunnelStopCommand:
- Surface tunnels owned by other processes via the file registry
- Stop sends SIGTERM (Unix) / TerminateProcess (Windows) and cleans the row

New keepercommander/commands/tunnel_registry.py:
- Atomic JSON writes under <tmp>/keeper-tunnel-sessions/<pid>.json
- Stale-entry cleanup, duplicate bind detection (host/port aware)
- 0o700 dir perms on POSIX

Made-with: Cursor
@msawczynk msawczynk force-pushed the feat/pam-tunnel-non-interactive branch from 1529bdf to b2f2229 Compare April 30, 2026 07:33
@msawczynk
Copy link
Copy Markdown
Contributor Author

Reviewed merged code against test doc - found one real interference, fixed.

Bug: pam tunnel start --foreground + workflow lease.
When the workflow lease expired, the lease-expiry timer printed the warning but had no way to wake the foreground process - _close_on_lease_expiry was defined before fg_shutdown = threading.Event(), so the closure could not signal it. Result: process hung on fg_shutdown.wait() past lease expiry until external SIGTERM/Ctrl+C. Wrong behaviour for systemd / script use - the workflow lease is the contract, the process should self-terminate.

Fix (amended into the rebase commit, force-pushed):

  • Module-level _LEASE_SHUTDOWN_EVENTS_BY_RECORD: dict[str, threading.Event].
  • _close_on_lease_expiry now reads that dict and .set()s the event if registered.
  • --foreground block registers fg_shutdown under record_uid before fg_shutdown.wait() and pops it in the finally.
  • Default interactive mode does NOT register (no blocking wait to break - user keeps SSH session until natural disconnect, matches doc test 3 expectation).

Other paths verified clean:

  • --background: early-return at line 809 before success branch -> never sees lease timer.
  • --run: subprocess runs to completion; lease warning fires; subprocess decides. Long-running --run past lease expiry is a known limitation (would need subprocess.Popen + poll loop to interrupt cleanly - left as follow-up).
  • Default interactive (no flag): unchanged - lease warning prints, user controls SSH.

Doc test scenarios re-checked: tests 1-18 all pass against the fixed code. Test 3 ('Lease expiry, tunnel path') still describes the soft-disconnect for the default interactive path - --foreground would now self-terminate at lease expiry, which is the correct behaviour for non-interactive automation.

@idimov-keeper idimov-keeper merged commit ae7e254 into Keeper-Security:release Apr 30, 2026
4 checks passed
sk-keeper pushed a commit that referenced this pull request May 1, 2026
… registry (#1993)

Adds non-interactive tunnel modes for automation use cases (CI/CD, systemd,
scripts) and a file-based session registry so `pam tunnel list/stop` work
across Commander processes.

PAMTunnelStartCommand:
- --foreground: run in current process, exit on Ctrl-C or --timeout
- --background: spawn detached child, parent returns immediately
- --run "<cmd>": start tunnel, run command, tear down on exit
- --timeout / --pid-file for lifecycle control
- Mutual-exclusivity checks; safe defaults when no TTY (batch mode)

PAMTunnelListCommand / PAMTunnelStopCommand:
- Surface tunnels owned by other processes via the file registry
- Stop sends SIGTERM (Unix) / TerminateProcess (Windows) and cleans the row

New keepercommander/commands/tunnel_registry.py:
- Atomic JSON writes under <tmp>/keeper-tunnel-sessions/<pid>.json
- Stale-entry cleanup, duplicate bind detection (host/port aware)
- 0o700 dir perms on POSIX
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.

2 participants