Skip to content

Commit 1f3a204

Browse files
ammachadoclaude
andcommitted
Address PR psss#469 review from @kwk
- List `token_command` in the gitlab and jira config examples, next to the `token` and `token_file` entries that were already there - Clarify that the token command inherits did's environment (so `BW_SESSION` and friends work) and only loses shell expansion - Include the command line in the `FileNotFoundError` and `CalledProcessError` messages, matching the timeout message - Document why caching the token for the process lifetime does not widen exposure of the secret - Reword a test comment that read as a typo Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 4cd37b4 commit 1f3a204

4 files changed

Lines changed: 25 additions & 13 deletions

File tree

‎did/base.py‎

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -554,16 +554,22 @@ def _run_token_command(command: str) -> str:
554554
"""
555555
Run `command` and return its stripped stdout.
556556
557-
The command line is parsed with `shlex.split` and executed without
558-
a shell, so config files cannot inject shell metacharacters. A
559-
30-second timeout protects against secret managers that block on
560-
interactive prompts (e.g. an expired ``op`` session). Non-zero
561-
exit, missing binary or timeout each raise `ConfigError`; stdout
562-
is never logged because it is the secret.
563-
564-
Results are memoized for the lifetime of the process so that
557+
The command line is parsed with `shlex.split` and executed
558+
directly, without a shell, so config files cannot inject shell
559+
metacharacters. The command inherits the environment of `did`
560+
(so variables such as ``BW_SESSION`` are visible to it) but gets
561+
no shell expansion. A 30-second timeout protects against secret
562+
managers that block on interactive prompts (e.g. an expired ``op``
563+
session). Non-zero exit, missing binary or timeout each raise
564+
`ConfigError`; stdout is never logged because it is the secret.
565+
566+
Results are cached for the lifetime of the process so that
565567
multiple config sections sharing the same command string only
566-
invoke the external tool once per run. Failures are not cached.
568+
invoke the external tool once per run. This does not widen the
569+
exposure of the secret: `did` is a short-lived command and the
570+
token is already held in memory by every caller of `get_token`.
571+
Nothing is written to disk and the cache dies with the process.
572+
Failures are not cached, so a transient error can be retried.
567573
"""
568574
try:
569575
result = subprocess.run(
@@ -573,14 +579,14 @@ def _run_token_command(command: str) -> str:
573579
)
574580
except FileNotFoundError as exc:
575581
raise ConfigError(
576-
f"Token command not found: {exc.filename}") from exc
582+
f"Token command not found: {exc.filename}: {command}") from exc
577583
except subprocess.TimeoutExpired as exc:
578584
raise ConfigError(
579585
f"Token command timed out after {exc.timeout}s: {command}"
580586
) from exc
581587
except subprocess.CalledProcessError as exc:
582588
raise ConfigError(
583-
f"Token command failed (exit {exc.returncode}): "
589+
f"Token command failed (exit {exc.returncode}): {command}: "
584590
f"{exc.stderr.strip()}") from exc
585591
return result.stdout.strip()
586592

‎did/plugins/gitlab.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
url = https://gitlab.com/
99
token = <authentication-token>
1010
token_file = <authentication-token-file>
11+
token_command = <command-to-fetch-token>
1112
login = <username>
1213
ssl_verify = true
1314

‎did/plugins/jira.py‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
url = https://issues.redhat.com/
1010
auth_type = token
1111
token_file = ~/.did/jira_api_token
12+
token_command = <command-to-fetch-token>
1213
token_expiration = 7
1314
token_name = did-token
1415
@@ -27,7 +28,11 @@
2728
Shell-style command line whose stdout is used as the token, e.g.
2829
``bw get password did-jira`` or
2930
``op read op://Personal/Jira/token``. The command is parsed with
30-
``shlex`` and executed without a shell.
31+
``shlex`` and executed directly, without a shell. It inherits the
32+
environment of ``did``, so variables such as ``BW_SESSION`` are
33+
visible to it, but there is no shell expansion: pipes, redirects,
34+
``$VAR`` substitution and globs are not interpreted. Wrap the
35+
command in ``sh -c '...'`` if you need any of those.
3136
3237
token_expiration
3338
Print warning if token with provided ``token_name`` expires within

‎tests/unit/test_base.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -306,7 +306,7 @@ class TestGetToken(unittest.TestCase):
306306
""" Tests for the `get_token` function """
307307

308308
def setUp(self) -> None:
309-
# Clear the per-process token-command cache so memoized results
309+
# Clear the per-process token-command cache so cached results
310310
# from previous tests do not bleed into this one.
311311
# pylint: disable=protected-access
312312
did.base._run_token_command.cache_clear()

0 commit comments

Comments
 (0)