Repository navigation
Add apm doctor tls trust-source diagnostic + test-suite ssl-isolation fixture #2035
Description
Context
Fast-follow from the #2005 / #2004 OS-trust-store work. Two observability/hygiene items the red team surfaced that were out of scope for the blocker fixes:
1. apm doctor tls (or apm doctor TLS section)
#2005 added a verbose trust-source log line ([i] TLS: verifying against OS trust store (truststore) / ... bundled CA (certifi fallback) / ... explicit CA bundle in use / ... disabled). That is the shippable minimum, but a dedicated diagnostic would let enterprise users self-serve when TLS fails:
- Print the active trust source (OS store / certifi fallback / explicit bundle / disabled).
- Print the resolved precedence (
APM_DISABLE_TRUSTSTORE,REQUESTS_CA_BUNDLE/CURL_CA_BUNDLE, futureAPM_EXTRA_CA_BUNDLE, OS store, certifi). - Optionally probe a URL and report which CA source verified it.
- Surface
APM_DISABLE_TRUSTSTORE=1in the TLS failure message (currently only OS store /REQUESTS_CA_BUNDLEare mentioned).
2. Test-suite ssl-isolation fixture
Importing apm_cli.cli (and now configure_tls_trust()) monkeypatches process-global ssl.SSLContext. ~110 tests import the CLI, so global SSL state can leak across tests (the #2005 verifier already needed a local truststore.extract_from_ssl() guard + autouse isolation in the TLS integration suites). Promote this to a shared, autouse conftest fixture that snapshots/restores ssl (extract_from_ssl) around tests, so no suite silently inherits an injected global.
Acceptance
apm doctorreports the active TLS trust source and precedence.- A shared autouse fixture guarantees no cross-test global-ssl bleed; the per-suite guards in the TLS tests can then be removed.
Activity
github-actions commentedon Jul 6, 2026
Triage decision
acceptProposed labels
Note:
area/docs-siteis a seventh label (over the 6-label soft cap) but is included because the implementing PR must document the newapm doctor tlssubcommand incli-commands.md. Maintainer may drop onearea/*if preferred.Milestone
null-- no open patch or minor milestone confirmed in this sweep.Suggested next action
Implement
apm doctor tlsas a new subcommand (or TLS section) underapm doctor, following the existing doctor-check pattern, and promote the per-suite ssl-isolation guards in the TLS integration tests to a shared autouseconftestfixture.Suggested issue comment
Per-lens notes (collapsed)
DevX UX Expert -- User-Need Reviewer
The request maps cleanly to the existing
apm doctordiagnostic surface (consistent withbrew doctor,npm doctorconventions). The proposed outputs -- active trust source, resolution precedence chain, optional URL probe -- are exactly what a user reaching for a terminal diagnostic needs when TLS fails. The explicit surfacing ofAPM_DISABLE_TRUSTSTORE=1in TLS failure messages closes a discoverability gap. Real user surface: enterprise users hitting TLS failures in corporate environments with custom CA bundles or OS trust store quirks. No UX anti-patterns. Both items are well-scoped and actionable without additional design gates.Supply Chain Security Expert -- Risk-Surface Reviewer
TLS trust-chain configuration is the transport security layer for all package downloads.
APM_DISABLE_TRUSTSTORE,REQUESTS_CA_BUNDLE,CURL_CA_BUNDLE, and OS trust store resolution are security-sensitive inputs; exposing them viaapm doctor tlsis a security-positive change (helps users detect misconfiguration, e.g. unknowingly running with TLS verification disabled). The ssl-isolation fixture prevents global SSL state leaks across 110+ tests -- hygiene improvement, no new attack surface.theme/securityis appropriate.area/testingapplies for the fixture scope. No integrity or supply chain risk introduced.OSS Growth Hacker -- Contributor-Tone Reviewer
Not activated -- author is a COLLABORATOR with established interaction history; first-contributor tone-tuning does not apply.
Python Architect -- Architecture Reviewer
Two bounded scopes. (1)
apm doctor tlssubcommand: extends existingapm doctorcommand; requires adding a new subcommand handler plugging into the doctor command registry. No new abstract base needed -- a function or small class following the existing doctor-check pattern is sufficient. (2) Shared autouse ssl-isolation fixture: the right architectural call for a process-global state problem. With 110+ tests importing the CLI, a sharedconftestautouse fixture (snapshot/restoressl.SSLContext) eliminates the per-suite guard duplication. Both items are withinstatus/acceptedscope with no pre-design step required.Doc Writer -- Documentation Reviewer
The new
apm doctor tlscommand (or TLS section withinapm doctor) is a new CLI surface that requires documentation indocs/src/content/docs/reference/cli-commands.md. The ssl-isolation fixture is an internal test change with no user-facing doc implications.area/docs-siteis included to flag that the implementing PR must update the CLI reference. The feature is user-facing and important for enterprise self-service.APM CEO -- Triage Arbiter
Two well-scoped fast-follow items from the OS trust-store security sprint. Both are clearly accepted:
apm doctor tlsdirectly supports enterprise adoption and reinforces APM's security positioning; the ssl-isolation fixture is overdue test hygiene. The Security Expert'stheme/securitycall is correct -- TLS configuration is the transport security layer for every package download. Priority ispriority/lowsince the critical #2005/#2004 fixes already shipped; these are improvements, not blockers.{ "decision": "accept", "decision_detail": "", "theme": "theme/security", "areas": ["area/cli", "area/testing", "area/docs-site"], "type": "type/feature", "status": "status/accepted", "priority": "priority/low", "preserved_labels": [], "milestone": null, "next_action": "Implement apm doctor tls as a new subcommand under apm doctor and promote per-suite ssl-isolation guards to a shared autouse conftest fixture.", "comment_markdown": "Thanks for the fast-follow write-up -- the red-team surfacing these two items after #2005/#2004 is exactly the right workflow.\n\nBoth items are accepted:\n\n1. `apm doctor tls` (or a TLS section in `apm doctor`): exposing the active trust source, the resolution precedence, and surfacing `APM_DISABLE_TRUSTSTORE=1` in failure messages is directly aligned with APM's \"failure mode is the product\" principle. Enterprise users hitting TLS failures need self-service diagnostics, and the existing `[i] TLS: verifying against ...` log line is the right foundation to build on.\n\n2. Shared autouse ssl-isolation fixture: with 110+ tests importing the CLI and `configure_tls_trust()` patching process-global SSL state, promoting the per-suite guards to a shared `conftest` autouse fixture is the correct architectural call (DRY, no silent cross-test leaks). The existing per-suite guards in the TLS integration suites can then be removed.\n\nThe implementing PR should update `docs/src/content/docs/reference/cli-commands.md` with the new `apm doctor tls` subcommand entry." }danielmeppiel commentedon Sep 12, 2026
At the lead maintainer's direction, the current acceptance of this item is withdrawn as part of APM's roadmap and contribution reset. Previous acceptance labels or comments must not be treated as approval to start or continue substantive implementation for inclusion in APM.
Thank you for the work and ideas already contributed. Our small maintainer team needs to agree supported scope and review capacity before inviting more implementation. This is not a rejection of the proposal or a judgment of your work, and it does not undo completed work or erase its history.
Please follow the contribution rules: a responsible human maintainer needs to record the bounded scope, acceptance criteria and review contact on the issue before substantive work resumes. Existing issue discussion, assignments and contributions remain available for that decision. Trivial typo and broken-link corrections retain their standing preapproval.
danielmeppiel commentedon Oct 21, 2026
Decision: approve
Area: project
Scope: For #2035, accept only the demonstrated correction to core-HTTPS TLS failure guidance. Make the normal error's suggested next action consistent with the trust mode actually selected by the existing canonical TLS owner. Do not recommend OS-store or additive-bundle changes as an effective remedy when the selected opt-out or explicit replacement ignores them. Identify the relevant active setting and a safe next action without changing settings or weakening verification. Preserve already-delivered DEBUG/--verbose TLS selection diagnostics, normal successful output and existing trust-selection semantics.
Done when: Targeted regressions demonstrate the old misleading hint and corrected advice for APM_DISABLE_TRUSTSTORE opt-out, REQUESTS_CA_BUNDLE and CURL_CA_BUNDLE replacement, replacement combined with opt-out, and ignored APM_EXTRA_CA_BUNDLE input. Keep default/OS/additive/certifi-fallback and unavailable-selected-state guidance truthful; do not invent a source or claim the selected setting caused the verification failure. Exercise the real CLI failure-rendering path with isolated settings and controlled failure behavior in normal and verbose modes, preserving failed exit status, invalid-input errors and successful output. Reuse the existing TLS decision owner rather than duplicate precedence or parse diagnostic strings as authority. Retain certificate verification and the accepted #2034 additive/replacement/opt-out boundaries. Update directly affected troubleshooting and usage guidance. Use existing test-isolation mechanisms; no broad fixture refactor is needed for this correction.
Out of scope: New doctor TLS output or subcommands, new logging/configuration infrastructure, general verbose-log cleanup, shared/global SSL-isolation redesign or broad test-fixture consolidation, live URL probes or claims to identify the CA that verified a connection, trust-policy or precedence changes, automatic setting edits, verification bypass, machine trust-store changes, new experimental-runtime propagation, native Git/Rust TLS reconfiguration, unrelated TLS work, contributor replacement, assignment or release commitments, technical approval or merge of #2602, implementation dispatch or restarting blocked delivery.
Review contact: Daniel Meppiel (@danielmeppiel)
danielmeppiel commentedon Oct 21, 2026
Decision: correct misleading failure advice, not add another TLS diagnostic feature
The maintainer has explicitly accepted only the bounded failure-guidance correction recorded separately on this issue.
What is already present
Current APM already reports its selected TLS trust mode through
apm --verbose install/APM_LOG_LEVEL=DEBUG. That includes OS trust, additional CA bundles, explicit replacements, ignored additive settings and fallback reasons. This acceptance does not rebuild that feature or add a TLS section toapm doctor.The concrete remaining defect
The normal TLS failure hint is unconditional. It can suggest a certificate change that the active configuration will ignore:
APM_DISABLE_TRUSTSTORE=1without a replacement bundleAPM_EXTRA_CA_BUNDLE, although this mode opts out of that integration.REQUESTS_CA_BUNDLEorCURL_CA_BUNDLEreplacementThe correction should direct the user to the relevant selected setting or bundle. If an opt-out or override is intentional, preserve it; if it is unintended, explain the safe configuration choice. Do not automatically unset settings, disable verification, or pretend the selected mode proves why the connection failed.
This was verified against main
7e20e044fb501c90279a325a278b69a72ad63367. The exact current selection and failure-message functions were exercised in an isolated process using synthetic opt-out/replacement inputs. They produced the contradictory guidance described above. No external connection, SSL injection or CA-file read was performed; this is a bounded message/selection reproduction, not a live network diagnosis.What completion includes
Keep the existing successful output and verbose diagnostics. Correct the affected failure guidance, add focused regressions and exercise the actual CLI error path with controlled failures. Preserve error exits, invalid-input handling, certificate verification, the canonical selection owner and the additive/replacement semantics already accepted in #2034. Update only directly affected troubleshooting and usage guidance.
Shared SSL test-fixture cleanup is not included. Differing suite-local cleanup was observed, but cross-test contamination was not reproduced; that maintenance work is not a prerequisite for this small correction. New doctor features, probes, logging infrastructure, machine-store changes and new child-runtime or native Git TLS behavior are also excluded.
Existing contribution and authority
The September 12 withdrawal and earlier discussion remain part of the issue history. This new acceptance is limited to the scope above; it does not reinstate every part of the original proposal.
Preserve Aryan Singh K. (@aryansk)'s existing #2602 and its assignment to Sergio Sisternes (@sergio-sisternes-epam). This record does not technically approve that PR, request a branch restart, assign work, dispatch implementation, promise a release or authorize merge. Daniel is the scope review contact; no new commitment is made for another maintainer.
Recorded by Copilot on behalf of Daniel Meppiel (@danielmeppiel) after his explicit scope choice. This explanation is AI-generated and may contain errors.