From eeaf9c262c5e0b08e8f73da61f073a452155e6b8 Mon Sep 17 00:00:00 2001 From: Vardhman Gupta Date: Tue, 20 Oct 2026 19:50:57 +0530 Subject: [PATCH 1/2] fix(vulnogram): exit cleanly without a TTY and point to --pmc (#1733) When executed from an agent shell escape or non-interactive environment, the default paste flow of vulnogram-api-setup raised EOFError with an unhandled traceback. - Catch EOFError/KeyboardInterrupt and detect non-interactive stdin in setup_session.py to exit cleanly with code 1 and actionable instructions - Protect the ASF account prompt in resolve_from_address against EOFError - Mention --pmc first in expired-token hints across client.py, push.py, skill guidance, and documentation - Add unit tests covering non-interactive stdin and EOF handling Generated-by: Antigravity --- .../skills/issue-sync/apply-and-push.md | 4 +- .../skills/issue-sync/gather.md | 4 +- tools/cve-tool-vulnogram/oauth-api/README.md | 4 +- .../oauth-api/src/vulnogram_api/client.py | 3 +- .../src/vulnogram_api/setup_session.py | 25 +++++-- .../oauth-api/tests/test_setup_session.py | 70 +++++++++++++++++++ tools/cve-tool-vulnogram/record.md | 4 +- .../security_management/issue_sync/push.py | 2 +- 8 files changed, 102 insertions(+), 14 deletions(-) diff --git a/plugins/magpie-security/skills/issue-sync/apply-and-push.md b/plugins/magpie-security/skills/issue-sync/apply-and-push.md index 58fea1202..9c1bf4df5 100644 --- a/plugins/magpie-security/skills/issue-sync/apply-and-push.md +++ b/plugins/magpie-security/skills/issue-sync/apply-and-push.md @@ -321,7 +321,7 @@ Capture the printed URL — it deep-links to the `## CVE JSON — paste-ready fo **When the operator's machine has a valid authenticated session** for the adapter named in `cve_authority.tool` (one-time setup per `tools//README.md`; for Vulnogram, -`uv run --project /tools/cve-tool-vulnogram/oauth-api vulnogram-api-setup`, see +`uv run --project /tools/cve-tool-vulnogram/oauth-api vulnogram-api-setup --pmc `, see [`tools/cve-tool-vulnogram/oauth-api/README.md`](../../../../tools/cve-tool-vulnogram/oauth-api/README.md)), **sync pushes the JSON to the record directly** through the adapter's `push_update(cve_id, fields, state_transition=None)` method (per [`tools/cve-tool/README.md`](../../../../tools/cve-tool/README.md)) @@ -461,7 +461,7 @@ the four gates that need judgement (trigger conditions, the incomplete-fix claus - **`valid`** → proceed to step 3. - **`expired`** → skip the push and surface a one-line reminder in the Step 6 recap: - *"CVE-tool authenticated session expired — re-run the adapter's setup entrypoint (for the Vulnogram adapter, `vulnogram-api-setup`) to restore automatic push; using manual-paste hand-off this run."* + *"CVE-tool authenticated session expired — re-run the adapter's setup entrypoint (for the Vulnogram adapter, `vulnogram-api-setup --pmc `) to restore automatic push; using manual-paste hand-off this run."* Use the manual-paste hand-off variant for any 5c comment work below. - **`not-configured`** → skip the push; the manual-paste hand-off (via the `cve_authority.source_tab_url_template` link) still works. diff --git a/plugins/magpie-security/skills/issue-sync/gather.md b/plugins/magpie-security/skills/issue-sync/gather.md index 6d0877e1a..46a9d9f09 100644 --- a/plugins/magpie-security/skills/issue-sync/gather.md +++ b/plugins/magpie-security/skills/issue-sync/gather.md @@ -311,7 +311,7 @@ check whether a CVE reviewer has left comments on the record. **Read the record's comments directly.** When the CVE-tool adapter exposes an authenticated record read — for the ASF default, [`vulnogram-api-record-fetch`](../../../../tools/cve-tool-vulnogram/oauth-api/README.md), -on the Bearer token stored by `vulnogram-api-setup` — +on the Bearer token stored by `vulnogram-api-setup --pmc ` — the reviewer comments are a **first-class field on the record** and that read is the primary signal path: ```bash @@ -332,7 +332,7 @@ Run it for **every** tracker with an allocated CVE, not only those in `REVIEW` reviewers comment on records that have moved on, and a comment on a `PUBLIC` record still needs an answer. > **Which records to poll.** One cheap, read-only fetch per tracker with an allocated CVE; it needs a live token. -> On an expired-token error, re-run `vulnogram-api-setup` before continuing; if that is not possible this run, +> On an expired-token error, re-run `vulnogram-api-setup --pmc ` before continuing; if that is not possible this run, > fall back to the mail path below and flag the gap in Step 2c rather than reporting *"no reviewer comments"* — > a failed read is not an empty result. diff --git a/tools/cve-tool-vulnogram/oauth-api/README.md b/tools/cve-tool-vulnogram/oauth-api/README.md index a37d4e2fd..54b3b1690 100644 --- a/tools/cve-tool-vulnogram/oauth-api/README.md +++ b/tools/cve-tool-vulnogram/oauth-api/README.md @@ -32,7 +32,7 @@ Console scripts: | Console script | Purpose | |---|---| -| `vulnogram-api-setup` | Interactive token capture — open `/users/token` in a regular browser (logging in there, MFA included, if needed), copy a token, paste it back into the script. | +| `vulnogram-api-setup` | Interactive token capture — approved in your browser via `--pmc ` (recommended) or copied from `/users/token` and pasted back. | | `vulnogram-api-record-update` | POST a CVE JSON to `/cve5/` (the upsert endpoint). Replaces the release-manager copy-paste-into-`#source` step. | | `vulnogram-api-record-publish` | Move a record `REVIEW` → `PUBLIC` by re-POSTing it with the new `CNA_private.state`. | | `vulnogram-api-record-fetch` | Read a record back — full stored JSON, or just `CNA_private.state` (`--state-only`), or just the reviewer-comment array (`--comments-only`). Read-only; never POSTs. | @@ -225,7 +225,7 @@ When the token has expired, the next API call returns `SessionExpired`: ```text ✗ Vulnogram redirected to the login page — the Bearer token is expired or invalid. - Log in again in the browser and re-run `vulnogram-api-setup` with the new token from /users/token. + Re-run `vulnogram-api-setup --pmc ` (recommended) or log in again in the browser and re-run `vulnogram-api-setup` with the new token from /users/token. ``` Re-running setup is a 30-second operation: open `/users/token`, log in again if prompted, copy the new token, paste it back. diff --git a/tools/cve-tool-vulnogram/oauth-api/src/vulnogram_api/client.py b/tools/cve-tool-vulnogram/oauth-api/src/vulnogram_api/client.py index b0215ec96..3f6063b55 100644 --- a/tools/cve-tool-vulnogram/oauth-api/src/vulnogram_api/client.py +++ b/tools/cve-tool-vulnogram/oauth-api/src/vulnogram_api/client.py @@ -61,7 +61,8 @@ _EXPIRED_HINT = ( "Vulnogram redirected to the login page — the Bearer token is expired or invalid. " - "Log in again in the browser and re-run `vulnogram-api-setup` with the new token from /users/token." + "Re-run `vulnogram-api-setup --pmc ` (recommended) or log in again in the browser " + "and re-run `vulnogram-api-setup` with the new token from /users/token." ) diff --git a/tools/cve-tool-vulnogram/oauth-api/src/vulnogram_api/setup_session.py b/tools/cve-tool-vulnogram/oauth-api/src/vulnogram_api/setup_session.py index 1918b0bac..7052b4975 100644 --- a/tools/cve-tool-vulnogram/oauth-api/src/vulnogram_api/setup_session.py +++ b/tools/cve-tool-vulnogram/oauth-api/src/vulnogram_api/setup_session.py @@ -150,7 +150,12 @@ def resolve_from_address( ) for _attempt in range(3): - answer = prompter("ASF account: ").strip() + try: + answer = prompter("ASF account: ").strip() + except (EOFError, KeyboardInterrupt): + raise SystemExit( + "\n✗ No terminal for the ASF account prompt — re-run with `--from-address @apache.org`." + ) from None if not answer: print(" Empty — required for *.apache.org hosts. Try again.") continue @@ -364,7 +369,18 @@ def _setup_one(args: argparse.Namespace) -> int: _print_walkthrough(args.host, from_address) token = args.token if not token: - token = getpass.getpass("Paste token: ").strip() + if not sys.stdin.isatty(): + raise SystemExit( + "✗ No terminal for the paste prompt — re-run with `--pmc ` " + "(or pass `--token `)." + ) + try: + token = getpass.getpass("Paste token: ").strip() + except (EOFError, KeyboardInterrupt): + raise SystemExit( + "\n✗ No terminal for the paste prompt — re-run with `--pmc ` " + "(or pass `--token `)." + ) from None if not token: raise SystemExit("Empty token — aborting; nothing written.") @@ -414,8 +430,9 @@ def _setup_one(args: argparse.Namespace) -> int: print( "✗ Vulnogram redirected to the login page. The token was either " "copied incompletely or has already expired (it lives only as long " - "as the browser login that minted it). Log in again, open " - "/users/token for a new token, and re-run setup.", + "as the browser login that minted it). Re-run with " + "`vulnogram-api-setup --pmc ` (recommended) or log in again, " + "open /users/token for a new token, and re-run setup.", file=sys.stderr, ) return 2 diff --git a/tools/cve-tool-vulnogram/oauth-api/tests/test_setup_session.py b/tools/cve-tool-vulnogram/oauth-api/tests/test_setup_session.py index 0d8a9bb97..2ae735df2 100644 --- a/tools/cve-tool-vulnogram/oauth-api/tests/test_setup_session.py +++ b/tools/cve-tool-vulnogram/oauth-api/tests/test_setup_session.py @@ -29,6 +29,15 @@ def _server_supports_browser_tokens(monkeypatch): monkeypatch.setattr(setup_session, "probe_server", lambda host: "supported") +@pytest.fixture(autouse=True) +def _interactive_stdin(monkeypatch): + """Default stdin to an interactive terminal so interactive prompt tests work; + tests covering non-interactive execution override it.""" + import sys + + monkeypatch.setattr(sys.stdin, "isatty", lambda: True) + + def test_skip_validate_writes_file(tmp_path, monkeypatch, capsys): out = tmp_path / "session.json" rc = setup_session.main( @@ -447,3 +456,64 @@ def test_auto_scope_without_the_browser_flow_pastes_one_token(tmp_path, monkeypa def test_auto_scope_rejects_a_single_out_path(tmp_path): with pytest.raises(SystemExit, match="--scope"): setup_session.main(["--pmc", "airflow", "--out", str(tmp_path / "s.json")]) + + +def test_paste_prompt_without_tty_aborts_cleanly(tmp_path, monkeypatch): + """When stdin has no controlling terminal, the paste flow must abort + cleanly with a hint to re-run with --pmc, without any traceback.""" + import sys + + monkeypatch.setattr(sys.stdin, "isatty", lambda: False) + out = tmp_path / "session.json" + with pytest.raises(SystemExit) as excinfo: + setup_session.main( + [ + "--out", + str(out), + "--from-address", + "you@apache.org", + "--skip-validate", + ] + ) + msg = str(excinfo.value) + assert "No terminal for the paste prompt" in msg + assert "--pmc " in msg + assert not out.exists() + + +def test_paste_prompt_eof_aborts_cleanly(tmp_path, monkeypatch): + """When getpass encounters EOF, the script must abort cleanly + with a hint to re-run with --pmc instead of an unhandled traceback.""" + + def fake_getpass(*_): + raise EOFError("simulated EOF") + + monkeypatch.setattr(setup_session.getpass, "getpass", fake_getpass) + out = tmp_path / "session.json" + with pytest.raises(SystemExit) as excinfo: + setup_session.main( + [ + "--out", + str(out), + "--from-address", + "you@apache.org", + "--skip-validate", + ] + ) + msg = str(excinfo.value) + assert "No terminal for the paste prompt" in msg + assert "--pmc " in msg + assert not out.exists() + + +def test_resolve_from_address_eof_aborts_cleanly(): + """When prompter encounters EOF, resolve_from_address must abort cleanly.""" + + def fake_prompter(_): + raise EOFError("simulated EOF") + + with pytest.raises(SystemExit) as excinfo: + setup_session.resolve_from_address("cveprocess.apache.org", None, prompter=fake_prompter) + msg = str(excinfo.value) + assert "No terminal for the ASF account prompt" in msg + assert "--from-address" in msg diff --git a/tools/cve-tool-vulnogram/record.md b/tools/cve-tool-vulnogram/record.md index d7f28c2f7..e2da269e5 100644 --- a/tools/cve-tool-vulnogram/record.md +++ b/tools/cve-tool-vulnogram/record.md @@ -78,10 +78,10 @@ outcomes drive the proposal: - **`valid` (exit 0)** — propose the API one-liner; the copy-paste recipe stays available as a one-line *"or open `#source` and paste"* fallback for users who want the visual confirmation. -- **`expired` (exit 1)** — propose `vulnogram-api-setup` first and +- **`expired` (exit 1)** — propose `vulnogram-api-setup --pmc ` first and the API one-liner second, with the copy-paste recipe as the fallback if the operator wants to skip re-setup for this CVE. -- **`not-configured` (exit 2)** — propose `vulnogram-api-setup` +- **`not-configured` (exit 2)** — propose `vulnogram-api-setup --pmc ` with a one-line *"or skip setup and use the copy-paste flow below"* hint. **Do not** force setup on an unwilling operator. diff --git a/tools/security-management/src/security_management/issue_sync/push.py b/tools/security-management/src/security_management/issue_sync/push.py index 57d010347..4a7e8cd5c 100644 --- a/tools/security-management/src/security_management/issue_sync/push.py +++ b/tools/security-management/src/security_management/issue_sync/push.py @@ -147,7 +147,7 @@ def out(action: str, why: str, **extra: Any) -> dict[str, Any]: ) if probe in ("expired", "not-configured"): why = ( - "the session expired: remind the operator to re-run vulnogram-api-setup; manual-paste hand-off this run" + "the session expired: remind the operator to re-run vulnogram-api-setup --pmc ; manual-paste hand-off this run" if probe == "expired" else "no session is configured: manual-paste hand-off through the source tab" ) From 46476832090f5fdc9b7f315617c27a4656b9306f Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Tue, 20 Oct 2026 22:14:41 +0200 Subject: [PATCH 2/2] fix(vulnogram): rely on EOFError alone and keep the --pmc hint from looping getpass reads from /dev/tty before stdin, so a non-tty stdin does not mean there is no terminal: drop the isatty() pre-check that rejected those runs. After a --pmc fallback to the paste prompt, point at --token rather than back at --pmc, and let Ctrl-C exit as before instead of reporting "No terminal". Generated-by: Claude Opus 5 --- tools/cve-tool-vulnogram/oauth-api/README.md | 3 +- .../src/vulnogram_api/setup_session.py | 21 ++++++------ .../oauth-api/tests/test_setup_session.py | 33 +++++++------------ 3 files changed, 23 insertions(+), 34 deletions(-) diff --git a/tools/cve-tool-vulnogram/oauth-api/README.md b/tools/cve-tool-vulnogram/oauth-api/README.md index 54b3b1690..507cea850 100644 --- a/tools/cve-tool-vulnogram/oauth-api/README.md +++ b/tools/cve-tool-vulnogram/oauth-api/README.md @@ -228,7 +228,8 @@ When the token has expired, the next API call returns `SessionExpired`: Re-run `vulnogram-api-setup --pmc ` (recommended) or log in again in the browser and re-run `vulnogram-api-setup` with the new token from /users/token. ``` -Re-running setup is a 30-second operation: open `/users/token`, log in again if prompted, copy the new token, paste it back. +Re-running setup is a 30-second operation: with `--pmc `, approve the new token in the browser; +without it, open `/users/token`, log in again if prompted, copy the new token, paste it back. The agentic skills detect `expired` automatically via `vulnogram-api-check` and surface the re-setup hint in the same proposal they would otherwise propose the API path in. diff --git a/tools/cve-tool-vulnogram/oauth-api/src/vulnogram_api/setup_session.py b/tools/cve-tool-vulnogram/oauth-api/src/vulnogram_api/setup_session.py index 7052b4975..b5b9cd985 100644 --- a/tools/cve-tool-vulnogram/oauth-api/src/vulnogram_api/setup_session.py +++ b/tools/cve-tool-vulnogram/oauth-api/src/vulnogram_api/setup_session.py @@ -152,7 +152,7 @@ def resolve_from_address( for _attempt in range(3): try: answer = prompter("ASF account: ").strip() - except (EOFError, KeyboardInterrupt): + except EOFError: raise SystemExit( "\n✗ No terminal for the ASF account prompt — re-run with `--from-address @apache.org`." ) from None @@ -369,18 +369,17 @@ def _setup_one(args: argparse.Namespace) -> int: _print_walkthrough(args.host, from_address) token = args.token if not token: - if not sys.stdin.isatty(): - raise SystemExit( - "✗ No terminal for the paste prompt — re-run with `--pmc ` " - "(or pass `--token `)." - ) try: token = getpass.getpass("Paste token: ").strip() - except (EOFError, KeyboardInterrupt): - raise SystemExit( - "\n✗ No terminal for the paste prompt — re-run with `--pmc ` " - "(or pass `--token `)." - ) from None + except EOFError: + # With --pmc we only get here when the server has no browser + # approval, so pointing back at --pmc would loop. + hint = ( + "pass `--token `" + if args.pmc + else "re-run with `--pmc ` (or pass `--token `)" + ) + raise SystemExit(f"\n✗ No terminal for the paste prompt — {hint}.") from None if not token: raise SystemExit("Empty token — aborting; nothing written.") diff --git a/tools/cve-tool-vulnogram/oauth-api/tests/test_setup_session.py b/tools/cve-tool-vulnogram/oauth-api/tests/test_setup_session.py index 2ae735df2..d4e7b804d 100644 --- a/tools/cve-tool-vulnogram/oauth-api/tests/test_setup_session.py +++ b/tools/cve-tool-vulnogram/oauth-api/tests/test_setup_session.py @@ -29,15 +29,6 @@ def _server_supports_browser_tokens(monkeypatch): monkeypatch.setattr(setup_session, "probe_server", lambda host: "supported") -@pytest.fixture(autouse=True) -def _interactive_stdin(monkeypatch): - """Default stdin to an interactive terminal so interactive prompt tests work; - tests covering non-interactive execution override it.""" - import sys - - monkeypatch.setattr(sys.stdin, "isatty", lambda: True) - - def test_skip_validate_writes_file(tmp_path, monkeypatch, capsys): out = tmp_path / "session.json" rc = setup_session.main( @@ -458,26 +449,24 @@ def test_auto_scope_rejects_a_single_out_path(tmp_path): setup_session.main(["--pmc", "airflow", "--out", str(tmp_path / "s.json")]) -def test_paste_prompt_without_tty_aborts_cleanly(tmp_path, monkeypatch): - """When stdin has no controlling terminal, the paste flow must abort - cleanly with a hint to re-run with --pmc, without any traceback.""" - import sys +def test_paste_prompt_eof_after_pmc_fallback_does_not_suggest_pmc(tmp_path, monkeypatch): + """With --pmc on a server without browser approval, the paste prompt is + the fallback; its no-terminal hint must not send the operator back to --pmc.""" - monkeypatch.setattr(sys.stdin, "isatty", lambda: False) + def fake_getpass(*_): + raise EOFError("simulated EOF") + + monkeypatch.setattr(setup_session, "probe_server", lambda host: "unsupported") + monkeypatch.setattr(setup_session.getpass, "getpass", fake_getpass) out = tmp_path / "session.json" with pytest.raises(SystemExit) as excinfo: setup_session.main( - [ - "--out", - str(out), - "--from-address", - "you@apache.org", - "--skip-validate", - ] + ["--pmc", "airflow", "--scope", "write", "--out", str(out), "--from-address", "you@apache.org"] ) msg = str(excinfo.value) assert "No terminal for the paste prompt" in msg - assert "--pmc " in msg + assert "--token " in msg + assert "--pmc" not in msg assert not out.exists()