[PATCH b4 0/3] review: add support for b4.pw-key-command

[PATCH b4 0/3] review: add support for b4.pw-key-command

[Brian Masney]

Currently the patchwork token needs to be stored plaintext in your git
configuration. This series adds support for b4.pw-key-command so that
the token can be stored in a password manager.

Signed-off-by: Brian Masney <masneyb@kernel.org>
---
Brian Masney (3):
      review: centralize lookup of patchwork key in one place
      review: introduce support for b4.pw-key-command
      review: tests: add tests for get_pw_key() and b4.pw-key-command

 docs/config.rst                    | 16 ++++++++++++
 src/b4/__init__.py                 | 53 ++++++++++++++++++++++++++++++++++++--
 src/b4/review/_review.py           |  4 +--
 src/b4/review_tui/_common.py       |  2 +-
 src/b4/review_tui/_entry.py        |  2 +-
 src/b4/review_tui/_tracking_app.py |  2 +-
 src/tests/test___init__.py         | 53 ++++++++++++++++++++++++++++++++++++++
 7 files changed, 125 insertions(+), 7 deletions(-)
---
base-commit: 6be7c6139d7734d84afddf989fa2874935b6d85b
change-id: 20261015-pw-key-command-12c3634b2e8f

Best regards,
--  
Brian Masney <masneyb@kernel.org>

[PATCH b4 1/3] review: centralize lookup of patchwork key in one place

[Brian Masney]

Introduce get_pw_key() so that the patchwork key lookup is done in one
place. This is in preparation for adding a pw-key-command configuration
option.

Assisted-by: LLM
Signed-off-by: Brian Masney <masneyb@kernel.org>
---
 src/b4/__init__.py                 | 14 ++++++++++++--
 src/b4/review/_review.py           |  4 ++--
 src/b4/review_tui/_common.py       |  2 +-
 src/b4/review_tui/_entry.py        |  2 +-
 src/b4/review_tui/_tracking_app.py |  2 +-
 5 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/src/b4/__init__.py b/src/b4/__init__.py
index eb04a2af55ee..16b7208f0b09 100644
--- a/src/b4/__init__.py
+++ b/src/b4/__init__.py
@@ -2504,7 +2504,7 @@ class LoreMessage:
     @staticmethod
     def get_patchwork_data_by_msgid(msgid: str) -> Dict[str, str]:
         config = get_main_config()
-        pwkey = str(config.get('pw-key', ''))
+        pwkey = get_pw_key(config)
         pwurl = str(config.get('pw-url', ''))
         pwproj = str(config.get('pw-project', ''))
         if not (pwkey and pwurl and pwproj):
@@ -6402,6 +6402,16 @@ def get_smtp(
     return SMTPConnector(_connect), fromaddr
 
 
+def get_pw_key(config: Optional[ConfigDictT] = None) -> str:
+    """Return the Patchwork API key from b4.pw-key.
+
+    Returns an empty string when no key is configured.
+    """
+    if config is None:
+        config = get_main_config()
+    return str(config.get('pw-key', '') or '')
+
+
 def get_patchwork_session(pwkey: str, pwurl: str) -> Tuple[requests.Session, str]:
     session = requests.session()
     session.headers.update(
@@ -6418,7 +6428,7 @@ def get_patchwork_session(pwkey: str, pwurl: str) -> Tuple[requests.Session, str
 def patchwork_set_state(msgids: List[str], state: str) -> None:
     # Do we have a pw-key defined in config?
     config = get_main_config()
-    pwkey = str(config.get('pw-key', ''))
+    pwkey = get_pw_key(config)
     pwurl = str(config.get('pw-url', ''))
     pwproj = str(config.get('pw-project', ''))
     if not (pwkey and pwurl and pwproj):
diff --git a/src/b4/review/_review.py b/src/b4/review/_review.py
index b69a7f094bc7..c5e4289ea930 100644
--- a/src/b4/review/_review.py
+++ b/src/b4/review/_review.py
@@ -4232,7 +4232,7 @@ def pw_update_series_state(
     Patchwork is not configured), False on failure.
     """
     config = b4.get_main_config()
-    pwkey = str(config.get('pw-key', ''))
+    pwkey = b4.get_pw_key(config)
     pwurl = str(config.get('pw-url', ''))
     if not (pwkey and pwurl):
         # Patchwork not configured, nothing to do
@@ -4270,7 +4270,7 @@ def pw_lookup_series_id(msgid: str) -> Optional[int]:
     configured or does not know the message.
     """
     config = b4.get_main_config()
-    pwkey = str(config.get('pw-key', ''))
+    pwkey = b4.get_pw_key(config)
     pwurl = str(config.get('pw-url', ''))
     pwproj = str(config.get('pw-project', ''))
     if not (pwkey and pwurl and pwproj):
diff --git a/src/b4/review_tui/_common.py b/src/b4/review_tui/_common.py
index 43db8c9358bf..205762956517 100644
--- a/src/b4/review_tui/_common.py
+++ b/src/b4/review_tui/_common.py
@@ -361,7 +361,7 @@ class CheckRunnerMixin:
 
         # Patchwork config for _builtin_patchwork
         config = b4.get_main_config()
-        pwkey = str(config.get('pw-key', ''))
+        pwkey = b4.get_pw_key(config)
         pwurl = str(config.get('pw-url', ''))
 
         # Dump tracking data to a temp file for external scripts.  This is
diff --git a/src/b4/review_tui/_entry.py b/src/b4/review_tui/_entry.py
index 700c5eae0e6a..275d6703949f 100644
--- a/src/b4/review_tui/_entry.py
+++ b/src/b4/review_tui/_entry.py
@@ -151,7 +151,7 @@ def run_tracking_tui(
             if branch_name == TrackingApp.PATCHWORK_SENTINEL:
                 # User pressed [p]atchwork — run PwApp and loop back
                 config = b4.get_main_config()
-                pwkey = str(config.get('pw-key', ''))
+                pwkey = b4.get_pw_key(config)
                 pwurl = str(config.get('pw-url', ''))
                 pwproj = str(config.get('pw-project', ''))
                 if pwkey and pwurl and pwproj:
diff --git a/src/b4/review_tui/_tracking_app.py b/src/b4/review_tui/_tracking_app.py
index 4ae826ee5b9e..75d652149e29 100644
--- a/src/b4/review_tui/_tracking_app.py
+++ b/src/b4/review_tui/_tracking_app.py
@@ -1159,7 +1159,7 @@ class TrackingApp(LoreNodeShutdownMixin, CheckRunnerMixin, App[Optional[str]]):
         self._db_mtime: float = 0.0
         # Detect patchwork configuration
         config = b4.get_main_config()
-        self._pwkey = str(config.get('pw-key', ''))
+        self._pwkey = b4.get_pw_key(config)
         self._pwurl = str(config.get('pw-url', ''))
         self._pwproj = str(config.get('pw-project', ''))
         # Remember last snooze choices within the session

-- 
2.55.0

[PATCH b4 2/3] review: introduce support for b4.pw-key-command

[Brian Masney]

Add support for b4.pw-key-command which is a command to run to obtain
the patchwork API key, so it can be kept in a password manager instead
of being stored in plain text in your git config. The first line of the
command's output is used as the key. The command is not run through a
shell, and it runs at most once per b4 invocation. When set, it takes
precedence over b4.pw-key.

Assisted-by: LLM
Signed-off-by: Brian Masney <masneyb@kernel.org>
---
 docs/config.rst    | 16 ++++++++++++++++
 src/b4/__init__.py | 45 ++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 58 insertions(+), 3 deletions(-)

diff --git a/docs/config.rst b/docs/config.rst
index 593129e8ae70..29f82a76f014 100644
--- a/docs/config.rst
+++ b/docs/config.rst
@@ -990,6 +990,22 @@ integrate your b4 workflow with patchwork.
 
      Default: ``None``
 
+   :term:`b4.pw-key-command`
+     A command to run to obtain the patchwork API key, so it can be kept
+     in a password manager instead of being stored in plain text in your
+     git config. The first line of the command's output is used as the
+     key. The command is not run through a shell, and it runs at most
+     once per b4 invocation. When set, it takes precedence over
+     :term:`b4.pw-key`. For example::
+
+         pw-key-command = pass show patchwork/api-key
+
+     This setting is ignored in the in-tree ``.b4-config`` file.
+
+     Default: ``None``
+
+     .. versionadded:: v0.17
+
    :term:`b4.pw-project`
      The name of the patchwork project, exactly as seen in the URL
      sub-path. For example::
diff --git a/src/b4/__init__.py b/src/b4/__init__.py
index 16b7208f0b09..1014815d4d33 100644
--- a/src/b4/__init__.py
+++ b/src/b4/__init__.py
@@ -392,6 +392,8 @@ LORENODE: Optional[liblore.LoreNode] = None
 URL_SERVERS: Dict[str, Tuple[str, bool]] = dict()
 # Indicates that we've cleaned cache already
 _CACHE_CLEANED = False
+# Patchwork API keys returned by pw-key-command, keyed by the command
+_PW_KEY_CACHE: Dict[str, str] = dict()
 # DNS answers for the life of the process (see _setup_dns_resolver)
 _DNS_CACHE = dns.resolver.LRUCache()
 # Used to track send-email alias replacements
@@ -5000,6 +5002,11 @@ def _setup_main_config(
         'trailer*',
         'pw-*',
     ]
+    # Never take these from an in-tree .b4-config: they run commands
+    # outside of any check flow, so a cloned tree could run anything.
+    wtdeny = [
+        'pw-key-command',
+    ]
     multivals = [
         'keyringsrc',
         'am-perpatch-check-cmd',
@@ -5015,6 +5022,9 @@ def _setup_main_config(
             wtconfig = get_config_from_git(r'b4\..*', multivals=multivals, source=wtcfg)
             logger.debug('wtcfg=%s', wtconfig)
             for key, val in wtconfig.items():
+                if key in wtdeny:
+                    logger.debug('wtcfg: ignoring %s', key)
+                    continue
                 if key in multivals:
                     val = [_val_to_path(topdir, x) for x in val]
                 else:
@@ -6403,13 +6413,42 @@ def get_smtp(
 
 
 def get_pw_key(config: Optional[ConfigDictT] = None) -> str:
-    """Return the Patchwork API key from b4.pw-key.
+    """Return the Patchwork API key.
 
-    Returns an empty string when no key is configured.
+    If b4.pw-key-command is set, run it and use the first line of its
+    output, so the key can live in a password manager instead of the git
+    config.  The result is cached for the life of the process, so the
+    command runs at most once.  Otherwise, fall back to b4.pw-key.
+    Returns an empty string when no key is configured or the command fails.
     """
     if config is None:
         config = get_main_config()
-    return str(config.get('pw-key', '') or '')
+    cmdstr = config.get('pw-key-command')
+    if not cmdstr:
+        return str(config.get('pw-key', '') or '')
+
+    cmdstr = str(cmdstr)
+    if cmdstr in _PW_KEY_CACHE:
+        return _PW_KEY_CACHE[cmdstr]
+
+    pwkey = ''
+    try:
+        cmdargs = shlex.split(cmdstr)
+        logger.debug('Running pw-key-command: %s', cmdargs[0] if cmdargs else '')
+        # Leave stdin and stderr alone so password managers can prompt
+        sp = subprocess.run(cmdargs, stdout=subprocess.PIPE, check=False)
+        if sp.returncode != 0:
+            logger.critical('pw-key-command exited with status %s', sp.returncode)
+        else:
+            lines = sp.stdout.decode(errors='replace').splitlines()
+            pwkey = lines[0].strip() if lines else ''
+            if not pwkey:
+                logger.critical('pw-key-command returned no output')
+    except (OSError, ValueError) as ex:
+        logger.critical('Could not run pw-key-command: %s', ex)
+
+    _PW_KEY_CACHE[cmdstr] = pwkey
+    return pwkey
 
 
 def get_patchwork_session(pwkey: str, pwurl: str) -> Tuple[requests.Session, str]:

-- 
2.55.0

[PATCH b4 3/3] review: tests: add tests for get_pw_key() and b4.pw-key-command

[Brian Masney]

Introduce tests for pw-key and pw-key-command.

Assisted-by: LLM
Signed-off-by: Brian Masney <masneyb@kernel.org>
---
 src/tests/test___init__.py | 53 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/src/tests/test___init__.py b/src/tests/test___init__.py
index 764829d7e756..0eb6e6193b1c 100644
--- a/src/tests/test___init__.py
+++ b/src/tests/test___init__.py
@@ -2924,3 +2924,56 @@ class TestGetAmMessageFromLines:
         lines = payload.decode().splitlines()
         assert 'From the manual, bar() must be called here.' in lines
         assert '>From an old thread.' in lines
+
+
+class TestGetPwKey:
+    """b4.pw-key-command fetches the Patchwork key from a password manager."""
+
+    @pytest.fixture(autouse=True)
+    def _clear_cache(self) -> None:
+        b4._PW_KEY_CACHE.clear()
+
+    def test_plain_key(self) -> None:
+        assert b4.get_pw_key({'pw-key': 'secret'}) == 'secret'
+
+    def test_unset(self) -> None:
+        assert b4.get_pw_key({}) == ''
+
+    def test_command_first_line(self) -> None:
+        """Only the first line counts, as with `pass show`."""
+        config: b4.ConfigDictT = {
+            'pw-key': 'ignored',
+            'pw-key-command': "printf 'fromcmd\\nuser: me\\n'",
+        }
+        assert b4.get_pw_key(config) == 'fromcmd'
+
+    def test_command_runs_once(self, tmp_path: pathlib.Path) -> None:
+        counter = tmp_path / 'count'
+        config: b4.ConfigDictT = {
+            'pw-key-command': f"sh -c 'echo x >> {counter}; echo key'",
+        }
+        assert b4.get_pw_key(config) == 'key'
+        assert b4.get_pw_key(config) == 'key'
+        assert counter.read_text().count('x') == 1
+
+    def test_command_fails(self, caplog: pytest.LogCaptureFixture) -> None:
+        config: b4.ConfigDictT = {'pw-key-command': 'false'}
+        with caplog.at_level(logging.CRITICAL, logger='b4'):
+            assert b4.get_pw_key(config) == ''
+        assert 'exited with status 1' in caplog.text
+
+    def test_command_missing(self, caplog: pytest.LogCaptureFixture) -> None:
+        config: b4.ConfigDictT = {'pw-key-command': '/nonexistent/b4-pw-helper'}
+        with caplog.at_level(logging.CRITICAL, logger='b4'):
+            assert b4.get_pw_key(config) == ''
+        assert 'Could not run pw-key-command' in caplog.text
+
+    def test_ignored_in_worktree_config(self, gitdir: str) -> None:
+        """A cloned tree's .b4-config must not get to run a command."""
+        wtcfg = pathlib.Path(gitdir) / '.b4-config'
+        wtcfg.write_text(
+            '[b4]\n\tpw-key-command = echo stolen\n\tpw-url = https://pw.example.org\n'
+        )
+        b4._setup_main_config(topdir=gitdir)
+        assert b4.MAIN_CONFIG.get('pw-url') == 'https://pw.example.org'
+        assert b4.MAIN_CONFIG.get('pw-key-command') is None

-- 
2.55.0