Repository navigation
ENG-8830 fix(network-ops): make certipy stdin confirmation uniform and hang-safe - #179
Merged
Merged
Conversation
…fe (ENG-8830) The certipy_* tools were split on auto-answering certipy's interactive prompts: certipy_auth/cert/req (and the high-level request/auth helpers) piped "y" to stdin, while certipy_account/ca/find/template passed input=None. The None path leaves stdin inherited, so when certipy prompts (e.g. overwrite a saved PFX/ccache) the tool blocks until the 20s command timeout and surfaces as a timeout failure. Make every certipy_* tool default to the same confirmation via a single, documented _CONFIRM_INPUT constant, so stdin is always written and closed and no tool can hang on a prompt. Behavior stays overridable through the existing family-wide `input` parameter (pass "n" to decline); an empty string falls back to the confirm default because execute() only closes stdin for a truthy input. The relay action is exempt — it is a streaming listener that never reads stdin. Bumps network-ops to 2.1.11 and adds regression tests covering the uniform default, explicit/empty overrides, and the relay exemption.
…orm-confirm # Conflicts: # capabilities/network-ops/capability.yaml
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
The
certipy_*tools were inconsistent about answering certipy's interactive prompts.certipy_auth/certipy_cert/certipy_req(and the high-levelcertipy_request_certificate/certipy_certificate_authhelpers) piped"y"to stdin;certipy_account/certipy_ca/certipy_find/certipy_templatepassedinput=None.input=Noneleaves the child's stdin inherited, so when certipy prompts (e.g. "overwrite the saved PFX/ccache?") the tool blocks until the 20s command timeout and surfaces as a timeout failure rather than doing anything useful.This makes every
certipy_*tool default to the same confirmation through one documented_CONFIRM_INPUTconstant, so stdin is always written and closed and no tool can hang on a prompt.Why this shape
input: str | Noneparameter rather than switching to aconfirm: bool, so certipy stays uniform with its siblings (impacket,bloodyad,coercion,krbrelayx), which all takeinputand guard the same prompt-hang this way.execute()only writes-and-closes stdin for a truthy input, so forwarding""would reopen the hang. Callers decline by passing an explicit answer like"n"; the docstrings say so.relayaction is exempt — it's a streaming listener that never reads stdin.Changes
tools/certipy.py: single_CONFIRM_INPUTdefault applied at every non-relayexecute()call; removed the two hardcodedinput="y"; uniform signatures; docstrings describe the default and how to override.capability.yaml: 2.1.10 → 2.1.11 (behavior change to the public tool surface).tests/test_tool_fixes.py: regression tests for the uniform default, explicit/empty overrides, and the relay exemption.Testing
pytest tests/test_tool_fixes.py— 84 passed.pre-commit(ruff, ruff-format, gitleaks) clean;dreadnode capability validatepasses.input=Nonehangs to the timeout while the new"y"default returns immediately.