fix(cli): non-interactive stdin auto-confirms irreversible admin and vote commands - #1650
Open
RealDiligent wants to merge 1 commit into
Open
Conversation
…irmation
`confirm_or_abort` returned True whenever stdin was not a TTY, so every
destructive issue-bounty command self-confirmed when run unattended.
The affected commands transfer contract ownership (documented in-code as
"IRREVERSIBLE. A mistyped address makes the contract unrecoverable"), pay out
and cancel bounties, reset active bounty amounts, and add/remove hotkeys from
the validator consensus whitelist. Any of them run under systemd, pm2, cron,
`docker run` without -it, a shell pipeline, or a non-interactive ssh command
executed immediately with no confirmation — which also made the documented
`--yes` opt-in ("Skip confirmation prompt (non-interactive/CI)") a no-op.
Fail closed instead: `--yes` remains the only way to skip the prompt, and a
non-interactive stdin exits 1 naming the action and the flag to pass. This
matches the convention used by apt, docker, terraform, gh, and btcli.
`issues register`, which spends real ALPHA, carried an inline copy of the same
fail-open expression; it now routes through the shared helper.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
confirm_or_abort— the confirmation gate in front of every destructive issue-bounty command — returnsTruewhenever stdin is not a TTY:So the safeguard silently disappears in exactly the environments where a mistake is least recoverable. These commands run unattended all the time:
systemd,pm2,cron,docker runwithout-it, a shell pipeline, or a non-interactivessh host 'gitt ...'.Eight destructive call sites are affected:
admin set-owneradmin set-treasuryadmin payout-issueadmin cancel-issueadmin add-vali/remove-valivote solution/vote cancelissues register(which spends real ALPHA) carried an inline copy of the same expression.This also made the documented opt-in a no-op —
--yesis advertised as "Skip confirmation prompt (non-interactive/CI)", but non-interactive runs already skipped it, so passing the flag changed nothing.Root cause
confirm_or_abortcollapses two distinct cases into one branch: "the operator explicitly approved" (yes) and "nobody can be asked" (no TTY). The second is treated as approval — a fail-open default on irreversible operations.Fix
Fail closed.
--yesremains the only way to skip the prompt; a non-interactive stdin now exits1, naming both the action and the flag to pass. This is the convention used byapt,docker,terraform,gh, andbtcli.issues registernow routes through the shared helper instead of duplicating the fail-open expression.Scope is 2 source files, +8/−4 lines of logic.
CLI output evidence
Literal terminal captures of
echo | gitt admin set-owner 5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY(piped stdin ⇒ not a TTY, no--yes).Before — the confirmation never happens. Execution runs straight past the gate into wallet/contract-client loading; in this capture environment it stops only because an optional dependency is absent. On an operator machine with dependencies installed, this call submits the ownership transfer:
After — execution stops at the gate and names the flag required to proceed:
With
--yesthe behaviour is unchanged from before this PR (prompt skipped, command proceeds).Related Issues
None open — this behaviour has not been reported previously.
Type of Change
Testing
New
tests/cli/test_confirm_or_abort.py(8 cases) covers the interactive accept/decline paths, both--yespaths, and the three fail-closed cases. The fail-closed tests were verified to fail against the unpatched helper and pass with it.Full CI on this branch: 968 tests passed;
ruff lint,ruff format,pyright, andvultureall pass with no auto-format changes.One existing test (
test_vote_solution_import_error_exits_non_zero) reached its import-error path only because non-TTY stdin self-confirmed —CliRunnerstdin is never a TTY. It now passes--yes, preserving the test's original intent.Risk / tradeoffs
Deliberately a behaviour change: a script that today relies on the implicit non-TTY skip will now exit
1instead of proceeding. That is the point of the fix, and the error message states the one-flag remedy. Anyone already following the documented--yespattern sees no change.Checklist
🤖 Generated with Claude Code