From 1b21c36efe14e68b46081ca5c8040ed2e9f64fd7 Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Sun, 18 Oct 2026 12:40:45 +0200 Subject: [PATCH 1/6] feat(secret-prompt): ask the release manager for an upload token without exposing it Release steps such as the PyPI upload of convenience packages need a credential only the release manager holds. When an agent runs the step, or a script with no terminal, the usual "read -s" prompt has nowhere to go, so the token ends up in a file or on a command line, and from there in shell history, ps or the agent's transcript. secret-prompt asks for the secret in the terminal when there is one, and otherwise as a macOS osascript dialog, a zenity/kdialog dialog on a Linux desktop, or pinentry. It then execs one command with the secret in one environment variable. Every prompt names that command, so the release manager can see where the secret goes before pasting it; a --prefix check rejects a wrong paste, and a cancelled prompt runs nothing. A backend that cannot ask falls through to the next one. The release-build template, the promote skill's convenience-artefacts reference and the release-management spec now recommend declaring credentialed stage/publish commands wrapped in it. Generated-by: Claude Code (Opus 5) --- .github/labeler.yml | 1 + docs/labels-and-capabilities.md | 3 +- docs/vendor-neutrality.md | 5 +- .../skills/promote/convenience-artefacts.md | 4 + .../magpie-setup/templates/release-build.md | 7 + pyproject.toml | 1 + tools/secret-prompt/README.md | 128 +++++++ tools/secret-prompt/pyproject.toml | 88 +++++ .../src/secret_prompt/__init__.py | 254 +++++++++++++ .../src/secret_prompt/__main__.py | 22 ++ .../secret-prompt/tests/test_secret_prompt.py | 345 ++++++++++++++++++ .../specs/release-management-lifecycle.md | 6 + uv.lock | 16 + 13 files changed, 877 insertions(+), 3 deletions(-) create mode 100644 tools/secret-prompt/README.md create mode 100644 tools/secret-prompt/pyproject.toml create mode 100644 tools/secret-prompt/src/secret_prompt/__init__.py create mode 100644 tools/secret-prompt/src/secret_prompt/__main__.py create mode 100644 tools/secret-prompt/tests/test_secret_prompt.py diff --git a/.github/labeler.yml b/.github/labeler.yml index a8fc3a09f..fba5af8f5 100644 --- a/.github/labeler.yml +++ b/.github/labeler.yml @@ -619,6 +619,7 @@ substrate:release: - 'tools/release-management/**' - 'tools/release-verify/**' - 'tools/reproducible-archive/**' + - 'tools/secret-prompt/**' substrate:review: - any: diff --git a/docs/labels-and-capabilities.md b/docs/labels-and-capabilities.md index 493af9d2b..8c9ec82b8 100644 --- a/docs/labels-and-capabilities.md +++ b/docs/labels-and-capabilities.md @@ -151,7 +151,7 @@ framework substrate: | `substrate:action-guard` | substrate | Deterministic pre-tool-use command guards. | | `substrate:privacy` | substrate | PII redaction / approved-LLM gating. | | `substrate:framework-dev` | substrate | Build / validate / eval the framework itself. | -| `substrate:release` | substrate | Release-artefact helpers an adopter's release process runs: reproducible-archive build, lint and comparison; maven-artifact-verify for staged JVM artefacts; release-config loading and pre-flight checks; release-verify checks of a staged release candidate; release-management rendering of the paste-ready release blocks. | +| `substrate:release` | substrate | Release-artefact helpers an adopter's release process runs: reproducible-archive build, lint and comparison; maven-artifact-verify for staged JVM artefacts; release-config loading and pre-flight checks; release-verify checks of a staged release candidate; release-management rendering of the paste-ready release blocks; secret-prompt for the release manager's upload credentials. | | `substrate:setup` | substrate | Adopter-side setup state an agent resolves at runtime rather than at development time: lock parsing, floor comparison, reconciliation fingerprints. | | `substrate:review` | substrate | A second model's read of a change before it is published: other models' CLIs run read-only, findings merged. | @@ -378,6 +378,7 @@ or a contract-free mix of substrates (e.g. `tools/spec-inventory` is | [`tools/reproducible-archive`](../tools/reproducible-archive/) | `substrate:release` | `repro-archive` — build, lint and compare reproducible source archives from a git ref (`git archive` + `.gitattributes` `export-ignore`, every reproducible-builds.org archive rule applied); used by `release-rc-cut` Step 2/2b and `release-verify-rc` Step 9 | | [`tools/maven-artifact-verify`](../tools/maven-artifact-verify/) | `substrate:release` | `maven-artifact-verify` — verify locally staged JVM RC artefacts: POM ALv2 licence / developers / scm (with parent-POM inheritance resolution), podling incubation disclaimer in ``, companion `-sources.jar` / `-javadoc.jar` presence with their own signatures and checksums; used by `release-verify-rc` Step 6b | | [`tools/release-verify`](../tools/release-verify/) | `substrate:release` | `release-verify` — deterministic, read-only checks of a staged RC (inventory, signatures against the project KEYS only, checksums, NOTICE/LICENSE, prohibited binaries, dangling symlinks, version strings) and the verdict roll-up, each as JSON; used by `release-verify-rc` Steps 1–3, 5–8 and 10; and `list-keys`, the fingerprints and UIDs of a KEYS file or fetched key, for `release-keys-sync` Steps 0–1 | +| [`tools/secret-prompt`](../tools/secret-prompt/) | `substrate:release` | `secret-prompt` — asks the release manager for a secret (terminal, macOS dialog, zenity/kdialog, pinentry) and runs one command with it in one environment variable, so an upload token never reaches the shell history, a file or an agent's transcript | | [`tools/symlink-lint`](../tools/symlink-lint/) | `substrate:framework-dev` | Self-adoption symlink hygiene — rejects cyclic symlinks, misdirected skill relays (canonical/relay target-correctness), and incomplete self-adoption symlink sets | | [`tools/pilot-report-validator`](../tools/pilot-report-validator/) | `substrate:framework-dev` | Adopter pilot-report validator — required frontmatter keys, no unfilled placeholders, valid profile, and required body sections; counterpart to `spec-validator` for `docs/pilot-report-template.md` | | [`tools/skill-reconciler-diff`](../tools/skill-reconciler-diff/) | `substrate:framework-dev` | Deterministic structural diff between two skill trees — parses frontmatter, section headings, step inventory, placeholders, support files, and safety-baseline clauses into a JSON diff object for the `skill-reconciler` skill | diff --git a/docs/vendor-neutrality.md b/docs/vendor-neutrality.md index 0191bf481..2c827d469 100644 --- a/docs/vendor-neutrality.md +++ b/docs/vendor-neutrality.md @@ -615,7 +615,7 @@ Organization scope (declared, orthogonal to vendor): ASF = 16, agnostic = 64. **LLM / agent-integration neutrality** -**Agent harness: 34/34 substrate tools run under any harness unchanged (100%).** Substrate tools are Magpie's own machinery; each declares the agent harness it integrates with (`**Harness:**`), or `agnostic`. A tool is neutral when it is harness-agnostic or supports two or more harnesses; *coupled* when it targets a single harness. +**Agent harness: 35/35 substrate tools run under any harness unchanged (100%).** Substrate tools are Magpie's own machinery; each declares the agent harness it integrates with (`**Harness:**`), or `agnostic`. A tool is neutral when it is harness-agnostic or supports two or more harnesses; *coupled* when it targets a single harness. | Substrate tool | Substrate | Harness support | Verdict | |---|---|---|---| @@ -639,6 +639,7 @@ Organization scope (declared, orthogonal to vendor): ASF = 16, agnostic = 64. | `release-verify` | release | any | ✅ agnostic | | `reproducible-archive` | release | any | ✅ agnostic | | `sandbox-lint` | sandbox | Claude Code, Codex, Cursor, Gemini CLI, Grok, Kiro, OpenCode | ✅ portable | +| `secret-prompt` | release | any | ✅ agnostic | | `security-management` | analytics | any | ✅ agnostic | | `security-tracker-stats-dashboard` | analytics | any | ✅ agnostic | | `setup-preflight` | setup | any | ✅ agnostic | @@ -665,7 +666,7 @@ Harness → substrate tools it supports: - **Junie** (1): `agent-guard` - **Kiro** (3): `agent-guard`, `sandbox-lint`, `spec-loop` - **OpenCode** (3): `agent-guard`, `sandbox-lint`, `spec-loop` -- **any harness** (31): `adversarial-review`, `agent-isolation`, `container-gateway`, `contributor-metrics`, `dashboard-generator`, `dev`, `egress-gateway`, `maven-artifact-verify`, `permission-audit`, `pilot-report-validator`, `pr-management`, `preflight-audit`, `privacy-llm`, `probe-templates`, `release-config`, `release-management`, `release-verify`, `reproducible-archive`, `security-management`, `security-tracker-stats-dashboard`, `setup-preflight`, `skill-and-tool-validator`, `skill-evals`, `skill-reconciler-diff`, `skill-token-count`, `spec-inventory`, `spec-status-index`, `spec-validator`, `symlink-lint`, `vendor-neutrality-score`, `vetted-ops` +- **any harness** (32): `adversarial-review`, `agent-isolation`, `container-gateway`, `contributor-metrics`, `dashboard-generator`, `dev`, `egress-gateway`, `maven-artifact-verify`, `permission-audit`, `pilot-report-validator`, `pr-management`, `preflight-audit`, `privacy-llm`, `probe-templates`, `release-config`, `release-management`, `release-verify`, `reproducible-archive`, `secret-prompt`, `security-management`, `security-tracker-stats-dashboard`, `setup-preflight`, `skill-and-tool-validator`, `skill-evals`, `skill-reconciler-diff`, `skill-token-count`, `spec-inventory`, `spec-status-index`, `spec-validator`, `symlink-lint`, `vendor-neutrality-score`, `vetted-ops` **Model endpoint: neutral by construction — 4 default-approved endpoint classes across independent trust domains, plus adopter opt-in.** From the [`privacy-llm` registry](../tools/privacy-llm/models.md): the framework keys approval on *endpoint identity*, not on who hosts the model, so no single LLM vendor is privileged. diff --git a/plugins/magpie-release-management/skills/promote/convenience-artefacts.md b/plugins/magpie-release-management/skills/promote/convenience-artefacts.md index aae5bf456..fa23e5686 100644 --- a/plugins/magpie-release-management/skills/promote/convenience-artefacts.md +++ b/plugins/magpie-release-management/skills/promote/convenience-artefacts.md @@ -12,6 +12,10 @@ The render emits it **after** the dist promotion and the final tag, as its own s (for example `twine upload dist/apache_-*`, `mvn nexus-staging:release -DstagingRepositoryId=`, `docker push /:`, `helm push …`). These are the project's own commands; the skill never invents a channel or a command the config does not declare. An entry with no `publish_command` emits nothing and comes back in `warnings`. + A `publish_command` that uploads with the release manager's token is best declared wrapped in + [`secret-prompt`](../../../../tools/secret-prompt/README.md) (`secret-prompt --env TWINE_PASSWORD --set TWINE_USERNAME=__token__ --prefix pypi- -- twine upload …`), + so running it asks the release manager for the token (the tool's README has the one-time `uv tool install`); + the render still emits it verbatim, and never asks for or handles the token itself. **Reproducibility gate — the artefact must be good before it is published.** A convenience artefact is publishable only if the `release-verify-rc` run recorded on the planning issue rebuilt it from the voted tag and it reproduced: diff --git a/plugins/magpie-setup/templates/release-build.md b/plugins/magpie-setup/templates/release-build.md index 03f4a0c7a..29d2944c4 100644 --- a/plugins/magpie-setup/templates/release-build.md +++ b/plugins/magpie-setup/templates/release-build.md @@ -133,6 +133,13 @@ convenience_artefacts: publish_command: null # run by the RM after the vote, e.g. `twine upload dist/*`, `mvn nexus-staging:release`, `docker push …`; dist-release needs none (promoted with the source) ``` +A `stage_command` or `publish_command` that needs a credential the release manager holds — an upload token — +is best declared wrapped in [`secret-prompt`](https://github.com/apache/magpie/tree/main/tools/secret-prompt), so running it asks the release manager +for the token (in their terminal, or as a dialog when an agent runs it) instead of reading one from a file: +`secret-prompt --env TWINE_PASSWORD --set TWINE_USERNAME=__token__ --prefix pypi- -- twine upload dist/*` +(the release manager installs it once with +`uv tool install "git+https://github.com/apache/magpie#subdirectory=tools/secret-prompt"`). + How each skill uses the list: | Skill | Uses | diff --git a/pyproject.toml b/pyproject.toml index e8da22bce..3309405d7 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -131,6 +131,7 @@ members = [ "tools/cve-tool-vulnogram/oauth-api", "tools/github-body-field", "tools/github-rollup", + "tools/secret-prompt", "tools/gmail/oauth-draft", "tools/jira", "tools/maildir", diff --git a/tools/secret-prompt/README.md b/tools/secret-prompt/README.md new file mode 100644 index 000000000..5daadb021 --- /dev/null +++ b/tools/secret-prompt/README.md @@ -0,0 +1,128 @@ + + + + + +- [`secret-prompt`](#secret-prompt) + - [Prerequisites](#prerequisites) + - [How to use](#how-to-use) + - [How it asks](#how-it-asks) + - [What it guarantees](#what-it-guarantees) + - [Tests](#tests) + + + +# `secret-prompt` + +**Capability:** substrate:release + +**Harness:** agnostic + +Asks the operator for a secret, such as a single-use PyPI token, and runs +**one command** with it in **one environment variable**. The secret goes from +the prompt into that command's environment and nowhere else: it is never +printed, written to a file, or passed on a command line, so it stays out of +the shell history, `ps`, CI logs, and an agent's transcript. + +It exists for the release steps that need a credential the release manager +holds — uploading the convenience packages to PyPI being the common one — +when the step is run by an agent on the release manager's behalf, or from a +script that has no terminal to prompt on. The release manager still types +or pastes the secret themselves, after checking the command the prompt names; +the agent only ever sees that command's output. + +## Prerequisites + +- **Runtime** — Python 3.11+ (stdlib only), run via `uv`. +- **CLIs** — one way to ask: a terminal, or `osascript` (macOS, built in), + `zenity` or `kdialog` (Linux desktop), or `pinentry` (any OS with GnuPG, + including `pinentry-mac`). +- **Credentials / auth** — None of its own; it carries the one secret the + operator enters. +- **Network** — None; the command it runs may need some. + +## How to use + +```bash +secret-prompt --env TWINE_PASSWORD --set TWINE_USERNAME=__token__ --prefix pypi- \ + --title "PyPI upload" --prompt "Paste the single-use PyPI token" \ + -- twine upload dist/*.whl dist/*.tar.gz +``` + +Install it once, so the bare `secret-prompt` in a release recipe resolves: + +```bash +uv tool install "git+https://github.com/apache/magpie#subdirectory=tools/secret-prompt" +``` + +Or run it without installing, from a checkout of this repository or straight from GitHub: + +```bash +uv run --directory tools/secret-prompt --project . secret-prompt --env … -- +uvx --from "git+https://github.com/apache/magpie#subdirectory=tools/secret-prompt" \ + secret-prompt --env … -- +``` + +| Option | Meaning | +| --- | --- | +| `--env NAME` | Environment variable that receives the secret (required). | +| `--set NAME=VALUE` | A non-secret variable for the command too, repeatable (`TWINE_USERNAME=__token__`). | +| `--prefix TEXT` | Refuse a secret that does not start with `TEXT` (`pypi-`), so a wrong paste runs nothing. | +| `--prompt` / `--title` | Text of the prompt and dialog title. | +| `--backend` | `auto` (default), or force `tty`, `osascript`, `zenity`, `kdialog`, `pinentry`. | + +Exit status is the command's own; `1` when no usable secret arrived (cancelled, +empty, the wrong prefix, or no backend could ask), `127` when the command cannot +be started. + +## How it asks + +`--backend auto` uses the first that applies: + +1. **A terminal** (`/dev/tty` can be opened) — a hidden prompt there, even when + stdin is redirected. +2. **macOS** — an `osascript` dialog with a hidden-answer field. The prompt text + reaches AppleScript as an argument, never spliced into the script. +3. **A Linux desktop** (`DISPLAY` or `WAYLAND_DISPLAY` set) — `zenity --password`, + else `kdialog --password`. +4. **`pinentry`** — the Assuan `GETPIN` exchange, which covers `pinentry-mac`, + `pinentry-gnome3`, `pinentry-qt` and the curses ones. + +When a backend *fails* rather than being cancelled — its program is missing, or +it cannot show a dialog — `auto` moves on to the next one and, if none can ask, +reports why. A **cancel** (the Cancel button, `Esc`, or `Ctrl-C`) stops there: +nothing else is asked and nothing runs. + +An agent's command has no terminal, so on the release manager's Mac it gets +the dialog; the same command in the release manager's own terminal prompts +there instead. A **sandboxed** agent cannot show the macOS dialog — the +Seatbelt sandbox turns off Apple Events — so run the step outside the sandbox +(it is the release manager's upload either way), or let it fall back to +`pinentry`. + +## What it guarantees + +- The secret is passed only in the environment of the one command it runs + (`exec`, so that command replaces the tool and keeps its exit status). +- Nothing runs unless a non-empty secret arrives that matches `--prefix`; a + cancelled dialog runs nothing. +- Error messages name the expected prefix, never what was entered. +- Every prompt ends with **`It will be passed to: `**, the exact command + that will receive the secret, and the caller cannot turn it off. Check that line + before pasting: an agent chooses the command, so this is what stops a look-alike + prompt from handing the token to something that prints it. + +It does **not** make the secret short-lived: create a single-use token for the +step and delete it straight after, as the release docs say. + +## Tests + +```bash +uv run --directory tools/secret-prompt --project . pytest +``` + +Every backend is exercised without a real dialog: `osascript` through a +patched `subprocess.run`, `pinentry` through a fake `pinentry` script on +`PATH`, which also drives an end-to-end run checking that the secret reaches +the command's environment and none of the output. diff --git a/tools/secret-prompt/pyproject.toml b/tools/secret-prompt/pyproject.toml new file mode 100644 index 000000000..d4714d1d9 --- /dev/null +++ b/tools/secret-prompt/pyproject.toml @@ -0,0 +1,88 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +[build-system] +requires = ["hatchling"] +build-backend = "hatchling.build" + +[project] +name = "secret-prompt" +version = "0.1.0" +description = "Ask the operator for a secret (terminal, macOS dialog, zenity/kdialog or pinentry) and run one command with it in one environment variable, never printing it." +readme = "README.md" +requires-python = ">=3.11" +license = { text = "Apache-2.0" } +# stdlib-only — getpass, subprocess and the platform dialogs cover every backend. +dependencies = [] + +[project.scripts] +secret-prompt = "secret_prompt:main" + +[tool.hatch.build.targets.wheel] +packages = ["src/secret_prompt"] + +[tool.ruff] +line-length = 110 +target-version = "py311" +src = ["src", "tests"] + +[tool.ruff.lint] +select = [ + "E", # pycodestyle errors + "W", # pycodestyle warnings + "F", # pyflakes + "I", # isort + "B", # flake8-bugbear + "UP", # pyupgrade + "SIM", # flake8-simplify + "C4", # flake8-comprehensions + "RUF", # ruff-specific +] +ignore = [ + "E501", # line-too-long — the 110-char limit above is already generous +] + +[tool.ruff.lint.per-file-ignores] +"tests/**" = ["B", "SIM"] + +[tool.mypy] +python_version = "3.11" +files = ["src", "tests"] +warn_unused_ignores = true +warn_redundant_casts = true +warn_unreachable = true +check_untyped_defs = true +no_implicit_optional = true +disallow_untyped_defs = true +disallow_incomplete_defs = true + +[[tool.mypy.overrides]] +module = "tests.*" +disallow_untyped_defs = false +disallow_incomplete_defs = false + +[tool.pytest.ini_options] +minversion = "8.0" +addopts = "-ra -q" +testpaths = ["tests"] + +[dependency-groups] +# The shared toolchain (mypy, pytest, ruff) comes from `magpie-dev` +# (tools/dev), declared once for the whole workspace. The checks run each tool +# via `uv run --directory --project . python -m ` — see +# tools/dev/run-workspace-check.sh. +dev = ["magpie-dev"] diff --git a/tools/secret-prompt/src/secret_prompt/__init__.py b/tools/secret-prompt/src/secret_prompt/__init__.py new file mode 100644 index 000000000..f0be23bba --- /dev/null +++ b/tools/secret-prompt/src/secret_prompt/__init__.py @@ -0,0 +1,254 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Ask the operator for a secret and run one command with it in one environment variable. + +The secret goes from the prompt into the child process's environment and nowhere else: it is +never printed, written to a file or passed on a command line, so it stays out of the shell +history, ``ps`` and an agent's transcript. A cancelled or rejected prompt runs nothing. +""" + +from __future__ import annotations + +import argparse +import getpass +import os +import platform +import shlex +import shutil +import subprocess +import sys +import urllib.parse +from collections.abc import Callable, Mapping, Sequence + +BACKENDS = ("tty", "osascript", "zenity", "kdialog", "pinentry") + +# The prompt text reaches AppleScript as an argument, never spliced into the script source, +# so a quote in it cannot change what the dialog runs. +_OSASCRIPT = """on run argv + set answer to display dialog (item 1 of argv) default answer "" with hidden answer with title (item 2 of argv) buttons {"Cancel", "OK"} default button "OK" + return text returned of answer +end run""" + + +class PromptCancelledError(Exception): + """The operator dismissed the prompt.""" + + +class PromptBackendError(Exception): + """A backend could not ask: its program is missing, or it failed without the operator's say.""" + + +# pinentry's GPG_ERR_CANCELED, as "ERR 83886179 Operation cancelled". +_PINENTRY_CANCELLED = "83886179" + + +def has_tty() -> bool: + try: + with open("/dev/tty"): + return True + except OSError: + return False + + +def has_display(env: Mapping[str, str]) -> bool: + return bool(env.get("DISPLAY") or env.get("WAYLAND_DISPLAY")) + + +def candidate_backends( + system: str, + env: Mapping[str, str], + tty: bool, + which: Callable[[str], str | None] = shutil.which, +) -> list[str]: + """The ways to ask, in order: the terminal when there is one, then the platform's dialogs.""" + names = ["tty"] if tty else [] + if system == "Darwin" and which("osascript"): + names.append("osascript") + if has_display(env): + names.extend(name for name in ("zenity", "kdialog") if which(name)) + if which("pinentry"): + names.append("pinentry") + if not names: + raise PromptBackendError( + "no terminal, and none of osascript (macOS), zenity or kdialog (Linux desktop) " + "or pinentry is available" + ) + return names + + +def _excerpt(text: str) -> str: + lines = [line.strip() for line in text.strip().splitlines() if line.strip()] + return lines[-1][:200] if lines else "no error output" + + +def ask_tty(prompt: str, title: str) -> str: + # getpass reads from /dev/tty with echo off, even when stdin is redirected. + try: + return getpass.getpass(f"{prompt}: ") + except (EOFError, KeyboardInterrupt) as err: + raise PromptCancelledError("prompt cancelled") from err + + +def _run(cmd: Sequence[str], input: str | None = None) -> subprocess.CompletedProcess[str]: + try: + return subprocess.run(cmd, input=input, capture_output=True, text=True, check=False) + except OSError as err: + raise PromptBackendError(f"cannot run {cmd[0]}: {err.strerror}") from err + + +def _run_dialog(cmd: Sequence[str], cancelled: Callable[[subprocess.CompletedProcess[str]], bool]) -> str: + result = _run(cmd) + if result.returncode == 0: + return result.stdout + if cancelled(result): + raise PromptCancelledError("prompt cancelled") + # A dialog's stderr never holds the secret, so an excerpt is safe to show. + raise PromptBackendError(f"{cmd[0]} failed: {_excerpt(result.stderr)}") + + +def ask_osascript(prompt: str, title: str) -> str: + # The Cancel button is AppleScript error -128; any other failure (e.g. Apple Events + # blocked by a sandbox) means no dialog was shown. + return _run_dialog(["osascript", "-e", _OSASCRIPT, prompt, title], lambda r: "-128" in r.stderr) + + +def ask_zenity(prompt: str, title: str) -> str: + # A real Cancel exits 1 with nothing on stderr; GTK also exits 1, with a message, when it + # cannot open the display. + return _run_dialog( + ["zenity", "--entry", "--hide-text", f"--title={title}", f"--text={prompt}"], + lambda r: r.returncode == 1 and not r.stderr.strip(), + ) + + +def ask_kdialog(prompt: str, title: str) -> str: + return _run_dialog(["kdialog", "--title", title, "--password", prompt], lambda r: r.returncode == 1) + + +def _assuan_quote(text: str) -> str: + return text.replace("%", "%25").replace("\r", "%0D").replace("\n", "%0A") + + +def ask_pinentry(prompt: str, title: str, program: str = "pinentry") -> str: + """Speak the Assuan protocol to pinentry: describe the request, then GETPIN.""" + script = ( + f"SETTITLE {_assuan_quote(title)}\nSETDESC {_assuan_quote(prompt)}\nSETPROMPT Secret:\nGETPIN\nBYE\n" + ) + result = _run([program], input=script) + for line in result.stdout.splitlines(): + if line.startswith("D "): + return urllib.parse.unquote(line[2:]) + if line.startswith("ERR"): + fields = line.split() + if len(fields) > 1 and fields[1] == _PINENTRY_CANCELLED: + raise PromptCancelledError("prompt cancelled") + raise PromptBackendError(f"{program} failed: {line[:200]}") + raise PromptBackendError(f"{program} failed: {_excerpt(result.stderr)}") + + +ASKERS: dict[str, Callable[[str, str], str]] = { + "tty": ask_tty, + "osascript": ask_osascript, + "zenity": ask_zenity, + "kdialog": ask_kdialog, + "pinentry": ask_pinentry, +} + + +def parse_args(argv: Sequence[str]) -> argparse.Namespace: + parser = argparse.ArgumentParser( + prog="secret-prompt", + description="Ask for a secret and run COMMAND with it in the environment variable --env.", + epilog="Example: secret-prompt --env TWINE_PASSWORD --prefix pypi- -- twine upload dist/*", + ) + parser.add_argument("--env", required=True, help="environment variable that receives the secret") + parser.add_argument("--prompt", help="text shown in the prompt (default: names the variable)") + parser.add_argument("--title", default="Secret needed", help="dialog title") + parser.add_argument( + "--prefix", default="", help="refuse a secret that does not start with this, e.g. pypi-" + ) + parser.add_argument( + "--set", + action="append", + default=[], + metavar="NAME=VALUE", + help="also set a non-secret variable for the command, e.g. TWINE_USERNAME=__token__", + ) + parser.add_argument("--backend", choices=("auto", *BACKENDS), default="auto") + parser.add_argument("command", nargs=argparse.REMAINDER, help="the command to run, after --") + args = parser.parse_args(argv) + if args.command and args.command[0] == "--": + args.command = args.command[1:] + if not args.command: + parser.error("give the command to run after --") + if not args.env.isidentifier(): + parser.error(f"--env {args.env!r} is not a valid environment variable name") + for item in args.set: + name, sep, _ = item.partition("=") + if not sep or not name.isidentifier(): + parser.error(f"--set {item!r} is not NAME=VALUE") + return args + + +def main(argv: Sequence[str] | None = None) -> int: + args = parse_args(sys.argv[1:] if argv is None else argv) + # Always name the receiving command, so the operator can see where the secret goes + # before pasting it; the caller cannot turn this off. + command = shlex.join(args.command) + if len(command) > 300: + command = command[:297] + "..." + prompt = f"{args.prompt or f'Paste the value for {args.env}'}\n\nIt will be passed to: {command}" + backends = [args.backend] if args.backend != "auto" else None + failures: list[str] = [] + secret = "" + try: + for backend in backends or candidate_backends(platform.system(), os.environ, has_tty()): + try: + secret = ASKERS[backend](prompt, args.title).strip() + break + except PromptBackendError as err: + failures.append(f"{backend}: {err}") + else: + raise PromptBackendError("; ".join(failures)) + except PromptCancelledError as err: + print(f"secret-prompt: {err}; {args.command[0]} was not run", file=sys.stderr) + return 1 + except PromptBackendError as err: + print( + f"secret-prompt: could not ask for the secret ({err}); {args.command[0]} was not run", + file=sys.stderr, + ) + return 1 + if not secret: + print(f"secret-prompt: no secret entered; {args.command[0]} was not run", file=sys.stderr) + return 1 + if args.prefix and not secret.startswith(args.prefix): + # Name only the expected prefix: echoing what was pasted would leak it. + print( + f"secret-prompt: the secret does not start with {args.prefix!r}; {args.command[0]} was not run", + file=sys.stderr, + ) + return 1 + env = dict(os.environ) + env.update(item.split("=", 1) for item in args.set) + env[args.env] = secret + try: + os.execvpe(args.command[0], args.command, env) + except OSError as err: + print(f"secret-prompt: cannot run {args.command[0]}: {err.strerror}", file=sys.stderr) + return 127 diff --git a/tools/secret-prompt/src/secret_prompt/__main__.py b/tools/secret-prompt/src/secret_prompt/__main__.py new file mode 100644 index 000000000..fec9f2649 --- /dev/null +++ b/tools/secret-prompt/src/secret_prompt/__main__.py @@ -0,0 +1,22 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +import sys + +from secret_prompt import main + +sys.exit(main()) diff --git a/tools/secret-prompt/tests/test_secret_prompt.py b/tools/secret-prompt/tests/test_secret_prompt.py new file mode 100644 index 000000000..d0f96aeb0 --- /dev/null +++ b/tools/secret-prompt/tests/test_secret_prompt.py @@ -0,0 +1,345 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Tests for secret-prompt: backend choice, the run gates, and the secret staying out of output.""" + +from __future__ import annotations + +import os +import subprocess +import sys +import textwrap +from collections.abc import Callable +from pathlib import Path +from typing import Any + +import pytest + +import secret_prompt +from secret_prompt import ( + PromptBackendError, + PromptCancelledError, + ask_osascript, + ask_pinentry, + candidate_backends, + main, +) + +SRC = Path(__file__).parent.parent / "src" + + +def _which(*present: str) -> Callable[[str], str | None]: + return lambda name: f"/usr/bin/{name}" if name in present else None + + +@pytest.mark.parametrize( + ("system", "env", "tty", "present", "expected"), + [ + ("Darwin", {}, True, ("osascript",), ["tty", "osascript"]), + ("Linux", {"DISPLAY": ":0"}, True, ("zenity",), ["tty", "zenity"]), + ("Darwin", {}, False, ("osascript", "pinentry"), ["osascript", "pinentry"]), + ("Linux", {"DISPLAY": ":0"}, False, ("zenity", "kdialog"), ["zenity", "kdialog"]), + ("Linux", {"WAYLAND_DISPLAY": "wayland-0"}, False, ("kdialog",), ["kdialog"]), + ("Linux", {}, False, ("zenity", "pinentry"), ["pinentry"]), + ("Darwin", {}, False, ("pinentry",), ["pinentry"]), + ], +) +def test_candidate_backends( + system: str, env: dict[str, str], tty: bool, present: tuple[str, ...], expected: list[str] +) -> None: + assert candidate_backends(system, env, tty, _which(*present)) == expected + + +def test_candidate_backends_with_no_way_to_ask() -> None: + with pytest.raises(PromptBackendError, match="no terminal"): + candidate_backends("Linux", {}, False, _which()) + + +@pytest.mark.parametrize( + ("argv", "message"), + [ + (["--env", "TOKEN"], "after --"), + (["--env", "NOT-VALID", "--", "true"], "not a valid environment variable"), + (["--env", "TOKEN", "--set", "NOEQUALS", "--", "true"], "not NAME=VALUE"), + ], +) +def test_bad_arguments(argv: list[str], message: str, capsys: pytest.CaptureFixture[str]) -> None: + with pytest.raises(SystemExit): + main(argv) + assert message in capsys.readouterr().err + + +@pytest.fixture +def exec_calls(monkeypatch: pytest.MonkeyPatch) -> list[tuple[str, list[str], dict[str, str]]]: + calls: list[tuple[str, list[str], dict[str, str]]] = [] + monkeypatch.setattr(os, "execvpe", lambda file, args, env: calls.append((file, args, env))) + return calls + + +def _answer( + monkeypatch: pytest.MonkeyPatch, value: str | None = None, error: Exception | None = None +) -> None: + def ask(prompt: str, title: str) -> str | None: + if error: + raise error + return value + + monkeypatch.setitem(secret_prompt.ASKERS, "tty", ask) + + +def test_runs_command_with_secret_and_extra_variables( + monkeypatch: pytest.MonkeyPatch, exec_calls: list[tuple[str, list[str], dict[str, str]]] +) -> None: + _answer(monkeypatch, " pypi-abc123\n") + main( + [ + "--env", + "TWINE_PASSWORD", + "--set", + "TWINE_USERNAME=__token__", + "--backend", + "tty", + "--", + "twine", + "upload", + "dist/x.whl", + ] + ) + [(file, args, env)] = exec_calls + assert (file, args) == ("twine", ["twine", "upload", "dist/x.whl"]) + assert env["TWINE_PASSWORD"] == "pypi-abc123" + assert env["TWINE_USERNAME"] == "__token__" + + +@pytest.mark.parametrize( + ("value", "error", "reason"), + [ + ("wrong-abc123", None, "does not start with 'pypi-'"), + (" ", None, "no secret entered"), + (None, PromptCancelledError("prompt cancelled"), "prompt cancelled"), + ], +) +def test_nothing_runs_unless_a_good_secret_arrives( + monkeypatch: pytest.MonkeyPatch, + exec_calls: list[tuple[str, list[str], dict[str, str]]], + capsys: pytest.CaptureFixture[str], + value: str | None, + error: Exception | None, + reason: str, +) -> None: + _answer(monkeypatch, value, error) + rc = main(["--env", "TWINE_PASSWORD", "--prefix", "pypi-", "--backend", "tty", "--", "twine", "upload"]) + err = capsys.readouterr().err + assert rc == 1 + assert exec_calls == [] + assert reason in err + assert "twine was not run" in err + assert "wrong-abc123" not in err + + +def test_missing_command_is_reported( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + _answer(monkeypatch, "pypi-abc") + assert main(["--env", "T", "--backend", "tty", "--", "/nonexistent/secret-prompt-cmd"]) == 127 + assert "cannot run /nonexistent/secret-prompt-cmd" in capsys.readouterr().err + + +def _completed(returncode: int, stdout: str = "", stderr: str = "") -> subprocess.CompletedProcess[str]: + return subprocess.CompletedProcess(["x"], returncode, stdout=stdout, stderr=stderr) + + +@pytest.mark.parametrize( + ("result", "error"), + [ + (_completed(1, stderr="execution error: User canceled. (-128)"), PromptCancelledError), + ( + _completed(1, stderr="execution error: Not authorized to send Apple events. (-1743)"), + PromptBackendError, + ), + ], +) +def test_osascript_tells_cancel_from_failure( + monkeypatch: pytest.MonkeyPatch, result: subprocess.CompletedProcess[str], error: type[Exception] +) -> None: + monkeypatch.setattr(subprocess, "run", lambda cmd, **kwargs: result) + with pytest.raises(error): + ask_osascript("Paste it", "PyPI upload") + + +def test_auto_falls_back_when_a_dialog_cannot_show( + monkeypatch: pytest.MonkeyPatch, exec_calls: list[tuple[str, list[str], dict[str, str]]] +) -> None: + def broken(prompt: str, title: str) -> str: + raise PromptBackendError("osascript failed: Not authorized to send Apple events") + + monkeypatch.setattr(secret_prompt, "candidate_backends", lambda *args: ["osascript", "pinentry"]) + monkeypatch.setitem(secret_prompt.ASKERS, "osascript", broken) + monkeypatch.setitem(secret_prompt.ASKERS, "pinentry", lambda prompt, title: "pypi-fallback") + main(["--env", "T", "--", "true"]) + [(_, _, env)] = exec_calls + assert env["T"] == "pypi-fallback" + + +def test_auto_stops_at_a_real_cancel( + monkeypatch: pytest.MonkeyPatch, + exec_calls: list[tuple[str, list[str], dict[str, str]]], + capsys: pytest.CaptureFixture[str], +) -> None: + def cancelled(prompt: str, title: str) -> str: + raise PromptCancelledError("prompt cancelled") + + asked: list[str] = [] + monkeypatch.setattr(secret_prompt, "candidate_backends", lambda *args: ["osascript", "pinentry"]) + monkeypatch.setitem(secret_prompt.ASKERS, "osascript", cancelled) + + def never_asked(prompt: str, title: str) -> str: + asked.append("pinentry") + return "x" + + monkeypatch.setitem(secret_prompt.ASKERS, "pinentry", never_asked) + assert main(["--env", "T", "--", "true"]) == 1 + assert asked == [] + assert exec_calls == [] + assert "prompt cancelled" in capsys.readouterr().err + + +def test_forced_backend_that_is_not_installed( + monkeypatch: pytest.MonkeyPatch, + exec_calls: list[tuple[str, list[str], dict[str, str]]], + capsys: pytest.CaptureFixture[str], +) -> None: + monkeypatch.setenv("PATH", "/nonexistent") + assert main(["--env", "T", "--backend", "zenity", "--", "true"]) == 1 + err = capsys.readouterr().err + assert "could not ask for the secret (zenity: cannot run zenity" in err + assert exec_calls == [] + + +def test_osascript_gets_the_prompt_as_an_argument(monkeypatch: pytest.MonkeyPatch) -> None: + seen: list[list[str]] = [] + + def fake_run(cmd: list[str], **kwargs: Any) -> subprocess.CompletedProcess[str]: + seen.append(cmd) + return subprocess.CompletedProcess(cmd, 0, stdout="pypi-xyz\n", stderr="") + + monkeypatch.setattr(subprocess, "run", fake_run) + prompt = 'Paste "it" here' + assert ask_osascript(prompt, "PyPI upload") == "pypi-xyz\n" + [cmd] = seen + assert cmd[-2:] == [prompt, "PyPI upload"] + assert prompt not in cmd[2] + + +FAKE_PINENTRY = textwrap.dedent( + """\ + #!/usr/bin/env bash + echo "OK Pleased to meet you" + while read -r line; do + case "$line" in + GETPIN) {reply} ;; + BYE) echo "OK closing connection"; exit 0 ;; + *) echo OK ;; + esac + done + """ +) + + +def _fake_pinentry(tmp_path: Path, reply: str) -> Path: + path = tmp_path / "pinentry" + path.write_text(FAKE_PINENTRY.replace("{reply}", reply)) + path.chmod(0o755) + return path + + +def test_pinentry_decodes_the_pin(tmp_path: Path) -> None: + program = _fake_pinentry(tmp_path, 'echo "D pypi-a%25b"; echo OK') + assert ask_pinentry("Paste it", "PyPI upload", program=str(program)) == "pypi-a%b" + + +def test_pinentry_failure_is_not_a_cancel(tmp_path: Path) -> None: + program = _fake_pinentry(tmp_path, 'echo "ERR 83918950 Inappropriate ioctl for device"') + with pytest.raises(PromptBackendError, match="Inappropriate ioctl"): + ask_pinentry("Paste it", "PyPI upload", program=str(program)) + + +def test_pinentry_cancel(tmp_path: Path) -> None: + program = _fake_pinentry(tmp_path, 'echo "ERR 83886179 Operation cancelled "') + with pytest.raises(PromptCancelledError): + ask_pinentry("Paste it", "PyPI upload", program=str(program)) + + +def test_secret_reaches_only_the_child_environment(tmp_path: Path) -> None: + _fake_pinentry(tmp_path, 'echo "D pypi-end-to-end"; echo OK') + env = {**os.environ, "PATH": f"{tmp_path}{os.pathsep}{os.environ['PATH']}", "PYTHONPATH": str(SRC)} + check = "import os; print(os.environ['UPLOAD_TOKEN'] == 'pypi-end-to-end')" + result = subprocess.run( + [ + sys.executable, + "-m", + "secret_prompt", + "--env", + "UPLOAD_TOKEN", + "--prefix", + "pypi-", + "--backend", + "pinentry", + "--", + sys.executable, + "-c", + check, + ], + capture_output=True, + text=True, + env=env, + check=False, + ) + assert result.returncode == 0, result.stderr + assert result.stdout.strip() == "True" + assert "pypi-end-to-end" not in result.stdout + result.stderr + + +def test_every_prompt_names_the_receiving_command( + monkeypatch: pytest.MonkeyPatch, exec_calls: list[tuple[str, list[str], dict[str, str]]] +) -> None: + prompts: list[str] = [] + + def ask(prompt: str, title: str) -> str: + prompts.append(prompt) + return "pypi-abc" + + monkeypatch.setitem(secret_prompt.ASKERS, "tty", ask) + main(["--env", "T", "--prompt", "Paste the PyPI token", "--backend", "tty", "--", "sh", "-c", "echo $T"]) + [prompt] = prompts + assert prompt.startswith("Paste the PyPI token") + assert prompt.endswith("It will be passed to: sh -c 'echo $T'") + + +@pytest.mark.parametrize( + ("result", "error"), + [ + (_completed(1), PromptCancelledError), + (_completed(1, stderr="(zenity:4242): Gtk-WARNING **: cannot open display: :0"), PromptBackendError), + ], +) +def test_zenity_tells_cancel_from_a_missing_display( + monkeypatch: pytest.MonkeyPatch, result: subprocess.CompletedProcess[str], error: type[Exception] +) -> None: + monkeypatch.setattr(subprocess, "run", lambda cmd, **kwargs: result) + with pytest.raises(error): + secret_prompt.ask_zenity("Paste it", "PyPI upload") diff --git a/tools/spec-loop/specs/release-management-lifecycle.md b/tools/spec-loop/specs/release-management-lifecycle.md index 9aaa68618..43f04e9c1 100644 --- a/tools/spec-loop/specs/release-management-lifecycle.md +++ b/tools/spec-loop/specs/release-management-lifecycle.md @@ -186,6 +186,12 @@ code lands. Release Manager runs every signing and `svn commit` operation as themselves. Mirrors `security-cve-allocate` (Vulnogram URL + paste-ready JSON, human submits). +- **Upload credentials stay with the Release Manager.** A `stage_command` or + `publish_command` that needs the RM's token (e.g. `twine upload`) is declared + wrapped in `tools/secret-prompt`, which asks the RM for it — in their + terminal, or as a macOS / Linux dialog when an agent runs the step — and + passes it only to that one command's environment, so it never reaches a + file, the shell history, or the agent's transcript. - **The agent never publishes the release.** Step 10 (promotion) and step 11 (`[ANNOUNCE]` send + site-bump merge) are the moments of release; the agent drafts artefacts, the RM and PMC execute and merge. diff --git a/uv.lock b/uv.lock index f5ba6bc4b..18b570491 100644 --- a/uv.lock +++ b/uv.lock @@ -49,6 +49,7 @@ members = [ "release-verify", "reproducible-archive", "sandbox-lint", + "secret-prompt", "security-management", "security-tracker-stats-dashboard", "setup-preflight", @@ -1971,6 +1972,21 @@ dev = [ [package.metadata.requires-dev] dev = [{ name = "magpie-dev", editable = "tools/dev" }] +[[package]] +name = "secret-prompt" +version = "0.1.0" +source = { editable = "tools/secret-prompt" } + +[package.dev-dependencies] +dev = [ + { name = "magpie-dev" }, +] + +[package.metadata] + +[package.metadata.requires-dev] +dev = [{ name = "magpie-dev", editable = "tools/dev" }] + [[package]] name = "security-management" version = "0.1.0" From 317720fbe23e2323affa9e6923e10ec836032e80 Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Sun, 18 Oct 2026 14:55:30 +0200 Subject: [PATCH 2/6] fix(secret-prompt): make the prompt name exactly what will receive the secret Showing the command is what lets the release manager check where the token goes, so it must not be possible to make that line lie. The command is now resolved to an absolute path before asking, shown in full (or refused, never truncated) and run exactly as shown; the prompt also lists every --set variable and any inherited variable sharing the secret's prefix, such as a TWINE_REPOSITORY_URL. Names that change how a command runs (PATH, HOME, LD_*, DYLD_*, PYTHON*, ...) cannot be set, --prompt and --title must be one plain line, osascript gets "--" before the text, and zenity/kdialog decline text they might render as markup. Generated-by: Claude Code (Opus 5) --- tools/secret-prompt/README.md | 27 +++- .../src/secret_prompt/__init__.py | 78 ++++++++++-- .../secret-prompt/tests/test_secret_prompt.py | 117 +++++++++++++++--- 3 files changed, 192 insertions(+), 30 deletions(-) diff --git a/tools/secret-prompt/README.md b/tools/secret-prompt/README.md index 5daadb021..90921739a 100644 --- a/tools/secret-prompt/README.md +++ b/tools/secret-prompt/README.md @@ -108,10 +108,29 @@ Seatbelt sandbox turns off Apple Events — so run the step outside the sandbox - Nothing runs unless a non-empty secret arrives that matches `--prefix`; a cancelled dialog runs nothing. - Error messages name the expected prefix, never what was entered. -- Every prompt ends with **`It will be passed to: `**, the exact command - that will receive the secret, and the caller cannot turn it off. Check that line - before pasting: an agent chooses the command, so this is what stops a look-alike - prompt from handing the token to something that prints it. +- Every prompt ends with what the caller cannot change: **`It will be passed to:`** + the command, resolved to an absolute path before asking and run exactly as + shown, and **`with:`** every `--set` variable plus any inherited variable that + shares the secret's prefix (`TWINE_*` for `TWINE_PASSWORD`, so a + `TWINE_REPOSITORY_URL` pointing elsewhere is visible). Check those lines + before pasting: an agent chooses the command, and this is what stops a + look-alike prompt from handing the token to something that prints or + forwards it. +- The command is shown in full or not at all: one longer than 1000 characters + is refused, never truncated. `--prompt` and `--title` must be a single + printable line of at most 200 characters that does not start with `-`, so + they cannot imitate the lines above; `zenity` and `kdialog`, whose toolkits may + render `<` and `&` as markup, decline a prompt containing them and `auto` + moves on to the next backend. +- `--env` and `--set` refuse names that change how the command runs or where it + reads its configuration: `PATH`, `IFS`, `HOME`, `ENV`, `BASH_ENV`, + `SHELLOPTS`, `BASHOPTS`, `PS4`, and the `LD_*`, `DYLD_*`, `PYTHON*`, `PERL5*`, + `RUBY*`, `NODE_*` and `GIT_*` families. + +What it cannot do is vouch for an environment the caller already controls: +an agent that sets `LD_PRELOAD` or edits `~/.pypirc` *before* calling it can +still subvert the command. The prompt shows what it can; when in doubt, run the +step yourself, in your own terminal. It does **not** make the secret short-lived: create a single-use token for the step and delete it straight after, as the release docs say. diff --git a/tools/secret-prompt/src/secret_prompt/__init__.py b/tools/secret-prompt/src/secret_prompt/__init__.py index f0be23bba..faf249f13 100644 --- a/tools/secret-prompt/src/secret_prompt/__init__.py +++ b/tools/secret-prompt/src/secret_prompt/__init__.py @@ -53,6 +53,15 @@ class PromptBackendError(Exception): """A backend could not ask: its program is missing, or it failed without the operator's say.""" +# Variables that change how a command runs or where it reads its configuration; letting the +# caller set them would make the command shown in the prompt run something else. +_DENIED_NAMES = {"PATH", "IFS", "HOME", "ENV", "BASH_ENV", "SHELLOPTS", "BASHOPTS", "PS4"} +_DENIED_PREFIXES = ("LD_", "DYLD_", "PYTHON", "PERL5", "RUBY", "NODE_", "GIT_") +_MAX_TEXT = 200 +_MAX_COMMAND = 1000 +# Characters GTK and Qt may render as markup in a dialog label. +_MARKUP = ("<", "&") + # pinentry's GPG_ERR_CANCELED, as "ERR 83886179 Operation cancelled". _PINENTRY_CANCELLED = "83886179" @@ -124,12 +133,19 @@ def _run_dialog(cmd: Sequence[str], cancelled: Callable[[subprocess.CompletedPro def ask_osascript(prompt: str, title: str) -> str: # The Cancel button is AppleScript error -128; any other failure (e.g. Apple Events # blocked by a sandbox) means no dialog was shown. - return _run_dialog(["osascript", "-e", _OSASCRIPT, prompt, title], lambda r: "-128" in r.stderr) + # "--" ends osascript's option parsing, so text starting with "-" stays an argument. + return _run_dialog(["osascript", "-e", _OSASCRIPT, "--", prompt, title], lambda r: "-128" in r.stderr) + + +def _refuse_markup(name: str, text: str) -> None: + if any(char in text for char in _MARKUP): + raise PromptBackendError(f"{name} may render '<' or '&' as markup, so it cannot show this prompt") def ask_zenity(prompt: str, title: str) -> str: # A real Cancel exits 1 with nothing on stderr; GTK also exits 1, with a message, when it # cannot open the display. + _refuse_markup("zenity", prompt + title) return _run_dialog( ["zenity", "--entry", "--hide-text", f"--title={title}", f"--text={prompt}"], lambda r: r.returncode == 1 and not r.stderr.strip(), @@ -137,6 +153,7 @@ def ask_zenity(prompt: str, title: str) -> str: def ask_kdialog(prompt: str, title: str) -> str: + _refuse_markup("kdialog", prompt + title) return _run_dialog(["kdialog", "--title", title, "--password", prompt], lambda r: r.returncode == 1) @@ -198,21 +215,64 @@ def parse_args(argv: Sequence[str]) -> argparse.Namespace: parser.error("give the command to run after --") if not args.env.isidentifier(): parser.error(f"--env {args.env!r} is not a valid environment variable name") + names = [args.env] for item in args.set: name, sep, _ = item.partition("=") if not sep or not name.isidentifier(): parser.error(f"--set {item!r} is not NAME=VALUE") + names.append(name) + for name in names: + if is_denied_name(name): + parser.error(f"{name} changes how the command runs, so secret-prompt will not set it") + for option, text in (("--prompt", args.prompt), ("--title", args.title)): + if text is not None and not is_plain_text(text): + parser.error( + f"{option} must be one line of at most {_MAX_TEXT} printable characters, not starting with '-'" + ) return args +def is_denied_name(name: str) -> bool: + upper = name.upper() + return upper in _DENIED_NAMES or upper.startswith(_DENIED_PREFIXES) + + +def is_plain_text(text: str) -> bool: + return len(text) <= _MAX_TEXT and not text.startswith("-") and all(char.isprintable() for char in text) + + +def build_prompt(text: str, command: Sequence[str], extra: Mapping[str, str]) -> str: + """The caller's text, then what it cannot change: the command and the variables it gets.""" + lines = [text, "", "It will be passed to:", shlex.join(command)] + if extra: + lines += ["", "with:", *(f" {name}={value}" for name, value in sorted(extra.items()))] + return "\n".join(lines) + + def main(argv: Sequence[str] | None = None) -> int: args = parse_args(sys.argv[1:] if argv is None else argv) - # Always name the receiving command, so the operator can see where the secret goes - # before pasting it; the caller cannot turn this off. - command = shlex.join(args.command) - if len(command) > 300: - command = command[:297] + "..." - prompt = f"{args.prompt or f'Paste the value for {args.env}'}\n\nIt will be passed to: {command}" + # Resolve the command before asking, and run exactly what the prompt shows. + resolved = shutil.which(args.command[0]) + if resolved is None: + print(f"secret-prompt: cannot run {args.command[0]}: not found; nothing was asked", file=sys.stderr) + return 127 + command = [os.path.abspath(resolved), *args.command[1:]] + if len(shlex.join(command)) > _MAX_COMMAND: + # Shown in full or not at all: a truncated command could hide what it does. + print( + f"secret-prompt: the command is longer than {_MAX_COMMAND} characters; nothing was asked", + file=sys.stderr, + ) + return 1 + extra = dict(item.split("=", 1) for item in args.set) + # Inherited variables sharing the secret's prefix (TWINE_* for TWINE_PASSWORD) can redirect + # where it goes, e.g. TWINE_REPOSITORY_URL, so the operator sees those too. + family = args.env.split("_", 1)[0] + "_" if "_" in args.env else None + if family: + for name, value in os.environ.items(): + if name.startswith(family) and name != args.env and name not in extra: + extra[name] = value + prompt = build_prompt(args.prompt or f"Paste the value for {args.env}", command, extra) backends = [args.backend] if args.backend != "auto" else None failures: list[str] = [] secret = "" @@ -248,7 +308,7 @@ def main(argv: Sequence[str] | None = None) -> int: env.update(item.split("=", 1) for item in args.set) env[args.env] = secret try: - os.execvpe(args.command[0], args.command, env) + os.execve(command[0], command, env) except OSError as err: - print(f"secret-prompt: cannot run {args.command[0]}: {err.strerror}", file=sys.stderr) + print(f"secret-prompt: cannot run {command[0]}: {err.strerror}", file=sys.stderr) return 127 diff --git a/tools/secret-prompt/tests/test_secret_prompt.py b/tools/secret-prompt/tests/test_secret_prompt.py index d0f96aeb0..9f71141c9 100644 --- a/tools/secret-prompt/tests/test_secret_prompt.py +++ b/tools/secret-prompt/tests/test_secret_prompt.py @@ -20,6 +20,7 @@ from __future__ import annotations import os +import shutil import subprocess import sys import textwrap @@ -86,7 +87,7 @@ def test_bad_arguments(argv: list[str], message: str, capsys: pytest.CaptureFixt @pytest.fixture def exec_calls(monkeypatch: pytest.MonkeyPatch) -> list[tuple[str, list[str], dict[str, str]]]: calls: list[tuple[str, list[str], dict[str, str]]] = [] - monkeypatch.setattr(os, "execvpe", lambda file, args, env: calls.append((file, args, env))) + monkeypatch.setattr(os, "execve", lambda file, args, env: calls.append((file, args, env))) return calls @@ -114,13 +115,14 @@ def test_runs_command_with_secret_and_extra_variables( "--backend", "tty", "--", - "twine", - "upload", - "dist/x.whl", + "sh", + "-c", + "true", ] ) [(file, args, env)] = exec_calls - assert (file, args) == ("twine", ["twine", "upload", "dist/x.whl"]) + sh = os.path.abspath(shutil.which("sh") or "") + assert (file, args) == (sh, [sh, "-c", "true"]) assert env["TWINE_PASSWORD"] == "pypi-abc123" assert env["TWINE_USERNAME"] == "__token__" @@ -142,21 +144,23 @@ def test_nothing_runs_unless_a_good_secret_arrives( reason: str, ) -> None: _answer(monkeypatch, value, error) - rc = main(["--env", "TWINE_PASSWORD", "--prefix", "pypi-", "--backend", "tty", "--", "twine", "upload"]) + rc = main(["--env", "TWINE_PASSWORD", "--prefix", "pypi-", "--backend", "tty", "--", "true"]) err = capsys.readouterr().err assert rc == 1 assert exec_calls == [] assert reason in err - assert "twine was not run" in err + assert "true was not run" in err assert "wrong-abc123" not in err -def test_missing_command_is_reported( +def test_missing_command_is_reported_before_asking( monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] ) -> None: - _answer(monkeypatch, "pypi-abc") + _answer(monkeypatch, error=AssertionError("must not ask")) assert main(["--env", "T", "--backend", "tty", "--", "/nonexistent/secret-prompt-cmd"]) == 127 - assert "cannot run /nonexistent/secret-prompt-cmd" in capsys.readouterr().err + assert ( + "cannot run /nonexistent/secret-prompt-cmd: not found; nothing was asked" in capsys.readouterr().err + ) def _completed(returncode: int, stdout: str = "", stderr: str = "") -> subprocess.CompletedProcess[str]: @@ -224,7 +228,7 @@ def test_forced_backend_that_is_not_installed( capsys: pytest.CaptureFixture[str], ) -> None: monkeypatch.setenv("PATH", "/nonexistent") - assert main(["--env", "T", "--backend", "zenity", "--", "true"]) == 1 + assert main(["--env", "T", "--backend", "zenity", "--", sys.executable]) == 1 err = capsys.readouterr().err assert "could not ask for the secret (zenity: cannot run zenity" in err assert exec_calls == [] @@ -314,9 +318,7 @@ def test_secret_reaches_only_the_child_environment(tmp_path: Path) -> None: assert "pypi-end-to-end" not in result.stdout + result.stderr -def test_every_prompt_names_the_receiving_command( - monkeypatch: pytest.MonkeyPatch, exec_calls: list[tuple[str, list[str], dict[str, str]]] -) -> None: +def _captured_prompt(monkeypatch: pytest.MonkeyPatch, argv: list[str]) -> str: prompts: list[str] = [] def ask(prompt: str, title: str) -> str: @@ -324,10 +326,91 @@ def ask(prompt: str, title: str) -> str: return "pypi-abc" monkeypatch.setitem(secret_prompt.ASKERS, "tty", ask) - main(["--env", "T", "--prompt", "Paste the PyPI token", "--backend", "tty", "--", "sh", "-c", "echo $T"]) + monkeypatch.setattr(os, "execve", lambda file, args, env: None) + main([*argv[:-1], "--backend", "tty", "--", *argv[-1].split("|")]) [prompt] = prompts - assert prompt.startswith("Paste the PyPI token") - assert prompt.endswith("It will be passed to: sh -c 'echo $T'") + return prompt + + +def test_every_prompt_names_the_resolved_command_and_variables(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("TWINE_REPOSITORY_URL", "https://example.invalid/legacy/") + prompt = _captured_prompt( + monkeypatch, + [ + "--env", + "TWINE_PASSWORD", + "--prompt", + "Paste the PyPI token", + "--set", + "TWINE_USERNAME=__token__", + "sh|-c|echo $TWINE_PASSWORD", + ], + ) + sh = os.path.abspath(shutil.which("sh") or "") + assert prompt.splitlines() == [ + "Paste the PyPI token", + "", + "It will be passed to:", + f"{sh} -c 'echo $TWINE_PASSWORD'", + "", + "with:", + " TWINE_REPOSITORY_URL=https://example.invalid/legacy/", + " TWINE_USERNAME=__token__", + ] + + +@pytest.mark.parametrize( + ("argv", "message"), + [ + (["--env", "PATH", "--", "true"], "PATH changes how the command runs"), + ( + ["--env", "T", "--set", "LD_PRELOAD=/tmp/x.so", "--", "true"], + "LD_PRELOAD changes how the command runs", + ), + (["--env", "T", "--set", "dyld_insert_libraries=x", "--", "true"], "dyld_insert_libraries changes"), + (["--env", "T", "--set", "PYTHONPATH=/tmp", "--", "true"], "PYTHONPATH changes"), + (["--env", "T", "--set", "HOME=/tmp", "--", "true"], "HOME changes"), + ( + ["--env", "T", "--prompt", "Paste\n\nIt will be passed to: twine", "--", "true"], + "--prompt must be one line", + ), + (["--env", "T", "--prompt=-x", "--", "true"], "--prompt must be one line"), + (["--env", "T", "--title", "x" * 201, "--", "true"], "--title must be one line"), + ], +) +def test_refuses_what_could_make_the_prompt_lie( + argv: list[str], message: str, capsys: pytest.CaptureFixture[str] +) -> None: + with pytest.raises(SystemExit): + main(argv) + assert message in capsys.readouterr().err + + +def test_too_long_command_is_refused_not_truncated( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + _answer(monkeypatch, error=AssertionError("must not ask")) + assert main(["--env", "T", "--backend", "tty", "--", "true", "x" * 1000]) == 1 + assert "longer than 1000 characters; nothing was asked" in capsys.readouterr().err + + +@pytest.mark.parametrize("asker", [secret_prompt.ask_zenity, secret_prompt.ask_kdialog]) +def test_markup_capable_dialogs_refuse_markup_characters(asker: object) -> None: + with pytest.raises(PromptBackendError, match="markup"): + asker("Paste it", "PyPI upload") # type: ignore[operator] + + +def test_osascript_ends_option_parsing_before_the_text(monkeypatch: pytest.MonkeyPatch) -> None: + seen: list[list[str]] = [] + + def fake_run(cmd: list[str], **kwargs: object) -> subprocess.CompletedProcess[str]: + seen.append(cmd) + return _completed(0, "pypi-x") + + monkeypatch.setattr(subprocess, "run", fake_run) + ask_osascript("-looks-like-an-option", "PyPI upload") + [cmd] = seen + assert cmd[3:] == ["--", "-looks-like-an-option", "PyPI upload"] @pytest.mark.parametrize( From 7674d726f57e11e5f9bde18c2fd9b9df7c29e211 Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Sun, 18 Oct 2026 15:48:29 +0200 Subject: [PATCH 3/6] fix(secret-prompt): keep other credentials and control characters out of the prompt The "It will be passed to" and "with:" lines are what the operator checks before pasting, so nothing the caller controls may rewrite them: an escape sequence, newline or bidi character in a command argument, a value or an inherited variable name could still fake those lines. And the prompt text reaches a dialog in its argv, so inherited values that may be credentials (another token, a URL with userinfo or a query) must not be copied into it. NODE_AUTH_TOKEN, npm's publish credential, was blocked by the NODE_ denial although it only authenticates. Generated-by: Claude Code (Opus 5) --- tools/secret-prompt/README.md | 19 ++++- .../src/secret_prompt/__init__.py | 63 +++++++++++++++- .../secret-prompt/tests/test_secret_prompt.py | 71 +++++++++++++++++++ 3 files changed, 149 insertions(+), 4 deletions(-) diff --git a/tools/secret-prompt/README.md b/tools/secret-prompt/README.md index 90921739a..a6cd1401b 100644 --- a/tools/secret-prompt/README.md +++ b/tools/secret-prompt/README.md @@ -116,8 +116,22 @@ Seatbelt sandbox turns off Apple Events — so run the step outside the sandbox before pasting: an agent chooses the command, and this is what stops a look-alike prompt from handing the token to something that prints or forwards it. +- An inherited variable's value is shown only when its name says where the + secret goes (`URL`, `REPOSITORY`, `REGISTRY`/`REGISTRIES`, `INDEX`, `HOST`, + `ENDPOINT`, `USER`) and nothing suggests it is a credential (`TOKEN`, `PASS`, + `SECRET`, `KEY`, `AUTH`, `CREDENTIAL`), or when it is one of the denied names + below (`NODE_OPTIONS` beside `NODE_AUTH_TOKEN`). Every other one is listed as + `NAME=(set; value not shown)`. A shown URL stops at its path, with any query + or fragment as `?***`/`#***`, and one containing `@` is hidden whole, so the + prompt, which a dialog receives in its argv, does not carry the credentials + these usually hold. An inherited name that is not a plain identifier is + refused. Only the secret's own prefix is scanned: npm's registry settings + (`npm_config_registry`, `.npmrc`) are not shown. - The command is shown in full or not at all: one longer than 1000 characters - is refused, never truncated. `--prompt` and `--title` must be a single + is refused, never truncated. Every command argument and every shown value + must be printable — no newline, escape sequence or invisible bidi character + that could rewrite or reorder those lines — and a value may be at most 200 + characters; otherwise nothing is asked. `--prompt` and `--title` must be a single printable line of at most 200 characters that does not start with `-`, so they cannot imitate the lines above; `zenity` and `kdialog`, whose toolkits may render `<` and `&` as markup, decline a prompt containing them and `auto` @@ -125,7 +139,8 @@ Seatbelt sandbox turns off Apple Events — so run the step outside the sandbox - `--env` and `--set` refuse names that change how the command runs or where it reads its configuration: `PATH`, `IFS`, `HOME`, `ENV`, `BASH_ENV`, `SHELLOPTS`, `BASHOPTS`, `PS4`, and the `LD_*`, `DYLD_*`, `PYTHON*`, `PERL5*`, - `RUBY*`, `NODE_*` and `GIT_*` families. + `RUBY*`, `NODE_*` and `GIT_*` families — except `NODE_AUTH_TOKEN`, npm's + publish credential. What it cannot do is vouch for an environment the caller already controls: an agent that sets `LD_PRELOAD` or edits `~/.pypirc` *before* calling it can diff --git a/tools/secret-prompt/src/secret_prompt/__init__.py b/tools/secret-prompt/src/secret_prompt/__init__.py index faf249f13..c35a5e0f9 100644 --- a/tools/secret-prompt/src/secret_prompt/__init__.py +++ b/tools/secret-prompt/src/secret_prompt/__init__.py @@ -57,6 +57,13 @@ class PromptBackendError(Exception): # caller set them would make the command shown in the prompt run something else. _DENIED_NAMES = {"PATH", "IFS", "HOME", "ENV", "BASH_ENV", "SHELLOPTS", "BASHOPTS", "PS4"} _DENIED_PREFIXES = ("LD_", "DYLD_", "PYTHON", "PERL5", "RUBY", "NODE_", "GIT_") +# npm's publish credential; it only authenticates, so it is safe to set despite its prefix. +_ALLOWED_NAMES = {"NODE_AUTH_TOKEN"} +# An inherited variable's value is shown only when its name says where the secret goes; any +# other may itself be a credential, and a dialog's prompt text is visible in its argv. +_ROUTING_WORDS = ("URL", "REPOSITORY", "REGISTRY", "REGISTRIES", "INDEX", "HOST", "ENDPOINT", "USER") +_SECRET_WORDS = ("TOKEN", "PASS", "SECRET", "KEY", "AUTH", "CREDENTIAL") +_HIDDEN_VALUE = "(set; value not shown)" _MAX_TEXT = 200 _MAX_COMMAND = 1000 # Characters GTK and Qt may render as markup in a dialog label. @@ -234,6 +241,8 @@ def parse_args(argv: Sequence[str]) -> argparse.Namespace: def is_denied_name(name: str) -> bool: upper = name.upper() + if upper in _ALLOWED_NAMES: + return False return upper in _DENIED_NAMES or upper.startswith(_DENIED_PREFIXES) @@ -241,11 +250,50 @@ def is_plain_text(text: str) -> bool: return len(text) <= _MAX_TEXT and not text.startswith("-") and all(char.isprintable() for char in text) +def is_routing_name(name: str) -> bool: + upper = name.upper() + return any(word in upper for word in _ROUTING_WORDS) and not any(word in upper for word in _SECRET_WORDS) + + +def find_unshowable(command: Sequence[str], extra: Mapping[str, str]) -> str | None: + """Name the first shown string that a control or bidi character could make lie, if any.""" + # shlex.join quotes a newline, ESC or U+202E but does not escape it, so it would still + # rewrite or reorder the line the operator checks. + if not all(arg.isprintable() for arg in command): + return "the command" + for name, value in extra.items(): + # An inherited name comes from the environment unchecked and can hold anything but '='. + if not name.isidentifier(): + return "an inherited variable name" + if len(value) > _MAX_TEXT or not value.isprintable(): + return name + return None + + +def redact_url(value: str) -> str: + """Show a URL up to its path: the host the operator checks, not a credential carried in it.""" + if "://" not in value: + return value + # urlsplit misreads an unencoded '/' in a password, so any '@' hides the whole URL. + if "@" in value: + return _HIDDEN_VALUE + try: + parts = urllib.parse.urlsplit(value) + except ValueError: + return _HIDDEN_VALUE + shown = urllib.parse.urlunsplit((parts.scheme, parts.netloc, parts.path, "", "")) + return shown + ("?***" if parts.query else "") + ("#***" if parts.fragment else "") + + def build_prompt(text: str, command: Sequence[str], extra: Mapping[str, str]) -> str: """The caller's text, then what it cannot change: the command and the variables it gets.""" lines = [text, "", "It will be passed to:", shlex.join(command)] if extra: - lines += ["", "with:", *(f" {name}={value}" for name, value in sorted(extra.items()))] + lines += [ + "", + "with:", + *(f" {name}={redact_url(value)}" for name, value in sorted(extra.items())), + ] return "\n".join(lines) @@ -271,7 +319,18 @@ def main(argv: Sequence[str] | None = None) -> int: if family: for name, value in os.environ.items(): if name.startswith(family) and name != args.env and name not in extra: - extra[name] = value + # A denied name (NODE_OPTIONS for NODE_AUTH_TOKEN) can make the command run + # something else, so its value is always shown. + shown = is_routing_name(name) or is_denied_name(name) + extra[name] = value if shown else _HIDDEN_VALUE + unshowable = find_unshowable(command, extra) + if unshowable: + print( + f"secret-prompt: {unshowable} cannot be shown as it is (a control or invisible character, " + f"or a value over {_MAX_TEXT} characters); nothing was asked", + file=sys.stderr, + ) + return 1 prompt = build_prompt(args.prompt or f"Paste the value for {args.env}", command, extra) backends = [args.backend] if args.backend != "auto" else None failures: list[str] = [] diff --git a/tools/secret-prompt/tests/test_secret_prompt.py b/tools/secret-prompt/tests/test_secret_prompt.py index 9f71141c9..65b65f2af 100644 --- a/tools/secret-prompt/tests/test_secret_prompt.py +++ b/tools/secret-prompt/tests/test_secret_prompt.py @@ -370,6 +370,8 @@ def test_every_prompt_names_the_resolved_command_and_variables(monkeypatch: pyte (["--env", "T", "--set", "dyld_insert_libraries=x", "--", "true"], "dyld_insert_libraries changes"), (["--env", "T", "--set", "PYTHONPATH=/tmp", "--", "true"], "PYTHONPATH changes"), (["--env", "T", "--set", "HOME=/tmp", "--", "true"], "HOME changes"), + (["--env", "T", "--set", "NODE_OPTIONS=--require=x", "--", "true"], "NODE_OPTIONS changes"), + (["--env", "T", "--set", "NODE_REPL_EXTERNAL_MODULE=x", "--", "true"], "NODE_REPL_EXTERNAL_MODULE"), ( ["--env", "T", "--prompt", "Paste\n\nIt will be passed to: twine", "--", "true"], "--prompt must be one line", @@ -394,6 +396,75 @@ def test_too_long_command_is_refused_not_truncated( assert "longer than 1000 characters; nothing was asked" in capsys.readouterr().err +def test_inherited_values_are_shown_only_for_routing_names(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("XSP_UPLOAD_URL", "https://example.invalid/") + monkeypatch.setenv("XSP_SECRET_KEY", "s3cr3t") + monkeypatch.setenv("XSP_AUTH_URL", "https://user:pw@example.invalid/") + monkeypatch.setenv("XSP_INDEX_URL", "https://__token__:pypi-Ag/E@upload.example.invalid/legacy/") + monkeypatch.setenv("XSP_REPOSITORY_URL", "https://example.invalid/simple?token=t0k#frag") + monkeypatch.setenv("XSP_MODE", "fast") + prompt = _captured_prompt(monkeypatch, ["--env", "XSP_TOKEN", "true"]) + assert prompt.splitlines()[-6:] == [ + " XSP_AUTH_URL=(set; value not shown)", + " XSP_INDEX_URL=(set; value not shown)", + " XSP_MODE=(set; value not shown)", + " XSP_REPOSITORY_URL=https://example.invalid/simple?***#***", + " XSP_SECRET_KEY=(set; value not shown)", + " XSP_UPLOAD_URL=https://example.invalid/", + ] + for leaked in ("s3cr3t", "pw@", "pypi-Ag", "t0k", "frag"): + assert leaked not in prompt + + +def test_inherited_denied_and_index_values_are_shown(monkeypatch: pytest.MonkeyPatch) -> None: + for name in [name for name in os.environ if name.startswith("NODE_")]: + monkeypatch.delenv(name) + monkeypatch.setenv("NODE_OPTIONS", "--require=/tmp/x.js") + monkeypatch.setenv("NODE_PUBLISH_INDEX", "https://example.invalid/") + prompt = _captured_prompt(monkeypatch, ["--env", "NODE_AUTH_TOKEN", "true"]) + assert prompt.splitlines()[-2:] == [ + " NODE_OPTIONS=--require=/tmp/x.js", + " NODE_PUBLISH_INDEX=https://example.invalid/", + ] + + +@pytest.mark.parametrize( + ("argv", "env", "what"), + [ + (["--", "sh", "-c", "curl evil\x1b[2K\r/usr/bin/twine upload"], {}, "the command"), + (["--", "true", "ok\nIt will be passed to:"], {}, "the command"), + (["--", "true", "‮exe.enimt"], {}, "the command"), + (["--set", "XSP_USER=a\nb", "--", "true"], {}, "XSP_USER"), + (["--set", "XSP_USER=" + "x" * 201, "--", "true"], {}, "XSP_USER"), + (["--", "true"], {"XSP_UPLOAD_URL": "https://ok.invalid/‮"}, "XSP_UPLOAD_URL"), + (["--", "true"], {"XSP_X\nIt will be passed to:": "v"}, "an inherited variable name"), + ], +) +def test_refuses_to_show_what_could_rewrite_the_prompt( + argv: list[str], + env: dict[str, str], + what: str, + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], +) -> None: + for name, value in env.items(): + monkeypatch.setenv(name, value) + _answer(monkeypatch, error=AssertionError("must not ask")) + assert main(["--env", "XSP_TOKEN", "--backend", "tty", *argv]) == 1 + assert f"{what} cannot be shown as it is" in capsys.readouterr().err + + +def test_npm_auth_token_is_allowed( + monkeypatch: pytest.MonkeyPatch, exec_calls: list[tuple[str, list[str], dict[str, str]]] +) -> None: + for name in [name for name in os.environ if name.startswith("NODE_")]: + monkeypatch.delenv(name) + _answer(monkeypatch, "npm_abc") + main(["--env", "NODE_AUTH_TOKEN", "--backend", "tty", "--", "true"]) + [(_, _, env)] = exec_calls + assert env["NODE_AUTH_TOKEN"] == "npm_abc" + + @pytest.mark.parametrize("asker", [secret_prompt.ask_zenity, secret_prompt.ask_kdialog]) def test_markup_capable_dialogs_refuse_markup_characters(asker: object) -> None: with pytest.raises(PromptBackendError, match="markup"): From e248c4fd0c384766d41ef80f26bdd71905d1ea97 Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Sun, 18 Oct 2026 16:55:09 +0200 Subject: [PATCH 4/6] fix(secret-prompt): never hide where the secret may go Hiding every inherited value whose name did not look like routing also hid variables that redirect the secret (a TWINE_CERT, an XSP_TOKEN_URL), so the prompt could not show where it went. Only values whose names say they are a credential are hidden now; a URL always shows its host. A value that can be neither shown nor hidden safely, such as a URL carrying a user and password or a credential-named variable that also changes how the command runs, stops the prompt instead of being quietly hidden. Generated-by: Claude Code (Opus 5) --- tools/secret-prompt/README.md | 34 ++++--- .../src/secret_prompt/__init__.py | 91 +++++++++++++----- .../secret-prompt/tests/test_secret_prompt.py | 96 +++++++++++++++---- 3 files changed, 169 insertions(+), 52 deletions(-) diff --git a/tools/secret-prompt/README.md b/tools/secret-prompt/README.md index a6cd1401b..4c96aca96 100644 --- a/tools/secret-prompt/README.md +++ b/tools/secret-prompt/README.md @@ -116,17 +116,29 @@ Seatbelt sandbox turns off Apple Events — so run the step outside the sandbox before pasting: an agent chooses the command, and this is what stops a look-alike prompt from handing the token to something that prints or forwards it. -- An inherited variable's value is shown only when its name says where the - secret goes (`URL`, `REPOSITORY`, `REGISTRY`/`REGISTRIES`, `INDEX`, `HOST`, - `ENDPOINT`, `USER`) and nothing suggests it is a credential (`TOKEN`, `PASS`, - `SECRET`, `KEY`, `AUTH`, `CREDENTIAL`), or when it is one of the denied names - below (`NODE_OPTIONS` beside `NODE_AUTH_TOKEN`). Every other one is listed as - `NAME=(set; value not shown)`. A shown URL stops at its path, with any query - or fragment as `?***`/`#***`, and one containing `@` is hidden whole, so the - prompt, which a dialog receives in its argv, does not carry the credentials - these usually hold. An inherited name that is not a plain identifier is - refused. Only the secret's own prefix is scanned: npm's registry settings - (`npm_config_registry`, `.npmrc`) are not shown. +- `--set` values are shown as given. An inherited variable's name is + credential-like when one of its `_`-separated segments, with or without a + trailing `S`, is `AUTH`, `CRED`, `CREDENTIAL`, `JWT`, `PAT`, `PW`, `PWD`, + `COOKIE`, `SESSION`, `BEARER`, `PRIVATE`, `WEBHOOK`, `HOOK` or `DSN`, or ends + in `TOKEN`, `PASS`, `PASSWORD`, `PASSWD`, `PASSPHRASE`, `SECRET` or `KEY`. An + inherited URL always shows its host, whatever the name: up to its path (query + and fragment as `?***`/`#***`), or only `scheme://host/***` under a + credential-like name. Any other credential-like value is hidden as + `NAME=(set; value not shown)`, because a dialog receives the prompt in its + argv; the rest are shown. +- An inherited variable with one of the denied names below (`NODE_OPTIONS` + beside `NODE_AUTH_TOKEN`) is shown verbatim, as it can make the command run + something else. +- Nothing is asked when an inherited variable can't be shown safely. That + means a URL containing `@` (a user or password, or a host that could be + disguised), a `://` value too quoted or spaced to parse, or a + `user:password@host` value. It also means a denied name + that is also credential-like, a value with a control character or over 200 + characters, or a name that is not a plain identifier. The error names the + variable to unset. The credential-like names are a heuristic, so a + credential under some other name would be shown. Only the secret's own + prefix is scanned: npm's registry settings (`npm_config_registry`, `.npmrc`) + are not shown. - The command is shown in full or not at all: one longer than 1000 characters is refused, never truncated. Every command argument and every shown value must be printable — no newline, escape sequence or invisible bidi character diff --git a/tools/secret-prompt/src/secret_prompt/__init__.py b/tools/secret-prompt/src/secret_prompt/__init__.py index c35a5e0f9..8c58445ed 100644 --- a/tools/secret-prompt/src/secret_prompt/__init__.py +++ b/tools/secret-prompt/src/secret_prompt/__init__.py @@ -59,10 +59,26 @@ class PromptBackendError(Exception): _DENIED_PREFIXES = ("LD_", "DYLD_", "PYTHON", "PERL5", "RUBY", "NODE_", "GIT_") # npm's publish credential; it only authenticates, so it is safe to set despite its prefix. _ALLOWED_NAMES = {"NODE_AUTH_TOKEN"} -# An inherited variable's value is shown only when its name says where the secret goes; any -# other may itself be a credential, and a dialog's prompt text is visible in its argv. -_ROUTING_WORDS = ("URL", "REPOSITORY", "REGISTRY", "REGISTRIES", "INDEX", "HOST", "ENDPOINT", "USER") -_SECRET_WORDS = ("TOKEN", "PASS", "SECRET", "KEY", "AUTH", "CREDENTIAL") +# An inherited value is hidden only when its name says it is a credential: a dialog's prompt +# text is visible in its argv, but hiding anything else could hide where the secret goes. +# Matched against whole name segments, so UNAUTHORIZED or KEYRING are not credentials. +_SECRET_SEGMENTS = { + "AUTH", + "BEARER", + "COOKIE", + "CRED", + "CREDENTIAL", + "DSN", + "HOOK", + "JWT", + "PAT", + "PRIVATE", + "PW", + "PWD", + "SESSION", + "WEBHOOK", +} +_SECRET_SUFFIXES = ("TOKEN", "PASS", "PASSWORD", "PASSWD", "PASSPHRASE", "SECRET", "KEY") _HIDDEN_VALUE = "(set; value not shown)" _MAX_TEXT = 200 _MAX_COMMAND = 1000 @@ -250,9 +266,10 @@ def is_plain_text(text: str) -> bool: return len(text) <= _MAX_TEXT and not text.startswith("-") and all(char.isprintable() for char in text) -def is_routing_name(name: str) -> bool: - upper = name.upper() - return any(word in upper for word in _ROUTING_WORDS) and not any(word in upper for word in _SECRET_WORDS) +def is_credential_name(name: str) -> bool: + # Each segment also without one trailing S, for TOKENS or SECRETS; never rstrip, or PASS loses it. + forms = {form for seg in name.upper().split("_") for form in (seg, seg.removesuffix("S"))} + return any(form in _SECRET_SEGMENTS or form.endswith(_SECRET_SUFFIXES) for form in forms) def find_unshowable(command: Sequence[str], extra: Mapping[str, str]) -> str | None: @@ -270,30 +287,53 @@ def find_unshowable(command: Sequence[str], extra: Mapping[str, str]) -> str | N return None -def redact_url(value: str) -> str: - """Show a URL up to its path: the host the operator checks, not a credential carried in it.""" - if "://" not in value: - return value - # urlsplit misreads an unencoded '/' in a password, so any '@' hides the whole URL. - if "@" in value: - return _HIDDEN_VALUE +def split_url(value: str) -> urllib.parse.SplitResult | None: + if any(char.isspace() or char in "\"'" for char in value): + return None try: parts = urllib.parse.urlsplit(value) except ValueError: + return None + return parts if parts.scheme and parts.netloc else None + + +def describe_inherited(name: str, value: str) -> str: + """How an inherited variable appears in the prompt; ValueError when it cannot appear safely.""" + credential = is_credential_name(name) + if is_denied_name(name): + # It can make the command run something else (NODE_OPTIONS, GIT_ASKPASS), so it is shown + # verbatim, or refused when it may also be a credential. + if credential: + raise ValueError(f"the inherited {name} changes how the command runs; unset it") + return value + at_hint = "; unset it, or remove the user and password from it" + parts = split_url(value) + if parts is not None: + # urlsplit misreads an unencoded '/' in a password, so any '@' could hide the real host. + if "@" in value: + raise ValueError(f"the inherited {name} is a URL with a user or password{at_hint}") + # A URL says where the secret may go, so its host is shown whatever the name + # (XSP_TOKEN_URL); under a credential name the path may be the secret itself. + if credential: + return f"{parts.scheme}://{parts.netloc}/***" + shown = urllib.parse.urlunsplit((parts.scheme, parts.netloc, parts.path, "", "")) + return shown + ("?***" if parts.query else "") + ("#***" if parts.fragment else "") + if credential: return _HIDDEN_VALUE - shown = urllib.parse.urlunsplit((parts.scheme, parts.netloc, parts.path, "", "")) - return shown + ("?***" if parts.query else "") + ("#***" if parts.fragment else "") + if "://" in value: + # Quoted or spaced, it is not parsed reliably enough to redact its query or userinfo. + raise ValueError(f"the inherited {name} is not a URL it can show; unset it") + # user:password@proxy:3128 carries a credential without being URL-shaped; me@example.org does not. + if ":" in value.partition("@")[0] and "@" in value: + raise ValueError(f"the inherited {name} holds a user and password{at_hint}") + return value def build_prompt(text: str, command: Sequence[str], extra: Mapping[str, str]) -> str: """The caller's text, then what it cannot change: the command and the variables it gets.""" lines = [text, "", "It will be passed to:", shlex.join(command)] if extra: - lines += [ - "", - "with:", - *(f" {name}={redact_url(value)}" for name, value in sorted(extra.items())), - ] + lines += ["", "with:", *(f" {name}={value}" for name, value in sorted(extra.items()))] return "\n".join(lines) @@ -319,10 +359,11 @@ def main(argv: Sequence[str] | None = None) -> int: if family: for name, value in os.environ.items(): if name.startswith(family) and name != args.env and name not in extra: - # A denied name (NODE_OPTIONS for NODE_AUTH_TOKEN) can make the command run - # something else, so its value is always shown. - shown = is_routing_name(name) or is_denied_name(name) - extra[name] = value if shown else _HIDDEN_VALUE + try: + extra[name] = describe_inherited(name, value) + except ValueError as err: + print(f"secret-prompt: {err}; nothing was asked", file=sys.stderr) + return 1 unshowable = find_unshowable(command, extra) if unshowable: print( diff --git a/tools/secret-prompt/tests/test_secret_prompt.py b/tools/secret-prompt/tests/test_secret_prompt.py index 65b65f2af..eaeecd0f1 100644 --- a/tools/secret-prompt/tests/test_secret_prompt.py +++ b/tools/secret-prompt/tests/test_secret_prompt.py @@ -396,36 +396,100 @@ def test_too_long_command_is_refused_not_truncated( assert "longer than 1000 characters; nothing was asked" in capsys.readouterr().err -def test_inherited_values_are_shown_only_for_routing_names(monkeypatch: pytest.MonkeyPatch) -> None: +def test_inherited_values_are_hidden_only_for_credential_names(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setenv("XSP_UPLOAD_URL", "https://example.invalid/") monkeypatch.setenv("XSP_SECRET_KEY", "s3cr3t") - monkeypatch.setenv("XSP_AUTH_URL", "https://user:pw@example.invalid/") - monkeypatch.setenv("XSP_INDEX_URL", "https://__token__:pypi-Ag/E@upload.example.invalid/legacy/") + monkeypatch.setenv("XSP_AUTH_URL", "https://auth.example.invalid/") + monkeypatch.setenv("XSP_PAT", "ghp_x") + monkeypatch.setenv("XSP_CREDS", "c") + monkeypatch.setenv("XSP_AUTH_CONFIG", '{"registry":"https://r.invalid","token":"js0n"}') + monkeypatch.setenv("XSP_WEBHOOK_SECRET", "https://hooks.invalid/services/hookpath") + monkeypatch.setenv("XSP_SLACK_WEBHOOK", "https://hooks.invalid/services/slackpath") + monkeypatch.setenv("XSP_TLS_REJECT_UNAUTHORIZED", "0") monkeypatch.setenv("XSP_REPOSITORY_URL", "https://example.invalid/simple?token=t0k#frag") - monkeypatch.setenv("XSP_MODE", "fast") - prompt = _captured_prompt(monkeypatch, ["--env", "XSP_TOKEN", "true"]) - assert prompt.splitlines()[-6:] == [ - " XSP_AUTH_URL=(set; value not shown)", - " XSP_INDEX_URL=(set; value not shown)", - " XSP_MODE=(set; value not shown)", + monkeypatch.setenv("XSP_CERT", "/etc/ssl/other.pem") + monkeypatch.setenv("XSP_USERNAME", "rm@example.invalid") + prompt = _captured_prompt( + monkeypatch, ["--env", "XSP_TOKEN", "--set", "XSP_TARGET=https://u@set.invalid/", "true"] + ) + assert prompt.splitlines()[-13:] == [ + " XSP_AUTH_CONFIG=(set; value not shown)", + " XSP_AUTH_URL=https://auth.example.invalid/***", + " XSP_CERT=/etc/ssl/other.pem", + " XSP_CREDS=(set; value not shown)", + " XSP_PAT=(set; value not shown)", " XSP_REPOSITORY_URL=https://example.invalid/simple?***#***", " XSP_SECRET_KEY=(set; value not shown)", + " XSP_SLACK_WEBHOOK=https://hooks.invalid/***", + " XSP_TARGET=https://u@set.invalid/", + " XSP_TLS_REJECT_UNAUTHORIZED=0", " XSP_UPLOAD_URL=https://example.invalid/", + " XSP_USERNAME=rm@example.invalid", + " XSP_WEBHOOK_SECRET=https://hooks.invalid/***", ] - for leaked in ("s3cr3t", "pw@", "pypi-Ag", "t0k", "frag"): + for leaked in ("s3cr3t", "ghp_x", "t0k", "frag", "js0n", "hookpath", "slackpath"): assert leaked not in prompt -def test_inherited_denied_and_index_values_are_shown(monkeypatch: pytest.MonkeyPatch) -> None: +@pytest.mark.parametrize( + ("name", "value", "message"), + [ + ( + "XSP_INDEX_URL", + "https://__token__:pypi-Ag/E@upload.invalid/", + "XSP_INDEX_URL is a URL with a user", + ), + ("XSP_ASKPASS", "/tmp/grab.sh", "XSP_ASKPASS changes how the command runs"), + ( + "XSP_INDEX_URL", + "'https://host.invalid/simple?token=t0k'", + "XSP_INDEX_URL is not a URL it can show", + ), + ( + "XSP_PROXY", + "user:pw@proxy.invalid:3128", + "XSP_PROXY holds a user and password; unset it, or remove", + ), + ( + "XSP_API_TOKEN", + "https://u:p/w@host.invalid/", + "XSP_API_TOKEN is a URL with a user or password; unset it, or remove", + ), + ], +) +def test_inherited_variable_that_cannot_be_shown_safely_is_refused( + name: str, + value: str, + message: str, + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], +) -> None: + monkeypatch.setenv(name, value) + monkeypatch.setattr(secret_prompt, "_DENIED_NAMES", {*secret_prompt._DENIED_NAMES, "XSP_ASKPASS"}) + _answer(monkeypatch, error=AssertionError("must not ask")) + assert main(["--env", "XSP_TOKEN", "--backend", "tty", "--", "true"]) == 1 + assert message in capsys.readouterr().err + + +@pytest.mark.parametrize( + "name", ["MAVEN_GPG_PASSPHRASE", "XSP_SECRETS", "XSP_TOKENS", "XSP_API_KEYS", "XSP_PW", "XSP_PASS"] +) +def test_credential_like_names(name: str) -> None: + assert secret_prompt.is_credential_name(name) + + +@pytest.mark.parametrize("name", ["XSP_TLS_REJECT_UNAUTHORIZED", "XSP_KEYRING_BACKEND", "XSP_PASSES_DIR"]) +def test_names_that_only_contain_a_credential_word(name: str) -> None: + assert not secret_prompt.is_credential_name(name) + + +def test_inherited_denied_values_are_shown(monkeypatch: pytest.MonkeyPatch) -> None: for name in [name for name in os.environ if name.startswith("NODE_")]: monkeypatch.delenv(name) monkeypatch.setenv("NODE_OPTIONS", "--require=/tmp/x.js") - monkeypatch.setenv("NODE_PUBLISH_INDEX", "https://example.invalid/") + monkeypatch.setenv("NODE_PATH", "x:///tmp/evil?q#f") prompt = _captured_prompt(monkeypatch, ["--env", "NODE_AUTH_TOKEN", "true"]) - assert prompt.splitlines()[-2:] == [ - " NODE_OPTIONS=--require=/tmp/x.js", - " NODE_PUBLISH_INDEX=https://example.invalid/", - ] + assert prompt.splitlines()[-2:] == [" NODE_OPTIONS=--require=/tmp/x.js", " NODE_PATH=x:///tmp/evil?q#f"] @pytest.mark.parametrize( From 65138bba71c7166a1dfaf0fba9e109b002dd25d1 Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Sun, 18 Oct 2026 17:01:50 +0200 Subject: [PATCH 5/6] fix(secret-prompt): show an inherited value only when its name routes the secret Showing every inherited value whose name did not look like a credential put any credential under an unexpected name (TOKEN2, a bare token@host) into the dialog's argv. Hiding by default fails closed, and a URL still always shows its host, so where the secret may go stays visible either way. Generated-by: Claude Code (Opus 5) --- tools/secret-prompt/README.md | 35 ++++++++------- .../src/secret_prompt/__init__.py | 43 +++++++++++++++---- .../secret-prompt/tests/test_secret_prompt.py | 42 +++++++++++------- 3 files changed, 80 insertions(+), 40 deletions(-) diff --git a/tools/secret-prompt/README.md b/tools/secret-prompt/README.md index 4c96aca96..4eef4ebd2 100644 --- a/tools/secret-prompt/README.md +++ b/tools/secret-prompt/README.md @@ -116,27 +116,30 @@ Seatbelt sandbox turns off Apple Events — so run the step outside the sandbox before pasting: an agent chooses the command, and this is what stops a look-alike prompt from handing the token to something that prints or forwards it. -- `--set` values are shown as given. An inherited variable's name is - credential-like when one of its `_`-separated segments, with or without a - trailing `S`, is `AUTH`, `CRED`, `CREDENTIAL`, `JWT`, `PAT`, `PW`, `PWD`, - `COOKIE`, `SESSION`, `BEARER`, `PRIVATE`, `WEBHOOK`, `HOOK` or `DSN`, or ends - in `TOKEN`, `PASS`, `PASSWORD`, `PASSWD`, `PASSPHRASE`, `SECRET` or `KEY`. An - inherited URL always shows its host, whatever the name: up to its path (query - and fragment as `?***`/`#***`), or only `scheme://host/***` under a - credential-like name. Any other credential-like value is hidden as - `NAME=(set; value not shown)`, because a dialog receives the prompt in its - argv; the rest are shown. +- `--set` values are shown as given. An inherited value is shown only when its + name has a routing segment (`URL`, `HOST`, `REPOSITORY`, `REGISTRY`, + `INDEX`, `ENDPOINT`, `USER`, `USERNAME`, `CERT`) and no credential-like one; + every other is listed as `NAME=(set; value not shown)`, because a dialog + receives the prompt in its argv. A name is credential-like when a segment, + with or without a trailing `S`, is `AUTH`, `CRED`, `CREDENTIAL`, `JWT`, + `PAT`, `PW`, `PWD`, `COOKIE`, `SESSION`, `BEARER`, `PRIVATE`, `WEBHOOK`, + `HOOK` or `DSN`, or ends in `TOKEN`, `PASS`, `PASSWORD`, `PASSWD`, + `PASSPHRASE`, `SECRET` or `KEY`. +- An inherited URL always shows its host, whatever its name, so where the + secret may go is never hidden: up to its path under a routing name (query and + fragment as `?***`/`#***`), otherwise only `scheme://host/***`. - An inherited variable with one of the denied names below (`NODE_OPTIONS` beside `NODE_AUTH_TOKEN`) is shown verbatim, as it can make the command run something else. - Nothing is asked when an inherited variable can't be shown safely. That means a URL containing `@` (a user or password, or a host that could be - disguised), a `://` value too quoted or spaced to parse, or a - `user:password@host` value. It also means a denied name - that is also credential-like, a value with a control character or over 200 - characters, or a name that is not a plain identifier. The error names the - variable to unset. The credential-like names are a heuristic, so a - credential under some other name would be shown. Only the secret's own + disguised), a `://` value too quoted or spaced to parse, or a routing value + that may carry a credential: an `@` that is not a plain e-mail address, a + `?` or `#`, or a `:` that is not a port (`user:pw@proxy`, `alice:s3cret`, + `host/simple?token=…`). It also + means a denied name that is also credential-like, a shown value with a + control character or over 200 characters, or a name that is not a plain + identifier. The error names the variable to unset. Only the secret's own prefix is scanned: npm's registry settings (`npm_config_registry`, `.npmrc`) are not shown. - The command is shown in full or not at all: one longer than 1000 characters diff --git a/tools/secret-prompt/src/secret_prompt/__init__.py b/tools/secret-prompt/src/secret_prompt/__init__.py index 8c58445ed..40a3a8eac 100644 --- a/tools/secret-prompt/src/secret_prompt/__init__.py +++ b/tools/secret-prompt/src/secret_prompt/__init__.py @@ -28,6 +28,7 @@ import getpass import os import platform +import re import shlex import shutil import subprocess @@ -59,9 +60,21 @@ class PromptBackendError(Exception): _DENIED_PREFIXES = ("LD_", "DYLD_", "PYTHON", "PERL5", "RUBY", "NODE_", "GIT_") # npm's publish credential; it only authenticates, so it is safe to set despite its prefix. _ALLOWED_NAMES = {"NODE_AUTH_TOKEN"} -# An inherited value is hidden only when its name says it is a credential: a dialog's prompt -# text is visible in its argv, but hiding anything else could hide where the secret goes. -# Matched against whole name segments, so UNAUTHORIZED or KEYRING are not credentials. +# A dialog's prompt text is visible in its argv, so an inherited value is shown only when its +# name says where the secret goes and nothing in it suggests a credential. Names are matched +# by whole segments, so UNAUTHORIZED or KEYRING are not credentials. +_ROUTING_SEGMENTS = { + "CERT", + "ENDPOINT", + "HOST", + "INDEX", + "REGISTRY", + "REGISTRIES", + "REPOSITORY", + "URL", + "USER", + "USERNAME", +} _SECRET_SEGMENTS = { "AUTH", "BEARER", @@ -272,6 +285,14 @@ def is_credential_name(name: str) -> bool: return any(form in _SECRET_SEGMENTS or form.endswith(_SECRET_SUFFIXES) for form in forms) +def is_routing_name(name: str) -> bool: + return not is_credential_name(name) and not _ROUTING_SEGMENTS.isdisjoint(name.upper().split("_")) + + +def is_plain_email(value: str) -> bool: + return re.fullmatch(r"[A-Za-z0-9._%+-]+@[A-Za-z0-9-]+(\.[A-Za-z0-9-]+)*\.[A-Za-z]{2,}", value) is not None + + def find_unshowable(command: Sequence[str], extra: Mapping[str, str]) -> str | None: """Name the first shown string that a control or bidi character could make lie, if any.""" # shlex.join quotes a newline, ESC or U+202E but does not escape it, so it would still @@ -312,9 +333,9 @@ def describe_inherited(name: str, value: str) -> str: # urlsplit misreads an unencoded '/' in a password, so any '@' could hide the real host. if "@" in value: raise ValueError(f"the inherited {name} is a URL with a user or password{at_hint}") - # A URL says where the secret may go, so its host is shown whatever the name - # (XSP_TOKEN_URL); under a credential name the path may be the secret itself. - if credential: + # A URL says where the secret may go, so its host is always shown; its path only under a + # routing name, as elsewhere it may be the secret itself (a webhook). + if not is_routing_name(name): return f"{parts.scheme}://{parts.netloc}/***" shown = urllib.parse.urlunsplit((parts.scheme, parts.netloc, parts.path, "", "")) return shown + ("?***" if parts.query else "") + ("#***" if parts.fragment else "") @@ -323,9 +344,13 @@ def describe_inherited(name: str, value: str) -> str: if "://" in value: # Quoted or spaced, it is not parsed reliably enough to redact its query or userinfo. raise ValueError(f"the inherited {name} is not a URL it can show; unset it") - # user:password@proxy:3128 carries a credential without being URL-shaped; me@example.org does not. - if ":" in value.partition("@")[0] and "@" in value: - raise ValueError(f"the inherited {name} holds a user and password{at_hint}") + if not is_routing_name(name): + return _HIDDEN_VALUE + # user:pw@proxy:3128, token@host, alice:s3cret or host/simple?token=... carry a credential; + # me@example.org and proxy:3128 do not. + has_userinfo = "@" in value and not is_plain_email(value) + if has_userinfo or "?" in value or "#" in value or re.search(r":(?!\d+(/|$))", value): + raise ValueError(f"the inherited {name} may hold a user, password or token{at_hint}") return value diff --git a/tools/secret-prompt/tests/test_secret_prompt.py b/tools/secret-prompt/tests/test_secret_prompt.py index eaeecd0f1..fbfc848a9 100644 --- a/tools/secret-prompt/tests/test_secret_prompt.py +++ b/tools/secret-prompt/tests/test_secret_prompt.py @@ -396,38 +396,51 @@ def test_too_long_command_is_refused_not_truncated( assert "longer than 1000 characters; nothing was asked" in capsys.readouterr().err -def test_inherited_values_are_hidden_only_for_credential_names(monkeypatch: pytest.MonkeyPatch) -> None: +def test_inherited_values_are_shown_only_for_routing_names(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setenv("XSP_UPLOAD_URL", "https://example.invalid/") monkeypatch.setenv("XSP_SECRET_KEY", "s3cr3t") - monkeypatch.setenv("XSP_AUTH_URL", "https://auth.example.invalid/") + monkeypatch.setenv("XSP_AUTH_URL", "https://auth.example.invalid/authpath") monkeypatch.setenv("XSP_PAT", "ghp_x") - monkeypatch.setenv("XSP_CREDS", "c") monkeypatch.setenv("XSP_AUTH_CONFIG", '{"registry":"https://r.invalid","token":"js0n"}') - monkeypatch.setenv("XSP_WEBHOOK_SECRET", "https://hooks.invalid/services/hookpath") monkeypatch.setenv("XSP_SLACK_WEBHOOK", "https://hooks.invalid/services/slackpath") - monkeypatch.setenv("XSP_TLS_REJECT_UNAUTHORIZED", "0") + monkeypatch.setenv("XSP_DEST", "https://other.invalid/destpath") + monkeypatch.setenv("XSP_TOKEN2", "t2") + monkeypatch.setenv("XSP_PROXY", "user:pw@proxy.invalid:3128") monkeypatch.setenv("XSP_REPOSITORY_URL", "https://example.invalid/simple?token=t0k#frag") monkeypatch.setenv("XSP_CERT", "/etc/ssl/other.pem") monkeypatch.setenv("XSP_USERNAME", "rm@example.invalid") + monkeypatch.setenv("XSP_HOST", "proxy.invalid:3128") prompt = _captured_prompt( monkeypatch, ["--env", "XSP_TOKEN", "--set", "XSP_TARGET=https://u@set.invalid/", "true"] ) - assert prompt.splitlines()[-13:] == [ + assert prompt.splitlines()[-14:] == [ " XSP_AUTH_CONFIG=(set; value not shown)", " XSP_AUTH_URL=https://auth.example.invalid/***", " XSP_CERT=/etc/ssl/other.pem", - " XSP_CREDS=(set; value not shown)", + " XSP_DEST=https://other.invalid/***", + " XSP_HOST=proxy.invalid:3128", " XSP_PAT=(set; value not shown)", + " XSP_PROXY=(set; value not shown)", " XSP_REPOSITORY_URL=https://example.invalid/simple?***#***", " XSP_SECRET_KEY=(set; value not shown)", " XSP_SLACK_WEBHOOK=https://hooks.invalid/***", " XSP_TARGET=https://u@set.invalid/", - " XSP_TLS_REJECT_UNAUTHORIZED=0", + " XSP_TOKEN2=(set; value not shown)", " XSP_UPLOAD_URL=https://example.invalid/", " XSP_USERNAME=rm@example.invalid", - " XSP_WEBHOOK_SECRET=https://hooks.invalid/***", ] - for leaked in ("s3cr3t", "ghp_x", "t0k", "frag", "js0n", "hookpath", "slackpath"): + for leaked in ( + "s3cr3t", + "ghp_x", + "t0k", + "frag", + "js0n", + "authpath", + "slackpath", + "destpath", + "t2", + "pw@", + ): assert leaked not in prompt @@ -445,11 +458,10 @@ def test_inherited_values_are_hidden_only_for_credential_names(monkeypatch: pyte "'https://host.invalid/simple?token=t0k'", "XSP_INDEX_URL is not a URL it can show", ), - ( - "XSP_PROXY", - "user:pw@proxy.invalid:3128", - "XSP_PROXY holds a user and password; unset it, or remove", - ), + ("XSP_HOST", "tok@proxy.invalid:3128", "XSP_HOST may hold a user, password or token; unset it"), + ("XSP_USER", "user:pw@proxy.invalid", "XSP_USER may hold a user, password or token"), + ("XSP_USER", "alice:s3cret", "XSP_USER may hold a user, password or token"), + ("XSP_INDEX", "host.invalid/simple?token=t0k", "XSP_INDEX may hold a user, password or token"), ( "XSP_API_TOKEN", "https://u:p/w@host.invalid/", From d2ccc51d7365f464d5963d3400fa8b6e6b6c88b0 Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Sun, 18 Oct 2026 20:15:12 +0200 Subject: [PATCH 6/6] fix(secret-prompt): answer the two CodeQL findings - main() ended in `try: os.execve(...)` with the only return in its `except`, so the end of the function looked reachable and returned None (py/mixed-returns). execve replaces the process or raises; the 127 is now returned after the try, so every path returns explicitly. - The tests imported secret_prompt both as a module and with `from ... import` (py/import-and-import-from). They import the names they use, and patch module attributes by their dotted path. Generated-by: Claude Opus 5 --- .../src/secret_prompt/__init__.py | 4 +-- .../secret-prompt/tests/test_secret_prompt.py | 32 +++++++++++-------- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/tools/secret-prompt/src/secret_prompt/__init__.py b/tools/secret-prompt/src/secret_prompt/__init__.py index 40a3a8eac..187f284a5 100644 --- a/tools/secret-prompt/src/secret_prompt/__init__.py +++ b/tools/secret-prompt/src/secret_prompt/__init__.py @@ -433,7 +433,7 @@ def main(argv: Sequence[str] | None = None) -> int: env.update(item.split("=", 1) for item in args.set) env[args.env] = secret try: - os.execve(command[0], command, env) + os.execve(command[0], command, env) # replaces this process; returns only by raising except OSError as err: print(f"secret-prompt: cannot run {command[0]}: {err.strerror}", file=sys.stderr) - return 127 + return 127 diff --git a/tools/secret-prompt/tests/test_secret_prompt.py b/tools/secret-prompt/tests/test_secret_prompt.py index fbfc848a9..d2cd26a3b 100644 --- a/tools/secret-prompt/tests/test_secret_prompt.py +++ b/tools/secret-prompt/tests/test_secret_prompt.py @@ -30,13 +30,17 @@ import pytest -import secret_prompt from secret_prompt import ( + _DENIED_NAMES, + ASKERS, PromptBackendError, PromptCancelledError, + ask_kdialog, ask_osascript, ask_pinentry, + ask_zenity, candidate_backends, + is_credential_name, main, ) @@ -99,7 +103,7 @@ def ask(prompt: str, title: str) -> str | None: raise error return value - monkeypatch.setitem(secret_prompt.ASKERS, "tty", ask) + monkeypatch.setitem(ASKERS, "tty", ask) def test_runs_command_with_secret_and_extra_variables( @@ -191,9 +195,9 @@ def test_auto_falls_back_when_a_dialog_cannot_show( def broken(prompt: str, title: str) -> str: raise PromptBackendError("osascript failed: Not authorized to send Apple events") - monkeypatch.setattr(secret_prompt, "candidate_backends", lambda *args: ["osascript", "pinentry"]) - monkeypatch.setitem(secret_prompt.ASKERS, "osascript", broken) - monkeypatch.setitem(secret_prompt.ASKERS, "pinentry", lambda prompt, title: "pypi-fallback") + monkeypatch.setattr("secret_prompt.candidate_backends", lambda *args: ["osascript", "pinentry"]) + monkeypatch.setitem(ASKERS, "osascript", broken) + monkeypatch.setitem(ASKERS, "pinentry", lambda prompt, title: "pypi-fallback") main(["--env", "T", "--", "true"]) [(_, _, env)] = exec_calls assert env["T"] == "pypi-fallback" @@ -208,14 +212,14 @@ def cancelled(prompt: str, title: str) -> str: raise PromptCancelledError("prompt cancelled") asked: list[str] = [] - monkeypatch.setattr(secret_prompt, "candidate_backends", lambda *args: ["osascript", "pinentry"]) - monkeypatch.setitem(secret_prompt.ASKERS, "osascript", cancelled) + monkeypatch.setattr("secret_prompt.candidate_backends", lambda *args: ["osascript", "pinentry"]) + monkeypatch.setitem(ASKERS, "osascript", cancelled) def never_asked(prompt: str, title: str) -> str: asked.append("pinentry") return "x" - monkeypatch.setitem(secret_prompt.ASKERS, "pinentry", never_asked) + monkeypatch.setitem(ASKERS, "pinentry", never_asked) assert main(["--env", "T", "--", "true"]) == 1 assert asked == [] assert exec_calls == [] @@ -325,7 +329,7 @@ def ask(prompt: str, title: str) -> str: prompts.append(prompt) return "pypi-abc" - monkeypatch.setitem(secret_prompt.ASKERS, "tty", ask) + monkeypatch.setitem(ASKERS, "tty", ask) monkeypatch.setattr(os, "execve", lambda file, args, env: None) main([*argv[:-1], "--backend", "tty", "--", *argv[-1].split("|")]) [prompt] = prompts @@ -477,7 +481,7 @@ def test_inherited_variable_that_cannot_be_shown_safely_is_refused( capsys: pytest.CaptureFixture[str], ) -> None: monkeypatch.setenv(name, value) - monkeypatch.setattr(secret_prompt, "_DENIED_NAMES", {*secret_prompt._DENIED_NAMES, "XSP_ASKPASS"}) + monkeypatch.setattr("secret_prompt._DENIED_NAMES", {*_DENIED_NAMES, "XSP_ASKPASS"}) _answer(monkeypatch, error=AssertionError("must not ask")) assert main(["--env", "XSP_TOKEN", "--backend", "tty", "--", "true"]) == 1 assert message in capsys.readouterr().err @@ -487,12 +491,12 @@ def test_inherited_variable_that_cannot_be_shown_safely_is_refused( "name", ["MAVEN_GPG_PASSPHRASE", "XSP_SECRETS", "XSP_TOKENS", "XSP_API_KEYS", "XSP_PW", "XSP_PASS"] ) def test_credential_like_names(name: str) -> None: - assert secret_prompt.is_credential_name(name) + assert is_credential_name(name) @pytest.mark.parametrize("name", ["XSP_TLS_REJECT_UNAUTHORIZED", "XSP_KEYRING_BACKEND", "XSP_PASSES_DIR"]) def test_names_that_only_contain_a_credential_word(name: str) -> None: - assert not secret_prompt.is_credential_name(name) + assert not is_credential_name(name) def test_inherited_denied_values_are_shown(monkeypatch: pytest.MonkeyPatch) -> None: @@ -541,7 +545,7 @@ def test_npm_auth_token_is_allowed( assert env["NODE_AUTH_TOKEN"] == "npm_abc" -@pytest.mark.parametrize("asker", [secret_prompt.ask_zenity, secret_prompt.ask_kdialog]) +@pytest.mark.parametrize("asker", [ask_zenity, ask_kdialog]) def test_markup_capable_dialogs_refuse_markup_characters(asker: object) -> None: with pytest.raises(PromptBackendError, match="markup"): asker("Paste it", "PyPI upload") # type: ignore[operator] @@ -572,4 +576,4 @@ def test_zenity_tells_cancel_from_a_missing_display( ) -> None: monkeypatch.setattr(subprocess, "run", lambda cmd, **kwargs: result) with pytest.raises(error): - secret_prompt.ask_zenity("Paste it", "PyPI upload") + ask_zenity("Paste it", "PyPI upload")