From fe8c0f7eef1edd447e53ba15d07589a98034c0c8 Mon Sep 17 00:00:00 2001
From: menhguin <17287724+menhguin@users.noreply.github.com>
Date: Sun, 12 Jul 2026 03:37:21 +0800
Subject: [PATCH] fix(secrets): pass OP_LOAD_DESKTOP_APP_SETTINGS through to
 the op child env
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 agent/secret_sources/onepassword.py |  3 +++
 tests/test_onepassword_secrets.py   | 25 +++++++++++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/agent/secret_sources/onepassword.py b/agent/secret_sources/onepassword.py
index c756353311f0..71b064926127 100644
--- a/agent/secret_sources/onepassword.py
+++ b/agent/secret_sources/onepassword.py
@@ -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",
 )
 
 
diff --git a/tests/test_onepassword_secrets.py b/tests/test_onepassword_secrets.py
index 38d33c8ab42e..1d986dcec1ad 100644
--- a/tests/test_onepassword_secrets.py
+++ b/tests/test_onepassword_secrets.py
@@ -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
 # ---------------------------------------------------------------------------