Harden Podman backend and verify GPU passthrough end-to-end

[mkitti]

Summary

• Ports JaneliaScientificComputingSystems/agentic-sandbox's battle-tested Podman wrapper hardening onto this repo's Podman scripts: per-job storage isolation (concurrent jobs on one GPU node no longer corrupt each other's storage), staleness recovery after a node reboot, a catatonit orphan watchdog, exit-code-safe cleanup traps, and an opt-in --allow network egress allowlist.
• Adapted rather than copied verbatim: this cluster has no /etc/subuid ranges, so per-job storage cleanup needed podman unshare rm -rf instead of a plain rm -rf (confirmed live -- a plain rm -rf hit Permission denied on every file).
• Adds a marimo-podman entry to runnables.yaml (mirroring the existing Apptainer marimo runnable), since Podman is the backend preferred by HPC admins.
• Adds a marimo-podman-https runnable, plus a BACKEND=apptainer|podman override in container/https-wrap.sh -- the HTTPS wrapper's backend auto-detection previously always preferred Apptainer when present, with no way to get Podman+HTTPS on a host that has both installed.
• Fixes stale script-name references in README.md left over from an earlier container/{apptainer,podman}/ reorganization.

Verification

Verified end-to-end on a real gpu_l4 LSF allocation:

• Podman's NVIDIA CDI GPU passthrough (previously "blocked in testing" per the old README) -- nvidia-smi -L inside the container now returns a real GPU UUID.
• Two concurrent GPU sessions on the same node, no storage corruption, correct distinct exit codes.
• The egress allowlist against the real published image: allowed host -> 200, unlisted host -> 403 (logged by the proxy).
• Default (no new flags) path regression-tested against the real image -- unchanged behavior.

One caveat documented transparently in the README rather than overclaiming: an orphaned catatonit occasionally survived the watchdog in testing (observed once), matching a known low-frequency residual risk agentic-sandbox's own investigation documents, not something this port introduced.

Test plan

• bash -n on all modified/new shell scripts
• python3 -m py_compile on the vendored allowlist_proxy.py/relay.py
• python3 -c "import yaml; yaml.safe_load(...)" on runnables.yaml
• Real gpu_l4 LSF job: GPU passthrough, concurrency, and allowlist tests (see above)
• Regression test of marimo.sh/shell.sh with no new flags against the real published image
• BACKEND=apptainer|podman|""|bogus override logic exercised directly (correct task selection, clear error on invalid value)

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR hardens the Podman container backend (storage isolation, staleness recovery, watchdog, and opt-in egress allowlisting) and wires it into Fileglancer runnables, while also adding a BACKEND=apptainer|podman override to enable Podman+HTTPS on hosts that have both backends installed.

Changes:

• Add new Fileglancer services marimo-podman and marimo-podman-https, including --allow support for opt-in egress allowlisting.
• Refactor Podman scripts to use a shared container/podman/lib.sh for per-job storage isolation, cleanup, and a catatonit watchdog.
• Add backend override selection to container/https-wrap.sh and update README paths/examples after the container script reorg.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
runnables.yaml	Adds Podman and Podman+HTTPS service runnables with parameters including --allow.
README.md	Updates repo layout/docs; documents Podman CDI verification, storage isolation, and allowlist behavior.
container/podman/shell.sh	Switches to lib-based storage/network setup and watched podman run for interactive sessions.
container/podman/relay.py	Adds in-container TCP↔UDS relay used for the egress allowlist proxy pattern.
container/podman/marimo.sh	Switches to lib-based storage/network setup and watched podman run, plus allowlist support.
container/podman/lib.sh	Introduces Podman hardening helpers: shared storage config, per-job isolation, watchdog, allowlist wiring.
container/podman/build.sh	Refactors build to use lib-based shared storage setup.
container/podman/allowlist_proxy.py	Adds host-side allowlist forward proxy for HTTP/HTTPS (CONNECT) over a Unix socket.
container/https-wrap.sh	Adds `BACKEND=apptainer
container/common.sh	Adds ALLOW_HOSTS parsing and --allow flag support passed through to Podman scripts.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[mkitti]

Addressed all 5 Copilot review comments in 8133931:

1. Misleading doc comment (container/common.sh) — fixed to point at container/podman/lib.sh where podman_network_setup actually lives.
2. --allow unconditional shift 2 (container/common.sh) — now errors clearly ("--allow requires a hostname argument") and exits instead of hitting a generic "shift count out of range" under set -e.
3. mktemp -u race (container/podman/lib.sh) — switched to mktemp -d for a private (0700) temp directory with the socket inside it, plus fail-fast (clear error + the proxy's own log printed) if allowlist_proxy.py dies immediately instead of silently continuing with a broken "allowlist".
4. Cleanup left /tmp clutter — podman_network_cleanup now removes the whole per-invocation directory (socket + log), not just the socket file.
5. HTTP CONNECT-adjacent plain-HTTP port bug (container/podman/allowlist_proxy.py) — the non-CONNECT path always connected to port 80 regardless of the Host: header's port; now parses and honors it (verified with a unit test: Host: example.com:8080 now connects to 8080, not 80).

All four fixes verified live (proxy directory permissions/cleanup, --allow error path, unit-tested port parsing) — see the commit message for details. Also carried these fixes forward through the rest of the stack (#12–#14), since #13 had already refactored this logic into shared common.sh functions used by both backends — the fix now protects Apptainer's allowlist path too, not just Podman's.

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

README.md:216

• README still references start.sh/shell.sh, but those top-level scripts no longer exist after the container/{apptainer,podman}/ reorg. This can confuse users following the read-only/autofs guidance; point to the current pixi run marimo/pixi run shell entrypoints instead.

> `/groups/scicompsoft`. `start.sh`/`shell.sh` **default `RO_PATHS` to your lab
> dirs** (`/groups/scicompsoft`, `/nrs/scicompsoft`) and **refuse** bare autofs
> parents. Set your own with `RO_PATHS="/groups/<lab> /nrs/<lab> ..."`, or

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Suppressed comments (6)

Previously missed (4) — in code that hasn't changed since the last review.

container/podman/lib.sh:36

• The Podman shared storage directories are created with the caller's umask, which can leave them world-readable/executable (e.g. 755). Since these directories can contain pulled layers and runtime metadata, it's safer to force restrictive permissions (700) after creation.

This issue also appears on line 78 of the same file.

    PODMAN_STORAGE_ROOT="${PODMAN_STORAGE_ROOT:-/scratch/$(id -un)/podman-storage}"
    PODMAN_RUN_ROOT="${PODMAN_RUN_ROOT:-/tmp/podman-run-$(id -u)/run}"
    mkdir -p "$PODMAN_STORAGE_ROOT" "$PODMAN_RUN_ROOT"

container/podman/lib.sh:140

• With set -e enabled in the callers, the grep ... && kill ... construct can cause the whole script to exit if kill -9 races and returns non-zero (the kill is the final command in an && list). Make the kill best-effort so the watchdog never turns a benign race into a hard failure.

This issue also appears on line 178 of the same file.

    for pid in $(pgrep -u "$USER" -x catatonit 2>/dev/null); do
        [[ "$(ps -o ppid= -p "$pid" 2>/dev/null | tr -d ' ')" == "1" ]] || continue
        grep -q "$job_dir" "/proc/$pid/mountinfo" 2>/dev/null && kill -9 "$pid" 2>/dev/null
    done

container/podman/allowlist_proxy.py:135

• host_allowed() lowercases the incoming host, but the allowlist values are not normalized. If a user passes an uppercase hostname (or mixed-case), it will be denied unexpectedly. Normalize the allowlist entries to lowercase once in main().

    sock_path = sys.argv[1]
    allowlist = sys.argv[2:]
    print(f"[allowlist_proxy] listening on {sock_path}, allowlist={allowlist}", file=sys.stderr)

container/podman/lib.sh:270

• podman_network_cleanup is invoked from EXIT traps in scripts that run with set -e. If the proxy process already exited, kill "$PODMAN_PROXY_PID" can return non-zero and cause the cleanup trap to fail/override the script's exit status. Make the kill best-effort.

podman_network_cleanup() {
    [[ -n "${PODMAN_PROXY_PID:-}" ]] && kill "$PODMAN_PROXY_PID" 2>/dev/null
    [[ -n "${PODMAN_PROXY_DIR:-}" && -e "$PODMAN_PROXY_DIR" ]] && rm -rf "$PODMAN_PROXY_DIR"
    return 0

container/podman/lib.sh:81

• Per-job storage directories are also created with default permissions, which can expose job-local container metadata to other users on a shared /scratch. Consider forcing 700 on the job directory and its root/run subdirs.

    local jobtag="${LSB_JOBID:-$$-$RANDOM}"
    PODMAN_JOB_STORAGE_DIR="/scratch/$(id -un)/podman-jobs/$jobtag"
    mkdir -p "$PODMAN_JOB_STORAGE_DIR/root" "$PODMAN_JOB_STORAGE_DIR/run"
    PODMAN_GLOBAL_ARGS=(

container/podman/lib.sh:182

• podman_run_watched runs under set -e (inherited from the caller). If the watchdog process exits quickly, kill "$watchdog_pid" can return non-zero (no such process) and prematurely abort the script, potentially masking the real podman run exit code. Make the kill best-effort.

    local exit_code=0
    wait "$podman_pid" || exit_code=$?
    kill "$watchdog_pid" 2>/dev/null
    wait "$watchdog_pid" 2>/dev/null || true
    _podman_kill_orphaned_catatonit "$job_dir"