mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-22 16:25:58 +00:00
fix(secrets): pass OP_LOAD_DESKTOP_APP_SETTINGS through to the op child env
The 1Password secret source builds a minimal allowlisted environment for the `op read` child process. The allowlist omits OP_LOAD_DESKTOP_APP_SETTINGS, so a user who exports it (shell, .env, or service unit) sees it silently stripped before it reaches `op`. That var is `op`'s documented switch to skip the desktop-app integration probe. When the 1Password desktop app is installed, `op` probes its settings/socket at startup *before* evaluating service-account auth. If the desktop app's group container is wedged (e.g. macOS 'Interrupted system call' on the 1Password group container), that probe blocks with no timeout, so `op read` hangs indefinitely even with a valid OP_SERVICE_ACCOUNT_TOKEN present. Setting OP_LOAD_DESKTOP_APP_SETTINGS=false is the intended escape hatch — but stripping it means it has no effect on exactly the headless boxes that need it. Fix: add OP_LOAD_DESKTOP_APP_SETTINGS to _OP_ENV_ALLOWLIST so the documented var reaches the child. No behavior change when it's unset. Adds a focused test alongside the existing allowlist test. Repro: on a machine with a wedged 1Password desktop container + a valid SA token, `op read` hangs 600s+ without the var and returns in ~4s with it — but only if it actually reaches the op process, which this allowlist entry ensures. Co-authored-by: Minh Nguyen <menhguin@users.noreply.github.com>
This commit is contained in:
parent
a616b9fb0f
commit
fe8c0f7eef
2 changed files with 28 additions and 0 deletions
|
|
@ -98,6 +98,9 @@ _OP_ENV_ALLOWLIST = (
|
|||
"OP_ACCOUNT",
|
||||
"OP_CONNECT_HOST",
|
||||
"OP_CONNECT_TOKEN",
|
||||
# Lets a user skip op's desktop-app integration probe (which can hang with
|
||||
# no timeout on a wedged desktop container) and go straight to token auth.
|
||||
"OP_LOAD_DESKTOP_APP_SETTINGS",
|
||||
)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -216,6 +216,31 @@ def test_fetch_child_env_is_allowlisted(monkeypatch, tmp_path):
|
|||
assert env.get("NO_COLOR") == "1"
|
||||
|
||||
|
||||
def test_fetch_child_env_passes_load_desktop_app_settings(monkeypatch, tmp_path):
|
||||
"""OP_LOAD_DESKTOP_APP_SETTINGS must reach the op child when the user sets it.
|
||||
|
||||
On a headless box with a valid service-account token but a wedged 1Password
|
||||
desktop app, `op read` hangs on the desktop-settings probe unless
|
||||
OP_LOAD_DESKTOP_APP_SETTINGS=false is passed through. It's an allowlisted
|
||||
var, so a user who exports it should see it in the op child env.
|
||||
"""
|
||||
fake_op = tmp_path / "op"
|
||||
fake_op.write_text("")
|
||||
monkeypatch.setenv("OP_SERVICE_ACCOUNT_TOKEN", "ops_tok")
|
||||
monkeypatch.setenv("OP_LOAD_DESKTOP_APP_SETTINGS", "false")
|
||||
captured = {}
|
||||
|
||||
def fake_run(cmd, **kwargs):
|
||||
captured["env"] = kwargs["env"]
|
||||
return _ok("v")
|
||||
|
||||
monkeypatch.setattr(op.subprocess, "run", fake_run)
|
||||
op.fetch_onepassword_secrets(
|
||||
references={"K": "op://V/I/F"}, binary=fake_op, use_cache=False
|
||||
)
|
||||
assert captured["env"].get("OP_LOAD_DESKTOP_APP_SETTINGS") == "false"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Caching
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue